From c7311ba48e1a055501e9dfb36c4f3b9878e12927 Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@carto.com>
Date: Fri, 12 Jul 2019 12:00:30 +0200
Subject: [PATCH 01/28] A stub of a convenient FDW function

---
 scripts-available/CDB_ForeignTable.sql | 33 ++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/scripts-available/CDB_ForeignTable.sql b/scripts-available/CDB_ForeignTable.sql
index f60b043..66807c2 100644
--- a/scripts-available/CDB_ForeignTable.sql
+++ b/scripts-available/CDB_ForeignTable.sql
@@ -139,6 +139,39 @@ $$
 LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE;
 
 
+-- A function to set up a user-defined foreign data server
+-- It does not read from CDB_Conf
+--
+-- Sample call:
+-- SELECT cartodb.CDB_SetUp_Foreign_Server('amazon', '{
+--    "server": {
+--      "extensions": "postgis",
+--      "dbname": "testdb",
+--      "host": "myhostname.us-east-2.rds.amazonaws.com",
+--      "port": "5432"
+--    },
+--    "user": "fdw_user",
+--    "password": "secret"
+-- ');
+--
+-- Underneath it will:
+--   * Set up postgresql_fdw
+--   * Create a server with the name 'amazon'
+--   * Create a role called 'amazon' to manage access
+--   * Create a user mapping with that role 'amazon'
+--   * Create a schema 'amazon' as a convenience to set up all foreign
+--     tables over there
+--
+-- It is the responsibility of the caller to grant that role to either:
+--   * Nobody
+--   * Specific roles: GRANT amazon TO role_name;
+--   * Members of the organization: SELECT cartodb.CDB_Grant_Role_To_Org_Members('amazon'); TODO
+--   * The publicuser: GRANT amazon TO publicuser;
+CREATE OR REPLACE FUNCTION @extschema@.CDB_SetUp_Foreign_Server(fdw_name NAME, config json)
+RETURNS void AS $$
+-- TODO Code here
+$$ LANGUAGE SQL VOLATILE PARALLEL UNSAFE;
+
 CREATE OR REPLACE FUNCTION @extschema@._cdb_dbname_of_foreign_table(reloid oid)
 RETURNS TEXT AS $$
     SELECT option_value FROM pg_options_to_table((

From 4da89d8abd1719f491e668c5057ff7408df1d13c Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@carto.com>
Date: Fri, 12 Jul 2019 12:40:02 +0200
Subject: [PATCH 02/28] First version of the function WIP

---
 scripts-available/CDB_ForeignTable.sql | 73 ++++++++++++++++++++++++--
 1 file changed, 69 insertions(+), 4 deletions(-)

diff --git a/scripts-available/CDB_ForeignTable.sql b/scripts-available/CDB_ForeignTable.sql
index 66807c2..a64b54b 100644
--- a/scripts-available/CDB_ForeignTable.sql
+++ b/scripts-available/CDB_ForeignTable.sql
@@ -150,8 +150,10 @@ LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE;
 --      "host": "myhostname.us-east-2.rds.amazonaws.com",
 --      "port": "5432"
 --    },
---    "user": "fdw_user",
---    "password": "secret"
+--    "user_mapping": {
+--      "user": "fdw_user",
+--      "password": "secret"
+--    }
 -- ');
 --
 -- Underneath it will:
@@ -169,8 +171,71 @@ LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE;
 --   * The publicuser: GRANT amazon TO publicuser;
 CREATE OR REPLACE FUNCTION @extschema@.CDB_SetUp_Foreign_Server(fdw_name NAME, config json)
 RETURNS void AS $$
--- TODO Code here
-$$ LANGUAGE SQL VOLATILE PARALLEL UNSAFE;
+DECLARE
+  row record;
+  option record;
+BEGIN
+  -- TODO: refactor with original function
+  -- This function tries to be as idempotent as possible, by not creating anything more than once
+  -- (not even using IF NOT EXIST to avoid throwing warnings)
+  IF NOT EXISTS ( SELECT * FROM pg_extension WHERE extname = 'postgres_fdw') THEN
+    CREATE EXTENSION postgres_fdw;
+  END IF;
+  -- Create FDW first if it does not exist
+  IF NOT EXISTS ( SELECT * FROM pg_foreign_server WHERE srvname = fdw_name)
+    THEN
+    EXECUTE FORMAT('CREATE SERVER %I FOREIGN DATA WRAPPER postgres_fdw', fdw_name);
+  END IF;
+
+  -- Set FDW settings
+  FOR row IN SELECT p.key, p.value from lateral json_each_text(config->'server') p
+    LOOP
+      IF NOT EXISTS (WITH a AS (select split_part(unnest(srvoptions), '=', 1) as options from pg_foreign_server where srvname=fdw_name) SELECT * from a where options = row.key)
+        THEN
+        EXECUTE FORMAT('ALTER SERVER %I OPTIONS (ADD %I %L)', fdw_name, row.key, row.value);
+      ELSE
+        EXECUTE FORMAT('ALTER SERVER %I OPTIONS (SET %I %L)', fdw_name, row.key, row.value);
+      END IF;
+    END LOOP;
+
+    -- Create specific role for this
+    IF NOT EXISTS ( SELECT 1 FROM pg_roles WHERE rolname = fdw_name) THEN
+       EXECUTE format('CREATE ROLE %I NOLOGIN', fdw_name);
+    END IF;
+
+    -- Create user mapping
+    IF NOT EXISTS ( SELECT * FROM pg_user_mappings WHERE srvname = fdw_name AND usename = fdw_name ) THEN
+        EXECUTE FORMAT ('CREATE USER MAPPING FOR %I SERVER %I', fdw_name, fdw_name);
+    END IF;
+
+    -- Update user mapping settings
+    FOR option IN SELECT o.key, o.value from lateral json_each_text('user_mapping') o LOOP
+        IF NOT EXISTS (WITH a AS (select split_part(unnest(umoptions), '=', 1) as options from pg_user_mappings WHERE srvname = fdw_name AND usename = fdw_name) SELECT * from a where options = option.key) THEN
+          EXECUTE FORMAT('ALTER USER MAPPING FOR %I SERVER %I OPTIONS (ADD %I %L)', fdw_name, fdw_name, option.key, option.value);
+        ELSE
+          EXECUTE FORMAT('ALTER USER MAPPING FOR %I SERVER %I OPTIONS (SET %I %L)', fdw_name, fdw_name, option.key, option.value);
+        END IF;
+      END LOOP;
+    END LOOP;
+
+    -- Grant usage on the wrapper and server to the fdw role
+    EXECUTE FORMAT ('GRANT USAGE ON FOREIGN DATA WRAPPER postgres_fdw TO %I', fdw_name);
+    EXECUTE FORMAT ('GRANT USAGE ON FOREIGN ON FOREIGN SERVER %I TO %I', fdw_name, fdw_name);
+
+    -- Create schema if it does not exist.
+    IF NOT EXISTS ( SELECT * from pg_namespace WHERE nspname=fdw_name) THEN
+      EXECUTE FORMAT ('CREATE SCHEMA %I', fdw_name);
+    END IF;
+
+    -- Give the fdw role usage permisions over the schema
+    EXECUTE FORMAT ('GRANT USAGE ON SCHEMA %I TO %I', fdw_name, fdw_name);
+
+    -- Grant the fdw role to the caller, and permissions to grant it to others
+    EXECUTE FORMAT ('GRANT %I TO %I WITH ADMIN OPTION', fdw_name, session_user);
+
+    -- TODO: Bring here the remote cdb_tablemetadata
+
+$$ LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE SECURITY DEFINER;
 
 CREATE OR REPLACE FUNCTION @extschema@._cdb_dbname_of_foreign_table(reloid oid)
 RETURNS TEXT AS $$

From 524bb6ad420a249d90d65cf169ce6dea3b98a0f8 Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@carto.com>
Date: Fri, 12 Jul 2019 12:49:59 +0200
Subject: [PATCH 03/28] Fix copy/paste typos

---
 scripts-available/CDB_ForeignTable.sql | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/scripts-available/CDB_ForeignTable.sql b/scripts-available/CDB_ForeignTable.sql
index a64b54b..3fbf862 100644
--- a/scripts-available/CDB_ForeignTable.sql
+++ b/scripts-available/CDB_ForeignTable.sql
@@ -215,7 +215,6 @@ BEGIN
         ELSE
           EXECUTE FORMAT('ALTER USER MAPPING FOR %I SERVER %I OPTIONS (SET %I %L)', fdw_name, fdw_name, option.key, option.value);
         END IF;
-      END LOOP;
     END LOOP;
 
     -- Grant usage on the wrapper and server to the fdw role
@@ -234,7 +233,7 @@ BEGIN
     EXECUTE FORMAT ('GRANT %I TO %I WITH ADMIN OPTION', fdw_name, session_user);
 
     -- TODO: Bring here the remote cdb_tablemetadata
-
+END
 $$ LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE SECURITY DEFINER;
 
 CREATE OR REPLACE FUNCTION @extschema@._cdb_dbname_of_foreign_table(reloid oid)

From 10a4d85c017629e5cc72002ba17e7c0d7f6bc841 Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@carto.com>
Date: Fri, 12 Jul 2019 13:02:48 +0200
Subject: [PATCH 04/28] Fix typo in example: missing closing }

---
 scripts-available/CDB_ForeignTable.sql | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts-available/CDB_ForeignTable.sql b/scripts-available/CDB_ForeignTable.sql
index 3fbf862..543270e 100644
--- a/scripts-available/CDB_ForeignTable.sql
+++ b/scripts-available/CDB_ForeignTable.sql
@@ -154,7 +154,7 @@ LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE;
 --      "user": "fdw_user",
 --      "password": "secret"
 --    }
--- ');
+-- }');
 --
 -- Underneath it will:
 --   * Set up postgresql_fdw

From 12d955075a6fe37bd477a1009593d89e28baa70a Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@carto.com>
Date: Fri, 12 Jul 2019 13:05:52 +0200
Subject: [PATCH 05/28] Fix bug iterating user_mapping options

---
 scripts-available/CDB_ForeignTable.sql | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts-available/CDB_ForeignTable.sql b/scripts-available/CDB_ForeignTable.sql
index 543270e..b623036 100644
--- a/scripts-available/CDB_ForeignTable.sql
+++ b/scripts-available/CDB_ForeignTable.sql
@@ -209,7 +209,7 @@ BEGIN
     END IF;
 
     -- Update user mapping settings
-    FOR option IN SELECT o.key, o.value from lateral json_each_text('user_mapping') o LOOP
+    FOR option IN SELECT o.key, o.value from lateral json_each_text(config->'user_mapping') o LOOP
         IF NOT EXISTS (WITH a AS (select split_part(unnest(umoptions), '=', 1) as options from pg_user_mappings WHERE srvname = fdw_name AND usename = fdw_name) SELECT * from a where options = option.key) THEN
           EXECUTE FORMAT('ALTER USER MAPPING FOR %I SERVER %I OPTIONS (ADD %I %L)', fdw_name, fdw_name, option.key, option.value);
         ELSE

From b7907ff82f56136af3c32c2007885840d851a1bd Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@carto.com>
Date: Fri, 12 Jul 2019 13:06:37 +0200
Subject: [PATCH 06/28] Fix typo granting perms

