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

[#5947] fix(auth): It will throw error if we enable authorization and rename catalog #5949

Merged
merged 3 commits into from
Dec 23, 2024

Conversation

jerqi
Copy link
Contributor

@jerqi jerqi commented Dec 23, 2024

What changes were proposed in this pull request?

Fix the issue of renaming catalogs or metalakes.

Why are the changes needed?

Fix: #5947

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Add UT.

@jerqi jerqi requested review from mchades and xunliu December 23, 2024 07:20
@@ -298,9 +298,19 @@ public static void authorizationPluginRenamePrivileges(
NameIdentifierUtil.toMetadataObject(NameIdentifier.of(ident.namespace(), newName), type);
MetadataObjectChange renameObject =
MetadataObjectChange.rename(oldMetadataObject, newMetadataObject);

String metalake;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can change to use ? : operand to simplify your code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Changed.

@mchades mchades added the branch-0.7 Automatically cherry-pick commit to branch-0.7 label Dec 23, 2024
@jerqi jerqi requested a review from jerryshao December 23, 2024 08:52
Copy link
Contributor

@mchades mchades left a comment

Choose a reason for hiding this comment

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

LGTM.

@jerryshao jerryshao merged commit 74c7aac into apache:main Dec 23, 2024
26 checks passed
jerqi added a commit to qqqttt123/gravitino that referenced this pull request Dec 23, 2024
…on and rename catalog (apache#5949)

Fix the issue of renaming catalogs or metalakes.

Fix: apache#5947

No.

Add UT.
jerqi added a commit to qqqttt123/gravitino that referenced this pull request Dec 23, 2024
…on and rename catalog (apache#5949)

Fix the issue of renaming catalogs or metalakes.

Fix: apache#5947

No.

Add UT.
Abyss-lord pushed a commit to Abyss-lord/gravitino that referenced this pull request Dec 23, 2024
…on and rename catalog (apache#5949)

### What changes were proposed in this pull request?

Fix the issue of renaming catalogs or metalakes.

### Why are the changes needed?

Fix: apache#5947

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?
Add UT.
mchades pushed a commit that referenced this pull request Dec 23, 2024
… rename catalog (#5949) (#5953)

### What changes were proposed in this pull request?
Fix the issue of renaming catalogs or metalakes.

### Why are the changes needed?
Fix: #5947

### Does this PR introduce _any_ user-facing change?
No.

### How was this patch tested?

Add UT.
Abyss-lord pushed a commit to Abyss-lord/gravitino that referenced this pull request Dec 29, 2024
…on and rename catalog (apache#5949)

### What changes were proposed in this pull request?

Fix the issue of renaming catalogs or metalakes.

### Why are the changes needed?

Fix: apache#5947

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?
Add UT.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch-0.7 Automatically cherry-pick commit to branch-0.7
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug report] Cannot rename catalog when enable authorization
3 participants