Differentiate MERGE queries with different structures
Hi,
pg_stat_statements module distinguishes queries with different
structures, but some visibly different MERGE queries were combined as
one pg_stat_statements entry.
For example,
MERGE INTO test1 USING test2 ON test1.id = test2.id WHEN MATCHED THEN
UPDATE var = 1;
MERGE INTO test1 USING test2 ON test1.id = test2.id WHEN MATCHED THEN
DELETE;
These two queries have different command types after WHEN (UPDATE and
DELETE), but they were regarded as one entry in pg_stat_statements.
I think that they should be sampled as distinct queries.
I attached a patch file that adds information about MERGE queries on the
documentation of pg_stat_statements, and lines of code that helps with
the calculation of queryid hash value to differentiate MERGE queries.
Any kind of feedback is appreciated.
Tatsu
Attachments:
merge_stat.patchtext/x-diff; name=merge_stat.patchDownload+12-2
Hi,
On Mon, Sep 26, 2022 at 03:12:46PM +0900, bt22nakamorit wrote:
pg_stat_statements module distinguishes queries with different structures,
but some visibly different MERGE queries were combined as one
pg_stat_statements entry.
For example,
MERGE INTO test1 USING test2 ON test1.id = test2.id WHEN MATCHED THEN UPDATE
var = 1;
MERGE INTO test1 USING test2 ON test1.id = test2.id WHEN MATCHED THEN
DELETE;
These two queries have different command types after WHEN (UPDATE and
DELETE), but they were regarded as one entry in pg_stat_statements.
I think that they should be sampled as distinct queries.
Agreed.
I attached a patch file that adds information about MERGE queries on the
documentation of pg_stat_statements, and lines of code that helps with the
calculation of queryid hash value to differentiate MERGE queries.
Any kind of feedback is appreciated.
I didn't test the patch (and never looked at MERGE implementation either), but
I'm wondering if MergeAction->matched and MergeAction->override should be
jumbled too?
Also, the patch should contain some extra tests to fully cover MERGE jumbling.
On 2022-Sep-26, Julien Rouhaud wrote:
On Mon, Sep 26, 2022 at 03:12:46PM +0900, bt22nakamorit wrote:
I attached a patch file that adds information about MERGE queries on the
documentation of pg_stat_statements, and lines of code that helps with the
calculation of queryid hash value to differentiate MERGE queries.
Any kind of feedback is appreciated.I didn't test the patch (and never looked at MERGE implementation either), but
I'm wondering if MergeAction->matched and MergeAction->override should be
jumbled too?
->matched distinguishes these two queries:
MERGE INTO foo USING bar ON (something)
WHEN MATCHED THEN DO NOTHING;
MERGE INTO foo USING bar ON (something)
WHEN NOT MATCHED THEN DO NOTHING;
because only DO NOTHING can be used with both MATCHED and NOT MATCHED,
though on the whole the distinction seems pointless. However I think if
you sent both these queries and got a single pgss entry with the text of
one of them and not the other, you're going to be confused about where
the other went. So +1 for jumbling it too.
->overriding is used in OVERRIDING SYSTEM VALUES (used for GENERATED
columns). I don't think there's any case where it's interesting
currently: if you specify the column it's going to be in the column list
(which does affect the query ID).
Also, the patch should contain some extra tests to fully cover MERGE
jumbling.
Agreed. I struggle to find a balance between not having anything and
going overboard, but I decided to add different for the different things
that should be jumbled, so that they would all appear in the view.
I moved the code around; instead of adding it at the end of the switch,
I did what the comment says, which is to mirror expression_tree_walker.
This made me realize that the latter is using the wrong order for fields
according to the struct definition, so I flipped that also.
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"La gente vulgar sólo piensa en pasar el tiempo;
el que tiene talento, en aprovecharlo"
Attachments:
v2-0001-Fix-pg_stat_statements-for-MERGE.patchtext/x-diff; charset=us-asciiDownload+77-6
Pushed, thanks.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
<Schwern> It does it in a really, really complicated way
<crab> why does it need to be complicated?
<Schwern> Because it's MakeMaker.