[CALCITE-7620] Result of FILTER clause in window functions is incorrect

[xuzifu666]

jira: https://issues.apache.org/jira/browse/CALCITE-7620

[xuzifu666]

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.

[xuzifu666]

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.

[iwanttobepowerful]

another case

select ename, job, hiredate,
  avg(sal) over (order by hiredate rows 3 preceding) as avg_sal,
  avg(sal) filter (where job = 'MANAGER') over (order by hiredate rows 3 preceding)
    as avg_mgr_sal
from emp
order by hiredate;

[xuzifu666]

OK, this test has been added; the AVG_MGR_SAL field is related to this filter modification, and the data is consistent with https://onecompiler.com/postgresql/44smkpfxb

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
88.9% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[xuzifu666]

Do you happy with current changes? Because this issue affects the data quality of the filter, want to fix it as soon as possible so that this capability is usable in production. PTAL~ @iwanttobepowerful

[iwanttobepowerful]

Curious about the motivation behind this update?

[xuzifu666]

This fix altered the sorting metadata derivation for the window operator, resulting in a legitimate change to the execution plan structure of two existing !plan tests, but the computation results remain unchanged.

Specific changes: Previously, the collation derivation for EnumerableWindow was optimistic, claiming to retain the input sorting. With CalcRelSplitter inheriting the original Calc's collation, the optimizer would push Sort down onto the Window and merge it with the outer Calc. Therefore, the original plan was:

EnumerableSort
  EnumerableCalc(condition + window output)
      EnumerableWindow

After the fix, EnumerableWindow no longer claims any sorting, and CalcRelSplitter no longer inherits the contaminated collation. Sort can only stop at the Window, wrapped in an outer Calc for projection. The plan becomes:

EnumerableCalc(projection)
  EnumerableSort
    EnumerableCalc(condition)
      EnumerableWindow

Both are logically equivalent; only the relative positions of Sort and the projected Calc have changed. Therefore, only the expected output of these two !plan blocks was updated, without modifying any result data.This also ensures complete consistency with the PostgreSQL sorting results.

[xiedeyantu]

The collation reset should be scoped to the LogicalWindow creation rather than applied in CalcRelSplitter’s constructor. The ordering issue is specific to window nodes: a Window does not preserve or expose a simple output collation, so it should not inherit the original Calc collation trait.
Suggested change:
ProjectToWindowRule.java:260

           @Override protected RelNode makeRel(RelOptCluster cluster, RelTraitSet traitSet,
               RelBuilder relBuilder, RelNode input, RexProgram program, List<RelHint> hints) {
             checkArgument(program.getCondition() == null,
                 "WindowedAggregateRel cannot accept a condition");
+            traitSet = traitSet.replaceIfs(RelCollationTraitDef.INSTANCE,
+                Collections::emptyList);
             return LogicalWindow.create(cluster, traitSet, relBuilder, input,
                 program, hints);
           }

This keeps the generic splitter behavior unchanged and makes the intent local to the window-producing RelType.

[xiedeyantu]

I rarely see the need for @nullable in code.

[xiedeyantu]

Suggested change

	Util.discard(mq);
	Util.discard(input);
	Util.discard(groups);
	return Collections.emptyList();
	return Collections.emptyList();

[xiedeyantu]

I'm unsure if the semantics of functions like FIRST_VALUE, LAST_VALUE, NTH_VALUE, LEAD, and LAG are correct after the rewrite.