postgres_fdw: Fix tests for CLOBBER_CACHE_ALWAYS.

The regression tests added in commits 708d165ddb and 411ae64997 caused
buildfarm failures when  CLOBBER_CACHE_ALWAYS was enabled.
This commit stabilizes those tests.

The foreign server connections established by postgres_fdw behaves
differently depending on whether CLOBBER_CACHE_ALWAYS is enabled or not.
If it's not enabled, those connections are cached. On the other hand,
if it's enabled, when the connections are established outside transaction
block, they are not cached (i.e., they are immediately closed at the end of
query that established them). So the subsequent postgres_fdw_get_connections()
cannot list those connections and postgres_fdw_disconnect() cannot close them
(because they are already closed).

When the connections are established inside transaction block, they are
cached whether CLOBBER_CACHE_ALWAYS was enabled or not. But if it's enabled,
they are immediately marked as invalid, otherwise not. This causes the
subsequent postgres_fdw_get_connections() to return different result in
"valid" column depending on whether CLOBBER_CACHE_ALWAYS was enabled or not.

This commit prevents the above differences of behavior from
affecting the regression tests.

Per buildfarm failure on trilobite.

Original patch by Bharath Rupireddy. I (Fujii Masao) extracted
the regression test fix from that and revised it a bit.

Reported-by: Tom Lane
Author: Bharath Rupireddy
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/2688508.1611865371@sss.pgh.pa.us

diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 07e06e5bf731024800b8b371f276e432b2b15849..b09dce63f5ffa83ca8f2102f59662c1cdd4ce6f6 100644 (file)
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -17,10 +17,6 @@ DO $d$
             OPTIONS (dbname '$$||current_database()||$$',
                      port '$$||current_setting('port')||$$'
             )$$;
-        EXECUTE $$CREATE SERVER loopback4 FOREIGN DATA WRAPPER postgres_fdw
-            OPTIONS (dbname '$$||current_database()||$$',
-                     port '$$||current_setting('port')||$$'
-            )$$;
     END;
 $d$;
 CREATE USER MAPPING FOR public SERVER testserver1
@@ -28,7 +24,6 @@ CREATE USER MAPPING FOR public SERVER testserver1
 CREATE USER MAPPING FOR CURRENT_USER SERVER loopback;
 CREATE USER MAPPING FOR CURRENT_USER SERVER loopback2;
 CREATE USER MAPPING FOR public SERVER loopback3;
-CREATE USER MAPPING FOR public SERVER loopback4;
 -- ===================================================================
 -- create objects used through FDW loopback server
 -- ===================================================================
@@ -144,11 +139,6 @@ CREATE FOREIGN TABLE ft7 (
    c2 int NOT NULL,
    c3 text
 ) SERVER loopback3 OPTIONS (schema_name 'S 1', table_name 'T 4');
-CREATE FOREIGN TABLE ft8 (
-   c1 int NOT NULL,
-   c2 int NOT NULL,
-   c3 text
-) SERVER loopback4 OPTIONS (schema_name 'S 1', table_name 'T 4');
 -- ===================================================================
 -- tests for validator
 -- ===================================================================
@@ -220,8 +210,7 @@ ALTER FOREIGN TABLE ft2 ALTER COLUMN c1 OPTIONS (column_name 'C 1');
  public | ft5   | loopback  | (schema_name 'S 1', table_name 'T 4') | 
  public | ft6   | loopback2 | (schema_name 'S 1', table_name 'T 4') | 
  public | ft7   | loopback3 | (schema_name 'S 1', table_name 'T 4') | 
- public | ft8   | loopback4 | (schema_name 'S 1', table_name 'T 4') | 
-(7 rows)
+(6 rows)
 
 -- Test that alteration of server options causes reconnection
 -- Remote's errors might be non-English, so hide them to ensure stable results
@@ -9066,15 +9055,21 @@ DROP PROCEDURE terminate_backend_and_wait(text);
 -- =============================================================================
 -- test connection invalidation cases and postgres_fdw_get_connections function
 -- =============================================================================
--- This test case is for closing the connection in pgfdw_xact_callback
-BEGIN;
--- List all the existing cached connections. Only loopback2 should be output.
-SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
- server_name | valid 
--------------+-------
- loopback2   | t
+-- Let's ensure to close all the existing cached connections.
+SELECT 1 FROM postgres_fdw_disconnect_all();
+ ?column? 
+----------
+        1
 (1 row)
 
