Re: [HACKERS] tuplesort_gettuple_common() and *should_free argument

On 2017-04-06 14:57:56 -0700, Peter Geoghegan wrote: > On Thu, Apr 6, 2017 at 2:50 PM, Andres Freund wrote: > > Pushed with very minor wording changes. > > This had a typo: > > + * If copy is true, the slot receives a copied tuple that'll that will stay Belatedly fixed.

Re: [HACKERS] tuplesort_gettuple_common() and *should_free argument

On Thu, Apr 6, 2017 at 2:50 PM, Andres Freund wrote: > Pushed with very minor wording changes. This had a typo: + * If copy is true, the slot receives a copied tuple that'll that will stay -- Peter Geoghegan VMware vCenter Server https://www.vmware.com/ -- Sent via

Re: [HACKERS] tuplesort_gettuple_common() and *should_free argument

On Thu, Apr 6, 2017 at 2:50 PM, Andres Freund wrote: > Pushed with very minor wording changes. Thanks Andres. -- Peter Geoghegan VMware vCenter Server https://www.vmware.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your

Re: [HACKERS] tuplesort_gettuple_common() and *should_free argument

On 2017-03-13 18:14:07 -0700, Peter Geoghegan wrote: > On Wed, Jan 25, 2017 at 3:11 PM, Peter Geoghegan wrote: > > On Wed, Jan 25, 2017 at 3:11 PM, Tom Lane wrote: > >> Please. You might want to hit the existing ones with a separate patch, > >> but it

Re: [HACKERS] tuplesort_gettuple_common() and *should_free argument

On Thu, Apr 6, 2017 at 2:05 PM, Andres Freund wrote: > I'm not sure you entirely got my point here. If tuplesort_gettupleslot > is called with copy = true, it'll store that tuple w/ > ExecStoreMinimalTuple passing shouldFree = copy = true. If the caller > is in a short-lived

Re: [HACKERS] tuplesort_gettuple_common() and *should_free argument

