Skip to content

Commit

Permalink
sql: fix tuple IS NULL logic
Browse files Browse the repository at this point in the history 
Previously, we treated all cases of `x IS NULL` as `x IS NOT DISTINCT
FROM NULL` and `x IS NOT NULL` as `x IS DISTINCT FROM NULL`. However,
these expressions are not equal when `x` is a tuple.

If all elements of `x` are `NULL`, then `x IS NULL` should evaluate to
true, but `x IS DISTINCT FROM NULL` should evaluate to false. If one
element of `x` is `NULL` and one is not null, then `x IS NOT NULL`
should evaluate to false, but `x IS DISTINCT FROM NULL` should evaluate
to true. Therefore, they are not equivalent.

Below is a table of the correct semantics for tuple expressions.

| Tuple        | IS NOT DISTINCT FROM NULL | IS NULL | IS DISTINCT FROM NULL | IS NOT NULL |
| ------------ | ------------------------- | ------- | --------------------- | ----------- |
| (1, 1)       | false                     | false   | true                  | true        |
| (1, NULL)    | false                     | false   | true                  | false       |
| (NULL, NULL) | false                     | true    | true                  | false       |

Notice that `IS NOT DISTINCT FROM` is always the inverse of `IS DISTINCT
FROM`. However, `IS NULL` and `IS NOT NULL` are not inverses given the
tuple `(1, NULL)`.

This commit introduces new SQL operators for `IS NULL` and `IS NOT
NULL`. These operators have evalaution logic that is different from `IS
NOT DISTINCT FROM NULL` and `IS DISTINCT FROM NULL`, respectively.

This commit also introduces new optimizer expression types,
`IsTupleNull` and `IsTupleNotNull`. Normalization rules have been added
for folding these expressions into boolean values when possible.

TODO: add # to actually link the issues below.
Fixes   46675
Informs 46908
Informs 12022

Release note (bug fix): Fixes incorrect logic for `IS NULL` and `IS NOT
NULL` operators with tuples, correctly differentiating them from `IS NOT
DISTINCT FROM NULL` and `IS DISTINCT FROM NULL`, respectively.
  • Loading branch information
