From 00821385036b08c43c1eab15032c8dd540ebba0f Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Tue, 20 May 2014 16:37:36 +0200 Subject: [PATCH 01/54] Do not force DROP FUNCTION but rely on CREATE OR REPLACE We'll bet on no old signature being left around. Presence of an old signature would break at runtime with a message like: ERROR: function CDB_UserTables() is not unique Closes #25 --- scripts-available/CDB_Hexagon.sql | 2 +- scripts-available/CDB_UserTables.sql | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts-available/CDB_Hexagon.sql b/scripts-available/CDB_Hexagon.sql index 131aa12..7b57a98 100644 --- a/scripts-available/CDB_Hexagon.sql +++ b/scripts-available/CDB_Hexagon.sql @@ -27,7 +27,7 @@ $$ LANGUAGE 'sql' IMMUTABLE STRICT; -- as the extent. -- -- -DROP FUNCTION IF EXISTS CDB_HexagonGrid(ext GEOMETRY, side FLOAT8); +-- DROP FUNCTION IF EXISTS CDB_HexagonGrid(ext GEOMETRY, side FLOAT8); CREATE OR REPLACE FUNCTION CDB_HexagonGrid(ext GEOMETRY, side FLOAT8, origin GEOMETRY DEFAULT NULL) RETURNS SETOF GEOMETRY AS $$ diff --git a/scripts-available/CDB_UserTables.sql b/scripts-available/CDB_UserTables.sql index e8654ca..c2fa4ee 100644 --- a/scripts-available/CDB_UserTables.sql +++ b/scripts-available/CDB_UserTables.sql @@ -5,7 +5,7 @@ -- -- Currently accepted permissions are: 'public', 'private' or 'all' -- -DROP FUNCTION IF EXISTS CDB_UserTables(); -- replaced by: +-- DROP FUNCTION IF EXISTS CDB_UserTables(); -- replaced by: CREATE OR REPLACE FUNCTION CDB_UserTables(perm text DEFAULT 'all') RETURNS SETOF information_schema.sql_identifier AS $$ From 8a8d198ea293cfd0d60d53ee07ecb39c115592a4 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Thu, 22 May 2014 11:40:01 +0200 Subject: [PATCH 02/54] Do not enable DDL triggers on extension create Use cartodb.cdb_enable_ddl_hooks() to enable them. Closes #19 --- cartodb_hooks.sql | 3 ++- expected/test_ddl_triggers.out | 12 ++++++++++++ sql/test_ddl_triggers.sql | 3 +++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/cartodb_hooks.sql b/cartodb_hooks.sql index 0950ec2..5c58844 100644 --- a/cartodb_hooks.sql +++ b/cartodb_hooks.sql @@ -184,4 +184,5 @@ CREATE OR REPLACE FUNCTION cartodb.cdb_enable_ddl_hooks() returns void AS $$ ON "column_add" EXECUTE PROCEDURE cartodb.cdb_handle_add_column(); $$ LANGUAGE sql; -SELECT cartodb.cdb_enable_ddl_hooks(); +-- Do not enable hooks by default +--SELECT cartodb.cdb_enable_ddl_hooks(); diff --git a/expected/test_ddl_triggers.out b/expected/test_ddl_triggers.out index fbbbbb4..cf0978c 100644 --- a/expected/test_ddl_triggers.out +++ b/expected/test_ddl_triggers.out @@ -6,6 +6,18 @@ SELECT CDB_SetUserQuotaInBytes(0); 0 (1 row) +-- Enable ddl triggers +SELECT cartodb.cdb_enable_ddl_hooks(); +NOTICE: event trigger "cdb_on_relation_create" does not exist, skipping +NOTICE: event trigger "cdb_on_relation_drop" does not exist, skipping +NOTICE: event trigger "cdb_on_alter_column" does not exist, skipping +NOTICE: event trigger "cdb_on_drop_column" does not exist, skipping +NOTICE: event trigger "cdb_on_add_column" does not exist, skipping + cdb_enable_ddl_hooks +---------------------- + +(1 row) + create schema c; CREATE USER cartodb_postgresql_unpriv_user; GRANT ALL ON SCHEMA c to cartodb_postgresql_unpriv_user; diff --git a/sql/test_ddl_triggers.sql b/sql/test_ddl_triggers.sql index 77fc23b..6972673 100644 --- a/sql/test_ddl_triggers.sql +++ b/sql/test_ddl_triggers.sql @@ -3,6 +3,9 @@ -- Set user quota to infinite SELECT CDB_SetUserQuotaInBytes(0); +-- Enable ddl triggers +SELECT cartodb.cdb_enable_ddl_hooks(); + create schema c; CREATE USER cartodb_postgresql_unpriv_user; From 75a40c9b636260f9fff837f30200091cf7282c1f Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Thu, 22 May 2014 12:32:52 +0200 Subject: [PATCH 03/54] Tag CDB_TableMetadata as a configuration table Closes #21 --- Makefile | 3 ++- cartodb_config_dump.sql | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) create mode 100644 cartodb_config_dump.sql diff --git a/Makefile b/Makefile index 45ae5da..195840d 100644 --- a/Makefile +++ b/Makefile @@ -25,7 +25,7 @@ PG_CONFIG = pg_config PGXS := $(shell $(PG_CONFIG) --pgxs) include $(PGXS) -$(EXTENSION)--$(EXTVERSION).sql: $(CDBSCRIPTS) cartodb_hooks.sql cartodb_version.sql Makefile +$(EXTENSION)--$(EXTVERSION).sql: $(CDBSCRIPTS) cartodb_hooks.sql cartodb_version.sql cartodb_config_dump.sql Makefile echo '\echo Use "CREATE EXTENSION $(EXTENSION)" to load this file. \quit' > $@ cat $(CDBSCRIPTS) | \ sed -e 's/\> $@ cat cartodb_hooks.sql >> $@ cat cartodb_version.sql >> $@ + cat cartodb_config_dump.sql >> $@ $(EXTENSION)--unpackaged--$(EXTVERSION).sql: $(EXTENSION)--$(EXTVERSION).sql util/create_from_unpackaged.sh Makefile ./util/create_from_unpackaged.sh $(EXTVERSION) diff --git a/cartodb_config_dump.sql b/cartodb_config_dump.sql new file mode 100644 index 0000000..36aa652 --- /dev/null +++ b/cartodb_config_dump.sql @@ -0,0 +1,2 @@ +SELECT pg_catalog.pg_extension_config_dump('cartodb.cdb_tablemetadata',''); + From 134d2d29b6a039ea5d50bfdcc20650781c221c18 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Thu, 22 May 2014 13:10:42 +0200 Subject: [PATCH 04/54] Try to cast TEXT typed updated_at and created_at if present If the cast fails we'll still rename those fields and create new ones. See #27 (cartodb_id is still not done) --- scripts-available/CDB_CartodbfyTable.sql | 41 ++++++++++++------------ test/CDB_CartodbfyTableTest.sql | 8 +++++ test/CDB_CartodbfyTableTest_expect | 4 +++ 3 files changed, 32 insertions(+), 21 deletions(-) diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql index 97c754c..8f8af8a 100644 --- a/scripts-available/CDB_CartodbfyTable.sql +++ b/scripts-available/CDB_CartodbfyTable.sql @@ -163,27 +163,26 @@ BEGIN IF had_column THEN - -- Check data type is a TIMESTAMP WITH TIMEZONE - SELECT t.typname, t.oid, a.attnotnull FROM pg_type t, pg_attribute a - WHERE a.atttypid = t.oid AND a.attrelid = reloid AND NOT a.attisdropped - AND a.attname = rec.cname - INTO STRICT rec2; - IF rec2.oid NOT IN (1184) THEN -- timestamptz { - RAISE NOTICE 'Existing % field is of invalid type % (need timestamptz), renaming', rec.cname, rec2.typname; - ELSE -- }{ - sql := 'ALTER TABLE ' || reloid::text || ' ALTER ' || rec.cname - || ' SET NOT NULL, ALTER ' || rec.cname || ' SET DEFAULT now()'; - BEGIN - RAISE DEBUG 'Running %', sql; - EXECUTE sql; - EXIT column_setup; - EXCEPTION - WHEN not_null_violation THEN - RAISE NOTICE '%, renaming', SQLERRM; - WHEN others THEN - RAISE EXCEPTION 'Got % (%)', SQLERRM, SQLSTATE; - END; - END IF; -- } + -- Ensure data type is a TIMESTAMP WITH TIMEZONE + sql := 'ALTER TABLE ' || reloid::text + || ' ALTER ' || rec.cname + || ' SET NOT NULL,' + || ' ALTER ' || rec.cname + || ' TYPE timestamptz USING ' || rec.cname || '::timestamptz,' + || ' ALTER ' || rec.cname + || ' SET DEFAULT now()'; + BEGIN + RAISE DEBUG 'Running %', sql; + EXECUTE sql; + EXIT column_setup; + EXCEPTION + WHEN not_null_violation THEN -- failed not-null + RAISE NOTICE '%, renaming', SQLERRM; + WHEN cannot_coerce THEN -- failed cast + RAISE NOTICE '%, renaming', SQLERRM; + WHEN others THEN + RAISE EXCEPTION 'Got % (%)', SQLERRM, SQLSTATE; + END; -- invalid column, need rename and re-create it i := 0; diff --git a/test/CDB_CartodbfyTableTest.sql b/test/CDB_CartodbfyTableTest.sql index a57be77..0951126 100644 --- a/test/CDB_CartodbfyTableTest.sql +++ b/test/CDB_CartodbfyTableTest.sql @@ -170,6 +170,14 @@ SELECT CDB_CartodbfyTableCheck('t', 'trigger-protected the_geom'); SELECT 'extent',ST_Extent(ST_SnapToGrid(the_geom,0.1)) FROM t; DROP TABLE t; +-- table with existing updated_at and created_at fields ot type text +CREATE TABLE t AS SELECT NOW()::text as created_at, + NOW()::text as updated_at, + NOW() as reftime; +SELECT CDB_CartodbfyTableCheck('t', 'text timestamps'); +SELECT reftime-created_at, reftime-updated_at FROM t; +DROP TABLE t; + -- TODO: table with existing custom-triggered the_geom DROP FUNCTION CDB_CartodbfyTableCheck(regclass, text); diff --git a/test/CDB_CartodbfyTableTest_expect b/test/CDB_CartodbfyTableTest_expect index b66c318..d51a41e 100644 --- a/test/CDB_CartodbfyTableTest_expect +++ b/test/CDB_CartodbfyTableTest_expect @@ -25,5 +25,9 @@ CREATE TRIGGER trigger-protected the_geom cartodbfied fine extent|BOX(1 1,2 2) DROP TABLE +SELECT 1 +text timestamps cartodbfied fine +@ 0|@ 0 +DROP TABLE DROP FUNCTION DROP FUNCTION From 967b7abec0750c940019f7978e0af52c1a975c63 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Thu, 22 May 2014 14:11:09 +0200 Subject: [PATCH 05/54] Try to retain existing cartodb_id values Also reset sequence when original cartodb_id values correctly cast to integers, and does not keep a backup in that case. Closes #27 --- scripts-available/CDB_CartodbfyTable.sql | 37 ++++++++++++++++++++++++ test/CDB_CartodbfyTableTest.sql | 12 ++++++++ test/CDB_CartodbfyTableTest_expect | 8 +++++ 3 files changed, 57 insertions(+) diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql index 8f8af8a..ed267ce 100644 --- a/scripts-available/CDB_CartodbfyTable.sql +++ b/scripts-available/CDB_CartodbfyTable.sql @@ -142,6 +142,43 @@ BEGIN END IF; END LOOP; -- } + -- Try to copy data from new name if possible + IF new_name IS NOT NULL THEN + RAISE NOTICE 'Trying to recover data from % coumn', new_name; + BEGIN + -- Copy existing values to new field + sql := 'UPDATE ' || reloid::text || ' SET cartodb_id = ' + || new_name || '::int4'; + RAISE DEBUG 'Running %', sql; + EXECUTE sql; + + -- Find max value + sql := 'SELECT max(cartodb_id) FROM ' || reloid::text; + RAISE DEBUG 'Running %', sql; + EXECUTE sql INTO rec; + + -- Find sequence name + SELECT pg_catalog.pg_get_serial_sequence(reloid::text, 'cartodb_id') + AS seq INTO rec2; + + -- Reset sequence name + sql := 'ALTER SEQUENCE ' || rec2.seq::text + || ' RESTART WITH ' || rec.max + 1; + RAISE DEBUG 'Running %', sql; + EXECUTE sql; + + -- Drop old column (all went find if we got here) + sql := 'ALTER TABLE ' || reloid::text || ' DROP ' || new_name; + RAISE DEBUG 'Running %', sql; + EXECUTE sql; + + EXCEPTION + WHEN others THEN + RAISE NOTICE 'Could not initialize cartodb_id with existing values: % (%)', + SQLERRM, SQLSTATE; + END; + END IF; + -- We need created_at and updated_at FOR rec IN SELECT * FROM ( VALUES ('created_at'), ('updated_at') ) t(cname) LOOP --{ << column_setup >> diff --git a/test/CDB_CartodbfyTableTest.sql b/test/CDB_CartodbfyTableTest.sql index 0951126..ca843b8 100644 --- a/test/CDB_CartodbfyTableTest.sql +++ b/test/CDB_CartodbfyTableTest.sql @@ -178,6 +178,18 @@ SELECT CDB_CartodbfyTableCheck('t', 'text timestamps'); SELECT reftime-created_at, reftime-updated_at FROM t; DROP TABLE t; +-- table with existing cartodb_id field ot type text +CREATE TABLE t AS SELECT 10::text as cartodb_id; +SELECT CDB_CartodbfyTableCheck('t', 'text cartodb_id'); +select cartodb_id/2 FROM t; +DROP TABLE t; + +-- table with existing cartodb_id field ot type text not casting +CREATE TABLE t AS SELECT 'nan' as cartodb_id; +SELECT CDB_CartodbfyTableCheck('t', 'uncasting text cartodb_id'); +select cartodb_id,_cartodb_id0 FROM t; +DROP TABLE t; + -- TODO: table with existing custom-triggered the_geom DROP FUNCTION CDB_CartodbfyTableCheck(regclass, text); diff --git a/test/CDB_CartodbfyTableTest_expect b/test/CDB_CartodbfyTableTest_expect index d51a41e..7c58681 100644 --- a/test/CDB_CartodbfyTableTest_expect +++ b/test/CDB_CartodbfyTableTest_expect @@ -29,5 +29,13 @@ SELECT 1 text timestamps cartodbfied fine @ 0|@ 0 DROP TABLE +SELECT 1 +text cartodb_id cartodbfied fine +5 +DROP TABLE +SELECT 1 +uncasting text cartodb_id cartodbfied fine +1|nan +DROP TABLE DROP FUNCTION DROP FUNCTION From 5602edd207aa9e2be11a0d0189d9ad0174d90853 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Thu, 22 May 2014 16:31:14 +0200 Subject: [PATCH 06/54] Make time interval test not dependent on intervalstyle Fixes testing from under cartodb/lib/sql --- test/CDB_CartodbfyTableTest.sql | 3 ++- test/CDB_CartodbfyTableTest_expect | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/test/CDB_CartodbfyTableTest.sql b/test/CDB_CartodbfyTableTest.sql index ca843b8..c999afc 100644 --- a/test/CDB_CartodbfyTableTest.sql +++ b/test/CDB_CartodbfyTableTest.sql @@ -175,7 +175,8 @@ CREATE TABLE t AS SELECT NOW()::text as created_at, NOW()::text as updated_at, NOW() as reftime; SELECT CDB_CartodbfyTableCheck('t', 'text timestamps'); -SELECT reftime-created_at, reftime-updated_at FROM t; +SELECT extract(secs from reftime-created_at), + extract(secs from reftime-updated_at) FROM t; DROP TABLE t; -- table with existing cartodb_id field ot type text diff --git a/test/CDB_CartodbfyTableTest_expect b/test/CDB_CartodbfyTableTest_expect index 7c58681..484b233 100644 --- a/test/CDB_CartodbfyTableTest_expect +++ b/test/CDB_CartodbfyTableTest_expect @@ -27,7 +27,7 @@ extent|BOX(1 1,2 2) DROP TABLE SELECT 1 text timestamps cartodbfied fine -@ 0|@ 0 +0|0 DROP TABLE SELECT 1 text cartodb_id cartodbfied fine From fe008b952dc1910f262c48bd340208d9a8984290 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Fri, 23 May 2014 15:33:11 +0200 Subject: [PATCH 07/54] Set version to 0.1.0 --- Makefile | 2 +- NEWS | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 195840d..f989756 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ # cartodb/Makefile EXTENSION = cartodb -EXTVERSION = 0.1dev +EXTVERSION = 0.1.0 CDBSCRIPTS = \ scripts-available/CDB_Roles.sql \ diff --git a/NEWS b/NEWS index 2e38bbb..530e3b5 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,4 @@ -0.1 - 2014-MM-DD ----------------- +0.1.0 - 2014-05-23 +------------------ Initial release From 102e8bd01852ee6b7d391e8a924a861b4bf4a55f Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Fri, 23 May 2014 16:04:54 +0200 Subject: [PATCH 08/54] Improve error message on failing CDB_CartodbfyTable --- scripts-available/CDB_CartodbfyTable.sql | 27 ++++++++++++++++-------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql index ed267ce..8441e67 100644 --- a/scripts-available/CDB_CartodbfyTable.sql +++ b/scripts-available/CDB_CartodbfyTable.sql @@ -83,7 +83,8 @@ BEGIN RAISE NOTICE 'Column cartodb_id already exists'; had_column := TRUE; WHEN others THEN - RAISE EXCEPTION 'Got % (%)', SQLERRM, SQLSTATE; + RAISE EXCEPTION 'Cartodbfying % (cartodb_id): % (%)', + reloid, SQLERRM, SQLSTATE; END; IF had_column THEN @@ -114,7 +115,8 @@ BEGIN WHEN unique_violation OR not_null_violation THEN RAISE NOTICE '%, renaming', SQLERRM; WHEN others THEN - RAISE EXCEPTION 'Got % (%)', SQLERRM, SQLSTATE; + RAISE EXCEPTION 'Cartodbfying % (cartodb_id): % (%)', + reloid, SQLERRM, SQLSTATE; END; END IF; -- } @@ -133,7 +135,8 @@ BEGIN i := i+1; CONTINUE rename_column; WHEN others THEN - RAISE EXCEPTION 'Got % (%)', SQLERRM, SQLSTATE; + RAISE EXCEPTION 'Cartodbfying % (renaming cartodb_id): % (%)', + reloid, SQLERRM, SQLSTATE; END; EXIT rename_column; END LOOP; --} @@ -195,7 +198,8 @@ BEGIN RAISE NOTICE 'Column % already exists', rec.cname; had_column := TRUE; WHEN others THEN - RAISE EXCEPTION 'Got % (%)', SQLERRM, SQLSTATE; + RAISE EXCEPTION 'Cartodbfying % (%): % (%)', + reloid, rec.cname, SQLERRM, SQLSTATE; END; IF had_column THEN @@ -218,7 +222,8 @@ BEGIN WHEN cannot_coerce THEN -- failed cast RAISE NOTICE '%, renaming', SQLERRM; WHEN others THEN - RAISE EXCEPTION 'Got % (%)', SQLERRM, SQLSTATE; + RAISE EXCEPTION 'Cartodbfying % (%): % (%)', + reloid, rec.cname, SQLERRM, SQLSTATE; END; -- invalid column, need rename and re-create it @@ -235,7 +240,8 @@ BEGIN i := i+1; CONTINUE rename_column; WHEN others THEN - RAISE EXCEPTION 'Got % (%)', SQLERRM, SQLSTATE; + RAISE EXCEPTION 'Cartodbfying % (renaming %): % (%)', + reloid, rec.cname, SQLERRM, SQLSTATE; END; EXIT rename_column; END LOOP; --} @@ -264,7 +270,8 @@ BEGIN exists_geom_cols := array_append(exists_geom_cols, true); RAISE NOTICE 'Column % already exists', rec.cname; WHEN others THEN - RAISE EXCEPTION 'Got % (%)', SQLERRM, SQLSTATE; + RAISE EXCEPTION 'Cartodbfying % (%): % (%)', + reloid, rec.cname, SQLERRM, SQLSTATE; END; << column_fixup >> @@ -316,7 +323,8 @@ BEGIN EXECUTE sql; EXCEPTION WHEN others THEN - RAISE EXCEPTION 'Got % (%)', SQLERRM, SQLSTATE; + RAISE EXCEPTION 'Cartodbfying % (% index): % (%)', + reloid, rec.cname, SQLERRM, SQLSTATE; END; END IF; -- } @@ -339,7 +347,8 @@ BEGIN i := i+1; CONTINUE rename_column; WHEN others THEN - RAISE EXCEPTION 'Got % (%)', SQLERRM, SQLSTATE; + RAISE EXCEPTION 'Cartodbfying % (rename %): % (%)', + reloid, rec.cname, SQLERRM, SQLSTATE; END; EXIT rename_column; END LOOP; --} From e9c6728e8e3440463f55150fec9673a696b0ba32 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Fri, 23 May 2014 16:10:03 +0200 Subject: [PATCH 09/54] Back to 0.1.0dev version (aborted final) --- Makefile | 2 +- NEWS | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index f989756..09d9562 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ # cartodb/Makefile EXTENSION = cartodb -EXTVERSION = 0.1.0 +EXTVERSION = 0.1.0dev CDBSCRIPTS = \ scripts-available/CDB_Roles.sql \ diff --git a/NEWS b/NEWS index 530e3b5..4f07037 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,4 @@ -0.1.0 - 2014-05-23 +0.1.0 - 2014-05-DD ------------------ Initial release From 50c5cd676c2a58a642c4f7fe9aa041d9cbd9cd6f Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Fri, 23 May 2014 16:43:01 +0200 Subject: [PATCH 10/54] Encode dependency of cartodb.control on Makefile --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 09d9562..029361b 100644 --- a/Makefile +++ b/Makefile @@ -38,7 +38,7 @@ $(EXTENSION)--$(EXTVERSION).sql: $(CDBSCRIPTS) cartodb_hooks.sql cartodb_version $(EXTENSION)--unpackaged--$(EXTVERSION).sql: $(EXTENSION)--$(EXTVERSION).sql util/create_from_unpackaged.sh Makefile ./util/create_from_unpackaged.sh $(EXTVERSION) -$(EXTENSION).control: $(EXTENSION).control.in +$(EXTENSION).control: $(EXTENSION).control.in Makefile sed -e 's/@@VERSION@@/$(EXTVERSION)/' $< > $@ cartodb_version.sql: cartodb_version.sql.in From 348177d23ad23489fef8be0cf37fa73066f5567c Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Fri, 23 May 2014 16:54:36 +0200 Subject: [PATCH 11/54] Fix cartodbfication of already-cartodbfied tables in presence of views Closes #30 Includes testcase --- scripts-available/CDB_CartodbfyTable.sql | 73 +++++++++++++++++------- test/CDB_CartodbfyTableTest.sql | 3 + test/CDB_CartodbfyTableTest_expect | 3 + 3 files changed, 58 insertions(+), 21 deletions(-) diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql index 8441e67..8c02e1c 100644 --- a/scripts-available/CDB_CartodbfyTable.sql +++ b/scripts-available/CDB_CartodbfyTable.sql @@ -184,6 +184,7 @@ BEGIN -- We need created_at and updated_at FOR rec IN SELECT * FROM ( VALUES ('created_at'), ('updated_at') ) t(cname) LOOP --{ + new_name := null; << column_setup >> LOOP --{ had_column := FALSE; @@ -204,27 +205,35 @@ BEGIN IF had_column THEN - -- Ensure data type is a TIMESTAMP WITH TIMEZONE - sql := 'ALTER TABLE ' || reloid::text - || ' ALTER ' || rec.cname - || ' SET NOT NULL,' - || ' ALTER ' || rec.cname - || ' TYPE timestamptz USING ' || rec.cname || '::timestamptz,' - || ' ALTER ' || rec.cname - || ' SET DEFAULT now()'; - BEGIN - RAISE DEBUG 'Running %', sql; - EXECUTE sql; - EXIT column_setup; - EXCEPTION - WHEN not_null_violation THEN -- failed not-null - RAISE NOTICE '%, renaming', SQLERRM; - WHEN cannot_coerce THEN -- failed cast - RAISE NOTICE '%, renaming', SQLERRM; - WHEN others THEN - RAISE EXCEPTION 'Cartodbfying % (%): % (%)', - reloid, rec.cname, SQLERRM, SQLSTATE; - END; + -- Check data type is a TIMESTAMP WITH TIMEZONE + SELECT t.typname, t.oid, a.attnotnull FROM pg_type t, pg_attribute a + WHERE a.atttypid = t.oid AND a.attrelid = reloid + AND NOT a.attisdropped AND a.attname = rec.cname + INTO STRICT rec2; + IF rec2.oid NOT IN (1184) THEN -- timestamptz { + RAISE NOTICE 'Existing % field is of invalid type % (need timestamptz), renaming', rec. +cname, rec2.typname; + ELSE -- }{ + -- Ensure data type is a TIMESTAMP WITH TIMEZONE + sql := 'ALTER TABLE ' || reloid::text + || ' ALTER ' || rec.cname + || ' SET NOT NULL,' + || ' ALTER ' || rec.cname + || ' SET DEFAULT now()'; + BEGIN + RAISE DEBUG 'Running %', sql; + EXECUTE sql; + EXIT column_setup; + EXCEPTION + WHEN not_null_violation THEN -- failed not-null + RAISE NOTICE '%, renaming', SQLERRM; + WHEN cannot_coerce THEN -- failed cast + RAISE NOTICE '%, renaming', SQLERRM; + WHEN others THEN + RAISE EXCEPTION 'Cartodbfying % (%): % (%)', + reloid, rec.cname, SQLERRM, SQLSTATE; + END; + END IF; -- } -- invalid column, need rename and re-create it i := 0; @@ -250,6 +259,28 @@ BEGIN END IF; END LOOP; -- } + -- Try to copy data from new name if possible + IF new_name IS NOT NULL THEN -- { + RAISE NOTICE 'Trying to recover data from % coumn', new_name; + BEGIN + -- Copy existing values to new field + sql := 'UPDATE ' || reloid::text || ' SET ' || rec.cname || ' = ' + || new_name || '::timestamptz'; + RAISE DEBUG 'Running %', sql; + EXECUTE sql; + + -- Drop old column (all went find if we got here) + sql := 'ALTER TABLE ' || reloid::text || ' DROP ' || new_name; + RAISE DEBUG 'Running %', sql; + EXECUTE sql; + + EXCEPTION + WHEN others THEN + RAISE NOTICE 'Could not initialize % with existing values: % (%)', + rec.cname, SQLERRM, SQLSTATE; + END; + END IF; -- } + END LOOP; -- } -- We need the_geom and the_geom_webmercator diff --git a/test/CDB_CartodbfyTableTest.sql b/test/CDB_CartodbfyTableTest.sql index c999afc..a467903 100644 --- a/test/CDB_CartodbfyTableTest.sql +++ b/test/CDB_CartodbfyTableTest.sql @@ -177,6 +177,9 @@ CREATE TABLE t AS SELECT NOW()::text as created_at, SELECT CDB_CartodbfyTableCheck('t', 'text timestamps'); SELECT extract(secs from reftime-created_at), extract(secs from reftime-updated_at) FROM t; +CREATE VIEW v AS SELECT * FROM t; +SELECT CDB_CartodbfyTableCheck('t', 'cartodbfied with view'); +DROP VIEW v; DROP TABLE t; -- table with existing cartodb_id field ot type text diff --git a/test/CDB_CartodbfyTableTest_expect b/test/CDB_CartodbfyTableTest_expect index 484b233..bee3b23 100644 --- a/test/CDB_CartodbfyTableTest_expect +++ b/test/CDB_CartodbfyTableTest_expect @@ -28,6 +28,9 @@ DROP TABLE SELECT 1 text timestamps cartodbfied fine 0|0 +CREATE VIEW +cartodbfied with view cartodbfied fine +DROP VIEW DROP TABLE SELECT 1 text cartodb_id cartodbfied fine From b2b93a362f5eb74daab4ac799c5b2588dc3bc4af Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Fri, 23 May 2014 16:58:26 +0200 Subject: [PATCH 12/54] Encode dependency of cartodb_version.sql on Makefile --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 029361b..3ee4ad8 100644 --- a/Makefile +++ b/Makefile @@ -41,7 +41,7 @@ $(EXTENSION)--unpackaged--$(EXTVERSION).sql: $(EXTENSION)--$(EXTVERSION).sql uti $(EXTENSION).control: $(EXTENSION).control.in Makefile sed -e 's/@@VERSION@@/$(EXTVERSION)/' $< > $@ -cartodb_version.sql: cartodb_version.sql.in +cartodb_version.sql: cartodb_version.sql.in Makefile sed -e 's/@@VERSION@@/$(EXTVERSION)/' $< > $@ legacy_regress: $(REGRESS_OLD) Makefile From 213cf0aee2b2983cc5ed688c963a3eb16049058b Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Fri, 23 May 2014 17:09:36 +0200 Subject: [PATCH 13/54] Set version to 0.1.0 again --- Makefile | 2 +- NEWS | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 3ee4ad8..3aafbe9 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ # cartodb/Makefile EXTENSION = cartodb -EXTVERSION = 0.1.0dev +EXTVERSION = 0.1.0 CDBSCRIPTS = \ scripts-available/CDB_Roles.sql \ diff --git a/NEWS b/NEWS index 4f07037..530e3b5 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,4 @@ -0.1.0 - 2014-05-DD +0.1.0 - 2014-05-23 ------------------ Initial release From 09cf368d62134bdc06d4b02ef68b8455b5d9648f Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Fri, 23 May 2014 17:40:14 +0200 Subject: [PATCH 14/54] Move DDL triggers and extension postamble under scripts-available Target 0.2.0 --- Makefile | 8 ++++---- NEWS | 4 ++++ .../CDB_DDLTriggers.sql | 0 .../CDB_ExtensionPost.sql | 0 4 files changed, 8 insertions(+), 4 deletions(-) rename cartodb_hooks.sql => scripts-available/CDB_DDLTriggers.sql (100%) rename cartodb_config_dump.sql => scripts-available/CDB_ExtensionPost.sql (100%) diff --git a/Makefile b/Makefile index 3aafbe9..2631263 100644 --- a/Makefile +++ b/Makefile @@ -1,12 +1,14 @@ # cartodb/Makefile EXTENSION = cartodb -EXTVERSION = 0.1.0 +EXTVERSION = 0.2.0dev CDBSCRIPTS = \ scripts-available/CDB_Roles.sql \ scripts-enabled/*.sql \ scripts-available/CDB_SearchPath.sql \ + scripts-available/CDB_DDLTriggers.sql \ + scripts-available/CDB_ExtensionPost.sql \ $(END) DATA_built = \ @@ -25,15 +27,13 @@ PG_CONFIG = pg_config PGXS := $(shell $(PG_CONFIG) --pgxs) include $(PGXS) -$(EXTENSION)--$(EXTVERSION).sql: $(CDBSCRIPTS) cartodb_hooks.sql cartodb_version.sql cartodb_config_dump.sql Makefile +$(EXTENSION)--$(EXTVERSION).sql: $(CDBSCRIPTS) cartodb_version.sql Makefile echo '\echo Use "CREATE EXTENSION $(EXTENSION)" to load this file. \quit' > $@ cat $(CDBSCRIPTS) | \ sed -e 's/\> $@ echo "GRANT USAGE ON SCHEMA cartodb TO public;" >> $@ - cat cartodb_hooks.sql >> $@ cat cartodb_version.sql >> $@ - cat cartodb_config_dump.sql >> $@ $(EXTENSION)--unpackaged--$(EXTVERSION).sql: $(EXTENSION)--$(EXTVERSION).sql util/create_from_unpackaged.sh Makefile ./util/create_from_unpackaged.sh $(EXTVERSION) diff --git a/NEWS b/NEWS index 530e3b5..7dc6e8d 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,7 @@ +0.2.0dev - 2014-MM-DD +------------------ + + 0.1.0 - 2014-05-23 ------------------ diff --git a/cartodb_hooks.sql b/scripts-available/CDB_DDLTriggers.sql similarity index 100% rename from cartodb_hooks.sql rename to scripts-available/CDB_DDLTriggers.sql diff --git a/cartodb_config_dump.sql b/scripts-available/CDB_ExtensionPost.sql similarity index 100% rename from cartodb_config_dump.sql rename to scripts-available/CDB_ExtensionPost.sql From 2ffa13af58b5488cef3a356110086349161f8b45 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Fri, 23 May 2014 18:16:10 +0200 Subject: [PATCH 15/54] Add tool to create upgrades --- util/create_upgrade.sh | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100755 util/create_upgrade.sh diff --git a/util/create_upgrade.sh b/util/create_upgrade.sh new file mode 100755 index 0000000..8b49ceb --- /dev/null +++ b/util/create_upgrade.sh @@ -0,0 +1,9 @@ +#!/bin/sh + +fromver=$1 +ver=$2 +input=cartodb--${ver}.sql +output=cartodb--${fromver}--${ver}.sql + +cat ${input} | grep -v 'duplicated extension$' > ${output} + From 85cc74b4f46c19a5c08ac531f3b0130d5e5d33bf Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Tue, 27 May 2014 15:51:45 +0200 Subject: [PATCH 16/54] Do not install cartodb_version.sql in system dirs --- Makefile | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 2631263..f9e8ea0 100644 --- a/Makefile +++ b/Makefile @@ -14,8 +14,9 @@ CDBSCRIPTS = \ DATA_built = \ $(EXTENSION)--$(EXTVERSION).sql \ $(EXTENSION)--unpackaged--$(EXTVERSION).sql \ - $(EXTENSION).control \ - cartodb_version.sql + $(EXTENSION).control + +EXTRA_CLEAN = cartodb_version.sql DOCS = README.md REGRESS_NEW = test_ddl_triggers From 2601364011b7bfe4bab26535452049f3dc1606ca Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 28 May 2014 09:02:02 +0200 Subject: [PATCH 17/54] Fix recursive DDL trigger on create table Closes #32 --- expected/test_ddl_triggers.out | 53 +++++++++++++++++++++++---- scripts-available/CDB_DDLTriggers.sql | 4 ++ sql/test_ddl_triggers.sql | 25 ++++++++++--- 3 files changed, 69 insertions(+), 13 deletions(-) diff --git a/expected/test_ddl_triggers.out b/expected/test_ddl_triggers.out index cf0978c..53fb677 100644 --- a/expected/test_ddl_triggers.out +++ b/expected/test_ddl_triggers.out @@ -32,6 +32,11 @@ NOTICE: trigger "update_the_geom_webmercator_trigger" for table "c.t3" does not NOTICE: trigger "update_updated_at_trigger" for table "c.t3" does not exist, skipping NOTICE: trigger "test_quota" for table "c.t3" does not exist, skipping NOTICE: trigger "test_quota_per_row" for table "c.t3" does not exist, skipping +NOTICE: event trigger "cdb_on_relation_create" does not exist, skipping +NOTICE: event trigger "cdb_on_relation_drop" does not exist, skipping +NOTICE: event trigger "cdb_on_alter_column" does not exist, skipping +NOTICE: event trigger "cdb_on_drop_column" does not exist, skipping +NOTICE: event trigger "cdb_on_add_column" does not exist, skipping NOTICE: cdb_invalidate_varnish(c.t3) called select cartodb_id, created_at=updated_at as "c=u", @@ -47,12 +52,46 @@ from c.t3; select tabname::text, round(extract('secs' from now() - updated_at)) as age -FROM CDB_TableMetadata; +FROM CDB_TableMetadata WHERE tabname = 'c.t3'::regclass; tabname | age ---------+----- c.t3 | 0 (1 row) +-- Table with cartodb_id field, see +-- http://github.com/CartoDB/cartodb-postgresql/issues/32 +select 1 as cartodb_id INTO c.t4; +NOTICE: trigger "track_updates" for table "c.t4" does not exist, skipping +NOTICE: trigger "update_the_geom_webmercator_trigger" for table "c.t4" does not exist, skipping +NOTICE: trigger "update_updated_at_trigger" for table "c.t4" does not exist, skipping +NOTICE: trigger "test_quota" for table "c.t4" does not exist, skipping +NOTICE: trigger "test_quota_per_row" for table "c.t4" does not exist, skipping +NOTICE: Column cartodb_id already exists +NOTICE: event trigger "cdb_on_relation_create" does not exist, skipping +NOTICE: event trigger "cdb_on_relation_drop" does not exist, skipping +NOTICE: event trigger "cdb_on_alter_column" does not exist, skipping +NOTICE: event trigger "cdb_on_drop_column" does not exist, skipping +NOTICE: event trigger "cdb_on_add_column" does not exist, skipping +NOTICE: cdb_invalidate_varnish(c.t4) called +select + cartodb_id, created_at=updated_at as "c=u", + NOW() - updated_at < '1 secs' as "u<1s", + the_geom, the_geom_webmercator +from c.t4; + cartodb_id | c=u | u<1s | the_geom | the_geom_webmercator +------------+-----+------+----------+---------------------- + 1 | t | t | | +(1 row) + +select + tabname::text, + round(extract('secs' from now() - updated_at)) as age +FROM CDB_TableMetadata WHERE tabname = 'c.t4'::regclass; + tabname | age +---------+----- + c.t4 | 0 +(1 row) + ---------------------------- -- ALTER TABLE RENAME COLUMN ---------------------------- @@ -87,7 +126,7 @@ from c.t3; select tabname::text, round(extract('secs' from now() - updated_at)*10) as agecs -FROM CDB_TableMetadata; +FROM CDB_TableMetadata WHERE tabname = 'c.t3'::regclass; tabname | agecs ---------+------- c.t3 | 0 @@ -124,7 +163,7 @@ from c.t3; select tabname::text, round(extract('secs' from now() - updated_at)*10) as agecs -FROM CDB_TableMetadata; +FROM CDB_TableMetadata WHERE tabname = 'c.t3'::regclass; tabname | agecs ---------+------- c.t3 | 0 @@ -164,7 +203,7 @@ from c.t3; select tabname::text, round(extract('secs' from now() - updated_at)*10) as agecs -FROM CDB_TableMetadata; +FROM CDB_TableMetadata WHERE tabname = 'c.t3'::regclass; tabname | agecs ---------+------- c.t3 | 0 @@ -195,7 +234,7 @@ from c.t3; select tabname::text, round(extract('secs' from now() - updated_at)*10) as agecs -FROM CDB_TableMetadata; +FROM CDB_TableMetadata WHERE tabname = 'c.t3'::regclass; tabname | agecs ---------+------- c.t3 | 0 @@ -206,8 +245,8 @@ FROM CDB_TableMetadata; ---------------------------- RESET SESSION AUTHORIZATION; drop schema c cascade; -NOTICE: drop cascades to table c.t3 -select count(*) from CDB_TableMetadata; +NOTICE: drop cascades to 2 other objects +select count(*) from CDB_TableMetadata; count ------- 0 diff --git a/scripts-available/CDB_DDLTriggers.sql b/scripts-available/CDB_DDLTriggers.sql index 5c58844..bce8fc4 100644 --- a/scripts-available/CDB_DDLTriggers.sql +++ b/scripts-available/CDB_DDLTriggers.sql @@ -24,9 +24,13 @@ BEGIN RETURN; END IF; + PERFORM cartodb.cdb_disable_ddl_hooks(); + -- CDB_CartodbfyTable must not create tables, or infinite loop will happen PERFORM cartodb.CDB_CartodbfyTable(event_info.relation); + PERFORM cartodb.cdb_enable_ddl_hooks(); + RAISE DEBUG 'Inserting into cartodb.CDB_TableMetadata'; -- Add entry to CDB_TableMetadata (should CartodbfyTable do this?) diff --git a/sql/test_ddl_triggers.sql b/sql/test_ddl_triggers.sql index 6972673..eae97c5 100644 --- a/sql/test_ddl_triggers.sql +++ b/sql/test_ddl_triggers.sql @@ -29,7 +29,20 @@ from c.t3; select tabname::text, round(extract('secs' from now() - updated_at)) as age -FROM CDB_TableMetadata; +FROM CDB_TableMetadata WHERE tabname = 'c.t3'::regclass; + +-- Table with cartodb_id field, see +-- http://github.com/CartoDB/cartodb-postgresql/issues/32 +select 1 as cartodb_id INTO c.t4; +select + cartodb_id, created_at=updated_at as "c=u", + NOW() - updated_at < '1 secs' as "u<1s", + the_geom, the_geom_webmercator +from c.t4; +select + tabname::text, + round(extract('secs' from now() - updated_at)) as age +FROM CDB_TableMetadata WHERE tabname = 'c.t4'::regclass; ---------------------------- -- ALTER TABLE RENAME COLUMN @@ -48,7 +61,7 @@ from c.t3; select tabname::text, round(extract('secs' from now() - updated_at)*10) as agecs -FROM CDB_TableMetadata; +FROM CDB_TableMetadata WHERE tabname = 'c.t3'::regclass; select pg_sleep(.1); alter table c.t3 rename column the_geom_webmercator to webmerc2; @@ -63,7 +76,7 @@ from c.t3; select tabname::text, round(extract('secs' from now() - updated_at)*10) as agecs -FROM CDB_TableMetadata; +FROM CDB_TableMetadata WHERE tabname = 'c.t3'::regclass; ---------------------------- -- ALTER TABLE DROP COLUMN @@ -82,7 +95,7 @@ from c.t3; select tabname::text, round(extract('secs' from now() - updated_at)*10) as agecs -FROM CDB_TableMetadata; +FROM CDB_TableMetadata WHERE tabname = 'c.t3'::regclass; ---------------------------- -- ALTER TABLE ADD COLUMN @@ -101,7 +114,7 @@ from c.t3; select tabname::text, round(extract('secs' from now() - updated_at)*10) as agecs -FROM CDB_TableMetadata; +FROM CDB_TableMetadata WHERE tabname = 'c.t3'::regclass; ---------------------------- -- DROP TABLE @@ -109,7 +122,7 @@ FROM CDB_TableMetadata; RESET SESSION AUTHORIZATION; drop schema c cascade; -select count(*) from CDB_TableMetadata; +select count(*) from CDB_TableMetadata; DROP USER cartodb_postgresql_unpriv_user; DROP FUNCTION _CDB_UserQuotaInBytes(); From d1990f1765beb0fe44165e51b725b4f2e139f0b8 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 28 May 2014 09:43:21 +0200 Subject: [PATCH 18/54] Provide upgrade paths Mainly from 0.1.0 and 0.1.1 Includes 0.2.0dev to next and back --- Makefile | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index f9e8ea0..7664d9d 100644 --- a/Makefile +++ b/Makefile @@ -11,9 +11,23 @@ CDBSCRIPTS = \ scripts-available/CDB_ExtensionPost.sql \ $(END) +UPGRADABLE = \ + unpackaged \ + 0.1.0 \ + 0.1.1 \ + $(EXTVERSION)next \ + $(END) + +UPGRADES = \ + $(shell echo $(UPGRADABLE) | \ + sed 's/^/$(EXTENSION)--/' | \ + sed 's/$$/--$(EXTVERSION).sql/' | \ + sed 's/ /--$(EXTVERSION).sql $(EXTENSION)--/g') + DATA_built = \ $(EXTENSION)--$(EXTVERSION).sql \ - $(EXTENSION)--unpackaged--$(EXTVERSION).sql \ + $(EXTENSION)--$(EXTVERSION)--$(EXTVERSION)next.sql \ + $(UPGRADES) \ $(EXTENSION).control EXTRA_CLEAN = cartodb_version.sql @@ -39,6 +53,12 @@ $(EXTENSION)--$(EXTVERSION).sql: $(CDBSCRIPTS) cartodb_version.sql Makefile $(EXTENSION)--unpackaged--$(EXTVERSION).sql: $(EXTENSION)--$(EXTVERSION).sql util/create_from_unpackaged.sh Makefile ./util/create_from_unpackaged.sh $(EXTVERSION) +$(EXTENSION)--%--$(EXTVERSION).sql: $(EXTENSION)--$(EXTVERSION).sql + cp $< $@ + +$(EXTENSION)--$(EXTVERSION)--$(EXTVERSION)next.sql: $(EXTENSION)--$(EXTVERSION).sql + cp $< $@ + $(EXTENSION).control: $(EXTENSION).control.in Makefile sed -e 's/@@VERSION@@/$(EXTVERSION)/' $< > $@ From 004c3c55439b99a6d5370e072a4079e6bccd8794 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 28 May 2014 10:44:23 +0200 Subject: [PATCH 19/54] Ensure cartodb_id uses an associated sequence Closes #33 --- expected/test_ddl_triggers.out | 2 ++ scripts-available/CDB_CartodbfyTable.sql | 12 ++++++++++-- test/CDB_CartodbfyTableTest.sql | 10 ++++++++-- test/CDB_CartodbfyTableTest_expect | 4 ++++ 4 files changed, 24 insertions(+), 4 deletions(-) diff --git a/expected/test_ddl_triggers.out b/expected/test_ddl_triggers.out index 53fb677..14e4b04 100644 --- a/expected/test_ddl_triggers.out +++ b/expected/test_ddl_triggers.out @@ -67,6 +67,8 @@ NOTICE: trigger "update_updated_at_trigger" for table "c.t4" does not exist, sk NOTICE: trigger "test_quota" for table "c.t4" does not exist, skipping NOTICE: trigger "test_quota_per_row" for table "c.t4" does not exist, skipping NOTICE: Column cartodb_id already exists +NOTICE: Existing cartodb_id field does not have an associated sequence, renaming +NOTICE: Trying to recover data from _cartodb_id0 coumn NOTICE: event trigger "cdb_on_relation_create" does not exist, skipping NOTICE: event trigger "cdb_on_relation_drop" does not exist, skipping NOTICE: event trigger "cdb_on_alter_column" does not exist, skipping diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql index 8c02e1c..72cd4ad 100644 --- a/scripts-available/CDB_CartodbfyTable.sql +++ b/scripts-available/CDB_CartodbfyTable.sql @@ -89,13 +89,21 @@ BEGIN IF had_column THEN + SELECT pg_catalog.pg_get_serial_sequence(reloid::text, 'cartodb_id') + AS seq INTO rec2; + -- Check data type is an integer - SELECT t.typname, t.oid, a.attnotnull FROM pg_type t, pg_attribute a + SELECT + pg_catalog.pg_get_serial_sequence(reloid::text, 'cartodb_id') as seq, + t.typname, t.oid, a.attnotnull FROM pg_type t, pg_attribute a WHERE a.atttypid = t.oid AND a.attrelid = reloid AND NOT a.attisdropped AND a.attname = 'cartodb_id' INTO STRICT rec; - IF rec.oid NOT IN (20,21,23) THEN -- int2, int4, int8 { + -- 20=int2, 21=int4, 23=int8 + IF rec.oid NOT IN (20,21,23) THEN RAISE NOTICE 'Existing cartodb_id field is of invalid type % (need int2, int4 or int8), renaming', rec.typname; + ELSIF rec.seq IS NULL THEN + RAISE NOTICE 'Existing cartodb_id field does not have an associated sequence, renaming'; ELSE -- }{ sql := 'ALTER TABLE ' || reloid::text || ' ALTER COLUMN cartodb_id SET NOT NULL'; IF NOT EXISTS ( SELECT c.conname FROM pg_constraint c, pg_attribute a diff --git a/test/CDB_CartodbfyTableTest.sql b/test/CDB_CartodbfyTableTest.sql index a467903..8102fc2 100644 --- a/test/CDB_CartodbfyTableTest.sql +++ b/test/CDB_CartodbfyTableTest.sql @@ -182,18 +182,24 @@ SELECT CDB_CartodbfyTableCheck('t', 'cartodbfied with view'); DROP VIEW v; DROP TABLE t; --- table with existing cartodb_id field ot type text +-- table with existing cartodb_id field of type text CREATE TABLE t AS SELECT 10::text as cartodb_id; SELECT CDB_CartodbfyTableCheck('t', 'text cartodb_id'); select cartodb_id/2 FROM t; DROP TABLE t; --- table with existing cartodb_id field ot type text not casting +-- table with existing cartodb_id field of type text not casting CREATE TABLE t AS SELECT 'nan' as cartodb_id; SELECT CDB_CartodbfyTableCheck('t', 'uncasting text cartodb_id'); select cartodb_id,_cartodb_id0 FROM t; DROP TABLE t; +-- table with existing cartodb_id field of type int4 not sequenced +CREATE TABLE t AS SELECT 1::int4 as cartodb_id; +SELECT CDB_CartodbfyTableCheck('t', 'unsequenced cartodb_id'); +select cartodb_id FROM t; +DROP TABLE t; + -- TODO: table with existing custom-triggered the_geom DROP FUNCTION CDB_CartodbfyTableCheck(regclass, text); diff --git a/test/CDB_CartodbfyTableTest_expect b/test/CDB_CartodbfyTableTest_expect index bee3b23..4a3d2b0 100644 --- a/test/CDB_CartodbfyTableTest_expect +++ b/test/CDB_CartodbfyTableTest_expect @@ -40,5 +40,9 @@ SELECT 1 uncasting text cartodb_id cartodbfied fine 1|nan DROP TABLE +SELECT 1 +unsequenced cartodb_id cartodbfied fine +1 +DROP TABLE DROP FUNCTION DROP FUNCTION From 7483f8d10bf187991581945e070779463c52a55f Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 28 May 2014 12:32:30 +0200 Subject: [PATCH 20/54] Update NEWS file (0.1.1 will likely not happen) --- NEWS | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/NEWS b/NEWS index 7dc6e8d..268a901 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,11 @@ 0.2.0dev - 2014-MM-DD ------------------ +Bug fixes: + + - Fix recursive trigger on create table (#32) + - Ensure cartodb_id uses an associated sequence (#33) + 0.1.0 - 2014-05-23 ------------------ From bfd56f0ea66256399466f331b99bdadcbef3507d Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 28 May 2014 12:52:35 +0200 Subject: [PATCH 21/54] Fully qualify call to cdb_disable_ddl_hooks from cdb_enable_ddl_hooks --- NEWS | 1 + scripts-available/CDB_DDLTriggers.sql | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 268a901..099d6c5 100644 --- a/NEWS +++ b/NEWS @@ -5,6 +5,7 @@ Bug fixes: - Fix recursive trigger on create table (#32) - Ensure cartodb_id uses an associated sequence (#33) + - Fully qualify call to cdb_disable_ddl_hooks from cdb_enable_ddl_hooks 0.1.0 - 2014-05-23 diff --git a/scripts-available/CDB_DDLTriggers.sql b/scripts-available/CDB_DDLTriggers.sql index bce8fc4..ccb25c8 100644 --- a/scripts-available/CDB_DDLTriggers.sql +++ b/scripts-available/CDB_DDLTriggers.sql @@ -175,7 +175,7 @@ CREATE OR REPLACE FUNCTION cartodb.cdb_disable_ddl_hooks() returns void AS $$ $$ LANGUAGE sql; CREATE OR REPLACE FUNCTION cartodb.cdb_enable_ddl_hooks() returns void AS $$ - SELECT cdb_disable_ddl_hooks(); + SELECT cartodb.cdb_disable_ddl_hooks(); CREATE EVENT TRIGGER cdb_on_relation_create ON "relation_create" EXECUTE PROCEDURE cartodb.cdb_handle_create_table(); CREATE EVENT TRIGGER cdb_on_relation_drop From f5ad0d043446ac1862b0628bc300f6c491f227e8 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 28 May 2014 17:42:11 +0200 Subject: [PATCH 22/54] Fully qualify call to CDB_UserDataSize from quota trigger --- NEWS | 1 + scripts-available/CDB_Quota.sql | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 099d6c5..73f92b0 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,7 @@ Bug fixes: - Fix recursive trigger on create table (#32) - Ensure cartodb_id uses an associated sequence (#33) - Fully qualify call to cdb_disable_ddl_hooks from cdb_enable_ddl_hooks + - Fully qualify call to CDB_UserDataSize from quota trigger 0.1.0 - 2014-05-23 diff --git a/scripts-available/CDB_Quota.sql b/scripts-available/CDB_Quota.sql index f8bfab2..4b5ca0b 100644 --- a/scripts-available/CDB_Quota.sql +++ b/scripts-available/CDB_Quota.sql @@ -50,7 +50,7 @@ BEGIN RETURN NEW; END IF; - SELECT CDB_UserDataSize() INTO quota; + SELECT public.CDB_UserDataSize() INTO quota; IF quota > qmax THEN RAISE EXCEPTION 'Quota exceeded by %KB', (quota-qmax)/1024; ELSE RAISE DEBUG 'User quota in bytes: % < % (max allowed)', quota, qmax; From ac53d6b0005db6426c12279233ed5e136db9b183 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Fri, 30 May 2014 12:22:54 +0200 Subject: [PATCH 23/54] Formatting only change --- scripts-available/CDB_CartodbfyTable.sql | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql index 72cd4ad..1c9157e 100644 --- a/scripts-available/CDB_CartodbfyTable.sql +++ b/scripts-available/CDB_CartodbfyTable.sql @@ -100,9 +100,9 @@ BEGIN AND a.attname = 'cartodb_id' INTO STRICT rec; -- 20=int2, 21=int4, 23=int8 - IF rec.oid NOT IN (20,21,23) THEN + IF rec.oid NOT IN (20,21,23) THEN -- { RAISE NOTICE 'Existing cartodb_id field is of invalid type % (need int2, int4 or int8), renaming', rec.typname; - ELSIF rec.seq IS NULL THEN + ELSIF rec.seq IS NULL THEN -- }{ RAISE NOTICE 'Existing cartodb_id field does not have an associated sequence, renaming'; ELSE -- }{ sql := 'ALTER TABLE ' || reloid::text || ' ALTER COLUMN cartodb_id SET NOT NULL'; @@ -191,7 +191,8 @@ BEGIN END IF; -- We need created_at and updated_at - FOR rec IN SELECT * FROM ( VALUES ('created_at'), ('updated_at') ) t(cname) LOOP --{ + FOR rec IN SELECT * FROM ( VALUES ('created_at'), ('updated_at') ) t(cname) + LOOP --{ new_name := null; << column_setup >> LOOP --{ From cc9cbdb7a728421794a181918ba8933b27fb5ad3 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Fri, 30 May 2014 18:29:52 +0200 Subject: [PATCH 24/54] Typo --- expected/test_ddl_triggers.out | 2 +- scripts-available/CDB_CartodbfyTable.sql | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/expected/test_ddl_triggers.out b/expected/test_ddl_triggers.out index 14e4b04..dc263a9 100644 --- a/expected/test_ddl_triggers.out +++ b/expected/test_ddl_triggers.out @@ -68,7 +68,7 @@ NOTICE: trigger "test_quota" for table "c.t4" does not exist, skipping NOTICE: trigger "test_quota_per_row" for table "c.t4" does not exist, skipping NOTICE: Column cartodb_id already exists NOTICE: Existing cartodb_id field does not have an associated sequence, renaming -NOTICE: Trying to recover data from _cartodb_id0 coumn +NOTICE: Trying to recover data from _cartodb_id0 column NOTICE: event trigger "cdb_on_relation_create" does not exist, skipping NOTICE: event trigger "cdb_on_relation_drop" does not exist, skipping NOTICE: event trigger "cdb_on_alter_column" does not exist, skipping diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql index 1c9157e..64eb2a4 100644 --- a/scripts-available/CDB_CartodbfyTable.sql +++ b/scripts-available/CDB_CartodbfyTable.sql @@ -155,7 +155,7 @@ BEGIN -- Try to copy data from new name if possible IF new_name IS NOT NULL THEN - RAISE NOTICE 'Trying to recover data from % coumn', new_name; + RAISE NOTICE 'Trying to recover data from % column', new_name; BEGIN -- Copy existing values to new field sql := 'UPDATE ' || reloid::text || ' SET cartodb_id = ' From ce20009fb29627181050e7b8a9dd7a58fed0540c Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Fri, 30 May 2014 19:09:24 +0200 Subject: [PATCH 25/54] Workaround PostgreSQL bug possibly triggering infinite loops Implements casting of existing system column values during cartodbfication using ALTER rather than UPDATE. It's faster for tables with many rows and works around a longstanding PostgreSQL bug. See http://www.postgresql.org/message-id/20140530143150.GA11051@localhost CDB-3059 --- NEWS | 1 + scripts-available/CDB_CartodbfyTable.sql | 12 ++++++++++-- test/CDB_CartodbfyTableTest.sql | 16 ++++++++++++++++ test/CDB_CartodbfyTableTest_expect | 3 +++ 4 files changed, 30 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 73f92b0..e730fa2 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,7 @@ Bug fixes: - Ensure cartodb_id uses an associated sequence (#33) - Fully qualify call to cdb_disable_ddl_hooks from cdb_enable_ddl_hooks - Fully qualify call to CDB_UserDataSize from quota trigger + - Fix potential infinite loop in CDB_CartodbfyTable 0.1.0 - 2014-05-23 diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql index 64eb2a4..c1467db 100644 --- a/scripts-available/CDB_CartodbfyTable.sql +++ b/scripts-available/CDB_CartodbfyTable.sql @@ -158,7 +158,11 @@ BEGIN RAISE NOTICE 'Trying to recover data from % column', new_name; BEGIN -- Copy existing values to new field - sql := 'UPDATE ' || reloid::text || ' SET cartodb_id = ' + -- NOTE: using ALTER is a workaround to a PostgreSQL bug and + -- is also known to be faster for tables with many rows + -- See http://www.postgresql.org/message-id/20140530143150.GA11051@localhost + sql := 'ALTER TABLE ' || reloid::text + || ' ALTER cartodb_id TYPE int USING ' || new_name || '::int4'; RAISE DEBUG 'Running %', sql; EXECUTE sql; @@ -273,7 +277,11 @@ cname, rec2.typname; RAISE NOTICE 'Trying to recover data from % coumn', new_name; BEGIN -- Copy existing values to new field - sql := 'UPDATE ' || reloid::text || ' SET ' || rec.cname || ' = ' + -- NOTE: using ALTER is a workaround to a PostgreSQL bug and + -- is also known to be faster for tables with many rows + -- See http://www.postgresql.org/message-id/20140530143150.GA11051@localhost + sql := 'ALTER TABLE ' || reloid::text || ' ALTER ' || rec.cname + || ' TYPE TIMESTAMPTZ USING ' || new_name || '::timestamptz'; RAISE DEBUG 'Running %', sql; EXECUTE sql; diff --git a/test/CDB_CartodbfyTableTest.sql b/test/CDB_CartodbfyTableTest.sql index 8102fc2..515470e 100644 --- a/test/CDB_CartodbfyTableTest.sql +++ b/test/CDB_CartodbfyTableTest.sql @@ -200,6 +200,22 @@ SELECT CDB_CartodbfyTableCheck('t', 'unsequenced cartodb_id'); select cartodb_id FROM t; 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 +CREATE TABLE t ( + the_geom geometry(Geometry,4326), + created_at timestamptz, + updated_at timestamptz +); +COPY t (the_geom, created_at, updated_at) FROM stdin; +0106000020E610000001000000010300000001000000050000009EB8244146435BC017B65E062AD343409EB8244146435BC0F51AF6E2708044400B99891683765AC0F51AF6E2708044400B99891683765AC017B65E062AD343409EB8244146435BC017B65E062AD34340 2012-06-06 21:59:08 2013-06-10 20:17:20 +0106000020E61000000100000001030000000100000005000000DA7763431A1A5CC0FBCEE869313C3A40DA7763431A1A5CC09C1B8F55BC494440F9F4A9C7993356C09C1B8F55BC494440F9F4A9C7993356C0FBCEE869313C3A40DA7763431A1A5CC0FBCEE869313C3A40 2012-06-06 21:59:08 2013-06-10 20:17:20 +\N \N \N +\. +SELECT CDB_CartodbfyTableCheck('t', 'null geom and timestamp values'); +DROP TABLE t; + -- TODO: table with existing custom-triggered the_geom DROP FUNCTION CDB_CartodbfyTableCheck(regclass, text); diff --git a/test/CDB_CartodbfyTableTest_expect b/test/CDB_CartodbfyTableTest_expect index 4a3d2b0..c3dd05c 100644 --- a/test/CDB_CartodbfyTableTest_expect +++ b/test/CDB_CartodbfyTableTest_expect @@ -44,5 +44,8 @@ SELECT 1 unsequenced cartodb_id cartodbfied fine 1 DROP TABLE +CREATE TABLE +null geom and timestamp values cartodbfied fine +DROP TABLE DROP FUNCTION DROP FUNCTION From 9230ffa925d5b0be10f6dcf5f52ab5546936b02e Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Tue, 3 Jun 2014 13:20:49 +0200 Subject: [PATCH 26/54] Include revision info in cdb_version() output Closes #34 NOTE: it will only work if "make" is run from within a git clone --- Makefile | 4 +++- NEWS | 4 ++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 7664d9d..4ef7f6d 100644 --- a/Makefile +++ b/Makefile @@ -24,6 +24,8 @@ UPGRADES = \ sed 's/$$/--$(EXTVERSION).sql/' | \ sed 's/ /--$(EXTVERSION).sql $(EXTENSION)--/g') +REV=$(shell git describe) + DATA_built = \ $(EXTENSION)--$(EXTVERSION).sql \ $(EXTENSION)--$(EXTVERSION)--$(EXTVERSION)next.sql \ @@ -63,7 +65,7 @@ $(EXTENSION).control: $(EXTENSION).control.in Makefile sed -e 's/@@VERSION@@/$(EXTVERSION)/' $< > $@ cartodb_version.sql: cartodb_version.sql.in Makefile - sed -e 's/@@VERSION@@/$(EXTVERSION)/' $< > $@ + sed -e 's/@@VERSION@@/$(EXTVERSION) $(REV)/' $< > $@ legacy_regress: $(REGRESS_OLD) Makefile mkdir -p sql/test/ diff --git a/NEWS b/NEWS index e730fa2..5c35f67 100644 --- a/NEWS +++ b/NEWS @@ -9,6 +9,10 @@ Bug fixes: - Fully qualify call to CDB_UserDataSize from quota trigger - Fix potential infinite loop in CDB_CartodbfyTable +Enhancements: + + - Include revision info in cdb_version() output (#34) + 0.1.0 - 2014-05-23 ------------------ From a650c951c8e6ca95e4e496f83165c98a6b77da55 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Tue, 3 Jun 2014 16:26:05 +0200 Subject: [PATCH 27/54] Add link to pg_schema_triggers fork --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 15086fe..672ec4c 100644 --- a/README.md +++ b/README.md @@ -14,6 +14,7 @@ Dependencies * PostgreSQL 9.3+ * [Schema triggers extension] (https://bitbucket.org/malloclabs/pg_schema_triggers) + (or [fork](https://github.com/CartoDB/pg_schema_triggers)) Install ------- From d762da1ca2216046824ee6f9499b8d9a22a734a2 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 4 Jun 2014 10:13:35 +0200 Subject: [PATCH 28/54] Add section about updating cartodb extension CDB-3049 --- README.md | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/README.md b/README.md index 672ec4c..e64df36 100644 --- a/README.md +++ b/README.md @@ -57,3 +57,38 @@ CREATE EXTENSION schema_triggers; CREATE EXTENSION cartodb FROM unpackaged; ``` +Update cartodb extension +------------------------ + +Updating the version of cartodb extension installed in a database +is done using ALTER EXTENSION. + +```sql +ALTER EXTENSION cartodb UPDATE TO '0.1.1'; +``` + +If the "TO 'x.y.z'" part is omitted, the extension will be updated to the +latest installed version, which you can find with the following command: + +```sh +grep default_version `pg_config --sharedir`/extension/cartodb.control +``` + +Updates are performed by PostgreSQL by loading one or more migration scripts +as needed to go from the installed version S to the target version T. +All migration scripts are in the "extension" directory of PostgreSQL: + +```sh +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'; +``` From c286727181379dc30aeeebd2ccc768f6f9295182 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 4 Jun 2014 10:16:42 +0200 Subject: [PATCH 29/54] Add note about extension needing to be installed before it can be upgraded to --- README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.md b/README.md index e64df36..1d16e06 100644 --- a/README.md +++ b/README.md @@ -67,6 +67,9 @@ is done using ALTER EXTENSION. ALTER EXTENSION cartodb UPDATE TO '0.1.1'; ``` +The target version needs to be installed on the system first +(see Install section). + If the "TO 'x.y.z'" part is omitted, the extension will be updated to the latest installed version, which you can find with the following command: From a92d7d7d37473502aab80ede8c3c3da2314aab0c Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 4 Jun 2014 10:28:48 +0200 Subject: [PATCH 30/54] Add cdb_extension_reload() utility function --- Makefile | 1 + NEWS | 4 ++++ README.md | 6 ++++++ scripts-available/CDB_ExtensionUtils.sql | 13 +++++++++++++ 4 files changed, 24 insertions(+) create mode 100644 scripts-available/CDB_ExtensionUtils.sql diff --git a/Makefile b/Makefile index 4ef7f6d..bd465bc 100644 --- a/Makefile +++ b/Makefile @@ -9,6 +9,7 @@ CDBSCRIPTS = \ scripts-available/CDB_SearchPath.sql \ scripts-available/CDB_DDLTriggers.sql \ scripts-available/CDB_ExtensionPost.sql \ + scripts-available/CDB_ExtensionUtils.sql \ $(END) UPGRADABLE = \ diff --git a/NEWS b/NEWS index 5c35f67..5ac9b9b 100644 --- a/NEWS +++ b/NEWS @@ -13,6 +13,10 @@ Enhancements: - Include revision info in cdb_version() output (#34) +New features: + + - Add a cdb_extension_reload() function + 0.1.0 - 2014-05-23 ------------------ diff --git a/README.md b/README.md index 1d16e06..f1fd88f 100644 --- a/README.md +++ b/README.md @@ -95,3 +95,9 @@ to "V". Example to upgrade a 0.2.0dev version: 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(); +``` diff --git a/scripts-available/CDB_ExtensionUtils.sql b/scripts-available/CDB_ExtensionUtils.sql new file mode 100644 index 0000000..853bd95 --- /dev/null +++ b/scripts-available/CDB_ExtensionUtils.sql @@ -0,0 +1,13 @@ +CREATE OR REPLACE FUNCTION cartodb.cdb_extension_reload() RETURNS void +AS $$ +DECLARE + ver TEXT; + sql TEXT; +BEGIN + ver := split_part(cartodb.cdb_version(), ' ', 1); + sql := 'ALTER EXTENSION cartodb UPDATE TO ''' || ver || 'next'''; + EXECUTE sql; + sql := 'ALTER EXTENSION cartodb UPDATE TO ''' || ver || ''''; + EXECUTE sql; +END; +$$ language 'plpgsql' VOLATILE; From 651bb8cdb73439b033490b77e06499d33de6fab4 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 4 Jun 2014 10:40:18 +0200 Subject: [PATCH 31/54] Add instructions about adding a test --- test/README | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 test/README diff --git a/test/README b/test/README new file mode 100644 index 0000000..6dcf328 --- /dev/null +++ b/test/README @@ -0,0 +1,15 @@ +Adding tests consists in adding 2 files in this directory: one file +containing the sql code and another containing the expected output. + +Example, to add a test for CDB_Something function, you'd add: + + - CDB_SomethingTest.sql + - CDB_SomethingTest_expect + +To easy the generation of the expected file you can initially omit it, +then run "make -C .. installcheck" from the top-level dir and copy +../results/test/CDB_SomethingTest.out to CDB_SomethingTest_expect chopping +off the first line: + + make -C .. installcheck + tail -n +2 ../results/test/CDB_SomethingTest.out > CDB_SomethingTest_expect From e993f83b8d6aafe83f049dceb8e2bd5cd7d5dde7 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 4 Jun 2014 10:41:34 +0200 Subject: [PATCH 32/54] Rename test to have "Test" suffix --- test/{CDB_UserTables.sql => CDB_UserTablesTest.sql} | 0 test/{CDB_UserTables_expect => CDB_UserTablesTest_expect} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename test/{CDB_UserTables.sql => CDB_UserTablesTest.sql} (100%) rename test/{CDB_UserTables_expect => CDB_UserTablesTest_expect} (100%) diff --git a/test/CDB_UserTables.sql b/test/CDB_UserTablesTest.sql similarity index 100% rename from test/CDB_UserTables.sql rename to test/CDB_UserTablesTest.sql diff --git a/test/CDB_UserTables_expect b/test/CDB_UserTablesTest_expect similarity index 100% rename from test/CDB_UserTables_expect rename to test/CDB_UserTablesTest_expect From 3fb1f69eb105b6975c5823507cd0cb7c944c5709 Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Wed, 4 Jun 2014 12:44:37 +0200 Subject: [PATCH 33/54] CDB-3086 adds troubleshooting section to readme with information about mac os x sed --- README.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/README.md b/README.md index f1fd88f..7a7d227 100644 --- a/README.md +++ b/README.md @@ -101,3 +101,11 @@ Starting with 0.2.0, the in-place reload can be done with an ad-hoc function: ```sql SELECT cartodb.cdb_extension_reload(); ``` + +Troubleshooting +--------------- +### Mac OS X sed +Mac OS X comes with sed from FreeBSD, some scripting in Makefile is incompatible with that version of sed so we recommend you to install GNU sed. The easiest way is via [Homebrew](http://brew.sh/) like in: +```sh +brew install gnu-sed --default-names +``` From e932ec3595ce2803d66c9161632fcb41e9a2e0aa Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 4 Jun 2014 12:53:14 +0200 Subject: [PATCH 34/54] Fix formatting --- README.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 7a7d227..62f2a6c 100644 --- a/README.md +++ b/README.md @@ -104,8 +104,13 @@ SELECT cartodb.cdb_extension_reload(); Troubleshooting --------------- + ### Mac OS X sed -Mac OS X comes with sed from FreeBSD, some scripting in Makefile is incompatible with that version of sed so we recommend you to install GNU sed. The easiest way is via [Homebrew](http://brew.sh/) like in: + +Mac OS X comes with sed from FreeBSD, some scripting in Makefile is +incompatible with that version of sed so we recommend you to install +GNU sed. The easiest way is via [Homebrew](http://brew.sh/) like in: + ```sh brew install gnu-sed --default-names ``` From 9322cecb3dd5b87dd00f7f932c6ba5e1d34c43f0 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 4 Jun 2014 12:58:19 +0200 Subject: [PATCH 35/54] Make sed command a variable --- Makefile | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/Makefile b/Makefile index bd465bc..55d0bbf 100644 --- a/Makefile +++ b/Makefile @@ -3,6 +3,8 @@ EXTENSION = cartodb EXTVERSION = 0.2.0dev +SED = sed + CDBSCRIPTS = \ scripts-available/CDB_Roles.sql \ scripts-enabled/*.sql \ @@ -21,9 +23,9 @@ UPGRADABLE = \ UPGRADES = \ $(shell echo $(UPGRADABLE) | \ - sed 's/^/$(EXTENSION)--/' | \ - sed 's/$$/--$(EXTVERSION).sql/' | \ - sed 's/ /--$(EXTVERSION).sql $(EXTENSION)--/g') + $(SED) 's/^/$(EXTENSION)--/' | \ + $(SED) 's/$$/--$(EXTVERSION).sql/' | \ + $(SED) 's/ /--$(EXTVERSION).sql $(EXTENSION)--/g') REV=$(shell git describe) @@ -48,7 +50,7 @@ include $(PGXS) $(EXTENSION)--$(EXTVERSION).sql: $(CDBSCRIPTS) cartodb_version.sql Makefile echo '\echo Use "CREATE EXTENSION $(EXTENSION)" to load this file. \quit' > $@ cat $(CDBSCRIPTS) | \ - sed -e 's/\> $@ echo "GRANT USAGE ON SCHEMA cartodb TO public;" >> $@ cat cartodb_version.sql >> $@ @@ -63,10 +65,10 @@ $(EXTENSION)--$(EXTVERSION)--$(EXTVERSION)next.sql: $(EXTENSION)--$(EXTVERSION). cp $< $@ $(EXTENSION).control: $(EXTENSION).control.in Makefile - sed -e 's/@@VERSION@@/$(EXTVERSION)/' $< > $@ + $(SED) -e 's/@@VERSION@@/$(EXTVERSION)/' $< > $@ cartodb_version.sql: cartodb_version.sql.in Makefile - sed -e 's/@@VERSION@@/$(EXTVERSION) $(REV)/' $< > $@ + $(SED) -e 's/@@VERSION@@/$(EXTVERSION) $(REV)/' $< > $@ legacy_regress: $(REGRESS_OLD) Makefile mkdir -p sql/test/ @@ -80,7 +82,7 @@ legacy_regress: $(REGRESS_OLD) Makefile echo '\\t' >> $${of}; \ echo '\\set QUIET off' >> $${of}; \ cat $${f} | \ - sed -e 's/\> $${of}; \ + $(SED) -e 's/\> $${of}; \ exp=expected/test/$${tn}.out; \ echo '\\set ECHO off' > $${exp}; \ cat test/$${tn}_expect >> $${exp}; \ From ca07c81f13e0abbf4958c7544adf4c6e7894df31 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 4 Jun 2014 15:11:30 +0200 Subject: [PATCH 36/54] Make sed usage more compatible ... and a bit less safe Might close #35 --- Makefile | 4 ++-- README.md | 12 ------------ 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/Makefile b/Makefile index 55d0bbf..3881e89 100644 --- a/Makefile +++ b/Makefile @@ -50,7 +50,7 @@ include $(PGXS) $(EXTENSION)--$(EXTVERSION).sql: $(CDBSCRIPTS) cartodb_version.sql Makefile echo '\echo Use "CREATE EXTENSION $(EXTENSION)" to load this file. \quit' > $@ cat $(CDBSCRIPTS) | \ - $(SED) -e 's/\> $@ echo "GRANT USAGE ON SCHEMA cartodb TO public;" >> $@ cat cartodb_version.sql >> $@ @@ -82,7 +82,7 @@ legacy_regress: $(REGRESS_OLD) Makefile echo '\\t' >> $${of}; \ echo '\\set QUIET off' >> $${of}; \ cat $${f} | \ - $(SED) -e 's/\> $${of}; \ + $(SED) -e 's/public\./cartodb./g' >> $${of}; \ exp=expected/test/$${tn}.out; \ echo '\\set ECHO off' > $${exp}; \ cat test/$${tn}_expect >> $${exp}; \ diff --git a/README.md b/README.md index 62f2a6c..5431fce 100644 --- a/README.md +++ b/README.md @@ -102,15 +102,3 @@ Starting with 0.2.0, the in-place reload can be done with an ad-hoc function: SELECT cartodb.cdb_extension_reload(); ``` -Troubleshooting ---------------- - -### Mac OS X sed - -Mac OS X comes with sed from FreeBSD, some scripting in Makefile is -incompatible with that version of sed so we recommend you to install -GNU sed. The easiest way is via [Homebrew](http://brew.sh/) like in: - -```sh -brew install gnu-sed --default-names -``` From fba8d3ab84a570f6dfcbce17fc4f489faf9ee9dc Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 4 Jun 2014 17:03:17 +0200 Subject: [PATCH 37/54] Do not assume cartodb functions install in "public" schema during testing --- test/CDB_CartodbfyTableTest.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/CDB_CartodbfyTableTest.sql b/test/CDB_CartodbfyTableTest.sql index 515470e..d1d3ab2 100644 --- a/test/CDB_CartodbfyTableTest.sql +++ b/test/CDB_CartodbfyTableTest.sql @@ -165,7 +165,7 @@ DROP TABLE t; -- table with existing triggered the_geom CREATE TABLE t AS SELECT 'SRID=4326;LINESTRING(1 1,2 2)'::geometry(geometry) as the_geom; CREATE TRIGGER update_the_geom_webmercator_trigger BEFORE UPDATE OF the_geom ON t - FOR EACH ROW EXECUTE PROCEDURE public._CDB_update_the_geom_webmercator(); + FOR EACH ROW EXECUTE PROCEDURE _CDB_update_the_geom_webmercator(); SELECT CDB_CartodbfyTableCheck('t', 'trigger-protected the_geom'); SELECT 'extent',ST_Extent(ST_SnapToGrid(the_geom,0.1)) FROM t; DROP TABLE t; From f9f73d2d62801022ecc198b72d366d96dd58523a Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Thu, 5 Jun 2014 15:00:57 +0200 Subject: [PATCH 38/54] Fix potential infinite loop in CDB_QueryStatements This fix was already present at one point in cartodb/lib/sql (where the code was copied from) but in a different branch than the one the code was initially copied from. The fix depends on plpython language which becomes a new dependency. --- NEWS | 5 ++++ README.md | 3 +- cartodb.control.in | 2 +- expected/test_setup.out | 1 + scripts-available/CDB_QueryStatements.sql | 17 +++++------ sql/test_setup.sql | 1 + test/CDB_QueryStatementsTest.sql | 35 +++++++++++++++++++++++ test/CDB_QueryStatementsTest_expect | 10 +++++++ 8 files changed, 64 insertions(+), 10 deletions(-) diff --git a/NEWS b/NEWS index 5ac9b9b..c4f469a 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,10 @@ 0.2.0dev - 2014-MM-DD ------------------ +Important changes: + + - This release adds dependency on "plpythonu" extension + Bug fixes: - Fix recursive trigger on create table (#32) @@ -8,6 +12,7 @@ Bug fixes: - Fully qualify call to cdb_disable_ddl_hooks from cdb_enable_ddl_hooks - Fully qualify call to CDB_UserDataSize from quota trigger - Fix potential infinite loop in CDB_CartodbfyTable + - Fix potential infinite loop in CDB_QueryStatements Enhancements: diff --git a/README.md b/README.md index 5431fce..698f16e 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,8 @@ See https://github.com/CartoDB/cartodb/wiki/CartoDB-PostgreSQL-extension Dependencies ------------ - * PostgreSQL 9.3+ + * PostgreSQL 9.3+ (with plpythonu extension) + * [PostGIS extension](http://postgis.net) * [Schema triggers extension] (https://bitbucket.org/malloclabs/pg_schema_triggers) (or [fork](https://github.com/CartoDB/pg_schema_triggers)) diff --git a/cartodb.control.in b/cartodb.control.in index 36fc0ee..3d783c7 100644 --- a/cartodb.control.in +++ b/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' +requires = 'schema_triggers, postgis, plpythonu' diff --git a/expected/test_setup.out b/expected/test_setup.out index 1f4bf2a..c05769f 100644 --- a/expected/test_setup.out +++ b/expected/test_setup.out @@ -1,5 +1,6 @@ CREATE EXTENSION postgis; CREATE EXTENSION schema_triggers; +CREATE EXTENSION plpythonu; CREATE EXTENSION cartodb; CREATE FUNCTION public.cdb_invalidate_varnish(table_name text) RETURNS void AS $$ diff --git a/scripts-available/CDB_QueryStatements.sql b/scripts-available/CDB_QueryStatements.sql index 9c8309a..bb4d10c 100644 --- a/scripts-available/CDB_QueryStatements.sql +++ b/scripts-available/CDB_QueryStatements.sql @@ -1,13 +1,14 @@ -- Return an array of statements found in the given query text -- --- Curtesy of Hubert Lubaczewski (depesz) +-- Regexp curtesy of Hubert Lubaczewski (depesz) +-- Implemented in plpython for performance reasons -- CREATE OR REPLACE FUNCTION CDB_QueryStatements(query text) RETURNS SETOF TEXT AS $$ - SELECT stmt FROM ( - SELECT btrim(q[1], E' \n\t\r;') as stmt FROM ( - SELECT regexp_matches( $1, $REG$((?:[^'"$;]+|"[^"]*"|'(?:[^']*|'')*'|(\$[^$]*\$).*?\2)+)$REG$, 'g' ) as q - ) i - ) j - WHERE stmt <> ''; -$$ language sql IMMUTABLE STRICT; + import re + pat = re.compile( r'''((?:[^'"$;]+|"[^"]*"|'[^']*'|(\$[^$]*\$).*?\2)+)''', re.DOTALL ) + for match in pat.findall(query): + cleaned = match[0].strip() + if ( cleaned ): + yield cleaned +$$ language 'plpythonu' IMMUTABLE STRICT; diff --git a/sql/test_setup.sql b/sql/test_setup.sql index 1f4bf2a..c05769f 100644 --- a/sql/test_setup.sql +++ b/sql/test_setup.sql @@ -1,5 +1,6 @@ CREATE EXTENSION postgis; CREATE EXTENSION schema_triggers; +CREATE EXTENSION plpythonu; CREATE EXTENSION cartodb; CREATE FUNCTION public.cdb_invalidate_varnish(table_name text) RETURNS void AS $$ diff --git a/test/CDB_QueryStatementsTest.sql b/test/CDB_QueryStatementsTest.sql index c1c5424..7e8873c 100644 --- a/test/CDB_QueryStatementsTest.sql +++ b/test/CDB_QueryStatementsTest.sql @@ -26,3 +26,38 @@ INSER INTO "my''""t" values ('''','""'';;'); SELECT $qu;oted$ hi $qu;oted$; $the_param$) as statement ) SELECT '5', row_number() over (), statement FROM q; + +WITH q AS ( SELECT CDB_QueryStatements($the_param$ +SELECT +1 ; SELECT +2 +$the_param$) as statement ) +SELECT '6', row_number() over (), statement FROM q; + +-- This is an insane input, illegal sql +-- we are really only testing that it does not +-- take forever to process.. +-- The actual result is not correct, so if the function +-- ever gets fixed check if it's better +WITH q AS ( SELECT CDB_QueryStatements($the_param$ + + + + + + + + + /a + $b$ + $c$d + ; +$the_param$) as statement ) +SELECT '7', row_number() over (), statement FROM q; + + +WITH q AS ( SELECT CDB_QueryStatements($the_param$ +SELECT $quoted$ hi +$quoted$; +$the_param$) as statement ) +SELECT '8', row_number() over (), statement FROM q; diff --git a/test/CDB_QueryStatementsTest_expect b/test/CDB_QueryStatementsTest_expect index 9ed8fb1..7334d4b 100644 --- a/test/CDB_QueryStatementsTest_expect +++ b/test/CDB_QueryStatementsTest_expect @@ -7,3 +7,13 @@ 4|4|SELECT 5 5|1|INSER INTO "my''""t" values ('''','""'';;') 5|2|SELECT $qu;oted$ hi $qu;oted$ +6|1|SELECT +1 +6|2|SELECT +2 +7|1|/a +7|2|b +7|3|c +7|4|d +8|1|SELECT $quoted$ hi +$quoted$ From 1f7b8db5323d63231f9a023f54ff29663a4c042f Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Thu, 5 Jun 2014 15:06:14 +0200 Subject: [PATCH 39/54] [Travis] install postgresql-plpython-9.3 package, now needed --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index c2a88a6..0113cd4 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,6 +6,7 @@ addons: before_install: #- sudo apt-get install -q postgresql-9.3-postgis-2.1 - sudo apt-get install -q postgresql-server-dev-9.3 + - sudo apt-get install -q postgresql-plpython-9.3 # Install schema_triggers - hg clone https://bitbucket.org/malloclabs/pg_schema_triggers && cd pg_schema_triggers && make && sudo make install && cd - From f6e04562652c17b735ad4d02bd0821f2e11fa3e8 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Thu, 5 Jun 2014 18:20:04 +0200 Subject: [PATCH 40/54] Increase tolerance in floating number based tests (extent) --- test/CDB_CartodbfyTableTest.sql | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/CDB_CartodbfyTableTest.sql b/test/CDB_CartodbfyTableTest.sql index d1d3ab2..6a4af42 100644 --- a/test/CDB_CartodbfyTableTest.sql +++ b/test/CDB_CartodbfyTableTest.sql @@ -146,7 +146,7 @@ DROP TABLE t; CREATE TABLE t AS SELECT ST_SetSRID(ST_MakePoint(-1,-1),4326) as the_geom UNION ALL SELECT ST_SetSRID(ST_MakePoint(0,0),3857); SELECT CDB_CartodbfyTableCheck('t', 'mixed-srid the_geom'); -SELECT 'extent',ST_Extent(the_geom) FROM t; +SELECT 'extent',ST_Extent(ST_SnapToGrid(the_geom,0.2)) FROM t; DROP TABLE t; -- table with wrong srid-constrained the_geom values @@ -159,7 +159,7 @@ DROP TABLE t; CREATE TABLE t AS SELECT 'SRID=4326;LINESTRING(1 1,2 2)'::geometry(geometry,4326) as the_geom_webmercator; SELECT CDB_CartodbfyTableCheck('t', 'wrong srid-constrained the_geom_webmercator'); -- expect the_geom to be populated from the_geom_webmercator -SELECT 'extent',ST_Extent(ST_SnapToGrid(the_geom,0.1)) FROM t; +SELECT 'extent',ST_Extent(ST_SnapToGrid(the_geom,0.2)) FROM t; DROP TABLE t; -- table with existing triggered the_geom @@ -167,7 +167,7 @@ CREATE TABLE t AS SELECT 'SRID=4326;LINESTRING(1 1,2 2)'::geometry(geometry) as CREATE TRIGGER update_the_geom_webmercator_trigger BEFORE UPDATE OF the_geom ON t FOR EACH ROW EXECUTE PROCEDURE _CDB_update_the_geom_webmercator(); SELECT CDB_CartodbfyTableCheck('t', 'trigger-protected the_geom'); -SELECT 'extent',ST_Extent(ST_SnapToGrid(the_geom,0.1)) FROM t; +SELECT 'extent',ST_Extent(ST_SnapToGrid(the_geom,0.2)) FROM t; DROP TABLE t; -- table with existing updated_at and created_at fields ot type text From edc56e60ee804d4a9a92529e370ea4dacfbf340c Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Thu, 5 Jun 2014 18:21:52 +0200 Subject: [PATCH 41/54] Further increase in test tolerance --- test/CDB_CartodbfyTableTest.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/CDB_CartodbfyTableTest.sql b/test/CDB_CartodbfyTableTest.sql index 6a4af42..e68cb8b 100644 --- a/test/CDB_CartodbfyTableTest.sql +++ b/test/CDB_CartodbfyTableTest.sql @@ -152,7 +152,7 @@ DROP TABLE t; -- table with wrong srid-constrained the_geom values CREATE TABLE t AS SELECT 'SRID=3857;LINESTRING(222638.981586547 222684.208505545, 111319.490793274 111325.142866385)'::geometry(geometry,3857) as the_geom; SELECT CDB_CartodbfyTableCheck('t', 'wrong srid-constrained the_geom'); -SELECT 'extent',ST_Extent(the_geom),ST_Extent(ST_SnapToGrid(the_geom_webmercator,1)) FROM t; +SELECT 'extent',ST_Extent(ST_SnapToGrid(the_geom,0.2)),ST_Extent(ST_SnapToGrid(the_geom_webmercator,1)) FROM t; DROP TABLE t; -- table with wrong srid-constrained the_geom_webmercator values (and no the_geom!) From 01ae7b8c1025de65dedab31c8c7b33bea8543aee Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Fri, 6 Jun 2014 11:02:51 +0200 Subject: [PATCH 42/54] Drop role management Roles are not created anymore, previously private functions for table information extraction (CDB_UserTables, CDB_TableIndexes, CDB_ColumnNames, CDB_ColumnType) will now be callable by anyone while only returning information about tables over which the calling user has SELECT privilege. Closes #36 --- Makefile | 1 - NEWS | 4 ++++ scripts-available/CDB_ColumnNames.sql | 3 --- scripts-available/CDB_ColumnType.sql | 3 --- scripts-available/CDB_DDLTriggers.sql | 5 ----- scripts-available/CDB_Roles.sql | 13 ------------- scripts-available/CDB_TableIndexes.sql | 6 ++---- scripts-available/CDB_UserTables.sql | 8 ++++---- 8 files changed, 10 insertions(+), 33 deletions(-) delete mode 100644 scripts-available/CDB_Roles.sql diff --git a/Makefile b/Makefile index 3881e89..c3ce7f8 100644 --- a/Makefile +++ b/Makefile @@ -6,7 +6,6 @@ EXTVERSION = 0.2.0dev SED = sed CDBSCRIPTS = \ - scripts-available/CDB_Roles.sql \ scripts-enabled/*.sql \ scripts-available/CDB_SearchPath.sql \ scripts-available/CDB_DDLTriggers.sql \ diff --git a/NEWS b/NEWS index c4f469a..5ca6552 100644 --- a/NEWS +++ b/NEWS @@ -4,6 +4,10 @@ Important changes: - This release adds dependency on "plpythonu" extension + - Roles are not created anymore, previously private functions + for table information extraction will now be callable by + anyone while only returning information about tables over + which the calling user has SELECT privilege (#36) Bug fixes: diff --git a/scripts-available/CDB_ColumnNames.sql b/scripts-available/CDB_ColumnNames.sql index 2c74dbc..c392321 100644 --- a/scripts-available/CDB_ColumnNames.sql +++ b/scripts-available/CDB_ColumnNames.sql @@ -11,6 +11,3 @@ AS $$ $$ LANGUAGE SQL; --- This is a private function, so only the db owner need privileges -REVOKE ALL ON FUNCTION CDB_ColumnNames(REGCLASS) FROM PUBLIC; -GRANT EXECUTE ON FUNCTION CDB_ColumnNames(REGCLASS) TO ":DATABASE_USERNAME"; diff --git a/scripts-available/CDB_ColumnType.sql b/scripts-available/CDB_ColumnType.sql index 6b8196e..4319bbe 100644 --- a/scripts-available/CDB_ColumnType.sql +++ b/scripts-available/CDB_ColumnType.sql @@ -12,6 +12,3 @@ AS $$ $$ LANGUAGE SQL; --- This is a private function, so only the db owner need privileges -REVOKE ALL ON FUNCTION CDB_ColumnType(REGCLASS, TEXT) FROM PUBLIC; -GRANT EXECUTE ON FUNCTION CDB_ColumnType(REGCLASS, TEXT) TO ":DATABASE_USERNAME"; diff --git a/scripts-available/CDB_DDLTriggers.sql b/scripts-available/CDB_DDLTriggers.sql index ccb25c8..37ab988 100644 --- a/scripts-available/CDB_DDLTriggers.sql +++ b/scripts-available/CDB_DDLTriggers.sql @@ -1,8 +1,3 @@ ---LOAD 'schema_triggers.so'; ---CREATE EXTENSION IF NOT EXISTS schema_triggers; - ---GRANT EXECUTE ON ALL FUNCTIONS IN SCHEMA schema_triggers TO public; - -- Table creation -- { CREATE OR REPLACE FUNCTION cartodb.cdb_handle_create_table () diff --git a/scripts-available/CDB_Roles.sql b/scripts-available/CDB_Roles.sql deleted file mode 100644 index c984d3c..0000000 --- a/scripts-available/CDB_Roles.sql +++ /dev/null @@ -1,13 +0,0 @@ -DO LANGUAGE 'plpgsql' $$ -BEGIN - IF NOT EXISTS ( SELECT * FROM pg_roles WHERE rolname= 'cdb_org_admin' ) - THEN - CREATE ROLE cdb_org_admin NOLOGIN; - END IF; - - IF NOT EXISTS ( SELECT * FROM pg_roles WHERE rolname= 'cdb_org_user' ) - THEN - CREATE ROLE cdb_org_user NOLOGIN; - END IF; -END -$$; diff --git a/scripts-available/CDB_TableIndexes.sql b/scripts-available/CDB_TableIndexes.sql index c6871fe..bbb7e00 100644 --- a/scripts-available/CDB_TableIndexes.sql +++ b/scripts-available/CDB_TableIndexes.sql @@ -17,10 +17,8 @@ AS $$ ON pg_class.oid = idx.indexrelid WHERE pg_indexes.tablename = '' || $1 || '' AND '' || $1 || '' IN (SELECT CDB_UserTables()) - AND pg_class.relname=pg_indexes.indexname; + AND pg_class.relname=pg_indexes.indexname + ; $$ LANGUAGE SQL; --- This is a private function, so only the db owner need privileges -REVOKE ALL ON FUNCTION CDB_TableIndexes(REGCLASS) FROM PUBLIC; -GRANT EXECUTE ON FUNCTION CDB_TableIndexes(REGCLASS) TO ":DATABASE_USERNAME"; diff --git a/scripts-available/CDB_UserTables.sql b/scripts-available/CDB_UserTables.sql index c2fa4ee..aa9e350 100644 --- a/scripts-available/CDB_UserTables.sql +++ b/scripts-available/CDB_UserTables.sql @@ -26,14 +26,14 @@ AS $$ FROM usertables ) SELECT t FROM perms - WHERE p = CASE WHEN $1 = 'private' THEN false + WHERE ( + p = CASE WHEN $1 = 'private' THEN false WHEN $1 = 'public' THEN true ELSE not p -- none END OR $1 = 'all' + ) + AND has_table_privilege('public'||'.'||t, 'SELECT') ; $$ LANGUAGE 'sql'; --- This is a private function, so only the db owner need privileges -REVOKE ALL ON FUNCTION CDB_UserTables(text) FROM PUBLIC; -GRANT EXECUTE ON FUNCTION CDB_UserTables(text) TO ":DATABASE_USERNAME"; From f8180123eb8abc4bcf84fa92f24956433c4fdd59 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Fri, 6 Jun 2014 11:33:34 +0200 Subject: [PATCH 43/54] Grant permission to run table-info functions to public This is needed as a way to migrate from versions < 0.2.0 See #36 --- NEWS | 7 ++++--- scripts-available/CDB_ColumnNames.sql | 3 +++ scripts-available/CDB_ColumnType.sql | 3 +++ scripts-available/CDB_TableIndexes.sql | 3 +++ scripts-available/CDB_UserTables.sql | 4 +++- 5 files changed, 16 insertions(+), 4 deletions(-) diff --git a/NEWS b/NEWS index 5ca6552..a77636b 100644 --- a/NEWS +++ b/NEWS @@ -5,9 +5,10 @@ Important changes: - This release adds dependency on "plpythonu" extension - Roles are not created anymore, previously private functions - for table information extraction will now be callable by - anyone while only returning information about tables over - which the calling user has SELECT privilege (#36) + for table information extraction (CDB_UserTables, CDB_TableIndexes, + CDB_ColumnNames, CDB_ColumnType) will now be callable by anyone while + only returning information about tables over which the calling user + has SELECT privilege (#36) Bug fixes: diff --git a/scripts-available/CDB_ColumnNames.sql b/scripts-available/CDB_ColumnNames.sql index c392321..693ba3d 100644 --- a/scripts-available/CDB_ColumnNames.sql +++ b/scripts-available/CDB_ColumnNames.sql @@ -11,3 +11,6 @@ AS $$ $$ LANGUAGE SQL; +-- This is to migrate from pre-0.2.0 version +-- See http://github.com/CartoDB/cartodb-postgresql/issues/36 +GRANT EXECUTE ON FUNCTION CDB_ColumnNames(REGCLASS) TO PUBLIC; diff --git a/scripts-available/CDB_ColumnType.sql b/scripts-available/CDB_ColumnType.sql index 4319bbe..f887771 100644 --- a/scripts-available/CDB_ColumnType.sql +++ b/scripts-available/CDB_ColumnType.sql @@ -12,3 +12,6 @@ AS $$ $$ LANGUAGE SQL; +-- This is to migrate from pre-0.2.0 version +-- See http://github.com/CartoDB/cartodb-postgresql/issues/36 +GRANT EXECUTE ON FUNCTION CDB_ColumnType(REGCLASS, TEXT) TO public; diff --git a/scripts-available/CDB_TableIndexes.sql b/scripts-available/CDB_TableIndexes.sql index bbb7e00..c0ae98d 100644 --- a/scripts-available/CDB_TableIndexes.sql +++ b/scripts-available/CDB_TableIndexes.sql @@ -22,3 +22,6 @@ AS $$ $$ LANGUAGE SQL; +-- This is to migrate from pre-0.2.0 version +-- See http://github.com/CartoDB/cartodb-postgresql/issues/36 +GRANT EXECUTE ON FUNCTION CDB_TableIndexes(REGCLASS) TO public; diff --git a/scripts-available/CDB_UserTables.sql b/scripts-available/CDB_UserTables.sql index aa9e350..40e2f51 100644 --- a/scripts-available/CDB_UserTables.sql +++ b/scripts-available/CDB_UserTables.sql @@ -5,7 +5,6 @@ -- -- Currently accepted permissions are: 'public', 'private' or 'all' -- --- DROP FUNCTION IF EXISTS CDB_UserTables(); -- replaced by: CREATE OR REPLACE FUNCTION CDB_UserTables(perm text DEFAULT 'all') RETURNS SETOF information_schema.sql_identifier AS $$ @@ -37,3 +36,6 @@ AS $$ ; $$ LANGUAGE 'sql'; +-- This is to migrate from pre-0.2.0 version +-- See http://github.com/CartoDB/cartodb-postgresql/issues/36 +GRANT EXECUTE ON FUNCTION CDB_UserTables(text) TO public; From 1692e792a78a070ea64032c2d0d0e28b72112b49 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Fri, 6 Jun 2014 13:36:21 +0200 Subject: [PATCH 44/54] Make .git/index a dependency of cartodb_version.sql Ensures proper rebuild of version function on git pull/commit --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index c3ce7f8..5b47851 100644 --- a/Makefile +++ b/Makefile @@ -66,7 +66,7 @@ $(EXTENSION)--$(EXTVERSION)--$(EXTVERSION)next.sql: $(EXTENSION)--$(EXTVERSION). $(EXTENSION).control: $(EXTENSION).control.in Makefile $(SED) -e 's/@@VERSION@@/$(EXTVERSION)/' $< > $@ -cartodb_version.sql: cartodb_version.sql.in Makefile +cartodb_version.sql: cartodb_version.sql.in Makefile .git/index $(SED) -e 's/@@VERSION@@/$(EXTVERSION) $(REV)/' $< > $@ legacy_regress: $(REGRESS_OLD) Makefile From 85afef40d80da9d3457b741daba2b0bb178a6e7f Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Fri, 6 Jun 2014 16:13:47 +0200 Subject: [PATCH 45/54] Make git dependency support use as a submodule --- Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 5b47851..dd5f731 100644 --- a/Makefile +++ b/Makefile @@ -27,6 +27,7 @@ UPGRADES = \ $(SED) 's/ /--$(EXTVERSION).sql $(EXTENSION)--/g') REV=$(shell git describe) +GITDIR=$(shell test -d .git && echo '.git' || cat .git | $(SED) 's/^gitdir: //') DATA_built = \ $(EXTENSION)--$(EXTVERSION).sql \ @@ -66,7 +67,7 @@ $(EXTENSION)--$(EXTVERSION)--$(EXTVERSION)next.sql: $(EXTENSION)--$(EXTVERSION). $(EXTENSION).control: $(EXTENSION).control.in Makefile $(SED) -e 's/@@VERSION@@/$(EXTVERSION)/' $< > $@ -cartodb_version.sql: cartodb_version.sql.in Makefile .git/index +cartodb_version.sql: cartodb_version.sql.in Makefile $(GITDIR)/index $(SED) -e 's/@@VERSION@@/$(EXTVERSION) $(REV)/' $< > $@ legacy_regress: $(REGRESS_OLD) Makefile From d52556c20e92ce011c12eef4f0d32a0ff7d6afe7 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Fri, 6 Jun 2014 17:28:02 +0200 Subject: [PATCH 46/54] Fully qualify call to CDB_TransformToWebmercator from CDB_CartodbfyTable --- NEWS | 1 + scripts-available/CDB_CartodbfyTable.sql | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index a77636b..a8baf16 100644 --- a/NEWS +++ b/NEWS @@ -16,6 +16,7 @@ Bug fixes: - Ensure cartodb_id uses an associated sequence (#33) - Fully qualify call to cdb_disable_ddl_hooks from cdb_enable_ddl_hooks - Fully qualify call to CDB_UserDataSize from quota trigger + - Fully qualify call to CDB_TransformToWebmercator from CDB_CartodbfyTable - Fix potential infinite loop in CDB_CartodbfyTable - Fix potential infinite loop in CDB_QueryStatements diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql index c1467db..1d4b53b 100644 --- a/scripts-available/CDB_CartodbfyTable.sql +++ b/scripts-available/CDB_CartodbfyTable.sql @@ -10,7 +10,7 @@ CREATE OR REPLACE FUNCTION _CDB_update_the_geom_webmercator() RETURNS trigger AS $$ BEGIN - NEW.the_geom_webmercator := CDB_TransformToWebmercator(NEW.the_geom); + NEW.the_geom_webmercator := public.CDB_TransformToWebmercator(NEW.the_geom); RETURN NEW; END; $$ LANGUAGE plpgsql VOLATILE; From f75c256b6e3357dfee5ed354a0ad15cfc9842100 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Fri, 6 Jun 2014 18:09:41 +0200 Subject: [PATCH 47/54] Another full qualification of CDB_TransformToWebmercator in CDB_CartodbfyTable --- scripts-available/CDB_CartodbfyTable.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql index 1d4b53b..e69f187 100644 --- a/scripts-available/CDB_CartodbfyTable.sql +++ b/scripts-available/CDB_CartodbfyTable.sql @@ -418,7 +418,7 @@ cname, rec2.typname; -- do this only if the_geom was found (not created) -- _and_ the_geom_webmercator was NOT found. IF exists_geom_cols[1] AND NOT exists_geom_cols[2] THEN - sql := 'UPDATE ' || reloid::text || ' SET the_geom_webmercator = CDB_TransformToWebmercator(the_geom) '; + sql := 'UPDATE ' || reloid::text || ' SET the_geom_webmercator = public.CDB_TransformToWebmercator(the_geom) '; EXECUTE sql; END IF; From befa7a10c8b22b9a4e029684bcb881a52454733e Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Mon, 9 Jun 2014 12:51:31 +0200 Subject: [PATCH 48/54] Set version to 0.2.0 --- Makefile | 3 ++- NEWS | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index dd5f731..4a81f31 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ # cartodb/Makefile EXTENSION = cartodb -EXTVERSION = 0.2.0dev +EXTVERSION = 0.2.0 SED = sed @@ -17,6 +17,7 @@ UPGRADABLE = \ unpackaged \ 0.1.0 \ 0.1.1 \ + 0.2.0dev \ $(EXTVERSION)next \ $(END) diff --git a/NEWS b/NEWS index a8baf16..cab3f59 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,4 @@ -0.2.0dev - 2014-MM-DD +0.2.0 - 2014-06-09 ------------------ Important changes: From 5a8b9021c8eb09a2b2d6b6bdfc358e1e504d96ee Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Mon, 9 Jun 2014 13:04:42 +0200 Subject: [PATCH 49/54] Prepare for 0.3.0 --- Makefile | 3 ++- NEWS | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 4a81f31..ffe3fde 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ # cartodb/Makefile EXTENSION = cartodb -EXTVERSION = 0.2.0 +EXTVERSION = 0.3.0dev SED = sed @@ -17,6 +17,7 @@ UPGRADABLE = \ unpackaged \ 0.1.0 \ 0.1.1 \ + 0.2.0 \ 0.2.0dev \ $(EXTVERSION)next \ $(END) diff --git a/NEWS b/NEWS index cab3f59..35c78d5 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,6 @@ +0.3.0dev - 2014-MM-DD +------------------ + 0.2.0 - 2014-06-09 ------------------ From 9dbe854df71b5d4cb1f3186c493b6ba30ea2dd32 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 11 Jun 2014 10:02:55 +0200 Subject: [PATCH 50/54] Do not force re-cartodbfication on CREATE FROM unpackaged --- Makefile | 4 +- NEWS | 7 ++++ util/create_from_unpackaged.sh | 73 +++++++++++++++++++++------------- 3 files changed, 54 insertions(+), 30 deletions(-) diff --git a/Makefile b/Makefile index 4a81f31..4c9012d 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ # cartodb/Makefile EXTENSION = cartodb -EXTVERSION = 0.2.0 +EXTVERSION = 0.2.1dev SED = sed @@ -17,7 +17,7 @@ UPGRADABLE = \ unpackaged \ 0.1.0 \ 0.1.1 \ - 0.2.0dev \ + 0.2.0 \ $(EXTVERSION)next \ $(END) diff --git a/NEWS b/NEWS index cab3f59..2b20877 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,10 @@ +0.2.1dev - 2014-MM-DD +------------------ + +Enhancements: + + - Do not force re-cartodbfication on CREATE FROM unpackaged + 0.2.0 - 2014-06-09 ------------------ diff --git a/util/create_from_unpackaged.sh b/util/create_from_unpackaged.sh index 7b3cf69..109c9ae 100755 --- a/util/create_from_unpackaged.sh +++ b/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} From 81bbfcbda8b33c068c8b662afc7c8fd413546b3b Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 11 Jun 2014 11:25:25 +0200 Subject: [PATCH 51/54] Drop useless DEFAULT specification in plpgsql variable declarations --- NEWS | 1 + scripts-available/CDB_DateToNumber.sql | 2 +- scripts-available/CDB_StringToDate.sql | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 2b20877..caca6d7 100644 --- a/NEWS +++ b/NEWS @@ -4,6 +4,7 @@ Enhancements: - Do not force re-cartodbfication on CREATE FROM unpackaged + - Drop useless DEFAULT specification in plpgsql variable declarations 0.2.0 - 2014-06-09 ------------------ diff --git a/scripts-available/CDB_DateToNumber.sql b/scripts-available/CDB_DateToNumber.sql index c879112..1b9a449 100644 --- a/scripts-available/CDB_DateToNumber.sql +++ b/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; diff --git a/scripts-available/CDB_StringToDate.sql b/scripts-available/CDB_StringToDate.sql index cbdf4ec..fa2db05 100644 --- a/scripts-available/CDB_StringToDate.sql +++ b/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; From afcc2498c822166ddd83d49290d7073cd198a390 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 11 Jun 2014 16:27:11 +0200 Subject: [PATCH 52/54] List plpythonu requirement first, so get pg_catalog scanned before public Since "plpythonu" is installed in "pg_catalog" schema, requiring it from cartodb.control adds the "pg_catalog" to the search_path. Schemas are added to search_path in order of appearence in the "requires" directive of the cartodb.contorl, right after the name of schema written in the "schema" directive. So this commit changes the resulting search_path from: cartodb,schema_triggers,public,pg_catalog To: cartodb,pg_catalog,schema_triggers,public Preventing presence of objects in the "public" schema from changing interpretation of function signatures and body from this extension. Spotted in the real world in presence of a "date" table changing intepretation of CDB_StringToDate function. --- NEWS | 1 + cartodb.control.in | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index caca6d7..497d9b1 100644 --- a/NEWS +++ b/NEWS @@ -5,6 +5,7 @@ Enhancements: - Do not force re-cartodbfication on CREATE FROM unpackaged - Drop useless DEFAULT specification in plpgsql variable declarations + - List plpythonu requirement first, so get pg_catalog scanned before public 0.2.0 - 2014-06-09 ------------------ diff --git a/cartodb.control.in b/cartodb.control.in index 3d783c7..c331343 100644 --- a/cartodb.control.in +++ b/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' From bc0966c46e813931cae243fefe1b3ff662ef6d94 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 11 Jun 2014 19:21:19 +0200 Subject: [PATCH 53/54] Do not add unique index on cartodb_id if already a primary key Closes #38 --- NEWS | 4 ++++ scripts-available/CDB_CartodbfyTable.sql | 2 +- test/CDB_CartodbfyTableTest.sql | 8 ++++++++ test/CDB_CartodbfyTableTest_expect | 4 ++++ 4 files changed, 17 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 497d9b1..960e027 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,10 @@ Enhancements: - Drop useless DEFAULT specification in plpgsql variable declarations - List plpythonu requirement first, so 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 ------------------ diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql index e69f187..ad56404 100644 --- a/scripts-available/CDB_CartodbfyTable.sql +++ b/scripts-available/CDB_CartodbfyTable.sql @@ -111,7 +111,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; diff --git a/test/CDB_CartodbfyTableTest.sql b/test/CDB_CartodbfyTableTest.sql index e68cb8b..74ece18 100644 --- a/test/CDB_CartodbfyTableTest.sql +++ b/test/CDB_CartodbfyTableTest.sql @@ -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 diff --git a/test/CDB_CartodbfyTableTest_expect b/test/CDB_CartodbfyTableTest_expect index c3dd05c..c5e79e6 100644 --- a/test/CDB_CartodbfyTableTest_expect +++ b/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 From 9afc59da5e1dc781f041ead3e43da7f3d36b0138 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 11 Jun 2014 16:34:06 +0200 Subject: [PATCH 54/54] Set release date for 0.2.1 --- Makefile | 2 +- NEWS | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 4c9012d..b734209 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ # cartodb/Makefile EXTENSION = cartodb -EXTVERSION = 0.2.1dev +EXTVERSION = 0.2.1 SED = sed diff --git a/NEWS b/NEWS index 960e027..5549f69 100644 --- a/NEWS +++ b/NEWS @@ -1,11 +1,11 @@ -0.2.1dev - 2014-MM-DD +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, so get pg_catalog scanned before public + - List plpythonu requirement first, to get pg_catalog scanned before public Bug fixes: