sqlalchemy / sqlalchemy/alembic
Augmented (TypeDecorator) JSON type results in data loss during SQLite migration
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 4.4k
- Forks
- 375
- PR merge metrics
- No merged PRs in 30d
Description
Describe the bug
A sqlalchemy type created using sa.TypeDecorator with a sa.JSON impl exhibits the same SQLite-related data loss as described in #697.
Expected behavior
A type whose impl is sa.JSON should 'look' to Alembic like sa.JSON for the purposes of deciding whether to CAST the data in the column under SQLite.
To Reproduce
class CustomJson(sa.TypeDecorator):
impl = sa.JSON
def upgrade():
with op.batch_alter_table('bar') as bop:
bop.alter_column('foo', type_=CustomJson)
This will result in your valid-as-JSON VARCHAR column being turned into a numeric data type with the value 0, because the SQL emitted contains the CAST clauses that need to be avoided to prevent this truncation, e.g. INSERT INTO _alembic_tmp_bar ... CAST(bar.foo AS JSON) AS foo) ...
Versions.
- OS: all
- Python: all
- Alembic: 1.8.1 and below
- SQLAlchemy: 1.4
- Database: SQLite
- DBAPI: all
Additional context
The issue is that the original fix uses isinstance(col, sa.JSON), but a custom column type should not be an instance of a base type according to https://docs.sqlalchemy.org/en/14/core/custom_types.html#augmenting-existing-types - instead, it's an instance of sa.TypeDecorator.
I suspect this should really be a more general purpose fix, where "sqlalchemy types hierarchy testing" in alembic should go through some kind of centralized function that understands how to 'unwrap' a sa.TypeDecorator and make its decision based on the underlying implementation.
I'd be happy to contribute some code if the maintainers are up for accepting a PR for this issue.
Workaround
(for anyone else running into this and wondering what to do if it isn't fixed yet)
Find a way to specify your columns as sa.JSON rather than your custom column in the migration. SQLite doesn't actually care (very much) what type your column is anyway. This will be ugly, but it will preserve your data, and then you can likely use the more correct column type at runtime with sqlalchemy just fine, since it seems to only be the CAST statement that causes this issue - the rest of the JSON logic lives in your sqlalchemy code.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reproducing the op.batch_alter_table upgrade with a TypeDecorator whose impl is sa.JSON, and inspect the SQLite migration logic that decides whether to emit CAST clauses. Done means the custom JSON type is recognized like sa.JSON for this decision and the migration preserves the existing column data without the harmful CAST.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python, sqlalchemy, sqlite
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100