public inbox for [email protected]
help / color / mirror / Atom feedRe: [HACKERS] Proposal: generic WAL compression
26+ messages / 11 participants
[nested] [flat]
* Re: [HACKERS] Proposal: generic WAL compression
@ 2017-11-16 17:31 Antonin Houska <[email protected]>
0 siblings, 2 replies; 26+ messages in thread
From: Antonin Houska @ 2017-11-16 17:31 UTC (permalink / raw)
To: pgsql-hackers; +Cc: Oleg Ivanov <[email protected]>
Oleg Ivanov <[email protected]> wrote:
> In order to overcome that issue, I would like to propose the patch, which
> provides possibility to use another approach of the WAL record
> construction.
After having spent several hours reviewing this patch I dare to send the
following comments:
* writeToPtr() and readFromPtr() are applied to the existing code. I think
this is a reason for a separate diff, to be applied before the actual patch.
BTW, if you're going to do any refactoring of the existing code, I think the
"Begin" and "Start" words should be used consistently, see "fragmentBegin" vs
"validStart" in computeRegionBaseDelta().
* What's the reason for changing FRAGMENT_HEADER_SIZE ? I see no change in
the data layout that writeFragment() produces.
* alignRegions()
** typo in the prologue: "mismathing".
** "An O(ND) Difference Algorithm and Its Variations" - I think the reference
should contain more information, e.g. name of the author(s). I think I even
saw URLs to scientific papers in the PG source but I'm not 100% sure right
now.
** As for the algorithm itself: Although I didn't have enough patience (and
time) to follow it in every detail for this first review, I think I can
imagine what it is about. Yet I think reading would be easier if the
concepts of alignment and "diff delta" were depicted in the comments,
perhaps using some sort of "ASCII graphics".
** DiffDeltaOperations enumeration: the individual values should be described.
* computeRegionDiffDelta()
** Does "previous delta" in one of the comments refer to "base delta",
i.e. the delta computation w/o your patch? If so, the comment should
mention it explicitly.
** If you use Min() to initialize the for-loop, it'd be more consistent if you
also used Max() when checking the limits:
for (i = Min(validStart, targetStart); i < Max(validEnd, targetEnd); ++i)
And similarly you can simplify the body of the loop.
* computeDelta()
As the expresssion
*((bool *) pageData->delta)
is used more than once, I think a separate (bool *) variable should be used.
--
Antonin Houska
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt
Web: http://www.postgresql-support.de, http://www.cybertec.at
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: [HACKERS] Proposal: generic WAL compression
@ 2017-11-20 07:49 Antonin Houska <[email protected]>
parent: Antonin Houska <[email protected]>
1 sibling, 1 reply; 26+ messages in thread
From: Antonin Houska @ 2017-11-20 07:49 UTC (permalink / raw)
To: Oleg Ivanov <[email protected]>; +Cc: pgsql-hackers
Antonin Houska <[email protected]> wrote:
> Oleg Ivanov <[email protected]> wrote:
>
> > In order to overcome that issue, I would like to propose the patch, which
> > provides possibility to use another approach of the WAL record
> > construction.
>
> After having spent several hours reviewing this patch I dare to send the
> following comments:
One more idea:
I think the metadata (ALIGN_GAP) should be stored separate from the actual
data so that you can use memcpy() instead of this loop:
while (i < j)
{
char c = targetRegionAligned[i++];
writeToPtr(ptr, c);
}
--
Antonin Houska
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt
Web: http://www.postgresql-support.de, http://www.cybertec.at
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: [HACKERS] Proposal: generic WAL compression
@ 2017-11-30 02:40 Michael Paquier <[email protected]>
parent: Antonin Houska <[email protected]>
0 siblings, 0 replies; 26+ messages in thread
From: Michael Paquier @ 2017-11-30 02:40 UTC (permalink / raw)
To: Antonin Houska <[email protected]>; +Cc: Oleg Ivanov <[email protected]>; pgsql-hackers
On Mon, Nov 20, 2017 at 4:49 PM, Antonin Houska <[email protected]> wrote:
> One more idea:
>
> I think the metadata (ALIGN_GAP) should be stored separate from the actual
> data so that you can use memcpy() instead of this loop:
>
> while (i < j)
> {
> char c = targetRegionAligned[i++];
>
> writeToPtr(ptr, c);
> }
Moved to next CF per lack of reviews. The patch still applies.
Oleg, I would recommend as well that you do more patch reviews. Let's
not forget that for one patch submitted you should review one patch
with a somewhat equal difficulty, and yours is quite specialized.
--
Michael
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: [HACKERS] Proposal: generic WAL compression
@ 2018-01-22 21:37 Stephen Frost <[email protected]>
parent: Antonin Houska <[email protected]>
1 sibling, 0 replies; 26+ messages in thread
From: Stephen Frost @ 2018-01-22 21:37 UTC (permalink / raw)
To: Antonin Houska <[email protected]>; +Cc: pgsql-hackers; Oleg Ivanov <[email protected]>
Oleg,
I'm not really sure why this is still in Needs Review as a review was
posted and I don't see any follow-up. I've changed this to be Waiting
for Author.
* Antonin Houska ([email protected]) wrote:
> Oleg Ivanov <[email protected]> wrote:
>
> > In order to overcome that issue, I would like to propose the patch, which
> > provides possibility to use another approach of the WAL record
> > construction.
>
> After having spent several hours reviewing this patch I dare to send the
> following comments:
Thanks for the review Antonin!
> * writeToPtr() and readFromPtr() are applied to the existing code. I think
> this is a reason for a separate diff, to be applied before the actual patch.
I'm not really a fan of using these, for my 2c. I'm not sure how others
feel, but having these macros which change one of the arguments to the
macro (by advancing the pointer) doesn't result in a net improvement to
readability for me.
The other review comments also seem pretty reasonable to me.
Thanks!
Stephen
Attachments:
[application/pgp-signature] signature.asc (819B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 26+ messages in thread
* [PATCH v32 10/11] Preserve pg_stat_file() isdir..
@ 2020-11-29 04:29 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 26+ messages in thread
From: Justin Pryzby @ 2020-11-29 04:29 UTC (permalink / raw)
..per Tom's suggestion
---
src/backend/utils/adt/genfile.c | 15 ++++++++++++---
src/include/catalog/pg_proc.dat | 12 ++++++------
src/test/regress/expected/misc_functions.out | 4 ++--
3 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index bef20178ea6..7171357f442 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/src/backend/utils/adt/genfile.c
@@ -471,11 +471,12 @@ pg_stat_file(PG_FUNCTION_ARGS)
text *filename_t = PG_GETARG_TEXT_PP(0);
char *filename;
struct stat fst;
- Datum values[6];
- bool nulls[6];
+ Datum values[7];
+ bool nulls[7];
HeapTuple tuple;
TupleDesc tupdesc;
bool missing_ok = false;
+ char type;
/* check the optional argument */
if (PG_NARGS() == 2)
@@ -497,7 +498,7 @@ pg_stat_file(PG_FUNCTION_ARGS)
* This record type had better match the output parameters declared for me
* in pg_proc.h.
*/
- tupdesc = CreateTemplateTupleDesc(6);
+ tupdesc = CreateTemplateTupleDesc(7);
TupleDescInitEntry(tupdesc, (AttrNumber) 1,
"size", INT8OID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 2,
@@ -509,11 +510,19 @@ pg_stat_file(PG_FUNCTION_ARGS)
TupleDescInitEntry(tupdesc, (AttrNumber) 5,
"creation", TIMESTAMPTZOID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 6,
+ "isdir", BOOLOID, -1, 0);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 7,
"type", CHAROID, -1, 0);
BlessTupleDesc(tupdesc);
memset(nulls, false, sizeof(nulls));
values_from_stat(&fst, filename, values, nulls);
+
+ /* For pg_stat_file, keep isdir column for backward compatibility */
+ type = DatumGetChar(values[5]);
+ values[5] = BoolGetDatum(type == 'd'); /* isdir */
+ values[6] = CharGetDatum(type); /* file type */
+
tuple = heap_form_tuple(tupdesc, values, nulls);
pfree(filename);
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 346c4c577c0..998f5164170 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -6375,16 +6375,16 @@
{ oid => '2623', descr => 'get information about file',
proname => 'pg_stat_file', provolatile => 'v', prorettype => 'record',
proargtypes => 'text',
- proallargtypes => '{text,int8,timestamptz,timestamptz,timestamptz,timestamptz,char}',
- proargmodes => '{i,o,o,o,o,o,o}',
- proargnames => '{filename,size,access,modification,change,creation,type}',
+ proallargtypes => '{text,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool,char}',
+ proargmodes => '{i,o,o,o,o,o,o,o}',
+ proargnames => '{filename,size,access,modification,change,creation,isdir,type}',
prosrc => 'pg_stat_file_1arg' },
{ oid => '3307', descr => 'get information about file',
proname => 'pg_stat_file', provolatile => 'v', prorettype => 'record',
proargtypes => 'text bool',
- proallargtypes => '{text,bool,int8,timestamptz,timestamptz,timestamptz,timestamptz,char}',
- proargmodes => '{i,i,o,o,o,o,o,o}',
- proargnames => '{filename,missing_ok,size,access,modification,change,creation,type}',
+ proallargtypes => '{text,bool,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool,char}',
+ proargmodes => '{i,i,o,o,o,o,o,o,o}',
+ proargnames => '{filename,missing_ok,size,access,modification,change,creation,isdir,type}',
prosrc => 'pg_stat_file' },
{ oid => '2624', descr => 'read text from a file',
proname => 'pg_read_file', provolatile => 'v', prorettype => 'text',
diff --git a/src/test/regress/expected/misc_functions.out b/src/test/regress/expected/misc_functions.out
index 8b26b5811e4..1a11661d9fe 100644
--- a/src/test/regress/expected/misc_functions.out
+++ b/src/test/regress/expected/misc_functions.out
@@ -263,8 +263,8 @@ select pg_ls_dir('does not exist'); -- fails with missingok=false
ERROR: could not open directory "does not exist": No such file or directory
-- Check that expected columns are present
select * from pg_stat_file('.') limit 0;
- size | access | modification | change | creation | type
-------+--------+--------------+--------+----------+------
+ size | access | modification | change | creation | isdir | type
+------+--------+--------------+--------+----------+-------+------
(0 rows)
-- This tests the missing_ok parameter, which causes pg_ls_tmpdir to succeed even if the tmpdir doesn't exist yet
--
2.17.0
--Bne5rrxQd65beI7a
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v32-0011-Add-recursion-option-in-pg_ls_dir_files.patch"
^ permalink raw reply [nested|flat] 26+ messages in thread
* [PATCH v31 10/11] Preserve pg_stat_file() isdir..
@ 2020-11-29 04:29 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 26+ messages in thread
From: Justin Pryzby @ 2020-11-29 04:29 UTC (permalink / raw)
..per Tom's suggestion
---
src/backend/utils/adt/genfile.c | 15 ++++++++++++---
src/include/catalog/pg_proc.dat | 12 ++++++------
src/test/regress/expected/misc_functions.out | 4 ++--
3 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index bef20178ea..7171357f44 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/src/backend/utils/adt/genfile.c
@@ -471,11 +471,12 @@ pg_stat_file(PG_FUNCTION_ARGS)
text *filename_t = PG_GETARG_TEXT_PP(0);
char *filename;
struct stat fst;
- Datum values[6];
- bool nulls[6];
+ Datum values[7];
+ bool nulls[7];
HeapTuple tuple;
TupleDesc tupdesc;
bool missing_ok = false;
+ char type;
/* check the optional argument */
if (PG_NARGS() == 2)
@@ -497,7 +498,7 @@ pg_stat_file(PG_FUNCTION_ARGS)
* This record type had better match the output parameters declared for me
* in pg_proc.h.
*/
- tupdesc = CreateTemplateTupleDesc(6);
+ tupdesc = CreateTemplateTupleDesc(7);
TupleDescInitEntry(tupdesc, (AttrNumber) 1,
"size", INT8OID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 2,
@@ -509,11 +510,19 @@ pg_stat_file(PG_FUNCTION_ARGS)
TupleDescInitEntry(tupdesc, (AttrNumber) 5,
"creation", TIMESTAMPTZOID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 6,
+ "isdir", BOOLOID, -1, 0);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 7,
"type", CHAROID, -1, 0);
BlessTupleDesc(tupdesc);
memset(nulls, false, sizeof(nulls));
values_from_stat(&fst, filename, values, nulls);
+
+ /* For pg_stat_file, keep isdir column for backward compatibility */
+ type = DatumGetChar(values[5]);
+ values[5] = BoolGetDatum(type == 'd'); /* isdir */
+ values[6] = CharGetDatum(type); /* file type */
+
tuple = heap_form_tuple(tupdesc, values, nulls);
pfree(filename);
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 61b39094d0..4508f69f48 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -6375,16 +6375,16 @@
{ oid => '2623', descr => 'get information about file',
proname => 'pg_stat_file', provolatile => 'v', prorettype => 'record',
proargtypes => 'text',
- proallargtypes => '{text,int8,timestamptz,timestamptz,timestamptz,timestamptz,char}',
- proargmodes => '{i,o,o,o,o,o,o}',
- proargnames => '{filename,size,access,modification,change,creation,type}',
+ proallargtypes => '{text,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool,char}',
+ proargmodes => '{i,o,o,o,o,o,o,o}',
+ proargnames => '{filename,size,access,modification,change,creation,isdir,type}',
prosrc => 'pg_stat_file_1arg' },
{ oid => '3307', descr => 'get information about file',
proname => 'pg_stat_file', provolatile => 'v', prorettype => 'record',
proargtypes => 'text bool',
- proallargtypes => '{text,bool,int8,timestamptz,timestamptz,timestamptz,timestamptz,char}',
- proargmodes => '{i,i,o,o,o,o,o,o}',
- proargnames => '{filename,missing_ok,size,access,modification,change,creation,type}',
+ proallargtypes => '{text,bool,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool,char}',
+ proargmodes => '{i,i,o,o,o,o,o,o,o}',
+ proargnames => '{filename,missing_ok,size,access,modification,change,creation,isdir,type}',
prosrc => 'pg_stat_file' },
{ oid => '2624', descr => 'read text from a file',
proname => 'pg_read_file', provolatile => 'v', prorettype => 'text',
diff --git a/src/test/regress/expected/misc_functions.out b/src/test/regress/expected/misc_functions.out
index 8b26b5811e..1a11661d9f 100644
--- a/src/test/regress/expected/misc_functions.out
+++ b/src/test/regress/expected/misc_functions.out
@@ -263,8 +263,8 @@ select pg_ls_dir('does not exist'); -- fails with missingok=false
ERROR: could not open directory "does not exist": No such file or directory
-- Check that expected columns are present
select * from pg_stat_file('.') limit 0;
- size | access | modification | change | creation | type
-------+--------+--------------+--------+----------+------
+ size | access | modification | change | creation | isdir | type
+------+--------+--------------+--------+----------+-------+------
(0 rows)
-- This tests the missing_ok parameter, which causes pg_ls_tmpdir to succeed even if the tmpdir doesn't exist yet
--
2.17.0
--qZVVwWJgpX9Jzs7f
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v31-0011-Add-recursion-option-in-pg_ls_dir_files.patch"
^ permalink raw reply [nested|flat] 26+ messages in thread
* [PATCH v33 10/11] Preserve pg_stat_file() isdir..
@ 2020-11-29 04:29 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 26+ messages in thread
From: Justin Pryzby @ 2020-11-29 04:29 UTC (permalink / raw)
..per Tom's suggestion
---
src/backend/utils/adt/genfile.c | 15 ++++++++++++---
src/include/catalog/pg_proc.dat | 12 ++++++------
src/test/regress/expected/misc_functions.out | 4 ++--
3 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index 8401743efad..9f5c2bed151 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/src/backend/utils/adt/genfile.c
@@ -471,11 +471,12 @@ pg_stat_file(PG_FUNCTION_ARGS)
text *filename_t = PG_GETARG_TEXT_PP(0);
char *filename;
struct stat fst;
- Datum values[6];
- bool nulls[6];
+ Datum values[7];
+ bool nulls[7];
HeapTuple tuple;
TupleDesc tupdesc;
bool missing_ok = false;
+ char type;
/* check the optional argument */
if (PG_NARGS() == 2)
@@ -497,7 +498,7 @@ pg_stat_file(PG_FUNCTION_ARGS)
* This record type had better match the output parameters declared for me
* in pg_proc.h.
*/
- tupdesc = CreateTemplateTupleDesc(6);
+ tupdesc = CreateTemplateTupleDesc(7);
TupleDescInitEntry(tupdesc, (AttrNumber) 1,
"size", INT8OID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 2,
@@ -509,11 +510,19 @@ pg_stat_file(PG_FUNCTION_ARGS)
TupleDescInitEntry(tupdesc, (AttrNumber) 5,
"creation", TIMESTAMPTZOID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 6,
+ "isdir", BOOLOID, -1, 0);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 7,
"type", CHAROID, -1, 0);
BlessTupleDesc(tupdesc);
memset(nulls, false, sizeof(nulls));
values_from_stat(&fst, filename, values, nulls);
+
+ /* For pg_stat_file, keep isdir column for backward compatibility */
+ type = DatumGetChar(values[5]);
+ values[5] = BoolGetDatum(type == 'd'); /* isdir */
+ values[6] = CharGetDatum(type); /* file type */
+
tuple = heap_form_tuple(tupdesc, values, nulls);
pfree(filename);
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 8f4c87c724d..b81fbd04bc4 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -6375,16 +6375,16 @@
{ oid => '2623', descr => 'get information about file',
proname => 'pg_stat_file', provolatile => 'v', prorettype => 'record',
proargtypes => 'text',
- proallargtypes => '{text,int8,timestamptz,timestamptz,timestamptz,timestamptz,char}',
- proargmodes => '{i,o,o,o,o,o,o}',
- proargnames => '{filename,size,access,modification,change,creation,type}',
+ proallargtypes => '{text,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool,char}',
+ proargmodes => '{i,o,o,o,o,o,o,o}',
+ proargnames => '{filename,size,access,modification,change,creation,isdir,type}',
prosrc => 'pg_stat_file_1arg' },
{ oid => '3307', descr => 'get information about file',
proname => 'pg_stat_file', provolatile => 'v', prorettype => 'record',
proargtypes => 'text bool',
- proallargtypes => '{text,bool,int8,timestamptz,timestamptz,timestamptz,timestamptz,char}',
- proargmodes => '{i,i,o,o,o,o,o,o}',
- proargnames => '{filename,missing_ok,size,access,modification,change,creation,type}',
+ proallargtypes => '{text,bool,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool,char}',
+ proargmodes => '{i,i,o,o,o,o,o,o,o}',
+ proargnames => '{filename,missing_ok,size,access,modification,change,creation,isdir,type}',
prosrc => 'pg_stat_file' },
{ oid => '2624', descr => 'read text from a file',
proname => 'pg_read_file', provolatile => 'v', prorettype => 'text',
diff --git a/src/test/regress/expected/misc_functions.out b/src/test/regress/expected/misc_functions.out
index 6a6cec31a2e..1695bf4a3a7 100644
--- a/src/test/regress/expected/misc_functions.out
+++ b/src/test/regress/expected/misc_functions.out
@@ -305,8 +305,8 @@ select * from pg_ls_waldir() limit 0;
(0 rows)
select * from pg_stat_file('.') limit 0;
- size | access | modification | change | creation | type
-------+--------+--------------+--------+----------+------
+ size | access | modification | change | creation | isdir | type
+------+--------+--------------+--------+----------+-------+------
(0 rows)
-- This tests the missing_ok parameter, which causes pg_ls_tmpdir to succeed even if the tmpdir doesn't exist yet
--
2.17.1
--9CzcV6dAFIr7O1Ie
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v33-0011-Add-recursion-option-in-pg_ls_dir_files.patch"
^ permalink raw reply [nested|flat] 26+ messages in thread
* [PATCH v30 10/11] Preserve pg_stat_file() isdir..
@ 2020-11-29 04:29 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 26+ messages in thread
From: Justin Pryzby @ 2020-11-29 04:29 UTC (permalink / raw)
..per Tom's suggestion
---
src/backend/utils/adt/genfile.c | 15 ++++++++++++---
src/include/catalog/pg_proc.dat | 12 ++++++------
src/test/regress/expected/misc_functions.out | 4 ++--
3 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index 10bcb0b049..0f42714d90 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/src/backend/utils/adt/genfile.c
@@ -470,11 +470,12 @@ pg_stat_file(PG_FUNCTION_ARGS)
text *filename_t = PG_GETARG_TEXT_PP(0);
char *filename;
struct stat fst;
- Datum values[6];
- bool nulls[6];
+ Datum values[7];
+ bool nulls[7];
HeapTuple tuple;
TupleDesc tupdesc;
bool missing_ok = false;
+ char type;
/* check the optional argument */
if (PG_NARGS() == 2)
@@ -496,7 +497,7 @@ pg_stat_file(PG_FUNCTION_ARGS)
* This record type had better match the output parameters declared for me
* in pg_proc.h.
*/
- tupdesc = CreateTemplateTupleDesc(6);
+ tupdesc = CreateTemplateTupleDesc(7);
TupleDescInitEntry(tupdesc, (AttrNumber) 1,
"size", INT8OID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 2,
@@ -508,11 +509,19 @@ pg_stat_file(PG_FUNCTION_ARGS)
TupleDescInitEntry(tupdesc, (AttrNumber) 5,
"creation", TIMESTAMPTZOID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 6,
+ "isdir", BOOLOID, -1, 0);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 7,
"type", CHAROID, -1, 0);
BlessTupleDesc(tupdesc);
memset(nulls, false, sizeof(nulls));
values_from_stat(&fst, filename, values, nulls);
+
+ /* For pg_stat_file, keep isdir column for backward compatibility */
+ type = DatumGetChar(values[5]);
+ values[5] = BoolGetDatum(type == 'd'); /* isdir */
+ values[6] = CharGetDatum(type); /* file type */
+
tuple = heap_form_tuple(tupdesc, values, nulls);
pfree(filename);
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 176501a87a..dfe27d2e3e 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -6303,16 +6303,16 @@
{ oid => '2623', descr => 'get information about file',
proname => 'pg_stat_file', provolatile => 'v', prorettype => 'record',
proargtypes => 'text',
- proallargtypes => '{text,int8,timestamptz,timestamptz,timestamptz,timestamptz,char}',
- proargmodes => '{i,o,o,o,o,o,o}',
- proargnames => '{filename,size,access,modification,change,creation,type}',
+ proallargtypes => '{text,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool,char}',
+ proargmodes => '{i,o,o,o,o,o,o,o}',
+ proargnames => '{filename,size,access,modification,change,creation,isdir,type}',
prosrc => 'pg_stat_file_1arg' },
{ oid => '3307', descr => 'get information about file',
proname => 'pg_stat_file', provolatile => 'v', prorettype => 'record',
proargtypes => 'text bool',
- proallargtypes => '{text,bool,int8,timestamptz,timestamptz,timestamptz,timestamptz,char}',
- proargmodes => '{i,i,o,o,o,o,o,o}',
- proargnames => '{filename,missing_ok,size,access,modification,change,creation,type}',
+ proallargtypes => '{text,bool,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool,char}',
+ proargmodes => '{i,i,o,o,o,o,o,o,o}',
+ proargnames => '{filename,missing_ok,size,access,modification,change,creation,isdir,type}',
prosrc => 'pg_stat_file' },
{ oid => '2624', descr => 'read text from a file',
proname => 'pg_read_file', provolatile => 'v', prorettype => 'text',
diff --git a/src/test/regress/expected/misc_functions.out b/src/test/regress/expected/misc_functions.out
index db3d64de3b..349a549744 100644
--- a/src/test/regress/expected/misc_functions.out
+++ b/src/test/regress/expected/misc_functions.out
@@ -234,8 +234,8 @@ select pg_ls_dir('does not exist'); -- fails with missingok=false
ERROR: could not open directory "does not exist": No such file or directory
-- Check that expected columns are present
select * from pg_stat_file('.') limit 0;
- size | access | modification | change | creation | type
-------+--------+--------------+--------+----------+------
+ size | access | modification | change | creation | isdir | type
+------+--------+--------------+--------+----------+-------+------
(0 rows)
-- This tests the missing_ok parameter, which causes pg_ls_tmpdir to succeed even if the tmpdir doesn't exist yet
--
2.17.0
--ZwgA9U+XZDXt4+m+
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v30-0011-Add-recursion-option-in-pg_ls_dir_files.patch"
^ permalink raw reply [nested|flat] 26+ messages in thread
* [PATCH v25 10/11] Preserve pg_stat_file() isdir..
@ 2020-11-29 04:29 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 26+ messages in thread
From: Justin Pryzby @ 2020-11-29 04:29 UTC (permalink / raw)
..per Tom's suggestion
---
src/backend/utils/adt/genfile.c | 15 ++++++++++++---
src/include/catalog/pg_proc.dat | 12 ++++++------
src/test/regress/expected/misc_functions.out | 4 ++--
3 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index 6e8898029f..5c4a7f748d 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/src/backend/utils/adt/genfile.c
@@ -468,11 +468,12 @@ pg_stat_file(PG_FUNCTION_ARGS)
text *filename_t = PG_GETARG_TEXT_PP(0);
char *filename;
struct stat fst;
- Datum values[6];
- bool nulls[6];
+ Datum values[7];
+ bool nulls[7];
HeapTuple tuple;
TupleDesc tupdesc;
bool missing_ok = false;
+ char type;
/* check the optional argument */
if (PG_NARGS() == 2)
@@ -494,7 +495,7 @@ pg_stat_file(PG_FUNCTION_ARGS)
* This record type had better match the output parameters declared for me
* in pg_proc.h.
*/
- tupdesc = CreateTemplateTupleDesc(6);
+ tupdesc = CreateTemplateTupleDesc(7);
TupleDescInitEntry(tupdesc, (AttrNumber) 1,
"size", INT8OID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 2,
@@ -506,11 +507,19 @@ pg_stat_file(PG_FUNCTION_ARGS)
TupleDescInitEntry(tupdesc, (AttrNumber) 5,
"creation", TIMESTAMPTZOID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 6,
+ "isdir", BOOLOID, -1, 0);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 7,
"type", CHAROID, -1, 0);
BlessTupleDesc(tupdesc);
memset(nulls, false, sizeof(nulls));
tuple_from_stat(&fst, filename, values, nulls);
+
+ /* For pg_stat_file, keep isdir column for backward compatibility */
+ type = DatumGetChar(values[5]);
+ values[5] = BoolGetDatum(type == 'd'); /* isdir */
+ values[6] = CharGetDatum(type); /* file type */
+
tuple = heap_form_tuple(tupdesc, values, nulls);
pfree(filename);
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 414b087756..a75929cb92 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -6203,16 +6203,16 @@
{ oid => '2623', descr => 'get information about file',
proname => 'pg_stat_file', provolatile => 'v', prorettype => 'record',
proargtypes => 'text',
- proallargtypes => '{text,int8,timestamptz,timestamptz,timestamptz,timestamptz,char}',
- proargmodes => '{i,o,o,o,o,o,o}',
- proargnames => '{filename,size,access,modification,change,creation,type}',
+ proallargtypes => '{text,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool,char}',
+ proargmodes => '{i,o,o,o,o,o,o,o}',
+ proargnames => '{filename,size,access,modification,change,creation,isdir,type}',
prosrc => 'pg_stat_file_1arg' },
{ oid => '3307', descr => 'get information about file',
proname => 'pg_stat_file', provolatile => 'v', prorettype => 'record',
proargtypes => 'text bool',
- proallargtypes => '{text,bool,int8,timestamptz,timestamptz,timestamptz,timestamptz,char}',
- proargmodes => '{i,i,o,o,o,o,o,o}',
- proargnames => '{filename,missing_ok,size,access,modification,change,creation,type}',
+ proallargtypes => '{text,bool,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool,char}',
+ proargmodes => '{i,i,o,o,o,o,o,o,o}',
+ proargnames => '{filename,missing_ok,size,access,modification,change,creation,isdir,type}',
prosrc => 'pg_stat_file' },
{ oid => '2624', descr => 'read text from a file',
proname => 'pg_read_file', provolatile => 'v', prorettype => 'text',
diff --git a/src/test/regress/expected/misc_functions.out b/src/test/regress/expected/misc_functions.out
index 5b87ab4844..e1e9ced303 100644
--- a/src/test/regress/expected/misc_functions.out
+++ b/src/test/regress/expected/misc_functions.out
@@ -221,8 +221,8 @@ select pg_ls_dir('does not exist'); -- fails with missingok=false
ERROR: could not open directory "does not exist": No such file or directory
-- Check that expected columns are present
select * from pg_stat_file('.') limit 0;
- size | access | modification | change | creation | type
-------+--------+--------------+--------+----------+------
+ size | access | modification | change | creation | isdir | type
+------+--------+--------------+--------+----------+-------+------
(0 rows)
-- This tests the missing_ok parameter, which causes pg_ls_tmpdir to succeed even if the tmpdir doesn't exist yet
--
2.17.0
--SBikYMzjhZGK9d4p
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v25-0011-Add-recursion-option-in-pg_ls_dir_files.patch"
^ permalink raw reply [nested|flat] 26+ messages in thread
* [PATCH v26 10/11] Preserve pg_stat_file() isdir..
@ 2020-11-29 04:29 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 26+ messages in thread
From: Justin Pryzby @ 2020-11-29 04:29 UTC (permalink / raw)
..per Tom's suggestion
---
src/backend/utils/adt/genfile.c | 15 ++++++++++++---
src/include/catalog/pg_proc.dat | 12 ++++++------
src/test/regress/expected/misc_functions.out | 4 ++--
3 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index 0855f5e88e..2ec44863b8 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/src/backend/utils/adt/genfile.c
@@ -468,11 +468,12 @@ pg_stat_file(PG_FUNCTION_ARGS)
text *filename_t = PG_GETARG_TEXT_PP(0);
char *filename;
struct stat fst;
- Datum values[6];
- bool nulls[6];
+ Datum values[7];
+ bool nulls[7];
HeapTuple tuple;
TupleDesc tupdesc;
bool missing_ok = false;
+ char type;
/* check the optional argument */
if (PG_NARGS() == 2)
@@ -494,7 +495,7 @@ pg_stat_file(PG_FUNCTION_ARGS)
* This record type had better match the output parameters declared for me
* in pg_proc.h.
*/
- tupdesc = CreateTemplateTupleDesc(6);
+ tupdesc = CreateTemplateTupleDesc(7);
TupleDescInitEntry(tupdesc, (AttrNumber) 1,
"size", INT8OID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 2,
@@ -506,11 +507,19 @@ pg_stat_file(PG_FUNCTION_ARGS)
TupleDescInitEntry(tupdesc, (AttrNumber) 5,
"creation", TIMESTAMPTZOID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 6,
+ "isdir", BOOLOID, -1, 0);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 7,
"type", CHAROID, -1, 0);
BlessTupleDesc(tupdesc);
memset(nulls, false, sizeof(nulls));
values_from_stat(&fst, filename, values, nulls);
+
+ /* For pg_stat_file, keep isdir column for backward compatibility */
+ type = DatumGetChar(values[5]);
+ values[5] = BoolGetDatum(type == 'd'); /* isdir */
+ values[6] = CharGetDatum(type); /* file type */
+
tuple = heap_form_tuple(tupdesc, values, nulls);
pfree(filename);
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 5d80cfa6c1..ee0c5eccec 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -6203,16 +6203,16 @@
{ oid => '2623', descr => 'get information about file',
proname => 'pg_stat_file', provolatile => 'v', prorettype => 'record',
proargtypes => 'text',
- proallargtypes => '{text,int8,timestamptz,timestamptz,timestamptz,timestamptz,char}',
- proargmodes => '{i,o,o,o,o,o,o}',
- proargnames => '{filename,size,access,modification,change,creation,type}',
+ proallargtypes => '{text,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool,char}',
+ proargmodes => '{i,o,o,o,o,o,o,o}',
+ proargnames => '{filename,size,access,modification,change,creation,isdir,type}',
prosrc => 'pg_stat_file_1arg' },
{ oid => '3307', descr => 'get information about file',
proname => 'pg_stat_file', provolatile => 'v', prorettype => 'record',
proargtypes => 'text bool',
- proallargtypes => '{text,bool,int8,timestamptz,timestamptz,timestamptz,timestamptz,char}',
- proargmodes => '{i,i,o,o,o,o,o,o}',
- proargnames => '{filename,missing_ok,size,access,modification,change,creation,type}',
+ proallargtypes => '{text,bool,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool,char}',
+ proargmodes => '{i,i,o,o,o,o,o,o,o}',
+ proargnames => '{filename,missing_ok,size,access,modification,change,creation,isdir,type}',
prosrc => 'pg_stat_file' },
{ oid => '2624', descr => 'read text from a file',
proname => 'pg_read_file', provolatile => 'v', prorettype => 'text',
diff --git a/src/test/regress/expected/misc_functions.out b/src/test/regress/expected/misc_functions.out
index 5b87ab4844..e1e9ced303 100644
--- a/src/test/regress/expected/misc_functions.out
+++ b/src/test/regress/expected/misc_functions.out
@@ -221,8 +221,8 @@ select pg_ls_dir('does not exist'); -- fails with missingok=false
ERROR: could not open directory "does not exist": No such file or directory
-- Check that expected columns are present
select * from pg_stat_file('.') limit 0;
- size | access | modification | change | creation | type
-------+--------+--------------+--------+----------+------
+ size | access | modification | change | creation | isdir | type
+------+--------+--------------+--------+----------+-------+------
(0 rows)
-- This tests the missing_ok parameter, which causes pg_ls_tmpdir to succeed even if the tmpdir doesn't exist yet
--
2.17.0
--yKkOmjQZXRsvHRX8
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v26-0011-Add-recursion-option-in-pg_ls_dir_files.patch"
^ permalink raw reply [nested|flat] 26+ messages in thread
* [PATCH v27 10/11] Preserve pg_stat_file() isdir..
@ 2020-11-29 04:29 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 26+ messages in thread
From: Justin Pryzby @ 2020-11-29 04:29 UTC (permalink / raw)
..per Tom's suggestion
---
src/backend/utils/adt/genfile.c | 15 ++++++++++++---
src/include/catalog/pg_proc.dat | 12 ++++++------
src/test/regress/expected/misc_functions.out | 4 ++--
3 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index af666d658b..ff31e38484 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/src/backend/utils/adt/genfile.c
@@ -471,11 +471,12 @@ pg_stat_file(PG_FUNCTION_ARGS)
text *filename_t = PG_GETARG_TEXT_PP(0);
char *filename;
struct stat fst;
- Datum values[6];
- bool nulls[6];
+ Datum values[7];
+ bool nulls[7];
HeapTuple tuple;
TupleDesc tupdesc;
bool missing_ok = false;
+ char type;
/* check the optional argument */
if (PG_NARGS() == 2)
@@ -497,7 +498,7 @@ pg_stat_file(PG_FUNCTION_ARGS)
* This record type had better match the output parameters declared for me
* in pg_proc.h.
*/
- tupdesc = CreateTemplateTupleDesc(6);
+ tupdesc = CreateTemplateTupleDesc(7);
TupleDescInitEntry(tupdesc, (AttrNumber) 1,
"size", INT8OID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 2,
@@ -509,11 +510,19 @@ pg_stat_file(PG_FUNCTION_ARGS)
TupleDescInitEntry(tupdesc, (AttrNumber) 5,
"creation", TIMESTAMPTZOID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 6,
+ "isdir", BOOLOID, -1, 0);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 7,
"type", CHAROID, -1, 0);
BlessTupleDesc(tupdesc);
memset(nulls, false, sizeof(nulls));
values_from_stat(&fst, filename, values, nulls);
+
+ /* For pg_stat_file, keep isdir column for backward compatibility */
+ type = DatumGetChar(values[5]);
+ values[5] = BoolGetDatum(type == 'd'); /* isdir */
+ values[6] = CharGetDatum(type); /* file type */
+
tuple = heap_form_tuple(tupdesc, values, nulls);
pfree(filename);
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 0a9d1e46bd..89ce701654 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -6302,16 +6302,16 @@
{ oid => '2623', descr => 'get information about file',
proname => 'pg_stat_file', provolatile => 'v', prorettype => 'record',
proargtypes => 'text',
- proallargtypes => '{text,int8,timestamptz,timestamptz,timestamptz,timestamptz,char}',
- proargmodes => '{i,o,o,o,o,o,o}',
- proargnames => '{filename,size,access,modification,change,creation,type}',
+ proallargtypes => '{text,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool,char}',
+ proargmodes => '{i,o,o,o,o,o,o,o}',
+ proargnames => '{filename,size,access,modification,change,creation,isdir,type}',
prosrc => 'pg_stat_file_1arg' },
{ oid => '3307', descr => 'get information about file',
proname => 'pg_stat_file', provolatile => 'v', prorettype => 'record',
proargtypes => 'text bool',
- proallargtypes => '{text,bool,int8,timestamptz,timestamptz,timestamptz,timestamptz,char}',
- proargmodes => '{i,i,o,o,o,o,o,o}',
- proargnames => '{filename,missing_ok,size,access,modification,change,creation,type}',
+ proallargtypes => '{text,bool,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool,char}',
+ proargmodes => '{i,i,o,o,o,o,o,o,o}',
+ proargnames => '{filename,missing_ok,size,access,modification,change,creation,isdir,type}',
prosrc => 'pg_stat_file' },
{ oid => '2624', descr => 'read text from a file',
proname => 'pg_read_file', provolatile => 'v', prorettype => 'text',
diff --git a/src/test/regress/expected/misc_functions.out b/src/test/regress/expected/misc_functions.out
index db3d64de3b..349a549744 100644
--- a/src/test/regress/expected/misc_functions.out
+++ b/src/test/regress/expected/misc_functions.out
@@ -234,8 +234,8 @@ select pg_ls_dir('does not exist'); -- fails with missingok=false
ERROR: could not open directory "does not exist": No such file or directory
-- Check that expected columns are present
select * from pg_stat_file('.') limit 0;
- size | access | modification | change | creation | type
-------+--------+--------------+--------+----------+------
+ size | access | modification | change | creation | isdir | type
+------+--------+--------------+--------+----------+-------+------
(0 rows)
-- This tests the missing_ok parameter, which causes pg_ls_tmpdir to succeed even if the tmpdir doesn't exist yet
--
2.17.0
--19uQFt6ulqmgNgg1
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v27-0011-Add-recursion-option-in-pg_ls_dir_files.patch"
^ permalink raw reply [nested|flat] 26+ messages in thread
* [PATCH v28 10/11] Preserve pg_stat_file() isdir..
@ 2020-11-29 04:29 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 26+ messages in thread
From: Justin Pryzby @ 2020-11-29 04:29 UTC (permalink / raw)
..per Tom's suggestion
---
src/backend/utils/adt/genfile.c | 15 ++++++++++++---
src/include/catalog/pg_proc.dat | 12 ++++++------
src/test/regress/expected/misc_functions.out | 4 ++--
3 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index af666d658b..ff31e38484 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/src/backend/utils/adt/genfile.c
@@ -471,11 +471,12 @@ pg_stat_file(PG_FUNCTION_ARGS)
text *filename_t = PG_GETARG_TEXT_PP(0);
char *filename;
struct stat fst;
- Datum values[6];
- bool nulls[6];
+ Datum values[7];
+ bool nulls[7];
HeapTuple tuple;
TupleDesc tupdesc;
bool missing_ok = false;
+ char type;
/* check the optional argument */
if (PG_NARGS() == 2)
@@ -497,7 +498,7 @@ pg_stat_file(PG_FUNCTION_ARGS)
* This record type had better match the output parameters declared for me
* in pg_proc.h.
*/
- tupdesc = CreateTemplateTupleDesc(6);
+ tupdesc = CreateTemplateTupleDesc(7);
TupleDescInitEntry(tupdesc, (AttrNumber) 1,
"size", INT8OID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 2,
@@ -509,11 +510,19 @@ pg_stat_file(PG_FUNCTION_ARGS)
TupleDescInitEntry(tupdesc, (AttrNumber) 5,
"creation", TIMESTAMPTZOID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 6,
+ "isdir", BOOLOID, -1, 0);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 7,
"type", CHAROID, -1, 0);
BlessTupleDesc(tupdesc);
memset(nulls, false, sizeof(nulls));
values_from_stat(&fst, filename, values, nulls);
+
+ /* For pg_stat_file, keep isdir column for backward compatibility */
+ type = DatumGetChar(values[5]);
+ values[5] = BoolGetDatum(type == 'd'); /* isdir */
+ values[6] = CharGetDatum(type); /* file type */
+
tuple = heap_form_tuple(tupdesc, values, nulls);
pfree(filename);
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 2fc1793b5a..a768176032 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -6319,16 +6319,16 @@
{ oid => '2623', descr => 'get information about file',
proname => 'pg_stat_file', provolatile => 'v', prorettype => 'record',
proargtypes => 'text',
- proallargtypes => '{text,int8,timestamptz,timestamptz,timestamptz,timestamptz,char}',
- proargmodes => '{i,o,o,o,o,o,o}',
- proargnames => '{filename,size,access,modification,change,creation,type}',
+ proallargtypes => '{text,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool,char}',
+ proargmodes => '{i,o,o,o,o,o,o,o}',
+ proargnames => '{filename,size,access,modification,change,creation,isdir,type}',
prosrc => 'pg_stat_file_1arg' },
{ oid => '3307', descr => 'get information about file',
proname => 'pg_stat_file', provolatile => 'v', prorettype => 'record',
proargtypes => 'text bool',
- proallargtypes => '{text,bool,int8,timestamptz,timestamptz,timestamptz,timestamptz,char}',
- proargmodes => '{i,i,o,o,o,o,o,o}',
- proargnames => '{filename,missing_ok,size,access,modification,change,creation,type}',
+ proallargtypes => '{text,bool,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool,char}',
+ proargmodes => '{i,i,o,o,o,o,o,o,o}',
+ proargnames => '{filename,missing_ok,size,access,modification,change,creation,isdir,type}',
prosrc => 'pg_stat_file' },
{ oid => '2624', descr => 'read text from a file',
proname => 'pg_read_file', provolatile => 'v', prorettype => 'text',
diff --git a/src/test/regress/expected/misc_functions.out b/src/test/regress/expected/misc_functions.out
index db3d64de3b..349a549744 100644
--- a/src/test/regress/expected/misc_functions.out
+++ b/src/test/regress/expected/misc_functions.out
@@ -234,8 +234,8 @@ select pg_ls_dir('does not exist'); -- fails with missingok=false
ERROR: could not open directory "does not exist": No such file or directory
-- Check that expected columns are present
select * from pg_stat_file('.') limit 0;
- size | access | modification | change | creation | type
-------+--------+--------------+--------+----------+------
+ size | access | modification | change | creation | isdir | type
+------+--------+--------------+--------+----------+-------+------
(0 rows)
-- This tests the missing_ok parameter, which causes pg_ls_tmpdir to succeed even if the tmpdir doesn't exist yet
--
2.17.0
--vk/v8fjDPiDepTtA
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v28-0011-Add-recursion-option-in-pg_ls_dir_files.patch"
^ permalink raw reply [nested|flat] 26+ messages in thread
* [PATCH v24 10/11] Preserve pg_stat_file() isdir..
@ 2020-11-29 04:29 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 26+ messages in thread
From: Justin Pryzby @ 2020-11-29 04:29 UTC (permalink / raw)
..per Tom's suggestion
---
src/backend/utils/adt/genfile.c | 15 ++++++++++++---
src/include/catalog/pg_proc.dat | 12 ++++++------
src/test/regress/expected/misc_functions.out | 4 ++--
3 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index 6e8898029f..5c4a7f748d 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/src/backend/utils/adt/genfile.c
@@ -468,11 +468,12 @@ pg_stat_file(PG_FUNCTION_ARGS)
text *filename_t = PG_GETARG_TEXT_PP(0);
char *filename;
struct stat fst;
- Datum values[6];
- bool nulls[6];
+ Datum values[7];
+ bool nulls[7];
HeapTuple tuple;
TupleDesc tupdesc;
bool missing_ok = false;
+ char type;
/* check the optional argument */
if (PG_NARGS() == 2)
@@ -494,7 +495,7 @@ pg_stat_file(PG_FUNCTION_ARGS)
* This record type had better match the output parameters declared for me
* in pg_proc.h.
*/
- tupdesc = CreateTemplateTupleDesc(6);
+ tupdesc = CreateTemplateTupleDesc(7);
TupleDescInitEntry(tupdesc, (AttrNumber) 1,
"size", INT8OID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 2,
@@ -506,11 +507,19 @@ pg_stat_file(PG_FUNCTION_ARGS)
TupleDescInitEntry(tupdesc, (AttrNumber) 5,
"creation", TIMESTAMPTZOID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 6,
+ "isdir", BOOLOID, -1, 0);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 7,
"type", CHAROID, -1, 0);
BlessTupleDesc(tupdesc);
memset(nulls, false, sizeof(nulls));
tuple_from_stat(&fst, filename, values, nulls);
+
+ /* For pg_stat_file, keep isdir column for backward compatibility */
+ type = DatumGetChar(values[5]);
+ values[5] = BoolGetDatum(type == 'd'); /* isdir */
+ values[6] = CharGetDatum(type); /* file type */
+
tuple = heap_form_tuple(tupdesc, values, nulls);
pfree(filename);
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 12a7391e73..d472afcf71 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -6202,16 +6202,16 @@
{ oid => '2623', descr => 'get information about file',
proname => 'pg_stat_file', provolatile => 'v', prorettype => 'record',
proargtypes => 'text',
- proallargtypes => '{text,int8,timestamptz,timestamptz,timestamptz,timestamptz,char}',
- proargmodes => '{i,o,o,o,o,o,o}',
- proargnames => '{filename,size,access,modification,change,creation,type}',
+ proallargtypes => '{text,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool,char}',
+ proargmodes => '{i,o,o,o,o,o,o,o}',
+ proargnames => '{filename,size,access,modification,change,creation,isdir,type}',
prosrc => 'pg_stat_file_1arg' },
{ oid => '3307', descr => 'get information about file',
proname => 'pg_stat_file', provolatile => 'v', prorettype => 'record',
proargtypes => 'text bool',
- proallargtypes => '{text,bool,int8,timestamptz,timestamptz,timestamptz,timestamptz,char}',
- proargmodes => '{i,i,o,o,o,o,o,o}',
- proargnames => '{filename,missing_ok,size,access,modification,change,creation,type}',
+ proallargtypes => '{text,bool,int8,timestamptz,timestamptz,timestamptz,timestamptz,bool,char}',
+ proargmodes => '{i,i,o,o,o,o,o,o,o}',
+ proargnames => '{filename,missing_ok,size,access,modification,change,creation,isdir,type}',
prosrc => 'pg_stat_file' },
{ oid => '2624', descr => 'read text from a file',
proname => 'pg_read_file', provolatile => 'v', prorettype => 'text',
diff --git a/src/test/regress/expected/misc_functions.out b/src/test/regress/expected/misc_functions.out
index 5b87ab4844..e1e9ced303 100644
--- a/src/test/regress/expected/misc_functions.out
+++ b/src/test/regress/expected/misc_functions.out
@@ -221,8 +221,8 @@ select pg_ls_dir('does not exist'); -- fails with missingok=false
ERROR: could not open directory "does not exist": No such file or directory
-- Check that expected columns are present
select * from pg_stat_file('.') limit 0;
- size | access | modification | change | creation | type
-------+--------+--------------+--------+----------+------
+ size | access | modification | change | creation | isdir | type
+------+--------+--------------+--------+----------+-------+------
(0 rows)
-- This tests the missing_ok parameter, which causes pg_ls_tmpdir to succeed even if the tmpdir doesn't exist yet
--
2.17.0
--mPTHnM80CEnHQ2WJ
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v24-0011-Add-recursion-option-in-pg_ls_dir_files.patch"
^ permalink raw reply [nested|flat] 26+ messages in thread
* Readd use of TAP subtests
@ 2021-12-08 13:26 Peter Eisentraut <[email protected]>
0 siblings, 2 replies; 26+ messages in thread
From: Peter Eisentraut @ 2021-12-08 13:26 UTC (permalink / raw)
To: pgsql-hackers
Now that subtests in TAP are supported again, I want to correct the
great historical injustice of 7912f9b7dc9e2d3f6cd81892ef6aa797578e9f06
and put those subtests back.
Much more work like this is possible, of course. I just wanted to get
this out of the way since the code was already written and I've had this
on my list for, uh, 7 years.
From d1e2df592b7eaa7952d621266d5ad5d6b80e503a Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Wed, 8 Dec 2021 14:20:57 +0100
Subject: [PATCH] Readd use of TAP subtests
Since 405f32fc49609eb94fa39e7b5e7c1fe2bb2b73aa, Test::More must be new
enough to support subtests.
The present patch effectively reverts
7912f9b7dc9e2d3f6cd81892ef6aa797578e9f06. Many more refactorings like
this are possible; this is just to get started.
---
contrib/oid2name/t/001_basic.pl | 2 +-
contrib/vacuumlo/t/001_basic.pl | 2 +-
src/bin/initdb/t/001_initdb.pl | 2 +-
src/bin/pg_amcheck/t/001_basic.pl | 2 +-
src/bin/pg_amcheck/t/005_opclass_damage.pl | 2 +-
.../t/010_pg_archivecleanup.pl | 2 +-
src/bin/pg_basebackup/t/010_pg_basebackup.pl | 2 +-
src/bin/pg_basebackup/t/020_pg_receivewal.pl | 2 +-
src/bin/pg_basebackup/t/030_pg_recvlogical.pl | 2 +-
src/bin/pg_checksums/t/001_basic.pl | 2 +-
src/bin/pg_checksums/t/002_actions.pl | 2 +-
src/bin/pg_config/t/001_pg_config.pl | 2 +-
.../pg_controldata/t/001_pg_controldata.pl | 2 +-
src/bin/pg_ctl/t/001_start_stop.pl | 2 +-
src/bin/pg_dump/t/001_basic.pl | 2 +-
src/bin/pg_resetwal/t/001_basic.pl | 2 +-
src/bin/pg_rewind/t/006_options.pl | 2 +-
src/bin/pg_test_fsync/t/001_basic.pl | 2 +-
src/bin/pg_test_timing/t/001_basic.pl | 2 +-
src/bin/pg_verifybackup/t/001_basic.pl | 2 +-
src/bin/pg_verifybackup/t/004_options.pl | 2 +-
src/bin/pg_verifybackup/t/006_encoding.pl | 2 +-
src/bin/pg_waldump/t/001_basic.pl | 2 +-
src/bin/psql/t/001_basic.pl | 2 +-
src/bin/scripts/t/010_clusterdb.pl | 2 +-
src/bin/scripts/t/011_clusterdb_all.pl | 2 +-
src/bin/scripts/t/020_createdb.pl | 2 +-
src/bin/scripts/t/040_createuser.pl | 2 +-
src/bin/scripts/t/050_dropdb.pl | 2 +-
src/bin/scripts/t/070_dropuser.pl | 2 +-
src/bin/scripts/t/080_pg_isready.pl | 2 +-
src/bin/scripts/t/090_reindexdb.pl | 2 +-
src/bin/scripts/t/091_reindexdb_all.pl | 2 +-
src/bin/scripts/t/100_vacuumdb.pl | 2 +-
src/bin/scripts/t/101_vacuumdb_all.pl | 2 +-
src/bin/scripts/t/102_vacuumdb_stages.pl | 2 +-
src/test/perl/PostgreSQL/Test/Cluster.pm | 16 +++--
src/test/perl/PostgreSQL/Test/Utils.pm | 68 +++++++++++--------
src/test/ssl/t/001_ssltests.pl | 2 +-
39 files changed, 87 insertions(+), 71 deletions(-)
diff --git a/contrib/oid2name/t/001_basic.pl b/contrib/oid2name/t/001_basic.pl
index efedba0aa1..9d1d1e3533 100644
--- a/contrib/oid2name/t/001_basic.pl
+++ b/contrib/oid2name/t/001_basic.pl
@@ -5,7 +5,7 @@
use warnings;
use PostgreSQL::Test::Utils;
-use Test::More tests => 8;
+use Test::More tests => 3;
#########################################
# Basic checks
diff --git a/contrib/vacuumlo/t/001_basic.pl b/contrib/vacuumlo/t/001_basic.pl
index 951dad0d47..8acb4a9317 100644
--- a/contrib/vacuumlo/t/001_basic.pl
+++ b/contrib/vacuumlo/t/001_basic.pl
@@ -5,7 +5,7 @@
use warnings;
use PostgreSQL::Test::Utils;
-use Test::More tests => 8;
+use Test::More tests => 3;
program_help_ok('vacuumlo');
program_version_ok('vacuumlo');
diff --git a/src/bin/initdb/t/001_initdb.pl b/src/bin/initdb/t/001_initdb.pl
index 6796d8520e..c621df8270 100644
--- a/src/bin/initdb/t/001_initdb.pl
+++ b/src/bin/initdb/t/001_initdb.pl
@@ -11,7 +11,7 @@
use File::stat qw{lstat};
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 22;
+use Test::More tests => 15;
my $tempdir = PostgreSQL::Test::Utils::tempdir;
my $xlogdir = "$tempdir/pgxlog";
diff --git a/src/bin/pg_amcheck/t/001_basic.pl b/src/bin/pg_amcheck/t/001_basic.pl
index d44fe60a4c..3c5052b3a6 100644
--- a/src/bin/pg_amcheck/t/001_basic.pl
+++ b/src/bin/pg_amcheck/t/001_basic.pl
@@ -5,7 +5,7 @@
use warnings;
use PostgreSQL::Test::Utils;
-use Test::More tests => 8;
+use Test::More tests => 3;
program_help_ok('pg_amcheck');
program_version_ok('pg_amcheck');
diff --git a/src/bin/pg_amcheck/t/005_opclass_damage.pl b/src/bin/pg_amcheck/t/005_opclass_damage.pl
index 2f86f4f2a4..d045e81e65 100644
--- a/src/bin/pg_amcheck/t/005_opclass_damage.pl
+++ b/src/bin/pg_amcheck/t/005_opclass_damage.pl
@@ -8,7 +8,7 @@
use warnings;
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 5;
+use Test::More tests => 3;
my $node = PostgreSQL::Test::Cluster->new('test');
$node->init;
diff --git a/src/bin/pg_archivecleanup/t/010_pg_archivecleanup.pl b/src/bin/pg_archivecleanup/t/010_pg_archivecleanup.pl
index 6b3f486cfa..0531da1115 100644
--- a/src/bin/pg_archivecleanup/t/010_pg_archivecleanup.pl
+++ b/src/bin/pg_archivecleanup/t/010_pg_archivecleanup.pl
@@ -4,7 +4,7 @@
use strict;
use warnings;
use PostgreSQL::Test::Utils;
-use Test::More tests => 42;
+use Test::More tests => 37;
program_help_ok('pg_archivecleanup');
program_version_ok('pg_archivecleanup');
diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index 89f45b77a3..c7bd78d988 100644
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -10,7 +10,7 @@
use Fcntl qw(:seek);
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 110;
+use Test::More tests => 105;
program_help_ok('pg_basebackup');
program_version_ok('pg_basebackup');
diff --git a/src/bin/pg_basebackup/t/020_pg_receivewal.pl b/src/bin/pg_basebackup/t/020_pg_receivewal.pl
index 43599d832b..54cee6248d 100644
--- a/src/bin/pg_basebackup/t/020_pg_receivewal.pl
+++ b/src/bin/pg_basebackup/t/020_pg_receivewal.pl
@@ -5,7 +5,7 @@
use warnings;
use PostgreSQL::Test::Utils;
use PostgreSQL::Test::Cluster;
-use Test::More tests => 42;
+use Test::More tests => 37;
program_help_ok('pg_receivewal');
program_version_ok('pg_receivewal');
diff --git a/src/bin/pg_basebackup/t/030_pg_recvlogical.pl b/src/bin/pg_basebackup/t/030_pg_recvlogical.pl
index 90da1662e3..ab566b116d 100644
--- a/src/bin/pg_basebackup/t/030_pg_recvlogical.pl
+++ b/src/bin/pg_basebackup/t/030_pg_recvlogical.pl
@@ -5,7 +5,7 @@
use warnings;
use PostgreSQL::Test::Utils;
use PostgreSQL::Test::Cluster;
-use Test::More tests => 20;
+use Test::More tests => 15;
program_help_ok('pg_recvlogical');
program_version_ok('pg_recvlogical');
diff --git a/src/bin/pg_checksums/t/001_basic.pl b/src/bin/pg_checksums/t/001_basic.pl
index e9eb3197a6..507ffacef0 100644
--- a/src/bin/pg_checksums/t/001_basic.pl
+++ b/src/bin/pg_checksums/t/001_basic.pl
@@ -4,7 +4,7 @@
use strict;
use warnings;
use PostgreSQL::Test::Utils;
-use Test::More tests => 8;
+use Test::More tests => 3;
program_help_ok('pg_checksums');
program_version_ok('pg_checksums');
diff --git a/src/bin/pg_checksums/t/002_actions.pl b/src/bin/pg_checksums/t/002_actions.pl
index 20a5f27840..f34245108d 100644
--- a/src/bin/pg_checksums/t/002_actions.pl
+++ b/src/bin/pg_checksums/t/002_actions.pl
@@ -11,7 +11,7 @@
use PostgreSQL::Test::Utils;
use Fcntl qw(:seek);
-use Test::More tests => 66;
+use Test::More tests => 58;
# Utility routine to create and check a table with corrupted checksums
diff --git a/src/bin/pg_config/t/001_pg_config.pl b/src/bin/pg_config/t/001_pg_config.pl
index 6c7f9b8602..ebf6aacc4a 100644
--- a/src/bin/pg_config/t/001_pg_config.pl
+++ b/src/bin/pg_config/t/001_pg_config.pl
@@ -4,7 +4,7 @@
use strict;
use warnings;
use PostgreSQL::Test::Utils;
-use Test::More tests => 20;
+use Test::More tests => 7;
program_help_ok('pg_config');
program_version_ok('pg_config');
diff --git a/src/bin/pg_controldata/t/001_pg_controldata.pl b/src/bin/pg_controldata/t/001_pg_controldata.pl
index ad7bacace5..fd3ecfaed7 100644
--- a/src/bin/pg_controldata/t/001_pg_controldata.pl
+++ b/src/bin/pg_controldata/t/001_pg_controldata.pl
@@ -5,7 +5,7 @@
use warnings;
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 17;
+use Test::More tests => 10;
program_help_ok('pg_controldata');
program_version_ok('pg_controldata');
diff --git a/src/bin/pg_ctl/t/001_start_stop.pl b/src/bin/pg_ctl/t/001_start_stop.pl
index f95352bf94..9fb675a079 100644
--- a/src/bin/pg_ctl/t/001_start_stop.pl
+++ b/src/bin/pg_ctl/t/001_start_stop.pl
@@ -9,7 +9,7 @@
use File::stat qw{lstat};
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 24;
+use Test::More tests => 17;
my $tempdir = PostgreSQL::Test::Utils::tempdir;
my $tempdir_short = PostgreSQL::Test::Utils::tempdir_short;
diff --git a/src/bin/pg_dump/t/001_basic.pl b/src/bin/pg_dump/t/001_basic.pl
index 863f4da3d8..d31a72a621 100644
--- a/src/bin/pg_dump/t/001_basic.pl
+++ b/src/bin/pg_dump/t/001_basic.pl
@@ -7,7 +7,7 @@
use Config;
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 82;
+use Test::More tests => 67;
my $tempdir = PostgreSQL::Test::Utils::tempdir;
diff --git a/src/bin/pg_resetwal/t/001_basic.pl b/src/bin/pg_resetwal/t/001_basic.pl
index 0f86aea68e..14aa98de40 100644
--- a/src/bin/pg_resetwal/t/001_basic.pl
+++ b/src/bin/pg_resetwal/t/001_basic.pl
@@ -6,7 +6,7 @@
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 12;
+use Test::More tests => 5;
program_help_ok('pg_resetwal');
program_version_ok('pg_resetwal');
diff --git a/src/bin/pg_rewind/t/006_options.pl b/src/bin/pg_rewind/t/006_options.pl
index 30c7bb46d2..ff08c80976 100644
--- a/src/bin/pg_rewind/t/006_options.pl
+++ b/src/bin/pg_rewind/t/006_options.pl
@@ -7,7 +7,7 @@
use strict;
use warnings;
use PostgreSQL::Test::Utils;
-use Test::More tests => 12;
+use Test::More tests => 7;
program_help_ok('pg_rewind');
program_version_ok('pg_rewind');
diff --git a/src/bin/pg_test_fsync/t/001_basic.pl b/src/bin/pg_test_fsync/t/001_basic.pl
index 8c71f1111e..973506238a 100644
--- a/src/bin/pg_test_fsync/t/001_basic.pl
+++ b/src/bin/pg_test_fsync/t/001_basic.pl
@@ -6,7 +6,7 @@
use Config;
use PostgreSQL::Test::Utils;
-use Test::More tests => 12;
+use Test::More tests => 7;
#########################################
# Basic checks
diff --git a/src/bin/pg_test_timing/t/001_basic.pl b/src/bin/pg_test_timing/t/001_basic.pl
index 3e58926c96..1d95e2a6a6 100644
--- a/src/bin/pg_test_timing/t/001_basic.pl
+++ b/src/bin/pg_test_timing/t/001_basic.pl
@@ -6,7 +6,7 @@
use Config;
use PostgreSQL::Test::Utils;
-use Test::More tests => 12;
+use Test::More tests => 7;
#########################################
# Basic checks
diff --git a/src/bin/pg_verifybackup/t/001_basic.pl b/src/bin/pg_verifybackup/t/001_basic.pl
index 33d6b38d33..9163f3e1b1 100644
--- a/src/bin/pg_verifybackup/t/001_basic.pl
+++ b/src/bin/pg_verifybackup/t/001_basic.pl
@@ -4,7 +4,7 @@
use strict;
use warnings;
use PostgreSQL::Test::Utils;
-use Test::More tests => 16;
+use Test::More tests => 11;
my $tempdir = PostgreSQL::Test::Utils::tempdir;
diff --git a/src/bin/pg_verifybackup/t/004_options.pl b/src/bin/pg_verifybackup/t/004_options.pl
index 22b1444091..234a529e2a 100644
--- a/src/bin/pg_verifybackup/t/004_options.pl
+++ b/src/bin/pg_verifybackup/t/004_options.pl
@@ -10,7 +10,7 @@
use File::Path qw(rmtree);
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 25;
+use Test::More tests => 19;
# Start up the server and take a backup.
my $primary = PostgreSQL::Test::Cluster->new('primary');
diff --git a/src/bin/pg_verifybackup/t/006_encoding.pl b/src/bin/pg_verifybackup/t/006_encoding.pl
index 21c4198b1c..d1b42edbe3 100644
--- a/src/bin/pg_verifybackup/t/006_encoding.pl
+++ b/src/bin/pg_verifybackup/t/006_encoding.pl
@@ -9,7 +9,7 @@
use Config;
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 5;
+use Test::More tests => 3;
my $primary = PostgreSQL::Test::Cluster->new('primary');
$primary->init(allows_streaming => 1);
diff --git a/src/bin/pg_waldump/t/001_basic.pl b/src/bin/pg_waldump/t/001_basic.pl
index fdc968a5ee..277eae749c 100644
--- a/src/bin/pg_waldump/t/001_basic.pl
+++ b/src/bin/pg_waldump/t/001_basic.pl
@@ -4,7 +4,7 @@
use strict;
use warnings;
use PostgreSQL::Test::Utils;
-use Test::More tests => 8;
+use Test::More tests => 3;
program_help_ok('pg_waldump');
program_version_ok('pg_waldump');
diff --git a/src/bin/psql/t/001_basic.pl b/src/bin/psql/t/001_basic.pl
index 6ca0bc75d0..bcc195dd69 100644
--- a/src/bin/psql/t/001_basic.pl
+++ b/src/bin/psql/t/001_basic.pl
@@ -6,7 +6,7 @@
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 25;
+use Test::More tests => 20;
program_help_ok('psql');
program_version_ok('psql');
diff --git a/src/bin/scripts/t/010_clusterdb.pl b/src/bin/scripts/t/010_clusterdb.pl
index 0ba4aa4876..02165aa6d5 100644
--- a/src/bin/scripts/t/010_clusterdb.pl
+++ b/src/bin/scripts/t/010_clusterdb.pl
@@ -6,7 +6,7 @@
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 14;
+use Test::More tests => 7;
program_help_ok('clusterdb');
program_version_ok('clusterdb');
diff --git a/src/bin/scripts/t/011_clusterdb_all.pl b/src/bin/scripts/t/011_clusterdb_all.pl
index d040b95cfb..b506bc9a10 100644
--- a/src/bin/scripts/t/011_clusterdb_all.pl
+++ b/src/bin/scripts/t/011_clusterdb_all.pl
@@ -6,7 +6,7 @@
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 2;
+use Test::More tests => 1;
my $node = PostgreSQL::Test::Cluster->new('main');
$node->init;
diff --git a/src/bin/scripts/t/020_createdb.pl b/src/bin/scripts/t/020_createdb.pl
index 6bcc59de08..cd849c2b1e 100644
--- a/src/bin/scripts/t/020_createdb.pl
+++ b/src/bin/scripts/t/020_createdb.pl
@@ -6,7 +6,7 @@
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 25;
+use Test::More tests => 17;
program_help_ok('createdb');
program_version_ok('createdb');
diff --git a/src/bin/scripts/t/040_createuser.pl b/src/bin/scripts/t/040_createuser.pl
index a865c01f5a..3ff38a4f55 100644
--- a/src/bin/scripts/t/040_createuser.pl
+++ b/src/bin/scripts/t/040_createuser.pl
@@ -6,7 +6,7 @@
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 17;
+use Test::More tests => 8;
program_help_ok('createuser');
program_version_ok('createuser');
diff --git a/src/bin/scripts/t/050_dropdb.pl b/src/bin/scripts/t/050_dropdb.pl
index 5c9342f290..6affaf083e 100644
--- a/src/bin/scripts/t/050_dropdb.pl
+++ b/src/bin/scripts/t/050_dropdb.pl
@@ -6,7 +6,7 @@
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 13;
+use Test::More tests => 6;
program_help_ok('dropdb');
program_version_ok('dropdb');
diff --git a/src/bin/scripts/t/070_dropuser.pl b/src/bin/scripts/t/070_dropuser.pl
index 5d6e75c903..4ce97d1a1d 100644
--- a/src/bin/scripts/t/070_dropuser.pl
+++ b/src/bin/scripts/t/070_dropuser.pl
@@ -6,7 +6,7 @@
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 11;
+use Test::More tests => 5;
program_help_ok('dropuser');
program_version_ok('dropuser');
diff --git a/src/bin/scripts/t/080_pg_isready.pl b/src/bin/scripts/t/080_pg_isready.pl
index 42be32decc..963c08df72 100644
--- a/src/bin/scripts/t/080_pg_isready.pl
+++ b/src/bin/scripts/t/080_pg_isready.pl
@@ -6,7 +6,7 @@
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 10;
+use Test::More tests => 5;
program_help_ok('pg_isready');
program_version_ok('pg_isready');
diff --git a/src/bin/scripts/t/090_reindexdb.pl b/src/bin/scripts/t/090_reindexdb.pl
index 5384b74ccd..fdf1fac52b 100644
--- a/src/bin/scripts/t/090_reindexdb.pl
+++ b/src/bin/scripts/t/090_reindexdb.pl
@@ -6,7 +6,7 @@
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 58;
+use Test::More tests => 37;
program_help_ok('reindexdb');
program_version_ok('reindexdb');
diff --git a/src/bin/scripts/t/091_reindexdb_all.pl b/src/bin/scripts/t/091_reindexdb_all.pl
index acb2418976..9c86c43390 100644
--- a/src/bin/scripts/t/091_reindexdb_all.pl
+++ b/src/bin/scripts/t/091_reindexdb_all.pl
@@ -5,7 +5,7 @@
use warnings;
use PostgreSQL::Test::Cluster;
-use Test::More tests => 2;
+use Test::More tests => 1;
my $node = PostgreSQL::Test::Cluster->new('main');
$node->init;
diff --git a/src/bin/scripts/t/100_vacuumdb.pl b/src/bin/scripts/t/100_vacuumdb.pl
index 6937a35bc4..aa2832a27e 100644
--- a/src/bin/scripts/t/100_vacuumdb.pl
+++ b/src/bin/scripts/t/100_vacuumdb.pl
@@ -6,7 +6,7 @@
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 58;
+use Test::More tests => 36;
program_help_ok('vacuumdb');
program_version_ok('vacuumdb');
diff --git a/src/bin/scripts/t/101_vacuumdb_all.pl b/src/bin/scripts/t/101_vacuumdb_all.pl
index 3dfbfbfdb2..1432cec745 100644
--- a/src/bin/scripts/t/101_vacuumdb_all.pl
+++ b/src/bin/scripts/t/101_vacuumdb_all.pl
@@ -5,7 +5,7 @@
use warnings;
use PostgreSQL::Test::Cluster;
-use Test::More tests => 2;
+use Test::More tests => 1;
my $node = PostgreSQL::Test::Cluster->new('main');
$node->init;
diff --git a/src/bin/scripts/t/102_vacuumdb_stages.pl b/src/bin/scripts/t/102_vacuumdb_stages.pl
index f7bd45ba92..f8d29b5f51 100644
--- a/src/bin/scripts/t/102_vacuumdb_stages.pl
+++ b/src/bin/scripts/t/102_vacuumdb_stages.pl
@@ -5,7 +5,7 @@
use warnings;
use PostgreSQL::Test::Cluster;
-use Test::More tests => 4;
+use Test::More tests => 2;
my $node = PostgreSQL::Test::Cluster->new('main');
$node->init;
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index 9467a199c8..ed2ac110ef 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -2408,14 +2408,18 @@ sub issues_sql_like
my ($self, $cmd, $expected_sql, $test_name) = @_;
- local %ENV = $self->_get_env();
+ subtest $test_name => sub {
+ plan tests => 2;
- my $log_location = -s $self->logfile;
+ local %ENV = $self->_get_env();
- my $result = PostgreSQL::Test::Utils::run_log($cmd);
- ok($result, "@$cmd exit code 0");
- my $log = PostgreSQL::Test::Utils::slurp_file($self->logfile, $log_location);
- like($log, $expected_sql, "$test_name: SQL found in server log");
+ my $log_location = -s $self->logfile;
+
+ my $result = PostgreSQL::Test::Utils::run_log($cmd);
+ ok($result, "@$cmd exit code 0");
+ my $log = PostgreSQL::Test::Utils::slurp_file($self->logfile, $log_location);
+ like($log, $expected_sql, "$test_name: SQL found in server log");
+ };
return;
}
diff --git a/src/test/perl/PostgreSQL/Test/Utils.pm b/src/test/perl/PostgreSQL/Test/Utils.pm
index 378d3f7bc1..3b214e04e8 100644
--- a/src/test/perl/PostgreSQL/Test/Utils.pm
+++ b/src/test/perl/PostgreSQL/Test/Utils.pm
@@ -805,13 +805,16 @@ sub program_help_ok
{
local $Test::Builder::Level = $Test::Builder::Level + 1;
my ($cmd) = @_;
- my ($stdout, $stderr);
- print("# Running: $cmd --help\n");
- my $result = IPC::Run::run [ $cmd, '--help' ], '>', \$stdout, '2>',
- \$stderr;
- ok($result, "$cmd --help exit code 0");
- isnt($stdout, '', "$cmd --help goes to stdout");
- is($stderr, '', "$cmd --help nothing to stderr");
+ subtest "$cmd --help" => sub {
+ plan tests => 3;
+ my ($stdout, $stderr);
+ print("# Running: $cmd --help\n");
+ my $result = IPC::Run::run [ $cmd, '--help' ], '>', \$stdout, '2>',
+ \$stderr;
+ ok($result, "$cmd --help exit code 0");
+ isnt($stdout, '', "$cmd --help goes to stdout");
+ is($stderr, '', "$cmd --help nothing to stderr");
+ };
return;
}
@@ -827,13 +830,16 @@ sub program_version_ok
{
local $Test::Builder::Level = $Test::Builder::Level + 1;
my ($cmd) = @_;
- my ($stdout, $stderr);
- print("# Running: $cmd --version\n");
- my $result = IPC::Run::run [ $cmd, '--version' ], '>', \$stdout, '2>',
- \$stderr;
- ok($result, "$cmd --version exit code 0");
- isnt($stdout, '', "$cmd --version goes to stdout");
- is($stderr, '', "$cmd --version nothing to stderr");
+ subtest "$cmd --version" => sub {
+ plan tests => 3;
+ my ($stdout, $stderr);
+ print("# Running: $cmd --version\n");
+ my $result = IPC::Run::run [ $cmd, '--version' ], '>', \$stdout, '2>',
+ \$stderr;
+ ok($result, "$cmd --version exit code 0");
+ isnt($stdout, '', "$cmd --version goes to stdout");
+ is($stderr, '', "$cmd --version nothing to stderr");
+ };
return;
}
@@ -850,13 +856,16 @@ sub program_options_handling_ok
{
local $Test::Builder::Level = $Test::Builder::Level + 1;
my ($cmd) = @_;
- my ($stdout, $stderr);
- print("# Running: $cmd --not-a-valid-option\n");
- my $result = IPC::Run::run [ $cmd, '--not-a-valid-option' ], '>',
- \$stdout,
- '2>', \$stderr;
- ok(!$result, "$cmd with invalid option nonzero exit code");
- isnt($stderr, '', "$cmd with invalid option prints error message");
+ subtest "$cmd options handling" => sub {
+ plan tests => 2;
+ my ($stdout, $stderr);
+ print("# Running: $cmd --not-a-valid-option\n");
+ my $result = IPC::Run::run [ $cmd, '--not-a-valid-option' ], '>',
+ \$stdout,
+ '2>', \$stderr;
+ ok(!$result, "$cmd with invalid option nonzero exit code");
+ isnt($stderr, '', "$cmd with invalid option prints error message");
+ };
return;
}
@@ -873,13 +882,16 @@ sub command_like
{
local $Test::Builder::Level = $Test::Builder::Level + 1;
my ($cmd, $expected_stdout, $test_name) = @_;
- my ($stdout, $stderr);
- print("# Running: " . join(" ", @{$cmd}) . "\n");
- my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr;
- ok($result, "$test_name: exit code 0");
- is($stderr, '', "$test_name: no stderr");
- $stdout =~ s/\r\n/\n/g if $Config{osname} eq 'msys';
- like($stdout, $expected_stdout, "$test_name: matches");
+ subtest $test_name => sub {
+ plan tests => 3;
+ my ($stdout, $stderr);
+ print("# Running: " . join(" ", @{$cmd}) . "\n");
+ my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr;
+ ok($result, "$test_name: exit code 0");
+ is($stderr, '', "$test_name: no stderr");
+ $stdout =~ s/\r\n/\n/g if $Config{osname} eq 'msys';
+ like($stdout, $expected_stdout, "$test_name: matches");
+ };
return;
}
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index 779ab66838..92d479c50f 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -21,7 +21,7 @@
}
else
{
- plan tests => 110;
+ plan tests => 106;
}
#### Some configuration
--
2.34.1
Attachments:
[text/plain] 0001-Readd-use-of-TAP-subtests.patch (22.2K, ../../[email protected]/2-0001-Readd-use-of-TAP-subtests.patch)
download | inline diff:
From d1e2df592b7eaa7952d621266d5ad5d6b80e503a Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Wed, 8 Dec 2021 14:20:57 +0100
Subject: [PATCH] Readd use of TAP subtests
Since 405f32fc49609eb94fa39e7b5e7c1fe2bb2b73aa, Test::More must be new
enough to support subtests.
The present patch effectively reverts
7912f9b7dc9e2d3f6cd81892ef6aa797578e9f06. Many more refactorings like
this are possible; this is just to get started.
---
contrib/oid2name/t/001_basic.pl | 2 +-
contrib/vacuumlo/t/001_basic.pl | 2 +-
src/bin/initdb/t/001_initdb.pl | 2 +-
src/bin/pg_amcheck/t/001_basic.pl | 2 +-
src/bin/pg_amcheck/t/005_opclass_damage.pl | 2 +-
.../t/010_pg_archivecleanup.pl | 2 +-
src/bin/pg_basebackup/t/010_pg_basebackup.pl | 2 +-
src/bin/pg_basebackup/t/020_pg_receivewal.pl | 2 +-
src/bin/pg_basebackup/t/030_pg_recvlogical.pl | 2 +-
src/bin/pg_checksums/t/001_basic.pl | 2 +-
src/bin/pg_checksums/t/002_actions.pl | 2 +-
src/bin/pg_config/t/001_pg_config.pl | 2 +-
.../pg_controldata/t/001_pg_controldata.pl | 2 +-
src/bin/pg_ctl/t/001_start_stop.pl | 2 +-
src/bin/pg_dump/t/001_basic.pl | 2 +-
src/bin/pg_resetwal/t/001_basic.pl | 2 +-
src/bin/pg_rewind/t/006_options.pl | 2 +-
src/bin/pg_test_fsync/t/001_basic.pl | 2 +-
src/bin/pg_test_timing/t/001_basic.pl | 2 +-
src/bin/pg_verifybackup/t/001_basic.pl | 2 +-
src/bin/pg_verifybackup/t/004_options.pl | 2 +-
src/bin/pg_verifybackup/t/006_encoding.pl | 2 +-
src/bin/pg_waldump/t/001_basic.pl | 2 +-
src/bin/psql/t/001_basic.pl | 2 +-
src/bin/scripts/t/010_clusterdb.pl | 2 +-
src/bin/scripts/t/011_clusterdb_all.pl | 2 +-
src/bin/scripts/t/020_createdb.pl | 2 +-
src/bin/scripts/t/040_createuser.pl | 2 +-
src/bin/scripts/t/050_dropdb.pl | 2 +-
src/bin/scripts/t/070_dropuser.pl | 2 +-
src/bin/scripts/t/080_pg_isready.pl | 2 +-
src/bin/scripts/t/090_reindexdb.pl | 2 +-
src/bin/scripts/t/091_reindexdb_all.pl | 2 +-
src/bin/scripts/t/100_vacuumdb.pl | 2 +-
src/bin/scripts/t/101_vacuumdb_all.pl | 2 +-
src/bin/scripts/t/102_vacuumdb_stages.pl | 2 +-
src/test/perl/PostgreSQL/Test/Cluster.pm | 16 +++--
src/test/perl/PostgreSQL/Test/Utils.pm | 68 +++++++++++--------
src/test/ssl/t/001_ssltests.pl | 2 +-
39 files changed, 87 insertions(+), 71 deletions(-)
diff --git a/contrib/oid2name/t/001_basic.pl b/contrib/oid2name/t/001_basic.pl
index efedba0aa1..9d1d1e3533 100644
--- a/contrib/oid2name/t/001_basic.pl
+++ b/contrib/oid2name/t/001_basic.pl
@@ -5,7 +5,7 @@
use warnings;
use PostgreSQL::Test::Utils;
-use Test::More tests => 8;
+use Test::More tests => 3;
#########################################
# Basic checks
diff --git a/contrib/vacuumlo/t/001_basic.pl b/contrib/vacuumlo/t/001_basic.pl
index 951dad0d47..8acb4a9317 100644
--- a/contrib/vacuumlo/t/001_basic.pl
+++ b/contrib/vacuumlo/t/001_basic.pl
@@ -5,7 +5,7 @@
use warnings;
use PostgreSQL::Test::Utils;
-use Test::More tests => 8;
+use Test::More tests => 3;
program_help_ok('vacuumlo');
program_version_ok('vacuumlo');
diff --git a/src/bin/initdb/t/001_initdb.pl b/src/bin/initdb/t/001_initdb.pl
index 6796d8520e..c621df8270 100644
--- a/src/bin/initdb/t/001_initdb.pl
+++ b/src/bin/initdb/t/001_initdb.pl
@@ -11,7 +11,7 @@
use File::stat qw{lstat};
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 22;
+use Test::More tests => 15;
my $tempdir = PostgreSQL::Test::Utils::tempdir;
my $xlogdir = "$tempdir/pgxlog";
diff --git a/src/bin/pg_amcheck/t/001_basic.pl b/src/bin/pg_amcheck/t/001_basic.pl
index d44fe60a4c..3c5052b3a6 100644
--- a/src/bin/pg_amcheck/t/001_basic.pl
+++ b/src/bin/pg_amcheck/t/001_basic.pl
@@ -5,7 +5,7 @@
use warnings;
use PostgreSQL::Test::Utils;
-use Test::More tests => 8;
+use Test::More tests => 3;
program_help_ok('pg_amcheck');
program_version_ok('pg_amcheck');
diff --git a/src/bin/pg_amcheck/t/005_opclass_damage.pl b/src/bin/pg_amcheck/t/005_opclass_damage.pl
index 2f86f4f2a4..d045e81e65 100644
--- a/src/bin/pg_amcheck/t/005_opclass_damage.pl
+++ b/src/bin/pg_amcheck/t/005_opclass_damage.pl
@@ -8,7 +8,7 @@
use warnings;
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 5;
+use Test::More tests => 3;
my $node = PostgreSQL::Test::Cluster->new('test');
$node->init;
diff --git a/src/bin/pg_archivecleanup/t/010_pg_archivecleanup.pl b/src/bin/pg_archivecleanup/t/010_pg_archivecleanup.pl
index 6b3f486cfa..0531da1115 100644
--- a/src/bin/pg_archivecleanup/t/010_pg_archivecleanup.pl
+++ b/src/bin/pg_archivecleanup/t/010_pg_archivecleanup.pl
@@ -4,7 +4,7 @@
use strict;
use warnings;
use PostgreSQL::Test::Utils;
-use Test::More tests => 42;
+use Test::More tests => 37;
program_help_ok('pg_archivecleanup');
program_version_ok('pg_archivecleanup');
diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index 89f45b77a3..c7bd78d988 100644
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -10,7 +10,7 @@
use Fcntl qw(:seek);
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 110;
+use Test::More tests => 105;
program_help_ok('pg_basebackup');
program_version_ok('pg_basebackup');
diff --git a/src/bin/pg_basebackup/t/020_pg_receivewal.pl b/src/bin/pg_basebackup/t/020_pg_receivewal.pl
index 43599d832b..54cee6248d 100644
--- a/src/bin/pg_basebackup/t/020_pg_receivewal.pl
+++ b/src/bin/pg_basebackup/t/020_pg_receivewal.pl
@@ -5,7 +5,7 @@
use warnings;
use PostgreSQL::Test::Utils;
use PostgreSQL::Test::Cluster;
-use Test::More tests => 42;
+use Test::More tests => 37;
program_help_ok('pg_receivewal');
program_version_ok('pg_receivewal');
diff --git a/src/bin/pg_basebackup/t/030_pg_recvlogical.pl b/src/bin/pg_basebackup/t/030_pg_recvlogical.pl
index 90da1662e3..ab566b116d 100644
--- a/src/bin/pg_basebackup/t/030_pg_recvlogical.pl
+++ b/src/bin/pg_basebackup/t/030_pg_recvlogical.pl
@@ -5,7 +5,7 @@
use warnings;
use PostgreSQL::Test::Utils;
use PostgreSQL::Test::Cluster;
-use Test::More tests => 20;
+use Test::More tests => 15;
program_help_ok('pg_recvlogical');
program_version_ok('pg_recvlogical');
diff --git a/src/bin/pg_checksums/t/001_basic.pl b/src/bin/pg_checksums/t/001_basic.pl
index e9eb3197a6..507ffacef0 100644
--- a/src/bin/pg_checksums/t/001_basic.pl
+++ b/src/bin/pg_checksums/t/001_basic.pl
@@ -4,7 +4,7 @@
use strict;
use warnings;
use PostgreSQL::Test::Utils;
-use Test::More tests => 8;
+use Test::More tests => 3;
program_help_ok('pg_checksums');
program_version_ok('pg_checksums');
diff --git a/src/bin/pg_checksums/t/002_actions.pl b/src/bin/pg_checksums/t/002_actions.pl
index 20a5f27840..f34245108d 100644
--- a/src/bin/pg_checksums/t/002_actions.pl
+++ b/src/bin/pg_checksums/t/002_actions.pl
@@ -11,7 +11,7 @@
use PostgreSQL::Test::Utils;
use Fcntl qw(:seek);
-use Test::More tests => 66;
+use Test::More tests => 58;
# Utility routine to create and check a table with corrupted checksums
diff --git a/src/bin/pg_config/t/001_pg_config.pl b/src/bin/pg_config/t/001_pg_config.pl
index 6c7f9b8602..ebf6aacc4a 100644
--- a/src/bin/pg_config/t/001_pg_config.pl
+++ b/src/bin/pg_config/t/001_pg_config.pl
@@ -4,7 +4,7 @@
use strict;
use warnings;
use PostgreSQL::Test::Utils;
-use Test::More tests => 20;
+use Test::More tests => 7;
program_help_ok('pg_config');
program_version_ok('pg_config');
diff --git a/src/bin/pg_controldata/t/001_pg_controldata.pl b/src/bin/pg_controldata/t/001_pg_controldata.pl
index ad7bacace5..fd3ecfaed7 100644
--- a/src/bin/pg_controldata/t/001_pg_controldata.pl
+++ b/src/bin/pg_controldata/t/001_pg_controldata.pl
@@ -5,7 +5,7 @@
use warnings;
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 17;
+use Test::More tests => 10;
program_help_ok('pg_controldata');
program_version_ok('pg_controldata');
diff --git a/src/bin/pg_ctl/t/001_start_stop.pl b/src/bin/pg_ctl/t/001_start_stop.pl
index f95352bf94..9fb675a079 100644
--- a/src/bin/pg_ctl/t/001_start_stop.pl
+++ b/src/bin/pg_ctl/t/001_start_stop.pl
@@ -9,7 +9,7 @@
use File::stat qw{lstat};
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 24;
+use Test::More tests => 17;
my $tempdir = PostgreSQL::Test::Utils::tempdir;
my $tempdir_short = PostgreSQL::Test::Utils::tempdir_short;
diff --git a/src/bin/pg_dump/t/001_basic.pl b/src/bin/pg_dump/t/001_basic.pl
index 863f4da3d8..d31a72a621 100644
--- a/src/bin/pg_dump/t/001_basic.pl
+++ b/src/bin/pg_dump/t/001_basic.pl
@@ -7,7 +7,7 @@
use Config;
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 82;
+use Test::More tests => 67;
my $tempdir = PostgreSQL::Test::Utils::tempdir;
diff --git a/src/bin/pg_resetwal/t/001_basic.pl b/src/bin/pg_resetwal/t/001_basic.pl
index 0f86aea68e..14aa98de40 100644
--- a/src/bin/pg_resetwal/t/001_basic.pl
+++ b/src/bin/pg_resetwal/t/001_basic.pl
@@ -6,7 +6,7 @@
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 12;
+use Test::More tests => 5;
program_help_ok('pg_resetwal');
program_version_ok('pg_resetwal');
diff --git a/src/bin/pg_rewind/t/006_options.pl b/src/bin/pg_rewind/t/006_options.pl
index 30c7bb46d2..ff08c80976 100644
--- a/src/bin/pg_rewind/t/006_options.pl
+++ b/src/bin/pg_rewind/t/006_options.pl
@@ -7,7 +7,7 @@
use strict;
use warnings;
use PostgreSQL::Test::Utils;
-use Test::More tests => 12;
+use Test::More tests => 7;
program_help_ok('pg_rewind');
program_version_ok('pg_rewind');
diff --git a/src/bin/pg_test_fsync/t/001_basic.pl b/src/bin/pg_test_fsync/t/001_basic.pl
index 8c71f1111e..973506238a 100644
--- a/src/bin/pg_test_fsync/t/001_basic.pl
+++ b/src/bin/pg_test_fsync/t/001_basic.pl
@@ -6,7 +6,7 @@
use Config;
use PostgreSQL::Test::Utils;
-use Test::More tests => 12;
+use Test::More tests => 7;
#########################################
# Basic checks
diff --git a/src/bin/pg_test_timing/t/001_basic.pl b/src/bin/pg_test_timing/t/001_basic.pl
index 3e58926c96..1d95e2a6a6 100644
--- a/src/bin/pg_test_timing/t/001_basic.pl
+++ b/src/bin/pg_test_timing/t/001_basic.pl
@@ -6,7 +6,7 @@
use Config;
use PostgreSQL::Test::Utils;
-use Test::More tests => 12;
+use Test::More tests => 7;
#########################################
# Basic checks
diff --git a/src/bin/pg_verifybackup/t/001_basic.pl b/src/bin/pg_verifybackup/t/001_basic.pl
index 33d6b38d33..9163f3e1b1 100644
--- a/src/bin/pg_verifybackup/t/001_basic.pl
+++ b/src/bin/pg_verifybackup/t/001_basic.pl
@@ -4,7 +4,7 @@
use strict;
use warnings;
use PostgreSQL::Test::Utils;
-use Test::More tests => 16;
+use Test::More tests => 11;
my $tempdir = PostgreSQL::Test::Utils::tempdir;
diff --git a/src/bin/pg_verifybackup/t/004_options.pl b/src/bin/pg_verifybackup/t/004_options.pl
index 22b1444091..234a529e2a 100644
--- a/src/bin/pg_verifybackup/t/004_options.pl
+++ b/src/bin/pg_verifybackup/t/004_options.pl
@@ -10,7 +10,7 @@
use File::Path qw(rmtree);
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 25;
+use Test::More tests => 19;
# Start up the server and take a backup.
my $primary = PostgreSQL::Test::Cluster->new('primary');
diff --git a/src/bin/pg_verifybackup/t/006_encoding.pl b/src/bin/pg_verifybackup/t/006_encoding.pl
index 21c4198b1c..d1b42edbe3 100644
--- a/src/bin/pg_verifybackup/t/006_encoding.pl
+++ b/src/bin/pg_verifybackup/t/006_encoding.pl
@@ -9,7 +9,7 @@
use Config;
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 5;
+use Test::More tests => 3;
my $primary = PostgreSQL::Test::Cluster->new('primary');
$primary->init(allows_streaming => 1);
diff --git a/src/bin/pg_waldump/t/001_basic.pl b/src/bin/pg_waldump/t/001_basic.pl
index fdc968a5ee..277eae749c 100644
--- a/src/bin/pg_waldump/t/001_basic.pl
+++ b/src/bin/pg_waldump/t/001_basic.pl
@@ -4,7 +4,7 @@
use strict;
use warnings;
use PostgreSQL::Test::Utils;
-use Test::More tests => 8;
+use Test::More tests => 3;
program_help_ok('pg_waldump');
program_version_ok('pg_waldump');
diff --git a/src/bin/psql/t/001_basic.pl b/src/bin/psql/t/001_basic.pl
index 6ca0bc75d0..bcc195dd69 100644
--- a/src/bin/psql/t/001_basic.pl
+++ b/src/bin/psql/t/001_basic.pl
@@ -6,7 +6,7 @@
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 25;
+use Test::More tests => 20;
program_help_ok('psql');
program_version_ok('psql');
diff --git a/src/bin/scripts/t/010_clusterdb.pl b/src/bin/scripts/t/010_clusterdb.pl
index 0ba4aa4876..02165aa6d5 100644
--- a/src/bin/scripts/t/010_clusterdb.pl
+++ b/src/bin/scripts/t/010_clusterdb.pl
@@ -6,7 +6,7 @@
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 14;
+use Test::More tests => 7;
program_help_ok('clusterdb');
program_version_ok('clusterdb');
diff --git a/src/bin/scripts/t/011_clusterdb_all.pl b/src/bin/scripts/t/011_clusterdb_all.pl
index d040b95cfb..b506bc9a10 100644
--- a/src/bin/scripts/t/011_clusterdb_all.pl
+++ b/src/bin/scripts/t/011_clusterdb_all.pl
@@ -6,7 +6,7 @@
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 2;
+use Test::More tests => 1;
my $node = PostgreSQL::Test::Cluster->new('main');
$node->init;
diff --git a/src/bin/scripts/t/020_createdb.pl b/src/bin/scripts/t/020_createdb.pl
index 6bcc59de08..cd849c2b1e 100644
--- a/src/bin/scripts/t/020_createdb.pl
+++ b/src/bin/scripts/t/020_createdb.pl
@@ -6,7 +6,7 @@
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 25;
+use Test::More tests => 17;
program_help_ok('createdb');
program_version_ok('createdb');
diff --git a/src/bin/scripts/t/040_createuser.pl b/src/bin/scripts/t/040_createuser.pl
index a865c01f5a..3ff38a4f55 100644
--- a/src/bin/scripts/t/040_createuser.pl
+++ b/src/bin/scripts/t/040_createuser.pl
@@ -6,7 +6,7 @@
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 17;
+use Test::More tests => 8;
program_help_ok('createuser');
program_version_ok('createuser');
diff --git a/src/bin/scripts/t/050_dropdb.pl b/src/bin/scripts/t/050_dropdb.pl
index 5c9342f290..6affaf083e 100644
--- a/src/bin/scripts/t/050_dropdb.pl
+++ b/src/bin/scripts/t/050_dropdb.pl
@@ -6,7 +6,7 @@
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 13;
+use Test::More tests => 6;
program_help_ok('dropdb');
program_version_ok('dropdb');
diff --git a/src/bin/scripts/t/070_dropuser.pl b/src/bin/scripts/t/070_dropuser.pl
index 5d6e75c903..4ce97d1a1d 100644
--- a/src/bin/scripts/t/070_dropuser.pl
+++ b/src/bin/scripts/t/070_dropuser.pl
@@ -6,7 +6,7 @@
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 11;
+use Test::More tests => 5;
program_help_ok('dropuser');
program_version_ok('dropuser');
diff --git a/src/bin/scripts/t/080_pg_isready.pl b/src/bin/scripts/t/080_pg_isready.pl
index 42be32decc..963c08df72 100644
--- a/src/bin/scripts/t/080_pg_isready.pl
+++ b/src/bin/scripts/t/080_pg_isready.pl
@@ -6,7 +6,7 @@
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 10;
+use Test::More tests => 5;
program_help_ok('pg_isready');
program_version_ok('pg_isready');
diff --git a/src/bin/scripts/t/090_reindexdb.pl b/src/bin/scripts/t/090_reindexdb.pl
index 5384b74ccd..fdf1fac52b 100644
--- a/src/bin/scripts/t/090_reindexdb.pl
+++ b/src/bin/scripts/t/090_reindexdb.pl
@@ -6,7 +6,7 @@
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 58;
+use Test::More tests => 37;
program_help_ok('reindexdb');
program_version_ok('reindexdb');
diff --git a/src/bin/scripts/t/091_reindexdb_all.pl b/src/bin/scripts/t/091_reindexdb_all.pl
index acb2418976..9c86c43390 100644
--- a/src/bin/scripts/t/091_reindexdb_all.pl
+++ b/src/bin/scripts/t/091_reindexdb_all.pl
@@ -5,7 +5,7 @@
use warnings;
use PostgreSQL::Test::Cluster;
-use Test::More tests => 2;
+use Test::More tests => 1;
my $node = PostgreSQL::Test::Cluster->new('main');
$node->init;
diff --git a/src/bin/scripts/t/100_vacuumdb.pl b/src/bin/scripts/t/100_vacuumdb.pl
index 6937a35bc4..aa2832a27e 100644
--- a/src/bin/scripts/t/100_vacuumdb.pl
+++ b/src/bin/scripts/t/100_vacuumdb.pl
@@ -6,7 +6,7 @@
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 58;
+use Test::More tests => 36;
program_help_ok('vacuumdb');
program_version_ok('vacuumdb');
diff --git a/src/bin/scripts/t/101_vacuumdb_all.pl b/src/bin/scripts/t/101_vacuumdb_all.pl
index 3dfbfbfdb2..1432cec745 100644
--- a/src/bin/scripts/t/101_vacuumdb_all.pl
+++ b/src/bin/scripts/t/101_vacuumdb_all.pl
@@ -5,7 +5,7 @@
use warnings;
use PostgreSQL::Test::Cluster;
-use Test::More tests => 2;
+use Test::More tests => 1;
my $node = PostgreSQL::Test::Cluster->new('main');
$node->init;
diff --git a/src/bin/scripts/t/102_vacuumdb_stages.pl b/src/bin/scripts/t/102_vacuumdb_stages.pl
index f7bd45ba92..f8d29b5f51 100644
--- a/src/bin/scripts/t/102_vacuumdb_stages.pl
+++ b/src/bin/scripts/t/102_vacuumdb_stages.pl
@@ -5,7 +5,7 @@
use warnings;
use PostgreSQL::Test::Cluster;
-use Test::More tests => 4;
+use Test::More tests => 2;
my $node = PostgreSQL::Test::Cluster->new('main');
$node->init;
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index 9467a199c8..ed2ac110ef 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -2408,14 +2408,18 @@ sub issues_sql_like
my ($self, $cmd, $expected_sql, $test_name) = @_;
- local %ENV = $self->_get_env();
+ subtest $test_name => sub {
+ plan tests => 2;
- my $log_location = -s $self->logfile;
+ local %ENV = $self->_get_env();
- my $result = PostgreSQL::Test::Utils::run_log($cmd);
- ok($result, "@$cmd exit code 0");
- my $log = PostgreSQL::Test::Utils::slurp_file($self->logfile, $log_location);
- like($log, $expected_sql, "$test_name: SQL found in server log");
+ my $log_location = -s $self->logfile;
+
+ my $result = PostgreSQL::Test::Utils::run_log($cmd);
+ ok($result, "@$cmd exit code 0");
+ my $log = PostgreSQL::Test::Utils::slurp_file($self->logfile, $log_location);
+ like($log, $expected_sql, "$test_name: SQL found in server log");
+ };
return;
}
diff --git a/src/test/perl/PostgreSQL/Test/Utils.pm b/src/test/perl/PostgreSQL/Test/Utils.pm
index 378d3f7bc1..3b214e04e8 100644
--- a/src/test/perl/PostgreSQL/Test/Utils.pm
+++ b/src/test/perl/PostgreSQL/Test/Utils.pm
@@ -805,13 +805,16 @@ sub program_help_ok
{
local $Test::Builder::Level = $Test::Builder::Level + 1;
my ($cmd) = @_;
- my ($stdout, $stderr);
- print("# Running: $cmd --help\n");
- my $result = IPC::Run::run [ $cmd, '--help' ], '>', \$stdout, '2>',
- \$stderr;
- ok($result, "$cmd --help exit code 0");
- isnt($stdout, '', "$cmd --help goes to stdout");
- is($stderr, '', "$cmd --help nothing to stderr");
+ subtest "$cmd --help" => sub {
+ plan tests => 3;
+ my ($stdout, $stderr);
+ print("# Running: $cmd --help\n");
+ my $result = IPC::Run::run [ $cmd, '--help' ], '>', \$stdout, '2>',
+ \$stderr;
+ ok($result, "$cmd --help exit code 0");
+ isnt($stdout, '', "$cmd --help goes to stdout");
+ is($stderr, '', "$cmd --help nothing to stderr");
+ };
return;
}
@@ -827,13 +830,16 @@ sub program_version_ok
{
local $Test::Builder::Level = $Test::Builder::Level + 1;
my ($cmd) = @_;
- my ($stdout, $stderr);
- print("# Running: $cmd --version\n");
- my $result = IPC::Run::run [ $cmd, '--version' ], '>', \$stdout, '2>',
- \$stderr;
- ok($result, "$cmd --version exit code 0");
- isnt($stdout, '', "$cmd --version goes to stdout");
- is($stderr, '', "$cmd --version nothing to stderr");
+ subtest "$cmd --version" => sub {
+ plan tests => 3;
+ my ($stdout, $stderr);
+ print("# Running: $cmd --version\n");
+ my $result = IPC::Run::run [ $cmd, '--version' ], '>', \$stdout, '2>',
+ \$stderr;
+ ok($result, "$cmd --version exit code 0");
+ isnt($stdout, '', "$cmd --version goes to stdout");
+ is($stderr, '', "$cmd --version nothing to stderr");
+ };
return;
}
@@ -850,13 +856,16 @@ sub program_options_handling_ok
{
local $Test::Builder::Level = $Test::Builder::Level + 1;
my ($cmd) = @_;
- my ($stdout, $stderr);
- print("# Running: $cmd --not-a-valid-option\n");
- my $result = IPC::Run::run [ $cmd, '--not-a-valid-option' ], '>',
- \$stdout,
- '2>', \$stderr;
- ok(!$result, "$cmd with invalid option nonzero exit code");
- isnt($stderr, '', "$cmd with invalid option prints error message");
+ subtest "$cmd options handling" => sub {
+ plan tests => 2;
+ my ($stdout, $stderr);
+ print("# Running: $cmd --not-a-valid-option\n");
+ my $result = IPC::Run::run [ $cmd, '--not-a-valid-option' ], '>',
+ \$stdout,
+ '2>', \$stderr;
+ ok(!$result, "$cmd with invalid option nonzero exit code");
+ isnt($stderr, '', "$cmd with invalid option prints error message");
+ };
return;
}
@@ -873,13 +882,16 @@ sub command_like
{
local $Test::Builder::Level = $Test::Builder::Level + 1;
my ($cmd, $expected_stdout, $test_name) = @_;
- my ($stdout, $stderr);
- print("# Running: " . join(" ", @{$cmd}) . "\n");
- my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr;
- ok($result, "$test_name: exit code 0");
- is($stderr, '', "$test_name: no stderr");
- $stdout =~ s/\r\n/\n/g if $Config{osname} eq 'msys';
- like($stdout, $expected_stdout, "$test_name: matches");
+ subtest $test_name => sub {
+ plan tests => 3;
+ my ($stdout, $stderr);
+ print("# Running: " . join(" ", @{$cmd}) . "\n");
+ my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr;
+ ok($result, "$test_name: exit code 0");
+ is($stderr, '', "$test_name: no stderr");
+ $stdout =~ s/\r\n/\n/g if $Config{osname} eq 'msys';
+ like($stdout, $expected_stdout, "$test_name: matches");
+ };
return;
}
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index 779ab66838..92d479c50f 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -21,7 +21,7 @@
}
else
{
- plan tests => 110;
+ plan tests => 106;
}
#### Some configuration
--
2.34.1
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: Readd use of TAP subtests
@ 2021-12-08 13:49 Dagfinn Ilmari Mannsåker <[email protected]>
parent: Peter Eisentraut <[email protected]>
1 sibling, 1 reply; 26+ messages in thread
From: Dagfinn Ilmari Mannsåker @ 2021-12-08 13:49 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: pgsql-hackers
Peter Eisentraut <[email protected]> writes:
> Now that subtests in TAP are supported again, I want to correct the
> great historical injustice of 7912f9b7dc9e2d3f6cd81892ef6aa797578e9f06
> and put those subtests back.
The updated Test::More version requirement also gives us done_testing()
(added in 0.88), which saves us from the constant maintenance headache
of updating the test counts every time. Do you fancy switching the
tests you're modifying anyway to that?
- ilmari
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: Readd use of TAP subtests
@ 2021-12-08 13:53 Daniel Gustafsson <[email protected]>
parent: Dagfinn Ilmari Mannsåker <[email protected]>
0 siblings, 1 reply; 26+ messages in thread
From: Daniel Gustafsson @ 2021-12-08 13:53 UTC (permalink / raw)
To: Dagfinn Ilmari Mannsåker <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; pgsql-hackers
> On 8 Dec 2021, at 14:49, Dagfinn Ilmari Mannsåker <[email protected]> wrote:
>
> Peter Eisentraut <[email protected]> writes:
>
>> Now that subtests in TAP are supported again, I want to correct the
>> great historical injustice of 7912f9b7dc9e2d3f6cd81892ef6aa797578e9f06
>> and put those subtests back.
>
> The updated Test::More version requirement also gives us done_testing()
> (added in 0.88), which saves us from the constant maintenance headache
> of updating the test counts every time. Do you fancy switching the
> tests you're modifying anyway to that?
We already call done_testing() in a number of tests, and have done so for a
number of years. src/test/ssl/t/002_scram.pl is one example.
--
Daniel Gustafsson https://vmware.com/
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: Readd use of TAP subtests
@ 2021-12-08 13:53 Andrew Dunstan <[email protected]>
parent: Peter Eisentraut <[email protected]>
1 sibling, 0 replies; 26+ messages in thread
From: Andrew Dunstan @ 2021-12-08 13:53 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; pgsql-hackers
On 12/8/21 08:26, Peter Eisentraut wrote:
>
> Now that subtests in TAP are supported again, I want to correct the
> great historical injustice of 7912f9b7dc9e2d3f6cd81892ef6aa797578e9f06
> and put those subtests back.
>
> Much more work like this is possible, of course. I just wanted to get
> this out of the way since the code was already written and I've had
> this on my list for, uh, 7 years.
+many as long as we cover all the cases in Cluster.pm and Utils.pm. I
suspect they have acquired some new multi-test subs in the intervening 7
years :-)
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: Readd use of TAP subtests
@ 2021-12-08 14:08 Dagfinn Ilmari Mannsåker <[email protected]>
parent: Daniel Gustafsson <[email protected]>
0 siblings, 1 reply; 26+ messages in thread
From: Dagfinn Ilmari Mannsåker @ 2021-12-08 14:08 UTC (permalink / raw)
To: Daniel Gustafsson <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; pgsql-hackers
Daniel Gustafsson <[email protected]> writes:
>> On 8 Dec 2021, at 14:49, Dagfinn Ilmari Mannsåker <[email protected]> wrote:
>>
>> Peter Eisentraut <[email protected]> writes:
>>
>>> Now that subtests in TAP are supported again, I want to correct the
>>> great historical injustice of 7912f9b7dc9e2d3f6cd81892ef6aa797578e9f06
>>> and put those subtests back.
>>
>> The updated Test::More version requirement also gives us done_testing()
>> (added in 0.88), which saves us from the constant maintenance headache
>> of updating the test counts every time. Do you fancy switching the
>> tests you're modifying anyway to that?
>
> We already call done_testing() in a number of tests, and have done so for a
> number of years. src/test/ssl/t/002_scram.pl is one example.
Reading the Test::More changelog more closely, it turns out that even
though we used to depend on version 0.87, that's effectively equivalent
0.88, because there was no stable 0.87 release, only 0.86 and
development releases 0.87_01 through _03.
Either way, I think we should be switching tests to done_testing()
whenever it would otherwise have to adjust the test count, to avoid
having to do that again and again and again going forward.
- ilmari
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: Readd use of TAP subtests
@ 2021-12-08 14:33 Andrew Dunstan <[email protected]>
parent: Dagfinn Ilmari Mannsåker <[email protected]>
0 siblings, 2 replies; 26+ messages in thread
From: Andrew Dunstan @ 2021-12-08 14:33 UTC (permalink / raw)
To: Dagfinn Ilmari Mannsåker <[email protected]>; Daniel Gustafsson <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; pgsql-hackers
On 12/8/21 09:08, Dagfinn Ilmari Mannsåker wrote:
>
> Either way, I think we should be switching tests to done_testing()
> whenever it would otherwise have to adjust the test count, to avoid
> having to do that again and again and again going forward.
>
I'm not so sure. I don't think its necessarily a bad idea to have to
declare how many tests you're going to run. I appreciate it gets hard in
some cases, which is why we have now insisted on a Test::More version
that supports subtests. I suppose we could just take the attitude that
we're happy with however many tests it actually runs, and as long as
they all pass we're good. It just seems slightly sloppy.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: Readd use of TAP subtests
@ 2021-12-08 14:48 Dagfinn Ilmari Mannsåker <[email protected]>
parent: Andrew Dunstan <[email protected]>
1 sibling, 1 reply; 26+ messages in thread
From: Dagfinn Ilmari Mannsåker @ 2021-12-08 14:48 UTC (permalink / raw)
To: Andrew Dunstan <[email protected]>; +Cc: Daniel Gustafsson <[email protected]>; Peter Eisentraut <[email protected]>; pgsql-hackers
Andrew Dunstan <[email protected]> writes:
> On 12/8/21 09:08, Dagfinn Ilmari Mannsåker wrote:
>>
>> Either way, I think we should be switching tests to done_testing()
>> whenever it would otherwise have to adjust the test count, to avoid
>> having to do that again and again and again going forward.
>>
>
> I'm not so sure. I don't think its necessarily a bad idea to have to
> declare how many tests you're going to run. I appreciate it gets hard in
> some cases, which is why we have now insisted on a Test::More version
> that supports subtests. I suppose we could just take the attitude that
> we're happy with however many tests it actually runs, and as long as
> they all pass we're good. It just seems slightly sloppy.
The point of done_testing() is to additionally assert that the test
script ran to completion, so you don't get silent failures if something
should end up calling exit(0) prematurely (a non-zero exit status is
considered a failure by the test harness).
The only cases where an explicit plan adds value is if you're running
tests in a loop and care about the number of iterations, or have a
callback with a test inside that you want to make sure gets called. For
these, it's better to explicitly assert that the list you're iterating
over is of the right length, or increment a counter in the loop or
callback and assert that it has the expected value. This has the added
benefit of the failure being coming from the relevant place and having a
helpful description, rather than a plan mismatch at the end which you
then have to hunt down the cause of.
- ilmari
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: Readd use of TAP subtests
@ 2021-12-08 15:25 Tom Lane <[email protected]>
parent: Andrew Dunstan <[email protected]>
1 sibling, 1 reply; 26+ messages in thread
From: Tom Lane @ 2021-12-08 15:25 UTC (permalink / raw)
To: Andrew Dunstan <[email protected]>; +Cc: Dagfinn Ilmari Mannsåker <[email protected]>; Daniel Gustafsson <[email protected]>; Peter Eisentraut <[email protected]>; pgsql-hackers
Andrew Dunstan <[email protected]> writes:
> On 12/8/21 09:08, Dagfinn Ilmari Mannsåker wrote:
>> Either way, I think we should be switching tests to done_testing()
>> whenever it would otherwise have to adjust the test count, to avoid
>> having to do that again and again and again going forward.
> I'm not so sure. I don't think its necessarily a bad idea to have to
> declare how many tests you're going to run.
I think the main point is to make sure that the test script reached an
intended exit point, rather than dying early someplace. It's not apparent
to me why reaching a done_testing() call is a less reliable indicator of
that than executing some specific number of tests --- and I agree with
ilmari that maintaining the test count is a serious PITA.
regards, tom lane
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: Readd use of TAP subtests
@ 2021-12-08 17:31 Tom Lane <[email protected]>
parent: Dagfinn Ilmari Mannsåker <[email protected]>
0 siblings, 1 reply; 26+ messages in thread
From: Tom Lane @ 2021-12-08 17:31 UTC (permalink / raw)
To: Dagfinn Ilmari Mannsåker <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Daniel Gustafsson <[email protected]>; Peter Eisentraut <[email protected]>; pgsql-hackers
=?utf-8?Q?Dagfinn_Ilmari_Manns=C3=A5ker?= <[email protected]> writes:
> The only cases where an explicit plan adds value is if you're running
> tests in a loop and care about the number of iterations, or have a
> callback with a test inside that you want to make sure gets called. For
> these, it's better to explicitly assert that the list you're iterating
> over is of the right length, or increment a counter in the loop or
> callback and assert that it has the expected value. This has the added
> benefit of the failure being coming from the relevant place and having a
> helpful description, rather than a plan mismatch at the end which you
> then have to hunt down the cause of.
Yeah. A different way of stating that is that the test count adds
security only if you re-derive its proper value from first principles
every time you modify the test script. I don't know about you guys,
but the only way I've ever adjusted those numbers is to put in whatever
the error message said was right. I don't see how that's adding
anything but make-work; it's certainly not doing much to help verify
the script's control flow.
A question that seems pretty relevant here is: what exactly is the
point of using the subtest feature, if we aren't especially interested
in its effect on the overall test count? I can see that it'd have
value when you wanted to use skip_all to control a subset of a test
run, but I'm not detecting where is the value-added in the cases in
Peter's proposed patch.
regards, tom lane
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: Readd use of TAP subtests
@ 2021-12-09 15:51 Peter Eisentraut <[email protected]>
parent: Tom Lane <[email protected]>
0 siblings, 0 replies; 26+ messages in thread
From: Peter Eisentraut @ 2021-12-09 15:51 UTC (permalink / raw)
To: Tom Lane <[email protected]>; Dagfinn Ilmari Mannsåker <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Daniel Gustafsson <[email protected]>; pgsql-hackers
On 08.12.21 18:31, Tom Lane wrote:
> A question that seems pretty relevant here is: what exactly is the
> point of using the subtest feature, if we aren't especially interested
> in its effect on the overall test count? I can see that it'd have
> value when you wanted to use skip_all to control a subset of a test
> run, but I'm not detecting where is the value-added in the cases in
> Peter's proposed patch.
It's useful if you edit a test file and add (what would appear to be) N
tests and want to update the number.
But I'm also OK with the done_testing() style, if there are no drawbacks
to that.
Does that call into question why we raised the Test::More version to
begin with? Or were there other reasons?
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: Readd use of TAP subtests
@ 2021-12-09 20:54 Daniel Gustafsson <[email protected]>
parent: Tom Lane <[email protected]>
0 siblings, 0 replies; 26+ messages in thread
From: Daniel Gustafsson @ 2021-12-09 20:54 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Dagfinn Ilmari Mannsåker <[email protected]>; Peter Eisentraut <[email protected]>; pgsql-hackers
> On 8 Dec 2021, at 16:25, Tom Lane <[email protected]> wrote:
> I think the main point is to make sure that the test script reached an
> intended exit point, rather than dying early someplace. It's not apparent
> to me why reaching a done_testing() call is a less reliable indicator of
> that than executing some specific number of tests --- and I agree with
> ilmari that maintaining the test count is a serious PITA.
FWIW I agree with this and am in favor of using done_testing().
--
Daniel Gustafsson https://vmware.com/
^ permalink raw reply [nested|flat] 26+ messages in thread
* ProcessStartupPacket(): database_name and user_name truncation
@ 2023-06-21 07:43 Drouvot, Bertrand <[email protected]>
0 siblings, 1 reply; 26+ messages in thread
From: Drouvot, Bertrand @ 2023-06-21 07:43 UTC (permalink / raw)
To: PostgreSQL Hackers <[email protected]>
Hi hackers,
Please find attached a patch to truncate (in ProcessStartupPacket())
the port->database_name and port->user_name in such a way to not break
multibyte character boundary.
Indeed, currently, one could create a database that way:
postgres=# create database ääääääääääääääääääääääääääääääää;
NOTICE: identifier "ääääääääääääääääääääääääääääääää" will be truncated to "äääääääääääääääääääääääääääääää"
CREATE DATABASE
The database name has been truncated from 64 bytes to 62 bytes thanks to pg_mbcliplen()
which ensures to not break multibyte character boundary.
postgres=# select datname, OCTET_LENGTH(datname),encoding from pg_database;
datname | octet_length | encoding
---------------------------------+--------------+----------
äääääääääääääääääääääääääääääää | 62 | 6
Trying to connect with the 64 bytes name:
$ psql -d ääääääääääääääääääääääääääääääää
psql: error: connection to server on socket "/tmp/.s.PGSQL.55448" failed: FATAL: database "äääääääääääääääääääääääääääääää" does not exist
It fails because the truncation done in ProcessStartupPacket():
"
if (strlen(port→database_name) >= NAMEDATALEN)
port→database_name[NAMEDATALEN - 1] = '\0';
"
does not take care about multibyte character boundary.
On the other hand it works with non multibyte character involved:
postgres=# create database abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijke;
NOTICE: identifier "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijke" will be truncated to "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijk"
CREATE DATABASE
postgres=# select datname, OCTET_LENGTH(datname),encoding from pg_database;
datname | octet_length | encoding
-----------------------------------------------------------------+--------------+----------
abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijk | 63 | 6
The database name is truncated to 63 bytes and then using the 64 bytes name would work:
$ psql -d abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijke
psql (16beta1)
Type "help" for help.
The comment in ProcessStartupPacket() states:
"
/*
* Truncate given database and user names to length of a Postgres name.
* This avoids lookup failures when overlength names are given.
*/
"
The last sentence is not right in case of mutlibyte character (as seen
in the first example).
About the patch:
As the database encoding is not known yet in ProcessStartupPacket() (
and we are even not sure the database provided does exist), the proposed
patch does not rely on pg_mbcliplen() but on pg_encoding_mbcliplen().
The proposed patch does use the client encoding that it retrieves that way:
- use the one requested in the startup packet (if we come across it)
- use the one from the locale (if we did not find a client encoding request
in the startup packet)
- use PG_SQL_ASCII (if none of the above have been satisfied)
Happy to discuss any other thoughts or suggestions if any.
With the proposed patch in place, using the first example above (and the
64 bytes name) we would get:
$ PGCLIENTENCODING=LATIN1 psql -d ääääääääääääääääääääääääääääääää
psql: error: connection to server on socket "/tmp/.s.PGSQL.55448" failed: FATAL: database "äääääääääääääääääääääääääääääää" does not exist
but this one would allow us to connect:
$ PGCLIENTENCODING=UTF8 psql -d ääääääääääääääääääääääääääääääää
psql (16beta1)
Type "help" for help.
The patch does not provide documentation update or related TAP test (but could be added
if we feel the need).
Looking forward to your feedback,
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
From 6ccb4f03f56dda9e9a2616c6e0875a97a8711d72 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email protected]>
Date: Tue, 20 Jun 2023 10:26:16 +0000
Subject: [PATCH v1] multibyte truncation for database and user name
database_name and user_name truncation done in ProcessStartupPacket() do not
take care of multibyte character boundary. In case of multibyte, this somehow
breaks the initial goal to avoid lookup failures when overlength names are given.
---
src/backend/postmaster/postmaster.c | 48 ++++++++++++++++++++++++++---
1 file changed, 44 insertions(+), 4 deletions(-)
100.0% src/backend/postmaster/
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 4c49393fc5..72991c4eab 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -1951,6 +1951,7 @@ ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done)
char *buf;
ProtocolVersion proto;
MemoryContext oldcontext;
+ int client_encoding = -1;
pq_startmsgread();
@@ -2238,6 +2239,18 @@ retry1:
{
port->application_name = pg_clean_ascii(valptr, 0);
}
+ /* Record the client_encoding if we come across it */
+ else if (strcmp(nameptr, "client_encoding") == 0)
+ {
+ client_encoding = pg_valid_client_encoding(valptr);
+
+ if (client_encoding < 0)
+ ereport(FATAL,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("invalid value for parameter \"%s\": \"%s\"",
+ "client_encoding",
+ valptr)));
+ }
}
offset = valoffset + strlen(valptr) + 1;
}
@@ -2291,13 +2304,40 @@ retry1:
}
/*
- * Truncate given database and user names to length of a Postgres name.
- * This avoids lookup failures when overlength names are given.
+ * Given the client encoding, truncate given database and user names to
+ * length of a Postgres name. This avoids lookup failures when overlength
+ * names are given.
*/
+ if (client_encoding < 0)
+ {
+ /*
+ * client_encoding has not been found in the startup packet so let's
+ * try to get it from locale.
+ */
+ client_encoding = pg_get_encoding_from_locale(NULL, true);
+
+ if (client_encoding < 0)
+ client_encoding = PG_SQL_ASCII;
+ }
+
if (strlen(port->database_name) >= NAMEDATALEN)
- port->database_name[NAMEDATALEN - 1] = '\0';
+ {
+ int newlen;
+
+ newlen = pg_encoding_mbcliplen(client_encoding, port->database_name,
+ strlen(port->database_name),
+ NAMEDATALEN - 1);
+ port->database_name[newlen] = '\0';
+ }
if (strlen(port->user_name) >= NAMEDATALEN)
- port->user_name[NAMEDATALEN - 1] = '\0';
+ {
+ int newlen;
+
+ newlen = pg_encoding_mbcliplen(client_encoding, port->user_name,
+ strlen(port->user_name),
+ NAMEDATALEN - 1);
+ port->user_name[newlen] = '\0';
+ }
if (am_walsender)
MyBackendType = B_WAL_SENDER;
--
2.34.1
Attachments:
[text/plain] v1-0001-multibyte-truncation-for-database-and-user-name.patch (2.9K, ../../[email protected]/2-v1-0001-multibyte-truncation-for-database-and-user-name.patch)
download | inline diff:
From 6ccb4f03f56dda9e9a2616c6e0875a97a8711d72 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email protected]>
Date: Tue, 20 Jun 2023 10:26:16 +0000
Subject: [PATCH v1] multibyte truncation for database and user name
database_name and user_name truncation done in ProcessStartupPacket() do not
take care of multibyte character boundary. In case of multibyte, this somehow
breaks the initial goal to avoid lookup failures when overlength names are given.
---
src/backend/postmaster/postmaster.c | 48 ++++++++++++++++++++++++++---
1 file changed, 44 insertions(+), 4 deletions(-)
100.0% src/backend/postmaster/
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 4c49393fc5..72991c4eab 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -1951,6 +1951,7 @@ ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done)
char *buf;
ProtocolVersion proto;
MemoryContext oldcontext;
+ int client_encoding = -1;
pq_startmsgread();
@@ -2238,6 +2239,18 @@ retry1:
{
port->application_name = pg_clean_ascii(valptr, 0);
}
+ /* Record the client_encoding if we come across it */
+ else if (strcmp(nameptr, "client_encoding") == 0)
+ {
+ client_encoding = pg_valid_client_encoding(valptr);
+
+ if (client_encoding < 0)
+ ereport(FATAL,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("invalid value for parameter \"%s\": \"%s\"",
+ "client_encoding",
+ valptr)));
+ }
}
offset = valoffset + strlen(valptr) + 1;
}
@@ -2291,13 +2304,40 @@ retry1:
}
/*
- * Truncate given database and user names to length of a Postgres name.
- * This avoids lookup failures when overlength names are given.
+ * Given the client encoding, truncate given database and user names to
+ * length of a Postgres name. This avoids lookup failures when overlength
+ * names are given.
*/
+ if (client_encoding < 0)
+ {
+ /*
+ * client_encoding has not been found in the startup packet so let's
+ * try to get it from locale.
+ */
+ client_encoding = pg_get_encoding_from_locale(NULL, true);
+
+ if (client_encoding < 0)
+ client_encoding = PG_SQL_ASCII;
+ }
+
if (strlen(port->database_name) >= NAMEDATALEN)
- port->database_name[NAMEDATALEN - 1] = '\0';
+ {
+ int newlen;
+
+ newlen = pg_encoding_mbcliplen(client_encoding, port->database_name,
+ strlen(port->database_name),
+ NAMEDATALEN - 1);
+ port->database_name[newlen] = '\0';
+ }
if (strlen(port->user_name) >= NAMEDATALEN)
- port->user_name[NAMEDATALEN - 1] = '\0';
+ {
+ int newlen;
+
+ newlen = pg_encoding_mbcliplen(client_encoding, port->user_name,
+ strlen(port->user_name),
+ NAMEDATALEN - 1);
+ port->user_name[newlen] = '\0';
+ }
if (am_walsender)
MyBackendType = B_WAL_SENDER;
--
2.34.1
^ permalink raw reply [nested|flat] 26+ messages in thread
* Re: ProcessStartupPacket(): database_name and user_name truncation
@ 2023-06-21 08:54 Kyotaro Horiguchi <[email protected]>
parent: Drouvot, Bertrand <[email protected]>
0 siblings, 0 replies; 26+ messages in thread
From: Kyotaro Horiguchi @ 2023-06-21 08:54 UTC (permalink / raw)
To: [email protected]; +Cc: [email protected]
At Wed, 21 Jun 2023 09:43:50 +0200, "Drouvot, Bertrand" <[email protected]> wrote in
> Trying to connect with the 64 bytes name:
>
> $ psql -d ääääääääääääääääääääääääääääääää
> psql: error: connection to server on socket "/tmp/.s.PGSQL.55448"
> failed: FATAL: database "äääääääääääääääääääääääääääääää" does not
> exist
IMHO, I'm not sure we should allow connections without the exact name
being provided. In that sense, I think we might want to consider
outright rejecting the estblishment of a connection when the given
database name doesn't fit the startup packet, since the database with
the exact given name cannot be found.
While it is somewhat off-topic, I cannot establish a connection if the
console encoding differs from the template database even if I provide
the identical database name. (I don't mean I want that behavior to be
"fix"ed.)
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
^ permalink raw reply [nested|flat] 26+ messages in thread
end of thread, other threads:[~2023-06-21 08:54 UTC | newest]
Thread overview: 26+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2017-11-16 17:31 Re: [HACKERS] Proposal: generic WAL compression Antonin Houska <[email protected]>
2017-11-20 07:49 ` Antonin Houska <[email protected]>
2017-11-30 02:40 ` Michael Paquier <[email protected]>
2018-01-22 21:37 ` Stephen Frost <[email protected]>
2020-11-29 04:29 [PATCH v33 10/11] Preserve pg_stat_file() isdir.. Justin Pryzby <[email protected]>
2020-11-29 04:29 [PATCH v28 10/11] Preserve pg_stat_file() isdir.. Justin Pryzby <[email protected]>
2020-11-29 04:29 [PATCH v32 10/11] Preserve pg_stat_file() isdir.. Justin Pryzby <[email protected]>
2020-11-29 04:29 [PATCH v30 10/11] Preserve pg_stat_file() isdir.. Justin Pryzby <[email protected]>
2020-11-29 04:29 [PATCH v27 10/11] Preserve pg_stat_file() isdir.. Justin Pryzby <[email protected]>
2020-11-29 04:29 [PATCH v24 10/11] Preserve pg_stat_file() isdir.. Justin Pryzby <[email protected]>
2020-11-29 04:29 [PATCH v31 10/11] Preserve pg_stat_file() isdir.. Justin Pryzby <[email protected]>
2020-11-29 04:29 [PATCH v25 10/11] Preserve pg_stat_file() isdir.. Justin Pryzby <[email protected]>
2020-11-29 04:29 [PATCH v26 10/11] Preserve pg_stat_file() isdir.. Justin Pryzby <[email protected]>
2021-12-08 13:26 Readd use of TAP subtests Peter Eisentraut <[email protected]>
2021-12-08 13:49 ` Re: Readd use of TAP subtests Dagfinn Ilmari Mannsåker <[email protected]>
2021-12-08 13:53 ` Re: Readd use of TAP subtests Daniel Gustafsson <[email protected]>
2021-12-08 14:08 ` Re: Readd use of TAP subtests Dagfinn Ilmari Mannsåker <[email protected]>
2021-12-08 14:33 ` Re: Readd use of TAP subtests Andrew Dunstan <[email protected]>
2021-12-08 14:48 ` Re: Readd use of TAP subtests Dagfinn Ilmari Mannsåker <[email protected]>
2021-12-08 17:31 ` Re: Readd use of TAP subtests Tom Lane <[email protected]>
2021-12-09 15:51 ` Re: Readd use of TAP subtests Peter Eisentraut <[email protected]>
2021-12-08 15:25 ` Re: Readd use of TAP subtests Tom Lane <[email protected]>
2021-12-09 20:54 ` Re: Readd use of TAP subtests Daniel Gustafsson <[email protected]>
2021-12-08 13:53 ` Re: Readd use of TAP subtests Andrew Dunstan <[email protected]>
2023-06-21 07:43 ProcessStartupPacket(): database_name and user_name truncation Drouvot, Bertrand <[email protected]>
2023-06-21 08:54 ` Re: ProcessStartupPacket(): database_name and user_name truncation Kyotaro Horiguchi <[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