Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(spanner/spansql): support EXTRACT #5218

Merged
merged 7 commits into from Dec 16, 2021
Merged

feat(spanner/spansql): support EXTRACT #5218

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7074964
feat(spanner/spansql): support EXTRACT
rahul2393 Dec 15, 2021
de49d20
added separate Expr for Extract func and added unit and integration t…
rahul2393 Dec 15, 2021
3e1d2f7
add test for year
rahul2393 Dec 15, 2021
123e3b5
repleace atTimeZone func with atTimeZone expression
rahul2393 Dec 15, 2021
631924c
fixing failing tests
rahul2393 Dec 15, 2021
e81d4e5
added negative test, reduced the valid extract part values.
rahul2393 Dec 15, 2021
d5df7a6
remove extra space
rahul2393 Dec 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion spanner/spansql/keywords.go
View file
Open in desktop
Expand Up @@ -134,9 +134,10 @@ func init() {
for _, f := range allFuncs {
funcs[f] = true
}
// Special case for CAST and SAFE_CAST
// Special case for CAST, SAFE_CAST and EXTRACT
funcArgParsers["CAST"] = typedArgParser
funcArgParsers["SAFE_CAST"] = typedArgParser
funcArgParsers["EXTRACT"] = extractArgParser
}

var allFuncs = []string{
Expand Down
60 changes: 60 additions & 0 deletions spanner/spansql/parser.go
View file
Open in desktop
Expand Up @@ -1901,6 +1901,39 @@ func (p *parser) parseType() (Type, *parseError) {
return p.parseBaseOrParameterizedType(true)
}

var extractPartTypes = map[string]TypeBase{
"NANOSECOND": Int64,
"MICROSECOND": Int64,
"MILLISECOND": Int64,
"SECOND": Int64,
"MINUTE": Int64,
"HOUR": Int64,
"DAYOFWEEK": Int64,
"DAY": Int64,
"DAYOFYEAR": Int64,
"WEEK": Int64,
"ISOWEEK": Int64,
"MONTH": Int64,
"QUARTER": Int64,
"YEAR": Int64,
"ISOYEAR": Int64,
"DATE": Date,
}

func (p *parser) parseExtractType() (Type, *parseError) {
var t Type
tok := p.next()
if tok.err != nil {
return Type{}, tok.err
}
base, ok := extractPartTypes[strings.ToUpper(tok.value)] // valid part types for EXTRACT is keyed by upper case strings.
if !ok {
return Type{}, p.errorf("got %q, want valid EXTRACT types", tok.value)
}
t.Base = base
return t, nil
}

func (p *parser) parseBaseOrParameterizedType(withParam bool) (Type, *parseError) {
debugf("parseBaseOrParameterizedType: %v", p)

Expand Down Expand Up @@ -2482,6 +2515,33 @@ var typedArgParser = func(p *parser) (Expr, *parseError) {
}, nil
}

// Special argument parser for EXTRACT
var extractArgParser = func(p *parser) (Expr, *parseError) {
partType, err := p.parseExtractType()
if err != nil {
return nil, err
}
if err := p.expect("FROM"); err != nil {
return nil, err
}
e, err := p.parseTableOrIndexOrColumnName()
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 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

if err != nil {
return nil, err
}
// AT TIME ZONE is optional
if p.eat("AT", "TIME", "ZONE") {
tok := p.next()
if tok.err != nil {
return nil, err
}
return TypedExpr{Expr: Func{Name: "AT TIME ZONE", Args: []Expr{e, StringLiteral(tok.string)}}, Type: partType}, nil
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 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.

}
return TypedExpr{
Expr: e,
Type: partType,
}, nil
}

/*
Expressions

Expand Down
18 changes: 16 additions & 2 deletions spanner/spansql/parser_test.go
View file
Open in desktop
Expand Up @@ -340,6 +340,10 @@ func TestParseExpr(t *testing.T) {
{`STARTS_WITH(Bar, 'B')`, Func{Name: "STARTS_WITH", Args: []Expr{ID("Bar"), StringLiteral("B")}}},
{`CAST(Bar AS STRING)`, Func{Name: "CAST", Args: []Expr{TypedExpr{Expr: ID("Bar"), Type: Type{Base: String}}}}},
{`SAFE_CAST(Bar AS INT64)`, Func{Name: "SAFE_CAST", Args: []Expr{TypedExpr{Expr: ID("Bar"), Type: Type{Base: Int64}}}}},
{`EXTRACT(DATE FROM TIMESTAMP AT TIME ZONE "America/Los_Angeles")`, Func{Name: "EXTRACT", Args: []Expr{
TypedExpr{Expr: Func{Name: "AT TIME ZONE", Args: []Expr{ID("TIMESTAMP"), StringLiteral("America/Los_Angeles")}}, Type: Type{Base: Date}},
}}},
{`EXTRACT(DAY FROM DATE)`, Func{Name: "EXTRACT", Args: []Expr{TypedExpr{Expr: ID("DATE"), Type: Type{Base: Int64}}}}},

// String literal:
// Accept double quote and single quote.
Expand Down Expand Up @@ -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,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: fix indentation

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,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: fix indentation

) PRIMARY KEY(user_id);

-- Trailing comment at end of file.
Expand Down Expand Up @@ -744,12 +750,20 @@ func TestParseDDL(t *testing.T) {
Columns: []ColumnDef{
{Name: "user_id", Type: Type{Base: String, Len: 36}, NotNull: true, Position: line(67)},
{Name: "some_string", Type: Type{Base: String, Len: 16}, NotNull: true, Position: line(68)},
{Name: "some_time", Type: Type{Base: Timestamp}, NotNull: true, Position: line(69)},
{
Name: "number_key", Type: Type{Base: Int64},
Generated: Func{Name: "SAFE_CAST", Args: []Expr{
TypedExpr{Expr: Func{Name: "SUBSTR", Args: []Expr{ID("some_string"), IntegerLiteral(2)}}, Type: Type{Base: Int64}},
}},
Position: line(69),
Position: line(70),
},
{
Name: "generated_date", Type: Type{Base: Date},
Generated: Func{Name: "EXTRACT", Args: []Expr{
TypedExpr{Expr: Func{Name: "AT TIME ZONE", Args: []Expr{ID("some_time"), StringLiteral("CET")}}, Type: Type{Base: Date}},
}},
Position: line(71),
},
},
PrimaryKey: []KeyPart{{Column: "user_id"}},
Expand Down Expand Up @@ -777,7 +791,7 @@ func TestParseDDL(t *testing.T) {
{Marker: "--", Isolated: true, Start: line(49), End: line(49), Text: []string{"Table with row deletion policy."}},

// Comment after everything else.
{Marker: "--", Isolated: true, Start: line(72), End: line(72), Text: []string{"Trailing comment at end of file."}},
{Marker: "--", Isolated: true, Start: line(74), End: line(74), Text: []string{"Trailing comment at end of file."}},
}}},
// No trailing comma:
{`ALTER TABLE T ADD COLUMN C2 INT64`, &DDL{Filename: "filename", List: []DDLStmt{
Expand Down