Merge cockroachdb#83727 cockroachdb#83737 cockroachdb#83802 cockroach…

…db#83824

83727: cmd/dev: add generate parser subcommand r=irfansharif a=ajwerner

This commit separates out the generated files needed for the parser into a
separate genbzl target and then add a reference to that target from dev.

You can now run `./dev generate parser` to regenerate the parser files.

Release note: None

83737: iterutil: More ergonomic interface r=postamar a=andrewbaptist

Replace the Done call in iterutil with Map. This removes a lot of
duplicate code and simplifies the way it is used. There are no
functional changes as a result of this.

Release note: None

83802: sql/schemachanger: add enum type values as an element r=fqazi a=fqazi

Previously, our decomposition of enum types only included
an element for the type itself. This was inadequate because
for multi-region support we need to be able to iterate over
the values for an enum. This patch adds further decomposition
for enums.

Release note: None

83824: sql/catalog/lease: check for cancelled ctx when draining lease Manager r=aayushshah15 a=aayushshah15

Previously, we'd retry indefinitely when `leaseMgr.SetDraining` was
called with a cancelled context. This showed up in cockroachdb#83060 as one of the
reasons a graceful node shutdown could stall forever.

See cockroachdb#83060 (comment) for more details.
Relates to cockroachdb#83060

Release note (bug fix): A bug causing a graceful node shutdown to stall
forever has been fixed.

Co-authored-by: Andrew Werner <[email protected]>
Co-authored-by: Andrew Baptist <[email protected]>
Co-authored-by: Faizan Qazi <[email protected]>
Co-authored-by: Aayush Shah <[email protected]>

dev

@@ -3,7 +3,7 @@
 set -euo pipefail
 
 # Bump this counter to force rebuilding `dev` on all machines.
-DEV_VERSION=38
+DEV_VERSION=39
 
 THIS_DIR=$(cd "$(dirname "$0")" && pwd)
 BINARY_DIR=$THIS_DIR/bin/dev-versions

pkg/ccl/changefeedccl/cdcevent/event.go

@@ -136,10 +136,7 @@ func (r Row) forEachDatum(fn DatumFn, colIndexes []int) error {
 		}
 
 		if err := fn(encDatum.Datum, col); err != nil {
-			if iterutil.Done(err) {
-				return nil
-			}
-			return err
+			return iterutil.Map(err)
 		}
 	}
 	return nil