+-- No cached connections, so no records should be output.
+SELECT server_name FROM postgres_fdw_get_connections() ORDER BY 1;
+ server_name 
+-------------
+(0 rows)
+
+-- This test case is for closing the connection in pgfdw_xact_callback
+BEGIN;
 -- Connection xact depth becomes 1 i.e. the connection is in midst of the xact.
 SELECT 1 FROM ft1 LIMIT 1;
  ?column? 
@@ -9088,19 +9083,18 @@ SELECT 1 FROM ft7 LIMIT 1;
         1
 (1 row)
 
--- List all the existing cached connections. loopback and loopback3
--- also should be output as valid connections.
-SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
- server_name | valid 
--------------+-------
- loopback    | t
- loopback2   | t
- loopback3   | t
-(3 rows)
+-- List all the existing cached connections. loopback and loopback3 should be
+-- output.
+SELECT server_name FROM postgres_fdw_get_connections() ORDER BY 1;
+ server_name 
+-------------
+ loopback
+ loopback3
+(2 rows)
 
--- Connection is not closed at the end of the alter statement in
--- pgfdw_inval_callback. That's because the connection is in midst of this
--- xact, it is just marked as invalid.
+-- Connections are not closed at the end of the alter and drop statements.
+-- That's because the connections are in midst of this xact,
+-- they are just marked as invalid in pgfdw_inval_callback.
 ALTER SERVER loopback OPTIONS (ADD use_remote_estimate 'off');
 DROP SERVER loopback3 CASCADE;
 NOTICE:  drop cascades to 2 other objects
@@ -9113,31 +9107,22 @@ SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
  server_name | valid 
 -------------+-------
  loopback    | f
- loopback2   | t
              | f
-(3 rows)
+(2 rows)
 
--- The invalid connection gets closed in pgfdw_xact_callback during commit.
+-- The invalid connections get closed in pgfdw_xact_callback during commit.
 COMMIT;
--- List all the existing cached connections. loopback and loopback3
--- should not be output because they should be closed at the end of
--- the above transaction.
-SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
- server_name | valid 
--------------+-------
- loopback2   | t
-(1 row)
+-- All cached connections were closed while committing above xact, so no
+-- records should be output.
+SELECT server_name FROM postgres_fdw_get_connections() ORDER BY 1;
+ server_name 
+-------------
+(0 rows)
 
 -- =======================================================================
 -- test postgres_fdw_disconnect and postgres_fdw_disconnect_all functions
 -- =======================================================================
--- Return true as all cached connections are closed.
-SELECT postgres_fdw_disconnect_all();
- postgres_fdw_disconnect_all 
------------------------------
- t
-(1 row)
-
+BEGIN;
 -- Ensure to cache loopback connection.
 SELECT 1 FROM ft1 LIMIT 1;
  ?column? 
@@ -9145,7 +9130,6 @@ SELECT 1 FROM ft1 LIMIT 1;
         1
 (1 row)
 
-BEGIN;
 -- Ensure to cache loopback2 connection.
 SELECT 1 FROM ft6 LIMIT 1;
  ?column? 
@@ -9155,66 +9139,31 @@ SELECT 1 FROM ft6 LIMIT 1;
 
 -- List all the existing cached connections. loopback and loopback2 should be
 -- output.
-SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
- server_name | valid 
--------------+-------
- loopback    | t
- loopback2   | t
+SELECT server_name FROM postgres_fdw_get_connections() ORDER BY 1;
+ server_name 
+-------------
+ loopback
+ loopback2
 (2 rows)
 
--- Issue a warning and return false as loopback2 connection is still in use and
+-- Issue a warning and return false as loopback connection is still in use and
 -- can not be closed.
-SELECT postgres_fdw_disconnect('loopback2');
-WARNING:  cannot close connection for server "loopback2" because it is still in use
+SELECT postgres_fdw_disconnect('loopback');
+WARNING:  cannot close connection for server "loopback" because it is still in use
  postgres_fdw_disconnect 
 -------------------------
  f
 (1 row)
 
