public inbox for [email protected]
help / color / mirror / Atom feedexplain analyze rows=%.0f
45+ messages / 18 participants
[nested] [flat]
* explain analyze rows=%.0f
@ 2009-05-29 01:30 Robert Haas <[email protected]>
0 siblings, 1 reply; 45+ messages in thread
From: Robert Haas @ 2009-05-29 01:30 UTC (permalink / raw)
To: pgsql-hackers
I have always assumed that there is some very good reason why EXPLAIN
ANALYZE reports the number of rows as an integer rather than a
floating point value, but in reading explain.c it seems that the
reason is just that we decided to round to zero decimal places. Any
chance we could reconsider this decision? I often find myself wanting
to know the value that is here called ntuples, but rounding
ntuples/nloops off to the nearest integer loses too much precision.
(Before someone mentions it, yes that would be a good thing to include
in XML-formatted explain output. But I don't see that including a
couple of decimal places would hurt the text output format either.)
...Robert
^ permalink raw reply [nested|flat] 45+ messages in thread
* Re: explain analyze rows=%.0f
@ 2009-05-29 03:00 Euler Taveira de Oliveira <[email protected]>
parent: Robert Haas <[email protected]>
0 siblings, 2 replies; 45+ messages in thread
From: Euler Taveira de Oliveira @ 2009-05-29 03:00 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: pgsql-hackers
Robert Haas escreveu:
> I have always assumed that there is some very good reason why EXPLAIN
> ANALYZE reports the number of rows as an integer rather than a
> floating point value, but in reading explain.c it seems that the
> reason is just that we decided to round to zero decimal places. Any
> chance we could reconsider this decision? I often find myself wanting
> to know the value that is here called ntuples, but rounding
> ntuples/nloops off to the nearest integer loses too much precision.
>
Don't you think is too strange having, for example, 6.67 rows? I would confuse
users and programs that parses the EXPLAIN output. However, I wouldn't object
to add ntuples to an extended explain output (as discussed in the other thread).
--
Euler Taveira de Oliveira
http://www.timbira.com/
^ permalink raw reply [nested|flat] 45+ messages in thread
* Re: explain analyze rows=%.0f
@ 2009-05-29 03:12 Robert Haas <[email protected]>
parent: Euler Taveira de Oliveira <[email protected]>
1 sibling, 1 reply; 45+ messages in thread
From: Robert Haas @ 2009-05-29 03:12 UTC (permalink / raw)
To: Euler Taveira de Oliveira <[email protected]>; +Cc: pgsql-hackers
On Thu, May 28, 2009 at 11:00 PM, Euler Taveira de Oliveira
<[email protected]> wrote:
> Robert Haas escreveu:
>> I have always assumed that there is some very good reason why EXPLAIN
>> ANALYZE reports the number of rows as an integer rather than a
>> floating point value, but in reading explain.c it seems that the
>> reason is just that we decided to round to zero decimal places. Any
>> chance we could reconsider this decision? I often find myself wanting
>> to know the value that is here called ntuples, but rounding
>> ntuples/nloops off to the nearest integer loses too much precision.
>>
> Don't you think is too strange having, for example, 6.67 rows?
No stranger than having it say 7 when it's really not. Actually mine
mostly come out 1 when the real value is somewhere between 0.5 and
1.49. :-(
...Robert
^ permalink raw reply [nested|flat] 45+ messages in thread
* Re: explain analyze rows=%.0f
@ 2009-05-29 04:16 Joshua Tolley <[email protected]>
parent: Robert Haas <[email protected]>
0 siblings, 1 reply; 45+ messages in thread
From: Joshua Tolley @ 2009-05-29 04:16 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Euler Taveira de Oliveira <[email protected]>; pgsql-hackers
On Thu, May 28, 2009 at 11:12:42PM -0400, Robert Haas wrote:
> On Thu, May 28, 2009 at 11:00 PM, Euler Taveira de Oliveira
> > Don't you think is too strange having, for example, 6.67 rows?
>
> No stranger than having it say 7 when it's really not. Actually mine
> mostly come out 1 when the real value is somewhere between 0.5 and
> 1.49. :-(
+1. It would help users realize more quickly that some of the values in the
EXPLAIN output are, for instance, *average* number of rows *per iteration* of a
nested loop, say, rather than total rows found in all loops. That's an
important distinction that isn't immediately clear to the novice EXPLAIN
reader, but would become so very quickly as users tried to figure out how a
scan could come up with a fractional row.
- Josh / eggyknap
Attachments:
[application/pgp-signature] signature.asc (197B, ../../20090529041623.GE15201@eddie/2-signature.asc)
download
^ permalink raw reply [nested|flat] 45+ messages in thread
* Re: explain analyze rows=%.0f
@ 2009-05-29 17:30 Tom Lane <[email protected]>
parent: Joshua Tolley <[email protected]>
0 siblings, 1 reply; 45+ messages in thread
From: Tom Lane @ 2009-05-29 17:30 UTC (permalink / raw)
To: Joshua Tolley <[email protected]>; +Cc: Robert Haas <[email protected]>; Euler Taveira de Oliveira <[email protected]>; pgsql-hackers
Joshua Tolley <[email protected]> writes:
> On Thu, May 28, 2009 at 11:12:42PM -0400, Robert Haas wrote:
>> On Thu, May 28, 2009 at 11:00 PM, Euler Taveira de Oliveira
>>> Don't you think is too strange having, for example, 6.67 rows?
>>
>> No stranger than having it say 7 when it's really not. Actually mine
>> mostly come out 1 when the real value is somewhere between 0.5 and
>> 1.49. :-(
> +1. It would help users realize more quickly that some of the values in the
> EXPLAIN output are, for instance, *average* number of rows *per iteration* of a
> nested loop, say, rather than total rows found in all loops.
I think it would only be sensible to show fractional digits if nloops is
greater than 1. Otherwise the value must in fact be an integer, and
you're just going to confuse people more by suggesting that it might not
be.
regards, tom lane
^ permalink raw reply [nested|flat] 45+ messages in thread
* Re: explain analyze rows=%.0f
@ 2009-05-29 18:39 Robert Haas <[email protected]>
parent: Tom Lane <[email protected]>
0 siblings, 0 replies; 45+ messages in thread
From: Robert Haas @ 2009-05-29 18:39 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Joshua Tolley <[email protected]>; Euler Taveira de Oliveira <[email protected]>; pgsql-hackers
On Fri, May 29, 2009 at 1:30 PM, Tom Lane <[email protected]> wrote:
> Joshua Tolley <[email protected]> writes:
>> On Thu, May 28, 2009 at 11:12:42PM -0400, Robert Haas wrote:
>>> On Thu, May 28, 2009 at 11:00 PM, Euler Taveira de Oliveira
>>>> Don't you think is too strange having, for example, 6.67 rows?
>>>
>>> No stranger than having it say 7 when it's really not. Actually mine
>>> mostly come out 1 when the real value is somewhere between 0.5 and
>>> 1.49. :-(
>
>> +1. It would help users realize more quickly that some of the values in the
>> EXPLAIN output are, for instance, *average* number of rows *per iteration* of a
>> nested loop, say, rather than total rows found in all loops.
>
> I think it would only be sensible to show fractional digits if nloops is
> greater than 1. Otherwise the value must in fact be an integer, and
> you're just going to confuse people more by suggesting that it might not
> be.
That might be over-engineering, but I'll take it.
...Robert
^ permalink raw reply [nested|flat] 45+ messages in thread
* Re: explain analyze rows=%.0f
@ 2009-06-02 03:30 Ron Mayer <[email protected]>
parent: Euler Taveira de Oliveira <[email protected]>
1 sibling, 1 reply; 45+ messages in thread
From: Ron Mayer @ 2009-06-02 03:30 UTC (permalink / raw)
To: Euler Taveira de Oliveira <[email protected]>; +Cc: Robert Haas <[email protected]>; pgsql-hackers
Euler Taveira de Oliveira wrote:
> Robert Haas escreveu:
>> ...EXPLAIN ANALYZE reports the number of rows as an integer... Any
>> chance we could reconsider this decision? I often find myself wanting
>> to know the value that is here called ntuples, but rounding
>> ntuples/nloops off to the nearest integer loses too much precision.
>>
> Don't you think is too strange having, for example, 6.67 rows? I would confuse
> users and programs that parses the EXPLAIN output. However, I wouldn't object
I don't think it's that confusing. If it says "0.1 rows", I imagine most
people would infer that this means "typically 0, but sometimes 1 or a few" rows.
What I'd find strange about "6.67 rows" in your example is more that on
the estimated rows side, it seems to imply an unrealistically precise estimate
in the same way that "667 rows" would seem unrealistically precise to me.
Maybe rounding to 2 significant digits would reduce confusion?
^ permalink raw reply [nested|flat] 45+ messages in thread
* Re: explain analyze rows=%.0f
@ 2009-06-02 13:41 Simon Riggs <[email protected]>
parent: Ron Mayer <[email protected]>
0 siblings, 1 reply; 45+ messages in thread
From: Simon Riggs @ 2009-06-02 13:41 UTC (permalink / raw)
To: Ron Mayer <[email protected]>; +Cc: Euler Taveira de Oliveira <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers
On Mon, 2009-06-01 at 20:30 -0700, Ron Mayer wrote:
> What I'd find strange about "6.67 rows" in your example is more that on
> the estimated rows side, it seems to imply an unrealistically precise estimate
> in the same way that "667 rows" would seem unrealistically precise to me.
> Maybe rounding to 2 significant digits would reduce confusion?
You're right that the number of significant digits already exceeds the
true accuracy of the computation. I think what Robert wants to see is
the exact value used in the calc, so the estimates can be checked more
thoroughly than is currently possible.
--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Training, Services and Support
^ permalink raw reply [nested|flat] 45+ messages in thread
* Re: explain analyze rows=%.0f
@ 2009-06-02 14:06 Robert Haas <[email protected]>
parent: Simon Riggs <[email protected]>
0 siblings, 1 reply; 45+ messages in thread
From: Robert Haas @ 2009-06-02 14:06 UTC (permalink / raw)
To: Simon Riggs <[email protected]>; +Cc: Ron Mayer <[email protected]>; Euler Taveira de Oliveira <[email protected]>; pgsql-hackers
On Jun 2, 2009, at 9:41 AM, Simon Riggs <[email protected]> wrote:
>
> On Mon, 2009-06-01 at 20:30 -0700, Ron Mayer wrote:
>
>> What I'd find strange about "6.67 rows" in your example is more
>> that on
>> the estimated rows side, it seems to imply an unrealistically
>> precise estimate
>> in the same way that "667 rows" would seem unrealistically precise
>> to me.
>> Maybe rounding to 2 significant digits would reduce confusion?
>
> You're right that the number of significant digits already exceeds the
> true accuracy of the computation. I think what Robert wants to see is
> the exact value used in the calc, so the estimates can be checked more
> thoroughly than is currently possible.
Bingo.
...Robert
^ permalink raw reply [nested|flat] 45+ messages in thread
* Re: explain analyze rows=%.0f
@ 2009-06-02 14:38 Tom Lane <[email protected]>
parent: Robert Haas <[email protected]>
0 siblings, 2 replies; 45+ messages in thread
From: Tom Lane @ 2009-06-02 14:38 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Simon Riggs <[email protected]>; Ron Mayer <[email protected]>; Euler Taveira de Oliveira <[email protected]>; pgsql-hackers
Robert Haas <[email protected]> writes:
> On Jun 2, 2009, at 9:41 AM, Simon Riggs <[email protected]> wrote:
>> You're right that the number of significant digits already exceeds the
>> true accuracy of the computation. I think what Robert wants to see is
>> the exact value used in the calc, so the estimates can be checked more
>> thoroughly than is currently possible.
> Bingo.
Uh, the planner's estimate *is* an integer. What was under discussion
(I thought) was showing some fractional digits in the case where EXPLAIN
ANALYZE is outputting a measured row count that is an average over
multiple loops, and therefore isn't necessarily an integer. In that
case the measured value can be considered arbitrarily precise --- though
I think in practice one or two fractional digits would be plenty.
regards, tom lane
^ permalink raw reply [nested|flat] 45+ messages in thread
* Re: explain analyze rows=%.0f
@ 2009-06-02 14:56 Robert Haas <[email protected]>
parent: Tom Lane <[email protected]>
1 sibling, 0 replies; 45+ messages in thread
From: Robert Haas @ 2009-06-02 14:56 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Simon Riggs <[email protected]>; Ron Mayer <[email protected]>; Euler Taveira de Oliveira <[email protected]>; pgsql-hackers
...Robert
On Jun 2, 2009, at 10:38 AM, Tom Lane <[email protected]> wrote:
> Robert Haas <[email protected]> writes:
>> On Jun 2, 2009, at 9:41 AM, Simon Riggs <[email protected]>
>> wrote:
>>> You're right that the number of significant digits already exceeds
>>> the
>>> true accuracy of the computation. I think what Robert wants to see
>>> is
>>> the exact value used in the calc, so the estimates can be checked
>>> more
>>> thoroughly than is currently possible.
>
>> Bingo.
>
> Uh, the planner's estimate *is* an integer. What was under discussion
> (I thought) was showing some fractional digits in the case where
> EXPLAIN
> ANALYZE is outputting a measured row count that is an average over
> multiple loops, and therefore isn't necessarily an integer. In that
> case the measured value can be considered arbitrarily precise ---
> though
> I think in practice one or two fractional digits would be plenty.
We're in violent agreement here.
...Robert
^ permalink raw reply [nested|flat] 45+ messages in thread
* [PATCH 08/21] vcregress: add alltaptests
@ 2022-02-25 23:00 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 45+ messages in thread
From: Justin Pryzby @ 2022-02-25 23:00 UTC (permalink / raw)
https://www.postgresql.org/message-id/[email protected]
https://www.postgresql.org/message-id/flat/[email protected]
In passing, document taptest PROVE_FLAGS
ci-os-only: windows
See also:
d835dd6685246f0737ca42ab68242210681bb220
13d856e177e69083f543d6383eeda9e12ce3c55c
fed6df486dca1b9e53d3f560031b9a236c99f4bb
---
.cirrus.yml | 10 ++-------
src/tools/msvc/vcregress.pl | 43 +++++++++++++++++++++++++++++++++----
2 files changed, 41 insertions(+), 12 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index f2bb05266cc..ea1f493923a 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -437,14 +437,8 @@ task:
test_ssl_script: |
set with_ssl=openssl
%T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/ssl/
- test_subscription_script: |
- %T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/subscription/
- test_authentication_script: |
- %T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/authentication/
- test_recovery_script: |
- %T_C% perl src/tools/msvc/vcregress.pl recoverycheck
- test_bin_script: |
- %T_C% perl src/tools/msvc/vcregress.pl bincheck
+ test_tap_script: |
+ %T_C% perl src/tools/msvc/vcregress.pl alltaptests
test_ecpg_script: |
rem tries to build additional stuff
vcvarsall x64
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index 54c31b0e14f..5dcc3419ad7 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -51,7 +51,7 @@ if (-e "src/tools/msvc/buildenv.pl")
my $what = shift || "";
if ($what =~
- /^(check|installcheck|plcheck|contribcheck|modulescheck|ecpgcheck|isolationcheck|upgradecheck|bincheck|recoverycheck|taptest)$/i
+ /^(check|installcheck|plcheck|contribcheck|modulescheck|ecpgcheck|isolationcheck|upgradecheck|bincheck|recoverycheck|taptest|alltaptests)$/i
)
{
$what = uc $what;
@@ -109,6 +109,7 @@ my %command = (
BINCHECK => \&bincheck,
RECOVERYCHECK => \&recoverycheck,
UPGRADECHECK => \&upgradecheck, # no-op
+ ALLTAPTESTS => \&alltaptests,
TAPTEST => \&taptest,);
my $proc = $command{$what};
@@ -291,6 +292,9 @@ sub tap_check
$ENV{PG_REGRESS} = "$topdir/$Config/pg_regress/pg_regress";
$ENV{REGRESS_SHLIB} = "$topdir/src/test/regress/regress.dll";
+ print "============================================================\n";
+ print "Checking $dir: @args\n";
+
rmtree('tmp_check');
system(@args);
my $status = $? >> 8;
@@ -305,8 +309,7 @@ sub bincheck
my $mstat = 0;
- # Find out all the existing TAP tests by looking for t/ directories
- # in the tree.
+ # Find the TAP tests by looking for t/ directories
my @bin_dirs = glob("$topdir/src/bin/*");
# Process each test
@@ -321,6 +324,36 @@ sub bincheck
return;
}
+sub alltaptests
+{
+ InstallTemp();
+
+ my $mstat = 0;
+
+ # Find out all the existing TAP tests by looking for t/ directories
+ # in the tree.
+ my @tap_dirs = ();
+ my @top_dir = ($topdir);
+ File::Find::find(
+ { wanted => sub {
+ /^t\z/s
+ && $File::Find::name !~ /\/(kerberos|ldap|ssl|ssl_passphrase_callback)\// # opt-in: warn about these?
+ && push(@tap_dirs, $File::Find::name);
+ }
+ },
+ @top_dir);
+
+ # Process each test
+ foreach my $test_path (@tap_dirs)
+ {
+ my $dir = dirname($test_path);
+ my $status = tap_check($dir);
+ $mstat ||= $status;
+ }
+ exit $mstat if $mstat;
+ return;
+}
+
sub taptest
{
my $dir = shift;
@@ -671,6 +704,7 @@ sub usage
print STDERR
"Usage: vcregress.pl <mode> [<arg>]\n\n",
"Options for <mode>:\n",
+ " alltaptests run all tap tests (except kerberos, ldap, ssl, ssl_passphrase_callback)\n",
" bincheck run tests of utilities in src/bin/\n",
" check deploy instance and run regression tests on it\n",
" contribcheck run tests of modules in contrib/\n",
@@ -688,6 +722,7 @@ sub usage
"\nOptions for <arg>: (used by contribcheck and modulescheck)\n",
" install also run tests which require a new instance\n",
"\nOption for <arg>: for taptest\n",
- " TEST_DIR (required) directory where tests reside\n";
+ " TEST_DIR (required) directory where tests reside\n",
+ " PROVE_FLAGS flags to pass to prove\n";
exit(1);
}
--
2.17.1
--STPqjqpCrtky8aYs
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="0009-tmp-run-tap-tests-first.patch"
^ permalink raw reply [nested|flat] 45+ messages in thread
* [PATCH 4/7] vcregress: add alltaptests
@ 2022-02-25 23:00 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 45+ messages in thread
From: Justin Pryzby @ 2022-02-25 23:00 UTC (permalink / raw)
In passing, document taptest PROVE_FLAGS
ci-os-only: windows
See also:
d835dd6685246f0737ca42ab68242210681bb220
13d856e177e69083f543d6383eeda9e12ce3c55c
fed6df486dca1b9e53d3f560031b9a236c99f4bb
---
.cirrus.yml | 10 ++-------
src/tools/msvc/vcregress.pl | 43 +++++++++++++++++++++++++++++++++----
2 files changed, 41 insertions(+), 12 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index 634c0b4933..bc4ae9fc28 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -417,14 +417,8 @@ task:
test_ssl_script: |
set with_ssl=openssl
%T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/ssl/
- test_subscription_script: |
- %T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/subscription/
- test_authentication_script: |
- %T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/authentication/
- test_recovery_script: |
- %T_C% perl src/tools/msvc/vcregress.pl recoverycheck
- test_bin_script: |
- %T_C% perl src/tools/msvc/vcregress.pl bincheck
+ test_tap_script: |
+ %T_C% perl src/tools/msvc/vcregress.pl alltaptests
test_pg_upgrade_script: |
%T_C% perl src/tools/msvc/vcregress.pl upgradecheck
test_ecpg_script: |
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index 74aa572d2f..e27c1d15e5 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -49,7 +49,7 @@ if (-e "src/tools/msvc/buildenv.pl")
my $what = shift || "";
if ($what =~
- /^(check|installcheck|plcheck|contribcheck|modulescheck|ecpgcheck|isolationcheck|upgradecheck|bincheck|recoverycheck|taptest)$/i
+ /^(check|installcheck|plcheck|contribcheck|modulescheck|ecpgcheck|isolationcheck|upgradecheck|bincheck|recoverycheck|taptest|alltaptests)$/i
)
{
$what = uc $what;
@@ -107,6 +107,7 @@ my %command = (
BINCHECK => \&bincheck,
RECOVERYCHECK => \&recoverycheck,
UPGRADECHECK => \&upgradecheck,
+ ALLTAPTESTS => \&alltaptests,
TAPTEST => \&taptest,);
my $proc = $command{$what};
@@ -261,6 +262,9 @@ sub tap_check
$ENV{PG_REGRESS} = "$topdir/$Config/pg_regress/pg_regress";
$ENV{REGRESS_SHLIB} = "$topdir/src/test/regress/regress.dll";
+ print "============================================================\n";
+ print "Checking @args\n";
+
rmtree('tmp_check');
system(@args);
my $status = $? >> 8;
@@ -273,8 +277,7 @@ sub bincheck
my $mstat = 0;
- # Find out all the existing TAP tests by looking for t/ directories
- # in the tree.
+ # Find the TAP tests by looking for t/ directories
my @bin_dirs = glob("$topdir/src/bin/*");
# Process each test
@@ -288,6 +291,36 @@ sub bincheck
return;
}
+sub alltaptests
+{
+ InstallTemp();
+
+ my $mstat = 0;
+
+ # Find out all the existing TAP tests by looking for t/ directories
+ # in the tree.
+ my @tap_dirs = ();
+ my @top_dir = ($topdir);
+ File::Find::find(
+ { wanted => sub {
+ /^t\z/s
+ && $File::Find::name !~ /\/(kerberos|ldap|ssl|ssl_passphrase_callback)\// # opt-in
+ && push(@tap_dirs, $File::Find::name);
+ }
+ },
+ @top_dir);
+
+ # Process each test
+ foreach my $test_path (@tap_dirs)
+ {
+ my $dir = dirname($test_path);
+ my $status = tap_check($dir);
+ $mstat ||= $status;
+ }
+ exit $mstat if $mstat;
+ return;
+}
+
sub taptest
{
my $dir = shift;
@@ -777,6 +810,7 @@ sub usage
print STDERR
"Usage: vcregress.pl <mode> [<arg>]\n\n",
"Options for <mode>:\n",
+ " alltaptests run all tap tests (except kerberos, ldap, ssl, ssl_passphrase_callback)\n",
" bincheck run tests of utilities in src/bin/\n",
" check deploy instance and run regression tests on it\n",
" contribcheck run tests of modules in contrib/\n",
@@ -792,6 +826,7 @@ sub usage
" serial serial mode\n",
" parallel parallel mode\n",
"\nOption for <arg>: for taptest\n",
- " TEST_DIR (required) directory where tests reside\n";
+ " TEST_DIR (required) directory where tests reside\n",
+ " PROVE_FLAGS flags to pass to prove\n";
exit(1);
}
--
2.17.1
--ibvzjYYg+QDzMCy1
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="0005-cirrus-include-hints-how-to-install-OS-packages.patch"
^ permalink raw reply [nested|flat] 45+ messages in thread
* [PATCH 11/25] vcregress: add alltaptests
@ 2022-02-25 23:00 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 45+ messages in thread
From: Justin Pryzby @ 2022-02-25 23:00 UTC (permalink / raw)
https://www.postgresql.org/message-id/[email protected]
https://www.postgresql.org/message-id/flat/[email protected]
In passing, document taptest PROVE_FLAGS
ci-os-only: windows
See also:
d835dd6685246f0737ca42ab68242210681bb220
13d856e177e69083f543d6383eeda9e12ce3c55c
fed6df486dca1b9e53d3f560031b9a236c99f4bb
---
.cirrus.yml | 10 ++-------
src/tools/msvc/vcregress.pl | 43 +++++++++++++++++++++++++++++++++----
2 files changed, 41 insertions(+), 12 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index 54124d0fe34..f19558e67e0 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -455,14 +455,8 @@ task:
test_ssl_script: |
set with_ssl=openssl
%T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/ssl/
- test_subscription_script: |
- %T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/subscription/
- test_authentication_script: |
- %T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/authentication/
- test_recovery_script: |
- %T_C% perl src/tools/msvc/vcregress.pl recoverycheck
- test_bin_script: |
- %T_C% perl src/tools/msvc/vcregress.pl bincheck
+ test_tap_script: |
+ %T_C% perl src/tools/msvc/vcregress.pl alltaptests
test_ecpg_script: |
rem tries to build additional stuff
vcvarsall x64
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index 2d6ccd45419..6495f33e593 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -51,7 +51,7 @@ if (-e "src/tools/msvc/buildenv.pl")
my $what = shift || "";
if ($what =~
- /^(check|installcheck|plcheck|contribcheck|modulescheck|ecpgcheck|isolationcheck|upgradecheck|bincheck|recoverycheck|taptest)$/i
+ /^(check|installcheck|plcheck|contribcheck|modulescheck|ecpgcheck|isolationcheck|upgradecheck|bincheck|recoverycheck|taptest|alltaptests)$/i
)
{
$what = uc $what;
@@ -109,6 +109,7 @@ my %command = (
BINCHECK => \&bincheck,
RECOVERYCHECK => \&recoverycheck,
UPGRADECHECK => \&upgradecheck, # no-op
+ ALLTAPTESTS => \&alltaptests,
TAPTEST => \&taptest,);
my $proc = $command{$what};
@@ -296,6 +297,9 @@ sub tap_check
# add the module build dir as the second element in the PATH
$ENV{PATH} =~ s!;!;$topdir/$Config/$module;!;
+ print "============================================================\n";
+ print "Checking $dir: @args\n";
+
rmtree('tmp_check');
system(@args);
my $status = $? >> 8;
@@ -310,8 +314,7 @@ sub bincheck
my $mstat = 0;
- # Find out all the existing TAP tests by looking for t/ directories
- # in the tree.
+ # Find the TAP tests by looking for t/ directories
my @bin_dirs = glob("$topdir/src/bin/*");
# Process each test
@@ -326,6 +329,36 @@ sub bincheck
return;
}
+sub alltaptests
+{
+ InstallTemp();
+
+ my $mstat = 0;
+
+ # Find out all the existing TAP tests by looking for t/ directories
+ # in the tree.
+ my @tap_dirs = ();
+ my @top_dir = ($topdir);
+ File::Find::find(
+ { wanted => sub {
+ /^t\z/s
+ && $File::Find::name !~ /\/(kerberos|ldap|ssl|ssl_passphrase_callback)\// # opt-in: warn about these?
+ && push(@tap_dirs, $File::Find::name);
+ }
+ },
+ @top_dir);
+
+ # Process each test
+ foreach my $test_path (@tap_dirs)
+ {
+ my $dir = dirname($test_path);
+ my $status = tap_check($dir);
+ $mstat ||= $status;
+ }
+ exit $mstat if $mstat;
+ return;
+}
+
sub taptest
{
my $dir = shift;
@@ -676,6 +709,7 @@ sub usage
print STDERR
"Usage: vcregress.pl <mode> [<arg>]\n\n",
"Options for <mode>:\n",
+ " alltaptests run all tap tests (except kerberos, ldap, ssl, ssl_passphrase_callback)\n",
" bincheck run tests of utilities in src/bin/\n",
" check deploy instance and run regression tests on it\n",
" contribcheck run tests of modules in contrib/\n",
@@ -693,6 +727,7 @@ sub usage
"\nOptions for <arg>: (used by contribcheck and modulescheck)\n",
" install also run tests which require a new instance\n",
"\nOption for <arg>: for taptest\n",
- " TEST_DIR (required) directory where tests reside\n";
+ " TEST_DIR (required) directory where tests reside\n",
+ " PROVE_FLAGS flags to pass to prove\n";
exit(1);
}
--
2.17.1
--IS0zKkzwUGydFO0o
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="0012-tmp-run-tap-tests-first.patch"
^ permalink raw reply [nested|flat] 45+ messages in thread
* [PATCH 08/19] vcregress: add alltaptests
@ 2022-02-25 23:00 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 45+ messages in thread
From: Justin Pryzby @ 2022-02-25 23:00 UTC (permalink / raw)
https://www.postgresql.org/message-id/[email protected]
https://www.postgresql.org/message-id/flat/[email protected]
In passing, document taptest PROVE_FLAGS
ci-os-only: windows
See also:
d835dd6685246f0737ca42ab68242210681bb220
13d856e177e69083f543d6383eeda9e12ce3c55c
fed6df486dca1b9e53d3f560031b9a236c99f4bb
---
.cirrus.yml | 10 ++-------
src/tools/msvc/vcregress.pl | 43 +++++++++++++++++++++++++++++++++----
2 files changed, 41 insertions(+), 12 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index f2bb05266cc..ea1f493923a 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -437,14 +437,8 @@ task:
test_ssl_script: |
set with_ssl=openssl
%T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/ssl/
- test_subscription_script: |
- %T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/subscription/
- test_authentication_script: |
- %T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/authentication/
- test_recovery_script: |
- %T_C% perl src/tools/msvc/vcregress.pl recoverycheck
- test_bin_script: |
- %T_C% perl src/tools/msvc/vcregress.pl bincheck
+ test_tap_script: |
+ %T_C% perl src/tools/msvc/vcregress.pl alltaptests
test_ecpg_script: |
rem tries to build additional stuff
vcvarsall x64
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index 54c31b0e14f..1b1387acac0 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -51,7 +51,7 @@ if (-e "src/tools/msvc/buildenv.pl")
my $what = shift || "";
if ($what =~
- /^(check|installcheck|plcheck|contribcheck|modulescheck|ecpgcheck|isolationcheck|upgradecheck|bincheck|recoverycheck|taptest)$/i
+ /^(check|installcheck|plcheck|contribcheck|modulescheck|ecpgcheck|isolationcheck|upgradecheck|bincheck|recoverycheck|taptest|alltaptests)$/i
)
{
$what = uc $what;
@@ -109,6 +109,7 @@ my %command = (
BINCHECK => \&bincheck,
RECOVERYCHECK => \&recoverycheck,
UPGRADECHECK => \&upgradecheck, # no-op
+ ALLTAPTESTS => \&alltaptests,
TAPTEST => \&taptest,);
my $proc = $command{$what};
@@ -291,6 +292,9 @@ sub tap_check
$ENV{PG_REGRESS} = "$topdir/$Config/pg_regress/pg_regress";
$ENV{REGRESS_SHLIB} = "$topdir/src/test/regress/regress.dll";
+ print "============================================================\n";
+ print "Checking $dir: @args\n";
+
rmtree('tmp_check');
system(@args);
my $status = $? >> 8;
@@ -305,8 +309,7 @@ sub bincheck
my $mstat = 0;
- # Find out all the existing TAP tests by looking for t/ directories
- # in the tree.
+ # Find the TAP tests by looking for t/ directories
my @bin_dirs = glob("$topdir/src/bin/*");
# Process each test
@@ -321,6 +324,36 @@ sub bincheck
return;
}
+sub alltaptests
+{
+ InstallTemp();
+
+ my $mstat = 0;
+
+ # Find out all the existing TAP tests by looking for t/ directories
+ # in the tree.
+ my @tap_dirs = ();
+ my @top_dir = ($topdir);
+ File::Find::find(
+ { wanted => sub {
+ /^t\z/s
+ && $File::Find::name !~ /\/(kerberos|ldap|ssl|ssl_passphrase_callback)\// # opt-in
+ && push(@tap_dirs, $File::Find::name);
+ }
+ },
+ @top_dir);
+
+ # Process each test
+ foreach my $test_path (@tap_dirs)
+ {
+ my $dir = dirname($test_path);
+ my $status = tap_check($dir);
+ $mstat ||= $status;
+ }
+ exit $mstat if $mstat;
+ return;
+}
+
sub taptest
{
my $dir = shift;
@@ -671,6 +704,7 @@ sub usage
print STDERR
"Usage: vcregress.pl <mode> [<arg>]\n\n",
"Options for <mode>:\n",
+ " alltaptests run all tap tests (except kerberos, ldap, ssl, ssl_passphrase_callback)\n",
" bincheck run tests of utilities in src/bin/\n",
" check deploy instance and run regression tests on it\n",
" contribcheck run tests of modules in contrib/\n",
@@ -688,6 +722,7 @@ sub usage
"\nOptions for <arg>: (used by contribcheck and modulescheck)\n",
" install also run tests which require a new instance\n",
"\nOption for <arg>: for taptest\n",
- " TEST_DIR (required) directory where tests reside\n";
+ " TEST_DIR (required) directory where tests reside\n",
+ " PROVE_FLAGS flags to pass to prove\n";
exit(1);
}
--
2.17.1
--Sw7tCqrGA+HQ0/zt
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="0009-tmp-run-tap-tests-first.patch"
^ permalink raw reply [nested|flat] 45+ messages in thread
* [PATCH 08/19] vcregress: add alltaptests
@ 2022-02-25 23:00 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 45+ messages in thread
From: Justin Pryzby @ 2022-02-25 23:00 UTC (permalink / raw)
https://www.postgresql.org/message-id/[email protected]
https://www.postgresql.org/message-id/flat/[email protected]
In passing, document taptest PROVE_FLAGS
ci-os-only: windows
See also:
d835dd6685246f0737ca42ab68242210681bb220
13d856e177e69083f543d6383eeda9e12ce3c55c
fed6df486dca1b9e53d3f560031b9a236c99f4bb
---
.cirrus.yml | 10 ++-------
src/tools/msvc/vcregress.pl | 43 +++++++++++++++++++++++++++++++++----
2 files changed, 41 insertions(+), 12 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index f2bb05266cc..ea1f493923a 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -437,14 +437,8 @@ task:
test_ssl_script: |
set with_ssl=openssl
%T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/ssl/
- test_subscription_script: |
- %T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/subscription/
- test_authentication_script: |
- %T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/authentication/
- test_recovery_script: |
- %T_C% perl src/tools/msvc/vcregress.pl recoverycheck
- test_bin_script: |
- %T_C% perl src/tools/msvc/vcregress.pl bincheck
+ test_tap_script: |
+ %T_C% perl src/tools/msvc/vcregress.pl alltaptests
test_ecpg_script: |
rem tries to build additional stuff
vcvarsall x64
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index 54c31b0e14f..1b1387acac0 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -51,7 +51,7 @@ if (-e "src/tools/msvc/buildenv.pl")
my $what = shift || "";
if ($what =~
- /^(check|installcheck|plcheck|contribcheck|modulescheck|ecpgcheck|isolationcheck|upgradecheck|bincheck|recoverycheck|taptest)$/i
+ /^(check|installcheck|plcheck|contribcheck|modulescheck|ecpgcheck|isolationcheck|upgradecheck|bincheck|recoverycheck|taptest|alltaptests)$/i
)
{
$what = uc $what;
@@ -109,6 +109,7 @@ my %command = (
BINCHECK => \&bincheck,
RECOVERYCHECK => \&recoverycheck,
UPGRADECHECK => \&upgradecheck, # no-op
+ ALLTAPTESTS => \&alltaptests,
TAPTEST => \&taptest,);
my $proc = $command{$what};
@@ -291,6 +292,9 @@ sub tap_check
$ENV{PG_REGRESS} = "$topdir/$Config/pg_regress/pg_regress";
$ENV{REGRESS_SHLIB} = "$topdir/src/test/regress/regress.dll";
+ print "============================================================\n";
+ print "Checking $dir: @args\n";
+
rmtree('tmp_check');
system(@args);
my $status = $? >> 8;
@@ -305,8 +309,7 @@ sub bincheck
my $mstat = 0;
- # Find out all the existing TAP tests by looking for t/ directories
- # in the tree.
+ # Find the TAP tests by looking for t/ directories
my @bin_dirs = glob("$topdir/src/bin/*");
# Process each test
@@ -321,6 +324,36 @@ sub bincheck
return;
}
+sub alltaptests
+{
+ InstallTemp();
+
+ my $mstat = 0;
+
+ # Find out all the existing TAP tests by looking for t/ directories
+ # in the tree.
+ my @tap_dirs = ();
+ my @top_dir = ($topdir);
+ File::Find::find(
+ { wanted => sub {
+ /^t\z/s
+ && $File::Find::name !~ /\/(kerberos|ldap|ssl|ssl_passphrase_callback)\// # opt-in
+ && push(@tap_dirs, $File::Find::name);
+ }
+ },
+ @top_dir);
+
+ # Process each test
+ foreach my $test_path (@tap_dirs)
+ {
+ my $dir = dirname($test_path);
+ my $status = tap_check($dir);
+ $mstat ||= $status;
+ }
+ exit $mstat if $mstat;
+ return;
+}
+
sub taptest
{
my $dir = shift;
@@ -671,6 +704,7 @@ sub usage
print STDERR
"Usage: vcregress.pl <mode> [<arg>]\n\n",
"Options for <mode>:\n",
+ " alltaptests run all tap tests (except kerberos, ldap, ssl, ssl_passphrase_callback)\n",
" bincheck run tests of utilities in src/bin/\n",
" check deploy instance and run regression tests on it\n",
" contribcheck run tests of modules in contrib/\n",
@@ -688,6 +722,7 @@ sub usage
"\nOptions for <arg>: (used by contribcheck and modulescheck)\n",
" install also run tests which require a new instance\n",
"\nOption for <arg>: for taptest\n",
- " TEST_DIR (required) directory where tests reside\n";
+ " TEST_DIR (required) directory where tests reside\n",
+ " PROVE_FLAGS flags to pass to prove\n";
exit(1);
}
--
2.17.1
--Sw7tCqrGA+HQ0/zt
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="0009-tmp-run-tap-tests-first.patch"
^ permalink raw reply [nested|flat] 45+ messages in thread
* [PATCH 09/23] vcregress: add alltaptests
@ 2022-02-25 23:00 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 45+ messages in thread
From: Justin Pryzby @ 2022-02-25 23:00 UTC (permalink / raw)
https://www.postgresql.org/message-id/[email protected]
https://www.postgresql.org/message-id/flat/[email protected]
In passing, document taptest PROVE_FLAGS
ci-os-only: windows
See also:
d835dd6685246f0737ca42ab68242210681bb220
13d856e177e69083f543d6383eeda9e12ce3c55c
fed6df486dca1b9e53d3f560031b9a236c99f4bb
---
.cirrus.yml | 14 ++++--------
src/tools/msvc/vcregress.pl | 43 +++++++++++++++++++++++++++++++++----
2 files changed, 43 insertions(+), 14 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index 2cabe6b7ec1..7b8ce6eadc3 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -401,9 +401,9 @@ task:
# If tests hang forever, cirrus eventually times out. In that case log
# output etc is not uploaded, making the problem hard to debug. Of course
# tests internally should have shorter timeouts, but that's proven to not
- # be sufficient. 15min currently is fast enough to finish individual test
+ # be sufficient. 25min currently is fast enough to finish individual test
# "suites".
- T_C: "\"C:/Program Files/Git/usr/bin/timeout.exe\" -v -k60s 15m"
+ T_C: "\"C:/Program Files/Git/usr/bin/timeout.exe\" -v -k60s 25m"
# startcreate_script starts a postgres instance that we don't want to get
# killed at the end of that script (it's stopped in stop_script). Can't
@@ -461,14 +461,8 @@ task:
test_ssl_script: |
set with_ssl=openssl
%T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/ssl/
- test_subscription_script: |
- %T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/subscription/
- test_authentication_script: |
- %T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/authentication/
- test_recovery_script: |
- %T_C% perl src/tools/msvc/vcregress.pl recoverycheck
- test_bin_script: |
- %T_C% perl src/tools/msvc/vcregress.pl bincheck
+ test_tap_script: |
+ %T_C% perl src/tools/msvc/vcregress.pl alltaptests
test_ecpg_script: |
rem tries to build additional stuff
vcvarsall x64
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index 2d6ccd45419..6495f33e593 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -51,7 +51,7 @@ if (-e "src/tools/msvc/buildenv.pl")
my $what = shift || "";
if ($what =~
- /^(check|installcheck|plcheck|contribcheck|modulescheck|ecpgcheck|isolationcheck|upgradecheck|bincheck|recoverycheck|taptest)$/i
+ /^(check|installcheck|plcheck|contribcheck|modulescheck|ecpgcheck|isolationcheck|upgradecheck|bincheck|recoverycheck|taptest|alltaptests)$/i
)
{
$what = uc $what;
@@ -109,6 +109,7 @@ my %command = (
BINCHECK => \&bincheck,
RECOVERYCHECK => \&recoverycheck,
UPGRADECHECK => \&upgradecheck, # no-op
+ ALLTAPTESTS => \&alltaptests,
TAPTEST => \&taptest,);
my $proc = $command{$what};
@@ -296,6 +297,9 @@ sub tap_check
# add the module build dir as the second element in the PATH
$ENV{PATH} =~ s!;!;$topdir/$Config/$module;!;
+ print "============================================================\n";
+ print "Checking $dir: @args\n";
+
rmtree('tmp_check');
system(@args);
my $status = $? >> 8;
@@ -310,8 +314,7 @@ sub bincheck
my $mstat = 0;
- # Find out all the existing TAP tests by looking for t/ directories
- # in the tree.
+ # Find the TAP tests by looking for t/ directories
my @bin_dirs = glob("$topdir/src/bin/*");
# Process each test
@@ -326,6 +329,36 @@ sub bincheck
return;
}
+sub alltaptests
+{
+ InstallTemp();
+
+ my $mstat = 0;
+
+ # Find out all the existing TAP tests by looking for t/ directories
+ # in the tree.
+ my @tap_dirs = ();
+ my @top_dir = ($topdir);
+ File::Find::find(
+ { wanted => sub {
+ /^t\z/s
+ && $File::Find::name !~ /\/(kerberos|ldap|ssl|ssl_passphrase_callback)\// # opt-in: warn about these?
+ && push(@tap_dirs, $File::Find::name);
+ }
+ },
+ @top_dir);
+
+ # Process each test
+ foreach my $test_path (@tap_dirs)
+ {
+ my $dir = dirname($test_path);
+ my $status = tap_check($dir);
+ $mstat ||= $status;
+ }
+ exit $mstat if $mstat;
+ return;
+}
+
sub taptest
{
my $dir = shift;
@@ -676,6 +709,7 @@ sub usage
print STDERR
"Usage: vcregress.pl <mode> [<arg>]\n\n",
"Options for <mode>:\n",
+ " alltaptests run all tap tests (except kerberos, ldap, ssl, ssl_passphrase_callback)\n",
" bincheck run tests of utilities in src/bin/\n",
" check deploy instance and run regression tests on it\n",
" contribcheck run tests of modules in contrib/\n",
@@ -693,6 +727,7 @@ sub usage
"\nOptions for <arg>: (used by contribcheck and modulescheck)\n",
" install also run tests which require a new instance\n",
"\nOption for <arg>: for taptest\n",
- " TEST_DIR (required) directory where tests reside\n";
+ " TEST_DIR (required) directory where tests reside\n",
+ " PROVE_FLAGS flags to pass to prove\n";
exit(1);
}
--
2.17.1
--tKkaNMvYmhQvRCRK
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="0010-tmp-run-tap-tests-first.patch"
^ permalink raw reply [nested|flat] 45+ messages in thread
* [PATCH 08/21] vcregress: add alltaptests
@ 2022-02-25 23:00 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 45+ messages in thread
From: Justin Pryzby @ 2022-02-25 23:00 UTC (permalink / raw)
https://www.postgresql.org/message-id/[email protected]
https://www.postgresql.org/message-id/flat/[email protected]
In passing, document taptest PROVE_FLAGS
ci-os-only: windows
See also:
d835dd6685246f0737ca42ab68242210681bb220
13d856e177e69083f543d6383eeda9e12ce3c55c
fed6df486dca1b9e53d3f560031b9a236c99f4bb
---
.cirrus.yml | 10 ++-------
src/tools/msvc/vcregress.pl | 43 +++++++++++++++++++++++++++++++++----
2 files changed, 41 insertions(+), 12 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index f2bb05266cc..ea1f493923a 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -437,14 +437,8 @@ task:
test_ssl_script: |
set with_ssl=openssl
%T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/ssl/
- test_subscription_script: |
- %T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/subscription/
- test_authentication_script: |
- %T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/authentication/
- test_recovery_script: |
- %T_C% perl src/tools/msvc/vcregress.pl recoverycheck
- test_bin_script: |
- %T_C% perl src/tools/msvc/vcregress.pl bincheck
+ test_tap_script: |
+ %T_C% perl src/tools/msvc/vcregress.pl alltaptests
test_ecpg_script: |
rem tries to build additional stuff
vcvarsall x64
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index 54c31b0e14f..5dcc3419ad7 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -51,7 +51,7 @@ if (-e "src/tools/msvc/buildenv.pl")
my $what = shift || "";
if ($what =~
- /^(check|installcheck|plcheck|contribcheck|modulescheck|ecpgcheck|isolationcheck|upgradecheck|bincheck|recoverycheck|taptest)$/i
+ /^(check|installcheck|plcheck|contribcheck|modulescheck|ecpgcheck|isolationcheck|upgradecheck|bincheck|recoverycheck|taptest|alltaptests)$/i
)
{
$what = uc $what;
@@ -109,6 +109,7 @@ my %command = (
BINCHECK => \&bincheck,
RECOVERYCHECK => \&recoverycheck,
UPGRADECHECK => \&upgradecheck, # no-op
+ ALLTAPTESTS => \&alltaptests,
TAPTEST => \&taptest,);
my $proc = $command{$what};
@@ -291,6 +292,9 @@ sub tap_check
$ENV{PG_REGRESS} = "$topdir/$Config/pg_regress/pg_regress";
$ENV{REGRESS_SHLIB} = "$topdir/src/test/regress/regress.dll";
+ print "============================================================\n";
+ print "Checking $dir: @args\n";
+
rmtree('tmp_check');
system(@args);
my $status = $? >> 8;
@@ -305,8 +309,7 @@ sub bincheck
my $mstat = 0;
- # Find out all the existing TAP tests by looking for t/ directories
- # in the tree.
+ # Find the TAP tests by looking for t/ directories
my @bin_dirs = glob("$topdir/src/bin/*");
# Process each test
@@ -321,6 +324,36 @@ sub bincheck
return;
}
+sub alltaptests
+{
+ InstallTemp();
+
+ my $mstat = 0;
+
+ # Find out all the existing TAP tests by looking for t/ directories
+ # in the tree.
+ my @tap_dirs = ();
+ my @top_dir = ($topdir);
+ File::Find::find(
+ { wanted => sub {
+ /^t\z/s
+ && $File::Find::name !~ /\/(kerberos|ldap|ssl|ssl_passphrase_callback)\// # opt-in: warn about these?
+ && push(@tap_dirs, $File::Find::name);
+ }
+ },
+ @top_dir);
+
+ # Process each test
+ foreach my $test_path (@tap_dirs)
+ {
+ my $dir = dirname($test_path);
+ my $status = tap_check($dir);
+ $mstat ||= $status;
+ }
+ exit $mstat if $mstat;
+ return;
+}
+
sub taptest
{
my $dir = shift;
@@ -671,6 +704,7 @@ sub usage
print STDERR
"Usage: vcregress.pl <mode> [<arg>]\n\n",
"Options for <mode>:\n",
+ " alltaptests run all tap tests (except kerberos, ldap, ssl, ssl_passphrase_callback)\n",
" bincheck run tests of utilities in src/bin/\n",
" check deploy instance and run regression tests on it\n",
" contribcheck run tests of modules in contrib/\n",
@@ -688,6 +722,7 @@ sub usage
"\nOptions for <arg>: (used by contribcheck and modulescheck)\n",
" install also run tests which require a new instance\n",
"\nOption for <arg>: for taptest\n",
- " TEST_DIR (required) directory where tests reside\n";
+ " TEST_DIR (required) directory where tests reside\n",
+ " PROVE_FLAGS flags to pass to prove\n";
exit(1);
}
--
2.17.1
--mln0rGgUGuXEqmuI
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="0009-tmp-run-tap-tests-first.patch"
^ permalink raw reply [nested|flat] 45+ messages in thread
* [PATCH 09/23] vcregress: add alltaptests
@ 2022-02-25 23:00 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 45+ messages in thread
From: Justin Pryzby @ 2022-02-25 23:00 UTC (permalink / raw)
https://www.postgresql.org/message-id/[email protected]
https://www.postgresql.org/message-id/flat/[email protected]
In passing, document taptest PROVE_FLAGS
ci-os-only: windows
See also:
d835dd6685246f0737ca42ab68242210681bb220
13d856e177e69083f543d6383eeda9e12ce3c55c
fed6df486dca1b9e53d3f560031b9a236c99f4bb
---
.cirrus.yml | 14 ++++--------
src/tools/msvc/vcregress.pl | 43 +++++++++++++++++++++++++++++++++----
2 files changed, 43 insertions(+), 14 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index 2cabe6b7ec1..7b8ce6eadc3 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -401,9 +401,9 @@ task:
# If tests hang forever, cirrus eventually times out. In that case log
# output etc is not uploaded, making the problem hard to debug. Of course
# tests internally should have shorter timeouts, but that's proven to not
- # be sufficient. 15min currently is fast enough to finish individual test
+ # be sufficient. 25min currently is fast enough to finish individual test
# "suites".
- T_C: "\"C:/Program Files/Git/usr/bin/timeout.exe\" -v -k60s 15m"
+ T_C: "\"C:/Program Files/Git/usr/bin/timeout.exe\" -v -k60s 25m"
# startcreate_script starts a postgres instance that we don't want to get
# killed at the end of that script (it's stopped in stop_script). Can't
@@ -461,14 +461,8 @@ task:
test_ssl_script: |
set with_ssl=openssl
%T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/ssl/
- test_subscription_script: |
- %T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/subscription/
- test_authentication_script: |
- %T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/authentication/
- test_recovery_script: |
- %T_C% perl src/tools/msvc/vcregress.pl recoverycheck
- test_bin_script: |
- %T_C% perl src/tools/msvc/vcregress.pl bincheck
+ test_tap_script: |
+ %T_C% perl src/tools/msvc/vcregress.pl alltaptests
test_ecpg_script: |
rem tries to build additional stuff
vcvarsall x64
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index 2d6ccd45419..6495f33e593 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -51,7 +51,7 @@ if (-e "src/tools/msvc/buildenv.pl")
my $what = shift || "";
if ($what =~
- /^(check|installcheck|plcheck|contribcheck|modulescheck|ecpgcheck|isolationcheck|upgradecheck|bincheck|recoverycheck|taptest)$/i
+ /^(check|installcheck|plcheck|contribcheck|modulescheck|ecpgcheck|isolationcheck|upgradecheck|bincheck|recoverycheck|taptest|alltaptests)$/i
)
{
$what = uc $what;
@@ -109,6 +109,7 @@ my %command = (
BINCHECK => \&bincheck,
RECOVERYCHECK => \&recoverycheck,
UPGRADECHECK => \&upgradecheck, # no-op
+ ALLTAPTESTS => \&alltaptests,
TAPTEST => \&taptest,);
my $proc = $command{$what};
@@ -296,6 +297,9 @@ sub tap_check
# add the module build dir as the second element in the PATH
$ENV{PATH} =~ s!;!;$topdir/$Config/$module;!;
+ print "============================================================\n";
+ print "Checking $dir: @args\n";
+
rmtree('tmp_check');
system(@args);
my $status = $? >> 8;
@@ -310,8 +314,7 @@ sub bincheck
my $mstat = 0;
- # Find out all the existing TAP tests by looking for t/ directories
- # in the tree.
+ # Find the TAP tests by looking for t/ directories
my @bin_dirs = glob("$topdir/src/bin/*");
# Process each test
@@ -326,6 +329,36 @@ sub bincheck
return;
}
+sub alltaptests
+{
+ InstallTemp();
+
+ my $mstat = 0;
+
+ # Find out all the existing TAP tests by looking for t/ directories
+ # in the tree.
+ my @tap_dirs = ();
+ my @top_dir = ($topdir);
+ File::Find::find(
+ { wanted => sub {
+ /^t\z/s
+ && $File::Find::name !~ /\/(kerberos|ldap|ssl|ssl_passphrase_callback)\// # opt-in: warn about these?
+ && push(@tap_dirs, $File::Find::name);
+ }
+ },
+ @top_dir);
+
+ # Process each test
+ foreach my $test_path (@tap_dirs)
+ {
+ my $dir = dirname($test_path);
+ my $status = tap_check($dir);
+ $mstat ||= $status;
+ }
+ exit $mstat if $mstat;
+ return;
+}
+
sub taptest
{
my $dir = shift;
@@ -676,6 +709,7 @@ sub usage
print STDERR
"Usage: vcregress.pl <mode> [<arg>]\n\n",
"Options for <mode>:\n",
+ " alltaptests run all tap tests (except kerberos, ldap, ssl, ssl_passphrase_callback)\n",
" bincheck run tests of utilities in src/bin/\n",
" check deploy instance and run regression tests on it\n",
" contribcheck run tests of modules in contrib/\n",
@@ -693,6 +727,7 @@ sub usage
"\nOptions for <arg>: (used by contribcheck and modulescheck)\n",
" install also run tests which require a new instance\n",
"\nOption for <arg>: for taptest\n",
- " TEST_DIR (required) directory where tests reside\n";
+ " TEST_DIR (required) directory where tests reside\n",
+ " PROVE_FLAGS flags to pass to prove\n";
exit(1);
}
--
2.17.1
--tKkaNMvYmhQvRCRK
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="0010-tmp-run-tap-tests-first.patch"
^ permalink raw reply [nested|flat] 45+ messages in thread
* [PATCH 08/19] vcregress: add alltaptests
@ 2022-02-25 23:00 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 45+ messages in thread
From: Justin Pryzby @ 2022-02-25 23:00 UTC (permalink / raw)
https://www.postgresql.org/message-id/[email protected]
https://www.postgresql.org/message-id/flat/[email protected]
In passing, document taptest PROVE_FLAGS
ci-os-only: windows
See also:
d835dd6685246f0737ca42ab68242210681bb220
13d856e177e69083f543d6383eeda9e12ce3c55c
fed6df486dca1b9e53d3f560031b9a236c99f4bb
---
.cirrus.yml | 10 ++-------
src/tools/msvc/vcregress.pl | 43 +++++++++++++++++++++++++++++++++----
2 files changed, 41 insertions(+), 12 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index f2bb05266cc..ea1f493923a 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -437,14 +437,8 @@ task:
test_ssl_script: |
set with_ssl=openssl
%T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/ssl/
- test_subscription_script: |
- %T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/subscription/
- test_authentication_script: |
- %T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/authentication/
- test_recovery_script: |
- %T_C% perl src/tools/msvc/vcregress.pl recoverycheck
- test_bin_script: |
- %T_C% perl src/tools/msvc/vcregress.pl bincheck
+ test_tap_script: |
+ %T_C% perl src/tools/msvc/vcregress.pl alltaptests
test_ecpg_script: |
rem tries to build additional stuff
vcvarsall x64
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index 54c31b0e14f..1b1387acac0 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -51,7 +51,7 @@ if (-e "src/tools/msvc/buildenv.pl")
my $what = shift || "";
if ($what =~
- /^(check|installcheck|plcheck|contribcheck|modulescheck|ecpgcheck|isolationcheck|upgradecheck|bincheck|recoverycheck|taptest)$/i
+ /^(check|installcheck|plcheck|contribcheck|modulescheck|ecpgcheck|isolationcheck|upgradecheck|bincheck|recoverycheck|taptest|alltaptests)$/i
)
{
$what = uc $what;
@@ -109,6 +109,7 @@ my %command = (
BINCHECK => \&bincheck,
RECOVERYCHECK => \&recoverycheck,
UPGRADECHECK => \&upgradecheck, # no-op
+ ALLTAPTESTS => \&alltaptests,
TAPTEST => \&taptest,);
my $proc = $command{$what};
@@ -291,6 +292,9 @@ sub tap_check
$ENV{PG_REGRESS} = "$topdir/$Config/pg_regress/pg_regress";
$ENV{REGRESS_SHLIB} = "$topdir/src/test/regress/regress.dll";
+ print "============================================================\n";
+ print "Checking $dir: @args\n";
+
rmtree('tmp_check');
system(@args);
my $status = $? >> 8;
@@ -305,8 +309,7 @@ sub bincheck
my $mstat = 0;
- # Find out all the existing TAP tests by looking for t/ directories
- # in the tree.
+ # Find the TAP tests by looking for t/ directories
my @bin_dirs = glob("$topdir/src/bin/*");
# Process each test
@@ -321,6 +324,36 @@ sub bincheck
return;
}
+sub alltaptests
+{
+ InstallTemp();
+
+ my $mstat = 0;
+
+ # Find out all the existing TAP tests by looking for t/ directories
+ # in the tree.
+ my @tap_dirs = ();
+ my @top_dir = ($topdir);
+ File::Find::find(
+ { wanted => sub {
+ /^t\z/s
+ && $File::Find::name !~ /\/(kerberos|ldap|ssl|ssl_passphrase_callback)\// # opt-in
+ && push(@tap_dirs, $File::Find::name);
+ }
+ },
+ @top_dir);
+
+ # Process each test
+ foreach my $test_path (@tap_dirs)
+ {
+ my $dir = dirname($test_path);
+ my $status = tap_check($dir);
+ $mstat ||= $status;
+ }
+ exit $mstat if $mstat;
+ return;
+}
+
sub taptest
{
my $dir = shift;
@@ -671,6 +704,7 @@ sub usage
print STDERR
"Usage: vcregress.pl <mode> [<arg>]\n\n",
"Options for <mode>:\n",
+ " alltaptests run all tap tests (except kerberos, ldap, ssl, ssl_passphrase_callback)\n",
" bincheck run tests of utilities in src/bin/\n",
" check deploy instance and run regression tests on it\n",
" contribcheck run tests of modules in contrib/\n",
@@ -688,6 +722,7 @@ sub usage
"\nOptions for <arg>: (used by contribcheck and modulescheck)\n",
" install also run tests which require a new instance\n",
"\nOption for <arg>: for taptest\n",
- " TEST_DIR (required) directory where tests reside\n";
+ " TEST_DIR (required) directory where tests reside\n",
+ " PROVE_FLAGS flags to pass to prove\n";
exit(1);
}
--
2.17.1
--Sw7tCqrGA+HQ0/zt
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="0009-tmp-run-tap-tests-first.patch"
^ permalink raw reply [nested|flat] 45+ messages in thread
* Re: explain analyze rows=%.0f
@ 2022-06-22 19:11 Ibrar Ahmed <[email protected]>
parent: Tom Lane <[email protected]>
1 sibling, 1 reply; 45+ messages in thread
From: Ibrar Ahmed @ 2022-06-22 19:11 UTC (permalink / raw)
To: pgsql-hackers; +Cc: Robert Haas <[email protected]>; Simon Riggs <[email protected]>; Ron Mayer <[email protected]>; Euler Taveira de Oliveira <[email protected]>; Tom Lane <[email protected]>
On Thu, Jun 23, 2022 at 12:01 AM Tom Lane <[email protected]> wrote:
> Robert Haas <[email protected]> writes:
> > On Jun 2, 2009, at 9:41 AM, Simon Riggs <[email protected]> wrote:
> >> You're right that the number of significant digits already exceeds the
> >> true accuracy of the computation. I think what Robert wants to see is
> >> the exact value used in the calc, so the estimates can be checked more
> >> thoroughly than is currently possible.
>
> > Bingo.
>
> Uh, the planner's estimate *is* an integer. What was under discussion
> (I thought) was showing some fractional digits in the case where EXPLAIN
> ANALYZE is outputting a measured row count that is an average over
> multiple loops, and therefore isn't necessarily an integer. In that
> case the measured value can be considered arbitrarily precise --- though
> I think in practice one or two fractional digits would be plenty.
>
> regards, tom lane
>
>
> Hi,
I was looking at the TODO list and found that the issue requires
a quick fix. Attached is a patch which shows output like this. It shows the
fraction digits in case of loops > 1
postgres=# explain analyze select * from foo;
QUERY PLAN
--------------------------------------------------------------------------------------------------------------
Seq Scan on foo (cost=0.00..64414.79 rows=2326379 width=8) (actual
time=0.025..277.096 rows=2344671 loops=1
Planning Time: 0.516 ms
Execution Time: 356.993 ms
(3 rows)
postgres=# explain analyze select * from foo where b = (select c from
bar where c = 1);
QUERY PLAN
----------------------------------------------------------------------------------------------------------------------------
Seq Scan on foo (cost=8094.37..78325.11 rows=2326379 width=8)
(actual time=72.352..519.159 rows=2344671 loops=1
Filter: (b = $1)
InitPlan 1 (returns $1)
-> Gather (cost=1000.00..8094.37 rows=1 width=4) (actual
time=0.872..72.434 rows=1 loops=1
Workers Planned: 2
Workers Launched: 2
-> Parallel Seq Scan on bar (cost=0.00..7094.27 rows=1
width=4) (actual time=41.931..65.382 rows=0.33 loops=3)
Filter: (c = 1)
Rows Removed by Filter: 245457
Planning Time: 0.277 ms
Execution Time: 597.795 ms
(11 rows)
--
Ibrar Ahmed
Attachments:
[application/octet-stream] explain_float_row.patch (3.0K, ../../CALtqXTeQ+QCFT0DWcM5LOjnf9_Fz59+ALFejNEFwpFhW-9o4ag@mail.gmail.com/3-explain_float_row.patch)
download | inline diff:
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 5d1f7089da..954f64b802 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -1624,13 +1624,21 @@ ExplainNode(PlanState *planstate, List *ancestors,
if (es->format == EXPLAIN_FORMAT_TEXT)
{
if (es->timing)
+ {
+ appendStringInfo(es->str, " (actual time=%.3f..%.3f",
+ startup_ms, total_ms);
appendStringInfo(es->str,
- " (actual time=%.3f..%.3f rows=%.0f loops=%.0f)",
- startup_ms, total_ms, rows, nloops);
+ nloops == 1 ? " rows=%.0f loops=%.0f" :
+ " rows=%.2f loops=%.0f)",
+ rows, nloops);
+ }
else
+ {
appendStringInfo(es->str,
- " (actual rows=%.0f loops=%.0f)",
- rows, nloops);
+ nloops == 1 ?
+ " (actual rows=%.0f loops=%.0f": " rows=%.2f loops=%.0f)",
+ rows, nloops);
+ }
}
else
{
@@ -1641,7 +1649,7 @@ ExplainNode(PlanState *planstate, List *ancestors,
ExplainPropertyFloat("Actual Total Time", "ms", total_ms,
3, es);
}
- ExplainPropertyFloat("Actual Rows", NULL, rows, 0, es);
+ ExplainPropertyFloat("Actual Rows", NULL, rows, nloops == 1 ? 0 : 2, es);
ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
}
}
@@ -1690,13 +1698,20 @@ ExplainNode(PlanState *planstate, List *ancestors,
{
ExplainIndentText(es);
if (es->timing)
- appendStringInfo(es->str,
- "actual time=%.3f..%.3f rows=%.0f loops=%.0f\n",
- startup_ms, total_ms, rows, nloops);
+ {
+ appendStringInfo(es->str, "actual time=%.3f..%.3f",
+ startup_ms, total_ms);
+ appendStringInfo(es->str,
+ nloops == 1 ? " rows=%.0f loops=%.0f" :
+ " rows=%.2f loops=%.0f",
+ rows, nloops);
+ }
else
- appendStringInfo(es->str,
- "actual rows=%.0f loops=%.0f\n",
- rows, nloops);
+ {
+ appendStringInfo(es->str, nloops == 1 ?
+ "actual rows=%.0f loops=%.0f": " rows=%.2f loops=%.0f",
+ rows, nloops);
+ }
}
else
{
@@ -1707,7 +1722,7 @@ ExplainNode(PlanState *planstate, List *ancestors,
ExplainPropertyFloat("Actual Total Time", "ms",
total_ms, 3, es);
}
- ExplainPropertyFloat("Actual Rows", NULL, rows, 0, es);
+ ExplainPropertyFloat("Actual Rows", NULL, rows, nloops == 1 ? 0 : 2, es);
ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
}
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index ce12915592..baecc40e0c 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -1937,7 +1937,7 @@ _outPathInfo(StringInfo str, const Path *node)
WRITE_BOOL_FIELD(parallel_aware);
WRITE_BOOL_FIELD(parallel_safe);
WRITE_INT_FIELD(parallel_workers);
- WRITE_FLOAT_FIELD(rows, "%.0f");
+ WRITE_FLOAT_FIELD(rows, "%.2f");
WRITE_FLOAT_FIELD(startup_cost, "%.2f");
WRITE_FLOAT_FIELD(total_cost, "%.2f");
WRITE_NODE_FIELD(pathkeys);
^ permalink raw reply [nested|flat] 45+ messages in thread
* Re: explain analyze rows=%.0f
@ 2022-06-22 20:04 David G. Johnston <[email protected]>
parent: Ibrar Ahmed <[email protected]>
0 siblings, 1 reply; 45+ messages in thread
From: David G. Johnston @ 2022-06-22 20:04 UTC (permalink / raw)
To: Ibrar Ahmed <[email protected]>; +Cc: pgsql-hackers; Robert Haas <[email protected]>; Simon Riggs <[email protected]>; Ron Mayer <[email protected]>; Euler Taveira de Oliveira <[email protected]>; Tom Lane <[email protected]>
On Wed, Jun 22, 2022 at 12:11 PM Ibrar Ahmed <[email protected]> wrote:
> On Thu, Jun 23, 2022 at 12:01 AM Tom Lane <[email protected]> wrote:
>
>> Robert Haas <[email protected]> writes:
>> > On Jun 2, 2009, at 9:41 AM, Simon Riggs <[email protected]> wrote:
>> >> You're right that the number of significant digits already exceeds the
>> >> true accuracy of the computation. I think what Robert wants to see is
>> >> the exact value used in the calc, so the estimates can be checked more
>> >> thoroughly than is currently possible.
>>
>> > Bingo.
>>
>> Uh, the planner's estimate *is* an integer. What was under discussion
>> (I thought) was showing some fractional digits in the case where EXPLAIN
>> ANALYZE is outputting a measured row count that is an average over
>> multiple loops, and therefore isn't necessarily an integer. In that
>> case the measured value can be considered arbitrarily precise --- though
>> I think in practice one or two fractional digits would be plenty.
>>
>> regards, tom lane
>>
>>
>> Hi,
> I was looking at the TODO list and found that the issue requires
> a quick fix. Attached is a patch which shows output like this.
>
Quick code review:
+ "actual rows=%.0f loops=%.0f": " rows=%.2f loops=%.0f",
The leading space before the else block "rows" does not belong.
There should be a space after the colon.
The word "actual" that you are dropping in the else block seems like it
should belong - it is a header for the entire section not just a modifier
for the word "rows". This is evidenced by the timing block verbiage where
rows is standalone and the word actual comes before time. In short, only
the format specifier should change under the current scheme. Both sections.
- WRITE_FLOAT_FIELD(rows, "%.0f");
+ WRITE_FLOAT_FIELD(rows, "%.2f");
This one looks suspicious, though I haven't dug into the code to see
exactly what all is being touched. That it doesn't have an nloops
condition like everything else stands out.
Tooling that expects an integer is the only downside I see here, but I
concur that the usability of always showing two decimal places when nloops
> 1 overcomes any objection I have on those grounds.
David J.
^ permalink raw reply [nested|flat] 45+ messages in thread
* Re: explain analyze rows=%.0f
@ 2022-06-22 20:55 Ibrar Ahmed <[email protected]>
parent: David G. Johnston <[email protected]>
0 siblings, 2 replies; 45+ messages in thread
From: Ibrar Ahmed @ 2022-06-22 20:55 UTC (permalink / raw)
To: David G. Johnston <[email protected]>; +Cc: pgsql-hackers; Robert Haas <[email protected]>; Simon Riggs <[email protected]>; Ron Mayer <[email protected]>; Euler Taveira de Oliveira <[email protected]>; Tom Lane <[email protected]>
On Thu, Jun 23, 2022 at 1:04 AM David G. Johnston <
[email protected]> wrote:
> On Wed, Jun 22, 2022 at 12:11 PM Ibrar Ahmed <[email protected]>
> wrote:
>
>> On Thu, Jun 23, 2022 at 12:01 AM Tom Lane <[email protected]> wrote:
>>
>>> Robert Haas <[email protected]> writes:
>>> > On Jun 2, 2009, at 9:41 AM, Simon Riggs <[email protected]> wrote:
>>> >> You're right that the number of significant digits already exceeds the
>>> >> true accuracy of the computation. I think what Robert wants to see is
>>> >> the exact value used in the calc, so the estimates can be checked more
>>> >> thoroughly than is currently possible.
>>>
>>> > Bingo.
>>>
>>> Uh, the planner's estimate *is* an integer. What was under discussion
>>> (I thought) was showing some fractional digits in the case where EXPLAIN
>>> ANALYZE is outputting a measured row count that is an average over
>>> multiple loops, and therefore isn't necessarily an integer. In that
>>> case the measured value can be considered arbitrarily precise --- though
>>> I think in practice one or two fractional digits would be plenty.
>>>
>>> regards, tom lane
>>>
>>>
>>> Hi,
>> I was looking at the TODO list and found that the issue requires
>> a quick fix. Attached is a patch which shows output like this.
>>
>
> Quick code review:
>
> + "actual rows=%.0f loops=%.0f": " rows=%.2f loops=%.0f",
>
> The leading space before the else block "rows" does not belong.
>
> There should be a space after the colon.
>
> Thanks, David for your quick response. I have updated the patch.
> The word "actual" that you are dropping in the else block seems like it
> should belong - it is a header for the entire section not just a modifier
> for the word "rows". This is evidenced by the timing block verbiage where
> rows is standalone and the word actual comes before time. In short, only
> the format specifier should change under the current scheme. Both sections.
>
> - WRITE_FLOAT_FIELD(rows, "%.0f");
> + WRITE_FLOAT_FIELD(rows, "%.2f");
>
> This one looks suspicious, though I haven't dug into the code to see
> exactly what all is being touched. That it doesn't have an nloops
> condition like everything else stands out.
>
> I was also thinking about that, but I don't see any harm when we
ultimately truncating that decimal
at a latter stage of code in case of loop = 1.
> Tooling that expects an integer is the only downside I see here, but I
> concur that the usability of always showing two decimal places when nloops
> > 1 overcomes any objection I have on those grounds.
>
> David J.
>
>
--
Ibrar Ahmed
Attachments:
[application/octet-stream] explain_float_row_v2.patch (3.4K, ../../CALtqXTcrXtwZ3RRg2FH8yt8mDxRypXrUbw6a=zp0x2fKm_5xNg@mail.gmail.com/3-explain_float_row_v2.patch)
download | inline diff:
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 47d80b0d25..bd60b55574 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -122,7 +122,7 @@ bool bsysscan = false;
* lookups as fast as possible.
*/
static FullTransactionId XactTopFullTransactionId = {InvalidTransactionId};
-static int nParallelCurrentXids = 0;
+static int nParallelCurrentXids = 0;
static TransactionId *ParallelCurrentXids;
/*
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 5d1f7089da..d53c9c7235 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -1624,13 +1624,20 @@ ExplainNode(PlanState *planstate, List *ancestors,
if (es->format == EXPLAIN_FORMAT_TEXT)
{
if (es->timing)
+ {
+ appendStringInfo(es->str, " (actual time=%.3f..%.3f",
+ startup_ms, total_ms);
appendStringInfo(es->str,
- " (actual time=%.3f..%.3f rows=%.0f loops=%.0f)",
- startup_ms, total_ms, rows, nloops);
+ nloops == 1 ? " rows=%.0f loops=%.0f)" : " rows=%.2f loops=%.0f)",
+ rows, nloops);
+ }
else
+ {
appendStringInfo(es->str,
- " (actual rows=%.0f loops=%.0f)",
+ nloops == 1 ?
+ " (actual rows=%.0f loops=%.0f)" : " (actual rows=%.2f loops=%.0f)",
rows, nloops);
+ }
}
else
{
@@ -1641,7 +1648,7 @@ ExplainNode(PlanState *planstate, List *ancestors,
ExplainPropertyFloat("Actual Total Time", "ms", total_ms,
3, es);
}
- ExplainPropertyFloat("Actual Rows", NULL, rows, 0, es);
+ ExplainPropertyFloat("Actual Rows", NULL, rows, nloops == 1 ? 0 : 2, es);
ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
}
}
@@ -1690,13 +1697,19 @@ ExplainNode(PlanState *planstate, List *ancestors,
{
ExplainIndentText(es);
if (es->timing)
+ {
+ appendStringInfo(es->str, "actual time=%.3f..%.3f",
+ startup_ms, total_ms);
appendStringInfo(es->str,
- "actual time=%.3f..%.3f rows=%.0f loops=%.0f\n",
- startup_ms, total_ms, rows, nloops);
+ nloops == 1 ? " rows=%.0f loops=%.0f" : " rows=%.2f loops=%.0f",
+ rows, nloops);
+ }
else
- appendStringInfo(es->str,
- "actual rows=%.0f loops=%.0f\n",
+ {
+ appendStringInfo(es->str, nloops == 1 ?
+ "actual rows=%.0f loops=%.0f" : "actual rows=%.2f loops=%.0f",
rows, nloops);
+ }
}
else
{
@@ -1707,7 +1720,7 @@ ExplainNode(PlanState *planstate, List *ancestors,
ExplainPropertyFloat("Actual Total Time", "ms",
total_ms, 3, es);
}
- ExplainPropertyFloat("Actual Rows", NULL, rows, 0, es);
+ ExplainPropertyFloat("Actual Rows", NULL, rows, nloops == 1 ? 0 : 2, es);
ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
}
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index ce12915592..baecc40e0c 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -1937,7 +1937,7 @@ _outPathInfo(StringInfo str, const Path *node)
WRITE_BOOL_FIELD(parallel_aware);
WRITE_BOOL_FIELD(parallel_safe);
WRITE_INT_FIELD(parallel_workers);
- WRITE_FLOAT_FIELD(rows, "%.0f");
+ WRITE_FLOAT_FIELD(rows, "%.2f");
WRITE_FLOAT_FIELD(startup_cost, "%.2f");
WRITE_FLOAT_FIELD(total_cost, "%.2f");
WRITE_NODE_FIELD(pathkeys);
^ permalink raw reply [nested|flat] 45+ messages in thread
* Re: explain analyze rows=%.0f
@ 2022-07-07 09:41 Amit Kapila <[email protected]>
parent: Ibrar Ahmed <[email protected]>
1 sibling, 1 reply; 45+ messages in thread
From: Amit Kapila @ 2022-07-07 09:41 UTC (permalink / raw)
To: Ibrar Ahmed <[email protected]>; +Cc: David G. Johnston <[email protected]>; pgsql-hackers; Robert Haas <[email protected]>; Simon Riggs <[email protected]>; Ron Mayer <[email protected]>; Euler Taveira de Oliveira <[email protected]>; Tom Lane <[email protected]>
On Thu, Jun 23, 2022 at 2:25 AM Ibrar Ahmed <[email protected]> wrote:
>
> On Thu, Jun 23, 2022 at 1:04 AM David G. Johnston <[email protected]> wrote:
>>
>> - WRITE_FLOAT_FIELD(rows, "%.0f");
>> + WRITE_FLOAT_FIELD(rows, "%.2f");
>>
>> This one looks suspicious, though I haven't dug into the code to see exactly what all is being touched. That it doesn't have an nloops condition like everything else stands out.
>>
> I was also thinking about that, but I don't see any harm when we ultimately truncating that decimal
> at a latter stage of code in case of loop = 1.
>
That change is in the path node which we anyway not going to target as
part of this change. We only want to change the display for actual
rows in Explain Analyze. So, I can't see how the quoted change can
help in any way.
Few miscellaneous comments:
========================
*
static FullTransactionId XactTopFullTransactionId = {InvalidTransactionId};
-static int nParallelCurrentXids = 0;
+static int nParallelCurrentXids = 0;
I don't see why this change is required.
* Can you please add a comment explaining why we are making this
change for actual rows?
* Can you please write a test case unless there is some existing test
that covers the change by displaying actual rows values in decimal but
in that case patch should have that changed output test? If you don't
think we can reliably write such a test then please let me know the
reason?
--
With Regards,
Amit Kapila.
^ permalink raw reply [nested|flat] 45+ messages in thread
* Re: explain analyze rows=%.0f
@ 2022-07-07 10:14 vignesh C <[email protected]>
parent: Ibrar Ahmed <[email protected]>
1 sibling, 2 replies; 45+ messages in thread
From: vignesh C @ 2022-07-07 10:14 UTC (permalink / raw)
To: Ibrar Ahmed <[email protected]>; +Cc: David G. Johnston <[email protected]>; pgsql-hackers; Robert Haas <[email protected]>; Simon Riggs <[email protected]>; Ron Mayer <[email protected]>; Euler Taveira de Oliveira <[email protected]>; Tom Lane <[email protected]>
On Thu, Jun 23, 2022 at 2:25 AM Ibrar Ahmed <[email protected]> wrote:
>
>
>
> On Thu, Jun 23, 2022 at 1:04 AM David G. Johnston <[email protected]> wrote:
>>
>> On Wed, Jun 22, 2022 at 12:11 PM Ibrar Ahmed <[email protected]> wrote:
>>>
>>> On Thu, Jun 23, 2022 at 12:01 AM Tom Lane <[email protected]> wrote:
>>>>
>>>> Robert Haas <[email protected]> writes:
>>>> > On Jun 2, 2009, at 9:41 AM, Simon Riggs <[email protected]> wrote:
>>>> >> You're right that the number of significant digits already exceeds the
>>>> >> true accuracy of the computation. I think what Robert wants to see is
>>>> >> the exact value used in the calc, so the estimates can be checked more
>>>> >> thoroughly than is currently possible.
>>>>
>>>> > Bingo.
>>>>
>>>> Uh, the planner's estimate *is* an integer. What was under discussion
>>>> (I thought) was showing some fractional digits in the case where EXPLAIN
>>>> ANALYZE is outputting a measured row count that is an average over
>>>> multiple loops, and therefore isn't necessarily an integer. In that
>>>> case the measured value can be considered arbitrarily precise --- though
>>>> I think in practice one or two fractional digits would be plenty.
>>>>
>>>> regards, tom lane
>>>>
>>>>
>>> Hi,
>>> I was looking at the TODO list and found that the issue requires
>>> a quick fix. Attached is a patch which shows output like this.
>>
>>
>> Quick code review:
>>
>> + "actual rows=%.0f loops=%.0f": " rows=%.2f loops=%.0f",
>>
>> The leading space before the else block "rows" does not belong.
>>
>> There should be a space after the colon.
>>
> Thanks, David for your quick response. I have updated the patch.
>
>>
>> The word "actual" that you are dropping in the else block seems like it should belong - it is a header for the entire section not just a modifier for the word "rows". This is evidenced by the timing block verbiage where rows is standalone and the word actual comes before time. In short, only the format specifier should change under the current scheme. Both sections.
>>
>> - WRITE_FLOAT_FIELD(rows, "%.0f");
>> + WRITE_FLOAT_FIELD(rows, "%.2f");
>>
>> This one looks suspicious, though I haven't dug into the code to see exactly what all is being touched. That it doesn't have an nloops condition like everything else stands out.
>>
> I was also thinking about that, but I don't see any harm when we ultimately truncating that decimal
> at a latter stage of code in case of loop = 1.
Thanks for the patch.
1) There are some existing regression tests that are failing, you
should update the expect files accordingly for the same:
--- /home/vignesh/postgres/src/test/regress/expected/select_parallel.out
2022-05-18 20:51:46.874818044 +0530
+++ /home/vignesh/postgres/src/test/regress/results/select_parallel.out
2022-07-07 15:27:34.450440922 +0530
@@ -545,17 +545,17 @@
explain (analyze, timing off, summary off, costs off)
select count(*) from tenk1, tenk2 where tenk1.hundred > 1
and tenk2.thousand=0;
- QUERY PLAN
---------------------------------------------------------------------------
+ QUERY PLAN
+-----------------------------------------------------------------------------
Aggregate (actual rows=1 loops=1)
-> Nested Loop (actual rows=98000 loops=1)
-> Seq Scan on tenk2 (actual rows=10 loops=1)
Filter: (thousand = 0)
Rows Removed by Filter: 9990
- -> Gather (actual rows=9800 loops=10)
+ -> Gather (actual rows=9800.00 loops=10)
Workers Planned: 4
Workers Launched: 4
- -> Parallel Seq Scan on tenk1 (actual rows=1960 loops=50)
+ -> Parallel Seq Scan on tenk1 (actual rows=1960.00 loops=50)
Filter: (hundred > 1)
test select_parallel ... FAILED 744 ms
partition_prune ... FAILED 861 ms
explain ... FAILED 134 ms
memoize ... FAILED 250 ms
2) This change is not required as part of this patch:
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -122,7 +122,7 @@ bool bsysscan = false;
* lookups as fast as possible.
*/
static FullTransactionId XactTopFullTransactionId = {InvalidTransactionId};
-static int nParallelCurrentXids = 0;
+static int nParallelCurrentXids = 0;
static TransactionId *ParallelCurrentXids;
Regards,
Vignesh
^ permalink raw reply [nested|flat] 45+ messages in thread
* Re: explain analyze rows=%.0f
@ 2022-07-07 17:53 Greg Stark <[email protected]>
parent: vignesh C <[email protected]>
1 sibling, 2 replies; 45+ messages in thread
From: Greg Stark @ 2022-07-07 17:53 UTC (permalink / raw)
To: vignesh C <[email protected]>; +Cc: Ibrar Ahmed <[email protected]>; David G. Johnston <[email protected]>; pgsql-hackers; Robert Haas <[email protected]>; Simon Riggs <[email protected]>; Ron Mayer <[email protected]>; Euler Taveira de Oliveira <[email protected]>; Tom Lane <[email protected]>
> - -> Parallel Seq Scan on tenk1 (actual rows=1960 loops=50)
> + -> Parallel Seq Scan on tenk1 (actual rows=1960.00
At the not inconsiderable risk of bike-shedding....
I'm wondering if printing something like 0.00 will be somewhat
deceptive when the real value is non-zero but less than 1 row per 200
loops. I wonder if the number of decimal places should be calculated
to produce a minimum of one non-zero digit for non-zero values.
--
greg
^ permalink raw reply [nested|flat] 45+ messages in thread
* Re: explain analyze rows=%.0f
@ 2022-07-07 20:21 Robert Haas <[email protected]>
parent: Greg Stark <[email protected]>
1 sibling, 3 replies; 45+ messages in thread
From: Robert Haas @ 2022-07-07 20:21 UTC (permalink / raw)
To: Greg Stark <[email protected]>; +Cc: vignesh C <[email protected]>; Ibrar Ahmed <[email protected]>; David G. Johnston <[email protected]>; pgsql-hackers; Simon Riggs <[email protected]>; Ron Mayer <[email protected]>; Euler Taveira de Oliveira <[email protected]>; Tom Lane <[email protected]>
On Thu, Jul 7, 2022 at 1:53 PM Greg Stark <[email protected]> wrote:
> > - -> Parallel Seq Scan on tenk1 (actual rows=1960 loops=50)
> > + -> Parallel Seq Scan on tenk1 (actual rows=1960.00
>
> At the not inconsiderable risk of bike-shedding....
>
> I'm wondering if printing something like 0.00 will be somewhat
> deceptive when the real value is non-zero but less than 1 row per 200
> loops. I wonder if the number of decimal places should be calculated
> to produce a minimum of one non-zero digit for non-zero values.
I mean, what I really want here if I'm honest is to not have the
system divide the number of rows by the loop count. And it sort of
sounds like maybe that's what you want, too. You want to know whether
the loop count is actually zero, not whether it's close to zero when
you divide it by some number that might be gigantic.
Parallel query's treatment of this topic has come in for some
criticism, but I don't know what else it could really do: there could
be any number of loops in each backend, and it need not be the same
across all backends, so all it can do is add up the loop counts just
like it adds up the row counts and times. And if we just printed out
those totals, the result would be understandable by everyone. But we
insist on dividing it by the loop count, and then things get really
obscure. Consider this example, which does not involve parallel query:
Nested Loop (actual time=TIME FOR THIS AND ALL CHILDREN rows=THE REAL
ROW COUNT loops=1)
-> Seq Scan on something (actual time=THE TIME IT REALLY TOOK rows=THE
REAL ROW COUNT loops=1)
-> Index Scan using someidx on somethingelse (actual time=NOT REALLY
HOW LONG IT TOOK rows=NOT REALLY HOW MANY ROWS WE GOT loops=HUGE
NUMBER)
If I'm looking at this plan and trying to find out what's gone wrong,
I want to know how much time got spent in the nested loop, how much
time got spent in the Seq Scan, and how much time got spent in the
Index Scan. It's easy to figure out how much time got spent in the Seq
Scan, but to find out how much time got spent in the Index Scan, I
have to multiply the time by the loop count. Then, I have to add that
number to the time spent in the Seq Scan and subtract that from the
time from the nested loop to find the time spent on the nested loop
itself. This is quite a lot of computation, especially if the plan
involves a dozen or two different nested loops, and if we didn't
insist on dividing the time by the loop count, it would be MUCH EASIER
to figure out whether the time spent in the Index Scan is a
significant percentage of the total time or not.
And likewise, if you're trying to understand the row count for the
nested loop, it would be a heck of a lot simpler if you could see the
*raw* row count for the index scan. It's unclear to me what value
there ever is in knowing that the number of rows per iteration was
about 0 or about 1 or about 2. The only thing I'm ever going to do
with the row count that gets printed here is multiply it by the loop
count and then try to figure out how much precision I've lost because
of limits on the number of decimal places. Right now that's basically
all of it because nearly every case ends up with the index scan having
rows=1, so even just adding 2 decimal places will help a lot. But I'm
still just going to be reverse engineering what I really want to know,
which is the original number, from what the system gives me, which is
a needlessly-obfuscated version of that value.
Grumble, grumble. It's sad that it's been 13 years and we haven't done
anything about this.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 45+ messages in thread
* Re: explain analyze rows=%.0f
@ 2022-07-07 20:45 Peter Geoghegan <[email protected]>
parent: Robert Haas <[email protected]>
2 siblings, 0 replies; 45+ messages in thread
From: Peter Geoghegan @ 2022-07-07 20:45 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Greg Stark <[email protected]>; vignesh C <[email protected]>; Ibrar Ahmed <[email protected]>; David G. Johnston <[email protected]>; pgsql-hackers; Simon Riggs <[email protected]>; Ron Mayer <[email protected]>; Euler Taveira de Oliveira <[email protected]>; Tom Lane <[email protected]>
On Thu, Jul 7, 2022 at 1:21 PM Robert Haas <[email protected]> wrote:
> Nested Loop (actual time=TIME FOR THIS AND ALL CHILDREN rows=THE REAL
> ROW COUNT loops=1)
> -> Seq Scan on something (actual time=THE TIME IT REALLY TOOK rows=THE
> REAL ROW COUNT loops=1)
> -> Index Scan using someidx on somethingelse (actual time=NOT REALLY
> HOW LONG IT TOOK rows=NOT REALLY HOW MANY ROWS WE GOT loops=HUGE
> NUMBER)
>
> If I'm looking at this plan and trying to find out what's gone wrong,
> I want to know how much time got spent in the nested loop, how much
> time got spent in the Seq Scan, and how much time got spent in the
> Index Scan. It's easy to figure out how much time got spent in the Seq
> Scan, but to find out how much time got spent in the Index Scan, I
> have to multiply the time by the loop count.
I agree that this general state of affairs is very confusing, and
seems like something that we should still improve. Just because it's a
very old way of presenting the information doesn't mean that it's the
best one, or even a particularly good one. Plus you could probably
make some kind of concession in the direction of maintaining
compatibility with the current approach if you had to. Right?
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 45+ messages in thread
* Re: explain analyze rows=%.0f
@ 2022-07-07 22:20 Justin Pryzby <[email protected]>
parent: Robert Haas <[email protected]>
2 siblings, 1 reply; 45+ messages in thread
From: Justin Pryzby @ 2022-07-07 22:20 UTC (permalink / raw)
To: Peter Geoghegan <[email protected]>; Robert Haas <[email protected]>; +Cc: Greg Stark <[email protected]>; vignesh C <[email protected]>; Ibrar Ahmed <[email protected]>; David G. Johnston <[email protected]>; pgsql-hackers; Simon Riggs <[email protected]>; Ron Mayer <[email protected]>; Euler Taveira de Oliveira <[email protected]>; Tom Lane <[email protected]>
On Thu, Jul 07, 2022 at 04:21:37PM -0400, Robert Haas wrote:
> I mean, what I really want here if I'm honest is to not have the
> system divide the number of rows by the loop count. And it sort of
> sounds like maybe that's what you want, too. You want to know whether
> the loop count is actually zero, not whether it's close to zero when
> you divide it by some number that might be gigantic.
...
> involves a dozen or two different nested loops, and if we didn't
> insist on dividing the time by the loop count, it would be MUCH EASIER
> to figure out whether the time spent in the Index Scan is a
> significant percentage of the total time or not.
I think the guiding princible for what to do should be to reduce how much is
needed to explain about how to interpret what explain is showing...
The docs say this:
| In such cases, the loops value reports the total number of executions of the
| node, and the actual time and rows values shown are averages per-execution.
| This is done to make the numbers comparable with the way that the cost
| estimates are shown. Multiply by the loops value to get the total time
| actually spent in the node.
On Thu, Jul 07, 2022 at 01:45:19PM -0700, Peter Geoghegan wrote:
> Plus you could probably
> make some kind of concession in the direction of maintaining
> compatibility with the current approach if you had to. Right?
The minimum would be to show the information in a way that makes it clear that
it's "new style" output showing a total and not an average, so that a person
who sees it knows how to interpret it (same for the web "explain tools")
A concession would be to show the current information *plus* total/raw values.
This thread is about how to display the existing values. But note that there's
a CF entry for also collecting more values to show things like min/max rows per
loop.
https://commitfest.postgresql.org/38/2765/
Add extra statistics to explain for Nested Loop
--
Justin
^ permalink raw reply [nested|flat] 45+ messages in thread
* Re: explain analyze rows=%.0f
@ 2022-07-08 18:09 Ibrar Ahmed <[email protected]>
parent: Amit Kapila <[email protected]>
0 siblings, 0 replies; 45+ messages in thread
From: Ibrar Ahmed @ 2022-07-08 18:09 UTC (permalink / raw)
To: Amit Kapila <[email protected]>; +Cc: David G. Johnston <[email protected]>; pgsql-hackers; Robert Haas <[email protected]>; Simon Riggs <[email protected]>; Ron Mayer <[email protected]>; Euler Taveira de Oliveira <[email protected]>; Tom Lane <[email protected]>
On Thu, Jul 7, 2022 at 2:41 PM Amit Kapila <[email protected]> wrote:
> On Thu, Jun 23, 2022 at 2:25 AM Ibrar Ahmed <[email protected]> wrote:
> >
> > On Thu, Jun 23, 2022 at 1:04 AM David G. Johnston <
> [email protected]> wrote:
> >>
> >> - WRITE_FLOAT_FIELD(rows, "%.0f");
> >> + WRITE_FLOAT_FIELD(rows, "%.2f");
> >>
> >> This one looks suspicious, though I haven't dug into the code to see
> exactly what all is being touched. That it doesn't have an nloops
> condition like everything else stands out.
> >>
> > I was also thinking about that, but I don't see any harm when we
> ultimately truncating that decimal
> > at a latter stage of code in case of loop = 1.
> >
>
> That change is in the path node which we anyway not going to target as
> part of this change. We only want to change the display for actual
> rows in Explain Analyze. So, I can't see how the quoted change can
> help in any way.
>
> Agreed removed.
> Few miscellaneous comments:
> ========================
> *
> static FullTransactionId XactTopFullTransactionId =
> {InvalidTransactionId};
> -static int nParallelCurrentXids = 0;
> +static int nParallelCurrentXids = 0;
>
> Removed.
> I don't see why this change is required.
>
> * Can you please add a comment explaining why we are making this
> change for actual rows?
>
Done
>
> * Can you please write a test case unless there is some existing test
> that covers the change by displaying actual rows values in decimal but
> in that case patch should have that changed output test? If you don't
> think we can reliably write such a test then please let me know the
> reason?
>
> I think there are tests, and I have updated the results accordingly.
> --
> With Regards,
> Amit Kapila.
>
--
Ibrar Ahmed
Attachments:
[application/octet-stream] explain_float_row_v3.patch (6.2K, ../../CALtqXTdVDc=GUHQzd2v38J_nhm1tsiZobKmmyotmVij7TSyhkQ@mail.gmail.com/3-explain_float_row_v3.patch)
download | inline diff:
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index e29c2ae206..ae078d86de 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -13,6 +13,8 @@
*/
#include "postgres.h"
+#include <math.h>
+
#include "access/xact.h"
#include "catalog/pg_type.h"
#include "commands/createas.h"
@@ -54,6 +56,9 @@ explain_get_index_name_hook_type explain_get_index_name_hook = NULL;
#define X_CLOSE_IMMEDIATE 2
#define X_NOWHITESPACE 4
+/* Check if float/double has any decimal number */
+#define HAS_DECIMAL(x) (floor(x) != x)
+
static void ExplainOneQuery(Query *query, int cursorOptions,
IntoClause *into, ExplainState *es,
const char *queryString, ParamListInfo params,
@@ -1643,16 +1648,29 @@ ExplainNode(PlanState *planstate, List *ancestors,
double total_ms = 1000.0 * planstate->instrument->total / nloops;
double rows = planstate->instrument->ntuples / nloops;
+ /*
+ * If the number of loops is greater than one, display the
+ * actual rows up to two decimal places instead of rounding
+ * off the value.
+ */
if (es->format == EXPLAIN_FORMAT_TEXT)
{
if (es->timing)
+ {
+ appendStringInfo(es->str, " (actual time=%.3f..%.3f",
+ startup_ms, total_ms);
appendStringInfo(es->str,
- " (actual time=%.3f..%.3f rows=%.0f loops=%.0f)",
- startup_ms, total_ms, rows, nloops);
+ (nloops == 1 || !HAS_DECIMAL(rows)) ?
+ " rows=%.0f loops=%.0f)" : " rows=%.2f loops=%.0f)",
+ rows, nloops);
+ }
else
+ {
appendStringInfo(es->str,
- " (actual rows=%.0f loops=%.0f)",
+ (nloops == 1 || !HAS_DECIMAL(rows)) ?
+ " (actual rows=%.0f loops=%.0f)" : " (actual rows=%.2f loops=%.0f)",
rows, nloops);
+ }
}
else
{
@@ -1663,7 +1681,7 @@ ExplainNode(PlanState *planstate, List *ancestors,
ExplainPropertyFloat("Actual Total Time", "ms", total_ms,
3, es);
}
- ExplainPropertyFloat("Actual Rows", NULL, rows, 0, es);
+ ExplainPropertyFloat("Actual Rows", NULL, rows, (nloops == 1 || !HAS_DECIMAL(rows)) ? 0 : 2, es);
ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
}
}
@@ -1707,18 +1725,30 @@ ExplainNode(PlanState *planstate, List *ancestors,
rows = instrument->ntuples / nloops;
ExplainOpenWorker(n, es);
+ /*
+ * If the number of loops is greater than one, display the
+ * actual rows up to two decimal places instead of rounding
+ * off the value.
+ */
if (es->format == EXPLAIN_FORMAT_TEXT)
{
ExplainIndentText(es);
if (es->timing)
+ {
+ appendStringInfo(es->str, "actual time=%.3f..%.3f",
+ startup_ms, total_ms);
appendStringInfo(es->str,
- "actual time=%.3f..%.3f rows=%.0f loops=%.0f\n",
- startup_ms, total_ms, rows, nloops);
+ (nloops == 1 || !HAS_DECIMAL(rows)) ?
+ " rows=%.0f loops=%.0f" : " rows=%.2f loops=%.0f",
+ rows, nloops);
+ }
else
- appendStringInfo(es->str,
- "actual rows=%.0f loops=%.0f\n",
+ {
+ appendStringInfo(es->str, (nloops == 1 || !HAS_DECIMAL(rows)) ?
+ "actual rows=%.0f loops=%.0f" : "actual rows=%.2f loops=%.0f",
rows, nloops);
+ }
}
else
{
@@ -1729,7 +1759,8 @@ ExplainNode(PlanState *planstate, List *ancestors,
ExplainPropertyFloat("Actual Total Time", "ms",
total_ms, 3, es);
}
- ExplainPropertyFloat("Actual Rows", NULL, rows, 0, es);
+ ExplainPropertyFloat("Actual Rows", NULL, rows,
+ (nloops == 1 || !HAS_DECIMAL(rows)) ? 0 : 2, es);
ExplainPropertyFloat("Actual Loops", NULL, nloops, 0, es);
}
diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out
index 7555764c77..002e5cc33d 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -2635,14 +2635,14 @@ order by tbl1.col1, tprt.col1;
insert into tbl1 values (1001), (1010), (1011);
explain (analyze, costs off, summary off, timing off)
select * from tbl1 inner join tprt on tbl1.col1 > tprt.col1;
- QUERY PLAN
---------------------------------------------------------------------------
+ QUERY PLAN
+-----------------------------------------------------------------------------
Nested Loop (actual rows=23 loops=1)
-> Seq Scan on tbl1 (actual rows=5 loops=1)
- -> Append (actual rows=5 loops=5)
+ -> Append (actual rows=4.60 loops=5)
-> Index Scan using tprt1_idx on tprt_1 (actual rows=2 loops=5)
Index Cond: (col1 < tbl1.col1)
- -> Index Scan using tprt2_idx on tprt_2 (actual rows=3 loops=4)
+ -> Index Scan using tprt2_idx on tprt_2 (actual rows=2.75 loops=4)
Index Cond: (col1 < tbl1.col1)
-> Index Scan using tprt3_idx on tprt_3 (actual rows=1 loops=2)
Index Cond: (col1 < tbl1.col1)
@@ -2656,16 +2656,16 @@ select * from tbl1 inner join tprt on tbl1.col1 > tprt.col1;
explain (analyze, costs off, summary off, timing off)
select * from tbl1 inner join tprt on tbl1.col1 = tprt.col1;
- QUERY PLAN
---------------------------------------------------------------------------
+ QUERY PLAN
+-----------------------------------------------------------------------------
Nested Loop (actual rows=3 loops=1)
-> Seq Scan on tbl1 (actual rows=5 loops=1)
- -> Append (actual rows=1 loops=5)
+ -> Append (actual rows=0.60 loops=5)
-> Index Scan using tprt1_idx on tprt_1 (never executed)
Index Cond: (col1 = tbl1.col1)
-> Index Scan using tprt2_idx on tprt_2 (actual rows=1 loops=2)
Index Cond: (col1 = tbl1.col1)
- -> Index Scan using tprt3_idx on tprt_3 (actual rows=0 loops=3)
+ -> Index Scan using tprt3_idx on tprt_3 (actual rows=0.33 loops=3)
Index Cond: (col1 = tbl1.col1)
-> Index Scan using tprt4_idx on tprt_4 (never executed)
Index Cond: (col1 = tbl1.col1)
^ permalink raw reply [nested|flat] 45+ messages in thread
* Re: explain analyze rows=%.0f
@ 2022-07-08 18:10 Ibrar Ahmed <[email protected]>
parent: vignesh C <[email protected]>
1 sibling, 0 replies; 45+ messages in thread
From: Ibrar Ahmed @ 2022-07-08 18:10 UTC (permalink / raw)
To: vignesh C <[email protected]>; +Cc: David G. Johnston <[email protected]>; pgsql-hackers; Robert Haas <[email protected]>; Simon Riggs <[email protected]>; Ron Mayer <[email protected]>; Euler Taveira de Oliveira <[email protected]>; Tom Lane <[email protected]>
On Thu, Jul 7, 2022 at 3:14 PM vignesh C <[email protected]> wrote:
> On Thu, Jun 23, 2022 at 2:25 AM Ibrar Ahmed <[email protected]> wrote:
> >
> >
> >
> > On Thu, Jun 23, 2022 at 1:04 AM David G. Johnston <
> [email protected]> wrote:
> >>
> >> On Wed, Jun 22, 2022 at 12:11 PM Ibrar Ahmed <[email protected]>
> wrote:
> >>>
> >>> On Thu, Jun 23, 2022 at 12:01 AM Tom Lane <[email protected]> wrote:
> >>>>
> >>>> Robert Haas <[email protected]> writes:
> >>>> > On Jun 2, 2009, at 9:41 AM, Simon Riggs <[email protected]>
> wrote:
> >>>> >> You're right that the number of significant digits already exceeds
> the
> >>>> >> true accuracy of the computation. I think what Robert wants to see
> is
> >>>> >> the exact value used in the calc, so the estimates can be checked
> more
> >>>> >> thoroughly than is currently possible.
> >>>>
> >>>> > Bingo.
> >>>>
> >>>> Uh, the planner's estimate *is* an integer. What was under discussion
> >>>> (I thought) was showing some fractional digits in the case where
> EXPLAIN
> >>>> ANALYZE is outputting a measured row count that is an average over
> >>>> multiple loops, and therefore isn't necessarily an integer. In that
> >>>> case the measured value can be considered arbitrarily precise ---
> though
> >>>> I think in practice one or two fractional digits would be plenty.
> >>>>
> >>>> regards, tom lane
> >>>>
> >>>>
> >>> Hi,
> >>> I was looking at the TODO list and found that the issue requires
> >>> a quick fix. Attached is a patch which shows output like this.
> >>
> >>
> >> Quick code review:
> >>
> >> + "actual rows=%.0f loops=%.0f": " rows=%.2f loops=%.0f",
> >>
> >> The leading space before the else block "rows" does not belong.
> >>
> >> There should be a space after the colon.
> >>
> > Thanks, David for your quick response. I have updated the patch.
> >
> >>
> >> The word "actual" that you are dropping in the else block seems like it
> should belong - it is a header for the entire section not just a modifier
> for the word "rows". This is evidenced by the timing block verbiage where
> rows is standalone and the word actual comes before time. In short, only
> the format specifier should change under the current scheme. Both sections.
> >>
> >> - WRITE_FLOAT_FIELD(rows, "%.0f");
> >> + WRITE_FLOAT_FIELD(rows, "%.2f");
> >>
> >> This one looks suspicious, though I haven't dug into the code to see
> exactly what all is being touched. That it doesn't have an nloops
> condition like everything else stands out.
> >>
> > I was also thinking about that, but I don't see any harm when we
> ultimately truncating that decimal
> > at a latter stage of code in case of loop = 1.
>
> Thanks for the patch.
>
Thanks for the review.
>
> 1) There are some existing regression tests that are failing, you
> should update the expect files accordingly for the same:
> --- /home/vignesh/postgres/src/test/regress/expected/select_parallel.out
> 2022-05-18 20:51:46.874818044 +0530
> +++ /home/vignesh/postgres/src/test/regress/results/select_parallel.out
> 2022-07-07 15:27:34.450440922 +0530
> @@ -545,17 +545,17 @@
> explain (analyze, timing off, summary off, costs off)
> select count(*) from tenk1, tenk2 where tenk1.hundred > 1
> and tenk2.thousand=0;
> - QUERY PLAN
> ---------------------------------------------------------------------------
> + QUERY PLAN
>
> +-----------------------------------------------------------------------------
> Aggregate (actual rows=1 loops=1)
> -> Nested Loop (actual rows=98000 loops=1)
> -> Seq Scan on tenk2 (actual rows=10 loops=1)
> Filter: (thousand = 0)
> Rows Removed by Filter: 9990
> - -> Gather (actual rows=9800 loops=10)
> + -> Gather (actual rows=9800.00 loops=10)
> Workers Planned: 4
> Workers Launched: 4
> - -> Parallel Seq Scan on tenk1 (actual rows=1960 loops=50)
> + -> Parallel Seq Scan on tenk1 (actual rows=1960.00
> loops=50)
> Filter: (hundred > 1)
>
> test select_parallel ... FAILED 744 ms
> partition_prune ... FAILED 861 ms
> explain ... FAILED 134 ms
> memoize ... FAILED 250 ms
>
> 2) This change is not required as part of this patch:
> --- a/src/backend/access/transam/xact.c
> +++ b/src/backend/access/transam/xact.c
> @@ -122,7 +122,7 @@ bool bsysscan = false;
> * lookups as fast as possible.
> */
> static FullTransactionId XactTopFullTransactionId =
> {InvalidTransactionId};
> -static int nParallelCurrentXids = 0;
> +static int nParallelCurrentXids = 0;
> static TransactionId *ParallelCurrentXids;
>
>
I have fixed the regression and removed non-related code.
> Regards,
> Vignesh
>
--
Ibrar Ahmed
^ permalink raw reply [nested|flat] 45+ messages in thread
* Re: explain analyze rows=%.0f
@ 2022-07-08 18:12 Ibrar Ahmed <[email protected]>
parent: Greg Stark <[email protected]>
1 sibling, 0 replies; 45+ messages in thread
From: Ibrar Ahmed @ 2022-07-08 18:12 UTC (permalink / raw)
To: Greg Stark <[email protected]>; +Cc: vignesh C <[email protected]>; David G. Johnston <[email protected]>; pgsql-hackers; Robert Haas <[email protected]>; Simon Riggs <[email protected]>; Ron Mayer <[email protected]>; Euler Taveira de Oliveira <[email protected]>; Tom Lane <[email protected]>
On Thu, Jul 7, 2022 at 10:53 PM Greg Stark <[email protected]> wrote:
> > - -> Parallel Seq Scan on tenk1 (actual rows=1960
> loops=50)
> > + -> Parallel Seq Scan on tenk1 (actual rows=1960.00
>
> At the not inconsiderable risk of bike-shedding....
>
> I'm wondering if printing something like 0.00 will be somewhat
> deceptive when the real value is non-zero but less than 1 row per 200
> loops. I wonder if the number of decimal places should be calculated
> to produce a minimum of one non-zero digit for non-zero values.
>
> --
> greg
>
+ -> Parallel Seq Scan on tenk1 (actual rows=1960.00
I have added a new check to remove any ".00" from the output because in
the case of parallel queries we are getting that. Secondly, it is
disturbing many test case outputs.
--
Ibrar Ahmed
^ permalink raw reply [nested|flat] 45+ messages in thread
* Re: explain analyze rows=%.0f
@ 2022-07-22 10:47 Amit Kapila <[email protected]>
parent: Justin Pryzby <[email protected]>
0 siblings, 2 replies; 45+ messages in thread
From: Amit Kapila @ 2022-07-22 10:47 UTC (permalink / raw)
To: Justin Pryzby <[email protected]>; +Cc: Peter Geoghegan <[email protected]>; Robert Haas <[email protected]>; Greg Stark <[email protected]>; vignesh C <[email protected]>; Ibrar Ahmed <[email protected]>; David G. Johnston <[email protected]>; pgsql-hackers; Simon Riggs <[email protected]>; Ron Mayer <[email protected]>; Euler Taveira de Oliveira <[email protected]>; Tom Lane <[email protected]>
On Fri, Jul 8, 2022 at 3:50 AM Justin Pryzby <[email protected]> wrote:
>
> On Thu, Jul 07, 2022 at 04:21:37PM -0400, Robert Haas wrote:
> > I mean, what I really want here if I'm honest is to not have the
> > system divide the number of rows by the loop count. And it sort of
> > sounds like maybe that's what you want, too. You want to know whether
> > the loop count is actually zero, not whether it's close to zero when
> > you divide it by some number that might be gigantic.
> ...
> > involves a dozen or two different nested loops, and if we didn't
> > insist on dividing the time by the loop count, it would be MUCH EASIER
> > to figure out whether the time spent in the Index Scan is a
> > significant percentage of the total time or not.
>
> I think the guiding princible for what to do should be to reduce how much is
> needed to explain about how to interpret what explain is showing...
>
> The docs say this:
> | In such cases, the loops value reports the total number of executions of the
> | node, and the actual time and rows values shown are averages per-execution.
> | This is done to make the numbers comparable with the way that the cost
> | estimates are shown. Multiply by the loops value to get the total time
> | actually spent in the node.
>
> On Thu, Jul 07, 2022 at 01:45:19PM -0700, Peter Geoghegan wrote:
> > Plus you could probably
> > make some kind of concession in the direction of maintaining
> > compatibility with the current approach if you had to. Right?
>
> The minimum would be to show the information in a way that makes it clear that
> it's "new style" output showing a total and not an average, so that a person
> who sees it knows how to interpret it (same for the web "explain tools")
>
> A concession would be to show the current information *plus* total/raw values.
>
> This thread is about how to display the existing values.
>
I feel the discussion has slightly deviated which makes it unclear
whether this patch is required or not?
--
With Regards,
Amit Kapila.
^ permalink raw reply [nested|flat] 45+ messages in thread
* Re: explain analyze rows=%.0f
@ 2022-07-22 12:07 Robert Haas <[email protected]>
parent: Amit Kapila <[email protected]>
1 sibling, 2 replies; 45+ messages in thread
From: Robert Haas @ 2022-07-22 12:07 UTC (permalink / raw)
To: Amit Kapila <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Peter Geoghegan <[email protected]>; Greg Stark <[email protected]>; vignesh C <[email protected]>; Ibrar Ahmed <[email protected]>; David G. Johnston <[email protected]>; pgsql-hackers; Simon Riggs <[email protected]>; Ron Mayer <[email protected]>; Euler Taveira de Oliveira <[email protected]>; Tom Lane <[email protected]>
On Fri, Jul 22, 2022 at 6:47 AM Amit Kapila <[email protected]> wrote:
> I feel the discussion has slightly deviated which makes it unclear
> whether this patch is required or not?
My opinion is that showing some fractional digits at least when
loops>1 would be better than what we have now. It might not be the
best thing we could do, but it would be better than doing nothing.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 45+ messages in thread
* Re: explain analyze rows=%.0f
@ 2022-07-22 20:01 Naeem Akhter <[email protected]>
parent: Robert Haas <[email protected]>
1 sibling, 1 reply; 45+ messages in thread
From: Naeem Akhter @ 2022-07-22 20:01 UTC (permalink / raw)
To: [email protected]; +Cc: Ibrar Ahmed <[email protected]>
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: not tested
Documentation: not tested
Verified patch 'explain_float_row_v3.patch' on master & REL_15_STABLE branches.
The new status of this patch is: Ready for Committer
^ permalink raw reply [nested|flat] 45+ messages in thread
* Re: explain analyze rows=%.0f
@ 2022-07-25 17:18 Hamid Akhtar <[email protected]>
parent: Naeem Akhter <[email protected]>
0 siblings, 1 reply; 45+ messages in thread
From: Hamid Akhtar @ 2022-07-25 17:18 UTC (permalink / raw)
To: [email protected]; +Cc: Ibrar Ahmed <[email protected]>
The following review has been posted through the commitfest application:
make installcheck-world: tested, failed
Implements feature: tested, failed
Spec compliant: not tested
Documentation: not tested
LGTM
^ permalink raw reply [nested|flat] 45+ messages in thread
* Re: explain analyze rows=%.0f
@ 2022-07-26 06:46 Hamid Akhtar <[email protected]>
parent: Hamid Akhtar <[email protected]>
0 siblings, 0 replies; 45+ messages in thread
From: Hamid Akhtar @ 2022-07-26 06:46 UTC (permalink / raw)
To: [email protected]; +Cc: Ibrar Ahmed <[email protected]>
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: not tested
Documentation: not tested
The previous review was incorrectly posted. Updating the pat.ch review
^ permalink raw reply [nested|flat] 45+ messages in thread
* Re: explain analyze rows=%.0f
@ 2022-11-06 05:12 Tom Lane <[email protected]>
parent: Robert Haas <[email protected]>
1 sibling, 0 replies; 45+ messages in thread
From: Tom Lane @ 2022-11-06 05:12 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Amit Kapila <[email protected]>; Justin Pryzby <[email protected]>; Peter Geoghegan <[email protected]>; Greg Stark <[email protected]>; vignesh C <[email protected]>; Ibrar Ahmed <[email protected]>; David G. Johnston <[email protected]>; [email protected]
Robert Haas <[email protected]> writes:
> On Fri, Jul 22, 2022 at 6:47 AM Amit Kapila <[email protected]> wr=
ote:
>> I feel the discussion has slightly deviated which makes it unclear
>> whether this patch is required or not?
> My opinion is that showing some fractional digits at least when
> loops>1 would be better than what we have now. It might not be the
> best thing we could do, but it would be better than doing nothing.
Yeah, I think that is a reasonable compromise.
I took a brief look through the patch, and I have some review
comments:
* Code like this is pretty awful:
appendStringInfo(es->str,
(nloops =3D=3D 1 || !HAS_DECIMAL(rows)) ?
" rows=3D%.0f loops=3D%.0f)" : " rows=3D%=
.2f loops=3D%.0f)",
rows, nloops);
Don't use variable format strings. They're hard to read and they
probably defeat compile-time checks that the arguments match the
format string. You could use a "*" field width instead, ie
appendStringInfo(es->str,
" rows=3D%.*f loops=3D%.0f)",
(nloops =3D=3D 1 || !HAS_DECIMAL(rows)) ?=
2 : 0,
rows, nloops);
That'd also allow you to reduce the code churn you've added by
splitting some appendStringInfo calls.
* I'm fairly concerned about how stable this'll be in the buildfarm,
in particular I fear HAS_DECIMAL() is not likely to give consistent
results across platforms. I gather that an earlier version of the patch
tried to check whether the fractional part would be zero to two decimal
places, rather than whether it's exactly zero. Probably want to put
back something like that.
* Another thought is that the non-text formats tend to prize output
consistency over readability, so maybe we should just always use 2
fractional digits there, rather than trying to minimize visible changes.
* We need some doc adjustments, surely, to explain what the heck this
means.
regards, tom lane
^ permalink raw reply [nested|flat] 45+ messages in thread
* Re: explain analyze rows=%.0f
@ 2022-11-07 04:20 Andrey Lepikhov <[email protected]>
parent: Amit Kapila <[email protected]>
1 sibling, 0 replies; 45+ messages in thread
From: Andrey Lepikhov @ 2022-11-07 04:20 UTC (permalink / raw)
To: Amit Kapila <[email protected]>; Justin Pryzby <[email protected]>; +Cc: Peter Geoghegan <[email protected]>; Robert Haas <[email protected]>; Greg Stark <[email protected]>; vignesh C <[email protected]>; Ibrar Ahmed <[email protected]>; David G. Johnston <[email protected]>; pgsql-hackers; Simon Riggs <[email protected]>; Ron Mayer <[email protected]>; Euler Taveira de Oliveira <[email protected]>; Tom Lane <[email protected]>
On 22/7/2022 16:47, Amit Kapila wrote:
> I feel the discussion has slightly deviated which makes it unclear
> whether this patch is required or not?
After quick review I want to express my thoughts.
At first, We have been waiting for this feature for years. Often clients
give an explain to us where we see something like:
"rows=0, loops=1000000".
Without verbose mode, I can't even understand whether this node produces
any rows or not.
So, I think this feature is useful for parameterized plans mostly.
Also, printing two decimal digits or even three isn't meaningful -
sometimes we have a plan where number of loops is about 1E6 or even 1E7,
but number of real rows is equal 100 or 1000.
To overcome this issue, I see two options:
1. Show the exact number of tuples without division by loops (fair case
but invasive and bad for automation tools).
2. Show rows in scientific format like X.XXEXX.
I vote for second option.
--
regards,
Andrey Lepikhov
Postgres Professional
^ permalink raw reply [nested|flat] 45+ messages in thread
* Re: explain analyze rows=%.0f
@ 2025-03-11 09:58 Ilia Evdokimov <[email protected]>
parent: Robert Haas <[email protected]>
2 siblings, 1 reply; 45+ messages in thread
From: Ilia Evdokimov @ 2025-03-11 09:58 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Matthias van de Meent <[email protected]>; Tom Lane <[email protected]>; Andrei Lepikhov <[email protected]>; [email protected]; Guillaume Lelarge <[email protected]>; Matheus Alcantara <[email protected]>; Alena Rybakina <[email protected]>
Hi,
In the stats_ext regression test, there is a function
check_estimated_rows that returns actual rows as an integer. Currently,
none of the test cases produce a non-zero fractional part in actual rows.
The question is: should this function be modified to return a fractional
number instead of an integer?
Personally, I don’t see much value in doing so, because the purpose of
the test is to compare the estimated row count with the actual number of
rows. It is also unlikely that there will be test cases where loops > 1,
and the presence of a fractional part does not change the essence of the
test.
What do you think?
--
Best regards,
Ilia Evdokimov,
Tantor Labs LLC.
^ permalink raw reply [nested|flat] 45+ messages in thread
* Re: explain analyze rows=%.0f
@ 2025-03-31 17:35 Robert Haas <[email protected]>
parent: Ilia Evdokimov <[email protected]>
0 siblings, 1 reply; 45+ messages in thread
From: Robert Haas @ 2025-03-31 17:35 UTC (permalink / raw)
To: Ilia Evdokimov <[email protected]>; +Cc: Matthias van de Meent <[email protected]>; Tom Lane <[email protected]>; Andrei Lepikhov <[email protected]>; [email protected]; Guillaume Lelarge <[email protected]>; Matheus Alcantara <[email protected]>; Alena Rybakina <[email protected]>
On Tue, Mar 11, 2025 at 5:58 AM Ilia Evdokimov
<[email protected]> wrote:
> In the stats_ext regression test, there is a function
> check_estimated_rows that returns actual rows as an integer. Currently,
> none of the test cases produce a non-zero fractional part in actual rows.
>
> The question is: should this function be modified to return a fractional
> number instead of an integer?
>
> Personally, I don’t see much value in doing so, because the purpose of
> the test is to compare the estimated row count with the actual number of
> rows. It is also unlikely that there will be test cases where loops > 1,
> and the presence of a fractional part does not change the essence of the
> test.
>
> What do you think?
I suppose the way this is currently coded, it will have the effect of
extracting the integer part of the row estimate. That doesn't really
seem like a bad idea to me, so I'm not sure it's worth trying to
adjust anything here.
One thing that I just noticed, though, is that we added two decimal
places to the actual row count, but not the estimated row count. So
now we get stuff like this:
-> Seq Scan on pgbench_branches b (cost=0.00..1.10 rows=10
width=364) (actual time=0.007..0.009 rows=10.00 loops=1)
But why isn't it just as valuable to have two decimal places for the
estimate? I theorize that the cases that are really a problem here are
those where the row count estimate is between 0 and 1 per row, and
rounding to an integer loses all precision.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 45+ messages in thread
* Re: explain analyze rows=%.0f
@ 2025-03-31 17:49 Tom Lane <[email protected]>
parent: Robert Haas <[email protected]>
0 siblings, 1 reply; 45+ messages in thread
From: Tom Lane @ 2025-03-31 17:49 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Ilia Evdokimov <[email protected]>; Matthias van de Meent <[email protected]>; Andrei Lepikhov <[email protected]>; [email protected]; Guillaume Lelarge <[email protected]>; Matheus Alcantara <[email protected]>; Alena Rybakina <[email protected]>
Robert Haas <[email protected]> writes:
> But why isn't it just as valuable to have two decimal places for the
> estimate? I theorize that the cases that are really a problem here are
> those where the row count estimate is between 0 and 1 per row, and
> rounding to an integer loses all precision.
Currently, the planner rounds *all* rowcount estimates to integers
(cf. clamp_row_est()). Maybe it'd be appropriate to rethink that,
but it's not just a matter of changing EXPLAIN's print format.
regards, tom lane
^ permalink raw reply [nested|flat] 45+ messages in thread
* Re: explain analyze rows=%.0f
@ 2025-03-31 19:09 Robert Haas <[email protected]>
parent: Tom Lane <[email protected]>
0 siblings, 1 reply; 45+ messages in thread
From: Robert Haas @ 2025-03-31 19:09 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Ilia Evdokimov <[email protected]>; Matthias van de Meent <[email protected]>; Andrei Lepikhov <[email protected]>; [email protected]; Guillaume Lelarge <[email protected]>; Matheus Alcantara <[email protected]>; Alena Rybakina <[email protected]>
On Mon, Mar 31, 2025 at 1:49 PM Tom Lane <[email protected]> wrote:
> Robert Haas <[email protected]> writes:
> > But why isn't it just as valuable to have two decimal places for the
> > estimate? I theorize that the cases that are really a problem here are
> > those where the row count estimate is between 0 and 1 per row, and
> > rounding to an integer loses all precision.
>
> Currently, the planner rounds *all* rowcount estimates to integers
> (cf. clamp_row_est()). Maybe it'd be appropriate to rethink that,
> but it's not just a matter of changing EXPLAIN's print format.
Oh, right. I've never really understood why we round off to integers,
but the fact that we don't allow row counts < 1 feels like something
pretty important. My intuition is that it probably helps a lot more
than it hurts, too.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 45+ messages in thread
* Re: explain analyze rows=%.0f
@ 2025-03-31 20:59 Ilia Evdokimov <[email protected]>
parent: Robert Haas <[email protected]>
0 siblings, 1 reply; 45+ messages in thread
From: Ilia Evdokimov @ 2025-03-31 20:59 UTC (permalink / raw)
To: Robert Haas <[email protected]>; Tom Lane <[email protected]>; +Cc: Matthias van de Meent <[email protected]>; Andrei Lepikhov <[email protected]>; [email protected]; Guillaume Lelarge <[email protected]>; Matheus Alcantara <[email protected]>; Alena Rybakina <[email protected]>
On 31.03.2025 22:09, Robert Haas wrote:
> Oh, right. I've never really understood why we round off to integers,
> but the fact that we don't allow row counts < 1 feels like something
> pretty important. My intuition is that it probably helps a lot more
> than it hurts, too.
We definitely shouldn’t remove the row counts < 1 check, since there are
many places in the planner where we divide by rows. This mechanism was
added specifically to prevent division by zero. Also, allowing rows
estimates below 1 can sometimes make the planner overly optimistic,
leading it to prefer cheaper-looking plans that may not perform well in
practice. For example, choosing a Nested Loop instead of a more
appropriate Hash Join.
Allowing fractional rows > 1 might help improve planner accuracy in some
cases, but this needs further study to fully understand the impact.
--
Best regards,
Ilia Evdokimov,
Tantor Labs LLC.
^ permalink raw reply [nested|flat] 45+ messages in thread
* Re: explain analyze rows=%.0f
@ 2025-04-09 19:40 Ilia Evdokimov <[email protected]>
parent: Ilia Evdokimov <[email protected]>
0 siblings, 0 replies; 45+ messages in thread
From: Ilia Evdokimov @ 2025-04-09 19:40 UTC (permalink / raw)
To: Robert Haas <[email protected]>; Tom Lane <[email protected]>; +Cc: Matthias van de Meent <[email protected]>; Andrei Lepikhov <[email protected]>; [email protected]; Guillaume Lelarge <[email protected]>; Matheus Alcantara <[email protected]>; Alena Rybakina <[email protected]>
On 31.03.2025 23:59, Ilia Evdokimov wrote:
>
> We definitely shouldn’t remove the row counts < 1 check, since there
> are many places in the planner where we divide by rows. This mechanism
> was added specifically to prevent division by zero. Also, allowing
> rows estimates below 1 can sometimes make the planner overly
> optimistic, leading it to prefer cheaper-looking plans that may not
> perform well in practice. For example, choosing a Nested Loop instead
> of a more appropriate Hash Join.
>
> Allowing fractional rows > 1 might help improve planner accuracy in
> some cases, but this needs further study to fully understand the impact.
>
I've been investigating whether it's worth removing rounding in row
estimates - and I believe it is.
[ v1-0001-Always-use-two-fractional-digits-for-estimated-rows_SRC.patch ]
Currently, we round most row estimates using rint() inside
clamp_row_est(). However, this function is also used for rounding tuples
and page counts. These should remain integral, but row estimates can and
should remain fractional for better precision. To address this, I
introduced a new function clamp_tuple_est() which retains the existing
rounding behavior (via rint()), while clamp_tuple_est() no longer
rounds. I use clamp_tuple_est() only for row estimates and
clamp_tuple_est() for tuples and pages elsewhere.
After removing rounding, two small issues needed fixing. First, there
was a check rows > 1 in the cost estimation for Nested Loop joins, which
is no longer reliable for values like 1.3. I updated it to rows >= 2 to
retain the original behavior. This can be refined further, but, in my
opinion, it's a practical compromise. Second, there is still a call to
rint() in cost of mergejoin which likely should be removed too - though
I haven’t included that here yet.
Also, if we're no longer rounding estimates, EXPLAIN should display them
with two decimal digits, just like it already does for actual rows.
[ v1-0002-Always-use-two-fractional-digits-for-estimated-rows_TESTS.patch ]
So, what does this change improve? Here are some of the observed plan
improvements:
* Previously, a Parallel Aggregate was chosen. With slightly more
accurate estimation, the planner switches to FinalizeAggregate,
which can be more efficient in distributed plans.
* In certain nested joins with constant subqueries, the planner
previously inserted an unnecessary Materialize. With improved
estimates, it now skips that step, reducing memory usage and latency.
* When the estimated number of iterations becomes non-integer but
still justifies caching, the planner adds Memoize instead of
re-running a function like generate_series(). This can speed up
execution significantly.
* In one case involving partitioned tables and filter conditions like
t1.b = 0, the planner now chooses an index-based nested loop join
instead of a hash join. This results in a more efficient plan with
fewer memory and CPU costs.
I know this patch still needs documentation updates to describe the new
estimation display and behaviors. But before that, I’d like to gather
feedback: does community agree that more precise estimates and
fractional values are better than always rounding?
If anyone would like to see the EXPLAIN ANALYZE VERBOSE output of
changes in regression tests, I’d be happy to share them.
All feedback and suggestions welcome!
--
Best regards,
Ilia Evdokimov,
Tantor Labs LLC.
Attachments:
[text/x-patch] v1-0001-Always-use-two-fractional-digits-for-estimated-rows_SRC.patch (17.1K, ../../[email protected]/3-v1-0001-Always-use-two-fractional-digits-for-estimated-rows_SRC.patch)
download | inline diff:
From a042ee22dbdae5049f4830116e83a3a72ea5e405 Mon Sep 17 00:00:00 2001
From: Evdokimov Ilia <[email protected]>
Date: Wed, 9 Apr 2025 22:24:01 +0300
Subject: [PATCH v1] Always use two fractional digits for estimated rows count
---
contrib/file_fdw/file_fdw.c | 4 +-
contrib/postgres_fdw/postgres_fdw.c | 4 +-
contrib/tsm_system_rows/tsm_system_rows.c | 4 +-
contrib/tsm_system_time/tsm_system_time.c | 4 +-
src/backend/access/table/tableam.c | 2 +-
src/backend/access/tablesample/bernoulli.c | 2 +-
src/backend/access/tablesample/system.c | 4 +-
src/backend/commands/explain.c | 4 +-
src/backend/optimizer/path/costsize.c | 53 +++++++++++++++-------
src/backend/optimizer/util/pathnode.c | 4 +-
src/backend/utils/adt/selfuncs.c | 14 +++---
src/include/optimizer/optimizer.h | 1 +
12 files changed, 61 insertions(+), 39 deletions(-)
diff --git a/contrib/file_fdw/file_fdw.c b/contrib/file_fdw/file_fdw.c
index a9a5671d95a..682bb86adde 100644
--- a/contrib/file_fdw/file_fdw.c
+++ b/contrib/file_fdw/file_fdw.c
@@ -1097,7 +1097,7 @@ estimate_size(PlannerInfo *root, RelOptInfo *baserel,
double density;
density = baserel->tuples / (double) baserel->pages;
- ntuples = clamp_row_est(density * (double) pages);
+ ntuples = clamp_tuple_est(density * (double) pages);
}
else
{
@@ -1113,7 +1113,7 @@ estimate_size(PlannerInfo *root, RelOptInfo *baserel,
tuple_width = MAXALIGN(baserel->reltarget->width) +
MAXALIGN(SizeofHeapTupleHeader);
- ntuples = clamp_row_est((double) stat_buf.st_size /
+ ntuples = clamp_tuple_est((double) stat_buf.st_size /
(double) tuple_width);
}
fdw_private->ntuples = ntuples;
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index a7e0cc9f323..5e1c7bb32e1 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -3278,7 +3278,7 @@ estimate_path_cost_size(PlannerInfo *root,
* Back into an estimate of the number of retrieved rows. Just in
* case this is nuts, clamp to at most nrows.
*/
- retrieved_rows = clamp_row_est(rows / fpinfo->local_conds_sel);
+ retrieved_rows = clamp_tuple_est(rows / fpinfo->local_conds_sel);
retrieved_rows = Min(retrieved_rows, nrows);
/*
@@ -3453,7 +3453,7 @@ estimate_path_cost_size(PlannerInfo *root,
* Back into an estimate of the number of retrieved rows. Just in
* case this is nuts, clamp to at most foreignrel->tuples.
*/
- retrieved_rows = clamp_row_est(rows / fpinfo->local_conds_sel);
+ retrieved_rows = clamp_tuple_est(rows / fpinfo->local_conds_sel);
retrieved_rows = Min(retrieved_rows, foreignrel->tuples);
/*
diff --git a/contrib/tsm_system_rows/tsm_system_rows.c b/contrib/tsm_system_rows/tsm_system_rows.c
index f401efa2131..308bb301da1 100644
--- a/contrib/tsm_system_rows/tsm_system_rows.c
+++ b/contrib/tsm_system_rows/tsm_system_rows.c
@@ -135,7 +135,7 @@ system_rows_samplescangetsamplesize(PlannerInfo *root,
/* Clamp to the estimated relation size */
if (ntuples > baserel->tuples)
ntuples = (int64) baserel->tuples;
- ntuples = clamp_row_est(ntuples);
+ ntuples = clamp_tuple_est(ntuples);
if (baserel->tuples > 0 && baserel->pages > 0)
{
@@ -151,7 +151,7 @@ system_rows_samplescangetsamplesize(PlannerInfo *root,
}
/* Clamp to sane value */
- npages = clamp_row_est(Min((double) baserel->pages, npages));
+ npages = clamp_tuple_est(Min((double) baserel->pages, npages));
*pages = npages;
*tuples = ntuples;
diff --git a/contrib/tsm_system_time/tsm_system_time.c b/contrib/tsm_system_time/tsm_system_time.c
index c9c71d8c3af..b2d1d7e37d7 100644
--- a/contrib/tsm_system_time/tsm_system_time.c
+++ b/contrib/tsm_system_time/tsm_system_time.c
@@ -151,7 +151,7 @@ system_time_samplescangetsamplesize(PlannerInfo *root,
npages = millis; /* even more bogus, but whatcha gonna do? */
/* Clamp to sane value */
- npages = clamp_row_est(Min((double) baserel->pages, npages));
+ npages = clamp_tuple_est(Min((double) baserel->pages, npages));
if (baserel->tuples > 0 && baserel->pages > 0)
{
@@ -167,7 +167,7 @@ system_time_samplescangetsamplesize(PlannerInfo *root,
}
/* Clamp to the estimated relation size */
- ntuples = clamp_row_est(Min(baserel->tuples, ntuples));
+ ntuples = clamp_tuple_est(Min(baserel->tuples, ntuples));
*pages = npages;
*tuples = ntuples;
diff --git a/src/backend/access/table/tableam.c b/src/backend/access/table/tableam.c
index a56c5eceb14..e14a315f319 100644
--- a/src/backend/access/table/tableam.c
+++ b/src/backend/access/table/tableam.c
@@ -742,7 +742,7 @@ table_block_relation_estimate_size(Relation rel, int32 *attr_widths,
/* note: integer division is intentional here */
density = (usable_bytes_per_page * fillfactor / 100) / tuple_width;
/* There's at least one row on the page, even with low fillfactor. */
- density = clamp_row_est(density);
+ density = clamp_tuple_est(density);
}
*tuples = rint(density * (double) curpages);
diff --git a/src/backend/access/tablesample/bernoulli.c b/src/backend/access/tablesample/bernoulli.c
index 5e1c5d2b723..d1993299158 100644
--- a/src/backend/access/tablesample/bernoulli.c
+++ b/src/backend/access/tablesample/bernoulli.c
@@ -117,7 +117,7 @@ bernoulli_samplescangetsamplesize(PlannerInfo *root,
/* We'll visit all pages of the baserel */
*pages = baserel->pages;
- *tuples = clamp_row_est(baserel->tuples * samplefract);
+ *tuples = clamp_tuple_est(baserel->tuples * samplefract);
}
/*
diff --git a/src/backend/access/tablesample/system.c b/src/backend/access/tablesample/system.c
index 8db813b89fc..8944fe28799 100644
--- a/src/backend/access/tablesample/system.c
+++ b/src/backend/access/tablesample/system.c
@@ -117,10 +117,10 @@ system_samplescangetsamplesize(PlannerInfo *root,
}
/* We'll visit a sample of the pages ... */
- *pages = clamp_row_est(baserel->pages * samplefract);
+ *pages = clamp_tuple_est(baserel->pages * samplefract);
/* ... and hopefully get a representative number of tuples from them */
- *tuples = clamp_row_est(baserel->tuples * samplefract);
+ *tuples = clamp_tuple_est(baserel->tuples * samplefract);
}
/*
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index ef8aa489af8..9d9da4880a3 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -1808,7 +1808,7 @@ ExplainNode(PlanState *planstate, List *ancestors,
{
if (es->format == EXPLAIN_FORMAT_TEXT)
{
- appendStringInfo(es->str, " (cost=%.2f..%.2f rows=%.0f width=%d)",
+ appendStringInfo(es->str, " (cost=%.2f..%.2f rows=%.2f width=%d)",
plan->startup_cost, plan->total_cost,
plan->plan_rows, plan->plan_width);
}
@@ -1819,7 +1819,7 @@ ExplainNode(PlanState *planstate, List *ancestors,
ExplainPropertyFloat("Total Cost", NULL, plan->total_cost,
2, es);
ExplainPropertyFloat("Plan Rows", NULL, plan->plan_rows,
- 0, es);
+ 2, es);
ExplainPropertyInteger("Plan Width", NULL, plan->plan_width,
es);
}
diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index 60b0fcfb6be..27f8d70eb2c 100644
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -205,6 +205,29 @@ static double page_size(double tuples, int width);
static double get_parallel_divisor(Path *path);
+/*
+ * clamp_tuple_est
+ * Force a tuple-count estimate to a sane value.
+ */
+double
+clamp_tuple_est(double ntuples)
+{
+ /*
+ * Avoid infinite and NaN tuple estimates. Costs derived from such values
+ * are going to be useless. Also force the estimate to be at least one
+ * tuple, to make explain output look better and to avoid possible
+ * divide-by-zero when interpolating costs. Make it an integer, too.
+ */
+ if (ntuples > MAXIMUM_ROWCOUNT || isnan(ntuples))
+ ntuples = MAXIMUM_ROWCOUNT;
+ else if (ntuples <= 1.0)
+ ntuples = 1.0;
+ else
+ ntuples = rint(ntuples);
+
+ return ntuples;
+}
+
/*
* clamp_row_est
* Force a row-count estimate to a sane value.
@@ -216,14 +239,12 @@ clamp_row_est(double nrows)
* Avoid infinite and NaN row estimates. Costs derived from such values
* are going to be useless. Also force the estimate to be at least one
* row, to make explain output look better and to avoid possible
- * divide-by-zero when interpolating costs. Make it an integer, too.
+ * divide-by-zero when interpolating costs.
*/
if (nrows > MAXIMUM_ROWCOUNT || isnan(nrows))
nrows = MAXIMUM_ROWCOUNT;
else if (nrows <= 1.0)
nrows = 1.0;
- else
- nrows = rint(nrows);
return nrows;
}
@@ -249,7 +270,7 @@ clamp_width_est(int64 tuple_width)
return (int32) MaxAllocSize;
/*
- * Unlike clamp_row_est, we just Assert that the value isn't negative,
+ * Unlike clamp_tuple_est, we just Assert that the value isn't negative,
* rather than masking such errors.
*/
Assert(tuple_width >= 0);
@@ -643,7 +664,7 @@ cost_index(IndexPath *path, PlannerInfo *root, double loop_count,
run_cost += indexTotalCost - indexStartupCost;
/* estimate number of main-table tuples fetched */
- tuples_fetched = clamp_row_est(indexSelectivity * baserel->tuples);
+ tuples_fetched = clamp_tuple_est(indexSelectivity * baserel->tuples);
/* fetch estimated page costs for tablespace containing table */
get_tablespace_page_costs(baserel->reltablespace,
@@ -901,7 +922,7 @@ extract_nonindex_conditions(List *qual_clauses, List *indexclauses)
* computed for us by make_one_rel.
*
* Caller is expected to have ensured that tuples_fetched is greater than zero
- * and rounded to integer (see clamp_row_est). The result will likewise be
+ * and rounded to integer (see clamp_tuple_est). The result will likewise be
* greater than zero and integral.
*/
double
@@ -3084,7 +3105,7 @@ get_windowclause_startup_tuples(PlannerInfo *root, WindowClause *wc,
* subnode.
*/
- return clamp_row_est(return_tuples);
+ return clamp_tuple_est(return_tuples);
}
/*
@@ -3298,7 +3319,7 @@ initial_cost_nestloop(PlannerInfo *root, JoinCostWorkspace *workspace,
*/
startup_cost += outer_path->startup_cost + inner_path->startup_cost;
run_cost += outer_path->total_cost - outer_path->startup_cost;
- if (outer_path_rows > 1)
+ if (outer_path_rows >= 2)
run_cost += (outer_path_rows - 1) * inner_rescan_start_cost;
inner_run_cost = inner_path->total_cost - inner_path->startup_cost;
@@ -3323,7 +3344,7 @@ initial_cost_nestloop(PlannerInfo *root, JoinCostWorkspace *workspace,
{
/* Normal case; we'll scan whole input rel for each outer row */
run_cost += inner_run_cost;
- if (outer_path_rows > 1)
+ if (outer_path_rows >= 2)
run_cost += (outer_path_rows - 1) * inner_rescan_run_cost;
}
@@ -3656,8 +3677,8 @@ initial_cost_mergejoin(PlannerInfo *root, JoinCostWorkspace *workspace,
* Convert selectivities to row counts. We force outer_rows and
* inner_rows to be at least 1, but the skip_rows estimates can be zero.
*/
- outer_skip_rows = rint(outer_path_rows * outerstartsel);
- inner_skip_rows = rint(inner_path_rows * innerstartsel);
+ outer_skip_rows = outer_path_rows * outerstartsel;
+ inner_skip_rows = inner_path_rows * innerstartsel;
outer_rows = clamp_row_est(outer_path_rows * outerendsel);
inner_rows = clamp_row_est(inner_path_rows * innerendsel);
@@ -4415,7 +4436,7 @@ final_cost_hashjoin(PlannerInfo *root, HashPath *path,
* that way, so it will be unable to drive the batch size below hash_mem
* when this is true.)
*/
- if (relation_byte_size(clamp_row_est(inner_path_rows * innermcvfreq),
+ if (relation_byte_size(clamp_tuple_est(inner_path_rows * innermcvfreq),
inner_path->pathtarget->width) > get_hash_memory_limit())
startup_cost += disable_cost;
@@ -4449,7 +4470,7 @@ final_cost_hashjoin(PlannerInfo *root, HashPath *path,
* to clamp inner_scan_frac to at most 1.0; but since match_count is
* at least 1, no such clamp is needed now.)
*/
- outer_matched_rows = rint(outer_path_rows * extra->semifactors.outer_match_frac);
+ outer_matched_rows = outer_path_rows * extra->semifactors.outer_match_frac;
inner_scan_frac = 2.0 / (extra->semifactors.match_count + 1.0);
startup_cost += hash_qual_cost.startup;
@@ -4573,7 +4594,7 @@ cost_subplan(PlannerInfo *root, SubPlan *subplan, Plan *plan)
if (subplan->subLinkType == EXISTS_SUBLINK)
{
/* we only need to fetch 1 tuple; clamp to avoid zero divide */
- sp_cost.per_tuple += plan_run_cost / clamp_row_est(plan->plan_rows);
+ sp_cost.per_tuple += plan_run_cost / clamp_tuple_est(plan->plan_rows);
}
else if (subplan->subLinkType == ALL_SUBLINK ||
subplan->subLinkType == ANY_SUBLINK)
@@ -6517,7 +6538,7 @@ compute_bitmap_pages(PlannerInfo *root, RelOptInfo *baserel,
/*
* Estimate number of main-table pages fetched.
*/
- tuples_fetched = clamp_row_est(indexSelectivity * baserel->tuples);
+ tuples_fetched = clamp_tuple_est(indexSelectivity * baserel->tuples);
T = (baserel->pages > 1) ? (double) baserel->pages : 1.0;
@@ -6583,7 +6604,7 @@ compute_bitmap_pages(PlannerInfo *root, RelOptInfo *baserel,
*/
if (lossy_pages > 0)
tuples_fetched =
- clamp_row_est(indexSelectivity *
+ clamp_tuple_est(indexSelectivity *
(exact_pages / heap_pages) * baserel->tuples +
(lossy_pages / heap_pages) * baserel->tuples);
}
diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index 93e73cb44db..0b9ec6b26fc 100644
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -1687,7 +1687,7 @@ create_memoize_path(PlannerInfo *root, RelOptInfo *rel, Path *subpath,
pathnode->param_exprs = param_exprs;
pathnode->singlerow = singlerow;
pathnode->binary_mode = binary_mode;
- pathnode->calls = clamp_row_est(calls);
+ pathnode->calls = clamp_tuple_est(calls);
/*
* For now we set est_entries to 0. cost_memoize_rescan() does all the
@@ -4049,7 +4049,7 @@ adjust_limit_rows_costs(double *rows, /* in/out parameter */
if (offset_est > 0)
offset_rows = (double) offset_est;
else
- offset_rows = clamp_row_est(input_rows * 0.10);
+ offset_rows = clamp_tuple_est(input_rows * 0.10);
if (offset_rows > *rows)
offset_rows = *rows;
if (input_rows > 0)
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index 588d991fa57..0c077c09175 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -2183,7 +2183,7 @@ estimate_array_length(PlannerInfo *root, Node *arrayexpr)
ATTSTATSSLOT_NUMBERS))
{
if (sslot.nnumbers > 0)
- nelem = clamp_row_est(sslot.numbers[sslot.nnumbers - 1]);
+ nelem = clamp_tuple_est(sslot.numbers[sslot.nnumbers - 1]);
free_attstatsslot(&sslot);
}
}
@@ -3462,7 +3462,7 @@ estimate_num_groups(PlannerInfo *root, List *groupExprs, double input_rows,
* estimate is usually already at least 1, but clamp it just in case it
* isn't.
*/
- input_rows = clamp_row_est(input_rows);
+ input_rows = clamp_tuple_est(input_rows);
/*
* If no grouping columns, there's exactly one group. (This can't happen
@@ -3755,7 +3755,7 @@ estimate_num_groups(PlannerInfo *root, List *groupExprs, double input_rows,
(1 - pow((rel->tuples - rel->rows) / rel->tuples,
rel->tuples / reldistinct));
}
- reldistinct = clamp_row_est(reldistinct);
+ reldistinct = clamp_tuple_est(reldistinct);
/*
* Update estimate of total distinct groups.
@@ -4071,7 +4071,7 @@ estimate_hash_bucket_stats(PlannerInfo *root, Node *hashkey, double nbuckets,
if (vardata.rel && vardata.rel->tuples > 0)
{
ndistinct *= vardata.rel->rows / vardata.rel->tuples;
- ndistinct = clamp_row_est(ndistinct);
+ ndistinct = clamp_tuple_est(ndistinct);
}
/*
@@ -6168,7 +6168,7 @@ get_variable_numdistinct(VariableStatData *vardata, bool *isdefault)
* If we had an absolute estimate, use that.
*/
if (stadistinct > 0.0)
- return clamp_row_est(stadistinct);
+ return clamp_tuple_est(stadistinct);
/*
* Otherwise we need to get the relation size; punt if not available.
@@ -6189,7 +6189,7 @@ get_variable_numdistinct(VariableStatData *vardata, bool *isdefault)
* If we had a relative estimate, use that.
*/
if (stadistinct < 0.0)
- return clamp_row_est(-stadistinct * ntuples);
+ return clamp_tuple_est(-stadistinct * ntuples);
/*
* With no data, estimate ndistinct = ntuples if the table is small, else
@@ -6197,7 +6197,7 @@ get_variable_numdistinct(VariableStatData *vardata, bool *isdefault)
* that the behavior isn't discontinuous.
*/
if (ntuples < DEFAULT_NUM_DISTINCT)
- return clamp_row_est(ntuples);
+ return clamp_tuple_est(ntuples);
*isdefault = true;
return DEFAULT_NUM_DISTINCT;
diff --git a/src/include/optimizer/optimizer.h b/src/include/optimizer/optimizer.h
index 546828b54bd..e1be9dc6175 100644
--- a/src/include/optimizer/optimizer.h
+++ b/src/include/optimizer/optimizer.h
@@ -90,6 +90,7 @@ extern PGDLLIMPORT double recursive_worktable_factor;
extern PGDLLIMPORT int effective_cache_size;
extern double clamp_row_est(double nrows);
+extern double clamp_tuple_est(double ntuples);
extern int32 clamp_width_est(int64 tuple_width);
extern long clamp_cardinality_to_long(Cardinality x);
--
2.34.1
[text/x-patch] v1-0002-Always-use-two-fractional-digits-for-estimated-rows_TESTS.patch (61.1K, ../../[email protected]/4-v1-0002-Always-use-two-fractional-digits-for-estimated-rows_TESTS.patch)
download | inline diff:
From a34fd63c47a87e79d6b0fe0a705c527a5b2b22bc Mon Sep 17 00:00:00 2001
From: Evdokimov Ilia <[email protected]>
Date: Wed, 9 Apr 2025 22:26:16 +0300
Subject: [PATCH v1] Always use two fractional digits for estimated rows count
---
.../expected/pg_overexplain.out | 12 +-
.../postgres_fdw/expected/postgres_fdw.out | 15 +-
src/test/regress/expected/explain.out | 146 +++++++++---------
src/test/regress/expected/inherit.out | 8 +-
src/test/regress/expected/join.out | 134 ++++++++--------
src/test/regress/expected/join_hash.out | 19 +--
src/test/regress/expected/misc_functions.out | 84 +++++-----
src/test/regress/expected/partition_join.out | 112 +++++++-------
src/test/regress/expected/select_views.out | 19 ++-
src/test/regress/expected/stats_ext.out | 12 +-
src/test/regress/sql/stats_ext.sql | 4 +-
11 files changed, 277 insertions(+), 288 deletions(-)
diff --git a/contrib/pg_overexplain/expected/pg_overexplain.out b/contrib/pg_overexplain/expected/pg_overexplain.out
index 28252dbff6c..731a0bf79a2 100644
--- a/contrib/pg_overexplain/expected/pg_overexplain.out
+++ b/contrib/pg_overexplain/expected/pg_overexplain.out
@@ -25,9 +25,9 @@ ERROR: unrecognized EXPLAIN option "debuff"
LINE 1: EXPLAIN (DEBUFF) SELECT 1;
^
EXPLAIN (DEBUG) SELECT 1;
- QUERY PLAN
-------------------------------------------
- Result (cost=0.00..0.01 rows=1 width=4)
+ QUERY PLAN
+---------------------------------------------
+ Result (cost=0.00..0.01 rows=1.00 width=4)
Disabled Nodes: 0
Parallel Safe: false
Plan Node ID: 0
@@ -41,9 +41,9 @@ EXPLAIN (DEBUG) SELECT 1;
(11 rows)
EXPLAIN (RANGE_TABLE) SELECT 1;
- QUERY PLAN
-------------------------------------------
- Result (cost=0.00..0.01 rows=1 width=4)
+ QUERY PLAN
+---------------------------------------------
+ Result (cost=0.00..0.01 rows=1.00 width=4)
RTI 1 (result):
Eref: "*RESULT*" ()
(3 rows)
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index d1acee5a5fa..e5b92d2017b 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -10151,13 +10151,16 @@ SELECT t1.a,t2.b,t3.c FROM fprt1 t1 INNER JOIN fprt2 t2 ON (t1.a = t2.b) INNER J
-- left outer join + nullable clause
EXPLAIN (VERBOSE, COSTS OFF)
SELECT t1.a,t2.b,t2.c FROM fprt1 t1 LEFT JOIN (SELECT * FROM fprt2 WHERE a < 10) t2 ON (t1.a = t2.b and t1.b = t2.a) WHERE t1.a < 10 ORDER BY 1,2,3;
- QUERY PLAN
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
- Foreign Scan
+ QUERY PLAN
+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ Sort
Output: t1.a, fprt2.b, fprt2.c
- Relations: (public.ftprt1_p1 t1) LEFT JOIN (public.ftprt2_p1 fprt2)
- Remote SQL: SELECT r5.a, r6.b, r6.c FROM (public.fprt1_p1 r5 LEFT JOIN public.fprt2_p1 r6 ON (((r5.a = r6.b)) AND ((r5.b = r6.a)) AND ((r6.a < 10)))) WHERE ((r5.a < 10)) ORDER BY r5.a ASC NULLS LAST, r6.b ASC NULLS LAST, r6.c ASC NULLS LAST
-(4 rows)
+ Sort Key: t1.a, fprt2.b, fprt2.c
+ -> Foreign Scan
+ Output: t1.a, fprt2.b, fprt2.c
+ Relations: (public.ftprt1_p1 t1) LEFT JOIN (public.ftprt2_p1 fprt2)
+ Remote SQL: SELECT r5.a, r6.b, r6.c FROM (public.fprt1_p1 r5 LEFT JOIN public.fprt2_p1 r6 ON (((r5.a = r6.b)) AND ((r5.b = r6.a)) AND ((r6.a < 10)))) WHERE ((r5.a < 10))
+(7 rows)
SELECT t1.a,t2.b,t2.c FROM fprt1 t1 LEFT JOIN (SELECT * FROM fprt2 WHERE a < 10) t2 ON (t1.a = t2.b and t1.b = t2.a) WHERE t1.a < 10 ORDER BY 1,2,3;
a | b | c
diff --git a/src/test/regress/expected/explain.out b/src/test/regress/expected/explain.out
index c53bf9c8aa3..28991ccb665 100644
--- a/src/test/regress/expected/explain.out
+++ b/src/test/regress/expected/explain.out
@@ -55,32 +55,32 @@ set jit = off;
set track_io_timing = off;
-- Simple cases
select explain_filter('explain select * from int8_tbl i8');
- explain_filter
----------------------------------------------------------
- Seq Scan on int8_tbl i8 (cost=N.N..N.N rows=N width=N)
+ explain_filter
+-----------------------------------------------------------
+ Seq Scan on int8_tbl i8 (cost=N.N..N.N rows=N.N width=N)
(1 row)
select explain_filter('explain (analyze, buffers off) select * from int8_tbl i8');
- explain_filter
--------------------------------------------------------------------------------------------------
- Seq Scan on int8_tbl i8 (cost=N.N..N.N rows=N width=N) (actual time=N.N..N.N rows=N.N loops=N)
+ explain_filter
+---------------------------------------------------------------------------------------------------
+ Seq Scan on int8_tbl i8 (cost=N.N..N.N rows=N.N width=N) (actual time=N.N..N.N rows=N.N loops=N)
Planning Time: N.N ms
Execution Time: N.N ms
(3 rows)
select explain_filter('explain (analyze, buffers off, verbose) select * from int8_tbl i8');
- explain_filter
---------------------------------------------------------------------------------------------------------
- Seq Scan on public.int8_tbl i8 (cost=N.N..N.N rows=N width=N) (actual time=N.N..N.N rows=N.N loops=N)
+ explain_filter
+----------------------------------------------------------------------------------------------------------
+ Seq Scan on public.int8_tbl i8 (cost=N.N..N.N rows=N.N width=N) (actual time=N.N..N.N rows=N.N loops=N)
Output: q1, q2
Planning Time: N.N ms
Execution Time: N.N ms
(4 rows)
select explain_filter('explain (analyze, buffers, format text) select * from int8_tbl i8');
- explain_filter
--------------------------------------------------------------------------------------------------
- Seq Scan on int8_tbl i8 (cost=N.N..N.N rows=N width=N) (actual time=N.N..N.N rows=N.N loops=N)
+ explain_filter
+---------------------------------------------------------------------------------------------------
+ Seq Scan on int8_tbl i8 (cost=N.N..N.N rows=N.N width=N) (actual time=N.N..N.N rows=N.N loops=N)
Planning Time: N.N ms
Execution Time: N.N ms
(3 rows)
@@ -98,7 +98,7 @@ select explain_filter('explain (analyze, buffers, format xml) select * from int8
<Alias>i8</Alias> +
<Startup-Cost>N.N</Startup-Cost> +
<Total-Cost>N.N</Total-Cost> +
- <Plan-Rows>N</Plan-Rows> +
+ <Plan-Rows>N.N</Plan-Rows> +
<Plan-Width>N</Plan-Width> +
<Actual-Startup-Time>N.N</Actual-Startup-Time> +
<Actual-Total-Time>N.N</Actual-Total-Time> +
@@ -147,7 +147,7 @@ select explain_filter('explain (analyze, serialize, buffers, format yaml) select
Alias: "i8" +
Startup Cost: N.N +
Total Cost: N.N +
- Plan Rows: N +
+ Plan Rows: N.N +
Plan Width: N +
Actual Startup Time: N.N +
Actual Total Time: N.N +
@@ -195,9 +195,9 @@ select explain_filter('explain (analyze, serialize, buffers, format yaml) select
(1 row)
select explain_filter('explain (buffers, format text) select * from int8_tbl i8');
- explain_filter
----------------------------------------------------------
- Seq Scan on int8_tbl i8 (cost=N.N..N.N rows=N width=N)
+ explain_filter
+-----------------------------------------------------------
+ Seq Scan on int8_tbl i8 (cost=N.N..N.N rows=N.N width=N)
(1 row)
select explain_filter('explain (buffers, format json) select * from int8_tbl i8');
@@ -213,7 +213,7 @@ select explain_filter('explain (buffers, format json) select * from int8_tbl i8'
"Alias": "i8", +
"Startup Cost": N.N, +
"Total Cost": N.N, +
- "Plan Rows": N, +
+ "Plan Rows": N.N, +
"Plan Width": N, +
"Disabled": false, +
"Shared Hit Blocks": N, +
@@ -247,35 +247,35 @@ select explain_filter('explain (buffers, format json) select * from int8_tbl i8'
select explain_filter('explain verbose select sum(unique1) over w, sum(unique2) over (w order by hundred), sum(tenthous) over (w order by hundred) from tenk1 window w as (partition by ten)');
explain_filter
-------------------------------------------------------------------------------------------------------
- WindowAgg (cost=N.N..N.N rows=N width=N)
+ WindowAgg (cost=N.N..N.N rows=N.N width=N)
Output: sum(unique1) OVER w, (sum(unique2) OVER w1), (sum(tenthous) OVER w1), ten, hundred
Window: w AS (PARTITION BY tenk1.ten)
- -> WindowAgg (cost=N.N..N.N rows=N width=N)
+ -> WindowAgg (cost=N.N..N.N rows=N.N width=N)
Output: ten, hundred, unique1, unique2, tenthous, sum(unique2) OVER w1, sum(tenthous) OVER w1
Window: w1 AS (PARTITION BY tenk1.ten ORDER BY tenk1.hundred)
- -> Sort (cost=N.N..N.N rows=N width=N)
+ -> Sort (cost=N.N..N.N rows=N.N width=N)
Output: ten, hundred, unique1, unique2, tenthous
Sort Key: tenk1.ten, tenk1.hundred
- -> Seq Scan on public.tenk1 (cost=N.N..N.N rows=N width=N)
+ -> Seq Scan on public.tenk1 (cost=N.N..N.N rows=N.N width=N)
Output: ten, hundred, unique1, unique2, tenthous
(11 rows)
select explain_filter('explain verbose select sum(unique1) over w1, sum(unique2) over (w1 order by hundred), sum(tenthous) over (w1 order by hundred rows 10 preceding) from tenk1 window w1 as (partition by ten)');
explain_filter
---------------------------------------------------------------------------------------------------------
- WindowAgg (cost=N.N..N.N rows=N width=N)
+ WindowAgg (cost=N.N..N.N rows=N.N width=N)
Output: sum(unique1) OVER w1, (sum(unique2) OVER w2), (sum(tenthous) OVER w3), ten, hundred
Window: w1 AS (PARTITION BY tenk1.ten)
- -> WindowAgg (cost=N.N..N.N rows=N width=N)
+ -> WindowAgg (cost=N.N..N.N rows=N.N width=N)
Output: ten, hundred, unique1, unique2, tenthous, (sum(unique2) OVER w2), sum(tenthous) OVER w3
Window: w3 AS (PARTITION BY tenk1.ten ORDER BY tenk1.hundred ROWS 'N'::bigint PRECEDING)
- -> WindowAgg (cost=N.N..N.N rows=N width=N)
+ -> WindowAgg (cost=N.N..N.N rows=N.N width=N)
Output: ten, hundred, unique1, unique2, tenthous, sum(unique2) OVER w2
Window: w2 AS (PARTITION BY tenk1.ten ORDER BY tenk1.hundred)
- -> Sort (cost=N.N..N.N rows=N width=N)
+ -> Sort (cost=N.N..N.N rows=N.N width=N)
Output: ten, hundred, unique1, unique2, tenthous
Sort Key: tenk1.ten, tenk1.hundred
- -> Seq Scan on public.tenk1 (cost=N.N..N.N rows=N width=N)
+ -> Seq Scan on public.tenk1 (cost=N.N..N.N rows=N.N width=N)
Output: ten, hundred, unique1, unique2, tenthous
(14 rows)
@@ -295,7 +295,7 @@ select explain_filter('explain (analyze, buffers, format json) select * from int
"Alias": "i8", +
"Startup Cost": N.N, +
"Total Cost": N.N, +
- "Plan Rows": N, +
+ "Plan Rows": N.N, +
"Plan Width": N, +
"Actual Startup Time": N.N, +
"Actual Total Time": N.N, +
@@ -368,11 +368,11 @@ select explain_filter_to_json('explain (settings, format json) select * from int
rollback;
-- GENERIC_PLAN option
select explain_filter('explain (generic_plan) select unique1 from tenk1 where thousand = $1');
- explain_filter
----------------------------------------------------------------------------------
- Bitmap Heap Scan on tenk1 (cost=N.N..N.N rows=N width=N)
+ explain_filter
+-----------------------------------------------------------------------------------
+ Bitmap Heap Scan on tenk1 (cost=N.N..N.N rows=N.N width=N)
Recheck Cond: (thousand = $N)
- -> Bitmap Index Scan on tenk1_thous_tenthous (cost=N.N..N.N rows=N width=N)
+ -> Bitmap Index Scan on tenk1_thous_tenthous (cost=N.N..N.N rows=N.N width=N)
Index Cond: (thousand = $N)
(4 rows)
@@ -382,16 +382,16 @@ ERROR: EXPLAIN options ANALYZE and GENERIC_PLAN cannot be used together
CONTEXT: PL/pgSQL function explain_filter(text) line 5 at FOR over EXECUTE statement
-- MEMORY option
select explain_filter('explain (memory) select * from int8_tbl i8');
- explain_filter
----------------------------------------------------------
- Seq Scan on int8_tbl i8 (cost=N.N..N.N rows=N width=N)
+ explain_filter
+-----------------------------------------------------------
+ Seq Scan on int8_tbl i8 (cost=N.N..N.N rows=N.N width=N)
Memory: used=NkB allocated=NkB
(2 rows)
select explain_filter('explain (memory, analyze, buffers off) select * from int8_tbl i8');
- explain_filter
--------------------------------------------------------------------------------------------------
- Seq Scan on int8_tbl i8 (cost=N.N..N.N rows=N width=N) (actual time=N.N..N.N rows=N.N loops=N)
+ explain_filter
+---------------------------------------------------------------------------------------------------
+ Seq Scan on int8_tbl i8 (cost=N.N..N.N rows=N.N width=N) (actual time=N.N..N.N rows=N.N loops=N)
Memory: used=NkB allocated=NkB
Planning Time: N.N ms
Execution Time: N.N ms
@@ -408,7 +408,7 @@ select explain_filter('explain (memory, summary, format yaml) select * from int8
Alias: "i8" +
Startup Cost: N.N +
Total Cost: N.N +
- Plan Rows: N +
+ Plan Rows: N.N +
Plan Width: N +
Disabled: false +
Planning: +
@@ -430,7 +430,7 @@ select explain_filter('explain (memory, analyze, format json) select * from int8
"Alias": "i8", +
"Startup Cost": N.N, +
"Total Cost": N.N, +
- "Plan Rows": N, +
+ "Plan Rows": N.N, +
"Plan Width": N, +
"Actual Startup Time": N.N, +
"Actual Total Time": N.N, +
@@ -472,9 +472,9 @@ select explain_filter('explain (memory, analyze, format json) select * from int8
prepare int8_query as select * from int8_tbl i8;
select explain_filter('explain (memory) execute int8_query');
- explain_filter
----------------------------------------------------------
- Seq Scan on int8_tbl i8 (cost=N.N..N.N rows=N width=N)
+ explain_filter
+-----------------------------------------------------------
+ Seq Scan on int8_tbl i8 (cost=N.N..N.N rows=N.N width=N)
Memory: used=NkB allocated=NkB
(2 rows)
@@ -496,12 +496,12 @@ create table gen_part_2
partition of gen_part for values in (2);
-- should scan gen_part_1_1 and gen_part_1_2, but not gen_part_2
select explain_filter('explain (generic_plan) select key1, key2 from gen_part where key1 = 1 and key2 = $1');
- explain_filter
----------------------------------------------------------------------------
- Append (cost=N.N..N.N rows=N width=N)
- -> Seq Scan on gen_part_1_1 gen_part_1 (cost=N.N..N.N rows=N width=N)
+ explain_filter
+-----------------------------------------------------------------------------
+ Append (cost=N.N..N.N rows=N.N width=N)
+ -> Seq Scan on gen_part_1_1 gen_part_1 (cost=N.N..N.N rows=N.N width=N)
Filter: ((key1 = N) AND (key2 = $N))
- -> Seq Scan on gen_part_1_2 gen_part_2 (cost=N.N..N.N rows=N width=N)
+ -> Seq Scan on gen_part_1_2 gen_part_2 (cost=N.N..N.N rows=N.N width=N)
Filter: ((key1 = N) AND (key2 = $N))
(5 rows)
@@ -561,7 +561,7 @@ select jsonb_pretty(
"Schema": "public", +
"Disabled": false, +
"Node Type": "Seq Scan", +
- "Plan Rows": 0, +
+ "Plan Rows": 0.0, +
"Plan Width": 0, +
"Total Cost": 0.0, +
"Actual Rows": 0.0, +
@@ -608,7 +608,7 @@ select jsonb_pretty(
"tenk1.tenthous" +
], +
"Node Type": "Sort", +
- "Plan Rows": 0, +
+ "Plan Rows": 0.0, +
"Plan Width": 0, +
"Total Cost": 0.0, +
"Actual Rows": 0.0, +
@@ -652,7 +652,7 @@ select jsonb_pretty(
], +
"Disabled": false, +
"Node Type": "Gather Merge", +
- "Plan Rows": 0, +
+ "Plan Rows": 0.0, +
"Plan Width": 0, +
"Total Cost": 0.0, +
"Actual Rows": 0.0, +
@@ -703,7 +703,7 @@ as 'begin return sin($1); end';
select explain_filter('explain (verbose) select * from t1 where pg_temp.mysin(f1) < 0.5');
explain_filter
------------------------------------------------------------
- Seq Scan on pg_temp.t1 (cost=N.N..N.N rows=N width=N)
+ Seq Scan on pg_temp.t1 (cost=N.N..N.N rows=N.N width=N)
Output: f1
Filter: (pg_temp.mysin(t1.f1) < 'N.N'::double precision)
(3 rows)
@@ -711,53 +711,53 @@ select explain_filter('explain (verbose) select * from t1 where pg_temp.mysin(f1
-- Test compute_query_id
set compute_query_id = on;
select explain_filter('explain (verbose) select * from int8_tbl i8');
- explain_filter
-----------------------------------------------------------------
- Seq Scan on public.int8_tbl i8 (cost=N.N..N.N rows=N width=N)
+ explain_filter
+------------------------------------------------------------------
+ Seq Scan on public.int8_tbl i8 (cost=N.N..N.N rows=N.N width=N)
Output: q1, q2
Query Identifier: N
(3 rows)
-- Test compute_query_id with utility statements containing plannable query
select explain_filter('explain (verbose) declare test_cur cursor for select * from int8_tbl');
- explain_filter
--------------------------------------------------------------
- Seq Scan on public.int8_tbl (cost=N.N..N.N rows=N width=N)
+ explain_filter
+---------------------------------------------------------------
+ Seq Scan on public.int8_tbl (cost=N.N..N.N rows=N.N width=N)
Output: q1, q2
Query Identifier: N
(3 rows)
select explain_filter('explain (verbose) create table test_ctas as select 1');
- explain_filter
-----------------------------------------
- Result (cost=N.N..N.N rows=N width=N)
+ explain_filter
+------------------------------------------
+ Result (cost=N.N..N.N rows=N.N width=N)
Output: N
Query Identifier: N
(3 rows)
-- Test SERIALIZE option
select explain_filter('explain (analyze,buffers off,serialize) select * from int8_tbl i8');
- explain_filter
--------------------------------------------------------------------------------------------------
- Seq Scan on int8_tbl i8 (cost=N.N..N.N rows=N width=N) (actual time=N.N..N.N rows=N.N loops=N)
+ explain_filter
+---------------------------------------------------------------------------------------------------
+ Seq Scan on int8_tbl i8 (cost=N.N..N.N rows=N.N width=N) (actual time=N.N..N.N rows=N.N loops=N)
Planning Time: N.N ms
Serialization: time=N.N ms output=NkB format=text
Execution Time: N.N ms
(4 rows)
select explain_filter('explain (analyze,serialize text,buffers,timing off) select * from int8_tbl i8');
- explain_filter
------------------------------------------------------------------------------------
- Seq Scan on int8_tbl i8 (cost=N.N..N.N rows=N width=N) (actual rows=N.N loops=N)
+ explain_filter
+-------------------------------------------------------------------------------------
+ Seq Scan on int8_tbl i8 (cost=N.N..N.N rows=N.N width=N) (actual rows=N.N loops=N)
Planning Time: N.N ms
Serialization: output=NkB format=text
Execution Time: N.N ms
(4 rows)
select explain_filter('explain (analyze,serialize binary,buffers,timing) select * from int8_tbl i8');
- explain_filter
--------------------------------------------------------------------------------------------------
- Seq Scan on int8_tbl i8 (cost=N.N..N.N rows=N width=N) (actual time=N.N..N.N rows=N.N loops=N)
+ explain_filter
+---------------------------------------------------------------------------------------------------
+ Seq Scan on int8_tbl i8 (cost=N.N..N.N rows=N.N width=N) (actual time=N.N..N.N rows=N.N loops=N)
Planning Time: N.N ms
Serialization: time=N.N ms output=NkB format=binary
Execution Time: N.N ms
@@ -765,9 +765,9 @@ select explain_filter('explain (analyze,serialize binary,buffers,timing) select
-- this tests an edge case where we have no data to return
select explain_filter('explain (analyze,buffers off,serialize) create temp table explain_temp as select * from int8_tbl i8');
- explain_filter
--------------------------------------------------------------------------------------------------
- Seq Scan on int8_tbl i8 (cost=N.N..N.N rows=N width=N) (actual time=N.N..N.N rows=N.N loops=N)
+ explain_filter
+---------------------------------------------------------------------------------------------------
+ Seq Scan on int8_tbl i8 (cost=N.N..N.N rows=N.N width=N) (actual time=N.N..N.N rows=N.N loops=N)
Planning Time: N.N ms
Serialization: time=N.N ms output=NkB format=text
Execution Time: N.N ms
diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index 2a8bfba768e..a6657496d58 100644
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -695,10 +695,10 @@ select tableoid::regclass::text as relname, parted_tab.* from parted_tab order b
-- modifies partition key, but no rows will actually be updated
explain update parted_tab set a = 2 where false;
- QUERY PLAN
---------------------------------------------------------
- Update on parted_tab (cost=0.00..0.00 rows=0 width=0)
- -> Result (cost=0.00..0.00 rows=0 width=10)
+ QUERY PLAN
+-----------------------------------------------------------
+ Update on parted_tab (cost=0.00..0.00 rows=0.00 width=0)
+ -> Result (cost=0.00..0.00 rows=0.00 width=10)
One-Time Filter: false
(3 rows)
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 14da5708451..25ae9f4e714 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -2422,8 +2422,8 @@ from int4_tbl t1, int4_tbl t2
left join int4_tbl t3 on t3.f1 > 0
left join int4_tbl t4 on t3.f1 > 1
where t4.f1 is null;
- QUERY PLAN
--------------------------------------------------------
+ QUERY PLAN
+-------------------------------------------------
Nested Loop
-> Nested Loop Left Join
Filter: (t4.f1 IS NULL)
@@ -2433,10 +2433,9 @@ where t4.f1 is null;
Join Filter: (t3.f1 > 1)
-> Seq Scan on int4_tbl t3
Filter: (f1 > 0)
- -> Materialize
- -> Seq Scan on int4_tbl t4
+ -> Seq Scan on int4_tbl t4
-> Seq Scan on int4_tbl t1
-(12 rows)
+(11 rows)
select t1.f1
from int4_tbl t1, int4_tbl t2
@@ -2573,11 +2572,10 @@ where t1.f1 = coalesce(t2.f1, 1);
-> Materialize
-> Seq Scan on int4_tbl t2
Filter: (f1 > 1)
- -> Materialize
- -> Seq Scan on int4_tbl t3
+ -> Seq Scan on int4_tbl t3
-> Materialize
-> Seq Scan on int4_tbl t4
-(14 rows)
+(13 rows)
explain (costs off)
select * from int4_tbl t1
@@ -3740,10 +3738,9 @@ where i41.f1 > 0;
-> Seq Scan on int8_tbl i81
-> Materialize
-> Seq Scan on int4_tbl i42
- -> Materialize
- -> Seq Scan on int4_tbl i43
- Filter: (f1 > 1)
-(12 rows)
+ -> Seq Scan on int4_tbl i43
+ Filter: (f1 > 1)
+(11 rows)
select * from
int4_tbl as i41,
@@ -4332,6 +4329,13 @@ select * from tenk1 a join tenk1 b on
-------------------------------------------------------------------------------------------------
Nested Loop
Join Filter: (((a.unique1 = 1) AND (b.unique1 = 2)) OR ((a.unique2 = 3) AND (b.hundred = 4)))
+ -> Bitmap Heap Scan on tenk1 a
+ Recheck Cond: ((unique1 = 1) OR (unique2 = 3))
+ -> BitmapOr
+ -> Bitmap Index Scan on tenk1_unique1
+ Index Cond: (unique1 = 1)
+ -> Bitmap Index Scan on tenk1_unique2
+ Index Cond: (unique2 = 3)
-> Bitmap Heap Scan on tenk1 b
Recheck Cond: ((unique1 = 2) OR (hundred = 4))
-> BitmapOr
@@ -4339,15 +4343,7 @@ select * from tenk1 a join tenk1 b on
Index Cond: (unique1 = 2)
-> Bitmap Index Scan on tenk1_hundred
Index Cond: (hundred = 4)
- -> Materialize
- -> Bitmap Heap Scan on tenk1 a
- Recheck Cond: ((unique1 = 1) OR (unique2 = 3))
- -> BitmapOr
- -> Bitmap Index Scan on tenk1_unique1
- Index Cond: (unique1 = 1)
- -> Bitmap Index Scan on tenk1_unique2
- Index Cond: (unique2 = 3)
-(17 rows)
+(16 rows)
explain (costs off)
select * from tenk1 a join tenk1 b on
@@ -4356,17 +4352,16 @@ select * from tenk1 a join tenk1 b on
---------------------------------------------------------------------------------------------
Nested Loop
Join Filter: (((a.unique1 = 1) AND (b.unique1 = 2)) OR ((a.unique2 = 3) AND (b.ten = 4)))
+ -> Bitmap Heap Scan on tenk1 a
+ Recheck Cond: ((unique1 = 1) OR (unique2 = 3))
+ -> BitmapOr
+ -> Bitmap Index Scan on tenk1_unique1
+ Index Cond: (unique1 = 1)
+ -> Bitmap Index Scan on tenk1_unique2
+ Index Cond: (unique2 = 3)
-> Seq Scan on tenk1 b
Filter: ((unique1 = 2) OR (ten = 4))
- -> Materialize
- -> Bitmap Heap Scan on tenk1 a
- Recheck Cond: ((unique1 = 1) OR (unique2 = 3))
- -> BitmapOr
- -> Bitmap Index Scan on tenk1_unique1
- Index Cond: (unique1 = 1)
- -> Bitmap Index Scan on tenk1_unique2
- Index Cond: (unique2 = 3)
-(12 rows)
+(11 rows)
explain (costs off)
select * from tenk1 a join tenk1 b on
@@ -5110,13 +5105,12 @@ select 1 from
Join Filter: (i4.f1 IS NOT NULL)
-> Seq Scan on int4_tbl i4
Filter: (2 < f1)
- -> Materialize
- -> Seq Scan on int8_tbl i8
+ -> Seq Scan on int8_tbl i8
-> Result
One-Time Filter: false
-> Materialize
-> Seq Scan on int4_tbl i42
-(16 rows)
+(15 rows)
--
-- test for appropriate join order in the presence of lateral references
@@ -6157,18 +6151,17 @@ FROM int4_tbl
JOIN ((SELECT 42 AS x FROM int8_tbl LEFT JOIN innertab ON q1 = id) AS ss1
RIGHT JOIN tenk1 ON NULL)
ON tenk1.unique1 = ss1.x OR tenk1.unique2 = ss1.x;
- QUERY PLAN
---------------------------------------------------------------------------
+ QUERY PLAN
+--------------------------------------------------------------------
Nested Loop
+ -> Nested Loop Left Join
+ Join Filter: NULL::boolean
+ Filter: ((tenk1.unique1 = (42)) OR (tenk1.unique2 = (42)))
+ -> Seq Scan on tenk1
+ -> Result
+ One-Time Filter: false
-> Seq Scan on int4_tbl
- -> Materialize
- -> Nested Loop Left Join
- Join Filter: NULL::boolean
- Filter: ((tenk1.unique1 = (42)) OR (tenk1.unique2 = (42)))
- -> Seq Scan on tenk1
- -> Result
- One-Time Filter: false
-(9 rows)
+(8 rows)
rollback;
-- another join removal bug: we must clean up correctly when removing a PHV
@@ -6566,28 +6559,26 @@ where t1.a = t2.a;
explain (costs off)
select * from sj x join sj y on x.a = y.a
left join int8_tbl z on x.a = z.q1;
- QUERY PLAN
-------------------------------------
+ QUERY PLAN
+---------------------------------
Nested Loop Left Join
Join Filter: (y.a = z.q1)
-> Seq Scan on sj y
Filter: (a IS NOT NULL)
- -> Materialize
- -> Seq Scan on int8_tbl z
-(6 rows)
+ -> Seq Scan on int8_tbl z
+(5 rows)
explain (costs off)
select * from sj x join sj y on x.a = y.a
left join int8_tbl z on y.a = z.q1;
- QUERY PLAN
-------------------------------------
+ QUERY PLAN
+---------------------------------
Nested Loop Left Join
Join Filter: (y.a = z.q1)
-> Seq Scan on sj y
Filter: (a IS NOT NULL)
- -> Materialize
- -> Seq Scan on int8_tbl z
-(6 rows)
+ -> Seq Scan on int8_tbl z
+(5 rows)
explain (costs off)
select * from (
@@ -6614,16 +6605,15 @@ left join (select coalesce(y.q1, 1) from int8_tbl y
right join sj j1 inner join sj j2 on j1.a = j2.a
on true) z
on true;
- QUERY PLAN
-------------------------------------------
+ QUERY PLAN
+---------------------------------------
Nested Loop Left Join
-> Result
-> Nested Loop Left Join
-> Seq Scan on sj j2
Filter: (a IS NOT NULL)
- -> Materialize
- -> Seq Scan on int8_tbl y
-(7 rows)
+ -> Seq Scan on int8_tbl y
+(6 rows)
-- Test that references to the removed rel in lateral subqueries are replaced
-- correctly after join removal
@@ -6980,32 +6970,36 @@ select p.b from sj p join sj q on p.a = q.a group by p.b having sum(p.a) = 1;
explain (verbose, costs off)
select 1 from (select x.* from sj x, sj y where x.a = y.a) q,
lateral generate_series(1, q.a) gs(i);
- QUERY PLAN
-------------------------------------------------------
+ QUERY PLAN
+------------------------------------------------------------
Nested Loop
Output: 1
-> Seq Scan on public.sj y
Output: y.a, y.b, y.c
Filter: (y.a IS NOT NULL)
- -> Function Scan on pg_catalog.generate_series gs
- Output: gs.i
- Function Call: generate_series(1, y.a)
-(8 rows)
+ -> Memoize
+ Cache Key: y.a
+ Cache Mode: binary
+ -> Function Scan on pg_catalog.generate_series gs
+ Function Call: generate_series(1, y.a)
+(10 rows)
explain (verbose, costs off)
select 1 from (select y.* from sj x, sj y where x.a = y.a) q,
lateral generate_series(1, q.a) gs(i);
- QUERY PLAN
-------------------------------------------------------
+ QUERY PLAN
+------------------------------------------------------------
Nested Loop
Output: 1
-> Seq Scan on public.sj y
Output: y.a, y.b, y.c
Filter: (y.a IS NOT NULL)
- -> Function Scan on pg_catalog.generate_series gs
- Output: gs.i
- Function Call: generate_series(1, y.a)
-(8 rows)
+ -> Memoize
+ Cache Key: y.a
+ Cache Mode: binary
+ -> Function Scan on pg_catalog.generate_series gs
+ Function Call: generate_series(1, y.a)
+(10 rows)
-- Test that a non-EC-derived join clause is processed correctly. Use an
-- outer join so that we can't form an EC.
diff --git a/src/test/regress/expected/join_hash.out b/src/test/regress/expected/join_hash.out
index 4fc34a0e72a..262fa71ed8d 100644
--- a/src/test/regress/expected/join_hash.out
+++ b/src/test/regress/expected/join_hash.out
@@ -508,17 +508,18 @@ set local hash_mem_multiplier = 1.0;
set local enable_parallel_hash = on;
explain (costs off)
select count(*) from simple r join extremely_skewed s using (id);
- QUERY PLAN
------------------------------------------------------------------
- Aggregate
+ QUERY PLAN
+-----------------------------------------------------------------------
+ Finalize Aggregate
-> Gather
Workers Planned: 1
- -> Parallel Hash Join
- Hash Cond: (r.id = s.id)
- -> Parallel Seq Scan on simple r
- -> Parallel Hash
- -> Parallel Seq Scan on extremely_skewed s
-(8 rows)
+ -> Partial Aggregate
+ -> Parallel Hash Join
+ Hash Cond: (r.id = s.id)
+ -> Parallel Seq Scan on simple r
+ -> Parallel Hash
+ -> Parallel Seq Scan on extremely_skewed s
+(9 rows)
select count(*) from simple r join extremely_skewed s using (id);
count
diff --git a/src/test/regress/expected/misc_functions.out b/src/test/regress/expected/misc_functions.out
index d3f5d16a672..6289d3e03d1 100644
--- a/src/test/regress/expected/misc_functions.out
+++ b/src/test/regress/expected/misc_functions.out
@@ -647,27 +647,27 @@ SELECT * FROM tenk1 a JOIN my_gen_series(1,10) g ON a.unique1 = g;
SELECT explain_mask_costs($$
SELECT * FROM generate_series(TIMESTAMPTZ '2024-02-01', TIMESTAMPTZ '2024-03-01', INTERVAL '1 day') g(s);$$,
true, true, false, true);
- explain_mask_costs
----------------------------------------------------------------------------------------------
- Function Scan on generate_series g (cost=N..N rows=30 width=N) (actual rows=30.00 loops=1)
+ explain_mask_costs
+------------------------------------------------------------------------------------------------
+ Function Scan on generate_series g (cost=N..N rows=30.00 width=N) (actual rows=30.00 loops=1)
(1 row)
-- As above but with generate_series_timestamp
SELECT explain_mask_costs($$
SELECT * FROM generate_series(TIMESTAMP '2024-02-01', TIMESTAMP '2024-03-01', INTERVAL '1 day') g(s);$$,
true, true, false, true);
- explain_mask_costs
----------------------------------------------------------------------------------------------
- Function Scan on generate_series g (cost=N..N rows=30 width=N) (actual rows=30.00 loops=1)
+ explain_mask_costs
+------------------------------------------------------------------------------------------------
+ Function Scan on generate_series g (cost=N..N rows=30.00 width=N) (actual rows=30.00 loops=1)
(1 row)
-- As above but with generate_series_timestamptz_at_zone()
SELECT explain_mask_costs($$
SELECT * FROM generate_series(TIMESTAMPTZ '2024-02-01', TIMESTAMPTZ '2024-03-01', INTERVAL '1 day', 'UTC') g(s);$$,
true, true, false, true);
- explain_mask_costs
----------------------------------------------------------------------------------------------
- Function Scan on generate_series g (cost=N..N rows=30 width=N) (actual rows=30.00 loops=1)
+ explain_mask_costs
+------------------------------------------------------------------------------------------------
+ Function Scan on generate_series g (cost=N..N rows=30.00 width=N) (actual rows=30.00 loops=1)
(1 row)
-- Ensure the estimated and actual row counts match when the range isn't
@@ -675,36 +675,36 @@ true, true, false, true);
SELECT explain_mask_costs($$
SELECT * FROM generate_series(TIMESTAMPTZ '2024-02-01', TIMESTAMPTZ '2024-03-01', INTERVAL '7 day') g(s);$$,
true, true, false, true);
- explain_mask_costs
--------------------------------------------------------------------------------------------
- Function Scan on generate_series g (cost=N..N rows=5 width=N) (actual rows=5.00 loops=1)
+ explain_mask_costs
+----------------------------------------------------------------------------------------------
+ Function Scan on generate_series g (cost=N..N rows=5.00 width=N) (actual rows=5.00 loops=1)
(1 row)
-- Ensure the estimates match when step is decreasing
SELECT explain_mask_costs($$
SELECT * FROM generate_series(TIMESTAMPTZ '2024-03-01', TIMESTAMPTZ '2024-02-01', INTERVAL '-1 day') g(s);$$,
true, true, false, true);
- explain_mask_costs
----------------------------------------------------------------------------------------------
- Function Scan on generate_series g (cost=N..N rows=30 width=N) (actual rows=30.00 loops=1)
+ explain_mask_costs
+------------------------------------------------------------------------------------------------
+ Function Scan on generate_series g (cost=N..N rows=30.00 width=N) (actual rows=30.00 loops=1)
(1 row)
-- Ensure an empty range estimates 1 row
SELECT explain_mask_costs($$
SELECT * FROM generate_series(TIMESTAMPTZ '2024-03-01', TIMESTAMPTZ '2024-02-01', INTERVAL '1 day') g(s);$$,
true, true, false, true);
- explain_mask_costs
--------------------------------------------------------------------------------------------
- Function Scan on generate_series g (cost=N..N rows=1 width=N) (actual rows=0.00 loops=1)
+ explain_mask_costs
+----------------------------------------------------------------------------------------------
+ Function Scan on generate_series g (cost=N..N rows=1.00 width=N) (actual rows=0.00 loops=1)
(1 row)
-- Ensure we get the default row estimate for infinity values
SELECT explain_mask_costs($$
SELECT * FROM generate_series(TIMESTAMPTZ '-infinity', TIMESTAMPTZ 'infinity', INTERVAL '1 day') g(s);$$,
false, true, false, true);
- explain_mask_costs
--------------------------------------------------------------------
- Function Scan on generate_series g (cost=N..N rows=1000 width=N)
+ explain_mask_costs
+----------------------------------------------------------------------
+ Function Scan on generate_series g (cost=N..N rows=1000.00 width=N)
(1 row)
-- Ensure the row estimate behaves correctly when step size is zero.
@@ -719,36 +719,36 @@ ERROR: step size cannot equal zero
SELECT explain_mask_costs($$
SELECT * FROM generate_series(1.0, 25.0) g(s);$$,
true, true, false, true);
- explain_mask_costs
----------------------------------------------------------------------------------------------
- Function Scan on generate_series g (cost=N..N rows=25 width=N) (actual rows=25.00 loops=1)
+ explain_mask_costs
+------------------------------------------------------------------------------------------------
+ Function Scan on generate_series g (cost=N..N rows=25.00 width=N) (actual rows=25.00 loops=1)
(1 row)
-- As above but with non-default step
SELECT explain_mask_costs($$
SELECT * FROM generate_series(1.0, 25.0, 2.0) g(s);$$,
true, true, false, true);
- explain_mask_costs
----------------------------------------------------------------------------------------------
- Function Scan on generate_series g (cost=N..N rows=13 width=N) (actual rows=13.00 loops=1)
+ explain_mask_costs
+------------------------------------------------------------------------------------------------
+ Function Scan on generate_series g (cost=N..N rows=13.00 width=N) (actual rows=13.00 loops=1)
(1 row)
-- Ensure the estimates match when step is decreasing
SELECT explain_mask_costs($$
SELECT * FROM generate_series(25.0, 1.0, -1.0) g(s);$$,
true, true, false, true);
- explain_mask_costs
----------------------------------------------------------------------------------------------
- Function Scan on generate_series g (cost=N..N rows=25 width=N) (actual rows=25.00 loops=1)
+ explain_mask_costs
+------------------------------------------------------------------------------------------------
+ Function Scan on generate_series g (cost=N..N rows=25.00 width=N) (actual rows=25.00 loops=1)
(1 row)
-- Ensure an empty range estimates 1 row
SELECT explain_mask_costs($$
SELECT * FROM generate_series(25.0, 1.0, 1.0) g(s);$$,
true, true, false, true);
- explain_mask_costs
--------------------------------------------------------------------------------------------
- Function Scan on generate_series g (cost=N..N rows=1 width=N) (actual rows=0.00 loops=1)
+ explain_mask_costs
+----------------------------------------------------------------------------------------------
+ Function Scan on generate_series g (cost=N..N rows=1.00 width=N) (actual rows=0.00 loops=1)
(1 row)
-- Ensure we get the default row estimate for error cases (infinity/NaN values
@@ -756,25 +756,25 @@ true, true, false, true);
SELECT explain_mask_costs($$
SELECT * FROM generate_series('-infinity'::NUMERIC, 'infinity'::NUMERIC, 1.0) g(s);$$,
false, true, false, true);
- explain_mask_costs
--------------------------------------------------------------------
- Function Scan on generate_series g (cost=N..N rows=1000 width=N)
+ explain_mask_costs
+----------------------------------------------------------------------
+ Function Scan on generate_series g (cost=N..N rows=1000.00 width=N)
(1 row)
SELECT explain_mask_costs($$
SELECT * FROM generate_series(1.0, 25.0, 'NaN'::NUMERIC) g(s);$$,
false, true, false, true);
- explain_mask_costs
--------------------------------------------------------------------
- Function Scan on generate_series g (cost=N..N rows=1000 width=N)
+ explain_mask_costs
+----------------------------------------------------------------------
+ Function Scan on generate_series g (cost=N..N rows=1000.00 width=N)
(1 row)
SELECT explain_mask_costs($$
SELECT * FROM generate_series(25.0, 2.0, 0.0) g(s);$$,
false, true, false, true);
- explain_mask_costs
--------------------------------------------------------------------
- Function Scan on generate_series g (cost=N..N rows=1000 width=N)
+ explain_mask_costs
+----------------------------------------------------------------------
+ Function Scan on generate_series g (cost=N..N rows=1000.00 width=N)
(1 row)
-- Test functions for control data
diff --git a/src/test/regress/expected/partition_join.out b/src/test/regress/expected/partition_join.out
index 6101c8c7cf1..fe960db944f 100644
--- a/src/test/regress/expected/partition_join.out
+++ b/src/test/regress/expected/partition_join.out
@@ -28,8 +28,8 @@ ANALYZE prt2;
-- inner join
EXPLAIN (COSTS OFF)
SELECT t1.a, t1.c, t2.b, t2.c FROM prt1 t1, prt2 t2 WHERE t1.a = t2.b AND t1.b = 0 ORDER BY t1.a, t2.b;
- QUERY PLAN
---------------------------------------------------
+ QUERY PLAN
+---------------------------------------------------------------
Sort
Sort Key: t1.a
-> Append
@@ -45,13 +45,12 @@ SELECT t1.a, t1.c, t2.b, t2.c FROM prt1 t1, prt2 t2 WHERE t1.a = t2.b AND t1.b =
-> Hash
-> Seq Scan on prt1_p2 t1_2
Filter: (b = 0)
- -> Hash Join
- Hash Cond: (t2_3.b = t1_3.a)
- -> Seq Scan on prt2_p3 t2_3
- -> Hash
- -> Seq Scan on prt1_p3 t1_3
- Filter: (b = 0)
-(21 rows)
+ -> Nested Loop
+ -> Seq Scan on prt1_p3 t1_3
+ Filter: (b = 0)
+ -> Index Scan using iprt2_p3_b on prt2_p3 t2_3
+ Index Cond: (b = t1_3.a)
+(20 rows)
SELECT t1.a, t1.c, t2.b, t2.c FROM prt1 t1, prt2 t2 WHERE t1.a = t2.b AND t1.b = 0 ORDER BY t1.a, t2.b;
a | c | b | c
@@ -834,15 +833,14 @@ SELECT t1.a, t1.c, t2.b, t2.c, t3.a + t3.b, t3.c FROM prt1 t1, prt2 t2, prt1_e t
Index Cond: (((a + b) / 2) = t2_2.b)
-> Nested Loop
Join Filter: (t1_3.a = ((t3_3.a + t3_3.b) / 2))
- -> Hash Join
- Hash Cond: (t2_3.b = t1_3.a)
- -> Seq Scan on prt2_p3 t2_3
- -> Hash
- -> Seq Scan on prt1_p3 t1_3
- Filter: (b = 0)
+ -> Nested Loop
+ -> Seq Scan on prt1_p3 t1_3
+ Filter: (b = 0)
+ -> Index Scan using iprt2_p3_b on prt2_p3 t2_3
+ Index Cond: (b = t1_3.a)
-> Index Scan using iprt1_e_p3_ab2 on prt1_e_p3 t3_3
Index Cond: (((a + b) / 2) = t2_3.b)
-(33 rows)
+(32 rows)
SELECT t1.a, t1.c, t2.b, t2.c, t3.a + t3.b, t3.c FROM prt1 t1, prt2 t2, prt1_e t3 WHERE t1.a = t2.b AND t1.a = (t3.a + t3.b)/2 AND t1.b = 0 ORDER BY t1.a, t2.b;
a | c | b | c | ?column? | c
@@ -855,8 +853,8 @@ SELECT t1.a, t1.c, t2.b, t2.c, t3.a + t3.b, t3.c FROM prt1 t1, prt2 t2, prt1_e t
EXPLAIN (COSTS OFF)
SELECT t1.a, t1.c, t2.b, t2.c, t3.a + t3.b, t3.c FROM (prt1 t1 LEFT JOIN prt2 t2 ON t1.a = t2.b) LEFT JOIN prt1_e t3 ON (t1.a = (t3.a + t3.b)/2) WHERE t1.b = 0 ORDER BY t1.a, t2.b, t3.a + t3.b;
- QUERY PLAN
---------------------------------------------------------------
+ QUERY PLAN
+---------------------------------------------------------------------
Sort
Sort Key: t1.a, t2.b, ((t3.a + t3.b))
-> Append
@@ -880,17 +878,15 @@ SELECT t1.a, t1.c, t2.b, t2.c, t3.a + t3.b, t3.c FROM (prt1 t1 LEFT JOIN prt2 t2
-> Hash
-> Seq Scan on prt1_p2 t1_2
Filter: (b = 0)
- -> Hash Right Join
- Hash Cond: (((t3_3.a + t3_3.b) / 2) = t1_3.a)
- -> Seq Scan on prt1_e_p3 t3_3
- -> Hash
- -> Hash Right Join
- Hash Cond: (t2_3.b = t1_3.a)
- -> Seq Scan on prt2_p3 t2_3
- -> Hash
- -> Seq Scan on prt1_p3 t1_3
- Filter: (b = 0)
-(33 rows)
+ -> Nested Loop Left Join
+ -> Nested Loop Left Join
+ -> Seq Scan on prt1_p3 t1_3
+ Filter: (b = 0)
+ -> Index Scan using iprt2_p3_b on prt2_p3 t2_3
+ Index Cond: (b = t1_3.a)
+ -> Index Scan using iprt1_e_p3_ab2 on prt1_e_p3 t3_3
+ Index Cond: (((a + b) / 2) = t1_3.a)
+(31 rows)
SELECT t1.a, t1.c, t2.b, t2.c, t3.a + t3.b, t3.c FROM (prt1 t1 LEFT JOIN prt2 t2 ON t1.a = t2.b) LEFT JOIN prt1_e t3 ON (t1.a = (t3.a + t3.b)/2) WHERE t1.b = 0 ORDER BY t1.a, t2.b, t3.a + t3.b;
a | c | b | c | ?column? | c
@@ -911,8 +907,8 @@ SELECT t1.a, t1.c, t2.b, t2.c, t3.a + t3.b, t3.c FROM (prt1 t1 LEFT JOIN prt2 t2
EXPLAIN (COSTS OFF)
SELECT t1.a, t1.c, t2.b, t2.c, t3.a + t3.b, t3.c FROM (prt1 t1 LEFT JOIN prt2 t2 ON t1.a = t2.b) RIGHT JOIN prt1_e t3 ON (t1.a = (t3.a + t3.b)/2) WHERE t3.c = 0 ORDER BY t1.a, t2.b, t3.a + t3.b;
- QUERY PLAN
--------------------------------------------------------------------
+ QUERY PLAN
+---------------------------------------------------------------------
Sort
Sort Key: t1.a, t2.b, ((t3.a + t3.b))
-> Append
@@ -935,15 +931,14 @@ SELECT t1.a, t1.c, t2.b, t2.c, t3.a + t3.b, t3.c FROM (prt1 t1 LEFT JOIN prt2 t2
-> Index Scan using iprt2_p2_b on prt2_p2 t2_2
Index Cond: (b = t1_2.a)
-> Nested Loop Left Join
- -> Hash Right Join
- Hash Cond: (t1_3.a = ((t3_3.a + t3_3.b) / 2))
- -> Seq Scan on prt1_p3 t1_3
- -> Hash
- -> Seq Scan on prt1_e_p3 t3_3
- Filter: (c = 0)
- -> Index Scan using iprt2_p3_b on prt2_p3 t2_3
- Index Cond: (b = t1_3.a)
-(30 rows)
+ -> Seq Scan on prt1_e_p3 t3_3
+ Filter: (c = 0)
+ -> Nested Loop Left Join
+ -> Index Scan using iprt1_p3_a on prt1_p3 t1_3
+ Index Cond: (a = ((t3_3.a + t3_3.b) / 2))
+ -> Index Scan using iprt2_p3_b on prt2_p3 t2_3
+ Index Cond: (b = t1_3.a)
+(29 rows)
SELECT t1.a, t1.c, t2.b, t2.c, t3.a + t3.b, t3.c FROM (prt1 t1 LEFT JOIN prt2 t2 ON t1.a = t2.b) RIGHT JOIN prt1_e t3 ON (t1.a = (t3.a + t3.b)/2) WHERE t3.c = 0 ORDER BY t1.a, t2.b, t3.a + t3.b;
a | c | b | c | ?column? | c
@@ -1749,8 +1744,8 @@ ALTER TABLE prt2 ATTACH PARTITION prt2_p3 DEFAULT;
ANALYZE prt2;
EXPLAIN (COSTS OFF)
SELECT t1.a, t1.c, t2.b, t2.c FROM prt1 t1, prt2 t2 WHERE t1.a = t2.b AND t1.b = 0 ORDER BY t1.a, t2.b;
- QUERY PLAN
---------------------------------------------------
+ QUERY PLAN
+---------------------------------------------------------------
Sort
Sort Key: t1.a
-> Append
@@ -1766,13 +1761,12 @@ SELECT t1.a, t1.c, t2.b, t2.c FROM prt1 t1, prt2 t2 WHERE t1.a = t2.b AND t1.b =
-> Hash
-> Seq Scan on prt1_p2 t1_2
Filter: (b = 0)
- -> Hash Join
- Hash Cond: (t2_3.b = t1_3.a)
- -> Seq Scan on prt2_p3 t2_3
- -> Hash
- -> Seq Scan on prt1_p3 t1_3
- Filter: (b = 0)
-(21 rows)
+ -> Nested Loop
+ -> Seq Scan on prt1_p3 t1_3
+ Filter: (b = 0)
+ -> Index Scan using iprt2_p3_b on prt2_p3 t2_3
+ Index Cond: (b = t1_3.a)
+(20 rows)
-- test default partition behavior for list
ALTER TABLE plt1 DETACH PARTITION plt1_p3;
@@ -5175,20 +5169,18 @@ SELECT t1.*, t2.* FROM alpha t1 INNER JOIN beta t2 ON (t1.a = t2.a AND t1.b = t2
Sort
Sort Key: t1.a, t1.b
-> Append
- -> Hash Join
- Hash Cond: ((t1_1.a = t2_1.a) AND (t1_1.b = t2_1.b) AND (t1_1.c = t2_1.c))
+ -> Nested Loop
+ Join Filter: ((t1_1.a = t2_1.a) AND (t1_1.b = t2_1.b) AND (t1_1.c = t2_1.c))
-> Seq Scan on alpha_neg_p1 t1_1
Filter: ((c = ANY ('{0004,0009}'::text[])) AND (((b >= 100) AND (b < 110)) OR ((b >= 200) AND (b < 210))))
- -> Hash
- -> Seq Scan on beta_neg_p1 t2_1
- Filter: (((b >= 100) AND (b < 110)) OR ((b >= 200) AND (b < 210)))
- -> Hash Join
- Hash Cond: ((t1_2.a = t2_2.a) AND (t1_2.b = t2_2.b) AND (t1_2.c = t2_2.c))
+ -> Seq Scan on beta_neg_p1 t2_1
+ Filter: (((b >= 100) AND (b < 110)) OR ((b >= 200) AND (b < 210)))
+ -> Nested Loop
+ Join Filter: ((t1_2.a = t2_2.a) AND (t1_2.b = t2_2.b) AND (t1_2.c = t2_2.c))
-> Seq Scan on alpha_neg_p2 t1_2
Filter: ((c = ANY ('{0004,0009}'::text[])) AND (((b >= 100) AND (b < 110)) OR ((b >= 200) AND (b < 210))))
- -> Hash
- -> Seq Scan on beta_neg_p2 t2_2
- Filter: (((b >= 100) AND (b < 110)) OR ((b >= 200) AND (b < 210)))
+ -> Seq Scan on beta_neg_p2 t2_2
+ Filter: (((b >= 100) AND (b < 110)) OR ((b >= 200) AND (b < 210)))
-> Nested Loop
Join Filter: ((t1_3.a = t2_3.a) AND (t1_3.b = t2_3.b) AND (t1_3.c = t2_3.c))
-> Seq Scan on alpha_pos_p2 t1_3
@@ -5201,7 +5193,7 @@ SELECT t1.*, t2.* FROM alpha t1 INNER JOIN beta t2 ON (t1.a = t2.a AND t1.b = t2
Filter: ((c = ANY ('{0004,0009}'::text[])) AND (((b >= 100) AND (b < 110)) OR ((b >= 200) AND (b < 210))))
-> Seq Scan on beta_pos_p3 t2_4
Filter: (((b >= 100) AND (b < 110)) OR ((b >= 200) AND (b < 210)))
-(29 rows)
+(27 rows)
SELECT t1.*, t2.* FROM alpha t1 INNER JOIN beta t2 ON (t1.a = t2.a AND t1.b = t2.b AND t1.c = t2.c) WHERE ((t1.b >= 100 AND t1.b < 110) OR (t1.b >= 200 AND t1.b < 210)) AND ((t2.b >= 100 AND t2.b < 110) OR (t2.b >= 200 AND t2.b < 210)) AND t1.c IN ('0004', '0009') ORDER BY t1.a, t1.b;
a | b | c | a | b | c
diff --git a/src/test/regress/expected/select_views.out b/src/test/regress/expected/select_views.out
index 1aeed8452bd..b3c0cbf0607 100644
--- a/src/test/regress/expected/select_views.out
+++ b/src/test/regress/expected/select_views.out
@@ -1461,18 +1461,17 @@ EXPLAIN (COSTS OFF) SELECT * FROM my_credit_card_usage_normal
------------------------------------------------------------------------------
Nested Loop
Join Filter: (l.cid = r.cid)
+ -> Subquery Scan on l
+ Filter: f_leak(l.cnum)
+ -> Hash Join
+ Hash Cond: (r_1.cid = l_1.cid)
+ -> Seq Scan on credit_card r_1
+ -> Hash
+ -> Seq Scan on customer l_1
+ Filter: (name = CURRENT_USER)
-> Seq Scan on credit_usage r
Filter: ((ymd >= '10-01-2011'::date) AND (ymd < '11-01-2011'::date))
- -> Materialize
- -> Subquery Scan on l
- Filter: f_leak(l.cnum)
- -> Hash Join
- Hash Cond: (r_1.cid = l_1.cid)
- -> Seq Scan on credit_card r_1
- -> Hash
- -> Seq Scan on customer l_1
- Filter: (name = CURRENT_USER)
-(13 rows)
+(12 rows)
SELECT * FROM my_credit_card_usage_secure
WHERE f_leak(cnum) AND ymd >= '2011-10-01' AND ymd < '2011-11-01';
diff --git a/src/test/regress/expected/stats_ext.out b/src/test/regress/expected/stats_ext.out
index 686d8c93aa8..5f17271202a 100644
--- a/src/test/regress/expected/stats_ext.out
+++ b/src/test/regress/expected/stats_ext.out
@@ -18,8 +18,8 @@ begin
loop
if first_row then
first_row := false;
- tmp := regexp_match(ln, 'rows=(\d*) .* rows=(\d*)');
- return query select tmp[1]::int, tmp[2]::int;
+ tmp := regexp_match(ln, 'rows=(\d+\.\d{2}) .* rows=(\d+\.\d{2})');
+ return query select round(tmp[1]::numeric)::int, round(tmp[2]::numeric)::int;
end if;
end loop;
end;
@@ -1345,13 +1345,13 @@ SELECT * FROM check_estimated_rows('SELECT * FROM functional_dependencies WHERE
SELECT * FROM check_estimated_rows('SELECT * FROM functional_dependencies WHERE (a = 1 OR a = 51) AND b = ''1''');
estimated | actual
-----------+--------
- 99 | 100
+ 100 | 100
(1 row)
SELECT * FROM check_estimated_rows('SELECT * FROM functional_dependencies WHERE (a = 1 OR a = 51) AND (b = ''1'' OR b = ''2'')');
estimated | actual
-----------+--------
- 99 | 100
+ 100 | 100
(1 row)
SELECT * FROM check_estimated_rows('SELECT * FROM functional_dependencies WHERE (a = 1 OR a = 2 OR a = 51 OR a = 52) AND (b = ''1'' OR b = ''2'')');
@@ -1687,13 +1687,13 @@ SELECT * FROM check_estimated_rows('SELECT * FROM functional_dependencies WHERE
SELECT * FROM check_estimated_rows('SELECT * FROM functional_dependencies WHERE ((a * 2) = 2 OR (a * 2) = 102) AND upper(b) = ''1''');
estimated | actual
-----------+--------
- 99 | 100
+ 100 | 100
(1 row)
SELECT * FROM check_estimated_rows('SELECT * FROM functional_dependencies WHERE ((a * 2) = 2 OR (a * 2) = 102) AND (upper(b) = ''1'' OR upper(b) = ''2'')');
estimated | actual
-----------+--------
- 99 | 100
+ 100 | 100
(1 row)
SELECT * FROM check_estimated_rows('SELECT * FROM functional_dependencies WHERE ((a * 2) = 2 OR (a * 2) = 4 OR (a * 2) = 102 OR (a * 2) = 104) AND (upper(b) = ''1'' OR upper(b) = ''2'')');
diff --git a/src/test/regress/sql/stats_ext.sql b/src/test/regress/sql/stats_ext.sql
index b71a6cd089f..f49a7b6a237 100644
--- a/src/test/regress/sql/stats_ext.sql
+++ b/src/test/regress/sql/stats_ext.sql
@@ -20,8 +20,8 @@ begin
loop
if first_row then
first_row := false;
- tmp := regexp_match(ln, 'rows=(\d*) .* rows=(\d*)');
- return query select tmp[1]::int, tmp[2]::int;
+ tmp := regexp_match(ln, 'rows=(\d+\.\d{2}) .* rows=(\d+\.\d{2})');
+ return query select round(tmp[1]::numeric)::int, round(tmp[2]::numeric)::int;
end if;
end loop;
end;
--
2.34.1
^ permalink raw reply [nested|flat] 45+ messages in thread
end of thread, other threads:[~2025-04-09 19:40 UTC | newest]
Thread overview: 45+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2009-05-29 01:30 explain analyze rows=%.0f Robert Haas <[email protected]>
2009-05-29 03:00 ` Euler Taveira de Oliveira <[email protected]>
2009-05-29 03:12 ` Robert Haas <[email protected]>
2009-05-29 04:16 ` Joshua Tolley <[email protected]>
2009-05-29 17:30 ` Tom Lane <[email protected]>
2009-05-29 18:39 ` Robert Haas <[email protected]>
2009-06-02 03:30 ` Ron Mayer <[email protected]>
2009-06-02 13:41 ` Simon Riggs <[email protected]>
2009-06-02 14:06 ` Robert Haas <[email protected]>
2009-06-02 14:38 ` Tom Lane <[email protected]>
2009-06-02 14:56 ` Robert Haas <[email protected]>
2022-06-22 19:11 ` Ibrar Ahmed <[email protected]>
2022-06-22 20:04 ` David G. Johnston <[email protected]>
2022-06-22 20:55 ` Ibrar Ahmed <[email protected]>
2022-07-07 09:41 ` Amit Kapila <[email protected]>
2022-07-08 18:09 ` Ibrar Ahmed <[email protected]>
2022-07-07 10:14 ` vignesh C <[email protected]>
2022-07-07 17:53 ` Greg Stark <[email protected]>
2022-07-07 20:21 ` Robert Haas <[email protected]>
2022-07-07 20:45 ` Peter Geoghegan <[email protected]>
2022-07-07 22:20 ` Justin Pryzby <[email protected]>
2022-07-22 10:47 ` Amit Kapila <[email protected]>
2022-07-22 12:07 ` Robert Haas <[email protected]>
2022-07-22 20:01 ` Naeem Akhter <[email protected]>
2022-07-25 17:18 ` Hamid Akhtar <[email protected]>
2022-07-26 06:46 ` Hamid Akhtar <[email protected]>
2022-11-06 05:12 ` Tom Lane <[email protected]>
2022-11-07 04:20 ` Andrey Lepikhov <[email protected]>
2025-03-11 09:58 ` Ilia Evdokimov <[email protected]>
2025-03-31 17:35 ` Robert Haas <[email protected]>
2025-03-31 17:49 ` Tom Lane <[email protected]>
2025-03-31 19:09 ` Robert Haas <[email protected]>
2025-03-31 20:59 ` Ilia Evdokimov <[email protected]>
2025-04-09 19:40 ` Ilia Evdokimov <[email protected]>
2022-07-08 18:12 ` Ibrar Ahmed <[email protected]>
2022-07-08 18:10 ` Ibrar Ahmed <[email protected]>
2022-02-25 23:00 [PATCH 08/21] vcregress: add alltaptests Justin Pryzby <[email protected]>
2022-02-25 23:00 [PATCH 08/19] vcregress: add alltaptests Justin Pryzby <[email protected]>
2022-02-25 23:00 [PATCH 4/7] vcregress: add alltaptests Justin Pryzby <[email protected]>
2022-02-25 23:00 [PATCH 08/19] vcregress: add alltaptests Justin Pryzby <[email protected]>
2022-02-25 23:00 [PATCH 11/25] vcregress: add alltaptests Justin Pryzby <[email protected]>
2022-02-25 23:00 [PATCH 09/23] vcregress: add alltaptests Justin Pryzby <[email protected]>
2022-02-25 23:00 [PATCH 08/19] vcregress: add alltaptests Justin Pryzby <[email protected]>
2022-02-25 23:00 [PATCH 09/23] vcregress: add alltaptests Justin Pryzby <[email protected]>
2022-02-25 23:00 [PATCH 08/21] vcregress: add alltaptests Justin Pryzby <[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