diff --git a/src/bgw/job.c b/src/bgw/job.c index 97dc1ec3c..fe1e616b8 100644 --- a/src/bgw/job.c +++ b/src/bgw/job.c @@ -85,22 +85,6 @@ job_execute_function(FuncExpr *funcexpr) FreeExecutorState(estate); } -/* This is copied from tsl/src/job.c */ -static void -job_execute_procedure(FuncExpr *funcexpr) -{ - CallStmt *call = makeNode(CallStmt); - call->funcexpr = funcexpr; - DestReceiver *dest = CreateDestReceiver(DestNone); - /* we don't need to create proper param list cause we pass in all arguments as Const */ -#ifdef PG11 - ParamListInfo params = palloc0(offsetof(ParamListInfoData, params)); -#else - ParamListInfo params = makeParamList(0); -#endif - ExecuteCallStmt(call, params, false, dest); -} - /** * Run configuration check validation function. * @@ -125,19 +109,13 @@ ts_bgw_job_run_config_check(Oid check, int32 job_id, Jsonb *config) FuncExpr *funcexpr = makeFuncExpr(check, VOIDOID, args, InvalidOid, InvalidOid, COERCE_EXPLICIT_CALL); - switch (get_func_prokind(check)) - { - case PROKIND_FUNCTION: - job_execute_function(funcexpr); - break; - case PROKIND_PROCEDURE: - job_execute_procedure(funcexpr); - break; - default: - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("unsupported function type"))); - break; - } + if (get_func_prokind(check) == PROKIND_FUNCTION) + job_execute_function(funcexpr); + else + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("unsupported function type")), + errdetail("Only functions are allowed as custom configuration checks"), + errhint("Use a FUNCTION instead")); } /* Run the check function on a configuration. It will generate errors if there @@ -147,7 +125,7 @@ static void job_config_check(BgwJob *job, Jsonb *config) { Oid proc; - ObjectWithArgs *object; + List *funcname; /* Both should either be empty or contain a schema and name */ Assert((strlen(NameStr(job->fd.check_schema)) == 0) == @@ -157,12 +135,13 @@ job_config_check(BgwJob *job, Jsonb *config) if (strlen(NameStr(job->fd.check_name)) == 0) return; - object = makeNode(ObjectWithArgs); + funcname = list_make2(makeString(NameStr(job->fd.check_schema)), + makeString(NameStr(job->fd.check_name))); - object->objname = list_make2(makeString(NameStr(job->fd.check_schema)), - makeString(NameStr(job->fd.check_name))); - object->objargs = list_make1(SystemTypeName("jsonb")); - proc = LookupFuncWithArgs(OBJECT_ROUTINE, object, true); + Oid argtypes[] = { JSONBOID }; + /* Only functions allowed as custom checks, as procedures can cause errors with COMMIT + * statements */ + proc = LookupFuncName(funcname, 1, argtypes, true); /* a check function has been registered but it can't be found anymore because it was dropped or renamed. Allow alter_job to run if that's the case @@ -171,7 +150,7 @@ job_config_check(BgwJob *job, Jsonb *config) ts_bgw_job_run_config_check(proc, job->fd.id, config); else elog(WARNING, - "function or procedure %s.%s(config jsonb) not found, skipping config validation for " + "function %s.%s(config jsonb) not found, skipping config validation for " "job %d", NameStr(job->fd.check_schema), NameStr(job->fd.check_name), diff --git a/tsl/test/expected/bgw_custom.out b/tsl/test/expected/bgw_custom.out index aa6ca917a..d30c8c2d6 100644 --- a/tsl/test/expected/bgw_custom.out +++ b/tsl/test/expected/bgw_custom.out @@ -619,22 +619,16 @@ BEGIN END $$; -- step 3, add the job with the config check function passed as argument --- test procedures +-- test procedures, should get an unsupported error select add_job('test_proc_with_check', '5 secs', config => '{}', check_config => 'test_config_check_proc'::regproc); -ERROR: Config must be not NULL and have drop_after -select add_job('test_proc_with_check', '5 secs', config => NULL, check_config => 'test_config_check_proc'::regproc); -ERROR: Config must be not NULL and have drop_after -select add_job('test_proc_with_check', '5 secs', config => '{"drop_after": "chicken"}', check_config => 'test_config_check_proc'::regproc); -ERROR: invalid input syntax for type interval: "chicken" -select add_job('test_proc_with_check', '5 secs', config => '{"drop_after": "2 weeks"}', check_config => 'test_config_check_proc'::regproc) -as job_with_proc_check_id \gset +ERROR: unsupported function type -- test functions select add_job('test_proc_with_check', '5 secs', config => '{}', check_config => 'test_config_check_func'::regproc); ERROR: Config can be NULL but must have drop_after if not select add_job('test_proc_with_check', '5 secs', config => NULL, check_config => 'test_config_check_func'::regproc); add_job --------- - 1006 + 1005 (1 row) select add_job('test_proc_with_check', '5 secs', config => '{"drop_after": "chicken"}', check_config => 'test_config_check_func'::regproc); @@ -647,13 +641,7 @@ ERROR: invalid input syntax for type interval: "chicken" select alter_job(:job_with_func_check_id, config => '{"drop_after":"5 years"}'); alter_job ----------------------------------------------------------------------------------------------------------------- - (1007,"@ 5 secs","@ 0",-1,"@ 5 mins",t,"{""drop_after"": ""5 years""}",-infinity,public.test_config_check_func) -(1 row) - -select alter_job(:job_with_proc_check_id, config => '{"drop_after":"4 days"}'); - alter_job ----------------------------------------------------------------------------------------------------------------- - (1005,"@ 5 secs","@ 0",-1,"@ 5 mins",t,"{""drop_after"": ""4 days""}",-infinity,public.test_config_check_proc) + (1006,"@ 5 secs","@ 0",-1,"@ 5 mins",t,"{""drop_after"": ""5 years""}",-infinity,public.test_config_check_func) (1 row) -- test that jobs with an incorrect check function signature will not be registered @@ -701,7 +689,7 @@ select add_job('test_proc_with_check', '5 secs', config => NULL, check_config => NOTICE: This message will get printed for both NULL and not NULL config add_job --------- - 1008 + 1007 (1 row) -- check done @@ -721,18 +709,18 @@ NOTICE: I print a message, and then I return least(1,2) -- the check is being skipped due to the check function missing ALTER FUNCTION test_config_check_func RENAME TO renamed_func; select alter_job(:job_id, schedule_interval => '1 hour'); -WARNING: function or procedure public.test_config_check_func(config jsonb) not found, skipping config validation for job 1009 +WARNING: function public.test_config_check_func(config jsonb) not found, skipping config validation for job 1008 alter_job ------------------------------------------------------------------------------------ - (1009,"@ 1 hour","@ 0",-1,"@ 5 mins",t,{},-infinity,public.test_config_check_func) + (1008,"@ 1 hour","@ 0",-1,"@ 5 mins",t,{},-infinity,public.test_config_check_func) (1 row) DROP FUNCTION test_config_check_func_returns_int; select alter_job(:job_id_int, config => '{"field":"value"}'); -WARNING: function or procedure public.test_config_check_func_returns_int(config jsonb) not found, skipping config validation for job 1010 +WARNING: function public.test_config_check_func_returns_int(config jsonb) not found, skipping config validation for job 1009 alter_job ---------------------------------------------------------------------------------------------------------------------- - (1010,"@ 5 secs","@ 0",-1,"@ 5 mins",t,"{""field"": ""value""}",-infinity,public.test_config_check_func_returns_int) + (1009,"@ 5 secs","@ 0",-1,"@ 5 mins",t,"{""field"": ""value""}",-infinity,public.test_config_check_func_returns_int) (1 row) -- rename the check function and then call alter_job to register the new name @@ -740,7 +728,7 @@ select alter_job(:job_id, check_config => 'renamed_func'::regproc); NOTICE: This message will get printed for both NULL and not NULL config alter_job -------------------------------------------------------------------------- - (1009,"@ 1 hour","@ 0",-1,"@ 5 mins",t,{},-infinity,public.renamed_func) + (1008,"@ 1 hour","@ 0",-1,"@ 5 mins",t,{},-infinity,public.renamed_func) (1 row) -- run alter again, should get a config check @@ -748,7 +736,7 @@ select alter_job(:job_id, config => '{}'); NOTICE: This message will get printed for both NULL and not NULL config alter_job -------------------------------------------------------------------------- - (1009,"@ 1 hour","@ 0",-1,"@ 5 mins",t,{},-infinity,public.renamed_func) + (1008,"@ 1 hour","@ 0",-1,"@ 5 mins",t,{},-infinity,public.renamed_func) (1 row) -- do not drop the current check function but register a new one @@ -763,14 +751,14 @@ select alter_job(:job_id, check_config => 'substitute_check_func'); NOTICE: This message is a substitute of the previously printed one alter_job ----------------------------------------------------------------------------------- - (1009,"@ 1 hour","@ 0",-1,"@ 5 mins",t,{},-infinity,public.substitute_check_func) + (1008,"@ 1 hour","@ 0",-1,"@ 5 mins",t,{},-infinity,public.substitute_check_func) (1 row) select alter_job(:job_id, config => '{}'); NOTICE: This message is a substitute of the previously printed one alter_job ----------------------------------------------------------------------------------- - (1009,"@ 1 hour","@ 0",-1,"@ 5 mins",t,{},-infinity,public.substitute_check_func) + (1008,"@ 1 hour","@ 0",-1,"@ 5 mins",t,{},-infinity,public.substitute_check_func) (1 row) RESET client_min_messages; @@ -810,21 +798,21 @@ select alter_job(:job_id_alter); NOTICE: This message will get printed for both NULL and not NULL config alter_job -------------------------------------------------------------------------- - (1011,"@ 5 secs","@ 0",-1,"@ 5 mins",t,{},-infinity,public.renamed_func) + (1010,"@ 5 secs","@ 0",-1,"@ 5 mins",t,{},-infinity,public.renamed_func) (1 row) -- test that we can unregister the check function select alter_job(:job_id_alter, check_config => 0); alter_job ------------------------------------------------------- - (1011,"@ 5 secs","@ 0",-1,"@ 5 mins",t,{},-infinity,) + (1010,"@ 5 secs","@ 0",-1,"@ 5 mins",t,{},-infinity,) (1 row) -- no message printed now select alter_job(:job_id_alter, config => '{}'); alter_job ------------------------------------------------------- - (1011,"@ 5 secs","@ 0",-1,"@ 5 mins",t,{},-infinity,) + (1010,"@ 5 secs","@ 0",-1,"@ 5 mins",t,{},-infinity,) (1 row) -- test the case where we have a background job that registers jobs with a check fn diff --git a/tsl/test/sql/bgw_custom.sql b/tsl/test/sql/bgw_custom.sql index cf6007d2d..f32b482fd 100644 --- a/tsl/test/sql/bgw_custom.sql +++ b/tsl/test/sql/bgw_custom.sql @@ -361,12 +361,8 @@ END $$; -- step 3, add the job with the config check function passed as argument --- test procedures +-- test procedures, should get an unsupported error select add_job('test_proc_with_check', '5 secs', config => '{}', check_config => 'test_config_check_proc'::regproc); -select add_job('test_proc_with_check', '5 secs', config => NULL, check_config => 'test_config_check_proc'::regproc); -select add_job('test_proc_with_check', '5 secs', config => '{"drop_after": "chicken"}', check_config => 'test_config_check_proc'::regproc); -select add_job('test_proc_with_check', '5 secs', config => '{"drop_after": "2 weeks"}', check_config => 'test_config_check_proc'::regproc) -as job_with_proc_check_id \gset -- test functions select add_job('test_proc_with_check', '5 secs', config => '{}', check_config => 'test_config_check_func'::regproc); @@ -380,8 +376,6 @@ as job_with_func_check_id \gset select alter_job(:job_with_func_check_id, config => '{"drop_after":"chicken"}'); select alter_job(:job_with_func_check_id, config => '{"drop_after":"5 years"}'); -select alter_job(:job_with_proc_check_id, config => '{"drop_after":"4 days"}'); - -- test that jobs with an incorrect check function signature will not be registered -- these are all incorrect function signatures