Skip to content
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

Add support for auto generating sqlite schema #1163

Merged
merged 12 commits into from
Apr 15, 2024

Conversation

habdelra
Copy link
Contributor

@habdelra habdelra commented Apr 12, 2024

This PR adds support for auto generating the sqlite schema from the postgres DB. The command pnpm schema automatically create a sqlite schema from the postgres db. To do this we leverage a SQL AST (actually a CST).

TODO:

  • add developer documentation for now to create a new migration
  • add developer documentation for running a migration manually
  • add developer documentation for generating the sqlite schema

Copy link

github-actions bot commented Apr 12, 2024

Test Results

584 tests  ±0   580 ✔️ ±0   8m 21s ⏱️ -6s
    1 suites ±0       4 💤 ±0 
    1 files   ±0       0 ±0 

Results for commit 4d4c3df. ± Comparison against base commit 563cc3a.

♻️ This comment has been updated with latest results.

@@ -1,7 +1,7 @@
exports.up = (pgm) => {
pgm.createTable('indexed_cards', {
card_url: { type: 'varchar', notNull: true },
realm_version: { type: 'varchar', notNull: true },
realm_version: { type: 'integer', notNull: true },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I opted to adjust this migration instead of making a new one since we don't actually have any data yet to migrate

Copy link
Contributor

@backspace backspace left a comment

Choose a reason for hiding this comment

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

very cool!

Co-authored-by: Buck Doyle <buck.doyle@cardstack.com>
README.md Outdated
pnpm migrate down
```

The boxel system also uses SQLite in order to run the DB in the browser as part of running browser tests (and eventually we may run the realm server in the browser to provide a local index). After you have completed your migration you then need to generate a new schema SQL file for SQLite. To generate a new SQLite schema, from `packages/realm-server`, execute:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The boxel system also uses SQLite in order to run the DB in the browser as part of running browser tests (and eventually we may run the realm server in the browser to provide a local index). After you have completed your migration you then need to generate a new schema SQL file for SQLite. To generate a new SQLite schema, from `packages/realm-server`, execute:
Boxel also uses SQLite in order to run the DB in the browser as part of running browser tests (and eventually we may run the realm server in the browser to provide a local index). We treat the Postgres database schema as the source of truth and derive the SQLite schema from it. Therefore, once you author and apply a migration, you should generate a new schema SQL file for SQLite. To generate a new SQLite schema, from `packages/realm-server`, execute:

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, would it be possible and desirable to automatically run pnpm make-schema after pnpm migrate up or pnpm migrate down?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's possible. not sure if it's desirable. one thing to keep in mind is that the application itself can run migrations (like a django app). so it's not necessarily the case that migrations will always happen as a result of running pnpm migrate up or pnpm migrate down. in order to make sure things keep in sync, though, I did add some logic when the host app starts to make sure the sqlite schema is up-to-date. My thinking was that I wanted for us to work with it for bit and based on dev feedback we can adjust as necessarily.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think another reason why we might want to keep the pg migration and sqlite schema generation decoupled for now is that the sqlite schema conversion is still pretty fresh. I could see scenarios where we start using a new constraint type that we haven't yet added support for in the sqlite schema generation script. in which case it might be easier for debugging if the migration and schema generation were separate.

habdelra and others added 2 commits April 15, 2024 10:35
Co-authored-by: Luke Melia <luke@lukemelia.com>
Co-authored-by: Luke Melia <luke@lukemelia.com>
@habdelra habdelra merged commit 96fd60a into main Apr 15, 2024
23 checks passed
@delete-merged-branch delete-merged-branch bot deleted the cs-6460-setup-node-pg-migrate-for-database-migrations branch April 15, 2024 14:59
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.

4 participants