From 06d7f616872ba68330212a886ee4357338321e50 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Thu, 31 Jan 2013 10:01:32 -0500 Subject: [PATCH] Add language validator Refactor some internals to make this possible. Mainly, FunctionCallInfo is not available when validating, so avoid accessing that if not required. Rename plproxy_compile() to plproxy_compile_and_cache() and the previously internal fn_compile() to plproxy_compile(). This matches their purpose better and allows the validator to call plproxy_compile() without invoking execution-time dependent code. Many error test cases have changed because the validator catches errors when the function is created, not when it is called. Raise the extension version to 2.5.1 to be able to upgrade from non-validator installations. --- META.json | 2 +- Makefile | 9 ++-- plproxy.control | 2 +- sql/ext_update_validator.sql | 4 ++ sql/plproxy_lang.sql | 6 ++- src/function.c | 66 ++++++++++++++---------- src/main.c | 24 ++++++++- src/plproxy.h | 4 +- test/expected/plproxy_dynamic_record.out | 6 ++- test/expected/plproxy_errors.out | 37 ++++++++++--- test/expected/plproxy_select.out | 6 ++- test/expected/plproxy_split.out | 24 +++++++-- test/expected/plproxy_target.out | 12 ++++- test/sql/plproxy_errors.sql | 1 + 14 files changed, 154 insertions(+), 49 deletions(-) create mode 100644 sql/ext_update_validator.sql diff --git a/META.json b/META.json index 671db7d..6eb0839 100644 --- a/META.json +++ b/META.json @@ -14,7 +14,7 @@ "abstract": "Database partitioning implemented as procedural language", "file": "sql/plproxy.sql", "docfile": "doc/tutorial.txt", - "version": "2.5.0" + "version": "2.5.1" } }, "prereqs": { diff --git a/Makefile b/Makefile index 1110b49..fd43e81 100644 --- a/Makefile +++ b/Makefile @@ -2,8 +2,8 @@ EXTENSION = plproxy # sync with NEWS, META.json, plproxy.control, debian/changelog DISTVERSION = 2.5 -EXTVERSION = 2.5.0 -UPGRADE_VERS = 2.3.0 2.4.0 +EXTVERSION = 2.5.1 +UPGRADE_VERS = 2.3.0 2.4.0 2.5.0 # set to 1 to disallow functions containing SELECT NO_SELECT = 0 @@ -112,8 +112,9 @@ sql/$(EXTENSION)--$(EXTVERSION).sql: $(PLPROXY_SQL) echo "create extension plproxy;" > sql/plproxy.sql cat $^ > $@ -$(foreach v,$(UPGRADE_VERS),sql/plproxy--$(v)--$(EXTVERSION).sql): - touch $@ +$(foreach v,$(UPGRADE_VERS),sql/plproxy--$(v)--$(EXTVERSION).sql): sql/ext_update_validator.sql + @mkdir -p sql + cat $< >$@ sql/plproxy--unpackaged--$(EXTVERSION).sql: sql/ext_unpackaged.sql @mkdir -p sql diff --git a/plproxy.control b/plproxy.control index 64b12a3..2da2189 100644 --- a/plproxy.control +++ b/plproxy.control @@ -1,6 +1,6 @@ # plproxy extension comment = 'Database partitioning implemented as procedural language' -default_version = '2.5.0' +default_version = '2.5.1' module_pathname = '$libdir/plproxy' relocatable = false # schema = pg_catalog diff --git a/sql/ext_update_validator.sql b/sql/ext_update_validator.sql new file mode 100644 index 0000000..6ab7afa --- /dev/null +++ b/sql/ext_update_validator.sql @@ -0,0 +1,4 @@ +CREATE FUNCTION plproxy_validator (oid) +RETURNS void AS 'plproxy' LANGUAGE C; + +CREATE OR REPLACE LANGUAGE plproxy HANDLER plproxy_call_handler VALIDATOR plproxy_validator; diff --git a/sql/plproxy_lang.sql b/sql/plproxy_lang.sql index 29d6c9d..cdc1906 100644 --- a/sql/plproxy_lang.sql +++ b/sql/plproxy_lang.sql @@ -3,6 +3,10 @@ CREATE FUNCTION plproxy_call_handler () RETURNS language_handler AS 'plproxy' LANGUAGE C; +-- validator function +CREATE FUNCTION plproxy_validator (oid) +RETURNS void AS 'plproxy' LANGUAGE C; + -- language -CREATE LANGUAGE plproxy HANDLER plproxy_call_handler; +CREATE LANGUAGE plproxy HANDLER plproxy_call_handler VALIDATOR plproxy_validator; diff --git a/src/function.c b/src/function.c index f5f1d42..eed5afc 100644 --- a/src/function.c +++ b/src/function.c @@ -227,7 +227,7 @@ fn_returns_dynamic_record(HeapTuple proc_tuple) * where everything is allocated. */ static ProxyFunction * -fn_new(FunctionCallInfo fcinfo, HeapTuple proc_tuple) +fn_new(HeapTuple proc_tuple) { ProxyFunction *f; MemoryContext f_ctx, @@ -243,7 +243,7 @@ fn_new(FunctionCallInfo fcinfo, HeapTuple proc_tuple) f = palloc0(sizeof(*f)); f->ctx = f_ctx; - f->oid = fcinfo->flinfo->fn_oid; + f->oid = HeapTupleGetOid(proc_tuple); plproxy_set_stamp(&f->stamp, proc_tuple); if (fn_returns_dynamic_record(proc_tuple)) @@ -337,7 +337,6 @@ fn_parse(ProxyFunction *func, HeapTuple proc_tuple) */ static void fn_get_arguments(ProxyFunction *func, - FunctionCallInfo fcinfo, HeapTuple proc_tuple) { Oid *types; @@ -469,28 +468,41 @@ fn_refresh_record(FunctionCallInfo fcinfo, func->remote_sql = plproxy_standard_query(func, true); } -/* Show part of compilation -- get source and parse */ -static ProxyFunction * -fn_compile(FunctionCallInfo fcinfo, +/* + * Show part of compilation -- get source and parse + * + * When called from the validator, validate_only is true, but there is no + * fcinfo. + */ +ProxyFunction * +plproxy_compile(FunctionCallInfo fcinfo, HeapTuple proc_tuple, - bool validate) + bool validate_only) { ProxyFunction *f; Form_pg_proc proc_struct; + Assert(fcinfo || validate_only); + proc_struct = (Form_pg_proc) GETSTRUCT(proc_tuple); if (proc_struct->provolatile != PROVOLATILE_VOLATILE) elog(ERROR, "PL/Proxy functions must be volatile"); - f = fn_new(fcinfo, proc_tuple); + f = fn_new(proc_tuple); /* keep reference in case of error half-way */ - partial_func = f; + if (!validate_only) + partial_func = f; /* info from system tables */ fn_set_name(f, proc_tuple); - fn_get_return_type(f, fcinfo, proc_tuple); - fn_get_arguments(f, fcinfo, proc_tuple); + /* + * Cannot check return type in validator, because there is no call info to + * resolve polymorphic types against. + */ + if (!validate_only) + fn_get_return_type(f, fcinfo, proc_tuple); + fn_get_arguments(f, proc_tuple); /* parse body */ fn_parse(f, proc_tuple); @@ -498,20 +510,10 @@ fn_compile(FunctionCallInfo fcinfo, if (f->dynamic_record && f->remote_sql) plproxy_error(f, "SELECT statement not allowed for dynamic RECORD functions"); - /* create SELECT stmt if not specified */ - if (f->remote_sql == NULL) - f->remote_sql = plproxy_standard_query(f, true); - - /* prepare local queries */ - if (f->cluster_sql) - plproxy_query_prepare(f, fcinfo, f->cluster_sql, false); - if (f->hash_sql) - plproxy_query_prepare(f, fcinfo, f->hash_sql, true); - if (f->connect_sql) - plproxy_query_prepare(f, fcinfo, f->connect_sql, false); - /* sanity check */ - if (f->run_type == R_ALL && !fcinfo->flinfo->fn_retset) + if (f->run_type == R_ALL && (fcinfo + ? !fcinfo->flinfo->fn_retset + : !get_func_retset(HeapTupleGetOid(proc_tuple)))) plproxy_error(f, "RUN ON ALL requires set-returning function"); return f; @@ -521,7 +523,7 @@ fn_compile(FunctionCallInfo fcinfo, * Compile and cache PL/Proxy function. */ ProxyFunction * -plproxy_compile(FunctionCallInfo fcinfo, bool validate) +plproxy_compile_and_cache(FunctionCallInfo fcinfo) { ProxyFunction *f; HeapTuple proc_tuple; @@ -554,7 +556,19 @@ plproxy_compile(FunctionCallInfo fcinfo, bool validate) if (!f) { - f = fn_compile(fcinfo, proc_tuple, validate); + f = plproxy_compile(fcinfo, proc_tuple, false); + + /* create SELECT stmt if not specified */ + if (f->remote_sql == NULL) + f->remote_sql = plproxy_standard_query(f, true); + + /* prepare local queries */ + if (f->cluster_sql) + plproxy_query_prepare(f, fcinfo, f->cluster_sql, false); + if (f->hash_sql) + plproxy_query_prepare(f, fcinfo, f->hash_sql, true); + if (f->connect_sql) + plproxy_query_prepare(f, fcinfo, f->connect_sql, false); fn_cache_insert(f); diff --git a/src/main.c b/src/main.c index 2e0f736..c459aae 100644 --- a/src/main.c +++ b/src/main.c @@ -57,6 +57,7 @@ PG_MODULE_MAGIC; #endif PG_FUNCTION_INFO_V1(plproxy_call_handler); +PG_FUNCTION_INFO_V1(plproxy_validator); /* * Centralised error reporting. @@ -178,7 +179,7 @@ compile_and_execute(FunctionCallInfo fcinfo) plproxy_startup_init(); /* compile code */ - func = plproxy_compile(fcinfo, false); + func = plproxy_compile_and_cache(fcinfo); /* get actual cluster to run on */ cluster = plproxy_find_cluster(func, fcinfo); @@ -266,3 +267,24 @@ plproxy_call_handler(PG_FUNCTION_ARGS) } return ret; } + +/* + * This function is called when a PL/Proxy function is created to + * check the syntax. + */ +Datum +plproxy_validator(PG_FUNCTION_ARGS) +{ + Oid oid = PG_GETARG_OID(0); + HeapTuple proc_tuple; + + proc_tuple = SearchSysCache(PROCOID, ObjectIdGetDatum(oid), 0, 0, 0); + if (!HeapTupleIsValid(proc_tuple)) + elog(ERROR, "cache lookup failed for function %u", oid); + + plproxy_compile(NULL, proc_tuple, true); + + ReleaseSysCache(proc_tuple); + + PG_RETURN_VOID(); +} diff --git a/src/plproxy.h b/src/plproxy.h index d478e1c..5c5c1af 100644 --- a/src/plproxy.h +++ b/src/plproxy.h @@ -434,6 +434,7 @@ typedef struct ProxyFunction /* main.c */ Datum plproxy_call_handler(PG_FUNCTION_ARGS); +Datum plproxy_validator(PG_FUNCTION_ARGS); void plproxy_error(ProxyFunction *func, const char *fmt, ...) __attribute__((format(PG_PRINTF_ATTRIBUTE, 2, 3))); void plproxy_remote_error(ProxyFunction *func, ProxyConnection *conn, const PGresult *res, bool iserr); @@ -445,7 +446,8 @@ char *plproxy_func_strdup(ProxyFunction *func, const char *s); int plproxy_get_parameter_index(ProxyFunction *func, const char *ident); bool plproxy_split_add_ident(ProxyFunction *func, const char *ident); void plproxy_split_all_arrays(ProxyFunction *func); -ProxyFunction *plproxy_compile(FunctionCallInfo fcinfo, bool validate); +ProxyFunction *plproxy_compile_and_cache(FunctionCallInfo fcinfo); +ProxyFunction *plproxy_compile(FunctionCallInfo fcinfo, HeapTuple proc_tuple, bool validate_only); /* execute.c */ void plproxy_exec(ProxyFunction *func, FunctionCallInfo fcinfo); diff --git a/test/expected/plproxy_dynamic_record.out b/test/expected/plproxy_dynamic_record.out index fc730dc..39ce442 100644 --- a/test/expected/plproxy_dynamic_record.out +++ b/test/expected/plproxy_dynamic_record.out @@ -45,5 +45,9 @@ returns setof record as $x$ run on all; select id, username from dynamic_query_test; $x$ language plproxy; -select * from dynamic_query_select() as (id integer, username text); ERROR: PL/Proxy function public.dynamic_query_select(0): SELECT statement not allowed for dynamic RECORD functions +select * from dynamic_query_select() as (id integer, username text); +ERROR: function dynamic_query_select() does not exist +LINE 1: select * from dynamic_query_select() as (id integer, usernam... + ^ +HINT: No function matches the given name and argument types. You might need to add explicit type casts. diff --git a/test/expected/plproxy_errors.out b/test/expected/plproxy_errors.out index 49acc3a..ec6a4fc 100644 --- a/test/expected/plproxy_errors.out +++ b/test/expected/plproxy_errors.out @@ -14,8 +14,12 @@ returns text as $$ cluster 'testcluster'; run on hashtext($2); $$ language plproxy; -select * from test_err2('dat'); ERROR: PL/Proxy function public.test_err2(1): Compile error at line 3: invalid argument reference: $2 +select * from test_err2('dat'); +ERROR: function test_err2(unknown) does not exist +LINE 1: select * from test_err2('dat'); + ^ +HINT: No function matches the given name and argument types. You might need to add explicit type casts. create function test_err3(dat text) returns text as $$ cluster 'nonexists'; @@ -62,34 +66,51 @@ returns record as $$ run on hashtext(dat); select dat as res2, 'foo' as res1; $$ language plproxy; -select * from test_map_err4('dat'); ERROR: PL/Proxy function public.test_map_err4(1): Compile error at line 5: CLUSTER statement missing +select * from test_map_err4('dat'); +ERROR: function test_map_err4(unknown) does not exist +LINE 1: select * from test_map_err4('dat'); + ^ +HINT: No function matches the given name and argument types. You might need to add explicit type casts. create function test_variadic_err(first text, rest variadic text[]) returns text as $$ cluster 'testcluster'; $$ language plproxy; -select * from test_variadic_err('dat', 'dat', 'dat'); ERROR: PL/Proxy does not support variadic args +select * from test_variadic_err('dat', 'dat', 'dat'); +ERROR: function test_variadic_err(unknown, unknown, unknown) does not exist +LINE 1: select * from test_variadic_err('dat', 'dat', 'dat'); + ^ +HINT: No function matches the given name and argument types. You might need to add explicit type casts. create function test_volatile_err(dat text) returns text stable as $$ cluster 'testcluster'; $$ language plproxy; -select * from test_volatile_err('dat'); ERROR: PL/Proxy functions must be volatile +select * from test_volatile_err('dat'); +ERROR: function test_volatile_err(unknown) does not exist +LINE 1: select * from test_volatile_err('dat'); + ^ +HINT: No function matches the given name and argument types. You might need to add explicit type casts. create function test_pseudo_arg_err(dat cstring) returns text as $$ cluster 'testcluster'; $$ language plproxy; -select * from test_pseudo_arg_err(textout('dat')); ERROR: PL/Proxy function public.test_pseudo_arg_err(0): unsupported pseudo type: cstring (2275) +select * from test_pseudo_arg_err(textout('dat')); +ERROR: function test_pseudo_arg_err(cstring) does not exist +LINE 1: select * from test_pseudo_arg_err(textout('dat')); + ^ +HINT: No function matches the given name and argument types. You might need to add explicit type casts. create function test_pseudo_ret_err(dat text) returns cstring as $$ cluster 'testcluster'; $$ language plproxy; +-- not detected in validator select * from test_pseudo_ret_err('dat'); ERROR: PL/Proxy function public.test_pseudo_ret_err(0): unsupported pseudo type: cstring (2275) create function test_runonall_err(dat text) @@ -98,5 +119,9 @@ as $$ cluster 'testcluster'; run on all; $$ language plproxy; -select * from test_runonall_err('dat'); ERROR: PL/Proxy function public.test_runonall_err(1): RUN ON ALL requires set-returning function +select * from test_runonall_err('dat'); +ERROR: function test_runonall_err(unknown) does not exist +LINE 1: select * from test_runonall_err('dat'); + ^ +HINT: No function matches the given name and argument types. You might need to add explicit type casts. diff --git a/test/expected/plproxy_select.out b/test/expected/plproxy_select.out index 55f0e1b..4b67e53 100644 --- a/test/expected/plproxy_select.out +++ b/test/expected/plproxy_select.out @@ -33,8 +33,12 @@ returns integer as $$ select id from sel_test where username = xuser; select id from sel_test where username = xuser; $$ language plproxy; -select * from test_select_err('user', true); ERROR: PL/Proxy function public.test_select_err(2): Compile error at line 5: Only one SELECT statement allowed +select * from test_select_err('user', true); +ERROR: function test_select_err(unknown, boolean) does not exist +LINE 1: select * from test_select_err('user', true); + ^ +HINT: No function matches the given name and argument types. You might need to add explicit type casts. create function get_zero() returns setof integer as $x$ cluster 'testcluster'; diff --git a/test/expected/plproxy_split.out b/test/expected/plproxy_split.out index 6f5e017..7794fd4 100644 --- a/test/expected/plproxy_split.out +++ b/test/expected/plproxy_split.out @@ -31,23 +31,39 @@ $$ language sql; -- invalid arg reference create or replace function test_array(a text[], b text[], c text) returns setof text as $$ split $4; cluster 'testcluster'; run on 0;$$ language plproxy; -select * from test_array(array['a'], array['g'], 'foo'); ERROR: PL/Proxy function public.test_array(3): Compile error at line 1: invalid argument reference: $4 +select * from test_array(array['a'], array['g'], 'foo'); +ERROR: function test_array(text[], text[], unknown) does not exist +LINE 1: select * from test_array(array['a'], array['g'], 'foo'); + ^ +HINT: No function matches the given name and argument types. You might need to add explicit type casts. -- invalid arg name create or replace function test_array(a text[], b text[], c text) returns setof text as $$ split x; cluster 'testcluster'; run on 0; $$ language plproxy; -select * from test_array(array['a'], array['b', 'c'], 'foo'); ERROR: PL/Proxy function public.test_array(3): Compile error at line 1: invalid argument reference: x +select * from test_array(array['a'], array['b', 'c'], 'foo'); +ERROR: function test_array(text[], text[], unknown) does not exist +LINE 1: select * from test_array(array['a'], array['b', 'c'], 'foo')... + ^ +HINT: No function matches the given name and argument types. You might need to add explicit type casts. -- cannot split more than once create or replace function test_array(a text[], b text[], c text) returns setof text as $$ split a, b, b; cluster 'testcluster'; run on 0; $$ language plproxy; -select * from test_array(array['a'], array['b', 'c'], 'foo'); ERROR: PL/Proxy function public.test_array(3): SPLIT parameter specified more than once: b +select * from test_array(array['a'], array['b', 'c'], 'foo'); +ERROR: function test_array(text[], text[], unknown) does not exist +LINE 1: select * from test_array(array['a'], array['b', 'c'], 'foo')... + ^ +HINT: No function matches the given name and argument types. You might need to add explicit type casts. -- attempt to split non-array create or replace function test_array(a text[], b text[], c text) returns setof text as $$ split $3; cluster 'testcluster'; run on 0;$$ language plproxy; -select * from test_array(array['a'], array['g'], 'foo'); ERROR: PL/Proxy function public.test_array(3): SPLIT parameter is not an array: $3 +select * from test_array(array['a'], array['g'], 'foo'); +ERROR: function test_array(text[], text[], unknown) does not exist +LINE 1: select * from test_array(array['a'], array['g'], 'foo'); + ^ +HINT: No function matches the given name and argument types. You might need to add explicit type casts. -- array size/dimensions mismatch create or replace function test_array(a text[], b text[], c text) returns setof text as $$ split a, b; cluster 'testcluster'; run on 0; $$ language plproxy; diff --git a/test/expected/plproxy_target.out b/test/expected/plproxy_target.out index 2c5c7c8..3b442cc 100644 --- a/test/expected/plproxy_target.out +++ b/test/expected/plproxy_target.out @@ -27,8 +27,12 @@ returns text as $$ target test_target_dst; target test_target_dst; $$ language plproxy; -select * from test_target_err1('asd'); ERROR: PL/Proxy function public.test_target_err1(1): Compile error at line 5: Only one TARGET statement allowed +select * from test_target_err1('asd'); +ERROR: function test_target_err1(unknown) does not exist +LINE 1: select * from test_target_err1('asd'); + ^ +HINT: No function matches the given name and argument types. You might need to add explicit type casts. create function test_target_err2(xuser text) returns text as $$ cluster 'testcluster'; @@ -36,5 +40,9 @@ returns text as $$ target test_target_dst; select 1; $$ language plproxy; -select * from test_target_err2('asd'); ERROR: PL/Proxy function public.test_target_err2(1): Compile error at line 6: TARGET cannot be used with SELECT +select * from test_target_err2('asd'); +ERROR: function test_target_err2(unknown) does not exist +LINE 1: select * from test_target_err2('asd'); + ^ +HINT: No function matches the given name and argument types. You might need to add explicit type casts. diff --git a/test/sql/plproxy_errors.sql b/test/sql/plproxy_errors.sql index 09f85a2..724ccb6 100644 --- a/test/sql/plproxy_errors.sql +++ b/test/sql/plproxy_errors.sql @@ -83,6 +83,7 @@ returns cstring as $$ cluster 'testcluster'; $$ language plproxy; +-- not detected in validator select * from test_pseudo_ret_err('dat'); create function test_runonall_err(dat text) -- 2.39.5