Improve have_db_index to better handle columns with multiple indexes
#1542
Conversation
The `have_db_index` matcher currently matches on the first index that has the matching column names. But when used with the `unique` qualifier that can result in the "wrong" index being matched causing the test to fail. For example, one unique BTree index, and another non-unique partial or GIN index.
abrom
force-pushed
the
improve-db-index-matcher
branch
from
51750db to
16f7ed8
Compare
| indexes = actual_indexes | ||
|
|
||
| if qualifiers.include?(:unique) | ||
| indexes.sort_by! do |index| | ||
| index.unique == qualifiers[:unique] ? 0 : 1 | ||
| end | ||
| end | ||
|
|
||
| indexes |
There was a problem hiding this comment.
The bang at the end of sort_by modifies the array in place. In-place mutation tends to be useful for performant or complex code, however, given how simple this method is, I wonder if we say things a bit more straightforwardly. What do you think about this?
| indexes = actual_indexes | |
| if qualifiers.include?(:unique) | |
| indexes.sort_by! do |index| | |
| index.unique == qualifiers[:unique] ? 0 : 1 | |
| end | |
| end | |
| indexes | |
| if qualifiers.include?(:unique) | |
| actual_indexes.sort_by do |index| | |
| index.unique == qualifiers[:unique] ? 0 : 1 | |
| end | |
| else | |
| actual_indexes | |
| end |
There was a problem hiding this comment.
Sure. I'd done it using the sort_by! in case there were any new qualifiers added to support applying other sorting to the list. But it can of course be refactored if/when that's the case! Will update.
|
|
||
| indexes | ||
| end | ||
|
|
||
| def actual_indexes |
There was a problem hiding this comment.
Since we now have a new method called sorted_indexes, what do you think about renaming this to unsorted_indexes?
There was a problem hiding this comment.
yeah, sure. Sounds good to me
|
Looks great! Thanks so much! |
The
have_db_indexmatcher currently matches on the first index that has the matching column names. But when used with theuniquequalifier that can result in the "wrong" index being matched causing the test to fail.For example, if a column had multiple indexes.. one unique BTree index, and another non-unique partial or GIN index. Using the
uniquequalifier fail because the non-unique index was identified as thematched_index.It would appear that the order returned by
actual_indexeswill be database specific (say by index name) or possibly pseudo-random??This change applies a "pre sort" of the indexes prior to finding the matching one if the unique qualifier is specified to give the match the best chance of being the "correct" one.
Note that this does of course complicate things if additional qualifiers were to be added because then sort order precedence would need to be considered.