On 2017-04-04 13:49:11 -0700, Peter Geoghegan wrote: > On Tue, Apr 4, 2017 at 1:32 PM, Andres Freund wrote: > >> static bool > >> tuplesort_gettuple_common(Tuplesortstate *state, bool forward, > >> @@ -2091,12 +2092,15 @@ tuplesort_gettuple_common(Tuplesortstate *state, >

Re: [HACKERS] tuplesort_gettuple_common() and *should_free argument

The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: not tested Documentation:not tested The patch looks fine to me. Changes are clear and all functions are

Re: [HACKERS] tuplesort_gettuple_common() and *should_free argument

On Tue, Apr 4, 2017 at 1:32 PM, Andres Freund wrote: > s/Avoid/Allow to avoid/ WFM. >> + * >> + * Callers cannot rely on memory for tuple in returned slot remaining valid >> + * past any subsequent manipulation of the sorter, such as another fetch of >> + * input from

Re: [HACKERS] tuplesort_gettuple_common() and *should_free argument

On 2017-03-13 18:14:07 -0700, Peter Geoghegan wrote: > From 5351b5db257cb39832647d9117465c0217e6268b Mon Sep 17 00:00:00 2001 > From: Peter Geoghegan > Date: Thu, 13 Oct 2016 10:54:31 -0700 > Subject: [PATCH 1/2] Avoid copying within tuplesort_gettupleslot(). s/Avoid/Allow to

Re: [HACKERS] tuplesort_gettuple_common() and *should_free argument

Hi Anastasia, On 3/13/17 9:14 PM, Peter Geoghegan wrote: > On Wed, Jan 25, 2017 at 3:11 PM, Peter Geoghegan wrote: >> On Wed, Jan 25, 2017 at 3:11 PM, Tom Lane wrote: >>> Please. You might want to hit the existing ones with a separate patch, >>> but it

Re: [HACKERS] tuplesort_gettuple_common() and *should_free argument

On Wed, Jan 25, 2017 at 3:11 PM, Peter Geoghegan wrote: > On Wed, Jan 25, 2017 at 3:11 PM, Tom Lane wrote: >> Please. You might want to hit the existing ones with a separate patch, >> but it doesn't much matter; I'd be just as happy with a patch that did >>

Re: [HACKERS] tuplesort_gettuple_common() and *should_free argument

On Wed, Jan 25, 2017 at 3:16 PM, Tom Lane wrote: > Um, I didn't find it all that self-explanatory. Why wouldn't we want > to avoid writing undefined data? For roughly the same reason we'd want to avoid it in existing cases that are next to the proposed new suppression. We

Re: [HACKERS] tuplesort_gettuple_common() and *should_free argument

On Mon, Mar 13, 2017 at 8:23 AM, David Steele wrote: > It's been a while since there was a new patch or any activity on this > thread. > > If you need more time to produce a patch, please post an explanation for > the delay and a schedule for the new patch. If no patch or

Re: [HACKERS] tuplesort_gettuple_common() and *should_free argument

Hi Peter, On 3/2/17 9:43 AM, David Steele wrote: > Peter, > > On 2/1/17 12:59 AM, Michael Paquier wrote: >> On Thu, Jan 26, 2017 at 8:16 AM, Tom Lane wrote: >>> [ in the service of closing out this thread... ] >>> >>> Peter Geoghegan writes: Finally,

Re: [HACKERS] tuplesort_gettuple_common() and *should_free argument

Peter, On 2/1/17 12:59 AM, Michael Paquier wrote: > On Thu, Jan 26, 2017 at 8:16 AM, Tom Lane wrote: >> [ in the service of closing out this thread... ] >> >> Peter Geoghegan writes: >>> Finally, 0003-* is a Valgrind suppression borrowed from my parallel >>>

Re: [HACKERS] tuplesort_gettuple_common() and *should_free argument

On Thu, Jan 26, 2017 at 8:16 AM, Tom Lane wrote: > [ in the service of closing out this thread... ] > > Peter Geoghegan writes: >> Finally, 0003-* is a Valgrind suppression borrowed from my parallel >> CREATE INDEX patch. It's self-explanatory. > > Um, I

Re: [HACKERS] tuplesort_gettuple_common() and *should_free argument

[ in the service of closing out this thread... ] Peter Geoghegan writes: > Finally, 0003-* is a Valgrind suppression borrowed from my parallel > CREATE INDEX patch. It's self-explanatory. Um, I didn't find it all that self-explanatory. Why wouldn't we want to avoid writing

Re: [HACKERS] tuplesort_gettuple_common() and *should_free argument

On Wed, Jan 25, 2017 at 3:11 PM, Tom Lane wrote: > Please. You might want to hit the existing ones with a separate patch, > but it doesn't much matter; I'd be just as happy with a patch that did > both things. Got it. -- Peter Geoghegan -- Sent via pgsql-hackers

Re: [HACKERS] tuplesort_gettuple_common() and *should_free argument

Peter Geoghegan writes: > It means "another call to tuplesort_gettupleslot", but I believe that > it would be safer (more future-proof) to actually specify "the slot > contents may be invalidated by any subsequent manipulation of the > tuplesort's state" instead. WFM. >> There

Re: [HACKERS] tuplesort_gettuple_common() and *should_free argument

On Wed, Jan 25, 2017 at 2:49 PM, Tom Lane wrote: > I looked at the 0002 patch, and while the code is probably OK, I am > dissatisfied with this API spec: > > + * If copy is TRUE, the slot receives a copied tuple that will stay valid > + * regardless of future manipulations of

Re: [HACKERS] tuplesort_gettuple_common() and *should_free argument

Peter Geoghegan writes: > I should have already specifically pointed out that the original > discussion on what became 0002-* is here: > postgr.es/m/7256.1476711...@sss.pgh.pa.us > As I said already, the general idea seems uncontroversial. I looked at the 0002 patch, and while

Re: [HACKERS] tuplesort_gettuple_common() and *should_free argument

On Mon, Dec 12, 2016 at 12:59 PM, Robert Haas wrote: > Committed 0001. Thanks. I should have already specifically pointed out that the original discussion on what became 0002-* is here: postgr.es/m/7256.1476711...@sss.pgh.pa.us As I said already, the general idea seems

Re: [HACKERS] tuplesort_gettuple_common() and *should_free argument

On Mon, Dec 12, 2016 at 2:30 PM, Peter Geoghegan wrote: > On Mon, Dec 12, 2016 at 9:31 AM, Robert Haas wrote: >> I think this patch might have a bug. In the existing code, >> tuplesort_gettupleslot sets should_free = true if it isn't already >> just

Re: [HACKERS] tuplesort_gettuple_common() and *should_free argument

On Mon, Dec 12, 2016 at 9:31 AM, Robert Haas wrote: > I think this patch might have a bug. In the existing code, > tuplesort_gettupleslot sets should_free = true if it isn't already > just before calling ExecStoreMinimalTuple((MinimalTuple) stup.tuple, > slot,

Re: [HACKERS] tuplesort_gettuple_common() and *should_free argument

On Fri, Dec 9, 2016 at 5:59 PM, Peter Geoghegan wrote: > Attached patch 0001-* removes all should_free arguments. To reiterate, > this is purely a refactoring patch. I think this patch might have a bug. In the existing code, tuplesort_gettupleslot sets should_free = true if it

Re: [HACKERS] tuplesort_gettuple_common() and *should_free argument

On Fri, Oct 21, 2016 at 4:45 PM, Peter Geoghegan wrote: > More importantly, there are no remaining cases where > tuplesort_gettuple_common() sets "*should_free = true", because there > is no remaining need for caller to *ever* pfree() tuple. Moreover, I > don't anticipate any

Re: [HACKERS] tuplesort_gettuple_common() and *should_free argument

Peter Geoghegan writes: > On Wed, Dec 7, 2016 at 11:55 PM, Heikki Linnakangas wrote: >> Should we be worried about breaking the API of tuplesort_get* functions? >> They might be used by extensions. I think that's OK, but wanted to bring it >> up. This would be

Re: [HACKERS] tuplesort_gettuple_common() and *should_free argument

On Wed, Dec 7, 2016 at 11:55 PM, Heikki Linnakangas wrote: > Should we be worried about breaking the API of tuplesort_get* functions? > They might be used by extensions. I think that's OK, but wanted to bring it > up. This would be only for master, of course, and any breakage

Re: [HACKERS] tuplesort_gettuple_common() and *should_free argument

On 10/22/2016 02:45 AM, Peter Geoghegan wrote: I noticed that the routine tuplesort_gettuple_common() fails to set *should_free in all paths in master branch (no bug in 9.6). Consider the TSS_SORTEDONTAPE case, where we can return false without also setting "*should_free = false" to instruct

[HACKERS] tuplesort_gettuple_common() and *should_free argument

I noticed that the routine tuplesort_gettuple_common() fails to set *should_free in all paths in master branch (no bug in 9.6). Consider the TSS_SORTEDONTAPE case, where we can return false without also setting "*should_free = false" to instruct caller not to pfree() tuple memory that tuplesort