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.

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

feat(spanner/spansql): support table_hint_expr at from_clause on query_statement #4457

Merged
merged 4 commits into from Jul 20, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
84 changes: 50 additions & 34 deletions spanner/spansql/parser.go
Expand Up @@ -1885,7 +1885,7 @@ func (p *parser) parseQuery() (Query, *parseError) {
[ LIMIT count [ OFFSET skip_rows ] ]
*/

// TODO: hints, sub-selects, etc.
// TODO: sub-selects, etc.

if err := p.expect("SELECT"); err != nil {
return Query{}, err
Expand Down Expand Up @@ -2111,6 +2111,13 @@ func (p *parser) parseSelectFrom() (SelectFrom, *parseError) {
return nil, err
}
sf := SelectFromTable{Table: tname}
if p.eat("@") {
hint, err := p.parseHints(map[string]string{}, false)
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm... It seems that the documentation is slightly off here. It is actually possible to have more than one table hint, so the variable here should be renamed to hints and the parseHints(..) function should be called with true rather than false.

For example the following query is accepted by Cloud Spanner:

select *
from albums@{FORCE_INDEX=_BASE_TABLE, GROUPBY_SCAN_OPTIMIZATION=FALSE}

if err != nil {
return nil, err
}
sf.Hint = hint
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: same as above, rename to sf.Hints

}

// TODO: The "AS" keyword is optional.
if p.eat("AS") {
Expand Down Expand Up @@ -2159,46 +2166,20 @@ func (p *parser) parseSelectFrom() (SelectFrom, *parseError) {
Type: jt,
LHS: sf,
}
setHint := func(k, v string) {
if sfj.Hints == nil {
sfj.Hints = make(map[string]string)
}
sfj.Hints[k] = v
}
var hints map[string]string
if hashJoin {
setHint("JOIN_METHOD", "HASH_JOIN")
hints = map[string]string{}
hints["JOIN_METHOD"] = "HASH_JOIN"
}

if p.eat("@") {
if err := p.expect("{"); err != nil {
return nil, err
}
for {
if p.sniff("}") {
break
}
tok := p.next()
if tok.err != nil {
return nil, tok.err
}
k := tok.value
if err := p.expect("="); err != nil {
return nil, err
}
tok = p.next()
if tok.err != nil {
return nil, tok.err
}
v := tok.value
setHint(k, v)
if !p.eat(",") {
break
}
}
if err := p.expect("}"); err != nil {
h, err := p.parseHints(hints, true)
if err != nil {
return nil, err
}
hints = h
}
sfj.Hints = hints

sfj.RHS, err = p.parseSelectFrom()
if err != nil {
Expand Down Expand Up @@ -2889,6 +2870,41 @@ func (p *parser) parseAlias() (ID, *parseError) {
return p.parseTableOrIndexOrColumnName()
}

func (p *parser) parseHints(hints map[string]string, multiple bool) (map[string]string, *parseError) {
if hints == nil {
hints = map[string]string{}
}
if err := p.expect("{"); err != nil {
return nil, err
}
for {
if p.sniff("}") {
break
}
tok := p.next()
if tok.err != nil {
return nil, tok.err
}
k := tok.value
if err := p.expect("="); err != nil {
return nil, err
}
tok = p.next()
if tok.err != nil {
return nil, tok.err
}
v := tok.value
hints[k] = v
if !multiple || !p.eat(",") {
break
}
}
if err := p.expect("}"); err != nil {
return nil, err
}
return hints, nil
}

func (p *parser) parseTableOrIndexOrColumnName() (ID, *parseError) {
/*
table_name and column_name and index_name:
Expand Down
14 changes: 14 additions & 0 deletions spanner/spansql/parser_test.go
Expand Up @@ -114,6 +114,20 @@ func TestParseQuery(t *testing.T) {
},
},
},
// with table hint
{`SELECT * FROM Packages@{FORCE_INDEX=PackagesIdx} WHERE package_idx=@packageIdx`,
nktks marked this conversation as resolved.
Show resolved Hide resolved
Query{
Select: Select{
List: []Expr{Star},
From: []SelectFrom{SelectFromTable{Table: "Packages", Hint: map[string]string{"FORCE_INDEX": "PackagesIdx"}}},
Where: ComparisonOp{
Op: Eq,
LHS: ID("package_idx"),
RHS: Param("packageIdx"),
},
},
},
},
{`SELECT * FROM A INNER JOIN B ON A.w = B.y`,
Query{
Select: Select{
Expand Down
8 changes: 8 additions & 0 deletions spanner/spansql/sql.go
Expand Up @@ -365,6 +365,14 @@ func (sel Select) addSQL(sb *strings.Builder) {

func (sft SelectFromTable) SQL() string {
str := sft.Table.SQL()
if len(sft.Hint) > 0 {
for k, v := range sft.Hint {
str += fmt.Sprintf("@{%s=%s}", k, v)
// table hint exists only one key
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above, it seems that the documentation is not completely clear on this, but Cloud Spanner does accept more than one table hint.

break
}
}

if sft.Alias != "" {
str += " AS " + sft.Alias.SQL()
}
Expand Down
18 changes: 18 additions & 0 deletions spanner/spansql/sql_test.go
Expand Up @@ -309,6 +309,24 @@ func TestSQL(t *testing.T) {
`SELECT A, B AS banana FROM Table WHERE C < "whelp" AND D IS NOT NULL ORDER BY OCol DESC LIMIT 1000`,
reparseQuery,
},
{
Query{
Select: Select{
List: []Expr{ID("A")},
From: []SelectFrom{SelectFromTable{
Table: "Table",
Hint: map[string]string{"FORCE_INDEX": "Idx"},
}},
Where: ComparisonOp{
LHS: ID("B"),
Op: Eq,
RHS: Param("b"),
},
},
},
`SELECT A FROM Table@{FORCE_INDEX=Idx} WHERE B = @b`,
reparseQuery,
},
{
Query{
Select: Select{
Expand Down
1 change: 1 addition & 0 deletions spanner/spansql/types.go
Expand Up @@ -394,6 +394,7 @@ type SelectFrom interface {
type SelectFromTable struct {
Table ID
Alias ID // empty if not aliased
Hint map[string]string
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: rename to Hints

}

func (SelectFromTable) isSelectFrom() {}
Expand Down