Re: [PATCH] [LARGE] select * from cursor foo

Alex, can I have an updated version of this patch for application to
7.3?

---------------------------------------------------------------------------

Alex Pilosov wrote:
> On Fri, 21 Sep 2001, Tom Lane wrote:
> >
> > I've looked this over, and I think it's not mature enough to apply at
> > this late stage of the 7.2 cycle; we'd better hold it over for more work
> > during 7.3. Major problems:
>
> > 1. Insufficient defense against queries that outlive the cursors they
> > select from. For example, I could create a view that selects from a
> > cursor. Yes, you check to see if the cursor name still exists ... but
> > what if that name now refers to a new cursor that delivers a completely
> > different set of columns? Instant coredump.
> Good point. I'll work on it.
>
> > 2. I don't understand the semantics of queries that read cursors
> > that've already had some rows fetched from them. Should we reset the
> > cursor to the start, so that all its data is implicitly available?
> > That seems strange, but if we don't do it, I think the behavior will be
> > quite unpredictable, since some join types are going to result in
> > resetting and re-reading the cursor multiple times anyway. (You've
> > punted on this issue by not implementing ExecPortalReScan, but that's
> > not acceptable for a production feature.)
> Yeah, I couldn't figure out which option is worse, which is why I didn't
> implement it. I think that rewinding the cursor on each query is better,
> but I wanted to get comments first.
>
> > 3. What does it mean to SELECT FOR UPDATE from a cursor? I don't think
> > ignoring the FOR UPDATE spec is acceptable; maybe we just have to raise
> > an error.
> OK, giving an error makes sense.
>
> > 4. Complete lack of documentation, including lack of attention to
> > updating the code's internal documentation. (For instance, you seem
> > to have changed some of the conventions described in nodes/relation.h,
> > but you didn't fix those comments.)
> OK.
>
> > The work you have done so far on changing RTE etc looks good ... but
> > I don't think the patch is ready to go. Nor am I comfortable with
> > applying it now on the assumption that the problems can be fixed during
> > beta.
>
> If you want to consider this argument: It won't break anything that's not
> using the feature. It is needed (its not a 'fringe change' to benefit few)
> (well, at least I think so :).
>
> It also is a base for my code to do 'select * from func(args)', which is
> definitely needed and base of many flames against postgres not having
> 'real' stored procedures (ones that return sets). I was hoping to get the
> rest of it in 7.2 so these flames can be put to rest.
>
> Changes to core code are obvious, and all documentation can be taken care
> of during beta.
>
> But I understand your apprehension...
>
> > I realize you originally sent this in a month ago, and perhaps you would
> > have had time to respond to these concerns if people had reviewed the
> > patch promptly. For myself, I can only apologize for not getting to it
> > sooner. I've had a few distractions over the past month :-(
> Can't blame you, completely understandable with GB situation...
>
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
> (send "unregister YourEmailAddressHere" to majordomo(at)postgresql(dot)org)
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026