-
Notifications
You must be signed in to change notification settings - Fork 28.5k
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-48348][SPARK-48376][FOLLOWUP][SQL] Replace parseScript with runSqlScript in SQL Scripting Interpreter test suite #48016
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.
@davidm-db Please, tag it as a follow up PR, and add the same JIRA tickets to PR's title.
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.
Waiting for CI.
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.
+1, LGTM. Merging to master. |
…nSqlScript in SQL Scripting Interpreter test suite ### What changes were proposed in this pull request? Previous [pull request](apache#47973) introduced new tests to `SqlScriptingInterpreterSuite` (among others) where accidentally `parseScript` was used instead of `runSqlScript`. While the same exception would get thrown (since it happens in the parsing phase) it violates the consistency among the tests in this suite and adds unnecessary import, so it would be nice to change it. ### Why are the changes needed? Changes are minor, they improve consistency among test suites for SQL scripting. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? This patch alters tests. ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#48016 from davidm-db/interpreter_test_suite_fix. Authored-by: David Milicevic <david.milicevic@databricks.com> Signed-off-by: Max Gekk <max.gekk@gmail.com>
…nSqlScript in SQL Scripting Interpreter test suite ### What changes were proposed in this pull request? Previous [pull request](apache#47973) introduced new tests to `SqlScriptingInterpreterSuite` (among others) where accidentally `parseScript` was used instead of `runSqlScript`. While the same exception would get thrown (since it happens in the parsing phase) it violates the consistency among the tests in this suite and adds unnecessary import, so it would be nice to change it. ### Why are the changes needed? Changes are minor, they improve consistency among test suites for SQL scripting. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? This patch alters tests. ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#48016 from davidm-db/interpreter_test_suite_fix. Authored-by: David Milicevic <david.milicevic@databricks.com> Signed-off-by: Max Gekk <max.gekk@gmail.com>
…nSqlScript in SQL Scripting Interpreter test suite ### What changes were proposed in this pull request? Previous [pull request](apache#47973) introduced new tests to `SqlScriptingInterpreterSuite` (among others) where accidentally `parseScript` was used instead of `runSqlScript`. While the same exception would get thrown (since it happens in the parsing phase) it violates the consistency among the tests in this suite and adds unnecessary import, so it would be nice to change it. ### Why are the changes needed? Changes are minor, they improve consistency among test suites for SQL scripting. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? This patch alters tests. ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#48016 from davidm-db/interpreter_test_suite_fix. Authored-by: David Milicevic <david.milicevic@databricks.com> Signed-off-by: Max Gekk <max.gekk@gmail.com>
What changes were proposed in this pull request?
Previous pull request introduced new tests to
SqlScriptingInterpreterSuite
(among others) where accidentallyparseScript
was used instead ofrunSqlScript
.While the same exception would get thrown (since it happens in the parsing phase) it violates the consistency among the tests in this suite and adds unnecessary import, so it would be nice to change it.
Why are the changes needed?
Changes are minor, they improve consistency among test suites for SQL scripting.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
This patch alters tests.
Was this patch authored or co-authored using generative AI tooling?
No.