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.

Sign up for GitHub

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

Jump to bottom

Fix GRANT, DENY, REVOKE on view and materialized view #20812

Merged
merged 2 commits into from
Feb 23, 2024
Merged

Fix GRANT, DENY, REVOKE on view and materialized view #20812

merged 2 commits into from
Feb 23, 2024

Conversation

findepi
Copy link
Member

@findepi findepi commented Feb 22, 2024

No description provided.

findepi and others added 2 commits February 22, 2024 16:46
@findepi @djsstarburst
Fix GRANT, DENY, REVOKE on view and materialized view
1abd058 
Co-authored-by: David Stryker <david.stryker@starburstdata.com>
@findepi
Fix Hive getTableHandle to reject views
a255da0 
Currently it's never called by engine if something is a view, but a
connector should not rely on unwritten "ordering guarantees".
@cla-bot cla-bot bot added the cla-signed label Feb 22, 2024
@github-actions github-actions bot added tests:hive hive Hive connector labels Feb 22, 2024
djsstarburst
djsstarburst approved these changes Feb 22, 2024
View reviewed changes
Copy link
Member

@djsstarburst djsstarburst left a comment

Choose a reason for hiding this comment

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

These changes look ok to me. That part I didn't understand was the failure mode without these changes. Perhaps you could add comments where you filter out materialized results and views saying why the filtering is needed.

@findepi
Copy link
Member Author

findepi commented Feb 23, 2024

the failure mode without these changes

depends on the connector. might be "view not found" for a well-imeplemented connector
for Hive -- no failure, since getTableHandle returns something also for views and MVs (but shouldn't)

@findepi findepi merged commit 9c0c159 into trinodb:master Feb 23, 2024
92 checks passed
@findepi findepi deleted the findepi/grant-view branch February 23, 2024 10:20
@github-actions github-actions bot added this to the 440 milestone Feb 23, 2024
@colebow colebow mentioned this pull request Feb 23, 2024
Merged
@colebow
Copy link
Member

colebow commented Feb 23, 2024

Does this need a release note? @findepi

@findepi
Copy link
Member Author

findepi commented Feb 26, 2024

yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@djsstarburst djsstarburst djsstarburst approved these changes

@dain dain dain approved these changes

@kokosing kokosing Awaiting requested review from kokosing

@lukasz-walkiewicz lukasz-walkiewicz Awaiting requested review from lukasz-walkiewicz

Assignees
No one assigned
Labels
cla-signed hive Hive connector
Milestone
440
Development

Successfully merging this pull request may close these issues.
4 participants
@findepi @colebow @dain @djsstarburst