[SPARK-29873][SQL][TESTS] Support `--import` directive to load queries from another test case in SQLQueryTestSuite #26497

maropu · 2019-11-13T08:11:16Z

What changes were proposed in this pull request?

This pr is to support --import directive to load queries from another test case in SQLQueryTestSuite.

This fix comes from the @cloud-fan suggestion in #26479 (comment)

Why are the changes needed?

This functionality might reduce duplicate test code in SQLQueryTestSuite.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Run SQLQueryTestSuite.

SparkQA · 2019-11-13T08:26:11Z

Test build #113681 has finished for PR 26497 at commit bfa07a5.

This patch fails to build.
This patch merges cleanly.
This patch adds the following public classes (experimental):
protected case class AnsiTestCase(

cloud-fan · 2019-11-13T09:16:01Z

sql/core/src/test/resources/sql-tests/inputs/literals.sql

-select interval;
-select interval 1 fake_unit;
-select interval 1 year to month;
-select 1 year to month;


we should move some tests to ansi/interval.sql. They are not duplicated tests, they test without the leading "interval".

Ah, I see. I'll recheck later. Thanks!

cloud-fan · 2019-11-13T11:26:24Z

sql/core/src/test/resources/sql-tests/inputs/ansi/decimalArithmeticOperations.sql

@@ -0,0 +1,2 @@
+-- throw an exception instead of returning NULL, according to SQL ANSI 2011


can we add a little more description? when will we throw an exception?

cloud-fan · 2019-11-13T11:27:19Z

sql/core/src/test/resources/sql-tests/inputs/ansi/literals.sql

@@ -0,0 +1,2 @@
+--- malformed interval literal with ansi mode


hmm, do we still have interval related tests in literals.sql?

Is it ok to move all the interval-related queries here into interval.sql or ansi/interval.sql?

I think so. @yaooqinn do you have time to take it later? IIRC there are a few interval tests in group-by.sql as well.

ok. As for literals.sql, I'll move all the interval-related queries into correct files in this pr.

some cases are using interval but to verify literals such as

-- awareness of the negative sign before type select -integer '7'; select -date '1999-01-01'; select -timestamp '1999-01-01'; select -x'2379ACFe'; select +integer '7'; select +interval '1 second';

the group-by.sql contains avg and sum support, which should be move to interval.sql

SparkQA · 2019-11-13T11:29:21Z

Test build #113698 has finished for PR 26497 at commit 56ceb9f.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
protected case class AnsiTestCase(

cloud-fan · 2019-11-13T11:30:38Z

sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala

        // vol used by boolean.sql and case.sql.
        localSparkSession.udf.register("vol", (s: String) => s)
        // PostgreSQL enabled cartesian product by default.
        localSparkSession.conf.set(SQLConf.CROSS_JOINS_ENABLED.key, true)


not related to this PR, but can we remove it here? CROSS JOIN is already enabled by default now.

Oh, yes! I'll remove it.

cloud-fan · 2019-11-13T11:31:33Z

sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala

        localSparkSession.udf.register("vol", (s: String) => s)
        // PostgreSQL enabled cartesian product by default.
        localSparkSession.conf.set(SQLConf.CROSS_JOINS_ENABLED.key, true)
        localSparkSession.conf.set(SQLConf.ANSI_ENABLED.key, true)


We should remove it in the followup, and see if it fails tests. Ideally pgsql dialect should not be affected by ansi mode config. cc @gengliangwang

cloud-fan · 2019-11-13T11:32:51Z

thanks for adding it!

cloud-fan · 2019-11-13T13:26:36Z

sql/core/src/test/resources/sql-tests/inputs/interval.sql

+select interval '12:11:10' hour to second '1' year;
+
+-- awareness of the negative sign before type
+select +integer '7';


we shouldn't test integer in interval.sql, but we should test -interval '1 second' to match the comment

Ur, I moved this by mistake. I'll revert.

cloud-fan · 2019-11-13T13:27:24Z

sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala

        localSparkSession.udf.register("boolne", (b1: Boolean, b2: Boolean) => b1 != b2)
        // vol used by boolean.sql and case.sql.
        localSparkSession.udf.register("vol", (s: String) => s)
        // PostgreSQL enabled cartesian product by default.


nit: this comment can be removed.

SparkQA · 2019-11-13T13:48:32Z

Test build #113707 has finished for PR 26497 at commit 615347e.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2019-11-13T14:55:08Z

Test build #113705 has finished for PR 26497 at commit 0d9e462.

This patch fails Spark unit tests.
This patch does not merge cleanly.
This patch adds no public classes.

SparkQA · 2019-11-13T16:40:09Z

Test build #113709 has finished for PR 26497 at commit 93051fc.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

dongjoon-hyun · 2019-11-13T17:55:10Z

sql/core/src/test/resources/sql-tests/inputs/ansi/decimalArithmeticOperations.sql

+-- SPARK-23179: SQL ANSI 2011 states that in case of overflow during arithmetic operations,
+-- an exception should be thrown instead of returning NULL.
+-- This is what most of the SQL DBs do (eg. SQLServer, DB2).
+--import decimalArithmeticOperations.sql


This seems to fail still.

[info] - ansi/decimalArithmeticOperations.sql *** FAILED *** (2 seconds, 158 milliseconds)

Yea, thanks for your catch-up! I've fixed now.

dongjoon-hyun · 2019-11-13T17:55:50Z

sql/core/src/test/resources/sql-tests/inputs/ansi/literals.sql

@@ -0,0 +1,2 @@
+--- malformed interval literal with ansi mode
+--import literals.sql


This also still fails, too.

[info] - literals.sql *** FAILED *** (1 second, 989 milliseconds)

maropu · 2019-11-14T01:28:03Z

sql/core/src/test/resources/sql-tests/inputs/ansi/decimalArithmeticOperations.sql

+-- SPARK-23179: SQL ANSI 2011 states that in case of overflow during arithmetic operations,
+-- an exception should be thrown instead of returning NULL.
+-- This is what most of the SQL DBs do (eg. SQLServer, DB2).
+


Since inputs/decimalArithmeticOperations.sql has some nondeterministic output with ansi=true, I inlined the ANSI-related queries in it.

this is surprising. what causes the nondeterminice?

For example, the queries below in inputs/decimalArithmeticOperations.sql throws multiple exceptions with a different error message in executors with ansi=true;

sql("SET spark.sql.decimalOperations.allowPrecisionLoss=false") sql("SET spark.sql.ansi.enabled=true") sql("create table decimals_test(id int, a decimal(38,18), b decimal(38,18)) using parquet") sql("insert into decimals_test values(1, 100.0, 999.0), (2, 12345.123, 12345.123), (3, 0.1234567891011, 1234.1), (4, 123456789123456789.0, 1.123456789123456789)") sql("select id, a+b, a-b, a*b, a/b from decimals_test order by id").show() java.lang.ArithmeticException: Decimal(expanded,138698367904130467.51562262075019052100,38,20}) cannot be represented as Decimal(38, 36). at org.apache.spark.sql.types.Decimal.toPrecision(Decimal.scala:357) at org.apache.spark.sql.catalyst.expressions.GeneratedClass$GeneratedIteratorForCodegenStage1.processNext(Unknown Source) ... java.lang.ArithmeticException: Decimal(expanded,99900.000000000000000000000000000000000,38,33}) cannot be represented as Decimal(38, 36). at org.apache.spark.sql.types.Decimal.toPrecision(Decimal.scala:357) at org.apache.spark.sql.catalyst.expressions.GeneratedClass$GeneratedIteratorForCodegenStage1.processNext(Unknown Source) at org.apache.spark.sql.execution.BufferedRowIterator.hasNext(BufferedRowIterator.java:43) ... java.lang.ArithmeticException: Decimal(expanded,152.35802342966751000000000000000000000,38,35}) cannot be represented as Decimal(38, 36). at org.apache.spark.sql.types.Decimal.toPrecision(Decimal.scala:357) at org.apache.spark.sql.catalyst.expressions.GeneratedClass$GeneratedIteratorForCodegenStage1.processNext(Unknown Source) at org.apache.spark.sql.execution.BufferedRowIterator.hasNext(BufferedRowIterator.java:43) ... <more exceptions below>

