Skip to content

Commit b63a868

Browse files
yuzefovichyuzefovich
authored
Merge pull request #41503 from yuzefovich/backport19.2-40847
release-19.2: colexec: fix projections of AND and OR operators when NULLs are present
2 parents a454b48 + 75071de commit b63a868

18 files changed

+959
-472
lines changed

‎Makefile

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -787,7 +787,7 @@ DOCGEN_TARGETS := bin/.docgen_bnfs bin/.docgen_functions
787787

788788
EXECGEN_TARGETS = \
789789
pkg/col/coldata/vec.eg.go \
790-
pkg/sql/colexec/and.eg.go \
790+
pkg/sql/colexec/and_or_projection.eg.go \
791791
pkg/sql/colexec/any_not_null_agg.eg.go \
792792
pkg/sql/colexec/avg_agg.eg.go \
793793
pkg/sql/colexec/cast.eg.go \
@@ -1471,7 +1471,7 @@ $(SETTINGS_DOC_PAGE): $(settings-doc-gen)
14711471
@$(settings-doc-gen) gen settings-list --format=html > $@
14721472

14731473
pkg/col/coldata/vec.eg.go: pkg/col/coldata/vec_tmpl.go
1474-
pkg/sql/colexec/and.eg.go: pkg/sql/colexec/and_tmpl.go
1474+
pkg/sql/colexec/and_or_projection.eg.go: pkg/sql/colexec/and_or_projection_tmpl.go
14751475
pkg/sql/colexec/any_not_null_agg.eg.go: pkg/sql/colexec/any_not_null_agg_tmpl.go
14761476
pkg/sql/colexec/avg_agg.eg.go: pkg/sql/colexec/avg_agg_tmpl.go
14771477
pkg/sql/colexec/cast.eg.go: pkg/sql/colexec/cast_tmpl.go

‎pkg/sql/colexec/.gitignore

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
and.eg.go
1+
and_or_projection.eg.go
22
any_not_null_agg.eg.go
33
avg_agg.eg.go
44
cast.eg.go

‎pkg/sql/colexec/and_or_projection_test.go

