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

[Native] Fix table writer to use the actual storage format #23923

Merged
merged 2 commits into from
Oct 31, 2024
Merged

[Native] Fix table writer to use the actual storage format #23923

merged 2 commits into from
Oct 31, 2024

Conversation

kewang1024
Copy link
Collaborator

@kewang1024 kewang1024 commented Oct 31, 2024 •
edited
Loading

Coordinator decides data storage format in following logic:

  1. Unpartitioned table use table format
  2. Partitioned table
    • use table format if respect-table-format is set to true
    • use hive.storage-format if respect-table-format is false

The final decision is stored in actualStorageFormat of
HiveOutputTableHandle. Switch to use actualStorageFormat
for prestissimo.

tableStorageFormat and partitionStorageFormat in
HiveOutputTableHandle would be deprecated later.

== NO RELEASE NOTE ==

@kewang1024
Update HiveWriterFactory to use actualStorageFormat
8cbf824 
Use `actualStorageFormat` because storage format decision is
already made, having duplicate logic in HiveWriterFactory could
result in bug.

Also getTableStorageFormat and getPartitionStorageFormat API
will be deprecated from HiveWritableTableHandle
@kewang1024
[native] Fix table writer to use the actual storage format
03f4504 
What storage format would be used is decided in coordinator and
passed in by actualStorageFormat.

tableStorageFormat and partitionStorageFormat would be
deprecated
@kewang1024 kewang1024 requested review from a team as code owners October 31, 2024 06:07
@kewang1024 kewang1024 requested a review from presto-oss October 31, 2024 06:07
@kewang1024 kewang1024 changed the title Fix storage format [Native] Fix table writer to use the actual storage format Oct 31, 2024
amitkdutta
amitkdutta approved these changes Oct 31, 2024
View reviewed changes
Copy link
Contributor

@amitkdutta amitkdutta left a comment

Choose a reason for hiding this comment

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

Thanks @kewang1024

@amitkdutta amitkdutta merged commit 68aea2f into prestodb:master Oct 31, 2024
60 checks passed
@kewang1024 kewang1024 mentioned this pull request Nov 5, 2024
Merged
kewang1024 added a commit to kewang1024/presto that referenced this pull request Nov 9, 2024
@kewang1024
[native] Add e2e table format test with native worker
e206ee9 
This is followup e2e test for prestodb#23923

For a partitioned table, when respect_table_format is set to false,
and table_format and hive_storage_format is different, the newly
added partition should use hive_storage_format.
kewang1024 added a commit to kewang1024/presto that referenced this pull request Nov 11, 2024
@kewang1024
[native] Add e2e table format test with native worker
8cff4c3 
This is followup e2e test for prestodb#23923

For a partitioned table, when respect_table_format is set to false,
and table_format and hive_storage_format is different, the newly
added partition should use hive_storage_format.
kewang1024 added a commit to kewang1024/presto that referenced this pull request Nov 11, 2024
@kewang1024
[native] Add e2e table format test with native worker
40ced4e 
This is followup e2e test for prestodb#23923

For a partitioned table, when respect_table_format is set to false,
and table_format and hive_storage_format is different, the newly
added partition should use hive_storage_format.
kewang1024 added a commit to kewang1024/presto that referenced this pull request Nov 11, 2024
@kewang1024
[native] Add e2e table format test with native worker
eab17eb 
This is followup e2e test for prestodb#23923

For a partitioned table, when respect_table_format is set to false,
and table_format and hive_storage_format is different, the newly
added partition should use hive_storage_format.
kewang1024 added a commit to kewang1024/presto that referenced this pull request Nov 12, 2024
@kewang1024
[native] Add e2e table format test with native worker
2bcde5a 
This is followup e2e test for prestodb#23923

For a partitioned table, when respect_table_format is set to false,
and table_format and hive_storage_format is different, the newly
added partition should use hive_storage_format.
kewang1024 added a commit to kewang1024/presto that referenced this pull request Nov 12, 2024
@kewang1024
[native] Add e2e table format test with native worker
99cea1a 
This is followup e2e test for prestodb#23923

For a partitioned table, when respect_table_format is set to false,
and table_format and hive_storage_format is different, the newly
added partition should use hive_storage_format.
kewang1024 added a commit to kewang1024/presto that referenced this pull request Nov 12, 2024
@kewang1024
[native] Add e2e table format test with native worker
21ce6ff 
This is followup e2e test for prestodb#23923

For a partitioned table, when respect_table_format is set to false,
and table_format and hive_storage_format is different, the newly
added partition should use hive_storage_format.
kewang1024 added a commit that referenced this pull request Nov 12, 2024
@kewang1024
[native] Add e2e table format test with native worker
d1e7b5e 
This is followup e2e test for #23923

For a partitioned table, when respect_table_format is set to false,
and table_format and hive_storage_format is different, the newly
added partition should use hive_storage_format.
kewang1024 added a commit to kewang1024/presto that referenced this pull request Dec 10, 2024
@kewang1024
[Native] Fix table writer to use the actual storage format for insert
f9e204e 
prestodb#23923 fixed the create handle.
As a follwup, also fix the InsertHandle
@kewang1024 kewang1024 mentioned this pull request Dec 10, 2024
Merged
xiaoxmeng pushed a commit that referenced this pull request Dec 10, 2024
@kewang1024 @xiaoxmeng
[Native] Fix table writer to use the actual storage format for insert
b2184a8 
#23923 fixed the create handle.
As a follwup, also fix the InsertHandle
@kewang1024 kewang1024 mentioned this pull request Dec 12, 2024
Merged
denodo-research-labs pushed a commit to denodo-research-labs/presto that referenced this pull request Dec 12, 2024
@kewang1024 @denodo-research-labs
[Native] Fix table writer to use the actual storage format for insert
bfea522 
prestodb#23923 fixed the create handle.
As a follwup, also fix the InsertHandle
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Yuhta Yuhta Yuhta approved these changes

@NikhilCollooru NikhilCollooru NikhilCollooru approved these changes

@amitkdutta amitkdutta amitkdutta approved these changes

@allysa08 allysa08 allysa08 approved these changes

@presto-oss presto-oss Awaiting requested review from presto-oss presto-oss is a code owner automatically assigned from prestodb/committers

@spershin spershin Awaiting requested review from spershin

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@kewang1024 @Yuhta @NikhilCollooru @amitkdutta @allysa08