--- Close loopback connection, return true and issue a warning as loopback2
--- connection is still in use and can not be closed.
-SELECT postgres_fdw_disconnect_all();
-WARNING:  cannot close connection for server "loopback2" because it is still in use
- postgres_fdw_disconnect_all 
------------------------------
- t
-(1 row)
-
--- List all the existing cached connections. loopback2 should be output.
-SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
- server_name | valid 
--------------+-------
- loopback2   | t
-(1 row)
-
--- Ensure to cache loopback connection.
-SELECT 1 FROM ft1 LIMIT 1;
- ?column? 
-----------
-        1
-(1 row)
-
--- Ensure to cache loopback4 connection.
-SELECT 1 FROM ft8 LIMIT 1;
- ?column? 
-----------
-        1
-(1 row)
-
--- List all the existing cached connections. loopback, loopback2, loopback4
--- should be output.
-SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
- server_name | valid 
--------------+-------
- loopback    | t
- loopback2   | t
- loopback4   | t
-(3 rows)
+-- List all the existing cached connections. loopback and loopback2 should be
+-- output.
+SELECT server_name FROM postgres_fdw_get_connections() ORDER BY 1;
+ server_name 
+-------------
+ loopback
+ loopback2
+(2 rows)
 
-DROP SERVER loopback4 CASCADE;
-NOTICE:  drop cascades to 2 other objects
-DETAIL:  drop cascades to user mapping for public on server loopback4
-drop cascades to foreign table ft8
 -- Return false as connections are still in use, warnings are issued.
 -- But disable warnings temporarily because the order of them is not stable.
 SET client_min_messages = 'ERROR';
@@ -9226,21 +9175,19 @@ SELECT postgres_fdw_disconnect_all();
 
 RESET client_min_messages;
 COMMIT;
--- Close loopback2 connection and return true.
-SELECT postgres_fdw_disconnect('loopback2');
- postgres_fdw_disconnect 
--------------------------
- t
+-- Ensure that loopback2 connection is closed.
+SELECT 1 FROM postgres_fdw_disconnect('loopback2');
+ ?column? 
+----------
+        1
 (1 row)
 
--- List all the existing cached connections. loopback should be output.
-SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
- server_name | valid 
--------------+-------
- loopback    | t
-(1 row)
+SELECT server_name FROM postgres_fdw_get_connections() WHERE server_name = 'loopback2';
+ server_name 
+-------------
+(0 rows)
 
--- Return false as loopback2 connectin is closed already.
+-- Return false as loopback2 connection is closed already.
 SELECT postgres_fdw_disconnect('loopback2');
  postgres_fdw_disconnect 
 -------------------------
@@ -9250,18 +9197,17 @@ SELECT postgres_fdw_disconnect('loopback2');
 -- Return an error as there is no foreign server with given name.
 SELECT postgres_fdw_disconnect('unknownserver');
 ERROR:  server "unknownserver" does not exist
--- Close loopback connection and return true.
-SELECT postgres_fdw_disconnect_all();
- postgres_fdw_disconnect_all 
------------------------------
- t
+-- Let's ensure to close all the existing cached connections.
+SELECT 1 FROM postgres_fdw_disconnect_all();
+ ?column? 
+----------
+        1
 (1 row)
 
--- List all the existing cached connections. No connection exists, so NULL
--- should be output.
-SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
- server_name | valid 
--------------+-------
+-- No cached connections, so no records should be output.
+SELECT server_name FROM postgres_fdw_get_connections() ORDER BY 1;
+ server_name 
+-------------
 (0 rows)
 
 -- =============================================================================
@@ -9271,6 +9217,7 @@ CREATE ROLE regress_multi_conn_user1 SUPERUSER;
 CREATE ROLE regress_multi_conn_user2 SUPERUSER;
 CREATE USER MAPPING FOR regress_multi_conn_user1 SERVER loopback;
 CREATE USER MAPPING FOR regress_multi_conn_user2 SERVER loopback;
+BEGIN;
 -- Will cache loopback connection with user mapping for regress_multi_conn_user1
 SET ROLE regress_multi_conn_user1;
 SELECT 1 FROM ft1 LIMIT 1;
@@ -9290,25 +9237,25 @@ SELECT 1 FROM ft1 LIMIT 1;
 
 RESET ROLE;
 -- Should output two connections for loopback server
-SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
- server_name | valid 
--------------+-------
- loopback    | t
- loopback    | t
+SELECT server_name FROM postgres_fdw_get_connections() ORDER BY 1;
+ server_name 
+-------------
+ loopback
+ loopback
 (2 rows)
 
