xuzifu666 commented on code in PR #5040:
URL: https://github.com/apache/calcite/pull/5040#discussion_r3457041800
##########
core/src/main/java/org/apache/calcite/rel/rules/CalcRelSplitter.java:
##########
@@ -113,7 +114,8 @@ public abstract class CalcRelSplitter {
this.program = calc.getProgram();
this.hints = calc.getHints();
this.cluster = calc.getCluster();
- this.traits = calc.getTraitSet();
+ this.traits = calc.getTraitSet()
Review Comment:
The reason for modifying CalcRelSplitter.java is that when
ProjectToWindowRule splits Calc/Project containing window functions, it passes
the original node's trait set (including the contaminated collation) to the new
node after splitting, causing the optimizer to incorrectly remove the top-level
Sort before the window expands.
##########
core/src/main/java/org/apache/calcite/rel/metadata/RelMdCollation.java:
##########
@@ -372,13 +373,16 @@ public static List<RelCollation> sort(RelCollation
collation) {
/** Helper method to determine a
* {@link org.apache.calcite.rel.core.Window}'s collation.
*
- * <p>A Window projects the fields of its input first, followed by the output
- * from each of its windows. Assuming (quite reasonably) that the
- * implementation does not re-order its input rows, then any collations of
its
- * input are preserved. */
+ * <p>A Window operator groups rows by PARTITION BY keys and sorts each
+ * partition by ORDER BY keys. The output order is therefore not defined by
+ * a simple collation in the general case, so we conservatively report no
+ * collations. */
public static @Nullable List<RelCollation> window(RelMetadataQuery mq,
RelNode input,
Review Comment:
The reason for modifying ```RelMdCollation.window``` is that the original
window sorting derivation was too optimistic, which would cause the optimizer
to mistakenly believe that the window output retained the input order, thus
mistakenly deleting the top-level Sort.
The original implementation had the following problem:
1. Previously, `RelMdCollation.window` directly returned
`mq.collations(input)`, meaning "the window operator will preserve the order of
the input rows as is." However, the actual implementation of `EnumerableWindow`
first groups the rows by the `PARTITION BY` key using `SortedMultiMap`, and
then sorts them within each group by the `window ORDER BY` key. Therefore, the
input order is not preserved; the global output order is `PARTITION BY keys +
ORDER BY keys`, not simply the input order.
This caused the top-level `Sort` to be incorrectly optimized away.
2. When `order by empno` is written in the SQL, if the window also happens
to be sorted by `empno`, the optimizer will mistakenly assume that the window
output is globally ordered, thus deleting the top-level `EnumerableSort`. The
resulting output is grouped by `deptno`, not sorted by `empno`.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
