sqlalchemy / alembic

A database migrations tool for SQLAlchemy.
MIT License
2.76k stars 241 forks source link

Improve re-writer implementation, fixing chain and adding support to callables #1337

Closed CaselIT closed 9 months ago

CaselIT commented 11 months ago

ive just read the source code for chain() and it's a bit of a disaster. because if i do this:

w = writer1.chain(writer2).chain(writer3)

writer2 is lost. it's doing the chaining wrong. my high school comp sci teacher would be unimpressed

it works for your use case right now because it only handles exactly two writers. but if we fix it to correctly implement chaining then it would not work for your case in the general sense.

we can fix both issues at once by 1. fixing the chaining so that if self._chain is non-None we traverse the list through to the end for the linked list append, and 2. looking at the type of object sent and if it's just a callable, wrapping it in a helper object CallableRewriter that uses that callable and also supports the rest of the Rewriter protocol including self._chain.

Originally posted by @zzzeek in https://github.com/sqlalchemy/alembic/discussions/1332#discussioncomment-7369839

sqla-tester commented 10 months ago

l-hedgehog has proposed a fix for this issue in the main branch:

Improve Rewriter implementation https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/4990