You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by "bowenliang123 (via GitHub)" <gi...@apache.org> on 2023/10/31 14:57:12 UTC
[PR] [AUTHZ] Support CreateTableAs command for Paimon [kyuubi]
bowenliang123 opened a new pull request, #5590:
URL: https://github.com/apache/kyuubi/pull/5590
<!--
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/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.
-->
Support CreateTableAs command for Paimon
- Create Table As Command: https://paimon.apache.org/docs/master/engines/spark3/#create-table
### _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.readthedocs.io/en/master/contributing/code/testing.html#running-tests) locally before make a pull request
### _Was this patch authored or co-authored using generative AI tooling?_
<!--
If a generative AI tooling has been used in the process of authoring this patch, please include
phrase 'Generated-by: ' followed by the name of the tool and its version.
If no, write 'No'.
Please refer to the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html) for details.
-->
No.
--
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
Re: [PR] [AUTHZ] Support CreateTableAs command for Paimon [kyuubi]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #5590:
URL: https://github.com/apache/kyuubi/pull/5590#issuecomment-1787661866
## [Codecov](https://app.codecov.io/gh/apache/kyuubi/pull/5590?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#5590](https://app.codecov.io/gh/apache/kyuubi/pull/5590?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (4287e90) into [master](https://app.codecov.io/gh/apache/kyuubi/commit/c24e984a3e8a41ad20bc4a5517e51a856e3e5452?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (c24e984) will **increase** coverage by `0.31%`.
> Report is 14 commits behind head on master.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #5590 +/- ##
============================================
+ Coverage 61.35% 61.67% +0.31%
Complexity 23 23
============================================
Files 600 603 +3
Lines 34316 35670 +1354
Branches 4501 4869 +368
============================================
+ Hits 21054 21998 +944
- Misses 11128 11290 +162
- Partials 2134 2382 +248
```
[see 63 files with indirect coverage changes](https://app.codecov.io/gh/apache/kyuubi/pull/5590/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
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
Re: [PR] [AUTHZ] Support CreateTableAs command for Paimon [kyuubi]
Posted by "davidyuan1223 (via GitHub)" <gi...@apache.org>.
davidyuan1223 commented on PR #5590:
URL: https://github.com/apache/kyuubi/pull/5590#issuecomment-1788378009
> @davidyuan1223 More investigation in the logical plan is required to find the reason. Maybe missing fields in parsed privilege objects. Let me update the test case for Paimon first. And thanks for mentioning possible bug for Delta test suite as well.
thanks your reply, if you find the root cause about this bug, wish you can teach me how to fix this, so that I can complete the issue #5470
--
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
Re: [PR] [AUTHZ] Support CreateTableAs command for Paimon [kyuubi]
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on PR #5590:
URL: https://github.com/apache/kyuubi/pull/5590#issuecomment-1788471652
Yes. Will have a dive-in debugging in extractors 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
Re: [PR] [AUTHZ] Support CreateTableAs command for Paimon [kyuubi]
Posted by "davidyuan1223 (via GitHub)" <gi...@apache.org>.
davidyuan1223 commented on PR #5590:
URL: https://github.com/apache/kyuubi/pull/5590#issuecomment-1787694278
> I want ask a question, why the admin to execute doAs in the next, don't use sql to execute, i found your code is `doAs(admin, createTableAs)`, why not is `doAs(admin,sql(createTableAs))`. And i found the DeltaCatalogRanger also use doAs(user,mySql),but the HudiCatalog use doAs(user,sql(mySql)). Which is the right way???
>
> @pan3793 could you give some advice
doAs code is
```
protected def doAs[T](user: String, f: => T): T = {
UserGroupInformation.createRemoteUser(user).doAs[T](
new PrivilegedExceptionAction[T] {
override def run(): T = f
})
}
```
If we not use spark.sql to execute our sql, it's only a string format???
--
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
Re: [PR] [AUTHZ] Support CreateTableAs command for Paimon [kyuubi]
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on PR #5590:
URL: https://github.com/apache/kyuubi/pull/5590#issuecomment-1788419003
The initial reason is that missing `database` field in RangerAccessRequest, and the admin user is granted on db-table-column `*-*-*` resource rules.
<img width="886" alt="image" src="https://github.com/apache/kyuubi/assets/1935105/bee6e6d4-68f1-4caf-a9a5-97f32246062b">
--
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
Re: [PR] [AUTHZ] Support CreateTableAs command for Paimon [kyuubi]
Posted by "davidyuan1223 (via GitHub)" <gi...@apache.org>.
davidyuan1223 commented on PR #5590:
URL: https://github.com/apache/kyuubi/pull/5590#issuecomment-1790139818
> It's normal to see lakehouse has individual implementation of spark plan in some commands. They're still part of spark plans. To support them, we could adapt our extractor for the details of the plan. But the precondition is that table identifier is carried by the plan. Should have look at filestore class of paimon for that.
The paimon's FileStoreTable top interface's method name is only get the tableName, what can we do to improve the paimon could return database? The source code in 0.5 only will return like `xxx.db/tableName` to tableName
--
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
Re: [PR] [AUTHZ] Support CreateTableAs command for Paimon [kyuubi]
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 closed pull request #5590: [AUTHZ] Support CreateTableAs command for Paimon
URL: https://github.com/apache/kyuubi/pull/5590
--
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
Re: [PR] [AUTHZ] Support CreateTableAs command for Paimon [kyuubi]
Posted by "davidyuan1223 (via GitHub)" <gi...@apache.org>.
davidyuan1223 commented on PR #5590:
URL: https://github.com/apache/kyuubi/pull/5590#issuecomment-1789945758
> Seems that there's a Paimon's `org.apache.paimon.AppendOnlyFileStore` as the Spark Table inside the input query plan, which is unsupported by the DataSourceV2RelationTableExtractor.
>
> <img alt="image" width="933" src="https://user-images.githubusercontent.com/1935105/279718993-eddbfd92-832b-4bea-adce-b0d39b5a7231.png">
looks like the paimon can not supported in kyuubi's authz,? we can not make sure the paimon's plan is same as spark's plan
--
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
Re: [PR] [AUTHZ] Support CreateTableAs command for Paimon [kyuubi]
Posted by "davidyuan1223 (via GitHub)" <gi...@apache.org>.
davidyuan1223 commented on PR #5590:
URL: https://github.com/apache/kyuubi/pull/5590#issuecomment-1788364208
> @davidyuan1223 I believe you are right, and @bowenliang123 made a mistake. Nice catch.
And I found the [DeltaCatalogRanger](https://github.com/apache/kyuubi/blob/master/extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/ranger/DeltaCatalogRangerSparkExtensionSuite.scala) also is this code style, I think you need to check the author module's test case to avoid this problem. This problem just like this [comment](https://github.com/apache/kyuubi/issues/5470#issuecomment-1787008858), I want know why admin create table but it can not select from the table?
--
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
Re: [PR] [AUTHZ] Support CreateTableAs command for Paimon [kyuubi]
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on PR #5590:
URL: https://github.com/apache/kyuubi/pull/5590#issuecomment-1788414085
> > @davidyuan1223 More investigation in the logical plan is required to find the reason. Maybe missing fields in parsed privilege objects. Let me update the test case for Paimon first. And thanks for mentioning possible bug for Delta test suite as well.
>
> thanks your reply, if you find the root cause about this bug, wish you can teach me how to fix this, so that I can complete the issue #5470
Sure. Will ping you when I came to any conclusion for this. Thanks for creating a bug issue for 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
Re: [PR] [AUTHZ] Support CreateTableAs command for Paimon [kyuubi]
Posted by "davidyuan1223 (via GitHub)" <gi...@apache.org>.
davidyuan1223 commented on PR #5590:
URL: https://github.com/apache/kyuubi/pull/5590#issuecomment-1788430480
> RangerAccessRequest
LGTM, I will debug in my env
--
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
Re: [PR] [AUTHZ] Support CreateTableAs command for Paimon [kyuubi]
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on PR #5590:
URL: https://github.com/apache/kyuubi/pull/5590#issuecomment-1789152107
Seems that there's a Paimon's `org.apache.paimon.AppendOnlyFileStore` as the Spark Table inside the input query plan, which is unsupported by the DataSourceV2RelationTableExtractor.
<img width="933" alt="image" src="https://github.com/apache/kyuubi/assets/1935105/eddbfd92-832b-4bea-adce-b0d39b5a7231">
--
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
Re: [PR] [AUTHZ] Support CreateTableAs command for Paimon [kyuubi]
Posted by "davidyuan1223 (via GitHub)" <gi...@apache.org>.
davidyuan1223 commented on PR #5590:
URL: https://github.com/apache/kyuubi/pull/5590#issuecomment-1787662100
Look like the test case can success in CI but can not success in local env, could you give some suggests? @pan3793
--
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
Re: [PR] [AUTHZ] Support CreateTableAs command for Paimon [kyuubi]
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on PR #5590:
URL: https://github.com/apache/kyuubi/pull/5590#issuecomment-1789955729
It's normal to see lakehouse has individual implementation of spark plan for some commands. They're still part of spark plans. To support them, we could adapt our extractor for the details of the plan. But the precondition is that table identifier is carried by the plan. Should have look at filestore class of paimon for that.
--
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
Re: [PR] [AUTHZ] Support CreateTableAs command for Paimon [kyuubi]
Posted by "davidyuan1223 (via GitHub)" <gi...@apache.org>.
davidyuan1223 commented on PR #5590:
URL: https://github.com/apache/kyuubi/pull/5590#issuecomment-1787687027
I want ask a question, why the admin to execute doAs in the next, don't use sql to execute, i found your code is `doAs(admin, createTableAs)`, why not is `doAs(admin,sql(createTableAs))`.
And i found the DeltaCatalogRanger also use doAs(user,mySql),but the HudiCaralog use doAs(user,sql(mySql)).
Which is the right way???
@pan3793 could you give some advice
--
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
Re: [PR] [AUTHZ] Support CreateTableAs command for Paimon [kyuubi]
Posted by "davidyuan1223 (via GitHub)" <gi...@apache.org>.
davidyuan1223 commented on PR #5590:
URL: https://github.com/apache/kyuubi/pull/5590#issuecomment-1788467636
> The initial reason is that missing `database` field in RangerAccessRequest, and the admin user is granted on db-table-column `*-*-*` resource rules.
>
> <img alt="image" width="886" src="https://user-images.githubusercontent.com/1935105/279581627-bee6e6d4-68f1-4caf-a9a5-97f32246062b.png">
this is the logic plan about createTableAs, looks like it's plan have some problem
```
CreateTableAsSelect org.apache.paimon.spark.SparkCatalog@4a5f435b, paimon_ns.table2, [provider=PAIMON], true
+- RelationV2[id#6, name#7, city#8] table1
```
--
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
Re: [PR] [AUTHZ] Support CreateTableAs command for Paimon [kyuubi]
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on PR #5590:
URL: https://github.com/apache/kyuubi/pull/5590#issuecomment-1788368172
@davidyuan1223 More investigation in the logical plan is required to find the reason. Let me update the test case for Paimon first. And thanks for mentioning possible bug for Delta test suite 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
Re: [PR] [AUTHZ] Support CreateTableAs command for Paimon [kyuubi]
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on PR #5590:
URL: https://github.com/apache/kyuubi/pull/5590#issuecomment-1788361847
@davidyuan1223 I believe you are right, and @bowenliang123 made a mistake. Nice catch.
--
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
Re: [PR] [AUTHZ] Support CreateTableAs command for Paimon [kyuubi]
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on PR #5590:
URL: https://github.com/apache/kyuubi/pull/5590#issuecomment-1788364246
Sorry about that. Will fix and have further investigation.
--
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