weiting-chen commented on code in PR #13214:
URL: https://github.com/apache/gluten/pull/13214#discussion_r4456193534
##########
backends-velox/src/test/scala/org/apache/gluten/functions/ScalarFunctionsValidateSuite.scala:
##########
@@ -197,6 +197,54 @@ class ScalarFunctionsValidateSuite extends
FunctionsValidateSuite {
}
}
+ test("shiftrightunsigned") {
+ withTempPath {
+ path =>
+ withTempView("shift_input") {
+ Seq[(Integer, java.lang.Long, Integer)](
+ (8, 8L, 1),
+ (-1, -1L, 1),
+ (Int.MinValue, Long.MinValue, 1),
+ (Int.MinValue, Long.MinValue, 31),
+ (-1, -1L, 32),
+ (-1, -1L, 33),
+ (-1, -1L, 63),
+ (-1, -1L, 64),
+ (-1, -1L, 65),
+ (-1, -1L, -1),
+ (Int.MinValue, Long.MinValue, Int.MaxValue),
+ (null, null, 1),
+ (1, 1L, null)
+ )
+ .toDF("int_value", "long_value", "shift")
+ .write
+ .parquet(path.getCanonicalPath)
+
+
spark.read.parquet(path.getCanonicalPath).createOrReplaceTempView("shift_input")
+ runQueryAndCompare(
+ """
+ |SELECT
+ | shiftrightunsigned(int_value, shift),
+ | shiftrightunsigned(long_value, shift)
+ |FROM shift_input
+ |""".stripMargin) {
+ checkGlutenPlan[ProjectExecTransformer]
+ }
Review Comment:
**Ensure both shift widths actually execute natively**
**Problem:** The suite disables its generic fallback check, and partial
projection is enabled by default. If native validation rejects only the INTEGER
signature while BIGINT remains supported, partial projection can evaluate the
INTEGER shift on the JVM using two of the three input columns and retain a
`ProjectExecTransformer` above it. Parity and this presence-only assertion then
still pass without native coverage for both widths. The same issue applies to
the operator query below. This is a coverage gap, not evidence that the current
implementation returns incorrect results.
**Evidence:**
```scala
| shiftrightunsigned(int_value, shift),
| shiftrightunsigned(long_value, shift)
|FROM shift_input
|""".stripMargin) {
checkGlutenPlan[ProjectExecTransformer]
```
**Suggested Fix:** Make each query's callback reject partial projection as
well as requiring a native project (add the `ColumnarPartialProjectExec`
import):
```scala
df =>
checkGlutenPlan[ProjectExecTransformer](df)
assert(!getExecutedPlan(df).exists {
case _: ColumnarPartialProjectExec => true
case _ => false
})
```
Alternatively, disable `spark.gluten.sql.columnar.partial.project` in a
`withSQLConf` scope around both parity queries so a lost signature cannot be
hidden by partial fallback.
<!-- pr-review-fingerprint:
prf-v1:e3285c09bf764adf49b325b27bf51554cea9f1881eb17cc1c31c18674b8f913f -->
--
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]
