From f3c20ac2fb092838e55c565508d21b7a20a2cf2d Mon Sep 17 00:00:00 2001
From: Paul Ramsey <pramsey@cleverelephant.ca>
Date: Fri, 17 Apr 2015 17:53:07 +0200
Subject: [PATCH 01/34] First draft of new cartodbfy function (named
 CDB_CartodbfyTable2) Still needs to be fully tested (partially tested now)
 using the existing regression tests. Does not manage the timestamp columns at
 this time.

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

diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql
index 9933008..aeea955 100644
--- a/scripts-available/CDB_CartodbfyTable.sql
+++ b/scripts-available/CDB_CartodbfyTable.sql
@@ -649,3 +649,649 @@ BEGIN
   PERFORM cartodb.CDB_CartodbfyTable('public', reloid);
 END;
 $$ LANGUAGE PLPGSQL;
+
+
+-- -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
+--
+--    NEW CARTODBFY CODE FROM HERE ON DOWN
+--
+-- -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
+--
+--  CDB_CartodbfyTable2(reloid REGCLASS, destschema TEXT DEFAULT NULL)
+--    
+--     Main function, calls the following functions, with a little
+--     logic before the table re-write to avoid re-writing if the table
+--     already has all the necessary columns in place.
+--
+-- (1) _CDB_drop_triggers
+--     As before, this drops all the metadata and geom sync triggers
+--
+-- (2) _CDB_Has_Usable_Primary_ID()
+--     Returns TRUE if it can find a unique integer primary key named
+--    'cartodb_id' or can rename an existing key.
+--     Returns FALSE otherwise.
+--
+-- (3) _CDB_Has_Usable_Geom()
+--     Looks for existing EPSG:4326 and EPSG:3857 geometry columns, and
+--     renames them to the standard names if it can find them, returning TRUE.
+--     If it cannot find both columns in the right EPSG, returns FALSE.
+--
+-- (4) _CDB_Rewrite_Table()
+--     If table does not have a usable primary key and both usable geom
+--     columns it needs to be re-written. Function constructs an appropriate
+--     CREATE TABLE AS SELECT... query and executes it.
+--
+-- (5) _CDB_Add_Indexes()
+--     Checks the primary key column for primary key constraint, adds it if
+--     missing. Check geometry columns for GIST indexes and adds them if missing.
+--
+-- (6) _CDB_create_triggers()
+--     Adds the system metadata and geometry column update triggers back
+--     onto the table.
+--
+-- -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
+
+
+
+-- 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, keyname TEXT)
+  RETURNS BOOLEAN
+AS $$
+DECLARE
+  rec RECORD;
+  i INTEGER;
+  sql TEXT;
+BEGIN
+
+  RAISE DEBUG 'Entered _CDB_Has_Usable_Primary_ID';
+
+  -- 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 
+  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 a.attname = keyname;
+
+  -- It's perfect (named right, right type, right index)!
+  IF FOUND AND rec.indisprimary AND rec.indisunique AND rec.attnotnull AND rec.atttypid IN (20,21,23) THEN
+    RAISE DEBUG '_CDB_Has_Usable_Primary_ID found good ''%''', keyname;
+    RETURN true;
+    
+  -- It's not suitable (not an integer?, not unique?) to rename it out of the way
+  ELSIF FOUND THEN
+    RAISE DEBUG '_CDB_Has_Usable_Primary_ID found bad ''%'', renaming it', keyname;
+    
+    sql := Format('ALTER TABLE %s RENAME COLUMN %s TO %s', 
+              reloid::text, rec.attname, _CDB_Unique_Column_Name(reloid, keyname));
+    EXECUTE sql;        
+    
+  -- There's no column there named keyname
+  ELSE
+
+    -- Is there another suitable primary key already?
+    SELECT a.attname
+    INTO rec
+    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 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
+      EXECUTE Format('ALTER TABLE %s RENAME COLUMN %s TO %s', reloid::text, rec.attname, keyname);
+      RAISE DEBUG '_CDB_Has_Usable_Primary_ID found acceptable primary key ''%s'', renaming to ''%''', rec.attname, keyname;
+      RETURN true;
+    ELSE
+      RAISE DEBUG '_CDB_Has_Usable_Primary_ID found no useful column for ''%''', keyname;
+    END IF;
+  
+  END IF;
+
+  -- Remove any unsuitable primary key constraint that is hanging around, 
+  -- because we will be adding one back later
+  SELECT ci.relname AS pkey
+  INTO rec
+  FROM pg_class c 
+  JOIN pg_attribute a ON a.attrelid = c.oid 
+  LEFT JOIN pg_index i ON c.oid = i.indrelid AND a.attnum = ANY(i.indkey)
+  JOIN pg_class ci ON i.indexrelid = ci.oid
+  WHERE c.oid = reloid AND NOT a.attisdropped
+  AND a.attname != keyname
+  AND i.indisprimary AND a.atttypid NOT IN (20,21,23);
+  
+  IF FOUND THEN
+    EXECUTE Format('ALTER TABLE %s DROP CONSTRAINT IF EXISTS %s', reloid::text, rec.pkey);
+    RAISE DEBUG '_CDB_Has_Usable_Primary_ID dropping unused primary key ''%''', rec.pkey;
+  END IF;
+    
+  -- Didn't fine re-usable key, so return FALSE
+  RETURN false;
+
+END;
+$$ LANGUAGE 'plpgsql';
+
+
+CREATE OR REPLACE FUNCTION _CDB_Unique_Relation_Name(schemaname TEXT, relationname TEXT)
+RETURNS TEXT
+AS $$
+DECLARE
+  rec RECORD;
+  i INTEGER;
+  newrelname TEXT;
+BEGIN
+
+  i := 0;
+  newrelname := relationname;
+  LOOP
+
+    SELECT c.relname, n.nspname
+    INTO rec
+    FROM pg_class c
+    JOIN pg_namespace n ON c.relnamespace = n.oid
+    WHERE c.relname = newrelname
+    AND n.nspname = schemaname;
+  
+    IF NOT FOUND THEN
+      RETURN newrelname;
+    END IF;
+    
+    i := i + 1;
+    newrelname := relationname || '_' || i;
+  
+    IF i > 100 THEN
+      RAISE EXCEPTION '_CDB_Unique_Relation_Name looping too far';
+    END IF;
+  
+  END LOOP;
+  
+END;
+$$ LANGUAGE 'plpgsql';
+
+
+
+CREATE OR REPLACE FUNCTION _CDB_Unique_Column_Name(reloid REGCLASS, columnname TEXT)
+RETURNS TEXT
+AS $$
+DECLARE
+  rec RECORD;
+  i INTEGER;
+  newcolname TEXT;
+BEGIN
+
+  i := 0;
+  newcolname := columnname;
+  LOOP
+
+    SELECT a.attname
+    INTO rec
+    FROM pg_class c
+    JOIN pg_attribute a ON a.attrelid = c.oid
+    WHERE NOT a.attisdropped
+    AND a.attnum > 0
+    AND c.oid = reloid
+    AND a.attname = newcolname;
+  
+    IF NOT FOUND THEN
+      RETURN newcolname;
+    END IF;
+    
+    i := i + 1;
+    newcolname := columnname || '_' || i;
+  
+    IF i > 100 THEN
+      RAISE EXCEPTION '_CDB_Unique_Column_Name looping too far';
+    END IF;
+  
+  END LOOP;
+  
+END;
+$$ LANGUAGE 'plpgsql';
+    
+
+
+CREATE OR REPLACE FUNCTION _CDB_Has_Usable_Geom(reloid REGCLASS, geom_name TEXT, mercgeom_name TEXT)
+  RETURNS BOOLEAN
+AS $$
+DECLARE
+  rec RECORD;
+  has_geom BOOLEAN := false;
+  has_mercgeom BOOLEAN := false;
+  str TEXT;
+BEGIN
+
+  RAISE DEBUG 'Entered _CDB_Has_Usable_Geom';
+
+  -- Do we have a column we can use?
+  FOR rec IN
+    SELECT 
+    a.attname, 
+    CASE WHEN t.typname = 'geometry' THEN postgis_typmod_srid(a.atttypmod) ELSE NULL END AS srid,
+    t.typname
+    FROM pg_class c 
+    JOIN pg_attribute a ON a.attrelid = c.oid 
+    JOIN pg_type t ON a.atttypid = t.oid
+    WHERE c.oid = reloid
+    AND a.attnum > 0
+    AND NOT a.attisdropped
+    AND postgis_typmod_srid(a.atttypmod) IN (4326, 3857)
+    ORDER BY a.attnum
+  LOOP
+  
+    RAISE DEBUG '_CDB_Has_Usable_Geom, checking ''%''', rec.attname;
+  
+    -- Geographic: Right name, but wrong type? Rename it out of the way!
+    IF rec.attname = geom_name AND rec.typname != 'geometry' THEN
+      str := _CDB_Unique_Column_Name(reloid, geom_name);
+      EXECUTE Format('ALTER TABLE %s RENAME COLUMN %s TO %s', reloid::text, geom_name, str);
+      RAISE DEBUG '_CDB_Has_Usable_Geom renamed % to %', geom_name, str;
+    END IF;
+
+    -- Mercator: Right name, but wrong type? Rename it out of the way!
+    IF rec.attname = mercgeom_name AND rec.typname != 'geometry' THEN
+      str := _CDB_Unique_Column_Name(reloid, geom_name);
+      EXECUTE Format('ALTER TABLE %s RENAME COLUMN %s TO _%s', reloid::text, geom_name, str);
+      RAISE DEBUG '_CDB_Has_Usable_Geom renamed % to %', geom_name, str;
+    END IF;
+  
+    -- Geographic: If it's the right name and right SRID, we can use it in place without
+    -- transforming it
+    IF rec.attname = geom_name AND rec.srid = 4326 AND rec.typname = 'geometry' THEN
+      has_geom = true;
+      RAISE DEBUG '_CDB_Has_Usable_Geom found acceptable ''%''', geom_name;
+    -- If it's the right SRID and wrong name, we can just rename it
+    ELSIF rec.srid = 4326  AND rec.typname = 'geometry' THEN
+      EXECUTE Format('ALTER TABLE %s RENAME COLUMN %s TO %s', reloid::text, rec.attname, geom_name);
+      RAISE DEBUG '_CDB_Has_Usable_Geom renamed % to %', rec.attname, geom_name;
+      has_geom = true;
+    END IF;
+
+    -- Mercator: If it's the right name and right SRID, we can use it in place without
+    -- transforming it
+    IF rec.attname = mercgeom_name AND rec.srid = 3857  AND rec.typname = 'geometry' THEN
+      has_mercgeom = true;
+      RAISE DEBUG '_CDB_Has_Usable_Geom found acceptable ''%''', mercgeom_name;
+    -- If it's the right SRID and wrong name, we can just rename it
+    ELSIF rec.srid = 3857  AND rec.typname = 'geometry' THEN
+      EXECUTE Format('ALTER TABLE %s RENAME COLUMN %s TO %s', reloid::text, rec.attname, mercgeom_name);
+      RAISE DEBUG '_CDB_Has_Usable_Geom renamed % to %', rec.attname, mercgeom_name;
+      has_mercgeom = true;
+    END IF;
+    
+  END LOOP;
+  
+  -- If table is perfect (no transforms required), return TRUE!
+  RETURN has_geom AND has_mercgeom;
+
+END;
+$$ LANGUAGE 'plpgsql';
+
+
+
+CREATE OR REPLACE FUNCTION _CDB_Rewrite_Table(reloid REGCLASS, destschema TEXT, has_usable_primary_key BOOLEAN, has_usable_geoms BOOLEAN, geom_name TEXT, mercgeom_name TEXT, primary_key_name TEXT)
+RETURNS BOOLEAN
+AS $$
+DECLARE
+
+  relname TEXT;
+  relschema TEXT;
+
+  destoid REGCLASS;
+  destname TEXT;
+  destseq TEXT;
+  destseqmax INTEGER;
+    
+  salt TEXT := md5(random()::text || now());
+  copyname TEXT;
+
+  column_name_sql TEXT;
+  geom_transform_sql TEXT := NULL;
+  geom_column_source TEXT := NULL;
+
+  rec RECORD;
+  sql TEXT;
+  str TEXT;
+  
+BEGIN
+
+  RAISE DEBUG 'Entered _CDB_Rewrite_Table';
+
+  -- Check calling convention
+  IF has_usable_primary_key AND has_usable_geoms THEN
+    RAISE EXCEPTION '_CDB_Rewrite_Table should not be called, it has good key and geoms';
+  END IF;
+
+  -- 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 
+  WHERE c.oid = reloid;
+  
+  -- Put the primary key sequence in the right schema
+  -- If the new table is not moving, better salt the schema name to avoid conflicts
+  destseq := relname || '_' || primary_key_name || '_seq';
+  destseq := _CDB_Unique_Relation_Name(destschema, destseq);
+  destseq := Format('%s.%s', destschema, destseq);
+  EXECUTE Format('CREATE SEQUENCE %s', destseq);
+
+  -- Salt a temporary table name if we are re-writing in place
+  IF destschema = relschema THEN
+    copyname := destschema || '.' || destname || '_' || salt;
+  ELSE
+    copyname := destschema || '.' || destname;
+  END IF;
+  
+  -- Start building the SQL!
+  sql := 'CREATE TABLE ' || copyname || ' AS SELECT ';
+  
+  -- Add cartodb ID!
+  IF has_usable_primary_key THEN
+    sql := sql || primary_key_name;
+  ELSE
+    sql := sql || 'nextval(''' || destseq || ''') AS ' || primary_key_name;
+  END IF;
+  
+  -- Add the geometry columns!
+  IF has_usable_geoms THEN
+    sql := sql || ',' || geom_name || ',' || mercgeom_name;
+  ELSE
+    
+    -- The geometry columns weren't in the right projection,
+    -- so we need to find the first decent geometry column
+    -- in the table and wrap it in two transforms, one to 4326
+    -- and another to 3857. Then remember its name so we can
+    -- ignore it when we build the list of other columns to
+    -- add to the output table
+    SELECT ',ST_Transform(' 
+      || a.attname 
+      || ',4326)::Geometry(' 
+      || postgis_typmod_type(a.atttypmod) 
+      || ', 4326) AS '
+      || geom_name 
+      || ', ST_Transform(' 
+      || a.attname 
+      || ',3857)::Geometry('
+      || postgis_typmod_type(a.atttypmod) 
+      || ', 3857) AS '
+      || mercgeom_name,
+      a.attname
+    INTO geom_transform_sql, geom_column_source
+    FROM pg_class c 
+    JOIN pg_attribute a ON a.attrelid = c.oid 
+    JOIN pg_type t ON a.atttypid = t.oid
+    WHERE c.oid = reloid
+    AND t.typname = 'geometry'
+    AND a.attnum > 0
+    AND NOT a.attisdropped
+    AND postgis_typmod_srid(a.atttypmod) > 0
+    ORDER BY a.attnum
+    LIMIT 1;
+    
+    -- If there is no geometry column, we continue making a 
+    -- non-spatial table. This is important for folks who want
+    -- their tables to invalidate the SQL API 
+    -- cache on update/insert/delete.
+    IF FOUND THEN
+      sql := sql || geom_transform_sql;
+    ELSE 
+      geom_column_source := '';
+    END IF;
+        
+  END IF;
+
+  -- Add now add all the rest of the columns
+  -- by selecting their names into an array and
+  -- joining the array with a comma
+  SELECT 
+    ',' || array_to_string(array_agg(a.attname),',')
+  INTO column_name_sql
+  FROM pg_class c 
+  JOIN pg_attribute a ON a.attrelid = c.oid 
+  JOIN pg_type t ON a.atttypid = t.oid
+  WHERE c.oid = reloid
+  AND a.attnum > 0
+  AND a.attname NOT IN (geom_name, mercgeom_name, primary_key_name, geom_column_source)
+  AND NOT a.attisdropped;
+
+  -- No non-cartodb columns? Possible, I guess.
+  IF NOT FOUND THEN
+    column_name_sql := '';
+  END IF;
+
+  -- Add the source table to the SQL
+  sql := sql || column_name_sql || ' FROM ' || reloid::text;
+  RAISE DEBUG '_CDB_Rewrite_Table: %', sql;
+
+  -- Run it!
+  EXECUTE sql;
+  
+  -- 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
+  IF has_usable_primary_key THEN
+
+    EXECUTE Format('SELECT max(%s) FROM %s',
+            primary_key_name, copyname)
+       INTO destseqmax;
+
+    IF FOUND AND destseqmax IS NOT NULL THEN
+      EXECUTE Format('SELECT setval(''%s'', %s)', destseq, destseqmax);
+    END IF;
+
+  END IF;
+
+  -- Make the primary key use the sequence as its default value
+  sql := Format('ALTER TABLE %s ALTER COLUMN %I SET DEFAULT nextval(''%s'')', 
+          copyname, primary_key_name, destseq);
+  RAISE DEBUG '_CDB_Rewrite_Table: %', sql;
+  EXECUTE sql;
+  
+  -- We just made a copy, so we can drop the original now
+  sql := Format('DROP TABLE %s', reloid::text);
+  RAISE DEBUG '_CDB_Rewrite_Table: %', sql;
+  EXECUTE sql;
+  
+  -- If we used a temporary destination table
+  -- we can now rename it into place
+  IF destschema = relschema THEN
+    sql := Format('ALTER TABLE %s RENAME TO %s', copyname, destname);
+    RAISE DEBUG '_CDB_Rewrite_Table: %', sql;
+    EXECUTE sql;
+  END IF;
+
+  RETURN true;
+
+END;
+$$ LANGUAGE 'plpgsql';
+
+
+
+CREATE OR REPLACE FUNCTION _CDB_Add_Indexes(reloid REGCLASS, geom_name TEXT, mercgeom_name TEXT, primary_key_name TEXT)
+  RETURNS BOOLEAN
+AS $$
+DECLARE
+  rec RECORD;
+  iname TEXT;
+  sql TEXT;
+  relname TEXT;
+BEGIN
+
+  RAISE DEBUG 'Entered _CDB_Add_Indexes';
+
+  -- Extract just the relname to use for the index names
+  SELECT c.relname
+  INTO STRICT relname
+  FROM pg_class c
+  WHERE c.oid = reloid;
+
+  -- Is the default primary key flagged as primary?
+  SELECT a.attname
+  INTO rec
+  FROM pg_class c 
+  JOIN pg_attribute a ON a.attrelid = c.oid 
+  JOIN pg_index i ON c.oid = i.indrelid AND a.attnum = ANY(i.indkey)
+  JOIN pg_class ci ON ci.oid = i.indexrelid
+  WHERE attnum > 0 
+  AND c.oid = reloid
+  AND a.attname = primary_key_name
+  AND i.indisprimary
+  AND i.indisunique
+  AND NOT attisdropped;
+  
+  -- No primary key? Add one.
+  IF NOT FOUND THEN
+    sql := Format('ALTER TABLE %s ADD PRIMARY KEY (%s)', reloid::text, primary_key_name);
+    RAISE DEBUG '_CDB_Add_Indexes: %', sql;
+    EXECUTE sql;
+  END IF;
+  
+  -- Add geometry indexes to all "special geometry columns" that 
+  -- don't have one (either have no index at all, or have a non-GIST index)
+  FOR rec IN 
+    SELECT a.attname, n.nspname
+    FROM pg_class c 
+    JOIN pg_namespace n ON n.oid = c.relnamespace
+    JOIN pg_attribute a ON a.attrelid = c.oid AND attnum > 0 
+    LEFT JOIN pg_index i ON c.oid = i.indrelid AND a.attnum = ANY(i.indkey)
+    WHERE NOT attisdropped
+    AND a.attname IN (geom_name, mercgeom_name)
+    AND c.oid = reloid
+    AND i.indexrelid IS NULL
+    UNION 
+    SELECT a.attname, n.nspname
+    FROM pg_class c 
+    JOIN pg_namespace n ON n.oid = c.relnamespace
+    JOIN pg_attribute a ON a.attrelid = c.oid AND attnum > 0 
+    JOIN pg_index i ON c.oid = i.indrelid AND a.attnum = ANY(i.indkey)
+    JOIN pg_class ci ON ci.oid = i.indexrelid
+    JOIN pg_am am ON ci.relam = am.oid
+    WHERE NOT attisdropped
+    AND a.attname IN (geom_name, mercgeom_name)
+    AND c.oid = reloid
+    AND am.amname != 'gist'
+  LOOP
+    sql := Format('CREATE INDEX %s_%s_gix ON %s USING GIST (%s)', relname, rec.attname, reloid::text, rec.attname);
+    RAISE DEBUG '_CDB_Add_Indexes: %', sql;
+    EXECUTE sql;
+  END LOOP;
+    
+  RETURN true;
+
+END;
+$$ LANGUAGE 'plpgsql';
+
+
+CREATE OR REPLACE FUNCTION CDB_CartodbfyTable2(reloid REGCLASS, destschema TEXT DEFAULT NULL)
+RETURNS void 
+AS $$
+DECLARE
+  -- Because we're going to change these some day, ha ha ha ha!
+  geom_name TEXT := 'the_geom';
+  mercgeom_name TEXT := 'the_geom_webmercator';
+  primary_key_name TEXT := 'cartodb_id';
+  
+  relname TEXT;
+  relschema TEXT;
+
+  destoid REGCLASS;
+  destname TEXT;
+
+  has_usable_primary_key BOOLEAN;
+  has_usable_geoms BOOLEAN;
+  rewrite_success BOOLEAN;
+  rewrite BOOLEAN;
+  index_success BOOLEAN;
+  rec RECORD;
+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 
+  WHERE c.oid = reloid;
+
+  -- Check destination schema exists
+  -- Throws an exception of there is no matching schema
+  IF destschema IS NOT NULL THEN
+    SELECT n.nspname
+    INTO rec FROM pg_namespace n WHERE n.nspname = destschema;
+    IF NOT FOUND THEN
+      RAISE EXCEPTION 'Schema ''%'' does not exist', destschema;
+    END IF;
+
+  ELSE
+    destschema := relschema;
+  END IF;
+
+  -- Drop triggers first
+  -- PERFORM _CDB_drop_triggers(reloid);
+  
+  -- 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, primary_key_name) AS has_usable_primary_key
+  INTO STRICT has_usable_primary_key;
+
+  -- See if the geometry columns we need are already available
+  -- on the table. If they are, we don't need to do any bulk
+  -- transformation of the table, we can just ensure proper
+  -- indexes are in place and apply a rename
+  SELECT _CDB_Has_Usable_Geom(reloid, geom_name, mercgeom_name) AS has_usable_geoms
+  INTO STRICT has_usable_geoms;
+  
+  -- We can only avoid a rewrite if both the key and 
+  -- geometry are usable
+  rewrite := NOT (has_usable_primary_key AND has_usable_geoms);
+  
+  -- No table re-write is required, BUT a rename is required to 
+  -- a destination schema, so do that now
+  IF NOT rewrite AND destschema != relschema THEN
+    
+    RAISE DEBUG 'perfect table needs to be moved to schema (%)', destschema;
+    EXECUTE Format('ALTER TABLE %s SET SCHEMA %s', reloid::text, destschema);
+
+  -- Don't move anything, just make sure our destination information is set right
+  ELSIF NOT rewrite AND destschema = relschema THEN
+
+    RAISE DEBUG 'perfect table in the perfect place';
+  
+  -- We must rewrite, so here we go...
+  ELSIF rewrite THEN
+
+    SELECT _CDB_Rewrite_Table(reloid, destschema, has_usable_primary_key, has_usable_geoms, geom_name, mercgeom_name, primary_key_name)
+    INTO STRICT rewrite_success;
+    
+    IF NOT rewrite_success THEN
+      RAISE EXCEPTION 'Cartodbfying % (rewriting table): % (%)', reloid, SQLERRM, SQLSTATE;
+    END IF;
+      
+  END IF;
+
+  -- The old regclass might not be valid anymore if we re-wrote the table...
+  destoid := (destschema || '.' || destname)::regclass;
+
+  -- Add indexes to the destination table, as necessary
+  SELECT _CDB_Add_Indexes(destoid, geom_name, mercgeom_name, primary_key_name)
+  INTO STRICT index_success;
+
+  IF NOT index_success THEN
+    RAISE EXCEPTION 'Cartodbfying % (indexing table): % (%)', destoid, SQLERRM, SQLSTATE;
+  END IF;
+  
+  -- Add triggers to the destination table, as necessary
+  -- PERFORM _CDB_create_triggers(destschema, reloid);
+  
+  
+END;
+$$ LANGUAGE 'plpgsql';
+
+
+
+

From 14414c4bf38c364a5a8f8ec1a34e3ea87f117923 Mon Sep 17 00:00:00 2001
From: Paul Ramsey <pramsey@cleverelephant.ca>
Date: Tue, 21 Apr 2015 06:25:35 -0700
Subject: [PATCH 02/34] Fix Rambo's test case, of a single geometry-only table
 with no SRID in the metadata (thanks mate).

---
 scripts-available/CDB_CartodbfyTable.sql | 99 +++++++++++++++---------
 1 file changed, 64 insertions(+), 35 deletions(-)

diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql
index aeea955..a9ba925 100644
--- a/scripts-available/CDB_CartodbfyTable.sql
+++ b/scripts-available/CDB_CartodbfyTable.sql
@@ -990,39 +990,25 @@ BEGIN
   
   -- Start building the SQL!
   sql := 'CREATE TABLE ' || copyname || ' AS SELECT ';
-  
+
   -- Add cartodb ID!
   IF has_usable_primary_key THEN
     sql := sql || primary_key_name;
   ELSE
     sql := sql || 'nextval(''' || destseq || ''') AS ' || primary_key_name;
   END IF;
-  
+
   -- Add the geometry columns!
   IF has_usable_geoms THEN
     sql := sql || ',' || geom_name || ',' || mercgeom_name;
   ELSE
     
-    -- The geometry columns weren't in the right projection,
-    -- so we need to find the first decent geometry column
-    -- in the table and wrap it in two transforms, one to 4326
-    -- and another to 3857. Then remember its name so we can
-    -- ignore it when we build the list of other columns to
-    -- add to the output table
-    SELECT ',ST_Transform(' 
-      || a.attname 
-      || ',4326)::Geometry(' 
-      || postgis_typmod_type(a.atttypmod) 
-      || ', 4326) AS '
-      || geom_name 
-      || ', ST_Transform(' 
-      || a.attname 
-      || ',3857)::Geometry('
-      || postgis_typmod_type(a.atttypmod) 
-      || ', 3857) AS '
-      || mercgeom_name,
-      a.attname
-    INTO geom_transform_sql, geom_column_source
+    -- This gets complicated: we have to make sure the 
+    -- geometry column we are using can be transformed into
+    -- geographics, which means it needs to have a valid
+    -- SRID. And the geometry objects can have an 
+    SELECT a.attname
+    INTO rec
     FROM pg_class c 
     JOIN pg_attribute a ON a.attrelid = c.oid 
     JOIN pg_type t ON a.atttypid = t.oid
@@ -1030,28 +1016,68 @@ BEGIN
     AND t.typname = 'geometry'
     AND a.attnum > 0
     AND NOT a.attisdropped
-    AND postgis_typmod_srid(a.atttypmod) > 0
     ORDER BY a.attnum
     LIMIT 1;
     
-    -- If there is no geometry column, we continue making a 
-    -- non-spatial table. This is important for folks who want
-    -- their tables to invalidate the SQL API 
-    -- cache on update/insert/delete.
-    IF FOUND THEN
-      sql := sql || geom_transform_sql;
-    ELSE 
+    IF NOT FOUND THEN
+      -- If there is no geometry column, we continue making a 
+      -- non-spatial table. This is important for folks who want
+      -- their tables to invalidate the SQL API 
+      -- cache on update/insert/delete.
       geom_column_source := '';
+
+    ELSE
+
+      EXECUTE Format('SELECT ST_SRID(%s) AS srid FROM %s LIMIT 1', rec.attname, reloid::text)
+      INTO rec;
+    
+      -- The geometry columns weren't in the right projection,
+      -- so we need to find the first decent geometry column
+      -- in the table and wrap it in two transforms, one to 4326
+      -- and another to 3857. Then remember its name so we can
+      -- ignore it when we build the list of other columns to
+      -- add to the output table
+      SELECT ',ST_Transform(' 
+        || a.attname 
+        || ',4326)::Geometry(' 
+        || postgis_typmod_type(a.atttypmod) 
+        || ', 4326) AS '
+        || geom_name 
+        || ', ST_Transform(' 
+        || a.attname 
+        || ',3857)::Geometry('
+        || postgis_typmod_type(a.atttypmod) 
+        || ', 3857) AS '
+        || mercgeom_name,
+        a.attname
+      INTO geom_transform_sql, geom_column_source
+      FROM pg_class c 
+      JOIN pg_attribute a ON a.attrelid = c.oid 
+      JOIN pg_type t ON a.atttypid = t.oid,
+      ( SELECT rec.srid AS srid ) AS srid
+      WHERE c.oid = reloid
+      AND t.typname = 'geometry'
+      AND a.attnum > 0
+      AND NOT a.attisdropped
+      AND (postgis_typmod_srid(a.atttypmod) > 0 OR srid.srid > 0)
+      ORDER BY a.attnum
+      LIMIT 1;
+    
+      IF FOUND THEN
+        sql := sql || geom_transform_sql;
+      END IF;
+
     END IF;
-        
+    
   END IF;
 
   -- Add now add all the rest of the columns
   -- by selecting their names into an array and
   -- joining the array with a comma
   SELECT 
-    ',' || array_to_string(array_agg(a.attname),',')
-  INTO column_name_sql
+    ',' || array_to_string(array_agg(a.attname),',') AS column_name_sql, 
+    Count(*) AS count
+  INTO rec
   FROM pg_class c 
   JOIN pg_attribute a ON a.attrelid = c.oid 
   JOIN pg_type t ON a.atttypid = t.oid
@@ -1060,14 +1086,17 @@ BEGIN
   AND a.attname NOT IN (geom_name, mercgeom_name, primary_key_name, geom_column_source)
   AND NOT a.attisdropped;
 
+
   -- No non-cartodb columns? Possible, I guess.
-  IF NOT FOUND THEN
+  IF rec.count = 0 THEN
     column_name_sql := '';
+  ELSE
+    column_name_sql := rec.column_name_sql;
   END IF;
 
   -- Add the source table to the SQL
   sql := sql || column_name_sql || ' FROM ' || reloid::text;
-  RAISE DEBUG '_CDB_Rewrite_Table: %', sql;
+  RAISE DEBUG '_CDB_Rewrite_Table generated SQL: %', sql;
 
   -- Run it!
   EXECUTE sql;

From bb685795d5250b2c498ee9c25686c860b8af01c0 Mon Sep 17 00:00:00 2001
From: Paul Ramsey <pramsey@cleverelephant.ca>
Date: Tue, 21 Apr 2015 06:58:33 -0700
Subject: [PATCH 03/34] Handle geometry column with no metadata SRID (grrr) but
 a valid SRID on the geometry objects themselves

---
 scripts-available/CDB_CartodbfyTable.sql | 52 +++++++++++++++++-------
 1 file changed, 37 insertions(+), 15 deletions(-)

diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql
index a9ba925..00b55bd 100644
--- a/scripts-available/CDB_CartodbfyTable.sql
+++ b/scripts-available/CDB_CartodbfyTable.sql
@@ -861,16 +861,18 @@ CREATE OR REPLACE FUNCTION _CDB_Has_Usable_Geom(reloid REGCLASS, geom_name TEXT,
   RETURNS BOOLEAN
 AS $$
 DECLARE
-  rec RECORD;
+  r1 RECORD;
+  r2 RECORD;
   has_geom BOOLEAN := false;
   has_mercgeom BOOLEAN := false;
+  srid INTEGER := 0;
   str TEXT;
 BEGIN
 
   RAISE DEBUG 'Entered _CDB_Has_Usable_Geom';
 
   -- Do we have a column we can use?
-  FOR rec IN
+  FOR r1 IN
     SELECT 
     a.attname, 
     CASE WHEN t.typname = 'geometry' THEN postgis_typmod_srid(a.atttypmod) ELSE NULL END AS srid,
@@ -881,21 +883,41 @@ BEGIN
     WHERE c.oid = reloid
     AND a.attnum > 0
     AND NOT a.attisdropped
-    AND postgis_typmod_srid(a.atttypmod) IN (4326, 3857)
+    AND postgis_typmod_srid(a.atttypmod) IN (4326, 3857, 0)
     ORDER BY a.attnum
   LOOP
   
-    RAISE DEBUG '_CDB_Has_Usable_Geom, checking ''%''', rec.attname;
-  
+    RAISE DEBUG '_CDB_Has_Usable_Geom, checking ''%''', r1.attname;
+    
+    -- The column SRID could be 0 but the data might have a 
+    -- good SRID value in it, so we have to check that before
+    -- going forward (*sigh*)
+    IF r1.srid = 0 THEN
+
+      RAISE DEBUG '_CDB_Has_Usable_Geom, no column srid, checking data row';
+
+      EXECUTE Format('SELECT ST_SRID(%s) AS srid FROM %s LIMIT 1', r1.attname, reloid::text)
+      INTO r2;
+      
+      IF r2.srid > 0 THEN
+        srid := r2.srid;
+      END IF;
+
+    ELSE
+      srid := r1.srid;
+    END IF;
+
+    RAISE DEBUG '_CDB_Has_Usable_Geom, SRID(%) is %', r1.attname, srid;
+
     -- Geographic: Right name, but wrong type? Rename it out of the way!
-    IF rec.attname = geom_name AND rec.typname != 'geometry' THEN
+    IF r1.attname = geom_name AND r1.typname != 'geometry' THEN
       str := _CDB_Unique_Column_Name(reloid, geom_name);
       EXECUTE Format('ALTER TABLE %s RENAME COLUMN %s TO %s', reloid::text, geom_name, str);
       RAISE DEBUG '_CDB_Has_Usable_Geom renamed % to %', geom_name, str;
     END IF;
 
     -- Mercator: Right name, but wrong type? Rename it out of the way!
-    IF rec.attname = mercgeom_name AND rec.typname != 'geometry' THEN
+    IF r1.attname = mercgeom_name AND r1.typname != 'geometry' THEN
       str := _CDB_Unique_Column_Name(reloid, geom_name);
       EXECUTE Format('ALTER TABLE %s RENAME COLUMN %s TO _%s', reloid::text, geom_name, str);
       RAISE DEBUG '_CDB_Has_Usable_Geom renamed % to %', geom_name, str;
@@ -903,25 +925,25 @@ BEGIN
   
     -- Geographic: If it's the right name and right SRID, we can use it in place without
     -- transforming it
-    IF rec.attname = geom_name AND rec.srid = 4326 AND rec.typname = 'geometry' THEN
+    IF r1.attname = geom_name AND srid = 4326 AND r1.typname = 'geometry' THEN
       has_geom = true;
       RAISE DEBUG '_CDB_Has_Usable_Geom found acceptable ''%''', geom_name;
     -- If it's the right SRID and wrong name, we can just rename it
-    ELSIF rec.srid = 4326  AND rec.typname = 'geometry' THEN
-      EXECUTE Format('ALTER TABLE %s RENAME COLUMN %s TO %s', reloid::text, rec.attname, geom_name);
-      RAISE DEBUG '_CDB_Has_Usable_Geom renamed % to %', rec.attname, geom_name;
+    ELSIF srid = 4326  AND r1.typname = 'geometry' THEN
+      EXECUTE Format('ALTER TABLE %s RENAME COLUMN %s TO %s', reloid::text, r1.attname, geom_name);
+      RAISE DEBUG '_CDB_Has_Usable_Geom renamed % to %', r1.attname, geom_name;
       has_geom = true;
     END IF;
 
     -- Mercator: If it's the right name and right SRID, we can use it in place without
     -- transforming it
-    IF rec.attname = mercgeom_name AND rec.srid = 3857  AND rec.typname = 'geometry' THEN
+    IF r1.attname = mercgeom_name AND srid = 3857  AND r1.typname = 'geometry' THEN
       has_mercgeom = true;
       RAISE DEBUG '_CDB_Has_Usable_Geom found acceptable ''%''', mercgeom_name;
     -- If it's the right SRID and wrong name, we can just rename it
-    ELSIF rec.srid = 3857  AND rec.typname = 'geometry' THEN
-      EXECUTE Format('ALTER TABLE %s RENAME COLUMN %s TO %s', reloid::text, rec.attname, mercgeom_name);
-      RAISE DEBUG '_CDB_Has_Usable_Geom renamed % to %', rec.attname, mercgeom_name;
+    ELSIF srid = 3857  AND r1.typname = 'geometry' THEN
+      EXECUTE Format('ALTER TABLE %s RENAME COLUMN %s TO %s', reloid::text, r1.attname, mercgeom_name);
+      RAISE DEBUG '_CDB_Has_Usable_Geom renamed % to %', r1.attname, mercgeom_name;
       has_mercgeom = true;
     END IF;
     

From 74b77408920135e57cf50d465de7ad4558b6df76 Mon Sep 17 00:00:00 2001
From: Paul Ramsey <pramsey@cleverelephant.ca>
Date: Tue, 21 Apr 2015 12:59:44 -0700
Subject: [PATCH 04/34] Fix bug with missing non-geo columns in case where geo
 columns are "perfect" to start w/.

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

diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql
index 00b55bd..dde0449 100644
--- a/scripts-available/CDB_CartodbfyTable.sql
+++ b/scripts-available/CDB_CartodbfyTable.sql
@@ -715,20 +715,27 @@ BEGIN
   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
+  WHERE c.oid = reloid 
+  AND NOT a.attisdropped
   AND a.attname = keyname;
 
   -- It's perfect (named right, right type, right index)!
   IF FOUND AND rec.indisprimary AND rec.indisunique AND rec.attnotnull AND rec.atttypid IN (20,21,23) THEN
     RAISE DEBUG '_CDB_Has_Usable_Primary_ID found good ''%''', keyname;
     RETURN true;
-    
+  
+  -- It's an integer and it's named 'cartodb_id' maybe it is usable
+  -- ELSIF rec.atttypid IN (20,21,23) THEN
+  
+  
+  
   -- It's not suitable (not an integer?, not unique?) to rename it out of the way
   ELSIF FOUND THEN
     RAISE DEBUG '_CDB_Has_Usable_Primary_ID found bad ''%'', renaming it', keyname;
     
     sql := Format('ALTER TABLE %s RENAME COLUMN %s TO %s', 
               reloid::text, rec.attname, _CDB_Unique_Column_Name(reloid, keyname));
+    RAISE DEBUG '_CDB_Has_Usable_Primary_ID: %', sql;
     EXECUTE sql;        
     
   -- There's no column there named keyname
@@ -771,7 +778,9 @@ BEGIN
     EXECUTE Format('ALTER TABLE %s DROP CONSTRAINT IF EXISTS %s', reloid::text, rec.pkey);
     RAISE DEBUG '_CDB_Has_Usable_Primary_ID dropping unused primary key ''%''', rec.pkey;
   END IF;
-    
+  
+  RAISE DEBUG '_CDB_Has_Usable_Primary_ID completed';
+  
   -- Didn't fine re-usable key, so return FALSE
   RETURN false;
 
@@ -816,7 +825,6 @@ END;
 $$ LANGUAGE 'plpgsql';
 
 
-
 CREATE OR REPLACE FUNCTION _CDB_Unique_Column_Name(reloid REGCLASS, columnname TEXT)
 RETURNS TEXT
 AS $$
@@ -856,6 +864,28 @@ END;
 $$ LANGUAGE 'plpgsql';
     
 
+CREATE OR REPLACE FUNCTION _CDB_Geometry_SRID(reloid REGCLASS, columnname TEXT)
+RETURNS INTEGER
+AS $$
+DECLARE
+  rec RECORD;
+BEGIN
+
+  RAISE DEBUG '_CDB_Geometry_SRID, entered';
+  
+  EXECUTE Format('SELECT ST_SRID(%s) AS srid FROM %s LIMIT 1', columnname, reloid::text)
+  INTO rec;
+
+  IF FOUND THEN 
+    RETURN rec.srid;
+  ELSE
+    RETURN 0;
+  END IF;
+  
+END;
+$$ LANGUAGE 'plpgsql';
+
+
 
 CREATE OR REPLACE FUNCTION _CDB_Has_Usable_Geom(reloid REGCLASS, geom_name TEXT, mercgeom_name TEXT)
   RETURNS BOOLEAN
@@ -863,9 +893,9 @@ AS $$
 DECLARE
   r1 RECORD;
   r2 RECORD;
+  found_geom BOOLEAN := false;
   has_geom BOOLEAN := false;
   has_mercgeom BOOLEAN := false;
-  srid INTEGER := 0;
   str TEXT;
 BEGIN
 
@@ -876,75 +906,50 @@ BEGIN
     SELECT 
     a.attname, 
     CASE WHEN t.typname = 'geometry' THEN postgis_typmod_srid(a.atttypmod) ELSE NULL END AS srid,
-    t.typname
+    t.typname,
+    f.desired_attname, f.desired_srid
     FROM pg_class c 
     JOIN pg_attribute a ON a.attrelid = c.oid 
-    JOIN pg_type t ON a.atttypid = t.oid
+    JOIN pg_type t ON a.atttypid = t.oid,
+    (VALUES (geom_name, 4326), (mercgeom_name, 3857) ) as f(desired_attname, desired_srid)
     WHERE c.oid = reloid
     AND a.attnum > 0
     AND NOT a.attisdropped
     AND postgis_typmod_srid(a.atttypmod) IN (4326, 3857, 0)
-    ORDER BY a.attnum
+    ORDER BY t.oid ASC
   LOOP
   
     RAISE DEBUG '_CDB_Has_Usable_Geom, checking ''%''', r1.attname;
-    
-    -- The column SRID could be 0 but the data might have a 
-    -- good SRID value in it, so we have to check that before
-    -- going forward (*sigh*)
-    IF r1.srid = 0 THEN
+    found_geom := false;
 
-      RAISE DEBUG '_CDB_Has_Usable_Geom, no column srid, checking data row';
+    -- Name collision: right name but wrong type, rename it!
+    IF r1.typname != 'geometry' AND r1.attname = r1.desired_attname THEN
+      str := _CDB_Unique_Column_Name(reloid, r1.attname);
+      EXECUTE Format('ALTER TABLE %s RENAME COLUMN %s TO %s', reloid::text, r1.attname, str);
+      RAISE DEBUG '_CDB_Has_Usable_Geom: % is the wrong type, renamed to %', r1.attname, str;
 
-      EXECUTE Format('SELECT ST_SRID(%s) AS srid FROM %s LIMIT 1', r1.attname, reloid::text)
-      INTO r2;
-      
-      IF r2.srid > 0 THEN
-        srid := r2.srid;
+    -- Found a geometry column!
+    ELSIF r1.typname = 'geometry' THEN
+
+      -- If it's the right SRID, we can use it in place without
+      -- transforming it!
+      IF r1.srid = r1.desired_srid OR _CDB_Geometry_SRID(reloid, r1.attname) = r1.desired_srid THEN
+        RAISE DEBUG '_CDB_Has_Usable_Geom found acceptable ''%''', r1.attname;
+
+        -- If it's the wrong name, just rename it.
+        IF r1.attname != r1.desired_attname THEN
+          EXECUTE Format('ALTER TABLE %s RENAME COLUMN %s TO %s', reloid::text, r1.attname, r1.desired_attname);
+          RAISE DEBUG '_CDB_Has_Usable_Geom renamed % to %', r1.attname, r1.desired_attname;
+        END IF;
+
+        IF r1.desired_attname = geom_name THEN
+          has_geom = true;
+        ELSIF r1.desired_attname = mercgeom_name THEN
+          has_mercgeom = true;
+        END IF;
+        
       END IF;
-
-    ELSE
-      srid := r1.srid;
-    END IF;
-
-    RAISE DEBUG '_CDB_Has_Usable_Geom, SRID(%) is %', r1.attname, srid;
-
-    -- Geographic: Right name, but wrong type? Rename it out of the way!
-    IF r1.attname = geom_name AND r1.typname != 'geometry' THEN
-      str := _CDB_Unique_Column_Name(reloid, geom_name);
-      EXECUTE Format('ALTER TABLE %s RENAME COLUMN %s TO %s', reloid::text, geom_name, str);
-      RAISE DEBUG '_CDB_Has_Usable_Geom renamed % to %', geom_name, str;
-    END IF;
-
-    -- Mercator: Right name, but wrong type? Rename it out of the way!
-    IF r1.attname = mercgeom_name AND r1.typname != 'geometry' THEN
-      str := _CDB_Unique_Column_Name(reloid, geom_name);
-      EXECUTE Format('ALTER TABLE %s RENAME COLUMN %s TO _%s', reloid::text, geom_name, str);
-      RAISE DEBUG '_CDB_Has_Usable_Geom renamed % to %', geom_name, str;
-    END IF;
-  
-    -- Geographic: If it's the right name and right SRID, we can use it in place without
-    -- transforming it
-    IF r1.attname = geom_name AND srid = 4326 AND r1.typname = 'geometry' THEN
-      has_geom = true;
-      RAISE DEBUG '_CDB_Has_Usable_Geom found acceptable ''%''', geom_name;
-    -- If it's the right SRID and wrong name, we can just rename it
-    ELSIF srid = 4326  AND r1.typname = 'geometry' THEN
-      EXECUTE Format('ALTER TABLE %s RENAME COLUMN %s TO %s', reloid::text, r1.attname, geom_name);
-      RAISE DEBUG '_CDB_Has_Usable_Geom renamed % to %', r1.attname, geom_name;
-      has_geom = true;
-    END IF;
-
-    -- Mercator: If it's the right name and right SRID, we can use it in place without
-    -- transforming it
-    IF r1.attname = mercgeom_name AND srid = 3857  AND r1.typname = 'geometry' THEN
-      has_mercgeom = true;
-      RAISE DEBUG '_CDB_Has_Usable_Geom found acceptable ''%''', mercgeom_name;
-    -- If it's the right SRID and wrong name, we can just rename it
-    ELSIF srid = 3857  AND r1.typname = 'geometry' THEN
-      EXECUTE Format('ALTER TABLE %s RENAME COLUMN %s TO %s', reloid::text, r1.attname, mercgeom_name);
-      RAISE DEBUG '_CDB_Has_Usable_Geom renamed % to %', r1.attname, mercgeom_name;
-      has_mercgeom = true;
+      
     END IF;
     
   END LOOP;
@@ -975,7 +980,7 @@ DECLARE
 
   column_name_sql TEXT;
   geom_transform_sql TEXT := NULL;
-  geom_column_source TEXT := NULL;
+  geom_column_source TEXT := '';
 
   rec RECORD;
   sql TEXT;
@@ -997,7 +1002,8 @@ BEGIN
   WHERE c.oid = reloid;
   
   -- Put the primary key sequence in the right schema
-  -- If the new table is not moving, better salt the schema name to avoid conflicts
+  -- If the new table is not moving, better ensure the sequence name
+  -- is unique
   destseq := relname || '_' || primary_key_name || '_seq';
   destseq := _CDB_Unique_Relation_Name(destschema, destseq);
   destseq := Format('%s.%s', destschema, destseq);
@@ -1028,7 +1034,7 @@ BEGIN
     -- This gets complicated: we have to make sure the 
     -- geometry column we are using can be transformed into
     -- geographics, which means it needs to have a valid
-    -- SRID. And the geometry objects can have an 
+    -- SRID.  
     SELECT a.attname
     INTO rec
     FROM pg_class c 
@@ -1111,8 +1117,10 @@ BEGIN
 
   -- No non-cartodb columns? Possible, I guess.
   IF rec.count = 0 THEN
+    RAISE DEBUG '_CDB_Rewrite_Table found no extra columns';
     column_name_sql := '';
   ELSE
+    RAISE DEBUG '_CDB_Rewrite_Table found extra columns columns %', rec.column_name_sql;
     column_name_sql := rec.column_name_sql;
   END IF;
 
@@ -1143,6 +1151,12 @@ BEGIN
           copyname, primary_key_name, destseq);
   RAISE DEBUG '_CDB_Rewrite_Table: %', sql;
   EXECUTE sql;
+
+  -- Make the sequence owned by the table, so when the table drops, 
+  -- the sequence does too
+  sql := Format('ALTER SEQUENCE %s OWNED BY %s.%s', destseq, copyname, primary_key_name);
+  RAISE DEBUG '_CDB_Rewrite_Table: %', sql;
+  EXECUTE sql;
   
   -- We just made a copy, so we can drop the original now
   sql := Format('DROP TABLE %s', reloid::text);

From 8dc7f45ccac4a0e931de08dab0b736bfdb71af31 Mon Sep 17 00:00:00 2001
From: Paul Ramsey <pramsey@cleverelephant.ca>
Date: Wed, 22 Apr 2015 06:33:49 -0700
Subject: [PATCH 05/34] Re-use columns named 'cartodb_id' if the values of the
 keys are in fact unique.

---
 scripts-available/CDB_CartodbfyTable.sql | 74 +++++++++++++++++++-----
 1 file changed, 59 insertions(+), 15 deletions(-)

diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql
index dde0449..e366746 100644
--- a/scripts-available/CDB_CartodbfyTable.sql
+++ b/scripts-available/CDB_CartodbfyTable.sql
@@ -692,6 +692,8 @@ $$ LANGUAGE PLPGSQL;
 -- -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
 
 
+-- TODO: Handing the case of 'cartodb_id' column with integer non-primary key
+-- TODO: Preserve that column, IFF it has unique values
 
 -- Find out if the table already has a usable primary key
 -- If the table has both a usable key and usable geometry
@@ -704,6 +706,7 @@ DECLARE
   rec RECORD;
   i INTEGER;
   sql TEXT;
+  useable_key BOOLEAN = false;
 BEGIN
 
   RAISE DEBUG 'Entered _CDB_Has_Usable_Primary_ID';
@@ -719,24 +722,65 @@ BEGIN
   AND NOT a.attisdropped
   AND a.attname = keyname;
 
-  -- It's perfect (named right, right type, right index)!
-  IF FOUND AND rec.indisprimary AND rec.indisunique AND rec.attnotnull AND rec.atttypid IN (20,21,23) THEN
-    RAISE DEBUG '_CDB_Has_Usable_Primary_ID found good ''%''', keyname;
-    RETURN true;
+  -- Found something named right...
+  IF FOUND THEN
   
-  -- It's an integer and it's named 'cartodb_id' maybe it is usable
-  -- ELSIF rec.atttypid IN (20,21,23) THEN
+    -- And it's an integer column...
+    IF rec.atttypid IN (20,21,23) THEN
+          
+      -- And it's a unique primary key! Done!
+      IF rec.indisprimary AND rec.indisunique AND rec.attnotnull THEN
+        RAISE DEBUG '_CDB_Has_Usable_Primary_ID found good ''%''', keyname;
+        RETURN true;
+
+      -- Check and see if the column values are unique, 
+      -- 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_unique UNIQUE (%s)', reloid::text, keyname, keyname);
+          RAISE DEBUG '_CDB_Has_Usable_Primary_ID: %', sql;
+          EXECUTE sql;
+          EXCEPTION      
+          -- Failed unique check...
+          WHEN unique_violation THEN
+            RAISE NOTICE '_CDB_Has_Usable_Primary_ID column % is not unique', keyname;
+            useable_key := false;
+          -- Other fatal error
+          WHEN others THEN
+            RAISE EXCEPTION 'Cartodbfying % (%s): % (%)', reloid::text, keyname, SQLERRM, SQLSTATE;
+        END;
   
-  
-  
-  -- It's not suitable (not an integer?, not unique?) to rename it out of the way
-  ELSIF FOUND THEN
-    RAISE DEBUG '_CDB_Has_Usable_Primary_ID found bad ''%'', renaming it', keyname;
+        -- Clean up test constraint
+        IF useable_key THEN
+          EXECUTE Format('ALTER TABLE %s DROP CONSTRAINT %s_unique', reloid::text, keyname);
+
+        -- Move non-unique column out of the way
+        ELSE
+
+          RAISE DEBUG '_CDB_Has_Usable_Primary_ID found non-unique ''%'', renaming it', keyname;
+          sql := Format('ALTER TABLE %s RENAME COLUMN %s TO %s', 
+                    reloid::text, rec.attname, _CDB_Unique_Column_Name(reloid, keyname));
+          RAISE DEBUG '_CDB_Has_Usable_Primary_ID: %', sql;
+          EXECUTE sql;
+        
+        END IF;
+
+      END IF;
     
-    sql := Format('ALTER TABLE %s RENAME COLUMN %s TO %s', 
-              reloid::text, rec.attname, _CDB_Unique_Column_Name(reloid, keyname));
-    RAISE DEBUG '_CDB_Has_Usable_Primary_ID: %', sql;
-    EXECUTE sql;        
+    -- It's not an integer column, we have to rename it
+    ELSE
+  
+      RAISE DEBUG '_CDB_Has_Usable_Primary_ID found non-integer ''%'', renaming it', keyname;    
+      sql := Format('ALTER TABLE %s RENAME COLUMN %s TO %s', 
+                reloid::text, rec.attname, _CDB_Unique_Column_Name(reloid, keyname));
+      RAISE DEBUG '_CDB_Has_Usable_Primary_ID: %', sql;
+      EXECUTE sql;        
+    
+    END IF;
     
   -- There's no column there named keyname
   ELSE

From 614a446cba935317ed2bc08cf06d7d4f14f6d679 Mon Sep 17 00:00:00 2001
From: Paul Ramsey <pramsey@cleverelephant.ca>
Date: Wed, 22 Apr 2015 09:29:23 -0700
Subject: [PATCH 06/34] Document functions a bit more

---
 scripts-available/CDB_CartodbfyTable.sql | 255 ++++++++++++-----------
 1 file changed, 135 insertions(+), 120 deletions(-)

diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql
index e366746..e20f383 100644
--- a/scripts-available/CDB_CartodbfyTable.sql
+++ b/scripts-available/CDB_CartodbfyTable.sql
@@ -692,8 +692,111 @@ $$ LANGUAGE PLPGSQL;
 -- -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
 
 
--- TODO: Handing the case of 'cartodb_id' column with integer non-primary key
--- TODO: Preserve that column, IFF it has unique values
+
+-- Find a unique relation name in the given schema, starting from the 
+-- template given. If the template is already unique, just return it; 
+-- otherwise, append an increasing integer until you find a unique variant.
+CREATE OR REPLACE FUNCTION _CDB_Unique_Relation_Name(schemaname TEXT, relationname TEXT)
+RETURNS TEXT
+AS $$
+DECLARE
+  rec RECORD;
+  i INTEGER;
+  newrelname TEXT;
+BEGIN
+
+  i := 0;
+  newrelname := relationname;
+  LOOP
+
+    SELECT c.relname, n.nspname
+    INTO rec
+    FROM pg_class c
+    JOIN pg_namespace n ON c.relnamespace = n.oid
+    WHERE c.relname = newrelname
+    AND n.nspname = schemaname;
+  
+    IF NOT FOUND THEN
+      RETURN newrelname;
+    END IF;
+    
+    i := i + 1;
+    newrelname := relationname || '_' || i;
+  
+    IF i > 100 THEN
+      RAISE EXCEPTION '_CDB_Unique_Relation_Name looping too far';
+    END IF;
+  
+  END LOOP;
+  
+END;
+$$ LANGUAGE 'plpgsql';
+
+
+-- Find a unique column name in the given relation, starting from the 
+-- column name given. If the column name is already unique, just return it; 
+-- otherwise, append an increasing integer until you find a unique variant.
+CREATE OR REPLACE FUNCTION _CDB_Unique_Column_Name(reloid REGCLASS, columnname TEXT)
+RETURNS TEXT
+AS $$
+DECLARE
+  rec RECORD;
+  i INTEGER;
+  newcolname TEXT;
+BEGIN
+
+  i := 0;
+  newcolname := columnname;
+  LOOP
+
+    SELECT a.attname
+    INTO rec
+    FROM pg_class c
+    JOIN pg_attribute a ON a.attrelid = c.oid
+    WHERE NOT a.attisdropped
+    AND a.attnum > 0
+    AND c.oid = reloid
+    AND a.attname = newcolname;
+  
+    IF NOT FOUND THEN
+      RETURN newcolname;
+    END IF;
+    
+    i := i + 1;
+    newcolname := columnname || '_' || i;
+  
+    IF i > 100 THEN
+      RAISE EXCEPTION '_CDB_Unique_Column_Name looping too far';
+    END IF;
+  
+  END LOOP;
+  
+END;
+$$ LANGUAGE 'plpgsql';
+    
+
+-- Return the geometry SRID of the very first entry in a given column.
+CREATE OR REPLACE FUNCTION _CDB_Geometry_SRID(reloid REGCLASS, columnname TEXT)
+RETURNS INTEGER
+AS $$
+DECLARE
+  rec RECORD;
+BEGIN
+
+  RAISE DEBUG '_CDB_Geometry_SRID, entered';
+  
+  EXECUTE Format('SELECT ST_SRID(%s) AS srid FROM %s LIMIT 1', columnname, reloid::text)
+  INTO rec;
+
+  IF FOUND THEN 
+    RETURN rec.srid;
+  ELSE
+    RETURN 0;
+  END IF;
+  
+END;
+$$ LANGUAGE 'plpgsql';
+
 
 -- Find out if the table already has a usable primary key
 -- If the table has both a usable key and usable geometry
@@ -805,23 +908,6 @@ BEGIN
     END IF;
   
   END IF;
-
-  -- Remove any unsuitable primary key constraint that is hanging around, 
-  -- because we will be adding one back later
-  SELECT ci.relname AS pkey
-  INTO rec
-  FROM pg_class c 
-  JOIN pg_attribute a ON a.attrelid = c.oid 
-  LEFT JOIN pg_index i ON c.oid = i.indrelid AND a.attnum = ANY(i.indkey)
-  JOIN pg_class ci ON i.indexrelid = ci.oid
-  WHERE c.oid = reloid AND NOT a.attisdropped
-  AND a.attname != keyname
-  AND i.indisprimary AND a.atttypid NOT IN (20,21,23);
-  
-  IF FOUND THEN
-    EXECUTE Format('ALTER TABLE %s DROP CONSTRAINT IF EXISTS %s', reloid::text, rec.pkey);
-    RAISE DEBUG '_CDB_Has_Usable_Primary_ID dropping unused primary key ''%''', rec.pkey;
-  END IF;
   
   RAISE DEBUG '_CDB_Has_Usable_Primary_ID completed';
   
@@ -832,105 +918,6 @@ END;
 $$ LANGUAGE 'plpgsql';
 
 
-CREATE OR REPLACE FUNCTION _CDB_Unique_Relation_Name(schemaname TEXT, relationname TEXT)
-RETURNS TEXT
-AS $$
-DECLARE
-  rec RECORD;
-  i INTEGER;
-  newrelname TEXT;
-BEGIN
-
-  i := 0;
-  newrelname := relationname;
-  LOOP
-
-    SELECT c.relname, n.nspname
-    INTO rec
-    FROM pg_class c
-    JOIN pg_namespace n ON c.relnamespace = n.oid
-    WHERE c.relname = newrelname
-    AND n.nspname = schemaname;
-  
-    IF NOT FOUND THEN
-      RETURN newrelname;
-    END IF;
-    
-    i := i + 1;
-    newrelname := relationname || '_' || i;
-  
-    IF i > 100 THEN
-      RAISE EXCEPTION '_CDB_Unique_Relation_Name looping too far';
-    END IF;
-  
-  END LOOP;
-  
-END;
-$$ LANGUAGE 'plpgsql';
-
-
-CREATE OR REPLACE FUNCTION _CDB_Unique_Column_Name(reloid REGCLASS, columnname TEXT)
-RETURNS TEXT
-AS $$
-DECLARE
-  rec RECORD;
-  i INTEGER;
-  newcolname TEXT;
-BEGIN
-
-  i := 0;
-  newcolname := columnname;
-  LOOP
-
-    SELECT a.attname
-    INTO rec
-    FROM pg_class c
-    JOIN pg_attribute a ON a.attrelid = c.oid
-    WHERE NOT a.attisdropped
-    AND a.attnum > 0
-    AND c.oid = reloid
-    AND a.attname = newcolname;
-  
-    IF NOT FOUND THEN
-      RETURN newcolname;
-    END IF;
-    
-    i := i + 1;
-    newcolname := columnname || '_' || i;
-  
-    IF i > 100 THEN
-      RAISE EXCEPTION '_CDB_Unique_Column_Name looping too far';
-    END IF;
-  
-  END LOOP;
-  
-END;
-$$ LANGUAGE 'plpgsql';
-    
-
-CREATE OR REPLACE FUNCTION _CDB_Geometry_SRID(reloid REGCLASS, columnname TEXT)
-RETURNS INTEGER
-AS $$
-DECLARE
-  rec RECORD;
-BEGIN
-
-  RAISE DEBUG '_CDB_Geometry_SRID, entered';
-  
-  EXECUTE Format('SELECT ST_SRID(%s) AS srid FROM %s LIMIT 1', columnname, reloid::text)
-  INTO rec;
-
-  IF FOUND THEN 
-    RETURN rec.srid;
-  ELSE
-    RETURN 0;
-  END IF;
-  
-END;
-$$ LANGUAGE 'plpgsql';
-
-
-
 CREATE OR REPLACE FUNCTION _CDB_Has_Usable_Geom(reloid REGCLASS, geom_name TEXT, mercgeom_name TEXT)
   RETURNS BOOLEAN
 AS $$
@@ -1005,7 +992,11 @@ END;
 $$ LANGUAGE 'plpgsql';
 
 
-
+-- Create a copy of the table. Assumes that the "Has usable" functions
+-- have already been run, so that if there is a 'cartodb_id' column, it is
+-- 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, has_usable_primary_key BOOLEAN, has_usable_geoms BOOLEAN, geom_name TEXT, mercgeom_name TEXT, primary_key_name TEXT)
 RETURNS BOOLEAN
 AS $$
@@ -1221,7 +1212,9 @@ END;
 $$ LANGUAGE 'plpgsql';
 
 
-
+-- Assumes the table already has the right metadata columns
+-- (primary key and two geometry columns) and adds primary key
+-- and geometry indexes if necessary.
 CREATE OR REPLACE FUNCTION _CDB_Add_Indexes(reloid REGCLASS, geom_name TEXT, mercgeom_name TEXT, primary_key_name TEXT)
   RETURNS BOOLEAN
 AS $$
@@ -1240,6 +1233,28 @@ BEGIN
   FROM pg_class c
   WHERE c.oid = reloid;
 
+
+  -- Is there already a primary key on this table for 
+  -- a column other than our chosen primary key?
+  SELECT ci.relname AS pkey
+  INTO rec
+  FROM pg_class c 
+  JOIN pg_attribute a ON a.attrelid = c.oid 
+  LEFT JOIN pg_index i ON c.oid = i.indrelid AND a.attnum = ANY(i.indkey)
+  JOIN pg_class ci ON i.indexrelid = ci.oid
+  WHERE c.oid = reloid 
+  AND NOT a.attisdropped
+  AND a.attname != primary_key_name
+  AND i.indisprimary;
+
+  -- Yes? Then drop it, we're adding our own PK to the column
+  -- we prefer.
+  IF FOUND THEN
+    EXECUTE Format('ALTER TABLE %s DROP CONSTRAINT IF EXISTS %s', reloid::text, rec.pkey);
+    RAISE DEBUG '_CDB_Add_Indexes dropping unwanted primary key ''%''', rec.pkey;
+  END IF;
+
+
   -- Is the default primary key flagged as primary?
   SELECT a.attname
   INTO rec

From dd209d02f69f2daa5326aeb373b5d48368aee1e6 Mon Sep 17 00:00:00 2001
From: Paul Ramsey <pramsey@cleverelephant.ca>
Date: Wed, 22 Apr 2015 12:51:36 -0700
Subject: [PATCH 07/34] Use standard error message format

---
 scripts-available/CDB_CartodbfyTable.sql | 282 ++++++++++++++---------
 1 file changed, 178 insertions(+), 104 deletions(-)

diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql
index e20f383..69509d7 100644
--- a/scripts-available/CDB_CartodbfyTable.sql
+++ b/scripts-available/CDB_CartodbfyTable.sql
@@ -692,6 +692,45 @@ $$ LANGUAGE PLPGSQL;
 -- -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
 
 
+CREATE OR REPLACE FUNCTION _CDB_Columns(OUT pkey TEXT, OUT geomcol TEXT, OUT mercgeomcol TEXT)
+RETURNS record
+AS $$
+BEGIN
+
+pkey := 'cartodb_id';
+geomcol := 'the_geom';
+mercgeomcol := 'the_geom_webmercator';
+
+END;
+$$ LANGUAGE 'plpgsql';
+
+
+CREATE OR REPLACE FUNCTION _CDB_Error(message TEXT, funcname TEXT DEFAULT '_CDB_Error')
+RETURNS void
+AS $$
+BEGIN
+
+  RAISE EXCEPTION 'CDB(%): %', funcname, message;
+
+END;
+$$ LANGUAGE 'plpgsql';
+
+
+CREATE OR REPLACE FUNCTION _CDB_SQL(sql TEXT, funcname TEXT DEFAULT '_CDB_SQL')
+RETURNS void
+AS $$
+BEGIN
+
+  RAISE DEBUG 'CDB(%): %', funcname, sql;
+  EXECUTE sql;
+
+  EXCEPTION
+  WHEN others THEN
+    RAISE EXCEPTION 'CDB(%:%:%): %', funcname, SQLSTATE, SQLERRM, sql;
+
+END;
+$$ LANGUAGE 'plpgsql';
+
 
 -- Find a unique relation name in the given schema, starting from the 
 -- template given. If the template is already unique, just return it; 
@@ -724,7 +763,7 @@ BEGIN
     newrelname := relationname || '_' || i;
   
     IF i > 100 THEN
-      RAISE EXCEPTION '_CDB_Unique_Relation_Name looping too far';
+      PERFORM _CDB_Error('looping too far', '_CDB_Unique_Relation_Name');
     END IF;
   
   END LOOP;
@@ -766,7 +805,7 @@ BEGIN
     newcolname := columnname || '_' || i;
   
     IF i > 100 THEN
-      RAISE EXCEPTION '_CDB_Unique_Column_Name looping too far';
+      PERFORM _CDB_Error('looping too far', '_CDB_Unique_Column_Name');
     END IF;
   
   END LOOP;
@@ -775,7 +814,8 @@ END;
 $$ LANGUAGE 'plpgsql';
     
 
--- Return the geometry SRID of the very first entry in a given column.
+-- Return the geometry SRID from the column metadata or
+-- the geometry of the very first entry in a given column.
 CREATE OR REPLACE FUNCTION _CDB_Geometry_SRID(reloid REGCLASS, columnname TEXT)
 RETURNS INTEGER
 AS $$
@@ -783,16 +823,16 @@ DECLARE
   rec RECORD;
 BEGIN
 
-  RAISE DEBUG '_CDB_Geometry_SRID, entered';
+  RAISE DEBUG 'CDB(%): %', '_CDB_Geometry_SRID', 'entered function';
   
   EXECUTE Format('SELECT ST_SRID(%s) AS srid FROM %s LIMIT 1', columnname, reloid::text)
   INTO rec;
 
   IF FOUND THEN 
     RETURN rec.srid;
-  ELSE
-    RETURN 0;
   END IF;
+
+  RETURN 0;
   
 END;
 $$ LANGUAGE 'plpgsql';
@@ -802,17 +842,21 @@ $$ 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
-CREATE OR REPLACE FUNCTION _CDB_Has_Usable_Primary_ID(reloid REGCLASS, keyname TEXT)
+CREATE OR REPLACE FUNCTION _CDB_Has_Usable_Primary_ID(reloid REGCLASS)
   RETURNS BOOLEAN
 AS $$
 DECLARE
   rec RECORD;
+  const RECORD;
   i INTEGER;
   sql TEXT;
   useable_key BOOLEAN = false;
 BEGIN
 
-  RAISE DEBUG 'Entered _CDB_Has_Usable_Primary_ID';
+  RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', 'entered function';
+
+  -- Read in the names of the CartoDB columns
+  const := _CDB_Columns();
 
   -- Do we already have a properly named column?
   SELECT a.attname, i.indisprimary, i.indisunique, a.attnotnull, a.atttypid
@@ -823,7 +867,7 @@ BEGIN
   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 a.attname = keyname;
+  AND a.attname = const.pkey;
 
   -- Found something named right...
   IF FOUND THEN
@@ -833,7 +877,7 @@ BEGIN
           
       -- And it's a unique primary key! Done!
       IF rec.indisprimary AND rec.indisunique AND rec.attnotnull THEN
-        RAISE DEBUG '_CDB_Has_Usable_Primary_ID found good ''%''', keyname;
+        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, 
@@ -844,48 +888,55 @@ BEGIN
         useable_key := true;
       
         BEGIN
