philo-he commented on PR #12158: URL: https://github.com/apache/gluten/pull/12158#issuecomment-4921875099
Thanks for the update. For expressions like `current_timestamp()`, I assume Spark's optimizer rewrites the project into an alias expression, which is then offloaded to Velox. In that case, it should be fine to use `checkGlutenPlan[ProjectExecTransformer]` for the check. > The original assertion checkFallbackOperators(df, 1) was incorrect. checkFallbackOperators counts (ColumnarToRow + RowToColumnar) - 1 (the "-1" removes the top-level VeloxColumnarToRow that is always present). When the Project falls back, it falls back at the top of the plan, above the single VeloxColumnarToRow that bridges the native scan to JVM. The plan shape is: > > JVMProject > +- VeloxColumnarToRow <- only transition, counts as 1 > +- BatchScanExecTransformer > Count = 1 - 1 = 0, not 1. The correct assertion is checkFallbackOperators(df, 0) Perhaps `checkFallbackOperators` should be implemented by inspecting the executed plan to check whether a plan node is a GlutenPlan. That said, this is beyond the scope of this PR, and I'm not sure whether the current test code already depends on the existing implementation. I am approving this PR, and will merge it soon. Thanks. -- 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]
