Switch map_index back to use server_default#20902
Conversation
This emits 'DEFAULT -1' in the column definition and helps migration.
This comment has been minimized.
This comment has been minimized.
| dag_id = Column(String(ID_LEN, **COLLATION_ARGS), primary_key=True, nullable=False) | ||
| run_id = Column(String(ID_LEN, **COLLATION_ARGS), primary_key=True, nullable=False) | ||
| map_index = Column(Integer, primary_key=True, nullable=False, default=-1) | ||
| map_index = Column(Integer, primary_key=True, nullable=False, server_default=text("-1")) |
There was a problem hiding this comment.
This one can possibly have default and server_default.
There was a problem hiding this comment.
I checked some sources and realised default isn’t actually much useful in ORM, only if we use the SQL expression builder. And with server_default set, default only really matters for UPDATE, which actually seems a bit wrong to me, so I’ll omit default until they are proven useful.
|
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease. |
github-actions
Bot
added
the
full tests needed
jedcunningham
added
the
changelog:skip
3 participants
Close #20876.
This ends up being simply reverting fff3a1e, but I forgot exactly why I made the switch in the first place. I guess the test suite will tell us if it works…