Skip to content

Commit

Permalink
sql: avoid writing to column families that do not exist in the primar…
Browse files Browse the repository at this point in the history
…y index

Previously, if a new column family was added during an add
column and an update/insert occurred concurrently, we could
end up writing to this new column family in any primary index.
This was inadequate because if the primary index did not store
the column, then runtime will have trouble reading data from this
table, since after a rollback the added column / column family
will get cleaned up from the table descriptor. To address this,
this patch avoids writing any columns not stored within an index
descriptor.

Fixes: #99950

Release note (bug fix): Concurrent DML while adding
a new column with a new column family can lead to
corruption in the existing primary index. If a rollback
occurs the table may no longer be accessible.
  • Loading branch information
fqazi committed Mar 29, 2023
1 parent 6ec1cd7 commit 710752e
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 1 deletion.
6 changes: 5 additions & 1 deletion pkg/sql/row/writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,11 @@ func prepareInsertOrUpdateBatch(
if !ok {
continue
}

// Skip any values with a default ID not stored in the primary index,
// which can happen if we are adding new columns,
if skip := helper.SkipColumnNotInPrimaryIndexValue(family.DefaultColumnID, values[idx]); skip {
continue
}
typ := fetchedCols[idx].GetType()
marshaled, err := valueside.MarshalLegacy(typ, values[idx])
if err != nil {
Expand Down
5 changes: 5 additions & 0 deletions pkg/sql/rowenc/index_encoding.go
Original file line number Diff line number Diff line change
Expand Up @@ -1079,6 +1079,11 @@ func EncodePrimaryIndex(
// The decoders expect that column family 0 is encoded with a TUPLE value tag, so we
// don't want to use the untagged value encoding.
if len(family.ColumnIDs) == 1 && family.ColumnIDs[0] == family.DefaultColumnID && family.ID != 0 {
// Single column value families which are not stored can be skipped, these
// may exist temporarily while adding a column.
if !storedColumns.Contains(family.DefaultColumnID) {
return nil
}
datum := findColumnValue(family.DefaultColumnID, colMap, values)
// We want to include this column if its value is non-null or
// we were requested to include all of the columns.
Expand Down

0 comments on commit 710752e

Please sign in to comment.