Skip to content

Commit

Permalink
Merge #121163
Browse files Browse the repository at this point in the history
121163: rowexec: prevent panics with UDFs in edge cases r=yuzefovich a=yuzefovich

This commit makes it so that we don't panic with assertion failure in the projectSet processor whenever the datum is of an unexpected type. We've seen this scenario happen in a few different UDFs / SPs, and normally it would lead to an internal error, but if vectorized engine is disabled, it would crash the node. This patch makes it so that it's always an internal error.

Informs: #113186.
Informs: #114846.
Informs: #120942.

Epic: None

Release note: None

Co-authored-by: Yahor Yuzefovich <[email protected]>
  • Loading branch information
craig[bot] and yuzefovich committed Mar 27, 2024
2 parents 548f610 + c7e5994 commit ec9444f
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 9 deletions.
7 changes: 7 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/udf
Original file line number Diff line number Diff line change
Expand Up @@ -785,3 +785,10 @@ FROM purchase
10.00 GBP

subtest end

statement ok
CREATE FUNCTION f113186() RETURNS RECORD AS $$ SELECT 1.99; $$ LANGUAGE SQL;

# Until #113186 is resolved, the internal error is expected.
statement error pgcode XX000 internal error: invalid datum type given: DECIMAL, expected INT8
SELECT * FROM f113186() AS foo(x INT);
18 changes: 15 additions & 3 deletions pkg/sql/rowenc/encoded_datum.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,15 +186,27 @@ func EncDatumValueFromBufferWithOffsetsAndType(

// DatumToEncDatum initializes an EncDatum with the given Datum.
func DatumToEncDatum(ctyp *types.T, d tree.Datum) EncDatum {
ed, err := DatumToEncDatumEx(ctyp, d)
if err != nil {
panic(err)
}
return ed
}

// DatumToEncDatumEx is the same as DatumToEncDatum that returns an error
// instead of panicking under unexpected circumstances.
// TODO(yuzefovich): we should probably get rid of DatumToEncDatum in favor of
// this method altogether.
func DatumToEncDatumEx(ctyp *types.T, d tree.Datum) (EncDatum, error) {
if d == nil {
panic(errors.AssertionFailedf("cannot convert nil datum to EncDatum"))
return EncDatum{}, errors.AssertionFailedf("cannot convert nil datum to EncDatum")
}

dTyp := d.ResolvedType()
if d != tree.DNull && !ctyp.Equivalent(dTyp) && !dTyp.IsAmbiguous() {
panic(errors.AssertionFailedf("invalid datum type given: %s, expected %s", dTyp.SQLStringForError(), ctyp.SQLStringForError()))
return EncDatum{}, errors.AssertionFailedf("invalid datum type given: %s, expected %s", dTyp.SQLStringForError(), ctyp.SQLStringForError())
}
return EncDatum{Datum: d}
return EncDatum{Datum: d}, nil
}

// NullEncDatum initializes an EncDatum with the NULL value.
Expand Down
24 changes: 18 additions & 6 deletions pkg/sql/rowexec/project_set.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,15 +237,21 @@ func (ps *projectSetProcessor) nextGeneratorValues() (newValAvail bool, err erro
return false, err
}
for _, value := range values {
ps.rowBuffer[colIdx] = ps.toEncDatum(value, colIdx)
ps.rowBuffer[colIdx], err = ps.toEncDatum(value, colIdx)
if err != nil {
return false, err
}
colIdx++
}
newValAvail = true
} else {
ps.done[i] = true
// No values left. Fill the buffer with NULLs for future results.
for j := 0; j < numCols; j++ {
ps.rowBuffer[colIdx] = ps.toEncDatum(tree.DNull, colIdx)
ps.rowBuffer[colIdx], err = ps.toEncDatum(tree.DNull, colIdx)
if err != nil {
return false, err
}
colIdx++
}
}
Expand All @@ -262,13 +268,19 @@ func (ps *projectSetProcessor) nextGeneratorValues() (newValAvail bool, err erro
if err != nil {
return false, err
}
ps.rowBuffer[colIdx] = ps.toEncDatum(value, colIdx)
ps.rowBuffer[colIdx], err = ps.toEncDatum(value, colIdx)
if err != nil {
return false, err
}
colIdx++
newValAvail = true
ps.done[i] = true
} else {
// Ensure that every row after the first returns a NULL value.
ps.rowBuffer[colIdx] = ps.toEncDatum(tree.DNull, colIdx)
ps.rowBuffer[colIdx], err = ps.toEncDatum(tree.DNull, colIdx)
if err != nil {
return false, err
}
colIdx++
}
}
Expand Down Expand Up @@ -327,10 +339,10 @@ func (ps *projectSetProcessor) Next() (rowenc.EncDatumRow, *execinfrapb.Producer
return nil, ps.DrainHelper()
}

func (ps *projectSetProcessor) toEncDatum(d tree.Datum, colIdx int) rowenc.EncDatum {
func (ps *projectSetProcessor) toEncDatum(d tree.Datum, colIdx int) (rowenc.EncDatum, error) {
generatedColIdx := colIdx - len(ps.input.OutputTypes())
ctyp := ps.spec.GeneratedColumns[generatedColIdx]
return rowenc.DatumToEncDatum(ctyp, d)
return rowenc.DatumToEncDatumEx(ctyp, d)
}

func (ps *projectSetProcessor) close() {
Expand Down

0 comments on commit ec9444f

Please sign in to comment.