You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/06/15 16:51:44 UTC
[GitHub] [iceberg] kbendick commented on a diff in pull request #5050: Flink: Enable upsert in the INSERT sql
kbendick commented on code in PR #5050:
URL: https://github.com/apache/iceberg/pull/5050#discussion_r898202097
##########
flink/v1.15/flink/src/main/java/org/apache/iceberg/flink/sink/FlinkSink.java:
##########
@@ -231,6 +231,11 @@ public Builder writeParallelism(int newWriteParallelism) {
return this;
}
+ public Builder properties(Map<String, String> properties) {
+ upsert(PropertyUtil.propertyAsBoolean(properties, UPSERT_ENABLED, UPSERT_ENABLED_DEFAULT));
+ return this;
+ }
Review Comment:
As a follow up, we should consider supporting more options from in-line SQL comments.
Is this `properties` needed for any SQL in-line comment based configuration or just for our own custom configurations that Flink doesn't know about?
##########
flink/v1.15/flink/src/main/java/org/apache/iceberg/flink/sink/FlinkSink.java:
##########
@@ -231,6 +231,11 @@ public Builder writeParallelism(int newWriteParallelism) {
return this;
}
+ public Builder properties(Map<String, String> properties) {
+ upsert(PropertyUtil.propertyAsBoolean(properties, UPSERT_ENABLED, UPSERT_ENABLED_DEFAULT));
Review Comment:
Nit: Please add a `Preconditions` check that the map isn't null (unless null needs to be allowed).
##########
flink/v1.15/flink/src/main/java/org/apache/iceberg/flink/sink/FlinkSink.java:
##########
@@ -231,6 +231,11 @@ public Builder writeParallelism(int newWriteParallelism) {
return this;
}
+ public Builder properties(Map<String, String> properties) {
Review Comment:
Nit: Can we add a javadoc here similar to the other ones? This might help me understand the flow better for non-SQL users. Users shouldn't pass in the run-time properties map, correct?
We might want to be somewhat more specific about which `properties` should come here.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org