Error out on bad args when processing invalidation

.unreleased/pr_8558

@@ -0,0 +1 @@
+Fixes: #8558 Error out on bad args when processing invalidation

src/utils.c

@@ -45,7 +45,6 @@
 #include "chunk.h"
 #include "cross_module_fn.h"
 #include "debug_point.h"
-#include "guc.h"
 #include "hypertable_cache.h"
 #include "jsonb_utils.h"
 #include "time_utils.h"
@@ -2026,3 +2025,26 @@ ts_get_attr_expr(Relation rel, AttrNumber attno)
 
 	return expr;
 }
+
+char *
+ts_list_to_string(List *list, append_cell_func append)
+{
+	StringInfoData info;
+	ListCell *lc;
+
+	initStringInfo(&info);
+
+	foreach (lc, list)
+	{
+		if (!lnext(list, lc))
+			appendStringInfoString(&info, "and ");
+		append(&info, lc);
+		if (lnext(list, lc))
+		{
+			if (list_length(list) > 2)
+				appendStringInfoChar(&info, ',');
+			appendStringInfoChar(&info, ' ');
+		}
+	}
+	return info.data;
+}

src/utils.h

@@ -406,10 +406,13 @@ ts_datum_set_objectid(const AttrNumber attno, NullableDatum *datums, const Oid v
 		datums[AttrNumberGetAttrOffset(attno)].isnull = true;
 }
 
+typedef void (*append_cell_func)(StringInfo, ListCell *);
+
 extern TSDLLEXPORT void ts_get_rel_info_by_name(const char *relnamespace, const char *relname,
 												Oid *relid, Oid *amoid, char *relkind);
 extern TSDLLEXPORT void ts_get_rel_info(Oid relid, Oid *amoid, char *relkind);
 extern TSDLLEXPORT Oid ts_get_rel_am(Oid relid);
 extern TSDLLEXPORT void ts_relation_set_reloption(Relation rel, List *options, LOCKMODE lockmode);
 extern TSDLLEXPORT Jsonb *ts_errdata_to_jsonb(ErrorData *edata, Name proc_schema, Name proc_name);
 extern TSDLLEXPORT char *ts_get_attr_expr(Relation rel, AttrNumber attno);
+extern TSDLLEXPORT char *ts_list_to_string(List *list, append_cell_func append);

tsl/src/continuous_aggs/invalidation.c

@@ -29,6 +29,7 @@
 #include <utils/snapmgr.h>
 #include <utils/tuplestore.h>
 
+#include "cache.h"
 #include "continuous_aggs/invalidation_multi.h"
 #include "continuous_aggs/invalidation_threshold.h"
 #include "continuous_aggs/materialize.h"
@@ -1241,6 +1242,17 @@ continuous_agg_process_hypertable_invalidations_single(Oid hypertable_relid)
 	invalidation_process_hypertable_log(hypertable_id, dimtype);
 }
 
+static void
+append_string_info_relid(StringInfo info, ListCell *lc)
+{
+	Oid relid = lfirst_oid(lc);
+	const char *name = get_rel_name(relid);
+	if (name)
+		appendStringInfo(info, "\"%s\"", name);
+	else
+		appendStringInfo(info, "%d", relid);
+}
+
 /*
  * PostgreSQL function to move hypertable invalidations to materialization
  * invalidation log.
@@ -1250,21 +1262,59 @@ continuous_agg_process_hypertable_invalidations_multi(ArrayType *hypertable_arra
 {
 	ArrayIterator array_iterator = array_create_iterator(hypertable_array, 0, NULL);
 	List *hypertables = NIL;
+	List *bad_objects = NIL;
 
 	Datum value;
 	bool isnull;
 	while (array_iterate(array_iterator, &value, &isnull))
 	{
+		/* Function signature only allow OIDs, so we will always have an OID. */
 		Oid hypertable_relid = DatumGetObjectId(value);
-		int32 hypertable_id = ts_hypertable_relid_to_id(hypertable_relid);
-		TS_DEBUG_LOG("add relation \"%s\" to list: hypertable_id=%d",
-					 get_rel_name(hypertable_relid),
-					 hypertable_id);
-		ts_hypertable_permissions_check(hypertable_relid, GetUserId());
-		hypertables = lappend_int(hypertables, hypertable_id);
+		Cache *hcache;
+		Hypertable *ht = ts_hypertable_cache_get_cache_and_entry(hypertable_relid,
+																 CACHE_FLAG_MISSING_OK,
+																 &hcache);
+		if (ht)
+		{
+			int32 hypertable_id = ht->fd.id;
+			TS_DEBUG_LOG("add relation \"%s\" to list: hypertable_id=%d",
+						 get_rel_name(hypertable_relid),
+						 hypertable_id);
+			ts_hypertable_permissions_check(hypertable_relid, GetUserId());
+			hypertables = lappend_int(hypertables, hypertable_id);
+		}
+		else
+		{
+			TS_DEBUG_LOG("relation \"%s\" is not a hypertable", get_rel_name(hypertable_relid));
+			bad_objects = lappend_oid(bad_objects, hypertable_relid);
+		}
+		ts_cache_release(&hcache);
 	}
 
 	array_free_iterator(array_iterator);
 
