Merge join inclusion correctness #1553
Conversation
max-hoffman
changed the title
sql/analyzer/indexed_joins.go
Outdated
| joinColExprs []*joinColExpr, | ||
| tableAliases TableAliases, | ||
| ) ([]sql.Expression, []bool) { | ||
| func indexMatchesKeyExprs(i sql.Index, joinColExprs []*joinColExpr, tableAliases TableAliases) ([]sql.Expression, []bool, sql.Expression) { |
There was a problem hiding this comment.
Would be super helpful to future maintainers to update the function docs to explain what the new/third return value is.
sql/analyzer/indexed_joins.go
Outdated
| case expression.Comparer, *expression.Literal, *expression.GetField, *expression.And, | ||
| *expression.Tuple, *expression.IsNull, *expression.BindVar: |
There was a problem hiding this comment.
(minor) Are there other edge cases here that are worth handling or noting? Would something like "GetField(x) + Literal(-1)" cause a problem?
There was a problem hiding this comment.
For that example, as long as the get fields are not negated, the value of the expression will be strictly increasing (monotonic). So a big negative literal in the expression value will still increase as we move along the index, as long as we are always adding x. On closer inspection, I should maybe take out the InTuple,Regex, and maybe LessThan Comparer types, which might decrease as x increases.
sql/analyzer/indexed_joins.go
Outdated
| } | ||
| valid := true | ||
| for i, c := range cols { | ||
| if strings.ToLower(idxExprs[i]) != source+"."+c { |
There was a problem hiding this comment.
Do the index expressions have to match cols without any gaps? For example... if we need columns a,b,c, could we still use an index on a,b,b2,c?
sql/analyzer/indexed_joins.go
Outdated
| @@ -432,24 +434,56 @@ func collectJoinConds(attributeSource string, filters ...sql.Expression) []*join | |||
| return conds | |||
| } | |||
|
|
|||
| func collectMergeJoinConds(la, ra string, filters ...sql.Expression) ([]*joinColExpr, []*joinColExpr) { | |||
There was a problem hiding this comment.
This function doesn't seem to be called from anywhere. Is this just dead code?
Merge join was not principled selecting 1) monotonic filters and 2) sorting indexes that aligned with the filter of choice. We also failed to push filters referencing a single table out of join conditions.