Airblader commented on a change in pull request #17811:
URL: https://github.com/apache/flink/pull/17811#discussion_r750645009
##########
File path:
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/config/ExecutionConfigOptions.java
##########
@@ -388,6 +409,20 @@
DROP
}
+ /**
+ * The enforcer to guarantee that precision of CHAR/VARCHAR columns is
respected when writing
+ * data into sink.
+ */
+ public enum CharPrecisionEnforcer {
+ /**
+ * Throws runtime exception when writing string values exceeding the
length defined in the
+ * type .
+ */
+ ERROR,
+ /** Trip string values to match the length defined as the CHAR/VARCHAR
precision. */
Review comment:
```suggestion
/** Trim string values to match the length defined as the
CHAR/VARCHAR precision. */
```
##########
File path:
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/config/ExecutionConfigOptions.java
##########
@@ -121,6 +121,27 @@
+ "into NOT NULL columns. Users can change
the behavior to 'drop' to "
+ "silently drop such records without
throwing exception.");
+ @Documentation.TableOption(execMode =
Documentation.ExecMode.BATCH_STREAMING)
+ public static final ConfigOption<CharPrecisionEnforcer>
+ TABLE_EXEC_SINK_CHAR_PRECISION_ENFORCER =
+ key("table.exec.sink.char-precision-enforcer")
+ .enumType(CharPrecisionEnforcer.class)
+ .defaultValue(CharPrecisionEnforcer.ERROR.ERROR)
+ .withDescription(
Review comment:
Please don't list enum values, their explanation, or the default value
in the description; rather, document the enum values (implementing
`DescribedEnum`). The docs generator van then do a better job.
##########
File path:
flink-table/flink-table-planner/src/test/scala/org/apache/flink/table/planner/runtime/stream/table/TableSinkITCase.scala
##########
@@ -635,6 +635,58 @@ class TableSinkITCase extends StreamingTestBase {
assertEquals(expected.sorted, result.sorted)
}
+ @Test
Review comment:
I think we could write these tests using `TableFactoryHarness`. This
would avoid Scala and using these sources that require static state. Just a
thought, though, and maybe something to consider for how to utilize this
infrastructure going forward.
--
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]
