Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 78 additions & 0 deletions integration_tests/src/main/python/iceberg/iceberg_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,84 @@ def distinct_after_spj(spark):
gpu_plan_assertion=_assert_partial_clustering_spj_plan)


@iceberg
@ignore_order(local=True)
@pytest.mark.skipif(
not is_spark_400_or_later(),
reason="spark.sql.sources.v2.bucketing.partition.filter.enabled was added in Spark 4.0.0")
@pytest.mark.parametrize("partition_filter", [True, False], ids=["filtered", "unfiltered"])
@pytest.mark.parametrize("partially_clustered", [True, False],
ids=["partially_clustered", "clustered"])
def test_iceberg_spj_partition_filter(spark_tmp_table_factory, partition_filter,
partially_clustered):
left_table = get_full_table_name(spark_tmp_table_factory)
right_table = get_full_table_name(spark_tmp_table_factory)
table_props = _build_tblprops({
# Keep separate INSERTs as separate scan splits so that id=1 is partially clustered.
"read.split.target-size": "1",
"read.split.open-file-cost": "1",
})
table_props_sql = ", ".join(f"'{k}' = '{v}'" for k, v in table_props.items())

def setup_iceberg_tables(spark):
spark.sql(
f"CREATE TABLE {left_table} (id INT, price DOUBLE) USING ICEBERG "
f"PARTITIONED BY (id) TBLPROPERTIES ({table_props_sql})")
spark.sql(
f"CREATE TABLE {right_table} (id INT, value STRING) USING ICEBERG "
f"PARTITIONED BY (id) TBLPROPERTIES ({table_props_sql})")

# id=3 exists only on the left, so partition filtering prunes it from the left scan
# while the union of both sides would have kept it. The two id=1 rows land in separate
# files, which is what makes the left scan partially clustered.
spark.sql(f"INSERT INTO {left_table} VALUES (1, 40.0), (2, 10.0), (3, 15.5)")
spark.sql(f"INSERT INTO {left_table} VALUES (1, 41.0)")
spark.sql(f"INSERT INTO {right_table} VALUES (1, 'a'), (2, 'b')")

@firestarman firestarman Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT:
The extra key that partition filtering should prune (id=3) only exists on the left table, and the comments above say the two id=1 files are what make the left scan partially clustered.

GpuBatchScanExec applies the SPARK-48949 filter to both scans, then branches on replicatePartitions (pad vs replicate). A regression that applied the filter only in the pad (!replicatePartitions) branch would still pass this matrix, because the replicated right scan never enumerates a group that is absent from commonPartitionValues.

Could we add a right-only partition key (e.g. id=4 on the right table only) in the filtered + partially clustered cell, or parametrize which side holds the extra key? That would force the replicated scan through the same commonPartValuesMap filter before Seq.fill(numSplits.get)(splits). Inner-join output would be unchanged — id=4 would not appear in the result.

Suggested change
spark.sql(f"INSERT INTO {left_table} VALUES (1, 40.0), (2, 10.0), (3, 15.5)")
spark.sql(f"INSERT INTO {left_table} VALUES (1, 41.0)")
spark.sql(f"INSERT INTO {right_table} VALUES (1, 'a'), (2, 'b')")
spark.sql(f"INSERT INTO {left_table} VALUES (1, 40.0), (2, 10.0), (3, 15.5)")
spark.sql(f"INSERT INTO {left_table} VALUES (1, 41.0)")
spark.sql(f"INSERT INTO {right_table} VALUES (1, 'a'), (2, 'b')")
# Right-only key so the replicated scan also enumerates a group absent from
# the intersection (mirror of left-only id=3 on the pad side).
spark.sql(f"INSERT INTO {right_table} VALUES (4, 'd')")

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, real gap. With the old fixture the right scan is the replicate side and its keys {1, 2} are exactly the intersection, so it never had a group to drop.

Verified it bites: I built with the filter skipped when replicatePartitions=true. The old fixture passed, the new one fails on partially_clustered-filtered with the expected AssertionError.

One wrinkle with adding only id=4: it swaps the pad and replicate roles, and the new pad side has one file per key, so every numSplits is 1 and the counts collapse to 4/2 for both partially_clustered and clustered. That quietly disables the assertion covering partial clustering.

