Re: CF 2009-09: initial reviewing assignments

And we're off!

Initial reviewing assignments are below. Please go to
http://commitfest.postgresql.org/action/commitfest_view/inprogress and
edit the patch you've been assigned, adding your name as a reviewer.
Please do this right away so that it's clear which patches still need
reviewers to be assigned. If possible, please attempt to complete
your initial review by end-of-day Saturday (five days from now). When
you post a review, please post a comment with your review AND ALSO
edit the status of the patch as appropriate - Waiting on Author,
Returned with Feedback (if not to be further considered this CF),
Ready for Committer, Rejected, etc.

Thanks,

...Robert

Jeff Davis <pgsql(at)j-davis(dot)com>
- Join optimization for inheritance tables

Jeff Janes <jeff(dot)janes(at)gmail(dot)com>
- opportunistic freezing during vacuum

Selena Deckelmann <selenamarie(at)gmail(dot)com>
- join removal

Robert Haas <robertmhaas(at)gmail(dot)com>
- Separate Heap Fetch from Index Scan

Andres Freund <andres(at)anarazel(dot)de>
- GRANT ON ALL IN schema

Stephen Frost <sfrost(at)snowman(dot)net>
- Reworks for Access Controls

Jaime Casanova <jcasanov(at)systemguards(dot)com(dot)ec>
- Largeobject access controls

Jan Urbański <wulczer(at)wulczer(dot)org>
- Default ACLs

Brendan Jurd <direvus(at)gmail(dot)com>
- Generalized Index Constraints

Joshua Tolley <eggyknap(at)gmail(dot)com>
- DML node in support of writeable CTEs

Dan Colish <dan(at)unencrypted(dot)org>
- generic copy options

David Fetter <david(at)fetter(dot)org>
- make plpgsql IN args mutable

Dimitri Fontaine <dfontaine(at)hi-media(dot)com>
- Anonymous code blocks

Nikhil Sontakke <nikhil(dot)sontakke(at)enterprisedb(dot)com>
- Linux LSB init script

David E. Wheeler <david(at)kineticode(dot)com>
- hstore enhancements

Josh Williams <joshwilliams(at)ij(dot)net> WINDOWS
- Encoding issues in console and eventlog on win32

Bernd Helmle <bernd(at)oopsware(dot)de>
- Allow more complex user/database default GUC settings

Logging configuration changes
- Abhijit Menon-Sen <ams(at)toroid(dot)org>

gabrielle <gorthx(at)gmail(dot)com> on behalf of PDXPUG CODE SPRINT
- Errcontext support in PL/Perl
- MOVE FORWARD n | BACKWARD n
- psql: Unicode UTF-8 table formatting for text output
- Pgbench Shell command
- unicode escapes (time permitting)

On Mon, Sep 14, 2009 at 9:08 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
Please go to
> http://commitfest.postgresql.org/action/commitfest_view/inprogress and
> edit the patch you've been assigned, adding your name as a reviewer.
> Please do this right away so that it's clear which patches still need
> reviewers to be assigned.

And by "right away" I mean "as soon as someone gets
wwwmaster.postgresql.org back on line so you can log in". :-(

It seems all community logins for all apps are down ATM.

...Robert

Robert Haas escreveu:
> Initial reviewing assignments are below.
>
I'm available to review too. I'll take 'Buffer usage in EXPLAIN and
pg_stat_statements', ok?

--
Euler Taveira de Oliveira
http://www.timbira.com/

On Mon, Sep 14, 2009 at 11:34 PM, Euler Taveira de Oliveira
<euler(at)timbira(dot)com> wrote:
> Robert Haas escreveu:
>> Initial reviewing assignments are below.
>>
> I'm available to review too. I'll take 'Buffer usage in EXPLAIN and
> pg_stat_statements', ok?

OK with me.

...Robert

At 2009-09-14 21:08:56 -0400, robertmhaas(at)gmail(dot)com wrote:
>
> Logging configuration changes
> - Abhijit Menon-Sen <ams(at)toroid(dot)org>

I reviewed this patch. It's fine. Ready for committer as-is.

But I suggested some minor changes that I thought were useful, and
posted a new version of the patch including those changes. Besides
that, I posted a patch to implement a suggestion Peter made in his
original posting.

What status should I assign the patch now? It's "Needs Review", in
a sense, but also "Waiting on Author", "Ready for Committer", and
"Returned with Feedback" ;-)

The patch is completely trivial, before and after, so I feel a bit silly
even to ask this question. Should I just advance it to "Ready"?

(I suppose my review will show up on -hackers by the time this message
shows up on -rrreviewers.)

