Re: Improve readability by using designated initializers when possible

On Mon, Mar 04, 2024 at 09:29:03AM +0800, jian he wrote:
> On Fri, Mar 1, 2024 at 5:26 PM Peter Eisentraut <peter@eisentraut.org> wrote:
>> Oops, there was a second commit in my branch that I neglected to send
>> in.  Here is my complete patch set.

Thanks for the new patch set.  The gains are neat, giving nice
numbers:
 7 files changed, 200 insertions(+), 644 deletions(-)

+       default:
+           DropObjectById(object);
+           break;

Hmm.  I am not sure that this is a good idea.  Wouldn't it be safer to
use as default path something that generates an ERROR so as this code
path would complain immediately when adding a new catalog?  My point
is to make people consider what they should do on deletion when adding
a catalog that would take this code path, rather than assuming that a
deletion is OK to happen.  So I would recommend to keep the list of
catalog OIDs for the DropObjectById case, keep the list for global
objects, and add a third path with a new ERROR.

-           /*
-            * There's intentionally no default: case here; we want the
-            * compiler to warn if a new OCLASS hasn't been handled above.
-            */

In getObjectDescription() and getObjectTypeDescription() this was a
safeguard, but we don't have that anymore.  So it seems to me that
this should be replaced with a default with elog(ERROR)?

There is a third one in getObjectIdentityParts() that has not been
removed, though, but same remark at the two others.

RememberAllDependentForRebuilding() uses a default, so this one looks
good to me.

> there is a `OCLASS` at the end of getObjectIdentityParts.

Nice catch.  A comment is not updated.

> There is a `ObjectClass` in typedefs.list

This is usually taken care of by committers or updated automatically.
--
Michael