Skip to content

Commit

Permalink
sql: ignore empty statements during Prepare
Browse files Browse the repository at this point in the history
Fixes #9093
Fixes #9518
  • Loading branch information
maddyblue committed Oct 10, 2016
1 parent cb36f48 commit ac5ac4c
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 12 deletions.
2 changes: 1 addition & 1 deletion GLOCKFILE
Expand Up @@ -65,7 +65,7 @@ github.com/kkaneda/returncheck bf081fa7155e3a27df1f056a49d50685edfa5b1b
github.com/kr/pretty cfb55aafdaf3ec08f0db22699ab822c50091b1c4
github.com/kr/text 7cafcd837844e784b526369c9bce262804aebc60
github.com/leekchan/timeutil 28917288c48df3d2c1cfe468c273e0b2adda0aa5
github.com/lib/pq 50761b0867bd1d9d069276790bcd4a3bccf2324a
github.com/lib/pq fcb9ef54da7cae1ea08f0b5a92f236d83e59294a
github.com/lightstep/lightstep-tracer-go 7ec5005048fddb1fc15627e1bf58796ce01d919e
github.com/mattn/go-isatty 66b8e73f3f5cda9f96b69efd03dd3d7fc4a5cdb8
github.com/mattn/go-runewidth d6bea18f789704b5f83375793155289da36a3c7f
Expand Down
12 changes: 11 additions & 1 deletion sql/executor.go
Expand Up @@ -340,10 +340,20 @@ func (e *Executor) Prepare(
if traceSQL {
log.Eventf(session.Ctx(), "preparing: %s", query)
}
stmt, err := parser.ParseOne(query, parser.Syntax(session.Syntax))
var p parser.Parser
stmts, err := p.Parse(query, parser.Syntax(session.Syntax))
if err != nil {
return nil, err
}
switch len(stmts) {
case 0:
return nil, nil
case 1:
// ignore
default:
return nil, errors.Errorf("expected 1 statement, but found %d", len(stmts))
}
stmt := stmts[0]
if err = pinfo.ProcessPlaceholderAnnotations(stmt); err != nil {
return nil, err
}
Expand Down
52 changes: 42 additions & 10 deletions sql/pgwire_test.go
Expand Up @@ -660,9 +660,10 @@ CREATE TABLE d.str (s STRING, b BYTES);`
}

type preparedExecTest struct {
qargs []interface{}
rowsAffected int64
error string
qargs []interface{}
rowsAffected int64
error string
rowsAffectedErr bool
}

func (p preparedExecTest) SetArgs(v ...interface{}) preparedExecTest {
Expand All @@ -680,6 +681,11 @@ func (p preparedExecTest) Error(err string) preparedExecTest {
return p
}

func (p preparedExecTest) RowsAffectedErr() preparedExecTest {
p.rowsAffectedErr = true
return p
}

func TestPGPreparedExec(t *testing.T) {
defer leaktest.AfterTest(t)()
var baseTest preparedExecTest
Expand Down Expand Up @@ -797,6 +803,28 @@ func TestPGPreparedExec(t *testing.T) {
baseTest,
},
},
// An empty string is valid in postgres.
{
"",
[]preparedExecTest{
baseTest.RowsAffectedErr(),
},
},
// Empty statements are permitted.
{
";",
[]preparedExecTest{
baseTest.RowsAffectedErr(),
},
},
// Any number of empty statements are permitted with a single statement
// anywhere.
{
"; ; SET DATABASE = system; ;",
[]preparedExecTest{
baseTest,
},
},
}

s, _, _ := serverutils.StartServer(t, base.TestServerArgs{})
Expand All @@ -823,8 +851,10 @@ func TestPGPreparedExec(t *testing.T) {
t.Errorf("%s: %v: expected error: %s, got %s", query, test.qargs, test.error, err)
}
} else {
if rowsAffected, err := result.RowsAffected(); err != nil {
t.Errorf("%s: %v: unexpected error: %s", query, test.qargs, err)
rowsAffected, err := result.RowsAffected()
hasRAErr := err != nil
if hasRAErr != test.rowsAffectedErr {
t.Errorf("%s: expected rows affected error: %v, got %v (error %+v)", query, test.rowsAffectedErr, hasRAErr, err)
} else if rowsAffected != test.rowsAffected {
t.Errorf("%s: %v: expected %v, got %v", query, test.qargs, test.rowsAffected, rowsAffected)
}
Expand Down Expand Up @@ -939,11 +969,13 @@ func TestCmdCompleteVsEmptyStatements(t *testing.T) {
if err != nil {
t.Fatal(err)
}
defer func() {
_ = recover()
}()
_, _ = empty.RowsAffected() // should panic if lib/pq returned a nil result as expected.
t.Fatal("should not get here -- empty result from empty query should panic first")
rows, err := empty.RowsAffected()
if rows != 0 {
t.Fatalf("expected 0 rows, got %d", rows)
}
if err == nil {
t.Fatal("expected error")
}
}

// Unfortunately lib/pq doesn't expose returned command tags directly, but we can test
Expand Down

0 comments on commit ac5ac4c

Please sign in to comment.