zclllyybb commented on issue #64184: URL: https://github.com/apache/doris/issues/64184#issuecomment-4643089186
Breakwater-GitHub-Analysis-Slot: slot_2ef9d6a05a2c Initial analysis: this looks like a valid report. On current upstream `master`, `ColumnComplexType<T>::get_data_at()` returns a `StringRef` over `reinterpret_cast<const char*>(&data[n])` with `sizeof(data[n])`, and `ColumnBitmap`, `ColumnHLL`, and `ColumnQuantileState` are aliases of that template. The stored value types (`BitmapValue`, `HyperLogLog`, and `QuantileState`) are non-trivial C++ objects with containers, heap-backed state, pointers/shared pointers, and padding. Their raw object bytes are therefore not a canonical logical representation; two logically equal values can have different raw bytes. So the failing assertions in `ColumnComplexTest.GetDataAtTest` are not proving a value mismatch. They are comparing implementation/object representation. The direction in the open PR #64185 is correct for the immediate UT failure: compare logical values (`get_element(...).to_string()` for bitmap/HLL and `operator[]` or another logical QuantileState check) instead of comparing `get_data_at()` byte strings. One related code path is still worth checking before closing this as test-only. Bitmap and HLL protobuf serdes already serialize logical binary payloads, but `DataTypeQuantileStateSerDe::write_column_to_pb()` currently writes `column.get_data_at(row_num)` bytes and `read_column_from_pb()` feeds those bytes into `insert_data()`. That has the same raw-object-byte problem and is unsafe as a durable/cross-process protobuf representation. The safer fix direction is to set the protobuf type to `PGenericType::QUANTILE_STATE`, serialize each `QuantileState` via `get_serialized_size()` / `serialize()`, and deserialize via `QuantileState::deserialize(Slice(...))` or the existing logical binary insertion path. Missing information: the issue already has enough information to classify the UT failure. For CI tracking, it would still be useful to attach the exact compiler/sanitizer/OS and the full `run-be-ut.sh --run --filter=ColumnComplexTest.GetDataAtTest` output from the failing run. Suggested next steps: - Keep the logical-comparison change in #64185 for this unit test. - Add or update QuantileState protobuf serde coverage so it compares logical values after round-trip and verifies the protobuf type id, instead of using `get_data_at()` equality. - Audit any remaining complex-column `get_data_at()` / `insert_data()` use and keep it out of serialization or persistence paths unless the in-process object-copy contract is explicitly intended and documented. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
