More ALTER TABLE Support

go.mod

@@ -12,7 +12,7 @@ require (
 	github.com/dolthub/dolt/go/gen/proto/dolt/services/eventsapi v0.0.0-20240827111219-e4bb9ca3442d
 	github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2
 	github.com/dolthub/go-icu-regex v0.0.0-20240916130659-0118adc6b662
-	github.com/dolthub/go-mysql-server v0.18.2-0.20241018220726-63ed221b1772
+	github.com/dolthub/go-mysql-server v0.18.2-0.20241021202742-64ebafee5225
 	github.com/dolthub/sqllogictest/go v0.0.0-20240618184124-ca47f9354216
 	github.com/dolthub/vitess v0.0.0-20241016191424-d14e107a654e
 	github.com/fatih/color v1.13.0

go.sum

@@ -226,6 +226,12 @@ github.com/dolthub/go-icu-regex v0.0.0-20240916130659-0118adc6b662 h1:aC17hZD6iw
 github.com/dolthub/go-icu-regex v0.0.0-20240916130659-0118adc6b662/go.mod h1:KPUcpx070QOfJK1gNe0zx4pA5sicIK1GMikIGLKC168=
 github.com/dolthub/go-mysql-server v0.18.2-0.20241018220726-63ed221b1772 h1:ckWYX5OXqrTVXe212Xori7VawOZaC552SJryjDiNrsc=
 github.com/dolthub/go-mysql-server v0.18.2-0.20241018220726-63ed221b1772/go.mod h1:z/GGuH2asedC+lkJA4sx+C3oyRH1HRx8ET6N9AGBVms=
+github.com/dolthub/go-mysql-server v0.18.2-0.20241018225641-706812a88f5c h1:tRNOSEgjv9RGWeB5HYN0wUB69TuzBb8eFlbh/UBfLUY=
+github.com/dolthub/go-mysql-server v0.18.2-0.20241018225641-706812a88f5c/go.mod h1:z/GGuH2asedC+lkJA4sx+C3oyRH1HRx8ET6N9AGBVms=
+github.com/dolthub/go-mysql-server v0.18.2-0.20241021180142-5f71687e5ab9 h1:aJHFkClqIh5xF651f6LjLEKmPuLOY6wSTTOJRRDbdHc=
+github.com/dolthub/go-mysql-server v0.18.2-0.20241021180142-5f71687e5ab9/go.mod h1:z/GGuH2asedC+lkJA4sx+C3oyRH1HRx8ET6N9AGBVms=
+github.com/dolthub/go-mysql-server v0.18.2-0.20241021202742-64ebafee5225 h1:n03pfpHGJaF2COzHj8SHiGC8mJhapnRt9MTzRiX5TZA=
+github.com/dolthub/go-mysql-server v0.18.2-0.20241021202742-64ebafee5225/go.mod h1:z/GGuH2asedC+lkJA4sx+C3oyRH1HRx8ET6N9AGBVms=
 github.com/dolthub/gozstd v0.0.0-20240423170813-23a2903bca63 h1:OAsXLAPL4du6tfbBgK0xXHZkOlos63RdKYS3Sgw/dfI=
 github.com/dolthub/gozstd v0.0.0-20240423170813-23a2903bca63/go.mod h1:lV7lUeuDhH5thVGDCKXbatwKy2KW80L4rMT46n+Y2/Q=
 github.com/dolthub/ishell v0.0.0-20240701202509-2b217167d718 h1:lT7hE5k+0nkBdj/1UOSFwjWpNxf+LCApbRHgnCA17XE=

server/ast/alter_table.go

@@ -79,6 +79,8 @@ func nodeAlterTableCmds(
 			statement, err = nodeAlterTableAddColumn(cmd, tableName, ifExists)
 		case *tree.AlterTableDropColumn:
 			statement, err = nodeAlterTableDropColumn(cmd, tableName, ifExists)
+		case *tree.AlterTableDropConstraint:
+			statement, err = nodeAlterTableDropConstraint(cmd, tableName, ifExists)
 		case *tree.AlterTableRenameColumn:
 			statement, err = nodeAlterTableRenameColumn(cmd, tableName, ifExists)
 		case *tree.AlterTableSetDefault:
@@ -116,6 +118,8 @@ func nodeAlterTableAddConstraint(
 	}
 
 	switch constraintDef := node.ConstraintDef.(type) {
+	case *tree.CheckConstraintTableDef:
+		return nodeCheckConstraintTableDef(constraintDef, tableName, ifExists)
 	case *tree.UniqueConstraintTableDef:
 		return nodeUniqueConstraintTableDef(constraintDef, tableName, ifExists)
 	case *tree.ForeignKeyConstraintTableDef:

server/ast/constraint_table_def.go

@@ -22,6 +22,73 @@ import (
 	"github.com/dolthub/doltgresql/postgres/parser/sem/tree"
 )
 
+// nodeCheckConstraintTableDef converts a tree.CheckConstraintTableDef instance
+// into a vitess.DDL instance that can be executed by GMS. |tableName| identifies
+// the table being altered, and |ifExists| indicates whether the IF EXISTS clause
+// was specified.
+func nodeCheckConstraintTableDef(
+	node *tree.CheckConstraintTableDef,
+	tableName vitess.TableName,
+	ifExists bool) (*vitess.DDL, error) {
+
+	if node.NoInherit {
+		return nil, fmt.Errorf("NO INHERIT is not yet supported for check constraints")
+	}
+
+	expr, err := nodeExpr(node.Expr)
+	if err != nil {
+		return nil, err
+	}
+
+	return &vitess.DDL{
+		Action:           "alter",
+		Table:            tableName,
+		IfExists:         ifExists,
+		ConstraintAction: "add",
+		TableSpec: &vitess.TableSpec{
+			Constraints: []*vitess.ConstraintDefinition{
+				{
+					Name: node.Name.String(),
+					Details: &vitess.CheckConstraintDefinition{
+						Expr:     expr,
+						Enforced: true,
+					},
+				},
+			},
+		},
+	}, nil
+}
+
+// nodeAlterTableDropConstraint converts a tree.AlterTableDropConstraint instance
+// into a vitess.DDL instance that can be executed by GMS. |tableName| identifies
+// the table being altered, and |ifExists| indicates whether the IF EXISTS clause
+// was specified.
+func nodeAlterTableDropConstraint(
+	node *tree.AlterTableDropConstraint,
+	tableName vitess.TableName,
+	ifExists bool) (*vitess.DDL, error) {
+
+	if node.DropBehavior == tree.DropCascade {
+		return nil, fmt.Errorf("CASCADE is not yet supported for drop constraint")
+	}
+
+	if node.IfExists {
+		return nil, fmt.Errorf("IF EXISTS is not yet supported for drop constraint")
+	}
+
+	return &vitess.DDL{
+		Action:           "alter",
+		Table:            tableName,
+		IfExists:         ifExists,
+		ConstraintAction: "drop",
+		TableSpec: &vitess.TableSpec{
+			Constraints: []*vitess.ConstraintDefinition{
+				{Name: node.Constraint.String()},
+			},
+		},
+	}, nil
+}
+
 // nodeUniqueConstraintTableDef converts a tree.UniqueConstraintTableDef instance
 // into a vitess.DDL instance that can be executed by GMS. |tableName| identifies
 // the table being altered, and |ifExists| indicates whether the IF EXISTS clause
@@ -48,18 +115,19 @@ func nodeUniqueConstraintTableDef(
 		return nil, err
 	}
 
+	indexType := "unique"
 	if node.PrimaryKey {
-		return &vitess.DDL{
-			Action:   "alter",
-			Table:    tableName,
-			IfExists: ifExists,
-			IndexSpec: &vitess.IndexSpec{
-				Action:  "create",
-				Type:    "primary",
-				Columns: columns,
-			},
-		}, nil
-	} else {
-		return nil, fmt.Errorf("Only PRIMARY KEY constraints are supported currently")
+		indexType = "primary"
 	}
+
+	return &vitess.DDL{
+		Action:   "alter",
+		Table:    tableName,
+		IfExists: ifExists,
+		IndexSpec: &vitess.IndexSpec{
+			Action:  "create",
+			Type:    indexType,
+			Columns: columns,
+		},
+	}, nil
 }

server/ast/expr.go

@@ -710,6 +710,9 @@ func nodeExpr(node tree.Expr) (vitess.Expr, error) {
 		return retExpr, nil
 	case *tree.StrVal:
 		// TODO: determine what to do when node.WasScannedAsBytes() is true
+		// For string literals, we mark the type as unknown, because Postgres has
+		// more permissive implicit casting rules for literals than it does for strongly
+		// typed values from a schema for example.
 		unknownLiteral := pgexprs.NewUnknownLiteral(node.RawString())
 		return vitess.InjectedExpr{
 			Expression: unknownLiteral,

server/ast/select.go

@@ -16,6 +16,7 @@ package ast
 
 import (
 	"fmt"
+	"strings"
 
 	vitess "github.com/dolthub/vitess/go/vt/sqlparser"
 
@@ -157,10 +158,16 @@ func nodeSelectExpr(node tree.SelectExpr) (vitess.SelectExpr, error) {
 		if ce, ok := expr.(*tree.CastExpr); ok && node.As == "" {
 			node.As = tree.UnrestrictedName(tree.AsString(ce.Expr))
 		}
+		// To be consistent with vitess handling, InputExpression always gets its outer qoutes trimmed
+		inputExpression := tree.AsString(&node)
+		if strings.HasPrefix(inputExpression, "'") && strings.HasSuffix(inputExpression, "'") {
+			inputExpression = inputExpression[1 : len(inputExpression)-1]
+		}
+
 		return &vitess.AliasedExpr{
 			Expr:            vitessExpr,
 			As:              vitess.NewColIdent(string(node.As)),
-			InputExpression: tree.AsString(&node),
+			InputExpression: inputExpression,
 		}, nil
 	}
 }

server/expression/literal.go

@@ -251,7 +251,14 @@ func (l *Literal) Resolved() bool {
 
 // String implements the sql.Expression interface.
 func (l *Literal) String() string {
-	return fmt.Sprintf("%v", l.value)
+	switch litVal := l.value.(type) {
+	case int, int8, int16, int32, int64, uint, uint8, uint16, uint32, uint64:
+		return fmt.Sprintf("%d", litVal)
+	case string:
+		return fmt.Sprintf("'%s'", litVal)
+	default:
+		return fmt.Sprintf("%v", litVal)
+	}
 }
 
 // ToVitessLiteral returns the literal as a Vitess literal. This is strictly for situations where GMS is hardcoded to

server/functions/framework/compiled_function.go

@@ -155,11 +155,7 @@ func (c *CompiledFunction) String() string {
 		if i > 0 {
 			sb.WriteString(", ")
 		}
-		if doltgresType, ok := param.Type().(pgtypes.DoltgresType); ok {
-			sb.WriteString(pgtypes.QuoteString(doltgresType.BaseID(), param.String()))
-		} else {
-			sb.WriteString(param.String())
-		}
+		sb.WriteString(param.String())
 	}
 	sb.WriteString(")")
 	return sb.String()

server/types/utils.go

@@ -16,7 +16,6 @@ package types
 
 import (
 	"fmt"
-	"strings"
 	"time"
 	"unicode/utf8"
 
@@ -25,17 +24,6 @@ import (
 	"github.com/dolthub/vitess/go/vt/proto/query"
 )
 
-// QuoteString will quote the string according to the type given. This means that some types will quote, and others will
-// not, or they may quote in a special way that is unique to that type.
-func QuoteString(baseID DoltgresTypeBaseID, str string) string {
-	switch baseID {
-	case DoltgresTypeBaseID_Char, DoltgresTypeBaseID_Name, DoltgresTypeBaseID_Text, DoltgresTypeBaseID_VarChar, DoltgresTypeBaseID_Unknown:
-		return `'` + strings.ReplaceAll(str, `'`, `''`) + `'`
-	default:
-		return str
-	}
-}
-
 // truncateString returns a string that has been truncated to the given length. Uses the rune count rather than the
 // byte count. Returns the input string if it's smaller than the length. Also returns the rune count of the string.
 func truncateString(val string, runeLimit uint32) (string, uint32) {

testing/go/alter_table_test.go

@@ -57,6 +57,81 @@ func TestAlterTable(t *testing.T) {
 				},
 			},
 		},
+		{
+			Name: "Add Unique Constraint",
+			SetUpScript: []string{
+				"create table t1 (pk int primary key, c1 int);",
+				"insert into t1 values (1,1);",
+				"create table t2 (pk int primary key, c1 int);",
+				"insert into t2 values (1,1);",
+			},
+			Assertions: []ScriptTestAssertion{
+				{
+					// Add a secondary unique index using create index
+					Query:    "CREATE UNIQUE INDEX ON t1(c1);",
+					Expected: []sql.Row{},
+				},
+				{
+					// Test that the unique constraint is working
+					Query:       "INSERT INTO t1 VALUES (2, 1);",
+					ExpectedErr: "unique",
+				},
+				{
+					// Add a secondary unique index using alter table
+					Query:    "ALTER TABLE t2 ADD CONSTRAINT uniq1 UNIQUE (c1);",
+					Expected: []sql.Row{},
+				},
+				{
+					// Test that the unique constraint is working
+					Query:       "INSERT INTO t2 VALUES (2, 1);",
+					ExpectedErr: "unique",
+				},
+			},
+		},
+		{
+			Name: "Add Check Constraint",
+			SetUpScript: []string{
+				"create table t1 (pk int primary key, c1 int);",
+				"insert into t1 values (1,1);",
+			},
+			Assertions: []ScriptTestAssertion{
+				{
+					// Add a check constraint that is already violated by the existing data
+					Query:       "ALTER TABLE t1 ADD CONSTRAINT constraint1 CHECK (c1 > 100);",
+					ExpectedErr: "violated",
+				},
+				{
+					// Add a check constraint
+					Query:    "ALTER TABLE t1 ADD CONSTRAINT constraint1 CHECK (c1 < 100);",
+					Expected: []sql.Row{},
+				},
+				{
+					Query:    "INSERT INTO t1 VALUES (2, 2);",
+					Expected: []sql.Row{},
+				},
+				{
+					Query:       "INSERT INTO t1 VALUES (3, 101);",
+					ExpectedErr: "violated",
+				},
+			},
+		},
+		{
+			Name: "Drop Constraint",
+			SetUpScript: []string{
+				"create table t1 (pk int primary key, c1 int);",
+				"ALTER TABLE t1 ADD CONSTRAINT constraint1 CHECK (c1 > 100);",
+			},
+			Assertions: []ScriptTestAssertion{
+				{
+					Query:    "ALTER TABLE t1 DROP CONSTRAINT constraint1;",
+					Expected: []sql.Row{},
+				},
+				{
+					Query:    "INSERT INTO t1 VALUES (1, 1);",
+					Expected: []sql.Row{},
+				},
+			},
+		},
 		{
 			Name: "Add Primary Key",
 			SetUpScript: []string{