With addition of Portal as RangeTblEntry, we'll have three very distinct
kinds of RTEs: Relation, Subquery and Portal.

They share some fields, but there are specific fields for each kind.

To encapsulate that (save on storage and gain cleanness), there are two
options:

1) Create three kinds of RTE, RangeTblEntryRelation, RangeTblEntryPortal
which will have these type fields. Then, code that wants to access
specific fields, will need to cast RTE to specific RTE type for access to
the fields.  Code would look like

RangeTblEntry *rte; 
Assert(e->rkind == RTE_RELATION);
(RangeTblEntryRelation rte)->relname

2) Keep one type, but unionize the fields. RTE definition would be:

typedef struct RangeTblEntry
{
    NodeTag     type;
    RTEType     rtype;

    /*
     * Fields valid in all RTEs:
     */
    Attr       *alias;          /* user-written alias clause, if any */
    Attr       *eref;           /* expanded reference names */
    bool        inh;            /* inheritance requested? */
    bool        inFromCl;       /* present in FROM clause */
    bool        checkForRead;   /* check rel for read access */
    bool        checkForWrite;  /* check rel for write access */
    Oid         checkAsUser;    /* if not zero, check access as this user
*/
 
    union {
        struct {
            /* Fields valid for a plain relation RTE (else NULL/zero): */
            char       *relname;        /* real name of the relation */
            Oid         relid;          /* OID of the relation */
        } rel;
    
        struct {
            /* Fields valid for a subquery RTE (else NULL) */
            Query      *subquery;       /* the sub-query */
        } sq;

        struct {
            /* Fields valid for function-as portal RTE */
            char       *portal;
            TupleDesc  tupleDesc;
        } func;
   } un;
}


And access would be: 

RangeTblEntry *rte; 
Assert(e->rkind == RTE_RELATION);
rte->un.rel.relname

I'm not sure which method is less ugly. I'm leaning towards 2).
But now I think that I'm always leaning in wrong direction :)


-alex


Sat, 30 Jun 2001 17:39:35 -0400 (EDT)
Alex Pilosov <alex@pilosoft.com>
RangeTblEntry modifications
Alex Pilosov <alex@pilosoft.com> writes:
> 2) Keep one type, but unionize the fields. RTE definition would be:
> I'm not sure which method is less ugly. I'm leaning towards 2).

I like that better, also, mainly because it would force code updates
everyplace that the RTE-type-specific fields are accessed; less chance
of incorrect code getting overlooked that way.

Note that some of the comments would be obsolete, eg

>             /* Fields valid for a plain relation RTE (else NULL/zero): */

They're not null/0 for a non-relation RTE, they're just not defined at
all.

>         struct {
>             /* Fields valid for function-as portal RTE */
>             char       *portal;
>             TupleDesc  tupleDesc;
>         } func;

I'm not sure I buy this, however.  You'll need an actual
RTE-as-function-call variant that has the function OID and compiled list
of arguments.  Remember that the parsetree may live a lot longer than
any specific Portal.  For a function call, we'll need to build the
Portal from the function-call-describing RTE at the start of execution,
not during parsing.

This point may also make "select from cursor" rather harder than it
appears at first glance.  You may want to back off on that part for now.

			regards, tom lane

Sat, 30 Jun 2001 18:49:19 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: RangeTblEntry modifications 
On Sat, 30 Jun 2001, Tom Lane wrote:

> Alex Pilosov <alex@pilosoft.com> writes:
> > 2) Keep one type, but unionize the fields. RTE definition would be:
> > I'm not sure which method is less ugly. I'm leaning towards 2).
> 
> I like that better, also, mainly because it would force code updates
> everyplace that the RTE-type-specific fields are accessed; less chance
> of incorrect code getting overlooked that way.

