From 32307ceef077a8eef8f06dabe733b002d833808a Mon Sep 17 00:00:00 2001
From: Carla Iriberri <iriberrig@gmail.com>
Date: Tue, 1 Mar 2016 15:24:40 +0100
Subject: [PATCH 01/15] Add support to detect string cartodb_id columns

---
 scripts-available/CDB_CartodbfyTable.sql | 65 +++++++++++++++---------
 1 file changed, 41 insertions(+), 24 deletions(-)

diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql
index 9700696..e9ddb7b 100644
--- a/scripts-available/CDB_CartodbfyTable.sql
+++ b/scripts-available/CDB_CartodbfyTable.sql
@@ -499,40 +499,57 @@ BEGIN
 
   -- Found something named right...
   IF FOUND THEN
-  
-    -- And it's an integer column...
-    IF rec.atttypid IN (20,21,23) THEN
-          
+
+    -- And it's an integer or varchar column...
+    IF rec.atttypid IN (20,21,23,1043) THEN
+
       -- And it's a unique primary key! Done!
       IF (rec.indisprimary OR rec.indisunique) AND rec.attnotnull THEN
         RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', Format('found good ''%s''', const.pkey);
         RETURN true;
 
-      -- Check and see if the column values are unique and not null, 
+      -- Check and see if the column values are unique and not null,
       -- if they are, we can use this column...
       ELSE
 
         -- Assume things are OK until proven otherwise...
         useable_key := true;
-      
-        BEGIN
-          sql := Format('ALTER TABLE %s ADD CONSTRAINT %s_pk PRIMARY KEY (%s)', reloid::text, const.pkey, const.pkey);
-          RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', sql;
-          EXECUTE sql;
-          EXCEPTION      
-          -- Failed unique check...
-          WHEN unique_violation THEN
-            RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', Format('column %s is not unique', const.pkey);
-            useable_key := false;
-          -- Failed not null check...
-          WHEN not_null_violation THEN
-            RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', Format('column %s contains nulls', const.pkey);
-            useable_key := false;
-          -- Other fatal error
-          WHEN others THEN
-            PERFORM _CDB_Error(sql, '_CDB_Has_Usable_Primary_ID');          
-        END;
-  
+
+        -- If the column is varchar, try to cast it to integer
+        IF rec.atttypid IN (1043) THEN
+          RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): Found text column %', rec.atttypid;
+
+          BEGIN
+            sql := Format('ALTER TABLE %s ALTER cartodb_id TYPE int USING %I::integer', reloid::text, rec.attname);
+            RAISE DEBUG 'Running %', sql;
+            EXECUTE sql;
+            EXCEPTION
+            WHEN others THEN
+              RAISE DEBUG 'Column % of type text is not a valid integer column', rec.attname;
+              useable_key := false;
+          END;
+
+        END IF;
+
+        IF useable_key THEN
+          BEGIN
+            sql := Format('ALTER TABLE %s ADD CONSTRAINT %s_pk PRIMARY KEY (%s)', reloid::text, const.pkey, const.pkey);
+            RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', sql;
+            EXECUTE sql;
+            EXCEPTION
+            -- Failed unique check...
+            WHEN unique_violation THEN
+              RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', Format('column %s is not unique', const.pkey);
+              useable_key := false;
+            -- Failed not null check...
+            WHEN not_null_violation THEN
+              RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', Format('column %s contains nulls', const.pkey);
+              useable_key := false;
+            -- Other fatal error
+            WHEN others THEN
+              PERFORM _CDB_Error(sql, '_CDB_Has_Usable_Primary_ID');
+          END;
+        END IF;
         -- Clean up test constraint
         IF useable_key THEN
           PERFORM _CDB_SQL(Format('ALTER TABLE %s DROP CONSTRAINT %s_pk', reloid::text, const.pkey));

From 1198454046d569c4d12d3d187cc543a41df677c8 Mon Sep 17 00:00:00 2001
From: Carla Iriberri <iriberrig@gmail.com>
Date: Tue, 1 Mar 2016 17:43:21 +0100
Subject: [PATCH 02/15] Better exception handling

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

diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql
index e9ddb7b..3da1cf5 100644
--- a/scripts-available/CDB_CartodbfyTable.sql
+++ b/scripts-available/CDB_CartodbfyTable.sql
@@ -521,11 +521,14 @@ BEGIN
 
           BEGIN
             sql := Format('ALTER TABLE %s ALTER cartodb_id TYPE int USING %I::integer', reloid::text, rec.attname);
-            RAISE DEBUG 'Running %', sql;
+            RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): Running %', sql;
             EXECUTE sql;
             EXCEPTION
+            WHEN invalid_text_representation THEN
+              RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): Column % of type text is not a valid integer column', rec.attname;
+              useable_key := false;
             WHEN others THEN
-              RAISE DEBUG 'Column % of type text is not a valid integer column', rec.attname;
+              RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): Exception %', SQLSTATE;
               useable_key := false;
           END;
 

From 7559257d49ea70f1dfe526174c9716b71c82d8d5 Mon Sep 17 00:00:00 2001
From: Carla Iriberri <iriberrig@gmail.com>
Date: Tue, 1 Mar 2016 17:45:15 +0100
Subject: [PATCH 03/15] cartodb_id string tests

---
 test/CDB_CartodbfyTableTest.sql    | 26 ++++++++++++++++----------
 test/CDB_CartodbfyTableTest_expect | 12 ++++++++++++
 2 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/test/CDB_CartodbfyTableTest.sql b/test/CDB_CartodbfyTableTest.sql
index 08d1a17..e9ebd82 100644
--- a/test/CDB_CartodbfyTableTest.sql
+++ b/test/CDB_CartodbfyTableTest.sql
@@ -165,18 +165,24 @@ SELECT 'extent',ST_Extent(ST_SnapToGrid(the_geom,0.2)) FROM t;
 DROP TABLE t;
 
 -- INFO: disabled because cartodbfy does not longer consider text columns for primary ID
--- -- 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 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;
 
 -- INFO: disabled because cartodbfy does not longer consider text columns for primary ID
--- -- 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 text not casting
+CREATE TABLE t AS SELECT 'nan' as cartodb_id;
+SELECT CDB_CartodbfyTableCheck('t', 'uncasting text cartodb_id');
+select cartodb_id,cartodb_id_0 FROM t;
+DROP TABLE t;
+
+-- table with empty cartodb_id field of type text
+CREATE TABLE t AS SELECT null::text as cartodb_id;
+SELECT CDB_CartodbfyTableCheck('t', 'empty text cartodb_id');
+select cartodb_id,cartodb_id_0 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;
diff --git a/test/CDB_CartodbfyTableTest_expect b/test/CDB_CartodbfyTableTest_expect
index 0b7419f..206c1a3 100644
--- a/test/CDB_CartodbfyTableTest_expect
+++ b/test/CDB_CartodbfyTableTest_expect
@@ -28,6 +28,18 @@ trigger-protected the_geom cartodbfied fine
 extent|BOX(1 1,2 2)
 DROP TABLE
 SELECT 1
+text cartodb_id cartodbfied fine
+0
+DROP TABLE
+SELECT 1
+uncasting text cartodb_id cartodbfied fine
+1|nan
+DROP TABLE
+SELECT 1
+empty text cartodb_id cartodbfied fine
+1|
+DROP TABLE
+SELECT 1
 unsequenced cartodb_id cartodbfied fine
 1
 DROP TABLE

From 0d5f83b3c4db309e41863442416899b1dbe0903c Mon Sep 17 00:00:00 2001
From: Carla Iriberri <iriberrig@gmail.com>
Date: Tue, 1 Mar 2016 19:17:45 +0100
Subject: [PATCH 04/15] Rewriting function

---
 scripts-available/CDB_CartodbfyTable.sql | 58 +++++++++++++++++-------
 1 file changed, 42 insertions(+), 16 deletions(-)

diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql
index 3da1cf5..2e82e73 100644
--- a/scripts-available/CDB_CartodbfyTable.sql
+++ b/scripts-available/CDB_CartodbfyTable.sql
@@ -466,19 +466,33 @@ END;
 $$ LANGUAGE 'plpgsql';
 
 
+
+DO $$
+BEGIN
+  IF NOT EXISTS (SELECT 1 FROM pg_type WHERE typname = '_cdb_has_usable_primary_id_record') THEN
+    CREATE TYPE _cdb_has_usable_primary_id_record
+      AS (has_usable_primary_key boolean,
+        text_key_column boolean);
+    END IF;
+END$$;
+
 -- Find out if the table already has a usable primary key
 -- If the table has both a usable key and usable geometry
 -- we can no-op on the table copy and just ensure that the 
 -- indexes and triggers are in place
-CREATE OR REPLACE FUNCTION _CDB_Has_Usable_Primary_ID(reloid REGCLASS)
-  RETURNS BOOLEAN
+CREATE OR REPLACE FUNCTION cartodb._CDB_Has_Usable_Primary_ID(reloid REGCLASS)
+  RETURNS _cdb_has_usable_primary_id_record
 AS $$
 DECLARE
   rec RECORD;
   const RECORD;
+  idc RECORD;
   i INTEGER;
   sql TEXT;
   useable_key BOOLEAN = false;
+  has_usable_primary_key BOOLEAN = false;
+  -- In case cartodb_id is a text column
+  text_key_column BOOLEAN = false;
 BEGIN
 
   RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', 'entered function';
@@ -506,7 +520,8 @@ BEGIN
       -- And it's a unique primary key! Done!
       IF (rec.indisprimary OR rec.indisunique) AND rec.attnotnull THEN
         RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', Format('found good ''%s''', const.pkey);
