From e546c15770ffb9781613c530d4b42e0a4da00bc3 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Fri, 11 Sep 2015 16:16:54 +0200 Subject: [PATCH 01/17] Test for failing scenario #141 --- test/CDB_CartodbfyTableTest.sql | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/CDB_CartodbfyTableTest.sql b/test/CDB_CartodbfyTableTest.sql index 405c038..09a5524 100644 --- a/test/CDB_CartodbfyTableTest.sql +++ b/test/CDB_CartodbfyTableTest.sql @@ -253,6 +253,17 @@ 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) +); +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; -- TODO: table with existing custom-triggered the_geom From 9ec24c1affa0e02c84b0419c6d8c4961add9a4e9 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Mon, 14 Sep 2015 11:18:12 +0200 Subject: [PATCH 02/17] Fix FOUND in _CDB_Geometry_SRID #141 --- scripts-available/CDB_CartodbfyTable.sql | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql index 47d899e..ea3dc1e 100644 --- a/scripts-available/CDB_CartodbfyTable.sql +++ b/scripts-available/CDB_CartodbfyTable.sql @@ -519,16 +519,16 @@ DECLARE 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 + IF rec IS NOT NULL THEN RETURN rec.srid; END IF; RETURN 0; - + END; $$ LANGUAGE 'plpgsql'; From 581835d4ff2fa0e612a6e903e9f48b7b53ccb52b Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Mon, 14 Sep 2015 11:50:10 +0200 Subject: [PATCH 03/17] Extract query into _cdb_geom_candidate_columns #141 --- scripts-available/CDB_CartodbfyTable.sql | 45 ++++++++++++++++-------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql index ea3dc1e..71c7cad 100644 --- a/scripts-available/CDB_CartodbfyTable.sql +++ b/scripts-available/CDB_CartodbfyTable.sql @@ -683,8 +683,36 @@ BEGIN END; $$ LANGUAGE 'plpgsql'; +-- Return a set of columns that can be candidates to be the_geom +-- with some extra information to filter them out. +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'; + -DROP FUNCTION IF EXISTS _CDB_Has_Usable_Geom(regclass); CREATE OR REPLACE FUNCTION _CDB_Has_Usable_Geom(reloid REGCLASS) RETURNS RECORD AS $$ @@ -718,20 +746,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); From e3bba2ee4bf6f794b25b6b2dafa768109e7cdae9 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Mon, 14 Sep 2015 16:48:57 +0200 Subject: [PATCH 04/17] Fix `the_geom` already exists error #141 --- scripts-available/CDB_CartodbfyTable.sql | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql index 71c7cad..dbea59a 100644 --- a/scripts-available/CDB_CartodbfyTable.sql +++ b/scripts-available/CDB_CartodbfyTable.sql @@ -826,8 +826,10 @@ 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); + IF has_geom AND has_geom_name != const.geomcol THEN + sql := Format('ALTER TABLE %I DROP COLUMN IF EXISTS %I', reloid::text, const.geomcol); + PERFORM _CDB_SQL(sql,'_CDB_Has_Usable_Geom'); + sql := Format('ALTER TABLE %I RENAME COLUMN %I TO %I', reloid::text, has_geom_name, const.geomcol); PERFORM _CDB_SQL(sql,'_CDB_Has_Usable_Geom'); END IF; From 731ee0a9ba390c8f7d8f67fdef8ad5d2a5c27cec Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Mon, 14 Sep 2015 17:05:59 +0200 Subject: [PATCH 05/17] Fix `the_geom_webmercator` already exists #141 --- scripts-available/CDB_CartodbfyTable.sql | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql index dbea59a..51fcff6 100644 --- a/scripts-available/CDB_CartodbfyTable.sql +++ b/scripts-available/CDB_CartodbfyTable.sql @@ -835,6 +835,8 @@ BEGIN -- If mercgeom is the wrong name, just rename it. IF has_mercgeom AND has_mercgeom_name != const.mercgeomcol THEN + sql := Format('ALTER TABLE %I DROP COLUMN IF EXISTS %I', reloid::text, const.mercgeomcol); + PERFORM _CDB_SQL(sql,'_CDB_Has_Usable_Geom'); 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; From dfd0454be39f32d37aee8bf8434f9dbba8c19caa Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Mon, 14 Sep 2015 17:53:34 +0200 Subject: [PATCH 06/17] Improve comment #141 --- 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 51fcff6..3270b8c 100644 --- a/scripts-available/CDB_CartodbfyTable.sql +++ b/scripts-available/CDB_CartodbfyTable.sql @@ -683,8 +683,8 @@ BEGIN END; $$ LANGUAGE 'plpgsql'; --- Return a set of columns that can be candidates to be the_geom --- with some extra information to filter them out. +-- 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 $$ From 1596bd56d88fcd0bdd3580a5832e617c812ad172 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Mon, 14 Sep 2015 17:54:35 +0200 Subject: [PATCH 07/17] Improve another EXECUTE+FOUND #141 --- 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 3270b8c..0304602 100644 --- a/scripts-available/CDB_CartodbfyTable.sql +++ b/scripts-available/CDB_CartodbfyTable.sql @@ -1158,7 +1158,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; From 9d8d79eb40ce7f21f2b84424b72b3df332588483 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Mon, 14 Sep 2015 17:55:30 +0200 Subject: [PATCH 08/17] Slightly improve the test #141 --- test/CDB_CartodbfyTableTest.sql | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/CDB_CartodbfyTableTest.sql b/test/CDB_CartodbfyTableTest.sql index 09a5524..b5678f4 100644 --- a/test/CDB_CartodbfyTableTest.sql +++ b/test/CDB_CartodbfyTableTest.sql @@ -256,7 +256,8 @@ 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) + wkb_geometry geometry(MultiPoint,4326), + description varchar ); INSERT INTO many_geometry_columns (the_geom, wkb_geometry) VALUES ('0104000020E61000000100000001010000007108B023698052C03CEEA53A2E5D4440', '0104000020E61000000100000001010000007108B023698052C03CEEA53A2E5D4440'), From 5caddc6cc75771b75c7a3d138e2d7d29d561b2f8 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Tue, 15 Sep 2015 10:36:56 +0200 Subject: [PATCH 09/17] Fix for MultiPoint geometry issue #141 --- scripts-available/CDB_CartodbfyTable.sql | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql index 0304602..add463a 100644 --- a/scripts-available/CDB_CartodbfyTable.sql +++ b/scripts-available/CDB_CartodbfyTable.sql @@ -1089,15 +1089,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 From 3fdce653684e334d7c4db45b250ed0badd2c1813 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Tue, 15 Sep 2015 15:22:57 +0200 Subject: [PATCH 10/17] Move column renaming out of _CDB_Has_Usable_Geom #141 --- scripts-available/CDB_CartodbfyTable.sql | 44 ++++++++++++++---------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql index add463a..6d520ae 100644 --- a/scripts-available/CDB_CartodbfyTable.sql +++ b/scripts-available/CDB_CartodbfyTable.sql @@ -825,27 +825,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 %I DROP COLUMN IF EXISTS %I', reloid::text, const.geomcol); - PERFORM _CDB_SQL(sql,'_CDB_Has_Usable_Geom'); - sql := Format('ALTER TABLE %I RENAME COLUMN %I TO %I', 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 %I DROP COLUMN IF EXISTS %I', reloid::text, const.mercgeomcol); - PERFORM _CDB_SQL(sql,'_CDB_Has_Usable_Geom'); - 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); @@ -855,6 +841,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 @@ -934,9 +921,30 @@ BEGIN AS (has_usable_geoms boolean, text_geom_column boolean, text_geom_column_name text, - text_geom_column_srid boolean) + text_geom_column_srid boolean, + has_geom boolean, + has_geom_name text, + has_mercgeom boolean, + has_mercgeom_name text) 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 %I DROP COLUMN IF EXISTS %I', reloid::text, const.geomcol); + PERFORM _CDB_SQL(sql,'_CDB_Rewrite_Table'); + sql := Format('ALTER TABLE %I 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 %I DROP COLUMN IF EXISTS %I', reloid::text, const.mercgeomcol); + PERFORM _CDB_SQL(sql,'_CDB_Rewrite_Table'); + sql := Format('ALTER TABLE %s RENAME COLUMN %s TO %s', 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 From 789e89a5d20f5a8738293cd456c38bb7a2638442 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Tue, 15 Sep 2015 15:23:26 +0200 Subject: [PATCH 11/17] Create a return type for _cdb_has_usable_geom_record #141 --- scripts-available/CDB_CartodbfyTable.sql | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql index 6d520ae..a8bdae1 100644 --- a/scripts-available/CDB_CartodbfyTable.sql +++ b/scripts-available/CDB_CartodbfyTable.sql @@ -713,8 +713,18 @@ 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); + CREATE OR REPLACE FUNCTION _CDB_Has_Usable_Geom(reloid REGCLASS) -RETURNS RECORD +RETURNS _cdb_has_usable_geom_record AS $$ DECLARE r1 RECORD; @@ -918,14 +928,6 @@ 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, - has_geom boolean, - has_geom_name text, - has_mercgeom boolean, - has_mercgeom_name text) INTO STRICT gc; -- If geom is the wrong name, just rename it. @@ -937,7 +939,7 @@ BEGIN END IF; -- If mercgeom is the wrong name, just rename it. - IF gc.has_mercgeom AND gc.has_mercgeom_name != const.mercgeomcol THEN + IF gc.has_mercgeom AND gc.has_mercgeom_name != const.mercgeomcol THEN sql := Format('ALTER TABLE %I DROP COLUMN IF EXISTS %I', reloid::text, const.mercgeomcol); PERFORM _CDB_SQL(sql,'_CDB_Rewrite_Table'); sql := Format('ALTER TABLE %s RENAME COLUMN %s TO %s', reloid::text, gc.has_mercgeom_name, const.mercgeomcol); From e7c974e95714a571d9065fc0d2e65376964554ec Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Tue, 15 Sep 2015 16:56:57 +0200 Subject: [PATCH 12/17] Fix silly typo #141 --- 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 a8bdae1..3b1611a 100644 --- a/scripts-available/CDB_CartodbfyTable.sql +++ b/scripts-available/CDB_CartodbfyTable.sql @@ -1099,7 +1099,7 @@ BEGIN ) SELECT ', ST_Transform(' || t.missing_srid_start || t.attname || t.missing_srid_end - || ',4326)::Geometry(GEOMETRY,4326) AS' + || ',4326)::Geometry(GEOMETRY,4326) AS ' || const.geomcol || ', cartodb.CDB_TransformToWebmercator(' || t.missing_srid_start || t.attname || t.missing_srid_end From 0ebd12a0eb74aa9f3ec63d5938beb917bbcc8507 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Tue, 15 Sep 2015 17:18:00 +0200 Subject: [PATCH 13/17] Avoid double-escaping of reloid::text #141 --- scripts-available/CDB_CartodbfyTable.sql | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql index 3b1611a..08c8065 100644 --- a/scripts-available/CDB_CartodbfyTable.sql +++ b/scripts-available/CDB_CartodbfyTable.sql @@ -932,17 +932,17 @@ BEGIN -- 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 %I DROP COLUMN IF EXISTS %I', reloid::text, const.geomcol); + 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 %I RENAME COLUMN %I TO %I', reloid::text, gc.has_geom_name, const.geomcol); + 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 %I DROP COLUMN IF EXISTS %I', reloid::text, const.mercgeomcol); + 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 %s TO %s', reloid::text, gc.has_mercgeom_name, const.mercgeomcol); + 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; From fa514a3b7c0341e614f7b586b11ee909ffb07f2a Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Tue, 15 Sep 2015 17:48:12 +0200 Subject: [PATCH 14/17] Add a couple of NOTICE's to expectations #141 --- expected/test_ddl_triggers.out | 2 ++ 1 file changed, 2 insertions(+) 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 From d26849703028d1dc7510677405cae598216d9fac Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Tue, 15 Sep 2015 19:14:06 +0200 Subject: [PATCH 15/17] Add expectation for new test #141 --- test/CDB_CartodbfyTableTest_expect | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/CDB_CartodbfyTableTest_expect b/test/CDB_CartodbfyTableTest_expect index 537c416..b075abe 100644 --- a/test/CDB_CartodbfyTableTest_expect +++ b/test/CDB_CartodbfyTableTest_expect @@ -69,5 +69,11 @@ 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 DROP FUNCTION DROP FUNCTION From c8e3cf55008bafff88de7ce0ada45ea94d08deac Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Wed, 16 Sep 2015 12:09:32 +0200 Subject: [PATCH 16/17] Add test courtesy of Paul #154 --- test/CDB_CartodbfyTableTest.sql | 15 +++++++++++++++ test/CDB_CartodbfyTableTest_expect | 4 ++++ 2 files changed, 19 insertions(+) diff --git a/test/CDB_CartodbfyTableTest.sql b/test/CDB_CartodbfyTableTest.sql index b5678f4..b35540b 100644 --- a/test/CDB_CartodbfyTableTest.sql +++ b/test/CDB_CartodbfyTableTest.sql @@ -266,6 +266,21 @@ SELECT CDB_CartodbfyTableCheck('many_geometry_columns', 'Table with both the_geo 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 b075abe..6287c3f 100644 --- a/test/CDB_CartodbfyTableTest_expect +++ b/test/CDB_CartodbfyTableTest_expect @@ -75,5 +75,9 @@ 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 From 6ba809e798b007406429a284c574226351c861c7 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Wed, 16 Sep 2015 12:11:02 +0200 Subject: [PATCH 17/17] Remove usage of _CDB_Geometry_SRID #154 This is only used from _CDB_Has_Usable_Geom. It doesn't do what's promised in the comment. The effect is that a column is taken as valid when it actually needs setting its SRID restriction so better not use it as it will need rewrite anyway. --- scripts-available/CDB_CartodbfyTable.sql | 28 ++---------------------- 1 file changed, 2 insertions(+), 26 deletions(-) diff --git a/scripts-available/CDB_CartodbfyTable.sql b/scripts-available/CDB_CartodbfyTable.sql index 08c8065..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 rec IS NOT NULL THEN - RETURN rec.srid; - END IF; - - RETURN 0; - -END; -$$ LANGUAGE 'plpgsql'; -- Find out if the table already has a usable primary key @@ -811,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); @@ -824,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;