Skip to content

Fix information schema for catalogs with system security#11694

Merged
dain merged 1 commit intotrinodb:masterfrom
dain:fix-information-schema-system-roles
Mar 29, 2022
Merged

Fix information schema for catalogs with system security#11694
dain merged 1 commit intotrinodb:masterfrom
dain:fix-information-schema-system-roles

Conversation

@dain
Copy link
Copy Markdown
Member

@dain dain commented Mar 29, 2022

Description

Fix information_schema role tables for catalogs using system roles. Before this change the querying the tables would simply throw an exception.

Documentation

(x) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

( ) No release notes entries required.
(x) Release notes entries required with the following suggested text:

# Section
* Fix failures in information schema role tables for catalogs using system roles. ({issue}`11694`)

@dain dain requested a review from electrum March 29, 2022 01:47
@cla-bot cla-bot bot added the cla-signed label Mar 29, 2022

private void addRolesRecords()
{
Optional<String> catalogName = metadata.isCatalogManagedSecurity(session, this.catalogName) ? Optional.of(this.catalogName) : Optional.empty();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Might be cleaner to use Optional.filter

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I tried that and it was less awesome

@dain dain merged commit 7f954b5 into trinodb:master Mar 29, 2022
@dain dain deleted the fix-information-schema-system-roles branch March 29, 2022 03:58
@github-actions github-actions bot added this to the 376 milestone Mar 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants