github-actions[bot] commented on code in PR #66849:
URL: https://github.com/apache/doris/pull/66849#discussion_r3796864533
##########
be/src/storage/iterator/block_reader.cpp:
##########
@@ -255,6 +310,11 @@ Status BlockReader::_min_delta_next_block(Block* block,
bool* eof) {
auto first_op = _read_binlog_op(*_stored_data_columns[_binlog_op_pos],
0);
auto last_op = _read_binlog_op(*_stored_data_columns[_binlog_op_pos],
group_size - 1);
auto result =
binlog::AggregateFunctionMinDelta::calculate_result(first_op, last_op);
+ if (result ==
binlog::AggregateFunctionMinDelta::ResultType::UPDATE_BEFORE_AFTER &&
+ binlog::is_valid_row_binlog_op(first_op) &&
binlog::is_valid_row_binlog_op(last_op) &&
+ _min_delta_values_equal(group_size - 1)) {
Review Comment:
[P1] Do not treat an unavailable BEFORE image as a real all-NULL row.
build_before_block() fills every BEFORE value with NULL when no historical row
exists. A delete sign for a missing key is still recorded as DELETE, and a
later reinsertion after that tombstone becomes APPEND. If all value columns are
NULL, DELETE-to-APPEND first yields UPDATE_BEFORE_AFTER, then this gate sees
NULL equal to NULL and drops the net absent-to-present INSERT as SKIP.
Compatibility placeholder BEFORE rows have the same ambiguity. Only suppress
when the first BEFORE image is known valid, or conservatively retain ambiguous
all-NULL images.
##########
be/src/storage/iterator/block_reader.cpp:
##########
@@ -165,6 +207,19 @@ int BlockReader::_resolve_source_column_index(int idx,
bool use_before) const {
return _before_column_idx[idx];
}
+bool BlockReader::_min_delta_values_equal(size_t last_row) const {
+ if (!_min_delta_value_comparison_complete) {
+ return false;
+ }
+ for (const auto& [after_idx, before_idx] : _min_delta_value_column_pairs) {
+ if (_stored_data_columns[before_idx]->compare_at(
Review Comment:
[P1] Do not suppress an observable signed-zero update. FLOAT and DOUBLE pass
the new gate, but CompareFloat (used by ColumnVector::compare_at) returns 0 for
+0.0 versus -0.0. Doris preserves and displays both signs, and signbit(+0.0) /
signbit(-0.0) returns false / true, so a valid MOW update between them changes
row state. This comparison turns that update into SKIP and can leave an
incremental consumer's derived state stale; nested floats inherit the same
behavior. Please use state equality that preserves signed zero, or
conservatively exclude floating shapes, and add an end-to-end MIN_DELTA test.
--
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]