-        RETURN true;
+        SELECT true as has_usable_primary_key, text_key_column INTO idc;
+        RETURN idc;
 
       -- Check and see if the column values are unique and not null,
       -- if they are, we can use this column...
@@ -520,9 +535,10 @@ BEGIN
           RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): Found text column %', rec.atttypid;
 
           BEGIN
-            sql := Format('ALTER TABLE %s ALTER cartodb_id TYPE int USING %I::integer', reloid::text, rec.attname);
+            sql := Format('SELECT %I::integer FROM %s', rec.attname, reloid::text);
             RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): Running %', sql;
             EXECUTE sql;
+            text_key_column := true
             EXCEPTION
             WHEN invalid_text_representation THEN
               RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): Column % of type text is not a valid integer column', rec.attname;
@@ -571,7 +587,8 @@ BEGIN
         
         END IF;
         
-        return useable_key;
+        SELECT useable_key as has_usable_primary_key, text_key_column INTO idc;
+        RETURN idc;
 
       END IF;
     
@@ -604,7 +621,8 @@ BEGIN
     -- Yes! Ok, rename it.
     IF FOUND THEN
       PERFORM _CDB_SQL(Format('ALTER TABLE %s RENAME COLUMN %s TO %s', reloid::text, rec.attname, const.pkey),'_CDB_Has_Usable_Primary_ID');
-      RETURN true;
+      SELECT true as has_usable_primary_key, text_key_column INTO idc;
+      RETURN idc;
     ELSE
       RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', 
         Format('found no useful column for ''%s''', const.pkey);
@@ -615,8 +633,8 @@ BEGIN
   RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', 'function complete';
 
   -- Didn't find re-usable key, so return FALSE
-  RETURN false;
-
+  SELECT false as has_usable_primary_key, text_key_column INTO idc;
+  RETURN idc;
 END;
 $$ LANGUAGE 'plpgsql';
 
@@ -819,7 +837,7 @@ $$ LANGUAGE 'plpgsql';
 -- a "good" one, and the same for the geometry columns. If all the required
 -- columns are in place already, it no-ops and just renames the table to 
 -- the destination if necessary.
-CREATE OR REPLACE FUNCTION _CDB_Rewrite_Table(reloid REGCLASS, destschema TEXT DEFAULT NULL)
+CREATE OR REPLACE FUNCTION cartodb._CDB_Rewrite_Table(reloid REGCLASS, destschema TEXT DEFAULT NULL)
 RETURNS BOOLEAN
 AS $$
 DECLARE
@@ -840,13 +858,13 @@ DECLARE
 
   rec RECORD;
   const RECORD;
+  idc RECORD;
   gc RECORD;
   sql TEXT;
   str TEXT;
   table_srid INTEGER;
   geom_srid INTEGER;
   
-  has_usable_primary_key BOOLEAN;
   has_usable_pk_sequence BOOLEAN;
   
 BEGIN
@@ -870,15 +888,16 @@ BEGIN
   -- See if there is a primary key column we need to carry along to the
   -- new table. If this is true, it implies there is an indexed
   -- primary key of integer type named (by default) cartodb_id
-  SELECT _CDB_Has_Usable_Primary_ID(reloid)
-  INTO STRICT has_usable_primary_key;
+  SELECT has_usable_primary_key, text_key_column
+  FROM _CDB_Has_Usable_Primary_ID(reloid)
+  INTO STRICT idc;
 
-  RAISE DEBUG 'CDB(_CDB_Rewrite_Table): has_usable_primary_key %', has_usable_primary_key;
+  RAISE DEBUG 'CDB(_CDB_Rewrite_Table): has_usable_primary_key %', idc.has_usable_primary_key;
 
   -- See if the candidate primary key column has a sequence for default
   -- values. No usable pk implies has_usable_pk_sequence = false.
   has_usable_pk_sequence := false;
-  IF has_usable_primary_key THEN
+  IF idc.has_usable_primary_key THEN
     SELECT _CDB_Has_Usable_PK_Sequence(reloid)
     INTO STRICT has_usable_pk_sequence;
   END IF;
@@ -913,9 +932,16 @@ BEGIN
   -- We can only avoid a rewrite if both the key and 
   -- geometry are usable
 
+  -- If cartodb_id column is of type text, cast it
+  IF idc.text_key_column THEN
+    sql := Format('ALTER TABLE %s ALTER cartodb_id TYPE int USING %I::integer', reloid::text, 'cartodb_id');
+    PERFORM _CDB_SQL(sql,'_CDB_Rewrite_Table');
+  END IF;
+
+  RAISE DEBUG 'idc %', idc;
   -- No table re-write is required, BUT a rename is required to
   -- a destination schema, so do that now
-  IF has_usable_primary_key AND has_usable_pk_sequence AND gc.has_usable_geoms THEN
+  IF idc.has_usable_primary_key AND has_usable_pk_sequence AND gc.has_usable_geoms THEN
     IF  destschema != relschema THEN
 
       RAISE DEBUG 'CDB(_CDB_Rewrite_Table): perfect table needs to be moved to schema (%)', destschema;
@@ -953,7 +979,7 @@ BEGIN
   sql := Format('CREATE TABLE %s AS SELECT ', copyname);
 
   -- Add cartodb ID!
