Merge #159065 #159378

159065: util/vector: remove unused error return argument from Encode r=yuzefovich a=yuzefovich

Epic: None
Release note: None

159378: sql/schemachanger: enforce LDR and schema_locked for truncate r=fqazi a=fqazi

Previously, the new implementation for TRUNCATE inside the declarative schema changer was not properly handling the schema_locked attribute. This patch modifies it to toggle schema_locked and enforce LDR.

Fixes: #159097

Release note (bug fix): TRUNCATE is not blocked when LDR is in use and does not behave correctly with schema_locked.

Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: Faizan Qazi <[email protected]>

Author: 3 people

pkg/sql/backfill/backfill.go

@@ -1551,10 +1551,7 @@ func (ib *IndexBackfiller) BuildIndexEntriesChunk(
 			}
 
 			vectorValue := tree.MustBeDPGVector(ib.rowVals[vectorIndexHelper.vectorOrd]).T
-			encodedVector, err := vector.Encode([]byte{}, vectorValue)
-			if err != nil {
-				return nil, nil, memUsedPerChunk, err
-			}
+			encodedVector := vector.Encode([]byte{}, vectorValue)
 			ib.vectorEncodingHelper.QuantizedVecs[indexID] = tree.NewDBytes(tree.DBytes(encodedVector))
 			ib.vectorEncodingHelper.PartitionKeys[indexID] = tree.NewDInt(tree.DInt(cspann.RootKey))
 		}

pkg/sql/rowenc/index_encoding_test.go

