Skip to content

Conversation

@netroy
Copy link

@netroy netroy commented Apr 29, 2025

There are many schema operations that sqlite has supported natively for years, but the sqlite driver hasn't implemented, and instead we end up re-creating entire tables for these operations, which when foreign-key constraints are enabled, leads to data-loss.
This PR re-implements some of these operations using native queries, whenever possible.

Copy link
Member

@ivov ivov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good on the face of it, but I'm not deeply familiar with this lib!

Is there a chance they would upstream this?

Comment on lines +497 to +502
if (shouldRecreateTable) {
const changedTable = table.clone()
columns.forEach((column) => changedTable.addColumn(column))
await this.recreateTable(changedTable, table)
return
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we log a warning here? Same for other cases that require recreating.

const tableColumn = table.columns.find(
(column) => column.name === oldColumn.name,
)
if (tableColumn) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we throw if not found instead?

@netroy netroy closed this Aug 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants