Merge pull request #306 from CartoDB/305-fix-unique-identifiers

305 fix unique identifiers

.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-9.5=9.5.2-3cdb3
-  - sudo apt-get -y install postgresql-server-dev-9.5=9.5.2-3cdb2
+  - 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-3cdb2
+  - 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
 

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
+  -- Accounts for the XXXX incremental suffix in case the identifier is taken
-  usedspace := 3;
+  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
+  -- Accounts for the XXXX incremental suffix in case the identifier is taken
-  usedspace := 3;
+  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;
 

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);

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

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;
 

test/CDB_HelperTest_expect

@@ -1,58 +1,58 @@
 relname
 prefix_relname
 relname_suffix
-largolargolargolargolargolargolargolargolargolargolargolargo
+largolargolargolargolargolargolargolargolargolargolargolarg
-prefix_largolargolargolargolargolargolargolargolargolargolar
+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
+largolargolargolargolargolargolargolargolargolargolargolarg
-prefix_largolargolargolargolargolargolargolargolargolargolar
+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ñ