From a5c119cd7658340109db71a0932cc8a3b4193b4d Mon Sep 17 00:00:00 2001 From: Yahor Yuzefovich Date: Tue, 11 Jun 2024 15:12:29 +0000 Subject: [PATCH] execbuilder: fix nil pointer in tryBuildFastPathInsert 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 d2f48b991a6f7dd5c753738fbe14f7e30a8fb68a 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. --- pkg/sql/logictest/testdata/logic_test/unique | 20 ++++++++++++++++++++ pkg/sql/opt/exec/execbuilder/mutation.go | 14 ++++++++++++-- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/pkg/sql/logictest/testdata/logic_test/unique b/pkg/sql/logictest/testdata/logic_test/unique index 3b279fbc9b5d..4c5ec2c46398 100644 --- a/pkg/sql/logictest/testdata/logic_test/unique +++ b/pkg/sql/logictest/testdata/logic_test/unique @@ -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 diff --git a/pkg/sql/opt/exec/execbuilder/mutation.go b/pkg/sql/opt/exec/execbuilder/mutation.go index 54b5cd8b1be1..792e8e353750 100644 --- a/pkg/sql/opt/exec/execbuilder/mutation.go +++ b/pkg/sql/opt/exec/execbuilder/mutation.go @@ -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]