Re: plpgsql: open for execute - add USING clause

Hello,

this small patch add missing USING clause to OPEN FOR EXECUTE statement
+ cleaning part of exec_stmt_open function

see http://archives.postgresql.org/pgsql-hackers/2009-11/msg00713.php

Regards
Pavel Stehule

On Tue, Nov 17, 2009 at 3:04 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
> Hello,
>
> this small patch add missing USING clause to OPEN FOR EXECUTE statement
> + cleaning part of exec_stmt_open function
>
>
> see http://archives.postgresql.org/pgsql-hackers/2009-11/msg00713.php

This is now the fourth patch you've submitted since the start of the
CommitFest...

...Robert

2009/11/17 Robert Haas <robertmhaas(at)gmail(dot)com>:
> On Tue, Nov 17, 2009 at 3:04 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>> Hello,
>>
>> this small patch add missing USING clause to OPEN FOR EXECUTE statement
>> + cleaning part of exec_stmt_open function
>>
>>
>> see http://archives.postgresql.org/pgsql-hackers/2009-11/msg00713.php
>
> This is now the fourth patch you've submitted since the start of the
> CommitFest...
>

These patches are for next commitfest. What I know, the current
commitfest is closed for new patches. Is it ok?

Pavel

> ...Robert
>

2009/11/17 Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>:
> 2009/11/17 Robert Haas <robertmhaas(at)gmail(dot)com>:
>> On Tue, Nov 17, 2009 at 3:04 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>>> Hello,
>>>
>>> this small patch add missing USING clause to OPEN FOR EXECUTE statement
>>> + cleaning part of exec_stmt_open function
>>>
>>>
>>> see http://archives.postgresql.org/pgsql-hackers/2009-11/msg00713.php
>>
>> This is now the fourth patch you've submitted since the start of the
>> CommitFest...
>>
>
> These patches are for next commitfest. What I know, the current
> commitfest is closed for new patches. Is it ok?
>

typmode support is for this commitfest. Others for next commitfest.

Pavel

> Pavel
>
>> ...Robert
>>
>

Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
> 2009/11/17 Robert Haas <robertmhaas(at)gmail(dot)com>:

>> This is now the fourth patch you've submitted since the start of
>> the CommitFest...
>>
>
> These patches are for next commitfest. What I know, the current
> commitfest is closed for new patches. Is it ok?

Until this moment I was unconvinced of the need for a strict rule that
patches from regular submitters who don't suspend patch development
to contribute to the commitfest reviews should be ignored.

-Kevin

2009/11/17 Kevin Grittner <Kevin(dot)Grittner(at)wicourts(dot)gov>:
> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>> 2009/11/17 Robert Haas <robertmhaas(at)gmail(dot)com>:
>
>>> This is now the fourth patch you've submitted since the start of
>>> the CommitFest...
>>>
>>
>> These patches are for next commitfest. What I know, the current
>> commitfest is closed for new patches. Is it ok?
>
> Until this moment I was unconvinced of the need for a strict rule that
> patches from regular submitters who don't suspend patch development
> to contribute to the commitfest reviews should be ignored.

what is wrong?

Patches typmodes for functions and enhancing psql was notificated in proposal:

*http://archives.postgresql.org/pgsql-hackers/2009-11/msg00934.php
*http://archives.postgresql.org/pgsql-hackers/2009-10/msg00519.php
(more than one moth old)

patch for OPEN EXECUTE USING is reaction on Tom's mail
http://archives.postgresql.org/pgsql-hackers/2009-11/msg00713.php (for
me it is like a proposal) and this patch is +/- bugfix.

I don't wont to apply these patches tomorrow, I don't sending these
patches for last moment. If I have to wait one weak or two weeks, ok.
Declare it. I'll respect it. But actually I respecting all rules, what
I know.

I don't would to generate thousand patches now. Simply I have a time
for postgres now. I wrote three patches. And I put it to commitfest,
because I thing so this work is serious. So anybody can comment it, so
anybody can test it. I put it to commitfest application, because this
code is finished (or finished for reviewing) and I would lost these
patches in this mailing list. Tomorrow I could be killed (maybe),
tomorrow I could to lost data in my hardisc. I have not other patches.
Don't afraid.

Pavel

>
> -Kevin
>

