Refactor to combine policy config code

Function to read common fields in policy configurations are duplicated,
so they are combined into a single module. Using more useful error
codes as well instead of "internal error".

Author: mkindahl

tsl/src/bgw_policy/CMakeLists.txt

@@ -4,6 +4,7 @@ set(SOURCES
     ${CMAKE_CURRENT_SOURCE_DIR}/job.c
     ${CMAKE_CURRENT_SOURCE_DIR}/job_api.c
     ${CMAKE_CURRENT_SOURCE_DIR}/reorder_api.c
+    ${CMAKE_CURRENT_SOURCE_DIR}/policy_config.c
     ${CMAKE_CURRENT_SOURCE_DIR}/retention_api.c
     ${CMAKE_CURRENT_SOURCE_DIR}/policy_utils.c
     ${CMAKE_CURRENT_SOURCE_DIR}/policies_v2.c)

tsl/src/bgw_policy/compression_api.c

@@ -19,7 +19,7 @@
 #include "bgw_policy/job.h"
 #include "bgw_policy/job_api.h"
 #include "bgw_policy/policies_v2.h"
-#include "compression/api.h"
+#include "bgw_policy/policy_config.h"
 #include "errors.h"
 #include "guc.h"
 #include "hypertable.h"
@@ -53,21 +53,6 @@ policy_compression_get_maxchunks_per_job(const Jsonb *config)
 	return (found && maxchunks > 0) ? maxchunks : 0;
 }
 
