ferruzzi commented on PR #66350: URL: https://github.com/apache/airflow/pull/66350#issuecomment-5592622852
@jakubmatyszewski - I think this is half of the fix. #71732 (@arose26) is the other half and I'd be more comfortable merging them as one PR so they don't somehow get split accidentally if one gets backported without the other or something. Your `asset_event` change here is right: `source_dag_id` is the right column name. My concern is `deadline` and `task_reschedule`, where this drops `dag_id_column_name` altogether. `_build_query` only applies the filter when the column is present so after this change `airflow db clean --dag-ids one_dag` stops crashing on those two tables and instead cleans them for every dag. For a destructive command going into 3.3.2, I'd rather not trade a loud failure for a silent delete. #71732 fixes that part properly. It adds `dag_id_via=(fk_column, parent_table, parent_pk_column)` and filters through the parent's `dag_id`. The EXISTS that arose26 put there is worth keeping. `deadline.dagrun_id` is nullable, and with `NOT IN` the NULL would swallow the comparison, so a row would survive the filter. Your PR also has two things #71732 doesn't, so combining them together beats either one alone. Re-applying `skip_if_referenced` on the DELETE closes the race between the archive INSERT and the DELETE, and `test_dag_id_column_name_matches_schema` is a great regression test. So the actual request: Could you cherry-pick @arose26's changes into this PR, and give co-author credit in the commit message? ``` Co-authored-by: arose26 <[email protected]> ``` Please bring their newsfragment across as well, and add `closes: #69030`, which is the issue #71732 closes. Then we can land one complete fix rather than half of one, close #71732 as folded in, and never have to worry about only half of the fix getting picked up. @arose26 and @potiuk - are you both alright with this plan? -- 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]
