Doris-Breakwater commented on issue #69931:
URL: https://github.com/apache/doris/issues/69931#issuecomment-6564545915

   Breakwater-GitHub-Analysis-Slot: slot_30e36283ef43
   
   The reported root cause is supported by source inspection: the complex 
`collect_list` intermediate state is not a lossless round trip for this STRING 
value. This is actionable as a BE aggregate-state serialization defect. The 
issue is currently open with no labels; aggregation / complex-type 
serialization is the appropriate triage area.
   
   **Verified evidence**
   
   I inspected the local `4.1.4` tag 
(`ad35a140c7fd0b842f18c23300bac581f7d04326`) and master snapshot 
(`82646c38c0069eee4b1704eafef416f969bd4b93`). The relevant collect-state 
write/read and struct/string serialization logic matches between these 
snapshots, ignoring whitespace. This does not establish every affected 4.1.x 
release or a fixed release.
   
   - 
[`AggregateFunctionCollectListData::write/read`](https://github.com/apache/doris/blob/ad35a140c7fd0b842f18c23300bac581f7d04326/be/src/exprs/aggregate/aggregate_function_collect.h#L308)
 writes a row count, one text-serialized complex value per row, and `max_size`. 
Reading invokes `deserialize_one_cell_from_json` and wraps its failure in the 
reported `INTERNAL_ERROR`. The options leave `escape_char` at its default zero.
   - The [nested STRING 
writer](https://github.com/apache/doris/blob/82646c38c0069eee4b1704eafef416f969bd4b93/be/src/core/data_type_serde/data_type_string_serde.cpp#L217)
 adds surrounding double quotes but writes the value bytes unchanged when 
escaping is disabled. The [struct 
reader](https://github.com/apache/doris/blob/82646c38c0069eee4b1704eafef416f969bd4b93/be/src/core/data_type_serde/data_type_struct_serde.cpp#L118)
 interprets both quote types and backslashes. For the supplied value, an 
embedded double quote closes the string span and the apostrophe opens a 
single-quoted span that remains open. The final struct field is consequently 
not appended, explaining “Actual struct field number 1 is less than schema 
field number 2.”
   - A small standalone model of that reader's delimiter/quote logic produced 
one field with an open quote for the supplied value, versus two fields for 
apostrophe-only and JSON-without-apostrophe controls. This is supporting parser 
evidence, **not an end-to-end Doris SQL reproduction**.
   - [`array_agg` selects the collect implementation for non-nullable 
arguments](https://github.com/apache/doris/blob/82646c38c0069eee4b1704eafef416f969bd4b93/be/src/exprs/aggregate/aggregate_function_array_agg.cpp#L27),
 which explains the shared failure for the supplied STRUCT expression. Nullable 
`array_agg` inputs use a different implementation and should be tested 
separately.
   
   **Suggested fix and validation**
   
   The proposed binary state representation is a sound direction. 
[`SingleValueDataComplexType`](https://github.com/apache/doris/blob/82646c38c0069eee4b1704eafef416f969bd4b93/be/src/exprs/aggregate/aggregate_function_min_max.h#L543)
 provides the existing size/serialize/deserialize pattern and passes 
`be_exec_version`. Struct binary serialization delegates to its child types; 
STRING serialization preserves offsets and character bytes without quote 
parsing. Preserve the collect limit and merge semantics when adapting that 
pattern.
   
   The compatibility gate must cover the **aggregate-state format change**, in 
addition to passing the execution version into datatype serialization. A new 
reader accepting both formats alone does not make a new binary writer 
compatible with an old reader. Keep legacy writes for execution versions 
requiring the old format, or establish another explicit compatibility 
mechanism, and test both old-writer/new-reader and new-writer/old-reader 
exchange under the supported upgrade protocol.
   
   Regression coverage should include the supplied query under phases 1 and 2, 
both functions, exact recovered field values as well as `SIZE`, 
quote/backslash/delimiter combinations, nested arrays/maps/structs, null and 
empty inputs, and limited `collect_list`. Include aggregate-state 
serialize/deserialize/merge tests. Merely avoiding an exception is insufficient 
to prove preservation of values.
   
   **Remaining information and interim action**
   
   Please provide the exact failing patch/build (`SELECT VERSION()` and FE/BE 
build versions, including whether versions are mixed). The table-free SQL and 
error already provide enough detail to start a regression test; a full profile 
or general log bundle is not needed initially. If that exact build behaves 
differently, add `EXPLAIN VERBOSE` for both phase settings and the query ID 
plus corresponding BE exception excerpt. Please also provide a concrete value 
producing the “more than schema” variant to cover it independently.
   
   For the supplied query, the reported `SET agg_phase = 1` workaround is 
consistent with avoiding intermediate collect-state serialization. Keep it 
scoped to affected queries/sessions and verify the plan and result; it is not a 
serialization fix or a performance recommendation. No repository code was 
changed, and no fixed version or cluster-level reproduction is claimed here.
   


-- 
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]

Reply via email to