twalthr commented on code in PR #27158:
URL: https://github.com/apache/flink/pull/27158#discussion_r2503109640
##########
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-nested-row-nullability</h5><br> <span
class="label label-primary">Batch</span> <span class="label
label-primary">Streaming</span></td>
+ <td style="word-wrap: break-word;">false</td>
+ <td>Boolean</td>
+ <td>Configures the default struct kind for rows processed by
ExtendedSqlRowTypeNameSpec. By default it uses PEEK_FIELDS_NO_EXPAND, legacy
behavior uses FULLY_QUALIFIED.</td>
Review Comment:
Also update this to a more user friendly, less internal description:
```
Before Flink 2.2, row types defined in SQL e.g. `SELECT CAST(f AS ROW<i NOT
NULL>)` did ignore the `NOT NULL` constraint. This was more aligned with the
SQL standard but caused many type inconsistencies and cryptic error message
when working on nested data. For example, it prevented using rows in computed
columns or join keys. The new behavior takes the nullability into consideration.
```
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/calcite/FlinkRexBuilder.java:
##########
@@ -102,4 +91,89 @@ 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 {@code CAST} present then it
will be dropped and
+ * replaced with a new one (if needed). For instance if there is a table
+ *
+ * <pre>{@code
+ * CREATE TABLE MyTable (
+ * `field1` ROW<`data` ROW<`nested` ROW<`trId` STRING>>NOT NULL>
+ * WITH ('connector' = 'datagen')
+ * }</pre>
+ *
+ * <p>and then there is a SQL query
+ *
+ * <pre>{@code
+ * SELECT `field1`.`data`.`nested`.`trId` AS transactionId FROM MyTable
+ * }</pre>
+ *
+ * <p>The {@code SELECT} picks a nested field only. In this case it should
go step by step
+ * checking each level.
+ *
+ * <ol>
+ * <li>Looking at {@code `field1`} type it is nullable, then no changes.
+ * <li>{@code `field1`.`data`} is {@code NOT NULL}, however keeping in
mind that enclosing
+ * type @{code `field1`} is nullable then need to change nullability
with {@code CAST}
+ * <li>{@code `field1`.`data`.`nested`} is nullable that means that in
this case no need for
+ * extra {@code CAST} inserted in previous step, so it will be
dropped.
+ * <li>{@code `field1`.`data`.`nested`.`trId`} is also nullable, so no
changes.
Review Comment:
Very nice explanation and example!
##########
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/config/TableConfigOptions.java:
##########
@@ -157,6 +157,15 @@ private TableConfigOptions() {}
+ "By default, all top-level
columns of the table's "
+ "schema are selected and nested
fields are retained.");
+ @Documentation.TableOption(execMode =
Documentation.ExecMode.BATCH_STREAMING)
+ public static final ConfigOption<Boolean> LEGACY_EXTENDED_ROW_STRUCT_KIND =
Review Comment:
Sync name of option with key.
--
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]
