Re: nitpick about poor style in MergeAttributes - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: nitpick about poor style in MergeAttributes
Date
Msg-id 20190523052108.GE5870@paquier.xyz
Whole thread Raw
In response to nitpick about poor style in MergeAttributes  (Mark Dilger <hornschnorter@gmail.com>)
Responses Re: nitpick about poor style in MergeAttributes  (Mark Dilger <hornschnorter@gmail.com>)
Re: nitpick about poor style in MergeAttributes  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
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

Attachment

pgsql-hackers by date:

Previous
From: Andrew Gierth
Date:
Subject: Re: Patch to fix write after end of array in hashed agg initialization
Next
From: David Rowley
Date:
Subject: Re: PostgreSQL 12 Beta 1 press release draft