Fix migration script: cast SQLite integer booleans to Postgres booleans #82

Merged

forgejo_admin merged 2 commits from 81-fix-migration-boolean-cast into main

2026-03-06 19:27:17 +00:00

Conversation 2 Commits 2 Files changed 1 +16

forgejo_admin commented

2026-03-06 19:22:00 +00:00

Contributor

Copy link

Summary

The SQLite-to-Postgres migration script inserts raw integer values (1/0) for boolean columns, causing DatatypeMismatch errors. This fix auto-detects boolean columns from the Postgres schema and converts values to Python booleans before insertion.

Changes

scripts/migrate_sqlite_to_postgres.py: Added sqlalchemy.types.Boolean import. After building row_dicts, inspect the Postgres table schema to find boolean columns, then cast any non-None integer values to bool() before executing the INSERT.

Test Plan

Tests pass locally (308 passed)
Ruff lint passes
Manual verification: run migration against Postgres with port-forward and confirm no DatatypeMismatch errors
No regressions in existing migration behavior

Review Checklist

Passed automated review-fix loop
No secrets committed
No unnecessary file changes
Commit messages are descriptive

Related Notes

plan-2026-02-26-tf-modularize-postgres -- Phase 3, Part D (data migration script)
Forgejo issue: #81

## Summary The SQLite-to-Postgres migration script inserts raw integer values (1/0) for boolean columns, causing `DatatypeMismatch` errors. This fix auto-detects boolean columns from the Postgres schema and converts values to Python booleans before insertion. ## Changes - `scripts/migrate_sqlite_to_postgres.py`: Added `sqlalchemy.types.Boolean` import. After building `row_dicts`, inspect the Postgres table schema to find boolean columns, then cast any non-None integer values to `bool()` before executing the INSERT. ## Test Plan - [x] Tests pass locally (308 passed) - [x] Ruff lint passes - [ ] Manual verification: run migration against Postgres with port-forward and confirm no `DatatypeMismatch` errors - [ ] No regressions in existing migration behavior ## Review Checklist - [x] Passed automated review-fix loop - [x] No secrets committed - [x] No unnecessary file changes - [x] Commit messages are descriptive ## Related Notes - `plan-2026-02-26-tf-modularize-postgres` -- Phase 3, Part D (data migration script) - Forgejo issue: #81

forgejo_admin added 1 commit

2026-03-06 19:22:00 +00:00

Fix migration script: cast SQLite integer booleans to Postgres booleans

Some checks failed

ci/woodpecker/pr/woodpecker Pipeline failed

Details

ed5fa5f13d

SQLite stores booleans as integers (1/0) while Postgres expects actual
booleans. Detect boolean columns from Postgres schema via inspect() and
convert values before INSERT. Handles None (nullable) gracefully.

Closes #81

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

forgejo_admin added 1 commit

2026-03-06 19:22:58 +00:00

Hoist pg_inspector creation outside table loop

Some checks failed

ci/woodpecker/pr/woodpecker Pipeline failed

Details

ad435b4266

Review fix: avoid redundant inspect() call on every loop iteration.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

forgejo_admin commented

2026-03-06 19:23:07 +00:00

Author

Contributor

Copy link

Review-fix round 1: Hoisted inspect(pg_engine) out of the for table_name loop to avoid redundant inspector creation on every iteration. No functional change.

**Review-fix round 1**: Hoisted `inspect(pg_engine)` out of the `for table_name` loop to avoid redundant inspector creation on every iteration. No functional change.

forgejo_admin commented

2026-03-06 19:25:49 +00:00

Author

Contributor

Copy link

PR #82 Review

BLOCKERS

None.

NITS

