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.

Sign up for GitHub

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

Jump to bottom

Simplify DeltaLakeMetadata with new model class #18133

Merged
merged 1 commit into from
Mar 1, 2024
Merged

Simplify DeltaLakeMetadata with new model class #18133

merged 1 commit into from
Mar 1, 2024

Conversation

ebyhr
Copy link
Member

@ebyhr ebyhr commented Jul 5, 2023 •
edited
Loading

Description

The motivation comes from #18013. Currently, DeltaLakeMetadata class calls some getColumn*(MetadataEntry) methods from several places. It's a little hard to use those methods correctly.
It would be better to manage those information in a single place.

@cla-bot cla-bot bot added the cla-signed label Jul 5, 2023
@github-actions github-actions bot added the delta-lake Delta Lake connector label Jul 5, 2023
@ebyhr ebyhr marked this pull request as ready for review July 5, 2023 05:59
@ebyhr ebyhr requested review from findepi, findinpath and pajaks July 5, 2023 05:59
@findepi findepi mentioned this pull request Jul 6, 2023
Merged
@ebyhr ebyhr marked this pull request as draft July 7, 2023 09:29
@ebyhr ebyhr force-pushed the ebi/delta-refactoring branch from 7e3adb9 to 51f9dd2 Compare July 13, 2023 09:02
@ebyhr ebyhr marked this pull request as ready for review July 13, 2023 09:02
@ebyhr ebyhr force-pushed the ebi/delta-refactoring branch 2 times, most recently from 3a6ada1 to 93a6dc5 Compare July 13, 2023 22:09
@ebyhr ebyhr requested a review from findepi July 13, 2023 22:09
@ebyhr ebyhr force-pushed the ebi/delta-refactoring branch from 93a6dc5 to 6f492d4 Compare July 18, 2023 10:40
@ebyhr
Copy link
Member Author

ebyhr commented Jul 18, 2023

Just rebased on master without changes.

@ebyhr ebyhr mentioned this pull request Jul 19, 2023
Merged
@ebyhr ebyhr force-pushed the ebi/delta-refactoring branch from 6f492d4 to 2c79f4c Compare August 3, 2023 01:41
@ebyhr ebyhr force-pushed the ebi/delta-refactoring branch from 2c79f4c to cfcf446 Compare August 15, 2023 23:39
@ebyhr
Copy link
Member Author

ebyhr commented Sep 6, 2023

I will rebase after #18423

@ebyhr ebyhr force-pushed the ebi/delta-refactoring branch from cfcf446 to 0871e5b Compare September 6, 2023 00:43
@ebyhr ebyhr force-pushed the ebi/delta-refactoring branch from 0871e5b to 50e2dbd Compare October 20, 2023 22:18
@ebyhr ebyhr force-pushed the ebi/delta-refactoring branch from 50e2dbd to 8513d22 Compare November 29, 2023 06:42
@ebyhr ebyhr marked this pull request as draft November 29, 2023 07:57
@ebyhr ebyhr closed this Jan 26, 2024
@ebyhr ebyhr deleted the ebi/delta-refactoring branch January 26, 2024 05:53
@ebyhr ebyhr restored the ebi/delta-refactoring branch February 15, 2024 07:28
@ebyhr ebyhr reopened this Feb 15, 2024
@ebyhr ebyhr force-pushed the ebi/delta-refactoring branch from 8513d22 to 2c9f7e0 Compare February 15, 2024 08:15
@ebyhr ebyhr self-assigned this Feb 15, 2024
@ebyhr ebyhr force-pushed the ebi/delta-refactoring branch 2 times, most recently from daec8b2 to f25818e Compare February 15, 2024 10:45
@ebyhr ebyhr marked this pull request as ready for review February 15, 2024 10:45
@ebyhr ebyhr added the no-release-notes This pull request does not require release notes entry label Feb 15, 2024
findinpath
findinpath reviewed Feb 16, 2024
View reviewed changes
Copy link
Contributor

@findinpath findinpath left a comment

Choose a reason for hiding this comment

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

DeltaTable / DeltaLakeTable otherwise said an object representation of the JSON content from DeltaLakeSchemaSupport seems pretty appealing.

plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaTable.java Outdated Show resolved Hide resolved
plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaTable.java Outdated Show resolved Hide resolved
plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaTable.java Outdated Show resolved Hide resolved
plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaTable.java Outdated Show resolved Hide resolved
plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java Outdated Show resolved Hide resolved
@ebyhr ebyhr force-pushed the ebi/delta-refactoring branch 4 times, most recently from a53ec16 to 17088c3 Compare February 17, 2024 00:50
@ebyhr ebyhr force-pushed the ebi/delta-refactoring branch 2 times, most recently from ecfab27 to ae0d7cf Compare February 20, 2024 00:39
@ebyhr
Copy link
Member Author

ebyhr commented Feb 20, 2024

@findepi Could you please review this PR when you have time?

findepi
findepi approved these changes Feb 22, 2024
View reviewed changes
Copy link
Member

@findepi findepi left a comment

Choose a reason for hiding this comment

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

skimmed. @alexjo2144 you may want to review this

@ebyhr ebyhr requested a review from alexjo2144 February 23, 2024 22:51
alexjo2144
alexjo2144 reviewed Feb 29, 2024
View reviewed changes
Copy link
Member

@alexjo2144 alexjo2144 left a comment

Choose a reason for hiding this comment

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

Nice, this is way better 👍
One small comment

plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java Outdated Show resolved Hide resolved
@ebyhr ebyhr force-pushed the ebi/delta-refactoring branch from ae0d7cf to f5ce751 Compare March 1, 2024 00:23
@ebyhr ebyhr merged commit 0123afa into trinodb:master Mar 1, 2024
24 checks passed
@ebyhr ebyhr deleted the ebi/delta-refactoring branch March 1, 2024 01:04
@github-actions github-actions bot added this to the 440 milestone Mar 1, 2024
@colebow colebow mentioned this pull request Mar 6, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexjo2144 alexjo2144 alexjo2144 left review comments

@pajaks pajaks pajaks approved these changes

@findinpath findinpath findinpath approved these changes

@findepi findepi findepi approved these changes

Assignees

@ebyhr ebyhr

Labels
cla-signed delta-lake Delta Lake connector no-release-notes This pull request does not require release notes entry
Milestone
440
Development

Successfully merging this pull request may close these issues.
5 participants
@ebyhr @findepi @alexjo2144 @findinpath @pajaks