mixed, named notation support

--On Donnerstag, März 05, 2009 08:41:28 +0100 Pavel Stehule
<pavel.stehule@gmail.com> wrote:

Hello

I did some cleaning on this feature, and I hope so I solve some Tom's
objections

Attached is a cleaned up version of the patch, the one linked from the
commit fest has some hunks failing to be applied to current CVS. This is
mainly for reporting and backup purposes, i'm proceeding looking into the
code deeper now. The patch passes make check and functionality check looks
fine.

Pavel, should I adjust the docs already (you've asked for volunteers...)?
If yes, have you something specific in mind?

--
Thanks

Bernd

2009/7/18 Bernd Helmle <mailings@oopsware.de>:

--On Donnerstag, März 05, 2009 08:41:28 +0100 Pavel Stehule
<pavel.stehule@gmail.com> wrote:

Hello

I did some cleaning on this feature, and I hope so I solve some Tom's
objections

Attached is a cleaned up version of the patch, the one linked from the
commit fest has some hunks failing to be applied to current CVS. This is
mainly for reporting and backup purposes, i'm proceeding looking into the
code deeper now. The patch passes make check and functionality check looks
fine.

Pavel, should I adjust the docs already (you've asked for volunteers...)? If
yes, have you something specific in mind?

sure, please. Doc needs mainly language correction - my English is
bad. And maybe some others - from me is too short and too technical.

Pavel

--On Donnerstag, März 05, 2009 08:41:28 +0100 Pavel Stehule
<pavel.stehule@gmail.com> wrote:

Hello

I did some cleaning on this feature, and I hope so I solve some Tom's
objections

features:
* PostgreSQL's specific syntax for named parameter: value AS name,
* Doesn't change rules for defaults,
* Get defaults for named, mixed notation in planner time.

Pavel, consider the following function:

CREATE OR REPLACE FUNCTION ftest(a int, b text)
RETURNS RECORD
LANGUAGE SQL
AS
$$
SELECT $1, $2 ;
$$;

#= SELECT ftest('blubb' AS b, 128 AS a);
ERROR: function ftest(unknown, integer) does not exist at character 8

#= SELECT ftest(128 AS a, 'abcd' AS b);
ftest
------------
(128,abcd)
(1 row)

Isn't the first one supposed to work?

--
Thanks

Bernd

2009/7/23 Bernd Helmle <mailings@oopsware.de>:

--On Donnerstag, März 05, 2009 08:41:28 +0100 Pavel Stehule
<pavel.stehule@gmail.com> wrote:

Hello

I did some cleaning on this feature, and I hope so I solve some Tom's
objections

features:
 * PostgreSQL's specific syntax for named parameter: value AS name,
 * Doesn't change rules for defaults,
 * Get defaults for named, mixed notation in planner time.

Pavel, consider the following function:

CREATE OR REPLACE FUNCTION ftest(a int, b text)
RETURNS RECORD
LANGUAGE SQL
AS
$$
      SELECT $1, $2 ;
$$;

#= SELECT ftest('blubb' AS b, 128 AS a);
ERROR:  function ftest(unknown, integer) does not exist at character 8

#= SELECT ftest(128 AS a, 'abcd' AS b);
 ftest
------------
(128,abcd)
(1 row)

Isn't the first one supposed to work?

it is probably bug. I'll look on it tomorrow.

Pavel

Hello,

fixed patch attached + more regress tests.

Regards
Pavel Stehule

2009/7/23 Pavel Stehule <pavel.stehule@gmail.com>:

Hi,

I sending a little bit modified version - I removed my forgotten
comment in gram.y

Regards
Pavel

2009/7/25 Pavel Stehule <pavel.stehule@gmail.com>:

--On Sonntag, Juli 26, 2009 06:17:49 +0200 Pavel Stehule
<pavel.stehule@gmail.com> wrote:

Hi,

I sending a little bit modified version - I removed my forgotten
comment in gram.y

Thanks, i'll look on it asap.

--
Thanks

Bernd

--On Montag, Juli 27, 2009 15:24:12 +0200 Bernd Helmle
<mailings@oopsware.de> wrote:

Hi,

I sending a little bit modified version - I removed my forgotten
comment in gram.y

Thanks, i'll look on it asap.

Looks good now.

Here is a slightly edited reviewed patch version. I've edited the docs and
fixed some spelling errors in a few error messages. It seems the patch
mangled some source comments in namespace.c, i've rearranged them and tried
to explain the purpose of VerifyCandidateNamedNotation(). I'm continuing
reviewing this but can't get on it again until sunday.

Pavel, can you have a look at the docs part of this patch and watch out for
any gotchas?

--
Thanks

Bernd

2009/7/31 Bernd Helmle <mailings@oopsware.de>:

--On Montag, Juli 27, 2009 15:24:12 +0200 Bernd Helmle
<mailings@oopsware.de> wrote:

Hi,

I sending a little bit modified version - I removed my forgotten
comment in gram.y

Thanks, i'll look on it asap.

Looks good now.

Here is a slightly edited reviewed patch version. I've edited the docs and
fixed some spelling errors in a few error messages. It seems the patch
mangled some source comments in namespace.c, i've rearranged them and tried
to explain the purpose of VerifyCandidateNamedNotation(). I'm continuing
reviewing this but can't get on it again until sunday.

Pavel, can you have a look at the docs part of this patch and watch out for
any gotchas?

Today, I'll look on it.

Pavel

2009/7/31 Bernd Helmle <mailings@oopsware.de>:

--On Montag, Juli 27, 2009 15:24:12 +0200 Bernd Helmle
<mailings@oopsware.de> wrote:

Hi,

I sending a little bit modified version - I removed my forgotten
comment in gram.y

Thanks, i'll look on it asap.

Looks good now.

Here is a slightly edited reviewed patch version. I've edited the docs and
fixed some spelling errors in a few error messages. It seems the patch
mangled some source comments in namespace.c, i've rearranged them and tried
to explain the purpose of VerifyCandidateNamedNotation(). I'm continuing
reviewing this but can't get on it again until sunday.

Pavel, can you have a look at the docs part of this patch and watch out for
any gotchas?

I looked there and I thing it's very good. I have only one idea about
samples in docs. Mathematical function isn't too much readable. Maybe
some string concation or returning record should be more readable.

create or replace function foo(a varchar, b varchar, uppercase boolean = false)
returns varchar as $$
select when uppercase then 'a=' || a ', b=' || b
else upper('a=' || a ', b=' || b) end;
$$ language sql immutable strict;

select foo('Hello','World');
select foo('Hello', World', true);
select foo('Hello' as a, 'World' as b);
select foo('Hello', 'World', true as uppercase);
select foo(true as uppercase, 'World' as b, 'Hello' as b);

or some similar

Thank You very much
Pavel

On Fri, Jul 31, 2009 at 8:46 AM, Pavel Stehule<pavel.stehule@gmail.com> wrote:

2009/7/31 Bernd Helmle <mailings@oopsware.de>:

--On Montag, Juli 27, 2009 15:24:12 +0200 Bernd Helmle
<mailings@oopsware.de> wrote:

Hi,

I sending a little bit modified version - I removed my forgotten
comment in gram.y

Thanks, i'll look on it asap.

Looks good now.

Here is a slightly edited reviewed patch version. I've edited the docs and
fixed some spelling errors in a few error messages. It seems the patch
mangled some source comments in namespace.c, i've rearranged them and tried
to explain the purpose of VerifyCandidateNamedNotation(). I'm continuing
reviewing this but can't get on it again until sunday.

Pavel, can you have a look at the docs part of this patch and watch out for
any gotchas?

I looked there and I thing it's very good. I have only one idea about
samples in docs. Mathematical function isn't too much readable. Maybe
some string concation or returning record should be more readable.

create or replace function foo(a varchar, b varchar, uppercase boolean = false)
returns varchar as $$
 select when uppercase then 'a=' || a ', b=' || b
                                      else upper('a=' || a ', b=' || b) end;
$$ language sql immutable strict;

select foo('Hello','World');
select foo('Hello', World', true);
select foo('Hello' as a, 'World' as b);
select foo('Hello', 'World', true as uppercase);
select foo(true as uppercase, 'World' as b, 'Hello' as b);

or some similar

Thank You very much
Pavel

What's the current status of this patch? Is someone (either Pavel or
Bernd) planning to update it further, or should it be marked Ready for
Committer?

Thanks,

...Robert

2009/8/2 Robert Haas <robertmhaas@gmail.com>:

On Fri, Jul 31, 2009 at 8:46 AM, Pavel Stehule<pavel.stehule@gmail.com> wrote:

2009/7/31 Bernd Helmle <mailings@oopsware.de>:

--On Montag, Juli 27, 2009 15:24:12 +0200 Bernd Helmle
<mailings@oopsware.de> wrote:

Hi,

I sending a little bit modified version - I removed my forgotten
comment in gram.y

Thanks, i'll look on it asap.

Looks good now.

Here is a slightly edited reviewed patch version. I've edited the docs and
fixed some spelling errors in a few error messages. It seems the patch
mangled some source comments in namespace.c, i've rearranged them and tried
to explain the purpose of VerifyCandidateNamedNotation(). I'm continuing
reviewing this but can't get on it again until sunday.

Pavel, can you have a look at the docs part of this patch and watch out for
any gotchas?

I looked there and I thing it's very good. I have only one idea about
samples in docs. Mathematical function isn't too much readable. Maybe
some string concation or returning record should be more readable.

create or replace function foo(a varchar, b varchar, uppercase boolean = false)
returns varchar as $$
 select when uppercase then 'a=' || a ', b=' || b
                                      else upper('a=' || a ', b=' || b) end;
$$ language sql immutable strict;

select foo('Hello','World');
select foo('Hello', World', true);
select foo('Hello' as a, 'World' as b);
select foo('Hello', 'World', true as uppercase);
select foo(true as uppercase, 'World' as b, 'Hello' as b);

or some similar

Thank You very much
Pavel

What's the current status of this patch?  Is someone (either Pavel or
Bernd) planning to update it further, or should it be marked Ready for
Committer?

Hello

Bernard wrote so will look on it today - I expect so there will be
only cosmetic changes in doc.

Pavel

--On Sonntag, August 02, 2009 11:38:22 -0400 Robert Haas
<robertmhaas@gmail.com> wrote:

What's the current status of this patch? Is someone (either Pavel or
Bernd) planning to update it further, or should it be marked Ready for
Committer?

I will incorporate some additional docs adjustments per Pavels suggestion
and will then mark it ready for committer review.

Please note that there are actually two patches involved here: one is
Pavels' patch the other is Steve Prentice' enhancement to allow named
notation in plpgsql.

--
Thanks

Bernd

2009/8/3 Bernd Helmle <mailings@oopsware.de>:

--On Sonntag, August 02, 2009 11:38:22 -0400 Robert Haas
<robertmhaas@gmail.com> wrote:

What's the current status of this patch?  Is someone (either Pavel or
Bernd) planning to update it further, or should it be marked Ready for
Committer?

I will incorporate some additional docs adjustments per Pavels suggestion
and will then mark it ready for committer review.

Please note that there are actually two patches involved here: one is
Pavels' patch the other is Steve Prentice' enhancement to allow named
notation in plpgsql.

I should to wait with Steve patch - I would to add main sql parser
into plpgsql - than Steve's patch is unnecessary. But if there will be
some problems, then we can use Steve's patch. It is simple - so there
are not big problems with commit.

On Aug 3, 2009, at 1:41 AM, Pavel Stehule wrote:

I should to wait with Steve patch - I would to add main sql parser
into plpgsql - than Steve's patch is unnecessary. But if there will be
some problems, then we can use Steve's patch. It is simple - so there
are not big problems with commit.

I was hoping we could get the small patch into plpgsql during this
commitfest. This makes plpgsql recognize 'AS' and not replace named
parameter labels with the variable reference. I understand there is an
effort underway to redo the plpgsql parser, but getting these two
patches in together will allow people to start playing with plpgsql +
named parameters at the end the of commitfest when the first alpha is
released. (You can use named parameters + plpgsql without this patch,
but not without some pretty serious limitations.)

Without this patch, this will fail:

create function create_user(alias text, display_name text) returns
void as $$
BEGIN
perform create_alias(alias AS alias);
...
END
$$ language plpgsql;

This is a common pattern for many of the stored procedures we are
porting and I'd imagine it's common elsewhere too. If the plpgsql
parser patch lands, this patch won't be needed, but it's hard to
predict when it will land.

As an aside, this pattern really shows how confusing the AS syntax can
be for named parameters. Which side is the label and which is the value?

Thanks,
-Steve

2009/8/3 Steve Prentice <prentice@cisco.com>:

On Aug 3, 2009, at 1:41 AM, Pavel Stehule wrote:

I should to wait with Steve patch - I would to add main sql parser
into plpgsql - than Steve's patch is unnecessary. But if there will be
some problems, then we can use Steve's patch. It is simple - so there
are not big problems with commit.

I was hoping we could get the small patch into plpgsql during this
commitfest. This makes plpgsql recognize 'AS' and not replace named
parameter labels with the variable reference. I understand there is an
effort underway to redo the plpgsql parser, but getting these two patches in
together will allow people to start playing with plpgsql + named parameters
at the end the of commitfest when the first alpha is released. (You can use
named parameters + plpgsql without this patch, but not without some pretty
serious limitations.)

Without this patch, this will fail:

create function create_user(alias text, display_name text) returns void as
$$
 BEGIN
   perform create_alias(alias AS alias);
   ...
 END
$$ language plpgsql;

This is a common pattern for many of the stored procedures we are porting
and I'd imagine it's common elsewhere too. If the plpgsql parser patch
lands, this patch won't be needed, but it's hard to predict when it will
land.

As an aside, this pattern really shows how confusing the AS syntax can be
for named parameters. Which side is the label and which is the value?

ok - I don't though about it. My goal is integration main parser next
commitfest, but it is true, so somebody would to play with named
params now. Commiting of Steve's patch doesn't break anything.

Pavel

On Mon, Aug 3, 2009 at 10:51 AM, Pavel Stehule<pavel.stehule@gmail.com> wrote:

2009/8/3 Steve Prentice <prentice@cisco.com>:

On Aug 3, 2009, at 1:41 AM, Pavel Stehule wrote:

I should to wait with Steve patch - I would to add main sql parser
into plpgsql - than Steve's patch is unnecessary. But if there will be
some problems, then we can use Steve's patch. It is simple - so there
are not big problems with commit.

I was hoping we could get the small patch into plpgsql during this
commitfest. This makes plpgsql recognize 'AS' and not replace named
parameter labels with the variable reference. I understand there is an
effort underway to redo the plpgsql parser, but getting these two patches in
together will allow people to start playing with plpgsql + named parameters
at the end the of commitfest when the first alpha is released. (You can use
named parameters + plpgsql without this patch, but not without some pretty
serious limitations.)

Without this patch, this will fail:

create function create_user(alias text, display_name text) returns void as
$$
 BEGIN
   perform create_alias(alias AS alias);
   ...
 END
$$ language plpgsql;

This is a common pattern for many of the stored procedures we are porting
and I'd imagine it's common elsewhere too. If the plpgsql parser patch
lands, this patch won't be needed, but it's hard to predict when it will
land.

As an aside, this pattern really shows how confusing the AS syntax can be
for named parameters. Which side is the label and which is the value?

ok - I don't though about it. My goal is integration main parser next
commitfest, but it is true, so somebody would to play with named
params now. Commiting of Steve's patch doesn't break anything.

I'm a little confused here. We are 19 days into a 31 day CommitFest;
you are almost three weeks too late to add a patch to the queue.
Unless you can convince a friendly committer to pick this up out of
sequence, I think the only patch that is under consideration here is
the one that has been being discussed on this thread for the last 17
days. I sent several notes adding for all patches to be added to
commitfest.postgresql.org prior to the start of CommitFest; AFAIK,
this one was never added.

You haven't actually specified which other patch of Steve's you're
talking about anyway, but I'm guessing it's this one here:
http://archives.postgresql.org/pgsql-hackers/2009-05/msg00801.php

I don't think that patch would get applied even if HAD been added to
the CommitFest page in time, see Tom's remarks here:
http://archives.postgresql.org/pgsql-hackers/2009-05/msg00802.php

Let's get back to focusing on the patch that is actually under
consideration here.

...Robert

ok - I don't though about it. My goal is integration main parser next
commitfest, but it is true, so somebody would to play with named
params now. Commiting of Steve's patch doesn't break anything.

I'm a little confused here.  We are 19 days into a 31 day CommitFest;
you are almost three weeks too late to add a patch to the queue.
Unless you can convince a friendly committer to pick this up out of
sequence, I think the only patch that is under consideration here is
the one that has been being discussed on this thread for the last 17
days.  I sent several notes adding for all patches to be added to
commitfest.postgresql.org prior to the start of CommitFest; AFAIK,
this one was never added.

You haven't actually specified which other patch of Steve's you're
talking about anyway, but I'm guessing it's this one here:
http://archives.postgresql.org/pgsql-hackers/2009-05/msg00801.php

I don't think that patch would get applied even if HAD been added to
the CommitFest page in time, see Tom's remarks here:
http://archives.postgresql.org/pgsql-hackers/2009-05/msg00802.php

Let's get back to focusing on the patch that is actually under
consideration here.

There are two patches - named and mixed parameters and protecting "AS"
as keyword. First patch is pl independed, second patch is related to
plpgsql. Both patches was in queue and Bernard did reviewing both I
thing. Second patch fix some unwanted behave in plpgsql and it does on
level that was possible on code base one month old.

I hope, so we will be able to integrate main parser to core. I hope so
this will be in next commitfest. If we doesn't release code after this
commitfest, then I'll prefer wait for integration, but now, when we
will release code, I thing, so for somebody should be nice to use
fixed plpgsql.

I thing so Steve fixed issues mentioned by Tom

http://archives.postgresql.org/pgsql-hackers/2009-05/msg00804.php

Integration of main parser to plpgsql will be very significant change
and it needs separate patch. This solve some others big problems
plpgsql and I would to solve it separately.

I'll be glad to answer all questions.

Pavel

On Aug 3, 2009, at 9:38 AM, Robert Haas wrote:

I sent several notes adding for all patches to be added to
commitfest.postgresql.org prior to the start of CommitFest; AFAIK,
this one was never added.

Hi Robert,

The patch for plpgsql was added as a comment to Pavel's patch. I added
it as a comment because it wouldn't make since to commit it or even
review it separately. This was done on the wiki before the migration.
Perhaps that was not the correct way to add it to the commitfest. If
not, my apologies.

-Steve