diff --git a/scripts-available/CDB_FederatedServer.sql b/scripts-available/CDB_FederatedServer.sql index 1d55198..0e113a7 100644 --- a/scripts-available/CDB_FederatedServer.sql +++ b/scripts-available/CDB_FederatedServer.sql @@ -64,9 +64,11 @@ RETURNS NAME AS $$ DECLARE hash_value text := md5(internal_server_name::text || '__' || schema_name::text); - schema_name text := format('%s%s%s', @extschema@.__CDB_FS_Name_Pattern(), 'schema_', hash_value); BEGIN - RETURN schema_name::name; + IF schema_name IS NULL THEN + RAISE EXCEPTION 'Schema name cannot be NULL'; + END IF; + RETURN format('%s%s%s', @extschema@.__CDB_FS_Name_Pattern(), 'schema_', hash_value)::name; END $$ LANGUAGE PLPGSQL IMMUTABLE PARALLEL SAFE; @@ -102,7 +104,11 @@ BEGIN -- easy way to check for permissions and keep all objects under the same owner BEGIN EXECUTE 'SET LOCAL ROLE ' || quote_ident(role_name); - EXCEPTION WHEN OTHERS THEN + EXCEPTION + WHEN invalid_parameter_value THEN + RAISE EXCEPTION 'Server "%" does not exist', + @extschema@.__CDB_FS_Extract_Server_Name(internal_server_name); + WHEN OTHERS THEN RAISE EXCEPTION 'Not enough permissions to access the server "%"', @extschema@.__CDB_FS_Extract_Server_Name(internal_server_name); END; diff --git a/scripts-available/CDB_FederatedServerListRemote.sql b/scripts-available/CDB_FederatedServerListRemote.sql index 09fb678..d8932c0 100644 --- a/scripts-available/CDB_FederatedServerListRemote.sql +++ b/scripts-available/CDB_FederatedServerListRemote.sql @@ -41,8 +41,8 @@ BEGIN ORDER BY remote_schema ', local_schema, remote_table, '''pg_%'''); EXCEPTION WHEN OTHERS THEN - RAISE EXCEPTION 'Not enough permissions to access the server "%": %', - @extschema@.__CDB_FS_Extract_Server_Name(server_internal), SQLERRM; + RAISE EXCEPTION 'Not enough permissions to access the server "%"', + @extschema@.__CDB_FS_Extract_Server_Name(server_internal); END; END $$ @@ -238,6 +238,10 @@ 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 + IF remote_table IS NULL THEN + RAISE EXCEPTION 'Remote table name cannot be NULL'; + END IF; + CASE server_type WHEN 'postgres_fdw' THEN RETURN QUERY SELECT * FROM @extschema@.__CDB_FS_List_Foreign_Columns_PG(server_internal, remote_schema, remote_table); diff --git a/scripts-available/CDB_FederatedServerTables.sql b/scripts-available/CDB_FederatedServerTables.sql index bd93445..db43d1c 100644 --- a/scripts-available/CDB_FederatedServerTables.sql +++ b/scripts-available/CDB_FederatedServerTables.sql @@ -104,6 +104,10 @@ DECLARE webmercator_expression TEXT; carto_columns_expression TEXT[]; BEGIN + IF remote_table IS NULL THEN + RAISE EXCEPTION 'Remote table name cannot be NULL'; + END IF; + -- Use geom_column as default for webmercator_column IF webmercator_column IS NULL THEN webmercator_column := geom_column; diff --git a/test/CDB_FederatedServerListRemote.sql b/test/CDB_FederatedServerListRemote.sql index f72fb42..e230ab0 100644 --- a/test/CDB_FederatedServerListRemote.sql +++ b/test/CDB_FederatedServerListRemote.sql @@ -200,6 +200,9 @@ SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Columns(server => 'loopba \echo '## Test listing of remote columns (rainy day): Remote table does not exist' SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Columns(server => 'loopback', remote_schema => 'S 1', remote_table => 'Does Not Exist'); +\echo '## Test listing of remote columns (rainy day): Remote table is NULL' +SELECT * FROM cartodb.CDB_Federated_Server_List_Remote_Columns(server => 'loopback', remote_schema => 'S 1', remote_table => NULL::text); + -- =================================================================== -- Test that using a different user to list tables and dropping it diff --git a/test/CDB_FederatedServerListRemote_expect b/test/CDB_FederatedServerListRemote_expect index 3228992..e05af3a 100644 --- a/test/CDB_FederatedServerListRemote_expect +++ b/test/CDB_FederatedServerListRemote_expect @@ -97,6 +97,8 @@ You are now connected to database "contrib_regression" as user "postgres". ERROR: Server "Does Not Exist" does not exist ## Test listing of remote columns (rainy day): Remote schema does not exist ## Test listing of remote columns (rainy day): Remote table does not exist +## Test listing of remote columns (rainy day): Remote table is NULL +ERROR: Remote table name cannot be NULL ## Test listing of remote objects with permissions (sunny day) You are now connected to database "contrib_regression" as user "cdb_fs_tester2". diff --git a/test/CDB_FederatedServerTables.sql b/test/CDB_FederatedServerTables.sql index 7ed7630..604dcfd 100644 --- a/test/CDB_FederatedServerTables.sql +++ b/test/CDB_FederatedServerTables.sql @@ -4,6 +4,7 @@ \set QUIET on SET client_min_messages TO error; \set VERBOSITY terse + SET SESSION AUTHORIZATION postgres; CREATE EXTENSION postgres_fdw; CREATE ROLE cdb_fs_tester SUPERUSER LOGIN PASSWORD 'cdb_fs_passwd'; @@ -121,9 +122,126 @@ Select 'list_remotes3', CDB_Federated_Server_List_Registered_Tables( remote_schema => 'remote_schema' ); --- Try to register with invalid / NULL server --- Try to register with invalid / NULL schema --- Try to register with invalid / NULL table +\echo '## Registering a table: Invalid server fails' +SELECT cartodb.CDB_Federated_Table_Register( + server => 'Does not exist', + remote_schema => 'remote_schema', + remote_table => 'remote_geom', + id_column => 'id', + geom_column => 'geom' + ); + +\echo '## Registering a table: NULL server fails' +SELECT cartodb.CDB_Federated_Table_Register( + server => NULL::text, + remote_schema => 'remote_schema', + remote_table => 'remote_geom', + id_column => 'id', + geom_column => 'geom' + ); + +\echo '## Registering a table: Invalid schema fails' +SELECT cartodb.CDB_Federated_Table_Register( + server => 'loopback', + remote_schema => 'Does not exist', + remote_table => 'remote_geom', + id_column => 'id', + geom_column => 'geom' + ); + +\echo '## Registering a table: NULL schema fails' +SELECT cartodb.CDB_Federated_Table_Register( + server => 'loopback', + remote_schema => NULL::text, + remote_table => 'remote_geom', + id_column => 'id', + geom_column => 'geom' + ); + +\echo '## Registering a table: Invalid table fails' +SELECT cartodb.CDB_Federated_Table_Register( + server => 'loopback', + remote_schema => 'remote_schema', + remote_table => 'Does not exist', + id_column => 'id', + geom_column => 'geom' + ); + +\echo '## Registering a table: NULL table fails' +SELECT cartodb.CDB_Federated_Table_Register( + server => 'loopback', + remote_schema => 'remote_schema', + remote_table => NULL::text, + id_column => 'id', + geom_column => 'geom' + ); + +\echo '## Registering a table: Invalid id fails' +SELECT cartodb.CDB_Federated_Table_Register( + server => 'loopback', + remote_schema => 'remote_schema', + remote_table => 'remote_geom', + id_column => 'Does not exist', + geom_column => 'geom' + ); + +\echo '## Registering a table: NULL id fails' +SELECT cartodb.CDB_Federated_Table_Register( + server => 'loopback', + remote_schema => 'remote_schema', + remote_table => 'remote_geom', + id_column => NULL::text, + geom_column => 'geom' + ); + +\echo '## Registering a table: Invalid geom_column fails' +SELECT cartodb.CDB_Federated_Table_Register( + server => 'loopback', + remote_schema => 'remote_schema', + remote_table => 'remote_geom', + id_column => 'id', + geom_column => 'Does not exists' + ); + +\echo '## Registering a table: NULL geom_column is OK' +SELECT cartodb.CDB_Federated_Table_Register( + server => 'loopback', + remote_schema => 'remote_schema', + remote_table => 'remote_geom', + id_column => 'id', + geom_column => NULL::text + ); +SELECT cartodb.CDB_Federated_Table_Unregister( + server => 'loopback', + remote_schema => 'remote_schema', + remote_table => 'remote_geom' + ); + +\echo '## Registering a table: Invalid webmercator_column fails' +SELECT cartodb.CDB_Federated_Table_Register( + server => 'loopback', + remote_schema => 'remote_schema', + remote_table => 'remote_geom', + id_column => 'id', + geom_column => 'geom', + webmercator_column => 'Does not exists' + ); + +\echo '## Registering a table: NULL webmercator_column is OK' +SELECT cartodb.CDB_Federated_Table_Register( + server => 'loopback', + remote_schema => 'remote_schema', + remote_table => 'remote_geom', + id_column => 'id', + geom_column => 'geom', + webmercator_column => NULL::text + ); +SELECT cartodb.CDB_Federated_Table_Unregister( + server => 'loopback', + remote_schema => 'remote_schema', + remote_table => 'remote_geom' + ); + -- Try to register with invalid / NULL id -- Try to register with invalid / NULL geom_column -- Try to register with invalid / NULL webmercator_column @@ -132,6 +250,12 @@ Select 'list_remotes3', CDB_Federated_Server_List_Registered_Tables( -- Try permissions tricks +-- Try registering and accessing a table as normal user + +-- Try register with one user and reading it with other +-- Try register with one user and deleting it with another + + -- =================================================================== -- Cleanup diff --git a/test/CDB_FederatedServerTables_expect b/test/CDB_FederatedServerTables_expect index 7298a72..497c057 100644 --- a/test/CDB_FederatedServerTables_expect +++ b/test/CDB_FederatedServerTables_expect @@ -27,4 +27,30 @@ S3_new|3|POINT(3 3)|patata U1| ERROR: relation "remote_geom" does not exist at character 71 list_remotes3|(remote_geom2,public.different_name) +## Registering a table: Invalid server fails +ERROR: Server "Does not exist" does not exist +## Registering a table: NULL server fails +ERROR: Server name cannot be NULL +## Registering a table: Invalid schema fails +ERROR: schema "Does not exist" is not present on foreign server "cdb_fs_loopback" +## Registering a table: NULL schema fails +ERROR: Schema name cannot be NULL +## Registering a table: Invalid table fails +ERROR: relation "cdb_fs_schema_b904664b5208433cd85a1693ba4f7570.Does not exist" does not exist +## Registering a table: NULL table fails +ERROR: Remote table name cannot be NULL +## Registering a table: Invalid id fails +ERROR: non integer id_column "Does not exist" +## Registering a table: NULL id fails +ERROR: non integer id_column "" +## Registering a table: Invalid geom_column fails +ERROR: non geometry column "Does not exists" +## Registering a table: NULL geom_column is OK + + +## Registering a table: Invalid webmercator_column fails +ERROR: non geometry column "Does not exists" +## Registering a table: NULL webmercator_column is OK + + D1|