Merge pull request #19 from liquidata-inc/zachmu/pr-feedback

PR feedback: added engine tests, removed Alterable interface, added comments.

Author: zachmu

engine_test.go

@@ -2443,7 +2443,7 @@ func TestAmbiguousColumnResolution(t *testing.T) {
 	require.Equal(expected, rs)
 }
 
-func TestDDL(t *testing.T) {
+func TestCreateTable(t *testing.T) {
 	require := require.New(t)
 
 	e := newEngine(t)
@@ -2474,6 +2474,83 @@ func TestDDL(t *testing.T) {
 	}
 
 	require.Equal(s, testTable.Schema())
+
+	testQuery(t, e,
+		"CREATE TABLE t2 (a INTEGER NOT NULL PRIMARY KEY, "+
+				"b VARCHAR(10) NOT NULL)",
+		[]sql.Row(nil),
+	)
+
+	db, err = e.Catalog.Database("mydb")
+	require.NoError(err)
+
+	testTable, ok = db.Tables()["t2"]
+	require.True(ok)
+
+	s = sql.Schema{
+		{Name: "a", Type: sql.Int32, Nullable: false, PrimaryKey: true, Source: "t2"},
+		{Name: "b", Type: sql.Text, Nullable: false, Source: "t2"},
+	}
+
+	require.Equal(s, testTable.Schema())
+
+	testQuery(t, e,
+		"CREATE TABLE t3(a INTEGER NOT NULL,"+
+				"b TEXT NOT NULL,"+
+				"c bool, primary key (a,b))",
+		[]sql.Row(nil),
+	)
+
+	db, err = e.Catalog.Database("mydb")
+	require.NoError(err)
+
+	testTable, ok = db.Tables()["t3"]
+	require.True(ok)
+
+	s = sql.Schema{
+		{Name: "a", Type: sql.Int32, Nullable: false, PrimaryKey: true, Source: "t3"},
+		{Name: "b", Type: sql.Text, Nullable: false, PrimaryKey: true, Source: "t3"},
+		{Name: "c", Type: sql.Uint8, Nullable: true, Source: "t3"},
+	}
+
+	require.Equal(s, testTable.Schema())
+}
+
+func TestDropTable(t *testing.T) {
+	require := require.New(t)
+
+	e := newEngine(t)
+	db, err := e.Catalog.Database("mydb")
+	require.NoError(err)
+
+	_, ok := db.Tables()["mytable"]
+	require.True(ok)
+
+	testQuery(t, e,
+		"DROP TABLE IF EXISTS mytable, not_exist",
+		[]sql.Row(nil),
+	)
+
+	_, ok = db.Tables()["mytable"]
+	require.False(ok)
+
+	_, ok = db.Tables()["othertable"]
+	require.True(ok)
+	_, ok = db.Tables()["tabletest"]
+	require.True(ok)
+
+	testQuery(t, e,
+		"DROP TABLE IF EXISTS othertable, tabletest",
+		[]sql.Row(nil),
+	)
+
+	_, ok = db.Tables()["othertable"]
+	require.False(ok)
+	_, ok = db.Tables()["tabletest"]
+	require.False(ok)
+
+	_, _, err = e.Query(newCtx(), "DROP TABLE not_exist")
+	require.Error(err)
 }
 
 func TestNaturalJoin(t *testing.T) {

memory/database.go

@@ -33,18 +33,7 @@ func (d *Database) AddTable(name string, t sql.Table) {
 	d.tables[name] = t
 }
 
-// Create creates a table with the given name and schema
-func (d *Database) Create(name string, schema sql.Schema) error {
-	_, ok := d.tables[name]
-	if ok {
-		return sql.ErrTableAlreadyExists.New(name)
-	}
-
-	d.tables[name] = NewTable(name, schema)
-	return nil
-}
-
-// Create creates a table with the given name and schema
+// CreateTable creates a table with the given name and schema
 func (d *Database) CreateTable(ctx *sql.Context, name string, schema sql.Schema) error {
 	_, ok := d.tables[name]
 	if ok {
@@ -55,6 +44,7 @@ func (d *Database) CreateTable(ctx *sql.Context, name string, schema sql.Schema)
 	return nil
 }
 
+// DropTable drops the table with the given name
 func (d *Database) DropTable(ctx *sql.Context, name string) error {
 	_, ok := d.tables[name]
 	if !ok {

sql/core.go

@@ -230,12 +230,6 @@ type Database interface {
 	Tables() map[string]Table
 }
 
-// DEPRECATED. Use TableCreator and TableDropper.
-// Alterable should be implemented by databases that can handle DDL statements
-type Alterable interface {
-	Create(name string, schema Schema) error
-}
-
 // TableCreator should be implemented by databases that can create new tables.
 type TableCreator interface {
 	CreateTable(ctx *Context, name string, schema Schema) error

sql/plan/ddl.go

@@ -7,7 +7,7 @@ import (
 )
 
 // ErrCreateTable is thrown when the database doesn't support table creation
-var ErrCreateTable = errors.NewKind("tables cannot be created on database %s")
+var ErrCreateTableNotSupported = errors.NewKind("tables cannot be created on database %s")
 var ErrDropTableNotSupported = errors.NewKind("tables cannot be dropped on database %s")
 
 // CreateTable is a node describing the creation of some table.
@@ -17,13 +17,6 @@ type CreateTable struct {
 	schema sql.Schema
 }
 
-// DropTable is a node describing dropping a table
-type DropTable struct {
-	db       sql.Database
-	names    []string
-	ifExists bool
-}
-
 // NewCreateTable creates a new CreateTable node
 func NewCreateTable(db sql.Database, name string, schema sql.Schema) *CreateTable {
 	for _, s := range schema {
@@ -64,13 +57,7 @@ func (c *CreateTable) RowIter(s *sql.Context) (sql.RowIter, error) {
 		return sql.RowsToRowIter(), creatable.CreateTable(s, c.name, c.schema)
 	}
 
-	// TODO: phase out this interface
-	d, ok := c.db.(sql.Alterable)
-	if !ok {
-		return nil, ErrCreateTable.New(c.db.Name())
-	}
-
-	return sql.RowsToRowIter(), d.Create(c.name, c.schema)
+	return nil, ErrCreateTableNotSupported.New(c.db.Name())
 }
 
 // Schema implements the Node interface.
@@ -91,6 +78,13 @@ func (c *CreateTable) String() string {
 	return "CreateTable"
 }
 
+// DropTable is a node describing dropping one or more tables
+type DropTable struct {
+	db       sql.Database
+	names    []string
+	ifExists bool
+}
+
 // NewDropTable creates a new DropTable node
 func NewDropTable(db sql.Database, ifExists bool, tableNames ...string) *DropTable {
 	return &DropTable{