[SPARK-29807][SQL] Rename "spark.sql.ansi.enabled" to "spark.sql.dialect.spark.ansi.enabled" #26444

xuanyuanking · 2019-11-09T04:19:34Z

What changes were proposed in this pull request?

Rename config "spark.sql.ansi.enabled" to "spark.sql.dialect.spark.ansi.enabled"

Why are the changes needed?

The relation between "spark.sql.ansi.enabled" and "spark.sql.dialect" is confusing, since the "PostgreSQL" dialect should contain the features of "spark.sql.ansi.enabled".

To make things clearer, we can rename the "spark.sql.ansi.enabled" to "spark.sql.dialect.spark.ansi.enabled", thus the option "spark.sql.dialect.spark.ansi.enabled" is only for Spark dialect.

For the casting and arithmetic operations, runtime exceptions should be thrown if "spark.sql.dialect" is "spark" and "spark.sql.dialect.spark.ansi.enabled" is true or "spark.sql.dialect" is PostgresSQL.

Does this PR introduce any user-facing change?

Yes, the config name changed.

How was this patch tested?

Existing UT.

SparkQA · 2019-11-09T04:26:26Z

Test build #113487 has finished for PR 26444 at commit 660e9f9.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2019-11-09T04:51:34Z

Test build #113488 has finished for PR 26444 at commit 52171db.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala

SparkQA · 2019-11-10T01:37:58Z

Test build #113509 has finished for PR 26444 at commit 0107aac.

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

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala

HyukjinKwon · 2019-11-11T01:38:29Z

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala

