[SPARK-27103][SQL][MINOR] List SparkSql reserved keywords in alphabet order#23985
[SPARK-27103][SQL][MINOR] List SparkSql reserved keywords in alphabet order#23985SongYadong wants to merge 2 commits intoapache:masterfrom
Conversation
| "terminated", "timestamp", "tinyint", "to", "touch", "trailing", "transaction", "transactions", | ||
| "trigger", "true", "truncate", "unarchive", "undo", "uniontype", "unlock", "unset", "unsigned", | ||
| "update", "uri", "use", "user", "utc", "utctimestamp", "values", "view", "while", "with", | ||
| "work", "write", "year") |
There was a problem hiding this comment.
put them in alphabet order and remove some repeated words, including:
"int", "smallint", "double", "float", "timestamp", "binary", "boolean", "both", "current_date", "current_timestamp", "date"
| "where", "having", "from", "to", "table", "with", "not") | ||
| val hiveStrictNonReservedKeyword = Seq("anti", "cross", "database", "except", "from", "full", | ||
| "having", "inner", "intersect", "join", "left", "natural", "not", "on", "right", "select", | ||
| "semi", "table", "to", "union", "where", "with") |
There was a problem hiding this comment.
put them in alphabet order and remove one repeated word: "from"
|
Test build #4594 has finished for PR 23985 at commit
|
|
cc @maropu |
|
The change itself looks ok to me. But, I feel its a bit cumbersome to always keep this alphabetical order and duplicated entries easily happen by the current hard-to-check format (multiple keywords in a line). For example, postgresql place a single keyword in a line: https://github.com/postgres/postgres/blob/277cb789836b5ddf81aabb80c2058268c70e2f36/src/backend/parser/gram.y#L14975 (this format makes the number of lines brow up though...) |
|
Is it a good time to transform into PostgrSQL style? For me, +1 for that kind of transition for the future. cc @hvanhovell , too. |
|
Indeed it's clearer in PostgreSQL style. But that makes files too long. I'm not sure how to balance. |
|
I don't mind letting this file grow long if it helps readability by adopting PostgreSQL style. |
|
I got it. Let me try now. |
|
Updated. |
|
Looks good. For such change, it might be worth creating a JIRA for this. |
|
@viirya thanks. I will do that. |
SongYadong
changed the title
dongjoon-hyun
left a comment
dongjoon-hyun
left a comment
There was a problem hiding this comment.
+1, LGTM, too. Merged to master.
Thank you, @SongYadong , @srowen , @maropu , @viirya .
|
Thanks for review. @srowen @dongjoon-hyun @maropu @viirya |
… order ## What changes were proposed in this pull request? This PR tries to correct spark-sql reserved keywords' position in list if they are not in alphabetical order. In test suite some repeated words are removed. Also some comments are added for remind. ## How was this patch tested? Existing unit tests. Closes apache#23985 from SongYadong/sql_reserved_alphabet. Authored-by: SongYadong <song.yadong1@zte.com.cn> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
6 participants
What changes were proposed in this pull request?
This PR tries to correct spark-sql reserved keywords' position in list if they are not in alphabetical order.
In test suite some repeated words are removed. Also some comments are added for remind.
How was this patch tested?
Existing unit tests.