[ 
https://issues.apache.org/jira/browse/CALCITE-7616?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

leishp updated CALCITE-7616:
----------------------------
    Description: 
When a JDBC schema's dialect supports window functions (e.g. MySQL, 
PostgreSQL), a query with CTE and RANK() throws AssertionError during 
VolcanoPlanner optimization:
{code:java}
AssertionError: Relational expression
  LogicalWindow.JDBC.TEST.[]
  has calling-convention JDBC.TEST but does not implement the required interface
  'interface org.apache.calcite.adapter.jdbc.JdbcRel' of that convention
{code}
h3. Reproduction

A JDBC schema with MySQL dialect ({{{}supportsWindowFunctions() = true{}}}), 
and a query with CTE + RANK():
{code:sql}
WITH ranked AS (
  SELECT customer_name, contract_amount,
    RANK() OVER (ORDER BY contract_amount DESC) AS rnk
  FROM sales_data
)
SELECT customer_name FROM ranked WHERE rnk <= 3
{code}
h3. Root cause

{{ProjectToLogicalProjectAndWindowRule}} passes the parent {{{}Project{}}}'s 
traitSet directly to {{LogicalWindow.create()}} via {{{}CalcRelSplitter{}}}. 
When the parent is a {{JdbcProject}} (JDBC convention), the generated 
{{LogicalWindow}} inherits the JDBC convention but does not implement the 
{{JdbcRel}} interface, causing an AssertionError in 
{{{}VolcanoPlanner.registerImpl{}}}.
{code:java}
// ProjectToWindowRule.java:161 — passes project.getTraitSet() directly
final LogicalCalc calc =
    new LogicalCalc(project.getCluster(), project.getTraitSet(),  // JDBC 
convention leaks in
        project.getHints(), input, program);
{code}
This is related to CALCITE-3352 (wrong collation on generated Window) — both 
bugs share the same root cause: {{ProjectToWindowRule}} blindly propagates the 
parent's traitSet to the generated {{{}LogicalWindow{}}}. Danny Chen commented 
on CALCITE-3352:
{quote}We should limit this rule to only match logical nodes because it is a 
plan rewrite.
{quote}
h3. Fix

Force {{Convention.NONE}} when creating the temporary {{{}LogicalCalc{}}}, so 
that {{LogicalWindow}} never inherits a non-NONE convention:
{code:java}
// ProjectToWindowRule.java
final RelTraitSet traitSet =
    project.getTraitSet().replace(Convention.NONE);
final LogicalCalc calc =
    new LogicalCalc(project.getCluster(), traitSet,
        project.getHints(), input, program); 
{code}
h3. Impact
 - When dialect supports window functions: window function is pushed down to 
database via {{{}JdbcProject{}}}. Correct behavior.
 - When dialect does NOT support window functions: 
{{ProjectToLogicalProjectAndWindowRule}} fires on {{LogicalProject(NONE)}} as 
before. No change.
 - No existing tests broken (verified {{RelOptRulesTest}} 902/902 passed).

h3. Scope

This is a targeted fix for the convention issue only. The collation issue 
(CALCITE-3352) is tracked separately.
h3. Test

A JUnit test {{JdbcWindowConventionTest}} is provided in the PR, using HSQLDB 
in-memory database with MySQL dialect override to reproduce the bug without an 
external database.

  was:
When a JDBC schema's dialect supports window functions (e.g. MySQL, 
PostgreSQL), a query with CTE and RANK() throws AssertionError during 
VolcanoPlanner optimization:

{code}
AssertionError: Relational expression
  LogicalWindow.JDBC.TEST.[]
  has calling-convention JDBC.TEST but does not implement the required interface
  'interface org.apache.calcite.adapter.jdbc.JdbcRel' of that convention
{code}

h3. Reproduction

A JDBC schema with MySQL dialect ({{supportsWindowFunctions() = true}}), and a 
query with CTE + RANK():

{code:sql}
WITH ranked AS (
  SELECT customer_name, contract_amount,
    RANK() OVER (ORDER BY contract_amount DESC) AS rnk
  FROM sales_data
)
SELECT customer_name FROM ranked WHERE rnk <= 3
{code}

h3. Root cause

{{ProjectToLogicalProjectAndWindowRule}} passes the parent {{Project}}'s 
traitSet directly to {{LogicalWindow.create()}} via {{CalcRelSplitter}}. When 
the parent is a {{JdbcProject}} (JDBC convention), the generated 
{{LogicalWindow}} inherits the JDBC convention but does not implement the 
{{JdbcRel}} interface, causing an AssertionError in 
{{VolcanoPlanner.registerImpl}}.

{code:java}
// ProjectToWindowRule.java:161 — passes project.getTraitSet() directly
final LogicalCalc calc =
    new LogicalCalc(project.getCluster(), project.getTraitSet(),  // JDBC 
convention leaks in
        project.getHints(), input, program);
{code}

This is related to CALCITE-3352 (wrong collation on generated Window) — both 
bugs share the same root cause: {{ProjectToWindowRule}} blindly propagates the 
parent's traitSet to the generated {{LogicalWindow}}. Danny Chen commented on 
CALCITE-3352:

{quote}
We should limit this rule to only match logical nodes because it is a plan 
rewrite.
{quote}

h3. Fix

Force {{Convention.NONE}} when creating the temporary {{LogicalCalc}}, so that 
{{LogicalWindow}} never inherits a non-NONE convention:

{code:java}
// ProjectToWindowRule.java
final RelTraitSet traitSet =
    project.getCluster().traitSetOf(Convention.NONE);  // force Convention.NONE
final LogicalCalc calc =
    new LogicalCalc(project.getCluster(), traitSet,
        project.getHints(), input, program);
{code}

h3. Impact

- When dialect supports window functions: window function is pushed down to 
database via {{JdbcProject}}. Correct behavior.
- When dialect does NOT support window functions: 
{{ProjectToLogicalProjectAndWindowRule}} fires on {{LogicalProject(NONE)}} as 
before. No change.
- No existing tests broken (verified {{RelOptRulesTest}} 902/902 passed).

h3. Scope

This is a targeted fix for the convention issue only. The collation issue 
(CALCITE-3352) is tracked separately.

h3. Test

A JUnit test {{JdbcWindowConventionTest}} is provided in the PR, using HSQLDB 
in-memory database with MySQL dialect override to reproduce the bug without an 
external database.



> ProjectToLogicalProjectAndWindowRule should not match non-logical Project 
> nodes with JDBC convention
> ----------------------------------------------------------------------------------------------------
>
>                 Key: CALCITE-7616
>                 URL: https://issues.apache.org/jira/browse/CALCITE-7616
>             Project: Calcite
>          Issue Type: Bug
>          Components: core
>            Reporter: leishp
>            Assignee: leishp
>            Priority: Major
>              Labels: pull-request-available
>
> When a JDBC schema's dialect supports window functions (e.g. MySQL, 
> PostgreSQL), a query with CTE and RANK() throws AssertionError during 
> VolcanoPlanner optimization:
> {code:java}
> AssertionError: Relational expression
>   LogicalWindow.JDBC.TEST.[]
>   has calling-convention JDBC.TEST but does not implement the required 
> interface
>   'interface org.apache.calcite.adapter.jdbc.JdbcRel' of that convention
> {code}
> h3. Reproduction
> A JDBC schema with MySQL dialect ({{{}supportsWindowFunctions() = true{}}}), 
> and a query with CTE + RANK():
> {code:sql}
> WITH ranked AS (
>   SELECT customer_name, contract_amount,
>     RANK() OVER (ORDER BY contract_amount DESC) AS rnk
>   FROM sales_data
> )
> SELECT customer_name FROM ranked WHERE rnk <= 3
> {code}
> h3. Root cause
> {{ProjectToLogicalProjectAndWindowRule}} passes the parent {{{}Project{}}}'s 
> traitSet directly to {{LogicalWindow.create()}} via {{{}CalcRelSplitter{}}}. 
> When the parent is a {{JdbcProject}} (JDBC convention), the generated 
> {{LogicalWindow}} inherits the JDBC convention but does not implement the 
> {{JdbcRel}} interface, causing an AssertionError in 
> {{{}VolcanoPlanner.registerImpl{}}}.
> {code:java}
> // ProjectToWindowRule.java:161 — passes project.getTraitSet() directly
> final LogicalCalc calc =
>     new LogicalCalc(project.getCluster(), project.getTraitSet(),  // JDBC 
> convention leaks in
>         project.getHints(), input, program);
> {code}
> This is related to CALCITE-3352 (wrong collation on generated Window) — both 
> bugs share the same root cause: {{ProjectToWindowRule}} blindly propagates 
> the parent's traitSet to the generated {{{}LogicalWindow{}}}. Danny Chen 
> commented on CALCITE-3352:
> {quote}We should limit this rule to only match logical nodes because it is a 
> plan rewrite.
> {quote}
> h3. Fix
> Force {{Convention.NONE}} when creating the temporary {{{}LogicalCalc{}}}, so 
> that {{LogicalWindow}} never inherits a non-NONE convention:
> {code:java}
> // ProjectToWindowRule.java
> final RelTraitSet traitSet =
>     project.getTraitSet().replace(Convention.NONE);
> final LogicalCalc calc =
>     new LogicalCalc(project.getCluster(), traitSet,
>         project.getHints(), input, program); 
> {code}
> h3. Impact
>  - When dialect supports window functions: window function is pushed down to 
> database via {{{}JdbcProject{}}}. Correct behavior.
>  - When dialect does NOT support window functions: 
> {{ProjectToLogicalProjectAndWindowRule}} fires on {{LogicalProject(NONE)}} as 
> before. No change.
>  - No existing tests broken (verified {{RelOptRulesTest}} 902/902 passed).
> h3. Scope
> This is a targeted fix for the convention issue only. The collation issue 
> (CALCITE-3352) is tracked separately.
> h3. Test
> A JUnit test {{JdbcWindowConventionTest}} is provided in the PR, using HSQLDB 
> in-memory database with MySQL dialect override to reproduce the bug without 
> an external database.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to