Mercurial > gemma

changeset 4161:64cd18281c76

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Improve performance of row level security policies Using constraint_column_usage instead of key_column_usage makes the query twice as fast. I did not explore why. Let's just take it. Using 'EXISTS(... WHERE ... = value) is often more efficient than value IN(...) since it allows the inner query to be executed only up to the point where it turns out to return more than nothing with filtering directly in place.
author Tom Gottfried <tom@intevation.de>
date Fri, 02 Aug 2019 17:14:13 +0200
parents 7cccf7fef3e8
children 8c5df0f3562e
files schema/auth.sql schema/updates/1105/01.improve_rls_performance.sql schema/version.sql
diffstat 3 files changed, 69 insertions(+), 18 deletions(-) [+]
line wrap: on
line diff
--- a/schema/auth.sql	Fri Aug 02 17:08:58 2019 +0200
+++ b/schema/auth.sql	Fri Aug 02 17:14:13 2019 +0200
@@ -76,8 +76,8 @@
 --
 -- Sometimes using FOR ALL because we rely on GRANTed privileges for allowing
 -- data modifications generally.
--- Sometimes using 'username IN(SELECT username FROM users.list_users)' instead
--- of 'username = current_user', because waterway_admin is intentionally
+-- Sometimes using 'EXISTS(SELECT 1 FROM list_users WHERE username = ...)'
+-- instead of 'username = current_user', since waterway_admin is intentionally
 -- allowed more with these policies (note that the subselect implies different
 -- filtering on list_users depending on current_user).
 --
@@ -176,11 +176,11 @@
 DECLARE ret boolean;
 BEGIN
     columnname = (SELECT column_name
-        FROM information_schema.key_column_usage k
+        FROM information_schema.constraint_column_usage k
         JOIN information_schema.table_constraints USING (constraint_name)
         WHERE k.table_name = tablename and constraint_type = 'PRIMARY KEY');
-    EXECUTE format('SELECT NOT $1 = ANY(SELECT %I FROM import.%I)',
-        columnname, tablename)
+    EXECUTE format('SELECT NOT EXISTS(SELECT 1 FROM import.%I WHERE %I = $1)',
+            tablename, columnname)
         INTO ret
         USING kv;
     RETURN ret;
@@ -192,24 +192,23 @@
 
 CREATE POLICY parent_allowed ON import.import_logs
     FOR ALL TO waterway_admin
-    USING (import_id IN (SELECT id FROM import.imports))
+    USING (EXISTS(SELECT 1 FROM import.imports WHERE id = import_id))
     WITH CHECK (import.is_new_key('imports', import_id)
-        OR import_id IN (SELECT id FROM import.imports));
+        OR EXISTS(SELECT 1 FROM import.imports WHERE id = import_id));
 ALTER table import.import_logs ENABLE ROW LEVEL SECURITY;
 
 CREATE POLICY parent_allowed ON import.track_imports
     FOR ALL TO waterway_admin
-    USING (import_id IN (SELECT id FROM import.imports))
+    USING (EXISTS(SELECT 1 FROM import.imports WHERE id = import_id))
     WITH CHECK (import.is_new_key('imports', import_id)
-        OR import_id IN (SELECT id FROM import.imports));
+        OR EXISTS(SELECT 1 FROM import.imports WHERE id = import_id));
 ALTER table import.track_imports ENABLE ROW LEVEL SECURITY;
 
 CREATE POLICY import_configuration_policy ON import.import_configuration
     FOR ALL TO waterway_admin
-    USING (
-        (SELECT country FROM users.list_users WHERE username = current_user) = (
-            SELECT country FROM users.list_users lu
-            WHERE lu.username = import.import_configuration.username));
+    -- Relies on a user seeing only users from his own country:
+    USING (EXISTS(SELECT 1 FROM users.list_users lu
+            WHERE lu.username = import_configuration.username));
 
 CREATE POLICY import_configuration_policy_sys_admin ON import.import_configuration
     FOR ALL TO sys_admin
