heguanhui commented on code in PR #64484:
URL: https://github.com/apache/doris/pull/64484#discussion_r3410726942
##########
be/src/storage/segment/row_binlog_segment_writer.cpp:
##########
@@ -331,8 +331,13 @@ Status RowBinlogSegmentWriter::_fill_binlog_columns(size_t
num_rows,
// (SegmentIterator::_update_tso_col_if_needed), so its on-disk value
is never used.
// Write a NULL placeholder.
IColumn* ts_col_ptr = binlog_prefix_columns[2].get();
- auto* ts_nullable_column = assert_cast<ColumnNullable*>(ts_col_ptr);
- ts_nullable_column->insert_many_defaults(num_rows); // NULL
placeholder (value + null map)
+ auto* ts_nullable_column =
check_and_get_column<ColumnNullable>(ts_col_ptr);
Review Comment:
Thanks for the review.
You're right that timestamp column should always be nullable by design.
However, during my testing after fixing the binlog column index issue, I still
encountered a coredump at this assert_cast in
`GroupRowsetWriterTest.sub_writer_rollback`.
The root cause is that in the unit test environment, the timestamp column is
not wrapped as `ColumnNullable` when creating the Block via
`create_block_by_cids`. This is because the test uses a simplified schema
creation path that doesn't go through the full FE->BE schema conversion.
I agree this should be investigated separately. But to make the current PR
clean and focused on the binlog column index issue, I will split this into two
PRs:
- **PR1 (this PR)**: Fix binlog column index in `init_schema_from_thrift`
- **PR2 (follow-up)**: Investigate and fix the nullable type mismatch for
timestamp column in unit tests
I'll revert the change in `row_binlog_segment_writer.cpp` from this PR and
keep only the binlog column index fix. The timestamp column issue will be
addressed in a separate PR after further investigation.
Thank you for pointing this out!
--
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]
