public inbox for [email protected]
help / color / mirror / Atom feed[PATCH 7/8] Ignore correlation for new BRIN opclasses
3+ messages / 3 participants
[nested] [flat]
* [PATCH 7/8] Ignore correlation for new BRIN opclasses
@ 2020-09-12 13:07 Tomas Vondra <[email protected]>
0 siblings, 0 replies; 3+ messages in thread
From: Tomas Vondra @ 2020-09-12 13:07 UTC (permalink / raw)
The new BRIN opclasses (bloom and minmax-multi) are less sensitive to
poorly correlated data, so just assume the data is perfectly correlated
during costing.
Author: Tomas Vondra <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
src/backend/access/brin/brin_bloom.c | 1 +
src/backend/access/brin/brin_minmax_multi.c | 1 +
src/backend/utils/adt/selfuncs.c | 19 ++++++++++++++++++-
src/include/access/brin_internal.h | 3 +++
4 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/src/backend/access/brin/brin_bloom.c b/src/backend/access/brin/brin_bloom.c
index c086e83236..22590b2351 100644
--- a/src/backend/access/brin/brin_bloom.c
+++ b/src/backend/access/brin/brin_bloom.c
@@ -412,6 +412,7 @@ brin_bloom_opcinfo(PG_FUNCTION_ARGS)
result = palloc0(MAXALIGN(SizeofBrinOpcInfo(1)) +
sizeof(BloomOpaque));
+ result->oi_ignore_correlation = true;
result->oi_nstored = 1;
result->oi_regular_nulls = true;
result->oi_opaque = (BloomOpaque *)
diff --git a/src/backend/access/brin/brin_minmax_multi.c b/src/backend/access/brin/brin_minmax_multi.c
index d19ebf08c2..6957be0bdb 100644
--- a/src/backend/access/brin/brin_minmax_multi.c
+++ b/src/backend/access/brin/brin_minmax_multi.c
@@ -1737,6 +1737,7 @@ brin_minmax_multi_opcinfo(PG_FUNCTION_ARGS)
result = palloc0(MAXALIGN(SizeofBrinOpcInfo(1)) +
sizeof(MinmaxMultiOpaque));
+ result->oi_ignore_correlation = true;
result->oi_nstored = 1;
result->oi_regular_nulls = true;
result->oi_opaque = (MinmaxMultiOpaque *)
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index 52314d3aa1..0320d128f6 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -98,6 +98,7 @@
#include <math.h>
#include "access/brin.h"
+#include "access/brin_internal.h"
#include "access/brin_page.h"
#include "access/gin.h"
#include "access/table.h"
@@ -7352,7 +7353,8 @@ brincostestimate(PlannerInfo *root, IndexPath *path, double loop_count,
double minimalRanges;
double estimatedRanges;
double selec;
- Relation indexRel;
+ Relation indexRel = NULL;
+ TupleDesc tupdesc = NULL;
ListCell *l;
VariableStatData vardata;
@@ -7374,6 +7376,7 @@ brincostestimate(PlannerInfo *root, IndexPath *path, double loop_count,
*/
indexRel = index_open(index->indexoid, NoLock);
brinGetStats(indexRel, &statsData);
+ tupdesc = RelationGetDescr(indexRel);
index_close(indexRel, NoLock);
/* work out the actual number of ranges in the index */
@@ -7407,6 +7410,17 @@ brincostestimate(PlannerInfo *root, IndexPath *path, double loop_count,
{
IndexClause *iclause = lfirst_node(IndexClause, l);
AttrNumber attnum = index->indexkeys[iclause->indexcol];
+ FmgrInfo *opcInfoFn;
+ BrinOpcInfo *opcInfo;
+ Form_pg_attribute attr = TupleDescAttr(tupdesc, iclause->indexcol);
+ bool ignore_correlation;
+
+ opcInfoFn = index_getprocinfo(indexRel, iclause->indexcol + 1, BRIN_PROCNUM_OPCINFO);
+
+ opcInfo = (BrinOpcInfo *)
+ DatumGetPointer(FunctionCall1(opcInfoFn, attr->atttypid));
+
+ ignore_correlation = opcInfo->oi_ignore_correlation;
/* attempt to lookup stats in relation for this index column */
if (attnum != 0)
@@ -7477,6 +7491,9 @@ brincostestimate(PlannerInfo *root, IndexPath *path, double loop_count,
if (sslot.nnumbers > 0)
varCorrelation = Abs(sslot.numbers[0]);
+ if (ignore_correlation)
+ varCorrelation = 1.0;
+
if (varCorrelation > *indexCorrelation)
*indexCorrelation = varCorrelation;
diff --git a/src/include/access/brin_internal.h b/src/include/access/brin_internal.h
index 89254b5766..063b703208 100644
--- a/src/include/access/brin_internal.h
+++ b/src/include/access/brin_internal.h
@@ -30,6 +30,9 @@ typedef struct BrinOpcInfo
/* Regular processing of NULLs in BrinValues? */
bool oi_regular_nulls;
+ /* Ignore correlation during cost estimation */
+ bool oi_ignore_correlation;
+
/* Opaque pointer for the opclass' private use */
void *oi_opaque;
--
2.26.2
--------------D524CD37E77B36A2883A7617
Content-Type: text/x-patch; charset=UTF-8;
name="0008-Consider-selectivity-in-choose_bitmap_and-20210311.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename*0="0008-Consider-selectivity-in-choose_bitmap_and-20210311.patc";
filename*1="h"
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Readd use of TAP subtests
@ 2022-02-24 10:16 Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 3+ messages in thread
From: Peter Eisentraut @ 2022-02-24 10:16 UTC (permalink / raw)
To: pgsql-hackers; +Cc: Andrew Dunstan <[email protected]>; Daniel Gustafsson <[email protected]>; Tom Lane <[email protected]>; Dagfinn Ilmari Mannsåker <[email protected]>
Now that we have switched everything to done_testing(), the subtests
feature isn't that relevant anymore, but it might still be useful to get
better output when running with PROVE_FLAGS=--verbose. Compare before:
t/001_basic.pl ..
1..8
ok 1 - vacuumlo --help exit code 0
ok 2 - vacuumlo --help goes to stdout
ok 3 - vacuumlo --help nothing to stderr
ok 4 - vacuumlo --version exit code 0
ok 5 - vacuumlo --version goes to stdout
ok 6 - vacuumlo --version nothing to stderr
ok 7 - vacuumlo with invalid option nonzero exit code
ok 8 - vacuumlo with invalid option prints error message
ok
All tests successful.
Files=1, Tests=8, 0 wallclock secs ( 0.02 usr 0.00 sys + 0.08 cusr
0.05 csys = 0.15 CPU)
Result: PASS
After (with attached patch):
t/001_basic.pl ..
# Subtest: vacuumlo --help
ok 1 - exit code 0
ok 2 - goes to stdout
ok 3 - nothing to stderr
1..3
ok 1 - vacuumlo --help
# Subtest: vacuumlo --version
ok 1 - exit code 0
ok 2 - goes to stdout
ok 3 - nothing to stderr
1..3
ok 2 - vacuumlo --version
# Subtest: vacuumlo options handling
ok 1 - invalid option nonzero exit code
ok 2 - invalid option prints error message
1..2
ok 3 - vacuumlo options handling
1..3
ok
All tests successful.
Files=1, Tests=3, 0 wallclock secs ( 0.02 usr 0.01 sys + 0.11 cusr
0.07 csys = 0.21 CPU)
Result: PASS
I think for deeply nested tests and test routines defined in other
modules, this helps structure the test output more like the test source
code is laid out, so it makes following the tests and locating failing
test code easier.
From 3f50bc5236d7793939904222a38f7e13a2cda47c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Thu, 24 Feb 2022 11:01:47 +0100
Subject: [PATCH v2] Readd use of TAP subtests
Since 405f32fc49609eb94fa39e7b5e7c1fe2bb2b73aa, Test::More must be new
enough to support subtests.
The present patch effectively reverts
7912f9b7dc9e2d3f6cd81892ef6aa797578e9f06. Many more refactorings like
this are possible; this is just to get started.
---
src/test/perl/PostgreSQL/Test/Cluster.pm | 14 +++---
src/test/perl/PostgreSQL/Test/Utils.pm | 62 +++++++++++++-----------
2 files changed, 43 insertions(+), 33 deletions(-)
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index be05845248..722bdf8a36 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -2473,14 +2473,16 @@ sub issues_sql_like
my ($self, $cmd, $expected_sql, $test_name) = @_;
- local %ENV = $self->_get_env();
+ subtest $test_name => sub {
+ local %ENV = $self->_get_env();
- my $log_location = -s $self->logfile;
+ my $log_location = -s $self->logfile;
- my $result = PostgreSQL::Test::Utils::run_log($cmd);
- ok($result, "@$cmd exit code 0");
- my $log = PostgreSQL::Test::Utils::slurp_file($self->logfile, $log_location);
- like($log, $expected_sql, "$test_name: SQL found in server log");
+ my $result = PostgreSQL::Test::Utils::run_log($cmd);
+ ok($result, "@$cmd exit code 0");
+ my $log = PostgreSQL::Test::Utils::slurp_file($self->logfile, $log_location);
+ like($log, $expected_sql, "SQL found in server log");
+ };
return;
}
diff --git a/src/test/perl/PostgreSQL/Test/Utils.pm b/src/test/perl/PostgreSQL/Test/Utils.pm
index 46cd746796..5869f060ee 100644
--- a/src/test/perl/PostgreSQL/Test/Utils.pm
+++ b/src/test/perl/PostgreSQL/Test/Utils.pm
@@ -792,13 +792,15 @@ sub program_help_ok
{
local $Test::Builder::Level = $Test::Builder::Level + 1;
my ($cmd) = @_;
- my ($stdout, $stderr);
- print("# Running: $cmd --help\n");
- my $result = IPC::Run::run [ $cmd, '--help' ], '>', \$stdout, '2>',
- \$stderr;
- ok($result, "$cmd --help exit code 0");
- isnt($stdout, '', "$cmd --help goes to stdout");
- is($stderr, '', "$cmd --help nothing to stderr");
+ subtest "$cmd --help" => sub {
+ my ($stdout, $stderr);
+ print("# Running: $cmd --help\n");
+ my $result = IPC::Run::run [ $cmd, '--help' ], '>', \$stdout, '2>',
+ \$stderr;
+ ok($result, "exit code 0");
+ isnt($stdout, '', "goes to stdout");
+ is($stderr, '', "nothing to stderr");
+ };
return;
}
@@ -814,13 +816,15 @@ sub program_version_ok
{
local $Test::Builder::Level = $Test::Builder::Level + 1;
my ($cmd) = @_;
- my ($stdout, $stderr);
- print("# Running: $cmd --version\n");
- my $result = IPC::Run::run [ $cmd, '--version' ], '>', \$stdout, '2>',
- \$stderr;
- ok($result, "$cmd --version exit code 0");
- isnt($stdout, '', "$cmd --version goes to stdout");
- is($stderr, '', "$cmd --version nothing to stderr");
+ subtest "$cmd --version" => sub {
+ my ($stdout, $stderr);
+ print("# Running: $cmd --version\n");
+ my $result = IPC::Run::run [ $cmd, '--version' ], '>', \$stdout, '2>',
+ \$stderr;
+ ok($result, "exit code 0");
+ isnt($stdout, '', "goes to stdout");
+ is($stderr, '', "nothing to stderr");
+ };
return;
}
@@ -837,13 +841,15 @@ sub program_options_handling_ok
{
local $Test::Builder::Level = $Test::Builder::Level + 1;
my ($cmd) = @_;
- my ($stdout, $stderr);
- print("# Running: $cmd --not-a-valid-option\n");
- my $result = IPC::Run::run [ $cmd, '--not-a-valid-option' ], '>',
- \$stdout,
- '2>', \$stderr;
- ok(!$result, "$cmd with invalid option nonzero exit code");
- isnt($stderr, '', "$cmd with invalid option prints error message");
+ subtest "$cmd options handling" => sub {
+ my ($stdout, $stderr);
+ print("# Running: $cmd --not-a-valid-option\n");
+ my $result = IPC::Run::run [ $cmd, '--not-a-valid-option' ], '>',
+ \$stdout,
+ '2>', \$stderr;
+ ok(!$result, "invalid option nonzero exit code");
+ isnt($stderr, '', "invalid option prints error message");
+ };
return;
}
@@ -860,12 +866,14 @@ sub command_like
{
local $Test::Builder::Level = $Test::Builder::Level + 1;
my ($cmd, $expected_stdout, $test_name) = @_;
- my ($stdout, $stderr);
- print("# Running: " . join(" ", @{$cmd}) . "\n");
- my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr;
- ok($result, "$test_name: exit code 0");
- is($stderr, '', "$test_name: no stderr");
- like($stdout, $expected_stdout, "$test_name: matches");
+ subtest $test_name => sub {
+ my ($stdout, $stderr);
+ print("# Running: " . join(" ", @{$cmd}) . "\n");
+ my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr;
+ ok($result, "exit code 0");
+ is($stderr, '', "no stderr");
+ like($stdout, $expected_stdout, "stdout matches");
+ };
return;
}
--
2.35.1
Attachments:
[text/plain] v2-0001-Readd-use-of-TAP-subtests.patch (4.8K, ../../[email protected]/2-v2-0001-Readd-use-of-TAP-subtests.patch)
download | inline diff:
From 3f50bc5236d7793939904222a38f7e13a2cda47c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Thu, 24 Feb 2022 11:01:47 +0100
Subject: [PATCH v2] Readd use of TAP subtests
Since 405f32fc49609eb94fa39e7b5e7c1fe2bb2b73aa, Test::More must be new
enough to support subtests.
The present patch effectively reverts
7912f9b7dc9e2d3f6cd81892ef6aa797578e9f06. Many more refactorings like
this are possible; this is just to get started.
---
src/test/perl/PostgreSQL/Test/Cluster.pm | 14 +++---
src/test/perl/PostgreSQL/Test/Utils.pm | 62 +++++++++++++-----------
2 files changed, 43 insertions(+), 33 deletions(-)
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index be05845248..722bdf8a36 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -2473,14 +2473,16 @@ sub issues_sql_like
my ($self, $cmd, $expected_sql, $test_name) = @_;
- local %ENV = $self->_get_env();
+ subtest $test_name => sub {
+ local %ENV = $self->_get_env();
- my $log_location = -s $self->logfile;
+ my $log_location = -s $self->logfile;
- my $result = PostgreSQL::Test::Utils::run_log($cmd);
- ok($result, "@$cmd exit code 0");
- my $log = PostgreSQL::Test::Utils::slurp_file($self->logfile, $log_location);
- like($log, $expected_sql, "$test_name: SQL found in server log");
+ my $result = PostgreSQL::Test::Utils::run_log($cmd);
+ ok($result, "@$cmd exit code 0");
+ my $log = PostgreSQL::Test::Utils::slurp_file($self->logfile, $log_location);
+ like($log, $expected_sql, "SQL found in server log");
+ };
return;
}
diff --git a/src/test/perl/PostgreSQL/Test/Utils.pm b/src/test/perl/PostgreSQL/Test/Utils.pm
index 46cd746796..5869f060ee 100644
--- a/src/test/perl/PostgreSQL/Test/Utils.pm
+++ b/src/test/perl/PostgreSQL/Test/Utils.pm
@@ -792,13 +792,15 @@ sub program_help_ok
{
local $Test::Builder::Level = $Test::Builder::Level + 1;
my ($cmd) = @_;
- my ($stdout, $stderr);
- print("# Running: $cmd --help\n");
- my $result = IPC::Run::run [ $cmd, '--help' ], '>', \$stdout, '2>',
- \$stderr;
- ok($result, "$cmd --help exit code 0");
- isnt($stdout, '', "$cmd --help goes to stdout");
- is($stderr, '', "$cmd --help nothing to stderr");
+ subtest "$cmd --help" => sub {
+ my ($stdout, $stderr);
+ print("# Running: $cmd --help\n");
+ my $result = IPC::Run::run [ $cmd, '--help' ], '>', \$stdout, '2>',
+ \$stderr;
+ ok($result, "exit code 0");
+ isnt($stdout, '', "goes to stdout");
+ is($stderr, '', "nothing to stderr");
+ };
return;
}
@@ -814,13 +816,15 @@ sub program_version_ok
{
local $Test::Builder::Level = $Test::Builder::Level + 1;
my ($cmd) = @_;
- my ($stdout, $stderr);
- print("# Running: $cmd --version\n");
- my $result = IPC::Run::run [ $cmd, '--version' ], '>', \$stdout, '2>',
- \$stderr;
- ok($result, "$cmd --version exit code 0");
- isnt($stdout, '', "$cmd --version goes to stdout");
- is($stderr, '', "$cmd --version nothing to stderr");
+ subtest "$cmd --version" => sub {
+ my ($stdout, $stderr);
+ print("# Running: $cmd --version\n");
+ my $result = IPC::Run::run [ $cmd, '--version' ], '>', \$stdout, '2>',
+ \$stderr;
+ ok($result, "exit code 0");
+ isnt($stdout, '', "goes to stdout");
+ is($stderr, '', "nothing to stderr");
+ };
return;
}
@@ -837,13 +841,15 @@ sub program_options_handling_ok
{
local $Test::Builder::Level = $Test::Builder::Level + 1;
my ($cmd) = @_;
- my ($stdout, $stderr);
- print("# Running: $cmd --not-a-valid-option\n");
- my $result = IPC::Run::run [ $cmd, '--not-a-valid-option' ], '>',
- \$stdout,
- '2>', \$stderr;
- ok(!$result, "$cmd with invalid option nonzero exit code");
- isnt($stderr, '', "$cmd with invalid option prints error message");
+ subtest "$cmd options handling" => sub {
+ my ($stdout, $stderr);
+ print("# Running: $cmd --not-a-valid-option\n");
+ my $result = IPC::Run::run [ $cmd, '--not-a-valid-option' ], '>',
+ \$stdout,
+ '2>', \$stderr;
+ ok(!$result, "invalid option nonzero exit code");
+ isnt($stderr, '', "invalid option prints error message");
+ };
return;
}
@@ -860,12 +866,14 @@ sub command_like
{
local $Test::Builder::Level = $Test::Builder::Level + 1;
my ($cmd, $expected_stdout, $test_name) = @_;
- my ($stdout, $stderr);
- print("# Running: " . join(" ", @{$cmd}) . "\n");
- my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr;
- ok($result, "$test_name: exit code 0");
- is($stderr, '', "$test_name: no stderr");
- like($stdout, $expected_stdout, "$test_name: matches");
+ subtest $test_name => sub {
+ my ($stdout, $stderr);
+ print("# Running: " . join(" ", @{$cmd}) . "\n");
+ my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr;
+ ok($result, "exit code 0");
+ is($stderr, '', "no stderr");
+ like($stdout, $expected_stdout, "stdout matches");
+ };
return;
}
--
2.35.1
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Readd use of TAP subtests
@ 2022-02-24 13:20 Daniel Gustafsson <[email protected]>
parent: Peter Eisentraut <[email protected]>
0 siblings, 0 replies; 3+ messages in thread
From: Daniel Gustafsson @ 2022-02-24 13:20 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: pgsql-hackers; Andrew Dunstan <[email protected]>; Tom Lane <[email protected]>; Dagfinn Ilmari Mannsåker <[email protected]>
> On 24 Feb 2022, at 11:16, Peter Eisentraut <[email protected]> wrote:
> I think for deeply nested tests and test routines defined in other modules,
> this helps structure the test output more like the test source code is laid
> out, so it makes following the tests and locating failing test code easier.
I don't have any strong opinions on this, but if we go ahead with it I think
there should be a short note in src/test/perl/README about when substest could
be considered.
--
Daniel Gustafsson https://vmware.com/
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2022-02-24 13:20 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-09-12 13:07 [PATCH 7/8] Ignore correlation for new BRIN opclasses Tomas Vondra <[email protected]>
2022-02-24 10:16 Re: Readd use of TAP subtests Peter Eisentraut <[email protected]>
2022-02-24 13:20 ` Re: Readd use of TAP subtests Daniel Gustafsson <[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