changeset 4158:5466562cca60

Remove utility function with possibly bad performance impact Since the PostgreSQL planner will call functions used in a filter condition once per row, even if the function is marked STABLE, under some circumstances (e.g. the removed usage in an RLS policy), remove the function from the database completely. The DEFAULT on users.templates that used the function is unused, thus remove it as a whole, too. Recreate the function as a helper for tests in order to minimize necessary changes there.
author Tom Gottfried <tom@intevation.de>
date Fri, 02 Aug 2019 16:10:42 +0200
parents dd61ee277aa6
children 80e9cfb2be98
files pkg/controllers/printtemplates.go pkg/imports/fa.go pkg/imports/gm.go schema/auth.sql schema/auth_tests.sql schema/manage_users.sql schema/updates/1104/01.remove_function.sql schema/version.sql
diffstat 8 files changed, 51 insertions(+), 28 deletions(-) [+]
line wrap: on
line diff
--- a/pkg/controllers/printtemplates.go	Fri Aug 02 13:37:40 2019 +0200
+++ b/pkg/controllers/printtemplates.go	Fri Aug 02 16:10:42 2019 +0200
@@ -59,7 +59,8 @@
   $2::template_types,
   $3,
   CASE WHEN pg_has_role('sys_admin', 'MEMBER') THEN NULL
-       ELSE users.current_user_country()
+       ELSE (SELECT country FROM users.list_users
+             WHERE username = current_user)
   END`
 
 	updatePrintTemplateSQL = `
--- a/pkg/imports/fa.go	Fri Aug 02 13:37:40 2019 +0200
+++ b/pkg/imports/fa.go	Fri Aug 02 16:10:42 2019 +0200
@@ -46,7 +46,8 @@
 SELECT DISTINCT
   bottleneck_id
 FROM waterway.bottlenecks
-WHERE responsible_country = users.current_user_country()
+WHERE responsible_country = (
+    SELECT country FROM users.list_users WHERE username = current_user)
   AND staging_done = true
 ORDER BY bottleneck_id
 `
--- a/pkg/imports/gm.go	Fri Aug 02 13:37:40 2019 +0200
+++ b/pkg/imports/gm.go	Fri Aug 02 16:10:42 2019 +0200
@@ -52,7 +52,8 @@
   (location).orc,
   (location).hectometre
 FROM waterway.gauges
-WHERE (location).country_code = users.current_user_country()
+WHERE (location).country_code = (
+    SELECT country FROM users.list_users WHERE username = current_user)
   OR pg_has_role('sys_admin', 'MEMBER')
 `
 
--- a/schema/auth.sql	Fri Aug 02 13:37:40 2019 +0200
+++ b/schema/auth.sql	Fri Aug 02 16:10:42 2019 +0200
@@ -41,9 +41,6 @@
 
 GRANT INSERT, UPDATE, DELETE ON
     users.templates TO waterway_admin;
--- Ensure templates are associated to the users country, if none is given
-ALTER TABLE users.templates ALTER COLUMN country
-    SET DEFAULT users.current_user_country();
 
 GRANT USAGE ON SCHEMA import TO waterway_admin;
 GRANT SELECT, INSERT ON ALL TABLES IN SCHEMA import TO waterway_admin;
@@ -157,8 +154,8 @@
 
 CREATE POLICY same_country ON import.imports
     FOR ALL TO waterway_admin
-    USING (users.current_user_country() = (
-        SELECT country FROM users.list_users lu
+    -- Relies on a user seeing only users from his own country:
+    USING (EXISTS(SELECT 1 FROM users.list_users lu
             WHERE lu.username = imports.username));
 ALTER table import.imports ENABLE ROW LEVEL SECURITY;
 
--- a/schema/auth_tests.sql	Fri Aug 02 13:37:40 2019 +0200
+++ b/schema/auth_tests.sql	Fri Aug 02 16:10:42 2019 +0200
@@ -15,6 +15,17 @@
 -- pgTAP test script for privileges and RLS policies
 --
 
+-- Helper function:
+CREATE OR REPLACE FUNCTION users.current_user_country()
+    RETURNS internal.user_profiles.country%TYPE
+    AS $$
+        SELECT country FROM users.list_users
+            WHERE username = current_user
+    $$
+    LANGUAGE SQL
+    STABLE PARALLEL SAFE;
+
+
 CREATE FUNCTION test_privs() RETURNS SETOF TEXT AS
 $$
 DECLARE the_schema CONSTANT varchar = 'waterway';
@@ -103,13 +114,9 @@
     'Waterway admin cannot insert data outside his region');
 
 -- template management
-SELECT results_eq($$
-    SELECT users.current_user_country()
-    $$,
-    $$
-    INSERT INTO users.templates (template_name, template_data)
-        VALUES ('New AT', '\x')
-        RETURNING country
+SELECT lives_ok($$
+    INSERT INTO users.templates (template_name, template_data, country)
+        VALUES ('New AT', '\x', 'AT')
     $$,
     'Waterway admin can add templates for his country');
 
--- a/schema/manage_users.sql	Fri Aug 02 13:37:40 2019 +0200
+++ b/schema/manage_users.sql	Fri Aug 02 16:10:42 2019 +0200
@@ -58,16 +58,6 @@
             OR pg_has_role('sys_admin', 'MEMBER');
 
 
-CREATE OR REPLACE FUNCTION users.current_user_country()
-    RETURNS internal.user_profiles.country%TYPE
-    AS $$
-        SELECT country FROM users.list_users
-            WHERE username = current_user
-    $$
-    LANGUAGE SQL
-    STABLE PARALLEL SAFE;
-
-
 CREATE OR REPLACE FUNCTION users.current_user_area_utm()
     RETURNS geometry
     AS $$
@@ -76,8 +66,9 @@
             SELECT ST_Transform(area::geometry, best_utm(area))
                 INTO STRICT utm_area
                 FROM users.responsibility_areas
-                WHERE country = users.current_user_country();
-            RETURN utm_area;
+                WHERE country = (SELECT country
+                    FROM users.list_users WHERE username = current_user);
+    RETURN utm_area;
         END;
     $$
     LANGUAGE plpgsql
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/schema/updates/1104/01.remove_function.sql	Fri Aug 02 16:10:42 2019 +0200
@@ -0,0 +1,25 @@
+DROP POLICY same_country ON import.imports;
+CREATE POLICY same_country ON import.imports
+    FOR ALL TO waterway_admin
+    USING (EXISTS(SELECT 1 FROM users.list_users lu
+            WHERE lu.username = imports.username));
+
+ALTER TABLE users.templates ALTER COLUMN country DROP DEFAULT;
+
+CREATE OR REPLACE FUNCTION users.current_user_area_utm()
+    RETURNS geometry
+    AS $$
+        DECLARE utm_area geometry;
+        BEGIN
+            SELECT ST_Transform(area::geometry, best_utm(area))
+                INTO STRICT utm_area
+                FROM users.responsibility_areas
+                WHERE country = (SELECT country
+                    FROM users.list_users WHERE username = current_user);
+    RETURN utm_area;
+        END;
+    $$
+    LANGUAGE plpgsql
+    STABLE PARALLEL SAFE;
+
+DROP FUNCTION users.current_user_country();
--- a/schema/version.sql	Fri Aug 02 13:37:40 2019 +0200
+++ b/schema/version.sql	Fri Aug 02 16:10:42 2019 +0200
@@ -1,1 +1,1 @@
-INSERT INTO gemma_schema_version(version) VALUES (1103);
+INSERT INTO gemma_schema_version(version) VALUES (1104);