Fix brown paper bag bug in pg_qualstats snapshot.

Commit 0e8915f added some predicates to filter out quals that don't have a matching entry in powa_statements, but there was a stupid copy paste error so that no quals could be stored. Also add new regression tests to make sure that this won't get broken again. Thanks to github user alepaes1975 for the report.
powa-team · Dec 18, 2020 · babce62 · babce62
1 parent 665726d
commit babce62
Show file tree

Hide file tree

Showing 5 changed files with 154 additions and 11 deletions.
diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md
@@ -30,3 +30,4 @@ Contributors to PoWA :
   * pessonnier
   * Andriy Bartash
   * TezkaRabota
+  * github user alepaes1975
diff --git a/expected/powa-archivist.out b/expected/powa-archivist.out
@@ -299,32 +299,75 @@ SELECT hostname FROM powa_servers WHERE id = 1;
 (1 row)
 
 -- Check missing powa_statements FK for pg_qualstats doesn't prevent snapshot
-INSERT INTO powa_qualstats_src_tmp(srvid, ts, uniquequalnodeid, dbid, userid,
+INSERT INTO public.powa_qualstats_src_tmp(srvid, ts, uniquequalnodeid, dbid, userid,
     qualnodeid, occurences, execution_count, nbfiltered,
     mean_err_estimate_ratio, mean_err_estimate_num,
     queryid, constvalues, quals)
     SELECT 1, now(), 1, 1, 1,
         1, 1000, 1, 0,
         0, 0,
-        1, '{}', ARRAY[(1259,1,607,'i')::qual_type];
-SELECT count(*) FROM powa_qualstats_src_tmp;
+        123456789, '{}', ARRAY[(1259,1,607,'i')::qual_type];
+SELECT count(*) FROM public.powa_qualstats_src_tmp;
  count 
 -------
      1
 (1 row)
 
-SELECT powa_qualstats_snapshot(1);
+SELECT public.powa_qualstats_snapshot(1);
  powa_qualstats_snapshot 
 -------------------------
 
 (1 row)
 
-SELECT count(*) FROM powa_qualstats_src_tmp;
+SELECT count(*) FROM public.powa_qualstats_src_tmp;
  count 
 -------
      0
 (1 row)
 
+SELECT count(*) FROM public.powa_qualstats_quals_history_current WHERE srvid = 1;
+ count 
+-------
+     0
+(1 row)
+
+-- Check snapshot of regular quals
+INSERT INTO public.powa_databases(srvid, oid, datname, dropped)
+    VALUES (1, 16384, 'postgres', NULL);
+INSERT INTO public.powa_statements(srvid, queryid, dbid, userid, query)
+    VALUES(1, 123456789, 16384, 10, 'query with qual');
+INSERT INTO public.powa_qualstats_src_tmp(srvid, ts, uniquequalnodeid, dbid, userid,
+    qualnodeid, occurences, execution_count, nbfiltered,
+    mean_err_estimate_ratio, mean_err_estimate_num,
+    queryid, constvalues, quals)
+    SELECT 1, now(), 1, 16384, 10,
+        1, 1000, 1, 0,
+        0, 0,
+        123456789, '{}', ARRAY[(1259,1,607,'i')::qual_type];
+SELECT count(*) FROM public.powa_qualstats_src_tmp;
+ count 
+-------
+     1
+(1 row)
+
+SELECT public.powa_qualstats_snapshot(1);
+ powa_qualstats_snapshot 
+-------------------------
+
+(1 row)
+
+SELECT count(*) FROM public.powa_qualstats_src_tmp;
+ count 
+-------
+     0
+(1 row)
+
+SELECT count(*) FROM public.powa_qualstats_quals_history_current WHERE srvid = 1;
+ count 
+-------
+     1
+(1 row)
+
 -- activate / deactivate extension
 SELECT * FROM public.powa_functions ORDER BY srvid, module, operation, function_name;
  srvid |          module          | operation  |         function_name         |      query_source       |        query_cleanup        | added_manually | enabled | priority |      extname       

diff --git a/powa--4.1.1--4.1.2.sql b/powa--4.1.1--4.1.2.sql
@@ -5,3 +5,83 @@ ALTER TABLE public.powa_extensions DROP CONSTRAINT powa_extensions_srvid_fkey;
 ALTER TABLE public.powa_extensions ADD
     FOREIGN KEY (srvid) REFERENCES public.powa_servers (id)
     MATCH FULL ON UPDATE CASCADE ON DELETE CASCADE;
+
+
+CREATE OR REPLACE FUNCTION powa_qualstats_snapshot(_srvid integer) RETURNS void as $PROC$
+DECLARE
+    result     bool;
+    v_funcname text := 'powa_qualstats_snapshot';
+    v_rowcount bigint;
+BEGIN
+  PERFORM powa_log(format('running %I', v_funcname));
+
+  PERFORM powa_prevent_concurrent_snapshot(_srvid);
+
+  WITH capture AS (
+    SELECT *
+    FROM powa_qualstats_src(_srvid) q
+    WHERE EXISTS (SELECT 1
+      FROM powa_statements s
+      WHERE s.srvid = _srvid
+      AND q.queryid = s.queryid
+      AND q.dbid = s.dbid
+      AND q.userid = s.userid)
+  ),
+  missing_quals AS (
+      INSERT INTO public.powa_qualstats_quals (srvid, qualid, queryid, dbid, userid, quals)
+        SELECT DISTINCT _srvid AS srvid, qs.qualnodeid, qs.queryid, qs.dbid, qs.userid,
+          array_agg(DISTINCT q::qual_type)
+        FROM capture qs,
+        LATERAL (SELECT (unnest(quals)).*) as q
+        WHERE NOT EXISTS (
+          SELECT 1
+          FROM powa_qualstats_quals nh
+          WHERE nh.srvid = _srvid
+            AND nh.qualid = qs.qualnodeid
+            AND nh.queryid = qs.queryid
+            AND nh.dbid = qs.dbid
+            AND nh.userid = qs.userid
+        )
+        GROUP BY srvid, qualnodeid, qs.queryid, qs.dbid, qs.userid
+      RETURNING *
+  ),
+  by_qual AS (
+      INSERT INTO public.powa_qualstats_quals_history_current (srvid, qualid, queryid,
+        dbid, userid, ts, occurences, execution_count, nbfiltered,
+        mean_err_estimate_ratio, mean_err_estimate_num)
+      SELECT _srvid AS srvid, qs.qualnodeid, qs.queryid, qs.dbid, qs.userid,
+          ts, sum(occurences), sum(execution_count), sum(nbfiltered),
+          avg(mean_err_estimate_ratio), avg(mean_err_estimate_num)
+        FROM capture as qs
+        GROUP BY srvid, ts, qualnodeid, qs.queryid, qs.dbid, qs.userid
+      RETURNING *
+  ),
+  by_qual_with_const AS (
+      INSERT INTO public.powa_qualstats_constvalues_history_current(srvid, qualid,
+        queryid, dbid, userid, ts, occurences, execution_count, nbfiltered,
+        mean_err_estimate_ratio, mean_err_estimate_num, constvalues)
+      SELECT _srvid, qualnodeid, qs.queryid, qs.dbid, qs.userid, ts,
+        occurences, execution_count, nbfiltered, mean_err_estimate_ratio,
+        mean_err_estimate_num, constvalues
+      FROM capture as qs
+  )
+  SELECT COUNT(*) into v_rowcount
+  FROM capture;
+
+  perform powa_log(format('%I - rowcount: %s',
+        v_funcname, v_rowcount));
+
+    IF (_srvid != 0) THEN
+        DELETE FROM powa_qualstats_src_tmp WHERE srvid = _srvid;
+    END IF;
+
+  result := true;
+
+  -- pg_qualstats metrics are not accumulated, so we force a reset after every
+  -- snapshot.  For local snapshot this is done here, remote snapshots will
+  -- rely on the collector doing it through query_cleanup.
+  IF (_srvid = 0) THEN
+    PERFORM pg_qualstats_reset();
+  END IF;
+END
+$PROC$ language plpgsql; /* end of powa_qualstats_snapshot */
diff --git a/powa--4.1.2.sql b/powa--4.1.2.sql
@@ -4029,7 +4029,7 @@ BEGIN
       WHERE s.srvid = _srvid
       AND q.queryid = s.queryid
       AND q.dbid = s.dbid
-      AND q.userid = s.dbid)
+      AND q.userid = s.userid)
   ),
   missing_quals AS (
       INSERT INTO public.powa_qualstats_quals (srvid, qualid, queryid, dbid, userid, quals)

diff --git a/sql/powa-archivist.sql b/sql/powa-archivist.sql
@@ -74,17 +74,36 @@ SELECT COUNT(*) FROM powa_servers;
 SELECT hostname FROM powa_servers WHERE id = 1;
 
 -- Check missing powa_statements FK for pg_qualstats doesn't prevent snapshot
-INSERT INTO powa_qualstats_src_tmp(srvid, ts, uniquequalnodeid, dbid, userid,
+INSERT INTO public.powa_qualstats_src_tmp(srvid, ts, uniquequalnodeid, dbid, userid,
     qualnodeid, occurences, execution_count, nbfiltered,
     mean_err_estimate_ratio, mean_err_estimate_num,
     queryid, constvalues, quals)
     SELECT 1, now(), 1, 1, 1,
         1, 1000, 1, 0,
         0, 0,
-        1, '{}', ARRAY[(1259,1,607,'i')::qual_type];
-SELECT count(*) FROM powa_qualstats_src_tmp;
-SELECT powa_qualstats_snapshot(1);
-SELECT count(*) FROM powa_qualstats_src_tmp;
+        123456789, '{}', ARRAY[(1259,1,607,'i')::qual_type];
+SELECT count(*) FROM public.powa_qualstats_src_tmp;
+SELECT public.powa_qualstats_snapshot(1);
+SELECT count(*) FROM public.powa_qualstats_src_tmp;
+SELECT count(*) FROM public.powa_qualstats_quals_history_current WHERE srvid = 1;
+
+-- Check snapshot of regular quals
+INSERT INTO public.powa_databases(srvid, oid, datname, dropped)
+    VALUES (1, 16384, 'postgres', NULL);
+INSERT INTO public.powa_statements(srvid, queryid, dbid, userid, query)
+    VALUES(1, 123456789, 16384, 10, 'query with qual');
+INSERT INTO public.powa_qualstats_src_tmp(srvid, ts, uniquequalnodeid, dbid, userid,
+    qualnodeid, occurences, execution_count, nbfiltered,
+    mean_err_estimate_ratio, mean_err_estimate_num,
+    queryid, constvalues, quals)
+    SELECT 1, now(), 1, 16384, 10,
+        1, 1000, 1, 0,
+        0, 0,
+        123456789, '{}', ARRAY[(1259,1,607,'i')::qual_type];
+SELECT count(*) FROM public.powa_qualstats_src_tmp;
+SELECT public.powa_qualstats_snapshot(1);
+SELECT count(*) FROM public.powa_qualstats_src_tmp;
+SELECT count(*) FROM public.powa_qualstats_quals_history_current WHERE srvid = 1;
 
 -- activate / deactivate extension
 SELECT * FROM public.powa_functions ORDER BY srvid, module, operation, function_name;