Skip to content

Commit

Permalink
sql: add support for EXPLAIN ANALYZE in SQL shell and a builtin
Browse files Browse the repository at this point in the history
This commit fixes a possible crash that could happen whenever the
internal executor is used to run EXPLAIN ANALYZE statement (the crash
happened because we'd attempt to call `ResetStmtType` on
`streamingCommandResult` which is unimplemented). This commit makes this
method a no-op since we don't care about the stmt type. This affects the
UI SQL shell as well as `crdb_internal.execute_internally` builtin.

It additionally audits all methods of `streamingCommandResult` that
panic. `AddBatch` is ok to not be implemented given that we have
`SupportsBatch` always return `false`. Another panic is removed in
`BufferedResultsLen` (used under read committed) where it's ok to be
a no-op. This commit also moves the code around a bit to have better
layout.

Release note (sql change): EXPLAIN ANALYZE statements are now supported
when executed via UI SQL shell.
  • Loading branch information
yuzefovich committed Jun 10, 2024
1 parent 9e2e4fb commit 0b643f4
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 37 deletions.
33 changes: 21 additions & 12 deletions pkg/server/api_v2_sql_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,18 +72,12 @@ func TestExecSQL(t *testing.T) {
require.NoError(t, err)

if d.HasArg("expect-error") {
type jsonError struct {
Code string `json:"code"`
Message string `json:"message"`
}
type errorResp struct {
Error jsonError `json:"error"`
}

er := errorResp{}
err := json.Unmarshal(r, &er)
require.NoError(t, err)
return fmt.Sprintf("%s|%s", er.Error.Code, er.Error.Message)
code, msg := getErrorResponse(t, r)
return fmt.Sprintf("%s|%s", code, msg)
} else if d.HasArg("expect-no-error") {
code, msg := getErrorResponse(t, r)
require.True(t, code == "" && msg == "", code, msg)
return ""
}
return getMarshalledResponse(t, r)
},
Expand Down Expand Up @@ -126,3 +120,18 @@ func getMarshalledResponse(t *testing.T, r []byte) string {
require.NoError(t, err)
return string(s)
}

func getErrorResponse(t *testing.T, r []byte) (code, message string) {
type jsonError struct {
Code string `json:"code"`
Message string `json:"message"`
}
type errorResp struct {
Error jsonError `json:"error"`
}

er := errorResp{}
err := json.Unmarshal(r, &er)
require.NoError(t, err)
return er.Error.Code, er.Error.Message
}
9 changes: 9 additions & 0 deletions pkg/server/testdata/api_v2_sql
Original file line number Diff line number Diff line change
Expand Up @@ -1062,3 +1062,12 @@ sql admin
},
"num_statements": 4
}

