Thread: RangeTblEntry modifications
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
thisuser
*/ 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
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
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; /*
realname 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
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?
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
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