Addressed last reviews from @mkindahl

timescale · Oct 2, 2024 · ee4ecc8 · ee4ecc8
1 parent 39b6ae2
commit ee4ecc8
Show file tree

Hide file tree

Showing 5 changed files with 55 additions and 32 deletions.
diff --git a/src/bgw/job.c b/src/bgw/job.c
@@ -1056,26 +1056,29 @@ zero_guc(const char *guc_name)
  (errcode(ERRCODE_INTERNAL_ERROR), errmsg("could not set \"%s\" guc", guc_name)));
 }
 
-extern Oid
+Oid
 ts_bgw_job_get_funcid(BgwJob *job)
 {
  ObjectWithArgs *object = makeNode(ObjectWithArgs);
  object->objname = list_make2(makeString(NameStr(job->fd.proc_schema)),
  makeString(NameStr(job->fd.proc_name)));
  object->objargs = list_make2(SystemTypeName("int4"), SystemTypeName("jsonb"));
 
- return LookupFuncWithArgs(OBJECT_ROUTINE, object, false);
+ /* Return InvalidOid if don't found */
+ return LookupFuncWithArgs(OBJECT_ROUTINE, object, true);
 }
 
 const char *
 ts_bgw_job_function_call_string(BgwJob *job)
 {
- char prokind = get_func_prokind(ts_bgw_job_get_funcid(job));
+ Oid funcid = ts_bgw_job_get_funcid(job);
+ /* If not found the function or procedure then fallback to PROKIND_FUNCTION */
+ char prokind = OidIsValid(funcid) ? get_func_prokind(funcid) : PROKIND_FUNCTION;
  StringInfo stmt = makeStringInfo();
  char *jsonb_str = "NULL";
 
  if (job->fd.config)
- jsonb_str = (char *) quote_identifier(
+ jsonb_str = quote_literal_cstr(
  JsonbToCString(NULL, &job->fd.config->root, VARSIZE(job->fd.config)));
 
  switch (prokind)

diff --git a/tsl/test/expected/bgw_custom.out b/tsl/test/expected/bgw_custom.out
@@ -1077,26 +1077,30 @@ SELECT delete_job(:job_proc);
 SELECT add_job('custom_func', '1h', config => '{"type":"function"}'::jsonb) AS job_func \gset
 SELECT add_job('custom_proc', '1h', config => '{"type":"procedure"}'::jsonb) AS job_proc \gset
 SELECT ts_test_bgw_job_function_call_string(:job_func);
-  ts_test_bgw_job_function_call_string  
----------------------------------------------------------------
- SELECT public.custom_func('1020', "{""type"": ""function""}")
+ ts_test_bgw_job_function_call_string 
+-----------------------------------------------------------
+ SELECT public.custom_func('1020', '{"type": "function"}')
 (1 row)
 
 SELECT ts_test_bgw_job_function_call_string(:job_proc);
- ts_test_bgw_job_function_call_string 
---------------------------------------------------------------
- CALL public.custom_proc('1021', "{""type"": ""procedure""}")
+ ts_test_bgw_job_function_call_string 
+----------------------------------------------------------
+ CALL public.custom_proc('1021', '{"type": "procedure"}')
+(1 row)
+
+-- Remove the procedure and let's check it fallingback to PROKIND_FUNCTION
+DROP PROCEDURE custom_proc(jobid int, args jsonb);
+SELECT ts_test_bgw_job_function_call_string(:job_proc);
+ ts_test_bgw_job_function_call_string 
+------------------------------------------------------------
+ SELECT public.custom_proc('1021', '{"type": "procedure"}')
 (1 row)
 
 \set ON_ERROR_STOP 0
--- Check for on existing function
-DROP FUNCTION custom_func(jobid int, args jsonb);
-SELECT ts_test_bgw_job_function_call_string(:job_func);
-ERROR: function public.custom_func(integer, jsonb) does not exist
 -- Mess with pg catalog to don't identify the PROKIND
 BEGIN;
-UPDATE pg_catalog.pg_proc SET prokind = 'X' WHERE oid = 'custom_proc(int,jsonb)'::regprocedure;
-SELECT ts_test_bgw_job_function_call_string(:job_proc);
+UPDATE pg_catalog.pg_proc SET prokind = 'X' WHERE oid = 'custom_func(int,jsonb)'::regprocedure;
+SELECT ts_test_bgw_job_function_call_string(:job_func);
 ERROR: unsupported function type: X
 ROLLBACK;
 \set ON_ERROR_STOP 1

diff --git a/tsl/test/expected/bgw_job_stat_history_errors.out b/tsl/test/expected/bgw_job_stat_history_errors.out
@@ -109,7 +109,7 @@ SELECT next_start FROM alter_job(3, next_start => '2060-01-01 00:00:00+00'::time
  Wed Dec 31 16:00:00 2059 PST
 (1 row)
 
-TRUNCATE TABLE _timescaledb_internal.bgw_job_stat_history;
+DELETE FROM _timescaledb_internal.bgw_job_stat_history;
 INSERT INTO _timescaledb_internal.bgw_job_stat_history(job_id, pid, succeeded, execution_start, execution_finish, data)
 VALUES (123, 12345, false, '2000-01-01 00:00:00+00'::timestamptz, '2000-01-01 00:00:10+00'::timestamptz, '{}'),
 (456, 45678, false, '2000-01-01 00:00:20+00'::timestamptz, '2000-01-01 00:00:40+00'::timestamptz, '{}'),
@@ -145,11 +145,22 @@ WHERE succeeded IS FALSE;
 (1 row)
 
 -- test failure when starting jobs
-TRUNCATE _timescaledb_internal.bgw_job_stat;
-TRUNCATE _timescaledb_internal.bgw_job_stat_history;
-TRUNCATE _timescaledb_config.bgw_job CASCADE;
-NOTICE: truncate cascades to table "bgw_job_stat"
-NOTICE: truncate cascades to table "bgw_policy_chunk_stats"
+\c :TEST_DBNAME :ROLE_SUPERUSER
+SELECT _timescaledb_functions.stop_background_workers();
+ stop_background_workers 
+-------------------------
+ t
+(1 row)
+
+DELETE FROM _timescaledb_internal.bgw_job_stat;
+DELETE FROM _timescaledb_internal.bgw_job_stat_history;
+DELETE FROM _timescaledb_config.bgw_job CASCADE;
+SELECT _timescaledb_functions.start_background_workers();
+ start_background_workers 
+--------------------------
+ t
+(1 row)
+
 \set VERBOSITY default
 -- Setup Jobs
 DO

diff --git a/tsl/test/sql/bgw_custom.sql b/tsl/test/sql/bgw_custom.sql
@@ -692,15 +692,15 @@ SELECT add_job('custom_proc', '1h', config => '{"type":"procedure"}'::jsonb) AS
 SELECT ts_test_bgw_job_function_call_string(:job_func);
 SELECT ts_test_bgw_job_function_call_string(:job_proc);
 
-\set ON_ERROR_STOP 0
--- Check for on existing function
-DROP FUNCTION custom_func(jobid int, args jsonb);
-SELECT ts_test_bgw_job_function_call_string(:job_func);
+-- Remove the procedure and let's check it fallingback to PROKIND_FUNCTION
+DROP PROCEDURE custom_proc(jobid int, args jsonb);
+SELECT ts_test_bgw_job_function_call_string(:job_proc);
 
+\set ON_ERROR_STOP 0
 -- Mess with pg catalog to don't identify the PROKIND
 BEGIN;
-UPDATE pg_catalog.pg_proc SET prokind = 'X' WHERE oid = 'custom_proc(int,jsonb)'::regprocedure;
-SELECT ts_test_bgw_job_function_call_string(:job_proc);
+UPDATE pg_catalog.pg_proc SET prokind = 'X' WHERE oid = 'custom_func(int,jsonb)'::regprocedure;
+SELECT ts_test_bgw_job_function_call_string(:job_func);
 ROLLBACK;
 \set ON_ERROR_STOP 1
 

diff --git a/tsl/test/sql/bgw_job_stat_history_errors.sql b/tsl/test/sql/bgw_job_stat_history_errors.sql
@@ -67,7 +67,7 @@ SELECT pg_reload_conf();
 
 -- test the retention job
 SELECT next_start FROM alter_job(3, next_start => '2060-01-01 00:00:00+00'::timestamptz);
-TRUNCATE TABLE _timescaledb_internal.bgw_job_stat_history;
+DELETE FROM _timescaledb_internal.bgw_job_stat_history;
 INSERT INTO _timescaledb_internal.bgw_job_stat_history(job_id, pid, succeeded, execution_start, execution_finish, data)
 VALUES (123, 12345, false, '2000-01-01 00:00:00+00'::timestamptz, '2000-01-01 00:00:10+00'::timestamptz, '{}'),
 (456, 45678, false, '2000-01-01 00:00:20+00'::timestamptz, '2000-01-01 00:00:40+00'::timestamptz, '{}'),
@@ -87,9 +87,14 @@ FROM _timescaledb_internal.bgw_job_stat_history
 WHERE succeeded IS FALSE;
 
 -- test failure when starting jobs
-TRUNCATE _timescaledb_internal.bgw_job_stat;
-TRUNCATE _timescaledb_internal.bgw_job_stat_history;
-TRUNCATE _timescaledb_config.bgw_job CASCADE;
+\c :TEST_DBNAME :ROLE_SUPERUSER
+SELECT _timescaledb_functions.stop_background_workers();
+
+DELETE FROM _timescaledb_internal.bgw_job_stat;
+DELETE FROM _timescaledb_internal.bgw_job_stat_history;
+DELETE FROM _timescaledb_config.bgw_job CASCADE;
+
+SELECT _timescaledb_functions.start_background_workers();
 
 \set VERBOSITY default
 -- Setup Jobs