You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "ndrluis (via GitHub)" <gi...@apache.org> on 2024/11/04 01:52:16 UTC
[PR] Add table statistics update [iceberg-python]
ndrluis opened a new pull request, #1285:
URL: https://github.com/apache/iceberg-python/pull/1285
(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] WIP: Add table statistics [iceberg-python]
Posted by "ndrluis (via GitHub)" <gi...@apache.org>.
ndrluis commented on code in PR #1285:
URL: https://github.com/apache/iceberg-python/pull/1285#discussion_r1828195893
##########
pyiceberg/table/__init__.py:
##########
@@ -663,6 +666,42 @@ def update_location(self, location: str) -> Transaction:
"""
raise NotImplementedError("Not yet implemented")
+ def set_statistics(self, snapshot_id: int, statistics_file: StatisticsFile) -> Transaction:
+ """Set the statistics for a snapshot.
+
+ Args:
+ snapshot_id: The snapshot ID to set the statistics for.
+ statistics_file: The statistics file to set.
+
+ Returns:
+ The alter table builder.
+ """
+ updates = (
+ SetStatisticsUpdate(
+ snapshot_id=snapshot_id,
+ statistics=statistics_file,
+ ),
+ )
+
+ return self._apply(updates, ())
+
+ def remove_statistics(self, snapshot_id: int) -> Transaction:
+ """Remove the statistics for a snapshot.
+
+ Args:
+ snapshot_id: The snapshot ID to remove the statistics for.
+
+ Returns:
+ The alter table builder.
+ """
+ updates = (
+ RemoveStatisticsUpdate(
Review Comment:
I understand that we want to remove the statistics of a specific snapshot, and I understand that we have one statistics file per snapshot.
The equivalent code would be the [SetStatistics](https://github.com/apache/iceberg/blob/af5be32fbef36690d32b5e53c4153b709d8db188/core/src/main/java/org/apache/iceberg/SetStatistics.java#L27) class, which follows the same pattern as our [ManageSnapshot](https://github.com/apache/iceberg-python/blob/main/pyiceberg/table/update/snapshot.py#L724) class. This is the scenario I want to double-check to ensure we follow the same pattern.
--
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] Add table statistics [iceberg-python]
Posted by "ndrluis (via GitHub)" <gi...@apache.org>.
ndrluis commented on PR #1285:
URL: https://github.com/apache/iceberg-python/pull/1285#issuecomment-2453710152
I plan to move the set/remove statistics methods from the Transaction class to another class, such as ManageSnapshot. In the meantime, I’d like to confirm with everyone if I’m heading in the right direction with the current implementation."
@Fokko @sungwy @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] Add table statistics [iceberg-python]
Posted by "ndrluis (via GitHub)" <gi...@apache.org>.
ndrluis commented on PR #1285:
URL: https://github.com/apache/iceberg-python/pull/1285#issuecomment-2466990871
@kevinjqliu could you please review it once more?
--
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] WIP: Add table statistics [iceberg-python]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #1285:
URL: https://github.com/apache/iceberg-python/pull/1285#discussion_r1828029917
##########
pyiceberg/table/statistics.py:
##########
@@ -0,0 +1,41 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from typing import (
+ Dict,
+ List,
+ Optional,
+)
+
+from pydantic import Field
+
+from pyiceberg.typedef import IcebergBaseModel
+
+
+class BlobMetadata(IcebergBaseModel):
+ type: str
+ snapshot_id: int = Field(alias="snapshot-id")
+ sequence_number: int = Field(alias="sequence-number")
+ fields: List[int]
+ properties: Optional[Dict[str, str]] = None
+
+
+class StatisticsFile(IcebergBaseModel):
+ snapshot_id: int = Field(alias="snapshot-id")
+ statistics_path: str = Field(alias="statistics-path")
+ file_size_in_bytes: int = Field(alias="file-size-in-bytes")
+ file_footer_size_in_bytes: int = Field(alias="file-footer-size-in-bytes")
+ blob_metadata: List[BlobMetadata] = Field(alias="blob-metadata")
Review Comment:
nit, missing `key_metadata`
https://iceberg.apache.org/spec/#table-statistics
##########
pyiceberg/table/metadata.py:
##########
@@ -221,6 +222,14 @@ class TableMetadataCommonFields(IcebergBaseModel):
There is always a main branch reference pointing to the
current-snapshot-id even if the refs map is null."""
+ statistics: List[StatisticsFile] = Field(default_factory=list)
Review Comment:
https://iceberg.apache.org/spec/#table-metadata-fields
do you want to include both `statistics` and `partition-statistics`?
##########
pyiceberg/table/update/__init__.py:
##########
@@ -477,6 +491,29 @@ def _(
return base_metadata.model_copy(update={"default_sort_order_id": new_sort_order_id})
+@_apply_table_update.register(SetStatisticsUpdate)
+def _(update: SetStatisticsUpdate, base_metadata: TableMetadata, context: _TableMetadataUpdateContext) -> TableMetadata:
+ if update.snapshot_id != update.statistics.snapshot_id:
+ raise ValueError("Snapshot id in statistics does not match the snapshot id in the update")
+
+ rest_statistics = [stat for stat in base_metadata.statistics if stat.snapshot_id != update.snapshot_id]
Review Comment:
nit: this can be a helper function to filter on snapshot_id
##########
pyiceberg/table/__init__.py:
##########
@@ -663,6 +666,42 @@ def update_location(self, location: str) -> Transaction:
"""
raise NotImplementedError("Not yet implemented")
+ def set_statistics(self, snapshot_id: int, statistics_file: StatisticsFile) -> Transaction:
+ """Set the statistics for a snapshot.
+
+ Args:
+ snapshot_id: The snapshot ID to set the statistics for.
+ statistics_file: The statistics file to set.
+
+ Returns:
+ The alter table builder.
+ """
+ updates = (
+ SetStatisticsUpdate(
+ snapshot_id=snapshot_id,
+ statistics=statistics_file,
+ ),
+ )
+
+ return self._apply(updates, ())
+
+ def remove_statistics(self, snapshot_id: int) -> Transaction:
+ """Remove the statistics for a snapshot.
+
+ Args:
+ snapshot_id: The snapshot ID to remove the statistics for.
+
+ Returns:
+ The alter table builder.
+ """
+ updates = (
+ RemoveStatisticsUpdate(
Review Comment:
do you mind linking the java implementation? do we want to remove all stats?
##########
tests/conftest.py:
##########
@@ -918,6 +918,87 @@ def generate_snapshot(
"refs": {"test": {"snapshot-id": 3051729675574597004, "type": "tag", "max-ref-age-ms": 10000000}},
}
+TABLE_METADATA_V2_WITH_STATISTICS = {
+ "format-version": 2,
+ "table-uuid": "9c12d441-03fe-4693-9a96-a0705ddf69c1",
+ "location": "s3://bucket/test/location",
+ "last-sequence-number": 34,
+ "last-updated-ms": 1602638573590,
+ "last-column-id": 3,
+ "current-schema-id": 0,
+ "schemas": [
+ {
+ "type": "struct",
+ "schema-id": 0,
+ "fields": [
+ {
+ "id": 1,
+ "name": "x",
+ "required": True,
+ "type": "long",
+ }
+ ],
+ }
+ ],
+ "default-spec-id": 0,
+ "partition-specs": [{"spec-id": 0, "fields": []}],
+ "last-partition-id": 1000,
+ "default-sort-order-id": 0,
+ "sort-orders": [{"order-id": 0, "fields": []}],
+ "properties": {},
+ "current-snapshot-id": 3055729675574597004,
+ "snapshots": [
+ {
+ "snapshot-id": 3051729675574597004,
+ "timestamp-ms": 1515100955770,
+ "sequence-number": 0,
+ "summary": {"operation": "append"},
+ "manifest-list": "s3://a/b/1.avro",
+ },
+ {
+ "snapshot-id": 3055729675574597004,
+ "parent-snapshot-id": 3051729675574597004,
+ "timestamp-ms": 1555100955770,
+ "sequence-number": 1,
+ "summary": {"operation": "append"},
+ "manifest-list": "s3://a/b/2.avro",
+ "schema-id": 1,
+ },
+ ],
+ "statistics": [
+ {
+ "snapshot-id": 3051729675574597004,
+ "statistics-path": "s3://a/b/stats.puffin",
+ "file-size-in-bytes": 413,
+ "file-footer-size-in-bytes": 42,
+ "blob-metadata": [
+ {
+ "type": "ndv",
+ "snapshot-id": 3051729675574597004,
+ "sequence-number": 1,
+ "fields": [1],
+ }
+ ],
+ },
+ {
+ "snapshot-id": 3055729675574597004,
+ "statistics-path": "s3://a/b/stats.puffin",
Review Comment:
this does file need to exist on disk?
--
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] Add table statistics [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #1285:
URL: https://github.com/apache/iceberg-python/pull/1285#discussion_r1916863476
##########
dev/provision.py:
##########
@@ -401,3 +401,27 @@
)
spark.sql(f"ALTER TABLE {catalog_name}.default.test_empty_scan_ordered_str WRITE ORDERED BY id")
spark.sql(f"INSERT INTO {catalog_name}.default.test_empty_scan_ordered_str VALUES 'a', 'c'")
+
+ spark.sql(
+ f"""
+ CREATE OR REPLACE TABLE {catalog_name}.default.test_table_statistics_operations (
+ number integer
+ )
+ USING iceberg
+ TBLPROPERTIES (
+ 'format-version'='2'
+ );
+ """
+ )
+ spark.sql(
+ f"""
+ INSERT INTO {catalog_name}.default.test_table_statistics_operations
+ VALUES (1)
+ """
+ )
+ spark.sql(
+ f"""
+ INSERT INTO {catalog_name}.default.test_table_statistics_operations
+ VALUES (2)
+ """
+ )
Review Comment:
Why do this in Spark? It looks like there is nothing special going on here, and we can also create the table in PyIceberg as part of the test (this makes the tests self-contained).
##########
mkdocs/docs/api.md:
##########
@@ -1250,6 +1250,29 @@ with table.manage_snapshots() as ms:
ms.create_branch(snapshot_id1, "Branch_A").create_tag(snapshot_id2, "tag789")
```
+## Table Statistics Management
+
+Manage table statistics with operations through the `Table` API:
+
+```python
+# To run a specific operation
+table.update_statistics().set_statistics(snapshot_id, statistics_file).commit()
Review Comment:
I like it how you did it in the pythondoc as well:
```suggestion
table.update_statistics().set_statistics(snapshot_id=1, statistics_file=statistics_file).commit()
```
##########
pyiceberg/table/update/__init__.py:
##########
@@ -475,6 +489,28 @@ def _(
return base_metadata.model_copy(update={"default_sort_order_id": new_sort_order_id})
+@_apply_table_update.register(SetStatisticsUpdate)
+def _(update: SetStatisticsUpdate, base_metadata: TableMetadata, context: _TableMetadataUpdateContext) -> TableMetadata:
+ if update.snapshot_id != update.statistics.snapshot_id:
+ raise ValueError("Snapshot id in statistics does not match the snapshot id in the update")
Review Comment:
It's a bit of an [awkward check](https://lists.apache.org/thread/6fnrp73wokfqlt5vormpjyjmtvl29ll1), but something that we have to live with I guess.
##########
mkdocs/docs/api.md:
##########
@@ -1250,6 +1250,29 @@ with table.manage_snapshots() as ms:
ms.create_branch(snapshot_id1, "Branch_A").create_tag(snapshot_id2, "tag789")
```
+## Table Statistics Management
+
+Manage table statistics with operations through the `Table` API:
+
+```python
+# To run a specific operation
+table.update_statistics().set_statistics(snapshot_id, statistics_file).commit()
+# To run multiple operations
+table.update_statistics()
+ .set_statistics(snapshot_id1, statistics_file1)
+ .remove_statistics(snapshot_id2)
+ .commit()
+# Operations are applied on commit.
+```
+
+You can also use context managers to make more changes:
+
+```python
+with table.update_statistics() as update:
+ update.set_statistics(snaphsot_id1, statistics_file)
+ update.remove_statistics(snapshot_id2)
Review Comment:
The other examples are in the four-spaces camp :)
```suggestion
update.set_statistics(snaphsot_id1, statistics_file)
update.remove_statistics(snapshot_id2)
```
##########
pyiceberg/table/statistics.py:
##########
@@ -0,0 +1,49 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from typing import (
+ Dict,
+ List,
+ Optional,
+)
+
+from pydantic import Field
+
+from pyiceberg.typedef import IcebergBaseModel
+
+
+class BlobMetadata(IcebergBaseModel):
+ type: str
Review Comment:
How about being explicit about the options:
```suggestion
type: Literal['apache-datasketches-theta-v1', 'deletion-vector-v1']
```
https://iceberg.apache.org/puffin-spec/#blobmetadata
--
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] Add table statistics [iceberg-python]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on PR #1285:
URL: https://github.com/apache/iceberg-python/pull/1285#issuecomment-2576706997
@ndrluis do you mind resolving the merge conflict 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
Re: [PR] Add table statistics [iceberg-python]
Posted by "ndrluis (via GitHub)" <gi...@apache.org>.
ndrluis commented on PR #1285:
URL: https://github.com/apache/iceberg-python/pull/1285#issuecomment-2470432842
> Do you know which engine currently can generate puffin files? would be great to add an integration with a spark generated puffin file
@kevinjqliu As far as I know, only Trino can generate them. What kind of test would you like to have? I believe we are covering all relevant cases for this PR. If PyIceberg could generate or read puffin files, then I agree it would be useful to add tests to check compatibility between engines. However, I think it only makes sense to test puffin files during reading, as testing generation would mean verifying the implementation of something that isn’t our responsibility. In this case, it’s just a metadata update.
What do you think?
--
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] Add table statistics [iceberg-python]
Posted by "ndrluis (via GitHub)" <gi...@apache.org>.
ndrluis commented on code in PR #1285:
URL: https://github.com/apache/iceberg-python/pull/1285#discussion_r1838046096
##########
mkdocs/docs/api.md:
##########
@@ -1129,6 +1129,28 @@ with table.manage_snapshots() as ms:
ms.create_branch(snapshot_id1, "Branch_A").create_tag(snapshot_id2, "tag789")
```
+## Table Statistics Management
+
+Manage table statistics with operations through the `Table` API:
+
+```python
+# To run a specific operation
+table.update_statistics().set_statistics(snapshot_id, statistics_file).commit()
+# To run multiple operations
+table.update_statistics()
+ .set_statistics(snapshot_id1, statistics_file1)
+ .remove_statistics(snapshot_id2)
+# Operations are applied on commit.
Review Comment:
I added the commit() and kept the comment consistent with the other examples.
--
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] Add table statistics [iceberg-python]
Posted by "ndrluis (via GitHub)" <gi...@apache.org>.
ndrluis commented on PR #1285:
URL: https://github.com/apache/iceberg-python/pull/1285#issuecomment-2578671151
@kevinjqliu Done!
--
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] Add table statistics [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko merged PR #1285:
URL: https://github.com/apache/iceberg-python/pull/1285
--
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] WIP: Add table statistics [iceberg-python]
Posted by "ndrluis (via GitHub)" <gi...@apache.org>.
ndrluis commented on code in PR #1285:
URL: https://github.com/apache/iceberg-python/pull/1285#discussion_r1828199465
##########
tests/conftest.py:
##########
@@ -918,6 +918,87 @@ def generate_snapshot(
"refs": {"test": {"snapshot-id": 3051729675574597004, "type": "tag", "max-ref-age-ms": 10000000}},
}
+TABLE_METADATA_V2_WITH_STATISTICS = {
+ "format-version": 2,
+ "table-uuid": "9c12d441-03fe-4693-9a96-a0705ddf69c1",
+ "location": "s3://bucket/test/location",
+ "last-sequence-number": 34,
+ "last-updated-ms": 1602638573590,
+ "last-column-id": 3,
+ "current-schema-id": 0,
+ "schemas": [
+ {
+ "type": "struct",
+ "schema-id": 0,
+ "fields": [
+ {
+ "id": 1,
+ "name": "x",
+ "required": True,
+ "type": "long",
+ }
+ ],
+ }
+ ],
+ "default-spec-id": 0,
+ "partition-specs": [{"spec-id": 0, "fields": []}],
+ "last-partition-id": 1000,
+ "default-sort-order-id": 0,
+ "sort-orders": [{"order-id": 0, "fields": []}],
+ "properties": {},
+ "current-snapshot-id": 3055729675574597004,
+ "snapshots": [
+ {
+ "snapshot-id": 3051729675574597004,
+ "timestamp-ms": 1515100955770,
+ "sequence-number": 0,
+ "summary": {"operation": "append"},
+ "manifest-list": "s3://a/b/1.avro",
+ },
+ {
+ "snapshot-id": 3055729675574597004,
+ "parent-snapshot-id": 3051729675574597004,
+ "timestamp-ms": 1555100955770,
+ "sequence-number": 1,
+ "summary": {"operation": "append"},
+ "manifest-list": "s3://a/b/2.avro",
+ "schema-id": 1,
+ },
+ ],
+ "statistics": [
+ {
+ "snapshot-id": 3051729675574597004,
+ "statistics-path": "s3://a/b/stats.puffin",
+ "file-size-in-bytes": 413,
+ "file-footer-size-in-bytes": 42,
+ "blob-metadata": [
+ {
+ "type": "ndv",
+ "snapshot-id": 3051729675574597004,
+ "sequence-number": 1,
+ "fields": [1],
+ }
+ ],
+ },
+ {
+ "snapshot-id": 3055729675574597004,
+ "statistics-path": "s3://a/b/stats.puffin",
Review Comment:
No, there is no validation in place. This is only used for clients that support puffin files and for the expire snapshot procedure, which removes this information from the metadata. If the user wants, they can also remove the file as part of the expire snapshot procedure.
--
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] WIP: Add table statistics [iceberg-python]
Posted by "ndrluis (via GitHub)" <gi...@apache.org>.
ndrluis commented on code in PR #1285:
URL: https://github.com/apache/iceberg-python/pull/1285#discussion_r1828189220
##########
pyiceberg/table/metadata.py:
##########
@@ -221,6 +222,14 @@ class TableMetadataCommonFields(IcebergBaseModel):
There is always a main branch reference pointing to the
current-snapshot-id even if the refs map is null."""
+ statistics: List[StatisticsFile] = Field(default_factory=list)
Review Comment:
I was thinking of adding partition-statistics in another PR to make it easier to 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
Re: [PR] Add table statistics [iceberg-python]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #1285:
URL: https://github.com/apache/iceberg-python/pull/1285#discussion_r1837496028
##########
mkdocs/docs/api.md:
##########
@@ -1129,6 +1129,28 @@ with table.manage_snapshots() as ms:
ms.create_branch(snapshot_id1, "Branch_A").create_tag(snapshot_id2, "tag789")
```
+## Table Statistics Management
+
+Manage table statistics with operations through the `Table` API:
+
+```python
+# To run a specific operation
+table.update_statistics().set_statistics(snapshot_id, statistics_file).commit()
+# To run multiple operations
+table.update_statistics()
+ .set_statistics(snapshot_id1, statistics_file1)
+ .remove_statistics(snapshot_id2)
+# Operations are applied on commit.
Review Comment:
nit: add `.commit()` instead of the comment
##########
mkdocs/docs/api.md:
##########
@@ -1129,6 +1129,28 @@ with table.manage_snapshots() as ms:
ms.create_branch(snapshot_id1, "Branch_A").create_tag(snapshot_id2, "tag789")
```
+## Table Statistics Management
+
+Manage table statistics with operations through the `Table` API:
+
+```python
+# To run a specific operation
+table.update_statistics().set_statistics(snapshot_id, statistics_file).commit()
+# To run multiple operations
+table.update_statistics()
+ .set_statistics(snapshot_id1, statistics_file1)
+ .remove_statistics(snapshot_id2)
+# Operations are applied on commit.
+```
+
+You can also use context managers to make more changes:
+
+```python
+with table.update_statistics() as update:
+ update.set_statistics(1, statistics_file)
+ update.remove_statistics(2)
Review Comment:
nit: replace `1`/`2` with `snapshot_id1`/`snapshot_id2` to show the input relation
##########
pyiceberg/table/statistics.py:
##########
@@ -0,0 +1,49 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from typing import (
+ Dict,
+ List,
+ Optional,
+)
+
+from pydantic import Field
+
+from pyiceberg.typedef import IcebergBaseModel
+
+
+class BlobMetadata(IcebergBaseModel):
+ type: str
+ snapshot_id: int = Field(alias="snapshot-id")
+ sequence_number: int = Field(alias="sequence-number")
+ fields: List[int]
+ properties: Optional[Dict[str, str]] = None
+
+
+class StatisticsFile(IcebergBaseModel):
+ snapshot_id: int = Field(alias="snapshot-id")
+ statistics_path: str = Field(alias="statistics-path")
+ file_size_in_bytes: int = Field(alias="file-size-in-bytes")
+ file_footer_size_in_bytes: int = Field(alias="file-footer-size-in-bytes")
+ key_metadata: Optional[str] = Field(alias="key-metadata", default=None)
+ blob_metadata: List[BlobMetadata] = Field(alias="blob-metadata")
+
+
+def reject_statistics(
Review Comment:
nit: how about `filter_statistics_by_snapshot_id`?
##########
mkdocs/docs/api.md:
##########
@@ -1129,6 +1129,28 @@ with table.manage_snapshots() as ms:
ms.create_branch(snapshot_id1, "Branch_A").create_tag(snapshot_id2, "tag789")
```
+## Table Statistics Management
+
+Manage table statistics with operations through the `Table` API:
+
+```python
+# To run a specific operation
+table.update_statistics().set_statistics(snapshot_id, statistics_file).commit()
+# To run multiple operations
+table.update_statistics()
+ .set_statistics(snapshot_id1, statistics_file1)
+ .remove_statistics(snapshot_id2)
+# Operations are applied on commit.
Review Comment:
or use `snapshot_id=1`
--
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