Skip to content

Commit

Permalink
sql: add assertion failure to prevent use of untested code path
Browse files Browse the repository at this point in the history
We need to write tests for multi-column families as these will not be
hit by the end-to-end tests.

Epic: none
Release note: None
  • Loading branch information
stevendanna committed Oct 4, 2024
1 parent ac2fcc3 commit 55ccbff
Showing 1 changed file with 24 additions and 12 deletions.
36 changes: 24 additions & 12 deletions pkg/sql/row/writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,15 @@ func prepareInsertOrUpdateBatch(
overwrite, traceKV bool,
) ([]byte, error) {
families := helper.TableDesc.GetFamilies()
// TODO(ssd): We don't currently support multiple column
// families on the LDR write path. As a result, we don't have
// good end-to-end testing of multi-column family writes with
// the origin timestamp helper set. Until we write such tests,
// we error if we ever see such writes.
if oth.IsSet() && len(families) > 1 {
return nil, errors.AssertionFailedf("OriginTimestampCPutHelper is not yet testing with multi-column family writes")
}

for i := range families {
family := &families[i]
update := false
Expand Down Expand Up @@ -156,14 +165,14 @@ func prepareInsertOrUpdateBatch(
}

if marshaled.RawBytes == nil {
if overwrite {
if oth.IsSet() {
// If using OriginTimestamp'd CPuts, we _always_ want to issue a Delete
// so that we can confirm our expected bytes were correct.
oth.DelWithCPut(ctx, batch, kvKey, oldVal, traceKV)
} else if overwrite {
// If the new family contains a NULL value, then we must
// delete any pre-existing row.
if oth.IsSet() {
oth.DelWithCPut(ctx, batch, kvKey, oldVal, traceKV)
} else {
insertDelFn(ctx, batch, kvKey, traceKV)
}
insertDelFn(ctx, batch, kvKey, traceKV)
}
} else {
// We only output non-NULL values. Non-existent column keys are
Expand Down Expand Up @@ -197,6 +206,9 @@ func prepareInsertOrUpdateBatch(

// TODO(ssd): Here and below investigate reducing the number of
// allocations required to marshal the old value.
//
// If we are using OriginTimestamp ConditionalPuts, calculate the expected
// value.
var expBytes []byte
if oth.IsSet() && len(oldValues) > 0 {
var oldBytes []byte
Expand All @@ -214,14 +226,14 @@ func prepareInsertOrUpdateBatch(
}

if family.ID != 0 && len(rawValueBuf) == 0 {
if overwrite {
if oth.IsSet() {
// If using OriginTimestamp'd CPuts, we _always_ want to issue a Delete
// so that we can confirm our expected bytes were correct.
oth.DelWithCPut(ctx, batch, kvKey, expBytes, traceKV)
} else if overwrite {
// The family might have already existed but every column in it is being
// set to NULL, so delete it.
if oth.IsSet() {
oth.DelWithCPut(ctx, batch, kvKey, expBytes, traceKV)
} else {
insertDelFn(ctx, batch, kvKey, traceKV)
}
insertDelFn(ctx, batch, kvKey, traceKV)
}
} else {
// Copy the contents of rawValueBuf into the roachpb.Value. This is
Expand Down

0 comments on commit 55ccbff

Please sign in to comment.