Merge #148296

148296: sql: simplify the sequence cache size helper r=bghal a=bghal

Cleaned up the helper and added testing for clarity.

Epic: CRDB-42587

Release note: None


Co-authored-by: Brendan Gerrity <[email protected]>

Author: craig[bot]

pkg/sql/catalog/descpb/structured.go

@@ -296,24 +296,25 @@ func (opts *TableDescriptor_SequenceOpts) HasOwner() bool {
 	return !opts.SequenceOwner.Equal(TableDescriptor_SequenceOpts_SequenceOwner{})
 }
 
-// EffectiveCacheSize returns the CacheSize or NodeCacheSize field of a sequence option with
-// the exception that it will return 1 if both fields are set to 0.
-// A cache size of 1 indicates that there is no caching. A node cache size of 0 indicates there is no
-// node-level caching. The returned value
-// will always be greater than or equal to 1.
+// EffectiveCacheSize evaluates the cache size fields of a sequence option.
+// A cache size of 1 indicates that the cache is disabled.
+// The session cache size takes precedence: If set and enabled, it will be the one used.
 //
-// Prior to #51259, sequence caching was unimplemented and cache sizes were
-// left uninitialized (ie. to have a value of 0). If a sequence has a cache
-// size of 0, it should be treated in the same was as sequences with cache
-// sizes of 1.
+// Note: An unset cache size is considered disabled.
 func (opts *TableDescriptor_SequenceOpts) EffectiveCacheSize() int64 {
-	if opts.CacheSize == 0 && opts.NodeCacheSize == 0 {
-		return 1
+	switch sessionCacheSize := opts.CacheSize; sessionCacheSize {
+	case 0, 1:
+	default:
+		return sessionCacheSize
 	}
-	if opts.CacheSize == 1 && opts.NodeCacheSize != 0 {
-		return opts.NodeCacheSize
+
+	switch nodeCacheSize := opts.NodeCacheSize; nodeCacheSize {
+	case 0, 1:
+	default:
+		return nodeCacheSize
 	}
-	return opts.CacheSize
+
+	return 1 // caches disabled
 }
 
 // SafeValue implements the redact.SafeValue interface.

pkg/sql/catalog/descpb/structured_test.go

@@ -74,3 +74,72 @@ func TestColumnIDsPermutationOf(t *testing.T) {
 		})
 	}
 }
+
+func TestEffectiveCacheSize(t *testing.T) {
+	type testCase struct {
+		name           string
+		session, node  int64
+		expectedResult int64
+	}
+
+	testCases := []testCase{
+		{
+			name:           "both unset",
+			session:        0,
+			node:           0,
+			expectedResult: 1,
+		},
+		{
+			name:           "session unset",
+			session:        0,
+			node:           5,
+			expectedResult: 5,
+		},
+		{
+			name:           "node unset",
+			session:        5,
+			node:           0,
+			expectedResult: 5,
+		},
+
+		{
+			name:           "both disabled",
+			session:        0,
+			node:           0,
+			expectedResult: 1,
+		},
+		{
+			name:           "session disabled",
+			session:        1,
+			node:           5,
+			expectedResult: 5,
+		},
+		{
+			name:           "session disabled",
+			session:        5,
+			node:           1,
+			expectedResult: 5,
+		},
+
+		{
+			name:           "both enabled",
+			session:        11,
+			node:           13,
+			expectedResult: 11,
+		},
+	}
+
+	for _, tt := range testCases {
+		t.Run(tt.name, func(t *testing.T) {
+			opts := TableDescriptor_SequenceOpts{
+				CacheSize:     tt.session,
+				NodeCacheSize: tt.node,
+			}
+			result := opts.EffectiveCacheSize()
+			if result != tt.expectedResult {
+				t.Errorf("EffectiveCacheSize() %s: got %v, want %v", tt.name, result, tt.expectedResult)
+			}
+		})
+	}
+
+}