[SPARK-56273][SQL] Simplify extracting fields from DataSourceV2ScanRelation by yyanyy · Pull Request #55070 · apache/spark

yyanyy · 2026-03-28T00:39:58Z

What changes were proposed in this pull request?

DataSourceV2ScanRelation is a case class with 5 fields. Many pattern match sites only need the scan field or a subset of (relation, scan, output), using wildcards for the rest. This couples every match site to the constructor arity, so adding or removing fields requires updating all of them.

This PR introduces two extractors following the ExtractV2Table precedent (SPARK-53720):

ExtractV2Scan: returns Scan
ExtractV2ScanRelation: returns (DataSourceV2Relation, Scan, Seq[AttributeReference])

Updated 14 pattern match sites across 10 files. Type-based matches and constructor calls are unchanged.

Why are the changes needed?

Code simplification. Similar to 6bae835

Does this PR introduce any user-facing change?

no

How was this patch tested?

This PR relies on existing tests.

Was this patch authored or co-authored using generative AI tooling?

Yes - Opus 4.6

szehon-ho

nice idea, just a few comments

szehon-ho · 2026-03-28T05:05:36Z

+ * An extractor that returns a 3-tuple of ([[DataSourceV2Relation]], [[Scan]],
+ * output attributes) from a [[DataSourceV2ScanRelation]].
+ *
+ * This is useful when a pattern match site needs the relation, scan, and output


same, should we remove?

szehon-ho · 2026-03-28T05:06:32Z

