brijrajk commented on PR #12158:
URL: https://github.com/apache/gluten/pull/12158#issuecomment-4911594567

   Thanks for the review @philo-he. You are right that the operator checks were 
dropped -- I've restored them in a follow-up edit.
   
   **`current_timestamp` / `now`**
   
   These two functions are folded to a wall-clock `TimestampType` literal by 
Spark's `ComputeCurrentTime` optimizer batch (which runs in an always-on batch 
that `excludedRules` cannot suppress). Because each execution captures a 
different instant, comparing the two runs (vanilla vs Gluten) always fails -- 
so `runQueryAndCompare` without `compareResult = false` would fail spuriously, 
not because of a Gluten bug. The fix is to use `compareResult = false` and add 
`checkGlutenPlan[ProjectExecTransformer]` to verify the Project still offloads 
natively. This is now done and the test passes.
   
   **`localtimestamp` -- validation enabled**
   
   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)`.
   
   **`localtimestamp` -- validation disabled**
   
   The original assertion `checkGlutenPlan[ProjectExecTransformer]` (native 
Project) is also incorrect. `localtimestamp()` constant-folds to a 
`TimestampNTZType` literal. Even with `enableTimestampNtzValidation = false`, 
`Validators.scala` (updated in #12064) only allows native Projects when the NTZ 
expression is `Hour(ntz_col)`. A `TimestampNTZType` constant literal does not 
qualify, so the Project still falls back. The correct assertion is 
`checkFallbackOperators(df, 0)` here as well.
   
   Both localtimestamp tests now run with `checkFallbackOperators(df, 0)` and 
`df.collect()`, confirmed passing locally.


-- 
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]

Reply via email to