Merge pull request #146 from CartoDB/138-fix-cartodbfy-no-default-seq-value3

138 fix cartodbfy no default seq value3

scripts-available/CDB_CartodbfyTable.sql

@@ -664,6 +664,26 @@ END;
 $$ LANGUAGE 'plpgsql';
 
 
+CREATE OR REPLACE FUNCTION _CDB_Has_Usable_PK_Sequence(reloid REGCLASS)
+RETURNS BOOLEAN
+AS $$
+DECLARE
+  seq TEXT;
+  const RECORD;
+  has_sequence BOOLEAN = false;
+BEGIN
+
+  const := _CDB_Columns();
+
+  SELECT pg_get_serial_sequence(reloid::text, const.pkey)
+  INTO STRICT seq;
+  has_sequence := seq IS NOT NULL;
+
+  RETURN has_sequence;
+END;
+$$ LANGUAGE 'plpgsql';
+
+
 DROP FUNCTION IF EXISTS _CDB_Has_Usable_Geom(regclass);
 CREATE OR REPLACE FUNCTION _CDB_Has_Usable_Geom(reloid REGCLASS)
 RETURNS RECORD
@@ -850,6 +870,7 @@ DECLARE
   geom_srid INTEGER;
   
   has_usable_primary_key BOOLEAN;
+  has_usable_pk_sequence BOOLEAN;
   
 BEGIN
 
@@ -877,6 +898,14 @@ BEGIN
 
   RAISE DEBUG 'CDB(_CDB_Rewrite_Table): has_usable_primary_key %', has_usable_primary_key;
 
+  -- See if the candidate primary key column has a sequence for default
+  -- values. No usable pk implies has_usable_pk_sequence = false.
+  has_usable_pk_sequence := false;
+  IF has_usable_primary_key THEN
+    SELECT _CDB_Has_Usable_PK_Sequence(reloid)
+    INTO STRICT has_usable_pk_sequence;
+  END IF;
+
   -- 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
@@ -894,18 +923,20 @@ BEGIN
   -- 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 
+  -- 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 gc.has_usable_geoms AND destschema != relschema THEN
+  IF has_usable_primary_key AND has_usable_pk_sequence AND gc.has_usable_geoms THEN
-  
+    IF  destschema != relschema THEN
-    RAISE DEBUG 'CDB(_CDB_Rewrite_Table): perfect table needs to be moved to schema (%)', destschema;
-    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
+      RAISE DEBUG 'CDB(_CDB_Rewrite_Table): perfect table needs to be moved to schema (%)', destschema;
-  ELSIF has_usable_primary_key AND gc.has_usable_geoms AND destschema = relschema THEN
+      PERFORM _CDB_SQL(Format('ALTER TABLE %s SET SCHEMA %I', reloid::text, destschema), '_CDB_Rewrite_Table');
+
+    ELSE
+
+      RAISE DEBUG 'CDB(_CDB_Rewrite_Table): perfect table in the perfect place';
+
+    END IF;
 
-    RAISE DEBUG 'CDB(_CDB_Rewrite_Table): perfect table in the perfect place';
     RETURN true;
 
   END IF;
@@ -1104,27 +1135,24 @@ BEGIN
   -- 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',
-
+          const.pkey, copyname)
-    EXECUTE Format('SELECT max(%s) FROM %s',
+     INTO destseqmax;
-            const.pkey, copyname)
-       INTO destseqmax;
-
-    IF FOUND AND destseqmax IS NOT NULL THEN
-      PERFORM _CDB_SQL(Format('SELECT setval(''%s'', %s)', destseq, destseqmax), '_CDB_Rewrite_Table');
-    END IF;
 
+  IF FOUND AND destseqmax IS NOT NULL THEN
+    PERFORM _CDB_SQL(Format('SELECT setval(''%s'', %s)', destseq, destseqmax), '_CDB_Rewrite_Table');
   END IF;
 
-  -- Make the primary key use the sequence as its default value
+   -- Make the primary key use the sequence as its default value
-  sql := Format('ALTER TABLE %s ALTER COLUMN %s 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');
 
-  -- Make the sequence owned by the table, so when the table drops, 
+  -- 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, 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);

test/CDB_CartodbfyTableTest.sql

@@ -225,6 +225,36 @@ SELECT CDB_CartodbfyTable('original');
 DROP TABLE original_renamed;
 DROP TABLE original;
 
+-- Table always have a default seq value after cartodbfy #138
+CREATE TABLE bug_empty_table_no_seq (
+  cartodb_id integer,
+  the_geom geometry(Geometry,4326),
+  the_geom_webmercator geometry(Geometry,3857),
+  name text,
+  description text
+);
+SELECT CDB_CartodbfyTableCheck('bug_empty_table_no_seq', 'Table always have a default seq value after cartodbfy #138');
+INSERT INTO bug_empty_table_no_seq DEFAULT VALUES;
+DROP TABLE bug_empty_table_no_seq;
+
+-- Existing cartodb_id values are respected
+CREATE table existing_cartodb_id (
+  cartodb_id integer,
+  the_geom geometry(Geometry,4326),
+  the_geom_webmercator geometry(Geometry,3857),
+  name text,
+  description text
+);
+INSERT INTO existing_cartodb_id (cartodb_id, description) VALUES
+       (10, 'a'),
+       (20, 'b'),
+       (30, 'c');
+SELECT CDB_CartodbfyTableCheck('existing_cartodb_id', 'Existing cartodb_id values are respected #138');
+SELECT * from existing_cartodb_id;
+DROP TABLE existing_cartodb_id;
+
+
+
 -- TODO: table with existing custom-triggered the_geom
 
 DROP FUNCTION CDB_CartodbfyTableCheck(regclass, text);

test/CDB_CartodbfyTableTest_expect

@@ -58,5 +58,16 @@ CREATE TABLE
 original
 DROP TABLE
 DROP TABLE
+CREATE TABLE
+Table always have a default seq value after cartodbfy #138 cartodbfied fine
+INSERT 0 1
+DROP TABLE
+CREATE TABLE
+INSERT 0 3
+Existing cartodb_id values are respected #138 cartodbfied fine
+10|||a|
+20|||b|
+30|||c|
+DROP TABLE
 DROP FUNCTION
 DROP FUNCTION

test/extension/test.sh

@@ -123,7 +123,7 @@ function create_role_and_schema() {
 
 function drop_role_and_schema() {
     local ROLE=$1
-    sql "DROP SCHEMA \"${ROLE}\";"
+    sql "DROP SCHEMA \"${ROLE}\" CASCADE;"
     sql "REVOKE CONNECT ON DATABASE \"${DATABASE}\" FROM \"${ROLE}\";"
     sql "DROP ROLE \"${ROLE}\";"
 }
@@ -210,8 +210,8 @@ function tear_down() {
     sql "DROP SCHEMA cartodb CASCADE"
 
     log_info "########################### TEAR DOWN ###########################"
-    sql 'DROP SCHEMA cdb_testmember_1;'
+    sql 'DROP SCHEMA cdb_testmember_1 CASCADE;'
-    sql 'DROP SCHEMA cdb_testmember_2;'
+    sql 'DROP SCHEMA cdb_testmember_2 CASCADE;'
 
     sql "REVOKE CONNECT ON DATABASE \"${DATABASE}\" FROM cdb_testmember_1;"
     sql "REVOKE CONNECT ON DATABASE \"${DATABASE}\" FROM cdb_testmember_2;"