-  IF has_usable_primary_key THEN
+  IF idc.has_usable_primary_key THEN
     sql := sql || const.pkey;
   ELSE
     sql := sql || 'nextval(''' || destseq || ''') AS ' || const.pkey;

From a18cbeb2cdada33258a6348eacb6cedf1b603a60 Mon Sep 17 00:00:00 2001
From: Carla Iriberri <iriberrig@gmail.com>
Date: Wed, 2 Mar 2016 14:58:53 +0100
Subject: [PATCH 05/15] Third iteration, expect a viable cartodb_id

---
 scripts-available/CDB_CartodbfyTable.sql | 199 ++++++++---------------
 1 file changed, 68 insertions(+), 131 deletions(-)

diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql
index 2e82e73..301c03b 100644
--- a/scripts-available/CDB_CartodbfyTable.sql
+++ b/scripts-available/CDB_CartodbfyTable.sql
@@ -466,33 +466,20 @@ END;
 $$ LANGUAGE 'plpgsql';
 
 
-
-DO $$
-BEGIN
-  IF NOT EXISTS (SELECT 1 FROM pg_type WHERE typname = '_cdb_has_usable_primary_id_record') THEN
-    CREATE TYPE _cdb_has_usable_primary_id_record
-      AS (has_usable_primary_key boolean,
-        text_key_column boolean);
-    END IF;
-END$$;
-
 -- Find out if the table already has a usable primary key
 -- If the table has both a usable key and usable geometry
 -- we can no-op on the table copy and just ensure that the 
 -- indexes and triggers are in place
-CREATE OR REPLACE FUNCTION cartodb._CDB_Has_Usable_Primary_ID(reloid REGCLASS)
-  RETURNS _cdb_has_usable_primary_id_record
+CREATE OR REPLACE FUNCTION _CDB_Has_Usable_Primary_ID(reloid REGCLASS)
+  RETURNS BOOLEAN
 AS $$
 DECLARE
   rec RECORD;
   const RECORD;
-  idc RECORD;
   i INTEGER;
   sql TEXT;
   useable_key BOOLEAN = false;
   has_usable_primary_key BOOLEAN = false;
-  -- In case cartodb_id is a text column
-  text_key_column BOOLEAN = false;
 BEGIN
 
   RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', 'entered function';
@@ -503,138 +490,96 @@ BEGIN
   -- Do we already have a properly named column?
   SELECT a.attname, i.indisprimary, i.indisunique, a.attnotnull, a.atttypid
   INTO rec
-  FROM pg_class c 
-  JOIN pg_attribute a ON a.attrelid = c.oid 
+  FROM pg_class c
+  JOIN pg_attribute a ON a.attrelid = c.oid
   JOIN pg_type t ON a.atttypid = t.oid
   LEFT JOIN pg_index i ON c.oid = i.indrelid AND a.attnum = ANY(i.indkey)
-  WHERE c.oid = reloid 
+  WHERE c.oid = reloid
   AND NOT a.attisdropped
   AND a.attname = const.pkey;
 
   -- Found something named right...
   IF FOUND THEN
 
-    -- And it's an integer or varchar column...
-    IF rec.atttypid IN (20,21,23,1043) THEN
+    -- And it's a unique primary key! Done!
+    IF (rec.indisprimary OR rec.indisunique) AND rec.attnotnull THEN
+      RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', Format('found good ''%s''', const.pkey);
+      RETURN true;
 
-      -- And it's a unique primary key! Done!
-      IF (rec.indisprimary OR rec.indisunique) AND rec.attnotnull THEN
-        RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', Format('found good ''%s''', const.pkey);
-        SELECT true as has_usable_primary_key, text_key_column INTO idc;
-        RETURN idc;
+    -- Check and see if the column values are unique and not null,
+    -- if they are, we can use this column...
+    ELSE
 
-      -- Check and see if the column values are unique and not null,
-      -- if they are, we can use this column...
+      -- Assume things are OK until proven otherwise...
+      useable_key := true;
+      BEGIN
+        sql := Format('ALTER TABLE %s ADD CONSTRAINT %s_pk PRIMARY KEY (%s)', reloid::text, const.pkey, const.pkey);
+        RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', sql;
+        EXECUTE sql;
+        EXCEPTION
+        -- Failed unique check...
+        WHEN unique_violation THEN
+          RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', Format('column %s is not unique', const.pkey);
+          useable_key := false;
+        -- Failed not null check...
+        WHEN not_null_violation THEN
+          RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', Format('column %s contains nulls', const.pkey);
+          useable_key := false;
+        -- Other fatal error
+        WHEN others THEN
+          PERFORM _CDB_Error(sql, '_CDB_Has_Usable_Primary_ID');
+      END;
+
+      -- Clean up test constraint
+      IF useable_key THEN
+        PERFORM _CDB_SQL(Format('ALTER TABLE %s DROP CONSTRAINT %s_pk', reloid::text, const.pkey));
+
+      -- Move non-unique column out of the way
       ELSE
 
-        -- Assume things are OK until proven otherwise...
-        useable_key := true;
+        RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %',
+          Format('found non-unique ''%s'', renaming it', const.pkey);
 
-        -- If the column is varchar, try to cast it to integer
-        IF rec.atttypid IN (1043) THEN
-          RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): Found text column %', rec.atttypid;
-
-          BEGIN
-            sql := Format('SELECT %I::integer FROM %s', rec.attname, reloid::text);
-            RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): Running %', sql;
-            EXECUTE sql;
-            text_key_column := true
-            EXCEPTION
-            WHEN invalid_text_representation THEN
-              RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): Column % of type text is not a valid integer column', rec.attname;
-              useable_key := false;
-            WHEN others THEN
-              RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): Exception %', SQLSTATE;
-              useable_key := false;
-          END;
-
-        END IF;
-
-        IF useable_key THEN
-          BEGIN
-            sql := Format('ALTER TABLE %s ADD CONSTRAINT %s_pk PRIMARY KEY (%s)', reloid::text, const.pkey, const.pkey);
-            RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', sql;
-            EXECUTE sql;
-            EXCEPTION
-            -- Failed unique check...
-            WHEN unique_violation THEN
-              RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', Format('column %s is not unique', const.pkey);
-              useable_key := false;
-            -- Failed not null check...
-            WHEN not_null_violation THEN
-              RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', Format('column %s contains nulls', const.pkey);
-              useable_key := false;
-            -- Other fatal error
-            WHEN others THEN
-              PERFORM _CDB_Error(sql, '_CDB_Has_Usable_Primary_ID');
-          END;
-        END IF;
-        -- Clean up test constraint
-        IF useable_key THEN
-          PERFORM _CDB_SQL(Format('ALTER TABLE %s DROP CONSTRAINT %s_pk', reloid::text, const.pkey));
-
-        -- Move non-unique column out of the way
-        ELSE
-        
-          RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %',
-            Format('found non-unique ''%s'', renaming it', const.pkey);
-
-          PERFORM _CDB_SQL(
-            Format('ALTER TABLE %s RENAME COLUMN %s TO %I',
-              reloid::text, rec.attname,
-              cartodb._CDB_Unique_Column_Identifier(NULL, const.pkey, NULL, reloid)),
-            '_CDB_Has_Usable_Primary_ID');
-        
-        END IF;
-        
-        SELECT useable_key as has_usable_primary_key, text_key_column INTO idc;
-        RETURN idc;
+        PERFORM _CDB_SQL(
+          Format('ALTER TABLE %s RENAME COLUMN %s TO %I',
+            reloid::text, rec.attname,
+            cartodb._CDB_Unique_Column_Identifier(NULL, const.pkey, NULL, reloid)),
+          '_CDB_Has_Usable_Primary_ID');
 
       END IF;
-    
-    -- It's not an integer column, we have to rename it
-    ELSE
-  
-      RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', 
-        Format('found non-integer ''%s'', renaming it', const.pkey);
 
-      PERFORM _CDB_SQL(
-        Format('ALTER TABLE %s RENAME COLUMN %s TO %I',
-                reloid::text, rec.attname, cartodb._CDB_Unique_Column_Identifier(NULL, const.pkey, NULL, reloid)),
-                '_CDB_Has_Usable_Primary_ID');
-    
+      RETURN useable_key;
+
     END IF;
-    
+
   -- There's no column there named pkey
   ELSE
 
-    -- Is there another suitable primary key already?
+    -- Is there another integer suitable primary key already?
     SELECT a.attname
     INTO rec
     FROM pg_class c 
-    JOIN pg_attribute a ON a.attrelid = c.oid 
+    JOIN pg_attribute a ON a.attrelid = c.oid
     JOIN pg_type t ON a.atttypid = t.oid
     LEFT JOIN pg_index i ON c.oid = i.indrelid AND a.attnum = ANY(i.indkey)
     WHERE c.oid = reloid AND NOT a.attisdropped
     AND i.indisprimary AND i.indisunique AND a.attnotnull AND a.atttypid IN (20,21,23);
-  
+
     -- Yes! Ok, rename it.
     IF FOUND THEN
       PERFORM _CDB_SQL(Format('ALTER TABLE %s RENAME COLUMN %s TO %s', reloid::text, rec.attname, const.pkey),'_CDB_Has_Usable_Primary_ID');
-      SELECT true as has_usable_primary_key, text_key_column INTO idc;
-      RETURN idc;
+      RETURN true;
     ELSE
-      RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', 
+      RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %',
         Format('found no useful column for ''%s''', const.pkey);
     END IF;
