Skip to content

[SPARK-53104][PS] Introduce ansi_mode_context to avoid multiple config checks per API call #51821

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

ueshin
Copy link
Member

@ueshin ueshin commented Aug 4, 2025

What changes were proposed in this pull request?

Introduces ansi_mode_context to avoid multiple config checks per API call.

Why are the changes needed?

Currently is_ansi_mode_enabled is called many times, which could cause a performance issue as it retrieves the related config every time it's called.

For example:

many_columns = ps.DataFrame({f"col_{i}": [i] for i in range(500)})
many_columns + 1

will cause the roundtrip between the client and the server 500 times. To be exact, classic needs two roundtrips to check configs, so the number is 1000 times.

We should reduce the number of retrievals to at most once (or twice in classic) per API call.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

The existing tests should pass, also manually checked the number of config retrivals.

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

No.

@ueshin ueshin requested a review from xinrong-meng August 4, 2025 20:59
@ueshin ueshin changed the title [SPARK-53104][PS} Introduce ansi_mode_context to avoid multiple config checks per API call [SPARK-53104][PS] Introduce ansi_mode_context to avoid multiple config checks per API call Aug 4, 2025
@ueshin ueshin requested a review from HyukjinKwon August 4, 2025 21:23
@xinrong-meng
Copy link
Member

The PR looks good. I think docstring on how to use is_ansi_mode_enabled and ansi_mode_context would be helpful. I can follow up with that.

@ueshin
Copy link
Member Author

ueshin commented Aug 5, 2025

Thanks! merging to master.

@ueshin ueshin closed this in af4e1d9 Aug 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants