feat(spanner/spansql): support EXTRACT #5218
Conversation
spanner/spansql/parser.go
Outdated
| if err := p.expect("FROM"); err != nil { | ||
| return nil, err | ||
| } | ||
| e, err := p.parseTableOrIndexOrColumnName() |
There was a problem hiding this comment.
This is formally not correct, but I'm fine with this being a limitation (that we in that case should add to the list of limitations): The definition of the EXTRACT function specifies that any timestamp expression may be given at this position. That also includes any timestamp literal or function call that returns a timestamp. This implementation only allows the use of a column that contains a timestamp.
So it means that the following statement would not parse correctly:
select extract(date from timestamp '2020-01-01T10:00:00+01:00' at time zone 'Europe/Amsterdam') as d
spanner/spansql/parser.go
Outdated
| if tok.err != nil { | ||
| return nil, err | ||
| } | ||
| return TypedExpr{Expr: Func{Name: "AT TIME ZONE", Args: []Expr{e, StringLiteral(tok.string)}}, Type: partType}, nil |
There was a problem hiding this comment.
This is modelling AT TIME ZONE as a function. I don't think that is correct (or at least it will be very difficult to work with when you want to implement it), as function names do normally not contain spaces. Instead, I think this should be considered as a timezone expression, and then be passed in as an optional argument to the EXTRACT function.
In order to get an idea of what works best, it might be good to add (an) empty implementation(s) of the function(s) in spannertest, and add tests for that as well. The implementation(s) can just return a fixed value, but then you know that you have chosen a parsing strategy that will work.
spanner/spansql/parser_test.go
Outdated
| @@ -524,7 +528,9 @@ func TestParseDDL(t *testing.T) { | |||
| CREATE TABLE users ( | |||
| user_id STRING(36) NOT NULL, | |||
| some_string STRING(16) NOT NULL, | |||
| some_time TIMESTAMP NOT NULL, | |||
spanner/spansql/parser_test.go
Outdated
| number_key INT64 AS (SAFE_CAST(SUBSTR(some_string, 2) AS INT64)) STORED, | ||
| generated_date DATE AS (EXTRACT(DATE FROM some_time AT TIME ZONE "CET")) STORED, |
spanner/spansql/sql.go
Outdated
| func (aze AtTimeZoneExpr) SQL() string { return buildSQL(aze) } | ||
| func (aze AtTimeZoneExpr) addSQL(sb *strings.Builder) { | ||
| aze.Expr.addSQL(sb) | ||
| sb.WriteString(" AT TIME ZONE ") |
There was a problem hiding this comment.
super nit: remove extra space
| sb.WriteString(" AT TIME ZONE ") | |
| sb.WriteString(" AT TIME ZONE ") |
* feat(spanner/spansql): support EXTRACT * added separate Expr for Extract func and added unit and integration tests * add test for year * repleace atTimeZone func with atTimeZone expression * fixing failing tests * added negative test, reduced the valid extract part values. * remove extra space Co-authored-by: Rahul Yadav <irahul@google.com>
This adds support for EXTRACT func and similar constructions
Fixes: #5209