Skip to content

Commit

Permalink
parser: don't cut off end-of-line comments
Browse files Browse the repository at this point in the history
Previously, the parser (actually the scanner) removed comments that came
at the end of a statement from the raw sql that was returned along with
the parsed AST. This behavior is now removed.

Release note: None
  • Loading branch information
jordanlewis committed Oct 26, 2022
1 parent ac280a9 commit 1de5495
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 9 deletions.
8 changes: 6 additions & 2 deletions pkg/sql/parser/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,6 @@ func (p *Parser) scanOneStmt() (sql string, tokens []sqlSymType, done bool) {
return p.scanner.In()[startPos:], tokens, true
}
preValID = lval.id
posBeforeScan := p.scanner.Pos()
tokens = append(tokens, sqlSymType{})
lval = &tokens[len(tokens)-1]
p.scanner.Scan(lval)
Expand All @@ -197,8 +196,13 @@ func (p *Parser) scanOneStmt() (sql string, tokens []sqlSymType, done bool) {
curFuncBodyCnt--
}
if lval.id == 0 || (curFuncBodyCnt == 0 && lval.id == ';') {
endPos := p.scanner.Pos()
if lval.id == ';' {
// Don't include the ending semicolon, if there is one, in the raw SQL.
endPos--
}
tokens = tokens[:len(tokens)-1]
return p.scanner.In()[startPos:posBeforeScan], tokens, (lval.id == 0)
return p.scanner.In()[startPos:endPos], tokens, (lval.id == 0)
}
lval.pos -= startPos
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/sql/parser/parse_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func TestScanOneStmt(t *testing.T) {
{
sql: `SELECT 1 /* comment */ ; /* comment */ ; /* comment */ `,
exp: []stmt{{
sql: `SELECT 1`,
sql: `SELECT 1 /* comment */ `,
tok: []int{SELECT, ICONST},
}},
},
Expand Down Expand Up @@ -73,11 +73,11 @@ func TestScanOneStmt(t *testing.T) {
sql: ` ; /* x */ SELECT 1 ; SET /* y */ ; ; INSERT INTO table; ;`,
exp: []stmt{
{
sql: `SELECT 1`,
sql: `SELECT 1 `,
tok: []int{SELECT, ICONST},
},
{
sql: `SET`,
sql: `SET /* y */ `,
tok: []int{SET},
},
{
Expand Down
10 changes: 6 additions & 4 deletions pkg/sql/parser/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -639,10 +639,12 @@ func TestParseSQL(t *testing.T) {
{in: ``, exp: nil},
{in: `SELECT 1`, exp: []string{`SELECT 1`}},
{in: `SELECT 1;`, exp: []string{`SELECT 1`}},
{in: `SELECT 1 /* comment */`, exp: []string{`SELECT 1`}},
// We currently chop off beginning-of-line comments.
{in: `/* comment */ SELECT 1`, exp: []string{`SELECT 1`}},
{in: `SELECT 1 /* comment */`, exp: []string{`SELECT 1 /* comment */`}},
{in: `SELECT 1;SELECT 2`, exp: []string{`SELECT 1`, `SELECT 2`}},
{in: `SELECT 1 /* comment */ ;SELECT 2`, exp: []string{`SELECT 1`, `SELECT 2`}},
{in: `SELECT 1 /* comment */ ; /* comment */ SELECT 2`, exp: []string{`SELECT 1`, `SELECT 2`}},
{in: `SELECT 1 /* comment */ ;SELECT 2`, exp: []string{`SELECT 1 /* comment */ `, `SELECT 2`}},
{in: `SELECT 1 /* comment */ ; SELECT 2`, exp: []string{`SELECT 1 /* comment */ `, `SELECT 2`}},
}
var p parser.Parser // Verify that the same parser can be reused.
for _, d := range testData {
Expand All @@ -656,7 +658,7 @@ func TestParseSQL(t *testing.T) {
res = append(res, stmts[i].SQL)
}
if !reflect.DeepEqual(res, d.exp) {
t.Errorf("expected \n%v\n, but found %v", res, d.exp)
t.Errorf("expected \n%v\n, but found %v", d.exp, res)
}
})
}
Expand Down
5 changes: 5 additions & 0 deletions pkg/sql/show_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -736,6 +736,11 @@ func TestShowQueriesFillsInValuesForPlaceholders(t *testing.T) {
[]interface{}{},
"SELECT 'hi'::STRING /* test */",
},
{
"SELECT /* test */ 'hi'::string /* fnord */",
[]interface{}{},
"SELECT 'hi'::STRING /* test */ /* fnord */",
},
}

// Perform both as a simple execution and as a prepared statement,
Expand Down

0 comments on commit 1de5495

Please sign in to comment.