Skip to content

fix(spark): create QueryExecution via SessionState.executePlan - #1432

Merged
bestbeforetoday merged 1 commit into
substrait-io:mainfrom
andrew-coleman:qe_constructor
Oct 27, 2026
Merged

bestbeforetoday merged 1 commit into
substrait-io:mainfrom
andrew-coleman:qe_constructor

Conversation

@andrew-coleman

Copy link
Copy Markdown
Member

Constructing QueryExecution directly binds to Apache Spark's exact constructor signature. Vendor builds such as the Databricks Runtime change that signature, so conversion fails there with NoSuchMethodError.
SessionState.executePlan is the entry point Spark itself uses (Dataset.ofRows), and it leaves the constructor to the session. The change is applied to all three variants so vendor builds based on Spark 3.x are covered too.

Constructing `QueryExecution` directly binds to Apache
Spark's exact constructor signature. Vendor builds such as
 the Databricks Runtime change that signature, so
 conversion fails there with `NoSuchMethodError`.
`SessionState.executePlan` is the entry point Spark itself
uses (`Dataset.ofRows`), and it leaves the constructor to
the session. The change is applied to all three variants so
vendor builds based on Spark 3.x are covered too.
@coderabbitai

coderabbitai Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between d35cc52 and 21228ee.

Files selected for processing: 3
  • spark/src/main/spark-3.4/io/substrait/spark/compat/SparkCompatImpl.scala
  • spark/src/main/spark-3.5/io/substrait/spark/compat/SparkCompatImpl.scala
  • spark/src/main/spark-4.0/io/substrait/spark/compat/SparkCompatImpl.scala

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

📝 Summary

Summary by CodeRabbit

  • Refactor
    • Query execution now uses Spark’s session-managed planning flow across supported Spark versions. This provides consistent handling across versions while preserving the existing public interface and expected query behavior. No user-facing features or changes to query results are described in this update.

The Spark 3.4, 3.5, and 4.0 compatibility implementations now create query executions through sessionState.executePlan(plan) instead of directly constructing QueryExecution.

Changes: Spark query execution

Layer / File(s) Summary
Delegate query execution to session state
spark/src/main/spark-*/io/substrait/spark/compat/SparkCompatImpl.scala
Each Spark compatibility implementation calls sessionState.executePlan(plan) instead of directly constructing QueryExecution.

Priority: Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: Minimal · up to 21228

Query executions for Spark 3.4, 3.5 and 4.0 are now created through Spark's standard planning entry point, which should avoid constructor-signature mismatches on vendor Spark builds. No remaining issue blocks merging.

Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title clearly describes the primary change and follows the required Conventional Commit format: fix(spark): create QueryExecution via SessionState.executePlan.
Description check Passed The description provides a clear rationale, identifies the vendor-build compatibility issue, explains the replacement API, and states that all three Spark variants are updated. It satisfies the reposi…
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within survi…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description provides a clear rationale, identifies the vendor-build compatibility issue, explains the replacement API, and states that all three Spark variants are updated. It satisfies the repository template.

Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 0 files. (3 skipped: 3 unsupported.)

Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@bestbeforetoday bestbeforetoday left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good.

@bestbeforetoday
bestbeforetoday merged commit ac2ee60 into substrait-io:main Oct 27, 2026
14 checks passed
@andrew-coleman
andrew-coleman deleted the qe_constructor branch October 27, 2026 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants