Merge #83331 #83685

83331: docs: add volatility to built-in functions table r=mgartner a=mgartner

#### sql: make "leakproof" one word

In Postgres's documentation "leakproof" is one, non-hyphenated word.
This commit updates all occurrences of the word to a single word.

Release note: None

#### docs: add volatility to built-in functions table

Release note: None


83685: kvstreamer: remove some redundant function calls r=yuzefovich a=yuzefovich

This commit refactors the logic of processing the BatchResponse in order
to avoid getting the "inner" request for each response - it is
sufficient for us to do a type switch on the response itself.

I did run the microbenchmarks, and they showed no difference, but
I believe this change should be beneficial.

Release note: None

Co-authored-by: Marcus Gartner <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>

pkg/cmd/docgen/funcs.go

@@ -215,7 +215,14 @@ func generateFunctions(from []string, categorize bool) []byte {
 				info := md.RenderToString([]byte(fn.Info))
 				extra = fmt.Sprintf("<span class=\"funcdesc\">%s</span>", info)
 			}
-			s := fmt.Sprintf("<tr><td><a name=\"%s\"></a><code>%s(%s) &rarr; %s</code></td><td>%s</td></tr>", name, name, linkArguments(args), linkArguments(ret), extra)
+			s := fmt.Sprintf("<tr><td><a name=\"%s\"></a><code>%s(%s) &rarr; %s</code></td><td>%s</td><td>%s</td></tr>",
+				name,
+				name,
+				linkArguments(args),
+				linkArguments(ret),
+				extra,
+				fn.Volatility.TitleString(),
+			)
 			functions[cat] = append(functions[cat], s)
 		}
 	}