-  
+
   END IF;
-  
+
   RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', 'function complete';
 
   -- Didn't find re-usable key, so return FALSE
-  SELECT false as has_usable_primary_key, text_key_column INTO idc;
-  RETURN idc;
+  RETURN false;
 END;
 $$ LANGUAGE 'plpgsql';
 
@@ -837,7 +782,7 @@ $$ LANGUAGE 'plpgsql';
 -- a "good" one, and the same for the geometry columns. If all the required
 -- columns are in place already, it no-ops and just renames the table to 
 -- the destination if necessary.
-CREATE OR REPLACE FUNCTION cartodb._CDB_Rewrite_Table(reloid REGCLASS, destschema TEXT DEFAULT NULL)
+CREATE OR REPLACE FUNCTION _CDB_Rewrite_Table(reloid REGCLASS, destschema TEXT DEFAULT NULL)
 RETURNS BOOLEAN
 AS $$
 DECLARE
@@ -858,15 +803,15 @@ DECLARE
 
   rec RECORD;
   const RECORD;
-  idc RECORD;
   gc RECORD;
   sql TEXT;
   str TEXT;
   table_srid INTEGER;
   geom_srid INTEGER;
-  
+
+  has_usable_primary_key BOOLEAN;
   has_usable_pk_sequence BOOLEAN;
-  
+
 BEGIN
 
   RAISE DEBUG 'CDB(_CDB_Rewrite_Table): %', 'entered function';
@@ -877,7 +822,7 @@ BEGIN
   -- Save the raw schema/table names for later
   SELECT n.nspname, c.relname, c.relname
   INTO STRICT relschema, relname, destname
-  FROM pg_class c JOIN pg_namespace n ON c.relnamespace = n.oid 
+  FROM pg_class c JOIN pg_namespace n ON c.relnamespace = n.oid
   WHERE c.oid = reloid;
 
   -- Default the destination to current schema if unspecified
@@ -888,16 +833,15 @@ BEGIN
   -- See if there is a primary key column we need to carry along to the
   -- new table. If this is true, it implies there is an indexed
   -- primary key of integer type named (by default) cartodb_id
-  SELECT has_usable_primary_key, text_key_column
-  FROM _CDB_Has_Usable_Primary_ID(reloid)
-  INTO STRICT idc;
+  SELECT _CDB_Has_Usable_Primary_ID(reloid)
+  INTO STRICT has_usable_primary_key;
 
-  RAISE DEBUG 'CDB(_CDB_Rewrite_Table): has_usable_primary_key %', idc.has_usable_primary_key;
+  RAISE DEBUG 'CDB(_CDB_Rewrite_Table): has_usable_primary_key %', has_usable_primary_key;
 
   -- See if the candidate primary key column has a sequence for default
   -- values. No usable pk implies has_usable_pk_sequence = false.
   has_usable_pk_sequence := false;
-  IF idc.has_usable_primary_key THEN
+  IF has_usable_primary_key THEN
     SELECT _CDB_Has_Usable_PK_Sequence(reloid)
     INTO STRICT has_usable_pk_sequence;
   END IF;
@@ -929,19 +873,12 @@ BEGIN
 
   RAISE DEBUG 'CDB(_CDB_Rewrite_Table): has_usable_geoms %', gc.has_usable_geoms;
 
-  -- We can only avoid a rewrite if both the key and 
+  -- We can only avoid a rewrite if both the key and
   -- geometry are usable
 
-  -- If cartodb_id column is of type text, cast it
-  IF idc.text_key_column THEN
-    sql := Format('ALTER TABLE %s ALTER cartodb_id TYPE int USING %I::integer', reloid::text, 'cartodb_id');
-    PERFORM _CDB_SQL(sql,'_CDB_Rewrite_Table');
-  END IF;
-
-  RAISE DEBUG 'idc %', idc;
   -- No table re-write is required, BUT a rename is required to
   -- a destination schema, so do that now
-  IF idc.has_usable_primary_key AND has_usable_pk_sequence AND gc.has_usable_geoms THEN
+  IF has_usable_primary_key AND has_usable_pk_sequence AND gc.has_usable_geoms THEN
     IF  destschema != relschema THEN
 
       RAISE DEBUG 'CDB(_CDB_Rewrite_Table): perfect table needs to be moved to schema (%)', destschema;
@@ -979,8 +916,8 @@ BEGIN
   sql := Format('CREATE TABLE %s AS SELECT ', copyname);
 
   -- Add cartodb ID!
