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

[#2081] feat(core): Add JDBC backend operations for catalog #2078

Merged
merged 35 commits into from
Feb 28, 2024

Conversation

xloya
Copy link
Contributor

@xloya xloya commented Feb 5, 2024

What changes were proposed in this pull request?

The purpose of this PR is to implement JDBC backend operation Catalog metadata. Depend on #1980 . Metadata operations of Schema, Table and Fileset will be supported in the remaining PRs.

Why are the changes needed?

Fix: #2081

How was this patch tested?

Add unit tests to test the catalog metadata ops.

@xloya xloya force-pushed the add-mysql-backend-ops-for-catalog branch from 47b817a to dd43519 Compare February 5, 2024 11:02
@xloya xloya changed the title [#1811] feat(relation-entity-store): Add MySQL backend operations for catalog [#2081] feat(relation-entity-store): Add MySQL backend operations for catalog Feb 5, 2024
@xloya xloya force-pushed the add-mysql-backend-ops-for-catalog branch from dd43519 to 5045da9 Compare February 6, 2024 06:42
@xloya xloya changed the title [#2081] feat(relation-entity-store): Add MySQL backend operations for catalog [#2081] feat(relation-entity-store): Add JDBC backend operations for catalog Feb 18, 2024
@xloya xloya force-pushed the add-mysql-backend-ops-for-catalog branch 4 times, most recently from 27f45e3 to 9c5eaa8 Compare February 18, 2024 11:48
@justinmclean
Copy link
Member

We use Spotless to ensure code is consistently formatted, look likes there is a formatting error that you can fix with ./gradlew :core:spotlessApply

@xloya
Copy link
Contributor Author

xloya commented Feb 20, 2024

We use Spotless to ensure code is consistently formatted, look likes there is a formatting error that you can fix with ./gradlew :core:spotlessApply

Since the dependent PR is still under review, the PR has not been updated in time. This PR will be updated today. Thanks.

@xloya xloya force-pushed the add-mysql-backend-ops-for-catalog branch 3 times, most recently from 972dcf8 to 82c4e8c Compare February 20, 2024 08:50
@xloya xloya marked this pull request as ready for review February 26, 2024 11:52
@xloya xloya force-pushed the add-mysql-backend-ops-for-catalog branch 3 times, most recently from f08e9f6 to e789c5a Compare February 27, 2024 02:53
@yuqi1129
Copy link
Contributor

@xloya
Is it ready for review?

@xloya xloya force-pushed the add-mysql-backend-ops-for-catalog branch from e789c5a to 3309d12 Compare February 27, 2024 02:57
@xloya
Copy link
Contributor Author

xloya commented Feb 27, 2024

@xloya Is it ready for review?

@yuqi1129 Yeah, I just completed the modification, please review it, thanks.

@xloya xloya force-pushed the add-mysql-backend-ops-for-catalog branch from 3309d12 to db62749 Compare February 27, 2024 03:08
@xloya
Copy link
Contributor Author

xloya commented Feb 27, 2024

Also gentle pin @jerryshao @qqqttt123 @coolderli for reviews.

@xloya xloya force-pushed the add-mysql-backend-ops-for-catalog branch from db62749 to fada86d Compare February 27, 2024 03:22
@xloya xloya force-pushed the add-mysql-backend-ops-for-catalog branch 2 times, most recently from 5ebec02 to 9aa6056 Compare February 27, 2024 06:17
@xloya xloya force-pushed the add-mysql-backend-ops-for-catalog branch from 9aa6056 to 70a91d0 Compare February 27, 2024 06:21
if (re.getCause() != null
&& re.getCause().getCause() != null
&& re.getCause().getCause() instanceof SQLIntegrityConstraintViolationException) {
// TODO We should make more fine-grained exception judgments
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be best to create an issue to track it at this point. The current implementation is not aesthetically pleasing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Had created the issue before: #2227

@yuqi1129
Copy link
Contributor

Could you also assist in taking a look? @qqqttt123

@qqqttt123
Copy link
Contributor

Could you also assist in taking a look? @qqqttt123

LGTM.

@yuqi1129
Copy link
Contributor

Could you also assist in taking a look? @qqqttt123

LGTM.

+1 @jerryshao , do you have any further comments?

@jerryshao
Copy link
Contributor

Could you also assist in taking a look? @qqqttt123

LGTM.

+1 @jerryshao , do you have any further comments?

I don't have further comment, you can go ahead if you feel ok @yuqi1129 .

Copy link
Contributor

@yuqi1129 yuqi1129 left a comment

Choose a reason for hiding this comment

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

LGTM

@yuqi1129 yuqi1129 merged commit 8726cc5 into apache:main Feb 28, 2024
26 of 27 checks passed
@xloya xloya deleted the add-mysql-backend-ops-for-catalog branch February 29, 2024 11:25
jerryshao pushed a commit that referenced this pull request Mar 4, 2024
### 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.

---------

Co-authored-by: xiaojiebao <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Subtask] Add JDBC Backend ops for Catalog
6 participants