st 19. 8. 2020 v 20:59 odesÃlatel Konstantin Knizhnik < k.knizh...@postgrespro.ru> napsal:
> > > On 19.08.2020 21:50, Pavel Stehule wrote: > > Hi > > st 19. 8. 2020 v 19:22 odesÃlatel Konstantin Knizhnik < > k.knizh...@postgrespro.ru> napsal: > >> Hi hackers, >> >> More than month ago I have sent bug report to pgsql-bugs: >> >> >> https://www.postgresql.org/message-id/flat/5d335911-fb25-60cd-4aa7-a5bd0954aea0%40postgrespro.ru >> >> with the proposed patch but have not received any response. >> >> I wonder if there is some other way to fix this issue and does somebody >> working on it. >> While the added check itself is trivial (just one line) the total patch >> is not so small because I have added walker for >> plpgsql statements tree. It is not strictly needed in this case (it is >> possible to find some other way to determine that stored procedure >> contains transaction control statements), but I hope such walker may be >> useful in other cases. >> >> In any case, I will be glad to receive any response, >> because this problem was reported by one of our customers and we need to >> provide some fix. >> It is better to include it in vanilla, rather than in our pgpro-ee fork. >> >> If it is desirable, I can add this patch to commitfest. >> > > > I don't like this design. It is not effective to repeat the walker for > every execution. Introducing a walker just for this case looks like > overengineering. > Personally I am not sure if a walker for plpgsql is a good idea (I thought > about it more times, when I wrote plpgsql_check). But anyway - there should > be good reason for introducing the walker and clean use case. > > If you want to introduce stmt walker, then it should be a separate patch > with some benefit on plpgsql environment length. > > If you think that plpgsql statement walker is not needed, then I do not > insist. > Are you going to commit your version of the patch? > I am afraid so it needs significantly much more work :(. My version is correct just for the case that you describe, but it doesn't fix the possibility of the end of the transaction inside the nested CALL. Some like DO $$ DECLARE v_r record; BEGIN FOR v_r in SELECT data FROM toasted LOOP INSERT INTO toasted(data) VALUES(v_r.data) CALL check_and_commit();END LOOP;END;$$; Probably my patch (or your patch) will fix on 99%, but still there will be a possibility of this issue. It is very similar to fixing releasing expr plans inside CALL statements. Current design of CALL statement is ugly workaround - it is slow, and there is brutal memory leak. Fixing memory leak is not hard. Fixing every time replaning (and sometimes useless) needs depper fix. Please check patch https://www.postgresql.org/message-id/attachment/112489/plpgsql-stmt_call-fix-2.patch Maybe this mechanism can be used for a clean fix of the problem mentioned in this thread. Regards Pavel
