On 03/06/2017 07:05 PM, Robert Haas wrote:
On Mon, Mar 6, 2017 at 12:44 PM, Andres Freund <and...@anarazel.de> wrote:
On 2017-03-06 12:40:18 -0500, Robert Haas wrote:
On Wed, Mar 1, 2017 at 5:55 PM, Andres Freund <and...@anarazel.de> wrote:
The issue was that on 32bit platforms the Datum returned by some
functions (int2int4_sum in this case) isn't actually a separately
allocated Datum, but rather just something embedded in a larger
struct. That, combined with the following code:
if (!peraggstate->resulttypeByVal && !*isnull &&
!MemoryContextContains(CurrentMemoryContext,
DatumGetPointer(*result)))
seems somewhat problematic to me. MemoryContextContains() can give
false positives when used on memory that's not a distinctly allocated
chunk, and if so, we violate memory lifetime rules. It's quite
unlikely, given the required bit patterns, but nonetheless it's making
me somewhat uncomfortable.
Do others think this isn't an issue and we can just live with it?
I think it's 100% broken to call MemoryContextContains() on something
that's not guaranteed to be a palloc'd chunk.
I agree, but to me it seems the only fix would be to just yank out the
whole optimization?
Dunno, haven't looked into it.
I think it might be fixable by adding a flag into the chunk, with 'true'
for regular allocations, and 'false' for the optimized ones. And then
only use MemoryContextContains() for 'flag=true' chunks.
The question however is whether this won't make the optimization
pointless. I also, wonder how much we save by this optimization and how
widely it's used? Can someone point me to some numbers?
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
