Skip to content

Commit

Permalink
Merge #76235 #76275
Browse files Browse the repository at this point in the history
76235: sql: delete TestTruncateWhileColumnBackfill r=ajwerner a=ajwerner

It's been skipped since 2020. We have no intention to fix it.

Closes #43990

Release note: None

76275: scpb: minor style fixes to elements.proto r=ajwerner a=otan

Makes this a bit more "standard" with other protobufs.

* Fixed an accidental uppercase for certain characters.
* Moved camelCase to snake_case for many fields
* Removed some redundant gogoproto.customnames
* Fixed some spacing issues

Release note: None



Co-authored-by: Andrew Werner <awerner32@gmail.com>
Co-authored-by: Oliver Tan <otan@cockroachlabs.com>
  • Loading branch information
3 people committed Feb 9, 2022
3 parents 456ff4c + c8ac9e0 + b54dbbd commit c089297
Show file tree
Hide file tree
Showing 12 changed files with 102 additions and 228 deletions.
126 changes: 0 additions & 126 deletions pkg/sql/schema_changer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4240,132 +4240,6 @@ CREATE TABLE t.test (k INT PRIMARY KEY, v INT, pi DECIMAL REFERENCES t.pi (d) DE
}
}

// Test TRUNCATE during a column backfill.
func TestTruncateWhileColumnBackfill(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)

skip.WithIssue(t, 43990)

backfillNotification := make(chan struct{})
backfillCount := int64(0)
params, _ := tests.CreateTestServerParams()
params.Knobs = base.TestingKnobs{
// Runs schema changes asynchronously.
SQLSchemaChanger: &sql.SchemaChangerTestingKnobs{
// TODO (lucy): if/when this test gets reinstated, figure out what knobs are
// needed.
},
DistSQL: &execinfra.TestingKnobs{
RunBeforeBackfillChunk: func(sp roachpb.Span) error {
switch atomic.LoadInt64(&backfillCount) {
case 3:
// Notify in the middle of a backfill.
if backfillNotification != nil {
close(backfillNotification)
backfillNotification = nil
}
// Never complete the backfill.
return context.DeadlineExceeded
default:
atomic.AddInt64(&backfillCount, 1)
}
return nil
},
},
}
s, sqlDB, kvDB := serverutils.StartServer(t, params)
defer s.Stopper().Stop(context.Background())

if _, err := sqlDB.Exec(`
CREATE DATABASE t;
CREATE TABLE t.test (k INT PRIMARY KEY, v INT);
`); err != nil {
t.Fatal(err)
}

// Bulk insert.
const maxValue = 5000
if err := sqltestutils.BulkInsertIntoTable(sqlDB, maxValue); err != nil {
t.Fatal(err)
}

notify := backfillNotification

const add_column = `ALTER TABLE t.public.test ADD COLUMN x DECIMAL NOT NULL DEFAULT 1.4::DECIMAL, ADD CHECK (x >= 0)`
if _, err := sqlDB.Exec(add_column); err != nil {
t.Fatal(err)
}

const drop_column = `ALTER TABLE t.public.test DROP COLUMN v`
if _, err := sqlDB.Exec(drop_column); err != nil {
t.Fatal(err)
}

// Check that an outstanding schema change exists.
tableDesc := desctestutils.TestingGetPublicTableDescriptor(kvDB, keys.SystemSQLCodec, "t", "test")
oldID := tableDesc.GetID()
if lenMutations := len(tableDesc.AllMutations()); lenMutations != 3 {
t.Fatalf("%d outstanding schema change", lenMutations)
}

// Run TRUNCATE.
var wg sync.WaitGroup
wg.Add(1)
go func() {
<-notify
if _, err := sqlDB.Exec("TRUNCATE TABLE t.test"); err != nil {
t.Error(err)
}
wg.Done()
}()
wg.Wait()

// The new table is truncated.
tableDesc = desctestutils.TestingGetPublicTableDescriptor(kvDB, keys.SystemSQLCodec, "t", "test")
tablePrefix := keys.SystemSQLCodec.TablePrefix(uint32(tableDesc.GetID()))
tableEnd := tablePrefix.PrefixEnd()
if kvs, err := kvDB.Scan(context.Background(), tablePrefix, tableEnd, 0); err != nil {
t.Fatal(err)
} else if e := 0; len(kvs) != e {
t.Fatalf("expected %d key value pairs, but got %d", e, len(kvs))
}

