From aa60e5443b8087c6005972cc30bba9c42db53bc9 Mon Sep 17 00:00:00 2001 From: Christian Beikov Date: Wed, 15 Mar 2017 15:37:36 +0100 Subject: [PATCH] Fixed #190 - Implemented fetchOnly() as a way to specify the fetch base for relations that should be fetched. Fixed date extract test. --- CHANGELOG.md | 1 + .../impl/AbstractCommonQueryBuilder.java | 10 +- .../impl/AbstractFullQueryBuilder.java | 12 -- .../impl/BaseSubqueryBuilderImpl.java | 2 +- ...nalSetOperationCTECriteriaBuilderImpl.java | 5 + .../FinalSetOperationCriteriaBuilderImpl.java | 5 + .../FinalSetOperationSubqueryBuilderImpl.java | 5 + .../persistence/impl/JoinManager.java | 118 +++++++++++++----- .../persistence/impl/JoinTreeNode.java | 4 - .../impl/PaginatedCriteriaBuilderImpl.java | 16 ++- .../persistence/impl/SelectManager.java | 43 ++++--- .../expression/AbstractExpressionFactory.java | 3 +- .../impl/expression/DateLiteral.java | 7 ++ .../impl/expression/EntityLiteral.java | 5 + .../impl/expression/EnumLiteral.java | 5 + .../impl/expression/NumericLiteral.java | 4 + .../impl/expression/StringLiteral.java | 5 + .../impl/expression/TimeLiteral.java | 7 ++ .../impl/expression/TimestampLiteral.java | 7 ++ .../persistence/impl/util/TypeUtils.java | 112 +++++++++++++++++ .../testsuite/DateExtractTest.java | 111 ++++++++++++---- .../persistence/testsuite/JoinTest.java | 78 +++++++++++- .../core/manual/en_US/04_from_clause.adoc | 21 ++++ .../persistence/criteria/JoinTest.java | 6 +- .../testsuite/base/SanePostgreSQLAdapter.java | 50 ++++++++ .../datanucleus/src/main/resources/plugin.xml | 1 + 26 files changed, 548 insertions(+), 95 deletions(-) create mode 100644 testsuite-base/datanucleus/src/main/java/com/blazebit/persistence/testsuite/base/SanePostgreSQLAdapter.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d660c8ab4..3547bc6092 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ Not yet released * Support enum and entity type literal like the JPA spec says * Introduction of `@MappingCorrelatedSimple` for simple correlations * Allow empty correlation result with `JOIN` fetch strategy +* Support for join fetching with scalar selects ### Bug fixes diff --git a/core/impl/src/main/java/com/blazebit/persistence/impl/AbstractCommonQueryBuilder.java b/core/impl/src/main/java/com/blazebit/persistence/impl/AbstractCommonQueryBuilder.java index 949c94c7d5..3add83e4b8 100644 --- a/core/impl/src/main/java/com/blazebit/persistence/impl/AbstractCommonQueryBuilder.java +++ b/core/impl/src/main/java/com/blazebit/persistence/impl/AbstractCommonQueryBuilder.java @@ -159,6 +159,8 @@ public abstract class AbstractCommonQueryBuilder fetchOwners = new HashMap<>(); private boolean checkSetBuilderEnded = true; private boolean implicitJoinsApplied = false; @@ -1392,6 +1394,12 @@ public void visit(JoinNode node) { selectManager.acceptVisitor(joinVisitor); joinVisitor.setJoinRequired(true); + // Only the main query does has fetch owners + if (isMainQuery) { + fetchOwners.clear(); + selectManager.collectFetchOwners(fetchOwners); + } + joinVisitor.setFromClause(ClauseType.WHERE); whereManager.acceptVisitor(joinVisitor); joinVisitor.setFromClause(ClauseType.GROUP_BY); @@ -1818,7 +1826,7 @@ protected void appendSelectClause(StringBuilder sbSelectFrom) { protected List appendFromClause(StringBuilder sbSelectFrom, boolean externalRepresentation) { List whereClauseConjuncts = new ArrayList<>(); - joinManager.buildClause(sbSelectFrom, EnumSet.noneOf(ClauseType.class), null, false, externalRepresentation, whereClauseConjuncts, explicitVersionEntities); + joinManager.buildClause(sbSelectFrom, EnumSet.noneOf(ClauseType.class), null, false, externalRepresentation, whereClauseConjuncts, explicitVersionEntities, fetchOwners); return whereClauseConjuncts; } diff --git a/core/impl/src/main/java/com/blazebit/persistence/impl/AbstractFullQueryBuilder.java b/core/impl/src/main/java/com/blazebit/persistence/impl/AbstractFullQueryBuilder.java index c7add2e576..c76cd30990 100644 --- a/core/impl/src/main/java/com/blazebit/persistence/impl/AbstractFullQueryBuilder.java +++ b/core/impl/src/main/java/com/blazebit/persistence/impl/AbstractFullQueryBuilder.java @@ -176,17 +176,10 @@ private void checkEntityId(Object entityId) { return (FullQueryBuilder) this; } - private void checkFetchJoinAllowed() { - if (selectManager.getSelectInfos().size() > 0) { - throw new IllegalStateException("Fetch joins are only possible if the root entity is selected"); - } - } - @Override @SuppressWarnings("unchecked") public X fetch(String path) { prepareForModification(); - checkFetchJoinAllowed(); verifyBuilderEnded(); joinManager.implicitJoin(expressionFactory.createPathExpression(path), true, null, null, false, false, true, true); return (X) this; @@ -196,7 +189,6 @@ public X fetch(String path) { @SuppressWarnings("unchecked") public X fetch(String... paths) { prepareForModification(); - checkFetchJoinAllowed(); verifyBuilderEnded(); for (String path : paths) { @@ -262,10 +254,6 @@ private X join(String path, String alias, JoinType type, boolean fetch, boolean throw new IllegalArgumentException("Empty alias"); } - if (fetch == true) { - checkFetchJoinAllowed(); - } - verifyBuilderEnded(); joinManager.join(path, alias, type, fetch, defaultJoin); return (X) this; diff --git a/core/impl/src/main/java/com/blazebit/persistence/impl/BaseSubqueryBuilderImpl.java b/core/impl/src/main/java/com/blazebit/persistence/impl/BaseSubqueryBuilderImpl.java index 6f5375981e..209abb1cd6 100644 --- a/core/impl/src/main/java/com/blazebit/persistence/impl/BaseSubqueryBuilderImpl.java +++ b/core/impl/src/main/java/com/blazebit/persistence/impl/BaseSubqueryBuilderImpl.java @@ -72,7 +72,7 @@ public Set getCorrelatedExpressions() { public T getResult() { return result; } - + protected BaseFinalSetOperationSubqueryBuilderImpl createFinalSetOperationBuilder(SetOperationType operator, boolean nested, boolean isSubquery) { SubqueryBuilderImpl newInitiator = finalSetOperationBuilder == null ? null : finalSetOperationBuilder.getInitiator(); return createFinalSetOperationBuilder(operator, nested, isSubquery, newInitiator); diff --git a/core/impl/src/main/java/com/blazebit/persistence/impl/FinalSetOperationCTECriteriaBuilderImpl.java b/core/impl/src/main/java/com/blazebit/persistence/impl/FinalSetOperationCTECriteriaBuilderImpl.java index b90d3fc7f0..a6d7bbbd42 100644 --- a/core/impl/src/main/java/com/blazebit/persistence/impl/FinalSetOperationCTECriteriaBuilderImpl.java +++ b/core/impl/src/main/java/com/blazebit/persistence/impl/FinalSetOperationCTECriteriaBuilderImpl.java @@ -31,6 +31,11 @@ public FinalSetOperationCTECriteriaBuilderImpl(MainQuery mainQuery, Class cla super(mainQuery, clazz, result, operator, nested, listener, initiator); } + @Override + protected void applyImplicitJoins() { + // There is nothing to do here for final builders as they don't have any nodes + } + @Override public T end() { subListener.verifyBuilderEnded(); diff --git a/core/impl/src/main/java/com/blazebit/persistence/impl/FinalSetOperationCriteriaBuilderImpl.java b/core/impl/src/main/java/com/blazebit/persistence/impl/FinalSetOperationCriteriaBuilderImpl.java index 352deb731e..e4c5d11316 100644 --- a/core/impl/src/main/java/com/blazebit/persistence/impl/FinalSetOperationCriteriaBuilderImpl.java +++ b/core/impl/src/main/java/com/blazebit/persistence/impl/FinalSetOperationCriteriaBuilderImpl.java @@ -31,4 +31,9 @@ public class FinalSetOperationCriteriaBuilderImpl extends BaseFinalSetOperati public FinalSetOperationCriteriaBuilderImpl(MainQuery mainQuery, boolean isMainQuery, Class clazz, SetOperationType operator, boolean nested, BuilderListener listener) { super(mainQuery, isMainQuery, clazz, operator, nested, listener, null); } + + @Override + protected void applyImplicitJoins() { + // There is nothing to do here for final builders as they don't have any nodes + } } diff --git a/core/impl/src/main/java/com/blazebit/persistence/impl/FinalSetOperationSubqueryBuilderImpl.java b/core/impl/src/main/java/com/blazebit/persistence/impl/FinalSetOperationSubqueryBuilderImpl.java index af58f5439e..21fa698d1b 100644 --- a/core/impl/src/main/java/com/blazebit/persistence/impl/FinalSetOperationSubqueryBuilderImpl.java +++ b/core/impl/src/main/java/com/blazebit/persistence/impl/FinalSetOperationSubqueryBuilderImpl.java @@ -31,6 +31,11 @@ public FinalSetOperationSubqueryBuilderImpl(MainQuery mainQuery, T result, SetOp super(mainQuery, result, operator, nested, listener, initiator); } + @Override + protected void applyImplicitJoins() { + // There is nothing to do here for final builders as they don't have any nodes + } + @Override public T end() { subListener.verifySubqueryBuilderEnded(); diff --git a/core/impl/src/main/java/com/blazebit/persistence/impl/JoinManager.java b/core/impl/src/main/java/com/blazebit/persistence/impl/JoinManager.java index 0e9ce7a40e..cf484a493c 100644 --- a/core/impl/src/main/java/com/blazebit/persistence/impl/JoinManager.java +++ b/core/impl/src/main/java/com/blazebit/persistence/impl/JoinManager.java @@ -68,6 +68,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.IdentityHashMap; import java.util.Iterator; @@ -606,8 +607,12 @@ void removeRoot() { } JoinNode getRootNodeOrFail(String string) { + return getRootNodeOrFail(string, "", ""); + } + + JoinNode getRootNodeOrFail(String prefix, Object middle, String suffix) { if (rootNodes.size() > 1) { - throw new IllegalArgumentException(string); + throw new IllegalArgumentException(prefix + middle + suffix); } return rootNodes.get(0); @@ -701,8 +706,9 @@ public SubqueryInitiatorFactory getSubqueryInitFactory() { return subqueryInitFactory; } - Set buildClause(StringBuilder sb, Set clauseExclusions, String aliasPrefix, boolean collectCollectionJoinNodes, boolean externalRepresenation, List whereConjuncts, Map, Map> explicitVersionEntities) { + Set buildClause(StringBuilder sb, Set clauseExclusions, String aliasPrefix, boolean collectCollectionJoinNodes, boolean externalRepresenation, List whereConjuncts, Map, Map> explicitVersionEntities, Map originalFetchOwners) { final boolean renderFetches = !clauseExclusions.contains(ClauseType.SELECT); + final Map fetchOwners = new HashMap<>(originalFetchOwners); StringBuilder tempSb = null; collectionJoinNodes.clear(); renderedJoins.clear(); @@ -717,6 +723,11 @@ Set buildClause(StringBuilder sb, Set clauseExclusions, St } JoinNode rootNode = nodes.get(i); + + if (fetchOwners.remove(rootNode) == Boolean.FALSE) { + allowChildFetching(rootNode, fetchOwners); + } + JoinNode correlationParent = rootNode.getCorrelationParent(); if (externalRepresenation && rootNode.getValueCount() > 0) { @@ -774,12 +785,12 @@ Set buildClause(StringBuilder sb, Set clauseExclusions, St // TODO: not sure if needed since applyImplicitJoins will already invoke that rootNode.registerDependencies(); - applyJoins(sb, rootNode.getAliasInfo(), rootNode.getNodes(), clauseExclusions, aliasPrefix, collectCollectionJoinNodes, renderFetches); + applyJoins(sb, rootNode.getAliasInfo(), rootNode.getNodes(), clauseExclusions, aliasPrefix, collectCollectionJoinNodes, renderFetches, fetchOwners); if (!rootNode.getEntityJoinNodes().isEmpty()) { // TODO: Fix this with #216 boolean isCollection = true; if (mainQuery.jpaProvider.supportsEntityJoin()) { - applyJoins(sb, rootNode.getAliasInfo(), new ArrayList(rootNode.getEntityJoinNodes()), isCollection, clauseExclusions, aliasPrefix, collectCollectionJoinNodes, renderFetches); + applyJoins(sb, rootNode.getAliasInfo(), new ArrayList(rootNode.getEntityJoinNodes()), isCollection, clauseExclusions, aliasPrefix, collectCollectionJoinNodes, renderFetches, fetchOwners); } else { Set entityNodes = rootNode.getEntityJoinNodes(); for (JoinNode entityNode : entityNodes) { @@ -818,15 +829,44 @@ Set buildClause(StringBuilder sb, Set clauseExclusions, St } renderedJoins.add(entityNode); - applyJoins(sb, entityNode.getAliasInfo(), entityNode.getNodes(), clauseExclusions, aliasPrefix, collectCollectionJoinNodes, renderFetches); + applyJoins(sb, entityNode.getAliasInfo(), entityNode.getNodes(), clauseExclusions, aliasPrefix, collectCollectionJoinNodes, renderFetches, fetchOwners); } } } } + checkFetchOwners(fetchOwners); return collectionJoinNodes; } + private void checkFetchOwners(Map fetchOwners) { + StringBuilder sb = null; + for (Map.Entry entry : fetchOwners.entrySet()) { + if (entry.getValue() == Boolean.FALSE) { + if (sb == null) { + sb = new StringBuilder(); + sb.append("Some join nodes specified fetch joining but their fetch owners weren't included in the select clause! Missing fetch owners: ["); + } else { + sb.append(", "); + } + sb.append(entry.getKey().getAlias()); + } + } + + if (sb != null) { + sb.append("]"); + throw new IllegalStateException(sb.toString()); + } + } + + private void allowChildFetching(JoinNode node, Map fetchOwners) { + for (JoinTreeNode childTreeNode : node.getNodes().values()) { + for (JoinNode childNode : childTreeNode.getJoinNodes().values()) { + fetchOwners.put(childNode, Boolean.TRUE); + } + } + } + void verifyBuilderEnded() { joinOnBuilderListener.verifyBuilderEnded(); } @@ -862,7 +902,7 @@ public void apply(ExpressionModifierVisitor visitor) } } - private void renderJoinNode(StringBuilder sb, JoinAliasInfo joinBase, JoinNode node, String aliasPrefix, boolean renderFetches) { + private void renderJoinNode(StringBuilder sb, JoinAliasInfo joinBase, JoinNode node, String aliasPrefix, boolean renderFetches, Map fetchOwners) { if (!renderedJoins.contains(node)) { final boolean fetch = node.isFetch() && renderFetches; // Don't render key joins unless fetching is specified on it @@ -883,8 +923,24 @@ private void renderJoinNode(StringBuilder sb, JoinAliasInfo joinBase, JoinNode n default: throw new IllegalArgumentException("Unknown join type: " + node.getJoinType()); } + + // We always remove the fetch owner, otherwise we'd get an error for non-fetched fetch owners + Boolean fetchable = fetchOwners.remove(node); if (fetch) { - sb.append("FETCH "); + if (fetchable == null) { + // This is a join node with fetching that isn't selected + // Signal this wrong fetching by putting it's fetch owner into the map + fetchOwners.put(getFetchOwner(node), Boolean.FALSE); + } else { + if (fetchable == Boolean.FALSE) { + // This is a fetch owner, so don't render a fetch join + } else { + sb.append("FETCH "); + } + + // All children of a fetch joined node are allowed to be fetch joined too + allowChildFetching(node, fetchOwners); + } } if (aliasPrefix != null) { @@ -962,9 +1018,9 @@ private void renderParentAlias(StringBuilder sb, JoinNode parentNode, String ali } } - private void renderReverseDependency(StringBuilder sb, JoinNode dependency, String aliasPrefix, boolean renderFetches) { + private void renderReverseDependency(StringBuilder sb, JoinNode dependency, String aliasPrefix, boolean renderFetches, Map fetchOwners) { if (dependency.getParent() != null) { - renderReverseDependency(sb, dependency.getParent(), aliasPrefix, renderFetches); + renderReverseDependency(sb, dependency.getParent(), aliasPrefix, renderFetches, fetchOwners); if (!dependency.getDependencies().isEmpty()) { markedJoinNodes.add(dependency); try { @@ -974,27 +1030,27 @@ private void renderReverseDependency(StringBuilder sb, JoinNode dependency, Stri + dep.getAliasInfo().getAbsolutePath() + "] with alias [" + dep.getAliasInfo().getAlias() + "]"); } // render reverse dependencies - renderReverseDependency(sb, dep, aliasPrefix, renderFetches); + renderReverseDependency(sb, dep, aliasPrefix, renderFetches, fetchOwners); } } finally { markedJoinNodes.remove(dependency); } } - renderJoinNode(sb, dependency.getParent().getAliasInfo(), dependency, aliasPrefix, renderFetches); + renderJoinNode(sb, dependency.getParent().getAliasInfo(), dependency, aliasPrefix, renderFetches, fetchOwners); } } - private void applyJoins(StringBuilder sb, JoinAliasInfo joinBase, Map nodes, Set clauseExclusions, String aliasPrefix, boolean collectCollectionJoinNodes, boolean renderFetches) { + private void applyJoins(StringBuilder sb, JoinAliasInfo joinBase, Map nodes, Set clauseExclusions, String aliasPrefix, boolean collectCollectionJoinNodes, boolean renderFetches, Map fetchOwners) { for (Map.Entry nodeEntry : nodes.entrySet()) { JoinTreeNode treeNode = nodeEntry.getValue(); List stack = new ArrayList(); stack.addAll(treeNode.getJoinNodes().descendingMap().values()); - applyJoins(sb, joinBase, stack, treeNode.isCollection(), clauseExclusions, aliasPrefix, collectCollectionJoinNodes, renderFetches); + applyJoins(sb, joinBase, stack, treeNode.isCollection(), clauseExclusions, aliasPrefix, collectCollectionJoinNodes, renderFetches, fetchOwners); } } - private void applyJoins(StringBuilder sb, JoinAliasInfo joinBase, List stack, boolean isCollection, Set clauseExclusions, String aliasPrefix, boolean collectCollectionJoinNodes, boolean renderFetches) { + private void applyJoins(StringBuilder sb, JoinAliasInfo joinBase, List stack, boolean isCollection, Set clauseExclusions, String aliasPrefix, boolean collectCollectionJoinNodes, boolean renderFetches, Map fetchOwners) { while (!stack.isEmpty()) { JoinNode node = stack.remove(stack.size() - 1); // If the clauses in which a join node occurs are all excluded or the join node is not mandatory for the cardinality, we skip it @@ -1006,7 +1062,7 @@ private void applyJoins(StringBuilder sb, JoinAliasInfo joinBase, List // We have to render any dependencies this join node has before actually rendering itself if (!node.getDependencies().isEmpty()) { - renderReverseDependency(sb, node, aliasPrefix, renderFetches); + renderReverseDependency(sb, node, aliasPrefix, renderFetches, fetchOwners); } // Collect the join nodes referring to collections @@ -1015,11 +1071,11 @@ private void applyJoins(StringBuilder sb, JoinAliasInfo joinBase, List } // Finally render this join node - renderJoinNode(sb, joinBase, node, aliasPrefix, renderFetches); + renderJoinNode(sb, joinBase, node, aliasPrefix, renderFetches, fetchOwners); // Render child nodes recursively if (!node.getNodes().isEmpty()) { - applyJoins(sb, node.getAliasInfo(), node.getNodes(), clauseExclusions, aliasPrefix, collectCollectionJoinNodes, renderFetches); + applyJoins(sb, node.getAliasInfo(), node.getNodes(), clauseExclusions, aliasPrefix, collectCollectionJoinNodes, renderFetches, fetchOwners); } } } @@ -1247,7 +1303,7 @@ JoinNode join(String path, String alias, JoinType type, boolean fetch, boolean d } else { List joinRelationAttributes = result.addToList(new ArrayList()); joinRelationAttributes.add(elementExpr.toString()); - current = current == null ? getRootNodeOrFail("Could not join path [" + path + "] because it did not use an absolute path but multiple root nodes are available!") : current; + current = current == null ? getRootNodeOrFail("Could not join path [", path, "] because it did not use an absolute path but multiple root nodes are available!") : current; result = createOrUpdateNode(current, result.typeName, joinRelationAttributes, treatType, alias, type, false, defaultJoin); } } @@ -1402,11 +1458,7 @@ public void implicitJoin(Expression expression, boolean objectLeafAllowed, Strin } else { // current might be null if (current == null) { - if (rootNodes.size() > 1) { - throw new IllegalArgumentException("Could not join path [" + expression + "] because it did not use an absolute path but multiple root nodes are available!"); - } - - current = rootNodes.get(0); + current = getRootNodeOrFail("Could not join path [", expression, "] because it did not use an absolute path but multiple root nodes are available!"); } if (singleValuedAssociationIdExpression) { @@ -1522,11 +1574,21 @@ public void implicitJoin(Expression expression, boolean objectLeafAllowed, Strin } else if (expression instanceof QualifiedExpression) { implicitJoin(((QualifiedExpression) expression).getPath(), objectLeafAllowed, null, fromClause, fromSubquery, fromSelectAlias, joinRequired); } else if (expression instanceof ArrayExpression || expression instanceof GeneralCaseExpression || expression instanceof TreatExpression) { + // TODO: Having a treat expression actually makes sense here for fetchOnly // NOTE: I haven't found a use case for this yet, so I'd like to throw an exception instead of silently not supporting this throw new IllegalArgumentException("Unsupported expression for implicit joining found: " + expression); + } else { + // Other expressions don't need handling } } + private JoinNode getFetchOwner(JoinNode node) { + while (node.isFetch()) { + node = node.getParent(); + } + return node; + } + private static class LazyPathReference implements PathReference { private final JoinNode baseNode; private final String field; @@ -1635,7 +1697,7 @@ private boolean isSingleValuedAssociationId(JoinResult joinResult, List getJoinNodes() { return joinNodes; } diff --git a/core/impl/src/main/java/com/blazebit/persistence/impl/PaginatedCriteriaBuilderImpl.java b/core/impl/src/main/java/com/blazebit/persistence/impl/PaginatedCriteriaBuilderImpl.java index ee79ee36f6..904fd2532d 100644 --- a/core/impl/src/main/java/com/blazebit/persistence/impl/PaginatedCriteriaBuilderImpl.java +++ b/core/impl/src/main/java/com/blazebit/persistence/impl/PaginatedCriteriaBuilderImpl.java @@ -491,8 +491,10 @@ private String buildPageCountQueryString(StringBuilder sbSelectFrom, boolean ext } List whereClauseConjuncts = new ArrayList<>(); + // The count query does not have any fetch owners + Map countFetchOwners = Collections.emptyMap(); // Collect usage of collection join nodes to optimize away the count distinct - Set collectionJoinNodes = joinManager.buildClause(sbSelectFrom, EnumSet.of(ClauseType.ORDER_BY, ClauseType.SELECT), null, true, externalRepresentation, whereClauseConjuncts, explicitVersionEntities); + Set collectionJoinNodes = joinManager.buildClause(sbSelectFrom, EnumSet.of(ClauseType.ORDER_BY, ClauseType.SELECT), null, true, externalRepresentation, whereClauseConjuncts, explicitVersionEntities, countFetchOwners); // TODO: Maybe we can improve this and treat array access joins like non-collection join nodes boolean hasCollectionJoinUsages = collectionJoinNodes.size() > 0; @@ -533,7 +535,9 @@ private String appendSimplePageIdQueryString(StringBuilder sbSelectFrom) { // TODO: if we do so, the page position function has to omit select items other than the first List whereClauseConjuncts = new ArrayList<>(); - joinManager.buildClause(sbSelectFrom, EnumSet.of(ClauseType.SELECT), PAGE_POSITION_ID_QUERY_ALIAS_PREFIX, false, false, whereClauseConjuncts, explicitVersionEntities); + // The id query does not have any fetch owners + Map idFetchOwners = Collections.emptyMap(); + joinManager.buildClause(sbSelectFrom, EnumSet.of(ClauseType.SELECT), PAGE_POSITION_ID_QUERY_ALIAS_PREFIX, false, false, whereClauseConjuncts, explicitVersionEntities, idFetchOwners); whereManager.buildClause(sbSelectFrom, whereClauseConjuncts); boolean inverseOrder = false; @@ -572,7 +576,9 @@ private String buildPageIdQueryString(StringBuilder sbSelectFrom, boolean extern } List whereClauseConjuncts = new ArrayList<>(); - joinManager.buildClause(sbSelectFrom, EnumSet.of(ClauseType.SELECT), null, false, externalRepresentation, whereClauseConjuncts, explicitVersionEntities); + // The id query does not have any fetch owners + Map idFetchOwners = Collections.emptyMap(); + joinManager.buildClause(sbSelectFrom, EnumSet.of(ClauseType.SELECT), null, false, externalRepresentation, whereClauseConjuncts, explicitVersionEntities, idFetchOwners); if (keysetMode == KeysetMode.NONE) { whereManager.buildClause(sbSelectFrom, whereClauseConjuncts); @@ -627,7 +633,7 @@ protected void buildBaseQueryString(StringBuilder sbSelectFrom, boolean external * ORDER_BY clause do not depend on */ List whereClauseConjuncts = new ArrayList<>(); - joinManager.buildClause(sbSelectFrom, EnumSet.complementOf(EnumSet.of(ClauseType.SELECT, ClauseType.ORDER_BY)), null, false, externalRepresentation, whereClauseConjuncts, explicitVersionEntities); + joinManager.buildClause(sbSelectFrom, EnumSet.complementOf(EnumSet.of(ClauseType.SELECT, ClauseType.ORDER_BY)), null, false, externalRepresentation, whereClauseConjuncts, explicitVersionEntities, fetchOwners); sbSelectFrom.append(" WHERE "); rootNode.appendAlias(sbSelectFrom, idName); sbSelectFrom.append(" IN :").append(ID_PARAM_NAME).append(""); @@ -690,7 +696,7 @@ private String buildObjectQueryString(StringBuilder sbSelectFrom, boolean extern } List whereClauseConjuncts = new ArrayList<>(); - joinManager.buildClause(sbSelectFrom, EnumSet.noneOf(ClauseType.class), null, false, externalRepresentation, whereClauseConjuncts, explicitVersionEntities); + joinManager.buildClause(sbSelectFrom, EnumSet.noneOf(ClauseType.class), null, false, externalRepresentation, whereClauseConjuncts, explicitVersionEntities, fetchOwners); if (keysetMode == KeysetMode.NONE) { whereManager.buildClause(sbSelectFrom, whereClauseConjuncts); diff --git a/core/impl/src/main/java/com/blazebit/persistence/impl/SelectManager.java b/core/impl/src/main/java/com/blazebit/persistence/impl/SelectManager.java index 41c97ee665..0f296957ad 100644 --- a/core/impl/src/main/java/com/blazebit/persistence/impl/SelectManager.java +++ b/core/impl/src/main/java/com/blazebit/persistence/impl/SelectManager.java @@ -130,6 +130,32 @@ public boolean containsSizeSelect() { return hasSizeSelect; } + public void collectFetchOwners(Map fetchOwners) { + List infos = selectInfos; + int size = selectInfos.size(); + + for (int i = 0; i < size; i++) { + final SelectInfo selectInfo = infos.get(i); + Expression expression = selectInfo.getExpression(); + if (!(expression instanceof PathExpression)) { + // We only look for entity selects and those can only be path expressions + continue; + } + PathExpression pathExpression = (PathExpression) expression; + JoinNode node = (JoinNode) pathExpression.getBaseNode(); + if (pathExpression.getField() == null) { + fetchOwners.put(node, Boolean.FALSE); + } + } + + if (size == 0) { + fetchOwners.put( + joinManager.getRootNodeOrFail("Empty select not allowed when having multiple roots!"), + Boolean.FALSE + ); + } + } + void acceptVisitor(Visitor v) { List infos = selectInfos; int size = selectInfos.size(); @@ -168,13 +194,7 @@ void buildGroupByClauses(final EntityMetamodel m, Set clauses) { // When no select infos are available, it can only be a root entity select if (selectInfos.isEmpty()) { // TODO: GroupByTest#testGroupByEntitySelect uses this. It's problematic because it's not aware of VALUES clause - List roots = joinManager.getRoots(); - - if (roots.size() > 1) { - throw new IllegalArgumentException("Empty select not allowed when having multiple roots!"); - } - - JoinNode rootNode = roots.get(0); + JoinNode rootNode = joinManager.getRootNodeOrFail("Empty select not allowed when having multiple roots!"); String rootAlias = rootNode.getAliasInfo().getAlias(); List path = Arrays.asList((PathElementExpression) new PropertyExpression(rootAlias)); @@ -231,13 +251,8 @@ void buildSelect(StringBuilder sb, boolean isInsertInto) { List infos = selectInfos; int size = selectInfos.size(); if (size == 0) { - List roots = joinManager.getRoots(); - - if (roots.size() > 1) { - throw new IllegalArgumentException("Empty select not allowed when having multiple roots!"); - } - - roots.get(0).appendAlias(sb, null); + JoinNode rootNode = joinManager.getRootNodeOrFail("Empty select not allowed when having multiple roots!"); + rootNode.appendAlias(sb, null); } else { // we must not replace select alias since we would loose the original expressions queryGenerator.setQueryBuffer(sb); diff --git a/core/parser/src/main/java/com/blazebit/persistence/impl/expression/AbstractExpressionFactory.java b/core/parser/src/main/java/com/blazebit/persistence/impl/expression/AbstractExpressionFactory.java index abcfdb3b19..040bfb218a 100644 --- a/core/parser/src/main/java/com/blazebit/persistence/impl/expression/AbstractExpressionFactory.java +++ b/core/parser/src/main/java/com/blazebit/persistence/impl/expression/AbstractExpressionFactory.java @@ -108,14 +108,13 @@ private Expression createExpression(RuleInvoker ruleInvoker, String expression, @Override public PathExpression createPathExpression(String expression, MacroConfiguration macroConfiguration) { - PathExpression path = (PathExpression) createExpression(new RuleInvoker() { + return (PathExpression) createExpression(new RuleInvoker() { @Override public ParserRuleContext invokeRule(JPQLSelectExpressionParser parser) { return parser.parsePath(); } }, expression, false, false, false, macroConfiguration); - return path; } @Override diff --git a/core/parser/src/main/java/com/blazebit/persistence/impl/expression/DateLiteral.java b/core/parser/src/main/java/com/blazebit/persistence/impl/expression/DateLiteral.java index a0cf82d534..c3a48e421f 100644 --- a/core/parser/src/main/java/com/blazebit/persistence/impl/expression/DateLiteral.java +++ b/core/parser/src/main/java/com/blazebit/persistence/impl/expression/DateLiteral.java @@ -16,6 +16,8 @@ package com.blazebit.persistence.impl.expression; +import com.blazebit.persistence.impl.util.TypeUtils; + import java.util.Date; /** @@ -43,4 +45,9 @@ public void accept(Visitor visitor) { public T accept(ResultVisitor visitor) { return visitor.visit(this); } + + @Override + public String toString() { + return TypeUtils.DATE_AS_DATE_CONVERTER.toString(value); + } } diff --git a/core/parser/src/main/java/com/blazebit/persistence/impl/expression/EntityLiteral.java b/core/parser/src/main/java/com/blazebit/persistence/impl/expression/EntityLiteral.java index 98d65f1392..de62aad607 100644 --- a/core/parser/src/main/java/com/blazebit/persistence/impl/expression/EntityLiteral.java +++ b/core/parser/src/main/java/com/blazebit/persistence/impl/expression/EntityLiteral.java @@ -73,4 +73,9 @@ public boolean equals(Object o) { public int hashCode() { return value != null ? value.hashCode() : 0; } + + @Override + public String toString() { + return originalExpression; + } } diff --git a/core/parser/src/main/java/com/blazebit/persistence/impl/expression/EnumLiteral.java b/core/parser/src/main/java/com/blazebit/persistence/impl/expression/EnumLiteral.java index d743d10bad..390fa8c18e 100644 --- a/core/parser/src/main/java/com/blazebit/persistence/impl/expression/EnumLiteral.java +++ b/core/parser/src/main/java/com/blazebit/persistence/impl/expression/EnumLiteral.java @@ -73,4 +73,9 @@ public boolean equals(Object o) { public int hashCode() { return value != null ? value.hashCode() : 0; } + + @Override + public String toString() { + return originalExpression; + } } diff --git a/core/parser/src/main/java/com/blazebit/persistence/impl/expression/NumericLiteral.java b/core/parser/src/main/java/com/blazebit/persistence/impl/expression/NumericLiteral.java index 4579028d78..26bb239d00 100644 --- a/core/parser/src/main/java/com/blazebit/persistence/impl/expression/NumericLiteral.java +++ b/core/parser/src/main/java/com/blazebit/persistence/impl/expression/NumericLiteral.java @@ -74,5 +74,9 @@ public int hashCode() { return result; } + @Override + public String toString() { + return value; + } } diff --git a/core/parser/src/main/java/com/blazebit/persistence/impl/expression/StringLiteral.java b/core/parser/src/main/java/com/blazebit/persistence/impl/expression/StringLiteral.java index 6563c9031b..2360de1255 100644 --- a/core/parser/src/main/java/com/blazebit/persistence/impl/expression/StringLiteral.java +++ b/core/parser/src/main/java/com/blazebit/persistence/impl/expression/StringLiteral.java @@ -67,4 +67,9 @@ public boolean equals(Object o) { public int hashCode() { return value != null ? value.hashCode() : 0; } + + @Override + public String toString() { + return '\'' + value + '\''; + } } diff --git a/core/parser/src/main/java/com/blazebit/persistence/impl/expression/TimeLiteral.java b/core/parser/src/main/java/com/blazebit/persistence/impl/expression/TimeLiteral.java index 6ec6054733..30227b0754 100644 --- a/core/parser/src/main/java/com/blazebit/persistence/impl/expression/TimeLiteral.java +++ b/core/parser/src/main/java/com/blazebit/persistence/impl/expression/TimeLiteral.java @@ -16,6 +16,8 @@ package com.blazebit.persistence.impl.expression; +import com.blazebit.persistence.impl.util.TypeUtils; + import java.util.Date; /** @@ -44,4 +46,9 @@ public T accept(ResultVisitor visitor) { return visitor.visit(this); } + @Override + public String toString() { + return TypeUtils.DATE_AS_TIME_CONVERTER.toString(value); + } + } diff --git a/core/parser/src/main/java/com/blazebit/persistence/impl/expression/TimestampLiteral.java b/core/parser/src/main/java/com/blazebit/persistence/impl/expression/TimestampLiteral.java index afa59e9e69..9e14df1c20 100644 --- a/core/parser/src/main/java/com/blazebit/persistence/impl/expression/TimestampLiteral.java +++ b/core/parser/src/main/java/com/blazebit/persistence/impl/expression/TimestampLiteral.java @@ -16,6 +16,8 @@ package com.blazebit.persistence.impl.expression; +import com.blazebit.persistence.impl.util.TypeUtils; + import java.util.Date; /** @@ -43,4 +45,9 @@ public void accept(Visitor visitor) { public T accept(ResultVisitor visitor) { return visitor.visit(this); } + + @Override + public String toString() { + return TypeUtils.TIMESTAMP_CONVERTER.toString(value); + } } diff --git a/core/parser/src/main/java/com/blazebit/persistence/impl/util/TypeUtils.java b/core/parser/src/main/java/com/blazebit/persistence/impl/util/TypeUtils.java index 1bd55e13e4..d3cc60279f 100644 --- a/core/parser/src/main/java/com/blazebit/persistence/impl/util/TypeUtils.java +++ b/core/parser/src/main/java/com/blazebit/persistence/impl/util/TypeUtils.java @@ -267,6 +267,35 @@ public String toString(java.sql.Time value) { } }; + public static final TypeConverter DATE_AS_TIME_CONVERTER = new AbstractTypeConverter() { + + private static final long serialVersionUID = 1L; + + @SuppressWarnings({ "deprecation" }) + public java.util.Date convert(Object value) { + if (value == null) { + return null; + } + if (value instanceof java.util.Date) { + java.util.Date date = (java.util.Date) value; + java.sql.Time result = new java.sql.Time(date.getHours(), date.getMinutes(), date.getSeconds()); + return result; + } else if (value instanceof java.util.Calendar) { + java.util.Calendar calendar = (java.util.Calendar) value; + java.sql.Time result = new java.sql.Time(calendar.get(Calendar.HOUR_OF_DAY), calendar.get(Calendar.MINUTE), calendar.get(Calendar.SECOND)); + return result; + } else if (value instanceof String) { + return java.sql.Time.valueOf((String) value); + } + throw unknownConversion(value, java.sql.Time.class); + } + + @Override + public String toString(java.util.Date value) { + return jdbcTime(value.getHours(), value.getMinutes(), value.getSeconds()); + } + }; + public static final TypeConverter DATE_CONVERTER = new AbstractTypeConverter() { private static final long serialVersionUID = 1L; @@ -296,6 +325,35 @@ public String toString(java.sql.Date value) { } }; + public static final TypeConverter DATE_AS_DATE_CONVERTER = new AbstractTypeConverter() { + + private static final long serialVersionUID = 1L; + + @SuppressWarnings({ "deprecation" }) + public java.sql.Date convert(Object value) { + if (value == null) { + return null; + } + if (value instanceof java.util.Date) { + java.util.Date date = (java.util.Date) value; + java.sql.Date result = new java.sql.Date(date.getYear(), date.getMonth(), date.getDate()); + return result; + } else if (value instanceof java.util.Calendar) { + java.util.Calendar calendar = (java.util.Calendar) value; + java.sql.Date result = new java.sql.Date(calendar.get(Calendar.YEAR), calendar.get(Calendar.MONTH), calendar.get(Calendar.DATE)); + return result; + } else if (value instanceof String) { + return java.sql.Date.valueOf((String) value); + } + throw unknownConversion(value, java.sql.Date.class); + } + + @Override + public String toString(java.util.Date value) { + return jdbcDate(value.getYear() + 1900, value.getMonth() + 1, value.getDate()); + } + }; + public static final TypeConverter TIMESTAMP_CONVERTER = new AbstractTypeConverter() { private static final long serialVersionUID = 1L; @@ -436,6 +494,60 @@ private static String jdbcTimestamp(int year, int month, int date, int hour, int return sb.toString(); } + private static String jdbcDate(int year, int month, int date) { + StringBuilder sb = new StringBuilder(16); + + sb.append("{d '"); + + sb.append(year); + sb.append('-'); + + if (month < 10) { + sb.append('0'); + } + + sb.append(month); + sb.append('-'); + + if (date < 10) { + sb.append('0'); + } + + sb.append(date); + + sb.append("'}"); + return sb.toString(); + } + + private static String jdbcTime(int hour, int minute, int second) { + StringBuilder sb = new StringBuilder(14); + + sb.append("{t '"); + + if (hour < 10) { + sb.append('0'); + } + + sb.append(hour); + sb.append(':'); + + if (minute < 10) { + sb.append('0'); + } + + sb.append(minute); + sb.append(':'); + + if (second < 10) { + sb.append('0'); + } + + sb.append(second); + + sb.append("'}"); + return sb.toString(); + } + static { Map, TypeConverter> c = new HashMap, TypeConverter>(); c.put(String.class, STRING_CONVERTER); diff --git a/core/testsuite/src/test/java/com/blazebit/persistence/testsuite/DateExtractTest.java b/core/testsuite/src/test/java/com/blazebit/persistence/testsuite/DateExtractTest.java index 41025a14b7..6fc6e448fc 100644 --- a/core/testsuite/src/test/java/com/blazebit/persistence/testsuite/DateExtractTest.java +++ b/core/testsuite/src/test/java/com/blazebit/persistence/testsuite/DateExtractTest.java @@ -18,7 +18,10 @@ import static org.junit.Assert.assertEquals; +import java.lang.reflect.Field; +import java.util.Arrays; import java.util.Calendar; +import java.util.Collection; import java.util.List; import java.util.TimeZone; @@ -26,6 +29,7 @@ import javax.persistence.Tuple; import com.blazebit.persistence.testsuite.tx.TxVoidWork; +import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.experimental.categories.Category; @@ -35,6 +39,8 @@ import com.blazebit.persistence.testsuite.entity.Document; import com.blazebit.persistence.testsuite.entity.Person; import com.blazebit.persistence.testsuite.entity.Version; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; /** * @@ -42,17 +48,47 @@ * @author Moritz Becker * @since 1.1.0 */ +@RunWith(Parameterized.class) public class DateExtractTest extends AbstractCoreTest { - + + private final TimeZone dbmsTimeZone = TimeZone.getDefault(); private Calendar c1; - private Calendar c1UTC0; private Calendar c2; - private Calendar c2UTC0; - private Document doc1; + private TimeZone producerTimeZone; + private TimeZone clientTimeZone; + + public DateExtractTest(TimeZone producerTimeZone, TimeZone clientTimeZone) { + this.producerTimeZone = producerTimeZone; + this.clientTimeZone = clientTimeZone; + } - public DateExtractTest() { - TimeZone.setDefault(TimeZone.getTimeZone("GMT+00:00")); + @Parameterized.Parameters + public static Collection producerConsumerTimezones() { + return Arrays.asList( + new Object[]{ TimeZone.getTimeZone("GMT+00:00"), TimeZone.getTimeZone("GMT+00:00") }, + new Object[]{ TimeZone.getTimeZone("GMT+00:00"), TimeZone.getTimeZone("GMT+01:00") }, + new Object[]{ TimeZone.getTimeZone("GMT+01:00"), TimeZone.getTimeZone("GMT+01:00") }, + new Object[]{ TimeZone.getTimeZone("GMT+01:00"), TimeZone.getTimeZone("GMT+00:00") }, + + new Object[]{ TimeZone.getTimeZone("GMT+12:00"), TimeZone.getTimeZone("GMT+06:00") }, + new Object[]{ TimeZone.getTimeZone("GMT-12:00"), TimeZone.getTimeZone("GMT-06:00") }, + new Object[]{ TimeZone.getTimeZone("GMT+06:00"), TimeZone.getTimeZone("GMT+12:00") }, + new Object[]{ TimeZone.getTimeZone("GMT-06:00"), TimeZone.getTimeZone("GMT-12:00") }, + + new Object[]{ TimeZone.getTimeZone("GMT-12:00"), TimeZone.getTimeZone("GMT+06:00") }, + new Object[]{ TimeZone.getTimeZone("GMT+12:00"), TimeZone.getTimeZone("GMT-06:00") }, + new Object[]{ TimeZone.getTimeZone("GMT-06:00"), TimeZone.getTimeZone("GMT+12:00") }, + new Object[]{ TimeZone.getTimeZone("GMT+06:00"), TimeZone.getTimeZone("GMT-12:00") } + ); + } + + // Doing this for every timezone + @Before + public void setUp() { + // Set the producer timezone + TimeZone.setDefault(producerTimeZone); + resetTimeZoneCaches(); c1 = Calendar.getInstance(); c1.set(2000, 0, 1, 0, 0, 0); @@ -61,10 +97,7 @@ public DateExtractTest() { c2 = Calendar.getInstance(); c2.set(2000, 0, 1, 1, 1, 1); c2.set(Calendar.MILLISECOND, 412); - } - @Override - public void setUpOnce() { cleanDatabase(); transactional(new TxVoidWork() { @Override @@ -76,7 +109,7 @@ public void work(EntityManager em) { Version v1 = new Version(); em.persist(v1); - doc1 = new Document("Doc1", p, v1); + Document doc1 = new Document("Doc1", p, v1); doc1.setCreationDate(c1); doc1.setLastModified(c2.getTime()); @@ -85,15 +118,43 @@ public void work(EntityManager em) { }); } - @Before - public void setUp() { - doc1 = cbf.create(em, Document.class).getSingleResult(); + @After + public void after() { + TimeZone.setDefault(dbmsTimeZone); + resetTimeZoneCaches(); + } + + private static void resetTimeZoneCaches() { + // The H2 JDBC driver is not able to handle timezone changes because of an internal cache + try { + Class.forName("org.h2.util.DateTimeUtils").getMethod("resetCalendar").invoke(null); + } catch (Exception e) { + // Ignore any exceptions. If it is H2 it will succeed, otherwise will fail on class lookup already + } + + // EclipseLink caches the timezone so we have to purge that cache + try { + Class helperClass = Class.forName("org.eclipse.persistence.internal.helper.Helper"); + Field f = helperClass.getDeclaredField("defaultTimeZone"); + f.setAccessible(true); + f.set(null, TimeZone.getDefault()); + + f = helperClass.getDeclaredField("calendarCache"); + f.setAccessible(true); + f.set(null, helperClass.getMethod("initCalendarCache").invoke(null)); + } catch (Exception e) { + // Ignore any exceptions. If it is EclipseLink it will succeed, otherwise will fail on class lookup already + } } // NOTE: MySQL is strange again https://bugs.mysql.com/bug.php?id=31990 @Test @Category({ NoMySQL.class }) public void testDateExtract() { + // Set the client timezone + TimeZone.setDefault(clientTimeZone); + resetTimeZoneCaches(); + CriteriaBuilder criteria = cbf.create(em, Tuple.class) .from(Document.class, "doc") .select("FUNCTION('YEAR', creationDate)") @@ -102,14 +163,14 @@ public void testDateExtract() { .select("FUNCTION('HOUR', creationDate)") .select("FUNCTION('MINUTE', creationDate)") .select("FUNCTION('SECOND', creationDate)") - .select("FUNCTION('EPOCH', creationDate)") + .select("FUNCTION('EPOCH', creationDate)") .select("FUNCTION('YEAR', lastModified)") .select("FUNCTION('MONTH', lastModified)") .select("FUNCTION('DAY', lastModified)") .select("FUNCTION('HOUR', lastModified)") .select("FUNCTION('MINUTE', lastModified)") .select("FUNCTION('SECOND', lastModified)") - .select("FUNCTION('EPOCH', lastModified)") + .select("FUNCTION('EPOCH', lastModified)") ; List list = criteria.getResultList(); @@ -117,20 +178,24 @@ public void testDateExtract() { Tuple actual = list.get(0); + int offsetInMillis1 = producerTimeZone.getOffset(c1.getTimeInMillis()); + int offsetInMillis2 = producerTimeZone.getOffset(c2.getTimeInMillis()); + assertEquals(c1.get(Calendar.YEAR), actual.get(0)); assertEquals(c1.get(Calendar.MONTH) + 1, actual.get(1)); assertEquals(c1.get(Calendar.DAY_OF_MONTH), actual.get(2)); - assertEquals(c1.get(Calendar.HOUR), actual.get(3)); - assertEquals(c1.get(Calendar.MINUTE), actual.get(4)); - assertEquals(c1.get(Calendar.SECOND), actual.get(5)); - assertEquals((int) (c1.getTimeInMillis() / 1000L), actual.get(6)); + assertEquals(c1.get(Calendar.HOUR_OF_DAY), (int) actual.get(3)); + assertEquals(c1.get(Calendar.MINUTE), (int) actual.get(4)); + assertEquals(c1.get(Calendar.SECOND), (int) actual.get(5)); + // But there is an offset in the time in millis as that is the epoch + assertEquals((int) (c1.getTimeInMillis() / 1000L), (int) actual.get(6) - (offsetInMillis1 / 1000L)); assertEquals(c2.get(Calendar.YEAR), actual.get(7)); assertEquals(c2.get(Calendar.MONTH) + 1, actual.get(8)); assertEquals(c2.get(Calendar.DAY_OF_MONTH), actual.get(9)); - assertEquals(c2.get(Calendar.HOUR), actual.get(10)); - assertEquals(c2.get(Calendar.MINUTE), actual.get(11)); - assertEquals(c2.get(Calendar.SECOND), actual.get(12)); - assertEquals((int) (c2.getTimeInMillis() / 1000L), actual.get(13)); + assertEquals(c2.get(Calendar.HOUR_OF_DAY), (int) actual.get(10)); + assertEquals(c2.get(Calendar.MINUTE), (int) actual.get(11)); + assertEquals(c2.get(Calendar.SECOND), (int) actual.get(12)); + assertEquals((int) (c2.getTimeInMillis() / 1000L), (int) actual.get(13) - (offsetInMillis2 / 1000L)); } } diff --git a/core/testsuite/src/test/java/com/blazebit/persistence/testsuite/JoinTest.java b/core/testsuite/src/test/java/com/blazebit/persistence/testsuite/JoinTest.java index 6195255ba0..5f55284a19 100644 --- a/core/testsuite/src/test/java/com/blazebit/persistence/testsuite/JoinTest.java +++ b/core/testsuite/src/test/java/com/blazebit/persistence/testsuite/JoinTest.java @@ -16,8 +16,10 @@ package com.blazebit.persistence.testsuite; +import static com.googlecode.catchexception.CatchException.caughtException; import static com.googlecode.catchexception.CatchException.verifyException; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import java.util.List; @@ -26,6 +28,11 @@ import com.blazebit.persistence.testsuite.base.category.NoDatanucleus; import com.blazebit.persistence.testsuite.base.category.NoEclipselink; +import com.blazebit.persistence.testsuite.base.category.NoHibernate; +import com.blazebit.persistence.testsuite.base.category.NoHibernate42; +import com.blazebit.persistence.testsuite.base.category.NoHibernate43; +import com.blazebit.persistence.testsuite.base.category.NoHibernate50; +import com.blazebit.persistence.testsuite.base.category.NoHibernate51; import com.blazebit.persistence.testsuite.base.category.NoOpenJPA; import com.blazebit.persistence.testsuite.entity.DocumentForEntityKeyMaps; import com.blazebit.persistence.testsuite.entity.PersonForEntityKeyMaps; @@ -314,14 +321,20 @@ public void testCallOrderInvariance() { public void testFetchJoinCheck1() { CriteriaBuilder crit = cbf.create(em, Tuple.class).from(Document.class, "d") .select("name"); - verifyException(crit, IllegalStateException.class).join("d.versions", "versions", JoinType.LEFT, true); + crit.join("d.versions", "versions", JoinType.LEFT, true); + verifyException(crit, IllegalStateException.class).getQueryString(); + String message = caughtException().getMessage(); + assertTrue(message.contains("Missing fetch owners: [d]")); } @Test public void testFetchJoinCheck2() { CriteriaBuilder crit = cbf.create(em, Tuple.class).from(Document.class, "d") .select("name"); - verifyException(crit, IllegalStateException.class).fetch("d.versions"); + crit.fetch("d.versions"); + verifyException(crit, IllegalStateException.class).getQueryString(); + String message = caughtException().getMessage(); + assertTrue(message.contains("Missing fetch owners: [d]")); } @Test @@ -350,6 +363,67 @@ public void testFetchAmbiguousImplicitAlias() { assertEquals("SELECT a FROM Document a JOIN FETCH a.owner owner_1 LEFT JOIN FETCH owner_1.partnerDocument partnerDocument_1 LEFT JOIN FETCH partnerDocument_1.owner owner_2", crit.getQueryString()); crit.getResultList(); } + + @Test + public void selectWithFetchSimpleRelation() { + CriteriaBuilder crit = cbf.create(em, Document.class, "a"); + crit.select("owner"); + crit.fetch("owner.partnerDocument.owner"); + + assertEquals("SELECT owner_1 FROM Document a JOIN a.owner owner_1 LEFT JOIN FETCH owner_1.partnerDocument partnerDocument_1 LEFT JOIN FETCH partnerDocument_1.owner owner_2", crit.getQueryString()); + crit.getResultList(); + } + + @Test + public void selectWithFetchMultipleRelations() { + CriteriaBuilder crit = cbf.create(em, Document.class, "a"); + crit.select("owner"); + crit.fetch("owner.partnerDocument.owner", "owner.ownedDocuments"); + + assertEquals("SELECT owner_1 FROM Document a JOIN a.owner owner_1 LEFT JOIN FETCH owner_1.ownedDocuments ownedDocuments_1 LEFT JOIN FETCH owner_1.partnerDocument partnerDocument_1 LEFT JOIN FETCH partnerDocument_1.owner owner_2", crit.getQueryString()); + crit.getResultList(); + } + + @Test + // Older Hibernate versions don't like fetch joining an element collection at all: https://hibernate.atlassian.net/browse/HHH-11140 + @Category({ NoHibernate42.class, NoHibernate43.class, NoHibernate50.class, NoHibernate51.class }) + public void selectWithFetchElementCollectionOnly() { + CriteriaBuilder crit = cbf.create(em, Document.class, "a"); + crit.select("owner"); + crit.fetch("owner.localized"); + + assertEquals("SELECT owner_1 FROM Document a JOIN a.owner owner_1 LEFT JOIN FETCH owner_1.localized localized_1", crit.getQueryString()); + crit.getResultList(); + } + + @Test + // But fetching the element collection together with other properties is still problematic + @Category({ NoHibernate.class }) + public void selectWithFetchElementCollectionAndOtherRelations() { + CriteriaBuilder crit = cbf.create(em, Document.class, "a"); + crit.select("owner"); + crit.fetch("owner.partnerDocument.owner", "owner.localized"); + + assertEquals("SELECT owner_1 FROM Document a JOIN a.owner owner_1 LEFT JOIN FETCH owner_1.localized localized_1 LEFT JOIN FETCH owner_1.partnerDocument partnerDocument_1 LEFT JOIN FETCH partnerDocument_1.owner owner_2", crit.getQueryString()); + crit.getResultList(); + } + + @Test + public void selectWithFetchNonExistingSubRelation() { + CriteriaBuilder crit = cbf.create(em, Document.class, "a"); + crit.select("owner"); + verifyException(crit, IllegalArgumentException.class).fetch("owner.intIdEntity"); + } + + @Test + public void selectNonFetchOwnerWithFetching() { + CriteriaBuilder crit = cbf.create(em, Document.class, "a"); + crit.select("owner.id"); + crit.fetch("owner"); + verifyException(crit, IllegalStateException.class).getQueryString(); + String message = caughtException().getMessage(); + assertTrue(message.contains("Missing fetch owners: [a]")); + } @Test public void testImplicitJoinNodeReuse() { diff --git a/documentation/src/main/asciidoc/core/manual/en_US/04_from_clause.adoc b/documentation/src/main/asciidoc/core/manual/en_US/04_from_clause.adoc index c86cac4016..50e29f0e5b 100644 --- a/documentation/src/main/asciidoc/core/manual/en_US/04_from_clause.adoc +++ b/documentation/src/main/asciidoc/core/manual/en_US/04_from_clause.adoc @@ -216,6 +216,27 @@ WHERE dad IS NULL WARNING: Although the JPA spec does not specifically allow aliasing fetch joins, every major JPA provider supports this. +When doing a scalar select instead of a query root select, {projectname} automatically adapts the fetches to the new fetch owners. + +[source,java] +---- +CriteriaBuilder cb = cbf.create(em, Cat.class) + .from(Cat.class) + .fetch("father.kittens") + .select("father"); +---- + +In this case we fetch the `father` relation and the `kittens` of the `father`. By also selecting the `father` relation, the fetch owner changes from the query root to the `father`. +This has the effect, that the `father` is not fetch joined, as that would be invalid. + +[source,sql] +---- +SELECT father_1 +FROM Cat cat +LEFT JOIN cat.father father_1 +LEFT JOIN FETCH father_1.kittens kittens_1 +---- + ==== Array joins Array joins are an extension to the JPQL grammar which offer a convenient way to create joins with an `ON` clause condition. diff --git a/jpa-criteria/testsuite/src/test/java/com/blazebit/persistence/criteria/JoinTest.java b/jpa-criteria/testsuite/src/test/java/com/blazebit/persistence/criteria/JoinTest.java index 540a3a2fa4..2c098d9e7b 100644 --- a/jpa-criteria/testsuite/src/test/java/com/blazebit/persistence/criteria/JoinTest.java +++ b/jpa-criteria/testsuite/src/test/java/com/blazebit/persistence/criteria/JoinTest.java @@ -75,17 +75,17 @@ public void joinSet() { @Test public void fetchSet() { - BlazeCriteriaQuery cq = BlazeCriteria.get(em, cbf, Long.class); + BlazeCriteriaQuery cq = BlazeCriteria.get(em, cbf, Document.class); BlazeCriteriaBuilder cb = cq.getCriteriaBuilder(); BlazeRoot root = cq.from(Document.class, "document"); BlazeJoin partners = root.fetch(Document_.partners, "partner"); BlazeJoin ownerDocuments = partners.join(Person_.ownedDocuments, "doc"); ownerDocuments.fetch(); - cq.select(root.get(Document_.id)); + cq.select(root); CriteriaBuilder criteriaBuilder = cq.createCriteriaBuilder(); - assertEquals("SELECT document.id FROM Document document JOIN FETCH document.partners partner JOIN FETCH partner.ownedDocuments doc", criteriaBuilder.getQueryString()); + assertEquals("SELECT document FROM Document document JOIN FETCH document.partners partner JOIN FETCH partner.ownedDocuments doc", criteriaBuilder.getQueryString()); } @Test diff --git a/testsuite-base/datanucleus/src/main/java/com/blazebit/persistence/testsuite/base/SanePostgreSQLAdapter.java b/testsuite-base/datanucleus/src/main/java/com/blazebit/persistence/testsuite/base/SanePostgreSQLAdapter.java new file mode 100644 index 0000000000..3ceee74ecc --- /dev/null +++ b/testsuite-base/datanucleus/src/main/java/com/blazebit/persistence/testsuite/base/SanePostgreSQLAdapter.java @@ -0,0 +1,50 @@ +/* + * Copyright 2014 - 2017 Blazebit. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.blazebit.persistence.testsuite.base; + +import org.datanucleus.store.connection.ManagedConnection; +import org.datanucleus.store.rdbms.schema.JDBCTypeInfo; +import org.datanucleus.store.rdbms.schema.RDBMSTypesInfo; +import org.datanucleus.store.rdbms.schema.SQLTypeInfo; +import org.datanucleus.store.schema.StoreSchemaHandler; + +import java.sql.DatabaseMetaData; +import java.sql.Types; + +/** + * @author Christian Beikov + * @since 1.2.0 + */ +public class SanePostgreSQLAdapter extends org.datanucleus.store.rdbms.adapter.PostgreSQLAdapter { + public SanePostgreSQLAdapter(DatabaseMetaData metadata) { + super(metadata); + } + + public void initialiseTypes(StoreSchemaHandler handler, ManagedConnection mconn) { + super.initialiseTypes(handler, mconn); + + RDBMSTypesInfo typesInfo = (RDBMSTypesInfo)handler.getSchemaData(mconn.getConnection(), "types", null); + + // Make use of the normal timestamp type without timezones to have comparable results + JDBCTypeInfo jdbcType = (JDBCTypeInfo)typesInfo.getChild(Integer.toString(Types.TIMESTAMP)); + if (jdbcType != null && jdbcType.getNumberOfChildren() > 0) { + SQLTypeInfo dfltTypeInfo = (SQLTypeInfo)jdbcType.getChildren().remove("timestamptz"); + dfltTypeInfo.setTypeName("timestamp"); + jdbcType.addChild(dfltTypeInfo); + } + } +} diff --git a/testsuite-base/datanucleus/src/main/resources/plugin.xml b/testsuite-base/datanucleus/src/main/resources/plugin.xml index 5e1276e90b..da93235c99 100644 --- a/testsuite-base/datanucleus/src/main/resources/plugin.xml +++ b/testsuite-base/datanucleus/src/main/resources/plugin.xml @@ -17,5 +17,6 @@ + \ No newline at end of file