So, the output string printed in decimalArithmeticOperations.sql.out depends on timing.

ah this is nasty, but no better ideas.

Yea, I have no idea, too.

SparkQA · 2019-11-14T05:12:44Z

Test build #113741 has finished for PR 26497 at commit ab0730b.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

cloud-fan · 2019-11-14T06:38:44Z

thanks, merging to master!

maropu · 2019-11-14T07:11:41Z

Thanks for merging!

…f ansi mode config ### What changes were proposed in this pull request? Fix the inconsistent behavior of build-in function SQL LEFT/RIGHT. ### Why are the changes needed? As the comment in #26497 (comment), Postgre dialect should not be affected by the ANSI mode config. During reran the existing tests, only the LEFT/RIGHT build-in SQL function broke the assumption. We fix this by following https://www.postgresql.org/docs/12/sql-keywords-appendix.html: `LEFT/RIGHT reserved (can be function or type)` ### Does this PR introduce any user-facing change? Yes, the Postgre dialect will not be affected by the ANSI mode config. ### How was this patch tested? Existing UT. Closes #26584 from xuanyuanking/SPARK-29951. Authored-by: Yuanjian Li <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>

…f ansi mode config Fix the inconsistent behavior of build-in function SQL LEFT/RIGHT. As the comment in apache#26497 (comment), Postgre dialect should not be affected by the ANSI mode config. During reran the existing tests, only the LEFT/RIGHT build-in SQL function broke the assumption. We fix this by following https://www.postgresql.org/docs/12/sql-keywords-appendix.html: `LEFT/RIGHT reserved (can be function or type)` Yes, the Postgre dialect will not be affected by the ANSI mode config. Existing UT. Closes apache#26584 from xuanyuanking/SPARK-29951. Authored-by: Yuanjian Li <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>

