[GitHub] [calcite] zhztheplayer commented on a change in pull request #897: [CALCITE-2648] Output collation of EnumerableWindow is not consistent…

[GitBox <gi...@apache.org>]

zhztheplayer commented on a change in pull request #897: [CALCITE-2648] Output collation of EnumerableWindow is not consistent…
URL: https://github.com/apache/calcite/pull/897#discussion_r286763898
 
 

 ##########
 File path: core/src/main/java/org/apache/calcite/rel/logical/LogicalWindow.java
 ##########
 @@ -274,12 +277,24 @@ public RexNode visitLocalRef(RexLocalRef localRef) {
       final RexInputRef ref = (RexInputRef) refToWindow.get(index);
       projectList.add(ref);
     }
-
-    return relBuilder.push(window)
+    RelNode sorted = requireSorted(window,
 
 Review comment:
   Sorry for the late reply, I've been working on some other issues these days.
   
   >   I believe, most of the implementation would sort the rows as it is prescribed by order by, so it makes sense to have those properties even at logical level, so the optimizer can remove redundant sorts, etc.
   
   @vlsi - Wouldn't having the properties in Enumerable level or user-defined convention level helps optimizer remove redundant sorts as well? SortRemoveRule doesn't force its input to be a logical node.
   
   > In case the "agreed" change is to make "LogicalWindow" to always have "empty collation", then LogicalProject with OVER expressions should better have EMPTY collation as well in order to keep consistency between different relations.
   
   Agreed. And I would write separate methods for logical window and enumerable window in RelMdCollation, returning empty collation/implementation collation respectively. Of course we should achieve agreement on whether to wipe LogicalWindow's collation before doing that.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services