Jay-ju commented on PR #66999: URL: https://github.com/apache/doris/pull/66999#issuecomment-5368075844
Thanks for the thorough review. Both findings were real; fixed in the latest revision (single commit, force-pushed). **[P1] Snapshot correctness — fixed.** The count carrier no longer serializes version 0/latest. It now pins the planned MVCC version via a new `LanceSplit.wholeDatasetCountAtVersion(uri, version, carriedRows)` factory, so a fallback scan (an old BE, or a current BE that declines the shortcut and opens the whole-dataset range in `_ensure_dataset_open`) reads exactly the snapshot the count was computed from. `wholeDatasetAtLatest` is retained only for the backend-local TVF, which is correctly latest. Added an EXPLAIN assertion (`lanceVersion=4` on the multi_frag fixture, built with three appends + three deletes) that fails if this ever regresses to latest. **[P2] Large-count parallelism — fixed.** Since BE materializes one synthetic row per counted row, `buildCountSplits()` now shards a count `>= COUNT_WITH_PARALLEL_SPLITS` (10000, matching Iceberg) across `parallelExecInstanceNum * numBackends` fixed-version carriers, with per-carrier shares summing back to the exact total (remainder folded into the last carrier). This mirrors `IcebergScanNode.assignCountToSplits` and reuses the existing multi-split `table_level_row_count` contract, so BE needs no change. A small count stays on one carrier for backward compatibility. Verified end to end on a real FE+BE+MinIO cluster: `count(*)` returns the logical 27 (not physical 30) for multi_frag and 12 for all_types, EXPLAIN shows `pushdown agg=COUNT (27)` with `lanceVersion=4`, and `run-regression-test test_lance_optimize_count` passes (All suites success, 0 failed). One note on the large-count test: I kept the committed fixtures lightweight rather than adding a >=10000-row binary dataset just to cross the parallel-split threshold. The sharding is pure FE arithmetic over the existing multi-split BE contract that Iceberg/Paimon already exercise at scale, and the small-table path (single carrier) is covered by the suite. Happy to add a large fixture if you'd prefer explicit end-to-end coverage of the parallel 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]
