minni31 commented on code in PR #13214:
URL: https://github.com/apache/gluten/pull/13214#discussion_r4481599324


##########
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:
   Follow-up in 6392fd1cc: both query callbacks now use the same local 
assertNativeUnsignedShift helper. It still requires ProjectExecTransformer and 
rejects every ColumnarPartialProjectExec exposed by getExecutedPlan; failures 
now include the offending plan nodes. All 13 input rows, both SQL queries, and 
the Spark 4.0 operator guard are unchanged.



##########
backends-velox/src/test/scala/org/apache/gluten/functions/ScalarFunctionsValidateSuite.scala:
##########
@@ -197,6 +197,62 @@ 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) {

Review Comment:
   Verified the comparison path: runQueryAndCompare uses checkAnswer, which 
delegates to GlutenQueryTest.sameRows. For an unordered query, sameRows sorts 
both result sequences before comparing them, preserving duplicate rows. Parquet 
read order therefore does not affect this parity check, so no additional ORDER 
BY is needed.



##########
backends-velox/src/test/scala/org/apache/gluten/functions/ScalarFunctionsValidateSuite.scala:
##########
@@ -197,6 +197,62 @@ 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) {
+            df =>
+              checkGlutenPlan[ProjectExecTransformer](df)
+              assert(
+                
!getExecutedPlan(df).exists(_.isInstanceOf[ColumnarPartialProjectExec]),
+                "Unsigned shifts must not use partial JVM projection")
+          }
+          if (isSparkVersionGE("4.0")) {
+            runQueryAndCompare(
+              """
+                |SELECT
+                |  int_value >>> shift,
+                |  long_value >>> shift
+                |FROM shift_input
+                |""".stripMargin) {

Review Comment:
   The operator query uses the same unordered-result comparison as the function 
query. GlutenQueryTest.sameRows sorts both result sequences before comparing 
them and preserves duplicate rows. Leaving ORDER BY out does not introduce an 
order-dependent comparison here.



##########
docs/backends/velox/velox-backend-scalar-function-support.md:
##########
@@ -1,6 +1,6 @@
 # Scalar Functions Support Status
 
-**Out of 433 scalar functions in Spark 4.1, Gluten currently fully supports 
261 functions and partially supports 31 functions.**
+**Out of 433 scalar functions in Spark 4.1, Gluten currently fully supports 
262 functions and partially supports 31 functions.**

Review Comment:
   The existing tools/scripts/gen-function-support-docs.py generator already 
emits both the headline totals and the support table in generate_function_doc. 
It computes num_supported and num_partially_supported from the function/support 
metadata before writing the document. This follow-up keeps that existing 
mechanism and its counts unchanged; no separate counting mechanism is 
introduced.



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