Towards easier AMs: Cleaning up inappropriate use of name "relkind"

From: Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>
To: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Towards easier AMs: Cleaning up inappropriate use of name "relkind"
Date: 2020-06-03 17:05:26
Message-ID: 609181AE-E399-47C7-9221-856E0F96BF93@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hackers,

The name "relkind" normally refers to a field of type 'char' with values like 'r' for "table" and 'i' for "index". In AlterTableStmt and CreateTableAsStmt, this naming convention was abused for a field of type enum ObjectType. Often, such fields are named "objtype", though also "kind", "removeType", "renameType", etc.

I don't care to debate those other names, though in passing I'll say that "kind" seems not great. The "relkind" name is particularly bad, though. It is confusing in functions that also operate on a RangeTblEntry object, which also has a field named "relkind", and is confusing in light of the function get_relkind_objtype() which maps from "relkind" to "objtype", implying quite correctly that those two things are distinct.

The attached patch cleans this up. How many toes am I stepping on here? Changing the names was straightforward and doesn't seem to cause any issues with 'make check-world'. Any objection?

For those interested in the larger context of this patch, I am trying to clean up any part of the code that makes it harder to write and test new access methods. When implementing a new AM, one currently needs to `grep -i relkind` to find a long list of files that need special treatment. One then needs to consider whether special logic for the new AM needs to be inserted into all these spots. As such, it is nice if these spots have as little confusing naming as possible. This patch makes that process a little easier. I have another patch (to be posted shortly) that cleans up the #define RELKIND_XXX stuff using a new RelKind enum and special macros while keeping the relkind fields as type 'char'. Along with converting code to use switch(relkind) rather than if (relkind...) statements, the compiler now warns on unhandled cases when you add a new RelKind to the list, making it easier to find all the places you need to update. I decided to keep that work independent of this patch, as the code is logically distinct.

Attachment Content-Type Size
v1-0001-Renaming-relkind-as-objtype-where-appropriate.patch application/octet-stream 13.5 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2020-06-03 17:26:28 Re: Towards easier AMs: Cleaning up inappropriate use of name "relkind"
Previous Message Martín Marqués 2020-06-03 16:32:28 Re: Read access for pg_monitor to pg_replication_origin_status view