62 Commits (81f72115cf189b0a428d3efca2e4beb02b825111)

Author	SHA1	Message	Date
Álvaro Herrera	16edc1b94f	pg_stat_statements: Fix handling of duplicate constant locations ... Two or more constants can have the same location. We handled this correctly for non squashed constants, but failed to do it if squashed (resulting in out-of-bounds memory access), because the code structure became broken by commit 0f65f3eec478: we failed to update 'last_loc' correctly when skipping these squashed constants. The simplest fix seems to be to get rid of 'last_loc' altogether -- in hindsight, it's quite pointless. Also, when ignoring a constant because of this, make sure to fulfill fill_in_constant_lengths's duty of setting its length to -1. Lastly, we can use == instead of <= because the locations have been sorted beforehand, so the < case cannot arise. Co-authored-by: Sami Imseih <samimseih@gmail.com> Co-authored-by: Dmitry Dolgov <9erthalion6@gmail.com> Reported-by: Konstantin Knizhnik <knizhnik@garret.ru> Backpatch-through: 18 Discussion: https://www.postgresql.org/message-id/2b91e358-0d99-43f7-be44-d2d4dbce37b3%40garret.ru	2 months ago
Michael Paquier	3357471cf9	pg_stat_statements: Add counters for generic and custom plans ... This patch adds two new counters to pg_stat_statements: - generic_plan_calls - custom_plan_calls These counters track how many times a prepared statement was executed using a generic or custom plan, respectively, providing a global equivalent at query level, for top and non-top levels, of pg_prepared_statements whose data is restricted to a single session. This commit builds upon e125e36002. The module is bumped to version 1.13. PGSS_FILE_HEADER is bumped as well, something that the latest patches touching the on-disk format of the PGSS file did not actually bother with since 2022.. Author: Sami Imseih <samimseih@gmail.com> Reviewed-by: Ilia Evdokimov <ilya.evdokimov@tantorlabs.com> Reviewed-by: Andrei Lepikhov <lepihov@gmail.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Reviewed-by: Nikolay Samokhvalov <nik@postgres.ai> Discussion: https://postgr.es/m/CAA5RZ0uFw8Y9GCFvafhC=OA8NnMqVZyzXPfv_EePOt+iv1T-qQ@mail.gmail.com	5 months ago
Michael Paquier	bee23ea4dd	Show sizes of FETCH queries as constants in pg_stat_statements ... Prior to this patch, every FETCH call would generate a unique queryId with a different size specified. Depending on the workloads, this could lead to a significant bloat in pg_stat_statements, as repeatedly calling a specific cursor would result in a new queryId each time. For example, FETCH 1 c1; and FETCH 2 c1; would produce different queryIds. This patch improves the situation by normalizing the fetch size, so as semantically similar statements generate the same queryId. As a result, statements like the below, which differ syntactically but have the same effect, will now share a single queryId: FETCH FROM c1 FETCH NEXT c1 FETCH 1 c1 In order to do a normalization based on the keyword used in FETCH, FetchStmt is tweaked with a new FetchDirectionKeywords. This matters for "howMany", which could be set to a negative value depending on the direction, and we want to normalize the queries with enough information about the direction keywords provided, including RELATIVE, ABSOLUTE or all the ALL variants. Author: Sami Imseih <samimseih@gmail.com> Discussion: https://postgr.es/m/CAA5RZ0tA6LbHCg2qSS+KuM850BZC_+ZgHV7Ug6BXw22TNyF+MA@mail.gmail.com	6 months ago
Álvaro Herrera	c2da1a5d63	Make query jumbling also squash PARAM_EXTERN params ... Commit 62d712ecfd made query jumbling squash lists of Consts as a single element, but there's no reason not to treat PARAM_EXTERN parameters the same. For these purposes, these values are indeed constants for any particular execution of a query. In particular, this should make list squashing more useful for applications using extended query protocol, which would use parameters extensively. A complication arises: if a query has both external parameters and squashable lists, then the parameter number used as placeholder for the squashed list might be inconsistent with regards to the parameter numbers used by the query literal. To reduce the surprise factor, all parameters are renumbered starting from 1 in that case. Author: Sami Imseih <samimseih@gmail.com> Author: Dmitry Dolgov <9erthalion6@gmail.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/CAA5RZ0tRXoPG2y6bMgBCWNDt0Tn=unRerbzYM=oW0syi1=C1OA@mail.gmail.com	6 months ago
Álvaro Herrera	debad29d22	Improve jumble squashing through CoerceViaIO and RelabelType ... There's no principled reason for query jumbling to only remove the first layer of RelabelType and CoerceViaIO. Change it to see through as many layers as there are.	6 months ago
Álvaro Herrera	0f65f3eec4	Fix squashing algorithm for query texts ... The algorithm to squash lists of constants added by commit 62d712ecfd was a bit too simplistic; we wanted to avoid adding unnecessary complexity, but cases like direct function calls of typecasting functions (and others) were missed, and bogus SQL syntax was being shown in pg_stat_statements normalized query text field. To fix normalization for those cases, we need the parser to transmit information about were each list of constant values starts and ends, so add that to a couple of nodes. Also add a few more test cases to make sure we're doing the right thing. The patch initially submitted by Sami added a new private struct in gram.y to carry the start/end information for A_Expr, but I (Álvaro) decided that a better fix was to remove the parser indirection via the in_expr production, and instead create separate components in the a_expr rule. I'm surprised that this works and doesn't require more changes, but I assume (without checking) that the grammar used to be more complex and got simplified at some point. Bump catversion. Author: Sami Imseih <samimseih@gmail.com> Author: Dmitry Dolgov <9erthalion6@gmail.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/CAA5RZ0tRXoPG2y6bMgBCWNDt0Tn=unRerbzYM=oW0syi1=C1OA@mail.gmail.com	6 months ago
Michael Paquier	f85f6ab051	Revert support for improved tracking of nested queries ... This commit reverts the two following commits: - 499edb0974, track more precisely query locations for nested statements. - 06450c7b8c, a follow-up fix of 499edb0974 with query locations. The test introduced in this commit is not reverted. This is proving useful to track a problem that only pgaudit was able to detect. These prove to have issues with the tracking of SELECT statements, when these use multiple parenthesis which is something supported by the grammar. Incorrect location and lengths are causing pg_stat_statements to become confused, failing its job in query normalization with potential out-of-bound writes because the location and the length may not match with what can be handled. A lot of the query patterns discussed when this issue was reported have no test coverage in the main regression test suite, or the recovery test 027_stream_regress.pl would have caught the problems as pg_stat_statements is loaded by the node running the regression tests. A first step would be to improve the test coverage to stress more the query normalization logic. A different portion of this work was done in 45e0ba30fc, with the addition of tests for nested queries. These can be left in the tree. They are useful to track the way inner queries are currently tracked by PGSS with non-top-level entries, and will be useful when reconsidering in the future the work reverted here. Reported-by: Alexander Kozhemyakin <a.kozhemyakin@postgrespro.ru> Discussion: https://postgr.es/m/18947-cdd2668beffe02bf@postgresql.org	6 months ago
Michael Paquier	35a428f30b	pg_stat_statements: Fix parameter number gaps in normalized queries ... pg_stat_statements anticipates that certain constant locations may be recorded multiple times and attempts to avoid calculating a length for these locations in fill_in_constant_lengths(). However, during generate_normalized_query() where normalized query strings are generated, these locations are not excluded from consideration. This could increment the parameter number counter for every recorded occurrence at such a location, leading to an incorrect normalization in certain cases with gaps in the numbers reported. For example, take this query: SELECT WHERE '1' IN ('2'::int, '3'::int::text) Before this commit, it would be normalized like that, with gaps in the parameter numbers: SELECT WHERE $1 IN ($3::int, $4::int::text) However the correct, less confusing one should be like that: SELECT WHERE $1 IN ($2::int, $3::int::text) This commit fixes the computation of the parameter numbers to track the number of constants replaced with an $n by a separate counter instead of the iterator used to loop through the list of locations. The underlying query IDs are not changed, neither are the normalized strings for existing PGSS hash entries. New entries with fresh normalized queries would automatically get reshaped based on the new parameter numbering. Issue discovered while discussing a separate problem for HEAD, but this affects all the stable branches. Author: Sami Imseih <samimseih@gmail.com> Discussion: https://postgr.es/m/CAA5RZ0tzxvWXsacGyxrixdhy3tTTDfJQqxyFBRFh31nNHBQ5qA@mail.gmail.com Backpatch-through: 13	7 months ago
Michael Paquier	06450c7b8c	Fix regression with location calculation of nested statements ... The statement location calculated for some nested query cases was wrong when multiple queries are sent as a single string, these being separated by semicolons. As pointed by Sami Imseih, the location calculation was incorrect when the last query of nested statement with multiple queries does **NOT** finish with a semicolon for the last statement. In this case, the statement length tracked by RawStmt is 0, which is equivalent to say that the string should be used until its end. The code previously discarded this case entirely, causing the location to remain at 0, the same as pointing at the beginning of the string. This caused pg_stat_statements to store incorrect query strings. This issue has been introduced in 499edb0974. I have looked at the diffs generated by pgaudit back then, and noticed the difference generated for this nested query case, but I have missed the point that it was an actual regression with an existing case. A test case is added in pg_stat_statements to provide some coverage, restoring the pre-17 behavior for the calculation of the query locations. Special thanks to David Steele, who, through an analysis of the test diffs generated by pgaudit with the new v18 logic, has poked me about the fact that my original analysis of the matter was wrong. The test output of pg_overexplain is updated to reflect the new logic, as the new locations refer to the beginning of the argument passed to the function explain_filter(). When the module was introduced in 8d5ceb113e, which was after 499edb0974 (for the new calculation method), the locations of the test were not actually right: the plan generated for the query string given in input of the function pointed to the top-level query, not the nested one. Reported-by: David Steele <david@pgbackrest.org> Author: Michael Paquier <michael@paquier.xyz> Reviewed-by: Anthonin Bonnefoy <anthonin.bonnefoy@datadoghq.com> Reviewed-by: Jian He <jian.universality@gmail.com> Reviewed-by: Sami Imseih <samimseih@gmail.com> Reviewed-by: David Steele <david@pgbackrest.org> Discussion: https://postgr.es/m/844a3b38-bbf1-4fb2-9fd6-f58c35c09917@pgbackrest.org	7 months ago
Peter Eisentraut	09a47c68e2	Fix whitespace	8 months ago
Álvaro Herrera	9fbd53dea5	Remove the query_id_squash_values GUC ... Commit 62d712ecfd introduced the capability to calculate the same queryId for queries with different lengths of constants in a list for an IN clause. This behavior was originally enabled with a GUC query_id_squash_values. After a discussion about the value of such a GUC, it was decided to back out of the use of a GUC and make the squashing behavior the only available option. Author: Sami Imseih <samimseih@gmail.com> Discussion: https://postgr.es/m/Z-LZyygkkNyA8-kR@msg.df7cb.de Discussion: https://postgr.es/m/CA+q6zcVTK-3C-8NWV1oY2NZrvtnMCDqnyYYyk1T7WMUG65MeOQ@mail.gmail.com	9 months ago
David Rowley	f31aad9b07	Fix query jumbling to account for NULL nodes ... Previously NULL nodes were ignored. This could cause issues where the computed query ID could match for queries where fields that are next to each other in their Node struct where one field was NULL and the other non-NULL. For example, the Query struct had distinctClause and sortClause next to each other. If someone wrote; SELECT DISTINCT c1 FROM t; and then; SELECT c1 FROM t ORDER BY c1; these would produce the same query ID since, in the first query, we ignored the NULL sortClause and appended the jumble bytes for the distictClause. In the latter query, since we did nothing for the NULL distinctClause then jumble the non-NULL sortClause, and since the node representation stored is the same in both cases, the query IDs were identical. Here we fix this by always accounting for NULL nodes by recording that we saw a NULL in the jumble buffer. This fixes the issue as the order that the NULL is recorded isn't the same in the above two queries. Author: Bykov Ivan <i.bykov@modernsys.ru> Author: Michael Paquier <michael@paquier.xyz> Author: David Rowley <dgrowleyml@gmail.com> Discussion: https://postgr.es/m/aafce7966e234372b2ba876c0193f1e9%40localhost.localdomain	9 months ago
Michael Paquier	3430215fe3	pg_stat_statements: Add more tests with temp tables and namespaces ... These tests provide coverage for RangeTblEntry and how query jumbling works with search_path, as well as the case where relations are re-created, generating a different query ID as the relation OID is used in the computation. A patch is under discussion to switch to a different approach based on the relation name, and there was no test coverage for this area, including how queries are currently grouped with search_path. This is useful to track how the situation changes between HEAD and any patches proposed. Christoph has proposed the test with ON COMMIT DROP temporary tables, and I have written the second part. Author: Christoph Berg <myon@debian.org> Author: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/Z9iWXKGwkm8RAC93@msg.df7cb.de	9 months ago
Álvaro Herrera	62d712ecfd	Introduce squashing of constant lists in query jumbling ... pg_stat_statements produces multiple entries for queries like SELECT something FROM table WHERE col IN (1, 2, 3, ...) depending on the number of parameters, because every element of ArrayExpr is individually jumbled. Most of the time that's undesirable, especially if the list becomes too large. Fix this by introducing a new GUC query_id_squash_values which modifies the node jumbling code to only consider the first and last element of a list of constants, rather than each list element individually. This affects both the query_id generated by query jumbling, as well as pg_stat_statements query normalization so that it suppresses printing of the individual elements of such a list. The default value is off, meaning the previous behavior is maintained. Author: Dmitry Dolgov <9erthalion6@gmail.com> Reviewed-by: Sergey Dudoladov (mysterious, off-list) Reviewed-by: David Geier <geidav.pg@gmail.com> Reviewed-by: Robert Haas <robertmhaas@gmail.com> Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: Sami Imseih <samimseih@gmail.com> Reviewed-by: Sutou Kouhei <kou@clear-code.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Michael Paquier <michael@paquier.xyz> Reviewed-by: Marcos Pegoraro <marcos@f10.com.br> Reviewed-by: Julien Rouhaud <rjuju123@gmail.com> Reviewed-by: Zhihong Yu <zyu@yugabyte.com> Tested-by: Yasuo Honda <yasuo.honda@gmail.com> Tested-by: Sergei Kornilov <sk@zsrv.org> Tested-by: Maciek Sakrejda <m.sakrejda@gmail.com> Tested-by: Chengxi Sun <sunchengxi@highgo.com> Tested-by: Jakub Wartak <jakub.wartak@enterprisedb.com> Discussion: https://postgr.es/m/CA+q6zcWtUbT_Sxj0V6HY6EZ89uv5wuG5aefpe_9n0Jr3VwntFg@mail.gmail.com	9 months ago
David Rowley	89988ac589	Fix further fallout from EXPLAIN ANALYZE BUFFERS change ... c2a4078eb adjusted EXPLAIN ANALYZE to default the BUFFERS to ON. This (hopefully) fixes the last remaining issue with regression test failures with -D RELCACHE_FORCE_RELEASE -D CATCACHE_FORCE_RELEASE builds, where the planner accesses more buffers due to the cold caches. Discussion: https://postgr.es/m/CAApHDvqLdzgz77JsE-yTki3w9UiKQ-uTMLRctazcu+99-ips3g@mail.gmail.com	1 year ago
Michael Paquier	499edb0974	Track more precisely query locations for nested statements ... Previously, a Query generated through the transform phase would have unset stmt_location, tracking the starting point of a query string. Extensions relying on the statement location to extract its relevant parts in the source text string would fallback to use the whole statement instead, leading to confusing results like in pg_stat_statements for queries relying on nested queries, like: - EXPLAIN, with top-level and nested query using the same query string, and a query ID coming from the nested query when the non-top-level entry. - Multi-statements, with only partial portions of queries being normalized. - COPY TO with a query, SELECT or DMLs. This patch improves things by keeping track of the statement locations and propagate it to Query during transform, allowing PGSS to only show the relevant part of the query for nested query. This leads to less bloat in entries for non-top-level entries, as queries can now be grouped within the same (toplevel, queryid) duos in pg_stat_statements. The result gives a stricter one-one mapping between query IDs and its query strings. The regression tests introduced in 45e0ba30fc produce differences reflecting the new logic. Author: Anthonin Bonnefoy Reviewed-by: Michael Paquier, Jian He Discussion: https://postgr.es/m/CAO6_XqqM6S9bQ2qd=75W+yKATwoazxSNhv5sjW06fjGAtHbTUA@mail.gmail.com	1 year ago
Tom Lane	14e5680eee	Improve parser's reporting of statement start locations. ... Up to now, the parser's reporting of a statement's stmt_location included any preceding whitespace or comments. This isn't really desirable but was done to avoid accounting honestly for nonterminals that reduce to empty. It causes problems for pg_stat_statements, which partially compensates by manually stripping whitespace, but is not bright enough to strip /*-style comments. There will be more problems with an upcoming patch to improve reporting of errors in extension scripts, so it's time to do something about this. The thing we have to do to make it work right is to adjust YYLLOC_DEFAULT to scan the inputs of each production to find the first one that has a valid location (i.e., did not reduce to empty). In theory this adds a little bit of per-reduction overhead, but in practice it's negligible. I checked by measuring the time to run raw_parser() on the contents of information_schema.sql, and there was basically no change. Having done that, we can rely on any nonterminal that didn't reduce to completely empty to have a correct starting location, and we don't need the kluges the stmtmulti production formerly used. This should have a side benefit of allowing parse error reports to include an error position in some cases where they formerly failed to do so, due to trying to report the position of an empty nonterminal. I did not go looking for an example though. The one previously known case where that could happen (OptSchemaEltList) no longer needs the kluge it had; but I rather doubt that that was the only case. Discussion: https://postgr.es/m/ZvV1ClhnbJLCz7Sm@msg.df7cb.de	1 year ago
Michael Paquier	45e0ba30fc	pg_stat_statements: Add tests for nested queries with level tracking ... There have never been any regression tests in PGSS for various query patterns for nested queries combined with level tracking, like: - Multi-statements. - CREATE TABLE AS - CREATE/REFRESH MATERIALIZED VIEW - DECLARE CURSOR - EXPLAIN, with a subset of the above supported. - COPY. All the tests added here track historical, sometimes confusing, existing behaviors. For example, EXPLAIN stores two PGSS entries with the same top-level query string but two different query IDs as one is calculated for the top-level EXPLAIN (this part is right) and a second one for the inner query in the EXPLAIN (this part is not right). A couple of patches are under discussion to improve the situation, and all the tests added here will prove useful to evaluate the changes discussed. Author: Anthonin Bonnefoy Reviewed-by: Michael Paquier, Jian He Discussion: https://postgr.es/m/CAO6_XqqM6S9bQ2qd=75W+yKATwoazxSNhv5sjW06fjGAtHbTUA@mail.gmail.com	1 year ago
Michael Paquier	cf54a2c002	pg_stat_statements: Add columns to track parallel worker activity ... The view pg_stat_statements gains two columns: - parallel_workers_to_launch, the number of parallel workers planned to be launched. - parallel_workers_launched, the number of parallel workers actually launched. The ratio of both columns offers hints that parallel workers are lacking on a per-statement basis, requiring some tuning, in coordination with "calls", the number of times a query is executed. As of now, these numbers are tracked within Gather and GatherMerge nodes. They could be extended to utilities that make use of parallel workers (parallel btree and brin, VACUUM). The module is bumped to 1.12. Author: Guillaume Lelarge Discussion: https://postgr.es/m/CAECtzeWtTGOK0UgKXdDGpfTVSa5bd_VbUt6K6xn8P7X+_dZqKw@mail.gmail.com	1 year ago
Michael Paquier	ba90eac7a9	pg_stat_statements: Expand tests for SET statements ... There are many grammar flavors that depend on the parse node VariableSetStmt. This closes the gap in pg_stat_statements by providing test coverage for what should be a large majority of them, improving more the work begun in de2aca2885. This will be used to ease the evaluation of a path towards more normalization of SET queries with query jumbling. Note that SET NAMES (grammar from the standard, synonym of SET client_encoding) is omitted on purpose, this could use UTF8 with a conditional script where UTF8 is supported, but that does not seem worth the maintenance cost for the sake of these tests. The author has submitted most of these in a TAP test (filled in any holes I could spot), still queries in a SQL file of pg_stat_statements is able to achieve the same goal while being easier to look at when testing normalization patterns. Author: Greg Sabino Mullane, Michael Paquier Discussion: https://postgr.es/m/CAKAnmmJtJY2jzQN91=2QAD2eAJAA-Per61eyO48-TyxEg-q0Rg@mail.gmail.com	1 year ago
Michael Paquier	933848d16d	Add missing query ID reporting in extended query protocol ... This commit adds query ID reports for two code paths when processing extended query protocol messages: - When receiving a bind message, setting it to the first Query retrieved from a cached cache. - When receiving an execute message, setting it to the first PlannedStmt stored in a portal. An advantage of this method is that this is able to cover all the types of portals handled in the extended query protocol, particularly these two when the report done in ExecutorStart() is not enough (neither is an addition in ExecutorRun(), actually, for the second point): - Multiple execute messages, with multiple ExecutorRun(). - Portal with execute/fetch messages, like a query with a RETURNING clause and a fetch size that stores the tuples in a first execute message going though ExecutorStart() and ExecuteRun(), followed by one or more execute messages doing only fetches from the tuplestore created in the first message. This corresponds to the case where execute_is_fetch is set, for example. Note that the query ID reporting done in ExecutorStart() is still necessary, as an EXECUTE requires it. Query ID reporting is optimistic and more calls to pgstat_report_query_id() don't matter as the first report takes priority except if the report is forced. The comment in ExecutorStart() is adjusted to reflect better the reality with the extended query protocol. The test added in pg_stat_statements is a courtesy of Robert Haas. This uses psql's \bind metacommand, hence this part is backpatched down to v16. Reported-by: Kaido Vaikla, Erik Wienhold Author: Sami Imseih Reviewed-by: Jian He, Andrei Lepikhov, Michael Paquier Discussion: https://postgr.es/m/CA+427g8DiW3aZ6pOpVgkPbqK97ouBdf18VLiHFesea2jUk3XoQ@mail.gmail.com Discussion: https://postgr.es/m/CA+TgmoZxtnf_jZ=VqBSyaU8hfUkkwoJCJ6ufy4LGpXaunKrjrg@mail.gmail.com Discussion: https://postgr.es/m/1391613709.939460.1684777418070@office.mailbox.org Backpatch-through: 14	1 year ago
Michael Paquier	7b1ddbae36	pg_stat_statements: Add tests with extended query protocol ... There are currently no tests in the tree checking that queries using the extended query protocol are able to map with their query ID. This can be achieved for some paths of the extended query protocol with the psql meta-commands \bind or \bind_named, so let's add some tests based on both. I have found that to be a useful addition while working on a different issue. Discussion: https://postgr.es/m/ZuEt6MOEBSlifBfn@paquier.xyz	1 year ago
Fujii Masao	97f2bc5aa5	pg_stat_statements: Add regression test for privilege handling. ... This commit adds a regression test to verify that pg_stat_statements correctly handles privileges, improving its test coverage. Author: Keisuke Kuroda Reviewed-by: Michael Paquier, Fujii Masao Discussion: https://postgr.es/m/2224ccf2e12c41ccb81702ef3303d5ac@nttcom.co.jp	1 year ago
Michael Paquier	c145f321b6	Propagate query IDs of utility statements in functions ... For utility statements defined within a function, the query tree is copied to a PlannedStmt as utility commands do not require planning. However, the query ID was missing from the information passed down. This leads to plugins relying on the query ID like pg_stat_statements to not be able to track utility statements within function calls. Tests are added to check this behavior, depending on pg_stat_statements.track. This is an old bug. Now, query IDs for utilities are compiled using their parsed trees rather than the query string since v16 (3db72ebcbe), leading to less bloat with utilities, so backpatch down only to this version. Author: Anthonin Bonnefoy Discussion: https://postgr.es/m/CAO6_XqrGp-uwBqi3vBPLuRULKkddjC7R5QZCgsFren=8E+m2Sg@mail.gmail.com Backpatch-through: 16	1 year ago
Peter Eisentraut	1141e29b61	Revert "pg_stat_statements: Add coverage for entry_dealloc()" ... This reverts commit 742f6b3e6d. The new test failed on big-endian platforms. Discussion: https://www.postgresql.org/message-id/flat/40d1e4f2-835f-448f-a541-8ff5db75bf3d@eisentraut.org	2 years ago
Peter Eisentraut	742f6b3e6d	pg_stat_statements: Add coverage for entry_dealloc() ... This involves creating more than pg_stat_statements.max entries and checking that the limit is kept and the least used entries are kicked out. Reviewed-by: Michael Paquier <michael@paquier.xyz> Reviewed-by: Julien Rouhaud <rjuju123@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/40d1e4f2-835f-448f-a541-8ff5db75bf3d@eisentraut.org	2 years ago
Peter Eisentraut	3e527aeeed	pg_stat_statements: Add test coverage for pg_stat_statements_reset_1_7 ... Run pg_stat_statements_reset() once while the appropriate extension version is installed. Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://www.postgresql.org/message-id/flat/40d1e4f2-835f-448f-a541-8ff5db75bf3d@eisentraut.org	2 years ago
Peter Eisentraut	3727b8d0e3	pg_stat_statements: Add test coverage for pg_stat_statements_1_8() ... This requires reading pg_stat_statements at least once while the 1.8 version of the extension is installed. Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://www.postgresql.org/message-id/flat/40d1e4f2-835f-448f-a541-8ff5db75bf3d@eisentraut.org	2 years ago
Alexander Korotkov	dc9f8a7983	Track statement entry timestamp in contrib/pg_stat_statements ... This patch adds 'stats_since' and 'minmax_stats_since' columns to the pg_stat_statements view and pg_stat_statements() function. The new min/max reset mode for the pg_stat_stetments_reset() function is controlled by the parameter minmax_only. 'stat_since' column is populated with the current timestamp when a new statement is added to the pg_stat_statements hashtable. It provides clean information about statistics collection time intervals for each statement. Besides it can be used by sampling solutions to detect situations when a statement was evicted and stored again between samples. Such a sampling solution could derive any pg_stat_statements statistic values for an interval between two samples with the exception of all min/max statistics. To address this issue this patch adds the ability to reset min/max statistics independently of the statement reset using the new minmax_only parameter of the pg_stat_statements_reset(userid oid, dbid oid, queryid bigint, minmax_only boolean) function. The timestamp of such reset is stored in the minmax_stats_since field for each statement. pg_stat_statements_reset() function now returns the timestamp of a reset as the result. Discussion: https://postgr.es/m/flat/72e80e7b160a6eb189df9ef6f068cce3765d37f8.camel%40moonset.ru Author: Andrei Zubkov Reviewed-by: Julien Rouhaud, Hayato Kuroda, Yuki Seino, Chengxi Sun Reviewed-by: Anton Melnikov, Darren Rush, Michael Paquier, Sergei Kornilov Reviewed-by: Alena Rybakina, Andrei Lepikhov	2 years ago
Alexander Korotkov	6ab1dbd26b	Add NOT NULL checking of pg_stat_statements_reset() in tests ... This is preliminary patch. It adds NOT NULL checking for the result of pg_stat_statements_reset() function. It is needed for upcoming patch "Track statement entry timestamp" that will change the result type of this function to the timestamp of a reset performed. Discussion: https://postgr.es/m/flat/72e80e7b160a6eb189df9ef6f068cce3765d37f8.camel%40moonset.ru Author: Andrei Zubkov Reviewed-by: Julien Rouhaud, Hayato Kuroda, Yuki Seino, Chengxi Sun Reviewed-by: Anton Melnikov, Darren Rush, Michael Paquier, Sergei Kornilov Reviewed-by: Alena Rybakina, Andrei Lepikhov	2 years ago
Michael Paquier	108161bcb9	pg_stat_statements: Remove duplicated tests for SET statements ... This looks like a copy-paste mistake introduced in de2aca2885, that has added checks for more patterns of SET statements while ignoring the original test block that existed. Backpatch down to where this has been introduced, as this shaves some cycles. Author: Sergei Kornilov Discussion: https://postgr.es/m/5689421699428803@mail-sendbernar-production-main-46.myt.yp-c.yandex.net Backpatch-through: 16	2 years ago
Tom Lane	76db9cb636	Fix some issues with tracking nesting level in pg_stat_statements. ... When we decide that we don't want to track execution time of a specific planner or ProcessUtility call, we still have to increment the nesting depth, or we'll make the wrong determination of whether we are at top level when considering nested statements. (PREPARE and EXECUTE are exceptions, for reasons explained in the code.) Counting planner nesting depth separately from executor nesting depth was a mistake: it causes us to make the wrong determination of whether we are at top level when considering nested statements that get executed during planning (as a result of constant-folding of functions, for example). Merge those counters into one. In passing, get rid of the PGSS_HANDLED_UTILITY macro in favor of explicitly listing statement types. It seems somewhat coincidental that PREPARE and EXECUTE are handled alike in each of the places where that was used: the reasoning tends to be different for each one. Thus, the macro seems as likely to encourage future bugs as prevent them, since it's quite unclear whether any future statement type that might need special-casing here would also need the same choices at each spot. Sergei Kornilov, Julien Rouhaud, and Tom Lane, per bug #17552 from Maxim Boguk. This is pretty clearly a bug fix, but it's also a behavioral change that might surprise somebody, so no back-patch. Discussion: https://postgr.es/m/17552-213b534c56ab5d02@postgresql.org	2 years ago
Daniel Gustafsson	c5a032e518	Update oldextversions test for pg_stat_statements 1.11 ... Commit 5a3423ad8e updated pg_stat_statements to 1.11 due to added columns in the view for jit deform counters. Fixing oldextversion was however missed in that commit. This adds a test for the 1.11 version view definition. Author: Nazir Bilal Yavuz <byavuz81@gmail.com> Discussion: https://postgr.es/m/CAN55FZ2Y7c2VEg4S1KDXFcaHjyF8=KZa_rMV6WOe+KwE-KE97g@mail.gmail.com	2 years ago
Michael Paquier	11c34b342b	Show parameters of CALL as constants in pg_stat_statements ... This commit changes the query jumbling of CallStmt so as its IN/OUT parameters are able to show up as constants with a parameter symbol in pg_stat_statements, like: CALL proc1($1, $2); CALL proc2($1, $2, $3); The transformed FuncExpr is used in the query ID computation instead of the FuncCall generated by the parser, so as it is sensitive to the OID of the procedure and its list of input arguments. The output arguments are handled in a separate list in CallStmt, which is also included in the computation. Tests are added to pg_stat_statements to show how this affects CALL with IN/OUT parameters as well as overloaded functions. Like 638d42a3c5 or 31de7e60da, this improves the monitoring of workloads with a lot of CALL statements, preventing unnecessary bloat when these use different input (or event output) values. Author: Sami Imseih Discussion: https://postgr.es/m/B44FA29D-EBD0-4DD9-ABC2-16F1CB087074@amazon.com	2 years ago
Andres Freund	7369798a83	Fix tracking of temp table relation extensions as writes ... Karina figured out that I (Andres) confused BufferUsage.temp_blks_written with BufferUsage.local_blks_written in fcdda1e4b5. Tests in core PG can't easily test this, as BufferUsage is just used for EXPLAIN (ANALYZE, BUFFERS) and pg_stat_statements. Thus this commit adds tests for this to pg_stat_statements. Reported-by: Karina Litskevich <litskevichkarina@gmail.com> Author: Karina Litskevich <litskevichkarina@gmail.com> Author: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/CACiT8ibxXA6+0amGikbeFhm8B84XdQVo6D0Qfd1pQ1s8zpsnxQ@mail.gmail.com Backpatch: 16-, where fcdda1e4b5 was merged	2 years ago
Michael Paquier	bb45156f34	Show names of DEALLOCATE as constants in pg_stat_statements ... This commit switches query jumbling so as prepared statement names are treated as constants in DeallocateStmt. A boolean field is added to DeallocateStmt to make a distinction between ALL and named prepared statements, as "name" was used to make this difference before, NULL meaning DEALLOCATE ALL. Prior to this commit, DEALLOCATE was not tracked in pg_stat_statements, for the reason that it was not possible to treat its name parameter as a constant. Now that query jumbling applies to all the utility nodes, this reason does not apply anymore. Like 638d42a3c5, this can be a huge advantage for monitoring where prepared statement names are randomly generated, preventing bloat in pg_stat_statements. A couple of tests are added to track the new behavior. Author: Dagfinn Ilmari Mannsåker, Michael Paquier Reviewed-by: Julien Rouhaud Discussion: https://postgr.es/m/ZMhT9kNtJJsHw6jK@paquier.xyz	2 years ago
Michael Paquier	638d42a3c5	Show GIDs of two-phase commit commands as constants in pg_stat_statements ... This relies on the "location" field added to TransactionStmt in 31de7e6, now applied to the "gid" field used by 2PC commands. These commands are now reported like: COMMIT PREPARED $1 PREPARE TRANSACTION $1 ROLLBACK PREPARED $1 Applying constants for these commands is a huge advantage for workloads that rely a lot on 2PC commands with different GIDs. Some tests are added to track the new behavior. Reviewed-by: Julien Rouhaud Discussion: https://postgr.es/m/ZMhT9kNtJJsHw6jK@paquier.xyz	2 years ago
Michael Paquier	31de7e60da	Show savepoint names as constants in pg_stat_statements ... In pg_stat_statements, savepoint names now show up as constants with a parameter symbol, using as base query string the one added as a new entry to the PGSS hash table, leading to: RELEASE $1 ROLLBACK TO $1 SAVEPOINT $1 Applying constants to these query parts is a huge advantage for workloads that generate randomly savepoint points, like ORMs (Django is at the origin of this patch). The ODBC driver is a second layer that likes a lot savepoints, though it does not use a random naming pattern. A "location" field is added to TransactionStmt, now set only for savepoints. The savepoint name is ignored by the query jumbling. The location can be extended to other query patterns, if required, like 2PC commands. Some tests are added to pg_stat_statements for all the query patterns supported by the parser. ROLLBACK, ROLLBACK TO SAVEPOINT and ROLLBACK TRANSACTION TO SAVEPOINT have the same Node representation, so all these are equivalents. The same happens for RELEASE and RELEASE SAVEPOINT. Author: Greg Sabino Mullane Discussion: https://postgr.es/m/CAKAnmm+2s9PA4OaumwMJReWHk8qvJ_-g1WqxDRDAN1BSUfxyTw@mail.gmail.com	2 years ago
Michael Paquier	9a714b9d6e	Improve cleanup phases in regression tests of pg_stat_statements ... As shaped, two DROP ROLE queries included in "user_activity" were showing in the reports for "wal". The intention is to keep each test isolated and independent, so this is incorrect. This commit adds some calls to pg_stat_statements_reset() to clean up the statistics once each test finishes, so as there are no risks of overlap in the reports for individial scenarios. The addition in "user_activity" fixes the output of "wal". The new resets done in "level_tracking" and "utility" are added for consistency with the rest, though they do not affect the stats generated in the other tests. Oversight in d0028e3. Reported-by: Andrei Zubkov Discussion: https://postgr.es/m/7beb722dd016bf54f1c78bfd6d44a684e28da624.camel@moonset.ru	3 years ago
Michael Paquier	d0028e35a0	Refactor more the regression tests of pg_stat_statements ... This commit expands more the refactoring of the regression tests of pg_stat_statements, with tests moved out of pg_stat_statements.sql into separate files. The following file structure is now used: - select is mostly the former pg_stat_statements.sql, renamed. - dml for INSERT/UPDATE/DELETE and MERGE - user_activity, to test role-level checks and stat resets. - wal, to check the WAL generation after some queries. Like e8dbdb1, there is no change in terms of code coverage or results, and this finishes the split I was aiming for in these tests. Most of the tests used "test" of "pgss_test" as names for the tables used, these are renamed to less generic names. Reviewed-by: Bertrand Drouvot Discussion: https://postgr.es/m/Y/7Y9U/y/keAW3qH@paquier.xyz	3 years ago
Michael Paquier	de2aca2885	Expand regression tests of pg_stat_statements for utility queries ... This commit adds more coverage for utility statements so as it is possible to track down all the effects of query normalization done for all the queries that use either Const or A_Const nodes, which are the nodes where normalization makes the most sense as they apply to constants (well, most of the time, really). This set of queries is extracted from an analysis done while looking at full dumps of the regression database when applying different levels of normalization to either Const or A_Const nodes for utilities, as of a minimal set of these, for: - All relkinds (CREATE, ALTER, DROP) - Policies - Cursors - Triggers - Types - Rules - Statistics - CALL - Transaction statements (isolation level, options) - EXPLAIN - COPY Note that pg_stat_statements is not switched yet to show any normalization for utilities, still it improves the default coverage of the query jumbling code (not by as much as enabling query jumbling on the main regression test suite, though): - queryjumblefuncs.funcs.c: 36.8% => 48.5% - queryjumblefuncs.switch.c: 33.2% => 43.1% Reviewed-by: Bertrand Drouvot Discussion: https://postgr.es/m/Y+MRdEq9W9XVa2AB@paquier.xyz	3 years ago
Michael Paquier	e8dbdb15db	Refactor tests of pg_stat_statements for planning, utility and level tracking ... pg_stat_statements.sql acts as the main file for all the core tests of the module, but things have become complicated to follow over the years as some of the sub-scenarios tested in this file rely on assumptions that come from completely different areas of it, like a GUC setup or a relation created previously. For example, row tracking for CTAS/COPY was looking at the number of plans, which was not necessary, or level tracking was mixed with checks on planner counts. This commit refactors the tests of pg_stat_statements, by moving test cases out of pg_stat_statements.sql into their own file, as of: - Planning-related tests in planning.sql, for [re]plan counts and top-level handling. These depend on pg_stat_statements.track_planning. - Utilities in utility.sql (pg_stat_statements.track_utility), that includes now the tests for: -- Row tracking for CTAS, CREATE MATERIALIZED VIEW, COPY. -- Basic utility statements. -- SET statements. - Tracking level, depending on pg_stat_statements.track. This part has been looking at scenarios with DO blocks, PL functions and SQL functions. pg_stat_statements.sql (still named the same for now) still includes some checks for role-level tracking and WAL generation metrics, that ought to become independent in the long term for clarity. While on it, this switches the order of the attributes when querying pg_stat_statements, the query field becoming last. This makes much easier the tracking of changes related to normalization, as queries are the only variable-length attributes queried (unaligned mode would be one extra choice, but that reduces the checks on the other fields). Test scenarios and their results match exactly with what was happening before this commit in terms of calls, number of plans, number of rows, cached data or level tracking, so this has no effect on the coverage in terms of what is produced by the reports in the table pg_stat_statements. A follow-up patch will extend more the tests of pg_stat_statements around utilities, so this split creates a foundation for this purpose, without complicating more pg_stat_statements.sql. Reviewed-by: Bertrand Drouvot Discussion: https://postgr.es/m/Y+MRdEq9W9XVa2AB@paquier.xyz	3 years ago
Michael Paquier	9ba37b2cb6	Include values of A_Const nodes in query jumbling ... Like the implementation for node copy, write and read, this node requires a custom implementation so as the query jumbling is able to consider the correct value assigned to it, depending on its type (int, float, bool, string, bitstring). Based on a dump of pg_stat_statements from the regression database, this would confuse the query jumbling of the following queries: - SET. - COPY TO with SELECT queries. - START TRANSACTION with different isolation levels. - ALTER TABLE with default expressions. - CREATE TABLE with partition bounds. Note that there may be a long-term argument in tracking the location of such nodes so as query strings holding such nodes could be normalized, but this is left as a separate discussion. Oversight in 3db72eb. Discussion: https://postgr.es/m/Y9+HuYslMAP6yyPb@paquier.xyz	3 years ago
Michael Paquier	3db72ebcbe	Generate code for query jumbling through gen_node_support.pl ... This commit changes the query jumbling code in queryjumblefuncs.c to be generated automatically based on the information of the nodes in the headers of src/include/nodes/ by using gen_node_support.pl. This approach offers many advantages: - Support for query jumbling for all the utility statements, based on the state of their parsed Nodes and not only their query string. This will greatly ease the switch to normalize the information of some DDLs, like SET or CALL for example (this is left unchanged and should be part of a separate discussion). With this feature, the number of entries stored for utilities in pg_stat_statements is reduced (for example now "CHECKPOINT" and "checkpoint" mean the same thing with the same query ID). - Documentation of query jumbling directly in the structure definition of the nodes. Since this code has been introduced in pg_stat_statements and then moved to code, the reasons behind the choices of what should be included in the jumble are rather sparse. Note that some explanation is added for the most relevant parts, as a start. - Overall code reduction and more consistency with the other parts generating read, write and copy depending on the nodes. The query jumbling is controlled by a couple of new node attributes, documented in nodes/nodes.h: - custom_query_jumble, to mark a Node as having a custom implementation. - no_query_jumble, to ignore entirely a Node. - query_jumble_ignore, to ignore a field in a Node. - query_jumble_location, to mark a location in a Node, for normalization. This can apply only to int fields, with "location" in their name (only Const as of this commit). There should be no compatibility impact on pg_stat_statements, as the new code applies the jumbling to the same fields for each node (its regression tests have no modification, for one). Some benchmark of the query jumbling between HEAD and this commit for SELECT and DMLs has proved that this new code does not cause a performance regression, with computation times close for both methods. For utility queries, the new method is slower than the previous method of calculating a hash of the query string, though we are talking about extra ns-level changes based on what I measured, which is unnoticeable even for OLTP workloads as a query ID is calculated once per query post-parse analysis. Author: Michael Paquier Reviewed-by: Peter Eisentraut Discussion: https://postgr.es/m/Y5BHOUhX3zTH/ig6@paquier.xyz	3 years ago
Alvaro Herrera	249b0409b1	Fix pg_stat_statements for MERGE ... We weren't jumbling the merge action list, so wildly different commands would be considered to use the same query ID. Add that, mention it in the docs, and some test lines. Backpatch to 15. Author: Tatsu <bt22nakamorit@oss.nttdata.com> Reviewed-by: Julien Rouhaud <rjuju123@gmail.com> Discussion: https://postgr.es/m/d87e391694db75a038abc3b2597828e8@oss.nttdata.com	3 years ago
Robert Haas	2d7ead8526	pg_stat_statements: Fix test that assumes wal_records = rows. ... It's not very robust to assume that each inserted row will produce exactly one WAL record and that no other WAL records will be generated in the process, because for example a particular transaction could always be the one that has to extend clog. Because these tests are not run by 'make installcheck' but only by 'make check', it may be that in our current testing infrastructure this can't be hit, but it doesn't seem like a good idea to rely on that, since unrelated changes to the regression tests or the way write-ahead logging is done could easily cause it to start happening, and debugging such failures is a pain. Adjust the regression test to be less sensitive. Anton Melnikov, reviewed by Julien Rouhaud Discussion: http://postgr.es/m/1ccd00d9-1723-6b68-ae56-655aab00d406@inbox.ru	4 years ago
Michael Paquier	76cbf7edb6	pg_stat_statements: Track I/O timing for temporary file blocks ... This commit adds two new columns to pg_stat_statements, called temp_blk_read_time and temp_blk_write_time. Those columns respectively show the time spent to read and write temporary file blocks on disk, whose tracking has been added in efb0ef9. This information is available when track_io_timing is enabled, like blk_read_time and blk_write_time. pg_stat_statements is updated to version to 1.10 as an effect of the newly-added columns. Tests for the upgrade path 1.9->1.10 are added. PGSS_FILE_HEADER is bumped for the new stats file format. Author: Masahiko Sawada Reviewed-by: Georgios Kokolatos, Melanie Plageman, Julien Rouhaud, Ranier Vilela Discussion: https://postgr.es/m/CAD21AoAJgotTeP83p6HiAGDhs_9Fw9pZ2J=_tYTsiO5Ob-V5GQ@mail.gmail.com	4 years ago
Michael Paquier	2b0da0365b	pg_stat_statements: Add some tests for older versions still usable ... When the newest version is loaded, the backend would load objects from the oldest complete SQL file (here 1.4) and then update to the latest version with transition scripts (up to 1.9 currently). This provides some coverage for upgrades of pg_stat_statements, but there is no test to show how things have changed across each version. This adds a couple of tests for the upgrade paths using objects from each version supported, stressing the objects whose behaviors have changed across each version supported. Author: Erica Zhang Reviewed-by: Julien Rouhaud, Michael Paquier Discussion: https://postgr.es/m/tencent_BBA974AFF61379F2345E782FD6C55891950A@qq.com	4 years ago
Tom Lane	0806d08d46	Harden pg_stat_statements tests against CLOBBER_CACHE_ALWAYS. ... Turns out the buildfarm hasn't been testing this, which will soon change. Julien Rouhaud, per report from me Discussion: https://postgr.es/m/42557.1627229005@sss.pgh.pa.us	4 years ago
Bruce Momjian	f7a97b6ec3	Update query_id computation ... Properly fix: - the "ONLY" in FROM [ONLY] isn't hashed - the agglevelsup field in GROUPING isn't hashed - WITH TIES not being hashed (new in PG 13) - "DISTINCT" in "GROUP BY [DISTINCT]" isn't hashed (new in PG 14) Reported-by: Julien Rouhaud Discussion: https://postgr.es/m/20210425081119.ulyzxqz23ueh3wuj@nol	5 years ago