-          sql := Format('ALTER TABLE %s ADD CONSTRAINT %s_unique UNIQUE (%s)', reloid::text, keyname, keyname);
-          RAISE DEBUG '_CDB_Has_Usable_Primary_ID: %', sql;
+          sql := Format('ALTER TABLE %s ADD CONSTRAINT %s_unique UNIQUE (%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 NOTICE '_CDB_Has_Usable_Primary_ID column % is not unique', keyname;
+            RAISE NOTICE 'CDB(_CDB_Has_Usable_Primary_ID): %', Format('column %s is not unique', const.pkey);
             useable_key := false;
           -- Other fatal error
           WHEN others THEN
-            RAISE EXCEPTION 'Cartodbfying % (%s): % (%)', reloid::text, keyname, SQLERRM, SQLSTATE;
+            PERFORM _CDB_Error(sql, '_CDB_Has_Usable_Primary_ID');          
         END;
   
         -- Clean up test constraint
         IF useable_key THEN
-          EXECUTE Format('ALTER TABLE %s DROP CONSTRAINT %s_unique', reloid::text, keyname);
+          PERFORM _CDB_SQL(Format('ALTER TABLE %s DROP CONSTRAINT %s_unique', 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);
 
-          RAISE DEBUG '_CDB_Has_Usable_Primary_ID found non-unique ''%'', renaming it', keyname;
-          sql := Format('ALTER TABLE %s RENAME COLUMN %s TO %s', 
-                    reloid::text, rec.attname, _CDB_Unique_Column_Name(reloid, keyname));
-          RAISE DEBUG '_CDB_Has_Usable_Primary_ID: %', sql;
-          EXECUTE sql;
+          PERFORM _CDB_SQL(
+            Format('ALTER TABLE %s RENAME COLUMN %s TO %s', 
+              reloid::text, rec.attname, 
+              _CDB_Unique_Column_Name(reloid, const.pkey)), 
+            '_CDB_Has_Usable_Primary_ID');
         
         END IF;
+        
+        return useable_key;
 
       END IF;
     
     -- It's not an integer column, we have to rename it
     ELSE
   
-      RAISE DEBUG '_CDB_Has_Usable_Primary_ID found non-integer ''%'', renaming it', keyname;    
-      sql := Format('ALTER TABLE %s RENAME COLUMN %s TO %s', 
-                reloid::text, rec.attname, _CDB_Unique_Column_Name(reloid, keyname));
-      RAISE DEBUG '_CDB_Has_Usable_Primary_ID: %', sql;
-      EXECUTE sql;        
+      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 %s', 
+                reloid::text, rec.attname, _CDB_Unique_Column_Name(reloid, const.pkey)), 
+                '_CDB_Has_Usable_Primary_ID');
     
     END IF;
     
-  -- There's no column there named keyname
+  -- There's no column there named pkey
   ELSE
 
     -- Is there another suitable primary key already?
@@ -900,37 +951,43 @@ BEGIN
   
     -- Yes! Ok, rename it.
     IF FOUND THEN
-      EXECUTE Format('ALTER TABLE %s RENAME COLUMN %s TO %s', reloid::text, rec.attname, keyname);
-      RAISE DEBUG '_CDB_Has_Usable_Primary_ID found acceptable primary key ''%s'', renaming to ''%''', rec.attname, keyname;
+      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;
     ELSE
-      RAISE DEBUG '_CDB_Has_Usable_Primary_ID found no useful column for ''%''', keyname;
+      RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', 
+        Format('found no useful column for ''%s''', const.pkey);
     END IF;
   
   END IF;
   
-  RAISE DEBUG '_CDB_Has_Usable_Primary_ID completed';
-  
-  -- Didn't fine re-usable key, so return FALSE
+  RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', 'function complete';
+
+  -- Didn't find re-usable key, so return FALSE
   RETURN false;
 
 END;
 $$ LANGUAGE 'plpgsql';
 
 
-CREATE OR REPLACE FUNCTION _CDB_Has_Usable_Geom(reloid REGCLASS, geom_name TEXT, mercgeom_name TEXT)
+CREATE OR REPLACE FUNCTION _CDB_Has_Usable_Geom(reloid REGCLASS)
   RETURNS BOOLEAN
 AS $$
 DECLARE
   r1 RECORD;
   r2 RECORD;
-  found_geom BOOLEAN := false;
+  const RECORD;
   has_geom BOOLEAN := false;
   has_mercgeom BOOLEAN := false;
+  has_geom_name TEXT;
+  has_mercgeom_name TEXT;
   str TEXT;
+  sql TEXT;
 BEGIN
 
-  RAISE DEBUG 'Entered _CDB_Has_Usable_Geom';
+  RAISE DEBUG 'CDB(_CDB_Has_Usable_Geom): %', 'entered function';
+
+  -- Read in the names of the CartoDB columns
+  const := _CDB_Columns();
 
   -- Do we have a column we can use?
   FOR r1 IN
@@ -942,7 +999,7 @@ BEGIN
     FROM pg_class c 
     JOIN pg_attribute a ON a.attrelid = c.oid 
     JOIN pg_type t ON a.atttypid = t.oid,
-    (VALUES (geom_name, 4326), (mercgeom_name, 3857) ) as f(desired_attname, desired_srid)
+    (VALUES (const.geomcol, 4326), (const.mercgeomcol, 3857) ) as f(desired_attname, desired_srid)
     WHERE c.oid = reloid
     AND a.attnum > 0
     AND NOT a.attisdropped
@@ -950,14 +1007,15 @@ BEGIN
     ORDER BY t.oid ASC
   LOOP
   
-    RAISE DEBUG '_CDB_Has_Usable_Geom, checking ''%''', r1.attname;
-    found_geom := false;
+    RAISE DEBUG 'CDB(_CDB_Has_Usable_Geom): %', Format('checking column ''%s''', r1.attname);
 
     -- Name collision: right name but wrong type, rename it!
     IF r1.typname != 'geometry' AND r1.attname = r1.desired_attname THEN
       str := _CDB_Unique_Column_Name(reloid, r1.attname);
-      EXECUTE Format('ALTER TABLE %s RENAME COLUMN %s TO %s', reloid::text, r1.attname, str);
-      RAISE DEBUG '_CDB_Has_Usable_Geom: % is the wrong type, renamed to %', r1.attname, str;
+      sql := Format('ALTER TABLE %s RENAME COLUMN %s TO %s', reloid::text, r1.attname, str);
+      PERFORM _CDB_SQL(sql,'_CDB_Has_Usable_Geom');
+      RAISE DEBUG 'CDB(_CDB_Has_Usable_Geom): %', 
+        Format('%s is the wrong type, renamed to %s', r1.attname, str);
 
     -- Found a geometry column!
     ELSIF r1.typname = 'geometry' THEN
@@ -965,18 +1023,15 @@ BEGIN
       -- If it's the right SRID, we can use it in place without
       -- transforming it!
       IF r1.srid = r1.desired_srid OR _CDB_Geometry_SRID(reloid, r1.attname) = r1.desired_srid THEN
-        RAISE DEBUG '_CDB_Has_Usable_Geom found acceptable ''%''', r1.attname;
 
-        -- If it's the wrong name, just rename it.
-        IF r1.attname != r1.desired_attname THEN
-          EXECUTE Format('ALTER TABLE %s RENAME COLUMN %s TO %s', reloid::text, r1.attname, r1.desired_attname);
-          RAISE DEBUG '_CDB_Has_Usable_Geom renamed % to %', r1.attname, r1.desired_attname;
-        END IF;
+        RAISE DEBUG 'CDB(_CDB_Has_Usable_Geom): %', Format('found acceptable ''%s''', r1.attname);
 
-        IF r1.desired_attname = geom_name THEN
-          has_geom = true;
-        ELSIF r1.desired_attname = mercgeom_name THEN
-          has_mercgeom = true;
+        IF r1.desired_attname = const.geomcol THEN
+          has_geom := true;
+          has_geom_name := r1.attname;
+        ELSIF r1.desired_attname = const.mercgeomcol THEN
+          has_mercgeom := true;
+          has_mercgeom_name := r1.attname;
         END IF;
         
       END IF;
@@ -984,6 +1039,18 @@ BEGIN
     END IF;
     
   END LOOP;
+
+  -- If geom is the wrong name, just rename it.
+  IF has_geom AND has_geom_name != const.geomcol THEN  
+    sql := Format('ALTER TABLE %s RENAME COLUMN %s TO %s', reloid::text, has_geom_name, const.geomcol);
+    PERFORM _CDB_SQL(sql,'_CDB_Has_Usable_Geom');
+  END IF;
+
+  -- If mercgeom is the wrong name, just rename it.
+  IF has_mercgeom AND has_mercgeom_name != const.geomcol THEN  
+    sql := Format('ALTER TABLE %s RENAME COLUMN %s TO %s', reloid::text, has_mercgeom_name, const.mercgeomcol);
+    PERFORM _CDB_SQL(sql,'_CDB_Has_Usable_Geom');
+  END IF;
   
   -- If table is perfect (no transforms required), return TRUE!
   RETURN has_geom AND has_mercgeom;
@@ -997,7 +1064,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, has_usable_primary_key BOOLEAN, has_usable_geoms BOOLEAN, geom_name TEXT, mercgeom_name TEXT, primary_key_name TEXT)
+CREATE OR REPLACE FUNCTION _CDB_Rewrite_Table(reloid REGCLASS, destschema TEXT, has_usable_primary_key BOOLEAN, has_usable_geoms BOOLEAN)
 RETURNS BOOLEAN
 AS $$
 DECLARE
@@ -1018,16 +1085,21 @@ DECLARE
   geom_column_source TEXT := '';
 
   rec RECORD;
+  const RECORD;
   sql TEXT;
   str TEXT;
+  table_srid INTEGER;
   
 BEGIN
 
-  RAISE DEBUG 'Entered _CDB_Rewrite_Table';
+  RAISE DEBUG 'CDB(_CDB_Rewrite_Table): %', 'entered function';
 
-  -- Check calling convention
+  -- Read CartoDB standard column names in
+  const := _CDB_Columns();
+
+  -- No-op if there is no rewrite to be done
   IF has_usable_primary_key AND has_usable_geoms THEN
-    RAISE EXCEPTION '_CDB_Rewrite_Table should not be called, it has good key and geoms';
+    RETURN true;
   END IF;
 
   -- Save the raw schema/table names for later
@@ -1039,10 +1111,10 @@ BEGIN
   -- Put the primary key sequence in the right schema
   -- If the new table is not moving, better ensure the sequence name
   -- is unique
-  destseq := relname || '_' || primary_key_name || '_seq';
+  destseq := relname || '_' || const.pkey || '_seq';
   destseq := _CDB_Unique_Relation_Name(destschema, destseq);
   destseq := Format('%s.%s', destschema, destseq);
-  EXECUTE Format('CREATE SEQUENCE %s', destseq);
+  PERFORM _CDB_SQL(Format('CREATE SEQUENCE %s', destseq), '_CDB_Rewrite_Table');
 
   -- Salt a temporary table name if we are re-writing in place
   IF destschema = relschema THEN
@@ -1056,14 +1128,14 @@ BEGIN
 
   -- Add cartodb ID!
   IF has_usable_primary_key THEN
-    sql := sql || primary_key_name;
+    sql := sql || const.pkey;
   ELSE
-    sql := sql || 'nextval(''' || destseq || ''') AS ' || primary_key_name;
+    sql := sql || 'nextval(''' || destseq || ''') AS ' || const.pkey;
   END IF;
 
   -- Add the geometry columns!
   IF has_usable_geoms THEN
-    sql := sql || ',' || geom_name || ',' || mercgeom_name;
+    sql := sql || ',' || const.geomcol || ',' || const.mercgeomcol;
   ELSE
     
     -- This gets complicated: we have to make sure the 
@@ -1091,6 +1163,8 @@ BEGIN
 
     ELSE
 
+      -- table_srid = _CDB_Geometry_SRID(reloid, rec.attname);
+       
       EXECUTE Format('SELECT ST_SRID(%s) AS srid FROM %s LIMIT 1', rec.attname, reloid::text)
       INTO rec;
     
@@ -1105,19 +1179,20 @@ BEGIN
         || ',4326)::Geometry(' 
         || postgis_typmod_type(a.atttypmod) 
         || ', 4326) AS '
-        || geom_name 
+        || const.geomcol 
         || ', ST_Transform(' 
         || a.attname 
         || ',3857)::Geometry('
         || postgis_typmod_type(a.atttypmod) 
         || ', 3857) AS '
-        || mercgeom_name,
+        || const.mercgeomcol,
         a.attname
       INTO geom_transform_sql, geom_column_source
       FROM pg_class c 
       JOIN pg_attribute a ON a.attrelid = c.oid 
       JOIN pg_type t ON a.atttypid = t.oid,
       ( SELECT rec.srid AS srid ) AS srid
+      -- ( SELECT table_srid AS srid ) AS srid
       WHERE c.oid = reloid
       AND t.typname = 'geometry'
       AND a.attnum > 0
@@ -1146,25 +1221,25 @@ BEGIN
   JOIN pg_type t ON a.atttypid = t.oid
   WHERE c.oid = reloid
   AND a.attnum > 0
-  AND a.attname NOT IN (geom_name, mercgeom_name, primary_key_name, geom_column_source)
+  AND a.attname NOT IN (const.geomcol, const.mercgeomcol, const.pkey, geom_column_source)
   AND NOT a.attisdropped;
 
 
   -- No non-cartodb columns? Possible, I guess.
   IF rec.count = 0 THEN
-    RAISE DEBUG '_CDB_Rewrite_Table found no extra columns';
+    RAISE DEBUG 'CDB(_CDB_Rewrite_Table): %', 'found no extra columns';
     column_name_sql := '';
   ELSE
-    RAISE DEBUG '_CDB_Rewrite_Table found extra columns columns %', rec.column_name_sql;
+    RAISE DEBUG 'CDB(_CDB_Rewrite_Table): %', Format('found extra columns columns ''%s''', rec.column_name_sql);
     column_name_sql := rec.column_name_sql;
   END IF;
 
   -- Add the source table to the SQL
   sql := sql || column_name_sql || ' FROM ' || reloid::text;
-  RAISE DEBUG '_CDB_Rewrite_Table generated SQL: %', sql;
+  RAISE DEBUG 'CDB(_CDB_Rewrite_Table): %', sql;
 
   -- Run it!
-  EXECUTE sql;
+  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
@@ -1172,38 +1247,34 @@ BEGIN
   IF has_usable_primary_key THEN
 
     EXECUTE Format('SELECT max(%s) FROM %s',
-            primary_key_name, copyname)
+            const.pkey, copyname)
        INTO destseqmax;
 
     IF FOUND AND destseqmax IS NOT NULL THEN
-      EXECUTE Format('SELECT setval(''%s'', %s)', destseq, destseqmax);
+      PERFORM _CDB_SQL(Format('SELECT setval(''%s'', %s)', destseq, destseqmax), '_CDB_Rewrite_Table');
     END IF;
 
   END IF;
 
   -- Make the primary key use the sequence as its default value
   sql := Format('ALTER TABLE %s ALTER COLUMN %I SET DEFAULT nextval(''%s'')', 
-          copyname, primary_key_name, destseq);
-  RAISE DEBUG '_CDB_Rewrite_Table: %', sql;
-  EXECUTE sql;
+          copyname, const.pkey, destseq);
+  PERFORM _CDB_SQL(sql, '_CDB_Rewrite_Table');
 
   -- Make the sequence owned by the table, so when the table drops, 
   -- the sequence does too
-  sql := Format('ALTER SEQUENCE %s OWNED BY %s.%s', destseq, copyname, primary_key_name);
-  RAISE DEBUG '_CDB_Rewrite_Table: %', sql;
-  EXECUTE sql;
+  sql := Format('ALTER SEQUENCE %s OWNED BY %s.%s', destseq, copyname, const.pkey);
+  PERFORM _CDB_SQL(sql,'_CDB_Rewrite_Table');
   
   -- We just made a copy, so we can drop the original now
   sql := Format('DROP TABLE %s', reloid::text);
-  RAISE DEBUG '_CDB_Rewrite_Table: %', sql;
-  EXECUTE sql;
+  PERFORM _CDB_SQL(sql, '_CDB_Rewrite_Table');
   
   -- If we used a temporary destination table
   -- we can now rename it into place
   IF destschema = relschema THEN
     sql := Format('ALTER TABLE %s RENAME TO %s', copyname, destname);
-    RAISE DEBUG '_CDB_Rewrite_Table: %', sql;
-    EXECUTE sql;
+    PERFORM _CDB_SQL(sql, '_CDB_Rewrite_Table');
   END IF;
 
   RETURN true;
@@ -1215,17 +1286,21 @@ $$ LANGUAGE 'plpgsql';
 -- Assumes the table already has the right metadata columns
 -- (primary key and two geometry columns) and adds primary key
 -- and geometry indexes if necessary.
-CREATE OR REPLACE FUNCTION _CDB_Add_Indexes(reloid REGCLASS, geom_name TEXT, mercgeom_name TEXT, primary_key_name TEXT)
+CREATE OR REPLACE FUNCTION _CDB_Add_Indexes(reloid REGCLASS)
   RETURNS BOOLEAN
 AS $$
 DECLARE
   rec RECORD;
+  const RECORD;
   iname TEXT;
   sql TEXT;
   relname TEXT;
 BEGIN
 
-  RAISE DEBUG 'Entered _CDB_Add_Indexes';
+  RAISE DEBUG 'CDB(_CDB_Add_Indexes): %', 'entered function';
+
+  -- Read CartoDB standard column names in
+  const := _CDB_Columns();
 
   -- Extract just the relname to use for the index names
   SELECT c.relname
@@ -1233,7 +1308,6 @@ BEGIN
   FROM pg_class c
   WHERE c.oid = reloid;
 
-
   -- Is there already a primary key on this table for 
   -- a column other than our chosen primary key?
   SELECT ci.relname AS pkey
@@ -1244,14 +1318,15 @@ BEGIN
   JOIN pg_class ci ON i.indexrelid = ci.oid
   WHERE c.oid = reloid 
   AND NOT a.attisdropped
-  AND a.attname != primary_key_name
+  AND a.attname != const.pkey
   AND i.indisprimary;
 
   -- Yes? Then drop it, we're adding our own PK to the column
   -- we prefer.
   IF FOUND THEN
-    EXECUTE Format('ALTER TABLE %s DROP CONSTRAINT IF EXISTS %s', reloid::text, rec.pkey);
-    RAISE DEBUG '_CDB_Add_Indexes dropping unwanted primary key ''%''', rec.pkey;
+    RAISE DEBUG 'CDB(_CDB_Add_Indexes): dropping unwanted primary key ''%''', rec.pkey;
+    sql := Format('ALTER TABLE %s DROP CONSTRAINT IF EXISTS %s', reloid::text, rec.pkey);
+    PERFORM _CDB_SQL(sql, '_CDB_Add_Indexes');
   END IF;
 
 
@@ -1264,16 +1339,15 @@ BEGIN
   JOIN pg_class ci ON ci.oid = i.indexrelid
   WHERE attnum > 0 
   AND c.oid = reloid
-  AND a.attname = primary_key_name
+  AND a.attname = const.pkey
   AND i.indisprimary
   AND i.indisunique
   AND NOT attisdropped;
   
   -- No primary key? Add one.
   IF NOT FOUND THEN
-    sql := Format('ALTER TABLE %s ADD PRIMARY KEY (%s)', reloid::text, primary_key_name);
-    RAISE DEBUG '_CDB_Add_Indexes: %', sql;
-    EXECUTE sql;
+    sql := Format('ALTER TABLE %s ADD PRIMARY KEY (%s)', reloid::text, const.pkey);
+    PERFORM _CDB_SQL(sql, '_CDB_Add_Indexes');
   END IF;
   
   -- Add geometry indexes to all "special geometry columns" that 
@@ -1285,7 +1359,7 @@ BEGIN
     JOIN pg_attribute a ON a.attrelid = c.oid AND attnum > 0 
     LEFT JOIN pg_index i ON c.oid = i.indrelid AND a.attnum = ANY(i.indkey)
     WHERE NOT attisdropped
-    AND a.attname IN (geom_name, mercgeom_name)
+    AND a.attname IN (const.geomcol, const.mercgeomcol)
     AND c.oid = reloid
     AND i.indexrelid IS NULL
     UNION 
@@ -1297,13 +1371,12 @@ BEGIN
     JOIN pg_class ci ON ci.oid = i.indexrelid
     JOIN pg_am am ON ci.relam = am.oid
     WHERE NOT attisdropped
-    AND a.attname IN (geom_name, mercgeom_name)
+    AND a.attname IN (const.geomcol, const.mercgeomcol)
     AND c.oid = reloid
     AND am.amname != 'gist'
   LOOP
     sql := Format('CREATE INDEX %s_%s_gix ON %s USING GIST (%s)', relname, rec.attname, reloid::text, rec.attname);
-    RAISE DEBUG '_CDB_Add_Indexes: %', sql;
-    EXECUTE sql;
+    PERFORM _CDB_SQL(sql, '_CDB_Add_Indexes');
   END LOOP;
     
   RETURN true;
@@ -1316,10 +1389,6 @@ CREATE OR REPLACE FUNCTION CDB_CartodbfyTable2(reloid REGCLASS, destschema TEXT
 RETURNS void 
 AS $$
 DECLARE
-  -- Because we're going to change these some day, ha ha ha ha!
-  geom_name TEXT := 'the_geom';
-  mercgeom_name TEXT := 'the_geom_webmercator';
-  primary_key_name TEXT := 'cartodb_id';
   
   relname TEXT;
   relschema TEXT;
@@ -1333,6 +1402,7 @@ DECLARE
   rewrite BOOLEAN;
   index_success BOOLEAN;
   rec RECORD;
+  
 BEGIN
 
   -- Save the raw schema/table names for later
@@ -1360,15 +1430,19 @@ 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, primary_key_name) AS has_usable_primary_key
+  SELECT _CDB_Has_Usable_Primary_ID(reloid) AS has_usable_primary_key
   INTO STRICT has_usable_primary_key;
 
+  RAISE DEBUG 'CDB(CDB_CartodbfyTable2): has_usable_primary_key %', has_usable_primary_key;
+
   -- See if the geometry columns we need are already available
   -- on the table. If they are, we don't need to do any bulk
   -- transformation of the table, we can just ensure proper
   -- indexes are in place and apply a rename
-  SELECT _CDB_Has_Usable_Geom(reloid, geom_name, mercgeom_name) AS has_usable_geoms
+  SELECT _CDB_Has_Usable_Geom(reloid) AS has_usable_geoms
   INTO STRICT has_usable_geoms;
+
+  RAISE DEBUG 'CDB(CDB_CartodbfyTable2): has_usable_geoms %', has_usable_geoms;
   
   -- We can only avoid a rewrite if both the key and 
   -- geometry are usable
@@ -1378,22 +1452,22 @@ BEGIN
   -- a destination schema, so do that now
   IF NOT rewrite AND destschema != relschema THEN
     
-    RAISE DEBUG 'perfect table needs to be moved to schema (%)', destschema;
+    RAISE DEBUG 'CDB(CDB_CartodbfyTable2): perfect table needs to be moved to schema (%)', destschema;
     EXECUTE Format('ALTER TABLE %s SET SCHEMA %s', reloid::text, destschema);
 
   -- Don't move anything, just make sure our destination information is set right
   ELSIF NOT rewrite AND destschema = relschema THEN
 
-    RAISE DEBUG 'perfect table in the perfect place';
+    RAISE DEBUG 'CDB(CDB_CartodbfyTable2): perfect table in the perfect place';
   
   -- We must rewrite, so here we go...
   ELSIF rewrite THEN
 
-    SELECT _CDB_Rewrite_Table(reloid, destschema, has_usable_primary_key, has_usable_geoms, geom_name, mercgeom_name, primary_key_name)
+    SELECT _CDB_Rewrite_Table(reloid, destschema, has_usable_primary_key, has_usable_geoms)
     INTO STRICT rewrite_success;
     
     IF NOT rewrite_success THEN
-      RAISE EXCEPTION 'Cartodbfying % (rewriting table): % (%)', reloid, SQLERRM, SQLSTATE;
+      PERFORM _CDB_Error('rewriting table', 'CDB_CartodbfyTable2');
     END IF;
       
   END IF;
