public inbox for [email protected]help / color / mirror / Atom feed
[PATCH 06/10] Default to LZ4.. 10+ messages / 4 participants [nested] [flat]
* [PATCH 06/10] Default to LZ4.. @ 2021-03-12 21:35 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 10+ messages in thread From: Justin Pryzby @ 2021-03-12 21:35 UTC (permalink / raw) this is meant to exercise in the CIs, and not meant to be merged --- configure | 6 ++++-- configure.ac | 4 ++-- src/backend/access/transam/xlog.c | 2 +- src/backend/utils/misc/guc.c | 2 +- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/configure b/configure index fed440adcf..8d76be00c1 100755 --- a/configure +++ b/configure @@ -1575,7 +1575,7 @@ Optional Packages: --with-system-tzdata=DIR use system time zone data in DIR --without-zlib do not use Zlib - --with-lz4 build with LZ4 support + --without-lz4 build without LZ4 support --with-gnu-ld assume the C compiler uses GNU ld [default=no] --with-ssl=LIB use LIB for SSL/TLS support (openssl) --with-openssl obsolete spelling of --with-ssl=openssl @@ -8598,7 +8598,9 @@ $as_echo "#define USE_LZ4 1" >>confdefs.h esac else - with_lz4=no + with_lz4=yes + +$as_echo "#define USE_LZ4 1" >>confdefs.h fi diff --git a/configure.ac b/configure.ac index 8c454128bb..bfcdc88be0 100644 --- a/configure.ac +++ b/configure.ac @@ -990,8 +990,8 @@ AC_SUBST(with_zlib) # LZ4 # AC_MSG_CHECKING([whether to build with LZ4 support]) -PGAC_ARG_BOOL(with, lz4, no, [build with LZ4 support], - [AC_DEFINE([USE_LZ4], 1, [Define to 1 to build with LZ4 support. (--with-lz4)])]) +PGAC_ARG_BOOL(with, lz4, yes, [build without LZ4 support], + [AC_DEFINE([USE_LZ4], 1, [Define to 1 to build without LZ4 support. (--without-lz4)])]) AC_MSG_RESULT([$with_lz4]) AC_SUBST(with_lz4) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 984ce39cc7..3657f74de9 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -99,7 +99,7 @@ bool EnableHotStandby = false; bool fullPageWrites = true; bool wal_log_hints = false; bool wal_compression = false; -int wal_compression_method = WAL_COMPRESSION_ZLIB; +int wal_compression_method = WAL_COMPRESSION_LZ4; char *wal_consistency_checking_string = NULL; bool *wal_consistency_checking = NULL; bool wal_init_zero = true; diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index c37a8313d3..52f9cd0242 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -4728,7 +4728,7 @@ static struct config_enum ConfigureNamesEnum[] = NULL }, &wal_compression_method, - WAL_COMPRESSION_ZLIB, wal_compression_options, + WAL_COMPRESSION_LZ4, wal_compression_options, NULL, NULL, NULL }, -- 2.17.0 --jozmn01XJZjDjM3N Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0007-add-wal_compression_method-zstd.patch" ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: pg_stat_statements and "IN" conditions @ 2024-03-23 07:13 Yasuo Honda <[email protected]> 0 siblings, 1 reply; 10+ messages in thread From: Yasuo Honda @ 2024-03-23 07:13 UTC (permalink / raw) To: Dmitry Dolgov <[email protected]>; +Cc: Tom Lane <[email protected]>; Peter Smith <[email protected]>; vignesh C <[email protected]>; Michael Paquier <[email protected]>; Nathan Bossart <[email protected]>; Gregory Stark (as CFM) <[email protected]>; David Geier <[email protected]>; Sergei Kornilov <[email protected]>; Alvaro Herrera <[email protected]>; Marcos Pegoraro <[email protected]>; Robert Haas <[email protected]>; Zhihong Yu <[email protected]>; David Steele <[email protected]>; pgsql-hackers; Pavel Trukhanov <[email protected]> Hi, I'm interested in this feature. It looks like these patches have some conflicts. http://cfbot.cputube.org/patch_47_2837.log Would you rebase these patches? Thanks, -- Yasuo Honda On Sat, Mar 23, 2024 at 4:11 PM Dmitry Dolgov <[email protected]> wrote: > > Oh, I see, thanks. Give me a moment, will fix this. > > Here is it. ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: pg_stat_statements and "IN" conditions @ 2024-03-23 18:20 Dmitry Dolgov <[email protected]> parent: Yasuo Honda <[email protected]> 0 siblings, 1 reply; 10+ messages in thread From: Dmitry Dolgov @ 2024-03-23 18:20 UTC (permalink / raw) To: Yasuo Honda <[email protected]>; +Cc: Tom Lane <[email protected]>; Peter Smith <[email protected]>; vignesh C <[email protected]>; Michael Paquier <[email protected]>; Nathan Bossart <[email protected]>; Gregory Stark (as CFM) <[email protected]>; David Geier <[email protected]>; Sergei Kornilov <[email protected]>; Alvaro Herrera <[email protected]>; Marcos Pegoraro <[email protected]>; Robert Haas <[email protected]>; Zhihong Yu <[email protected]>; David Steele <[email protected]>; pgsql-hackers; Pavel Trukhanov <[email protected]> > On Sat, Mar 23, 2024 at 04:13:44PM +0900, Yasuo Honda wrote: > Hi, I'm interested in this feature. It looks like these patches have > some conflicts. > > http://cfbot.cputube.org/patch_47_2837.log > > Would you rebase these patches? Sure, I can rebase, give me a moment. If you don't want to wait, there is a base commit in the patch, against which it should be applied without issues, 0eb23285a2. ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: pg_stat_statements and "IN" conditions @ 2024-03-24 14:36 Yasuo Honda <[email protected]> parent: Dmitry Dolgov <[email protected]> 0 siblings, 1 reply; 10+ messages in thread From: Yasuo Honda @ 2024-03-24 14:36 UTC (permalink / raw) To: Dmitry Dolgov <[email protected]>; +Cc: Tom Lane <[email protected]>; Peter Smith <[email protected]>; vignesh C <[email protected]>; Michael Paquier <[email protected]>; Nathan Bossart <[email protected]>; Gregory Stark (as CFM) <[email protected]>; David Geier <[email protected]>; Sergei Kornilov <[email protected]>; Alvaro Herrera <[email protected]>; Marcos Pegoraro <[email protected]>; Robert Haas <[email protected]>; Zhihong Yu <[email protected]>; David Steele <[email protected]>; pgsql-hackers; Pavel Trukhanov <[email protected]> Thanks for the information. I can apply these 4 patches from 0eb23285a2 . I tested this branch from Ruby on Rails and it gets some unexpected behavior from my point of view. Setting pg_stat_statements.query_id_const_merge_threshold = 5 does not normalize sql queries whose number of in clauses exceeds 5. Here are test steps. https://gist.github.com/yahonda/825ffccc4dcb58aa60e12ce33d25cd45#expected-behavior It would be appreciated if I can get my understanding correct. -- Yasuo Honda On Sun, Mar 24, 2024 at 3:20 AM Dmitry Dolgov <[email protected]> wrote: > Sure, I can rebase, give me a moment. If you don't want to wait, there > is a base commit in the patch, against which it should be applied > without issues, 0eb23285a2. ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: pg_stat_statements and "IN" conditions @ 2024-03-25 16:35 Dmitry Dolgov <[email protected]> parent: Yasuo Honda <[email protected]> 0 siblings, 1 reply; 10+ messages in thread From: Dmitry Dolgov @ 2024-03-25 16:35 UTC (permalink / raw) To: Yasuo Honda <[email protected]>; +Cc: Tom Lane <[email protected]>; Peter Smith <[email protected]>; vignesh C <[email protected]>; Michael Paquier <[email protected]>; Nathan Bossart <[email protected]>; Gregory Stark (as CFM) <[email protected]>; David Geier <[email protected]>; Sergei Kornilov <[email protected]>; Alvaro Herrera <[email protected]>; Marcos Pegoraro <[email protected]>; Robert Haas <[email protected]>; Zhihong Yu <[email protected]>; David Steele <[email protected]>; pgsql-hackers; Pavel Trukhanov <[email protected]> > On Sun, Mar 24, 2024 at 11:36:38PM +0900, Yasuo Honda wrote: > Thanks for the information. I can apply these 4 patches from > 0eb23285a2 . I tested this branch from Ruby on Rails and it gets some > unexpected behavior from my point of view. > Setting pg_stat_statements.query_id_const_merge_threshold = 5 does not > normalize sql queries whose number of in clauses exceeds 5. > > Here are test steps. > https://gist.github.com/yahonda/825ffccc4dcb58aa60e12ce33d25cd45#expected-behavior > > It would be appreciated if I can get my understanding correct. From what I understand out of the description this ruby script uses prepared statements, passing values as parameters, right? Unfortunately the current version of the patch doesn't handle that, it works with constants only [1]. The original incarnation of this feature was able to handle that, but the implementation was considered to be not suitable -- thus, to make some progress, it was left outside. The plan is, if everything goes fine at some point, to do a follow-up patch to handle Params and the rest. [1]: https://www.postgresql.org/message-id/20230211104707.grsicemegr7d3mgh%40erthalion.local ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: pg_stat_statements and "IN" conditions @ 2024-03-26 07:21 Yasuo Honda <[email protected]> parent: Dmitry Dolgov <[email protected]> 0 siblings, 1 reply; 10+ messages in thread From: Yasuo Honda @ 2024-03-26 07:21 UTC (permalink / raw) To: Dmitry Dolgov <[email protected]>; +Cc: Tom Lane <[email protected]>; Peter Smith <[email protected]>; vignesh C <[email protected]>; Michael Paquier <[email protected]>; Nathan Bossart <[email protected]>; Gregory Stark (as CFM) <[email protected]>; David Geier <[email protected]>; Sergei Kornilov <[email protected]>; Alvaro Herrera <[email protected]>; Marcos Pegoraro <[email protected]>; Robert Haas <[email protected]>; David Steele <[email protected]>; pgsql-hackers; Pavel Trukhanov <[email protected]> Yes. The script uses prepared statements because Ruby on Rails enables prepared statements by default for PostgreSQL databases. Then I tested this branch https://github.com/yahonda/postgres/tree/pg_stat_statements without using prepared statements as follows and all of them do not normalize in clause values. - Disabled prepared statements by setting `prepared_statements: false` https://gist.github.com/yahonda/2c2d6ac7a955886a305750eecfd07c5e - Use ruby-pg https://gist.github.com/yahonda/2f0efb11ae888d8f6b27a07e0b833fdf - Use psql https://gist.github.com/yahonda/c830379b33d66a743aef159aa03d7e49 I do not know why even if I use psql, the query column at pg_stat_sql_statement shows it is like a prepared statement "IN ($1, $2)". On Tue, Mar 26, 2024 at 1:35 AM Dmitry Dolgov <[email protected]> wrote: > From what I understand out of the description this ruby script uses > prepared statements, passing values as parameters, right? Unfortunately > the current version of the patch doesn't handle that, it works with > constants only [1]. The original incarnation of this feature was able to > handle that, but the implementation was considered to be not suitable -- > thus, to make some progress, it was left outside. ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: pg_stat_statements and "IN" conditions @ 2024-03-26 20:59 Dmitry Dolgov <[email protected]> parent: Yasuo Honda <[email protected]> 0 siblings, 1 reply; 10+ messages in thread From: Dmitry Dolgov @ 2024-03-26 20:59 UTC (permalink / raw) To: Yasuo Honda <[email protected]>; +Cc: Tom Lane <[email protected]>; Peter Smith <[email protected]>; vignesh C <[email protected]>; Michael Paquier <[email protected]>; Nathan Bossart <[email protected]>; Gregory Stark (as CFM) <[email protected]>; David Geier <[email protected]>; Sergei Kornilov <[email protected]>; Alvaro Herrera <[email protected]>; Marcos Pegoraro <[email protected]>; Robert Haas <[email protected]>; David Steele <[email protected]>; pgsql-hackers; Pavel Trukhanov <[email protected]> > On Tue, Mar 26, 2024 at 04:21:46PM +0900, Yasuo Honda wrote: > Yes. The script uses prepared statements because Ruby on Rails enables > prepared statements by default for PostgreSQL databases. > > Then I tested this branch > https://github.com/yahonda/postgres/tree/pg_stat_statements without > using prepared statements as follows and all of them do not normalize > in clause values. > > - Disabled prepared statements by setting `prepared_statements: false` > https://gist.github.com/yahonda/2c2d6ac7a955886a305750eecfd07c5e > > - Use ruby-pg > https://gist.github.com/yahonda/2f0efb11ae888d8f6b27a07e0b833fdf > > - Use psql > https://gist.github.com/yahonda/c830379b33d66a743aef159aa03d7e49 > > I do not know why even if I use psql, the query column at > pg_stat_sql_statement shows it is like a prepared statement "IN ($1, > $2)". It's a similar case: the column is defined as bigint, thus PostgreSQL has to wrap every constant expression in a function expression that converts its type to bigint. The current patch version doesn't try to reduce a FuncExpr into Const (event if the wrapped value is a Const), thus this array is not getting merged. If you replace bigint with an int, no type conversion would be required and merging logic will kick in. Again, the original version of the patch was able to handle this case, but it was stripped away to make the patch smaller in hope of moving forward. Anyway, thanks for reminding about how annoying the current handling of constant arrays can look like in practice! ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: pg_stat_statements and "IN" conditions @ 2024-03-26 23:56 Yasuo Honda <[email protected]> parent: Dmitry Dolgov <[email protected]> 0 siblings, 1 reply; 10+ messages in thread From: Yasuo Honda @ 2024-03-26 23:56 UTC (permalink / raw) To: Dmitry Dolgov <[email protected]>; +Cc: Tom Lane <[email protected]>; Peter Smith <[email protected]>; vignesh C <[email protected]>; Michael Paquier <[email protected]>; Nathan Bossart <[email protected]>; Gregory Stark (as CFM) <[email protected]>; David Geier <[email protected]>; Sergei Kornilov <[email protected]>; Alvaro Herrera <[email protected]>; Marcos Pegoraro <[email protected]>; Robert Haas <[email protected]>; David Steele <[email protected]>; pgsql-hackers; Pavel Trukhanov <[email protected]> Thanks for the useful info. Ruby on Rails uses bigint as a default data type for the primary key and prepared statements have been enabled by default for PostgreSQL. I'm looking forward to these current patches being merged as a first step and future versions of pg_stat_statements will support normalizing bigint and prepared statements. On Wed, Mar 27, 2024 at 6:00 AM Dmitry Dolgov <[email protected]> wrote: > It's a similar case: the column is defined as bigint, thus PostgreSQL > has to wrap every constant expression in a function expression that > converts its type to bigint. The current patch version doesn't try to > reduce a FuncExpr into Const (event if the wrapped value is a Const), > thus this array is not getting merged. If you replace bigint with an > int, no type conversion would be required and merging logic will kick > in. > > Again, the original version of the patch was able to handle this case, > but it was stripped away to make the patch smaller in hope of moving > forward. Anyway, thanks for reminding about how annoying the current > handling of constant arrays can look like in practice! ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: pg_stat_statements and "IN" conditions @ 2024-04-04 14:35 Dmitry Dolgov <[email protected]> parent: Yasuo Honda <[email protected]> 0 siblings, 1 reply; 10+ messages in thread From: Dmitry Dolgov @ 2024-04-04 14:35 UTC (permalink / raw) To: Yasuo Honda <[email protected]>; +Cc: Tom Lane <[email protected]>; Peter Smith <[email protected]>; vignesh C <[email protected]>; Michael Paquier <[email protected]>; Nathan Bossart <[email protected]>; Gregory Stark (as CFM) <[email protected]>; David Geier <[email protected]>; Sergei Kornilov <[email protected]>; Alvaro Herrera <[email protected]>; Marcos Pegoraro <[email protected]>; Robert Haas <[email protected]>; David Steele <[email protected]>; pgsql-hackers; Pavel Trukhanov <[email protected]> > On Wed, Mar 27, 2024 at 08:56:12AM +0900, Yasuo Honda wrote: > Thanks for the useful info. > > Ruby on Rails uses bigint as a default data type for the primary key > and prepared statements have been enabled by default for PostgreSQL. > I'm looking forward to these current patches being merged as a first > step and future versions of pg_stat_statements will support > normalizing bigint and prepared statements. Here is the rebased version. In the meantime I'm going to experiment with how to support more use cases in a way that will be more acceptable for the community. Attachments: [text/x-diff] v19-0001-Prevent-jumbling-of-every-element-in-ArrayExpr.patch (27.7K, ../../[email protected]/2-v19-0001-Prevent-jumbling-of-every-element-in-ArrayExpr.patch) download | inline diff: From 21d6e88cc9594745e3b88938be1547aa526b2a29 Mon Sep 17 00:00:00 2001 From: Dmitrii Dolgov <[email protected]> Date: Wed, 3 Apr 2024 20:02:51 +0200 Subject: [PATCH v19 1/4] Prevent jumbling of every element in ArrayExpr 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 jumbled. In certain situations it's undesirable, especially if the list becomes too large. Make an array of Const expressions contribute only the first/last elements to the jumble hash. Allow to enable this behavior via the new pg_stat_statements parameter query_id_const_merge with the default value off. Reviewed-by: Zhihong Yu, Sergey Dudoladov, Robert Haas, Tom Lane, Michael Paquier, Sergei Kornilov, Alvaro Herrera, David Geier Tested-by: Chengxi Sun --- contrib/pg_stat_statements/Makefile | 2 +- .../pg_stat_statements/expected/merging.out | 167 ++++++++++++++++++ contrib/pg_stat_statements/meson.build | 1 + .../pg_stat_statements/pg_stat_statements.c | 62 ++++++- contrib/pg_stat_statements/sql/merging.sql | 58 ++++++ doc/src/sgml/pgstatstatements.sgml | 57 +++++- src/backend/nodes/gen_node_support.pl | 21 ++- src/backend/nodes/queryjumblefuncs.c | 100 ++++++++++- src/backend/postmaster/launch_backend.c | 3 + src/backend/utils/misc/postgresql.conf.sample | 1 - src/include/nodes/primnodes.h | 2 +- src/include/nodes/queryjumble.h | 9 +- 12 files changed, 458 insertions(+), 25 deletions(-) create mode 100644 contrib/pg_stat_statements/expected/merging.out create mode 100644 contrib/pg_stat_statements/sql/merging.sql diff --git a/contrib/pg_stat_statements/Makefile b/contrib/pg_stat_statements/Makefile index 414a30856e4..03a62d685f3 100644 --- a/contrib/pg_stat_statements/Makefile +++ b/contrib/pg_stat_statements/Makefile @@ -19,7 +19,7 @@ LDFLAGS_SL += $(filter -lm, $(LIBS)) REGRESS_OPTS = --temp-config $(top_srcdir)/contrib/pg_stat_statements/pg_stat_statements.conf REGRESS = select dml cursors utility level_tracking planning \ - user_activity wal entry_timestamp cleanup oldextversions + user_activity wal entry_timestamp cleanup oldextversions merging # Disabled because these tests require "shared_preload_libraries=pg_stat_statements", # which typical installcheck users do not have (e.g. buildfarm clients). NO_INSTALLCHECK = 1 diff --git a/contrib/pg_stat_statements/expected/merging.out b/contrib/pg_stat_statements/expected/merging.out new file mode 100644 index 00000000000..1e58283afe8 --- /dev/null +++ b/contrib/pg_stat_statements/expected/merging.out @@ -0,0 +1,167 @@ +-- +-- Const merging functionality +-- +CREATE EXTENSION pg_stat_statements; +CREATE TABLE test_merge (id int, data int); +-- IN queries +-- No merging is performed, as a baseline result +SELECT pg_stat_statements_reset() IS NOT NULL AS t; + t +--- + t +(1 row) + +SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9); + id | data +----+------ +(0 rows) + +SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10); + id | data +----+------ +(0 rows) + +SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11); + id | data +----+------ +(0 rows) + +SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C"; + query | calls +-------------------------------------------------------------------------------------+------- + SELECT * FROM test_merge WHERE id IN ($1, $2, $3, $4, $5, $6, $7, $8, $9) | 1 + SELECT * FROM test_merge WHERE id IN ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) | 1 + SELECT * FROM test_merge WHERE id IN ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11) | 1 + SELECT pg_stat_statements_reset() IS NOT NULL AS t | 1 +(4 rows) + +-- Normal scenario, too many simple constants for an IN query +SET pg_stat_statements.query_id_const_merge = on; +SELECT pg_stat_statements_reset() IS NOT NULL AS t; + t +--- + t +(1 row) + +SELECT * FROM test_merge WHERE id IN (1); + id | data +----+------ +(0 rows) + +SELECT * FROM test_merge WHERE id IN (1, 2, 3); + id | data +----+------ +(0 rows) + +SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C"; + query | calls +----------------------------------------------------+------- + SELECT * FROM test_merge WHERE id IN ($1) | 1 + SELECT * FROM test_merge WHERE id IN (...) | 1 + SELECT pg_stat_statements_reset() IS NOT NULL AS t | 1 +(3 rows) + +SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9); + id | data +----+------ +(0 rows) + +SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10); + id | data +----+------ +(0 rows) + +SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11); + id | data +----+------ +(0 rows) + +SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C"; + query | calls +------------------------------------------------------------------------+------- + SELECT * FROM test_merge WHERE id IN ($1) | 1 + SELECT * FROM test_merge WHERE id IN (...) | 4 + SELECT pg_stat_statements_reset() IS NOT NULL AS t | 1 + SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 1 +(4 rows) + +-- More conditions in the query +SELECT pg_stat_statements_reset() IS NOT NULL AS t; + t +--- + t +(1 row) + +SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9) and data = 2; + id | data +----+------ +(0 rows) + +SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10) and data = 2; + id | data +----+------ +(0 rows) + +SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11) and data = 2; + id | data +----+------ +(0 rows) + +SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C"; + query | calls +----------------------------------------------------------+------- + SELECT * FROM test_merge WHERE id IN (...) and data = $3 | 3 + SELECT pg_stat_statements_reset() IS NOT NULL AS t | 1 +(2 rows) + +-- No constants simplification +SELECT pg_stat_statements_reset() IS NOT NULL AS t; + t +--- + t +(1 row) + +SELECT * FROM test_merge WHERE id IN (1 + 1, 2 + 2, 3 + 3, 4 + 4, 5 + 5, 6 + 6, 7 + 7, 8 + 8, 9 + 9); + id | data +----+------ +(0 rows) + +SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C"; + query | calls +---------------------------------------------------------------------------------------------------------------------------------+------- + SELECT * FROM test_merge WHERE id IN ($1 + $2, $3 + $4, $5 + $6, $7 + $8, $9 + $10, $11 + $12, $13 + $14, $15 + $16, $17 + $18) | 1 + SELECT pg_stat_statements_reset() IS NOT NULL AS t | 1 +(2 rows) + +-- Numeric type +CREATE TABLE test_merge_numeric (id int, data numeric(5, 2)); +SELECT pg_stat_statements_reset() IS NOT NULL AS t; + t +--- + t +(1 row) + +SELECT * FROM test_merge_numeric WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11); + id | data +----+------ +(0 rows) + +SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C"; + query | calls +----------------------------------------------------+------- + SELECT * FROM test_merge_numeric WHERE id IN (...) | 1 + SELECT pg_stat_statements_reset() IS NOT NULL AS t | 1 +(2 rows) + +-- Test constants evaluation, verifies a tricky part to make sure there are no +-- issues in the merging implementation +WITH cte AS ( + SELECT 'const' as const FROM test_merge +) +SELECT ARRAY['a', 'b', 'c', const::varchar] AS result +FROM cte; + result +-------- +(0 rows) + +RESET pg_stat_statements.query_id_const_merge; diff --git a/contrib/pg_stat_statements/meson.build b/contrib/pg_stat_statements/meson.build index 9bfc9657e1a..6bd62f92b9a 100644 --- a/contrib/pg_stat_statements/meson.build +++ b/contrib/pg_stat_statements/meson.build @@ -52,6 +52,7 @@ tests += { 'entry_timestamp', 'cleanup', 'oldextversions', + 'merging', ], 'regress_args': ['--temp-config', files('pg_stat_statements.conf')], # Disabled because these tests require diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c index 67cec865ba1..d7841b51cc9 100644 --- a/contrib/pg_stat_statements/pg_stat_statements.c +++ b/contrib/pg_stat_statements/pg_stat_statements.c @@ -265,6 +265,9 @@ static ExecutorFinish_hook_type prev_ExecutorFinish = NULL; static ExecutorEnd_hook_type prev_ExecutorEnd = NULL; static ProcessUtility_hook_type prev_ProcessUtility = NULL; +/* An assign hook to keep query_id_const_merge in sync */ +static void pgss_query_id_const_merge_assign_hook(bool newvalue, void *extra); + /* Links to shared memory state */ static pgssSharedState *pgss = NULL; static HTAB *pgss_hash = NULL; @@ -292,7 +295,8 @@ static bool pgss_track_utility = true; /* whether to track utility commands */ static bool pgss_track_planning = false; /* whether to track planning * duration */ static bool pgss_save = true; /* whether to save stats across shutdown */ - +static bool pgss_query_id_const_merge = false; /* request constants merging + * when computing query_id */ #define pgss_enabled(level) \ (!IsParallelWorker() && \ @@ -455,8 +459,21 @@ _PG_init(void) NULL, NULL); + DefineCustomBoolVariable("pg_stat_statements.query_id_const_merge", + "Whether to merge constants in a list when computing query_id.", + NULL, + &pgss_query_id_const_merge, + false, + PGC_SUSET, + 0, + NULL, + pgss_query_id_const_merge_assign_hook, + NULL); + MarkGUCPrefixReserved("pg_stat_statements"); + SetQueryIdConstMerge(pgss_query_id_const_merge); + /* * Install hooks. */ @@ -2808,6 +2825,9 @@ generate_normalized_query(JumbleState *jstate, const char *query, n_quer_loc = 0, /* Normalized query byte location */ last_off = 0, /* Offset from start for previous tok */ last_tok_len = 0; /* Length (in bytes) of that tok */ + bool skip = false; /* Signals that certain constants are + merged together and have to be skipped */ + /* * Get constants' lengths (core system only gives us locations). Note @@ -2831,7 +2851,6 @@ generate_normalized_query(JumbleState *jstate, const char *query, { int off, /* Offset from start for cur tok */ tok_len; /* Length (in bytes) of that tok */ - off = jstate->clocations[i].location; /* Adjust recorded location if we're dealing with partial string */ off -= query_loc; @@ -2846,12 +2865,32 @@ generate_normalized_query(JumbleState *jstate, const char *query, len_to_wrt -= last_tok_len; Assert(len_to_wrt >= 0); - memcpy(norm_query + n_quer_loc, query + quer_loc, len_to_wrt); - n_quer_loc += len_to_wrt; - /* And insert a param symbol in place of the constant token */ - n_quer_loc += sprintf(norm_query + n_quer_loc, "$%d", - i + 1 + jstate->highest_extern_param_id); + /* Normal path, non merged constant */ + if (!jstate->clocations[i].merged) + { + memcpy(norm_query + n_quer_loc, query + quer_loc, len_to_wrt); + n_quer_loc += len_to_wrt; + + /* And insert a param symbol in place of the constant token */ + n_quer_loc += sprintf(norm_query + n_quer_loc, "$%d", + i + 1 + jstate->highest_extern_param_id); + + /* In case previous constants were merged away, stop doing that */ + if (skip) + skip = false; + } + /* The firsts merged constant */ + else if (!skip) + { + memcpy(norm_query + n_quer_loc, query + quer_loc, len_to_wrt); + n_quer_loc += len_to_wrt; + + /* Skip the following until a non merged constant appear */ + skip = true; + n_quer_loc += sprintf(norm_query + n_quer_loc, "..."); + } + /* Otherwise the constant is merged away */ quer_loc = off + tok_len; last_off = off; @@ -3010,3 +3049,12 @@ comp_location(const void *a, const void *b) return pg_cmp_s32(l, r); } + +/* + * Notify query jumbling about query_id_const_merge status + */ +static void +pgss_query_id_const_merge_assign_hook(bool newvalue, void *extra) +{ + SetQueryIdConstMerge(newvalue); +} diff --git a/contrib/pg_stat_statements/sql/merging.sql b/contrib/pg_stat_statements/sql/merging.sql new file mode 100644 index 00000000000..71985bb1cd9 --- /dev/null +++ b/contrib/pg_stat_statements/sql/merging.sql @@ -0,0 +1,58 @@ +-- +-- Const merging functionality +-- +CREATE EXTENSION pg_stat_statements; + +CREATE TABLE test_merge (id int, data int); + +-- IN queries + +-- No merging is performed, as a baseline result +SELECT pg_stat_statements_reset() IS NOT NULL AS t; +SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9); +SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10); +SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11); +SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C"; + +-- Normal scenario, too many simple constants for an IN query +SET pg_stat_statements.query_id_const_merge = on; + +SELECT pg_stat_statements_reset() IS NOT NULL AS t; +SELECT * FROM test_merge WHERE id IN (1); +SELECT * FROM test_merge WHERE id IN (1, 2, 3); +SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C"; + +SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9); +SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10); +SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11); +SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C"; + +-- More conditions in the query +SELECT pg_stat_statements_reset() IS NOT NULL AS t; + +SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9) and data = 2; +SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10) and data = 2; +SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11) and data = 2; +SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C"; + +-- No constants simplification +SELECT pg_stat_statements_reset() IS NOT NULL AS t; + +SELECT * FROM test_merge WHERE id IN (1 + 1, 2 + 2, 3 + 3, 4 + 4, 5 + 5, 6 + 6, 7 + 7, 8 + 8, 9 + 9); +SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C"; + +-- Numeric type +CREATE TABLE test_merge_numeric (id int, data numeric(5, 2)); +SELECT pg_stat_statements_reset() IS NOT NULL AS t; +SELECT * FROM test_merge_numeric WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11); +SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C"; + +-- Test constants evaluation, verifies a tricky part to make sure there are no +-- issues in the merging implementation +WITH cte AS ( + SELECT 'const' as const FROM test_merge +) +SELECT ARRAY['a', 'b', 'c', const::varchar] AS result +FROM cte; + +RESET pg_stat_statements.query_id_const_merge; diff --git a/doc/src/sgml/pgstatstatements.sgml b/doc/src/sgml/pgstatstatements.sgml index 44dd4db7ce7..2b4c142f647 100644 --- a/doc/src/sgml/pgstatstatements.sgml +++ b/doc/src/sgml/pgstatstatements.sgml @@ -603,11 +603,29 @@ <para> In some cases, queries with visibly different texts might get merged into a - single <structname>pg_stat_statements</structname> entry. Normally this will happen - only for semantically equivalent queries, but there is a small chance of - hash collisions causing unrelated queries to be merged into one entry. - (This cannot happen for queries belonging to different users or databases, - however.) + single <structname>pg_stat_statements</structname> entry. Normally this + will happen only for semantically equivalent queries, or if + <varname>pg_stat_statements.query_id_const_merge</varname> is enabled and + the only difference between queries is the length of an array with constants + they contain: + +<screen> +=# SET query_id_const_merge = on; +=# SELECT pg_stat_statements_reset(); +=# SELECT * FROM test WHERE a IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11); +=# SELECT * FROM test WHERE a IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12); +=# SELECT query, calls FROM pg_stat_statements; +-[ RECORD 1 ]------------------------------ +query | SELECT * FROM test WHERE a IN (...) +calls | 2 +-[ RECORD 2 ]------------------------------ +query | SELECT pg_stat_statements_reset() +calls | 1 +</screen> + + But there is a small chance of hash collisions causing unrelated queries to + be merged into one entry. (This cannot happen for queries belonging to + different users or databases, however.) </para> <para> @@ -932,6 +950,35 @@ </para> </listitem> </varlistentry> + + <varlistentry> + <term> + <varname>pg_stat_statements.query_id_const_merge</varname> (<type>bool</type>) + <indexterm> + <primary><varname>pg_stat_statements.query_id_const_merge</varname> configuration parameter</primary> + </indexterm> + </term> + + <listitem> + <para> + Specifies how an array of constants (e.g. for an "IN" clause) + contributes to the query identifier computation. Normally every element + of an array contributes to the query identifier, which means the same + query will get multiple different identifiers, one for each occurrence + with an array of different lenght. + + If this parameter is on, an array of constants will contribute only the + first and the last elements to the query identifier. It means two + occurences of the same query, where the only difference is number of + constants in the array, are going to get the same query identifier. + Such queries are represented in form <literal>'(...)'</literal>. + + The parameter could be used to reduce amount of repeating data stored + via <structname>pg_stat_statements</structname>. The default value is off. + </para> + </listitem> + </varlistentry> + </variablelist> <para> diff --git a/src/backend/nodes/gen_node_support.pl b/src/backend/nodes/gen_node_support.pl index d4244facbb3..021073151b2 100644 --- a/src/backend/nodes/gen_node_support.pl +++ b/src/backend/nodes/gen_node_support.pl @@ -475,6 +475,7 @@ foreach my $infile (@ARGV) equal_ignore_if_zero query_jumble_ignore query_jumble_location + query_jumble_merge read_write_ignore write_only_relids write_only_nondefault_pathtarget @@ -1282,6 +1283,7 @@ _jumble${n}(JumbleState *jstate, Node *node) my @a = @{ $node_type_info{$n}->{field_attrs}{$f} }; my $query_jumble_ignore = $struct_no_query_jumble; my $query_jumble_location = 0; + my $query_jumble_merge = 0; # extract per-field attributes foreach my $a (@a) @@ -1294,21 +1296,34 @@ _jumble${n}(JumbleState *jstate, Node *node) { $query_jumble_location = 1; } + elsif ($a eq 'query_jumble_merge') + { + $query_jumble_merge = 1; + } } # node type if (($t =~ /^(\w+)\*$/ or $t =~ /^struct\s+(\w+)\*$/) and elem $1, @node_types) { - print $jff "\tJUMBLE_NODE($f);\n" - unless $query_jumble_ignore; + # Merge constants if requested. + if ($query_jumble_merge) + { + print $jff "\tJUMBLE_ELEMENTS($f);\n" + unless $query_jumble_ignore; + } + else + { + print $jff "\tJUMBLE_NODE($f);\n" + unless $query_jumble_ignore; + } } elsif ($t eq 'ParseLoc') { # Track the node's location only if directly requested. if ($query_jumble_location) { - print $jff "\tJUMBLE_LOCATION($f);\n" + print $jff "\tJUMBLE_LOCATION($f, false);\n" unless $query_jumble_ignore; } } diff --git a/src/backend/nodes/queryjumblefuncs.c b/src/backend/nodes/queryjumblefuncs.c index be823a7f8fa..e9473def361 100644 --- a/src/backend/nodes/queryjumblefuncs.c +++ b/src/backend/nodes/queryjumblefuncs.c @@ -42,6 +42,9 @@ /* GUC parameters */ int compute_query_id = COMPUTE_QUERY_ID_AUTO; +/* Whether to merge constants in a list when computing query_id */ +bool query_id_const_merge = false; + /* * True when compute_query_id is ON or AUTO, and a module requests them. * @@ -53,8 +56,10 @@ bool query_id_enabled = false; static void AppendJumble(JumbleState *jstate, const unsigned char *item, Size size); -static void RecordConstLocation(JumbleState *jstate, int location); +static void RecordConstLocation(JumbleState *jstate, + int location, bool merged); static void _jumbleNode(JumbleState *jstate, Node *node); +static void _jumbleElements(JumbleState *jstate, List *elements); static void _jumbleA_Const(JumbleState *jstate, Node *node); static void _jumbleList(JumbleState *jstate, Node *node); static void _jumbleRangeTblEntry(JumbleState *jstate, Node *node); @@ -154,6 +159,18 @@ EnableQueryId(void) query_id_enabled = true; } +/* + * Controls constants merging for query identifier computation. + * + * Third-party plugins can use this function to enable/disable merging + * of constants in a list when query identifier is computed. + */ +void +SetQueryIdConstMerge(bool value) +{ + query_id_const_merge = value; +} + /* * AppendJumble: Append a value that is substantive in a given query to * the current jumble. @@ -192,11 +209,15 @@ AppendJumble(JumbleState *jstate, const unsigned char *item, Size size) } /* - * Record location of constant within query string of query tree - * that is currently being walked. + * Record location of constant within query string of query tree that is + * currently being walked. + * + * Merged argument signals that the constant represents the first or the last + * element in a series of merged constants, and everything but the first/last + * element contributes nothing to the jumble hash. */ static void -RecordConstLocation(JumbleState *jstate, int location) +RecordConstLocation(JumbleState *jstate, int location, bool merged) { /* -1 indicates unknown or undefined location */ if (location >= 0) @@ -212,15 +233,67 @@ RecordConstLocation(JumbleState *jstate, int location) } jstate->clocations[jstate->clocations_count].location = location; /* initialize lengths to -1 to simplify third-party module usage */ + jstate->clocations[jstate->clocations_count].merged = merged; jstate->clocations[jstate->clocations_count].length = -1; jstate->clocations_count++; } } +/* + * Verify if the provided list contains could be merged down, which means it + * contains only constant expressions. + * + * Return value indicates if merging is possible. + * + * Note that this function searches only for explicit Const nodes and does not + * try to simplify expressions. + */ +static bool +IsMergeableConstList(List *elements, Const **firstConst, Const **lastConst) +{ + ListCell *temp; + Node *firstExpr = NULL; + + if (elements == NULL) + return false; + + if (!query_id_const_merge) + { + /* Merging is disabled, process everything one by one */ + return false; + } + + firstExpr = linitial(elements); + + /* + * If the first expression is a constant, verify if the following elements + * are constants as well. If yes, the list is eligible for merging, and the + * order of magnitude need to be calculated. + */ + if (IsA(firstExpr, Const)) + { + foreach(temp, elements) + if (!IsA(lfirst(temp), Const)) + return false; + + *firstConst = (Const *) firstExpr; + *lastConst = llast_node(Const, elements); + return true; + } + + /* + * If we end up here, it means no constants merging is possible, process + * the list as usual. + */ + return false; +} + #define JUMBLE_NODE(item) \ _jumbleNode(jstate, (Node *) expr->item) -#define JUMBLE_LOCATION(location) \ - RecordConstLocation(jstate, expr->location) +#define JUMBLE_ELEMENTS(list) \ + _jumbleElements(jstate, (List *) expr->list) +#define JUMBLE_LOCATION(location, merged) \ + RecordConstLocation(jstate, expr->location, merged) #define JUMBLE_FIELD(item) \ AppendJumble(jstate, (const unsigned char *) &(expr->item), sizeof(expr->item)) #define JUMBLE_FIELD_SINGLE(item) \ @@ -233,6 +306,21 @@ do { \ #include "queryjumblefuncs.funcs.c" +static void +_jumbleElements(JumbleState *jstate, List *elements) +{ + Const *first, *last; + if(IsMergeableConstList(elements, &first, &last)) + { + RecordConstLocation(jstate, first->location, true); + RecordConstLocation(jstate, last->location, true); + } + else + { + _jumbleNode(jstate, (Node *) elements); + } +} + static void _jumbleNode(JumbleState *jstate, Node *node) { diff --git a/src/backend/postmaster/launch_backend.c b/src/backend/postmaster/launch_backend.c index cb0c3e2f8ab..5bb2f00c6ef 100644 --- a/src/backend/postmaster/launch_backend.c +++ b/src/backend/postmaster/launch_backend.c @@ -122,6 +122,7 @@ typedef struct bool redirection_done; bool IsBinaryUpgrade; bool query_id_enabled; + bool query_id_const_merge; int max_safe_fds; int MaxBackends; #ifdef WIN32 @@ -742,6 +743,7 @@ save_backend_variables(BackendParameters *param, ClientSocket *client_sock, param->redirection_done = redirection_done; param->IsBinaryUpgrade = IsBinaryUpgrade; param->query_id_enabled = query_id_enabled; + param->query_id_const_merge = query_id_const_merge; param->max_safe_fds = max_safe_fds; param->MaxBackends = MaxBackends; @@ -1000,6 +1002,7 @@ restore_backend_variables(BackendParameters *param) redirection_done = param->redirection_done; IsBinaryUpgrade = param->IsBinaryUpgrade; query_id_enabled = param->query_id_enabled; + query_id_const_merge = param->query_id_const_merge; max_safe_fds = param->max_safe_fds; MaxBackends = param->MaxBackends; diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index baecde28410..cdb5bea780d 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -651,7 +651,6 @@ #log_planner_stats = off #log_executor_stats = off - #------------------------------------------------------------------------------ # AUTOVACUUM #------------------------------------------------------------------------------ diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h index aa727e722cc..cf4f900d4ed 100644 --- a/src/include/nodes/primnodes.h +++ b/src/include/nodes/primnodes.h @@ -1333,7 +1333,7 @@ typedef struct ArrayExpr /* common type of array elements */ Oid element_typeid pg_node_attr(query_jumble_ignore); /* the array elements or sub-arrays */ - List *elements; + List *elements pg_node_attr(query_jumble_merge); /* true if elements are sub-arrays */ bool multidims pg_node_attr(query_jumble_ignore); /* token location, or -1 if unknown */ diff --git a/src/include/nodes/queryjumble.h b/src/include/nodes/queryjumble.h index f1c55c8067f..8daf0725d7f 100644 --- a/src/include/nodes/queryjumble.h +++ b/src/include/nodes/queryjumble.h @@ -23,6 +23,12 @@ typedef struct LocationLen { int location; /* start offset in query text */ int length; /* length in bytes, or -1 to ignore */ + + /* + * Indicates the constant represents the beginning or the end of a merged + * constants interval. + */ + bool merged; } LocationLen; /* @@ -62,12 +68,13 @@ enum ComputeQueryIdType /* GUC parameters */ extern PGDLLIMPORT int compute_query_id; - extern const char *CleanQuerytext(const char *query, int *location, int *len); extern JumbleState *JumbleQuery(Query *query); extern void EnableQueryId(void); +extern void SetQueryIdConstMerge(bool value); extern PGDLLIMPORT bool query_id_enabled; +extern PGDLLIMPORT bool query_id_const_merge; /* * Returns whether query identifier computation has been enabled, either base-commit: c9920a9068eac2e6c8fb34988d18c0b42b9bf811 -- 2.41.0 [text/x-diff] v19-0002-Reusable-decimalLength-functions.patch (4.7K, ../../[email protected]/3-v19-0002-Reusable-decimalLength-functions.patch) download | inline diff: From 439c3c3dbbf59d7b3ed204178de9c34a60734756 Mon Sep 17 00:00:00 2001 From: Dmitrii Dolgov <[email protected]> Date: Wed, 3 Apr 2024 20:03:08 +0200 Subject: [PATCH v19 2/4] Reusable decimalLength functions Move out decimalLength functions to reuse in the following patch. --- src/backend/utils/adt/numutils.c | 48 +---------------------- src/include/utils/numutils.h | 67 ++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 47 deletions(-) create mode 100644 src/include/utils/numutils.h diff --git a/src/backend/utils/adt/numutils.c b/src/backend/utils/adt/numutils.c index adc1e8a4cba..adfad9f89fa 100644 --- a/src/backend/utils/adt/numutils.c +++ b/src/backend/utils/adt/numutils.c @@ -20,6 +20,7 @@ #include "port/pg_bitutils.h" #include "utils/builtins.h" +#include "utils/numutils.h" /* * A table of all two-digit numbers. This is used to speed up decimal digit @@ -37,53 +38,6 @@ static const char DIGIT_TABLE[200] = "80" "81" "82" "83" "84" "85" "86" "87" "88" "89" "90" "91" "92" "93" "94" "95" "96" "97" "98" "99"; -/* - * Adapted from http://graphics.stanford.edu/~seander/bithacks.html#IntegerLog10 - */ -static inline int -decimalLength32(const uint32 v) -{ - int t; - static const uint32 PowersOfTen[] = { - 1, 10, 100, - 1000, 10000, 100000, - 1000000, 10000000, 100000000, - 1000000000 - }; - - /* - * Compute base-10 logarithm by dividing the base-2 logarithm by a - * good-enough approximation of the base-2 logarithm of 10 - */ - t = (pg_leftmost_one_pos32(v) + 1) * 1233 / 4096; - return t + (v >= PowersOfTen[t]); -} - -static inline int -decimalLength64(const uint64 v) -{ - int t; - static const uint64 PowersOfTen[] = { - UINT64CONST(1), UINT64CONST(10), - UINT64CONST(100), UINT64CONST(1000), - UINT64CONST(10000), UINT64CONST(100000), - UINT64CONST(1000000), UINT64CONST(10000000), - UINT64CONST(100000000), UINT64CONST(1000000000), - UINT64CONST(10000000000), UINT64CONST(100000000000), - UINT64CONST(1000000000000), UINT64CONST(10000000000000), - UINT64CONST(100000000000000), UINT64CONST(1000000000000000), - UINT64CONST(10000000000000000), UINT64CONST(100000000000000000), - UINT64CONST(1000000000000000000), UINT64CONST(10000000000000000000) - }; - - /* - * Compute base-10 logarithm by dividing the base-2 logarithm by a - * good-enough approximation of the base-2 logarithm of 10 - */ - t = (pg_leftmost_one_pos64(v) + 1) * 1233 / 4096; - return t + (v >= PowersOfTen[t]); -} - static const int8 hexlookup[128] = { -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, diff --git a/src/include/utils/numutils.h b/src/include/utils/numutils.h new file mode 100644 index 00000000000..876e64f2df9 --- /dev/null +++ b/src/include/utils/numutils.h @@ -0,0 +1,67 @@ +/*------------------------------------------------------------------------- + * + * numutils.h + * Decimal length functions for numutils.c + * + * + * Portions Copyright (c) 1996-2023, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * src/include/utils/numutils.h + * + *------------------------------------------------------------------------- + */ +#ifndef NUMUTILS_H +#define NUMUTILS_H + +#include "common/int.h" +#include "port/pg_bitutils.h" + +/* + * Adapted from http://graphics.stanford.edu/~seander/bithacks.html#IntegerLog10 + */ +static inline int +decimalLength32(const uint32 v) +{ + int t; + static const uint32 PowersOfTen[] = { + 1, 10, 100, + 1000, 10000, 100000, + 1000000, 10000000, 100000000, + 1000000000 + }; + + /* + * Compute base-10 logarithm by dividing the base-2 logarithm by a + * good-enough approximation of the base-2 logarithm of 10 + */ + t = (pg_leftmost_one_pos32(v) + 1) * 1233 / 4096; + return t + (v >= PowersOfTen[t]); +} + +static inline int +decimalLength64(const uint64 v) +{ + int t; + static const uint64 PowersOfTen[] = { + UINT64CONST(1), UINT64CONST(10), + UINT64CONST(100), UINT64CONST(1000), + UINT64CONST(10000), UINT64CONST(100000), + UINT64CONST(1000000), UINT64CONST(10000000), + UINT64CONST(100000000), UINT64CONST(1000000000), + UINT64CONST(10000000000), UINT64CONST(100000000000), + UINT64CONST(1000000000000), UINT64CONST(10000000000000), + UINT64CONST(100000000000000), UINT64CONST(1000000000000000), + UINT64CONST(10000000000000000), UINT64CONST(100000000000000000), + UINT64CONST(1000000000000000000), UINT64CONST(10000000000000000000) + }; + + /* + * Compute base-10 logarithm by dividing the base-2 logarithm by a + * good-enough approximation of the base-2 logarithm of 10 + */ + t = (pg_leftmost_one_pos64(v) + 1) * 1233 / 4096; + return t + (v >= PowersOfTen[t]); +} + +#endif /* NUMUTILS_H */ -- 2.41.0 [text/x-diff] v19-0003-Merge-constants-in-ArrayExpr-into-groups.patch (17.3K, ../../[email protected]/4-v19-0003-Merge-constants-in-ArrayExpr-into-groups.patch) download | inline diff: From 401184675e25ef7f314cd9ac7c6986d4e18148d9 Mon Sep 17 00:00:00 2001 From: Dmitrii Dolgov <[email protected]> Date: Mon, 22 Jan 2024 21:20:45 +0100 Subject: [PATCH v19 3/4] Merge constants in ArrayExpr into groups Using query_id_const_merge only first/last element in an ArrayExpr will be used to compute query id. Extend this to take into account number of elements, and merge constants into groups based on it. Resulting groups are powers of 10, i.e. 1 to 9, 10 to 99, etc. --- .../pg_stat_statements/expected/merging.out | 84 +++++++++++++++---- .../pg_stat_statements/pg_stat_statements.c | 17 +++- contrib/pg_stat_statements/sql/merging.sql | 13 +++ doc/src/sgml/pgstatstatements.sgml | 11 +-- src/backend/nodes/queryjumblefuncs.c | 52 ++++++++---- src/include/nodes/queryjumble.h | 7 +- 6 files changed, 142 insertions(+), 42 deletions(-) diff --git a/contrib/pg_stat_statements/expected/merging.out b/contrib/pg_stat_statements/expected/merging.out index 1e58283afe8..0cb4f67b8b7 100644 --- a/contrib/pg_stat_statements/expected/merging.out +++ b/contrib/pg_stat_statements/expected/merging.out @@ -54,11 +54,11 @@ SELECT * FROM test_merge WHERE id IN (1, 2, 3); (0 rows) SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C"; - query | calls -----------------------------------------------------+------- - SELECT * FROM test_merge WHERE id IN ($1) | 1 - SELECT * FROM test_merge WHERE id IN (...) | 1 - SELECT pg_stat_statements_reset() IS NOT NULL AS t | 1 + query | calls +----------------------------------------------------------+------- + SELECT * FROM test_merge WHERE id IN ($1) | 1 + SELECT * FROM test_merge WHERE id IN (... [1-9 entries]) | 1 + SELECT pg_stat_statements_reset() IS NOT NULL AS t | 1 (3 rows) SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9); @@ -80,7 +80,60 @@ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C"; query | calls ------------------------------------------------------------------------+------- SELECT * FROM test_merge WHERE id IN ($1) | 1 - SELECT * FROM test_merge WHERE id IN (...) | 4 + SELECT * FROM test_merge WHERE id IN (... [1-9 entries]) | 2 + SELECT * FROM test_merge WHERE id IN (... [10-99 entries]) | 2 + SELECT pg_stat_statements_reset() IS NOT NULL AS t | 1 + SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 1 +(5 rows) + +-- Second order of magnitude, brace yourself +SELECT pg_stat_statements_reset() IS NOT NULL AS t; + t +--- + t +(1 row) + +SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99, 100, 101, 102, 103, 104, 105, 106, 107, 108, 109, 110); + id | data +----+------ +(0 rows) + +SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C"; + query | calls +--------------------------------------------------------------+------- + SELECT * FROM test_merge WHERE id IN (... [100-999 entries]) | 1 + SELECT pg_stat_statements_reset() IS NOT NULL AS t | 1 +(2 rows) + +-- With gaps on the threshold +SELECT pg_stat_statements_reset() IS NOT NULL AS t; + t +--- + t +(1 row) + +SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4); + id | data +----+------ +(0 rows) + +SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C"; + query | calls +----------------------------------------------------------+------- + SELECT * FROM test_merge WHERE id IN (... [1-9 entries]) | 1 + SELECT pg_stat_statements_reset() IS NOT NULL AS t | 1 +(2 rows) + +SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15); + id | data +----+------ +(0 rows) + +SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C"; + query | calls +------------------------------------------------------------------------+------- + SELECT * FROM test_merge WHERE id IN (... [1-9 entries]) | 1 + SELECT * FROM test_merge WHERE id IN (... [10-99 entries]) | 1 SELECT pg_stat_statements_reset() IS NOT NULL AS t | 1 SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 1 (4 rows) @@ -108,11 +161,12 @@ SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11) and dat (0 rows) SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C"; - query | calls -----------------------------------------------------------+------- - SELECT * FROM test_merge WHERE id IN (...) and data = $3 | 3 - SELECT pg_stat_statements_reset() IS NOT NULL AS t | 1 -(2 rows) + query | calls +--------------------------------------------------------------------------+------- + SELECT * FROM test_merge WHERE id IN (... [1-9 entries]) and data = $3 | 1 + SELECT * FROM test_merge WHERE id IN (... [10-99 entries]) and data = $3 | 2 + SELECT pg_stat_statements_reset() IS NOT NULL AS t | 1 +(3 rows) -- No constants simplification SELECT pg_stat_statements_reset() IS NOT NULL AS t; @@ -147,10 +201,10 @@ SELECT * FROM test_merge_numeric WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11) (0 rows) SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C"; - query | calls -----------------------------------------------------+------- - SELECT * FROM test_merge_numeric WHERE id IN (...) | 1 - SELECT pg_stat_statements_reset() IS NOT NULL AS t | 1 + query | calls +--------------------------------------------------------------------+------- + SELECT * FROM test_merge_numeric WHERE id IN (... [10-99 entries]) | 1 + SELECT pg_stat_statements_reset() IS NOT NULL AS t | 1 (2 rows) -- Test constants evaluation, verifies a tricky part to make sure there are no diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c index d7841b51cc9..00eec30feb1 100644 --- a/contrib/pg_stat_statements/pg_stat_statements.c +++ b/contrib/pg_stat_statements/pg_stat_statements.c @@ -2827,6 +2827,8 @@ generate_normalized_query(JumbleState *jstate, const char *query, last_tok_len = 0; /* Length (in bytes) of that tok */ bool skip = false; /* Signals that certain constants are merged together and have to be skipped */ + int magnitude; /* Order of magnitute for number of merged + constants */ /* @@ -2867,7 +2869,8 @@ generate_normalized_query(JumbleState *jstate, const char *query, Assert(len_to_wrt >= 0); /* Normal path, non merged constant */ - if (!jstate->clocations[i].merged) + magnitude = jstate->clocations[i].magnitude; + if (magnitude == 0) { memcpy(norm_query + n_quer_loc, query + quer_loc, len_to_wrt); n_quer_loc += len_to_wrt; @@ -2883,12 +2886,22 @@ generate_normalized_query(JumbleState *jstate, const char *query, /* The firsts merged constant */ else if (!skip) { + static const uint32 powers_of_ten[] = { + 1, 10, 100, + 1000, 10000, 100000, + 1000000, 10000000, 100000000, + 1000000000 + }; + int lower_merged = powers_of_ten[magnitude - 1]; + int upper_merged = powers_of_ten[magnitude]; + memcpy(norm_query + n_quer_loc, query + quer_loc, len_to_wrt); n_quer_loc += len_to_wrt; /* Skip the following until a non merged constant appear */ skip = true; - n_quer_loc += sprintf(norm_query + n_quer_loc, "..."); + n_quer_loc += sprintf(norm_query + n_quer_loc, "... [%d-%d entries]", + lower_merged, upper_merged - 1); } /* Otherwise the constant is merged away */ diff --git a/contrib/pg_stat_statements/sql/merging.sql b/contrib/pg_stat_statements/sql/merging.sql index 71985bb1cd9..657044faded 100644 --- a/contrib/pg_stat_statements/sql/merging.sql +++ b/contrib/pg_stat_statements/sql/merging.sql @@ -27,6 +27,19 @@ SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10); SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11); SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C"; +-- Second order of magnitude, brace yourself +SELECT pg_stat_statements_reset() IS NOT NULL AS t; +SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99, 100, 101, 102, 103, 104, 105, 106, 107, 108, 109, 110); +SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C"; + +-- With gaps on the threshold +SELECT pg_stat_statements_reset() IS NOT NULL AS t; +SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4); +SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C"; + +SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15); +SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C"; + -- More conditions in the query SELECT pg_stat_statements_reset() IS NOT NULL AS t; diff --git a/doc/src/sgml/pgstatstatements.sgml b/doc/src/sgml/pgstatstatements.sgml index 2b4c142f647..bac029430be 100644 --- a/doc/src/sgml/pgstatstatements.sgml +++ b/doc/src/sgml/pgstatstatements.sgml @@ -616,7 +616,7 @@ =# SELECT * FROM test WHERE a IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12); =# SELECT query, calls FROM pg_stat_statements; -[ RECORD 1 ]------------------------------ -query | SELECT * FROM test WHERE a IN (...) +query | SELECT * FROM test WHERE a IN (... [10-99 entries]) calls | 2 -[ RECORD 2 ]------------------------------ query | SELECT pg_stat_statements_reset() @@ -968,10 +968,11 @@ calls | 1 with an array of different lenght. If this parameter is on, an array of constants will contribute only the - first and the last elements to the query identifier. It means two - occurences of the same query, where the only difference is number of - constants in the array, are going to get the same query identifier. - Such queries are represented in form <literal>'(...)'</literal>. + first element, the last element and the number of elements to the query + identifier. It means two occurences of the same query, where the only + difference is number of constants in the array, are going to get the + same query identifier if the arrays are of similar length. + Such queries are represented in form <literal>'(... [10-99 entries])'</literal>. The parameter could be used to reduce amount of repeating data stored via <structname>pg_stat_statements</structname>. The default value is off. diff --git a/src/backend/nodes/queryjumblefuncs.c b/src/backend/nodes/queryjumblefuncs.c index e9473def361..a27202cb279 100644 --- a/src/backend/nodes/queryjumblefuncs.c +++ b/src/backend/nodes/queryjumblefuncs.c @@ -37,6 +37,8 @@ #include "nodes/queryjumble.h" #include "parser/scansup.h" +#include "utils/numutils.h" + #define JUMBLE_SIZE 1024 /* query serialization buffer size */ /* GUC parameters */ @@ -57,7 +59,7 @@ bool query_id_enabled = false; static void AppendJumble(JumbleState *jstate, const unsigned char *item, Size size); static void RecordConstLocation(JumbleState *jstate, - int location, bool merged); + int location, int magnitude); static void _jumbleNode(JumbleState *jstate, Node *node); static void _jumbleElements(JumbleState *jstate, List *elements); static void _jumbleA_Const(JumbleState *jstate, Node *node); @@ -212,12 +214,15 @@ AppendJumble(JumbleState *jstate, const unsigned char *item, Size size) * Record location of constant within query string of query tree that is * currently being walked. * - * Merged argument signals that the constant represents the first or the last - * element in a series of merged constants, and everything but the first/last - * element contributes nothing to the jumble hash. + * Magnitude argument larger than zero signals that the constant represents the + * first or the last element in a series of merged constants, and everything + * but such first/last element will contribute nothing to the jumble hash. The + * magnitute value specifies order of magnitute (i.e. how many digits it has) + * for the number of elements in the series, to represent the fact of merging + * later on. */ static void -RecordConstLocation(JumbleState *jstate, int location, bool merged) +RecordConstLocation(JumbleState *jstate, int location, int magnitude) { /* -1 indicates unknown or undefined location */ if (location >= 0) @@ -233,7 +238,7 @@ RecordConstLocation(JumbleState *jstate, int location, bool merged) } jstate->clocations[jstate->clocations_count].location = location; /* initialize lengths to -1 to simplify third-party module usage */ - jstate->clocations[jstate->clocations_count].merged = merged; + jstate->clocations[jstate->clocations_count].magnitude = magnitude; jstate->clocations[jstate->clocations_count].length = -1; jstate->clocations_count++; } @@ -243,24 +248,26 @@ RecordConstLocation(JumbleState *jstate, int location, bool merged) * Verify if the provided list contains could be merged down, which means it * contains only constant expressions. * - * Return value indicates if merging is possible. + * Return value is the order of magnitude (i.e. how many digits it has) for + * length of the list (to use for representation purposes later on) if merging + * is possible, otherwise zero. * * Note that this function searches only for explicit Const nodes and does not * try to simplify expressions. */ -static bool +static int IsMergeableConstList(List *elements, Const **firstConst, Const **lastConst) { ListCell *temp; Node *firstExpr = NULL; if (elements == NULL) - return false; + return 0; if (!query_id_const_merge) { /* Merging is disabled, process everything one by one */ - return false; + return 0; } firstExpr = linitial(elements); @@ -274,26 +281,26 @@ IsMergeableConstList(List *elements, Const **firstConst, Const **lastConst) { foreach(temp, elements) if (!IsA(lfirst(temp), Const)) - return false; + return 0; *firstConst = (Const *) firstExpr; *lastConst = llast_node(Const, elements); - return true; + return decimalLength32(elements->length); } /* * If we end up here, it means no constants merging is possible, process * the list as usual. */ - return false; + return 0; } #define JUMBLE_NODE(item) \ _jumbleNode(jstate, (Node *) expr->item) #define JUMBLE_ELEMENTS(list) \ _jumbleElements(jstate, (List *) expr->list) -#define JUMBLE_LOCATION(location, merged) \ - RecordConstLocation(jstate, expr->location, merged) +#define JUMBLE_LOCATION(location, magnitude) \ + RecordConstLocation(jstate, expr->location, magnitude) #define JUMBLE_FIELD(item) \ AppendJumble(jstate, (const unsigned char *) &(expr->item), sizeof(expr->item)) #define JUMBLE_FIELD_SINGLE(item) \ @@ -310,10 +317,19 @@ static void _jumbleElements(JumbleState *jstate, List *elements) { Const *first, *last; - if(IsMergeableConstList(elements, &first, &last)) + int magnitude = IsMergeableConstList(elements, &first, &last); + + if (magnitude) { - RecordConstLocation(jstate, first->location, true); - RecordConstLocation(jstate, last->location, true); + RecordConstLocation(jstate, first->location, magnitude); + RecordConstLocation(jstate, last->location, magnitude); + + /* + * After merging constants down we end up with only two constants, the + * first and the last one. To distinguish the order of magnitute behind + * merged constants, add its value into the jumble. + */ + JUMBLE_FIELD_SINGLE(magnitude); } else { diff --git a/src/include/nodes/queryjumble.h b/src/include/nodes/queryjumble.h index 8daf0725d7f..551555494e0 100644 --- a/src/include/nodes/queryjumble.h +++ b/src/include/nodes/queryjumble.h @@ -26,9 +26,12 @@ typedef struct LocationLen /* * Indicates the constant represents the beginning or the end of a merged - * constants interval. + * constants interval. The value shows how many constants were merged away + * (up to a power of 10), or in other words the order of manitude for + * number of merged constants (i.e. how many digits it has). Otherwise the + * value is 0, indicating that no merging was performed. */ - bool merged; + int magnitude; } LocationLen; /* -- 2.41.0 [text/x-diff] v19-0004-Introduce-query_id_const_merge_threshold.patch (14.9K, ../../[email protected]/5-v19-0004-Introduce-query_id_const_merge_threshold.patch) download | inline diff: From 751999d19472f3427eb8f232ba4501eb8ac1f4d3 Mon Sep 17 00:00:00 2001 From: Dmitrii Dolgov <[email protected]> Date: Wed, 3 Apr 2024 20:03:45 +0200 Subject: [PATCH v19 4/4] Introduce query_id_const_merge_threshold Replace query_id_const_merge with a threshold to allow merging only if the number of elements is larger than specified value, which could be configured using pg_stat_statements parameter query_id_const_merge_threshold. --- .../pg_stat_statements/expected/merging.out | 68 ++++++++++++++++++- .../pg_stat_statements/pg_stat_statements.c | 36 +++++----- contrib/pg_stat_statements/sql/merging.sql | 21 +++++- doc/src/sgml/pgstatstatements.sgml | 23 ++++--- src/backend/nodes/queryjumblefuncs.c | 23 +++++-- src/backend/postmaster/launch_backend.c | 6 +- src/include/nodes/queryjumble.h | 4 +- 7 files changed, 137 insertions(+), 44 deletions(-) diff --git a/contrib/pg_stat_statements/expected/merging.out b/contrib/pg_stat_statements/expected/merging.out index 0cb4f67b8b7..552e248ff14 100644 --- a/contrib/pg_stat_statements/expected/merging.out +++ b/contrib/pg_stat_statements/expected/merging.out @@ -36,7 +36,7 @@ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C"; (4 rows) -- Normal scenario, too many simple constants for an IN query -SET pg_stat_statements.query_id_const_merge = on; +SET pg_stat_statements.query_id_const_merge_threshold = 1; SELECT pg_stat_statements_reset() IS NOT NULL AS t; t --- @@ -218,4 +218,68 @@ FROM cte; -------- (0 rows) -RESET pg_stat_statements.query_id_const_merge; +-- With the threshold +SET pg_stat_statements.query_id_const_merge_threshold = 10; +SELECT pg_stat_statements_reset() IS NOT NULL AS t; + t +--- + t +(1 row) + +SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9); + id | data +----+------ +(0 rows) + +SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10); + id | data +----+------ +(0 rows) + +SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11); + id | data +----+------ +(0 rows) + +SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C"; + query | calls +---------------------------------------------------------------------------+------- + SELECT * FROM test_merge WHERE id IN ($1, $2, $3, $4, $5, $6, $7, $8, $9) | 1 + SELECT * FROM test_merge WHERE id IN (... [10-99 entries]) | 2 + SELECT pg_stat_statements_reset() IS NOT NULL AS t | 1 +(3 rows) + +-- With gaps on the threshold +SELECT pg_stat_statements_reset() IS NOT NULL AS t; + t +--- + t +(1 row) + +SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4); + id | data +----+------ +(0 rows) + +SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C"; + query | calls +-------------------------------------------------------+------- + SELECT * FROM test_merge WHERE id IN ($1, $2, $3, $4) | 1 + SELECT pg_stat_statements_reset() IS NOT NULL AS t | 1 +(2 rows) + +SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15); + id | data +----+------ +(0 rows) + +SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C"; + query | calls +------------------------------------------------------------------------+------- + SELECT * FROM test_merge WHERE id IN ($1, $2, $3, $4) | 1 + SELECT * FROM test_merge WHERE id IN (... [10-99 entries]) | 1 + SELECT pg_stat_statements_reset() IS NOT NULL AS t | 1 + SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 1 +(4 rows) + +RESET pg_stat_statements.query_id_const_merge_threshold; diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c index 00eec30feb1..569a74c4368 100644 --- a/contrib/pg_stat_statements/pg_stat_statements.c +++ b/contrib/pg_stat_statements/pg_stat_statements.c @@ -265,8 +265,8 @@ static ExecutorFinish_hook_type prev_ExecutorFinish = NULL; static ExecutorEnd_hook_type prev_ExecutorEnd = NULL; static ProcessUtility_hook_type prev_ProcessUtility = NULL; -/* An assign hook to keep query_id_const_merge in sync */ -static void pgss_query_id_const_merge_assign_hook(bool newvalue, void *extra); +/* An assign hook to keep query_id_const_merge_threshold in sync */ +static void pgss_query_id_const_merge_assign_hook(int newvalue, void *extra); /* Links to shared memory state */ static pgssSharedState *pgss = NULL; @@ -295,8 +295,8 @@ static bool pgss_track_utility = true; /* whether to track utility commands */ static bool pgss_track_planning = false; /* whether to track planning * duration */ static bool pgss_save = true; /* whether to save stats across shutdown */ -static bool pgss_query_id_const_merge = false; /* request constants merging - * when computing query_id */ +static int pgss_query_id_const_merge_threshold = 0; /* request constants merging + * when computing query_id */ #define pgss_enabled(level) \ (!IsParallelWorker() && \ @@ -459,20 +459,22 @@ _PG_init(void) NULL, NULL); - DefineCustomBoolVariable("pg_stat_statements.query_id_const_merge", - "Whether to merge constants in a list when computing query_id.", - NULL, - &pgss_query_id_const_merge, - false, - PGC_SUSET, - 0, - NULL, - pgss_query_id_const_merge_assign_hook, - NULL); + DefineCustomIntVariable("pg_stat_statements.query_id_const_merge_threshold", + "Whether to merge constants in a list when computing query_id.", + NULL, + &pgss_query_id_const_merge_threshold, + 0, + 0, + INT_MAX, + PGC_SUSET, + 0, + NULL, + pgss_query_id_const_merge_assign_hook, + NULL); MarkGUCPrefixReserved("pg_stat_statements"); - SetQueryIdConstMerge(pgss_query_id_const_merge); + SetQueryIdConstMerge(pgss_query_id_const_merge_threshold); /* * Install hooks. @@ -3064,10 +3066,10 @@ comp_location(const void *a, const void *b) } /* - * Notify query jumbling about query_id_const_merge status + * Notify query jumbling about query_id_const_merge_threshold status */ static void -pgss_query_id_const_merge_assign_hook(bool newvalue, void *extra) +pgss_query_id_const_merge_assign_hook(int newvalue, void *extra) { SetQueryIdConstMerge(newvalue); } diff --git a/contrib/pg_stat_statements/sql/merging.sql b/contrib/pg_stat_statements/sql/merging.sql index 657044faded..fedeb35b8f5 100644 --- a/contrib/pg_stat_statements/sql/merging.sql +++ b/contrib/pg_stat_statements/sql/merging.sql @@ -15,7 +15,7 @@ SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11); SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C"; -- Normal scenario, too many simple constants for an IN query -SET pg_stat_statements.query_id_const_merge = on; +SET pg_stat_statements.query_id_const_merge_threshold = 1; SELECT pg_stat_statements_reset() IS NOT NULL AS t; SELECT * FROM test_merge WHERE id IN (1); @@ -68,4 +68,21 @@ WITH cte AS ( SELECT ARRAY['a', 'b', 'c', const::varchar] AS result FROM cte; -RESET pg_stat_statements.query_id_const_merge; +-- With the threshold +SET pg_stat_statements.query_id_const_merge_threshold = 10; + +SELECT pg_stat_statements_reset() IS NOT NULL AS t; +SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9); +SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10); +SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11); +SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C"; + +-- With gaps on the threshold +SELECT pg_stat_statements_reset() IS NOT NULL AS t; +SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4); +SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C"; + +SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15); +SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C"; + +RESET pg_stat_statements.query_id_const_merge_threshold; diff --git a/doc/src/sgml/pgstatstatements.sgml b/doc/src/sgml/pgstatstatements.sgml index bac029430be..3a060935bff 100644 --- a/doc/src/sgml/pgstatstatements.sgml +++ b/doc/src/sgml/pgstatstatements.sgml @@ -605,12 +605,12 @@ In some cases, queries with visibly different texts might get merged into a single <structname>pg_stat_statements</structname> entry. Normally this will happen only for semantically equivalent queries, or if - <varname>pg_stat_statements.query_id_const_merge</varname> is enabled and - the only difference between queries is the length of an array with constants - they contain: + <varname>pg_stat_statements.query_id_const_merge_threshold</varname> is + enabled and the only difference between queries is the length of an array + with constants they contain: <screen> -=# SET query_id_const_merge = on; +=# SET query_id_const_merge_threshold = 1; =# SELECT pg_stat_statements_reset(); =# SELECT * FROM test WHERE a IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11); =# SELECT * FROM test WHERE a IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12); @@ -953,9 +953,9 @@ calls | 1 <varlistentry> <term> - <varname>pg_stat_statements.query_id_const_merge</varname> (<type>bool</type>) + <varname>pg_stat_statements.query_id_const_merge_threshold</varname> (<type>integer</type>) <indexterm> - <primary><varname>pg_stat_statements.query_id_const_merge</varname> configuration parameter</primary> + <primary><varname>pg_stat_statements.query_id_const_merge_threshold</varname> configuration parameter</primary> </indexterm> </term> @@ -967,11 +967,12 @@ calls | 1 query will get multiple different identifiers, one for each occurrence with an array of different lenght. - If this parameter is on, an array of constants will contribute only the - first element, the last element and the number of elements to the query - identifier. It means two occurences of the same query, where the only - difference is number of constants in the array, are going to get the - same query identifier if the arrays are of similar length. + If this parameter is greater than 0, an array with more than + <varname>pg_stat_statements.query_id_const_merge_threshold</varname> + constants will contribute only the first element, the last element + and the number of elements to the query identifier. It means two + occurences of the same query, where the only difference is number of + constants in the array, are going to get the same query identifier. Such queries are represented in form <literal>'(... [10-99 entries])'</literal>. The parameter could be used to reduce amount of repeating data stored diff --git a/src/backend/nodes/queryjumblefuncs.c b/src/backend/nodes/queryjumblefuncs.c index a27202cb279..66696c87475 100644 --- a/src/backend/nodes/queryjumblefuncs.c +++ b/src/backend/nodes/queryjumblefuncs.c @@ -44,8 +44,8 @@ /* GUC parameters */ int compute_query_id = COMPUTE_QUERY_ID_AUTO; -/* Whether to merge constants in a list when computing query_id */ -bool query_id_const_merge = false; +/* Lower threshold for the list length to merge constants when computing query_id */ +int query_id_const_merge_threshold = 1; /* * True when compute_query_id is ON or AUTO, and a module requests them. @@ -165,12 +165,14 @@ EnableQueryId(void) * Controls constants merging for query identifier computation. * * Third-party plugins can use this function to enable/disable merging - * of constants in a list when query identifier is computed. + * of constants in a list when query identifier is computed. The argument + * specifies the lower threshold for an array length, above which merging will + * be applied. */ void -SetQueryIdConstMerge(bool value) +SetQueryIdConstMerge(int threshold) { - query_id_const_merge = value; + query_id_const_merge_threshold = threshold; } /* @@ -246,7 +248,8 @@ RecordConstLocation(JumbleState *jstate, int location, int magnitude) /* * Verify if the provided list contains could be merged down, which means it - * contains only constant expressions. + * contains only constant expressions and the list contains more than + * query_id_const_merge_threshold elements. * * Return value is the order of magnitude (i.e. how many digits it has) for * length of the list (to use for representation purposes later on) if merging @@ -264,12 +267,18 @@ IsMergeableConstList(List *elements, Const **firstConst, Const **lastConst) if (elements == NULL) return 0; - if (!query_id_const_merge) + if (query_id_const_merge_threshold < 1) { /* Merging is disabled, process everything one by one */ return 0; } + if (elements->length < query_id_const_merge_threshold) + { + /* The list is not large enough */ + return 0; + } + firstExpr = linitial(elements); /* diff --git a/src/backend/postmaster/launch_backend.c b/src/backend/postmaster/launch_backend.c index 5bb2f00c6ef..84b214a5952 100644 --- a/src/backend/postmaster/launch_backend.c +++ b/src/backend/postmaster/launch_backend.c @@ -122,7 +122,7 @@ typedef struct bool redirection_done; bool IsBinaryUpgrade; bool query_id_enabled; - bool query_id_const_merge; + int query_id_const_merge_threshold; int max_safe_fds; int MaxBackends; #ifdef WIN32 @@ -743,7 +743,7 @@ save_backend_variables(BackendParameters *param, ClientSocket *client_sock, param->redirection_done = redirection_done; param->IsBinaryUpgrade = IsBinaryUpgrade; param->query_id_enabled = query_id_enabled; - param->query_id_const_merge = query_id_const_merge; + param->query_id_const_merge_threshold = query_id_const_merge_threshold; param->max_safe_fds = max_safe_fds; param->MaxBackends = MaxBackends; @@ -1002,7 +1002,7 @@ restore_backend_variables(BackendParameters *param) redirection_done = param->redirection_done; IsBinaryUpgrade = param->IsBinaryUpgrade; query_id_enabled = param->query_id_enabled; - query_id_const_merge = param->query_id_const_merge; + query_id_const_merge_threshold = param->query_id_const_merge_threshold; max_safe_fds = param->max_safe_fds; MaxBackends = param->MaxBackends; diff --git a/src/include/nodes/queryjumble.h b/src/include/nodes/queryjumble.h index 551555494e0..ae5907aed84 100644 --- a/src/include/nodes/queryjumble.h +++ b/src/include/nodes/queryjumble.h @@ -74,10 +74,10 @@ extern PGDLLIMPORT int compute_query_id; extern const char *CleanQuerytext(const char *query, int *location, int *len); extern JumbleState *JumbleQuery(Query *query); extern void EnableQueryId(void); -extern void SetQueryIdConstMerge(bool value); +extern void SetQueryIdConstMerge(int threshold); extern PGDLLIMPORT bool query_id_enabled; -extern PGDLLIMPORT bool query_id_const_merge; +extern PGDLLIMPORT int query_id_const_merge_threshold; /* * Returns whether query identifier computation has been enabled, either -- 2.41.0 ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: pg_stat_statements and "IN" conditions @ 2024-04-15 09:09 Sutou Kouhei <[email protected]> parent: Dmitry Dolgov <[email protected]> 0 siblings, 0 replies; 10+ messages in thread From: Sutou Kouhei @ 2024-04-15 09:09 UTC (permalink / raw) To: [email protected]; +Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; pgsql-hackers; [email protected] Hi, In <[email protected]> "Re: pg_stat_statements and "IN" conditions" on Thu, 4 Apr 2024 16:35:14 +0200, Dmitry Dolgov <[email protected]> wrote: > Here is the rebased version. Thanks. I'm not familiar with this code base but I've reviewed these patches because I'm interested in this feature too. 0001: > diff --git a/src/backend/nodes/queryjumblefuncs.c b/src/backend/nodes/queryjumblefuncs.c > index be823a7f8fa..e9473def361 100644 > --- a/src/backend/nodes/queryjumblefuncs.c > +++ b/src/backend/nodes/queryjumblefuncs.c > > @@ -212,15 +233,67 @@ RecordConstLocation(JumbleState *jstate, int location) > ... > +static bool > +IsMergeableConstList(List *elements, Const **firstConst, Const **lastConst) > +{ > + ListCell *temp; > + Node *firstExpr = NULL; > + > + if (elements == NULL) "elements == NIL" will be better for List. > +static void > +_jumbleElements(JumbleState *jstate, List *elements) > +{ > + Const *first, *last; > + if(IsMergeableConstList(elements, &first, &last)) A space is missing between "if" and "(". > diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h > index aa727e722cc..cf4f900d4ed 100644 > --- a/src/include/nodes/primnodes.h > +++ b/src/include/nodes/primnodes.h > @@ -1333,7 +1333,7 @@ typedef struct ArrayExpr > /* common type of array elements */ > Oid element_typeid pg_node_attr(query_jumble_ignore); > /* the array elements or sub-arrays */ > - List *elements; > + List *elements pg_node_attr(query_jumble_merge); Should we also update the pg_node_attr() comment for query_jumble_merge in nodes.h? 0003: > diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c > index d7841b51cc9..00eec30feb1 100644 > --- a/contrib/pg_stat_statements/pg_stat_statements.c > +++ b/contrib/pg_stat_statements/pg_stat_statements.c > ... > @@ -2883,12 +2886,22 @@ generate_normalized_query(JumbleState *jstate, const char *query, > /* The firsts merged constant */ > else if (!skip) > { > + static const uint32 powers_of_ten[] = { > + 1, 10, 100, > + 1000, 10000, 100000, > + 1000000, 10000000, 100000000, > + 1000000000 > + }; > + int lower_merged = powers_of_ten[magnitude - 1]; > + int upper_merged = powers_of_ten[magnitude]; How about adding a reverse function of decimalLength32() to numutils.h and use it here? > - n_quer_loc += sprintf(norm_query + n_quer_loc, "..."); > + n_quer_loc += sprintf(norm_query + n_quer_loc, "... [%d-%d entries]", > + lower_merged, upper_merged - 1); Do we still have enough space in norm_query for this change? It seems that norm_query expects up to 10 additional bytes per jstate->clocations[i]. It seems that we can merge 0001, 0003 and 0004 to one patch. (Sorry. I haven't read all discussions yet. If we already discuss this, sorry for this noise.) Thanks, -- kou ^ permalink raw reply [nested|flat] 10+ messages in thread
end of thread, other threads:[~2024-04-15 09:09 UTC | newest] Thread overview: 10+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2021-03-12 21:35 [PATCH 06/10] Default to LZ4.. Justin Pryzby <[email protected]> 2024-03-23 07:13 Re: pg_stat_statements and "IN" conditions Yasuo Honda <[email protected]> 2024-03-23 18:20 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]> 2024-03-24 14:36 ` Re: pg_stat_statements and "IN" conditions Yasuo Honda <[email protected]> 2024-03-25 16:35 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]> 2024-03-26 07:21 ` Re: pg_stat_statements and "IN" conditions Yasuo Honda <[email protected]> 2024-03-26 20:59 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]> 2024-03-26 23:56 ` Re: pg_stat_statements and "IN" conditions Yasuo Honda <[email protected]> 2024-04-04 14:35 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]> 2024-04-15 09:09 ` Re: pg_stat_statements and "IN" conditions Sutou Kouhei <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox