diff --git a/dax/test/dax/dax_test.go b/dax/test/dax/dax_test.go index 6a96354d5..547a02ebe 100644 --- a/dax/test/dax/dax_test.go +++ b/dax/test/dax/dax_test.go @@ -95,6 +95,7 @@ func TestDAXIntegration(t *testing.T) { "testinsert/test-5", // error messages differ "percentile_test/test-6", // related to TODO in orchestrator.executePercentile "innerjointest/innerjoin-aggregate-groupby", // join test which won't work until we support multiple tables + "alterTable/alterTableBadTable", // looks like table does not exist is a different error in DAX } doSkip := func(name string) bool { diff --git a/sql3/planner/compilealtertable.go b/sql3/planner/compilealtertable.go index 15fe6b50a..a8ce0af7b 100644 --- a/sql3/planner/compilealtertable.go +++ b/sql3/planner/compilealtertable.go @@ -3,11 +3,14 @@ package planner import ( + "context" "strings" - "github.com/featurebasedb/featurebase/v3/sql3" - "github.com/featurebasedb/featurebase/v3/sql3/parser" - "github.com/featurebasedb/featurebase/v3/sql3/planner/types" + pilosa "github.com/molecula/featurebase/v3" + "github.com/molecula/featurebase/v3/sql3" + "github.com/molecula/featurebase/v3/sql3/parser" + "github.com/molecula/featurebase/v3/sql3/planner/types" + "github.com/pkg/errors" ) type alterOperation int64 @@ -22,12 +25,43 @@ const ( // PlanOperator. func (p *ExecutionPlanner) compileAlterTableStatement(stmt *parser.AlterTableStatement) (_ types.PlanOperator, err error) { tableName := parser.IdentName(stmt.Name) + + // does the table exist + table, err := p.schemaAPI.IndexInfo(context.Background(), tableName) + if err != nil { + if errors.Is(err, pilosa.ErrIndexNotFound) { + return nil, sql3.NewErrTableNotFound(stmt.Name.NamePos.Line, stmt.Name.NamePos.Column, tableName) + } + return nil, err + } + if stmt.Drop.IsValid() { columnName := parser.IdentName(stmt.DropColumnName) + + // does this column exist + found := false + for _, f := range table.Fields { + if strings.EqualFold(f.Name, columnName) { + found = true + break + } + } + if !found { + return nil, sql3.NewErrColumnNotFound(stmt.DropColumnName.NamePos.Line, stmt.DropColumnName.NamePos.Column, columnName) + } + return NewPlanOpQuery(NewPlanOpAlterTable(p, tableName, alterOpDrop, columnName, "", nil), p.sql), nil } else if stmt.Add.IsValid() { col := stmt.ColumnDef columnName := parser.IdentName(col.Name) + + // does this column exist + for _, f := range table.Fields { + if strings.EqualFold(f.Name, columnName) { + return nil, sql3.NewErrDuplicateColumn(col.Name.NamePos.Line, col.Name.NamePos.Column, columnName) + } + } + column, err := p.compileColumn(col) if err != nil { return nil, err @@ -46,6 +80,7 @@ func (p *ExecutionPlanner) compileAlterTableStatement(stmt *parser.AlterTableSta // analyzeAlterTableStatement analyze an ALTER TABLE statement and returns an // error if anything is invalid. func (p *ExecutionPlanner) analyzeAlterTableStatement(stmt *parser.AlterTableStatement) error { + if stmt.Drop.IsValid() { //no checks for now } else if stmt.Add.IsValid() { diff --git a/sql3/test/defs/defs.go b/sql3/test/defs/defs.go index 45526f59f..9eec1db2d 100644 --- a/sql3/test/defs/defs.go +++ b/sql3/test/defs/defs.go @@ -151,6 +151,7 @@ var TableTests []TableTest = []TableTest{ //create table tests createTable, + alterTable, //joins joinTestsUsers, diff --git a/sql3/test/defs/defs_create_table.go b/sql3/test/defs/defs_create_table.go index fe2767704..b32ed9897 100644 --- a/sql3/test/defs/defs_create_table.go +++ b/sql3/test/defs/defs_create_table.go @@ -46,3 +46,38 @@ var createTable = TableTest{ }, }, } + +var alterTable = TableTest{ + name: "alterTable", + Table: tbl( + "alter_table_test", + srcHdrs( + srcHdr("_id", fldTypeID), + srcHdr("a_int", fldTypeInt), + ), + srcRows(), + ), + SQLTests: []SQLTest{ + { + name: "alterTableBadTable", + SQLs: sqls( + "alter table alter_table_test_foo add column a_int int", + ), + ExpErr: "table 'alter_table_test_foo' not found", + }, + { + name: "alterTableAddExistingCol", + SQLs: sqls( + "alter table alter_table_test add column a_int int", + ), + ExpErr: "duplicate column 'a_int'", + }, + { + name: "alterTableDropNonExistentCol", + SQLs: sqls( + "alter table alter_table_test drop column b_int", + ), + ExpErr: "column 'b_int' not found", + }, + }, +}