ferruzzi commented on code in PR #66350:
URL: https://github.com/apache/airflow/pull/66350#discussion_r3994190662


##########
airflow-core/tests/unit/utils/test_db_cleanup.py:
##########
@@ -551,6 +655,78 @@ def 
test_dag_version_cleanup_skips_versions_pinned_by_task_instance(self):
         assert latest_id in remaining  # kept by keep_last
         assert orphan_id not in remaining  # old and unreferenced -> pruned
 
+    def test_do_delete_skip_if_referenced_guards_against_race(self):
+        """_do_delete must not issue a DELETE that violates an ON DELETE 
RESTRICT FK.
+
+        Simulates a race where a dag_version row passes the SELECT filter (no 
TI
+        references it at archive-creation time) but a TI referencing it is 
inserted
+        before the DELETE runs.  The skip_if_referenced guard on the DELETE 
itself
+        must leave the row in place instead of failing with IntegrityError.
+        """
+        from airflow.utils.db import reflect_tables
+
+        base_date = pendulum.DateTime(2020, 1, 1, 
tzinfo=pendulum.timezone("UTC"))
+        bundle_name = f"race-test-{uuid4()}"
+        dag_id = f"race_dag_{uuid4()}"
+
+        with create_session() as session:
+            session.add(DagBundleModel(name=bundle_name))
+            session.flush()
+            session.add(DagModel(dag_id=dag_id, bundle_name=bundle_name))
+            session.flush()
+
+            dv = DagVersion(
+                dag_id=dag_id,
+                version_number=1,
+                bundle_name=bundle_name,
+                created_at=base_date,
+                last_updated=base_date,
+            )
+            session.add(dv)
+            session.flush()
+            dv_id = dv.id
+
+            # Manually create an archive table containing this dag_version row,
+            # simulating the CTAS step that ran before the TI was inserted.
+            archive_name = f"{ARCHIVE_TABLE_PREFIX}dag_version__race_test"
+            session.execute(
+                text(f"CREATE TABLE {archive_name} AS SELECT * FROM 
dag_version WHERE id = '{dv_id}'")

Review Comment:
   FWIW, my Claude agrees with your Claude that `bindparams` was the right 
answer here, but suggests `bindparams(dag_id=dag_id)`. :sweat_smile:    It's a 
bit outside my wheelhouse though, so a human confirmation would be required 
before I feel comfortable approving/merging.
   



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