This is an automated email from the ASF dual-hosted git repository. ntimofeev pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/cayenne.git
The following commit(s) were added to refs/heads/master by this push: new c00d5d330 CAY-2847 Improve duplicate select column detection when using order by c00d5d330 is described below commit c00d5d330b6d7f42022de12ca569bf455eeb4028 Author: Nikita Timofeev <stari...@gmail.com> AuthorDate: Mon Jun 3 16:49:00 2024 +0400 CAY-2847 Improve duplicate select column detection when using order by --- .../access/sqlbuilder/sqltree/AliasedNode.java | 16 ++ .../access/sqlbuilder/sqltree/BetweenNode.java | 16 ++ .../access/sqlbuilder/sqltree/ColumnNode.java | 16 ++ .../access/sqlbuilder/sqltree/FunctionNode.java | 16 ++ .../cayenne/access/sqlbuilder/sqltree/InNode.java | 15 ++ .../access/sqlbuilder/sqltree/LikeNode.java | 16 ++ .../access/sqlbuilder/sqltree/LimitOffsetNode.java | 16 ++ .../cayenne/access/sqlbuilder/sqltree/Node.java | 30 ++++ .../access/sqlbuilder/sqltree/OffsetNode.java | 16 ++ .../sqlbuilder/sqltree/OpExpressionNode.java | 16 ++ .../access/sqlbuilder/sqltree/SubqueryNode.java | 15 ++ .../access/sqlbuilder/sqltree/TableNode.java | 14 ++ .../access/sqlbuilder/sqltree/TextNode.java | 16 ++ .../cayenne/access/sqlbuilder/sqltree/TopNode.java | 16 ++ .../sqlbuilder/sqltree/TrimmingColumnNode.java | 15 ++ .../access/sqlbuilder/sqltree/ValueNode.java | 17 ++ .../translator/select/DefaultSelectTranslator.java | 2 +- .../translator/select/OrderingAbstractStage.java | 174 ++------------------- ...istictStage.java => OrderingDistinctStage.java} | 15 +- .../translator/select/OrderingGroupByStage.java | 7 +- .../translator/select/OrderingStageTest.java | 2 +- 21 files changed, 284 insertions(+), 182 deletions(-) diff --git a/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/AliasedNode.java b/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/AliasedNode.java index 3f9973d8b..f3e512165 100644 --- a/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/AliasedNode.java +++ b/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/AliasedNode.java @@ -22,6 +22,8 @@ package org.apache.cayenne.access.sqlbuilder.sqltree; import org.apache.cayenne.access.sqlbuilder.NodeTreeVisitor; import org.apache.cayenne.access.sqlbuilder.QuotingAppendable; +import java.util.Objects; + /** * @since 4.2 */ @@ -86,4 +88,18 @@ public class AliasedNode extends Node { } return true; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + if (!super.equals(o)) return false; + AliasedNode that = (AliasedNode) o; + return Objects.equals(alias, that.alias); + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), alias); + } } diff --git a/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/BetweenNode.java b/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/BetweenNode.java index 48351cb09..7b6b5aed3 100644 --- a/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/BetweenNode.java +++ b/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/BetweenNode.java @@ -22,6 +22,8 @@ package org.apache.cayenne.access.sqlbuilder.sqltree; import org.apache.cayenne.access.sqlbuilder.QuotingAppendable; +import java.util.Objects; + /** * @since 4.2 */ @@ -53,4 +55,18 @@ public class BetweenNode extends ExpressionNode { public boolean isNot() { return not; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + if (!super.equals(o)) return false; + BetweenNode that = (BetweenNode) o; + return not == that.not; + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), not); + } } diff --git a/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/ColumnNode.java b/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/ColumnNode.java index bfed7d648..58dfcc7c2 100644 --- a/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/ColumnNode.java +++ b/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/ColumnNode.java @@ -22,6 +22,8 @@ package org.apache.cayenne.access.sqlbuilder.sqltree; import org.apache.cayenne.access.sqlbuilder.QuotingAppendable; import org.apache.cayenne.map.DbAttribute; +import java.util.Objects; + /** * @since 4.2 */ @@ -77,4 +79,18 @@ public class ColumnNode extends Node { public Node copy() { return new ColumnNode(table, column, alias, attribute); } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + if (!super.equals(o)) return false; + ColumnNode that = (ColumnNode) o; + return Objects.equals(table, that.table) && Objects.equals(column, that.column) && Objects.equals(alias, that.alias); + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), table, column, alias); + } } diff --git a/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/FunctionNode.java b/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/FunctionNode.java index 8920c860b..ff92e6238 100644 --- a/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/FunctionNode.java +++ b/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/FunctionNode.java @@ -22,6 +22,8 @@ package org.apache.cayenne.access.sqlbuilder.sqltree; import org.apache.cayenne.access.sqlbuilder.NodeTreeVisitor; import org.apache.cayenne.access.sqlbuilder.QuotingAppendable; +import java.util.Objects; + /** * @since 4.2 */ @@ -134,4 +136,18 @@ public class FunctionNode extends Node { // has alias and not in result node return alias != null && notInResultNode(); } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + if (!super.equals(o)) return false; + FunctionNode that = (FunctionNode) o; + return Objects.equals(functionName, that.functionName) && Objects.equals(alias, that.alias); + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), functionName, alias); + } } diff --git a/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/InNode.java b/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/InNode.java index 65c89cd89..a56ba76b1 100644 --- a/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/InNode.java +++ b/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/InNode.java @@ -21,6 +21,8 @@ package org.apache.cayenne.access.sqlbuilder.sqltree; import org.apache.cayenne.access.sqlbuilder.QuotingAppendable; +import java.util.Objects; + /** * @since 4.2 */ @@ -62,4 +64,17 @@ public class InNode extends Node { return not; } + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + if (!super.equals(o)) return false; + InNode inNode = (InNode) o; + return not == inNode.not; + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), not); + } } diff --git a/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/LikeNode.java b/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/LikeNode.java index 108b913a8..e8637aa29 100644 --- a/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/LikeNode.java +++ b/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/LikeNode.java @@ -21,6 +21,8 @@ package org.apache.cayenne.access.sqlbuilder.sqltree; import org.apache.cayenne.access.sqlbuilder.QuotingAppendable; +import java.util.Objects; + /** * expressions: LIKE, ILIKE, NOT LIKE, NOT ILIKE + ESCAPE * @@ -86,4 +88,18 @@ public class LikeNode extends ExpressionNode { public char getEscape() { return escape; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + if (!super.equals(o)) return false; + LikeNode likeNode = (LikeNode) o; + return ignoreCase == likeNode.ignoreCase && not == likeNode.not && escape == likeNode.escape; + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), ignoreCase, not, escape); + } } diff --git a/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/LimitOffsetNode.java b/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/LimitOffsetNode.java index 91ef08f1b..d7c44f612 100644 --- a/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/LimitOffsetNode.java +++ b/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/LimitOffsetNode.java @@ -21,6 +21,8 @@ package org.apache.cayenne.access.sqlbuilder.sqltree; import org.apache.cayenne.access.sqlbuilder.QuotingAppendable; +import java.util.Objects; + /** * @since 4.2 */ @@ -55,4 +57,18 @@ public class LimitOffsetNode extends Node { public Node copy() { return new LimitOffsetNode(limit, offset); } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + if (!super.equals(o)) return false; + LimitOffsetNode that = (LimitOffsetNode) o; + return limit == that.limit && offset == that.offset; + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), limit, offset); + } } diff --git a/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/Node.java b/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/Node.java index 94ac188f2..0beaea61d 100644 --- a/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/Node.java +++ b/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/Node.java @@ -149,4 +149,34 @@ public abstract class Node { public void appendChildrenEnd(QuotingAppendable buffer) { } + + /** + * @param node to compare with + * @return true if this node and all it's children are equal to the given node + * @since 5.0 + */ + public boolean deepEquals(Node node) { + if(!equals(node) || childrenCount != node.childrenCount) { + return false; + } + for(int i=0; i<childrenCount; i++) { + if(!children[i].deepEquals(node.children[i])) { + return false; + } + } + return true; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Node node = (Node) o; + return type == node.type; + } + + @Override + public int hashCode() { + return type.hashCode(); + } } diff --git a/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/OffsetNode.java b/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/OffsetNode.java index 7e7590a9c..b2966050f 100644 --- a/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/OffsetNode.java +++ b/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/OffsetNode.java @@ -21,6 +21,8 @@ package org.apache.cayenne.access.sqlbuilder.sqltree; import org.apache.cayenne.access.sqlbuilder.QuotingAppendable; +import java.util.Objects; + /** * @since 4.2 */ @@ -40,4 +42,18 @@ public class OffsetNode extends Node { public Node copy() { return new OffsetNode(offset); } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + if (!super.equals(o)) return false; + OffsetNode that = (OffsetNode) o; + return offset == that.offset; + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), offset); + } } diff --git a/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/OpExpressionNode.java b/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/OpExpressionNode.java index a711bdbe8..d305106f6 100644 --- a/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/OpExpressionNode.java +++ b/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/OpExpressionNode.java @@ -21,6 +21,8 @@ package org.apache.cayenne.access.sqlbuilder.sqltree; import org.apache.cayenne.access.sqlbuilder.QuotingAppendable; +import java.util.Objects; + /** * @since 4.2 */ @@ -45,4 +47,18 @@ public class OpExpressionNode extends ExpressionNode { public String getOp() { return op; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + if (!super.equals(o)) return false; + OpExpressionNode that = (OpExpressionNode) o; + return Objects.equals(op, that.op); + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), op); + } } diff --git a/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/SubqueryNode.java b/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/SubqueryNode.java index 8fdd725b3..b53dac2b0 100644 --- a/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/SubqueryNode.java +++ b/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/SubqueryNode.java @@ -21,6 +21,8 @@ package org.apache.cayenne.access.sqlbuilder.sqltree; import org.apache.cayenne.access.sqlbuilder.QuotingAppendable; +import java.util.Objects; + /** * @since 4.2 */ @@ -42,4 +44,17 @@ public class SubqueryNode extends Node { return new SubqueryNode(subqueryType); } + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + if (!super.equals(o)) return false; + SubqueryNode that = (SubqueryNode) o; + return Objects.equals(subqueryType, that.subqueryType); + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), subqueryType); + } } diff --git a/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/TableNode.java b/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/TableNode.java index 56d2c8822..d9ddb8e86 100644 --- a/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/TableNode.java +++ b/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/TableNode.java @@ -71,4 +71,18 @@ public class TableNode extends Node { public Node copy() { return new TableNode(tableName, alias); } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + if (!super.equals(o)) return false; + TableNode tableNode = (TableNode) o; + return Objects.equals(tableName, tableNode.tableName) && Objects.equals(alias, tableNode.alias); + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), tableName, alias); + } } diff --git a/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/TextNode.java b/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/TextNode.java index d475f347d..f7eda8717 100644 --- a/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/TextNode.java +++ b/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/TextNode.java @@ -21,6 +21,8 @@ package org.apache.cayenne.access.sqlbuilder.sqltree; import org.apache.cayenne.access.sqlbuilder.QuotingAppendable; +import java.util.Objects; + /** * @since 4.2 */ @@ -45,4 +47,18 @@ public class TextNode extends Node { public CharSequence getText() { return text; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + if (!super.equals(o)) return false; + TextNode textNode = (TextNode) o; + return Objects.equals(text, textNode.text); + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), text); + } } diff --git a/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/TopNode.java b/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/TopNode.java index 3479ab64c..b09482bf4 100644 --- a/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/TopNode.java +++ b/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/TopNode.java @@ -21,6 +21,8 @@ package org.apache.cayenne.access.sqlbuilder.sqltree; import org.apache.cayenne.access.sqlbuilder.QuotingAppendable; +import java.util.Objects; + /** * @since 4.2 */ @@ -41,4 +43,18 @@ public class TopNode extends Node { public Node copy() { return new TopNode(count); } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + if (!super.equals(o)) return false; + TopNode topNode = (TopNode) o; + return count == topNode.count; + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), count); + } } diff --git a/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/TrimmingColumnNode.java b/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/TrimmingColumnNode.java index b856e6be1..bf1f56997 100644 --- a/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/TrimmingColumnNode.java +++ b/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/TrimmingColumnNode.java @@ -22,6 +22,7 @@ package org.apache.cayenne.access.sqlbuilder.sqltree; import org.apache.cayenne.access.sqlbuilder.QuotingAppendable; import java.sql.Types; +import java.util.Objects; /** * @since 4.2 @@ -157,4 +158,18 @@ public class TrimmingColumnNode extends Node { public Node copy() { return new TrimmingColumnNode(columnNode.deepCopy()); } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + if (!super.equals(o)) return false; + TrimmingColumnNode that = (TrimmingColumnNode) o; + return Objects.equals(columnNode, that.columnNode); + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), columnNode); + } } diff --git a/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/ValueNode.java b/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/ValueNode.java index 8dda608aa..a6c50792c 100644 --- a/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/ValueNode.java +++ b/cayenne/src/main/java/org/apache/cayenne/access/sqlbuilder/sqltree/ValueNode.java @@ -19,6 +19,7 @@ package org.apache.cayenne.access.sqlbuilder.sqltree; +import java.util.Objects; import java.util.function.Supplier; import org.apache.cayenne.CayenneRuntimeException; @@ -275,4 +276,20 @@ public class ValueNode extends Node { public boolean isNeedBinding() { return needBinding; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + if (!super.equals(o)) return false; + ValueNode valueNode = (ValueNode) o; + return isArray == valueNode.isArray + && needBinding == valueNode.needBinding + && Objects.equals(value, valueNode.value); + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), value, isArray, needBinding); + } } 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 8faa056d9..e0cc7c1d8 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 @@ -46,7 +46,7 @@ public class DefaultSelectTranslator implements SelectTranslator { new OrderingGroupByStage(), new GroupByStage(), new DistinctStage(), - new OrderingDistictStage(), + new OrderingDistinctStage(), new LimitOffsetStage(), new ColumnDescriptorStage(), new TableTreeQualifierStage(), diff --git a/cayenne/src/main/java/org/apache/cayenne/access/translator/select/OrderingAbstractStage.java b/cayenne/src/main/java/org/apache/cayenne/access/translator/select/OrderingAbstractStage.java index 4ef90600c..545e59a5a 100644 --- a/cayenne/src/main/java/org/apache/cayenne/access/translator/select/OrderingAbstractStage.java +++ b/cayenne/src/main/java/org/apache/cayenne/access/translator/select/OrderingAbstractStage.java @@ -19,193 +19,37 @@ package org.apache.cayenne.access.translator.select; -import static org.apache.cayenne.access.sqlbuilder.SQLBuilder.*; - -import java.util.Collections; -import java.util.LinkedList; -import java.util.List; -import java.util.stream.Collectors; - import org.apache.cayenne.access.sqlbuilder.NodeBuilder; -import org.apache.cayenne.access.sqlbuilder.QuotingAppendable; -import org.apache.cayenne.access.sqlbuilder.SQLGenerationContext; -import org.apache.cayenne.access.sqlbuilder.sqltree.AliasedNode; -import org.apache.cayenne.access.sqlbuilder.sqltree.ColumnNode; -import org.apache.cayenne.access.sqlbuilder.sqltree.FunctionNode; import org.apache.cayenne.access.sqlbuilder.sqltree.Node; -import org.apache.cayenne.access.sqlbuilder.sqltree.SimpleNodeTreeVisitor; import org.apache.cayenne.exp.Expression; import org.apache.cayenne.exp.parser.ASTAggregateFunctionCall; import org.apache.cayenne.query.Ordering; +import static org.apache.cayenne.access.sqlbuilder.SQLBuilder.*; + abstract class OrderingAbstractStage implements TranslationStage { protected void processOrdering(QualifierTranslator qualifierTranslator, TranslatorContext context, Ordering ordering) { Expression orderExp = ordering.getSortSpec(); NodeBuilder nodeBuilder = node(qualifierTranslator.translate(orderExp)); - if(ordering.isCaseInsensitive()) { + if (ordering.isCaseInsensitive()) { nodeBuilder = function("UPPER", nodeBuilder); } // If query is DISTINCT or GROUPING then we need to add the order column as a result column - if (orderColumnAbsent(context, nodeBuilder)) { + Node orderingNode = nodeBuilder.build(); + if (orderColumnAbsent(context, orderingNode)) { // deepCopy as some DB expect exactly the same expression in select and in ordering - ResultNodeDescriptor descriptor = context.addResultNode(nodeBuilder.build().deepCopy()); - if(orderExp instanceof ASTAggregateFunctionCall) { + ResultNodeDescriptor descriptor = context.addResultNode(orderingNode.deepCopy()); + if (orderExp instanceof ASTAggregateFunctionCall) { descriptor.setAggregate(true); } } } - private boolean orderColumnAbsent(TranslatorContext context, NodeBuilder nodeBuilder) - { - OrderNodeVisitor orderVisitor = new OrderNodeVisitor(); - nodeBuilder.build().visit( orderVisitor ); - List<CharSequence> orderParts = orderVisitor.getParts(); - + private boolean orderColumnAbsent(TranslatorContext context, Node orderingNode) { return context.getResultNodeList().stream() - .noneMatch( result -> { - ResultNodeVisitor resultVisitor = new ResultNodeVisitor(orderParts); - // Visitor aborts as soon as there's a mismatch with orderParts - result.getNode().visit(resultVisitor); - return resultVisitor.matches(); - }); - } - - private class OrderNodeVisitor extends AppendableVisitor // see below - { - @Override - public boolean onNodeStart(Node node) { - node.append( this ); - node.appendChildrenStart(this); - return true; - } - - @Override - public void onChildNodeEnd(Node parent, Node child, int index, boolean hasMore) { - if (hasMore && parent != null) { - parent.appendChildrenSeparator(this, index); - } - } - - @Override - public void onNodeEnd(Node node) { - node.appendChildrenEnd(this); - } - - List<CharSequence> getParts() { - return Collections.unmodifiableList(partList); - } - } - - private class ResultNodeVisitor extends AppendableVisitor // see below - { - private List<CharSequence> orderItemParts; - private boolean itemsMatch = true; - private int lastIndex = 0; - - ResultNodeVisitor(List<CharSequence> orderParts) { - orderItemParts = orderParts; - } - - @Override - public boolean onNodeStart(Node node) { - node.append(this); - if (node instanceof ColumnNode && ((ColumnNode) node).getAlias() != null) { - partList.removeLast(); // Remove appended alias - } - if (!(node.getParent() instanceof AliasedNode)) { - // Prevent appending opening bracket - node.appendChildrenStart(this); - } - return isEqualSoFar(); - } - - @Override - public void onChildNodeEnd(Node parent, Node child, int index, boolean hasMore) { - if (hasMore && parent != null) { - parent.appendChildrenSeparator(this, index); - isEqualSoFar(); - } - } - - @Override - public void onNodeEnd(Node node) { - // Prevent appending alias or closing bracket - if (!(node instanceof AliasedNode || node.getParent() instanceof AliasedNode)) { - node.appendChildrenEnd(this); - if (node instanceof FunctionNode && ((FunctionNode) node).getAlias() != null) { - if (partList.getLast().equals(((FunctionNode) node).getAlias())) { - partList.removeLast(); // Remove appended alias - } - } - isEqualSoFar(); - } - } - - private boolean isEqualSoFar() { - int currentSize = partList.size(); - if (currentSize == lastIndex) return itemsMatch; - if (currentSize > orderItemParts.size()) itemsMatch = false; - if (itemsMatch) { - // In reverse to fail fast by hopefully comparing column names first - for (int x = currentSize-1; x >= lastIndex; x--) { - if (!partList.get(x).equals(orderItemParts.get(x))) { - itemsMatch = false; - break; - } - } - } - lastIndex = partList.size(); - return itemsMatch; - } - - boolean matches() { - return isEqualSoFar(); - } - } - - private class AppendableVisitor extends SimpleNodeTreeVisitor implements QuotingAppendable - { - protected final LinkedList<CharSequence> partList = new LinkedList<>(); - - @Override - public QuotingAppendable append(CharSequence csq) { - partList.add(csq); - return this; - } - - @Override - public QuotingAppendable append(CharSequence csq, int start, int end) { - return this; - } - - @Override - public QuotingAppendable append(char c) { - if (c != '.' && c != ' ') partList.add(String.valueOf(c)); - return this; - } - - @Override - public QuotingAppendable append(int c) { - return this; - } - - @Override - public QuotingAppendable appendQuoted(CharSequence csq) { - partList.add(csq); - return this; - } - - @Override - public SQLGenerationContext getContext() { - return null; - } - - @Override - public String toString() { - return partList.stream().collect( Collectors.joining("|") ); - } + .noneMatch(result -> result.getNode().deepEquals(orderingNode)); } } diff --git a/cayenne/src/main/java/org/apache/cayenne/access/translator/select/OrderingDistictStage.java b/cayenne/src/main/java/org/apache/cayenne/access/translator/select/OrderingDistinctStage.java similarity index 80% rename from cayenne/src/main/java/org/apache/cayenne/access/translator/select/OrderingDistictStage.java rename to cayenne/src/main/java/org/apache/cayenne/access/translator/select/OrderingDistinctStage.java index 57a3a9b24..b38bc61be 100644 --- a/cayenne/src/main/java/org/apache/cayenne/access/translator/select/OrderingDistictStage.java +++ b/cayenne/src/main/java/org/apache/cayenne/access/translator/select/OrderingDistinctStage.java @@ -21,23 +21,18 @@ package org.apache.cayenne.access.translator.select; import org.apache.cayenne.query.Ordering; -import static org.apache.cayenne.access.sqlbuilder.SQLBuilder.*; - -/** - * @since 4.2.1 - */ -class OrderingDistictStage extends OrderingAbstractStage { +class OrderingDistinctStage extends OrderingAbstractStage { @Override public void perform(TranslatorContext context) { - if(context.getQuery().getOrderings() == null) { + if (context.getQuery().getOrderings() == null) { return; } if (isDistinct(context)) { // If query is DISTINCT then we need to add the order column as a result column QualifierTranslator qualifierTranslator = context.getQualifierTranslator(); - for(Ordering ordering : context.getQuery().getOrderings()) { + for (Ordering ordering : context.getQuery().getOrderings()) { processOrdering(qualifierTranslator, context, ordering); } } @@ -45,7 +40,7 @@ class OrderingDistictStage extends OrderingAbstractStage { private boolean isDistinct(TranslatorContext context) { return !context.isDistinctSuppression() - && (context.getQuery().isDistinct() - || context.getTableTree().hasToManyJoin()); + && (context.getQuery().isDistinct() + || context.getTableTree().hasToManyJoin()); } } diff --git a/cayenne/src/main/java/org/apache/cayenne/access/translator/select/OrderingGroupByStage.java b/cayenne/src/main/java/org/apache/cayenne/access/translator/select/OrderingGroupByStage.java index d577f4f7c..0dcbaa416 100644 --- a/cayenne/src/main/java/org/apache/cayenne/access/translator/select/OrderingGroupByStage.java +++ b/cayenne/src/main/java/org/apache/cayenne/access/translator/select/OrderingGroupByStage.java @@ -21,21 +21,18 @@ package org.apache.cayenne.access.translator.select; import org.apache.cayenne.query.Ordering; -/** - * @since 4.2.1 - */ class OrderingGroupByStage extends OrderingAbstractStage { @Override public void perform(TranslatorContext context) { - if(context.getQuery().getOrderings() == null) { + if (context.getQuery().getOrderings() == null) { return; } if (context.hasAggregate()) { // If query is GROUPING then we need to add the order column as a result column QualifierTranslator qualifierTranslator = context.getQualifierTranslator(); - for(Ordering ordering : context.getQuery().getOrderings()) { + for (Ordering ordering : context.getQuery().getOrderings()) { processOrdering(qualifierTranslator, context, ordering); } } diff --git a/cayenne/src/test/java/org/apache/cayenne/access/translator/select/OrderingStageTest.java b/cayenne/src/test/java/org/apache/cayenne/access/translator/select/OrderingStageTest.java index c360fd294..ae72e29d9 100644 --- a/cayenne/src/test/java/org/apache/cayenne/access/translator/select/OrderingStageTest.java +++ b/cayenne/src/test/java/org/apache/cayenne/access/translator/select/OrderingStageTest.java @@ -115,7 +115,7 @@ public class OrderingStageTest { ColumnExtractorStage columnStage = new ColumnExtractorStage(); columnStage.perform(context); - OrderingDistictStage orderingStage = new OrderingDistictStage(); + OrderingDistinctStage orderingStage = new OrderingDistinctStage(); orderingStage.perform(context); assertTrue(context.getQuery().isDistinct());