github-actions[bot] commented on code in PR #64545:
URL: https://github.com/apache/doris/pull/64545#discussion_r3418174521
##########
be/src/storage/tablet/tablet_meta.cpp:
##########
@@ -523,9 +523,15 @@ void TabletMeta::init_schema_from_thrift(const
TTabletSchema& tablet_schema,
} else {
unique_id = col_ordinal_to_unique_id.at(col_ordinal);
}
- col_ordinal++;
init_column_from_tcolumn(unique_id, tcolumn, column);
+ if (column->name() == BINLOG_LSN_COL) {
+ tablet_schema_pb->set_binlog_lsn_col_idx(col_ordinal);
Review Comment:
Persisting these indexes only for new thrift-created tablet metas leaves
existing row-binlog schema PBs broken. `TabletSchema::init_from_pb()` first
discovers `BINLOG_LSN_COL` / `BINLOG_TIMESTAMP_COL` by name while appending
columns, but then unconditionally assigns `_binlog_lsn_col_idx =
schema.binlog_lsn_col_idx()` and `_binlog_timestamp_col_idx =
schema.binlog_timestamp_col_idx()`. For any row-binlog tablet meta, snapshot,
or cloud/local PB written before these fields were populated, those accessors
return the proto default `-1` and erase the discovered indexes. The next
row-binlog write can still hit `RowBinlogSegmentWriter::init()` with
`lsn_col_id < 0`, and timestamp predicates also see the timestamp column as
missing. Please make PB loading backward-compatible too, for example by only
overriding the name-derived indexes when the PB actually has these fields, or
by deriving them by name in `init_from_pb` when they are missing.
--
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]
