Skip to content

Commit

Permalink
colexec: fixed nullableArgs projection operators
Browse files Browse the repository at this point in the history
Previously, the `Concat` projection operators on arrays directly copy nulls into
the output without performing the actual projection whenever either argument was
null. This is incorrect because `Concat` operator can actually handle null arguments,
and we want the actual projection result instead. Since currently the only
projection operator that needs `nullableArgs == true` behaviour is
`ConcatDatumDatum`, this PR fixed this by adding a boolean field `nullableArgs`
to the logic for operators with `Datum`, `Datum`. If we later introduce another
projection operation that has nullableArgs == true, we should update this code
accordingly.

Fixes: cockroachdb#83091

Release note (bug fix): Fixed a bug in `Concat` projection operators on arrays
that gave output of nulls when the projection operator can actually handle null
arguments and may result in a non-null output.
  • Loading branch information
wenyihu6 committed Jul 18, 2022
1 parent 0e0037e commit 1001a8c
Show file tree
Hide file tree
Showing 9 changed files with 3,282 additions and 1,086 deletions.
11 changes: 6 additions & 5 deletions pkg/sql/colexec/colbuilder/execplan.go
Original file line number Diff line number Diff line change
Expand Up @@ -1991,7 +1991,7 @@ func planProjectionOperators(
}
return planProjectionExpr(
ctx, evalCtx, t.Operator, t.ResolvedType(), t.TypedLeft(), t.TypedRight(),
columnTypes, input, acc, factory, t.Fn.Fn, nil /* cmpExpr */, releasables,
columnTypes, input, acc, factory, t.Fn.Fn, nil /* cmpExpr */, releasables, t.Fn.NullableArgs,
)
case *tree.CaseExpr:
allocator := colmem.NewAllocator(ctx, acc, factory)
Expand Down Expand Up @@ -2143,7 +2143,7 @@ func planProjectionOperators(
case *tree.ComparisonExpr:
return planProjectionExpr(
ctx, evalCtx, t.Operator, t.ResolvedType(), t.TypedLeft(), t.TypedRight(),
columnTypes, input, acc, factory, nil /* binFn */, t, releasables,
columnTypes, input, acc, factory, nil /* binFn */, t, releasables, t.Fn.NullableArgs,
)
case tree.Datum:
op, err = projectDatum(t)
Expand Down Expand Up @@ -2310,6 +2310,7 @@ func planProjectionExpr(
binFn tree.TwoArgFn,
cmpExpr *tree.ComparisonExpr,
releasables *[]execinfra.Releasable,
nullableArgs bool,
) (op colexecop.Operator, resultIdx int, typs []*types.T, err error) {
if err := checkSupportedProjectionExpr(left, right); err != nil {
return nil, resultIdx, typs, err
Expand Down Expand Up @@ -2346,7 +2347,7 @@ func planProjectionExpr(
// appended to the input batch.
op, err = colexecproj.GetProjectionLConstOperator(
allocator, typs, left.ResolvedType(), outputType, projOp, input,
rightIdx, lConstArg, resultIdx, evalCtx, binFn, cmpExpr,
rightIdx, lConstArg, resultIdx, evalCtx, binFn, cmpExpr, nullableArgs,
)
} else {
var leftIdx int
Expand Down Expand Up @@ -2423,7 +2424,7 @@ func planProjectionExpr(
// all other projection operators.
op, err = colexecproj.GetProjectionRConstOperator(
allocator, typs, right.ResolvedType(), outputType, projOp,
input, leftIdx, rConstArg, resultIdx, evalCtx, binFn, cmpExpr,
input, leftIdx, rConstArg, resultIdx, evalCtx, binFn, cmpExpr, nullableArgs,
)
}
} else {
Expand All @@ -2438,7 +2439,7 @@ func planProjectionExpr(
resultIdx = len(typs)
op, err = colexecproj.GetProjectionOperator(
allocator, typs, outputType, projOp, input, leftIdx, rightIdx,
resultIdx, evalCtx, binFn, cmpExpr,
resultIdx, evalCtx, binFn, cmpExpr, nullableArgs,
)
}
}
Expand Down
Loading

0 comments on commit 1001a8c

Please sign in to comment.