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

[#1253] bugfix(jdbc): fix jdbc catalog store empty audit info issue #1257

Merged
merged 3 commits into from
Dec 26, 2023

Conversation

Clearvive
Copy link
Contributor

What changes were proposed in this pull request?

Mysql&Postgres catalog store empty audit info fix.

Why are the changes needed?

Fix: #1253

Does this PR introduce any user-facing change?

Users will receive audit information

How was this patch tested?

IT

@Clearvive Clearvive self-assigned this Dec 26, 2023
@Clearvive Clearvive added this to the Gravitino 0.4.0 milestone Dec 26, 2023
@qqqttt123
Copy link
Contributor

Will this pull request back port to branch 0.3?

@Clearvive
Copy link
Contributor Author

Will this pull request back port to branch 0.3?

I have added a label

@jerryshao jerryshao added the need backport Issues that need to backport to another branch label Dec 26, 2023
@jerryshao
Copy link
Contributor

@qqqttt123 please help to review.

qqqttt123
qqqttt123 previously approved these changes Dec 26, 2023
Copy link
Contributor

@qqqttt123 qqqttt123 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @jerryshao @Clearvive

@@ -276,7 +276,7 @@ public Table loadTable(NameIdentifier tableIdent) throws NoSuchTableException {
}
Map<String, String> properties =
load.properties() == null ? Maps.newHashMap() : Maps.newHashMap(load.properties());
StringIdentifier.addToProperties(id, properties);
properties = StringIdentifier.addToProperties(id, properties);
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you rename addToProperties to a more proper name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, added.

@qqqttt123 qqqttt123 dismissed their stale review December 26, 2023 10:00

left some nits.

@qqqttt123 qqqttt123 closed this Dec 26, 2023
@qqqttt123 qqqttt123 reopened this Dec 26, 2023
@jerryshao jerryshao changed the title [#1253] bugfix(jdbc): jdbc catalog store empty audit info fix. [#1253] bugfix(jdbc): fix jdbc catalog store empty audit info issue Dec 26, 2023
@jerryshao jerryshao merged commit 06f8d47 into apache:main Dec 26, 2023
10 checks passed
github-actions bot pushed a commit that referenced this pull request Dec 26, 2023
…1257)

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

Mysql&Postgres catalog store empty audit info fix.

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

### Does this PR introduce _any_ user-facing change?
Users will receive audit information

### How was this patch tested?
IT

---------

Co-authored-by: Clearvive <clearvive@datastrato.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch-0.3 bug Something isn't working need backport Issues that need to backport to another branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug report] Mysql&Postgres catalog store empty audit info
3 participants