Fix long standing bind bug with query cache.
authorTatsuo Ishii <ishii@postgresql.org>
Thu, 8 May 2025 10:49:10 +0000 (19:49 +0900)
committerTatsuo Ishii <ishii@postgresql.org>
Thu, 8 May 2025 12:22:28 +0000 (21:22 +0900)
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

src/protocol/pool_proto_modules.c
src/test/regression/tests/006.memqcache/expected.6 [new file with mode: 0644]
src/test/regression/tests/006.memqcache/expected.7 [new file with mode: 0644]
src/test/regression/tests/006.memqcache/query_cache_bug6.data [new file with mode: 0644]
src/test/regression/tests/006.memqcache/query_cache_bug7.data [new file with mode: 0644]
src/test/regression/tests/006.memqcache/test.sh

index 6185b605c79ef53359661680c0696fd56cf0e3e5..393292487231682b80216af4e435f46de3bfe92b 100644 (file)
@@ -1731,6 +1731,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
@@ -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
@@ -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
@@ -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
@@ -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'
index f133f415a8e4cd1f173809ea852fe3eff37c81b5..335f14df4207a71cab5a150bd3abe065b38774a0 100755 (executable)
@@ -488,7 +488,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.
@@ -503,6 +503,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