Skip to content

Add a Query option for ShowQuery instead of requiring query_stats.query_text #67

Description

@apstndb

Context

In-process graph rendering wants --show-query (or equivalent) to print the SQL that the caller already holds. visualize.Plan and the dot / graphviz / mermaid / d2 renderers only build the query-text node from ResultSetStats.query_stats key query_text:

// visualize/build_tree.go (v0.11.0)
const queryTextKey = "query_text"
q := &graphmodel.QueryText{Text: m[queryTextKey].GetStringValue()}

There is no Query string field on visualize.BuildOptions, visualize.GraphOptions, or the renderer option structs. The CLI path is fine because Spanner PROFILE JSON usually already contains query_text. Library callers that executed the statement themselves typically have the SQL as a separate string and an empty or stats-only QueryStats map.

execspansql therefore clones ResultSetStats and injects the key before visualize.BuildPlan:

https://github.com/apstndb/execspansql/blob/ff4f7d15df6a88d8d4c9d9ba57e636fcc0c54bb7/internal/planrender/render.go#L213-L225

cloned.QueryStats.Fields["query_text"] = structpb.NewStringValue(opts.Query)

That works, but it mutates a cloned protobuf using an undocumented map key and is easy to get wrong (missing clone, overwriting other stats, ShowQuery true with empty text).

Verified against published github.com/apstndb/spannerplanviz@v0.11.0 and current main (visualize/graph.go, mermaid/options.go). No existing issue.

Request

Add a first-class Query string on the renderer / graph options (or on visualize.BuildOptions / Plan) that ShowQuery uses when set.

If a dedicated field is not wanted, document query_stats.query_text as the supported contract for ShowQuery on BuildPlan / BuildGraph so in-process callers can rely on it without reading buildQueryText.

Alternatives considered

  • Keep cloning ResultSetStats in every downstream (current execspansql workaround).
  • Require callers to round-trip through JSON that already contains query_text (the CLI shape). That is unnecessary when the SQL is already in memory.
  • Put the string only on mermaid/d2 Options and not on dot/graphviz. Worse: ShowQuery already exists on every backend.

Preferred: explicit Query string that takes precedence over query_stats.query_text, leaving the map key as the file/CLI interchange.

Downstream evidence

execspansql PRs #94 (in-process internal/planrender over spannerplanviz v0.11.0) and #95 (plan artifact sink that later feeds that renderer). Merged at ff4f7d1.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions