github-actions[bot] commented on code in PR #63182:
URL: https://github.com/apache/doris/pull/63182#discussion_r3228450615
##########
be/src/storage/partial_update_info.cpp:
##########
@@ -770,7 +803,7 @@ Status
FlexibleReadPlan::fill_non_primary_key_columns_for_row_store(
auto block_pos = block_start_pos + idx;
auto pos_in_old_block = read_index[segment_pos];
Review Comment:
This row-store path can access old-row data even when no old row was read.
For a row-store table, a flexible partial update for a new key (or a path where
`_probe_key_for_mow()` sets `use_default_or_null_flag` and does not call
`prepare_to_read()`) leaves `read_index` without `segment_pos`, but
`read_index[segment_pos]` inserts `0`. Then
`fill_non_primary_key_cell_for_row_store()` evaluates
`delete_sign_column_data[pos_in_old_block]` before `use_default` prevents
old-row merging. If `old_value_block` is empty this is an out-of-bounds read;
if other rows were read it can use the wrong row's delete sign. The
column-store path avoids this by checking `read_index.contains(...)` before
merging old VARIANT data. Please make the row-store path preserve whether an
old row exists and skip all old-row/delete-sign access when it does not.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AlterTableCommand.java:
##########
@@ -230,6 +232,33 @@ private void rewriteAlterOpForOlapTable(ConnectContext
ctx, OlapTable table) thr
ops = alterTableOps;
}
+ private void validateAlterVariantColumnsForFlexiblePartialUpdate(OlapTable
table) throws UserException {
+ boolean enableFlexiblePartialUpdate = table.hasSkipBitmapColumn();
Review Comment:
This validation still runs before `SchemaChangeHandler.process()` takes the
table write lock, so it can be invalidated by a concurrent ALTER. A concrete
race is: ALTER A validates `ADD COLUMN v VARIANT<doc mode>` while the table has
no skip-bitmap column; ALTER B then enables `UPDATE_FLEXIBLE_COLUMNS` and
commits under the table lock; ALTER A later enters
`SchemaChangeHandler.process()` under the lock and adds the doc-mode VARIANT to
a now-flexible table because the handler does not re-run this validation
against the locked/current schema. Please revalidate the final schema under the
schema-change lock before committing light schema change metadata, or move this
table-shape check into the locked schema-change processing path.
--
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]
