You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "kevinjqliu (via GitHub)" <gi...@apache.org> on 2025/09/28 06:32:01 UTC
[PR] infra: use spark base image for docker [iceberg-python]
kevinjqliu opened a new pull request, #2540:
URL: https://github.com/apache/iceberg-python/pull/2540
<!--
Thanks for opening a pull request!
-->
<!-- In the case this PR will resolve an issue, please replace ${GITHUB_ISSUE_ID} below with the actual Github issue id. -->
<!-- Closes #${GITHUB_ISSUE_ID} -->
# Rationale for this change
## 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] infra: use spark base image for docker [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko merged PR #2540:
URL: https://github.com/apache/iceberg-python/pull/2540
--
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] infra: use spark base image for docker [iceberg-python]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #2540:
URL: https://github.com/apache/iceberg-python/pull/2540#discussion_r2395293508
##########
dev/provision.py:
##########
@@ -23,35 +22,27 @@
from pyiceberg.schema import Schema
from pyiceberg.types import FixedType, NestedField, UUIDType
-# The configuration is important, otherwise we get many small
-# parquet files with a single row. When a positional delete
-# hits the Parquet file with one row, the parquet file gets
-# dropped instead of having a merge-on-read delete file.
-spark = (
- SparkSession
- .builder
- .config("spark.sql.shuffle.partitions", "1")
- .config("spark.default.parallelism", "1")
Review Comment:
Looks like spark connect doesnt support these options
```
.config("spark.sql.shuffle.partitions", "1")
.config("spark.default.parallelism", "1")
```
And `INSERT INTO` writes 1 data file per row. In order to force a single data file, im using
```
.coalesce(1).writeTo(identifier).append()
```
--
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] infra: use spark base image for docker [iceberg-python]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #2540:
URL: https://github.com/apache/iceberg-python/pull/2540#discussion_r2386232833
##########
dev/provision.py:
##########
@@ -23,35 +22,27 @@
from pyiceberg.schema import Schema
from pyiceberg.types import FixedType, NestedField, UUIDType
-# The configuration is important, otherwise we get many small
-# parquet files with a single row. When a positional delete
-# hits the Parquet file with one row, the parquet file gets
-# dropped instead of having a merge-on-read delete file.
-spark = (
- SparkSession
- .builder
- .config("spark.sql.shuffle.partitions", "1")
- .config("spark.default.parallelism", "1")
Review Comment:
yea i remember these, i can add them to spark-defaults but the tests are passing now without 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] infra: use spark base image for docker [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #2540:
URL: https://github.com/apache/iceberg-python/pull/2540#discussion_r2392242974
##########
dev/provision.py:
##########
@@ -23,35 +22,27 @@
from pyiceberg.schema import Schema
from pyiceberg.types import FixedType, NestedField, UUIDType
-# The configuration is important, otherwise we get many small
-# parquet files with a single row. When a positional delete
-# hits the Parquet file with one row, the parquet file gets
-# dropped instead of having a merge-on-read delete file.
-spark = (
- SparkSession
- .builder
- .config("spark.sql.shuffle.partitions", "1")
- .config("spark.default.parallelism", "1")
Review Comment:
The tests are passing, but we're not testing the positional deletes anymore since Spark will throw away the whole file, instead of creating the positional deletes:
The following test illustrates the problem:
```
diff --git a/tests/integration/test_reads.py b/tests/integration/test_reads.py
index 375eb35b2..ed6e805e3 100644
--- a/tests/integration/test_reads.py
+++ b/tests/integration/test_reads.py
@@ -432,6 +432,11 @@ def test_pyarrow_deletes(catalog: Catalog, format_version: int) -> None:
# (11, 'k'),
# (12, 'l')
test_positional_mor_deletes = catalog.load_table(f"default.test_positional_mor_deletes_v{format_version}")
+
+ if format_version == 2:
+ files = test_positional_mor_deletes.scan().plan_files()
+ assert all([len(file.delete_files) > 0 for file in files])
+
arrow_table = test_positional_mor_deletes.scan().to_arrow()
assert arrow_table["number"].to_pylist() == [1, 2, 3, 4, 5, 6, 7, 8, 10, 11, 12]
```
This one passes on main but fails on this branch.
--
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] infra: use spark base image for docker [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #2540:
URL: https://github.com/apache/iceberg-python/pull/2540#discussion_r2386230181
##########
dev/provision.py:
##########
@@ -23,35 +22,27 @@
from pyiceberg.schema import Schema
from pyiceberg.types import FixedType, NestedField, UUIDType
-# The configuration is important, otherwise we get many small
-# parquet files with a single row. When a positional delete
-# hits the Parquet file with one row, the parquet file gets
-# dropped instead of having a merge-on-read delete file.
-spark = (
- SparkSession
- .builder
- .config("spark.sql.shuffle.partitions", "1")
- .config("spark.default.parallelism", "1")
Review Comment:
I think we've set these to avoid creating multiple files with just one row. This way, when a shuffle is performed, it will be coalesced into a single file. This affects tests such as positional deletes, because when all the rows in a single file are marked for deletion, the whole file is dropped instead of creating merge-on-read deletes.
--
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