raminqaf opened a new pull request, #29836:
URL: https://github.com/apache/flink/pull/29836
## What is the purpose of the change
A VARIANT is never cast implicitly, but when it met another type in `UNION
ALL`, `CASE`, `VALUES` or an `ARRAY` or `MAP` constructor, the planner picked a
common type anyway. The last input type won, and the other inputs were cast to
it without an explicit `CAST`. With `t(v)` holding `PARSE_JSON('1')` and
`PARSE_JSON('"x"')`:
```sql
SELECT v FROM t UNION ALL SELECT 'abc';
```
| Statement | Before
| After
|
|----------------------------------------|--------------------------------------------------------------------------------|---------------------------------------------------------------|
| `v UNION ALL 'abc'` | type `CHAR(3)`, rows `'1 '`, `'x
'`, `'abc'` | `Type mismatch in column 1 of
UNION ALL` |
| `'abc' UNION ALL v` | type `VARIANT`, rows `"abc"`,
`1`, `"x"` | `Type mismatch in column 1
of UNION ALL` |
| `CASE WHEN .. THEN v ELSE 1 END` | type `INT`, the job fails on
`"x"`: `Cannot cast a VARIANT STRING value to INTEGER.` | `Illegal mixing of
types in CASE or COALESCE statement` |
| `VALUES (PARSE_JSON('1')), ('abc')` | type `CHAR(3)`, rows `'1 '`,
`'abc'` | `Values passed to VALUES
operator must have compatible types` |
| `ARRAY[v, 'abc']`, `MAP['k', v, 'l', 2]` | `AssertionError: Conversion to
relational algebra failed to preserve datatypes` | `Parameters must be of the
same type` |
| `ARRAY[v] UNION ALL ARRAY[1]` | common type `ARRAY<INT>`
| `Type mismatch in column 1 of
UNION ALL` |
The new messages are the ones Flink already gives for other incompatible
types, e.g. `ARRAY[TRUE]` in place of the VARIANT. VARIANT with VARIANT,
including different nullability, and VARIANT with an untyped `NULL` still
resolve, so `COALESCE(v, PARSE_JSON('0'))` and `ARRAY[v, PARSE_JSON('2')]` work
as before.
The cause is the Calcite upgrade in FLINK-39817: `SqlTypeUtil.canCastFrom`
returns true whenever either side is a VARIANT, and
`SqlTypeFactoryImpl.leastRestrictiveByCast` then keeps the last type. 2.3.0
doesn't have that shortcut, and release-2.4 does. I plan to backport this to
release-2.4 so 2.4.0 never ships the implicit cast.
## Brief change log
- `FlinkTypeFactory.leastRestrictive` finds no common type when a VARIANT
meets any type other than VARIANT or an untyped `NULL`. The check also looks
inside ARRAY, MULTISET, MAP and ROW. When their elements have no common type,
Calcite still resolves the whole constructed type by cast, so a top-level check
alone would leave `ARRAY<VARIANT>` with `ARRAY<INT>` resolving to `ARRAY<INT>`.
The check lives in Flink's type factory because the copied Calcite classes stay
as they are.
## Verifying this change
This change added tests and can be verified as follows:
- `FlinkTypeFactoryTest#testLeastRestrictiveOfVariantAndOtherType` covers
VARIANT with STRING in both orders and VARIANT nested in ARRAY, MAP and ROW.
Without the fix it fails with `expected: null but was: INTEGER ARRAY` and
similar. With only a top-level check, the ARRAY, MAP and ROW cases still fail.
- `FlinkTypeFactoryTest#testLeastRestrictive` gets three cases that must
keep resolving: VARIANT with VARIANT of different nullability, VARIANT with
`NULL`, and the same nested in ARRAY.
- `VariantSemanticTest` gets `VARIANT_MIXED_WITH_OTHER_TYPE`, which pins
the user-facing messages for `UNION ALL`, `CASE`, `VALUES` and the `ARRAY`
constructor. Without the fix it fails with `Expecting code to raise a
throwable.`
- The other planner tests that use VARIANT pass, 3963 tests across 18
classes, among them `CastFunctionITCase`, `JsonFunctionsITCase`,
`VariantFunctionsITCase`, `ComparableTypeValidatorTest` and `CalcRestoreTest`.
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): no
- The public API, i.e., is any changed class annotated with
`@Public(Evolving)`: no
- The serializers: no
- The runtime per-record code paths (performance sensitive): no, the check
runs during planning
- Anything that affects deployment or recovery: JobManager (and its
components), Checkpointing, Kubernetes/Yarn, ZooKeeper: no
- The S3 file system connector: no
## Documentation
- Does this pull request introduce a new feature? no
- If yes, how is the feature documented? not applicable. The data types
docs already say a cast to VARIANT is never applied implicitly.
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes (please specify the tool below)
Generated-by: Claude Code 2.1.319 (Claude Opus 5.5)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
