Bug 355 - execute script () modifies search_path
Summary: execute script () modifies search_path
Status: ASSIGNED
Alias: None
Product: Slony-I
Classification: Unclassified
Component: slonik (show other bugs)
Version: devel
Hardware: All All
: low enhancement
Assignee: Christopher Browne
URL:
Depends on:
Blocks:
 
Reported: 2014-11-07 13:21 UTC by Pawel Rybak
Modified: 2014-12-17 14:47 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Pawel Rybak 2014-11-07 13:21:41 UTC
If there is a
set search_path to ...
statement in script provided to execute script function, the setting remains after slonik finishes.

It manifests itself as a problem when there is a replicated table with a trigger set to enable always on it.
On a replica such a trigger will execute function with search_path set by the last dll ran by slonik's execute script function.
Comment 1 Christopher Browne 2014-11-12 13:27:19 UTC
The stored function should be protecting itself; if it depends on having a particular search path, it should do so as part of the stored function definition.

So it seems to me that there's a bug in the stored function.  (And I suspect I myself might very well get assigned to fix that bug...)

Slony itself operates relatively safely as it specifies fully qualified object names in most (hard to prove "all") cases.

It seems like it would be a good idea for the code that runs at the end of DDL processing (see src/backend/slony1_funcs.c) to add in "RESET ALL;" to reset the environment at the end of DDL processing in case search paths or other GUCs got messed with by DDL.

There's a further bit of risk; if a DDL script is messing around with GUCs, that risks messing with another DDL script that might be run in the same SYNC.  It seems a good idea to add a warning to users that messing around with GUCs in DDL could injure other users...
Comment 2 Christopher Browne 2014-12-17 14:01:41 UTC
In order for the DDL to be processed consistently, we will need to run RESET ALL after execution of each statement on the origin, too, otherwise it will run differently on origin-versus-subscribers.
Comment 3 Christopher Browne 2014-12-17 14:47:56 UTC
Strike that, RESET ALL resets session_replication_role, so we'd have to re-set that after every statement.

I'm going to propose just the documentation change to indicate that "best practice" is that functions need to have search_path expressly set as part of the function definition.