Skip to content

Conversation

@ebyhr
Copy link
Member

@ebyhr ebyhr commented Feb 26, 2025

Description

Syntax proposal: https://docs.google.com/document/d/1jEF4IkWu-2Gzk5ii2Nb0exuEnAUeo98UbiM3i0xtgWQ/edit?usp=sharing

This adds a new privilege CREATE BRANCH for access control of branches.

Relates to #12844

Release notes

## General
* Add support for `GRANT`, `DENY`, `REVOKE` with branches. ({issue}`25152`)

@cla-bot cla-bot bot added the cla-signed label Feb 26, 2025
@github-actions github-actions bot added iceberg Iceberg connector hive Hive connector labels Feb 26, 2025
@ebyhr ebyhr force-pushed the ebi/iceberg-branching-v3 branch 5 times, most recently from 8322da8 to 03d2d0d Compare February 26, 2025 13:36
@ebyhr ebyhr force-pushed the ebi/iceberg-branching-v3 branch from 03d2d0d to 8eee28a Compare March 11, 2025 03:41
@ebyhr ebyhr force-pushed the ebi/iceberg-branching-v3 branch 11 times, most recently from 96303ed to 611d6cd Compare April 6, 2025 08:08
@ebyhr ebyhr force-pushed the ebi/iceberg-branching-v3 branch from 611d6cd to 4c6df1f Compare April 7, 2025 11:03
@ebyhr ebyhr changed the title Add support for branching in engine and Iceberg Add support for branching Apr 7, 2025
@ebyhr ebyhr changed the title Add support for branching Add support for table level branching Apr 14, 2025
@ebyhr ebyhr force-pushed the ebi/iceberg-branching-v3 branch 3 times, most recently from 0b7aa49 to 91bc773 Compare April 17, 2025 06:46
@harksin
Copy link
Contributor

harksin commented Apr 21, 2025

awesome PR!
iceberg branching will be so usefull.
Any plan to be embed this in an upcomming release or is it too early ?

@ebyhr ebyhr force-pushed the ebi/iceberg-branching-v3 branch from 91bc773 to a02b041 Compare April 24, 2025 05:11
Copy link
Member

@kasiafi kasiafi left a comment

Choose a reason for hiding this comment

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

Skimmed through the two initial commits.

@kasiafi kasiafi reopened this Jun 27, 2025
@github-actions github-actions bot removed the stale label Jun 27, 2025
@ebyhr ebyhr force-pushed the ebi/iceberg-branching-v3 branch from e735a89 to 5c15495 Compare July 9, 2025 02:55
@ebyhr ebyhr changed the title Add support for table level branching Add support for GRANT/DENY/REVOKE on branches Jul 9, 2025
@trinodb trinodb deleted a comment from github-actions bot Jul 9, 2025
@trinodb trinodb deleted a comment from github-actions bot Jul 9, 2025
@ebyhr ebyhr force-pushed the ebi/iceberg-branching-v3 branch 4 times, most recently from c3b9148 to 6c8b7f7 Compare July 9, 2025 23:27
@ebyhr ebyhr marked this pull request as ready for review July 10, 2025 02:37
Copy link
Member

@kasiafi kasiafi left a comment

Choose a reason for hiding this comment

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

LGTM % comments. Most importantly, we should decide upon handling the identifier.

metadata.denyTablePrivileges(session, tableName, privileges, createPrincipal(statement.getGrantee()));
}
else {
String branchName = branch.get().getValue();
Copy link
Member

Choose a reason for hiding this comment

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

We don't have proper Identifier semantics in Trino, but it would be closer to correct if we used getCanonicalValue() instead of getValue().

However, I just noticed that we also rely on getValue() when creating the branch...

On the other hand, when creating a QualifiedName, we rely on this method, which lowercases identifiers.

I think that for branches, we should also lowercase the name to keep it locally consistent.
Same applies to GrantTask and RevokeTask.

@martint what do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

I will send a follow-up PR if Martin request changes.

@ebyhr ebyhr force-pushed the ebi/iceberg-branching-v3 branch from 6c8b7f7 to ec04151 Compare July 22, 2025 00:12
@ebyhr ebyhr force-pushed the ebi/iceberg-branching-v3 branch from ec04151 to 6c3100c Compare July 23, 2025 23:51
@ebyhr ebyhr merged commit a2faed5 into trinodb:master Jul 28, 2025
95 checks passed
@ebyhr ebyhr deleted the ebi/iceberg-branching-v3 branch July 28, 2025 02:18
@github-actions github-actions bot added this to the 477 milestone Jul 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

4 participants