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 24, 2022
1 parent 34f7dd1 commit 2c97172
Show file tree
Hide file tree
Showing 9 changed files with 3,283 additions and 1,080 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 @@ -2111,15 +2111,15 @@ 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.BinaryExpr:
if err = checkSupportedBinaryExpr(t.TypedLeft(), t.TypedRight(), t.ResolvedType()); err != nil {
return op, resultIdx, typs, err
}
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.IsNullExpr:
return planIsNullProjectionOp(ctx, evalCtx, t.ResolvedType(), t.TypedInnerExpr(), columnTypes, input, acc, false /* negate */, factory, releasables)
Expand Down Expand Up @@ -2349,6 +2349,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 @@ -2385,7 +2386,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 @@ -2462,7 +2463,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 @@ -2477,7 +2478,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 2c97172

Please sign in to comment.