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

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

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