Both client-side compression and server-side compression are now
supported for zstd. In addition, a backup compressed by the server
using zstd can now be decompressed by the client in order to
accommodate the use of -Fp.
Jeevan Ladhe, with some edits by me.
Discussion: http://postgr.es/m/CA+Tgmobyzfbz=gyze2_LL1ZumZunmaEKbHQxjrFkOR7APZGu-g@mail.gmail.com
Slow hosts may avoid load-induced, spurious failures by setting
environment variable PG_TEST_TIMEOUT_DEFAULT to some number of seconds
greater than 180. Developers may see faster failures by setting that
environment variable to some lesser number of seconds. In tests, write
$PostgreSQL::Test::Utils::timeout_default wherever the convention has
been to write 180. This change raises the default for some briefer
timeouts. Back-patch to v10 (all supported versions).
Discussion: https://postgr.es/m/20220218052842.GA3627003@rfd.leadboat.com
Commit 52e4f0cd47 didn't add tests for pg_dump support, so add a few tests
for it. Additionally, verify that catalogs are updated after few
ALTER PUBLICATION commands that modify row filters by using \d.
Reported-by: Tomas Vondra
Author: Shi yu, based on initial by Tomas Vondra
Reviewed-by: Euler Taveira and Amit Kapila
Discussion: https://postgr.es/m/6bdbd7fc-e81a-9a77-d963-24adeb95f29e@enterprisedb.com
The checks currently done at the startup of pg_upgrade on PGHOST and
PGHOSTADDR to avoid any attempts to access to an external cluster fail
setting those parameters to Windows paths or even temporary paths
prefixed by an '@', as it only considers as a valid path strings
beginning with a slash.
As mentioned by Andres, is_unixsock_path() is designed to detect such
cases, so, like any other code paths dealing with the same problem (psql
and libpq), use it rather than assuming that all valid paths are
prefixed with just a slash.
This issue has been found while testing the TAP tests of pg_upgrade
through the CI on Windows. This is a bug, but nobody has complained
about it since pg_upgrade exists so no backpatch is done, at least for
now.
Analyzed-by: Andres Freund, Michael Paquier
Discussion: https://postgr.es/m/YeYj4DU5qY/rtKXT@paquier.xyz
pg_rewind generates and executes internally up to two commands to work
on the target cluster, depending on the options given by its caller:
- postgres -C to retrieve the value of restore_command, when using
-c/--restore-target-wal.
- postgres --single to enforce recovery once and get the target cluster
in a clean shutdown state.
Both commands have been applying incorrect quoting rules, which could
lead to failures when for example using a target data directory with
unexpected characters like CRLFs. Those commands are now generated with
PQExpBuffer, making use of string_utils.h to quote those commands as
they should. We may extend those commands in the future with more
options, so this makes any upcoming additions easier.
This is arguably a bug fix, but nobody has complained about the existing
code being a problem either, so no backpatch is done.
Extracted from a larger patch by the same author.
Author: Gunnar "Nick" Bluth
Discussion: https://postgr.es/m/7c59265d-ac50-b0aa-ca1e-65e8bd27642a@pro-open.de
When opening a WAL file smaller than XLOG_BLCKSZ (e.g. 0 bytes long) while
determining the wal_segment_size, pg_waldump checked errno, despite errno not
being set by the short read. Resulting in a bogus error message.
Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://postgr.es/m/20220214.181847.775024684568733277.horikyota.ntt@gmail.com
Backpatch: 11-, the bug was introducedin fc49e24fa
If the log streaming child process (thread on Windows) dies during
backup then the whole backup will be aborted at the end of the
backup. Instead, trap ungraceful termination of the log streaming
child and exit early. This also adds a TAP test for simulating this
by terminating the responsible backend.
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Magnus Hagander <magnus@hagander.net>
Discussion: https://postgr.es/m/0F69E282-97F9-4DB7-8D6D-F927AA6340C8@yesql.se
Discussion: https://postgr.es/m/VI1PR83MB0189818B82C19059CB62E26199A89@VI1PR83MB0189.EURPRD83.prod.outlook.com
Commit 61081e75c introduced pg_rewind along with the test suite, which
ensured that subroutines didn't incur more than one test to plan. Now
that we no longer explicitly plan tests (since 549ec201d), we can use
the usual Test::More functions.
Reviewed-by: Andrew Dunstan <andrew@dunslane.net>
Discussion: https://postgr.es/m/AA527525-F0CC-4AA2-AF98-543CABFDAF59@yesql.se
I have not been able to reproduce the occasional failures of
019_replslot_limit.pl we are seeing in the buildfarm and not for lack of
trying. The additional logging and increased log level will hopefully help.
Will be reverted once the cause is identified.
Discussion: https://postgr.es/m/20220218231415.c4plkp4i3reqcwip@alap3.anarazel.de
This feature adds row filtering for publication tables. When a publication
is defined or modified, an optional WHERE clause can be specified. Rows
that don't satisfy this WHERE clause will be filtered out. This allows a
set of tables to be partially replicated. The row filter is per table. A
new row filter can be added simply by specifying a WHERE clause after the
table name. The WHERE clause must be enclosed by parentheses.
The row filter WHERE clause for a table added to a publication that
publishes UPDATE and/or DELETE operations must contain only columns that
are covered by REPLICA IDENTITY. The row filter WHERE clause for a table
added to a publication that publishes INSERT can use any column. If the
row filter evaluates to NULL, it is regarded as "false". The WHERE clause
only allows simple expressions that don't have user-defined functions,
user-defined operators, user-defined types, user-defined collations,
non-immutable built-in functions, or references to system columns. These
restrictions could be addressed in the future.
If you choose to do the initial table synchronization, only data that
satisfies the row filters is copied to the subscriber. If the subscription
has several publications in which a table has been published with
different WHERE clauses, rows that satisfy ANY of the expressions will be
copied. If a subscriber is a pre-15 version, the initial table
synchronization won't use row filters even if they are defined in the
publisher.
The row filters are applied before publishing the changes. If the
subscription has several publications in which the same table has been
published with different filters (for the same publish operation), those
expressions get OR'ed together so that rows satisfying any of the
expressions will be replicated.
This means all the other filters become redundant if (a) one of the
publications have no filter at all, (b) one of the publications was
created using FOR ALL TABLES, (c) one of the publications was created
using FOR ALL TABLES IN SCHEMA and the table belongs to that same schema.
If your publication contains a partitioned table, the publication
parameter publish_via_partition_root determines if it uses the partition's
row filter (if the parameter is false, the default) or the root
partitioned table's row filter.
Psql commands \dRp+ and \d <table-name> will display any row filters.
Author: Hou Zhijie, Euler Taveira, Peter Smith, Ajin Cherian
Reviewed-by: Greg Nancarrow, Haiying Tang, Amit Kapila, Tomas Vondra, Dilip Kumar, Vignesh C, Alvaro Herrera, Andres Freund, Wei Wang
Discussion: https://www.postgresql.org/message-id/flat/CAHE3wggb715X%2BmK_DitLXF25B%3DjE6xyNCH4YOwM860JR7HarGQ%40mail.gmail.com
Until this change pg_upgrade with output redirected to a file / pipe would end
up printing all files in the cluster. This has made check-world output
exceedingly verbose.
Author: Andres Freund <andres@anarazel.de>
Reviewed-By: Justin Pryzby <pryzby@telsasoft.com>
Reviewed-By: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/CA+hUKGKjrV61ZVJ8OSag+3rKRmCZXPc03bDyWMqhXg3rdZ=fOw@mail.gmail.com
Following migration of Windows buildfarm members running TAP tests to
use of ucrt64 perl for those tests, special processing for msys perl is
no longer necessary and so is removed.
Backpatch to release 10
Discussion: https://postgr.es/m/c65a8781-77ac-ea95-d185-6db291e1baeb@dunslane.net
Ill-considered refactoring in 23a1c6578 led to progress_filename
sometimes pointing to data that had gone out of scope. The most
bulletproof fix is to hang onto a copy of whatever's passed in.
Compared to the work spent elsewhere per file, that's not very
expensive, plus we can skip it except in verbose logging mode.
Per buildfarm.
Discussion: https://postgr.es/m/20220212211316.GK31460@telsasoft.com
38bfae3 has mixed the "dump/" and "log/" subdirectories generated in
"pg_upgrade_output.d/", causing the internal dump files to be generated
in "log/" and the log files to be in "dump/", but the opposite should be
done. This was not directly an issue for pg_upgrade runs, as the
internal dump files were still picked up at the location of their
creation, but the newest version of the buildfarm client would have
reported the dump files instead of the log files on failures of
pg_upgrade.
Issue spotted while testing the TAP tests of pg_upgrade.
Move scanint8() to numutils.c and rename to pg_strtoint64(). We
already have a "16" and "32" version of that, and the code inside the
functions was aligned, so this move makes all three versions
consistent. The API is also changed to no longer provide the errorOK
case. Users that need the error checking can use strtoi64().
Reviewed-by: John Naylor <john.naylor@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/flat/b239564c-cad0-b23e-c57e-166d883cb97d@enterprisedb.com
This adds to database objects the same version tracking that collation
objects have. There is a new pg_database column datcollversion that
stores the version, a new function
pg_database_collation_actual_version() to get the version from the
operating system, and a new subcommand ALTER DATABASE ... REFRESH
COLLATION VERSION.
This was not originally added together with pg_collation.collversion,
since originally version tracking was only supported for ICU, and ICU
on a database-level is not currently supported. But we now have
version tracking for glibc (since PG13), FreeBSD (since PG14), and
Windows (since PG13), so this is useful to have now.
Reviewed-by: Julien Rouhaud <rjuju123@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/f0ff3190-29a3-5b39-a179-fa32eee57db6%40enterprisedb.com
Some environments may compile with --with-lz4 while the command "lz4"
goes missing, causing two failures in the TAP tests of pg_verifybackup
(008_untar.pl and 010_client_untar.pl) as the code assumed that the
command always existed with a hardcoded value in src/Makefile.global.
Rather than this method, this adds a ./configure check based on
PGAC_PATH_PROGS() to find automatically the command and get an absolute
path to it.
Both tests need to be adjusted for the case where the command does not
exist, actually, as Makefile.global would set now LZ4 to an empty value
in this case. The TAP tests of pg_receivewal already do that.
Per report from buildfarm member copperhead, as an effect of dab2984.
The origin of the failure is actually babbbb5 that did not centralize
the check for the existence of a "lz4" command at ./configure to shave a
few cycles. Note that one just needs to tweak an environment to move
"lz4" out of the way to reproduce the problem, which is what I did to
test this change.
Per discussion with Robert Haas, Tom Lane, Andres Freund and myself.
Discussion: https://postgr.es/m/Ygc51WVAFGocSu4h@paquier.xyz
Depending on compiler version and optimization level, we might
get a complaint that lazy_scan_heap's "freespace" is used
uninitialized.
Compilers not aware that ereport(ERROR) doesn't return complained
about bbsink_lz4_new().
Assigning "-1" to a uint64 value has unportable results; fortunately,
the value of xlogreadsegno is unimportant when xlogreadfd is -1.
(It looks to me like there is no need for xlogreadsegno to be static
in the first place, but I didn't venture to change that.)
Rather than doing manual book keeping to plan the number of tests to run
in each TAP suite, conclude each run with done_testing() summing up the
the number of tests that ran. This removes the need for maintaning and
updating the plan count at the expense of an accurate count of remaining
during the test suite runtime.
This patch has been discussed a number of times, often in the context of
other patches which updates tests, so a larger number of discussions can
be found in the archives.
Reviewed-by: Julien Rouhaud <rjuju123@gmail.com>
Reviewed-by: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://postgr.es/m/DD399313-3D56-4666-8079-88949DAC870F@yesql.se
LZ4 compression can now be performed on the client using
pg_basebackup -Ft --compress client-lz4, and LZ4 decompression of
a backup compressed on the server can be performed on the client
using pg_basebackup -Fp --compress server-lz4.
Dipesh Pandit, reviewed and tested by Jeevan Ladhe and Tushar Ahuja,
with a few corrections - and some documentation - by me.
Discussion: http://postgr.es/m/CAN1g5_FeDmiA9D8wdG2W6Lkq5CpubxOAqTmd2et9hsinTJtsMQ@mail.gmail.com
LZ4 compression can be a lot faster than gzip compression, so users
may prefer it even if the compression ratio is not as good. We will
want pg_basebackup to support LZ4 compression and decompression on the
client side as well, and there is a pending patch for that, but it's
by a different author, so I am committing this part separately for
that reason.
Jeevan Ladhe, reviewed by Tushar Ahuja and by me.
Discussion: http://postgr.es/m/CANm22Cg9cArXEaYgHVZhCnzPLfqXCZLAzjwTq7Fc0quXRPfbxA@mail.gmail.com
"pg_ctl stop/restart" checked that the postmaster PID is valid just
once, as a side-effect of sending the stop signal, and then would
wait-till-timeout for the postmaster.pid file to go away. This
neglects the case wherein the postmaster dies uncleanly after we
signal it. Similarly, once "pg_ctl promote" has sent the signal,
it'd wait for the corresponding on-disk state change to occur
even if the postmaster dies.
I'm not sure how we've managed not to notice this problem, but it
seems to explain slow execution of the 017_shm.pl test script on AIX
since commit 4fdbf9af5, which added a speculative "pg_ctl stop" with
the idea of making real sure that the postmaster isn't there. In the
test steps that kill-9 and then restart the postmaster, it's possible
to get past the initial signal attempt before kill() stops working
for the doomed postmaster. If that happens, pg_ctl waited till
PGCTLTIMEOUT before giving up ... and the buildfarm's AIX members
have that set very high.
To fix, include a "kill(pid, 0)" test (similar to what
postmaster_is_alive uses) in these wait loops, so that we'll
give up immediately if the postmaster PID disappears.
While here, I chose to refactor those loops out of where they were.
do_stop() and do_restart() can perfectly well share one copy of the
wait-for-stop loop, and it seems desirable to put a similar function
beside that for wait-for-promote.
Back-patch to all supported versions, since pg_ctl's wait logic
is substantially identical in all, and we're seeing the slow test
behavior in all branches.
Discussion: https://postgr.es/m/20220210023537.GA3222837@rfd.leadboat.com
The behavior I proposed, of matching case only when only keywords
are available to complete, turns out to be too cute. It adds about
as many problems as it removes. Simplify down to ilmari's original
proposal of just always matching case when completing a keyword.
Also, I noticed while testing this that we've pessimized the behavior
for qualified GUC names: the code is insisting that they be
double-quoted, which was not the case before. Fix that by treating
GUC names as verbatim matches instead of possibly-schema-qualified
names. (While it's tempting to try to split qualified GUC names
so that we *could* treat them with the schema-qualified-name code
path, that really isn't going to work in light of guc.c's willingness
to allow more than two name components.)
Dagfinn Ilmari Mannsåker and Tom Lane
Discussion: https://postgr.es/m/445692.1644018081@sss.pgh.pa.us
ReadStr returns allocated memory which the caller is responsible for
freeing when done with the string. This commit ensures that memory is
freed in one case which used ReadStr in a conditional. While the leak
might not be too concerning, this makes the code consistent across all
ReadStr callsites in ReadToc. Due to the lack of complaints of issues
in production from this, no backpatch is performed at this point.
Author: Bharath Rupireddy, Georgios Kokolatos
Reviewed-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/oZwKiUxFsVaetG2xOJp7Hwao8F1AKIdfFDQLNJrnwoaxmjyB-45r_aYmhgXHKLcMI3GT24m9L6HafSi2ns7WFxXe0mw2_tIJpD-Z3vb_eyI=@pm.me
Most calls of rmtree() report an error, and the code coming from 38bfae3
has introduced one caller where this is not done. The previous behavior
was to not fail hard if any log file generated is not properly unlinked
when cleaning up the contents generated once the upgrade has completed,
so add a cast to (void) to indicate the intention behind this new code.
Per gripe from Coverity.
Historically, the location of any files generated by pg_upgrade, as of
the per-database logs and internal dumps, has been the current working
directory, leaving all those files behind when using --retain or on a
failure.
Putting all those contents in a targeted subdirectory makes the whole
easier to debug, and simplifies the code in charge of cleaning up the
logs. Note that another reason is that this facilitates the move of
pg_upgrade to TAP with a fixed location for all the logs to grab if the
test fails repeatedly.
Initially, we thought about being able to specify the output directory
with a new option, but we have settled on using a subdirectory located
at the root of the new cluster's data folder, "pg_upgrade_output.d",
instead, as at the end the new data directory is the location of all the
data generated by pg_upgrade. There is a take with group permissions
here though: if the new data folder has been initialized with this
option, we need to create all the files and paths with the correct
permissions or a base backup taken after a pg_upgrade --retain would
fail, meaning that GetDataDirectoryCreatePerm() has to be called before
creating the log paths, before a couple of sanity checks on the clusters
and before getting the socket directory for the cluster's host settings.
The idea of the new location is based on a suggestion from Peter
Eisentraut.
Also thanks to Andrew Dunstan, Peter Eisentraut, Daniel Gustafsson, Tom
Lane and Bruce Momjian for the discussion (in alphabetical order).
Author: Justin Pryzby
Discussion: https://postgr.es/m/20211212025017.GN17618@telsasoft.com
Commit 8e2b6d45a0 added a new unprivileged user for testing
pg_basebackup, but omitted to add them to the cluster's authorized
logins, breaking Windows tests run without using Unix sockets.
The SQL standard has been ambiguous about whether null values in
unique constraints should be considered equal or not. Different
implementations have different behaviors. In the SQL:202x draft, this
has been formalized by making this implementation-defined and adding
an option on unique constraint definitions UNIQUE [ NULLS [NOT]
DISTINCT ] to choose a behavior explicitly.
This patch adds this option to PostgreSQL. The default behavior
remains UNIQUE NULLS DISTINCT. Making this happen in the btree code
is pretty easy; most of the patch is just to carry the flag around to
all the places that need it.
The CREATE UNIQUE INDEX syntax extension is not from the standard,
it's my own invention.
I named all the internal flags, catalog columns, etc. in the negative
("nulls not distinct") so that the default PostgreSQL behavior is the
default if the flag is false.
Reviewed-by: Maxim Orlov <orlovmg@gmail.com>
Reviewed-by: Pavel Borisov <pashkin.elfe@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/84e5ee1b-387e-9a54-c326-9082674bde78@enterprisedb.com
The comment for PGAC_READLINE_VARIABLES says "Readline versions < 2.1
don't have rl_completion_append_character". It seems certain that such
versions are extinct in the wild, though; for sure there are none in the
buildfarm. Libedit has had this variable for at least twenty years too.
Also, tab-complete.c's behavior without it is quite unfriendly, since
we'll emit a space even when completion fails; but we've had no
complaints about that.
Therefore, let's assume this variable is always there, and drop the
configure check to save a few build cycles.
Discussion: https://postgr.es/m/147685.1643858911@sss.pgh.pa.us
Fix up recently-added test cases in 010_tab_completion.pl
so that they pass with the rather seriously broken libedit
found in Debian 10 (Buster).
Also, add a few more test cases to improve code coverage.
The total line coverage still looks pretty awful, because
we exercise only a few paths of the giant if-else chain in
psql_completion(). However, this now covers almost all of
the code that isn't in one of those if-blocks.
Discussion: https://postgr.es/m/960764.1643751011@sss.pgh.pa.us
When this code executed as superuser it appeared to work because no
system catalog lookups happened, but otherwise it crashes because there
is no transaction environment. Fix that.
Report and code change by me. Test case by Dagfinn Ilmari Mannsåker.
Discussion: http://postgr.es/m/CA+TgmobiKLXne-2AVzYyWRiO8=rChBQ=7ywoxp=2SmcFw=oDDw@mail.gmail.com
When completing keywords that are offered alongside names obtained
from a query, preserve the user's choice of keyword case. This
would have been messy to do before 02b8048ba, but now it's fairly
simple. A complication is that we want keywords to be shown in
upper case in any tab-completion menus that include both keywords
and non-keywords, so we can't switch their case until enough has
been typed that only keyword(s) remain to be chosen.
Also, adjust some places where 02b8048ba thoughtlessly held over
a previous choice to display keywords in lower case. (I think
I got confused as to whether those words were keywords or variable
names, but they're the former.)
Dagfinn Ilmari Mannsåker and Tom Lane
Discussion: https://postgr.es/m/8735l41ynm.fsf@wibble.ilmari.org
This patch improves tab completion's ability to deal with
valid variant spellings of SQL identifiers. Notably:
* Unquoted upper-case identifiers are now downcased as the backend
would do, allowing them to be completed correctly.
* Tab completion can now match identifiers that are quoted even
though they don't need to be; for example "f<TAB> now completes
to "foo" if that's the only available name. Previously, only
names that require quotes would be offered.
* Schema-qualified identifiers are now supported where SQL syntax
allows it; many lesser-used completion rules neglected this.
* Completion operations that refer back to some previously-typed
name (for example, to complete names of columns belonging to a
previously-mentioned table) now allow variant spellings of the
previous name too.
In addition, performance of tab completion queries has been
improved for databases containing many objects, although
you'd only be likely to notice with a heavily-loaded server.
Authors of future tab-completion patches should note that this
commit changes many details about how tab completion queries
must be written:
* Tab completion queries now deal in raw object names; do not
use quote_ident().
* The name-matching restriction in a query must now be written
as "outputcol LIKE '%s'", not "substring(outputcol,1,%d)='%s'".
* The SchemaQuery mechanism has been extended so that it can
handle queries that refer back to a previous name. Most completion
queries that do that should be converted to SchemaQuery form.
Only consider using a literal query if the previous name can
never be schema-qualified. Don't use a literal query if the
name-to-be-completed can validly be schema-qualified, either.
* Use set_completion_reference() to specify which word is the previous
name to consider, for either a SchemaQuery or a literal query.
* If you want to offer some keywords in addition to a query result
(for example, offer COLUMN in addition to column names after
"ALTER TABLE t RENAME"), do not use the old hack of tacking the
keywords on with UNION. Instead use the new QUERY_PLUS macros
to write such keywords separately from the query proper. The
"addon" macro arguments that used to be used for this purpose
are gone.
* If your query returns something that's not a SQL identifier
(such as an attribute number or enum label), use the new
QUERY_VERBATIM macros to prevent the result from incorrectly
getting double-quoted. You may still need to use quote_literal
in such a query, too.
Tom Lane and Haiying Tang
Discussion: https://postgr.es/m/a63cbd45e3884cf9b3961c2a6a95dcb7@G08CNEXMBPEKD05.g08.fujitsu.local
Tushar Ahuja discovered that if you use both --compress and --gzip,
or --compress multiple times, the last instance of one of these
options doesn't in all cases overwrite the compression level set by
an earlier option. That's not a serious bug, but it also has nothing
to recommend it. Repair.
Discussion: http://postgr.es/m/CA+TgmoZfP=rsZB_9vDGfhuNgSu_M_09UWu8SjvsP65y_1pQFCg@mail.gmail.com
I intended to include a change to the "skip" count in the test
case, but it didn't get folded into the commit. Do that now,
so that non-zlib builds don't break.
The new file bbstreamer_gzip.c needs <unistd.h> to avoid
complaints about dup() not having a prototype, as per buildfarm
returns.
If you have a low-bandwidth connection between the client and the
server, it's reasonable to want to compress on the server side but
then decompress and extract the backup on the client side. This
commit allows you do to do just that.
Dipesh Pandit, with minor and mostly cosmetic changes by me.
Discussion: http://postgr.es/m/CAN1g5_HiSh8ajUMd4ePtGyCXo89iKZTzaNyzP_qv1eJbi4YHXA@mail.gmail.com
