The Msys shell mangles certain patterns in its command line, so avoid
handing arbitrary SQL to psql on the command line and instead use
IPC::Run's redirection facility for stdin. This pattern is already
mostly whats used, but query_poll_until() was not doing the right thing.
Problem discovered on the buildfarm when a new TAP test failed on msys.
Various environment variables were not getting reset in the TAP tests,
which would cause failures depending on the tests or the environment
variables involved. For example, PGSSL{MAX,MIN}PROTOCOLVERSION could
cause failures in the SSL tests. Even worse, a junk value of
PGCLIENTENCODING makes a server startup fail. The list of variables
reset is adjusted in each stable branch depending on what is supported.
While on it, simplify a bit the code per a suggestion from Andrew
Dunstan, using a list of variables instead of doing single deletions.
Reviewed-by: Andrew Dunstan, Daniel Gustafsson
Discussion: https://postgr.es/m/YLbjjRpucIeZ78VQ@paquier.xyz
Backpatch-through: 9.6
A lamentable oversight on my part meant that when PostgresVersion.pm was
added in commit 4c4eaf3d19 provision to install it was not added to the
Makefile, so it was not installed along with the other perl modules.
Older versions of perl on Windows don't like the list form of pipe open,
and perlcritic doesn't like the string form of open, so we avoid both
with a simpler formulation using qx{}.
Per complaint from Amit Kapila.
Also "make reformat-dat-files".
The only change worthy of note is that pgindent messed up the formatting
of launcher.c's struct LogicalRepWorkerId, which led me to notice that
that struct wasn't used at all anymore, so I just took it out.
Handle the situation where perl swaps the order of operands of
the comparison operator. See `perldoc overload` for details:
The third argument is set to TRUE if (and only if) the two
operands have been swapped. Perl may do this to ensure that the
first argument ($self) is an object implementing the overloaded
operation, in line with general object calling conventions.
As well as 'devel' version_stamp.pl provides for 'alphaN'
'betaN' and 'rcN', so teach PostgresVersion about those.
Also stash the version string instead of trying to reconstruct it during
stringification.
Discussion: https://postgr.es/m/YIHlw5nSgAHs4dK1@paquier.xyz
A new PostgresVersion object type is created and this is used in
PostgresNode using the output of `pg_config --version` and the result
stored in the PostgresNode object. This object can be compared to other
PostgresVersion objects, or to a number or string.
PostgresNode is currently believed to be compatible with versions down
to release 12, so PostgresNode will issue a warning if used with a
version prior to that.
No attempt has been made to deal with incompatibilities in older
versions - that remains work to be undertaken in a subsequent
development cycle.
Based on code from Mark Dilger and Jehan-Guillaume de Rorthais.
Discussion: https://postgr.es/m/a80421c0-3d7e-def1-bcfe-24777f15e344@dunslane.net
Commit b34ca595ab provided for installation-aware instances of
PostgresNode. However, it turns out that IPC::Run works against this by
caching the path to a binary and not consulting the path again, even if
it has changed. We work around this by calling Postgres binaries with
the installed path rather than just a bare name to be looked up in the
environment path, if there is an installed path. For the common case
where there is no installed path we continue to use the bare command
name.
Diagnosis and solution from Mark Dilger
Discussion: https://postgr.es/m/E8F512F8-B4D6-4514-BA8D-2E671439DA92@enterprisedb.com
In order to avoid getting old logfile contents certain functions in
PostgresNode were doing one of two things. On Windows it rotated the
logfile and restarted the server, while elsewhere it truncated the log
file. Both of these are unnecessary. We borrow from the buildfarm which
does this instead: note the size of the logfile before we start, and
then when fetching the logfile skip to that position before accumulating
contents. This is spelled differently on Windows but the effect is the
same. This is largely centralized in TestLib's slurp_file function,
which has a new optional parameter, the offset to skip to before
starting to reading the file. Code in the client becomes much neater.
Backpatch to all live branches.
Michael Paquier, slightly modified by me.
Discussion: https://postgr.es/m/YHajnhcMAI3++pJL@paquier.xyz
The truncation of the log file, that this set of tests relies on to make
sure that a connection attempt matches with its expected backend log
pattern, fails, as reported by buildfarm member fairywren. Instead of a
truncation, do a rotation of the log file and restart the node. This
will ensure that the connection attempt data is unique for each test.
Discussion: https://postgr.es/m/YG05nCI8x8B+Ad3G@paquier.xyz
The "authenticated identity" is the string used by an authentication
method to identify a particular user. In many common cases, this is the
same as the PostgreSQL username, but for some third-party authentication
methods, the identifier in use may be shortened or otherwise translated
(e.g. through pg_ident user mappings) before the server stores it.
To help administrators see who has actually interacted with the system,
this commit adds the capability to store the original identity when
authentication succeeds within the backend's Port, and generates a log
entry when log_connections is enabled. The log entries generated look
something like this (where a local user named "foouser" is connecting to
the database as the database user called "admin"):
LOG: connection received: host=[local]
LOG: connection authenticated: identity="foouser" method=peer (/data/pg_hba.conf:88)
LOG: connection authorized: user=admin database=postgres application_name=psql
Port->authn_id is set according to the authentication method:
bsd: the PostgreSQL username (aka the local username)
cert: the client's Subject DN
gss: the user principal
ident: the remote username
ldap: the final bind DN
pam: the PostgreSQL username (aka PAM username)
password (and all pw-challenge methods): the PostgreSQL username
peer: the peer's pw_name
radius: the PostgreSQL username (aka the RADIUS username)
sspi: either the down-level (SAM-compatible) logon name, if
compat_realm=1, or the User Principal Name if compat_realm=0
The trust auth method does not set an authenticated identity. Neither
does clientcert=verify-full.
Port->authn_id could be used for other purposes, like a superuser-only
extra column in pg_stat_activity, but this is left as future work.
PostgresNode::connect_{ok,fails}() have been modified to let tests check
the backend log files for required or prohibited patterns, using the
new log_like and log_unlike parameters. This uses a method based on a
truncation of the existing server log file, like issues_sql_like().
Tests are added to the ldap, kerberos, authentication and SSL test
suites.
Author: Jacob Champion
Reviewed-by: Stephen Frost, Magnus Hagander, Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/c55788dd1773c521c862e8e0dddb367df51222be.camel@vmware.com
This type of failure is similar to what has been fixed in c757a3da,
where an authentication failure combined with psql pushing a command
down its communication pipe causes a test failure. This routine is
designed to fail, so sending a query has little sense anyway.
Per buildfarm members gaur and hoverfly, based on an analysis and fix
from Tom Lane.
Discussion: https://postgr.es/m/513200.1617634642@sss.pgh.pa.us
This commit refactors more TAP tests to adapt with the recent
introduction of connect_ok() and connect_fails() in PostgresNode,
introduced by 0d1a3343. This changes the following test suites to use
the same code paths for connection checks:
- Kerberos
- LDAP
- SSL
- Authentication
Those routines are extended to be able to handle optional parameters
that are set depending on each suite's needs, as of:
- custom SQL query.
- expected stderr matching pattern.
- expected stdout matching pattern.
The new design is extensible with more parameters, and there are some
plans for those routines in the future with checks based on the contents
of the backend logs.
Author: Jacob Champion, Michael Paquier
Discussion: https://postgr.es/m/d17b919e27474abfa55d97786cb9cfadfe2b59e9.camel@vmware.com
test_connect_ok() and test_connect_fails() have always been part of the
SSL tests, and check if a connection to the backend should work or not,
and there are sanity checks done on specific error patterns dropped by
libpq if the connection fails.
This was fundamentally wrong on two aspects. First, SSLServer.pm works
mostly on setting up and changing the SSL configuration of a
PostgresNode, and has really nothing to do with the client. Second,
the situation became worse in light of b34ca595, where the SSL tests
would finish by using a psql command that may not come from the same
installation as the node set up.
This commit moves those client routines into PostgresNode, making easier
the refactoring of SSLServer to become more SSL-implementation aware.
This can also be reused by the ldap, kerberos and authentication test
suites for connection checks, and a follow-up patch should extend those
interfaces to match with backend log patterns.
Author: Michael Paquier
Reviewed-by: Andrew Dunstan, Daniel Gustafsson, Álvaro Herrera
Discussion: https://postgr.es/m/YGLKNBf9zyh6+WSt@paquier.xyz
Currently instances of PostgresNode find their Postgres executables in
the PATH of the caller. This modification allows for instances that know
the installation path they are supposed to use, and the module adjusts
the environment of methods that call Postgres executables appropriately.
This facility is activated by passing the installation path to the
constructor:
my $node = PostgresNode->get_new_node('mynode',
installation_path => '/path/to/installation');
This makes a number of things substantially easier, including
. testing third party modules
. testing different versions of postgres together
. testing different builds of postgres together
Discussion: https://postgr.es/m/a94c74f9-6b71-1957-7973-a734ea3cbef1@dunslane.net
Reviewed-By: Alvaro Herrera, Michael Paquier, Dagfinn Ilmari Mannsåker
The existing test script does run pg_basebackup with the -Ft option,
but it makes no real attempt to verify the sanity of the results.
We wouldn't know if the output is incompatible with standard "tar"
programs, nor if the server fails to start from the restored output.
Notably, this means that xlog.c's read_tablespace_map() is not being
meaningfully tested, since that code is used only in the tar-format
case. (We do have reasonable coverage of restoring from plain-format
output, though it's over in src/test/recovery not here.)
Hence, attempt to untar the output and start a server from it,
rather just hoping it's OK.
This test assumes that the local "tar" has the "-C directory"
switch. Although that's not promised by POSIX, my research
suggests that all non-extinct tar implementations have it.
Should the buildfarm's opinion differ, we can complicate the
test a bit to avoid requiring that.
Possibly this should be back-patched, but I'm unsure about
whether it could work on Windows before d66b23b03.
Commit 151c0c5f7 neglected the possibility that a TEMP_CONFIG file
would explicitly set max_wal_senders=0; as indeed buildfarm member
thorntail does, so that it can test wal_level=minimal in other test
suites. Hence, rather than assuming that max_wal_senders=10 will
prevail if we say nothing, set it explicitly.
Set max_replication_slots=10 explicitly too, just to be safe.
Back-patch to v10, like the previous patch.
Discussion: https://postgr.es/m/723911.1601417626@sss.pgh.pa.us
PostgresNode.pm set "max_wal_senders = 5" for replication testing,
but this seems to be slightly too low for our current test suite.
Slower buildfarm members frequently report "number of requested standby
connections exceeds max_wal_senders" failures, due to old walsenders
not exiting instantaneously. Usually, the test does not fail overall
because of automatic walreceiver restart, but sometimes the failure
becomes visible; and in any case such retries slow down the test.
That value came in with commit 89ac7004d, but was soon obsoleted by
f6d6d2920, which raised the built-in default from zero to 10; so that
PostgresNode.pm is actually setting it to less than the conservative
built-in default. That seems pretty pointless, so let's remove the
special setting and let the default prevail, in hopes of making
the TAP tests more robust.
Likewise, the setting "max_replication_slots = 5" is obsolete and
can be removed.
While here, reverse-engineer a comment about why we're choosing
less-than-default values for some other settings.
(Note: before v12, max_wal_senders counted against max_connections
so that the latter setting also needs some fiddling with.)
Back-patch to v10 where the subscription tests were added.
It's likely that the older branches aren't pushing the boundaries
of max_wal_senders, but I'm disinclined to spend time trying to
figure out exactly when it started to be a problem.
Discussion: https://postgr.es/m/723911.1601417626@sss.pgh.pa.us
Reuse cautionary language from src/test/ssl/README in
src/test/kerberos/README. SLRUs have had access to six-character
segments names since commit 73c986adde,
and recovery stopped calling HeapTupleHeaderAdvanceLatestRemovedXid() in
commit 558a9165e0. The other corrections
are more self-evident.
Windows has junction points which function as symbolic links for
directories. This patch introduces a new function TestLib::dir_symlink()
which creates a junction point on Windows and a standard Unix type
symbolic link elsewhere.
The function TestLib::perl2host is also modified, first to use cygpath
where it's available (e.g. msys2) and second to allow it to succeed if
the gandparent directory exists but the parent does not.
Given these changes the only symlink tests that need to be skipped on
Windows are those related to permissions or to use of readlink. The
relevant tests for pg_basebackup and pg_rewind are therefore adjusted
accordingly.
Andrew Dunstan, reviewed by Peter Eisentraut and Michael Paquier.
Discussion: https://postgr.es/m/c50a646c-d9bb-7c62-a4bf-8256ff6ff338@2ndquadrant.com
Buildfarm results now imply that Perl's IPC::Run does CRLF conversion
for us if we're using native Perl, but not when using MSys Perl.
Restrict the conversions done by PostgresNode.pm to act only in the
latter case. (Similar conversions done in TestLib.pm and RewindTest.pm
were already handled this way.)
Discussion: https://postgr.es/m/412ae8da-76bb-640f-039a-f3513499e53d@gmx.net
The previous approach was to search-and-destroy all \r occurrences
no matter what. That seems more likely to hide bugs than anything
else; indeed it seems to be hiding one now. Fix things so that
we only transform \r\n to \n.
Side effects: must do this before, not after, chomp'ing if we're
going to chomp, else we'd fail to clean up a trailing \r\n. Also,
remove safe_psql's redundant repetition of what psql already did;
else it might reduce \r\r\n to \n, which is exactly the scenario
I'm hoping to expose.
Perhaps this should be back-patched, but for now I'm content to
see what happens in HEAD.
Discussion: https://postgr.es/m/412ae8da-76bb-640f-039a-f3513499e53d@gmx.net
In commit 850196b610 I (Álvaro) failed to handle the case of walsender
shutting down on an error before setting up its 'xlogreader' pointer;
the error handling code dereferences the pointer, causing a crash.
Fix by testing the pointer before trying to dereference it.
Kyotaro authored the code fix; I adopted Nathan's test case to be used
by the TAP tests and added the necessary PostgresNode change.
Reported-by: Nathan Bossart <bossartn@amazon.com>
Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/C04FC24E-903D-4423-B312-6910E4D846E5@amazon.com
Includes some manual cleanup of places that pgindent messed up,
most of which weren't per project style anyway.
Notably, it seems some people didn't absorb the style rules of
commit c9d297751, because there were a bunch of new occurrences
of function calls with a newline just after the left paren, all
with faulty expectations about how the rest of the call would get
indented.
We've had a mixture of the warnings pragma, the -w switch on the shebang
line, and no warnings at all. This patch removes the -w swicth and add
the warnings pragma to all perl sources missing it. It raises the
severity of the TestingAndDebugging::RequireUseWarnings perlcritic
policy to level 5, so that we catch any future violations.
Discussion: https://postgr.es/m/20200412074245.GB623763@rfd.leadboat.com
The test suites currently don't use Unix-domain sockets on Windows.
This optionally allows enabling that by setting the environment
variable PG_TEST_USE_UNIX_SOCKETS.
This should currently be considered experimental. In particular,
pg_regress.c contains some comments that the cleanup code for
Unix-domain sockets doesn't work correctly under Windows, which hasn't
been an problem until now. But it's good enough for locally
supervised testing of the functionality.
Reviewed-by: Andrew Dunstan <andrew.dunstan@2ndquadrant.com>
Discussion: https://www.postgresql.org/message-id/flat/54bde68c-d134-4eb8-5bd3-8af33b72a010@2ndquadrant.com
PostgresNode already retained base directories in such cases. Stop
using $SIG{__DIE__}, which is redundant with the exit status check, in
lieu of proliferating it to TestLib. Back-patch to 9.6, where commit
88802e0680 introduced retention on
failure.
Reviewed by Daniel Gustafsson.
Discussion: https://postgr.es/m/20200202170155.GA3264196@rfd.leadboat.com
Before, if a recovery target is configured, but the archive ended
before the target was reached, recovery would end and the server would
promote without further notice. That was deemed to be pretty wrong.
With this change, if the recovery target is not reached, it is a fatal
error.
Based-on-patch-by: Leif Gunnar Erlandsen <leif@lako.no>
Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/993736dd3f1713ec1f63fc3b653839f5@lako.no
Up to now, psql's tab-complete.c has had exactly no regression test
coverage. This patch is an experimental attempt to add some.
This needs Perl's IO::Pty module, which isn't installed everywhere,
so the test script just skips all tests if that's not present.
There may be other portability gotchas too, so I await buildfarm
results with interest.
So far this just covers a few very basic keyword-completion and
query-driven-completion scenarios, which should be enough to let us
get a feel for whether this is practical at all from a portability
standpoint. If it is, there's lots more that can be done.
Discussion: https://postgr.es/m/10967.1577562752@sss.pgh.pa.us
This commit revert the commits to add a test case that tests the 'force'
option when there is an active backend connected to the database being
dropped.
This feature internally sends SIGTERM to all the backends connected to the
database being dropped and then the same is reported to the client. We
found that on Windows, the client end of the socket is not able to read
the data once we close the socket in the server which leads to loss of
error message which is not what we expect. We also observed similar
behavior in other cases like pg_terminate_backend(),
pg_ctl kill TERM <pid>. There are probably a few others like that. The
fix for this requires further study.
Discussion: https://postgr.es/m/E1iaD8h-0004us-K9@gemulon.postgresql.org
The subroutine pump_until provides the functionality to poll until the
given string is matched, or a timeout occurs. This can be used from other
places as well, so moving it to TestLib.pm. The immediate need is for an
upcoming regression test patch for dropdb utility.
Author: Vignesh C
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/CAP_rwwmLJJbn70vLOZFpxGw3XD7nLB_7+NKz46H5EOO2k5H7OQ@mail.gmail.com
Commit 114541d58e has upset some buildfarm members running Msys, that
weren't previously having problems, so the use of native Windows methods
to open files is restricted by this patch to only MSVC builds.
Commit 9af34f3c6b naively assumed that all non-windows platforms would
have pseudoterminals and that perl would have IO::Pty. Such is not the
case. Instead of assumung anything, test for the presence of IO::Pty and
only use code that might depend on it if it's present. The test result is
exposed in $TestLib::have_io_pty so that it can be conveniently used in
SKIP tests.
Discussion: https://postgr.es/m/20191126044110.GB5435@paquier.xyz
Where possible, do this using a pseudoterminal, so that things like
openssl that want to open /dev/tty if stdin isn't a tty won't.
Elsewhere, i.e. Windows, just close by providing an empty string using
the standard IPC::Run pipe mechanism.
Patch by Andrew Dunstan, based on an idea from Craig Ringer.
Reviewed by Mark Dilger.
Discussion: https://postgr.es/m/873ebb57-fc98-340d-949d-691b1810bf66@2ndQuadrant.com
It is hoped that this will avoid some errors that we have seen before,
but lately with greater frequency, in buildfarm animals.
For now apply only to master. If this proves effective it can be
backpatched.
Discussion: https://postgr.es/m/13900.1572839580@sss.pgh.pa.us
Author: Juan José Santamaría Flecha
Andrew Dunstan
Michael Paquier
Noah Misch
Tom Lane
Bruce Momjian
Peter Eisentraut
Alvaro Herrera
Andres Freund
Amit Kapila