diff --git a/server/ast/alter_table.go b/server/ast/alter_table.go index 741d0f380..e31d523a2 100644 --- a/server/ast/alter_table.go +++ b/server/ast/alter_table.go @@ -23,7 +23,7 @@ import ( ) // nodeAlterTable handles *tree.AlterTable nodes. -func nodeAlterTable(ctx *Context, node *tree.AlterTable) (*vitess.AlterTable, error) { +func nodeAlterTable(ctx *Context, node *tree.AlterTable) (vitess.Statement, error) { if node == nil { return nil, nil } @@ -34,11 +34,24 @@ func nodeAlterTable(ctx *Context, node *tree.AlterTable) (*vitess.AlterTable, er return nil, err } - statements, err := nodeAlterTableCmds(ctx, node.Cmds, tableName, node.IfExists) + statements, noOps, err := nodeAlterTableCmds(ctx, node.Cmds, tableName, node.IfExists) if err != nil { return nil, err } + // If there are no valid statements return a no-op statement + if len(noOps) > 0 && len(statements) == 0 { + return NewNoOp(noOps), nil + } + + // Otherwise emit warnings now, then return an AlterTable statement + // TODO: we don't have a way to send or store the warnings alongside a valid AlterTable statement. We could either + // get a *sql.Context here and emit warnings, or we could store the warnings in the Context and make the caller + // emit them before it sends |ReadyForQuery| + // if len(noOps) > 0 { + // emit warnings + // } + return &vitess.AlterTable{ Table: tableName, Statements: statements, @@ -53,54 +66,87 @@ func nodeAlterTableSetSchema(ctx *Context, node *tree.AlterTableSetSchema) (vite return nil, fmt.Errorf("ALTER TABLE SET SCHEMA is not yet supported") } -// nodeAlterTableCmds converts tree.AlterTableCmds into a slice of vitess.DDL -// instances that can be executed by GMS. |tableName| identifies the table -// being altered, and |ifExists| indicates whether the IF EXISTS clause was -// specified. +// nodeAlterTableCmds converts tree.AlterTableCmds into a slice of vitess.DDL instances that can be executed by GMS. +// |tableName| identifies the table being altered, and |ifExists| indicates whether the IF EXISTS clause was specified. +// A second slice of unsupported but safely ignored statements is return as well. func nodeAlterTableCmds( ctx *Context, node tree.AlterTableCmds, tableName vitess.TableName, - ifExists bool) ([]*vitess.DDL, error) { + ifExists bool) ([]*vitess.DDL, []string, error) { if len(node) == 0 { - return nil, fmt.Errorf("no commands specified for ALTER TABLE statement") + return nil, nil, fmt.Errorf("no commands specified for ALTER TABLE statement") } vitessDdlCmds := make([]*vitess.DDL, 0, len(node)) + var unsupportedWarnings []string for _, cmd := range node { var err error var statement *vitess.DDL switch cmd := cmd.(type) { case *tree.AlterTableAddConstraint: statement, err = nodeAlterTableAddConstraint(ctx, cmd, tableName, ifExists) + if err != nil { + return nil, nil, err + } + vitessDdlCmds = append(vitessDdlCmds, statement) case *tree.AlterTableAddColumn: statement, err = nodeAlterTableAddColumn(ctx, cmd, tableName, ifExists) + if err != nil { + return nil, nil, err + } + vitessDdlCmds = append(vitessDdlCmds, statement) case *tree.AlterTableDropColumn: statement, err = nodeAlterTableDropColumn(ctx, cmd, tableName, ifExists) + if err != nil { + return nil, nil, err + } + vitessDdlCmds = append(vitessDdlCmds, statement) case *tree.AlterTableDropConstraint: statement, err = nodeAlterTableDropConstraint(ctx, cmd, tableName, ifExists) + if err != nil { + return nil, nil, err + } + vitessDdlCmds = append(vitessDdlCmds, statement) case *tree.AlterTableRenameColumn: statement, err = nodeAlterTableRenameColumn(ctx, cmd, tableName, ifExists) + if err != nil { + return nil, nil, err + } + vitessDdlCmds = append(vitessDdlCmds, statement) case *tree.AlterTableSetDefault: statement, err = nodeAlterTableSetDefault(ctx, cmd, tableName, ifExists) + if err != nil { + return nil, nil, err + } + vitessDdlCmds = append(vitessDdlCmds, statement) case *tree.AlterTableDropNotNull: statement, err = nodeAlterTableDropNotNull(ctx, cmd, tableName, ifExists) + if err != nil { + return nil, nil, err + } + vitessDdlCmds = append(vitessDdlCmds, statement) case *tree.AlterTableSetNotNull: statement, err = nodeAlterTableSetNotNull(ctx, cmd, tableName, ifExists) + if err != nil { + return nil, nil, err + } + vitessDdlCmds = append(vitessDdlCmds, statement) case *tree.AlterTableAlterColumnType: statement, err = nodeAlterTableAlterColumnType(ctx, cmd, tableName, ifExists) + if err != nil { + return nil, nil, err + } + vitessDdlCmds = append(vitessDdlCmds, statement) + case *tree.AlterTableOwner: + unsupportedWarnings = append(unsupportedWarnings, fmt.Sprintf("ALTER TABLE %s OWNER TO %s", tableName.String(), cmd.Owner)) default: - return nil, fmt.Errorf("ALTER TABLE with unsupported command type %T", cmd) - } - - if err != nil { - return nil, err + return nil, nil, fmt.Errorf("ALTER TABLE with unsupported command type %T", cmd) } - vitessDdlCmds = append(vitessDdlCmds, statement) } - return vitessDdlCmds, nil + return vitessDdlCmds, unsupportedWarnings, nil } // nodeAlterTableAddConstraint converts a tree.AlterTableAddConstraint instance diff --git a/server/ast/no_op.go b/server/ast/no_op.go new file mode 100755 index 000000000..9a8eae8ac --- /dev/null +++ b/server/ast/no_op.go @@ -0,0 +1,31 @@ +// Copyright 2025 Dolthub, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package ast + +import ( + vitess "github.com/dolthub/vitess/go/vt/sqlparser" + + pgnodes "github.com/dolthub/doltgresql/server/node" +) + +// NewNoOp returns a new NoOp statement which does nothing and issues zero or more warnings when run. +// Used for statements that aren't directly supported but which we don't want to cause errors. +func NewNoOp(warnings []string) vitess.InjectedStatement { + return vitess.InjectedStatement{ + Statement: pgnodes.NoOp{ + Warnings: warnings, + }, + } +} diff --git a/server/node/no_op.go b/server/node/no_op.go new file mode 100755 index 000000000..fc2aa5d4b --- /dev/null +++ b/server/node/no_op.go @@ -0,0 +1,79 @@ +// Copyright 2025 Dolthub, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package node + +import ( + "fmt" + "io" + + "github.com/dolthub/go-mysql-server/sql" + vitess "github.com/dolthub/vitess/go/vt/sqlparser" +) + +var _ vitess.Injectable = (*NoOp)(nil) +var _ sql.ExecSourceRel = (*NoOp)(nil) + +// NoOp is a node that does nothing and issues zero or more warnings when run. +// Used when a statement should parse but isn't expected to do anything, for compatibility with Postgres dumps / tools. +type NoOp struct { + Warnings []string +} + +func (n NoOp) Resolved() bool { + return true +} + +func (n NoOp) String() string { + return fmt.Sprintf("%v", n.Warnings) +} + +func (n NoOp) Schema() sql.Schema { + return nil +} + +func (n NoOp) Children() []sql.Node { + return nil +} + +func (n NoOp) WithChildren(children ...sql.Node) (sql.Node, error) { + return n, nil +} + +func (n NoOp) IsReadOnly() bool { + return true +} + +func (n NoOp) WithResolvedChildren(children []any) (any, error) { + return n, nil +} + +type noOpRowIter struct { + warnings []string +} + +func (n noOpRowIter) Next(ctx *sql.Context) (sql.Row, error) { + return nil, io.EOF +} + +func (n noOpRowIter) Close(ctx *sql.Context) error { + for _, warning := range n.warnings { + ctx.Warn(0, fmt.Sprintf("%s is unimplemented", warning)) + } + return nil +} + +func (n NoOp) RowIter(ctx *sql.Context, r sql.Row) (sql.RowIter, error) { + return noOpRowIter{warnings: n.Warnings}, nil +} diff --git a/testing/go/alter_table_test.go b/testing/go/alter_table_test.go index b34a8905d..364cd65d4 100644 --- a/testing/go/alter_table_test.go +++ b/testing/go/alter_table_test.go @@ -417,5 +417,16 @@ func TestAlterTable(t *testing.T) { }, }, }, + { + Name: "alter table owner", + SetUpScript: []string{ + "CREATE TABLE t1 (a INT, b INT);", + }, + Assertions: []ScriptTestAssertion{ + { + Query: "ALTER TABLE t1 OWNER TO new_owner;", // no error is all we expect here + }, + }, + }, }) }