-- ams

On Wed, Sep 16, 2009 at 1:38 AM, Abhijit Menon-Sen <ams(at)toroid(dot)org> wrote:
> At 2009-09-14 21:08:56 -0400, robertmhaas(at)gmail(dot)com wrote:
>>
>> Logging configuration changes
>> - Abhijit Menon-Sen <ams(at)toroid(dot)org>
>
> I reviewed this patch. It's fine. Ready for committer as-is.
>
> But I suggested some minor changes that I thought were useful, and
> posted a new version of the patch including those changes. Besides
> that, I posted a patch to implement a suggestion Peter made in his
> original posting.
>
> What status should I assign the patch now? It's "Needs Review", in
> a sense, but also "Waiting on Author", "Ready for Committer", and
> "Returned with Feedback" ;-)
>
> The patch is completely trivial, before and after, so I feel a bit silly
> even to ask this question. Should I just advance it to "Ready"?

Yep, Ready for Committer sounds good to me.

...Robert

On Wed, Sep 16, 2009 at 12:44 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Wed, Sep 16, 2009 at 1:38 AM, Abhijit Menon-Sen <ams(at)toroid(dot)org> wrote:
>> At 2009-09-14 21:08:56 -0400, robertmhaas(at)gmail(dot)com wrote:
>>>
>>> Logging configuration changes
>>> - Abhijit Menon-Sen <ams(at)toroid(dot)org>
>>
>> I reviewed this patch. It's fine. Ready for committer as-is.
>>
>> But I suggested some minor changes that I thought were useful, and
>> posted a new version of the patch including those changes. Besides
>> that, I posted a patch to implement a suggestion Peter made in his
>> original posting.
>>
>> What status should I assign the patch now? It's "Needs Review", in
>> a sense, but also "Waiting on Author", "Ready for Committer", and
>> "Returned with Feedback" ;-)
>>
>> The patch is completely trivial, before and after, so I feel a bit silly
>> even to ask this question. Should I just advance it to "Ready"?
>
> Yep, Ready for Committer sounds good to me.

Oh, and make sure to add a comment of type "Patch" with a link to your
version of the patch.

...Robert

Hi,

> Nikhil Sontakke <nikhil(dot)sontakke(at)enterprisedb(dot)com>
> - Linux LSB init script
>

Peter is already actively taking a look at this one. Please let me
know if I can help elsewhere or can just lurk around since this is a
small commitfest.

Regards,
Nikhils
--
http://www.enterprisedb.com

I am free to start on something new. Any suggestions?

--
--Dan

On Thu, Sep 17, 2009 at 10:00 PM, Dan Colish <dan(at)unencrypted(dot)org> wrote:
> I am free to start on something new. Any suggestions?

If you're game to dive into ECPG, there are a bunch of patches from
Zoltan that could use a review.

If not, either of the two patches under miscellaneous whose names
begin with "syslog" could use some further review. I don't think
that either of them are likely to be committed in their present form,
so the goal is not so much to ascertain that as to perhaps suggest
some other approaches that might be workable.

...Robert

On Tue, Sep 15, 2009 at 9:55 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Mon, Sep 14, 2009 at 11:34 PM, Euler Taveira de Oliveira
> <euler(at)timbira(dot)com> wrote:
>> Robert Haas escreveu:
>>> Initial reviewing assignments are below.
>>>
>> I'm available to review too. I'll take 'Buffer usage in EXPLAIN and
>> pg_stat_statements', ok?
>
> OK with me.

If you're going to review this patch, you should edit it and list your
name as a reviewer, so that others know that it is already being
reviewed. If you're not going to review this patch, please let us
know so that someone else can pick it up.

...Robert

On Thu, Sep 17, 2009 at 10:29:11PM -0400, Robert Haas wrote:
> On Thu, Sep 17, 2009 at 10:00 PM, Dan Colish <dan(at)unencrypted(dot)org> wrote:
> > I am free to start on something new. Any suggestions?
>
> If you're game to dive into ECPG, there are a bunch of patches from
> Zoltan that could use a review.
>
> If not, either of the two patches under miscellaneous whose names
> begin with "syslog" could use some further review. I don't think
> that either of them are likely to be committed in their present form,
> so the goal is not so much to ascertain that as to perhaps suggest
> some other approaches that might be workable.
>
> ...Robert

I'll take the dynamic cursor support for ECPG

--
--Dan

Gabrielle and PDXPUG,

Thanks for your reviewing. A little followup:

