Recost LookupJoins to remove special casing for indexes where every column is used in a filter. #1889
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…olumn is used in a filter.
…nique. Rather than changes those tests, I made the index unique.
…al due to a bug, link to the bug. (Linking to the bug is not possible in `query_plans.go` because the tests are generated.)
nicktobey
force-pushed
the
nicktobey/bad-index
branch
from
b2e8c54
to
01dac86
Compare
max-hoffman
reviewed
Jul 25, 2023
| @@ -326,7 +326,7 @@ where | |||
| " │ │ └─ org1 (longtext)\n" + | |||
| " │ └─ TableAlias(dimension)\n" + | |||
| " │ └─ IndexedTableAccess(asset)\n" + | |||
| " │ ├─ index: [asset.orgId,asset.assetId]\n" + | |||
| " │ ├─ index: [asset.orgId,asset.name,asset.val]\n" + | |||
There was a problem hiding this comment.
we should be picking (orgId,name,assetId) here, bit concerned that I missed us changing it to (orgId,assetID)
There was a problem hiding this comment.
It's happening because of the low cardinality of the joined tables.
Basically, when costing non-unique lookups, we assume that each key expression cuts the table in half. If the table is small enough, and if the index has enough key expressions, this can result in a cost lower than the cost of a unique lookup.
As the size of the input tables increases, this effect disappears. I added just four extra rows to the test table and it now chooses the correct index.
max-hoffman
approved these changes
Jul 25, 2023
At extremely low cardinalities, we may pick a non-unique index with more columns than a unique index with fewer columns. This behavior disappears at higher cardinalities, which is where we care more about the performance.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The only joins that are affected by this recosting are joins that use an index where every column in the index has a key expression, but we can't prove that lookups are constant.
We currently special case this, giving these a slightly elevated priority over joins that don't use every column in the index (and we can't prove that lookups are constant.)
So for example a lookup with one key expression on an index with one column ends up being preferred over say, a lookup with three key expressions on an index with four columns, even though it looks like in practice the latter is a more efficient join. Removing this special casing resulted in improved plans in specific client queries.
Most of the changed plans in this PR are workarounds for #1893, #1894, and #1895. A few are actually better plans than we were previously generating.