Skip to content

Commit

Permalink
opt: fix plan gist decoding internal error
Browse files Browse the repository at this point in the history
This commit fixes some cases where `crdb_internal.decode_plan_gist`
could raise internal index-out-of-bound errors when given incorrectly
formed input.

Fixes #109560

Release note: None
  • Loading branch information
mgartner committed Aug 29, 2023
1 parent 60c5154 commit a561201
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 0 deletions.
7 changes: 7 additions & 0 deletions pkg/sql/opt/exec/execbuilder/testdata/explain_gist
Original file line number Diff line number Diff line change
Expand Up @@ -189,3 +189,10 @@ SELECT crdb_internal.decode_plan_gist('AgGwAgQAgQIAAgAEBQITsAICAxgGDA==')
└── • scan
table: ?@?
spans: 1+ spans

# Regression test for #109560. Incorrectly formed plan gist should not cause
# internal error.
query T nosort
SELECT crdb_internal.decode_external_plan_gist('Ag8f')
----
• union all
7 changes: 7 additions & 0 deletions pkg/sql/opt/exec/explain/explain_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,13 @@ func (n *Node) Annotate(id exec.ExplainAnnotationID, value interface{}) {
func newNode(
op execOperator, args interface{}, ordering exec.OutputOrdering, children ...*Node,
) (*Node, error) {
nonNilChildren := make([]*Node, 0, len(children))
for i := range children {
if children[i] != nil {
nonNilChildren = append(nonNilChildren, children[i])
}
}
children = nonNilChildren
inputNodeCols := make([]colinfo.ResultColumns, len(children))
for i := range children {
inputNodeCols[i] = children[i].Columns()
Expand Down
3 changes: 3 additions & 0 deletions pkg/sql/opt/exec/explain/plan_gist_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,9 @@ func (f *PlanGistFactory) decodeOp() execOperator {

func (f *PlanGistFactory) popChild() *Node {
l := len(f.nodeStack)
if l == 0 {
return nil
}
n := f.nodeStack[l-1]
f.nodeStack = f.nodeStack[:l-1]

Expand Down
36 changes: 36 additions & 0 deletions pkg/sql/opt/exec/explain/result_columns.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,22 @@ func getResultColumns(
case filterOp, invertedFilterOp, limitOp, max1RowOp, sortOp, topKOp, bufferOp, hashSetOpOp,
streamingSetOpOp, unionAllOp, distinctOp, saveTableOp, recursiveCTEOp:
// These ops inherit the columns from their first input.
if len(inputs) == 0 {
return nil, nil
}
return inputs[0], nil

case simpleProjectOp:
if len(inputs) == 0 {
return nil, nil
}
a := args.(*simpleProjectArgs)
return projectCols(inputs[0], a.Cols, nil /* colNames */), nil

case serializingProjectOp:
if len(inputs) == 0 {
return nil, nil
}
a := args.(*serializingProjectArgs)
return projectCols(inputs[0], a.Cols, a.ColNames), nil

Expand All @@ -67,19 +76,34 @@ func getResultColumns(
return args.(*renderArgs).Columns, nil

case projectSetOp:
if len(inputs) == 0 {
return nil, nil
}
return appendColumns(inputs[0], args.(*projectSetArgs).ZipCols...), nil

case applyJoinOp:
if len(inputs) == 0 {
return nil, nil
}
a := args.(*applyJoinArgs)
return joinColumns(a.JoinType, inputs[0], a.RightColumns), nil

case hashJoinOp:
if len(inputs) < 2 {
return nil, nil
}
return joinColumns(args.(*hashJoinArgs).JoinType, inputs[0], inputs[1]), nil

case mergeJoinOp:
if len(inputs) < 2 {
return nil, nil
}
return joinColumns(args.(*mergeJoinArgs).JoinType, inputs[0], inputs[1]), nil

case lookupJoinOp:
if len(inputs) == 0 {
return nil, nil
}
a := args.(*lookupJoinArgs)
cols := joinColumns(a.JoinType, inputs[0], tableColumns(a.Table, a.LookupCols))
// The following matches the behavior of execFactory.ConstructLookupJoin.
Expand All @@ -89,23 +113,35 @@ func getResultColumns(
return cols, nil

case ordinalityOp:
if len(inputs) == 0 {
return nil, nil
}
return appendColumns(inputs[0], colinfo.ResultColumn{
Name: args.(*ordinalityArgs).ColName,
Typ: types.Int,
}), nil

case groupByOp:
if len(inputs) == 0 {
return nil, nil
}
a := args.(*groupByArgs)
return groupByColumns(inputs[0], a.GroupCols, a.Aggregations), nil

case scalarGroupByOp:
if len(inputs) == 0 {
return nil, nil
}
a := args.(*scalarGroupByArgs)
return groupByColumns(inputs[0], nil /* groupCols */, a.Aggregations), nil

case windowOp:
return args.(*windowArgs).Window.Cols, nil

case invertedJoinOp:
if len(inputs) == 0 {
return nil, nil
}
a := args.(*invertedJoinArgs)
cols := joinColumns(a.JoinType, inputs[0], tableColumns(a.Table, a.LookupCols))
// The following matches the behavior of execFactory.ConstructInvertedJoin.
Expand Down

0 comments on commit a561201

Please sign in to comment.