public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v32 10/11] Preserve pg_stat_file() isdir..
25+ messages / 11 participants
[nested] [flat]
* [PATCH v32 10/11] Preserve pg_stat_file() isdir..
@ 2020-11-29 04:29 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 25+ 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] 25+ 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; 25+ 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] 25+ 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; 25+ 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] 25+ 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; 25+ 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] 25+ 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; 25+ 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] 25+ 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; 25+ 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] 25+ 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; 25+ 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] 25+ 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; 25+ 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] 25+ 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; 25+ 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] 25+ messages in thread
* Readd use of TAP subtests
@ 2021-12-08 13:26 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 Andrew Dunstan <[email protected]>
0 siblings, 2 replies; 25+ 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] 25+ messages in thread
* Re: Readd use of TAP subtests
2021-12-08 13:26 Readd use of TAP subtests Peter Eisentraut <[email protected]>
@ 2021-12-08 13:49 ` Dagfinn Ilmari Mannsåker <[email protected]>
2021-12-08 13:53 ` Re: Readd use of TAP subtests Daniel Gustafsson <[email protected]>
1 sibling, 1 reply; 25+ 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] 25+ messages in thread
* Re: Readd use of TAP subtests
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 ` Daniel Gustafsson <[email protected]>
2021-12-08 14:08 ` Re: Readd use of TAP subtests Dagfinn Ilmari Mannsåker <[email protected]>
0 siblings, 1 reply; 25+ 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] 25+ messages in thread
* Re: Readd use of TAP subtests
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 ` Dagfinn Ilmari Mannsåker <[email protected]>
2021-12-08 14:33 ` Re: Readd use of TAP subtests Andrew Dunstan <[email protected]>
0 siblings, 1 reply; 25+ 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] 25+ messages in thread
* Re: Readd use of TAP subtests
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 ` Andrew Dunstan <[email protected]>
2021-12-08 14:48 ` Re: Readd use of TAP subtests Dagfinn Ilmari Mannsåker <[email protected]>
2021-12-08 15:25 ` Re: Readd use of TAP subtests Tom Lane <[email protected]>
0 siblings, 2 replies; 25+ 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] 25+ messages in thread
* Re: Readd use of TAP subtests
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 ` Dagfinn Ilmari Mannsåker <[email protected]>
2021-12-08 17:31 ` Re: Readd use of TAP subtests Tom Lane <[email protected]>
1 sibling, 1 reply; 25+ 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] 25+ messages in thread
* Re: Readd use of TAP subtests
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 ` Tom Lane <[email protected]>
2021-12-09 15:51 ` Re: Readd use of TAP subtests Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 25+ 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] 25+ messages in thread
* Re: Readd use of TAP subtests
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 ` Peter Eisentraut <[email protected]>
0 siblings, 0 replies; 25+ 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] 25+ messages in thread
* Re: Readd use of TAP subtests
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 15:25 ` Tom Lane <[email protected]>
2021-12-09 20:54 ` Re: Readd use of TAP subtests Daniel Gustafsson <[email protected]>
1 sibling, 1 reply; 25+ 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] 25+ messages in thread
* Re: Readd use of TAP subtests
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 15:25 ` Re: Readd use of TAP subtests Tom Lane <[email protected]>
@ 2021-12-09 20:54 ` Daniel Gustafsson <[email protected]>
0 siblings, 0 replies; 25+ 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] 25+ messages in thread
* Re: Readd use of TAP subtests
2021-12-08 13:26 Readd use of TAP subtests Peter Eisentraut <[email protected]>
@ 2021-12-08 13:53 ` Andrew Dunstan <[email protected]>
1 sibling, 0 replies; 25+ 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] 25+ messages in thread
* RE: Make default subscription streaming option as Parallel
@ 2024-10-07 06:56 Hayato Kuroda (Fujitsu) <[email protected]>
2024-10-07 10:33 ` Re: Make default subscription streaming option as Parallel vignesh C <[email protected]>
0 siblings, 1 reply; 25+ messages in thread
From: Hayato Kuroda (Fujitsu) @ 2024-10-07 06:56 UTC (permalink / raw)
To: 'vignesh C' <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
Dear Vignesh,
> One key point to consider is that the lock on transaction objects will
> be held for a longer duration when using streaming in parallel. This
> occurs because the parallel apply worker initiates the transaction as
> soon as streaming begins, maintaining the lock until the transaction
> is fully completed. As a result, for long-running transactions, this
> extended lock can hinder concurrent access that requires a lock.
So, the current default is the most conservative and can run anywhere; your
proposal is a bit optimistic, right? Since long transactions should be avoided
in any case, and everyone knows it, I agree with your point.
One comment for your patch;
Shouldn't we add a streaming=off case for pg_dump test? You added lines but no one
passes it.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Make default subscription streaming option as Parallel
2024-10-07 06:56 RE: Make default subscription streaming option as Parallel Hayato Kuroda (Fujitsu) <[email protected]>
@ 2024-10-07 10:33 ` vignesh C <[email protected]>
2024-10-08 08:55 ` Re: Make default subscription streaming option as Parallel shveta malik <[email protected]>
2024-10-21 06:09 ` Re: Make default subscription streaming option as Parallel Amit Kapila <[email protected]>
0 siblings, 2 replies; 25+ messages in thread
From: vignesh C @ 2024-10-07 10:33 UTC (permalink / raw)
To: Hayato Kuroda (Fujitsu) <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On Mon, 7 Oct 2024 at 12:26, Hayato Kuroda (Fujitsu)
<[email protected]> wrote:
>
> Dear Vignesh,
>
> > One key point to consider is that the lock on transaction objects will
> > be held for a longer duration when using streaming in parallel. This
> > occurs because the parallel apply worker initiates the transaction as
> > soon as streaming begins, maintaining the lock until the transaction
> > is fully completed. As a result, for long-running transactions, this
> > extended lock can hinder concurrent access that requires a lock.
>
> So, the current default is the most conservative and can run anywhere; your
> proposal is a bit optimistic, right?
Yes, that is correct.
> One comment for your patch;
> Shouldn't we add a streaming=off case for pg_dump test? You added lines but no one
> passes it.
>
Update existing pg_dump tests to cover the streaming options, the
attached patch has the changes for the same.
Regards,
Vignesh
Attachments:
[application/octet-stream] v2-0001-Make-default-value-for-susbcription-streaming-opt.patch (21.5K, ../../CALDaNm2RDVhF-6k=Kyg6nEUOJ26ShZ_SYo8fm_26Km1mPW7THw@mail.gmail.com/2-v2-0001-Make-default-value-for-susbcription-streaming-opt.patch)
download | inline diff:
From 5ee49e587ad8783c78fcbb6715cd9170838ebb39 Mon Sep 17 00:00:00 2001
From: Vignesh C <[email protected]>
Date: Wed, 25 Sep 2024 14:42:20 +0530
Subject: [PATCH v2] Make default value for susbcription streaming option to
parallel.
Currently default value of streaming option is set to false. All
transactions are fully decoded on the publisher before being sent
to the subscriber. This approach can leads reduced performance,
particularly under heavy load.
Changing default streaming option to parallel, by doing this,
incoming changes will be directly applied by one of the available
parallel apply workers. This method significantly improves the
performance of commit operations.
---
doc/src/sgml/ref/create_subscription.sgml | 19 ++++++++---------
src/backend/commands/subscriptioncmds.c | 2 +-
src/bin/pg_dump/pg_dump.c | 2 ++
src/bin/pg_dump/t/002_pg_dump.pl | 10 ++++-----
src/test/regress/expected/subscription.out | 24 +++++++++++-----------
5 files changed, 29 insertions(+), 28 deletions(-)
diff --git a/doc/src/sgml/ref/create_subscription.sgml b/doc/src/sgml/ref/create_subscription.sgml
index 8a3096e62b..43f1b1f881 100644
--- a/doc/src/sgml/ref/create_subscription.sgml
+++ b/doc/src/sgml/ref/create_subscription.sgml
@@ -271,9 +271,13 @@ CREATE SUBSCRIPTION <replaceable class="parameter">subscription_name</replaceabl
<listitem>
<para>
Specifies whether to enable streaming of in-progress transactions
- for this subscription. The default value is <literal>off</literal>,
- meaning all transactions are fully decoded on the publisher and only
- then sent to the subscriber as a whole.
+ for this subscription. The default value is <literal>parallel</literal>,
+ meaning incoming changes are directly applied via one of the parallel
+ apply workers, if available. If no parallel apply worker is free to
+ handle streaming transactions then the changes are written to
+ temporary files and applied after the transaction is committed. Note
+ that if an error happens in a parallel apply worker, the finish LSN
+ of the remote transaction might not be reported in the server log.
</para>
<para>
@@ -283,13 +287,8 @@ CREATE SUBSCRIPTION <replaceable class="parameter">subscription_name</replaceabl
</para>
<para>
- If set to <literal>parallel</literal>, incoming changes are directly
- applied via one of the parallel apply workers, if available. If no
- parallel apply worker is free to handle streaming transactions then
- the changes are written to temporary files and applied after the
- transaction is committed. Note that if an error happens in a
- parallel apply worker, the finish LSN of the remote transaction
- might not be reported in the server log.
+ If set to <literal>off</literal>, all transactions are fully decoded
+ on the publisher and only then sent to the subscriber as a whole.
</para>
</listitem>
</varlistentry>
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 02ccc636b8..0a7a618855 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -151,7 +151,7 @@ parse_subscription_options(ParseState *pstate, List *stmt_options,
if (IsSet(supported_opts, SUBOPT_BINARY))
opts->binary = false;
if (IsSet(supported_opts, SUBOPT_STREAMING))
- opts->streaming = LOGICALREP_STREAM_OFF;
+ opts->streaming = LOGICALREP_STREAM_PARALLEL;
if (IsSet(supported_opts, SUBOPT_TWOPHASE_COMMIT))
opts->twophase = false;
if (IsSet(supported_opts, SUBOPT_DISABLE_ON_ERR))
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 1b47c388ce..d8c6330732 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -5235,6 +5235,8 @@ dumpSubscription(Archive *fout, const SubscriptionInfo *subinfo)
appendPQExpBufferStr(query, ", streaming = on");
else if (strcmp(subinfo->substream, "p") == 0)
appendPQExpBufferStr(query, ", streaming = parallel");
+ else
+ appendPQExpBufferStr(query, ", streaming = off");
if (strcmp(subinfo->subtwophasestate, two_phase_disabled) != 0)
appendPQExpBufferStr(query, ", two_phase = on");
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index ab6c830491..ac60829d68 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -2992,7 +2992,7 @@ my %tests = (
CONNECTION \'dbname=doesnotexist\' PUBLICATION pub1
WITH (connect = false);',
regexp => qr/^
- \QCREATE SUBSCRIPTION sub1 CONNECTION 'dbname=doesnotexist' PUBLICATION pub1 WITH (connect = false, slot_name = 'sub1');\E
+ \QCREATE SUBSCRIPTION sub1 CONNECTION 'dbname=doesnotexist' PUBLICATION pub1 WITH (connect = false, slot_name = 'sub1', streaming = parallel);\E
/xm,
like => { %full_runs, section_post_data => 1, },
},
@@ -3001,9 +3001,9 @@ my %tests = (
create_order => 50,
create_sql => 'CREATE SUBSCRIPTION sub2
CONNECTION \'dbname=doesnotexist\' PUBLICATION pub1
- WITH (connect = false, origin = none);',
+ WITH (connect = false, origin = none, streaming = off);',
regexp => qr/^
- \QCREATE SUBSCRIPTION sub2 CONNECTION 'dbname=doesnotexist' PUBLICATION pub1 WITH (connect = false, slot_name = 'sub2', origin = none);\E
+ \QCREATE SUBSCRIPTION sub2 CONNECTION 'dbname=doesnotexist' PUBLICATION pub1 WITH (connect = false, slot_name = 'sub2', streaming = off, origin = none);\E
/xm,
like => { %full_runs, section_post_data => 1, },
},
@@ -3012,9 +3012,9 @@ my %tests = (
create_order => 50,
create_sql => 'CREATE SUBSCRIPTION sub3
CONNECTION \'dbname=doesnotexist\' PUBLICATION pub1
- WITH (connect = false, origin = any);',
+ WITH (connect = false, origin = any, streaming = on);',
regexp => qr/^
- \QCREATE SUBSCRIPTION sub3 CONNECTION 'dbname=doesnotexist' PUBLICATION pub1 WITH (connect = false, slot_name = 'sub3');\E
+ \QCREATE SUBSCRIPTION sub3 CONNECTION 'dbname=doesnotexist' PUBLICATION pub1 WITH (connect = false, slot_name = 'sub3', streaming = on);\E
/xm,
like => { %full_runs, section_post_data => 1, },
},
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index 17d48b1685..1443e1d929 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -119,7 +119,7 @@ HINT: To initiate replication, you must manually create the replication slot, e
List of subscriptions
Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Password required | Run as owner? | Failover | Synchronous commit | Conninfo | Skip LSN
------------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+-------------------+---------------+----------+--------------------+-----------------------------+----------
- regress_testsub4 | regress_subscription_user | f | {testpub} | f | off | d | f | none | t | f | f | off | dbname=regress_doesnotexist | 0/0
+ regress_testsub4 | regress_subscription_user | f | {testpub} | f | parallel | d | f | none | t | f | f | off | dbname=regress_doesnotexist | 0/0
(1 row)
ALTER SUBSCRIPTION regress_testsub4 SET (origin = any);
@@ -127,7 +127,7 @@ ALTER SUBSCRIPTION regress_testsub4 SET (origin = any);
List of subscriptions
Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Password required | Run as owner? | Failover | Synchronous commit | Conninfo | Skip LSN
------------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+-------------------+---------------+----------+--------------------+-----------------------------+----------
- regress_testsub4 | regress_subscription_user | f | {testpub} | f | off | d | f | any | t | f | f | off | dbname=regress_doesnotexist | 0/0
+ regress_testsub4 | regress_subscription_user | f | {testpub} | f | parallel | d | f | any | t | f | f | off | dbname=regress_doesnotexist | 0/0
(1 row)
DROP SUBSCRIPTION regress_testsub3;
@@ -148,7 +148,7 @@ ERROR: invalid connection string syntax: missing "=" after "foobar" in connecti
List of subscriptions
Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Password required | Run as owner? | Failover | Synchronous commit | Conninfo | Skip LSN
-----------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+-------------------+---------------+----------+--------------------+-----------------------------+----------
- regress_testsub | regress_subscription_user | f | {testpub} | f | off | d | f | any | t | f | f | off | dbname=regress_doesnotexist | 0/0
+ regress_testsub | regress_subscription_user | f | {testpub} | f | parallel | d | f | any | t | f | f | off | dbname=regress_doesnotexist | 0/0
(1 row)
ALTER SUBSCRIPTION regress_testsub SET PUBLICATION testpub2, testpub3 WITH (refresh = false);
@@ -160,7 +160,7 @@ ALTER SUBSCRIPTION regress_testsub SET (run_as_owner = true);
List of subscriptions
Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Password required | Run as owner? | Failover | Synchronous commit | Conninfo | Skip LSN
-----------------+---------------------------+---------+---------------------+--------+-----------+------------------+------------------+--------+-------------------+---------------+----------+--------------------+------------------------------+----------
- regress_testsub | regress_subscription_user | f | {testpub2,testpub3} | f | off | d | f | any | f | t | f | off | dbname=regress_doesnotexist2 | 0/0
+ regress_testsub | regress_subscription_user | f | {testpub2,testpub3} | f | parallel | d | f | any | f | t | f | off | dbname=regress_doesnotexist2 | 0/0
(1 row)
ALTER SUBSCRIPTION regress_testsub SET (password_required = true);
@@ -179,7 +179,7 @@ ALTER SUBSCRIPTION regress_testsub SKIP (lsn = '0/12345');
List of subscriptions
Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Password required | Run as owner? | Failover | Synchronous commit | Conninfo | Skip LSN
-----------------+---------------------------+---------+---------------------+--------+-----------+------------------+------------------+--------+-------------------+---------------+----------+--------------------+------------------------------+----------
- regress_testsub | regress_subscription_user | f | {testpub2,testpub3} | f | off | d | f | any | t | f | f | off | dbname=regress_doesnotexist2 | 0/12345
+ regress_testsub | regress_subscription_user | f | {testpub2,testpub3} | f | parallel | d | f | any | t | f | f | off | dbname=regress_doesnotexist2 | 0/12345
(1 row)
-- ok - with lsn = NONE
@@ -191,7 +191,7 @@ ERROR: invalid WAL location (LSN): 0/0
List of subscriptions
Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Password required | Run as owner? | Failover | Synchronous commit | Conninfo | Skip LSN
-----------------+---------------------------+---------+---------------------+--------+-----------+------------------+------------------+--------+-------------------+---------------+----------+--------------------+------------------------------+----------
- regress_testsub | regress_subscription_user | f | {testpub2,testpub3} | f | off | d | f | any | t | f | f | off | dbname=regress_doesnotexist2 | 0/0
+ regress_testsub | regress_subscription_user | f | {testpub2,testpub3} | f | parallel | d | f | any | t | f | f | off | dbname=regress_doesnotexist2 | 0/0
(1 row)
BEGIN;
@@ -226,7 +226,7 @@ HINT: Available values: local, remote_write, remote_apply, on, off.
List of subscriptions
Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Password required | Run as owner? | Failover | Synchronous commit | Conninfo | Skip LSN
---------------------+---------------------------+---------+---------------------+--------+-----------+------------------+------------------+--------+-------------------+---------------+----------+--------------------+------------------------------+----------
- regress_testsub_foo | regress_subscription_user | f | {testpub2,testpub3} | f | off | d | f | any | t | f | f | local | dbname=regress_doesnotexist2 | 0/0
+ regress_testsub_foo | regress_subscription_user | f | {testpub2,testpub3} | f | parallel | d | f | any | t | f | f | local | dbname=regress_doesnotexist2 | 0/0
(1 row)
-- rename back to keep the rest simple
@@ -258,7 +258,7 @@ HINT: To initiate replication, you must manually create the replication slot, e
List of subscriptions
Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Password required | Run as owner? | Failover | Synchronous commit | Conninfo | Skip LSN
-----------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+-------------------+---------------+----------+--------------------+-----------------------------+----------
- regress_testsub | regress_subscription_user | f | {testpub} | t | off | d | f | any | t | f | f | off | dbname=regress_doesnotexist | 0/0
+ regress_testsub | regress_subscription_user | f | {testpub} | t | parallel | d | f | any | t | f | f | off | dbname=regress_doesnotexist | 0/0
(1 row)
ALTER SUBSCRIPTION regress_testsub SET (binary = false);
@@ -267,7 +267,7 @@ ALTER SUBSCRIPTION regress_testsub SET (slot_name = NONE);
List of subscriptions
Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Password required | Run as owner? | Failover | Synchronous commit | Conninfo | Skip LSN
-----------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+-------------------+---------------+----------+--------------------+-----------------------------+----------
- regress_testsub | regress_subscription_user | f | {testpub} | f | off | d | f | any | t | f | f | off | dbname=regress_doesnotexist | 0/0
+ regress_testsub | regress_subscription_user | f | {testpub} | f | parallel | d | f | any | t | f | f | off | dbname=regress_doesnotexist | 0/0
(1 row)
DROP SUBSCRIPTION regress_testsub;
@@ -374,7 +374,7 @@ HINT: To initiate replication, you must manually create the replication slot, e
List of subscriptions
Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Password required | Run as owner? | Failover | Synchronous commit | Conninfo | Skip LSN
-----------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+-------------------+---------------+----------+--------------------+-----------------------------+----------
- regress_testsub | regress_subscription_user | f | {testpub} | f | off | p | f | any | t | f | f | off | dbname=regress_doesnotexist | 0/0
+ regress_testsub | regress_subscription_user | f | {testpub} | f | parallel | p | f | any | t | f | f | off | dbname=regress_doesnotexist | 0/0
(1 row)
-- we can alter streaming when two_phase enabled
@@ -412,7 +412,7 @@ HINT: To initiate replication, you must manually create the replication slot, e
List of subscriptions
Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Password required | Run as owner? | Failover | Synchronous commit | Conninfo | Skip LSN
-----------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+-------------------+---------------+----------+--------------------+-----------------------------+----------
- regress_testsub | regress_subscription_user | f | {testpub} | f | off | d | f | any | t | f | f | off | dbname=regress_doesnotexist | 0/0
+ regress_testsub | regress_subscription_user | f | {testpub} | f | parallel | d | f | any | t | f | f | off | dbname=regress_doesnotexist | 0/0
(1 row)
ALTER SUBSCRIPTION regress_testsub SET (disable_on_error = true);
@@ -420,7 +420,7 @@ ALTER SUBSCRIPTION regress_testsub SET (disable_on_error = true);
List of subscriptions
Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Password required | Run as owner? | Failover | Synchronous commit | Conninfo | Skip LSN
-----------------+---------------------------+---------+-------------+--------+-----------+------------------+------------------+--------+-------------------+---------------+----------+--------------------+-----------------------------+----------
- regress_testsub | regress_subscription_user | f | {testpub} | f | off | d | t | any | t | f | f | off | dbname=regress_doesnotexist | 0/0
+ regress_testsub | regress_subscription_user | f | {testpub} | f | parallel | d | t | any | t | f | f | off | dbname=regress_doesnotexist | 0/0
(1 row)
ALTER SUBSCRIPTION regress_testsub SET (slot_name = NONE);
--
2.34.1
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Make default subscription streaming option as Parallel
2024-10-07 06:56 RE: Make default subscription streaming option as Parallel Hayato Kuroda (Fujitsu) <[email protected]>
2024-10-07 10:33 ` Re: Make default subscription streaming option as Parallel vignesh C <[email protected]>
@ 2024-10-08 08:55 ` shveta malik <[email protected]>
1 sibling, 0 replies; 25+ messages in thread
From: shveta malik @ 2024-10-08 08:55 UTC (permalink / raw)
To: vignesh C <[email protected]>; +Cc: Hayato Kuroda (Fujitsu) <[email protected]>; PostgreSQL Hackers <[email protected]>; shveta malik <[email protected]>
On Mon, Oct 7, 2024 at 4:03 PM vignesh C <[email protected]> wrote:
>
With parallel streaming as default, do you think there is a need to
increase the default for 'max_logical_replication_workers' as IIUC
parallel workers are taken from the same pool.
thanks
Shveta
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Make default subscription streaming option as Parallel
2024-10-07 06:56 RE: Make default subscription streaming option as Parallel Hayato Kuroda (Fujitsu) <[email protected]>
2024-10-07 10:33 ` Re: Make default subscription streaming option as Parallel vignesh C <[email protected]>
@ 2024-10-21 06:09 ` Amit Kapila <[email protected]>
2024-10-21 09:57 ` Re: Make default subscription streaming option as Parallel Peter Smith <[email protected]>
1 sibling, 1 reply; 25+ messages in thread
From: Amit Kapila @ 2024-10-21 06:09 UTC (permalink / raw)
To: vignesh C <[email protected]>; +Cc: Hayato Kuroda (Fujitsu) <[email protected]>; PostgreSQL Hackers <[email protected]>
On Mon, Oct 7, 2024 at 4:03 PM vignesh C <[email protected]> wrote:
>
> On Mon, 7 Oct 2024 at 12:26, Hayato Kuroda (Fujitsu)
> <[email protected]> wrote:
>
> > One comment for your patch;
> > Shouldn't we add a streaming=off case for pg_dump test? You added lines but no one
> > passes it.
> >
>
> Update existing pg_dump tests to cover the streaming options, the
> attached patch has the changes for the same.
>
@@ -5235,6 +5235,8 @@ dumpSubscription(Archive *fout, const
SubscriptionInfo *subinfo)
appendPQExpBufferStr(query, ", streaming = on");
else if (strcmp(subinfo->substream, "p") == 0)
appendPQExpBufferStr(query, ", streaming = parallel");
+ else
+ appendPQExpBufferStr(query, ", streaming = off");
For newer versions (>=18), we shouldn't need to specify "streaming =
parallel" while creating a subscription as that will be the default.
However, with the above code pg_dump statements will still have that.
There is nothing wrong with that but ideally, it won't be required.
Now, OTOH, doing that would require some version-handling code, which
is nothing new for pg_dump but not sure it makes sense for this
particular case. Another related point is that normally we don't
recommend people to use dump generated with pg_dump to use with lower
server versions than pg_dump itself, but the current proposed patch
will allow that. However, if we change it as I am saying that won't be
possible. So, I am okay with the current code but want to see if
anyone else thinks otherwise or if I am missing something.
--
With Regards,
Amit Kapila.
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: Make default subscription streaming option as Parallel
2024-10-07 06:56 RE: Make default subscription streaming option as Parallel Hayato Kuroda (Fujitsu) <[email protected]>
2024-10-07 10:33 ` Re: Make default subscription streaming option as Parallel vignesh C <[email protected]>
2024-10-21 06:09 ` Re: Make default subscription streaming option as Parallel Amit Kapila <[email protected]>
@ 2024-10-21 09:57 ` Peter Smith <[email protected]>
0 siblings, 0 replies; 25+ messages in thread
From: Peter Smith @ 2024-10-21 09:57 UTC (permalink / raw)
To: Amit Kapila <[email protected]>; +Cc: vignesh C <[email protected]>; Hayato Kuroda (Fujitsu) <[email protected]>; PostgreSQL Hackers <[email protected]>
On Mon, Oct 21, 2024 at 5:09 PM Amit Kapila <[email protected]> wrote:
>
> On Mon, Oct 7, 2024 at 4:03 PM vignesh C <[email protected]> wrote:
> >
> > On Mon, 7 Oct 2024 at 12:26, Hayato Kuroda (Fujitsu)
> > <[email protected]> wrote:
> >
> > > One comment for your patch;
> > > Shouldn't we add a streaming=off case for pg_dump test? You added lines but no one
> > > passes it.
> > >
> >
> > Update existing pg_dump tests to cover the streaming options, the
> > attached patch has the changes for the same.
> >
>
> @@ -5235,6 +5235,8 @@ dumpSubscription(Archive *fout, const
> SubscriptionInfo *subinfo)
> appendPQExpBufferStr(query, ", streaming = on");
> else if (strcmp(subinfo->substream, "p") == 0)
> appendPQExpBufferStr(query, ", streaming = parallel");
> + else
> + appendPQExpBufferStr(query, ", streaming = off");
>
> For newer versions (>=18), we shouldn't need to specify "streaming =
> parallel" while creating a subscription as that will be the default.
> However, with the above code pg_dump statements will still have that.
> There is nothing wrong with that but ideally, it won't be required.
> Now, OTOH, doing that would require some version-handling code, which
> is nothing new for pg_dump but not sure it makes sense for this
> particular case. Another related point is that normally we don't
> recommend people to use dump generated with pg_dump to use with lower
> server versions than pg_dump itself, but the current proposed patch
> will allow that. However, if we change it as I am saying that won't be
> possible.
Leaving the patch as-is seems better to me.
PROS
- The simple code explicitly setting all parameter values is easy to
understand as-is.
- AFAICT it works for all that the pg_dump docs [1] guarantees.
- No version handling code will be needed...
- Therefore, no risk of accidentally introducing any versioning bugs.
- Yields a more portable dump file (even though not guaranteed by pg_dump docs)
CONS
- A few more chars in the dump file?
- What else?
======
[1] https://www.postgresql.org/docs/devel/app-pgdump.html
Kind Regards,
Peter Smith.
Fujitsu Australia
^ permalink raw reply [nested|flat] 25+ messages in thread
end of thread, other threads:[~2024-10-21 09:57 UTC | newest]
Thread overview: 25+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-11-29 04:29 [PATCH v32 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 v30 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 v28 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]>
2020-11-29 04:29 [PATCH v33 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]>
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]>
2024-10-07 06:56 RE: Make default subscription streaming option as Parallel Hayato Kuroda (Fujitsu) <[email protected]>
2024-10-07 10:33 ` Re: Make default subscription streaming option as Parallel vignesh C <[email protected]>
2024-10-08 08:55 ` Re: Make default subscription streaming option as Parallel shveta malik <[email protected]>
2024-10-21 06:09 ` Re: Make default subscription streaming option as Parallel Amit Kapila <[email protected]>
2024-10-21 09:57 ` Re: Make default subscription streaming option as Parallel Peter Smith <[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