Fix long standing bind bug with query cache.

When a named statement is prepared, it is possible to bind then
execute without a parse message. Problem is, table oids which are
necessary to invalidate query cache at execute or COMMIT was collected
only in parse messages process (Parse()). Thus if bind is executed
without parse after previous execute, no table oids were collected,
and pgpool failed to invalidate query cache.

Fix is collecting table oids at bind time too.

Add regression test to 006.memqcache.

Problem reported by and 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 5b3e03d5959b5207e8db09304c9e34f09b61c39e..d793572114f974f6cb48757980cbf461526c7c61 100644 (file)
--- a/src/protocol/pool_proto_modules.c
+++ b/src/protocol/pool_proto_modules.c
@@ -1779,6 +1779,33 @@ Bind(POOL_CONNECTION * frontend, POOL_CONNECTION_POOL * backend,
                                                        query_context->parse_tree);
        }
 
+       /*
+        * If query cache is enabled and we updating table, save table oids. It
+        * maybe possible that Parse() already collected the information but if
+        * Bind is called without prior parse (this could happen if named prepared
+        * statement is used), we miss the information. So we re-collect the
+        * information here. This could result in duplicate oids in oid table, but
+        * it's Okay. The duplication will be ignored anyway.
+        */
+       if (pool_config->memory_cache_enabled)
+       {
+               /*
+                * Save table oids if we are updating the table.
+                */
+               if (!query_context->is_parse_error)
+               {
+                       int                     num_oids;
+                       int                *oids;
+                       int                     i;
+
+                       num_oids = pool_extract_table_oids(query_context->parse_tree,
+                                                                                          &oids);
+                       /* Save to oid buffer */
+                       for (i = 0; i < num_oids; i++)
+                                       pool_add_dml_table_oid(oids[i]);
+               }
+       }
+
        /*
         * Start a transaction if necessary in replication mode
         */

diff --git a/src/test/regression/tests/006.memqcache/expected.6 b/src/test/regression/tests/006.memqcache/expected.6
new file mode 100644 (file)
index 0000000..83440bb
--- /dev/null
+++ b/src/test/regression/tests/006.memqcache/expected.6
@@ -0,0 +1,38 @@
+FE=> Query (query="CREATE TABLE regress_test(i int)")
+<= BE CommandComplete(CREATE TABLE)
+<= BE ReadyForQuery(I)
+FE=> Parse(stmt="S1", query="INSERT INTO regress_test VALUES(1)")
+FE=> Bind(stmt="S1", portal="")
+FE=> Execute(portal="")
+FE=> Sync
+<= BE ParseComplete
+<= BE BindComplete
+<= BE CommandComplete(INSERT 0 1)
+<= BE ReadyForQuery(I)
+FE=> Parse(stmt="S2", query="SELECT * FROM regress_test WHERE i = 1")
+FE=> Bind(stmt="S2", portal="")
+FE=> Execute(portal="")
+FE=> Sync
+<= BE ParseComplete
+<= BE BindComplete
+<= BE DataRow
+<= BE CommandComplete(SELECT 1)
+<= BE ReadyForQuery(I)
+FE=> Bind(stmt="S1", portal="")
+FE=> Execute(portal="")
+FE=> Sync
+<= BE BindComplete
+<= BE CommandComplete(INSERT 0 1)
+<= BE ReadyForQuery(I)
+FE=> Bind(stmt="S2", portal="")
+FE=> Execute(portal="")
+FE=> Sync
+<= BE BindComplete
+<= BE DataRow
+<= BE DataRow
+<= BE CommandComplete(SELECT 2)
+<= 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/expected.7 b/src/test/regression/tests/006.memqcache/expected.7
new file mode 100644 (file)
index 0000000..7304ce2
--- /dev/null
+++ b/src/test/regression/tests/006.memqcache/expected.7
@@ -0,0 +1,54 @@
+FE=> Query (query="CREATE TABLE regress_test(i int)")
+<= BE CommandComplete(CREATE TABLE)
+<= BE ReadyForQuery(I)
+FE=> Parse(stmt="S1", query="INSERT INTO regress_test VALUES(1)")
+FE=> Bind(stmt="S1", portal="")
+FE=> Execute(portal="")
+FE=> Sync
+<= BE ParseComplete
+<= BE BindComplete
+<= BE CommandComplete(INSERT 0 1)
+<= BE ReadyForQuery(I)
+FE=> Parse(stmt="S2", query="SELECT * FROM regress_test WHERE i = 1")
+FE=> Bind(stmt="S2", portal="")
+FE=> Execute(portal="")
+FE=> Sync
+<= BE ParseComplete
+<= BE BindComplete
+<= BE DataRow
+<= BE CommandComplete(SELECT 1)
+<= BE ReadyForQuery(I)
+FE=> Parse(stmt="S3", query="BEGIN")
+FE=> Bind(stmt="S3", portal="")
+FE=> Execute(portal="")
+FE=> Sync
+<= BE ParseComplete
+<= BE BindComplete
+<= BE CommandComplete(BEGIN)
+<= BE ReadyForQuery(T)
+FE=> Bind(stmt="S1", portal="")
+FE=> Execute(portal="")
+FE=> Sync
+<= BE BindComplete
+<= BE CommandComplete(INSERT 0 1)
+<= BE ReadyForQuery(T)
+FE=> Parse(stmt="S4", query="COMMIT")
+FE=> Bind(stmt="S4", portal="")
+FE=> Execute(portal="")
+FE=> Sync
+<= BE ParseComplete
+<= BE BindComplete
+<= BE CommandComplete(COMMIT)
+<= BE ReadyForQuery(I)
+FE=> Bind(stmt="S2", portal="")
+FE=> Execute(portal="")
+FE=> Sync
+<= BE BindComplete
+<= BE DataRow
+<= BE DataRow
+<= BE CommandComplete(SELECT 2)
+<= 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_bug6.data b/src/test/regression/tests/006.memqcache/query_cache_bug6.data
new file mode 100644 (file)
index 0000000..360c81a
--- /dev/null
+++ b/src/test/regression/tests/006.memqcache/query_cache_bug6.data
@@ -0,0 +1,38 @@
+# Test case for query cache invalidation bug.
+# 
+
+# create a test table "regress_test".
+'Q'    "CREATE TABLE regress_test(i int)"
+'Y'
+
+# create named statement S1 for INSERT.
+'P'    "S1"    "INSERT INTO regress_test VALUES(1)"    0
+'B'    ""      "S1"    0       0       0
+'E'    ""      0
+'S'
+'Y'
+
+# create query cache.
+'P'    "S2"    "SELECT * FROM regress_test WHERE i = 1"        0
+'B'    ""      "S2"    0       0       0   
+'E'    ""      0
+'S'
+'Y'
+
+# INSERT one more row without bind message.
+'B'    ""      "S1"    0       0       0
+'E'    ""      0
+'S'
+'Y'
+
+# This should return 2 rows if query cache validation succeeds.
+'B'    ""      "S2"    0       0       0   
+'E'    ""      0
+'S'
+'Y'
+
+# drop the test table.
+'Q'    "DROP TABLE regress_test"
+'Y'
+
+'X'

