Fix bug in PrestoVectorSerializers when serializing empty Vectors with complex types #9157
Conversation
This file contains 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
…h complex types (facebookincubator#9157) Summary: PrestoIterativeVectorSerializer currently only supports serializing empty Vectors/selections when the columns are flat primitives. If the columns are complex attempting to serialize them will trigger a SIGSEGV. PrestoBatchVectorSerializer doesn't support serializing empty Vectors/selections at all. The fixes VectorStream to initialize itself and its children correctly when the Vector is empty. It also updates PrestoBatchVectorSerializer to only call serialize on the children if the Vector is non-empty. With these two changes both serializers support any column types in empty Vectors. Differential Revision: D55090408
|
This pull request was exported from Phabricator. Differential Revision: D55090408 |
✅ Deploy Preview for meta-velox canceled.
|
8fca219 to
55178a0
Compare
|
This pull request was exported from Phabricator. Differential Revision: D55090408 |
|
This pull request has been merged in 1d95a47. |
Joe-Abraham
pushed a commit
to Joe-Abraham/velox
that referenced
this pull request
Jun 7, 2024
…h complex types (facebookincubator#9157) Summary: Pull Request resolved: facebookincubator#9157 PrestoIterativeVectorSerializer currently only supports serializing empty Vectors/selections when the columns are flat primitives. If the columns are complex attempting to serialize them will trigger a SIGSEGV. PrestoBatchVectorSerializer doesn't support serializing empty Vectors/selections at all. The fixes VectorStream to initialize itself and its children correctly when the Vector is empty. It also updates PrestoBatchVectorSerializer to only call serialize on the children if the Vector is non-empty. With these two changes both serializers support any column types in empty Vectors. Reviewed By: aozturk Differential Revision: D55090408 fbshipit-source-id: 720bba19c265f0e1ee7dc55245e344aa4cda29fa
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.
Summary:
PrestoIterativeVectorSerializer currently only supports serializing empty Vectors/selections when the columns are flat
primitives. If the columns are complex attempting to serialize them will trigger a SIGSEGV.
PrestoBatchVectorSerializer doesn't support serializing empty Vectors/selections at all.
The fixes VectorStream to initialize itself and its children correctly when the Vector is empty. It also updates
PrestoBatchVectorSerializer to only call serialize on the children if the Vector is non-empty. With these two changes both
serializers support any column types in empty Vectors.
Differential Revision: D55090408