Bugfix. Fix omissions related to shifting from 32-bit query hash to 64-bit hash

Author: danolivo

aqo.c

@@ -303,11 +303,13 @@ get_aqo_schema(void)
  * Init userlock
  */
 void
-init_lock_tag(LOCKTAG *tag, uint32 key1, uint32 key2)
+init_lock_tag(LOCKTAG *tag, uint64 key1, int32 key2)
 {
+	uint32 key = key1 % UINT32_MAX;
+
 	tag->locktag_field1 = AQO_MODULE_MAGIC;
-	tag->locktag_field2 = key1;
-	tag->locktag_field3 = key2;
+	tag->locktag_field2 = key;
+	tag->locktag_field3 = (uint32) key2;
 	tag->locktag_field4 = 0;
 	tag->locktag_type = LOCKTAG_USERLOCK;
 	tag->locktag_lockmethodid = USER_LOCKMETHOD;

aqo.h

@@ -280,7 +280,7 @@ int get_clause_hash(Expr *clause, int nargs, int *args_hash, int *eclass_hash);
 
 
 /* Storage interaction */
-extern bool find_query(uint64 qhash, Datum *search_values, bool *search_nulls);
+extern bool find_query(uint64 qhash, QueryContextData *ctx);
 extern bool update_query(uint64 qhash, uint64 fhash,
 						 bool learn_aqo, bool use_aqo, bool auto_tuning);
 extern bool add_query_text(uint64 query_hash, const char *query_string);
@@ -343,7 +343,7 @@ extern double *selectivity_cache_find_global_relid(int clause_hash,
 extern void selectivity_cache_clear(void);
 
 extern Oid get_aqo_schema(void);
-extern void init_lock_tag(LOCKTAG *tag, uint32 key1, uint32 key2);
+extern void init_lock_tag(LOCKTAG *tag, uint64 key1, int32 key2);
 extern bool IsQueryDisabled(void);
 
 extern List *cur_classes;

auto_tuning.c

@@ -144,7 +144,7 @@ is_in_infinite_loop_cq(double *elems, int nelems)
  * this query to false.
  */
 void
-automatical_query_tuning(uint64 query_hash, QueryStat * stat)
+automatical_query_tuning(uint64 qhash, QueryStat * stat)
 {
 	double		unstability = auto_tuning_exploration;
 	double		t_aqo,
@@ -203,11 +203,11 @@ automatical_query_tuning(uint64 query_hash, QueryStat * stat)
 		query_context.learn_aqo = query_context.use_aqo;
 	}
 	if (num_iterations <= auto_tuning_max_iterations || p_use > 0.5)
-		update_query(query_hash,
+		update_query(qhash,
 					 query_context.fspace_hash,
 					 query_context.learn_aqo,
 					 query_context.use_aqo,
 					 true);
 	else
-		update_query(query_hash, query_context.fspace_hash, false, false, false);
+		update_query(qhash, query_context.fspace_hash, false, false, false);
 }

expected/plancache.out

@@ -37,7 +37,7 @@ BEGIN
 END $$ LANGUAGE 'plpgsql';
 -- The function shows 6 executions without an AQO support (nnex) and
 -- 4 executions with usage of an AQO knowledge base (nex). Planning time in the
--- case of AQO support (pt) is equal to '-1', because the query plan is exracted
+-- case of AQO support (pt) is equal to '-1', because the query plan is extracted
 -- from the plan cache.
 SELECT * FROM f1();
  nnex | nex |      pt       

postprocessing.c

@@ -94,7 +94,7 @@ atomic_fss_learn_step(uint64 fhash, int fss_hash, int ncols,
 	LOCKTAG	tag;
 	int		nrows;
 
-	init_lock_tag(&tag, (uint32) fhash, fss_hash);
+	init_lock_tag(&tag, fhash, fss_hash);
 	LockAcquire(&tag, ExclusiveLock, false, false);
 
 	if (!load_fss(fhash, fss_hash, ncols, matrix, targets, &nrows, NULL))
@@ -671,10 +671,9 @@ aqo_ExecutorEnd(QueryDesc *queryDesc)
 			cardinality_error = cardinality_sum_errors / cardinality_num_objects;
 		else
 			cardinality_error = -1;
-		Assert(query_context.query_hash>=0);
+
 		/* Prevent concurrent updates. */
-		init_lock_tag(&tag, (uint32) query_context.query_hash,//my code
-					 (uint32) query_context.fspace_hash);//possible here
+		init_lock_tag(&tag, query_context.query_hash, query_context.fspace_hash);
 		LockAcquire(&tag, ExclusiveLock, false, false);
 
 		if (stat != NULL)
@@ -706,7 +705,6 @@ aqo_ExecutorEnd(QueryDesc *queryDesc)
 									 &stat->executions_without_aqo);
 
 			/* Store all learn data into the AQO service relations. */
-			Assert(query_context.query_hash>=0);
 			if (!query_context.adding_query && query_context.auto_tuning)
 				automatical_query_tuning(query_context.query_hash, stat);
 
@@ -970,7 +968,6 @@ print_into_explain(PlannedStmt *plannedstmt, IntoClause *into,
 	 */
 	if (aqo_mode != AQO_MODE_DISABLED || force_collect_stat)
 	{
-		Assert(query_context.query_hash>=0);
 		if (aqo_show_hash)
 			ExplainPropertyInteger("Query hash", NULL,
 									query_context.query_hash, es);

preprocessing.c

@@ -175,8 +175,6 @@ aqo_planner(Query *parse,
 			ParamListInfo boundParams)
 {
 	bool		query_is_stored = false;
-	Datum		query_params[5];
-	bool		query_nulls[5] = {false, false, false, false, false};
 	LOCKTAG		tag;
 	MemoryContext oldCxt;
 
@@ -226,7 +224,7 @@ aqo_planner(Query *parse,
 									boundParams);
 	}
 
-	elog(DEBUG1, "AQO will be used for query '%s', class %ld",
+	elog(DEBUG1, "AQO will be used for query '%s', class "UINT64_FORMAT,
 		 query_string ? query_string : "null string", query_context.query_hash);
 
 	oldCxt = MemoryContextSwitchTo(AQOMemoryContext);
@@ -240,8 +238,7 @@ aqo_planner(Query *parse,
 		goto ignore_query_settings;
 	}
 
-	query_is_stored = find_query(query_context.query_hash, &query_params[0],
-															&query_nulls[0]);
+	query_is_stored = find_query(query_context.query_hash, &query_context);
 
 	if (!query_is_stored)
 	{
@@ -295,16 +292,12 @@ aqo_planner(Query *parse,
 	else /* Query class exists in a ML knowledge base. */
 	{
 		query_context.adding_query = false;
-		query_context.learn_aqo = DatumGetBool(query_params[1]);
-		query_context.use_aqo = DatumGetBool(query_params[2]);
-		query_context.fspace_hash = DatumGetInt64(query_params[3]);
-		query_context.auto_tuning = DatumGetBool(query_params[4]);
-		query_context.collect_stat = query_context.auto_tuning;
+
+		/* Other query_context fields filled in the find_query() routine. */
 
 		/*
 		 * Deactivate query if no one reason exists for usage of an AQO machinery.
 		 */
-		Assert(query_context.query_hash>=0);
 		if (!query_context.learn_aqo && !query_context.use_aqo &&
 			!query_context.auto_tuning && !force_collect_stat)
 			add_deactivated_query(query_context.query_hash);
@@ -330,7 +323,6 @@ aqo_planner(Query *parse,
 			 * In this mode we want to learn with incoming query (if it is not
 			 * suppressed manually) and collect stats.
 			 */
-			Assert(query_context.query_hash>=0);
 			query_context.collect_stat = true;
 			query_context.fspace_hash = query_context.query_hash;
 			break;
@@ -354,15 +346,13 @@ aqo_planner(Query *parse,
 		 * find-add query and query text must be atomic operation to prevent
 		 * concurrent insertions.
 		 */
-		Assert(query_context.query_hash>=0);
-		init_lock_tag(&tag, (uint32) query_context.query_hash, (uint32) 0);//my code
+		init_lock_tag(&tag, query_context.query_hash, 0);
 		LockAcquire(&tag, ExclusiveLock, false, false);
 		/*
 		 * Add query into the AQO knowledge base. To process an error with
 		 * concurrent addition from another backend we will try to restart
 		 * preprocessing routine.
 		 */
-		Assert(query_context.query_hash>=0);
 		update_query(query_context.query_hash, query_context.fspace_hash,
 					 query_context.learn_aqo, query_context.use_aqo,
 					 query_context.auto_tuning);
@@ -371,7 +361,6 @@ aqo_planner(Query *parse,
 		 * Add query text into the ML-knowledge base. Just for further
 		 * analysis. In the case of cached plans we could have NULL query text.
 		 */
-		Assert(query_context.query_hash>=0);
 		if (query_string != NULL)
 			add_query_text(query_context.query_hash, query_string);
 
@@ -385,7 +374,6 @@ aqo_planner(Query *parse,
 		 * query execution statistics in any mode.
 		 */
 		query_context.collect_stat = true;
-		Assert(query_context.query_hash>=0);
 		query_context.fspace_hash = query_context.query_hash;
 	}
 

sql/plancache.sql

@@ -42,7 +42,7 @@ END $$ LANGUAGE 'plpgsql';
 
 -- The function shows 6 executions without an AQO support (nnex) and
 -- 4 executions with usage of an AQO knowledge base (nex). Planning time in the
--- case of AQO support (pt) is equal to '-1', because the query plan is exracted
+-- case of AQO support (pt) is equal to '-1', because the query plan is extracted
 -- from the plan cache.
 SELECT * FROM f1();
 

storage.c

@@ -93,44 +93,53 @@ open_aqo_relation(char *heaprelnspname, char *heaprelname,
  *
  * Use dirty snapshot to see all (include in-progress) data. We want to prevent
  * wait in the XactLockTableWait routine.
+ * If query is found in the knowledge base, fill the query context struct.
  */
 bool
-find_query(uint64 qhash, Datum *search_values, bool *search_nulls)
+find_query(uint64 qhash, QueryContextData *ctx)
 {
-	Relation	hrel;
-	Relation	irel;
-	HeapTuple	tuple;
+	Relation		hrel;
+	Relation		irel;
+	HeapTuple		tuple;
 	TupleTableSlot *slot;
-	bool shouldFree;
-	IndexScanDesc scan;
-	ScanKeyData key;
-	SnapshotData snap;
-	bool		find_ok = false;
+	bool			shouldFree = true;
+	IndexScanDesc	scan;
+	ScanKeyData		key;
+	SnapshotData	snap;
+	bool			find_ok = false;
+	Datum			values[5];
+	bool			nulls[5] = {false, false, false, false, false};
 
 	if (!open_aqo_relation("public", "aqo_queries", "aqo_queries_query_hash_idx",
 		AccessShareLock, &hrel, &irel))
 		return false;
 
 	InitDirtySnapshot(snap);
 	scan = index_beginscan(hrel, irel, &snap, 1, 0);
-	ScanKeyInit(&key, 1, BTEqualStrategyNumber, F_INT4EQ, Int64GetDatum(qhash));
+	ScanKeyInit(&key, 1, BTEqualStrategyNumber, F_INT8EQ, Int64GetDatum(qhash));
 
 	index_rescan(scan, &key, 1, NULL, 0);
 	slot = MakeSingleTupleTableSlot(hrel->rd_att, &TTSOpsBufferHeapTuple);
 	find_ok = index_getnext_slot(scan, ForwardScanDirection, slot);
 
-	if (find_ok && search_values != NULL)
+	if (find_ok)
 	{
 		tuple = ExecFetchSlotHeapTuple(slot, true, &shouldFree);
 		Assert(shouldFree != true);
-		heap_deform_tuple(tuple, hrel->rd_att, search_values, search_nulls);
+		heap_deform_tuple(tuple, hrel->rd_att, values, nulls);
+
+		/* Fill query context data */
+		ctx->learn_aqo = DatumGetBool(values[1]);
+		ctx->use_aqo = DatumGetBool(values[2]);
+		ctx->fspace_hash = DatumGetInt64(values[3]);
+		ctx->auto_tuning = DatumGetBool(values[4]);
+		ctx->collect_stat = query_context.auto_tuning;
 	}
 
 	ExecDropSingleTupleTableSlot(slot);
 	index_endscan(scan);
 	index_close(irel,  AccessShareLock);
 	table_close(hrel,  AccessShareLock);
-
 	return find_ok;
 }
 
@@ -176,7 +185,7 @@ update_query(uint64 qhash, uint64 fhash,
 	 */
 	InitDirtySnapshot(snap);
 	scan = index_beginscan(hrel, irel, &snap, 1, 0);
-	ScanKeyInit(&key, 1, BTEqualStrategyNumber, F_INT4EQ, Int32GetDatum(qhash));
+	ScanKeyInit(&key, 1, BTEqualStrategyNumber, F_INT8EQ, Int64GetDatum(qhash));
 
 	index_rescan(scan, &key, 1, NULL, 0);
 	slot = MakeSingleTupleTableSlot(hrel->rd_att, &TTSOpsBufferHeapTuple);
@@ -221,7 +230,8 @@ update_query(uint64 qhash, uint64 fhash,
 			 * Ooops, somebody concurrently updated the tuple. It is possible
 			 * only in the case of changes made by third-party code.
 			 */
-			elog(ERROR, "AQO feature space data for signature (%ld, %ld) concurrently"
+			elog(ERROR, "AQO feature space data for signature ("UINT64_FORMAT \
+						", "UINT64_FORMAT") concurrently"
 						" updated by a stranger backend.",
 						qhash, fhash);
 			result = false;
@@ -283,7 +293,7 @@ add_query_text(uint64 qhash, const char *query_string)
 	 */
 	InitDirtySnapshot(snap);
 	scan = index_beginscan(hrel, irel, &snap, 1, 0);
-	ScanKeyInit(&key, 1, BTEqualStrategyNumber, F_INT4EQ, Int32GetDatum(qhash));
+	ScanKeyInit(&key, 1, BTEqualStrategyNumber, F_INT8EQ, Int64GetDatum(qhash));
 
 	index_rescan(scan, &key, 1, NULL, 0);
 	slot = MakeSingleTupleTableSlot(hrel->rd_att, &TTSOpsBufferHeapTuple);
@@ -390,7 +400,7 @@ load_fss(uint64 fhash, int fss_hash,
 		return false;
 
 	scan = index_beginscan(hrel, irel, SnapshotSelf, 2, 0);
-	ScanKeyInit(&key[0], 1, BTEqualStrategyNumber, F_INT4EQ, Int32GetDatum(fhash));
+	ScanKeyInit(&key[0], 1, BTEqualStrategyNumber, F_INT8EQ, Int64GetDatum(fhash));
 	ScanKeyInit(&key[1], 2, BTEqualStrategyNumber, F_INT4EQ, Int32GetDatum(fss_hash));
 	index_rescan(scan, key, 2, NULL, 0);
 
@@ -422,9 +432,10 @@ load_fss(uint64 fhash, int fss_hash,
 				*relids = deform_oids_vector(values[5]);
 		}
 		else
-			elog(ERROR, "unexpected number of features for hash (%ld, %d):\
-						   expected %d features, obtained %d",
-						   fhash, fss_hash, ncols, DatumGetInt32(values[2]));
+			elog(ERROR, "unexpected number of features for hash (" \
+						UINT64_FORMAT", %d):\
+						expected %d features, obtained %d",
+						fhash, fss_hash, ncols, DatumGetInt32(values[2]));
 	}
 	else
 		success = false;
@@ -483,7 +494,7 @@ update_fss(uint64 fhash, int fsshash, int nrows, int ncols,
 	InitDirtySnapshot(snap);
 	scan = index_beginscan(hrel, irel, &snap, 2, 0);
 
-	ScanKeyInit(&key[0], 1, BTEqualStrategyNumber, F_INT4EQ, Int32GetDatum(fhash));
+	ScanKeyInit(&key[0], 1, BTEqualStrategyNumber, F_INT8EQ, Int64GetDatum(fhash));
 	ScanKeyInit(&key[1], 2, BTEqualStrategyNumber, F_INT4EQ, Int32GetDatum(fsshash));
 
 	index_rescan(scan, key, 2, NULL, 0);
@@ -493,7 +504,7 @@ update_fss(uint64 fhash, int fsshash, int nrows, int ncols,
 
 	if (!find_ok)
 	{
-		values[0] = Int32GetDatum(fhash);
+		values[0] = Int64GetDatum(fhash);
 		values[1] = Int32GetDatum(fsshash);
 		values[2] = Int32GetDatum(ncols);
 
@@ -548,8 +559,8 @@ update_fss(uint64 fhash, int fsshash, int nrows, int ncols,
 			 * Ooops, somebody concurrently updated the tuple. It is possible
 			 * only in the case of changes made by third-party code.
 			 */
-			elog(ERROR, "AQO data piece (%ld %d) concurrently updated"
-				 " by a stranger backend.",
+			elog(ERROR, "AQO data piece ("UINT64_FORMAT" %d) concurrently"
+				 " updated by a stranger backend.",
 				 fhash, fsshash);
 			result = false;
 		}
@@ -595,7 +606,7 @@ get_aqo_stat(uint64 qhash)
 		return false;
 
 	scan = index_beginscan(hrel, irel, SnapshotSelf, 1, 0);
-	ScanKeyInit(&key, 1, BTEqualStrategyNumber, F_INT4EQ, Int32GetDatum(qhash));
+	ScanKeyInit(&key, 1, BTEqualStrategyNumber, F_INT8EQ, Int64GetDatum(qhash));
 	index_rescan(scan, &key, 1, NULL, 0);
 	slot = MakeSingleTupleTableSlot(hrel->rd_att, &TTSOpsBufferHeapTuple);
 
@@ -666,7 +677,7 @@ update_aqo_stat(uint64 qhash, QueryStat *stat)
 
 	InitDirtySnapshot(snap);
 	scan = index_beginscan(hrel, irel, &snap, 1, 0);
-	ScanKeyInit(&key, 1, BTEqualStrategyNumber, F_INT4EQ, Int64GetDatum(qhash));
+	ScanKeyInit(&key, 1, BTEqualStrategyNumber, F_INT8EQ, Int64GetDatum(qhash));
 	index_rescan(scan, &key, 1, NULL, 0);
 	slot = MakeSingleTupleTableSlot(hrel->rd_att, &TTSOpsBufferHeapTuple);
 
@@ -712,8 +723,8 @@ update_aqo_stat(uint64 qhash, QueryStat *stat)
 			 * Ooops, somebody concurrently updated the tuple. It is possible
 			 * only in the case of changes made by third-party code.
 			 */
-			elog(ERROR, "AQO statistic data for query signature %ld concurrently"
-						" updated by a stranger backend.",
+			elog(ERROR, "AQO statistic data for query signature "UINT64_FORMAT
+						" concurrently updated by a stranger backend.",
 				 qhash);
 		}
 	}
@@ -913,8 +924,8 @@ init_deactivated_queries_storage(void)
 
 	/* Create the hashtable proper */
 	MemSet(&hash_ctl, 0, sizeof(hash_ctl));
-	hash_ctl.keysize = sizeof(int);
-	hash_ctl.entrysize = sizeof(int);
+	hash_ctl.keysize = sizeof(uint64);
+	hash_ctl.entrysize = sizeof(uint64);
 	deactivated_queries = hash_create("aqo_deactivated_queries",
 									  128,		/* start small and extend */
 									  &hash_ctl,