Re: [HACKERS] Free indexed_tlist memory explicitly within set_plan_refs()

2015-07-24 Thread Andres Freund
On 2015-07-12 22:45:18 +0200, Andres Freund wrote: I'm right now not really coming up with a better idea how to fix this. So I guess I'll apply something close to this tomorrow. Took a bit longer than that :( I've pushed a version of your patch. I just opted to remove p_is_update instead of

Re: [HACKERS] Free indexed_tlist memory explicitly within set_plan_refs()

2015-07-24 Thread Andres Freund
On 2015-05-28 18:31:56 -0700, Peter Geoghegan wrote: Subject: [PATCH 3/3] Fix bug in unique index inference ON CONFLICT unique index inference had a thinko that could affect cases where the user-supplied inference clause required that an attribute match a particular (user named) collation

Re: [HACKERS] Free indexed_tlist memory explicitly within set_plan_refs()

2015-07-24 Thread Peter Geoghegan
On Fri, Jul 24, 2015 at 2:58 AM, Andres Freund and...@anarazel.de wrote: I've pushed a version of your patch. I just opted to remove p_is_update instead of allowing both to be set at the same time. To me that seemed simpler. I would be hesitant to remove a struct field from a struct that

Re: [HACKERS] Free indexed_tlist memory explicitly within set_plan_refs()

2015-07-24 Thread Andres Freund
On July 24, 2015 7:57:43 PM GMT+02:00, Peter Geoghegan p...@heroku.com wrote: On Fri, Jul 24, 2015 at 2:58 AM, Andres Freund and...@anarazel.de wrote: I've pushed a version of your patch. I just opted to remove p_is_update instead of allowing both to be set at the same time. To me that seemed

Re: [HACKERS] Free indexed_tlist memory explicitly within set_plan_refs()

2015-07-24 Thread Peter Geoghegan
On Fri, Jul 24, 2015 at 3:08 AM, Andres Freund and...@anarazel.de wrote: + else + { + Node *nattExpr = list_nth(idxExprs, (natt - 1) - nplain); + + /* + * Note that unlike routines like

Re: [HACKERS] Free indexed_tlist memory explicitly within set_plan_refs()

2015-07-12 Thread Andres Freund
On 2015-05-28 14:37:43 -0700, Peter Geoghegan wrote: To fix, allow ParseState to reflect that an individual statement can be both p_is_insert and p_is_update at the same time. /* Process DO UPDATE */ if (onConflictClause-action == ONCONFLICT_UPDATE) { + /*

Re: [HACKERS] Free indexed_tlist memory explicitly within set_plan_refs()

2015-07-12 Thread Peter Geoghegan
On Sun, Jul 12, 2015 at 1:45 PM, Andres Freund and...@anarazel.de wrote: But that's more crummy API's fault than yours. As you probably noticed, the only reason the p_is_update and p_is_insert fields exist is for transformAssignedExpr() -- in fact, in the master branch, nothing checks the value

Re: [HACKERS] Free indexed_tlist memory explicitly within set_plan_refs()

2015-05-30 Thread Peter Geoghegan
On Sat, May 30, 2015 at 3:12 PM, Peter Geoghegan p...@heroku.com wrote: Debugging this allowed me to come up with a significantly simplified approach. Attached is a new version of the original fix. Details are in commit message -- there is no actual need to have search_indexed_tlist_for_var()

Re: [HACKERS] Free indexed_tlist memory explicitly within set_plan_refs()

2015-05-30 Thread Peter Geoghegan
On Sat, May 30, 2015 at 1:07 AM, Peter Geoghegan p...@heroku.com wrote: My fix for this issue (0001-Fix-bug-with-whole-row-Vars-in-excluded-targetlist.patch) still missed something. There needs to be additional handling in ruleutils.c: Debugging this allowed me to come up with a significantly

Re: [HACKERS] Free indexed_tlist memory explicitly within set_plan_refs()

2015-05-30 Thread Peter Geoghegan
On Thu, May 28, 2015 at 2:37 PM, Peter Geoghegan p...@heroku.com wrote: Attached, revised version incorporates this small fix, while adding an additional big fix, and a number of small style tweaks. This is mainly concerned with fixing the bug I was trying to fix when I spotted the minor

Re: [HACKERS] Free indexed_tlist memory explicitly within set_plan_refs()

2015-05-29 Thread Peter Geoghegan
On Thu, May 28, 2015 at 6:31 PM, Peter Geoghegan p...@heroku.com wrote: This concerns a thinko in unique index inference. See the commit message for full details. It seems I missed a required defensive measure here. Attached patch adds it, too. -- Peter Geoghegan From

Re: [HACKERS] Free indexed_tlist memory explicitly within set_plan_refs()

2015-05-28 Thread Peter Geoghegan
On Sun, May 24, 2015 at 6:17 PM, Peter Geoghegan p...@heroku.com wrote: Attached patch adds such a pfree() call. Attached, revised version incorporates this small fix, while adding an additional big fix, and a number of small style tweaks. This is mainly concerned with fixing the bug I was

Re: [HACKERS] Free indexed_tlist memory explicitly within set_plan_refs()

2015-05-28 Thread Peter Geoghegan
On Thu, May 28, 2015 at 2:37 PM, Peter Geoghegan p...@heroku.com wrote: A second attached patch fixes another, largely independent bug. I noticed array assignment with ON CONFLICT DO UPDATE was broken. See commit message for full details. Finally, here is a third patch, fixing the final bug