Made it symmetric instead: a key each side lacks (id=3 left, id=4 right), plus a two-file key inside the intersection on each side. Both branches are then covered whichever side Spark picks, counts are 3/5/2/4, and all four cells pass.


with_cpu_session(setup_iceberg_tables)

conf = {
"spark.sql.adaptive.enabled": "false",
"spark.sql.autoBroadcastJoinThreshold": "-1",
"spark.sql.sources.v2.bucketing.enabled": "true",
"spark.sql.sources.v2.bucketing.pushPartValues.enabled": "true",
"spark.sql.sources.v2.bucketing.partition.filter.enabled":
str(partition_filter).lower(),
"spark.sql.sources.v2.bucketing.partiallyClusteredDistribution.enabled":
str(partially_clustered).lower(),
"spark.sql.iceberg.planning.preserve-data-grouping": "true",
}

def join_after_spj(spark):
return spark.sql(
f"""
SELECT l.id, l.price, r.value
FROM {left_table} l
JOIN {right_table} r ON l.id = r.id
""")

# Both scans plan one partition per common partition value. Partition filtering drops id=3,
# which only the left side has, and partial clustering adds one back because the two id=1
# files get a partition each. KeyGroupedPartitioning.isPartiallyClustered would say this
# more directly but only exists on Spark 3.5.9+/4.0.3+/4.1.2+, which is the gate this test
# is deliberately avoiding.
expected_partitions = (2 if partition_filter else 3) + (1 if partially_clustered else 0)

def assert_plan(plan):
scans = _assert_spj_join_shape(plan, expect_spj=True)
counts = [scan.outputPartitioning().numPartitions() for scan in scans]
assert counts == [expected_partitions] * len(scans), \
f"Expected {expected_partitions} partitions per scan, found {counts}:\n{plan}"

# Asserting join output rather than SELECT DISTINCT keeps this off SPARK-55848, which is
# what forces the patch-level gate on test_iceberg_spj_partial_clustering_distinct.
assert_cpu_and_gpu_are_equal_collect_with_capture(
join_after_spj,
conf=conf,
require_non_empty=True,
gpu_plan_assertion=assert_plan)


# Enough rows that every bucket of the wider bucket(4) side is populated, so reducing it to
# gcd(4, 2) = 2 buckets moves rows into partition values the raw-keyed lookup cannot find.
_SPJ_REDUCIBLE_ROWS = 64
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,17 @@ case class GpuBatchScanExec(
.get
.map(t => (InternalRowComparableWrapper(t._1, p.expressions), t._2))
.toMap
val nestGroupedPartitions = groupedPartitions.map {
// SPARK-48949. Inert here: the `...v2.bucketing.partition.filter.enabled` config
// that makes `commonPartitionValues` an intersection instead of a union does not
// exist before Spark 4.0, so this filter can never drop a group on the versions
// this shim serves. Kept identical to the spark350db143 copy so the next SPJ
// audit diff between the two stays cheap.
val filteredGroupedPartitions = groupedPartitions.filter {
case (partValues, _) =>
commonPartValuesMap.keySet.contains(
InternalRowComparableWrapper(partValues, p.expressions))
}
val nestGroupedPartitions = filteredGroupedPartitions.map {
case (partValue, splits) =>
// `commonPartValuesMap` should contain the part value since it's the super set.
val numSplits = commonPartValuesMap
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,16 @@ case class GpuBatchScanExec(
.get
.map(t => (InternalRowComparableWrapper(t._1, partExpressions), t._2))
.toMap
val nestGroupedPartitions = finalGroupedPartitions.map { case (partValue, splits) =>
// SPARK-48949: with `...v2.bucketing.partition.filter.enabled`,
// `commonPartitionValues` is the intersection of the two join sides rather than
// their union, so this scan can still enumerate groups the planner pruned away.
// Dropping them here is what keeps the assert below true.
val filteredGroupedPartitions = finalGroupedPartitions.filter {
case (partValues, _) =>
commonPartValuesMap.keySet.contains(
InternalRowComparableWrapper(partValues, partExpressions))
}
val nestGroupedPartitions = filteredGroupedPartitions.map { case (partValue, splits) =>
// `commonPartValuesMap` should contain the part value since it's the super set.
val numSplits = commonPartValuesMap
.get(InternalRowComparableWrapper(partValue, partExpressions))
Expand Down
Loading