You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "vrjdev (via GitHub)" <gi...@apache.org> on 2026/09/09 20:17:46 UTC
[PR] [SPARK-59387][SQL][TESTS] Cover restricted-mode rejection of banned functions in subqueries [spark]
vrjdev opened a new pull request, #58674:
URL: https://github.com/apache/spark/pull/58674
### What changes were proposed in this pull request?
Adds a test to `RestrictedModeSuite` verifying that the restricted SQL execution mode (SPARK-59319) rejects banned functions (`reflect` / `java_method` / `try_reflect`) when they are nested inside subqueries — both `EXISTS` and scalar subqueries — asserting `UNSUPPORTED_FEATURE.SQL_RESTRICTED_MODE`, plus a mode-off control. No production code is changed.
### Why are the changes needed?
The restricted-mode gate promises banned features "cannot slip through": it walks the whole plan including `SubqueryExpression` plans. That traversal branch had zero coverage — the existing suite only exercises top-level calls, top-level TRANSFORM, and analyzed-subplan reuse. A regression there would fail silently (banned code runs, no test goes red), which is the worst failure mode for a security gate. Subqueries are also the most natural place to hide a banned call from a top-level-only check.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
New test `restricted mode rejects banned functions nested in subqueries` in `RestrictedModeSuite` (analysis-only, no session needed):
```
build/sbt 'catalyst/testOnly org.apache.spark.sql.catalyst.analysis.RestrictedModeSuite'
```
Result: Tests: succeeded 4, failed 0 (3 existing + 1 new).
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: MetaCode
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59387][SQL][TESTS] Cover restricted-mode rejection of banned functions in subqueries [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on PR #58674:
URL: https://github.com/apache/spark/pull/58674#issuecomment-5632840464
cc @HyukjinKwon here ^^
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59387][SQL][TESTS] Cover restricted-mode rejection of banned functions in subqueries [spark]
Posted by "vrjdev (via GitHub)" <gi...@apache.org>.
vrjdev commented on PR #58674:
URL: https://github.com/apache/spark/pull/58674#issuecomment-5608500216
@HyukjinKwon added a minor test case for your change, PTAL.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59387][SQL][TESTS] Cover restricted-mode rejection of banned functions in subqueries [spark]
Posted by "vrjdev (via GitHub)" <gi...@apache.org>.
vrjdev commented on code in PR #58674:
URL: https://github.com/apache/spark/pull/58674#discussion_r3983822465
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/RestrictedModeSuite.scala:
##########
@@ -137,4 +139,31 @@ class RestrictedModeSuite extends AnalysisTest {
checkRestrictedError(e, "The TRANSFORM ... USING clause")
}
}
+
+ test("restricted mode rejects banned functions nested in subqueries") {
+ Seq("reflect", "java_method", "try_reflect").foreach { fn =>
+ withRestrictedMode(true) {
+ val analyzer = getAnalyzer
+ val e = intercept[AnalysisException] {
+ analyzer.checkAnalysis(analyzer.execute(
+ Filter(Exists(functionProject(fn)), TestRelations.testRelation)))
+ }
+ checkRestrictedError(e, s"The `$fn` function")
+ }
+ }
+ withRestrictedMode(true) {
+ val analyzer = getAnalyzer
+ val e = intercept[AnalysisException] {
+ analyzer.checkAnalysis(analyzer.execute(Project(
+ Seq(UnresolvedAlias(ScalarSubquery(functionProject("reflect")))),
+ TestRelations.testRelation)))
+ }
+ checkRestrictedError(e, "The `reflect` function")
+ }
+ withRestrictedMode(false) {
+ val analyzer = getAnalyzer
+ analyzer.checkAnalysis(analyzer.execute(
+ Filter(Exists(functionProject("reflect")), TestRelations.testRelation)))
+ }
+ }
Review Comment:
Thanks for the review, I will check and will update the PR and test accordingly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59387][SQL][TESTS] Cover restricted-mode rejection of banned functions in subqueries [spark]
Posted by "vrjdev (via GitHub)" <gi...@apache.org>.
vrjdev commented on PR #58674:
URL: https://github.com/apache/spark/pull/58674#issuecomment-5622792006
cc : @uros-b PTAL
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59387][SQL][TESTS] Cover restricted-mode rejection of banned functions in subqueries [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on code in PR #58674:
URL: https://github.com/apache/spark/pull/58674#discussion_r3983057049
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/RestrictedModeSuite.scala:
##########
@@ -137,4 +139,31 @@ class RestrictedModeSuite extends AnalysisTest {
checkRestrictedError(e, "The TRANSFORM ... USING clause")
}
}
+
+ test("restricted mode rejects banned functions nested in subqueries") {
+ Seq("reflect", "java_method", "try_reflect").foreach { fn =>
+ withRestrictedMode(true) {
+ val analyzer = getAnalyzer
+ val e = intercept[AnalysisException] {
+ analyzer.checkAnalysis(analyzer.execute(
+ Filter(Exists(functionProject(fn)), TestRelations.testRelation)))
+ }
+ checkRestrictedError(e, s"The `$fn` function")
+ }
+ }
+ withRestrictedMode(true) {
+ val analyzer = getAnalyzer
+ val e = intercept[AnalysisException] {
+ analyzer.checkAnalysis(analyzer.execute(Project(
+ Seq(UnresolvedAlias(ScalarSubquery(functionProject("reflect")))),
+ TestRelations.testRelation)))
+ }
+ checkRestrictedError(e, "The `reflect` function")
+ }
+ withRestrictedMode(false) {
+ val analyzer = getAnalyzer
+ analyzer.checkAnalysis(analyzer.execute(
+ Filter(Exists(functionProject("reflect")), TestRelations.testRelation)))
+ }
+ }
Review Comment:
RestrictedModeSuite.scala (new test) & PR body -- Overlap re-confirmed against current master: the sibling `RestrictedModeCommandSuite` (sql/core) still has `"restricted mode reaches a feature nested inside a scalar subquery"` (`SELECT (SELECT reflect(...)) AS c`) plus `"restricted mode allows deeply nested subqueries..."` (40-deep), both driving the same `SubqueryExpression => checkPlan(s.plan)` branch through real SQL. So the body's "zero coverage" / "a regression would fail silently (every existing test stays green)" framing is repo-wide inaccurate, and the new unit scalar-subquery case duplicates that sql/core coverage. Narrow the justification to the catalyst suite `RestrictedModeSuite`, and trim or re-focus the scalar case onto the genuinely-new shapes: the `EXISTS` predicate (all three functions) and `java_method`/`try_reflect` nested in a subquery, which no suite covers.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59387][SQL][TESTS] Cover restricted-mode rejection of banned functions in subqueries [spark]
Posted by "vrjdev (via GitHub)" <gi...@apache.org>.
vrjdev commented on code in PR #58674:
URL: https://github.com/apache/spark/pull/58674#discussion_r3983957741
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/RestrictedModeSuite.scala:
##########
@@ -137,4 +139,31 @@ class RestrictedModeSuite extends AnalysisTest {
checkRestrictedError(e, "The TRANSFORM ... USING clause")
}
}
+
+ test("restricted mode rejects banned functions nested in subqueries") {
+ Seq("reflect", "java_method", "try_reflect").foreach { fn =>
+ withRestrictedMode(true) {
+ val analyzer = getAnalyzer
+ val e = intercept[AnalysisException] {
+ analyzer.checkAnalysis(analyzer.execute(
+ Filter(Exists(functionProject(fn)), TestRelations.testRelation)))
+ }
+ checkRestrictedError(e, s"The `$fn` function")
+ }
+ }
+ withRestrictedMode(true) {
+ val analyzer = getAnalyzer
+ val e = intercept[AnalysisException] {
+ analyzer.checkAnalysis(analyzer.execute(Project(
+ Seq(UnresolvedAlias(ScalarSubquery(functionProject("reflect")))),
+ TestRelations.testRelation)))
+ }
+ checkRestrictedError(e, "The `reflect` function")
+ }
+ withRestrictedMode(false) {
+ val analyzer = getAnalyzer
+ analyzer.checkAnalysis(analyzer.execute(
+ Filter(Exists(functionProject("reflect")), TestRelations.testRelation)))
+ }
+ }
Review Comment:
Updated the tests based on the comment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org