Skip to content

Commit ef6397d

Browse files
committed
Some mistakes, revealed by regression tests, passed at Raspberry PI4:
1. Fix uint64 format in some output messages. 2. Input parameters conversion mistake in aqo_queries_update. 3. Unneeded routine 'get_aqo_schema'. 4. Fix type of the first parameter (counter) in aqo_cardinality_error() and aqo_execution_time() routines. 5. Fix aqo_data() routine.
1 parent e8633ee commit ef6397d

File tree

8 files changed

+69
-83
lines changed

8 files changed

+69
-83
lines changed

aqo--1.4--1.5.sql

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ CREATE FUNCTION aqo_data (
6262
OUT features double precision[][],
6363
OUT targets double precision[],
6464
OUT reliability double precision[],
65-
OUT oids integer[]
65+
OUT oids Oid[]
6666
)
6767
RETURNS SETOF record
6868
AS 'MODULE_PATHNAME', 'aqo_data'
@@ -106,7 +106,7 @@ LANGUAGE C VOLATILE;
106106
-- nexecs - number of executions of queries associated with this ID.
107107
--
108108
CREATE OR REPLACE FUNCTION aqo_cardinality_error(controlled boolean)
109-
RETURNS TABLE(num bigint, id bigint, fshash bigint, error float, nexecs bigint)
109+
RETURNS TABLE(num integer, id bigint, fshash bigint, error double precision, nexecs bigint)
110110
AS 'MODULE_PATHNAME', 'aqo_cardinality_error'
111111
LANGUAGE C STRICT VOLATILE;
112112
COMMENT ON FUNCTION aqo_cardinality_error(boolean) IS
@@ -119,7 +119,7 @@ COMMENT ON FUNCTION aqo_cardinality_error(boolean) IS
119119
-- Last case is possible in disabled mode with aqo.force_collect_stat = 'on'.
120120
--
121121
CREATE OR REPLACE FUNCTION aqo_execution_time(controlled boolean)
122-
RETURNS TABLE(num bigint, id bigint, fshash bigint, exec_time float, nexecs bigint)
122+
RETURNS TABLE(num integer, id bigint, fshash bigint, exec_time double precision, nexecs bigint)
123123
AS 'MODULE_PATHNAME', 'aqo_execution_time'
124124
LANGUAGE C STRICT VOLATILE;
125125
COMMENT ON FUNCTION aqo_execution_time(boolean) IS

aqo.c

Lines changed: 0 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -332,52 +332,6 @@ _PG_init(void)
332332
MarkGUCPrefixReserved("aqo");
333333
}
334334

335-
/*
336-
* Return AQO schema's Oid or InvalidOid if that's not possible.
337-
*/
338-
Oid
339-
get_aqo_schema(void)
340-
{
341-
Oid result;
342-
Relation rel;
343-
SysScanDesc scandesc;
344-
HeapTuple tuple;
345-
ScanKeyData entry[1];
346-
Oid ext_oid;
347-
348-
/* It's impossible to fetch pg_aqo's schema now */
349-
if (!IsTransactionState())
350-
return InvalidOid;
351-
352-
ext_oid = get_extension_oid("aqo", true);
353-
if (ext_oid == InvalidOid)
354-
return InvalidOid; /* exit if pg_aqo does not exist */
355-
356-
ScanKeyInit(&entry[0],
357-
#if PG_VERSION_NUM >= 120000
358-
Anum_pg_extension_oid,
359-
#else
360-
ObjectIdAttributeNumber,
361-
#endif
362-
BTEqualStrategyNumber, F_OIDEQ,
363-
ObjectIdGetDatum(ext_oid));
364-
365-
rel = relation_open(ExtensionRelationId, AccessShareLock);
366-
scandesc = systable_beginscan(rel, ExtensionOidIndexId, true,
367-
NULL, 1, entry);
368-
tuple = systable_getnext(scandesc);
369-
370-
/* We assume that there can be at most one matching tuple */
371-
if (HeapTupleIsValid(tuple))
372-
result = ((Form_pg_extension) GETSTRUCT(tuple))->extnamespace;
373-
else
374-
result = InvalidOid;
375-
376-
systable_endscan(scandesc);
377-
relation_close(rel, AccessShareLock);
378-
return result;
379-
}
380-
381335
/*
382336
* AQO is really needed for any activity?
383337
*/

