From: | Sami Imseih <samimseih(at)gmail(dot)com> |
---|---|
To: | Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com> |
Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Lukas Fittl <lukas(at)fittl(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz> |
Subject: | Re: Proposal - Allow extensions to set a Plan Identifier |
Date: | 2025-02-13 17:10:27 |
Message-ID: | CAA5RZ0ugEq8Mu3uM2RU-iVGkFukmESWo6Xb_963rWN+AZoS3-g@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Thanks for the feedback!
> I think that makes sense and then the idea would be later on to move to something
> like 5fd9dfa5f50, but for the "planId": is my understanding correct?
correct. This is adding infrastructure to eventually have an in-core
planId; but in the
meanwhile give extensions that already compute a planId the ability to
broadcast the planId in pg_stat_activity.
> > Proposal Overview:
> >
> > 1/ Add a planId field to PgBackendStatus.
> > 2/ Add a planId field to PlannedStmt.
> > 3/ Create APIs for extensions to set the current planId.
>
> Note sure 3 is needed (MyBEEntry is available publicly and PlannedStmt also
> through some hooks). But did not look close enough and would probably depends
> of the implementation, so let's see.
I don't think direct setting of values is a good idea. We will need an API
similar to pgstat_report_query_id which ensures we are only reporting top
level planIds -and- in the case of multiple extensions with the capability
to set a planId, only the first one in the stack wins. pgstat_report_query_id
does allow for forcing a queryId ( force flag which is false by default ), and
I think this new API should allow the same.
> > Storing planId in PlannedStmt ensures it is available with
> > cached plans.
> >
> > One consideration is whether reserving a 64-bit integer in PgBackendStatus
> > and PlannedStmt is acceptable, given that planId may not always be used.
>
> From what I can see PgBackendStatus is currently 432 bytes and PlannedStmt
> is 152 bytes. It looks like that adding an uint64 at the end of those structs
> would not add extra padding (that's what "ptype /o struct <struct>" tells me)
> so would increase to 440 bytes and 160 bytes respectively. I don't think that's
> an issue.
Correct, thanks for adding this detail.
--
Sami
From | Date | Subject | |
---|---|---|---|
Next Message | Andrew Atkinson | 2025-02-13 17:13:45 | DROP CONSTRAINT, printing a notice and/or skipping when no action is taken |
Previous Message | Dmitry Koval | 2025-02-13 17:08:26 | Assignment before assert |