--- Close loopback connections and return true.
-SELECT postgres_fdw_disconnect('loopback');
- postgres_fdw_disconnect 
--------------------------
- t
+COMMIT;
+-- Let's ensure to close all the existing cached connections.
+SELECT 1 FROM postgres_fdw_disconnect_all();
+ ?column? 
+----------
+        1
 (1 row)
 
--- List all the existing cached connections. No connection exists, so NULL
--- should be output.
-SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
- server_name | valid 
--------------+-------
+-- No cached connections, so no records should be output.
+SELECT server_name FROM postgres_fdw_get_connections() ORDER BY 1;
+ server_name 
+-------------
 (0 rows)
 
 -- Clean up

diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 647192cf6ace65f3a6f200321149c8056e9822d5..319c15d635c0652d4c1fd4409d64f0119d7fc734 100644 (file)
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -19,10 +19,6 @@ DO $d$
             OPTIONS (dbname '$$||current_database()||$$',
                      port '$$||current_setting('port')||$$'
             )$$;
-        EXECUTE $$CREATE SERVER loopback4 FOREIGN DATA WRAPPER postgres_fdw
-            OPTIONS (dbname '$$||current_database()||$$',
-                     port '$$||current_setting('port')||$$'
-            )$$;
     END;
 $d$;
 
@@ -31,7 +27,6 @@ CREATE USER MAPPING FOR public SERVER testserver1
 CREATE USER MAPPING FOR CURRENT_USER SERVER loopback;
 CREATE USER MAPPING FOR CURRENT_USER SERVER loopback2;
 CREATE USER MAPPING FOR public SERVER loopback3;
-CREATE USER MAPPING FOR public SERVER loopback4;
 
 -- ===================================================================
 -- create objects used through FDW loopback server
@@ -158,12 +153,6 @@ CREATE FOREIGN TABLE ft7 (
    c3 text
 ) SERVER loopback3 OPTIONS (schema_name 'S 1', table_name 'T 4');
 
-CREATE FOREIGN TABLE ft8 (
-   c1 int NOT NULL,
-   c2 int NOT NULL,
-   c3 text
-) SERVER loopback4 OPTIONS (schema_name 'S 1', table_name 'T 4');
-
 -- ===================================================================
 -- tests for validator
 -- ===================================================================
@@ -2723,80 +2712,67 @@ DROP PROCEDURE terminate_backend_and_wait(text);
 -- =============================================================================
 -- test connection invalidation cases and postgres_fdw_get_connections function
 -- =============================================================================
+-- Let's ensure to close all the existing cached connections.
+SELECT 1 FROM postgres_fdw_disconnect_all();
+-- No cached connections, so no records should be output.
+SELECT server_name FROM postgres_fdw_get_connections() ORDER BY 1;
 -- This test case is for closing the connection in pgfdw_xact_callback
 BEGIN;
--- List all the existing cached connections. Only loopback2 should be output.
-SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
 -- Connection xact depth becomes 1 i.e. the connection is in midst of the xact.
 SELECT 1 FROM ft1 LIMIT 1;
 SELECT 1 FROM ft7 LIMIT 1;
--- List all the existing cached connections. loopback and loopback3
--- also should be output as valid connections.
-SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
--- Connection is not closed at the end of the alter statement in
--- pgfdw_inval_callback. That's because the connection is in midst of this
--- xact, it is just marked as invalid.
+-- List all the existing cached connections. loopback and loopback3 should be
+-- output.
+SELECT server_name FROM postgres_fdw_get_connections() ORDER BY 1;
+-- Connections are not closed at the end of the alter and drop statements.
+-- That's because the connections are in midst of this xact,
+-- they are just marked as invalid in pgfdw_inval_callback.
 ALTER SERVER loopback OPTIONS (ADD use_remote_estimate 'off');
 DROP SERVER loopback3 CASCADE;
 -- List all the existing cached connections. loopback and loopback3
 -- should be output as invalid connections. Also the server name for
 -- loopback3 should be NULL because the server was dropped.
 SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
--- The invalid connection gets closed in pgfdw_xact_callback during commit.
+-- The invalid connections get closed in pgfdw_xact_callback during commit.
 COMMIT;
--- List all the existing cached connections. loopback and loopback3
--- should not be output because they should be closed at the end of
--- the above transaction.
-SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
+-- All cached connections were closed while committing above xact, so no
+-- records should be output.
+SELECT server_name FROM postgres_fdw_get_connections() ORDER BY 1;
 
 -- =======================================================================
 -- test postgres_fdw_disconnect and postgres_fdw_disconnect_all functions
 -- =======================================================================
