github-actions[bot] commented on code in PR #69660:
URL: https://github.com/apache/doris/pull/69660#discussion_r4384969143
##########
be/src/exec/operator/group_commit_scan_operator.cpp:
##########
@@ -37,6 +37,15 @@ Status GroupCommitOperatorX::get_block_impl(RuntimeState*
state, Block* block, b
return Status::OK();
}
+Status GroupCommitOperatorX::get_block_after_projects(RuntimeState* state,
Block* block,
+ bool* eos) {
+ // ScanOperatorX skips operator projections because ordinary scans apply
them in Scanner.
+ // Group commit reads queued blocks directly, so it must apply the
consumer's projections here.
+ // For example, a producer's [base columns, mv_int, mv_string] must not be
interpreted as
+ // the consumer's [base columns, mv_string, mv_int] without projecting
from the base columns.
+ return OperatorXBase::get_block_after_projects(state, block, eos);
Review Comment:
[P2] Retain producer values for volatile generated columns
Generated columns admit scalar functions such as `now(6)`
(`CreateTableInfo.checkFunctionInGeneratedColumn()` does not reject them). With
`t(k INT, producer_time DATETIMEV2(6) DEFAULT CURRENT_TIMESTAMP(6), g
DATETIMEV2(6) AS(now(6)))`, a group-commit insert of `k` queues `[k,P,P]`,
since both timestamps use the producer statement's query globals. The internal
consumer forwards `producer_time`, but `BindSink` reconstructs `g=now(6)`,
which this new call evaluates using the separate consumer query's time `C`. The
stored row becomes `[k,P,C]`, breaking the previously matching timestamps and
reusing the consumer's time for later producers in the same batch. Here `g` is
last, so this remains even if the input-position issue is fixed. Internal group
commit should forward already computed generated base-column values rather than
evaluate their expressions again.
##########
be/src/exec/operator/group_commit_scan_operator.cpp:
##########
@@ -37,6 +37,15 @@ Status GroupCommitOperatorX::get_block_impl(RuntimeState*
state, Block* block, b
return Status::OK();
}
+Status GroupCommitOperatorX::get_block_after_projects(RuntimeState* state,
Block* block,
+ bool* eos) {
+ // ScanOperatorX skips operator projections because ordinary scans apply
them in Scanner.
+ // Group commit reads queued blocks directly, so it must apply the
consumer's projections here.
+ // For example, a producer's [base columns, mv_int, mv_string] must not be
interpreted as
+ // the consumer's [base columns, mv_string, mv_int] without projecting
from the base columns.
+ return OperatorXBase::get_block_after_projects(state, block, eos);
Review Comment:
[P1] Align queued columns with the pruned consumer tuple
For a supported table `t(k INT, g INT AS(k+1), v INT)`, a group-commit
`INSERT INTO t(k,v) VALUES(1,99)` queues `[k,g,v]=[1,2,99]`. The reduced
consumer plan is:
```text
OlapTableSink(k,g,v)
Project(c1, c1+1 AS g, c3)
GroupCommitScan(c1,c3)
```
`BindSink.getColumnToOutput()` recomputes `g`, so
`PhysicalPlanTranslator.updateScanSlotsMaterialization()` removes its unused
incoming slot `c2`. `VSlotRef::prepare()` consequently resolves `c3` to
position 1, but `LoadBlockQueue::get_block()` still returns the full producer
block with `g` at position 1. This newly enabled projection therefore writes
`v=2` instead of `99`; equal types make this silent corruption. Previously
forwarding the queued block preserved the base values. Preserve the full queued
input slot layout, or remap the queued columns to the consumer tuple before
evaluating projections, and cover a generated column followed by an ordinary
column in the regression 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]
