postgres

Commit Graph

Author	SHA1	Message	Date
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	3 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	4 months ago
Michael Paquier	88e947136b	Fix typos and grammar in the code The large majority of these have been introduced by recent commits done in the v18 development cycle. Author: Alexander Lakhin <exclusion@gmail.com> Discussion: https://postgr.es/m/9a7763ab-5252-429d-a943-b28941e0e28b@gmail.com	5 months ago
Peter Geoghegan	a6cab6a78e	Harmonize function parameter names for Postgres 18. Make sure that function declarations use names that exactly match the corresponding names from function definitions in a few places. These inconsistencies were all introduced during Postgres 18 development. This commit was written with help from clang-tidy, by mechanically applying the same rules as similar clean-up commits (the earliest such commit was commit `035ce1fe`).	5 months ago
David Rowley	928394b664	Improve various new-to-v18 appendStringInfo calls Similar to `8461424fd`, here we adjust a few new locations which were not using the most suitable appendStringInfo* function for the intended purpose. Author: David Rowley <drowleyml@gmail.com Discussion: https://postgr.es/m/CAApHDvqJnNjueb=Eoj8K+8n0g7nj_AcPWSiCj5RNV4fDejAfqA@mail.gmail.com	5 months ago
Robert Haas	83ccc85859	pg_overexplain: Use PG_MODULE_MAGIC_EXT. I committed this contrib module just after Tom committed 55527368bd07248e91e3d37a782bf66b76f06865; adjust it to match. Author: Man Zeng <zengman@halodbtech.com> Discussion: http://postgr.es/m/174313513707.60295.16516085012903412705.pgcf@coridan.postgresql.org	6 months ago
Robert Haas	9f0c36aea0	pg_overexplain: Call previous hooks as appropriate. It makes no sense to remember the previous values of the hook variables and then never bother calling those functions. Thanks to Andrei for spotting my goof. Author: Andrei Lepikhov <lepihov@gmail.com> Discussion: http://postgr.es/m/41a344e3-ffb1-4296-8ba7-801f1e9642e5@gmail.com	6 months ago
Robert Haas	081ec08e6a	pg_overexplain: Filter out actual row count from test result. Per buildfarm, these are not stable. In particular, 1/8 is sometimes 0.12 and sometimes 0.13.	6 months ago
Robert Haas	47a1f076a7	pg_overexplain: SET jit=off when running tests. Per buildfarm.	6 months ago
Robert Haas	de65c4dade	Fix oversights in commit `8d5ceb113e` It added bogus whitespace at the end of a line in the documentation. It should not have done that. The pg_overexplain tests must SET debug_parallel_query = false, not just RESET debug_parallel_query, or we get failures on test machines that make debug_parallel_query = true the defualt.	6 months ago
Robert Haas	8d5ceb113e	pg_overexplain: Additional EXPLAIN options for debugging. There's a fair amount of information in the Plan and PlanState trees that isn't printed by any existing EXPLAIN option. This means that, when working on the planner, it's often necessary to rely on facilities such as debug_print_plan, which produce excessively voluminous output. Hence, use the new EXPLAIN extension facilities to implement EXPLAIN (DEBUG) and EXPLAIN (RANGE_TABLE) as extensions to the core EXPLAIN facility. A great deal more could be done here, and the specific choices about what to print and how are definitely arguable, but this is at least a starting point for discussion and a jumping-off point for possible future improvements. Reviewed-by: Sami Imseih <samimseih@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviweed-by: Andrei Lepikhov <lepihov@gmail.com> (who didn't like it) Discussion: http://postgr.es/m/CA+TgmoZfvQUBWQ2P8iO30jywhfEAKyNzMZSR+uc2xr9PZBw6eQ@mail.gmail.com	6 months ago

11 Commits (ab4a35b4ea9d2290b1ee5ec25f65df4c739077cb)