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:
[email protected]
With regards,
Apache Git Services
