Merge #64025 #64073

64025: kv: s/txn.Timestamp/txn.WriteTimestamp in comments r=andreimatei a=andreimatei

txn.Timestamp had been renamed to WriteTimestamp, but sufficiently many
comments were left dangling.

Release note: None

64073: sql: allow 'experimental_always' for 'vectorize' cluster setting r=yuzefovich a=yuzefovich

Originally, we disallowed it because it was hard to get out of that
state. Now we support `SET CLUSTER SETTING` in a more native manner in
the vectorized engine, so we can have an experimental value as the
cluster setting.

Also return the error that prompted us to wrap a row-execution processor
into the vectorized flow rather than the error that the wrapping is
prohibited.

Release note: None

Co-authored-by: Andrei Matei <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>

pkg/kv/kvserver/batcheval/cmd_push_txn.go

@@ -93,7 +93,7 @@ func declareKeysPushTransaction(
 // Push can proceed: the pushee's transaction record is modified and
 // rewritten, based on the value of args.PushType. If args.PushType
 // is PUSH_ABORT, txn.Status is set to ABORTED. If args.PushType is
-// PUSH_TIMESTAMP, txn.Timestamp is set to just after args.PushTo.
+// PUSH_TIMESTAMP, txn.WriteTimestamp is set to just after args.PushTo.
 //
 // If the pushee is aborted, its timestamp will be forwarded to match
 // its last client activity timestamp (i.e. last heartbeat), if available.

pkg/kv/kvserver/replica_evaluate.go

@@ -451,8 +451,9 @@ func evaluateBatch(
 		return nil, mergedResult, roachpb.NewErrorWithTxn(writeTooOldState.err, baHeader.Txn)
 	}
 
-	// The batch evaluation will not return an error (i.e. either everything went fine or
-	// we're deferring a WriteTooOldError by having bumped baHeader.Txn.Timestamp).
+	// The batch evaluation will not return an error (i.e. either everything went
+	// fine or we're deferring a WriteTooOldError by having bumped
+	// baHeader.Txn.WriteTimestamp).
 
 	// Update the batch response timestamp field to the timestamp at which the
 	// batch's reads were evaluated.

pkg/kv/txn.go

@@ -1148,7 +1148,7 @@ func (txn *Txn) SetFixedTimestamp(ctx context.Context, ts hlc.Timestamp) {
 //
 // The transaction's epoch is bumped, simulating to an extent what the
 // TxnCoordSender does on retriable errors. The transaction's timestamp is only
-// bumped to the extent that txn.ReadTimestamp is racheted up to txn.Timestamp.
+// bumped to the extent that txn.ReadTimestamp is racheted up to txn.WriteTimestamp.
 // TODO(andrei): This method should take in an up-to-date timestamp, but
 // unfortunately its callers don't currently have that handy.
 func (txn *Txn) GenerateForcedRetryableError(ctx context.Context, msg string) error {

pkg/roachpb/batch.go

@@ -41,7 +41,7 @@ func (h Header) WriteTimestamp() hlc.Timestamp {
 // SetActiveTimestamp sets the correct timestamp at which the request is to be
 // carried out. For transactional requests, ba.Timestamp must be zero initially
 // and it will be set to txn.ReadTimestamp (note though this mostly impacts
-// reads; writes use txn.Timestamp). For non-transactional requests, if no
+// reads; writes use txn.WriteTimestamp). For non-transactional requests, if no
 // timestamp is specified, nowFn is used to create and set one.
 func (ba *BatchRequest) SetActiveTimestamp(nowFn func() hlc.Timestamp) error {
 	if txn := ba.Txn; txn != nil {

pkg/sql/catalog/lease/lease.go

@@ -1270,7 +1270,7 @@ func makeNameCacheKey(parentID descpb.ID, parentSchemaID descpb.ID, name string)
 // API. The descriptor acquired needs to be released. A transaction
 // can use a descriptor as long as its timestamp is within the
 // validity window for the descriptor:
-// descriptor.ModificationTime <= txn.Timestamp < expirationTime
+// descriptor.ModificationTime <= txn.ReadTimestamp < expirationTime
 //
 // Exported only for testing.
 //

pkg/sql/colexec/colbuilder/execplan.go

@@ -710,7 +710,9 @@ func NewColOperator(
 	resultPreSpecPlanningStateShallowCopy := *r
 
 	if err = supportedNatively(spec); err != nil {
-		if err := canWrap(flowCtx.EvalCtx.SessionData.VectorizeMode, spec); err != nil {
+		if wrapErr := canWrap(flowCtx.EvalCtx.SessionData.VectorizeMode, spec); wrapErr != nil {
+			// Return the original error for why we don't support this spec
+			// natively since it is more interesting.
 			return r, err
 		}
 		if log.V(1) {

pkg/sql/exec_util.go

@@ -431,8 +431,9 @@ var VectorizeClusterMode = settings.RegisterEnumSetting(
 	"default vectorize mode",
 	"on",
 	map[int64]string{
-		int64(sessiondatapb.VectorizeOff): "off",
-		int64(sessiondatapb.VectorizeOn):  "on",
+		int64(sessiondatapb.VectorizeOff):                "off",
+		int64(sessiondatapb.VectorizeOn):                 "on",
+		int64(sessiondatapb.VectorizeExperimentalAlways): "experimental_always",
 	},
 )
 

pkg/storage/mvcc.go

@@ -1373,7 +1373,7 @@ func replayTransactionalWrite(
 // Note that, when writing transactionally, the txn's timestamps
 // dictate the timestamp of the operation, and the timestamp parameter
 // is redundant. Specifically, the intent is written at the txn's
-// provisional commit timestamp, txn.Timestamp, unless it is
+// provisional commit timestamp, txn.WriteTimestamp, unless it is
 // forwarded by an existing committed value above that timestamp.
 // However, reads (e.g., for a ConditionalPut) are performed at the
 // txn's read timestamp (txn.ReadTimestamp) to ensure that the
@@ -1472,9 +1472,9 @@ func mvccPutInternal(
 
 	// Determine the read and write timestamps for the write. For a
 	// non-transactional write, these will be identical. For a transactional
-	// write, we read at the transaction's original timestamp (forwarded by any
-	// refresh timestamp) but write intents at its provisional commit timestamp.
-	// See the comment on the txn.Timestamp field definition for rationale.
+	// write, we read at the transaction's read timestamp but write intents at its
+	// provisional commit timestamp. See the comment on the txn.WriteTimestamp field
+	// definition for rationale.
 	readTimestamp := timestamp
 	writeTimestamp := timestamp
 	if txn != nil {

pkg/storage/mvcc_test.go

@@ -2988,7 +2988,7 @@ func TestMVCCResolveIntentTxnTimestampMismatch(t *testing.T) {
 			tsEarly := txn.WriteTimestamp
 			txn.TxnMeta.WriteTimestamp.Forward(tsEarly.Add(10, 0))
 
-			// Write an intent which has txn.Timestamp > meta.timestamp.
+			// Write an intent which has txn.WriteTimestamp > meta.timestamp.
 			if err := MVCCPut(ctx, engine, nil, testKey1, tsEarly, value1, txn); err != nil {
 				t.Fatal(err)
 			}