From 133adb5154bcdbf8509820a03065d0d33c7aace0 Mon Sep 17 00:00:00 2001 From: Tanzeel Khan <140405735+tanscorpio7@users.noreply.github.com> Date: Tue, 17 Oct 2023 15:49:40 +0530 Subject: [PATCH] Schema Resolution for multiple functions in single stmt (#227) In SQL Server the default schema inside functions is same as functions schema, followed by the actual query default schema. We previously handled this behaviour on a statement level which worked correctly when only one schema & one function was used in the statement. When multiple functions and schemas are specified or the schema is specified with some other object along with a non schema specified function, this approach becomes ineffective, since we would resolve everything to the one schema. To solve this we must resolve the schema (pg search path) at function execution level. We should only do this for user defined functions. All procedures & triggers will be excluded from the change since their schema resolution follows different strategy and is already implemented else where in the code. Cross DB functions call are not allowed in babelfish, so need not to look at those cases. System defined functions (sys, pg_catalog, ...) should be excluded from this change since they are expected to run in postgres environment. The implementation uses the SET option available for PG function definitions. This SET cmd is picked up from PG catalog and locally set at run time(only for function execution). We do not store this information in the catalog itself. Instead we search the function owner schema and call the SET command during function initialisation. But instead of adding the new search path to proconfig attribute of functions, we set it directly using the search_path global variable, to avoid performance drop. We also store the new search path in fcache->prosearchpath to avoid multiple calls. And we skip setting search path if language is not pltsql ex: Select s2.f(); s2.f() { -- first we search objects in s2 -- then we search in database default schema s2.dbo } Extension PR: https://github.com/babelfish-for-postgresql/babelfish_extensions/pull/1885 Issues Resolved:BABEL-4442 Signed-off-by: Tanzeel Khan --- src/backend/utils/fmgr/fmgr.c | 57 +++++++++++++++++++++++++++-------- src/include/fmgr.h | 3 ++ 2 files changed, 48 insertions(+), 12 deletions(-) diff --git a/src/backend/utils/fmgr/fmgr.c b/src/backend/utils/fmgr/fmgr.c index 222755da691..b100619ac78 100644 --- a/src/backend/utils/fmgr/fmgr.c +++ b/src/backend/utils/fmgr/fmgr.c @@ -45,6 +45,7 @@ PGDLLIMPORT fmgr_hook_type fmgr_hook = NULL; PGDLLIMPORT non_tsql_proc_entry_hook_type non_tsql_proc_entry_hook = NULL; PGDLLIMPORT get_func_language_oids_hook_type get_func_language_oids_hook = NULL; PGDLLIMPORT pgstat_function_wrapper_hook_type pgstat_function_wrapper_hook = NULL; +set_local_schema_for_func_hook_type set_local_schema_for_func_hook; /* * Hashtable for fast lookup of external C functions @@ -673,6 +674,7 @@ struct fmgr_security_definer_cache Datum arg; /* passthrough argument for plugin modules */ Oid pronamespace; char prokind; + char *prosearchpath; Oid prolang; Oid sys_nspoid; }; @@ -707,6 +709,17 @@ fmgr_security_definer(PG_FUNCTION_ARGS) int non_tsql_proc_count = 0; void *newextra = NULL; char *cacheTupleProcname = NULL; + char *old_search_path = NULL; + + if (get_func_language_oids_hook) + get_func_language_oids_hook(&pltsql_lang_oid, &pltsql_validator_oid); + else + { + pltsql_lang_oid = InvalidOid; + pltsql_validator_oid = InvalidOid; + } + + set_sql_dialect = pltsql_lang_oid != InvalidOid; if (!fcinfo->flinfo->fn_extra) { @@ -739,6 +752,19 @@ fmgr_security_definer(PG_FUNCTION_ARGS) fcache->prolang = procedureStruct->prolang; fcache->pronamespace = procedureStruct->pronamespace; fcache->sys_nspoid = InvalidOid; + if((set_sql_dialect && (fcache->prolang == pltsql_lang_oid || fcache->prolang == pltsql_validator_oid)) + && strcmp(format_type_be(procedureStruct->prorettype), "trigger") != 0 + && fcache->prokind == PROKIND_FUNCTION) + { + char *new_search_path = NULL; + new_search_path = (*set_local_schema_for_func_hook)(fcache->pronamespace); + if(new_search_path) + { + oldcxt = MemoryContextSwitchTo(fcinfo->flinfo->fn_mcxt); + fcache->prosearchpath = pstrdup(new_search_path); + MemoryContextSwitchTo(oldcxt); + } + } datum = SysCacheGetAttr(PROCOID, tuple, Anum_pg_proc_proconfig, &isnull); @@ -756,17 +782,6 @@ fmgr_security_definer(PG_FUNCTION_ARGS) else fcache = fcinfo->flinfo->fn_extra; - if (get_func_language_oids_hook) - get_func_language_oids_hook(&pltsql_lang_oid, &pltsql_validator_oid); - else - { - pltsql_lang_oid = InvalidOid; - pltsql_validator_oid = InvalidOid; - } - - // get_language_procs("pltsql", &pltsql_lang_oid, &pltsql_validator_oid); - set_sql_dialect = pltsql_lang_oid != InvalidOid; - /* GetUserIdAndSecContext is cheap enough that no harm in a wasted call */ GetUserIdAndSecContext(&save_userid, &save_sec_context); if (fcache->proconfig) /* Need a new GUC nesting level */ @@ -786,6 +801,13 @@ fmgr_security_definer(PG_FUNCTION_ARGS) GUC_ACTION_SAVE); } + if (fcache->prosearchpath) + { + old_search_path = namespace_search_path; + namespace_search_path = fcache->prosearchpath; + assign_search_path(fcache->prosearchpath, newextra); + } + if (set_sql_dialect && IsTransactionState()) { if ((fcache->prolang == pltsql_lang_oid) || (fcache->prolang == pltsql_validator_oid)) @@ -881,6 +903,12 @@ fmgr_security_definer(PG_FUNCTION_ARGS) sql_dialect = sql_dialect_value_old; assign_sql_dialect(sql_dialect_value_old, newextra); } + + if (old_search_path) + { + namespace_search_path = old_search_path; + assign_search_path(old_search_path, newextra); + } PG_RE_THROW(); } @@ -888,9 +916,14 @@ fmgr_security_definer(PG_FUNCTION_ARGS) fcinfo->flinfo = save_flinfo; - if (set_sql_dialect) + if (old_search_path) { + namespace_search_path = old_search_path; + assign_search_path(old_search_path, newextra); + } + if (set_sql_dialect) + { sql_dialect = sql_dialect_value_old; assign_sql_dialect(sql_dialect_value_old, newextra); diff --git a/src/include/fmgr.h b/src/include/fmgr.h index 205b004b513..e8dd3ae9e77 100644 --- a/src/include/fmgr.h +++ b/src/include/fmgr.h @@ -778,6 +778,9 @@ typedef void (*non_tsql_proc_entry_hook_type) (int, int); typedef void (*get_func_language_oids_hook_type)(Oid *, Oid *); +typedef char *(*set_local_schema_for_func_hook_type) (Oid proc_nsp_oid); +extern set_local_schema_for_func_hook_type set_local_schema_for_func_hook; + extern PGDLLIMPORT needs_fmgr_hook_type needs_fmgr_hook; extern PGDLLIMPORT fmgr_hook_type fmgr_hook; extern PGDLLIMPORT non_tsql_proc_entry_hook_type non_tsql_proc_entry_hook;