More dump-related fixes

Hydrocharged · Hydrocharged · commit 9edc4b95b8ff · 2025-10-14T05:02:29.000-07:00
diff --git a/server/ast/alter_database.go b/server/ast/alter_database.go
@@ -26,9 +26,9 @@ func nodeAlterDatabase(ctx *Context, node *tree.AlterDatabase) (vitess.Statement
 		return nil, nil
 	}
 
-	// We can handle the common ALTER DATABASE .. TO OWNER case since it's a no-op
-	if node.Owner != "" {
-		return NewNoOp("owners are unsupported"), nil
+	// We intentionally don't support OWNER TO since we don't support owning objects
+	if node.Owner != "" && len(node.Options) == 0 {
+		return NewNoOp("OWNER TO is unsupported and ignored"), nil
 	}
 
 	return NotYetSupportedError("ALTER DATABASE is not yet supported")
diff --git a/server/ast/alter_function.go b/server/ast/alter_function.go
@@ -27,9 +27,9 @@ func nodeAlterFunction(ctx *Context, node *tree.AlterFunction) (vitess.Statement
 		return nil, err
 	}
 
-	// We can handle the common ALTER FUNCTION .. TO OWNER case since it's a no-op
-	if node.Owner != "" {
-		return NewNoOp("owners are unsupported"), nil
+	// We intentionally don't support OWNER TO since we don't support owning objects
+	if node.Owner != "" && len(node.Options) == 0 {
+		return NewNoOp("OWNER TO is unsupported and ignored"), nil
 	}
 
 	return NotYetSupportedError("ALTER FUNCTION statement is not yet supported")
diff --git a/server/ast/alter_schema.go b/server/ast/alter_schema.go
@@ -26,9 +26,9 @@ func nodeAlterSchema(ctx *Context, node *tree.AlterSchema) (vitess.Statement, er
 		return nil, nil
 	}
 
-	// We can handle the common ALTER SCHEMA .. TO OWNER case since it's a no-op
+	// We intentionally don't support OWNER TO since we don't support owning objects
 	if _, ok := node.Cmd.(*tree.AlterSchemaOwner); ok {
-		return NewNoOp("owners are unsupported"), nil
+		return NewNoOp("OWNER TO is unsupported and ignored"), nil
 	}
 
 	return NotYetSupportedError("ALTER SCHEMA is not yet supported")
diff --git a/server/ast/alter_sequence.go b/server/ast/alter_sequence.go
@@ -32,6 +32,7 @@ func nodeAlterSequence(ctx *Context, node *tree.AlterSequence) (vitess.Statement
 
 	var warnings []string
 	if len(node.Owner) > 0 {
+		// We intentionally don't support OWNER TO since we don't support owning objects
 		if len(node.Options) == 0 {
 			return NewNoOp("OWNER TO is unsupported and ignored"), nil
 		} else {
diff --git a/server/ast/alter_type.go b/server/ast/alter_type.go
@@ -26,9 +26,9 @@ func nodeAlterType(ctx *Context, node *tree.AlterType) (vitess.Statement, error)
 		return nil, nil
 	}
 
-	// We can handle the common ALTER TYPE .. TO OWNER case since it's a no-op
+	// We intentionally don't support OWNER TO since we don't support owning objects
 	if _, ok := node.Cmd.(*tree.AlterTypeOwner); ok {
-		return NewNoOp("owners are unsupported"), nil
+		return NewNoOp("OWNER TO is unsupported and ignored"), nil
 	}
 
 	return NotYetSupportedError("ALTER TYPE is not yet supported")
diff --git a/server/ast/alter_view.go b/server/ast/alter_view.go
@@ -26,9 +26,9 @@ func nodeAlterView(ctx *Context, stmt *tree.AlterView) (sqlparser.Statement, err
 		return nil, nil
 	}
 
-	// We can handle the common ALTER VIEW .. TO OWNER case since it's a no-op
+	// We intentionally don't support OWNER TO since we don't support owning objects
 	if _, ok := stmt.Cmd.(*tree.AlterViewOwnerTo); ok {
-		return NewNoOp("owners are unsupported"), nil
+		return NewNoOp("OWNER TO is unsupported and ignored"), nil
 	}
 
 	return NotYetSupportedError("ALTER VIEW is not yet supported")
diff --git a/server/cast/timestamp.go b/server/cast/timestamp.go
@@ -37,7 +37,11 @@ func timestampAssignment() {
 		FromType: pgtypes.Timestamp,
 		ToType:   pgtypes.Date,
 		Function: func(ctx *sql.Context, val any, targetType *pgtypes.DoltgresType) (any, error) {
-			return pgdate.MakeDateFromTime(val.(time.Time))
+			d, err := pgdate.MakeDateFromTime(val.(time.Time))
+			if err != nil {
+				return nil, err
+			}
+			return d.ToTime()
 		},
 	})
 	framework.MustAddAssignmentTypeCast(framework.TypeCast{
diff --git a/server/cast/timestamptz.go b/server/cast/timestamptz.go
@@ -37,7 +37,11 @@ func timestampTZAssignment() {
 		FromType: pgtypes.TimestampTZ,
 		ToType:   pgtypes.Date,
 		Function: func(ctx *sql.Context, val any, targetType *pgtypes.DoltgresType) (any, error) {
-			return pgdate.MakeDateFromTime(val.(time.Time))
+			d, err := pgdate.MakeDateFromTime(val.(time.Time))
+			if err != nil {
+				return nil, err
+			}
+			return d.ToTime()
 		},
 	})
 	framework.MustAddAssignmentTypeCast(framework.TypeCast{
diff --git a/server/functions/framework/interpreted_function.go b/server/functions/framework/interpreted_function.go
@@ -102,23 +102,10 @@ func (iFunc InterpretedFunction) VariadicIndex() int {
 }
 
 // QuerySingleReturn handles queries that are supposed to return a single value.
-func (InterpretedFunction) QuerySingleReturn(ctx *sql.Context, stack plpgsql.InterpreterStack, stmt string, targetType *pgtypes.DoltgresType, bindings []string) (val any, err error) {
-	if len(bindings) > 0 {
-		for i, bindingName := range bindings {
-			variable := stack.GetVariable(bindingName)
-			if variable.Type == nil {
-				return nil, fmt.Errorf("variable `%s` could not be found", bindingName)
-			}
-			formattedVar, err := variable.Type.FormatValue(*variable.Value)
-			if err != nil {
-				return nil, err
-			}
-			switch variable.Type.TypCategory {
-			case pgtypes.TypeCategory_ArrayTypes, pgtypes.TypeCategory_StringTypes:
-				formattedVar = pq.QuoteLiteral(formattedVar)
-			}
-			stmt = strings.Replace(stmt, "$"+strconv.Itoa(i+1), formattedVar, 1)
-		}
+func (iFunc InterpretedFunction) QuerySingleReturn(ctx *sql.Context, stack plpgsql.InterpreterStack, stmt string, targetType *pgtypes.DoltgresType, bindings []string) (val any, err error) {
+	stmt, _, err = iFunc.ApplyBindings(ctx, stack, stmt, bindings, true)
+	if err != nil {
+		return nil, err
 	}
 	return sql.RunInterpreted(ctx, func(subCtx *sql.Context) (any, error) {
 		sch, rowIter, _, err := stack.Runner().QueryWithBindings(subCtx, stmt, nil, nil, nil)
@@ -177,23 +164,10 @@ func (InterpretedFunction) QuerySingleReturn(ctx *sql.Context, stack plpgsql.Int
 }
 
 // QueryMultiReturn handles queries that may return multiple values over multiple rows.
-func (InterpretedFunction) QueryMultiReturn(ctx *sql.Context, stack plpgsql.InterpreterStack, stmt string, bindings []string) (rows []sql.Row, err error) {
-	if len(bindings) > 0 {
-		for i, bindingName := range bindings {
-			variable := stack.GetVariable(bindingName)
-			if variable.Type == nil {
-				return nil, fmt.Errorf("variable `%s` could not be found", bindingName)
-			}
-			formattedVar, err := variable.Type.FormatValue(*variable.Value)
-			if err != nil {
-				return nil, err
-			}
-			switch variable.Type.TypCategory {
-			case pgtypes.TypeCategory_ArrayTypes, pgtypes.TypeCategory_StringTypes:
-				formattedVar = pq.QuoteLiteral(formattedVar)
-			}
-			stmt = strings.Replace(stmt, "$"+strconv.Itoa(i+1), formattedVar, 1)
-		}
+func (iFunc InterpretedFunction) QueryMultiReturn(ctx *sql.Context, stack plpgsql.InterpreterStack, stmt string, bindings []string) (rows []sql.Row, err error) {
+	stmt, _, err = iFunc.ApplyBindings(ctx, stack, stmt, bindings, true)
+	if err != nil {
+		return nil, err
 	}
 	return sql.RunInterpreted(ctx, func(subCtx *sql.Context) ([]sql.Row, error) {
 		_, rowIter, _, err := stack.Runner().QueryWithBindings(subCtx, stmt, nil, nil, nil)
@@ -207,5 +181,43 @@ func (InterpretedFunction) QueryMultiReturn(ctx *sql.Context, stack plpgsql.Inte
 	})
 }
 
+// ApplyBindings applies the given bindings to the statement. If `varFound` is false, then the error will be state that
+// the variable was not found (which means the error may be ignored if you're only concerned with finding a variable).
+// If `varFound` is true, then the error is related to formatting the variable. `enforceType` adds casting and quotes to
+// ensure that the value is correctly represented in the string.
+func (InterpretedFunction) ApplyBindings(ctx *sql.Context, stack plpgsql.InterpreterStack, stmt string, bindings []string, enforceType bool) (newStmt string, varFound bool, err error) {
+	if len(bindings) == 0 {
+		return stmt, false, nil
+	}
+	newStmt = stmt
+	for i, bindingName := range bindings {
+		variable := stack.GetVariable(bindingName)
+		if variable.Type == nil {
+			return newStmt, false, fmt.Errorf("variable `%s` could not be found", bindingName)
+		}
+		var formattedVar string
+		if *variable.Value != nil {
+			formattedVar, err = variable.Type.FormatValue(*variable.Value)
+			if err != nil {
+				return newStmt, true, err
+			}
+			if enforceType {
+				switch variable.Type.TypCategory {
+				case pgtypes.TypeCategory_ArrayTypes, pgtypes.TypeCategory_DateTimeTypes, pgtypes.TypeCategory_StringTypes:
+					formattedVar = pq.QuoteLiteral(formattedVar)
+				}
+			}
+		} else {
+			formattedVar = "NULL"
+		}
+		if enforceType {
+			newStmt = strings.Replace(newStmt, "$"+strconv.Itoa(i+1), fmt.Sprintf(`(%s)::%s`, formattedVar, variable.Type.String()), 1)
+		} else {
+			newStmt = strings.Replace(newStmt, "$"+strconv.Itoa(i+1), formattedVar, 1)
+		}
+	}
+	return newStmt, true, nil
+}
+
 // enforceInterfaceInheritance implements the interface FunctionInterface.
 func (iFunc InterpretedFunction) enforceInterfaceInheritance(error) {}
diff --git a/server/plpgsql/interpreter_logic.go b/server/plpgsql/interpreter_logic.go
@@ -19,6 +19,7 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/cockroachdb/errors"
 	"github.com/dolthub/dolt/go/libraries/doltcore/sqle/dsess"
 	"github.com/dolthub/go-mysql-server/sql"
 	"github.com/jackc/pgx/v5/pgproto3"
@@ -32,6 +33,7 @@ import (
 // InterpretedFunction is an interface that essentially mirrors the implementation of InterpretedFunction in the
 // framework package.
 type InterpretedFunction interface {
+	ApplyBindings(ctx *sql.Context, stack InterpreterStack, stmt string, bindings []string, enforceType bool) (newStmt string, varFound bool, err error)
 	GetParameters() []*pgtypes.DoltgresType
 	GetParameterNames() []string
 	GetReturn() *pgtypes.DoltgresType
@@ -207,23 +209,25 @@ func call(ctx *sql.Context, iFunc InterpretedFunction, stack InterpreterStack) (
 			//       notice levels to send to the client.
 			// https://www.postgresql.org/docs/current/runtime-config-client.html#GUC-CLIENT-MIN-MESSAGES
 
-			// TODO: Notices at the EXCEPTION level should also abort the current tx.
-
 			message, err := evaluteNoticeMessage(ctx, iFunc, operation, stack)
 			if err != nil {
 				return nil, err
 			}
 
-			noticeResponse := &pgproto3.NoticeResponse{
-				Severity: operation.PrimaryData,
-				Message:  message,
-			}
-
-			if err = applyNoticeOptions(ctx, noticeResponse, operation.Options); err != nil {
-				return nil, err
+			if operation.PrimaryData == "EXCEPTION" {
+				// TODO: Notices at the EXCEPTION level should also abort the current tx.
+				return nil, errors.New(message)
+			} else {
+				noticeResponse := &pgproto3.NoticeResponse{
+					Severity: operation.PrimaryData,
+					Message:  message,
+				}
+				if err = applyNoticeOptions(ctx, noticeResponse, operation.Options); err != nil {
+					return nil, err
+				}
+				sess := dsess.DSessFromSess(ctx.Session)
+				sess.Notice(noticeResponse)
 			}
-			sess := dsess.DSessFromSess(ctx.Session)
-			sess.Notice(noticeResponse)
 		case OpCode_Return:
 			if len(operation.PrimaryData) == 0 {
 				return nil, nil
@@ -295,22 +299,36 @@ func evaluteNoticeMessage(ctx *sql.Context, iFunc InterpretedFunction,
 	message := operation.SecondaryData[0]
 	if len(operation.SecondaryData) > 1 {
 		params := operation.SecondaryData[1:]
-		currentParam := 0
+		currentParamIdx := 0
 
 		parts := strings.Split(message, "%%")
 		for i, part := range parts {
 			for strings.Contains(part, "%") {
-				retVal, err := iFunc.QuerySingleReturn(ctx, stack, "SELECT "+params[currentParam], nil, nil)
-				if err != nil {
-					return "", err
+				if currentParamIdx >= len(params) {
+					return "", errors.New("too few parameters specified for RAISE")
+				}
+				currentParam := params[currentParamIdx]
+				currentParamIdx += 1
+				formattedVar, varFound, err := iFunc.ApplyBindings(ctx, stack, "$1", []string{currentParam}, false)
+				if varFound {
+					if err != nil {
+						return "", err
+					}
+					part = strings.Replace(part, "%", formattedVar, 1)
+				} else {
+					retVal, err := iFunc.QuerySingleReturn(ctx, stack, fmt.Sprintf("SELECT (%s)::text", currentParam), nil, nil)
+					if err != nil {
+						return "", err
+					}
+					stringVal := fmt.Sprintf("%v", retVal) // We should always return a string, but this is just a safety net
+					part = strings.Replace(part, "%", stringVal, 1)
 				}
-				currentParam += 1
-
-				s := fmt.Sprintf("%v", retVal)
-				part = strings.Replace(part, "%", s, 1)
 			}
 			parts[i] = part
 		}
+		if currentParamIdx < len(params) {
+			return "", errors.New("too many parameters specified for RAISE")
+		}
 		message = strings.Join(parts, "%")
 	}
 	return message, nil
diff --git a/server/plpgsql/json.go b/server/plpgsql/json.go
@@ -411,7 +411,11 @@ func (stmt *plpgSQL_stmt_exit) Convert() Statement {
 func (stmt *plpgSQL_stmt_if) Convert() (Block, error) {
 	// We store all GOTOs that will need to go to the end of the block. Since we can't know that ahead of time, we store
 	// their indexes and set them at the end of the function.
-	var gotoEndIndexes []int32
+	type gotoEndIndex struct {
+		BodyIndex int
+		GotoIndex int32
+	}
+	var gotoEndIndexes []gotoEndIndex
 	returnBlock := Block{
 		Body: []Statement{
 			If{
@@ -431,7 +435,10 @@ func (stmt *plpgSQL_stmt_if) Convert() (Block, error) {
 	returnBlock.Body = append(returnBlock.Body, thenStmts...)
 	// Then we want to append the GOTO that finishes the THEN block, but we don't know the end just yet, so we'll save
 	// its index and fill it in later
-	gotoEndIndexes = append(gotoEndIndexes, OperationSizeForStatements(returnBlock.Body))
+	gotoEndIndexes = append(gotoEndIndexes, gotoEndIndex{
+		BodyIndex: len(returnBlock.Body),
+		GotoIndex: OperationSizeForStatements(returnBlock.Body),
+	})
 	returnBlock.Body = append(returnBlock.Body, Goto{})
 	// We repeat the same process for each ELSIF statement (refer to the comments above)
 	for _, elseIf := range stmt.ElseIf {
@@ -445,7 +452,10 @@ func (stmt *plpgSQL_stmt_if) Convert() (Block, error) {
 		}
 		returnBlock.Body = append(returnBlock.Body, Goto{Offset: OperationSizeForStatements(elseIfStmts) + 2})
 		returnBlock.Body = append(returnBlock.Body, elseIfStmts...)
-		gotoEndIndexes = append(gotoEndIndexes, OperationSizeForStatements(returnBlock.Body))
+		gotoEndIndexes = append(gotoEndIndexes, gotoEndIndex{
+			BodyIndex: len(returnBlock.Body),
+			GotoIndex: OperationSizeForStatements(returnBlock.Body),
+		})
 		returnBlock.Body = append(returnBlock.Body, Goto{})
 	}
 	// Finally we handle our ELSE statements. We don't have a condition to check, so we don't have to append any
@@ -457,8 +467,8 @@ func (stmt *plpgSQL_stmt_if) Convert() (Block, error) {
 	returnBlock.Body = append(returnBlock.Body, elseStmts...)
 	// Now we'll set all of our GOTOs so that they skip to the end of the block.
 	// We have to take their index position into account, since we want to skip to the end from their relative position.
-	for _, gotoEndIndex := range gotoEndIndexes {
-		returnBlock.Body[gotoEndIndex] = Goto{Offset: OperationSizeForStatements(returnBlock.Body) - gotoEndIndex}
+	for _, idx := range gotoEndIndexes {
+		returnBlock.Body[idx.BodyIndex] = Goto{Offset: OperationSizeForStatements(returnBlock.Body) - idx.GotoIndex}
 	}
 	return returnBlock, nil
 }
diff --git a/testing/go/alter_test.go b/testing/go/alter_test.go
@@ -28,7 +28,7 @@ func TestAlterStatements(t *testing.T) {
 					ExpectedNotices: []ExpectedNotice{
 						{
 							Severity: "WARNING",
-							Message:  "owners are unsupported",
+							Message:  "OWNER TO is unsupported and ignored",
 						},
 					},
 				},
@@ -62,7 +62,7 @@ func TestAlterStatements(t *testing.T) {
 					ExpectedNotices: []ExpectedNotice{
 						{
 							Severity: "WARNING",
-							Message:  "owners are unsupported",
+							Message:  "OWNER TO is unsupported and ignored",
 						},
 					},
 				},
@@ -79,7 +79,7 @@ func TestAlterStatements(t *testing.T) {
 					ExpectedNotices: []ExpectedNotice{
 						{
 							Severity: "WARNING",
-							Message:  "owners are unsupported",
+							Message:  "OWNER TO is unsupported and ignored",
 						},
 					},
 				},
@@ -96,7 +96,7 @@ func TestAlterStatements(t *testing.T) {
 					ExpectedNotices: []ExpectedNotice{
 						{
 							Severity: "WARNING",
-							Message:  "owners are unsupported",
+							Message:  "OWNER TO is unsupported and ignored",
 						},
 					},
 				},
@@ -113,7 +113,7 @@ func TestAlterStatements(t *testing.T) {
 					ExpectedNotices: []ExpectedNotice{
 						{
 							Severity: "WARNING",
-							Message:  "owners are unsupported",
+							Message:  "OWNER TO is unsupported and ignored",
 						},
 					},
 				},
diff --git a/testing/go/create_function_plpgsql_test.go b/testing/go/create_function_plpgsql_test.go

Original file line number	Diff line number	Diff line change
`@@ -26,9 +26,9 @@ func nodeAlterDatabase(ctx Context, node tree.AlterDatabase) (vitess.Statement`
`26`	`26`	`return nil, nil`
`27`	`27`	`}`
`28`	`28`
`29`		`- // We can handle the common ALTER DATABASE .. TO OWNER case since it's a no-op`
`30`		`- if node.Owner != "" {`
`31`		`- return NewNoOp("owners are unsupported"), nil`
	`29`	`+ // We intentionally don't support OWNER TO since we don't support owning objects`
	`30`	`+ if node.Owner != "" && len(node.Options) == 0 {`
	`31`	`+ return NewNoOp("OWNER TO is unsupported and ignored"), nil`
`32`	`32`	`}`
`33`	`33`
`34`	`34`	`return NotYetSupportedError("ALTER DATABASE is not yet supported")`
Original file line number	Diff line number	Diff line change
`@@ -27,9 +27,9 @@ func nodeAlterFunction(ctx Context, node tree.AlterFunction) (vitess.Statement`
`27`	`27`	`return nil, err`
`28`	`28`	`}`
`29`	`29`
`30`		`- // We can handle the common ALTER FUNCTION .. TO OWNER case since it's a no-op`
`31`		`- if node.Owner != "" {`
`32`		`- return NewNoOp("owners are unsupported"), nil`
	`30`	`+ // We intentionally don't support OWNER TO since we don't support owning objects`
	`31`	`+ if node.Owner != "" && len(node.Options) == 0 {`
	`32`	`+ return NewNoOp("OWNER TO is unsupported and ignored"), nil`
`33`	`33`	`}`
`34`	`34`
`35`	`35`	`return NotYetSupportedError("ALTER FUNCTION statement is not yet supported")`
Original file line number	Diff line number	Diff line change
`@@ -26,9 +26,9 @@ func nodeAlterSchema(ctx Context, node tree.AlterSchema) (vitess.Statement, er`
`26`	`26`	`return nil, nil`
`27`	`27`	`}`
`28`	`28`
`29`		`- // We can handle the common ALTER SCHEMA .. TO OWNER case since it's a no-op`
	`29`	`+ // We intentionally don't support OWNER TO since we don't support owning objects`
`30`	`30`	`if _, ok := node.Cmd.(*tree.AlterSchemaOwner); ok {`
`31`		`- return NewNoOp("owners are unsupported"), nil`
	`31`	`+ return NewNoOp("OWNER TO is unsupported and ignored"), nil`
`32`	`32`	`}`
`33`	`33`
`34`	`34`	`return NotYetSupportedError("ALTER SCHEMA is not yet supported")`
Original file line number	Diff line number	Diff line change
`@@ -26,9 +26,9 @@ func nodeAlterType(ctx Context, node tree.AlterType) (vitess.Statement, error)`
`26`	`26`	`return nil, nil`
`27`	`27`	`}`
`28`	`28`
`29`		`- // We can handle the common ALTER TYPE .. TO OWNER case since it's a no-op`
	`29`	`+ // We intentionally don't support OWNER TO since we don't support owning objects`
`30`	`30`	`if _, ok := node.Cmd.(*tree.AlterTypeOwner); ok {`
`31`		`- return NewNoOp("owners are unsupported"), nil`
	`31`	`+ return NewNoOp("OWNER TO is unsupported and ignored"), nil`
`32`	`32`	`}`
`33`	`33`
`34`	`34`	`return NotYetSupportedError("ALTER TYPE is not yet supported")`
Original file line number	Diff line number	Diff line change
`@@ -26,9 +26,9 @@ func nodeAlterView(ctx Context, stmt tree.AlterView) (sqlparser.Statement, err`
`26`	`26`	`return nil, nil`
`27`	`27`	`}`
`28`	`28`
`29`		`- // We can handle the common ALTER VIEW .. TO OWNER case since it's a no-op`
	`29`	`+ // We intentionally don't support OWNER TO since we don't support owning objects`
`30`	`30`	`if _, ok := stmt.Cmd.(*tree.AlterViewOwnerTo); ok {`
`31`		`- return NewNoOp("owners are unsupported"), nil`
	`31`	`+ return NewNoOp("OWNER TO is unsupported and ignored"), nil`
`32`	`32`	`}`
`33`	`33`
`34`	`34`	`return NotYetSupportedError("ALTER VIEW is not yet supported")`