Fix query cache invalidation bug.

When an execute message is received, pgpool checks its max number of
rows paramter. If it's not zero, pgpool sets "partial_fetch" flag to
instruct pool_handle_query_cache() to not create query cache.  Problem
is, commit 2a99aa5d1 missed that even INSERT/UPDATE/DELETE sets the
execute message parameter to non 0 (mostly 1) and pgpool set the flag
for even none SELECTs. This resulted in failing to invalidate query
cache because if the flag is true, subsequent code in
pool_handle_query_cache() skips cache invalidation.  It was an
oversight in this commit (my fault):
https://git.postgresql.org/gitweb/?p=pgpool2.git;a=commit;h=2a99aa5d1910f1fd4855c8eb6751a26cbaa5e48d

To fix this change Execute() to check if the query is read only SELECT
before setting the flag.

Also add test to 006.memqcache.

Problem reported by and a test program provided by Achilleas Mantzios <a.mantzios@cloud.gatewaynet.com>.

Discussion: [pgpool-general: 9427] Clarification on query results cache visibility
https://www.pgpool.net/pipermail/pgpool-general/2025-April/009430.html

Backpatch-through: v4.2

diff --git a/src/protocol/pool_proto_modules.c b/src/protocol/pool_proto_modules.c
index 59647fe8108c9e94de609b80c7617e396a0d8aba..5b3e03d5959b5207e8db09304c9e34f09b61c39e 100644 (file)
--- a/src/protocol/pool_proto_modules.c
+++ b/src/protocol/pool_proto_modules.c
@@ -3,7 +3,7 @@
  * pgpool: a language independent connection pool server for PostgreSQL
  * written by Tatsuo Ishii
  *
- * Copyright (c) 2003-2024     PgPool Global Development Group
+ * Copyright (c) 2003-2025     PgPool Global Development Group
  *
  * Permission to use, copy, modify, and distribute this software and
  * its documentation for any purpose and without fee is hereby
