malinjawi commented on PR #13158: URL: https://github.com/apache/gluten/pull/13158#issuecomment-6019123311
> @malinjawi thanks for the PR. My concern is the fallback can cause major perf regression for Merge command. > > If we change Velox would be possible to fully support increment metric? Thanks @felipepessoto , fair point. Yes, the fallback is a regression for MERGE. The write projection runs on Spark, and if CDF is on there is also a row-to-columnar hop on every written row that the parent rule doesn't catch. So this PR is the minimal correctness fix, not where we want to end up. The native version is in #13165, stacked on this one. No Velox change needed. Velox already counts how many rows each expression is evaluated on, and `FilterProject` exports those counts per function name when `operator_track_expression_stats` is on. So each Delta metric becomes a non-deterministic pass-through function named after the metric, the counts come back with the operator stats, and the write projection stays on Velox with exact counts, including inside CASE WHEN branches. Both PRs are green on the Delta UT gate. We also ran the two against each other: 10M-row Delta target, CDF on, 120 cores, 7 runs per case. Native was 12% faster on a mixed MERGE with a 1% source and 9% faster on a no-op MERGE with a 1% source. At a 10% source the join dominates and the difference is within noise. Rows, CDF output and all the persisted counters matched a plain Spark run every time. Details are in the #13165 description. Either order works for us: merge this one first as the safe step and rebase #13165 on top, or go straight to #13165. One caveat: with the projection back on Velox, a pre-existing struct field-order bug in MERGE schema evolution shows up again (this fallback was hiding it). I'll open a separate fix for that. -- 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]
