Jugen commented on code in PR #610:
URL: https://github.com/apache/cayenne/pull/610#discussion_r1529869933


##########
cayenne/src/main/java/org/apache/cayenne/access/translator/select/OrderingAbstractStage.java:
##########
@@ -57,51 +57,154 @@ protected void processOrdering(QualifierTranslator 
qualifierTranslator, Translat
         }
     }
 
-    private DbAttribute getOrderDbAttribute(Node translatedOrderNode)
+    private boolean orderColumnAbsent(TranslatorContext context, NodeBuilder 
nodeBuilder) 
     {
-        DbAttribute[] orderDbAttribute = {null};
-        translatedOrderNode.visit(new SimpleNodeTreeVisitor() {
-            @Override
-            public boolean onNodeStart(Node node) {
-                if (node.getType() == NodeType.COLUMN) {
-                    orderDbAttribute[0] = ((ColumnNode) node).getAttribute();
-                    return false;
-                }
-                return true;
+        OrderNodeVisitor orderVisitor = new OrderNodeVisitor();
+        nodeBuilder.build().visit( orderVisitor );
+        List<CharSequence> orderParts = orderVisitor.getParts();
+
+        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);
             }
-        });
-        return orderDbAttribute[0];
+        }
+
+        @Override
+        public void onNodeEnd(Node node) {
+            node.appendChildrenEnd(this);
+        }
+
+        List<CharSequence> getParts() {
+            return partList;
+        }
     }
 
-    private boolean orderColumnAbsent(TranslatorContext context, NodeBuilder 
nodeBuilder)
+    private class ResultNodeVisitor extends AppendableVisitor // see below
     {
-        var orderDbAttribute = getOrderDbAttribute(nodeBuilder.build());
-        if (orderDbAttribute == null) return false; // Alias ?
+        private List<CharSequence> orderItemParts;
+        private boolean itemsMatch = true;
+        private int lastIndex = 0;
 
-        var orderEntity = orderDbAttribute.getEntity().getName();
-        var orderColumn = orderDbAttribute.getName();
+        ResultNodeVisitor(List<CharSequence> orderParts) {
+            orderItemParts = orderParts;
+        }
 
-        Predicate<DbAttribute> columnAndEntity = dba -> dba != null
-                                    && orderColumn.equals(dba.getName())
-                                    && 
orderEntity.equals(dba.getEntity().getName());
+        @Override
+        public boolean onNodeStart(Node node) {
+            node.append(this);
+            if (node instanceof ColumnNode && ((ColumnNode) node).getAlias() 
!= null) {
+                partList.remove(partList.size()-1); // Remove appended alias

Review Comment:
   I had a look at this and due to line 153
   ```java
       if (!partList.get(x).equals(orderItemParts.get(x))) {
   ```
   where direct access to an element is required I think we'd have to use a 
`LinkedList` instead.
   However this will incur a minimal performance hit on `orderItemParts.get(x)` 
as the LinkedList has to be traversed from one end each time.
   
   As an alternative I could extend `ArrayList` adding _getLast()_ and 
_removeLast()_
   ```java
   private class ListArray extends ArrayList<CharSequence>
   {
       public CharSequence getLast() {
           return get(size()-1);
       }
       public CharSequence removeLast() {
           return remove(size()-1);
       }
   }
   ```
   Your thoughts ?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@cayenne.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to