-
Notifications
You must be signed in to change notification settings - Fork 28.3k
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
[SPARK-45475][SQL] Uses DataFrame.foreachPartition instead of RDD.foreachPartition in JdbcUtils #43304
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
QQ: is there an E2E test for this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, LGTM.
Added a simple test |
Merged to master and branch-3.5. |
…eachPartition in JdbcUtils This PR is kind of a followup for #39976 that addresses #39976 (comment) comment. In order to probably assign the SQL execution ID so `df.observe` works with this. Yes. `df.observe` will work with JDBC connectors. Manually tested. Unit test was added. Closes #43304 from HyukjinKwon/foreachbatch. Authored-by: Hyukjin Kwon <gurwls223@apache.org> Signed-off-by: Hyukjin Kwon <gurwls223@apache.org> (cherry picked from commit 39cc4ab) Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM later.
@HyukjinKwon did this break the 3.5 build? I'm seeing an error building:
|
Oops, please feel free to revert directly. I'll make a PR tmr in my time. |
I reverted it. I will make a PR tmr. |
…D.foreachPartition in JdbcUtils This PR cherry-picks #43304 to branch-3.5 --- ### What changes were proposed in this pull request? This PR is kind of a followup for #39976 that addresses #39976 (comment) comment. ### Why are the changes needed? In order to probably assign the SQL execution ID so `df.observe` works with this. ### Does this PR introduce _any_ user-facing change? Yes. `df.observe` will work with JDBC connectors. ### How was this patch tested? Manually tested. ### Was this patch authored or co-authored using generative AI tooling? Unit test was added. Closes #43322 from HyukjinKwon/SPARK-45475-3.5. Authored-by: Hyukjin Kwon <gurwls223@apache.org> Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
What changes were proposed in this pull request?
This PR is kind of a followup for #39976 that addresses #39976 (comment) comment.
Why are the changes needed?
In order to probably assign the SQL execution ID so
df.observe
works with this.Does this PR introduce any user-facing change?
Yes.
df.observe
will work with JDBC connectors.How was this patch tested?
Manually tested.
Was this patch authored or co-authored using generative AI tooling?
Unit test was added.