On Tue, 2009-11-17 at 14:33 -0600, Kevin Grittner wrote:
> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
> > 2009/11/17 Robert Haas <robertmhaas(at)gmail(dot)com>:
>
> >> This is now the fourth patch you've submitted since the start of
> >> the CommitFest...
> >>
> >
> > These patches are for next commitfest. What I know, the current
> > commitfest is closed for new patches. Is it ok?
>
> Until this moment I was unconvinced of the need for a strict rule that
> patches from regular submitters who don't suspend patch development
> to contribute to the commitfest reviews should be ignored.

I agree they should be ignored until the NEXT commitfest. I do not agree
that they should be dropped into a bucket.

Joshua D. Drake

>
> -Kevin
>

--
PostgreSQL.org Major Contributor
Command Prompt, Inc: http://www.commandprompt.com/ - 503.667.4564
Consulting, Training, Support, Custom Development, Engineering
If the world pushes look it in the eye and GRR. Then push back harder. - Salamander

2009/11/17 Joshua D. Drake <jd(at)commandprompt(dot)com>:
> On Tue, 2009-11-17 at 14:33 -0600, Kevin Grittner wrote:
>> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>> > 2009/11/17 Robert Haas <robertmhaas(at)gmail(dot)com>:
>>
>> >> This is now the fourth patch you've submitted since the start of
>> >> the CommitFest...
>> >>
>> >
>> > These patches are for next commitfest. What I know, the current
>> > commitfest is closed for new patches. Is it ok?
>>
>> Until this moment I was unconvinced of the need for a strict rule that
>> patches from regular submitters who don't suspend patch development
>> to contribute to the commitfest reviews should be ignored.
>
> I agree they should be ignored until the NEXT commitfest. I do not agree
> that they should be dropped into a bucket.

I never sent these (last two) patches to THIS commitfest. Is it clean?
I am maybe crazy, but I know when commitfest starting. Have I next
month be quite? First patch I resent, because patch was broken. But
this patch was sent to 2009-11-04. Really, I don't would to push my
patches to this commitfest.

Pavel

>
> Joshua D. Drake
>
>
>>
>> -Kevin
>>
>
>
> --
> PostgreSQL.org Major Contributor
> Command Prompt, Inc: http://www.commandprompt.com/ - 503.667.4564
> Consulting, Training, Support, Custom Development, Engineering
> If the world pushes look it in the eye and GRR. Then push back harder. - Salamander
>
>

Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:

> I never sent these (last two) patches to THIS commitfest. Is it
> clean?

Counting the "In Progress" commitfest and the two preceding ones, you
have submitted nine patches and contributed to the review of none.
Surely you noticed recent threads about how the review and commit
steps are the bottleneck, help is desperately needed for the review
process, and the point of commitfests is to get everyone to take a
break from coding to help review the work of others? Several regular
contributors have expressed frustration that while they are taking
time off from their preferred activity of coding to contribute to the
review process, others are stacking up a pile of patches for the next
review cycle.

Robert in particular has been burning himself out trying to keep the
patch reviews rolling through so that everyone's patches can get
proper consideration. I certainly appreciate that you are making
contributions of patches to help make PostgreSQL better; but since the
review process is the bottleneck, if you don't help review patches,
any time spent by someone reviewing your patches comes out of the time
they would be writing patches themselves.

I'm sure it would be much appreciated, and help to alleviate the
frustration and burnout of some other contributors, if you could take
a turn at reviewing -- at least one patch each commitfest.

-Kevin

On Tue, 2009-11-17 at 15:40 -0600, Kevin Grittner wrote:
> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>
> > I never sent these (last two) patches to THIS commitfest. Is it
> > clean?
>
> I'm sure it would be much appreciated, and help to alleviate the
> frustration and burnout of some other contributors, if you could take
> a turn at reviewing -- at least one patch each commitfest.

In short Pavel,

Nobody is complaining about your patches. It would just be really nice
if you could help review some existing patches in this commit fest.
Would you be willing to do so?

Sincerely,

Joshua D. Drake

--
PostgreSQL.org Major Contributor
Command Prompt, Inc: http://www.commandprompt.com/ - 503.667.4564
Consulting, Training, Support, Custom Development, Engineering
If the world pushes look it in the eye and GRR. Then push back harder. - Salamander

