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"
>From b1aff66110ec54e4b58517289a18451906876ed0 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera <alvhe...@alvh.no-ip.org> Date: Mon, 26 Sep 2022 13:27:24 +0200 Subject: [PATCH v2] Fix pg_stat_statements for MERGE Author: Tatsu <bt22nakamo...@oss.nttdata.com> Reviewed-by: Julien Rouhaud <rjuju...@gmail.com> Discussion: https://postgr.es/m/d87e391694db75a038abc3b259782...@oss.nttdata.com --- .../expected/pg_stat_statements.out | 41 ++++++++++++++++++- .../sql/pg_stat_statements.sql | 22 ++++++++++ doc/src/sgml/pgstatstatements.sgml | 4 +- src/backend/nodes/nodeFuncs.c | 4 +- src/backend/utils/misc/queryjumble.c | 11 +++++ 5 files changed, 77 insertions(+), 5 deletions(-) diff --git a/contrib/pg_stat_statements/expected/pg_stat_statements.out b/contrib/pg_stat_statements/expected/pg_stat_statements.out index ff0166fb9d..9ac5c87c3a 100644 --- a/contrib/pg_stat_statements/expected/pg_stat_statements.out +++ b/contrib/pg_stat_statements/expected/pg_stat_statements.out @@ -222,12 +222,51 @@ SELECT * FROM test WHERE a IN (1, 2, 3, 4, 5); 3 | c (8 rows) +-- MERGE +MERGE INTO test USING test st ON (st.a = test.a AND st.a >= 4) + WHEN MATCHED THEN UPDATE SET b = st.b || st.a::text; +MERGE INTO test USING test st ON (st.a = test.a AND st.a >= 4) + WHEN MATCHED THEN UPDATE SET b = test.b || st.a::text; +MERGE INTO test USING test st ON (st.a = test.a AND st.a >= 4) + WHEN MATCHED AND length(st.b) > 1 THEN UPDATE SET b = test.b || st.a::text; +MERGE INTO test USING test st ON (st.a = test.a) + WHEN NOT MATCHED THEN INSERT (a, b) VALUES (0, NULL); +MERGE INTO test USING test st ON (st.a = test.a) + WHEN NOT MATCHED THEN INSERT VALUES (0, NULL); -- same as above +MERGE INTO test USING test st ON (st.a = test.a) + WHEN NOT MATCHED THEN INSERT (b, a) VALUES (NULL, 0); +MERGE INTO test USING test st ON (st.a = test.a) + WHEN NOT MATCHED THEN INSERT (a) VALUES (0); +MERGE INTO test USING test st ON (st.a = test.a AND st.a >= 4) + WHEN MATCHED THEN DELETE; +MERGE INTO test USING test st ON (st.a = test.a AND st.a >= 4) + WHEN MATCHED THEN DO NOTHING; +MERGE INTO test USING test st ON (st.a = test.a AND st.a >= 4) + WHEN NOT MATCHED THEN DO NOTHING; SELECT query, calls, rows FROM pg_stat_statements ORDER BY query COLLATE "C"; query | calls | rows ------------------------------------------------------------------------------+-------+------ DELETE FROM test WHERE a > $1 | 1 | 1 INSERT INTO test (a, b) VALUES ($1, $2), ($3, $4), ($5, $6) | 1 | 3 INSERT INTO test VALUES(generate_series($1, $2), $3) | 1 | 10 + MERGE INTO test USING test st ON (st.a = test.a AND st.a >= $1) +| 1 | 6 + WHEN MATCHED AND length(st.b) > $2 THEN UPDATE SET b = test.b || st.a::text | | + MERGE INTO test USING test st ON (st.a = test.a AND st.a >= $1) +| 1 | 6 + WHEN MATCHED THEN DELETE | | + MERGE INTO test USING test st ON (st.a = test.a AND st.a >= $1) +| 1 | 0 + WHEN MATCHED THEN DO NOTHING | | + MERGE INTO test USING test st ON (st.a = test.a AND st.a >= $1) +| 1 | 6 + WHEN MATCHED THEN UPDATE SET b = st.b || st.a::text | | + MERGE INTO test USING test st ON (st.a = test.a AND st.a >= $1) +| 1 | 6 + WHEN MATCHED THEN UPDATE SET b = test.b || st.a::text | | + MERGE INTO test USING test st ON (st.a = test.a AND st.a >= $1) +| 1 | 0 + WHEN NOT MATCHED THEN DO NOTHING | | + MERGE INTO test USING test st ON (st.a = test.a) +| 1 | 0 + WHEN NOT MATCHED THEN INSERT (a) VALUES ($1) | | + MERGE INTO test USING test st ON (st.a = test.a) +| 2 | 0 + WHEN NOT MATCHED THEN INSERT (a, b) VALUES ($1, $2) | | + MERGE INTO test USING test st ON (st.a = test.a) +| 1 | 0 + WHEN NOT MATCHED THEN INSERT (b, a) VALUES ($1, $2) | | SELECT * FROM test ORDER BY a | 1 | 12 SELECT * FROM test WHERE a > $1 ORDER BY a | 2 | 4 SELECT * FROM test WHERE a IN ($1, $2, $3, $4, $5) | 1 | 8 @@ -235,7 +274,7 @@ SELECT query, calls, rows FROM pg_stat_statements ORDER BY query COLLATE "C"; SELECT query, calls, rows FROM pg_stat_statements ORDER BY query COLLATE "C" | 0 | 0 UPDATE test SET b = $1 WHERE a = $2 | 6 | 6 UPDATE test SET b = $1 WHERE a > $2 | 1 | 3 -(10 rows) +(19 rows) -- -- INSERT, UPDATE, DELETE on test table to validate WAL generation metrics diff --git a/contrib/pg_stat_statements/sql/pg_stat_statements.sql b/contrib/pg_stat_statements/sql/pg_stat_statements.sql index a01f183727..8f5c866225 100644 --- a/contrib/pg_stat_statements/sql/pg_stat_statements.sql +++ b/contrib/pg_stat_statements/sql/pg_stat_statements.sql @@ -100,6 +100,28 @@ SELECT * FROM test ORDER BY a; -- SELECT with IN clause SELECT * FROM test WHERE a IN (1, 2, 3, 4, 5); +-- MERGE +MERGE INTO test USING test st ON (st.a = test.a AND st.a >= 4) + WHEN MATCHED THEN UPDATE SET b = st.b || st.a::text; +MERGE INTO test USING test st ON (st.a = test.a AND st.a >= 4) + WHEN MATCHED THEN UPDATE SET b = test.b || st.a::text; +MERGE INTO test USING test st ON (st.a = test.a AND st.a >= 4) + WHEN MATCHED AND length(st.b) > 1 THEN UPDATE SET b = test.b || st.a::text; +MERGE INTO test USING test st ON (st.a = test.a) + WHEN NOT MATCHED THEN INSERT (a, b) VALUES (0, NULL); +MERGE INTO test USING test st ON (st.a = test.a) + WHEN NOT MATCHED THEN INSERT VALUES (0, NULL); -- same as above +MERGE INTO test USING test st ON (st.a = test.a) + WHEN NOT MATCHED THEN INSERT (b, a) VALUES (NULL, 0); +MERGE INTO test USING test st ON (st.a = test.a) + WHEN NOT MATCHED THEN INSERT (a) VALUES (0); +MERGE INTO test USING test st ON (st.a = test.a AND st.a >= 4) + WHEN MATCHED THEN DELETE; +MERGE INTO test USING test st ON (st.a = test.a AND st.a >= 4) + WHEN MATCHED THEN DO NOTHING; +MERGE INTO test USING test st ON (st.a = test.a AND st.a >= 4) + WHEN NOT MATCHED THEN DO NOTHING; + SELECT query, calls, rows FROM pg_stat_statements ORDER BY query COLLATE "C"; -- diff --git a/doc/src/sgml/pgstatstatements.sgml b/doc/src/sgml/pgstatstatements.sgml index ecf6cd6bf3..ea90365c7f 100644 --- a/doc/src/sgml/pgstatstatements.sgml +++ b/doc/src/sgml/pgstatstatements.sgml @@ -487,7 +487,7 @@ <para> Plannable queries (that is, <command>SELECT</command>, <command>INSERT</command>, - <command>UPDATE</command>, and <command>DELETE</command>) are combined into a single + <command>UPDATE</command>, <command>DELETE</command>, and <command>MERGE</command>) are combined into a single <structname>pg_stat_statements</structname> entry whenever they have identical query structures according to an internal hash calculation. Typically, two queries will be considered the same for this purpose if they are @@ -783,7 +783,7 @@ <varname>pg_stat_statements.track_utility</varname> controls whether utility commands are tracked by the module. Utility commands are all those other than <command>SELECT</command>, <command>INSERT</command>, - <command>UPDATE</command> and <command>DELETE</command>. + <command>UPDATE</command>, <command>DELETE</command>, and <command>MERGE</command>. The default value is <literal>on</literal>. Only superusers can change this setting. </para> diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c index 724d076674..0a7b22f97e 100644 --- a/src/backend/nodes/nodeFuncs.c +++ b/src/backend/nodes/nodeFuncs.c @@ -2259,10 +2259,10 @@ expression_tree_walker_impl(Node *node, { MergeAction *action = (MergeAction *) node; - if (WALK(action->targetList)) - return true; if (WALK(action->qual)) return true; + if (WALK(action->targetList)) + return true; } break; case T_PartitionPruneStepOp: diff --git a/src/backend/utils/misc/queryjumble.c b/src/backend/utils/misc/queryjumble.c index 6e75acda27..e795115c2e 100644 --- a/src/backend/utils/misc/queryjumble.c +++ b/src/backend/utils/misc/queryjumble.c @@ -258,6 +258,7 @@ JumbleQueryInternal(JumbleState *jstate, Query *query) JumbleExpr(jstate, (Node *) query->windowClause); JumbleExpr(jstate, (Node *) query->distinctClause); JumbleExpr(jstate, (Node *) query->sortClause); + JumbleExpr(jstate, (Node *) query->mergeActionList); JumbleExpr(jstate, query->limitOffset); JumbleExpr(jstate, query->limitCount); APP_JUMB(query->limitOption); @@ -738,6 +739,16 @@ JumbleExpr(JumbleState *jstate, Node *node) JumbleExpr(jstate, (Node *) conf->exclRelTlist); } break; + case T_MergeAction: + { + MergeAction *mergeaction = (MergeAction *) node; + + APP_JUMB(mergeaction->matched); + APP_JUMB(mergeaction->commandType); + JumbleExpr(jstate, mergeaction->qual); + JumbleExpr(jstate, (Node *) mergeaction->targetList); + } + break; case T_List: foreach(temp, (List *) node) { -- 2.30.2