nateab commented on code in PR #26719:
URL: https://github.com/apache/flink/pull/26719#discussion_r2178469627
##########
flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/codegen/calls/ScalarOperatorGens.scala:
##########
@@ -1423,7 +1423,7 @@ object ScalarOperatorGens {
| $resultTerm = $nullTerm ? $defaultValue : $arrayGet;
| break;
| default:
- | throw new RuntimeException("Array has more than one element.");
+ | throw new TableRuntimeException("Array has more than one
element.");
Review Comment:
We should use TableRuntimeException then I believe, since we don't know the
actual size of the array until runtime.
I tried adding a test, but it's not straightforward to me how to test this
runtime exception without breaking the testing framework
```
+++
b/flink-table/flink-table-planner/src/test/scala/org/apache/flink/table/planner/expressions/ArrayTypeTest.scala
@@ -21,7 +21,6 @@ import
org.apache.flink.core.testutils.FlinkAssertions.anyCauseMatches
import org.apache.flink.table.api._
import org.apache.flink.table.planner.expressions.utils.ArrayTypeTestBase
import org.apache.flink.table.planner.utils.DateTimeTestUtil.{localDate,
localDateTime, localTime => gLocalTime}
-
import org.assertj.core.api.Assertions.assertThatThrownBy
import org.junit.jupiter.api.Test
@@ -242,6 +241,13 @@ class ArrayTypeTest extends ArrayTypeTestBase {
"Array element access needs an index starting at 1 but was 0.")
}
+ @Test
+ def testElement(): Unit = {
+ testExpectedSqlException(
+ "ELEMENT(f2)",
+ "Array has more than one element.")
+ }
+
@Test
def testReturnNullWhenArrayIndexOutOfBounds(): Unit = {
// ARRAY<INT NOT NULL>
@@ -250,4 +256,11 @@ class ArrayTypeTest extends ArrayTypeTestBase {
// ARRAY<INT>
testAllApis('f11.at(3), "f11[4]", "NULL")
}
+
+ @Test
+ def testElementFailsOnMultiElementArray(): Unit = {
+ testExpectedSqlException(
+ "ELEMENT(ARRAY[1, 2])",
+ "Array has more than one element.")
+ }
}
```
These tests would fail with something like:
```
java.lang.AssertionError:
Expecting code to raise a throwable.
at
org.apache.flink.table.planner.expressions.ArrayTypeTest.testElementFailsOnMultiElementArrayInTableApi(ArrayTypeTest.scala:271)
at java.base/java.lang.reflect.Method.invoke(Method.java:568)
at
java.base/java.util.concurrent.ForkJoinTask.doExec$$$capture(ForkJoinTask.java:373)
at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java)
at
java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1182)
at
java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1655)
at
java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1622)
at
java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:165)
Suppressed: java.lang.AssertionError: Error when executing the
expression. Expression code:
// Using option 'table.exec.legacy-cast-behaviour':'false'
// Timezone: org.apache.flink.table.api.TableConfig@11f6af40
public class TestFunction$5
extends org.apache.flink.api.common.functions.RichMapFunction {
org.apache.flink.table.data.binary.BinaryArrayData array$0 = new
org.apache.flink.table.data.binary.BinaryArrayData();
org.apache.flink.table.data.writer.BinaryArrayWriter writer$1 = new
org.apache.flink.table.data.writer.BinaryArrayWriter(array$0, 2, 4);
org.apache.flink.table.data.binary.BinaryRowData out = new
org.apache.flink.table.data.binary.BinaryRowData(1);
org.apache.flink.table.data.writer.BinaryRowWriter outWriter = new
org.apache.flink.table.data.writer.BinaryRowWriter(out);
public TestFunction$5(Object[] references) throws Exception {
writer$1.reset();
if (false) {
writer$1.setNullInt(0);
} else {
writer$1.writeInt(0, ((int) 1));
}
if (false) {
writer$1.setNullInt(1);
} else {
writer$1.writeInt(1, ((int) 2));
}
writer$1.complete();
}
@Override
public void open(org.apache.flink.api.common.functions.OpenContext
openContext) throws Exception {
}
@Override
public Object map(Object _in1) throws Exception {
org.apache.flink.table.data.RowData in1 =
(org.apache.flink.table.data.RowData) _in1;
boolean isNull$3;
org.apache.flink.table.data.binary.BinaryStringData result$4;
outWriter.reset();
boolean isNull$2;
int result$2;
switch (false ? 0 : array$0.size()) {
case 0:
isNull$2 = true;
result$2 = -1;
break;
case 1:
isNull$2 = array$0.isNullAt(0);
result$2 = isNull$2 ? -1 : array$0.getInt(0);
break;
default:
throw new
org.apache.flink.table.api.ValidationException("Array has more than one
element.");
}
// --- Cast section generated by
org.apache.flink.table.planner.functions.casting.CharVarCharTrimPadCastRule
isNull$3 = isNull$2;
if (!isNull$3) {
result$4 =
org.apache.flink.table.data.binary.BinaryStringData.fromString("" + result$2);
isNull$3 = result$4 == null;
} else {
result$4 =
org.apache.flink.table.data.binary.BinaryStringData.EMPTY_UTF8;
}
// --- End cast section
if (isNull$3) {
outWriter.setNullAt(0);
} else {
outWriter.writeString(0, result$4);
}
outWriter.complete();
return out;
}
@Override
public void close() throws Exception {
}
}
at
org.apache.flink.table.planner.expressions.utils.ExpressionTestBase.evaluateFunctionResult(ExpressionTestBase.scala:285)
at
org.apache.flink.table.planner.expressions.utils.ExpressionTestBase.evaluateGivenExprs(ExpressionTestBase.scala:346)
at
org.apache.flink.table.planner.expressions.utils.ExpressionTestBase.evaluateExprs(ExpressionTestBase.scala:141)
... 7 more
Caused by: org.apache.flink.table.api.ValidationException: Array has
more than one element.
at TestFunction$5.map(Unknown Source)
at
org.apache.flink.table.planner.expressions.utils.ExpressionTestBase.evaluateFunctionResult(ExpressionTestBase.scala:260)
... 9 more
```
Since existing tests pass with this change, could we skip adding a test
since it is a small change?
--
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]
