kaxil commented on code in PR #74409:
URL: https://github.com/apache/airflow/pull/74409#discussion_r4209886145
##########
airflow-core/src/airflow/migrations/versions/0136_3_4_0_fold_task_map_into_xcom_mapped_length.py:
##########
@@ -116,9 +116,16 @@ def build_restore_statement(xcom_name: str = "xcom",
task_map_name: str = "task_
def upgrade():
"""Fold task_map into xcom.mapped_length."""
with disable_sqlite_fkeys(op):
+ dialect = op.get_bind().dialect.name
with op.batch_alter_table("xcom", schema=None) as batch_op:
batch_op.add_column(sa.Column("mapped_length", sa.Integer(),
nullable=True))
- batch_op.create_check_constraint("mapped_length_not_negative",
"mapped_length >= 0")
+ # MySQL can only add a CHECK by copying the whole table
(ALGORITHM=COPY).
+ if dialect != "mysql":
Review Comment:
Does anything still write to this table once 0142 renames it to `xcom_v1`?
From what I can see every write goes through `set_for_attempt` into `xcom_v2`
(which has its own CHECK, created on an empty table so it's cheap on MySQL),
and `xcom_v1` is only read from or deleted from. So the only values that ever
land in this column come from the backfill out of `task_map.length`, which
already has `length >= 0`.
If that's right, could we drop this CHECK on every dialect instead of
skipping it on MySQL? That also gets rid of the `_is_not_mysql` helper on the
model, the Postgres constraint that stays NOT VALID forever, and the SQLite
table rebuild the batch op does only because of `create_check_constraint` (a
lone `add_column` goes through plain ALTER).
##########
airflow-core/src/airflow/models/xcom.py:
##########
@@ -72,6 +72,11 @@
XCOM_RETURN_KEY = "return_value"
+def _is_not_mysql(*args: Any, dialect: Dialect, **kwargs: Any) -> bool:
Review Comment:
If the constraint stays, `.ddl_if(dialect=("postgresql", "sqlite"))` does
the same thing without the helper or the extra `Dialect` import.
`DDLIf._should_execute` accepts a tuple of dialect names.
--
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]