fix: Don't add indices on columns with UNIQUE constraint #113
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
SQL::Translator::Parser::DBIx::Class adds an index to foreign key
columns as this "is normally the sensible thing to do". Agreed.
Besides SQL::Translator::Parser::DBIx::Class takes care to not add an
additional index to primary key columns ("some RDBMS croak on this, and
it generally doesn't make much sense") but doesn't consider columns with
a UNIQUE constraint where it doesn't make any sense either.
Generally this should not be dangerous but it doesn't help at all.
From my understanding the SQL optimizer will use the better index thus
the UNIQUE constraint would win in any case.
Even worse any INSERT, UPDATE or DELETE operation would need to maintain
the index which is never used. This does not sound sensible to me.
The supposed patch avoids the adding of those indices if the column is
checked if it is *the first column* of a UNIQUE constraint. If it's not
the index is added, if it is the first column it is handled like a
primary key.
|
Another consideration in favor of this is the disk space that is taken up by an unnecessary index. BTW, is something wrong with the Travis setup? I tried to look into the supposed failure and the one I picked that said it failed looked like it was displaying the output of a successful build ("all tests passed", and "job exited with 0", etc in the output). The one I looked at was https://travis-ci.org/dbsrgits/dbix-class/jobs/198374714 |
I want to achieve a test checking the given schema for automatic indices on foreign key columns actually being covered by a UNIQUE constraint. Unfortunately I don't understand the difference between this test and t/86sqlt. * I have to "use SQL::Translator" * tables are not populated in SQL::Translator->schema * SQL::Translator->schema->get_table returns undef
Test were failing earlier, thus adding SQL::Translator build dep.
ribasushi
force-pushed
the
maint/0.0828xx
branch
from
f9d765f
to
2e629b8
Compare
ribasushi
force-pushed
the
maint/0.0828xx
branch
2 times, most recently
from
1245f48
to
c11281c
Compare
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.
SQL::Translator::Parser::DBIx::Class adds an index to foreign key
columns as this "is normally the sensible thing to do". Agreed.
Besides SQL::Translator::Parser::DBIx::Class takes care to not add an
additional index to primary key columns ("some RDBMS croak on this, and
it generally doesn't make much sense") but doesn't consider columns with
a UNIQUE constraint where it doesn't make any sense either.
Generally this should not be dangerous but it doesn't help at all.
From my understanding the SQL optimizer will use the better index thus
the UNIQUE constraint would win in any case.
Even worse any INSERT, UPDATE or DELETE operation would need to maintain
the index which is never used. This does not sound sensible to me.
The supposed patch avoids the adding of those indices if the column is
checked if it is the first column of a UNIQUE constraint. If it's not
the index is added, if it is the first column it is handled like a
primary key.