-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Add support to provide thrift codec for connector specific fields #25242
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
58c0a15
to
0326cee
Compare
58be3d3
to
8b437b7
Compare
shangm2
commented
Jun 4, 2025
presto-hive/src/main/java/com/facebook/presto/hive/HiveSplitCodec.java
Outdated
Show resolved
Hide resolved
shangm2
commented
Jun 4, 2025
presto-hive/src/main/java/com/facebook/presto/hive/HiveTransactionHandleCodec.java
Outdated
Show resolved
Hide resolved
shangm2
commented
Jun 4, 2025
presto-main-base/src/main/java/com/facebook/presto/connector/ConnectorManager.java
Outdated
Show resolved
Hide resolved
shangm2
commented
Jun 4, 2025
presto-main-base/src/main/java/com/facebook/presto/server/thrift/SplitThriftCodec.java
Outdated
Show resolved
Hide resolved
shangm2
commented
Jun 4, 2025
presto-main-base/src/main/java/com/facebook/presto/server/thrift/SplitThriftCodec.java
Outdated
Show resolved
Hide resolved
shangm2
commented
Jun 4, 2025
presto-main-base/src/main/java/com/facebook/presto/server/thrift/MetadataUpdatesCodec.java
Outdated
Show resolved
Hide resolved
shangm2
commented
Jun 4, 2025
presto-main-base/src/main/java/com/facebook/presto/server/thrift/SplitThriftCodec.java
Outdated
Show resolved
Hide resolved
shangm2
commented
Jun 4, 2025
presto-main-base/src/main/java/com/facebook/presto/server/thrift/SplitThriftCodec.java
Outdated
Show resolved
Hide resolved
shangm2
commented
Jun 4, 2025
presto-main-base/src/main/java/com/facebook/presto/server/thrift/SplitThriftCodec.java
Outdated
Show resolved
Hide resolved
shangm2
commented
Jun 4, 2025
presto-main-base/src/main/java/com/facebook/presto/server/thrift/SplitThriftCodec.java
Outdated
Show resolved
Hide resolved
shangm2
commented
Jun 4, 2025
presto-main-base/src/main/java/com/facebook/presto/server/thrift/SplitThriftCodec.java
Outdated
Show resolved
Hide resolved
shangm2
commented
Jun 4, 2025
presto-spi/src/main/java/com/facebook/presto/spi/ConnectorSpecificCodec.java
Outdated
Show resolved
Hide resolved
shangm2
commented
Jun 4, 2025
presto-hive/src/main/java/com/facebook/presto/hive/HiveSplitCodec.java
Outdated
Show resolved
Hide resolved
arhimondr
reviewed
Jun 9, 2025
presto-hive/src/main/java/com/facebook/presto/hive/HiveTransactionHandleCodec.java
Outdated
Show resolved
Hide resolved
presto-main-base/src/main/java/com/facebook/presto/connector/ConnectorSpecificCodecManager.java
Outdated
Show resolved
Hide resolved
presto-main-base/src/main/java/com/facebook/presto/server/thrift/TableWriteInfoCodec.java
Outdated
Show resolved
Hide resolved
presto-main-base/src/main/java/com/facebook/presto/server/thrift/SplitThriftCodec.java
Outdated
Show resolved
Hide resolved
shangm2
commented
Jun 9, 2025
presto-spi/src/main/java/com/facebook/presto/spi/connector/ConnectorSpecificCodecProvider.java
Show resolved
Hide resolved
c4c8759
to
fe0490a
Compare
lga-zurich
pushed a commit
to lga-zurich/presto-exchange
that referenced
this pull request
Sep 2, 2025
## Description - Native changes to expand connector specific fields for thrift migration - Depends on prestodb#25242 ## Motivation and Context prestodb/rfcs#38 ## Test Plan Build package and test in verifier: 230498 ## Release Notes Please follow [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines) and fill in the release notes below. ``` == RELEASE NOTES == Prestissimo (Native Execution) Changes * Update thrift IDL to expand connector specific fields ```
lga-zurich
pushed a commit
to lga-zurich/presto-exchange
that referenced
this pull request
Sep 2, 2025
## Description 1. Extract the cleanup from prestodb#25242 ## Motivation and Context 1. clean up the codebase 2. separate concern from the original pr ## Impact <!---Describe any public API or user-facing feature change or any performance impact--> ## Test Plan 1. passed verifier test 2. ## Contributor checklist - [ ] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [ ] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [ ] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [ ] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [ ] Adequate tests were added if applicable. - [ ] CI passed. ## Release Notes ``` == NO RELEASE NOTE == ```
lga-zurich
pushed a commit
to lga-zurich/presto-exchange
that referenced
this pull request
Sep 2, 2025
…estodb#25242) ## Description TODO: Clean up the json field prestodb#25671 To keep the trunk cleaner and also move this pr forward, [we had a offline discussion and agree to update this pr with the following items](https://prestodb.slack.com/archives/C091L9VBAFR/p1752721296470699?thread_ts=1752715197.618069&cid=C091L9VBAFR): 1. Keep ConnectorSplit and ConnectorTransactionHandle in ConnectorCodecProvider. 2. Remove Hive implementation, in particular Hive*Split and Hive*TransactionHandle removing mixed JSON/Thrift support 3. Make ExecutionWriterTarget a Union and add support in `ConnectorCodecProvider` for related handles . 4. Deprecate MetadataUpdate. The original description: 1. Add thrift support for split and transaction handle 2. but only activated if the feature toggle is on and a proper connector specific codec is provided 3. **We will be using byte array for serialization for now and will iterate on the _interface definition within SPI_ to avoid unnecessary allocations for better performance.** 4. Instructions on how to use the thrift .idl file can be found in the [rfc](prestodb/rfcs#38). <img width="1035" height="427" alt="Screenshot 2025-07-16 at 07 58 01" src="https://github.com/user-attachments/assets/7f043ce7-7faa-43be-be8d-62d4ebdc81ee" /> 5. cpp side changes: prestodb#25595 ## Motivation and Context prestodb/rfcs#38 ## Impact <!---Describe any public API or user-facing feature change or any performance impact--> ## Test Plan 1. passed verifier run ## Contributor checklist - [ ] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [ ] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [ ] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [ ] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [ ] Adequate tests were added if applicable. - [ ] CI passed. ## Release Notes Please follow [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines) and fill in the release notes below. ``` == RELEASE NOTES == General Changes * Improve efficiency of coordinator by supporting thrift codec for connector-specific data. ```
lga-zurich
pushed a commit
to lga-zurich/presto-exchange
that referenced
this pull request
Sep 2, 2025
…riterTargetUnion (prestodb#25595) ## Description 1. Add thrift codec for remote split 2. Add support for ExecutionWriterTargetUnion, including expanding multiple connector specific data 3. Depends on prestodb#25242 ## Motivation and Context prestodb/rfcs#38 ## Impact <!---Describe any public API or user-facing feature change or any performance impact--> ## Test Plan - Verifier: 234471 - Performance testing ## Contributor checklist - [x] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [x] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [x] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [x] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [x] Adequate tests were added if applicable. - [ ] CI passed. ## Release Notes Please follow [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines) and fill in the release notes below. ``` == RELEASE NOTES == General Changes * Improve efficiency by supporting thrift codec for connector-specific data. ```
lga-zurich
pushed a commit
to lga-zurich/presto-exchange
that referenced
this pull request
Sep 8, 2025
## Description - Native changes to expand connector specific fields for thrift migration - Depends on prestodb#25242 ## Motivation and Context prestodb/rfcs#38 ## Test Plan Build package and test in verifier: 230498 ## Release Notes Please follow [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines) and fill in the release notes below. ``` == RELEASE NOTES == Prestissimo (Native Execution) Changes * Update thrift IDL to expand connector specific fields ```
lga-zurich
pushed a commit
to lga-zurich/presto-exchange
that referenced
this pull request
Sep 8, 2025
## Description 1. Extract the cleanup from prestodb#25242 ## Motivation and Context 1. clean up the codebase 2. separate concern from the original pr ## Impact <!---Describe any public API or user-facing feature change or any performance impact--> ## Test Plan 1. passed verifier test 2. ## Contributor checklist - [ ] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [ ] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [ ] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [ ] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [ ] Adequate tests were added if applicable. - [ ] CI passed. ## Release Notes ``` == NO RELEASE NOTE == ```
lga-zurich
pushed a commit
to lga-zurich/presto-exchange
that referenced
this pull request
Sep 8, 2025
…estodb#25242) ## Description TODO: Clean up the json field prestodb#25671 To keep the trunk cleaner and also move this pr forward, [we had a offline discussion and agree to update this pr with the following items](https://prestodb.slack.com/archives/C091L9VBAFR/p1752721296470699?thread_ts=1752715197.618069&cid=C091L9VBAFR): 1. Keep ConnectorSplit and ConnectorTransactionHandle in ConnectorCodecProvider. 2. Remove Hive implementation, in particular Hive*Split and Hive*TransactionHandle removing mixed JSON/Thrift support 3. Make ExecutionWriterTarget a Union and add support in `ConnectorCodecProvider` for related handles . 4. Deprecate MetadataUpdate. The original description: 1. Add thrift support for split and transaction handle 2. but only activated if the feature toggle is on and a proper connector specific codec is provided 3. **We will be using byte array for serialization for now and will iterate on the _interface definition within SPI_ to avoid unnecessary allocations for better performance.** 4. Instructions on how to use the thrift .idl file can be found in the [rfc](prestodb/rfcs#38). <img width="1035" height="427" alt="Screenshot 2025-07-16 at 07 58 01" src="https://github.com/user-attachments/assets/7f043ce7-7faa-43be-be8d-62d4ebdc81ee" /> 5. cpp side changes: prestodb#25595 ## Motivation and Context prestodb/rfcs#38 ## Impact <!---Describe any public API or user-facing feature change or any performance impact--> ## Test Plan 1. passed verifier run ## Contributor checklist - [ ] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [ ] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [ ] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [ ] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [ ] Adequate tests were added if applicable. - [ ] CI passed. ## Release Notes Please follow [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines) and fill in the release notes below. ``` == RELEASE NOTES == General Changes * Improve efficiency of coordinator by supporting thrift codec for connector-specific data. ```
lga-zurich
pushed a commit
to lga-zurich/presto-exchange
that referenced
this pull request
Sep 8, 2025
…riterTargetUnion (prestodb#25595) ## Description 1. Add thrift codec for remote split 2. Add support for ExecutionWriterTargetUnion, including expanding multiple connector specific data 3. Depends on prestodb#25242 ## Motivation and Context prestodb/rfcs#38 ## Impact <!---Describe any public API or user-facing feature change or any performance impact--> ## Test Plan - Verifier: 234471 - Performance testing ## Contributor checklist - [x] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [x] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [x] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [x] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [x] Adequate tests were added if applicable. - [ ] CI passed. ## Release Notes Please follow [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines) and fill in the release notes below. ``` == RELEASE NOTES == General Changes * Improve efficiency by supporting thrift codec for connector-specific data. ```
lga-zurich
pushed a commit
to lga-zurich/presto-exchange
that referenced
this pull request
Sep 8, 2025
## Description - Native changes to expand connector specific fields for thrift migration - Depends on prestodb#25242 ## Motivation and Context prestodb/rfcs#38 ## Test Plan Build package and test in verifier: 230498 ## Release Notes Please follow [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines) and fill in the release notes below. ``` == RELEASE NOTES == Prestissimo (Native Execution) Changes * Update thrift IDL to expand connector specific fields ```
lga-zurich
pushed a commit
to lga-zurich/presto-exchange
that referenced
this pull request
Sep 8, 2025
## Description 1. Extract the cleanup from prestodb#25242 ## Motivation and Context 1. clean up the codebase 2. separate concern from the original pr ## Impact <!---Describe any public API or user-facing feature change or any performance impact--> ## Test Plan 1. passed verifier test 2. ## Contributor checklist - [ ] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [ ] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [ ] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [ ] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [ ] Adequate tests were added if applicable. - [ ] CI passed. ## Release Notes ``` == NO RELEASE NOTE == ```
lga-zurich
pushed a commit
to lga-zurich/presto-exchange
that referenced
this pull request
Sep 8, 2025
…estodb#25242) ## Description TODO: Clean up the json field prestodb#25671 To keep the trunk cleaner and also move this pr forward, [we had a offline discussion and agree to update this pr with the following items](https://prestodb.slack.com/archives/C091L9VBAFR/p1752721296470699?thread_ts=1752715197.618069&cid=C091L9VBAFR): 1. Keep ConnectorSplit and ConnectorTransactionHandle in ConnectorCodecProvider. 2. Remove Hive implementation, in particular Hive*Split and Hive*TransactionHandle removing mixed JSON/Thrift support 3. Make ExecutionWriterTarget a Union and add support in `ConnectorCodecProvider` for related handles . 4. Deprecate MetadataUpdate. The original description: 1. Add thrift support for split and transaction handle 2. but only activated if the feature toggle is on and a proper connector specific codec is provided 3. **We will be using byte array for serialization for now and will iterate on the _interface definition within SPI_ to avoid unnecessary allocations for better performance.** 4. Instructions on how to use the thrift .idl file can be found in the [rfc](prestodb/rfcs#38). <img width="1035" height="427" alt="Screenshot 2025-07-16 at 07 58 01" src="https://github.com/user-attachments/assets/7f043ce7-7faa-43be-be8d-62d4ebdc81ee" /> 5. cpp side changes: prestodb#25595 ## Motivation and Context prestodb/rfcs#38 ## Impact <!---Describe any public API or user-facing feature change or any performance impact--> ## Test Plan 1. passed verifier run ## Contributor checklist - [ ] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [ ] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [ ] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [ ] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [ ] Adequate tests were added if applicable. - [ ] CI passed. ## Release Notes Please follow [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines) and fill in the release notes below. ``` == RELEASE NOTES == General Changes * Improve efficiency of coordinator by supporting thrift codec for connector-specific data. ```
lga-zurich
pushed a commit
to lga-zurich/presto-exchange
that referenced
this pull request
Sep 8, 2025
…riterTargetUnion (prestodb#25595) ## Description 1. Add thrift codec for remote split 2. Add support for ExecutionWriterTargetUnion, including expanding multiple connector specific data 3. Depends on prestodb#25242 ## Motivation and Context prestodb/rfcs#38 ## Impact <!---Describe any public API or user-facing feature change or any performance impact--> ## Test Plan - Verifier: 234471 - Performance testing ## Contributor checklist - [x] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [x] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [x] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [x] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [x] Adequate tests were added if applicable. - [ ] CI passed. ## Release Notes Please follow [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines) and fill in the release notes below. ``` == RELEASE NOTES == General Changes * Improve efficiency by supporting thrift codec for connector-specific data. ```
lga-zurich
pushed a commit
to lga-zurich/presto-exchange
that referenced
this pull request
Sep 8, 2025
## Description - Native changes to expand connector specific fields for thrift migration - Depends on prestodb#25242 ## Motivation and Context prestodb/rfcs#38 ## Test Plan Build package and test in verifier: 230498 ## Release Notes Please follow [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines) and fill in the release notes below. ``` == RELEASE NOTES == Prestissimo (Native Execution) Changes * Update thrift IDL to expand connector specific fields ```
lga-zurich
pushed a commit
to lga-zurich/presto-exchange
that referenced
this pull request
Sep 8, 2025
## Description 1. Extract the cleanup from prestodb#25242 ## Motivation and Context 1. clean up the codebase 2. separate concern from the original pr ## Impact <!---Describe any public API or user-facing feature change or any performance impact--> ## Test Plan 1. passed verifier test 2. ## Contributor checklist - [ ] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [ ] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [ ] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [ ] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [ ] Adequate tests were added if applicable. - [ ] CI passed. ## Release Notes ``` == NO RELEASE NOTE == ```
lga-zurich
pushed a commit
to lga-zurich/presto-exchange
that referenced
this pull request
Sep 8, 2025
…estodb#25242) ## Description TODO: Clean up the json field prestodb#25671 To keep the trunk cleaner and also move this pr forward, [we had a offline discussion and agree to update this pr with the following items](https://prestodb.slack.com/archives/C091L9VBAFR/p1752721296470699?thread_ts=1752715197.618069&cid=C091L9VBAFR): 1. Keep ConnectorSplit and ConnectorTransactionHandle in ConnectorCodecProvider. 2. Remove Hive implementation, in particular Hive*Split and Hive*TransactionHandle removing mixed JSON/Thrift support 3. Make ExecutionWriterTarget a Union and add support in `ConnectorCodecProvider` for related handles . 4. Deprecate MetadataUpdate. The original description: 1. Add thrift support for split and transaction handle 2. but only activated if the feature toggle is on and a proper connector specific codec is provided 3. **We will be using byte array for serialization for now and will iterate on the _interface definition within SPI_ to avoid unnecessary allocations for better performance.** 4. Instructions on how to use the thrift .idl file can be found in the [rfc](prestodb/rfcs#38). <img width="1035" height="427" alt="Screenshot 2025-07-16 at 07 58 01" src="https://github.com/user-attachments/assets/7f043ce7-7faa-43be-be8d-62d4ebdc81ee" /> 5. cpp side changes: prestodb#25595 ## Motivation and Context prestodb/rfcs#38 ## Impact <!---Describe any public API or user-facing feature change or any performance impact--> ## Test Plan 1. passed verifier run ## Contributor checklist - [ ] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [ ] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [ ] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [ ] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [ ] Adequate tests were added if applicable. - [ ] CI passed. ## Release Notes Please follow [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines) and fill in the release notes below. ``` == RELEASE NOTES == General Changes * Improve efficiency of coordinator by supporting thrift codec for connector-specific data. ```
lga-zurich
pushed a commit
to lga-zurich/presto-exchange
that referenced
this pull request
Sep 8, 2025
…riterTargetUnion (prestodb#25595) ## Description 1. Add thrift codec for remote split 2. Add support for ExecutionWriterTargetUnion, including expanding multiple connector specific data 3. Depends on prestodb#25242 ## Motivation and Context prestodb/rfcs#38 ## Impact <!---Describe any public API or user-facing feature change or any performance impact--> ## Test Plan - Verifier: 234471 - Performance testing ## Contributor checklist - [x] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [x] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [x] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [x] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [x] Adequate tests were added if applicable. - [ ] CI passed. ## Release Notes Please follow [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines) and fill in the release notes below. ``` == RELEASE NOTES == General Changes * Improve efficiency by supporting thrift codec for connector-specific data. ```
lga-zurich
pushed a commit
to lga-zurich/presto-exchange
that referenced
this pull request
Sep 8, 2025
## Description - Native changes to expand connector specific fields for thrift migration - Depends on prestodb#25242 ## Motivation and Context prestodb/rfcs#38 ## Test Plan Build package and test in verifier: 230498 ## Release Notes Please follow [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines) and fill in the release notes below. ``` == RELEASE NOTES == Prestissimo (Native Execution) Changes * Update thrift IDL to expand connector specific fields ```
lga-zurich
pushed a commit
to lga-zurich/presto-exchange
that referenced
this pull request
Sep 8, 2025
## Description 1. Extract the cleanup from prestodb#25242 ## Motivation and Context 1. clean up the codebase 2. separate concern from the original pr ## Impact <!---Describe any public API or user-facing feature change or any performance impact--> ## Test Plan 1. passed verifier test 2. ## Contributor checklist - [ ] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [ ] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [ ] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [ ] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [ ] Adequate tests were added if applicable. - [ ] CI passed. ## Release Notes ``` == NO RELEASE NOTE == ```
lga-zurich
pushed a commit
to lga-zurich/presto-exchange
that referenced
this pull request
Sep 8, 2025
…estodb#25242) ## Description TODO: Clean up the json field prestodb#25671 To keep the trunk cleaner and also move this pr forward, [we had a offline discussion and agree to update this pr with the following items](https://prestodb.slack.com/archives/C091L9VBAFR/p1752721296470699?thread_ts=1752715197.618069&cid=C091L9VBAFR): 1. Keep ConnectorSplit and ConnectorTransactionHandle in ConnectorCodecProvider. 2. Remove Hive implementation, in particular Hive*Split and Hive*TransactionHandle removing mixed JSON/Thrift support 3. Make ExecutionWriterTarget a Union and add support in `ConnectorCodecProvider` for related handles . 4. Deprecate MetadataUpdate. The original description: 1. Add thrift support for split and transaction handle 2. but only activated if the feature toggle is on and a proper connector specific codec is provided 3. **We will be using byte array for serialization for now and will iterate on the _interface definition within SPI_ to avoid unnecessary allocations for better performance.** 4. Instructions on how to use the thrift .idl file can be found in the [rfc](prestodb/rfcs#38). <img width="1035" height="427" alt="Screenshot 2025-07-16 at 07 58 01" src="https://github.com/user-attachments/assets/7f043ce7-7faa-43be-be8d-62d4ebdc81ee" /> 5. cpp side changes: prestodb#25595 ## Motivation and Context prestodb/rfcs#38 ## Impact <!---Describe any public API or user-facing feature change or any performance impact--> ## Test Plan 1. passed verifier run ## Contributor checklist - [ ] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [ ] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [ ] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [ ] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [ ] Adequate tests were added if applicable. - [ ] CI passed. ## Release Notes Please follow [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines) and fill in the release notes below. ``` == RELEASE NOTES == General Changes * Improve efficiency of coordinator by supporting thrift codec for connector-specific data. ```
lga-zurich
pushed a commit
to lga-zurich/presto-exchange
that referenced
this pull request
Sep 8, 2025
…riterTargetUnion (prestodb#25595) ## Description 1. Add thrift codec for remote split 2. Add support for ExecutionWriterTargetUnion, including expanding multiple connector specific data 3. Depends on prestodb#25242 ## Motivation and Context prestodb/rfcs#38 ## Impact <!---Describe any public API or user-facing feature change or any performance impact--> ## Test Plan - Verifier: 234471 - Performance testing ## Contributor checklist - [x] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [x] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [x] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [x] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [x] Adequate tests were added if applicable. - [ ] CI passed. ## Release Notes Please follow [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines) and fill in the release notes below. ``` == RELEASE NOTES == General Changes * Improve efficiency by supporting thrift codec for connector-specific data. ```
lga-zurich
pushed a commit
to lga-zurich/presto-exchange
that referenced
this pull request
Sep 8, 2025
## Description - Native changes to expand connector specific fields for thrift migration - Depends on prestodb#25242 ## Motivation and Context prestodb/rfcs#38 ## Test Plan Build package and test in verifier: 230498 ## Release Notes Please follow [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines) and fill in the release notes below. ``` == RELEASE NOTES == Prestissimo (Native Execution) Changes * Update thrift IDL to expand connector specific fields ```
lga-zurich
pushed a commit
to lga-zurich/presto-exchange
that referenced
this pull request
Sep 8, 2025
## Description 1. Extract the cleanup from prestodb#25242 ## Motivation and Context 1. clean up the codebase 2. separate concern from the original pr ## Impact <!---Describe any public API or user-facing feature change or any performance impact--> ## Test Plan 1. passed verifier test 2. ## Contributor checklist - [ ] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [ ] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [ ] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [ ] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [ ] Adequate tests were added if applicable. - [ ] CI passed. ## Release Notes ``` == NO RELEASE NOTE == ```
lga-zurich
pushed a commit
to lga-zurich/presto-exchange
that referenced
this pull request
Sep 8, 2025
…estodb#25242) ## Description TODO: Clean up the json field prestodb#25671 To keep the trunk cleaner and also move this pr forward, [we had a offline discussion and agree to update this pr with the following items](https://prestodb.slack.com/archives/C091L9VBAFR/p1752721296470699?thread_ts=1752715197.618069&cid=C091L9VBAFR): 1. Keep ConnectorSplit and ConnectorTransactionHandle in ConnectorCodecProvider. 2. Remove Hive implementation, in particular Hive*Split and Hive*TransactionHandle removing mixed JSON/Thrift support 3. Make ExecutionWriterTarget a Union and add support in `ConnectorCodecProvider` for related handles . 4. Deprecate MetadataUpdate. The original description: 1. Add thrift support for split and transaction handle 2. but only activated if the feature toggle is on and a proper connector specific codec is provided 3. **We will be using byte array for serialization for now and will iterate on the _interface definition within SPI_ to avoid unnecessary allocations for better performance.** 4. Instructions on how to use the thrift .idl file can be found in the [rfc](prestodb/rfcs#38). <img width="1035" height="427" alt="Screenshot 2025-07-16 at 07 58 01" src="https://github.com/user-attachments/assets/7f043ce7-7faa-43be-be8d-62d4ebdc81ee" /> 5. cpp side changes: prestodb#25595 ## Motivation and Context prestodb/rfcs#38 ## Impact <!---Describe any public API or user-facing feature change or any performance impact--> ## Test Plan 1. passed verifier run ## Contributor checklist - [ ] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [ ] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [ ] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [ ] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [ ] Adequate tests were added if applicable. - [ ] CI passed. ## Release Notes Please follow [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines) and fill in the release notes below. ``` == RELEASE NOTES == General Changes * Improve efficiency of coordinator by supporting thrift codec for connector-specific data. ```
lga-zurich
pushed a commit
to lga-zurich/presto-exchange
that referenced
this pull request
Sep 8, 2025
…riterTargetUnion (prestodb#25595) ## Description 1. Add thrift codec for remote split 2. Add support for ExecutionWriterTargetUnion, including expanding multiple connector specific data 3. Depends on prestodb#25242 ## Motivation and Context prestodb/rfcs#38 ## Impact <!---Describe any public API or user-facing feature change or any performance impact--> ## Test Plan - Verifier: 234471 - Performance testing ## Contributor checklist - [x] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [x] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [x] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [x] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [x] Adequate tests were added if applicable. - [ ] CI passed. ## Release Notes Please follow [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines) and fill in the release notes below. ``` == RELEASE NOTES == General Changes * Improve efficiency by supporting thrift codec for connector-specific data. ```
lga-zurich
pushed a commit
to lga-zurich/presto-exchange
that referenced
this pull request
Sep 8, 2025
## Description - Native changes to expand connector specific fields for thrift migration - Depends on prestodb#25242 ## Motivation and Context prestodb/rfcs#38 ## Test Plan Build package and test in verifier: 230498 ## Release Notes Please follow [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines) and fill in the release notes below. ``` == RELEASE NOTES == Prestissimo (Native Execution) Changes * Update thrift IDL to expand connector specific fields ```
lga-zurich
pushed a commit
to lga-zurich/presto-exchange
that referenced
this pull request
Sep 8, 2025
## Description 1. Extract the cleanup from prestodb#25242 ## Motivation and Context 1. clean up the codebase 2. separate concern from the original pr ## Impact <!---Describe any public API or user-facing feature change or any performance impact--> ## Test Plan 1. passed verifier test 2. ## Contributor checklist - [ ] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [ ] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [ ] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [ ] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [ ] Adequate tests were added if applicable. - [ ] CI passed. ## Release Notes ``` == NO RELEASE NOTE == ```
lga-zurich
pushed a commit
to lga-zurich/presto-exchange
that referenced
this pull request
Sep 8, 2025
…estodb#25242) ## Description TODO: Clean up the json field prestodb#25671 To keep the trunk cleaner and also move this pr forward, [we had a offline discussion and agree to update this pr with the following items](https://prestodb.slack.com/archives/C091L9VBAFR/p1752721296470699?thread_ts=1752715197.618069&cid=C091L9VBAFR): 1. Keep ConnectorSplit and ConnectorTransactionHandle in ConnectorCodecProvider. 2. Remove Hive implementation, in particular Hive*Split and Hive*TransactionHandle removing mixed JSON/Thrift support 3. Make ExecutionWriterTarget a Union and add support in `ConnectorCodecProvider` for related handles . 4. Deprecate MetadataUpdate. The original description: 1. Add thrift support for split and transaction handle 2. but only activated if the feature toggle is on and a proper connector specific codec is provided 3. **We will be using byte array for serialization for now and will iterate on the _interface definition within SPI_ to avoid unnecessary allocations for better performance.** 4. Instructions on how to use the thrift .idl file can be found in the [rfc](prestodb/rfcs#38). <img width="1035" height="427" alt="Screenshot 2025-07-16 at 07 58 01" src="https://github.com/user-attachments/assets/7f043ce7-7faa-43be-be8d-62d4ebdc81ee" /> 5. cpp side changes: prestodb#25595 ## Motivation and Context prestodb/rfcs#38 ## Impact <!---Describe any public API or user-facing feature change or any performance impact--> ## Test Plan 1. passed verifier run ## Contributor checklist - [ ] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [ ] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [ ] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [ ] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [ ] Adequate tests were added if applicable. - [ ] CI passed. ## Release Notes Please follow [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines) and fill in the release notes below. ``` == RELEASE NOTES == General Changes * Improve efficiency of coordinator by supporting thrift codec for connector-specific data. ```
lga-zurich
pushed a commit
to lga-zurich/presto-exchange
that referenced
this pull request
Sep 8, 2025
…riterTargetUnion (prestodb#25595) ## Description 1. Add thrift codec for remote split 2. Add support for ExecutionWriterTargetUnion, including expanding multiple connector specific data 3. Depends on prestodb#25242 ## Motivation and Context prestodb/rfcs#38 ## Impact <!---Describe any public API or user-facing feature change or any performance impact--> ## Test Plan - Verifier: 234471 - Performance testing ## Contributor checklist - [x] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [x] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [x] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [x] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [x] Adequate tests were added if applicable. - [ ] CI passed. ## Release Notes Please follow [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines) and fill in the release notes below. ``` == RELEASE NOTES == General Changes * Improve efficiency by supporting thrift codec for connector-specific data. ```
This was referenced Sep 11, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
TODO: Clean up the json field #25671
To keep the trunk cleaner and also move this pr forward, we had a offline discussion and agree to update this pr with the following items:
ConnectorCodecProvider
for related handles .The original description:
Motivation and Context
prestodb/rfcs#38
Impact
Test Plan
Contributor checklist
Release Notes
Please follow release notes guidelines and fill in the release notes below.