postgres/timescaledb
1
0
Fork 0
mirror of https://github.com/timescale/timescaledb.git synced 2025-05-17 11:03:36 +08:00
Code Issues Packages Projects Releases Wiki Activity

Remove support for procedures as custom checks

Browse Source 
Procedures doing their own transaction handling could lead to errors
or even crashes.

Fixes #4703
This commit is contained in:
Konstantina Skovola 2022-09-15 15:57:28 +03:00 committed by Konstantina Skovola
parent 042735f1f5
commit 97a603fe5c
3 changed files with 32 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

51
src/bgw/job.c
View File

@ -85,22 +85,6 @@ job_execute_function(FuncExpr *funcexpr)
FreeExecutorState(estate); 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. * Run configuration check validation function.
* *
@ -125,19 +109,13 @@ ts_bgw_job_run_config_check(Oid check, int32 job_id, Jsonb *config)
FuncExpr *funcexpr = FuncExpr *funcexpr =
makeFuncExpr(check, VOIDOID, args, InvalidOid, InvalidOid, COERCE_EXPLICIT_CALL); makeFuncExpr(check, VOIDOID, args, InvalidOid, InvalidOid, COERCE_EXPLICIT_CALL);
switch (get_func_prokind(check)) if (get_func_prokind(check) == PROKIND_FUNCTION)
{ job_execute_function(funcexpr);
case PROKIND_FUNCTION: else
job_execute_function(funcexpr); ereport(ERROR,
break; (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("unsupported function type")),
case PROKIND_PROCEDURE: errdetail("Only functions are allowed as custom configuration checks"),
job_execute_procedure(funcexpr); errhint("Use a FUNCTION instead"));
break;
default:
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("unsupported function type")));
break;
}
} }
/* Run the check function on a configuration. It will generate errors if there /* 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) job_config_check(BgwJob *job, Jsonb *config)
{ {
Oid proc; Oid proc;
ObjectWithArgs *object; List *funcname;
/* Both should either be empty or contain a schema and name */ /* Both should either be empty or contain a schema and name */
Assert((strlen(NameStr(job->fd.check_schema)) == 0) == 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) if (strlen(NameStr(job->fd.check_name)) == 0)
return; 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)), Oid argtypes[] = { JSONBOID };
makeString(NameStr(job->fd.check_name))); /* Only functions allowed as custom checks, as procedures can cause errors with COMMIT
object->objargs = list_make1(SystemTypeName("jsonb")); * statements */
proc = LookupFuncWithArgs(OBJECT_ROUTINE, object, true); proc = LookupFuncName(funcname, 1, argtypes, true);
/* a check function has been registered but it can't be found anymore /* 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 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); ts_bgw_job_run_config_check(proc, job->fd.id, config);
else else
elog(WARNING, 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", "job %d",
NameStr(job->fd.check_schema), NameStr(job->fd.check_schema),
NameStr(job->fd.check_name), NameStr(job->fd.check_name),

44
tsl/test/expected/bgw_custom.out
View File

@ -619,22 +619,16 @@ BEGIN
END END
$$; $$;
-- step 3, add the job with the config check function passed as argument -- 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 => '{}', check_config => 'test_config_check_proc'::regproc);
ERROR: Config must be not NULL and have drop_after ERROR: unsupported function type
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
-- test functions -- test functions
select add_job('test_proc_with_check', '5 secs', config => '{}', check_config => 'test_config_check_func'::regproc); 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 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); select add_job('test_proc_with_check', '5 secs', config => NULL, check_config => 'test_config_check_func'::regproc);
add_job add_job
--------- ---------
1006 1005
(1 row) (1 row)
select add_job('test_proc_with_check', '5 secs', config => '{"drop_after": "chicken"}', check_config => 'test_config_check_func'::regproc); 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"}'); select alter_job(:job_with_func_check_id, config => '{"drop_after":"5 years"}');
alter_job alter_job
----------------------------------------------------------------------------------------------------------------- -----------------------------------------------------------------------------------------------------------------
(1007,"@ 5 secs","@ 0",-1,"@ 5 mins",t,"{""drop_after"": ""5 years""}",-infinity,public.test_config_check_func) (1006,"@ 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)
(1 row) (1 row)
-- test that jobs with an incorrect check function signature will not be registered -- 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 NOTICE: This message will get printed for both NULL and not NULL config
add_job add_job
--------- ---------
1008 1007
(1 row) (1 row)
-- check done -- 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 -- the check is being skipped due to the check function missing
ALTER FUNCTION test_config_check_func RENAME TO renamed_func; ALTER FUNCTION test_config_check_func RENAME TO renamed_func;
select alter_job(:job_id, schedule_interval => '1 hour'); 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 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) (1 row)
DROP FUNCTION test_config_check_func_returns_int; DROP FUNCTION test_config_check_func_returns_int;
select alter_job(:job_id_int, config => '{"field":"value"}'); 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 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) (1 row)
-- rename the check function and then call alter_job to register the new name -- 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 NOTICE: This message will get printed for both NULL and not NULL config
alter_job 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) (1 row)
-- run alter again, should get a config check -- 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 NOTICE: This message will get printed for both NULL and not NULL config
alter_job 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) (1 row)
-- do not drop the current check function but register a new one -- 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 NOTICE: This message is a substitute of the previously printed one
alter_job 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) (1 row)
select alter_job(:job_id, config => '{}'); select alter_job(:job_id, config => '{}');
NOTICE: This message is a substitute of the previously printed one NOTICE: This message is a substitute of the previously printed one
alter_job 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) (1 row)
RESET client_min_messages; 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 NOTICE: This message will get printed for both NULL and not NULL config
alter_job 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) (1 row)
-- test that we can unregister the check function -- test that we can unregister the check function
select alter_job(:job_id_alter, check_config => 0); select alter_job(:job_id_alter, check_config => 0);
alter_job alter_job
------------------------------------------------------- -------------------------------------------------------
(1011,"@ 5 secs","@ 0",-1,"@ 5 mins",t,{},-infinity,) (1010,"@ 5 secs","@ 0",-1,"@ 5 mins",t,{},-infinity,)
(1 row) (1 row)
-- no message printed now -- no message printed now
select alter_job(:job_id_alter, config => '{}'); select alter_job(:job_id_alter, config => '{}');
alter_job alter_job
------------------------------------------------------- -------------------------------------------------------
(1011,"@ 5 secs","@ 0",-1,"@ 5 mins",t,{},-infinity,) (1010,"@ 5 secs","@ 0",-1,"@ 5 mins",t,{},-infinity,)
(1 row) (1 row)
-- test the case where we have a background job that registers jobs with a check fn -- test the case where we have a background job that registers jobs with a check fn

8
tsl/test/sql/bgw_custom.sql
View File

@ -361,12 +361,8 @@ END
$$; $$;
-- step 3, add the job with the config check function passed as argument -- 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 => '{}', 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 -- test functions
select add_job('test_proc_with_check', '5 secs', config => '{}', check_config => 'test_config_check_func'::regproc); 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":"chicken"}');
select alter_job(:job_with_func_check_id, config => '{"drop_after":"5 years"}'); 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 -- test that jobs with an incorrect check function signature will not be registered
-- these are all incorrect function signatures -- these are all incorrect function signatures