Skip to content

Commit 1e2a8a5

Browse files
mkindahlmkindahl
committed
Error out on bad args when processing invalidation
When calling `_timescaledb_functions.process_hypertable_invalidations` either directly or indirectly, it is necessary to check that the array contain only hypertables and error out on any elements that are not hypertables.
1 parent 16d8b98 commit 1e2a8a5

File tree

6 files changed

+124
-7
lines changed

6 files changed

+124
-7
lines changed

‎.unreleased/pr_8558‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fixes: #8558 Error out on bad args when processing invalidation

‎src/utils.c‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@
4545
#include "chunk.h"
4646
#include "cross_module_fn.h"
4747
#include "debug_point.h"
48-
#include "guc.h"
4948
#include "hypertable_cache.h"
5049
#include "jsonb_utils.h"
5150
#include "time_utils.h"
@@ -2026,3 +2025,26 @@ ts_get_attr_expr(Relation rel, AttrNumber attno)
20262025

20272026
return expr;
20282027
}
2028+
2029+
char *
2030+
ts_list_to_string(List *list, append_cell_func append)
2031+
{
2032+
StringInfoData info;
2033+
ListCell *lc;
2034+
2035+
initStringInfo(&info);
2036+
2037+
foreach (lc, list)
2038+
{
2039+
if (!lnext(list, lc))
2040+
appendStringInfoString(&info, "and ");
2041+
append(&info, lc);
2042+
if (lnext(list, lc))
2043+
{
2044+
if (list_length(list) > 2)
2045+
appendStringInfoChar(&info, ',');
2046+
appendStringInfoChar(&info, ' ');
2047+
}
2048+
}
2049+
return info.data;
2050+
}

‎src/utils.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -406,10 +406,13 @@ ts_datum_set_objectid(const AttrNumber attno, NullableDatum *datums, const Oid v
406406
datums[AttrNumberGetAttrOffset(attno)].isnull = true;
407407
}
408408

409+
typedef void (*append_cell_func)(StringInfo, ListCell *);
410+
409411
extern TSDLLEXPORT void ts_get_rel_info_by_name(const char *relnamespace, const char *relname,
410412
Oid *relid, Oid *amoid, char *relkind);
411413
extern TSDLLEXPORT void ts_get_rel_info(Oid relid, Oid *amoid, char *relkind);
412414
extern TSDLLEXPORT Oid ts_get_rel_am(Oid relid);
413415
extern TSDLLEXPORT void ts_relation_set_reloption(Relation rel, List *options, LOCKMODE lockmode);
414416
extern TSDLLEXPORT Jsonb *ts_errdata_to_jsonb(ErrorData *edata, Name proc_schema, Name proc_name);
415417
extern TSDLLEXPORT char *ts_get_attr_expr(Relation rel, AttrNumber attno);
418+
extern TSDLLEXPORT char *ts_list_to_string(List *list, append_cell_func append);

‎tsl/src/continuous_aggs/invalidation.c‎

Lines changed: 56 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include <utils/snapmgr.h>
3030
#include <utils/tuplestore.h>
3131

32+
#include "cache.h"
3233
#include "continuous_aggs/invalidation_multi.h"
3334
#include "continuous_aggs/invalidation_threshold.h"
3435
#include "continuous_aggs/materialize.h"
@@ -1241,6 +1242,17 @@ continuous_agg_process_hypertable_invalidations_single(Oid hypertable_relid)
12411242
invalidation_process_hypertable_log(hypertable_id, dimtype);
12421243
}
12431244

1245+
static void
1246+
append_string_info_relid(StringInfo info, ListCell *lc)
1247+
{
1248+
Oid relid = lfirst_oid(lc);
1249+
const char *name = get_rel_name(relid);
1250+
if (name)
1251+
appendStringInfo(info, "\"%s\"", name);
1252+
else
1253+
appendStringInfo(info, "%d", relid);
1254+
}
1255+
12441256
/*
12451257
* PostgreSQL function to move hypertable invalidations to materialization
12461258
* invalidation log.
@@ -1250,21 +1262,59 @@ continuous_agg_process_hypertable_invalidations_multi(ArrayType *hypertable_arra
12501262
{
12511263
ArrayIterator array_iterator = array_create_iterator(hypertable_array, 0, NULL);
12521264
List *hypertables = NIL;
1265+
List *bad_objects = NIL;
12531266

12541267
Datum value;
12551268
bool isnull;
12561269
while (array_iterate(array_iterator, &value, &isnull))
12571270
{
1271+
/* Function signature only allow OIDs, so we will always have an OID. */
12581272
Oid hypertable_relid = DatumGetObjectId(value);
1259-
int32 hypertable_id = ts_hypertable_relid_to_id(hypertable_relid);
1260-
TS_DEBUG_LOG("add relation \"%s\" to list: hypertable_id=%d",
1261-
get_rel_name(hypertable_relid),
1262-
hypertable_id);
1263-
ts_hypertable_permissions_check(hypertable_relid, GetUserId());
1264-
hypertables = lappend_int(hypertables, hypertable_id);
1273+
Cache *hcache;
1274+
Hypertable *ht = ts_hypertable_cache_get_cache_and_entry(hypertable_relid,
1275+
CACHE_FLAG_MISSING_OK,
1276+
&hcache);
1277+
if (ht)
1278+
{
1279+
int32 hypertable_id = ht->fd.id;
1280+
TS_DEBUG_LOG("add relation \"%s\" to list: hypertable_id=%d",
1281+
get_rel_name(hypertable_relid),
1282+
hypertable_id);
1283+
ts_hypertable_permissions_check(hypertable_relid, GetUserId());
1284+
hypertables = lappend_int(hypertables, hypertable_id);
1285+
}
1286+
else
1287+
{
1288+
TS_DEBUG_LOG("relation \"%s\" is not a hypertable", get_rel_name(hypertable_relid));
1289+
bad_objects = lappend_oid(bad_objects, hypertable_relid);
1290+
}
1291+
ts_cache_release(&hcache);
12651292
}
12661293

