On Wed, Jan 25, 2017 at 2:49 PM, Tom Lane <t...@sss.pgh.pa.us> 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 the tuplesort's state. Memory is > + * owned by the caller. If copy is FALSE, the slot may just receive a > pointer > + * to a tuple held within the tuplesort. The latter is more efficient, but > + * the slot contents may be corrupted if there is another call here before > + * previous slot contents are used. > > What does "here" mean? If that means specifically "another call of > tuplesort_gettupleslot", say so. If "here" refers to the whole module, > it would be better to say something like "the slot contents may be > invalidated by any subsequent manipulation of the tuplesort's state". > In any case it'd be a good idea to delineate safe usage patterns, perhaps > "copy=FALSE is recommended only when the next tuplesort manipulation will > be another tuplesort_gettupleslot fetch into the same slot."
I agree with your analysis. 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. > There are several other uses of "call here", both in this patch and > pre-existing in tuplesort.c, that I find equally vague and unsatisfactory. > Let's try to improve that. Should I write a patch along those lines? -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
