Thread: 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

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"

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.

2022-09-27 17:48 に Alvaro Herrera さんは書きました:
> Pushed, thanks.
The code and the tests went fine on my environment.
Thank you Alvaro for your help, and thank you Julien for your review!

Best,
Tatsuhiro Nakamori