Skip to content

Commit

Permalink
sql: Protect against accidental treatment of views as tables
Browse files Browse the repository at this point in the history
  • Loading branch information
a-robinson committed Sep 13, 2016
1 parent ed43b5d commit fde5bdf
Show file tree
Hide file tree
Showing 8 changed files with 85 additions and 38 deletions.
12 changes: 6 additions & 6 deletions sql/drop.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func (p *planner) DropDatabase(n *parser.DropDatabase) (planNode, error) {

td := make([]*sqlbase.TableDescriptor, len(tbNames))
for i := range tbNames {
tbDesc, err := p.dropTablePrepare(&tbNames[i])
tbDesc, err := p.dropTableOrViewPrepare(&tbNames[i])
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -334,7 +334,7 @@ func (p *planner) DropView(n *parser.DropView) (planNode, error) {
return nil, err
}

droppedDesc, err := p.dropTablePrepare(tn)
droppedDesc, err := p.dropTableOrViewPrepare(tn)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -425,7 +425,7 @@ func (p *planner) DropTable(n *parser.DropTable) (planNode, error) {
return nil, err
}

droppedDesc, err := p.dropTablePrepare(tn)
droppedDesc, err := p.dropTableOrViewPrepare(tn)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -573,7 +573,7 @@ func (n *dropTableNode) ExplainPlan(v bool) (string, string, []planNode) {
return "drop table", "", nil
}

// dropTablePrepare/dropTableImpl is used to drop a single table by
// dropTableOrViewPrepare/dropTableImpl is used to drop a single table by
// name, which can result from either a DROP TABLE or DROP DATABASE
// statement. This method returns the dropped table descriptor, to be
// used for the purpose of logging the event. The table is not
Expand All @@ -585,9 +585,9 @@ func (n *dropTableNode) ExplainPlan(v bool) (string, string, []planNode) {
// the deleted bit set, meaning the lease manager will not hand out
// new leases for it and existing leases are released).
// If the table does not exist, this function returns a nil descriptor.
func (p *planner) dropTablePrepare(name *parser.TableName,
func (p *planner) dropTableOrViewPrepare(name *parser.TableName,
) (*sqlbase.TableDescriptor, error) {
tableDesc, err := p.getTableDesc(name)
tableDesc, err := p.getTableOrViewDesc(name)
if err != nil {
return nil, err
}
Expand Down
5 changes: 5 additions & 0 deletions sql/lease.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,11 @@ func (s LeaseStore) Acquire(
if tableDesc == nil {
return nil, errors.Errorf("ID %d is not a table", tableID)
}
// TODO(a-robinson): Allow taking leases on view descriptors once this
// method's callers know how to handle views.
if !tableDesc.IsTable() {
return nil, errors.Errorf("ID %d is a %s, not a table", tableID, tableDesc.TypeName())
}
if err := filterTableState(tableDesc); err != nil {
return nil, err
}
Expand Down
46 changes: 41 additions & 5 deletions sql/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,23 @@ type SchemaAccessor interface {
// returns the list of matching tables.
expandTableGlob(expr parser.TablePattern) (parser.TableNames, error)

// getTableDesc returns a table descriptor, or nil if the descriptor
// is not found. If you want the "not found" condition to return an error,
// use mustGetTableDesc() instead.
// getTableOrViewDesc returns a table descriptor for either a table or view,
// or nil if the descriptor is not found.
getTableOrViewDesc(tn *parser.TableName) (*sqlbase.TableDescriptor, error)

// getTableDesc returns a table descriptor for a table, or nil if the
// descriptor is not found. If you want the "not found" condition to
// return an error, use mustGetTableDesc() instead.
// Returns an error if the underlying table descriptor actually
// represents a view rather than a table.
getTableDesc(tn *parser.TableName) (*sqlbase.TableDescriptor, error)

// getViewDesc returns a table descriptor for a table, or nil if the
// descriptor is not found.
// Returns an error if the underlying table descriptor actually
// represents a table rather than a view.
getViewDesc(tn *parser.TableName) (*sqlbase.TableDescriptor, error)

// mustGetTableDesc returns a table descriptor, or an error if the descriptor
// is not found.
mustGetTableDesc(tn *parser.TableName) (*sqlbase.TableDescriptor, error)
Expand All @@ -107,8 +119,8 @@ type SchemaAccessor interface {

var _ SchemaAccessor = &planner{}

// getTableDesc implements the SchemaAccessor interface.
func (p *planner) getTableDesc(tn *parser.TableName) (*sqlbase.TableDescriptor, error) {
// getTableOrViewDesc implements the SchemaAccessor interface.
func (p *planner) getTableOrViewDesc(tn *parser.TableName) (*sqlbase.TableDescriptor, error) {
virtual, err := getVirtualTableDesc(tn)
if err != nil || virtual != nil {
return virtual, err
Expand All @@ -130,6 +142,30 @@ func (p *planner) getTableDesc(tn *parser.TableName) (*sqlbase.TableDescriptor,
return &desc, nil
}

// getTableDesc implements the SchemaAccessor interface.
func (p *planner) getTableDesc(tn *parser.TableName) (*sqlbase.TableDescriptor, error) {
desc, err := p.getTableOrViewDesc(tn)
if err != nil {
return desc, err
}
if desc != nil && !desc.IsTable() {
return nil, sqlbase.NewWrongObjectTypeError(tn.String(), "table")
}
return desc, nil
}

// getViewDesc implements the SchemaAccessor interface.
func (p *planner) getViewDesc(tn *parser.TableName) (*sqlbase.TableDescriptor, error) {
desc, err := p.getTableOrViewDesc(tn)
if err != nil {
return desc, err
}
if desc != nil && !desc.IsView() {
return nil, sqlbase.NewWrongObjectTypeError(tn.String(), "view")
}
return desc, nil
}

// mustGetTableDesc implements the SchemaAccessor interface.
func (p *planner) mustGetTableDesc(tn *parser.TableName) (*sqlbase.TableDescriptor, error) {
desc, err := p.getTableDesc(tn)
Expand Down
11 changes: 11 additions & 0 deletions sql/testdata/alter_table
Original file line number Diff line number Diff line change
Expand Up @@ -504,3 +504,14 @@ ALTER TABLE nonexistent SPLIT AT (42)

statement error table "nonexistent" does not exist
ALTER INDEX nonexistent@noindex SPLIT AT (42)

user root

statement ok
CREATE VIEW privsview AS SELECT * FROM privs

statement error pgcode 42809 "privsview" is not a table
ALTER TABLE privsview ADD d INT

statement error pgcode 42809 "privsview" is not a table
ALTER TABLE privsview SPLIT AT (42)
9 changes: 9 additions & 0 deletions sql/testdata/create_index
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,15 @@ t b_asc false 2 c DESC false
statement ok
INSERT INTO t VALUES (1,1,1)

statement error pgcode 42P01 table "foo" does not exist
CREATE INDEX fail ON foo (b DESC)

statement ok
CREATE VIEW v AS SELECT * FROM t

statement error pgcode 42809 "v" is not a table
CREATE INDEX failview ON v (b DESC)

statement ok
CREATE TABLE privs (a INT PRIMARY KEY, b INT)

Expand Down
20 changes: 0 additions & 20 deletions sql/testdata/information_schema
Original file line number Diff line number Diff line change
Expand Up @@ -417,26 +417,6 @@ def other_db xyz BASE TABLE 5

user root

statement ok
GRANT SELECT ON other_db.abc TO testuser

user testuser

query TTTTI colnames
SELECT * FROM information_schema.tables
----
TABLE_CATALOG TABLE_SCHEMA TABLE_NAME TABLE_TYPE VERSION
def information_schema columns SYSTEM VIEW 1
def information_schema schema_privileges SYSTEM VIEW 1
def information_schema schemata SYSTEM VIEW 1
def information_schema table_constraints SYSTEM VIEW 1
def information_schema table_privileges SYSTEM VIEW 1
def information_schema tables SYSTEM VIEW 1
def other_db abc VIEW 2
def other_db xyz BASE TABLE 5

user root

statement ok
DROP DATABASE other_db

Expand Down
9 changes: 9 additions & 0 deletions sql/testdata/rename_table
Original file line number Diff line number Diff line change
Expand Up @@ -144,3 +144,12 @@ SELECT * FROM test2.t2
----
4 16
5 25

statement ok
CREATE VIEW test2.v1 AS SELECT * FROM test2.t2

statement error pgcode 42809 "test2.v1" is not a table
ALTER TABLE test2.v1 RENAME TO test2.v2

statement error pgcode 42809 "test2.v1" is not a table
ALTER TABLE test2.v1 RENAME TO test2.v1
11 changes: 4 additions & 7 deletions sql/testdata/views
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ CREATE VIEW v3 (x) AS select * from t;
statement error pgcode 42601 CREATE VIEW specifies 3 column names, but data source has 2 columns
CREATE VIEW v4 (x, y, z) AS select * from t;

statement ok
CREATE VIEW v5 AS select * from v1;

statement error pgcode 42P01 table "dne" does not exist
CREATE VIEW v6 AS select * from dne;
CREATE VIEW v5 AS select * from dne;

# TODO(a-robinson): Add test that creates view by selecting from another
# view once selecting from views is supported.

statement error pgcode 42809 "v1" is not a table
DROP TABLE v1;
Expand All @@ -40,8 +40,5 @@ DROP VIEW v1;
statement ok
DROP VIEW v2;

statement ok
DROP VIEW v5;

statement ok
DROP TABLE t;

0 comments on commit fde5bdf

Please sign in to comment.