Merge cockroachdb#80459 cockroachdb#81060

80459: sql/schemachanger: Implement comment on in declarative sc r=chengxiong-ruan a=chengxiong-ruan Split out from cockroachdb#78025 This commit implements COMMENT ON with the new schema changer framework. Release note: None 81060: publish-{provisional,}-artifacts: create arm64 builds for non-release r=rail a=rickystewart These new binaries will only be built for the non-release case. Release note: None Co-authored-by: Chengxiong Ruan <[email protected]> Co-authored-by: Ricky Stewart <[email protected]>
nicktrav · May 5, 2022 · da04bc2 · da04bc2
3 parents 51c7020 + 006a842 + d1ce315
commit da04bc2
Show file tree

Hide file tree

Showing 45 changed files with 1,649 additions and 594 deletions.
diff --git a/pkg/cmd/publish-artifacts/main.go b/pkg/cmd/publish-artifacts/main.go
@@ -109,7 +109,7 @@ type runFlags struct {
 }
 
 func run(svc s3putter, flags runFlags, execFn release.ExecFn) {
-	for _, platform := range []release.Platform{release.PlatformLinux, release.PlatformMacOS, release.PlatformWindows} {
+	for _, platform := range []release.Platform{release.PlatformLinux, release.PlatformLinuxArm, release.PlatformMacOS, release.PlatformWindows} {
 		var o opts
 		o.Platform = platform
 		o.ReleaseVersionStrs = []string{flags.sha}

diff --git a/pkg/cmd/publish-artifacts/main_test.go b/pkg/cmd/publish-artifacts/main_test.go
@@ -148,6 +148,8 @@ func TestPublish(t *testing.T) {
 				"env=[] args=bazel info bazel-bin -c opt --config=ci --config=force_build_cdeps --config=with_ui --config=crosslinuxbase",
 				"env=[MALLOC_CONF=prof:true] args=./cockroach.linux-2.6.32-gnu-amd64 version",
 				"env=[] args=ldd ./cockroach.linux-2.6.32-gnu-amd64",
+				"env=[] args=bazel build //pkg/cmd/cockroach //c-deps:libgeos //pkg/cmd/cockroach-sql '--workspace_status_command=./build/bazelutil/stamp.sh aarch64-unknown-linux-gnu official-binary' -c opt --config=ci --config=force_build_cdeps --config=with_ui --config=crosslinuxarmbase",
+				"env=[] args=bazel info bazel-bin -c opt --config=ci --config=force_build_cdeps --config=with_ui --config=crosslinuxarmbase",
 				"env=[] args=bazel build //pkg/cmd/cockroach //c-deps:libgeos //pkg/cmd/cockroach-sql " +
 					"'--workspace_status_command=./build/bazelutil/stamp.sh x86_64-apple-darwin19 official-binary' -c opt --config=ci --config=force_build_cdeps --config=with_ui --config=crossmacosbase",
 				"env=[] args=bazel info bazel-bin -c opt --config=ci --config=force_build_cdeps --config=with_ui --config=crossmacosbase",
@@ -179,6 +181,26 @@ func TestPublish(t *testing.T) {
 					"'--workspace_status_command=./build/bazelutil/stamp.sh x86_64-pc-linux-gnu official-binary' " +
 					"-c opt --config=ci --config=force_build_cdeps --config=with_ui --config=crosslinuxbase",
 				"s3://cockroach/cockroach/lib/libgeos_c.linux-gnu-amd64.so.LATEST/no-cache REDIRECT /cockroach/lib/libgeos_c.linux-gnu-amd64.1234567890abcdef.so",
+				"s3://cockroach//cockroach/cockroach.linux-gnu-aarch64.1234567890abcdef CONTENTS env=[] args=bazel build " +
+					"//pkg/cmd/cockroach //c-deps:libgeos //pkg/cmd/cockroach-sql " +
+					"'--workspace_status_command=./build/bazelutil/stamp.sh aarch64-unknown-linux-gnu official-binary' " +
+					"-c opt --config=ci --config=force_build_cdeps --config=with_ui --config=crosslinuxarmbase",
+				"s3://cockroach/cockroach/cockroach.linux-gnu-aarch64.LATEST/no-cache REDIRECT /cockroach/cockroach.linux-gnu-aarch64.1234567890abcdef",
+				"s3://cockroach//cockroach/cockroach-sql.linux-gnu-aarch64.1234567890abcdef CONTENTS env=[] args=bazel build " +
+					"//pkg/cmd/cockroach //c-deps:libgeos //pkg/cmd/cockroach-sql " +
+					"'--workspace_status_command=./build/bazelutil/stamp.sh aarch64-unknown-linux-gnu official-binary' " +
+					"-c opt --config=ci --config=force_build_cdeps --config=with_ui --config=crosslinuxarmbase",
+				"s3://cockroach/cockroach/cockroach-sql.linux-gnu-aarch64.LATEST/no-cache REDIRECT /cockroach/cockroach-sql.linux-gnu-aarch64.1234567890abcdef",
+				"s3://cockroach//cockroach/lib/libgeos.linux-gnu-aarch64.1234567890abcdef.so CONTENTS env=[] args=bazel build " +
+					"//pkg/cmd/cockroach //c-deps:libgeos //pkg/cmd/cockroach-sql " +
+					"'--workspace_status_command=./build/bazelutil/stamp.sh aarch64-unknown-linux-gnu official-binary' " +
+					"-c opt --config=ci --config=force_build_cdeps --config=with_ui --config=crosslinuxarmbase",
+				"s3://cockroach/cockroach/lib/libgeos.linux-gnu-aarch64.so.LATEST/no-cache REDIRECT /cockroach/lib/libgeos.linux-gnu-aarch64.1234567890abcdef.so",
+				"s3://cockroach//cockroach/lib/libgeos_c.linux-gnu-aarch64.1234567890abcdef.so CONTENTS env=[] args=bazel build " +
+					"//pkg/cmd/cockroach //c-deps:libgeos //pkg/cmd/cockroach-sql " +
+					"'--workspace_status_command=./build/bazelutil/stamp.sh aarch64-unknown-linux-gnu official-binary' " +
+					"-c opt --config=ci --config=force_build_cdeps --config=with_ui --config=crosslinuxarmbase",
+				"s3://cockroach/cockroach/lib/libgeos_c.linux-gnu-aarch64.so.LATEST/no-cache REDIRECT /cockroach/lib/libgeos_c.linux-gnu-aarch64.1234567890abcdef.so",
 				"s3://cockroach//cockroach/cockroach.darwin-amd64.1234567890abcdef CONTENTS env=[] args=bazel build " +
 					"//pkg/cmd/cockroach //c-deps:libgeos //pkg/cmd/cockroach-sql " +
 					"'--workspace_status_command=./build/bazelutil/stamp.sh x86_64-apple-darwin19 official-binary' " +

diff --git a/pkg/cmd/publish-provisional-artifacts/main.go b/pkg/cmd/publish-provisional-artifacts/main.go
@@ -164,8 +164,12 @@ func run(svc s3I, flags runFlags, execFn release.ExecFn) {
 	}
 	log.Printf("Using S3 bucket: %s", bucketName)
 
+	platforms := []release.Platform{release.PlatformLinux, release.PlatformMacOS, release.PlatformWindows}
+	if !flags.isRelease {
+		platforms = append(platforms, release.PlatformLinuxArm)
+	}
 	var cockroachBuildOpts []opts
-	for _, platform := range []release.Platform{release.PlatformLinux, release.PlatformMacOS, release.PlatformWindows} {
+	for _, platform := range platforms {
 		var o opts
 		o.Platform = platform
 		o.PkgDir = flags.pkgDir

diff --git a/pkg/cmd/publish-provisional-artifacts/main_test.go b/pkg/cmd/publish-provisional-artifacts/main_test.go
@@ -206,6 +206,9 @@ func TestProvisional(t *testing.T) {
 				"env=[] args=bazel build //pkg/cmd/cockroach //c-deps:libgeos //pkg/cmd/cockroach-sql " +
 					"'--workspace_status_command=./build/bazelutil/stamp.sh x86_64-w64-mingw32 official-binary' -c opt --config=ci --config=force_build_cdeps --config=with_ui --config=crosswindowsbase",
 				"env=[] args=bazel info bazel-bin -c opt --config=ci --config=force_build_cdeps --config=with_ui --config=crosswindowsbase",
+				"env=[] args=bazel build //pkg/cmd/cockroach //c-deps:libgeos //pkg/cmd/cockroach-sql " +
+					"'--workspace_status_command=./build/bazelutil/stamp.sh aarch64-unknown-linux-gnu official-binary' -c opt --config=ci --config=force_build_cdeps --config=with_ui --config=crosslinuxarmbase",
+				"env=[] args=bazel info bazel-bin -c opt --config=ci --config=force_build_cdeps --config=with_ui --config=crosslinuxarmbase",
 			},
 			expectedGets: nil,
 			expectedPuts: []string{
@@ -258,6 +261,22 @@ func TestProvisional(t *testing.T) {
 					"dll CONTENTS env=[] args=bazel build //pkg/cmd/cockroach //c-deps:libgeos //pkg/cmd/cockroach-sql " +
 					"'--workspace_status_command=./build/bazelutil/stamp.sh x86_64-w64-mingw32 official-binary' -c opt --config=ci --config=force_build_cdeps --config=with_ui --config=crosswindowsbase",
 				"s3://cockroach/cockroach/lib/libgeos_c.windows-amd64.dll.LATEST/no-cache REDIRECT /cockroach/lib/libgeos_c.windows-amd64.00SHA00.dll",
+				"s3://cockroach//cockroach/cockroach.linux-gnu-aarch64.00SHA00 " +
+					"CONTENTS env=[] args=bazel build //pkg/cmd/cockroach //c-deps:libgeos //pkg/cmd/cockroach-sql " +
+					"'--workspace_status_command=./build/bazelutil/stamp." +
+					"sh aarch64-unknown-linux-gnu official-binary' -c opt --config=ci --config=force_build_cdeps --config=with_ui --config=crosslinuxarmbase",
+				"s3://cockroach/cockroach/cockroach.linux-gnu-aarch64.LATEST/no-cache " +
+					"REDIRECT /cockroach/cockroach.linux-gnu-aarch64.00SHA00",
+				"s3://cockroach//cockroach/cockroach-sql.linux-gnu-aarch64.00SHA00 CONTENTS env=[] args=bazel build //pkg/cmd/cockroach //c-deps:libgeos //pkg/cmd/cockroach-sql '--workspace_status_command=./build/bazelutil/stamp.sh aarch64-unknown-linux-gnu official-binary' -c opt --config=ci --config=force_build_cdeps --config=with_ui --config=crosslinuxarmbase",
+				"s3://cockroach/cockroach/cockroach-sql.linux-gnu-aarch64.LATEST/no-cache REDIRECT /cockroach/cockroach-sql.linux-gnu-aarch64.00SHA00",
+				"s3://cockroach//cockroach/lib/libgeos.linux-gnu-aarch64.00SHA00." +
+					"so CONTENTS env=[] args=bazel build //pkg/cmd/cockroach //c-deps:libgeos //pkg/cmd/cockroach-sql " +
+					"'--workspace_status_command=./build/bazelutil/stamp.sh aarch64-unknown-linux-gnu official-binary' -c opt --config=ci --config=force_build_cdeps --config=with_ui --config=crosslinuxarmbase",
+				"s3://cockroach/cockroach/lib/libgeos.linux-gnu-aarch64.so.LATEST/no-cache REDIRECT /cockroach/lib/libgeos.linux-gnu-aarch64.00SHA00.so",
+				"s3://cockroach//cockroach/lib/libgeos_c.linux-gnu-aarch64.00SHA00." +
+					"so CONTENTS env=[] args=bazel build //pkg/cmd/cockroach //c-deps:libgeos //pkg/cmd/cockroach-sql " +
+					"'--workspace_status_command=./build/bazelutil/stamp.sh aarch64-unknown-linux-gnu official-binary' -c opt --config=ci --config=force_build_cdeps --config=with_ui --config=crosslinuxarmbase",
+				"s3://cockroach/cockroach/lib/libgeos_c.linux-gnu-aarch64.so.LATEST/no-cache REDIRECT /cockroach/lib/libgeos_c.linux-gnu-aarch64.00SHA00.so",
 			},
 		},
 	}

diff --git a/pkg/sql/alter_table.go b/pkg/sql/alter_table.go
@@ -1370,7 +1370,7 @@ func insertJSONStatistic(
 }
 
 func (p *planner) removeColumnComment(
-	ctx context.Context, tableID descpb.ID, columnID descpb.ColumnID,
+	ctx context.Context, tableID descpb.ID, pgAttributeNum descpb.PGAttributeNum,
 ) error {
 	_, err := p.ExtendedEvalContext().ExecCfg.InternalExecutor.ExecEx(
 		ctx,
@@ -1380,7 +1380,7 @@ func (p *planner) removeColumnComment(
 		"DELETE FROM system.comments WHERE type=$1 AND object_id=$2 AND sub_id=$3",
 		keys.ColumnCommentType,
 		tableID,
-		columnID)
+		pgAttributeNum)
 
 	return err
 }
@@ -1783,7 +1783,7 @@ func dropColumnImpl(
 		}
 	}
 
-	if err := params.p.removeColumnComment(params.ctx, tableDesc.ID, colToDrop.GetID()); err != nil {
+	if err := params.p.removeColumnComment(params.ctx, tableDesc.ID, colToDrop.GetPGAttributeNum()); err != nil {
 		return nil, err
 	}
 

diff --git a/pkg/sql/catalog/descpb/constraint.go b/pkg/sql/catalog/descpb/constraint.go
@@ -106,3 +106,22 @@ type ConstraintDetail struct {
 	// Only populated for Check Constraints.
 	CheckConstraint *TableDescriptor_CheckConstraint
 }
+
+// GetConstraintName retrieves correct constraint name base on the constraint
+// type.
+func (c *ConstraintDetail) GetConstraintName() string {
+	switch c.Kind {
+	case ConstraintTypePK:
+		return c.Index.Name
+	case ConstraintTypeUnique:
+		if c.Index != nil {
+			return c.Index.Name
+		}
+		return c.UniqueWithoutIndexConstraint.Name
+	case ConstraintTypeFK:
+		return c.FK.Name
+	case ConstraintTypeCheck:
+		return c.CheckConstraint.Name
+	}
+	return ""
+}
diff --git a/pkg/sql/catalog/descriptor.go b/pkg/sql/catalog/descriptor.go
@@ -628,6 +628,8 @@ type TableDescriptor interface {
 	GetConstraintInfoWithLookup(fn TableLookupFn) (map[string]descpb.ConstraintDetail, error)
 	// GetConstraintInfo returns a summary of all constraints on the table.
 	GetConstraintInfo() (map[string]descpb.ConstraintDetail, error)
+	// FindConstraintWithID returns a constraint given a constraint id.
+	FindConstraintWithID(id descpb.ConstraintID) (*descpb.ConstraintDetail, error)
 
 	// GetUniqueWithoutIndexConstraints returns all the unique constraints defined
 	// on this table that are not enforced by an index.

diff --git a/pkg/sql/catalog/tabledesc/table.go b/pkg/sql/catalog/tabledesc/table.go
@@ -300,6 +300,22 @@ func (desc *wrapper) GetConstraintInfo() (map[string]descpb.ConstraintDetail, er
 	return desc.collectConstraintInfo(nil)
 }
 
+// FindConstraintWithID implements the TableDescriptor interface.
+func (desc *wrapper) FindConstraintWithID(
+	id descpb.ConstraintID,
+) (*descpb.ConstraintDetail, error) {
+	constraintInfo, err := desc.GetConstraintInfo()
+	if err != nil {
+		return nil, err
+	}
+	for _, info := range constraintInfo {
+		if info.ConstraintID == id {
+			return &info, nil
+		}
+	}
+	return nil, nil
+}
+
 // GetConstraintInfoWithLookup implements the TableDescriptor interface.
 func (desc *wrapper) GetConstraintInfoWithLookup(
 	tableLookup catalog.TableLookupFn,