From d960cb095c82a650aa830328b8bb5138be792dab Mon Sep 17 00:00:00 2001
From: Max Hoffman <maximilian.wolfgang1@gmail.com>
Date: Mon, 3 Feb 2025 16:47:52 -0800
Subject: [PATCH] [kvexec] literal types cast to lookup expr type

---
 go/libraries/doltcore/sqle/kvexec/builder.go      |  2 +-
 go/libraries/doltcore/sqle/kvexec/lookup_join.go  | 15 +++++++++++----
 .../doltcore/sqle/kvexec/lookup_join_test.go      | 11 +++++++++++
 3 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/go/libraries/doltcore/sqle/kvexec/builder.go b/go/libraries/doltcore/sqle/kvexec/builder.go
index 7380c6f115f..e982f34f8e0 100644
--- a/go/libraries/doltcore/sqle/kvexec/builder.go
+++ b/go/libraries/doltcore/sqle/kvexec/builder.go
@@ -57,7 +57,7 @@ func (b Builder) Build(ctx *sql.Context, n sql.Node, r sql.Row) (sql.RowIter, er
 			if ita, ok := getIta(n.Right()); ok && len(r) == 0 && simpleLookupExpressions(ita.Expressions()) {
 				if _, _, _, dstIter, _, _, dstTags, dstFilter, err := getSourceKv(ctx, n.Right(), false); err == nil && dstIter != nil {
 					if srcMap, _, srcIter, _, srcSchema, _, srcTags, srcFilter, err := getSourceKv(ctx, n.Left(), true); err == nil && srcSchema != nil {
-						if keyLookupMapper := newLookupKeyMapping(ctx, srcSchema, dstIter.InputKeyDesc(), ita.Expressions(), srcMap.NodeStore()); keyLookupMapper.valid() {
+						if keyLookupMapper, err := newLookupKeyMapping(ctx, srcSchema, dstIter.InputKeyDesc(), ita.Expressions(), ita.Index().ColumnExpressionTypes(), srcMap.NodeStore()); err == nil && keyLookupMapper.valid() {
 							// conditions:
 							// (1) lookup or left lookup join
 							// (2) left-side is something we read KVs from (table or indexscan, ex: no subqueries)
diff --git a/go/libraries/doltcore/sqle/kvexec/lookup_join.go b/go/libraries/doltcore/sqle/kvexec/lookup_join.go
index 65bfebc927a..b36b3995e29 100644
--- a/go/libraries/doltcore/sqle/kvexec/lookup_join.go
+++ b/go/libraries/doltcore/sqle/kvexec/lookup_join.go
@@ -200,7 +200,7 @@ type lookupMapping struct {
 	pool pool.BuffPool
 }
 
-func newLookupKeyMapping(ctx context.Context, sourceSch schema.Schema, tgtKeyDesc val.TupleDesc, keyExprs []sql.Expression, ns tree.NodeStore) *lookupMapping {
+func newLookupKeyMapping(ctx context.Context, sourceSch schema.Schema, tgtKeyDesc val.TupleDesc, keyExprs []sql.Expression, typs []sql.ColumnExpressionType, ns tree.NodeStore) (*lookupMapping, error) {
 	keyless := schema.IsKeyless(sourceSch)
 	// |split| is an index into the schema separating the key and value fields
 	var split int
@@ -223,7 +223,7 @@ func newLookupKeyMapping(ctx context.Context, sourceSch schema.Schema, tgtKeyDes
 			// map the schema order index to the physical storage index
 			col, ok := sourceSch.GetAllCols().LowerNameToCol[strings.ToLower(e.Name())]
 			if !ok {
-				return nil
+				return nil, fmt.Errorf("failed to build lookup mapping, column missing from schema: %s", e.Name())
 			}
 			if col.IsPartOfPK {
 				srcMapping[i] = sourceSch.GetPKCols().TagToIdx[col.Tag]
@@ -242,7 +242,14 @@ func newLookupKeyMapping(ctx context.Context, sourceSch schema.Schema, tgtKeyDes
 	litDesc := val.NewTupleDescriptor(litTypes...)
 	litTb := val.NewTupleBuilder(litDesc)
 	for i, j := range litMappings {
-		tree.PutField(ctx, ns, litTb, i, keyExprs[j].(*expression.Literal).Value())
+		val := keyExprs[j].(*expression.Literal).Value()
+		val, _, err := typs[j].Type.Convert(val)
+		if err != nil {
+			return nil, err
+		}
+		if err := tree.PutField(ctx, ns, litTb, i, val); err != nil {
+			return nil, err
+		}
 	}
 
 	var litTuple val.Tuple
@@ -260,7 +267,7 @@ func newLookupKeyMapping(ctx context.Context, sourceSch schema.Schema, tgtKeyDes
 		targetKb:   val.NewTupleBuilder(tgtKeyDesc),
 		ns:         ns,
 		pool:       ns.Pool(),
-	}
+	}, nil
 }
 
 // valid returns whether the source and destination key types
diff --git a/go/libraries/doltcore/sqle/kvexec/lookup_join_test.go b/go/libraries/doltcore/sqle/kvexec/lookup_join_test.go
index fc252486bdf..ea455153f66 100644
--- a/go/libraries/doltcore/sqle/kvexec/lookup_join_test.go
+++ b/go/libraries/doltcore/sqle/kvexec/lookup_join_test.go
@@ -145,6 +145,17 @@ func TestLookupJoin(t *testing.T) {
 			join:      "select /*+ LOOKUP_JOIN(dolt_diff_xy,dolt_log) */ count(*) from dolt_diff_xy join dolt_log on commit_hash = from_commit",
 			doRowexec: false,
 		},
+		{
+			name: "type conversion panic bug",
+			setup: []string{
+				"create table xy (x int primary key, y int, z varchar(10), key (y,z));",
+				"insert into xy values (0,0,'0'), (1,1,'1');",
+				"create table ab (a int primary key, b int);",
+				"insert into ab values (0,0), (1,1);",
+			},
+			join:      "select /*+ LOOKUP_JOIN(ab,xy) JOIN_ORDER(ab,xy) */ count(*) from xy join ab on y = a and z = 0",
+			doRowexec: true,
+		},
 	}
 
 	for _, tt := range tests {