[SPARK-58223][SQL] Fix SparkSession.Builder.classic() building a Connect session#57379
Closed
LuciferYang wants to merge 1 commit into
Closed
[SPARK-58223][SQL] Fix SparkSession.Builder.classic() building a Connect session#57379LuciferYang wants to merge 1 commit into
LuciferYang wants to merge 1 commit into
Conversation
…ect session `SparkSession.Builder.classic()` passed `CONNECT_COMPANION` to `mode(...)`, identical to `connect()`, so `SparkSession.builder().classic().getOrCreate()` built a Connect session instead of a Classic one on a classpath with both implementations. Change `classic()` to pass `CLASSIC_COMPANION`. The `classic()` / `connect()` mode selectors had no test coverage, which is why this went unnoticed since SPARK-49700. Add a shared binding test exercised by both the Classic and Connect concrete suites.
HyukjinKwon
approved these changes
Jul 20, 2026
LuciferYang
added a commit
that referenced
this pull request
Jul 21, 2026
…ect session ### What changes were proposed in this pull request? `SparkSession.Builder.classic()` passed `CONNECT_COMPANION` to `mode(...)`, identical to `connect()`: ```scala def classic(): this.type = mode(CONNECT_COMPANION) def connect(): this.type = mode(CONNECT_COMPANION) ``` `companion` drives `builder()` (`companion.builder()`), so on a classpath with both implementations `SparkSession.builder().classic().getOrCreate()` built a Connect session rather than a Classic one. This changes `classic()` to pass `CLASSIC_COMPANION` (which is already defined and used by the config-key path `API_MODE_CLASSIC => CLASSIC_COMPANION`). It also adds regression coverage: `SparkSessionBuilderImplementationBindingSuite` gains a shared `implementationSpecificBuilder` hook and a test asserting the built session is in the expected package, exercised by both the Classic (`.classic()`) and Connect (`.connect()`) concrete suites -- the mode selectors previously had no coverage. ### Why are the changes needed? `builder().classic()` does not honor its contract: it returns a Connect session (or fails, if only the classic implementation is available), contradicting the method name and scaladoc. Introduced by SPARK-49700; present since 4.1.0. ### Does this PR introduce _any_ user-facing change? Yes. `SparkSession.builder().classic().getOrCreate()` now builds a Classic session as documented, instead of a Connect session. This is a bug fix relative to released 4.1.x. ### How was this patch tested? New shared test `SPARK-58223: mode selector binds to the correct implementation` in `SparkSessionBuilderImplementationBindingSuite`, run by both the Classic and Connect suites. Verified reproduce-then-fix: the Classic case fails on the unfixed tree (the built session is not in `org.apache.spark.sql.classic`) and passes after the fix; the classic suite passes. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 4.8) Closes #57379 from LuciferYang/SPARK-classic-builder. Authored-by: YangJie <yangjie01@baidu.com> Signed-off-by: yangjie01 <yangjie01@baidu.com> (cherry picked from commit 55e386d) Signed-off-by: yangjie01 <yangjie01@baidu.com>
LuciferYang
added a commit
that referenced
this pull request
Jul 21, 2026
…ect session ### What changes were proposed in this pull request? `SparkSession.Builder.classic()` passed `CONNECT_COMPANION` to `mode(...)`, identical to `connect()`: ```scala def classic(): this.type = mode(CONNECT_COMPANION) def connect(): this.type = mode(CONNECT_COMPANION) ``` `companion` drives `builder()` (`companion.builder()`), so on a classpath with both implementations `SparkSession.builder().classic().getOrCreate()` built a Connect session rather than a Classic one. This changes `classic()` to pass `CLASSIC_COMPANION` (which is already defined and used by the config-key path `API_MODE_CLASSIC => CLASSIC_COMPANION`). It also adds regression coverage: `SparkSessionBuilderImplementationBindingSuite` gains a shared `implementationSpecificBuilder` hook and a test asserting the built session is in the expected package, exercised by both the Classic (`.classic()`) and Connect (`.connect()`) concrete suites -- the mode selectors previously had no coverage. ### Why are the changes needed? `builder().classic()` does not honor its contract: it returns a Connect session (or fails, if only the classic implementation is available), contradicting the method name and scaladoc. Introduced by SPARK-49700; present since 4.1.0. ### Does this PR introduce _any_ user-facing change? Yes. `SparkSession.builder().classic().getOrCreate()` now builds a Classic session as documented, instead of a Connect session. This is a bug fix relative to released 4.1.x. ### How was this patch tested? New shared test `SPARK-58223: mode selector binds to the correct implementation` in `SparkSessionBuilderImplementationBindingSuite`, run by both the Classic and Connect suites. Verified reproduce-then-fix: the Classic case fails on the unfixed tree (the built session is not in `org.apache.spark.sql.classic`) and passes after the fix; the classic suite passes. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 4.8) Closes #57379 from LuciferYang/SPARK-classic-builder. Authored-by: YangJie <yangjie01@baidu.com> Signed-off-by: yangjie01 <yangjie01@baidu.com> (cherry picked from commit 55e386d) Signed-off-by: yangjie01 <yangjie01@baidu.com>
Contributor
Author
Contributor
Author
|
Thank you @HyukjinKwon |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
SparkSession.Builder.classic()passedCONNECT_COMPANIONtomode(...), identical toconnect():companiondrivesbuilder()(companion.builder()), so on a classpath with both implementationsSparkSession.builder().classic().getOrCreate()built a Connect session rather than a Classic one. This changesclassic()to passCLASSIC_COMPANION(which is already defined and used by the config-key pathAPI_MODE_CLASSIC => CLASSIC_COMPANION).It also adds regression coverage:
SparkSessionBuilderImplementationBindingSuitegains a sharedimplementationSpecificBuilderhook and a test asserting the built session is in the expected package, exercised by both the Classic (.classic()) and Connect (.connect()) concrete suites -- the mode selectors previously had no coverage.Why are the changes needed?
builder().classic()does not honor its contract: it returns a Connect session (or fails, if only the classic implementation is available), contradicting the method name and scaladoc. Introduced by SPARK-49700; present since 4.1.0.Does this PR introduce any user-facing change?
Yes.
SparkSession.builder().classic().getOrCreate()now builds a Classic session as documented, instead of a Connect session. This is a bug fix relative to released 4.1.x.How was this patch tested?
New shared test
SPARK-58223: mode selector binds to the correct implementationinSparkSessionBuilderImplementationBindingSuite, run by both the Classic and Connect suites. Verified reproduce-then-fix: the Classic case fails on the unfixed tree (the built session is not inorg.apache.spark.sql.classic) and passes after the fix; the classic suite passes.Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.8)