Gabriel39 commented on PR #66498: URL: https://github.com/apache/doris/pull/66498#issuecomment-5288194375
I reviewed the current head `8ff74c6abee`. The latest commit addresses the key-dynamic index TTL, `ignore-delete` partition-move, same-basename qualifier, and duplicate DELETE USING findings, but the following merge blockers remain. ### [P1] MERGE branch expressions are still not short-circuited `PaimonMergePlanner.generateFinalExpression()` and `generateBranchLabel()` build ordinary `If` trees. Doris serializes those functions using the session-level `short_circuit_evaluation` setting, which defaults to `false`. In a mixed batch, expressions from an inactive assignment branch or a later WHEN predicate can therefore still be evaluated and abort an otherwise valid MERGE, for example when an inactive branch contains `assert_true`, a failing cast, or another row-sensitive expression. Please ensure planner-generated MERGE guards always preserve SQL branch short-circuit semantics independently of the session variable, and add a mixed-row regression under the default setting. ### [P1] Scan-derived UPDATE rows still have no concurrency fence UPDATE and MERGE UPDATE copy every unassigned target column from the scan snapshot and later commit a complete `UPDATE_AFTER` row. The write binding and commit path do not carry the starting snapshot or a per-key version. A concurrent writer can therefore change another column after the scan and have that value overwritten by this stale full row. If the concurrent writer advances `sequence.field`, Paimon may instead discard the Doris update while Doris still reports success. Please reject or retry when the target changed since the scan, or otherwise serialize/fence row-change DML, and add concurrent UPDATE and MERGE UPDATE coverage. ### [P2] Unconditional full-row DISTINCT breaks valid DELETEs and adds a large memory cost `PaimonRowChangePlanBuilder.buildDelete()` now returns `new LogicalProject<>(projects, true, child)` for every DELETE. The analyzer lowers this to an aggregate grouped by the operation plus every target column, even for a simple DELETE with no USING join. This has two consequences: - With Paimon binary/varbinary mapping enabled, a table containing a VARBINARY column cannot be deleted from because Doris rejects VARBINARY GROUP BY expressions during analysis. - Large deletes must hash/shuffle the complete high-cardinality row image, adding substantial memory and spill pressure merely to remove duplicates that only DELETE USING can introduce. Please apply deduplication only to shapes that can multiply target rows and deduplicate by target primary key rather than by the full row. Add a VARBINARY-table DELETE regression and a large duplicate-USING case. The PR is also currently conflicting with `branch-4.1`, and the latest P0, External, NonConcurrent, cloud_p0, FE coverage, and ShellCheck results are failing. I do not think the current head is ready to merge. -- 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]
