Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql: sanitize the "no current db" story #24735

Merged
merged 1 commit into from
May 22, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions pkg/ccl/backupccl/backup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1970,7 +1970,7 @@ func TestRestoreAsOfSystemTimeGCBounds(t *testing.T) {
gcr := roachpb.GCRequest{
// Bogus span to make it a valid request.
Span: roachpb.Span{
Key: keys.MakeTablePrefix(keys.MaxReservedDescID + 1),
Key: keys.MakeTablePrefix(keys.MinUserDescID),
EndKey: keys.MaxKey,
},
Threshold: tc.Server(0).Clock().Now(),
Expand Down Expand Up @@ -2857,7 +2857,7 @@ func TestBackupRestoreSequence(t *testing.T) {

if _, err := newDB.DB.Exec(
`RESTORE TABLE t FROM $1`, localFoo,
); !testutils.IsError(err, "pq: cannot restore table \"t\" without referenced sequence 52 \\(or \"skip_missing_sequences\" option\\)") {
); !testutils.IsError(err, "pq: cannot restore table \"t\" without referenced sequence 54 \\(or \"skip_missing_sequences\" option\\)") {
t.Fatal(err)
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/importccl/import_stmt.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ const (
// We need to choose arbitrary database and table IDs. These aren't important,
// but they do match what would happen when creating a new database and
// table on an empty cluster.
defaultCSVParentID sqlbase.ID = keys.MaxReservedDescID + 1
defaultCSVParentID sqlbase.ID = keys.MinNonPredefinedUserDescID
defaultCSVTableID sqlbase.ID = defaultCSVParentID + 1
)

Expand Down
20 changes: 10 additions & 10 deletions pkg/ccl/logictestccl/testdata/logic_test/crdb_internal
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,13 @@ CREATE table t2 (a STRING PRIMARY KEY) PARTITION BY LIST (a) (
query IITTI
SELECT * FROM crdb_internal.partitions ORDER BY table_id, index_id, name
----
51 1 NULL p12 1
51 1 p12 p12p3 1
51 1 p12p3 p12p3p8 1
51 1 NULL p6 1
51 1 p6 p6p7 1
51 1 p6 p6p8 1
51 1 p6 p6px 1
51 1 p12 pd 1
51 2 NULL p00 2
52 1 NULL pfoo 1
53 1 NULL p12 1
53 1 p12 p12p3 1
53 1 p12p3 p12p3p8 1
53 1 NULL p6 1
53 1 p6 p6p7 1
53 1 p6 p6p8 1
53 1 p6 p6px 1
53 1 p12 pd 1
53 2 NULL p00 2
54 1 NULL pfoo 1
2 changes: 1 addition & 1 deletion pkg/ccl/partitionccl/partition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ func (t *partitioningTest) parse() error {
return errors.Errorf("expected *tree.CreateTable got %T", stmt)
}
st := cluster.MakeTestingClusterSettings()
const parentID, tableID = keys.MaxReservedDescID + 1, keys.MaxReservedDescID + 2
const parentID, tableID = keys.MinUserDescID, keys.MinUserDescID + 1
t.parsed.tableDesc, err = importccl.MakeSimpleTableDescriptor(
ctx, st, createTable, parentID, tableID, hlc.UnixNano())
if err != nil {
Expand Down
16 changes: 9 additions & 7 deletions pkg/ccl/partitionccl/zone_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ func TestValidIndexPartitionSetShowZones(t *testing.T) {
zoneOverride := config.DefaultZoneConfig()
zoneOverride.GC.TTLSeconds = 42

dbDescID := uint32(keys.MinNonPredefinedUserDescID)

defaultRow := sqlutils.ZoneRow{
ID: keys.RootNamespaceID,
CLISpecifier: ".default",
Expand All @@ -59,27 +61,27 @@ func TestValidIndexPartitionSetShowZones(t *testing.T) {
Config: zoneOverride,
}
dbRow := sqlutils.ZoneRow{
ID: keys.MaxReservedDescID + 1,
ID: dbDescID,
CLISpecifier: "d",
Config: zoneOverride,
}
tableRow := sqlutils.ZoneRow{
ID: keys.MaxReservedDescID + 2,
ID: dbDescID + 1,
CLISpecifier: "d.t",
Config: zoneOverride,
}
primaryRow := sqlutils.ZoneRow{
ID: keys.MaxReservedDescID + 2,
ID: dbDescID + 1,
CLISpecifier: "d.t@primary",
Config: zoneOverride,
}
p0Row := sqlutils.ZoneRow{
ID: keys.MaxReservedDescID + 2,
ID: dbDescID + 1,
CLISpecifier: "d.t.p0",
Config: zoneOverride,
}
p1Row := sqlutils.ZoneRow{
ID: keys.MaxReservedDescID + 2,
ID: dbDescID + 1,
CLISpecifier: "d.t.p1",
Config: zoneOverride,
}
Expand Down Expand Up @@ -220,11 +222,11 @@ func TestInvalidIndexPartitionSetShowZones(t *testing.T) {
}{
{
"ALTER INDEX foo EXPERIMENTAL CONFIGURE ZONE ''",
`no schema has been selected to search index: "foo"`,
`index "foo" does not exist`,
},
{
"EXPERIMENTAL SHOW ZONE CONFIGURATION FOR INDEX foo",
`no schema has been selected to search index: "foo"`,
`index "foo" does not exist`,
},
{
"USE system; ALTER INDEX foo EXPERIMENTAL CONFIGURE ZONE ''",
Expand Down
6 changes: 3 additions & 3 deletions pkg/ccl/storageccl/bench_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func BenchmarkAddSSTable(b *testing.B) {
defer tc.Stopper().Stop(ctx)
kvDB := tc.Server(0).DB()

id := sqlbase.ID(keys.MaxReservedDescID + 1)
id := sqlbase.ID(keys.MinUserDescID)

var totalLen int64
b.StopTimer()
Expand Down Expand Up @@ -105,7 +105,7 @@ func BenchmarkWriteBatch(b *testing.B) {
defer tc.Stopper().Stop(ctx)
kvDB := tc.Server(0).DB()

id := sqlbase.ID(keys.MaxReservedDescID + 1)
id := sqlbase.ID(keys.MinUserDescID)
var batch engine.RocksDBBatchBuilder

var totalLen int64
Expand Down Expand Up @@ -161,7 +161,7 @@ func BenchmarkImport(b *testing.B) {
defer tc.Stopper().Stop(ctx)
kvDB := tc.Server(0).DB()

id := sqlbase.ID(keys.MaxReservedDescID + 1)
id := sqlbase.ID(keys.MinUserDescID)

var totalLen int64
b.StopTimer()
Expand Down
8 changes: 7 additions & 1 deletion pkg/cli/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,8 @@ func Example_sql() {
// 42 69
// sql --execute=show databases
// Database
// defaultdb
// postgres
// system
// t
// sql -e select 1; select 2
Expand Down Expand Up @@ -2001,7 +2003,7 @@ func checkNodeStatus(t *testing.T, c cliTest, output string, start time.Time) {
testcases = append(testcases,
testCase{"leader_ranges", baseIdx, 3},
testCase{"leaseholder_ranges", baseIdx + 1, 3},
testCase{"ranges", baseIdx + 2, 20},
testCase{"ranges", baseIdx + 2, 22},
testCase{"unavailable_ranges", baseIdx + 3, 1},
testCase{"underreplicated_ranges", baseIdx + 4, 1},
)
Expand Down Expand Up @@ -2239,6 +2241,10 @@ writing ` + os.DevNull + `
debug/nodes/1/ranges/18
debug/nodes/1/ranges/19
debug/nodes/1/ranges/20
debug/nodes/1/ranges/21
debug/nodes/1/ranges/22
debug/schema/defaultdb@details
debug/schema/postgres@details
debug/schema/system@details
debug/schema/system/descriptor
debug/schema/system/eventlog
Expand Down
6 changes: 0 additions & 6 deletions pkg/cli/interactive_tests/test_contextual_help.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,6 @@ start_server $argv

spawn $argv sql

start_test "Check that a client without a current database suggests to use SET."
eexpect "warning: no current database set"
eexpect "SET database"
eexpect root@
end_test

start_test "Check that a syntax error can make suggestions."
send "select * from;\r"
eexpect "syntax error"
Expand Down
2 changes: 1 addition & 1 deletion pkg/cli/interactive_tests/test_local_cmds.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ expect {
}
root@ {}
}
eexpect "/> "
eexpect "/defaultdb> "
# restore
send "\\set show_times\r"
end_test
Expand Down
7 changes: 6 additions & 1 deletion pkg/cli/interactive_tests/test_txn_prompt.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,15 @@ send "SET DATABASE = testdb;\r"
eexpect "\nSET\r\n"
eexpect root@
eexpect "/testdb>"
send "SET DATABASE = '';\r"
send "SET sql_safe_updates = false;\r"
eexpect "\nSET\r\n"
send "SET database = '';\r"
eexpect "\nSET\r\n"
eexpect root@
eexpect "/>"
send "SET database = 'defaultdb';\r"
eexpect "\nSET\r\n"
eexpect root@
end_test

start_test "Test that prompt becomes OPEN when txn is opened."
Expand Down
12 changes: 6 additions & 6 deletions pkg/cli/sql.go
Original file line number Diff line number Diff line change
Expand Up @@ -549,6 +549,12 @@ func (c *cliState) refreshDatabaseName() (string, bool) {
return "", false
}

if dbVal == "" {
// Attempt to be helpful to new users.
fmt.Fprintln(stderr, "warning: no current database set."+
" Use SET database = <dbname> to change, CREATE DATABASE to make a new database.")
}

dbName := formatVal(dbVal.(string),
false /* showPrintableUnicode */, false /* shownewLinesAndTabs */)

Expand All @@ -570,12 +576,6 @@ func preparePrompts(dbURL string) (promptPrefix, fullPrompt, continuePrompt stri
username = parsedURL.User.Username()
}
promptPrefix = fmt.Sprintf("%s@%s", username, parsedURL.Host)

if parsedURL.Path == "" {
// Attempt to be helpful to new users.
fmt.Fprintln(stderr, "warning: no current database set."+
" Use SET database = <dbname> to change, CREATE DATABASE to make a new database.")
}
}

if len(promptPrefix) == 0 {
Expand Down
6 changes: 3 additions & 3 deletions pkg/config/system_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ func TestComputeSplitKeyTableIDs(t *testing.T) {
defer leaktest.AfterTest(t)()

const (
start = keys.MaxReservedDescID + 1
start = keys.MinUserDescID
reservedStart = keys.MaxSystemConfigDescID + 1
)

Expand Down Expand Up @@ -401,8 +401,8 @@ func TestGetZoneConfigForKey(t *testing.T) {
{tkey(keys.LeaseTableID), keys.SystemDatabaseID},
{tkey(keys.JobsTableID), keys.SystemDatabaseID},
{tkey(keys.LocationsTableID), keys.SystemDatabaseID},
{tkey(keys.MaxReservedDescID + 1), keys.MaxReservedDescID + 1},
{tkey(keys.MaxReservedDescID + 23), keys.MaxReservedDescID + 23},
{tkey(keys.MinUserDescID), keys.MinUserDescID},
{tkey(keys.MinUserDescID + 22), keys.MinUserDescID + 22},
{roachpb.RKeyMax, keys.RootNamespaceID},
}

Expand Down
11 changes: 10 additions & 1 deletion pkg/keys/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ var (
SystemConfigTableDataMax = roachpb.Key(MakeTablePrefix(MaxSystemConfigDescID + 1))

// UserTableDataMin is the start key of user structured data.
UserTableDataMin = roachpb.Key(MakeTablePrefix(MaxReservedDescID + 1))
UserTableDataMin = roachpb.Key(MakeTablePrefix(MinUserDescID))

// MaxKey is the infinity marker which is larger than any other key.
MaxKey = roachpb.KeyMax
Expand All @@ -277,6 +277,15 @@ const (
// cockroach.
MaxReservedDescID = 49

// MinUserDescID is the first descriptor ID available for user
// structured data.
MinUserDescID = MaxReservedDescID + 1

// MinNonPredefinedUserDescID is the first descriptor ID used by
// user-level objects that are not created automatically on empty
// clusters (default databases).
MinNonPredefinedUserDescID = MinUserDescID + 2

// VirtualDescriptorID is the ID used by all virtual descriptors.
VirtualDescriptorID = math.MaxUint32

Expand Down
4 changes: 2 additions & 2 deletions pkg/server/admin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ func TestAdminAPIDatabases(t *testing.T) {
t.Fatal(err)
}

expectedDBs := []string{"system", testdb}
expectedDBs := []string{"defaultdb", "postgres", "system", testdb}
if a, e := len(resp.Databases), len(expectedDBs); a != e {
t.Fatalf("length of result %d != expected %d", a, e)
}
Expand Down Expand Up @@ -689,7 +689,7 @@ func TestAdminAPIEvents(t *testing.T) {
{sql.EventLogNodeJoin, false, 0, 1},
{sql.EventLogNodeRestart, false, 0, 0},
{sql.EventLogDropDatabase, false, 0, 0},
{sql.EventLogCreateDatabase, false, 0, 1},
{sql.EventLogCreateDatabase, false, 0, 3},
{sql.EventLogDropTable, false, 0, 2},
{sql.EventLogCreateTable, false, 0, 3},
{sql.EventLogSetClusterSetting, false, 0, 5},
Expand Down
26 changes: 21 additions & 5 deletions pkg/server/testserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -373,16 +373,32 @@ func ExpectedInitialRangeCount(db *client.DB) (int, error) {
if err != nil {
return 0, err
}
maxDescriptorID := descriptorIDs[len(descriptorIDs)-1]

// System table splits occur at every possible table boundary between the end
// of the system config ID space (keys.MaxSystemConfigDescID) and the system
// table with the maximum ID (maxDescriptorID), even when an ID within the
// span does not have an associated descriptor.
systemTableSplits := int(maxDescriptorID - keys.MaxSystemConfigDescID)
// table with the maximum ID (maxSystemDescriptorID), even when an ID within
// the span does not have an associated descriptor.
maxSystemDescriptorID := descriptorIDs[0]
for _, descID := range descriptorIDs {
if descID > maxSystemDescriptorID && descID <= keys.MaxReservedDescID {
maxSystemDescriptorID = descID
}
}
systemTableSplits := int(maxSystemDescriptorID - keys.MaxSystemConfigDescID)

// User table splits are analogous to system table splits: they occur at every
// possible table boundary between the end of the system ID space
// (keys.MaxReservedDescID) and the user table with the maximum ID
// (maxUserDescriptorID), even when an ID within the span does not have an
// associated descriptor.
maxUserDescriptorID := descriptorIDs[len(descriptorIDs)-1]
userTableSplits := 0
if maxUserDescriptorID >= keys.MaxReservedDescID {
userTableSplits = int(maxUserDescriptorID - keys.MaxReservedDescID)
}

// `n` splits create `n+1` ranges.
return len(config.StaticSplits()) + systemTableSplits + 1, nil
return len(config.StaticSplits()) + systemTableSplits + userTableSplits + 1, nil
}

// WaitForInitialSplits waits for the server to complete its expected initial
Expand Down
6 changes: 5 additions & 1 deletion pkg/sql/conn_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -421,9 +421,13 @@ func (sp sessionParams) sessionData(
if sp.data != nil {
return *sp.data
}
curDb := sp.args.Database
if curDb == "" {
curDb = sessiondata.DefaultDatabaseName
}
sd := sessiondata.SessionData{
ApplicationName: sp.args.ApplicationName,
Database: sp.args.Database,
Database: curDb,
DistSQLMode: sessiondata.DistSQLExecMode(DistSQLClusterExecMode.Get(&settings.SV)),
SearchPath: sqlbase.DefaultSearchPath,
Location: time.UTC,
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/conn_executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ func TestNonRetriableErrorOnAutoCommit(t *testing.T) {
func TestErrorOnRollback(t *testing.T) {
defer leaktest.AfterTest(t)()

const targetKeyString string = "/Table/51/1/1/0"
const targetKeyString string = "/Table/53/1/1/0"
var injectedErr int64

// We're going to inject an error into our EndTransaction.
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestDatabaseDescriptor(t *testing.T) {
defer s.Stopper().Stop(context.TODO())
ctx := context.TODO()

expectedCounter := int64(keys.MaxReservedDescID + 1)
expectedCounter := int64(keys.MinNonPredefinedUserDescID)

// Test values before creating the database.
// descriptor ID counter.
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/distsqlrun/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,7 @@ func TestDistSQLReadsFillGatewayID(t *testing.T) {
if !ok {
return nil
}
if !strings.HasPrefix(scanReq.Span.Key.String(), "/Table/51/1") {
if !strings.HasPrefix(scanReq.Span.Key.String(), "/Table/53/1") {
return nil
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/drop_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,7 @@ func TestDropTable(t *testing.T) {
}

tableDesc := sqlbase.GetTableDescriptor(kvDB, "t", "kv")
nameKey := sqlbase.MakeNameMetadataKey(keys.MaxReservedDescID+1, "kv")
nameKey := sqlbase.MakeNameMetadataKey(keys.MinNonPredefinedUserDescID, "kv")
gr, err := kvDB.Get(ctx, nameKey)

if err != nil {
Expand Down Expand Up @@ -590,7 +590,7 @@ func TestDropTableDeleteData(t *testing.T) {

descs = append(descs, sqlbase.GetTableDescriptor(kvDB, "t", tableName))

nameKey := sqlbase.MakeNameMetadataKey(keys.MaxReservedDescID+1, tableName)
nameKey := sqlbase.MakeNameMetadataKey(keys.MinNonPredefinedUserDescID, tableName)
gr, err := kvDB.Get(ctx, nameKey)
if err != nil {
t.Fatal(err)
Expand Down
Loading