feat(native): Add customSerializedValue routing for binary connector deserialization#155
Open
20001020ycx wants to merge 5 commits into
Open
feat(native): Add customSerializedValue routing for binary connector deserialization#15520001020ycx wants to merge 5 commits into
20001020ycx wants to merge 5 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
4 tasks
20001020ycx
force-pushed
the
yscope/feat/custom-serialized-value-routing
branch
5 times, most recently
from
bb977a3 to
b25d4db
Compare
Routes customSerializedValue in from_json() to ConnectorProtocol::deserialize() for the 5 handle types used by the CLP connector: ColumnHandle, ConnectorTableHandle, ConnectorTableLayoutHandle, ConnectorSplit, ConnectorTransactionHandle. Also adds serialize/deserialize virtuals to ConnectorProtocol.h for ColumnHandle, and VELOX_NYI stubs to presto_protocol_core.h.
20001020ycx
force-pushed
the
yscope/feat/custom-serialized-value-routing
branch
4 times, most recently
from
e1eea3d to
becf9fc
Compare
20001020ycx
force-pushed
the
yscope/feat/custom-serialized-value-routing
branch
from
becf9fc to
6f307dc
Compare
Add tests verifying that from_json() correctly routes Base64-encoded customSerializedValue payloads to ConnectorProtocol::deserialize() for all 10 connector handle types.
…butedProcedureHandle Add customSerializedValue routing in ConnectorDistributedProcedureHandle.cpp.inc and add serialize/deserialize virtuals to ConnectorProtocol and ConnectorProtocolTemplate. Add test coverage for this handle type.
…edProcedureHandle Add VELOX_NYI serialize/deserialize stubs to ConnectorDistributedProcedureHandle so that the customSerializedValue routing added in the previous commit compiles. Without these stubs, IcebergDistributedProcedureHandle fails to compile when inheriting the ConnectorProtocol template methods.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
Add
customSerializedValuerouting infrom_json()for all 10 connector handle types. WhencustomSerializedValueis present in the incoming JSON, the routing Base64-decodes it and dispatches toConnectorProtocol::deserialize()instead of the default JSON-to-struct parsing.Motivation and Context
Enables C++ plugin connectors to use binary serialization codecs instead of JSON, as proposed in prestodb/rfcs#49.
This is the C++ worker-side counterpart to the Java
ConnectorCodec/CodecSerializerpipeline (prestodb#26257).Test Plan
presto_serverwith the changesplugin-dirRelease Notes