pg_inspector created outside connection context (line 77): The inspector is created from pg_engine before the with pg_engine.connect() block. This works because inspect(engine) opens its own transient connection, but it means you have two independent connections to Postgres (the inspector's implicit one and the explicit dst_conn). Not a correctness issue, but you could reuse the inspector already created on line 70 (inspector = inspect(pg_engine)) by hoisting it or renaming. Minor -- no functional impact.
No test coverage for the migration script: There are no unit tests exercising the boolean casting logic. Given this is a one-shot migration script and manual verification is planned, this is acceptable, but worth noting.
bool(0) returns False, bool(1) returns True -- but bool(2) returns True as well. If any SQLite integer boolean column somehow has a value other than 0/1, this will silently coerce it. Extremely unlikely for boolean-modeled columns, but worth being aware of.

SOP COMPLIANCE

Branch named after issue (81-fix-migration-boolean-cast references #81)
PR body has: Summary, Changes, Test Plan, Related
Related section references plan slug (plan-2026-02-26-tf-modularize-postgres)
No secrets committed
No unnecessary file changes (1 file, 16 additions, 0 deletions -- tightly scoped)
Commit messages are descriptive

CODE REVIEW

The fix is clean and correct:

Uses inspect() to dynamically detect boolean columns from the Postgres schema rather than hardcoding column names. This is future-proof -- any new boolean columns added later will be handled automatically.
The isinstance(c["type"], Boolean) check is the right way to match SQLAlchemy column types from inspector metadata.
The None guard (row_dict[col] is not None) correctly preserves nullable booleans.
The col in row_dict check handles the case where SQLite and Postgres schemas might have different columns (defensive).
Placement is correct: casting happens after row_dicts are built but before dst_conn.execute(insert_sql, row_dicts).

The two boolean columns in the current schema are notes.is_approved and projects.is_public, both of which will be correctly detected and cast.

VERDICT: APPROVED

## PR #82 Review ### BLOCKERS None. ### NITS 1. **`pg_inspector` created outside connection context** (line 77): The inspector is created from `pg_engine` before the `with pg_engine.connect()` block. This works because `inspect(engine)` opens its own transient connection, but it means you have two independent connections to Postgres (the inspector's implicit one and the explicit `dst_conn`). Not a correctness issue, but you could reuse the inspector already created on line 70 (`inspector = inspect(pg_engine)`) by hoisting it or renaming. Minor -- no functional impact. 2. **No test coverage for the migration script**: There are no unit tests exercising the boolean casting logic. Given this is a one-shot migration script and manual verification is planned, this is acceptable, but worth noting. 3. **`bool(0)` returns `False`, `bool(1)` returns `True`** -- but `bool(2)` returns `True` as well. If any SQLite integer boolean column somehow has a value other than 0/1, this will silently coerce it. Extremely unlikely for boolean-modeled columns, but worth being aware of. ### SOP COMPLIANCE - [x] Branch named after issue (`81-fix-migration-boolean-cast` references #81) - [x] PR body has: Summary, Changes, Test Plan, Related - [x] Related section references plan slug (`plan-2026-02-26-tf-modularize-postgres`) - [x] No secrets committed - [x] No unnecessary file changes (1 file, 16 additions, 0 deletions -- tightly scoped) - [x] Commit messages are descriptive ### CODE REVIEW The fix is clean and correct: - Uses `inspect()` to dynamically detect boolean columns from the Postgres schema rather than hardcoding column names. This is future-proof -- any new boolean columns added later will be handled automatically. - The `isinstance(c["type"], Boolean)` check is the right way to match SQLAlchemy column types from inspector metadata. - The None guard (`row_dict[col] is not None`) correctly preserves nullable booleans. - The `col in row_dict` check handles the case where SQLite and Postgres schemas might have different columns (defensive). - Placement is correct: casting happens after `row_dicts` are built but before `dst_conn.execute(insert_sql, row_dicts)`. The two boolean columns in the current schema are `notes.is_approved` and `projects.is_public`, both of which will be correctly detected and cast. ### VERDICT: APPROVED

forgejo_admin merged commit 0ab1f61dc1 into main

2026-03-06 19:27:17 +00:00

forgejo_admin deleted branch 81-fix-migration-boolean-cast

2026-03-06 19:27:17 +00:00

forgejo_admin referenced this pull request from a commit

2026-03-06 19:27:17 +00:00

Fix migration script: cast SQLite integer booleans to Postgres booleans (#82)

forgejo_admin referenced this pull request

2026-03-06 20:57:55 +00:00

Fix CI: ruff format failure on migrate_sqlite_to_postgres.py #85

Commenting is not possible because the repository is archived.

Reviewers

No reviewers

Labels

Clear labels

QA passed, awaiting merge approval

status:in-progress

Dev agent is actively working

status:needs-fix

QA found issues, back to dev

status:qa

PR submitted, awaiting QA review

type:bug

Bug fix

type:devops

Infrastructure/CI/config work

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

ldraney/pal-e-api!82

Reference in a new issue

Repository

ldraney/pal-e-api

Title

Body

No description provided.

Delete branch "81-fix-migration-boolean-cast"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?

Rows
Columns