Skip to content

Commit 91adc3f

Browse files
packyanyaooqinn
authored andcommitted
[KYUUBI #2399] Fix PrivilegesBuilder Build Wrong PrivilegeObjets When Query Without Project But With OrderBy/PartitionBy
### _Why are the changes needed?_ To close #2399 `PrivilegesBuilder#buildQuery()` should have more cases to cover some ***NO PROJECT*** situation. ### _How was this patch tested?_ - [x] Add some test cases that check the changes thoroughly including negative and positive cases if possible - [ ] Add screenshots for manual tests if appropriate - [x] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request Closes #2400 from packyan/bugfix_privileges_builder_return_wrong_result. Closes #2399 a4ad182 [Deng An] add more unit test for PrivilegesBuilder 59fbde3 [Deng An] add more cases in buildQuery method Authored-by: Deng An <36296995+packyan@users.noreply.github.com> Signed-off-by: Kent Yao <yao@apache.org>
1 parent 4b42c73 commit 91adc3f

File tree

2 files changed

+133
-19
lines changed

2 files changed

+133
-19
lines changed

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import org.apache.spark.sql.catalyst.{FunctionIdentifier, TableIdentifier}
2323
import org.apache.spark.sql.catalyst.catalog.CatalogTable
2424
import org.apache.spark.sql.catalyst.catalog.CatalogTypes.TablePartitionSpec
2525
import org.apache.spark.sql.catalyst.expressions.{Expression, NamedExpression}
26-
import org.apache.spark.sql.catalyst.plans.logical.{Command, Filter, Join, LogicalPlan, Project}
26+
import org.apache.spark.sql.catalyst.plans.logical.{Command, Filter, Join, LogicalPlan, Project, Sort, Window}
2727
import org.apache.spark.sql.execution.datasources.LogicalRelation
2828
import org.apache.spark.sql.types.StructField
2929

@@ -104,6 +104,17 @@ object PrivilegesBuilder {
104104
val cols = conditionList ++ collectLeaves(f.condition)
105105
buildQuery(f.child, privilegeObjects, projectionList, cols)
106106

107+
case w: Window =>
108+
val orderCols = w.orderSpec.flatMap(orderSpec => collectLeaves(orderSpec))
109+
val partitionCols = w.partitionSpec.flatMap(partitionSpec => collectLeaves(partitionSpec))
110+
val cols = conditionList ++ orderCols ++ partitionCols
111+
buildQuery(w.child, privilegeObjects, projectionList, cols)
112+
113+
case s: Sort =>
114+
val sortCols = s.order.flatMap(sortOrder => collectLeaves(sortOrder))
115+
val cols = conditionList ++ sortCols
116+
buildQuery(s.child, privilegeObjects, projectionList, cols)
117+
107118
case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
108119
mergeProjection(getHiveTable(hiveTableRelation), hiveTableRelation)
109120

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

Lines changed: 121 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -165,24 +165,6 @@ abstract class PrivilegesBuilderSuite extends KyuubiFunSuite with SparkSessionPr
165165
}
166166
}
167167

168-
test("AlterTableAddColumnsCommand") {
169-
val plan = sql(s"ALTER TABLE $reusedTable" +
170-
s" ADD COLUMNS (a int)").queryExecution.analyzed
171-
val operationType = OperationType(plan.nodeName)
172-
assert(operationType === ALTERTABLE_ADDCOLS)
173-
val tuple = PrivilegesBuilder.build(plan)
174-
assert(tuple._1.isEmpty)
175-
assert(tuple._2.size === 1)
176-
val po = tuple._2.head
177-
assert(po.actionType === PrivilegeObjectActionType.OTHER)
178-
assert(po.privilegeObjectType === PrivilegeObjectType.TABLE_OR_VIEW)
179-
assert(po.dbname equalsIgnoreCase reusedDb)
180-
assert(po.objectName === getClass.getSimpleName)
181-
assert(po.columns.head === "a")
182-
val accessType = ranger.AccessType(po, operationType, isInput = false)
183-
assert(accessType === AccessType.ALTER)
184-
}
185-
186168
test("AlterTableAddPartitionCommand") {
187169
val plan = sql(s"ALTER TABLE $reusedPartTable ADD IF NOT EXISTS PARTITION (pid=1)")
188170
.queryExecution.analyzed
@@ -891,6 +873,58 @@ abstract class PrivilegesBuilderSuite extends KyuubiFunSuite with SparkSessionPr
891873
Seq("value", "pid"))
892874
}
893875

