-
Notifications
You must be signed in to change notification settings - Fork 606
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Views: throw a human-readable error in case of a missing WITH (security_invoker = true)
#9320
Conversation
fdd2ace
to
38b5866
Compare
…er = true)` Validation is moved to KQP to be able to use evaluated expressions to set the `security_invoker` option. See the added unit tests for details.
38b5866
to
cd8f8de
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
show that the option name is case-sensitive
⚪ ⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
…ity_invoker = true)` (ydb-platform#9320)
…ity_invoker = true)` (ydb-platform#9320)
…ity_invoker = true)` (ydb-platform#9320)
You can see by the changes in the YQL grammar that previously
with_table_settings
clause was mandatory for theCREATE VIEW
statement. It caused a grammar mismatch error whenever a user forgot to add theWITH (security_invoker = true)
clause. We want the error to be easy to understand, so we made thewith_table_settings
⭐ clause an optional parameter in the grammar (i.e. added a?
sign after thewith_table_settings
clause) and are validating the contents of this clause later during compilation of theCREATE VIEW
statement.⭐ The other interesting part of the PR is the change of the
with_table_settings
clause to thecreate_object_features
clause in thecreate_view_stmt
. I don't remember why I have chosenwith_table_settings
initially, but now it seems an error. (Probably, the reason was that the oldcreate_object_features
clause couldn't handlebool
object features. I have corrected this flaw in this PR too.) All objects can be created with a genericCREATE OBJECT
statement, so in order to support this scenario for views, we need to use the genericcreate_object_features
in thecreate_view_stmt
.Lastly, I have moved the view options validation to KQP which have enabled us to use named expressions for the
security_invoker
option. It would not be possible if the validation stayed in the parser.