Chais commented on code in PR #58149:
URL: https://github.com/apache/airflow/pull/58149#discussion_r2697341306


##########
airflow-core/src/airflow/migrations/versions/0060_3_0_0_add_try_id_to_ti_and_tih.py:
##########
@@ -127,34 +127,8 @@ def upgrade():
 
 def downgrade():
     """Unapply Add try_id to TI and TIH."""
-    dialect_name = op.get_bind().dialect.name
     with op.batch_alter_table("task_instance_history", schema=None) as 
batch_op:
         batch_op.drop_constraint(batch_op.f("task_instance_history_pkey"), 
type_="primary")
-        batch_op.add_column(sa.Column("id", sa.INTEGER, nullable=True))
+        batch_op.add_column(sa.Column("id", sa.INTEGER, primary_key=True, 
autoincrement=True))

Review Comment:
   I had hoped this would make it sufficiently clear that I did:
   > The downgrade SQL generated for psql looks like this:
   
   Technically, the generated code doesn't use `AUTO_INCREMENT`:
   ```sql
   ALTER TABLE test ADD COLUMN id SERIAL NOT NULL;
   ```
   But as we can see in the PostgreSQL documentation, the `SERIAL` type is an 
autoincrementing integer, which is just what we need: 
https://www.postgresql.org/docs/current/datatype-numeric.html  
   Maybe Alembic translates `sa.INTEGER, autoincrement=True` into `SERIAL` for 
Postgres.
   
   Also, it's not like I invented this definition, either. 
`task_instance_history` was introduced in revision `d482b7261ff9` (number 21, 
by the new count) with the `id` column defined like this:
   ```py
   sa.Column("id", sa.Integer(), primary_key=True, autoincrement=True),
   ```
   So clearly, this worked before (and for all supported DBs, I might add) and 
I see no reason why it shouldn't work again.  
   And the fact remains, that the current code leaves the DB broken 
post-downgrade. While it restores the `id` column and refills it in what may be 
the most cumbersome way possible, it neglects the autoincrement the codebase 
expects at the relevant version, leading to queries failing. Please see the 
issue I linked in the original post for details.



-- 
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]

Reply via email to