@@ -149,10 +146,7 @@ func (r Row) forEachDatum(fn DatumFn, colIndexes []int) error {
 func (r Row) forEachColumn(fn ColumnFn, colIndexes []int) error {
 	for _, colIdx := range colIndexes {
 		if err := fn(r.cols[colIdx]); err != nil {
-			if iterutil.Done(err) {
-				return nil
-			}
-			return err
+			return iterutil.Map(err)
 		}
 	}
 	return nil

pkg/ccl/schemachangerccl/testdata/decomp/multiregion

@@ -964,3 +964,18 @@ ElementState:
     objectId: 105
     parentSchemaId: 106
   Status: PUBLIC
+- EnumTypeValue:
+    logicalRepresentation: us-east1
+    physicalRepresentation: QA==
+    typeId: 105
+  Status: PUBLIC
+- EnumTypeValue:
+    logicalRepresentation: us-east2
+    physicalRepresentation: gA==
+    typeId: 105
+  Status: PUBLIC
+- EnumTypeValue:
+    logicalRepresentation: us-east3
+    physicalRepresentation: wA==
+    typeId: 105
+  Status: PUBLIC

pkg/ccl/schemachangerccl/testdata/end_to_end/drop_database_multiregion_primary_region

@@ -227,6 +227,9 @@ upsert descriptor #105
   +    - ABSENT
   +    - DROPPED
   +    - ABSENT
+  +    - ABSENT
+  +    - ABSENT
+  +    - ABSENT
   +    jobId: "1"
   +    relevantStatements:
   +    - statement:
@@ -241,6 +244,9 @@ upsert descriptor #105
   +    - 19
   +    - 20
   +    - 21
+  +    - 22
+  +    - 23
+  +    - 24
   +    targets:
   +    - elementProto:
   +        namespace:
@@ -297,6 +303,33 @@ upsert descriptor #105
   +        subWorkId: 1
   +      targetStatus: ABSENT
   +    - elementProto:
+  +        enumTypeValue:
+  +          logicalRepresentation: us-east1
+  +          physicalRepresentation: QA==
+  +          typeId: 105
+  +      metadata:
+  +        sourceElementId: 3
+  +        subWorkId: 1
+  +      targetStatus: ABSENT
+  +    - elementProto:
+  +        enumTypeValue:
+  +          logicalRepresentation: us-east2
+  +          physicalRepresentation: gA==
+  +          typeId: 105
+  +      metadata:
+  +        sourceElementId: 3
+  +        subWorkId: 1
+  +      targetStatus: ABSENT
+  +    - elementProto:
+  +        enumTypeValue:
+  +          logicalRepresentation: us-east3
+  +          physicalRepresentation: wA==
+  +          typeId: 105
+  +      metadata:
+  +        sourceElementId: 3
+  +        subWorkId: 1
+  +      targetStatus: ABSENT
+  +    - elementProto:
   +        objectParent:
   +          objectId: 105
   +          parentSchemaId: 106
@@ -432,13 +465,13 @@ upsert descriptor #107
   +        statement: DROP DATABASE multi_region_test_db CASCADE
   +        statementTag: DROP DATABASE
   +    targetRanks:
-  +    - 22
-  +    - 23
-  +    - 24
   +    - 25
   +    - 26
   +    - 27
   +    - 28
+  +    - 29
+  +    - 30
+  +    - 31
   +    targets:
   +    - elementProto:
   +        namespace:
@@ -556,9 +589,6 @@ upsert descriptor #108
   +        statement: DROP DATABASE multi_region_test_db CASCADE
   +        statementTag: DROP DATABASE
   +    targetRanks:
-  +    - 29
-  +    - 30
-  +    - 31
   +    - 32
   +    - 33
   +    - 34
@@ -576,6 +606,9 @@ upsert descriptor #108
   +    - 46
   +    - 47
   +    - 48
+  +    - 49
+  +    - 50
+  +    - 51
   +    targets:
   +    - elementProto:
   +        namespace:
@@ -830,9 +863,6 @@ upsert descriptor #108
   -        statement: DROP DATABASE multi_region_test_db CASCADE
   -        statementTag: DROP DATABASE
   -    targetRanks:
-  -    - 29
-  -    - 30
-  -    - 31
   -    - 32
   -    - 33
   -    - 34
@@ -850,6 +880,9 @@ upsert descriptor #108
   -    - 46
   -    - 47
   -    - 48
+  -    - 49
+  -    - 50
+  -    - 51
   -    targets:
   -    - elementProto:
   -        namespace:

pkg/cmd/dev/generate.go

@@ -40,6 +40,7 @@ func makeGenerateCmd(runE func(cmd *cobra.Command, args []string) error) *cobra.
         dev generate go        # generates go code (execgen, stringer, protobufs, etc.), plus everything 'cgo' generates
         dev generate go_nocgo  # generates go code (execgen, stringer, protobufs, etc.)
         dev generate protobuf  # *.pb.go files (subset of 'dev generate go')
+        dev generate parser    # sql.go and parser dependencies (subset of 'dev generate go')
 `,
 		Args: cobra.MinimumNArgs(0),
 		// TODO(irfansharif): Errors but default just eaten up. Let's wrap these
@@ -65,6 +66,7 @@ func (d *dev) generate(cmd *cobra.Command, targets []string) error {
 		"go":       d.generateGo,
 		"go_nocgo": d.generateGoNoCgo,
 		"protobuf": d.generateProtobuf,
+		"parser":   d.generateParser,
 	}
 
 	if len(targets) == 0 {
@@ -175,6 +177,10 @@ func (d *dev) generateProtobuf(cmd *cobra.Command) error {
 	return d.generateTarget(cmd.Context(), "//pkg/gen:go_proto")
 }
 
+func (d *dev) generateParser(cmd *cobra.Command) error {
+	return d.generateTarget(cmd.Context(), "//pkg/gen:parser")
+}
+
 func (d *dev) generateTarget(ctx context.Context, target string) error {
 	if err := d.exec.CommandContextInheritingStdStreams(
 		ctx, "bazel", "run", target,

pkg/gen/BUILD.bazel

@@ -1,4 +1,4 @@
-load(":gen.bzl", "bindata", "docs", "execgen", "gen", "go_proto", "gomock", "misc", "optgen", "stringer")
+load(":gen.bzl", "bindata", "docs", "execgen", "gen", "go_proto", "gomock", "misc", "optgen", "parser", "stringer")
 
 bindata()
 
@@ -16,6 +16,8 @@ misc()
 
 docs()
 
+parser()
+
 gen(
     name = "gen",
     srcs = [
@@ -33,6 +35,7 @@ gen(
         ":gomock",
         ":misc",
         ":optgen",
+        ":parser",
         ":stringer",
     ],
 )

pkg/gen/gen.bzl

@@ -27,6 +27,7 @@ load(":misc.bzl", "MISC_SRCS")
 load(":optgen.bzl", "OPTGEN_SRCS")
 load(":protobuf.bzl", "PROTOBUF_SRCS")
 load(":stringer.bzl", "STRINGER_SRCS")
+load(":parser.bzl", "PARSER_SRCS")
 
 # GeneratedFileInfo provides two pieces of information to the _hoist_files
 # rule. It provides the set of files to be hoisted via the generated_files
@@ -281,5 +282,11 @@ def bindata():
     srcs = BINDATA_SRCS,
   )
 
+def parser():
+  _hoist_no_prefix(
+    name = "parser",
+    srcs = PARSER_SRCS,
+  )
+
 def gen(name, srcs):
   _hoist_files(name = name, data = srcs, tags = ["no-remote-exec"])

pkg/gen/genbzl/targets.go

@@ -59,6 +59,10 @@ in $og - filter(".*:.*(-gen|gen-).*", $og)`,
 kind("generated file", //docs/...:*)
   - labels("outs", //docs/generated/sql/bnf:svg)`,
 	},
+	{
+		target: "parser",
+		query:  `labels("outs", kind("genrule rule", //pkg/sql/sem/... + //pkg/sql/parser/... + //pkg/sql/lexbase/...))`,
+	},
 	{
 		target: "excluded",
 		query: `
@@ -88,6 +92,7 @@ kind("generated file", {{ .All }}) - (
   + {{ template "optgen" $ }}
   + {{ template "docs" $ }}
   + {{ template "excluded" $ }}
+  + {{ template "parser" $ }}
 )`,
 	},
 }

pkg/gen/misc.bzl

@@ -9,19 +9,11 @@ MISC_SRCS = [
   "//pkg/roachprod/vm/aws:terraform/main.tf",
   "//pkg/spanconfig/spanconfigstore:entry_interval_btree.go",
   "//pkg/spanconfig/spanconfigstore:entry_interval_btree_test.go",
-  "//pkg/sql/lexbase:keywords.go",
-  "//pkg/sql/lexbase:reserved_keywords.go",
-  "//pkg/sql/lexbase:tokens.go",
-  "//pkg/sql/parser:help_messages.go",
-  "//pkg/sql/parser:helpmap_test.go",
-  "//pkg/sql/parser:sql.go",
   "//pkg/sql/schemachanger/scop:backfill_visitor_generated.go",
   "//pkg/sql/schemachanger/scop:mutation_visitor_generated.go",
   "//pkg/sql/schemachanger/scop:validation_visitor_generated.go",
   "//pkg/sql/schemachanger/scpb:elements_generated.go",
   "//pkg/sql/schemachanger/scpb:uml/table.puml",
-  "//pkg/sql/sem/tree:eval_expr_generated.go",
-  "//pkg/sql/sem/tree:eval_op_generated.go",
   "//pkg/sql:txnstatetransitions_diagram.gv",
   "//pkg/sql:txnstatetransitions_report.txt",
   "//pkg/util/interval/generic:example_interval_btree.go",

pkg/gen/parser.bzl

@@ -0,0 +1,15 @@
+# Generated by genbzl
+
+PARSER_SRCS = [
+  "//pkg/sql/lexbase:keywords.go",
+  "//pkg/sql/lexbase:reserved_keywords.go",
+  "//pkg/sql/lexbase:tokens.go",
+  "//pkg/sql/parser:help_messages.go",
+  "//pkg/sql/parser:helpmap_test.go",
+  "//pkg/sql/parser:sql.go",
+  "//pkg/sql/sem/tree:createtypevariety_string.go",
+  "//pkg/sql/sem/tree:eval_expr_generated.go",
+  "//pkg/sql/sem/tree:eval_op_generated.go",
+  "//pkg/sql/sem/tree:statementreturntype_string.go",
+  "//pkg/sql/sem/tree:statementtype_string.go",
+]

pkg/kv/kvclient/kvcoord/dist_sender_rangefeed.go

@@ -210,11 +210,7 @@ func (ds *DistSender) ForEachActiveRangeFeed(fn ActiveRangeFeedIterFn) (iterErr
 		return iterErr == nil
 	})
 
-	if iterutil.Done(iterErr) {
-		iterErr = nil // Early termination is fine.
-	}
-
-	return
+	return iterutil.Map(iterErr)
 }
 
 // activeRangeFeed is a thread safe PartialRangeFeed.

pkg/kv/kvserver/replica_raftstorage.go

@@ -260,10 +260,7 @@ func iterateEntries(
 			return errors.Wrap(err, "unable to unmarshal raft Entry")
 		}
 		if err := f(ent); err != nil {
-			if iterutil.Done(err) {
-				return nil
-			}
-			return err
+			return iterutil.Map(err)
 		}
 	}
 }

