From 97415e16ce00d135f97621870866115a95e9957a Mon Sep 17 00:00:00 2001 From: Radu Berinde Date: Fri, 11 Nov 2016 17:56:19 -0500 Subject: [PATCH] sql: move query result checking in testutils/sqlutils Moving code to verify the result of a Query to sqlutils, where it can be used by other tests. In the descriptor mutation test we now check the number of keys against known values rather than relying on checkQueryResponse to count the number of NULLs in the expected result (which we know). Embedding a SQLRunner in mutationTest which allows us to use the new function directly, as well as the other Fatal-on-error wrappers. --- pkg/sql/descriptor_mutation_test.go | 248 ++++++++++----------------- pkg/sql/schema_changer_test.go | 22 +-- pkg/testutils/sqlutils/sql_runner.go | 55 ++++++ 3 files changed, 146 insertions(+), 179 deletions(-) diff --git a/pkg/sql/descriptor_mutation_test.go b/pkg/sql/descriptor_mutation_test.go index 0806e845fc49..d8c803657f23 100644 --- a/pkg/sql/descriptor_mutation_test.go +++ b/pkg/sql/descriptor_mutation_test.go @@ -18,7 +18,6 @@ package sql_test import ( gosql "database/sql" - "fmt" "math/rand" "testing" @@ -32,70 +31,25 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/sqlbase" "github.com/cockroachdb/cockroach/pkg/testutils" "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" + "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils" "github.com/cockroachdb/cockroach/pkg/util/leaktest" ) type mutationTest struct { - *testing.T + // SQLRunner embeds testing.TB + *sqlutils.SQLRunner kvDB *client.DB - sqlDB *gosql.DB tableDesc *sqlbase.TableDescriptor } -// checkQueryResponse runs the sql query q, and checks that it matches -// the expected response. It returns the total number of non-null values -// returned in the response (num-rows*num-columns - total-num-null-values), -// as a measure of the number of key:value pairs visible to it. -func (mt mutationTest) checkQueryResponse(q string, e [][]string) int { - // Read from DB. - rows, err := mt.sqlDB.Query(q) - if err != nil { - mt.Fatal(err) - } - cols, err := rows.Columns() - if err != nil { - mt.Fatal(err) - } - if len(e) > 0 && len(cols) != len(e[0]) { - mt.Fatalf("wrong number of columns %d in response to query %s", len(cols), q) - } - vals := make([]interface{}, len(cols)) - for i := range vals { - vals[i] = new(interface{}) - } - i := 0 - // Number of non-NULL values. - numVals := 0 - for ; rows.Next(); i++ { - if i >= len(e) { - mt.Errorf("expected less than %d rows, got %d rows:, %v", len(e), i, e) - return numVals - } - if err := rows.Scan(vals...); err != nil { - mt.Fatal(err) - } - for j, v := range vals { - if val := *v.(*interface{}); val != nil { - var s string - switch t := val.(type) { - case []byte: - s = string(t) - default: - s = fmt.Sprint(val) - } - if e[i][j] != s { - mt.Errorf("expected %v, found %v", e[i][j], s) - } - numVals++ - } else if e[i][j] != "NULL" { - mt.Errorf("expected %v, found %v", e[i][j], "NULL") - } - } - } - if i != len(e) { - mt.Errorf("fewer rows read than expected: found %d, expected %v", i, e) +func makeMutationTest( + t *testing.T, kvDB *client.DB, db *gosql.DB, tableDesc *sqlbase.TableDescriptor, +) mutationTest { + return mutationTest{ + SQLRunner: sqlutils.MakeSQLRunner(t, db), + kvDB: kvDB, + tableDesc: tableDesc, } - return numVals } // checkTableSize checks that the number of key:value pairs stored @@ -219,35 +173,24 @@ CREATE TABLE t.test (k CHAR PRIMARY KEY, v CHAR, i CHAR DEFAULT 'i', FAMILY (k), // read table descriptor tableDesc := sqlbase.GetTableDescriptor(kvDB, "t", "test") - mTest := mutationTest{ - T: t, - kvDB: kvDB, - sqlDB: sqlDB, - tableDesc: tableDesc, - } + mTest := makeMutationTest(t, kvDB, sqlDB, tableDesc) starQuery := `SELECT * FROM t.test` for _, useUpsert := range []bool{true, false} { // Run the tests for both states. for _, state := range []sqlbase.DescriptorMutation_State{sqlbase.DescriptorMutation_DELETE_ONLY, sqlbase.DescriptorMutation_WRITE_ONLY} { // Init table to start state. - if _, err := sqlDB.Exec(`TRUNCATE TABLE t.test`); err != nil { - t.Fatal(err) - } + mTest.Exec(`TRUNCATE TABLE t.test`) initRows := [][]string{{"a", "z", "q"}} for _, row := range initRows { if useUpsert { - if _, err := sqlDB.Exec(`UPSERT INTO t.test VALUES ($1, $2, $3)`, row[0], row[1], row[2]); err != nil { - t.Fatal(err) - } + mTest.Exec(`UPSERT INTO t.test VALUES ($1, $2, $3)`, row[0], row[1], row[2]) } else { - if _, err := sqlDB.Exec(`INSERT INTO t.test VALUES ($1, $2, $3)`, row[0], row[1], row[2]); err != nil { - t.Fatal(err) - } + mTest.Exec(`INSERT INTO t.test VALUES ($1, $2, $3)`, row[0], row[1], row[2]) } } // Check that the table only contains the initRows. - _ = mTest.checkQueryResponse(starQuery, initRows) + mTest.CheckQueryResults(starQuery, initRows) // Add column "i" as a mutation. mTest.writeColumnMutation("i", sqlbase.DescriptorMutation{State: state}) @@ -256,7 +199,7 @@ CREATE TABLE t.test (k CHAR PRIMARY KEY, v CHAR, i CHAR DEFAULT 'i', FAMILY (k), t.Fatalf("Read succeeded despite column being in %v state", sqlbase.DescriptorMutation{State: state}) } // The table only contains columns "k" and "v". - _ = mTest.checkQueryResponse(starQuery, [][]string{{"a", "z"}}) + mTest.CheckQueryResults(starQuery, [][]string{{"a", "z"}}) // The column backfill uses Put instead of CPut because it depends on // an INSERT of a column in the WRITE_ONLY state failing. These two @@ -286,9 +229,10 @@ CREATE TABLE t.test (k CHAR PRIMARY KEY, v CHAR, i CHAR DEFAULT 'i', FAMILY (k), // Make column "i" live so that it is read. mTest.makeMutationsActive() // Check that we can read all the rows and columns. - _ = mTest.checkQueryResponse(starQuery, initRows) + mTest.CheckQueryResults(starQuery, initRows) var afterInsert, afterUpdate, afterDelete [][]string + var afterDeleteKeys int if state == sqlbase.DescriptorMutation_DELETE_ONLY { // The default value of "i" for column "i" is not written. afterInsert = [][]string{{"a", "z", "q"}, {"c", "x", "NULL"}} @@ -296,6 +240,7 @@ CREATE TABLE t.test (k CHAR PRIMARY KEY, v CHAR, i CHAR DEFAULT 'i', FAMILY (k), afterUpdate = [][]string{{"a", "u", "q"}, {"c", "x", "NULL"}} // Delete also deletes column "i". afterDelete = [][]string{{"c", "x", "NULL"}} + afterDeleteKeys = 2 } else { // The default value of "i" for column "i" is written. afterInsert = [][]string{{"a", "z", "q"}, {"c", "x", "i"}} @@ -309,24 +254,21 @@ CREATE TABLE t.test (k CHAR PRIMARY KEY, v CHAR, i CHAR DEFAULT 'i', FAMILY (k), } // Delete also deletes column "i". afterDelete = [][]string{{"c", "x", "i"}} + afterDeleteKeys = 3 } // Make column "i" a mutation. mTest.writeColumnMutation("i", sqlbase.DescriptorMutation{State: state}) // Insert a row into the table. if useUpsert { - if _, err := sqlDB.Exec(`UPSERT INTO t.test VALUES ('c', 'x')`); err != nil { - t.Fatal(err) - } + mTest.Exec(`UPSERT INTO t.test VALUES ('c', 'x')`) } else { - if _, err := sqlDB.Exec(`INSERT INTO t.test VALUES ('c', 'x')`); err != nil { - t.Fatal(err) - } + mTest.Exec(`INSERT INTO t.test VALUES ('c', 'x')`) } // Make column "i" live so that it is read. mTest.makeMutationsActive() // Notice that the default value of "i" is only written when the // descriptor is in the WRITE_ONLY state. - _ = mTest.checkQueryResponse(starQuery, afterInsert) + mTest.CheckQueryResults(starQuery, afterInsert) // The column backfill uses Put instead of CPut because it depends on // an UPDATE of a column in the WRITE_ONLY state failing. This test @@ -336,51 +278,45 @@ CREATE TABLE t.test (k CHAR PRIMARY KEY, v CHAR, i CHAR DEFAULT 'i', FAMILY (k), mTest.writeColumnMutation("i", sqlbase.DescriptorMutation{State: state}) // Updating column "i" for a row fails. if useUpsert { - if _, err := sqlDB.Exec(`UPSERT INTO t.test VALUES ('a', 'u', 'u')`); !testutils.IsError(err, `table t.test has 2 columns but 3 values were supplied`) { + _, err := sqlDB.Exec(`UPSERT INTO t.test VALUES ('a', 'u', 'u')`) + if !testutils.IsError(err, `table t.test has 2 columns but 3 values were supplied`) { t.Fatal(err) } } else { - if _, err := sqlDB.Exec(`UPDATE t.test SET (v, i) = ('u', 'u') WHERE k = 'a'`); !testutils.IsError(err, `column "i" does not exist`) { + _, err := sqlDB.Exec(`UPDATE t.test SET (v, i) = ('u', 'u') WHERE k = 'a'`) + if !testutils.IsError(err, `column "i" does not exist`) { t.Fatal(err) } } // Make column "i" live so that it is read. mTest.makeMutationsActive() // The above failed update was a noop. - _ = mTest.checkQueryResponse(starQuery, afterInsert) + mTest.CheckQueryResults(starQuery, afterInsert) // Make column "i" a mutation. mTest.writeColumnMutation("i", sqlbase.DescriptorMutation{State: state}) // Update a row without specifying mutation column "i". if useUpsert { - if _, err := sqlDB.Exec(`UPSERT INTO t.test VALUES ('a', 'u')`); err != nil { - t.Fatal(err) - } + mTest.Exec(`UPSERT INTO t.test VALUES ('a', 'u')`) } else { - if _, err := sqlDB.Exec(`UPDATE t.test SET v = 'u' WHERE k = 'a'`); err != nil { - t.Fatal(err) - } + mTest.Exec(`UPDATE t.test SET v = 'u' WHERE k = 'a'`) } // Make column "i" live so that it is read. mTest.makeMutationsActive() // The update to column "v" is seen; there is no effect on column "i". - _ = mTest.checkQueryResponse(starQuery, afterUpdate) + mTest.CheckQueryResults(starQuery, afterUpdate) // Make column "i" a mutation. mTest.writeColumnMutation("i", sqlbase.DescriptorMutation{State: state}) // Delete row "a". - if _, err := sqlDB.Exec(`DELETE FROM t.test WHERE k = 'a'`); err != nil { - t.Fatal(err) - } + mTest.Exec(`DELETE FROM t.test WHERE k = 'a'`) // Make column "i" live so that it is read. mTest.makeMutationsActive() - // Row "a" is deleted. numVals is the number of non-NULL values seen, - // or the number of KV values belonging to all the rows in the table - // excluding row "a" since it's deleted. - numVals := mTest.checkQueryResponse(starQuery, afterDelete) + // Row "a" is deleted. + mTest.CheckQueryResults(starQuery, afterDelete) // Check that there are no hidden KV values for row "a", // and column "i" for row "a" was deleted. - mTest.checkTableSize(numVals) + mTest.checkTableSize(afterDeleteKeys) } } @@ -444,12 +380,7 @@ CREATE TABLE t.test (k CHAR PRIMARY KEY, v CHAR, INDEX foo (v)); // read table descriptor tableDesc := sqlbase.GetTableDescriptor(kvDB, "t", "test") - mTest := mutationTest{ - T: t, - kvDB: kvDB, - sqlDB: sqlDB, - tableDesc: tableDesc, - } + mTest := makeMutationTest(t, kvDB, sqlDB, tableDesc) starQuery := `SELECT * FROM t.test` indexQuery := `SELECT v FROM t.test@foo` @@ -472,9 +403,9 @@ CREATE TABLE t.test (k CHAR PRIMARY KEY, v CHAR, INDEX foo (v)); } } } - _ = mTest.checkQueryResponse(starQuery, initRows) + mTest.CheckQueryResults(starQuery, initRows) // Index foo is visible. - _ = mTest.checkQueryResponse(indexQuery, [][]string{{"y"}, {"z"}}) + mTest.CheckQueryResults(indexQuery, [][]string{{"y"}, {"z"}}) // Index foo is invisible once it's a mutation. mTest.writeIndexMutation("foo", sqlbase.DescriptorMutation{State: state}) @@ -492,16 +423,16 @@ CREATE TABLE t.test (k CHAR PRIMARY KEY, v CHAR, INDEX foo (v)); t.Fatal(err) } } - _ = mTest.checkQueryResponse(starQuery, [][]string{{"a", "z"}, {"b", "y"}, {"c", "x"}}) + mTest.CheckQueryResults(starQuery, [][]string{{"a", "z"}, {"b", "y"}, {"c", "x"}}) // Make index "foo" live so that we can read it. mTest.makeMutationsActive() if state == sqlbase.DescriptorMutation_DELETE_ONLY { // "x" didn't get added to the index. - _ = mTest.checkQueryResponse(indexQuery, [][]string{{"y"}, {"z"}}) + mTest.CheckQueryResults(indexQuery, [][]string{{"y"}, {"z"}}) } else { // "x" got added to the index. - _ = mTest.checkQueryResponse(indexQuery, [][]string{{"x"}, {"y"}, {"z"}}) + mTest.CheckQueryResults(indexQuery, [][]string{{"x"}, {"y"}, {"z"}}) } // Make "foo" a mutation. @@ -524,35 +455,33 @@ CREATE TABLE t.test (k CHAR PRIMARY KEY, v CHAR, INDEX foo (v)); t.Fatal(err) } } - _ = mTest.checkQueryResponse(starQuery, [][]string{{"a", "z"}, {"b", "y"}, {"c", "w"}}) + mTest.CheckQueryResults(starQuery, [][]string{{"a", "z"}, {"b", "y"}, {"c", "w"}}) // Make index "foo" live so that we can read it. mTest.makeMutationsActive() if state == sqlbase.DescriptorMutation_DELETE_ONLY { // updating "x" -> "w" is a noop on the index, // updating "z" -> "z" results in "z" being deleted from the index. - _ = mTest.checkQueryResponse(indexQuery, [][]string{{"y"}, {"z"}}) + mTest.CheckQueryResults(indexQuery, [][]string{{"y"}, {"z"}}) } else { // updating "x" -> "w" results in the index updating from "x" -> "w", // updating "z" -> "z" is a noop on the index. - _ = mTest.checkQueryResponse(indexQuery, [][]string{{"w"}, {"y"}, {"z"}}) + mTest.CheckQueryResults(indexQuery, [][]string{{"w"}, {"y"}, {"z"}}) } // Make "foo" a mutation. mTest.writeIndexMutation("foo", sqlbase.DescriptorMutation{State: state}) // Delete row "b". - if _, err := sqlDB.Exec(`DELETE FROM t.test WHERE k = 'b'`); err != nil { - t.Fatal(err) - } - _ = mTest.checkQueryResponse(starQuery, [][]string{{"a", "z"}, {"c", "w"}}) + mTest.Exec(`DELETE FROM t.test WHERE k = 'b'`) + mTest.CheckQueryResults(starQuery, [][]string{{"a", "z"}, {"c", "w"}}) // Make index "foo" live so that we can read it. mTest.makeMutationsActive() // Deleting row "b" deletes "y" from the index. if state == sqlbase.DescriptorMutation_DELETE_ONLY { - mTest.checkQueryResponse(indexQuery, [][]string{{"z"}}) + mTest.CheckQueryResults(indexQuery, [][]string{{"z"}}) } else { - mTest.checkQueryResponse(indexQuery, [][]string{{"w"}, {"z"}}) + mTest.CheckQueryResults(indexQuery, [][]string{{"w"}, {"z"}}) } } } @@ -594,12 +523,7 @@ CREATE TABLE t.test (k CHAR PRIMARY KEY, v CHAR, i CHAR, INDEX foo (i, v), FAMIL // read table descriptor tableDesc := sqlbase.GetTableDescriptor(kvDB, "t", "test") - mTest := mutationTest{ - T: t, - kvDB: kvDB, - sqlDB: sqlDB, - tableDesc: tableDesc, - } + mTest := makeMutationTest(t, kvDB, sqlDB, tableDesc) starQuery := `SELECT * FROM t.test` indexQuery := `SELECT i FROM t.test@foo` @@ -626,17 +550,13 @@ CREATE TABLE t.test (k CHAR PRIMARY KEY, v CHAR, i CHAR, INDEX foo (i, v), FAMIL initRows := [][]string{{"a", "z", "q"}, {"b", "y", "r"}} for _, row := range initRows { if useUpsert { - if _, err := sqlDB.Exec(`UPSERT INTO t.test VALUES ($1, $2, $3)`, row[0], row[1], row[2]); err != nil { - t.Fatal(err) - } + mTest.Exec(`UPSERT INTO t.test VALUES ($1, $2, $3)`, row[0], row[1], row[2]) } else { - if _, err := sqlDB.Exec(`INSERT INTO t.test VALUES ($1, $2, $3)`, row[0], row[1], row[2]); err != nil { - t.Fatal(err) - } + mTest.Exec(`INSERT INTO t.test VALUES ($1, $2, $3)`, row[0], row[1], row[2]) } } // Check that the table only contains the initRows. - _ = mTest.checkQueryResponse(starQuery, initRows) + mTest.CheckQueryResults(starQuery, initRows) // Add index "foo" as a mutation. mTest.writeIndexMutation("foo", sqlbase.DescriptorMutation{State: idxState}) @@ -657,13 +577,13 @@ CREATE TABLE t.test (k CHAR PRIMARY KEY, v CHAR, i CHAR, INDEX foo (i, v), FAMIL // Make column "i" and index "foo" live. mTest.makeMutationsActive() // column "i" has no entry. - _ = mTest.checkQueryResponse(starQuery, [][]string{{"a", "z", "q"}, {"b", "y", "r"}, {"c", "x", "NULL"}}) + mTest.CheckQueryResults(starQuery, [][]string{{"a", "z", "q"}, {"b", "y", "r"}, {"c", "x", "NULL"}}) if idxState == sqlbase.DescriptorMutation_DELETE_ONLY { // No index entry for row "c" - _ = mTest.checkQueryResponse(indexQuery, [][]string{{"q"}, {"r"}}) + mTest.CheckQueryResults(indexQuery, [][]string{{"q"}, {"r"}}) } else { // Index entry for row "c" - _ = mTest.checkQueryResponse(indexQuery, [][]string{{"NULL"}, {"q"}, {"r"}}) + mTest.CheckQueryResults(indexQuery, [][]string{{"NULL"}, {"q"}, {"r"}}) } // Add index "foo" as a mutation. @@ -696,13 +616,13 @@ CREATE TABLE t.test (k CHAR PRIMARY KEY, v CHAR, i CHAR, INDEX foo (i, v), FAMIL mTest.makeMutationsActive() // The update to column "v" is seen; there is no effect on column "i". - _ = mTest.checkQueryResponse(starQuery, [][]string{{"a", "u", "q"}, {"b", "y", "r"}, {"c", "x", "NULL"}}) + mTest.CheckQueryResults(starQuery, [][]string{{"a", "u", "q"}, {"b", "y", "r"}, {"c", "x", "NULL"}}) if idxState == sqlbase.DescriptorMutation_DELETE_ONLY { // Index entry for row "a" is deleted. - _ = mTest.checkQueryResponse(indexQuery, [][]string{{"r"}}) + mTest.CheckQueryResults(indexQuery, [][]string{{"r"}}) } else { // No change in index "foo" - _ = mTest.checkQueryResponse(indexQuery, [][]string{{"NULL"}, {"q"}, {"r"}}) + mTest.CheckQueryResults(indexQuery, [][]string{{"NULL"}, {"q"}, {"r"}}) } // Add index "foo" as a mutation. @@ -716,26 +636,24 @@ CREATE TABLE t.test (k CHAR PRIMARY KEY, v CHAR, i CHAR, INDEX foo (i, v), FAMIL } // Make column "i" and index "foo" live. mTest.makeMutationsActive() - // Row "b" is deleted. numVals is the number of non-NULL values seen, - // or the number of KV values belonging to all the rows in the table - // excluding row "b" since it's deleted. - numVals := mTest.checkQueryResponse(starQuery, [][]string{{"a", "u", "q"}, {"c", "x", "NULL"}}) - // idxVals is the number of index values seen. - var idxVals int + // Row "b" is deleted. + mTest.CheckQueryResults(starQuery, [][]string{{"a", "u", "q"}, {"c", "x", "NULL"}}) + // numKVs is the number of expected key-values. We start with the number + // of non-NULL values above. + numKVs := 5 if idxState == sqlbase.DescriptorMutation_DELETE_ONLY { // Index entry for row "b" is deleted. - idxVals = mTest.checkQueryResponse(indexQuery, [][]string{}) + mTest.CheckQueryResults(indexQuery, [][]string{}) } else { - // Index entry for row "b" is deleted. idxVals doesn't account for - // the NULL value seen. - idxVals = mTest.checkQueryResponse(indexQuery, [][]string{{"NULL"}, {"q"}}) - // Increment idxVals to account for the NULL value seen above. - idxVals++ + // Index entry for row "b" is deleted. + mTest.CheckQueryResults(indexQuery, [][]string{{"NULL"}, {"q"}}) + // We have two index values. + numKVs += 2 } // Check that there are no hidden KV values for row "b", and column // "i" for row "b" was deleted. Also check that the index values are // all accounted for. - mTest.checkTableSize(numVals + idxVals) + mTest.checkTableSize(numKVs) } } } @@ -767,12 +685,7 @@ CREATE TABLE t.test (a CHAR PRIMARY KEY, b CHAR, c CHAR, INDEX foo (c)); // Read table descriptor tableDesc := sqlbase.GetTableDescriptor(kvDB, "t", "test") - mt := mutationTest{ - T: t, - kvDB: kvDB, - sqlDB: sqlDB, - tableDesc: tableDesc, - } + mt := makeMutationTest(t, kvDB, sqlDB, tableDesc) // Test CREATE INDEX in the presence of mutations. @@ -875,9 +788,7 @@ CREATE TABLE t.test (a CHAR PRIMARY KEY, b CHAR, c CHAR, INDEX foo (c)); // "b" is being added. mt.writeColumnMutation("b", sqlbase.DescriptorMutation{Direction: sqlbase.DescriptorMutation_ADD}) // Noop. - if _, err := sqlDB.Exec(`ALTER TABLE t.test ADD CONSTRAINT bar UNIQUE (b)`); err != nil { - t.Fatal(err) - } + mt.Exec(`ALTER TABLE t.test ADD CONSTRAINT bar UNIQUE (b)`) // Make "b" live. mt.makeMutationsActive() @@ -916,7 +827,13 @@ CREATE TABLE t.test (a CHAR PRIMARY KEY, b CHAR, c CHAR, INDEX foo (c)); // Make "ufo" live. mt.makeMutationsActive() // The index has been renamed to ufo, and the column to d. - _ = mt.checkQueryResponse("SHOW INDEXES FROM t.test", [][]string{{"test", "primary", "true", "1", "a", "ASC", "false"}, {"test", "ufo", "false", "1", "d", "ASC", "false"}}) + mt.CheckQueryResults( + "SHOW INDEXES FROM t.test", + [][]string{ + {"test", "primary", "true", "1", "a", "ASC", "false"}, + {"test", "ufo", "false", "1", "d", "ASC", "false"}, + }, + ) // Rename column under mutation works properly. @@ -933,7 +850,14 @@ CREATE TABLE t.test (a CHAR PRIMARY KEY, b CHAR, c CHAR, INDEX foo (c)); // Make column "e" live. mt.makeMutationsActive() // Column b changed to d. - _ = mt.checkQueryResponse("SHOW COLUMNS FROM t.test", [][]string{{"a", "STRING", "false", "NULL"}, {"d", "STRING", "true", "NULL"}, {"e", "STRING", "true", "NULL"}}) + mt.CheckQueryResults( + "SHOW COLUMNS FROM t.test", + [][]string{ + {"a", "STRING", "false", "NULL"}, + {"d", "STRING", "true", "NULL"}, + {"e", "STRING", "true", "NULL"}, + }, + ) // Try to change column defaults while column is under mutation. mt.writeColumnMutation("e", sqlbase.DescriptorMutation{Direction: sqlbase.DescriptorMutation_ADD}) diff --git a/pkg/sql/schema_changer_test.go b/pkg/sql/schema_changer_test.go index b78924e8db26..c7d3a42c3c6a 100644 --- a/pkg/sql/schema_changer_test.go +++ b/pkg/sql/schema_changer_test.go @@ -380,14 +380,9 @@ CREATE INDEX foo ON t.test (v) } // Ensure that the indexes have been created. - mTest := mutationTest{ - T: t, - kvDB: kvDB, - sqlDB: sqlDB, - tableDesc: tableDesc, - } + mTest := makeMutationTest(t, kvDB, sqlDB, tableDesc) indexQuery := `SELECT v FROM t.test@foo` - _ = mTest.checkQueryResponse(indexQuery, [][]string{{"b"}, {"d"}}) + mTest.CheckQueryResults(indexQuery, [][]string{{"b"}, {"d"}}) // Ensure that the version has been incremented. tableDesc = sqlbase.GetTableDescriptor(kvDB, "t", "test") @@ -399,11 +394,7 @@ CREATE INDEX foo ON t.test (v) // Apply a schema change that only sets the UpVersion bit. expectedVersion = newVersion + 1 - if _, err := sqlDB.Exec(` -ALTER INDEX t.test@foo RENAME TO ufo -`); err != nil { - t.Fatal(err) - } + mTest.Exec(`ALTER INDEX t.test@foo RENAME TO ufo`) for r := retry.Start(retryOpts); r.Next(); { // Ensure that the version gets incremented. @@ -422,10 +413,7 @@ ALTER INDEX t.test@foo RENAME TO ufo // that they all get executed. count := 5 for i := 0; i < count; i++ { - cmd := fmt.Sprintf(`CREATE INDEX foo%d ON t.test (v)`, i) - if _, err := sqlDB.Exec(cmd); err != nil { - t.Fatal(err) - } + mTest.Exec(fmt.Sprintf(`CREATE INDEX foo%d ON t.test (v)`, i)) } // Wait until indexes are created. for r := retry.Start(retryOpts); r.Next(); { @@ -436,7 +424,7 @@ ALTER INDEX t.test@foo RENAME TO ufo } for i := 0; i < count; i++ { indexQuery := fmt.Sprintf(`SELECT v FROM t.test@foo%d`, i) - _ = mTest.checkQueryResponse(indexQuery, [][]string{{"b"}, {"d"}}) + mTest.CheckQueryResults(indexQuery, [][]string{{"b"}, {"d"}}) } } diff --git a/pkg/testutils/sqlutils/sql_runner.go b/pkg/testutils/sqlutils/sql_runner.go index 6240b608bb03..3df2d37a33e7 100644 --- a/pkg/testutils/sqlutils/sql_runner.go +++ b/pkg/testutils/sqlutils/sql_runner.go @@ -18,7 +18,10 @@ package sqlutils import ( gosql "database/sql" + "fmt" "testing" + + "github.com/cockroachdb/cockroach/pkg/util/caller" ) // SQLRunner wraps a testing.TB and *gosql.DB connection and provides @@ -81,3 +84,55 @@ func (r *Row) Scan(dest ...interface{}) { func (sr *SQLRunner) QueryRow(query string, args ...interface{}) *Row { return &Row{sr.TB, sr.DB.QueryRow(query, args...)} } + +// CheckQueryResults checks that the rows returned by a query match the expected +// response. +func (sr *SQLRunner) CheckQueryResults(query string, expected [][]string) { + file, line, _ := caller.Lookup(1) + info := fmt.Sprintf("%s:%d query '%s'", file, line, query) + + rows := sr.Query(query) + cols, err := rows.Columns() + if err != nil { + sr.Error(err) + return + } + if len(expected) > 0 && len(cols) != len(expected[0]) { + sr.Errorf("%s: wrong number of columns %d", info, len(cols)) + return + } + vals := make([]interface{}, len(cols)) + for i := range vals { + vals[i] = new(interface{}) + } + i := 0 + for ; rows.Next(); i++ { + if i >= len(expected) { + sr.Errorf("%s: expected %d rows, got more", info, len(expected)) + return + } + if err := rows.Scan(vals...); err != nil { + sr.Error(err) + return + } + for j, v := range vals { + if val := *v.(*interface{}); val != nil { + var s string + switch t := val.(type) { + case []byte: + s = string(t) + default: + s = fmt.Sprint(val) + } + if expected[i][j] != s { + sr.Errorf("%s: expected %v, found %v", info, expected[i][j], s) + } + } else if expected[i][j] != "NULL" { + sr.Errorf("%s: expected %v, found %v", info, expected[i][j], "NULL") + } + } + } + if i != len(expected) { + sr.Errorf("%s: found %d rows, expected %d", info, i, len(expected)) + } +}