Re: Problem with accessing TOAST data in stored procedures

Поиск
Список
Период
Сортировка
От Pavel Stehule
Тема Re: Problem with accessing TOAST data in stored procedures
Дата
Msg-id CAFj8pRBA3NvYPzFKFs7Y+_WNqjkd3cP+2kgmcVu4VONu6AEe8Q@mail.gmail.com
обсуждение исходный текст
Ответ на Re: Problem with accessing TOAST data in stored procedures  (Konstantin Knizhnik <k.knizhnik@postgrespro.ru>)
Ответы Re: Problem with accessing TOAST data in stored procedures  (Konstantin Knizhnik <k.knizhnik@postgrespro.ru>)
Список pgsql-hackers


st 19. 8. 2020 v 20:59 odesílatel Konstantin Knizhnik <k.knizhnik@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.knizhnik@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

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Konstantin Knizhnik
Дата:
Сообщение: Re: Problem with accessing TOAST data in stored procedures
Следующее
От: Alvaro Herrera
Дата:
Сообщение: "ccold" left by reindex concurrently are droppable?