Skip to content

Commit

Permalink
sql: fix qualified index name resolution
Browse files Browse the repository at this point in the history
In CockroachDB index names are relative to a table name
(e.g. `tbl@idx`) but in Postgres index names live in the schema
namespace. To offer compatibility with Postgres, CockroachDB
implements a special name resolution algorithm for index names
specified without the '@' syntax. For example, `DROP INDEX foo` will
search all tables in the current schema to find one with index name
`foo`.

Prior to this patch, CockroachDB was not able to search for a
_qualified_ index name specified without the '@' syntax. For example,
TypeORM issues `DROP INDEX public."primary"`, expecting to be able to
drop the primary index of some table in the `public` schema. This was
not recognized by CockroachDB.

This patch extends the name resolution for index names to support both
partial and complete qualification, using the same name resolution
rules as other objects.

Release note (sql change): CockroachDB now supports more ways to
specify an index name for statements that require one (e.g., `DROP
INDEX`, `ALTER INDEX ... RENAME`, etc.), in a way more compatible with
PostgreSQL.
  • Loading branch information
knz committed Apr 15, 2018
1 parent 266c831 commit f9595d6
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 29 deletions.
4 changes: 2 additions & 2 deletions pkg/ccl/partitionccl/zone_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,11 +220,11 @@ func TestInvalidIndexPartitionSetShowZones(t *testing.T) {
}{
{
"ALTER INDEX foo EXPERIMENTAL CONFIGURE ZONE ''",
`no database specified: "foo"`,
`no schema has been selected to search index: "foo"`,
},
{
"EXPERIMENTAL SHOW ZONE CONFIGURATION FOR INDEX foo",
`no database specified: "foo"`,
`no schema has been selected to search index: "foo"`,
},
{
"USE system; ALTER INDEX foo EXPERIMENTAL CONFIGURE ZONE ''",
Expand Down
51 changes: 44 additions & 7 deletions pkg/sql/logictest/testdata/logic_test/namespace
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,15 @@ t
# Of course one can also list the tables in "public" by making it the
# current database.
statement ok
SET DATABASE = public
SET database = public

query T
SHOW TABLES
----
t

statement ok
SET DATABASE = test; DROP DATABASE public
SET database = test; DROP DATABASE public

# Unqualified pg_type resolves from pg_catalog.
query T
Expand All @@ -40,7 +40,7 @@ date

# Override table and check name resolves properly.
statement ok
SET SEARCH_PATH=public,pg_catalog
SET search_path=public,pg_catalog

statement ok
CREATE TABLE pg_type(x INT); INSERT INTO pg_type VALUES(42)
Expand All @@ -53,23 +53,60 @@ SELECT x FROM pg_type
# Leave database, check name resolves to default.
# The expected error can only occur on the virtual pg_type, not the physical one.
query error cannot access virtual schema in anonymous database
SET DATABASE = ''; SELECT * FROM pg_type
SET database = ''; SELECT * FROM pg_type

# Go to different database, check name still resolves to default.
query T
CREATE DATABASE foo; SET DATABASE = foo; SELECT typname FROM pg_type WHERE typname = 'date'
CREATE DATABASE foo; SET database = foo; SELECT typname FROM pg_type WHERE typname = 'date'
----
date

# Verify that pg_catalog at the beginning of the search path takes precedence.
query T
SET DATABASE = test; SET SEARCH_PATH = pg_catalog,public; SELECT typname FROM pg_type WHERE typname = 'date'
SET database = test; SET search_path = pg_catalog,public; SELECT typname FROM pg_type WHERE typname = 'date'
----
date

# Now set the search path to the testdb, placing pg_catalog explicitly
# at the end.
query I
SET SEARCH_PATH = public,pg_catalog; SELECT x FROM pg_type
SET search_path = public,pg_catalog; SELECT x FROM pg_type
----
42

statement ok
DROP TABLE pg_type; RESET search_path; SET database = test

# Unqualified index name resolution.
statement ok
ALTER INDEX "primary" RENAME TO a_pk

# Schema-qualified index name resolution.
statement ok
ALTER INDEX public.a_pk RENAME TO a_pk2

# DB-qualified index name resolution (CRDB 1.x compat).
statement ok
ALTER INDEX test.a_pk2 RENAME TO a_pk3

statement ok
CREATE DATABASE public; CREATE TABLE public.public.t(a INT)

# We can't see the DB "public" with DB-qualified index name resolution.
statement error index "primary" does not exist
ALTER INDEX public."primary" RENAME TO t_pk

# But we can see it with sufficient qualification.
statement ok
ALTER INDEX public.public."primary" RENAME TO t_pk

# If the search path is invalid, we get a special error.
statement ok
SET search_path = invalid

statement error no schema has been selected to search index: "a_pk3"
ALTER INDEX a_pk3 RENAME TO a_pk4

# But qualification resolves the problem.
statement ok
ALTER INDEX public.a_pk3 RENAME TO a_pk4
51 changes: 31 additions & 20 deletions pkg/sql/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -283,23 +283,26 @@ func (p *planner) getQualifiedTableName(
return tbName.String(), nil
}

// findTableContainingIndex returns the name of the table containing an
// index of the given name and its descriptor.
// findTableContainingIndex returns the descriptor of a table
// containing the index of the given name.
// This is used by expandIndexName().
//
// An error is returned if the index name is ambiguous (i.e. exists in
// multiple tables). If no table is found and requireTable is true, an
// error will be returned, otherwise the TableName and descriptor
// returned will be nil.
func findTableContainingIndex(
sc SchemaAccessor, dbName string, idxName tree.UnrestrictedName, lookupFlags CommonLookupFlags,
sc SchemaAccessor,
dbName, scName string,
idxName tree.UnrestrictedName,
lookupFlags CommonLookupFlags,
) (result *tree.TableName, desc *sqlbase.TableDescriptor, err error) {
dbDesc, err := sc.GetDatabaseDesc(dbName, lookupFlags)
if dbDesc == nil || err != nil {
return nil, nil, err
}

tns, err := sc.GetObjectNames(dbDesc, tree.PublicSchema,
tns, err := sc.GetObjectNames(dbDesc, scName,
DatabaseListFlags{CommonLookupFlags: lookupFlags, explicitPrefix: true})
if err != nil {
return nil, nil, err
Expand Down Expand Up @@ -370,32 +373,40 @@ func expandIndexName(
// subsequent call to expandIndexName() will generate tn using the
// new value of index.Table, which is a table name.

// Just an assertion: if we got there, there cannot be a path prefix
// in tn or a value in index.Index yet.
if tn.ExplicitSchema || tn.ExplicitCatalog || index.Index != "" {
// Just an assertion: if we got there, there cannot be a value in index.Index yet.
if index.Index != "" {
return nil, nil, pgerror.NewErrorf(pgerror.CodeInternalError,
"programmer error: not-searched index name found already qualified: %s@%s", tn, index.Index)
}

curDb := sc.CurrentDatabase()
if curDb == "" {
return nil, nil, pgerror.NewErrorf(pgerror.CodeUndefinedObjectError,
"no database specified: %q", tree.ErrString(index))
index.Index = tree.UnrestrictedName(tn.TableName)

// Look up the table prefix.
found, _, err := tn.TableNamePrefix.Resolve(ctx, sc, sc.CurrentDatabase(), sc.CurrentSearchPath())
if err != nil {
return nil, nil, err
}
if !found {
if requireTable {
return nil, nil, pgerror.NewErrorf(pgerror.CodeUndefinedObjectError,
"no schema has been selected to search index: %q",
tree.ErrString(&index.Index)).SetHintf(
"check the current database and search_path are valid")
}
return nil, nil, nil
}

index.Index = tree.UnrestrictedName(tn.TableName)
lookupFlags := sc.CommonLookupFlags(ctx, requireTable)
tn, desc, err = findTableContainingIndex(
sc.LogicalSchemaAccessor(), curDb, index.Index, lookupFlags)
var foundTn *tree.TableName
foundTn, desc, err = findTableContainingIndex(
sc.LogicalSchemaAccessor(), tn.Catalog(), tn.Schema(), index.Index, lookupFlags)
if err != nil {
return nil, nil, err
} else if tn == nil {
// NB: tn is nil here if and only if requireTable is
// false, otherwise err would be non-nil.
return nil, nil, nil
} else if foundTn != nil {
// Memoize the table name that was found. tn is a reference to the table name
// stored in index.Table.
*tn = *foundTn
}
// Memoize the resolved table name in case expandIndexName() is called again.
index.Table.TableNameReference = tn
}
return tn, desc, nil
}
Expand Down

0 comments on commit f9595d6

Please sign in to comment.