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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org