I found several issues with v4. It does not deal correctly with pipelining,
and we should only really be concerned with dropping an unnamed
portal only.
So, v5 now moves the DropPortal call after the unnamed portal was
executed to completion ( as v4 was doing ), but does so only in the
case in whi
Thanks for testing. I also tested it a bit more today with other
patterns like different fetch sizes, named portal, etc. and I can't
find an issue with this, but I could be missing something.
I will go ahead and attach this change in patch form.
--
Sami Imseih
Amazon Web Services (AWS)
v4-Corre
On 4/23/25 18:13, Sami Imseih wrote:
Also, another strange behavior of the way portal cleanup occurs is that
in extended-query-protocol and within a transaction, ExecutorEnd for the
last query is not actually called until the next command. This just seems
odd to me especially for extensions th
> Yes, I think I had misunderstood what Tom said. Thank you for pointing
> that out.
>
> However, is it really unsafe?
I have not been able to find a case where this is unsafe, but
the documentation in mmgr/README does indicate that the
query string may live shorter than the portal in some cases.
On 4/23/25 09:41, Frédéric Yhuel wrote:
On 4/22/25 19:37, Sami Imseih wrote:
the patch relies on looking up queryDesc->sourceText inside DropPortal,
which Tom raised concerns about earlier in the thread [0]
Yes, I think I had misunderstood what Tom said. Thank you for pointing
that out.
On 4/22/25 19:37, Sami Imseih wrote:
the patch relies on looking up queryDesc->sourceText inside DropPortal,
which Tom raised concerns about earlier in the thread [0]
Yes, I think I had misunderstood what Tom said. Thank you for pointing
that out.
However, is it really unsafe?
In exec_bi
> So, It seems to me we are better off just setting debug_query_string
> to NULL in DropPortal, or alternatively why not just log the statement
> automatically at the start of execution whenever we have log_temp_files > 0.
> That will allow us to safely capture the statement to blame for the
> temp
> In any case, my v3 patch seems to fix all these cases.
>
> (I'm not saying it's good enough to be committed as is. I think I should
> at least add some comments. Anything else?)
the patch relies on looking up queryDesc->sourceText inside DropPortal,
which Tom raised concerns about earlier in the
On 4/21/25 07:46, Frédéric Yhuel wrote:
On 4/20/25 00:42, Sami Imseih wrote:
(In my testing, the "temporary file:" message comes
out without any attached STATEMENT most of the time already, so this
isn't losing much as far as that's concerned.)
Indeed, this happens when using autocommit /
On 4/21/25 07:46, Frédéric Yhuel wrote:
I can try to implement Tom's idea if we have a consensus.
v3 attached. Would that do?From 07d331ba0f91b999fcefd12696bfc1eda7e8f20f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Yhuel?=
Date: Fri, 18 Apr 2025 13:20:52 +0200
Subject: [PA
On 4/20/25 00:42, Sami Imseih wrote:
(In my testing, the "temporary file:" message comes
out without any attached STATEMENT most of the time already, so this
isn't losing much as far as that's concerned.)
Indeed, this happens when using autocommit / implicit transactions.
But if you disabl
Hi,
On 20/04/2025 00:42, Sami Imseih wrote:
[...]
I'm frankly inclined to do nothing, but if we must do something,
the way to fix it here would be to transiently set debug_query_string
to NULL so that the actions of PortalDrop aren't blamed on any
particular query.
I think this is better, bec
> Sami Imseih writes:
> > I think the solution proposed by Frédéric seems reasonable: to switch
> > the debug_query_string
> > inside PortalDrop. However, I do not like the way the
> > debug_query_string changes values
> > after the CreatePortal call inside exec_bind_message; that seems incorrect.
Sami Imseih writes:
> I think the solution proposed by Frédéric seems reasonable: to switch
> the debug_query_string
> inside PortalDrop. However, I do not like the way the
> debug_query_string changes values
> after the CreatePortal call inside exec_bind_message; that seems incorrect.
> So, I bel
I looked into this a bit more.
What occurs during exec_bind_message is that the debug_query_string of the
query is set early on. Later in that routine, a new portal is created
via CreatePortal,
which also drops the existing unnamed portal from the previous
execution and which
also logs the temp fi
> [...] LOG: temporary file: path "base/pgsql_tmp/pgsql_tmp525566.0",
> size 2416640
> [..] STATEMENT: SELECT 1
> but it should be:
> [...] LOG: temporary file: path "base/pgsql_tmp/pgsql_tmp538230.0",
> size 2416640
> [...] STATEMENT: SELECT * FROM foo ORDER BY a OFFSET $1 LIMIT 2
hmm, loo
On 4/18/25 10:49, Frédéric Yhuel wrote:
Hi,
It seems there's a bug in the logging of temporary file usage when the
extended protocol is used with unnamed portals.
FWIW, the attached patch seems to fix the problem.
Best regards,
FrédéricFrom afb228f07f847c467ba05dbe204861e7be2ffc32 Mon Se
17 matches
Mail list logo
