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

ast: change the order of visiting select stmt #522

Merged
merged 4 commits into from
Aug 28, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 10 additions & 8 deletions ast/dml.go
Original file line number Diff line number Diff line change
Expand Up @@ -764,6 +764,8 @@ type SelectStmt struct {
IsAfterUnionDistinct bool
// IsInBraces indicates whether it's a stmt in brace.
IsInBraces bool
// QueryBlockOffset indicates the order of this SelectStmt if counted from left to right in the sql text.
QueryBlockOffset int
Copy link
Contributor

Choose a reason for hiding this comment

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

Where would this field be set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks pretty weird that the values is set from another repo, can we set it during the induction of parser?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems that there is no way to set it correctly here, since the subpart of select stmt will get the offset first. I don't find good ways to set it correctly here.

alivxxx marked this conversation as resolved.
Show resolved Hide resolved
}

// Restore implements Node interface.
Expand Down Expand Up @@ -906,6 +908,14 @@ func (n *SelectStmt) Accept(v Visitor) (Node, bool) {
n.TableHints = newHints
}

if n.Fields != nil {
node, ok := n.Fields.Accept(v)
if !ok {
return n, false
}
n.Fields = node.(*FieldList)
}

if n.From != nil {
node, ok := n.From.Accept(v)
if !ok {
Expand All @@ -922,14 +932,6 @@ func (n *SelectStmt) Accept(v Visitor) (Node, bool) {
n.Where = node.(ExprNode)
}

if n.Fields != nil {
node, ok := n.Fields.Accept(v)
if !ok {
return n, false
}
n.Fields = node.(*FieldList)
}

if n.GroupBy != nil {
node, ok := n.GroupBy.Accept(v)
if !ok {
Expand Down
9 changes: 6 additions & 3 deletions parser.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions parser.y
Original file line number Diff line number Diff line change
Expand Up @@ -5738,6 +5738,7 @@ SelectStmtBasic:
SelectStmtOpts: $2.(*ast.SelectStmtOpts),
Distinct: $2.(*ast.SelectStmtOpts).Distinct,
Fields: $3.(*ast.FieldList),
QueryBlockOffset: parser.queryBlockOffset(),
}
if st.SelectStmtOpts.TableHints != nil {
st.TableHints = st.SelectStmtOpts.TableHints
Expand Down Expand Up @@ -8102,6 +8103,7 @@ StatementList:
s.SetText(lexer.stmtText())
}
parser.result = append(parser.result, s)
parser.blockOffset = 0
}
}
| StatementList ';' Statement
Expand All @@ -8112,6 +8114,7 @@ StatementList:
s.SetText(lexer.stmtText())
}
parser.result = append(parser.result, s)
parser.blockOffset = 0
}
}

Expand Down
40 changes: 40 additions & 0 deletions parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4290,3 +4290,43 @@ func (checker *nodeTextCleaner) Enter(in ast.Node) (out ast.Node, skipChildren b
func (checker *nodeTextCleaner) Leave(in ast.Node) (out ast.Node, ok bool) {
return in, true
}

type queryBlockOffsetChecker struct {
curOffset int
mismatch bool
}

func (checker *queryBlockOffsetChecker) Enter(in ast.Node) (ast.Node, bool) {
sel, ok := in.(*ast.SelectStmt)
if !ok {
return in, false
}
checker.curOffset++
if sel.QueryBlockOffset != checker.curOffset {
checker.mismatch = true
}
return in, false
}

func (checker *queryBlockOffsetChecker) Leave(in ast.Node) (out ast.Node, ok bool) {
return in, true
}

func (s *testParserSuite) SelectStmtOffset(c *C) {
parser := parser.New()
sqls := []string{
"select * from t; select * from t",
"select a, (select count(*) from t t1 where t1.b > t.a) from t where b > (select b from t t2 where t2.b = t.a limit 1)",
"select count(*) from t t1 where t1.a < (select count(*) from t t2 where t1.a > t2.a)",
}
checker := &queryBlockOffsetChecker{}
for _, sql := range sqls {
stmts, _, err := parser.Parse(sql, "", "")
c.Assert(err, IsNil)
for _, stmt := range stmts {
checker.curOffset = 0
stmt.Accept(checker)
c.Assert(checker.mismatch, IsFalse)
}
}
}
17 changes: 12 additions & 5 deletions yy_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,12 @@ func TrimComment(txt string) string {

// Parser represents a parser instance. Some temporary objects are stored in it to reduce object allocation during Parse function.
type Parser struct {
charset string
collation string
result []ast.StmtNode
src string
lexer Scanner
charset string
collation string
result []ast.StmtNode
src string
lexer Scanner
blockOffset int

// the following fields are used by yyParse to reduce allocation.
cache []yySymType
Expand Down Expand Up @@ -134,6 +135,7 @@ func (parser *Parser) Parse(sql, charset, collation string) (stmt []ast.StmtNode
parser.collation = collation
parser.src = sql
parser.result = parser.result[:0]
parser.blockOffset = 0

var l yyLexer
parser.lexer.reset(sql)
Expand Down Expand Up @@ -217,6 +219,11 @@ func (parser *Parser) endOffset(v *yySymType) int {
return offset
}

func (parser *Parser) queryBlockOffset() int {
parser.blockOffset++
return parser.blockOffset
}

func toInt(l yyLexer, lval *yySymType, str string) int {
n, err := strconv.ParseUint(str, 10, 64)
if err != nil {
Expand Down