Bugfix. Switch off quickly all AQO features if queryId is disabled.

One installcheck test was added into the github actions workflow.

Bugfix. Do away with conflict between two extern hooks in sr_plan an AQO with
the same name.

It is may be not enough. Should we make all hooks as static?

Draft

Author: danolivo

.github/workflows/installchecks.yml

@@ -132,12 +132,24 @@ jobs:
         psql -c "ALTER SYSTEM SET aqo.mode = 'learn'"
         psql -c "ALTER SYSTEM SET aqo.force_collect_stat = 'on'"
         psql -c "SELECT pg_reload_conf()"
-        learn_result=$(make -k installcheck-world)
+        make -k installcheck-world
+
+    # Should work like a total off for all the AQO features
+    - name: installcheck_learn_queryid_off
+      continue-on-error: true
+      run: |
+        cd $PG_DIR
+        aqo_instance_launch.sh
+        psql -c "ALTER SYSTEM SET compute_query_id = 'off'"
+        psql -c "SELECT pg_reload_conf()"
+        # The AQO tests itself wouldn't pass
+        make -k installcheck-world
 
     - name: installcheck_intelligent
       continue-on-error: true
       run: |
         cd $PG_DIR
+        psql -c "ALTER SYSTEM SET compute_query_id = 'regress'"
         psql -c "ALTER SYSTEM SET aqo.mode = 'intelligent'"
         psql -c "ALTER SYSTEM SET aqo.force_collect_stat = 'on'"
         psql -c "SELECT pg_reload_conf()"

aqo.c

@@ -19,9 +19,7 @@
 
 #include "aqo.h"
 #include "aqo_shared.h"
-#include "cardinality_hooks.h"
 #include "path_utils.h"
-#include "preprocessing.h"
 #include "storage.h"
 
 
@@ -98,21 +96,6 @@ MemoryContext 		AQOLearnMemCtx = NULL;
 /* Additional plan info */
 int njoins;
 
-/* Saved hook values */
-post_parse_analyze_hook_type				prev_post_parse_analyze_hook;
-planner_hook_type							prev_planner_hook;
-ExecutorStart_hook_type						prev_ExecutorStart_hook;
-ExecutorRun_hook_type						prev_ExecutorRun;
-ExecutorEnd_hook_type						prev_ExecutorEnd_hook;
-set_baserel_rows_estimate_hook_type			prev_set_foreign_rows_estimate_hook;
-set_baserel_rows_estimate_hook_type			prev_set_baserel_rows_estimate_hook;
-get_parameterized_baserel_size_hook_type	prev_get_parameterized_baserel_size_hook;
-set_joinrel_size_estimates_hook_type		prev_set_joinrel_size_estimates_hook;
-get_parameterized_joinrel_size_hook_type	prev_get_parameterized_joinrel_size_hook;
-ExplainOnePlan_hook_type					prev_ExplainOnePlan_hook;
-ExplainOneNode_hook_type					prev_ExplainOneNode_hook;
-static shmem_request_hook_type				prev_shmem_request_hook = NULL;
-
 /*****************************************************************************
  *
  *	CREATE/DROP EXTENSION FUNCTIONS
@@ -135,18 +118,6 @@ aqo_free_callback(ResourceReleasePhase phase,
 	}
 }
 
-/*
- * Requests any additional shared memory required for aqo.
- */
-static void
-aqo_shmem_request(void)
-{
-	if (prev_shmem_request_hook)
-		prev_shmem_request_hook();
-
-	RequestAddinShmemSpace(aqo_memsize());
-}
-
 void
 _PG_init(void)
 {
@@ -343,45 +314,11 @@ _PG_init(void)
 							 NULL,
 							 NULL);
 
