LiamHecht commented on code in PR #75562:
URL: https://github.com/apache/airflow/pull/75562#discussion_r4502598759


##########
airflow-core/tests/unit/models/test_trigger.py:
##########
@@ -180,6 +180,96 @@ def test_clean_unused(session, dag_maker):
     assert {result.id for result in results} == {trigger1.id, trigger4.id, 
trigger5.id, trigger6.id}
 
 
+def _create_unused_trigger(session) -> int:
+    trigger = Trigger(classpath="airflow.triggers.testing.SuccessTrigger", 
kwargs={})
+    session.add(trigger)
+    session.commit()
+    return trigger.id
+
+
+def _clean_unused_adding_reference_before_delete(session, trigger_id, 
add_reference):
+    """Run clean_unused, executing ``add_reference`` right before its DELETE 
(#72061)."""
+    reference_added = False
+
+    def add_reference_before_delete(connection, cursor, statement, parameters, 
context, executemany):
+        nonlocal reference_added
+        normalized_statement = " ".join(statement.lower().replace('"', 
"").replace("`", "").split())
+        if not reference_added and "delete from trigger" in 
normalized_statement:
+            reference_added = True

Review Comment:
   > Seems like you are trying to add the event listener only once, meaning 
only for the first query, isn't the first query a select the other being a 
delete? If so, the delete from should be enough no?
   
   Yes, that's what it does now. The hook only reacts to the DELETE FROM 
trigger statement, so there's no "only the first query" logic any more. I 
normalise quoting and case in the statement so it also matches on MySQL and 
Postgres.



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