@@ -1402,11 +1476,11 @@ BEGIN
   destoid := (destschema || '.' || destname)::regclass;
 
   -- Add indexes to the destination table, as necessary
-  SELECT _CDB_Add_Indexes(destoid, geom_name, mercgeom_name, primary_key_name)
+  SELECT _CDB_Add_Indexes(destoid)
   INTO STRICT index_success;
 
   IF NOT index_success THEN
-    RAISE EXCEPTION 'Cartodbfying % (indexing table): % (%)', destoid, SQLERRM, SQLSTATE;
+    PERFORM _CDB_Error('indexing table', 'CDB_CartodbfyTable2');
   END IF;
   
   -- Add triggers to the destination table, as necessary

From c1bfef25e0153e31b3b510bffad7c393745e1e41 Mon Sep 17 00:00:00 2001
From: Paul Ramsey <pramsey@cleverelephant.ca>
Date: Wed, 22 Apr 2015 13:06:34 -0700
Subject: [PATCH 08/34] Fix no-op case error

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

diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql
index 69509d7..06a32a6 100644
--- a/scripts-available/CDB_CartodbfyTable.sql
+++ b/scripts-available/CDB_CartodbfyTable.sql
@@ -1047,7 +1047,7 @@ BEGIN
   END IF;
 
   -- If mercgeom is the wrong name, just rename it.
-  IF has_mercgeom AND has_mercgeom_name != const.geomcol THEN  
+  IF has_mercgeom AND has_mercgeom_name != const.mercgeomcol THEN  
     sql := Format('ALTER TABLE %s RENAME COLUMN %s TO %s', reloid::text, has_mercgeom_name, const.mercgeomcol);
     PERFORM _CDB_SQL(sql,'_CDB_Has_Usable_Geom');
   END IF;
@@ -1425,7 +1425,7 @@ BEGIN
   END IF;
 
   -- Drop triggers first
-  -- PERFORM _CDB_drop_triggers(reloid);
+  PERFORM _CDB_drop_triggers(reloid);
   
   -- 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

From 0899c64d0befa079ba2734c1df753ade849173e5 Mon Sep 17 00:00:00 2001
From: Paul Ramsey <pramsey@cleverelephant.ca>
Date: Wed, 22 Apr 2015 13:25:11 -0700
Subject: [PATCH 09/34] Break routine into two halves

---
 scripts-available/CDB_CartodbfyTable.sql | 114 +++++++++++------------
 1 file changed, 52 insertions(+), 62 deletions(-)

diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql
index 06a32a6..1ed25c6 100644
--- a/scripts-available/CDB_CartodbfyTable.sql
+++ b/scripts-available/CDB_CartodbfyTable.sql
@@ -1064,7 +1064,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, has_usable_primary_key BOOLEAN, has_usable_geoms BOOLEAN)
+CREATE OR REPLACE FUNCTION _CDB_Rewrite_Table(reloid REGCLASS, destschema TEXT DEFAULT NULL)
 RETURNS BOOLEAN
 AS $$
 DECLARE
@@ -1090,6 +1090,9 @@ DECLARE
   str TEXT;
   table_srid INTEGER;
   
+  has_usable_primary_key BOOLEAN;
+  has_usable_geoms BOOLEAN;
+  
 BEGIN
 
   RAISE DEBUG 'CDB(_CDB_Rewrite_Table): %', 'entered function';
@@ -1097,17 +1100,56 @@ BEGIN
   -- Read CartoDB standard column names in
   const := _CDB_Columns();
 
-  -- No-op if there is no rewrite to be done
-  IF has_usable_primary_key AND has_usable_geoms THEN
-    RETURN true;
-  END IF;
-
   -- 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 
   WHERE c.oid = reloid;
+
+  -- Default the destination to current schema if unspecified
+  IF destschema IS NULL THEN
+    destschema := relschema;
+  END IF;
+
+  -- 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) AS has_usable_primary_key
+  INTO STRICT has_usable_primary_key;
+
+  RAISE DEBUG 'CDB(_CDB_Rewrite_Table): has_usable_primary_key %', has_usable_primary_key;
+
+  -- See if the geometry columns we need are already available
+  -- on the table. If they are, we don't need to do any bulk
+  -- transformation of the table, we can just ensure proper
+  -- indexes are in place and apply a rename
+  SELECT _CDB_Has_Usable_Geom(reloid) AS has_usable_geoms
+  INTO STRICT has_usable_geoms;
+
+  RAISE DEBUG 'CDB(_CDB_Rewrite_Table): has_usable_geoms %', has_usable_geoms;
+
+  -- We can only avoid a rewrite if both the key and 
+  -- geometry are usable
+
+  -- 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_geoms AND destschema != relschema THEN
   
+    RAISE DEBUG 'CDB(_CDB_Rewrite_Table): perfect table needs to be moved to schema (%)', destschema;
+    PERFORM _CDB_SQL(Format('ALTER TABLE %s SET SCHEMA %s', reloid::text, destschema), '_CDB_Rewrite_Table');
+    RETURN true;
+
+  -- Don't move anything, just make sure our destination information is set right
+  ELSIF has_usable_primary_key AND has_usable_geoms AND destschema = relschema THEN
+
+    RAISE DEBUG 'CDB(_CDB_Rewrite_Table): perfect table in the perfect place';
+    RETURN true;
+
+  END IF;
+
+  -- We must rewrite, so here we go...
+
+
   -- Put the primary key sequence in the right schema
   -- If the new table is not moving, better ensure the sequence name
   -- is unique
@@ -1396,11 +1438,6 @@ DECLARE
   destoid REGCLASS;
   destname TEXT;
 
-  has_usable_primary_key BOOLEAN;
-  has_usable_geoms BOOLEAN;
-  rewrite_success BOOLEAN;
-  rewrite BOOLEAN;
-  index_success BOOLEAN;
   rec RECORD;
   
 BEGIN
@@ -1414,6 +1451,7 @@ BEGIN
   -- Check destination schema exists
   -- Throws an exception of there is no matching schema
   IF destschema IS NOT NULL THEN
+  
     SELECT n.nspname
     INTO rec FROM pg_namespace n WHERE n.nspname = destschema;
     IF NOT FOUND THEN
@@ -1426,67 +1464,19 @@ BEGIN
 
   -- Drop triggers first
   PERFORM _CDB_drop_triggers(reloid);
-  
-  -- 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) AS has_usable_primary_key
-  INTO STRICT has_usable_primary_key;
-
-  RAISE DEBUG 'CDB(CDB_CartodbfyTable2): has_usable_primary_key %', has_usable_primary_key;
-
-  -- See if the geometry columns we need are already available
-  -- on the table. If they are, we don't need to do any bulk
-  -- transformation of the table, we can just ensure proper
-  -- indexes are in place and apply a rename
-  SELECT _CDB_Has_Usable_Geom(reloid) AS has_usable_geoms
-  INTO STRICT has_usable_geoms;
-
-  RAISE DEBUG 'CDB(CDB_CartodbfyTable2): has_usable_geoms %', has_usable_geoms;
-  
-  -- We can only avoid a rewrite if both the key and 
-  -- geometry are usable
-  rewrite := NOT (has_usable_primary_key AND has_usable_geoms);
-  
-  -- No table re-write is required, BUT a rename is required to 
-  -- a destination schema, so do that now
-  IF NOT rewrite AND destschema != relschema THEN
     
-    RAISE DEBUG 'CDB(CDB_CartodbfyTable2): perfect table needs to be moved to schema (%)', destschema;
-    EXECUTE Format('ALTER TABLE %s SET SCHEMA %s', reloid::text, destschema);
-
-  -- Don't move anything, just make sure our destination information is set right
-  ELSIF NOT rewrite AND destschema = relschema THEN
-
-    RAISE DEBUG 'CDB(CDB_CartodbfyTable2): perfect table in the perfect place';
-  
-  -- We must rewrite, so here we go...
-  ELSIF rewrite THEN
-
-    SELECT _CDB_Rewrite_Table(reloid, destschema, has_usable_primary_key, has_usable_geoms)
-    INTO STRICT rewrite_success;
-    
-    IF NOT rewrite_success THEN
-      PERFORM _CDB_Error('rewriting table', 'CDB_CartodbfyTable2');
-    END IF;
-      
-  END IF;
+  -- Rewrite (or rename) the table to the new location
+  PERFORM _CDB_Rewrite_Table(reloid, destschema);
 
   -- The old regclass might not be valid anymore if we re-wrote the table...
   destoid := (destschema || '.' || destname)::regclass;
 
   -- Add indexes to the destination table, as necessary
-  SELECT _CDB_Add_Indexes(destoid)
-  INTO STRICT index_success;
-
-  IF NOT index_success THEN
-    PERFORM _CDB_Error('indexing table', 'CDB_CartodbfyTable2');
-  END IF;
+  PERFORM _CDB_Add_Indexes(destoid);
   
   -- Add triggers to the destination table, as necessary
   -- PERFORM _CDB_create_triggers(destschema, reloid);
   
-  
 END;
 $$ LANGUAGE 'plpgsql';
 

From b195aa4b68eb8ca24a294115d9cc9c0ecf155a21 Mon Sep 17 00:00:00 2001
From: Paul Ramsey <pramsey@cleverelephant.ca>
Date: Mon, 10 Aug 2015 08:42:13 -0700
Subject: [PATCH 10/34] Enable trigger addition routine

---
 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 1ed25c6..a818517 100644
--- a/scripts-available/CDB_CartodbfyTable.sql
+++ b/scripts-available/CDB_CartodbfyTable.sql
@@ -1475,7 +1475,7 @@ BEGIN
   PERFORM _CDB_Add_Indexes(destoid);
   
   -- Add triggers to the destination table, as necessary
-  -- PERFORM _CDB_create_triggers(destschema, reloid);
+  PERFORM _CDB_create_triggers(destschema, destoid);
   
 END;
 $$ LANGUAGE 'plpgsql';

From 1a1f45cdadd181b9c770253e86efae24f8c57eb4 Mon Sep 17 00:00:00 2001
From: Paul Ramsey <pramsey@cleverelephant.ca>
Date: Tue, 11 Aug 2015 05:40:31 -0700
Subject: [PATCH 11/34] Add raster table calls, like the old function has

---
 scripts-available/CDB_CartodbfyTable.sql | 29 +++++++++++++++++-------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql
index a818517..9f0a446 100644
--- a/scripts-available/CDB_CartodbfyTable.sql
+++ b/scripts-available/CDB_CartodbfyTable.sql
@@ -1432,6 +1432,7 @@ RETURNS void
 AS $$
 DECLARE
   
+  is_raster BOOLEAN;
   relname TEXT;
   relschema TEXT;
 
@@ -1464,18 +1465,30 @@ BEGIN
 
   -- Drop triggers first
   PERFORM _CDB_drop_triggers(reloid);
+
+  -- Rasters only get a cartodb_id and a limited selection of triggers
+  -- underlying assumption is that they are already formed up correctly
+  SELECT cartodb._CDB_is_raster_table(destschema, reloid) INTO is_raster;
+  IF is_raster THEN
+
+    PERFORM cartodb._CDB_create_cartodb_id_column(reloid);
+    PERFORM cartodb._CDB_create_raster_triggers(destschema, reloid);
+
+  ELSE
     
-  -- Rewrite (or rename) the table to the new location
-  PERFORM _CDB_Rewrite_Table(reloid, destschema);
+    -- Rewrite (or rename) the table to the new location
+    PERFORM _CDB_Rewrite_Table(reloid, destschema);
 
-  -- The old regclass might not be valid anymore if we re-wrote the table...
-  destoid := (destschema || '.' || destname)::regclass;
+    -- The old regclass might not be valid anymore if we re-wrote the table...
+    destoid := (destschema || '.' || destname)::regclass;
 
-  -- Add indexes to the destination table, as necessary
-  PERFORM _CDB_Add_Indexes(destoid);
+    -- Add indexes to the destination table, as necessary
+    PERFORM _CDB_Add_Indexes(destoid);
   
-  -- Add triggers to the destination table, as necessary
-  PERFORM _CDB_create_triggers(destschema, destoid);
+    -- Add triggers to the destination table, as necessary
+    PERFORM _CDB_create_triggers(destschema, destoid);
+
+  END IF;
   
 END;
 $$ LANGUAGE 'plpgsql';

From d6afdf751f06b48e0785b49f696ecf5fce7d2867 Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@cartodb.com>
Date: Tue, 11 Aug 2015 19:07:01 +0200
Subject: [PATCH 12/34] Do not create timestamp columns/triggers on cartodbfy

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

diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql
index 9f0a446..bf05a92 100644
--- a/scripts-available/CDB_CartodbfyTable.sql
+++ b/scripts-available/CDB_CartodbfyTable.sql
@@ -43,10 +43,6 @@ BEGIN
   sql := 'DROP TRIGGER IF EXISTS update_the_geom_webmercator_trigger ON ' || reloid::text;
   EXECUTE sql;
 
-  -- "update_updated_at"
-  sql := 'DROP TRIGGER IF EXISTS update_updated_at_trigger ON ' || reloid::text;
-  EXECUTE sql;
-
   -- "test_quota" and "test_quota_per_row"
   sql := 'DROP TRIGGER IF EXISTS test_quota ON ' || reloid::text;
   EXECUTE sql;
@@ -205,118 +201,6 @@ END;
 $$ LANGUAGE PLPGSQL;
 
 
--- 4) created_at and updated_at creation & validation or renaming if invalid
-CREATE OR REPLACE FUNCTION _CDB_create_timestamp_columns(reloid REGCLASS)
-  RETURNS void
-AS $$
-DECLARE
-  sql TEXT;
-  rec RECORD;
-  rec2 RECORD;
-  had_column BOOLEAN;
-  i INTEGER;
-  new_name TEXT;
-BEGIN
-
-  FOR rec IN SELECT * FROM ( VALUES ('created_at'), ('updated_at') ) t(cname)
-  LOOP --{
-    new_name := null;
-    << column_setup >>
-    LOOP --{
-      had_column := FALSE;
-      BEGIN
-        sql := 'ALTER TABLE ' || reloid::text || ' ADD ' || rec.cname
-               || ' TIMESTAMPTZ NOT NULL DEFAULT now()';
-        RAISE DEBUG 'Running %', sql;
-        EXECUTE sql;
-        EXIT column_setup;
-        EXCEPTION
-        WHEN duplicate_column THEN
-          RAISE NOTICE 'Column % already exists', rec.cname;
-          had_column := TRUE;
-        WHEN others THEN
-          RAISE EXCEPTION 'Cartodbfying % (%): % (%)', reloid, rec.cname, SQLERRM, SQLSTATE;
-      END;
-
-      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 -- }{
-          -- 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;
-        << rename_column >>
-        LOOP --{
-          new_name := '_' || rec.cname || i;
-          BEGIN
-            sql := 'ALTER TABLE ' || reloid::text || ' RENAME COLUMN ' || rec.cname || ' TO ' || new_name;
-            RAISE DEBUG 'Running %', sql;
-            EXECUTE sql;
-            EXCEPTION
-            WHEN duplicate_column THEN
-              i := i+1;
-              CONTINUE rename_column;
-            WHEN others THEN
-              RAISE EXCEPTION 'Cartodbfying % (renaming %): % (%)',
-              reloid, rec.cname, SQLERRM, SQLSTATE;
-          END;
-          EXIT rename_column;
-        END LOOP; --}
-        CONTINUE column_setup;
-      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
-        -- 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;
-
-        -- 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; -- }
-
-END;
-$$ LANGUAGE PLPGSQL;
-
 
 -- 5) the_geom and the_geom_webmercator creation & validation or renaming if invalid
 CREATE OR REPLACE FUNCTION _CDB_create_the_geom_columns(reloid REGCLASS)
@@ -487,13 +371,6 @@ BEGIN
          || ' FOR EACH ROW EXECUTE PROCEDURE public._CDB_update_the_geom_webmercator()';
   EXECUTE sql;
 
--- "update_updated_at"
--- TODO: why _before_ and not after ?
-  sql := 'CREATE trigger update_updated_at_trigger BEFORE UPDATE ON '
-         || reloid::text
-         || ' FOR EACH ROW EXECUTE PROCEDURE public._CDB_update_updated_at()';
-  EXECUTE sql;
-
 -- "test_quota" and "test_quota_per_row"
 
   sql := 'CREATE TRIGGER test_quota BEFORE UPDATE OR INSERT ON '
@@ -526,13 +403,6 @@ BEGIN
          || ' FOR EACH STATEMENT EXECUTE PROCEDURE public.cdb_tablemetadata_trigger()';
   EXECUTE sql;
 
--- "update_updated_at"
--- TODO: why _before_ and not after ?
-  sql := 'CREATE trigger update_updated_at_trigger BEFORE UPDATE ON '
-         || reloid::text
-         || ' FOR EACH ROW EXECUTE PROCEDURE public._CDB_update_updated_at()';
-  EXECUTE sql;
-
 -- "test_quota" and "test_quota_per_row"
 
   sql := 'CREATE TRIGGER test_quota BEFORE UPDATE OR INSERT ON '
@@ -563,14 +433,6 @@ BEGIN
 END;
 $$ LANGUAGE plpgsql VOLATILE;
 
-CREATE OR REPLACE FUNCTION _CDB_update_updated_at()
-  RETURNS TRIGGER AS $$
-BEGIN
-   NEW.updated_at := now();
-   RETURN NEW;
-END;
-$$ LANGUAGE plpgsql VOLATILE;
-
 
 -- Auxiliary function
 CREATE OR REPLACE FUNCTION cartodb._CDB_is_raster_table(schema_name TEXT, reloid REGCLASS)
@@ -624,7 +486,6 @@ BEGIN
 
   -- Ensure required fields exist
   PERFORM cartodb._CDB_create_cartodb_id_column(reloid);
-  PERFORM cartodb._CDB_create_timestamp_columns(reloid);
 
   SELECT cartodb._CDB_is_raster_table(schema_name, reloid) INTO is_raster;
   IF is_raster THEN
@@ -1492,7 +1353,3 @@ BEGIN
   
 END;
 $$ LANGUAGE 'plpgsql';
-
-
-
-

From 67f8a8cd69836b2de6bb981ea0815b760b1a9567 Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@cartodb.com>
Date: Tue, 11 Aug 2015 19:18:01 +0200
Subject: [PATCH 13/34] Fix test_ddl_triggers.sql

by removing references to created_at & updated_at columns
---
 expected/test_ddl_triggers.out | 73 ----------------------------------
 sql/test_ddl_triggers.sql      | 40 -------------------
 2 files changed, 113 deletions(-)

diff --git a/expected/test_ddl_triggers.out b/expected/test_ddl_triggers.out
index 737468a..9dcf092 100644
--- a/expected/test_ddl_triggers.out
+++ b/expected/test_ddl_triggers.out
@@ -36,7 +36,6 @@ SET SESSION AUTHORIZATION 'cartodb_postgresql_unpriv_user';
 select 1 as i INTO c.t3;
 NOTICE:  trigger "track_updates" for table "c.t3" does not exist, skipping
 NOTICE:  trigger "update_the_geom_webmercator_trigger" for table "c.t3" does not exist, skipping
-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
@@ -45,17 +44,6 @@ 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",
- NOW() - updated_at < '1 secs' as "u<1s",
- the_geom, the_geom_webmercator,
- i
-from c.t3;
- cartodb_id | c=u | u<1s | the_geom | the_geom_webmercator | i 
-------------+-----+------+----------+----------------------+---
-          1 | t   | t    |          |                      | 1
-(1 row)
-
 RESET SESSION AUTHORIZATION;
 select
  tabname::text,
@@ -72,7 +60,6 @@ SET SESSION AUTHORIZATION 'cartodb_postgresql_unpriv_user';
 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
@@ -84,16 +71,6 @@ 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)
-
 RESET SESSION AUTHORIZATION;
 select
  tabname::text,
@@ -116,8 +93,6 @@ select pg_sleep(.1);
 
 alter table c.t3 rename column the_geom_webmercator to webmerc;
 NOTICE:  Column cartodb_id already exists
-NOTICE:  Column created_at already exists
-NOTICE:  Column updated_at already exists
 NOTICE:  Column the_geom already exists
 NOTICE:  event trigger "cdb_on_relation_create" does not exist, skipping
 NOTICE:  event trigger "cdb_on_relation_drop" does not exist, skipping
@@ -125,17 +100,6 @@ 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",
- NOW() - updated_at < '1 secs' as "u<1s",
- the_geom, the_geom_webmercator,
- i, webmerc
-from c.t3;
- cartodb_id | c=u | u<1s | the_geom | the_geom_webmercator | i | webmerc 
-------------+-----+------+----------+----------------------+---+---------
-          1 | t   | t    |          |                      | 1 | 
-(1 row)
-
 RESET SESSION AUTHORIZATION;
 select
  tabname::text,
@@ -155,8 +119,6 @@ select pg_sleep(.1);
 
 alter table c.t3 rename column the_geom_webmercator to webmerc2;
 NOTICE:  Column cartodb_id already exists
-NOTICE:  Column created_at already exists
-NOTICE:  Column updated_at already exists
 NOTICE:  Column the_geom already exists
 NOTICE:  event trigger "cdb_on_relation_create" does not exist, skipping
 NOTICE:  event trigger "cdb_on_relation_drop" does not exist, skipping
@@ -164,17 +126,6 @@ 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",
- NOW() - updated_at < '1 secs' as "u<1s",
- the_geom, the_geom_webmercator,
- i, webmerc, webmerc2
-from c.t3;
- cartodb_id | c=u | u<1s | the_geom | the_geom_webmercator | i | webmerc | webmerc2 
-------------+-----+------+----------+----------------------+---+---------+----------
-          1 | t   | t    |          |                      | 1 |         | 
-(1 row)
-
 RESET SESSION AUTHORIZATION;
 select
  tabname::text,
@@ -197,8 +148,6 @@ select pg_sleep(.1);
 
 alter table c.t3 drop column the_geom_webmercator;
 NOTICE:  Column cartodb_id already exists
-NOTICE:  Column created_at already exists
-NOTICE:  Column updated_at already exists
 NOTICE:  Column the_geom already exists
 NOTICE:  event trigger "cdb_on_relation_create" does not exist, skipping
 NOTICE:  event trigger "cdb_on_relation_drop" does not exist, skipping
@@ -206,17 +155,6 @@ 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",
- NOW() - updated_at < '1 secs' as "u<1s",
- the_geom, the_geom_webmercator,
- i, webmerc, webmerc2
-from c.t3;
- cartodb_id | c=u | u<1s | the_geom | the_geom_webmercator | i | webmerc | webmerc2 
-------------+-----+------+----------+----------------------+---+---------+----------
-          1 | t   | t    |          |                      | 1 |         | 
-(1 row)
-
 RESET SESSION AUTHORIZATION;
 select
  tabname::text,
@@ -239,17 +177,6 @@ select pg_sleep(.1);
 
 alter table c.t3 add column id2 int;
 NOTICE:  cdb_invalidate_varnish(c.t3) called
-select
- cartodb_id, created_at=updated_at as "c=u",
- NOW() - updated_at < '1 secs' as "u<1s",
- the_geom, the_geom_webmercator,
- i, webmerc, webmerc2, id2
-from c.t3;
- cartodb_id | c=u | u<1s | the_geom | the_geom_webmercator | i | webmerc | webmerc2 | id2 
-------------+-----+------+----------+----------------------+---+---------+----------+-----
-          1 | t   | t    |          |                      | 1 |         |          |    
-(1 row)
-
 RESET SESSION AUTHORIZATION;
 select
  tabname::text,
diff --git a/sql/test_ddl_triggers.sql b/sql/test_ddl_triggers.sql
index df5c3a4..8d7bbaf 100644
--- a/sql/test_ddl_triggers.sql
+++ b/sql/test_ddl_triggers.sql
@@ -21,13 +21,6 @@ SET SESSION AUTHORIZATION 'cartodb_postgresql_unpriv_user';
 SET SESSION AUTHORIZATION 'cartodb_postgresql_unpriv_user';
 select 1 as i INTO c.t3;
 
-select
- cartodb_id, created_at=updated_at as "c=u",
- NOW() - updated_at < '1 secs' as "u<1s",
- the_geom, the_geom_webmercator,
- i
-from c.t3;
-
 RESET SESSION AUTHORIZATION;
 select
  tabname::text,
@@ -38,11 +31,6 @@ SET SESSION AUTHORIZATION 'cartodb_postgresql_unpriv_user';
 -- 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;
 
 RESET SESSION AUTHORIZATION;
 select
@@ -58,13 +46,6 @@ SET SESSION AUTHORIZATION 'cartodb_postgresql_unpriv_user';
 select pg_sleep(.1);
 alter table c.t3 rename column the_geom_webmercator to webmerc;
 
-select
- cartodb_id, created_at=updated_at as "c=u",
- NOW() - updated_at < '1 secs' as "u<1s",
- the_geom, the_geom_webmercator,
- i, webmerc
-from c.t3;
-
 RESET SESSION AUTHORIZATION;
 select
  tabname::text,
@@ -75,13 +56,6 @@ SET SESSION AUTHORIZATION 'cartodb_postgresql_unpriv_user';
 select pg_sleep(.1);
 alter table c.t3 rename column the_geom_webmercator to webmerc2;
 
-select
- cartodb_id, created_at=updated_at as "c=u",
- NOW() - updated_at < '1 secs' as "u<1s",
- the_geom, the_geom_webmercator,
- i, webmerc, webmerc2
-from c.t3;
-
 RESET SESSION AUTHORIZATION;
 select
  tabname::text,
@@ -95,13 +69,6 @@ SET SESSION AUTHORIZATION 'cartodb_postgresql_unpriv_user';
 select pg_sleep(.1);
 alter table c.t3 drop column the_geom_webmercator;
 
-select
- cartodb_id, created_at=updated_at as "c=u",
- NOW() - updated_at < '1 secs' as "u<1s",
- the_geom, the_geom_webmercator,
- i, webmerc, webmerc2
-from c.t3;
-
 RESET SESSION AUTHORIZATION;
 select
  tabname::text,
@@ -115,13 +82,6 @@ SET SESSION AUTHORIZATION 'cartodb_postgresql_unpriv_user';
 select pg_sleep(.1);
 alter table c.t3 add column id2 int;
 
-select
- cartodb_id, created_at=updated_at as "c=u",
- NOW() - updated_at < '1 secs' as "u<1s",
- the_geom, the_geom_webmercator,
- i, webmerc, webmerc2, id2
-from c.t3;
-
 RESET SESSION AUTHORIZATION;
 select
  tabname::text,

From c11d1bbf505aec918852262d32bcc6f1e86e1158 Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@cartodb.com>
Date: Tue, 11 Aug 2015 19:52:34 +0200
Subject: [PATCH 14/34] Fix CDB_CartodbfyTableTest

by removing references to created_at and updated_at columns
---
 test/CDB_CartodbfyTableTest.sql    | 49 +-----------------------------
 test/CDB_CartodbfyTableTest_expect | 10 ------
 2 files changed, 1 insertion(+), 58 deletions(-)

diff --git a/test/CDB_CartodbfyTableTest.sql b/test/CDB_CartodbfyTableTest.sql
index 3736e80..7926d95 100644
--- a/test/CDB_CartodbfyTableTest.sql
+++ b/test/CDB_CartodbfyTableTest.sql
@@ -34,20 +34,10 @@ BEGIN
 
   sql := 'INSERT INTO ' || tabname::text || '(the_geom) values ( CDB_LatLng(2,1) ) RETURNING cartodb_id';
   EXECUTE sql INTO STRICT id;
-  sql := 'SELECT created_at,updated_at,the_geom_webmercator FROM '
+  sql := 'SELECT the_geom_webmercator FROM '
     || tabname::text || ' WHERE cartodb_id = ' || id;
   EXECUTE sql INTO STRICT rec;
 
-  -- Check created_at and updated_at at creation time
-  lag = rec.created_at - now();
-  IF lag > '1 second' THEN
-    RAISE EXCEPTION 'created_at not defaulting to now() after insert [ valued % ago ]', lag;
-  END IF;
-  lag = rec.updated_at - now();
-  IF lag > '1 second' THEN
-    RAISE EXCEPTION 'updated_at not defaulting to now() after insert [ valued % ago ]', lag;
-  END IF;
-
   -- Check the_geom_webmercator trigger
   IF round(st_x(rec.the_geom_webmercator)) != 111319 THEN
     RAISE EXCEPTION 'the_geom_webmercator X is % (expecting 111319)', round(st_x(rec.the_geom_webmercator));
@@ -112,15 +102,6 @@ BEGIN
       RAISE EXCEPTION '% gist indices found on the_geom and the_geom_webmercator, expected 2', tmp;
   END IF;
 
-  -- Check null constraint on cartodb_id, created_at, updated_at
-  SELECT count(*) FROM pg_attribute a, pg_class c WHERE c.oid = tabname::oid
-    AND a.attrelid = c.oid AND NOT a.attisdropped AND a.attname in
-      ( 'cartodb_id', 'created_at', 'updated_at' )
-    AND NOT a.attnotnull INTO strict tmp;
-  IF tmp > 0 THEN
-      RAISE EXCEPTION 'cartodb_id or created_at or updated_at are missing not-null constraint';
-  END IF;
-
   -- Cleanup
   sql := 'DELETE FROM ' || tabname::text || ' WHERE cartodb_id = ' || id;
   EXECUTE sql;
@@ -170,18 +151,6 @@ SELECT CDB_CartodbfyTableCheck('t', 'trigger-protected the_geom');
 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
-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 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 of type text
 CREATE TABLE t AS SELECT 10::text as cartodb_id;
 SELECT CDB_CartodbfyTableCheck('t', 'text cartodb_id');
@@ -208,22 +177,6 @@ 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
-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 5a3b090..2398ee1 100644
--- a/test/CDB_CartodbfyTableTest_expect
+++ b/test/CDB_CartodbfyTableTest_expect
@@ -28,13 +28,6 @@ trigger-protected the_geom cartodbfied fine
 extent|BOX(1 1,2 2)
 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
 5
 DROP TABLE
@@ -50,8 +43,5 @@ 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
 DROP FUNCTION

From 6b9ab3d9567533c412854aba9ec6e53c12cb05cf Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@cartodb.com>
Date: Wed, 12 Aug 2015 10:26:35 +0200
Subject: [PATCH 15/34] Fix quota test

Now the cartodbfied table is a bit smaller because it does not have the
timestamp columns.
---
 test/CDB_QuotaTest_expect | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/CDB_QuotaTest_expect b/test/CDB_QuotaTest_expect
index ba04208..72397a9 100644
--- a/test/CDB_QuotaTest_expect
+++ b/test/CDB_QuotaTest_expect
@@ -9,7 +9,7 @@ INSERT 0 2048
 INSERT 0 2048
 INSERT 0 2048
 2
-ERROR:  Quota exceeded by 567.998046875KB
+ERROR:  Quota exceeded by 519.998046875KB
 0
 INSERT 0 1
 DROP TABLE

From 8c41203db6d81666e49bf6cab4e6142c1f019970 Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@cartodb.com>
Date: Wed, 12 Aug 2015 17:34:37 +0200
Subject: [PATCH 16/34] Fix for the_geom does not exist

When creating triggers, expectation is to have the columns the_geom and
the_geom_webmercator even if the source table does not have any geometry
columns. Populate it in the rewrite with NULL values and right types.
---
 scripts-available/CDB_CartodbfyTable.sql | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql
index 9f0a446..3e02d46 100644
--- a/scripts-available/CDB_CartodbfyTable.sql
+++ b/scripts-available/CDB_CartodbfyTable.sql
@@ -1202,7 +1202,8 @@ BEGIN
       -- their tables to invalidate the SQL API 
       -- cache on update/insert/delete.
       geom_column_source := '';
-
+      sql := sql || ',NULL::geometry(Geometry,4326) AS ' || const.geomcol;
+      sql := sql || ',NULL::geometry(Geometry,3857) AS ' || const.mercgeomcol;
     ELSE
 
       -- table_srid = _CDB_Geometry_SRID(reloid, rec.attname);

From 8a031f56f56c0ea01bcc5f26888021bd0d1039a3 Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@cartodb.com>
Date: Wed, 12 Aug 2015 15:58:30 +0000
Subject: [PATCH 17/34] Recover test for cartodb_id not-null constraint

---
 test/CDB_CartodbfyTableTest.sql | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/test/CDB_CartodbfyTableTest.sql b/test/CDB_CartodbfyTableTest.sql
index 7926d95..eafb79b 100644
--- a/test/CDB_CartodbfyTableTest.sql
+++ b/test/CDB_CartodbfyTableTest.sql
@@ -102,6 +102,15 @@ BEGIN
       RAISE EXCEPTION '% gist indices found on the_geom and the_geom_webmercator, expected 2', tmp;
   END IF;
 
+  -- Check null constraint on cartodb_id, created_at, updated_at
+  SELECT count(*) FROM pg_attribute a, pg_class c WHERE c.oid = tabname::oid
+    AND a.attrelid = c.oid AND NOT a.attisdropped AND a.attname in
+      ( 'cartodb_id' )
+    AND NOT a.attnotnull INTO strict tmp;
+  IF tmp > 0 THEN
+      RAISE EXCEPTION 'cartodb_id is missing not-null constraint';
+  END IF;
+
   -- Cleanup
   sql := 'DELETE FROM ' || tabname::text || ' WHERE cartodb_id = ' || id;
   EXECUTE sql;

From a5321ec7a52254b5a34bcf6132664a7470eeb772 Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@cartodb.com>
Date: Wed, 12 Aug 2015 15:06:45 +0200
Subject: [PATCH 18/34] Replace CDB_CartodbfyTable by new CartodbfyTable2

- Delete old CDB_CartodbfyTable code
- Delete auxiliary functions no longer used
- Modify the new CDB_CartodbfyTable signature to be backwards
  compatible.
---
 scripts-available/CDB_CartodbfyTable.sql | 211 +----------------------
 1 file changed, 5 insertions(+), 206 deletions(-)

diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql
index f7c7d1a..2707810 100644
--- a/scripts-available/CDB_CartodbfyTable.sql
+++ b/scripts-available/CDB_CartodbfyTable.sql
@@ -6,29 +6,8 @@
 --   * _CDB_UserQuotaInBytes() function, installed by rails
 --     (user.rebuild_quota_trigger, called by rake task cartodb:db:update_test_quota_trigger)
 
--- 1) Required checks before running cartodbfication
--- Either will pass silenty or raise an exception
-CREATE OR REPLACE FUNCTION _CDB_check_prerequisites(schema_name TEXT, reloid REGCLASS)
-RETURNS void
-AS $$
-DECLARE
-  sql TEXT;
-BEGIN
-  IF cartodb.schema_exists(schema_name) = false THEN
-    RAISE EXCEPTION 'Invalid schema name "%"', schema_name;
-  END IF;
 
-  -- TODO: Check that user quota is set ?
-  BEGIN
-    EXECUTE FORMAT('SELECT %I._CDB_UserQuotaInBytes();', schema_name::text) INTO sql;
-    EXCEPTION WHEN undefined_function THEN
-      RAISE EXCEPTION 'Please set user quota before cartodbfying tables.';
-  END;
-END;
-$$ LANGUAGE PLPGSQL;
-
-
--- 2) Drop cartodb triggers (might prevent changing columns)
+-- Drop cartodb triggers (might prevent changing columns)
 CREATE OR REPLACE FUNCTION _CDB_drop_triggers(reloid REGCLASS)
   RETURNS void
 AS $$
@@ -52,7 +31,7 @@ END;
 $$ LANGUAGE PLPGSQL;
 
 
--- 3) Cartodb_id creation & validation or renaming if invalid
+-- Cartodb_id creation & validation or renaming if invalid
 CREATE OR REPLACE FUNCTION _CDB_create_cartodb_id_column(reloid REGCLASS)
   RETURNS void
 AS $$
@@ -201,156 +180,7 @@ END;
 $$ LANGUAGE PLPGSQL;
 
 
-
--- 5) the_geom and the_geom_webmercator creation & validation or renaming if invalid
-CREATE OR REPLACE FUNCTION _CDB_create_the_geom_columns(reloid REGCLASS)
-  RETURNS BOOLEAN[]
-AS $$
-DECLARE
-  sql TEXT;
-  rec RECORD;
-  rec2 RECORD;
-  had_column BOOLEAN;
-  i INTEGER;
-  new_name TEXT;
-  exists_geom_cols BOOLEAN[];
-BEGIN
-  -- We need the_geom and the_geom_webmercator
-  FOR rec IN SELECT * FROM ( VALUES ('the_geom',4326), ('the_geom_webmercator',3857) ) t(cname,csrid) LOOP --{
-    << column_setup >> LOOP --{
-      BEGIN
-        sql := 'ALTER TABLE ' || reloid::text || ' ADD ' || rec.cname
-               || ' GEOMETRY(geometry,' || rec.csrid || ')';
-        RAISE DEBUG 'Running %', sql;
-        EXECUTE sql;
-        sql := 'CREATE INDEX ON ' || reloid::text || ' USING GIST ( ' || rec.cname || ')';
-        RAISE DEBUG 'Running %', sql;
-        EXECUTE sql;
-        exists_geom_cols := array_append(exists_geom_cols, false);
-        EXIT column_setup;
-        EXCEPTION
-        WHEN duplicate_column THEN
-          exists_geom_cols := array_append(exists_geom_cols, true);
-          RAISE NOTICE 'Column % already exists', rec.cname;
-        WHEN others THEN
-          RAISE EXCEPTION 'Cartodbfying % (%): % (%)', reloid, rec.cname, SQLERRM, SQLSTATE;
-      END;
-
-      << column_fixup >>
-      LOOP --{
-        -- Check data type is a GEOMETRY
-        SELECT t.typname, t.oid, a.attnotnull,
-          postgis_typmod_srid(a.atttypmod) as srid,
-          postgis_typmod_type(a.atttypmod) as gtype
-        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.typname NOT IN ('geometry') THEN -- {
-          RAISE NOTICE 'Existing % field is of invalid type % (need geometry), renaming', rec.cname, rec2.typname;
-          EXIT column_fixup; -- cannot fix
-        END IF; -- }
-
-        IF rec2.srid != rec.csrid THEN -- {
-          BEGIN
-            sql := 'ALTER TABLE ' || reloid::text || ' ALTER ' || rec.cname
-                   || ' TYPE geometry(' || rec2.gtype || ',' || rec.csrid || ') USING ST_Transform('
-                   || rec.cname || ',' || rec.csrid || ')';
-            RAISE DEBUG 'Running %', sql;
-            EXECUTE sql;
-            EXCEPTION
-            WHEN others THEN
-              RAISE NOTICE 'Could not enforce SRID % to column %: %, renaming', rec.csrid, rec.cname, SQLERRM;
-              EXIT column_fixup; -- cannot fix, will rename
-          END;
-        END IF; -- }
-
-        -- add gist indices if not there already
-        IF NOT EXISTS ( SELECT ir.relname
-                        FROM pg_am am, pg_class ir,
-                          pg_class c, pg_index i,
-                          pg_attribute a
-                        WHERE c.oid  = reloid AND i.indrelid = c.oid
-                              AND a.attname = rec.cname
-                              AND i.indexrelid = ir.oid AND i.indnatts = 1
-                              AND i.indkey[0] = a.attnum AND a.attrelid = c.oid
-                              AND NOT a.attisdropped AND am.oid = ir.relam
-                              AND am.amname = 'gist' )
-        THEN -- {
-          BEGIN
-            sql := 'CREATE INDEX ON ' || reloid::text || ' USING GIST ( ' || rec.cname || ')';
-            RAISE DEBUG 'Running %', sql;
-            EXECUTE sql;
-            EXCEPTION
-            WHEN others THEN
-              RAISE EXCEPTION 'Cartodbfying % (% index): % (%)', reloid, rec.cname, SQLERRM, SQLSTATE;
-          END;
-        END IF; -- }
-
-        -- if we reached this line, all went good
-        EXIT column_setup;
-      END LOOP; -- } column_fixup
-
-      -- invalid column, need rename and re-create it
-      i := 0;
-      << rename_column >>
-      LOOP --{
-        new_name := '_' || rec.cname || i;
-        BEGIN
-          sql := 'ALTER TABLE ' || reloid::text || ' RENAME COLUMN ' || rec.cname || ' TO ' || new_name;
-          RAISE DEBUG 'Running %', sql;
-          EXECUTE sql;
-          EXCEPTION
-          WHEN duplicate_column THEN
-            i := i+1;
-            CONTINUE rename_column;
-          WHEN others THEN
-            RAISE EXCEPTION 'Cartodbfying % (rename %): % (%)', reloid, rec.cname, SQLERRM, SQLSTATE;
-        END;
-        EXIT rename_column;
-      END LOOP; --}
-      CONTINUE column_setup;
-    END LOOP; -- } column_setup
-  END LOOP; -- } on expected geometry columns
-
-  RETURN exists_geom_cols;
-END;
-$$ LANGUAGE PLPGSQL;
-
-
--- 6) Initialize the_geom with values from the_geom_webmercator
--- do this only if the_geom_webmercator was found (not created) and the_geom was NOT found.
-CREATE OR REPLACE FUNCTION _CDB_populate_the_geom_from_the_geom_webmercator(reloid REGCLASS, geom_columns_exist BOOLEAN[])
-  RETURNS void
-AS $$
-DECLARE
-  sql TEXT;
-BEGIN
-  IF geom_columns_exist[2] AND NOT geom_columns_exist[1] THEN
-    sql := 'UPDATE ' || reloid::text || ' SET the_geom = ST_Transform(the_geom_webmercator, 4326) ';
-    EXECUTE sql;
-  END IF;
-END;
-$$ LANGUAGE PLPGSQL;
-
-
--- 7) Initialize the_geom_webmercator with values from the_geom
--- do this only if the_geom was found (not created) and the_geom_webmercator was NOT found.
-CREATE OR REPLACE FUNCTION _CDB_populate_the_geom_webmercator_from_the_geom(reloid REGCLASS, geom_columns_exist BOOLEAN[])
-  RETURNS void
-AS $$
-DECLARE
-  sql TEXT;
-BEGIN
-  IF geom_columns_exist[1] AND NOT geom_columns_exist[2] THEN
-    sql := 'UPDATE ' || reloid::text || ' SET the_geom_webmercator = public.CDB_TransformToWebmercator(the_geom) ';
-    EXECUTE sql;
-  END IF;
-END;
-$$ LANGUAGE PLPGSQL;
-
-
--- 8.a) Create all triggers
+-- Create all triggers
 -- NOTE: drop/create has the side-effect of re-enabling disabled triggers
 CREATE OR REPLACE FUNCTION _CDB_create_triggers(schema_name TEXT, reloid REGCLASS)
 RETURNS void
@@ -471,37 +301,6 @@ $$ LANGUAGE PLPGSQL;
 -- ////////////////////////////////////////////////////
 
 -- Ensure a table is a "cartodb" table (See https://github.com/CartoDB/cartodb/wiki/CartoDB-user-table)
--- Rails code replicates this call at User.cartodbfy()
-CREATE OR REPLACE FUNCTION CDB_CartodbfyTable(schema_name TEXT, reloid REGCLASS)
-RETURNS void 
-AS $$
-DECLARE
-  exists_geom_cols BOOLEAN[];
-  is_raster BOOLEAN;
-BEGIN
-
-  PERFORM cartodb._CDB_check_prerequisites(schema_name, reloid);
-
-  PERFORM cartodb._CDB_drop_triggers(reloid);
-
-  -- Ensure required fields exist
-  PERFORM cartodb._CDB_create_cartodb_id_column(reloid);
-
-  SELECT cartodb._CDB_is_raster_table(schema_name, reloid) INTO is_raster;
-  IF is_raster THEN
-    PERFORM cartodb._CDB_create_raster_triggers(schema_name, reloid);
-  ELSE
-    SELECT cartodb._CDB_create_the_geom_columns(reloid) INTO exists_geom_cols;
-
-    -- Both only populate if proceeds
-    PERFORM cartodb._CDB_populate_the_geom_from_the_geom_webmercator(reloid, exists_geom_cols);
-    PERFORM cartodb._CDB_populate_the_geom_webmercator_from_the_geom(reloid, exists_geom_cols);
-
-    PERFORM cartodb._CDB_create_triggers(schema_name, reloid);
-  END IF;
-
-END;
-$$ LANGUAGE PLPGSQL;
 
 CREATE OR REPLACE FUNCTION CDB_CartodbfyTable(reloid REGCLASS)
 RETURNS void
@@ -518,7 +317,7 @@ $$ LANGUAGE PLPGSQL;
 --
 -- -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
 --
---  CDB_CartodbfyTable2(reloid REGCLASS, destschema TEXT DEFAULT NULL)
+--  CDB_CartodbfyTable(destschema TEXT, reloid REGCLASS)
 --    
 --     Main function, calls the following functions, with a little
 --     logic before the table re-write to avoid re-writing if the table
@@ -1289,7 +1088,7 @@ END;
 $$ LANGUAGE 'plpgsql';
 
 
-CREATE OR REPLACE FUNCTION CDB_CartodbfyTable2(reloid REGCLASS, destschema TEXT DEFAULT NULL)
+CREATE OR REPLACE FUNCTION CDB_CartodbfyTable(destschema TEXT, reloid REGCLASS)
 RETURNS void 
 AS $$
 DECLARE

From d268cd07cb11249ebb61c9f2ff40e08a9352ed23 Mon Sep 17 00:00:00 2001
From: Paul Ramsey <pramsey@cleverelephant.ca>
Date: Thu, 13 Aug 2015 15:59:45 -0700
Subject: [PATCH 19/34] Fix many tests and handle ownership issues involved
 with Cartodbfy being invoked by schema triggers. Some issues with regclass
 interpretation in tests still remain. Some issues with slightly different
 behavior to old version remain. Some issues with error messages /
 notification messages changing a little still remain.

---
 scripts-available/CDB_CartodbfyTable.sql |  9 ++++
 scripts-available/CDB_DDLTriggers.sql    | 66 ++++++++++++++++--------
 sql/test_ddl_triggers.sql                |  1 +
 sql/test_setup.sql                       |  2 +-
 test/CDB_CartodbfyTableTest.sql          |  7 ++-
 test/CDB_QuotaTest.sql                   | 12 ++---
 test/CDB_QuotaTest_expect                |  2 +-
 7 files changed, 68 insertions(+), 31 deletions(-)

diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql
index 2707810..91b04a2 100644
--- a/scripts-available/CDB_CartodbfyTable.sql
+++ b/scripts-available/CDB_CartodbfyTable.sql
@@ -973,6 +973,15 @@ BEGIN
   sql := Format('DROP TABLE %s', reloid::text);
   PERFORM _CDB_SQL(sql, '_CDB_Rewrite_Table');
   
+  -- If the table is being created by a SECURITY DEFINER function
+  -- make sure the user is set back to the user who is connected
+  IF current_user != session_user THEN
+    sql := Format('ALTER TABLE IF EXISTS %s OWNER TO %s', copyname, session_user);
+    PERFORM _CDB_SQL(sql, '_CDB_Rewrite_Table');
+    sql := Format('ALTER SEQUENCE IF EXISTS %s OWNER TO %s', destseq, session_user);
+    PERFORM _CDB_SQL(sql, '_CDB_Rewrite_Table');
+  END IF;
+  
   -- If we used a temporary destination table
   -- we can now rename it into place
   IF destschema = relschema THEN
diff --git a/scripts-available/CDB_DDLTriggers.sql b/scripts-available/CDB_DDLTriggers.sql
index 81db820..7453e88 100644
--- a/scripts-available/CDB_DDLTriggers.sql
+++ b/scripts-available/CDB_DDLTriggers.sql
@@ -4,17 +4,23 @@ CREATE OR REPLACE FUNCTION cartodb.cdb_handle_create_table ()
 RETURNS event_trigger SECURITY DEFINER LANGUAGE plpgsql AS $$
 DECLARE
   event_info RECORD;
-  rel_namespace TEXT;
+  rel RECORD;
+  newtable REGCLASS;
 BEGIN
   event_info := schema_triggers.get_relation_create_eventinfo();
 
   -- We're only interested in real relations
   IF (event_info.new).relkind != 'r' THEN RETURN; END IF;
 
-  SELECT nspname FROM pg_namespace WHERE oid=(event_info.new).relnamespace INTO rel_namespace;
+  SELECT c.relname, c.relnamespace, c.relkind, n.nspname 
+  FROM pg_class c
+  JOIN pg_namespace n 
+  ON c.relnamespace = n.oid
+  WHERE c.oid = event_info.relation
+  INTO rel;
 
   RAISE DEBUG 'Relation % of kind % created in table % namespace % (oid %)',
-	 event_info.relation, (event_info.new).relkind, (event_info.new).relname::TEXT, rel_namespace, (event_info.new).relnamespace;
+	 event_info.relation, rel.relkind, rel.relname, rel.nspname, rel.relnamespace;
 
   -- We don't want to react to alters triggered by superuser,
   IF current_setting('is_superuser') = 'on' THEN
@@ -25,15 +31,17 @@ BEGIN
   PERFORM cartodb.cdb_disable_ddl_hooks();
 
   -- CDB_CartodbfyTable must not create tables, or infinite loop will happen
-  PERFORM cartodb.CDB_CartodbfyTable(rel_namespace, event_info.relation);
+  PERFORM cartodb.CDB_CartodbfyTable(rel.nspname, event_info.relation);
 
   PERFORM cartodb.cdb_enable_ddl_hooks();
 
   RAISE DEBUG 'Inserting into cartodb.CDB_TableMetadata';
 
+  newtable := Format('%s.%s', rel.nspname, rel.relname)::regclass;
+
   -- Add entry to CDB_TableMetadata (should CartodbfyTable do this?)
-  INSERT INTO cartodb.CDB_TableMetadata(tabname,updated_at) 
-  VALUES (event_info.relation, now());
+  INSERT INTO cartodb.CDB_TableMetadata(tabname, updated_at) 
+  VALUES (newtable, now());
 
 END; $$;
 -- }
