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/12/30 03:18:22 UTC
[GitHub] [kyuubi] yaooqinn opened a new pull request, #4044: [WIP][Authz] Serde for queries
yaooqinn opened a new pull request, #4044:
URL: https://github.com/apache/kyuubi/pull/4044
<!--
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/CONTRIBUTING.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.
-->
### _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
- [ ] [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] [kyuubi] bowenliang123 commented on pull request #4044: [WIP][Authz] Serde for queries
Posted by GitBox <gi...@apache.org>.
bowenliang123 commented on PR #4044:
URL: https://github.com/apache/kyuubi/pull/4044#issuecomment-1367935155
We should also consider ScanSpec for building function privileges of perm function usages in queries, as in pending PR 3633 (https://github.com/apache/kyuubi/pull/3633). The core logic to this is scanning logical plan by `transformAllExpressions` and build function privileges for UDFs.
--
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] [kyuubi] bowenliang123 commented on pull request #4044: [Authz] Serde for queries
Posted by GitBox <gi...@apache.org>.
bowenliang123 commented on PR #4044:
URL: https://github.com/apache/kyuubi/pull/4044#issuecomment-1370564463
LGTM. Just some minor comments. Let's merge it after passing the CI.
Glad to see this PR reaching a milestone for Serde refactoring, by completely removing hardcoded rules and extraction of `v2Commands` and `IcebergCommands` and using spec generation instead.
Big applause to @yaooqinn for your elegant redesign and all the hard work. 👏🏻 🍻
--
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] [kyuubi] yaooqinn commented on pull request #4044: [WIP][Authz] Serde for queries
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on PR #4044:
URL: https://github.com/apache/kyuubi/pull/4044#issuecomment-1369321546
> We should also consider ScanSpec for building function privileges of perm function usages in queries
How can ScanSpec configure functions?
--
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] [kyuubi] bowenliang123 commented on pull request #4044: [WIP][Authz] Serde for queries
Posted by GitBox <gi...@apache.org>.
bowenliang123 commented on PR #4044:
URL: https://github.com/apache/kyuubi/pull/4044#issuecomment-1369333854
> How can ScanSpec configure functions?
If `ScanSpec` is used for extracting all related resources, functions could be considered one type among them.
Or if it is limited to scanned resources, the used function support in queries can be introduced later.
--
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] [kyuubi] bowenliang123 commented on a diff in pull request #4044: [Authz] Serde for queries
Posted by GitBox <gi...@apache.org>.
bowenliang123 commented on code in PR #4044:
URL: https://github.com/apache/kyuubi/pull/4044#discussion_r1061188043
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/serde/CommandSpec.scala:
##########
@@ -71,4 +79,33 @@ case class TableCommandSpec(
classname: String,
tableDescs: Seq[TableDesc],
opType: String = "QUERY",
- queryDescs: Seq[QueryDesc] = Nil) extends CommandSpec
+ queryDescs: Seq[QueryDesc] = Nil) extends CommandSpec {
+ def queries: LogicalPlan => Seq[LogicalPlan] = plan => {
+ queryDescs.flatMap { qd =>
+ try {
+ qd.extract(plan)
+ } catch {
+ case e: Exception =>
+ LOG.warn(qd.error(plan, e))
+ None
+ }
+ }
+ }
+}
+
+case class ScanSpec(
+ classname: String,
+ scanDescs: Seq[ScanDesc]) extends CommandSpec {
+ override def opType: String = "QUERY"
Review Comment:
maybe extract this repeated opType "QUERY" to the object for preventing magic value? or use `OperationType.QUERY.toString`.
--
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] [kyuubi] bowenliang123 commented on a diff in pull request #4044: [Authz] Serde for queries
Posted by GitBox <gi...@apache.org>.
bowenliang123 commented on code in PR #4044:
URL: https://github.com/apache/kyuubi/pull/4044#discussion_r1061189429
##########
extensions/spark/kyuubi-spark-authz/src/main/resources/META-INF/services/org.apache.kyuubi.plugin.spark.authz.serde.TableExtractor:
##########
@@ -16,6 +16,7 @@
#
org.apache.kyuubi.plugin.spark.authz.serde.CatalogTableTableExtractor
+org.apache.kyuubi.plugin.spark.authz.serde.CatalogTableOptionTableExtractor
Review Comment:
```suggestion
org.apache.kyuubi.plugin.spark.authz.serde.CatalogTableOptionTableExtractor
org.apache.kyuubi.plugin.spark.authz.serde.CatalogTableTableExtractor
```
sort the extractors in alphabet order.
--
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] [kyuubi] yaooqinn commented on pull request #4044: [Authz] Serde for queries
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on PR #4044:
URL: https://github.com/apache/kyuubi/pull/4044#issuecomment-1370669571
thanks @bowenliang123 for the check, 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] [kyuubi] yaooqinn closed pull request #4044: [Authz] Serde for queries
Posted by GitBox <gi...@apache.org>.
yaooqinn closed pull request #4044: [Authz] Serde for queries
URL: https://github.com/apache/kyuubi/pull/4044
--
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