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

Lino Rosa updated CALCITE-7812:
-------------------------------
    Description: 
h2. Problem

When type coercion is on, {{TypeCoercionImpl}} rewrites the validated 
{{SqlNode}} tree. It wraps operands and select items in {{{}CAST(x AS T){}}}, 
where {{T}} is the type Calcite's own rules chose. These casts become ordinary 
{{RexCall(CAST)}} nodes after {{{}SqlToRelConverter{}}}. {{RelToSqlConverter}} 
then unparses them into the SQL sent to the target engine.

That is fine only when Calcite and the engine type the expression in a 
compatible way. Otherwise it generates invalid queries.
h4. Example: struct fields and {{SUM}} nullability on Spark
{code:java}
-- target: t2 (s ROW(a DOUBLE))
-- source: t1 (k INT NOT NULL, x BIGINT NOT NULL)
INSERT INTO t2 (s)
SELECT ROW(SUM(x)) FROM t1 GROUP BY k {code}
 # Calcite types {{SUM}} as {{{}BIGINT NOT NULL{}}}, because {{x}} is {{NOT 
NULL}} and the query is grouped.
 # {{coerceColumnType}} wraps the row in a {{CAST}} to the target struct type.
 # {{SqlTypeUtil.convertTypeToSpec}} loses per-field nullability 
(CALCITE-6932). The unparsed cast spells the field {{{}NOT NULL{}}}.
 # Spark types {{SUM}} as nullable. It refuses to cast a struct with a nullable 
field to one with a {{NOT NULL}} field, so the query fails at analysis. Fixing 
CALCITE-6932 alone would not settle this. The cast still carries Calcite's view 
of nullability, which is correct for Calcite and wrong for Spark. The engine 
would have accepted the uncast expression.

h4. Existing workaround is not enough

{{SqlDialect.supportsImplicitTypeCoercion}} together with 
{{SqlImplementor.stripCastFromString}} already recognizes that engines coerce 
for themselves. Its scope is limited but its existing is promising and may lead 
to a fix.
h3. Proposals
h4. 1. Carry the coerced type outside the tree
h4. The validator would record the coerced type without rewriting the tree, and 
conversion would pick it up from there. This is the cleanest option but hard 
today, because the tree is the only place coercion is stored. Much of the 
downstream logic (e.g. {{FamilyOperandTypeChecker, }}{{SqlToRelConverter) rely 
on the existence of this CAST.}}
h4. {{{{}}{}}}2. Mark coercion casts, then let the dialect decide (preferred)
h4. Keep inserting a cast, but make it distinguishable from a user cast. Two 
ways to do that:
 * A dedicated operator of kind {{{}CAST{}}}, for example 
{{{}SqlStdOperatorTable.IMPLICIT_CAST{}}}.
 * A flag on the cast.

The marker has to survive {{{{{}SqlToRelConverter{}}}}} and appear on the 
{{{}RexCall{}}}. Because its kind stays {{{}CAST{}}}, {{RexSimplify}} and the 
rules keep treating it as a cast.
 

And here we go back to {{SqlDialect.supportsImplicitTypeCoercion.}} We can now 
simply unparse this marker as either a \{{CAST }}or not unparse anything 
depending on that flag.

  was:
h2. Problem

When type coercion is on, {{TypeCoercionImpl}} rewrites the validated 
{{SqlNode}} tree. It wraps operands and select items in {{{}CAST(x AS T){}}}, 
where {{T}} is the type Calcite's own rules chose. These casts become ordinary 
{{RexCall(CAST)}} nodes after {{{}SqlToRelConverter{}}}. {{RelToSqlConverter}} 
then unparses them into the SQL sent to the target engine.

That is fine only when Calcite and the engine type the expression in a 
compatible way. Otherwise it generates invalid queries.
h4. Example: struct fields and {{SUM}} nullability on Spark
{code:java}
-- target: t2 (s ROW(a DOUBLE))
-- source: t1 (k INT NOT NULL, x BIGINT NOT NULL)
INSERT INTO t2 (s)
SELECT ROW(SUM(x)) FROM t1 GROUP BY k {code}
Calcite types {{SUM}} as {{{}BIGINT NOT NULL{}}}, because {{x}} is {{NOT NULL}} 
and the query is grouped.

{{coerceColumnType}} wraps the row in a {{CAST}} to the target struct type.

{{SqlTypeUtil.convertTypeToSpec}} loses per-field nullability (CALCITE-6932). 
The unparsed cast spells the field {{{}NOT NULL{}}}.

Spark types {{SUM}} as nullable. It refuses to cast a struct with a nullable 
field to one with a {{NOT NULL}} field, so the query fails at analysis. Fixing 
CALCITE-6932 alone would not settle this. The cast still carries Calcite's view 
of nullability, which is correct for Calcite and wrong for Spark. The engine 
would have accepted the uncast expression.
h4. Existing workaround is not enough

