Add comments on avoid reuse of parse-time snapshot

Apparently, reusing the parse-time query snapshot for later steps
(execution) is a frequently considered optimization ... but it doesn't
work, for reasons discovered in thread [1].  Adding some comments about
why it doesn't really work can relieve some future hackers from wasting
time reimplementing it again.

[1] https://postgr.es/m/flat/5075D8DF.6050500@fuzzy.cz

Author: Michail Nikolaev
Discussion: https://postgr.es/m/CANtu0ogp6cTvMJObXP8n=k+JtqxY1iT9UV5MbGCpjjPa5crCiw@mail.gmail.com

diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 0a6f80963bda933cc874c37c38ea14c8b2e8cd76..23661ae15f5c6b56b5bcfcb0f82b0b88edcdf10e 100644 (file)
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -1155,7 +1155,16 @@ exec_simple_query(const char *query_string)
        plantree_list = pg_plan_queries(querytree_list,
                                        CURSOR_OPT_PARALLEL_OK, NULL);
 
-       /* Done with the snapshot used for parsing/planning */
+       /*
+        * Done with the snapshot used for parsing/planning.
+        *
+        * While it looks promising to reuse the same snapshot for query
+        * execution (at least for simple protocol), unfortunately it causes
+        * execution to use a snapshot that has been acquired before locking
+        * any of the tables mentioned in the query.  This creates user-
+        * visible anomalies, so refrain.  Refer to
+        * https://postgr.es/m/flat/5075D8DF.6050500@fuzzy.cz for details.
+        */
        if (snapshot_set)
            PopActiveSnapshot();