Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-04-26 Thread Pavel Stehule
2016-04-25 19:40 GMT+02:00 Bruce Momjian :

>
> Good summary.  Is there a TODO item here?
>

no, it is not

Regars

Pavel


>
> ---
>
> On Tue, Mar 15, 2016 at 08:17:07PM -0400, Tom Lane wrote:
> > Pavel Stehule  writes:
> > >> Robert Haas  writes:
> > >>> That's not a dumb idea.  I think %TYPE is an Oracle-ism, and it
> > >>> doesn't seem to have been their best-ever design decision.
> >
> > > Using %TYPE has sense in PostgreSQL too.
> >
> > It's certainly useful functionality; the question is whether this
> > particular syntax is an appropriate base for extended features.
> >
> > As I see it, what we're talking about here could be called type
> operators:
> > given a type name or some other kind of SQL expression, produce the name
> > of a related type.  The existing things of that sort are %TYPE and []
> > (we don't really implement [] as a type operator, but a user could
> > reasonably think of it as one).  This patch proposes to make %TYPE and []
> > composable into a single operator, and then it proposes to add ELEMENT OF
> > as a different operator; and these things are only implemented in
> plpgsql.
> >
> > My concern is basically that I don't want to stop there.  I think we want
> > more type operators in future, such as the rowtype-related operators
> > I sketched upthread; and I think we will want these operators anywhere
> > that you can write a type name.
> >
> > Now, in the core grammar we have [] which can be attached to any type
> > name, and we have %TYPE but it only works in very limited contexts.
> > There's a fundamental problem with extending %TYPE to be used anywhere
> > a type name can: consider
> >
> >   select 'foo'::x%type from t;
> >
> > It's ambiguous whether this is an invocation of %TYPE syntax or whether %
> > is meant to be a regular operator and TYPE the name of a variable.  Now,
> > we could remove that ambiguity by promoting TYPE to be a fully reserved
> > word (it is unreserved today).  But that's not very palatable, and even
> > if we did reserve TYPE, I think we'd still need a lexer kluge to convert
> > %TYPE into a single token, else bison will have lookahead problems.
> > That sort of kluge is ugly, costs performance, and tends to have
> > unforeseen side-effects.
> >
> > So my opinion is that rather than extending %TYPE, we need a new syntax
> > that is capable of being used in more general contexts.
> >
> > There's another problem with the proposal as given: it adds a prefix
> > type operator (ELEMENT OF) where before we only had postfix ones.
> > That means there's an ambiguity about which one binds tighter.  This is
> > not a big deal right now, since there'd be little point in combining
> > ELEMENT OF and [] in the same operation, but it's going to create a mess
> > when we try to add additional type operators.  You're going to need to
> > allow parentheses to control binding order.  I also find it unsightly
> > that the prefix operator looks so little like the postfix operators
> > syntactically, even though they do very similar sorts of things.
> >
> > In short there basically isn't much to like about these syntax details.
> >
> > I also do not like adding the feature to plpgsql first.  At best, that's
> > going to be code we throw away when we implement the same functionality
> > in the core's typename parser.  At worst, we'll have a permanent
> > incompatibility because we find we can't make the core parser use exactly
> > the same syntax.  (For example, it's possible we'd find out we have to
> > make ELEMENT a fully-reserved word in order to use this ELEMENT OF
> syntax.
> > Or maybe it's fine; but until we've tried to cram it into the Typename
> > production, we won't know.  I'm a bit suspicious of expecting it to be
> > fine, though, since AFAICS this patch breaks the ability to use "element"
> > as a plain type name in a plpgsql variable declaration.  Handwritten
> > parsing code like this tends to be full of such gotchas.)
> >
> > In short, I think we should reject this implementation and instead try
> > to implement the type operators we want in the core grammar's Typename
> > production, from which plpgsql will pick it up automatically.  That is
> > going to require some other syntax than this.  As I said, I'm not
> > particularly pushing the function-like syntax I wrote upthread; but
> > I want to see something that is capable of supporting all those features
> > and can be extended later if we think of other type operators we want.
> >
> >   regards, tom lane
> >
> >
> > --
> > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> > To make changes to your subscription:
> > http://www.postgresql.org/mailpref/pgsql-hackers
>
> --
>   Bruce Momjian  http://momjian.us
>   EnterpriseDB http://enterprisedb.com
>
> + As you 

Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-04-25 Thread Bruce Momjian

Good summary.  Is there a TODO item here?

---

On Tue, Mar 15, 2016 at 08:17:07PM -0400, Tom Lane wrote:
> Pavel Stehule  writes:
> >> Robert Haas  writes:
> >>> That's not a dumb idea.  I think %TYPE is an Oracle-ism, and it
> >>> doesn't seem to have been their best-ever design decision.
> 
> > Using %TYPE has sense in PostgreSQL too.
> 
> It's certainly useful functionality; the question is whether this
> particular syntax is an appropriate base for extended features.
> 
> As I see it, what we're talking about here could be called type operators:
> given a type name or some other kind of SQL expression, produce the name
> of a related type.  The existing things of that sort are %TYPE and []
> (we don't really implement [] as a type operator, but a user could
> reasonably think of it as one).  This patch proposes to make %TYPE and []
> composable into a single operator, and then it proposes to add ELEMENT OF
> as a different operator; and these things are only implemented in plpgsql.
> 
> My concern is basically that I don't want to stop there.  I think we want
> more type operators in future, such as the rowtype-related operators
> I sketched upthread; and I think we will want these operators anywhere
> that you can write a type name.
> 
> Now, in the core grammar we have [] which can be attached to any type
> name, and we have %TYPE but it only works in very limited contexts.
> There's a fundamental problem with extending %TYPE to be used anywhere
> a type name can: consider
> 
>   select 'foo'::x%type from t;
> 
> It's ambiguous whether this is an invocation of %TYPE syntax or whether %
> is meant to be a regular operator and TYPE the name of a variable.  Now,
> we could remove that ambiguity by promoting TYPE to be a fully reserved
> word (it is unreserved today).  But that's not very palatable, and even
> if we did reserve TYPE, I think we'd still need a lexer kluge to convert
> %TYPE into a single token, else bison will have lookahead problems.
> That sort of kluge is ugly, costs performance, and tends to have
> unforeseen side-effects.
> 
> So my opinion is that rather than extending %TYPE, we need a new syntax
> that is capable of being used in more general contexts.
> 
> There's another problem with the proposal as given: it adds a prefix
> type operator (ELEMENT OF) where before we only had postfix ones.
> That means there's an ambiguity about which one binds tighter.  This is
> not a big deal right now, since there'd be little point in combining
> ELEMENT OF and [] in the same operation, but it's going to create a mess
> when we try to add additional type operators.  You're going to need to
> allow parentheses to control binding order.  I also find it unsightly
> that the prefix operator looks so little like the postfix operators
> syntactically, even though they do very similar sorts of things.
> 
> In short there basically isn't much to like about these syntax details.
> 
> I also do not like adding the feature to plpgsql first.  At best, that's
> going to be code we throw away when we implement the same functionality
> in the core's typename parser.  At worst, we'll have a permanent
> incompatibility because we find we can't make the core parser use exactly
> the same syntax.  (For example, it's possible we'd find out we have to
> make ELEMENT a fully-reserved word in order to use this ELEMENT OF syntax.
> Or maybe it's fine; but until we've tried to cram it into the Typename
> production, we won't know.  I'm a bit suspicious of expecting it to be
> fine, though, since AFAICS this patch breaks the ability to use "element"
> as a plain type name in a plpgsql variable declaration.  Handwritten
> parsing code like this tends to be full of such gotchas.)
> 
> In short, I think we should reject this implementation and instead try
> to implement the type operators we want in the core grammar's Typename
> production, from which plpgsql will pick it up automatically.  That is
> going to require some other syntax than this.  As I said, I'm not
> particularly pushing the function-like syntax I wrote upthread; but
> I want to see something that is capable of supporting all those features
> and can be extended later if we think of other type operators we want.
> 
>   regards, tom lane
> 
> 
> -- 
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Ancient Roman grave inscription +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-03-19 Thread Tom Lane
Jim Nasby  writes:
> On 3/3/16 4:51 AM, Pavel Stehule wrote:
>> CREATE TABLE a(a int);
>> CREATE TABLE b(a a.a%TYPE)
>> 
>> And the people expecting the living relation between table a and table
>> b. So when I do ALTER a.a, then b.a should be changed. What if I drop
>> a.a or drop a?
>> 
>> So this is reason, why I don't would this feature in SQL side.

> I don't buy that. plpgsql doesn't work that way, so why would this? 
> *especially* with the %TYPE decorator.

Yeah.  The %TYPE decorator doesn't work like that in the core parser
either: when you use it, the referenced type is determined immediately
and then it's just as if you'd written that type name to begin with.
I do not see a reason for any of these "type operators" to work
differently.

Another analogy that might help make the point is

set search_path = a;
create table myschema.tab(f1 mytype);
set search_path = b;

If there are types "mytype" in both schemas a and b, is myschema.tab.f1
now of type b.mytype?  No.  The meaning of the type reference is
determined when the command executes, and then you're done.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-03-19 Thread Pavel Stehule
2016-03-16 20:53 GMT+01:00 Jim Nasby :

> On 3/3/16 4:51 AM, Pavel Stehule wrote:
>
>> CREATE TABLE a(a int);
>> CREATE TABLE b(a a.a%TYPE)
>>
>> And the people expecting the living relation between table a and table
>> b. So when I do ALTER a.a, then b.a should be changed. What if I drop
>> a.a or drop a?
>>
>> So this is reason, why I don't would this feature in SQL side.
>>
>
> I don't buy that. plpgsql doesn't work that way, so why would this?
> *especially* with the %TYPE decorator.
>
> Now, if the syntax was
>
> CREATE TABLE b(a a.a)
>
> then I would expect b.a to be a foreign key reference to a.


probably we are talking about different situations. It is not important,
because this patch was rejected.

Regards

Pavel


>
> --
> Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
> Experts in Analytics, Data Architecture and PostgreSQL
> Data in Trouble? Get it in Treble! http://BlueTreble.com
>


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-03-19 Thread Joe Conway
On 03/16/2016 09:38 AM, Pavel Stehule wrote:
> 2016-03-16 16:50 GMT+01:00 Pavel Stehule  >:
> 2016-03-16 16:46 GMT+01:00 Joe Conway  >:
> 
> On 03/15/2016 05:17 PM, Tom Lane wrote:
> > In short, I think we should reject this implementation and instead 
> try
> > to implement the type operators we want in the core grammar's 
> Typename
> > production, from which plpgsql will pick it up automatically.  That 
> is
> > going to require some other syntax than this.  As I said, I'm not
> > particularly pushing the function-like syntax I wrote upthread; but
> > I want to see something that is capable of supporting all those 
> features
> > and can be extended later if we think of other type operators we 
> want.
> 
> +1
> 
> Anyone want to argue against changing the status of this to
> Rejected or
> at least Returned with feedback?
> 
> 
> I would to reduce this patch to fix row type issue. There is not any
> disagreement. I'll send reduced patch today.
> 
> Any other functionality is not 9.6 topic.
> 
> I played with the reduced patch, and the benefit without all other
> things is negligible. It should be rejected.

Ok, thanks -- done.

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development



signature.asc
Description: OpenPGP digital signature


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-03-19 Thread Pavel Stehule
2016-03-16 16:46 GMT+01:00 Joe Conway :

> On 03/15/2016 05:17 PM, Tom Lane wrote:
> > In short, I think we should reject this implementation and instead try
> > to implement the type operators we want in the core grammar's Typename
> > production, from which plpgsql will pick it up automatically.  That is
> > going to require some other syntax than this.  As I said, I'm not
> > particularly pushing the function-like syntax I wrote upthread; but
> > I want to see something that is capable of supporting all those features
> > and can be extended later if we think of other type operators we want.
>
> +1
>
> Anyone want to argue against changing the status of this to Rejected or
> at least Returned with feedback?
>

I would to reduce this patch to fix row type issue. There is not any
disagreement. I'll send reduced patch today.

Any other functionality is not 9.6 topic.

Regards

Pavel


> Joe
>
> --
> Crunchy Data - http://crunchydata.com
> PostgreSQL Support for Secure Enterprises
> Consulting, Training, & Open Source Development
>
>


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-03-19 Thread Pavel Stehule
2016-03-17 1:02 GMT+01:00 David G. Johnston :

