Re: Improve readability by using designated initializers when possible

On 27.02.24 08:57, Alvaro Herrera wrote:
> On 2024-Feb-27, Michael Paquier wrote:
> 
>> These would cause compilation failures.  Saying that, this is a very
>> nice cleanup, so I've fixed these and applied the patch after checking
>> that the one-one replacements were correct.
> 
> Oh, I thought we were going to get rid of ObjectClass altogether -- I
> mean, have getObjectClass() return ObjectAddress->classId, and then
> define the OCLASS values for each catalog OID [... tries to ...]  But
> this(*) doesn't work for two reasons:

I have long wondered what the point of ObjectClass is.  I find the extra 
layer of redirection, which is used only in small parts of the code, and 
the similarity to ObjectType confusing.  I happened to have a draft 
patch for its removal lying around, so I'll show it here, rebased over 
what has already been done in this thread.

> 1. some switches processing the OCLASS enum don't have "default:" cases.
> This is so that the compiler tells us when we fail to add support for
> some new object class (and it's been helpful).  If we were to

I think you can also handle that with some assertions and proper test 
coverage.  It's not even clear how strong this benefit is.  For example, 
in AlterObjectNamespace_oid(), you could still put a new OCLASS into the 
"ignore object types that don't have schema-qualified names" case, and 
it might or might not be wrong.  Also, there are already various OCLASS 
switches that do have a default case, so it's not even clear what the 
preferred coding style should be.

> 2. all users of getObjectClass would have to include the catalog header
> specific to every catalog it wants to handle; so tablecmds.c and
> dependency.c would have to include almost all catalog includes, for
> example.

This doesn't seem to be a problem in practice; see patch.