Skip to content

Commit

Permalink
Merge #38607
Browse files Browse the repository at this point in the history
38607: exec: don't panic on unhandled datum conversion r=solongordon a=solongordon

Previously GetDatumToPhysicalFn panicked if it encountered an unhandled
Datum type. This could cause server panics during vectorized planning. I
changed it to return an error instead so that the planner can gracefully
fall back in these scenarios.

Release note: None

Co-authored-by: Solon Gordon <[email protected]>
  • Loading branch information
craig[bot] and solongordon committed Jul 3, 2019
2 parents 2a0d991 + ae39b9c commit c552725
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 1 deletion.
8 changes: 7 additions & 1 deletion pkg/sql/exec/types/conv/conv.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,5 +163,11 @@ func GetDatumToPhysicalFn(ct *semtypes.T) func(tree.Datum) (interface{}, error)
return d.Decimal, nil
}
}
panic(fmt.Sprintf("unhandled type %s", ct.DebugString()))
// It would probably be more correct to return an error here, rather than a
// function which always returns an error. But since the function tends to be
// invoked immediately after GetDatumToPhysicalFn is called, this works just
// as well and makes the error handling less messy for the caller.
return func(datum tree.Datum) (interface{}, error) {
return nil, errors.Errorf("unhandled type %s", ct.DebugString())
}
}
6 changes: 6 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/vectorize
Original file line number Diff line number Diff line change
Expand Up @@ -382,3 +382,9 @@ NULL
1
1.0
1.00

# Test unhandled type conversion. (Should fall back to distsql.)
query T
SELECT ARRAY(SELECT 1) FROM a LIMIT 1
----
{1}

0 comments on commit c552725

Please sign in to comment.