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/07/22 06:35:15 UTC

[GitHub] [iceberg] deadwind4 opened a new pull request, #5334: Docs: Add doc of the upsert option

deadwind4 opened a new pull request, #5334:
URL: https://github.com/apache/iceberg/pull/5334

   Issue link: https://github.com/apache/iceberg/issues/5333


-- 
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


[GitHub] [iceberg] deadwind4 commented on a diff in pull request #5334: Docs: Add doc of the upsert option

Posted by GitBox <gi...@apache.org>.
deadwind4 commented on code in PR #5334:
URL: https://github.com/apache/iceberg/pull/5334#discussion_r928453975


##########
docs/configuration.md:
##########
@@ -79,6 +79,7 @@ Iceberg tables support table properties to configure table behavior, like the de
 | write.object-storage.enabled       | false              | Enables the object storage location provider that adds a hash component to file paths |
 | write.data.path                    | table location + /data | Base location for data files |
 | write.metadata.path                | table location + /metadata | Base location for metadata files |
+| write.upsert.enabled               | false              | Enables upsert mode (currently flink only) |

Review Comment:
   https://github.com/apache/iceberg/blob/5951ece2cedf82c69ae717a20c6b392b0b76237c/docs/flink-getting-started.md?plain=1#L570
   
   The `upsert-enabled` option in the Flink section is not general table property, but the `write.upsert.enabled` option is general table property. Maybe, in the future, we will implement `write.upsert.enabled` in Spark.
   
   The `upsert-enabled` option has been in the doc, but there is no `write.upsert.enabled` in the doc. I have a fear that some users get confusing.



-- 
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


[GitHub] [iceberg] deadwind4 commented on pull request #5334: Docs: Add doc of the upsert option

Posted by GitBox <gi...@apache.org>.
deadwind4 commented on PR #5334:
URL: https://github.com/apache/iceberg/pull/5334#issuecomment-1203559865

   @rdblue I have moved this option into the Flink section.


-- 
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


[GitHub] [iceberg] rdblue commented on a diff in pull request #5334: Docs: Add doc of the upsert option

Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5334:
URL: https://github.com/apache/iceberg/pull/5334#discussion_r930208363


##########
docs/configuration.md:
##########
@@ -79,6 +79,7 @@ Iceberg tables support table properties to configure table behavior, like the de
 | write.object-storage.enabled       | false              | Enables the object storage location provider that adds a hash component to file paths |
 | write.data.path                    | table location + /data | Base location for data files |
 | write.metadata.path                | table location + /metadata | Base location for metadata files |
+| write.upsert.enabled               | false              | Enables upsert mode (currently flink only) |

Review Comment:
   I think it's a problem that this is a general property that is unsupported by all engines other than Flink.
   
   Since we can't really fix the property name, let's move it to a Flink section so it is clear that only Flink supports it.



-- 
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


[GitHub] [iceberg] deadwind4 commented on a diff in pull request #5334: Docs: Add doc of the upsert option

Posted by GitBox <gi...@apache.org>.
deadwind4 commented on code in PR #5334:
URL: https://github.com/apache/iceberg/pull/5334#discussion_r932915189


##########
docs/configuration.md:
##########
@@ -79,6 +79,7 @@ Iceberg tables support table properties to configure table behavior, like the de
 | write.object-storage.enabled       | false              | Enables the object storage location provider that adds a hash component to file paths |
 | write.data.path                    | table location + /data | Base location for data files |
 | write.metadata.path                | table location + /metadata | Base location for metadata files |
+| write.upsert.enabled               | false              | Enables upsert mode (currently flink only) |

Review Comment:
   I have moved this option into the Flink section.



-- 
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


[GitHub] [iceberg] deadwind4 commented on a diff in pull request #5334: Docs: Add doc of the upsert option

Posted by GitBox <gi...@apache.org>.
deadwind4 commented on code in PR #5334:
URL: https://github.com/apache/iceberg/pull/5334#discussion_r928453975


##########
docs/configuration.md:
##########
@@ -79,6 +79,7 @@ Iceberg tables support table properties to configure table behavior, like the de
 | write.object-storage.enabled       | false              | Enables the object storage location provider that adds a hash component to file paths |
 | write.data.path                    | table location + /data | Base location for data files |
 | write.metadata.path                | table location + /metadata | Base location for metadata files |
+| write.upsert.enabled               | false              | Enables upsert mode (currently flink only) |

Review Comment:
   https://github.com/apache/iceberg/blob/5951ece2cedf82c69ae717a20c6b392b0b76237c/docs/flink-getting-started.md?plain=1#L570
   
   The `upsert-enabled` option in the Flink section is not general table property, but the `write.upsert.enabled` option is general table property. Maybe, in the future, we will implement `write.upsert.enabled` in Spark.
   
   The `upsert-enabled` option has been in the doc, but there is no `write.upsert.enabled` in the doc. It could get confused for users.



-- 
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


[GitHub] [iceberg] rdblue commented on a diff in pull request #5334: Docs: Add doc of the upsert option

Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5334:
URL: https://github.com/apache/iceberg/pull/5334#discussion_r927852230


##########
docs/configuration.md:
##########
@@ -79,6 +79,7 @@ Iceberg tables support table properties to configure table behavior, like the de
 | write.object-storage.enabled       | false              | Enables the object storage location provider that adds a hash component to file paths |
 | write.data.path                    | table location + /data | Base location for data files |
 | write.metadata.path                | table location + /metadata | Base location for metadata files |
+| write.upsert.enabled               | false              | Enables upsert mode (currently flink only) |

Review Comment:
   I think this should be in a Flink section, not in general table properties. Is there a Flink section for these?



-- 
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