twalthr commented on code in PR #27158:
URL: https://github.com/apache/flink/pull/27158#discussion_r2486412391
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/CastFunctionMiscITCase.java:
##########
@@ -103,7 +105,8 @@ Stream<TestSetSpec> getTestSetSpecs() {
// the inner NOT NULL is ignored in SQL
because the outer ROW is
Review Comment:
update the comment?
##########
docs/layouts/shortcodes/generated/table_config_configuration.html:
##########
@@ -56,6 +56,12 @@
<td>Integer</td>
<td>Specifies a threshold where generated code will be split into
sub-function calls. Java has a maximum method length of 64 KB. This setting
allows for finer granularity if necessary. Default value is 4000 instead of
64KB as by default JIT refuses to work on methods with more than 8K byte
code.</td>
</tr>
+ <tr>
+
<td><h5>table.legacy-extended-row-struct-kind-as-fully-qualified</h5><br> <span
class="label label-primary">Batch</span> <span class="label
label-primary">Streaming</span></td>
Review Comment:
Maybe a more user centric name?
```suggestion
<td><h5>table.legacy-nested-row-nullability</h5><br> <span
class="label label-primary">Batch</span> <span class="label
label-primary">Streaming</span></td>
```
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/calcite/FlinkCalciteSqlValidator.java:
##########
@@ -122,7 +123,16 @@ public FlinkCalciteSqlValidator(
RelOptTable.ToRelContext toRelcontext,
RelOptCluster relOptCluster,
FrameworkConfig frameworkConfig) {
- super(opTab, catalogReader, typeFactory, config);
+ super(
+ opTab,
+ catalogReader,
+ typeFactory,
+ config,
+ ShortcutUtils.unwrapTableConfig(relOptCluster)
Review Comment:
use a boolean flag instead, the
```
StructKind.FULLY_QUALIFIED
: StructKind.PEEK_FIELDS_NO_EXPAND
```
should be local to where it is used
##########
flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/FlinkSqlParsingValidator.java:
##########
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.sql.parser;
+
+import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.rel.type.StructKind;
+import org.apache.calcite.sql.SqlOperatorTable;
+import org.apache.calcite.sql.validate.SqlValidatorCatalogReader;
+import org.apache.calcite.sql.validate.SqlValidatorImpl;
+
+/**
+ * Extends Calcite's {@link org.apache.calcite.sql.validate.SqlValidator}.
Right now it extends by
+ * row struct kind depending on a table config option value {@code
+ * table.legacy-extended-row-struct-kind-as-fully-qualified}. However, in
future more config option
+ * dependent fields might be added here.
Review Comment:
```suggestion
* Extends Calcite's {@link org.apache.calcite.sql.validate.SqlValidator}.
It allows for parameterizing the parsing based on feature flags backed by
options.
```
##########
flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/FlinkSqlParsingValidator.java:
##########
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.sql.parser;
+
+import org.apache.calcite.rel.type.RelDataTypeFactory;
+import org.apache.calcite.rel.type.StructKind;
+import org.apache.calcite.sql.SqlOperatorTable;
+import org.apache.calcite.sql.validate.SqlValidatorCatalogReader;
+import org.apache.calcite.sql.validate.SqlValidatorImpl;
+
+/**
+ * Extends Calcite's {@link org.apache.calcite.sql.validate.SqlValidator}.
Right now it extends by
+ * row struct kind depending on a table config option value {@code
+ * table.legacy-extended-row-struct-kind-as-fully-qualified}. However, in
future more config option
+ * dependent fields might be added here.
+ */
+public class FlinkSqlParsingValidator extends SqlValidatorImpl {
+ private final StructKind extendedRowStructKind;
+
+ protected FlinkSqlParsingValidator(
+ SqlOperatorTable opTab,
+ SqlValidatorCatalogReader catalogReader,
+ RelDataTypeFactory typeFactory,
+ Config config,
+ StructKind extendedRowStructKind) {
+ super(opTab, catalogReader, typeFactory, config);
+ this.extendedRowStructKind = extendedRowStructKind;
+ }
+
+ public final StructKind getExtendedRowStructKind() {
Review Comment:
How about we make this a boolean flag `isLegacyNestedRowNullability`
instead? And have the main logic in `ExtendedSqlRowTypeNameSpec`
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/calcite/FlinkRexBuilder.java:
##########
@@ -102,4 +91,65 @@ public RexLiteral makeZeroLiteral(RelDataType type) {
return super.makeZeroLiteral(type);
}
}
+
+ /**
+ * Adjust the nullability of the nested column based on the nullability of
the enclosing type.
+ * However, if there is former nullability CAST present then it will be
dropped and replaced
+ * with a new one (if needed).
Review Comment:
can you add some examples?
--
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]
