Skip to content

Commit

Permalink
sql: SHOW QUERIES doesn't interpolate placeholders
Browse files Browse the repository at this point in the history
Previously, SHOW QUERIES (and crdb_internal.node_queries,
cluster_queries) interpolated placeholder values into the statement so
that it was possible to see the placeholder values of a prepared
statement.

However, this was an expensive process that spends a lot of CPU for
little reason, since the interpolation was happening in the hot path of
every query.

Now, we include the placeholder values as a separate array in these
tables, to avoid the runtime interpolation costs.

As a side effect of this change, the original comments in a query are
now inclued in SHOW QUERIES and the two active queries tables.

Release note (sql change): the query field in the
crdb_internal.node_queries, crdb_internal.cluster_queries, and SHOW
QUERIES commands no longer interpolates placeholders. The placeholders
are now included as a SQL text array in the new placeholders column of
those tables. Comments from queries are also now included in the
aforementioned query tables.
  • Loading branch information
jordanlewis committed Aug 26, 2022
1 parent 036b50a commit 76a121e
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 30 deletions.
3 changes: 3 additions & 0 deletions pkg/server/serverpb/status.proto
Original file line number Diff line number Diff line change
Expand Up @@ -905,6 +905,9 @@ message ActiveQuery {
// field is only valid if the query is in the EXECUTING phase.
bool is_full_scan = 10;

// The placeholders if any.
repeated string placeholders = 11;

}

// Request object for ListSessions and ListLocalSessions.
Expand Down
17 changes: 11 additions & 6 deletions pkg/sql/conn_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -3153,20 +3153,25 @@ func (ex *connExecutor) serialize() serverpb.Session {
if query.hidden {
continue
}
ast, err := query.getStatement()
if err != nil {
continue
sqlNoConstants := truncateSQL(formatStatementHideConstants(query.stmt.AST))
nPlaceholders := 0
if query.placeholders != nil {
nPlaceholders = len(query.placeholders.Values)
}
placeholders := make([]string, nPlaceholders)
for i := range placeholders {
placeholders[i] = tree.AsStringWithFlags(query.placeholders.Values[i], tree.FmtPgwireText)
}
sqlNoConstants := truncateSQL(formatStatementHideConstants(ast))
sql := truncateSQL(ast.String())
sql := truncateSQL(query.stmt.SQL)
progress := math.Float64frombits(atomic.LoadUint64(&query.progressAtomic))
activeQueries = append(activeQueries, serverpb.ActiveQuery{
TxnID: query.txnID,
ID: id.String(),
Start: query.start.UTC(),
Sql: sql,
SqlNoConstants: sqlNoConstants,
SqlSummary: formatStatementSummary(ast),
SqlSummary: formatStatementSummary(query.stmt.AST),
Placeholders: placeholders,
IsDistributed: query.isDistributed,
Phase: (serverpb.ActiveQuery_Phase)(query.phase),
Progress: float32(progress),
Expand Down
18 changes: 8 additions & 10 deletions pkg/sql/conn_executor_exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,15 +301,9 @@ func (ex *connExecutor) execStmtInOpenState(
// doneAfterFunc will be allocated only when timeoutTicker is non-nil.
var doneAfterFunc chan struct{}

// Early-associate placeholder info with the eval context,
// so that we can fill in placeholder values in our call to addActiveQuery, below.
if !ex.planner.EvalContext().HasPlaceholders() {
ex.planner.EvalContext().Placeholders = pinfo
}

var cancelQuery context.CancelFunc
ctx, cancelQuery = contextutil.WithCancel(ctx)
ex.addActiveQuery(ast, formatWithPlaceholders(ast, ex.planner.EvalContext()), queryID, cancelQuery)
ex.addActiveQuery(parserStmt, pinfo, queryID, cancelQuery)

// Make sure that we always unregister the query. It also deals with
// overwriting res.Error to a more user-friendly message in case of query
Expand Down Expand Up @@ -2046,13 +2040,17 @@ func (ex *connExecutor) enableTracing(modes []string) error {

// addActiveQuery adds a running query to the list of running queries.
func (ex *connExecutor) addActiveQuery(
ast tree.Statement, rawStmt string, queryID clusterunique.ID, cancelQuery context.CancelFunc,
stmt parser.Statement,
placeholders *tree.PlaceholderInfo,
queryID clusterunique.ID,
cancelQuery context.CancelFunc,
) {
_, hidden := ast.(tree.HiddenFromShowQueries)
_, hidden := stmt.AST.(tree.HiddenFromShowQueries)
qm := &queryMeta{
txnID: ex.state.mu.txn.ID(),
start: ex.phaseTimes.GetSessionPhaseTime(sessionphase.SessionQueryReceived),
rawStmt: rawStmt,
stmt: stmt,
placeholders: placeholders,
phase: preparing,
isDistributed: false,
isFullScan: false,
Expand Down
9 changes: 9 additions & 0 deletions pkg/sql/crdb_internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -1758,6 +1758,7 @@ CREATE TABLE crdb_internal.%s (
user_name STRING, -- the user running the query
start TIMESTAMP, -- the start time of the query
query STRING, -- the SQL code of the query
placeholders STRING[], -- the placeholders, if any, passed for the query
client_address STRING, -- the address of the client that issued the query
application_name STRING, -- the name of the application as per SET application_name
distributed BOOL, -- whether the query is running distributed
Expand Down Expand Up @@ -1890,6 +1891,13 @@ func populateQueriesTable(
if err != nil {
return err
}
placeholders := tree.NewDArray(types.String)
placeholders.Array = make(tree.Datums, 0, len(query.Placeholders))
for _, v := range query.Placeholders {
if err := placeholders.Append(tree.NewDString(v)); err != nil {
return err
}
}
if err := addRow(
tree.NewDString(query.ID),
txnID,
Expand All @@ -1898,6 +1906,7 @@ func populateQueriesTable(
tree.NewDString(session.Username),
ts,
tree.NewDString(query.Sql),
placeholders,
tree.NewDString(session.ClientAddress),
tree.NewDString(session.ApplicationName),
isDistributedDatum,
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/delegate/show_queries.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ SELECT
user_name,
start,
query,
placeholders,
client_address,
application_name,
distributed,
Expand Down
19 changes: 5 additions & 14 deletions pkg/sql/exec_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -1933,10 +1933,11 @@ type queryMeta struct {
// The timestamp when this query began execution.
start time.Time

// The string of the SQL statement being executed. This string may
// contain sensitive information, so it must be converted back into
// an AST and dumped before use in logging.
rawStmt string
// The SQL statement being executed.
stmt parser.Statement

// The placeholders that the query was executed with if any.
placeholders *tree.PlaceholderInfo

// States whether this query is distributed. Note that all queries,
// including those that are distributed, have this field set to false until
Expand Down Expand Up @@ -1971,16 +1972,6 @@ func (q *queryMeta) cancel() {
q.cancelQuery()
}

// getStatement returns a cleaned version of the query associated
// with this queryMeta.
func (q *queryMeta) getStatement() (tree.Statement, error) {
parsed, err := parser.ParseOne(q.rawStmt)
if err != nil {
return nil, err
}
return parsed.AST, nil
}

// SessionDefaults mirrors fields in Session, for restoring default
// configuration values in SET ... TO DEFAULT (or RESET ...) statements.
type SessionDefaults map[string]string
Expand Down

0 comments on commit 76a121e

Please sign in to comment.