You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "hulincup (via GitHub)" <gi...@apache.org> on 2026/08/06 01:45:35 UTC
[PR] [FLINK-40338][table-runtime] ELT() throws ClassCastException when index is not INT [flink]
hulincup opened a new pull request, #28931:
URL: https://github.com/apache/flink/pull/28931
## Problem fixed & how
`ELT(index, expr, exprs...)` accepts any `INTEGER_NUMERIC` index (`TINYINT`/`SMALLINT`/`INT`/`BIGINT`). However, `EltFunction#eval` declares `index` as `java.lang.Number` and indexes the varargs array with `exprs[(int) index - 1]`. Per JLS 5.5, casting a `Number` reference to `int` compiles to a `checkcast` to `Integer` followed by unboxing, so a `Byte`, `Short`, or `Long` value throws `ClassCastException` on the success path (`1 <= index <= exprs.length`).
```sql
SELECT ELT(CAST(2 AS BIGINT), 'scala', 'java');
-- java.lang.ClassCastException: class java.lang.Long cannot be cast to class java.lang.Integer
```
Same for `CAST(2 AS TINYINT)` and `CAST(2 AS SMALLINT)`.
The out-of-range guard above the cast uses `index.longValue()`, so out-of-range indices of any type still return `NULL` correctly; the exception only fires in the valid range. That is why the existing test `ELT(9223372036854775807, 'ab', 'b')` passes (returns `NULL` before reaching the cast) and every other existing test uses an `INT` literal.
Present since [FLINK-35987](https://issues.apache.org/jira/browse/FLINK-35987) introduced ELT; confirmed absent from release-1.20 and present from release-2.0.
**Fix:** narrow the already-unboxed `long idx` (computed above for the range check) via primitive narrowing `(int) idx` (JLS 5.1.3, no `checkcast`) instead of casting the `Number` reference.
## Behavior modified
- **previous:** `ELT` with a non-INT `INTEGER_NUMERIC` index (`TINYINT`/`SMALLINT`/`BIGINT`) in the valid range `1 <= index <= exprs.length` threw `ClassCastException`.
- **now:** non-INT integer indices correctly return the corresponding expression.
- **impact:** only the success path for non-INT integer indices; `INT` indices, `NULL`, and out-of-range behavior are unchanged.
## Code refactored
`EltFunction.eval`: `exprs[(int) index - 1]` → `exprs[(int) idx - 1]`, with a 3-line comment explaining the JLS rationale.
## Features added
N/A
## Functions optimized
N/A
## Test plan
- [x] Added 3 regression cases to `StringFunctionsITCase.eltTestCases()` covering `TINYINT`, `SMALLINT`, and `BIGINT` indices (the three types that previously threw `ClassCastException`). Each expects the correct expression (`"java"` for index 2).
- [ ] The local environment runs Java 8 and cannot build Flink master (requires Java 11+), so verification relies on CI:
```
mvn -pl flink-table/flink-table-planner -am test -Dtest=StringFunctionsITCase
```
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-40338][table-runtime] ELT() throws ClassCastException when index is not INT [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #28931:
URL: https://github.com/apache/flink/pull/28931#discussion_r3734057653
##########
flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/scalar/EltFunction.java:
##########
@@ -40,6 +40,9 @@ public EltFunction(SpecializedContext context) {
if (idx < 1 || idx > exprs.length) {
return null;
}
- return exprs[(int) index - 1];
+ // Narrow the already-unboxed long instead of casting the Number reference.
+ // Casting `index` (java.lang.Number) to int compiles to a checkcast to Integer
+ // followed by unboxing, which throws ClassCastException for Byte/Short/Long.
Review Comment:
other wise with such approach we should comment on every line the same which I don't think is the right way
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-40338][table-runtime] ELT() throws ClassCastException when index is not INT [flink]
Posted by "SEPURI-SAI-KRISHNA (via GitHub)" <gi...@apache.org>.
SEPURI-SAI-KRISHNA commented on code in PR #28931:
URL: https://github.com/apache/flink/pull/28931#discussion_r3729216289
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/StringFunctionsITCase.java:
##########
@@ -182,6 +182,22 @@ private Stream<TestSetSpec> eltTestCases() {
DataTypes.VARCHAR(5))
.testResult(
lit(2).elt("a", "b"), "ELT(2, 'a', 'b')", "b", DataTypes.CHAR(1))
+ // FLINK-40338: non-INT INTEGER_NUMERIC index must not throw ClassCastException
+ .testResult(
+ lit(2).cast(DataTypes.TINYINT()).elt("scala", "java"),
+ "ELT(CAST(2 AS TINYINT), 'scala', 'java')",
+ "java",
+ DataTypes.VARCHAR(5))
+ .testResult(
+ lit(2).cast(DataTypes.SMALLINT()).elt("scala", "java"),
+ "ELT(CAST(2 AS SMALLINT), 'scala', 'java')",
+ "java",
+ DataTypes.VARCHAR(5))
+ .testResult(
+ lit(2).cast(DataTypes.BIGINT()).elt("scala", "java"),
+ "ELT(CAST(2 AS BIGINT), 'scala', 'java')",
+ "java",
+ DataTypes.VARCHAR(5))
Review Comment:
Thanks for picking this up, the fix itself looks right. One coverage gap worth closing before merge.
All three new cases pass constant arguments, so `ExpressionReducer` folds the entire call during optimization and the generated runtime code is never reached. The plans:
```
-- ELT(CAST(2 AS TINYINT), 'scala', 'java')
== Optimized Execution Plan ==
Calc(select=[CAST('java' AS VARCHAR(5)) AS EXPR$0]) <- ELT folded away
-- ELT(b, 'scala', 'java') where b TINYINT
== Optimized Execution Plan ==
Calc(select=[ELT(b, 'scala', 'java') AS EXPR$0]) <- ELT reaches the operator
```
To be clear, these cases *do* fail without the fix, because the reducer executes the function at plan time. But they only cover the constant-folding path, not the codegen'd operator path that a real job hits, so a future regression in the runtime path wouldn't be caught here.
The case just below at line 201 already uses the field-reference pattern, so extending the existing fields covers it:
```java
.onFieldsWithData(null, null, null, new byte[] {1, 2, 3}, (byte) 2, (short) 2, 2L)
.andDataTypes(
DataTypes.INT(), DataTypes.STRING(), DataTypes.BYTES(), DataTypes.BYTES(),
DataTypes.TINYINT(), DataTypes.SMALLINT(), DataTypes.BIGINT())
```
and then `$("f4").elt("scala", "java")`, `$("f5")`, `$("f6")`, keeping one of the constant cases so the reducer path stays covered too.
I'm the reporter of FLINK-40338 and already have these written and verified locally (they fail with `ClassCastException` on `java.lang.Byte`/`Short`/`Long` without the fix). Happy to hand them over for you to include here, or to open them as a follow-up if you'd rather keep this PR as is, whichever you prefer.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-40338][table-runtime] ELT() throws ClassCastException when index is not INT [flink]
Posted by "raminqaf (via GitHub)" <gi...@apache.org>.
raminqaf commented on code in PR #28931:
URL: https://github.com/apache/flink/pull/28931#discussion_r3728578803
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/StringFunctionsITCase.java:
##########
@@ -182,6 +182,22 @@ private Stream<TestSetSpec> eltTestCases() {
DataTypes.VARCHAR(5))
.testResult(
lit(2).elt("a", "b"), "ELT(2, 'a', 'b')", "b", DataTypes.CHAR(1))
+ // FLINK-40338: non-INT INTEGER_NUMERIC index must not throw ClassCastException
Review Comment:
I would remove this
```suggestion
```
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-40338][table-runtime] ELT() throws ClassCastException when index is not INT [flink]
Posted by "hulincup (via GitHub)" <gi...@apache.org>.
hulincup commented on PR #28931:
URL: https://github.com/apache/flink/pull/28931#issuecomment-5213771850
@flinkbot run azure
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-40338][table-runtime] ELT() throws ClassCastException when index is not INT [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #28931:
URL: https://github.com/apache/flink/pull/28931#discussion_r3751371776
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/StringFunctionsITCase.java:
##########
@@ -182,6 +186,26 @@ private Stream<TestSetSpec> eltTestCases() {
DataTypes.VARCHAR(5))
.testResult(
lit(2).elt("a", "b"), "ELT(2, 'a', 'b')", "b", DataTypes.CHAR(1))
+ .testResult(
+ lit(2).cast(DataTypes.TINYINT()).elt("scala", "java"),
+ "ELT(CAST(2 AS TINYINT), 'scala', 'java')",
+ "java",
+ DataTypes.VARCHAR(5))
+ .testResult(
+ $("f4").elt("scala", "java"),
+ "ELT(f4, 'scala', 'java')",
+ "java",
+ DataTypes.VARCHAR(5))
+ .testResult(
Review Comment:
can we also have tests where one of the values is `null`?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-40338][table-runtime] ELT() throws ClassCastException when index is not INT [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #28931:
URL: https://github.com/apache/flink/pull/28931#discussion_r3921351135
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/StringFunctionsITCase.java:
##########
@@ -182,6 +186,26 @@ private Stream<TestSetSpec> eltTestCases() {
DataTypes.VARCHAR(5))
.testResult(
lit(2).elt("a", "b"), "ELT(2, 'a', 'b')", "b", DataTypes.CHAR(1))
+ .testResult(
+ lit(2).cast(DataTypes.TINYINT()).elt("scala", "java"),
+ "ELT(CAST(2 AS TINYINT), 'scala', 'java')",
+ "java",
+ DataTypes.VARCHAR(5))
+ .testResult(
+ $("f4").elt("scala", "java"),
+ "ELT(f4, 'scala', 'java')",
+ "java",
+ DataTypes.VARCHAR(5))
+ .testResult(
Review Comment:
have you really tested it?
At some point I have impression you only translate AI slop ouput
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-40338][table-runtime] ELT() throws ClassCastException when index is not INT [flink]
Posted by "hulincup (via GitHub)" <gi...@apache.org>.
hulincup commented on code in PR #28931:
URL: https://github.com/apache/flink/pull/28931#discussion_r3749355773
##########
flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/scalar/EltFunction.java:
##########
@@ -40,6 +40,9 @@ public EltFunction(SpecializedContext context) {
if (idx < 1 || idx > exprs.length) {
return null;
}
- return exprs[(int) index - 1];
+ // Narrow the already-unboxed long instead of casting the Number reference.
+ // Casting `index` (java.lang.Number) to int compiles to a checkcast to Integer
+ // followed by unboxing, which throws ClassCastException for Byte/Short/Long.
Review Comment:
Removed in e8302d4. The field-reference tests (f4/f5/f6) catch a revert of this line, and the root cause is in a1d7732's commit message — the inline comment duplicated both.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-40338][table-runtime] ELT() throws ClassCastException when index is not INT [flink]
Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #28931:
URL: https://github.com/apache/flink/pull/28931#issuecomment-5199473677
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "a1d7732ae7efbf16fa72e1a05e156c2868a8fd1d",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "a1d7732ae7efbf16fa72e1a05e156c2868a8fd1d",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a1d7732ae7efbf16fa72e1a05e156c2868a8fd1d UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run azure` re-run the last Azure build
</details>
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-40338][table-runtime] ELT() throws ClassCastException when index is not INT [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #28931:
URL: https://github.com/apache/flink/pull/28931#discussion_r3921360141
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/StringFunctionsITCase.java:
##########
@@ -182,6 +186,26 @@ private Stream<TestSetSpec> eltTestCases() {
DataTypes.VARCHAR(5))
.testResult(
lit(2).elt("a", "b"), "ELT(2, 'a', 'b')", "b", DataTypes.CHAR(1))
+ .testResult(
+ lit(2).cast(DataTypes.TINYINT()).elt("scala", "java"),
+ "ELT(CAST(2 AS TINYINT), 'scala', 'java')",
+ "java",
+ DataTypes.VARCHAR(5))
+ .testResult(
+ $("f4").elt("scala", "java"),
+ "ELT(f4, 'scala', 'java')",
+ "java",
+ DataTypes.VARCHAR(5))
+ .testResult(
Review Comment:
1. run `sql-client.sh`
2. execute `select elt(1, 2, 3);`
3. it fails as
```
[ERROR] Could not execute SQL statement. Reason:
org.apache.flink.table.api.ValidationException: Could not infer arguments in range: [1, null].
```
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-40338][table-runtime] ELT() throws ClassCastException when index is not INT [flink]
Posted by "raminqaf (via GitHub)" <gi...@apache.org>.
raminqaf commented on code in PR #28931:
URL: https://github.com/apache/flink/pull/28931#discussion_r4323203061
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/StringFunctionsITCase.java:
##########
@@ -182,6 +186,26 @@ private Stream<TestSetSpec> eltTestCases() {
DataTypes.VARCHAR(5))
.testResult(
lit(2).elt("a", "b"), "ELT(2, 'a', 'b')", "b", DataTypes.CHAR(1))
+ .testResult(
+ lit(2).cast(DataTypes.TINYINT()).elt("scala", "java"),
+ "ELT(CAST(2 AS TINYINT), 'scala', 'java')",
+ "java",
+ DataTypes.VARCHAR(5))
+ .testResult(
+ $("f4").elt("scala", "java"),
+ "ELT(f4, 'scala', 'java')",
+ "java",
+ DataTypes.VARCHAR(5))
+ .testResult(
Review Comment:
@hulincup Could you please run this and see if the `ETL()` function works?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-40338][table-runtime] ELT() throws ClassCastException when index is not INT [flink]
Posted by "hulincup (via GitHub)" <gi...@apache.org>.
hulincup commented on PR #28931:
URL: https://github.com/apache/flink/pull/28931#issuecomment-5213085792
Done — added the trailer to the test commit in `8ee7dd53` (rewritten; new SHA after force-push). Tests are now attributed via `Co-authored-by: SEPURI-SAI-KRISHNA <sa...@gmail.com>`. Thanks for the thorough review and for flagging both the spotless line and the attribution.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-40338][table-runtime] ELT() throws ClassCastException when index is not INT [flink]
Posted by "SEPURI-SAI-KRISHNA (via GitHub)" <gi...@apache.org>.
SEPURI-SAI-KRISHNA commented on PR #28931:
URL: https://github.com/apache/flink/pull/28931#issuecomment-5211163876
Thanks @hulincup, appreciated — and no hard feelings, it's an easy thing to miss.
One build heads-up: the new onFieldsWithData line is 106 chars, over the 100-column AOSP limit that spotless enforces (google-java-format 1.24.0, pom.xml). mvn spotless:apply will wrap it to:
.onFieldsWithData(
null, null, null, new byte[] {1, 2, 3}, (byte) 2, (short) 2, 2L)
Worth running before the next push — CI is red on the earlier commit.
Minor: since the test code was taken as-is, a Co-authored-by: trailer on 8ee7dd53 would be the conventional attribution. Entirely up to you — the mention in the message is already generous.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-40338][table-runtime] ELT() throws ClassCastException when index is not INT [flink]
Posted by "hulincup (via GitHub)" <gi...@apache.org>.
hulincup commented on code in PR #28931:
URL: https://github.com/apache/flink/pull/28931#discussion_r3840151232
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/StringFunctionsITCase.java:
##########
@@ -182,6 +186,26 @@ private Stream<TestSetSpec> eltTestCases() {
DataTypes.VARCHAR(5))
.testResult(
lit(2).elt("a", "b"), "ELT(2, 'a', 'b')", "b", DataTypes.CHAR(1))
+ .testResult(
+ lit(2).cast(DataTypes.TINYINT()).elt("scala", "java"),
+ "ELT(CAST(2 AS TINYINT), 'scala', 'java')",
+ "java",
+ DataTypes.VARCHAR(5))
+ .testResult(
+ $("f4").elt("scala", "java"),
+ "ELT(f4, 'scala', 'java')",
+ "java",
+ DataTypes.VARCHAR(5))
+ .testResult(
Review Comment:
@snuyanzin Good question, thanks for raising it.
I did check `SELECT ELT(1, 2, 3)` — it fails at planning time, not runtime. ELT's `inputTypeStrategy` (in `BuiltInFunctionDefinitions.java`) constrains the `expr`/`exprs` arguments to `CHARACTER_STRING` or `BINARY_STRING` type families only. Integer exprs are rejected during validation with "Invalid input arguments. Expected signatures are: ELT(index <INTEGER_NUMERIC>, expr <CHARACTER_STRING>, exprs <CHARACTER_STRING>...) / ELT(index <INTEGER_NUMERIC>, expr <BINARY_STRING>, exprs <BINARY_STRING>...)", so they never reach `EltFunction.eval` and can't trigger the ClassCastException.
This non-string-expr case is already covered in the same `eltTestCases()` method — the existing "Validation Error" `TestSetSpec` exercises it (e.g. with `f2 = DECIMAL(1,0)` via `testTableApiValidationError` / `testSqlValidationError`).
The ClassCastException this PR fixes is specifically about the **index** argument, not the exprs. The index is declared as `Number` (`INTEGER_NUMERIC` family), and the bug was in `exprs[(int) index - 1]` — casting a `Number` reference (`Byte`/`Short`/`Long`) to `int` triggers a `checkcast` to `Integer`. The exprs are type-narrowed to string/binary before eval, so they're never involved in the buggy cast. That's why all test exprs are strings — they have to be, per the type strategy.
Regarding "strings only" — the `BINARY_STRING` path is actually exercised too. The existing tests include cases like `lit(2).elt($("f2"), $("f3"), $("f3"))` where `f2`/`f3` are `BYTES`, returning `byte[]`. So both `CHARACTER_STRING` and `BINARY_STRING` families are covered.
The new test cases (f4/f5/f6 with `TINYINT`/`SMALLINT`/`BIGINT` indices) target exactly the bug being fixed — a non-INT `INTEGER_NUMERIC` index reaching `eval`.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-40338][table-runtime] ELT() throws ClassCastException when index is not INT [flink]
Posted by "hulincup (via GitHub)" <gi...@apache.org>.
hulincup commented on code in PR #28931:
URL: https://github.com/apache/flink/pull/28931#discussion_r3817988490
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/StringFunctionsITCase.java:
##########
@@ -182,6 +186,26 @@ private Stream<TestSetSpec> eltTestCases() {
DataTypes.VARCHAR(5))
.testResult(
lit(2).elt("a", "b"), "ELT(2, 'a', 'b')", "b", DataTypes.CHAR(1))
+ .testResult(
+ lit(2).cast(DataTypes.TINYINT()).elt("scala", "java"),
+ "ELT(CAST(2 AS TINYINT), 'scala', 'java')",
+ "java",
+ DataTypes.VARCHAR(5))
+ .testResult(
+ $("f4").elt("scala", "java"),
+ "ELT(f4, 'scala', 'java')",
+ "java",
+ DataTypes.VARCHAR(5))
+ .testResult(
Review Comment:
Good question — honest answer: not interactively in the sqlclient. My local environment only has JDK 8, so I can't build/run Flink's sqlclient here; I've been relying on the ITCase + CI.
That said, these null cases do exercise the runtime rather than getting folded at plan time: the index/expr are field references (`f0`, `f1`, `f2`, `f4`) loaded with null data via `onFieldsWithData`, so `ExpressionReducer` can't constant-fold them and the `ELT` call reaches the codegen'd operator — the same path sqlclient would hit. `EltFunction.eval` returns null explicitly for `index == null` and out-of-range, and returns the (possibly null) selected expr otherwise, so all three null shapes (null index, null expr at selected position, null expr at unselected position) are covered. Since `StringFunctionsITCase` runs the query through the full runtime, it's effectively the same verification as running it in sqlclient.
If there's a specific sqlclient shape you'd like confirmed (e.g. literal `NULL` index or `CAST(NULL AS …)` exprs), I can spin up a JDK 17 + Flink binary and capture the actual output — happy to do that.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-40338][table-runtime] ELT() throws ClassCastException when index is not INT [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #28931:
URL: https://github.com/apache/flink/pull/28931#discussion_r3836582078
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/StringFunctionsITCase.java:
##########
@@ -182,6 +186,26 @@ private Stream<TestSetSpec> eltTestCases() {
DataTypes.VARCHAR(5))
.testResult(
lit(2).elt("a", "b"), "ELT(2, 'a', 'b')", "b", DataTypes.CHAR(1))
+ .testResult(
+ lit(2).cast(DataTypes.TINYINT()).elt("scala", "java"),
+ "ELT(CAST(2 AS TINYINT), 'scala', 'java')",
+ "java",
+ DataTypes.VARCHAR(5))
+ .testResult(
+ $("f4").elt("scala", "java"),
+ "ELT(f4, 'scala', 'java')",
+ "java",
+ DataTypes.VARCHAR(5))
+ .testResult(
Review Comment:
why are you sticked to strings only?
have you tried
```sql
SELECT ELT(1, 2, 3);
```
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-40338][table-runtime] ELT() throws ClassCastException when index is not INT [flink]
Posted by "hulincup (via GitHub)" <gi...@apache.org>.
hulincup commented on PR #28931:
URL: https://github.com/apache/flink/pull/28931#issuecomment-5210617394
Hi @SEPURI-SAI-KRISHNA, you're right, and I apologize for missing your claim on the JIRA ticket before opening this PR. That was my mistake — I should have scanned the comments for assignment signals first.
Since @raminqaf has started reviewing here and you've generously offered to hand over the test cases, I've incorporated your field-reference tests directly (credited in the new commit). The codegen path is now covered via `f4`/`f5`/`f6` (TINYINT/SMALLINT/BIGINT), with one constant case kept for the reducer path. Thanks for the verified patch and the detailed review.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-40338][table-runtime] ELT() throws ClassCastException when index is not INT [flink]
Posted by "raminqaf (via GitHub)" <gi...@apache.org>.
raminqaf commented on code in PR #28931:
URL: https://github.com/apache/flink/pull/28931#discussion_r3729398464
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/StringFunctionsITCase.java:
##########
@@ -182,6 +182,22 @@ private Stream<TestSetSpec> eltTestCases() {
DataTypes.VARCHAR(5))
.testResult(
lit(2).elt("a", "b"), "ELT(2, 'a', 'b')", "b", DataTypes.CHAR(1))
+ // FLINK-40338: non-INT INTEGER_NUMERIC index must not throw ClassCastException
+ .testResult(
+ lit(2).cast(DataTypes.TINYINT()).elt("scala", "java"),
+ "ELT(CAST(2 AS TINYINT), 'scala', 'java')",
+ "java",
+ DataTypes.VARCHAR(5))
+ .testResult(
+ lit(2).cast(DataTypes.SMALLINT()).elt("scala", "java"),
+ "ELT(CAST(2 AS SMALLINT), 'scala', 'java')",
+ "java",
+ DataTypes.VARCHAR(5))
+ .testResult(
+ lit(2).cast(DataTypes.BIGINT()).elt("scala", "java"),
+ "ELT(CAST(2 AS BIGINT), 'scala', 'java')",
+ "java",
+ DataTypes.VARCHAR(5))
Review Comment:
Just in case write a test with non-literals too please
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-40338][table-runtime] ELT() throws ClassCastException when index is not INT [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #28931:
URL: https://github.com/apache/flink/pull/28931#discussion_r3734055617
##########
flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/scalar/EltFunction.java:
##########
@@ -40,6 +40,9 @@ public EltFunction(SpecializedContext context) {
if (idx < 1 || idx > exprs.length) {
return null;
}
- return exprs[(int) index - 1];
+ // Narrow the already-unboxed long instead of casting the Number reference.
+ // Casting `index` (java.lang.Number) to int compiles to a checkcast to Integer
+ // followed by unboxing, which throws ClassCastException for Byte/Short/Long.
Review Comment:
why do we need it if we have tests?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-40338][table-runtime] ELT() throws ClassCastException when index is not INT [flink]
Posted by "SEPURI-SAI-KRISHNA (via GitHub)" <gi...@apache.org>.
SEPURI-SAI-KRISHNA commented on code in PR #28931:
URL: https://github.com/apache/flink/pull/28931#discussion_r3729690331
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/StringFunctionsITCase.java:
##########
@@ -182,6 +182,22 @@ private Stream<TestSetSpec> eltTestCases() {
DataTypes.VARCHAR(5))
.testResult(
lit(2).elt("a", "b"), "ELT(2, 'a', 'b')", "b", DataTypes.CHAR(1))
+ // FLINK-40338: non-INT INTEGER_NUMERIC index must not throw ClassCastException
+ .testResult(
+ lit(2).cast(DataTypes.TINYINT()).elt("scala", "java"),
+ "ELT(CAST(2 AS TINYINT), 'scala', 'java')",
+ "java",
+ DataTypes.VARCHAR(5))
+ .testResult(
+ lit(2).cast(DataTypes.SMALLINT()).elt("scala", "java"),
+ "ELT(CAST(2 AS SMALLINT), 'scala', 'java')",
+ "java",
+ DataTypes.VARCHAR(5))
+ .testResult(
+ lit(2).cast(DataTypes.BIGINT()).elt("scala", "java"),
+ "ELT(CAST(2 AS BIGINT), 'scala', 'java')",
+ "java",
+ DataTypes.VARCHAR(5))
Review Comment:
@raminqaf agreed. Here's the patch I have locally, already verified, it fails with `ClassCastException` on `java.lang.Byte`, `Short` and `Long` before the fix and passes after:
```java
.onFieldsWithData(null, null, null, new byte[] {1, 2, 3}, (byte) 2, (short) 2, 2L)
.andDataTypes(
DataTypes.INT(), DataTypes.STRING(), DataTypes.BYTES(), DataTypes.BYTES(),
DataTypes.TINYINT(), DataTypes.SMALLINT(), DataTypes.BIGINT())
```
```java
.testResult(
$("f4").elt("scala", "java"),
"ELT(f4, 'scala', 'java')",
"java",
DataTypes.VARCHAR(5))
.testResult(
$("f5").elt("scala", "java"),
"ELT(f5, 'scala', 'java')",
"java",
DataTypes.VARCHAR(5))
.testResult(
$("f6").elt("scala", "java"),
"ELT(f6, 'scala', 'java')",
"java",
DataTypes.VARCHAR(5))
```
@hulincup feel free to take these directly.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-40338][table-runtime] ELT() throws ClassCastException when index is not INT [flink]
Posted by "hulincup (via GitHub)" <gi...@apache.org>.
hulincup commented on code in PR #28931:
URL: https://github.com/apache/flink/pull/28931#discussion_r3732739260
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/StringFunctionsITCase.java:
##########
@@ -182,6 +182,22 @@ private Stream<TestSetSpec> eltTestCases() {
DataTypes.VARCHAR(5))
.testResult(
lit(2).elt("a", "b"), "ELT(2, 'a', 'b')", "b", DataTypes.CHAR(1))
+ // FLINK-40338: non-INT INTEGER_NUMERIC index must not throw ClassCastException
Review Comment:
Done — removed in `778b833f`.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-40338][table-runtime] ELT() throws ClassCastException when index is not INT [flink]
Posted by "SEPURI-SAI-KRISHNA (via GitHub)" <gi...@apache.org>.
SEPURI-SAI-KRISHNA commented on PR #28931:
URL: https://github.com/apache/flink/pull/28931#issuecomment-5211460650
Yes please, if it's not too much trouble — the tests went in as-is, so the trailer is the accurate form. Exact line:
Co-authored-by: SEPURI-SAI-KRISHNA <sa...@gmail.com>
That address is linked to my GitHub account, so it'll attribute properly. 8ee7dd53 is the natural place, or whichever commit ends up carrying the tests if this gets squashed at
merge — whatever's least disruptive.
Thanks for wrapping the line, and for how you've handled all of this.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-40338][table-runtime] ELT() throws ClassCastException when index is not INT [flink]
Posted by "hulincup (via GitHub)" <gi...@apache.org>.
hulincup commented on PR #28931:
URL: https://github.com/apache/flink/pull/28931#issuecomment-5211371565
Thanks @SEPURI-SAI-KRISHNA — wrapped it in `eea2481`, exactly as `mvn spotless:apply` would. Good catch on the 100-col limit; should go green on the next run.
Re: attribution — the test-plan credit already lives in the `8ee7dd53` message ("suggested by @SEPURI-SAI-KRISHNA and @raminqaf during review"), which feels right to me, but happy to add the `Co-authored-by` trailer too if you'd prefer. Either way, appreciate the heads-up.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-40338][table-runtime] ELT() throws ClassCastException when index is not INT [flink]
Posted by "hulincup (via GitHub)" <gi...@apache.org>.
hulincup commented on code in PR #28931:
URL: https://github.com/apache/flink/pull/28931#discussion_r3732716480
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/StringFunctionsITCase.java:
##########
@@ -182,6 +182,22 @@ private Stream<TestSetSpec> eltTestCases() {
DataTypes.VARCHAR(5))
.testResult(
lit(2).elt("a", "b"), "ELT(2, 'a', 'b')", "b", DataTypes.CHAR(1))
+ // FLINK-40338: non-INT INTEGER_NUMERIC index must not throw ClassCastException
+ .testResult(
+ lit(2).cast(DataTypes.TINYINT()).elt("scala", "java"),
+ "ELT(CAST(2 AS TINYINT), 'scala', 'java')",
+ "java",
+ DataTypes.VARCHAR(5))
+ .testResult(
+ lit(2).cast(DataTypes.SMALLINT()).elt("scala", "java"),
+ "ELT(CAST(2 AS SMALLINT), 'scala', 'java')",
+ "java",
+ DataTypes.VARCHAR(5))
+ .testResult(
+ lit(2).cast(DataTypes.BIGINT()).elt("scala", "java"),
+ "ELT(CAST(2 AS BIGINT), 'scala', 'java')",
+ "java",
+ DataTypes.VARCHAR(5))
Review Comment:
Done — removed the redundant constant cases and added field-reference tests (`f4`/`f5`/`f6`) per your suggestion, so the codegen'd operator path is now covered alongside the reducer path. Thanks @SEPURI-SAI-KRISHNA for the verified patch and @raminqaf for catching the gap.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-40338][table-runtime] ELT() throws ClassCastException when index is not INT [flink]
Posted by "SEPURI-SAI-KRISHNA (via GitHub)" <gi...@apache.org>.
SEPURI-SAI-KRISHNA commented on PR #28931:
URL: https://github.com/apache/flink/pull/28931#issuecomment-5200125938
Hi @hulincup — I'm the reporter of FLINK-40338 and had asked to be assigned before this PR was opened, with a patch and regression tests ready. Per the contribute-code guide, PRs on unassigned tickets aren't reviewed or merged, so could we wait for a committer to decide who takes it?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-40338][table-runtime] ELT() throws ClassCastException when index is not INT [flink]
Posted by "hulincup (via GitHub)" <gi...@apache.org>.
hulincup commented on code in PR #28931:
URL: https://github.com/apache/flink/pull/28931#discussion_r3755725017
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/StringFunctionsITCase.java:
##########
@@ -182,6 +186,26 @@ private Stream<TestSetSpec> eltTestCases() {
DataTypes.VARCHAR(5))
.testResult(
lit(2).elt("a", "b"), "ELT(2, 'a', 'b')", "b", DataTypes.CHAR(1))
+ .testResult(
+ lit(2).cast(DataTypes.TINYINT()).elt("scala", "java"),
+ "ELT(CAST(2 AS TINYINT), 'scala', 'java')",
+ "java",
+ DataTypes.VARCHAR(5))
+ .testResult(
+ $("f4").elt("scala", "java"),
+ "ELT(f4, 'scala', 'java')",
+ "java",
+ DataTypes.VARCHAR(5))
+ .testResult(
Review Comment:
Added in a4e350f — `$("f4").elt($("f3"), $("f2"))`: f4 is a non-literal TINYINT index (the codegen path), f2 holds null BYTES data at the selected position, so the result is null.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-40338][table-runtime] ELT() throws ClassCastException when index is not INT [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #28931:
URL: https://github.com/apache/flink/pull/28931#discussion_r3816796157
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/StringFunctionsITCase.java:
##########
@@ -182,6 +186,26 @@ private Stream<TestSetSpec> eltTestCases() {
DataTypes.VARCHAR(5))
.testResult(
lit(2).elt("a", "b"), "ELT(2, 'a', 'b')", "b", DataTypes.CHAR(1))
+ .testResult(
+ lit(2).cast(DataTypes.TINYINT()).elt("scala", "java"),
+ "ELT(CAST(2 AS TINYINT), 'scala', 'java')",
+ "java",
+ DataTypes.VARCHAR(5))
+ .testResult(
+ $("f4").elt("scala", "java"),
+ "ELT(f4, 'scala', 'java')",
+ "java",
+ DataTypes.VARCHAR(5))
+ .testResult(
Review Comment:
have you tried such queries in sqlclient?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-40338][table-runtime] ELT() throws ClassCastException when index is not INT [flink]
Posted by "hulincup (via GitHub)" <gi...@apache.org>.
hulincup commented on code in PR #28931:
URL: https://github.com/apache/flink/pull/28931#discussion_r3817988490
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/StringFunctionsITCase.java:
##########
@@ -182,6 +186,26 @@ private Stream<TestSetSpec> eltTestCases() {
DataTypes.VARCHAR(5))
.testResult(
lit(2).elt("a", "b"), "ELT(2, 'a', 'b')", "b", DataTypes.CHAR(1))
+ .testResult(
+ lit(2).cast(DataTypes.TINYINT()).elt("scala", "java"),
+ "ELT(CAST(2 AS TINYINT), 'scala', 'java')",
+ "java",
+ DataTypes.VARCHAR(5))
+ .testResult(
+ $("f4").elt("scala", "java"),
+ "ELT(f4, 'scala', 'java')",
+ "java",
+ DataTypes.VARCHAR(5))
+ .testResult(
Review Comment:
@/tmp/flink_28931_reply.md
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org