Copilot commented on code in PR #13214:
URL: https://github.com/apache/gluten/pull/13214#discussion_r4266933426
##########
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:
`withTempView("shift_input")` is redundant with the subsequent
`createOrReplaceTempView("shift_input")` and makes the lifecycle a bit harder
to follow. Consider dropping `withTempView(...)` and explicitly dropping the
view in a `try/finally`, or (preferably) build the DataFrame and register it
once inside the helper without wrapping another temp-view helper around it.
##########
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:
Both tests call `withUnsignedShiftInput`, which writes and reads the same
Parquet dataset twice. To reduce test runtime/IO, consider creating the
Parquet-backed temp view once (e.g., in a shared fixture / `beforeAll` with a
single temp path) and reusing it across both tests while still preventing
constant folding.
--
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]