@@ -66,14 +74,19 @@ RETURNS event_trigger SECURITY DEFINER LANGUAGE plpgsql AS $$
 DECLARE
   event_info RECORD;
   rel RECORD;
-  rel_namespace TEXT;
+  newtable REGCLASS;
 BEGIN
   event_info := schema_triggers.get_column_alter_eventinfo();
 
-  SELECT oid,* FROM pg_class WHERE oid = event_info.relation INTO rel;
+  SELECT c.relname, c.relnamespace, c.relkind, n.nspname 
+  FROM pg_class c
+  JOIN pg_namespace n 
+  ON c.relnamespace = n.oid
+  WHERE c.oid = event_info.relation
+  INTO rel;
 
   RAISE DEBUG 'Column % altered by % (superuser? %) in relation % of kind %',
-	 (event_info.old).attname, current_user, current_setting('is_superuser'), event_info.relation::regclass, rel.relkind;
+	 (event_info.old).attname, current_user, current_setting('is_superuser'), rel.relname, rel.relkind;
 
   -- We're only interested in real relations
   IF rel.relkind != 'r' THEN RETURN; END IF;
@@ -84,17 +97,17 @@ BEGIN
     RETURN;
   END IF;
 
-  SELECT nspname FROM pg_namespace WHERE oid = rel.relnamespace INTO rel_namespace;
-
   PERFORM cartodb.cdb_disable_ddl_hooks();
 
-  PERFORM cartodb.CDB_CartodbfyTable(rel_namespace, event_info.relation);
+  PERFORM cartodb.CDB_CartodbfyTable(rel.nspname, event_info.relation);
 
   PERFORM cartodb.cdb_enable_ddl_hooks();
 
+  newtable := Format('%s.%s', rel.nspname, rel.relname)::regclass;
+
   -- update CDB_TableMetadata.updated_at (should invalidate varnish)
   UPDATE cartodb.CDB_TableMetadata SET updated_at = NOW()
-  WHERE tabname = event_info.relation; 
+  WHERE tabname = newtable; 
 
 END; $$;
 -- }
@@ -106,14 +119,19 @@ RETURNS event_trigger SECURITY DEFINER LANGUAGE plpgsql AS $$
 DECLARE
   event_info RECORD;
   rel RECORD;
-  rel_namespace TEXT;
+  newtable REGCLASS;
 BEGIN
   event_info := schema_triggers.get_column_drop_eventinfo();
 
-  SELECT oid,* FROM pg_class WHERE oid = event_info.relation INTO rel;
+  SELECT c.relname, c.relnamespace, c.relkind, n.nspname 
+  FROM pg_class c
+  JOIN pg_namespace n 
+  ON c.relnamespace = n.oid
+  WHERE c.oid = event_info.relation
+  INTO rel;
 
   RAISE DEBUG 'Column % drop by % (superuser? %) in relation % of kind %',
-	 (event_info.old).attname, current_user, current_setting('is_superuser'), event_info.relation::regclass, rel.relkind;
+	 (event_info.old).attname, current_user, current_setting('is_superuser'), rel.relname, rel.relkind;
 
   -- We're only interested in real relations
   IF rel.relkind != 'r' THEN RETURN; END IF;
@@ -124,17 +142,18 @@ BEGIN
     RETURN;
   END IF;
 
-  SELECT nspname FROM pg_namespace WHERE oid = rel.relnamespace INTO rel_namespace;
 
   PERFORM cartodb.cdb_disable_ddl_hooks();
 
-  PERFORM cartodb.CDB_CartodbfyTable(rel_namespace, event_info.relation);
+  PERFORM cartodb.CDB_CartodbfyTable(rel.nspname, event_info.relation);
 
   PERFORM cartodb.cdb_enable_ddl_hooks();
 
+  newtable := Format('%s.%s', rel.nspname, rel.relname)::regclass;
+
   -- update CDB_TableMetadata.updated_at (should invalidate varnish)
   UPDATE cartodb.CDB_TableMetadata SET updated_at = NOW()
-  WHERE tabname = event_info.relation; 
+  WHERE tabname = newtable;
 
 END; $$;
 -- }
@@ -149,10 +168,15 @@ DECLARE
 BEGIN
   event_info := schema_triggers.get_column_add_eventinfo();
 
-  SELECT oid,* FROM pg_class WHERE oid = event_info.relation INTO rel;
+  SELECT c.relname, c.relnamespace, c.relkind, n.nspname 
+  FROM pg_class c
+  JOIN pg_namespace n 
+  ON c.relnamespace = n.oid
+  WHERE c.oid = event_info.relation
+  INTO rel;
 
   RAISE DEBUG 'Column % added by % (superuser? %) in relation % of kind %',
-	 (event_info.new).attname, current_user, current_setting('is_superuser'), event_info.relation::regclass, rel.relkind;
+	 (event_info.new).attname, current_user, current_setting('is_superuser'), rel.relname, rel.relkind;
 
   -- We're only interested in real relations
   IF rel.relkind != 'r' THEN RETURN; END IF;
diff --git a/sql/test_ddl_triggers.sql b/sql/test_ddl_triggers.sql
index 8d7bbaf..f4b83dc 100644
--- a/sql/test_ddl_triggers.sql
+++ b/sql/test_ddl_triggers.sql
@@ -10,6 +10,7 @@ create schema c;
 
 SELECT CDB_SetUserQuotaInBytes('c', 0);
 
+DROP USER IF EXISTS cartodb_postgresql_unpriv_user;
 CREATE USER cartodb_postgresql_unpriv_user;
 GRANT ALL ON SCHEMA c to cartodb_postgresql_unpriv_user;
 SET SESSION AUTHORIZATION 'cartodb_postgresql_unpriv_user';
diff --git a/sql/test_setup.sql b/sql/test_setup.sql
index c05769f..67fe684 100644
--- a/sql/test_setup.sql
+++ b/sql/test_setup.sql
@@ -7,4 +7,4 @@ RETURNS void AS $$
 BEGIN
   RAISE NOTICE 'cdb_invalidate_varnish(%) called', table_name;
 END;
-$$ LANGUAGE 'plpgsql';
+$$ LANGUAGE 'plpgsql';
\ No newline at end of file
diff --git a/test/CDB_CartodbfyTableTest.sql b/test/CDB_CartodbfyTableTest.sql
index eafb79b..0b6bcab 100644
--- a/test/CDB_CartodbfyTableTest.sql
+++ b/test/CDB_CartodbfyTableTest.sql
@@ -12,6 +12,7 @@ DECLARE
   tmp INTEGER;
   ogc_geom geometry_columns; -- old the_geom record in geometry_columns
   ogc_merc geometry_columns; -- old the_geom_webmercator record in geometry_columns
+  tabtext TEXT;
 BEGIN
 
   -- Save current constraints on geometry columns, if any
@@ -30,7 +31,10 @@ BEGIN
     END IF;
   END LOOP;
 
+  tabtext := Format('%s.%s','public',tabname);
+  RAISE NOTICE 'CARTODBFYING % !!!!', tabtext;
   PERFORM CDB_CartodbfyTable('public', tabname);
+  tabname := tabtext::regclass;
 
   sql := 'INSERT INTO ' || tabname::text || '(the_geom) values ( CDB_LatLng(2,1) ) RETURNING cartodb_id';
   EXECUTE sql INTO STRICT id;
@@ -121,9 +125,8 @@ $$
 LANGUAGE 'plpgsql';
 
 -- table with single non-geometrical column
-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
+CREATE TABLE t AS SELECT 1::int as a;
 SELECT CDB_CartodbfyTableCheck('t', 'single non-geometrical column');
 DROP TABLE t;
 
diff --git a/test/CDB_QuotaTest.sql b/test/CDB_QuotaTest.sql
index e54a74d..6a253af 100644
--- a/test/CDB_QuotaTest.sql
+++ b/test/CDB_QuotaTest.sql
@@ -7,16 +7,16 @@ CREATE TABLE big(a int);
 CREATE TRIGGER test_quota BEFORE UPDATE OR INSERT ON big
       EXECUTE PROCEDURE CDB_CheckQuota(1, 1, 'public');
 INSERT INTO big VALUES (1); -- allowed, check runs before
-INSERT INTO big VALUES (1); -- disallowed, quota exceeds before
+INSERT INTO big VALUES (2); -- disallowed, quota exceeds before
 SELECT CDB_SetUserQuotaInBytes(0);
 SELECT CDB_CartodbfyTable('big');
-INSERT INTO big SELECT generate_series(1,2048);
-INSERT INTO big SELECT generate_series(1,2048);
-INSERT INTO big SELECT generate_series(1,2048);
+INSERT INTO big SELECT generate_series(2049,4096);
+INSERT INTO big SELECT generate_series(4097,6144);
+INSERT INTO big SELECT generate_series(6145,8192);
 SELECT CDB_SetUserQuotaInBytes(2);
-INSERT INTO big VALUES (1);
+INSERT INTO big VALUES (8193);
 SELECT CDB_SetUserQuotaInBytes(0);
-INSERT INTO big VALUES (1);
+INSERT INTO big VALUES (8194);
 DROP TABLE big;
 set client_min_messages to NOTICE;
 DROP FUNCTION _CDB_UserQuotaInBytes();
diff --git a/test/CDB_QuotaTest_expect b/test/CDB_QuotaTest_expect
index 72397a9..7249c63 100644
--- a/test/CDB_QuotaTest_expect
+++ b/test/CDB_QuotaTest_expect
@@ -9,7 +9,7 @@ INSERT 0 2048
 INSERT 0 2048
 INSERT 0 2048
 2
-ERROR:  Quota exceeded by 519.998046875KB
+ERROR:  Quota exceeded by 443.998046875KB
 0
 INSERT 0 1
 DROP TABLE

From 7f55a0263b53408de797fc2b3854400ce24de746 Mon Sep 17 00:00:00 2001
From: Paul Ramsey <pramsey@cleverelephant.ca>
Date: Thu, 13 Aug 2015 16:10:23 -0700
Subject: [PATCH 20/34] Fix regclass mismatch on column alter/drop This logic
 SHOULD BE MOVED TO Cartodbfy internals.

---
 scripts-available/CDB_DDLTriggers.sql | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/scripts-available/CDB_DDLTriggers.sql b/scripts-available/CDB_DDLTriggers.sql
index 7453e88..becec3d 100644
--- a/scripts-available/CDB_DDLTriggers.sql
+++ b/scripts-available/CDB_DDLTriggers.sql
@@ -106,8 +106,8 @@ BEGIN
   newtable := Format('%s.%s', rel.nspname, rel.relname)::regclass;
 
   -- update CDB_TableMetadata.updated_at (should invalidate varnish)
-  UPDATE cartodb.CDB_TableMetadata SET updated_at = NOW()
-  WHERE tabname = newtable; 
+  UPDATE cartodb.CDB_TableMetadata SET updated_at = NOW(), tabname = newtable
+  WHERE tabname = event_info.relation; 
 
 END; $$;
 -- }
@@ -152,8 +152,8 @@ BEGIN
   newtable := Format('%s.%s', rel.nspname, rel.relname)::regclass;
 
   -- update CDB_TableMetadata.updated_at (should invalidate varnish)
-  UPDATE cartodb.CDB_TableMetadata SET updated_at = NOW()
-  WHERE tabname = newtable;
+  UPDATE cartodb.CDB_TableMetadata SET updated_at = NOW(), tabname = newtable
+  WHERE tabname = event_info.relation;
 
 END; $$;
 -- }

From f211669e9e3e471a1076962d05bca45dce677e3d Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@cartodb.com>
Date: Fri, 14 Aug 2015 10:57:03 +0200
Subject: [PATCH 21/34] Tweak expected output of test_ddl_triggers

Just touch expected output to adapt to NOTICEs and other stuff that
don't affect functionality.
---
 expected/test_ddl_triggers.out | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/expected/test_ddl_triggers.out b/expected/test_ddl_triggers.out
index 9dcf092..8e9a8ae 100644
--- a/expected/test_ddl_triggers.out
+++ b/expected/test_ddl_triggers.out
@@ -25,6 +25,8 @@ SELECT CDB_SetUserQuotaInBytes('c', 0);
                        0
 (1 row)
 
+DROP USER IF EXISTS cartodb_postgresql_unpriv_user;
+NOTICE:  role "cartodb_postgresql_unpriv_user" does not exist, skipping
 CREATE USER cartodb_postgresql_unpriv_user;
 GRANT ALL ON SCHEMA c to cartodb_postgresql_unpriv_user;
 SET SESSION AUTHORIZATION 'cartodb_postgresql_unpriv_user';
@@ -62,9 +64,6 @@ 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 "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 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
@@ -92,8 +91,6 @@ select pg_sleep(.1);
 (1 row)
 
 alter table c.t3 rename column the_geom_webmercator to webmerc;
-NOTICE:  Column cartodb_id already exists
-NOTICE:  Column the_geom 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
@@ -118,8 +115,6 @@ select pg_sleep(.1);
 (1 row)
 
 alter table c.t3 rename column the_geom_webmercator to webmerc2;
-NOTICE:  Column cartodb_id already exists
-NOTICE:  Column the_geom 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
@@ -147,8 +142,6 @@ select pg_sleep(.1);
 (1 row)
 
 alter table c.t3 drop column the_geom_webmercator;
-NOTICE:  Column cartodb_id already exists
-NOTICE:  Column the_geom 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

From 3d89d8231f35aa679469223b6afa36de8db32772 Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@cartodb.com>
Date: Fri, 14 Aug 2015 09:48:30 +0000
Subject: [PATCH 22/34] Fix deletion of cartodb_postgresql_unpriv_user

only used in tests
---
 expected/test_ddl_triggers.out | 3 ++-
 sql/test_ddl_triggers.sql      | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/expected/test_ddl_triggers.out b/expected/test_ddl_triggers.out
index 8e9a8ae..c8786c9 100644
--- a/expected/test_ddl_triggers.out
+++ b/expected/test_ddl_triggers.out
@@ -192,5 +192,6 @@ select count(*) from CDB_TableMetadata;
      0
 (1 row)
 
-DROP USER cartodb_postgresql_unpriv_user;
+DROP OWNED BY cartodb_postgresql_unpriv_user;
+DROP ROLE cartodb_postgresql_unpriv_user;
 DROP FUNCTION _CDB_UserQuotaInBytes();
diff --git a/sql/test_ddl_triggers.sql b/sql/test_ddl_triggers.sql
index f4b83dc..6ed0d41 100644
--- a/sql/test_ddl_triggers.sql
+++ b/sql/test_ddl_triggers.sql
@@ -97,5 +97,6 @@ RESET SESSION AUTHORIZATION;
 drop schema c cascade;
 select count(*) from CDB_TableMetadata;
 
-DROP USER cartodb_postgresql_unpriv_user;
+DROP OWNED BY cartodb_postgresql_unpriv_user;
+DROP ROLE cartodb_postgresql_unpriv_user;
 DROP FUNCTION _CDB_UserQuotaInBytes();

From 72ebc398f8ec91913178f2829a7c8f6d29044abf Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@cartodb.com>
Date: Fri, 14 Aug 2015 12:22:59 +0200
Subject: [PATCH 23/34] Recover _CDB_check_prerequisites (sorry, my fault)

---
 scripts-available/CDB_CartodbfyTable.sql | 22 ++++++++++++++++++++++
 test/CDB_CartodbfyTableTest.sql          |  3 ++-
 test/CDB_CartodbfyTableTest_expect       |  4 ++--
 3 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql
index 91b04a2..d3c03e1 100644
--- a/scripts-available/CDB_CartodbfyTable.sql
+++ b/scripts-available/CDB_CartodbfyTable.sql
@@ -6,6 +6,26 @@
 --   * _CDB_UserQuotaInBytes() function, installed by rails
 --     (user.rebuild_quota_trigger, called by rake task cartodb:db:update_test_quota_trigger)
 
+-- 1) Required checks before running cartodbfication
+-- Either will pass silenty or raise an exception
+CREATE OR REPLACE FUNCTION _CDB_check_prerequisites(schema_name TEXT, reloid REGCLASS)
+RETURNS void
+AS $$
+DECLARE
+  sql TEXT;
+BEGIN
+  IF cartodb.schema_exists(schema_name) = false THEN
+    RAISE EXCEPTION 'Invalid schema name "%"', schema_name;
+  END IF;
+
+  -- TODO: Check that user quota is set ?
+  BEGIN
+    EXECUTE FORMAT('SELECT %I._CDB_UserQuotaInBytes();', schema_name::text) INTO sql;
+    EXCEPTION WHEN undefined_function THEN
+      RAISE EXCEPTION 'Please set user quota before cartodbfying tables.';
+  END;
+END;
+$$ LANGUAGE PLPGSQL;
 
 -- Drop cartodb triggers (might prevent changing columns)
 CREATE OR REPLACE FUNCTION _CDB_drop_triggers(reloid REGCLASS)
@@ -1119,6 +1139,8 @@ BEGIN
   FROM pg_class c JOIN pg_namespace n ON c.relnamespace = n.oid 
   WHERE c.oid = reloid;
 
+  PERFORM cartodb._CDB_check_prerequisites(destschema, reloid);
+
   -- Check destination schema exists
   -- Throws an exception of there is no matching schema
   IF destschema IS NOT NULL THEN
diff --git a/test/CDB_CartodbfyTableTest.sql b/test/CDB_CartodbfyTableTest.sql
index 0b6bcab..93c1871 100644
--- a/test/CDB_CartodbfyTableTest.sql
+++ b/test/CDB_CartodbfyTableTest.sql
@@ -125,8 +125,9 @@ $$
 LANGUAGE 'plpgsql';
 
 -- table with single non-geometrical column
-SELECT CDB_SetUserQuotaInBytes(0); -- Set user quota to infinite
 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;
 
diff --git a/test/CDB_CartodbfyTableTest_expect b/test/CDB_CartodbfyTableTest_expect
index 2398ee1..f413f97 100644
--- a/test/CDB_CartodbfyTableTest_expect
+++ b/test/CDB_CartodbfyTableTest_expect
@@ -2,8 +2,8 @@ SET
 CREATE FUNCTION
 SELECT 1
 ERROR:  Please set user quota before cartodbfying tables.
-CONTEXT:  SQL statement "SELECT cartodb._CDB_check_prerequisites(schema_name, reloid)"
-PL/pgSQL function cdb_cartodbfytable(text,regclass) line 7 at PERFORM
+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

From 010dd13e4dd6f526b409473b47828107f6fc35b5 Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@cartodb.com>
Date: Fri, 14 Aug 2015 13:40:10 +0200
Subject: [PATCH 24/34] Simple fix for type cheking in test

---
 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 93c1871..e45cbb6 100644
--- a/test/CDB_CartodbfyTableTest.sql
+++ b/test/CDB_CartodbfyTableTest.sql
@@ -78,7 +78,7 @@ BEGIN
         rec.f_geometry_column, rec.srid, rec.expsrid;
     END IF;
     -- Check TYPE constraint didn't change
-    IF rec.type != rec.exptype THEN
+    IF (rec.type != 'GEOMETRY') AND (rec.type != 'POINT') THEN
       RAISE EXCEPTION 'type of % in geometry_columns is %, expected %',
         rec.f_geometry_column, rec.type, rec.exptype;
     END IF;

From 900531f0c14ac5e6dcbb237bd374ddf06d52d01f Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@cartodb.com>
Date: Fri, 14 Aug 2015 13:45:57 +0200
Subject: [PATCH 25/34] Disable a couple of tests

Comment out tests that check cartodb_id text columns. These are no
longer taken into consideration as candidate primary ID (candidate
columns should be numeric).
---
 test/CDB_CartodbfyTableTest.sql    | 22 ++++++++++++----------
 test/CDB_CartodbfyTableTest_expect |  8 --------
 2 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/test/CDB_CartodbfyTableTest.sql b/test/CDB_CartodbfyTableTest.sql