Pavel Stehule wrote:
> I don't wont to apply these patches tomorrow, I don't sending these
> patches for last moment. If I have to wait one weak or two weeks, ok.
> Declare it. I'll respect it. But actually I respecting all rules, what
> I know.
>
If you're sending stuff intended for the next CommitFest in the middle
of an active one (which we'd prefer not to see at all but you have your
own schedule limitations), it would be helpful if you were to label
those patches as such. It's difficult for the rest of us to tell which
of the ones you're generating are in response to patches that are active
during this one, and which are intended for future review but you're
just dropping them off now. Had your new stuff been labeled "This is
for the next CommitFest, I'm just sending it to the list now", it would
have made it easier on everyone else to figure out which of your
messages we need to pay attention to and what should be ignored for now.

--
Greg Smith 2ndQuadrant Baltimore, MD
PostgreSQL Training, Services and Support
greg(at)2ndQuadrant(dot)com www.2ndQuadrant.com

On Tue, Nov 17, 2009 at 5:34 PM, Greg Smith <greg(at)2ndquadrant(dot)com> wrote:
> Pavel Stehule wrote:
>>
>> I don't wont to apply these patches tomorrow, I don't sending these
>> patches for last moment. If I have to wait one weak or two weeks, ok.
>> Declare it. I'll respect it. But actually I respecting all rules, what
>> I know.
>
> If you're sending stuff intended for the next CommitFest in the middle of an
> active one (which we'd prefer not to see at all but you have your own
> schedule limitations), it would be helpful if you were to label those
> patches as such.  It's difficult for the rest of us to tell which of the
> ones you're generating are in response to patches that are active during
> this one, and which are intended for future review but you're just dropping
> them off now.  Had your new stuff been labeled "This is for the next
> CommitFest, I'm just sending it to the list now", it would have made it
> easier on everyone else to figure out which of your messages we need to pay
> attention to and what should be ignored for now.

This expresses my feelings on the topic exactly, and perhaps merits
inclusion in a Wiki page someplace. Maybe we need to have a wiki page
on commitfest rules & expectations.

...Robert

Robert Haas wrote:
> This expresses my feelings on the topic exactly, and perhaps merits
> inclusion in a Wiki page someplace. Maybe we need to have a wiki page
> on commitfest rules & expectations.
>
I put a note at
http://wiki.postgresql.org/wiki/Submitting_a_Patch#Submission_timing
which seems the logical place to warn people about patch submission
guidelines; there was already one there about avoiding submissions
during the beta I moved into the new section.

There's still some debris from the old wiki-based CommitFest approach
floating around the wiki that makes it harder to figure out how things
fit together than it should be. I started cleaning that up with
refreshing http://wiki.postgresql.org/wiki/CommitFest , which is
probably the right place to document general rules and expectations better.

--
Greg Smith 2ndQuadrant Baltimore, MD
PostgreSQL Training, Services and Support
greg(at)2ndQuadrant(dot)com www.2ndQuadrant.com

2009/11/17 Joshua D. Drake <jd(at)commandprompt(dot)com>:
> On Tue, 2009-11-17 at 15:40 -0600, Kevin Grittner wrote:
>> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>>
>> > I never sent these (last two) patches to THIS commitfest. Is it
>> > clean?
>>
>> I'm sure it would be much appreciated, and help to alleviate the
>> frustration and burnout of some other contributors, if you could take
>> a turn at reviewing -- at least one patch each commitfest.
>
> In short Pavel,
>
> Nobody is complaining about your patches. It would just be really nice
> if you could help review some existing patches in this commit fest.
> Would you be willing to do so?

I understand so there are missing people who can do a review. I could
to help with plpgsql or psql code - or some catalog code.

Pavel

>
> Sincerely,
>
> Joshua D. Drake
>
>
> --
> PostgreSQL.org Major Contributor
> Command Prompt, Inc: http://www.commandprompt.com/ - 503.667.4564
> Consulting, Training, Support, Custom Development, Engineering
> If the world pushes look it in the eye and GRR. Then push back harder. - Salamander
>
>

