You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "manuzhang (via GitHub)" <gi...@apache.org> on 2025/05/22 07:50:14 UTC
[PR] Docs: Fix Flink upsert doc on equality fields requirement [iceberg]
manuzhang opened a new pull request, #13127:
URL: https://github.com/apache/iceberg/pull/13127
(no comment)
--
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
Re: [PR] Docs: Fix Flink upsert doc on equality fields requirement [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on code in PR #13127:
URL: https://github.com/apache/iceberg/pull/13127#discussion_r2102701620
##########
docs/docs/flink-writes.md:
##########
@@ -75,7 +75,7 @@ Iceberg supports `UPSERT` based on the primary key when writing data into v2 tab
```
!!! info
- OVERWRITE and UPSERT can't be set together. In UPSERT mode, if the table is partitioned, the partition fields should be included in equality fields.
+ OVERWRITE and UPSERT can't be set together. In UPSERT mode, if the table is partitioned, the source columns of partition fields should be included in equality fields.
Review Comment:
Basically descriptions of the following codes
https://github.com/apache/iceberg/blob/91dff9886e6e6c494f6a970129f4c08487c98a0d/flink/v2.0/flink/src/main/java/org/apache/iceberg/flink/sink/FlinkSink.java#L537-L545
I just noticed the error message is not accurate either. For a table with partition field `days(ts)`, it's `ts` that's required to be included in equality fields.
--
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
Re: [PR] Docs: Fix Flink upsert doc on equality fields requirement [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on code in PR #13127:
URL: https://github.com/apache/iceberg/pull/13127#discussion_r2102316783
##########
docs/docs/flink-writes.md:
##########
@@ -75,7 +75,7 @@ Iceberg supports `UPSERT` based on the primary key when writing data into v2 tab
```
!!! info
- OVERWRITE and UPSERT can't be set together. In UPSERT mode, if the table is partitioned, the partition fields should be included in equality fields.
+ OVERWRITE and UPSERT can't be set together. In UPSERT mode, if the table is partitioned, the source columns of partition fields should be included in equality fields.
Review Comment:
I don't understand this change. (I don't understand the sentence).
Could you please help me understand?
Thanks,
Peter
--
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
Re: [PR] Docs, Flink: Fix equality fields requirement in upsert mode [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary merged PR #13127:
URL: https://github.com/apache/iceberg/pull/13127
--
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
Re: [PR] Docs: Fix Flink upsert doc on equality fields requirement [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on code in PR #13127:
URL: https://github.com/apache/iceberg/pull/13127#discussion_r2102701620
##########
docs/docs/flink-writes.md:
##########
@@ -75,7 +75,7 @@ Iceberg supports `UPSERT` based on the primary key when writing data into v2 tab
```
!!! info
- OVERWRITE and UPSERT can't be set together. In UPSERT mode, if the table is partitioned, the partition fields should be included in equality fields.
+ OVERWRITE and UPSERT can't be set together. In UPSERT mode, if the table is partitioned, the source columns of partition fields should be included in equality fields.
Review Comment:
Basically descriptions of the following codes
https://github.com/apache/iceberg/blob/91dff9886e6e6c494f6a970129f4c08487c98a0d/flink/v2.0/flink/src/main/java/org/apache/iceberg/flink/sink/FlinkSink.java#L537-L545
I just noticed the error message is not accurate either. For a table with partition field `days(ts)`, it's `ts` that's required to be included in equality fields. We can't add `days(ts)` to equality fields.
--
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
Re: [PR] Docs: Fix Flink upsert doc on equality fields requirement [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on code in PR #13127:
URL: https://github.com/apache/iceberg/pull/13127#discussion_r2104092388
##########
docs/docs/flink-writes.md:
##########
@@ -75,7 +75,7 @@ Iceberg supports `UPSERT` based on the primary key when writing data into v2 tab
```
!!! info
- OVERWRITE and UPSERT can't be set together. In UPSERT mode, if the table is partitioned, the partition fields should be included in equality fields.
+ OVERWRITE and UPSERT can't be set together. In UPSERT mode, if the table is partitioned, the source columns of partition fields should be included in equality fields.
Review Comment:
Could you please add an example like this to the doc?
Now, I understand what `source columns of` means. But I needed the example to understand the exact meaning.
Thanks,
Peter
--
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
Re: [PR] Docs: Fix Flink upsert doc on equality fields requirement [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on code in PR #13127:
URL: https://github.com/apache/iceberg/pull/13127#discussion_r2102701620
##########
docs/docs/flink-writes.md:
##########
@@ -75,7 +75,7 @@ Iceberg supports `UPSERT` based on the primary key when writing data into v2 tab
```
!!! info
- OVERWRITE and UPSERT can't be set together. In UPSERT mode, if the table is partitioned, the partition fields should be included in equality fields.
+ OVERWRITE and UPSERT can't be set together. In UPSERT mode, if the table is partitioned, the source columns of partition fields should be included in equality fields.
Review Comment:
Basically descriptions of the following codes
https://github.com/apache/iceberg/blob/91dff9886e6e6c494f6a970129f4c08487c98a0d/flink/v2.0/flink/src/main/java/org/apache/iceberg/flink/sink/FlinkSink.java#L537-L545
--
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
Re: [PR] Docs, Flink: Fix equality fields requirement in upsert mode [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on code in PR #13127:
URL: https://github.com/apache/iceberg/pull/13127#discussion_r2106716484
##########
docs/docs/flink-writes.md:
##########
@@ -75,7 +75,7 @@ Iceberg supports `UPSERT` based on the primary key when writing data into v2 tab
```
!!! info
- OVERWRITE and UPSERT can't be set together. In UPSERT mode, if the table is partitioned, the partition fields should be included in equality fields.
+ OVERWRITE and UPSERT can't be set together. In UPSERT mode, if the table is partitioned, the source columns of partition fields should be included in equality fields. For partition field `days(ts)`, the source column `ts` should be included in equality fields.
Review Comment:
Maybe:
> OVERWRITE and UPSERT modes are mutually exclusive and cannot be enabled at the same time. When using UPSERT mode with a partitioned table, all of the source columns corresponding to the partition fields must be included in the equality fields. For example, if the partition field is `days(ts)`, then the id of the column `ts` must be part of the equality fields.
--
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
Re: [PR] Docs, Flink: Fix equality fields requirement in upsert mode [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on PR #13127:
URL: https://github.com/apache/iceberg/pull/13127#issuecomment-2916483984
Merged to main.
Thanks @manuzhang for the PR and @mxm for the review!
--
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