You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "Fokko (via GitHub)" <gi...@apache.org> on 2025/04/03 11:11:51 UTC
[PR] Fix the snapshot summary of a partial overwrite [iceberg-python]
Fokko opened a new pull request, #1879:
URL: https://github.com/apache/iceberg-python/pull/1879
# Rationale for this change
@kevinjqliu PTAL. I took the liberty of providing a fix for this since I was curious where this was coming from, hope you don't mind! I've cherry-picked your commit with the test.
Java produces:
```json
{
"added-data-files": "1",
"added-files-size": "707",
"added-records": "2",
"app-id": "local-1743678304626",
"changed-partition-count": "1",
"deleted-data-files": "1",
"deleted-records": "3",
"engine-name": "spark",
"engine-version": "3.5.5",
"iceberg-version": "Apache Iceberg 1.8.1 (commit 9ce0fcf0af7becf25ad9fc996c3bad2afdcfd33d)",
"removed-files-size": "693",
"spark.app.id": "local-1743678304626",
"total-data-files": "3",
"total-delete-files": "0",
"total-equality-deletes": "0",
"total-files-size": "1993",
"total-position-deletes": "0",
"total-records": "4"
}
```
# Are these changes tested?
# Are there any user-facing changes?
<!-- In the case of user-facing changes, please add the changelog label. -->
--
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] Fix the snapshot summary of a partial overwrite [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #1879:
URL: https://github.com/apache/iceberg-python/pull/1879#discussion_r2044253646
##########
tests/integration/test_deletes.py:
##########
@@ -467,21 +467,19 @@ def test_partitioned_table_positional_deletes_sequence_number(spark: SparkSessio
assert snapshots[2].summary == Summary(
Review Comment:
Interesting. Looking at it, snapshot summary 1 seems incorrect. Could you open up an issue on the Java side? It would be good to get some historical context around this.
Also, we should set the `total-positional-deletes` to `0`, since we rewrite them.
--
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] Fix the snapshot summary of a partial overwrite [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #1879:
URL: https://github.com/apache/iceberg-python/pull/1879#discussion_r2044253646
##########
tests/integration/test_deletes.py:
##########
@@ -467,21 +467,19 @@ def test_partitioned_table_positional_deletes_sequence_number(spark: SparkSessio
assert snapshots[2].summary == Summary(
Review Comment:
Interesting. Looking at it, snapshot summary 1 seems incorrect. Could you open up an issue on the Java side? It would be good to get some historical context around this.
--
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] Fix the snapshot summary of a partial overwrite [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #1879:
URL: https://github.com/apache/iceberg-python/pull/1879#discussion_r2027379456
##########
pyiceberg/table/update/snapshot.py:
##########
@@ -236,7 +236,6 @@ def _summary(self, snapshot_properties: Dict[str, str] = EMPTY_DICT) -> Summary:
return update_snapshot_summaries(
summary=Summary(operation=self._operation, **ssc.build(), **snapshot_properties),
previous_summary=previous_snapshot.summary if previous_snapshot is not None else None,
- truncate_full_table=self._operation == Operation.OVERWRITE,
Review Comment:
Good one! We have a test here: https://github.com/apache/iceberg-python/blob/4d4714a46241d0d89519a2a605dbce27b713a60e/tests/integration/test_writes/test_writes.py#L253-L264
Let me double-check with Spark, because I think we should also include the files that are dropped 🤔
--
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] Fix the snapshot summary of a partial overwrite [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #1879:
URL: https://github.com/apache/iceberg-python/pull/1879#discussion_r2044268567
##########
tests/integration/test_deletes.py:
##########
@@ -467,21 +467,19 @@ def test_partitioned_table_positional_deletes_sequence_number(spark: SparkSessio
assert snapshots[2].summary == Summary(
Operation.OVERWRITE,
Review Comment:
When I change it into CoW, I get for snapshot summary 1 (the delete performend by Spark):
```json
{
"spark.app.id": "local-1744714815877",
"added-data-files": "1",
"deleted-data-files": "1",
"added-records": "1",
"deleted-records": "2",
"added-files-size": "714",
"removed-files-size": "743",
"changed-partition-count": "1",
"total-records": "4",
"total-files-size": "1461",
"total-data-files": "2",
"total-delete-files": "0",
"total-position-deletes": "0",
"total-equality-deletes": "0",
"engine-version": "3.5.1",
"app-id": "local-1744714815877",
"engine-name": "spark",
"iceberg-version": "Apache Iceberg 1.8.0 (commit c277c2014a1b37fe755cfe37f173b6465bb8cb73)"
}
```
Which seems correct:
```
(10, 100),
(10, 101), <- Deleted by Spark
(20, 200),
(20, 201),
(20, 202)
```
PyIceberg has a different approach, where this is an `Overwrite`, and first creates a snapshot that rewrites the original data file, then appends a new file with the new updated record.
To reproduce this, I just removed the `TBLPROPERTIES` to set MoR.
--
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] Fix the snapshot summary of a partial overwrite [iceberg-python]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #1879:
URL: https://github.com/apache/iceberg-python/pull/1879#discussion_r2048195532
##########
tests/integration/test_deletes.py:
##########
@@ -467,21 +467,19 @@ def test_partitioned_table_positional_deletes_sequence_number(spark: SparkSessio
assert snapshots[2].summary == Summary(
Review Comment:
I was able to reproduce it using #1926. I'll also open an issue on the java side
--
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] Fix the snapshot summary of a partial overwrite [iceberg-python]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #1879:
URL: https://github.com/apache/iceberg-python/pull/1879#discussion_r2048213266
##########
tests/integration/test_deletes.py:
##########
@@ -467,21 +467,19 @@ def test_partitioned_table_positional_deletes_sequence_number(spark: SparkSessio
assert snapshots[2].summary == Summary(
Review Comment:
https://github.com/apache/iceberg/issues/12823 opened an issue on the java side
--
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] Fix the snapshot summary of a partial overwrite [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #1879:
URL: https://github.com/apache/iceberg-python/pull/1879#discussion_r2027376060
##########
tests/integration/test_writes/test_writes.py:
##########
@@ -262,6 +262,100 @@ def test_summaries(spark: SparkSession, session_catalog: Catalog, arrow_table_wi
}
+@pytest.mark.integration
+def test_summaries_partial_overwrite(spark: SparkSession, session_catalog: Catalog) -> None:
+ identifier = "default.test_summaries_partial_overwrite"
Review Comment:
I was hoping that the JSON in the comments would be sufficient. The `dev/provision.py` is pretty slow, so I think we want to be reluctant with adding stuff there.
--
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] Fix the snapshot summary of a partial overwrite [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on PR #1879:
URL: https://github.com/apache/iceberg-python/pull/1879#issuecomment-2810618650
Thanks for the review @kevinjqliu 🙌
--
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] Fix the snapshot summary of a partial overwrite [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #1879:
URL: https://github.com/apache/iceberg-python/pull/1879#discussion_r2027518680
##########
pyiceberg/table/update/snapshot.py:
##########
@@ -236,7 +236,6 @@ def _summary(self, snapshot_properties: Dict[str, str] = EMPTY_DICT) -> Summary:
return update_snapshot_summaries(
summary=Summary(operation=self._operation, **ssc.build(), **snapshot_properties),
previous_summary=previous_snapshot.summary if previous_snapshot is not None else None,
- truncate_full_table=self._operation == Operation.OVERWRITE,
Review Comment:
It looks like we also need to record the dropped entries:
```json
{
"engine-version": "3.5.5",
"added-data-files": "1",
"total-equality-deletes": "0",
"app-id": "local-1743703872146",
"added-records": "5",
"deleted-data-files": "1",
"deleted-records": "5",
"total-records": "5",
"spark.app.id": "local-1743703872146",
"removed-files-size": "750",
"changed-partition-count": "1",
"engine-name": "spark",
"total-position-deletes": "0",
"added-files-size": "750",
"total-delete-files": "0",
"iceberg-version": "Apache Iceberg 1.8.1 (commit 9ce0fcf0af7becf25ad9fc996c3bad2afdcfd33d)",
"total-files-size": "750",
"total-data-files": "1"
}
```
Let's get that fixed!
--
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] Fix the snapshot summary of a partial overwrite [iceberg-python]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #1879:
URL: https://github.com/apache/iceberg-python/pull/1879#discussion_r2027360226
##########
tests/integration/test_writes/test_writes.py:
##########
@@ -262,6 +262,100 @@ def test_summaries(spark: SparkSession, session_catalog: Catalog, arrow_table_wi
}
+@pytest.mark.integration
+def test_summaries_partial_overwrite(spark: SparkSession, session_catalog: Catalog) -> None:
+ identifier = "default.test_summaries_partial_overwrite"
Review Comment:
should we just add your jupyter notebook example to `dev/provision.py` and verify that the 2 operations produce the same snapshot summary?
##########
pyiceberg/table/update/snapshot.py:
##########
@@ -236,7 +236,6 @@ def _summary(self, snapshot_properties: Dict[str, str] = EMPTY_DICT) -> Summary:
return update_snapshot_summaries(
summary=Summary(operation=self._operation, **ssc.build(), **snapshot_properties),
previous_summary=previous_snapshot.summary if previous_snapshot is not None else None,
- truncate_full_table=self._operation == Operation.OVERWRITE,
Review Comment:
do you know if there's a test to verify that full table overwrite still has the correct snapshot summary?
--
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] Fix the snapshot summary of a partial overwrite [iceberg-python]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #1879:
URL: https://github.com/apache/iceberg-python/pull/1879#discussion_r2048195204
##########
tests/integration/test_deletes.py:
##########
@@ -467,21 +467,19 @@ def test_partitioned_table_positional_deletes_sequence_number(spark: SparkSessio
assert snapshots[2].summary == Summary(
Operation.OVERWRITE,
Review Comment:
taking another look this summary is correct,
```
{
'added-data-files': '1', # CoW replaced (delete+add) a data file
'added-files-size': '949',
'added-records': '2', # CoW removed 3 records and added 2 records (effectively removing 1 record)
'changed-partition-count': '1', # change a single partition
'deleted-data-files': '1', # CoW replaced (delete+add) a data file
'deleted-records': '3', # CoW removed 3 records and added 2 records (effectively removing 1 record)
'removed-files-size': '747',
'total-data-files': '2', # still 2 data files total, 1 per partition
'total-delete-files': '1', # positional delete from previous delete
'total-equality-deletes': '0',
'total-files-size': '3402',
'total-position-deletes': '1', # positional delete from previous delete
'total-records': '4' # decrement previous total-records
}
```
--
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] Fix the snapshot summary of a partial overwrite [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #1879:
URL: https://github.com/apache/iceberg-python/pull/1879#discussion_r2044186417
##########
pyiceberg/table/snapshots.py:
##########
@@ -356,6 +357,11 @@ def update_snapshot_summaries(
raise ValueError(f"Operation not implemented: {summary.operation}")
if truncate_full_table and summary.operation == Operation.OVERWRITE and previous_summary is not None:
+ deprecation_message(
+ deprecated_in="0.10.0",
+ removed_in="0.11.0",
+ help_message="The truncate-full-table should be used.",
Review Comment:
```suggestion
help_message="The truncate-full-table shouldn't be used.",
```
--
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] Fix the snapshot summary of a partial overwrite [iceberg-python]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #1879:
URL: https://github.com/apache/iceberg-python/pull/1879#discussion_r2040215134
##########
tests/integration/test_deletes.py:
##########
@@ -467,21 +467,19 @@ def test_partitioned_table_positional_deletes_sequence_number(spark: SparkSessio
assert snapshots[2].summary == Summary(
Operation.OVERWRITE,
Review Comment:
maybe something in spark is off... the `total-records` here should be `3` based on the assert right below
##########
tests/integration/test_deletes.py:
##########
@@ -467,21 +467,19 @@ def test_partitioned_table_positional_deletes_sequence_number(spark: SparkSessio
assert snapshots[2].summary == Summary(
Operation.OVERWRITE,
Review Comment:
hm i used spark to perform the same partial overwrite,
```
run_spark_commands(
spark,
[
f"""
ALTER TABLE {identifier}
SET TBLPROPERTIES (
'write.delete.mode' = 'copy-on-write'
);
""",
# Replace data file
f"""
DELETE FROM {identifier} WHERE number = 201
""",
],
)
```
and this is the snapshot summary
```
{
"added-data-files": "1",
"added-files-size": "763",
"added-records": "2",
"changed-partition-count": "1",
"deleted-data-files": "1",
"deleted-records": "3",
"removed-files-size": "747",
"total-data-files": "2",
"total-delete-files": "1",
"total-equality-deletes": "0",
"total-files-size": "3214",
"total-position-deletes": "1",
"total-records": "4",
}
```
which is the same from above...
##########
tests/integration/test_deletes.py:
##########
@@ -467,21 +467,19 @@ def test_partitioned_table_positional_deletes_sequence_number(spark: SparkSessio
assert snapshots[2].summary == Summary(
Operation.OVERWRITE,
Review Comment:
this snapshot summary seems wrong
```
{
'added-data-files': '1',
'added-files-size': '949',
'added-records': '2', # ? this op only deleted 1 record
'changed-partition-count': '1',
'deleted-data-files': '1',
'deleted-records': '3', # ? this op only deleted 1 record
'removed-files-size': '747',
'total-data-files': '2',
'total-delete-files': '1', # ? should be no delete files
'total-equality-deletes': '0',
'total-files-size': '3402',
'total-position-deletes': '1', # ? should be no delete files
'total-records': '4' # ? should only have 3 records (see below assert)
}
```
`tbl.delete(EqualTo("number", 201))` is COW, it replaces the original data file for the `20` partition with a new data file with 1 less record
##########
tests/integration/test_deletes.py:
##########
@@ -467,21 +467,19 @@ def test_partitioned_table_positional_deletes_sequence_number(spark: SparkSessio
assert snapshots[2].summary == Summary(
Review Comment:
i ran this locally. snapshot 0 has the following metadata, which is correct
```
{
'added-data-files': '2',
'added-files-size': '1490',
'added-records': '5',
'changed-partition-count': '2',
'total-data-files': '2',
'total-delete-files': '0',
'total-equality-deletes': '0',
'total-files-size': '1490',
'total-position-deletes': '0',
'total-records': '5'
}
```
but snapshot 1, DELETE op with the positional delete, has the following metadata,
```
{
'added-delete-files': '1',
'added-files-size': '1710',
'added-position-delete-files': '1',
'added-position-deletes': '1',
'changed-partition-count': '1',
'total-data-files': '2',
'total-delete-files': '1',
'total-equality-deletes': '0',
'total-files-size': '3200',
'total-position-deletes': '1',
'total-records': '5'
}
```
everything looks right except for the `total-records`. we started off with 5 records, and the DELETE op removed 1 record. So `total-records` should be `4` here.
Is this a bug in the spark snapshot summary?
##########
tests/integration/test_writes/test_writes.py:
##########
@@ -262,6 +262,100 @@ def test_summaries(spark: SparkSession, session_catalog: Catalog, arrow_table_wi
}
+@pytest.mark.integration
+def test_summaries_partial_overwrite(spark: SparkSession, session_catalog: Catalog) -> None:
+ identifier = "default.test_summaries_partial_overwrite"
+ TEST_DATA = {
+ "id": [1, 2, 3, 1, 1],
+ "name": ["AB", "CD", "EF", "CD", "EF"],
+ }
+ pa_schema = pa.schema(
+ [
+ pa.field("id", pa.int32()),
+ pa.field("name", pa.string()),
+ ]
+ )
+ arrow_table = pa.Table.from_pydict(TEST_DATA, schema=pa_schema)
+ tbl = _create_table(session_catalog, identifier, {"format-version": "2"}, schema=pa_schema)
+ with tbl.update_spec() as txn:
+ txn.add_identity("id")
+ tbl.append(arrow_table)
+
+ # TODO: We might want to check why this ends up in 3 files
+ assert len(tbl.inspect.data_files()) == 3
Review Comment:
its in 3 files because there are 3 unique `id` values and we partition by `id`, right?
```suggestion
assert len(tbl.inspect.data_files()) == 3
```
--
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] Fix the snapshot summary of a partial overwrite [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #1879:
URL: https://github.com/apache/iceberg-python/pull/1879#discussion_r2027518680
##########
pyiceberg/table/update/snapshot.py:
##########
@@ -236,7 +236,6 @@ def _summary(self, snapshot_properties: Dict[str, str] = EMPTY_DICT) -> Summary:
return update_snapshot_summaries(
summary=Summary(operation=self._operation, **ssc.build(), **snapshot_properties),
previous_summary=previous_snapshot.summary if previous_snapshot is not None else None,
- truncate_full_table=self._operation == Operation.OVERWRITE,
Review Comment:
It looks like we also need to record the dropped entries:
```json
{
"engine-version": "3.5.5",
"added-data-files": "1",
"total-equality-deletes": "0",
"app-id": "local-1743703872146",
"added-records": "5",
"deleted-data-files": "1",
"deleted-records": "5",
"total-records": "5",
"spark.app.id": "local-1743703872146",
"removed-files-size": "750",
"changed-partition-count": "1",
"engine-name": "spark",
"total-position-deletes": "0",
"added-files-size": "750",
"total-delete-files": "0",
"iceberg-version": "Apache Iceberg 1.8.1 (commit 9ce0fcf0af7becf25ad9fc996c3bad2afdcfd33d)",
"total-files-size": "750",
"total-data-files": "1"
}
```
Let's get that fixed!
--
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] Fix the snapshot summary of a partial overwrite [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #1879:
URL: https://github.com/apache/iceberg-python/pull/1879#discussion_r2044183681
##########
tests/integration/test_writes/test_writes.py:
##########
@@ -262,6 +262,100 @@ def test_summaries(spark: SparkSession, session_catalog: Catalog, arrow_table_wi
}
+@pytest.mark.integration
+def test_summaries_partial_overwrite(spark: SparkSession, session_catalog: Catalog) -> None:
+ identifier = "default.test_summaries_partial_overwrite"
+ TEST_DATA = {
+ "id": [1, 2, 3, 1, 1],
+ "name": ["AB", "CD", "EF", "CD", "EF"],
+ }
+ pa_schema = pa.schema(
+ [
+ pa.field("id", pa.int32()),
+ pa.field("name", pa.string()),
+ ]
+ )
+ arrow_table = pa.Table.from_pydict(TEST_DATA, schema=pa_schema)
+ tbl = _create_table(session_catalog, identifier, {"format-version": "2"}, schema=pa_schema)
+ with tbl.update_spec() as txn:
+ txn.add_identity("id")
+ tbl.append(arrow_table)
+
+ # TODO: We might want to check why this ends up in 3 files
+ assert len(tbl.inspect.data_files()) == 3
Review Comment:
Yes, your correct. I missed the partitioning when first looking at the code :)
--
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] Fix the snapshot summary of a partial overwrite [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko merged PR #1879:
URL: https://github.com/apache/iceberg-python/pull/1879
--
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] Fix the snapshot summary of a partial overwrite [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #1879:
URL: https://github.com/apache/iceberg-python/pull/1879#discussion_r2027521950
##########
pyiceberg/table/update/snapshot.py:
##########
@@ -236,7 +236,6 @@ def _summary(self, snapshot_properties: Dict[str, str] = EMPTY_DICT) -> Summary:
return update_snapshot_summaries(
summary=Summary(operation=self._operation, **ssc.build(), **snapshot_properties),
previous_summary=previous_snapshot.summary if previous_snapshot is not None else None,
- truncate_full_table=self._operation == Operation.OVERWRITE,
Review Comment:
Ah, it is correct! We write a delete+append == overwrite.
--
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] Fix the snapshot summary of a partial overwrite [iceberg-python]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #1879:
URL: https://github.com/apache/iceberg-python/pull/1879#discussion_r2027386623
##########
tests/integration/test_writes/test_writes.py:
##########
@@ -262,6 +262,100 @@ def test_summaries(spark: SparkSession, session_catalog: Catalog, arrow_table_wi
}
+@pytest.mark.integration
+def test_summaries_partial_overwrite(spark: SparkSession, session_catalog: Catalog) -> None:
+ identifier = "default.test_summaries_partial_overwrite"
Review Comment:
sounds good. Spark snapshot summary wont be changing anyways so its fine having just the 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