-
Notifications
You must be signed in to change notification settings - Fork 4
Eliminate redundant indices #938
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
labkey-jeckels
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment on Luminex Postgres index. Otherwise, tested and looks good.
| -- This index overlaps with uq_analyte_lsid | ||
| DROP INDEX luminex.ix_luminexdatarow_lsid; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uq_analyte_lsid is on a different table. luminex.analyte vs luminex.datarow. Am I missing something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's confusing... but if you look at the PostgreSQL luminex-0.000-23.000.sql script, you'll see:
CREATE INDEX IX_LuminexDataRow_LSID ON luminex.Analyte (LSID);
Based on the name and the fact that the SQL Server script has:
CREATE INDEX IX_LuminexDataRow_LSID ON luminex.DataRow (LSID);
...I concluded that the PostgreSQL definition is flat-out wrong. That's why the new incremental script recreates this index (correctly) after the drop statements. Also why there are six index drops on SQL Server and seven on PostgreSQL (that was my first clue that something must be off).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, interesting. I saw that the scripts were different in this PR but didn't realize it was because the original script was wrong.
Yes, this seems like the correct fixup now. I swear that I saw IX_LuminexDataRow_LSID on datarow when looking directly in the DB, but maybe I was just checking the SQLServer side.
| -- This index overlaps with uq_analyte_lsid | ||
| DROP INDEX luminex.ix_luminexdatarow_lsid; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, interesting. I saw that the scripts were different in this PR but didn't realize it was because the original script was wrong.
Yes, this seems like the correct fixup now. I swear that I saw IX_LuminexDataRow_LSID on datarow when looking directly in the DB, but maybe I was just checking the SQLServer side.
Rationale
Our utility has pointed out some redundant indices in ms2, luminex, and prot