Always validate existing database and extension

This change ensures the database and extension is validated whenever
these objects aren't created, instead of only doing validation when
`bootstrap=>false` is passed when adding a data node.

This fixes a corner case where a data node could be added and removed
several times, even though the data node's database was already marked
as having been part of a multi-node setup.

A new test checks that a data node cannot be re-added after deleting
it on the access node, irrespective of whether one bootstraps the data
node or not when it is added.

tsl/src/data_node.c

@@ -454,7 +454,7 @@ data_node_validate_as_data_node(TSConnection *conn)
 	if (PQresultStatus(res) != PGRES_TUPLES_OK)
 		ereport(ERROR,
 				(errcode(ERRCODE_TS_DATA_NODE_INVALID_CONFIG),
-				 (errmsg("%s is not valid as data node", remote_connection_node_name(conn)),
+				 (errmsg("cannot add \"%s\" as a data node", remote_connection_node_name(conn)),
 				  errdetail("%s", PQresultErrorMessage(res)))));
 
 	remote_result_close(res);
@@ -762,13 +762,16 @@ data_node_add_internal(PG_FUNCTION_ARGS, bool set_distid)
 
 		if (bootstrap)
 			extension_created = data_node_bootstrap_extension(conn);
-		else
+
+		if (!database_created)
 		{
 			data_node_validate_database(conn, &database);
-			data_node_validate_extension(conn);
 			data_node_validate_as_data_node(conn);
 		}
 
+		if (!extension_created)
+			data_node_validate_extension(conn);
+
 		/* After the node is verified or bootstrapped, we set the `dist_uuid`
 		 * using the same connection. We skip this if clustering checks are
 		 * disabled, which means that the `dist_uuid` is neither set nor

tsl/test/expected/data_node_bootstrap.out

@@ -47,7 +47,8 @@ SELECT PG_ENCODING_TO_CHAR(encoding) = :enc
 (1 row)
 
 \c :TEST_DBNAME :ROLE_CLUSTER_SUPERUSER;
--- After delete database and extension should still be there
+-- After delete_data_node, the database and extension should still
+-- exist on the data node
 SELECT * FROM delete_data_node('bootstrap_test');
  delete_data_node 
 ------------------
@@ -70,6 +71,16 @@ AND e.extname = 'timescaledb';
 (1 row)
 
 \c :TEST_DBNAME :ROLE_CLUSTER_SUPERUSER;
+\set ON_ERROR_STOP 0
+-- Trying to add the data node again should fail, with or without
+-- bootstrapping.
+SELECT * FROM add_data_node('bootstrap_test', host => 'localhost', database => 'bootstrap_test', bootstrap=>false);
+ERROR:  cannot add "bootstrap_test" as a data node
+SELECT * FROM add_data_node('bootstrap_test', host => 'localhost', database => 'bootstrap_test');
+NOTICE:  database "bootstrap_test" already exists on data node, skipping
+NOTICE:  extension "timescaledb" already exists on data node, skipping
+ERROR:  cannot add "bootstrap_test" as a data node
+\set ON_ERROR_STOP 0
 DROP DATABASE bootstrap_test;
 ----------------------------------------------------------------------
 -- Bootstrap the database and check that calling it without
@@ -88,7 +99,6 @@ SELECT * FROM show_data_nodes();
  bootstrap_test | localhost | 55432 | bootstrap_test
 (1 row)
 
-\c :TEST_DBNAME :ROLE_CLUSTER_SUPERUSER;
 SELECT * FROM delete_data_node('bootstrap_test');
  delete_data_node 
 ------------------
@@ -327,7 +337,7 @@ SELECT extname FROM pg_extension WHERE extname = 'timescaledb';
 \c :TEST_DBNAME :ROLE_CLUSTER_SUPERUSER;
 SELECT * FROM add_data_node('bootstrap_test', host => 'localhost',
                             database => 'bootstrap_test', bootstrap => false);
-ERROR:  database does not have TimescaleDB extension loaded
+ERROR:  cannot add "bootstrap_test" as a data node
 \set ON_ERROR_STOP 1
 DROP DATABASE bootstrap_test;
 -----------------------------------------------------------------------

tsl/test/expected/dist_util.out

@@ -131,18 +131,18 @@ SET client_min_messages TO NOTICE;
 SELECT * FROM add_data_node('invalid_data_node', host => 'localhost', database => 'frontend_2', bootstrap => true);
 NOTICE:  database "frontend_2" already exists on data node, skipping
 NOTICE:  extension "timescaledb" already exists on data node, skipping
-ERROR:  [invalid_data_node]: database is already a member of a distributed database
+ERROR:  cannot add "invalid_data_node" as a data node
 SELECT * FROM add_data_node('invalid_data_node', host => 'localhost', database => 'frontend_2', bootstrap => false);
-ERROR:  invalid_data_node is not valid as data node
+ERROR:  cannot add "invalid_data_node" as a data node
 ----------------------------------------------------------------
 -- Adding backend from a different group as a backend should fail
 \c frontend_1 :ROLE_CLUSTER_SUPERUSER
 SELECT * FROM add_data_node('invalid_data_node', host => 'localhost', database => 'backend_2_1', bootstrap => true);
 NOTICE:  database "backend_2_1" already exists on data node, skipping
 NOTICE:  extension "timescaledb" already exists on data node, skipping
-ERROR:  [invalid_data_node]: database is already a member of a distributed database
+ERROR:  cannot add "invalid_data_node" as a data node
 SELECT * FROM add_data_node('invalid_data_node', host => 'localhost', database => 'backend_2_1', bootstrap => false);
-ERROR:  invalid_data_node is not valid as data node
+ERROR:  cannot add "invalid_data_node" as a data node
 ----------------------------------------------------------------
 -- Adding a valid backend target but to an existing backend should fail
 \c backend_1_1 :ROLE_CLUSTER_SUPERUSER
@@ -156,9 +156,9 @@ ERROR:  unable to assign data nodes from an existing distributed database
 SELECT * FROM add_data_node('invalid_data_node', host => 'localhost', database => 'frontend_1', bootstrap => true);
 NOTICE:  database "frontend_1" already exists on data node, skipping
 NOTICE:  extension "timescaledb" already exists on data node, skipping
-ERROR:  [invalid_data_node]: database is already a member of a distributed database
+ERROR:  cannot add "invalid_data_node" as a data node
 SELECT * FROM add_data_node('invalid_data_node', host => 'localhost', database => 'frontend_1', bootstrap => false);
-ERROR:  invalid_data_node is not valid as data node
+ERROR:  cannot add "invalid_data_node" as a data node
 \set ON_ERROR_STOP 1
 ----------------------------------------------------------------
 -- Test that a data node can be moved to a different frontend if it is

tsl/test/sql/data_node_bootstrap.sql

@@ -39,7 +39,8 @@ SELECT PG_ENCODING_TO_CHAR(encoding) = :enc
  WHERE datname = current_database();
 
 \c :TEST_DBNAME :ROLE_CLUSTER_SUPERUSER;
--- After delete database and extension should still be there
+-- After delete_data_node, the database and extension should still
+-- exist on the data node
 SELECT * FROM delete_data_node('bootstrap_test');
 SELECT * FROM show_data_nodes();
 
@@ -51,6 +52,13 @@ WHERE e.extnamespace = n.oid
 AND e.extname = 'timescaledb';
 
 \c :TEST_DBNAME :ROLE_CLUSTER_SUPERUSER;
+\set ON_ERROR_STOP 0
+-- Trying to add the data node again should fail, with or without
+-- bootstrapping.
+SELECT * FROM add_data_node('bootstrap_test', host => 'localhost', database => 'bootstrap_test', bootstrap=>false);
+SELECT * FROM add_data_node('bootstrap_test', host => 'localhost', database => 'bootstrap_test');
+\set ON_ERROR_STOP 0
+
 DROP DATABASE bootstrap_test;
 
 ----------------------------------------------------------------------
@@ -61,7 +69,6 @@ SELECT * FROM add_data_node('bootstrap_test', host => 'localhost',
                             database => 'bootstrap_test', bootstrap => true);
 SELECT * FROM show_data_nodes();
 
-\c :TEST_DBNAME :ROLE_CLUSTER_SUPERUSER;
 SELECT * FROM delete_data_node('bootstrap_test');
 
 \c bootstrap_test :ROLE_CLUSTER_SUPERUSER;