Skip to content
Closed
Prev Previous commit
Next Next commit
fix test failures
  • Loading branch information
gengliangwang committed Oct 14, 2019
commit 6f9cfa1d00295af0fc6d14ca7445709112f54d10
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ object Cast {
*/
def canANSIStoreAssign(from: DataType, to: DataType): Boolean = (from, to) match {
case _ if from == to => true
case (NullType, _) => true
case (_: NumericType, _: NumericType) => true
case (_: AtomicType, StringType) => true
case (_: CalendarIntervalType, StringType) => true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,8 @@ object DataType {

fieldCompatible

case (_: NullType, _) => true

case (w: AtomicType, r: AtomicType) if storeAssignmentPolicy == STRICT =>
if (!Cast.canUpCast(w, r)) {
addError(s"Cannot safely cast '$context': $w to $r")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,19 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession {

/** List of test cases to ignore, in lower cases. */
protected def blackList: Set[String] = Set(
"blacklist.sql" // Do NOT remove this one. It is here to test the blacklist functionality.
"blacklist.sql", // Do NOT remove this one. It is here to test the blacklist functionality.
// SPARK-28885 String value is not allowed to be stored as numeric type with
// ANSI store assignment policy.
"postgreSQL/numeric.sql",
"postgreSQL/int2.sql",
"postgreSQL/int4.sql",
"postgreSQL/int8.sql",
"postgreSQL/float4.sql",
"postgreSQL/float8.sql",
// SPARK-28885 String value is not allowed to be stored as date/timestamp type with
// ANSI store assignment policy.
"postgreSQL/date.sql",
Copy link
Member

@MaxGekk MaxGekk Nov 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gengliangwang Sorry, I just realized recently that my changed are not tested by date.sql and timestamp.sql any more when I run: build/sbt "sql/test-only *SQLQueryTestSuite -- -z date.sql". Did you disable them forever or are they executed in some way by jenkins?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about just setting storeAssignmentPolicy=LEGACY for the PgSQL tests? They have a lots of INSERT queries having implicit casts from string literals to numeric values.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maropu I am not sure about that.
For PgSQL, it disable inserting string values to numeric columns except for literals. Setting storeAssignmentPolicy=LEGACY for all the PgSQL tests seems inaccurate.

Copy link
Member

@maropu maropu Nov 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But, before this pr's been merged, we tested the PgSQL tests in the LEGACY mode? Is my understanding wrong? Personally, I think we need to explicitly file issues in jira (Or, comment out them?) if we have inaccurate tests in pgSQL/.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about changing the timestamp/date values from string literal to timestamp/date literal in those sql files, just as https://github.com/apache/spark/pull/26107/files#diff-431a4d1f056a06e853da8a60c46e9ca0R68
I am not sure about whether there is a guideline in porting the PgSQL test files. Such modifications are allowed, right?

Copy link
Member

@maropu maropu Nov 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure about whether there is a guideline in porting the PgSQL test files. Such modifications are allowed, right?

Yea, I think that's ok. cc: @wangyum @dongjoon-hyun @HyukjinKwon
If you have no time to do, I'll check next week.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm +1 for @gengliangwang 's suggestion enabling those tests with that modification.

"postgreSQL/timestamp.sql"
)

// Create all the test cases.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,9 @@ abstract class OrcSuite extends OrcTest with BeforeAndAfterAll {
}
}

test("SPARK-23340 Empty float/double array columns raise EOFException") {
// SPARK-28885 String value is not allowed to be stored as numeric type with
// ANSI store assignment policy.
ignore("SPARK-23340 Empty float/double array columns raise EOFException") {
Seq(Seq(Array.empty[Float]).toDF(), Seq(Array.empty[Double]).toDF()).foreach { df =>
withTempPath { path =>
df.write.format("orc").save(path.getCanonicalPath)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,17 @@ class ThriftServerQueryTestSuite extends SQLQueryTestSuite {
"date.sql",
// SPARK-28620
"postgreSQL/float4.sql",
// SPARK-28885 String value is not allowed to be stored as numeric type with
// ANSI store assignment policy.
"postgreSQL/numeric.sql",
"postgreSQL/int2.sql",
"postgreSQL/int4.sql",
"postgreSQL/int8.sql",
"postgreSQL/float8.sql",
// SPARK-28885 String value is not allowed to be stored as date/timestamp type with
// ANSI store assignment policy.
"postgreSQL/date.sql",
"postgreSQL/timestamp.sql"
// SPARK-28636
"decimalArithmeticOperations.sql",
"literals.sql",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import org.apache.spark.sql.catalyst.rules.RuleExecutor
import org.apache.spark.sql.hive.HiveUtils
import org.apache.spark.sql.hive.test.TestHive
import org.apache.spark.sql.internal.SQLConf
import org.apache.spark.sql.internal.SQLConf.StoreAssignmentPolicy

/**
* Runs the test cases that are included in the hive distribution.
Expand Down Expand Up @@ -59,6 +60,8 @@ class HiveCompatibilitySuite extends HiveQueryFileTest with BeforeAndAfter {
TestHive.setConf(SQLConf.IN_MEMORY_PARTITION_PRUNING, true)
// Ensures that cross joins are enabled so that we can test them
TestHive.setConf(SQLConf.CROSS_JOINS_ENABLED, true)
// Ensures that the table insertion behaivor is consistent with Hive
TestHive.setConf(SQLConf.STORE_ASSIGNMENT_POLICY, StoreAssignmentPolicy.LEGACY.toString)
// Fix session local timezone to America/Los_Angeles for those timezone sensitive tests
// (timestamp_*)
TestHive.setConf(SQLConf.SESSION_LOCAL_TIMEZONE, "America/Los_Angeles")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,9 @@ class HiveOrcQuerySuite extends OrcQueryTest with TestHiveSingleton {
}
}

test("SPARK-23340 Empty float/double array columns raise EOFException") {
// SPARK-28885 String value is not allowed to be stored as numeric type with
// ANSI store assignment policy.
ignore("SPARK-23340 Empty float/double array columns raise EOFException") {
withSQLConf(HiveUtils.CONVERT_METASTORE_ORC.key -> "false") {
withTable("spark_23340") {
sql("CREATE TABLE spark_23340(a array<float>, b array<double>) STORED AS ORC")
Expand Down