@mgartner
mgartner committed May 8, 2020
1 parent aa07ae0 commit bacea2d
Show file tree
Hide file tree
Showing 54 changed files with 1,477 additions and 692 deletions.
32 changes: 16 additions & 16 deletions pkg/sql/colexec/is_null_ops_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,37 +44,37 @@ func TestIsNullProjOp(t *testing.T) {
negate bool
}{
{
desc: "SELECT c, c IS NULL FROM t -- both",
desc: "SELECT c, c IS NOT DISTINCT FROM NULL FROM t -- both",
inputTuples: tuples{{0}, {nil}, {1}, {2}, {nil}},
outputTuples: tuples{{0, false}, {nil, true}, {1, false}, {2, false}, {nil, true}},
negate: false,
},
{
desc: "SELECT c, c IS NULL FROM t -- no NULLs",
desc: "SELECT c, c IS NOT DISTINCT FROM NULL FROM t -- no NULLs",
inputTuples: tuples{{0}, {1}, {2}},
outputTuples: tuples{{0, false}, {1, false}, {2, false}},
negate: false,
},
{
desc: "SELECT c, c IS NULL FROM t -- only NULLs",
desc: "SELECT c, c IS NOT DISTINCT FROM NULL FROM t -- only NULLs",
inputTuples: tuples{{nil}, {nil}},
outputTuples: tuples{{nil, true}, {nil, true}},
negate: false,
},
{
desc: "SELECT c, c IS NOT NULL FROM t -- both",
desc: "SELECT c, c IS DISTINCT FROM NULL FROM t -- both",
inputTuples: tuples{{0}, {nil}, {1}, {2}, {nil}},
outputTuples: tuples{{0, true}, {nil, false}, {1, true}, {2, true}, {nil, false}},
negate: true,
},
{
desc: "SELECT c, c IS NOT NULL FROM t -- no NULLs",
desc: "SELECT c, c IS DISTINCT FROM NULL FROM t -- no NULLs",
inputTuples: tuples{{0}, {1}, {2}},
outputTuples: tuples{{0, true}, {1, true}, {2, true}},
negate: true,
},
{
desc: "SELECT c, c IS NOT NULL FROM t -- only NULLs",
desc: "SELECT c, c IS DISTINCT FROM NULL FROM t -- only NULLs",
inputTuples: tuples{{nil}, {nil}},
outputTuples: tuples{{nil, false}, {nil, false}},
negate: true,
Expand All @@ -84,9 +84,9 @@ func TestIsNullProjOp(t *testing.T) {
for _, c := range testCases {
t.Run(c.desc, func(t *testing.T) {
opConstructor := func(input []colexecbase.Operator) (colexecbase.Operator, error) {
projExpr := "IS NULL"
projExpr := "IS NOT DISTINCT FROM NULL"
if c.negate {
projExpr = "IS NOT NULL"
projExpr = "IS DISTINCT FROM NULL"
}
return createTestProjectingOperator(
ctx, flowCtx, input[0], []*types.T{types.Int},
Expand Down Expand Up @@ -118,37 +118,37 @@ func TestIsNullSelOp(t *testing.T) {
negate bool
}{
{
desc: "SELECT c FROM t WHERE c IS NULL -- both",
desc: "SELECT c FROM t WHERE c IS NOT DISTINCT FROM NULL -- both",
inputTuples: tuples{{0}, {nil}, {1}, {2}, {nil}},
outputTuples: tuples{{nil}, {nil}},
negate: false,
},
{
desc: "SELECT c FROM t WHERE c IS NULL -- no NULLs",
desc: "SELECT c FROM t WHERE c IS NOT DISTINCT FROM NULL -- no NULLs",
inputTuples: tuples{{0}, {1}, {2}},
outputTuples: tuples{},
negate: false,
},
{
desc: "SELECT c FROM t WHERE c IS NULL -- only NULLs",
desc: "SELECT c FROM t WHERE c IS NOT DISTINCT FROM NULL -- only NULLs",
inputTuples: tuples{{nil}, {nil}},
outputTuples: tuples{{nil}, {nil}},
negate: false,
},
{
desc: "SELECT c FROM t WHERE c IS NOT NULL -- both",
desc: "SELECT c FROM t WHERE c IS DISTINCT FROM NULL -- both",
inputTuples: tuples{{0}, {nil}, {1}, {2}, {nil}},
outputTuples: tuples{{0}, {1}, {2}},
negate: true,
},
{
desc: "SELECT c FROM t WHERE c IS NOT NULL -- no NULLs",
desc: "SELECT c FROM t WHERE c IS DISTINCT FROM NULL -- no NULLs",
inputTuples: tuples{{0}, {1}, {2}},
outputTuples: tuples{{0}, {1}, {2}},
negate: true,
},
{
desc: "SELECT c FROM t WHERE c IS NOT NULL -- only NULLs",
desc: "SELECT c FROM t WHERE c IS DISTINCT FROM NULL -- only NULLs",
inputTuples: tuples{{nil}, {nil}},
outputTuples: tuples{},
negate: true,
Expand All @@ -158,9 +158,9 @@ func TestIsNullSelOp(t *testing.T) {
for _, c := range testCases {
t.Run(c.desc, func(t *testing.T) {
opConstructor := func(input []colexecbase.Operator) (colexecbase.Operator, error) {
selExpr := "IS NULL"
selExpr := "IS NOT DISTINCT FROM NULL"
if c.negate {
selExpr = "IS NOT NULL"
selExpr = "IS DISTINCT FROM NULL"
}
spec := &execinfrapb.ProcessorSpec{
Input: []execinfrapb.InputSyncSpec{{ColumnTypes: []*types.T{types.Int}}},
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/logictest/testdata/logic_test/alter_table
View file
Original file line number Diff line number Diff line change
Expand Up @@ -930,7 +930,7 @@ CREATE TABLE t (a INT)
statement ok
INSERT INTO t VALUES (1), (NULL)

statement error validation of NOT NULL constraint failed: validation of CHECK "a IS NOT NULL" failed
statement error validation of NOT NULL constraint failed: validation of CHECK "a IS DISTINCT FROM NULL" failed
ALTER TABLE t ALTER COLUMN a SET NOT NULL

statement ok
Expand Down
14 changes: 14 additions & 0 deletions pkg/sql/opt/exec/execbuilder/scalar.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,20 @@ func (b *Builder) buildBoolean(ctx *buildScalarCtx, scalar opt.ScalarExpr) (tree
case opt.RangeOp:
return b.buildScalar(ctx, scalar.Child(0).(opt.ScalarExpr))

case opt.IsTupleNullOp:
expr, err := b.buildScalar(ctx, scalar.Child(0).(opt.ScalarExpr))
if err != nil {
return nil, err
}
return tree.NewTypedIsNullExpr(expr), nil

case opt.IsTupleNotNullOp:
expr, err := b.buildScalar(ctx, scalar.Child(0).(opt.ScalarExpr))
if err != nil {
return nil, err
}
return tree.NewTypedIsNotNullExpr(expr), nil

default:
panic(errors.AssertionFailedf("invalid op %s", log.Safe(scalar.Op())))
}
Expand Down
88 changes: 44 additions & 44 deletions pkg/sql/opt/exec/execbuilder/testdata/check_constraints
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,51 +21,51 @@ CREATE TABLE t9 (
query TTTTT
EXPLAIN (VERBOSE) UPDATE t9 SET b = b + 1 WHERE a = 5
----
· distributed false · ·
· vectorized false · ·
count · · () ·
└── update · · () ·
│ table t9 · ·
│ set b · ·
│ strategy updater · ·
│ auto commit · · ·
└── render · · (a, b, column11, check1, check2) ·
│ render 0 a · ·
│ render 1 b · ·
│ render 2 column11 · ·
│ render 3 a > column11 · ·
│ render 4 d IS NULL · ·
└── render · · (column11, a, b, d) ·
│ render 0 b + 1 · ·
│ render 1 a · ·
│ render 2 b · ·
│ render 3 d · ·
└── scan · · (a, b, d) ·
· table t9@primary · ·
· spans /5/0-/5/1/2 /5/3/1-/5/3/2 · ·
· parallel · · ·
· distributed false · ·
· vectorized false · ·
count · · () ·
└── update · · () ·
│ table t9 · ·
│ set b · ·
│ strategy updater · ·
│ auto commit · · ·
└── render · · (a, b, column11, check1, check2) ·
│ render 0 a · ·
│ render 1 b · ·
│ render 2 column11 · ·
│ render 3 a > column11 · ·
│ render 4 d IS NOT DISTINCT FROM NULL · ·
└── render · · (column11, a, b, d) ·
│ render 0 b + 1 · ·
│ render 1 a · ·
│ render 2 b · ·
│ render 3 d · ·
└── scan · · (a, b, d) ·
· table t9@primary · ·
· spans /5/0-/5/1/2 /5/3/1-/5/3/2 · ·
· parallel · · ·

query TTTTT
EXPLAIN (VERBOSE) UPDATE t9 SET a = 2 WHERE a = 5
----
· distributed false · ·
· vectorized false · ·
count · · () ·
└── update · · () ·
│ table t9 · ·
│ set a · ·
│ strategy updater · ·
│ auto commit · · ·
└── render · · (a, b, c, d, e, column11, check1, check2) ·
│ render 0 a · ·
│ render 1 b · ·
│ render 2 c · ·
│ render 3 d · ·
│ render 4 e · ·
│ render 5 2 · ·
│ render 6 b < 2 · ·
│ render 7 d IS NULL · ·
└── scan · · (a, b, c, d, e) ·
· table t9@primary · ·
· spans /5-/5/# · ·
· locking strength for update · ·
· distributed false · ·
· vectorized false · ·
count · · () ·
└── update · · () ·
│ table t9 · ·
│ set a · ·
│ strategy updater · ·
│ auto commit · · ·
└── render · · (a, b, c, d, e, column11, check1, check2) ·
│ render 0 a · ·
│ render 1 b · ·
│ render 2 c · ·
│ render 3 d · ·
│ render 4 e · ·
│ render 5 2 · ·
│ render 6 b < 2 · ·
│ render 7 d IS NOT DISTINCT FROM NULL · ·
└── scan · · (a, b, c, d, e) ·
· table t9@primary · ·
· spans /5-/5/# · ·
· locking strength for update · ·
4 changes: 2 additions & 2 deletions pkg/sql/opt/exec/execbuilder/testdata/distsql_tighten_spans
View file
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ EXPLAIN SELECT * FROM p2@p2_id WHERE i >= 1 AND d IS NULL
scan · ·
· table p2@p2_id
· spans /1/#/55/2/NULL-
· filter d IS NULL
· filter d IS NOT DISTINCT FROM NULL

# IS NOT NULL

Expand All @@ -321,7 +321,7 @@ EXPLAIN SELECT * FROM p2@p2_id WHERE i >= 1 AND d IS NOT NULL
scan · ·
· table p2@p2_id
· spans /1/#/55/2/!NULL-
· filter d IS NOT NULL
· filter d IS DISTINCT FROM NULL

# String table

Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/opt/exec/execbuilder/testdata/explain
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ render · ·
│ order -column18,-started
└── render · ·
└── filter · ·
│ filter ((job_type IS NULL) OR (job_type != 'AUTO CREATE STATS')) AND ((finished IS NULL) OR (finished > (now() - '12:00:00')))
│ filter ((job_type IS NOT DISTINCT FROM NULL) OR (job_type != 'AUTO CREATE STATS')) AND ((finished IS NOT DISTINCT FROM NULL) OR (finished > (now() - '12:00:00')))
└── render · ·
└── virtual table · ·
· source jobs@primary
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/opt/exec/execbuilder/testdata/fk
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ tree field description
· vectorized true
render · ·
└── filter · ·
│ filter z IS NULL
│ filter z IS NOT DISTINCT FROM NULL
└── merge-join · ·
│ type left outer
│ equality (a_z, a_y, a_x) = (z, y, x)
Expand All @@ -47,7 +47,7 @@ render · ·
├── scan · ·
│ table b@idx
│ spans /!NULL-
│ filter (a_y IS NOT NULL) AND (a_x IS NOT NULL)
│ filter (a_y IS DISTINCT FROM NULL) AND (a_x IS DISTINCT FROM NULL)
└── scan · ·
· table a@primary
· spans FULL SCAN
Expand Down
Loading

0 comments on commit bacea2d

Please sign in to comment.