@@ -239,7 +246,7 @@ func generateFunctions(from []string, categorize bool) []byte {
 		if categorize {
 			fmt.Fprintf(b, "### %s functions\n\n", cat)
 		}
-		b.WriteString("<table>\n<thead><tr><th>Function &rarr; Returns</th><th>Description</th></tr></thead>\n")
+		b.WriteString("<table>\n<thead><tr><th>Function &rarr; Returns</th><th>Description</th><th>Volatility</th></tr></thead>\n")
 		b.WriteString("<tbody>\n")
 		b.WriteString(strings.Join(functions[cat], "\n"))
 		b.WriteString("</tbody>\n</table>\n\n")

pkg/kv/kvclient/kvstreamer/streamer.go

@@ -221,6 +221,7 @@ type Streamer struct {
 	hints         Hints
 	maxKeysPerRow int32
 	budget        *budget
+	keyLocking    lock.Strength
 
 	streamerStatistics
 
@@ -354,6 +355,7 @@ func NewStreamer(
 	limitBytes int64,
 	acc *mon.BoundAccount,
 	batchRequestsIssued *int64,
+	keyLocking lock.Strength,
 ) *Streamer {
 	if txn.Type() != kv.LeafTxn {
 		panic(errors.AssertionFailedf("RootTxn is given to the Streamer"))
@@ -362,6 +364,7 @@ func NewStreamer(
 		distSender: distSender,
 		stopper:    stopper,
 		budget:     newBudget(acc, limitBytes),
+		keyLocking: keyLocking,
 	}
 	if batchRequestsIssued == nil {
 		batchRequestsIssued = new(int64)
@@ -1428,9 +1431,9 @@ func processSingleRangeResults(
 			subRequestIdx = req.subRequestIdx[i]
 		}
 		reply := resp.GetInner()
-		switch req.reqs[i].GetInner().(type) {
-		case *roachpb.GetRequest:
-			get := reply.(*roachpb.GetResponse)
+		switch response := reply.(type) {
+		case *roachpb.GetResponse:
+			get := response
 			if get.ResumeSpan != nil {
 				// This Get wasn't completed.
 				continue
@@ -1454,8 +1457,8 @@ func processSingleRangeResults(
 			}
 			s.results.addLocked(result)
 
-		case *roachpb.ScanRequest:
-			scan := reply.(*roachpb.ScanResponse)
+		case *roachpb.ScanResponse:
+			scan := response
 			if len(scan.BatchResponses) == 0 && scan.ResumeSpan != nil {
 				// Only the first part of the conditional is true whenever we
 				// received an empty response for the Scan request (i.e. there
@@ -1546,9 +1549,9 @@ func buildResumeSingleRangeBatch(
 	for i, resp := range br.Responses {
 		position := req.positions[i]
 		reply := resp.GetInner()
-		switch origRequest := req.reqs[i].GetInner().(type) {
-		case *roachpb.GetRequest:
-			get := reply.(*roachpb.GetResponse)
+		switch response := reply.(type) {
+		case *roachpb.GetResponse:
+			get := response
 			if get.ResumeSpan == nil {
 				emptyResponse = false
 				continue
@@ -1557,10 +1560,10 @@ func buildResumeSingleRangeBatch(
 			// can just reuse the original request since it hasn't been
 			// modified which is also asserted below).
 			if buildutil.CrdbTestBuild {
-				if !get.ResumeSpan.Equal(origRequest.Span()) {
+				if origSpan := req.reqs[i].GetInner().Header().Span(); !get.ResumeSpan.Equal(origSpan) {
 					panic(errors.AssertionFailedf(
 						"unexpectedly the ResumeSpan %s on the GetResponse is different from the original span %s",
-						get.ResumeSpan, origRequest.Span(),
+						get.ResumeSpan, origSpan,
 					))
 				}
 			}
@@ -1574,8 +1577,8 @@ func buildResumeSingleRangeBatch(
 			}
 			resumeReqIdx++
 
-		case *roachpb.ScanRequest:
-			scan := reply.(*roachpb.ScanResponse)
+		case *roachpb.ScanResponse:
+			scan := response
 			if scan.ResumeSpan == nil {
 				emptyResponse = false
 				continue
@@ -1586,7 +1589,7 @@ func buildResumeSingleRangeBatch(
 			scans = scans[1:]
 			newScan.req.SetSpan(*scan.ResumeSpan)
 			newScan.req.ScanFormat = roachpb.BATCH_RESPONSE
-			newScan.req.KeyLocking = origRequest.KeyLocking
+			newScan.req.KeyLocking = s.keyLocking
 			newScan.union.Scan = &newScan.req
 			resumeReq.reqs[resumeReqIdx].Value = &newScan.union
 			resumeReq.positions = append(resumeReq.positions, position)

pkg/kv/kvclient/kvstreamer/streamer_test.go

@@ -47,6 +47,7 @@ func getStreamer(
 		limitBytes,
 		acc,
 		nil, /* batchRequestsIssued */
+		lock.None,
 	)
 }
 
@@ -98,6 +99,7 @@ func TestStreamerLimitations(t *testing.T) {
 				math.MaxInt64, /* limitBytes */
 				nil,           /* acc */
 				nil,           /* batchRequestsIssued */
+				lock.None,
 			)
 		})
 	})

pkg/sql/opt/exec/execbuilder/mutation.go

@@ -920,7 +920,7 @@ func (b *Builder) canAutoCommit(rel memo.RelExpr) bool {
 		// Allow Project on top, as long as the expressions are not side-effecting.
 		proj := rel.(*memo.ProjectExpr)
 		for i := 0; i < len(proj.Projections); i++ {
-			if !proj.Projections[i].ScalarProps().VolatilitySet.IsLeakProof() {
+			if !proj.Projections[i].ScalarProps().VolatilitySet.IsLeakproof() {
 				return false
 			}
 		}

pkg/sql/opt/memo/expr_format.go

@@ -740,7 +740,7 @@ func (f *ExprFmtCtx) formatRelational(e RelExpr, tp treeprinter.Node) {
 			f.Buffer.WriteString(name)
 		}
 
-		if !relational.VolatilitySet.IsLeakProof() {
+		if !relational.VolatilitySet.IsLeakproof() {
 			writeFlag(relational.VolatilitySet.String())
 		}
 		if relational.CanMutate {
@@ -1023,7 +1023,7 @@ func (f *ExprFmtCtx) scalarPropsStrings(scalar opt.ScalarExpr) []string {
 			if !scalarProps.OuterCols.Empty() {
 				emitProp("outer=%s", scalarProps.OuterCols)
 			}
-			if !scalarProps.VolatilitySet.IsLeakProof() {
+			if !scalarProps.VolatilitySet.IsLeakproof() {
 				emitProp(scalarProps.VolatilitySet.String())
 			}
 			if scalarProps.HasCorrelatedSubquery {

pkg/sql/opt/norm/decorrelate_funcs.go

@@ -87,13 +87,13 @@ func (c *CustomFuncs) deriveHasHoistableSubquery(scalar opt.ScalarExpr) bool {
 				// Determine whether this is the Else child.
 				if child == t.OrElse {
 					memo.BuildSharedProps(child, &sharedProps, c.f.evalCtx)
-					hasHoistableSubquery = sharedProps.VolatilitySet.IsLeakProof()
+					hasHoistableSubquery = sharedProps.VolatilitySet.IsLeakproof()
 				}
 
 			case *memo.WhenExpr:
 				if child == t.Value {
 					memo.BuildSharedProps(child, &sharedProps, c.f.evalCtx)
-					hasHoistableSubquery = sharedProps.VolatilitySet.IsLeakProof()
+					hasHoistableSubquery = sharedProps.VolatilitySet.IsLeakproof()
 				}
 
 			case *memo.IfErrExpr:
@@ -102,7 +102,7 @@ func (c *CustomFuncs) deriveHasHoistableSubquery(scalar opt.ScalarExpr) bool {
 				// it's at position 1.
 				if i == 1 {
 					memo.BuildSharedProps(child, &sharedProps, c.f.evalCtx)
-					hasHoistableSubquery = sharedProps.VolatilitySet.IsLeakProof()
+					hasHoistableSubquery = sharedProps.VolatilitySet.IsLeakproof()
 				}
 			}
 

pkg/sql/opt/norm/factory.go

@@ -353,10 +353,10 @@ func (f *Factory) onConstructRelational(rel memo.RelExpr) memo.RelExpr {
 	// the logical properties of the group in question.
 	if rel.Op() != opt.ValuesOp {
 		relational := rel.Relational()
-		// We can do this if we only contain leak-proof operators. As an example of
+		// We can do this if we only contain leakproof operators. As an example of
 		// an immutable operator that should not be folded: a Limit on top of an
 		// empty input has to error out if the limit turns out to be negative.
-		if relational.Cardinality.IsZero() && relational.VolatilitySet.IsLeakProof() {
+		if relational.Cardinality.IsZero() && relational.VolatilitySet.IsLeakproof() {
 			if f.matchedRule == nil || f.matchedRule(opt.SimplifyZeroCardinalityGroup) {
 				values := f.funcs.ConstructEmptyValues(relational.OutputCols)
 				if f.appliedRule != nil {

pkg/sql/opt/props/volatility.go

@@ -28,7 +28,7 @@ import "github.com/cockroachdb/cockroach/pkg/sql/sem/volatility"
 // The optimizer makes *only* the following side-effect related guarantees:
 //
 //   1. CASE/IF branches are only evaluated if the branch condition is true or
-//      if all operators are LeakProof. Therefore, the following is guaranteed
+//      if all operators are Leakproof. Therefore, the following is guaranteed
 //      to never raise a divide by zero error, regardless of how cleverly the
 //      optimizer rewrites the expression:
 //
@@ -110,10 +110,10 @@ func (vs *VolatilitySet) UnionWith(other VolatilitySet) {
 	*vs = *vs | other
 }
 
-// IsLeakProof returns true if the set is empty or only contains
-// LeakProof.
-func (vs VolatilitySet) IsLeakProof() bool {
-	return vs == 0 || vs == volatilityBit(volatility.LeakProof)
+// IsLeakproof returns true if the set is empty or only contains
+// Leakproof.
+func (vs VolatilitySet) IsLeakproof() bool {
+	return vs == 0 || vs == volatilityBit(volatility.Leakproof)
 }
 
 // HasStable returns true if the set contains Stable.
@@ -127,21 +127,21 @@ func (vs VolatilitySet) HasVolatile() bool {
 }
 
 func (vs VolatilitySet) String() string {
-	// The only properties we care about are IsLeakProof(), HasStable() and
+	// The only properties we care about are IsLeakproof(), HasStable() and
 	// HasVolatile(). We print one of the strings below:
 	//
-	//    String            | IsLeakProof | HasStable | HasVolatile
+	//    String            | IsLeakproof | HasStable | HasVolatile
 	//   -------------------+-------------+-----------+-------------
-	//    "leak-proof"      | true        | false     | false
+	//    "leakproof"       | true        | false     | false
 	//    "immutable"       | false       | false     | false
 	//    "stable"          | false       | true      | false
 	//    "volatile"        | false       | false     | true
 	//    "stable+volatile" | false       | true      | true
 	//
 	// These are the only valid combinations for these properties.
 	//
-	if vs.IsLeakProof() {
-		return "leak-proof"
+	if vs.IsLeakproof() {
+		return "leakproof"
 	}
 	hasStable := vs.HasStable()
 	hasVolatile := vs.HasVolatile()

pkg/sql/opt/props/volatility_test.go

@@ -20,18 +20,18 @@ import (
 func TestVolatilitySet(t *testing.T) {
 	var v VolatilitySet
 
-	check := func(str string, isLeakProof, hasStable, hasVolatile bool) {
+	check := func(str string, isLeakproof, hasStable, hasVolatile bool) {
 		t.Helper()
 
 		require.Equal(t, v.String(), str)
-		require.Equal(t, v.IsLeakProof(), isLeakProof)
+		require.Equal(t, v.IsLeakproof(), isLeakproof)
 		require.Equal(t, v.HasStable(), hasStable)
 		require.Equal(t, v.HasVolatile(), hasVolatile)
 	}
-	check("leak-proof", true, false, false)
+	check("leakproof", true, false, false)
 
-	v.Add(volatility.LeakProof)
-	check("leak-proof", true, false, false)
+	v.Add(volatility.Leakproof)
+	check("leakproof", true, false, false)
 
 	v.AddImmutable()
 	check("immutable", false, false, false)

pkg/sql/row/kv_fetcher.go

@@ -149,6 +149,7 @@ func NewStreamingKVFetcher(
 		streamerBudgetLimit,
 		streamerBudgetAcc,
 		&batchRequestsIssued,
+		getKeyLockingStrength(lockStrength),
 	)
 	mode := kvstreamer.OutOfOrder
 	if maintainOrdering {

pkg/sql/sem/builtins/builtins.go

@@ -8184,7 +8184,7 @@ func hashBuiltin(newHash func() hash.Hash, info string) builtinDefinition {
 				return tree.NewDString(fmt.Sprintf("%x", h.Sum(nil))), nil
 			},
 			Info:       info,
-			Volatility: volatility.LeakProof,
+			Volatility: volatility.Leakproof,
 		},
 		tree.Overload{
 			Types:      tree.VariadicType{VarType: types.Bytes},
@@ -8197,7 +8197,7 @@ func hashBuiltin(newHash func() hash.Hash, info string) builtinDefinition {
 				return tree.NewDString(fmt.Sprintf("%x", h.Sum(nil))), nil
 			},
 			Info:       info,
-			Volatility: volatility.LeakProof,
+			Volatility: volatility.Leakproof,
 		},
 	)
 }
@@ -8215,7 +8215,7 @@ func hash32Builtin(newHash func() hash.Hash32, info string) builtinDefinition {
 				return tree.NewDInt(tree.DInt(h.Sum32())), nil
 			},
 			Info:       info,
-			Volatility: volatility.LeakProof,
+			Volatility: volatility.Leakproof,
 		},
 		tree.Overload{
 			Types:      tree.VariadicType{VarType: types.Bytes},
@@ -8228,7 +8228,7 @@ func hash32Builtin(newHash func() hash.Hash32, info string) builtinDefinition {
 				return tree.NewDInt(tree.DInt(h.Sum32())), nil
 			},
 			Info:       info,
-			Volatility: volatility.LeakProof,
+			Volatility: volatility.Leakproof,
 		},
 	)
 }
@@ -8246,7 +8246,7 @@ func hash64Builtin(newHash func() hash.Hash64, info string) builtinDefinition {
 				return tree.NewDInt(tree.DInt(h.Sum64())), nil
 			},
 			Info:       info,
-			Volatility: volatility.LeakProof,
+			Volatility: volatility.Leakproof,
 		},
 		tree.Overload{
 			Types:      tree.VariadicType{VarType: types.Bytes},
@@ -8259,7 +8259,7 @@ func hash64Builtin(newHash func() hash.Hash64, info string) builtinDefinition {
 				return tree.NewDInt(tree.DInt(h.Sum64())), nil
 			},
 			Info:       info,
-			Volatility: volatility.LeakProof,
+			Volatility: volatility.Leakproof,
 		},
 	)
 }

pkg/sql/sem/builtins/pgcrypto_builtins.go

@@ -84,7 +84,7 @@ var pgcryptoBuiltins = map[string]builtinDefinition{
 			},
 			Info: "Computes a binary hash of the given `data`. `type` is the algorithm " +
 				"to use (md5, sha1, sha224, sha256, sha384, or sha512).",
-			Volatility: volatility.LeakProof,
+			Volatility: volatility.Leakproof,
 		},
 		tree.Overload{
 			Types:      tree.ArgTypes{{"data", types.Bytes}, {"type", types.String}},
@@ -161,7 +161,7 @@ var pgcryptoBuiltins = map[string]builtinDefinition{
 				return tree.NewDBytes(tree.DBytes(h.Sum(nil))), nil
 			},
 			Info:       "Calculates hashed MAC for `data` with key `key`. `type` is the same as in `digest()`.",
-			Volatility: volatility.LeakProof,
+			Volatility: volatility.Leakproof,
 		},
 		tree.Overload{
 			Types:      tree.ArgTypes{{"data", types.Bytes}, {"key", types.Bytes}, {"type", types.String}},

pkg/sql/sem/cast/cast.go

@@ -324,7 +324,7 @@ func LookupCastVolatility(from, to *types.T) (_ volatility.V, ok bool) {
 		if len(fromTypes) != len(toTypes) {
 			return 0, false
 		}
-		maxVolatility := volatility.LeakProof
+		maxVolatility := volatility.Leakproof
 		for i := range fromTypes {
 			v, lookupOk := LookupCastVolatility(fromTypes[i], toTypes[i])
 			if !lookupOk {

pkg/sql/sem/cast/cast_map.go

@@ -466,7 +466,7 @@ var castMap = map[oid.Oid]map[oid.Oid]Cast{
 	},
 	oid.T_name: {
 		oid.T_bpchar:  {MaxContext: ContextAssignment, origin: ContextOriginPgCast, Volatility: volatility.Immutable},
-		oid.T_text:    {MaxContext: ContextImplicit, origin: ContextOriginPgCast, Volatility: volatility.LeakProof},
+		oid.T_text:    {MaxContext: ContextImplicit, origin: ContextOriginPgCast, Volatility: volatility.Leakproof},
 		oid.T_varchar: {MaxContext: ContextAssignment, origin: ContextOriginPgCast, Volatility: volatility.Immutable},
 		// Automatic I/O conversions to string types.
 		oid.T_char: {MaxContext: ContextAssignment, origin: ContextOriginAutomaticIOConversion, Volatility: volatility.Immutable},
@@ -687,7 +687,7 @@ var castMap = map[oid.Oid]map[oid.Oid]Cast{
 		oid.T_bpchar:      {MaxContext: ContextImplicit, origin: ContextOriginPgCast, Volatility: volatility.Immutable},
 		oid.T_char:        {MaxContext: ContextAssignment, origin: ContextOriginPgCast, Volatility: volatility.Immutable},
 		oidext.T_geometry: {MaxContext: ContextImplicit, origin: ContextOriginPgCast, Volatility: volatility.Immutable},
-		oid.T_name:        {MaxContext: ContextImplicit, origin: ContextOriginPgCast, Volatility: volatility.LeakProof},
+		oid.T_name:        {MaxContext: ContextImplicit, origin: ContextOriginPgCast, Volatility: volatility.Leakproof},
 		oid.T_regclass:    {MaxContext: ContextImplicit, origin: ContextOriginPgCast, Volatility: volatility.Stable},
 		// We include a TEXT->TEXT entry to mimic the VARCHAR->VARCHAR entry
 		// that is included in the pg_cast table. Postgres doesn't include a

pkg/sql/sem/cast/cast_test.go

@@ -193,7 +193,7 @@ func TestTupleCastVolatility(t *testing.T) {
 		{
 			from: nil,
 			to:   nil,
-			exp:  "leak-proof",
+			exp:  "leakproof",
 		},
 		{
 			from: nil,