Test around NULLs and invalids arguments

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;

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 "%": %',
+        RAISE EXCEPTION 'Not enough permissions to access the server "%"',
-                        @extschema@.__CDB_FS_Extract_Server_Name(server_internal), SQLERRM;
+                        @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);

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;

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

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".

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
+\echo '## Registering a table: Invalid server fails'
--- Try to register with invalid / NULL schema
+SELECT cartodb.CDB_Federated_Table_Register(
--- Try to register with invalid / NULL table
+    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

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 "<NULL>"
+## 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|