Re: PL/pgSQL cursors should get generated portal names by default - Mailing list pgsql-hackers
From | Jan Wieck |
---|---|
Subject | Re: PL/pgSQL cursors should get generated portal names by default |
Date | |
Msg-id | 12e8f79d-0cb3-0b4d-4ceb-c0fd620289ad@wi3ck.info Whole thread Raw |
In response to | PL/pgSQL cursors should get generated portal names by default (Tom Lane <tgl@sss.pgh.pa.us>) |
List | pgsql-hackers |
My comments were in no way meant as an argument for or against the change itself. Only to clearly document the side effect it will have. Regards, Jan On 11/7/22 11:57, Kirk Wolak wrote: > > > On Mon, Nov 7, 2022 at 11:10 AM Jan Wieck <jan@wi3ck.info > <mailto:jan@wi3ck.info>> wrote: > > On 11/4/22 19:46, Tom Lane wrote: > > Jan Wieck <jan@wi3ck.info <mailto:jan@wi3ck.info>> writes: > >> I need to do some testing on this. I seem to recall that the > naming was > >> originally done because a reference cursor is basically a named > cursor > >> that can be handed around between functions and even the top SQL > level > >> of the application. For the latter to work the application needs > to know > >> the name of the portal. > > > > Right. With this patch, it'd be necessary to hand back the actual > > portal name (by returning the refcursor value), or else manually > > set the refcursor value before OPEN to preserve the previous > behavior. > > But as far as I saw, all our documentation examples show handing back > > the portal name, so I'm hoping most people do it like that already. > > I was mostly concerned that we may unintentionally break > underdocumented > behavior that was originally implemented on purpose. As long as > everyone > is aware that this is breaking backwards compatibility in the way it > does, that's fine. > > > I respect the concern, and applied some deeper thinking to it... > > Here is the logic I am applying to this compatibility issue and what may > break. > [FWIW, my motto is to be wrong out loud, as you learn faster] > > At first pass, I thought "Well, since this does not break a refcursor, > which is the obvious use case for RETURNING/PASSING, we are fine!" > > But in trying to DEFEND this case, I have come up with example of code > (that makes some SENSE, but would break): > > CREATE FUNCTION test() RETURNS refcursor() LANGUAGE plpgsql AS $$ > DECLARE > cur_this cursor FOR SELECT 1; > ref_cur refcursor; > BEGIN > OPEN cur_this; > ref_cur := 'cur_this'; -- Using the NAME of the cursor as the > portal name: Should do: ref_cur := cur_this; -- Only works after OPEN > RETURN ref_cur; > END; > $$; > > As noted in the comments. If the code were: > ref_cur := 'cur_this'; -- Now you can't just use ref_cur := cur_this; > OPEN cur_this; > RETURN ref_cur; > Then it would break now... And even the CORRECT syntax would break, > since the cursor was not opened, so "cur_this" is null. > > Now, I have NO IDEA if someone would actually do this. It is almost > pathological. The use case would be a complex cursor with parameters, > and they changed the code to return a refcursor! > This was the ONLY use case I could think of that wasn't HACKY! > > HACKY use cases involve a child routine setting: local_ref_cursor := > 'cur_this'; in order to access a cursor that was NOT passed to the child. > FWIW, I tested this, and it works, and I can FETCH in the child routine, > and it affects the parents' LOOP as it should... WOW. I would be HAPPY > to break such horrible code, it has to be a security concern at some level. > > Personally (and my 2 cents really shouldn't matter much), I think this > should still be fixed. > Because I believe this small use case is rare, it will break > immediately, and the fix is trivial (just initialize cur_this := > 'cur_this' in this example), > and the fix removes the Orthogonal Behavior Tom pointed out, which led > me to reporting this. > > I think I have exhausted examples of how this impacts a VALID > refcursor implementation. I believe any other such versions are > variations of this! > And maybe we document that if a refcursor of a cursor is to be returned, > that the refcursor is ASSIGNED after the OPEN of the cursor, and it is > done without the quotes, as: > ref_cursor := cur_this; -- assign the name after opening. > > Thanks! > >
pgsql-hackers by date: