From 5c3c0f5fc9a05c1e034d2694937fa94d3bb91a26 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 6 Apr 2016 18:57:52 +0200 Subject: [PATCH 01/10] Fix bug in CDB_DropOverviews Fixes #223 --- scripts-available/CDB_Overviews.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts-available/CDB_Overviews.sql b/scripts-available/CDB_Overviews.sql index b80be3d..bfd1a09 100644 --- a/scripts-available/CDB_Overviews.sql +++ b/scripts-available/CDB_Overviews.sql @@ -120,7 +120,7 @@ BEGIN FOR row IN SELECT * FROM CDB_Overviews(reloid) LOOP - EXECUTE Format('DROP TABLE %I.%I;', schema_name, row.overview_table); + EXECUTE Format('DROP TABLE %s;', row.overview_table); RAISE NOTICE 'Dropped overview for level %: %', row.z, row.overview_table; END LOOP; END; From c1fc07d2ac37fdfa141e8bd6225293bea387b3f7 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 6 Apr 2016 18:58:37 +0200 Subject: [PATCH 02/10] Fix typo This function isn't beint actively used; should consider removing it or testing it properly --- scripts-available/CDB_Overviews.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts-available/CDB_Overviews.sql b/scripts-available/CDB_Overviews.sql index bfd1a09..ec26813 100644 --- a/scripts-available/CDB_Overviews.sql +++ b/scripts-available/CDB_Overviews.sql @@ -370,7 +370,7 @@ AS $$ SELECT * FROM _cdb_split_table_name(reloid) INTO schema_name, table_name; - EXECUTE Format('DROP TABLE IF EXISTS %I.%I CASCADE;', schema_name.overview_rel); + EXECUTE Format('DROP TABLE IF EXISTS %I.%I CASCADE;', schema_name, overview_rel); -- Estimate number of rows SELECT reltuples, relpages FROM pg_class INTO STRICT class_info From 84cac16d1c780d5ea927e6ea315d4887f7c314a5 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 6 Apr 2016 22:05:00 +0200 Subject: [PATCH 03/10] Temporary fix --- scripts-available/CDB_Overviews.sql | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/scripts-available/CDB_Overviews.sql b/scripts-available/CDB_Overviews.sql index ec26813..47ecb86 100644 --- a/scripts-available/CDB_Overviews.sql +++ b/scripts-available/CDB_Overviews.sql @@ -142,9 +142,12 @@ AS $$ schema_name TEXT; table_name TEXT; BEGIN - -- TODO: review implementation of CDB_UserTables an suitability for this SELECT * FROM _cdb_split_table_name(reloid) INTO schema_name, table_name; - RETURN QUERY SELECT + -- TODO: replace use of CDB_UserTables by obtaining the user tables + -- in a specific schema + -- Meanwhile we'll use DISTINCT here to avoid picking multiple tables + -- from different schemas + RETURN QUERY SELECT DISTINCT reloid AS base_table, _CDB_OverviewTableZ(cdb_usertables) AS z, ('"' || schema_name|| '"."' ||cdb_usertables || '"')::regclass AS overview_table From 34c39662ec3c30b9da4787e405c3e0d016c844e8 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Thu, 7 Apr 2016 00:07:45 +0200 Subject: [PATCH 04/10] Replace use of CDB_UserTables in CDB_Overviews Use a function that returns reclasses and schema names properly instead. --- scripts-available/CDB_Overviews.sql | 56 ++++++++++++++++++++--------- 1 file changed, 40 insertions(+), 16 deletions(-) diff --git a/scripts-available/CDB_Overviews.sql b/scripts-available/CDB_Overviews.sql index 47ecb86..65dfbb3 100644 --- a/scripts-available/CDB_Overviews.sql +++ b/scripts-available/CDB_Overviews.sql @@ -1,4 +1,30 @@ --- security definer +-- Information about tables in a schema. +-- If the schema name parameter is NULL, then tables from all schemas +-- that may contain user tables are returned. +-- The optional second argument restricts the result to tables +-- of the specified access type. +-- Currently accepted permissions are: 'public', 'private' or 'all'. +-- For each table, the regclass, schema name and table name are returned. +-- Scope: private. +CREATE OR REPLACE FUNCTION _CDB_UserTablesInSchema(schema_name text DEFAULT NULL, perm text DEFAULT 'all') +RETURNS TABLE(table_regclass REGCLASS, schema_name TEXT, table_name TEXT) +AS $$ + SELECT + c.oid::regclass AS table_regclass, + n.nspname::text AS schema_name, + c.relname::text AS table_relname + FROM pg_class c + JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE c.relkind = 'r' + AND c.relname NOT IN ('cdb_tablemetadata', 'spatial_ref_sys') + AND CASE WHEN schema_name IS NULL + THEN n.nspname NOT IN ('pg_catalog', 'information_schema', 'topology', 'cartodb') + ELSE n.nspname = schema_name END + AND CASE WHEN perm = 'public' THEN has_table_privilege('publicuser', c.oid, 'SELECT') + WHEN perm = 'private' THEN has_table_privilege(current_user, c.oid, 'SELECT') AND NOT has_table_privilege('publicuser', c.oid, 'SELECT') + WHEN perm = 'all' THEN has_table_privilege(current_user, c.oid, 'SELECT') OR has_table_privilege('publicuser', c.oid, 'SELECT') + ELSE false END; +$$ LANGUAGE 'sql'; -- Pattern that can be used to detect overview tables and Extract -- the intended zoom level from the table name. @@ -140,19 +166,15 @@ RETURNS TABLE(base_table REGCLASS, z integer, overview_table REGCLASS) AS $$ DECLARE schema_name TEXT; - table_name TEXT; + _table_name TEXT; BEGIN - SELECT * FROM _cdb_split_table_name(reloid) INTO schema_name, table_name; - -- TODO: replace use of CDB_UserTables by obtaining the user tables - -- in a specific schema - -- Meanwhile we'll use DISTINCT here to avoid picking multiple tables - -- from different schemas - RETURN QUERY SELECT DISTINCT + SELECT * FROM _cdb_split_table_name(reloid) INTO schema_name, _table_name; + RETURN QUERY SELECT reloid AS base_table, - _CDB_OverviewTableZ(cdb_usertables) AS z, - ('"' || schema_name|| '"."' ||cdb_usertables || '"')::regclass AS overview_table - FROM CDB_UserTables() - WHERE _CDB_IsOverviewTableOf((SELECT relname FROM pg_class WHERE oid=reloid), cdb_usertables) + _CDB_OverviewTableZ(table_name) AS z, + table_regclass AS overview_table + FROM _CDB_UserTablesInSchema(schema_name) + WHERE _CDB_IsOverviewTableOf((SELECT relname FROM pg_class WHERE oid=reloid), table_name) ORDER BY z; END $$ LANGUAGE PLPGSQL; @@ -171,11 +193,13 @@ RETURNS TABLE(base_table REGCLASS, z integer, overview_table REGCLASS) AS $$ SELECT base_table::regclass AS base_table, - _CDB_OverviewTableZ(cdb_usertables) AS z, - ('"' || _cdb_schema_name(base_table::regclass) || '"."' || cdb_usertables || '"')::regclass AS overview_table + _CDB_OverviewTableZ(table_name) AS z, + table_regclass AS overview_table FROM - CDB_UserTables(), unnest(tables) base_table - WHERE _CDB_IsOverviewTableOf((SELECT relname FROM pg_class WHERE oid=base_table), cdb_usertables) + _CDB_UserTablesInSchema(), unnest(tables) base_table + WHERE + schema_name = _cdb_schema_name(base_table) + AND _CDB_IsOverviewTableOf((SELECT relname FROM pg_class WHERE oid=base_table), table_name) ORDER BY base_table, z; $$ LANGUAGE SQL; From fb910be12f412b123c41e5d1075d8b4b18c70b92 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Thu, 7 Apr 2016 07:07:20 +0200 Subject: [PATCH 05/10] Fix conversion of regclass to indentifier --- scripts-available/CDB_Overviews.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts-available/CDB_Overviews.sql b/scripts-available/CDB_Overviews.sql index 65dfbb3..1295097 100644 --- a/scripts-available/CDB_Overviews.sql +++ b/scripts-available/CDB_Overviews.sql @@ -231,7 +231,7 @@ AS $$ -- This is the typical ERROR: stats for "mytable" do not exist WHEN internal_error THEN -- Get stats and execute again - EXECUTE format('ANALYZE %1$I', reloid); + EXECUTE format('ANALYZE %1$s', reloid); EXECUTE ext_query INTO ext; END; From 49e7094c8a4ea8649bc68759da576f2112b924d3 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Thu, 7 Apr 2016 07:52:58 +0200 Subject: [PATCH 06/10] Make CDB_CreateOverview usable by superuser CDB_CreateOverview had to be executed with the user role corresponding to the owner of the table; now it can be executed by the postgres user. --- scripts-available/CDB_Overviews.sql | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/scripts-available/CDB_Overviews.sql b/scripts-available/CDB_Overviews.sql index 1295097..3c64429 100644 --- a/scripts-available/CDB_Overviews.sql +++ b/scripts-available/CDB_Overviews.sql @@ -411,16 +411,16 @@ AS $$ ELSE num_samples := ceil(class_info.reltuples*fraction); EXECUTE Format(' - CREATE TABLE %1$I AS SELECT * FROM %2$s + CREATE TABLE %4$I.%1$I AS SELECT * FROM %2$s WHERE ctid = ANY ( ARRAY[ (SELECT CDB_RandomTids(''%2$s'', %3$s)) ] ); - ', overview_rel, reloid, num_samples); + ', overview_rel, reloid, num_samples, schema_name); END IF; - RETURN overview_rel; + RETURN Format('%I.%I', schema_name, overview_rel)::regclass; END; $$ LANGUAGE PLPGSQL; @@ -456,9 +456,12 @@ AS $$ -- preserve the owner of the base table SELECT u.usename - FROM pg_catalog.pg_class c JOIN pg_catalog.pg_user u ON (c.relowner=u.usesysid) - WHERE c.relname = dataset::text + FROM pg_catalog.pg_class c + JOIN pg_catalog.pg_user u ON (c.relowner=u.usesysid) + JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE c.relname = dataset_name::text AND n.nspname = dataset_scheme INTO table_owner; + EXECUTE Format('ALTER TABLE IF EXISTS %s OWNER TO %I;', overview_table::text, table_owner); -- preserve the table privileges @@ -682,7 +685,7 @@ AS $$ -- If we had a selected numeric attribute of interest we could use it -- as a weight for the average coordinates. EXECUTE Format(' - CREATE TABLE %3$I AS + CREATE TABLE %7$I.%3$I AS WITH clusters AS ( SELECT %5$s @@ -696,9 +699,9 @@ AS $$ GROUP BY gx, gy ) SELECT %6$s FROM clusters - ', reloid::text, grid_m, overview_rel, attributes, aggr_attributes, columns); + ', reloid::text, grid_m, overview_rel, attributes, aggr_attributes, columns, schema_name); - RETURN overview_rel; + RETURN Format('%I.%I', schema_name, overview_rel)::regclass; END; $$ LANGUAGE PLPGSQL; From ee61d461002c240d3b3c01535ba0ce415221bc46 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Thu, 7 Apr 2016 10:24:02 +0200 Subject: [PATCH 07/10] :lipstick: rename variable for clarity --- scripts-available/CDB_Overviews.sql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts-available/CDB_Overviews.sql b/scripts-available/CDB_Overviews.sql index 3c64429..bb3c5f6 100644 --- a/scripts-available/CDB_Overviews.sql +++ b/scripts-available/CDB_Overviews.sql @@ -166,9 +166,9 @@ RETURNS TABLE(base_table REGCLASS, z integer, overview_table REGCLASS) AS $$ DECLARE schema_name TEXT; - _table_name TEXT; + base_table_name TEXT; BEGIN - SELECT * FROM _cdb_split_table_name(reloid) INTO schema_name, _table_name; + SELECT * FROM _cdb_split_table_name(reloid) INTO schema_name, base_table_name; RETURN QUERY SELECT reloid AS base_table, _CDB_OverviewTableZ(table_name) AS z, From 15ac9a2cd943d4e7fc9734120db01496fad6c6dc Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Thu, 7 Apr 2016 10:30:10 +0200 Subject: [PATCH 08/10] Remove unneeded code --- scripts-available/CDB_Overviews.sql | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/scripts-available/CDB_Overviews.sql b/scripts-available/CDB_Overviews.sql index bb3c5f6..f8e2871 100644 --- a/scripts-available/CDB_Overviews.sql +++ b/scripts-available/CDB_Overviews.sql @@ -1,12 +1,9 @@ -- Information about tables in a schema. -- If the schema name parameter is NULL, then tables from all schemas -- that may contain user tables are returned. --- The optional second argument restricts the result to tables --- of the specified access type. --- Currently accepted permissions are: 'public', 'private' or 'all'. -- For each table, the regclass, schema name and table name are returned. -- Scope: private. -CREATE OR REPLACE FUNCTION _CDB_UserTablesInSchema(schema_name text DEFAULT NULL, perm text DEFAULT 'all') +CREATE OR REPLACE FUNCTION _CDB_UserTablesInSchema(schema_name text DEFAULT NULL) RETURNS TABLE(table_regclass REGCLASS, schema_name TEXT, table_name TEXT) AS $$ SELECT @@ -19,11 +16,8 @@ AS $$ AND c.relname NOT IN ('cdb_tablemetadata', 'spatial_ref_sys') AND CASE WHEN schema_name IS NULL THEN n.nspname NOT IN ('pg_catalog', 'information_schema', 'topology', 'cartodb') - ELSE n.nspname = schema_name END - AND CASE WHEN perm = 'public' THEN has_table_privilege('publicuser', c.oid, 'SELECT') - WHEN perm = 'private' THEN has_table_privilege(current_user, c.oid, 'SELECT') AND NOT has_table_privilege('publicuser', c.oid, 'SELECT') - WHEN perm = 'all' THEN has_table_privilege(current_user, c.oid, 'SELECT') OR has_table_privilege('publicuser', c.oid, 'SELECT') - ELSE false END; + ELSE n.nspname = schema_name + END; $$ LANGUAGE 'sql'; -- Pattern that can be used to detect overview tables and Extract From f96163265b82d7421f12dbfa2c59dd121ad8164c Mon Sep 17 00:00:00 2001 From: Carla Iriberri Date: Wed, 13 Apr 2016 17:49:38 +0200 Subject: [PATCH 09/10] Fix bug for tables without geom or with no potential overviews If the table doesn't have geometries but the createoverviews function is called, the current geometry type checks won't work because "null" will not give a boolean value in the type comparisons. Also, if the createoverviews function is called over a simple table with would not require overviews according to the strategies it is handled correctly. --- scripts-available/CDB_Overviews.sql | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/scripts-available/CDB_Overviews.sql b/scripts-available/CDB_Overviews.sql index f8e2871..807c683 100644 --- a/scripts-available/CDB_Overviews.sql +++ b/scripts-available/CDB_Overviews.sql @@ -226,6 +226,12 @@ AS $$ WHEN internal_error THEN -- Get stats and execute again EXECUTE format('ANALYZE %1$s', reloid); + + -- We check the geometry type in case the error is due to empty geometries + IF _CDB_GeometryTypes(reloid) IS NULL THEN + RETURN NULL; + END IF; + EXECUTE ext_query INTO ext; END; @@ -622,7 +628,7 @@ AS $$ table_name TEXT; BEGIN SELECT _CDB_GeometryTypes(reloid) INTO gtypes; - IF array_upper(gtypes, 1) <> 1 OR gtypes[1] <> 'ST_Point' THEN + IF gtypes IS NULL OR array_upper(gtypes, 1) <> 1 OR gtypes[1] <> 'ST_Point' THEN -- This strategy only supports datasets with point geomety RETURN NULL; RETURN 'x'; @@ -738,6 +744,10 @@ BEGIN -- Determine the referece zoom level EXECUTE 'SELECT ' || quote_ident(refscale_strategy::text) || Format('(''%s'', %s);', reloid, tolerance_px) INTO ref_z; + IF ref_z < 0 OR ref_z IS NULL THEN + RETURN NULL; + END IF; + -- Determine overlay zoom levels -- TODO: should be handled by the refscale_strategy? overview_z := ref_z - 1; From 8f44f5347ac294c20855f48a49a671b31d24467c Mon Sep 17 00:00:00 2001 From: Carla Iriberri Date: Wed, 13 Apr 2016 17:51:30 +0200 Subject: [PATCH 10/10] Fix indent for code clarity --- scripts-available/CDB_Overviews.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts-available/CDB_Overviews.sql b/scripts-available/CDB_Overviews.sql index 807c683..7f92faf 100644 --- a/scripts-available/CDB_Overviews.sql +++ b/scripts-available/CDB_Overviews.sql @@ -221,7 +221,7 @@ AS $$ BEGIN EXECUTE ext_query INTO ext; - EXCEPTION + EXCEPTION -- This is the typical ERROR: stats for "mytable" do not exist WHEN internal_error THEN -- Get stats and execute again