12671294
array_free_iterator(array_iterator);
12681295

1296+
const int bad_count = list_length(bad_objects);
1297+
if (bad_count == 1)
1298+
{
1299+
const Oid relid = linitial_oid(bad_objects);
1300+
const char *name = get_rel_name(relid);
1301+
if (name)
1302+
ereport(ERROR,
1303+
errcode(ERRCODE_INVALID_PARAMETER_VALUE),
1304+
errmsg("table \"%s\" is not a hypertable", name));
1305+
else
1306+
ereport(ERROR,
1307+
errcode(ERRCODE_INVALID_PARAMETER_VALUE),
1308+
errmsg("OID %d is not a hypertable", relid));
1309+
}
1310+
else if (bad_count > 1)
1311+
{
1312+
ereport(ERROR,
1313+
errcode(ERRCODE_INVALID_PARAMETER_VALUE),
1314+
errmsg("%d objects in list are not hypertables", bad_count),
1315+
errdetail("Bad objects are %s.",
1316+
ts_list_to_string(bad_objects, append_string_info_relid)));
1317+
}
1318+
12691319
multi_invalidation_process_hypertable_log(hypertables);
12701320
}

‎tsl/test/expected/cagg_invalidation_multi.out‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,3 +231,28 @@ GROUP BY 1 ORDER BY 1,2;
231231
cond_20 | [0,120)
232232
(2 rows)
233233

234+
-- This should be fine but not process any tables. Odd usage, but
235+
-- nothing wrong with it.
236+
CALL _timescaledb_functions.process_hypertable_invalidations(ARRAY[]::regclass[]);
237+
-- These should error out
238+
\set ON_ERROR_STOP 0
239+
\set VERBOSITY default
240+
CALL _timescaledb_functions.process_hypertable_invalidations(ARRAY['cond_10']);
241+
ERROR: table "cond_10" is not a hypertable
242+
CALL _timescaledb_functions.process_hypertable_invalidations(ARRAY[0]);
243+
ERROR: OID 0 is not a hypertable
244+
CALL _timescaledb_functions.process_hypertable_invalidations(ARRAY['cond_10', 'cond_20']);
245+
ERROR: 2 objects in list are not hypertables
246+
DETAIL: Bad objects are "cond_10" and "cond_20".
247+
CALL _timescaledb_functions.process_hypertable_invalidations(ARRAY['conditions', 'cond_20']);
248+
ERROR: table "cond_20" is not a hypertable
249+
CALL _timescaledb_functions.process_hypertable_invalidations(ARRAY['conditions', 123456]::regclass[]);
250+
ERROR: OID 123456 is not a hypertable
251+
CALL _timescaledb_functions.process_hypertable_invalidations(ARRAY['conditions', 123456, 'cond_10']::regclass[]);
252+
ERROR: 2 objects in list are not hypertables
253+
DETAIL: Bad objects are 123456 and "cond_10".
254+
CALL _timescaledb_functions.process_hypertable_invalidations(ARRAY['conditions', 123456, 'cond_10', 'cond_20']::regclass[]);
255+
ERROR: 3 objects in list are not hypertables
256+
DETAIL: Bad objects are 123456, "cond_10", and "cond_20".
257+
\set VERBOSITY terse
258+
\set ON_ERROR_STOP 1

‎tsl/test/sql/cagg_invalidation_multi.sql‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,3 +176,19 @@ SELECT m.aggregate_name,
176176
WHERE m.table_name IS NULL OR s.table_name IS NULL
177177
GROUP BY 1 ORDER BY 1,2;
178178

179+
-- This should be fine but not process any tables. Odd usage, but
180+
-- nothing wrong with it.
181+
CALL _timescaledb_functions.process_hypertable_invalidations(ARRAY[]::regclass[]);
182+
183+
-- These should error out
184+
\set ON_ERROR_STOP 0
185+
\set VERBOSITY default
186+
CALL _timescaledb_functions.process_hypertable_invalidations(ARRAY['cond_10']);
187+
CALL _timescaledb_functions.process_hypertable_invalidations(ARRAY[0]);
188+
CALL _timescaledb_functions.process_hypertable_invalidations(ARRAY['cond_10', 'cond_20']);
189+
CALL _timescaledb_functions.process_hypertable_invalidations(ARRAY['conditions', 'cond_20']);
190+
CALL _timescaledb_functions.process_hypertable_invalidations(ARRAY['conditions', 123456]::regclass[]);
191+
CALL _timescaledb_functions.process_hypertable_invalidations(ARRAY['conditions', 123456, 'cond_10']::regclass[]);
192+
CALL _timescaledb_functions.process_hypertable_invalidations(ARRAY['conditions', 123456, 'cond_10', 'cond_20']::regclass[]);
193+
\set VERBOSITY terse
194+
\set ON_ERROR_STOP 1

0 commit comments

Comments
 (0)