[GitHub] [airflow] kaxil commented on a change in pull request #12586: Remove foreign key constraint on SerializedDagModel's dag_runs field.

[GitBox <gi...@apache.org>]

kaxil commented on a change in pull request #12586:
URL: https://github.com/apache/airflow/pull/12586#discussion_r529594435



##########
File path: airflow/models/serialized_dag.py
##########
@@ -73,8 +73,7 @@ class SerializedDagModel(Base):
 
     dag_runs = relationship(
         DagRun,
-        primaryjoin=dag_id == DagRun.dag_id,
-        foreign_keys=dag_id,
+        primaryjoin=dag_id == foreign(DagRun.dag_id),

Review comment:
       I am still not sure if we explicitly require `foreign` keyword over here.
   
   Can you explain why it is needed?
   
   based on the 2nd section in https://docs.sqlalchemy.org/en/13/orm/join_conditions.html#creating-custom-foreign-conditions
   
   >An alternative syntax to the above is to use the foreign() and remote() annotations, inline within the relationship.primaryjoin expression. This syntax represents the annotations that relationship() normally applies by itself to the join condition given the relationship.foreign_keys and relationship.remote_side arguments. These functions may be more succinct when an explicit join condition is present, and additionally serve to mark exactly the column that is “foreign” or “remote” independent of whether that column is stated multiple times or within complex SQL expressions:




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org