diff --git a/src/test/regression/tests/006.memqcache/query_cache_bug7.data b/src/test/regression/tests/006.memqcache/query_cache_bug7.data
new file mode 100644 (file)
index 0000000..01f31ba
--- /dev/null
+++ b/src/test/regression/tests/006.memqcache/query_cache_bug7.data
@@ -0,0 +1,52 @@
+# Test case for query cache invalidation bug with explicit
+# transaction.
+
+# create a test table "regress_test".
+'Q'    "CREATE TABLE regress_test(i int)"
+'Y'
+
+# create named statement S1 for INSERT.
+'P'    "S1"    "INSERT INTO regress_test VALUES(1)"    0
+'B'    ""      "S1"    0       0       0
+'E'    ""      1
+'S'
+'Y'
+
+# create query cache.
+'P'    "S2"    "SELECT * FROM regress_test WHERE i = 1"        0
+'B'    ""      "S2"    0       0       0   
+'E'    ""      0
+'S'
+'Y'
+
+# start an explicit transaction.
+'P'    "S3"    "BEGIN" 0
+'B'    ""      "S3"    0       0       0   
+'E'    ""      0
+'S'
+'Y'
+
+# INSERT one more row without bind message.
+'B'    ""      "S1"    0       0       0
+'E'    ""      1
+'S'
+'Y'
+
+# commit the transaction.
+'P'    "S4"    "COMMIT"        0
+'B'    ""      "S4"    0       0       0   
+'E'    ""      0
+'S'
+'Y'
+
+# This should return 2 rows if query cache validation succeeds.
+'B'    ""      "S2"    0       0       0   
+'E'    ""      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 b674fdf70e04f9534662c76a9a8ccacdfca46519..d00bdd084764ba52259edcc5dd489b48b68942b2 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 5
+for i in 1 2 3 4 4 5 6 7
 do
     #
     # case 1: failed with kind mismatch error at #5.
@@ -555,6 +555,11 @@ do
     # the test works for either query cache exists or does not exist
     #
     # case 5: simple cache invalidation test.
+    #
+    # case 6: execute INSERT without parse message cache invalidation test.
+    #
+    # case 7: similar to case 6 except this uses an explicit transaction.
+
     cd $TESTDIR
 
     # case 5 includes UPDATE, and we want the result without disturbed