Horton
Menu

Thread

  1. plpgsql: open for execute - add USING clause

    Pavel Stehule <pavel.stehule@gmail.com> — 2009-11-17T20:04:03Z

    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

  2. Re: plpgsql: open for execute - add USING clause

    Robert Haas <robertmhaas@gmail.com> — 2009-11-17T20:20:52Z

    On Tue, Nov 17, 2009 at 3:04 PM, Pavel Stehule <pavel.stehule@gmail.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

  3. Re: plpgsql: open for execute - add USING clause

    Pavel Stehule <pavel.stehule@gmail.com> — 2009-11-17T20:25:50Z

    2009/11/17 Robert Haas <robertmhaas@gmail.com>:
> On Tue, Nov 17, 2009 at 3:04 PM, Pavel Stehule <pavel.stehule@gmail.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
>

  4. Re: plpgsql: open for execute - add USING clause

    Pavel Stehule <pavel.stehule@gmail.com> — 2009-11-17T20:27:38Z

    2009/11/17 Pavel Stehule <pavel.stehule@gmail.com>:
> 2009/11/17 Robert Haas <robertmhaas@gmail.com>:
>> On Tue, Nov 17, 2009 at 3:04 PM, Pavel Stehule <pavel.stehule@gmail.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
>>
>

  5. Re: plpgsql: open for execute - add USING clause

    Kevin Grittner <kevin.grittner@wicourts.gov> — 2009-11-17T20:33:08Z

    Pavel Stehule <pavel.stehule@gmail.com> wrote:
> 2009/11/17 Robert Haas <robertmhaas@gmail.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

  6. Re: plpgsql: open for execute - add USING clause

    Pavel Stehule <pavel.stehule@gmail.com> — 2009-11-17T20:52:17Z

    2009/11/17 Kevin Grittner <Kevin.Grittner@wicourts.gov>:
> Pavel Stehule <pavel.stehule@gmail.com> wrote:
>> 2009/11/17 Robert Haas <robertmhaas@gmail.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
>

  7. Re: plpgsql: open for execute - add USING clause

    Joshua D. Drake <jd@commandprompt.com> — 2009-11-17T20:58:26Z

    On Tue, 2009-11-17 at 14:33 -0600, Kevin Grittner wrote:
> Pavel Stehule <pavel.stehule@gmail.com> wrote:
> > 2009/11/17 Robert Haas <robertmhaas@gmail.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

  8. Re: plpgsql: open for execute - add USING clause

    Pavel Stehule <pavel.stehule@gmail.com> — 2009-11-17T21:07:31Z

    2009/11/17 Joshua D. Drake <jd@commandprompt.com>:
> On Tue, 2009-11-17 at 14:33 -0600, Kevin Grittner wrote:
>> Pavel Stehule <pavel.stehule@gmail.com> wrote:
>> > 2009/11/17 Robert Haas <robertmhaas@gmail.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
>
>

  9. Re: plpgsql: open for execute - add USING clause

    Kevin Grittner <kevin.grittner@wicourts.gov> — 2009-11-17T21:40:07Z

    Pavel Stehule <pavel.stehule@gmail.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

  10. Re: plpgsql: open for execute - add USING clause

    Joshua D. Drake <jd@commandprompt.com> — 2009-11-17T22:12:45Z

    On Tue, 2009-11-17 at 15:40 -0600, Kevin Grittner wrote:
> Pavel Stehule <pavel.stehule@gmail.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

  11. Re: plpgsql: open for execute - add USING clause

    Greg Smith <greg@2ndquadrant.com> — 2009-11-17T22:34:38Z

    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@2ndQuadrant.com  www.2ndQuadrant.com

  12. Re: plpgsql: open for execute - add USING clause

    Robert Haas <robertmhaas@gmail.com> — 2009-11-17T22:59:42Z

    On Tue, Nov 17, 2009 at 5:34 PM, Greg Smith <greg@2ndquadrant.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

  13. Re: CommitFest expectations

    Greg Smith <greg@2ndquadrant.com> — 2009-11-17T23:41:40Z

    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@2ndQuadrant.com  www.2ndQuadrant.com

  14. Re: plpgsql: open for execute - add USING clause

    Pavel Stehule <pavel.stehule@gmail.com> — 2009-11-18T05:56:00Z

    2009/11/17 Joshua D. Drake <jd@commandprompt.com>:
> On Tue, 2009-11-17 at 15:40 -0600, Kevin Grittner wrote:
>> Pavel Stehule <pavel.stehule@gmail.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
>
>

  15. Re: plpgsql: open for execute - add USING clause

    Pavel Stehule <pavel.stehule@gmail.com> — 2009-11-18T06:05:08Z

    2009/11/17 Robert Haas <robertmhaas@gmail.com>:
> On Tue, Nov 17, 2009 at 5:34 PM, Greg Smith <greg@2ndquadrant.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
>

  16. Re: plpgsql: open for execute - add USING clause

    Takahiro Itagaki <itagaki.takahiro@oss.ntt.co.jp> — 2010-01-12T09:38:00Z

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

Pavel Stehule <pavel.stehule@gmail.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

  17. Re: plpgsql: open for execute - add USING clause

    Pavel Stehule <pavel.stehule@gmail.com> — 2010-01-13T08:05:35Z

    2010/1/12 Takahiro Itagaki <itagaki.takahiro@oss.ntt.co.jp>:
> Hi, I'm reviewing OPEN FOR EXECUTE USING patch and have a couple of
> trivial comments.
>
> Pavel Stehule <pavel.stehule@gmail.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
>
>
>

  18. Re: plpgsql: open for execute - add USING clause

    Takahiro Itagaki <itagaki.takahiro@oss.ntt.co.jp> — 2010-01-14T01:34:21Z

    Pavel Stehule <pavel.stehule@gmail.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

  19. Re: plpgsql: open for execute - add USING clause

    Pavel Stehule <pavel.stehule@gmail.com> — 2010-01-14T07:19:49Z

    2010/1/14 Takahiro Itagaki <itagaki.takahiro@oss.ntt.co.jp>:
>
> Pavel Stehule <pavel.stehule@gmail.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
>
>
>

  20. Re: plpgsql: open for execute - add USING clause

    Tom Lane <tgl@sss.pgh.pa.us> — 2010-01-19T01:37:17Z

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

Applied with minor editorialization.

			regards, tom lane

  21. Re: plpgsql: open for execute - add USING clause

    Pavel Stehule <pavel.stehule@gmail.com> — 2010-01-19T07:02:53Z

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

thank you

Pavel
>                        regards, tom lane
>