Merge pull request #1437 from dolthub/zachmu/literals-alt

Fixes for prepared statements, removed custom literal type

Author: zachmu

go.mod

@@ -10,7 +10,7 @@ require (
 	github.com/dolthub/dolt/go/gen/proto/dolt/services/eventsapi v0.0.0-20241119094239-f4e529af734d
 	github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2
 	github.com/dolthub/go-icu-regex v0.0.0-20250327004329-6799764f2dad
-	github.com/dolthub/go-mysql-server v0.19.1-0.20250430183344-d3b95b2013dd
+	github.com/dolthub/go-mysql-server v0.19.1-0.20250501222915-ec787601a4cb
 	github.com/dolthub/sqllogictest/go v0.0.0-20240618184124-ca47f9354216
 	github.com/dolthub/vitess v0.0.0-20250430180243-0eee73763bc5
 	github.com/fatih/color v1.13.0
@@ -124,7 +124,7 @@ require (
 	github.com/jackc/pgtype v1.14.0 // indirect
 	github.com/juju/gnuflag v0.0.0-20171113085948-2ce1bb71843d // indirect
 	github.com/kch42/buzhash v0.0.0-20160816060738-9bdec3dec7c6 // indirect
-	github.com/klauspost/compress v1.10.5 // indirect
+	github.com/klauspost/compress v1.10.10 // indirect
 	github.com/klauspost/cpuid/v2 v2.0.12 // indirect
 	github.com/kr/pretty v0.3.0 // indirect
 	github.com/kr/text v0.2.0 // indirect

go.sum

@@ -266,8 +266,8 @@ github.com/dolthub/fslock v0.0.3 h1:iLMpUIvJKMKm92+N1fmHVdxJP5NdyDK5bK7z7Ba2s2U=
 github.com/dolthub/fslock v0.0.3/go.mod h1:QWql+P17oAAMLnL4HGB5tiovtDuAjdDTPbuqx7bYfa0=
 github.com/dolthub/go-icu-regex v0.0.0-20250327004329-6799764f2dad h1:66ZPawHszNu37VPQckdhX1BPPVzREsGgNxQeefnlm3g=
 github.com/dolthub/go-icu-regex v0.0.0-20250327004329-6799764f2dad/go.mod h1:ylU4XjUpsMcvl/BKeRRMXSH7e7WBrPXdSLvnRJYrxEA=
-github.com/dolthub/go-mysql-server v0.19.1-0.20250430183344-d3b95b2013dd h1:WC8c6Wh+kWxw4xNQruFtqkDxTw3PMeFcS57ok3+rUV4=
-github.com/dolthub/go-mysql-server v0.19.1-0.20250430183344-d3b95b2013dd/go.mod h1:pj8c/fGAEdKYy1BZWcN0xO+vEgMRI+kWVXhuEy1BN+Q=
+github.com/dolthub/go-mysql-server v0.19.1-0.20250501222915-ec787601a4cb h1:T+/YvBBXMMl13Mmu0PJzpA7IKv8rqLOOSQMSj/tL+28=
+github.com/dolthub/go-mysql-server v0.19.1-0.20250501222915-ec787601a4cb/go.mod h1:pj8c/fGAEdKYy1BZWcN0xO+vEgMRI+kWVXhuEy1BN+Q=
 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=
@@ -617,8 +617,9 @@ github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+o
 github.com/klauspost/compress v1.8.2/go.mod h1:RyIbtBH6LamlWaDj8nUwkbUhJ87Yi3uG0guNDohfE1A=
 github.com/klauspost/compress v1.9.0/go.mod h1:RyIbtBH6LamlWaDj8nUwkbUhJ87Yi3uG0guNDohfE1A=
 github.com/klauspost/compress v1.9.7/go.mod h1:RyIbtBH6LamlWaDj8nUwkbUhJ87Yi3uG0guNDohfE1A=
-github.com/klauspost/compress v1.10.5 h1:7q6vHIqubShURwQz8cQK6yIe/xC3IF0Vm7TGfqjewrc=
 github.com/klauspost/compress v1.10.5/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs=
+github.com/klauspost/compress v1.10.10 h1:a/y8CglcM7gLGYmlbP/stPE5sR3hbhFRUjCBfd/0B3I=
+github.com/klauspost/compress v1.10.10/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs=
 github.com/klauspost/cpuid v1.2.1/go.mod h1:Pj4uuM528wm8OyEC2QMXAi2YiTZ96dNQPGgoMS4s3ek=
 github.com/klauspost/cpuid/v2 v2.0.12 h1:p9dKCg8i4gmOxtv35DvrYoWqYzQrvEVdjQ762Y0OqZE=
 github.com/klauspost/cpuid/v2 v2.0.12/go.mod h1:g2LTdtYhdyuGPqyWyv7qRAmj1WBqxuObKfj5c0PQa7c=

server/analyzer/serial.go

@@ -22,6 +22,7 @@ import (
 	"github.com/dolthub/dolt/go/libraries/doltcore/doltdb"
 	"github.com/dolthub/go-mysql-server/sql"
 	"github.com/dolthub/go-mysql-server/sql/analyzer"
+	"github.com/dolthub/go-mysql-server/sql/expression"
 	"github.com/dolthub/go-mysql-server/sql/plan"
 	"github.com/dolthub/go-mysql-server/sql/transform"
 
@@ -61,7 +62,7 @@ func ReplaceSerial(ctx *sql.Context, a *analyzer.Analyzer, node sql.Node, scope
 					if strings.ToLower(expr.Name) == "nextval" {
 						seenNextVal = true
 					}
-				case *pgexprs.Literal:
+				case *expression.Literal:
 					placeholderName := fmt.Sprintf("'%s'", ast.DoltCreateTablePlaceholderSequenceName)
 					if expr.String() == placeholderName {
 						isGeneratedFromSequence = true

server/ast/group_by.go

@@ -15,6 +15,7 @@
 package ast
 
 import (
+	"github.com/dolthub/go-mysql-server/sql/expression"
 	vitess "github.com/dolthub/vitess/go/vt/sqlparser"
 
 	pgexprs "github.com/dolthub/doltgresql/server/expression"
@@ -41,8 +42,8 @@ func nodeGroupBy(ctx *Context, node tree.GroupBy) (vitess.GroupBy, error) {
 		// the query, which is not a requirement for Postgres. Whenever we add that functionality, we also need to
 		// remove the dependency on vitess.SQLVal. For now, we'll just convert our literals to a vitess.SQLVal.
 		if injectedExpr, ok := groupBys[i].(vitess.InjectedExpr); ok {
-			if literal, ok := injectedExpr.Expression.(*pgexprs.Literal); ok {
-				groupBys[i] = literal.ToVitessLiteral()
+			if literal, ok := injectedExpr.Expression.(*expression.Literal); ok {
+				groupBys[i] = pgexprs.ToVitessLiteral(literal)
 			}
 		}
 	}

server/ast/limit.go

@@ -16,6 +16,7 @@ package ast
 
 import (
 	"github.com/cockroachdb/errors"
+	"github.com/dolthub/go-mysql-server/sql/expression"
 
 	vitess "github.com/dolthub/vitess/go/vt/sqlparser"
 
@@ -43,7 +44,7 @@ func nodeLimit(ctx *Context, node *tree.Limit) (*vitess.Limit, error) {
 	// GMS is hardcoded to expect vitess.SQLVal for expressions such as `LIMIT 1 OFFSET 1`.
 	// We need to remove the hard dependency, but for now, we'll just convert our literals to a vitess.SQLVal.
 	if injectedExpr, ok := count.(vitess.InjectedExpr); ok {
-		if literal, ok := injectedExpr.Expression.(*pgexprs.Literal); ok {
+		if literal, ok := injectedExpr.Expression.(*expression.Literal); ok {
 			l := literal.Value()
 			limitValue, err := int64ValueForLimit(l)
 			if err != nil {
@@ -54,11 +55,11 @@ func nodeLimit(ctx *Context, node *tree.Limit) (*vitess.Limit, error) {
 				return nil, errors.Errorf("LIMIT must be greater than or equal to 0")
 			}
 
-			count = literal.ToVitessLiteral()
+			count = pgexprs.ToVitessLiteral(literal)
 		}
 	}
 	if injectedExpr, ok := offset.(vitess.InjectedExpr); ok {
-		if literal, ok := injectedExpr.Expression.(*pgexprs.Literal); ok {
+		if literal, ok := injectedExpr.Expression.(*expression.Literal); ok {
 			o := literal.Value()
 			offsetVal, err := int64ValueForLimit(o)
 			if err != nil {
@@ -69,7 +70,7 @@ func nodeLimit(ctx *Context, node *tree.Limit) (*vitess.Limit, error) {
 				return nil, errors.Errorf("OFFSET must be greater than or equal to 0")
 			}
 
-			offset = literal.ToVitessLiteral()
+			offset = pgexprs.ToVitessLiteral(literal)
 		}
 	}
 	return &vitess.Limit{

server/ast/order_by.go

@@ -16,6 +16,7 @@ package ast
 
 import (
 	"github.com/cockroachdb/errors"
+	"github.com/dolthub/go-mysql-server/sql/expression"
 
 	vitess "github.com/dolthub/vitess/go/vt/sqlparser"
 
@@ -69,8 +70,8 @@ func nodeOrderBy(ctx *Context, node tree.OrderBy) (vitess.OrderBy, error) {
 		// the query, which is not a requirement for Postgres. Whenever we add that functionality, we also need to
 		// remove the dependency on vitess.SQLVal. For now, we'll just convert our literals to a vitess.SQLVal.
 		if injectedExpr, ok := expr.(vitess.InjectedExpr); ok {
-			if literal, ok := injectedExpr.Expression.(*pgexprs.Literal); ok {
-				expr = literal.ToVitessLiteral()
+			if literal, ok := injectedExpr.Expression.(*expression.Literal); ok {
+				expr = pgexprs.ToVitessLiteral(literal)
 			}
 		}
 		orderBys[i] = &vitess.Order{

server/ast/select_clause.go

@@ -16,6 +16,7 @@ package ast
 
 import (
 	"github.com/cockroachdb/errors"
+	"github.com/dolthub/go-mysql-server/sql/expression"
 
 	vitess "github.com/dolthub/vitess/go/vt/sqlparser"
 
@@ -141,8 +142,8 @@ PostJoinRewrite:
 								for _, fExpr := range funcExpr.Exprs {
 									if aliasedExpr, ok := fExpr.(*vitess.AliasedExpr); ok {
 										if injectedExpr, ok := aliasedExpr.Expr.(vitess.InjectedExpr); ok {
-											if literal, ok := injectedExpr.Expression.(*pgexprs.Literal); ok {
-												aliasedExpr.Expr = literal.ToVitessLiteral()
+											if literal, ok := injectedExpr.Expression.(*expression.Literal); ok {
+												aliasedExpr.Expr = pgexprs.ToVitessLiteral(literal)
 											}
 										}
 									}

server/connection_data.go

@@ -113,15 +113,32 @@ func extractBindVarTypes(queryPlan sql.Node) ([]uint32, error) {
 
 	types := make([]uint32, 0)
 	var err error
-	extractBindVars := func(expr sql.Expression) bool {
+	var extractBindVars func(n sql.Node, expr sql.Expression) bool
+	extractBindVars = func(n sql.Node, expr sql.Expression) bool {
 		if err != nil {
 			return false
 		}
+
 		switch e := expr.(type) {
+		// Subquery doesn't walk its Node child via Expressions, so we must walk it separately here
+		case *plan.Subquery:
+			transform.InspectExpressionsWithNode(e.Query, extractBindVars)
 		case *expression.BindVar:
 			var typOid uint32
 			if doltgresType, ok := e.Type().(*pgtypes.DoltgresType); ok {
 				typOid = id.Cache().ToOID(doltgresType.ID.AsId())
+			} else if _, ok := e.Type().(sql.DeferredType); ok {
+				// for a deferred type, we can make a guess to its type based on the containing node
+				switch n.(type) {
+				case *plan.Limit:
+					typOid = uint32(oid.T_int4)
+				default:
+					typOid, err = VitessTypeToObjectID(e.Type().Type())
+					if err != nil {
+						err = errors.Errorf("could not determine OID for placeholder %s: %w", e.Name, err)
+						return false
+					}
+				}
 			} else {
 				// TODO: should remove usage non doltgres type
 				typOid, err = VitessTypeToObjectID(e.Type().Type())
@@ -159,11 +176,10 @@ func extractBindVarTypes(queryPlan sql.Node) ([]uint32, error) {
 				return false
 			}
 		}
-
 		return true
 	}
 
-	transform.InspectExpressions(inspectNode, extractBindVars)
+	transform.InspectExpressionsWithNode(inspectNode, extractBindVars)
 	return types, err
 }
 

server/expression/any.go

@@ -19,6 +19,7 @@ import (
 
 	"github.com/cockroachdb/errors"
 	"github.com/dolthub/go-mysql-server/sql"
+	"github.com/dolthub/go-mysql-server/sql/expression"
 	"github.com/dolthub/go-mysql-server/sql/plan"
 
 	"github.com/dolthub/doltgresql/server/functions/framework"
@@ -39,16 +40,16 @@ type AnyExpr struct {
 // subqueryAnyExpr represents the resolved comparison functions for a plan.Subquery.
 type subqueryAnyExpr struct {
 	rightSub      *plan.Subquery
-	staticLiteral *Literal
-	arrayLiterals []*Literal
+	staticLiteral *expression.Literal
+	arrayLiterals []*expression.Literal
 	compFuncs     []framework.Function
 }
 
 // expressionAnyExpr represents the resolved comparison function for a sql.Expression.
 type expressionAnyExpr struct {
 	rightExpr     sql.Expression
-	staticLiteral *Literal
-	arrayLiteral  *Literal
+	staticLiteral *expression.Literal
+	arrayLiteral  *expression.Literal
 	compFunc      framework.Function
 }
 
@@ -130,7 +131,7 @@ func (a *subqueryAnyExpr) eval(ctx *sql.Context, subOperator string, row sql.Row
 		}
 
 		for i := len(a.arrayLiterals); i < len(rightValues); i++ {
-			arrayLiteral := &Literal{typ: a.arrayLiterals[0].typ}
+			arrayLiteral := expression.NewLiteral(nil, a.arrayLiterals[0].Type())
 			a.arrayLiterals = append(a.arrayLiterals, arrayLiteral)
 			compFunc := framework.GetBinaryFunction(op).Compile("internal_any_comparison", a.staticLiteral, a.arrayLiterals[i])
 			a.compFuncs = append(a.compFuncs, compFunc)
@@ -142,9 +143,10 @@ func (a *subqueryAnyExpr) eval(ctx *sql.Context, subOperator string, row sql.Row
 	}
 
 	// Next we'll assign our evaluated values to the expressions that the comparison functions reference
-	a.staticLiteral.value = left
+	// Note that the compiled function has a reference to the staticLiteral and arrayLiterals, so we must alter them in place
+	a.staticLiteral.Val = left
 	for i, rightValue := range rightValues {
-		a.arrayLiterals[i].value = rightValue
+		a.arrayLiterals[i].Val = rightValue
 	}
 	// Now we can loop over all comparison functions, as they'll reference their respective values
 	for _, compFunc := range a.compFuncs {
@@ -192,9 +194,10 @@ func (a *expressionAnyExpr) eval(ctx *sql.Context, row sql.Row, left interface{}
 	}
 
 	// Next we'll assign our evaluated values to the expressions that the comparison function reference
-	a.staticLiteral.value = left
+	// Note that the compiled function has a reference to the staticLiteral and arrayLiteral, so we must alter them in place
+	a.staticLiteral.Val = left
 	for _, rightValue := range rightValues {
-		a.arrayLiteral.value = rightValue
+		a.arrayLiteral.Val = rightValue
 		result, err := a.compFunc.Eval(ctx, row)
 		if err != nil {
 			return nil, err
@@ -293,12 +296,12 @@ func anySubqueryWithChildren(anyExpr *AnyExpr, sub *plan.Subquery) (sql.Expressi
 
 	if leftType, ok := anyExpr.leftExpr.Type().(*pgtypes.DoltgresType); ok {
 		// Resolve comparison functions once and reuse the functions in Eval.
-		staticLiteral := &Literal{typ: leftType}
-		arrayLiterals := make([]*Literal, len(subTypes))
+		staticLiteral := expression.NewLiteral(nil, leftType)
+		arrayLiterals := make([]*expression.Literal, len(subTypes))
 		// Each expression may be a different type (which is valid), so we need a comparison function for each expression.
 		compFuncs := make([]framework.Function, len(subTypes))
 		for i, rightType := range subTypes {
-			arrayLiterals[i] = &Literal{typ: rightType}
+			arrayLiterals[i] = expression.NewLiteral(nil, rightType)
 			compFuncs[i] = framework.GetBinaryFunction(op).Compile("internal_any_comparison", staticLiteral, arrayLiterals[i])
 			if compFuncs[i] == nil {
 				return nil, errors.Errorf("operator does not exist: %s = %s", leftType.String(), rightType.String())
@@ -334,8 +337,8 @@ func anyExpressionWithChildren(anyExpr *AnyExpr) (sql.Expression, error) {
 
 	if leftType, ok := anyExpr.leftExpr.Type().(*pgtypes.DoltgresType); ok {
 		// Resolve comparison function once and reuse the function in Eval.
-		staticLiteral := &Literal{typ: leftType}
-		arrayLiteral := &Literal{typ: rightType}
+		staticLiteral := expression.NewLiteral(nil, leftType)
+		arrayLiteral := expression.NewLiteral(nil, rightType)
 		compFunc := framework.GetBinaryFunction(op).Compile("internal_any_comparison", staticLiteral, arrayLiteral)
 		if compFunc == nil {
 			return nil, errors.Errorf("operator does not exist: %s = %s", leftType.String(), rightType.String())

server/expression/in_subquery.go

@@ -35,8 +35,8 @@ type InSubquery struct {
 
 	// These variables are used so that we can resolve the comparison functions once and reuse them as we iterate over rows.
 	// These are assigned in WithChildren, so refer there for more information.
-	leftLiteral   *Literal
-	rightLiterals []*Literal
+	leftLiteral   *expression.Literal
+	rightLiterals []*expression.Literal
 	compFuncs     []framework.Function
 }
 
@@ -127,9 +127,10 @@ func (in *InSubquery) Eval(ctx *sql.Context, row sql.Row) (any, error) {
 // assigned to |compFuncs| during analysis. If the left value is a single scalar, then |row| has a single value as
 // well. Otherwise, (left is a tuple), |row| has a matching number of values.
 func (in *InSubquery) valuesEqual(ctx *sql.Context, left interface{}, row sql.Row) (bool, error) {
-	in.leftLiteral.value = left
+	// Note that we have to edit the literals in place, since the comparison functions reference them directly.
+	in.leftLiteral.Val = left
 	for i, v := range row {
-		in.rightLiterals[i].value = v
+		in.rightLiterals[i].Val = v
 	}
 
 	for _, compFunc := range in.compFuncs {
@@ -198,8 +199,8 @@ func (in *InSubquery) WithChildren(children ...sql.Expression) (sql.Expression,
 
 		// We need a comparison function for each type in the query result
 		sch := sq.Query.Schema()
-		leftLiteral := &Literal{typ: leftType}
-		rightLiterals := make([]*Literal, len(sch))
+		leftLiteral := expression.NewLiteral(nil, leftType)
+		rightLiterals := make([]*expression.Literal, len(sch))
 		compFuncs := make([]framework.Function, len(sch))
 		allValidChildren := true
 		for i, rightCol := range sch {
@@ -208,7 +209,7 @@ func (in *InSubquery) WithChildren(children ...sql.Expression) (sql.Expression,
 				allValidChildren = false
 				break
 			}
-			rightLiterals[i] = &Literal{typ: rightType}
+			rightLiterals[i] = expression.NewLiteral(nil, rightType)
 			compFuncs[i] = framework.GetBinaryFunction(framework.Operator_BinaryEqual).Compile("internal_in_comparison", leftLiteral, rightLiterals[i])
 			if compFuncs[i] == nil {
 				return nil, errors.Errorf("operator does not exist: %s = %s", leftType.String(), rightType.String())

server/expression/in_tuple.go

@@ -33,8 +33,8 @@ type InTuple struct {
 
 	// These variables are used so that we can resolve the comparison functions once and reuse them as we iterate over rows.
 	// These are assigned in WithChildren, so refer there for more information.
-	staticLiteral *Literal
-	arrayLiterals []*Literal
+	staticLiteral *expression.Literal
+	arrayLiterals []*expression.Literal
 	compFuncs     []framework.Function
 }
 
@@ -112,9 +112,10 @@ func (it *InTuple) Eval(ctx *sql.Context, row sql.Row) (any, error) {
 		}
 	}
 	// Next we'll assign our evaluated values to the expressions that the comparison functions reference
-	it.staticLiteral.value = left
+	// Note that the compiled functions already have a reference to this literal, so we have to edit it in place
+	it.staticLiteral.Val = left
 	for i, rightValue := range rightValues {
-		it.arrayLiterals[i].value = rightValue
+		it.arrayLiterals[i].Val = rightValue
 	}
 
 	// Now we can loop over all of the comparison functions, as they'll reference their respective values
@@ -195,8 +196,8 @@ func (it *InTuple) WithChildren(children ...sql.Expression) (sql.Expression, err
 		// the parameters, and assigning the values to our own literals, we do not have to worry. This offers a
 		// significant speedup as function resolution is very expensive, so we want to do it as few times as possible
 		// (preferably once).
-		staticLiteral := &Literal{typ: leftType}
-		arrayLiterals := make([]*Literal, len(rightTuple))
+		staticLiteral := expression.NewLiteral(nil, leftType)
+		arrayLiterals := make([]*expression.Literal, len(rightTuple))
 		// Each expression may be a different type (which is valid), so we need a comparison function for each expression.
 		compFuncs := make([]framework.Function, len(rightTuple))
 		allValidChildren := true
@@ -206,7 +207,7 @@ func (it *InTuple) WithChildren(children ...sql.Expression) (sql.Expression, err
 				allValidChildren = false
 				break
 			}
-			arrayLiterals[i] = &Literal{typ: rightType}
+			arrayLiterals[i] = expression.NewLiteral(nil, rightType)
 			compFuncs[i] = framework.GetBinaryFunction(framework.Operator_BinaryEqual).Compile("internal_in_comparison", staticLiteral, arrayLiterals[i])
 			if compFuncs[i] == nil {
 				return nil, errors.Errorf("operator does not exist: %s = %s", leftType.String(), rightType.String())