gimgit commented on code in PR #17504:
URL: https://github.com/apache/iceberg/pull/17504#discussion_r3835997183
##########
spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/actions/SparkShufflingDataRewritePlanner.java:
##########
@@ -46,7 +52,24 @@ class SparkShufflingDataRewritePlanner extends
BinPackRewriteFilePlanner {
public static final double COMPRESSION_FACTOR_DEFAULT = 1.0;
+ /**
+ * The overlap depth at which files are selected for rewrite regardless of
their size.
+ *
+ * <p>Size based selection cannot detect files that are large enough but
whose sort key ranges sit
+ * on top of each other, so a sorted rewrite may report success without
improving clustering. When
+ * this option is set, files that intersect a region of the table sort key
covered by at least
+ * this many files of the same partition are added to the rewrite, on top of
the files selected by
+ * size and by deletes.
+ *
+ * <p>The overlap is measured on the table sort order using data file bounds
only, as reported by
+ * the {@code compute_sort_order_stats} procedure. Because column bounds may
be truncated, the
+ * measured depth is an upper-bound estimate. Unset by default, which leaves
selection unchanged.
+ */
+ public static final String MIN_OVERLAP_DEPTH = "min-overlap-depth";
Review Comment:
Done in 30ab350 — `min-overlap-depth` with zorder is now rejected in
`validateAndInitOptions` with an explicit error, and the option is documented
as sort-only.
##########
spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/procedures/ComputeSortOrderStatsProcedure.java:
##########
@@ -0,0 +1,135 @@
+/*
+ * 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.
+ */
+package org.apache.iceberg.spark.procedures;
+
+import java.util.Iterator;
+import java.util.List;
+import org.apache.iceberg.PartitionSpec;
+import org.apache.iceberg.SortOrderStatsHandler;
+import org.apache.iceberg.SortOrderStatsHandler.PartitionOverlapStats;
+import org.apache.iceberg.spark.procedures.SparkProcedures.ProcedureBuilder;
+import org.apache.spark.sql.catalyst.InternalRow;
+import org.apache.spark.sql.connector.catalog.Identifier;
+import org.apache.spark.sql.connector.catalog.TableCatalog;
+import org.apache.spark.sql.connector.catalog.procedures.BoundProcedure;
+import org.apache.spark.sql.connector.catalog.procedures.ProcedureParameter;
+import org.apache.spark.sql.connector.read.Scan;
+import org.apache.spark.sql.types.DataTypes;
+import org.apache.spark.sql.types.Metadata;
+import org.apache.spark.sql.types.StructField;
+import org.apache.spark.sql.types.StructType;
+import org.apache.spark.unsafe.types.UTF8String;
+
+/**
+ * A procedure that computes per-partition file overlap statistics on the
table's sort order,
+ * reading only data file metadata (column bounds) — no data files are opened
and nothing is
+ * committed to the table.
+ *
+ * <p>The maximum overlap depth of a partition is the largest number of data
files whose sort-key
+ * ranges cover a single point. A value of 1 means the partition is perfectly
clustered on the first
+ * sort field; values close to the file count mean sorting has not
materialized in the file layout.
+ *
+ * @see SortOrderStatsHandler
+ */
+public class ComputeSortOrderStatsProcedure extends BaseProcedure {
Review Comment:
Done in 30ab350. The separate procedure is gone; `report-only` now lives on
`rewrite_data_files` with the fixed superset schema from the earlier comment —
normal mode returns the single summary row as before (overlap columns null),
report-only returns one row per partition and commits nothing (tested). With
`min-overlap-depth` also set, `candidate_file_count`/`candidate_bytes` show
what a rewrite at that threshold would select, and the PR surface is smaller
than the procedure version.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]