You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "paultmathew (via GitHub)" <gi...@apache.org> on 2026/05/20 19:43:53 UTC
[PR] perf(upsert): prune destination scan via df partition-column ranges a… [iceberg-python]
paultmathew opened a new pull request, #3387:
URL: https://github.com/apache/iceberg-python/pull/3387
# Rationale for this change
`Transaction.upsert` builds its scan `row_filter` from `join_cols`
alone via `create_match_filter`. When the partition spec sources from
columns NOT in `join_cols` — a common pattern for append-only event
logs partitioned by time but keyed by composite IDs — two amplifying
problems fall out at scan plan time:
- `inclusive_projection` collapses the predicate to `AlwaysTrue`
against the partition spec, so **partition pruning never fires** and
every file in the table is listed (related: #2138, #2159, #3129).
- Per-file metrics evaluation of the `Or(And(EqualTo, EqualTo), …)`
predicate on UUID-shaped key columns **can't prune either** —
per-file `lower_bound` / `upper_bound` stats span essentially the
full UUID space, so the metrics evaluator passes every file.
The result is a full-table scan at every upsert. For tables with 10k+
partitions this is multi-minute / multi-gigabyte work per call.
## What this PR does
Two complementary optimisations to `Transaction.upsert`:
1. **Partition-range augmentation**. New helper
`upsert_util.augment_filter_with_partition_ranges` derives
`[min, max]` predicates from `df` for every partition source column
present in the frame and ANDs them into the row filter.
`inclusive_projection` then projects each range through its
partition transform (`hours`, `days`, `months`, `years`, `identity`,
`truncate`) at scan-plan time, enabling manifest- and file-level
pruning.
2. **Column-projection for the insert-only path**. When
`when_matched_update_all=False` the consumer loop only reads
`join_cols` off each destination batch (to build the per-batch
source-side match filter). Passing `selected_fields=tuple(join_cols)`
to `DataScan` lets the parquet reader prune wide non-key columns.
The existing `_projected_field_ids.union(extract_field_ids(...))`
keeps the partition-range predicate's columns readable.
### Correctness guards
The augmentation skips per-column in three cases:
- The partition source column isn't present in `df` (no bound to derive).
- The column is entirely null in `df` (no `min`/`max`).
- The column has **any** null in `df` — a non-null `GreaterThanOrEqual`
predicate would exclude NULL-partition destination rows whose
`(join_cols)` may collide with null-partition source rows. Skip
pruning over emitting an unsafe predicate.
When `min == max`, an `EqualTo` is emitted instead of the range pair.
Multiple partition fields sourcing from the same column emit one
source-column range; `inclusive_projection` projects through each
partition field independently at scan time. Bucket and other
non-monotonic transforms return `None` from their `project` method on
inequalities — the projection contributes `AlwaysTrue` for them, no
harm.
## Are these changes tested?
Yes:
- 13 new unit tests in `tests/table/test_upsert.py`:
- Pure-function tests for `augment_filter_with_partition_ranges`
(unpartitioned, missing column, all-null, partial-null,
single-value, range, multi-field-sharing-source).
- End-to-end upsert semantics with partition spec NOT in
`join_cols`, IN `join_cols`, and unpartitioned.
- `DataScan.plan_files()` count assertion against a deterministically
seeded table that defeats per-file metrics pruning — confirms the
augmentation prunes vs the original predicate.
- `selected_fields` projection assertions for both
`when_matched_update_all=True` (legacy `('*',)`) and `=False`
(narrow `join_cols`-only).
- End-to-end upsert with `when_matched_update_all=True` against a
wide table to confirm column projection doesn't break the update
path.
- 23 existing upsert tests still pass.
### Smoke test — real Iceberg-on-S3 + Glue table
Run against a real Iceberg table representative of the workload this
optimisation targets.
**Stack**
- `pyiceberg.catalog.glue.GlueCatalog`
- AWS S3 warehouse, parquet data files
- Iceberg format v2
**Target table**
- Write mode: append-only event log
- `unique_keys`: `[conversation_id, id]` (composite UUID/string key)
- `partition_spec`: `hours(created_at)`
- Size at the test snapshot: **~10,450 data files, ~3.2 GiB total**
- Hourly partitions over ~15 months of history
- Avg file size: ~0.32 MiB (post hourly OPTIMIZE compaction)
- Schema (6 columns):
- `conversation_id` (string, UUID v4) — key
- `id` (string, UUID v4) — key
- `event` (string, short tag, ~10 B/row)
- `log` (string, JSON payload, **~400 B/row median**)
- `created_at` (timestamp[us, UTC]) — partition source
- `version` (int32)
**Source synthesis** (two modes for the comparison):
- `synthetic`: random UUIDs; `conversation_id` drawn from a pool sized
`rows/30` so leading-key cardinality matches realistic parent-child
distribution; `created_at` uniformly in `[now − hours, now]`. Keys
don't overlap destination → metrics evaluator rejects every file at
scan-plan time, so both projections return 0 files. **Isolates the
planning cost.**
- `from-destination`: reads N recent rows from the destination via a
`created_at` range scan, used as the source. Keys DO overlap →
exercises the **file-count reduction** and **column-read** effect.
**Results** (read-only via `DataScan.plan_files()` and
`to_arrow_batch_reader()`):
| Scenario | Original plan | Augmented plan | Plan-time win | Wide read | Narrow read |
|---|---|---|---|---|---|
| 1 000 synthetic rows / 24 h | 0 / 10 452 (454.04 s) | 0 / 10 452 (3.11 s) | **146×** | — | — |
| 1 000 dest-sampled rows / 24 h | (skipped, 7-min baseline) | **7 / 10 452 (99.93%)** | — | 493 KiB / 1 000 rows | 68 KiB / 1 000 rows (**86% smaller**) |
| 10 000 dest-sampled rows / 168 h (catch-up flush) | (skipped) | **58 / 10 492 (99.4%)** | — | — | — |
The 146× plan-time win is on a 1 000-disjunct predicate against a
~10k-file table; the original cost scales linearly with table file
count and predicate disjunct count, the augmented cost scales with the
source's `created_at` span instead.
The 86% byte reduction is dominated by skipping the `log` (JSON
payload) column at the parquet reader — that one column carries ~80%
of the row width on this table.
For a representative larger flush — 1.16M source rows spanning ~24 h
— extrapolating both wins reduces the destination-scan working set
from multiple GiB (which is OOM-territory on 8 GiB worker pods) down
to tens of MiB.
## Are there any user-facing changes?
No API change. The optimisation is purely internal to
`Transaction.upsert`:
- The new helper is exported from `pyiceberg.table.upsert_util` for
testability but isn't part of the public API.
- `selected_fields=tuple(join_cols)` is passed conditionally inside the
method — no signature change to `Table.upsert` or
`Transaction.upsert`.
## Related
- Relates to #2138 (partition-pruning suggestion by `@koenvo`), #2159
(umbrella slow-upsert tracker), #3129 (recent: `create_match_filter`
+ no-partition-prune).
- Complementary to (closed-stale) #2943's "coarse match filter"
approach — that PR shrinks the row predicate itself; this one adds
partition pruning the row predicate can't trigger on its own. Both
can coexist.
##### Was generative AI tooling used to co-author this PR?
- [X] Yes — Claude (Cursor agent)
--
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] perf(upsert): prune destination scan via df partition-column ranges a… [iceberg-python]
Posted by "abnobdoss (via GitHub)" <gi...@apache.org>.
abnobdoss commented on PR #3387:
URL: https://github.com/apache/iceberg-python/pull/3387#issuecomment-4539397818
Hey @paultmathew, thanks for the clarification. I’m curious whether your use case still has performance problems when the partition source column is included in `join_cols`, as I think you’re proposing here.
My assumption is that in that case `create_match_filter` already includes exact predicates on the partition source column, so partition pruning should have enough information to fire. If it still doesn’t, is the issue that the exact `Or(And(EqualTo, ...), ...)` predicate is too expensive/large for planning rather than that the partition signal is missing?
--
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] perf(upsert): prune destination scan via df partition-column ranges a… [iceberg-python]
Posted by "paultmathew (via GitHub)" <gi...@apache.org>.
paultmathew commented on PR #3387:
URL: https://github.com/apache/iceberg-python/pull/3387#issuecomment-4539710229
@abnobdoss You're right. My goal was to replace the Or(And(EqualTo, ...)) predicate with an O(1) range so the projection visitor would do less work, but I didn't account for the fact that inclusive_projection walks the full N-disjunct tree regardless of what I AND onto it — so the augmentation adds work rather than substituting for it, and create_match_filter's own per-disjunct projection is already producing the prune target.
I'll drop the partition augmentation and keep just the selected_fields=tuple(join_cols) change. The column-projection win is independent of partition spec and stands on its own.
The underlying gap that motivated the augmentation — large flushes against partition-source-not-in-join-cols tables hitting full-table scans — is a separate problem that needs a different primitive. Equality deletes by primary key are the cleanest Iceberg-native fit for it, so I'll follow up on that thread.
Will trim the diff and update the PR description shortly.
--
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] perf(upsert): project join_cols only on destination scan when when_matched_update_all=False [iceberg-python]
Posted by "abnobdoss (via GitHub)" <gi...@apache.org>.
abnobdoss commented on code in PR #3387:
URL: https://github.com/apache/iceberg-python/pull/3387#discussion_r3321956520
##########
tests/table/test_upsert.py:
##########
@@ -888,3 +892,176 @@ def test_upsert_snapshot_properties(catalog: Catalog) -> None:
for snapshot in snapshots[initial_snapshot_count:]:
assert snapshot.summary is not None
assert snapshot.summary.additional_properties.get("test_prop") == "test_value"
+
+
+class TestUpsertScanProjection:
Review Comment:
This feels slightly excessive to test the destination scan's fields are filtered - would one or two test functions suffice?
--
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] perf(upsert): project join_cols only on destination scan when when_matched_update_all=False [iceberg-python]
Posted by "paultmathew (via GitHub)" <gi...@apache.org>.
paultmathew commented on code in PR #3387:
URL: https://github.com/apache/iceberg-python/pull/3387#discussion_r3359839261
##########
tests/table/test_upsert.py:
##########
@@ -888,3 +892,176 @@ def test_upsert_snapshot_properties(catalog: Catalog) -> None:
for snapshot in snapshots[initial_snapshot_count:]:
assert snapshot.summary is not None
assert snapshot.summary.additional_properties.get("test_prop") == "test_value"
+
+
+class TestUpsertScanProjection:
Review Comment:
Good call. I trimmed it down to 1 test.
--
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] perf(upsert): prune destination scan via df partition-column ranges a… [iceberg-python]
Posted by "abnobdoss (via GitHub)" <gi...@apache.org>.
abnobdoss commented on code in PR #3387:
URL: https://github.com/apache/iceberg-python/pull/3387#discussion_r3300276591
##########
tests/table/test_upsert.py:
##########
@@ -888,3 +892,660 @@ def test_upsert_snapshot_properties(catalog: Catalog) -> None:
for snapshot in snapshots[initial_snapshot_count:]:
assert snapshot.summary is not None
assert snapshot.summary.additional_properties.get("test_prop") == "test_value"
+
+
+# ---------------------------------------------------------------------------
+# Partition-range augmentation for upsert row filters.
+#
+# ``Transaction.upsert`` builds its scan ``row_filter`` from ``join_cols``
+# alone via ``create_match_filter``. When the partition spec sources from
+# columns NOT in ``join_cols`` (a common pattern for append-only event logs
+# partitioned by time but keyed by composite IDs), ``inclusive_projection``
+# collapses the entire predicate to ``AlwaysTrue`` against the partition
+# spec and ``DataScan.plan_files`` falls through to a full table scan.
+#
+# ``augment_filter_with_partition_ranges`` derives ``[min, max]`` predicates
+# from ``df`` for every partition source column present in the frame and
+# ANDs them into the row filter. Iceberg's inclusive projection then
+# projects each range through the partition transform when planning the
+# scan, enabling manifest- and file-level pruning.
+#
+# See related issues #2138, #2159, #3129.
+# ---------------------------------------------------------------------------
+
+
+class TestAugmentFilterWithPartitionRanges:
+ """Pure-function tests for ``augment_filter_with_partition_ranges``.
+
+ Asserts the structural shape of the augmented predicate. End-to-end
+ file-pruning behaviour is exercised by the upsert integration tests
+ below.
+ """
+
+ @staticmethod
+ def _orders_schema() -> Schema:
+ return Schema(
+ NestedField(1, "order_id", IntegerType(), required=True),
+ NestedField(2, "order_date", DateType(), required=True),
+ NestedField(3, "order_type", StringType(), required=True),
+ )
+
+ @staticmethod
+ def _orders_pa_schema() -> pa.Schema:
+ return pa.schema(
+ [
+ pa.field("order_id", pa.int32(), nullable=False),
+ pa.field("order_date", pa.date32(), nullable=False),
+ pa.field("order_type", pa.string(), nullable=False),
+ ]
+ )
+
+ def _df(self, rows: list[dict[str, object]]) -> pa.Table:
+ return pa.Table.from_pylist(rows, schema=self._orders_pa_schema())
+
+ def test_unpartitioned_spec_returns_input_unchanged(self) -> None:
+ """Tables without a partition spec have nothing to project through.
+ The augmentation must short-circuit and hand back the exact
+ ``matched_predicate`` object — no allocation, no semantic change."""
+ df = self._df([{"order_id": 1, "order_date": datetime.date(2026, 1, 1), "order_type": "A"}])
+ matched = create_match_filter(df, ["order_id"])
+ augmented = augment_filter_with_partition_ranges(
+ matched_predicate=matched,
+ df=df,
+ schema=self._orders_schema(),
+ spec=UNPARTITIONED_PARTITION_SPEC,
+ )
+ assert augmented is matched
+
+ def test_partition_source_column_not_in_df_skipped(self) -> None:
+ """Source frames that don't contain the partition source column
+ can't contribute a bound — the augmentation has to skip rather
+ than guess. Returns ``matched_predicate`` unchanged so the
+ existing scan behaviour applies."""
+ df_no_date = pa.Table.from_pylist(
+ [{"order_id": 1, "order_type": "A"}],
+ schema=pa.schema(
+ [
+ pa.field("order_id", pa.int32(), nullable=False),
+ pa.field("order_type", pa.string(), nullable=False),
+ ]
+ ),
+ )
+ matched = create_match_filter(df_no_date, ["order_id"])
+ spec = PartitionSpec(PartitionField(source_id=2, field_id=1000, transform=IdentityTransform(), name="order_date"))
+ augmented = augment_filter_with_partition_ranges(
+ matched_predicate=matched,
+ df=df_no_date,
+ schema=self._orders_schema(),
+ spec=spec,
+ )
+ assert augmented == matched
+
+ def test_partition_source_column_all_nulls_skipped(self) -> None:
+ """When every value of the partition source column in ``df`` is
+ null, there is no meaningful ``min`` / ``max`` to bound the
+ predicate. Skip rather than emit a vacuous augmentation."""
+ df = pa.Table.from_pylist(
+ [{"order_id": 1, "order_date": None, "order_type": "A"}],
+ schema=pa.schema(
+ [
+ pa.field("order_id", pa.int32(), nullable=False),
+ pa.field("order_date", pa.date32(), nullable=True),
+ pa.field("order_type", pa.string(), nullable=False),
+ ]
+ ),
+ )
+ matched = create_match_filter(df, ["order_id"])
+ spec = PartitionSpec(PartitionField(source_id=2, field_id=1000, transform=IdentityTransform(), name="order_date"))
+ augmented = augment_filter_with_partition_ranges(
+ matched_predicate=matched,
+ df=df,
+ schema=self._orders_schema(),
+ spec=spec,
+ )
+ assert augmented == matched
+
+ def test_partition_source_column_some_nulls_skipped(self) -> None:
+ """Correctness guard: a partial-null source column cannot use a
+ non-null ``GreaterThanOrEqual`` augmentation because destination
+ rows whose partition value is NULL would be excluded from the
+ match scan even though their ``(key)`` may collide with the
+ null-partition source rows. Skip pruning over emitting an unsafe
+ predicate."""
+ df = pa.Table.from_pylist(
+ [
+ {"order_id": 1, "order_date": datetime.date(2026, 1, 1), "order_type": "A"},
+ {"order_id": 2, "order_date": None, "order_type": "B"},
+ ],
+ schema=pa.schema(
+ [
+ pa.field("order_id", pa.int32(), nullable=False),
+ pa.field("order_date", pa.date32(), nullable=True),
+ pa.field("order_type", pa.string(), nullable=False),
+ ]
+ ),
+ )
+ matched = create_match_filter(df, ["order_id"])
+ spec = PartitionSpec(PartitionField(source_id=2, field_id=1000, transform=IdentityTransform(), name="order_date"))
+ augmented = augment_filter_with_partition_ranges(
+ matched_predicate=matched,
+ df=df,
+ schema=self._orders_schema(),
+ spec=spec,
+ )
+ assert augmented == matched
+
+ def test_single_value_partition_column_emits_equal_to(self) -> None:
+ """``min == max`` collapses to a single ``EqualTo`` — tighter than
+ the range pair and lets exact partition pruning fire (e.g. when
+ every source row falls in the same hourly bucket)."""
+ df = self._df(
+ [
+ {"order_id": 1, "order_date": datetime.date(2026, 1, 1), "order_type": "A"},
+ {"order_id": 2, "order_date": datetime.date(2026, 1, 1), "order_type": "B"},
+ ]
+ )
+ matched = create_match_filter(df, ["order_id"])
+ spec = PartitionSpec(PartitionField(source_id=2, field_id=1000, transform=IdentityTransform(), name="order_date"))
+ augmented = augment_filter_with_partition_ranges(
+ matched_predicate=matched,
+ df=df,
+ schema=self._orders_schema(),
+ spec=spec,
+ )
+ assert augmented == And(matched, EqualTo("order_date", datetime.date(2026, 1, 1)))
+
+ def test_range_emits_gteq_and_lteq(self) -> None:
+ """Multiple distinct values → ``GreaterThanOrEqual(min) AND
+ LessThanOrEqual(max)`` pair, AND'd onto the original matched
+ predicate. Inclusive_projection handles the partition-transform
+ projection at scan time."""
+ df = self._df(
+ [
+ {"order_id": 1, "order_date": datetime.date(2026, 1, 1), "order_type": "A"},
+ {"order_id": 2, "order_date": datetime.date(2026, 1, 15), "order_type": "B"},
+ {"order_id": 3, "order_date": datetime.date(2026, 2, 1), "order_type": "C"},
+ ]
+ )
+ matched = create_match_filter(df, ["order_id"])
+ spec = PartitionSpec(PartitionField(source_id=2, field_id=1000, transform=IdentityTransform(), name="order_date"))
+ augmented = augment_filter_with_partition_ranges(
+ matched_predicate=matched,
+ df=df,
+ schema=self._orders_schema(),
+ spec=spec,
+ )
+ assert augmented == And(
+ And(matched, GreaterThanOrEqual("order_date", datetime.date(2026, 1, 1))),
+ LessThanOrEqual("order_date", datetime.date(2026, 2, 1)),
+ )
+
+ def test_multiple_partition_fields_share_source_id_emitted_once(self) -> None:
+ """When two partition fields source from the same column (e.g.
+ ``bucket(8, id), truncate(4, id)``), only one source-column range
+ is emitted. ``inclusive_projection`` projects through each
+ partition field independently at scan time, so a single source-
+ range predicate suffices for both."""
+ df = self._df(
+ [
+ {"order_id": 1, "order_date": datetime.date(2026, 1, 1), "order_type": "A"},
+ {"order_id": 10, "order_date": datetime.date(2026, 1, 2), "order_type": "B"},
+ ]
+ )
+ matched = create_match_filter(df, ["order_type"])
+
+ from pyiceberg.transforms import BucketTransform, TruncateTransform
+
+ spec = PartitionSpec(
+ PartitionField(source_id=1, field_id=1000, transform=BucketTransform(8), name="order_id_bucket"),
+ PartitionField(source_id=1, field_id=1001, transform=TruncateTransform(4), name="order_id_trunc"),
+ )
+ augmented = augment_filter_with_partition_ranges(
+ matched_predicate=matched,
+ df=df,
+ schema=self._orders_schema(),
+ spec=spec,
+ )
+ # Exactly one ``GreaterThanOrEqual`` + ``LessThanOrEqual`` pair on
+ # ``order_id`` — not duplicated for each partition field.
+ assert augmented == And(
+ And(matched, GreaterThanOrEqual("order_id", 1)),
+ LessThanOrEqual("order_id", 10),
+ )
+
+
+class TestUpsertPartitionPruningIntegration:
+ """End-to-end upsert against partitioned tables.
+
+ Verifies that the augmented row filter doesn't change upsert
+ semantics — ``rows_updated`` / ``rows_inserted`` match the original
+ behaviour — across the three structural cases:
+
+ 1. Partition source not in ``join_cols`` (the case the augmentation
+ fires for; biggest perf gain).
+ 2. Partition source IS in ``join_cols`` (augmentation contributes
+ redundantly but doesn't change correctness).
+ 3. Unpartitioned (augmentation is a no-op).
+ """
+
+ def test_upsert_correct_when_partition_col_not_in_join_cols(self, catalog: Catalog) -> None:
+ """Source partitioned by ``order_date`` but keyed on ``order_id``.
+ Augmentation fires — semantics must be identical to the
+ unpartitioned baseline."""
+ identifier = "default.test_upsert_partition_not_in_join_cols"
+ _drop_table(catalog, identifier)
+
+ schema = Schema(
+ NestedField(1, "order_id", IntegerType(), required=True),
+ NestedField(2, "order_date", DateType(), required=True),
+ NestedField(3, "order_type", StringType(), required=True),
+ )
+ spec = PartitionSpec(PartitionField(source_id=2, field_id=1000, transform=IdentityTransform(), name="order_date"))
+ table = catalog.create_table(identifier, schema=schema, partition_spec=spec)
+
+ arrow_schema = pa.schema(
+ [
+ pa.field("order_id", pa.int32(), nullable=False),
+ pa.field("order_date", pa.date32(), nullable=False),
+ pa.field("order_type", pa.string(), nullable=False),
+ ]
+ )
+ # Initial load: ids 1-5 across two different partitions.
+ initial = pa.Table.from_pylist(
+ [
+ {"order_id": 1, "order_date": datetime.date(2026, 1, 1), "order_type": "A"},
+ {"order_id": 2, "order_date": datetime.date(2026, 1, 1), "order_type": "A"},
+ {"order_id": 3, "order_date": datetime.date(2026, 1, 2), "order_type": "A"},
Review Comment:
Does this still pass if this target row keeps order_id=3 but changes order_date to datetime.date(2026, 5, 1)?
The quirk with this change is that it seems to assume the partition column is part of the row identity. I’m not sure when that’s valid outside cases where the partition is derived from the join key, and in those cases I’d expect the join-key filter to already be sufficient for pruning.
--
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] perf(upsert): project join_cols only on destination scan when when_matched_update_all=False [iceberg-python]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #3387:
URL: https://github.com/apache/iceberg-python/pull/3387#issuecomment-4964194722
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
--
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] perf(upsert): project join_cols only on destination scan when when_matched_update_all=False [iceberg-python]
Posted by "paultmathew (via GitHub)" <gi...@apache.org>.
paultmathew commented on code in PR #3387:
URL: https://github.com/apache/iceberg-python/pull/3387#discussion_r3359839261
##########
tests/table/test_upsert.py:
##########
@@ -888,3 +892,176 @@ def test_upsert_snapshot_properties(catalog: Catalog) -> None:
for snapshot in snapshots[initial_snapshot_count:]:
assert snapshot.summary is not None
assert snapshot.summary.additional_properties.get("test_prop") == "test_value"
+
+
+class TestUpsertScanProjection:
Review Comment:
Good call. I trimmed it down to 2 tests.
--
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] perf(upsert): project join_cols only on destination scan when when_matched_update_all=False [iceberg-python]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #3387: perf(upsert): project join_cols only on destination scan when when_matched_update_all=False
URL: https://github.com/apache/iceberg-python/pull/3387
--
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] perf(upsert): prune destination scan via df partition-column ranges a… [iceberg-python]
Posted by "paultmathew (via GitHub)" <gi...@apache.org>.
paultmathew commented on PR #3387:
URL: https://github.com/apache/iceberg-python/pull/3387#issuecomment-4539210702
@abnobdoss Thanks for the review. I agree that this would break things for existing users. I'll update the PR to gate the augmentation on a structural check: augment only when every partition source column is in join_cols (directly or via a deterministic transform on a join column). When that holds, the partition value is a function of the join key, so the augmentation can never exclude a destination file holding the matching key — correctness is preserved by construction. When it doesn't, the row filter is left unchanged and tx.upsert falls back to today's full-table-scan behaviour. The optimisation still covers the workload that motivated the PR — high-cardinality state tables with bucket(N, key) + unique_keys=[key], where partition source ⊆ join_cols by construction. I'll swap the smoke-test scenario to that shape so the benchmarks reflect the case the augmentation is actually safe for.
The underlying limitation — "find the row with key=X regardless of which partition holds it" — fundamentally requires either a full-table scan (today's tx.upsert behaviour without augmentation) or a delete-by-value primitive. Equality deletes by primary key are the only Iceberg-native mechanism I see that would let pyiceberg do partition-migration-correct upserts without scanning every file. I know the discussion at #3270 leans toward "don't write equality deletes, write deletion vectors instead", but deletion vectors are still position-keyed — they reduce the rewrite footprint once you've located a row, but they don't help with the lookup itself. So for the partition-migration class of upsert bug specifically, deletion vectors aren't the right tool.
@Fokko @kevinjqliu I take the maintainer concerns about equality-delete writes (compaction obligation, read-side merge cost, the rewrite-equality-to-position story) — but I think for the upsert workload the trade-off goes the other way. Happy to take a stab at an equality-delete write path in a separate PR, scoped initially to the cases tx.upsert could opt into, if there's appetite for revisiting that direction.
--
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] perf(upsert): project join_cols only on destination scan when when_matched_update_all=False [iceberg-python]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #3387:
URL: https://github.com/apache/iceberg-python/pull/3387#issuecomment-4888207659
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that's incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.
--
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