-  IF idc.has_usable_primary_key THEN
-    sql := sql || const.pkey;
+  IF has_usable_primary_key THEN
+    sql := sql || const.pkey || '::bigint ';
   ELSE
     sql := sql || 'nextval(''' || destseq || ''') AS ' || const.pkey;
   END IF;

From 7e2193d3dbb9427e09288f5729187da63ca484ce Mon Sep 17 00:00:00 2001
From: Carla Iriberri <iriberrig@gmail.com>
Date: Wed, 2 Mar 2016 15:52:57 +0100
Subject: [PATCH 06/15] Fix test expectation

---
 test/CDB_CartodbfyTableTest_expect | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/CDB_CartodbfyTableTest_expect b/test/CDB_CartodbfyTableTest_expect
index 206c1a3..da6e0f5 100644
--- a/test/CDB_CartodbfyTableTest_expect
+++ b/test/CDB_CartodbfyTableTest_expect
@@ -29,7 +29,7 @@ extent|BOX(1 1,2 2)
 DROP TABLE
 SELECT 1
 text cartodb_id cartodbfied fine
-0
+5
 DROP TABLE
 SELECT 1
 uncasting text cartodb_id cartodbfied fine

From a80664e72a357a36baca88a2daa5b448b749aa8f Mon Sep 17 00:00:00 2001
From: Carla Iriberri <iriberrig@gmail.com>
Date: Wed, 2 Mar 2016 15:53:20 +0100
Subject: [PATCH 07/15] Delete outdated comments

---
 test/CDB_CartodbfyTableTest.sql | 2 --
 1 file changed, 2 deletions(-)

diff --git a/test/CDB_CartodbfyTableTest.sql b/test/CDB_CartodbfyTableTest.sql
index e9ebd82..06c557b 100644
--- a/test/CDB_CartodbfyTableTest.sql
+++ b/test/CDB_CartodbfyTableTest.sql
@@ -164,14 +164,12 @@ SELECT CDB_CartodbfyTableCheck('t', 'trigger-protected the_geom');
 SELECT 'extent',ST_Extent(ST_SnapToGrid(the_geom,0.2)) FROM t;
 DROP TABLE t;
 
--- INFO: disabled because cartodbfy does not longer consider text columns for primary ID
 -- 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;
 
--- INFO: disabled because cartodbfy does not longer consider text columns for primary ID
 -- 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');

From 76a2cb91329332e6e5f9b69c9d98a63ebc898d59 Mon Sep 17 00:00:00 2001
From: Carla Iriberri <iriberrig@gmail.com>
Date: Wed, 2 Mar 2016 17:36:45 +0100
Subject: [PATCH 08/15] Improve exception error message

---
 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 301c03b..d6fba32 100644
--- a/scripts-available/CDB_CartodbfyTable.sql
+++ b/scripts-available/CDB_CartodbfyTable.sql
@@ -527,7 +527,7 @@ BEGIN
           useable_key := false;
         -- Other fatal error
         WHEN others THEN
-          PERFORM _CDB_Error(sql, '_CDB_Has_Usable_Primary_ID');
+          PERFORM _CDB_Error(sql, Format('_CDB_Has_Usable_Primary_ID: %s', SQLERRM));
       END;
 
       -- Clean up test constraint

From 03ff0365fda5aacccd6a2dd408091e0ae32a8310 Mon Sep 17 00:00:00 2001
From: Carla Iriberri <iriberrig@gmail.com>
Date: Wed, 2 Mar 2016 17:41:02 +0100
Subject: [PATCH 09/15] Change exception message and edit tests

---
 test/CDB_CartodbfyTableTest.sql    |  3 +--
 test/CDB_CartodbfyTableTest_expect | 19 ++++++++++++++++---
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/test/CDB_CartodbfyTableTest.sql b/test/CDB_CartodbfyTableTest.sql
index 06c557b..f491aca 100644
--- a/test/CDB_CartodbfyTableTest.sql
+++ b/test/CDB_CartodbfyTableTest.sql
@@ -171,9 +171,8 @@ select cartodb_id/2 FROM t;
 DROP TABLE t;
 
 -- table with existing cartodb_id field of type text not casting
-CREATE TABLE t AS SELECT 'nan' as cartodb_id;
+CREATE TABLE t AS SELECT 'nan'::text as cartodb_id;
 SELECT CDB_CartodbfyTableCheck('t', 'uncasting text cartodb_id');
-select cartodb_id,cartodb_id_0 FROM t;
 DROP TABLE t;
 
 -- table with empty cartodb_id field of type text
diff --git a/test/CDB_CartodbfyTableTest_expect b/test/CDB_CartodbfyTableTest_expect
index da6e0f5..e144354 100644
--- a/test/CDB_CartodbfyTableTest_expect
+++ b/test/CDB_CartodbfyTableTest_expect
@@ -32,8 +32,13 @@ text cartodb_id cartodbfied fine
 5
 DROP TABLE
 SELECT 1
-uncasting text cartodb_id cartodbfied fine
-1|nan
+ERROR:  CDB(_CDB_Rewrite_Table:22P02:invalid input syntax for integer: "nan"): CREATE TABLE public.t_0 AS SELECT cartodb_id::bigint ,NULL::geometry(Geometry,4326) AS the_geom,NULL::geometry(Geometry,3857) AS the_geom_webmercator FROM t
+CONTEXT:  SQL statement "SELECT _CDB_SQL(sql, '_CDB_Rewrite_Table')"
+PL/pgSQL function _cdb_rewrite_table(regclass,text) line 295 at PERFORM
+SQL statement "SELECT _CDB_Rewrite_Table(reloid, destschema)"
+PL/pgSQL function cdb_cartodbfytable(text,regclass) line 51 at PERFORM
+SQL statement "SELECT CDB_CartodbfyTable('public', tabname)"
+PL/pgSQL function cdb_cartodbfytablecheck(regclass,text) line 31 at PERFORM
 DROP TABLE
 SELECT 1
 empty text cartodb_id cartodbfied fine
@@ -89,7 +94,15 @@ Table with both the_geom and wkb_geometry #141 cartodbfied fine
 DROP TABLE
 CREATE TABLE
 INSERT 0 1
-Many colliding columns #141 cartodbfied fine
+ERROR:  CDB(_CDB_Has_Usable_Primary_ID: multiple primary keys for table "many_colliding_columns" are not allowed): ALTER TABLE many_colliding_columns ADD CONSTRAINT cartodb_id_pk PRIMARY KEY (cartodb_id)
+CONTEXT:  SQL statement "SELECT _CDB_Error(sql, Format('_CDB_Has_Usable_Primary_ID: %s', SQLERRM))"
+PL/pgSQL function _cdb_has_usable_primary_id(regclass) line 56 at PERFORM
+SQL statement "SELECT _CDB_Has_Usable_Primary_ID(reloid)"
+PL/pgSQL function _cdb_rewrite_table(regclass,text) line 50 at SQL statement
+SQL statement "SELECT _CDB_Rewrite_Table(reloid, destschema)"
+PL/pgSQL function cdb_cartodbfytable(text,regclass) line 51 at PERFORM
+SQL statement "SELECT CDB_CartodbfyTable('public', tabname)"
+PL/pgSQL function cdb_cartodbfytablecheck(regclass,text) line 31 at PERFORM
 DROP TABLE
 CREATE TABLE
 INSERT 0 4

From 73906b60da91434cfb657deb2f8abff611eef96d Mon Sep 17 00:00:00 2001
From: Carla Iriberri <iriberrig@gmail.com>
Date: Wed, 2 Mar 2016 18:19:21 +0100
Subject: [PATCH 10/15] Verbosity terse for tests

---
 test/CDB_CartodbfyTableTest.sql    |  2 +-
 test/CDB_CartodbfyTableTest_expect | 16 ----------------
 2 files changed, 1 insertion(+), 17 deletions(-)

diff --git a/test/CDB_CartodbfyTableTest.sql b/test/CDB_CartodbfyTableTest.sql
index f491aca..2144d0b 100644
--- a/test/CDB_CartodbfyTableTest.sql
+++ b/test/CDB_CartodbfyTableTest.sql
@@ -1,5 +1,5 @@
 SET client_min_messages TO error;
-\set VERBOSITY default
+\set VERBOSITY terse
 
 CREATE OR REPLACE FUNCTION CDB_CartodbfyTableCheck(tabname regclass, label text)
 RETURNS text AS
diff --git a/test/CDB_CartodbfyTableTest_expect b/test/CDB_CartodbfyTableTest_expect
index e144354..aeadfa1 100644
--- a/test/CDB_CartodbfyTableTest_expect
+++ b/test/CDB_CartodbfyTableTest_expect
@@ -2,8 +2,6 @@ SET
 CREATE FUNCTION
 SELECT 1
 ERROR:  Please set user quota before cartodbfying tables.
-CONTEXT:  SQL statement "SELECT cartodb._CDB_check_prerequisites(destschema, reloid)"
-PL/pgSQL function cdb_cartodbfytable(text,regclass) line 21 at PERFORM
 0
 single non-geometrical column cartodbfied fine
 DROP TABLE
@@ -33,12 +31,6 @@ text cartodb_id cartodbfied fine
 DROP TABLE
 SELECT 1
 ERROR:  CDB(_CDB_Rewrite_Table:22P02:invalid input syntax for integer: "nan"): CREATE TABLE public.t_0 AS SELECT cartodb_id::bigint ,NULL::geometry(Geometry,4326) AS the_geom,NULL::geometry(Geometry,3857) AS the_geom_webmercator FROM t
-CONTEXT:  SQL statement "SELECT _CDB_SQL(sql, '_CDB_Rewrite_Table')"
-PL/pgSQL function _cdb_rewrite_table(regclass,text) line 295 at PERFORM
-SQL statement "SELECT _CDB_Rewrite_Table(reloid, destschema)"
-PL/pgSQL function cdb_cartodbfytable(text,regclass) line 51 at PERFORM
-SQL statement "SELECT CDB_CartodbfyTable('public', tabname)"
-PL/pgSQL function cdb_cartodbfytablecheck(regclass,text) line 31 at PERFORM
 DROP TABLE
 SELECT 1
 empty text cartodb_id cartodbfied fine
@@ -95,14 +87,6 @@ DROP TABLE
 CREATE TABLE
 INSERT 0 1
 ERROR:  CDB(_CDB_Has_Usable_Primary_ID: multiple primary keys for table "many_colliding_columns" are not allowed): ALTER TABLE many_colliding_columns ADD CONSTRAINT cartodb_id_pk PRIMARY KEY (cartodb_id)
-CONTEXT:  SQL statement "SELECT _CDB_Error(sql, Format('_CDB_Has_Usable_Primary_ID: %s', SQLERRM))"
-PL/pgSQL function _cdb_has_usable_primary_id(regclass) line 56 at PERFORM
-SQL statement "SELECT _CDB_Has_Usable_Primary_ID(reloid)"
-PL/pgSQL function _cdb_rewrite_table(regclass,text) line 50 at SQL statement
-SQL statement "SELECT _CDB_Rewrite_Table(reloid, destschema)"
-PL/pgSQL function cdb_cartodbfytable(text,regclass) line 51 at PERFORM
-SQL statement "SELECT CDB_CartodbfyTable('public', tabname)"
-PL/pgSQL function cdb_cartodbfytablecheck(regclass,text) line 31 at PERFORM
 DROP TABLE
 CREATE TABLE
 INSERT 0 4

From 5bc725c8abb2261d88a73237bc219c99456df7ce Mon Sep 17 00:00:00 2001
From: Carla <carla@cartodb.com>
Date: Thu, 3 Mar 2016 16:58:31 +0100
Subject: [PATCH 11/15] Add drop function if exists

---
 scripts-available/CDB_CartodbfyTable.sql | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql
index d6fba32..43eb920 100644
--- a/scripts-available/CDB_CartodbfyTable.sql
+++ b/scripts-available/CDB_CartodbfyTable.sql
@@ -470,6 +470,7 @@ $$ LANGUAGE 'plpgsql';
 -- If the table has both a usable key and usable geometry
 -- we can no-op on the table copy and just ensure that the 
 -- indexes and triggers are in place
+DROP FUNCTION IF EXISTS _CDB_Has_Usable_Primary_ID(reloid REGCLASS);
 CREATE OR REPLACE FUNCTION _CDB_Has_Usable_Primary_ID(reloid REGCLASS)
   RETURNS BOOLEAN
 AS $$

From f4b51807a11ba6d58ffb22c95d50c4f493d8dd4c Mon Sep 17 00:00:00 2001
From: Carla Iriberri <iriberrig@gmail.com>
Date: Tue, 8 Mar 2016 11:58:30 +0100
Subject: [PATCH 12/15] Make cartodb_id inconsistencies fail and update tests

---
 scripts-available/CDB_CartodbfyTable.sql | 10 +++----
 test/CDB_CartodbfyTableTest.sql          |  8 +++---
 test/CDB_CartodbfyTableTest_expect       | 34 ++++++++++++------------
 3 files changed, 24 insertions(+), 28 deletions(-)

diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql
index d6fba32..f5d795c 100644
--- a/scripts-available/CDB_CartodbfyTable.sql
+++ b/scripts-available/CDB_CartodbfyTable.sql
@@ -527,7 +527,7 @@ BEGIN
           useable_key := false;
         -- Other fatal error
         WHEN others THEN
-          PERFORM _CDB_Error(sql, Format('_CDB_Has_Usable_Primary_ID: %s', SQLERRM));
+          PERFORM _CDB_Error(sql, Format('_CDB_Has_Usable_Primary_ID: not valid %s', SQLERRM));
       END;
 
       -- Clean up test constraint
@@ -538,13 +538,9 @@ BEGIN
       ELSE
 
         RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %',
-          Format('found non-unique ''%s'', renaming it', const.pkey);
+          Format('found non-unique ''%s''', const.pkey);
 
