From ab94583f157337c07eef16fe72843c303d440e45 Mon Sep 17 00:00:00 2001 From: Jurgen <5031427+Jugen@users.noreply.github.com> Date: Mon, 12 Feb 2024 16:50:36 +0200 Subject: [PATCH] Extracted shared code into StageUtil and restored original Stage ordering. --- .../select/DefaultSelectTranslator.java | 4 +- .../translator/select/DistinctStage.java | 42 +---------- .../translator/select/GroupByStage.java | 27 +------ .../translator/select/OrderingStage.java | 15 ++-- .../access/translator/select/StageUtil.java | 74 +++++++++++++++++++ .../translator/select/DistinctStageTest.java | 24 +++--- 6 files changed, 97 insertions(+), 89 deletions(-) create mode 100644 cayenne/src/main/java/org/apache/cayenne/access/translator/select/StageUtil.java diff --git a/cayenne/src/main/java/org/apache/cayenne/access/translator/select/DefaultSelectTranslator.java b/cayenne/src/main/java/org/apache/cayenne/access/translator/select/DefaultSelectTranslator.java index da70e50cac..0aacf108fb 100644 --- a/cayenne/src/main/java/org/apache/cayenne/access/translator/select/DefaultSelectTranslator.java +++ b/cayenne/src/main/java/org/apache/cayenne/access/translator/select/DefaultSelectTranslator.java @@ -40,11 +40,11 @@ public class DefaultSelectTranslator implements SelectTranslator { private static final TranslationStage[] TRANSLATION_STAGES = { new ColumnExtractorStage(), new PrefetchNodeStage(), + new OrderingStage(), new QualifierTranslationStage(), new HavingTranslationStage(), + new GroupByStage(), new DistinctStage(), - new OrderingStage(), // DistinctStage is a prerequisite - new GroupByStage(), // OrderingStage is a prerequisite new LimitOffsetStage(), new ColumnDescriptorStage(), new TableTreeQualifierStage(), diff --git a/cayenne/src/main/java/org/apache/cayenne/access/translator/select/DistinctStage.java b/cayenne/src/main/java/org/apache/cayenne/access/translator/select/DistinctStage.java index bdf9c90c38..c217752b74 100644 --- a/cayenne/src/main/java/org/apache/cayenne/access/translator/select/DistinctStage.java +++ b/cayenne/src/main/java/org/apache/cayenne/access/translator/select/DistinctStage.java @@ -19,53 +19,15 @@ package org.apache.cayenne.access.translator.select; -import java.sql.Types; - /** * @since 4.2 */ class DistinctStage implements TranslationStage { - private static final int[] UNSUPPORTED_DISTINCT_TYPES = { - Types.BLOB, - Types.CLOB, - Types.NCLOB, - Types.LONGVARCHAR, - Types.LONGNVARCHAR, - Types.LONGVARBINARY - }; - - static boolean isUnsupportedForDistinct(int type) { - for (int unsupportedDistinctType : UNSUPPORTED_DISTINCT_TYPES) { - if (unsupportedDistinctType == type) { - return true; - } - } - - return false; - } - @Override public void perform(TranslatorContext context) { - // explicit suppressing of distinct - if(context.getMetadata().isSuppressingDistinct()) { - context.setDistinctSuppression(true); - return; - } - - // query forcing distinct or query have joins (qualifier or prefetch) - if(!context.getQuery().isDistinct() && !context.getTableTree().hasToManyJoin()) { - return; - } - - // unsuitable jdbc type for distinct clause - for(ResultNodeDescriptor node : context.getResultNodeList()) { - // TODO: make it per adapter rather than one-for-all - if(isUnsupportedForDistinct(node.getJdbcType())) { - context.setDistinctSuppression(true); - return; - } + if (StageUtil.isDistinct(context)) { + context.getSelectBuilder().distinct(); } - context.getSelectBuilder().distinct(); } } diff --git a/cayenne/src/main/java/org/apache/cayenne/access/translator/select/GroupByStage.java b/cayenne/src/main/java/org/apache/cayenne/access/translator/select/GroupByStage.java index ca2509d76d..f1326527c3 100644 --- a/cayenne/src/main/java/org/apache/cayenne/access/translator/select/GroupByStage.java +++ b/cayenne/src/main/java/org/apache/cayenne/access/translator/select/GroupByStage.java @@ -19,9 +19,6 @@ package org.apache.cayenne.access.translator.select; -import org.apache.cayenne.exp.parser.ASTAggregateFunctionCall; -import org.apache.cayenne.query.Ordering; - /** * @since 4.2 */ @@ -29,7 +26,7 @@ class GroupByStage implements TranslationStage { @Override public void perform(TranslatorContext context) { - if (!hasAggregate(context)) { + if (!StageUtil.hasAggregate(context)) { return; } @@ -40,26 +37,4 @@ public void perform(TranslatorContext context) { context.getSelectBuilder().groupBy(resultNode.getNode().deepCopy()); } } - - static boolean hasAggregate(TranslatorContext context) { - if(context.getQuery().getHavingQualifier() != null) { - return true; - } - - for(ResultNodeDescriptor resultNode : context.getResultNodeList()) { - if(resultNode.isAggregate()) { - return true; - } - } - - if(context.getQuery().getOrderings() != null) { - for(Ordering ordering : context.getQuery().getOrderings()) { - if(ordering.getSortSpec() instanceof ASTAggregateFunctionCall) { - return true; - } - } - } - - return false; - } } diff --git a/cayenne/src/main/java/org/apache/cayenne/access/translator/select/OrderingStage.java b/cayenne/src/main/java/org/apache/cayenne/access/translator/select/OrderingStage.java index 8d01f7cd33..95a119c239 100644 --- a/cayenne/src/main/java/org/apache/cayenne/access/translator/select/OrderingStage.java +++ b/cayenne/src/main/java/org/apache/cayenne/access/translator/select/OrderingStage.java @@ -20,7 +20,6 @@ package org.apache.cayenne.access.translator.select; import java.util.Optional; -import java.util.concurrent.atomic.AtomicBoolean; import org.apache.cayenne.access.sqlbuilder.NodeBuilder; import org.apache.cayenne.access.sqlbuilder.OrderingNodeBuilder; @@ -48,7 +47,7 @@ public void perform(TranslatorContext context) { return; } - isDistinctOrGroupByQuery = GroupByStage.hasAggregate(context) || isDistinct(context); + isDistinctOrGroupByQuery = StageUtil.hasAggregate(context) || StageUtil.isDistinct(context); QualifierTranslator qualifierTranslator = context.getQualifierTranslator(); for(Ordering ordering : context.getQuery().getOrderings()) { @@ -66,7 +65,7 @@ private void processOrdering(QualifierTranslator qualifierTranslator, Translator } // If query is DISTINCT or GROUPING then we need to add all missing ORDER BY clauses as result columns - if (isDistinctOrGroupByQuery) { + if (isDistinctOrGroupByQuery || isCompoundPath(ordering)) { Optional column = getResultColumn(context, order(nodeBuilder)); if (!column.isPresent()) { // deepCopy as some DB expect exactly the same expression in select and in ordering @@ -84,6 +83,10 @@ private void processOrdering(QualifierTranslator qualifierTranslator, Translator context.getSelectBuilder().orderBy(orderingNodeBuilder); } + private boolean isCompoundPath(Ordering ordering) { + return ordering.getSortSpecString().indexOf('.') > 0; + } + private Optional getResultColumn(TranslatorContext context, OrderingNodeBuilder orderBuilder) { String orderStr = getSqlString(orderBuilder); @@ -105,10 +108,4 @@ private String getSqlString(NodeBuilder nb) { node.visit(sqlVisitor); return strBuilder.append(' ').toString(); } - - private boolean isDistinct(TranslatorContext context) { - return !context.isDistinctSuppression() - && (context.getQuery().isDistinct() - || context.getTableTree().hasToManyJoin()); - } } diff --git a/cayenne/src/main/java/org/apache/cayenne/access/translator/select/StageUtil.java b/cayenne/src/main/java/org/apache/cayenne/access/translator/select/StageUtil.java new file mode 100644 index 0000000000..9eb085ed6c --- /dev/null +++ b/cayenne/src/main/java/org/apache/cayenne/access/translator/select/StageUtil.java @@ -0,0 +1,74 @@ +package org.apache.cayenne.access.translator.select; + +import java.sql.Types; + +import org.apache.cayenne.exp.parser.ASTAggregateFunctionCall; +import org.apache.cayenne.query.Ordering; + +public class StageUtil +{ + private static final int[] UNSUPPORTED_DISTINCT_TYPES = { + Types.BLOB, + Types.CLOB, + Types.NCLOB, + Types.LONGVARCHAR, + Types.LONGNVARCHAR, + Types.LONGVARBINARY + }; + + private static boolean isUnsupportedForDistinct(int type) { + for (int unsupportedDistinctType : UNSUPPORTED_DISTINCT_TYPES) { + if (unsupportedDistinctType == type) { + return true; + } + } + + return false; + } + + static boolean isDistinct(TranslatorContext context) { + // explicit suppressing of distinct + if(context.getMetadata().isSuppressingDistinct()) { + context.setDistinctSuppression(true); + return false; + } + + // query forcing distinct or query have joins (qualifier or prefetch) + if(!context.getQuery().isDistinct() && !context.getTableTree().hasToManyJoin()) { + return false; + } + + // unsuitable jdbc type for distinct clause + for(ResultNodeDescriptor node : context.getResultNodeList()) { + // TODO: make it per adapter rather than one-for-all + if(isUnsupportedForDistinct(node.getJdbcType())) { + context.setDistinctSuppression(true); + return false; + } + } + + return true; + } + + static boolean hasAggregate(TranslatorContext context) { + if(context.getQuery().getHavingQualifier() != null) { + return true; + } + + for(ResultNodeDescriptor resultNode : context.getResultNodeList()) { + if(resultNode.isAggregate()) { + return true; + } + } + + if(context.getQuery().getOrderings() != null) { + for(Ordering ordering : context.getQuery().getOrderings()) { + if(ordering.getSortSpec() instanceof ASTAggregateFunctionCall) { + return true; + } + } + } + + return false; + } +} diff --git a/cayenne/src/test/java/org/apache/cayenne/access/translator/select/DistinctStageTest.java b/cayenne/src/test/java/org/apache/cayenne/access/translator/select/DistinctStageTest.java index 8f57cd4843..e515205858 100644 --- a/cayenne/src/test/java/org/apache/cayenne/access/translator/select/DistinctStageTest.java +++ b/cayenne/src/test/java/org/apache/cayenne/access/translator/select/DistinctStageTest.java @@ -35,18 +35,18 @@ public class DistinctStageTest { @Test public void isUnsupportedForDistinct() { - assertTrue(DistinctStage.isUnsupportedForDistinct(Types.BLOB)); - assertTrue(DistinctStage.isUnsupportedForDistinct(Types.CLOB)); - assertTrue(DistinctStage.isUnsupportedForDistinct(Types.NCLOB)); - assertTrue(DistinctStage.isUnsupportedForDistinct(Types.LONGVARCHAR)); - assertTrue(DistinctStage.isUnsupportedForDistinct(Types.LONGNVARCHAR)); - assertTrue(DistinctStage.isUnsupportedForDistinct(Types.LONGVARBINARY)); - assertFalse(DistinctStage.isUnsupportedForDistinct(Types.INTEGER)); - assertFalse(DistinctStage.isUnsupportedForDistinct(Types.DATE)); - assertFalse(DistinctStage.isUnsupportedForDistinct(Types.CHAR)); - assertFalse(DistinctStage.isUnsupportedForDistinct(Types.DECIMAL)); - assertFalse(DistinctStage.isUnsupportedForDistinct(Types.FLOAT)); - assertFalse(DistinctStage.isUnsupportedForDistinct(Types.VARCHAR)); + assertTrue(StageUtil.isUnsupportedForDistinct(Types.BLOB)); + assertTrue(StageUtil.isUnsupportedForDistinct(Types.CLOB)); + assertTrue(StageUtil.isUnsupportedForDistinct(Types.NCLOB)); + assertTrue(StageUtil.isUnsupportedForDistinct(Types.LONGVARCHAR)); + assertTrue(StageUtil.isUnsupportedForDistinct(Types.LONGNVARCHAR)); + assertTrue(StageUtil.isUnsupportedForDistinct(Types.LONGVARBINARY)); + assertFalse(StageUtil.isUnsupportedForDistinct(Types.INTEGER)); + assertFalse(StageUtil.isUnsupportedForDistinct(Types.DATE)); + assertFalse(StageUtil.isUnsupportedForDistinct(Types.CHAR)); + assertFalse(StageUtil.isUnsupportedForDistinct(Types.DECIMAL)); + assertFalse(StageUtil.isUnsupportedForDistinct(Types.FLOAT)); + assertFalse(StageUtil.isUnsupportedForDistinct(Types.VARCHAR)); } @Test