Skip to content

Commit 3f21010

Browse files
sezrubyclaude
andcommitted
[Spark] Add value-exact tier to conflict-time data skipping
Conflict-time data skipping (row-level concurrency) uses column min/max stats to decide whether a concurrently-added file could match a transaction's read predicates. Min/max cannot model predicates like `c % 2 = 1` or other non-range expressions, so a file whose stats range spans the predicate is kept as a conflict candidate even when none of its rows actually match, causing an unnecessary ConcurrentAppendException. Add an opt-in second tier: for the files min/max could not rule out, build one DataFrame over exactly those added files and evaluate the real read predicates (AND within a read, OR across reads). The caller only needs to know whether any candidate can still match -- an unpartitioned table collapses the survivors to headOption, and a partitioned table runs this before its partition filter -- so this is a single short-circuiting existence check: keep all candidates if any row matches, drop them all if none do. Gated by conflictDetection.dataSkipping.valueExact.enabled (default off). One-way safe (files are dropped only when a scan proves no row matches) and fail-safe (any error keeps all files). Stacked on delta-io#7358. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent e4ac0da commit 3f21010

3 files changed

Lines changed: 328 additions & 26 deletions

File tree

‎spark/src/main/scala/org/apache/spark/sql/delta/sources/DeltaSQLConf.scala‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -545,6 +545,20 @@ trait DeltaSQLConfBase extends DeltaSQLConfUtils {
545545
.booleanConf
546546
.createWithDefault(false)
547547

548+
val DELTA_CONFLICT_DETECTION_DATA_SKIPPING_VALUE_EXACT_ENABLED =
549+
buildConf("conflictDetection.dataSkipping.valueExact.enabled")
550+
.internal()
551+
.doc(
552+
"""When enabled (together with conflictDetection.dataSkipping.enabled), conflict detection
553+
|reads the actual data of concurrently-added files that column-statistics skipping could
554+
|not rule out, and excludes those whose rows do not actually match the current
555+
|transaction's read predicates. This resolves predicates min/max stats cannot skip (e.g.
556+
|modulo or other non-range expressions), at the cost of reading the (already stats-
557+
|narrowed) added files during commit. One-way safe: a file is excluded only when an actual
558+
|scan proves no row matches.""".stripMargin)
559+
.booleanConf
560+
.createWithDefault(false)
561+
548562
val DELTA_PROTOCOL_DEFAULT_WRITER_VERSION =
549563
buildConf("properties.defaults.minWriterVersion")
550564
.doc("The default writer protocol version to create new tables with, unless a feature " +

‎spark/src/main/scala/org/apache/spark/sql/delta/stats/ConflictDataSkippingReader.scala‎

Lines changed: 136 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ import org.apache.spark.sql.delta.metering.DeltaLogging
2626
import org.apache.spark.sql.delta.schema.SchemaUtils
2727
import org.apache.spark.sql.delta.sources.DeltaSQLConf
2828

29-
import org.apache.spark.sql.Column
29+
import org.apache.spark.SparkContext
30+
import org.apache.spark.sql.{Column, DataFrame}
3031
import org.apache.spark.sql.catalyst.expressions._
3132
import org.apache.spark.sql.functions.{col, from_json}
3233

@@ -62,6 +63,23 @@ trait ConflictDataSkippingReader extends DeltaLogging { self: DataSkippingReader
6263
}
6364
}
6465

66+
/**
67+
* Filters `dataFilters` to those safe to use for conflict-time skipping: no subqueries, fully
68+
* deterministic, no metadata attributes. Mirrors [[filesForScan]] eligibility so we never build a
69+
* predicate (stats or value-exact) that could wrongly exclude a matching file. Shared by the
70+
* stats tier ([[buildDataSkippingPredicate]]) and the value-exact tier
71+
* ([[filterFilesByValueExactScan]]).
72+
*/
73+
private def eligibleSkippingFilters(dataFilters: Seq[Expression]): Seq[Expression] = {
74+
import DeltaTableUtils._
75+
dataFilters.filterNot { f =>
76+
containsSubquery(f) || !f.deterministic || f.exists {
77+
case MetadataAttribute(_) => true
78+
case _ => false
79+
}
80+
}
81+
}
82+
6583
/**
6684
* Builds a single [[DataSkippingPredicate]] (skipping expression + the stats it references) from
6785
* `dataFilters`, mirroring [[filesForScan]]'s eligibility filtering, per-filter construction and
@@ -71,18 +89,10 @@ trait ConflictDataSkippingReader extends DeltaLogging { self: DataSkippingReader
7189
*/
7290
private[delta] def buildDataSkippingPredicate(
7391
dataFilters: Seq[Expression]): Option[DataSkippingPredicate] = {
74-
import DeltaTableUtils._
7592
// Stats-skipping conf, inlined from DataSkippingReaderBase.useStats (file-private, so not
7693
// visible here). Reading it inline avoids widening that member's visibility.
7794
if (!spark.sessionState.conf.getConf(DeltaSQLConf.DELTA_STATS_SKIPPING)) return None
78-
// Mirror filesForScan eligibility: drop subquery / non-deterministic / metadata filters, so we
79-
// never build a skipping predicate that could wrongly exclude a matching file.
80-
val eligibleFilters = dataFilters.filterNot { f =>
81-
containsSubquery(f) || !f.deterministic || f.exists {
82-
case MetadataAttribute(_) => true
83-
case _ => false
84-
}
85-
}
95+
val eligibleFilters = eligibleSkippingFilters(dataFilters)
8696
val constructDataFilters = new DataFiltersBuilder(
8797
spark = spark,
8898
dataSkippingType = DeltaDataSkippingType.dataSkippingOnlyV1,
@@ -112,26 +122,53 @@ trait ConflictDataSkippingReader extends DeltaLogging { self: DataSkippingReader
112122
/**
113123
* Conflict-detection helper (reader-side data skipping) over several INDEPENDENT reads: returns
114124
* the subset of `files` that could still match ANY read and therefore must be treated as
115-
* conflicts. Each inner `Seq[Expression]` is one logical read's data filters (AND-combined via
116-
* [[buildDataSkippingPredicate]]); the reads are OR-combined, matching read semantics -- a file
117-
* is a candidate if it could match any one of them.
125+
* conflicts. Each inner `Seq[Expression]` is one logical read's data filters (AND-combined); the
126+
* reads are OR-combined, matching read semantics -- a file is a candidate if it could match any
127+
* one of them.
128+
*
129+
* Two tiers, both one-way safe (a file is dropped only when it is *proven* to fail EVERY read, so
130+
* a real conflict is never a false negative) and both fail-safe (any error falls back to keeping
131+
* all `files`, since skipping is a pure optimization over correct conservative detection):
132+
* 1. [[filterFilesByStatsSkipping]] -- column min/max stats, no data I/O (always on with the
133+
* feature). Cannot resolve predicates min/max does not model (modulo, non-range exprs).
134+
* 2. [[filterFilesByValueExactScan]] -- opt-in via
135+
* [[DeltaSQLConf.DELTA_CONFLICT_DETECTION_DATA_SKIPPING_VALUE_EXACT_ENABLED]]: for the files
136+
* tier 1 could not rule out, reads their actual rows and evaluates the real predicates,
137+
* dropping any file with zero matching rows. Resolves the stats-inconclusive cases at the
138+
* cost of reading the (already narrowed) added files during commit.
139+
*/
140+
private[delta] def filterFilesMatchingAnyReadPredicate(
141+
files: Seq[AddFile],
142+
dataFiltersPerRead: Seq[Seq[Expression]]): Seq[AddFile] = {
143+
// Tier 1 (cheap, no data I/O): column min/max stats skipping.
144+
val statsSurvivors = filterFilesByStatsSkipping(files, dataFiltersPerRead)
145+
// Tier 2 (opt-in, reads data): for files stats could NOT rule out, evaluate the real read
146+
// predicates against their actual rows. Resolves predicates min/max cannot skip (e.g. modulo).
147+
if (statsSurvivors.isEmpty ||
148+
!spark.sessionState.conf.getConf(
149+
DeltaSQLConf.DELTA_CONFLICT_DETECTION_DATA_SKIPPING_VALUE_EXACT_ENABLED)) {
150+
statsSurvivors
151+
} else {
152+
filterFilesByValueExactScan(statsSurvivors, dataFiltersPerRead)
153+
}
154+
}
155+
156+
/**
157+
* Tier 1 -- column min/max stats skipping (see [[filterFilesMatchingAnyReadPredicate]]). Returns
158+
* the subset of `files` whose stats do NOT prove they fail every read.
118159
*
119160
* All reads are evaluated in a SINGLE Spark job: the per-read skipping predicates are OR-ed
120161
* together into one `where` clause, rather than filtering per read and unioning the survivors
121162
* (which launched one job per read). Note we cannot instead flatten every read's filters into one
122163
* predicate -- that would AND them (read1 AND read2), the opposite of the OR we need.
123164
*
124165
* One-way safe: each read contributes `expr || !verifyStatsForFilter(...)` exactly as
125-
* [[getDataSkippedFiles]], so a file is dropped only when its stats *prove* it fails EVERY read
126-
* -- a real conflict is never a false negative. Files with missing/insufficient stats, a
127-
* read with no usable skipping predicate (empty / ineligible filters -> matches everything), or a
128-
* table without stats are all kept. Returns the original [[AddFile]]s (matched by path).
129-
*
130-
* Fail-safe: skipping here is a pure optimization over correct (conservative) conflict detection,
131-
* so if building or evaluating the predicate throws we fall back to the default behavior of
132-
* keeping all `files` as conflict candidates rather than failing the commit.
166+
* [[getDataSkippedFiles]], so a file is dropped only when its stats *prove* it fails EVERY read.
167+
* Files with missing/insufficient stats, a read with no usable skipping predicate (empty /
168+
* ineligible filters -> matches everything), or a table without stats are all kept. Returns the
169+
* original [[AddFile]]s (matched by path). Any failure falls back to keeping all `files`.
133170
*/
134-
private[delta] def filterFilesMatchingAnyReadPredicate(
171+
private def filterFilesByStatsSkipping(
135172
files: Seq[AddFile],
136173
dataFiltersPerRead: Seq[Seq[Expression]]): Seq[AddFile] = {
137174
import org.apache.spark.sql.delta.implicits._
@@ -169,4 +206,81 @@ trait ConflictDataSkippingReader extends DeltaLogging { self: DataSkippingReader
169206
files
170207
}
171208
}
209+
210+
/**
211+
* Tier 2 -- value-exact scan (see [[filterFilesMatchingAnyReadPredicate]]). For the `files` that
212+
* tier 1 stats skipping could not rule out, read their ACTUAL data and evaluate the real read
213+
* predicates, keeping only files that genuinely have >= 1 matching row. This resolves predicates
214+
* column min/max cannot skip (e.g. `a % 2 = 1`, other non-range expressions): a file whose stats
215+
* range spans the predicate but whose values never satisfy it is dropped here, avoiding an
216+
* unnecessary conflict.
217+
*
218+
* Reads are OR-combined, AND within a read -- the same semantics tier 1 approximates. A read with
219+
* no eligible (deterministic, subquery-free, non-metadata) filter matches everything, so we
220+
* cannot prove non-match and keep all `files`. Using only the eligible filters is a sound
221+
* over-approximation of the true read predicate (dropping a conjunct only makes it match more),
222+
* so a real conflict is never a false negative.
223+
*
224+
* Runs a SINGLE, short-circuiting Spark job over the (already stats-narrowed) files: does ANY of
225+
* them still hold a row matching the read predicates? If so, keep them ALL as conflict
226+
* candidates; if not, drop them all. That is exactly what the caller
227+
* ([[org.apache.spark.sql.delta.ConflictChecker]]) consumes -- an unpartitioned table collapses
228+
* the survivors to `headOption`, and a partitioned table runs this before its partition filter,
229+
* so in both cases only "can anything still match?" matters, never which file. Deliberately an
230+
* existence check rather than per-file attribution: we never map a scanned row back to its
231+
* [[AddFile]], so there is no `_metadata.file_path` distinct/collect and no chance that a
232+
* path-canonicalization mismatch drops a genuinely-matching file (which would be a missed
233+
* conflict). One-way safe: dropping all files is sound only because the scan proved no candidate
234+
* has a matching row. Any failure falls back to keeping all `files`.
235+
*/
236+
private[delta] def filterFilesByValueExactScan(
237+
files: Seq[AddFile],
238+
dataFiltersPerRead: Seq[Seq[Expression]]): Seq[AddFile] = {
239+
if (files.isEmpty || dataFiltersPerRead.isEmpty || schema.isEmpty) return files
240+
try {
241+
// AND within a read over its eligible filters; a read with none matches everything, so we
242+
// cannot prove any file fails it -> keep all files (no job).
243+
val perReadEligible = dataFiltersPerRead.map(eligibleSkippingFilters)
244+
if (perReadEligible.exists(_.isEmpty)) return files
245+
val snapshot = snapshotToScan
246+
val df = snapshot.deltaLog.createDataFrame(
247+
snapshot, files, actionTypeOpt = Some("conflictDetectionValueExact"))
248+
// Rebind each filter's attributes to the fresh DataFrame's columns by name, AND within a
249+
// read, OR across reads -- a file matches if any read's predicate matches any of its rows.
250+
val condition = perReadEligible
251+
.map(filters => filters.map(f => rebindToDataFrame(f, df)).reduce(_ && _))
252+
.reduce(_ || _)
253+
// The caller only needs a boolean (see the scaladoc), so run one short-circuiting existence
254+
// check instead of attributing rows back to files: keep all candidates if any row matches,
255+
// else drop them all.
256+
val sc = spark.sparkContext
257+
val prevJobDesc = sc.getLocalProperty(SparkContext.SPARK_JOB_DESCRIPTION)
258+
val anyRowMatches =
259+
try {
260+
sc.setJobDescription("Delta conflict detection: value-exact data skipping")
261+
recordFrameProfile("Delta", "DataSkippingReader.filterFilesByValueExactScan") {
262+
!df.where(condition).isEmpty
263+
}
264+
} finally {
265+
sc.setJobDescription(prevJobDesc)
266+
}
267+
if (anyRowMatches) files else Nil
268+
} catch {
269+
case NonFatal(e) =>
270+
// Optimization only: never let a value-exact scan failure abort a commit. Fall back to
271+
// keeping every stats-surviving file as a conflict candidate.
272+
logWarning(log"Conflict-time value-exact scan failed to evaluate; falling back to " +
273+
log"keeping all stats-surviving files as conflict candidates", e)
274+
files
275+
}
276+
}
277+
278+
/**
279+
* Rebinds `e`'s attribute references to `df`'s output columns by name, so a read predicate
280+
* resolved against the transaction's plan can be evaluated on a freshly built DataFrame. Nested
281+
* accesses (`GetStructField` over a top-level struct column) are preserved. If a referenced name
282+
* is not a column of `df`, `df.col` throws and the caller's fail-safe keeps all files.
283+
*/
284+
private def rebindToDataFrame(e: Expression, df: DataFrame): Column =
285+
Column(e.transform { case a: AttributeReference => df.col(a.name).expr })
172286
}

0 commit comments

Comments
 (0)