// Col "x" is public and col "v" is dropped.
if num := len(tableDesc.AllMutations()); num > 0 {
t.Fatalf("%d outstanding mutation", num)
}
if lenCols := len(tableDesc.PublicColumns()); lenCols != 2 {
t.Fatalf("%d columns", lenCols)
}
if k, x := tableDesc.PublicColumns()[0].GetName(), tableDesc.PublicColumns()[1].GetName(); k != "k" && x != "x" {
t.Fatalf("columns %q, %q in descriptor", k, x)
}
if checks := tableDesc.AllActiveAndInactiveChecks(); len(checks) != 1 {
t.Fatalf("expected 1 check, found %d", len(checks))
}

sqlRun := sqlutils.MakeSQLRunner(sqlDB)
if err := jobutils.VerifySystemJob(t, sqlRun, 0, jobspb.TypeSchemaChange, jobs.StatusSucceeded, jobs.Record{
Username: security.RootUserName(),
Description: add_column,
DescriptorIDs: descpb.IDs{
oldID,
},
}); err != nil {
t.Fatal(err)
}
if err := jobutils.VerifySystemJob(t, sqlRun, 1, jobspb.TypeSchemaChange, jobs.StatusSucceeded, jobs.Record{
Username: security.RootUserName(),
Description: drop_column,
DescriptorIDs: descpb.IDs{
oldID,
},
}); err != nil {
t.Fatal(err)
}
}