-        PERFORM _CDB_SQL(
-          Format('ALTER TABLE %s RENAME COLUMN %s TO %I',
-            reloid::text, rec.attname,
-            cartodb._CDB_Unique_Column_Identifier(NULL, const.pkey, NULL, reloid)),
-          '_CDB_Has_Usable_Primary_ID');
+        PERFORM _CDB_Error(sql, Format('_CDB_Has_Usable_Primary_ID: non-unique %s', const.pkey));
 
       END IF;
 
diff --git a/test/CDB_CartodbfyTableTest.sql b/test/CDB_CartodbfyTableTest.sql
index 2144d0b..d32f1e8 100644
--- a/test/CDB_CartodbfyTableTest.sql
+++ b/test/CDB_CartodbfyTableTest.sql
@@ -178,7 +178,7 @@ DROP TABLE t;
 -- table with empty cartodb_id field of type text
 CREATE TABLE t AS SELECT null::text as cartodb_id;
 SELECT CDB_CartodbfyTableCheck('t', 'empty text cartodb_id');
-select cartodb_id,cartodb_id_0 FROM t;
+SELECT cartodb_id from t;
 DROP TABLE t;
 
 -- table with existing cartodb_id field of type int4 not sequenced
@@ -294,7 +294,7 @@ INSERT INTO test VALUES
   (NULL),
   (3);
 SELECT CDB_CartodbfyTableCheck('test', 'Table with null cartodb_id #148');
-SELECT cartodb_id, cartodb_id_0 from test;
+SELECT cartodb_id from test;
 DROP TABLE test;
 
 -- Table with non unique cartodb_id
@@ -306,7 +306,7 @@ INSERT INTO test VALUES
   (2),
   (2);
 SELECT CDB_CartodbfyTableCheck('test', 'Table with non unique cartodb_id #148');
-SELECT cartodb_id, cartodb_id_0 from test;
+SELECT cartodb_id from test;
 DROP TABLE test;
 
 -- Table with non unique and null cartodb_id
@@ -319,7 +319,7 @@ INSERT INTO test VALUES
   (NULL),
   (2);
 SELECT CDB_CartodbfyTableCheck('test', 'Table with non unique and null cartodb_id #148');