@@ -950,6 +950,9 @@ Execute(POOL_CONNECTION * frontend, POOL_CONNECTION_POOL * backend,
        /* obtain number of returning rows */
        p = contents + strlen(contents) + 1;
        memcpy(&num_rows, p, sizeof(num_rows));
+       num_rows = ntohl(num_rows);
+       ereport(DEBUG1,
+                       (errmsg("Execute: num_rows: %d", num_rows)));
 
        bind_msg = pool_get_sent_message('B', contents, POOL_SENT_MESSAGE_CREATED);
        if (!bind_msg)
@@ -981,11 +984,12 @@ Execute(POOL_CONNECTION * frontend, POOL_CONNECTION_POOL * backend,
        query = bind_msg->query_context->original_query;
 
        /*
-        * If execute message's parameter is not 0, set partial_fetch flag to true
-        * so that subsequent execute message knows that the portal started with
-        * partial fetching.
+        * If execute message's parameter is not 0 and the query is cache safe
+        * (i.e. read only SELECT), set partial_fetch flag to true so that
+        * subsequent execute message knows that the portal started with partial
+        * fetching.
         */
-       if (num_rows != 0)
+       if (num_rows != 0 && query_context->is_cache_safe && !query_cache_disabled())
        {
                query_context->partial_fetch = true;
                elog(DEBUG1, "set partial_fetch in execute");

diff --git a/src/test/regression/tests/006.memqcache/expected.5 b/src/test/regression/tests/006.memqcache/expected.5
new file mode 100644 (file)
index 0000000..cd425f0
--- /dev/null
+++ b/src/test/regression/tests/006.memqcache/expected.5
@@ -0,0 +1,37 @@
+FE=> Query (query="CREATE TABLE regress_test(i int)")
+<= BE CommandComplete(CREATE TABLE)
+<= BE ReadyForQuery(I)
+FE=> Query (query="INSERT INTO regress_test VALUES(1)")
+<= BE CommandComplete(INSERT 0 1)
+<= BE ReadyForQuery(I)
+FE=> Parse(stmt="S0", query="SELECT * FROM regress_test WHERE i = 1")
+FE=> Bind(stmt="S0", portal="P0")
+FE=> Describe(portal="P0")
+FE=> Execute(portal="P0")
+FE=> Sync
+<= BE ParseComplete
+<= BE BindComplete
+<= BE RowDescription
+<= BE DataRow
+<= BE CommandComplete(SELECT 1)
+<= BE ReadyForQuery(I)
+FE=> Parse(stmt="S2", query="UPDATE regress_test SET i = 2")
+FE=> Bind(stmt="S2", portal="P2")
+FE=> Execute(portal="P2")
+FE=> Sync
+<= BE ParseComplete
+<= BE BindComplete
+<= BE CommandComplete(UPDATE 1)
+<= BE ReadyForQuery(I)
+FE=> Bind(stmt="S0", portal="P0")
+FE=> Describe(portal="P0")
+FE=> Execute(portal="P0")
+FE=> Sync
+<= BE BindComplete
+<= BE RowDescription
+<= BE CommandComplete(SELECT 0)
+<= BE ReadyForQuery(I)
+FE=> Query (query="DROP TABLE regress_test")
+<= BE CommandComplete(DROP TABLE)
+<= BE ReadyForQuery(I)
+FE=> Terminate

diff --git a/src/test/regression/tests/006.memqcache/query_cache_bug5.data b/src/test/regression/tests/006.memqcache/query_cache_bug5.data
new file mode 100644 (file)
index 0000000..f013f11
--- /dev/null
+++ b/src/test/regression/tests/006.memqcache/query_cache_bug5.data
@@ -0,0 +1,37 @@
+# Execute UPDATE to check if the query cache is invalidated.
+
+# create a test table "regress_test".
+'Q'    "CREATE TABLE regress_test(i int)"
+'Y'
+'Q'    "INSERT INTO regress_test VALUES(1)"
+'Y'
+
+'P'    "S0"    "SELECT * FROM regress_test WHERE i = 1"        0
+'B'    "P0"    "S0"    0       0       0   
+'D'    'P'     "P0"
+# This SELECT is expected to return 1 row and it should have created
+# query cache.
+'E'    "P0"    0
+'S'
+'Y'
+
+'P'    "S2"    "UPDATE regress_test SET i = 2" 0
+'B'    "P2"    "S2"    0       0       0
+# Set "maxrows" parameter to 1 of this execute message to trigger bug.
+'E'    "P2"    1
+'S'
+'Y'
+
+'B'    "P0"    "S0"    0       0       0
+'D'    'P'     "P0"
+# This SELECT is expected to return 0 row because previous UPDATE
+# should have deleted the query cache.
+'E'    "P0"    0
+'S'
+'Y'
+
+# drop the test table.
+'Q'    "DROP TABLE regress_test"
+'Y'
+
+'X'

diff --git a/src/test/regression/tests/006.memqcache/test.sh b/src/test/regression/tests/006.memqcache/test.sh
index 04f1ba13139692ba55a97825fe5252eb32b4a87d..b674fdf70e04f9534662c76a9a8ccacdfca46519 100755 (executable)
--- a/src/test/regression/tests/006.memqcache/test.sh
+++ b/src/test/regression/tests/006.memqcache/test.sh
@@ -540,7 +540,7 @@ echo "done."
 echo "memory_cache_enabled = on" >> etc/pgpool.conf
 cd ..
 
-for i in 1 2 3 4 4
+for i in 1 2 3 4 4 5
 do
     #
     # case 1: failed with kind mismatch error at #5.
@@ -553,7 +553,15 @@ do
     # case 4: various cases including portal suspended
     # Note that case4 is executed twice to make sure that
     # the test works for either query cache exists or does not exist
+    #
+    # case 5: simple cache invalidation test.
     cd $TESTDIR
+
+    # case 5 includes UPDATE, and we want the result without disturbed
+    # by replication delay.
+    if [ $i = 5 ];then
+       echo "backend_weight1 = 0" >> etc/pgpool.conf
+    fi
     ./startall
     wait_for_pgpool_startup
     timeout 1 $PGPROTO -d test -f ../query_cache_bug$i.data |& del_details_from_error > result