Skip to content

Commit 47ba9d9

Browse files
yuzefovichyuzefovich
committed
sql: fix recent regression in EXPLAIN ANALYZE
This commit fixes a regression in EXPLAIN ANALYZE output recently added in 44da781. Before that change we always performed the association of the current planNode with the last stage of the physical plan, and after that change we started doing the association only when a new stage is added. However, this resulted in missing associations when a particular planNode didn't result in a new stage of the physical plan (for example, renderNode is handled as a projection by adjusting PostProcessSpec of already existing stage). As a result, we could lose the ability to associate execution statistics to some nodes in EXPLAIN ANALYZE output. This is now fixed by bringing back the unconditional call to associate the current planNode with the last stage of the physical plan, even if a new stage isn't added. This required teaching `associateNodeWithComponents` to "swallow" duplicate associations and make them no-ops (otherwise, we would double-count statistics in some cases). Release note: None
1 parent ff8d2bd commit 47ba9d9

File tree

3 files changed

+140
-3
lines changed

3 files changed

+140
-3
lines changed

‎pkg/sql/distsql_physical_planner.go

+16-2
Original file line numberDiff line numberDiff line change
@@ -1066,8 +1066,9 @@ func (p *PlanningCtx) setUpForMainQuery(
10661066
}
10671067

