philo-he commented on code in PR #12976:
URL: https://github.com/apache/gluten/pull/12976#discussion_r4010726196
##########
gluten-ut/spark34/src/test/scala/org/apache/spark/sql/GlutenCharVarcharTestSuite.scala:
##########
@@ -301,4 +305,74 @@ class GlutenDSV2CharVarcharTestSuite extends
DSV2CharVarcharTestSuite with Glute
}
}
}
+
+ testGluten("length check for input string values: nested in map key") {
+ // Before: testTableWrite { typeName =>
Review Comment:
This comment line seems to be useless, can we remove it?
##########
backends-velox/src/test/scala/org/apache/gluten/execution/MiscOperatorSuite.scala:
##########
@@ -2342,7 +2342,7 @@ class MiscOperatorSuite extends
VeloxWholeStageTransformerSuite with AdaptiveSpa
test("Expression unsupported by backend can be handled by
ColumnarPartialProject") {
runQueryAndCompare(
- "SELECT c_custkey, map_from_arrays(array(c_name), array(c_comment)) FROM
customer") {
+ "SELECT c_custkey, c_name, sequence(c_custkey, c_custkey + 2) FROM
customer") {
Review Comment:
This original test is fragile since it depends on that one of the
expressions in use is not supported. But once it is supported, the test will
fail.
I think the following test already guards the functionality of
ColumnarPartialProject, so perhaps we can remove the above test.
https://github.com/pedrumj2/gluten/blob/84a2b7a5737744d9ffb75e5548d7a638d58b4175/backends-velox/src/test/scala/org/apache/gluten/execution/MiscOperatorSuite.scala#L2221
##########
gluten-ut/spark35/src/test/scala/org/apache/spark/sql/GlutenCharVarcharTestSuite.scala:
##########
@@ -300,4 +304,116 @@ class GlutenDSV2CharVarcharTestSuite extends
DSV2CharVarcharTestSuite with Glute
}
}
}
+
+ testGluten("length check for input string values: nested in map key") {
+ // Before: testTableWrite { typeName =>
+ testTableWrite {
+ typeName =>
+ sql(s"CREATE TABLE t(c MAP<$typeName(5), STRING>) USING $format")
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES
(map('123456', 'a'))"))
+ // Before: checkError(
+ // exception = e.getCause match {
+ // case c: SparkRuntimeException => c
+ // case c: SparkException =>
c.getCause.asInstanceOf[SparkRuntimeException]
+ // },
+ // errorClass = "EXCEED_LIMIT_LENGTH",
+ // parameters = Map("limit" -> "5")
+ // )
+ assert(e.getMessage.contains(VELOX_ERROR_MESSAGE))
+ }
+ }
+
+ testGluten("length check for input string values: nested in map value") {
+ // Before: testTableWrite { typeName =>
+ testTableWrite {
+ typeName =>
+ sql(s"CREATE TABLE t(c MAP<STRING, $typeName(5)>) USING $format")
+ sql("INSERT INTO t VALUES (map('a', null))")
+ checkAnswer(spark.table("t"), Row(Map("a" -> null)))
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES (map('a',
'123456'))"))
+ // Before: checkError(
+ // exception = e.getCause match {
+ // case c: SparkRuntimeException => c
+ // case c: SparkException =>
c.getCause.asInstanceOf[SparkRuntimeException]
+ // },
+ // errorClass = "EXCEED_LIMIT_LENGTH",
+ // parameters = Map("limit" -> "5")
+ // )
+ assert(e.getMessage.contains(VELOX_ERROR_MESSAGE))
+ }
+ }
+
+ testGluten("length check for input string values: nested in both map key and
value") {
+ // Before: testTableWrite { typeName =>
Review Comment:
no need to keep this comment. Just code format difference. Please remove
similar ones. Thanks.
##########
gluten-ut/spark34/src/test/scala/org/apache/spark/sql/GlutenCharVarcharTestSuite.scala:
##########
@@ -301,4 +305,74 @@ class GlutenDSV2CharVarcharTestSuite extends
DSV2CharVarcharTestSuite with Glute
}
}
}
+
+ testGluten("length check for input string values: nested in map key") {
+ // Before: testTableWrite { typeName =>
+ testTableWrite {
+ typeName =>
+ sql(s"CREATE TABLE t(c MAP<$typeName(5), STRING>) USING $format")
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES
(map('123456', 'a'))"))
+ // Before: assert(e.getMessage.contains(ERROR_MESSAGE))
Review Comment:
It would be better to use "Original: " instead of "Before: " for clarity.
Ditto for other places.
##########
gluten-ut/spark34/src/test/scala/org/apache/spark/sql/GlutenCharVarcharTestSuite.scala:
##########
@@ -301,4 +305,74 @@ class GlutenDSV2CharVarcharTestSuite extends
DSV2CharVarcharTestSuite with Glute
}
}
}
+
+ testGluten("length check for input string values: nested in map key") {
+ // Before: testTableWrite { typeName =>
+ testTableWrite {
+ typeName =>
+ sql(s"CREATE TABLE t(c MAP<$typeName(5), STRING>) USING $format")
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES
(map('123456', 'a'))"))
+ // Before: assert(e.getMessage.contains(ERROR_MESSAGE))
+ assert(e.getMessage.contains(VELOX_ERROR_MESSAGE))
+ }
+ }
+
+ testGluten("length check for input string values: nested in map value") {
+ // Before: testTableWrite { typeName =>
+ testTableWrite {
+ typeName =>
+ sql(s"CREATE TABLE t(c MAP<STRING, $typeName(5)>) USING $format")
+ sql("INSERT INTO t VALUES (map('a', null))")
+ checkAnswer(spark.table("t"), Row(Map("a" -> null)))
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES (map('a',
'123456'))"))
+ // Before: assert(e.getMessage.contains(ERROR_MESSAGE))
+ assert(e.getMessage.contains(VELOX_ERROR_MESSAGE))
+ }
+ }
+
+ testGluten("length check for input string values: nested in both map key and
value") {
+ // Before: testTableWrite { typeName =>
+ testTableWrite {
+ typeName =>
+ sql(s"CREATE TABLE t(c MAP<$typeName(5), $typeName(5)>) USING $format")
+ val e1 = intercept[SparkException](sql("INSERT INTO t VALUES
(map('123456', 'a'))"))
+ // Before: assert(e1.getMessage.contains(ERROR_MESSAGE))
+ assert(e1.getMessage.contains(VELOX_ERROR_MESSAGE))
+ val e2 = intercept[SparkException](sql("INSERT INTO t VALUES (map('a',
'123456'))"))
+ // Before: assert(e2.getMessage.contains(ERROR_MESSAGE))
+ assert(e2.getMessage.contains(VELOX_ERROR_MESSAGE))
+ }
+ }
+
+ testGluten("SPARK-42611: check char/varchar length in reordered structs
within map keys") {
+ // Before: Seq("CHAR(5)", "VARCHAR(5)").foreach { typ =>
+ Seq("CHAR(5)", "VARCHAR(5)").foreach {
+ typ =>
+ withTable("t") {
+ sql(s"CREATE TABLE t(m MAP<STRUCT<n_c: $typ, n_i: INT>, INT>) USING
$format")
+
+ val inputDF = sql("SELECT map(named_struct('n_i', 1, 'n_c',
'123456'), 1) AS m")
+
+ val e = intercept[SparkException](inputDF.writeTo("t").append())
+ // Before: assert(e.getCause.getMessage.contains(ERROR_MESSAGE))
+ assert(e.getMessage.contains(VELOX_ERROR_MESSAGE))
+ }
+ }
+ }
+
+ testGluten("SPARK-42611: check char/varchar length in reordered structs
within map values") {
+ // Before: Seq("CHAR(5)", "VARCHAR(5)").foreach { typ =>
Review Comment:
ditto
##########
gluten-ut/spark34/src/test/scala/org/apache/spark/sql/GlutenCharVarcharTestSuite.scala:
##########
@@ -301,4 +305,74 @@ class GlutenDSV2CharVarcharTestSuite extends
DSV2CharVarcharTestSuite with Glute
}
}
}
+
+ testGluten("length check for input string values: nested in map key") {
+ // Before: testTableWrite { typeName =>
+ testTableWrite {
+ typeName =>
+ sql(s"CREATE TABLE t(c MAP<$typeName(5), STRING>) USING $format")
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES
(map('123456', 'a'))"))
+ // Before: assert(e.getMessage.contains(ERROR_MESSAGE))
+ assert(e.getMessage.contains(VELOX_ERROR_MESSAGE))
+ }
+ }
+
+ testGluten("length check for input string values: nested in map value") {
+ // Before: testTableWrite { typeName =>
+ testTableWrite {
+ typeName =>
+ sql(s"CREATE TABLE t(c MAP<STRING, $typeName(5)>) USING $format")
+ sql("INSERT INTO t VALUES (map('a', null))")
+ checkAnswer(spark.table("t"), Row(Map("a" -> null)))
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES (map('a',
'123456'))"))
+ // Before: assert(e.getMessage.contains(ERROR_MESSAGE))
+ assert(e.getMessage.contains(VELOX_ERROR_MESSAGE))
+ }
+ }
+
+ testGluten("length check for input string values: nested in both map key and
value") {
+ // Before: testTableWrite { typeName =>
+ testTableWrite {
+ typeName =>
+ sql(s"CREATE TABLE t(c MAP<$typeName(5), $typeName(5)>) USING $format")
+ val e1 = intercept[SparkException](sql("INSERT INTO t VALUES
(map('123456', 'a'))"))
+ // Before: assert(e1.getMessage.contains(ERROR_MESSAGE))
+ assert(e1.getMessage.contains(VELOX_ERROR_MESSAGE))
+ val e2 = intercept[SparkException](sql("INSERT INTO t VALUES (map('a',
'123456'))"))
+ // Before: assert(e2.getMessage.contains(ERROR_MESSAGE))
+ assert(e2.getMessage.contains(VELOX_ERROR_MESSAGE))
+ }
+ }
+
+ testGluten("SPARK-42611: check char/varchar length in reordered structs
within map keys") {
+ // Before: Seq("CHAR(5)", "VARCHAR(5)").foreach { typ =>
Review Comment:
ditto
##########
gluten-ut/spark34/src/test/scala/org/apache/spark/sql/GlutenCharVarcharTestSuite.scala:
##########
@@ -301,4 +305,74 @@ class GlutenDSV2CharVarcharTestSuite extends
DSV2CharVarcharTestSuite with Glute
}
}
}
+
+ testGluten("length check for input string values: nested in map key") {
+ // Before: testTableWrite { typeName =>
+ testTableWrite {
+ typeName =>
+ sql(s"CREATE TABLE t(c MAP<$typeName(5), STRING>) USING $format")
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES
(map('123456', 'a'))"))
+ // Before: assert(e.getMessage.contains(ERROR_MESSAGE))
+ assert(e.getMessage.contains(VELOX_ERROR_MESSAGE))
+ }
+ }
+
+ testGluten("length check for input string values: nested in map value") {
+ // Before: testTableWrite { typeName =>
+ testTableWrite {
+ typeName =>
+ sql(s"CREATE TABLE t(c MAP<STRING, $typeName(5)>) USING $format")
+ sql("INSERT INTO t VALUES (map('a', null))")
+ checkAnswer(spark.table("t"), Row(Map("a" -> null)))
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES (map('a',
'123456'))"))
+ // Before: assert(e.getMessage.contains(ERROR_MESSAGE))
+ assert(e.getMessage.contains(VELOX_ERROR_MESSAGE))
+ }
+ }
+
+ testGluten("length check for input string values: nested in both map key and
value") {
+ // Before: testTableWrite { typeName =>
Review Comment:
ditto
##########
gluten-ut/spark34/src/test/scala/org/apache/spark/sql/GlutenCharVarcharTestSuite.scala:
##########
@@ -301,4 +305,74 @@ class GlutenDSV2CharVarcharTestSuite extends
DSV2CharVarcharTestSuite with Glute
}
}
}
+
+ testGluten("length check for input string values: nested in map key") {
+ // Before: testTableWrite { typeName =>
+ testTableWrite {
+ typeName =>
+ sql(s"CREATE TABLE t(c MAP<$typeName(5), STRING>) USING $format")
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES
(map('123456', 'a'))"))
+ // Before: assert(e.getMessage.contains(ERROR_MESSAGE))
+ assert(e.getMessage.contains(VELOX_ERROR_MESSAGE))
+ }
+ }
+
+ testGluten("length check for input string values: nested in map value") {
+ // Before: testTableWrite { typeName =>
Review Comment:
ditto
##########
gluten-ut/spark35/src/test/scala/org/apache/spark/sql/GlutenCharVarcharTestSuite.scala:
##########
@@ -300,4 +304,116 @@ class GlutenDSV2CharVarcharTestSuite extends
DSV2CharVarcharTestSuite with Glute
}
}
}
+
+ testGluten("length check for input string values: nested in map key") {
+ // Before: testTableWrite { typeName =>
+ testTableWrite {
+ typeName =>
+ sql(s"CREATE TABLE t(c MAP<$typeName(5), STRING>) USING $format")
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES
(map('123456', 'a'))"))
+ // Before: checkError(
+ // exception = e.getCause match {
+ // case c: SparkRuntimeException => c
+ // case c: SparkException =>
c.getCause.asInstanceOf[SparkRuntimeException]
+ // },
+ // errorClass = "EXCEED_LIMIT_LENGTH",
+ // parameters = Map("limit" -> "5")
+ // )
+ assert(e.getMessage.contains(VELOX_ERROR_MESSAGE))
+ }
+ }
+
+ testGluten("length check for input string values: nested in map value") {
+ // Before: testTableWrite { typeName =>
+ testTableWrite {
+ typeName =>
+ sql(s"CREATE TABLE t(c MAP<STRING, $typeName(5)>) USING $format")
+ sql("INSERT INTO t VALUES (map('a', null))")
+ checkAnswer(spark.table("t"), Row(Map("a" -> null)))
+ val e = intercept[SparkException](sql("INSERT INTO t VALUES (map('a',
'123456'))"))
+ // Before: checkError(
+ // exception = e.getCause match {
+ // case c: SparkRuntimeException => c
+ // case c: SparkException =>
c.getCause.asInstanceOf[SparkRuntimeException]
+ // },
+ // errorClass = "EXCEED_LIMIT_LENGTH",
+ // parameters = Map("limit" -> "5")
+ // )
+ assert(e.getMessage.contains(VELOX_ERROR_MESSAGE))
+ }
+ }
+
+ testGluten("length check for input string values: nested in both map key and
value") {
+ // Before: testTableWrite { typeName =>
+ testTableWrite {
+ typeName =>
+ sql(s"CREATE TABLE t(c MAP<$typeName(5), $typeName(5)>) USING $format")
+ val e1 = intercept[SparkException](sql("INSERT INTO t VALUES
(map('123456', 'a'))"))
+ // Before: checkError(
+ // exception = e1.getCause match {
+ // case c: SparkRuntimeException => c
+ // case c: SparkException =>
c.getCause.asInstanceOf[SparkRuntimeException]
+ // },
+ // errorClass = "EXCEED_LIMIT_LENGTH",
+ // parameters = Map("limit" -> "5")
+ // )
Review Comment:
Perhaps, no need to keep the original test code as comment for reference. We
can just leave a brief comment like: Gluten exception differs from Spark.
Developers can check the original Spark test to get the detailed differences.
Ditto for other applicable places.
--
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]
