Skip to content

Commit 2426faf

Browse files
davidradlsnuyanzin
davidradl
and
snuyanzin
authored
[FLINK-38913][table] ArrayIndexOutOfBoundsException while unparsing ExtendedSqlRowTypeNameSpec
--------- Signed-off-by: davidradl <david_radley@uk.ibm.com> Co-authored-by: Sergey Nuyanzin <snuyanzin@gmail.com>
1 parent 294a7c3 commit 2426faf

File tree

3 files changed

+171
-1
lines changed

3 files changed

+171
-1
lines changed

‎flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/type/ExtendedSqlRowTypeNameSpec.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,8 @@ public void unparse(SqlWriter writer, int leftPrec, int rightPrec) {
121121
if (p.right.getNullable() != null && !p.right.getNullable()) {
122122
writer.keyword("NOT NULL");
123123
}
124-
if (comments.get(i) != null) {
124+
// With bounds check - prevents IndexOutOfBoundsException
125+
if (i < comments.size() && comments.get(i) != null) {
125126
comments.get(i).unparse(writer, leftPrec, rightPrec);
126127
}
127128
i += 1;

‎flink-table/flink-sql-parser/src/test/java/org/apache/flink/sql/parser/ExtendedSqlRowTypeNameSpecTest.java‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to you under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
package org.apache.flink.sql.parser;
19+
20+
import org.apache.flink.sql.parser.impl.FlinkSqlParserImpl;
21+
import org.apache.flink.sql.parser.type.ExtendedSqlRowTypeNameSpec;
22+
23+
import org.apache.calcite.avatica.util.Casing;
24+
import org.apache.calcite.avatica.util.Quoting;
25+
import org.apache.calcite.sql.SqlBasicTypeNameSpec;
26+
import org.apache.calcite.sql.SqlDataTypeSpec;
27+
import org.apache.calcite.sql.SqlDialect;
28+
import org.apache.calcite.sql.SqlIdentifier;
29+
import org.apache.calcite.sql.SqlWriter;
30+
import org.apache.calcite.sql.dialect.CalciteSqlDialect;
31+
import org.apache.calcite.sql.fun.SqlStdOperatorTable;
32+
import org.apache.calcite.sql.parser.SqlParser;
33+
import org.apache.calcite.sql.parser.SqlParserImplFactory;
34+
import org.apache.calcite.sql.parser.SqlParserPos;
35+
import org.apache.calcite.sql.pretty.SqlPrettyWriter;
36+
import org.apache.calcite.sql.type.SqlTypeName;
37+
import org.apache.calcite.sql.validate.SqlConformance;
38+
import org.apache.calcite.sql.validate.SqlConformanceEnum;
39+
import org.junit.jupiter.api.Test;
40+
41+
import java.util.List;
42+
import java.util.Map;
43+
44+
/** Tests for {@link ExtendedSqlRowTypeNameSpec}. */
45+
class ExtendedSqlRowTypeNameSpecTest {
46+
@Test
47+
void testExtendedRowWithNoComments() {
48+
final ExtendedSqlRowTypeNameSpec spec =
49+
new ExtendedSqlRowTypeNameSpec(
50+
SqlParserPos.ZERO,
51+
List.of(
52+
new SqlIdentifier("t1", SqlParserPos.ZERO),
53+
new SqlIdentifier("t2", SqlParserPos.ZERO),
54+
new SqlIdentifier("t3", SqlParserPos.ZERO)),
55+
List.of(
56+
new SqlDataTypeSpec(
57+
new SqlBasicTypeNameSpec(
58+
SqlTypeName.INTEGER, SqlParserPos.ZERO),
59+
SqlParserPos.ZERO),
60+
new SqlDataTypeSpec(
61+
new SqlBasicTypeNameSpec(
62+
SqlTypeName.DATE, SqlParserPos.ZERO),
63+
SqlParserPos.ZERO),
64+
new SqlDataTypeSpec(
65+
new SqlBasicTypeNameSpec(
66+
SqlTypeName.TIME, SqlParserPos.ZERO),
67+
SqlParserPos.ZERO)),
68+
List.of(),
69+
false);
70+
SqlWriter writer = getSqlWriter();
71+
spec.unparse(writer, 0, 0);
72+
}
73+
74+
private SqlWriter getSqlWriter() {
75+
final Map<String, ?> options =
76+
Map.ofEntries(
77+
Map.entry("quoting", Quoting.BACK_TICK),
78+
Map.entry("quotedCasing", Casing.UNCHANGED),
79+
Map.entry("unquotedCasing", Casing.UNCHANGED),
80+
Map.entry("caseSensitive", true),
81+
Map.entry("enableTypeCoercion", false),
82+
Map.entry("conformance", SqlConformanceEnum.DEFAULT),
83+
Map.entry("operatorTable", SqlStdOperatorTable.instance()),
84+
Map.entry("parserFactory", FlinkSqlParserImpl.FACTORY));
85+
final SqlParser.Config parserConfig =
86+
SqlParser.config()
87+
.withQuoting((Quoting) options.get("quoting"))
88+
.withUnquotedCasing((Casing) options.get("unquotedCasing"))
89+
.withQuotedCasing((Casing) options.get("quotedCasing"))
90+
.withConformance((SqlConformance) options.get("conformance"))
91+
.withCaseSensitive((boolean) options.get("caseSensitive"))
92+
.withParserFactory((SqlParserImplFactory) options.get("parserFactory"));
93+
94+
return new SqlPrettyWriter(
95+
new CalciteSqlDialect(
96+
SqlDialect.EMPTY_CONTEXT
97+
.withQuotedCasing(parserConfig.unquotedCasing())
98+
.withConformance(parserConfig.conformance())
99+
.withUnquotedCasing(parserConfig.unquotedCasing())
100+
.withIdentifierQuoteString(parserConfig.quoting().string)),
101+
false);
102+
}
103+
}

‎flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/calcite/SqlTypeUtilTest.java‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
package org.apache.flink.table.planner.calcite;
20+
21+
import org.apache.flink.table.planner.typeutils.LogicalRelDataTypeConverter;
22+
import org.apache.flink.table.types.logical.IntType;
23+
import org.apache.flink.table.types.logical.LogicalType;
24+
import org.apache.flink.table.types.logical.RowType;
25+
import org.apache.flink.table.types.logical.VarCharType;
26+
27+
import org.apache.calcite.rel.type.RelDataType;
28+
import org.apache.calcite.sql.SqlDataTypeSpec;
29+
import org.apache.calcite.sql.SqlWriter;
30+
import org.apache.calcite.sql.pretty.SqlPrettyWriter;
31+
import org.apache.calcite.sql.type.SqlTypeUtil;
32+
import org.junit.jupiter.api.Test;
33+
34+
import static org.assertj.core.api.Assertions.assertThat;
35+
36+
/** Tests for {@link SqlTypeUtil}. */
37+
class SqlTypeUtilTest {
38+
/**
39+
* Test case for <a href="https://issues.apache.org/jira/browse/FLINK-38913">[FLINK-38913]
40+
* ArrayIndexOutOfBoundsException when creating a table with computed rows including casts to
41+
* null</a>.
42+
*/
43+
@Test
44+
void testConvertRowTypeToSpecAndUnparse() {
45+
FlinkTypeFactory typeFactory =
46+
new FlinkTypeFactory(
47+
Thread.currentThread().getContextClassLoader(), FlinkTypeSystem.INSTANCE);
48+
RowType rowType =
49+
RowType.of(
50+
new LogicalType[] {new IntType(), new VarCharType(1)},
51+
new String[] {"a", "b"});
52+
RelDataType relDataType = LogicalRelDataTypeConverter.toRelDataType(rowType, typeFactory);
53+
SqlDataTypeSpec typeSpec = SqlTypeUtil.convertTypeToSpec(relDataType);
54+
SqlWriter writer =
55+
new SqlPrettyWriter(
56+
SqlPrettyWriter.config()
57+
.withAlwaysUseParentheses(false)
58+
.withSelectListItemsOnSeparateLines(false)
59+
.withIndentation(0));
60+
// unparse that will end up passing no comments through
61+
typeSpec.unparse(writer, 0, 0);
62+
String result = writer.toSqlString().getSql();
63+
assertThat(result)
64+
.hasToString("ROW(\"a\" INTEGER, \"b\" VARCHAR(1) CHARACTER SET \"UTF-16LE\")");
65+
}
66+
}

0 commit comments

Comments
 (0)