Skip to content

Commit

Permalink
execbuilder: fix nil pointer in tryBuildFastPathInsert
Browse files Browse the repository at this point in the history
This commit fixes a nil pointer error that we previously would always
hit in `tryBuildFastPathInsert` whenever we had a bool value in the
VALUES clause. The bug was introduced in
d2f48b9 where we added the support for
fast path with single row VALUES clauses. The issue is that we ignored
the "ok" boolean for the type conversion, which is now fixed by
explicitly checking a couple of bool expressions as well as not using
the fast path if non-const non-bool expression is present.

Release note (bug fix): Previously, CockroachDB would hit an internal
error when evaluating INSERTs into REGIONAL BY ROW tables where the
source is a VALUES clause with a single row and at least one boolean
expression, and this is now fixed. The bug was introduced in 23.2.0
release.
  • Loading branch information
yuzefovich committed Jun 11, 2024
1 parent 9545977 commit a5c119c
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 2 deletions.
20 changes: 20 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/unique
Original file line number Diff line number Diff line change
Expand Up @@ -1048,3 +1048,23 @@ statement error pgcode 23505 duplicate key value violates unique constraint \"un
INSERT INTO t115377 VALUES (2, 1, 1, 'east')

subtest end

# Regression test for hitting a nil pointer in the insert fast path when seeing
# a bool expression in the VALUES clause (#123103).
statement ok
CREATE TABLE t123103 (
id STRING DEFAULT gen_random_uuid()::STRING NOT NULL,
r region NOT NULL DEFAULT 'us-east'::region,
a STRING NOT NULL,
b BOOL NOT NULL,
CONSTRAINT tab_pkey PRIMARY KEY (r ASC, id ASC),
CONSTRAINT tab_id_key UNIQUE WITHOUT INDEX (id ASC),
CONSTRAINT tab_r_a_b_key UNIQUE (r ASC, a ASC, b ASC),
CONSTRAINT tab_a_b_key UNIQUE WITHOUT INDEX (a ASC, b ASC)
);
INSERT INTO t123103 (id, a, b) VALUES ('1234567890', 'foo', true);

query TTB
SELECT id, a, b FROM t123103;
----
1234567890 foo true
14 changes: 12 additions & 2 deletions pkg/sql/opt/exec/execbuilder/mutation.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,8 +193,18 @@ func (b *Builder) tryBuildFastPathInsert(ins *memo.InsertExpr) (_ execPlan, ok b
panic(errors.AssertionFailedf("expected %d tuple elements in insert fast path uniqueness check, found %d", len(c.InsertCols), len(tuple.Elems)))
}
for k := 0; k < len(tuple.Elems); k++ {
constExpr, _ := tuple.Elems[k].(*memo.ConstExpr)
execFastPathCheck.DatumsFromConstraint[j][execFastPathCheck.InsertCols[k]] = constExpr.Value
var constDatum tree.Datum
switch e := tuple.Elems[k].(type) {
case *memo.ConstExpr:
constDatum = e.Value
case *memo.TrueExpr:
constDatum = tree.DBoolTrue
case *memo.FalseExpr:
constDatum = tree.DBoolFalse
default:
return execPlan{}, false, nil
}
execFastPathCheck.DatumsFromConstraint[j][execFastPathCheck.InsertCols[k]] = constDatum
}
}
uniqCheck := &ins.UniqueChecks[i]
Expand Down

0 comments on commit a5c119c

Please sign in to comment.