Updates apt-get so it can pick up latest postgres packages

- fixes tests to match new signature.
- does not revoke access to the schema when revoking access to a table. TODO

.travis.yml

@@ -5,6 +5,7 @@ addons:
 
 before_install:
   #- sudo apt-get install -q postgresql-9.3-postgis-2.1
+  - sudo apt-get update
   - sudo apt-get install -q postgresql-server-dev-9.3
   - sudo apt-get install -q postgresql-plpython-9.3
   # Install schema_triggers
@@ -19,5 +20,5 @@ before_install:
 script:
   - make
   - sudo make install
-  - PGOPTIONS='-c client_min_messages=NOTICE' make installcheck ||
+  - PGOPTIONS='-c client_min_messages=NOTICE' PGUSER=postgres make installcheck ||
     { cat regression.diffs; false; }

CONTRIBUTING

@@ -0,0 +1,61 @@
+The development tracker for cartodb-postgresql is on github:
+http://github.com/cartodb/cartodb-postgresql/
+
+Bug fixes are best reported as pull requests over there.
+Features are best discussed on the mailing list:
+https://groups.google.com/d/forum/cartodb
+
+Adding features to the extension
+--------------------------------
+
+Extension features are coded in scripts found under the
+"scripts-available" directory. A feature can be a single function
+or a group of function with a specific scope.
+
+The "scripts-enabled" directory contains symlinks to the scripts
+in "scripts-available". Any symlink in that directory is automatically
+included in the extension. Numbering can be used to enforce the order
+in which those scripts are loaded.
+
+Scripts would be best coded in a way to be usable both for creation
+and upgrade of the objects. This means using CREATE OR REPLACE for
+the functions, and whatever it takes to check existance of any previous
+version of objects in other cases.
+
+When used as an extension (probably always from version 0.2.0 onwards)
+all the objects will be installed in a "cartodb" schema. Take this into
+account to fully-qualify internal calls to avoid (possibly dangerous)
+name clashes.
+
+Every new feature (as well as bugfixes) should come with a testcase,
+see next session.
+
+Writing testcases
+-----------------
+
+Tests reside in the test/ directory.
+You can find information about how to write tests in test/README
+
+Testing changes live
+--------------------
+
+Testing changes made during development requires upgrading
+the extension into your test database. 
+
+During development the cartodb extension version doesn't change with
+every commit, so testing latest change requires cheating with PostgreSQL
+so to enforce re-load of the scripts. To help with cheating, "make install"
+also installs migration scripts to go from "V" to "V"next and from "V"next
+to "V". Example to upgrade a 0.2.0dev version:
+
+```sql
+ALTER EXTENSION cartodb UPDATE TO '0.2.0devnext';
+ALTER EXTENSION cartodb UPDATE TO '0.2.0dev';
+```
+
+Starting with 0.2.0, the in-place reload can be done with an ad-hoc function:
+
+```sql
+SELECT cartodb.cdb_extension_reload();
+```
+

Makefile

@@ -1,7 +1,7 @@
 # cartodb/Makefile
 
 EXTENSION = cartodb
-EXTVERSION = 0.2.0
+EXTVERSION = 0.3.2
 
 SED = sed
 
@@ -17,7 +17,12 @@ UPGRADABLE = \
   unpackaged \
   0.1.0 \
   0.1.1 \
-  0.2.0dev \
+  0.2.0 \
+  0.2.1 \
+  0.3.0 \
+  0.3.0dev \
+  0.3.1 \
+  $(EXTVERSION)dev \
   $(EXTVERSION)next \
   $(END)
 
@@ -89,7 +94,10 @@ legacy_regress: $(REGRESS_OLD) Makefile
     cat test/$${tn}_expect >> $${exp}; \
   done
 
+test_organization:
+	bash test/organization/test.sh
+
 legacy_tests: legacy_regress 
 
-installcheck: legacy_tests
+installcheck: legacy_tests test_organization
 

NEWS

