Feedback-ID: id4a34324:Fastmail
Date: Mon, 25 Mar 2024 19:34:47 -0700
From: Andres Freund <andres@anarazel.de>
To: Robert Haas <robertmhaas@gmail.com>
Cc: torikoshia <torikoshia@oss.nttdata.com>,
	pgsql-hackers@lists.postgresql.org,
	James Coleman <jtc331@gmail.com>,
	Julien Rouhaud <rjuju123@gmail.com>,
	=?utf-8?Q?=C3=89tienne?= BERSAC <etienne.bersac@dalibo.com>,
	ashutosh.bapat.oss@gmail.com, rafaelthca@gmail.com,
	jian.universality@gmail.com
Subject: Re: RFC: Logging plan of the running query
Message-ID: <20240326023447.yj5fi5qcbz5ocyrt@awork3.anarazel.de>
References: <CAAaqYe-gMkdL=M4v47=H0F3+-zi2qL9zFqAv3QsizkRjFiQR0w@mail.gmail.com>
 <CA+TgmoYkztFSryutQMNv9DnQ__NDwGE8XhhgnbdCG8e_o4T7yQ@mail.gmail.com>
 <hzicr3kidnmwbcqwmwortldzq4fijrfca7c3prncoepgt6rpwf@uydrxvi7qhpk>
 <CA+Tgmoa6QniTueRJ1rKcp_-TpVc0c8FZxOtu4ehDop+R=pEcpA@mail.gmail.com>
 <2c9a3fd390d653f1829b795d0bad4653@oss.nttdata.com>
 <CA+TgmoYC3_KG8i1WfwGKK6+3YAS7Z0wP0gaTUC4ggEQ=1DrEEg@mail.gmail.com>
 <CAAaqYe-7QiAufDmfExAOZKcZTOR5dkuVSMxXRPEooaCaPfFqtA@mail.gmail.com>
 <CA+TgmoZVYG+Ot=yPg3HAr2H=2fPvQOyahrx5qwaRf5cqkM7PAQ@mail.gmail.com>
 <ac6c51071316279bf903078cf264c37a@oss.nttdata.com>
 <CA+TgmoaaEQtuope6za=3GSCZ+WJFT4DbF5Cnv0QXFmDnZ_PqFw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CA+TgmoaaEQtuope6za=3GSCZ+WJFT4DbF5Cnv0QXFmDnZ_PqFw@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/20240326023447.yj5fi5qcbz5ocyrt%40awork3.anarazel.de>
Precedence: bulk

Hi,

On 2024-03-13 15:33:02 -0400, Robert Haas wrote:
> But also ... having to wrap the entire plan tree like this seems
> pretty awful. I don't really like the idea of a large-scan plan
> modification like this in the middle of the query.

It's not great. But I also don't really see an alternative with this approach.

I guess we could invent a new CFI version that gets the current PlanState and
use that in all of src/backend/executor/node* and pass the PlanState to that -
but then we could just as well just directly process the interrupt there.


> I also wonder whether it interacts properly with JIT.

I don't think there's a problem unless somebody invests a lot of time in
JITing much more of the query. Which will require a lot more work, basically
redesigning the executor...


> Andres, did you have some clever idea for this feature that would
> avoid the need to do this?

No. I think it's acceptable though.

However it might be worth inventing an executor tree walker in a preliminary
step. We have already quite a few switches over all plan nodes, which we could
largely replace with a helper.

Greetings,

Andres Freund