github-actions[bot] commented on code in PR #65684:
URL: https://github.com/apache/doris/pull/65684#discussion_r4214939756
##########
be/src/storage/segment/segment.cpp:
##########
@@ -1056,26 +1013,181 @@ Status Segment::traverse_column_meta_pbs(const
std::function<void(const ColumnMe
return _column_meta_accessor->traverse_metas(*footer_pb_shared, visitor,
&dummy_stats);
}
-Status Segment::get_column_reader(const TabletColumn& col,
- std::shared_ptr<ColumnReader>* column_reader,
- OlapReaderStatistics* stats, const
io::IOContext* source_io_ctx,
- std::optional<Field> const_value) {
- RETURN_IF_ERROR(_create_column_meta_once(stats, source_io_ctx));
- SCOPED_RAW_TIMER(&stats->segment_create_column_readers_timer_ns);
- int col_uid = col.unique_id() >= 0 ? col.unique_id() :
col.parent_unique_id();
- // The column is not in this segment, return nullptr
- if (!_tablet_schema->has_column_unique_id(col_uid)) {
- *column_reader = nullptr;
- return Status::Error<ErrorCode::NOT_FOUND, false>("column not found in
segment, col_uid={}",
- col_uid);
- }
- if (col.has_path_info()) {
- PathInData relative_path = col.path_info_ptr()->copy_pop_front();
- return _column_reader_cache->get_path_column_reader(col_uid,
relative_path, column_reader,
- stats, nullptr,
source_io_ctx);
- }
- return _column_reader_cache->get_column_reader(col_uid, column_reader,
stats, source_io_ctx,
- std::move(const_value));
+Status Segment::_get_column_reader_for_read(const TabletColumn& col,
+ const StorageReadOptions&
read_options,
+ std::shared_ptr<ColumnReader>*
column_reader) {
+ DORIS_CHECK(read_options.stats != nullptr);
+ const int32_t col_uid = col.unique_id() >= 0 ? col.unique_id() :
col.parent_unique_id();
+ DORIS_CHECK_GE(col_uid, 0) << "column does not have a resolvable uid: " <<
col.debug_string();
+ RETURN_IF_ERROR(_create_column_meta_once(read_options.stats,
&read_options.io_ctx));
+
SCOPED_RAW_TIMER(&read_options.stats->segment_create_column_readers_timer_ns);
+
+ if (col.name() == VERSION_COL) {
+ // A singleton rowset exposes its rowset version for every row.
Segment writers store a
+ // placeholder because the publish version is not known when the
segment is written.
+ if (read_options.version.first == read_options.version.second) {
+ *column_reader = std::make_shared<ConstantColumnReader>(
+
Field::create_field<TYPE_BIGINT>(read_options.version.second), col.type());
+ return Status::OK();
+ }
+ if (!_column_meta_accessor->has_column_uid(col_uid)) {
+ return Status::InternalError("could not find version column read
version is {}-{}",
+ read_options.version.first,
read_options.version.second);
+ }
+ return _column_reader_cache->get_column_reader(col_uid, column_reader,
read_options.stats,
+ &read_options.io_ctx);
+ }
+
+ if (col.name() == BINLOG_TSO_COL || col.name() == COMMIT_TSO_COL) {
+ const int64_t start_tso = read_options.commit_tso.start_tso();
+ const int64_t end_tso = read_options.commit_tso.end_tso();
+ // Version [0-0] identifies a pre-publish physical read, such as
segment compaction while a
+ // RowsetWriter is still open. Its TSO is intentionally [-1--1], so
preserve the on-disk
+ // COMMIT_TSO_COL=0 or BINLOG_TSO_COL=NULL placeholder instead of
synthesizing a value.
+ if (read_options.version == Version(0, 0) && start_tso == -1 &&
end_tso == -1) {
+ if (!_column_meta_accessor->has_column_uid(col_uid)) {
+ return Status::InternalError("could not find {} column",
col.name());
+ }
+ return _column_reader_cache->get_column_reader(
+ col_uid, column_reader, read_options.stats,
&read_options.io_ctx);
+ }
+ if (read_options.version.first == read_options.version.second) {
+ // A singleton needs its publish TSO to replace the on-disk NULL/0
placeholder.
+ // Range rowsets already contain materialized TSOs: compacting the
initial empty
Review Comment:
[P1] Preserve hidden metadata when ordered compaction links singleton
segments. This range branch assumes COMMIT_TSO pages were materialized, but
ordinary ordered compaction can link two tidy singleton rowsets without
rewriting their physical zero placeholders. A primary DUP_KEYS table with ROW
binlog then reads TSO=0, so `FOR TIME AS OF` can include rows committed after
the snapshot; hidden-column pruning and MIN/MAX also use those zeros. The same
link path leaves `__DORIS_VERSION_COL__=0` on eligible UNIQUE non-MoW tables.
Materialize these values, retain per-input rowset metadata for linked segments,
or disable linking for affected schemas; test snapshots before and between the
two commits.
--
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]