aqo.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,6 @@ extern double *selectivity_cache_find_global_relid(int clause_hash,
294294
int global_relid);
295295
extern void selectivity_cache_clear(void);
296296

297-
extern Oid get_aqo_schema(void);
298297
extern bool IsQueryDisabled(void);
299298

300299
extern List *cur_classes;

expected/gucs.out

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -69,17 +69,17 @@ SELECT obj_description('aqo_reset'::regproc::oid);
6969
(1 row)
7070

7171
\df aqo_cardinality_error
72-
List of functions
73-
Schema | Name | Result data type | Argument data types | Type
74-
--------+-----------------------+------------------------------------------------------------------------------------+---------------------+------
75-
public | aqo_cardinality_error | TABLE(num bigint, id bigint, fshash bigint, error double precision, nexecs bigint) | controlled boolean | func
72+
List of functions
73+
Schema | Name | Result data type | Argument data types | Type
74+
--------+-----------------------+-------------------------------------------------------------------------------------+---------------------+------
75+
public | aqo_cardinality_error | TABLE(num integer, id bigint, fshash bigint, error double precision, nexecs bigint) | controlled boolean | func
7676
(1 row)
7777

7878
\df aqo_execution_time
7979
List of functions
80-
Schema | Name | Result data type | Argument data types | Type
81-
--------+--------------------+----------------------------------------------------------------------------------------+---------------------+------
82-
public | aqo_execution_time | TABLE(num bigint, id bigint, fshash bigint, exec_time double precision, nexecs bigint) | controlled boolean | func
80+
Schema | Name | Result data type | Argument data types | Type
81+
--------+--------------------+-----------------------------------------------------------------------------------------+---------------------+------
82+
public | aqo_execution_time | TABLE(num integer, id bigint, fshash bigint, exec_time double precision, nexecs bigint) | controlled boolean | func
8383
(1 row)
8484

8585
\df aqo_drop_class

learn_cache.c

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -112,11 +112,19 @@ lc_update_fss(uint64 fs, int fss, OkNNrdata *data, List *reloids)
112112
ptr = (char *) hdr + sizeof(dsm_block_hdr); /* start point of variable data */
113113

114114
/* copy the matrix into DSM storage */
115-
for (i = 0; i < aqo_K; ++i)
115+
116+
if (hdr->cols > 0)
116117
{
117-
if (i < hdr->rows)
118+
for (i = 0; i < aqo_K; ++i)
119+
{
120+
if (i >= hdr->rows)
121+
break;
122+
123+
if (!ptr || !data->matrix[i])
124+
elog(PANIC, "Something disruptive have happened! %d, %d (%d %d)", i, hdr->rows, found, hdr->cols);
118125
memcpy(ptr, data->matrix[i], sizeof(double) * hdr->cols);
119-
ptr += sizeof(double) * data->cols;
126+
ptr += sizeof(double) * data->cols;
127+
}
120128
}
121129

122130
/* copy targets into DSM storage */
@@ -177,7 +185,7 @@ lc_load_fss(uint64 fs, int fss, OkNNrdata *data, List **reloids)
177185
Assert(fss_htab && aqo_learn_statement_timeout);
178186

