github-actions[bot] commented on code in PR #64776:
URL: https://github.com/apache/doris/pull/64776#discussion_r3510310547
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/InsertIntoTableCommand.java:
##########
@@ -297,45 +287,12 @@ public AbstractInsertExecutor initPlan(ConnectContext
ctx, StmtExecutor stmtExec
return insertExecutor;
}
- List<ScanNode> tableStreamScanNodes =
- buildResult.planner.getScanNodes().stream()
- .filter((s -> (s.getTableIf() instanceof
OlapTableStreamWrapper
- || s instanceof OlapScanNode &&
((OlapScanNode) s).isIncrementalScan())))
- .collect(Collectors.toList());
-
- if (!tableStreamScanNodes.isEmpty()) {
+ List<TableStreamUpdateInfo> infos =
StreamConsumptionInfoExtractor.extract(analyzedPlan);
Review Comment:
Please don't extract table-stream offsets from the analyzed plan here. The
analyzed plan can still contain CTE producers that the rewrite phase removes
before execution. For example, `WITH s AS (SELECT * FROM stream_tbl@incr)
INSERT INTO target SELECT 1` is analyzed as a `LogicalCTEAnchor` whose producer
contains the stream scan, but `CTEInline` returns only the right child when
there is no consumer. This code still records the dead producer's offsets and
writes them to `TransactionState`, advancing a stream that the insert never
read. Extract from the rewritten/optimized plan that is actually executed, or
make the extractor ignore unused CTE producers.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/CreateTableInfo.java:
##########
@@ -715,6 +716,17 @@ public void validate(ConnectContext ctx) {
}
}
+ // __DORIS_COMMIT_TSO_COL__ injection for time-travel:
+ // only on dup / mow tables with row binlog enabled
(binlog.enable=true && binlog.format=ROW).
+ if (keysType.equals(KeysType.DUP_KEYS)
+ || (keysType.equals(KeysType.UNIQUE_KEYS) &&
isEnableMergeOnWrite)) {
+ BinlogConfig binlogConfig = new BinlogConfig();
Review Comment:
Please include inherited database binlog config in this decision.
`createTableInfo.validate()` runs before `InternalCatalog.createTable()` merges
`db.getBinlogConfig()` into `createTableInfo.getProperties()`, so a table
created inside a database whose default binlog config is `ROW` reaches this
block with no table-level binlog properties and does not get
`__DORIS_COMMIT_TSO_COL__`. Later the merged config is installed, row-binlog
metadata is created, and `enableTso()` returns true; time travel then fails the
`addCommitTsoFilter` precondition because the scan output has no commit-TSO
slot. Use the merged binlog config before schema generation, or inject the
hidden column after inheritance is applied.
##########
regression-test/data/table_stream_p0/test_olap_table_stream_mixed_history_incremental.out:
##########
@@ -0,0 +1,9 @@
+-- This file is automatically generated. You should know what you did if you
want to edit this
+-- !mixed_history_incremental_rows --
+1 history_p1 APPEND
+11 incremental_p2 APPEND
+
+-- !mixed_history_incremental_count --
+1 1
+11 1
+
Review Comment:
`git diff --check` currently fails because this generated output ends with a
new blank line at EOF. The same issue appears in
`test_table_stream_query_comprehensive.out`, `test_time_travel_dup.out`,
`test_time_travel_mow.out`, and `test_time_travel_mow_complex.out`. Please trim
or regenerate these `.out` files so the style check passes.
--
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]