2009/11/17 Robert Haas <robertmhaas(at)gmail(dot)com>:
> On Tue, Nov 17, 2009 at 5:34 PM, Greg Smith <greg(at)2ndquadrant(dot)com> wrote:
>> Pavel Stehule wrote:
>>>
>>> I don't wont to apply these patches tomorrow, I don't sending these
>>> patches for last moment. If I have to wait one weak or two weeks, ok.
>>> Declare it. I'll respect it. But actually I respecting all rules, what
>>> I know.
>>
>> If you're sending stuff intended for the next CommitFest in the middle of an
>> active one (which we'd prefer not to see at all but you have your own
>> schedule limitations), it would be helpful if you were to label those
>> patches as such.  It's difficult for the rest of us to tell which of the
>> ones you're generating are in response to patches that are active during
>> this one, and which are intended for future review but you're just dropping
>> them off now.  Had your new stuff been labeled "This is for the next
>> CommitFest, I'm just sending it to the list now", it would have made it
>> easier on everyone else to figure out which of your messages we need to pay
>> attention to and what should be ignored for now.
>
> This expresses my feelings on the topic exactly, and perhaps merits
> inclusion in a Wiki page someplace.  Maybe we need to have a wiki page
> on commitfest rules & expectations.

Ok, It's my mistake. I didn't would to attack anybody. I though so is
sufficient information is registration in commitfest application.
Patch in mailing list is one thing, but registration in second -
crucial. And when commitfest is closed, then is clean, so new patches
goes to next commitfest. I agree - It should frustrating - and it
means some work more (for reades of mailing list). I have not a
problem with labeling, when patch isn't used for current commitfest.

Pavel

>
> ...Robert
>

Hi, I'm reviewing OPEN FOR EXECUTE USING patch and have a couple of
trivial comments.

Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:

> this small patch add missing USING clause to OPEN FOR EXECUTE statement
> + cleaning part of exec_stmt_open function

- Can we use read_sql_expression2() instead of read_sql_construct()
in gram.y? It could simplify the code a bit.

- I'd prefer to change the new argument for exec_dynquery_with_params()
from "char *portalname" to "const char *curname".

Other than the above minor issues, this patch is ready to commit.

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center

2010/1/12 Takahiro Itagaki <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>:
> Hi, I'm reviewing OPEN FOR EXECUTE USING patch and have a couple of
> trivial comments.
>
> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>
>> this small patch add missing USING clause to OPEN FOR EXECUTE statement
>> + cleaning part of exec_stmt_open function
>
> - Can we use read_sql_expression2() instead of read_sql_construct()
>  in gram.y? It could simplify the code a bit.
>
> - I'd prefer to change the new argument for exec_dynquery_with_params()
>  from "char *portalname" to "const char *curname".
>

ok, I accept all comments.

revised version are attached.

Thank you,

Pavel Stehule

> Other than the above minor issues, this patch is ready to commit.
>
> Regards,
> ---
> Takahiro Itagaki
> NTT Open Source Software Center
>
>
>

Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:

> ok, I accept all comments.
> revised version are attached.

Good. This patch is ready to commit. I'll do it soon if no objections.

BTW, I found inconsistent parameter dumps in the codes. Some of them
add '$', but others does not. Are they intentional? Or, should we
adjust them to use one of the formats?

[pl_funcs.c]
dump_dynexecute()
dump_raise()
printf(" parameter %d: ", i++);
dump_dynfors()
dump_open()
dump_return_query()
printf(" parameter $%d: ", i++);

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center

2010/1/14 Takahiro Itagaki <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>:
>
> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>
>> ok, I accept all comments.
>> revised version are attached.
>
> Good. This patch is ready to commit. I'll do it soon if no objections.
>
> BTW, I found inconsistent parameter dumps in the codes. Some of them
> add '$', but others does not. Are they intentional? Or, should we
> adjust them to use one of the formats?
>
> [pl_funcs.c]
> dump_dynexecute()
> dump_raise()
>                        printf("    parameter %d: ", i++);
> dump_dynfors()
> dump_open()
> dump_return_query()
>                        printf("    parameter $%d: ", i++);
>

isn't parameter of raise statement different than query parameter?

I thing so $x convention respects parameter holder syntax.

Regards
Pavel

>
> Regards,
> ---
> Takahiro Itagaki
> NTT Open Source Software Center
>
>
>

Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
> ok, I accept all comments.
> revised version are attached.

Applied with minor editorialization.

regards, tom lane

2010/1/19 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
>> ok, I accept all comments.
>> revised version are attached.
>
> Applied with minor editorialization.
>

thank you

Pavel
>                        regards, tom lane
>