github-actions[bot] commented on code in PR #65660:
URL: https://github.com/apache/doris/pull/65660#discussion_r3587564814
##########
be/src/core/column/column_variant.cpp:
##########
@@ -1835,6 +1835,11 @@ bool ColumnVariant::is_visible_root_value(size_t nrow)
const {
}
}
+ const auto& sparse_offsets = serialized_sparse_column_offsets();
Review Comment:
This still misses sparse rows after any prior doc-snapshot row. Once this
new check returns false for a row with sparse entries,
`serialize_one_row_to_json_format()` immediately tests
`doc_value_column_map.get_offsets()[row_num] != 0`; that is the cumulative end
offset, so it remains true for later rows even when the current row has no
doc-value entries. If a result column is assembled from mixed-format or
compatibility rowsets where an earlier segment has doc-value snapshots and a
later row has sparse entries, the sparse row can serialize as `{}` and return
before the sparse map below is considered. Please gate the doc snapshot branch
with the current-row delta, e.g. the existing `has_doc_value_column(row_num)` /
`offsets[row_num - 1] < offsets[row_num]`, and add a unit case with a prior doc
row and a current sparse row.
##########
regression-test/data/variant_p0/test_topn_lazy_materialize_sparse_variant.out:
##########
@@ -0,0 +1,17 @@
+-- This file is automatically generated. You should know what you did if you
want to edit this
+-- !topn_lazy_sparse_variant --
+0 1 {} {}
+1 -1 {} [1,2,3]
+3 42 {"k":1} {}
+4 -9223372036854775808 [1,2,3] {"k":1}
+6 1 [1,2,3] {}
+7 0 [1,2,3] [1,2,3]
+9 \N {} []
Review Comment:
This expected row does not match the inserted value. The suite inserts
`col_variant = '{}'` for pk 9 (and again for pk 15), but the new oracle records
`[]`. Other rows inserted as `{}` in this same output remain `{}`, so this is
not just a display convention; it locks in an object-to-array change in the
regression that is supposed to protect VARIANT materialization correctness.
Please fix the underlying serialization case and regenerate this as `{}` (or
remove these rows from this regression and cover the object/array issue
separately).
--
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]
