Re: Clarify planner_hook calling convention
On 1/3/22 8:59 PM, Tom Lane wrote: "Andrey V. Lepikhov" writes: planner hook is frequently used in monitoring and advising extensions. Yeah. The call to this hook is implemented in the way, that the standard_planner routine must be called at least once in the hook's call chain. But, as I see in [1], it should allow us "... replace the planner altogether". In such situation it haven't sense to call standard_planner at all. That's possible in theory, but who's going to do it in practice? We use it in an extension that freezes a plan for specific parameterized query (using plancache + shared storage) - exactly the same technique as extended query protocol does, but spreading across all backends. As I know, the community doesn't like such features, and we use it in enterprise code only. But, maybe more simple solution is to describe requirements to such kind of extensions in the code and documentation (See patch in attachment)? + * 2. If your extension implements some planning activity, write in the extension + * docs a requirement to set the extension at the begining of shared libraries list. This advice seems pretty unhelpful. If more than one extension is getting into the planner_hook, they can't all be first. I want to check planner_hook on startup and log an error if it isn't NULL and give a user an advice how to fix it. I want to legalize this logic, if permissible. (Also, largely the same issue applies to very many of our other hooks.) Agreed. Interference between extensions is a very annoying issue now. -- regards, Andrey Lepikhov Postgres Professional
Re: Clarify planner_hook calling convention
"Andrey V. Lepikhov" writes: > planner hook is frequently used in monitoring and advising extensions. Yeah. > The call to this hook is implemented in the way, that the > standard_planner routine must be called at least once in the hook's call > chain. > But, as I see in [1], it should allow us "... replace the planner > altogether". > In such situation it haven't sense to call standard_planner at all. That's possible in theory, but who's going to do it in practice? There is a monstrous amount of code you'd have to replace. Moreover, if you felt compelled to do it, it's likely because you are making fundamental changes elsewhere too, which means you are more likely going to end up with a fork not an extension. > But, maybe more simple solution is to describe requirements to such kind > of extensions in the code and documentation (See patch in attachment)? > + * 2. If your extension implements some planning activity, write in the > extension > + * docs a requirement to set the extension at the begining of shared > libraries list. This advice seems pretty unhelpful. If more than one extension is getting into the planner_hook, they can't all be first. (Also, largely the same issue applies to very many of our other hooks.) regards, tom lane
Clarify planner_hook calling convention
Hi,
planner hook is frequently used in monitoring and advising extensions.
The call to this hook is implemented in the way, that the
standard_planner routine must be called at least once in the hook's call
chain.
But, as I see in [1], it should allow us "... replace the planner
altogether".
In such situation it haven't sense to call standard_planner at all.
Moreover, if an extension make some expensive planning activity,
monitoring tools, like pg_stat_statements, can produce different
results, depending on a hook calling order.
I thought about additional hooks, explicit hook priorities and so on.
But, maybe more simple solution is to describe requirements to such kind
of extensions in the code and documentation (See patch in attachment)?
It would allow an extension developer legally check and log a situation,
when the extension doesn't last in the call chain.
[1]
https://www.postgresql.org/message-id/flat/27516.1180053940%40sss.pgh.pa.us
--
regards,
Andrey Lepikhov
Postgres Professional
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index afbb6c35e3..79a5602850 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -9401,6 +9401,7 @@ SET XML OPTION { DOCUMENT | CONTENT };
double quotes if you need to include whitespace or commas in the name.
This parameter can only be set at server start. If a specified
library is not found, the server will fail to start.
+Libraries are loaded in the order in which they appear in the list.
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index bd01ec0526..7251b88ad1 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -261,8 +261,11 @@ static int common_prefix_cmp(const void *a, const void *b);
* after the standard planning process. The plugin would normally call
* standard_planner().
*
- * Note to plugin authors: standard_planner() scribbles on its Query input,
- * so you'd better copy that data structure if you want to plan more than once.
+ * Notes to plugin authors:
+ * 1. standard_planner() scribbles on its Query input, so you'd better copy that
+ * data structure if you want to plan more than once.
+ * 2. If your extension implements some planning activity, write in the extension
+ * docs a requirement to set the extension at the begining of shared libraries list.
*
*/
PlannedStmt *
