Revert "Drop unnamed portal immediately after execution to completion"HEADmaster

This reverts commit 1fd981f05369, based on concerns that the logging
improvements do not justify the protocol breakage of dropping an unnamed
portal once its execution has completed.

It seems unlikely that one would try to send an execute or describe
message after the portal has been used, but if they do such
post-completion messages would not be able to process as the previous
versions.  Let's revert this change for now so as we keep compatibility
and consider a different solution.

The tests added by 76bba033128a track the pre-1fd981f05369 behavior, and
are still valid.

Discussion: https://postgr.es/m/CA+TgmoYFJyJNQw3RT7veO3M2BWRE9Aw4hprC5rOcawHZti-f8g@mail.gmail.com

3 files changed, 18 insertions, 29 deletions

diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index d1b9af11b07..9d755232873 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -1006,8 +1006,8 @@ SELCT 1/0;<!-- this typo is intentional -->
    <para>
     If successfully created, a named portal object lasts till the end of the
     current transaction, unless explicitly destroyed.  An unnamed portal is
-    destroyed at the end of the transaction, or as soon as the statement
-    specifying the unnamed portal as destination is processed to completion.  (Note
+    destroyed at the end of the transaction, or as soon as the next Bind
+    statement specifying the unnamed portal as destination is issued.  (Note
     that a simple Query message also destroys the unnamed portal.)  Named
     portals must be explicitly closed before they can be redefined by another
     Bind message, but this is not required for the unnamed portal.
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 2bd89102686..7dd75a490aa 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -2327,16 +2327,6 @@ exec_execute_message(const char *portal_name, long max_rows)
 			 * message.  The next protocol message will start a fresh timeout.
 			 */
 			disable_statement_timeout();
-
-			/*
-			 * We completed fetching from an unnamed portal.  There is no need
-			 * for it beyond this point, so drop it now rather than wait for
-			 * the next Bind message to do this cleanup.  This ensures that
-			 * the correct statement is logged when cleaning up temporary file
-			 * usage.
-			 */
-			if (portal->name[0] == '\0')
-				PortalDrop(portal, false);
 		}
 
 		/* Send appropriate CommandComplete to client */
diff --git a/src/test/modules/test_misc/t/009_log_temp_files.pl b/src/test/modules/test_misc/t/009_log_temp_files.pl
index 7ecd301ae29..462a949e411 100644
--- a/src/test/modules/test_misc/t/009_log_temp_files.pl
+++ b/src/test/modules/test_misc/t/009_log_temp_files.pl
@@ -29,7 +29,7 @@ CREATE UNLOGGED TABLE foo(a int);
 INSERT INTO foo(a) SELECT * FROM generate_series(1, 5000);
 });
 
-note "unnamed portal: temporary file dropped under first SELECT query";
+note "unnamed portal: temporary file dropped under second SELECT query";
 my $log_offset = -s $node->logfile;
 $node->safe_psql(
 	"postgres", qq{
@@ -39,23 +39,22 @@ SELECT 'unnamed portal';
 END;
 });
 ok( $node->log_contains(
-		qr/LOG:\s+temporary file: path.*\n.*\ STATEMENT:\s+SELECT a FROM foo ORDER BY a OFFSET \$1/s,
+		qr/LOG:\s+temporary file: path.*\n.*\ STATEMENT:\s+SELECT 'unnamed portal'/s,
 		$log_offset),
 	"unnamed portal");
 
-note
-  "bind and implicit transaction: temporary file dropped under single query";
+note "bind and implicit transaction: temporary file dropped without query";
 $log_offset = -s $node->logfile;
 $node->safe_psql(
 	"postgres", qq{
 SELECT a FROM foo ORDER BY a OFFSET \$1 \\bind 4991 \\g
 });
-ok( $node->log_contains(
-		qr/LOG:\s+temporary file: path.*\n.*\ STATEMENT:\s+SELECT a FROM foo ORDER BY a OFFSET \$1/s,
-		$log_offset),
-	"bind and implicit transaction");
+ok( $node->log_contains(qr/LOG:\s+temporary file:/s, $log_offset),
+	"bind and implicit transaction, temporary file removed");
+ok( !$node->log_contains(qr/STATEMENT:/s, $log_offset),
+	"bind and implicit transaction, no statement logged");
 
-note "named portal: temporary file dropped under first SELECT query";
+note "named portal: temporary file dropped under second SELECT query";
 $node->safe_psql(
 	"postgres", qq{
 BEGIN;
@@ -65,11 +64,11 @@ SELECT 'named portal';
 END;
 });
 ok( $node->log_contains(
-		qr/LOG:\s+temporary file: path.*\n.*\ STATEMENT:\s+SELECT a FROM foo ORDER BY a OFFSET \$1/s,
+		qr/LOG:\s+temporary file: path.*\n.*\ STATEMENT:\s+SELECT 'named portal'/s,
 		$log_offset),
 	"named portal");
 
-note "pipelined query: temporary file dropped under first SELECT query";
+note "pipelined query: temporary file dropped under second SELECT query";
 $log_offset = -s $node->logfile;
 $node->safe_psql(
 	"postgres", qq{
@@ -79,21 +78,21 @@ SELECT 'pipelined query';
 \\endpipeline
 });
 ok( $node->log_contains(
-		qr/LOG:\s+temporary file: path.*\n.*\ STATEMENT:\s+SELECT a FROM foo ORDER BY a OFFSET \$1/s,
+		qr/LOG:\s+temporary file: path.*\n.*\ STATEMENT:\s+SELECT 'pipelined query'/s,
 		$log_offset),
 	"pipelined query");
 
-note "parse and bind: temporary file dropped under SELECT query";
+note "parse and bind: temporary file dropped without query";
 $log_offset = -s $node->logfile;
 $node->safe_psql(
 	"postgres", qq{
 SELECT a, a, a FROM foo ORDER BY a OFFSET \$1 \\parse p1
 \\bind_named p1 4993 \\g
 });
-ok( $node->log_contains(
-		qr/LOG:\s+temporary file: path.*\n.*\ STATEMENT:\s+SELECT a, a, a FROM foo ORDER BY a OFFSET \$1/s,
-		$log_offset),
-	"parse and bind");
+ok($node->log_contains(qr/LOG:\s+temporary file:/s, $log_offset),
+	"parse and bind, temporary file removed");
+ok(!$node->log_contains(qr/STATEMENT:/s, $log_offset),
+	"bind and bind, no statement logged");
 
 note "simple query: temporary file dropped under SELECT query";
 $log_offset = -s $node->logfile;