Skip to content

[SPARK-27161][SQL] improve the document of SQL keywords#24093

Closed
cloud-fan wants to merge 2 commits intoapache:masterfrom
cloud-fan:parser
Closed

[SPARK-27161][SQL] improve the document of SQL keywords#24093
cloud-fan wants to merge 2 commits intoapache:masterfrom
cloud-fan:parser

Conversation

@cloud-fan
Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Make it more clear about how Spark categories keywords regarding to the config spark.sql.parser.ansi.enabled

How was this patch tested?

existing tests

//============================
// Start of the keywords list
//============================
SELECT: 'SELECT';
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We should sort the keyword list. It's better to do it in a followup to keep the diff small for this PR.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1

Comment thread docs/sql-keywords.md

By default `spark.sql.parser.ansi.enabled` is false.

Below is a list of all the keywords in Spark SQL.
Copy link
Copy Markdown
Contributor Author

@cloud-fan cloud-fan Mar 14, 2019

Choose a reason for hiding this comment

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

I noticed that this list does not exactly match the keywords in Spark, e.g. ABS is not keyword in Spark, SETMINUS is not in this list.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@maropu can you take a closer look later?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ok, I'll check and fix as followup.

@cloud-fan
Copy link
Copy Markdown
Contributor Author

cc @gatorsmile

@SparkQA
Copy link
Copy Markdown

SparkQA commented Mar 14, 2019

Test build #103501 has finished for PR 24093 at commit b75cc81.

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

Comment thread docs/sql-keywords.md Outdated
column names, column aliases, table aliases) in other contexts. Reserved keywords can't be used as
table alias, but can be used as other identifiers.
When `spark.sql.parser.ansi.enabled` is true, Spark SQL has two kinds of keywords:
* Reserved keywords: Keywords that reserved and can't be used as identifiers for table, view, column, alias, etc.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: * Reserved keywords: Keywords that are reserved and can't be used as identifiers for tables, views, columns, aliases, etc.?

Comment thread docs/sql-keywords.md Outdated
table alias, but can be used as other identifiers.
When `spark.sql.parser.ansi.enabled` is true, Spark SQL has two kinds of keywords:
* Reserved keywords: Keywords that reserved and can't be used as identifiers for table, view, column, alias, etc.
* Non-reserved keywords: Keywords that have a special meaning only in particular contexts and can be used as identifiers in other contexts.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: in other contexts. -> in the other contexts, e.g., SELECT 1 WEEK means interval type data, but WEEK can be used as identifiers?

Comment thread docs/sql-keywords.md

By default `spark.sql.parser.ansi.enabled` is false.

Below is a list of all the keywords in Spark SQL.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ok, I'll check and fix as followup.

//============================
// Start of the keywords list
//============================
SELECT: 'SELECT';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1

Comment thread docs/sql-keywords.md
title: SQL Reserved/Non-Reserved Keywords
displayTitle: SQL Reserved/Non-Reserved Keywords
title: Spark SQL Keywords
displayTitle: Spark SQL Keywords
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

spark.sql.parser.ansi.enabled affects parsing behaviours, too, e.g., when true, it makes interval optional. In future, we could change the behaivour of overflow handling in execution for the more strict ANSI compliance. These behaivour changes affected by the ANSI option should be documented not in this document but in another document?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yea, this document is about keywords, not everything about the ansi mode.

Comment thread docs/sql-keywords.md
`spark.sql.parser.ansi.enabled`, which is false by default.
When `spark.sql.parser.ansi.enabled` is false, Spark SQL has two kinds of keywords:
* Non-reserved keywords: Keywords that have a special meaning only in particular contexts and can be used as identifiers in other contexts.
* Strict-non-reserved keywords: A strict version of non-reserved keywords, which can not be used as table alias.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great and this new group is easy-to-understand.

@cloud-fan
Copy link
Copy Markdown
Contributor Author

retest this please

1 similar comment
@cloud-fan
Copy link
Copy Markdown
Contributor Author

retest this please

@SparkQA
Copy link
Copy Markdown

SparkQA commented Mar 15, 2019

Test build #103534 has finished for PR 24093 at commit c066799.

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

@maropu
Copy link
Copy Markdown
Member

maropu commented Mar 15, 2019

retest this please

@SparkQA
Copy link
Copy Markdown

SparkQA commented Mar 16, 2019

Test build #103554 has finished for PR 24093 at commit c066799.

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

@maropu
Copy link
Copy Markdown
Member

maropu commented Mar 18, 2019

Thanks! Merged to master

@maropu maropu closed this Mar 18, 2019
@maropu
Copy link
Copy Markdown
Member

maropu commented Mar 18, 2019

I'll fix some minor issues in follow-up.

maropu pushed a commit that referenced this pull request Mar 18, 2019
## What changes were proposed in this pull request?

Make it more clear about how Spark categories keywords regarding to the config `spark.sql.parser.ansi.enabled`

## How was this patch tested?

existing tests

Closes #24093 from cloud-fan/parser.

Authored-by: Wenchen Fan <wenchen@databricks.com>
Signed-off-by: Takeshi Yamamuro <yamamuro@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants