pg.ddx.io pgsql-committers@postgresql.org mailing list archive
help / color / mirror / Atom feedpgsql: Improve BRIN minmax-multi opclass test coverage
3+ messages / 2 participants
[nested] [flat]
Commits from this thread:
2023-11-25 86b64bafc1 Use fipshash in brin_multi test
* pgsql: Improve BRIN minmax-multi opclass test coverage
@ 2023-07-02 08:34 Tomas Vondra <tomas.vondra@postgresql.org>
2023-08-22 06:43 ` Re: pgsql: Improve BRIN minmax-multi opclass test coverage Peter Eisentraut <peter@eisentraut.org>
0 siblings, 1 reply; 3+ messages in thread
From: Tomas Vondra @ 2023-07-02 08:34 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Improve BRIN minmax-multi opclass test coverage
Per the code coverage report, the existing regression tests did not
exercice some a couple important BRIN minmax-multi code paths.
- The tests focused on testing planning with a range of scan key
strategies, but not the execution. Fixed by adding queries that
actually test query execution for both equality and inequality.
- All tests created indexes after inserting data, but this only
exercises the CREATE INDEX strategy that sees all values at once, not
incremental summary updates. The new tests flip the order and create
the index before adding data.
- The assert check(s) validating correctness of expanded ranges were
present only in the "union" code path, which is not covered by
regression tests at all (as it requires concurrency etc.). Fixed by
adding the asserts to a couple more places.
Reviewed-by: Heikki Linnakangas
Discussion: https://postgr.es/m/57020b2e-d9c9-9bc7-4892-b36d9bb07563%40enterprisedb.com
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/0457109344b46f481f9bf09b85674695ba57c8e4
Modified Files
--------------
src/backend/access/brin/brin_minmax_multi.c | 9 +
src/test/regress/expected/brin_multi.out | 357 ++++++++++++++++++++++++++++
src/test/regress/sql/brin_multi.sql | 165 +++++++++++++
3 files changed, 531 insertions(+)
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: pgsql: Improve BRIN minmax-multi opclass test coverage
2023-07-02 08:34 pgsql: Improve BRIN minmax-multi opclass test coverage Tomas Vondra <tomas.vondra@postgresql.org>
@ 2023-08-22 06:43 ` Peter Eisentraut <peter@eisentraut.org>
2023-11-23 07:46 ` Re: pgsql: Improve BRIN minmax-multi opclass test coverage Peter Eisentraut <peter@eisentraut.org>
0 siblings, 1 reply; 3+ messages in thread
From: Peter Eisentraut @ 2023-08-22 06:43 UTC (permalink / raw)
To: Tomas Vondra <tomas.vondra@postgresql.org>; pgsql-committers@lists.postgresql.org
These new tests introduced a few new calls of the md5() function. This
should be avoided (see commit 208bf364a9). You can replace these easily
with the fipshash() function instead, but I think you then also need to
change some of the constants in the tests. Could you look at this again?
On 02.07.23 10:34, Tomas Vondra wrote:
> Improve BRIN minmax-multi opclass test coverage
>
> Per the code coverage report, the existing regression tests did not
> exercice some a couple important BRIN minmax-multi code paths.
>
> - The tests focused on testing planning with a range of scan key
> strategies, but not the execution. Fixed by adding queries that
> actually test query execution for both equality and inequality.
>
> - All tests created indexes after inserting data, but this only
> exercises the CREATE INDEX strategy that sees all values at once, not
> incremental summary updates. The new tests flip the order and create
> the index before adding data.
>
> - The assert check(s) validating correctness of expanded ranges were
> present only in the "union" code path, which is not covered by
> regression tests at all (as it requires concurrency etc.). Fixed by
> adding the asserts to a couple more places.
>
> Reviewed-by: Heikki Linnakangas
> Discussion: https://postgr.es/m/57020b2e-d9c9-9bc7-4892-b36d9bb07563%40enterprisedb.com
>
> Branch
> ------
> master
>
> Details
> -------
> https://git.postgresql.org/pg/commitdiff/0457109344b46f481f9bf09b85674695ba57c8e4
>
> Modified Files
> --------------
> src/backend/access/brin/brin_minmax_multi.c | 9 +
> src/test/regress/expected/brin_multi.out | 357 ++++++++++++++++++++++++++++
> src/test/regress/sql/brin_multi.sql | 165 +++++++++++++
> 3 files changed, 531 insertions(+)
>
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: pgsql: Improve BRIN minmax-multi opclass test coverage
2023-07-02 08:34 pgsql: Improve BRIN minmax-multi opclass test coverage Tomas Vondra <tomas.vondra@postgresql.org>
2023-08-22 06:43 ` Re: pgsql: Improve BRIN minmax-multi opclass test coverage Peter Eisentraut <peter@eisentraut.org>
@ 2023-11-23 07:46 ` Peter Eisentraut <peter@eisentraut.org>
0 siblings, 0 replies; 3+ messages in thread
From: Peter Eisentraut @ 2023-11-23 07:46 UTC (permalink / raw)
To: Tomas Vondra <tomas.vondra@postgresql.org>; pgsql-committers@lists.postgresql.org
On 22.08.23 08:43, Peter Eisentraut wrote:
> These new tests introduced a few new calls of the md5() function. This
> should be avoided (see commit 208bf364a9). You can replace these easily
> with the fipshash() function instead, but I think you then also need to
> change some of the constants in the tests. Could you look at this again?
I have created a patch that updates the test accordingly. It replaces
the md5() function with fipshash() and adjusts the test values to
correspond to the values generated in the original tests.
Attachments:
0001-Use-fipshash-in-brin_multi-test.patch (text/plain, 8.3K) download
From 8f33ef19f5e360ce013bf94c13b7a6ebf3be3ee9 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Thu, 23 Nov 2023 08:43:07 +0100
Subject: [PATCH] Use fipshash in brin_multi test
This fixes some md5() calls that snuck in in 0457109344 after we had
removed them all in 208bf364a9.
Discussion: https://www.postgresql.org/message-id/a35eaf0f-b19c-7797-e296-7b7e2adc473e@eisentraut.org
---
src/test/regress/expected/brin_multi.out | 32 ++++++++++++------------
src/test/regress/sql/brin_multi.sql | 28 ++++++++++-----------
2 files changed, 30 insertions(+), 30 deletions(-)
diff --git a/src/test/regress/expected/brin_multi.out b/src/test/regress/expected/brin_multi.out
index 8f4c95b9e6..7df42865da 100644
--- a/src/test/regress/expected/brin_multi.out
+++ b/src/test/regress/expected/brin_multi.out
@@ -740,40 +740,40 @@ RESET enable_seqscan;
-- do some inequality tests for varlena data types
CREATE TABLE brin_test_multi_2 (a UUID) WITH (fillfactor=10);
INSERT INTO brin_test_multi_2
-SELECT v::uuid FROM (SELECT row_number() OVER (ORDER BY v) c, v FROM (SELECT md5((i/13)::text) AS v FROM generate_series(1,1000) s(i)) foo) bar ORDER BY c + 25 * random();
+SELECT v::uuid FROM (SELECT row_number() OVER (ORDER BY v) c, v FROM (SELECT fipshash((i/13)::text) AS v FROM generate_series(1,1000) s(i)) foo) bar ORDER BY c + 25 * random();
CREATE INDEX brin_test_multi_2_idx ON brin_test_multi_2 USING brin (a uuid_minmax_multi_ops) WITH (pages_per_range=5);
SET enable_seqscan=off;
-SELECT COUNT(*) FROM brin_test_multi_2 WHERE a < '33e75ff0-9dd6-01bb-e69f-351039152189';
+SELECT COUNT(*) FROM brin_test_multi_2 WHERE a < '3d914f93-48c9-cc0f-f8a7-9716700b9fcd';
count
-------
195
(1 row)
-SELECT COUNT(*) FROM brin_test_multi_2 WHERE a > '33e75ff0-9dd6-01bb-e69f-351039152189';
+SELECT COUNT(*) FROM brin_test_multi_2 WHERE a > '3d914f93-48c9-cc0f-f8a7-9716700b9fcd';
count
-------
792
(1 row)
-SELECT COUNT(*) FROM brin_test_multi_2 WHERE a <= 'f457c545-a9de-d88f-18ec-ee47145a72c0';
+SELECT COUNT(*) FROM brin_test_multi_2 WHERE a <= 'f369cb89-fc62-7e66-8987-007d121ed1ea';
count
-------
961
(1 row)
-SELECT COUNT(*) FROM brin_test_multi_2 WHERE a >= 'c51ce410-c124-a10e-0db5-e4b97fc2af39';
+SELECT COUNT(*) FROM brin_test_multi_2 WHERE a >= 'aea92132-c4cb-eb26-3e6a-c2bf6c183b5d';
count
-------
- 272
+ 273
(1 row)
-SELECT COUNT(*) FROM brin_test_multi_2 WHERE a = 'cfcd2084-95d5-65ef-66e7-dff9f98764da';
+SELECT COUNT(*) FROM brin_test_multi_2 WHERE a = '5feceb66-ffc8-6f38-d952-786c6d696c79';
count
-------
12
(1 row)
-SELECT COUNT(*) FROM brin_test_multi_2 WHERE a = 'aab32389-22bc-c25a-6f60-6eb525ffdc56';
+SELECT COUNT(*) FROM brin_test_multi_2 WHERE a = '86e50149-6586-6131-2a9e-0b35558d84f6';
count
-------
13
@@ -784,38 +784,38 @@ SELECT COUNT(*) FROM brin_test_multi_2 WHERE a = 'aab32389-22bc-c25a-6f60-6eb525
-- approach of adding rows into existing ranges
TRUNCATE brin_test_multi_2;
INSERT INTO brin_test_multi_2
-SELECT v::uuid FROM (SELECT row_number() OVER (ORDER BY v) c, v FROM (SELECT md5((i/13)::text) AS v FROM generate_series(1,1000) s(i)) foo) bar ORDER BY c + 25 * random();
-SELECT COUNT(*) FROM brin_test_multi_2 WHERE a < '33e75ff0-9dd6-01bb-e69f-351039152189';
+SELECT v::uuid FROM (SELECT row_number() OVER (ORDER BY v) c, v FROM (SELECT fipshash((i/13)::text) AS v FROM generate_series(1,1000) s(i)) foo) bar ORDER BY c + 25 * random();
+SELECT COUNT(*) FROM brin_test_multi_2 WHERE a < '3d914f93-48c9-cc0f-f8a7-9716700b9fcd';
count
-------
195
(1 row)
-SELECT COUNT(*) FROM brin_test_multi_2 WHERE a > '33e75ff0-9dd6-01bb-e69f-351039152189';
+SELECT COUNT(*) FROM brin_test_multi_2 WHERE a > '3d914f93-48c9-cc0f-f8a7-9716700b9fcd';
count
-------
792
(1 row)
-SELECT COUNT(*) FROM brin_test_multi_2 WHERE a <= 'f457c545-a9de-d88f-18ec-ee47145a72c0';
+SELECT COUNT(*) FROM brin_test_multi_2 WHERE a <= 'f369cb89-fc62-7e66-8987-007d121ed1ea';
count
-------
961
(1 row)
-SELECT COUNT(*) FROM brin_test_multi_2 WHERE a >= 'c51ce410-c124-a10e-0db5-e4b97fc2af39';
+SELECT COUNT(*) FROM brin_test_multi_2 WHERE a >= 'aea92132-c4cb-eb26-3e6a-c2bf6c183b5d';
count
-------
- 272
+ 273
(1 row)
-SELECT COUNT(*) FROM brin_test_multi_2 WHERE a = 'cfcd2084-95d5-65ef-66e7-dff9f98764da';
+SELECT COUNT(*) FROM brin_test_multi_2 WHERE a = '5feceb66-ffc8-6f38-d952-786c6d696c79';
count
-------
12
(1 row)
-SELECT COUNT(*) FROM brin_test_multi_2 WHERE a = 'aab32389-22bc-c25a-6f60-6eb525ffdc56';
+SELECT COUNT(*) FROM brin_test_multi_2 WHERE a = '86e50149-6586-6131-2a9e-0b35558d84f6';
count
-------
13
diff --git a/src/test/regress/sql/brin_multi.sql b/src/test/regress/sql/brin_multi.sql
index 116106d30f..c5a8484584 100644
--- a/src/test/regress/sql/brin_multi.sql
+++ b/src/test/regress/sql/brin_multi.sql
@@ -545,23 +545,23 @@ CREATE INDEX brin_test_multi_1_idx_2 ON brin_test_multi_1 USING brin (b int8_min
-- do some inequality tests for varlena data types
CREATE TABLE brin_test_multi_2 (a UUID) WITH (fillfactor=10);
INSERT INTO brin_test_multi_2
-SELECT v::uuid FROM (SELECT row_number() OVER (ORDER BY v) c, v FROM (SELECT md5((i/13)::text) AS v FROM generate_series(1,1000) s(i)) foo) bar ORDER BY c + 25 * random();
+SELECT v::uuid FROM (SELECT row_number() OVER (ORDER BY v) c, v FROM (SELECT fipshash((i/13)::text) AS v FROM generate_series(1,1000) s(i)) foo) bar ORDER BY c + 25 * random();
CREATE INDEX brin_test_multi_2_idx ON brin_test_multi_2 USING brin (a uuid_minmax_multi_ops) WITH (pages_per_range=5);
SET enable_seqscan=off;
-SELECT COUNT(*) FROM brin_test_multi_2 WHERE a < '33e75ff0-9dd6-01bb-e69f-351039152189';
+SELECT COUNT(*) FROM brin_test_multi_2 WHERE a < '3d914f93-48c9-cc0f-f8a7-9716700b9fcd';
-SELECT COUNT(*) FROM brin_test_multi_2 WHERE a > '33e75ff0-9dd6-01bb-e69f-351039152189';
+SELECT COUNT(*) FROM brin_test_multi_2 WHERE a > '3d914f93-48c9-cc0f-f8a7-9716700b9fcd';
-SELECT COUNT(*) FROM brin_test_multi_2 WHERE a <= 'f457c545-a9de-d88f-18ec-ee47145a72c0';
+SELECT COUNT(*) FROM brin_test_multi_2 WHERE a <= 'f369cb89-fc62-7e66-8987-007d121ed1ea';
-SELECT COUNT(*) FROM brin_test_multi_2 WHERE a >= 'c51ce410-c124-a10e-0db5-e4b97fc2af39';
+SELECT COUNT(*) FROM brin_test_multi_2 WHERE a >= 'aea92132-c4cb-eb26-3e6a-c2bf6c183b5d';
-SELECT COUNT(*) FROM brin_test_multi_2 WHERE a = 'cfcd2084-95d5-65ef-66e7-dff9f98764da';
+SELECT COUNT(*) FROM brin_test_multi_2 WHERE a = '5feceb66-ffc8-6f38-d952-786c6d696c79';
-SELECT COUNT(*) FROM brin_test_multi_2 WHERE a = 'aab32389-22bc-c25a-6f60-6eb525ffdc56';
+SELECT COUNT(*) FROM brin_test_multi_2 WHERE a = '86e50149-6586-6131-2a9e-0b35558d84f6';
-- now do the same, but insert the rows with the indexes already created
@@ -570,19 +570,19 @@ CREATE INDEX brin_test_multi_2_idx ON brin_test_multi_2 USING brin (a uuid_minma
TRUNCATE brin_test_multi_2;
INSERT INTO brin_test_multi_2
-SELECT v::uuid FROM (SELECT row_number() OVER (ORDER BY v) c, v FROM (SELECT md5((i/13)::text) AS v FROM generate_series(1,1000) s(i)) foo) bar ORDER BY c + 25 * random();
+SELECT v::uuid FROM (SELECT row_number() OVER (ORDER BY v) c, v FROM (SELECT fipshash((i/13)::text) AS v FROM generate_series(1,1000) s(i)) foo) bar ORDER BY c + 25 * random();
-SELECT COUNT(*) FROM brin_test_multi_2 WHERE a < '33e75ff0-9dd6-01bb-e69f-351039152189';
+SELECT COUNT(*) FROM brin_test_multi_2 WHERE a < '3d914f93-48c9-cc0f-f8a7-9716700b9fcd';
-SELECT COUNT(*) FROM brin_test_multi_2 WHERE a > '33e75ff0-9dd6-01bb-e69f-351039152189';
+SELECT COUNT(*) FROM brin_test_multi_2 WHERE a > '3d914f93-48c9-cc0f-f8a7-9716700b9fcd';
-SELECT COUNT(*) FROM brin_test_multi_2 WHERE a <= 'f457c545-a9de-d88f-18ec-ee47145a72c0';
+SELECT COUNT(*) FROM brin_test_multi_2 WHERE a <= 'f369cb89-fc62-7e66-8987-007d121ed1ea';
-SELECT COUNT(*) FROM brin_test_multi_2 WHERE a >= 'c51ce410-c124-a10e-0db5-e4b97fc2af39';
+SELECT COUNT(*) FROM brin_test_multi_2 WHERE a >= 'aea92132-c4cb-eb26-3e6a-c2bf6c183b5d';
-SELECT COUNT(*) FROM brin_test_multi_2 WHERE a = 'cfcd2084-95d5-65ef-66e7-dff9f98764da';
+SELECT COUNT(*) FROM brin_test_multi_2 WHERE a = '5feceb66-ffc8-6f38-d952-786c6d696c79';
-SELECT COUNT(*) FROM brin_test_multi_2 WHERE a = 'aab32389-22bc-c25a-6f60-6eb525ffdc56';
+SELECT COUNT(*) FROM brin_test_multi_2 WHERE a = '86e50149-6586-6131-2a9e-0b35558d84f6';
DROP TABLE brin_test_multi_2;
RESET enable_seqscan;
--
2.42.1
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2023-11-23 07:46 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-07-02 08:34 pgsql: Improve BRIN minmax-multi opclass test coverage Tomas Vondra <tomas.vondra@postgresql.org>
2023-08-22 06:43 ` Peter Eisentraut <peter@eisentraut.org>
2023-11-23 07:46 ` Peter Eisentraut <peter@eisentraut.org>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox