[
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 whose dialect reports {{supportsWindowFunctions() = true}}
(e.g. MySQL), and a window query:
{code:sql}
SELECT empno, RANK() OVER (ORDER BY sal DESC) AS rnk
FROM emp
{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
Narrow the rule's operand from {{Project.class}} to {{LogicalProject.class}},
so it no longer matches non-logical projects such as {{JdbcProject}}. This rule
is a *logical plan rewrite* (it
emits a {{LogicalWindow}} of Convention.NONE), so it must only apply to logical
nodes — exactly as Danny Chen advised on CALCITE-3352. With the operand
narrowed, the windowed expression stays
in {{JdbcProject}} and is pushed down to the database by {{JdbcProjectRule}}
when the dialect supports window functions.
{code:java}
// ProjectToWindowRule.java — ProjectToLogicalProjectAndWindowRuleConfig
.withOperandSupplier(b ->
b.operand(LogicalProject.class) // was Project.class
.predicate(Project::containsOver)
.anyInputs())
{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
Added {{JdbcAdapterTest#testWindowFunctionJdbcConvention}} plus a nested
{{WindowSupportingJdbcSchemaFactory}}, in the *existing* {{JdbcAdapterTest}}
(not a new top-level class, to avoid
changing the Gradle test-class execution order). It runs against an in-memory
HSQLDB with a MySQL dialect override ({{supportsWindowFunctions() = true}}) and
asserts the planner no longer
throws the "calling-convention" AssertionError.
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: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.
> 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
> 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 whose dialect reports {{supportsWindowFunctions() = true}}
> (e.g. MySQL), and a window query:
> {code:sql}
> SELECT empno, RANK() OVER (ORDER BY sal DESC) AS rnk
> FROM emp
> {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
> Narrow the rule's operand from {{Project.class}} to {{LogicalProject.class}},
> so it no longer matches non-logical projects such as {{JdbcProject}}. This
> rule is a *logical plan rewrite* (it
> emits a {{LogicalWindow}} of Convention.NONE), so it must only apply to
> logical nodes — exactly as Danny Chen advised on CALCITE-3352. With the
> operand narrowed, the windowed expression stays
> in {{JdbcProject}} and is pushed down to the database by {{JdbcProjectRule}}
> when the dialect supports window functions.
> {code:java}
> // ProjectToWindowRule.java — ProjectToLogicalProjectAndWindowRuleConfig
> .withOperandSupplier(b ->
> b.operand(LogicalProject.class) // was Project.class
> .predicate(Project::containsOver)
> .anyInputs())
> {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
> Added {{JdbcAdapterTest#testWindowFunctionJdbcConvention}} plus a nested
> {{WindowSupportingJdbcSchemaFactory}}, in the *existing* {{JdbcAdapterTest}}
> (not a new top-level class, to avoid
> changing the Gradle test-class execution order). It runs against an
> in-memory HSQLDB with a MySQL dialect override ({{supportsWindowFunctions() =
> true}}) and asserts the planner no longer
> throws the "calling-convention" AssertionError.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