@@ -1655,6 +1655,20 @@ object SQLConf {
      .checkValues(Dialect.values.map(_.toString))
      .createWithDefault(Dialect.SPARK.toString)

+  val ANSI_ENABLED = buildConf("spark.sql.ansi.enabled")


Will we deprecate this?

I think so, maybe after a few versions, this config can be deleted.

SparkQA · 2019-11-11T03:09:55Z

Test build #113551 has finished for PR 26444 at commit 1b8a0da.

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

SparkQA · 2019-11-11T04:03:45Z

Test build #113552 has finished for PR 26444 at commit d76d6a5.

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

HyukjinKwon

Looks fine to me

SparkQA · 2019-11-13T05:25:34Z

Test build #113662 has finished for PR 26444 at commit 281fc61.

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

SparkQA · 2019-11-13T08:05:01Z

Test build #113671 has finished for PR 26444 at commit e21de38.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

xuanyuanking · 2019-11-13T08:39:06Z

retest this please.

HyukjinKwon · 2019-11-13T08:44:43Z

retest this please

SparkQA · 2019-11-13T12:41:51Z

Test build #113692 has finished for PR 26444 at commit e21de38.

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

SparkQA · 2019-11-13T12:55:33Z

Test build #113694 has finished for PR 26444 at commit e21de38.

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

dongjoon-hyun · 2019-11-13T18:49:20Z

cc @gatorsmile

SparkQA · 2019-11-14T21:46:23Z

Test build #113803 has finished for PR 26444 at commit 015539f.

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

cloud-fan · 2019-11-15T08:16:46Z

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala

@@ -1673,6 +1673,19 @@ object SQLConf {
      .checkValues(Dialect.values.map(_.toString))
      .createWithDefault(Dialect.SPARK.toString)

+  val ANSI_ENABLED = buildConf("spark.sql.ansi.enabled")
+    .doc("This configuration will be deprecated in the future releases and replaced by" +


is deprecated and will be removed in future releases

Thanks, done in 1cedb3d.

SparkQA · 2019-11-15T12:24:46Z

Test build #113863 has finished for PR 26444 at commit 1cedb3d.

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

cloud-fan · 2019-11-16T09:47:46Z

thanks, merging to master!

xuanyuanking · 2019-11-17T14:13:55Z

Thanks all for the review.

### What changes were proposed in this pull request? Reprocess all PostgreSQL dialect related PRs, listing in order: - #25158: PostgreSQL integral division support [revert] - #25170: UT changes for the integral division support [revert] - #25458: Accept "true", "yes", "1", "false", "no", "0", and unique prefixes as input and trim input for the boolean data type. [revert] - #25697: Combine below 2 feature tags into "spark.sql.dialect" [revert] - #26112: Date substraction support [keep the ANSI-compliant part] - #26444: Rename config "spark.sql.ansi.enabled" to "spark.sql.dialect.spark.ansi.enabled" [revert] - #26463: Cast to boolean support for PostgreSQL dialect [revert] - #26584: Make the behavior of Postgre dialect independent of ansi mode config [keep the ANSI-compliant part] ### Why are the changes needed? As the discussion in http://apache-spark-developers-list.1001551.n3.nabble.com/DISCUSS-PostgreSQL-dialect-td28417.html, we need to remove PostgreSQL dialect form code base for several reasons: 1. The current approach makes the codebase complicated and hard to maintain. 2. Fully migrating PostgreSQL workloads to Spark SQL is not our focus for now. ### Does this PR introduce any user-facing change? Yes, the config `spark.sql.dialect` will be removed. ### How was this patch tested? Existing UT. Closes #26763 from xuanyuanking/SPARK-30125. Lead-authored-by: Yuanjian Li <xyliyuanjian@gmail.com> Co-authored-by: Maxim Gekk <max.gekk@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>

xuanyuanking force-pushed the SPARK-29807 branch from 660e9f9 to 52171db Compare November 9, 2019 04:40

MaxGekk reviewed Nov 9, 2019

View reviewed changes

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala Show resolved Hide resolved

srowen reviewed Nov 9, 2019

View reviewed changes

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala Show resolved Hide resolved

dongjoon-hyun added the SQL label Nov 9, 2019

xuanyuanking force-pushed the SPARK-29807 branch from 52171db to 0107aac Compare November 9, 2019 22:45

xuanyuanking force-pushed the SPARK-29807 branch from 1b8a0da to d76d6a5 Compare November 11, 2019 01:18

HyukjinKwon reviewed Nov 11, 2019

View reviewed changes

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala Outdated Show resolved Hide resolved

HyukjinKwon reviewed Nov 11, 2019

View reviewed changes

maropu mentioned this pull request Nov 12, 2019

[SPARK-29783][SQL] Support SQL Standard/ISO_8601 output style for interval type #26418

Closed

HyukjinKwon approved these changes Nov 13, 2019

View reviewed changes

xuanyuanking added 5 commits November 15, 2019 01:10

Rename the config

162e960

fix

a028662

Address comments

1d2a80e

Fix and comment address

960b1dc

fix

015539f

xuanyuanking force-pushed the SPARK-29807 branch from e21de38 to 015539f Compare November 14, 2019 17:56

cloud-fan reviewed Nov 15, 2019

View reviewed changes

deprecate the old config

1cedb3d

cloud-fan closed this in 40ea4a1 Nov 16, 2019

xuanyuanking deleted the SPARK-29807 branch November 17, 2019 14:13

xuanyuanking mentioned this pull request Dec 4, 2019

[SPARK-30125][SQL] Remove PostgreSQL dialect #26763

Closed

[SPARK-29807][SQL] Rename "spark.sql.ansi.enabled" to "spark.sql.dialect.spark.ansi.enabled" #26444

[SPARK-29807][SQL] Rename "spark.sql.ansi.enabled" to "spark.sql.dialect.spark.ansi.enabled" #26444

Uh oh!

Conversation

xuanyuanking commented Nov 9, 2019

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 9, 2019

Uh oh!

SparkQA commented Nov 9, 2019

Uh oh!

Uh oh!

Uh oh!

SparkQA commented Nov 10, 2019

Uh oh!

Uh oh!

HyukjinKwon Nov 11, 2019

Choose a reason for hiding this comment

Uh oh!

xuanyuanking Nov 13, 2019

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Nov 11, 2019

Uh oh!

SparkQA commented Nov 11, 2019

Uh oh!

HyukjinKwon left a comment

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Nov 13, 2019

Uh oh!

SparkQA commented Nov 13, 2019

Uh oh!

xuanyuanking commented Nov 13, 2019

Uh oh!

HyukjinKwon commented Nov 13, 2019

Uh oh!

SparkQA commented Nov 13, 2019

Uh oh!

SparkQA commented Nov 13, 2019

Uh oh!

dongjoon-hyun commented Nov 13, 2019

Uh oh!

SparkQA commented Nov 14, 2019

Uh oh!

cloud-fan Nov 15, 2019

Choose a reason for hiding this comment

Uh oh!

xuanyuanking Nov 15, 2019

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Nov 15, 2019

Uh oh!

cloud-fan commented Nov 16, 2019

Uh oh!

xuanyuanking commented Nov 17, 2019

Uh oh!

Uh oh!