You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by GitBox <gi...@apache.org> on 2022/08/08 06:19:34 UTC
[GitHub] [incubator-kyuubi] bowenliang123 opened a new pull request, #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
bowenliang123 opened a new pull request, #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197
<!--
Thanks for sending a pull request!
Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://kyuubi.readthedocs.io/en/latest/community/contributions.html
2. If the PR is related to an issue in https://github.com/apache/incubator-kyuubi/issues, add '[KYUUBI #XXXX]' in your PR title, e.g., '[KYUUBI #XXXX] Your PR title ...'.
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][KYUUBI #XXXX] Your PR title ...'.
-->
### _Why are the changes needed?_
<!--
Please clarify why the changes are needed. For instance,
1. If you add a feature, you can talk about the use case of it.
2. If you fix a bug, you can clarify why it is a bug.
-->
`RuleApplyRowFilterAndDataMasking` in AuthZ currently applys row-level filters correctly on both `HiveTableRelation` fro Hive tables and `LogicalRelation`, but is not affcecting the execution plan for Iceberg tables.
As Iceberg table is accssesed via `DataSourceV2Relation`, `RuleApplyRowFilterAndDataMasking` is adopted to apply row filtering on `TableIdentifier` for different relation (including HiveTableRelation and LogicalRelation) and fetch correct table identifier from `DataSourceV2Relation`.
Considering an iceberg table `gftest.sampleice`, with an row-level filter as `id='1'`.
SQL：
`EXPLAIN EXTENDED select *from gftest.sampleice limit 5;`
Execution Plan before this PR：
```
== Parsed Logical Plan ==
'GlobalLimit 5
+- 'LocalLimit 5
+- 'Project [*]
+- 'UnresolvedRelation [gftest, sampleice], [], false
== Analyzed Logical Plan ==
id: bigint, data: string
GlobalLimit 5
+- LocalLimit 5
+- Project [id#13332L, data#13333]
+- SubqueryAlias spark_catalog.gftest.sampleice
+- RelationV2[id#13332L, data#13333] spark_catalog.gftest.sampleice
== Optimized Logical Plan ==
GlobalLimit 5
+- LocalLimit 5
+- RelationV2[id#13332L, data#13333] spark_catalog.gftest.sampleice
== Physical Plan ==
CollectLimit 5
+- *(1) ColumnarToRow
+- BatchScan[id#13332L, data#13333] spark_catalog.gftest.sampleice [filters=] RuntimeFilters: []
```
Execution Plan before this PR：
```
== Parsed Logical Plan ==
'GlobalLimit 5
+- 'LocalLimit 5
+- 'Project [*]
+- 'UnresolvedRelation [gftest, sampleice], [], false
== Analyzed Logical Plan ==
id: bigint, data: string
GlobalLimit 5
+- LocalLimit 5
+- Project [id#142L, data#143]
+- SubqueryAlias spark_catalog.gftest.sampleice
+- Project [id#142L, data#143]
+- Filter (id#142L = cast(1 as bigint))
+- RowFilterAndDataMaskingMarker
+- RelationV2[id#142L, data#143] spark_catalog.gftest.sampleice
== Optimized Logical Plan ==
GlobalLimit 5
+- LocalLimit 5
+- Filter (isnotnull(id#142L) AND (id#142L = 1))
+- RelationV2[id#142L, data#143] spark_catalog.gftest.sampleice
== Physical Plan ==
CollectLimit 5
+- *(1) Filter (isnotnull(id#142L) AND (id#142L = 1))
+- *(1) ColumnarToRow
+- BatchScan[id#142L, data#143] spark_catalog.gftest.sampleice [filters=id IS NOT NULL, id = 1] RuntimeFilters: []
```
### _How was this patch tested?_
- [ ] 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
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yikf commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
yikf commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r939995010
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyRowFilterAndDataMasking.scala:
##########
@@ -30,29 +30,37 @@ import org.apache.kyuubi.plugin.spark.authz.util.RowFilterAndDataMaskingMarker
class RuleApplyRowFilterAndDataMasking(spark: SparkSession) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
- // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation with
+ // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation/DataSourceV2Relation with
// RowFilterAndDataMaskingMarker if it is not wrapped yet.
plan mapChildren {
case p: RowFilterAndDataMaskingMarker => p
case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
val table = getHiveTable(hiveTableRelation)
- applyFilterAndMasking(hiveTableRelation, table, spark)
+ applyFilterAndMasking(hiveTableRelation, table.identifier, spark)
case logicalRelation if hasResolvedDatasourceTable(logicalRelation) =>
val table = getDatasourceTable(logicalRelation)
if (table.isEmpty) {
logicalRelation
} else {
- applyFilterAndMasking(logicalRelation, table.get, spark)
+ applyFilterAndMasking(logicalRelation, table.get.identifier, spark)
+ }
+ case datasourceV2Relation if hasResolvedDatasourceV2Table(datasourceV2Relation) =>
Review Comment:
```suggestion
case relation @ DataSourceV2Relation(_, _, _, Some(identifier), _) if relation.resolved =>
```
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyRowFilterAndDataMasking.scala:
##########
@@ -30,29 +30,37 @@ import org.apache.kyuubi.plugin.spark.authz.util.RowFilterAndDataMaskingMarker
class RuleApplyRowFilterAndDataMasking(spark: SparkSession) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
- // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation with
+ // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation/DataSourceV2Relation with
// RowFilterAndDataMaskingMarker if it is not wrapped yet.
plan mapChildren {
case p: RowFilterAndDataMaskingMarker => p
case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
val table = getHiveTable(hiveTableRelation)
- applyFilterAndMasking(hiveTableRelation, table, spark)
+ applyFilterAndMasking(hiveTableRelation, table.identifier, spark)
case logicalRelation if hasResolvedDatasourceTable(logicalRelation) =>
Review Comment:
It looks like these can be refactored as well
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] bowenliang123 commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
bowenliang123 commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r940003576
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyRowFilterAndDataMasking.scala:
##########
@@ -30,29 +30,37 @@ import org.apache.kyuubi.plugin.spark.authz.util.RowFilterAndDataMaskingMarker
class RuleApplyRowFilterAndDataMasking(spark: SparkSession) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
- // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation with
+ // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation/DataSourceV2Relation with
// RowFilterAndDataMaskingMarker if it is not wrapped yet.
plan mapChildren {
case p: RowFilterAndDataMaskingMarker => p
case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
val table = getHiveTable(hiveTableRelation)
- applyFilterAndMasking(hiveTableRelation, table, spark)
+ applyFilterAndMasking(hiveTableRelation, table.identifier, spark)
case logicalRelation if hasResolvedDatasourceTable(logicalRelation) =>
val table = getDatasourceTable(logicalRelation)
if (table.isEmpty) {
logicalRelation
} else {
- applyFilterAndMasking(logicalRelation, table.get, spark)
+ applyFilterAndMasking(logicalRelation, table.get.identifier, spark)
+ }
+ case datasourceV2Relation if hasResolvedDatasourceV2Table(datasourceV2Relation) =>
+ val identifier = getDatasourceV2Identifier(datasourceV2Relation)
+ if (identifier.isEmpty) {
+ return datasourceV2Relation
Review Comment:
thx. applied.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r940021424
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyRowFilterAndDataMasking.scala:
##########
@@ -30,29 +30,37 @@ import org.apache.kyuubi.plugin.spark.authz.util.RowFilterAndDataMaskingMarker
class RuleApplyRowFilterAndDataMasking(spark: SparkSession) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
- // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation with
+ // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation/DataSourceV2Relation with
// RowFilterAndDataMaskingMarker if it is not wrapped yet.
plan mapChildren {
case p: RowFilterAndDataMaskingMarker => p
case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
val table = getHiveTable(hiveTableRelation)
- applyFilterAndMasking(hiveTableRelation, table, spark)
+ applyFilterAndMasking(hiveTableRelation, table.identifier, spark)
case logicalRelation if hasResolvedDatasourceTable(logicalRelation) =>
val table = getDatasourceTable(logicalRelation)
if (table.isEmpty) {
logicalRelation
} else {
- applyFilterAndMasking(logicalRelation, table.get, spark)
+ applyFilterAndMasking(logicalRelation, table.get.identifier, spark)
+ }
+ case datasourceV2Relation if hasResolvedDatasourceV2Table(datasourceV2Relation) =>
+ val identifier = getDatasourceV2Identifier(datasourceV2Relation)
+ if (identifier.isEmpty) {
+ return datasourceV2Relation
+ } else {
+ val tableIdentifier = TableIdentifier(identifier.get.name,
+ Option(identifier.get.namespace.mkString(".")))
Review Comment:
org.apache.kyuubi.plugin.spark.authz.PrivilegesBuilder#quote
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] bowenliang123 commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
bowenliang123 commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r940042795
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyRowFilterAndDataMasking.scala:
##########
@@ -30,29 +30,37 @@ import org.apache.kyuubi.plugin.spark.authz.util.RowFilterAndDataMaskingMarker
class RuleApplyRowFilterAndDataMasking(spark: SparkSession) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
- // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation with
+ // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation/DataSourceV2Relation with
// RowFilterAndDataMaskingMarker if it is not wrapped yet.
plan mapChildren {
case p: RowFilterAndDataMaskingMarker => p
case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
val table = getHiveTable(hiveTableRelation)
- applyFilterAndMasking(hiveTableRelation, table, spark)
+ applyFilterAndMasking(hiveTableRelation, table.identifier, spark)
case logicalRelation if hasResolvedDatasourceTable(logicalRelation) =>
val table = getDatasourceTable(logicalRelation)
if (table.isEmpty) {
logicalRelation
} else {
- applyFilterAndMasking(logicalRelation, table.get, spark)
+ applyFilterAndMasking(logicalRelation, table.get.identifier, spark)
+ }
+ case datasourceV2Relation if hasResolvedDatasourceV2Table(datasourceV2Relation) =>
+ val identifier = getDatasourceV2Identifier(datasourceV2Relation)
+ if (identifier.isEmpty) {
+ return datasourceV2Relation
+ } else {
+ val tableIdentifier = TableIdentifier(identifier.get.name,
+ Option(identifier.get.namespace.mkString(".")))
Review Comment:
Another problem related is `quote` automatically determine `quoteIfNeeded` and add quotes to it. However this escaping is not probablely necessary for naming and looking up of ranger policy. Quotes may cause confusions to it with inconsistency in namespaces and tables.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] bowenliang123 commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
bowenliang123 commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r939907558
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyRowFilterAndDataMasking.scala:
##########
@@ -30,29 +30,40 @@ import org.apache.kyuubi.plugin.spark.authz.util.RowFilterAndDataMaskingMarker
class RuleApplyRowFilterAndDataMasking(spark: SparkSession) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
- // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation with
+ // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation/DataSourceV2Relation with
// RowFilterAndDataMaskingMarker if it is not wrapped yet.
plan mapChildren {
case p: RowFilterAndDataMaskingMarker => p
case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
val table = getHiveTable(hiveTableRelation)
- applyFilterAndMasking(hiveTableRelation, table, spark)
+ applyFilterAndMasking(hiveTableRelation, table.identifier, spark)
case logicalRelation if hasResolvedDatasourceTable(logicalRelation) =>
val table = getDatasourceTable(logicalRelation)
if (table.isEmpty) {
logicalRelation
} else {
- applyFilterAndMasking(logicalRelation, table.get, spark)
+ applyFilterAndMasking(logicalRelation, table.get.identifier, spark)
+ }
+ case datasourceV2Relation if hasResolvedDatasourceV2Table(datasourceV2Relation) =>
+ val table = getDatasourceV2Table(datasourceV2Relation)
+ if (table == null) {
+ datasourceV2Relation
+ } else {
+ val catalogDbTable = table.name.split("\\.")
Review Comment:
I have also noticed the shortcomings. The current implementation is not elegant as espected.
In `org.apache.spark.sql.connector.catalog.Table`, `name()` method is not providing enough infomation to identify all these situations.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] bowenliang123 commented on pull request #3197: [KYUUBI #3186] Support applying Row-level Filter and Data Masking policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
bowenliang123 commented on PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#issuecomment-1210039733
> @bowenliang123 thanks for the contribution, and the patience all the way through.
>
> Merged to master.
Very inspiring and delightful PR experience here. Thanks for all the guidances from feature idea to details of the implementation and even specific code samples.
Also many thanks to my colleague @jeanlyn from Guangfa Securities envolved in this feature.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] bowenliang123 commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
bowenliang123 commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r940031212
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyRowFilterAndDataMasking.scala:
##########
@@ -30,29 +30,37 @@ import org.apache.kyuubi.plugin.spark.authz.util.RowFilterAndDataMaskingMarker
class RuleApplyRowFilterAndDataMasking(spark: SparkSession) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
- // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation with
+ // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation/DataSourceV2Relation with
// RowFilterAndDataMaskingMarker if it is not wrapped yet.
plan mapChildren {
case p: RowFilterAndDataMaskingMarker => p
case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
val table = getHiveTable(hiveTableRelation)
- applyFilterAndMasking(hiveTableRelation, table, spark)
+ applyFilterAndMasking(hiveTableRelation, table.identifier, spark)
case logicalRelation if hasResolvedDatasourceTable(logicalRelation) =>
val table = getDatasourceTable(logicalRelation)
if (table.isEmpty) {
logicalRelation
} else {
- applyFilterAndMasking(logicalRelation, table.get, spark)
+ applyFilterAndMasking(logicalRelation, table.get.identifier, spark)
+ }
+ case datasourceV2Relation if hasResolvedDatasourceV2Table(datasourceV2Relation) =>
+ val identifier = getDatasourceV2Identifier(datasourceV2Relation)
+ if (identifier.isEmpty) {
+ return datasourceV2Relation
+ } else {
+ val tableIdentifier = TableIdentifier(identifier.get.name,
+ Option(identifier.get.namespace.mkString(".")))
Review Comment:
Thx for guidance. But is it a private method `quote` of PrivilegesBuilder which is inaccessible inside `RuleApplyRowFilterAndDataMasking` ? Trying to move `quote` method to AuthzUtils.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] bowenliang123 commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
bowenliang123 commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r940034441
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyRowFilterAndDataMasking.scala:
##########
@@ -30,29 +30,37 @@ import org.apache.kyuubi.plugin.spark.authz.util.RowFilterAndDataMaskingMarker
class RuleApplyRowFilterAndDataMasking(spark: SparkSession) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
- // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation with
+ // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation/DataSourceV2Relation with
// RowFilterAndDataMaskingMarker if it is not wrapped yet.
plan mapChildren {
case p: RowFilterAndDataMaskingMarker => p
case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
val table = getHiveTable(hiveTableRelation)
- applyFilterAndMasking(hiveTableRelation, table, spark)
+ applyFilterAndMasking(hiveTableRelation, table.identifier, spark)
case logicalRelation if hasResolvedDatasourceTable(logicalRelation) =>
Review Comment:
same situation as above。
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyRowFilterAndDataMasking.scala:
##########
@@ -30,29 +30,37 @@ import org.apache.kyuubi.plugin.spark.authz.util.RowFilterAndDataMaskingMarker
class RuleApplyRowFilterAndDataMasking(spark: SparkSession) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
- // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation with
+ // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation/DataSourceV2Relation with
// RowFilterAndDataMaskingMarker if it is not wrapped yet.
plan mapChildren {
case p: RowFilterAndDataMaskingMarker => p
case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
val table = getHiveTable(hiveTableRelation)
- applyFilterAndMasking(hiveTableRelation, table, spark)
+ applyFilterAndMasking(hiveTableRelation, table.identifier, spark)
case logicalRelation if hasResolvedDatasourceTable(logicalRelation) =>
Review Comment:
same situation as above.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] bowenliang123 commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
bowenliang123 commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r940004682
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyRowFilterAndDataMasking.scala:
##########
@@ -30,29 +30,37 @@ import org.apache.kyuubi.plugin.spark.authz.util.RowFilterAndDataMaskingMarker
class RuleApplyRowFilterAndDataMasking(spark: SparkSession) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
- // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation with
+ // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation/DataSourceV2Relation with
// RowFilterAndDataMaskingMarker if it is not wrapped yet.
plan mapChildren {
case p: RowFilterAndDataMaskingMarker => p
case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
val table = getHiveTable(hiveTableRelation)
- applyFilterAndMasking(hiveTableRelation, table, spark)
+ applyFilterAndMasking(hiveTableRelation, table.identifier, spark)
case logicalRelation if hasResolvedDatasourceTable(logicalRelation) =>
val table = getDatasourceTable(logicalRelation)
if (table.isEmpty) {
logicalRelation
} else {
- applyFilterAndMasking(logicalRelation, table.get, spark)
+ applyFilterAndMasking(logicalRelation, table.get.identifier, spark)
+ }
+ case datasourceV2Relation if hasResolvedDatasourceV2Table(datasourceV2Relation) =>
Review Comment:
Good suggestion though. I am still using the `hasXXXX` pattern method to follow the existed method for HiveTableRelation and LogicalRelation as hasResolvedHiveTable and hasResolvedDatasourceTable. And I couldn't see enough clarification improvement by using guard pattern for this.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] bowenliang123 commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
bowenliang123 commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r940830356
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyRowFilterAndDataMasking.scala:
##########
@@ -30,29 +30,35 @@ import org.apache.kyuubi.plugin.spark.authz.util.RowFilterAndDataMaskingMarker
class RuleApplyRowFilterAndDataMasking(spark: SparkSession) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
- // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation with
+ // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation/DataSourceV2Relation with
// RowFilterAndDataMaskingMarker if it is not wrapped yet.
plan mapChildren {
case p: RowFilterAndDataMaskingMarker => p
case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
val table = getHiveTable(hiveTableRelation)
- applyFilterAndMasking(hiveTableRelation, table, spark)
+ applyFilterAndMasking(hiveTableRelation, table.identifier, spark)
case logicalRelation if hasResolvedDatasourceTable(logicalRelation) =>
val table = getDatasourceTable(logicalRelation)
if (table.isEmpty) {
logicalRelation
} else {
- applyFilterAndMasking(logicalRelation, table.get, spark)
+ applyFilterAndMasking(logicalRelation, table.get.identifier, spark)
+ }
+ case datasourceV2Relation if hasResolvedDatasourceV2Table(datasourceV2Relation) =>
Review Comment:
what does 'nit' stand for (if I may ask) ?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r940057263
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyRowFilterAndDataMasking.scala:
##########
@@ -30,29 +30,37 @@ import org.apache.kyuubi.plugin.spark.authz.util.RowFilterAndDataMaskingMarker
class RuleApplyRowFilterAndDataMasking(spark: SparkSession) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
- // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation with
+ // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation/DataSourceV2Relation with
// RowFilterAndDataMaskingMarker if it is not wrapped yet.
plan mapChildren {
case p: RowFilterAndDataMaskingMarker => p
case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
val table = getHiveTable(hiveTableRelation)
- applyFilterAndMasking(hiveTableRelation, table, spark)
+ applyFilterAndMasking(hiveTableRelation, table.identifier, spark)
case logicalRelation if hasResolvedDatasourceTable(logicalRelation) =>
val table = getDatasourceTable(logicalRelation)
if (table.isEmpty) {
logicalRelation
} else {
- applyFilterAndMasking(logicalRelation, table.get, spark)
+ applyFilterAndMasking(logicalRelation, table.get.identifier, spark)
+ }
+ case datasourceV2Relation if hasResolvedDatasourceV2Table(datasourceV2Relation) =>
+ val identifier = getDatasourceV2Identifier(datasourceV2Relation)
+ if (identifier.isEmpty) {
+ return datasourceV2Relation
+ } else {
+ val tableIdentifier = TableIdentifier(identifier.get.name,
+ Option(identifier.get.namespace.mkString(".")))
Review Comment:
1)for normal cases that a namespace part does not contain '.', for instance, abc.mnl.xyz, which is a 3-level namespace in the form of `/abc/mnl/xyz`.
2)for some abnormal cases that a namespace part contains '.', for instance, abc.\`mnl.xyz\`, which is a 2-level namespace in the form of `/abc/mnl.xyz`.
Case 1) and 2) are not the same, we only quote for 2) which is necessary but rarely happens in the real world.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] bowenliang123 commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
bowenliang123 commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r939939978
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyRowFilterAndDataMasking.scala:
##########
@@ -30,29 +30,40 @@ import org.apache.kyuubi.plugin.spark.authz.util.RowFilterAndDataMaskingMarker
class RuleApplyRowFilterAndDataMasking(spark: SparkSession) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
- // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation with
+ // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation/DataSourceV2Relation with
// RowFilterAndDataMaskingMarker if it is not wrapped yet.
plan mapChildren {
case p: RowFilterAndDataMaskingMarker => p
case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
val table = getHiveTable(hiveTableRelation)
- applyFilterAndMasking(hiveTableRelation, table, spark)
+ applyFilterAndMasking(hiveTableRelation, table.identifier, spark)
case logicalRelation if hasResolvedDatasourceTable(logicalRelation) =>
val table = getDatasourceTable(logicalRelation)
if (table.isEmpty) {
logicalRelation
} else {
- applyFilterAndMasking(logicalRelation, table.get, spark)
+ applyFilterAndMasking(logicalRelation, table.get.identifier, spark)
+ }
+ case datasourceV2Relation if hasResolvedDatasourceV2Table(datasourceV2Relation) =>
+ val table = getDatasourceV2Table(datasourceV2Relation)
+ if (table == null) {
+ datasourceV2Relation
+ } else {
+ val catalogDbTable = table.name.split("\\.")
Review Comment:
I wil have more investigation in `org.apache.spark.sql.connector.catalog.Identifier` in `DataSourceV2Relation` to see whether it is a possible path for this problem.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] bowenliang123 commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
bowenliang123 commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r940049818
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/util/AuthZUtils.scala:
##########
@@ -84,6 +85,14 @@ private[authz] object AuthZUtils {
getFieldVal[Option[CatalogTable]](plan, "catalogTable")
}
+ def hasResolvedDatasourceV2Table(plan: LogicalPlan): Boolean = {
+ plan.nodeName == "DataSourceV2Relation" && plan.resolved
+ }
+
+ def getDatasourceV2Identifier(plan: LogicalPlan): Option[Identifier] = {
Review Comment:
Good suggestion. Let me follow it.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] bowenliang123 commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
bowenliang123 commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r940017847
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyRowFilterAndDataMasking.scala:
##########
@@ -30,29 +30,37 @@ import org.apache.kyuubi.plugin.spark.authz.util.RowFilterAndDataMaskingMarker
class RuleApplyRowFilterAndDataMasking(spark: SparkSession) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
- // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation with
+ // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation/DataSourceV2Relation with
// RowFilterAndDataMaskingMarker if it is not wrapped yet.
plan mapChildren {
case p: RowFilterAndDataMaskingMarker => p
case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
val table = getHiveTable(hiveTableRelation)
- applyFilterAndMasking(hiveTableRelation, table, spark)
+ applyFilterAndMasking(hiveTableRelation, table.identifier, spark)
case logicalRelation if hasResolvedDatasourceTable(logicalRelation) =>
val table = getDatasourceTable(logicalRelation)
if (table.isEmpty) {
logicalRelation
} else {
- applyFilterAndMasking(logicalRelation, table.get, spark)
+ applyFilterAndMasking(logicalRelation, table.get.identifier, spark)
+ }
+ case datasourceV2Relation if hasResolvedDatasourceV2Table(datasourceV2Relation) =>
+ val identifier = getDatasourceV2Identifier(datasourceV2Relation)
+ if (identifier.isEmpty) {
+ return datasourceV2Relation
+ } else {
+ val tableIdentifier = TableIdentifier(identifier.get.name,
+ Option(identifier.get.namespace.mkString(".")))
Review Comment:
Couldn't locate the method you mentioned. Any further hints for `PrivillegeBuilder` like in which package of kyuubi or scala ?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] bowenliang123 commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
bowenliang123 commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r940020357
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/util/AuthZUtils.scala:
##########
@@ -84,6 +85,14 @@ private[authz] object AuthZUtils {
getFieldVal[Option[CatalogTable]](plan, "catalogTable")
}
+ def hasResolvedDatasourceV2Table(plan: LogicalPlan): Boolean = {
+ plan.nodeName == "DataSourceV2Relation" && plan.resolved
+ }
+
+ def getDatasourceV2Identifier(plan: LogicalPlan): Option[Identifier] = {
Review Comment:
It is a possible downside for using it, that I didn't realized. Anything I could improve the compatibility?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] bowenliang123 commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
bowenliang123 commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r940044691
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyRowFilterAndDataMasking.scala:
##########
@@ -30,29 +30,37 @@ import org.apache.kyuubi.plugin.spark.authz.util.RowFilterAndDataMaskingMarker
class RuleApplyRowFilterAndDataMasking(spark: SparkSession) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
- // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation with
+ // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation/DataSourceV2Relation with
// RowFilterAndDataMaskingMarker if it is not wrapped yet.
plan mapChildren {
case p: RowFilterAndDataMaskingMarker => p
case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
val table = getHiveTable(hiveTableRelation)
- applyFilterAndMasking(hiveTableRelation, table, spark)
+ applyFilterAndMasking(hiveTableRelation, table.identifier, spark)
case logicalRelation if hasResolvedDatasourceTable(logicalRelation) =>
val table = getDatasourceTable(logicalRelation)
if (table.isEmpty) {
logicalRelation
} else {
- applyFilterAndMasking(logicalRelation, table.get, spark)
+ applyFilterAndMasking(logicalRelation, table.get.identifier, spark)
+ }
+ case datasourceV2Relation if hasResolvedDatasourceV2Table(datasourceV2Relation) =>
Review Comment:
May I have your suggestion here. If you voted in @yikf ‘s guard condition pattern suggestion, i will follow it. @yaooqinn
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] bowenliang123 commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
bowenliang123 commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r939907558
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyRowFilterAndDataMasking.scala:
##########
@@ -30,29 +30,40 @@ import org.apache.kyuubi.plugin.spark.authz.util.RowFilterAndDataMaskingMarker
class RuleApplyRowFilterAndDataMasking(spark: SparkSession) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
- // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation with
+ // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation/DataSourceV2Relation with
// RowFilterAndDataMaskingMarker if it is not wrapped yet.
plan mapChildren {
case p: RowFilterAndDataMaskingMarker => p
case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
val table = getHiveTable(hiveTableRelation)
- applyFilterAndMasking(hiveTableRelation, table, spark)
+ applyFilterAndMasking(hiveTableRelation, table.identifier, spark)
case logicalRelation if hasResolvedDatasourceTable(logicalRelation) =>
val table = getDatasourceTable(logicalRelation)
if (table.isEmpty) {
logicalRelation
} else {
- applyFilterAndMasking(logicalRelation, table.get, spark)
+ applyFilterAndMasking(logicalRelation, table.get.identifier, spark)
+ }
+ case datasourceV2Relation if hasResolvedDatasourceV2Table(datasourceV2Relation) =>
+ val table = getDatasourceV2Table(datasourceV2Relation)
+ if (table == null) {
+ datasourceV2Relation
+ } else {
+ val catalogDbTable = table.name.split("\\.")
Review Comment:
I have also noticed the shortcomings. The current implementation is not elegant as espected.
In `org.apache.spark.sql.connector.catalog.Table`, `name()` method is not providing enough infomation to identify all these situations.
Suggestion 0: (current implementation)
forcing spliting table name in explicit 3 string items to identify catalog、db、table. In my case, I am using iceberg via spark_catlog, so the missing catalog name works fine.
Suggestion 1:
only apply to `org.apache.iceberg.spark.source.SparkTable` for iceberg tables instead all DatasourceV2 tables.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] bowenliang123 commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
bowenliang123 commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r940049818
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/util/AuthZUtils.scala:
##########
@@ -84,6 +85,14 @@ private[authz] object AuthZUtils {
getFieldVal[Option[CatalogTable]](plan, "catalogTable")
}
+ def hasResolvedDatasourceV2Table(plan: LogicalPlan): Boolean = {
+ plan.nodeName == "DataSourceV2Relation" && plan.resolved
+ }
+
+ def getDatasourceV2Identifier(plan: LogicalPlan): Option[Identifier] = {
Review Comment:
Good suggestion. Followed your example and avoiding import packages for spark version compatibility. Please have a check.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r939902714
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyRowFilterAndDataMasking.scala:
##########
@@ -30,29 +30,40 @@ import org.apache.kyuubi.plugin.spark.authz.util.RowFilterAndDataMaskingMarker
class RuleApplyRowFilterAndDataMasking(spark: SparkSession) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
- // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation with
+ // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation/DataSourceV2Relation with
// RowFilterAndDataMaskingMarker if it is not wrapped yet.
plan mapChildren {
case p: RowFilterAndDataMaskingMarker => p
case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
val table = getHiveTable(hiveTableRelation)
- applyFilterAndMasking(hiveTableRelation, table, spark)
+ applyFilterAndMasking(hiveTableRelation, table.identifier, spark)
case logicalRelation if hasResolvedDatasourceTable(logicalRelation) =>
val table = getDatasourceTable(logicalRelation)
if (table.isEmpty) {
logicalRelation
} else {
- applyFilterAndMasking(logicalRelation, table.get, spark)
+ applyFilterAndMasking(logicalRelation, table.get.identifier, spark)
+ }
+ case datasourceV2Relation if hasResolvedDatasourceV2Table(datasourceV2Relation) =>
+ val table = getDatasourceV2Table(datasourceV2Relation)
+ if (table == null) {
+ datasourceV2Relation
+ } else {
+ val catalogDbTable = table.name.split("\\.")
Review Comment:
```java
/**
* A name to identify this table. Implementations should provide a meaningful name, like the
* database and table name from catalog, or the location of files for this table.
*/
```
Check the comments above from the spark codebase, the cases below seem not to be handled,
1. the location of files for this table
2. when the catalog name is absent
3. backtick-quoted name parts, such as cat1.db.\`abc.xyz\`
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] bowenliang123 commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
bowenliang123 commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r940031212
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyRowFilterAndDataMasking.scala:
##########
@@ -30,29 +30,37 @@ import org.apache.kyuubi.plugin.spark.authz.util.RowFilterAndDataMaskingMarker
class RuleApplyRowFilterAndDataMasking(spark: SparkSession) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
- // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation with
+ // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation/DataSourceV2Relation with
// RowFilterAndDataMaskingMarker if it is not wrapped yet.
plan mapChildren {
case p: RowFilterAndDataMaskingMarker => p
case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
val table = getHiveTable(hiveTableRelation)
- applyFilterAndMasking(hiveTableRelation, table, spark)
+ applyFilterAndMasking(hiveTableRelation, table.identifier, spark)
case logicalRelation if hasResolvedDatasourceTable(logicalRelation) =>
val table = getDatasourceTable(logicalRelation)
if (table.isEmpty) {
logicalRelation
} else {
- applyFilterAndMasking(logicalRelation, table.get, spark)
+ applyFilterAndMasking(logicalRelation, table.get.identifier, spark)
+ }
+ case datasourceV2Relation if hasResolvedDatasourceV2Table(datasourceV2Relation) =>
+ val identifier = getDatasourceV2Identifier(datasourceV2Relation)
+ if (identifier.isEmpty) {
+ return datasourceV2Relation
+ } else {
+ val tableIdentifier = TableIdentifier(identifier.get.name,
+ Option(identifier.get.namespace.mkString(".")))
Review Comment:
Thx for guidance. But is it a private method `quote` of PrivilegesBuilder which is inaccessible inside `RuleApplyRowFilterAndDataMasking` ? Whether I should using it forcingly convert it from `private` to `public` or just replicating it?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r940033542
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/util/AuthZUtils.scala:
##########
@@ -84,6 +85,14 @@ private[authz] object AuthZUtils {
getFieldVal[Option[CatalogTable]](plan, "catalogTable")
}
+ def hasResolvedDatasourceV2Table(plan: LogicalPlan): Boolean = {
+ plan.nodeName == "DataSourceV2Relation" && plan.resolved
+ }
+
+ def getDatasourceV2Identifier(plan: LogicalPlan): Option[Identifier] = {
Review Comment:
I don't whether it works or not, but something like this?
```scala
def quoteIfNeeded(part: String): String = {
if (part.matches("[a-zA-Z0-9_]+") && !part.matches("\\d+")) {
part
} else {
s"`${part.replace("`", "``")}`"
}
}
def quote(parts: Seq[String]): String = {
parts.map(quoteIfNeeded).mkString(".")
}
def getDatasourceV2Identifier(plan: LogicalPlan): Option[TableIdentifier] = {
val identifier = getFieldVal[Option[AnyRef]](plan, "identifier")
identifier.map { id =>
val namespaces = invoke(id, "namespaces").asInstanceOf[Array[String]]
val table = invoke(id, "name").asInstanceOf[String]
TableIdentifier(table, Some(quote(namespaces)))
}
}
```
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on pull request #3197: [KYUUBI #3186] Support applying Row-level Filter and Data Masking policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#issuecomment-1209253030
@bowenliang123 thanks for the contribution, and the patience all the way through.
Merged to master.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] bowenliang123 commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
bowenliang123 commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r940075213
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyRowFilterAndDataMasking.scala:
##########
@@ -30,29 +30,37 @@ import org.apache.kyuubi.plugin.spark.authz.util.RowFilterAndDataMaskingMarker
class RuleApplyRowFilterAndDataMasking(spark: SparkSession) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
- // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation with
+ // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation/DataSourceV2Relation with
// RowFilterAndDataMaskingMarker if it is not wrapped yet.
plan mapChildren {
case p: RowFilterAndDataMaskingMarker => p
case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
val table = getHiveTable(hiveTableRelation)
- applyFilterAndMasking(hiveTableRelation, table, spark)
+ applyFilterAndMasking(hiveTableRelation, table.identifier, spark)
case logicalRelation if hasResolvedDatasourceTable(logicalRelation) =>
val table = getDatasourceTable(logicalRelation)
if (table.isEmpty) {
logicalRelation
} else {
- applyFilterAndMasking(logicalRelation, table.get, spark)
+ applyFilterAndMasking(logicalRelation, table.get.identifier, spark)
+ }
+ case datasourceV2Relation if hasResolvedDatasourceV2Table(datasourceV2Relation) =>
Review Comment:
OK. It has bee refactored without importing DataSourceV2Relation.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r939902714
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyRowFilterAndDataMasking.scala:
##########
@@ -30,29 +30,40 @@ import org.apache.kyuubi.plugin.spark.authz.util.RowFilterAndDataMaskingMarker
class RuleApplyRowFilterAndDataMasking(spark: SparkSession) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
- // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation with
+ // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation/DataSourceV2Relation with
// RowFilterAndDataMaskingMarker if it is not wrapped yet.
plan mapChildren {
case p: RowFilterAndDataMaskingMarker => p
case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
val table = getHiveTable(hiveTableRelation)
- applyFilterAndMasking(hiveTableRelation, table, spark)
+ applyFilterAndMasking(hiveTableRelation, table.identifier, spark)
case logicalRelation if hasResolvedDatasourceTable(logicalRelation) =>
val table = getDatasourceTable(logicalRelation)
if (table.isEmpty) {
logicalRelation
} else {
- applyFilterAndMasking(logicalRelation, table.get, spark)
+ applyFilterAndMasking(logicalRelation, table.get.identifier, spark)
+ }
+ case datasourceV2Relation if hasResolvedDatasourceV2Table(datasourceV2Relation) =>
+ val table = getDatasourceV2Table(datasourceV2Relation)
+ if (table == null) {
+ datasourceV2Relation
+ } else {
+ val catalogDbTable = table.name.split("\\.")
Review Comment:
```java
/**
* A name to identify this table. Implementations should provide a meaningful name, like the
* database and table name from catalog, or the location of files for this table.
*/
```
Check the comments above from the spark codebase, the cases below seem not to be handled,
1. the location of files for this table
2. when the catalog name is absent
3. backtick-quoted name parts, such as "cat1.db.`abc.xyz`
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] bowenliang123 commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
bowenliang123 commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r940004682
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyRowFilterAndDataMasking.scala:
##########
@@ -30,29 +30,37 @@ import org.apache.kyuubi.plugin.spark.authz.util.RowFilterAndDataMaskingMarker
class RuleApplyRowFilterAndDataMasking(spark: SparkSession) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
- // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation with
+ // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation/DataSourceV2Relation with
// RowFilterAndDataMaskingMarker if it is not wrapped yet.
plan mapChildren {
case p: RowFilterAndDataMaskingMarker => p
case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
val table = getHiveTable(hiveTableRelation)
- applyFilterAndMasking(hiveTableRelation, table, spark)
+ applyFilterAndMasking(hiveTableRelation, table.identifier, spark)
case logicalRelation if hasResolvedDatasourceTable(logicalRelation) =>
val table = getDatasourceTable(logicalRelation)
if (table.isEmpty) {
logicalRelation
} else {
- applyFilterAndMasking(logicalRelation, table.get, spark)
+ applyFilterAndMasking(logicalRelation, table.get.identifier, spark)
+ }
+ case datasourceV2Relation if hasResolvedDatasourceV2Table(datasourceV2Relation) =>
Review Comment:
Good suggestion though. I am still using the `hasXXXX` pattern method to follow the existed method for HiveTableRelation and LogicalRelation as hasResolvedHiveTable and hasResolvedDatasourceTable. And I couldn't see any clarification improvement by using guard pattern for this.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] bowenliang123 commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
bowenliang123 commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r940042795
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyRowFilterAndDataMasking.scala:
##########
@@ -30,29 +30,37 @@ import org.apache.kyuubi.plugin.spark.authz.util.RowFilterAndDataMaskingMarker
class RuleApplyRowFilterAndDataMasking(spark: SparkSession) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
- // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation with
+ // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation/DataSourceV2Relation with
// RowFilterAndDataMaskingMarker if it is not wrapped yet.
plan mapChildren {
case p: RowFilterAndDataMaskingMarker => p
case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
val table = getHiveTable(hiveTableRelation)
- applyFilterAndMasking(hiveTableRelation, table, spark)
+ applyFilterAndMasking(hiveTableRelation, table.identifier, spark)
case logicalRelation if hasResolvedDatasourceTable(logicalRelation) =>
val table = getDatasourceTable(logicalRelation)
if (table.isEmpty) {
logicalRelation
} else {
- applyFilterAndMasking(logicalRelation, table.get, spark)
+ applyFilterAndMasking(logicalRelation, table.get.identifier, spark)
+ }
+ case datasourceV2Relation if hasResolvedDatasourceV2Table(datasourceV2Relation) =>
+ val identifier = getDatasourceV2Identifier(datasourceV2Relation)
+ if (identifier.isEmpty) {
+ return datasourceV2Relation
+ } else {
+ val tableIdentifier = TableIdentifier(identifier.get.name,
+ Option(identifier.get.namespace.mkString(".")))
Review Comment:
Another problem related is `quote` automatically determine `quoteIfNeeded` and add quotes to it. However this escaping is not probablely necessary for naming and looking up of ranger policy. Quotes may cause confusions to it with inconsistency in 拿命namespaces and tables.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] bowenliang123 commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
bowenliang123 commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r939907558
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyRowFilterAndDataMasking.scala:
##########
@@ -30,29 +30,40 @@ import org.apache.kyuubi.plugin.spark.authz.util.RowFilterAndDataMaskingMarker
class RuleApplyRowFilterAndDataMasking(spark: SparkSession) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
- // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation with
+ // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation/DataSourceV2Relation with
// RowFilterAndDataMaskingMarker if it is not wrapped yet.
plan mapChildren {
case p: RowFilterAndDataMaskingMarker => p
case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
val table = getHiveTable(hiveTableRelation)
- applyFilterAndMasking(hiveTableRelation, table, spark)
+ applyFilterAndMasking(hiveTableRelation, table.identifier, spark)
case logicalRelation if hasResolvedDatasourceTable(logicalRelation) =>
val table = getDatasourceTable(logicalRelation)
if (table.isEmpty) {
logicalRelation
} else {
- applyFilterAndMasking(logicalRelation, table.get, spark)
+ applyFilterAndMasking(logicalRelation, table.get.identifier, spark)
+ }
+ case datasourceV2Relation if hasResolvedDatasourceV2Table(datasourceV2Relation) =>
+ val table = getDatasourceV2Table(datasourceV2Relation)
+ if (table == null) {
+ datasourceV2Relation
+ } else {
+ val catalogDbTable = table.name.split("\\.")
Review Comment:
I have also noticed the shortcomings. The current implementation is not elegant as espected.
In `org.apache.spark.sql.connector.catalog.Table`, `name()` method is not providing enough infomation to identify all these situations.
Suggestion 0: (current implementation)
forcing spliting table name to identify catalog、db、table. In my case, I am using iceberg via spark_catlog, so the missing catalog name works fine.
Suggestion 1:
only apply to `org.apache.iceberg.spark.source.SparkTable` for iceberg tables instead all DatasourceV2 tables.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] bowenliang123 commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
bowenliang123 commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r939979285
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyRowFilterAndDataMasking.scala:
##########
@@ -30,29 +30,40 @@ import org.apache.kyuubi.plugin.spark.authz.util.RowFilterAndDataMaskingMarker
class RuleApplyRowFilterAndDataMasking(spark: SparkSession) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
- // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation with
+ // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation/DataSourceV2Relation with
// RowFilterAndDataMaskingMarker if it is not wrapped yet.
plan mapChildren {
case p: RowFilterAndDataMaskingMarker => p
case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
val table = getHiveTable(hiveTableRelation)
- applyFilterAndMasking(hiveTableRelation, table, spark)
+ applyFilterAndMasking(hiveTableRelation, table.identifier, spark)
case logicalRelation if hasResolvedDatasourceTable(logicalRelation) =>
val table = getDatasourceTable(logicalRelation)
if (table.isEmpty) {
logicalRelation
} else {
- applyFilterAndMasking(logicalRelation, table.get, spark)
+ applyFilterAndMasking(logicalRelation, table.get.identifier, spark)
+ }
+ case datasourceV2Relation if hasResolvedDatasourceV2Table(datasourceV2Relation) =>
+ val table = getDatasourceV2Table(datasourceV2Relation)
+ if (table == null) {
+ datasourceV2Relation
+ } else {
+ val catalogDbTable = table.name.split("\\.")
Review Comment:
Changed to rely on `org.apache.spark.sql.connector.catalog.Identifier` of `DataSourceV2Relation` instead of unconvincing name() in`Table`.
Resolving the previously listed problems by:
1. using the name of Identifier as accurate table name
2. concating namespaces in Identifier as the full database name (joining with "." refering to `IdentifierImpl` in Spark)
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] bowenliang123 commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
bowenliang123 commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r940073290
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyRowFilterAndDataMasking.scala:
##########
@@ -30,29 +30,37 @@ import org.apache.kyuubi.plugin.spark.authz.util.RowFilterAndDataMaskingMarker
class RuleApplyRowFilterAndDataMasking(spark: SparkSession) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
- // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation with
+ // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation/DataSourceV2Relation with
// RowFilterAndDataMaskingMarker if it is not wrapped yet.
plan mapChildren {
case p: RowFilterAndDataMaskingMarker => p
case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
val table = getHiveTable(hiveTableRelation)
- applyFilterAndMasking(hiveTableRelation, table, spark)
+ applyFilterAndMasking(hiveTableRelation, table.identifier, spark)
case logicalRelation if hasResolvedDatasourceTable(logicalRelation) =>
val table = getDatasourceTable(logicalRelation)
if (table.isEmpty) {
logicalRelation
} else {
- applyFilterAndMasking(logicalRelation, table.get, spark)
+ applyFilterAndMasking(logicalRelation, table.get.identifier, spark)
+ }
+ case datasourceV2Relation if hasResolvedDatasourceV2Table(datasourceV2Relation) =>
+ val identifier = getDatasourceV2Identifier(datasourceV2Relation)
+ if (identifier.isEmpty) {
+ return datasourceV2Relation
+ } else {
+ val tableIdentifier = TableIdentifier(identifier.get.name,
+ Option(identifier.get.namespace.mkString(".")))
Review Comment:
Agree with your guidance. And moved `quoted` and `quoteIfNeeded` to AuthZUtils as pulbic methods.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r940002993
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyRowFilterAndDataMasking.scala:
##########
@@ -30,29 +30,37 @@ import org.apache.kyuubi.plugin.spark.authz.util.RowFilterAndDataMaskingMarker
class RuleApplyRowFilterAndDataMasking(spark: SparkSession) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
- // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation with
+ // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation/DataSourceV2Relation with
// RowFilterAndDataMaskingMarker if it is not wrapped yet.
plan mapChildren {
case p: RowFilterAndDataMaskingMarker => p
case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
val table = getHiveTable(hiveTableRelation)
- applyFilterAndMasking(hiveTableRelation, table, spark)
+ applyFilterAndMasking(hiveTableRelation, table.identifier, spark)
case logicalRelation if hasResolvedDatasourceTable(logicalRelation) =>
val table = getDatasourceTable(logicalRelation)
if (table.isEmpty) {
logicalRelation
} else {
- applyFilterAndMasking(logicalRelation, table.get, spark)
+ applyFilterAndMasking(logicalRelation, table.get.identifier, spark)
+ }
+ case datasourceV2Relation if hasResolvedDatasourceV2Table(datasourceV2Relation) =>
+ val identifier = getDatasourceV2Identifier(datasourceV2Relation)
+ if (identifier.isEmpty) {
+ return datasourceV2Relation
+ } else {
+ val tableIdentifier = TableIdentifier(identifier.get.name,
+ Option(identifier.get.namespace.mkString(".")))
Review Comment:
Better use `quote` from PrivillegeBuilder
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r940046907
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyRowFilterAndDataMasking.scala:
##########
@@ -30,29 +30,37 @@ import org.apache.kyuubi.plugin.spark.authz.util.RowFilterAndDataMaskingMarker
class RuleApplyRowFilterAndDataMasking(spark: SparkSession) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
- // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation with
+ // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation/DataSourceV2Relation with
// RowFilterAndDataMaskingMarker if it is not wrapped yet.
plan mapChildren {
case p: RowFilterAndDataMaskingMarker => p
case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
val table = getHiveTable(hiveTableRelation)
- applyFilterAndMasking(hiveTableRelation, table, spark)
+ applyFilterAndMasking(hiveTableRelation, table.identifier, spark)
case logicalRelation if hasResolvedDatasourceTable(logicalRelation) =>
val table = getDatasourceTable(logicalRelation)
if (table.isEmpty) {
logicalRelation
} else {
- applyFilterAndMasking(logicalRelation, table.get, spark)
+ applyFilterAndMasking(logicalRelation, table.get.identifier, spark)
+ }
+ case datasourceV2Relation if hasResolvedDatasourceV2Table(datasourceV2Relation) =>
Review Comment:
let's keep it for multiple spark version support
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] bowenliang123 commented on pull request #3197: [KYUUBI #3186] Support applying Row-level Filter and Data Masking policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
bowenliang123 commented on PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#issuecomment-1208850934
Extended PR title, as data masking policies has been also tested locally.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r939894226
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyRowFilterAndDataMasking.scala:
##########
@@ -30,29 +30,40 @@ import org.apache.kyuubi.plugin.spark.authz.util.RowFilterAndDataMaskingMarker
class RuleApplyRowFilterAndDataMasking(spark: SparkSession) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
- // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation with
+ // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation/DataSourceV2Relation with
// RowFilterAndDataMaskingMarker if it is not wrapped yet.
plan mapChildren {
case p: RowFilterAndDataMaskingMarker => p
case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
val table = getHiveTable(hiveTableRelation)
- applyFilterAndMasking(hiveTableRelation, table, spark)
+ applyFilterAndMasking(hiveTableRelation, table.identifier, spark)
case logicalRelation if hasResolvedDatasourceTable(logicalRelation) =>
val table = getDatasourceTable(logicalRelation)
if (table.isEmpty) {
logicalRelation
} else {
- applyFilterAndMasking(logicalRelation, table.get, spark)
+ applyFilterAndMasking(logicalRelation, table.get.identifier, spark)
+ }
+ case datasourceV2Relation if hasResolvedDatasourceV2Table(datasourceV2Relation) =>
+ val table = getDatasourceV2Table(datasourceV2Relation)
+ if (table == null) {
Review Comment:
this seems unnecessary
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r940001304
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/util/AuthZUtils.scala:
##########
@@ -84,6 +85,14 @@ private[authz] object AuthZUtils {
getFieldVal[Option[CatalogTable]](plan, "catalogTable")
}
+ def hasResolvedDatasourceV2Table(plan: LogicalPlan): Boolean = {
+ plan.nodeName == "DataSourceV2Relation" && plan.resolved
+ }
+
+ def getDatasourceV2Identifier(plan: LogicalPlan): Option[Identifier] = {
Review Comment:
If we use Identifier here, we need to update the doc that we do not support spark 2 anymore
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r940822466
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyRowFilterAndDataMasking.scala:
##########
@@ -30,29 +30,35 @@ import org.apache.kyuubi.plugin.spark.authz.util.RowFilterAndDataMaskingMarker
class RuleApplyRowFilterAndDataMasking(spark: SparkSession) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
- // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation with
+ // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation/DataSourceV2Relation with
// RowFilterAndDataMaskingMarker if it is not wrapped yet.
plan mapChildren {
case p: RowFilterAndDataMaskingMarker => p
case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
val table = getHiveTable(hiveTableRelation)
- applyFilterAndMasking(hiveTableRelation, table, spark)
+ applyFilterAndMasking(hiveTableRelation, table.identifier, spark)
case logicalRelation if hasResolvedDatasourceTable(logicalRelation) =>
val table = getDatasourceTable(logicalRelation)
if (table.isEmpty) {
logicalRelation
} else {
- applyFilterAndMasking(logicalRelation, table.get, spark)
+ applyFilterAndMasking(logicalRelation, table.get.identifier, spark)
+ }
+ case datasourceV2Relation if hasResolvedDatasourceV2Table(datasourceV2Relation) =>
Review Comment:
nit: dataSourceV2Relation, hasResolvedDataSourceV2Table and getDataSourceV2Identifier
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] codecov-commenter commented on pull request #3197: [KYUUBI #3186] Support applying Row-level Filter and Data Masking policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#issuecomment-1208851956
# [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/3197?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#3197](https://codecov.io/gh/apache/incubator-kyuubi/pull/3197?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1d47630) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/a46d6550c773d20653b28720662cee9c1e88daba?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a46d655) will **decrease** coverage by `0.03%`.
> The diff coverage is `29.41%`.
```diff
@@ Coverage Diff @@
## master #3197 +/- ##
============================================
- Coverage 51.40% 51.36% -0.04%
Complexity 6 6
============================================
Files 456 456
Lines 25422 25432 +10
Branches 3540 3543 +3
============================================
- Hits 13067 13064 -3
- Misses 11097 11106 +9
- Partials 1258 1262 +4
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/3197?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../kyuubi/plugin/spark/authz/PrivilegesBuilder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3197/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZW5zaW9ucy9zcGFyay9reXV1Ymktc3BhcmstYXV0aHovc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvcGx1Z2luL3NwYXJrL2F1dGh6L1ByaXZpbGVnZXNCdWlsZGVyLnNjYWxh) | `69.37% <ø> (-0.07%)` | :arrow_down: |
| [...uthz/ranger/RuleApplyRowFilterAndDataMasking.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3197/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZW5zaW9ucy9zcGFyay9reXV1Ymktc3BhcmstYXV0aHovc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvcGx1Z2luL3NwYXJrL2F1dGh6L3Jhbmdlci9SdWxlQXBwbHlSb3dGaWx0ZXJBbmREYXRhTWFza2luZy5zY2FsYQ==) | `80.00% <28.57%> (-13.55%)` | :arrow_down: |
| [...he/kyuubi/plugin/spark/authz/util/AuthZUtils.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3197/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZW5zaW9ucy9zcGFyay9reXV1Ymktc3BhcmstYXV0aHovc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvcGx1Z2luL3NwYXJrL2F1dGh6L3V0aWwvQXV0aFpVdGlscy5zY2FsYQ==) | `43.85% <30.00%> (-2.95%)` | :arrow_down: |
| [...rg/apache/kyuubi/ctl/cmd/log/LogBatchCommand.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3197/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWN0bC9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jdGwvY21kL2xvZy9Mb2dCYXRjaENvbW1hbmQuc2NhbGE=) | `78.00% <0.00%> (-2.00%)` | :arrow_down: |
| [...ache/kyuubi/operation/KyuubiOperationManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3197/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vS3l1dWJpT3BlcmF0aW9uTWFuYWdlci5zY2FsYQ==) | `80.82% <0.00%> (-1.37%)` | :arrow_down: |
| [...ain/scala/org/apache/kyuubi/engine/EngineRef.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3197/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9lbmdpbmUvRW5naW5lUmVmLnNjYWxh) | `75.23% <0.00%> (-0.96%)` | :arrow_down: |
| [...n/scala/org/apache/kyuubi/engine/ProcBuilder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3197/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9lbmdpbmUvUHJvY0J1aWxkZXIuc2NhbGE=) | `83.12% <0.00%> (-0.63%)` | :arrow_down: |
| [...a/org/apache/kyuubi/service/TFrontendService.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3197/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL1RGcm9udGVuZFNlcnZpY2Uuc2NhbGE=) | `91.17% <0.00%> (-0.30%)` | :arrow_down: |
| [...org/apache/kyuubi/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3197/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vRXhlY3V0ZVN0YXRlbWVudC5zY2FsYQ==) | `80.51% <0.00%> (ø)` | |
| [...in/scala/org/apache/kyuubi/config/KyuubiConf.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3197/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jb25maWcvS3l1dWJpQ29uZi5zY2FsYQ==) | `97.34% <0.00%> (+0.09%)` | :arrow_up: |
| ... and [1 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/3197/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: Codecov can now indicate which changes are the most critical in Pull Requests. [Learn more](https://about.codecov.io/product/feature/runtime-insights/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] bowenliang123 commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
bowenliang123 commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r940020357
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/util/AuthZUtils.scala:
##########
@@ -84,6 +85,14 @@ private[authz] object AuthZUtils {
getFieldVal[Option[CatalogTable]](plan, "catalogTable")
}
+ def hasResolvedDatasourceV2Table(plan: LogicalPlan): Boolean = {
+ plan.nodeName == "DataSourceV2Relation" && plan.resolved
+ }
+
+ def getDatasourceV2Identifier(plan: LogicalPlan): Option[Identifier] = {
Review Comment:
It is a possible downside for using it, that I didn't realized. Anything I coulld do to improve the compatibility?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn closed pull request #3197: [KYUUBI #3186] Support applying Row-level Filter and Data Masking policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
yaooqinn closed pull request #3197: [KYUUBI #3186] Support applying Row-level Filter and Data Masking policies for DatasourceV2 in Authz module
URL: https://github.com/apache/incubator-kyuubi/pull/3197
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] bowenliang123 commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
bowenliang123 commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r939979285
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyRowFilterAndDataMasking.scala:
##########
@@ -30,29 +30,40 @@ import org.apache.kyuubi.plugin.spark.authz.util.RowFilterAndDataMaskingMarker
class RuleApplyRowFilterAndDataMasking(spark: SparkSession) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
- // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation with
+ // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation/DataSourceV2Relation with
// RowFilterAndDataMaskingMarker if it is not wrapped yet.
plan mapChildren {
case p: RowFilterAndDataMaskingMarker => p
case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
val table = getHiveTable(hiveTableRelation)
- applyFilterAndMasking(hiveTableRelation, table, spark)
+ applyFilterAndMasking(hiveTableRelation, table.identifier, spark)
case logicalRelation if hasResolvedDatasourceTable(logicalRelation) =>
val table = getDatasourceTable(logicalRelation)
if (table.isEmpty) {
logicalRelation
} else {
- applyFilterAndMasking(logicalRelation, table.get, spark)
+ applyFilterAndMasking(logicalRelation, table.get.identifier, spark)
+ }
+ case datasourceV2Relation if hasResolvedDatasourceV2Table(datasourceV2Relation) =>
+ val table = getDatasourceV2Table(datasourceV2Relation)
+ if (table == null) {
+ datasourceV2Relation
+ } else {
+ val catalogDbTable = table.name.split("\\.")
Review Comment:
Changed to rely on `org.apache.spark.sql.connector.catalog.Identifier` of `DataSourceV2Relation` instead of unconvincing name() in`Table`.
Resolving the previously listed problems by:
1. using the name of Identifier as accurate table name
2. concating namespaces in Identifier as the full database name
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r940002052
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyRowFilterAndDataMasking.scala:
##########
@@ -30,29 +30,37 @@ import org.apache.kyuubi.plugin.spark.authz.util.RowFilterAndDataMaskingMarker
class RuleApplyRowFilterAndDataMasking(spark: SparkSession) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
- // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation with
+ // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation/DataSourceV2Relation with
// RowFilterAndDataMaskingMarker if it is not wrapped yet.
plan mapChildren {
case p: RowFilterAndDataMaskingMarker => p
case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
val table = getHiveTable(hiveTableRelation)
- applyFilterAndMasking(hiveTableRelation, table, spark)
+ applyFilterAndMasking(hiveTableRelation, table.identifier, spark)
case logicalRelation if hasResolvedDatasourceTable(logicalRelation) =>
val table = getDatasourceTable(logicalRelation)
if (table.isEmpty) {
logicalRelation
} else {
- applyFilterAndMasking(logicalRelation, table.get, spark)
+ applyFilterAndMasking(logicalRelation, table.get.identifier, spark)
+ }
+ case datasourceV2Relation if hasResolvedDatasourceV2Table(datasourceV2Relation) =>
+ val identifier = getDatasourceV2Identifier(datasourceV2Relation)
+ if (identifier.isEmpty) {
+ return datasourceV2Relation
Review Comment:
nit: return is unnecessary
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r940021424
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyRowFilterAndDataMasking.scala:
##########
@@ -30,29 +30,37 @@ import org.apache.kyuubi.plugin.spark.authz.util.RowFilterAndDataMaskingMarker
class RuleApplyRowFilterAndDataMasking(spark: SparkSession) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
- // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation with
+ // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation/DataSourceV2Relation with
// RowFilterAndDataMaskingMarker if it is not wrapped yet.
plan mapChildren {
case p: RowFilterAndDataMaskingMarker => p
case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
val table = getHiveTable(hiveTableRelation)
- applyFilterAndMasking(hiveTableRelation, table, spark)
+ applyFilterAndMasking(hiveTableRelation, table.identifier, spark)
case logicalRelation if hasResolvedDatasourceTable(logicalRelation) =>
val table = getDatasourceTable(logicalRelation)
if (table.isEmpty) {
logicalRelation
} else {
- applyFilterAndMasking(logicalRelation, table.get, spark)
+ applyFilterAndMasking(logicalRelation, table.get.identifier, spark)
+ }
+ case datasourceV2Relation if hasResolvedDatasourceV2Table(datasourceV2Relation) =>
+ val identifier = getDatasourceV2Identifier(datasourceV2Relation)
+ if (identifier.isEmpty) {
+ return datasourceV2Relation
+ } else {
+ val tableIdentifier = TableIdentifier(identifier.get.name,
+ Option(identifier.get.namespace.mkString(".")))
Review Comment:
org.apache.kyuubi.plugin.spark.authz.PrivilegesBuilder#quote, and we can move it to AuthzUtils
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] bowenliang123 commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
bowenliang123 commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r939902746
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyRowFilterAndDataMasking.scala:
##########
@@ -30,29 +30,40 @@ import org.apache.kyuubi.plugin.spark.authz.util.RowFilterAndDataMaskingMarker
class RuleApplyRowFilterAndDataMasking(spark: SparkSession) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
- // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation with
+ // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation/DataSourceV2Relation with
// RowFilterAndDataMaskingMarker if it is not wrapped yet.
plan mapChildren {
case p: RowFilterAndDataMaskingMarker => p
case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
val table = getHiveTable(hiveTableRelation)
- applyFilterAndMasking(hiveTableRelation, table, spark)
+ applyFilterAndMasking(hiveTableRelation, table.identifier, spark)
case logicalRelation if hasResolvedDatasourceTable(logicalRelation) =>
val table = getDatasourceTable(logicalRelation)
if (table.isEmpty) {
logicalRelation
} else {
- applyFilterAndMasking(logicalRelation, table.get, spark)
+ applyFilterAndMasking(logicalRelation, table.get.identifier, spark)
+ }
+ case datasourceV2Relation if hasResolvedDatasourceV2Table(datasourceV2Relation) =>
+ val table = getDatasourceV2Table(datasourceV2Relation)
+ if (table == null) {
Review Comment:
well noted and has removed this `table == null` condition.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] bowenliang123 commented on a diff in pull request #3197: [KYUUBI #3186] Support applying Row-level Filter policies for DatasourceV2 in Authz module
Posted by GitBox <gi...@apache.org>.
bowenliang123 commented on code in PR #3197:
URL: https://github.com/apache/incubator-kyuubi/pull/3197#discussion_r939979285
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyRowFilterAndDataMasking.scala:
##########
@@ -30,29 +30,40 @@ import org.apache.kyuubi.plugin.spark.authz.util.RowFilterAndDataMaskingMarker
class RuleApplyRowFilterAndDataMasking(spark: SparkSession) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = {
- // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation with
+ // Apply FilterAndMasking and wrap HiveTableRelation/LogicalRelation/DataSourceV2Relation with
// RowFilterAndDataMaskingMarker if it is not wrapped yet.
plan mapChildren {
case p: RowFilterAndDataMaskingMarker => p
case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
val table = getHiveTable(hiveTableRelation)
- applyFilterAndMasking(hiveTableRelation, table, spark)
+ applyFilterAndMasking(hiveTableRelation, table.identifier, spark)
case logicalRelation if hasResolvedDatasourceTable(logicalRelation) =>
val table = getDatasourceTable(logicalRelation)
if (table.isEmpty) {
logicalRelation
} else {
- applyFilterAndMasking(logicalRelation, table.get, spark)
+ applyFilterAndMasking(logicalRelation, table.get.identifier, spark)
+ }
+ case datasourceV2Relation if hasResolvedDatasourceV2Table(datasourceV2Relation) =>
+ val table = getDatasourceV2Table(datasourceV2Relation)
+ if (table == null) {
+ datasourceV2Relation
+ } else {
+ val catalogDbTable = table.name.split("\\.")
Review Comment:
Changed to rely on `org.apache.spark.sql.connector.catalog.Identifier` of `DataSourceV2Relation` instead of unconvincing name() in`Table`.
Resolving the previously listed problems by:
1. using the name of Identifier as accurate table name
2. concating namespaces in Identifier as the full database name (joining with "." refering to `IdentifierImpl` in Spark)
3. no absent catalog name problem since `org.apache.spark.sql.connector.catalog.Identifier` is identifying object inside the catalog
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org