From 8db73ae9bd88b93a98f7cb48ef234a9dd8f532a1 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Thu, 29 Jun 2017 15:58:17 +0200 Subject: [PATCH 1/4] Add test for faulty use case #305 --- test/CDB_CartodbfyTableTest.sql | 13 +++++++++++++ test/CDB_CartodbfyTableTest_expect | 3 +++ 2 files changed, 16 insertions(+) diff --git a/test/CDB_CartodbfyTableTest.sql b/test/CDB_CartodbfyTableTest.sql index 02c9039..3eef85d 100644 --- a/test/CDB_CartodbfyTableTest.sql +++ b/test/CDB_CartodbfyTableTest.sql @@ -372,6 +372,19 @@ SELECT column_name FROM information_schema.columns WHERE table_name = 'test' AND DROP TABLE test; SET client_min_messages TO error; +-- Unique identifier generation can break CDB_CartodbfyTable #305 +BEGIN; + DO $$ + BEGIN + FOR i IN 1..150 LOOP + EXECUTE 'CREATE TABLE untitled_table();'; + EXECUTE $query$SELECT CDB_CartodbfyTable('untitled_table');$query$; + EXECUTE 'ALTER TABLE untitled_table RENAME TO my_renamed_table_' || i; + END LOOP; + END; + $$; +ROLLBACK; + -- 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 de2e9ae..5fca9ec 100644 --- a/test/CDB_CartodbfyTableTest_expect +++ b/test/CDB_CartodbfyTableTest_expect @@ -147,5 +147,8 @@ NOTICE: Trying to recover data from _cartodb_id0 column DROP TABLE SET +BEGIN +DO +ROLLBACK DROP FUNCTION DROP FUNCTION From ffb779eb746956c25853b69b0dff2e9f50a429f8 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Thu, 29 Jun 2017 17:54:42 +0200 Subject: [PATCH 2/4] Increase search space of ids by 100x #305 --- scripts-available/CDB_Helper.sql | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/scripts-available/CDB_Helper.sql b/scripts-available/CDB_Helper.sql index 5540086..edf9196 100644 --- a/scripts-available/CDB_Helper.sql +++ b/scripts-available/CDB_Helper.sql @@ -1,3 +1,7 @@ +-- Create a sequence that belongs to the schema of the extension. +-- It will be used to generate unique identifiers within the + + -- UTF8 safe and length aware. Find a unique identifier with a given prefix -- and/or suffix and withing a schema. If a schema is not specified, the identifier -- is guaranteed to be unique for all schemas. @@ -15,8 +19,8 @@ DECLARE i INTEGER; BEGIN - -- Accounts for the _XX incremental suffix in case the identifier is taken - usedspace := 3; + -- Accounts for the XXXX incremental suffix in case the identifier is taken + usedspace := 4; usedspace := usedspace + coalesce(octet_length(prefix), 0); usedspace := usedspace + coalesce(octet_length(suffix), 0); @@ -31,7 +35,7 @@ BEGIN i := 0; origident := ident; - WHILE i < 100 LOOP + WHILE i < 10000 LOOP IF schema IS NOT NULL THEN SELECT c.relname, n.nspname INTO rec @@ -51,7 +55,7 @@ BEGIN RETURN ident; END IF; - ident := origident || '_' || i; + ident := origident || i; i := i + 1; END LOOP; @@ -76,8 +80,8 @@ DECLARE i INTEGER; BEGIN - -- Accounts for the _XX incremental suffix in case the identifier is taken - usedspace := 3; + -- Accounts for the XXXX incremental suffix in case the identifier is taken + usedspace := 4; usedspace := usedspace + coalesce(octet_length(prefix), 0); usedspace := usedspace + coalesce(octet_length(suffix), 0); @@ -92,7 +96,7 @@ BEGIN i := 0; origident := ident; - WHILE i < 100 LOOP + WHILE i < 10000 LOOP SELECT a.attname INTO rec FROM pg_class c @@ -106,7 +110,7 @@ BEGIN RETURN ident; END IF; - ident := origident || '_' || i; + ident := origident || i; i := i + 1; END LOOP; From 711c954d1aa6b640e13ac305f37b76684024273f Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Thu, 29 Jun 2017 18:53:24 +0200 Subject: [PATCH 3/4] Fix evil tests #305 Basically force generation of long name identifiers that conflict with first choice of another unique identifier. --- test/CDB_HelperTest.sql | 12 ++++++------ test/CDB_HelperTest_expect | 32 ++++++++++++++++---------------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/test/CDB_HelperTest.sql b/test/CDB_HelperTest.sql index 86f1e90..ac543f5 100644 --- a/test/CDB_HelperTest.sql +++ b/test/CDB_HelperTest.sql @@ -14,17 +14,17 @@ SELECT * FROM cartodb._CDB_Unique_Identifier(NULL, 'largolargolargolargolargolar SELECT * FROM cartodb._CDB_Unique_Identifier('prefix_', 'largolargolargolargolargolargolargolargolargolargolargolargolar', NULL); -- Test new identifier is found when name is taken from previous case -CREATE TABLE prefix_largolargolargolargolargolargolargolargolargolargolar (name text); +CREATE TABLE prefix_largolargolargolargolargolargolargolargolargolargola (name text); SELECT * FROM cartodb._CDB_Unique_Identifier('prefix_', 'largolargolargolargolargolargolargolargolargolargolargolargolar', NULL); -DROP TABLE prefix_largolargolargolargolargolargolargolargolargolargolar; +DROP TABLE prefix_largolargolargolargolargolargolargolargolargolargola; -- Test unique identifier creation with suffix with long length normal relname SELECT * FROM cartodb._CDB_Unique_Identifier(NULL, 'largolargolargolargolargolargolargolargolargolargolargolargolar', '_suffix'); -- Test new identifier is found when name is taken from previous case -CREATE TABLE largolargolargolargolargolargolargolargolargolargolar_suffix (name text); +CREATE TABLE largolargolargolargolargolargolargolargolargolargola_suffix (name text); SELECT * FROM cartodb._CDB_Unique_Identifier(NULL, 'largolargolargolargolargolargolargolargolargolargolargolargolar', '_suffix'); -DROP TABLE largolargolargolargolargolargolargolargolargolargolar_suffix; +DROP TABLE largolargolargolargolargolargolargolargolargolargola_suffix; -- Test unique identifier creation with normal length UTF8 relname SELECT * FROM cartodb._CDB_Unique_Identifier(NULL, 'piraña', NULL); @@ -72,7 +72,7 @@ SELECT * FROM cartodb._CDB_Unique_Column_Identifier('prefix_', 'largolargolargol DROP TABLE test; -- Test new identifier is found when name is taken from previous case -CREATE TABLE test (prefix_largolargolargolargolargolargolargolargolargolargolar text); +CREATE TABLE test (prefix_largolargolargolargolargolargolargolargolargolargola text); SELECT * FROM cartodb._CDB_Unique_Column_Identifier('prefix_', 'largolargolargolargolargolargolargolargolargolargolargolargolar', NULL, 'test'::regclass); DROP TABLE test; @@ -82,7 +82,7 @@ SELECT * FROM cartodb._CDB_Unique_Column_Identifier(NULL, 'largolargolargolargol DROP TABLE test; -- Test new identifier is found when name is taken from previous case -CREATE TABLE test (largolargolargolargolargolargolargolargolargolargolar_suffix text); +CREATE TABLE test (largolargolargolargolargolargolargolargolargolargola_suffix text); SELECT * FROM cartodb._CDB_Unique_Column_Identifier(NULL, 'largolargolargolargolargolargolargolargolargolargolargolargolar', '_suffix', 'test'::regclass); DROP TABLE test; diff --git a/test/CDB_HelperTest_expect b/test/CDB_HelperTest_expect index 1c1acf2..10a8d48 100644 --- a/test/CDB_HelperTest_expect +++ b/test/CDB_HelperTest_expect @@ -1,58 +1,58 @@ relname prefix_relname relname_suffix -largolargolargolargolargolargolargolargolargolargolargolargo -prefix_largolargolargolargolargolargolargolargolargolargolar +largolargolargolargolargolargolargolargolargolargolargolarg +prefix_largolargolargolargolargolargolargolargolargolargola CREATE TABLE -prefix_largolargolargolargolargolargolargolargolargolargolar_0 +prefix_largolargolargolargolargolargolargolargolargolargola0 DROP TABLE -largolargolargolargolargolargolargolargolargolargolar_suffix +largolargolargolargolargolargolargolargolargolargola_suffix CREATE TABLE -largolargolargolargolargolargolargolargolargolargolar_suffix_0 +largolargolargolargolargolargolargolargolargolargola_suffix0 DROP TABLE piraña prefix_piraña piraña_suffix -piñaácidpiñaácidpiñaácidpiñaácidpiñaácidpiñaácid +piñaácidpiñaácidpiñaácidpiñaácidpiñaácidpiñaáci prefix_piñaácidpiñaácidpiñaácidpiñaácidpiñaácidpi CREATE TABLE -prefix_piñaácidpiñaácidpiñaácidpiñaácidpiñaácidpi_0 +prefix_piñaácidpiñaácidpiñaácidpiñaácidpiñaácidpi0 DROP TABLE piñaácidpiñaácidpiñaácidpiñaácidpiñaácidpi_suffix CREATE TABLE -piñaácidpiñaácidpiñaácidpiñaácidpiñaácidpi_suffix_0 +piñaácidpiñaácidpiñaácidpiñaácidpiñaácidpi_suffix0 DROP TABLE CREATE TABLE colname prefix_colname colname_suffix -largolargolargolargolargolargolargolargolargolargolargolargo -prefix_largolargolargolargolargolargolargolargolargolargolar +largolargolargolargolargolargolargolargolargolargolargolarg +prefix_largolargolargolargolargolargolargolargolargolargola DROP TABLE CREATE TABLE -prefix_largolargolargolargolargolargolargolargolargolargolar_0 +prefix_largolargolargolargolargolargolargolargolargolargola0 DROP TABLE CREATE TABLE -largolargolargolargolargolargolargolargolargolargolar_suffix +largolargolargolargolargolargolargolargolargolargola_suffix DROP TABLE CREATE TABLE -largolargolargolargolargolargolargolargolargolargolar_suffix_0 +largolargolargolargolargolargolargolargolargolargola_suffix0 DROP TABLE CREATE TABLE piraña prefix_piraña piraña_suffix -piñaácidpiñaácidpiñaácidpiñaácidpiñaácidpiñaácid +piñaácidpiñaácidpiñaácidpiñaácidpiñaácidpiñaáci prefix_piñaácidpiñaácidpiñaácidpiñaácidpiñaácidpi DROP TABLE CREATE TABLE -prefix_piñaácidpiñaácidpiñaácidpiñaácidpiñaácidpi_0 +prefix_piñaácidpiñaácidpiñaácidpiñaácidpiñaácidpi0 DROP TABLE CREATE TABLE piñaácidpiñaácidpiñaácidpiñaácidpiñaácidpi_suffix DROP TABLE CREATE TABLE -piñaácidpiñaácidpiñaácidpiñaácidpiñaácidpi_suffix_0 +piñaácidpiñaácidpiñaácidpiñaácidpiñaácidpi_suffix0 DROP TABLE pira pirañ From c379946c95594e2dd41bf9a38fa7307d64bf866c Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Fri, 30 Jun 2017 12:52:26 +0200 Subject: [PATCH 4/4] Update dependencies of travis script #305 --- .travis.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index f608433..2ee6e90 100644 --- a/.travis.yml +++ b/.travis.yml @@ -26,9 +26,9 @@ before_install: - sudo apt-get -y remove --purge postgis-2.2 - sudo apt-get -y autoremove - - sudo apt-get -y install postgresql-9.5=9.5.2-3cdb2 - - sudo apt-get -y install postgresql-server-dev-9.5=9.5.2-3cdb2 - - sudo apt-get -y install postgresql-plpython-9.5=9.5.2-3cdb2 + - sudo apt-get -y install postgresql-9.5=9.5.2-3cdb3 + - sudo apt-get -y install postgresql-server-dev-9.5=9.5.2-3cdb3 + - sudo apt-get -y install postgresql-plpython-9.5=9.5.2-3cdb3 - sudo apt-get -y install postgresql-9.5-postgis-scripts=2.2.2.0-cdb2 - sudo apt-get -y install postgresql-9.5-postgis-2.2=2.2.2.0-cdb2