-	prev_shmem_startup_hook						= shmem_startup_hook;
-	shmem_startup_hook							= aqo_init_shmem;
-	prev_planner_hook							= planner_hook;
-	planner_hook								= aqo_planner;
-	prev_ExecutorStart_hook						= ExecutorStart_hook;
-	ExecutorStart_hook							= aqo_ExecutorStart;
-	prev_ExecutorRun							= ExecutorRun_hook;
-	ExecutorRun_hook							= aqo_ExecutorRun;
-	prev_ExecutorEnd_hook						= ExecutorEnd_hook;
-	ExecutorEnd_hook							= aqo_ExecutorEnd;
-
-	/* Cardinality prediction hooks. */
-	prev_set_baserel_rows_estimate_hook			= set_baserel_rows_estimate_hook;
-	set_foreign_rows_estimate_hook				= aqo_set_baserel_rows_estimate;
-	set_baserel_rows_estimate_hook				= aqo_set_baserel_rows_estimate;
-	prev_get_parameterized_baserel_size_hook	= get_parameterized_baserel_size_hook;
-	get_parameterized_baserel_size_hook			= aqo_get_parameterized_baserel_size;
-	prev_set_joinrel_size_estimates_hook		= set_joinrel_size_estimates_hook;
-	set_joinrel_size_estimates_hook				= aqo_set_joinrel_size_estimates;
-	prev_get_parameterized_joinrel_size_hook	= get_parameterized_joinrel_size_hook;
-	get_parameterized_joinrel_size_hook			= aqo_get_parameterized_joinrel_size;
-	prev_estimate_num_groups_hook				= estimate_num_groups_hook;
-	estimate_num_groups_hook					= aqo_estimate_num_groups_hook;
-	parampathinfo_postinit_hook					= ppi_hook;
-
-	prev_create_plan_hook						= create_plan_hook;
-	create_plan_hook							= aqo_create_plan_hook;
-
-	/* Service hooks. */
-	prev_ExplainOnePlan_hook					= ExplainOnePlan_hook;
-	ExplainOnePlan_hook							= print_into_explain;
-	prev_ExplainOneNode_hook					= ExplainOneNode_hook;
-	ExplainOneNode_hook							= print_node_explain;
-
-	prev_create_upper_paths_hook				= create_upper_paths_hook;
-	create_upper_paths_hook						= aqo_store_upper_signature_hook;
-
-	prev_shmem_request_hook = shmem_request_hook;
-	shmem_request_hook = aqo_shmem_request;
+	aqo_shmem_init();
+	aqo_preprocessing_init();
+	aqo_postprocessing_init();
+	aqo_cardinality_hooks_init();
+	aqo_path_utils_init();
 
 	init_deactivated_queries_storage();
 

aqo.h

@@ -132,19 +132,16 @@
 #include "nodes/nodeFuncs.h"
 #include "optimizer/pathnode.h"
 #include "optimizer/planner.h"
-#include "optimizer/cost.h"
 #include "parser/analyze.h"
 #include "parser/parsetree.h"
 #include "utils/builtins.h"
 #include "utils/guc.h"
 #include "utils/hsearch.h"
 #include "utils/memutils.h"
 #include "utils/rel.h"
-#include "utils/fmgroids.h"
 #include "utils/snapmgr.h"
 
 #include "machine_learning.h"
-//#include "storage.h"
 
 /* Check PostgreSQL version (9.6.0 contains important changes in planner) */
 #if PG_VERSION_NUM < 90600
@@ -237,58 +234,15 @@ extern MemoryContext AQOCacheMemCtx;
 extern MemoryContext AQOPredictMemCtx;
 extern MemoryContext AQOLearnMemCtx;
 
-/* Saved hook values in case of unload */
-extern post_parse_analyze_hook_type prev_post_parse_analyze_hook;
-extern planner_hook_type prev_planner_hook;
-extern ExecutorStart_hook_type prev_ExecutorStart_hook;
-extern ExecutorRun_hook_type prev_ExecutorRun;
-extern ExecutorEnd_hook_type prev_ExecutorEnd_hook;
-extern set_baserel_rows_estimate_hook_type
-										prev_set_foreign_rows_estimate_hook;
-extern set_baserel_rows_estimate_hook_type
-										prev_set_baserel_rows_estimate_hook;
-extern get_parameterized_baserel_size_hook_type
-									prev_get_parameterized_baserel_size_hook;
-extern set_joinrel_size_estimates_hook_type
-										prev_set_joinrel_size_estimates_hook;
-extern get_parameterized_joinrel_size_hook_type
-									prev_get_parameterized_joinrel_size_hook;
-extern ExplainOnePlan_hook_type prev_ExplainOnePlan_hook;
-extern ExplainOneNode_hook_type prev_ExplainOneNode_hook;
-
-extern void ppi_hook(ParamPathInfo *ppi);
 extern int aqo_statement_timeout;
 
-/* Hash functions */
-void get_eclasses(List *clauselist, int *nargs, int **args_hash,
-				  int **eclass_hash);
-int get_clause_hash(Expr *clause, int nargs, int *args_hash, int *eclass_hash);
-
-
-/* Storage interaction */
-extern bool load_fss_ext(uint64 fs, int fss, OkNNrdata *data, List **reloids);
-extern bool update_fss_ext(uint64 fs, int fss, OkNNrdata *data, List *reloids);
-
-/* Query preprocessing hooks */
-extern void print_into_explain(PlannedStmt *plannedstmt, IntoClause *into,
-							   ExplainState *es, const char *queryString,
-							   ParamListInfo params,
-							   const instr_time *planduration,
-							   QueryEnvironment *queryEnv);
-extern void print_node_explain(ExplainState *es, PlanState *ps, Plan *plan);
-
 /* Cardinality estimation */
 extern double predict_for_relation(List *restrict_clauses, List *selectivities,
 								   List *relsigns, int *fss);
 
