Re: V0.1 patch for TODO Item: SQL-language reference parameters by name.

Lists: pgsql-hackerspgsql-patches
From: "Gevik Babakhani" <pgdev(at)xs4all(dot)nl>
To: <pgsql-patches(at)postgresql(dot)org>
Subject: V0.1 patch for TODO Item: SQL-language reference parameters by name.
Date: 2007-11-02 14:58:58
Message-ID: 003001c81d60$e5c5a3a0$0a01a8c0@gevmus
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers pgsql-patches

Hello all,

Hereby an alpha version regarding the:

TODO Item: SQL-language reference parameters by name.

I am sending this patch to check if I am on the right track.

So please take a look at this if possible.

What does this patch do?

As discussed in thread:
http://archives.postgresql.org/pgsql-hackers/2007-10/msg01490.php,

this patch adds an additional parameter (char **argnames) to
pg_parse_and_rewrite and

pg_analyze_and_rewrite and ParseState.

When transformColumnRef is about to report an error for a non existing
column,a final match is

performed to see if the non existing column is a parameter name. (argnames)

If true, then a new node is created by transformParamRef

NOTE:

- This patch is created using MSVC++ !

- Nothing is done yet for polymorphic arguments.

My test where:

create table tbl1(id serial,field1 integer,field2 varchar);

insert into tbl1 (field1,field2) values(11,'zzzz');

insert into tbl1 (field1,field2) values(22,'yyyy');

create or replace function func1(par1 integer,par2 integer,par3 varchar)
returns setof record as

$$

select

par1::text,

par2,

par1+par2,

par2+par1,

par1+field1,

(field1+par2)::varchar,

par3,

field2 || ' ' || par3

from

tbl1;

$$ language sql;

select func1(2,4,'aaaa');

select * from func1(5,16,'bbbb') as (a text ,b int ,c int, e int, f int,g
varchar,h varchar,i text);

results:

"(2,4,6,6,13,15,aaaa,"zzzz aaaa")"

"(2,4,6,6,24,26,aaaa,"yyyy aaaa")"

And

"5";16;21;21;16;"27";"bbbb";"zzzz bbbb"

"5";16;21;21;27;"38";"bbbb";"yyyy bbbb"

Regards,

Gevik

Attachment Content-Type Size
func-name-args-v0.1.patch application/octet-stream 15.8 KB

From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Gevik Babakhani <pgdev(at)xs4all(dot)nl>
Cc: pgsql-patches(at)postgresql(dot)org
Subject: Re: V0.1 patch for TODO Item: SQL-language reference parameters by name.
Date: 2007-11-02 15:19:19
Message-ID: 472B3FF7.7070809@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers pgsql-patches

Gevik Babakhani wrote:
>
> Hello all,
>
>
>
> Hereby an alpha version regarding the:
>
> TODO Item: SQL-language reference parameters by name.
>
>
>
> I am sending this patch to check if I am on the right track.
>
> So please take a look at this if possible.
>

Step 1: don't use c++ style comments like this:

+ //TODO: Check here

C89 is basically our standard. gcc -std=c89 will check that it complies.

cheers

andrew


From: "Pavel Stehule" <pavel(dot)stehule(at)gmail(dot)com>
To: "Gevik Babakhani" <pgdev(at)xs4all(dot)nl>
Cc: pgsql-patches(at)postgresql(dot)org
Subject: Re: V0.1 patch for TODO Item: SQL-language reference parameters by name.
Date: 2007-11-02 15:20:27
Message-ID: 162867790711020820x221fb2b5g70e12d4806369805@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers pgsql-patches

> Hello all,
>
>
>
> Hereby an alpha version regarding the:
>
> TODO Item: SQL-language reference parameters by name.
>
>

what about name's collision? Maybe is better use some prefix, like $
or :. Without it we only propagate one problem from plpgsql to others
languages.

It can be more wide used:
* named params in prepared statements
* named params in SPI
* ..

Regards
Pavel Stehule


From: "Gevik Babakhani" <pgdev(at)xs4all(dot)nl>
To: "'Andrew Dunstan'" <andrew(at)dunslane(dot)net>
Cc: <pgsql-patches(at)postgresql(dot)org>
Subject: Re: V0.1 patch for TODO Item: SQL-language reference parameters by name.
Date: 2007-11-02 15:47:54
Message-ID: 003c01c81d67$bee00ad0$0a01a8c0@gevmus
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers pgsql-patches

Noted. Thank you.

-----Original Message-----
From: pgsql-patches-owner(at)postgresql(dot)org
[mailto:pgsql-patches-owner(at)postgresql(dot)org] On Behalf Of Andrew Dunstan
Sent: Friday, November 02, 2007 4:19 PM
To: Gevik Babakhani
Cc: pgsql-patches(at)postgresql(dot)org
Subject: Re: [PATCHES] V0.1 patch for TODO Item: SQL-language reference
parameters by name.

Gevik Babakhani wrote:
>
> Hello all,
>
>
>
> Hereby an alpha version regarding the:
>
> TODO Item: SQL-language reference parameters by name.
>
>
>
> I am sending this patch to check if I am on the right track.
>
> So please take a look at this if possible.
>

Step 1: don't use c++ style comments like this:

+ //TODO: Check here

C89 is basically our standard. gcc -std=c89 will check that it complies.

cheers

andrew

---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend


From: "Gevik Babakhani" <pgdev(at)xs4all(dot)nl>
To: "'Pavel Stehule'" <pavel(dot)stehule(at)gmail(dot)com>
Cc: <pgsql-patches(at)postgresql(dot)org>
Subject: Re: V0.1 patch for TODO Item: SQL-language reference parameters by name.
Date: 2007-11-02 15:56:27
Message-ID: 003d01c81d68$edc4c880$0a01a8c0@gevmus
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers pgsql-patches


Hi,

> what about name's collision? Maybe is better use some prefix,
> like $ or :. Without it we only propagate one problem from
> plpgsql to others languages.
>
Please explain.

Perhaps I am wrong, but plpgsql handles arsgument names before it
passes the query to be executed. Please see:

plpgsql/pl_comp.c/do_compile(...)/line: 393

Regards,
Gevik.


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: "Gevik Babakhani" <pgdev(at)xs4all(dot)nl>
Cc: pgsql-patches(at)postgresql(dot)org
Subject: Re: V0.1 patch for TODO Item: SQL-language reference parameters by name.
Date: 2007-11-02 16:06:11
Message-ID: 6790.1194019571@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers pgsql-patches

"Gevik Babakhani" <pgdev(at)xs4all(dot)nl> writes:
> I am sending this patch to check if I am on the right track.
> So please take a look at this if possible.

You seem not to have understood my recommendation to use a callback
function. This patch might work nicely for SQL functions but there
will be no good way to use it for plpgsql, or probably any other
PL function language. If we're going to change the parser API
then I'd like to have a more general solution.

regards, tom lane


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: "Gevik Babakhani" <pgdev(at)xs4all(dot)nl>
Cc: "'Pavel Stehule'" <pavel(dot)stehule(at)gmail(dot)com>, pgsql-patches(at)postgresql(dot)org
Subject: Re: V0.1 patch for TODO Item: SQL-language reference parameters by name.
Date: 2007-11-02 16:17:04
Message-ID: 6960.1194020224@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers pgsql-patches

"Gevik Babakhani" <pgdev(at)xs4all(dot)nl> writes:
>> what about name's collision? Maybe is better use some prefix,
>> like $ or :. Without it we only propagate one problem from
>> plpgsql to others languages.

> Please explain.
> Perhaps I am wrong, but plpgsql handles arsgument names before it
> passes the query to be executed.

Which is actually the Wrong Thing to do: really the parameters should be
seen as being in a name scope that's outside that of the query (and thus
ambiguous names should be resolved first as column names of the query).
The proposed patch does this in the right order and so I think that
Pavel's concern is without foundation.

One point here is that it would be good to be able to qualify the
argument names with the function name, for example

create function myfunc(x int) ...
select ... from t where t.x = myfunc.x

If t has a column named x then this will be the only way that the
function parameter x can be referenced within that query. We are
partway to that point with plpgsql but haven't bitten the bullet
of changing the lookup order.

Note that this consideration is another reason for having a callback
function that's responsible for trying to resolve unresolved names.
I certainly wouldn't like to have a notion of "function name" wired
into the parser API, and if we did do that it still wouldn't be
sufficient for plpgsql which can have multiple block-label namespaces
accessible at once.

regards, tom lane


From: "Pavel Stehule" <pavel(dot)stehule(at)gmail(dot)com>
To: "Gevik Babakhani" <pgdev(at)xs4all(dot)nl>
Cc: pgsql-patches(at)postgresql(dot)org
Subject: Re: V0.1 patch for TODO Item: SQL-language reference parameters by name.
Date: 2007-11-02 17:24:53
Message-ID: 162867790711021024m2479bc25le6096a8bc2e182de@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers pgsql-patches

On 02/11/2007, Gevik Babakhani <pgdev(at)xs4all(dot)nl> wrote:
>
> Hi,
>
> > what about name's collision? Maybe is better use some prefix,
> > like $ or :. Without it we only propagate one problem from
> > plpgsql to others languages.
> >
> Please explain.
>
> Perhaps I am wrong, but plpgsql handles arsgument names before it
> passes the query to be executed. Please see:
>
> plpgsql/pl_comp.c/do_compile(...)/line: 393
>
> Regards,
> Gevik.
>

it's one from mystic bugs:

create table t(a integer, b integer);
insert into t values(10,20);

create function foo(a integer)
returns integer
as $$
select a from t
where a <> b and a = 10;
$$ languge sql;

select foo(20);
output? expected 10, but you will get NULL!

Regards
Pavel Stehule

so some prefixes can help

create function foo(a integer)
returns integer
as $$
select a from t
where :a <> b and a = 10;
$$ languge sql;

Oracle use symbol ':'

I don't know what others databases has.

Regards
Pavel Stehule


From: "Gevik Babakhani" <pgdev(at)xs4all(dot)nl>
To: "'Tom Lane'" <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: <pgsql-hackers(at)postgresql(dot)org>
Subject: Continue [PATCHES] V0.1 patch for TODO Item: SQL-language reference parameters by name.
Date: 2007-11-02 20:43:04
Message-ID: 000001c81d90$fb5ea880$0a01a8c0@gevmus
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers pgsql-patches

Hello,

> You seem not to have understood my recommendation to use a
> callback function. This patch might work nicely for SQL
> functions but there will be no good way to use it for
> plpgsql, or probably any other PL function language. If
> we're going to change the parser API then I'd like to have a
> more general solution.
>

Perhaps I did not look well enough, but is there any callback mechanism like
the
error_context_stack etc... in the parser?

( If not, I guess one has to be created :) )

Thank you.
Gevik.