---
 scripts-available/CDB_ForeignTable.sql | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts-available/CDB_ForeignTable.sql b/scripts-available/CDB_ForeignTable.sql
index b623036..66c4e3c 100644
--- a/scripts-available/CDB_ForeignTable.sql
+++ b/scripts-available/CDB_ForeignTable.sql
@@ -219,7 +219,7 @@ BEGIN
 
     -- Grant usage on the wrapper and server to the fdw role
     EXECUTE FORMAT ('GRANT USAGE ON FOREIGN DATA WRAPPER postgres_fdw TO %I', fdw_name);
-    EXECUTE FORMAT ('GRANT USAGE ON FOREIGN ON FOREIGN SERVER %I TO %I', fdw_name, fdw_name);
+    EXECUTE FORMAT ('GRANT USAGE ON FOREIGN SERVER %I TO %I', fdw_name, fdw_name);
 
     -- Create schema if it does not exist.
     IF NOT EXISTS ( SELECT * from pg_namespace WHERE nspname=fdw_name) THEN

From c58a0841020595fd94f9f7d4ff68fb3e9b03887d Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@carto.com>
Date: Fri, 12 Jul 2019 13:27:41 +0200
Subject: [PATCH 07/28] Tweak ownership of db objects

---
 scripts-available/CDB_ForeignTable.sql | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/scripts-available/CDB_ForeignTable.sql b/scripts-available/CDB_ForeignTable.sql
index 66c4e3c..29fb4bc 100644
--- a/scripts-available/CDB_ForeignTable.sql
+++ b/scripts-available/CDB_ForeignTable.sql
@@ -203,6 +203,9 @@ BEGIN
        EXECUTE format('CREATE ROLE %I NOLOGIN', fdw_name);
     END IF;
 
+    -- Transfer ownership of the server to the fdw role
+    EXECUTE format('ALTER SERVER %I OWNER TO %I', fdw_name, fdw_name);
+
     -- Create user mapping
     IF NOT EXISTS ( SELECT * FROM pg_user_mappings WHERE srvname = fdw_name AND usename = fdw_name ) THEN
         EXECUTE FORMAT ('CREATE USER MAPPING FOR %I SERVER %I', fdw_name, fdw_name);
@@ -226,8 +229,8 @@ BEGIN
       EXECUTE FORMAT ('CREATE SCHEMA %I', fdw_name);
     END IF;
 
-    -- Give the fdw role usage permisions over the schema
-    EXECUTE FORMAT ('GRANT USAGE ON SCHEMA %I TO %I', fdw_name, fdw_name);
+    -- Give the fdw role ownership over the schema
+    EXECUTE FORMAT ('ALTER SCHEMA %I OWNER TO %I', fdw_name, fdw_name);
 
     -- Grant the fdw role to the caller, and permissions to grant it to others
     EXECUTE FORMAT ('GRANT %I TO %I WITH ADMIN OPTION', fdw_name, session_user);

From 99e92e25056d9c6151eea068c03bdaf29fce7c62 Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@carto.com>
Date: Fri, 12 Jul 2019 16:23:34 +0200
Subject: [PATCH 08/28] Create a "PUBLIC" user mapping

---
 scripts-available/CDB_ForeignTable.sql | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/scripts-available/CDB_ForeignTable.sql b/scripts-available/CDB_ForeignTable.sql
index 29fb4bc..efa98eb 100644
--- a/scripts-available/CDB_ForeignTable.sql
+++ b/scripts-available/CDB_ForeignTable.sql
@@ -207,16 +207,18 @@ BEGIN
     EXECUTE format('ALTER SERVER %I OWNER TO %I', fdw_name, fdw_name);
 
     -- Create user mapping
-    IF NOT EXISTS ( SELECT * FROM pg_user_mappings WHERE srvname = fdw_name AND usename = fdw_name ) THEN
-        EXECUTE FORMAT ('CREATE USER MAPPING FOR %I SERVER %I', fdw_name, fdw_name);
+    -- NOTE: we use a PUBLIC user mapping but control access to the SERVER
+    -- so that we don't need to create a mapping for every user nor store credentials elsewhere
+    IF NOT EXISTS ( SELECT * FROM pg_user_mappings WHERE srvname = fdw_name AND usename = 'public' ) THEN
+        EXECUTE FORMAT ('CREATE USER MAPPING FOR public SERVER %I', fdw_name);
     END IF;
 
     -- Update user mapping settings
     FOR option IN SELECT o.key, o.value from lateral json_each_text(config->'user_mapping') o LOOP
-        IF NOT EXISTS (WITH a AS (select split_part(unnest(umoptions), '=', 1) as options from pg_user_mappings WHERE srvname = fdw_name AND usename = fdw_name) SELECT * from a where options = option.key) THEN
-          EXECUTE FORMAT('ALTER USER MAPPING FOR %I SERVER %I OPTIONS (ADD %I %L)', fdw_name, fdw_name, option.key, option.value);
+        IF NOT EXISTS (WITH a AS (select split_part(unnest(umoptions), '=', 1) as options from pg_user_mappings WHERE srvname = fdw_name AND usename = 'public') SELECT * from a where options = option.key) THEN
+          EXECUTE FORMAT('ALTER USER MAPPING FOR PUBLIC SERVER %I OPTIONS (ADD %I %L)', fdw_name, option.key, option.value);
         ELSE
-          EXECUTE FORMAT('ALTER USER MAPPING FOR %I SERVER %I OPTIONS (SET %I %L)', fdw_name, fdw_name, option.key, option.value);
+          EXECUTE FORMAT('ALTER USER MAPPING FOR PUBLIC SERVER %I OPTIONS (SET %I %L)', fdw_name, option.key, option.value);
         END IF;
     END LOOP;
 

From 34dec227c45945afee9f2ed0ee548f1b5791e2ba Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@carto.com>
Date: Fri, 12 Jul 2019 16:36:34 +0200
Subject: [PATCH 09/28] Rename to CDB_SetUp_User_Foreign_Server

---
 scripts-available/CDB_ForeignTable.sql | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts-available/CDB_ForeignTable.sql b/scripts-available/CDB_ForeignTable.sql
index efa98eb..7729ee6 100644
--- a/scripts-available/CDB_ForeignTable.sql
+++ b/scripts-available/CDB_ForeignTable.sql
@@ -143,7 +143,7 @@ LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE;
 -- It does not read from CDB_Conf
 --
 -- Sample call:
