-
Notifications
You must be signed in to change notification settings - Fork 8
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
(VDB-1050) Add missing indexes in public schema #14
Conversation
@@ -8,8 +8,13 @@ ALTER TABLE full_sync_logs | |||
REFERENCES full_sync_receipts (id) | |||
ON DELETE CASCADE; | |||
|
|||
CREATE INDEX full_sync_logs_receipt |
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.
Have we switched to an append-only migration workflow in vDB yet? Migrations like this one make me think we have, but there have also been changes to migrations as recently as a week ago.
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.
We have not, but migrations like this are a product of when we had made an attempt to do so earlier
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.
@@ -17,6 +17,13 @@ CREATE TABLE header_sync_logs | |||
UNIQUE (header_id, tx_index, log_index) | |||
); | |||
|
|||
CREATE INDEX header_sync_logs_address | |||
ON header_sync_logs (address); |
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.
Not introduced by this PR (nor something that needs to be addressed by it), but I'm wondering if address
should be renamed to address_id
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.
Yeah that makes sense. I'll just do it in this PR
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.
Actually nvm, I didn't consider that would entail changes to the transformer repo as well 😅 but I'll take care of it later
c0e4425
to
a20768a
Compare
Add the indexes that postgraphile complains about lacking when run with the
--no-ignore-indexes
flag