Skip to content
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

SNOW-1820719 Refactor Session.sql Function #175

Merged
merged 2 commits into from
Nov 22, 2024
Merged

SNOW-1820719 Refactor Session.sql Function #175

merged 2 commits into from
Nov 22, 2024

Conversation

sfc-gh-bli
Copy link
Collaborator

We found a regression in the release tests. It is a legacy issue, Java Sproc can invoke Scala session. However, when Java code invokes Scala functions, the default value of function arguments will be ignored. Therefore, the new change in the Session.sql will break all existing code related this function.
This PR refactor the Session.sql function to split it to two functions, with and without parameters. Then the new API will not impact any existing code.

@@ -65,7 +65,7 @@ public static SessionBuilder builder() {
* @since 0.8.0
*/
public DataFrame sql(String query) {
return new DataFrame(session.sql(query, JavaUtils.objectArrayToSeq(new Object[0])));
return new DataFrame(session.sql(query));
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change guarantees Java code can invoke Session.sql function without parameter. So no additional test is required.

Copy link
Contributor

@sfc-gh-evandenberg sfc-gh-evandenberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sfc-gh-bli sfc-gh-bli merged commit 9dc5eb2 into main Nov 22, 2024
16 checks passed
@sfc-gh-bli sfc-gh-bli deleted the snow-1820719 branch November 22, 2024 00:56
sfc-gh-bli added a commit that referenced this pull request Nov 22, 2024
* refactor session sql

* fix java api
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