Lines changed: 298 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,298 @@
1+
// Copyright 2019 The Cockroach Authors.
2+
//
3+
// Use of this software is governed by the Business Source License
4+
// included in the file licenses/BSL.txt.
5+
//
6+
// As of the Change Date specified in that file, in accordance with
7+
// the Business Source License, use of this software will be governed
8+
// by the Apache License, Version 2.0, included in the file
9+
// licenses/APL.txt.
10+
11+
package colexec
12+
13+
import (
14+
"context"
15+
"fmt"
16+
"testing"
17+
18+
"github.com/cockroachdb/cockroach/pkg/col/coldata"
19+
"github.com/cockroachdb/cockroach/pkg/col/coltypes"
20+
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
21+
"github.com/cockroachdb/cockroach/pkg/sql/execinfra"
22+
"github.com/cockroachdb/cockroach/pkg/sql/execinfrapb"
23+
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
24+
"github.com/cockroachdb/cockroach/pkg/sql/types"
25+
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
26+
"github.com/cockroachdb/cockroach/pkg/util/randutil"
27+
)
28+
29+
type andOrTestCase struct {
30+
tuples []tuple
31+
expected []tuple
32+
skipAllNullsInjection bool
33+
}
34+
35+
var (
36+
andTestCases []andOrTestCase
37+
orTestCases []andOrTestCase
38+
)
39+
40+
func init() {
41+
andTestCases = []andOrTestCase{
42+
// All variations of pairs separately first.
43+
{
44+
tuples: tuples{{false, true}},
45+
expected: tuples{{false}},
46+
},
47+
{
48+
tuples: tuples{{false, nil}},
49+
expected: tuples{{false}},
50+
},
51+
{
52+
tuples: tuples{{false, false}},
53+
expected: tuples{{false}},
54+
},
55+
{
56+
tuples: tuples{{true, true}},
57+
expected: tuples{{true}},
58+
},
59+
{
60+
tuples: tuples{{true, false}},
61+
expected: tuples{{false}},
62+
},
63+
{
64+
tuples: tuples{{true, nil}},
65+
expected: tuples{{nil}},
66+
// The case of {nil, nil} is explicitly tested below.
67+
skipAllNullsInjection: true,
68+
},
69+
{
70+
tuples: tuples{{nil, true}},
71+
expected: tuples{{nil}},
72+
// The case of {nil, nil} is explicitly tested below.
73+
skipAllNullsInjection: true,
74+
},
75+
{
76+
tuples: tuples{{nil, false}},
77+
expected: tuples{{false}},
78+
},
79+
{
80+
tuples: tuples{{nil, nil}},
81+
expected: tuples{{nil}},
82+
},
83+
// Now all variations of pairs combined together to make sure that nothing
84+
// funky going on with multiple tuples.
85+
{
86+
tuples: tuples{
87+
{false, true}, {false, nil}, {false, false},
88+
{true, true}, {true, false}, {true, nil},
89+
{nil, true}, {nil, false}, {nil, nil},
90+
},
91+
expected: tuples{
92+
{false}, {false}, {false},
93+
{true}, {false}, {nil},
94+
{nil}, {false}, {nil},
95+
},
96+
},
97+
}
98+
99+
orTestCases = []andOrTestCase{
100+
// All variations of pairs separately first.
101+
{
102+
tuples: tuples{{false, true}},
103+
expected: tuples{{true}},
104+
},
105+
{
106+
tuples: tuples{{false, nil}},
107+
expected: tuples{{nil}},
108+
// The case of {nil, nil} is explicitly tested below.
109+
skipAllNullsInjection: true,
110+
},
111+
{
112+
tuples: tuples{{false, false}},
113+
expected: tuples{{false}},
114+
},
115+
{
116+
tuples: tuples{{true, true}},
117+
expected: tuples{{true}},
118+
},
119+
{
120+
tuples: tuples{{true, false}},
121+
expected: tuples{{true}},
122+
},
123+
{
124+
tuples: tuples{{true, nil}},
125+
expected: tuples{{true}},
126+
},
127+
{
128+
tuples: tuples{{nil, true}},
129+
expected: tuples{{true}},
130+
},
131+
{
132+
tuples: tuples{{nil, false}},
133+
expected: tuples{{nil}},
134+
// The case of {nil, nil} is explicitly tested below.
135+
skipAllNullsInjection: true,
136+
},
137+
{
138+
tuples: tuples{{nil, nil}},
139+
expected: tuples{{nil}},
140+
},
141+
// Now all variations of pairs combined together to make sure that nothing
142+
// funky going on with multiple tuples.
143+
{
144+
tuples: tuples{
145+
{false, true}, {false, nil}, {false, false},
146+
{true, true}, {true, false}, {true, nil},
147+
{nil, true}, {nil, false}, {nil, nil},
148+
},
149+
expected: tuples{
150+
{true}, {nil}, {false},
151+
{true}, {true}, {true},
152+
{true}, {nil}, {nil},
153+
},
154+
},
155+
}
156+
}
157+
158+
func TestAndOrOps(t *testing.T) {
159+
defer leaktest.AfterTest(t)()
160+
ctx := context.Background()
161+
st := cluster.MakeTestingClusterSettings()
162+
evalCtx := tree.MakeTestingEvalContext(st)
163+
defer evalCtx.Stop(ctx)
164+
flowCtx := &execinfra.FlowCtx{
165+
EvalCtx: &evalCtx,
166+
Cfg: &execinfra.ServerConfig{
167+
Settings: st,
168+
},
169+
}
170+
171+
for _, test := range []struct {
172+
operation string
173+
cases []andOrTestCase
174+
}{
175+
{
176+
operation: "AND",
177+
cases: andTestCases,
178+
},
179+
{
180+
operation: "OR",
181+
cases: orTestCases,
182+
},
183+
} {
184+
t.Run(test.operation, func(t *testing.T) {
185+
for _, tc := range test.cases {
186+
var runner testRunner
187+
if tc.skipAllNullsInjection {
188+
// We're omitting all nulls injection test. See comments for each such
189+
// test case.
190+
runner = runTestsWithoutAllNullsInjection
191+
} else {
192+
runner = runTestsWithTyps
193+
}
194+
runner(
195+
t,
196+
[]tuples{tc.tuples},
197+
[]coltypes.T{coltypes.Bool, coltypes.Bool},
198+
tc.expected,
199+
orderedVerifier,
200+
func(input []Operator) (Operator, error) {
201+
spec := &execinfrapb.ProcessorSpec{
202+
Input: []execinfrapb.InputSyncSpec{{ColumnTypes: []types.T{*types.Bool, *types.Bool}}},
203+
Core: execinfrapb.ProcessorCoreUnion{
204+
Noop: &execinfrapb.NoopCoreSpec{},
205+
},
206+
Post: execinfrapb.PostProcessSpec{
207+
RenderExprs: []execinfrapb.Expression{{Expr: fmt.Sprintf("@1 %s @2", test.operation)}},
208+
},
209+
}
210+
result, err := NewColOperator(ctx, flowCtx, spec, input)
211+
if err != nil {
212+
return nil, err
213+
}
214+
return result.Op, nil
215+
})
216+
}
217+
})
218+
}
219+
}
220+
221+
func benchmarkLogicalProjOp(
222+
b *testing.B, operation string, useSelectionVector bool, hasNulls bool,
223+
) {
224+
ctx := context.Background()
225+
st := cluster.MakeTestingClusterSettings()
226+
evalCtx := tree.MakeTestingEvalContext(st)
227+
defer evalCtx.Stop(ctx)
228+
flowCtx := &execinfra.FlowCtx{
229+
EvalCtx: &evalCtx,
230+
Cfg: &execinfra.ServerConfig{
231+
Settings: st,
232+
},
233+
}
234+
rng, _ := randutil.NewPseudoRand()
235+
236+
batch := coldata.NewMemBatch([]coltypes.T{coltypes.Bool, coltypes.Bool})
237+
col1 := batch.ColVec(0).Bool()
238+
col2 := batch.ColVec(0).Bool()
239+
for i := 0; i < int(coldata.BatchSize()); i++ {
240+
col1[i] = rng.Float64() < 0.5
241+
col2[i] = rng.Float64() < 0.5
242+
}
243+
if hasNulls {
244+
nulls1 := batch.ColVec(0).Nulls()
245+
nulls2 := batch.ColVec(0).Nulls()
246+
for i := 0; i < int(coldata.BatchSize()); i++ {
247+
if rng.Float64() < nullProbability {
248+
nulls1.SetNull(uint16(i))
249+
}
250+
if rng.Float64() < nullProbability {
251+
nulls2.SetNull(uint16(i))
252+
}
253+
}
254+
}
255+
batch.SetLength(coldata.BatchSize())
256+
if useSelectionVector {
257+
batch.SetSelection(true)
258+
sel := batch.Selection()
259+
for i := 0; i < int(coldata.BatchSize()); i++ {
260+
sel[i] = uint16(i)
261+
}
262+
}
263+
input := NewRepeatableBatchSource(batch)
264+
265+
spec := &execinfrapb.ProcessorSpec{
266+
Input: []execinfrapb.InputSyncSpec{{ColumnTypes: []types.T{*types.Bool, *types.Bool}}},
267+
Core: execinfrapb.ProcessorCoreUnion{
268+
Noop: &execinfrapb.NoopCoreSpec{},
269+
},
270+
Post: execinfrapb.PostProcessSpec{
271+
RenderExprs: []execinfrapb.Expression{{Expr: fmt.Sprintf("@1 %s @2", operation)}},
272+
},
273+
}
274+
275+
result, err := NewColOperator(ctx, flowCtx, spec, []Operator{input})
276+
if err != nil {
277+
b.Fatal(err)
278+
}
279+
logicalProjOp := result.Op
280+
logicalProjOp.Init()
281+
282+
b.SetBytes(int64(8 * coldata.BatchSize()))
283+
for i := 0; i < b.N; i++ {
284+
logicalProjOp.Next(ctx)
285+
}
286+
}
287+
288+
func BenchmarkLogicalProjOp(b *testing.B) {
289+
for _, operation := range []string{"AND", "OR"} {
290+
for _, useSel := range []bool{true, false} {
291+
for _, hasNulls := range []bool{true, false} {
292+
b.Run(fmt.Sprintf("%s,useSel=%t,hasNulls=%t", operation, useSel, hasNulls), func(b *testing.B) {
293+
benchmarkLogicalProjOp(b, operation, useSel, hasNulls)
294+
})
295+
}
296+
}
297+
}
298+
}

0 commit comments

Comments
 (0)