github-actions[bot] commented on code in PR #68125:
URL: https://github.com/apache/doris/pull/68125#discussion_r4226457853
##########
be/src/exec/rowid_fetcher.cpp:
##########
@@ -995,61 +1044,63 @@ Status RowIdStorageReader::read_doris_format_row(
segment = seg_item.segment;
}
+ if (fetch_columns.size() < slots.size()) {
+ return Status::InternalError(
+ "fetch request carries {} column descs for {} slots, slot {}
has none",
+ fetch_columns.size(), slots.size(),
slots[fetch_columns.size()].col_name());
+ }
+
// if row_store_read_struct not empty, means the line we should read from
row_store
if (!row_store_read_struct.default_values.empty()) {
if (!tablet->tablet_schema()->has_row_store_for_all_columns()) {
return Status::InternalError("Tablet {} does not have row store
for all columns",
tablet->tablet_id());
}
- auto result_columns_guard = result_block.mutate_columns_scoped();
- MutableColumns& result_columns =
result_columns_guard.mutable_columns();
- io::IOContext io_ctx;
- io_ctx.reader_type = ReaderType::READER_QUERY;
- io_ctx.file_cache_stats = &stats.file_cache_stats;
- io_ctx.file_cache_miss_policy = file_cache_miss_policy;
- for (auto row_id : row_ids) {
- RowLocation loc(rowset_id, segment->id(),
cast_set<uint32_t>(row_id));
- row_store_read_struct.row_store_buffer.clear();
- RETURN_IF_ERROR(scope_timer_run(
- [&]() {
- return tablet->lookup_row_data({}, loc, rowset, stats,
-
row_store_read_struct.row_store_buffer,
- false, &io_ctx);
- },
- lookup_row_data_ms));
-
- RETURN_IF_ERROR(JsonbSerializeUtil::jsonb_to_columns(
- row_store_read_struct.serdes,
row_store_read_struct.row_store_buffer.data(),
- row_store_read_struct.row_store_buffer.size(),
- row_store_read_struct.col_uid_to_idx, result_columns,
- row_store_read_struct.default_values, {}));
+ if (row_store_read_struct.decode_row_store) {
+ auto result_columns_guard = result_block.mutate_columns_scoped();
+ MutableColumns& result_columns =
result_columns_guard.mutable_columns();
+ io::IOContext io_ctx;
+ io_ctx.reader_type = ReaderType::READER_QUERY;
+ io_ctx.file_cache_stats = &stats.file_cache_stats;
+ io_ctx.file_cache_miss_policy = file_cache_miss_policy;
+ for (auto row_id : row_ids) {
+ RowLocation loc(rowset_id, segment->id(),
cast_set<uint32_t>(row_id));
+ row_store_read_struct.row_store_buffer.clear();
+ RETURN_IF_ERROR(scope_timer_run(
+ [&]() {
+ return tablet->lookup_row_data({}, loc, rowset,
stats,
+
row_store_read_struct.row_store_buffer,
+ false, &io_ctx);
+ },
+ lookup_row_data_ms));
+
+ RETURN_IF_ERROR(JsonbSerializeUtil::jsonb_to_columns(
+ row_store_read_struct.serdes,
row_store_read_struct.row_store_buffer.data(),
+ row_store_read_struct.row_store_buffer.size(),
+ row_store_read_struct.col_uid_to_idx, result_columns,
+ row_store_read_struct.default_values,
+ row_store_read_struct.include_col_uids));
Review Comment:
[P1] Keep the JSONB row count aligned when a hidden slot comes first. A
full-row-store lazy TopN fetch can request VERSION or COMMIT_TSO as slot 0,
then batch two rows from one segment. This branch decodes both JSONB rows
before the hidden slot is populated, but `jsonb_to_columns()` takes `num_rows`
from `dst_columns[0]->size()`. It therefore sees zero again on the second row:
present ordinary fields trip its size `DCHECK`, and a field absent from older
JSONB after schema change gets only one default, leaving the output columns
uneven for scattering. Please make the decoder's row count independent of the
excluded slot (or fill that slot per row) and cover a hidden-first, multi-row
lazy fetch with a missing JSONB field.
--
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]