179187
if (aqo_show_details)
180-
elog(NOTICE, "[AQO] Load ML data for fs %lu, fss %d from the cache",
188+
elog(NOTICE, "[AQO] Load ML data for fs "UINT64_FORMAT", fss %d from the cache",
181189
fs, fss);
182190

183191
LWLockAcquire(&aqo_state->lock, LW_SHARED);
@@ -213,6 +221,7 @@ init_with_dsm(OkNNrdata *data, dsm_block_hdr *hdr, List **reloids)
213221
Assert(LWLockHeldByMeInMode(&aqo_state->lock, LW_EXCLUSIVE) ||
214222
LWLockHeldByMeInMode(&aqo_state->lock, LW_SHARED));
215223
Assert(hdr->magic == AQO_SHARED_MAGIC);
224+
Assert(hdr && ptr);
216225

217226
data->rows = hdr->rows;
218227
data->cols = hdr->cols;
@@ -264,7 +273,7 @@ lc_flush_data(void)
264273
ptr = get_dsm_all(&size);
265274

266275
/* Iterate through records and store them into the aqo_data table */
267-
while(size > 0)
276+
while (size > 0)
268277
{
269278
dsm_block_hdr *hdr = (dsm_block_hdr *) ptr;
270279
OkNNrdata data;

path_utils.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ create_aqo_plan_node()
5353
{
5454
AQOPlanNode *node = (AQOPlanNode *) newNode(sizeof(AQOPlanNode),
5555
T_ExtensibleNode);
56-
56+
Assert(node != NULL);
5757
memcpy(node, &DefaultAQOPlanNode, sizeof(AQOPlanNode));
5858
node->rels = palloc(sizeof(RelSortOut));
5959
node->rels->hrels = NIL;
@@ -557,6 +557,7 @@ AQOnodeCopy(struct ExtensibleNode *enew, const struct ExtensibleNode *eold)
557557

558558
Assert(IsA(old, ExtensibleNode));
559559
Assert(strcmp(old->node.extnodename, AQO_PLAN_NODE) == 0);
560+
Assert(new && old);
560561

561562
/* Copy static fields in one command */
562563
memcpy(new, old, sizeof(AQOPlanNode));

postprocessing.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ should_learn(PlanState *ps, AQOPlanNode *node, aqo_obj_stat *ctx,
318318
/* This node s*/
319319
if (aqo_show_details)
320320
elog(NOTICE,
321-
"[AQO] Learn on a plan node (%lu, %d), "
321+
"[AQO] Learn on a plan node ("UINT64_FORMAT", %d), "
322322
"predicted rows: %.0lf, updated prediction: %.0lf",
323323
query_context.query_hash, node->fss, predicted, *nrows);
324324

@@ -334,7 +334,7 @@ should_learn(PlanState *ps, AQOPlanNode *node, aqo_obj_stat *ctx,
334334
if (ctx->learn && aqo_show_details &&
335335
fabs(*nrows - predicted) / predicted > 0.2)
336336
elog(NOTICE,
337-
"[AQO] Learn on a finished plan node (%lu, %d), "
337+
"[AQO] Learn on a finished plan node ("UINT64_FORMAT", %d), "
338338
"predicted rows: %.0lf, updated prediction: %.0lf",
339339
query_context.query_hash, node->fss, predicted, *nrows);
340340

@@ -845,6 +845,7 @@ StoreToQueryEnv(QueryDesc *queryDesc)
845845
enr->md.reliddesc = InvalidOid;
846846
enr->md.tupdesc = NULL;
847847
enr->reldata = palloc0(qcsize);
848+
Assert(enr->reldata != NULL);
848849
memcpy(enr->reldata, &query_context, qcsize);
849850

850851
if (newentry)
@@ -906,6 +907,7 @@ StorePlanInternals(QueryDesc *queryDesc)
906907
enr->md.reliddesc = InvalidOid;
907908
enr->md.tupdesc = NULL;
908909
enr->reldata = palloc0(sizeof(int));
910+
Assert(enr->reldata != NULL);
909911
memcpy(enr->reldata, &njoins, sizeof(int));
910912

911913
if (newentry)
@@ -935,6 +937,7 @@ ExtractFromQueryEnv(QueryDesc *queryDesc)
935937
if (enr == NULL)
936938
return false;
937939

940+
Assert(enr->reldata != NULL);
938941
memcpy(&query_context, enr->reldata, sizeof(QueryContextData));
939942

940943
return true;

0 commit comments

Comments
 (0)