Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
70840: jobs: fix a flakey test by adopting SucceedsSoon r=postamar a=ajwerner

The test failed because the operation stopped retrying. The defaults
for the retrier were less than the duration of a KV lease.

Fixes #70664.

Release note: None

70944: roachprod: upgrade Azure Ubuntu image to 20.04 r=jlinder a=rail

Previously, currently used Ubuntu 18.04 doesn't support `systemd-run
--same-dir`, which is used by some roachprod scripts. Additionally, GCE
and AWS already use Ubuntu 20.04 based images for roachprod.

Updating the base image to Ubuntu 20.04 fixes the issue above and aligns
the version with other cloud providers.

Release note: None

71097: sql: fix FK check bug in ALTER COLUMN TYPE r=postamar a=postamar

This commit fixes an implementation bug when checking that the table's
foreign keys didn't hold a reference to the altered column.

Fixes #71089.

Release note (bug fix): fixes a bug which caused ALTER COLUMN TYPE
statements to fail when they shouldn't have.

Co-authored-by: Andrew Werner <[email protected]>
Co-authored-by: Rail Aliiev <[email protected]>
Co-authored-by: Marius Posta <[email protected]>
  • Loading branch information
4 people committed Oct 5, 2021
4 parents 92f153c + 2e653ad + 9bf0789 + 9f6cc9c commit d9eecf8
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 17 deletions.
12 changes: 9 additions & 3 deletions pkg/cmd/roachprod/vm/azure/azure.go
Original file line number Diff line number Diff line change
Expand Up @@ -514,11 +514,17 @@ func (p *Provider) createVM(
VMSize: compute.VirtualMachineSizeTypes(p.opts.machineType),
},
StorageProfile: &compute.StorageProfile{
// From https://discourse.ubuntu.com/t/find-ubuntu-images-on-microsoft-azure/18918
// You can find available versions by running the following command:
// az vm image list --all --publisher Canonical
// To get the latest 20.04 version:
// az vm image list --all --publisher Canonical | \
// jq '[.[] | select(.sku=="20_04-lts")] | max_by(.version)'
ImageReference: &compute.ImageReference{
Publisher: to.StringPtr("Canonical"),
Offer: to.StringPtr("UbuntuServer"),
Sku: to.StringPtr("18.04-LTS"),
Version: to.StringPtr("latest"),
Offer: to.StringPtr("0001-com-ubuntu-server-focal"),
Sku: to.StringPtr("20_04-lts"),
Version: to.StringPtr("20.04.202109080"),
},
OsDisk: &compute.OSDisk{
CreateOption: compute.DiskCreateOptionTypesFromImage,
Expand Down
1 change: 0 additions & 1 deletion pkg/jobs/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@ go_test(
"//pkg/util/metric",
"//pkg/util/protoutil",
"//pkg/util/randutil",
"//pkg/util/retry",
"//pkg/util/stop",
"//pkg/util/syncutil",
"//pkg/util/timeutil",
Expand Down
12 changes: 2 additions & 10 deletions pkg/jobs/jobs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/protoutil"
"github.com/cockroachdb/cockroach/pkg/util/retry"
"github.com/cockroachdb/cockroach/pkg/util/syncutil"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
"github.com/cockroachdb/cockroach/pkg/util/tracing"
Expand Down Expand Up @@ -341,12 +340,7 @@ func (rts *registryTestSuite) tearDown() {

func (rts *registryTestSuite) check(t *testing.T, expectedStatus jobs.Status) {
t.Helper()
opts := retry.Options{
InitialBackoff: 5 * time.Millisecond,
MaxBackoff: time.Second,
Multiplier: 2,
}
if err := retry.WithMaxAttempts(rts.ctx, opts, 10, func() error {
testutils.SucceedsSoon(t, func() error {
rts.mu.Lock()
defer rts.mu.Unlock()
if diff := cmp.Diff(rts.mu.e, rts.mu.a); diff != "" {
Expand All @@ -363,9 +357,7 @@ func (rts *registryTestSuite) check(t *testing.T, expectedStatus jobs.Status) {
return errors.Errorf("expected job status: %s but got: %s", expectedStatus, st)
}
return nil
}); err != nil {
t.Fatal(err)
}
})
}

func TestRegistryLifecycle(t *testing.T) {
Expand Down
15 changes: 12 additions & 3 deletions pkg/sql/alter_column_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,9 +203,18 @@ func alterColumnTypeGeneral(
// Disallow ALTER COLUMN TYPE general for columns that have a foreign key
// constraint.
for _, fk := range tableDesc.AllActiveAndInactiveForeignKeys() {
for _, id := range append(fk.OriginColumnIDs, fk.ReferencedColumnIDs...) {
if col.GetID() == id {
return colWithConstraintNotSupportedErr
if fk.OriginTableID == tableDesc.GetID() {
for _, id := range fk.OriginColumnIDs {
if col.GetID() == id {
return colWithConstraintNotSupportedErr
}
}
}
if fk.ReferencedTableID == tableDesc.GetID() {
for _, id := range fk.ReferencedColumnIDs {
if col.GetID() == id {
return colWithConstraintNotSupportedErr
}
}
}
}
Expand Down
9 changes: 9 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/alter_column_type
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,15 @@ SELECT x FROM t29 ORDER BY x
2
3

# Regression 71089, check that foreign key references are checked properly.

statement ok
CREATE TABLE parent_71089 (id INT PRIMARY KEY);
CREATE TABLE child_71089 (a INT, b INT REFERENCES parent_71089 (id) NOT NULL)

statement ok
ALTER TABLE child_71089 ALTER COLUMN a TYPE FLOAT;

# ColumnConversionValidate should error out if the conversion is not valid.
# STRING -> BYTES is a ColumnConversionValidate type conversion, it should
# try the conversion and error out if the cast cannot be applied.
Expand Down

0 comments on commit d9eecf8

Please sign in to comment.