> On Wed, Mar 16, 2016 at 4:39 PM, Tom Lane  wrote:
>
>> Jim Nasby  writes:
>> > On 3/3/16 4:51 AM, Pavel Stehule wrote:
>> >> CREATE TABLE a(a int);
>> >> CREATE TABLE b(a a.a%TYPE)
>> >>
>> >> And the people expecting the living relation between table a and table
>> >> b. So when I do ALTER a.a, then b.a should be changed. What if I drop
>> >> a.a or drop a?
>> >>
>> >> So this is reason, why I don't would this feature in SQL side.
>>
>> > I don't buy that. plpgsql doesn't work that way, so why would this?
>> > *especially* with the %TYPE decorator.
>>
>> Yeah.  The %TYPE decorator doesn't work like that in the core parser
>> either: when you use it, the referenced type is determined immediately
>> and then it's just as if you'd written that type name to begin with.
>>
>
> I'm missing something here...%TYPE ends up getting parsed repeatedly and
> so appears to be change if the variable upon which it is based changes -
> even if once parsed it remains constant for the lifetime of the function's
> evaluation.​
>
> I guess what is being said is that the "constant" behavior in SQL ends up
> being permanent because a given statement is only ever conceptually parsed
> and executed a single time - unlike a function body.  The nature of any
> solution would still have the same characteristics within a function
> because the inherent re-parsing nature and not because of any direct
> capability of %TYPE itself.
>
> I do not see a reason for any of these "type operators" to work
>> differently.
>>
>> Another analogy that might help make the point is
>>
>> set search_path = a;
>> create table myschema.tab(f1 mytype);
>> set search_path = b;
>>
>> If there are types "mytype" in both schemas a and b, is myschema.tab.f1
>> now of type b.mytype?  No.  The meaning of the type reference is
>> determined when the command executes, and then you're done.
>>
> ​
> And its no different than our treatment of "*"
>
> CREATE VIEW test_view
> SELECT *
> FROM temp_table;
>
> Adding columns to temp_table doesn't impact which columns the view returns.
>

yes, but there is strong limit. You can append column, but you cannot to
alter existing column.

Pavel


>
> David J.​
>
>
>
>


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-03-19 Thread David G. Johnston
On Wed, Mar 16, 2016 at 4:39 PM, Tom Lane  wrote:

> Jim Nasby  writes:
> > On 3/3/16 4:51 AM, Pavel Stehule wrote:
> >> CREATE TABLE a(a int);
> >> CREATE TABLE b(a a.a%TYPE)
> >>
> >> And the people expecting the living relation between table a and table
> >> b. So when I do ALTER a.a, then b.a should be changed. What if I drop
> >> a.a or drop a?
> >>
> >> So this is reason, why I don't would this feature in SQL side.
>
> > I don't buy that. plpgsql doesn't work that way, so why would this?
> > *especially* with the %TYPE decorator.
>
> Yeah.  The %TYPE decorator doesn't work like that in the core parser
> either: when you use it, the referenced type is determined immediately
> and then it's just as if you'd written that type name to begin with.
>

I'm missing something here...%TYPE ends up getting parsed repeatedly and so
appears to be change if the variable upon which it is based changes - even
if once parsed it remains constant for the lifetime of the function's
evaluation.​

I guess what is being said is that the "constant" behavior in SQL ends up
being permanent because a given statement is only ever conceptually parsed
and executed a single time - unlike a function body.  The nature of any
solution would still have the same characteristics within a function
because the inherent re-parsing nature and not because of any direct
capability of %TYPE itself.

I do not see a reason for any of these "type operators" to work
> differently.
>
> Another analogy that might help make the point is
>
> set search_path = a;
> create table myschema.tab(f1 mytype);
> set search_path = b;
>
> If there are types "mytype" in both schemas a and b, is myschema.tab.f1
> now of type b.mytype?  No.  The meaning of the type reference is
> determined when the command executes, and then you're done.
>
​
And its no different than our treatment of "*"

CREATE VIEW test_view
SELECT *
FROM temp_table;

Adding columns to temp_table doesn't impact which columns the view returns.

David J.​


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-03-19 Thread Jim Nasby

On 3/15/16 7:17 PM, Tom Lane wrote:

In short, I think we should reject this implementation and instead try
to implement the type operators we want in the core grammar's Typename
production, from which plpgsql will pick it up automatically.


+1.

Something else that's been discussed is allowing [] referencing to be 
more modular. Offhand I don't see how that would impact this new type 
referencing stuff, but maybe someone else sees an issue.


BTW, it might also be useful to allow {} to work as a reference method.
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-03-19 Thread Jim Nasby

On 3/3/16 4:51 AM, Pavel Stehule wrote:

CREATE TABLE a(a int);
CREATE TABLE b(a a.a%TYPE)

And the people expecting the living relation between table a and table
b. So when I do ALTER a.a, then b.a should be changed. What if I drop
a.a or drop a?

So this is reason, why I don't would this feature in SQL side.


I don't buy that. plpgsql doesn't work that way, so why would this? 
*especially* with the %TYPE decorator.


Now, if the syntax was

CREATE TABLE b(a a.a)

then I would expect b.a to be a foreign key reference to a.
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-03-19 Thread Pavel Stehule
2016-03-17 0:39 GMT+01:00 Tom Lane :

> Jim Nasby  writes:
> > On 3/3/16 4:51 AM, Pavel Stehule wrote:
> >> CREATE TABLE a(a int);
> >> CREATE TABLE b(a a.a%TYPE)
> >>
> >> And the people expecting the living relation between table a and table
> >> b. So when I do ALTER a.a, then b.a should be changed. What if I drop
> >> a.a or drop a?
> >>
> >> So this is reason, why I don't would this feature in SQL side.
>
> > I don't buy that. plpgsql doesn't work that way, so why would this?
> > *especially* with the %TYPE decorator.
>
> Yeah.  The %TYPE decorator doesn't work like that in the core parser
> either: when you use it, the referenced type is determined immediately
> and then it's just as if you'd written that type name to begin with.
> I do not see a reason for any of these "type operators" to work
> differently.
>
> Another analogy that might help make the point is
>
> set search_path = a;
> create table myschema.tab(f1 mytype);
> set search_path = b;
>
> If there are types "mytype" in both schemas a and b, is myschema.tab.f1
> now of type b.mytype?  No.  The meaning of the type reference is
> determined when the command executes, and then you're done.
>

This is valid for PostgreSQL. I am not sure if it is true in Oracle, if
%TYPE means only reference to type, or %TYPE holds reference to original
object - and when you change the original object, then the function is
invalidated.

Using %TYPE with create time only semantic has not big practical benefit.
But when %TYPE enforce all life dependency, then I have guaranteed so
change on original object will be propagated to depend object. With all
advantages and disadvantages.

Postgres uses %TYPE in create time only semantic - but it is not big issue
in PLpgSQL, because the creation time there is often - every first
execution in session.

The usage of %TYPE outer PL/pgSQL is probably only in FK. But nothing
similar is in standard, and I don't see a reason, why we should to
implement it. In this moment I don't see any important use case.

Pavel


>
> regards, tom lane
>


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-03-18 Thread Pavel Stehule
2016-03-16 16:50 GMT+01:00 Pavel Stehule :

>
>
> 2016-03-16 16:46 GMT+01:00 Joe Conway :
>
>> On 03/15/2016 05:17 PM, Tom Lane wrote:
>> > In short, I think we should reject this implementation and instead try
>> > to implement the type operators we want in the core grammar's Typename
>> > production, from which plpgsql will pick it up automatically.  That is
>> > going to require some other syntax than this.  As I said, I'm not
>> > particularly pushing the function-like syntax I wrote upthread; but
>> > I want to see something that is capable of supporting all those features
>> > and can be extended later if we think of other type operators we want.
>>
>> +1
>>
>> Anyone want to argue against changing the status of this to Rejected or
>> at least Returned with feedback?
>>
>
> I would to reduce this patch to fix row type issue. There is not any
> disagreement. I'll send reduced patch today.
>
> Any other functionality is not 9.6 topic.
>

I played with the reduced patch, and the benefit without all other things
is negligible. It should be rejected.

Regards

Pavel


>
> Regards
>
> Pavel
>
>
>> Joe
>>
>> --
>> Crunchy Data - http://crunchydata.com
>> PostgreSQL Support for Secure Enterprises
>> Consulting, Training, & Open Source Development
>>
>>
>


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-03-18 Thread Joe Conway
On 03/15/2016 05:17 PM, Tom Lane wrote:
> In short, I think we should reject this implementation and instead try
> to implement the type operators we want in the core grammar's Typename
> production, from which plpgsql will pick it up automatically.  That is
> going to require some other syntax than this.  As I said, I'm not
> particularly pushing the function-like syntax I wrote upthread; but
> I want to see something that is capable of supporting all those features
> and can be extended later if we think of other type operators we want.

+1

Anyone want to argue against changing the status of this to Rejected or
at least Returned with feedback?

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development



signature.asc
Description: OpenPGP digital signature


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-03-15 Thread Tom Lane
Pavel Stehule  writes:
>> Robert Haas  writes:
>>> That's not a dumb idea.  I think %TYPE is an Oracle-ism, and it
>>> doesn't seem to have been their best-ever design decision.

> Using %TYPE has sense in PostgreSQL too.

It's certainly useful functionality; the question is whether this
particular syntax is an appropriate base for extended features.

As I see it, what we're talking about here could be called type operators:
given a type name or some other kind of SQL expression, produce the name
of a related type.  The existing things of that sort are %TYPE and []
(we don't really implement [] as a type operator, but a user could
reasonably think of it as one).  This patch proposes to make %TYPE and []
composable into a single operator, and then it proposes to add ELEMENT OF
as a different operator; and these things are only implemented in plpgsql.

My concern is basically that I don't want to stop there.  I think we want
more type operators in future, such as the rowtype-related operators
I sketched upthread; and I think we will want these operators anywhere
that you can write a type name.

Now, in the core grammar we have [] which can be attached to any type
name, and we have %TYPE but it only works in very limited contexts.
There's a fundamental problem with extending %TYPE to be used anywhere
a type name can: consider

select 'foo'::x%type from t;

It's ambiguous whether this is an invocation of %TYPE syntax or whether %
is meant to be a regular operator and TYPE the name of a variable.  Now,
we could remove that ambiguity by promoting TYPE to be a fully reserved
word (it is unreserved today).  But that's not very palatable, and even
if we did reserve TYPE, I think we'd still need a lexer kluge to convert
%TYPE into a single token, else bison will have lookahead problems.
That sort of kluge is ugly, costs performance, and tends to have
unforeseen side-effects.

So my opinion is that rather than extending %TYPE, we need a new syntax
that is capable of being used in more general contexts.

There's another problem with the proposal as given: it adds a prefix
type operator (ELEMENT OF) where before we only had postfix ones.
That means there's an ambiguity about which one binds tighter.  This is
not a big deal right now, since there'd be little point in combining
ELEMENT OF and [] in the same operation, but it's going to create a mess
when we try to add additional type operators.  You're going to need to
allow parentheses to control binding order.  I also find it unsightly
that the prefix operator looks so little like the postfix operators
syntactically, even though they do very similar sorts of things.

In short there basically isn't much to like about these syntax details.

I also do not like adding the feature to plpgsql first.  At best, that's
going to be code we throw away when we implement the same functionality
in the core's typename parser.  At worst, we'll have a permanent
incompatibility because we find we can't make the core parser use exactly
the same syntax.  (For example, it's possible we'd find out we have to
make ELEMENT a fully-reserved word in order to use this ELEMENT OF syntax.
Or maybe it's fine; but until we've tried to cram it into the Typename
production, we won't know.  I'm a bit suspicious of expecting it to be
fine, though, since AFAICS this patch breaks the ability to use "element"
as a plain type name in a plpgsql variable declaration.  Handwritten
parsing code like this tends to be full of such gotchas.)

In short, I think we should reject this implementation and instead try
to implement the type operators we want in the core grammar's Typename
production, from which plpgsql will pick it up automatically.  That is
going to require some other syntax than this.  As I said, I'm not
particularly pushing the function-like syntax I wrote upthread; but
I want to see something that is capable of supporting all those features
and can be extended later if we think of other type operators we want.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-03-14 Thread Tom Lane
Pavel Stehule  writes:
> Where you are expecting the implementation? In PLpgSQL only, or generally
> in DDL, or in both levels?

