Skip to content

[SPARK-58223][SQL][4.1] Fix SparkSession.Builder.classic() building a Connect session#57391

Open
LuciferYang wants to merge 1 commit into
apache:branch-4.1from
LuciferYang:SPARK-58223-branch-4.1
Open

[SPARK-58223][SQL][4.1] Fix SparkSession.Builder.classic() building a Connect session#57391
LuciferYang wants to merge 1 commit into
apache:branch-4.1from
LuciferYang:SPARK-58223-branch-4.1

Conversation

@LuciferYang

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

SparkSession.Builder.classic() passed CONNECT_COMPANION to mode(...), identical to connect():

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)

…ect session

`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.

`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.

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.

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.

Generated-by: Claude Code (Opus 4.8)

Closes apache#57379 from LuciferYang/SPARK-classic-builder.

Authored-by: YangJie <yangjie01@baidu.com>
Signed-off-by: yangjie01 <yangjie01@baidu.com>
(cherry picked from commit 55e386d)

@uros-b uros-b 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.

Thank you @LuciferYang! cc @HyukjinKwon

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