From: | Heikki Linnakangas <hlinnakangas(at)vmware(dot)com> |
---|---|
To: | Nicholas White <n(dot)j(dot)white(at)gmail(dot)com> |
Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net>, Jeff Davis <pgsql(at)j-davis(dot)com>, Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>, Troels Nielsen <bn(dot)troels(at)gmail(dot)com>, Hitoshi Harada <umi(dot)tanuki(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Re: Request for Patch Feedback: Lag & Lead Window Functions Can Ignore Nulls |
Date: | 2013-09-30 13:56:16 |
Message-ID: | 52498300.7000406@vmware.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 29.09.2013 23:32, Nicholas White wrote:
>> bms_add_member() is an accident waiting to happen
>
> I've attached a patch that makes it use repalloc as suggested
You'll have to zero out the extended portion.
I tried to demonstrate that by setting RANDOMIZE_ALLOCATED_MEMORY, but
surprisingly regression tests still passed. I guess the regression suite
doesn't use wide enough bitmapsets to exercise that. But this causes an
assertion failure, with RANDOMIZE_ALLOCATED_MEMORY:
create table t (i int4);
select * from t as t1, t as t2, t as t3, t as t4, t as t5, t as t6, t as
t7, t as t8, t as t9, t as t10, t as t11, t as t12, t as t13, t as t14,
t as t15, t as t16, t as t17, t as t18, t as t19, t as t20, t as t21, t
as t22, t as t23, t as t24, t as t25, t as t26, t as t27, t as t28, t as
t29, t as t30, t as t31, t as t32, t as t33, t as t34, t as t35, t as
t36, t as t37, t as t38, t as t39, t as t40;
> - is it OK to commit separately? I'll address the lead-lag patch
> comments in the next couple of days. Thanks
Yep, thanks. I committed the attached.
After thinking about this some more, I realized that bms_add_member() is
still sensitive to CurrentMemoryContext, if the 'a' argument is NULL.
That's probably OK for the lag&lead patch - I didn't check - but if
we're going to start relying on the fact that bms_add_member() no longer
allocates a new bms in CurrentMemoryContext, that needs to be
documented. bitmapset.c currently doesn't say a word about memory contexts.
So what needs to be done next is to document how the functions in
bitmapset.c work wrt. memory contexts. Then double-check that the
behavior of all the other "recycling" bms functions is consistent. (At
least bms_add_members() needs a similar change).
- Heikki
Attachment | Content-Type | Size |
---|---|---|
0001-In-bms_add_member-use-repalloc-if-the-bms-needs-to-b.patch | text/x-diff | 1.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2013-09-30 14:50:28 | Cmpact commits and changeset extraction |
Previous Message | Peter Eisentraut | 2013-09-30 13:15:00 | Re: [PERFORM] Cpu usage 100% on slave. s_lock problem. |