-int32
-policy_compression_get_hypertable_id(const Jsonb *config)
-{
-	bool found;
-	int32 hypertable_id =
-		ts_jsonb_get_int32_field(config, POL_COMPRESSION_CONF_KEY_HYPERTABLE_ID, &found);
-
-	if (!found)
-		ereport(ERROR,
-				(errcode(ERRCODE_INTERNAL_ERROR),
-				 errmsg("could not find hypertable_id in config for job")));
-
-	return hypertable_id;
-}
-
 int64
 policy_recompression_get_recompress_after_int(const Jsonb *config)
 {
@@ -296,7 +281,7 @@ policy_compression_add_internal(Oid user_rel_oid, Datum compress_after_datum,
 	JsonbParseState *parse_state = NULL;
 
 	pushJsonbValue(&parse_state, WJB_BEGIN_OBJECT, NULL);
-	ts_jsonb_add_int32(parse_state, POL_COMPRESSION_CONF_KEY_HYPERTABLE_ID, hypertable->fd.id);
+	ts_jsonb_add_int32(parse_state, POLICY_CONFIG_KEY_HYPERTABLE_ID, hypertable->fd.id);
 	validate_compress_after_type(dim, partitioning_type, compress_after_type);
 
 	if (use_access_method != USE_AM_NULL)

tsl/src/bgw_policy/continuous_aggregate_api.c

@@ -43,7 +43,7 @@ policy_continuous_aggregate_get_mat_hypertable_id(const Jsonb *config)
 
 	if (!found)
 		ereport(ERROR,
-				(errcode(ERRCODE_INTERNAL_ERROR),
+				(errcode(ERRCODE_SQL_JSON_MEMBER_NOT_FOUND),
 				 errmsg("could not find \"%s\" in config for job",
 						POL_REFRESH_CONF_KEY_MAT_HYPERTABLE_ID)));
 

tsl/src/bgw_policy/job.c

@@ -33,6 +33,7 @@
 #include "bgw_policy/chunk_stats.h"
 #include "bgw_policy/compression_api.h"
 #include "bgw_policy/continuous_aggregate_api.h"
+#include "bgw_policy/policy_config.h"
 #include "bgw_policy/policy_utils.h"
 #include "bgw_policy/reorder_api.h"
 #include "bgw_policy/retention_api.h"
@@ -266,7 +267,7 @@ policy_reorder_execute(int32 job_id, Jsonb *config)
 void
 policy_reorder_read_and_validate_config(Jsonb *config, PolicyReorderData *policy)
 {
-	int32 htid = policy_reorder_get_hypertable_id(config);
+	int32 htid = policy_config_get_hypertable_id(config);
 	Hypertable *ht = ts_hypertable_get_by_id(htid);
 
 	if (!ht)
@@ -319,7 +320,7 @@ policy_retention_read_and_validate_config(Jsonb *config, PolicyRetentionData *po
 	interval_getter = policy_retention_get_drop_after_interval;
 	bool use_creation_time = false;
 
-	object_relid = ts_hypertable_id_to_relid(policy_retention_get_hypertable_id(config), false);
+	object_relid = ts_hypertable_id_to_relid(policy_config_get_hypertable_id(config), false);
 	hypertable = ts_hypertable_cache_get_cache_and_entry(object_relid, CACHE_FLAG_NONE, &hcache);
 
 	open_dim = get_open_dimension_for_hypertable(hypertable, false);
@@ -527,8 +528,7 @@ policy_refresh_cagg_read_and_validate_config(Jsonb *config, PolicyContinuousAggD
 void
 policy_compression_read_and_validate_config(Jsonb *config, PolicyCompressionData *policy_data)
 {
-	Oid table_relid =
-		ts_hypertable_id_to_relid(policy_compression_get_hypertable_id(config), false);
+	Oid table_relid = ts_hypertable_id_to_relid(policy_config_get_hypertable_id(config), false);
 	Cache *hcache;
 	Hypertable *hypertable =
 		ts_hypertable_cache_get_cache_and_entry(table_relid, CACHE_FLAG_NONE, &hcache);
@@ -542,8 +542,7 @@ policy_compression_read_and_validate_config(Jsonb *config, PolicyCompressionData
 void
 policy_recompression_read_and_validate_config(Jsonb *config, PolicyCompressionData *policy_data)
 {
-	Oid table_relid =
-		ts_hypertable_id_to_relid(policy_compression_get_hypertable_id(config), false);
+	Oid table_relid = ts_hypertable_id_to_relid(policy_config_get_hypertable_id(config), false);
 	Cache *hcache;
 	Hypertable *hypertable =
 		ts_hypertable_cache_get_cache_and_entry(table_relid, CACHE_FLAG_NONE, &hcache);

tsl/src/bgw_policy/policies_v2.h

@@ -26,7 +26,6 @@
 
 #define POLICY_COMPRESSION_PROC_NAME "policy_compression"
 #define POLICY_COMPRESSION_CHECK_NAME "policy_compression_check"
-#define POL_COMPRESSION_CONF_KEY_HYPERTABLE_ID "hypertable_id"
 #define POL_COMPRESSION_CONF_KEY_COMPRESS_AFTER "compress_after"
 #define POL_COMPRESSION_CONF_KEY_MAXCHUNKS_TO_COMPRESS "maxchunks_to_compress"
 #define POL_COMPRESSION_CONF_KEY_COMPRESS_CREATED_BEFORE "compress_created_before"
@@ -37,11 +36,9 @@
 
 #define POLICY_RETENTION_PROC_NAME "policy_retention"
 #define POLICY_RETENTION_CHECK_NAME "policy_retention_check"
-#define POL_RETENTION_CONF_KEY_HYPERTABLE_ID "hypertable_id"
 #define POL_RETENTION_CONF_KEY_DROP_AFTER "drop_after"
 #define POL_RETENTION_CONF_KEY_DROP_CREATED_BEFORE "drop_created_before"
 
-#define SHOW_POLICY_KEY_HYPERTABLE_ID "hypertable_id"
 #define SHOW_POLICY_KEY_POLICY_NAME "policy_name"
 #define SHOW_POLICY_KEY_REFRESH_INTERVAL "refresh_interval"
 #define SHOW_POLICY_KEY_REFRESH_START_OFFSET "refresh_start_offset"

tsl/src/bgw_policy/policy_config.c

@@ -0,0 +1,29 @@
+/*
+ * This file and its contents are licensed under the Timescale License.
+ * Please see the included NOTICE for copyright information and
+ * LICENSE-TIMESCALE for a copy of the license.
+ */
+
+/*
+ * Module with common functions, variables, and constants for working with
+ * policy configurations.
+ */
+
+#include <postgres.h>
+
+#include "jsonb_utils.h"
+#include "policy_config.h"
+
+int32
+policy_config_get_hypertable_id(const Jsonb *config)
+{
+	bool found;
+	int32 hypertable_id = ts_jsonb_get_int32_field(config, POLICY_CONFIG_KEY_HYPERTABLE_ID, &found);
+
+	if (!found)
+		ereport(ERROR,
+				(errcode(ERRCODE_SQL_JSON_MEMBER_NOT_FOUND),
+				 errmsg("could not find hypertable_id in config for job")));
+
+	return hypertable_id;
+}

tsl/src/bgw_policy/policy_config.h

@@ -0,0 +1,18 @@
+/*
+ * This file and its contents are licensed under the Timescale License.
+ * Please see the included NOTICE for copyright information and
+ * LICENSE-TIMESCALE for a copy of the license.
+ */
+#pragma once
+
+/*
+ * Common functions, variables, and constants for working with policy
+ * configurations.
+ */
+
+#include <postgres.h>
+#include <utils/jsonb.h>
+
+#define POLICY_CONFIG_KEY_HYPERTABLE_ID "hypertable_id"
+
+extern int32 policy_config_get_hypertable_id(const Jsonb *config);

tsl/src/bgw_policy/reorder_api.c

@@ -24,11 +24,10 @@
 #include "bgw/timer.h"
 #include "bgw_policy/job.h"
 #include "bgw_policy/job_api.h"
+#include "bgw_policy/policy_config.h"
 #include "bgw_policy/reorder_api.h"
-#include "errors.h"
 #include "guc.h"
 #include "hypertable.h"
-#include "reorder.h"
 #include "utils.h"
 /*
  * Default scheduled interval for reorder jobs should be 1/2 of the default chunk length.
@@ -48,26 +47,11 @@
 #define DEFAULT_RETRY_PERIOD                                                                       \
 	DatumGetIntervalP(DirectFunctionCall3(interval_in, CStringGetDatum("5 min"), InvalidOid, -1))
 
-#define CONFIG_KEY_HYPERTABLE_ID "hypertable_id"
 #define CONFIG_KEY_INDEX_NAME "index_name"
 
 #define POLICY_REORDER_PROC_NAME "policy_reorder"
 #define POLICY_REORDER_CHECK_NAME "policy_reorder_check"
 
-int32
-policy_reorder_get_hypertable_id(const Jsonb *config)
-{
-	bool found;
-	int32 hypertable_id = ts_jsonb_get_int32_field(config, CONFIG_KEY_HYPERTABLE_ID, &found);
-
-	if (!found)
-		ereport(ERROR,
-				(errcode(ERRCODE_INTERNAL_ERROR),
-				 errmsg("could not find hypertable_id in config for job")));
-
-	return hypertable_id;
-}
-
 char *
 policy_reorder_get_index_name(const Jsonb *config)
 {
@@ -262,7 +246,7 @@ policy_reorder_add(PG_FUNCTION_ARGS)
 	JsonbParseState *parse_state = NULL;
 
 	pushJsonbValue(&parse_state, WJB_BEGIN_OBJECT, NULL);
-	ts_jsonb_add_int32(parse_state, CONFIG_KEY_HYPERTABLE_ID, hypertable_id);
+	ts_jsonb_add_int32(parse_state, POLICY_CONFIG_KEY_HYPERTABLE_ID, hypertable_id);
 	ts_jsonb_add_str(parse_state, CONFIG_KEY_INDEX_NAME, NameStr(*index_name));
 	JsonbValue *result = pushJsonbValue(&parse_state, WJB_END_OBJECT, NULL);
 	Jsonb *config = JsonbValueToJsonb(result);

tsl/src/bgw_policy/reorder_api.h

@@ -13,5 +13,4 @@ extern Datum policy_reorder_remove(PG_FUNCTION_ARGS);
 extern Datum policy_reorder_proc(PG_FUNCTION_ARGS);
 extern Datum policy_reorder_check(PG_FUNCTION_ARGS);
 
-extern int32 policy_reorder_get_hypertable_id(const Jsonb *config);
 extern char *policy_reorder_get_index_name(const Jsonb *config);

tsl/src/bgw_policy/retention_api.c

@@ -19,6 +19,7 @@
 #include "bgw/timer.h"
 #include "bgw_policy/job.h"
 #include "bgw_policy/policies_v2.h"
+#include "bgw_policy/policy_config.h"
 #include "chunk.h"
 #include "dimension.h"
 #include "errors.h"
@@ -60,21 +61,6 @@ policy_retention_check(PG_FUNCTION_ARGS)
 	PG_RETURN_VOID();
 }
 
-int32
-policy_retention_get_hypertable_id(const Jsonb *config)
-{
-	bool found;
-	int32 hypertable_id =
-		ts_jsonb_get_int32_field(config, POL_RETENTION_CONF_KEY_HYPERTABLE_ID, &found);
-
-	if (!found)
-		ereport(ERROR,
-				(errcode(ERRCODE_INTERNAL_ERROR),
-				 errmsg("could not find hypertable_id in config for job")));
-
-	return hypertable_id;
-}
-
 int64
 policy_retention_get_drop_after_int(const Jsonb *config)
 {
@@ -287,7 +273,7 @@ policy_retention_add_internal(Oid ht_oid, Oid window_type, Datum window_datum,
 	JsonbParseState *parse_state = NULL;
 
 	pushJsonbValue(&parse_state, WJB_BEGIN_OBJECT, NULL);
-	ts_jsonb_add_int32(parse_state, POL_RETENTION_CONF_KEY_HYPERTABLE_ID, hypertable->fd.id);
+	ts_jsonb_add_int32(parse_state, POLICY_CONFIG_KEY_HYPERTABLE_ID, hypertable->fd.id);
 
 	switch (window_type)
 	{