fix(spanner/spansql): fix DATE and TIMESTAMP parsing. #4480
Conversation
This enables to parse below cases. - DATE, TIMESTAMP used as identifier.(these words does not reserved keyword.) - DATE, TIMESTAMP used as function.
google-cla
bot
added
the
cla: yes
spanner/spansql/parser.go
Outdated
| @@ -2714,11 +2714,15 @@ func (p *parser) parseLit() (Expr, *parseError) { | |||
| p.back() | |||
| return p.parseArrayLit() | |||
| case tok.caseEqual("DATE"): | |||
| p.back() | |||
| return p.parseDateLit() | |||
| if !p.sniff("=") && !p.sniff(",") && !p.sniff("FROM") && !p.sniff("AS") { | |||
There was a problem hiding this comment.
I think this should be implemented using a positive look-ahead instead of a negative-look ahead. In other words: If the DATE keyword is followed by a string literal, then it is a date literal. Otherwise it is not. The current implementation does not work for for example the following valid query:
SELECT UNIX_DATE(DATE "2008-12-25")Also, adding additional exceptions to the current if-not list will not solve that permanently, as there might be changes to the query syntax that would add additional keywords (i.e. not only AS and FROM) that could follow an identifier.
There was a problem hiding this comment.
Agreeed.
Fixed at 007bcf2
spanner/spansql/keywords.go
Outdated
| @@ -158,6 +158,7 @@ var allFuncs = []string{ | |||
| "BYTE_LENGTH", "CHAR_LENGTH", "CHARACTER_LENGTH", | |||
| "CODE_POINTS_TO_BYTES", "CODE_POINTS_TO_STRING", | |||
| "CONCAT", | |||
| "DATE", | |||
There was a problem hiding this comment.
nit: Do not add this to the list of string functions (see comment above the list)
There was a problem hiding this comment.
Thanks!
Fixed and added more date timestamp functions at 0cb3b33
This modified parsing DATE, TIMESTAMP that if next token is string litelal, then current token treated as DATE, TIMESTAMP literals.
olavloite
added
the
kokoro:force-run
kokoro-team
removed
the
kokoro:force-run
hengfengli
changed the title
|
Thank you! |
This enables to parse below cases.
keyword.)