Skip to content
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

Remove deprecated checkCanCreateSchema #15618

Conversation

findepi
Copy link
Member

@findepi findepi commented Jan 5, 2023

Retaining the deprecated method was not sufficient for maintaining compatibility, because the new method did not delegate to the old one. Thus, if an implementor kept implementing only the old method, the CREATE SCHEMA would get denied, irrespective of the implemented logic. Removing the deprecated method makes the incompatibility clearer, as it fails at the compilation time.

Follows #15153 (comment)

Retaining the deprecated method was not sufficient for maintaining
compatibility, because the new method did not delegate to the old one.
Thus, if an implementor kept implementing only the old method, the
CREATE SCHEMA would get denied, irrespective of the implemented logic.
Removing the deprecated method makes the incompatibility clearer, as it
fails at the compilation time.
@findepi findepi added the no-release-notes This pull request does not require release notes entry label Jan 5, 2023
@cla-bot cla-bot bot added the cla-signed label Jan 5, 2023
Copy link
Member

@electrum electrum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The access control manager should have called both methods. But it's fine to remove the old one now.

@kokosing
Copy link
Member

kokosing commented Jan 5, 2023

Thank you

@findepi
Copy link
Member Author

findepi commented Jan 11, 2023

The access control manager should have called both methods.

How would it be better than delegating from the new one to the old one?

But it's fine to remove the old one now.

Agreed.

@findepi findepi merged commit 0fac087 into trinodb:master Jan 11, 2023
@findepi findepi deleted the findepi/remove-deprecated-checkcancreateschema-270ea8 branch January 11, 2023 13:22
@github-actions github-actions bot added this to the 406 milestone Jan 11, 2023
utk-spartan added a commit to utk-spartan/ranger that referenced this pull request Mar 8, 2023
Incorporate following changes

Remove deprecated checkCanCreateSchema
trinodb/trino#15618
trinodb/trino@0fac087

Disallow multiple masks on a given column
trinodb/trino#15680
trinodb/trino@bdd1cb5
utk-spartan added a commit to razorpay/ranger that referenced this pull request Mar 8, 2023
Incorporate following changes

Remove deprecated checkCanCreateSchema
trinodb/trino#15618
trinodb/trino@0fac087

Disallow multiple masks on a given column
trinodb/trino#15680
trinodb/trino@bdd1cb5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed no-release-notes This pull request does not require release notes entry
Development

Successfully merging this pull request may close these issues.

5 participants