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:
[email protected]
