Skip to content

Commit f00ff8d

Browse files
craig[bot]mw5h
andcommitted
Merge #152399
152399: lookup join: add a cast when mapping references in computed columns r=mw5h a=mw5h Previously, when using a lookup join on an index with a virtual column, we would require the references inside that virtual column to be typed identically on the left and right side. This was a change from our previous behavior, which was to require they be equivalent, but not identical. This tightening of the requirements was done to address issue 124732, which pertains to virtual columns using functions that are senstive to the actual type of the input (e.g. 'pg_typeof'). By tightening the reference requirement, we excluded a set of queries from using lookup joins that had previously been able to do so safely, regressing their performance. In this patch, we add a cast around the left hand side reference to the right hand side's type, so that type sensitive functions return the correct type. This allows us to go back to the old behavior of allowing the types of these references to be equivalent rather than identical. Fixes: #124732 Release note (performance improvement): Lookup joins can now be used on tables with virtual columns even if the type of the search argument is not identical to the column type referenced in the virtual column. Co-authored-by: Matt White <[email protected]>
2 parents 49f8c70 + 00f720c commit f00ff8d

File tree

3 files changed

+25
-15
lines changed

3 files changed

+25
-15
lines changed

pkg/sql/logictest/testdata/logic_test/lookup_join

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1708,8 +1708,10 @@ SELECT col1_6 FROM table_1_124732 INNER HASH JOIN table_3_124732 ON col3_0 = col
17081708
----
17091709
-
17101710

1711-
statement error pgcode XXUUU pq: could not produce a query plan conforming to the LOOKUP JOIN hint
1711+
query T
17121712
SELECT col1_6 FROM table_1_124732 INNER LOOKUP JOIN table_3_124732 ON col3_0 = col1_6;
1713+
----
1714+
-
17131715

17141716
# Regression test for incorrectly remapping columns in a composite-sensitive
17151717
# expression to produce a lookup join (#124732).

pkg/sql/opt/lookupjoin/constraint_builder.go

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -246,17 +246,6 @@ func (b *ConstraintBuilder) Build(
246246
rightSideCols = append(rightSideCols, rightCol)
247247
}
248248

249-
// rightEqIdenticalTypeCols is the set of columns in rightEq that have
250-
// identical types to the corresponding columns in leftEq. This is used to
251-
// determine if a computed column can be synthesized for a column in the
252-
// index in order to allow a lookup join.
253-
var rightEqIdenticalTypeCols opt.ColSet
254-
for i := range rightEq {
255-
if b.md.ColumnMeta(rightEq[i]).Type.Identical(b.md.ColumnMeta(leftEq[i]).Type) {
256-
rightEqIdenticalTypeCols.Add(rightEq[i])
257-
}
258-
}
259-
260249
// All the lookup conditions must apply to the prefix of the index and so
261250
// the projected columns created must be created in order.
262251
for j := 0; j < numIndexKeyCols; j++ {
@@ -281,7 +270,7 @@ func (b *ConstraintBuilder) Build(
281270
//
282271
// NOTE: we must only consider equivalent columns with identical types,
283272
// since column remapping is otherwise not valid.
284-
if expr, ok := b.findComputedColJoinEquality(b.table, idxCol, rightEqIdenticalTypeCols); ok {
273+
if expr, ok := b.findComputedColJoinEquality(b.table, idxCol, rightEq.ToSet()); ok {
285274
colMeta := b.md.ColumnMeta(idxCol)
286275
compEqCol := b.md.AddColumn(fmt.Sprintf("%s_eq", colMeta.Alias), colMeta.Type)
287276

@@ -294,7 +283,22 @@ func (b *ConstraintBuilder) Build(
294283

295284
// Project the computed column expression, mapping all columns
296285
// in rightEq to corresponding columns in leftEq.
297-
projection := b.f.ConstructProjectionsItem(b.f.RemapCols(expr, b.eqColMap), compEqCol)
286+
var replace norm.ReplaceFunc
287+
replace = func(e opt.Expr) opt.Expr {
288+
if v, ok := e.(*memo.VariableExpr); ok {
289+
if col, ok := b.eqColMap.Get(int(v.Col)); ok {
290+
// If the column is a computed column, we need to
291+
// cast it to the type of the original column so that
292+
// functions which are type sensitive still return the
293+
// expected results.
294+
return b.f.ConstructCast(b.f.ConstructVariable(opt.ColumnID(col)), b.md.ColumnMeta(v.Col).Type)
295+
} else {
296+
return e
297+
}
298+
}
299+
return b.f.Replace(e, replace)
300+
}
301+
projection := b.f.ConstructProjectionsItem(b.f.Replace(expr, replace).(opt.ScalarExpr), compEqCol)
298302
inputProjections = append(inputProjections, projection)
299303
addEqualityColumns(compEqCol, idxCol)
300304
derivedEquivCols.Add(compEqCol)

pkg/sql/opt/lookupjoin/testdata/computed

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,11 @@ lookup expression:
175175
lookup-constraints left=(a regclass, b int) right=(x oid, v string not null as (x::string) stored) index=(v, x)
176176
x = a
177177
----
178-
lookup join not possible
178+
key cols:
179+
v = v_eq
180+
x = a
181+
input projections:
182+
v_eq = a::OID::STRING [type=STRING]
179183

180184
# Computed columns cannot be remapped if the expression is composite-sensitive.
181185
lookup-constraints left=(a decimal, b int) right=(x decimal, v int not null as (x::int) stored) index=(v, x)

0 commit comments

Comments
 (0)