On Mon, Sep 14, 2009 at 9:08 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> gabrielle <gorthx(at)gmail(dot)com> on behalf of PDXPUG CODE SPRINT
> - Errcontext support in PL/Perl

This was committed.

> - MOVE FORWARD n | BACKWARD n

This is listed as to be reviewed by Selena Deckelmann and John Naylor,
but no review posted yet.

> - psql: Unicode UTF-8 table formatting for text output

This is still listed as having no reviewer - are you folks planning to
post a review? If so, please edit patch and list reviewer names.

> - Pgbench Shell command

This is waiting on the author to clarify how to use this. The email
requesting this clarification should probably be added as a comment,
so that it's easy to understand why this is marked as "Waiting on
Author".

> - unicode escapes (time permitting)

Are you guys doing anything with this one? If so, please list
reviewer name on app.

Thanks,

...Robert

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> reviewers to be assigned. If possible, please attempt to complete
> your initial review by end-of-day Saturday (five days from now). When
> [...]
> Dimitri Fontaine <dfontaine(at)hi-media(dot)com>
> - Anonymous code blocks

Well I can't see being able to review this before tomorow evening, I
guess it'd be realistic to target next thursday. Should this slow down
too much the fest or what, please feel free to beat me to it :)

Regards,
--
dim

On Mon, 2009-09-14 at 21:08 -0400, Robert Haas wrote:
> Jeff Davis <pgsql(at)j-davis(dot)com>
> - Join optimization for inheritance tables

Tom Lane already reviewed this patch, and it looks like a new approach
is required, so further review wouldn't be too helpful:

http://archives.postgresql.org/message-id/4690.1253300592@sss.pgh.pa.us

Can you direct me to review another patch?

Regards,
Jeff Davis

2009/9/15 Robert Haas <robertmhaas(at)gmail(dot)com>:
> Brendan Jurd <direvus(at)gmail(dot)com>
> - Generalized Index Constraints
>

It seems that this patch is in the midst of re-design discussion, so I
think I'm free to review another patch.

Happy to take "CREATE LIKE INCLUDING COMMENTS and STORAGE" but feel
free to assign me something else.

Cheers,
BJ

On Sun, Sep 20, 2009 at 8:32 PM, Jeff Davis <pgsql(at)j-davis(dot)com> wrote:
> On Mon, 2009-09-14 at 21:08 -0400, Robert Haas wrote:
>> Jeff Davis <pgsql(at)j-davis(dot)com>
>> - Join optimization for inheritance tables
>
> Tom Lane already reviewed this patch, and it looks like a new approach
> is required, so further review wouldn't be too helpful:
>
> http://archives.postgresql.org/message-id/4690.1253300592@sss.pgh.pa.us
>
> Can you direct me to review another patch?

Yep, how about Named and mixed notation support?

...Robert

On Sun, Sep 20, 2009 at 8:40 PM, Brendan Jurd <direvus(at)gmail(dot)com> wrote:
> 2009/9/15 Robert Haas <robertmhaas(at)gmail(dot)com>:
>> Brendan Jurd <direvus(at)gmail(dot)com>
>> - Generalized Index Constraints
>>
>
> It seems that this patch is in the midst of re-design discussion, so I
> think I'm free to review another patch.
>
> Happy to take "CREATE LIKE INCLUDING COMMENTS and STORAGE" but feel
> free to assign me something else.

Sounds good.

...Robert

On Mon, Sep 14, 2009 at 09:08:56PM -0400, Robert Haas wrote:
> Joshua Tolley <eggyknap(at)gmail(dot)com>
> - DML node in support of writeable CTEs

I hate to say it, but I'm quickly getting the idea that I'll not find the time
I need to review this. I'd better ask to pull out before it gets too much
later. Sorry, all...

--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com

On Mon, Sep 21, 2009 at 10:23 AM, Joshua Tolley <eggyknap(at)gmail(dot)com> wrote:
> On Mon, Sep 14, 2009 at 09:08:56PM -0400, Robert Haas wrote:
>> Joshua Tolley <eggyknap(at)gmail(dot)com>
>> - DML node in support of writeable CTEs
>
> I hate to say it, but I'm quickly getting the idea that I'll not find the time
> I need to review this. I'd better ask to pull out before it gets too much
> later. Sorry, all...

OK. I'll assign that one to myself, I've been curious about it anyhow.

...Robert

Hi Robert,

Is there another patch you'd like me to work on?

Lock wait statistics says it needs review, but the last comment
suggests it is waiting on author.

Enhancements to COPY (error logging and autopartitioning) says it is
waiting on author but last comment suggests perhaps it is ready for
review.

I've taken a look at ECPG, but I couldn't make heads or tails of it.
I guess I could try harder :)

It looks like the ECPG patches are not independent and need to applied
in a particular order in order for them to apply cleanly to HEAD.

So I think I need some guidance on what I should be doing.

Thanks,

Jeff

On Sat, Sep 26, 2009 at 11:39:45AM -0700, Jeff Janes wrote:
> Hi Robert,
>
> Is there another patch you'd like me to work on?
>
> Lock wait statistics says it needs review, but the last comment
> suggests it is waiting on author.
>
> Enhancements to COPY (error logging and autopartitioning) says it is
> waiting on author but last comment suggests perhaps it is ready for
> review.
>
> I've taken a look at ECPG, but I couldn't make heads or tails of it.
> I guess I could try harder :)
>
> It looks like the ECPG patches are not independent and need to applied
> in a particular order in order for them to apply cleanly to HEAD.
>
> So I think I need some guidance on what I should be doing.
>
> Thanks,
>
> Jeff
>

I've been looking at the dynamic cursor names patch, so if you have any
insights I would really appreciate them. I am having some trouble fully
reviewing this patch because I am not very familiar with the ecpg code.

--
--Dan

On Sat, Sep 26, 2009 at 1:39 PM, Jeff Janes <jeff(dot)janes(at)gmail(dot)com> wrote:
> Hi Robert,
>
> Is there another patch you'd like me to work on?
>
> Lock wait statistics says it needs review, but the last comment
> suggests it is waiting on author.
>

some days ago i said i could took this one from saturday (that means
today) if nobody has taken it until now i will start reviewing this

--
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157

On Sat, Sep 26, 2009 at 2:13 PM, Jaime Casanova
<jcasanov(at)systemguards(dot)com(dot)ec> wrote:
> On Sat, Sep 26, 2009 at 1:39 PM, Jeff Janes <jeff(dot)janes(at)gmail(dot)com> wrote:
>> Hi Robert,
>>
>> Is there another patch you'd like me to work on?
>>
>> Lock wait statistics says it needs review, but the last comment
>> suggests it is waiting on author.
>>
>
> some days ago i said i could took this one from saturday (that means
> today) if nobody has taken it until now i will start reviewing this
>

here there are two patches:

Mark Kirkwood's in:
http://archives.postgresql.org/pgsql-hackers/2009-08/msg00664.php
and
Pierre Frédéric Caillau's in:
http://archives.postgresql.org/pgsql-hackers/2009-09/msg01260.php

the later was submmited on september 18th (3 days after commitfest
starts and seems to be only for LWLocks) there is any benefit inf
putting it on the next commitfest?

while mark says there is some work still pending i will review what is
done until now

--
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157

On Sat, Sep 26, 2009 at 2:39 PM, Jeff Janes <jeff(dot)janes(at)gmail(dot)com> wrote:
> Hi Robert,
>
> Is there another patch you'd like me to work on?
>
> Lock wait statistics says it needs review, but the last comment
> suggests it is waiting on author.

It looks like there is one outstanding TODO item, but it doesn't seem
to be a very big one, so I think that would be an excellent choice.

> Enhancements to COPY (error logging and autopartitioning) says it is
> waiting on author but last comment suggests perhaps it is ready for
> review.

Good catch, fixed.

> So I think I need some guidance on what I should be doing.

Go for lock wait stats.

As a side note, what we really need to focus on at this point in the
CommitFest is getting closure. If you have reviewed a patch and a new
version has been posted, please check whether it looks good and if so
mark the patch Ready for Committer. If you have reviewed a patch and
a new version has NOT been posted for 4-5 days, post a note saying
that you are marking the patch Returned with Feedback and do so.

The CommitFest will be half over in 48 hours and we have only closed
19 of 48 patches - that is not good.

...Robert

On Tue, Sep 15, 2009 at 9:55 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Mon, Sep 14, 2009 at 11:34 PM, Euler Taveira de Oliveira
> <euler(at)timbira(dot)com> wrote:
>> Robert Haas escreveu:
>>> Initial reviewing assignments are below.
>>>
>> I'm available to review too. I'll take 'Buffer usage in EXPLAIN and
>> pg_stat_statements', ok?
>
> OK with me.

Euler,

When can we expect to see this review posted?

Thanks,

...Robert

Robert Haas escreveu:
> When can we expect to see this review posted?
>
Sorry for the long delay. I already posted an updated patch that addresses all
of the problems I found. Also, I wrote some docs (that are missing).

I changed the status to "waiting on author" because I want some feedback for
the posted patch.

--
Euler Taveira de Oliveira
http://www.timbira.com/