raminqaf opened a new pull request, #29806:
URL: https://github.com/apache/flink/pull/29806
## What is the purpose of the change
SQL array access on a VARIANT returned NULL for a literal index below 1. The
same literal already fails during planning for a plain ARRAY, and for
`VARIANT.at(INT)` in the Table API. With a VARIANT column `v` holding `[1,
"hello", 3.14]`:
```sql
SELECT v[0] FROM t;
```
| Expression | Before | After
|
|------------------------|--------|-----------------------------------------------------------------|
| `v[0]` | `NULL` | `Array element access needs an index
starting at 1 but was 0.` |
| `v[-1]` | `NULL` | `Array element access needs an index
starting at 1 but was -1.` |
| `v[CAST(0 AS BIGINT)]` | `NULL` | `Array element access needs an index
starting at 1 but was 0.` |
| `v[i]` with `i = 0` | `NULL` | `NULL`
|
The index is 1-based, so such a literal can never address an element. A
computed index below 1 still returns NULL, as it does for ARRAY. DuckDB and
Trino, the other engines with a 1-based VARIANT index, also reject index 0.
A query with such a literal returned NULL for every row and now fails during
planning. That includes restoring a compiled plan from 2.3, where `v[i]` first
shipped, because the check runs during code generation.
## Brief change log
- `ScalarOperatorGens.generateVariantGet` runs the ARRAY literal-index
check. The check moves into a shared helper and matches a literal of any
integer type, since a VARIANT index can be TINYINT to BIGINT. ARRAY still only
reaches it with INT, because `requireInteger` runs first.
- The `variant '[' INT ']'` entry in `sql_functions.yml` says that a
literal index below 1 fails during planning.
## Verifying this change
This change added tests and can be verified as follows:
- Added `VARIANT_ARRAY_ACCESS_WITH_LITERAL_INDEX_BELOW_ONE` to
`VariantSemanticTest`. Without the fix it fails with `Expecting code to raise a
throwable.`
- The existing `VARIANT_ARRAY_ACCESS_WITH_NON_LITERAL_INDEX` covers a
computed index of 0, which still returns NULL.
- `ArrayTypeTest#testArrayIndexStaticCheckForSql` still pins the ARRAY
message.
## 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
---
##### 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]
