-
Notifications
You must be signed in to change notification settings - Fork 383
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
[#2082] feat(core): Add JDBC backend operations for schema #2377
Conversation
342c1b7
to
1d10a0f
Compare
@yuqi1129 This PR is ready for review, please take a look when you have time, thanks! |
core/src/main/java/com/datastrato/gravitino/storage/relational/service/SchemaMetaService.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/datastrato/gravitino/storage/relational/service/SchemaMetaService.java
Outdated
Show resolved
Hide resolved
@jerryshao @yuqi1129 @qqqttt123 Have fixed the comments, please take a look again when you have time. Hope we can finish the reviews of the remaining PRs today, thanks! |
@qqqttt123 |
core/src/main/java/com/datastrato/gravitino/storage/relational/service/SchemaMetaService.java
Show resolved
Hide resolved
Generally LGTM, just some nits. |
core/src/main/java/com/datastrato/gravitino/storage/relational/service/SchemaMetaService.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/datastrato/gravitino/storage/relational/service/SchemaMetaService.java
Outdated
Show resolved
Hide resolved
@yuqi1129 @qqqttt123 Any further comment for this? Thanks! |
core/src/main/java/com/datastrato/gravitino/storage/relational/service/CommonMetaService.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/datastrato/gravitino/storage/relational/service/CommonMetaService.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/datastrato/gravitino/storage/relational/service/CommonMetaService.java
Outdated
Show resolved
Hide resolved
LGTM. Let @yuqi1129 take a final look. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. @yuqi1129 please help to review again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
### What changes were proposed in this pull request? The purpose of this PR is to implement JDBC backend operation for Table metadata. Depend on #2377 . Metadata operations of Fileset will be supported in the remaining PRs. ### Why are the changes needed? Fix: #2083 ### How was this patch tested? Add unit tests to test the table metadata ops. --------- Co-authored-by: xiaojiebao <[email protected]>
What changes were proposed in this pull request?
The purpose of this PR is to implement JDBC backend operation Schema metadata. Depend on #2078 . Metadata operations of Table and Fileset will be supported in the remaining PRs.
Why are the changes needed?
Fix: #2082
How was this patch tested?
Add unit tests to test the schema metadata ops.