--- SELECT cartodb.CDB_SetUp_Foreign_Server('amazon', '{
+-- SELECT cartodb.CDB_SetUp_User_Foreign_Server('amazon', '{
 --    "server": {
 --      "extensions": "postgis",
 --      "dbname": "testdb",
@@ -169,7 +169,7 @@ LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE;
 --   * Specific roles: GRANT amazon TO role_name;
 --   * Members of the organization: SELECT cartodb.CDB_Grant_Role_To_Org_Members('amazon'); TODO
 --   * The publicuser: GRANT amazon TO publicuser;
-CREATE OR REPLACE FUNCTION @extschema@.CDB_SetUp_Foreign_Server(fdw_name NAME, config json)
+CREATE OR REPLACE FUNCTION @extschema@.CDB_SetUp_User_Foreign_Server(fdw_name NAME, config json)
 RETURNS void AS $$
 DECLARE
   row record;

From d2d909145d40f61e68bd751b7765026c1fef627b Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@carto.com>
Date: Fri, 12 Jul 2019 16:47:18 +0200
Subject: [PATCH 10/28] Add convenience function to import fdw tables

---
 scripts-available/CDB_ForeignTable.sql | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/scripts-available/CDB_ForeignTable.sql b/scripts-available/CDB_ForeignTable.sql
index 7729ee6..498daf5 100644
--- a/scripts-available/CDB_ForeignTable.sql
+++ b/scripts-available/CDB_ForeignTable.sql
@@ -241,6 +241,17 @@ BEGIN
 END
 $$ LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE SECURITY DEFINER;
 
+
+CREATE OR REPLACE FUNCTION @extschema@.CDB_SetUp_User_Foreign_Table(fdw_name NAME, table_name NAME)
+RETURNS void AS $$
+BEGIN
+  EXECUTE FORMAT ('IMPORT FOREIGN SCHEMA carto_lite LIMIT TO (%I) FROM SERVER %I INTO %I;', table_name, fdw_name, fdw_name);
+  --- Grant SELECT to fdw role
+  EXECUTE FORMAT ('GRANT SELECT ON %I.%I TO %I;', fdw_name, table_name, fdw_name);
+END
+$$ LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE;
+
+
 CREATE OR REPLACE FUNCTION @extschema@._cdb_dbname_of_foreign_table(reloid oid)
 RETURNS TEXT AS $$
     SELECT option_value FROM pg_options_to_table((

From 70220e04c18f6c15927ded175c457bda940bf192 Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@carto.com>
Date: Mon, 15 Jul 2019 13:13:21 +0200
Subject: [PATCH 11/28] Allow for imports of tables in different source schemas

---
 scripts-available/CDB_ForeignTable.sql | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/scripts-available/CDB_ForeignTable.sql b/scripts-available/CDB_ForeignTable.sql
index 498daf5..e4cfc40 100644
--- a/scripts-available/CDB_ForeignTable.sql
+++ b/scripts-available/CDB_ForeignTable.sql
@@ -242,10 +242,14 @@ END
 $$ LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE SECURITY DEFINER;
 
 
-CREATE OR REPLACE FUNCTION @extschema@.CDB_SetUp_User_Foreign_Table(fdw_name NAME, table_name NAME)
+-- Set up a user foreign table
+-- E.g:
+--   SELECT cartodb.CDB_SetUp_User_Foreign_Table('amazon', 'carto_lite', 'mytable');
+--   SELECT * FROM amazon.my_table;
+CREATE OR REPLACE FUNCTION @extschema@.CDB_SetUp_User_Foreign_Table(fdw_name NAME, foreign_schema NAME, table_name NAME)
 RETURNS void AS $$
 BEGIN
-  EXECUTE FORMAT ('IMPORT FOREIGN SCHEMA carto_lite LIMIT TO (%I) FROM SERVER %I INTO %I;', table_name, fdw_name, fdw_name);
+  EXECUTE FORMAT ('IMPORT FOREIGN SCHEMA %I LIMIT TO (%I) FROM SERVER %I INTO %I;', foreign_schema, table_name, fdw_name, fdw_name);
   --- Grant SELECT to fdw role
   EXECUTE FORMAT ('GRANT SELECT ON %I.%I TO %I;', fdw_name, table_name, fdw_name);
 END

From 6e34e16b8dbca71a51bc2952474e719bf97f99fd Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@carto.com>
Date: Mon, 15 Jul 2019 13:16:14 +0200
Subject: [PATCH 12/28] Add basic test for user-defined FDW's

---
 test/extension/test.sh | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/test/extension/test.sh b/test/extension/test.sh
index 1d9e7ee..7f879f3 100755
--- a/test/extension/test.sh
+++ b/test/extension/test.sh
@@ -590,6 +590,33 @@ test_extension|public|"local-table-with-dashes"'
     sql postgres "DROP FOREIGN TABLE IF EXISTS test_fdw.cdb_tablemetadata;"
     sql postgres "SELECT cartodb.CDB_Get_Foreign_Updated_At('test_fdw.foo') IS NULL" should 't'
 
+
+    # Check user-defined FDW's
+    # Set up a user foreign server
+    read -d '' ufdw_config <<- EOF
+{
+   "server": {
+     "extensions": "postgis",
+     "dbname": "fdw_target",
+     "host": "localhost",
+     "port": ${PGPORT:-5432}
+   },
+   "user_mapping": {
+     "user": "fdw_user",
+     "password": "foobarino"
+   }
+}
+EOF
+    sql postgres "SELECT cartodb.CDB_SetUp_User_Foreign_Server('test_user_fdw', '$ufdw_config');"
+
+    # Set up a user foreign table
+    sql postgres "SELECT cartodb.CDB_SetUp_User_Foreign_Table('test_user_fdw', 'test_fdw', 'foo');"
+
+    # Check that the table can be accessed
+    sql postgres "SELECT * from test_user_fdw.foo;"
+    sql postgres "SELECT a from test_user_fdw.foo LIMIT 1;" should 42
+
+
     # Teardown
     DATABASE=fdw_target sql postgres 'REVOKE USAGE ON SCHEMA test_fdw FROM fdw_user;'
     DATABASE=fdw_target sql postgres 'REVOKE SELECT ON test_fdw.foo FROM fdw_user;'

From 8cfc8e65cfe9be1c210a113f27bc334be0eb9fb9 Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@carto.com>
Date: Mon, 15 Jul 2019 14:44:21 +0200
Subject: [PATCH 13/28] Test with a regular user (non-superadmin)

---
 test/extension/test.sh | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/test/extension/test.sh b/test/extension/test.sh
index 7f879f3..58b2ead 100755
--- a/test/extension/test.sh
+++ b/test/extension/test.sh
@@ -607,14 +607,14 @@ test_extension|public|"local-table-with-dashes"'
    }
 }
 EOF
-    sql postgres "SELECT cartodb.CDB_SetUp_User_Foreign_Server('test_user_fdw', '$ufdw_config');"
+    sql cdb_testmember_1 "SELECT cartodb.CDB_SetUp_User_Foreign_Server('test_user_fdw', '$ufdw_config');"
 
     # Set up a user foreign table
-    sql postgres "SELECT cartodb.CDB_SetUp_User_Foreign_Table('test_user_fdw', 'test_fdw', 'foo');"
+    sql cdb_testmember_1 "SELECT cartodb.CDB_SetUp_User_Foreign_Table('test_user_fdw', 'test_fdw', 'foo');"
 
     # Check that the table can be accessed
-    sql postgres "SELECT * from test_user_fdw.foo;"
-    sql postgres "SELECT a from test_user_fdw.foo LIMIT 1;" should 42
+    sql cdb_testmember_1 "SELECT * from test_user_fdw.foo;"
+    sql cdb_testmember_1 "SELECT a from test_user_fdw.foo LIMIT 1;" should 42
 
 
     # Teardown
@@ -624,6 +624,12 @@ EOF
     DATABASE=fdw_target sql postgres 'REVOKE SELECT ON cdb_tablemetadata_text FROM fdw_user;'
     DATABASE=fdw_target sql postgres 'DROP ROLE fdw_user;'
 
+    # TODO add to function to delete stuff
+    sql postgres 'DROP FOREIGN TABLE test_user_fdw.foo;'
+    sql postgres 'DROP schema test_user_fdw;'
+    sql postgres 'DROP USER MAPPING FOR public SERVER test_user_fdw;'
+    sql postgres 'DROP SERVER test_user_fdw;'
+
     sql postgres "select pg_terminate_backend(pid) from pg_stat_activity where datname='fdw_target';"
     DATABASE=fdw_target tear_down_database
 }

From 1189d70b2a1fa049f8e95ac397ee732a20c11239 Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@carto.com>
Date: Mon, 15 Jul 2019 14:52:39 +0200
Subject: [PATCH 14/28] Request host auth to use password

This is required for non superusers to use FDW's. See
https://www.postgresql.org/docs/11/postgres-fdw.html#id-1.11.7.42.10
---
 .travis.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.travis.yml b/.travis.yml
index 1132fcc..f448081 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -24,7 +24,7 @@ before_install:
   - sudo apt-get install -y --allow-unauthenticated postgresql-$POSTGRESQL_VERSION-postgis-$POSTGIS_VERSION postgresql-$POSTGRESQL_VERSION-postgis-$POSTGIS_VERSION-scripts postgis postgresql-plpython-$POSTGRESQL_VERSION
   - sudo pg_dropcluster --stop $POSTGRESQL_VERSION main
   - sudo rm -rf /etc/postgresql/$POSTGRESQL_VERSION /var/lib/postgresql/$POSTGRESQL_VERSION
-  - sudo pg_createcluster -u postgres $POSTGRESQL_VERSION main -- -A trust
+  - sudo pg_createcluster -u postgres $POSTGRESQL_VERSION main -- --auth-local trust --auth-host password
   - sudo /etc/init.d/postgresql start $POSTGRESQL_VERSION || sudo journalctl -xe
   - sudo pip install redis==2.4.9
 script:

From 37004db0476c621f57cb7747a89c73bc4c1cb3fd Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@carto.com>
Date: Mon, 15 Jul 2019 16:14:07 +0200
Subject: [PATCH 15/28] Add new function to drop a user-defined foreign server

---
 scripts-available/CDB_ForeignTable.sql | 9 +++++++++
 test/extension/test.sh                 | 7 ++-----
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/scripts-available/CDB_ForeignTable.sql b/scripts-available/CDB_ForeignTable.sql
index e4cfc40..7e184f7 100644
--- a/scripts-available/CDB_ForeignTable.sql
+++ b/scripts-available/CDB_ForeignTable.sql
@@ -241,6 +241,15 @@ BEGIN
 END
 $$ LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE SECURITY DEFINER;
 
+CREATE OR REPLACE FUNCTION @extschema@.CDB_Drop_User_Foreign_Server(fdw_name NAME)
+RETURNS void AS $$
+BEGIN
+    EXECUTE FORMAT ('DROP SCHEMA %I', fdw_name);
+    EXECUTE FORMAT ('DROP USER MAPPING FOR public SERVER %I', fdw_name);
+    EXECUTE FORMAT ('DROP SERVER %I', fdw_name);
+END
+$$ LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE;
+
 
 -- Set up a user foreign table
 -- E.g:
diff --git a/test/extension/test.sh b/test/extension/test.sh
index 58b2ead..3c8b334 100755
--- a/test/extension/test.sh
+++ b/test/extension/test.sh
@@ -624,11 +624,8 @@ EOF
     DATABASE=fdw_target sql postgres 'REVOKE SELECT ON cdb_tablemetadata_text FROM fdw_user;'
     DATABASE=fdw_target sql postgres 'DROP ROLE fdw_user;'
 
-    # TODO add to function to delete stuff
-    sql postgres 'DROP FOREIGN TABLE test_user_fdw.foo;'
-    sql postgres 'DROP schema test_user_fdw;'
-    sql postgres 'DROP USER MAPPING FOR public SERVER test_user_fdw;'
-    sql postgres 'DROP SERVER test_user_fdw;'
+    sql cdb_testmember_1 "DROP FOREIGN TABLE test_user_fdw.foo;"
+    sql cdb_testmember_1 "SELECT cartodb.CDB_Drop_User_Foreign_Server('test_user_fdw')"
 
     sql postgres "select pg_terminate_backend(pid) from pg_stat_activity where datname='fdw_target';"
     DATABASE=fdw_target tear_down_database

From a20676f391d3e3301cbd386d176890c23a735fc4 Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@carto.com>
Date: Mon, 15 Jul 2019 16:19:06 +0200
Subject: [PATCH 16/28] Add a test/example of granting the fdw role

---
 test/extension/test.sh | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/test/extension/test.sh b/test/extension/test.sh
index 3c8b334..f89bb68 100755
--- a/test/extension/test.sh
+++ b/test/extension/test.sh
@@ -612,10 +612,15 @@ EOF
     # Set up a user foreign table
     sql cdb_testmember_1 "SELECT cartodb.CDB_SetUp_User_Foreign_Table('test_user_fdw', 'test_fdw', 'foo');"
 
-    # Check that the table can be accessed
+    # Check that the table can be accessed by the owner/creator
     sql cdb_testmember_1 "SELECT * from test_user_fdw.foo;"
     sql cdb_testmember_1 "SELECT a from test_user_fdw.foo LIMIT 1;" should 42
 
+    # Check that the table can be accessed by some other user by granting the role
+    sql cdb_testmember_1 "GRANT test_user_fdw TO cdb_testmember_2;"
+    sql cdb_testmember_2 "SELECT a from test_user_fdw.foo LIMIT 1;" should 42
+    sql cdb_testmember_1 "REVOKE test_user_fdw FROM cdb_testmember_2;"
+
 
     # Teardown
     DATABASE=fdw_target sql postgres 'REVOKE USAGE ON SCHEMA test_fdw FROM fdw_user;'

From 3a10ef7e764f175a7a72703d70c316927292db38 Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@carto.com>
Date: Mon, 15 Jul 2019 16:54:23 +0200
Subject: [PATCH 17/28] Add ability to grant fdw role to org members

---
 scripts-available/CDB_ForeignTable.sql  |  2 +-
 scripts-available/CDB_Organizations.sql | 27 +++++++++++++++++++++++++
 test/extension/test.sh                  |  5 +++++
 3 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/scripts-available/CDB_ForeignTable.sql b/scripts-available/CDB_ForeignTable.sql
index 7e184f7..a284efc 100644
--- a/scripts-available/CDB_ForeignTable.sql
+++ b/scripts-available/CDB_ForeignTable.sql
@@ -167,7 +167,7 @@ LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE;
 -- It is the responsibility of the caller to grant that role to either:
 --   * Nobody
 --   * Specific roles: GRANT amazon TO role_name;
---   * Members of the organization: SELECT cartodb.CDB_Grant_Role_To_Org_Members('amazon'); TODO
+--   * Members of the organization: SELECT cartodb.CDB_Organization_Grant_Role('amazon');
 --   * The publicuser: GRANT amazon TO publicuser;
 CREATE OR REPLACE FUNCTION @extschema@.CDB_SetUp_User_Foreign_Server(fdw_name NAME, config json)
 RETURNS void AS $$
diff --git a/scripts-available/CDB_Organizations.sql b/scripts-available/CDB_Organizations.sql
index c97208e..c532ed5 100644
--- a/scripts-available/CDB_Organizations.sql
+++ b/scripts-available/CDB_Organizations.sql
@@ -169,3 +169,30 @@ BEGIN
     EXECUTE 'SELECT @extschema@.CDB_Organization_Remove_Access_Permission(''' || from_schema || ''', ''' || table_name || ''', ''' || @extschema@.CDB_Organization_Member_Group_Role_Member_Name() || ''');';
 END
 $$ LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE;
+
+
+--------------------------------------------------------------------------------
+-- Role management
+--------------------------------------------------------------------------------
+CREATE OR REPLACE
+FUNCTION @extschema@.CDB_Organization_Grant_Role(role_name name)
+RETURNS VOID AS $$
+DECLARE
+    org_role TEXT;
+BEGIN
+    org_role := @extschema@.CDB_Organization_Member_Group_Role_Member_Name();
+    EXECUTE format('GRANT %I TO %I', role_name, org_role);
+END
+$$ LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE;
+
+
+CREATE OR REPLACE
+FUNCTION @extschema@.CDB_Organization_Revoke_Role(role_name name)
+RETURNS VOID AS $$
+DECLARE
+    org_role TEXT;
+BEGIN
+    org_role := @extschema@.CDB_Organization_Member_Group_Role_Member_Name();
+    EXECUTE format('REVOKE %I FROM %I', role_name, org_role);
+END
+$$ LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE;
diff --git a/test/extension/test.sh b/test/extension/test.sh
index f89bb68..d93d89b 100755
--- a/test/extension/test.sh
+++ b/test/extension/test.sh
@@ -621,6 +621,11 @@ EOF
     sql cdb_testmember_2 "SELECT a from test_user_fdw.foo LIMIT 1;" should 42
     sql cdb_testmember_1 "REVOKE test_user_fdw FROM cdb_testmember_2;"
 
+    # Check that the table can be accessed by org members
+    sql cdb_testmember_1 "SELECT cartodb.CDB_Organization_Grant_Role('test_user_fdw');"
+    sql cdb_testmember_2 "SELECT a from test_user_fdw.foo LIMIT 1;" should 42
+    sql cdb_testmember_1 "SELECT cartodb.CDB_Organization_Revoke_Role('test_user_fdw');"
+
 
     # Teardown
     DATABASE=fdw_target sql postgres 'REVOKE USAGE ON SCHEMA test_fdw FROM fdw_user;'

From 99096d41e0bbd65f77f798fade32e0353a235365 Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@carto.com>
Date: Mon, 15 Jul 2019 17:25:48 +0200
Subject: [PATCH 18/28] Drop the role when dropping a user-defined FDW

---
 scripts-available/CDB_ForeignTable.sql | 12 +++++++++++-
 test/extension/test.sh                 |  3 +++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/scripts-available/CDB_ForeignTable.sql b/scripts-available/CDB_ForeignTable.sql
index a284efc..d48465b 100644
--- a/scripts-available/CDB_ForeignTable.sql
+++ b/scripts-available/CDB_ForeignTable.sql
@@ -241,14 +241,24 @@ BEGIN
 END
 $$ LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE SECURITY DEFINER;
 
+
+-- A function to drop a user-defined foreign server and all related objects
+-- It does not read from CDB_Conf
+--
+-- Sample call:
+--   SELECT cartodb.CDB_Drop_User_Foreign_Server('amazon')
+--
+-- Note: if there's any dependent object (i.e. foreign table) this call will fail
 CREATE OR REPLACE FUNCTION @extschema@.CDB_Drop_User_Foreign_Server(fdw_name NAME)
 RETURNS void AS $$
 BEGIN
     EXECUTE FORMAT ('DROP SCHEMA %I', fdw_name);
     EXECUTE FORMAT ('DROP USER MAPPING FOR public SERVER %I', fdw_name);
     EXECUTE FORMAT ('DROP SERVER %I', fdw_name);
+    EXECUTE FORMAT ('REVOKE USAGE ON FOREIGN DATA WRAPPER postgres_fdw FROM %I', fdw_name);
+    EXECUTE FORMAT ('DROP ROLE %I', fdw_name);
 END
-$$ LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE;
+$$ LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE SECURITY DEFINER;
 
 
 -- Set up a user foreign table
diff --git a/test/extension/test.sh b/test/extension/test.sh
index d93d89b..c4af8b7 100755
--- a/test/extension/test.sh
+++ b/test/extension/test.sh
@@ -626,6 +626,9 @@ EOF
     sql cdb_testmember_2 "SELECT a from test_user_fdw.foo LIMIT 1;" should 42
     sql cdb_testmember_1 "SELECT cartodb.CDB_Organization_Revoke_Role('test_user_fdw');"
 
+    # If there are dependent objects, we cannot drop the foreign server
+    sql cdb_testmember_1 "SELECT cartodb.CDB_Drop_User_Foreign_Server('test_user_fdw')" fails
+
 
     # Teardown
     DATABASE=fdw_target sql postgres 'REVOKE USAGE ON SCHEMA test_fdw FROM fdw_user;'

From 2e9f642378b88efd62e33749d4dca24fe9d8b87e Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@carto.com>
Date: Mon, 15 Jul 2019 17:28:37 +0200
Subject: [PATCH 19/28] Check when users shall not have permissions to the FDW

---
 test/extension/test.sh | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/test/extension/test.sh b/test/extension/test.sh
index c4af8b7..4b59853 100755
--- a/test/extension/test.sh
+++ b/test/extension/test.sh
@@ -616,12 +616,17 @@ EOF
     sql cdb_testmember_1 "SELECT * from test_user_fdw.foo;"
     sql cdb_testmember_1 "SELECT a from test_user_fdw.foo LIMIT 1;" should 42
 
+    # Check that a role with no permissions cannot use the FDW to access a remote table
+    sql cdb_testmember_2 "IMPORT FOREIGN SCHEMA test_fdw LIMIT TO (foo) FROM SERVER test_user_fdw INTO public" fails
+
     # Check that the table can be accessed by some other user by granting the role
+    sql cdb_testmember_2 "SELECT a from test_user_fdw.foo LIMIT 1;" fails
     sql cdb_testmember_1 "GRANT test_user_fdw TO cdb_testmember_2;"
     sql cdb_testmember_2 "SELECT a from test_user_fdw.foo LIMIT 1;" should 42
     sql cdb_testmember_1 "REVOKE test_user_fdw FROM cdb_testmember_2;"
 
     # Check that the table can be accessed by org members
+    sql cdb_testmember_2 "SELECT a from test_user_fdw.foo LIMIT 1;" fails
     sql cdb_testmember_1 "SELECT cartodb.CDB_Organization_Grant_Role('test_user_fdw');"
     sql cdb_testmember_2 "SELECT a from test_user_fdw.foo LIMIT 1;" should 42
     sql cdb_testmember_1 "SELECT cartodb.CDB_Organization_Revoke_Role('test_user_fdw');"

From c4e2549dc891e296b278b04121a40dc05754eee8 Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@carto.com>
Date: Mon, 15 Jul 2019 18:14:23 +0200
Subject: [PATCH 20/28] A few more permissions tests for completeness

---
 test/extension/test.sh | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/test/extension/test.sh b/test/extension/test.sh
index 4b59853..ffe2ba3 100755
--- a/test/extension/test.sh
+++ b/test/extension/test.sh
@@ -631,8 +631,16 @@ EOF
     sql cdb_testmember_2 "SELECT a from test_user_fdw.foo LIMIT 1;" should 42
     sql cdb_testmember_1 "SELECT cartodb.CDB_Organization_Revoke_Role('test_user_fdw');"
 
+    # By default publicuser cannot access the FDW
+    sql publicuser "SELECT a from test_user_fdw.foo LIMIT 1;" fails
+    sql cdb_testmember_1 "GRANT test_user_fdw TO publicuser;" # but can be granted
+    sql publicuser "SELECT a from test_user_fdw.foo LIMIT 1;" should 42
+    sql cdb_testmember_1 "REVOKE test_user_fdw FROM publicuser;"
+
     # If there are dependent objects, we cannot drop the foreign server
     sql cdb_testmember_1 "SELECT cartodb.CDB_Drop_User_Foreign_Server('test_user_fdw')" fails
+    sql cdb_testmember_1 "DROP FOREIGN TABLE test_user_fdw.foo;"
+    sql cdb_testmember_1 "SELECT cartodb.CDB_Drop_User_Foreign_Server('test_user_fdw')"
 
 
     # Teardown
@@ -642,9 +650,6 @@ EOF
     DATABASE=fdw_target sql postgres 'REVOKE SELECT ON cdb_tablemetadata_text FROM fdw_user;'
     DATABASE=fdw_target sql postgres 'DROP ROLE fdw_user;'
 
-    sql cdb_testmember_1 "DROP FOREIGN TABLE test_user_fdw.foo;"
-    sql cdb_testmember_1 "SELECT cartodb.CDB_Drop_User_Foreign_Server('test_user_fdw')"
-
     sql postgres "select pg_terminate_backend(pid) from pg_stat_activity where datname='fdw_target';"
     DATABASE=fdw_target tear_down_database
 }

From 3a255df9d0081da3068f872ae23466d456231162 Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@carto.com>
Date: Tue, 16 Jul 2019 13:14:11 +0200
Subject: [PATCH 21/28] Rename PG-FDW's-specific functions to _PG_FDW_

As per review comment.
---
 scripts-available/CDB_ForeignTable.sql | 12 ++++++------
 test/extension/test.sh                 |  8 ++++----
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/scripts-available/CDB_ForeignTable.sql b/scripts-available/CDB_ForeignTable.sql
index d48465b..ae86b87 100644
--- a/scripts-available/CDB_ForeignTable.sql
+++ b/scripts-available/CDB_ForeignTable.sql
@@ -143,7 +143,7 @@ LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE;
 -- It does not read from CDB_Conf
 --
 -- Sample call:
--- SELECT cartodb.CDB_SetUp_User_Foreign_Server('amazon', '{
+-- SELECT cartodb.CDB_SetUp_User_PG_FDW_Server('amazon', '{
 --    "server": {
 --      "extensions": "postgis",
 --      "dbname": "testdb",
@@ -169,7 +169,7 @@ LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE;
 --   * Specific roles: GRANT amazon TO role_name;
 --   * Members of the organization: SELECT cartodb.CDB_Organization_Grant_Role('amazon');
 --   * The publicuser: GRANT amazon TO publicuser;
-CREATE OR REPLACE FUNCTION @extschema@.CDB_SetUp_User_Foreign_Server(fdw_name NAME, config json)
+CREATE OR REPLACE FUNCTION @extschema@.CDB_SetUp_User_PG_FDW_Server(fdw_name NAME, config json)
 RETURNS void AS $$
 DECLARE
   row record;
@@ -246,10 +246,10 @@ $$ LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE SECURITY DEFINER;
 -- It does not read from CDB_Conf
 --
 -- Sample call:
---   SELECT cartodb.CDB_Drop_User_Foreign_Server('amazon')
+--   SELECT cartodb.CDB_Drop_User_PG_FDW_Server('amazon')
 --
 -- Note: if there's any dependent object (i.e. foreign table) this call will fail
-CREATE OR REPLACE FUNCTION @extschema@.CDB_Drop_User_Foreign_Server(fdw_name NAME)
+CREATE OR REPLACE FUNCTION @extschema@.CDB_Drop_User_PG_FDW_Server(fdw_name NAME)
 RETURNS void AS $$
 BEGIN
     EXECUTE FORMAT ('DROP SCHEMA %I', fdw_name);
@@ -263,9 +263,9 @@ $$ LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE SECURITY DEFINER;
 
 -- Set up a user foreign table
 -- E.g:
---   SELECT cartodb.CDB_SetUp_User_Foreign_Table('amazon', 'carto_lite', 'mytable');
+--   SELECT cartodb.CDB_SetUp_User_PG_FDW_Table('amazon', 'carto_lite', 'mytable');
 --   SELECT * FROM amazon.my_table;
-CREATE OR REPLACE FUNCTION @extschema@.CDB_SetUp_User_Foreign_Table(fdw_name NAME, foreign_schema NAME, table_name NAME)
+CREATE OR REPLACE FUNCTION @extschema@.CDB_SetUp_User_PG_FDW_Table(fdw_name NAME, foreign_schema NAME, table_name NAME)
 RETURNS void AS $$
 BEGIN
   EXECUTE FORMAT ('IMPORT FOREIGN SCHEMA %I LIMIT TO (%I) FROM SERVER %I INTO %I;', foreign_schema, table_name, fdw_name, fdw_name);
diff --git a/test/extension/test.sh b/test/extension/test.sh
index ffe2ba3..4d3ea2d 100755
--- a/test/extension/test.sh
+++ b/test/extension/test.sh
@@ -607,10 +607,10 @@ test_extension|public|"local-table-with-dashes"'
    }
 }
 EOF
-    sql cdb_testmember_1 "SELECT cartodb.CDB_SetUp_User_Foreign_Server('test_user_fdw', '$ufdw_config');"
+    sql cdb_testmember_1 "SELECT cartodb.CDB_SetUp_User_PG_FDW_Server('test_user_fdw', '$ufdw_config');"
 
     # Set up a user foreign table
-    sql cdb_testmember_1 "SELECT cartodb.CDB_SetUp_User_Foreign_Table('test_user_fdw', 'test_fdw', 'foo');"
+    sql cdb_testmember_1 "SELECT cartodb.CDB_SetUp_User_PG_FDW_Table('test_user_fdw', 'test_fdw', 'foo');"
 
     # Check that the table can be accessed by the owner/creator
     sql cdb_testmember_1 "SELECT * from test_user_fdw.foo;"
@@ -638,9 +638,9 @@ EOF
     sql cdb_testmember_1 "REVOKE test_user_fdw FROM publicuser;"
 
     # If there are dependent objects, we cannot drop the foreign server
-    sql cdb_testmember_1 "SELECT cartodb.CDB_Drop_User_Foreign_Server('test_user_fdw')" fails
+    sql cdb_testmember_1 "SELECT cartodb.CDB_Drop_User_PG_FDW_Server('test_user_fdw')" fails
     sql cdb_testmember_1 "DROP FOREIGN TABLE test_user_fdw.foo;"
-    sql cdb_testmember_1 "SELECT cartodb.CDB_Drop_User_Foreign_Server('test_user_fdw')"
+    sql cdb_testmember_1 "SELECT cartodb.CDB_Drop_User_PG_FDW_Server('test_user_fdw')"
 
 
     # Teardown

From a32dea0282a5420831f72a3491858fb7f5d62a05 Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@carto.com>
Date: Tue, 16 Jul 2019 13:26:03 +0200
Subject: [PATCH 22/28] Remove SECURITY DEFINER from user-defined FDW's

---
 scripts-available/CDB_ForeignTable.sql | 13 ++++++-------
 test/extension/test.sh                 |  9 ++++++---
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/scripts-available/CDB_ForeignTable.sql b/scripts-available/CDB_ForeignTable.sql
index ae86b87..17a018c 100644
--- a/scripts-available/CDB_ForeignTable.sql
+++ b/scripts-available/CDB_ForeignTable.sql
@@ -140,7 +140,8 @@ LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE;
 
 
 -- A function to set up a user-defined foreign data server
--- It does not read from CDB_Conf
+-- It does not read from CDB_Conf.
+-- Only superuser roles can invoke it successfully
 --
 -- Sample call:
 -- SELECT cartodb.CDB_SetUp_User_PG_FDW_Server('amazon', '{
@@ -164,7 +165,7 @@ LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE;
 --   * Create a schema 'amazon' as a convenience to set up all foreign
 --     tables over there
 --
--- It is the responsibility of the caller to grant that role to either:
+-- It is the responsibility of the superuser to grant that role to either:
 --   * Nobody
 --   * Specific roles: GRANT amazon TO role_name;
 --   * Members of the organization: SELECT cartodb.CDB_Organization_Grant_Role('amazon');
@@ -234,16 +235,14 @@ BEGIN
     -- Give the fdw role ownership over the schema
     EXECUTE FORMAT ('ALTER SCHEMA %I OWNER TO %I', fdw_name, fdw_name);
 
-    -- Grant the fdw role to the caller, and permissions to grant it to others
-    EXECUTE FORMAT ('GRANT %I TO %I WITH ADMIN OPTION', fdw_name, session_user);
-
     -- TODO: Bring here the remote cdb_tablemetadata
 END
-$$ LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE SECURITY DEFINER;
+$$ LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE;
 
 
 -- A function to drop a user-defined foreign server and all related objects
 -- It does not read from CDB_Conf
+-- It must be executed with a superuser role to succeed
 --
 -- Sample call:
 --   SELECT cartodb.CDB_Drop_User_PG_FDW_Server('amazon')
@@ -258,7 +257,7 @@ BEGIN
     EXECUTE FORMAT ('REVOKE USAGE ON FOREIGN DATA WRAPPER postgres_fdw FROM %I', fdw_name);
     EXECUTE FORMAT ('DROP ROLE %I', fdw_name);
 END
-$$ LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE SECURITY DEFINER;
+$$ LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE;
 
 
 -- Set up a user foreign table
diff --git a/test/extension/test.sh b/test/extension/test.sh
index 4d3ea2d..3e4b471 100755
--- a/test/extension/test.sh
+++ b/test/extension/test.sh
@@ -607,7 +607,10 @@ test_extension|public|"local-table-with-dashes"'
    }
 }
 EOF
-    sql cdb_testmember_1 "SELECT cartodb.CDB_SetUp_User_PG_FDW_Server('test_user_fdw', '$ufdw_config');"
+    sql postgres "SELECT cartodb.CDB_SetUp_User_PG_FDW_Server('test_user_fdw', '$ufdw_config');"
+
+    # Grant a user access to that FDW, and to grant to others
+    sql postgres "GRANT test_user_fdw TO cdb_testmember_1 WITH ADMIN OPTION;"
 
     # Set up a user foreign table
     sql cdb_testmember_1 "SELECT cartodb.CDB_SetUp_User_PG_FDW_Table('test_user_fdw', 'test_fdw', 'foo');"
@@ -638,9 +641,9 @@ EOF
     sql cdb_testmember_1 "REVOKE test_user_fdw FROM publicuser;"
 
     # If there are dependent objects, we cannot drop the foreign server
-    sql cdb_testmember_1 "SELECT cartodb.CDB_Drop_User_PG_FDW_Server('test_user_fdw')" fails
+    sql postgres "SELECT cartodb.CDB_Drop_User_PG_FDW_Server('test_user_fdw')" fails
     sql cdb_testmember_1 "DROP FOREIGN TABLE test_user_fdw.foo;"
-    sql cdb_testmember_1 "SELECT cartodb.CDB_Drop_User_PG_FDW_Server('test_user_fdw')"
+    sql postgres "SELECT cartodb.CDB_Drop_User_PG_FDW_Server('test_user_fdw')"
 
 
     # Teardown

From 0f33ee8b22a2a5fe122f480bde43a25b590e1680 Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@carto.com>
Date: Tue, 16 Jul 2019 14:32:32 +0200
Subject: [PATCH 23/28] Prepend an underscore (_) to functions meant to be run
 by superuser

_CDB_SetUp_User_PG_FDW_Server and _CDB_Drop_User_PG_FDW_Server are
meant to be executed by a superuser. Therefore they shouldn't be
considered part of the public API and hence the _CDB_Private_Function
naming convention.
---
 scripts-available/CDB_ForeignTable.sql | 4 ++--
 test/extension/test.sh                 | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/scripts-available/CDB_ForeignTable.sql b/scripts-available/CDB_ForeignTable.sql
index 17a018c..ed36449 100644
--- a/scripts-available/CDB_ForeignTable.sql
+++ b/scripts-available/CDB_ForeignTable.sql
@@ -170,7 +170,7 @@ LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE;
 --   * Specific roles: GRANT amazon TO role_name;
 --   * Members of the organization: SELECT cartodb.CDB_Organization_Grant_Role('amazon');
 --   * The publicuser: GRANT amazon TO publicuser;
-CREATE OR REPLACE FUNCTION @extschema@.CDB_SetUp_User_PG_FDW_Server(fdw_name NAME, config json)
+CREATE OR REPLACE FUNCTION @extschema@._CDB_SetUp_User_PG_FDW_Server(fdw_name NAME, config json)
 RETURNS void AS $$
 DECLARE
   row record;
@@ -248,7 +248,7 @@ $$ LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE;
 --   SELECT cartodb.CDB_Drop_User_PG_FDW_Server('amazon')
 --
 -- Note: if there's any dependent object (i.e. foreign table) this call will fail
-CREATE OR REPLACE FUNCTION @extschema@.CDB_Drop_User_PG_FDW_Server(fdw_name NAME)
+CREATE OR REPLACE FUNCTION @extschema@._CDB_Drop_User_PG_FDW_Server(fdw_name NAME)
 RETURNS void AS $$
 BEGIN
     EXECUTE FORMAT ('DROP SCHEMA %I', fdw_name);
diff --git a/test/extension/test.sh b/test/extension/test.sh
index 3e4b471..b520343 100755
--- a/test/extension/test.sh
+++ b/test/extension/test.sh
@@ -607,7 +607,7 @@ test_extension|public|"local-table-with-dashes"'
    }
 }
 EOF
-    sql postgres "SELECT cartodb.CDB_SetUp_User_PG_FDW_Server('test_user_fdw', '$ufdw_config');"
+    sql postgres "SELECT cartodb._CDB_SetUp_User_PG_FDW_Server('test_user_fdw', '$ufdw_config');"
 
     # Grant a user access to that FDW, and to grant to others
     sql postgres "GRANT test_user_fdw TO cdb_testmember_1 WITH ADMIN OPTION;"
@@ -641,9 +641,9 @@ EOF
     sql cdb_testmember_1 "REVOKE test_user_fdw FROM publicuser;"
 
     # If there are dependent objects, we cannot drop the foreign server
-    sql postgres "SELECT cartodb.CDB_Drop_User_PG_FDW_Server('test_user_fdw')" fails
+    sql postgres "SELECT cartodb._CDB_Drop_User_PG_FDW_Server('test_user_fdw')" fails
     sql cdb_testmember_1 "DROP FOREIGN TABLE test_user_fdw.foo;"
-    sql postgres "SELECT cartodb.CDB_Drop_User_PG_FDW_Server('test_user_fdw')"
+    sql postgres "SELECT cartodb._CDB_Drop_User_PG_FDW_Server('test_user_fdw')"
 
 
     # Teardown

From ce1e9ac41c61404174229fa34ff195b941d32c84 Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@carto.com>
Date: Tue, 16 Jul 2019 14:51:17 +0200
Subject: [PATCH 24/28] Prefix all objects created with cdb_fdw_

Build the DB objects related to a user FDW with the following form:
`cdb_fdw_name`. This is aimed at easily inspect and filter them.

As requested in code review.
---
 scripts-available/CDB_ForeignTable.sql | 82 +++++++++++++++-----------
 test/extension/test.sh                 | 42 ++++++-------
 2 files changed, 69 insertions(+), 55 deletions(-)

diff --git a/scripts-available/CDB_ForeignTable.sql b/scripts-available/CDB_ForeignTable.sql
index ed36449..e6e86e7 100644
--- a/scripts-available/CDB_ForeignTable.sql
+++ b/scripts-available/CDB_ForeignTable.sql
@@ -139,6 +139,15 @@ $$
 LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE;
 
 
+-- Produce a valid DB name for objects created for the user FDW's
+CREATE OR REPLACE FUNCTION @extschema@.__CDB_User_FDW_Object_Names(fdw_input_name NAME)
+RETURNS NAME AS $$
+  -- Note on input we use %s and on output we use %I, in order to
+  -- avoid double escaping
+  SELECT format('cdb_fdw_%s', fdw_input_name)::NAME;
+$$
+LANGUAGE sql IMMUTABLE PARALLEL SAFE;
+
 -- A function to set up a user-defined foreign data server
 -- It does not read from CDB_Conf.
 -- Only superuser roles can invoke it successfully
@@ -159,22 +168,23 @@ LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE;
 --
 -- Underneath it will:
 --   * Set up postgresql_fdw
---   * Create a server with the name 'amazon'
---   * Create a role called 'amazon' to manage access
---   * Create a user mapping with that role 'amazon'
---   * Create a schema 'amazon' as a convenience to set up all foreign
+--   * Create a server with the name 'cdb_fdw_amazon'
+--   * Create a role called 'cdb_fdw_amazon' to manage access
+--   * Create a user mapping with that role 'cdb_fdw_amazon'
+--   * Create a schema 'cdb_fdw_amazon' as a convenience to set up all foreign
 --     tables over there
 --
 -- It is the responsibility of the superuser to grant that role to either:
 --   * Nobody
 --   * Specific roles: GRANT amazon TO role_name;
---   * Members of the organization: SELECT cartodb.CDB_Organization_Grant_Role('amazon');
---   * The publicuser: GRANT amazon TO publicuser;
-CREATE OR REPLACE FUNCTION @extschema@._CDB_SetUp_User_PG_FDW_Server(fdw_name NAME, config json)
+--   * Members of the organization: SELECT cartodb.CDB_Organization_Grant_Role('cdb_fdw_amazon');
+--   * The publicuser: GRANT cdb_fdw_amazon TO publicuser;
+CREATE OR REPLACE FUNCTION @extschema@._CDB_SetUp_User_PG_FDW_Server(fdw_input_name NAME, config json)
 RETURNS void AS $$
 DECLARE
   row record;
   option record;
+  fdw_objects_name NAME := @extschema@.__CDB_User_FDW_Object_Names(fdw_input_name);
 BEGIN
   -- TODO: refactor with original function
   -- This function tries to be as idempotent as possible, by not creating anything more than once
@@ -183,57 +193,57 @@ BEGIN
     CREATE EXTENSION postgres_fdw;
   END IF;
   -- Create FDW first if it does not exist
-  IF NOT EXISTS ( SELECT * FROM pg_foreign_server WHERE srvname = fdw_name)
+  IF NOT EXISTS ( SELECT * FROM pg_foreign_server WHERE srvname = fdw_objects_name)
     THEN
-    EXECUTE FORMAT('CREATE SERVER %I FOREIGN DATA WRAPPER postgres_fdw', fdw_name);
+    EXECUTE FORMAT('CREATE SERVER %I FOREIGN DATA WRAPPER postgres_fdw', fdw_objects_name);
   END IF;
 
   -- Set FDW settings
   FOR row IN SELECT p.key, p.value from lateral json_each_text(config->'server') p
     LOOP
-      IF NOT EXISTS (WITH a AS (select split_part(unnest(srvoptions), '=', 1) as options from pg_foreign_server where srvname=fdw_name) SELECT * from a where options = row.key)
+      IF NOT EXISTS (WITH a AS (select split_part(unnest(srvoptions), '=', 1) as options from pg_foreign_server where srvname=fdw_objects_name) SELECT * from a where options = row.key)
         THEN
-        EXECUTE FORMAT('ALTER SERVER %I OPTIONS (ADD %I %L)', fdw_name, row.key, row.value);
+        EXECUTE FORMAT('ALTER SERVER %I OPTIONS (ADD %I %L)', fdw_objects_name, row.key, row.value);
       ELSE
-        EXECUTE FORMAT('ALTER SERVER %I OPTIONS (SET %I %L)', fdw_name, row.key, row.value);
+        EXECUTE FORMAT('ALTER SERVER %I OPTIONS (SET %I %L)', fdw_objects_name, row.key, row.value);
       END IF;
     END LOOP;
 
     -- Create specific role for this
-    IF NOT EXISTS ( SELECT 1 FROM pg_roles WHERE rolname = fdw_name) THEN
-       EXECUTE format('CREATE ROLE %I NOLOGIN', fdw_name);
+    IF NOT EXISTS ( SELECT 1 FROM pg_roles WHERE rolname = fdw_objects_name) THEN
+       EXECUTE format('CREATE ROLE %I NOLOGIN', fdw_objects_name);
     END IF;
 
     -- Transfer ownership of the server to the fdw role
-    EXECUTE format('ALTER SERVER %I OWNER TO %I', fdw_name, fdw_name);
+    EXECUTE format('ALTER SERVER %I OWNER TO %I', fdw_objects_name, fdw_objects_name);
 
     -- Create user mapping
     -- NOTE: we use a PUBLIC user mapping but control access to the SERVER
     -- so that we don't need to create a mapping for every user nor store credentials elsewhere
-    IF NOT EXISTS ( SELECT * FROM pg_user_mappings WHERE srvname = fdw_name AND usename = 'public' ) THEN
-        EXECUTE FORMAT ('CREATE USER MAPPING FOR public SERVER %I', fdw_name);
+    IF NOT EXISTS ( SELECT * FROM pg_user_mappings WHERE srvname = fdw_objects_name AND usename = 'public' ) THEN
+        EXECUTE FORMAT ('CREATE USER MAPPING FOR public SERVER %I', fdw_objects_name);
     END IF;
 
     -- Update user mapping settings
     FOR option IN SELECT o.key, o.value from lateral json_each_text(config->'user_mapping') o LOOP
-        IF NOT EXISTS (WITH a AS (select split_part(unnest(umoptions), '=', 1) as options from pg_user_mappings WHERE srvname = fdw_name AND usename = 'public') SELECT * from a where options = option.key) THEN
-          EXECUTE FORMAT('ALTER USER MAPPING FOR PUBLIC SERVER %I OPTIONS (ADD %I %L)', fdw_name, option.key, option.value);
+        IF NOT EXISTS (WITH a AS (select split_part(unnest(umoptions), '=', 1) as options from pg_user_mappings WHERE srvname = fdw_objects_name AND usename = 'public') SELECT * from a where options = option.key) THEN
+          EXECUTE FORMAT('ALTER USER MAPPING FOR PUBLIC SERVER %I OPTIONS (ADD %I %L)', fdw_objects_name, option.key, option.value);
         ELSE
-          EXECUTE FORMAT('ALTER USER MAPPING FOR PUBLIC SERVER %I OPTIONS (SET %I %L)', fdw_name, option.key, option.value);
+          EXECUTE FORMAT('ALTER USER MAPPING FOR PUBLIC SERVER %I OPTIONS (SET %I %L)', fdw_objects_name, option.key, option.value);
         END IF;
     END LOOP;
 
     -- Grant usage on the wrapper and server to the fdw role
-    EXECUTE FORMAT ('GRANT USAGE ON FOREIGN DATA WRAPPER postgres_fdw TO %I', fdw_name);
-    EXECUTE FORMAT ('GRANT USAGE ON FOREIGN SERVER %I TO %I', fdw_name, fdw_name);
+    EXECUTE FORMAT ('GRANT USAGE ON FOREIGN DATA WRAPPER postgres_fdw TO %I', fdw_objects_name);
+    EXECUTE FORMAT ('GRANT USAGE ON FOREIGN SERVER %I TO %I', fdw_objects_name, fdw_objects_name);
 
     -- Create schema if it does not exist.
-    IF NOT EXISTS ( SELECT * from pg_namespace WHERE nspname=fdw_name) THEN
-      EXECUTE FORMAT ('CREATE SCHEMA %I', fdw_name);
+    IF NOT EXISTS ( SELECT * from pg_namespace WHERE nspname=fdw_objects_name) THEN
+      EXECUTE FORMAT ('CREATE SCHEMA %I', fdw_objects_name);
     END IF;
 
     -- Give the fdw role ownership over the schema
-    EXECUTE FORMAT ('ALTER SCHEMA %I OWNER TO %I', fdw_name, fdw_name);
+    EXECUTE FORMAT ('ALTER SCHEMA %I OWNER TO %I', fdw_objects_name, fdw_objects_name);
 
     -- TODO: Bring here the remote cdb_tablemetadata
 END
@@ -248,14 +258,16 @@ $$ LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE;
 --   SELECT cartodb.CDB_Drop_User_PG_FDW_Server('amazon')
 --
 -- Note: if there's any dependent object (i.e. foreign table) this call will fail
-CREATE OR REPLACE FUNCTION @extschema@._CDB_Drop_User_PG_FDW_Server(fdw_name NAME)
+CREATE OR REPLACE FUNCTION @extschema@._CDB_Drop_User_PG_FDW_Server(fdw_input_name NAME)
 RETURNS void AS $$
+DECLARE
+    fdw_objects_name NAME := @extschema@.__CDB_User_FDW_Object_Names(fdw_input_name);
 BEGIN
-    EXECUTE FORMAT ('DROP SCHEMA %I', fdw_name);
-    EXECUTE FORMAT ('DROP USER MAPPING FOR public SERVER %I', fdw_name);
-    EXECUTE FORMAT ('DROP SERVER %I', fdw_name);
-    EXECUTE FORMAT ('REVOKE USAGE ON FOREIGN DATA WRAPPER postgres_fdw FROM %I', fdw_name);
-    EXECUTE FORMAT ('DROP ROLE %I', fdw_name);
+    EXECUTE FORMAT ('DROP SCHEMA %I', fdw_objects_name);
+    EXECUTE FORMAT ('DROP USER MAPPING FOR public SERVER %I', fdw_objects_name);
+    EXECUTE FORMAT ('DROP SERVER %I', fdw_objects_name);
+    EXECUTE FORMAT ('REVOKE USAGE ON FOREIGN DATA WRAPPER postgres_fdw FROM %I', fdw_objects_name);
+    EXECUTE FORMAT ('DROP ROLE %I', fdw_objects_name);
 END
 $$ LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE;
 
@@ -264,12 +276,14 @@ $$ LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE;
 -- E.g:
 --   SELECT cartodb.CDB_SetUp_User_PG_FDW_Table('amazon', 'carto_lite', 'mytable');
 --   SELECT * FROM amazon.my_table;
-CREATE OR REPLACE FUNCTION @extschema@.CDB_SetUp_User_PG_FDW_Table(fdw_name NAME, foreign_schema NAME, table_name NAME)
+CREATE OR REPLACE FUNCTION @extschema@.CDB_SetUp_User_PG_FDW_Table(fdw_input_name NAME, foreign_schema NAME, table_name NAME)
 RETURNS void AS $$
+DECLARE
+    fdw_objects_name NAME := @extschema@.__CDB_User_FDW_Object_Names(fdw_input_name);
 BEGIN
-  EXECUTE FORMAT ('IMPORT FOREIGN SCHEMA %I LIMIT TO (%I) FROM SERVER %I INTO %I;', foreign_schema, table_name, fdw_name, fdw_name);
+  EXECUTE FORMAT ('IMPORT FOREIGN SCHEMA %I LIMIT TO (%I) FROM SERVER %I INTO %I;', foreign_schema, table_name, fdw_objects_name, fdw_objects_name);
   --- Grant SELECT to fdw role
-  EXECUTE FORMAT ('GRANT SELECT ON %I.%I TO %I;', fdw_name, table_name, fdw_name);
+  EXECUTE FORMAT ('GRANT SELECT ON %I.%I TO %I;', fdw_objects_name, table_name, fdw_objects_name);
 END
 $$ LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE;
 
diff --git a/test/extension/test.sh b/test/extension/test.sh
index b520343..3c441e1 100755
--- a/test/extension/test.sh
+++ b/test/extension/test.sh
@@ -607,43 +607,43 @@ test_extension|public|"local-table-with-dashes"'
    }
 }
 EOF
-    sql postgres "SELECT cartodb._CDB_SetUp_User_PG_FDW_Server('test_user_fdw', '$ufdw_config');"
+    sql postgres "SELECT cartodb._CDB_SetUp_User_PG_FDW_Server('user_defined_test', '$ufdw_config');"
 
     # Grant a user access to that FDW, and to grant to others
-    sql postgres "GRANT test_user_fdw TO cdb_testmember_1 WITH ADMIN OPTION;"
+    sql postgres "GRANT cdb_fdw_user_defined_test TO cdb_testmember_1 WITH ADMIN OPTION;"
 
     # Set up a user foreign table
-    sql cdb_testmember_1 "SELECT cartodb.CDB_SetUp_User_PG_FDW_Table('test_user_fdw', 'test_fdw', 'foo');"
+    sql cdb_testmember_1 "SELECT cartodb.CDB_SetUp_User_PG_FDW_Table('user_defined_test', 'test_fdw', 'foo');"
 
     # Check that the table can be accessed by the owner/creator
-    sql cdb_testmember_1 "SELECT * from test_user_fdw.foo;"
-    sql cdb_testmember_1 "SELECT a from test_user_fdw.foo LIMIT 1;" should 42
+    sql cdb_testmember_1 "SELECT * from cdb_fdw_user_defined_test.foo;"
+    sql cdb_testmember_1 "SELECT a from cdb_fdw_user_defined_test.foo LIMIT 1;" should 42
 
     # Check that a role with no permissions cannot use the FDW to access a remote table
-    sql cdb_testmember_2 "IMPORT FOREIGN SCHEMA test_fdw LIMIT TO (foo) FROM SERVER test_user_fdw INTO public" fails
+    sql cdb_testmember_2 "IMPORT FOREIGN SCHEMA test_fdw LIMIT TO (foo) FROM SERVER cdb_fdw_user_defined_test INTO public" fails
 
     # Check that the table can be accessed by some other user by granting the role
-    sql cdb_testmember_2 "SELECT a from test_user_fdw.foo LIMIT 1;" fails
-    sql cdb_testmember_1 "GRANT test_user_fdw TO cdb_testmember_2;"
-    sql cdb_testmember_2 "SELECT a from test_user_fdw.foo LIMIT 1;" should 42
-    sql cdb_testmember_1 "REVOKE test_user_fdw FROM cdb_testmember_2;"
+    sql cdb_testmember_2 "SELECT a from cdb_fdw_user_defined_test.foo LIMIT 1;" fails
+    sql cdb_testmember_1 "GRANT cdb_fdw_user_defined_test TO cdb_testmember_2;"
+    sql cdb_testmember_2 "SELECT a from cdb_fdw_user_defined_test.foo LIMIT 1;" should 42
+    sql cdb_testmember_1 "REVOKE cdb_fdw_user_defined_test FROM cdb_testmember_2;"
 
     # Check that the table can be accessed by org members
-    sql cdb_testmember_2 "SELECT a from test_user_fdw.foo LIMIT 1;" fails
-    sql cdb_testmember_1 "SELECT cartodb.CDB_Organization_Grant_Role('test_user_fdw');"
-    sql cdb_testmember_2 "SELECT a from test_user_fdw.foo LIMIT 1;" should 42
-    sql cdb_testmember_1 "SELECT cartodb.CDB_Organization_Revoke_Role('test_user_fdw');"
+    sql cdb_testmember_2 "SELECT a from cdb_fdw_user_defined_test.foo LIMIT 1;" fails
+    sql cdb_testmember_1 "SELECT cartodb.CDB_Organization_Grant_Role('cdb_fdw_user_defined_test');"
+    sql cdb_testmember_2 "SELECT a from cdb_fdw_user_defined_test.foo LIMIT 1;" should 42
+    sql cdb_testmember_1 "SELECT cartodb.CDB_Organization_Revoke_Role('cdb_fdw_user_defined_test');"
 
     # By default publicuser cannot access the FDW
-    sql publicuser "SELECT a from test_user_fdw.foo LIMIT 1;" fails
-    sql cdb_testmember_1 "GRANT test_user_fdw TO publicuser;" # but can be granted
-    sql publicuser "SELECT a from test_user_fdw.foo LIMIT 1;" should 42
-    sql cdb_testmember_1 "REVOKE test_user_fdw FROM publicuser;"
+    sql publicuser "SELECT a from cdb_fdw_user_defined_test.foo LIMIT 1;" fails
+    sql cdb_testmember_1 "GRANT cdb_fdw_user_defined_test TO publicuser;" # but can be granted
+    sql publicuser "SELECT a from cdb_fdw_user_defined_test.foo LIMIT 1;" should 42
+    sql cdb_testmember_1 "REVOKE cdb_fdw_user_defined_test FROM publicuser;"
 
     # If there are dependent objects, we cannot drop the foreign server
-    sql postgres "SELECT cartodb._CDB_Drop_User_PG_FDW_Server('test_user_fdw')" fails
-    sql cdb_testmember_1 "DROP FOREIGN TABLE test_user_fdw.foo;"
-    sql postgres "SELECT cartodb._CDB_Drop_User_PG_FDW_Server('test_user_fdw')"
+    sql postgres "SELECT cartodb._CDB_Drop_User_PG_FDW_Server('user_defined_test')" fails
+    sql cdb_testmember_1 "DROP FOREIGN TABLE cdb_fdw_user_defined_test.foo;"
+    sql postgres "SELECT cartodb._CDB_Drop_User_PG_FDW_Server('user_defined_test')"
 
 
     # Teardown

From 076207c49cebcf2fd5d08334413e4b51f633a390 Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@carto.com>
Date: Tue, 16 Jul 2019 16:24:02 +0200
Subject: [PATCH 25/28] Make sure there are no (double)escaping issues

---
 test/extension/test.sh | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/test/extension/test.sh b/test/extension/test.sh
index 3c441e1..d506a91 100755
--- a/test/extension/test.sh
+++ b/test/extension/test.sh
@@ -607,43 +607,43 @@ test_extension|public|"local-table-with-dashes"'
    }
 }
 EOF
-    sql postgres "SELECT cartodb._CDB_SetUp_User_PG_FDW_Server('user_defined_test', '$ufdw_config');"
+    sql postgres "SELECT cartodb._CDB_SetUp_User_PG_FDW_Server('user-defined-test', '$ufdw_config');"
 
     # Grant a user access to that FDW, and to grant to others
-    sql postgres "GRANT cdb_fdw_user_defined_test TO cdb_testmember_1 WITH ADMIN OPTION;"
+    sql postgres 'GRANT "cdb_fdw_user-defined-test" TO cdb_testmember_1 WITH ADMIN OPTION;'
 
     # Set up a user foreign table
-    sql cdb_testmember_1 "SELECT cartodb.CDB_SetUp_User_PG_FDW_Table('user_defined_test', 'test_fdw', 'foo');"
+    sql cdb_testmember_1 "SELECT cartodb.CDB_SetUp_User_PG_FDW_Table('user-defined-test', 'test_fdw', 'foo');"
 
     # Check that the table can be accessed by the owner/creator
-    sql cdb_testmember_1 "SELECT * from cdb_fdw_user_defined_test.foo;"
-    sql cdb_testmember_1 "SELECT a from cdb_fdw_user_defined_test.foo LIMIT 1;" should 42
+    sql cdb_testmember_1 'SELECT * from "cdb_fdw_user-defined-test".foo;'
+    sql cdb_testmember_1 'SELECT a from "cdb_fdw_user-defined-test".foo LIMIT 1;' should 42
 
     # Check that a role with no permissions cannot use the FDW to access a remote table
-    sql cdb_testmember_2 "IMPORT FOREIGN SCHEMA test_fdw LIMIT TO (foo) FROM SERVER cdb_fdw_user_defined_test INTO public" fails
+    sql cdb_testmember_2 'IMPORT FOREIGN SCHEMA test_fdw LIMIT TO (foo) FROM SERVER "cdb_fdw_user-defined-test" INTO public' fails
 
     # Check that the table can be accessed by some other user by granting the role
-    sql cdb_testmember_2 "SELECT a from cdb_fdw_user_defined_test.foo LIMIT 1;" fails
-    sql cdb_testmember_1 "GRANT cdb_fdw_user_defined_test TO cdb_testmember_2;"
-    sql cdb_testmember_2 "SELECT a from cdb_fdw_user_defined_test.foo LIMIT 1;" should 42
-    sql cdb_testmember_1 "REVOKE cdb_fdw_user_defined_test FROM cdb_testmember_2;"
+    sql cdb_testmember_2 'SELECT a from "cdb_fdw_user-defined-test".foo LIMIT 1;' fails
+    sql cdb_testmember_1 'GRANT "cdb_fdw_user-defined-test" TO cdb_testmember_2;'
+    sql cdb_testmember_2 'SELECT a from "cdb_fdw_user-defined-test".foo LIMIT 1;' should 42
+    sql cdb_testmember_1 'REVOKE "cdb_fdw_user-defined-test" FROM cdb_testmember_2;'
 
     # Check that the table can be accessed by org members
-    sql cdb_testmember_2 "SELECT a from cdb_fdw_user_defined_test.foo LIMIT 1;" fails
-    sql cdb_testmember_1 "SELECT cartodb.CDB_Organization_Grant_Role('cdb_fdw_user_defined_test');"
-    sql cdb_testmember_2 "SELECT a from cdb_fdw_user_defined_test.foo LIMIT 1;" should 42
-    sql cdb_testmember_1 "SELECT cartodb.CDB_Organization_Revoke_Role('cdb_fdw_user_defined_test');"
+    sql cdb_testmember_2 'SELECT a from "cdb_fdw_user-defined-test".foo LIMIT 1;' fails
+    sql cdb_testmember_1 "SELECT cartodb.CDB_Organization_Grant_Role('cdb_fdw_user-defined-test');"
+    sql cdb_testmember_2 'SELECT a from "cdb_fdw_user-defined-test".foo LIMIT 1;' should 42
+    sql cdb_testmember_1 "SELECT cartodb.CDB_Organization_Revoke_Role('cdb_fdw_user-defined-test');"
 
     # By default publicuser cannot access the FDW
-    sql publicuser "SELECT a from cdb_fdw_user_defined_test.foo LIMIT 1;" fails
-    sql cdb_testmember_1 "GRANT cdb_fdw_user_defined_test TO publicuser;" # but can be granted
-    sql publicuser "SELECT a from cdb_fdw_user_defined_test.foo LIMIT 1;" should 42
-    sql cdb_testmember_1 "REVOKE cdb_fdw_user_defined_test FROM publicuser;"
+    sql publicuser 'SELECT a from "cdb_fdw_user-defined-test".foo LIMIT 1;' fails
+    sql cdb_testmember_1 'GRANT "cdb_fdw_user-defined-test" TO publicuser;' # but can be granted
+    sql publicuser 'SELECT a from "cdb_fdw_user-defined-test".foo LIMIT 1;' should 42
+    sql cdb_testmember_1 'REVOKE "cdb_fdw_user-defined-test" FROM publicuser;'
 
     # If there are dependent objects, we cannot drop the foreign server
-    sql postgres "SELECT cartodb._CDB_Drop_User_PG_FDW_Server('user_defined_test')" fails
-    sql cdb_testmember_1 "DROP FOREIGN TABLE cdb_fdw_user_defined_test.foo;"
-    sql postgres "SELECT cartodb._CDB_Drop_User_PG_FDW_Server('user_defined_test')"
+    sql postgres "SELECT cartodb._CDB_Drop_User_PG_FDW_Server('user-defined-test')" fails
+    sql cdb_testmember_1 'DROP FOREIGN TABLE "cdb_fdw_user-defined-test".foo;'
+    sql postgres "SELECT cartodb._CDB_Drop_User_PG_FDW_Server('user-defined-test')"
 
 
     # Teardown

From 3c460f1a85a7ec4fe041540af0a7c622f8cdd753 Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@carto.com>
Date: Tue, 16 Jul 2019 17:03:23 +0200
Subject: [PATCH 26/28] Add a bunch of RAISE NOTICE's to inform user about
 progress

---
 scripts-available/CDB_ForeignTable.sql | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/scripts-available/CDB_ForeignTable.sql b/scripts-available/CDB_ForeignTable.sql
index e6e86e7..b66fa8c 100644
--- a/scripts-available/CDB_ForeignTable.sql
+++ b/scripts-available/CDB_ForeignTable.sql
@@ -191,11 +191,13 @@ BEGIN
   -- (not even using IF NOT EXIST to avoid throwing warnings)
   IF NOT EXISTS ( SELECT * FROM pg_extension WHERE extname = 'postgres_fdw') THEN
     CREATE EXTENSION postgres_fdw;
+    RAISE NOTICE 'Created postgres_fdw extension';
   END IF;
   -- Create FDW first if it does not exist
   IF NOT EXISTS ( SELECT * FROM pg_foreign_server WHERE srvname = fdw_objects_name)
     THEN
     EXECUTE FORMAT('CREATE SERVER %I FOREIGN DATA WRAPPER postgres_fdw', fdw_objects_name);
+    RAISE NOTICE 'Created server % using postgres_fdw', fdw_objects_name;
   END IF;
 
   -- Set FDW settings
@@ -212,6 +214,7 @@ BEGIN
     -- Create specific role for this
     IF NOT EXISTS ( SELECT 1 FROM pg_roles WHERE rolname = fdw_objects_name) THEN
        EXECUTE format('CREATE ROLE %I NOLOGIN', fdw_objects_name);
+       RAISE NOTICE 'Created special role % to access the correponding FDW', fdw_objects_name;
     END IF;
 
     -- Transfer ownership of the server to the fdw role
@@ -222,6 +225,7 @@ BEGIN
     -- so that we don't need to create a mapping for every user nor store credentials elsewhere
     IF NOT EXISTS ( SELECT * FROM pg_user_mappings WHERE srvname = fdw_objects_name AND usename = 'public' ) THEN
         EXECUTE FORMAT ('CREATE USER MAPPING FOR public SERVER %I', fdw_objects_name);
+        RAISE NOTICE 'Created user mapping for accesing foreign server %', fdw_objects_name;
     END IF;
 
     -- Update user mapping settings
@@ -235,15 +239,19 @@ BEGIN
 
     -- Grant usage on the wrapper and server to the fdw role
     EXECUTE FORMAT ('GRANT USAGE ON FOREIGN DATA WRAPPER postgres_fdw TO %I', fdw_objects_name);
+    RAISE NOTICE 'Granted usage on the postgres_fdw to the role %', fdw_objects_name;
     EXECUTE FORMAT ('GRANT USAGE ON FOREIGN SERVER %I TO %I', fdw_objects_name, fdw_objects_name);
+    RAISE NOTICE 'Granted usage on the foreign server to the role %', fdw_objects_name;
 
     -- Create schema if it does not exist.
     IF NOT EXISTS ( SELECT * from pg_namespace WHERE nspname=fdw_objects_name) THEN
       EXECUTE FORMAT ('CREATE SCHEMA %I', fdw_objects_name);
+      RAISE NOTICE 'Created schema % to host foreign tables', fdw_objects_name;
     END IF;
 
     -- Give the fdw role ownership over the schema
     EXECUTE FORMAT ('ALTER SCHEMA %I OWNER TO %I', fdw_objects_name, fdw_objects_name);
+    RAISE NOTICE 'Gave ownership on the schema % to %', fdw_objects_name, fdw_objects_name;
 
     -- TODO: Bring here the remote cdb_tablemetadata
 END
@@ -264,10 +272,15 @@ DECLARE
     fdw_objects_name NAME := @extschema@.__CDB_User_FDW_Object_Names(fdw_input_name);
 BEGIN
     EXECUTE FORMAT ('DROP SCHEMA %I', fdw_objects_name);
+    RAISE NOTICE 'Dropped schema %', fdw_objects_name;
     EXECUTE FORMAT ('DROP USER MAPPING FOR public SERVER %I', fdw_objects_name);
+    RAISE NOTICE 'Dropped user mapping for server %', fdw_objects_name;
     EXECUTE FORMAT ('DROP SERVER %I', fdw_objects_name);
+    RAISE NOTICE 'Dropped foreign server %', fdw_objects_name;
     EXECUTE FORMAT ('REVOKE USAGE ON FOREIGN DATA WRAPPER postgres_fdw FROM %I', fdw_objects_name);
+    RAISE NOTICE 'Revoked usage on postgres_fdw from %', fdw_objects_name;
     EXECUTE FORMAT ('DROP ROLE %I', fdw_objects_name);
+    RAISE NOTICE 'Dropped role %', fdw_objects_name;
 END
 $$ LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE;
 

From e41d2ec0190de3562596b87824c2dee351e98182 Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@carto.com>
Date: Tue, 16 Jul 2019 17:35:41 +0200
Subject: [PATCH 27/28] Add a flag to force drop of user FDW and related
 objects

If force = true then it will add the subclause `CASCADE` to the SQL
DDL sentences that support it, otherwise it'll use `RESTRICT` which is
the default and exact opposite.
---
 scripts-available/CDB_ForeignTable.sql | 16 ++++++++++++----
 test/extension/test.sh                 |  6 ++++++
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/scripts-available/CDB_ForeignTable.sql b/scripts-available/CDB_ForeignTable.sql
index b66fa8c..ae3a5ea 100644
--- a/scripts-available/CDB_ForeignTable.sql
+++ b/scripts-available/CDB_ForeignTable.sql
@@ -266,18 +266,26 @@ $$ LANGUAGE plpgsql VOLATILE PARALLEL UNSAFE;
 --   SELECT cartodb.CDB_Drop_User_PG_FDW_Server('amazon')
 --
 -- Note: if there's any dependent object (i.e. foreign table) this call will fail
-CREATE OR REPLACE FUNCTION @extschema@._CDB_Drop_User_PG_FDW_Server(fdw_input_name NAME)
+CREATE OR REPLACE FUNCTION @extschema@._CDB_Drop_User_PG_FDW_Server(fdw_input_name NAME, force boolean = false)
 RETURNS void AS $$
 DECLARE
     fdw_objects_name NAME := @extschema@.__CDB_User_FDW_Object_Names(fdw_input_name);
+    cascade_clause NAME;
 BEGIN
-    EXECUTE FORMAT ('DROP SCHEMA %I', fdw_objects_name);
+    CASE force
+        WHEN true THEN
+            cascade_clause := 'CASCADE';
+        ELSE
+            cascade_clause := 'RESTRICT';
+    END CASE;
+
+    EXECUTE FORMAT ('DROP SCHEMA %I %s', fdw_objects_name, cascade_clause);
     RAISE NOTICE 'Dropped schema %', fdw_objects_name;
     EXECUTE FORMAT ('DROP USER MAPPING FOR public SERVER %I', fdw_objects_name);
     RAISE NOTICE 'Dropped user mapping for server %', fdw_objects_name;
-    EXECUTE FORMAT ('DROP SERVER %I', fdw_objects_name);
+    EXECUTE FORMAT ('DROP SERVER %I %s', fdw_objects_name, cascade_clause);
     RAISE NOTICE 'Dropped foreign server %', fdw_objects_name;
-    EXECUTE FORMAT ('REVOKE USAGE ON FOREIGN DATA WRAPPER postgres_fdw FROM %I', fdw_objects_name);
+    EXECUTE FORMAT ('REVOKE USAGE ON FOREIGN DATA WRAPPER postgres_fdw FROM %I %s', fdw_objects_name, cascade_clause);
     RAISE NOTICE 'Revoked usage on postgres_fdw from %', fdw_objects_name;
     EXECUTE FORMAT ('DROP ROLE %I', fdw_objects_name);
     RAISE NOTICE 'Dropped role %', fdw_objects_name;
diff --git a/test/extension/test.sh b/test/extension/test.sh
index d506a91..73491ec 100755
--- a/test/extension/test.sh
+++ b/test/extension/test.sh
@@ -645,6 +645,12 @@ EOF
     sql cdb_testmember_1 'DROP FOREIGN TABLE "cdb_fdw_user-defined-test".foo;'
     sql postgres "SELECT cartodb._CDB_Drop_User_PG_FDW_Server('user-defined-test')"
 
+    # But if there are, we can set the force flag to true to drop everything (defaults to false)
+    sql postgres "SELECT cartodb._CDB_SetUp_User_PG_FDW_Server('another_user_defined_test', '$ufdw_config');"
+    sql postgres 'GRANT cdb_fdw_another_user_defined_test TO cdb_testmember_1 WITH ADMIN OPTION;'
+    sql cdb_testmember_1 "SELECT cartodb.CDB_SetUp_User_PG_FDW_Table('another_user_defined_test', 'test_fdw', 'foo');"
+    sql postgres "SELECT cartodb._CDB_Drop_User_PG_FDW_Server('another_user_defined_test', /* force = */ true)"
+
 
     # Teardown
     DATABASE=fdw_target sql postgres 'REVOKE USAGE ON SCHEMA test_fdw FROM fdw_user;'

From f9bd469ea9a973f2ab077e57b22e640c2e8e83ba Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@carto.com>
Date: Wed, 17 Jul 2019 09:46:49 +0200
Subject: [PATCH 28/28] Make oauth tests a bit more robust

---
 test/CDB_OAuth.sql | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/test/CDB_OAuth.sql b/test/CDB_OAuth.sql
index 4a2b447..c3c14e6 100644
--- a/test/CDB_OAuth.sql
+++ b/test/CDB_OAuth.sql
@@ -1,7 +1,9 @@
 -- Create user and enable OAuth event trigger
 \set QUIET on
 SET client_min_messages TO error;
+DROP ROLE IF EXISTS "creator_role";
 CREATE ROLE "creator_role" LOGIN;
+DROP ROLE IF EXISTS "ownership_role";
 CREATE ROLE "ownership_role" LOGIN;
 GRANT ALL ON SCHEMA cartodb TO "creator_role";
 SELECT CDB_Conf_SetConf('api_keys_creator_role', '{"username": "creator_role", "permissions":[]}');