Re: [HACKERS] selecting from cursor
On Mon, 2 Jul 2001, Alex Pilosov wrote:
Erm, forgot to attach the patch. Here it is.
(yow) don't even bother looking at this patch. mail server delayed this
message by almost a week, and by now, the code is totally changed.
I took Tom's suggestion and made RTE a union. So, the below is a new
definition of RTE:
I have most of portal-related code working, only executor needs some more
fixes. Code properly makes PortalScan Path entry, PortalScan Plan nodes,
etc. I have added PortalReScan to tell portal it needs to rescan itself.
I'll post a correct patch next week. Thank you to everyone and especially
Tom for bearing with my often stupid questions.
--cut here--rte definition--
typedef enum RTEType {
RTE_RELATION,
RTE_SUBSELECT,
RTE_PORTAL
} RTEType;
typedef struct RangeTblEntry
{
NodeTag type;
RTEType rtetype;
/*
* Fields valid in all RTEs:
*/
Attr *alias; /* user-written alias clause, if any */
Attr *eref; /* expanded reference names */
boolinh;/* inheritance requested? */
boolinFromCl; /* present in FROM clause */
boolcheckForRead; /* check rel for read access */
boolcheckForWrite; /* check rel for write access */
Oid checkAsUser;/* if not zero, check access as this user
*/
union {
struct {
/* Fields for a plain relation RTE (rtetype=RTE_RELATION) */
char *relname;/* real name of the relation */
Oid relid; /* OID of the relation */
} rel;
struct {
/* Fields for a subquery RTE (rtetype=RTE_SUBSELECT) */
Query *subquery; /* the sub-query */
} sub;
struct {
/* fields for portal RTE (rtetype=RTE_PORTAL) */
char *portalname;/* portal's name */
} portal;
} u;
} RangeTblEntry;
---(end of broadcast)---
TIP 6: Have you searched our list archives?
http://www.postgresql.org/search.mpl
AW: [HACKERS] selecting from cursor
That's gonna have to be fixed. If you're not up for it, don't implement this. Given that cursors (are supposed to) support FETCH BACKWARDS, I really don't see why they shouldn't be expected to handle ReScan... I thought only scrollable cursors can do that. What if cursor isn't scrollable? Should it error during the execution? In PostgreSQL, all cursors are scrollable. The allowed grammar keyword is simply ignored. I am actually not sure that this is optimal, since there are a few very effective optimizations, that you can do if you know, that ReScan is not needed (like e.g. not storing the result temporarily). Andreas ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://www.postgresql.org/search.mpl
Re: [HACKERS] selecting from cursor
Alex Pilosov [EMAIL PROTECTED] writes: And what are you doing with the places that don't care which kind of RTE they are dealing with (which is most of them IIRC)? While you haven't They just have things declared as RangeTblEntry *, and as long as they don't access type-specific fields, they are fine. So you have four (soon to be six or seven) different structs that *must* have the same fields? I don't think that's cleaner than a union ... at the very least, declare it as structs containing RangeTblEntry, similar to the way the various Plan node types work (see plannodes.h). For scrollable cursors, Rescan should be implemented as 'scroll backwards until you can't scroll no more', correct? No, it should be implemented as Rescan. The portal mechanism needs to expose the Rescan call for the contained querytree. regards, tom lane ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/users-lounge/docs/faq.html
Re: AW: [HACKERS] selecting from cursor
Zeugswetter Andreas SB [EMAIL PROTECTED] writes: this. Given that cursors (are supposed to) support FETCH BACKWARDS, I really don't see why they shouldn't be expected to handle ReScan... I thought only scrollable cursors can do that. What if cursor isn't scrollable? Should it error during the execution? In PostgreSQL, all cursors are scrollable. The allowed grammar keyword is simply ignored. I am actually not sure that this is optimal, since there are a few very effective optimizations, that you can do if you know, that ReScan is not needed (like e.g. not storing the result temporarily). It's worse than that: we don't distinguish plans for cursors from plans for any other query, hence *all* query plans are supposed to be able to run backwards. (In practice, a lot of them don't work :-(.) Someday that needs to be improved. It would be good if the system understood whether a particular plan node would ever be asked to rescan itself or run backwards, and could optimize things on that basis. regards, tom lane ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [HACKERS] selecting from cursor
Alex Pilosov [EMAIL PROTECTED] writes: On Tue, 3 Jul 2001, Tom Lane wrote: So you have four (soon to be six or seven) different structs that *must* have the same fields? I don't think that's cleaner than a union ... Please see my diffs. Its implemented via #define to declare all common fields. #define RTE_COMMON_FIELDS \ NodeTag type; \ [etc] I don't think that technique is cleaner than a union, either ;-). The macro definition is a pain in the neck: you have to play games with semicolon placement, most tools won't autoindent it nicely, etc etc. But the main point is that I think NodeType = RangeTblEntry with a separate subtype field is a better way to go than making a bunch of different NodeType values. When most of the fields are common, as in this case, it's going to be true that many places only want to know is it a rangetable entry or not? regards, tom lane ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] selecting from cursor
Alex Pilosov [EMAIL PROTECTED] writes:
True true. On other hand, unlike union, its automatically typechecked, you
cannot by mistake reference a field you shouldn't be referencing.
Only true to the extent that you have cast a generic pointer to the
correct type to begin with. However, we've probably wasted more time
arguing the point than it's really worth.
I would suggest leaving off the final semicolon in the macro definition
so that you can write
typedef struct RangeTblEntryRelation
{
RTE_COMMON_FIELDS;
/* Fields valid for a plain relation RTE */
char *relname;/* real name of the relation */
Oid relid; /* OID of the relation */
Without this, tools like pgindent will almost certainly mess up these
struct declarations (I know emacs' C mode will get it wrong...)
regards, tom lane
---(end of broadcast)---
TIP 2: you can get off all lists at once with the unregister command
(send unregister YourEmailAddressHere to [EMAIL PROTECTED])
Re: [HACKERS] selecting from cursor
Alex Pilosov [EMAIL PROTECTED] writes: I'm done with change of RangeTblEntry into three different node types: RangeTblEntryRelation,RangeTblEntrySubSelect,RangeTblEntryPortal which have different fields. All the existing places instead of using rte-subquery to determine type now use IsA(rte, RangeTblEntrySubSelect), and later access fields after casting ((RangeTblEntrySubSelect *)rte)-xxx Some functions that always work on Relation RTEs are declared to accept RangeTblEntryRelation. Asserts are added everywhere before casting of RTE into specific type. (Unless there was an IsA before, then I didn't put an Assert). Let me know if that is an acceptable way of doing things, or casting makes things too ugly. (I believe its the best way, unions are more dangerous in this context). And what are you doing with the places that don't care which kind of RTE they are dealing with (which is most of them IIRC)? While you haven't shown us the proposed changes, I really suspect that a union would be cleaner, because it'd avoid ugliness in those places. Bear in mind that the three RTE types that you have are going to become five or six real soon now, because I have other things to fix that need to be done that way --- so the notational advantage of a union is going to increase. ... you cannot ReScan a portal. That's gonna have to be fixed. If you're not up for it, don't implement this. Given that cursors (are supposed to) support FETCH BACKWARDS, I really don't see why they shouldn't be expected to handle ReScan... regards, tom lane ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