-SELECT cartodb_id, cartodb_id_0 from test;
+SELECT cartodb_id from test;
 DROP TABLE test;
 
 CREATE TABLE test (
diff --git a/test/CDB_CartodbfyTableTest_expect b/test/CDB_CartodbfyTableTest_expect
index aeadfa1..b97e9d6 100644
--- a/test/CDB_CartodbfyTableTest_expect
+++ b/test/CDB_CartodbfyTableTest_expect
@@ -33,8 +33,8 @@ SELECT 1
 ERROR:  CDB(_CDB_Rewrite_Table:22P02:invalid input syntax for integer: "nan"): CREATE TABLE public.t_0 AS SELECT cartodb_id::bigint ,NULL::geometry(Geometry,4326) AS the_geom,NULL::geometry(Geometry,3857) AS the_geom_webmercator FROM t
 DROP TABLE
 SELECT 1
-empty text cartodb_id cartodbfied fine
-1|
+ERROR:  CDB(_CDB_Has_Usable_Primary_ID: non-unique cartodb_id): ALTER TABLE t ADD CONSTRAINT cartodb_id_pk PRIMARY KEY (cartodb_id)
+
 DROP TABLE
 SELECT 1
 unsequenced cartodb_id cartodbfied fine
@@ -86,30 +86,30 @@ Table with both the_geom and wkb_geometry #141 cartodbfied fine
 DROP TABLE
 CREATE TABLE
 INSERT 0 1
-ERROR:  CDB(_CDB_Has_Usable_Primary_ID: multiple primary keys for table "many_colliding_columns" are not allowed): ALTER TABLE many_colliding_columns ADD CONSTRAINT cartodb_id_pk PRIMARY KEY (cartodb_id)
+ERROR:  CDB(_CDB_Has_Usable_Primary_ID: not valid multiple primary keys for table "many_colliding_columns" are not allowed): ALTER TABLE many_colliding_columns ADD CONSTRAINT cartodb_id_pk PRIMARY KEY (cartodb_id)
 DROP TABLE
 CREATE TABLE
 INSERT 0 4
-Table with null cartodb_id #148 cartodbfied fine
-1|1
-2|2
-3|
-4|3
+ERROR:  CDB(_CDB_Has_Usable_Primary_ID: non-unique cartodb_id): ALTER TABLE test ADD CONSTRAINT cartodb_id_pk PRIMARY KEY (cartodb_id)
+1
+2
+
+3
 DROP TABLE
 CREATE TABLE
 INSERT 0 3
-Table with non unique cartodb_id #148 cartodbfied fine
-1|1
-2|2
-3|2
+ERROR:  CDB(_CDB_Has_Usable_Primary_ID: non-unique cartodb_id): ALTER TABLE test ADD CONSTRAINT cartodb_id_pk PRIMARY KEY (cartodb_id)
+1
+2
+2
 DROP TABLE
 CREATE TABLE
 INSERT 0 4
-Table with non unique and null cartodb_id #148 cartodbfied fine
-1|1
-2|2
-3|
-4|2
+ERROR:  CDB(_CDB_Has_Usable_Primary_ID: non-unique cartodb_id): ALTER TABLE test ADD CONSTRAINT cartodb_id_pk PRIMARY KEY (cartodb_id)
+1
+2
+
+2
 DROP TABLE
 CREATE TABLE
 CREATE INDEX

From 3d0f580fc2d5a0dbc27da47ef159aa874d82e89f Mon Sep 17 00:00:00 2001
From: Carla Iriberri <iriberrig@gmail.com>
Date: Tue, 8 Mar 2016 12:10:46 +0100
Subject: [PATCH 13/15] Add idempotence test

---
 test/CDB_CartodbfyTableTest.sql    | 10 +++++++++-
 test/CDB_CartodbfyTableTest_expect |  6 ++++++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/test/CDB_CartodbfyTableTest.sql b/test/CDB_CartodbfyTableTest.sql
index d32f1e8..02c9039 100644
--- a/test/CDB_CartodbfyTableTest.sql
+++ b/test/CDB_CartodbfyTableTest.sql
@@ -124,13 +124,21 @@ END;
 $$
 LANGUAGE 'plpgsql';
 
--- table with single non-geometrical column
+-- check cartodbfytable idempotence
 CREATE TABLE t AS SELECT 1::int as a;
 SELECT CDB_CartodbfyTable('public', 't'); -- should fail
 SELECT CDB_SetUserQuotaInBytes(0); -- Set user quota to infinite
 SELECT CDB_CartodbfyTableCheck('t', 'single non-geometrical column');
 DROP TABLE t;
 
+-- table with single non-geometrical column
+CREATE TABLE t AS SELECT ST_SetSRID(ST_MakePoint(-1,-1),4326) as the_geom, 1::int as cartodb_id, 'this is a sentence' as description;
+SELECT CDB_CartodbfyTableCheck('t', 'check function idempotence');
+SELECT * FROM t;
+SELECT CDB_CartodbfyTableCheck('t', 'check function idempotence');
+SELECT * FROM t;
+DROP TABLE t;
+
 -- table with existing srid-unconstrained (but type-constrained) the_geom
 CREATE TABLE t AS SELECT ST_SetSRID(ST_MakePoint(0,0),4326)::geometry(point) as the_geom;
 SELECT CDB_CartodbfyTableCheck('t', 'srid-unconstrained the_geom');
diff --git a/test/CDB_CartodbfyTableTest_expect b/test/CDB_CartodbfyTableTest_expect
index b97e9d6..89eb1b9 100644
--- a/test/CDB_CartodbfyTableTest_expect
+++ b/test/CDB_CartodbfyTableTest_expect
@@ -6,6 +6,12 @@ ERROR:  Please set user quota before cartodbfying tables.
 single non-geometrical column cartodbfied fine
 DROP TABLE
 SELECT 1
+check function idempotence cartodbfied fine
+1|0101000020E6100000000000000000F0BF000000000000F0BF|0101000020110F0000DB0B4ADA772DFBC046432E49D22DFBC0|this is a sentence
+check function idempotence cartodbfied fine
+1|0101000020E6100000000000000000F0BF000000000000F0BF|0101000020110F0000DB0B4ADA772DFBC046432E49D22DFBC0|this is a sentence
+DROP TABLE
+SELECT 1
 srid-unconstrained the_geom cartodbfied fine
 DROP TABLE
 SELECT 2

From 7b48c2765e19340852907a164e1f1735ae55381e Mon Sep 17 00:00:00 2001
From: Carla Iriberri <iriberrig@gmail.com>
Date: Tue, 8 Mar 2016 14:06:58 +0100
Subject: [PATCH 14/15] Fix error detection and fix tests

---
 scripts-available/CDB_CartodbfyTable.sql | 16 +++++++++++-----
 test/CDB_CartodbfyTableTest_expect       | 16 ++++++++--------
 2 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql
index a1130bd..149d7c0 100644
--- a/scripts-available/CDB_CartodbfyTable.sql
+++ b/scripts-available/CDB_CartodbfyTable.sql
@@ -515,6 +515,7 @@ BEGIN
       useable_key := true;
       BEGIN
         sql := Format('ALTER TABLE %s ADD CONSTRAINT %s_pk PRIMARY KEY (%s)', reloid::text, const.pkey, const.pkey);
+        sql := sql || ', ' || Format('ADD CONSTRAINT %s_integer CHECK (%s::integer >=0);', const.pkey, const.pkey);
         RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', sql;
         EXECUTE sql;
         EXCEPTION
@@ -526,22 +527,27 @@ BEGIN
         WHEN not_null_violation THEN
           RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', Format('column %s contains nulls', const.pkey);
           useable_key := false;
+        -- Failed integer check...
+        WHEN invalid_text_representation THEN
+          RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', Format('invalid input syntax for integer %s', const.pkey);
+          useable_key := false;
         -- Other fatal error
         WHEN others THEN
-          PERFORM _CDB_Error(sql, Format('_CDB_Has_Usable_Primary_ID: not valid %s', SQLERRM));
+          PERFORM _CDB_Error(sql, Format('_CDB_Has_Usable_Primary_ID: %s', SQLERRM));
       END;
 
       -- Clean up test constraint
       IF useable_key THEN
         PERFORM _CDB_SQL(Format('ALTER TABLE %s DROP CONSTRAINT %s_pk', reloid::text, const.pkey));
+        PERFORM _CDB_SQL(Format('ALTER TABLE %s DROP CONSTRAINT %s_integer', reloid::text, const.pkey));
 
-      -- Move non-unique column out of the way
+      -- Move non-valid column out of the way
       ELSE
 
         RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %',
-          Format('found non-unique ''%s''', const.pkey);
+          Format('found non-valid ''%s''', const.pkey);
 
-        PERFORM _CDB_Error(sql, Format('_CDB_Has_Usable_Primary_ID: non-unique %s', const.pkey));
+        PERFORM _CDB_Error(sql, Format('_CDB_Has_Usable_Primary_ID: Error: invalid cartodb_id, %s', const.pkey));
 
       END IF;
 
@@ -1076,7 +1082,7 @@ BEGIN
 
   -- Run it!
   PERFORM _CDB_SQL(sql, '_CDB_Rewrite_Table');
-  
+
   -- Set up the primary key sequence
   -- If we copied the primary key from the original data, we need
   -- to set the sequence to the maximum value of that key
diff --git a/test/CDB_CartodbfyTableTest_expect b/test/CDB_CartodbfyTableTest_expect
index 89eb1b9..de2e9ae 100644
--- a/test/CDB_CartodbfyTableTest_expect
+++ b/test/CDB_CartodbfyTableTest_expect
@@ -7,9 +7,9 @@ single non-geometrical column cartodbfied fine
 DROP TABLE
 SELECT 1
 check function idempotence cartodbfied fine
-1|0101000020E6100000000000000000F0BF000000000000F0BF|0101000020110F0000DB0B4ADA772DFBC046432E49D22DFBC0|this is a sentence
+1|0101000020E6100000000000000000F0BF000000000000F0BF|0101000020110F0000DB0B4ADA772DFBC077432E49D22DFBC0|this is a sentence
 check function idempotence cartodbfied fine
-1|0101000020E6100000000000000000F0BF000000000000F0BF|0101000020110F0000DB0B4ADA772DFBC046432E49D22DFBC0|this is a sentence
+1|0101000020E6100000000000000000F0BF000000000000F0BF|0101000020110F0000DB0B4ADA772DFBC077432E49D22DFBC0|this is a sentence
 DROP TABLE
 SELECT 1
 srid-unconstrained the_geom cartodbfied fine
@@ -36,10 +36,10 @@ text cartodb_id cartodbfied fine
 5
 DROP TABLE
 SELECT 1
-ERROR:  CDB(_CDB_Rewrite_Table:22P02:invalid input syntax for integer: "nan"): CREATE TABLE public.t_0 AS SELECT cartodb_id::bigint ,NULL::geometry(Geometry,4326) AS the_geom,NULL::geometry(Geometry,3857) AS the_geom_webmercator FROM t
+ERROR:  CDB(_CDB_Has_Usable_Primary_ID: Error: invalid cartodb_id, cartodb_id): ALTER TABLE t ADD CONSTRAINT cartodb_id_pk PRIMARY KEY (cartodb_id), ADD CONSTRAINT cartodb_id_integer CHECK (cartodb_id::integer >=0);
 DROP TABLE
 SELECT 1
-ERROR:  CDB(_CDB_Has_Usable_Primary_ID: non-unique cartodb_id): ALTER TABLE t ADD CONSTRAINT cartodb_id_pk PRIMARY KEY (cartodb_id)
+ERROR:  CDB(_CDB_Has_Usable_Primary_ID: Error: invalid cartodb_id, cartodb_id): ALTER TABLE t ADD CONSTRAINT cartodb_id_pk PRIMARY KEY (cartodb_id), ADD CONSTRAINT cartodb_id_integer CHECK (cartodb_id::integer >=0);
 
 DROP TABLE
 SELECT 1
@@ -92,11 +92,11 @@ Table with both the_geom and wkb_geometry #141 cartodbfied fine
 DROP TABLE
 CREATE TABLE
 INSERT 0 1
-ERROR:  CDB(_CDB_Has_Usable_Primary_ID: not valid multiple primary keys for table "many_colliding_columns" are not allowed): ALTER TABLE many_colliding_columns ADD CONSTRAINT cartodb_id_pk PRIMARY KEY (cartodb_id)
+ERROR:  CDB(_CDB_Has_Usable_Primary_ID: multiple primary keys for table "many_colliding_columns" are not allowed): ALTER TABLE many_colliding_columns ADD CONSTRAINT cartodb_id_pk PRIMARY KEY (cartodb_id), ADD CONSTRAINT cartodb_id_integer CHECK (cartodb_id::integer >=0);
 DROP TABLE
 CREATE TABLE
 INSERT 0 4
-ERROR:  CDB(_CDB_Has_Usable_Primary_ID: non-unique cartodb_id): ALTER TABLE test ADD CONSTRAINT cartodb_id_pk PRIMARY KEY (cartodb_id)
+ERROR:  CDB(_CDB_Has_Usable_Primary_ID: Error: invalid cartodb_id, cartodb_id): ALTER TABLE test ADD CONSTRAINT cartodb_id_pk PRIMARY KEY (cartodb_id), ADD CONSTRAINT cartodb_id_integer CHECK (cartodb_id::integer >=0);
 1
 2
 
@@ -104,14 +104,14 @@ ERROR:  CDB(_CDB_Has_Usable_Primary_ID: non-unique cartodb_id): ALTER TABLE test
 DROP TABLE
 CREATE TABLE
 INSERT 0 3
-ERROR:  CDB(_CDB_Has_Usable_Primary_ID: non-unique cartodb_id): ALTER TABLE test ADD CONSTRAINT cartodb_id_pk PRIMARY KEY (cartodb_id)
+ERROR:  CDB(_CDB_Has_Usable_Primary_ID: Error: invalid cartodb_id, cartodb_id): ALTER TABLE test ADD CONSTRAINT cartodb_id_pk PRIMARY KEY (cartodb_id), ADD CONSTRAINT cartodb_id_integer CHECK (cartodb_id::integer >=0);
 1
 2
 2
 DROP TABLE
 CREATE TABLE
 INSERT 0 4
-ERROR:  CDB(_CDB_Has_Usable_Primary_ID: non-unique cartodb_id): ALTER TABLE test ADD CONSTRAINT cartodb_id_pk PRIMARY KEY (cartodb_id)
+ERROR:  CDB(_CDB_Has_Usable_Primary_ID: Error: invalid cartodb_id, cartodb_id): ALTER TABLE test ADD CONSTRAINT cartodb_id_pk PRIMARY KEY (cartodb_id), ADD CONSTRAINT cartodb_id_integer CHECK (cartodb_id::integer >=0);
 1
 2
 

From 0b8bada55336d5725ab33ca37a9f4162bde27d26 Mon Sep 17 00:00:00 2001
From: Carla <carla@cartodb.com>
Date: Wed, 9 Mar 2016 11:54:59 +0100
Subject: [PATCH 15/15] Remove unused variable

---
 scripts-available/CDB_CartodbfyTable.sql | 1 -
 1 file changed, 1 deletion(-)

diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql
index 149d7c0..eb80b78 100644
--- a/scripts-available/CDB_CartodbfyTable.sql
+++ b/scripts-available/CDB_CartodbfyTable.sql
@@ -480,7 +480,6 @@ DECLARE
   i INTEGER;
   sql TEXT;
   useable_key BOOLEAN = false;
-  has_usable_primary_key BOOLEAN = false;
 BEGIN
 
   RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', 'entered function';