@@ -219,12 +218,12 @@
 
 CREATE POLICY parent_allowed ON import.import_configuration_attributes
     FOR ALL TO waterway_admin
-    USING (import_configuration_id IN (
-        SELECT id FROM import.import_configuration))
+    USING (EXISTS(SELECT 1 FROM import.import_configuration
+            WHERE id = import_configuration_id))
     WITH CHECK (
         import.is_new_key('import_configuration', import_configuration_id)
-        OR import_configuration_id IN (
-            SELECT id FROM import.import_configuration));
+        OR EXISTS(SELECT 1 FROM import.import_configuration
+            WHERE id = import_configuration_id));
 ALTER table import.import_configuration_attributes ENABLE ROW LEVEL SECURITY;
 
 COMMIT;
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/schema/updates/1105/01.improve_rls_performance.sql	Fri Aug 02 17:14:13 2019 +0200
@@ -0,0 +1,52 @@
+CREATE OR REPLACE FUNCTION import.is_new_key(
+        tablename varchar,
+        kv anyelement)
+    RETURNS boolean
+AS $$
+DECLARE columnname varchar;
+DECLARE ret boolean;
+BEGIN
+    columnname = (SELECT column_name
+        FROM information_schema.constraint_column_usage k
+        JOIN information_schema.table_constraints USING (constraint_name)
+        WHERE k.table_name = tablename and constraint_type = 'PRIMARY KEY');
+    EXECUTE format('SELECT NOT EXISTS(SELECT 1 FROM import.%I WHERE %I = $1)',
+            tablename, columnname)
+        INTO ret
+        USING kv;
+    RETURN ret;
+END;
+$$
+    LANGUAGE plpgsql
+    SECURITY DEFINER
+    STABLE PARALLEL SAFE;
+
+DROP POLICY parent_allowed ON import.import_logs;
+CREATE POLICY parent_allowed ON import.import_logs
+    FOR ALL TO waterway_admin
+    USING (EXISTS(SELECT 1 FROM import.imports WHERE id = import_id))
+    WITH CHECK (import.is_new_key('imports', import_id)
+        OR EXISTS(SELECT 1 FROM import.imports WHERE id = import_id));
+
+DROP POLICY parent_allowed ON import.track_imports;
+CREATE POLICY parent_allowed ON import.track_imports
+    FOR ALL TO waterway_admin
+    USING (EXISTS(SELECT 1 FROM import.imports WHERE id = import_id))
+    WITH CHECK (import.is_new_key('imports', import_id)
+        OR EXISTS(SELECT 1 FROM import.imports WHERE id = import_id));
+
+DROP POLICY import_configuration_policy ON import.import_configuration;
+CREATE POLICY import_configuration_policy ON import.import_configuration
+    FOR ALL TO waterway_admin
+    USING (EXISTS(SELECT 1 FROM users.list_users lu
+            WHERE lu.username = import_configuration.username));
+
+DROP POLICY parent_allowed ON import.import_configuration_attributes;
+CREATE POLICY parent_allowed ON import.import_configuration_attributes
+    FOR ALL TO waterway_admin
+    USING (EXISTS(SELECT 1 FROM import.import_configuration
+            WHERE id = import_configuration_id))
+    WITH CHECK (
+        import.is_new_key('import_configuration', import_configuration_id)
+        OR EXISTS(SELECT 1 FROM import.import_configuration
+            WHERE id = import_configuration_id));
--- a/schema/version.sql	Fri Aug 02 17:08:58 2019 +0200
+++ b/schema/version.sql	Fri Aug 02 17:14:13 2019 +0200
@@ -1,1 +1,1 @@
-INSERT INTO gemma_schema_version(version) VALUES (1104);
+INSERT INTO gemma_schema_version(version) VALUES (1105);