Skip to content

Conversation

@xuanyuanking
Copy link
Member

@xuanyuanking xuanyuanking commented Dec 4, 2019

What changes were proposed in this pull request?

Reprocess all PostgreSQL dialect related PRs, listing in order:

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.

@SparkQA
Copy link

SparkQA commented Dec 5, 2019

Test build #114883 has finished for PR 26763 at commit f7ad82e.

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

@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Dec 5, 2019

cc @gatorsmile because this PR seems to be ready.

@xuanyuanking
Copy link
Member Author

Thanks Dongjoon, also cc @cloud-fan

: qualifiedName (',' qualifiedName)*
;

functionName
Copy link
Contributor

Choose a reason for hiding this comment

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

this is for ANSI-compliant, not pgsql dialect, we should keep it.

Copy link
Member Author

Choose a reason for hiding this comment

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

ditto

@cloud-fan
Copy link
Contributor

Note that, we want to revert pgsql dialect, but not all pgsql compatibility features. A simple policy: if a behavior is not protected by conf.usePostgreSQLDialect, it should not be reverted.

xuanyuanking and others added 14 commits December 9, 2019 17:43
…endent of ansi mode config"

This reverts commit 23b3c4f.
…sql.dialect.spark.ansi.enabled""

This reverts commit 40ea4a1.
…"0", and unique prefixes as input and trim input for the boolean data type"

This reverts commit 3b07a4e.
….preferIntegralDivision for PostgreSQL testing"

This reverts commit 71882f1.
… SQL standard

Proposed new expression `SubtractDates` which is used in `date1` - `date2`. It has the `INTERVAL` type, and returns the interval from `date1` (inclusive) and `date2` (exclusive). For example:
```sql
> select date'tomorrow' - date'yesterday';
interval 2 days
```

Closes apache#26034

- To conform the SQL standard which states the result type of `date operand 1` - `date operand 2` must be the interval type. See [4.5.3  Operations involving datetimes and intervals](http://www.contrib.andrew.cmu.edu/~shadow/sql/sql1992.txt).
- Improve Spark SQL UX and allow mixing date and timestamp in subtractions. For example: `select timestamp'now' + (date'2019-10-01' - date'2019-09-15')`

Before the query below returns number of days:
```sql
spark-sql> select date'2019-10-05' - date'2018-09-01';
399
```
After it returns an interval:
```sql
spark-sql> select date'2019-10-05' - date'2018-09-01';
interval 1 years 1 months 4 days
```

- by new tests in `DateExpressionsSuite` and `TypeCoercionSuite`.
- by existing tests in `date.sql`

Closes apache#26112 from MaxGekk/date-subtract.

Authored-by: Maxim Gekk <[email protected]>
Signed-off-by: Yuming Wang <[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]>
@xuanyuanking
Copy link
Member Author

@cloud-fan Thanks for the guidance, it takes me some time to reprocess all the related commits.
The last change keeps the date subtraction and ANSI-compliant support. I also changed the PR description correspondingly.

@SparkQA
Copy link

SparkQA commented Dec 9, 2019

Test build #115041 has finished for PR 26763 at commit 6d978c6.

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

@SparkQA
Copy link

SparkQA commented Dec 9, 2019

Test build #115038 has finished for PR 26763 at commit cc14274.

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

@SparkQA
Copy link

SparkQA commented Dec 10, 2019

Test build #115101 has finished for PR 26763 at commit 17a3c77.

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

@cloud-fan cloud-fan closed this in d9b3069 Dec 10, 2019
@cloud-fan
Copy link
Contributor

thanks, merging to master!

@xuanyuanking xuanyuanking deleted the SPARK-30125 branch December 10, 2019 17:36
@xuanyuanking
Copy link
Member Author

Thanks Wenchen!

@dongjoon-hyun
Copy link
Member

Thank you, @xuanyuanking and @cloud-fan .

Also, cc to the original authors, too. Although we removed these, we are thankful for your original contributions. Sorry again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants