Thread: CF 2009-09: initial reviewing assignments

CF 2009-09: initial reviewing assignments

From
Robert Haas
Date:
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@j-davis.com>
- Join optimization for inheritance tables

Jeff Janes <jeff.janes@gmail.com>
- opportunistic freezing during vacuum

Selena Deckelmann <selenamarie@gmail.com>
- join removal

Robert Haas <robertmhaas@gmail.com>
- Separate Heap Fetch from Index Scan

Andres Freund <andres@anarazel.de>
- GRANT ON ALL IN schema

Stephen Frost <sfrost@snowman.net>
- Reworks for Access Controls

Jaime Casanova <jcasanov@systemguards.com.ec>
- Largeobject access controls

Jan Urbański <wulczer@wulczer.org>
- Default ACLs

Brendan Jurd <direvus@gmail.com>
- Generalized Index Constraints

Joshua Tolley <eggyknap@gmail.com>
- DML node in support of writeable CTEs

Dan Colish <dan@unencrypted.org>
- generic copy options

David Fetter <david@fetter.org>
- make plpgsql IN args mutable

Dimitri Fontaine <dfontaine@hi-media.com>
- Anonymous code blocks

Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>
- Linux LSB init script

David E. Wheeler <david@kineticode.com>
- hstore enhancements

Josh Williams <joshwilliams@ij.net> WINDOWS
- Encoding issues in console and eventlog on win32

Bernd Helmle <bernd@oopsware.de>
- Allow more complex user/database default GUC settings

Logging configuration changes
- Abhijit Menon-Sen <ams@toroid.org>

gabrielle <gorthx@gmail.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)

Re: CF 2009-09: initial reviewing assignments

From
Robert Haas
Date:
On Mon, Sep 14, 2009 at 9:08 PM, Robert Haas <robertmhaas@gmail.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

Re: CF 2009-09: initial reviewing assignments

From
Euler Taveira de Oliveira
Date:
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/

Re: CF 2009-09: initial reviewing assignments

From
Robert Haas
Date:
On Mon, Sep 14, 2009 at 11:34 PM, Euler Taveira de Oliveira
<euler@timbira.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

Re: CF 2009-09: initial reviewing assignments

From
Abhijit Menon-Sen
Date:
At 2009-09-14 21:08:56 -0400, robertmhaas@gmail.com wrote:
>
> Logging configuration changes
> - Abhijit Menon-Sen <ams@toroid.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

Re: CF 2009-09: initial reviewing assignments

From
Robert Haas
Date:
On Wed, Sep 16, 2009 at 1:38 AM, Abhijit Menon-Sen <ams@toroid.org> wrote:
> At 2009-09-14 21:08:56 -0400, robertmhaas@gmail.com wrote:
>>
>> Logging configuration changes
>> - Abhijit Menon-Sen <ams@toroid.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

Re: CF 2009-09: initial reviewing assignments

From
Robert Haas
Date:
On Wed, Sep 16, 2009 at 12:44 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Wed, Sep 16, 2009 at 1:38 AM, Abhijit Menon-Sen <ams@toroid.org> wrote:
>> At 2009-09-14 21:08:56 -0400, robertmhaas@gmail.com wrote:
>>>
>>> Logging configuration changes
>>> - Abhijit Menon-Sen <ams@toroid.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

Re: CF 2009-09: initial reviewing assignments

From
Nikhil Sontakke
Date:
Hi,

> Nikhil Sontakke <nikhil.sontakke@enterprisedb.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

Re: CF 2009-09: initial reviewing assignments

From
Dan Colish
Date:
I am free to start on something new. Any suggestions?

--
--Dan

Re: CF 2009-09: initial reviewing assignments

From
Robert Haas
Date:
On Thu, Sep 17, 2009 at 10:00 PM, Dan Colish <dan@unencrypted.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

Re: CF 2009-09: initial reviewing assignments

From
Robert Haas
Date:
On Tue, Sep 15, 2009 at 9:55 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Mon, Sep 14, 2009 at 11:34 PM, Euler Taveira de Oliveira
> <euler@timbira.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

Re: CF 2009-09: initial reviewing assignments

From
Dan Colish
Date:
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@unencrypted.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

Re: CF 2009-09: initial reviewing assignments

From
Robert Haas
Date:
Gabrielle and PDXPUG,

Thanks for your reviewing.  A little followup:

On Mon, Sep 14, 2009 at 9:08 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> gabrielle <gorthx@gmail.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

Re: CF 2009-09: initial reviewing assignments

From
Dimitri Fontaine
Date:
Robert Haas <robertmhaas@gmail.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@hi-media.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

Re: CF 2009-09: initial reviewing assignments

From
Jeff Davis
Date:
On Mon, 2009-09-14 at 21:08 -0400, Robert Haas wrote:
> Jeff Davis <pgsql@j-davis.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




Re: CF 2009-09: initial reviewing assignments

From
Brendan Jurd
Date:
2009/9/15 Robert Haas <robertmhaas@gmail.com>:
> Brendan Jurd <direvus@gmail.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

Re: CF 2009-09: initial reviewing assignments

From
Robert Haas
Date:
On Sun, Sep 20, 2009 at 8:32 PM, Jeff Davis <pgsql@j-davis.com> wrote:
> On Mon, 2009-09-14 at 21:08 -0400, Robert Haas wrote:
>> Jeff Davis <pgsql@j-davis.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

Re: CF 2009-09: initial reviewing assignments

From
Robert Haas
Date:
On Sun, Sep 20, 2009 at 8:40 PM, Brendan Jurd <direvus@gmail.com> wrote:
> 2009/9/15 Robert Haas <robertmhaas@gmail.com>:
>> Brendan Jurd <direvus@gmail.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

Re: CF 2009-09: initial reviewing assignments

From
Joshua Tolley
Date:
On Mon, Sep 14, 2009 at 09:08:56PM -0400, Robert Haas wrote:
> Joshua Tolley <eggyknap@gmail.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

Attachment

Re: CF 2009-09: initial reviewing assignments

From
Robert Haas
Date:
On Mon, Sep 21, 2009 at 10:23 AM, Joshua Tolley <eggyknap@gmail.com> wrote:
> On Mon, Sep 14, 2009 at 09:08:56PM -0400, Robert Haas wrote:
>> Joshua Tolley <eggyknap@gmail.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

Re: CF 2009-09: initial reviewing assignments

From
Jeff Janes
Date:
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

Re: CF 2009-09: initial reviewing assignments

From
Dan Colish
Date:
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



Re: CF 2009-09: initial reviewing assignments

From
Jaime Casanova
Date:
On Sat, Sep 26, 2009 at 1:39 PM, Jeff Janes <jeff.janes@gmail.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

Re: CF 2009-09: initial reviewing assignments

From
Jaime Casanova
Date:
On Sat, Sep 26, 2009 at 2:13 PM, Jaime Casanova
<jcasanov@systemguards.com.ec> wrote:
> On Sat, Sep 26, 2009 at 1:39 PM, Jeff Janes <jeff.janes@gmail.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

Re: CF 2009-09: initial reviewing assignments

From
Boszormenyi Zoltan
Date:
>
> 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
>

Asketh and you shall be given. :-)

May I help you in understanding ECPG?

The dynamic cursorname patch was started on 8.3.7 first
and we moved to 8.4 (then 8.5) CVS because in 8.4,
ECPG grammar was rewritten to be auto-generated
from the core grammar. I had to dive in kneep deep
before I could modify it...

Basic thing is that ECPG modifies and extends the core
grammar in a way that
1) every token in ECPG is <str> type. New tokens are
   defined in ecpg.tokens, types are defined in ecpg.type
2) most tokens from the core grammar are simply converted
   to literals concatenated together to form the SQL string
   passed to the server, this is done by parse.pl.
3) some rules need side-effects, actions are either added
   or completely overridden (compared to the basic token
   concatenation) for them, these are defined in ecpg.addons,
   the rules for ecpg.addons are explained below.
4) new grammar rules are needed for ECPG metacommands.
   These are in ecpg.trailer.
5) ecpg.header contains common functions, etc. used by
   actions for grammar rules.

In "ecpg.addons", every modified rule follows this pattern:
       ECPG: dumpedtokens postfix
where "dumpedtokens" is simply tokens from core gram.y's
rules concatenated together. e.g. if gram.y has this:
       ruleA: tokenA tokenB tokenC {...}
then "dumpedtokens" is "ruleAtokenAtokenBtokenC".
"postfix" above can be:
a) "block" - the automatic rule created by parse.pl is completely
    overridden, the code block has to be written completely as
    it were in a plain bison grammar
b) "rule" - the automatic rule is extended on, so new syntaxes
    are accepted for "ruleA". E.g.:
      ECPG: ruleAtokenAtokenBtokenC rule
          | tokenD tokenE { action_code; }
          ...
    It will be substituted with:
      ruleA: <original syntax forms and actions up to and including
                    "tokenA tokenB tokenC">
             | tokenD tokenE { action_code; }
             ...
c) "addon" - the automatic action for the rule (SQL syntax constructed
    from the tokens concatenated together) is prepended with a new
    action code part. This code part is written as is's already inside
    the { ... }

Multiple "addon" or "block" lines may appear together with the
new code block if the code block is common for those rules, which
is a very smart thing.

This was what I gathered from the code. The documentation
seems to be missing from the rewritten ECPG grammar in 8.4.
Michael, am I missing something?

Dan, please, start the review in light of the above.
If you have any questions, don't hesitate to ask.

Best regards,
Zoltán Böszörményi

--
Bible has answers for everything. Proof:
"But let your communication be, Yea, yea; Nay, nay: for whatsoever is more
than these cometh of evil." (Matthew 5:37) - basics of digital technology.
"May your kingdom come" - superficial description of plate tectonics

----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
http://www.postgresql.at/


Re: CF 2009-09: initial reviewing assignments

From
Robert Haas
Date:
On Sat, Sep 26, 2009 at 2:39 PM, Jeff Janes <jeff.janes@gmail.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

Re: CF 2009-09: initial reviewing assignments

From
Robert Haas
Date:
2009/9/27 Boszormenyi Zoltan <zb@cybertec.at>:
>>
>> 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
>>
>
> Asketh and you shall be given. :-)
>
> May I help you in understanding ECPG?
>
> The dynamic cursorname patch was started on 8.3.7 first
> and we moved to 8.4 (then 8.5) CVS because in 8.4,
> ECPG grammar was rewritten to be auto-generated
> from the core grammar. I had to dive in kneep deep
> before I could modify it...
>
> Basic thing is that ECPG modifies and extends the core
> grammar in a way that
> 1) every token in ECPG is <str> type. New tokens are
>   defined in ecpg.tokens, types are defined in ecpg.type
> 2) most tokens from the core grammar are simply converted
>   to literals concatenated together to form the SQL string
>   passed to the server, this is done by parse.pl.
> 3) some rules need side-effects, actions are either added
>   or completely overridden (compared to the basic token
>   concatenation) for them, these are defined in ecpg.addons,
>   the rules for ecpg.addons are explained below.
> 4) new grammar rules are needed for ECPG metacommands.
>   These are in ecpg.trailer.
> 5) ecpg.header contains common functions, etc. used by
>   actions for grammar rules.
>
> In "ecpg.addons", every modified rule follows this pattern:
>       ECPG: dumpedtokens postfix
> where "dumpedtokens" is simply tokens from core gram.y's
> rules concatenated together. e.g. if gram.y has this:
>       ruleA: tokenA tokenB tokenC {...}
> then "dumpedtokens" is "ruleAtokenAtokenBtokenC".
> "postfix" above can be:
> a) "block" - the automatic rule created by parse.pl is completely
>    overridden, the code block has to be written completely as
>    it were in a plain bison grammar
> b) "rule" - the automatic rule is extended on, so new syntaxes
>    are accepted for "ruleA". E.g.:
>      ECPG: ruleAtokenAtokenBtokenC rule
>          | tokenD tokenE { action_code; }
>          ...
>    It will be substituted with:
>      ruleA: <original syntax forms and actions up to and including
>                    "tokenA tokenB tokenC">
>             | tokenD tokenE { action_code; }
>             ...
> c) "addon" - the automatic action for the rule (SQL syntax constructed
>    from the tokens concatenated together) is prepended with a new
>    action code part. This code part is written as is's already inside
>    the { ... }
>
> Multiple "addon" or "block" lines may appear together with the
> new code block if the code block is common for those rules, which
> is a very smart thing.
>
> This was what I gathered from the code. The documentation
> seems to be missing from the rewritten ECPG grammar in 8.4.
> Michael, am I missing something?
>
> Dan, please, start the review in light of the above.
> If you have any questions, don't hesitate to ask.

Please move this discussion to -hackers, maybe with a link back to
this post.  Good info, wrong place.  :-)

...Robert

Re: CF 2009-09: initial reviewing assignments

From
Robert Haas
Date:
On Tue, Sep 15, 2009 at 9:55 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Mon, Sep 14, 2009 at 11:34 PM, Euler Taveira de Oliveira
> <euler@timbira.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

Re: CF 2009-09: initial reviewing assignments

From
Euler Taveira de Oliveira
Date:
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/