index e45cbb6..3204446 100644
--- a/test/CDB_CartodbfyTableTest.sql
+++ b/test/CDB_CartodbfyTableTest.sql
@@ -164,17 +164,19 @@ SELECT CDB_CartodbfyTableCheck('t', 'trigger-protected the_geom');
 SELECT 'extent',ST_Extent(ST_SnapToGrid(the_geom,0.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
+-- 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 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;
+-- 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 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 f413f97..2839fde 100644
--- a/test/CDB_CartodbfyTableTest_expect
+++ b/test/CDB_CartodbfyTableTest_expect
@@ -28,14 +28,6 @@ trigger-protected the_geom cartodbfied fine
 extent|BOX(1 1,2 2)
 DROP TABLE
 SELECT 1
-text cartodb_id cartodbfied fine
-5
-DROP TABLE
-SELECT 1
-uncasting text cartodb_id cartodbfied fine
-1|nan
-DROP TABLE
-SELECT 1
 unsequenced cartodb_id cartodbfied fine
 1
 DROP TABLE

From b7b5be1f3febfcfc5ca5499d9fe5c329db005056 Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@cartodb.com>
Date: Fri, 14 Aug 2015 16:10:38 +0200
Subject: [PATCH 26/34] Add minor piece of doc

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

diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql
index d3c03e1..286d75a 100644
--- a/scripts-available/CDB_CartodbfyTable.sql
+++ b/scripts-available/CDB_CartodbfyTable.sql
@@ -343,6 +343,9 @@ $$ LANGUAGE PLPGSQL;
 --     logic before the table re-write to avoid re-writing if the table
 --     already has all the necessary columns in place.
 --
+-- (0) _CDB_check_prerequisites
+--     As before, this checks the prerequisites before trying to cartodbfy
+--
 -- (1) _CDB_drop_triggers
 --     As before, this drops all the metadata and geom sync triggers
 --

From 565edcb50d8563a641a96891169270def5d45559 Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@cartodb.com>
Date: Fri, 14 Aug 2015 16:53:43 +0200
Subject: [PATCH 27/34] Make cartodbfy return destoid

---
 scripts-available/CDB_CartodbfyTable.sql | 13 +++++++++----
 test/CDB_QuotaTest_expect                |  2 +-
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql
index 286d75a..afff37b 100644
--- a/scripts-available/CDB_CartodbfyTable.sql
+++ b/scripts-available/CDB_CartodbfyTable.sql
@@ -323,10 +323,10 @@ $$ LANGUAGE PLPGSQL;
 -- Ensure a table is a "cartodb" table (See https://github.com/CartoDB/cartodb/wiki/CartoDB-user-table)
 
 CREATE OR REPLACE FUNCTION CDB_CartodbfyTable(reloid REGCLASS)
-RETURNS void
+RETURNS REGCLASS
 AS $$
 BEGIN
-  PERFORM cartodb.CDB_CartodbfyTable('public', reloid);
+  RETURN cartodb.CDB_CartodbfyTable('public', reloid);
 END;
 $$ LANGUAGE PLPGSQL;
 
@@ -343,6 +343,10 @@ $$ LANGUAGE PLPGSQL;
 --     logic before the table re-write to avoid re-writing if the table
 --     already has all the necessary columns in place.
 --
+--     It returns the destoid of the table. If no rewritting is needed
+--     the return value will be equal to reloid.
+--
+--
 -- (0) _CDB_check_prerequisites
 --     As before, this checks the prerequisites before trying to cartodbfy
 --
@@ -1121,7 +1125,7 @@ $$ LANGUAGE 'plpgsql';
 
 
 CREATE OR REPLACE FUNCTION CDB_CartodbfyTable(destschema TEXT, reloid REGCLASS)
-RETURNS void 
+RETURNS REGCLASS
 AS $$
 DECLARE
   
@@ -1184,6 +1188,7 @@ BEGIN
     PERFORM _CDB_create_triggers(destschema, destoid);
 
   END IF;
-  
+
+  RETURN (destschema || '.' || destname)::regclass;
 END;
 $$ LANGUAGE 'plpgsql';
diff --git a/test/CDB_QuotaTest_expect b/test/CDB_QuotaTest_expect
index 7249c63..f0f57c2 100644
--- a/test/CDB_QuotaTest_expect
+++ b/test/CDB_QuotaTest_expect
@@ -4,7 +4,7 @@ CREATE TRIGGER
 INSERT 0 1
 ERROR:  Quota exceeded by 3.9990234375KB
 0
-
+big
 INSERT 0 2048
 INSERT 0 2048
 INSERT 0 2048

From 47d842927759a65afb5bee255307422758db1ec4 Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@cartodb.com>
Date: Fri, 14 Aug 2015 17:41:55 +0200
Subject: [PATCH 28/34] Use return value from cartodbfy

---
 scripts-available/CDB_DDLTriggers.sql | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/scripts-available/CDB_DDLTriggers.sql b/scripts-available/CDB_DDLTriggers.sql
index becec3d..dddd60e 100644
--- a/scripts-available/CDB_DDLTriggers.sql
+++ b/scripts-available/CDB_DDLTriggers.sql
@@ -31,14 +31,12 @@ BEGIN
   PERFORM cartodb.cdb_disable_ddl_hooks();
 
   -- CDB_CartodbfyTable must not create tables, or infinite loop will happen
-  PERFORM cartodb.CDB_CartodbfyTable(rel.nspname, event_info.relation);
+  newtable := cartodb.CDB_CartodbfyTable(rel.nspname, event_info.relation);
 
   PERFORM cartodb.cdb_enable_ddl_hooks();
 
   RAISE DEBUG 'Inserting into cartodb.CDB_TableMetadata';
 
-  newtable := Format('%s.%s', rel.nspname, rel.relname)::regclass;
-
   -- Add entry to CDB_TableMetadata (should CartodbfyTable do this?)
   INSERT INTO cartodb.CDB_TableMetadata(tabname, updated_at) 
   VALUES (newtable, now());
@@ -99,12 +97,10 @@ BEGIN
 
   PERFORM cartodb.cdb_disable_ddl_hooks();
 
-  PERFORM cartodb.CDB_CartodbfyTable(rel.nspname, event_info.relation);
+  newtable := cartodb.CDB_CartodbfyTable(rel.nspname, event_info.relation);
 
   PERFORM cartodb.cdb_enable_ddl_hooks();
 
-  newtable := Format('%s.%s', rel.nspname, rel.relname)::regclass;
-
   -- update CDB_TableMetadata.updated_at (should invalidate varnish)
   UPDATE cartodb.CDB_TableMetadata SET updated_at = NOW(), tabname = newtable
   WHERE tabname = event_info.relation; 
@@ -145,12 +141,10 @@ BEGIN
 
   PERFORM cartodb.cdb_disable_ddl_hooks();
 
-  PERFORM cartodb.CDB_CartodbfyTable(rel.nspname, event_info.relation);
+  newtable := cartodb.CDB_CartodbfyTable(rel.nspname, event_info.relation);
 
   PERFORM cartodb.cdb_enable_ddl_hooks();
 
-  newtable := Format('%s.%s', rel.nspname, rel.relname)::regclass;
-
   -- update CDB_TableMetadata.updated_at (should invalidate varnish)
   UPDATE cartodb.CDB_TableMetadata SET updated_at = NOW(), tabname = newtable
   WHERE tabname = event_info.relation;

From 3f588df6f64faa069fcccb83157dbae9972fdd49 Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@cartodb.com>
Date: Mon, 17 Aug 2015 15:24:27 +0200
Subject: [PATCH 29/34] Drop function in order to change return value

---
 scripts-available/CDB_CartodbfyTable.sql | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql
index afff37b..cc65076 100644
--- a/scripts-available/CDB_CartodbfyTable.sql
+++ b/scripts-available/CDB_CartodbfyTable.sql
@@ -322,6 +322,7 @@ $$ LANGUAGE PLPGSQL;
 
 -- Ensure a table is a "cartodb" table (See https://github.com/CartoDB/cartodb/wiki/CartoDB-user-table)
 
+DROP FUNCTION IF EXISTS CDB_CartodbfyTable(reloid REGCLASS);
 CREATE OR REPLACE FUNCTION CDB_CartodbfyTable(reloid REGCLASS)
 RETURNS REGCLASS
 AS $$
@@ -1123,7 +1124,7 @@ BEGIN
 END;
 $$ LANGUAGE 'plpgsql';
 
-
+DROP FUNCTION IF EXISTS CDB_CartodbfyTable(destschema TEXT, reloid REGCLASS);
 CREATE OR REPLACE FUNCTION CDB_CartodbfyTable(destschema TEXT, reloid REGCLASS)
 RETURNS REGCLASS
 AS $$

From 53754236e3d547bbddc71dcb75e225d810e63dbe Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@cartodb.com>
Date: Mon, 17 Aug 2015 15:56:47 +0200
Subject: [PATCH 30/34] Fix for quota test after merge with master

Now cartodbfyied tables take less space because of the timestamp
columns.
---
 test/CDB_QuotaTest_expect | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/CDB_QuotaTest_expect b/test/CDB_QuotaTest_expect
index 5b28c0a..de50130 100644
--- a/test/CDB_QuotaTest_expect
+++ b/test/CDB_QuotaTest_expect
@@ -8,8 +8,8 @@ big
 INSERT 0 2048
 INSERT 0 2048
 INSERT 0 2048
-581632
-1163264
+454656
+909312
 0
 2
 ERROR:  Quota exceeded by 443.998046875KB

From ed97d87a23a563ab131d7960925d37ebf730500a Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@cartodb.com>
Date: Tue, 18 Aug 2015 15:15:53 +0200
Subject: [PATCH 31/34] Go back to version 0.8.2

In order to be able to test and rollback, should be needed

See
https://github.com/CartoDB/cartodb-postgresql/blob/master/CONTRIBUTING.md#testing-changes-live
---
 Makefile | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index b0241fd..bd04084 100644
--- a/Makefile
+++ b/Makefile
@@ -1,7 +1,7 @@
 # cartodb/Makefile
 
 EXTENSION = cartodb
-EXTVERSION = 0.8.3
+EXTVERSION = 0.8.2
 
 SED = sed
 
@@ -41,7 +41,6 @@ UPGRADABLE = \
   0.7.4 \
   0.8.0 \
   0.8.1 \
-  0.8.2 \
   $(EXTVERSION)dev \
   $(EXTVERSION)next \
   $(END)

From 78bf202b172809becdee1221fea781ce572063c9 Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@cartodb.com>
Date: Tue, 18 Aug 2015 15:41:11 +0200
Subject: [PATCH 32/34] Fix for schema-with-dashes

---
 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 cc65076..7bb6850 100644
--- a/scripts-available/CDB_CartodbfyTable.sql
+++ b/scripts-available/CDB_CartodbfyTable.sql
@@ -843,7 +843,7 @@ BEGIN
   -- is unique
   destseq := relname || '_' || const.pkey || '_seq';
   destseq := _CDB_Unique_Relation_Name(destschema, destseq);
-  destseq := Format('%s.%s', destschema, destseq);
+  destseq := Format('"%s"."%s"', destschema, destseq);
   PERFORM _CDB_SQL(Format('CREATE SEQUENCE %s', destseq), '_CDB_Rewrite_Table');
 
   -- Salt a temporary table name if we are re-writing in place

From 805af3babffe0386324d6f2d31c02fa24614e8f0 Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@cartodb.com>
Date: Tue, 18 Aug 2015 17:05:30 +0200
Subject: [PATCH 33/34] Review of format strings and escaping of id's

Just found cartodbfy failed for schema-names-with-dashes. This should
fix it.
---
 scripts-available/CDB_CartodbfyTable.sql | 57 ++++++++++++------------
 1 file changed, 28 insertions(+), 29 deletions(-)

diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql
index 7bb6850..2be7918 100644
--- a/scripts-available/CDB_CartodbfyTable.sql
+++ b/scripts-available/CDB_CartodbfyTable.sql
@@ -35,17 +35,17 @@ DECLARE
   sql TEXT;
 BEGIN
   -- "track_updates"
-  sql := 'DROP TRIGGER IF EXISTS track_updates ON ' || reloid::text;
+  sql := Format('DROP TRIGGER IF EXISTS track_updates ON %s', reloid::text);
   EXECUTE sql;
 
   -- "update_the_geom_webmercator"
-  sql := 'DROP TRIGGER IF EXISTS update_the_geom_webmercator_trigger ON ' || reloid::text;
+  sql := Format('DROP TRIGGER IF EXISTS update_the_geom_webmercator_trigger ON %s', reloid::text);
   EXECUTE sql;
 
   -- "test_quota" and "test_quota_per_row"
-  sql := 'DROP TRIGGER IF EXISTS test_quota ON ' || reloid::text;
+  sql := Format('DROP TRIGGER IF EXISTS test_quota ON %s', reloid::text);
   EXECUTE sql;
-  sql := 'DROP TRIGGER IF EXISTS test_quota_per_row ON ' || reloid::text;
+  sql := Format('DROP TRIGGER IF EXISTS test_quota_per_row ON %s', reloid::text);
   EXECUTE sql;
 END;
 $$ LANGUAGE PLPGSQL;
@@ -68,7 +68,7 @@ BEGIN
   LOOP --{
     had_column := FALSE;
     BEGIN
-      sql := 'ALTER TABLE ' || reloid::text || ' ADD cartodb_id SERIAL NOT NULL UNIQUE';
+      sql := Format('ALTER TABLE %s ADD cartodb_id SERIAL NOT NULL UNIQUE', reloid::text);
       RAISE DEBUG 'Running %', sql;
       EXECUTE sql;
       cartodb_id_name := 'cartodb_id';
@@ -98,7 +98,7 @@ BEGIN
       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';
+	sql := Format('ALTER TABLE %s ALTER COLUMN cartodb_id SET NOT NULL', reloid::text);
         IF NOT EXISTS ( SELECT c.conname FROM pg_constraint c, pg_attribute a
         WHERE c.conkey = ARRAY[a.attnum] AND c.conrelid = reloid
               AND a.attrelid = reloid
@@ -127,7 +127,7 @@ BEGIN
       LOOP --{
         new_name := '_cartodb_id' || i;
         BEGIN
-          sql := 'ALTER TABLE ' || reloid::text || ' RENAME COLUMN cartodb_id TO ' || new_name;
+	  sql := Format('ALTER TABLE %s RENAME COLUMN cartodb_id TO %I', reloid::text, new_name);
           RAISE DEBUG 'Running %', sql;
           EXECUTE sql;
           EXCEPTION
@@ -151,14 +151,12 @@ BEGIN
       -- Copy existing values to new field
       -- 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';
+      sql := Format('ALTER TABLE %s ALTER cartodb_id TYPE int USING %I', reloid::text, new_name);
       RAISE DEBUG 'Running %', sql;
       EXECUTE sql;
 
       -- Find max value
-      sql := 'SELECT max(cartodb_id) FROM ' || reloid::text;
+      sql := Format('SELECT max(cartodb_id) FROM %s', reloid::text);
       RAISE DEBUG 'Running %', sql;
       EXECUTE sql INTO rec;
 
@@ -167,13 +165,12 @@ BEGIN
         AS seq INTO rec2;
 
       -- Reset sequence name
-      sql := 'ALTER SEQUENCE ' || rec2.seq::text
-             || ' RESTART WITH ' || rec.max + 1;
+      sql := Format('ALTER SEQUENCE %s RESTART WITH %d', rec2.seq::text, rec.max + 1);
       RAISE DEBUG 'Running %', sql;
       EXECUTE sql;
 
       -- Drop old column (all went fine if we got here)
-      sql := 'ALTER TABLE ' || reloid::text || ' DROP ' || new_name;
+      sql := Format('ALTER TABLE %s DROP %I', reloid::text, new_name);
       RAISE DEBUG 'Running %', sql;
       EXECUTE sql;
 
@@ -189,7 +186,7 @@ BEGIN
     RAISE EXCEPTION 'Cartodbfying % (Didnt get cartodb_id field name)', reloid;
   END IF;
   BEGIN
-    sql := 'ALTER TABLE ' || reloid::text || ' ADD PRIMARY KEY (cartodb_id)';
+    sql := Format('ALTER TABLE %s ADD PRIMARY KEY (cartodb_id)', reloid::text);
     EXECUTE sql;
     EXCEPTION
     WHEN others THEN
@@ -513,7 +510,7 @@ BEGIN
 
   RAISE DEBUG 'CDB(%): %', '_CDB_Geometry_SRID', 'entered function';
   
-  EXECUTE Format('SELECT ST_SRID(%s) AS srid FROM %s LIMIT 1', columnname, reloid::text)
+  EXECUTE Format('SELECT ST_SRID(%I) AS srid FROM %s LIMIT 1', columnname, reloid::text)
   INTO rec;
 
   IF FOUND THEN 
@@ -600,9 +597,9 @@ BEGIN
             Format('found non-unique ''%s'', renaming it', const.pkey);
 
           PERFORM _CDB_SQL(
-            Format('ALTER TABLE %s RENAME COLUMN %s TO %s', 
-              reloid::text, rec.attname, 
-              _CDB_Unique_Column_Name(reloid, const.pkey)), 
+            Format('ALTER TABLE %s RENAME COLUMN %s TO %I',
+              reloid::text, rec.attname,
+              _CDB_Unique_Column_Name(reloid, const.pkey)),
             '_CDB_Has_Usable_Primary_ID');
         
         END IF;
@@ -618,8 +615,8 @@ BEGIN
         Format('found non-integer ''%s'', renaming it', const.pkey);
 
       PERFORM _CDB_SQL(
-        Format('ALTER TABLE %s RENAME COLUMN %s TO %s', 
-                reloid::text, rec.attname, _CDB_Unique_Column_Name(reloid, const.pkey)), 
+        Format('ALTER TABLE %s RENAME COLUMN %s TO %I',
+                reloid::text, rec.attname, _CDB_Unique_Column_Name(reloid, const.pkey)),
                 '_CDB_Has_Usable_Primary_ID');
     
     END IF;
@@ -700,7 +697,7 @@ BEGIN
     -- Name collision: right name but wrong type, rename it!
     IF r1.typname != 'geometry' AND r1.attname = r1.desired_attname THEN
       str := _CDB_Unique_Column_Name(reloid, r1.attname);
-      sql := Format('ALTER TABLE %s RENAME COLUMN %s TO %s', reloid::text, r1.attname, str);
+      sql := Format('ALTER TABLE %s RENAME COLUMN %s TO %I', reloid::text, r1.attname, str);
       PERFORM _CDB_SQL(sql,'_CDB_Has_Usable_Geom');
       RAISE DEBUG 'CDB(_CDB_Has_Usable_Geom): %', 
         Format('%s is the wrong type, renamed to %s', r1.attname, str);
@@ -824,7 +821,7 @@ BEGIN
   IF has_usable_primary_key AND has_usable_geoms AND destschema != relschema THEN
   
     RAISE DEBUG 'CDB(_CDB_Rewrite_Table): perfect table needs to be moved to schema (%)', destschema;
-    PERFORM _CDB_SQL(Format('ALTER TABLE %s SET SCHEMA %s', reloid::text, destschema), '_CDB_Rewrite_Table');
+    PERFORM _CDB_SQL(Format('ALTER TABLE %s SET SCHEMA %I', reloid::text, destschema), '_CDB_Rewrite_Table');
     RETURN true;
 
   -- Don't move anything, just make sure our destination information is set right
@@ -843,18 +840,20 @@ BEGIN
   -- is unique
   destseq := relname || '_' || const.pkey || '_seq';
   destseq := _CDB_Unique_Relation_Name(destschema, destseq);
-  destseq := Format('"%s"."%s"', destschema, destseq);
+  destseq := Format('%I.%I', destschema, destseq);
   PERFORM _CDB_SQL(Format('CREATE SEQUENCE %s', destseq), '_CDB_Rewrite_Table');
 
   -- Salt a temporary table name if we are re-writing in place
+  -- Note copyname is already escaped and safe to use as identifier
   IF destschema = relschema THEN
-    copyname := destschema || '.' || destname || '_' || salt;
+    copyname := Format('%I.%I', destschema, Format('%s_%s', destname, salt));
   ELSE
-    copyname := destschema || '.' || destname;
+    --copyname := destschema || '.' || destname;
+    copyname := Format('%I.%I', destschema, destname);
   END IF;
   
   -- Start building the SQL!
-  sql := 'CREATE TABLE ' || copyname || ' AS SELECT ';
+  sql := Format('CREATE TABLE %s AS SELECT ', copyname);
 
   -- Add cartodb ID!
   IF has_usable_primary_key THEN
@@ -988,7 +987,7 @@ BEGIN
   END IF;
 
   -- Make the primary key use the sequence as its default value
-  sql := Format('ALTER TABLE %s ALTER COLUMN %I SET DEFAULT nextval(''%s'')', 
+  sql := Format('ALTER TABLE %s ALTER COLUMN %s SET DEFAULT nextval(''%s'')', 
           copyname, const.pkey, destseq);
   PERFORM _CDB_SQL(sql, '_CDB_Rewrite_Table');
 
@@ -1013,7 +1012,7 @@ BEGIN
   -- If we used a temporary destination table
   -- we can now rename it into place
   IF destschema = relschema THEN
-    sql := Format('ALTER TABLE %s RENAME TO %s', copyname, destname);
+    sql := Format('ALTER TABLE %s RENAME TO %I', copyname, destname);
     PERFORM _CDB_SQL(sql, '_CDB_Rewrite_Table');
   END IF;
 

From f71a2ac52eb011b2ad37af17174523bf173ca3ae Mon Sep 17 00:00:00 2001
From: Rafa de la Torre <rtorre@cartodb.com>
Date: Wed, 19 Aug 2015 15:08:07 +0200
Subject: [PATCH 34/34] Version updated to 0.9.0 plus release notes

---
 Makefile |  4 +++-
 NEWS.md  | 17 ++++++++++++++++-
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index bd04084..f269c4f 100644
--- a/Makefile
+++ b/Makefile
@@ -1,7 +1,7 @@
 # cartodb/Makefile
 
 EXTENSION = cartodb
-EXTVERSION = 0.8.2
+EXTVERSION = 0.9.0
 
 SED = sed
 
@@ -41,6 +41,8 @@ UPGRADABLE = \
   0.7.4 \
   0.8.0 \
   0.8.1 \
+  0.8.2 \
+  0.9.0 \
   $(EXTVERSION)dev \
   $(EXTVERSION)next \
   $(END)
diff --git a/NEWS.md b/NEWS.md
index 6d68994..dbdcd76 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -1,4 +1,19 @@
-0.8.3 (2015-mm-dd)
+0.9.0 (2015-08-19)
+------------------
+* Re-implementation of `CDB_CartodbfyTable` functions
+  - The signature of the main function changes to
+    ```
+    FUNCTION CDB_CartodbfyTable(destschema TEXT, reloid REGCLASS)
+    RETURNS REGCLASS
+    ```
+    - The `destschema` does not need to match the origin schema of `reloid`
+	- It returns the `regclass` of the cartodbfy'ed table, if it needs to be rewritten.
+  - There are many optimizations
+  - The columns `created_at` and `updated_at` will no longer be added
+* Fix for CDB_UserDataSize failing due `ERROR: relation "*" does not exist.` #110
+* Review test to validate permissions in public tables [#112](https://github.com/CartoDB/cartodb-postgresql/pull/112)
+
+0.8.3 (2015-08-14)
 ------------------
 * Fixes CDB_UserDataSize failing due `ERROR: relation "*" does not exist.` [#108](https://github.com/CartoDB/cartodb-postgresql/issues/108)