dstandish commented on a change in pull request #21806:
URL: https://github.com/apache/airflow/pull/21806#discussion_r815339573
##########
File path:
airflow/migrations/versions/c306b5b5ae4a_switch_xcom_table_to_use_run_id.py
##########
@@ -41,7 +41,7 @@
def _get_new_xcom_columns() -> Sequence[Column]:
return [
- Column("dagrun_id", Integer(), nullable=False),
+ Column("dag_run_id", Integer(), nullable=False),
Review comment:
> And dag_run is also an outlier in Airflow since most other tables e.g.
taskinstance do lack the undercores).
it's not actually true, they are all snake case. the table for
`TaskInstance` is `task_instance`. and there's not a single table in airflow
db that's not snake case table name. (But the _python module_ names are like
that.)
> I guess this is essentially down to personally preference
To some extent that's true but consistency and convention also count for
something. Particularly in the context of database object naming conventions,
in this area it's a lot easier to make conventions about than some other
decisions we have to make. And referring to the PK as `f"{table_name}_id` is i
think pretty standard[1]. E.g. you would not create a table like this:
```sql
create table dag_run (
dagrun_id int identity,
val varchar
)
```
That would not make any sense. Why not call the table `dagrun` in that
case? I think it's essentially the same here. The table is called `dag_run`
so it's ID should be referred to as `dag_run_id`. Since the table names have
already made the decision to go snake case, why should the columns go backwards
on that decision? And it's not that we can't ever deviate from the standard
choice, but that side of the argument bears the burden; there should be a good
reason to make an exception.
As always, I'll go with the majority view, but just trying to present what
makes sense to me. And of course it's always better if we can reach consensus.
---
[1] Jetbrains can even [suggest
joins](https://www.jetbrains.com/help/datagrip/foreign-keys.html#configure-rules-for-virtual-foreign-keys)
for this table naming pattern when no FK is defined.
--
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]