@@ -1305,8 +1305,7 @@ func TestVectorEncoding(t *testing.T) {
 	tableDesc := desctestutils.TestingGetPublicTableDescriptor(kvDB, codec, "defaultdb", "prefix_cols")
 
 	testVector := vector.T{1, 2, 4}
-	encodedVector, err := vecencoding.EncodeUnquantizerVector([]byte{}, testVector)
-	require.NoError(t, err)
+	encodedVector := vecencoding.EncodeUnquantizerVector([]byte{}, testVector)
 
 	vh := VectorIndexEncodingHelper{
 		PartitionKeys: make(map[descpb.IndexID]tree.Datum),
@@ -1395,8 +1394,7 @@ func TestVectorCompositeEncoding(t *testing.T) {
 	tableDesc := desctestutils.TestingGetPublicTableDescriptor(kvDB, codec, "defaultdb", "prefix_cols")
 
 	testVector := vector.T{1, 2, 4}
-	encodedVector, err := vecencoding.EncodeUnquantizerVector([]byte{}, testVector)
-	require.NoError(t, err)
+	encodedVector := vecencoding.EncodeUnquantizerVector([]byte{}, testVector)
 
 	vh := VectorIndexEncodingHelper{
 		PartitionKeys: make(map[descpb.IndexID]tree.Datum),

pkg/sql/rowenc/valueside/array.go

@@ -353,10 +353,7 @@ func encodeArrayElement(b []byte, d tree.Datum) ([]byte, error) {
 		}
 		return encoding.EncodeUntaggedBytesValue(b, encoded), nil
 	case *tree.DPGVector:
-		encoded, err := vector.Encode(nil, t.T)
-		if err != nil {
-			return nil, err
-		}
+		encoded := vector.Encode(nil, t.T)
 		return encoding.EncodeUntaggedBytesValue(b, encoded), nil
 	case *tree.DLTree:
 		return encoding.EncodeUntaggedLTreeValue(b, t.LTree), nil

pkg/sql/rowenc/valueside/encode.go

@@ -108,10 +108,7 @@ func EncodeWithScratch(
 		}
 		return encoding.EncodeTSVectorValue(appendTo, uint32(colID), scratch), scratch, nil
 	case *tree.DPGVector:
-		scratch, err = vector.Encode(scratch[:0], t.T)
-		if err != nil {
-			return nil, nil, err
-		}
+		scratch = vector.Encode(scratch[:0], t.T)
 		return encoding.EncodePGVectorValue(appendTo, uint32(colID), scratch), scratch, nil
 	case *tree.DArray:
 		scratch, err = encodeArray(t, scratch[:0])

pkg/sql/rowenc/valueside/legacy.go

@@ -168,10 +168,7 @@ func MarshalLegacy(colType *types.T, val tree.Datum) (roachpb.Value, error) {
 		}
 	case types.PGVectorFamily:
 		if v, ok := val.(*tree.DPGVector); ok {
-			data, err := vector.Encode(nil, v.T)
-			if err != nil {
-				return r, err
-			}
+			data := vector.Encode(nil, v.T)
 			r.SetBytes(data)
 			return r, nil
 		}

pkg/sql/schemachanger/scbuild/internal/scbuildstmt/truncate.go

@@ -97,6 +97,9 @@ func Truncate(b BuildCtx, stmt *tree.Truncate) {
 }
 
 func truncateTable(b BuildCtx, n *tree.Truncate, elts ElementResultSet) {
+	// Enforce schema_locked / LDR for truncate.
+	lockedCleanup := checkTableSchemaChangePrerequisites(b, elts, n)
+	defer lockedCleanup()
 	tbl := elts.FilterTable().MustGetOneElement()
 	// Fall back to legacy schema changer if we need to rewrite index references.
 	backRefs := b.BackReferences(tbl.TableID)

pkg/sql/schemachanger/testdata/end_to_end/truncate/truncate.explain

@@ -19,11 +19,14 @@ Schema change plan for TRUNCATE TABLE ‹defaultdb›.‹public›.‹t›;
  │         │    ├── ABSENT → PUBLIC    IndexColumn:{DescID: 104 (t), ColumnID: 2 (j), IndexID: 4 (t_j_key+)}
  │         │    ├── ABSENT → PUBLIC    IndexColumn:{DescID: 104 (t), ColumnID: 1 (i), IndexID: 4 (t_j_key+)}
  │         │    └── ABSENT → PUBLIC    IndexData:{DescID: 104 (t), IndexID: 4 (t_j_key+)}
+ │         ├── 1 element transitioning toward TRANSIENT_PUBLIC
+ │         │    └── PUBLIC → ABSENT    TableSchemaLocked:{DescID: 104 (t)}
  │         ├── 3 elements transitioning toward ABSENT
  │         │    ├── PUBLIC → VALIDATED PrimaryIndex:{DescID: 104 (t), IndexID: 1 (t_pkey-), ConstraintID: 2}
  │         │    ├── PUBLIC → ABSENT    IndexName:{DescID: 104 (t), Name: "t_pkey", IndexID: 1 (t_pkey-)}
  │         │    └── PUBLIC → VALIDATED SecondaryIndex:{DescID: 104 (t), IndexID: 2 (t_j_key-), ConstraintID: 1, RecreateSourceIndexID: 0, RecreateTargetIndexID: 0}
- │         └── 21 Mutation operations
+ │         └── 22 Mutation operations
+ │              ├── SetTableSchemaLocked {"TableID":104}
  │              ├── MakeAbsentIndexBackfilling {"Index":{"ConstraintID":3,"IndexID":3,"IsUnique":true,"SourceIndexID":1,"TableID":104}}
  │              ├── AddColumnToIndex {"ColumnID":1,"IndexID":3,"TableID":104}
  │              ├── AddColumnToIndex {"ColumnID":2,"IndexID":3,"Kind":2,"TableID":104}
@@ -58,6 +61,8 @@ Schema change plan for TRUNCATE TABLE ‹defaultdb›.‹public›.‹t›;
  │    │    │    ├── PUBLIC    → ABSENT IndexColumn:{DescID: 104 (t), ColumnID: 2 (j), IndexID: 4 (t_j_key+)}
  │    │    │    ├── PUBLIC    → ABSENT IndexColumn:{DescID: 104 (t), ColumnID: 1 (i), IndexID: 4 (t_j_key+)}
  │    │    │    └── PUBLIC    → ABSENT IndexData:{DescID: 104 (t), IndexID: 4 (t_j_key+)}
+ │    │    ├── 1 element transitioning toward TRANSIENT_PUBLIC
+ │    │    │    └── ABSENT    → PUBLIC TableSchemaLocked:{DescID: 104 (t)}
  │    │    ├── 3 elements transitioning toward ABSENT
  │    │    │    ├── VALIDATED → PUBLIC PrimaryIndex:{DescID: 104 (t), IndexID: 1 (t_pkey-), ConstraintID: 2}
  │    │    │    ├── ABSENT    → PUBLIC IndexName:{DescID: 104 (t), Name: "t_pkey", IndexID: 1 (t_pkey-)}
@@ -76,11 +81,14 @@ Schema change plan for TRUNCATE TABLE ‹defaultdb›.‹public›.‹t›;
  │         │    ├── ABSENT → PUBLIC    IndexColumn:{DescID: 104 (t), ColumnID: 2 (j), IndexID: 4 (t_j_key+)}
  │         │    ├── ABSENT → PUBLIC    IndexColumn:{DescID: 104 (t), ColumnID: 1 (i), IndexID: 4 (t_j_key+)}
  │         │    └── ABSENT → PUBLIC    IndexData:{DescID: 104 (t), IndexID: 4 (t_j_key+)}
+ │         ├── 1 element transitioning toward TRANSIENT_PUBLIC
+ │         │    └── PUBLIC → ABSENT    TableSchemaLocked:{DescID: 104 (t)}
  │         ├── 3 elements transitioning toward ABSENT
  │         │    ├── PUBLIC → VALIDATED PrimaryIndex:{DescID: 104 (t), IndexID: 1 (t_pkey-), ConstraintID: 2}
  │         │    ├── PUBLIC → ABSENT    IndexName:{DescID: 104 (t), Name: "t_pkey", IndexID: 1 (t_pkey-)}
  │         │    └── PUBLIC → VALIDATED SecondaryIndex:{DescID: 104 (t), IndexID: 2 (t_j_key-), ConstraintID: 1, RecreateSourceIndexID: 0, RecreateTargetIndexID: 0}
- │         └── 26 Mutation operations
+ │         └── 27 Mutation operations
+ │              ├── SetTableSchemaLocked {"TableID":104}
  │              ├── MakeAbsentIndexBackfilling {"Index":{"ConstraintID":3,"IndexID":3,"IsUnique":true,"SourceIndexID":1,"TableID":104}}
  │              ├── MaybeAddSplitForIndex {"CopyIndexID":1,"IndexID":3,"TableID":104}
  │              ├── AddColumnToIndex {"ColumnID":1,"IndexID":3,"TableID":104}
@@ -108,7 +116,7 @@ Schema change plan for TRUNCATE TABLE ‹defaultdb›.‹public›.‹t›;
  │              ├── SetJobStateOnDescriptor {"DescriptorID":104,"Initialize":true}
  │              └── CreateSchemaChangerJob {"NonCancelable":true,"RunningStatus":"Pending: Updatin..."}
  └── PostCommitNonRevertiblePhase
-      ├── Stage 1 of 2 in PostCommitNonRevertiblePhase
+      ├── Stage 1 of 3 in PostCommitNonRevertiblePhase
       │    ├── 7 elements transitioning toward ABSENT
       │    │    ├── PUBLIC    → ABSENT      IndexColumn:{DescID: 104 (t), ColumnID: 1 (i), IndexID: 1 (t_pkey-)}
       │    │    ├── PUBLIC    → ABSENT      IndexColumn:{DescID: 104 (t), ColumnID: 2 (j), IndexID: 1 (t_pkey-)}
@@ -127,16 +135,23 @@ Schema change plan for TRUNCATE TABLE ‹defaultdb›.‹public›.‹t›;
       │         ├── RemoveColumnFromIndex {"ColumnID":1,"IndexID":2,"Kind":1,"TableID":104}
       │         ├── SetJobStateOnDescriptor {"DescriptorID":104}
       │         └── UpdateSchemaChangerJob {"IsNonCancelable":true,"RunningStatus":"Pending: Updatin..."}
-      └── Stage 2 of 2 in PostCommitNonRevertiblePhase
-           ├── 4 elements transitioning toward ABSENT
-           │    ├── DELETE_ONLY → ABSENT PrimaryIndex:{DescID: 104 (t), IndexID: 1 (t_pkey-), ConstraintID: 2}
-           │    ├── PUBLIC      → ABSENT IndexData:{DescID: 104 (t), IndexID: 1 (t_pkey-)}
-           │    ├── DELETE_ONLY → ABSENT SecondaryIndex:{DescID: 104 (t), IndexID: 2 (t_j_key-), ConstraintID: 1, RecreateSourceIndexID: 0, RecreateTargetIndexID: 0}
-           │    └── PUBLIC      → ABSENT IndexData:{DescID: 104 (t), IndexID: 2 (t_j_key-)}
-           └── 6 Mutation operations
-                ├── MakeIndexAbsent {"IndexID":1,"TableID":104}
-                ├── CreateGCJobForIndex {"IndexID":1,"TableID":104}
-                ├── MakeIndexAbsent {"IndexID":2,"TableID":104}
-                ├── CreateGCJobForIndex {"IndexID":2,"TableID":104}
+      ├── Stage 2 of 3 in PostCommitNonRevertiblePhase
+      │    ├── 4 elements transitioning toward ABSENT
+      │    │    ├── DELETE_ONLY → ABSENT PrimaryIndex:{DescID: 104 (t), IndexID: 1 (t_pkey-), ConstraintID: 2}
+      │    │    ├── PUBLIC      → ABSENT IndexData:{DescID: 104 (t), IndexID: 1 (t_pkey-)}
+      │    │    ├── DELETE_ONLY → ABSENT SecondaryIndex:{DescID: 104 (t), IndexID: 2 (t_j_key-), ConstraintID: 1, RecreateSourceIndexID: 0, RecreateTargetIndexID: 0}
+      │    │    └── PUBLIC      → ABSENT IndexData:{DescID: 104 (t), IndexID: 2 (t_j_key-)}
+      │    └── 6 Mutation operations
+      │         ├── MakeIndexAbsent {"IndexID":1,"TableID":104}
+      │         ├── CreateGCJobForIndex {"IndexID":1,"TableID":104}
+      │         ├── MakeIndexAbsent {"IndexID":2,"TableID":104}
+      │         ├── CreateGCJobForIndex {"IndexID":2,"TableID":104}
+      │         ├── SetJobStateOnDescriptor {"DescriptorID":104}
+      │         └── UpdateSchemaChangerJob {"IsNonCancelable":true,"RunningStatus":"Pending: Updatin..."}
+      └── Stage 3 of 3 in PostCommitNonRevertiblePhase
+           ├── 1 element transitioning toward TRANSIENT_PUBLIC
+           │    └── ABSENT → TRANSIENT_PUBLIC TableSchemaLocked:{DescID: 104 (t)}
+           └── 3 Mutation operations
+                ├── SetTableSchemaLocked {"Locked":true,"TableID":104}
                 ├── RemoveJobStateFromDescriptor {"DescriptorID":104}
                 └── UpdateSchemaChangerJob {"IsNonCancelable":true,"RunningStatus":"all stages compl..."}

pkg/sql/schemachanger/testdata/end_to_end/truncate/truncate.explain_shape

@@ -6,4 +6,4 @@ INSERT INTO t VALUES (1, 1), (2, 2);
 EXPLAIN (DDL, SHAPE) TRUNCATE TABLE t;
 ----
 Schema change plan for TRUNCATE TABLE ‹defaultdb›.‹public›.‹t›;
- └── execute 3 system table mutations transactions
+ └── execute 4 system table mutations transactions

pkg/sql/schemachanger/testdata/end_to_end/truncate/truncate.side_effects

@@ -18,7 +18,7 @@ write *eventpb.TruncateTable to event log:
     tag: TRUNCATE
     user: root
   tableName: defaultdb.public.t
-## StatementPhase stage 1 of 1 with 21 MutationType ops
+## StatementPhase stage 1 of 1 with 22 MutationType ops
 upsert descriptor #104
   ...
      id: 104
@@ -117,7 +117,9 @@ upsert descriptor #104
        interleave: {}
        keyColumnDirections:
   ...
-     schemaLocked: true
+     replacementOf:
+       time: {}
+  -  schemaLocked: true
      unexposedParentSchemaId: 101
   -  version: "1"
   +  version: "2"
@@ -126,7 +128,7 @@ upsert descriptor #104
 ## PreCommitPhase stage 1 of 2 with 1 MutationType op
 undo all catalog changes within txn #1
 persist all catalog changes to storage
-## PreCommitPhase stage 2 of 2 with 26 MutationType ops
+## PreCommitPhase stage 2 of 2 with 27 MutationType ops
 upsert descriptor #104
   ...
      createAsOfTime:
@@ -257,7 +259,9 @@ upsert descriptor #104
        interleave: {}
        keyColumnDirections:
   ...
-     schemaLocked: true
+     replacementOf:
+       time: {}
+  -  schemaLocked: true
      unexposedParentSchemaId: 101
   -  version: "1"
   +  version: "2"
@@ -272,7 +276,7 @@ notified job registry to adopt jobs: [1]
 begin transaction #2
 commit transaction #2
 begin transaction #3
-## PostCommitNonRevertiblePhase stage 1 of 2 with 9 MutationType ops
+## PostCommitNonRevertiblePhase stage 1 of 3 with 9 MutationType ops
 upsert descriptor #104
   ...
          version: 4
@@ -296,48 +300,16 @@ upsert descriptor #104
      name: t
      nextColumnId: 3
   ...
-     schemaLocked: true
+       time: {}
      unexposedParentSchemaId: 101
   -  version: "2"
   +  version: "3"
 persist all catalog changes to storage
-update progress of schema change job #1: "Pending: Updating schema metadata (4 operations) — PostCommitNonRevertible phase (stage 2 of 2)."
+update progress of schema change job #1: "Pending: Updating schema metadata (4 operations) — PostCommitNonRevertible phase (stage 2 of 3)."
 commit transaction #3
 begin transaction #4
-## PostCommitNonRevertiblePhase stage 2 of 2 with 6 MutationType ops
+## PostCommitNonRevertiblePhase stage 2 of 3 with 6 MutationType ops
 upsert descriptor #104
-  ...
-     createAsOfTime:
-       wallTime: "1640995200000000000"
-  -  declarativeSchemaChangerState:
-  -    authorization:
-  -      userName: root
-  -    currentStatuses: <redacted>
-  -    jobId: "1"
-  -    nameMapping:
-  -      columns:
-  -        "1": i
-  -        "2": j
-  -        "4294967292": crdb_internal_origin_timestamp
-  -        "4294967293": crdb_internal_origin_id
-  -        "4294967294": tableoid
-  -        "4294967295": crdb_internal_mvcc_timestamp
-  -      families:
-  -        "0": primary
-  -      id: 104
-  -      indexes:
-  -        "3": t_pkey
-  -        "4": t_j_key
-  -      name: t
-  -    relevantStatements:
-  -    - statement:
-  -        redactedStatement: TRUNCATE TABLE ‹defaultdb›.‹public›.‹t›
-  -        statement: TRUNCATE TABLE t
-  -        statementTag: TRUNCATE
-  -    targetRanks: <redacted>
-  -    targets: <redacted>
-     families:
-     - columnIds:
   ...
        version: 4
      modificationTime: {}
@@ -397,19 +369,64 @@ upsert descriptor #104
      name: t
      nextColumnId: 3
   ...
-     schemaLocked: true
+       time: {}
      unexposedParentSchemaId: 101
   -  version: "3"
   +  version: "4"
 persist all catalog changes to storage
 create job #2 (non-cancelable: true): "GC for TRUNCATE TABLE defaultdb.public.t"
   descriptor IDs: [104]
+update progress of schema change job #1: "Pending: Updating schema metadata (1 operation) — PostCommitNonRevertible phase (stage 3 of 3)."
+commit transaction #4
+notified job registry to adopt jobs: [2]
+begin transaction #5
+## PostCommitNonRevertiblePhase stage 3 of 3 with 3 MutationType ops
+upsert descriptor #104
+  ...
+     createAsOfTime:
+       wallTime: "1640995200000000000"
+  -  declarativeSchemaChangerState:
+  -    authorization:
+  -      userName: root
+  -    currentStatuses: <redacted>
+  -    jobId: "1"
+  -    nameMapping:
+  -      columns:
+  -        "1": i
+  -        "2": j
+  -        "4294967292": crdb_internal_origin_timestamp
+  -        "4294967293": crdb_internal_origin_id
+  -        "4294967294": tableoid
+  -        "4294967295": crdb_internal_mvcc_timestamp
+  -      families:
+  -        "0": primary
+  -      id: 104
+  -      indexes:
+  -        "3": t_pkey
+  -        "4": t_j_key
+  -      name: t
+  -    relevantStatements:
+  -    - statement:
+  -        redactedStatement: TRUNCATE TABLE ‹defaultdb›.‹public›.‹t›
+  -        statement: TRUNCATE TABLE t
+  -        statementTag: TRUNCATE
+  -    targetRanks: <redacted>
+  -    targets: <redacted>
+     families:
+     - columnIds:
+  ...
+     replacementOf:
+       time: {}
+  +  schemaLocked: true
+     unexposedParentSchemaId: 101
+  -  version: "4"
+  +  version: "5"
+persist all catalog changes to storage
 update progress of schema change job #1: "all stages completed"
 set schema change job #1 to non-cancellable
 updated schema change job #1 descriptor IDs to []
 write *eventpb.FinishSchemaChange to event log:
   sc:
     descriptorId: 104
-commit transaction #4
-notified job registry to adopt jobs: [2]
+commit transaction #5
 # end PostCommitPhase

pkg/sql/vecindex/vecencoding/encoding.go

@@ -199,9 +199,7 @@ func EncodeMetadataValue(metadata cspann.PartitionMetadata) []byte {
 	buf = encoding.EncodeUint64Ascending(buf, uint64(metadata.StateDetails.Source))
 	buf = encoding.EncodeUntaggedTimeValue(buf, metadata.StateDetails.Timestamp)
 
-	// vector.Encode never returns a non-nil error, so suppress return value.
-	buf, _ = vector.Encode(buf, metadata.Centroid)
-	return buf
+	return vector.Encode(buf, metadata.Centroid)
 }
 
 // EncodeRaBitQVector encodes a RaBitQ vector into the given byte slice.
@@ -222,7 +220,7 @@ func EncodeRaBitQVectorFromSet(
 
 // EncodeUnquantizerVector encodes an Unquantizer vector into the given byte
 // slice.
-func EncodeUnquantizerVector(appendTo []byte, v vector.T) ([]byte, error) {
+func EncodeUnquantizerVector(appendTo []byte, v vector.T) []byte {
 	// For backwards compatibility, encode a zero float32. Previously, the
 	// distance of the vector to the centroid was encoded, but that is no longer
 	// necessary.