Skip to content

Commit

Permalink
[kvexec] literal types cast to lookup expr type
Browse files Browse the repository at this point in the history
  • Loading branch information
max-hoffman committed Feb 4, 2025
1 parent b4b6d71 commit d960cb0
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 5 deletions.
2 changes: 1 addition & 1 deletion go/libraries/doltcore/sqle/kvexec/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
15 changes: 11 additions & 4 deletions go/libraries/doltcore/sqle/kvexec/lookup_join.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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]
Expand All @@ -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
Expand All @@ -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
Expand Down
11 changes: 11 additions & 0 deletions go/libraries/doltcore/sqle/kvexec/lookup_join_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit d960cb0

Please sign in to comment.