Add queryid to the output of index adviser

[tedyu]

Currently the suggestion from index adviser doesn't indicate which query would benefit from the index.

This PR adds output of queryid which can be used to look up the query from pg_stat_statements.

Here is sample output with the PR:

yugabyte=# select * from pg_qualstats_index_advisor(50);
                                             pg_qualstats_index_advisor
--------------------------------------------------------------------------------------------------------------------
 {"indexes" : ["CREATE INDEX ON public.pgqs USING btree (id)"], "unoptimised" : [], "queryid" : -4649779342992481902}
(1 row)

yugabyte=# select query from pg_stat_statements where queryid = -4649779342992481902;
                  query
-----------------------------------------
 SELECT COUNT(*) FROM pgqs WHERE id = $1
(1 row)

[tedyu]

cc @rjuju

[tedyu]

Currently I don't account for queryid's for unoptimised statements. See if the issue with UNION has been addressed. This is sample output:

yugabyte=# select * from pg_qualstats_index_advisor(50);
                                                                                                                       pg_qualstats_index_advisor

----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
--------------------------------------------
 {"indexes" : ["CREATE INDEX ON public.pgqs USING lsm (id)","CREATE INDEX ON public.pgqs USING lsm (id)","CREATE INDEX ON public.pgqs USING lsm (id)"], "unoptimised" : ["pgqs.val ~ ?"], "queryid" : [-3679505690025174459,
-4649779342992481902,-3679505690025174459]}
(1 row)

It seems some map type should be used to dedup the index -> queryid mapping. Is jsonb the only choice for the map type ?

[tedyu]

I tried using the following to dedup:

        IF NOT v_indexes @> ARRAY[v_ddl] THEN v_indexes := array_append(v_indexes, v_ddl); END IF;

        IF NOT v_queryid @> ARRAY[rec.queryid] THEN v_queryid := array_append(v_queryid, rec.queryid); END IF;

However, the second line doesn't work for bigint array, queryid column ends up being null.

[rjuju]

Currently I don't account for queryid's for unoptimised statements.

I can see that, and that's not ok. Similarly, the output isn't really unhelpful. The queryids should be associated with each indexes (or quals), not a top level attribute.

This is sample output: [...] "queryid" : [-3679505690025174459, -4649779342992481902,-3679505690025174459]}

Without the test case I can't really comment. I tried with the regression tests and as far as I can see it's broken as it reports way less queryids than different optimisable statements. I didn't check thoroughly but my first idea would be that you only reports the parent queryids and not the included ones.

It seems some map type should be used to dedup the index -> queryid mapping. Is jsonb the only choice for the map type ?

Note that this extension cannot rely on jsonb type as it's 9.5+ only, and the extension should be compatible with pg 9.4 too, so only json is used.

I went ahead and wrote that feature to get the output that I want and also take care of having a new version, upgrade script and some other cleanup. See the commit https://github.com/powa-team/pg_qualstats/commit/ac193ce7a75fadd0df13100332d8c6fe6c2797aa on the advisor_queryid branch.

For the regression tests I get this output (manually reformatted for readibility):

{
  "indexes": [
    {
      "ddl": "CREATE INDEX ON public.pgqs USING btree (id)",
      "queryids": [-1029592211154622342]
    },
    {
      "ddl": "CREATE INDEX ON public.adv USING btree (val, id1, id2, id3)",
      "queryids": [-3920764419217515443, -2089652391198357662, 7707337575053310673, 8016440963540719750]
    },
    {
      "ddl": "CREATE INDEX ON public.adv USING btree (id1)",
      "queryids": [-8974793744028755130, -5501496355956623735]
    }
  ],
  "unoptimised": {
    "quals": ["adv.val ~~* ?"],
    "queryids": [-9184309353809931755]
  }
}

which seems correct after checking the underlying query text. Does it also address your needs?

[tedyu]

pg_qualstats--2.1.0.sql is not in the master branch. I cannot apply commit ac193ce7a75fadd0df13100332d8c6fe6c2797aa directly. I saved pg_qualstats--2.1.0.sql and tried to replace the adviser function, I got:

ERROR:  syntax error at or near "@"
LINE 41:           opno, eval_type)::@extschema@.qual AS qual, queryi...
                                     ^

If I remove @extschema@, I get:

ERROR:  record type has not been registered
CONTEXT:  SQL statement "WITH pgqs AS (
...
        WHERE rownum = 1"
PL/pgSQL function pg_qualstats_index_advisor(integer,integer,text[]) line 63 at FOR over SELECT rows

[tedyu]

Is 2.1.0 going to be released soon ?

[rjuju]

As I mentioned this is for now in a dedicated branch: advisor_queryid. The commit reference was just for the advisor specific changes.

You should checkout that branch and proceed with standard build / installation steps.

[tedyu]

Please go ahead with merging advisor_queryid branch.

[rjuju]

I did check on older branches and indeed those don't support anonymous record. I will fix that first.

[rjuju]

I fixed the problem and validate that it works in 9.4. I unfortunately had to create a new type for that.

Is 2.1.0 going to be released soon ?

A new version is required for any change, especially in the SQL code. I chose 2.1.x version as the structure of the returned json in the advisor changed, so that's an API break.

Do you want to make some test first to check if that works for you?

[tedyu]

I think you can go ahead with releasing.

[rjuju]

Ok, I merged the patches on the master branch for now.