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

sql script to init DB + empty sqlite db initialised #18

Merged
merged 6 commits into from
Mar 16, 2025
Merged

sql script to init DB + empty sqlite db initialised #18

merged 6 commits into from
Mar 16, 2025

Conversation

MagicalLay
Copy link
Contributor

No description provided.

@MagicalLay MagicalLay requested review from Gbury and But2ene February 16, 2025 18:06
Copy link
Contributor

@But2ene But2ene left a comment

Choose a reason for hiding this comment

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

ça m'a l'air fonctionnel,
une question sur les participations non notées, ça semble géré dans le code plutot que la BDD avec la structure actuelle

@But2ene But2ene linked an issue Mar 2, 2025 that may be closed by this pull request

BEGIN;

CREATE TABLE IF NOT EXISTS events (
Copy link
Contributor

Choose a reason for hiding this comment

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

It may be standard to have the IF NOT EXISTS, but since we have a DROP IF EXISTS above, aren't we guaranteed that the table does not exists at this point ?

Copy link
Contributor

Choose a reason for hiding this comment

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

The redundancy appeared because I added the "DROP IF EXISTS" at the begininng.

Actually, the sql script is more like an executable specification documentation. This is a good design document.
Tables are created by the ocaml code, and I feel like it should be kept that way.

It might be interesting to automatically compare table definitions by ocaml with our sql specification some day.

avoiding if not exists in this case (because we are always deleting schema) ensure table definition is updated, but it becomes possible to accidently delete data if we execute the script on the production database. What do you prefer?

@But2ene But2ene self-requested a review March 12, 2025 21:44
Copy link
Contributor Author

@MagicalLay MagicalLay left a comment

Choose a reason for hiding this comment

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

to me there is no real point to have sql creation queries in 2 places: the lib and the script. it's just an opportunity for future desynchronisation. IMO, we should either:

  • use the script to create the DB (a true option to me, I don't see what would be the true issue with that ; but I'm fine with other options as well), and make the lib only writes to it but not create the tables
  • update the lib code to reflect on the new schema, but in such case we don't really need it outside (apart for work planification like now)

The only case it would make sense to me to keep it there alongside the lib code if the lib code is creating the table would be if it was written to by a Github action or sort of (like pgdump), to keep track of what the current database schema is ; a bit like a documentation. but in such case it would be just that, a documentation generated from the code, which is the real source of truth

@Gbury Gbury merged commit c337907 into main Mar 16, 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.

design db database schemas
3 participants