zach-overflow opened a new issue, #59871: URL: https://github.com/apache/airflow/issues/59871
### Apache Airflow version Other Airflow 3 version (please specify below) ### If "Other Airflow 3 version" selected, which one? Affects 2.9.0 through 3.1 ### What happened? At the moment, there is at least [one alembic migration script](https://github.com/apache/airflow/blob/3f28f059c79500124b473f8a856036e0c425bdff/airflow-core/src/airflow/migrations/versions/0015_2_9_0_update_trigger_kwargs_type.py#L84) which executes upgrade/downgrade operations using a sqlalchemy ORM model directly. Using the models directly in the migration scripts can cause a number of issues if the model changes. One such example pertaining to the `trigger` table migration script in `2.9.0` is described in [this thread](https://github.com/apache/airflow/pull/59239#discussion_r2649195334). In that scenario, if the use of the ORM in the `2.9.0` migration script is left in place, it will prevent the addition of a column in the new migration targetting `3.2.0` since downgrades from `3.2.X` to ≤ `2.9.0` will rely on an ORM definition which still references the new column in `3.2.0`, leading to [failures like this](https://github.com/apache/airflow/actions/runs/20346147416/job/58459619112?pr=59239): ``` sqlalchemy.exc.ProgrammingError: (psycopg2.errors.UndefinedColumn) column trigger.trigger_queue does not exist LINE 1: ...te, trigger.triggerer_id AS trigger_triggerer_id, trigger.tr... ``` I'm not an alembic expert, but my understanding is that it is generally [not recommended](https://stackoverflow.com/questions/13676744/using-the-sqlalchemy-orm-inside-an-alembic-migration-how-do-i#comment64254760_36087108) to use sqlalchemy models in alembic migration scripts as they can cause issues like the one described above. It seems this particular example has also caused past issues such as in this [past PR](https://github.com/apache/airflow/pull/38743). ### What you think should happen instead? The ORM classes should not be referenced in any of the alembic migration scripts. To achieve this ideal setup, I believe there are 2 necessary changes: 1. Modify the current single problematic migration script ([`0015_2_9_0_update_trigger_kwargs_type.py`](https://github.com/apache/airflow/blob/3f28f059c79500124b473f8a856036e0c425bdff/airflow-core/src/airflow/migrations/versions/0015_2_9_0_update_trigger_kwargs_type.py)) so that it does not use `airflow.models.Trigger` directly in the upgrade/downgrade queries. Obviously, not ideal to modify a past migration script, but it seems that might be the only viable solution in this case, and that seems to align with the [past discussion here](https://github.com/apache/airflow/pull/38743). 2. To prevent this problem from occurring in the future, add a pre-commit hook which ensures no alembic scripts rely on ORM classes in their operations. ### How to reproduce As of December 28th, 2025 you can reproduce as follows: 1. Cut a feature branch from upstream `main` latest 2. Add a new column attribute named `test_col` to `airflow.models.trigger.Trigger` without any constraints (it does not matter what type the column is, and the column may be nullable). 3. Create a new alembic migration script for the change you introduced in step 2. 4. Run the CI migration check commands, which test upgrading from `2.11.0` to current, and then downgrading from current to `2.7.0` (commands pasted below for convenience): ```shell breeze shell 'airflow db reset --skip-init -y && airflow db migrate --to-revision heads' \ --use-airflow-version 2.11.0 \ --airflow-extras pydantic \ --answer y \ && breeze shell "export AIRFLOW__DATABASE__EXTERNAL_DB_MANAGERS=airflow.providers.fab.auth_manager.models.db.FABDBManager \ && airflow db migrate \ --to-revision heads \ && airflow db downgrade \ -n 2.7.0 \ -y \ && airflow db migrate" ``` After running the migration testing commands in step 4, you should see that the upgrade portion (from 2.11.0 to current head) runs without issues, but then you will see an error on the downgrade test portion like the following: ``` Running downgrade 1949afb29106 -> ee1467d4aa35, update trigger kwargs type and encrypt. [alembic.runtime.migration] loc=migration.py:622 Performing downgrade with database ***postgres/airflow Traceback (most recent call last): File "/usr/python/lib/python3.10/site-packages/sqlalchemy/engine/base.py", line 1910, in _execute_context self.dialect.do_execute( File "/usr/python/lib/python3.10/site-packages/sqlalchemy/engine/default.py", line 736, in do_execute cursor.execute(statement, parameters) psycopg2.errors.UndefinedColumn: column trigger.test_col does not exist LINE 1: ...te, trigger.triggerer_id AS trigger_triggerer_id, trigger.tr... ``` ### Operating System MacOS 15.7 Apple M3 Max, Also whatever OS the CI checks run on ### Versions of Apache Airflow Providers _No response_ ### Deployment Other ### Deployment details _No response_ ### Anything else? I'm happy to put together the pre-commit check for this in a separate PR. Coincidentally I have a PR which attempts to remove the `Trigger` class reference in the `2.9.0` alembic script, but TBD if the old migration script changes land as part of that PR or not. ### Are you willing to submit PR? - [x] Yes I am willing to submit a PR! ### Code of Conduct - [x] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