+	const int bad_count = list_length(bad_objects);
+	if (bad_count == 1)
+	{
+		const Oid relid = linitial_oid(bad_objects);
+		const char *name = get_rel_name(relid);
+		if (name)
+			ereport(ERROR,
+					errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+					errmsg("table \"%s\" is not a hypertable", name));
+		else
+			ereport(ERROR,
+					errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+					errmsg("OID %d is not a hypertable", relid));
+	}
+	else if (bad_count > 1)
+	{
+		ereport(ERROR,
+				errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				errmsg("%d objects in list are not hypertables", bad_count),
+				errdetail("Bad objects are %s.",
+						  ts_list_to_string(bad_objects, append_string_info_relid)));
+	}
+
 	multi_invalidation_process_hypertable_log(hypertables);
 }

tsl/test/expected/cagg_invalidation_multi.out

@@ -231,3 +231,28 @@ GROUP BY 1 ORDER BY 1,2;
  cond_20        | [0,120)
 (2 rows)
 
+-- This should be fine but not process any tables. Odd usage, but
+-- nothing wrong with it.
+CALL _timescaledb_functions.process_hypertable_invalidations(ARRAY[]::regclass[]);
+-- These should error out
+\set ON_ERROR_STOP 0
+\set VERBOSITY default
+CALL _timescaledb_functions.process_hypertable_invalidations(ARRAY['cond_10']);
+ERROR:  table "cond_10" is not a hypertable
+CALL _timescaledb_functions.process_hypertable_invalidations(ARRAY[0]);
+ERROR:  OID 0 is not a hypertable
+CALL _timescaledb_functions.process_hypertable_invalidations(ARRAY['cond_10', 'cond_20']);
+ERROR:  2 objects in list are not hypertables
+DETAIL:  Bad objects are "cond_10" and "cond_20".
+CALL _timescaledb_functions.process_hypertable_invalidations(ARRAY['conditions', 'cond_20']);
+ERROR:  table "cond_20" is not a hypertable
+CALL _timescaledb_functions.process_hypertable_invalidations(ARRAY['conditions', 123456]::regclass[]);
+ERROR:  OID 123456 is not a hypertable
+CALL _timescaledb_functions.process_hypertable_invalidations(ARRAY['conditions', 123456, 'cond_10']::regclass[]);
+ERROR:  2 objects in list are not hypertables
+DETAIL:  Bad objects are 123456 and "cond_10".
+CALL _timescaledb_functions.process_hypertable_invalidations(ARRAY['conditions', 123456, 'cond_10', 'cond_20']::regclass[]);
+ERROR:  3 objects in list are not hypertables
+DETAIL:  Bad objects are 123456, "cond_10", and "cond_20".
+\set VERBOSITY terse
+\set ON_ERROR_STOP 1

tsl/test/sql/cagg_invalidation_multi.sql

@@ -176,3 +176,19 @@ SELECT m.aggregate_name,
  WHERE m.table_name IS NULL OR s.table_name IS NULL
 GROUP BY 1 ORDER BY 1,2;
 
+-- This should be fine but not process any tables. Odd usage, but
+-- nothing wrong with it.
+CALL _timescaledb_functions.process_hypertable_invalidations(ARRAY[]::regclass[]);
+
+-- These should error out
+\set ON_ERROR_STOP 0
+\set VERBOSITY default
+CALL _timescaledb_functions.process_hypertable_invalidations(ARRAY['cond_10']);
+CALL _timescaledb_functions.process_hypertable_invalidations(ARRAY[0]);
+CALL _timescaledb_functions.process_hypertable_invalidations(ARRAY['cond_10', 'cond_20']);
+CALL _timescaledb_functions.process_hypertable_invalidations(ARRAY['conditions', 'cond_20']);
+CALL _timescaledb_functions.process_hypertable_invalidations(ARRAY['conditions', 123456]::regclass[]);
+CALL _timescaledb_functions.process_hypertable_invalidations(ARRAY['conditions', 123456, 'cond_10']::regclass[]);
+CALL _timescaledb_functions.process_hypertable_invalidations(ARRAY['conditions', 123456, 'cond_10', 'cond_20']::regclass[]);
+\set VERBOSITY terse
+\set ON_ERROR_STOP 1