10681068
// associateWithPlanNode returns a callback function (possibly nil) that should
1069-
// be invoked when a new stage of processors is added that corresponds to the
1070-
// given planNode.
1069+
// be invoked to associate the last stage of processors with the given planNode.
1070+
// The returned function can be safely invoked multiple times for the given
1071+
// planNode and the same last stage of processors.
10711072
func (p *PlanningCtx) associateWithPlanNode(node planNode) func(*physicalplan.PhysicalPlan) {
10721073
if p.associateNodeWithComponents == nil {
10731074
return nil
@@ -4227,6 +4228,19 @@ func (dsp *DistSQLPlanner) createPhysPlanForPlanNode(
42274228
return nil, err
42284229
}
42294230

4231+
if associateWithPlanNode := planCtx.associateWithPlanNode(node); associateWithPlanNode != nil {
4232+
// Even though we might not have created a new stage in the physical
4233+
// plan for the current planNode, we still need to associate it with the
4234+
// last stage. This is needed to handle cases where a single physical
4235+
// plan stage handles multiple planNodes (e.g. renderNode that is done
4236+
// by adjusting PostProcessSpec).
4237+
//
4238+
// Note that it is ok if we already did the association with the current
4239+
// planNode when created the last stage of the physical plan - the
4240+
// callback handles this case safely (i.e. without double-counting).
4241+
associateWithPlanNode(&plan.PhysicalPlan)
4242+
}
4243+
42304244
return plan, err
42314245
}
42324246

‎pkg/sql/instrumentation.go

+25-1
Original file line numberDiff line numberDiff line change
@@ -959,11 +959,35 @@ type execNodeTraceMetadata map[exec.Node][]execComponents
959959
type execComponents []execinfrapb.ComponentID
960960

961961
// associateNodeWithComponents is called during planning, as processors are
962-
// planned for an execution operator.
962+
// planned for an execution operator. This function can be called multiple times
963+
// for the same exec.Node and execComponents.
963964
func (m execNodeTraceMetadata) associateNodeWithComponents(
964965
node exec.Node, components execComponents,
965966
) {
966967
if prevComponents, ok := m[node]; ok {
968+
// We already have some components associated with this node. Check
969+
// whether this is a duplicate association (that should be a no-op).
970+
var dup bool
971+
for _, oldComponents := range prevComponents {
972+
if len(oldComponents) != len(components) {
973+
continue
974+
}
975+
dup = true
976+
for i := range oldComponents {
977+
// Duplicate association can only happen when component IDs are
978+
// in exactly the same order.
979+
if oldComponents[i] != components[i] {
980+
dup = false
981+
break
982+
}
983+
}
984+
if dup {
985+
// This association has already been performed.
986+
return
987+
}
988+
}
989+
// This must be a new stage in the physical plan, so we want to extend
990+
// the mapping for the exec.Node.
967991
m[node] = append(prevComponents, components)
968992
} else {
969993
m[node] = []execComponents{components}

‎pkg/sql/opt/exec/execbuilder/testdata/explain_analyze

+99
Original file line numberDiff line numberDiff line change
@@ -82,3 +82,102 @@ vectorized: true
8282
estimated row count: 10 (missing stats)
8383
table: privileges@privileges_path_user_id_key
8484
spans: /"vtable/crdb_internal/tables"-/"vtable/crdb_internal/tables"/PrefixEnd
85+
86+
query T
87+
EXPLAIN ANALYZE SHOW TABLES;
88+
----
89+
planning time: 10µs
90+
execution time: 100µs
91+
distribution: <hidden>
92+
vectorized: <hidden>
93+
plan type: custom
94+
maximum memory usage: <hidden>
95+
network usage: <hidden>
96+
regions: <hidden>
97+
isolation level: serializable
98+
priority: normal
99+
quality of service: regular
100+
·
101+
• sort
102+
│ sql nodes: <hidden>
103+
│ regions: <hidden>
104+
│ actual row count: 1
105+
│ estimated max memory allocated: 0 B
106+
│ order: +nspname,+relname
107+
│
108+
└── • render
109+
│
110+
└── • hash join (left outer)
111+
│ sql nodes: <hidden>
112+
│ regions: <hidden>
113+
│ actual row count: 1
114+
│ estimated max memory allocated: 0 B
115+
│ equality: (column80) = (table_id)
116+
│
117+
├── • render
118+
│ │
119+
│ └── • hash join (left outer)
120+
│ │ sql nodes: <hidden>
121+
│ │ regions: <hidden>
122+
│ │ actual row count: 1
123+
│ │ estimated max memory allocated: 0 B
124+
│ │ equality: (column62) = (table_id)
125+
│ │
126+
│ ├── • render
127+
│ │ │
128+
│ │ └── • hash join (right outer)
129+
│ │ │ sql nodes: <hidden>
130+
│ │ │ regions: <hidden>
131+
│ │ │ actual row count: 1
132+
│ │ │ estimated max memory allocated: 0 B
133+
│ │ │ equality: (oid) = (relowner)
134+
│ │ │
135+
│ │ ├── • virtual table
136+
│ │ │ sql nodes: <hidden>
137+
│ │ │ regions: <hidden>
138+
│ │ │ actual row count: 4
139+
│ │ │ table: pg_roles@primary
140+
│ │ │
141+
│ │ └── • hash join
142+
│ │ │ sql nodes: <hidden>
143+
│ │ │ regions: <hidden>
144+
│ │ │ actual row count: 1
145+
│ │ │ estimated max memory allocated: 0 B
146+
│ │ │ equality: (oid) = (relnamespace)
147+
│ │ │
148+
│ │ ├── • filter
149+
│ │ │ │ sql nodes: <hidden>
150+
│ │ │ │ regions: <hidden>
151+
│ │ │ │ actual row count: 1
152+
│ │ │ │ filter: nspname NOT IN ('crdb_internal', 'information_schema', __more1_10__, 'pg_extension')
153+
│ │ │ │
154+
│ │ │ └── • virtual table
155+
│ │ │ sql nodes: <hidden>
156+
│ │ │ regions: <hidden>
157+
│ │ │ actual row count: 5
158+
│ │ │ table: pg_namespace@primary
159+
│ │ │
160+
│ │ └── • filter
161+
│ │ │ sql nodes: <hidden>
162+
│ │ │ regions: <hidden>
163+
│ │ │ actual row count: 330
164+
│ │ │ filter: relkind IN ('S', 'm', __more1_10__, 'v')
165+
│ │ │
166+
│ │ └── • virtual table
167+
│ │ sql nodes: <hidden>
168+
│ │ regions: <hidden>
169+
│ │ actual row count: 362
170+
│ │ table: pg_class@primary
171+
│ │
172+
│ └── • virtual table
173+
│ sql nodes: <hidden>
174+
│ regions: <hidden>
175+
│ actual row count: 330
176+
│ table: table_row_statistics@primary
177+
│
178+
└── • virtual table
179+
sql nodes: <hidden>
180+
regions: <hidden>
181+
actual row count: 1
182+
table: tables@tables_database_name_idx (partial index)
183+
spans: [/'test' - /'test']

0 commit comments

Comments
 (0)