[SPARK-54581][SQL] Making fetchsize option case-insensitive for Postgres connector #53308
+77
−2
Conversation
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
marko-sisovic-db
force-pushed
the
msisovic/postgres-fetchsize-fix
branch
from
d32f1e9 to
56294cb
Compare
dongjoon-hyun
requested changes
Dec 4, 2025
| * @param connection The connection object | ||
| * @param properties The connection properties. This is passed through from the relation. | ||
| */ | ||
| @deprecated("Use beforeFetch(Connection, JDBCOptions) instead", "4.0.0") |
Member
There was a problem hiding this comment.
To @marko-sisovic-db , since Apache Spark 4.0.0 and 4.0.1 are released already, this is wrong.
Author
There was a problem hiding this comment.
Thanks, changed to 4.2.0.
dongjoon-hyun
changed the title
cloud-fan
reviewed
Dec 4, 2025
| * @param connection The connection object | ||
| * @param properties The connection properties. This is passed through from the relation. | ||
| */ | ||
| @deprecated("Use beforeFetch(Connection, JDBCOptions) instead", "4.0.0") |
Contributor
There was a problem hiding this comment.
This should be 4.1.0 or 4.2.0, depending on whether we want this bug fix in 4.1 or not. cc @dongjoon-hyun for the decision.
Member
There was a problem hiding this comment.
+1 for 4.2.0 because 4.1.0 already has several pending decisions. Thank you!
Author
There was a problem hiding this comment.
Thanks folks, changed to 4.2.0.
Contributor
|
@marko-sisovic-db can you update the PR description to match the actual change? |
cloud-fan
approved these changes
Dec 5, 2025
Contributor
|
@marko-sisovic-db can you re-trigger the failed CI jobs? seems flaky |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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?
Making new API for
beforeFetchinJDBCDialect, where it accepts options asJDBCOptionsinstead ofMap[String, String], and deprecating the old API starting from Spark version4.2.0. Spark docs state that options are case insensitive, so this will make it easier for dialects to respect that. Even if we have some edge case where we need the original casing, we can access the original map inside theJDBCOptionsobject.Why are the changes needed?
The option
fetchsizerequires another option,autocommitto be set tofalsefor the Postgres connector. We have logic for this:spark/sql/core/src/main/scala/org/apache/spark/sql/jdbc/PostgresDialect.scala
Lines 203 to 205 in 415f505
However, this logic is case-sensitive, and will only work for lowercased
fetchsize. When passingfetchSizefor example, the correct value for the fetchsize will be set on the Postgres driver, but it won't haveautocommit -> false, so the fetch size will be ignored.Does this PR introduce any user-facing change?
No.
How was this patch tested?
New test:
PostgresDialectSuite.Was this patch authored or co-authored using generative AI tooling?
Yes, partly generated-by: claude code.