I'd envision this as something the main parser does and plpgsql piggybacks
on.  One of the many half-baked things about %TYPE is that the main parser
has an implementation, and plpgsql has its own not-entirely-compatible
one.  (And one thing I especially don't like about the submitted patch is
that it makes those two diverge even further.)

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-03-14 Thread Pavel Stehule
2016-03-14 20:38 GMT+01:00 Tom Lane :

> Robert Haas  writes:
> > On Mon, Mar 14, 2016 at 12:04 PM, Tom Lane  wrote:
> >> Or in short: maybe it's time to blow up %TYPE and start fresh.
>
> > That's not a dumb idea.  I think %TYPE is an Oracle-ism, and it
> > doesn't seem to have been their best-ever design decision.
>
>
Using %TYPE has sense in PostgreSQL too. It is protection against a
domain's explosion - I don't need to declare the domains like varchar_10,
varchar_100, etc. It does more work in Oracle due living relation between
table columns and PL/SQL variables - but this differences are same for
domain types in PL/pgSQL. What is redundant in Postgres, is using %TYPE and
%ROWTYPE. Because native PL languages has strong relation to system catalog
I see this feature like well designed and helpful. Some different can be
our implementation.


>
> It is, and it wasn't.  What concerns me about the present patch is
> that it's trying to shoehorn more functionality into something that
> was badly designed to start with.  I think we'd be better off leaving
> %TYPE as a deprecated backwards-compatibility feature and inventing
> something new and more extensible.
>
> I'm not wedded to any part of the syntax I just wrote, but I do say
> that we need something that allows composability of type selectors.
>

Last version of this patch doesn't modify %TYPE part - and the implemented
syntax is near to your proposed (not all cases), and near to ADA syntax.
But, probably, you are unhappy with it.

Can you describe your expectations from this feature little bit more,
please?

We can leave the original discussion about %TYPE. It was my mistake. I push
two different ingredients to one soup.

There are two independent features - referenced types - the original %TYPE
and %ROWTYPE features, the possibility to take type from system catalog
information.

Last patch implements linear ELEMENT OF ... , ARRAY OF ... . Can be
solution recursive enhancing of last patch? We can implement other type
modificator like RANGE OF (any other?)

Then we can write some like

ARRAY OF RANGE OF int;  or ELEMENT OF ELEMENT OF array_of_ranges

or if we use functional syntax

ARRAY_OF(RANGE_OF(int))

I prefer non functional syntax - it looks more natural in DECLARE part, but
it is detail in this moment. I can live with functional syntax too. The
functional syntax is easy parserable, but the SQL is not functional - so I
see it foreign there.

Where you are expecting the implementation? In PLpgSQL only, or generally
in DDL, or in both levels?

Regards

Pavel



>
> regards, tom lane
>


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-03-14 Thread Tom Lane
Robert Haas  writes:
> On Mon, Mar 14, 2016 at 12:04 PM, Tom Lane  wrote:
>> Or in short: maybe it's time to blow up %TYPE and start fresh.

> That's not a dumb idea.  I think %TYPE is an Oracle-ism, and it
> doesn't seem to have been their best-ever design decision.

It is, and it wasn't.  What concerns me about the present patch is
that it's trying to shoehorn more functionality into something that
was badly designed to start with.  I think we'd be better off leaving
%TYPE as a deprecated backwards-compatibility feature and inventing
something new and more extensible.

I'm not wedded to any part of the syntax I just wrote, but I do say
that we need something that allows composability of type selectors.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-03-14 Thread Robert Haas
On Mon, Mar 14, 2016 at 12:04 PM, Tom Lane  wrote:
> I wrote:
>> However ... one thing I was intending to mention on this thread is that
>> "get the array type over this type" isn't the only extension one might
>> wish for.  Another likely desire is "get the type of field 'foo' of this
>> composite type".  I don't suggest that this patch needs to implement
>> that right now; but it would be a good thing if we could see how the
>> chosen syntax could be extended in such a direction.  Otherwise we might
>> be painting ourselves into a corner.
>
> To enlarge a little bit: it seems to me that what we're really wishing for
> here is a type name syntax that goes beyond simple names.  If we were
> starting in a green field, we might choose a recursively-composable syntax
> like the following.
>
> type_name can be:
>
> * A simple type name, such as int8 or varchar[42].
>
> * TYPE_OF(expression), meaning that the SQL expression is parsed but never
> executed, we just take this construct as naming its result type.
>
> * ARRAY_OF(type_name), meaning the array type having type_name as its
> element type.
>
> * ELEMENT_OF(type_name), meaning the element type of the array type
> named by type_name.
>
> * ROW_OF(type_name [, type_name ...]), meaning the composite type with
> those types as columns.
>
> * FIELD_OF(type_name, foo), meaning the type of column "foo" of the
> composite type named by type_name.  I'm not sure if there would be
> use-cases for selecting a column other than by a simple literal name,
> but there could be variants of this function if so.
>
> It's possible to think of other cases, for example what about range
> types?  You could allow ELEMENT_OF() to apply to range types, certainly.
> I'm not sure about the other direction, because multiple range types
> could have the same element type; but it's possible to hope that some
> type-naming function along the lines of RANGE_OF(type_name, other args)
> could disambiguate.  The main reason I'm thinking of a function-like
> syntax here is that it can easily handle additional arguments when
> needed.
>
> Comparing this flight of fancy to where we are today, we have
> %TYPE as a remarkably ugly and limited implementation of TYPE_OF(),
> and we have the precedent that foo[] means ARRAY_OF(foo).  I'm not
> sure how we get any extensibility out of either of those things.
>
> Or in short: maybe it's time to blow up %TYPE and start fresh.

That's not a dumb idea.  I think %TYPE is an Oracle-ism, and it
doesn't seem to have been their best-ever design decision.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-03-14 Thread Tom Lane
I wrote:
> However ... one thing I was intending to mention on this thread is that
> "get the array type over this type" isn't the only extension one might
> wish for.  Another likely desire is "get the type of field 'foo' of this
> composite type".  I don't suggest that this patch needs to implement
> that right now; but it would be a good thing if we could see how the
> chosen syntax could be extended in such a direction.  Otherwise we might
> be painting ourselves into a corner.

To enlarge a little bit: it seems to me that what we're really wishing for
here is a type name syntax that goes beyond simple names.  If we were
starting in a green field, we might choose a recursively-composable syntax
like the following.

type_name can be:

* A simple type name, such as int8 or varchar[42].

* TYPE_OF(expression), meaning that the SQL expression is parsed but never
executed, we just take this construct as naming its result type.

* ARRAY_OF(type_name), meaning the array type having type_name as its
element type.

* ELEMENT_OF(type_name), meaning the element type of the array type
named by type_name.

* ROW_OF(type_name [, type_name ...]), meaning the composite type with
those types as columns.

* FIELD_OF(type_name, foo), meaning the type of column "foo" of the
composite type named by type_name.  I'm not sure if there would be
use-cases for selecting a column other than by a simple literal name,
but there could be variants of this function if so.

It's possible to think of other cases, for example what about range
types?  You could allow ELEMENT_OF() to apply to range types, certainly.
I'm not sure about the other direction, because multiple range types
could have the same element type; but it's possible to hope that some
type-naming function along the lines of RANGE_OF(type_name, other args)
could disambiguate.  The main reason I'm thinking of a function-like
syntax here is that it can easily handle additional arguments when
needed.

Comparing this flight of fancy to where we are today, we have
%TYPE as a remarkably ugly and limited implementation of TYPE_OF(),
and we have the precedent that foo[] means ARRAY_OF(foo).  I'm not
sure how we get any extensibility out of either of those things.

Or in short: maybe it's time to blow up %TYPE and start fresh.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-03-14 Thread Artur Zakirov

On 14.03.2016 17:54, Tom Lane wrote:

Joe Conway  writes:

This new version of the patch was posted after the commitfest item was
marked ready for committer. Does anyone have further comments or
objections to the concept or syntax before I try to take this forward?


The quoted excerpt fails to say what solution was adopted to the array
syntax issues, so it's impossible to have an opinion without actually
reading the patch.

However ... one thing I was intending to mention on this thread is that
"get the array type over this type" isn't the only extension one might
wish for.  Another likely desire is "get the type of field 'foo' of this
composite type".  I don't suggest that this patch needs to implement
that right now; but it would be a good thing if we could see how the
chosen syntax could be extended in such a direction.  Otherwise we might
be painting ourselves into a corner.

regards, tom lane



I looked this patch and the previous. The patch applies correctly to 
HEAD. Regression tests pass successfully, without errors.


In comparison with the previous patch it adds the following functionality:
- %TYPE - now can be used for composite types (same syntax).
- %TYPE[] - new functionality, provides the array type from a
variable or table column (syntax was changed).
- var ELEMENT OF othervar%TYPE - new funcitonality, provides the element 
type of a given array (syntax was changed).


Was changed plpgsql_derive_type(). Now it has the following definition:


PLpgSQL_type *
plpgsql_derive_type(PLpgSQL_type *base_type, bool to_element_type, bool 
to_array_type)


Previously it had the following definition:


static PLpgSQL_type *
derive_type(PLpgSQL_type *base_type, PLpgSQL_reftype reftype)


where PLpgSQL_reftype was the enum:


+ typedef enum
+ {
+   PLPGSQL_REFTYPE_TYPE,   /* use type of some variable */
+   PLPGSQL_REFTYPE_ELEMENT,/* use a element type of referenced 
variable */
+   PLPGSQL_REFTYPE_ARRAY   /* use a array type of referenced 
variable */
+ } PLpgSQL_reftype;


I think the previous version was better, because enum is better than 
additional function parameters. But it is only for me.


Also there is a little typo here:


