pgpool2.git / summary / log / commit / refs

commit    6f66cdbf0cc043bdd0f9fe7484fb1de8a99bbea3
Author:   Tatsuo Ishii <ishii@postgresql.org>
Date:     Sun Sep 06 21:33:12 2026 +0000

    Revert "Fix do_query to send sync rather than flush."
    
    This reverts commit e4a3a0c13e4b5e1a015aca3db238b21b88e73e2f.
    
    This breaks implicit transaction behavior created by a pipeline.
    -----------------------------------------------------------------
    $ psql -p 11000 -a -f failure.sql test
    DROP TABLE test;
    DROP TABLE
    -- start a pipeline
    \startpipeline
    -- CREATE a table
    CREATE TABLE test(i int);
    -- SELECT non-existent table, which raises an error,
    -- and aborts the implicit transaction started by the pipeline.
    SELECT * from a;
    -- Recover from the error and closes the implicit transaction.
    \syncpipeline
    \endpipeline
    CREATE TABLE
    psql:failure.sql:11: ERROR:  relation "a" does not exist
    LINE 1: SELECT * from a;
                          ^
    -- Try to SELECT the table created in the previous pipeline.
    -- This should fail because the creation of the table "test" was rollbacked.
    SELECT * from test;
     i
    ---
    (0 rows)
    -----------------------------------------------------------------
    
    In this example, a table named "test" is created in a pipeline.  Then
    a SELECT is executed. Because the SELECT tries to retrieve rows from
    non-existent table "a", it causes an error and a roll back of the
    implicit transaction started by the pipeline. As a result, the table
    "test" created in the pipeline does not exist at the end of the
    pipeline. However, a "sync" message was issued by do_query() while
    obtaining information regarding table "a".  As the sync message caused
    commit of the implicit transaction started by the pipeline, creation
    of "test" was not roll backed by the subsequent erroneous SELECT.
    
    IMO this is a serious data consistency issue because it breaks the
    transaction semantics. If I directly connects to PostgreSQL or pgpool
    by the previous commit, and run the script, it ends up with:
    
    SELECT * from test;
    psql:failure.sql:14: ERROR:  relation "test" does not exist
    LINE 1: SELECT * from test;
                          ^
    which is the expected behavior.
    
    Discussion: https://www.postgresql.org/message-id/20260907.062521.1780975513572548706.ishii@postgresql.org
    Backpatch-through: v4.3


src/protocol/pool_process_query.c | 26 +++++++++++++--------- .../tests/039.log_backend_messages/expected.s | 1 + 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/protocol/pool_process_query.c b/src/protocol/pool_process_query.c index ca01d1ea5..202afa2a3 100644 --- a/src/protocol/pool_process_query.c +++ b/src/protocol/pool_process_query.c @@ -1959,7 +1959,6 @@ do_query(POOL_CONNECTION *backend, char *query, POOL_SELECT_RESULT **result, int int num_close_complete; int state; bool data_pushed; - bool sent_sync = false; /* true if we sent a Sync (not Flush) */ data_pushed = false; @@ -2108,14 +2107,21 @@ do_query(POOL_CONNECTION *backend, char *query, POOL_SELECT_RESULT **result, int pool_write(backend, prepared_name, pname_len); /* - * Send Sync message. If we are in an explicit transaction, sending - * "sync" is safe because it will not break user's unnamed portal. If - * we are not in an explicit transaction, sending a sync message - * closes an unnamed portal. But next user's bind message will create - * the unnamed portal anyway. + * Send sync or flush message. If we are in an explicit transaction, + * sending "sync" is safe because it will not break unnamed portal. + * Also this is desirable because if no user queries are sent after + * do_query(), COMMIT command could cause statement time out, because + * flush message does not clear the alarm for statement time out which + * has been set when do_query() issues query. */ - pool_write(backend, "S", 1); /* send "sync" message */ - sent_sync = true; + if (backend->tstate == 'T') + pool_write(backend, "S", 1); /* send "sync" message */ + else + { + pool_write(backend, "H", 1); /* send "flush" message */ + /* remember that we sent queries but did not send sync message */ + pool_get_session_context(true)->pending_sync_map[backend->db_node_id] = true; + } len = htonl(sizeof(len)); pool_write_and_flush(backend, &len, sizeof(len)); } @@ -2226,7 +2232,7 @@ do_query(POOL_CONNECTION *backend, char *query, POOL_SELECT_RESULT **result, int return; /* If "sync" message was issued, 'Z' is expected. */ - if (doing_extended && sent_sync) + if (doing_extended && backend->tstate == 'T') state |= COMMAND_COMPLETE_RECEIVED; break; @@ -2238,7 +2244,7 @@ do_query(POOL_CONNECTION *backend, char *query, POOL_SELECT_RESULT **result, int * If "sync" message was issued, 'Z' is expected, else we are * done with 'C'. */ - if (!doing_extended || !sent_sync) + if (!doing_extended || backend->tstate != 'T') state |= COMMAND_COMPLETE_RECEIVED; /* diff --git a/src/test/regression/tests/039.log_backend_messages/expected.s b/src/test/regression/tests/039.log_backend_messages/expected.s index a5ffeebec..5877fe361 100644 --- a/src/test/regression/tests/039.log_backend_messages/expected.s +++ b/src/test/regression/tests/039.log_backend_messages/expected.s @@ -88,6 +88,7 @@ FE=> Sync <= BE DataRow <= BE NoticeResponse(S LOG C XX000 M CommandComplete message from backend 1 <= BE CommandComplete(SELECT 3) +<= BE NoticeResponse(S LOG C XX000 M ReadyForQuery message from backend 0 <= BE NoticeResponse(S LOG C XX000 M ReadyForQuery message from backend 1 <= BE ReadyForQuery(I) FE=> Terminate