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

[#1398] feat(table): Add index api for tables. #1399

Merged
merged 6 commits into from
Jan 23, 2024

Conversation

Clearvive
Copy link
Contributor

@Clearvive Clearvive commented Jan 9, 2024

What changes were proposed in this pull request?

Provide the Index interface implementation of the Table to the user side

Why are the changes needed?

Fix: #1398

Does this PR introduce any user-facing change?

Add table index impl.

How was this patch tested?

NO

@Clearvive Clearvive added this to the Gravitino 0.4.0 milestone Jan 9, 2024
@Clearvive Clearvive self-assigned this Jan 9, 2024
@Clearvive Clearvive force-pushed the 1398/add-index-to-user branch from 9adb618 to 9614ffd Compare January 9, 2024 10:42
@yuqi1129
Copy link
Contributor

yuqi1129 commented Jan 9, 2024

I suggest adding some UT for the catalog that supports Index like MySQL catalog.

@Clearvive
Copy link
Contributor Author

I suggest adding some UT for the catalog that supports Index like MySQL catalog.

This is just a pull request for the API interface, which will be submitted in another PR

@Clearvive Clearvive changed the title [#1339] feat(table): Add index for tables. [#1339] feat(table): Add index api for tables. Jan 10, 2024
@Clearvive Clearvive changed the title [#1339] feat(table): Add index api for tables. [#1398] feat(table): Add index api for tables. Jan 10, 2024
@Clearvive
Copy link
Contributor Author

@jerryshao Can you help me review it?

@Clearvive
Copy link
Contributor Author

@mchades At present, except for the complex index design of hive3.0, it is not considered to be supported in this issue. The current interface can support common systems. Do you have any suggestions? Can you help review it?

@Clearvive Clearvive requested a review from FANNG1 January 18, 2024 09:05
@FANNG1
Copy link
Contributor

FANNG1 commented Jan 18, 2024

please paste the design document

@FANNG1
Copy link
Contributor

FANNG1 commented Jan 18, 2024

LGTM, except some minor comments

@Clearvive Clearvive force-pushed the 1398/add-index-to-user branch from 517e797 to 137dd1a Compare January 22, 2024 02:33
@jerryshao
Copy link
Contributor

@mchades @FANNG1 can you please also take a look? Thanks.

@Clearvive Clearvive requested review from mchades and FANNG1 January 22, 2024 08:34
@jerryshao
Copy link
Contributor

@FANNG1 @mchades do you have further comments?

@mchades
Copy link
Contributor

mchades commented Jan 22, 2024

@FANNG1 @mchades do you have further comments?

There is only one comment above that needs to be resolved.

@FANNG1
Copy link
Contributor

FANNG1 commented Jan 22, 2024

LGTM except some minor comments.

@jerryshao jerryshao merged commit 2face0e into apache:main Jan 23, 2024
12 checks passed
mchades pushed a commit to mchades/gravitino that referenced this pull request Jan 24, 2024
### What changes were proposed in this pull request?

Provide the Index interface implementation of the Table to the user side

### Why are the changes needed?
Fix: apache#1398 

### Does this PR introduce _any_ user-facing change?
Add table index impl.

### How was this patch tested?
NO

---------

Co-authored-by: Clearvive <[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.

[FEATURE] Provide the Index interface implementation of the Table to the user side
5 participants