public inbox for [email protected]
help / color / mirror / Atom feed[PATCH 5/8] add special pg_brin_bloom_summary data type
12+ messages / 8 participants
[nested] [flat]
* [PATCH 5/8] add special pg_brin_bloom_summary data type
@ 2020-08-06 15:56 Tomas Vondra <[email protected]>
0 siblings, 0 replies; 12+ messages in thread
From: Tomas Vondra @ 2020-08-06 15:56 UTC (permalink / raw)
---
src/backend/access/brin/brin_bloom.c | 91 ++++++++++++++++++++++-
src/include/catalog/pg_proc.dat | 14 ++++
src/include/catalog/pg_type.dat | 7 ++
src/test/regress/expected/type_sanity.out | 7 +-
4 files changed, 115 insertions(+), 4 deletions(-)
diff --git a/src/backend/access/brin/brin_bloom.c b/src/backend/access/brin/brin_bloom.c
index b119e4e264..e7ca100821 100644
--- a/src/backend/access/brin/brin_bloom.c
+++ b/src/backend/access/brin/brin_bloom.c
@@ -647,7 +647,7 @@ brin_bloom_opcinfo(PG_FUNCTION_ARGS)
result->oi_regular_nulls = true;
result->oi_opaque = (BloomOpaque *)
MAXALIGN((char *) result + SizeofBrinOpcInfo(1));
- result->oi_typcache[0] = lookup_type_cache(BYTEAOID, 0);
+ result->oi_typcache[0] = lookup_type_cache(BRINBLOOMSUMMARYOID, 0);
PG_RETURN_POINTER(result);
}
@@ -978,3 +978,92 @@ brin_bloom_options(PG_FUNCTION_ARGS)
PG_RETURN_VOID();
}
+
+/*
+ * brin_bloom_summary_in
+ * - input routine for type brin_bloom_summary.
+ *
+ * brin_bloom_summary is only used internally to represent summaries
+ * in BRIN bloom indexes, so it has no operations of its own, and we
+ * disallow input too.
+ */
+Datum
+brin_bloom_summary_in(PG_FUNCTION_ARGS)
+{
+ /*
+ * brin_bloom_summary stores the data in binary form and parsing
+ * text input is not needed, so disallow this.
+ */
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot accept a value of type %s", "pg_brin_bloom_summary")));
+
+ PG_RETURN_VOID(); /* keep compiler quiet */
+}
+
+
+/*
+ * brin_bloom_summary_out
+ * - output routine for type brin_bloom_summary.
+ *
+ * BRIN bloom summaries are serialized into a bytea value, but we want
+ * to output something nicer humans can understand.
+ */
+Datum
+brin_bloom_summary_out(PG_FUNCTION_ARGS)
+{
+ BloomFilter *filter;
+ StringInfoData str;
+
+ initStringInfo(&str);
+ appendStringInfoChar(&str, '{');
+
+ /*
+ * XXX not sure the detoasting is necessary (probably not, this
+ * can only be in an index).
+ */
+ filter = (BloomFilter *) PG_DETOAST_DATUM(PG_GETARG_BYTEA_PP(0));
+
+ if (BLOOM_IS_HASHED(filter))
+ {
+ appendStringInfo(&str, "mode: hashed nhashes: %u nbits: %u nbits_set: %u",
+ filter->nhashes, filter->nbits, filter->nbits_set);
+ }
+ else
+ {
+ appendStringInfo(&str, "mode: sorted nvalues: %u nsorted: %u",
+ filter->nvalues, filter->nsorted);
+ /* TODO include the sorted/unsorted values */
+ }
+
+ appendStringInfoChar(&str, '}');
+
+ PG_RETURN_CSTRING(str.data);
+}
+
+/*
+ * brin_bloom_summary_recv
+ * - binary input routine for type brin_bloom_summary.
+ */
+Datum
+brin_bloom_summary_recv(PG_FUNCTION_ARGS)
+{
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot accept a value of type %s", "pg_brin_bloom_summary")));
+
+ PG_RETURN_VOID(); /* keep compiler quiet */
+}
+
+/*
+ * brin_bloom_summary_send
+ * - binary output routine for type brin_bloom_summary.
+ *
+ * BRIN bloom summaries are serialized in a bytea value (although the
+ * type is named differently), so let's just send that.
+ */
+Datum
+brin_bloom_summary_send(PG_FUNCTION_ARGS)
+{
+ return byteasend(fcinfo);
+}
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 5336aa7b9c..da4c9c6202 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -10975,3 +10975,17 @@
prosrc => 'unicode_is_normalized' },
]
+
+{ oid => '9035', descr => 'I/O',
+ proname => 'brin_bloom_summary_in', prorettype => 'pg_brin_bloom_summary',
+ proargtypes => 'cstring', prosrc => 'brin_bloom_summary_in' },
+{ oid => '9036', descr => 'I/O',
+ proname => 'brin_bloom_summary_out', prorettype => 'cstring',
+ proargtypes => 'pg_brin_bloom_summary', prosrc => 'brin_bloom_summary_out' },
+{ oid => '9037', descr => 'I/O',
+ proname => 'brin_bloom_summary_recv', provolatile => 's',
+ prorettype => 'pg_brin_bloom_summary', proargtypes => 'internal',
+ prosrc => 'brin_bloom_summary_recv' },
+{ oid => '9038', descr => 'I/O',
+ proname => 'brin_bloom_summary_send', provolatile => 's', prorettype => 'bytea',
+ proargtypes => 'pg_brin_bloom_summary', prosrc => 'brin_bloom_summary_send' },
diff --git a/src/include/catalog/pg_type.dat b/src/include/catalog/pg_type.dat
index b2cec07416..a41c2e5418 100644
--- a/src/include/catalog/pg_type.dat
+++ b/src/include/catalog/pg_type.dat
@@ -631,3 +631,10 @@
typalign => 'd', typstorage => 'x' },
]
+
+{ oid => '9034', oid_symbol => 'BRINBLOOMSUMMARYOID',
+ descr => 'BRIN bloom summary',
+ typname => 'pg_brin_bloom_summary', typlen => '-1', typbyval => 'f', typcategory => 'S',
+ typinput => 'brin_bloom_summary_in', typoutput => 'brin_bloom_summary_out',
+ typreceive => 'brin_bloom_summary_recv', typsend => 'brin_bloom_summary_send',
+ typalign => 'i', typstorage => 'x', typcollation => 'default' },
diff --git a/src/test/regress/expected/type_sanity.out b/src/test/regress/expected/type_sanity.out
index 274130e706..97bf9797de 100644
--- a/src/test/regress/expected/type_sanity.out
+++ b/src/test/regress/expected/type_sanity.out
@@ -67,13 +67,14 @@ WHERE p1.typtype not in ('c','d','p') AND p1.typname NOT LIKE E'\\_%'
(SELECT 1 FROM pg_type as p2
WHERE p2.typname = ('_' || p1.typname)::name AND
p2.typelem = p1.oid and p1.typarray = p2.oid);
- oid | typname
-------+-----------------
+ oid | typname
+------+-----------------------
194 | pg_node_tree
3361 | pg_ndistinct
3402 | pg_dependencies
5017 | pg_mcv_list
-(4 rows)
+ 9034 | pg_brin_bloom_summary
+(5 rows)
-- Make sure typarray points to a varlena array type of our own base
SELECT p1.oid, p1.typname as basetype, p2.typname as arraytype,
--
2.25.4
--jsivyprvf2oxfjz3
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
filename="0006-BRIN-multi-range-minmax-indexes-20200807.patch"
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pg_stat_statements and "IN" conditions
@ 2023-02-02 14:07 Alvaro Herrera <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Alvaro Herrera @ 2023-02-02 14:07 UTC (permalink / raw)
To: Dmitry Dolgov <[email protected]>; +Cc: Marcos Pegoraro <[email protected]>; vignesh C <[email protected]>; Sergei Kornilov <[email protected]>; Robert Haas <[email protected]>; Zhihong Yu <[email protected]>; David Steele <[email protected]>; pgsql-hackers; Greg Stark <[email protected]>; Pavel Trukhanov <[email protected]>; Tom Lane <[email protected]>
This appears to have massive conflicts. Would you please rebase?
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"¿Cómo puedes confiar en algo que pagas y que no ves,
y no confiar en algo que te dan y te lo muestran?" (Germán Poo)
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pg_stat_statements and "IN" conditions
@ 2023-02-02 15:05 Dmitry Dolgov <[email protected]>
parent: Alvaro Herrera <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Dmitry Dolgov @ 2023-02-02 15:05 UTC (permalink / raw)
To: Alvaro Herrera <[email protected]>; +Cc: Marcos Pegoraro <[email protected]>; vignesh C <[email protected]>; Sergei Kornilov <[email protected]>; Robert Haas <[email protected]>; Zhihong Yu <[email protected]>; David Steele <[email protected]>; pgsql-hackers; Greg Stark <[email protected]>; Pavel Trukhanov <[email protected]>; Tom Lane <[email protected]>
> On Thu, Feb 02, 2023 at 03:07:27PM +0100, Alvaro Herrera wrote:
> This appears to have massive conflicts. Would you please rebase?
Sure, I was already mentally preparing myself to do so in the view of
recent changes in query jumbling. Will post soon.
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pg_stat_statements and "IN" conditions
@ 2023-02-04 17:08 Dmitry Dolgov <[email protected]>
parent: Dmitry Dolgov <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Dmitry Dolgov @ 2023-02-04 17:08 UTC (permalink / raw)
To: Alvaro Herrera <[email protected]>; +Cc: Marcos Pegoraro <[email protected]>; vignesh C <[email protected]>; Sergei Kornilov <[email protected]>; Robert Haas <[email protected]>; Zhihong Yu <[email protected]>; David Steele <[email protected]>; pgsql-hackers; Greg Stark <[email protected]>; Pavel Trukhanov <[email protected]>; Tom Lane <[email protected]>
> On Thu, Feb 02, 2023 at 04:05:54PM +0100, Dmitry Dolgov wrote:
> > On Thu, Feb 02, 2023 at 03:07:27PM +0100, Alvaro Herrera wrote:
> > This appears to have massive conflicts. Would you please rebase?
>
> Sure, I was already mentally preparing myself to do so in the view of
> recent changes in query jumbling. Will post soon.
Here is the rebased version. To adapt to the latest changes, I've marked
ArrayExpr with custom_query_jumble to implement this functionality, but
tried to make the actual merge logic relatively independent. Otherwise,
everything is the same.
Attachments:
[text/x-diff] v12-0001-Prevent-jumbling-of-every-element-in-ArrayExpr.patch (34.3K, ../../[email protected]/2-v12-0001-Prevent-jumbling-of-every-element-in-ArrayExpr.patch)
download | inline diff:
From e72f6b8990dace82667d46b3578062bee92af472 Mon Sep 17 00:00:00 2001
From: Dmitrii Dolgov <[email protected]>
Date: Sun, 24 Jul 2022 11:43:25 +0200
Subject: [PATCH v12] 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 number of parameters, because every element of ArrayExpr is
jumbled. In certain situations it's undesirable, especially if the list
becomes too large.
Make Const expressions contribute nothing to the jumble hash if they're
a part of an ArrayExpr, which length is larger than specified threshold.
Allow to configure the threshold via the new GUC const_merge_threshold
with the default value zero, which disables this feature.
Reviewed-by: Zhihong Yu, Sergey Dudoladov, Robert Haas, Tom Lane
Tested-by: Chengxi Sun
---
.../expected/pg_stat_statements.out | 412 ++++++++++++++++++
.../pg_stat_statements/pg_stat_statements.c | 33 +-
.../sql/pg_stat_statements.sql | 107 +++++
doc/src/sgml/config.sgml | 26 ++
doc/src/sgml/pgstatstatements.sgml | 28 +-
src/backend/nodes/gen_node_support.pl | 2 +-
src/backend/nodes/queryjumblefuncs.c | 102 ++++-
src/backend/utils/misc/guc_tables.c | 13 +
src/backend/utils/misc/postgresql.conf.sample | 2 +-
src/include/nodes/primnodes.h | 2 +
src/include/nodes/queryjumble.h | 5 +-
11 files changed, 712 insertions(+), 20 deletions(-)
diff --git a/contrib/pg_stat_statements/expected/pg_stat_statements.out b/contrib/pg_stat_statements/expected/pg_stat_statements.out
index fb9ccd920f..9acdb55c9a 100644
--- a/contrib/pg_stat_statements/expected/pg_stat_statements.out
+++ b/contrib/pg_stat_statements/expected/pg_stat_statements.out
@@ -1142,4 +1142,416 @@ SELECT COUNT(*) FROM pg_stat_statements WHERE query LIKE '%SELECT GROUPING%';
2
(1 row)
+--
+-- Consts merging
+--
+CREATE TABLE test_merge (id int, data int);
+-- IN queries
+-- No merging
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(1 row)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8);
+ id | data
+----+------
+(0 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 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) | 1
+ SELECT * FROM test_merge WHERE id IN ($1, $2, $3, $4, $5, $6, $7) | 1
+ SELECT * FROM test_merge WHERE id IN ($1, $2, $3, $4, $5, $6, $7, $8) | 1
+ 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 pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
+(7 rows)
+
+-- Normal
+SET const_merge_threshold = 5;
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(1 row)
+
+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, $2, $3) | 1
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
+(3 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8);
+ id | data
+----+------
+(0 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 query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge WHERE id IN ($1, $2, $3) | 1
+ SELECT * FROM test_merge WHERE id IN (...) | 5
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 1
+(4 rows)
+
+-- On the merge threshold
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(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() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
+(3 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8);
+ id | data
+----+------
+(0 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 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 (...) | 6
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 1
+(4 rows)
+
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(1 row)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5);
+ 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
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
+(3 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8);
+ id | data
+----+------
+(0 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 query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge WHERE id IN (...) | 6
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 1
+(3 rows)
+
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(1 row)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6);
+ 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
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
+(3 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7);
+ id | data
+----+------
+(0 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8);
+ id | data
+----+------
+(0 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 query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge WHERE id IN (...) | 5
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 1
+(3 rows)
+
+-- With gaps on the threshold
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(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() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
+(3 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+ 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 (...) | 1
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 1
+(4 rows)
+
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(1 row)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5);
+ 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
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
+(3 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+ 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 (...) | 2
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 1
+(3 rows)
+
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(1 row)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6);
+ 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
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
+(3 rows)
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+ 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 (...) | 2
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 1
+(3 rows)
+
+-- test constants after merge
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(1 row)
+
+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 query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+ query | calls
+------------------------------------------------------------------------+-------
+ SELECT * FROM test_merge WHERE id IN (...) and data = $3 | 1
+ SELECT pg_stat_statements_reset() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
+(3 rows)
+
+-- On table, numeric type causes every constant being wrapped into functions.
+CREATE TABLE test_merge_numeric (id int, data numeric(5, 2));
+SELECT pg_stat_statements_reset();
+ pg_stat_statements_reset
+--------------------------
+
+(1 row)
+
+SELECT * FROM test_merge_numeric WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+ 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() | 1
+ SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C" | 0
+(3 rows)
+
+-- Test find_const_walker
+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 const_merge_threshold;
DROP EXTENSION pg_stat_statements;
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index ad1fe44496..b26ae1f234 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -2666,6 +2666,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
@@ -2689,7 +2692,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;
@@ -2704,12 +2706,31 @@ 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, "...");
+ }
quer_loc = off + tok_len;
last_off = off;
diff --git a/contrib/pg_stat_statements/sql/pg_stat_statements.sql b/contrib/pg_stat_statements/sql/pg_stat_statements.sql
index b82cddf16f..f561edbe83 100644
--- a/contrib/pg_stat_statements/sql/pg_stat_statements.sql
+++ b/contrib/pg_stat_statements/sql/pg_stat_statements.sql
@@ -465,4 +465,111 @@ SELECT (
SELECT COUNT(*) FROM pg_stat_statements WHERE query LIKE '%SELECT GROUPING%';
+--
+-- Consts merging
+--
+CREATE TABLE test_merge (id int, data int);
+
+-- IN queries
+
+-- No merging
+SELECT pg_stat_statements_reset();
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8);
+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 query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+-- Normal
+SET const_merge_threshold = 5;
+
+SELECT pg_stat_statements_reset();
+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);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8);
+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 query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+-- On the merge threshold
+SELECT pg_stat_statements_reset();
+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);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8);
+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 query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+SELECT pg_stat_statements_reset();
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5);
+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);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8);
+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 query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+SELECT pg_stat_statements_reset();
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6);
+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);
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8);
+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 query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+-- With gaps on the threshold
+SELECT pg_stat_statements_reset();
+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);
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+SELECT pg_stat_statements_reset();
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5);
+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);
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+SELECT pg_stat_statements_reset();
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6);
+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);
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+-- test constants after merge
+SELECT pg_stat_statements_reset();
+
+SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10) and data = 2;
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+-- On table, numeric type causes every constant being wrapped into functions.
+CREATE TABLE test_merge_numeric (id int, data numeric(5, 2));
+SELECT pg_stat_statements_reset();
+SELECT * FROM test_merge_numeric WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+SELECT query, calls FROM pg_stat_statements ORDER BY query COLLATE "C";
+
+-- Test find_const_walker
+WITH cte AS (
+ SELECT 'const' as const FROM test_merge
+)
+SELECT ARRAY['a', 'b', 'c', const::varchar] AS result
+FROM cte;
+
+RESET const_merge_threshold;
+
DROP EXTENSION pg_stat_statements;
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index d190be1925..501a132a4a 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8278,6 +8278,32 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
</listitem>
</varlistentry>
+ <varlistentry id="guc-const-merge-threshold" xreflabel="const_merge_treshold">
+ <term><varname>const_merge_threshold</varname> (<type>integer</type>)
+ <indexterm>
+ <primary><varname>const_merge_threshold</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Specifies the minimal length of an array to be eligible for constants
+ collapsing. Normally every element of an array contributes to a query
+ identifier, which means the same query containing an array of constants
+ could get multiple different identifiers, depending of size of the
+ array. If this parameter is nonzero, the array contains only constants
+ and it's length is larger than <varname> const_merge_threshold </varname>,
+ then array elements will contribute nothing to the query identifier.
+ Thus the query will get the same identifier no matter how many constants
+ it contains.
+
+ Zero turns off collapsing, and it is the default value.
+
+ The <xref linkend="pgstatstatements"/> extension will represent such
+ collapsed constants via <literal>'(...)'</literal>.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</sect2>
diff --git a/doc/src/sgml/pgstatstatements.sgml b/doc/src/sgml/pgstatstatements.sgml
index efc36da602..f7e2e9fe85 100644
--- a/doc/src/sgml/pgstatstatements.sgml
+++ b/doc/src/sgml/pgstatstatements.sgml
@@ -519,10 +519,30 @@
<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.)
+ only for semantically equivalent queries, for example when queries are
+ different only in values of constants they use. Another valid possibility for
+ merging queries into a single <structname>pg_stat_statements</structname>
+ entry is when <xref linkend="guc-const-merge-threshold"/> is nonzero and the
+ queries contain an array with more than <varname>const_merge_threshold</varname>
+ constants in it:
+
+<screen>
+=# SET const_merge_threshold = 5;
+=# SELECT pg_stat_statements_reset();
+=# SELECT * FROM test WHERE a IN (1, 2, 3, 4, 5, 6, 7, 8, 9);
+=# SELECT * FROM test WHERE a IN (1, 2, 3, 4, 5, 6, 7);
+=# 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>
diff --git a/src/backend/nodes/gen_node_support.pl b/src/backend/nodes/gen_node_support.pl
index 19ed29657c..86c4223ace 100644
--- a/src/backend/nodes/gen_node_support.pl
+++ b/src/backend/nodes/gen_node_support.pl
@@ -1306,7 +1306,7 @@ _jumble${n}(JumbleState *jstate, Node *node)
# 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 223d1bc826..06551f6a1b 100644
--- a/src/backend/nodes/queryjumblefuncs.c
+++ b/src/backend/nodes/queryjumblefuncs.c
@@ -42,12 +42,15 @@
/* GUC parameters */
int compute_query_id = COMPUTE_QUERY_ID_AUTO;
+/* Minimal numer of constants in an array after which they will be merged */
+int const_merge_threshold = 0;
+
/* True when compute_query_id is ON, or AUTO and a module requests them */
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 _jumbleList(JumbleState *jstate, Node *node);
static void _jumbleRangeTblEntry(JumbleState *jstate, Node *node);
@@ -109,7 +112,7 @@ JumbleQuery(Query *query, const char *querytext)
jstate->jumble_len = 0;
jstate->clocations_buf_size = 32;
jstate->clocations = (LocationLen *)
- palloc(jstate->clocations_buf_size * sizeof(LocationLen));
+ palloc0(jstate->clocations_buf_size * sizeof(LocationLen));
jstate->clocations_count = 0;
jstate->highest_extern_param_id = 0;
@@ -185,11 +188,13 @@ 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 do not
+ * contribute to the jumble hash, and any reader of constants array may want to
+ * use this information to represent such constants differently.
*/
static void
-RecordConstLocation(JumbleState *jstate, int location)
+RecordConstLocation(JumbleState *jstate, int location, bool merged)
{
/* -1 indicates unknown or undefined location */
if (location >= 0)
@@ -205,15 +210,81 @@ 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 only constant expressions and its
+ * length is greater than or equal to const_merge_threshold, thus making it
+ * eligible for constants merging.
+ *
+ * Note that this function searches only for Const directly and do not tries to
+ * simplify expressions.
+ */
+static bool
+IsConstList(List *elements, Const **firstConst, Const **lastConst)
+{
+ ListCell *temp;
+ Node *firstExpr = NULL;
+ bool allConst = true;
+
+ if (elements == NULL)
+ return false;
+
+ if (const_merge_threshold == 0)
+ {
+ /* Merging is disabled, process everything one by one. */
+ return false;
+ }
+
+ if (elements->length < const_merge_threshold)
+ {
+ /* The list is not large enough to collapse it. */
+ 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 collapsing --
+ * mark it as merged and return from the function.
+ */
+ if (IsA(firstExpr, Const))
+ {
+ foreach(temp, elements)
+ {
+ Node *expr = (Node *) lfirst(temp);
+
+ if (!IsA(expr, Const))
+ {
+ allConst = false;
+ break;
+ }
+ }
+
+ if (allConst)
+ {
+ *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_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) \
@@ -226,6 +297,23 @@ do { \
#include "queryjumblefuncs.funcs.c"
+static void
+_jumbleArrayExpr(JumbleState *jstate, Node *node)
+{
+ ArrayExpr *expr = (ArrayExpr *) node;
+ Const *first, *last;
+
+ if (IsConstList(expr->elements, &first, &last))
+ {
+ RecordConstLocation(jstate, first->location, true);
+ RecordConstLocation(jstate, last->location, true);
+ }
+ else
+ {
+ JUMBLE_NODE(elements);
+ }
+}
+
static void
_jumbleNode(JumbleState *jstate, Node *node)
{
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index b46e3b8c55..77ab38172b 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -3467,6 +3467,19 @@ struct config_int ConfigureNamesInt[] =
NULL, NULL, NULL
},
+ {
+ {"const_merge_threshold", PGC_SUSET, STATS_MONITORING,
+ gettext_noop("Sets the minimal numer of constants in an array"
+ " after which they will be merged"),
+ gettext_noop("Computing query id for an array of constants"
+ " will produce the same id for all arrays with length"
+ " larger than this value. Zero turns off merging."),
+ },
+ &const_merge_threshold,
+ 0, 0, INT_MAX,
+ NULL, NULL, NULL
+ },
+
/* End-of-list marker */
{
{NULL, 0, 0, NULL, NULL}, NULL, 0, 0, 0, NULL, NULL, NULL
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index d06074b86f..0594eb17b2 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -627,7 +627,7 @@
#log_parser_stats = off
#log_planner_stats = off
#log_executor_stats = off
-
+#const_merge_threshold = 0
#------------------------------------------------------------------------------
# AUTOVACUUM
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index 6d740be5c0..8e568947bd 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -1287,6 +1287,8 @@ typedef struct CaseTestExpr
*/
typedef struct ArrayExpr
{
+ pg_node_attr(custom_query_jumble)
+
Expr xpr;
/* type of expression result */
Oid array_typeid pg_node_attr(query_jumble_ignore);
diff --git a/src/include/nodes/queryjumble.h b/src/include/nodes/queryjumble.h
index 204b8f74fd..4410e2cf61 100644
--- a/src/include/nodes/queryjumble.h
+++ b/src/include/nodes/queryjumble.h
@@ -15,6 +15,7 @@
#define QUERYJUBLE_H
#include "nodes/parsenodes.h"
+#include "nodes/nodeFuncs.h"
/*
* Struct for tracking locations/lengths of constants during normalization
@@ -23,6 +24,8 @@ typedef struct LocationLen
{
int location; /* start offset in query text */
int length; /* length in bytes, or -1 to ignore */
+ bool merged; /* whether or not the location was marked as
+ not contributing to jumble */
} LocationLen;
/*
@@ -61,7 +64,7 @@ enum ComputeQueryIdType
/* GUC parameters */
extern PGDLLIMPORT int compute_query_id;
-
+extern PGDLLIMPORT int const_merge_threshold;
extern const char *CleanQuerytext(const char *query, int *location, int *len);
extern JumbleState *JumbleQuery(Query *query, const char *querytext);
--
2.32.0
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pg_stat_statements and "IN" conditions
@ 2023-02-05 01:30 Michael Paquier <[email protected]>
parent: Dmitry Dolgov <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Michael Paquier @ 2023-02-05 01:30 UTC (permalink / raw)
To: Dmitry Dolgov <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; Marcos Pegoraro <[email protected]>; vignesh C <[email protected]>; Sergei Kornilov <[email protected]>; Robert Haas <[email protected]>; Zhihong Yu <[email protected]>; David Steele <[email protected]>; pgsql-hackers; Greg Stark <[email protected]>; Pavel Trukhanov <[email protected]>; Tom Lane <[email protected]>
On Sat, Feb 04, 2023 at 06:08:41PM +0100, Dmitry Dolgov wrote:
> Here is the rebased version. To adapt to the latest changes, I've marked
> ArrayExpr with custom_query_jumble to implement this functionality, but
> tried to make the actual merge logic relatively independent. Otherwise,
> everything is the same.
I was quickly looking at this patch, so these are rough impressions.
+ bool merged; /* whether or not the location was marked as
+ not contributing to jumble */
This part of the patch is a bit disturbing.. We have node attributes
to track if portions of a node should be ignored or have a location
marked, still this "merged" flag is used as an extension to track if a
location should be done or not. Is that a concept that had better be
controlled via a new node attribute?
+--
+-- Consts merging
+--
+CREATE TABLE test_merge (id int, data int);
+-- IN queries
+-- No merging
Would it be better to split this set of tests into a new file? FWIW,
I have a patch in baking process that refactors a bit the whole,
before being able to extend it so as we have more coverage for
normalized utility queries, as of now the query strings stored by
pg_stat_statements don't reflect that even if the jumbling computation
marks the location of the Const nodes included in utility statements
(partition bounds, queries of COPY, etc.). I should be able to send
that tomorrow, and my guess that you could take advantage of that
even for this thread.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pg_stat_statements and "IN" conditions
@ 2023-02-05 11:33 Dmitry Dolgov <[email protected]>
parent: Michael Paquier <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Dmitry Dolgov @ 2023-02-05 11:33 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; Marcos Pegoraro <[email protected]>; vignesh C <[email protected]>; Sergei Kornilov <[email protected]>; Robert Haas <[email protected]>; Zhihong Yu <[email protected]>; David Steele <[email protected]>; pgsql-hackers; Greg Stark <[email protected]>; Pavel Trukhanov <[email protected]>; Tom Lane <[email protected]>
> On Sun, Feb 05, 2023 at 10:30:25AM +0900, Michael Paquier wrote:
> On Sat, Feb 04, 2023 at 06:08:41PM +0100, Dmitry Dolgov wrote:
> > Here is the rebased version. To adapt to the latest changes, I've marked
> > ArrayExpr with custom_query_jumble to implement this functionality, but
> > tried to make the actual merge logic relatively independent. Otherwise,
> > everything is the same.
>
> I was quickly looking at this patch, so these are rough impressions.
>
> + bool merged; /* whether or not the location was marked as
> + not contributing to jumble */
>
> This part of the patch is a bit disturbing.. We have node attributes
> to track if portions of a node should be ignored or have a location
> marked, still this "merged" flag is used as an extension to track if a
> location should be done or not. Is that a concept that had better be
> controlled via a new node attribute?
Good question. I need to think a bit more if it's possible to leverage
node attributes mechanism, but at the moment I'm still inclined to
extend LocationLen. The reason is that it doesn't exactly serve the
tracking purpose, i.e. whether to capture a location (I have to update
the code commentary), it helps differentiate cases when locations A and
D are obtained from merging A B C D instead of just being A and D.
I'm thinking about this in the following way: the core jumbling logic is
responsible for deriving locations based on the input expressions; in
the case of merging we produce less locations; pgss have to represent
the result only using locations and has to be able to differentiate
simple locations and locations after merging.
> +--
> +-- Consts merging
> +--
> +CREATE TABLE test_merge (id int, data int);
> +-- IN queries
> +-- No merging
>
> Would it be better to split this set of tests into a new file? FWIW,
> I have a patch in baking process that refactors a bit the whole,
> before being able to extend it so as we have more coverage for
> normalized utility queries, as of now the query strings stored by
> pg_stat_statements don't reflect that even if the jumbling computation
> marks the location of the Const nodes included in utility statements
> (partition bounds, queries of COPY, etc.). I should be able to send
> that tomorrow, and my guess that you could take advantage of that
> even for this thread.
Sure, I'll take a look how I can benefit from your work, thanks.
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pg_stat_statements and "IN" conditions
@ 2023-02-05 16:02 Tom Lane <[email protected]>
parent: Dmitry Dolgov <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Tom Lane @ 2023-02-05 16:02 UTC (permalink / raw)
To: Dmitry Dolgov <[email protected]>; +Cc: Michael Paquier <[email protected]>; Alvaro Herrera <[email protected]>; Marcos Pegoraro <[email protected]>; vignesh C <[email protected]>; Sergei Kornilov <[email protected]>; Robert Haas <[email protected]>; Zhihong Yu <[email protected]>; David Steele <[email protected]>; pgsql-hackers; Greg Stark <[email protected]>; Pavel Trukhanov <[email protected]>
Dmitry Dolgov <[email protected]> writes:
> I'm thinking about this in the following way: the core jumbling logic is
> responsible for deriving locations based on the input expressions; in
> the case of merging we produce less locations; pgss have to represent
> the result only using locations and has to be able to differentiate
> simple locations and locations after merging.
Uh ... why? ISTM you're just going to elide all inside the IN,
so why do you need more than a start and stop position?
regards, tom lane
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pg_stat_statements and "IN" conditions
@ 2023-02-05 19:56 Dmitry Dolgov <[email protected]>
parent: Tom Lane <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Dmitry Dolgov @ 2023-02-05 19:56 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Michael Paquier <[email protected]>; Alvaro Herrera <[email protected]>; Marcos Pegoraro <[email protected]>; vignesh C <[email protected]>; Sergei Kornilov <[email protected]>; Robert Haas <[email protected]>; Zhihong Yu <[email protected]>; David Steele <[email protected]>; pgsql-hackers; Greg Stark <[email protected]>; Pavel Trukhanov <[email protected]>
> On Sun, Feb 05, 2023 at 11:02:32AM -0500, Tom Lane wrote:
> Dmitry Dolgov <[email protected]> writes:
> > I'm thinking about this in the following way: the core jumbling logic is
> > responsible for deriving locations based on the input expressions; in
> > the case of merging we produce less locations; pgss have to represent
> > the result only using locations and has to be able to differentiate
> > simple locations and locations after merging.
>
> Uh ... why? ISTM you're just going to elide all inside the IN,
> so why do you need more than a start and stop position?
Exactly, start and stop positions. But if there would be no information
that merging was applied, the following queries will look the same after
jumbling, right?
-- input query
SELECT * FROM test_merge WHERE id IN (1, 2);
-- jumbling result, two LocationLen, for values 1 and 2
SELECT * FROM test_merge WHERE id IN ($1, $2);
-- input query
SELECT * FROM test_merge WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
-- jumbling result, two LocationLen after merging, for values 1 and 10
SELECT * FROM test_merge WHERE id IN (...);
-- without remembering about merging the result would be
SELECT * FROM test_merge WHERE id IN ($1, $2);
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re:pg_stat_statements and "IN" conditions
@ 2023-02-07 20:14 Sergei Kornilov <[email protected]>
parent: Dmitry Dolgov <[email protected]>
0 siblings, 2 replies; 12+ messages in thread
From: Sergei Kornilov @ 2023-02-07 20:14 UTC (permalink / raw)
To: Dmitry Dolgov <[email protected]>; +Cc: Michael Paquier <[email protected]>; Alvaro Herrera <[email protected]>; Marcos Pegoraro <[email protected]>; vignesh C <[email protected]>; Robert Haas <[email protected]>; Zhihong Yu <[email protected]>; David Steele <[email protected]>; pgsql-hackers; Greg Stark <[email protected]>; Pavel Trukhanov <[email protected]>; Tom Lane <[email protected]>
Hello!
Unfortunately, rebase is needed again due to recent changes in queryjumblefuncs ( 9ba37b2cb6a174b37fc51d0649ef73e56eae27fc )
It seems a little strange to me that with const_merge_threshold = 1, such a test case gives the same result as with const_merge_threshold = 2
select pg_stat_statements_reset();
set const_merge_threshold to 1;
select * from test where i in (1,2,3);
select * from test where i in (1,2);
select * from test where i in (1);
select query, calls from pg_stat_statements order by query;
query | calls
-------------------------------------+-------
select * from test where i in (...) | 2
select * from test where i in ($1) | 1
Probably const_merge_threshold = 1 should produce only "i in (...)"?
const_merge_threshold is "the minimal length of an array" (more or equal) or "array .. length is larger than" (not equals)? I think the documentation is ambiguous in this regard.
I also noticed a typo in guc_tables.c: "Sets the minimal numer of constants in an array" -> number
regards, Sergei
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pg_stat_statements and "IN" conditions
@ 2023-02-09 13:30 Peter Eisentraut <[email protected]>
parent: Sergei Kornilov <[email protected]>
1 sibling, 0 replies; 12+ messages in thread
From: Peter Eisentraut @ 2023-02-09 13:30 UTC (permalink / raw)
To: Sergei Kornilov <[email protected]>; Dmitry Dolgov <[email protected]>; +Cc: Michael Paquier <[email protected]>; Alvaro Herrera <[email protected]>; Marcos Pegoraro <[email protected]>; vignesh C <[email protected]>; Robert Haas <[email protected]>; Zhihong Yu <[email protected]>; David Steele <[email protected]>; pgsql-hackers; Greg Stark <[email protected]>; Pavel Trukhanov <[email protected]>; Tom Lane <[email protected]>
On 07.02.23 21:14, Sergei Kornilov wrote:
> It seems a little strange to me that with const_merge_threshold = 1, such a test case gives the same result as with const_merge_threshold = 2
What is the point of making this a numeric setting? Either you want to
merge all values or you don't want to merge any values.
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pg_stat_statements and "IN" conditions
@ 2023-02-09 15:02 Dmitry Dolgov <[email protected]>
parent: Sergei Kornilov <[email protected]>
1 sibling, 1 reply; 12+ messages in thread
From: Dmitry Dolgov @ 2023-02-09 15:02 UTC (permalink / raw)
To: Sergei Kornilov <[email protected]>; +Cc: Michael Paquier <[email protected]>; Alvaro Herrera <[email protected]>; Marcos Pegoraro <[email protected]>; vignesh C <[email protected]>; Robert Haas <[email protected]>; Zhihong Yu <[email protected]>; David Steele <[email protected]>; pgsql-hackers; Greg Stark <[email protected]>; Pavel Trukhanov <[email protected]>; Tom Lane <[email protected]>
> On Tue, Feb 07, 2023 at 11:14:52PM +0300, Sergei Kornilov wrote:
> Hello!
Thanks for reviewing.
> Unfortunately, rebase is needed again due to recent changes in queryjumblefuncs ( 9ba37b2cb6a174b37fc51d0649ef73e56eae27fc )
Yep, my favourite game, rebaseball. Will post a new version soon, after
figuring out all the recent questions.
> It seems a little strange to me that with const_merge_threshold = 1, such a test case gives the same result as with const_merge_threshold = 2
>
> select pg_stat_statements_reset();
> set const_merge_threshold to 1;
> select * from test where i in (1,2,3);
> select * from test where i in (1,2);
> select * from test where i in (1);
> select query, calls from pg_stat_statements order by query;
>
> query | calls
> -------------------------------------+-------
> select * from test where i in (...) | 2
> select * from test where i in ($1) | 1
>
> Probably const_merge_threshold = 1 should produce only "i in (...)"?
Well, it's not intentional, probably I need to be more careful with
off-by-one. Although I agree to a certain extent with Peter questioning
the value of having numerical option here, let me think about this.
> const_merge_threshold is "the minimal length of an array" (more or equal) or "array .. length is larger than" (not equals)? I think the documentation is ambiguous in this regard.
>
> I also noticed a typo in guc_tables.c: "Sets the minimal numer of constants in an array" -> number
Yep, I'll rephrase the documentation.
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pg_stat_statements and "IN" conditions
@ 2023-02-11 10:03 David Geier <[email protected]>
parent: Dmitry Dolgov <[email protected]>
0 siblings, 0 replies; 12+ messages in thread
From: David Geier @ 2023-02-11 10:03 UTC (permalink / raw)
To: Dmitry Dolgov <[email protected]>; Sergei Kornilov <[email protected]>; +Cc: Michael Paquier <[email protected]>; Alvaro Herrera <[email protected]>; Marcos Pegoraro <[email protected]>; vignesh C <[email protected]>; Robert Haas <[email protected]>; Zhihong Yu <[email protected]>; David Steele <[email protected]>; pgsql-hackers; Greg Stark <[email protected]>; Pavel Trukhanov <[email protected]>; Tom Lane <[email protected]>
Hi,
On 2/9/23 16:02, Dmitry Dolgov wrote:
>> Unfortunately, rebase is needed again due to recent changes in queryjumblefuncs ( 9ba37b2cb6a174b37fc51d0649ef73e56eae27fc )
I reviewed the last patch applied to some commit from Feb. 4th.
>> It seems a little strange to me that with const_merge_threshold = 1, such a test case gives the same result as with const_merge_threshold = 2
>>
>> select pg_stat_statements_reset();
>> set const_merge_threshold to 1;
>> select * from test where i in (1,2,3);
>> select * from test where i in (1,2);
>> select * from test where i in (1);
>> select query, calls from pg_stat_statements order by query;
>>
>> query | calls
>> -------------------------------------+-------
>> select * from test where i in (...) | 2
>> select * from test where i in ($1) | 1
>>
>> Probably const_merge_threshold = 1 should produce only "i in (...)"?
> Well, it's not intentional, probably I need to be more careful with
> off-by-one. Although I agree to a certain extent with Peter questioning
Please add tests for all the corner cases. At least for (1) IN only
contains a single element and (2) const_merge_threshold = 1.
Beyond that:
- There's a comment about find_const_walker(). I cannot find that
function anywhere. What am I missing?
- What about renaming IsConstList() to IsMergableConstList().
- Don't you intend to use the NUMERIC data column in SELECT * FROM
test_merge_numeric WHERE id IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10)?
Otherwise, the test is identical to previous test cases and you're not
checking for what happens with NUMERICs which are wrapped in FuncExpr
because of the implicit coercion.
- Don't we want to extend IsConstList() to allow a list of all
implicitly coerced constants? It's inconsistent that otherwise e.g.
NUMERICs don't work.
- Typo in /* The firsts merged constant */ (first not firsts)
- Prepared statements are not supported as they contain INs with Param
instead of Const nodes. While less likely, I've seen applications that
use prepared statements in conjunction with queries generated through a
UI which ended up with tons of prepared queries with different number of
elements in the IN clause. Not necessarily something that must go into
this patch but maybe worth thinking about.
- The setting name const_merge_threshold is not very telling without
knowing the context. While being a little longer, what about
jumble_const_merge_threshold or queryid_const_merge_threshold or similar?
- Why do we actually only want to merge constants? Why don't we ignore
the type of element in the IN and merge whatever is there? Is this
because the original jumbling logic as of now only has support for
constants?
- Ideally we would even remove duplicates. That would even improve
cardinality estimation but I guess we don't want to spend the cycles on
doing so in the planner?
- Why did you change palloc() to palloc0() for clocations array? The
length is initialized to 0 and FWICS RecordConstLocation() initializes
all members. Seems to me like we don't have to spend these cycles.
- Can't the logic at the end of IsConstList() not be simplified to:
foreach(temp, elements)
if (!IsA(lfirst(temp), Const))
return false;
// All elements are of type Const
*firstConst = linitial_node(Const, elements);
*lastConst = llast_node(Const, elements);
return true;
--
David Geier
(ServiceNow)
^ permalink raw reply [nested|flat] 12+ messages in thread
end of thread, other threads:[~2023-02-11 10:03 UTC | newest]
Thread overview: 12+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-08-06 15:56 [PATCH 5/8] add special pg_brin_bloom_summary data type Tomas Vondra <[email protected]>
2023-02-02 14:07 Re: pg_stat_statements and "IN" conditions Alvaro Herrera <[email protected]>
2023-02-02 15:05 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
2023-02-04 17:08 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
2023-02-05 01:30 ` Re: pg_stat_statements and "IN" conditions Michael Paquier <[email protected]>
2023-02-05 11:33 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
2023-02-05 16:02 ` Re: pg_stat_statements and "IN" conditions Tom Lane <[email protected]>
2023-02-05 19:56 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
2023-02-07 20:14 ` Re:pg_stat_statements and "IN" conditions Sergei Kornilov <[email protected]>
2023-02-09 13:30 ` Re: pg_stat_statements and "IN" conditions Peter Eisentraut <[email protected]>
2023-02-09 15:02 ` Re: pg_stat_statements and "IN" conditions Dmitry Dolgov <[email protected]>
2023-02-11 10:03 ` Re: pg_stat_statements and "IN" conditions David Geier <[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