+  * This routine is used for generating element or array type from base type.
+  * The options to_element_type and to_array_type can be used together, when
+  * we would to ensure valid result. The array array type is original type, so
+  * this direction is safe. The element of scalar type is not allowed, but if
+  * we do "to array" transformation first, then this direction should be safe
+  * too. This design is tolerant, because we should to support a design of
+  * polymorphic parameters, where a array value can be passed as anyelement
+  * or anyarray parameter.
+  */
+ PLpgSQL_type *
+ plpgsql_derive_type(PLpgSQL_type *base_type,


Here the word "array" occurs two times in the third line.

--
Artur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-03-14 Thread Tom Lane
Joe Conway  writes:
> This new version of the patch was posted after the commitfest item was
> marked ready for committer. Does anyone have further comments or
> objections to the concept or syntax before I try to take this forward?

The quoted excerpt fails to say what solution was adopted to the array
syntax issues, so it's impossible to have an opinion without actually
reading the patch.

However ... one thing I was intending to mention on this thread is that
"get the array type over this type" isn't the only extension one might
wish for.  Another likely desire is "get the type of field 'foo' of this
composite type".  I don't suggest that this patch needs to implement
that right now; but it would be a good thing if we could see how the
chosen syntax could be extended in such a direction.  Otherwise we might
be painting ourselves into a corner.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-03-14 Thread Joe Conway
On 03/03/2016 05:45 AM, Pavel Stehule wrote:
> 2016-02-24 22:18 GMT+01:00 Peter Eisentraut  >:
> 
> On 1/18/16 4:21 PM, Robert Haas wrote:
> > One idea that occurs to me is: If you can DECLARE BAR FOO%TYPE, but
> > then you want to make BAR an array of that type rather than a scalar,
> > why not write that as DECLARE BAR FOO%TYPE[]?  That seems quite
> > natural to me.
> 
> Right, and it's arguably dubious that that doesn't already work.
> Unfortunately, these % things are just random plpgsql parser hacks, not
> real types.  Maybe this should be done in the main PostgreSQL parser
> with parameter hooks, if we wanted this feature to be available outside
> plpgsql as well.
> 
> > I think the part of this patch that makes %TYPE work for more kinds of
> > types is probably a good idea, although I haven't carefully studied
> > exactly what it does.
> 
> I agree that this should be more general.  For instance, this patch
> would allow you to get the element type of an array-typed variable, but
> there is no way to get the element type of just another type.  If we
> could do something like
> 
> DECLARE
>   var ELEMENT OF point;
> 
> (not necessary that syntax)
> 
> then
> 
> DECLARE
>   var ELEMENT OF othervar%TYPE;
> 
> should just fall into place.
> 
> I am sending update of this patch. The basic concept is same, syntax was
> changed per your and Robert requirement.

This new version of the patch was posted after the commitfest item was
marked ready for committer. Does anyone have further comments or
objections to the concept or syntax before I try to take this forward?

Thanks,

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development



signature.asc
Description: OpenPGP digital signature


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-03-03 Thread Pavel Stehule
Hi

2016-03-03 0:27 GMT+01:00 Jim Nasby :

> On 3/2/16 3:52 PM, Pavel Stehule wrote:
>
>> Right, and it's arguably dubious that that doesn't already work.
>> Unfortunately, these % things are just random plpgsql parser hacks,
>> not
>> real types.  Maybe this should be done in the main PostgreSQL parser
>> with parameter hooks, if we wanted this feature to be available
>> outside
>> plpgsql as well.
>>
>>
>> I am not fan to propagate this feature outside PLpgSQL - it is possible
>> new dependency between database object, and the cost is higher than
>> benefits.
>>
>
> I fail to see how it'd be a dependency. I'd expect it to look up the type
> when you run the command, just like plpgsql does. I think it'd be useful to
> have.
>

if we publish this feature to SQL, then somebody can use it in table
definition

CREATE TABLE a(a int);
CREATE TABLE b(a a.a%TYPE)

And the people expecting the living relation between table a and table b.
So when I do ALTER a.a, then b.a should be changed. What if I drop a.a or
drop a?

So this is reason, why I don't would this feature in SQL side.

Regards

Pavel


>
> That said, I think that should be a completely separate patch and
> discussion. Lets at least get it into plpgsql first.
>
> As for the array of element/element of array feature; I agree it would be
> nice, but we're pretty late in the game for that, and I don't see why that
> couldn't be added later.
>
> --
> Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
> Experts in Analytics, Data Architecture and PostgreSQL
> Data in Trouble? Get it in Treble! http://BlueTreble.com
>


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-03-03 Thread Pavel Stehule
Hi

2016-02-24 22:18 GMT+01:00 Peter Eisentraut :

> On 1/18/16 4:21 PM, Robert Haas wrote:
> > One idea that occurs to me is: If you can DECLARE BAR FOO%TYPE, but
> > then you want to make BAR an array of that type rather than a scalar,
> > why not write that as DECLARE BAR FOO%TYPE[]?  That seems quite
> > natural to me.
>
> Right, and it's arguably dubious that that doesn't already work.
> Unfortunately, these % things are just random plpgsql parser hacks, not
> real types.  Maybe this should be done in the main PostgreSQL parser
> with parameter hooks, if we wanted this feature to be available outside
> plpgsql as well.
>
> > I think the part of this patch that makes %TYPE work for more kinds of
> > types is probably a good idea, although I haven't carefully studied
> > exactly what it does.
>
> I agree that this should be more general.  For instance, this patch
> would allow you to get the element type of an array-typed variable, but
> there is no way to get the element type of just another type.  If we
> could do something like
>
> DECLARE
>   var ELEMENT OF point;
>
> (not necessary that syntax)
>
> then
>
> DECLARE
>   var ELEMENT OF othervar%TYPE;
>
> should just fall into place.
>
>
I am sending update of this patch. The basic concept is same, syntax was
changed per your and Robert requirement.

Regards

Pavel
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
new file mode 100644
index 9786242..5587839
*** a/doc/src/sgml/plpgsql.sgml
--- b/doc/src/sgml/plpgsql.sgml
*** url varchar;
*** 322,334 
  myrow tablename%ROWTYPE;
  myfield tablename.columnname%TYPE;
  arow RECORD;
  
  
  
  
   The general syntax of a variable declaration is:
  
! name  CONSTANT  type  COLLATE collation_name   NOT NULL   { DEFAULT | := | = } expression ;
  
The DEFAULT clause, if given, specifies the initial value assigned
to the variable when the block is entered.  If the DEFAULT clause
--- 322,336 
  myrow tablename%ROWTYPE;
  myfield tablename.columnname%TYPE;
  arow RECORD;
+ myarray tablename.columnname%TYPE[];
+ myelement ELEMENT OF arrayparam%TYPE;
  
  
  
  
   The general syntax of a variable declaration is:
  
! name  CONSTANT   ELEMENT OF  type  COLLATE collation_name   NOT NULL   { DEFAULT | := | = } expression ;
  
The DEFAULT clause, if given, specifies the initial value assigned
to the variable when the block is entered.  If the DEFAULT clause
*** arow RECORD;
*** 337,342 
--- 339,347 
The CONSTANT option prevents the variable from being
assigned to after initialization, so that its value will remain constant
for the duration of the block.
+   The ELEMENT OF ensure using the element type of a given array type.
+   This construct is valuable in polymorphic functions, since the data types needed
+   for internal variables can change from one call to the next call.
The COLLATE option specifies a collation to use for the
variable (see ).
If NOT NULL
*** user_id users.user_id%TYPE;
*** 611,616 
--- 616,666 
  change from one call to the next.  Appropriate variables can be
  created by applying %TYPE to the function's
  arguments or result placeholders.
+ 
+ CREATE OR REPLACE FUNCTION array_init(v anyelement, size integer)
+ RETURNS anyarray AS $$
+ DECLARE
+ result v%TYPE[] DEFAULT '{}';
+ BEGIN
+ -- prefer builtin function array_fill
+ FOR i IN 1 .. size
+ LOOP
+ result := result || v;
+ END LOOP;
+ RETURN result;
+ END;
+ $$ LANGUAGE plpgsql;
+ 
+ SELECT array_init(0::numeric, 10);
+ SELECT array_init(''::varchar, 10);
+ 
+ 
+ CREATE OR REPLACE FUNCTION bubble_sort(a anyarray)
+ RETURNS anyarray AS $$
+ DECLARE
+ aux ELEMENT OF a%TYPE;
+ repeat_again boolean DEFAULT true;
+ BEGIN
+ -- Don't use this code for large arrays!
+ -- use builtin sort
+ WHILE repeat_again
+ LOOP
+ repeat_again := false;
+ FOR i IN array_lower(a, 1) .. array_upper(a, 1)
+ LOOP
+ IF a[i] > a[i+1] THEN
+ aux := a[i+1];
+ a[i+1] := a[i]; a[i] := aux;
+ repeat_again := true;
+ END IF;
+ END LOOP;
+ END LOOP;
+ RETURN a;
+ END;
+ $$ LANGUAGE plpgsql;
+ 
+ SELECT bubble_sort(ARRAY[3,2,4,6,1]);
+ 
 
  

diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
new file mode 100644
index 2aeab96..b77117e
*** a/src/pl/plpgsql/src/pl_comp.c
--- b/src/pl/plpgsql/src/pl_comp.c
*** plpgsql_parse_wordtype(char *ident)
*** 1646,1653 
  			case PLPGSQL_NSTYPE_VAR:
  return ((PLpgSQL_var *) (plpgsql_Datums[nse->itemno]))->datatype;
  
! /* XXX perhaps allow REC/ROW here? */
  
  			default:
  return NULL;
  		}
