http://www.slony.info/bugzilla/show_bug.cgi?id=75

           Summary: Bugs: (1) Initialize forgotten cs->plan_active_log. (2)
                    Session_replication_role and "SET datestyle TO 'ISO'" in
                    slonik at connection start.
           Product: Slony-I
           Version: devel
          Platform: PC
        OS/Version: Windows
            Status: NEW
          Severity: critical
          Priority: medium
         Component: slonik
        AssignedTo: slony1-bugs at lists.slony.info
        ReportedBy: dmitry.koterov at gmail.com
                CC: slony1-bugs at lists.slony.info
   Estimated Hours: 0.0


Created an attachment (id=29)
 --> (http://www.slony.info/bugzilla/attachment.cgi?id=29)
Patches for these 2 issues

First, a little patch (1)
-------------------------

--- slony1-2.0.1-orig/src/backend/slony1_funcs.c        2009-02-23
22:09:40.000000000 +0300
+++ slony1-2.0.1/src/backend/slony1_funcs.c     2009-03-08 21:58:38.000000000
+0300
@@ -332,5 +332,5 @@
         * Do the following only once per transaction.
         */
-       if (!TransactionIdEquals(cs->currentXid, newXid))
+       if (!TransactionIdEquals(cs->currentXid, newXid) ||
!cs->plan_active_log)
        {
                int32           log_status;

The patch just adds logical consistence: the code inside this "if" initializes
both cs->currentXid and plan_active_log once per transaction, but checks only
cs->currentXid. Additional check for cs->currentXid is safe and seems to be
reasonable anyway.

(Note that this issue correlate with the second one, so it will break slony if
you will not apply the second patch too. The effect of the second bug is
destroyed by the first bug.)


Second, bug with new session_replication_role and "SET datestyle TO 'ISO'" (2)
------------------------------------------------------------------------------

I detected that logTrigger is NOT turned off during the EXECUTE QUERY in
Slony-I v2.0.1.
Take a look at the following query log produced by slonik:

[postgres at slonopotam_m 1623] LOG:  команда: begin transaction;
[postgres at slonopotam_m 1623] LOG:  команда: SET datestyle TO 'ISO'; SET
session_replication_role TO local;
[postgres at slonopotam_m 1623] LOG:  команда: select version();
[postgres at slonopotam_m 1623] LOG:  команда: rollback transaction;
...
[postgres at slonopotam_m 1623] LOG:  команда: begin transaction;
[postgres at slonopotam_m 1623] LOG:  команда: select
"_sloncluster".ddlScript_prepare(4725, 2736);
[postgres at slonopotam_m 1623] LOG:  команда: UPDATE a SET id_copy = id;
[postgres at slonopotam_m 1623] LOG:  execute <unnamed>: select
"_sloncluster".ddlScript_complete(4725, $1::text, 2736);
[postgres at slonopotam_m 1623] LOG:  команда: commit transaction;
...

You see, the effect of "SET session_replication_role TO local" is NOT visible
within a next transaction, because it is rolled back. This is because
db_connect() executes it on a connection start (correct), but via
db_exec_command (wrong), and db_exec_command begins a transaction at its first
run.

(Frankly, thanks to (1) it is not visible, because cs->plan_active_log is
surprisely null within a logTrigger call. So, logTrigger IS CALLED, but
executes a NULL query which is ignored by Postgres.)

The solution is to perform connection initialization outside a transaction, via
direct PQexec call, not via db_begin_xact().
Here is the patch:

diff -U2 slony1-2.0.1-orig/src/slonik/dbutil.c slony1-2.0.1/src/slonik/dbutil.c
--- slony1-2.0.1-orig/src/slonik/dbutil.c       2008-04-24 00:34:21.000000000
+0400
+++ slony1-2.0.1/src/slonik/dbutil.c    2009-03-08 23:26:18.000000000 +0300
@@ -113,4 +113,5 @@
        PGconn     *dbconn;
        SlonDString     query;
+       PGresult   *res;

        db_notice_stmt = stmt;
@@ -144,10 +145,20 @@

        adminfo->dbconn = dbconn;
-       if (db_exec_command(stmt, adminfo, &query) < 0)
+
+       res = PQexec(adminfo->dbconn, dstring_data(&query));
+       if (PQresultStatus(res) != PGRES_COMMAND_OK &&
+               PQresultStatus(res) != PGRES_TUPLES_OK &&
+               PQresultStatus(res) != PGRES_EMPTY_QUERY)
        {
+               fprintf(stderr, "%s:%d: %s %s - %s",
+                               stmt->stmt_filename, stmt->stmt_lno,
+                               PQresStatus(PQresultStatus(res)),
+                               dstring_data(&query),
PQresultErrorMessage(res));
                printf("Unable to set datestyle\n");
+               PQclear(res);
                dstring_free(&query);
                return -1;
        }
+       PQclear(res);
        dstring_free(&query);
        return 0;




P.S.
Why postgres rolls back session variable assigned via "SET
session_replication_role TO local"? Because it is the standard behaviour, and
here is an illustration:

postgres=# show session_replication_role;
--> origin
postgres=# begin;
--> BEGIN
postgres=# set session_replication_role to replica;
--> SET
postgres=# show session_replication_role;
--> replica
postgres=# rollback;
--> ROLLBACK
postgres=# show se ssion_replication_role;
--> origin


-- 
Configure bugmail: http://www.slony.info/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.
You are the assignee for the bug.