876+
test("Query: Subquery With Window") {
877+
val plan = sql(
878+
s"""
879+
|SELECT value, rank FROM(
880+
|SELECT *,
881+
|RANK() OVER (PARTITION BY key ORDER BY pid) AS rank
882+
|FROM $reusedPartTable)
883+
|""".stripMargin).queryExecution.optimizedPlan
884+
val operationType = OperationType(plan.nodeName)
885+
assert(operationType === QUERY)
886+
val tuple = PrivilegesBuilder.build(plan)
887+
assert(tuple._1.size === 1)
888+
tuple._1.foreach { po =>
889+
assert(po.actionType === PrivilegeObjectActionType.OTHER)
890+
assert(po.privilegeObjectType === PrivilegeObjectType.TABLE_OR_VIEW)
891+
assert(po.dbname equalsIgnoreCase reusedDb)
892+
assert(po.objectName startsWith reusedTableShort.toLowerCase)
893+
assert(
894+
po.columns === Seq("value", "pid", "key"),
895+
s"$reusedPartTable both 'key', 'value' and 'pid' should be authenticated")
896+
val accessType = ranger.AccessType(po, operationType, isInput = true)
897+
assert(accessType === AccessType.SELECT)
898+
}
899+
assert(tuple._2.size === 0)
900+
}
901+
902+
test("Query: Subquery With Order") {
903+
val plan = sql(
904+
s"""
905+
|SELECT value FROM(
906+
|SELECT *
907+
|FROM $reusedPartTable
908+
|ORDER BY key, pid)
909+
|""".stripMargin).queryExecution.optimizedPlan
910+
val operationType = OperationType(plan.nodeName)
911+
assert(operationType === QUERY)
912+
val tuple = PrivilegesBuilder.build(plan)
913+
assert(tuple._1.size === 1)
914+
tuple._1.foreach { po =>
915+
assert(po.actionType === PrivilegeObjectActionType.OTHER)
916+
assert(po.privilegeObjectType === PrivilegeObjectType.TABLE_OR_VIEW)
917+
assert(po.dbname equalsIgnoreCase reusedDb)
918+
assert(po.objectName startsWith reusedTableShort.toLowerCase)
919+
assert(
920+
po.columns === Seq("value", "key", "pid"),
921+
s"$reusedPartTable both 'key', 'value' and 'pid' should be authenticated")
922+
val accessType = ranger.AccessType(po, operationType, isInput = true)
923+
assert(accessType === AccessType.SELECT)
924+
}
925+
assert(tuple._2.size === 0)
926+
}
927+
894928
test("Query: INNER JOIN") {
895929
val sqlStr =
896930
s"""
@@ -977,6 +1011,56 @@ abstract class PrivilegesBuilderSuite extends KyuubiFunSuite with SparkSessionPr
9771011
assert(tuple._2.size === 0)
9781012
}
9791013

1014+
test("Query: Window Without Project") {
1015+
val plan = sql(
1016+
s"""
1017+
|SELECT key,
1018+
|RANK() OVER (PARTITION BY key ORDER BY value) AS rank
1019+
|FROM $reusedTable
1020+
|""".stripMargin).queryExecution.optimizedPlan
1021+
val operationType = OperationType(plan.nodeName)
1022+
assert(operationType === QUERY)
1023+
val tuple = PrivilegesBuilder.build(plan)
1024+
assert(tuple._1.size === 1)
1025+
tuple._1.foreach { po =>
1026+
assert(po.actionType === PrivilegeObjectActionType.OTHER)
1027+
assert(po.privilegeObjectType === PrivilegeObjectType.TABLE_OR_VIEW)
1028+
assert(po.dbname equalsIgnoreCase reusedDb)
1029+
assert(po.objectName startsWith reusedTableShort.toLowerCase)
1030+
assert(
1031+
po.columns === Seq("key", "value"),
1032+
s"$reusedPartTable both 'key' and 'value' should be authenticated")
1033+
val accessType = ranger.AccessType(po, operationType, isInput = true)
1034+
assert(accessType === AccessType.SELECT)
1035+
}
1036+
assert(tuple._2.size === 0)
1037+
}
1038+
1039+
test("Query: Order By Without Project") {
1040+
val plan = sql(
1041+
s"""
1042+
|SELECT key
1043+
|FROM $reusedPartTable
1044+
|ORDER BY key, value, pid
1045+
|""".stripMargin).queryExecution.optimizedPlan
1046+
val operationType = OperationType(plan.nodeName)
1047+
assert(operationType === QUERY)
1048+
val tuple = PrivilegesBuilder.build(plan)
1049+
assert(tuple._1.size === 1)
1050+
tuple._1.foreach { po =>
1051+
assert(po.actionType === PrivilegeObjectActionType.OTHER)
1052+
assert(po.privilegeObjectType === PrivilegeObjectType.TABLE_OR_VIEW)
1053+
assert(po.dbname equalsIgnoreCase reusedDb)
1054+
assert(po.objectName startsWith reusedTableShort.toLowerCase)
1055+
assert(
1056+
po.columns === Seq("key", "value", "pid"),
1057+
s"$reusedPartTable both 'key', 'value' and 'pid' should be authenticated")
1058+
val accessType = ranger.AccessType(po, operationType, isInput = true)
1059+
assert(accessType === AccessType.SELECT)
1060+
}
1061+
assert(tuple._2.size === 0)
1062+
}
1063+
9801064
test("Query: Union") {
9811065
val plan = sql(
9821066
s"""
@@ -1155,4 +1239,23 @@ class HiveCatalogPrivilegeBuilderSuite extends PrivilegesBuilderSuite {
11551239
assert(tuple._2.size === 0)
11561240
}
11571241
}
1242+
// ALTER TABLE ADD COLUMNS It should be run at end
1243+
// to prevent affecting the cases that rely on the $reusedTable's table structure
1244+
test("AlterTableAddColumnsCommand") {
1245+
val plan = sql(s"ALTER TABLE $reusedTable" +
1246+
s" ADD COLUMNS (a int)").queryExecution.analyzed
1247+
val operationType = OperationType(plan.nodeName)
1248+
assert(operationType === ALTERTABLE_ADDCOLS)
1249+
val tuple = PrivilegesBuilder.build(plan)
1250+
assert(tuple._1.isEmpty)
1251+
assert(tuple._2.size === 1)
1252+
val po = tuple._2.head
1253+
assert(po.actionType === PrivilegeObjectActionType.OTHER)
1254+
assert(po.privilegeObjectType === PrivilegeObjectType.TABLE_OR_VIEW)
1255+
assert(po.dbname equalsIgnoreCase reusedDb)
1256+
assert(po.objectName === getClass.getSimpleName)
1257+
assert(po.columns.head === "a")
1258+
val accessType = ranger.AccessType(po, operationType, isInput = false)
1259+
assert(accessType === AccessType.ALTER)
1260+
}
11581261
}

0 commit comments

Comments
 (0)