+ * This is useful when a pattern match site only needs the scan and should not be
+ * coupled to the full constructor signature of [[DataSourceV2ScanRelation]].
+ */
+object ExtractV2Scan {


nit: should we move it to the other Extractor for DSV2Relation below (like ExtractV2Table)

szehon-ho · 2026-03-28T05:06:56Z

+/**
+ * An extractor that returns the [[Scan]] from a [[DataSourceV2ScanRelation]].
+ *
+ * This is useful when a pattern match site only needs the scan and should not be


nit: should we remove this comment? (thinking, else why not put it on all the extractors?)

cloud-fan · 2026-03-30T08:07:29Z

+ */
+object ExtractV2ScanRelation {
+  def unapply(scanRelation: DataSourceV2ScanRelation)
+      : Option[(DataSourceV2Relation, Scan, Seq[AttributeReference])] =


ExtractV2Scan returns v2 scan, this returns DataSourceV2Relation, shall we name it ExtractV2Relation?

yyanyy · 2026-03-30T23:04:53Z

Thank you both for the quick review! I have updated the PR per comment, please take a look again when you have a chance

aokolnychyi · 2026-04-02T18:05:40Z

+    Some(scanRelation.scan)
+}
+
+object ExtractV2Relation {


We should still think a bit more about the name here. It does extract a relation but it does this in a scan context and the name doesn't say anything about that. I agree ExtractV2ScanRelation is also somewhat not accurate as we are not extracting it. I wonder if things like ExtractV2ScanInfo would be better.

Let me think. Name alternatives are welcome.

+1 to change the name to indicate it extract more than relation (sorry didnt reply earlier to #55070 (comment)) ExtractV2ScanComponents is another one, but maybe Info is better as its shorter

If we end up not extracting output, it could be something like: ExtractV2RelationAndScan.

The key ask: the name has to indicate it applies to scanning.

nit: do we have to include Extract in the name? might be cleaner w/o it (it's already assumed it is an extractor..). V2RelationAndScan / V2Relation / V2ScanContext

@anton5798, I had a similar question when we did extractors for DataSourceV2Relation but the consensus was that it is beneficial to have ExtractXXX explicitly.

I'm sure ExtractV2ScanRelationInfo has crossed some of your mind but it's probably too long?

I think I like ExtractV2ScanInfo the most and still include output, let me update the PR with this

aokolnychyi · 2026-04-02T19:15:30Z

+
+object ExtractV2Relation {
+  def unapply(scanRelation: DataSourceV2ScanRelation)
+      : Option[(DataSourceV2Relation, Scan, Seq[AttributeReference])] =


Is it common that we need output as well? Technically, you could do like this:

r @ ExtractV2Relation(_, scan: LocalScan) => r.output

3 of the 6 instances that requires ExtractV2Relation would need output but all usages are currently in DataSourceV2Strategy so I guess we can consider it either way...

aokolnychyi · 2026-04-02T19:18:57Z

@@ -258,7 +258,7 @@ class CollatedFilterPushDownToParquetV2Suite extends CollatedFilterPushDownToPar
  override def getPushedDownFilters(query: DataFrame): Seq[Filter] = {
    query.queryExecution.optimizedPlan.collectFirst {
      case PhysicalOperation(_, _,
-          DataSourceV2ScanRelation(_, scan: ParquetScan, _, _, _)) =>
+          ExtractV2Scan(scan: ParquetScan)) =>


Can this now be moved to the same line?

aokolnychyi · 2026-04-02T19:27:31Z

I support this idea as well.

…lation `DataSourceV2ScanRelation` is a case class with 5 fields. Many pattern match sites only need the `scan` field or a subset of `(relation, scan, output)`, using wildcards for the rest. This couples every match site to the constructor arity, so adding or removing fields requires updating all of them. This PR introduces two extractors following the `ExtractV2Table` precedent (SPARK-53720): - `ExtractV2Scan`: returns `Scan` - `ExtractV2ScanRelation`: returns `(DataSourceV2Relation, Scan, Seq[AttributeReference])` Updated 14 pattern match sites across 10 files. Type-based matches and constructor calls are unchanged.

…ractV2ScanRelation - Remove scaladoc comments from ExtractV2Scan and ExtractV2ScanRelation for consistency with other extractors (ExtractV2Table, etc.) - Move both extractors next to ExtractV2Table - Rename ExtractV2ScanRelation -> ExtractV2Relation per cloud-fan's suggestion

…ntifier Group extractors by the type they operate on: - DataSourceV2Relation extractors: ExtractV2Table, ExtractV2CatalogAndIdentifier - DataSourceV2ScanRelation extractors: ExtractV2Scan, ExtractV2Relation

…o, collapse single-line match

…+ OracleIntegrationSuite failures

cloud-fan · 2026-04-07T00:03:21Z

the failed test is unrelated, thanks, merging to master!

szehon-ho reviewed Mar 28, 2026

View reviewed changes

cloud-fan reviewed Mar 30, 2026

View reviewed changes

szehon-ho approved these changes Mar 31, 2026

View reviewed changes

aokolnychyi reviewed Apr 2, 2026

View reviewed changes

yyanyy added 4 commits April 2, 2026 21:40

Move DataSourceV2ScanRelation extractors after ExtractV2CatalogAndIde…

ea06939

…ntifier Group extractors by the type they operate on: - DataSourceV2Relation extractors: ExtractV2Table, ExtractV2CatalogAndIdentifier - DataSourceV2ScanRelation extractors: ExtractV2Scan, ExtractV2Relation

Address review feedback: rename ExtractV2Relation to ExtractV2ScanInf…

d57ee7c

…o, collapse single-line match

yyanyy force-pushed the extract-v2-scan-relation-extractors branch from 5937f3b to d57ee7c Compare April 2, 2026 21:50

aokolnychyi approved these changes Apr 3, 2026

View reviewed changes

Retrigger CI: flaky RocksDBStateStoreIntegrationSuiteWithRowChecksum …

84ce4cc

…+ OracleIntegrationSuite failures

cloud-fan closed this in af9c8b3 Apr 7, 2026

Conversation

yyanyy commented Mar 28, 2026

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

Uh oh!

szehon-ho left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yyanyy commented Mar 30, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anton5798 Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aokolnychyi commented Apr 2, 2026

Uh oh!

cloud-fan commented Apr 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

anton5798 Apr 2, 2026 •

edited

Loading