pkg/kv/kvserver/store.go

@@ -1716,18 +1716,15 @@ func IterateIDPrefixKeys(
 		}
 
 		if err := f(rangeID); err != nil {
-			if iterutil.Done(err) {
-				return nil
-			}
-			return err
+			return iterutil.Map(err)
 		}
 		rangeID++
 	}
 }
 
 // IterateRangeDescriptorsFromDisk calls the provided function with each
 // descriptor from the provided Engine. The return values of this method and fn
-// have semantics similar to engine.MVCCIterate.
+// have semantics similar to storage.MVCCIterate.
 func IterateRangeDescriptorsFromDisk(
 	ctx context.Context, reader storage.Reader, fn func(desc roachpb.RangeDescriptor) error,
 ) error {
@@ -1755,7 +1752,11 @@ func IterateRangeDescriptorsFromDisk(
 			return err
 		}
 		matchCount++
-		if err := fn(desc); iterutil.Done(err) {
+		err = fn(desc)
+		if err == nil {
+			return nil
+		}
+		if iterutil.Map(err) == nil {
 			return iterutil.StopIteration()
 		}
 		return err

pkg/kv/kvserver/store_replica_btree.go

@@ -210,10 +210,7 @@ func (b *storeReplicaBTree) descendLessOrEqual(
 		err = visitor(ctx, itemToReplicaOrPlaceholder(it))
 		return err == nil // more?
 	})
-	if iterutil.Done(err) {
-		return nil
-	}
-	return err
+	return iterutil.Map(err)
 }
 
 func (b *storeReplicaBTree) ascendRange(
@@ -226,10 +223,7 @@ func (b *storeReplicaBTree) ascendRange(
 		err = visitor(ctx, itemToReplicaOrPlaceholder(it))
 		return err == nil // more
 	})
-	if iterutil.Done(err) {
-		return nil
-	}
-	return err
+	return iterutil.Map(err)
 }
 
 func (b *storeReplicaBTree) bt() *btree.BTree {

pkg/kv/util.go

@@ -36,7 +36,7 @@ func marshalKey(k interface{}) (roachpb.Key, error) {
 	case string:
 		return roachpb.Key(t), nil
 	case []byte:
-		return roachpb.Key(t), nil
+		return t, nil
 	}
 	return nil, fmt.Errorf("unable to marshal key: %T %q", k, k)
 }
@@ -74,7 +74,7 @@ func marshalValue(v interface{}) (roachpb.Value, error) {
 		return r, err
 
 	case roachpb.Key:
-		r.SetBytes([]byte(t))
+		r.SetBytes(t)
 		return r, nil
 
 	case time.Time: