minni31 commented on code in PR #13214:
URL: https://github.com/apache/gluten/pull/13214#discussion_r4479618058
##########
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:
Thanks for catching this coverage gap. Fixed in d5a76a609: both the function
and operator callbacks still require ProjectExecTransformer and now explicitly
reject ColumnarPartialProjectExec in the executed plan. Partial projection
remains enabled; the assertions directly reject mixed execution. The Spark 4.0
operator guard is unchanged.
The Spark 4.1 group-1 CI report for this commit confirms that
shiftrightunsigned passed; ScalarFunctionsValidateSuite completed all 89 tests
with zero failures or errors.
##########
backends-velox/src/test/scala/org/apache/gluten/functions/ScalarFunctionsValidateSuite.scala:
##########
@@ -197,6 +197,63 @@ class ScalarFunctionsValidateSuite extends
FunctionsValidateSuite {
}
}
+ private def withUnsignedShiftInput(body: => Unit): Unit = {
+ 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")
+ body
Review Comment:
Keeping withTempView here: it provides exception-safe cleanup by dropping
the named view when the scope exits. createOrReplaceTempView only
registers/replaces the view; it does not supply that cleanup. The wrapper is
therefore intentional and avoids a handwritten try/finally.
##########
backends-velox/src/test/scala/org/apache/gluten/functions/ScalarFunctionsValidateSuite.scala:
##########
@@ -197,6 +197,63 @@ class ScalarFunctionsValidateSuite extends
FunctionsValidateSuite {
}
}
+ private def withUnsignedShiftInput(body: => Unit): Unit = {
+ 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")
+ body
+ }
+ }
+ }
+
+ test("shiftrightunsigned") {
+ withUnsignedShiftInput {
+ runQueryAndCompare(
+ """
+ |SELECT
+ | shiftrightunsigned(int_value, shift),
+ | shiftrightunsigned(long_value, shift)
+ |FROM shift_input
+ |""".stripMargin) {
+ checkGlutenPlan[ProjectExecTransformer]
+ }
+ }
+ }
+
+ testWithMinSparkVersion("unsigned shift operator", "4.0") {
+ withUnsignedShiftInput {
+ runQueryAndCompare(
+ """
+ |SELECT
+ | int_value >>> shift,
+ | long_value >>> shift
+ |FROM shift_input
+ |""".stripMargin) {
+ checkGlutenPlan[ProjectExecTransformer]
+ }
+ }
+ }
Review Comment:
Addressed in 807e07a36. The function and operator comparisons now share one
scoped test and one Parquet write/read. The function runs on every supported
profile, while only the operator query is guarded by Spark 4.0+. All 13 input
rows and the temp-path/view cleanup are retained.
--
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]
