On 03/06/2017 08:08 PM, Andres Freund wrote:
Hi,
On 2017-03-06 19:49:56 +0100, Tomas Vondra wrote:
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.
I'm not quite following here. We only get a Datum and the knowledge
that it's a pass-by-ref argument, so we really don't know that much. We
could create an "EmbeddedDatum" type that has a preceding chunk header
(appropriately for the version), that just gets zeroed out at start. Is
that what you mean?
Yes, that's roughly what I meant.
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?
I don't recall any recent numbers. I'm more than a bit doubful that it
really matters - it's only used for the results of aggregate/window
functions, and surely they've a good chunk of their own overhead...
And if the benefit is negligible, trying to keep the optimization might
easily result in slowdown (compared to non-optimized code).
But I'm puzzled why we haven't seen any reports of failures? I mean,
doing sum(int4) is not particularly extravagant thing, if there really
is an issue, shouldn't we see a lot of reports? What are we missing?
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
