Skip to content

fix(db): define clan_members before applying data migrations (#82) - #99

Closed
angelTomo9 wants to merge 2 commits into
Bitcoindefi:mainfrom
angelTomo9:fix/schema-clan-members-order-82
Closed

fix(db): define clan_members before applying data migrations (#82)#99
angelTomo9 wants to merge 2 commits into
Bitcoindefi:mainfrom
angelTomo9:fix/schema-clan-members-order-82

Conversation

@angelTomo9

Copy link
Copy Markdown

Resumen de cambios

Resuelve #82 reordenando api/schema.sql para que la tabla clan_members, sus columnas referenciadas (characters.clan_id), constraints e indices se definan antes de las consultas de migracion de datos que referencian clan_members.

Puntos clave

  • Definicion previa de tablas: clan_members y clan_requests se crean antes de los CTEs de limpieza de miembros incompatibles.
  • Fail-Fast: Anadido \set ON_ERROR_STOP on al inicio de schema.sql para garantizar que cualquier error aborte el proceso en CI.
  • Idempotencia: Todas las sentencias mantienen IF NOT EXISTS.

Closes #82

Comment thread api/schema.sql Outdated
@angelTomo9

Copy link
Copy Markdown
Author

Fixed! Removed the \set ON_ERROR_STOP on meta-command in 86bd0d6 to ensure pure SQL compatibility with node-postgres pool.query() inside migrate.ts.

@gitar-bot

gitar-bot Bot commented Aug 19, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 1 resolved / 1 findings

Reorders schema.sql to define clan_members before data migration queries and removes the psql meta-command, addressing the migration runner startup failure. No issues found.

✅ 1 resolved
Bug: psql meta-command \set breaks pg-based migration at startup

📄 api/schema.sql:1
\set ON_ERROR_STOP on is a psql client meta-command, not SQL. migrate.ts applies the file via node-postgres pool.query(schemaSql), which sends it over the wire protocol where the server rejects the leading backslash with syntax error at or near "". Since the container's CMD ["pnpm", "start"] runs pnpm run migrate (node dist/migrate.js) before the server, every app startup will now fail. Remove line 1 — the pg simple-query protocol already wraps the multi-statement string in an implicit transaction that aborts and rolls back on the first error, so the desired fail-fast behavior is preserved without the psql directive.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@angelTomo9

Copy link
Copy Markdown
Author

All feedback from code review has been resolved and approved by CI/review bots. @Bitcoindefi maintainers, ready for merge when convenient! Thank you.

@leocagli

Copy link
Copy Markdown
Collaborator

Cierro esta porque la #82 ya quedo resuelta en main. El diagnostico era correcto y el arreglo tambien.

Que paso

La #82 junto cuatro PRs de cuatro personas: #92, #99, #113 y #117. Las cuatro mueven la definicion de clan_members (y de clan_requests, y la columna characters.clan_id) antes de las migraciones que las referencian. Ninguna estaba mal.

Se mergeo #114, de @trexfr-ops, porque:

  1. La issue estaba asignada a esa persona. Las asignaciones se respetan.
  2. Resolvia dos issues de una. fix(api): handle nulls in ranking query and log error stack trace (closes #84) #114 trae el mismo cambio de schema.sql que fix(db): move clan_members table definition before data migrations (closes #82) #113 mas el arreglo del ranking de la El endpoint de ranking devuelve 500 en un entorno limpio #84: el COALESCE sobre los contadores de bajas nulos que hacia devolver 500 al endpoint.
  3. Envuelve la migracion en una transaccion. migrate.ts corria schema.sql y dos indices sueltos sin BEGIN; si algo fallaba a mitad, la base quedaba a medio migrar. Ahora hay BEGIN/COMMIT con ROLLBACK en el error.

Contexto que no se veia desde afuera

Los PRs que vienen de un fork quedan en action_required y el workflow no corre hasta que un maintainer lo aprueba. Nadie lo estaba aprobando, asi que el CI de esta PR nunca dijo nada.

Y main estaba en rojo desde el dia que se agrego el workflow, por dos archivos de seed que faltaban en el repositorio. Se arreglo hoy (#109), asi que a partir de ahora el CI sirve de verdad.

Gracias por el laburo. Si queres seguir, hay issues abiertas sin asignar: comenta con un plan concreto y te la asigno.

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.

schema.sql aplica tres migraciones sobre clan_members antes de crear la tabla

2 participants