Skip to content

Commit

Permalink
opt: do not include virtual computed columns in lock columns
Browse files Browse the repository at this point in the history
Virtual computed columns cannot be included in lock columns because
these columns are fetched from the primary index when the lock is
translated into a lookup join. The primary index does not actually store
virtual columns, so attempting to fetch `NOT NULL` virtual columns would
confuse the execution engine into thinking they should be present, and
cause errors with the message "internal error: Non-nullable column ...".

Fixes #130661

Release note (bug fix): A bug has been fixed which could cause errors
with the message "internal error: Non-nullable column ..." when
executing statements under read-committed isolation that involve tables
with `NOT NULL` virtual columns.
  • Loading branch information
mgartner committed Sep 20, 2024
1 parent 6027197 commit 191d8b5
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 3 deletions.
25 changes: 25 additions & 0 deletions pkg/ccl/logictestccl/testdata/logic_test/read_committed
Original file line number Diff line number Diff line change
Expand Up @@ -538,3 +538,28 @@ user testuser

statement ok
COMMIT

subtest regression_130661

statement ok
CREATE TABLE t130661 (
id INT PRIMARY KEY NOT NULL,
i INT NOT NULL,
v INT AS (i + 1) VIRTUAL NOT NULL,
FAMILY (id),
FAMILY (i)
)

statement ok
INSERT INTO t130661 VALUES (1, 10)

statement ok
BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED

statement ok
SELECT * FROM t130661 WHERE id = 1 FOR UPDATE

statement ok
COMMIT

subtest end
8 changes: 5 additions & 3 deletions pkg/sql/opt/optbuilder/locking.go
Original file line number Diff line number Diff line change
Expand Up @@ -438,16 +438,18 @@ func (b *Builder) buildLock(lb *lockBuilder, locking opt.Locking, inScope *scope
newTabID := md.DuplicateTable(lb.table, b.factory.RemapCols)
newTab := md.Table(newTabID)
// Add remapped columns for the new table reference. For now we lock all
// column families of the primary index of the table, so include all ordinary
// and mutation columns.
// non-virtual columns of all families of the primary index of the table, so
// include all ordinary and mutation columns.
ordinals := tableOrdinals(newTab, columnKinds{
includeMutations: true,
includeSystem: false,
includeInverted: false,
})
var lockCols opt.ColSet
for _, ord := range ordinals {
lockCols.Add(newTabID.ColumnID(ord))
if !tab.Column(ord).IsVirtualComputed() {
lockCols.Add(newTabID.ColumnID(ord))
}
}
private := &memo.LockPrivate{
Table: newTabID,
Expand Down
35 changes: 35 additions & 0 deletions pkg/sql/opt/optbuilder/testdata/read-committed
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# Tests for read-committed isolation level.

exec-ddl
CREATE TABLE t130661 (
id INT PRIMARY KEY,
i INT NOT NULL,
v INT AS (i + 10) VIRTUAL NOT NULL
)
----

# Regression test for #130661. The lock columns should not include virtual
# computed columns (in this case the column with ID 8).
build isolation=ReadCommitted
SELECT * FROM t130661 WHERE id = 1 FOR UPDATE
----
lock t130661
├── columns: id:1!null i:2!null v:3!null
├── key columns: id:1
├── lock columns: (6,7)
├── locking: for-update,durability-guaranteed
└── project
├── columns: id:1!null i:2!null v:3!null
└── select
├── columns: id:1!null i:2!null v:3!null crdb_internal_mvcc_timestamp:4 tableoid:5
├── project
│ ├── columns: v:3!null id:1!null i:2!null crdb_internal_mvcc_timestamp:4 tableoid:5
│ ├── scan t130661
│ │ ├── columns: id:1!null i:2!null crdb_internal_mvcc_timestamp:4 tableoid:5
│ │ └── computed column expressions
│ │ └── v:3
│ │ └── i:2 + 10
│ └── projections
│ └── i:2 + 10 [as=v:3]
└── filters
└── id:1 = 1

0 comments on commit 191d8b5

Please sign in to comment.