// Test that, when DDL statements are run in a transaction, their errors are
// received as the results of the commit statement.
func TestSchemaChangeErrorOnCommit(t *testing.T) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ func columnDescToElement(
Inaccessible: column.Inaccessible,
GeneratedAsIdentitySequenceOption: nilToEmptyString(column.GeneratedAsIdentitySequenceOption),
GeneratedAsIdentityType: column.GeneratedAsIdentityType,
UsesSequenceIds: column.UsesSequenceIds,
UsesSequenceIDs: column.UsesSequenceIds,
ComputerExpr: nilToEmptyString(column.ComputeExpr),
PgAttributeNum: column.GetPGAttributeNum(),
SystemColumnKind: column.SystemColumnKind,
Expand Down
20 changes: 10 additions & 10 deletions pkg/sql/schemachanger/scbuild/testdata/drop_database
Original file line number Diff line number Diff line change
Expand Up @@ -228,14 +228,14 @@ DROP DATABASE db1 CASCADE
columnId: 3
defaultExpr: nextval(108:::REGCLASS)
tableId: 109
usesSequenceIDs:
usesSequenceIds:
- 108
- [[DefaultExpression:{DescID: 110, ColumnID: 3}, ABSENT], PUBLIC]
details:
columnId: 3
defaultExpr: nextval(107:::REGCLASS)
tableId: 110
usesSequenceIDs:
usesSequenceIds:
- 107
- [[IndexComment:{DescID: 109, IndexID: 1}, ABSENT], PUBLIC]
details:
Expand Down Expand Up @@ -418,42 +418,42 @@ DROP DATABASE db1 CASCADE
unique: true
- [[RelationDependedOnBy:{DescID: 107, ReferencedDescID: 110}, ABSENT], PUBLIC]
details:
columnID: 3
columnId: 3
dependedOn: 110
tableId: 107
- [[RelationDependedOnBy:{DescID: 108, ReferencedDescID: 109}, ABSENT], PUBLIC]
details:
columnID: 3
columnId: 3
dependedOn: 109
tableId: 108
- [[RelationDependedOnBy:{DescID: 109, ReferencedDescID: 111}, ABSENT], PUBLIC]
details:
columnID: 2
columnId: 2
dependedOn: 111
tableId: 109
- [[RelationDependedOnBy:{DescID: 111, ReferencedDescID: 112}, ABSENT], PUBLIC]
details:
columnID: 1
columnId: 1
dependedOn: 112
tableId: 111
- [[RelationDependedOnBy:{DescID: 111, ReferencedDescID: 113}, ABSENT], PUBLIC]
details:
columnID: 1
columnId: 1
dependedOn: 113
tableId: 111
- [[RelationDependedOnBy:{DescID: 112, ReferencedDescID: 113}, ABSENT], PUBLIC]
details:
columnID: 1
columnId: 1
dependedOn: 113
tableId: 112
- [[RelationDependedOnBy:{DescID: 112, ReferencedDescID: 114}, ABSENT], PUBLIC]
details:
columnID: 1
columnId: 1
dependedOn: 114
tableId: 112
- [[RelationDependedOnBy:{DescID: 114, ReferencedDescID: 117}, ABSENT], PUBLIC]
details:
columnID: 1
columnId: 1
dependedOn: 117
tableId: 114
- [[Schema:{DescID: 105}, ABSENT], PUBLIC]
Expand Down
18 changes: 9 additions & 9 deletions pkg/sql/schemachanger/scbuild/testdata/drop_schema
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ DROP SCHEMA defaultdb.SC1 CASCADE
columnId: 3
defaultExpr: nextval(106:::REGCLASS)
tableId: 107
usesSequenceIDs:
usesSequenceIds:
- 106
- [[IndexComment:{DescID: 107, IndexID: 1}, ABSENT], PUBLIC]
details:
Expand Down Expand Up @@ -274,42 +274,42 @@ DROP SCHEMA defaultdb.SC1 CASCADE
unique: true
- [[RelationDependedOnBy:{DescID: 106, ReferencedDescID: 107}, ABSENT], PUBLIC]
details:
columnID: 3
columnId: 3
dependedOn: 107
tableId: 106
- [[RelationDependedOnBy:{DescID: 107, ReferencedDescID: 108}, ABSENT], PUBLIC]
details:
columnID: 2
columnId: 2
dependedOn: 108
tableId: 107
- [[RelationDependedOnBy:{DescID: 108, ReferencedDescID: 109}, ABSENT], PUBLIC]
details:
columnID: 1
columnId: 1
dependedOn: 109
tableId: 108
- [[RelationDependedOnBy:{DescID: 108, ReferencedDescID: 110}, ABSENT], PUBLIC]
details:
columnID: 1
columnId: 1
dependedOn: 110
tableId: 108
- [[RelationDependedOnBy:{DescID: 109, ReferencedDescID: 110}, ABSENT], PUBLIC]
details:
columnID: 1
columnId: 1
dependedOn: 110
tableId: 109
- [[RelationDependedOnBy:{DescID: 109, ReferencedDescID: 111}, ABSENT], PUBLIC]
details:
columnID: 1
columnId: 1
dependedOn: 111
tableId: 109
- [[RelationDependedOnBy:{DescID: 111, ReferencedDescID: 114}, ABSENT], PUBLIC]
details:
columnID: 1
columnId: 1
dependedOn: 114
tableId: 111
- [[RelationDependedOnBy:{DescID: 111, ReferencedDescID: 115}, ABSENT], PUBLIC]
details:
columnID: 1
columnId: 1
dependedOn: 115
tableId: 111
- [[Schema:{DescID: 104}, ABSENT], PUBLIC]
Expand Down
12 changes: 6 additions & 6 deletions pkg/sql/schemachanger/scbuild/testdata/drop_sequence
Original file line number Diff line number Diff line change
Expand Up @@ -75,21 +75,21 @@ DROP SEQUENCE defaultdb.SQ1 CASCADE
columnId: 2
defaultExpr: nextval(104:::REGCLASS)
tableId: 105
usesSequenceIDs:
usesSequenceIds:
- 104
- [[DefaultExpression:{DescID: 106, ColumnID: 2}, ABSENT], PUBLIC]
details:
columnId: 2
defaultExpr: nextval(104:::REGCLASS)
tableId: 106
usesSequenceIDs:
usesSequenceIds:
- 104
- [[DefaultExpression:{DescID: 109, ColumnID: 2}, ABSENT], PUBLIC]
details:
columnId: 2
defaultExpr: CAST(chr(nextval(104:::REGCLASS)) AS @100107)
tableId: 109
usesSequenceIDs:
usesSequenceIds:
- 104
- [[Locality:{DescID: 104}, ABSENT], PUBLIC]
details:
Expand All @@ -106,17 +106,17 @@ DROP SEQUENCE defaultdb.SQ1 CASCADE
owner: root
- [[RelationDependedOnBy:{DescID: 104, ReferencedDescID: 105}, ABSENT], PUBLIC]
details:
columnID: 2
columnId: 2
dependedOn: 105
tableId: 104
- [[RelationDependedOnBy:{DescID: 104, ReferencedDescID: 106}, ABSENT], PUBLIC]
details:
columnID: 2
columnId: 2
dependedOn: 106
tableId: 104
- [[RelationDependedOnBy:{DescID: 104, ReferencedDescID: 109}, ABSENT], PUBLIC]
details:
columnID: 2
columnId: 2
dependedOn: 109
tableId: 104
- [[Sequence:{DescID: 104}, ABSENT], PUBLIC]
Expand Down
8 changes: 4 additions & 4 deletions pkg/sql/schemachanger/scbuild/testdata/drop_table
Original file line number Diff line number Diff line change
Expand Up @@ -220,13 +220,13 @@ DROP TABLE defaultdb.shipments CASCADE;
columnId: 1
defaultExpr: gen_random_uuid()
tableId: 109
usesSequenceIDs: []
usesSequenceIds: []
- [[DefaultExpression:{DescID: 109, ColumnID: 5}, ABSENT], PUBLIC]
details:
columnId: 5
defaultExpr: nextval(106:::REGCLASS)
tableId: 109
usesSequenceIDs:
usesSequenceIds:
- 106
- [[ForeignKey:{DescID: 109, ReferencedDescID: 104, Name: fk_customers}, ABSENT], PUBLIC]
details:
Expand Down Expand Up @@ -333,12 +333,12 @@ DROP TABLE defaultdb.shipments CASCADE;
unique: true
- [[RelationDependedOnBy:{DescID: 106, ReferencedDescID: 109}, ABSENT], PUBLIC]
details:
columnID: 5
columnId: 5
dependedOn: 109
tableId: 106
- [[RelationDependedOnBy:{DescID: 109, ReferencedDescID: 111}, ABSENT], PUBLIC]
details:
columnID: 2
columnId: 2
dependedOn: 111
tableId: 109
- [[Sequence:{DescID: 110}, ABSENT], PUBLIC]
Expand Down
10 changes: 5 additions & 5 deletions pkg/sql/schemachanger/scbuild/testdata/drop_view
Original file line number Diff line number Diff line change
Expand Up @@ -143,27 +143,27 @@ DROP VIEW defaultdb.v1 CASCADE
tableId: 104
- [[RelationDependedOnBy:{DescID: 105, ReferencedDescID: 106}, ABSENT], PUBLIC]
details:
columnID: 1
columnId: 1
dependedOn: 106
tableId: 105
- [[RelationDependedOnBy:{DescID: 105, ReferencedDescID: 107}, ABSENT], PUBLIC]
details:
columnID: 1
columnId: 1
dependedOn: 107
tableId: 105
- [[RelationDependedOnBy:{DescID: 106, ReferencedDescID: 107}, ABSENT], PUBLIC]
details:
columnID: 1
columnId: 1
dependedOn: 107
tableId: 106
- [[RelationDependedOnBy:{DescID: 106, ReferencedDescID: 108}, ABSENT], PUBLIC]
details:
columnID: 1
columnId: 1
dependedOn: 108
tableId: 106
- [[RelationDependedOnBy:{DescID: 108, ReferencedDescID: 111}, ABSENT], PUBLIC]
details:
columnID: 1
columnId: 1
dependedOn: 111
tableId: 108
- [[TableComment:{DescID: 105}, ABSENT], PUBLIC]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,7 @@ func (m *visitor) MakeAddedColumnDeleteOnly(
Inaccessible: op.Inaccessible,
GeneratedAsIdentityType: op.GeneratedAsIdentityType,
GeneratedAsIdentitySequenceOption: emptyStrToNil(op.GeneratedAsIdentitySequenceOption),
UsesSequenceIds: op.UsesSequenceIds,
UsesSequenceIds: op.UsesSequenceIDs,
ComputeExpr: emptyStrToNil(op.ComputerExpr),
PGAttributeNum: op.PgAttributeNum,
SystemColumnKind: op.SystemColumnKind,
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/schemachanger/scop/mutation.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ type MakeAddedColumnDeleteOnly struct {
Inaccessible bool
GeneratedAsIdentityType catpb.GeneratedAsIdentityType
GeneratedAsIdentitySequenceOption string
UsesSequenceIds []descpb.ID
UsesSequenceIDs []descpb.ID
ComputerExpr string
PgAttributeNum uint32
SystemColumnKind catpb.SystemColumnKind
Expand Down
Loading

0 comments on commit c089297

Please sign in to comment.