Skip to content

Commit

Permalink
sql: Let parser determine if query is empty
Browse files Browse the repository at this point in the history
  • Loading branch information
dt committed Feb 22, 2016
1 parent 3873c14 commit ae9d003
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 10 deletions.
8 changes: 8 additions & 0 deletions sql/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@ type Response struct {
// The list of results. There is one result object per SQL statement in the
// request.
Results []Result

// Indicates that after parsing, the request contained 0 non-empty statements.
Empty bool
}

// Result corresponds to the execution of a single SQL statement.
Expand Down Expand Up @@ -337,6 +340,11 @@ func (e *Executor) execStmts(sql string, planMaker *planner) Response {
resp.Results = append(resp.Results, makeResultFromError(planMaker, roachpb.NewError(err)))
return resp
}
if len(stmts) == 0 {
resp.Empty = true
return resp
}

for _, stmt := range stmts {
result, err := e.execStmt(stmt, planMaker)
if err != nil {
Expand Down
13 changes: 5 additions & 8 deletions sql/pgwire/v3.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"net"
"reflect"
"strconv"
"strings"

"github.com/lib/pq/oid"

Expand Down Expand Up @@ -553,13 +552,6 @@ func (c *v3Conn) executeStatements(stmts string, params []parser.Datum, formatCo

c.session.Database = c.opts.database

// TODO(dt): this is a clumsy check better left to the actual parser. #3852
if len(strings.TrimSpace(stmts)) == 0 {
// Skip executor and just send EmptyQueryResponse.
c.writeBuf.initMsg(serverMsgEmptyQuery)
return c.writeBuf.finishMsg(c.wr)
}

resp, _, err := c.executor.ExecuteStatements(c.opts.user, c.session, stmts, params)
if err != nil {
return c.sendError(err.Error())
Expand All @@ -570,6 +562,11 @@ func (c *v3Conn) executeStatements(stmts string, params []parser.Datum, formatCo

c.opts.database = c.session.Database
tracing.AnnotateTrace()
if resp.Empty {
// Skip executor and just send EmptyQueryResponse.
c.writeBuf.initMsg(serverMsgEmptyQuery)
return c.writeBuf.finishMsg(c.wr)
}
return c.sendResponse(resp, formatCodes, sendDescription, limit)
}

Expand Down
3 changes: 1 addition & 2 deletions sql/pgwire_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@ func TestCmdCompleteVsEmptyStatements(t *testing.T) {
}
_, _ = nonempty.RowsAffected() // should not panic if lib/pq returned a non-nil result.

empty, err := db.Exec(" ")
empty, err := db.Exec(" ; ; ;")
if err != nil {
t.Fatal(err)
}
Expand All @@ -565,7 +565,6 @@ func TestCmdCompleteVsEmptyStatements(t *testing.T) {
}()
_, _ = 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")
// TODO(dt): clean this up with testify/assert and add tests for less trivial empty queries.
}

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

0 comments on commit ae9d003

Please sign in to comment.