# Note that we don't verify the contents of the response because it's not
# deterministic, so we only check that it didn't result in an error.
sql admin expect-no-error
{
"execute": true,
"statements": [{"sql": "EXPLAIN ANALYZE SELECT 1"}]
}
----
30 changes: 16 additions & 14 deletions pkg/sql/conn_io.go
Original file line number Diff line number Diff line change
Expand Up @@ -868,18 +868,18 @@ type RestrictedCommandResult interface {
// soon as AddBatch returns.
AddBatch(ctx context.Context, batch coldata.Batch) error

// SupportsAddBatch returns whether this command result supports AddBatch
// method of adding the data. If false is returned, then the behavior of
// AddBatch is undefined.
SupportsAddBatch() bool

// BufferedResultsLen returns the length of the results buffer.
BufferedResultsLen() int

// TruncateBufferedResults clears any results that have been buffered after
// given index, and returns true iff any results were actually truncated.
TruncateBufferedResults(idx int) bool

// SupportsAddBatch returns whether this command result supports AddBatch
// method of adding the data. If false is returned, then the behavior of
// AddBatch is undefined.
SupportsAddBatch() bool

// SetRowsAffected sets RowsAffected counter to n. This is used for all
// result types other than tree.Rows.
SetRowsAffected(ctx context.Context, n int)
Expand Down Expand Up @@ -1120,7 +1120,8 @@ func (r *streamingCommandResult) SendNotice(ctx context.Context, notice pgnotice

// ResetStmtType is part of the RestrictedCommandResult interface.
func (r *streamingCommandResult) ResetStmtType(stmt tree.Statement) {
panic("unimplemented")
// This command result doesn't care about the stmt type since it doesn't
// produce pgwire messages.
}

// GetFormatCode is part of the sql.RestrictedCommandResult interface.
Expand Down Expand Up @@ -1150,23 +1151,24 @@ func (r *streamingCommandResult) AddBatch(context.Context, coldata.Batch) error
panic("unimplemented")
}

// SupportsAddBatch is part of the RestrictedCommandResult interface.
func (r *streamingCommandResult) SupportsAddBatch() bool {
return false
}

// BufferedResultsLen is part of the RestrictedCommandResult interface.
func (r *streamingCommandResult) BufferedResultsLen() int {
// Since this implementation is streaming, there is no sensible return
// value here.
panic("unimplemented")
// Since this implementation is streaming, we cannot truncate some buffered
// results. This is achieved by unconditionally returning false in
// TruncateBufferedResults, so this return value doesn't actually matter.
return 0
}

// TruncateBufferedResults is part of the RestrictedCommandResult interface.
func (r *streamingCommandResult) TruncateBufferedResults(int) bool {
return false
}

// SupportsAddBatch is part of the RestrictedCommandResult interface.
func (r *streamingCommandResult) SupportsAddBatch() bool {
return false
}

func (r *streamingCommandResult) DisableBuffering() {
panic("cannot disable buffering here")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,3 +212,6 @@ query T
SELECT crdb_internal.execute_internally('SELECT session_user;', 'User=testuser');
----
root

statement ok
SELECT crdb_internal.execute_internally('EXPLAIN ANALYZE SELECT 1;');
22 changes: 11 additions & 11 deletions pkg/sql/pgwire/command_result.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,11 @@ func (r *commandResult) AddBatch(ctx context.Context, batch coldata.Batch) error
return r.conn.bufferBatch(ctx, batch, r)
}

// SupportsAddBatch is part of the sql.RestrictedCommandResult interface.
func (r *commandResult) SupportsAddBatch() bool {
return true
}

// BufferedResultsLen is part of the sql.RestrictedCommandResult interface.
func (r *commandResult) BufferedResultsLen() int {
return r.conn.writerState.buf.Len()
Expand All @@ -308,11 +313,6 @@ func (r *commandResult) TruncateBufferedResults(idx int) bool {
return true
}

// SupportsAddBatch is part of the sql.RestrictedCommandResult interface.
func (r *commandResult) SupportsAddBatch() bool {
return true
}

// DisableBuffering is part of the sql.RestrictedCommandResult interface.
func (r *commandResult) DisableBuffering() {
r.assertNotReleased()
Expand Down Expand Up @@ -560,18 +560,18 @@ func (r *limitedCommandResult) AddRow(ctx context.Context, row tree.Datums) erro
return nil
}

// RevokePortalPausability is part of the sql.RestrictedCommandResult interface.
func (r *limitedCommandResult) RevokePortalPausability() error {
r.portalPausablity = sql.NotPausablePortalForUnsupportedStmt
return nil
}

// SupportsAddBatch is part of the sql.RestrictedCommandResult interface.
// TODO(yuzefovich): implement limiting behavior for AddBatch.
func (r *limitedCommandResult) SupportsAddBatch() bool {
return false
}

// RevokePortalPausability is part of the sql.RestrictedCommandResult interface.
func (r *limitedCommandResult) RevokePortalPausability() error {
r.portalPausablity = sql.NotPausablePortalForUnsupportedStmt
return nil
}

// moreResultsNeeded is a restricted connection handler that waits for more
// requests for rows from the active portal, during the "execute portal" flow
// when a limit has been specified.
Expand Down

0 comments on commit 0b643f4

Please sign in to comment.