pgsql: Handle logical slot conflicts on standby
Handle logical slot conflicts on standby During WAL replay on the standby, when a conflict with a logical slot is identified, invalidate such slots. There are two sources of conflicts: 1) Using the information added in 6af1793954e, logical slots are invalidated if required rows are removed 2) wal_level on the primary server is reduced to below logical Uses the infrastructure introduced in the prior commit. FIXME: add commit reference. Change InvalidatePossiblyObsoleteSlot() to use a recovery conflict to interrupt use of a slot, if called in the startup process. The new recovery conflict is added to pg_stat_database_conflicts, as confl_active_logicalslot. See 6af1793954e for an overall design of logical decoding on a standby. Bumps catversion for the addition of the pg_stat_database_conflicts column. Bumps PGSTAT_FILE_FORMAT_ID for the same reason. Author: "Drouvot, Bertrand" Author: Andres Freund Author: Amit Khandekar (in an older version) Reviewed-by: "Drouvot, Bertrand" Reviewed-by: Andres Freund Reviewed-by: Robert Haas Reviewed-by: Fabrízio de Royes Mello Reviewed-by: Bharath Rupireddy Reviewed-by: Amit Kapila Reviewed-by: Alvaro Herrera Discussion: https://postgr.es/m/20230407075009.igg7be27ha2ht...@awork3.anarazel.de Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/26669757b6a7665c1069e77e6472bd8550193ca6 Modified Files -- doc/src/sgml/monitoring.sgml | 11 +++ src/backend/access/gist/gistxlog.c | 2 ++ src/backend/access/hash/hash_xlog.c | 1 + src/backend/access/heap/heapam.c | 3 +++ src/backend/access/nbtree/nbtxlog.c | 2 ++ src/backend/access/spgist/spgxlog.c | 1 + src/backend/access/transam/xlog.c| 15 +++ src/backend/catalog/system_views.sql | 3 ++- src/backend/replication/slot.c | 8 +++- src/backend/storage/ipc/procsignal.c | 3 +++ src/backend/storage/ipc/standby.c| 20 +++- src/backend/tcop/postgres.c | 9 + src/backend/utils/activity/pgstat_database.c | 4 src/backend/utils/adt/pgstatfuncs.c | 3 +++ src/include/catalog/catversion.h | 2 +- src/include/catalog/pg_proc.dat | 5 + src/include/pgstat.h | 3 ++- src/include/storage/procsignal.h | 1 + src/include/storage/standby.h| 2 ++ src/test/regress/expected/rules.out | 3 ++- 20 files changed, 95 insertions(+), 6 deletions(-)
pgsql: For cascading replication, wake physical and logical walsenders
For cascading replication, wake physical and logical walsenders separately Physical walsenders can't send data until it's been flushed; logical walsenders can't decode and send data until it's been applied. On the standby, the WAL is flushed first, which will only wake up physical walsenders; and then applied, which will only wake up logical walsenders. Previously, all walsenders were awakened when the WAL was flushed. That was fine for logical walsenders on the primary; but on the standby the flushed WAL would have been not applied yet, so logical walsenders were awakened too early. Per idea from Jeff Davis and Amit Kapila. Author: "Drouvot, Bertrand" Reviewed-By: Jeff Davis Reviewed-By: Robert Haas Reviewed-by: Amit Kapila Reviewed-by: Masahiko Sawada Discussion: https://postgr.es/m/caa4ek1+zo5lueisabx10c81lu-fwmko4m9wyg1cdkbw7hqh...@mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/e101dfac3a53c20bfbf1ca85d30a368c2954facf Modified Files -- src/backend/access/transam/xlog.c | 6 ++--- src/backend/access/transam/xlogarchive.c| 2 +- src/backend/access/transam/xlogrecovery.c | 37 +++--- src/backend/replication/walreceiver.c | 2 +- src/backend/replication/walsender.c | 41 ++--- src/include/replication/walsender.h | 22 src/include/replication/walsender_private.h | 3 +++ 7 files changed, 84 insertions(+), 29 deletions(-)
pgsql: TAP test for logical decoding on standby
TAP test for logical decoding on standby Author: "Drouvot, Bertrand" Author: Amit Khandekar Author: Craig Ringer (in an older version) Author: Andres Freund Reviewed-by: "Drouvot, Bertrand" Reviewed-by: Andres Freund Reviewed-by: Robert Haas Reviewed-by: Amit Kapila Reviewed-by: Fabrízio de Royes Mello Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/fcd77d53217b4c4049d176072a1763d6e11ca478 Modified Files -- src/test/perl/PostgreSQL/Test/Cluster.pm | 37 ++ src/test/recovery/meson.build | 1 + .../recovery/t/035_standby_logical_decoding.pl | 734 + 3 files changed, 772 insertions(+)
pgsql: Allow logical decoding on standbys
Allow logical decoding on standbys Unsurprisingly, this requires wal_level = logical to be set on the primary and standby. The infrastructure added in 26669757b6a ensures that slots are invalidated if the primary's wal_level is lowered. Creating a slot on a standby waits for a xl_running_xact record to be processed. If the primary is idle (and thus not emitting xl_running_xact records), that can take a while. To make that faster, this commit also introduces the pg_log_standby_snapshot() function. By executing it on the primary, completion of slot creation on the standby can be accelerated. Note that logical decoding on a standby does not itself enforce that required catalog rows are not removed. The user has to use physical replication slots + hot_standby_feedback or other measures to prevent that. If catalog rows required for a slot are removed, the slot is invalidated. See 6af1793954e for an overall design of logical decoding on a standby. Bumps catversion, for the addition of the pg_log_standby_snapshot() function. Author: "Drouvot, Bertrand" Author: Andres Freund (in an older version) Author: Amit Khandekar (in an older version) Reviewed-by: Andres Freund Reviewed-by: FabrÌzio de Royes Mello Reviewed-by: Amit Kapila Reviewed-By: Robert Haas Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/0fdab27ad68a059a1663fa5ce48d76333f1bd74c Modified Files -- doc/src/sgml/func.sgml| 15 doc/src/sgml/logicaldecoding.sgml | 27 +++ src/backend/access/transam/xlog.c | 11 ++ src/backend/access/transam/xlogfuncs.c| 31 + src/backend/catalog/system_functions.sql | 2 ++ src/backend/replication/logical/decode.c | 30 +++- src/backend/replication/logical/logical.c | 36 ++- src/backend/replication/slot.c| 57 --- src/backend/replication/walsender.c | 48 +- src/include/access/xlog.h | 1 + src/include/catalog/catversion.h | 2 +- src/include/catalog/pg_proc.dat | 3 ++ 12 files changed, 202 insertions(+), 61 deletions(-)
pgsql: Redesign interrupt/cancel API for regex engine.
Redesign interrupt/cancel API for regex engine. Previously, a PostgreSQL-specific callback checked by the regex engine had a way to trigger a special error code REG_CANCEL if it detected that the next call to CHECK_FOR_INTERRUPTS() would certainly throw via ereport(). A later proposed bugfix aims to move some complex logic out of signal handlers, so that it won't run until the next CHECK_FOR_INTERRUPTS(), which makes the above design impossible unless we split CHECK_FOR_INTERRUPTS() into two phases, one to run logic and another to ereport(). We may develop such a system in the future, but for the regex code it is no longer necessary. An earlier commit moved regex memory management over to our MemoryContext system. Given that the purpose of the two-phase interrupt checking was to free memory before throwing, something we don't need to worry about anymore, it seems simpler to inject CHECK_FOR_INTERRUPTS() directly into cancelation points, and just let it throw. Since the plan is to keep PostgreSQL-specific concerns separate from the main regex engine code (with a view to bein able to stay in sync with other projects), do this with a new macro INTERRUPT(), customizable in regcustom.h and defaulting to nothing. Reviewed-by: Tom Lane Discussion: https://postgr.es/m/CA%2BhUKGK3PGKwcKqzoosamn36YW-fsuTdOPPF1i_rtEO%3DnEYKSg%40mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/db4f21e4a34b1d5a3f7123e28e77f575d1a971ea Modified Files -- src/backend/regex/regc_locale.c | 6 +--- src/backend/regex/regc_nfa.c | 48 ++-- src/backend/regex/regcomp.c | 18 src/backend/regex/rege_dfa.c | 6 +--- src/backend/regex/regexec.c | 3 +- src/backend/utils/adt/jsonpath_gram.y| 2 -- src/backend/utils/adt/regexp.c | 11 src/backend/utils/adt/varlena.c | 1 - src/include/regex/regcustom.h| 3 +- src/include/regex/regerrs.h | 4 --- src/include/regex/regex.h| 1 - src/include/regex/regguts.h | 9 +++--- src/test/modules/test_regex/test_regex.c | 10 --- 13 files changed, 18 insertions(+), 104 deletions(-)
pgsql: Update contrib/trgm_regexp's memory management.
Update contrib/trgm_regexp's memory management. While no code change was necessary for this code to keep working, we don't need to use PG_TRY()/PG_FINALLY() with explicit clean-up while working with regexes anymore. Reviewed-by: Tom Lane Discussion: https://postgr.es/m/CA%2BhUKGK3PGKwcKqzoosamn36YW-fsuTdOPPF1i_rtEO%3DnEYKSg%40mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/6db75edb2ecbc9f6912f90b671b01ab4ac3a01b0 Modified Files -- contrib/pg_trgm/trgm_regexp.c | 17 ++--- 1 file changed, 2 insertions(+), 15 deletions(-)
pgsql: Use MemoryContext API for regex memory management.
Use MemoryContext API for regex memory management. Previously, regex_t objects' memory was managed with malloc() and free() directly. Switch to palloc()-based memory management instead. Advantages: * memory used by cached regexes is now visible with MemoryContext observability tools * cleanup can be done automatically in certain failure modes (something that later commits will take advantage of) * cleanup can be done in bulk On the downside, there may be more fragmentation (wasted memory) due to per-regex MemoryContext objects. This is a problem shared with other cached objects in PostgreSQL and can probably be improved with later tuning. Thanks to Noah Misch for suggesting this general approach, which unblocks later work on interrupts. Suggested-by: Noah Misch Reviewed-by: Tom Lane Discussion: https://postgr.es/m/CA%2BhUKGK3PGKwcKqzoosamn36YW-fsuTdOPPF1i_rtEO%3DnEYKSg%40mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/bea3d7e3831fa6a1395eadbad7d97cebc7aa8aee Modified Files -- src/backend/regex/regprefix.c | 2 +- src/backend/utils/adt/regexp.c | 57 ++ src/include/regex/regcustom.h | 6 ++--- 3 files changed, 45 insertions(+), 20 deletions(-)
pgsql: Update tsearch regex memory management.
Update tsearch regex memory management. Now that our regex engine uses palloc(), it's not necessary to set up a special memory context callback to free compiled regexes. The regex has no resources other than the memory that is already going to be freed in bulk. Reviewed-by: Tom Lane Discussion: https://postgr.es/m/CA%2BhUKGK3PGKwcKqzoosamn36YW-fsuTdOPPF1i_rtEO%3DnEYKSg%40mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/4f51429dd7f194e36af32b557ecdce555b5ab51b Modified Files -- src/backend/tsearch/spell.c | 34 +++--- src/include/tsearch/dicts/spell.h | 18 ++ 2 files changed, 13 insertions(+), 39 deletions(-)
pgsql: Revert "Add support for Kerberos credential delegation"
Revert "Add support for Kerberos credential delegation" This reverts commit 3d4fa227bce4294ce1cc214b4a9d3b7caa3f0454. Per discussion and buildfarm, this depends on APIs that seem to not be available on at least one platform (NetBSD). Should be certainly possible to rework to be optional on that platform if necessary but bit late for that at this point. Discussion: https://postgr.es/m/3286097.1680922...@sss.pgh.pa.us Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/3d03b24c350ab060bb223623bdff38835bd7afd0 Modified Files -- contrib/dblink/dblink.c| 127 -- contrib/dblink/expected/dblink.out | 4 +- contrib/postgres_fdw/connection.c | 72 ++ contrib/postgres_fdw/expected/postgres_fdw.out | 19 +- contrib/postgres_fdw/option.c | 6 - contrib/postgres_fdw/sql/postgres_fdw.sql | 3 +- doc/src/sgml/config.sgml | 17 -- doc/src/sgml/dblink.sgml | 5 +- doc/src/sgml/libpq.sgml| 41 --- doc/src/sgml/monitoring.sgml | 9 - doc/src/sgml/postgres-fdw.sgml | 7 +- src/backend/catalog/system_views.sql | 3 +- src/backend/foreign/foreign.c | 1 - src/backend/libpq/auth.c | 13 +- src/backend/libpq/be-gssapi-common.c | 53 src/backend/libpq/be-secure-gssapi.c | 26 +- src/backend/utils/activity/backend_status.c| 1 - src/backend/utils/adt/pgstatfuncs.c| 21 +- src/backend/utils/init/postinit.c | 8 +- src/backend/utils/misc/guc_tables.c| 10 - src/backend/utils/misc/postgresql.conf.sample | 1 - src/include/catalog/pg_proc.dat| 6 +- src/include/libpq/auth.h | 1 - src/include/libpq/be-gssapi-common.h | 3 - src/include/libpq/libpq-be.h | 2 - src/include/utils/backend_status.h | 1 - src/interfaces/libpq/exports.txt | 1 - src/interfaces/libpq/fe-auth.c | 15 +- src/interfaces/libpq/fe-connect.c | 17 -- src/interfaces/libpq/fe-secure-gssapi.c| 23 +- src/interfaces/libpq/libpq-fe.h| 1 - src/interfaces/libpq/libpq-int.h | 2 - src/test/kerberos/Makefile | 3 - src/test/kerberos/t/001_auth.pl| 331 +++-- src/test/perl/PostgreSQL/Test/Utils.pm | 27 -- src/test/regress/expected/rules.out| 11 +- 36 files changed, 136 insertions(+), 755 deletions(-)
pgsql: Try to unbreak MSVC builds for fuzzystrmatch
Try to unbreak MSVC builds for fuzzystrmatch Commit a290378a37 neglrected to add a recipe for MSVC to build the daitch_motokoff.h file. Per buildfarm animal bowerbird. Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/0e9b271890c09ecb60b08e45d377031b2385ebeb Modified Files -- src/tools/msvc/Solution.pm | 10 ++ 1 file changed, 10 insertions(+)
pgsql: Skip \password TAP test on old IPC::Run versions
Skip \password TAP test on old IPC::Run versions IPC::Run versions prior to 0.98 cause the interactive session to time out, so SKIP the test in case these versions are detected (they are within the base requirement for our TAP tests in general). Error reported by the BF and investigation by Tom Lane. Discussion: https://postgr.es/m/414a86bd-986b-48a7-a1e4-eebce5af0...@yesql.se Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/2e57ffe12f6b5c1498f29cb7c0d9e17c797d9da6 Modified Files -- src/test/authentication/t/001_password.pl | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-)
Re: pgsql: Try to unbreak MSVC builds for fuzzystrmatch
Andrew Dunstan writes: > Try to unbreak MSVC builds for fuzzystrmatch > Commit a290378a37 neglrected to add a recipe for MSVC to build the > daitch_motokoff.h file. Oh! Thank you. I'd even made a mental note yesterday "what about MSVC" and then forgot it by the time I'd finished reviewing other aspects of that patch. regards, tom lane
pgsql: Suppress bogus printout during new 035_standby_logical_decoding.
Suppress bogus printout during new 035_standby_logical_decoding.pl test. Our convention for some time has been that successful tests shouldn't print anything on stderr. A stray "diag" call violated that, and for that matter messed up the normal TAP progress display. Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/07690aab46ed5530643942726b27a503995dd835 Modified Files -- src/test/recovery/t/035_standby_logical_decoding.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
pgsql: Try to unbreak MSVC builds for pg_waldump
Try to unbreak MSVC builds for pg_waldump remedy an omission in commit 7d8219a444 Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/bbec50de16d2bf6e1a2878ff0f7e39dbd86ecff8 Modified Files -- src/tools/msvc/Mkvcbuild.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Re: pgsql: Skip \password TAP test on old IPC::Run versions
On 2023-04-08 Sa 09:57, Daniel Gustafsson wrote:
Skip \password TAP test on old IPC::Run versions
IPC::Run versions prior to 0.98 cause the interactive session to time out,
so SKIP the test in case these versions are detected (they are within the
base requirement for our TAP tests in general). Error reported by the BF
and investigation by Tom Lane.
Discussion:https://postgr.es/m/414a86bd-986b-48a7-a1e4-eebce5af0...@yesql.se
Stylistic nitpick: It's not necessary to have 2 evals here:
+ skip "IO::Pty and IPC::Run >= 0.98 required", 1 unless
+ (eval { require IO::Pty; } && eval { $IPC::Run::VERSION >=
'0.98' });
just say "eval { require IO::Pty; return $IPC::Run::VERSION >= '0.98'; }"
cheers
andrew
--
Andrew Dunstan
EDB:https://www.enterprisedb.com
pgsql: Improve indentation of multiline initialization expressions.
Improve indentation of multiline initialization expressions. If a variable has an initialization expression that wraps onto the next line(s), pg_bsd_indent will now indent the continuation lines one stop, instead of aligning them flush with the variable declaration. We've been holding off applying this until the last v16 CF finished, but now it's time. Thomas Munro and Tom Lane Discussion: https://postgr.es/m/20230120013137.7ky7nl4e4zjor...@awork3.anarazel.de Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/064750af4f4ebab9c0d47d502c7ff7e3c9533f9f Modified Files -- src/tools/pg_bsd_indent/args.c | 2 +- src/tools/pg_bsd_indent/io.c | 11 ++- src/tools/pgindent/pgindent| 2 +- 3 files changed, 8 insertions(+), 7 deletions(-)
Re: pgsql: Skip \password TAP test on old IPC::Run versions
Andrew Dunstan writes:
> Stylistic nitpick: It's not necessary to have 2 evals here:
> + skip "IO::Pty and IPC::Run >= 0.98 required", 1 unless
> + (eval { require IO::Pty; } && eval { $IPC::Run::VERSION >=
> '0.98' });
> just say "eval { require IO::Pty; return $IPC::Run::VERSION >= '0.98'; }"
What I was wondering about was if it's safe to depend on the VERSION
being fully numeric. Can't we write "use IPC::Run 0.98;" and let
some other code manage the version comparison?
regards, tom lane
Re: pgsql: Try to unbreak MSVC builds for pg_waldump
On Sat, Apr 8, 2023 at 8:22 AM Andrew Dunstan wrote: > Try to unbreak MSVC builds for pg_waldump > > remedy an omission in commit 7d8219a444 Thanks for taking care of this. I was under the impression that issues like this would be caught by CI. We used to have an MSVC CI target, but that's no longer the case -- not since commit e6b6ea02. I'll need to remember that. -- Peter Geoghegan
Re: pgsql: Try to unbreak MSVC builds for pg_waldump
Peter Geoghegan writes: > I was under the impression that issues like this would be caught by > CI. We used to have an MSVC CI target, but that's no longer the case > -- not since commit e6b6ea02. I'll need to remember that. I thought we were planning to drop the MSVC build scripts for v16. Is that no longer happening? If they're going to be around for awhile, maybe that CI job needs to be resurrected. regards, tom lane
Re: pgsql: Try to unbreak MSVC builds for pg_waldump
On Sat, Apr 8, 2023 at 10:53 AM Tom Lane wrote: > I thought we were planning to drop the MSVC build scripts for v16. > Is that no longer happening? If they're going to be around for > awhile, maybe that CI job needs to be resurrected. It seems as if commit e6b6ea02 had very good reasons to replace the MSVC build scripts with a meson build; it's really slow, and wastes a lot of cycles, while testing much less than meson. So I doubt that we'd want to restore exactly what was in place prior to that commit. If we can't just drop the MSVC build scripts, perhaps we could add something akin to the existing CompilerWarnings target. Something that's purely intended to detect these kinds of oversights. Even that seems excessive to me, though. I hope that we can just get rid of the scripts before long. -- Peter Geoghegan
Re: pgsql: Try to unbreak MSVC builds for pg_waldump
Hi, On 2023-04-08 13:53:34 -0400, Tom Lane wrote: > Peter Geoghegan writes: > > I was under the impression that issues like this would be caught by > > CI. We used to have an MSVC CI target, but that's no longer the case > > -- not since commit e6b6ea02. I'll need to remember that. > > I thought we were planning to drop the MSVC build scripts for v16. > Is that no longer happening? If they're going to be around for > awhile, maybe that CI job needs to be resurrected. I was holding off submitting a patch until the BF supported meson - which only happened recently. I was actually planning to start a thread about the topic in the next few days. As far as I can tell, the meson build has more than feature parity with the "homegrown" build - the only two dependencies I haven't been able to test is building with are ossp-uuid (doesn't build) and kerberos (quite complicated, last release for windows is ages ago) Greetings, Andres Freund
Re: pgsql: Try to unbreak MSVC builds for pg_waldump
On 2023-04-08 Sa 14:13, Andres Freund wrote: Hi, On 2023-04-08 13:53:34 -0400, Tom Lane wrote: Peter Geoghegan writes: I was under the impression that issues like this would be caught by CI. We used to have an MSVC CI target, but that's no longer the case -- not since commit e6b6ea02. I'll need to remember that. I thought we were planning to drop the MSVC build scripts for v16. Is that no longer happening? If they're going to be around for awhile, maybe that CI job needs to be resurrected. I was holding off submitting a patch until the BF supported meson - which only happened recently. I was actually planning to start a thread about the topic in the next few days. As far as I can tell, the meson build has more than feature parity with the "homegrown" build - the only two dependencies I haven't been able to test is building with are ossp-uuid (doesn't build) and kerberos (quite complicated, last release for windows is ages ago) I am still fighting a few issues, was intending to work more on it this week, so I'm not signing off on buildfarm support for meson quite yet. I think we need to support the old system until meson support is fully baked. cheers andrew -- Andrew Dunstan EDB:https://www.enterprisedb.com
Re: pgsql: Skip \password TAP test on old IPC::Run versions
> On 8 Apr 2023, at 18:23, Tom Lane wrote:
>
> Andrew Dunstan writes:
>> Stylistic nitpick: It's not necessary to have 2 evals here:
>
>> + skip "IO::Pty and IPC::Run >= 0.98 required", 1 unless
>> + (eval { require IO::Pty; } && eval { $IPC::Run::VERSION >=
>> '0.98' });
>
>> just say "eval { require IO::Pty; return $IPC::Run::VERSION >= '0.98'; }"
>
> What I was wondering about was if it's safe to depend on the VERSION
> being fully numeric.
Reading documentation online pointed at this being the established way, and
trying to read the Perl5 code it seems to essentially turn whatever is set in
$VERSION into a numeric.
> Can't we write "use IPC::Run 0.98;" and let
> some other code manage the version comparison?
We can, but that AFAIK (Andrew might have a better answer) requires the below
diff which I think leaves some readability to be desired:
- (eval { require IO::Pty; } && eval { $IPC::Run::VERSION >= '0.98' });
+ (eval { require IO::Pty; } && !!eval { IPC::Run->VERSION('0.98'); 1 });
This is needed since ->VERSION die()'s if the version isn't satisfied. That
seems to work fine though, and will ensure that the UNIVERSAL version method
does the version comparison. We can of course expand the comment on why that
construct is needed.
--
Daniel Gustafsson
Re: pgsql: Skip \password TAP test on old IPC::Run versions
Daniel Gustafsson writes:
> On 8 Apr 2023, at 18:23, Tom Lane wrote:
>> Can't we write "use IPC::Run 0.98;" and let
>> some other code manage the version comparison?
> We can, but that AFAIK (Andrew might have a better answer) requires the below
> diff which I think leaves some readability to be desired:
> - (eval { require IO::Pty; } && eval { $IPC::Run::VERSION >= '0.98' });
> + (eval { require IO::Pty; } && !!eval { IPC::Run->VERSION('0.98'); 1 });
Maybe I'm missing something, but I was envisioning
eval { require IO::Pty; use IPC::Run 0.98; }
with no need to do more than check if the eval traps an error.
regards, tom lane
pgsql: Use higher wal_level for 004_io_direct.pl.
Use higher wal_level for 004_io_direct.pl. The new direct I/O test deliberately uses a very small shared_buffers to force some disk transfers without making the data set large and slow, but ran into a problem with wal_level = minimal: log_newpage_range() pins many buffers, leading to a few intermittent "no unpinned buffers available" errors. We could presumably fix that by adjusting shared_buffers, but crake seems to be trying to tell us something interesting with these settings, so let's just avoid wal_level = minimal in this test for now. Reported-by: Andres Freund Discussion: https://postgr.es/m/20230408060408.n7xdwk3mxj5oykt6%40awork3.anarazel.de Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/980e8879f54a7a00ca6a5bae2fe9486c87ef3e8e Modified Files -- src/test/modules/test_misc/t/004_io_direct.pl | 1 + 1 file changed, 1 insertion(+)
Re: pgsql: Skip \password TAP test on old IPC::Run versions
On 2023-04-08 Sa 16:20, Daniel Gustafsson wrote:
On 8 Apr 2023, at 18:23, Tom Lane wrote:
Andrew Dunstan writes:
Stylistic nitpick: It's not necessary to have 2 evals here:
+ skip "IO::Pty and IPC::Run >= 0.98 required", 1 unless
+ (eval { require IO::Pty; } && eval { $IPC::Run::VERSION >=
'0.98' });
just say "eval { require IO::Pty; return $IPC::Run::VERSION >= '0.98'; }"
What I was wondering about was if it's safe to depend on the VERSION
being fully numeric.
Reading documentation online pointed at this being the established way, and
trying to read the Perl5 code it seems to essentially turn whatever is set in
$VERSION into a numeric.
Can't we write "use IPC::Run 0.98;" and let
some other code manage the version comparison?
We can, but that AFAIK (Andrew might have a better answer) requires the below
diff which I think leaves some readability to be desired:
- (eval { require IO::Pty; } && eval { $IPC::Run::VERSION >= '0.98' });
+ (eval { require IO::Pty; } && !!eval { IPC::Run->VERSION('0.98'); 1 });
This is needed since ->VERSION die()'s if the version isn't satisfied. That
seems to work fine though, and will ensure that the UNIVERSAL version method
does the version comparison. We can of course expand the comment on why that
construct is needed.
I still don't understand why you're using two evals here. This should be
sufficient and seems simpler to me:
unless eval { require IO:Pty; IPC::Run->VERSION('0.98'); }
cheers
andrew
--
Andrew Dunstan
EDB:https://www.enterprisedb.com
Re: pgsql: Skip \password TAP test on old IPC::Run versions
On 2023-04-08 Sa 16:30, Tom Lane wrote:
Daniel Gustafsson writes:
On 8 Apr 2023, at 18:23, Tom Lane wrote:
Can't we write "use IPC::Run 0.98;" and let
some other code manage the version comparison?
We can, but that AFAIK (Andrew might have a better answer) requires the below
diff which I think leaves some readability to be desired:
- (eval { require IO::Pty; } && eval { $IPC::Run::VERSION >= '0.98' });
+ (eval { require IO::Pty; } && !!eval { IPC::Run->VERSION('0.98'); 1 });
Maybe I'm missing something, but I was envisioning
eval { require IO::Pty; use IPC::Run 0.98; }
with no need to do more than check if the eval traps an error.
You need to be careful with "use". It is executed in the compile phase,
so I'd avoid it here.
cheers
andrew
--
Andrew Dunstan
EDB:https://www.enterprisedb.com
Re: pgsql: Skip \password TAP test on old IPC::Run versions
> On 8 Apr 2023, at 22:37, Andrew Dunstan wrote:
> On 2023-04-08 Sa 16:20, Daniel Gustafsson wrote:
>>> On 8 Apr 2023, at 18:23, Tom Lane
>>> wrote:
>>>
>>> Andrew Dunstan
>>>
>>> writes:
>>>
Stylistic nitpick: It's not necessary to have 2 evals here:
+ skip "IO::Pty and IPC::Run >= 0.98 required", 1 unless
+ (eval { require IO::Pty; } && eval { $IPC::Run::VERSION >=
'0.98' });
just say "eval { require IO::Pty; return $IPC::Run::VERSION >= '0.98'; }"
>>> What I was wondering about was if it's safe to depend on the VERSION
>>> being fully numeric.
>>>
>> Reading documentation online pointed at this being the established way, and
>> trying to read the Perl5 code it seems to essentially turn whatever is set in
>> $VERSION into a numeric.
>>
>>
>>> Can't we write "use IPC::Run 0.98;" and let
>>> some other code manage the version comparison?
>>>
>> We can, but that AFAIK (Andrew might have a better answer) requires the below
>> diff which I think leaves some readability to be desired:
>>
>> - (eval { require IO::Pty; } && eval { $IPC::Run::VERSION >= '0.98' });
>> + (eval { require IO::Pty; } && !!eval { IPC::Run->VERSION('0.98'); 1 });
>>
>> This is needed since ->VERSION die()'s if the version isn't satisfied. That
>> seems to work fine though, and will ensure that the UNIVERSAL version method
>> does the version comparison. We can of course expand the comment on why that
>> construct is needed.
>
> I still don't understand why you're using two evals here. This should be
> sufficient and seems simpler to me:
>
> unless eval { require IO:Pty; IPC::Run->VERSION('0.98'); }
Because I was trying to get "use IPC::Run" to work and that required multiple
evals, and got stuck on that track. I agree that your version is better and
does indeed work (and offloads version comparison to the UNIVERSAL class), so
I'll go ahead with the below.
- (eval { require IO::Pty; } && eval { $IPC::Run::VERSION >= '0.98' });
+ eval { require IO::Pty; IPC::Run->VERSION('0.98'); };
--
Daniel Gustafsson
pgsql: Simplify version check for SKIP clause
Simplify version check for SKIP clause Checking for the required versions of IO::Pty as well as IPC::Run can be achieved with a single eval call, and by using the VERSION function the comparison is guaranteed to follow the same rules as calling 'use' on the module with a version. Reported-by: Andrew Dunstan Discussion: https://postgr.es/m/6d880ea2-f8ca-f458-4dcd-a7a3e6d6c...@dunslane.net Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/6ff2e8cdd410f70057cfa6259ad395c1119aeb32 Modified Files -- src/test/authentication/t/001_password.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