-/* Query execution statistics collecting hooks */
-void aqo_ExecutorStart(QueryDesc *queryDesc, int eflags);
-void aqo_ExecutorRun(QueryDesc *queryDesc, ScanDirection direction,
-					 uint64 count, bool execute_once);
-void aqo_ExecutorEnd(QueryDesc *queryDesc);
-
 /* Automatic query tuning */
 extern void automatical_query_tuning(uint64 query_hash, struct StatEntry *stat);
+extern double get_mean(double *elems, int nelems);
 
 /* Utilities */
 extern int int_cmp(const void *a, const void *b);
@@ -306,8 +260,10 @@ extern void selectivity_cache_clear(void);
 
 extern bool IsQueryDisabled(void);
 
-extern bool update_query_timeout(uint64 queryid, int64 smart_timeout);
-extern double get_mean(double *elems, int nelems);
-
 extern List *cur_classes;
+
+extern void aqo_cardinality_hooks_init(void);
+extern void aqo_preprocessing_init(void);
+extern void aqo_postprocessing_init(void);
+
 #endif

aqo_shared.c

@@ -6,27 +6,30 @@
 
 #include "lib/dshash.h"
 #include "miscadmin.h"
+#include "storage/ipc.h"
 #include "storage/shmem.h"
 
 #include "aqo_shared.h"
 #include "storage.h"
 
 
-shmem_startup_hook_type prev_shmem_startup_hook = NULL;
 AQOSharedState *aqo_state = NULL;
 int fs_max_items = 10000; /* Max number of different feature spaces in ML model */
 int fss_max_items = 100000; /* Max number of different feature subspaces in ML model */
 
+static shmem_startup_hook_type	aqo_shmem_startup_next = NULL;
+static shmem_request_hook_type	aqo_shmem_request_next = NULL;
+
 static void on_shmem_shutdown(int code, Datum arg);
 
-void
+static void
 aqo_init_shmem(void)
 {
 	bool		found;
 	HASHCTL		info;
 
-	if (prev_shmem_startup_hook)
-		prev_shmem_startup_hook();
+	if (aqo_shmem_startup_next)
+		aqo_shmem_startup_next();
 
 	aqo_state = NULL;
 	stat_htab = NULL;
@@ -116,10 +119,17 @@ on_shmem_shutdown(int code, Datum arg)
 	return;
 }
 
-Size
-aqo_memsize(void)
+
+/*
+ * Requests any additional shared memory required for aqo.
+ */
+static void
+aqo_shmem_request(void)
 {
-	Size		size;
+	Size	size;
+
+	if (aqo_shmem_request_next)
+		aqo_shmem_request_next();
 
 	size = MAXALIGN(sizeof(AQOSharedState));
 	size = add_size(size, hash_estimate_size(fs_max_items, sizeof(AQOSharedState)));
@@ -128,5 +138,14 @@ aqo_memsize(void)
 	size = add_size(size, hash_estimate_size(fss_max_items, sizeof(DataEntry)));
 	size = add_size(size, hash_estimate_size(fs_max_items, sizeof(QueriesEntry)));
 
-	return size;
+	RequestAddinShmemSpace(size);
+}
+
+void
+aqo_shmem_init(void)
+{
+	aqo_shmem_startup_next	= shmem_startup_hook;
+	shmem_startup_hook		= aqo_init_shmem;
+	aqo_shmem_request_next	= shmem_request_hook;
+	shmem_request_hook		= aqo_shmem_request;
 }

aqo_shared.h

@@ -1,9 +1,6 @@
 #ifndef AQO_SHARED_H
 #define AQO_SHARED_H
 
-#include "lib/dshash.h"
-#include "storage/dsm.h"
-#include "storage/ipc.h"
 #include "storage/lwlock.h"
 #include "utils/dsa.h"
 
@@ -31,13 +28,11 @@ typedef struct AQOSharedState
 } AQOSharedState;
 
 
-extern shmem_startup_hook_type prev_shmem_startup_hook;
 extern AQOSharedState *aqo_state;
 
 extern int fs_max_items; /* Max number of feature spaces that AQO can operate */
 extern int fss_max_items;
 
-extern Size aqo_memsize(void);
-extern void aqo_init_shmem(void);
+extern void aqo_shmem_init(void);
 
 #endif /* AQO_SHARED_H */