On 07/22/2013 05:37 PM, Christopher Browne wrote:
> The behaviour should be a bit further different...
>

>
> In master (pre-2.2), we're not explicitly setting the role for DDL; that
> *should* be OK as the reference in dbutil.c is, I think, invoked when
> running a connection to a subscriber.
>
> cbbrowne at cbbrowne ~/P/slony1-engine.master> ack-grep
> session_replication_role
> src/slonik/dbutil.c
> 166: "SET session_replication_role TO local; ");
>
> src/slon/remote_worker.c
> 353: "set session_replication_role = replica; ");
> 4958: "set session_replication_role to replica;\n"


I am a bit unclear why you this this *should* be okay in master? As far 
as I can tell from the above code snippet when the remote_worker calls 
the stored functions to execute the DDL_SCRIPT session replication_role 
is replica this different than when slonik invoked it on the event node 
(where session replication_role was local according to above). The 
DDL_SCRIPT will behave differently with respect to any normal user-added 
triggers on the tables.

I think the above paragraph describes a bug we've introduced into 2.2 
which is different than Robert's issue of a duplicate key error in 2.1

I have changes to the testddl regression test that reproduces this which 
I will attach to a bugzilla bug shortly.

>
> src/slony_logshipper/parser.y
> 327: "set session_replication_role to replica;") < 0)
>
> src/slony_logshipper/scan.l
> 135:session_replication_role { return K_SESSION_ROLE;   }
>
> tools/slony1_dump.sh
> 301:set session_replication_role='replica';
>
> doc/adminguide/triggers.sgml
> 23:<para> A new GUC variable, <envar>session_replication_role</envar>
>
> doc/adminguide/security.sgml
> 51:<para> set the session_replication_role to replica</para>
>
> The equivalent against 2.1 provides something rather more like 2.0, and
> sets the role to "local" appropriately.
>
> Here's what's in 2.1:
>
> cbbrowne at cbbrowne ~/P/s/src> ack-grep session_replication_role
> slonik/dbutil.c
> 166: "SET session_replication_role TO local; ");
>
> slon/remote_worker.c
> 386: "set session_replication_role = replica; ",
> 1474: "set session_replication_role to local; "
> 1543: "set session_replication_role to replica; ",
> 1566:                                                   if
> (archive_append_str(node, "set session_replication_role to local;\n") < 0)
> 1570:                                                   if
> (archive_append_str(node, "set session_replication_role to replica;\n") < 0)
> 5589: "set session_replication_role to replica;\n"
>
> backend/slony1_funcs.sql
> 3360:           execute 'create temp table
> _slony1_saved_session_replication_role (
> 3362:           execute 'insert into _slony1_saved_session_replication_role
> 3364:                                   where name =
> ''session_replication_role'';';
> 3366:                   execute 'set session_replication_role to local;';
> 3400: 'select setting from _slony1_saved_session_replication_role' loop
> 3401:                   v_query := 'set session_replication_role to ' ||
> v_row.setting;
> 3404:           execute 'drop table _slony1_saved_session_replication_role';
>
> slony_logshipper/parser.y
> 327: "set session_replication_role to replica;") < 0)
>
> slony_logshipper/scan.l
> 135:session_replication_role { return K_SESSION_ROLE;   }
>
> The relevant bits in 2.0 seem identical to 2.1.  (My ack-grep run finds
> the very same code.)
>
> It seems to me that we should ensure we have a regression test that does
> both DDL and DML, to make sure that we process both properly, as that
> should work OK.