@@ -1,3 +1,29 @@
+0.3.2 (2014-07-28)
+------------------
+* Make 0.3.0dev version upgradeable
+
+0.3.1 (2014-07-22)
+------------------
+* Dummy version. We start using semantic versioning
+
+0.3.0 (2014-07-15)
+------------------
+* Permission management functions
+* Adapt functions to use schemas
+
+0.2.1 - 2014-06-11
+------------------
+
+Enhancements:
+
+ - Do not force re-cartodbfication on CREATE FROM unpackaged
+ - Drop useless DEFAULT specification in plpgsql variable declarations
+ - List plpythonu requirement first, to get pg_catalog scanned before public
+
+Bug fixes:
+
+ - Do not add unique index on cartodb_id if already a primary key (#38)
+
 0.2.0 - 2014-06-09
 ------------------
 

README.md

@@ -31,6 +31,9 @@ NOTE: if ``test_ddl_triggers`` fails it's likely due to an incomplete
       installation of schema_triggers: you need to add ``schema_triggers.so``
       to the ``shared_preload_libraries`` setting in postgresql.conf !
 
+NOTE: you need to run the installcheck as a superuser, use PGUSER
+      env variable if needed, like: PGUSER=postgres make installcheck
+
 Enable database
 ---------------
 
@@ -87,19 +90,5 @@ ls `pg_config --sharedir`/extension/cartodb*
 ```
 
 During development the cartodb extension version doesn't change with
-every commit, so testing latest change requires cheating with PostgreSQL
-so to enforce re-load of the scripts. To help with cheating, "make install"
-also installs migration scripts to go from "V" to "V"next and from "V"next
-to "V". Example to upgrade a 0.2.0dev version:
-
-```sql
-ALTER EXTENSION cartodb UPDATE TO '0.2.0devnext';
-ALTER EXTENSION cartodb UPDATE TO '0.2.0dev';
-```
-
-Starting with 0.2.0, the in-place reload can be done with an ad-hoc function:
-
-```sql
-SELECT cartodb.cdb_extension_reload();
-```
-
+every commit, so testing latest change requires special steps documented
+in the CONTRIBUTING document, under "Testing changes live".

cartodb.control.in

@@ -3,4 +3,4 @@ comment = 'Turn a database into a cartodb user database.'
 superuser = true
 relocatable = false
 schema = cartodb
-requires = 'schema_triggers, postgis, plpythonu'
+requires = 'plpythonu, schema_triggers, postgis'

scripts-available/CDB_CartodbfyTable.sql

@@ -25,7 +25,7 @@ $$ LANGUAGE plpgsql VOLATILE;
 
 -- Ensure a table is a "cartodb" table
 -- See https://github.com/CartoDB/cartodb/wiki/CartoDB-user-table
-CREATE OR REPLACE FUNCTION CDB_CartodbfyTable(reloid REGCLASS)
+CREATE OR REPLACE FUNCTION CDB_CartodbfyTable(schema_name TEXT, reloid REGCLASS)
 RETURNS void 
 AS $$
 DECLARE
@@ -40,9 +40,14 @@ DECLARE
   exists_geom_cols BOOLEAN[];
 BEGIN
 
+  IF cartodb.schema_exists(schema_name) = false THEN
+    RAISE EXCEPTION 'Invalid schema name "%"', schema_name;
+  END IF;
+
   -- TODO: Check that user quota is set ?
   BEGIN
-    PERFORM public._CDB_UserQuotaInBytes();
+  -- Content will be discarded
+    EXECUTE FORMAT('SELECT %I._CDB_UserQuotaInBytes();', schema_name::text) INTO sql;
   EXCEPTION WHEN undefined_function THEN
     RAISE EXCEPTION 'Please set user quota before cartodbfying tables.';
   END;
@@ -111,7 +116,7 @@ BEGIN
                         AND a.attrelid = reloid
                         AND NOT a.attisdropped
                         AND a.attname = 'cartodb_id'
-                        AND c.contype = 'u' ) -- unique
+                        AND c.contype IN ( 'u', 'p' ) ) -- unique or pkey 
         THEN
           sql := sql || ', ADD unique(cartodb_id)';
         END IF;
@@ -450,13 +455,27 @@ cname, rec2.typname;
 
   sql := 'CREATE TRIGGER test_quota BEFORE UPDATE OR INSERT ON '
       || reloid::text
-      || ' EXECUTE PROCEDURE public.CDB_CheckQuota(1)';
+      || ' EXECUTE PROCEDURE public.CDB_CheckQuota(1, ''-1'', '''
+      || schema_name::text
+      || ''')';
   EXECUTE sql;
 
   sql := 'CREATE TRIGGER test_quota_per_row BEFORE UPDATE OR INSERT ON '
       || reloid::text
-      || ' FOR EACH ROW EXECUTE PROCEDURE public.CDB_CheckQuota(0.001)';
+      || ' FOR EACH ROW EXECUTE PROCEDURE public.CDB_CheckQuota(0.001, ''-1'', '''
+      || schema_name::text
+      || ''')';
   EXECUTE sql;
  
 END;
 $$ LANGUAGE PLPGSQL;
+
+
+CREATE OR REPLACE FUNCTION CDB_CartodbfyTable(reloid REGCLASS)
+RETURNS void
+AS $$
+BEGIN
+  PERFORM public.CDB_CartodbfyTable('public', reloid);
+END;
+$$
+LANGUAGE PLPGSQL;

scripts-available/CDB_DateToNumber.sql

@@ -2,7 +2,7 @@
 --
 CREATE OR REPLACE FUNCTION CDB_DateToNumber(input timestamp)
 RETURNS double precision AS $$
-DECLARE output double precision DEFAULT NULL;
+DECLARE output double precision;
 BEGIN
     BEGIN
         SELECT extract (EPOCH FROM input) INTO output;

scripts-available/CDB_ExtensionUtils.sql

@@ -11,3 +11,10 @@ BEGIN
   EXECUTE sql;
 END;
 $$ language 'plpgsql' VOLATILE;
+
+CREATE OR REPLACE FUNCTION cartodb.schema_exists(schema_name text)
+RETURNS boolean AS
+$$
+  SELECT EXISTS(SELECT 1 FROM pg_namespace WHERE nspname = schema_name::text);
+$$
+language sql VOLATILE;

scripts-available/CDB_Organizations.sql

@@ -0,0 +1,90 @@
+CREATE OR REPLACE
+FUNCTION cartodb.CDB_Organization_Member_Group_Role_Member_Name()
+    RETURNS TEXT
+AS 'SELECT ''cdb_org_member''::text || ''_'' || md5(current_database());'
+LANGUAGE SQL IMMUTABLE;
+
+DO LANGUAGE 'plpgsql' $$
+DECLARE
+    cdb_org_member_role_name TEXT;
+BEGIN
+    cdb_org_member_role_name := cartodb.CDB_Organization_Member_Group_Role_Member_Name();
+  IF NOT EXISTS ( SELECT * FROM pg_roles WHERE rolname= cdb_org_member_role_name )
+  THEN
+    EXECUTE 'CREATE ROLE "' || cdb_org_member_role_name || '" NOLOGIN;';
+  END IF;
+END
+$$;
+
+CREATE OR REPLACE
+FUNCTION cartodb.CDB_Organization_Create_Member(role_name text)
+    RETURNS void
+AS $$
+BEGIN
+    EXECUTE 'GRANT "' || cartodb.CDB_Organization_Member_Group_Role_Member_Name() || '" TO "' || role_name || '"';
+END
+$$ LANGUAGE PLPGSQL VOLATILE;
+
+
+-------------------------------------------------------------------------------
+-- Sharing tables
+-------------------------------------------------------------------------------
+CREATE OR REPLACE
+FUNCTION cartodb.CDB_Organization_Add_Table_Read_Permission(from_schema text, table_name text, to_role_name text)
+    RETURNS void
+AS $$
+BEGIN
+    EXECUTE 'GRANT USAGE ON SCHEMA "' || from_schema || '" TO "' || to_role_name || '"';
+    EXECUTE 'GRANT SELECT ON "' || from_schema || '".' || table_name || ' TO "' || to_role_name || '"';
+END
+$$ LANGUAGE PLPGSQL VOLATILE;
+
+CREATE OR REPLACE
+FUNCTION cartodb.CDB_Organization_Add_Table_Organization_Read_Permission(from_schema text, table_name text)
+    RETURNS void
+AS $$
+BEGIN
+    EXECUTE 'SELECT cartodb.CDB_Organization_Add_Table_Read_Permission(''' || from_schema || ''', ''' || table_name || ''', ''' || cartodb.CDB_Organization_Member_Group_Role_Member_Name() || ''');';
+END
+$$ LANGUAGE PLPGSQL VOLATILE;
+
+CREATE OR REPLACE
+FUNCTION cartodb.CDB_Organization_Add_Table_Read_Write_Permission(from_schema text, table_name text, to_role_name text)
+    RETURNS void
+AS $$
+BEGIN
+    EXECUTE 'GRANT USAGE ON SCHEMA "' || from_schema || '" TO "' || to_role_name || '"';
+    EXECUTE 'GRANT SELECT, INSERT, UPDATE, DELETE ON "' || from_schema || '".' || table_name || ' TO "' || to_role_name || '"';
+END
+$$ LANGUAGE PLPGSQL VOLATILE;
+
+CREATE OR REPLACE
+FUNCTION cartodb.CDB_Organization_Add_Table_Organization_Read_Write_Permission(from_schema text, table_name text)
+    RETURNS void
+AS $$
+BEGIN
+    EXECUTE 'SELECT cartodb.CDB_Organization_Add_Table_Read_Write_Permission(''' || from_schema || ''', ''' || table_name || ''', ''' || cartodb.CDB_Organization_Member_Group_Role_Member_Name() || ''');';
+END
+$$ LANGUAGE PLPGSQL VOLATILE;
+
+
+CREATE OR REPLACE
+FUNCTION cartodb.CDB_Organization_Remove_Access_Permission(from_schema text, table_name text, to_role_name text)
+    RETURNS void
+AS $$
+BEGIN
+    EXECUTE 'REVOKE ALL PRIVILEGES ON TABLE "' || from_schema || '".' || table_name || ' FROM "' || to_role_name || '"';
+    -- EXECUTE 'REVOKE USAGE ON SCHEMA ' || from_schema || ' FROM "' || to_role_name || '"';
+    -- We need to revoke usage on schema only if we are revoking privileges from the last table where to_role_name has
+    -- any permission granted within the schema from_schema
+END
+$$ LANGUAGE PLPGSQL VOLATILE;
+
+CREATE OR REPLACE
+FUNCTION cartodb.CDB_Organization_Remove_Organization_Access_Permission(from_schema text, table_name text)
+    RETURNS void
+AS $$
+BEGIN
+    EXECUTE 'SELECT cartodb.CDB_Organization_Remove_Access_Permission(''' || from_schema || ''', ''' || table_name || ''', ''' || cartodb.CDB_Organization_Member_Group_Role_Member_Name() || ''');';
+END
+$$ LANGUAGE PLPGSQL VOLATILE;

scripts-available/CDB_QueryTables.sql

@@ -22,7 +22,7 @@ BEGIN
     END IF;
 
     BEGIN
-      EXECUTE 'EXPLAIN (FORMAT XML) ' || rec.q INTO STRICT exp;
+      EXECUTE 'EXPLAIN (FORMAT XML, VERBOSE) ' || rec.q INTO STRICT exp;
     EXCEPTION WHEN others THEN
       -- TODO: if error is 'relation "xxxxxx" does not exist', take xxxxxx as
       --       the affected table ?
@@ -33,14 +33,19 @@ BEGIN
 
     -- Now need to extract all values of <Relation-Name>
 
-    --RAISE DEBUG 'Explain: %', exp;
+    -- RAISE DEBUG 'Explain: %', exp;
 
     FOR rec2 IN WITH
-      inp AS ( SELECT xpath('//x:Relation-Name/text()', exp, ARRAY[ARRAY['x', 'http://www.postgresql.org/2009/explain']]) as x )
-      SELECT unnest(x)::name as p from inp
+      inp AS (
+        SELECT
+          xpath('//x:Relation-Name/text()', exp, ARRAY[ARRAY['x', 'http://www.postgresql.org/2009/explain']]) as x,
+          xpath('//x:Schema/text()', exp, ARRAY[ARRAY['x', 'http://www.postgresql.org/2009/explain']]) as s
+      )
+      SELECT unnest(x)::name as p, unnest(s)::name as sc from inp
     LOOP
-      --RAISE DEBUG 'tab: %', rec2.p;
-      tables := array_append(tables, rec2.p);
+      -- RAISE DEBUG 'tab: %', rec2.p;
+      -- RAISE DEBUG 'sc: %', rec2.sc;
+      tables := array_append(tables, (rec2.sc || '.' || rec2.p)::name);
     END LOOP;
 
     -- RAISE DEBUG 'Tables: %', tables;

scripts-available/CDB_Quota.sql

@@ -1,5 +1,5 @@
 -- Return the estimated size of user data. Used for quota checking.
-CREATE OR REPLACE FUNCTION CDB_UserDataSize()
+CREATE OR REPLACE FUNCTION CDB_UserDataSize(schema_name TEXT)
 RETURNS bigint AS
 $$
   -- TODO: double check this query. Maybe use CDB_TableMetadata for lookup ?
@@ -7,55 +7,71 @@ $$
   --
   -- NOTE: division by 2 is an hack for the_geom_webmercator
   --
-  SELECT coalesce(int8(sum(pg_total_relation_size(quote_ident(table_name))) / 2), 0)
+  SELECT coalesce(int8(sum(pg_total_relation_size(schema_name || '.' || table_name)) / 2), 0)
     AS quota
   FROM information_schema.tables
-  WHERE table_catalog = current_database() AND table_schema = 'public'
-    AND table_name != 'spatial_ref_sys'
-    AND table_name != 'cdb_tablemetadata'
-    AND table_type = 'BASE TABLE';
+  WHERE table_catalog = current_database() AND table_schema = schema_name
+        AND table_name != 'spatial_ref_sys'
+        AND table_name != 'cdb_tablemetadata'
+        AND table_type = 'BASE TABLE';
 $$
 LANGUAGE 'sql' VOLATILE;
 
+
+-- Return the estimated size of user data. Used for quota checking.
+-- Implicit schema version for backwards compatibility
+CREATE OR REPLACE FUNCTION CDB_UserDataSize()
+RETURNS bigint AS
+$$
+  SELECT public.CDB_UserDataSize('public');
+$$
+LANGUAGE 'sql' VOLATILE;
+
+-- Triggers cannot have declared arguments: pbfact float8, qmax int8, schema_name text
 CREATE OR REPLACE FUNCTION CDB_CheckQuota()
 RETURNS trigger AS
 $$
 DECLARE
-
   pbfact float8;
   qmax int8;
+  schema_name text;
   dice float8;
   quota float8;
 BEGIN
-
+  IF TG_NARGS = 3 THEN
+    schema_name := TG_ARGV[2];
+    IF cartodb.schema_exists(schema_name) = false THEN
+      RAISE EXCEPTION 'Invalid schema name "%"', schema_name;
+    END IF;
+  ELSE
+    schema_name := 'public';
+  END IF;
+  -- Hack to support old versions of CDB_CheckQuota with 2 params but without schema_name
+  IF TG_NARGS >= 2 AND TG_ARGV[1] <> '-1' THEN
+    qmax := TG_ARGV[1];
+  ELSE
+    BEGIN
+      EXECUTE FORMAT('SELECT %I._CDB_UserQuotaInBytes();', schema_name) INTO qmax;
+      EXCEPTION WHEN undefined_function THEN
+      RAISE EXCEPTION 'Missing "%"._CDB_UserQuotaInBytes()', schema_name;
+    END;
+  END IF;
   pbfact := TG_ARGV[0];
-  dice := random();
 
-  -- RAISE DEBUG 'CDB_CheckQuota enter: pbfact=% dice=%', pbfact, dice;
+  dice := random();
 
   IF dice < pbfact THEN
     RAISE DEBUG 'Checking quota on table % (dice:%, needed:<%)', TG_RELID::text, dice, pbfact;
-    BEGIN
-      qmax := public._CDB_UserQuotaInBytes();
-    EXCEPTION WHEN undefined_function THEN
-      IF TG_NARGS > 1 THEN
-        RAISE NOTICE 'Using quota specified via trigger parameter';
-        qmax := TG_ARGV[1];
-      ELSE
-        RAISE EXCEPTION 'Missing _CDB_UserQuotaInBytes(), and no quota provided as parameter';
-      END IF;
-    END;
 
     IF qmax = 0 THEN
       RETURN NEW;
     END IF;
 
-    SELECT public.CDB_UserDataSize() INTO quota;
+    SELECT public.CDB_UserDataSize(schema_name) INTO quota;
     IF quota > qmax THEN
-        RAISE EXCEPTION 'Quota exceeded by %KB', (quota-qmax)/1024;
+      RAISE EXCEPTION 'Quota exceeded by %KB', (quota-qmax)/1024;
     ELSE RAISE DEBUG 'User quota in bytes: % < % (max allowed)', quota, qmax;
     END IF;
-  -- ELSE RAISE DEBUG 'Not checking quota on table % (dice:%, needed:<%)', TG_RELID::text, dice, pbfact;
   END IF;
 
   RETURN NEW;
@@ -63,26 +79,33 @@ END;
 $$
 LANGUAGE 'plpgsql' VOLATILE;
 
-CREATE OR REPLACE FUNCTION CDB_SetUserQuotaInBytes(bytes int8)
+
+CREATE OR REPLACE FUNCTION CDB_SetUserQuotaInBytes(schema_name text, bytes int8)
 RETURNS int8 AS
 $$
 DECLARE
-  current_quota int8;
   sql text;
 BEGIN
-  BEGIN
-    current_quota := public._CDB_UserQuotaInBytes();
-  EXCEPTION WHEN undefined_function THEN
-    current_quota := 0;
-  END;
+  IF cartodb.schema_exists(schema_name::text) = false THEN
+    RAISE EXCEPTION 'Invalid schema name "%"', schema_name::text;
+  END IF;
 
-  sql := 'CREATE OR REPLACE FUNCTION public._CDB_UserQuotaInBytes() '
+  sql := 'CREATE OR REPLACE FUNCTION "' || schema_name::text || '"._CDB_UserQuotaInBytes() '
     || 'RETURNS int8 AS $X$ SELECT ' || bytes
     || '::int8 $X$ LANGUAGE sql IMMUTABLE';
   EXECUTE sql;
 
-  return current_quota;
-
+  return bytes;
 END
 $$
 LANGUAGE 'plpgsql' VOLATILE STRICT;
+
+
+CREATE OR REPLACE FUNCTION CDB_SetUserQuotaInBytes(bytes int8)
+RETURNS int8 AS
+$$
+BEGIN
+  return public.CDB_SetUserQuotaInBytes('public', bytes);
+END;
+$$
+LANGUAGE 'plpgsql' VOLATILE STRICT;

scripts-available/CDB_StringToDate.sql

@@ -2,7 +2,7 @@
 --
 CREATE OR REPLACE FUNCTION CDB_StringToDate(input character varying)
 RETURNS date AS $$
-DECLARE output DATE DEFAULT NULL;
+DECLARE output DATE;
 BEGIN
     BEGIN
         output := input::date;

scripts-enabled/210-CDB_Organizations.sql

@@ -0,0 +1 @@
+../scripts-available/CDB_Organizations.sql

test/CDB_CartodbfyTableTest.sql

@@ -30,7 +30,7 @@ BEGIN
     END IF;
   END LOOP;
 
-  PERFORM CDB_CartodbfyTable(tabname); 
+  PERFORM CDB_CartodbfyTable('public', tabname);
 
   sql := 'INSERT INTO ' || tabname::text || '(the_geom) values ( CDB_LatLng(2,1) ) RETURNING cartodb_id';
   EXECUTE sql INTO STRICT id;
@@ -132,7 +132,7 @@ LANGUAGE 'plpgsql';
 
 -- table with single non-geometrical column
 CREATE TABLE t AS SELECT 1::int as a;
-SELECT CDB_CartodbfyTable('t'); -- should fail
+SELECT CDB_CartodbfyTable('public', 't'); -- should fail
 SELECT CDB_SetUserQuotaInBytes(0); -- Set user quota to infinite
 SELECT CDB_CartodbfyTableCheck('t', 'single non-geometrical column');
 DROP TABLE t;
@@ -200,6 +200,14 @@ SELECT CDB_CartodbfyTableCheck('t', 'unsequenced cartodb_id');
 select cartodb_id FROM t; 
 DROP TABLE t;
 
+-- table with existing cartodb_id serial primary key
+CREATE TABLE t ( cartodb_id serial primary key );
+SELECT CDB_CartodbfyTableCheck('t', 'cartodb_id serial primary key');
+SELECT c.conname, a.attname FROM pg_constraint c, pg_attribute a
+WHERE c.conrelid = 't'::regclass and a.attrelid = c.conrelid
+AND c.conkey[1] = a.attnum AND NOT a.attisdropped;
+DROP TABLE t;
+
 -- table with existing the_geom and created_at and containing null values
 -- Really, a test for surviving an longstanding PostgreSQL bug:
 -- http://www.postgresql.org/message-id/20140530143150.GA11051@localhost

test/CDB_CartodbfyTableTest_expect

@@ -45,6 +45,10 @@ unsequenced cartodb_id cartodbfied fine
 1
 DROP TABLE
 CREATE TABLE
+cartodb_id serial primary key cartodbfied fine
+t_pkey|cartodb_id
+DROP TABLE
+CREATE TABLE
 null geom and timestamp values cartodbfied fine
 DROP TABLE
 DROP FUNCTION

test/CDB_QueryTablesTest.sql

@@ -25,3 +25,9 @@ WITH inp AS ( select 'create table test (a int); insert into test values (1); se
 
 WITH inp AS ( select 'WITH a AS (select * from pg_class) select * from a'::text as q )
  SELECT q, CDB_QueryTables(q) from inp;
+
+CREATE SCHEMA sc;
+create table sc.test (a int);
+insert into sc.test values (1);
+WITH inp AS ( select 'select * from sc.test'::text as q )
+ SELECT q, CDB_QueryTables(q) from inp;

test/CDB_QueryTablesTest_expect

@@ -1,11 +1,15 @@
-SELECT * FROM geometry_columns|{pg_attribute,pg_class,pg_namespace,pg_type}
-SELECT a.attname FROM pg_class c JOIN pg_attribute a on (a.attrelid = c.oid)|{pg_attribute,pg_class}
+SELECT * FROM geometry_columns|{pg_catalog.pg_attribute,pg_catalog.pg_class,pg_catalog.pg_namespace,pg_catalog.pg_type}
+SELECT a.attname FROM pg_class c JOIN pg_attribute a on (a.attrelid = c.oid)|{pg_catalog.pg_attribute,pg_catalog.pg_class}
 CREATE table "my'tab;le" as select 1|{}
-SELECT a.oid, b.oid FROM pg_class a, pg_class b|{pg_class}
+SELECT a.oid, b.oid FROM pg_class a, pg_class b|{pg_catalog.pg_class}
 SELECT 1 as col1; select 2 as col2|{}
 WARNING:  CDB_QueryTables cannot explain query: select 1 from nonexistant (42P01: relation "nonexistant" does not exist)
 ERROR:  relation "nonexistant" does not exist
-begin; select * from pg_class; commit;|{pg_class}
+begin; select * from pg_class; commit;|{pg_catalog.pg_class}
 WARNING:  CDB_QueryTables cannot explain query: select * from test (42P01: relation "test" does not exist)
 ERROR:  relation "test" does not exist
-WITH a AS (select * from pg_class) select * from a|{pg_class}
+WITH a AS (select * from pg_class) select * from a|{pg_catalog.pg_class}
+CREATE SCHEMA
+CREATE TABLE
+INSERT 0 1
+select * from sc.test|{sc.test}

test/CDB_QuotaTest.sql

@@ -4,7 +4,7 @@ CREATE TABLE big(a int);
 -- Try the legacy interface
 -- See https://github.com/CartoDB/cartodb-postgresql/issues/13
 CREATE TRIGGER test_quota BEFORE UPDATE OR INSERT ON big
-      EXECUTE PROCEDURE CDB_CheckQuota(1,1);
+      EXECUTE PROCEDURE CDB_CheckQuota(1, 1, 'public');
 INSERT INTO big VALUES (1); -- allowed, check runs before
 INSERT INTO big VALUES (1); -- disallowed, quota exceeds before
 SELECT CDB_SetUserQuotaInBytes(0);

test/CDB_QuotaTest_expect

@@ -6,9 +6,9 @@ ERROR:  Quota exceeded by 3.9990234375KB
 0
 
 INSERT 0 1024
-0
-ERROR:  Quota exceeded by 103.9921875KB
 8
+ERROR:  Quota exceeded by 103.9921875KB
+0
 INSERT 0 1
 DROP TABLE
 SET

test/organization/test.sh

@@ -0,0 +1,334 @@
+#!/bin/sh
+
+#
+# It is expected that you run this script
+# as a PostgreSQL superuser, for example:
+#
+#   PGUSER=postgres bash ./test.sh
+#
+
+DATABASE=test_organizations
+CMD='echo psql'
+CMD=psql
+
+OK=0
+PARTIALOK=0
+
+function set_failed() {
+    OK=1
+    PARTIALOK=1
+}
+
+
+function clear_partial_result() {
+    PARTIALOK=0
+}
+
+
+function sql() {
+    local ROLE
+    local QUERY
+    if [[ $# -ge 2 ]]
+    then
+        ROLE="$1"
+        QUERY="$2"
+    else
+        QUERY="$1"
+    fi
+
+    if [ -n "${ROLE}" ]; then
+      log_debug "Executing query '${QUERY}' as ${ROLE}"
+      RESULT=`${CMD} -U "${ROLE}" ${DATABASE} -c "${QUERY}" -A -t`
+    else
+      log_debug "Executing query '${QUERY}'"
+      RESULT=`${CMD} ${DATABASE} -c "${QUERY}" -A -t`
+    fi
+    CODERESULT=$?
+
+    echo ${RESULT}
+    echo
+
+    if [[ ${CODERESULT} -ne 0 ]]
+    then
+        echo -n "FAILED TO EXECUTE QUERY: "
+        log_warning "${QUERY}"
+        if [[ "$3" != "fails" ]]
+        then
+            log_error "${QUERY}"
+            set_failed
+        fi
+    else
+        if [[ "$3" == "fails" ]]
+        then
+            log_error "QUERY: '${QUERY}' was expected to fail and it did not fail"
+            set_failed
+        fi
+    fi
+
+    if [[ "$3" == "should" ]]
+    then
+        if [[ "${RESULT}" != "$4" ]]
+        then
+            log_error "QUERY '${QUERY}' expected result '${4}' but got '${RESULT}'"
+            set_failed
+        fi
+    fi
+}
+
+
+function log_info()
+{
+    echo
+    echo
+    echo
+    _log "1;34m" "$1"
+}
+
+function log_error() {
+    _log "1;31m" "$1"
+}
+
+function log_debug() {
+    _log "1;32m" "> $1"
+}
+
+function log_warning() {
+    _log "0;33m" "$1"
+}
+
+function _log() {
+    echo -e "\033[$1$2\033[0m"
+}
+
+# '############################ HELPERS #############################'
+function create_role_and_schema() {
+    local ROLE=$1
+    sql "CREATE ROLE ${ROLE} LOGIN;"
+    sql "GRANT CONNECT ON DATABASE \"${DATABASE}\" TO ${ROLE};"
+    sql "CREATE SCHEMA ${ROLE} AUTHORIZATION ${ROLE};"
+    sql "SELECT cartodb.CDB_Organization_Create_Member('${ROLE}')"
+}
+
+
+function drop_role_and_schema() {
+    local ROLE=$1
+    sql "DROP SCHEMA \"${ROLE}\";"
+    sql "REVOKE CONNECT ON DATABASE \"${DATABASE}\" FROM \"${ROLE}\";"
+    sql "DROP ROLE \"${ROLE}\";"
+}
+
+
+function create_table() {
+    if [[ $# -ne 2 ]]
+    then
+        log_error "create_table requires two arguments: role and table_name"
+        exit 1
+    fi
+    local ROLE="$1"
+    local TABLENAME="$2"
+    sql ${ROLE} "CREATE TABLE ${ROLE}.${TABLENAME} ( a int );"
+}
+
+
+function setup() {
+    ${CMD} -c "CREATE DATABASE ${DATABASE}"
+    sql "CREATE SCHEMA cartodb;"
+    sql "GRANT USAGE ON SCHEMA cartodb TO public;"
+
+    log_info "########################### BOOTSTRAP ###########################"
+    ${CMD} -d ${DATABASE} -f scripts-available/CDB_Organizations.sql
+
+
+    log_info "############################# SETUP #############################"
+    create_role_and_schema cdb_testmember_1
+    create_role_and_schema cdb_testmember_2
+
+    create_table cdb_testmember_1 foo
+    sql cdb_testmember_1 'INSERT INTO cdb_testmember_1.foo VALUES (1), (2), (3), (4), (5);'
+    sql cdb_testmember_1 'SELECT * FROM cdb_testmember_1.foo;'
+
+    create_table cdb_testmember_2 bar
+    sql cdb_testmember_2 'INSERT INTO bar VALUES (1), (2), (3), (4), (5);'
+    sql cdb_testmember_2 'SELECT * FROM cdb_testmember_2.bar;'
+}
+
+function tear_down() {
+    log_info "########################### USER TEAR DOWN ###########################"
+    sql cdb_testmember_1 "SELECT * FROM cartodb.CDB_Organization_Remove_Access_Permission('cdb_testmember_1', 'foo', 'cdb_testmember_2');"
+    sql cdb_testmember_2 "SELECT * FROM cartodb.CDB_Organization_Remove_Access_Permission('cdb_testmember_2', 'bar', 'cdb_testmember_1');"
+
+    sql cdb_testmember_1 'DROP TABLE cdb_testmember_1.foo;'
+    sql cdb_testmember_2 'DROP TABLE cdb_testmember_2.bar;'
+
+    sql "DROP SCHEMA cartodb CASCADE"
+
+    log_info "########################### TEAR DOWN ###########################"
+    sql 'DROP SCHEMA cdb_testmember_1;'
+    sql 'DROP SCHEMA cdb_testmember_2;'
+
+    sql "REVOKE CONNECT ON DATABASE \"${DATABASE}\" FROM cdb_testmember_1;"
+    sql "REVOKE CONNECT ON DATABASE \"${DATABASE}\" FROM cdb_testmember_2;"
+
+    sql 'DROP ROLE cdb_testmember_1;'
+    sql 'DROP ROLE cdb_testmember_2;'
+
+    ${CMD} -c "DROP DATABASE ${DATABASE}"
+}
+
+function run_tests() {
+    local FAILED_TESTS=()
+
+    local TESTS
+    if [[ $# -ge 1 ]]
+    then
+        TESTS="$@"
+    else
+        TESTS=`cat $0 | perl -n -e'/function (test.*)\(\)/ && print "$1\n"'`
+    fi
+    for t in ${TESTS}
+    do
+        echo "####################################################################"
+        echo "#"
+        echo "# Running: ${t}"
+        echo "#"
+        echo "####################################################################"
+        clear_partial_result
+        setup
+        eval ${t}
+        if [[ ${PARTIALOK} -ne 0 ]]
+        then
+            FAILED_TESTS+=(${t})
+        fi
+        tear_down
+    done
+    if [[ ${OK} -ne 0 ]]
+    then
+        echo
+        log_error "The following tests are failing:"
+        printf -- '\t%s\n' "${FAILED_TESTS[@]}"
+    fi
+}
+
+
+
+#################################################### TESTS GO HERE ####################################################
+
+function test_member_2_cannot_read_without_permission() {
+    sql cdb_testmember_2 'SELECT count(*) FROM cdb_testmember_1.foo;' fails
+}
+
+function test_member_1_cannot_grant_read_permission_to_other_schema_than_its_one() {
+    sql cdb_testmember_1 "SELECT * FROM cartodb.CDB_Organization_Add_Table_Read_Permission('cdb_testmember_2', 'foo', 'cdb_testmember_2')" fails
+}
+
+function test_member_1_grants_read_permission_and_member_2_can_read() {
+    sql cdb_testmember_1 "SELECT * FROM cartodb.CDB_Organization_Add_Table_Read_Permission('cdb_testmember_1', 'foo', 'cdb_testmember_2')"
+    sql cdb_testmember_2 'SELECT count(*) FROM cdb_testmember_1.foo;' should 5
+    sql cdb_testmember_1 'SELECT count(*) FROM cdb_testmember_2.bar;' fails
+}
+
+function test_member_2_cannot_add_table_to_member_1_schema_after_table_permission_added() {
+    sql cdb_testmember_1 "SELECT * FROM cartodb.CDB_Organization_Add_Table_Read_Permission('cdb_testmember_1', 'foo', 'cdb_testmember_2')"
+    sql cdb_testmember_2 "CREATE TABLE cdb_testmember_1.bar ( a int );" fails
+}
+
+function test_grant_read_permission_between_two_members() {
+    sql cdb_testmember_1 "SELECT * FROM cartodb.CDB_Organization_Add_Table_Read_Permission('cdb_testmember_1', 'foo', 'cdb_testmember_2')"
+    sql cdb_testmember_2 "SELECT * FROM cartodb.CDB_Organization_Add_Table_Read_Permission('cdb_testmember_2', 'bar', 'cdb_testmember_1')"
+    sql cdb_testmember_2 'SELECT count(*) FROM cdb_testmember_1.foo;' should 5
+    sql cdb_testmember_1 'SELECT count(*) FROM cdb_testmember_2.bar;' should 5
+}
+
+function test_member_2_cannot_write_to_member_1_table() {
+    sql cdb_testmember_2 'INSERT INTO cdb_testmember_1.foo VALUES (5), (6), (7), (8), (9);' fails
+}
+
+function test_member_1_cannot_grant_read_write_permission_to_other_schema_than_its_one() {
+    sql cdb_testmember_1 "SELECT * FROM cartodb.CDB_Organization_Add_Table_Read_Write_Permission('cdb_testmember_2', 'foo', 'cdb_testmember_2')" fails
+}
+
+function test_member_2_can_write_to_member_1_table_after_write_permission_is_added() {
+    sql cdb_testmember_1 "SELECT * FROM cartodb.CDB_Organization_Add_Table_Read_Write_Permission('cdb_testmember_1', 'foo', 'cdb_testmember_2')"
+    sql cdb_testmember_2 'INSERT INTO cdb_testmember_1.foo VALUES (5), (6), (7), (8), (9);'
+    sql cdb_testmember_1 'SELECT count(*) FROM cdb_testmember_1.foo;' should 10
+    sql cdb_testmember_2 'SELECT count(*) FROM cdb_testmember_1.foo;' should 10
+    sql cdb_testmember_2 'DELETE FROM cdb_testmember_1.foo where a = 9;'
+    sql cdb_testmember_1 'SELECT count(*) FROM cdb_testmember_1.foo;' should 9
+    sql cdb_testmember_2 'SELECT count(*) FROM cdb_testmember_1.foo;' should 9
+}
+
+function test_member_1_removes_access_and_member_2_can_no_longer_query_the_table() {
+    sql cdb_testmember_1 "SELECT * FROM cartodb.CDB_Organization_Add_Table_Read_Permission('cdb_testmember_1', 'foo', 'cdb_testmember_2')"
+    sql cdb_testmember_2 'SELECT count(*) FROM cdb_testmember_1.foo;' should 5
+    sql cdb_testmember_1 "SELECT * FROM cartodb.CDB_Organization_Remove_Access_Permission('cdb_testmember_1', 'foo', 'cdb_testmember_2')"
+    sql cdb_testmember_2 'SELECT * FROM cdb_testmember_1.foo;' fails
+}
+
+function test_member_1_removes_access_and_member_2_can_no_longer_write_to_the_table() {
+    sql cdb_testmember_1 "SELECT * FROM cartodb.CDB_Organization_Add_Table_Read_Write_Permission('cdb_testmember_1', 'foo', 'cdb_testmember_2')"
+    sql cdb_testmember_2 'INSERT INTO cdb_testmember_1.foo VALUES (5), (6), (7), (8), (9);'
+    sql cdb_testmember_1 "SELECT * FROM cartodb.CDB_Organization_Remove_Access_Permission('cdb_testmember_1', 'foo', 'cdb_testmember_2')"
+    sql cdb_testmember_2 'INSERT INTO cdb_testmember_1.foo VALUES (5), (6), (7), (8), (9);' fails
+}
+
+function test_giving_permissions_to_two_tables_and_removing_from_first_table_should_not_remove_from_second() {
+    #### test setup
+    # create an extra table for cdb_testmember_1
+    create_table cdb_testmember_1 foo_2
+    sql cdb_testmember_1 'INSERT INTO cdb_testmember_1.foo_2 VALUES (1), (2), (3), (4), (5);'
+    sql cdb_testmember_1 'SELECT * FROM cdb_testmember_1.foo_2;'
+
+    # gives read permission to both tables
+    sql cdb_testmember_1 "SELECT * FROM cartodb.CDB_Organization_Add_Table_Read_Permission('cdb_testmember_1', 'foo', 'cdb_testmember_2')"
+    sql cdb_testmember_1 "SELECT * FROM cartodb.CDB_Organization_Add_Table_Read_Permission('cdb_testmember_1', 'foo_2', 'cdb_testmember_2')"
+
+    # cdb_testmember_2 has access to both tables
+    sql cdb_testmember_2 'SELECT count(*) FROM cdb_testmember_1.foo;' should 5
+    sql cdb_testmember_2 'SELECT count(*) FROM cdb_testmember_1.foo_2;' should 5
+
+    # cdb_testmember_1 removes access to foo table
+    sql cdb_testmember_1 "SELECT * FROM cartodb.CDB_Organization_Remove_Access_Permission('cdb_testmember_1', 'foo', 'cdb_testmember_2')"
+
+    # cdb_testmember_2 should have access to foo_2 table but not to table foo
+    sql cdb_testmember_2 'SELECT count(*) FROM cdb_testmember_1.foo;' fails
+    sql cdb_testmember_2 'SELECT count(*) FROM cdb_testmember_1.foo_2;' should 5
+
+
+    #### test tear down
+    sql cdb_testmember_1 'DROP TABLE cdb_testmember_1.foo_2;'
+}
+
+function test_cdb_org_member_role_allows_reading_to_all_users_without_explicit_permission() {
+    sql cdb_testmember_2 'SELECT count(*) FROM cdb_testmember_1.foo;' fails
+    sql cdb_testmember_1 "SELECT cartodb.CDB_Organization_Add_Table_Organization_Read_Permission('cdb_testmember_1', 'foo');"
+    sql cdb_testmember_2 'SELECT count(*) FROM cdb_testmember_1.foo;' should 5
+}
+
+function test_user_can_read_when_it_has_permission_after_organization_permission_is_removed() {
+    create_role_and_schema cdb_testmember_3
+
+    # shares with cdb_testmember_2 and can read but cdb_testmember_3 cannot
+    sql cdb_testmember_1 "SELECT * FROM cartodb.CDB_Organization_Add_Table_Read_Permission('cdb_testmember_1', 'foo', 'cdb_testmember_2')"
+    sql cdb_testmember_2 'SELECT count(*) FROM cdb_testmember_1.foo;' should 5
+    sql cdb_testmember_3 'SELECT count(*) FROM cdb_testmember_1.foo;' fails
+
+    # granting to organization allows to read to both: cdb_testmember_2 and cdb_testmember_3
+    sql cdb_testmember_1 "SELECT cartodb.CDB_Organization_Add_Table_Organization_Read_Permission('cdb_testmember_1', 'foo');"
+    sql cdb_testmember_2 'SELECT count(*) FROM cdb_testmember_1.foo;' should 5
+    sql cdb_testmember_3 'SELECT count(*) FROM cdb_testmember_1.foo;' should 5
+
+    # removing access from organization should keep permission on cdb_testmember_2 but drop it to cdb_testmember_3
+    sql cdb_testmember_1 "SELECT cartodb.CDB_Organization_Remove_Organization_Access_Permission('cdb_testmember_1', 'foo');"
+    sql cdb_testmember_2 'SELECT count(*) FROM cdb_testmember_1.foo;' should 5
+    sql cdb_testmember_3 'SELECT count(*) FROM cdb_testmember_1.foo;' fails
+
+    drop_role_and_schema cdb_testmember_3
+}
+
+#################################################### TESTS END HERE ####################################################
+
+
+
+run_tests $@
+
+exit ${OK}

util/create_from_unpackaged.sh

@@ -4,7 +4,22 @@ ver=$1
 input=cartodb--${ver}.sql
 output=cartodb--unpackaged--${ver}.sql
 
-cat ${input} | grep -v 'duplicated extension$' > ${output}
+echo "-- Script generated by $0 on `date`" > ${output}
+
+# Migrate CDB functions from public schema to cartodb schema
+cat ${input} |
+  grep '^ *CREATE OR REPLACE FUNCTION' |
+  grep -v ' cartodb\.' | # should  only match DDL hooks
+  sed 's/).*$/)/' |
+  sed 's/DEFAULT [^ ,)]*//g' |
+  sed 's/CREATE OR REPLACE FUNCTION /ALTER FUNCTION public./' |
+  sed 's/$/ SET SCHEMA cartodb;/' |
+  sed 's/^/DO LANGUAGE plpgsql \$\$ BEGIN /' |
+  sed "s/$/ EXCEPTION WHEN OTHERS THEN RAISE NOTICE 'Got % (%)', SQLERRM, SQLSTATE; END; \$\$;/" |
+  cat >> ${output}
+
+# Upgrade all functions
+cat ${input} | grep -v 'duplicated extension$' >> ${output}
 
 # Migrate CDB_TableMetadata
 cat >> ${output} <<'EOF'
@@ -29,32 +44,34 @@ BEGIN
   DROP FUNCTION public._CDB_UserQuotaInBytes();
 END;
 $$ LANGUAGE 'plpgsql';
-
--- Cartodbfy tables with a trigger using 'CDB_CheckQuota' or
--- 'CDB_TableMetadata_Trigger' from the 'public' schema
-select cartodb.CDB_CartodbfyTable(relname::regclass) from ( 
-  -- names of tables using public.CDB_CheckQuota or
-  -- public.CDB_TableMetadata_Trigger in their triggers
-  SELECT distinct c.relname
-  FROM
-    pg_trigger t,
-    pg_class c,
-    pg_proc p,
-    pg_namespace n
-  WHERE
-    n.nspname = 'public' AND
-    p.pronamespace = n.oid AND
-    p.proname IN ( 'cdb_checkquota', 'cdb_tablemetadata_trigger' ) AND
-    t.tgrelid = c.oid AND
-    p.oid = t.tgfoid 
-) as foo;
 EOF
 
-# Drop functions from public schema
-cat ${input} |
-  grep '^ *CREATE OR REPLACE FUNCTION' |
-  grep -v ' cartodb\.' | # should  only match DDL hooks
-  sed 's/).*$/);/' |
-  sed 's/DEFAULT [^ ,)]*//g' |
-  sed 's/CREATE OR REPLACE FUNCTION /DROP FUNCTION public./' |
-  cat >> ${output}
+## Cartodbfy tables with a trigger using 'CDB_CheckQuota' or
+## 'CDB_TableMetadata_Trigger' from the 'public' schema
+#cat >> ${output} <<'EOF'
+#select cartodb.CDB_CartodbfyTable(relname::regclass) from ( 
+#  -- names of tables using public.CDB_CheckQuota or
+#  -- public.CDB_TableMetadata_Trigger in their triggers
+#  SELECT distinct c.relname
+#  FROM
+#    pg_trigger t,
+#    pg_class c,
+#    pg_proc p,
+#    pg_namespace n
+#  WHERE
+#    n.nspname = 'public' AND
+#    p.pronamespace = n.oid AND
+#    p.proname IN ( 'cdb_checkquota', 'cdb_tablemetadata_trigger' ) AND
+#    t.tgrelid = c.oid AND
+#    p.oid = t.tgfoid 
+#) as foo;
+#EOF
+
+## Drop any leftover function from public schema (there should be none)
+#cat ${input} |
+#  grep '^ *CREATE OR REPLACE FUNCTION' |
+#  grep -v ' cartodb\.' | # should  only match DDL hooks
+#  sed 's/).*$/);/' |
+#  sed 's/DEFAULT [^ ,)]*//g' |
+#  sed 's/CREATE OR REPLACE FUNCTION /DROP FUNCTION IF EXISTS public./' |
+#  cat >> ${output}