Thread: need more reviewers for index changes
Folks, There has as yet been no review of the latest version of the very important "Indexam API changes" patch or the also-very-important "Index-only quals" patch which depends on it. Volunteers, please? ...Robert
On Tue, Jul 21, 2009 at 12:02 AM, Robert Haas<robertmhaas@gmail.com> wrote: > Folks, > > There has as yet been no review of the latest version of the very > important "Indexam API changes" patch or the also-very-important > "Index-only quals" patch which depends on it. Volunteers, please? > i've sent a new version of the patch to fix the bug for dropped columns, i've marked it as "needs review"... so now i'm available until one of the other patches are reviewed... which one you prefer i start to play with? -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157
On Tue, Jul 21, 2009 at 1:07 AM, Jaime Casanova<jcasanov@systemguards.com.ec> wrote: > On Tue, Jul 21, 2009 at 12:02 AM, Robert Haas<robertmhaas@gmail.com> wrote: >> Folks, >> >> There has as yet been no review of the latest version of the very >> important "Indexam API changes" patch or the also-very-important >> "Index-only quals" patch which depends on it. Volunteers, please? >> > > i've sent a new version of the patch to fix the bug for dropped > columns, i've marked it as "needs review"... so now i'm available > until one of the other patches are reviewed... > > which one you prefer i start to play with? It probably makes sense to start with indexam api changes, since the other one depends on that one. ...Robert
Robert, * Robert Haas (robertmhaas@gmail.com) wrote: > It probably makes sense to start with indexam api changes, since the > other one depends on that one. I've looked the patch over, but the problem is that the patch doesn't do terribly much by itself, and Tom's already commented on things he doesn't like about it. Alot of the patch here: http://archives.postgresql.org/message-id/4A5ADFE6.6060507@enterprisedb.com is changing heap_hot_search_buffer to have heapTuple passed in as a pointer rather than a local var, which makes for many '.' to '->' changes. The rest is just splitting index_getnext into two pieces. To be honest, I'm not convinced I'm qualified to review this patch anyway, but I was giving it a go. Of course, it strikes me that this patch has had a fair bit of feedback from committers already, and anything I try to add will likely end up being wrong anyway. :) Thanks, Stephen
Attachment
On Tue, Jul 21, 2009 at 6:40 AM, Stephen Frost<sfrost@snowman.net> wrote: > Robert, > > * Robert Haas (robertmhaas@gmail.com) wrote: >> It probably makes sense to start with indexam api changes, since the >> other one depends on that one. > > I've looked the patch over, but the problem is that the patch doesn't do > terribly much by itself, and Tom's already commented on things he > doesn't like about it. Alot of the patch here: > http://archives.postgresql.org/message-id/4A5ADFE6.6060507@enterprisedb.com > is changing heap_hot_search_buffer to have heapTuple passed in as a > pointer rather than a local var, which makes for many '.' to '->' > changes. The rest is just splitting index_getnext into two pieces. Ah, yes, I see your point. So I think this is actually waiting on author. We should poke Heikki. ...Robert
On 7/21/09, Robert Haas <robertmhaas@gmail.com> wrote: > On Tue, Jul 21, 2009 at 6:40 AM, Stephen Frost<sfrost@snowman.net> wrote: >> Robert, >> >> * Robert Haas (robertmhaas@gmail.com) wrote: >>> It probably makes sense to start with indexam api changes, since the >>> other one depends on that one. >> >> I've looked the patch over, but the problem is that the patch doesn't do >> terribly much by itself, and Tom's already commented on things he >> doesn't like about it. Alot of the patch here: >> http://archives.postgresql.org/message-id/4A5ADFE6.6060507@enterprisedb.com >> is changing heap_hot_search_buffer to have heapTuple passed in as a >> pointer rather than a local var, which makes for many '.' to '->' >> changes. The rest is just splitting index_getnext into two pieces. > > Ah, yes, I see your point. So I think this is actually waiting on > author. We should poke Heikki. I believe he's on vacation. -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com
Robert,
* Robert Haas (robertmhaas@gmail.com) wrote:
> Ah, yes, I see your point. So I think this is actually waiting on
> author. We should poke Heikki.
Feel free to give me another patch.
Thanks,
Stephen
Attachment
On Tue, Jul 21, 2009 at 5:41 PM, Stephen Frost<sfrost@snowman.net> wrote: > * Robert Haas (robertmhaas@gmail.com) wrote: >> Ah, yes, I see your point. So I think this is actually waiting on >> author. We should poke Heikki. > > Feel free to give me another patch. How 'bout lock wait statistics? ...Robert