--- 1646,1660 
  			case PLPGSQL_NSTYPE_VAR:
  return ((PLpgSQL_var *) 

Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-03-02 Thread Jim Nasby

On 3/2/16 3:52 PM, Pavel Stehule wrote:

Right, and it's arguably dubious that that doesn't already work.
Unfortunately, these % things are just random plpgsql parser hacks, not
real types.  Maybe this should be done in the main PostgreSQL parser
with parameter hooks, if we wanted this feature to be available outside
plpgsql as well.


I am not fan to propagate this feature outside PLpgSQL - it is possible
new dependency between database object, and the cost is higher than
benefits.


I fail to see how it'd be a dependency. I'd expect it to look up the 
type when you run the command, just like plpgsql does. I think it'd be 
useful to have.


That said, I think that should be a completely separate patch and 
discussion. Lets at least get it into plpgsql first.


As for the array of element/element of array feature; I agree it would 
be nice, but we're pretty late in the game for that, and I don't see why 
that couldn't be added later.

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-03-02 Thread Pavel Stehule
Hi

2016-02-24 22:18 GMT+01:00 Peter Eisentraut :

> On 1/18/16 4:21 PM, Robert Haas wrote:
> > One idea that occurs to me is: If you can DECLARE BAR FOO%TYPE, but
> > then you want to make BAR an array of that type rather than a scalar,
> > why not write that as DECLARE BAR FOO%TYPE[]?  That seems quite
> > natural to me.
>
> Right, and it's arguably dubious that that doesn't already work.
> Unfortunately, these % things are just random plpgsql parser hacks, not
> real types.  Maybe this should be done in the main PostgreSQL parser
> with parameter hooks, if we wanted this feature to be available outside
> plpgsql as well.
>

I am not fan to propagate this feature outside PLpgSQL - it is possible new
dependency between database object, and the cost is higher than benefits.


>
> > I think the part of this patch that makes %TYPE work for more kinds of
> > types is probably a good idea, although I haven't carefully studied
> > exactly what it does.
>
> I agree that this should be more general.  For instance, this patch
> would allow you to get the element type of an array-typed variable, but
> there is no way to get the element type of just another type.  If we
> could do something like
>
> DECLARE
>   var ELEMENT OF point;
>

isn't it bug? What is sense of this construct? Our other manipulation with
a arrays we raise a error, when we try to to take a element from non array
value.

Today I did work on this patch and I am able to implement the syntax
proposed by you. It is proprietary, but similar to ADA anonymous types.

DECLARE x array() of type

Regards

Pavel


>
> (not necessary that syntax)
>
> then
>
> DECLARE
>   var ELEMENT OF othervar%TYPE;
>
> should just fall into place.
>
>


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-02-24 Thread Peter Eisentraut
On 1/18/16 4:21 PM, Robert Haas wrote:
> One idea that occurs to me is: If you can DECLARE BAR FOO%TYPE, but
> then you want to make BAR an array of that type rather than a scalar,
> why not write that as DECLARE BAR FOO%TYPE[]?  That seems quite
> natural to me.

Right, and it's arguably dubious that that doesn't already work.
Unfortunately, these % things are just random plpgsql parser hacks, not
real types.  Maybe this should be done in the main PostgreSQL parser
with parameter hooks, if we wanted this feature to be available outside
plpgsql as well.

> I think the part of this patch that makes %TYPE work for more kinds of
> types is probably a good idea, although I haven't carefully studied
> exactly what it does.

I agree that this should be more general.  For instance, this patch
would allow you to get the element type of an array-typed variable, but
there is no way to get the element type of just another type.  If we
could do something like

DECLARE
  var ELEMENT OF point;

(not necessary that syntax)

then

DECLARE
  var ELEMENT OF othervar%TYPE;

should just fall into place.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-02-24 Thread Pavel Stehule
Hi

2016-02-24 10:48 GMT+01:00 Artur Zakirov :

On 21.02.2016 11:31, Pavel Stehule wrote:
>
>> Hi
>>
>> I am sending updated version - the changes are related to fix comments.
>>
>>
> Great.
>
> I am new in reviewing, I think Pavel took into account all comments. This
> patch is compiled and regression tests are passed. So I change its status
> to "Ready for Committer".
>

Thank you very much

Regards

Pavel


>
>
> --
> Artur Zakirov
> Postgres Professional: http://www.postgrespro.com
> Russian Postgres Company
>


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-02-24 Thread Artur Zakirov

On 21.02.2016 11:31, Pavel Stehule wrote:

Hi

I am sending updated version - the changes are related to fix comments.



Great.

I am new in reviewing, I think Pavel took into account all comments. 
This patch is compiled and regression tests are passed. So I change its 
status to "Ready for Committer".





 By the way, these functions are misnamed after this patch.
They are
 called "wordtype" and "cwordtype" originally because they
accept
 "word%TYPE" and "compositeword%TYPE", but after the patch
they not only
 accept TYPE at the right of the percent sign but also
ELEMENTTYPE and
 ARRAYTYPE.  Not sure that this is something we want to be
too strict
 about.


Understand - used name ***reftype instead type


I am not sure, but it seems that new names is a little worse. I
think original names are good too. They accept a word and return the
PLpgSQL_type structure.


The "TYPE" word in this name was related to syntax %TYPE. And because
new syntax allows more constructs, then the change name is correct. I am
think. But choosing names is hard work. The new name little bit more
strongly show relation to work with referenced types.



Agree.


--
Artur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-02-21 Thread Pavel Stehule
Hi

I am sending updated version - the changes are related to fix comments.

2016-02-19 10:41 GMT+01:00 Artur Zakirov :

> It seems all fixes are done. I tested the patch and regression tests
> passed.
>
> I think here Alvaro means that you should keep original comment without
> the ROW. Like this:
>
> /* XXX perhaps allow REC here? */


I tried rewording this comment

>
>
>
>>
>> By the way, these functions are misnamed after this patch.  They are
>> called "wordtype" and "cwordtype" originally because they accept
>> "word%TYPE" and "compositeword%TYPE", but after the patch they not
>> only
>> accept TYPE at the right of the percent sign but also ELEMENTTYPE and
>> ARRAYTYPE.  Not sure that this is something we want to be too strict
>> about.
>>
>>
>> Understand - used name ***reftype instead type
>>
>
> I am not sure, but it seems that new names is a little worse. I think
> original names are good too. They accept a word and return the PLpgSQL_type
> structure.
>

The "TYPE" word in this name was related to syntax %TYPE. And because new
syntax allows more constructs, then the change name is correct. I am think.
But choosing names is hard work. The new name little bit more strongly show
relation to work with referenced types.


>
>
>>
> I noticed a little typo in the comment in the derive_type():
> /* Return base_type, when it is a array already */
>
> should be:
> /* Return base_type, when it is an array already */
>
>
fixed

Regards

Pavel


>
> --
> Artur Zakirov
> Postgres Professional: http://www.postgrespro.com
> Russian Postgres Company
>
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
new file mode 100644
index 9786242..140c81f
*** a/doc/src/sgml/plpgsql.sgml
--- b/doc/src/sgml/plpgsql.sgml
*** SELECT merge_fields(t.*) FROM table1 t W
*** 710,715 
--- 710,792 
 

  
+   
+Array Types
+ 
+ 
+ variable%ARRAYTYPE
+ 
+ 
+
+ %ARRAYTYPE provides the array type from a variable or
+ table column. %ARRAYTYPE is particularly valuable in
+ polymorphic functions, since the data types needed for internal variables can
+ change from one call to the next.  Appropriate variables can be
+ created by applying %ARRAYTYPE to the function's
+ arguments or result placeholders:
+ 
+ CREATE OR REPLACE FUNCTION array_init(v anyelement, size integer)
+ RETURNS anyarray AS $$
+ DECLARE
+ result v%ARRAYTYPE DEFAULT '{}';
+ BEGIN
+ -- prefer builtin function array_fill
+ FOR i IN 1 .. size
+ LOOP
+ result := result || v;
+ END LOOP;
+ RETURN result;
+ END;
+ $$ LANGUAGE plpgsql;
+ 
+ SELECT array_init(0::numeric, 10);
+ SELECT array_init(''::varchar, 10);
+ 
+
+   
+ 
+   
+Array Element Types
+ 
+ 
+ variable%ELEMENTTYPE
+ 
+ 
+
+ %ELEMENTTYPE provides the element type of a given
+ array.  %ELEMENTTYPE is particularly valuable in polymorphic
+ functions, since the data types needed for internal variables can
+ change from one call to the next.  Appropriate variables can be
+ created by applying %ELEMENTTYPE to the function's
+ arguments or result placeholders:
+ 
+ CREATE OR REPLACE FUNCTION bubble_sort(a anyarray)
+ RETURNS anyarray AS $$
+ DECLARE
+ aux a%ELEMENTTYPE;
+ repeat_again boolean DEFAULT true;
+ BEGIN
+ -- Don't use this code for large arrays!
+ -- use builtin sort
+ WHILE repeat_again
+ LOOP
+ repeat_again := false;
+ FOR i IN array_lower(a, 1) .. array_upper(a, 1)
+ LOOP
+ IF a[i] > a[i+1] THEN
+ aux := a[i+1];
+ a[i+1] := a[i]; a[i] := aux;
+ repeat_again := true;
+ END IF;
+ END LOOP;
+ END LOOP;
+ RETURN a;
+ END;
+ $$ LANGUAGE plpgsql;
+ 
+
+   
+ 

 Collation of PL/pgSQL Variables
  
diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
new file mode 100644
index ebe152d..07569f7
*** a/src/pl/plpgsql/src/pl_comp.c
--- b/src/pl/plpgsql/src/pl_comp.c
*** plpgsql_parse_tripword(char *word1, char
*** 1617,1632 
  	return false;
  }
  
  
  /* --
!  * plpgsql_parse_wordtype	The scanner found word%TYPE. word can be
!  *a variable name or a basetype.
   *
   * Returns datatype struct, or NULL if no match found for word.
   * --
   */
  PLpgSQL_type *
! plpgsql_parse_wordtype(char *ident)
  {
  	PLpgSQL_type *dtype;
  	PLpgSQL_nsitem *nse;
--- 1617,1683 
  	return false;
  }
  
+ /*
+  * Derive type from any base type controlled by reftype.
+  *
+  * This routine allows to take array type from an array variable. This behave
+  * is not consistent with enforce_generic_type_consistency, where same task
+  * fails on ERROR: 42704: could not find array type for data type xxx[].
+  */
+ static PLpgSQL_type *
+ derive_type(PLpgSQL_type *base_type, PLpgSQL_reftype reftype)
+ {
+ 	Oid typoid;
+ 
+ 	switch (reftype)

Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-02-19 Thread Artur Zakirov

It seems all fixes are done. I tested the patch and regression tests passed.

On 27.01.2016 20:58, Pavel Stehule wrote:



 > --- 1681,1687 
 >* --
 >*/
 >   PLpgSQL_type *
 > ! plpgsql_parse_wordtype(char *ident, int reftype_mode)
 >   {
 >   PLpgSQL_type *dtype;
 >   PLpgSQL_nsitem *nse;

Use the typedef'ed enum, as above.

 > --- 1699,1721 
 >   switch (nse->itemtype)
 >   {
 >   case PLPGSQL_NSTYPE_VAR:
 > ! {
 > ! dtype = ((PLpgSQL_var *)
(plpgsql_Datums[nse->itemno]))->datatype;
 > ! return derive_type(dtype,
reftype_mode);
 > ! }
 >
 > ! case PLPGSQL_NSTYPE_ROW:
 > ! {
 > ! dtype = ((PLpgSQL_row *)
(plpgsql_Datums[nse->itemno]))->datatype;
 > ! return derive_type(dtype,
reftype_mode);
 > ! }
 >
 > + /*
 > +  * XXX perhaps allow REC here? Probably it
has not any sense, because
 > +  * in this moment, because PLpgSQL doesn't
support rec parameters, so
 > +  * there should not be any rec polymorphic
parameter, and any work can
 > +  * be done inside function.
 > +  */

I think you should remove from the "?" onwards in that comment, i.e.
just keep what was already in the original comment (minus the ROW)


I tried to fix it, not sure if understood well.


I think here Alvaro means that you should keep original comment without 
the ROW. Like this:


/* XXX perhaps allow REC here? */



 > *** extern bool plpgsql_parse_dblword(char *
 > *** 961,968 
 > PLwdatum *wdatum, PLcword
*cword);
 >   extern bool plpgsql_parse_tripword(char *word1, char *word2,
char *word3,
 >  PLwdatum *wdatum,
PLcword *cword);
 > ! extern PLpgSQL_type *plpgsql_parse_wordtype(char *ident);
 > ! extern PLpgSQL_type *plpgsql_parse_cwordtype(List *idents);
 >   extern PLpgSQL_type *plpgsql_parse_wordrowtype(char *ident);
 >   extern PLpgSQL_type *plpgsql_parse_cwordrowtype(List *idents);
 >   extern PLpgSQL_type *plpgsql_build_datatype(Oid typeOid, int32
typmod,
 > --- 973,980 
 > PLwdatum *wdatum, PLcword
*cword);
 >   extern bool plpgsql_parse_tripword(char *word1, char *word2,
char *word3,
 >  PLwdatum *wdatum,
PLcword *cword);
 > ! extern PLpgSQL_type *plpgsql_parse_wordtype(char *ident, int
reftype_mode);
 > ! extern PLpgSQL_type *plpgsql_parse_cwordtype(List *idents, int
reftype_mode);
 >   extern PLpgSQL_type *plpgsql_parse_wordrowtype(char *ident);
 >   extern PLpgSQL_type *plpgsql_parse_cwordrowtype(List *idents);
 >   extern PLpgSQL_type *plpgsql_build_datatype(Oid typeOid, int32
typmod,

By the way, these functions are misnamed after this patch.  They are
called "wordtype" and "cwordtype" originally because they accept
"word%TYPE" and "compositeword%TYPE", but after the patch they not only
accept TYPE at the right of the percent sign but also ELEMENTTYPE and
ARRAYTYPE.  Not sure that this is something we want to be too strict
about.


Understand - used name ***reftype instead type


I am not sure, but it seems that new names is a little worse. I think 
original names are good too. They accept a word and return the 
PLpgSQL_type structure.




Thank you for your comment

Attached updated patch

Regards

Pavel



--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services




I noticed a little typo in the comment in the derive_type():
/* Return base_type, when it is a array already */

should be:
/* Return base_type, when it is an array already */

--
Artur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-01-27 Thread Pavel Stehule
Hi

2016-01-18 21:37 GMT+01:00 Alvaro Herrera :

> > diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
> > new file mode 100644
> > index 1ae4bb7..c819517
> > *** a/src/pl/plpgsql/src/pl_comp.c
> > --- b/src/pl/plpgsql/src/pl_comp.c
> > *** plpgsql_parse_tripword(char *word1, char
> > *** 1617,1622 
> > --- 1617,1677 
> >   return false;
> >   }
> >
> > + /*
> > +  * Derive type from ny base type controlled by reftype_mode
> > +  */
> > + static PLpgSQL_type *
> > + derive_type(PLpgSQL_type *base_type, int reftype_mode)
> > + {
> > + Oid typoid;
>
> I think you should add a typedef to the REFTYPE enum, and have this
> function take that type rather than int.
>

done


>
> > + case PLPGSQL_REFTYPE_ARRAY:
> > + {
> > + /*
> > +  * Question: can we allow anyelement (array or
> nonarray) -> array direction.
> > +  * if yes, then probably we have to modify
> enforce_generic_type_consistency,
> > +  * parse_coerce.c where still is check on scalar
> type -> raise error
> > +  * ERROR:  42704: could not find array type for
> data type integer[]
> > +  *
> > + if
> (OidIsValid(get_element_type(base_type->typoid)))
> > + return base_type;
> > + */
>
> I think it would be better to resolve this question outside a code
> comment.
>

done


>
> > + typoid = get_array_type(base_type->typoid);
> > + if (!OidIsValid(typoid))
> > + ereport(ERROR,
> > +
>  (errcode(ERRCODE_DATATYPE_MISMATCH),
> > +  errmsg("there are not
> array type for type %s",
> > +
>  format_type_be(base_type->typoid;
>
> nodeFuncs.c uses this wording:
> errmsg("could not find array type for data type %s",
> which I think you should adopt.
>

sure, fixed


>
> > --- 1681,1687 
> >* --
> >*/
> >   PLpgSQL_type *
> > ! plpgsql_parse_wordtype(char *ident, int reftype_mode)
> >   {
> >   PLpgSQL_type *dtype;
> >   PLpgSQL_nsitem *nse;
>
> Use the typedef'ed enum, as above.
>
> > --- 1699,1721 
> >   switch (nse->itemtype)
> >   {
> >   case PLPGSQL_NSTYPE_VAR:
> > ! {
> > ! dtype = ((PLpgSQL_var *)
> (plpgsql_Datums[nse->itemno]))->datatype;
> > ! return derive_type(dtype, reftype_mode);
> > ! }
> >
> > ! case PLPGSQL_NSTYPE_ROW:
> > ! {
> > ! dtype = ((PLpgSQL_row *)
> (plpgsql_Datums[nse->itemno]))->datatype;
> > ! return derive_type(dtype, reftype_mode);
> > ! }
> >
> > + /*
> > +  * XXX perhaps allow REC here? Probably it has not
> any sense, because
> > +  * in this moment, because PLpgSQL doesn't support
> rec parameters, so
> > +  * there should not be any rec polymorphic
> parameter, and any work can
> > +  * be done inside function.
> > +  */
>
> I think you should remove from the "?" onwards in that comment, i.e.
> just keep what was already in the original comment (minus the ROW)
>

I tried to fix it, not sure if understood well.


>
> > --- 1757,1763 
> >* --
> >*/
> >   PLpgSQL_type *
> > ! plpgsql_parse_cwordtype(List *idents, int reftype_mode)
> >   {
> >   PLpgSQL_type *dtype = NULL;
> >   PLpgSQL_nsitem *nse;
>
> Typedef.
>
> > --- 2720,2737 
> >   tok = yylex();
> >   if (tok_is_keyword(tok, ,
> >  K_TYPE, "type"))
> > ! result = plpgsql_parse_wordtype(dtname,
> PLPGSQL_REFTYPE_TYPE);
> > ! else if (tok_is_keyword(tok, ,
> > !
>  K_ELEMENTTYPE, "elementtype"))
> > ! result = plpgsql_parse_wordtype(dtname,
> PLPGSQL_REFTYPE_ELEMENT);
> > ! else if (tok_is_keyword(tok, ,
> > !
>  K_ARRAYTYPE, "arraytype"))
> > ! result = plpgsql_parse_wordtype(dtname,
> PLPGSQL_REFTYPE_ARRAY);
> >   else if (tok_is_keyword(tok, ,
> >
>  K_ROWTYPE, "rowtype"))
> >   result = plpgsql_parse_wordrowtype(dtname);
> > ! if (result)
> > ! return result;
> >   }
>
> This plpgsql parser stuff is pretty tiresome.  (Not this patch's fault
> -- just saying.)
>
>
> > *** extern bool plpgsql_parse_dblword(char *
> > *** 961,968 
> > PLwdatum *wdatum, 

Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-01-19 Thread Pavel Stehule
2016-01-18 22:48 GMT+01:00 Robert Haas :

> On Mon, Jan 18, 2016 at 4:35 PM, Pavel Stehule 
> wrote:
> >> I know that Oracle uses syntax of this general type, but I've always
> >> found it ugly.  It's also pretty non-extensible.  You could want
> >> similar things for range types and any other container types we might
> >> get in the future, but clearly adding new reserved words for each one
> >> is no good.
> >
> > It doesn't use reserved worlds.
>
> OK - keywords, then.
>
> >> One idea that occurs to me is: If you can DECLARE BAR FOO%TYPE, but
> >> then you want to make BAR an array of that type rather than a scalar,
> >> why not write that as DECLARE BAR FOO%TYPE[]?  That seems quite
> >> natural to me.
> >
> > what you propose for syntax for taking a element of array?
>
> No idea.
>

the syntax for "array from" is natural, but for any other is hard. So it is
reason, why I used text form. Using Oracle's pattern source%operation
allows to use nonreserved keywords. Probably any text can be there. The
keywords isn't necessary (not tested).


> >> I think the part of this patch that makes %TYPE work for more kinds of
> >> types is probably a good idea, although I haven't carefully studied
> >> exactly what it does.
> >
> >
> > I invite any ideas, but currently used notation is only in direction
> > type->array. The working with symbols looks more difficult, than using
> words
> > (in design area).
> >
> > More - the textual form is more near to our system of polymorphics types:
> > anyelement, anyarray, ... We have not anyelement[]
>
> True, but this is hardly a straightforward extension of what we have
> today either.
>

It is, but sometime the polymorphic types can help.

The proposed feature/syntax has sense primary for polymorphic types. It
should to follow our polymorphic types. The primary pair is
"anyarray","anyelement"  -> "arraytype","elemementtype".

If you don't use polymorphic parameters in plpgsql, then proposed feature
can look like useless.
Regards

Pavel



>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-01-19 Thread Robert Haas
On Tue, Jan 19, 2016 at 4:53 AM, Pavel Stehule  wrote:
> It is, but sometime the polymorphic types can help.
>
> The proposed feature/syntax has sense primary for polymorphic types. It
> should to follow our polymorphic types. The primary pair is
> "anyarray","anyelement"  -> "arraytype","elemementtype".
>
> If you don't use polymorphic parameters in plpgsql, then proposed feature
> can look like useless.

I don't think it's useless, but I do think the syntax is ugly.  Maybe
it's the best we can do and we should just live with it, but Alvaro
asked for opinions, so there's mine.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-01-19 Thread Pavel Stehule
2016-01-20 0:34 GMT+01:00 Robert Haas :

> On Tue, Jan 19, 2016 at 4:53 AM, Pavel Stehule 
> wrote:
> > It is, but sometime the polymorphic types can help.
> >
> > The proposed feature/syntax has sense primary for polymorphic types. It
> > should to follow our polymorphic types. The primary pair is
> > "anyarray","anyelement"  -> "arraytype","elemementtype".
> >
> > If you don't use polymorphic parameters in plpgsql, then proposed feature
> > can look like useless.
>
> I don't think it's useless, but I do think the syntax is ugly.  Maybe
> it's the best we can do and we should just live with it, but Alvaro
> asked for opinions, so there's mine.
>

ok

5 years ago, maybe more - I proposed more nice syntax - and it was rejected
as too complex (reserved worlds was required). So this solution try to
attack it from different side. It is simple and effective.

Regards

Pavel


>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-01-18 Thread Alvaro Herrera
> diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
> new file mode 100644
> index 1ae4bb7..c819517
> *** a/src/pl/plpgsql/src/pl_comp.c
> --- b/src/pl/plpgsql/src/pl_comp.c
> *** plpgsql_parse_tripword(char *word1, char
> *** 1617,1622 
> --- 1617,1677 
>   return false;
>   }
>   
> + /*
> +  * Derive type from ny base type controlled by reftype_mode
> +  */
> + static PLpgSQL_type *
> + derive_type(PLpgSQL_type *base_type, int reftype_mode)
> + {
> + Oid typoid;

I think you should add a typedef to the REFTYPE enum, and have this
function take that type rather than int.

> + case PLPGSQL_REFTYPE_ARRAY:
> + {
> + /*
> +  * Question: can we allow anyelement (array or 
> nonarray) -> array direction.
> +  * if yes, then probably we have to modify 
> enforce_generic_type_consistency,
> +  * parse_coerce.c where still is check on scalar type 
> -> raise error
> +  * ERROR:  42704: could not find array type for data 
> type integer[]
> +  *
> + if (OidIsValid(get_element_type(base_type->typoid)))
> + return base_type;
> + */

I think it would be better to resolve this question outside a code
comment.

> + typoid = get_array_type(base_type->typoid);
> + if (!OidIsValid(typoid))
> + ereport(ERROR,
> + 
> (errcode(ERRCODE_DATATYPE_MISMATCH),
> +  errmsg("there are not array 
> type for type %s",
> + 
> format_type_be(base_type->typoid;

nodeFuncs.c uses this wording:
errmsg("could not find array type for data type %s",
which I think you should adopt.

> --- 1681,1687 
>* --
>*/
>   PLpgSQL_type *
> ! plpgsql_parse_wordtype(char *ident, int reftype_mode)
>   {
>   PLpgSQL_type *dtype;
>   PLpgSQL_nsitem *nse;

Use the typedef'ed enum, as above.

> --- 1699,1721 
>   switch (nse->itemtype)
>   {
>   case PLPGSQL_NSTYPE_VAR:
> ! {
> ! dtype = ((PLpgSQL_var *) 
> (plpgsql_Datums[nse->itemno]))->datatype;
> ! return derive_type(dtype, reftype_mode);
> ! }
>   
> ! case PLPGSQL_NSTYPE_ROW:
> ! {
> ! dtype = ((PLpgSQL_row *) 
> (plpgsql_Datums[nse->itemno]))->datatype;
> ! return derive_type(dtype, reftype_mode);
> ! }
>   
> + /*
> +  * XXX perhaps allow REC here? Probably it has not any 
> sense, because
> +  * in this moment, because PLpgSQL doesn't support rec 
> parameters, so
> +  * there should not be any rec polymorphic parameter, 
> and any work can
> +  * be done inside function.
> +  */

I think you should remove from the "?" onwards in that comment, i.e.
just keep what was already in the original comment (minus the ROW)

> --- 1757,1763 
>* --
>*/
>   PLpgSQL_type *
> ! plpgsql_parse_cwordtype(List *idents, int reftype_mode)
>   {
>   PLpgSQL_type *dtype = NULL;
>   PLpgSQL_nsitem *nse;

Typedef.

> --- 2720,2737 
>   tok = yylex();
>   if (tok_is_keyword(tok, ,
>  K_TYPE, "type"))
> ! result = plpgsql_parse_wordtype(dtname, 
> PLPGSQL_REFTYPE_TYPE);
> ! else if (tok_is_keyword(tok, ,
> ! 
> K_ELEMENTTYPE, "elementtype"))
> ! result = plpgsql_parse_wordtype(dtname, 
> PLPGSQL_REFTYPE_ELEMENT);
> ! else if (tok_is_keyword(tok, ,
> ! 
> K_ARRAYTYPE, "arraytype"))
> ! result = plpgsql_parse_wordtype(dtname, 
> PLPGSQL_REFTYPE_ARRAY);
>   else if (tok_is_keyword(tok, ,
>   
> K_ROWTYPE, "rowtype"))
>   result = plpgsql_parse_wordrowtype(dtname);
> ! if (result)
> ! return result;
>   }

This plpgsql parser stuff is pretty tiresome.  (Not this patch's fault
-- just saying.)

   
> *** extern bool plpgsql_parse_dblword(char *
> *** 961,968 
> PLwdatum *wdatum, PLcword *cword);
>   extern bool plpgsql_parse_tripword(char 

Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-01-18 Thread Alvaro Herrera
FWIW the reason I read through this patch is that I wondered if there
was anything in common with this other patch
https://commitfest.postgresql.org/8/459/ -- and the answer seems to be
"no".  What that patch does is add a new construct
  TYPE(1+1) 
which in this case returns "int4"; I guess if we wanted to augment that
functionality to cover Pavel's use case we would additionally need
  ELEMENTTYPE(somearray)
and
  ARRAYTYPE(some-non-array)
in the core grammar ... sounds like a hard sell.

BTW are we all agreed that enabling
  foo%ARRAYTYPE
and
  foo%ELEMENTYPE
in plpgsql's DECLARE section is what we want for this?

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-01-18 Thread Robert Haas
On Mon, Jan 18, 2016 at 4:35 PM, Pavel Stehule  wrote:
>> I know that Oracle uses syntax of this general type, but I've always
>> found it ugly.  It's also pretty non-extensible.  You could want
>> similar things for range types and any other container types we might
>> get in the future, but clearly adding new reserved words for each one
>> is no good.
>
> It doesn't use reserved worlds.

OK - keywords, then.

>> One idea that occurs to me is: If you can DECLARE BAR FOO%TYPE, but
>> then you want to make BAR an array of that type rather than a scalar,
>> why not write that as DECLARE BAR FOO%TYPE[]?  That seems quite
>> natural to me.
>
> what you propose for syntax for taking a element of array?

No idea.

>> I think the part of this patch that makes %TYPE work for more kinds of
>> types is probably a good idea, although I haven't carefully studied
>> exactly what it does.
>
>
> I invite any ideas, but currently used notation is only in direction
> type->array. The working with symbols looks more difficult, than using words
> (in design area).
>
> More - the textual form is more near to our system of polymorphics types:
> anyelement, anyarray, ... We have not anyelement[]

True, but this is hardly a straightforward extension of what we have
today either.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-01-18 Thread Robert Haas
On Mon, Jan 18, 2016 at 3:51 PM, Alvaro Herrera
 wrote:
> BTW are we all agreed that enabling
>   foo%ARRAYTYPE
> and
>   foo%ELEMENTYPE
> in plpgsql's DECLARE section is what we want for this?

I know that Oracle uses syntax of this general type, but I've always
found it ugly.  It's also pretty non-extensible.  You could want
similar things for range types and any other container types we might
get in the future, but clearly adding new reserved words for each one
is no good.

One idea that occurs to me is: If you can DECLARE BAR FOO%TYPE, but
then you want to make BAR an array of that type rather than a scalar,
why not write that as DECLARE BAR FOO%TYPE[]?  That seems quite
natural to me.

I think the part of this patch that makes %TYPE work for more kinds of
types is probably a good idea, although I haven't carefully studied
exactly what it does.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2016-01-18 Thread Pavel Stehule
2016-01-18 22:21 GMT+01:00 Robert Haas :

> On Mon, Jan 18, 2016 at 3:51 PM, Alvaro Herrera
>  wrote:
> > BTW are we all agreed that enabling
> >   foo%ARRAYTYPE
> > and
> >   foo%ELEMENTYPE
> > in plpgsql's DECLARE section is what we want for this?
>
> I know that Oracle uses syntax of this general type, but I've always
> found it ugly.  It's also pretty non-extensible.  You could want
> similar things for range types and any other container types we might
> get in the future, but clearly adding new reserved words for each one
> is no good.
>

It doesn't use reserved worlds.


>
> One idea that occurs to me is: If you can DECLARE BAR FOO%TYPE, but
> then you want to make BAR an array of that type rather than a scalar,
> why not write that as DECLARE BAR FOO%TYPE[]?  That seems quite
> natural to me.
>

what you propose for syntax for taking a element of array?


>
> I think the part of this patch that makes %TYPE work for more kinds of
> types is probably a good idea, although I haven't carefully studied
> exactly what it does.
>

I invite any ideas, but currently used notation is only in direction
type->array. The working with symbols looks more difficult, than using
words (in design area).

More - the textual form is more near to our system of polymorphics types:
anyelement, anyarray, ... We have not anyelement[]

Regards

Pavel


>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2015-12-23 Thread Michael Paquier
On Tue, Dec 22, 2015 at 5:59 PM, Pavel Stehule  wrote:
> Hi
>
> 2015-12-21 16:21 GMT+01:00 Artur Zakirov :
>>
>> Hi.
>> I have tried to do some review of this patch. Below are my comments.
>>
>> Introduction:
>>
>> This patch fixes and adds the following functionality:
>> - %TYPE - now can be used for composite types.
>> - %ARRAYTYPE - new functionality, provides the array type from a variable
>> or table column.
>> - %ELEMENTTYPE - new funcitonality, provides the element type of a given
>> array.
>>
>> New regression tests are included in the patch. Changes to the
>> documentation are not provided.
>>
>> Initial Run:
>>
>> The patch applies correctly to HEAD. Regression tests pass successfully,
>> without errors. It seems that the patch work as you expected.
>>
>> Performance:
>>
>> It seems patch have not possible performance issues for the existing
>> functionality.
>>
>> Coding:
>>
>> The style looks fine. I attached the patch that does some corrections in
>> code and documentation. I have corrected indentation in pl_comp.c and
>> "read_datatype" function in pl_gram.y. I think changes in "read_datatype"
>> function would be better to avoid a code duplication. But I could be wrong
>> of course.
>
>
> I merged Artur's patch and appended examples to doc.
>
>
>>
>>
>> Conclusion:
>>
>> The patch could be applied on master with documentation corrections. But
>> I'm not sure that your task could be resloved only by adding %ARRAYTYPE and
>> %ELEMENTTYPE. Maybe you will give some examples?
>
>
> It fixes the most missed/known features related to this part of plpgsql,
> what I found. But any ideas for this or follofup patches are welcome.

Patch moved to next CF, this entry is still very active.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2015-12-22 Thread Pavel Stehule
Hi

2015-12-21 16:21 GMT+01:00 Artur Zakirov :

> Hi.
> I have tried to do some review of this patch. Below are my comments.
>
> Introduction:
>
> This patch fixes and adds the following functionality:
> - %TYPE - now can be used for composite types.
> - %ARRAYTYPE - new functionality, provides the array type from a variable
> or table column.
> - %ELEMENTTYPE - new funcitonality, provides the element type of a given
> array.
>
> New regression tests are included in the patch. Changes to the
> documentation are not provided.
>
> Initial Run:
>
> The patch applies correctly to HEAD. Regression tests pass successfully,
> without errors. It seems that the patch work as you expected.
>
> Performance:
>
> It seems patch have not possible performance issues for the existing
> functionality.
>
> Coding:
>
> The style looks fine. I attached the patch that does some corrections in
> code and documentation. I have corrected indentation in pl_comp.c and
> "read_datatype" function in pl_gram.y. I think changes in "read_datatype"
> function would be better to avoid a code duplication. But I could be wrong
> of course.
>

I merged Artur's patch and appended examples to doc.



>
> Conclusion:
>
> The patch could be applied on master with documentation corrections. But
> I'm not sure that your task could be resloved only by adding %ARRAYTYPE and
> %ELEMENTTYPE. Maybe you will give some examples?
>

It fixes the most missed/known features related to this part of plpgsql,
what I found. But any ideas for this or follofup patches are welcome.

Regards

Pavel



>
> --
> Artur Zakirov
> Postgres Professional: http://www.postgrespro.com
> Russian Postgres Company
>
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
new file mode 100644
index 9786242..140c81f
*** a/doc/src/sgml/plpgsql.sgml
--- b/doc/src/sgml/plpgsql.sgml
*** SELECT merge_fields(t.*) FROM table1 t W
*** 710,715 
--- 710,792 
 

  
+   
+Array Types
+ 
+ 
+ variable%ARRAYTYPE
+ 
+ 
+
+ %ARRAYTYPE provides the array type from a variable or
+ table column. %ARRAYTYPE is particularly valuable in
+ polymorphic functions, since the data types needed for internal variables can
+ change from one call to the next.  Appropriate variables can be
+ created by applying %ARRAYTYPE to the function's
+ arguments or result placeholders:
+ 
+ CREATE OR REPLACE FUNCTION array_init(v anyelement, size integer)
+ RETURNS anyarray AS $$
+ DECLARE
+ result v%ARRAYTYPE DEFAULT '{}';
+ BEGIN
+ -- prefer builtin function array_fill
+ FOR i IN 1 .. size
+ LOOP
+ result := result || v;
+ END LOOP;
+ RETURN result;
+ END;
+ $$ LANGUAGE plpgsql;
+ 
+ SELECT array_init(0::numeric, 10);
+ SELECT array_init(''::varchar, 10);
+ 
+
+   
+ 
+   
+Array Element Types
+ 
+ 
+ variable%ELEMENTTYPE
+ 
+ 
+
+ %ELEMENTTYPE provides the element type of a given
+ array.  %ELEMENTTYPE is particularly valuable in polymorphic
+ functions, since the data types needed for internal variables can
+ change from one call to the next.  Appropriate variables can be
+ created by applying %ELEMENTTYPE to the function's
+ arguments or result placeholders:
+ 
+ CREATE OR REPLACE FUNCTION bubble_sort(a anyarray)
+ RETURNS anyarray AS $$
+ DECLARE
+ aux a%ELEMENTTYPE;
+ repeat_again boolean DEFAULT true;
+ BEGIN
+ -- Don't use this code for large arrays!
+ -- use builtin sort
+ WHILE repeat_again
+ LOOP
+ repeat_again := false;
+ FOR i IN array_lower(a, 1) .. array_upper(a, 1)
+ LOOP
+ IF a[i] > a[i+1] THEN
+ aux := a[i+1];
+ a[i+1] := a[i]; a[i] := aux;
+ repeat_again := true;
+ END IF;
+ END LOOP;
+ END LOOP;
+ RETURN a;
+ END;
+ $$ LANGUAGE plpgsql;
+ 
+
+   
+ 

 Collation of PL/pgSQL Variables
  
diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
new file mode 100644
index 1ae4bb7..c819517
*** a/src/pl/plpgsql/src/pl_comp.c
--- b/src/pl/plpgsql/src/pl_comp.c
*** plpgsql_parse_tripword(char *word1, char
*** 1617,1622 
--- 1617,1677 
  	return false;
  }
  
+ /*
+  * Derive type from ny base type controlled by reftype_mode
+  */
+ static PLpgSQL_type *
+ derive_type(PLpgSQL_type *base_type, int reftype_mode)
+ {
+ 	Oid typoid;
+ 
+ 	switch (reftype_mode)
+ 	{
+ 		case PLPGSQL_REFTYPE_TYPE:
+ 			return base_type;
+ 
+ 		case PLPGSQL_REFTYPE_ELEMENT:
+ 		{
+ 			typoid = get_element_type(base_type->typoid);
+ 			if (!OidIsValid(typoid))
+ ereport(ERROR,
+ 		(errcode(ERRCODE_DATATYPE_MISMATCH),
+ 		 errmsg("referenced variable should be an array, not type %s",
+ format_type_be(base_type->typoid;
+ 
+ 			return plpgsql_build_datatype(typoid, -1,
+ 			plpgsql_curr_compile->fn_input_collation);
+ 		}
+ 
+ 		case PLPGSQL_REFTYPE_ARRAY:
+ 		{
+ 		

Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2015-12-21 Thread Pavel Stehule
Hi



2015-12-21 16:21 GMT+01:00 Artur Zakirov :

> Hi.
> I have tried to do some review of this patch. Below are my comments.
>
> Introduction:
>
> This patch fixes and adds the following functionality:
> - %TYPE - now can be used for composite types.
> - %ARRAYTYPE - new functionality, provides the array type from a variable
> or table column.
> - %ELEMENTTYPE - new funcitonality, provides the element type of a given
> array.
>
> New regression tests are included in the patch. Changes to the
> documentation are not provided.
>
> Initial Run:
>
> The patch applies correctly to HEAD. Regression tests pass successfully,
> without errors. It seems that the patch work as you expected.
>
> Performance:
>
> It seems patch have not possible performance issues for the existing
> functionality.
>
> Coding:
>
> The style looks fine. I attached the patch that does some corrections in
> code and documentation. I have corrected indentation in pl_comp.c and
> "read_datatype" function in pl_gram.y. I think changes in "read_datatype"
> function would be better to avoid a code duplication. But I could be wrong
> of course.
>
> Conclusion:
>
> The patch could be applied on master with documentation corrections. But
> I'm not sure that your task could be resloved only by adding %ARRAYTYPE and
> %ELEMENTTYPE. Maybe you will give some examples?
>

Thank you for review. The changes in code are good idea.

I waited with documentation if there will be some objections to syntax. The
month later, there are not any known objection.

The target of this feature isn't using for storing of database objects
only, but for storing the values of polymorphic parameters.

CREATE OR REPLACE FUNCTION buble_sort(a anyarray)
RETURNS anyarray AS $$
DECLARE
  aux a%ELEMENTTYPE;
  repeat_again boolean DEFAULT true;
BEGIN
  -- Don't use this code for large arrays!
  -- use builtin sort
  WHILE repeat_again
repeat_again := false;
FOR i IN array_lower(a, 1) .. array_upper(a, 2)
LOOP
  IF a[i] > a[i+1] THEN
aux := a[i+1];
a[i+1] := a[i]; a[i] := aux;
repeat_again := true;
  END IF;
END LOOP;
  END WHILE;
  RETURN a;
END;
$$ LANGUAGE plpgsql;

CREATE OR REPLACE FUNCTION array_init(v anyelement, size integer)
RETURNS anyarray AS $$
DECLARE result v%ARRAYTYPE DEFAULT '{}';
BEGIN
  -- prefer builtin function array_fill
  FOR i IN 1 .. size
  LOOP
result := result || v;
  END LOOP;
  RETURN result;
END;
$$ LANGUAGE plpgsql;


Regards

Pavel


>
> --
> Artur Zakirov
> Postgres Professional: http://www.postgrespro.com
> Russian Postgres Company
>


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2015-12-21 Thread Artur Zakirov

Hi.
I have tried to do some review of this patch. Below are my comments.

Introduction:

This patch fixes and adds the following functionality:
- %TYPE - now can be used for composite types.
- %ARRAYTYPE - new functionality, provides the array type from a 
variable or table column.
- %ELEMENTTYPE - new funcitonality, provides the element type of a given 
array.


New regression tests are included in the patch. Changes to the 
documentation are not provided.


Initial Run:

The patch applies correctly to HEAD. Regression tests pass successfully, 
without errors. It seems that the patch work as you expected.


Performance:

It seems patch have not possible performance issues for the existing 
functionality.


Coding:

The style looks fine. I attached the patch that does some corrections in 
code and documentation. I have corrected indentation in pl_comp.c and 
"read_datatype" function in pl_gram.y. I think changes in 
"read_datatype" function would be better to avoid a code duplication. 
But I could be wrong of course.


Conclusion:

The patch could be applied on master with documentation corrections. But 
I'm not sure that your task could be resloved only by adding %ARRAYTYPE 
and %ELEMENTTYPE. Maybe you will give some examples?


--
Artur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
*** a/doc/src/sgml/plpgsql.sgml
--- b/doc/src/sgml/plpgsql.sgml
***
*** 710,715  SELECT merge_fields(t.*) FROM table1 t WHERE ... ;
--- 710,756 
 

  
+   
+Array Types
+ 
+ 
+ variable%ARRAYTYPE
+ 
+ 
+
+ %ARRAYTYPE provides the array type from a variable or
+ table column.  You can use this to declare array variables that will
+ hold database values.  To declare an array variable that will hold
+ values from users.user_id you can write:
+ 
+ user_id users.user_id%ARRAYTYPE;
+ 
+
+ 
+
+ By using %ARRAYTYPE you don't need to know the data
+ type of elements you are referencing.
+
+   
+ 
+   
+Array Element Types
+ 
+ 
+ variable%ELEMENTTYPE
+ 
+ 
+
+ %ELEMENTTYPE provides the element type of a given
+ array.  To declare a variable with the same data type as
+ users array element you can write:
+ 
+ user_id users%ELEMENTTYPE;
+ 
+
+ 
+   
+ 

 Collation of PL/pgSQL Variables
  
*** a/src/pl/plpgsql/src/pl_comp.c
--- b/src/pl/plpgsql/src/pl_comp.c
***
*** 1619,1625  plpgsql_parse_tripword(char *word1, char *word2, char *word3,
  
  /*
   * Derive type from ny base type controlled by reftype_mode
-  *
   */
  static PLpgSQL_type *
  derive_type(PLpgSQL_type *base_type, int reftype_mode)
--- 1619,1624 
***
*** 1661,1667  derive_type(PLpgSQL_type *base_type, int reftype_mode)
  ereport(ERROR,
  		(errcode(ERRCODE_DATATYPE_MISMATCH),
  		 errmsg("there are not array type for type %s",
! 	format_type_be(base_type->typoid;
  
  			return plpgsql_build_datatype(typoid, -1,
  			plpgsql_curr_compile->fn_input_collation);
--- 1660,1666 
  ereport(ERROR,
  		(errcode(ERRCODE_DATATYPE_MISMATCH),
  		 errmsg("there are not array type for type %s",
! format_type_be(base_type->typoid;
  
  			return plpgsql_build_datatype(typoid, -1,
  			plpgsql_curr_compile->fn_input_collation);
*** a/src/pl/plpgsql/src/pl_gram.y
--- b/src/pl/plpgsql/src/pl_gram.y
***
*** 2694,2700  read_datatype(int tok)
  	StringInfoData		ds;
  	char			   *type_name;
  	int	startlocation;
! 	PLpgSQL_type		*result;
  	int	parenlevel = 0;
  
  	/* Should only be called while parsing DECLARE sections */
--- 2694,2700 
  	StringInfoData		ds;
  	char			   *type_name;
  	int	startlocation;
! 	PLpgSQL_type		*result = 0;
  	int	parenlevel = 0;
  
  	/* Should only be called while parsing DECLARE sections */
***
*** 2720,2751  read_datatype(int tok)
  			tok = yylex();
  			if (tok_is_keyword(tok, ,
  			   K_TYPE, "type"))
- 			{
  result = plpgsql_parse_wordtype(dtname, PLPGSQL_REFTYPE_TYPE);
! if (result)
! 	return result;
! 			}
! 			if (tok_is_keyword(tok, ,
! 			   K_ELEMENTTYPE, "elementtype"))
! 			{
  result = plpgsql_parse_wordtype(dtname, PLPGSQL_REFTYPE_ELEMENT);
! if (result)
! 	return result;
! 			}
! 			if (tok_is_keyword(tok, ,
! 			   K_ARRAYTYPE, "arraytype"))
! 			{
  result = plpgsql_parse_wordtype(dtname, PLPGSQL_REFTYPE_ARRAY);
- if (result)
- 	return result;
- 			}
  			else if (tok_is_keyword(tok, ,
  	K_ROWTYPE, "rowtype"))
- 			{
  result = plpgsql_parse_wordrowtype(dtname);
! if (result)
! 	return result;
! 			}
  		}
  	}
  	else if (plpgsql_token_is_unreserved_keyword(tok))
--- 2720,2737 
  			tok = yylex();
  			if (tok_is_keyword(tok, ,
  			   K_TYPE, "type"))
  result = plpgsql_parse_wordtype(dtname, PLPGSQL_REFTYPE_TYPE);
! 			else if (tok_is_keyword(tok, ,
! 	K_ELEMENTTYPE, 

Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2015-12-20 Thread Jim Nasby

On 10/30/15 6:01 AM, Pavel Stehule wrote:

I am sending patch that enables to use references to polymorphic
parameters of row types. Another functionality is possibility to get
array or element type of referenced variable. It removes some gaps when
polymorphic parameters are used.


Did this make it into a commitfest?
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2015-12-20 Thread Pavel Stehule
2015-12-21 1:06 GMT+01:00 Jim Nasby :

> On 10/30/15 6:01 AM, Pavel Stehule wrote:
>
>> I am sending patch that enables to use references to polymorphic
>> parameters of row types. Another functionality is possibility to get
>> array or element type of referenced variable. It removes some gaps when
>> polymorphic parameters are used.
>>
>
> Did this make it into a commitfest?
>

yes, it is relative trivial small patch without any side effects or
possible performance issues.

The important (and possible disputable) part of this patch is new syntax

DECLARE
  var othervar%arraytype,
  var othervar%elementtype;

It is consistent with current state, and doesn't increase a complexity of
DECLARE part in plpgsql parser - what was reason for reject this idea 5
years ago (no necessary reserved keywords, ...) .

Regards

Pavel



> --
> Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
> Experts in Analytics, Data Architecture and PostgreSQL
> Data in Trouble? Get it in Treble! http://BlueTreble.com
>


Re: [HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2015-10-30 Thread Pavel Stehule
2015-10-19 9:52 GMT+02:00 Pavel Stehule :

> Hi,
>
> We cannot to declare variable with referenced type on other composite
> variable. This limit is probably artificial, because any composite type is
> any type too in PostgreSQL.
>
> The issue:
>
> referencing on composite variables doesn't work
>
> do $$ declare x int; y x%type; begin end; $$; -- ok
> do $$ declare x pg_class; y x%type; begin end; $$; -- invalid type name
> "x%type"
> do $$ declare x pg_class; y x%rowtype; begin end; $$; -- relation "x" does
> not exist
>
> The %ROWTYPE needs a record in pg_class. Probably we should not to change
> it. The change can bring a compatibility issues. So there are two
> possibilities:
>
> 1. %TYPE can be used for any kind of variables. This behave will be
> consistent with polymorphic parameters - we have "anyelement", and we have
> not "anyrow".
>
> 2. introduce new keyword - %RECTYPE .. it can work, but there will be gap
> between polymorphic parameters.
>
> Comments, notices?
>
>
Hi

I am sending patch that enables to use references to polymorphic parameters
of row types. Another functionality is possibility to get array or element
type of referenced variable. It removes some gaps when polymorphic
parameters are used.

 create type test_composite_type as (x int, y int);
create or replace function test_simple(src anyelement)
returns anyelement as $$
declare dest src%type;
begin
  dest := src;
  return dest;
end;
$$ language plpgsql;
select test_simple(10);
 test_simple
-
  10
(1 row)

select test_simple('hoj'::text);
 test_simple
-
 hoj
(1 row)

select test_simple((10,20)::test_composite_type);
 test_simple
-
 (10,20)
(1 row)

create or replace function test_poly_element(x anyelement)
returns anyarray as $$
declare result x%arraytype;
begin
  result := ARRAY[x];
  raise notice '% %', pg_typeof(result), result;
  return result;
end;
$$ language plpgsql;
select test_poly_element(1);
NOTICE:  integer[] {1}
 test_poly_element
---
 {1}
(1 row)

select test_poly_element('hoj'::text);
NOTICE:  text[] {hoj}
 test_poly_element
---
 {hoj}
(1 row)

select test_poly_element((10,20)::test_composite_type);
NOTICE:  test_composite_type[] {"(10,20)"}
 test_poly_element
---
 {"(10,20)"}
(1 row)

create or replace function test_poly_array(x anyarray)
returns anyelement as $$
declare result x%elementtype;
begin
  result := x[1];
  raise notice '% %', pg_typeof(result), result;
  return result;
end;
$$ language plpgsql;
select test_poly_array(ARRAY[1]);
NOTICE:  integer 1
 test_poly_array
-
   1
(1 row)

select test_poly_array(ARRAY['hoj'::text]);
NOTICE:  text hoj
 test_poly_array
-
 hoj
(1 row)

select test_poly_array(ARRAY[(10,20)::test_composite_type]);
NOTICE:  test_composite_type (10,20)
 test_poly_array
-
 (10,20)
(1 row)

Regards

Pavel



> Regards
>
> Pavel
>
>
>
commit 76d258edf9ef8e9645f47645a18d79f0d4245d41
Author: Pavel Stehule 
Date:   Fri Oct 30 11:48:33 2015 +0100

enhancing referenced types - possibility to get array or element type of referenced variable type.
row variables and row values are supported now too.

diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index 1ae4bb7..333d2bc 100644
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -1617,6 +1617,62 @@ plpgsql_parse_tripword(char *word1, char *word2, char *word3,
 	return false;
 }
 
+/*
+ * Derive type from ny base type controlled by reftype_mode
+ *
+ */
+static PLpgSQL_type *
+derive_type(PLpgSQL_type *base_type, int reftype_mode)
+{
+	Oid typoid;
+
+	switch (reftype_mode)
+	{
+		case PLPGSQL_REFTYPE_TYPE:
+			return base_type;
+
+		case PLPGSQL_REFTYPE_ELEMENT:
+		{
+			typoid = get_element_type(base_type->typoid);
+			if (!OidIsValid(typoid))
+ereport(ERROR,
+		(errcode(ERRCODE_DATATYPE_MISMATCH),
+		 errmsg("referenced variable should be an array, not type %s",
+format_type_be(base_type->typoid;
+
+			return plpgsql_build_datatype(typoid, -1,
+			plpgsql_curr_compile->fn_input_collation);
+		}
+
+		case PLPGSQL_REFTYPE_ARRAY:
+		{
+			/*
+			 * Question: can we allow anyelement (array or nonarray) -> array direction.
+			 * if yes, then probably we have to modify enforce_generic_type_consistency,
+			 * parse_coerce.c where still is check on scalar type -> raise error
+			 * ERROR:  42704: could not find array type for data type integer[]
+			 *
+			if (OidIsValid(get_element_type(base_type->typoid)))
+return base_type;
+			*/
+
+			typoid = get_array_type(base_type->typoid);
+			if (!OidIsValid(typoid))
+ereport(ERROR,
+		(errcode(ERRCODE_DATATYPE_MISMATCH),
+		 errmsg("there are not array type for type %s",
+	format_type_be(base_type->typoid;
+
+			return plpgsql_build_datatype(typoid, -1,
+			

[HACKERS] plpgsql - DECLARE - cannot to use %TYPE or %ROWTYPE for composite types

2015-10-19 Thread Pavel Stehule
Hi,

We cannot to declare variable with referenced type on other composite
variable. This limit is probably artificial, because any composite type is
any type too in PostgreSQL.

The issue:

referencing on composite variables doesn't work

do $$ declare x int; y x%type; begin end; $$; -- ok
do $$ declare x pg_class; y x%type; begin end; $$; -- invalid type name
"x%type"
do $$ declare x pg_class; y x%rowtype; begin end; $$; -- relation "x" does
not exist

The %ROWTYPE needs a record in pg_class. Probably we should not to change
it. The change can bring a compatibility issues. So there are two
possibilities:

1. %TYPE can be used for any kind of variables. This behave will be
consistent with polymorphic parameters - we have "anyelement", and we have
not "anyrow".

2. introduce new keyword - %RECTYPE .. it can work, but there will be gap
between polymorphic parameters.

Comments, notices?

Regards

Pavel