pgsql: Fix further fallout from EXPLAIN ANALYZE BUFFERS change
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-ip...@mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/89988ac5891b3d41725472a65e50ae4e192313aa Modified Files -- .../pg_stat_statements/expected/level_tracking.out | 24 +++--- contrib/pg_stat_statements/sql/level_tracking.sql | 8 2 files changed, 16 insertions(+), 16 deletions(-)
Re: pgsql: Enable BUFFERS with EXPLAIN ANALYZE by default
On Thu, 12 Dec 2024 at 08:13, Peter Geoghegan wrote: > FYI, I saw a harmless Cirrus CI failure which is likely tied to this > commit, affecting pg_stat_statements -- see attached diffs file. I had been holding off pushing the fix for that one until I could verify that was the last issue with a complete local make check-world run with -DRELCACHE_FORCE_RELEASE -DCATCACHE_FORCE_RELEASE. Going by my machine, that's the final failure. Thanks for the report. David
pgsql: Unmark gen_random_uuid() function leakproof.
Unmark gen_random_uuid() function leakproof. The functions without arguments don't need to be marked leakproof. This commit unmarks gen_random_uuid() leakproof for consistency with upcoming UUID generation functions. Also, this commit adds a regression test to prevent reintroducing such cases. Bump catalog version. Reported-by: Peter Eisentraut Reviewed-by: Andres Freund Discussion: https://postgr.es/m/CAD21AoBE1ePPWY1NQEgk3DkqjYzLPZwYTzCySHm0e%2B9a69PfZw%40mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/398d3e3b5b8f4c81795e07655f28036839cc0550 Modified Files -- src/include/catalog/catversion.h | 2 +- src/include/catalog/pg_proc.dat | 2 +- src/test/regress/expected/opr_sanity.out | 7 ++- src/test/regress/sql/opr_sanity.sql | 7 +++ 4 files changed, 15 insertions(+), 3 deletions(-)
pgsql: Use pg_memory_is_all_zeros() in pgstatfuncs.c.
Use pg_memory_is_all_zeros() in pgstatfuncs.c. There are a few places in this file that use memset() and memcmp() to determine whether a section of memory is all zeros. This commit modifies them to use pg_memory_is_all_zeros() instead. These aren't expected to be hot code paths, but this may optimize them a bit. Plus, this allows us to remove some variables that were only needed for the memset() and memcmp(). Author: Bertrand Drouvot Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/Z1hNubHfvMxlW6eu%40ip-10-97-1-34.eu-west-3.compute.internal Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/e8d59294282bd01bc06f2af13c79b9155024a917 Modified Files -- src/backend/utils/adt/pgstatfuncs.c | 18 ++ 1 file changed, 6 insertions(+), 12 deletions(-)
Re: pgsql: Enable BUFFERS with EXPLAIN ANALYZE by default
Le mer. 11 déc. 2024 à 20:13, Peter Geoghegan a écrit : > On Wed, Dec 11, 2024 at 4:35 AM David Rowley > wrote: > > Enable BUFFERS with EXPLAIN ANALYZE by default > > FYI, I saw a harmless Cirrus CI failure which is likely tied to this > commit, affecting pg_stat_statements -- see attached diffs file. > > I spotted this same issue on > http://cfbot.cputube.org/highlights/all.html. It looks like the > pg_stat_statements test is the only regression seen on CI. Note that > this includes test runs that took place with all of the current > cleanup commits that were pushed so far, so I'm confident that this > still needs to be fixed. > > I guess some of them could be fixed with this patch. But I have no idea how I can test it. -- Guillaume. diff --git a/contrib/pg_stat_statements/expected/level_tracking.out b/contrib/pg_stat_statements/expected/level_tracking.out index 9aee9f5010..f7381b4a41 100644 --- a/contrib/pg_stat_statements/expected/level_tracking.out +++ b/contrib/pg_stat_statements/expected/level_tracking.out @@ -919,8 +919,8 @@ EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) SELECT toplevel, calls, query FROM pg_stat_statements ORDER BY query COLLATE "C"; toplevel | calls | query ---+---+- - t| 1 | EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) + +--+---+- + t| 1 | EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF, BUFFERS OFF) + | | DECLARE foocur CURSOR FOR SELECT * FROM stats_track_tab t| 1 | EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) SELECT $1 f| 1 | SELECT $1 @@ -942,7 +942,7 @@ EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) SELECT 100; Result (actual rows=1 loops=1) (1 row) -EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) +EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF, BUFFERS OFF) DECLARE foocur CURSOR FOR SELECT * FROM stats_track_tab; QUERY PLAN - @@ -952,8 +952,8 @@ EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) SELECT toplevel, calls, query FROM pg_stat_statements ORDER BY query COLLATE "C"; toplevel | calls | query ---+---+- - t| 1 | EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) + +--+---+- + t| 1 | EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF, BUFFERS OFF) + | | DECLARE foocur CURSOR FOR SELECT * FROM stats_track_tab t| 1 | EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) SELECT $1 t| 1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t diff --git a/contrib/pg_stat_statements/sql/level_tracking.sql b/contrib/pg_stat_statements/sql/level_tracking.sql index 91ada1e938..9ff0607969 100644 --- a/contrib/pg_stat_statements/sql/level_tracking.sql +++ b/contrib/pg_stat_statements/sql/level_tracking.sql @@ -176,7 +176,7 @@ SELECT toplevel, calls, query FROM pg_stat_statements SET pg_stat_statements.track = 'all'; SELECT pg_stat_statements_reset() IS NOT NULL AS t; EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) SELECT 100; -EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) +EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF, BUFFERS OFF) DECLARE foocur CURSOR FOR SELECT * FROM stats_track_tab; SELECT toplevel, calls, query FROM pg_stat_statements ORDER BY query COLLATE "C"; @@ -185,7 +185,7 @@ SELECT toplevel, calls, query FROM pg_stat_statements SET pg_stat_statements.track = 'top'; SELECT pg_stat_statements_reset() IS NOT NULL AS t; EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) SELECT 100; -EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) +EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF, BUFFERS OFF) DECLARE foocur CURSOR FOR SELECT * FROM stats_track_tab; SELECT toplevel, calls, query FROM pg_stat_statements ORDER BY query COLLATE "C";
pgsql: Fix a memory leak in dumping functions with TRANSFORMs
Fix a memory leak in dumping functions with TRANSFORMs The gneration of the dump clause for functions with TRANSFORM calls would leak the memory for holding the result of the Oid array parsing. Fix by freeing. While in there, switch to using pg_malloc instead of palloc in order to be consistent with the rest of the file. Author: Oleg Tselebrovskiy Reviewed-by: Daniel Gustafsson Discussion: https://postgr.es/m/baf1ae4511288e5b421f41e79a3df...@postgrespro.ru Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/0e033f5b6de569e712d5f94b77878b8ffacf6397 Modified Files -- src/bin/pg_dump/pg_dump.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
Re: pgsql: Enable BUFFERS with EXPLAIN ANALYZE by default
On Wed, Dec 11, 2024 at 4:35 AM David Rowley wrote: > Enable BUFFERS with EXPLAIN ANALYZE by default FYI, I saw a harmless Cirrus CI failure which is likely tied to this commit, affecting pg_stat_statements -- see attached diffs file. I spotted this same issue on http://cfbot.cputube.org/highlights/all.html. It looks like the pg_stat_statements test is the only regression seen on CI. Note that this includes test runs that took place with all of the current cleanup commits that were pushed so far, so I'm confident that this still needs to be fixed. -- Peter Geoghegan diff -U3 /tmp/cirrus-ci-build/contrib/pg_stat_statements/expected/level_tracking.out /tmp/cirrus-ci-build/build/testrun/pg_stat_statements/regress/results/level_tracking.out --- /tmp/cirrus-ci-build/contrib/pg_stat_statements/expected/level_tracking.out 2024-12-11 18:50:58.66852 + +++ /tmp/cirrus-ci-build/build/testrun/pg_stat_statements/regress/results/level_tracking.out 2024-12-11 18:54:25.37105 + @@ -914,7 +914,9 @@ QUERY PLAN - Seq Scan on stats_track_tab (actual rows=0 loops=1) -(1 row) + Planning: + Buffers: shared hit=20 +(3 rows) SELECT toplevel, calls, query FROM pg_stat_statements ORDER BY query COLLATE "C"; @@ -947,7 +949,9 @@ QUERY PLAN - Seq Scan on stats_track_tab (actual rows=0 loops=1) -(1 row) + Planning: + Buffers: shared hit=20 +(3 rows) SELECT toplevel, calls, query FROM pg_stat_statements ORDER BY query COLLATE "C";
pgsql: Enable BUFFERS with EXPLAIN ANALYZE by default
Enable BUFFERS with EXPLAIN ANALYZE by default The topic of turning EXPLAIN's BUFFERS option on with the ANALYZE option has come up a few times over the past few years. In many ways, doing this seems like a good idea as it may be more obvious to users why a given query is running more slowly than they might expect. Also, from my own (David's) personal experience, I've seen users posting to the mailing lists with two identical plans, one slow and one fast asking why their query is sometimes slow. In many cases, this is due to additional reads. Having BUFFERS on by default may help reduce some of these questions, and if not, make it more obvious to the user before they post, or save a round-trip to the mailing list when additional I/O effort is the cause of the slowness. The general consensus is that we want BUFFERS on by default with ANALYZE. However, there were more than zero concerns raised with doing so. The primary reason against is the additional verbosity, making it harder to read large plans. Another concern was that buffer information isn't always useful so may not make sense to have it on by default. It's currently December, so let's commit this to see if anyone comes forward with a strong objection against making this change. We have over half a year remaining in the v18 cycle where we could still easily consider reverting this if someone were to come forward with a convincing enough reason as to why doing this is a bad idea. There were two patches independently submitted to achieve this goal, one by me and the other by Guillaume. This commit is a mix of both of these patches with some additional work done by me to adjust various additional places in the documentation which include EXPLAIN ANALYZE output. Author: Guillaume Lelarge, David Rowley Reviewed-by: Robert Haas, Greg Sabino Mullane, Michael Christofides Discussion: https://postgr.es/m/CANNMO++W7MM8T0KyXN3ZheXXt-uLVM3aEtZd+WNfZ=obxff...@mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/c2a4078ebad71999dd451ae7d4358be3c9290b07 Modified Files -- contrib/postgres_fdw/expected/postgres_fdw.out | 8 +-- contrib/postgres_fdw/sql/postgres_fdw.sql | 8 +-- doc/src/sgml/bloom.sgml| 16 +++-- doc/src/sgml/jit.sgml | 5 +- doc/src/sgml/perform.sgml | 44 doc/src/sgml/planstats.sgml| 18 ++--- doc/src/sgml/ref/explain.sgml | 10 ++- src/backend/commands/explain.c | 7 ++ src/test/regress/expected/brin_multi.out | 18 ++--- src/test/regress/expected/explain.out | 38 +++--- src/test/regress/expected/incremental_sort.out | 4 +- src/test/regress/expected/memoize.out | 2 +- src/test/regress/expected/merge.out| 2 +- src/test/regress/expected/partition_prune.out | 96 +- src/test/regress/expected/select.out | 2 +- src/test/regress/expected/select_into.out | 4 +- src/test/regress/expected/select_parallel.out | 6 +- src/test/regress/expected/subselect.out| 2 +- src/test/regress/expected/tidscan.out | 6 +- src/test/regress/sql/brin_multi.sql| 18 ++--- src/test/regress/sql/explain.sql | 16 ++--- src/test/regress/sql/incremental_sort.sql | 4 +- src/test/regress/sql/memoize.sql | 2 +- src/test/regress/sql/merge.sql | 2 +- src/test/regress/sql/partition_prune.sql | 96 +- src/test/regress/sql/select.sql| 2 +- src/test/regress/sql/select_into.sql | 4 +- src/test/regress/sql/select_parallel.sql | 6 +- src/test/regress/sql/subselect.sql | 2 +- src/test/regress/sql/tidscan.sql | 6 +- 30 files changed, 257 insertions(+), 197 deletions(-)
pgsql: Add missing BUFFERS OFF in regression tests, take 2
Add missing BUFFERS OFF in regression tests, take 2 Similar to 9fa1aaa65, but running with -D RELCACHE_FORCE_RELEASE and -D CATCACHE_FORCE_RELEASE yielded some additional missing places that needed BUFFERS OFF. Discussion: https://postgr.es/m/CANNMO++W7MM8T0KyXN3ZheXXt-uLVM3aEtZd+WNfZ=obxff...@mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/9df2a4b9316fae76477187bb2b64197169f5c346 Modified Files -- src/test/regress/expected/matview.out| 12 ++-- src/test/regress/expected/misc_functions.out | 2 +- src/test/regress/sql/matview.sql | 12 ++-- src/test/regress/sql/misc_functions.sql | 2 +- 4 files changed, 14 insertions(+), 14 deletions(-)
pgsql: Add missing BUFFERS OFF in select_into regression tests
Add missing BUFFERS OFF in select_into regression tests c2a4078eb adjusted EXPLAIN ANALYZE to include BUFFERS by default, but a few tests in select_into.sql neglected to add BUFFERS OFF. The failing tests seem unlikely to ever access buffers during execution, but they certainly could during planning. Per buildfarm member kestrel, tayra and calliphoridae. Discussion: https://postgr.es/m/CANNMO++W7MM8T0KyXN3ZheXXt-uLVM3aEtZd+WNfZ=obxff...@mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/9fa1aaa6525af044385dbcefe9d9abb5d57578cd Modified Files -- src/test/regress/expected/select_into.out | 4 ++-- src/test/regress/sql/select_into.sql | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)
pgsql: Add some regression tests for missing DDL patterns
Add some regression tests for missing DDL patterns The following commands gain increased coverage for some of the errors they can trigger: - ALTER TABLE .. ALTER COLUMN - CREATE DOMAIN - CREATE TYPE (LIKE) This has come up while discussing the possibility to add more information about the location of the error in such queries, and it is useful on its own as there was no coverage until now for the patterns added in this commit. Author: Jian He, Kirill Reshke Reviewed-By: Álvaro Herrera, Michael Paquier Discussion: https://postgr.es/m/CALdSSPhqfvKbDwqJaY=yeepi_aq61gmmpw88i6zh7cmg_2z...@mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/0172b4c9449e92a3988f669d9e7e9000454d16ce Modified Files -- src/test/regress/expected/alter_table.out | 7 +++ src/test/regress/expected/domain.out | 27 +++ src/test/regress/expected/float8.out | 2 ++ src/test/regress/expected/identity.out| 2 ++ src/test/regress/sql/alter_table.sql | 5 + src/test/regress/sql/domain.sql | 14 ++ src/test/regress/sql/float8.sql | 1 + src/test/regress/sql/identity.sql | 1 + 8 files changed, 59 insertions(+)
pgsql: Improve the test case from 5668a857d
Improve the test case from 5668a857d In commit 5668a857d, we fixed an issue with incorrect results in right semi joins and introduced a test case to verify the fix. The test case involves SubPlans and InitPlans, which may not be immediately apparent in relation to the issue we addressed. This patch simplifies the test case with a more straightforward query. Per discussion with Melanie Plageman. Author: Richard Guo Discussion: https://postgr.es/m/caakru_a-cip2xcxp13fmxq+t9bhlavaphtyjr94awl2mbxh...@mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/d8f335156c57f0df6ae3b1ec31e55979838eb882 Modified Files -- src/test/regress/expected/join.out | 84 ++ src/test/regress/sql/join.sql | 29 + 2 files changed, 40 insertions(+), 73 deletions(-)
pgsql: Add UUID version 7 generation function.
Add UUID version 7 generation function. This commit introduces the uuidv7() SQL function, which generates UUID version 7 as specified in RFC 9652. UUIDv7 combines a Unix timestamp in milliseconds and random bits, offering both uniqueness and sortability. In our implementation, the 12-bit sub-millisecond timestamp fraction is stored immediately after the timestamp, in the space referred to as "rand_a" in the RFC. This ensures additional monotonicity within a millisecond. The rand_a bits also function as a counter. We select a sub-millisecond timestamp so that it monotonically increases for generated UUIDs within the same backend, even when the system clock goes backward or when generating UUIDs at very high frequency. Therefore, the monotonicity of generated UUIDs is ensured within the same backend. This commit also expands the uuid_extract_timestamp() function to support UUID version 7. Additionally, an alias uuidv4() is added for the existing gen_random_uuid() SQL function to maintain consistency. Bump catalog version. Author: Andrey Borodin Reviewed-by: Sergey Prokhorenko, Przemysław Sztoch, Nikolay Samokhvalov Reviewed-by: Peter Eisentraut, Jelte Fennema-Nio, Aleksander Alekseev Reviewed-by: Masahiko Sawada, Lukas Fittl, Michael Paquier, Japin Li Reviewed-by: Marcos Pegoraro, Junwang Zhao, Stepan Neretin Reviewed-by: Daniel Vérité Discussion: https://postgr.es/m/CAAhFRxitJv%3DyoGnXUgeLB_O%2BM7J2BJAmb5jqAT9gZ3bij3uLDA%40mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/78c5e141e9c139fc2ff36a220334e4aa25e1b0eb Modified Files -- doc/src/sgml/datatype.sgml | 2 +- doc/src/sgml/func.sgml | 30 +++-- src/backend/utils/adt/uuid.c | 248 +++-- src/include/catalog/catversion.h | 2 +- src/include/catalog/pg_proc.dat| 11 +- src/test/regress/expected/uuid.out | 56 - src/test/regress/sql/uuid.sql | 28 - 7 files changed, 353 insertions(+), 24 deletions(-)
pgsql: Detect redundant GROUP BY columns using UNIQUE indexes
Detect redundant GROUP BY columns using UNIQUE indexes d4c3a156c added support that when the GROUP BY contained all of the columns belonging to a relation's PRIMARY KEY, all other columns belonging to that relation would be removed from the GROUP BY clause. That's possible because all other columns are functionally dependent on the PRIMARY KEY and those columns alone ensure the groups are distinct. Here we expand on that optimization and allow it to work for any unique indexes on the table rather than just the PRIMARY KEY index. This normally requires that all columns in the index are defined with NOT NULL, however, we can relax that requirement when the index is defined with NULLS NOT DISTINCT. When there are multiple suitable indexes to allow columns to be removed, we prefer the index with the least number of columns as this allows us to remove the highest number of GROUP BY columns. One day, we may want to revisit that decision as it may make more sense to use the narrower set of columns in terms of the width of the data types and stored/queried data. This also adjusts the code to make use of RelOptInfo.indexlist rather than looking up the catalog tables. In passing, add another short-circuit path to allow bailing out earlier in cases where it's certainly not possible to remove redundant GROUP BY columns. This early exit is now cheaper to do than when this code was originally written as 00b41463c made it cheaper to check for empty Bitmapsets. Patch originally by Zhang Mingli and later worked on by jian he, but after I (David) worked on it, there was very little of the original left. Author: Zhang Mingli, jian he, David Rowley Reviewed-by: jian he, Andrei Lepikhov Discussion: https://postgr.es/m/327990c8-b9b2-4b0c-bffb-462249f82de0%40Spark Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/bd10ec529796a13670645e6acd640c6f290df020 Modified Files -- src/backend/optimizer/plan/initsplan.c | 120 +-- src/backend/optimizer/util/plancat.c | 1 + src/include/nodes/pathnodes.h| 2 + src/test/regress/expected/aggregates.out | 67 + src/test/regress/sql/aggregates.sql | 32 + 5 files changed, 200 insertions(+), 22 deletions(-)
pgsql: Defer remove_useless_groupby_columns() work until query_planner(
Defer remove_useless_groupby_columns() work until query_planner() Traditionally, remove_useless_groupby_columns() was called during grouping_planner() directly after the call to preprocess_groupclause(). While in many ways, it made sense to populate the field and remove the functionally dependent columns from processed_groupClause at the same time, it's just that doing so had the disadvantage that remove_useless_groupby_columns() was being called before the RelOptInfos were populated for the relations mentioned in the query. Not having RelOptInfos available meant we needed to manually query the catalog tables to get the required details about the primary key constraint for the table. Here we move the remove_useless_groupby_columns() call to query_planner() and put it directly after the RelOptInfos are populated. This is fine to do as processed_groupClause still isn't final at this point as it can still be modified inside standard_qp_callback() by make_pathkeys_for_sortclauses_extended(). This commit is just a refactor and simply moves remove_useless_groupby_columns() into initsplan.c. A planned follow-up commit will adjust that function so it uses RelOptInfo instead of doing catalog lookups and also teach it how to use unique indexes as proofs to expand the cases where we can remove functionally dependent columns from the GROUP BY. Reviewed-by: Andrei Lepikhov, jian he Discussion: https://postgr.es/m/caaphdvqlezkwoebbqd0dp4y9mdkfbdbny0f3szeeqofou7z...@mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/430a5952deb3bfbfe1e2537315d44427b7c41fb1 Modified Files -- src/backend/optimizer/plan/initsplan.c | 167 - src/backend/optimizer/plan/planmain.c | 3 + src/backend/optimizer/plan/planner.c | 164 src/include/optimizer/planmain.h | 1 + 4 files changed, 170 insertions(+), 165 deletions(-)
