Skip to content

Commit

Permalink
colexec: fixed nullableArgs projection operators
Browse files Browse the repository at this point in the history
Previously, the projection operators for binary or comparison expressions
directly copy nulls into the output without performing the actual projection
whenever either argument was null. This is incorrect because some function
operators can 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 only adding a
boolean field `nullableArgs` to operators with `Datum`, `Datum`. If we later
introduce another projection operation that has nullableArgs == true, we should
update code accordingly.

Fixes: cockroachdb#83091

Release note (bug fix): Fixed a bug in projection operators that gave output of
nulls when the expression can actually handle null arguments and may result in
a non-null output. Note that if we later
introduce another projection operation that has nullableArgs == true, we should
update code accordingly.
  • Loading branch information
wenyihu6 committed Jul 18, 2022
1 parent 0e11835 commit 7a44e8e
Show file tree
Hide file tree
Showing 11 changed files with 3,290 additions and 1,087 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 @@ -1995,7 +1995,7 @@ func planProjectionOperators(
}
return planProjectionExpr(
ctx, evalCtx, t.Operator, t.ResolvedType(), t.TypedLeft(), t.TypedRight(),
columnTypes, input, acc, factory, t.Op.EvalOp, nil /* cmpExpr */, releasables,
columnTypes, input, acc, factory, t.Op.EvalOp, nil /* cmpExpr */, releasables, t.Op.NullableArgs,
)
case *tree.CaseExpr:
allocator := colmem.NewAllocator(ctx, acc, factory)
Expand Down Expand Up @@ -2147,7 +2147,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.Op.NullableArgs,
)
case tree.Datum:
op, err = projectDatum(t)
Expand Down Expand Up @@ -2314,6 +2314,7 @@ func planProjectionExpr(
binOp tree.BinaryEvalOp,
cmpExpr *tree.ComparisonExpr,
releasables *[]execreleasable.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 @@ -2350,7 +2351,7 @@ func planProjectionExpr(
// appended to the input batch.
op, err = colexecprojconst.GetProjectionLConstOperator(
allocator, typs, left.ResolvedType(), outputType, projOp, input,
rightIdx, lConstArg, resultIdx, evalCtx, binOp, cmpExpr,
rightIdx, lConstArg, resultIdx, evalCtx, binOp, cmpExpr, nullableArgs,
)
} else {
var leftIdx int
Expand Down Expand Up @@ -2427,7 +2428,7 @@ func planProjectionExpr(
// all other projection operators.
op, err = colexecprojconst.GetProjectionRConstOperator(
allocator, typs, right.ResolvedType(), outputType, projOp,
input, leftIdx, rConstArg, resultIdx, evalCtx, binOp, cmpExpr,
input, leftIdx, rConstArg, resultIdx, evalCtx, binOp, cmpExpr, nullableArgs,
)
}
} else {
Expand All @@ -2442,7 +2443,7 @@ func planProjectionExpr(
resultIdx = len(typs)
op, err = colexecproj.GetProjectionOperator(
allocator, typs, outputType, projOp, input, leftIdx, rightIdx,
resultIdx, evalCtx, binOp, cmpExpr,
resultIdx, evalCtx, binOp, cmpExpr, nullableArgs,
)
}
}
Expand Down
Loading

0 comments on commit 7a44e8e

Please sign in to comment.