Fix cost model problem where useless INs were not consistently rejected#4000
Draft
alecgrieser wants to merge 19 commits intoFoundationDB:mainfrom
Draft
Fix cost model problem where useless INs were not consistently rejected#4000alecgrieser wants to merge 19 commits intoFoundationDB:mainfrom
alecgrieser wants to merge 19 commits intoFoundationDB:mainfrom
Conversation
… interface definition
One thing that was subtle was that the choice of PickLeft (always returning -1) meant that the comparison in the PlanGenerator was always updating to the new plans when one was available. This flips the comparison so that we now only update the plan if the new plan is strictly better than the old plan. Because the as-yet-best plan is the right operand in the comparator, ties will result in us sticking with that plan.
… few teamscale things
… as the stable selector cost model
…f RewritingCostModel
Collaborator
Author
|
This is built on top of #3993, and it should be rebased once that one is in. |
The idea of this tiebreaker in the cost model is that we don't want to use an IN plan (IN-join or IN-union) if the underlying IN predicate does not participate in some kind of index scan. As written, the previous implementation would only do that if: (1) one of the two plans was not an IN plan or (2) the useless IN plan was on the left. If there was a useful IN plan on the left, then it would consider the two plans equal (moving on to other tiebreakers); if both of the plans were useless INs, then it would always favor the right, despite the fact that this tiebreaker really shouldn't have distinguished the two plans. The new logic now determines if the plan is useless or not, and if it is, it favors the other plan, unless they're both useless, in which case, it moves on to the next tiebreaker. This fixes FoundationDB#3998.
alecgrieser
force-pushed
the
make-in-operator-antisymmetric
branch
from
031e8dd to
7825b85
Compare
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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 idea of this tiebreaker in the cost model is that we don't want to use an IN plan (IN-join or IN-union) if the underlying IN predicate does not participate in some kind of index scan. As written, the previous implementation would only do that if: (1) one of the two plans was not an IN plan or (2) the useless IN plan was on the left. If there was a useful IN plan on the left, then it would consider the two plans equal (moving on to other tiebreakers); if both of the plans were useless INs, then it would always favor the right, despite the fact that this tiebreaker really shouldn't have distinguished the two plans. The new logic now determines if the plan is useless or not, and if it is, it favors the other plan, unless they're both useless, in which case, it moves on to the next tiebreaker.
This fixes #3998.