[
https://issues.apache.org/jira/browse/CALCITE-7812?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18118092#comment-18118092
]
Lino Rosa commented on CALCITE-7812:
------------------------------------
[~julianhyde] I'd be happy to reword it. I've been struggling with it for the
past week and I can attest it was very hard for me to understand it as well.
What I'm trying to convey is that because a `CAST` added by type coercion may
be incompatible with the target dialect. And the target dialect is potentially
already doing its own type coercion anyway. Proposal #2 would be a way to
maintain the existing validation logic while at the same time allowing for a
`CAST` that was introduced because of type coercion to be dropped by a dialect
that doesn't need it.
It's most likely that the only real issue this causes is with regard to
nullability, like in the example that affected us: type coercion introduced a
`CAST` to `NOT NULL` where in the target dialect it should be `NULL`.
More explicitly, given:
{code:java}
NAMED_STRUCT('freq', SUM("num")) {code}
... and assuming Calcite needs to coerce this into BIGINT for some reason, we'd
end up with, for Spark:
{code:java}
CAST(NAMED_STRUCT('freq', SUM(`num`)) AS STRUCT<`freq`: BIGINT NOT NULL>) {code}
Now we have a problem: That CAST has decided that {{freq}} will be {{{}NOT
NULL{}}}. But in Spark, the result of `SUM` is a `NULL`-able value. So this
will fail during query planning in Spark.
IMHO the crux of the problem is the coupling of type coercion with having a
`CAST` node injected on the syntax plan.{{{}{}}}
One could argue that the real problem is that `SUM` should be redefined on
Calcite to return a `NULL`-able result, but that's tricky - maybe impractical
to do for each dialect, or even impossible if they're closed source.
{{}}
> 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)
