sunchao commented on PR #5318: URL: https://github.com/apache/datafusion-comet/pull/5318#issuecomment-5852235619
Found **one P2 issue** at head `edd1c6ba8`. **[P2] Skip memory-consumer registration when cardinality checking is disabled** — [merge_rows.rs:248](codex://review?pr=https%3A%2F%2Fgithub.com%2Fapache%2Fdatafusion-comet%2Fpull%2F5318&path=native%2Fcore%2Fsrc%2Fexecution%2Foperators%2Fmerge_rows.rs&line=248&side=right). `execute()` always registers a memory consumer, including insert-only MERGEs with multiple clauses where `row_id_ordinal=None`. That consumer never reserves memory, but the default `fair_unified` pool counts it when dividing the task’s budget. Enabling native MergeRows can therefore make an otherwise successful upstream join fail. I reproduced this with the actual native operator, fair pool, and LeftAnti hash join: the join returned **4,096 rows with 96 KiB**, but wrapping it in MergeRows reduced its allowance to **48 KiB** and caused `ResourcesExhausted`. The probe used unlimited mock Spark grants, isolating the failure to native memory accounting. Register the consumer conditionally or lazily when cardinality state is needed. Validation: - **23 existing native MergeRows tests passed** with `--no-default-features`. - The additional [regression probe](/tmp/comet5318-memory-probe.patch) confirmed the issue. - Earlier correctness findings appear addressed; no additional defects confirmed. - JVM/Iceberg execution remained blocked by Maven dependency resolution. [Current CI](https://github.com/apache/datafusion-comet/actions/runs/36280016105) requires maintainer action and provides no build/test result. Checkout restored and clean. Nothing posted to GitHub. -- 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]
