Fix kind mimatch error with DEALLOCATE
authorTatsuo Ishii <ishii@sraoss.co.jp>
Tue, 31 Jan 2023 09:49:48 +0000 (18:49 +0900)
committerTatsuo Ishii <ishii@sraoss.co.jp>
Tue, 31 Jan 2023 09:49:48 +0000 (18:49 +0900)
When conditions below are all met:
- streaming replication mode
- load balance node is other than primary
- PREPARE is used in a multi-statement query

Kind mimatch error occurs.

For DEALLOCATE pool_where_to_send() sets the nodes to be sent to all
backend if pgpool failed to find a prepared statement previously
received. For example with "SELECT 1\;PREPARE foo;", pgpool ignores
"PREPARE" part and just sends the whole multi-statement query to
primary. So primary actually has the prepared statement "foo" but
pgpool thinks that there's no prepared statement named "foo". And
pgpool sends DEALLOCATE to both primary and standby, then a kind
mismatch error raised. Fix is, just sending DEALLOCATE to primary node
in this case if pgpool is in streaming replication mode.  Same thing
can be said to EXECUTE too. I fixed this and merge similar treatment
with EXECUTE into where_to_send_deallocate() to make the code simpler.

I also found another bug: in replication mode or SI mode, pgpool needs
to send multi-statement query to all backend because the
multi-statement query maybe a write query. However pgpool sends to
main node only in this case.

Test cases are added to 071..execute_and_deallocate.

Backpatch-through: 4.0

Problem reported in:
https://www.pgpool.net/mantisbt/view.php?id=780

src/context/pool_query_context.c
src/test/regression/tests/071.execute_and_deallocate/test.sh

index 2cf51326e2983c433d15e11874354a2ad3145631..6df01213b5181482065f141c55d7bfe1cc056f78 100644 (file)
@@ -4,7 +4,7 @@
  * pgpool: a language independent connection pool server for PostgreSQL
  * written by Tatsuo Ishii
  *
- * Copyright (c) 2003-2020     PgPool Global Development Group
+ * Copyright (c) 2003-2023     PgPool Global Development Group
  *
  * Permission to use, copy, modify, and distribute this software and
  * its documentation for any purpose and without fee is hereby
@@ -646,6 +646,10 @@ pool_where_to_send(POOL_QUERY_CONTEXT * query_context, char *query, Node *node)
                        }
                }
        }
+       else if (REPLICATION && query_context->is_multi_statement)
+       {
+               pool_setall_node_to_be_sent(query_context);
+       }
        else if (REPLICATION)
        {
                if (pool_config->load_balance_mode &&
@@ -756,24 +760,9 @@ pool_where_to_send(POOL_QUERY_CONTEXT * query_context, char *query, Node *node)
        }
 
        /*
-        * EXECUTE?
-        */
-       if (IsA(node, ExecuteStmt))
-       {
-               POOL_SENT_MESSAGE *msg;
-
-               msg = pool_get_sent_message('Q', ((ExecuteStmt *) node)->name, POOL_SENT_MESSAGE_CREATED);
-               if (!msg)
-                       msg = pool_get_sent_message('P', ((ExecuteStmt *) node)->name, POOL_SENT_MESSAGE_CREATED);
-               if (msg)
-                       pool_copy_prep_where(msg->query_context->where_to_send,
-                                                                query_context->where_to_send);
-       }
-
-       /*
-        * DEALLOCATE?
+        * DEALLOCATE or EXECUTE?
         */
-       else if (IsA(node, DeallocateStmt))
+       if (IsA(node, DeallocateStmt) || IsA(node, ExecuteStmt))
        {
                where_to_send_deallocate(query_context, node);
        }
@@ -1520,26 +1509,55 @@ static POOL_DEST send_to_where(Node *node, char *query)
        return POOL_PRIMARY;
 }
 
