diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql index 9700696..eb80b78 100644 --- a/scripts-available/CDB_CartodbfyTable.sql +++ b/scripts-available/CDB_CartodbfyTable.sql @@ -470,6 +470,7 @@ $$ LANGUAGE 'plpgsql'; -- If the table has both a usable key and usable geometry -- we can no-op on the table copy and just ensure that the -- indexes and triggers are in place +DROP FUNCTION IF EXISTS _CDB_Has_Usable_Primary_ID(reloid REGCLASS); CREATE OR REPLACE FUNCTION _CDB_Has_Usable_Primary_ID(reloid REGCLASS) RETURNS BOOLEAN AS $$ @@ -489,114 +490,98 @@ BEGIN -- Do we already have a properly named column? SELECT a.attname, i.indisprimary, i.indisunique, a.attnotnull, a.atttypid INTO rec - FROM pg_class c - JOIN pg_attribute a ON a.attrelid = c.oid + FROM pg_class c + JOIN pg_attribute a ON a.attrelid = c.oid JOIN pg_type t ON a.atttypid = t.oid LEFT JOIN pg_index i ON c.oid = i.indrelid AND a.attnum = ANY(i.indkey) - WHERE c.oid = reloid + WHERE c.oid = reloid AND NOT a.attisdropped AND a.attname = const.pkey; -- Found something named right... IF FOUND THEN - - -- And it's an integer column... - IF rec.atttypid IN (20,21,23) THEN - - -- And it's a unique primary key! Done! - IF (rec.indisprimary OR rec.indisunique) AND rec.attnotnull THEN - RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', Format('found good ''%s''', const.pkey); - RETURN true; - -- Check and see if the column values are unique and not null, - -- if they are, we can use this column... + -- And it's a unique primary key! Done! + IF (rec.indisprimary OR rec.indisunique) AND rec.attnotnull THEN + RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', Format('found good ''%s''', const.pkey); + RETURN true; + + -- Check and see if the column values are unique and not null, + -- if they are, we can use this column... + ELSE + + -- Assume things are OK until proven otherwise... + useable_key := true; + BEGIN + sql := Format('ALTER TABLE %s ADD CONSTRAINT %s_pk PRIMARY KEY (%s)', reloid::text, const.pkey, const.pkey); + sql := sql || ', ' || Format('ADD CONSTRAINT %s_integer CHECK (%s::integer >=0);', const.pkey, const.pkey); + RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', sql; + EXECUTE sql; + EXCEPTION + -- Failed unique check... + WHEN unique_violation THEN + RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', Format('column %s is not unique', const.pkey); + useable_key := false; + -- Failed not null check... + WHEN not_null_violation THEN + RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', Format('column %s contains nulls', const.pkey); + useable_key := false; + -- Failed integer check... + WHEN invalid_text_representation THEN + RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', Format('invalid input syntax for integer %s', const.pkey); + useable_key := false; + -- Other fatal error + WHEN others THEN + PERFORM _CDB_Error(sql, Format('_CDB_Has_Usable_Primary_ID: %s', SQLERRM)); + END; + + -- Clean up test constraint + IF useable_key THEN + PERFORM _CDB_SQL(Format('ALTER TABLE %s DROP CONSTRAINT %s_pk', reloid::text, const.pkey)); + PERFORM _CDB_SQL(Format('ALTER TABLE %s DROP CONSTRAINT %s_integer', reloid::text, const.pkey)); + + -- Move non-valid column out of the way ELSE - -- Assume things are OK until proven otherwise... - useable_key := true; - - BEGIN - sql := Format('ALTER TABLE %s ADD CONSTRAINT %s_pk PRIMARY KEY (%s)', reloid::text, const.pkey, const.pkey); - RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', sql; - EXECUTE sql; - EXCEPTION - -- Failed unique check... - WHEN unique_violation THEN - RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', Format('column %s is not unique', const.pkey); - useable_key := false; - -- Failed not null check... - WHEN not_null_violation THEN - RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', Format('column %s contains nulls', const.pkey); - useable_key := false; - -- Other fatal error - WHEN others THEN - PERFORM _CDB_Error(sql, '_CDB_Has_Usable_Primary_ID'); - END; - - -- Clean up test constraint - IF useable_key THEN - PERFORM _CDB_SQL(Format('ALTER TABLE %s DROP CONSTRAINT %s_pk', reloid::text, const.pkey)); + RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', + Format('found non-valid ''%s''', const.pkey); - -- Move non-unique column out of the way - ELSE - - RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', - Format('found non-unique ''%s'', renaming it', const.pkey); - - PERFORM _CDB_SQL( - Format('ALTER TABLE %s RENAME COLUMN %s TO %I', - reloid::text, rec.attname, - cartodb._CDB_Unique_Column_Identifier(NULL, const.pkey, NULL, reloid)), - '_CDB_Has_Usable_Primary_ID'); - - END IF; - - return useable_key; + PERFORM _CDB_Error(sql, Format('_CDB_Has_Usable_Primary_ID: Error: invalid cartodb_id, %s', const.pkey)); END IF; - - -- It's not an integer column, we have to rename it - ELSE - - RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', - Format('found non-integer ''%s'', renaming it', const.pkey); - PERFORM _CDB_SQL( - Format('ALTER TABLE %s RENAME COLUMN %s TO %I', - reloid::text, rec.attname, cartodb._CDB_Unique_Column_Identifier(NULL, const.pkey, NULL, reloid)), - '_CDB_Has_Usable_Primary_ID'); - + RETURN useable_key; + END IF; - + -- There's no column there named pkey ELSE - -- Is there another suitable primary key already? + -- Is there another integer suitable primary key already? SELECT a.attname INTO rec FROM pg_class c - JOIN pg_attribute a ON a.attrelid = c.oid + JOIN pg_attribute a ON a.attrelid = c.oid JOIN pg_type t ON a.atttypid = t.oid LEFT JOIN pg_index i ON c.oid = i.indrelid AND a.attnum = ANY(i.indkey) WHERE c.oid = reloid AND NOT a.attisdropped AND i.indisprimary AND i.indisunique AND a.attnotnull AND a.atttypid IN (20,21,23); - + -- Yes! Ok, rename it. IF FOUND THEN PERFORM _CDB_SQL(Format('ALTER TABLE %s RENAME COLUMN %s TO %s', reloid::text, rec.attname, const.pkey),'_CDB_Has_Usable_Primary_ID'); RETURN true; ELSE - RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', + RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', Format('found no useful column for ''%s''', const.pkey); END IF; - + END IF; - + RAISE DEBUG 'CDB(_CDB_Has_Usable_Primary_ID): %', 'function complete'; -- Didn't find re-usable key, so return FALSE RETURN false; - END; $$ LANGUAGE 'plpgsql'; @@ -825,10 +810,10 @@ DECLARE str TEXT; table_srid INTEGER; geom_srid INTEGER; - + has_usable_primary_key BOOLEAN; has_usable_pk_sequence BOOLEAN; - + BEGIN RAISE DEBUG 'CDB(_CDB_Rewrite_Table): %', 'entered function'; @@ -839,7 +824,7 @@ BEGIN -- Save the raw schema/table names for later SELECT n.nspname, c.relname, c.relname INTO STRICT relschema, relname, destname - FROM pg_class c JOIN pg_namespace n ON c.relnamespace = n.oid + FROM pg_class c JOIN pg_namespace n ON c.relnamespace = n.oid WHERE c.oid = reloid; -- Default the destination to current schema if unspecified @@ -890,7 +875,7 @@ BEGIN RAISE DEBUG 'CDB(_CDB_Rewrite_Table): has_usable_geoms %', gc.has_usable_geoms; - -- We can only avoid a rewrite if both the key and + -- We can only avoid a rewrite if both the key and -- geometry are usable -- No table re-write is required, BUT a rename is required to @@ -934,7 +919,7 @@ BEGIN -- Add cartodb ID! IF has_usable_primary_key THEN - sql := sql || const.pkey; + sql := sql || const.pkey || '::bigint '; ELSE sql := sql || 'nextval(''' || destseq || ''') AS ' || const.pkey; END IF; @@ -1096,7 +1081,7 @@ BEGIN -- Run it! PERFORM _CDB_SQL(sql, '_CDB_Rewrite_Table'); - + -- Set up the primary key sequence -- If we copied the primary key from the original data, we need -- to set the sequence to the maximum value of that key diff --git a/test/CDB_CartodbfyTableTest.sql b/test/CDB_CartodbfyTableTest.sql index 08d1a17..02c9039 100644 --- a/test/CDB_CartodbfyTableTest.sql +++ b/test/CDB_CartodbfyTableTest.sql @@ -1,5 +1,5 @@ SET client_min_messages TO error; -\set VERBOSITY default +\set VERBOSITY terse CREATE OR REPLACE FUNCTION CDB_CartodbfyTableCheck(tabname regclass, label text) RETURNS text AS @@ -124,13 +124,21 @@ END; $$ LANGUAGE 'plpgsql'; --- table with single non-geometrical column +-- check cartodbfytable idempotence CREATE TABLE t AS SELECT 1::int as a; SELECT CDB_CartodbfyTable('public', 't'); -- should fail SELECT CDB_SetUserQuotaInBytes(0); -- Set user quota to infinite SELECT CDB_CartodbfyTableCheck('t', 'single non-geometrical column'); DROP TABLE t; +-- table with single non-geometrical column +CREATE TABLE t AS SELECT ST_SetSRID(ST_MakePoint(-1,-1),4326) as the_geom, 1::int as cartodb_id, 'this is a sentence' as description; +SELECT CDB_CartodbfyTableCheck('t', 'check function idempotence'); +SELECT * FROM t; +SELECT CDB_CartodbfyTableCheck('t', 'check function idempotence'); +SELECT * FROM t; +DROP TABLE t; + -- table with existing srid-unconstrained (but type-constrained) the_geom CREATE TABLE t AS SELECT ST_SetSRID(ST_MakePoint(0,0),4326)::geometry(point) as the_geom; SELECT CDB_CartodbfyTableCheck('t', 'srid-unconstrained the_geom'); @@ -164,19 +172,22 @@ SELECT CDB_CartodbfyTableCheck('t', 'trigger-protected the_geom'); SELECT 'extent',ST_Extent(ST_SnapToGrid(the_geom,0.2)) FROM t; DROP TABLE t; --- INFO: disabled because cartodbfy does not longer consider text columns for primary ID --- -- table with existing cartodb_id field of type text --- CREATE TABLE t AS SELECT 10::text as cartodb_id; --- SELECT CDB_CartodbfyTableCheck('t', 'text cartodb_id'); --- select cartodb_id/2 FROM t; --- DROP TABLE t; +-- table with existing cartodb_id field of type text +CREATE TABLE t AS SELECT 10::text as cartodb_id; +SELECT CDB_CartodbfyTableCheck('t', 'text cartodb_id'); +select cartodb_id/2 FROM t; +DROP TABLE t; --- INFO: disabled because cartodbfy does not longer consider text columns for primary ID --- -- table with existing cartodb_id field of type text not casting --- CREATE TABLE t AS SELECT 'nan' as cartodb_id; --- SELECT CDB_CartodbfyTableCheck('t', 'uncasting text cartodb_id'); --- select cartodb_id,_cartodb_id0 FROM t; --- DROP TABLE t; +-- table with existing cartodb_id field of type text not casting +CREATE TABLE t AS SELECT 'nan'::text as cartodb_id; +SELECT CDB_CartodbfyTableCheck('t', 'uncasting text cartodb_id'); +DROP TABLE t; + +-- table with empty cartodb_id field of type text +CREATE TABLE t AS SELECT null::text as cartodb_id; +SELECT CDB_CartodbfyTableCheck('t', 'empty text cartodb_id'); +SELECT cartodb_id 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; @@ -291,7 +302,7 @@ INSERT INTO test VALUES (NULL), (3); SELECT CDB_CartodbfyTableCheck('test', 'Table with null cartodb_id #148'); -SELECT cartodb_id, cartodb_id_0 from test; +SELECT cartodb_id from test; DROP TABLE test; -- Table with non unique cartodb_id @@ -303,7 +314,7 @@ INSERT INTO test VALUES (2), (2); SELECT CDB_CartodbfyTableCheck('test', 'Table with non unique cartodb_id #148'); -SELECT cartodb_id, cartodb_id_0 from test; +SELECT cartodb_id from test; DROP TABLE test; -- Table with non unique and null cartodb_id @@ -316,7 +327,7 @@ INSERT INTO test VALUES (NULL), (2); SELECT CDB_CartodbfyTableCheck('test', 'Table with non unique and null cartodb_id #148'); -SELECT cartodb_id, cartodb_id_0 from test; +SELECT cartodb_id from test; DROP TABLE test; CREATE TABLE test ( diff --git a/test/CDB_CartodbfyTableTest_expect b/test/CDB_CartodbfyTableTest_expect index 0b7419f..de2e9ae 100644 --- a/test/CDB_CartodbfyTableTest_expect +++ b/test/CDB_CartodbfyTableTest_expect @@ -2,12 +2,16 @@ SET CREATE FUNCTION SELECT 1 ERROR: Please set user quota before cartodbfying tables. -CONTEXT: SQL statement "SELECT cartodb._CDB_check_prerequisites(destschema, reloid)" -PL/pgSQL function cdb_cartodbfytable(text,regclass) line 21 at PERFORM 0 single non-geometrical column cartodbfied fine DROP TABLE SELECT 1 +check function idempotence cartodbfied fine +1|0101000020E6100000000000000000F0BF000000000000F0BF|0101000020110F0000DB0B4ADA772DFBC077432E49D22DFBC0|this is a sentence +check function idempotence cartodbfied fine +1|0101000020E6100000000000000000F0BF000000000000F0BF|0101000020110F0000DB0B4ADA772DFBC077432E49D22DFBC0|this is a sentence +DROP TABLE +SELECT 1 srid-unconstrained the_geom cartodbfied fine DROP TABLE SELECT 2 @@ -26,6 +30,17 @@ SELECT 1 CREATE TRIGGER 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 +ERROR: CDB(_CDB_Has_Usable_Primary_ID: Error: invalid cartodb_id, cartodb_id): ALTER TABLE t ADD CONSTRAINT cartodb_id_pk PRIMARY KEY (cartodb_id), ADD CONSTRAINT cartodb_id_integer CHECK (cartodb_id::integer >=0); +DROP TABLE +SELECT 1 +ERROR: CDB(_CDB_Has_Usable_Primary_ID: Error: invalid cartodb_id, cartodb_id): ALTER TABLE t ADD CONSTRAINT cartodb_id_pk PRIMARY KEY (cartodb_id), ADD CONSTRAINT cartodb_id_integer CHECK (cartodb_id::integer >=0); + DROP TABLE SELECT 1 unsequenced cartodb_id cartodbfied fine @@ -77,30 +92,30 @@ Table with both the_geom and wkb_geometry #141 cartodbfied fine DROP TABLE CREATE TABLE INSERT 0 1 -Many colliding columns #141 cartodbfied fine +ERROR: CDB(_CDB_Has_Usable_Primary_ID: multiple primary keys for table "many_colliding_columns" are not allowed): ALTER TABLE many_colliding_columns ADD CONSTRAINT cartodb_id_pk PRIMARY KEY (cartodb_id), ADD CONSTRAINT cartodb_id_integer CHECK (cartodb_id::integer >=0); DROP TABLE CREATE TABLE INSERT 0 4 -Table with null cartodb_id #148 cartodbfied fine -1|1 -2|2 -3| -4|3 +ERROR: CDB(_CDB_Has_Usable_Primary_ID: Error: invalid cartodb_id, cartodb_id): ALTER TABLE test ADD CONSTRAINT cartodb_id_pk PRIMARY KEY (cartodb_id), ADD CONSTRAINT cartodb_id_integer CHECK (cartodb_id::integer >=0); +1 +2 + +3 DROP TABLE CREATE TABLE INSERT 0 3 -Table with non unique cartodb_id #148 cartodbfied fine -1|1 -2|2 -3|2 +ERROR: CDB(_CDB_Has_Usable_Primary_ID: Error: invalid cartodb_id, cartodb_id): ALTER TABLE test ADD CONSTRAINT cartodb_id_pk PRIMARY KEY (cartodb_id), ADD CONSTRAINT cartodb_id_integer CHECK (cartodb_id::integer >=0); +1 +2 +2 DROP TABLE CREATE TABLE INSERT 0 4 -Table with non unique and null cartodb_id #148 cartodbfied fine -1|1 -2|2 -3| -4|2 +ERROR: CDB(_CDB_Has_Usable_Primary_ID: Error: invalid cartodb_id, cartodb_id): ALTER TABLE test ADD CONSTRAINT cartodb_id_pk PRIMARY KEY (cartodb_id), ADD CONSTRAINT cartodb_id_integer CHECK (cartodb_id::integer >=0); +1 +2 + +2 DROP TABLE CREATE TABLE CREATE INDEX