From 4100b66f3bddf4ccadf2525296abb3c3c92fc3d1 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Fri, 8 Nov 2019 15:25:57 +0100 Subject: [PATCH 01/16] Scaffolding for diagnostic function(s) --- scripts-available/CDB_FederatedServerDiagnostics.sql | 12 ++++++++++++ .../403-CDB_FederatedServerDiagnostics.sql | 1 + test/CDB_FederatedServerDiagnostics.sql | 2 ++ test/CDB_FederatedServerDiagnostics_expect | 2 ++ 4 files changed, 17 insertions(+) create mode 100644 scripts-available/CDB_FederatedServerDiagnostics.sql create mode 120000 scripts-enabled/403-CDB_FederatedServerDiagnostics.sql create mode 100644 test/CDB_FederatedServerDiagnostics.sql create mode 100644 test/CDB_FederatedServerDiagnostics_expect diff --git a/scripts-available/CDB_FederatedServerDiagnostics.sql b/scripts-available/CDB_FederatedServerDiagnostics.sql new file mode 100644 index 0000000..7b16263 --- /dev/null +++ b/scripts-available/CDB_FederatedServerDiagnostics.sql @@ -0,0 +1,12 @@ +-------------------------------------------------------------------------------- +-- Public functions +-------------------------------------------------------------------------------- + +CREATE OR REPLACE FUNCTION @extschema@.CDB_Federated_Server_Diagnostics(server TEXT) +RETURNS json -- TODO decide if json or jsonb +AS $$ +BEGIN + RETURN '{}'::json; +END +$$ +LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE; diff --git a/scripts-enabled/403-CDB_FederatedServerDiagnostics.sql b/scripts-enabled/403-CDB_FederatedServerDiagnostics.sql new file mode 120000 index 0000000..edab4d4 --- /dev/null +++ b/scripts-enabled/403-CDB_FederatedServerDiagnostics.sql @@ -0,0 +1 @@ +../scripts-available/CDB_FederatedServerDiagnostics.sql \ No newline at end of file diff --git a/test/CDB_FederatedServerDiagnostics.sql b/test/CDB_FederatedServerDiagnostics.sql new file mode 100644 index 0000000..d0af580 --- /dev/null +++ b/test/CDB_FederatedServerDiagnostics.sql @@ -0,0 +1,2 @@ +\echo '%% It raises an error if the server does not exist' +SELECT '1.1', cartodb.CDB_Federated_Server_Diagnostics(server => 'doesNotExist'); diff --git a/test/CDB_FederatedServerDiagnostics_expect b/test/CDB_FederatedServerDiagnostics_expect new file mode 100644 index 0000000..ea8ac49 --- /dev/null +++ b/test/CDB_FederatedServerDiagnostics_expect @@ -0,0 +1,2 @@ +%% It raises an error if the server does not exist +1.1|{} From 35b2b7e5894a557eeacd77d26dbc59ea48dd20bf Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Fri, 8 Nov 2019 17:14:43 +0100 Subject: [PATCH 02/16] Check the server is of type PG and return jsonb --- .../CDB_FederatedServerDiagnostics.sql | 30 +++++++++++- test/CDB_FederatedServerDiagnostics.sql | 48 +++++++++++++++++++ test/CDB_FederatedServerDiagnostics_expect | 6 ++- 3 files changed, 81 insertions(+), 3 deletions(-) diff --git a/scripts-available/CDB_FederatedServerDiagnostics.sql b/scripts-available/CDB_FederatedServerDiagnostics.sql index 7b16263..2f727ea 100644 --- a/scripts-available/CDB_FederatedServerDiagnostics.sql +++ b/scripts-available/CDB_FederatedServerDiagnostics.sql @@ -1,12 +1,38 @@ +-------------------------------------------------------------------------------- +-- Private functions +-------------------------------------------------------------------------------- + +CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_Server_Diagnostics_PG(server_internal name) +RETURNS jsonb +AS $$ +BEGIN + RETURN '{}'::jsonb; +END +$$ +LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE; + + + -------------------------------------------------------------------------------- -- Public functions -------------------------------------------------------------------------------- +-- +-- TODO: function documentation +-- CREATE OR REPLACE FUNCTION @extschema@.CDB_Federated_Server_Diagnostics(server TEXT) -RETURNS json -- TODO decide if json or jsonb +RETURNS jsonb AS $$ +DECLARE + server_internal name := @extschema@.__CDB_FS_Generate_Server_Name(input_name => server, check_existence => true); + server_type name := @extschema@.__CDB_FS_server_type(server_internal); BEGIN - RETURN '{}'::json; + CASE server_type + WHEN 'postgres_fdw' THEN + RETURN @extschema@.__CDB_FS_Server_Diagnostics_PG(server_internal); + ELSE + RAISE EXCEPTION 'Not implemented server type % for remote server %', server_type, server; + END CASE; END $$ LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE; diff --git a/test/CDB_FederatedServerDiagnostics.sql b/test/CDB_FederatedServerDiagnostics.sql index d0af580..0c1097e 100644 --- a/test/CDB_FederatedServerDiagnostics.sql +++ b/test/CDB_FederatedServerDiagnostics.sql @@ -1,2 +1,50 @@ +-- =================================================================== +-- create FDW objects +-- =================================================================== +\set QUIET on +SET client_min_messages TO error; +\set VERBOSITY terse +CREATE EXTENSION postgres_fdw; + +CREATE ROLE cdb_fs_tester LOGIN PASSWORD 'cdb_fs_passwd'; +GRANT CONNECT ON DATABASE contrib_regression TO cdb_fs_tester; + +-- Create database to be used as remote +CREATE DATABASE cdb_fs_tester OWNER cdb_fs_tester; + +SELECT 'C1', cartodb.CDB_Federated_Server_Register_PG(server => 'loopback'::text, config => '{ + "server": { + "host": "localhost", + "port": @@PGPORT@@ + }, + "credentials": { + "username": "cdb_fs_tester", + "password": "cdb_fs_passwd" + } +}'::jsonb); +\set QUIET off + + +-- =================================================================== +-- Test server diagnostics function(s) +-- =================================================================== \echo '%% It raises an error if the server does not exist' SELECT '1.1', cartodb.CDB_Federated_Server_Diagnostics(server => 'doesNotExist'); + +\echo '%% It returns a jsonb object' +SELECT '1.2', pg_typeof(cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback')); + + +-- =================================================================== +-- Cleanup +-- =================================================================== +\set QUIET on +SELECT 'D1', cartodb.CDB_Federated_Server_Unregister(server => 'loopback'::text); +DROP DATABASE cdb_fs_tester; + +-- Drop role +REVOKE CONNECT ON DATABASE contrib_regression FROM cdb_fs_tester; +DROP ROLE cdb_fs_tester; + +DROP EXTENSION postgres_fdw; +\set QUIET off diff --git a/test/CDB_FederatedServerDiagnostics_expect b/test/CDB_FederatedServerDiagnostics_expect index ea8ac49..b8ffd9e 100644 --- a/test/CDB_FederatedServerDiagnostics_expect +++ b/test/CDB_FederatedServerDiagnostics_expect @@ -1,2 +1,6 @@ +C1| %% It raises an error if the server does not exist -1.1|{} +ERROR: Server "doesNotExist" does not exist +%% It returns a jsonb object +1.2|jsonb +D1| From 40a2ba9569df87d9b4d6838e7eba6e2179c9867a Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Fri, 8 Nov 2019 18:09:22 +0100 Subject: [PATCH 03/16] Stub function to add server_version to diagnostics --- .../CDB_FederatedServerDiagnostics.sql | 15 ++++++++++++++- test/CDB_FederatedServerDiagnostics.sql | 3 +++ test/CDB_FederatedServerDiagnostics_expect | 2 ++ 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/scripts-available/CDB_FederatedServerDiagnostics.sql b/scripts-available/CDB_FederatedServerDiagnostics.sql index 2f727ea..fd00e95 100644 --- a/scripts-available/CDB_FederatedServerDiagnostics.sql +++ b/scripts-available/CDB_FederatedServerDiagnostics.sql @@ -2,11 +2,24 @@ -- Private functions -------------------------------------------------------------------------------- +CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_Server_Version_PG(server_internal name) +RETURNS text +AS $$ +BEGIN + -- TODO Implement + RETURN '14.0'; +END +$$ +LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE; + + CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_Server_Diagnostics_PG(server_internal name) RETURNS jsonb AS $$ +DECLARE + remote_server_version text := @extschema@.__CDB_FS_Server_Version_PG(server_internal); BEGIN - RETURN '{}'::jsonb; + RETURN jsonb_build_object('server_version', remote_server_version); END $$ LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE; diff --git a/test/CDB_FederatedServerDiagnostics.sql b/test/CDB_FederatedServerDiagnostics.sql index 0c1097e..6783118 100644 --- a/test/CDB_FederatedServerDiagnostics.sql +++ b/test/CDB_FederatedServerDiagnostics.sql @@ -34,6 +34,9 @@ SELECT '1.1', cartodb.CDB_Federated_Server_Diagnostics(server => 'doesNotExist') \echo '%% It returns a jsonb object' SELECT '1.2', pg_typeof(cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback')); +\echo '%% It returns the server version' +SELECT '1.3', cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback') @> '{"server_version": "14.0"}'::jsonb; + -- =================================================================== -- Cleanup diff --git a/test/CDB_FederatedServerDiagnostics_expect b/test/CDB_FederatedServerDiagnostics_expect index b8ffd9e..9ec5b49 100644 --- a/test/CDB_FederatedServerDiagnostics_expect +++ b/test/CDB_FederatedServerDiagnostics_expect @@ -3,4 +3,6 @@ C1| ERROR: Server "doesNotExist" does not exist %% It returns a jsonb object 1.2|jsonb +%% It returns the server version +1.3|t D1| From efd757ffa32f2148125893836fac92f48de69455 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Fri, 8 Nov 2019 18:18:28 +0100 Subject: [PATCH 04/16] Check against correct server version --- test/CDB_FederatedServerDiagnostics.sql | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/CDB_FederatedServerDiagnostics.sql b/test/CDB_FederatedServerDiagnostics.sql index 6783118..e26b615 100644 --- a/test/CDB_FederatedServerDiagnostics.sql +++ b/test/CDB_FederatedServerDiagnostics.sql @@ -35,7 +35,8 @@ SELECT '1.1', cartodb.CDB_Federated_Server_Diagnostics(server => 'doesNotExist') SELECT '1.2', pg_typeof(cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback')); \echo '%% It returns the server version' -SELECT '1.3', cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback') @> '{"server_version": "14.0"}'::jsonb; +SELECT '1.3', cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback') @> format('{"server_version": "%s"}', setting)::jsonb + FROM pg_settings WHERE name = 'server_version'; -- =================================================================== From 87302920975ff64112ef6964d71cf68b7eaf9cd2 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Fri, 8 Nov 2019 19:20:08 +0100 Subject: [PATCH 05/16] Function to retrieve remote PG server ver --- .../CDB_FederatedServerDiagnostics.sql | 41 ++++++++++++++++--- test/CDB_FederatedServerDiagnostics.sql | 2 + 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/scripts-available/CDB_FederatedServerDiagnostics.sql b/scripts-available/CDB_FederatedServerDiagnostics.sql index fd00e95..e55f90f 100644 --- a/scripts-available/CDB_FederatedServerDiagnostics.sql +++ b/scripts-available/CDB_FederatedServerDiagnostics.sql @@ -2,22 +2,53 @@ -- Private functions -------------------------------------------------------------------------------- -CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_Server_Version_PG(server_internal name) +-- +-- Get the version of a remote PG server +-- +CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_Foreign_Server_Version_PG(server_internal name) RETURNS text AS $$ +DECLARE + -- Import pg_settings from pg_catalog + remote_schema name := 'pg_catalog'; + remote_table name := 'pg_settings'; + local_schema name := @extschema@.__CDB_FS_Create_Schema(server_internal, remote_schema); + role_name name := @extschema@.__CDB_FS_Generate_Server_Role_Name(server_internal); + remote_server_version text; BEGIN - -- TODO Implement - RETURN '14.0'; + -- Import the foreign pg_settings table + IF NOT EXISTS ( + SELECT * FROM pg_class + WHERE relnamespace = (SELECT oid FROM pg_namespace WHERE nspname = local_schema) + AND relname = remote_table + ) THEN + EXECUTE format('IMPORT FOREIGN SCHEMA %I LIMIT TO (%I) FROM SERVER %I INTO %I', + remote_schema, remote_table, server_internal, local_schema); + END IF; + + BEGIN + EXECUTE format(' + SELECT setting FROM %I.%I WHERE name = ''server_version''; + ', local_schema, remote_table) INTO remote_server_version; + EXCEPTION WHEN OTHERS THEN + RAISE EXCEPTION 'Not enough permissions to access the server "%"', + @extschema@.__CDB_FS_Extract_Server_Name(server_internal); + END; + + RETURN remote_server_version; END $$ LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE; +-- +-- Collect and return diagnostics info from a remote PG into a jsonb +-- CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_Server_Diagnostics_PG(server_internal name) RETURNS jsonb AS $$ DECLARE - remote_server_version text := @extschema@.__CDB_FS_Server_Version_PG(server_internal); + remote_server_version text := @extschema@.__CDB_FS_Foreign_Server_Version_PG(server_internal); BEGIN RETURN jsonb_build_object('server_version', remote_server_version); END @@ -31,7 +62,7 @@ LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE; -------------------------------------------------------------------------------- -- --- TODO: function documentation +-- Collect and return diagnostics info from a remote PG into a jsonb -- CREATE OR REPLACE FUNCTION @extschema@.CDB_Federated_Server_Diagnostics(server TEXT) RETURNS jsonb diff --git a/test/CDB_FederatedServerDiagnostics.sql b/test/CDB_FederatedServerDiagnostics.sql index e26b615..a94776d 100644 --- a/test/CDB_FederatedServerDiagnostics.sql +++ b/test/CDB_FederatedServerDiagnostics.sql @@ -44,6 +44,8 @@ SELECT '1.3', cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback') @> -- =================================================================== \set QUIET on SELECT 'D1', cartodb.CDB_Federated_Server_Unregister(server => 'loopback'::text); +-- Reconnect, using a new session in order to close FDW connections +\connect DROP DATABASE cdb_fs_tester; -- Drop role From d4bc69cd3c773e0ef6435b66b59ef837f7fe642c Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Fri, 8 Nov 2019 19:49:16 +0100 Subject: [PATCH 06/16] Return postgis version in diagnostics info (WIP) --- .../CDB_FederatedServerDiagnostics.sql | 19 +++++++++++++++++-- test/CDB_FederatedServerDiagnostics.sql | 4 ++++ test/CDB_FederatedServerDiagnostics_expect | 2 ++ 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/scripts-available/CDB_FederatedServerDiagnostics.sql b/scripts-available/CDB_FederatedServerDiagnostics.sql index e55f90f..edc0851 100644 --- a/scripts-available/CDB_FederatedServerDiagnostics.sql +++ b/scripts-available/CDB_FederatedServerDiagnostics.sql @@ -41,6 +41,17 @@ $$ LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE; +CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_Foreign_PostGIS_Version_PG(server_internal name) +RETURNS text +AS $$ +BEGIN + -- TODO implement + RETURN '4.0'; +END +$$ +LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE; + + -- -- Collect and return diagnostics info from a remote PG into a jsonb -- @@ -48,9 +59,13 @@ CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_Server_Diagnostics_PG(server_int RETURNS jsonb AS $$ DECLARE - remote_server_version text := @extschema@.__CDB_FS_Foreign_Server_Version_PG(server_internal); + remote_server_version text := @extschema@.__CDB_FS_Foreign_Server_Version_PG(server_internal); + remote_postgis_version text := @extschema@.__CDB_FS_Foreign_PostGIS_Version_PG(server_internal); BEGIN - RETURN jsonb_build_object('server_version', remote_server_version); + RETURN jsonb_build_object( + 'server_version', remote_server_version, + 'postgis_version', remote_postgis_version + ); END $$ LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE; diff --git a/test/CDB_FederatedServerDiagnostics.sql b/test/CDB_FederatedServerDiagnostics.sql index a94776d..864f012 100644 --- a/test/CDB_FederatedServerDiagnostics.sql +++ b/test/CDB_FederatedServerDiagnostics.sql @@ -38,6 +38,10 @@ SELECT '1.2', pg_typeof(cartodb.CDB_Federated_Server_Diagnostics(server => 'loop SELECT '1.3', cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback') @> format('{"server_version": "%s"}', setting)::jsonb FROM pg_settings WHERE name = 'server_version'; +\echo '%% It returns the postgis version' +SELECT '1.4', cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback') @> format('{"postgis_version": "%s"}', extversion)::jsonb + FROM pg_extension WHERE extname = 'postgis'; + -- =================================================================== -- Cleanup diff --git a/test/CDB_FederatedServerDiagnostics_expect b/test/CDB_FederatedServerDiagnostics_expect index 9ec5b49..1c58eda 100644 --- a/test/CDB_FederatedServerDiagnostics_expect +++ b/test/CDB_FederatedServerDiagnostics_expect @@ -5,4 +5,6 @@ ERROR: Server "doesNotExist" does not exist 1.2|jsonb %% It returns the server version 1.3|t +%% It returns the postgis version +1.4|t D1| From 80f01d4a51a2c8593f388e10f14eb784dc3743c4 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Fri, 8 Nov 2019 20:02:51 +0100 Subject: [PATCH 07/16] Implement retrieval of remote postgis version --- .../CDB_FederatedServerDiagnostics.sql | 32 +++++++++++++++++-- test/CDB_FederatedServerDiagnostics.sql | 4 +++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/scripts-available/CDB_FederatedServerDiagnostics.sql b/scripts-available/CDB_FederatedServerDiagnostics.sql index edc0851..5b327b2 100644 --- a/scripts-available/CDB_FederatedServerDiagnostics.sql +++ b/scripts-available/CDB_FederatedServerDiagnostics.sql @@ -41,12 +41,40 @@ $$ LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE; +-- +-- Get the PostGIS extension version of a remote PG server +-- CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_Foreign_PostGIS_Version_PG(server_internal name) RETURNS text AS $$ +DECLARE + -- Import pg_settings from pg_catalog + remote_schema name := 'pg_catalog'; + remote_table name := 'pg_extension'; + local_schema name := @extschema@.__CDB_FS_Create_Schema(server_internal, remote_schema); + role_name name := @extschema@.__CDB_FS_Generate_Server_Role_Name(server_internal); + remote_postgis_version text; BEGIN - -- TODO implement - RETURN '4.0'; + -- Import the foreign pg_extension table + IF NOT EXISTS ( + SELECT * FROM pg_class + WHERE relnamespace = (SELECT oid FROM pg_namespace WHERE nspname = local_schema) + AND relname = remote_table + ) THEN + EXECUTE format('IMPORT FOREIGN SCHEMA %I LIMIT TO (%I) FROM SERVER %I INTO %I', + remote_schema, remote_table, server_internal, local_schema); + END IF; + + BEGIN + EXECUTE format(' + SELECT extversion FROM %I.%I WHERE extname = ''postgis''; + ', local_schema, remote_table) INTO remote_postgis_version; + EXCEPTION WHEN OTHERS THEN + RAISE EXCEPTION 'Not enough permissions to access the server "%"', + @extschema@.__CDB_FS_Extract_Server_Name(server_internal); + END; + + RETURN remote_postgis_version; END $$ LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE; diff --git a/test/CDB_FederatedServerDiagnostics.sql b/test/CDB_FederatedServerDiagnostics.sql index 864f012..e3b880c 100644 --- a/test/CDB_FederatedServerDiagnostics.sql +++ b/test/CDB_FederatedServerDiagnostics.sql @@ -22,6 +22,10 @@ SELECT 'C1', cartodb.CDB_Federated_Server_Register_PG(server => 'loopback'::text "password": "cdb_fs_passwd" } }'::jsonb); + +\c cdb_fs_tester postgres +CREATE EXTENSION postgis; +\c contrib_regression postgres \set QUIET off From 511e24a40ebfd60f7bca5cfd5bdbe9e5353983b7 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Fri, 8 Nov 2019 20:05:08 +0100 Subject: [PATCH 08/16] Remove unused var role_name --- scripts-available/CDB_FederatedServerDiagnostics.sql | 2 -- 1 file changed, 2 deletions(-) diff --git a/scripts-available/CDB_FederatedServerDiagnostics.sql b/scripts-available/CDB_FederatedServerDiagnostics.sql index 5b327b2..ee5992e 100644 --- a/scripts-available/CDB_FederatedServerDiagnostics.sql +++ b/scripts-available/CDB_FederatedServerDiagnostics.sql @@ -13,7 +13,6 @@ DECLARE remote_schema name := 'pg_catalog'; remote_table name := 'pg_settings'; local_schema name := @extschema@.__CDB_FS_Create_Schema(server_internal, remote_schema); - role_name name := @extschema@.__CDB_FS_Generate_Server_Role_Name(server_internal); remote_server_version text; BEGIN -- Import the foreign pg_settings table @@ -52,7 +51,6 @@ DECLARE remote_schema name := 'pg_catalog'; remote_table name := 'pg_extension'; local_schema name := @extschema@.__CDB_FS_Create_Schema(server_internal, remote_schema); - role_name name := @extschema@.__CDB_FS_Generate_Server_Role_Name(server_internal); remote_postgis_version text; BEGIN -- Import the foreign pg_extension table From 6700d24232ab9bc58b7c90af763c92b21f791309 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Fri, 8 Nov 2019 20:16:26 +0100 Subject: [PATCH 09/16] Refactor: extract __CDB_FS_Import_If_Not_Exists --- .../CDB_FederatedServerDiagnostics.sql | 39 ++++++++++--------- 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/scripts-available/CDB_FederatedServerDiagnostics.sql b/scripts-available/CDB_FederatedServerDiagnostics.sql index ee5992e..8f451af 100644 --- a/scripts-available/CDB_FederatedServerDiagnostics.sql +++ b/scripts-available/CDB_FederatedServerDiagnostics.sql @@ -3,19 +3,14 @@ -------------------------------------------------------------------------------- -- --- Get the version of a remote PG server +-- Import a foreign table if it does not exist -- -CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_Foreign_Server_Version_PG(server_internal name) -RETURNS text +CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_Import_If_Not_Exists(server_internal name, remote_schema name, remote_table name) +RETURNS void AS $$ DECLARE - -- Import pg_settings from pg_catalog - remote_schema name := 'pg_catalog'; - remote_table name := 'pg_settings'; local_schema name := @extschema@.__CDB_FS_Create_Schema(server_internal, remote_schema); - remote_server_version text; BEGIN - -- Import the foreign pg_settings table IF NOT EXISTS ( SELECT * FROM pg_class WHERE relnamespace = (SELECT oid FROM pg_namespace WHERE nspname = local_schema) @@ -24,6 +19,23 @@ BEGIN EXECUTE format('IMPORT FOREIGN SCHEMA %I LIMIT TO (%I) FROM SERVER %I INTO %I', remote_schema, remote_table, server_internal, local_schema); END IF; +END +$$ +LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE; + +-- +-- Get the version of a remote PG server +-- +CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_Foreign_Server_Version_PG(server_internal name) +RETURNS text +AS $$ +DECLARE + remote_schema name := 'pg_catalog'; + remote_table name := 'pg_settings'; + local_schema name := @extschema@.__CDB_FS_Create_Schema(server_internal, remote_schema); + remote_server_version text; +BEGIN + PERFORM @extschema@.__CDB_FS_Import_If_Not_Exists(server_internal, remote_schema, remote_table); BEGIN EXECUTE format(' @@ -47,21 +59,12 @@ CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_Foreign_PostGIS_Version_PG(serve RETURNS text AS $$ DECLARE - -- Import pg_settings from pg_catalog remote_schema name := 'pg_catalog'; remote_table name := 'pg_extension'; local_schema name := @extschema@.__CDB_FS_Create_Schema(server_internal, remote_schema); remote_postgis_version text; BEGIN - -- Import the foreign pg_extension table - IF NOT EXISTS ( - SELECT * FROM pg_class - WHERE relnamespace = (SELECT oid FROM pg_namespace WHERE nspname = local_schema) - AND relname = remote_table - ) THEN - EXECUTE format('IMPORT FOREIGN SCHEMA %I LIMIT TO (%I) FROM SERVER %I INTO %I', - remote_schema, remote_table, server_internal, local_schema); - END IF; + PERFORM @extschema@.__CDB_FS_Import_If_Not_Exists(server_internal, remote_schema, remote_table); BEGIN EXECUTE format(' From eba6cf45659ccb83f6c6aac26f26b184445ddf5e Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Fri, 8 Nov 2019 20:27:24 +0100 Subject: [PATCH 10/16] Test when postgis is not installed in the remote --- test/CDB_FederatedServerDiagnostics.sql | 8 ++++++++ test/CDB_FederatedServerDiagnostics_expect | 2 ++ 2 files changed, 10 insertions(+) diff --git a/test/CDB_FederatedServerDiagnostics.sql b/test/CDB_FederatedServerDiagnostics.sql index e3b880c..ef0dbf1 100644 --- a/test/CDB_FederatedServerDiagnostics.sql +++ b/test/CDB_FederatedServerDiagnostics.sql @@ -46,6 +46,14 @@ SELECT '1.3', cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback') @> SELECT '1.4', cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback') @> format('{"postgis_version": "%s"}', extversion)::jsonb FROM pg_extension WHERE extname = 'postgis'; +\echo '%% It returns null as the postgis version if it is not installed' +\set QUIET on +\c cdb_fs_tester postgres +DROP EXTENSION postgis; +\c contrib_regression postgres +\set QUIET off +SELECT '1.5', cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback') @> '{"postgis_version": null}'::jsonb; + -- =================================================================== -- Cleanup diff --git a/test/CDB_FederatedServerDiagnostics_expect b/test/CDB_FederatedServerDiagnostics_expect index 1c58eda..bff416f 100644 --- a/test/CDB_FederatedServerDiagnostics_expect +++ b/test/CDB_FederatedServerDiagnostics_expect @@ -7,4 +7,6 @@ ERROR: Server "doesNotExist" does not exist 1.3|t %% It returns the postgis version 1.4|t +%% It returns null as the postgis version if it is not installed +1.5|t D1| From d76e3ccc3e30358bd8bf2442ad0e4e917c58f248 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Mon, 11 Nov 2019 15:56:17 +0100 Subject: [PATCH 11/16] Add server options to diagnostics --- .../CDB_FederatedServerDiagnostics.sql | 20 ++++++++++++++++++- test/CDB_FederatedServerDiagnostics.sql | 3 +++ test/CDB_FederatedServerDiagnostics_expect | 2 ++ 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/scripts-available/CDB_FederatedServerDiagnostics.sql b/scripts-available/CDB_FederatedServerDiagnostics.sql index 8f451af..e05732d 100644 --- a/scripts-available/CDB_FederatedServerDiagnostics.sql +++ b/scripts-available/CDB_FederatedServerDiagnostics.sql @@ -81,6 +81,22 @@ $$ LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE; +-- +-- Get the foreign server options of a remote PG server +-- +CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_Foreign_Server_Options_PG(server_internal name) +RETURNS jsonb +AS $$ + -- See https://www.postgresql.org/docs/current/catalog-pg-foreign-server.html + SELECT jsonb_object(server_options) FROM ( + SELECT ARRAY(SELECT regexp_split_to_table(opts, '=') + FROM pg_foreign_server, unnest(srvoptions) opts + WHERE srvname = cartodb.__CDB_FS_Generate_Server_Name(input_name => 'loopback', check_existence => true)) server_options + ) q; +$$ +LANGUAGE SQL VOLATILE PARALLEL UNSAFE; + + -- -- Collect and return diagnostics info from a remote PG into a jsonb -- @@ -90,10 +106,12 @@ AS $$ DECLARE remote_server_version text := @extschema@.__CDB_FS_Foreign_Server_Version_PG(server_internal); remote_postgis_version text := @extschema@.__CDB_FS_Foreign_PostGIS_Version_PG(server_internal); + remote_server_options jsonb := @extschema@.__CDB_FS_Foreign_Server_Options_PG(server_internal); BEGIN RETURN jsonb_build_object( 'server_version', remote_server_version, - 'postgis_version', remote_postgis_version + 'postgis_version', remote_postgis_version, + 'server_options', remote_server_options ); END $$ diff --git a/test/CDB_FederatedServerDiagnostics.sql b/test/CDB_FederatedServerDiagnostics.sql index ef0dbf1..f66e563 100644 --- a/test/CDB_FederatedServerDiagnostics.sql +++ b/test/CDB_FederatedServerDiagnostics.sql @@ -54,6 +54,9 @@ DROP EXTENSION postgis; \set QUIET off SELECT '1.5', cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback') @> '{"postgis_version": null}'::jsonb; +\echo '%% It returns the remote server options' +SELECT '1.6', cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback') @> '{"server_options": {"host": "localhost", "port": "5432", "updatable": "false", "extensions": "postgis", "fetch_size": "1000", "use_remote_estimate": "true"}}'::jsonb; + -- =================================================================== -- Cleanup diff --git a/test/CDB_FederatedServerDiagnostics_expect b/test/CDB_FederatedServerDiagnostics_expect index bff416f..2f1f7ae 100644 --- a/test/CDB_FederatedServerDiagnostics_expect +++ b/test/CDB_FederatedServerDiagnostics_expect @@ -9,4 +9,6 @@ ERROR: Server "doesNotExist" does not exist 1.4|t %% It returns null as the postgis version if it is not installed 1.5|t +%% It returns the remote server options +1.6|t D1| From 5c9f6964a321558c6005ab26418f6467d8539251 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Mon, 11 Nov 2019 18:46:07 +0100 Subject: [PATCH 12/16] Fix typo: hardcoded loopback server --- scripts-available/CDB_FederatedServerDiagnostics.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts-available/CDB_FederatedServerDiagnostics.sql b/scripts-available/CDB_FederatedServerDiagnostics.sql index e05732d..abbb0d8 100644 --- a/scripts-available/CDB_FederatedServerDiagnostics.sql +++ b/scripts-available/CDB_FederatedServerDiagnostics.sql @@ -91,7 +91,7 @@ AS $$ SELECT jsonb_object(server_options) FROM ( SELECT ARRAY(SELECT regexp_split_to_table(opts, '=') FROM pg_foreign_server, unnest(srvoptions) opts - WHERE srvname = cartodb.__CDB_FS_Generate_Server_Name(input_name => 'loopback', check_existence => true)) server_options + WHERE srvname = server_internal) server_options ) q; $$ LANGUAGE SQL VOLATILE PARALLEL UNSAFE; From d87d27d7e50685a571467a2438d16fa08163c2f9 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Mon, 11 Nov 2019 20:42:00 +0100 Subject: [PATCH 13/16] Use pg_options_to_table instead of manual parsing --- .../CDB_FederatedServerDiagnostics.sql | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/scripts-available/CDB_FederatedServerDiagnostics.sql b/scripts-available/CDB_FederatedServerDiagnostics.sql index abbb0d8..7bf5bee 100644 --- a/scripts-available/CDB_FederatedServerDiagnostics.sql +++ b/scripts-available/CDB_FederatedServerDiagnostics.sql @@ -87,14 +87,23 @@ LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE; CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_Foreign_Server_Options_PG(server_internal name) RETURNS jsonb AS $$ +DECLARE + res jsonb := '{}'; + opt record; +BEGIN -- See https://www.postgresql.org/docs/current/catalog-pg-foreign-server.html - SELECT jsonb_object(server_options) FROM ( - SELECT ARRAY(SELECT regexp_split_to_table(opts, '=') - FROM pg_foreign_server, unnest(srvoptions) opts - WHERE srvname = server_internal) server_options - ) q; + -- See https://www.postgresql.org/docs/current/functions-info.html + + -- SELECT jsonb_build_object(opt.option_name, opt.option_value) FROM (SELECT (pg_options_to_table(srvoptions)).* FROM pg_foreign_server WHERE srvname = 'cdb_fdw_local_pg11') opt; + FOR opt IN + SELECT (pg_options_to_table(srvoptions)).* FROM pg_foreign_server WHERE srvname = server_internal + LOOP + res := res || jsonb_build_object(opt.option_name, opt.option_value); + END LOOP; + RETURN res; +END $$ -LANGUAGE SQL VOLATILE PARALLEL UNSAFE; +LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE; -- From 6e9ee296c594239db5689c25fbd5553bfcb96a3f Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Mon, 11 Nov 2019 20:48:14 +0100 Subject: [PATCH 14/16] Remove useless commented code --- scripts-available/CDB_FederatedServerDiagnostics.sql | 2 -- 1 file changed, 2 deletions(-) diff --git a/scripts-available/CDB_FederatedServerDiagnostics.sql b/scripts-available/CDB_FederatedServerDiagnostics.sql index 7bf5bee..bf2524e 100644 --- a/scripts-available/CDB_FederatedServerDiagnostics.sql +++ b/scripts-available/CDB_FederatedServerDiagnostics.sql @@ -93,8 +93,6 @@ DECLARE BEGIN -- See https://www.postgresql.org/docs/current/catalog-pg-foreign-server.html -- See https://www.postgresql.org/docs/current/functions-info.html - - -- SELECT jsonb_build_object(opt.option_name, opt.option_value) FROM (SELECT (pg_options_to_table(srvoptions)).* FROM pg_foreign_server WHERE srvname = 'cdb_fdw_local_pg11') opt; FOR opt IN SELECT (pg_options_to_table(srvoptions)).* FROM pg_foreign_server WHERE srvname = server_internal LOOP From 0ad7cd485df0c88c5811e04a7e1b2c3545f781dc Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Mon, 11 Nov 2019 21:34:51 +0100 Subject: [PATCH 15/16] Simplify function with aggregate --- .../CDB_FederatedServerDiagnostics.sql | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/scripts-available/CDB_FederatedServerDiagnostics.sql b/scripts-available/CDB_FederatedServerDiagnostics.sql index bf2524e..459338d 100644 --- a/scripts-available/CDB_FederatedServerDiagnostics.sql +++ b/scripts-available/CDB_FederatedServerDiagnostics.sql @@ -87,21 +87,15 @@ LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE; CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_Foreign_Server_Options_PG(server_internal name) RETURNS jsonb AS $$ -DECLARE - res jsonb := '{}'; - opt record; -BEGIN -- See https://www.postgresql.org/docs/current/catalog-pg-foreign-server.html -- See https://www.postgresql.org/docs/current/functions-info.html - FOR opt IN - SELECT (pg_options_to_table(srvoptions)).* FROM pg_foreign_server WHERE srvname = server_internal - LOOP - res := res || jsonb_build_object(opt.option_name, opt.option_value); - END LOOP; - RETURN res; -END + SELECT jsonb_object_agg(opt.option_name, opt.option_value) FROM ( + SELECT (pg_options_to_table(srvoptions)).* + FROM pg_foreign_server + WHERE srvname = server_internal + ) AS opt; $$ -LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE; +LANGUAGE SQL VOLATILE PARALLEL UNSAFE; -- From d5ecb3925041dd3aaf36290ad9e0a32a8a316d38 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Tue, 12 Nov 2019 10:32:25 +0100 Subject: [PATCH 16/16] Replace hardcoded 5432 by @@PGPORT@@ as requested in review comment --- test/CDB_FederatedServerDiagnostics.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/CDB_FederatedServerDiagnostics.sql b/test/CDB_FederatedServerDiagnostics.sql index f66e563..d626004 100644 --- a/test/CDB_FederatedServerDiagnostics.sql +++ b/test/CDB_FederatedServerDiagnostics.sql @@ -55,7 +55,7 @@ DROP EXTENSION postgis; SELECT '1.5', cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback') @> '{"postgis_version": null}'::jsonb; \echo '%% It returns the remote server options' -SELECT '1.6', cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback') @> '{"server_options": {"host": "localhost", "port": "5432", "updatable": "false", "extensions": "postgis", "fetch_size": "1000", "use_remote_estimate": "true"}}'::jsonb; +SELECT '1.6', cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback') @> '{"server_options": {"host": "localhost", "port": "@@PGPORT@@", "updatable": "false", "extensions": "postgis", "fetch_size": "1000", "use_remote_estimate": "true"}}'::jsonb; -- ===================================================================