-
Notifications
You must be signed in to change notification settings - Fork 0
22 create the database schema #38
Conversation
All tables of the data model, with join tables for routines and gymnast element lists, CatalogEntry and CatalogWord for the search, and CHECK constraints and the C collation on catalog_words.word added in SQL in the migration.
One catalog entry per code with the words of all variants, several search words that must all match, a trailing "s" removed, and the C collation on the word column instead of a database-wide setting.
Review 1/4: Schema and indexesNice work. I applied the migration to a fresh PostgreSQL 18: The numbers below are from a local test database with synthetic data about 400× the real catalog (225 000 entries), queried through Prisma. They show the shape of the problem, not exact timings. Priority: High = fix before merge, Medium = fix or decide soon, Low = nice to have. 1. Name sort ascending cannot use its index. High
2. Filter + difficulty/code sort is slow in one direction. High
3. What does 4. Comment made with Claude Code in discussion with Ronja |
Review 2/4: Search and backend notesThese affect #23 and #24 more than this PR, but they follow from the schema, so I am listing them here. Same test setup as in comment 1/4. Priority: High = fix before merge, Medium = fix or decide soon, Low = nice to have. 1. Wildcards are not escaped. High (for #24) 2. 3. Decimal values. Medium 4. Seed. Medium 5. Generic query plans. Low Comment made with Claude Code in discussion with Ronja |
Review 3/4: Open questionsThings I could not tell from the PR or the docs. Answers can be a sentence in Priority: High = needs an answer before merge, Medium = decide soon, Low = nice to have. 1. The gymnast's element list has no GraphQL field. High 2. Removed elements. Medium
3. Duplicates and positions in a routine. Medium 4. Value ranges and limits. Low Comment made with Claude Code in discussion with Ronja |
Review 4/4: Docs and processPriority: High = fix before merge, Medium = fix or decide soon, Low = nice to have. Medium
Low
Comment made with Claude Code in discussion with Ronja |
|
Thanks for the thorough reviews! Here is what I did with each point. Review 1: indexes, difficulty and timestamps
Review 2: search safety and backend rules
Review 3: open questions
All decisions from this review have their own sections under "Design decisions" in Review 4: docs and issues
I also updated the README and |
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.
Lgtm
Summary
Adds the Prisma schema for the whole data model and the first migration, which creates all the tables in PostgreSQL. The search follows the design in
docs/API.md("Search without raw SQL"), with a few changes described below.Changes
apparatus(withpositionfor the sort order),element_groups,variant_groups,elementsandvaults. The catalog keeps the readable ids fromdata/.coaches,gymnastsandroutines(withcreated_atandupdated_at). These get generated ids (cuid).routine_elements, with apositionthat is unique per routinegymnast_elements, with no duplicates per gymnastvariant_group_idinstead of a list on the variant group.valueandd_valueareDecimal(3, 1), so summing D-values has no rounding errors.removed_at, so they can be hidden without being deleted.catalog_entriesandcatalog_words, with the indexes fromAPI.md.docs/DATA_MODEL.mdhas a new "Storage in PostgreSQL" section and updated diagram, tables and constraint numbers, and explains why vault is its own tablenpm run db:migratestepsearchargument inschema.graphqlis updatedChanges to the search design in docs/API.md
The design is the one in "Search without raw SQL". These are the differences:
Ccollation is set on thewordcolumn only, not on the whole database.API.mdfirst suggested creating the database with theCcollation. Instead the migration sets it oncatalog_words.word, andschema.prismahas an ordinary@@index([word]).en-US, so an ordinary index is not used for a prefix search (startsWith). We checked this withEXPLAIN.Ccollation would change the sorting of all text, includingnameSortfor names with accents. With the column setting, everything else keepsen-US.text_pattern_ops. We tried it on the index first. Prisma accepts it inschema.prisma, but it can't read it back from the database, so everyprisma migrate devwanted to drop and recreate the index.CatalogEntryhas one row per code, the-avariant, because the catalog list shows one row per code.CatalogWordholds the words of all variants of the code, so a word that is only in variant-bstill finds it. The seed script will also add written-out abbreviations (for examplehandstandforhstd).mode: 'insensitive'. It turnsLIKEintoILIKE, and then the index isn't used. The words are stored in lowercase, and the search term is lowercased before the query.How to test
npm cifrom the rootnpm run db:migrateto create the tables in your local databasecd backendand thennpx prisma migrate status. It should end with "Database schema is up to date!"npx prisma migrate devinbackend/. It should say "Already in sync", which means the schema and the migration matchCloses #22
🤖 Generated with Claude Code