[
https://issues.apache.org/jira/browse/CALCITE-7812?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18119205#comment-18119205
]
Lino Rosa commented on CALCITE-7812:
------------------------------------
Maybe it helps getting back to specifics after all this discussion.
h3. Reiterating the original problem
Given this operation:
{code:java}
SUM(x){code}
If for any reason Calcite needs to do type coercion here, it would add the cast:
{code:java}
CAST SUM(x) AS INTEGER NOT NULL{code}
Once this is unparsed into a dialect where {{SUM}} is {{{}NULL{}}}-able, it
will crash at runtime.
h3. Reiterating your rebuttal
My suggestion of dropping the {{CAST}} based on the dialect goes against
Calcite's philosophy where Calcite enforces Calcite's semantics.
I recognize my suggestion may also may introduce other semantic
inconsistencies. I won't push for it any further.
h3. What I want to achieve
The original problem remains. Currently `SUM` will, in some situations,
generate queries in Spark that do not run.
I can see no other option that to allow overriding the definitions of build-in
{{{}SqlOperation{}}}s. Developers could then override {{{}SUM{}}}'s return type
to be compatible with a particular dialect. This is already somewhat possible
with overriding {{{}RelDataTypeSystem#deriveSumType{}}}. Maybe Calcite would
benefit from a more generic way to allow overriding of return types of all
built-in functions?
> 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
> Assignee: krooswu
> 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)
