diff --git a/expected/test_ddl_triggers.out b/expected/test_ddl_triggers.out index c8786c9..6746e61 100644 --- a/expected/test_ddl_triggers.out +++ b/expected/test_ddl_triggers.out @@ -91,6 +91,7 @@ select pg_sleep(.1); (1 row) alter table c.t3 rename column the_geom_webmercator to webmerc; +NOTICE: column "the_geom_webmercator" of relation "t3" does not exist, skipping 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 @@ -115,6 +116,7 @@ select pg_sleep(.1); (1 row) alter table c.t3 rename column the_geom_webmercator to webmerc2; +NOTICE: column "the_geom_webmercator" of relation "t3" does not exist, skipping 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 diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql index 47d899e..ea483da 100644 --- a/scripts-available/CDB_CartodbfyTable.sql +++ b/scripts-available/CDB_CartodbfyTable.sql @@ -507,30 +507,6 @@ BEGIN END; $$ LANGUAGE 'plpgsql'; - - --- 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 $$ -DECLARE - rec RECORD; -BEGIN - - RAISE DEBUG 'CDB(%): %', '_CDB_Geometry_SRID', 'entered function'; - - EXECUTE Format('SELECT ST_SRID(%I) AS srid FROM %s LIMIT 1', columnname, reloid::text) - INTO rec; - - IF FOUND THEN - RETURN rec.srid; - END IF; - - RETURN 0; - -END; -$$ LANGUAGE 'plpgsql'; -- Find out if the table already has a usable primary key @@ -683,10 +659,48 @@ BEGIN END; $$ LANGUAGE 'plpgsql'; +-- Return a set of columns that can be candidates to be the_geom[webmercator] +-- with some extra information to analyze them. +CREATE OR REPLACE FUNCTION _cdb_geom_candidate_columns(reloid REGCLASS) +RETURNS TABLE (attname name, srid integer, typname name, desired_attname text, desired_srid integer) +AS $$ +DECLARE + const RECORD; +BEGIN + + const := _CDB_Columns(); + + RETURN QUERY + SELECT + a.attname, + CASE WHEN t.typname = 'geometry' THEN postgis_typmod_srid(a.atttypmod) ELSE NULL END AS srid, + 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, + (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 + AND postgis_typmod_srid(a.atttypmod) IN (4326, 3857, 0) + ORDER BY t.oid ASC; +END; +$$ LANGUAGE 'plpgsql'; + + +CREATE TYPE _cdb_has_usable_geom_record + AS (has_usable_geoms boolean, + text_geom_column boolean, + text_geom_column_name text, + text_geom_column_srid boolean, + has_geom boolean, + has_geom_name text, + has_mercgeom boolean, + has_mercgeom_name text); -DROP FUNCTION IF EXISTS _CDB_Has_Usable_Geom(regclass); CREATE OR REPLACE FUNCTION _CDB_Has_Usable_Geom(reloid REGCLASS) -RETURNS RECORD +RETURNS _cdb_has_usable_geom_record AS $$ DECLARE r1 RECORD; @@ -718,20 +732,7 @@ BEGIN -- Do we have a column we can use? FOR r1 IN - SELECT - a.attname, - CASE WHEN t.typname = 'geometry' THEN postgis_typmod_srid(a.atttypmod) ELSE NULL END AS srid, - 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, - (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 - AND postgis_typmod_srid(a.atttypmod) IN (4326, 3857, 0) - ORDER BY t.oid ASC + SELECT * FROM _cdb_geom_candidate_columns(reloid) LOOP RAISE DEBUG 'CDB(_CDB_Has_Usable_Geom): %', Format('checking column ''%s''', r1.attname); @@ -786,7 +787,7 @@ 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 + IF r1.srid = r1.desired_srid THEN RAISE DEBUG 'CDB(_CDB_Has_Usable_Geom): %', Format('found acceptable ''%s''', r1.attname); @@ -799,7 +800,7 @@ BEGIN END IF; -- If it's an unknown SRID, we need to know that too - ELSIF r1.srid = 0 OR _CDB_Geometry_SRID(reloid, r1.attname) = 0 THEN + ELSIF r1.srid = 0 THEN -- Unknown SRID, we'll have to fill it in later text_geom_column_srid := true; @@ -810,23 +811,13 @@ BEGIN 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.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; - SELECT -- If table is perfect (no transforms required), return TRUE! has_geom AND has_mercgeom AS has_usable_geoms, -- If the geometry column is hiding in a text field, return enough info to deal w/ it. - text_geom_column, text_geom_column_name, text_geom_column_srid + text_geom_column, text_geom_column_name, text_geom_column_srid, + -- Return enough info to rename geom columns if needed + has_geom, has_geom_name, has_mercgeom, has_mercgeom_name INTO rv; RAISE DEBUG 'CDB(_CDB_Has_Usable_Geom): %', Format('returning %s', rv); @@ -836,6 +827,7 @@ BEGIN 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 @@ -912,12 +904,25 @@ BEGIN -- indexes are in place and apply a rename SELECT * FROM _CDB_Has_Usable_Geom(reloid) - AS (has_usable_geoms boolean, - text_geom_column boolean, - text_geom_column_name text, - text_geom_column_srid boolean) INTO STRICT gc; + -- If geom is the wrong name, just rename it. + IF gc.has_geom AND gc.has_geom_name != const.geomcol THEN + sql := Format('ALTER TABLE %s DROP COLUMN IF EXISTS %I', reloid::text, const.geomcol); + PERFORM _CDB_SQL(sql,'_CDB_Rewrite_Table'); + sql := Format('ALTER TABLE %s RENAME COLUMN %I TO %I', reloid::text, gc.has_geom_name, const.geomcol); + PERFORM _CDB_SQL(sql,'_CDB_Rewrite_Table'); + END IF; + + -- If mercgeom is the wrong name, just rename it. + IF gc.has_mercgeom AND gc.has_mercgeom_name != const.mercgeomcol THEN + sql := Format('ALTER TABLE %s DROP COLUMN IF EXISTS %I', reloid::text, const.mercgeomcol); + PERFORM _CDB_SQL(sql,'_CDB_Rewrite_Table'); + sql := Format('ALTER TABLE %s RENAME COLUMN %I TO %I', reloid::text, gc.has_mercgeom_name, const.mercgeomcol); + PERFORM _CDB_SQL(sql,'_CDB_Rewrite_Table'); + END IF; + + RAISE DEBUG 'CDB(_CDB_Rewrite_Table): has_usable_geoms %', gc.has_usable_geoms; -- We can only avoid a rewrite if both the key and @@ -1070,15 +1075,11 @@ BEGIN ) SELECT ', ST_Transform(' || t.missing_srid_start || t.attname || t.missing_srid_end - || ',4326)::Geometry(' - || t.geomtype - || ',4326) AS ' + || ',4326)::Geometry(GEOMETRY,4326) AS ' || const.geomcol || ', cartodb.CDB_TransformToWebmercator(' || t.missing_srid_start || t.attname || t.missing_srid_end - || ')::Geometry(' - || t.geomtype - || ',3857) AS ' + || ')::Geometry(GEOMETRY,3857) AS ' || const.mercgeomcol, t.attname INTO geom_transform_sql, geom_column_source @@ -1139,7 +1140,7 @@ BEGIN const.pkey, copyname) INTO destseqmax; - IF FOUND AND destseqmax IS NOT NULL THEN + IF destseqmax IS NOT NULL THEN PERFORM _CDB_SQL(Format('SELECT setval(''%s'', %s)', destseq, destseqmax), '_CDB_Rewrite_Table'); END IF; diff --git a/test/CDB_CartodbfyTableTest.sql b/test/CDB_CartodbfyTableTest.sql index 405c038..b35540b 100644 --- a/test/CDB_CartodbfyTableTest.sql +++ b/test/CDB_CartodbfyTableTest.sql @@ -253,6 +253,33 @@ SELECT CDB_CartodbfyTableCheck('existing_cartodb_id', 'Existing cartodb_id value SELECT * from existing_cartodb_id; DROP TABLE existing_cartodb_id; +-- Table with both the_geom and wkb_geometry +CREATE TABLE many_geometry_columns ( + the_geom geometry, + wkb_geometry geometry(MultiPoint,4326), + description varchar +); +INSERT INTO many_geometry_columns (the_geom, wkb_geometry) VALUES + ('0104000020E61000000100000001010000007108B023698052C03CEEA53A2E5D4440', '0104000020E61000000100000001010000007108B023698052C03CEEA53A2E5D4440'), + ('0104000020E6100000010000000101000000864C9E57618052C0994F0C7F3C5B4440', '0104000020E6100000010000000101000000864C9E57618052C0994F0C7F3C5B4440'); +SELECT CDB_CartodbfyTableCheck('many_geometry_columns', 'Table with both the_geom and wkb_geometry #141'); +SELECT * FROM many_geometry_columns; +DROP TABLE many_geometry_columns; + +-- Many colliding geom columns +CREATE TABLE many_colliding_columns ( + the_geom varchar, + the_geom_webmercator varchar, + my_geom geometry, + my_mercgeom geometry(Point, 3857), + cartodb_id varchar, + my_pk integer primary key +); +INSERT INTO many_colliding_columns VALUES ( + 'foo', 'bar', 'SRID=4326;POINT(0 0)', 'SRID=3857;POINT(0 0)', 'nerf', 1 +); +SELECT CDB_CartodbfyTableCheck('many_colliding_columns', 'Many colliding columns #141'); +DROP TABLE many_colliding_columns; -- TODO: table with existing custom-triggered the_geom diff --git a/test/CDB_CartodbfyTableTest_expect b/test/CDB_CartodbfyTableTest_expect index 537c416..6287c3f 100644 --- a/test/CDB_CartodbfyTableTest_expect +++ b/test/CDB_CartodbfyTableTest_expect @@ -69,5 +69,15 @@ Existing cartodb_id values are respected #138 cartodbfied fine 20|||b| 30|||c| DROP TABLE +CREATE TABLE +INSERT 0 2 +Table with both the_geom and wkb_geometry #141 cartodbfied fine +1|0104000020E61000000100000001010000007108B023698052C03CEEA53A2E5D4440|0104000020110F00000100000001010000004A9F662B456D5FC11392690DC3F75241| +2|0104000020E6100000010000000101000000864C9E57618052C0994F0C7F3C5B4440|0104000020110F00000100000001010000002858E0EC376D5FC1CAE8DB4B95F55241| +DROP TABLE +CREATE TABLE +INSERT 0 1 +Many colliding columns #141 cartodbfied fine +DROP TABLE DROP FUNCTION DROP FUNCTION