Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename database tables #5619

Open
gravitystorm opened this issue Feb 5, 2025 · 2 comments
Open

Rename database tables #5619

gravitystorm opened this issue Feb 5, 2025 · 2 comments
Labels
dx Developer Experience refactor Refactoring, or things that work but could be done better

Comments

@gravitystorm
Copy link
Collaborator

For historical reasons, we've ended up with quite a few database tables names that aren't aligned with the model names. I think in every case we prefer the model names (since these are easier to change, they tend to have been changed already without renaming the underlying table).

andy@denali:~/src/openstreetmap-website$ grep -rn 'self\.table_name' app/models/
app/models/old_way.rb:24:  self.table_name = "ways"
app/models/old_way_tag.rb:16:  self.table_name = "way_tags"
app/models/way_tag.rb:15:  self.table_name = "current_way_tags"
app/models/tracepoint.rb:26:  self.table_name = "gps_points"
app/models/trace.rb:31:  self.table_name = "gpx_files"
app/models/way_node.rb:20:  self.table_name = "current_way_nodes"
app/models/relation_member.rb:21:  self.table_name = "current_relation_members"
app/models/node.rb:31:  self.table_name = "current_nodes"
app/models/way.rb:26:  self.table_name = "current_ways"
app/models/old_way_node.rb:20:  self.table_name = "way_nodes"
app/models/old_relation_tag.rb:16:  self.table_name = "relation_tags"
app/models/old_relation.rb:24:  self.table_name = "relations"
app/models/relation.rb:26:  self.table_name = "current_relations"
app/models/old_relation_member.rb:22:  self.table_name = "relation_members"
app/models/node_tag.rb:15:  self.table_name = "current_node_tags"
app/models/old_node.rb:30:  self.table_name = "nodes"
app/models/relation_tag.rb:15:  self.table_name = "current_relation_tags"
app/models/follow.rb:22:  self.table_name = "friends"
app/models/tracetag.rb:20:  self.table_name = "gpx_file_tags"
app/models/old_node_tag.rb:16:  self.table_name = "node_tags"

We might also want to rename certain tables that don't currently have a model, such as changeset_subscribers (to e.g. changeset_subscriptions) but I haven't made a list of these.

Having these mismatched table/model names doesn't provide any benefits to anyone, as far as I can tell. It also makes it harder for new developers to get started, since they might look at the database structure (e.g. #5308) and then get confused when they can't find the corresponding models. In particular, I found it very confusing when I first started, since we have a table called "nodes" and a "Node" model, but these do not correspond with each other!

So my proposal is to rename the tables to match the model names.

There is a zero-downtime approach to renaming tables in PostgreSQL, as described at https://brandur.org/fragments/postgres-table-rename . In short, we

  • Rename the table, and create a view with the old name, in a transaction.
  • Update any code that accesses that table (e.g. rails, cgimap, replication utilities) to use the new table name directly.
  • Eventually, drop the view.

For a small number of renames, we will need to do this in multiple stages, due to conflicts (e.g. nodes -> old_nodes then current_nodes -> nodes some time later).

Before getting started on this, my main question is whether the OSMF operations team (e.g. @tomhughes @Firefishy ) see any problem with this, for example, any details about how the OSMF database replication works that would make this a non-starter. Or if there's any reasons that I haven't considered why this might not work.

@gravitystorm gravitystorm added dx Developer Experience refactor Refactoring, or things that work but could be done better labels Feb 5, 2025
@mmd-osm
Copy link
Contributor

mmd-osm commented Feb 5, 2025

So my proposal is to rename the tables to match the model names.

Have you also evaluated the other way around, i.e. change the model names to match the db table names? Assuming that's feasible, that's significantly less risk and effort for anything non-Rails (planetdump-ng, osmdbt, cgimap, changeset replication).

In particular the planetdump will probably not work at all with views, since it's using a Postgresql dump to generate the planet pbf files.

osmdbt will probably require some changes, too. My understanding is that even though you're creating additional views to mimic the previous table names, logical replication would still be based on the underlying tables.

Multiple transition steps from nodes -> old_nodes and then again current_nodes -> nodes need to be very carefully coordinated across at least 4 external tools.

I really don't see the need for this change, which in the end is a cosmetic change with a somewhat unfavorable cost-benefit ratio.

@nenad-vujicic
Copy link
Contributor

So my proposal is to rename the tables to match the model names.

Have you also evaluated the other way around, i.e. change the model names to match the db table names? Assuming that's feasible, that's significantly less risk and effort for anything non-Rails (planetdump-ng, osmdbt, cgimap, changeset replication).

Honestly, changing the model names seems like a much better option than altering the database table names in this case, as it improves readability. For example, current_nodes (with the model name Node vs CurrentNode) contains only the latest (the current) versions of nodes, while nodes (with the model name OldNode vs Node) includes all versions of node definitions. I believe this falls under the category of naming things (like #5539) and is highly subjective, so opinions will vary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dx Developer Experience refactor Refactoring, or things that work but could be done better
Projects
None yet
Development

No branches or pull requests

3 participants