Re: [PATCH] [LARGE] select * from cursor foo - Mailing list pgsql-hackers

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@postgresql.org)
> 

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