-
Notifications
You must be signed in to change notification settings - Fork 186
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
chore(database): indices for columns used in 'where' and 'order by' clauses #3880
base: main
Are you sure you want to change the base?
Changes from 1 commit
872b524
4eb88eb
54a0acb
89efb5d
10602a0
885df00
4bf140d
f033a79
7ccbcf1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,92 @@ | ||
import { Knex } from 'knex' | ||
|
||
export async function up(knex: Knex): Promise<void> { | ||
await knex.schema.alterTable('stream_commits', (table) => { | ||
// table.index('streamId') // index should already be available due to this being the first item in the composite primary key | ||
table.index('commitId') | ||
}) | ||
await knex.schema.alterTable('branch_commits', (table) => { | ||
// table.index('branchId') // index should already be available due to this being the first item in the composite primary key | ||
table.index('commitId') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. markCommitBranchUpdatedFactory, moveCommitsToBranchFactory, getCommitBranchesFactory, switchCommitBranchFactory has a 'where' statement on this column alone |
||
}) | ||
await knex.schema.alterTable('refresh_tokens', (table) => { | ||
// table.index('id') // index should already be available due to this being the primary key | ||
table.index('userId') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. deleteExistingAuthTokensFactory has a 'where' statement on this column alone. (Though I'm confused as to what the statement in that factory is actually doing, it appears to query and then not use the result) |
||
}) | ||
await knex.schema.alterTable('commits', (table) => { | ||
// table.index('id') // index should already be available due to this being the primary key | ||
table.index('author') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do these really need to be separate indices, instead of composite ones? it was at least the case in MySQL that if you had an index like "x, y, z", then it would be used even if you filter by only "x" or "x" and "y", as long as the order is the same (so it wouldn't work when just filtering by "z") There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. important to think about this, cause they do take up space There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was the best explanation I found on the subject: https://stackoverflow.com/questions/11352056/postgresql-composite-primary-key#comment113601692_11352543 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's even better than what I expected then, I didn't think there would be a speedup on just filtering "y" at all. But my point is that we need to look at what the actual filter conditions are, and if "y" is usually used together with "x", then there's no need to make "y" have its own index There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just having X and Y as separate indices could even be worse than having a "X, Y" index, if both are filtered together. It's just going to use the X or the Y index, where "X, Y" would be the better one There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
table.index('sourceApplication') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
table.index('referencedObject') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
table.index('createdAt') // used in an ORDER BY clause. https://www.postgresql.org/docs/current/indexes-ordering.html | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
}) | ||
await knex.schema.alterTable('file_uploads', (table) => { | ||
// table.index('id') // index should already be available due to this being the primary key | ||
// table.index('streamId') // already explicitly indexed | ||
table.index('convertedStatus') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
table.index('uploadDate') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
table.index('branchName') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
}) | ||
await knex.schema.alterTable('server_invites', (table) => { | ||
//table.index('id') // index should already be available due to this being the primary key | ||
table.index('updatedAt') // used in an ORDER BY clause. https://www.postgresql.org/docs/current/indexes-ordering.html | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
table.index('target') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
table.index('createdAt') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
}) | ||
await knex.schema.alterTable('gendo_ai_renders', (table) => { | ||
//table.index('id') // index should already be available due to this being the primary key | ||
// table.index('gendoGenerationId') // already explicitly indexed | ||
table.index('versionId') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
table.index('createdAt') // used in an ORDER BY clause. https://www.postgresql.org/docs/current/indexes-ordering.html | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
}) | ||
await knex.schema.alterTable('pwdreset_tokens', (table) => { | ||
// table.index('id') // index should already be available due to this being the primary key | ||
table.index('email') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
table.index('createdAt') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
}) | ||
await knex.schema.alterTable('automation_triggers', (table) => { | ||
// table.index('automationRevisionId') // index should already be available due to this being the first item in the composite primary key | ||
table.index('triggerType') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
table.index('triggeringId') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
}) | ||
} | ||
|
||
export async function down(knex: Knex): Promise<void> { | ||
await knex.schema.alterTable('stream_commits', (table) => { | ||
table.dropIndex('commitId') | ||
}) | ||
await knex.schema.alterTable('branch_commits', (table) => { | ||
table.dropIndex('commitId') | ||
}) | ||
await knex.schema.alterTable('refresh_tokens', (table) => { | ||
table.dropIndex('userId') | ||
}) | ||
await knex.schema.alterTable('commits', (table) => { | ||
table.dropIndex('author') | ||
table.dropIndex('sourceApplication') | ||
table.dropIndex('referencedObject') | ||
table.dropIndex('createdAt') | ||
}) | ||
await knex.schema.alterTable('file_uploads', (table) => { | ||
table.dropIndex('convertedStatus') | ||
table.dropIndex('uploadDate') | ||
table.dropIndex('branchName') | ||
}) | ||
await knex.schema.alterTable('server_invites', (table) => { | ||
table.dropIndex('target') | ||
table.dropIndex('updatedAt') | ||
table.dropIndex('createdAt') | ||
}) | ||
await knex.schema.alterTable('gendo_ai_renders', (table) => { | ||
table.dropIndex('versionId') | ||
table.dropIndex('createdAt') | ||
}) | ||
await knex.schema.alterTable('pwdreset_tokens', (table) => { | ||
table.dropIndex('email') | ||
table.dropIndex('createdAt') | ||
}) | ||
await knex.schema.alterTable('automation_triggers', (table) => { | ||
table.dropIndex('triggerType') | ||
table.dropIndex('triggeringId') | ||
}) | ||
} |
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.
getCommitStreamsFactory & markCommitStreamUpdatedFactory has a 'where' statement on this column alone