{{SqlDialect.supportsImplicitTypeCoercion}} together with 
{{SqlImplementor.stripCastFromString}} already recognizes that engines coerce 
for themselves. Its scope is limited but its existing is promising and may lead 
to a fix.
h3. Proposals
h4. 1. Carry the coerced type outside the tree
h4. The validator would record the coerced type without rewriting the tree, and 
conversion would pick it up from there. This is the cleanest option but hard 
today, because the tree is the only place coercion is stored. Much of the 
downstream logic (e.g. {{FamilyOperandTypeChecker, }}{{SqlToRelConverter) rely 
on the existence of this CAST.}}
h4. {{{{}}{}}}2. Mark coercion casts, then let the dialect decide (preferred)
h4. Keep inserting a cast, but make it distinguishable from a user cast. Two 
ways to do that:
 * A dedicated operator of kind {{{}CAST{}}}, for example 
{{{}SqlStdOperatorTable.IMPLICIT_CAST{}}}.
 * A flag on the cast.

The marker has to survive {{{{{}SqlToRelConverter{}}}}} and appear on the 
{{{}RexCall{}}}. Because its kind stays {{{}CAST{}}}, {{RexSimplify}} and the 
rules keep treating it as a cast.
 

And here we go back to {{SqlDialect.supportsImplicitTypeCoercion.}} We can now 
simply unparse this marker as either a \{{CAST }}or not unparse anything 
depending on that flag.


> Implicit type coercion emits CASTs in unparsed SQL that the target dialect 
> rejects
> ----------------------------------------------------------------------------------
>
>                 Key: CALCITE-7812
>                 URL: https://issues.apache.org/jira/browse/CALCITE-7812
>             Project: Calcite
>          Issue Type: Bug
>            Reporter: Lino Rosa
>            Priority: Major
>
> h2. Problem
> When type coercion is on, {{TypeCoercionImpl}} rewrites the validated 
> {{SqlNode}} tree. It wraps operands and select items in {{{}CAST(x AS T){}}}, 
> where {{T}} is the type Calcite's own rules chose. These casts become 
> ordinary {{RexCall(CAST)}} nodes after {{{}SqlToRelConverter{}}}. 
> {{RelToSqlConverter}} then unparses them into the SQL sent to the target 
> engine.
> That is fine only when Calcite and the engine type the expression in a 
> compatible way. Otherwise it generates invalid queries.
> h4. Example: struct fields and {{SUM}} nullability on Spark
> {code:java}
> -- target: t2 (s ROW(a DOUBLE))
> -- source: t1 (k INT NOT NULL, x BIGINT NOT NULL)
> INSERT INTO t2 (s)
> SELECT ROW(SUM(x)) FROM t1 GROUP BY k {code}
>  # Calcite types {{SUM}} as {{{}BIGINT NOT NULL{}}}, because {{x}} is {{NOT 
> NULL}} and the query is grouped.
>  # {{coerceColumnType}} wraps the row in a {{CAST}} to the target struct type.
>  # {{SqlTypeUtil.convertTypeToSpec}} loses per-field nullability 
> (CALCITE-6932). The unparsed cast spells the field {{{}NOT NULL{}}}.
>  # Spark types {{SUM}} as nullable. It refuses to cast a struct with a 
> nullable field to one with a {{NOT NULL}} field, so the query fails at 
> analysis. Fixing CALCITE-6932 alone would not settle this. The cast still 
> carries Calcite's view of nullability, which is correct for Calcite and wrong 
> for Spark. The engine would have accepted the uncast expression.
> h4. Existing workaround is not enough
> {{SqlDialect.supportsImplicitTypeCoercion}} together with 
> {{SqlImplementor.stripCastFromString}} already recognizes that engines coerce 
> for themselves. Its scope is limited but its existing is promising and may 
> lead to a fix.
> h3. Proposals
> h4. 1. Carry the coerced type outside the tree
> h4. The validator would record the coerced type without rewriting the tree, 
> and conversion would pick it up from there. This is the cleanest option but 
> hard today, because the tree is the only place coercion is stored. Much of 
> the downstream logic (e.g. {{FamilyOperandTypeChecker, }}{{SqlToRelConverter) 
> rely on the existence of this CAST.}}
> h4. {{{{}}{}}}2. Mark coercion casts, then let the dialect decide (preferred)
> h4. Keep inserting a cast, but make it distinguishable from a user cast. Two 
> ways to do that:
>  * A dedicated operator of kind {{{}CAST{}}}, for example 
> {{{}SqlStdOperatorTable.IMPLICIT_CAST{}}}.
>  * A flag on the cast.
> The marker has to survive {{{{{}SqlToRelConverter{}}}}} and appear on the 
> {{{}RexCall{}}}. Because its kind stays {{{}CAST{}}}, {{RexSimplify}} and the 
> rules keep treating it as a cast.
>  
> And here we go back to {{SqlDialect.supportsImplicitTypeCoercion.}} We can 
> now simply unparse this marker as either a \{{CAST }}or not unparse anything 
> depending on that flag.



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

Reply via email to