Re: nitpick about poor style in MergeAttributes

2019-06-05 Thread Michael Paquier
On Tue, Jun 04, 2019 at 04:18:30PM -0400, Alvaro Herrera wrote: > Yeah, I was not quite understanding why it was being blamed on a commit > that actually *removed* one other callsite that did the same thing. (I > didn't actually realize at the time that this bug was there, mind.) I completely

Re: nitpick about poor style in MergeAttributes

2019-06-04 Thread Alvaro Herrera
On 2019-May-23, Mark Dilger wrote: > On Thu, May 23, 2019 at 5:24 PM Michael Paquier wrote: > > Looking closer, this code is not new as of v12. We have that since > > e7b3349 which has introduced CREATE TABLE OF. Yeah, I was not quite understanding why it was being blamed on a commit that

Re: nitpick about poor style in MergeAttributes

2019-05-23 Thread Mark Dilger
On Thu, May 23, 2019 at 5:24 PM Michael Paquier wrote: > > On Thu, May 23, 2019 at 08:27:19AM -0700, Mark Dilger wrote: > > On Thu, May 23, 2019 at 7:54 AM Tom Lane wrote: > >> Are we sure that's not just a newly-introduced bug, ie it has not > >> been tested in cases where the tlist could

Re: nitpick about poor style in MergeAttributes

2019-05-23 Thread Michael Paquier
On Thu, May 23, 2019 at 08:27:19AM -0700, Mark Dilger wrote: > On Thu, May 23, 2019 at 7:54 AM Tom Lane wrote: >> Are we sure that's not just a newly-introduced bug, ie it has not >> been tested in cases where the tlist could become empty? My first >> thought would be to assign the list pointer

Re: nitpick about poor style in MergeAttributes

2019-05-23 Thread Mark Dilger
On Thu, May 23, 2019 at 7:54 AM Tom Lane wrote: > > Michael Paquier writes: > > 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,

Re: nitpick about poor style in MergeAttributes

2019-05-23 Thread Tom Lane
Michael Paquier writes: > 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

Re: nitpick about poor style in MergeAttributes

2019-05-23 Thread Mark Dilger
On Wed, May 22, 2019 at 10:21 PM Michael Paquier 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

Re: nitpick about poor style in MergeAttributes

2019-05-22 Thread Michael Paquier
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

nitpick about poor style in MergeAttributes

2019-05-22 Thread Mark Dilger
Hackers, I have been auditing the v12 source code for places which inappropriately ignore the return value of a function and have found another example which seems to me a fertile source of future bugs. In src/backend/nodes/list.c, list_delete_cell frees the list and returns NIL when you delete