I decided to go first way, it also forces code updates in all the places
(boy there's a lot of them), but actually it forced some cleanup of code.

Instead of checking whether relid is not null or relname is not null, it
now does IsA(rte, RangeTblEntryRelation). Certain functions do take only
RangeTblEntryRelation, and thus its possible to typecheck more, etc.

In parsenodes, I have:

#define RTE_COMMON_FIELDS \
    NodeTag     type; \
    /* \
     * Fields valid in all RTEs: \
     */ \
    Attr       *alias;          /* user-written alias clause, if any */ \
    Attr       *eref;           /* expanded reference names */ \
    bool        inh;            /* inheritance requested? */ \
    bool        inFromCl;       /* present in FROM clause */ \
    bool        checkForRead;   /* check rel for read access */ \
    bool        checkForWrite;  /* check rel for write access */ \
    Oid         checkAsUser;    /* if not zero, check access as this user*/ \

typedef struct RangeTblEntry
{
    RTE_COMMON_FIELDS
} RangeTblEntry;

typedef struct RangeTblEntryRelation
{
    RTE_COMMON_FIELDS
    /* Fields valid for a plain relation RTE (else NULL/zero): */
    char       *relname;        /* real name of the relation */
    Oid         relid;          /* OID of the relation */
} RangeTblEntryRelation;

typedef struct RangeTblEntrySubSelect
{
    RTE_COMMON_FIELDS
    /* Fields valid for a subquery RTE (else NULL) */
    Query      *subquery;       /* the sub-query */
} RangeTblEntrySubSelect;


> Note that some of the comments would be obsolete, eg
> 
> >             /* Fields valid for a plain relation RTE (else NULL/zero): */
> 
> They're not null/0 for a non-relation RTE, they're just not defined at
> all.
Right...

> 
> >         struct {
> >             /* Fields valid for function-as portal RTE */
> >             char       *portal;
> >             TupleDesc  tupleDesc;
> >         } func;
> 
> I'm not sure I buy this, however.  You'll need an actual
> RTE-as-function-call variant that has the function OID and compiled list
> of arguments.  Remember that the parsetree may live a lot longer than
> any specific Portal.  For a function call, we'll need to build the
> Portal from the function-call-describing RTE at the start of execution,
> not during parsing.
Yes, this is only a first try, just trying to get parser to work straight.
I may need to add more fields.

> This point may also make "select from cursor" rather harder than it
> appears at first glance.  You may want to back off on that part for now.
It is harder than I anticipated ;)

Unfortunately (fortunately?) I decided to do 'select from cursor foo'
first. (I hope you don't mind this syntax, it forces user to explicitly
choose that he's accessing cursor). 

This will get me where I need to be, if a function returns a refcursor, I
can do following:

declare foo cursor for func()

select * from cursor foo;

Thank you so much for the help, Tom.

-alex


Sat, 30 Jun 2001 19:29:19 -0400 (EDT)
Alex Pilosov <alex@pilosoft.com>
Re: RangeTblEntry modifications 
On Sat, 30 Jun 2001, Tom Lane wrote:

> I'm not sure I buy this, however.  You'll need an actual
> RTE-as-function-call variant that has the function OID and compiled list
> of arguments.  Remember that the parsetree may live a lot longer than
> any specific Portal.  For a function call, we'll need to build the
> Portal from the function-call-describing RTE at the start of execution,
> not during parsing.

I think I just understood what you were saying: having tupleDesc in RTE is
not kosher, because RTE can last longer than a given tupleDesc?

So, essentially I will need to extract attnames/atttypes from TupleDesc
and store them separately...

Or I'm totally off my rocker?


Sat, 30 Jun 2001 20:01:54 -0400 (EDT)
Alex Pilosov <alex@pilosoft.com>
Re: RangeTblEntry modifications 
Alex Pilosov <alex@pilosoft.com> writes:
> I think I just understood what you were saying: having tupleDesc in RTE is
> not kosher, because RTE can last longer than a given tupleDesc?

Depends where you got the tupleDesc from --- if you copy it into the
parse context then it's OK in terms of not disappearing.  However,
that doesn't mean it's still *valid*.  Consider

	begin;
	declare foo cursor for select * from bar;
	create view v1 as select * from cursor foo;
	end;

Now the cursor foo is no more, but v1 still exists ... what happens
when we try to select from v1?

			regards, tom lane

Sun, 01 Jul 2001 00:47:41 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: RangeTblEntry modifications 
On Sun, 1 Jul 2001, Tom Lane wrote:

> Alex Pilosov <alex@pilosoft.com> writes:
> > I think I just understood what you were saying: having tupleDesc in RTE is
> > not kosher, because RTE can last longer than a given tupleDesc?
> 
> Depends where you got the tupleDesc from --- if you copy it into the
> parse context then it's OK in terms of not disappearing.  However,
> that doesn't mean it's still *valid*.  Consider
> 
> 	begin;
> 	declare foo cursor for select * from bar;
> 	create view v1 as select * from cursor foo;
> 	end;
> 
> Now the cursor foo is no more, but v1 still exists ... what happens
> when we try to select from v1?
I believe it will fail in the executor trying to open the portal...That's
the expected behaviour, right?

-alex 


Sun, 1 Jul 2001 08:58:28 -0400 (EDT)
Alex Pilosov <alex@pilosoft.com>
Re: RangeTblEntry modifications 