On Wed, May 22, 2019 at 10:21 PM Michael Paquier <mich...@paquier.xyz> wrote: > > 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?
I think assigning the return value (as I did in my small patch) and then asserting that 'schema' is not NIL would be good. > Here is an idea of message for the elog(ERROR) if we go that way: > "no remaining columns after merging column \"%s\"". Perhaps. I like your idea of adding an assertion better. mark