+/*
+ * Decide where to send given message.
+ * "node" must be a parse tree of either DEALLOCATE or EXECUTE.
+ */
 static
 void
 where_to_send_deallocate(POOL_QUERY_CONTEXT * query_context, Node *node)
 {
-       DeallocateStmt *d = (DeallocateStmt *) node;
+       DeallocateStmt *d = NULL;
+       ExecuteStmt *e = NULL;
+       char            *name;
        POOL_SENT_MESSAGE *msg;
 
+       if (IsA(node, DeallocateStmt))
+       {
+               d = (DeallocateStmt *) node;
+               name = d->name;
+       }
+       else if (IsA(node, ExecuteStmt))
+       {
+               e = (ExecuteStmt *) node;
+               name = e->name;
+       }
+       else
+       {
+               ereport(ERROR,
+                               (errmsg("invalid node type for where_to_send_deallocate")));
+               return;
+       }
+
        /* DEALLOCATE ALL? */
-       if (d->name == NULL)
+       if (d && (name == NULL))
        {
+               /* send to all backend node */
                pool_setall_node_to_be_sent(query_context);
+               return;
        }
+
+       /* ordinary DEALLOCATE or EXECUTE */
        else
        {
-               msg = pool_get_sent_message('Q', d->name, POOL_SENT_MESSAGE_CREATED);
+               /* check if message was created by SQL PREPARE */
+               msg = pool_get_sent_message('Q', name, POOL_SENT_MESSAGE_CREATED);
                if (!msg)
-                       msg = pool_get_sent_message('P', d->name, POOL_SENT_MESSAGE_CREATED);
+                       /* message may be created by Parse message */
+                       msg = pool_get_sent_message('P', name, POOL_SENT_MESSAGE_CREATED);
                if (msg)
                {
-                       /* Inherit same map from PREPARE or PARSE */
+                       /* Inherit same map from PREPARE or Parse */
                        pool_copy_prep_where(msg->query_context->where_to_send,
                                                                 query_context->where_to_send);
 
@@ -1547,8 +1565,33 @@ where_to_send_deallocate(POOL_QUERY_CONTEXT * query_context, Node *node)
                        query_context->load_balance_node_id = msg->query_context->load_balance_node_id;
                }
                else
-                       /* prepared statement was not found */
-                       pool_setall_node_to_be_sent(query_context);
+               {
+                       /*
+                        * prepared statement was not found.
+                        * There are two cases when this could happen.
+                        * (1) mistakes by client. In this case backend will return ERROR
+                        * anyway.
+                        * (2) previous query was issued as multi-statement query. e.g.
+                        * SELECT 1\;PREPARE foo AS SELECT 1;
+                        * In this case pgpool does not know anything about the prepared
+                        * statement "foo".
+                        */
+                       if (SL_MODE)
+                       {
+                               /*
+                                * In streaming replication or logical replication, sent to
+                                * primary node only.
+                                */
+                               pool_set_node_to_be_sent(query_context, PRIMARY_NODE_ID);
+                       }
+                       else
+                       {
+                               /*
+                                * In other mode, sent to all node.
+                                */
+                               pool_setall_node_to_be_sent(query_context);
+                       }
+               }
        }
 }
 
index 99b46900b3992e28513593f088488522514de68b..08b3cb7b95730944fc72afa788ad7171f6ceb05f 100755 (executable)
@@ -10,9 +10,9 @@ for mode in s r i n
 do
     echo "=== starting test in \"$mode\" mode ==="
     if [ $mode = "n" ];then
-       num_tests=5
-    else
        num_tests=6
+    else
+       num_tests=7
     fi
     success_count=0
 
@@ -52,10 +52,17 @@ PREPARE test2 AS UPDATE test_tbl SET id =2;
 EXECUTE test2;
 DEALLOCATE test2;
 DEALLOCATE all;
+EOF
+
+    # run test3 multi-statement
+    $PSQL -p 11000 test <<EOF
+SELECT 1\;PREPARE test3 AS SELECT 2;
+DEALLOCATE test3;
 EOF
 
     expect1=`fgrep "PREPARE test1" log/pgpool.log  | awk '{print substr($0, index($0, "DB node id:"),13)}'`
     expect2=`fgrep "PREPARE test2" log/pgpool.log  | awk '{print substr($0, index($0, "DB node id:"),13)}'`
+    expect3=`fgrep "PREPARE test3" log/pgpool.log  | awk '{print substr($0, index($0, "DB node id:"),13)}'`
 
     #test1 result
     echo -n "case 1: PREPARE and EXECUTE with SELECT query..."
@@ -105,7 +112,6 @@ EOF
        echo "failed."
     fi
 
-    echo -n "case 6: node1 DEALLOCATE all query..."
     if [ $mode = "n" ];then
        echo "this test is not applied to mode \"$mode\" and skipped."
     else
@@ -118,7 +124,15 @@ EOF
        fi
     fi
 
-    echo "In mode \"$mode\" out of $success_count, $num_tests cases succeeded."
+    # DEALLOCATE in multi-statement
+    echo -n "case 6: DEALLOCATE in multi-statement..."
+    result=`fgrep "DEALLOCATE test3" log/pgpool.log  | awk '{print substr($0, index($0, "DB node id:"),13)}'`
+    if [  "$expect3" = "$result" ]; then
+        success_count=$(( success_count + 1 ))
+       echo "ok."
+    else
+       echo "failed."
+    fi
 
     ./shutdownall