Re: nitpick about poor style in MergeAttributes

On Wed, May 22, 2019 at 06:20:01PM -0700, Mark Dilger wrote:
> What to do about this is harder to say. In the following
> patch, I'm just doing what I think is standard for callers
> of list_delete_cell, and assigning the return value back
> to the list (similar to how a call to repalloc should do).
> But since there is an implicit assumption that the list
> is never emptied by this operation, perhaps checking
> against NIL and elog'ing makes more sense?

Yes, I agree that this is a bit fuzzy, and this code is new as of
705d433. As you say, I agree that making sure that the return value
of list_delete_cell is not NIL is a sensible choice.

I don't think that an elog() is in place here though as this does not
rely directly on catalog contents, what about just an assertion?

Here is an idea of message for the elog(ERROR) if we go that way:
"no remaining columns after merging column \"%s\"".
--
Michael