cloud-fan reviewed Nov 13, 2019

View reviewed changes

maropu force-pushed the ImportTests branch from bfa07a5 to 56ceb9f Compare November 13, 2019 10:48

cloud-fan reviewed Nov 13, 2019

View reviewed changes

maropu added 2 commits November 13, 2019 22:06

Fix

8f2c066

Fix

615347e

maropu force-pushed the ImportTests branch from 0d9e462 to 615347e Compare November 13, 2019 13:17

cloud-fan reviewed Nov 13, 2019

View reviewed changes

cloud-fan approved these changes Nov 13, 2019

View reviewed changes

Fix

93051fc

dongjoon-hyun reviewed Nov 13, 2019

View reviewed changes

dongjoon-hyun added SQL TESTS labels Nov 13, 2019

Fix

ab0730b

maropu commented Nov 14, 2019

View reviewed changes

cloud-fan closed this in b5a02d3 Nov 14, 2019

xuanyuanking mentioned this pull request Nov 18, 2019

[SPARK-29951][SQL] Make the behavior of Postgre dialect independent of ansi mode config #26584

Closed

		@@ -0,0 +1,2 @@
		-- throw an exception instead of returning NULL, according to SQL ANSI 2011

		@@ -0,0 +1,2 @@
		--- malformed interval literal with ansi mode

		@@ -0,0 +1,2 @@
		--- malformed interval literal with ansi mode
		--import literals.sql

[SPARK-29873][SQL][TESTS] Support --import directive to load queries from another test case in SQLQueryTestSuite #26497

[SPARK-29873][SQL][TESTS] Support --import directive to load queries from another test case in SQLQueryTestSuite #26497

Uh oh!

Conversation

maropu commented Nov 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

SparkQA commented Nov 13, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Nov 13, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cloud-fan commented Nov 13, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Nov 13, 2019

Uh oh!

SparkQA commented Nov 13, 2019

Uh oh!

SparkQA commented Nov 13, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maropu Nov 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Nov 14, 2019

Uh oh!

cloud-fan commented Nov 14, 2019

Uh oh!

maropu commented Nov 14, 2019

Uh oh!

Reviewers

Assignees

Labels

[SPARK-29873][SQL][TESTS] Support `--import` directive to load queries from another test case in SQLQueryTestSuite #26497

[SPARK-29873][SQL][TESTS] Support `--import` directive to load queries from another test case in SQLQueryTestSuite #26497

maropu commented Nov 13, 2019 •

edited

Loading

maropu Nov 14, 2019 •

edited

Loading