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 2024/03/15 14:03:36 UTC
[PR] Add Snapshots table metadata [iceberg-python]
Fokko opened a new pull request, #524:
URL: https://github.com/apache/iceberg-python/pull/524
(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] Add Snapshots table metadata [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #524:
URL: https://github.com/apache/iceberg-python/pull/524#discussion_r1528049941
##########
tests/integration/test_writes.py:
##########
@@ -664,3 +668,55 @@ def test_table_properties_raise_for_none_value(
session_catalog, identifier, {"format-version": format_version, **property_with_none}, [arrow_table_with_null]
)
assert "None type is not a supported value in properties: property_name" in str(exc_info.value)
+
+
+@pytest.mark.integration
+@pytest.mark.parametrize("format_version", [1, 2])
+def test_inspect_snapshots(
+ spark: SparkSession, session_catalog: Catalog, arrow_table_with_null: pa.Table, format_version: int
+) -> None:
+ identifier = "default.table_metadata_snapshots"
+ tbl = _create_table(session_catalog, identifier, properties={"format-version": format_version})
+
+ tbl.overwrite(arrow_table_with_null)
+ # should produce a DELETE entry
+ tbl.overwrite(arrow_table_with_null)
+ # Since we don't rewrite, this should produce a new manifest with an ADDED entry
+ tbl.append(arrow_table_with_null)
+
+ df = tbl.inspect.snapshots()
Review Comment:
Ooh, I like that!
--
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 Snapshots table metadata [iceberg-python]
Posted by "Gowthami03B (via GitHub)" <gi...@apache.org>.
Gowthami03B commented on PR #524:
URL: https://github.com/apache/iceberg-python/pull/524#issuecomment-2013119342
@Fokko Can we merge this? I am almost done with "Files" table, so I can rebase my code before creating a PR.
--
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 Snapshots table metadata [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on PR #524:
URL: https://github.com/apache/iceberg-python/pull/524#issuecomment-2013620848
> Just have one question: I was thinking if later we need those metadata table classes, StaticTableScan, and StaticDataTask like what Java did. These may become useful when other engines (Daft, Ray) wants to represent the metadata tables in their dataframe. But since the metadata tables are normally not very large so using pyarrow as a bridge may be enough?
I'm open for that, but I would like to defer that to a later PR. I don't like the hard dependency on PyArrow, and would love to get rid of that, but I'm not sure what the best format is then. An Arrow table can be used in most engines without any copying.
I'll move this forward so @Gowthami03B can continue her work.
--
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 Snapshots table metadata [iceberg-python]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #524:
URL: https://github.com/apache/iceberg-python/pull/524#discussion_r1527191777
##########
tests/integration/test_writes.py:
##########
@@ -664,3 +668,55 @@ def test_table_properties_raise_for_none_value(
session_catalog, identifier, {"format-version": format_version, **property_with_none}, [arrow_table_with_null]
)
assert "None type is not a supported value in properties: property_name" in str(exc_info.value)
+
+
+@pytest.mark.integration
+@pytest.mark.parametrize("format_version", [1, 2])
+def test_inspect_snapshots(
+ spark: SparkSession, session_catalog: Catalog, arrow_table_with_null: pa.Table, format_version: int
+) -> None:
+ identifier = "default.table_metadata_snapshots"
+ tbl = _create_table(session_catalog, identifier, properties={"format-version": format_version})
+
+ tbl.overwrite(arrow_table_with_null)
+ # should produce a DELETE entry
+ tbl.overwrite(arrow_table_with_null)
+ # Since we don't rewrite, this should produce a new manifest with an ADDED entry
+ tbl.append(arrow_table_with_null)
+
+ df = tbl.inspect.snapshots()
Review Comment:
nit: since this is an integration test, should we also test spark's metadata table against pythons
--
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 Snapshots table metadata [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #524:
URL: https://github.com/apache/iceberg-python/pull/524#discussion_r1528301582
##########
tests/integration/test_writes.py:
##########
@@ -664,3 +668,55 @@ def test_table_properties_raise_for_none_value(
session_catalog, identifier, {"format-version": format_version, **property_with_none}, [arrow_table_with_null]
)
assert "None type is not a supported value in properties: property_name" in str(exc_info.value)
+
+
+@pytest.mark.integration
+@pytest.mark.parametrize("format_version", [1, 2])
+def test_inspect_snapshots(
+ spark: SparkSession, session_catalog: Catalog, arrow_table_with_null: pa.Table, format_version: int
+) -> None:
+ identifier = "default.table_metadata_snapshots"
+ tbl = _create_table(session_catalog, identifier, properties={"format-version": format_version})
+
+ tbl.overwrite(arrow_table_with_null)
+ # should produce a DELETE entry
+ tbl.overwrite(arrow_table_with_null)
+ # Since we don't rewrite, this should produce a new manifest with an ADDED entry
+ tbl.append(arrow_table_with_null)
+
+ df = tbl.inspect.snapshots()
Review Comment:
That was an excellent suggestion and actually caught a bug 🙌
--
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 Snapshots table metadata [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko merged PR #524:
URL: https://github.com/apache/iceberg-python/pull/524
--
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