--- Return true as all cached connections are closed.
-SELECT postgres_fdw_disconnect_all();
+BEGIN;
 -- Ensure to cache loopback connection.
 SELECT 1 FROM ft1 LIMIT 1;
-BEGIN;
 -- Ensure to cache loopback2 connection.
 SELECT 1 FROM ft6 LIMIT 1;
 -- List all the existing cached connections. loopback and loopback2 should be
 -- output.
-SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
--- Issue a warning and return false as loopback2 connection is still in use and
+SELECT server_name FROM postgres_fdw_get_connections() ORDER BY 1;
+-- Issue a warning and return false as loopback connection is still in use and
 -- can not be closed.
-SELECT postgres_fdw_disconnect('loopback2');
--- Close loopback connection, return true and issue a warning as loopback2
--- connection is still in use and can not be closed.
-SELECT postgres_fdw_disconnect_all();
--- List all the existing cached connections. loopback2 should be output.
-SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
--- Ensure to cache loopback connection.
-SELECT 1 FROM ft1 LIMIT 1;
--- Ensure to cache loopback4 connection.
-SELECT 1 FROM ft8 LIMIT 1;
--- List all the existing cached connections. loopback, loopback2, loopback4
--- should be output.
-SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
-DROP SERVER loopback4 CASCADE;
+SELECT postgres_fdw_disconnect('loopback');
+-- List all the existing cached connections. loopback and loopback2 should be
+-- output.
+SELECT server_name FROM postgres_fdw_get_connections() ORDER BY 1;
 -- Return false as connections are still in use, warnings are issued.
 -- But disable warnings temporarily because the order of them is not stable.
 SET client_min_messages = 'ERROR';
 SELECT postgres_fdw_disconnect_all();
 RESET client_min_messages;
 COMMIT;
--- Close loopback2 connection and return true.
-SELECT postgres_fdw_disconnect('loopback2');
--- List all the existing cached connections. loopback should be output.
-SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
--- Return false as loopback2 connectin is closed already.
+-- Ensure that loopback2 connection is closed.
+SELECT 1 FROM postgres_fdw_disconnect('loopback2');
+SELECT server_name FROM postgres_fdw_get_connections() WHERE server_name = 'loopback2';
+-- Return false as loopback2 connection is closed already.
 SELECT postgres_fdw_disconnect('loopback2');
 -- Return an error as there is no foreign server with given name.
 SELECT postgres_fdw_disconnect('unknownserver');
--- Close loopback connection and return true.
-SELECT postgres_fdw_disconnect_all();
--- List all the existing cached connections. No connection exists, so NULL
--- should be output.
-SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
+-- Let's ensure to close all the existing cached connections.
+SELECT 1 FROM postgres_fdw_disconnect_all();
+-- No cached connections, so no records should be output.
+SELECT server_name FROM postgres_fdw_get_connections() ORDER BY 1;
 
 -- =============================================================================
 -- test case for having multiple cached connections for a foreign server
@@ -2806,6 +2782,7 @@ CREATE ROLE regress_multi_conn_user2 SUPERUSER;
 CREATE USER MAPPING FOR regress_multi_conn_user1 SERVER loopback;
 CREATE USER MAPPING FOR regress_multi_conn_user2 SERVER loopback;
 
+BEGIN;
 -- Will cache loopback connection with user mapping for regress_multi_conn_user1
 SET ROLE regress_multi_conn_user1;
 SELECT 1 FROM ft1 LIMIT 1;
@@ -2817,14 +2794,12 @@ SELECT 1 FROM ft1 LIMIT 1;
 RESET ROLE;
 
 -- Should output two connections for loopback server
-SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
-
--- Close loopback connections and return true.
-SELECT postgres_fdw_disconnect('loopback');
-
--- List all the existing cached connections. No connection exists, so NULL
--- should be output.
-SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
+SELECT server_name FROM postgres_fdw_get_connections() ORDER BY 1;
+COMMIT;
+-- Let's ensure to close all the existing cached connections.
+SELECT 1 FROM postgres_fdw_disconnect_all();
+-- No cached connections, so no records should be output.
+SELECT server_name FROM postgres_fdw_get_connections() ORDER BY 1;
 
 -- Clean up
 DROP USER MAPPING FOR regress_multi_conn_user1 SERVER loopback;