[fix](be) Fix QuantileState protobuf serialization in write/read_column_to_pb#64152
Open
heguanhui wants to merge 1 commit into
Open
[fix](be) Fix QuantileState protobuf serialization in write/read_column_to_pb#64152heguanhui wants to merge 1 commit into
heguanhui wants to merge 1 commit into
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
/review |
Contributor
Author
|
run buildall |
heguanhui
force-pushed
the
fix/quantile-state-serde-pb-serialization
branch
from
78d120b to
c3f87fe
Compare
…mn_to_pb ### What problem does this PR solve? Issue Number: close apache#64149 Problem Summary: DataTypeQuantileStateSerDe::write_column_to_pb used get_data_at() to copy sizeof(QuantileState) bytes of raw memory, but QuantileState contains heap-allocated members (shared_ptr<TDigest>, vector<double>) whose data is not stored inline. read_column_from_pb then used insert_data() to reinterpret this incomplete memory as a QuantileState object, resulting in dangling pointers and data corruption. In DEBUG/ASAN builds this caused test failures (crash or assertion errors); in RELEASE builds it appeared to work by accident (undefined behavior accessing stale heap pointers). Root cause: The serialization was fundamentally wrong — copying sizeof(QuantileState) bytes never captures the heap data. The correct approach (matching HLL/Bitmap implementations) is to call QuantileState::serialize()/deserialize() which properly handle the full object state. Additionally, write_column_to_pb was missing the PGenericType::QUANTILE_STATE type id assignment that HLL and Bitmap both set. The test also had a bug: it compared QuantileState objects via get_data_at() (raw memory comparison), which compares pointer values rather than logical equality. Fixed to compare via serialize() output. ### Release note Fixed QuantileState type corruption in RPC UDF / RPC aggregate function / fold constant paths. Previously, QuantileState values could be silently corrupted or lost when transmitted via protobuf. ### Check List (For Author) - Test: Unit Test (QuantileStateSerdeTest.writeColumnToPb) - Behavior changed: No (fixes broken behavior) - Does this need documentation: No
heguanhui
force-pushed
the
fix/quantile-state-serde-pb-serialization
branch
from
c3f87fe to
9f6743d
Compare
Contributor
Author
|
run buildall |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
TPC-H: Total hot run time: 29375 ms |
Contributor
TPC-DS: Total hot run time: 169788 ms |
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
2 participants
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.
What problem does this PR solve?
Issue Number: close #64149
Problem Summary:
DataTypeQuantileStateSerDe::write_column_to_pbusedget_data_at()to copysizeof(QuantileState)bytes of raw memory, butQuantileStatecontains heap-allocated members (shared_ptr<TDigest>,vector<double>) whose data is not stored inline.read_column_from_pbthen usedinsert_data()to reinterpret this incomplete memory as aQuantileStateobject, resulting in dangling pointers and data corruption.Additionally,
write_column_to_pbwas missing thePGenericType::QUANTILE_STATEtype id assignment.The test also had a bug: it compared
QuantileStateobjects viaget_data_at()(raw memory comparison), which compares pointer values rather than logical equality.Release note
Fixed QuantileState type corruption in RPC UDF / RPC aggregate function / fold constant paths. Previously, QuantileState values could be silently corrupted or lost when transmitted via protobuf.
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)