Skip to content

Spark: Support variant_get predicate pushdown for file skipping - #15385

Draft
qlong wants to merge 2 commits into
apache:mainfrom
qlong:variant-file-skipping-sparkv2filters
Draft

Spark: Support variant_get predicate pushdown for file skipping#15385
qlong wants to merge 2 commits into
apache:mainfrom
qlong:variant-file-skipping-sparkv2filters

Conversation

@qlong

@qlong qlong commented Feb 20, 2026 •
edited
Loading

Copy link
Copy Markdown

This is to support manifest-based file skipping for variant columns.

Changes:

  • SparkV2Filters: Convert new connector expression VariantGet to Expressions.extract()
  • Spark3Util.describe: Output extract terms as variant_get() for EXPLAIN

Tests:

  • Added unit tests

Do not merge this PR until the following two are merged:

  1. Api: Support variant extract and fix manifest bounds byte order #15384
  2. #54394, introuduced a new connector expression VariantGet and pushdown.

Related issue:

  1. Variant Data Type Support #10392

CI status (expected until #15384)

CI fails because dependency PRs are not merged. This PR will stay in Draft until that those merged.

@github-actions github-actions Bot added the spark label Feb 20, 2026
@qlong qlong mentioned this pull request Feb 20, 2026
Closed
@huaxingao

huaxingao commented Feb 25, 2026

Copy link
Copy Markdown
Contributor

cc @aihuaxu

This was referenced Feb 28, 2026
Open
Open
@github-actions

github-actions Bot commented Mar 31, 2026

Copy link
Copy Markdown

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.

@github-actions github-actions Bot added the stale label Mar 31, 2026
@steveloughran

steveloughran commented Mar 31, 2026

Copy link
Copy Markdown
Contributor

Not stale! we need this! queries on variants are pretty bad right now, and skipping files can start to recover that.

@github-actions github-actions Bot removed the stale label Apr 1, 2026
@steveloughran

steveloughran commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

This is why my benchmarking of rowgroup filtering aren't working...needed for the variant_get() passthrough

nssalian
nssalian reviewed Apr 30, 2026
View reviewed changes

@nssalian nssalian left a comment

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.

Mostly looks good otherwise. Left a nit comment. @huaxingao @aihuaxu PTAL

Comment thread spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/SparkV2Filters.java
@steveloughran steveloughran mentioned this pull request Apr 30, 2026
Open
3 tasks
@steveloughran

steveloughran commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

i'm going to pull this into my rowgroup filtering pr as

  1. it's needed
  2. that's actually how this work is validated

steveloughran added a commit to steveloughran/iceberg that referenced this pull request Apr 30, 2026
@steveloughran
apache#15385 Spark: Support variant_get predicate pushdown for file s…
9b92fb8 
…kipping

copilot's solution to why pushdown wasn't working, independent of
qlong's apache#15385

I plan to take qlong's and pull what is extra from this one.
steveloughran added a commit to steveloughran/iceberg that referenced this pull request Apr 30, 2026
@steveloughran
Combine with apache#15385 changes; tests are main diff.
2098c8e
steveloughran added a commit to steveloughran/iceberg that referenced this pull request May 11, 2026
@steveloughran
apache#15385 Spark: Support variant_get predicate pushdown for file s…
d26a9dc 
…kipping

copilot's solution to why pushdown wasn't working, independent of
qlong's apache#15385

I plan to take qlong's and pull what is extra from this one.
steveloughran added a commit to steveloughran/iceberg that referenced this pull request May 11, 2026
@steveloughran
Combine with apache#15385 changes; tests are main diff.
b698c26
steveloughran added a commit to steveloughran/iceberg that referenced this pull request May 14, 2026
@steveloughran
apache#15385 Spark: Support variant_get predicate pushdown for file s…
2e77f60 
…kipping

copilot's solution to why pushdown wasn't working, independent of
qlong's apache#15385

I plan to take qlong's and pull what is extra from this one.
@qlong
qlong force-pushed the variant-file-skipping-sparkv2filters branch 2 times, most recently from a1bf0b6 to e926014 Compare May 18, 2026 18:45
@qlong
qlong force-pushed the variant-file-skipping-sparkv2filters branch from e926014 to 044867f Compare May 19, 2026 23:08
@soumilshah1995

soumilshah1995 commented May 19, 2026

Copy link
Copy Markdown

Sharing a small test I completed on my local laptop.
I pulled the branch, built the JARs, and then ran the build locally.

image

Although this was done on my local machine with a small dataset, I will run tests on ~400GB of data next. I do see improvements in the results so far.

@qlong
qlong force-pushed the variant-file-skipping-sparkv2filters branch from 044867f to 2fa7c02 Compare May 20, 2026 14:32
@steveloughran

steveloughran commented May 20, 2026

Copy link
Copy Markdown
Contributor

FYI I have evidence of this working in my latest benchmark runs.

@soumilshah1995

soumilshah1995 commented May 20, 2026 •
edited
Loading

Copy link
Copy Markdown

FYI I have evidence of this working in my latest benchmark runs.

can you share them links please ? @qlong steveloughran

@qlong

qlong commented May 20, 2026

Copy link
Copy Markdown
Author

@soumilshah1995 i verified file skipping by looking at spark UI. If you have any shredded columns, the performance could be worse compared to unshredded or plain json. See #16448

file_skipping

@qlong qlong mentioned this pull request May 20, 2026
Open
3 tasks
@soumilshah1995

soumilshah1995 commented May 20, 2026

Copy link
Copy Markdown

@soumilshah1995 i verified file skipping by looking at spark UI. If you have any shredded columns, the performance could be worse compared to unshredded or plain json. See #16448

file_skipping

Yes you are correct my test says same unshredded still perform better then shredding

@qlong
qlong force-pushed the variant-file-skipping-sparkv2filters branch 3 times, most recently from 0701d8e to b5eeafc Compare May 21, 2026 01:49
@github-actions github-actions Bot removed the API label May 21, 2026
@qlong
qlong force-pushed the variant-file-skipping-sparkv2filters branch from b5eeafc to bfa668d Compare May 21, 2026 03:30
@qlong
Spark: Support variant_get predicate pushdown for file skipping
c12ae73 
- SparkV2Filters: Convert variant_get/try_variant_get to
  Expressions.extract()
- Spark3Util.describe: Output extract terms as variant_get() for EXPLAIN
- Add tests for both

Depends on Spark PR:
- apache#15384
- apache/spark#54394
@qlong
qlong force-pushed the variant-file-skipping-sparkv2filters branch from bfa668d to c12ae73 Compare May 21, 2026 03:41
steveloughran added a commit to steveloughran/iceberg that referenced this pull request May 21, 2026
@steveloughran
apache#15385 Spark: Support variant_get predicate pushdown for file s…
45b4a92 
…kipping

copilot's solution to why pushdown wasn't working, independent of
qlong's apache#15385

I plan to take qlong's and pull what is extra from this one.
@qlong
qlong marked this pull request as draft June 12, 2026 04:07
@qlong
Spark: Use typed VariantGet connector expression in SparkV2Filters
74e0c9d 
Replace UserDefinedScalarFunc name-matching with instanceof VariantGet
for variant_get/try_variant_get predicate pushdown in SparkV2Filters.

Spark PR #54394 (SPARK-55617) changed from using a UDSF to a dedicated
org.apache.spark.sql.connector.expressions.VariantGet connector class
that carries structured fields (child, path, targetType, failOnError,
timeZoneId). Changes:
- Replace isVariantGetFunc (UDSF name-matching) with isVariantGetExpr
  (instanceof VariantGet)
- Add a variantGetToTerm method that reads typed fields directly from
  VariantGet instead of parsing string literals from UDSF children
- Remove the variant_get/try_variant_get branch from udfToTerm
- Update TestSparkV2Filters to construct VariantGet directly instead
  of building UserDefinedScalarFunc with encoded string children

Note: The new connector class introduces a compile-time dependency on
Spark. This change requires Spark PR #54394 to be merged and released
before this PR can be merged.
@qlong

qlong commented Jun 12, 2026

Copy link
Copy Markdown
Author

Switch to draft, as apache/spark#54394 switched to connector expression for UDSF. That introduced hard compile time dependence.

@qlong

qlong commented Jun 18, 2026

Copy link
Copy Markdown
Author

apache/spark#54394 merged into Spark 4.x. Will publish this when Spark 4.3 is released

@qlong qlong mentioned this pull request Jun 21, 2026
Open
@github-actions

github-actions Bot commented Jul 19, 2026

Copy link
Copy Markdown

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.

@github-actions github-actions Bot added the stale label Jul 19, 2026
@qlong

qlong commented Jul 19, 2026

Copy link
Copy Markdown
Author

This is needed, waiting for new api spark 4.3 https://issues.apache.org/jira/browse/SPARK-55617

@github-actions github-actions Bot removed the stale label Jul 20, 2026
@qlong

qlong commented Aug 4, 2026

Copy link
Copy Markdown
Author

confirmed the change is in Spark 4.3 branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@nssalian nssalian nssalian left review comments
Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

core spark

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@qlong @huaxingao @steveloughran @soumilshah1995 @nssalian