Skip to content

Commit 711b3b6

Browse files
j1wonparkpan3793
authored andcommitted
[KYUUBI #7568][AUTHZ] Eliminate TypeOfPlaceHolder across the whole plan
### Why are the changes needed? Fixes #7568. `RuleApplyTypeOfMarker` marks every `TypeOf` in the whole plan (`transformAllExpressions`), but since #5997 `RuleEliminateTypeOf` reverts them with `transformExpressionsUp`, which only visits the root node's expressions. A `TypeOfPlaceHolder` below the root — under `Sort`, `Union`, `Aggregate`, or a subquery — survives into execution and fails with `CLASS_NOT_OVERRIDE_EXPECTED_METHOD`, since it implements `doGenCode` but no `eval`. This change uses `transformAllExpressions` on the eliminating side as well, so both rules cover the same scope in a single pass. Affects 1.9.0 onwards, including master. ### How was this patch tested? Added a test to `RangerSparkExtensionSuite` covering the root shapes the existing `Project`-root-only test misses: `Sort`, `Union`, `Aggregate`, and a subquery. It fails on master and passes with this change. Full `extensions/spark/kyuubi-spark-authz` suite: 657 passed, 0 failed. ### Was this patch authored or co-authored using generative AI tooling? Assisted-by: Claude:claude-opus-4-8 Closes #7569 from j1wonpark/fix/authz-eliminate-typeof-whole-plan. Closes #7568 38464f1 [Jiwon Park] Sort collected rows in the UNION ALL case to avoid order dependency while keeping Union as the root node 9ffea2d [Jiwon Park] [KYUUBI #7568][AUTHZ] Eliminate TypeOfPlaceHolder across the whole plan Authored-by: Jiwon Park <jpark92@outlook.kr> Signed-off-by: Cheng Pan <chengpan@apache.org> (cherry picked from commit 8fe59c5) Signed-off-by: Cheng Pan <chengpan@apache.org>
1 parent c8d2d1e commit 711b3b6

2 files changed

Lines changed: 57 additions & 1 deletion

File tree

extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/rule/RuleEliminateTypeOf.scala

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,11 @@ import org.apache.kyuubi.plugin.spark.authz.rule.expression.TypeOfPlaceHolder
2525

2626
object RuleEliminateTypeOf extends Rule[LogicalPlan] {
2727
override def apply(plan: LogicalPlan): LogicalPlan = {
28-
plan.transformExpressionsUp {
28+
// Must cover the whole plan, mirroring RuleApplyTypeOfMarker. transformExpressionsUp only
29+
// visits the root node's expressions, leaving any placeholder below it (under Sort, Union,
30+
// Aggregate, a subquery, ...) to reach execution, where it fails: TypeOfPlaceHolder
31+
// implements no eval, only doGenCode.
32+
plan.transformAllExpressions {
2933
case toph: TypeOfPlaceHolder => TypeOf(toph.expr)
3034
}
3135
}

extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RangerSparkExtensionSuite.scala

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1344,6 +1344,58 @@ class HiveCatalogRangerSparkExtensionSuite extends RangerSparkExtensionSuite {
13441344
}
13451345
}
13461346

1347+
test("[KYUUBI #7568][AUTHZ] Typeof expression is eliminated below the root plan node") {
1348+
// RuleEliminateTypeOf must walk the whole plan, mirroring RuleApplyTypeOfMarker which
1349+
// marks with transformAllExpressions. When only the root node is visited, a TypeOfPlaceHolder
1350+
// living under Sort/Union/Aggregate survives into execution and fails with
1351+
// CLASS_NOT_OVERRIDE_EXPECTED_METHOD, since the placeholder implements no eval.
1352+
val db1 = defaultDb
1353+
val table1 = "table1"
1354+
withSingleCallEnabled {
1355+
withCleanTmpResources(Seq((s"$db1.$table1", "table"))) {
1356+
doAs(
1357+
admin,
1358+
sql(
1359+
s"""
1360+
|CREATE TABLE IF NOT EXISTS $db1.$table1(
1361+
|id int,
1362+
|scope int,
1363+
|day string)
1364+
|""".stripMargin))
1365+
doAs(admin, sql(s"INSERT INTO $db1.$table1 SELECT 1, 2, 'TONY'"))
1366+
1367+
// root is Sort
1368+
checkAnswer(
1369+
admin,
1370+
s"SELECT typeof(id) AS t FROM $db1.$table1 ORDER BY t",
1371+
Seq(Row("int")))
1372+
1373+
// root is Union. UNION ALL has no order guarantee, and an ORDER BY would put a Sort
1374+
// above the Union, so sort the collected rows instead to keep Union as the root node.
1375+
doAs(
1376+
admin,
1377+
assert(sql(
1378+
s"""
1379+
|SELECT typeof(id) FROM $db1.$table1
1380+
|UNION ALL
1381+
|SELECT typeof(day) FROM $db1.$table1""".stripMargin)
1382+
.collect().sortBy(_.getString(0)) === Seq(Row("int"), Row("string"))))
1383+
1384+
// root is Aggregate
1385+
checkAnswer(
1386+
admin,
1387+
s"SELECT typeof(id), count(*) FROM $db1.$table1 GROUP BY typeof(id)",
1388+
Seq(Row("int", 1)))
1389+
1390+
// typeof inside a subquery
1391+
checkAnswer(
1392+
admin,
1393+
s"SELECT t FROM (SELECT typeof(scope) AS t FROM $db1.$table1) x WHERE t = 'int'",
1394+
Seq(Row("int")))
1395+
}
1396+
}
1397+
}
1398+
13471399
test("[KYUUBI #5692][Bug] Authz not skip explain command") {
13481400
val db1 = defaultDb
13491401
val table1 = "table1"

0 commit comments

Comments
 (0)