Commit 0ad8032910 failed to update
the pg_basebackup documentation to mention that "client-" or
"server-" can now be prepended to the compression method name. Fix
it there, and also in the --help output that you get from running
the binary.
Also in the documentation, there's an old issue that the arguments to
--checkpoint shouldn't be marked as parameters, because "fast" and
"spread" are literal strings. Fix that too.
Dagfinn Ilmari Mannsåker, partly as per a report from
Shinoda Noriyoshi.
Discussion: http://postgr.es/m/PH7PR84MB1885C1CF433057807551172BEE5F9@PH7PR84MB1885.NAMPRD84.PROD.OUTLOOK.COM
This fixes a set of issues that have accumulated over the past months
(or years) in various code areas. Most fixes are related to some recent
additions, as of the development of v15.
Author: Justin Pryzby
Discussion: https://postgr.es/m/20220124030001.GQ23027@telsasoft.com
pg_basebackup's --compression option now lets you write either
"client-gzip" or "server-gzip" instead of just "gzip" to specify
where the compression should be performed. If you write simply
"gzip" it's taken to mean "client-gzip" unless you also use
--target, in which case it is interpreted to mean "server-gzip",
because that's the only thing that makes any sense in that case.
To make this work, the BASE_BACKUP command now takes new
COMPRESSION and COMPRESSION_LEVEL options.
At present, pg_basebackup cannot decompress .gz files, so
server-side compression will cause a failure if (1) -Ft is not
used or (2) -R is used or (3) -D- is used without --no-manifest.
Along the way, I removed the information message added by commit
5c649fe153 which occurred if you
specified no compression level and told you that the default level
had been used instead. That seemed like more output than most
people would want.
Also along the way, this adds a check to the server for
unrecognized base backup options. This repairs a bug introduced
by commit 0ba281cb4b.
This commit also adds some new test cases for pg_verifybackup.
They take a server-side backup with and without compression, and
then extract the backup if we have the OS facilities available
to do so, and then run pg_verifybackup on the extracted
directory. That is a good test of the functionality added by
this commit and also improves test coverage for the backup target
patch (commit 3500ccc39b) and for
pg_verifybackup itself.
Patch by me, with a bug fix by Jeevan Ladhe. The patch set of which
this is a part has also had review and/or testing from Tushar Ahuja,
Suraj Kharage, Dipesh Pandit, and Mark Dilger.
Discussion: http://postgr.es/m/CA+Tgmoa-ST7fMLsVJduOB7Eub=2WjfpHS+QxHVEpUoinf4bOSg@mail.gmail.com
Commit 9a974cbcba arranged to preserve
relfilenodes and tablespace OIDs. For similar reasons, also arrange
to preserve database OIDs.
One problem is that, up until now, the OIDs assigned to the template0
and postgres databases have not been fixed. This could be a problem
when upgrading, because pg_upgrade might try to migrate a database
from the old cluster to the new cluster while keeping the OID and find
a different database with that OID, resulting in a failure. If it finds
a database with the same name and the same OID that's OK: it will be
dropped and recreated. But the same OID and a different name is a
problem.
To prevent that, fix the OIDs for postgres and template0 to specific
values less than 16384. To avoid running afoul of this rule, these
values should not be changed in future releases. It's not a problem
that these OIDs aren't fixed in existing releases, because the OIDs
that we're assigning here weren't used for either of these databases
in any previous release. Thus, there's no chance that an upgrade of
a cluster from any previous release will collide with the OIDs we're
assigning here. And going forward, the OIDs will always be fixed, so
the only potential collision is with a system database having the
same name and the same OID, which is OK.
This patch lets users assign a specific OID to a database as well,
provided however that it can't be less than 16384. I (rhaas) thought
it might be better not to expose this capability to users, but the
consensus was otherwise, so the syntax is documented. Letting users
assign OIDs below 16384 would not be OK, though, because a
user-created database with a low-numbered OID might collide with a
system-created database in a future release. We therefore prohibit
that.
Shruthi KC, based on an earlier patch from Antonin Houska, reviewed
and with some adjustments by me.
Discussion: http://postgr.es/m/CA+TgmoYgTwYcUmB=e8+hRHOFA0kkS6Kde85+UNdon6q7bt1niQ@mail.gmail.com
Discussion: http://postgr.es/m/CAASxf_Mnwm1Dh2vd5FAhVX6S1nwNSZUB1z12VddYtM++H2+p7w@mail.gmail.com
Commit 6df7a9698 wrote appendPQExpBuffer where it should have
written printfPQExpBuffer. This resulted in re-issuing the
previous query along with the desired one, which very accidentally
had no negative consequences except for some wasted cycles.
Back-patch to v14 where that came in.
Discussion: https://postgr.es/m/1714711.1642962663@sss.pgh.pa.us
Commit 5c649fe15 introduced a memory leak into pg_basebackup's
parse_compress_options. (I simplified nearby code while at it.)
Commit 9a974cbcb introduced a memory leak into pg_dump's
binary_upgrade_set_pg_class_oids.
Coverity also complained about a call of SnapBuildProcessChange that
ignored the result, unlike every other call of that function. This
is evidently intentional, so add a (void) cast to indicate that.
(It's also old, dating to b89e15105; I suppose the reason it showed
up now is 7a5f6b474's recent rearrangement of nearby code.)
The option --compress is extended to accept a compression method and an
optional compression level, as of the grammar METHOD[:LEVEL]. The
methods currently support are "none" and "gzip", for client-side
compression. Any of those methods use only an integer value for the
compression level, but any method implemented in the future could use
more specific keywords if necessary.
This commit keeps the logic backward-compatible. Hence, the following
compatibility rules apply for the new format of the option --compress:
* -z/--gzip is a synonym of --compress=gzip.
* --compress=NUM implies:
** --compress=none if NUM = 0.
** --compress=gzip:NUM if NUM > 0.
Note that there are also plans to extend more this grammar with
server-side compression.
Reviewed-by: Robert Haas, Magnus Hagander, Álvaro Herrera, David
G. Johnston, Georgios Kokolatos
Discussion: https://postgr.es/m/Yb3GEgWwcu4wZDuA@paquier.xyz
pg_basebackup now has a --target=TARGET[:DETAIL] option. If specfied,
it is sent to the server as the value of the TARGET option to the
BASE_BACKUP command. If DETAIL is included, it is sent as the value of
the new TARGET_DETAIL option to the BASE_BACKUP command. If the
target is anything other than 'client', pg_basebackup assumes that it
will now be the server's job to write the backup in a location somehow
defined by the target, and that it therefore needs to write nothing
locally. However, the server will still send messages to the client
for progress reporting purposes.
On the server side, we now support two additional types of backup
targets. There is a 'blackhole' target, which just throws away the
backup data without doing anything at all with it. Naturally, this
should only be used for testing and debugging purposes, since you will
not actually have a backup when it finishes running. More usefully,
there is also a 'server' target, so you can now use something like
'pg_basebackup -Xnone -t server:/SOME/PATH' to write a backup to some
location on the server. We can extend this to more types of targets
in the future, and might even want to create an extensibility
mechanism for adding new target types.
Since WAL fetching is handled with separate client-side logic, it's
not part of this mechanism; thus, backups with non-default targets
must use -Xnone or -Xfetch.
Patch by me, with a bug fix by Jeevan Ladhe. The patch set of which
this is a part has also had review and/or testing from Tushar Ahuja,
Suraj Kharage, Dipesh Pandit, and Mark Dilger.
Discussion: http://postgr.es/m/CA+TgmoaYZbz0=Yk797aOJwkGJC-LK3iXn+wzzMx7KdwNpZhS5g@mail.gmail.com
In the new approach, all files across all tablespaces are sent in a
single COPY OUT operation. The CopyData messages are no longer raw
archive content; rather, each message is prefixed with a type byte
that describes its purpose, e.g. 'n' signifies the start of a new
archive and 'd' signifies archive or manifest data. This protocol
is significantly more extensible than the old approach, since we can
later create more message types, though not without concern for
backward compatibility.
The new protocol sends a few things to the client that the old one
did not. First, it sends the name of each archive explicitly, instead
of letting the client compute it. This is intended to make it easier
to write future patches that might send archives in a format other
that tar (e.g. cpio, pax, tar.gz). Second, it sends explicit progress
messages rather than allowing the client to assume that progress is
defined by the number of bytes received. This will help with future
features where the server compresses the data, or sends it someplace
directly rather than transmitting it to the client.
The old protocol is still supported for compatibility with previous
releases. The new protocol is selected by means of a new
TARGET option to the BASE_BACKUP command. Currently, the
only supported target is 'client'. Support for additional
targets will be added in a later commit.
Patch by me. The patch set of which this is a part has had review
and/or testing from Jeevan Ladhe, Tushar Ahuja, Suraj Kharage,
Dipesh Pandit, and Mark Dilger.
Discussion: http://postgr.es/m/CA+TgmoaYZbz0=Yk797aOJwkGJC-LK3iXn+wzzMx7KdwNpZhS5g@mail.gmail.com
The original coding (from c33869cc3) failed with "more than one row
returned by a subquery used as an expression" if there were unrelated
triggers of the same tgname on parent partitioned tables. (That's
possible because statement-level triggers don't get inherited.) Fix
by applying LIMIT 1 after sorting the candidates by inheritance level.
Also, wrap the subquery in a CASE so that we don't have to execute it at
all when the trigger is visibly non-inherited. Aside from saving some
cycles, this avoids the need for a confusing and undocumented NULLIF().
While here, tweak the format of the emitted query to look a bit
nicer for "psql -E", and add some explanation of this subquery,
because it badly needs it.
Report and patch by Justin Pryzby (with some editing by me).
Back-patch to v13 where the faulty code came in.
Discussion: https://postgr.es/m/20211217154356.GJ17618@telsasoft.com
Most tests invoking pg_basebackup themselves did not yet use -cfast, which
makes pg_basebackup take considerably longer. The only reason this didn't
cause the tests to take many minutes is that spread checkpoints only throttle
when writing out a buffer and there aren't that many dirty buffers in the
tests...
Discussion: https://postgr.es/m/20220117195711.xx4qbxutrrlmo2dg@alap3.anarazel.de
Currently, database OIDs, relfilenodes, and tablespace OIDs can all
change when a cluster is upgraded using pg_upgrade. It seems better
to preserve them, because (1) it makes troubleshooting pg_upgrade
easier, since you don't have to do a lot of work to match up files
in the old and new clusters, (2) it allows 'rsync' to save bandwidth
when used to re-sync a cluster after an upgrade, and (3) if we ever
encrypt or sign blocks, we would likely want to use a nonce that
depends on these values.
This patch only arranges to preserve relfilenodes and tablespace
OIDs. The task of preserving database OIDs is left for another patch,
since it involves some complexities that don't exist in these cases.
Database OIDs have a similar issue, but there are some tricky points
in that case that do not apply to these cases, so that problem is left
for another patch.
Shruthi KC, based on an earlier patch from Antonin Houska, reviewed
and with some adjustments by me.
Discussion: http://postgr.es/m/CA+TgmoYgTwYcUmB=e8+hRHOFA0kkS6Kde85+UNdon6q7bt1niQ@mail.gmail.com
The logic is similar to default_tablespace in some ways, so as no SET
queries on default_table_access_method are generated before dumping or
restoring an object (table or materialized view support table AMs) when
specifying this new option.
This option is useful to enforce the use of a default access method even
if some tables included in a dump use an AM different than the system's
default.
There are already two cases in the TAP tests of pg_dump with a table and
a materialized view that use a non-default table AM, and these are
extended that the new option does not generate SET clauses on
default_table_access_method.
Author: Justin Pryzby
Discussion: https://postgr.es/m/20211207153930.GR17618@telsasoft.com
"jsonlog" is a new value that can be added to log_destination to provide
logs in the JSON format, with its output written to a file, making it
the third type of destination of this kind, after "stderr" and
"csvlog". The format is convenient to feed logs to other applications.
There is also a plugin external to core that provided this feature using
the hook in elog.c, but this had to overwrite the output of "stderr" to
work, so being able to do both at the same time was not possible. The
files generated by this log format are suffixed with ".json", and use
the same rotation policies as the other two formats depending on the
backend configuration.
This takes advantage of the refactoring work done previously in ac7c807,
bed6ed3, 8b76f89 and 2d77d83 for the backend parts, and 72b76f7 for the
TAP tests, making the addition of any new file-based format rather
straight-forward.
The documentation is updated to list all the keys and the values that
can exist in this new format. pg_current_logfile() also required a
refresh for the new option.
Author: Sehrope Sarkuni, Michael Paquier
Reviewed-by: Nathan Bossart, Justin Pryzby
Discussion: https://postgr.es/m/CAH7T-aqswBM6JWe4pDehi1uOiufqe06DJWaU5=X7dDLyqUExHg@mail.gmail.com
Since enum labels have to be single-quoted, this part of the
tab completion machinery got side-swiped by commit cd69ec66c.
A side-effect of that commit is that (at least with some versions
of Readline) the text string passed for completion will omit the
leading quote mark of the enum label literal. Libedit still acts
the same as before, though, so adapt COMPLETE_WITH_ENUM_VALUE so
that it can cope with either convention.
Also, when we fail to find any valid completion, set
rl_completion_suppress_quote = 1. Otherwise readline will
go ahead and append a closing quote, which is unwanted.
Per report from Peter Eisentraut. Back-patch to v13 where
cd69ec66c came in.
Discussion: https://postgr.es/m/8ca82d89-ec3d-8b28-8291-500efaf23b25@enterprisedb.com
By default, wait_for_catchup() waits for the replication connection
to reach the primary's write LSN. That's fine, but in an apparent
attempt to save one query round-trip, it was coded so that we
executed pg_current_wal_lsn() again during each probe query.
Thus, we presented the standby with a moving target to be reached.
(While the test script itself couldn't be causing the write LSN
to advance while it's blocked in wait_for_catchup(), it's plenty
plausible that background activity such as autovacuum is emitting
more WAL.) That could make the test take longer than necessary,
and potentially it could mask bugs by allowing the standby to process
more WAL than a strict interpretation of the test scenario allows.
So, change wait_for_catchup() to do it "by the book", explicitly
collecting the write LSN to wait for at the outset.
Also, various call sites were instructing wait_for_catchup() to
wait for the standby to reach the primary's insert LSN rather than
its write LSN. This also seems like a bad idea. While in most
test scenarios those are the same, if they are different then the
inserted-but-not-yet-written WAL is not presently available to the
standby. The test isn't doing anything to make it become so, so
again we have the potential for unwanted test delay, perhaps even
a test timeout. (Again, background activity would be needed to
make this more than a hypothetical problem.) Hence, change the
callers where necessary so that the wait target is always the
primary's write LSN.
While at it, simplify callers by making use of wait_for_catchup's
default arguments wherever possible (the preceding change makes
this possible in more places than it was before). And rewrite
wait_for_catchup's documentation a bit.
Patch by me; thanks to Julien Rouhaud for review.
Discussion: https://postgr.es/m/2368336.1641843098@sss.pgh.pa.us
The point of this patch is to reduce inclusion spam by not needing
to #include <netdb.h> or <pwd.h> in port.h (which is read by every
compile in our tree). To do that, we must remove port.h's
declarations of pqGetpwuid and pqGethostbyname.
pqGethostbyname is only used, and is only ever likely to be used,
in src/port/getaddrinfo.c --- which isn't even built on most
platforms, making pqGethostbyname dead code for most people.
Hence, deal with that by just moving it into getaddrinfo.c.
To clean up pqGetpwuid, invent a couple of simple wrapper
functions with less-messy APIs. This allows removing some
duplicate error-handling code, too.
In passing, remove thread.c from the MSVC build, since it
contains nothing we use on Windows.
Noted while working on 376ce3e40.
Discussion: https://postgr.es/m/1634252654444.90107@mit.edu
Try to disable ASLR when building in EXEC_BACKEND mode, to avoid random
memory mapping failures while testing. For developer use only, no
effect on regular builds.
Suggested-by: Andres Freund <andres@anarazel.de>
Tested-by: Bossart, Nathan <bossartn@amazon.com>
Discussion: https://postgr.es/m/20210806032944.m4tz7j2w47mant26%40alap3.anarazel.de
When we need to identify the home directory on non-Windows, first
consult getenv("HOME"). If that's empty or unset, fall back
on our previous method of checking the <pwd.h> database.
Preferring $HOME allows the user to intentionally point at some
other directory, and it seems to be in line with the behavior of
most other utilities. However, we shouldn't rely on it completely,
as $HOME is likely to be unset when running as a daemon.
Anders Kaseorg
Discussion: https://postgr.es/m/1634252654444.90107@mit.edu
This addresses some problems in the describe queries used for extended
statistics:
- Two schema qualifications for the text type were missing for \dX.
- The list of extended statistics listed for a table through \d was
ordered based on the object OIDs, but it is more consistent with the
other commands to order by namespace and then by object name.
- A couple of aliases were not used in \d. These are removed.
This is similar to commits 1f092a3 and 07f8a9e.
Author: Justin Pryzby
Discussion: https://postgr.es/m/20220107022235.GA14051@telsasoft.com
Backpatch-through: 14
pg_basebackup.c relies on the compression level to not be 0 to decide if
compression should be used, but 000f3adf missed the fact that the
default compression (Z_DEFAULT_COMPRESSION) is -1, which would be used
if specifying --gzip without --compress.
While on it, add some coverage for --gzip, as this is rather easy to
miss.
Reported-by: Christoph Berg
Discussion: https://postgr.es/m/YdhRDMLjabtXOnhY@msg.df7cb.de
pg_basebackup is able to use gzip to compress the contents of backups
with the tar format, but there were no tests for that. This adds a
minimalistic set of tests to check the contents of such base backups,
including sanity checks on the contents generated with gzip commands.
The tests are skipped if Postgres is compiled --without-zlib, and the
checks based on the gzip command are skipped if nothing can be found,
following the same model as the existing tests for pg_receivewal.
Reviewed-by: Georgios Kokolatos
Discussion: https://postgr.es/m/Yb3GEgWwcu4wZDuA@paquier.xyz
Since d62bcc8, the directory method of walmethods.c uses the compression
method to determine which code path to take. The tar method, used by
pg_basebackup --format=t, was inconsistent regarding that, as it relied
on the compression level to check if no compression or gzip should be
used. This commit makes the code more consistent as a whole in this
file, making the tar logic use a compression method rather than
assigning COMPRESSION_NONE that would be ignored.
The options of pg_basebackup are planned to be reworked but we are not
sure yet of the shape they should have as this has some dependency with
the integration of the server-side compression for base backups, so this
is left out for the moment. This change has as benefit to make easier
the future integration of new compression methods for the tar method of
walmethods.c, for the client-side compression.
Reviewed-by: Georgios Kokolatos
Discussion: https://postgr.es/m/Yb3GEgWwcu4wZDuA@paquier.xyz
The ACL is printed when you add + to the command, similarly to
various other psql backslash commands.
Along the way, move the code for this into describe.c,
where it is a better fit (and can share some code).
Pavel Luzanov, reviewed by Georgios Kokolatos
Discussion: https://postgr.es/m/6d722115-6297-bc53-bb7f-5f150e765299@postgrespro.ru
While user-defined triggers defined on a partitioned table have
a catalog definition for both it and its partitions, internal
triggers used by foreign keys defined on partitioned tables only
have a catalog definition for its partitions. This commit fixes
that so that partitioned tables get the foreign key triggers too,
just like user-defined triggers. Moreover, like user-defined
triggers, partitions' internal triggers will now also have their
tgparentid set appropriately. This is to allow subsequent commit(s)
to make the foreign key related events to be fired in some cases
using the parent table triggers instead of those of partitions'.
This also changes what tgisinternal means in some cases. Currently,
it means either that the trigger is an internal implementation object
of a foreign key constraint, or a "child" trigger on a partition
cloned from the trigger on the parent. This commit changes it to
only mean the former to avoid confusion. As for the latter, it can
be told by tgparentid being nonzero, which is now true both for user-
defined and foreign key's internal triggers.
Author: Amit Langote <amitlangote09@gmail.com>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Reviewed-by: Arne Roland <A.Roland@index.de>
Discussion: https://postgr.es/m/CA+HiwqG7LQSK+n8Bki8tWv7piHD=PnZro2y6ysU2-28JS6cfgQ@mail.gmail.com
Improve psql so that "ALTER TABLE foo VALIDATE CONSTRAINT <TAB>"
only offers not-convalidated entries. While it's not formally
wrong to offer validated ones, there's not much point either,
and it can save some typing if we incorporate this knowledge.
David Fetter, reviewed by Aleksander Alekseev
Discussion: https://postgr.es/m/20210427002433.GB17834@fetter.org
Get rid of the "username_subquery" mechanism in favor of doing
local lookups of role names from role OIDs. The PG backend isn't
terribly smart about scalar SubLinks in SELECT output lists,
so this offers a small performance improvement, at least in
installations with more than a couple of users. In any case
the old method didn't make for particularly readable SQL code.
While at it, I removed the various custom warning messages about
failing to find an object's owner, in favor of just fatal'ing
in the local lookup function. AFAIK there is no reason any
longer to treat that as anything but a catalog-corruption case,
and certainly no reason to make translators deal with a dozen
different messages where one would do. (If it turns out that
fatal() is indeed a bad idea, we can back off to issuing
pg_log_warning() and returning an empty string, resulting in
the same behavior as before, except more consistent.)
Also drop an entirely unnecessary sub-SELECT to check on the
pg_depend status of a sequence relation: we already have a
LEFT JOIN to fetch the row of interest in the FROM clause.
Discussion: https://postgr.es/m/2460369.1640903318@sss.pgh.pa.us
dumpPublication, dumpPublicationNamespace, dumpPublicationTable, and
dumpSubscription failed to check dataOnly. This is just a latent bug,
because pg_backup_archiver.c would filter out the ArchiveEntry later;
but they're wasting cycles in data-only dumps, and the omission might
become a live bug someday. In any case, it's not good to have some
dumpFoo functions do this and some not.
On the same reasoning, make dumpPublicationNamespace follow the
same pattern as every other dumpFoo function for checking the
DUMP_COMPONENT_DEFINITION flag. (Since 5209c0ba0, we wouldn't
even get here if that flag isn't set, so checking it is just
pro forma right now. But it might not be so forever.)
Since this is just cosmetic and/or future-proofing, no need for
back-patch.
In the wake of commits 05649b88c and 5209c0ba0, findComments() and
findSecLabels() no longer use their "Archive *fout" arguments,
so get rid of those.
While doing that, I noticed that there's no very good reason why
dumpCompositeTypeColComments() should be doing its own query to fetch
the column names of the composite type, when the calling function has
just fetched the same data. Tweak it to use that query result. This
probably doesn't save a lot for most people, because since 5209c0ba0
we won't get into this code at all unless the composite type has at
least one comment. Nonetheless, it's a wasted query.
\getenv fetches the value of an environment variable into a psql
variable. This is the inverse of the \setenv command that was added
over ten years ago. We'd not seen a compelling use-case for \getenv
at the time, but upcoming regression test refactoring provides a
sufficient reason to add it now.
Discussion: https://postgr.es/m/1655733.1639871614@sss.pgh.pa.us
Coverity complained that parts of dumpFunc() and buildACLCommands()
were now unreachable, as indeed they are. Remove 'em.
In passing, make dumpFunc's handling of protrftypes less gratuitously
different from other fields.
This is an option consistent with what the other tools of src/bin/
(pg_checksums, pg_dump, pg_rewind and pg_basebackup) provide which is
useful for leveraging the I/O effort when testing things. This is not
to be used in a production environment.
All the regression tests of pg_upgrade are updated to use this new
option. This happens to cut at most a couple of seconds in environments
constrained on I/O, by avoiding a flush of data folder for the new
cluster upgraded.
Author: Michael Paquier
Reviewed-by: Peter Eisentraut
Discussion: https://postgr.es/m/YbrhzuBmBxS/DkfX@paquier.xyz
Per discussion, we'll limit support for old servers to those branches
that can still be built easily on modern platforms, which as of now
is 9.2 and up.
Aside from removing code that is dead per the assumption of
server >= 9.2, I tweaked the startup warning for unsupported versions
to complain about too-old servers as well as too-new ones. The
warning that "Some psql features might not work" applies precisely
to both cases.
Discussion: https://postgr.es/m/2923349.1634942313@sss.pgh.pa.us
Server versions for which there was a plausible reason to
use this switch are all out of support now. Leaving it
around would accomplish little except to let careless DBAs
shoot themselves in the foot.
Discussion: https://postgr.es/m/556122.1639520324@sss.pgh.pa.us
Per discussion, we'll limit support for old servers to those branches
that can still be built easily on modern platforms, which as of now
is 9.2 and up.
Discussion: https://postgr.es/m/2923349.1634942313@sss.pgh.pa.us
Per discussion, we'll limit support for old servers to those branches
that can still be built easily on modern platforms, which as of now
is 9.2 and up. Remove over a thousand lines of code dedicated to
dumping from older server versions. (As in previous changes of
this sort, we aren't removing pg_restore's ability to read older
archive files ... though it's fair to wonder how that might be
tested nowadays.) This cleans up some dead code left behind by
commit 989596152.
Discussion: https://postgr.es/m/2923349.1634942313@sss.pgh.pa.us
We need to replace windows-style \ path separators with / when putting socket
directories either in postgresql.conf or libpq connection strings, otherwise
they are interpreted as escapes.
Author: Andres Freund <andres@anarazel.de>
Reviewed-By: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Discussion: https://postgr.es/m/4da250a5-4222-1522-f14d-8a72bcf7e38e@enterprisedb.com
The idea here is that when a performance problem is known to have
occurred at a certain point in time, it's a good thing if there is
some information available from the logs to help figure out what
might have happened around that time.
This change attracted an above-average amount of dissent, because
it means that a server with default settings will produce some amount
of log output even if nothing has gone wrong. However, by my count,
the mailing list discussion had about twice as many people in favor
of the change as opposed. The reasons for believing that the extra
log output is not an issue in practice are: (1) the rate at which
messages can be generated by this setting is bounded to one every
few minutes on a properly-configured system and (2) production
systems tend to have a lot more junk in the log from that due to
failed connection attempts, ERROR messages generated by application
activity, and the like.
Bharath Rupireddy, reviewed by Fujii Masao and by me. Many other
people commented on the thread, but as far as I can see that was
discussion of the merits of the change rather than review of the
patch.
Discussion: https://postgr.es/m/CALj2ACX-rW_OeDcp4gqrFUAkf1f50Fnh138dmkd0JkvCNQRKGA@mail.gmail.com
Historically we've put type "char" into the S (String) typcategory,
although calling it a string is a stretch considering it can only
store one byte. (In our actual usage, it's more like an enum.)
This choice now seems wrong in view of the special heuristics
that parse_func.c and parse_coerce.c have for TYPCATEGORY_STRING:
it's not a great idea for "char" to have those preferential casting
behaviors.
Worse than that, recent patches inventing special-purpose types
like pg_node_tree have assigned typcategory S to those types,
meaning they also get preferential casting treatment that's designed
on the assumption that they can hold arbitrary text.
To fix, invent a new category TYPCATEGORY_INTERNAL for internal-use
types, and assign that to all these types. I used code 'Z' for
lack of a better idea ('I' was already taken).
This change breaks one query in psql/describe.c, which now needs to
explicitly cast a catalog "char" column to text before concatenating
it with an undecorated literal. Also, a test case in contrib/citext
now needs an explicit cast to convert citext to "char". Since the
point of this change is to not have "char" be a surprisingly-available
cast target, these breakages seem OK.
Per report from Ian Campbell.
Discussion: https://postgr.es/m/2216388.1638480141@sss.pgh.pa.us
In parallel mode, pg_dump tries to order the table-data-dumping
jobs with the largest tables first. However, it was only
consulting the pg_class.relpages value to determine table size.
This ignores TOAST data, and so we could make poor scheduling
decisions in cases where some large tables are mostly TOASTed
data while others have very little. To fix, add in the relpages
value for the TOAST table as well.
This patch also fixes a potential integer-overflow issue that
could result in poor scheduling on machines where off_t is
only 32 bits wide. Such platforms are probably extinct in the
wild, but we do still nominally support them, so repair.
Per complaint from Hans Buschmann.
Discussion: https://postgr.es/m/7d7eb6128f40401d81b3b7a898b6b4de@W2012-02.nidsa.loc
For objects such as functions, pg_dump issues the same secondary
data-collection query against each object to be dumped. This can't
readily be refactored to avoid the repetitive queries, but we can
PREPARE these queries to reduce planning costs.
This patch applies the idea to functions, aggregates, operators, and
data types. While it could be carried further, the remaining sorts of
objects aren't likely to appear in typical databases enough times to
be worth worrying over. Moreover, doing the PREPARE is likely to be a
net loss if there aren't at least some dozens of objects to apply the
prepared query to.
Discussion: https://postgr.es/m/7d7eb6128f40401d81b3b7a898b6b4de@W2012-02.nidsa.loc
Instead of issuing a secondary data-collection query against each
table to be dumped, issue just one query, with a WHERE clause
restricting it to be applied to only the tables we intend to dump.
Likewise for indexes, constraints, and triggers. This greatly
reduces the number of queries needed to dump a database containing
many tables. It might seem that WHERE clauses listing many target
OIDs could be inefficient, but at least on recent server versions
this provides a very substantial speedup.
(In principle the same thing could be done with other object types
such as functions; but that would require significant refactoring
of pg_dump, so those will be tackled in a different way in a
following patch.)
The new WHERE clauses depend on the unnest() function, which is
only present in 8.4 and above. We could implement them differently
for older servers, but there is an ongoing discussion that will
probably result in dropping pg_dump support for servers before 9.2,
so that seems like it'd be wasted work. For now, just bump the
server version check to require >= 8.4, without stopping to remove
any of the code that's thereby rendered dead. We'll mop that
situation up soon.
Patch by me, based on an idea from Andres Freund.
Discussion: https://postgr.es/m/7d7eb6128f40401d81b3b7a898b6b4de@W2012-02.nidsa.loc
Avoid calling pg_get_partkeydef(), pg_get_expr(relpartbound),
and regtypeout until we have lock on the relevant tables.
The existing coding is at serious risk of failure if there
are any concurrent DROP TABLE commands going on --- including
drops of other sessions' temp tables.
Arguably this is a bug fix that should be back-patched, but it's
moderately invasive and we've not had all that many complaints
about such failures. Let's just put it in HEAD for now.
Discussion: https://postgr.es/m/2273648.1634764485@sss.pgh.pa.us
Discussion: https://postgr.es/m/7d7eb6128f40401d81b3b7a898b6b4de@W2012-02.nidsa.loc
Throw away most of the existing logic for this, as it was very
inefficient thanks to expensive sub-selects executed to collect
ACL data that we very possibly would have no interest in dumping.
Reduce the ACL handling in the initial per-object-type queries
to be just collection of the catalog ACL fields, as it was
originally. Fetch pg_init_privs data separately in a single
scan of that catalog, and do the merging calculations on the
client side. Remove the separate code path used for pre-9.6
source servers; there is no good reason to treat them differently
from newer servers that happen to have empty pg_init_privs.
Discussion: https://postgr.es/m/2273648.1634764485@sss.pgh.pa.us
Discussion: https://postgr.es/m/7d7eb6128f40401d81b3b7a898b6b4de@W2012-02.nidsa.loc
Split the DumpableObject.dump bitmask field into separate bitmasks
tracking which components are requested to be dumped (in the
existing "dump" field) and which components exist for the particular
object (in the new "components" field). This gets rid of some
klugy and easily-broken logic that involved setting bits and later
clearing them. More importantly, it restores the originally intended
behavior that pg_dump's secondary data-gathering queries should not
be executed for objects we have no interest in dumping. That
optimization got broken when the dump flag was turned into a bitmask,
because irrelevant bits tended to remain set in many cases. Since
the "components" field starts from a minimal set of bits and is
added onto as needed, ANDing it with "dump" provides a reliable
indicator of what we actually have to dump, without having to
complicate the logic that manages the request bits. This makes
a significant difference in the number of queries needed when,
for example, there are many functions in extensions.
Discussion: https://postgr.es/m/2273648.1634764485@sss.pgh.pa.us
Discussion: https://postgr.es/m/7d7eb6128f40401d81b3b7a898b6b4de@W2012-02.nidsa.loc
Checking for RELKIND_MATVIEW was forgotten in
guessConstraintInheritance(). This isn't a live problem, since it is
checked in flagInhTables() which relkinds can have parents, and those
entries will have numParents==0 after that. But after discussion it
was felt that this place should be kept consistent with
flagInhTables() and flagInhAttrs().
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/flat/a574c8f1-9c84-93ad-a9e5-65233d6fc00f@enterprisedb.com
Previously, pg_waldump would not display its statistics summary if it
got interrupted by SIGINT (or say a simple Ctrl+C). It gains with this
commit a signal handler for SIGINT, trapping the signal to exit at the
earliest convenience to allow a display of the stats summary before
exiting. This makes the reports more interactive, similarly to strace
-c.
This new behavior makes the combination of the options --stats and
--follow much more useful, so as the user will get a report for any
invocation of pg_waldump in such a case. Information about the LSN
range of the stats computed is added as a header to the report
displayed.
This implementation comes from a suggestion by Álvaro Herrera and
myself, following a complaint by the author of this patch about --stats
and --follow not being useful together originally.
As documented, this is not supported on Windows, though its support
would be possible by catching the terminal events associated to Ctrl+C,
for example (this may require a more centralized implementation, as
other tools could benefit from a common API).
Author: Bharath Rupireddy
Discussion: https://postgr.es/m/CALj2ACUUx3PcK2z9h0_m7vehreZAUbcmOky9WSEpe8TofhV=PQ@mail.gmail.com
The existing pg_upgrade/test.sh and the buildfarm code have been holding
the same set of SQL queries when doing cross-version upgrade tests to
adapt the objects created by the regression tests before the upgrade
(mostly, incompatible or non-existing objects need to be dropped from
the origin, perhaps re-created).
This moves all those SQL queries into a new, separate, file with a set
of \if clauses to handle the version checks depending on the old version
of the cluster to-be-upgraded.
The long-term plan is to make the buildfarm code re-use this new SQL
file, so as committers are able to fix any compatibility issues in the
tests of pg_upgrade with a refresh of the core code, without having to
poke at the buildfarm client. Note that this is only able to handle the
main regression test suite, and that nothing is done yet for contrib
modules yet (these have more issues like their database names).
A backpatch down to 10 is done, adapting the version checks as this
script needs to be only backward-compatible, so as it becomes possible
to clean up a maximum amount of code within the buildfarm client.
Author: Justin Pryzby, Michael Paquier
Discussion: https://postgr.es/m/20201206180248.GI24052@telsasoft.com
Backpatch-through: 10
Readline's meta-# command is supposed to insert a comment marker
at the start of the current line. However, the default marker is
"#" which is entirely unhelpful for SQL. Set it to "-- " instead.
(This setting can still be overridden in one's ~/.inputrc file,
so this change won't affect people who have already taken steps
to make the command useful.)
Discussion: https://postgr.es/m/CAJcOf-cAdMVr7azeYR7nWKsNp7qhORzc84rV6d7m7knG5Hrtsw@mail.gmail.com
If we've not yet collected any non-whitespace, non-comment token for a
new query, flush the current input line to history before reading
another line. This aligns psql's history behavior with the observation
that lines containing only comments are generally not thought of as
being part of the next query. psql's prompting behavior is consistent
with that view, too, since it won't change the prompt until you
enter something that's neither whitespace nor a "--" comment.
Greg Nancarrow, simplified a bit by me
Discussion: https://postgr.es/m/CAJcOf-cAdMVr7azeYR7nWKsNp7qhORzc84rV6d7m7knG5Hrtsw@mail.gmail.com
This commit moves the timestamp computation of the control file within
the routine of src/common/ in charge of updating the backend's control
file, which is shared by multiple frontend tools (pg_rewind,
pg_checksums and pg_resetwal) and the backend itself.
This change has as direct effect to update the control file's timestamp
when writing the control file in pg_rewind and pg_checksums, something
that is helpful to keep track of control file updates for those
operations, something also tracked by the backend at startup within its
logs. This part is arguably a bug, as ControlFileData->time should be
updated each time a new version of the control file is written, but this
is a behavior change so no backpatch is done.
Author: Amul Sul
Reviewed-by: Nathan Bossart, Michael Paquier, Bharath Rupireddy
Discussion: https://postgr.es/m/CAAJ_b97nd_ghRpyFV9Djf9RLXkoTbOUqnocq11WGq9TisX09Fw@mail.gmail.com
Standardize on xoroshiro128** as our basic PRNG algorithm, eliminating
a bunch of platform dependencies as well as fundamentally-obsolete PRNG
code. In addition, this API replacement will ease replacing the
algorithm again in future, should that become necessary.
xoroshiro128** is a few percent slower than the drand48 family,
but it can produce full-width 64-bit random values not only 48-bit,
and it should be much more trustworthy. It's likely to be noticeably
faster than the platform's random(), depending on which platform you
are thinking about; and we can have non-global state vectors easily,
unlike with random(). It is not cryptographically strong, but neither
are the functions it replaces.
Fabien Coelho, reviewed by Dean Rasheed, Aleksander Alekseev, and myself
Discussion: https://postgr.es/m/alpine.DEB.2.22.394.2105241211230.165418@pseudo
The following improvements are done:
- Addition of type completion for ALTER SEQUENCE AS.
- Ignore ALTER for transforms, as the command is not supported.
- Addition of more completion for ALTER FOREIGN DATA WRAPPER.
- Addition of options related to columns in ALTER VIEW.
This is a continuation of the work done in 0cd6d3b.
Author: Ken Kato
Discussion: https://postgr.es/m/9497ae9ca1b31eb9b1e97aded1c2ab07@oss.nttdata.com
If an error occurs when writing the LZ4 file header, LZ4F_compressEnd()
was called in the error code path of write(), followed by
LZ4F_freeCompressionContext() to finish the cleanup. The code as-is was
not broken, but the LZ4F_compressEnd() proves to not be necessary as
there are no contents to flush at this stage, so remove it.
Per gripe from Jeevan Ladhe and Robert Haas.
Discussion: https://postgr.es/m/CAOgcT0PE33wbD7giAT1OSkNJt=p-vu8huq++qh=ny9O=SCP5aA@mail.gmail.com
When a stored expression depends on a user-defined cast, the backend
records the dependency as being on the cast's implementation function
--- or indeed, if there's no cast function involved but just
RelabelType or CoerceViaIO, no dependency is recorded at all. This
is problematic for pg_dump, which is at risk of dumping things in the
wrong order leading to restore failures. Given the lack of previous
reports, the risk isn't that high, but it can be demonstrated if the
cast is used in some view whose rowtype is then used as an input or
result type for some other function. (That results in the view
getting hoisted into the functions portion of the dump, ahead of
the cast.)
A logically bulletproof fix for this would require including the
cast's OID in the parsed form of the expression, whence it could be
extracted by dependency.c, and then the stored dependency would force
pg_dump to do the right thing. Such a change would be fairly invasive,
and certainly not back-patchable. Moreover, since we'd prefer that
an expression using cast syntax be equal() to one doing the same
thing by explicit function call, the cast OID field would have to
have special ignored-by-comparisons semantics, making things messy.
So, let's instead fix this by a very simple hack in pg_dump: change
the object-type priority order so that casts are initially sorted
before functions, immediately after types. This fixes the problem
in a fairly direct way for casts that have no implementation function.
For those that do, the implementation function will be hoisted to just
before the cast by the dependency sorting step, so that we still have
a valid dump order. (I'm not sure that this provides a full guarantee
of no problems; but since it's been like this for many years without
any previous reports, this is probably enough to fix it in practice.)
Per report from Дмитрий Иванов.
Back-patch to all supported branches.
Discussion: https://postgr.es/m/CAPL5KHoGa3uvyKp6z6m48LwCnTsK+LRQ_mcA4uKGfqAVSEjV_A@mail.gmail.com
If a table contains a generated column that's preceded by a dropped
column, dumpTableData_insert failed to account for the dropped
column, and would emit DEFAULT placeholder(s) in the wrong column(s).
This resulted in failures at restore time. The default COPY code path
did not have this bug, likely explaining why it wasn't noticed sooner.
While we're fixing this, we can be a little smarter about the
situation: (1) avoid unnecessarily fetching the values of generated
columns, (2) omit generated columns from the output, too, if we're
using --column-inserts. While these modes aren't expected to be
as high-performance as the COPY path, we might as well be as
efficient as we can; it doesn't add much complexity.
Per report from Дмитрий Иванов.
Back-patch to v12 where generated columns came in.
Discussion: https://postgr.es/m/CAPL5KHrkBniyQt5e1rafm5DdXvbgiiqfEQEJ9GjtVzN71Jj5pA@mail.gmail.com
A couple of spots can benefit from an added errdetail(), which matches
what we were already doing in other places; and those that cannot
withstand errdetail() can get a more descriptive primary message.
Author: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Julien Rouhaud <rjuju123@gmail.com>
Discussion: https://postgr.es/m/CALj2ACV+cX1eM03GfcA=ZMLXh5fSn1X1auJLz3yuS1duPSb9QA@mail.gmail.com
Previously it was impossible to terminate these programs via control-C
while they were prompting for a password. We can fix that trivially
for their initial password prompts, by moving setup of the SIGINT
handler from just before to just after their initial GetConnection()
calls.
This fix doesn't permit escaping out of later re-prompts, but those
should be exceedingly rare, since the user's password or the server's
authentication setup would have to have changed meanwhile. We
considered applying a fix similar to commit 46d665bc2, but that
seemed more complicated than it'd be worth. Moreover, this way is
back-patchable, which that wasn't.
The misbehavior exists in all supported versions, so back-patch to all.
Tom Lane and Nathan Bossart
Discussion: https://postgr.es/m/747443.1635536754@sss.pgh.pa.us
This fills in the work left un-done by 5f1148224. \prompt can
be canceled out of now, and so can password prompts issued during
\connect. (We don't need to do anything for password prompts
issued during startup, because we aren't yet trapping SIGINT
at that point.)
Nathan Bossart
Discussion: https://postgr.es/m/747443.1635536754@sss.pgh.pa.us
The following improvements are done:
- Addition of some tab completion for CREATE DOMAIN.
- Addition of some tab completion for CREATE TRANSFORM.
- Addition of type completion for CREATE SEQUENCE AS.
Author: Ken Kato
Reviewed-by: Kyotaro Horiguchi, Michael Paquier
Discussion: https://postgr.es/m/8d370135aef066659eef8e8fbfa6315b@oss.nttdata.com
Up to now, you couldn't escape out of psql's \password command
by typing control-C (or other local spelling of SIGINT). This
is pretty user-unfriendly, so improve it. To do so, we have to
modify the functions provided by pg_get_line.c; but we don't
want to mess with psql's SIGINT handler setup, so provide an
API that lets that handler cause the cancel to occur.
This relies on the assumption that we won't do any major harm by
longjmp'ing out of fgets(). While that's obviously a little shaky,
we've long had the same assumption in the main input loop, and few
issues have been reported.
psql has some other simple_prompt() calls that could usefully
be improved the same way; for now, just deal with \password.
Nathan Bossart, minor tweaks by me
Discussion: https://postgr.es/m/747443.1635536754@sss.pgh.pa.us
The error handling here was a mess, as a result of a fundamentally
bad design (relying on errno to keep its value much longer than is
safe to assume) as well as a lot of just plain sloppiness, both as
to noticing errors at all and as to reporting the correct errno.
Moreover, the recent addition of LZ4 compression broke things
completely, because liblz4 doesn't use errno to report errors.
To improve matters, keep the error state in the DirectoryMethodData or
TarMethodData struct, and add a string field so we can handle cases
that don't set errno. (The tar methods already had a version of this,
but it can be done more efficiently since all these cases use a
constant error string.) Make the dir and tar methods handle errors
in basically identical ways, which they didn't before.
This requires copying errno into the state struct in a lot of places,
which is a bit tedious, but it has the virtue that we can get rid of
ad-hoc code to save and restore errno in a number of places ... not
to mention that it fixes other places that should've saved/restored
errno but neglected to.
In passing, fix some pointlessly static buffers to be ordinary
local variables.
There remains an issue about exactly how to handle errors from
fsync(), but that seems like material for its own patch.
While the LZ4 problems are new, all the rest of this is fixes for
old bugs, so backpatch to v10 where walmethods.c was introduced.
Patch by me; thanks to Michael Paquier for review.
Discussion: https://postgr.es/m/1343113.1636489231@sss.pgh.pa.us
Coverity complained that applying get_gz_error after a failed gzclose,
as we did in one place in pg_basebackup, is unsafe. I think it's
right: it's entirely likely that the call is touching freed memory.
Change that to inspect errno, as we do for other gzclose calls.
Also, be careful to initialize errno to zero immediately before any
gzclose() call where we care about the error status. (There are
some calls where we don't, because we already failed at some previous
step.) This ensures that we don't get a misleadingly irrelevant
error code if gzclose() fails in a way that doesn't set errno.
We could work harder at that, but it looks to me like all such cases
are basically can't-happen if we're not misusing zlib, so it's
not worth the extra notational cruft that would be required.
Also, fix several places that simply failed to check for close-time
errors at all, mostly at some remove from the close or gzclose itself;
and one place that did check but didn't bother to report the errno.
Back-patch to v12. These mistakes are older than that, but between
the frontend logging API changes that happened in v12 and the fact
that frontend code can't rely on %m before that, the patch would need
substantial revision to work in older branches. It doesn't quite
seem worth the trouble given the lack of related field complaints.
Patch by me; thanks to Michael Paquier for review.
Discussion: https://postgr.es/m/1343113.1636489231@sss.pgh.pa.us
The documentation says plainly that \password acts on "the current user"
by default. What it actually acted on, or tried to, was the username
used to log into the current session. This is not the same thing if
one has since done SET ROLE or SET SESSION AUTHENTICATION. Aside from
the possible surprise factor, it's quite likely that the current role
doesn't have permissions to set the password of the original role.
To fix, use "SELECT CURRENT_USER" to get the role name to act on.
(This syntax works with servers at least back to 7.0.) Also, in
hopes of reducing confusion, include the role name that will be
acted on in the password prompt.
The discrepancy from the documentation makes this a bug, so
back-patch to all supported branches.
Patch by me; thanks to Nathan Bossart for review.
Discussion: https://postgr.es/m/747443.1635536754@sss.pgh.pa.us
Earlier versions of PostgreSQL featured a version of pg_basebackup
that wanted to edit tar archives but was too dumb to parse them
properly. The server made things easier for the client by failing
to add the two blocks of zero bytes that ought to end a tar file,
leaving it up to the client to do that.
But since commit 23a1c6578c, we
don't need this hack any more, because pg_basebackup is now smarter
and can parse tar files even if they are properly terminated! So
change the server to always properly terminate the tar files. Older
versions of pg_basebackup can't talk to new servers anyway, so
there's no compatibility break.
On the pg_basebackup side, we see still need to add the terminating
zero bytes if we're talking to an older server, but not when the
server is v15+. Hopefully at some point we'll be able to remove
some of this compatibility cruft, but it seems best to hang on to
it for now.
In passing, add a file header comment to bbstreamer_tar.c, to make
it clearer what's going on here.
Discussion: http://postgr.es/m/CA+TgmoZbNzsWwM4BE5Jb_qHncY817DYZwGf+2-7hkMQ27ZwsMQ@mail.gmail.com
Commit 5a2832465f introduced some enums to represent all tables in schema
publications and used REL in their names. Use TABLE instead of REL in
those enums to avoid confusion with other objects like SEQUENCES that can
be part of a publication in the future.
In the passing, (a) Change one of the newly introduced error messages to
make it consistent for Create and Alter commands, (b) add missing alias in
one of the SQL Statements that is used to print publications associated
with the table.
Reported-by: Tomas Vondra, Peter Smith
Author: Vignesh C
Reviewed-by: Hou Zhijie, Peter Smith
Discussion: https://www.postgresql.org/message-id/CALDaNm0OANxuJ6RXqwZsM1MSY4s19nuH3734j4a72etDwvBETQ%40mail.gmail.com
Commit 23a1c6578c improved
pg_basebackup's ability to parse tar archives, but also arranged
to parse them only when we need to make some modification to the
contents of the archive. That's a problem, because the server
doesn't actually terminate tar archives. When the new parsing
logic was engaged, pg_basebackup would properly terminate the
tar file, but when it was skipped, pg_basebackup would just write
whatever it got from the server, meaning that the terminator
was missing.
Most versions of tar are willing to overlook the missing terminator, but
the AIX buildfarm animals were not. Fix by inventing a new kind of
bbstreamer that just blindly adds a terminator, and using it whenever we
don't parse the tar archive.
Discussion: http://postgr.es/m/CA+TgmoZbNzsWwM4BE5Jb_qHncY817DYZwGf+2-7hkMQ27ZwsMQ@mail.gmail.com
Re-ordering the #include's is a bit problematic here because
libpq/libpq-be.h needs to include <openssl/ssl.h>. Instead,
let's #undef the unwanted macro after all the #includes.
This is definitely uglier than the other way, but it should
work despite possible future header rearrangements.
(A look at the openssl headers indicates that X509_NAME is the
only conflicting symbol that we use.)
In passing, remove a related but long-incorrect comment in
pg_backup_archiver.h.
Discussion: https://postgr.es/m/1051867.1635720347@sss.pgh.pa.us
pg_basebackup knows how to do quite a few things with a backup that it
gets from the server, like just write out the files, or compress them
first, or even parse the tar format and inject a modified
postgresql.auto.conf file into the archive generated by the server.
Unforatunely, this makes pg_basebackup.c a very large source file, and
also somewhat difficult to enhance, because for example the knowledge
that the server is sending us a 'tar' file rather than some other sort
of archive is spread all over the place rather than centralized.
In an effort to improve this situation, this commit invents a new
'bbstreamer' abstraction. Each archive received from the server is
fed to a bbstreamer which may choose to dispose of it or pass it
along to some other bbstreamer. Chunks may also be "labelled"
according to whether they are part of the payload data of a file
in the archive or part of the archive metadata.
So, for example, if we want to take a tar file, modify the
postgresql.auto.conf file it contains, and the gzip the result
and write it out, we can use a bbstreamer_tar_parser to parse the
tar file received from the server, a bbstreamer_recovery_injector
to modify the contents of postgresql.auto.conf, a
bbstreamer_tar_archiver to replace the tar headers for the file
modified in the previous step with newly-built ones that are
correct for the modified file, and a bbstreamer_gzip_writer to
gzip and write the resulting data. Only the objects with "tar"
in the name know anything about the tar archive format, and in
theory we could re-archive using some other format rather than
"tar" if somebody wanted to write the code.
These chances do add a substantial amount of code, but I think the
result is a lot more maintainable and extensible. pg_basebackup.c
itself shrinks by roughly a third, with a lot of the complexity
previously contained there moving into the newly-added files.
Patch by me. The larger patch series of which this is a part has been
reviewed and tested at various times by Andres Freund, Sumanta
Mukherjee, Dilip Kumar, Suraj Kharage, Dipesh Pandit, Tushar Ahuja,
Mark Dilger, Sergei Kornilov, and Jeevan Ladhe.
Discussion: https://postgr.es/m/CA+TgmoZGwR=ZVWFeecncubEyPdwghnvfkkdBe9BLccLSiqdf9Q@mail.gmail.com
Discussion: https://postgr.es/m/CA+TgmoZvqk7UuzxsX1xjJRmMGkqoUGYTZLDCH8SmU1xTPr1Xig@mail.gmail.com
Expand the checks of toasted attributes to complain if the rawsize is
overlarge. For compressed attributes, also complain if compression
appears to have expanded the attribute or if the compression method is
invalid.
Mark Dilger, reviewed by Justin Pryzby, Alexander Alekseev, Heikki
Linnakangas, Greg Stark, and me.
Discussion: http://postgr.es/m/8E42250D-586A-4A27-B317-8B062C3816A8@enterprisedb.com
Completion is added for more object types, like domain constraints, text
search-ish objects or policies. Moreover, the area is reorganized,
changing the list of objects supported by COMMENT to be in the same
order as the documentation to ease future additions.
Author: Ken Kato
Reviewed-by: Fujii Masao, Shinya Kato, Suraj Khamkar, Michael Paquier
Discussion: https://postgr.es/m/6e0c2f3f657b229bea32d098d118f307@oss.nttdata.com
pg_receivewal gains a new option, --compression-method=lz4, available
when the code is compiled with --with-lz4. Similarly to gzip, this
gives the possibility to compress archived WAL segments with LZ4. This
option is not compatible with --compress.
The implementation uses LZ4 frames, and is compatible with simple lz4
commands. Like gzip, using --synchronous ensures that any data will be
flushed to disk within the current .partial segment, so as it is
possible to retrieve as much WAL data as possible even from a
non-completed segment (this requires completing the partial file with
zeros up to the WAL segment size supported by the backend after
decompression, but this is the same as gzip).
The calculation of the streaming start LSN is able to transparently find
and check LZ4-compressed segments. Contrary to gzip where the
uncompressed size is directly stored in the object read, the LZ4 chunk
protocol does not store the uncompressed data by default. There is
contentSize that can be used with LZ4 frames by that would not help if
using an archive that includes segments compressed with the defaults of
a "lz4" command, where this is not stored. So, this commit has taken
the most extensible approach by decompressing the already-archived
segment to check its uncompressed size, through a blank output buffer in
chunks of 64kB (no actual performance difference noticed with 8kB, 16kB
or 32kB, and the operation in itself is actually fast).
Tests have been added to verify the creation and correctness of the
generated LZ4 files. The latter is achieved by the use of command
"lz4", if found in the environment.
The tar-based WAL method in walmethods.c, used now only by
pg_basebackup, does not know yet about LZ4. Its code could be extended
for this purpose.
Author: Georgios Kokolatos
Reviewed-by: Michael Paquier, Jian Guo, Magnus Hagander, Dilip Kumar
Discussion: https://postgr.es/m/ZCm1J5vfyQ2E6dYvXz8si39HQ2gwxSZ3IpYaVgYa3lUwY88SLapx9EEnOf5uEwrddhx2twG7zYKjVeuP5MwZXCNPybtsGouDsAD1o2L_I5E=@pm.me
The option name was incorrect in one of the error messages, and the
short option 'I' was used in the code but we did not intend things to be
this way. While on it, fix the documentation to refer to a "method",
and not a "level.
Oversights in commit d62bcc8, that I have detected after more review of
the LZ4 patch for pg_receivewal.
pg_receivewal includes since cada1af the option --compress, to allow the
compression of WAL segments using gzip, with a value of 0 (the default)
meaning that no compression can be used.
This commit introduces a new option, called --compression-method, able
to use as values "none", the default, and "gzip", to make things more
extensible. The case of --compress=0 becomes fuzzy with this option
layer, so we have made the choice to make pg_receivewal return an error
when using "none" and a non-zero compression level, meaning that the
authorized values of --compress are now [1,9] instead of [0,9]. Not
specifying --compress with "gzip" as compression method makes
pg_receivewal use the default of zlib instead (Z_DEFAULT_COMPRESSION).
The code in charge of finding the streaming start LSN when scanning the
existing archives is refactored and made more extensible. While on it,
rename "compression" to "compression_level" in walmethods.c, to reduce
the confusion with the introduction of the compression method, even if
the tar method used by pg_basebackup does not rely on the compression
method (yet, at least), but just on the compression level (this area
could be improved more, actually).
This is in preparation for an upcoming patch that adds LZ4 support to
pg_receivewal.
Author: Georgios Kokolatos
Reviewed-by: Michael Paquier, Jian Guo, Magnus Hagander, Dilip Kumar,
Robert Haas
Discussion: https://postgr.es/m/ZCm1J5vfyQ2E6dYvXz8si39HQ2gwxSZ3IpYaVgYa3lUwY88SLapx9EEnOf5uEwrddhx2twG7zYKjVeuP5MwZXCNPybtsGouDsAD1o2L_I5E=@pm.me
Previously failures of initial connection and logfile open caused pgbench
to proceed the benchmarking, report the incomplete results and exit with
status 2. It didn't make sense to proceed the benchmarking even when
pgbench could not start as prescribed.
This commit improves pgbench so that early errors that occur when
starting benchmark such as those failures should make pgbench exit
immediately with status 1.
Author: Yugo Nagata
Reviewed-by: Fabien COELHO, Kyotaro Horiguchi, Fujii Masao
Discussion: https://postgr.es/m/TYCPR01MB5870057375ACA8A73099C649F5349@TYCPR01MB5870.jpnprd01.prod.outlook.com
pg_receivewal is able to follow a timeline switch, but this was not
tested. This test uses an empty archive location with a restart done
from a slot, making its implementation a tad simpler than if we would
reuse an existing archive directory.
Author: Ronan Dunklau
Reviewed-by: Kyotaro Horiguchi, Michael Paquier
Discussion: https://postgr.es/m/18708360.4lzOvYHigE@aivenronan
Rearrange src/test/perl/README so that the first section is more
clearly "how to run these tests", and the rest "how to write new
tests". Add some basic info there about debugging test failures.
Then, add cross-refs to that READNE from other READMEs that
describe how to run TAP tests.
Per suggestion from Kevin Burke, though this is not his original
patch.
Discussion: https://postgr.es/m/CAKcy5eiSbwiQnmCfnOnDCVC7B8fYyev3E=6pvvECP9pLE-Fcuw@mail.gmail.com
This commit improves the speed of those tests by 25~30%, using some
simple ideas to reduce the amount of data written by pg_receivewal:
- Use a segment size of 1MB. While reducing the amount of data zeroed
by pg_receivewal for the new segments, this improves the code coverage
with a non-default segment size.
- In the last test involving a slot's restart_lsn, generate a checkpoint
to advance the redo LSN and the WAL retained by the slot created,
reducing the number of segments that need to be archived. This counts
for most of the gain.
- Minimize the amount of data inserted into the dummy table.
Reviewed-by: Ronan Dunklau
Discussion: https://postgr.es/m/YXqYKAdVEqmyTltK@paquier.xyz
As 98ec35b has proved, there has never been any coverage in this area of
the code. This commit adds a new TAP test with a template database that
includes a small set of shared dependencies copied to a new database.
The test is added in createdb, where we have never tested that -T
generates a query with TEMPLATE, either.
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/YXDTl+PfSnqmbbkE@paquier.xyz
A new option "FOR ALL TABLES IN SCHEMA" in Create/Alter Publication allows
one or more schemas to be specified, whose tables are selected by the
publisher for sending the data to the subscriber.
The new syntax allows specifying both the tables and schemas. For example:
CREATE PUBLICATION pub1 FOR TABLE t1,t2,t3, ALL TABLES IN SCHEMA s1,s2;
OR
ALTER PUBLICATION pub1 ADD TABLE t1,t2,t3, ALL TABLES IN SCHEMA s1,s2;
A new system table "pg_publication_namespace" has been added, to maintain
the schemas that the user wants to publish through the publication.
Modified the output plugin (pgoutput) to publish the changes if the
relation is part of schema publication.
Updates pg_dump to identify and dump schema publications. Updates the \d
family of commands to display schema publications and \dRp+ variant will
now display associated schemas if any.
Author: Vignesh C, Hou Zhijie, Amit Kapila
Syntax-Suggested-by: Tom Lane, Alvaro Herrera
Reviewed-by: Greg Nancarrow, Masahiko Sawada, Hou Zhijie, Amit Kapila, Haiying Tang, Ajin Cherian, Rahila Syed, Bharath Rupireddy, Mark Dilger
Tested-by: Haiying Tang
Discussion: https://www.postgresql.org/message-id/CALDaNm0OANxuJ6RXqwZsM1MSY4s19nuH3734j4a72etDwvBETQ@mail.gmail.com
Prior to this patch, when running pg_receivewal, the streaming start
point would be the current location of the archives if anything is
found in the local directory where WAL segments are written, and
pg_receivewal would fall back to the current WAL flush location if there
are no archives, as of the result of an IDENTIFY_SYSTEM command.
If for some reason the WAL files from pg_receivewal were moved, it is
better to try a restart where we left at, which is the replication
slot's restart_lsn instead of skipping right to the current flush
location, to avoid holes in the WAL backed up. This commit changes
pg_receivewal to use the following sequence of methods to determine the
starting streaming LSN:
- Scan the local archives.
- Use the slot's restart_lsn, if supported by the backend and if a slot
is defined.
- Fallback to the current flush LSN as reported by IDENTIFY_SYSTEM.
To keep compatibility with older server versions, we only attempt to use
READ_REPLICATION_SLOT if the backend version is at least 15, and
fallback to the older behavior of streaming from the current flush
LSN if the command is not supported.
Some TAP tests are added to cover this feature.
Author: Ronan Dunklau
Reviewed-by: Kyotaro Horiguchi, Michael Paquier, Bharath Rupireddy
Discussion: https://postgr.es/m/18708360.4lzOvYHigE@aivenronan
I found these by running pg_dump under "valgrind --leak-check=full".
The changes in flagInhIndexes() and getIndexes() replace allocation of
an array of which we use only some elements by individual allocations
of just the actually-needed objects. The previous coding wasted some
memory, but more importantly it confused valgrind's leak tracking.
collectComments() and collectSecLabels() remain major blots on
the valgrind report, because they don't PQclear their query
results, in order to avoid a lot of strdup's. That's a dubious
tradeoff, but I'll leave it alone here; an upcoming patch will
modify those functions enough to justify changing the tradeoff.
The five modules in our TAP test framework all had names in the top
level namespace. This is unwise because, even though we're not
exporting them to CPAN, the names can leak, for example if they are
exported by the RPM build process. We therefore move the modules to the
PostgreSQL::Test namespace. In the process PostgresNode is renamed to
Cluster, and TestLib is renamed to Utils. PostgresVersion becomes simply
PostgreSQL::Version, to avoid possible confusion about what it's the
version of.
Discussion: https://postgr.es/m/aede93a4-7d92-ef26-398f-5094944c2504@dunslane.net
Reviewed by Erik Rijkers and Michael Paquier
CIC and REINDEX CONCURRENTLY assume backends see their catalog changes
no later than each backend's next transaction start. That failed to
hold when a backend absorbed a relevant invalidation in the middle of
running RelationBuildDesc() on the CIC index. Queries that use the
resulting index can silently fail to find rows. Fix this for future
index builds by making RelationBuildDesc() loop until it finishes
without accepting a relevant invalidation. It may be necessary to
reindex to recover from past occurrences; REINDEX CONCURRENTLY suffices.
Back-patch to 9.6 (all supported versions).
Noah Misch and Andrey Borodin, reviewed (in earlier versions) by Andres
Freund.
Discussion: https://postgr.es/m/20210730022548.GA1940096@gust.leadboat.com
Create a hash table that indexes dumpable objects by CatalogId
(that is, catalog OID + object OID). Use this to replace the
former catalogIdMap array, as well as various other single-
catalog index arrays, and also the extension membership map.
In principle this should be faster for databases with many objects,
since lookups are now O(1) not O(log N). However, it seems that these
lookups are pretty much negligible in context, so that no overall
performance change can be measured. But having only one lookup
data structure to maintain makes the code simpler and more flexible,
so let's do it anyway.
Discussion: https://postgr.es/m/2595220.1634855245@sss.pgh.pa.us
Non-global default privilege entries should be dumped as-is,
not made relative to the default ACL for their object type.
This would typically only matter if one had revoked some
on-by-default privileges in a global entry, and then wanted
to grant them again in a non-global entry.
Per report from Boris Korzun. This is an old bug, so back-patch
to all supported branches.
Neil Chen, test case by Masahiko Sawada
Discussion: https://postgr.es/m/111621616618184@mail.yandex.ru
Discussion: https://postgr.es/m/CAA3qoJnr2+1dVJObNtfec=qW4Z0nz=A9+r5bZKoTSy5RDjskMw@mail.gmail.com
Along the same lines as 047329624, ed2c7f65b and daa9fe8a5, reduce
code duplication by having just one copy of the parts of the query
that are the same across all server versions; and make the
conditionals control the smallest possible amount of code.
This also gets rid of the confusing assortment of different ways
to accomplish the same result that we had here before.
While at it, make sure all three relevant parts of the function
list the fields in the same order. This is just neatnik-ism,
of course.
Discussion: https://postgr.es/m/1240992.1634419055@sss.pgh.pa.us
If the blob TOC file cannot be parsed, the error message was failing
to print the filename as the variable holding it was shadowed by the
destination buffer for parsing. When the filename fails to parse,
the error will print an empty string:
./pg_restore -d foo -F d dump
pg_restore: error: invalid line in large object TOC file "": ..
..instead of the intended error message:
./pg_restore -d foo -F d dump
pg_restore: error: invalid line in large object TOC file "dump/blobs.toc": ..
Fix by renaming both variables as the shared name was too generic to
store either and still convey what the variable held.
Backpatch all the way down to 9.6.
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/A2B151F5-B32B-4F2C-BA4A-6870856D9BDE@yesql.se
Backpatch-through: 9.6
Make sure that the string parsing is limited by the size of the
destination buffer.
In pg_basebackup the available values sent from the server
is limited to two characters so there was no risk of overflow.
In pg_dump the buffer is bounded by MAXPGPATH, and thus the limit
must be inserted via preprocessor expansion and the buffer increased
by one to account for the terminator. There is no risk of overflow
here, since in this case, the buffer scanned is smaller than the
destination buffer.
Backpatch the pg_basebackup fix to 11 where it was introduced, and
the pg_dump fix all the way down to 9.6.
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/B14D3D7B-F98C-4E20-9459-C122C67647FB@yesql.se
Backpatch-through: 11 and 9.6
The tests added by c0280bc and d9ddc50 in 001_basic.pl have introduced
commands calling directly psql, making them sensitive to the
environment. One issue was that those commands forgot -X to not use a
local .psqlrc, causing all those tests to fail if psql cannot properly
parse this file.
TAP tests should be designed so as they run in an isolated fashion,
without any dependency on the environment where they are run. As
PostgresNode::psql gives already all the facilities those new tests
need, switch to that instead of calling plain psql commands where
interactions with a backend are needed. The test is slightly refactored
to be able to check after the expected patterns of stdout and stderr,
keeping the same amount of coverage as previously.
Reported-by: Peter Geoghegan
Discussion: https://postgr.es/m/CAH2-Wzn8ftvcDPwomn+y04JJzbT=TG7TN=QsmSEATUOW-ZuvQQ@mail.gmail.com
It was clearly the intent to do so all along, but the original coding
fat-fingered this by checking the wrong array element. We fixed it
in passing in 403a3d91c, but that later got reverted, and we forgot
to keep this bug fix.
Most of the time this'd be relatively harmless, since once we lock
any of the partitioned table's leaf partitions, that would suffice
to prevent major DDL on the partitioned table itself. However, a
childless partitioned table would get dumped with no relevant lock
whatsoever, possibly allowing dump failure or inconsistent output.
Unlike 403a3d91c, there are no versioning concerns, since every server
version that has partitioned tables will allow you to lock one.
Back-patch to v10 where partitioned tables were introduced.
Discussion: https://postgr.es/m/1018205.1634346327@sss.pgh.pa.us
Avoid calling contrib/amcheck functions with relations that are
unsuitable for checking. Specifically, don't attempt verification of
temporary relations, or indexes whose pg_index entry indicates that the
index is invalid, or not ready.
These relations are not supported by any of the contrib/amcheck
functions, for reasons that are pretty fundamental. For example, the
implementation of REINDEX CONCURRENTLY can add its own "transient"
pg_index entries, which has rather unclear implications for the B-Tree
verification functions, at least in the general case -- so they just
treat it as an error. It falls to the amcheck caller (in this case
pg_amcheck) to deal with the situation at a higher level.
pg_amcheck now simply treats these conditions as additional "visibility
concerns" when it queries system catalogs. This is a little arbitrary.
It seems to have the least problems among any of the available
alternatives.
Author: Mark Dilger <mark.dilger@enterprisedb.com>
Reported-By: Alexander Lakhin <exclusion@gmail.com>
Reviewed-By: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: Robert Haas <robertmhaas@gmail.com>
Bug: #17212
Discussion: https://postgr.es/m/17212-34dd4a1d6bba98bf@postgresql.org
Backpatch: 14-, where pg_amcheck was introduced.
This fixes a set of issues that cause different breakages or annoyances
when using pg_upgrade's test.sh to do upgrades across different major
versions:
- test.sh is completely broken when using v14 as new version because of
the removal of testtablespace/ as Makefile rule. Older versions of
pg_regress don't support --make-tablespacedir, blocking the creation of
the tablespace. In order to fix that, it is simple enough to create
those directories in the script itself, but only do that when an old
version is involved. This fix is needed on HEAD and REL_14_STABLE.
- The script would fail when using PG <= v11 as old version because of
WITH OIDS relations not supported in v12. In order to fix this, this
steals a method from the buildfarm that uses a DO block to change all
the relations marked as WITH OIDS, allowing pg_upgrade to pass. This is
more portable than using ALTER TABLE queries on the relations causing
issues. This is fixed down to v12, and authored originally by Andrew
Dunstan.
- Not using --extra-float-digits=0 with v11 as old version causes
a lot of diffs in the dumps, making the whole unreadable. This gets
only done when using v11 as old version. This is fixed down to v12.
The buildfarm code uses that already.
Note that the addition of --wal-segsize and --allow-group-access breaks
the script when using v10 or older at initdb time as these got added in
11. 10 would be EOL'd next year and nobody has complained about those
problems yet, so nothing is done about that. This means that this
commit fixes upgrade tests using test.sh with v11 as minimum older
version, up to HEAD, and that it is enough to apply this change down to
12. The old and new dumps still generate diffs, still require manual
checks, and more could be done to reduce the noise, but this allows the
tests to run with a rather minimal amount of them.
I have tested this commit and test.sh with v11 as minimum across all the
branches where this is applied. Note that this commit has no impact on
the normal pg_upgrade test run with a simple "make check".
Author: Justin Pryzby, Andrew Dunstan, Michael Paquier
Discussion: https://postgr.es/m/20201206180248.GI24052@telsasoft.com
Backpatch-through: 12
A repeated complaint was that scan-build thought that if the \timing
setting changes during processing of a query, the post-processing
might read garbage time values. This is probably not possible right
now, but it's not entirely inconceivable given the code structure. So
silence this warning with small restructuring that makes this more
robust. The other warnings were a few dead stores that are easy to
remove.
Discussion: https://www.postgresql.org/message-id/2570e2ae-fa0f-aac9-f72f-bb59a9983a20@enterprisedb.com
Incrementing the level of the call stack reported is useful for
debugging purposes as it allows to control which part of the test is
exactly failing, especially if a test is structured with subroutines
that call routines from Test::More.
This adds more incrementations of $Test::Builder::Level where debugging
gets improved (for example it does not make sense for some paths like
pg_rewind where long subroutines are used).
A note is added to src/test/perl/README about that, based on a
suggestion from Andrew Dunstan and a wording coming from both of us.
Usage of Test::Builder::Level has spread in 12, so a backpatch down to
this version is done.
Reviewed-by: Andrew Dunstan, Peter Eisentraut, Daniel Gustafsson
Discussion: https://postgr.es/m/YV1CCFwgM1RV1LeS@paquier.xyz
Backpatch-through: 12
Like BASE_BACKUP, CREATE_REPLICATION_SLOT has historically used a
hard-coded syntax. To improve future extensibility, adopt a flexible
options syntax here, too.
In the new syntax, instead of three mutually exclusive options
EXPORT_SNAPSHOT, USE_SNAPSHOT, and NOEXPORT_SNAPSHOT, there is now a single
SNAPSHOT option with three possible values: 'export', 'use', and 'nothing'.
This commit does not remove support for the old syntax. It just adds
the new one as an additional option, makes pg_receivewal,
pg_recvlogical, and walreceiver processes use it.
Patch by me, reviewed by Fabien Coelho, Sergei Kornilov, and
Fujii Masao.
Discussion: http://postgr.es/m/CA+TgmobAczXDRO_Gr2euo_TxgzaH1JxbNxvFx=HYvBinefNH8Q@mail.gmail.com
Discussion: http://postgr.es/m/CA+TgmoZGwR=ZVWFeecncubEyPdwghnvfkkdBe9BLccLSiqdf9Q@mail.gmail.com
Previously, BASE_BACKUP used an entirely hard-coded syntax, but that's
hard to extend. Instead, adopt the same kind of syntax we've used for
SQL commands such as VACUUM, ANALYZE, COPY, and EXPLAIN, where it's
not necessary for all of the option names to be parser keywords.
In the new syntax, most of the options now take an optional Boolean
argument. To match our practice in other in places, the options which
the old syntax called NOWAIT and NOVERIFY_CHECKSUMS options are in the
new syntax called WAIT and VERIFY_CHECKUMS, and the default value is
false. In the new syntax, the FAST option has been replaced by a
CHECKSUM option whose value may be 'fast' or 'spread'.
This commit does not remove support for the old syntax. It just adds
the new one as an additional option, and makes pg_basebackup prefer
the new syntax when the server is new enough to support it.
Patch by me, reviewed and tested by Fabien Coelho, Sergei Kornilov,
Fujii Masao, and Tushar Ahuja.
Discussion: http://postgr.es/m/CA+TgmobAczXDRO_Gr2euo_TxgzaH1JxbNxvFx=HYvBinefNH8Q@mail.gmail.com
Discussion: http://postgr.es/m/CA+TgmoZGwR=ZVWFeecncubEyPdwghnvfkkdBe9BLccLSiqdf9Q@mail.gmail.com
Per discussion, let's just follow CLDR's default zone mappings
faithfully. There are two changes here that are clear improvements:
* Mapping "Greenwich Standard Time" to Atlantic/Reykjavik is actually
a better fit than using London, because Iceland hasn't observed DST
since 1968, so this is more nearly what people might expect.
* Since the "Samoa" zone is specified to be UTC+13:00, we must map
it to Pacific/Apia not Pacific/Samoa; the latter refers to American
Samoa which is now on the other side of the date line.
The rest of these changes look like they're choosing the most populous
IANA zone as representative. Whatever the details, we're just going
to say "if you don't like this mapping, complain to CLDR".
Discussion: https://postgr.es/m/3266414.1633045628@sss.pgh.pa.us
This corrects a bunch of entries in win32_tzmap[], and adds a few
new ones, based on the CLDR project's windowsZones.xml file.
Non-cosmetic changes fall into four main categories:
* Flat-out errors:
US/Aleutan doesn't exist
America/Salvador doesn't exist
Asia/Baku is wrong for Yerevan
Asia/Dhaka (Bangladesh) is wrong for Astana (Kazakhstan)
Europe/Bucharest is wrong for Chisinau
America/Mexico_City is wrong for Chetumal
America/Buenos_Aires is wrong for Cayenne
America/Caracas has its own zone, so poor fit for La Paz
US/Eastern is wrong for Haiti
US/Eastern is wrong for Indiana (East)
Asia/Karachi is wrong for Tashkent
Etc/UTC+12 doesn't exist
Signs of Etc/GMT zones were backwards
* Judgment calls:
(These changes follow CLDR's choices, except for the first one)
Use Europe/London for "Greenwich Standard Time", since that seems much
more likely than Africa/Casablanca to be what people will think that
zone name means. CLDR has Atlantic/Reykjavik here, but that's no better.
Asia/Shanghai seems a better fit than Hong Kong for "China Standard
Time".
Europe/Sarajevo is now a link to Belgrade, ie "Central Europe Standard
Time"; so use Warsaw for "Central European Standard Time".
America/Sao_Paulo seems more representative than Araguaina for
"E. South America Standard Time".
Africa/Johannesburg seems more representative than Harare for
"South Africa Standard Time".
* New Windows zone names:
"Israel Standard Time"
"Kaliningrad Standard Time"
"Russia Time Zone N" for various N
"Singapore Standard Time"
"South Sudan Standard Time"
"W. Central Africa Standard Time"
"West Bank Standard Time"
"Yukon Standard Time"
Some of these replace older spellings, but I kept the older spellings
too in case our code runs on a machine with the older data.
* Replace aliases (tzdb Links) with underlying city-named zones:
(This tracks tzdb's longstanding practice, and reduces inconsistency
with the rest of the entries, as well as with CLDR.)
US/Alaska
Asia/Kuwait
Asia/Muscat
Canada/Atlantic
Australia/Canberra
Canada/Saskatchewan
US/Central
US/Eastern
US/Hawaii
US/Mountain
Canada/Newfoundland
US/Pacific
Back-patch to all supported branches, as is our usual practice for
time zone data updates.
Discussion: https://postgr.es/m/3266414.1633045628@sss.pgh.pa.us
The original intent seems to have been to sort case-insensitively
by the Windows zone name, but various changes over the years did
not get that memo. This commit just moves a few entries to
restore exact alphabetic order, to ease comparison to the outputs
of processing scripts.
Back-patch to all supported branches, as is our usual practice for
time zone data updates.
Discussion: https://postgr.es/m/3266414.1633045628@sss.pgh.pa.us
Previously socket errors such as invalid socket or socket wait method failures
during benchmark caused pgbench to exit with status 0. Instead, errors during
the run should result in exit status 2.
Back-patch to v12 where pgbench started reporting exit status.
Original complaint and patch by Hayato Kuroda.
Author: Yugo Nagata, Fabien COELHO
Reviewed-by: Kyotaro Horiguchi, Fujii Masao
Discussion: https://postgr.es/m/TYCPR01MB5870057375ACA8A73099C649F5349@TYCPR01MB5870.jpnprd01.prod.outlook.com
The failure of socket wait method like "select()" doesn't terminate pgbench.
So the log level of error message when that failure happens should be ERROR.
But previously FATAL was used in that case.
Back-patch to v13 where pgbench started using common logging API.
Author: Yugo Nagata, Fabien COELHO
Reviewed-by: Kyotaro Horiguchi, Fujii Masao
Discussion: https://postgr.es/m/20210617005934.8bd37bf72efd5f1b38e6f482@sraoss.co.jp
Sequences were left out of the list of relation kinds that
verify_heapam knew how to check, though it is fairly trivial to allow
them. Doing that, and while at it, updating pg_amcheck to include
sequences in relations matched by table and relation patterns.
Author: Mark Dilger <mark.dilger@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/flat/81ad4757-92c1-4aa3-7bee-f609544837e3%40enterprisedb.com
That allows to include just visibilitymapdefs.h from file.c, and in turn,
remove include of postgres.h from relcache.h.
Reported-by: Andres Freund
Discussion: https://postgr.es/m/20210913232614.czafiubr435l6egi%40alap3.anarazel.de
Author: Alexander Korotkov
Reviewed-by: Andres Freund, Tom Lane, Alvaro Herrera
Backpatch-through: 13
This addresses two issues with the tests added in 0c39c292 for runtime
GUCs:
- Re-enable the test on Msys. The test could fail because of \r\n
generated by Msys perl. 0d91c52a has taken care of this issue.
- Allow the test to run in the context of a privileged account. CIs
running under privileged accounts would fail on permission failures, as
reported by Andres Freund. This issue is fixed by wrapping the postgres
command within pg_ctl as the latter will take care of any permissions
needed. The test checking a failure of postgres -C for a runtime
parameter with an instance running is removed, as pg_ctl produces an
unstable error code (no need for a CI to reproduce that).
Discussion: https://postgr.es/m/20210921032040.lyl4lcax37aedx2x@alap3.anarazel.de
The following things are improved:
- Fetch the system identifier from the source server before any
WAL streaming loop. This triggers extra checks to make sure that
pg_receivewal is still connected to a server with the same system ID
with a correct timeline.
- Switch umask() (for file creation mode mask) and RetrieveWalSegSize()
(to fetch the size of WAL segments) a bit later before the initial
stream attempt. If the connection was done with a database,
pg_receivewal would fail but those commands were still executed, which
was a waste. The slot creation and drop are now done before retrieving
the segment size.
Author: Bharath Rupireddy
Reviewed-by: Ronan Dunklau, Michael Paquier
Discussion: https://postgr.es/m/CALj2ACX00YYeyBfoi55Cy=NrP-FcfMgiYYx1qRUEib3yjCVoaA@mail.gmail.com
A WAL segment closed during a WAL stream for pg_receivewal would
generate incorrect error messages depending on the context, as the file
name used when referring to a WAL segment ignored partial files or the
compression method used. In such cases, the error message generated
(failure on close, seek or rename) would not match a physical file
name. The same code paths are used by pg_basebackup, but it uses no
partial suffix so it is not impacted.
7fbe0c8 has introduced in walmethods.c a callback to get the exact
physical file name used for a given context, this commit makes use of it
to improve those error messages. This could be extended to more code
paths of pg_basebackup/ in the future, if necessary.
Extracted from a larger patch by the same author.
Author: Georgios Kokolatos
Discussion: https://postgr.es/m/ZCm1J5vfyQ2E6dYvXz8si39HQ2gwxSZ3IpYaVgYa3lUwY88SLapx9EEnOf5uEwrddhx2twG7zYKjVeuP5MwZXCNPybtsGouDsAD1o2L_I5E=@pm.me
The output generated on Msys is incorrect because of the different way
IPC::Run processes outputs with native Perl (converts natively \r\n to
\n) and Msys perl (\r\n kept as-is), causing this test to fail.
For now, just disable the test to bring the buildfarm to a green state.
I think that the correct long-term solution would be to tweak all the
routines command_checks_* in PostgresNode.pm to handle this output like
psql does when using Msys, by discarding \r automatically before
comparing it.
Per report from jacana and fairywren. Thanks to Tom Lane for the ping.
Discussion: https://postgr.es/m/1252480.1631829409@sss.pgh.pa.us
Until now, the -C option of postgres was handled before a small subset
of GUCs computed at runtime are initialized, leading to incorrect
results as GUC machinery would fall back to default values for such
parameters.
For example, data_checksums could report "off" for a cluster as the
control file is not loaded yet. Or wal_segment_size would show a
segment size at 16MB even if initdb --wal-segsize used something else.
Worse, the command would fail to properly report the recently-introduced
shared_memory, that requires to load shared_preload_libraries as these
could ask for a chunk of shared memory.
Support for runtime GUCs comes with a limitation, as the operation is
now allowed on a running server. One notable reason for this is that
_PG_init() functions of loadable libraries are called before all
runtime-computed GUCs are initialized, and this is not guaranteed to be
safe to do on running servers. For the case of shared_memory_size,
where we want to know how much memory would be used without allocating
it, this limitation is fine. Another case where this will help is for
huge pages, with the introduction of a different GUC to evaluate the
amount of huge pages required for a server before starting it, without
having to allocate large chunks of memory.
This feature is controlled with a new GUC flag, and four parameters are
classified as runtime-computed as of this change:
- data_checksums
- shared_memory_size
- data_directory_mode
- wal_segment_size
Some TAP tests are added to provide some coverage here, using
data_checksums in the tests of pg_checksums.
Per discussion with Andres Freund, Justin Pryzby, Magnus Hagander and
more.
Author: Nathan Bossart
Discussion: https://postgr.es/m/F2772387-CE0F-46BF-B5F1-CC55516EB885@amazon.com
Also remove obsolete comments about why the 64-bit integers need to be
printed in a separate buffer. The reason used to be portability, but
now the remaining reason is that we need the string lengths for the
progress displays. That is evident by looking at the code right
below, so a new comment doesn't seem necessary.
These are added in the existing tests of pg_ctl for log rotation, that
already tested stderr. The same amount of coverage is added for csvlog:
- Checks for pg_current_logfile().
- Log rotation with expected file name.
- Log contents generated.
This test is refactored to minimize the amount of work required to add
tests for new log formats, easing some upcoming work.
Author: Michael Paquier, Sehrope Sarkuni
Discussion: https://postgr.es/m/CAH7T-aqswBM6JWe4pDehi1uOiufqe06DJWaU5=X7dDLyqUExHg@mail.gmail.com
This switches the default ACL to what the documentation has recommended
since CVE-2018-1058. Upgrades will carry forward any old ownership and
ACL. Sites that declined the 2018 recommendation should take a fresh
look. Recipes for commissioning a new database cluster from scratch may
need to create a schema, grant more privileges, etc. Out-of-tree test
suites may require such updates.
Reviewed by Peter Eisentraut.
Discussion: https://postgr.es/m/20201031163518.GB4039133@rfd.leadboat.com
When throttling is used, transactions that lag behind schedule by
more than the latency limit are counted and reported as skipped.
Previously, there was the case where pgbench counted all skipped
transactions even if the timer specified in -T option was exceeded.
This could take a very long time to do that especially when unrealistically
high rate setting in -R option caused quite a lot of transactions that
lagged behind schedule. This could prevent pgbench from ending
immediately, and so pgbench could look like it got stuck to users.
To fix the issue, this commit changes pgbench so that it stops counting
skipped transactions as soon as the timer is exceeded. The timer can
make pgbench end soon even when there are lots of skipped transactions
that have not been counted yet.
Note that there is no guarantee that all skipped transactions are
counted under -T though there is under -t. This is OK in practice
because it's very unlikely to happen with realistic setting. Also this is
not the issue that this commit newly introdues. There used to be
the case where pgbench ended without counting all skipped
transactions since before.
Back-patch to v14. Per discussion, we decided not to bother
back-patch to the stable branches because it's hard to imagine
the issue happens in practice (with realistic setting).
Author: Yugo Nagata, Fabien COELHO
Reviewed-by: Greg Sabino Mullane, Fujii Masao
Discussion: https://postgr.es/m/20210613040151.265ff59d832f835bbcf8b3ba@sraoss.co.jp
Coverity complained that one caller of getFormattedTypeName() failed
to free the returned string. Which is true, but rather than fixing
that one, let's get rid of this tedious and error-prone requirement.
Now that getFormattedTypeName() caches its result, strdup'ing that
result and expecting the caller to free it accomplishes little except
to waste cycles. We do create a leak in the case where getTypes didn't
make a TypeInfo for the type, but that basically shouldn't ever happen.
Back-patch, as commit 6c450a861 was. This isn't a particularly
interesting bug fix, but the API change seems like a hazard for
future back-patching activity if we don't back-patch it.
Various psql backslash commands have both single-letter and long
forms, for example \e and \edit. Previously, tab completion
generally offered the single-letter form but not the long form.
It seems more sensible to offer the long form, because (a) no
useful completion can happen when you've already typed the single
letter, and (b) if you're not so familiar with the command set
as to know that, the long form is likely to be less confusing.
Haiying Tang, reviewed by Dagfinn Ilmari Mannsåker and myself
Discussion: https://postgr.es/m/OS0PR01MB61136018064660F095CB57A8FB129@OS0PR01MB6113.jpnprd01.prod.outlook.com
Previously pgwin32_is_service() would falsely return true when postgres is
started from somewhere within a service, but not as a service. That is
e.g. always the case with windows docker containers, which some CI services
use to run windows tests in.
When postgres falsely thinks its running as a service, no messages are
writting to stdout / stderr. That can be very confusing and causes a few tests
to fail.
To fix additionally check if stderr is invalid in pgwin32_is_service(). For
that to work in backend processes, pg_ctl is changed to pass down handles so
that postgres can do the same check (otherwise "default" handles are created).
While this problem exists in all branches, there have been no reports by
users, the prospective CI usage currently is only for master, and I am not a
windows expert. So doing the change in only master for now seems the sanest
approach.
Author: Andres Freund <andres@anarazel.de>
Reviewed-By: Magnus Hagander <magnus@hagander.net>
Discussion: https://postgr.es/m/20210305185752.3up5eq2eanb7ofmb@alap3.anarazel.de
Replace fixed-length command buffers with psprintf() calls. We didn't
have anything as convenient as psprintf() when this code was written,
but now that we do, there's little reason for the limitation to
stand. Removing it eliminates some corner cases where (for example)
starting the postmaster with a whole lot of options fails.
Most individual file names that pg_ctl deals with are still restricted
to MAXPGPATH, but we've seldom had complaints about that limitation
so long as it only applies to one filename.
Back-patch to all supported branches.
Phil Krylov
Discussion: https://postgr.es/m/567e199c6b97ee19deee600311515b86@krylov.eu
While populating the pgbench_accounts table, plain COPY was
unconditionally used. By changing it to COPY FREEZE, the time for
VACUUM is significantly reduced, thus the total time of "pgbench -i"
is also reduced. This only happens if pgbench runs against PostgreSQL
14 or later because COPY FREEZE in previous versions of PostgreSQL does
not bring the benefit. Also if partitioning is used, COPY FREEZE
cannot be used. In this case plain COPY will be used too.
Author: Tatsuo Ishii
Discussion: https://postgr.es/m/20210308.143907.2014279678657453983.t-ishii@gmail.com
Reviewed-by: Fabien COELHO, Laurenz Albe, Peter Geoghegan, Dean Rasheed
When -C/--connect option is specified, pgbench establishes and closes
the connection for each transaction. In this case pgbench needs to
measure the times taken for all those connections and disconnections,
to include the average connection time in the benchmark result.
But previously pgbench could not measure those disconnection delays.
To fix the bug, this commit makes pgbench measure the disconnection
delays whenever the connection is closed at the end of transaction,
if -C/--connect option is specified.
Back-patch to v14. Per discussion, we concluded not to back-patch to v13
or before because changing that behavior in stable branches would
surprise users rather than providing benefits.
Author: Yugo Nagata
Reviewed-by: Fabien COELHO, Tatsuo Ishii, Asif Rehman, Fujii Masao
Discussion: https://postgr.es/m/20210614151155.a393bc7d8fed183e38c9f52a@sraoss.co.jp
The code printing expressions for extended statistics doubled the
parens, producing results like ((a+1)), which is unnecessary and not
consistent with how we print expressions elsewhere.
Fixed by tweaking the code to produce just a single set of parens.
Reported by Mark Dilger, fix by me. Backpatch to 14, where support for
extended statistics on expressions was added.
Reported-by: Mark Dilger
Discussion: https://postgr.es/m/20210122040101.GF27167%40telsasoft.com
For no particularly good reason, getPolicies() queried pg_policy
separately for each table. We can collect all the policies in
a single query instead, and attach them to the correct TableInfo
objects using findTableByOid() lookups. On the regression
database, this reduces the number of queries substantially, and
provides a visible savings even when running against a local
server.
Per complaint from Hubert Depesz Lubaczewski. Since this is such
a simple fix and can have a visible performance benefit, back-patch
to all supported branches.
Discussion: https://postgr.es/m/20210826084430.GA26282@depesz.com
There's long been a "TODO: there might be some value in caching
the results" annotation on pg_dump's getFormattedTypeName function;
but we hadn't gotten around to checking what it was costing us to
repetitively look up type names. It turns out that when dumping the
current regression database, about 10% of the total number of queries
issued are duplicative format_type() queries. However, Hubert Depesz
Lubaczewski reported a not-unusual case where these account for over
half of the queries issued by pg_dump. Individually these queries
aren't expensive, but when network lag is a factor, they add up to a
problem. We can very easily add some caching to getFormattedTypeName
to solve it.
Since this is such a simple fix and can have a visible performance
benefit, back-patch to all supported branches.
Discussion: https://postgr.es/m/20210826084430.GA26282@depesz.com
0c013e0 has done a large refactoring to unify all the code paths using
replication commands, but forgot one code path doing WAL streaming that
checks the validity of a cluster connecting to with IDENTIFY_SYSTEM.
There is a generic routine able to handle that, so make use of it in
this code path. This impacts pg_receivewal and pg_basebackup.
Author: Bharath Rupireddy
Discussion: https://postgr.es/m/CALj2ACVKKYUMC8GE72Y7BP9g1batrrq3sEwUh+1_i2krWZC_2Q@mail.gmail.com
Per our message style guidelines, for human consumption we quote
qualified names as a whole rather than each part separately; but commits
bc085205c8 introduced a deviation for extended statistics and
a4d75c86bf copied it. I don't agree with this policy applying to
names shown by psql, but that's a poor reason to deviate from the
practice only in two obscure corners, so make said corners use the same
style as everywhere else.
Backpatch to 14. The first of these is older, but I'm not sure we want
to destabilize the psql output in the older branches for such a small
thing.
Discussion: https://postgr.es/m/20210828181618.GS26465@telsasoft.com
Commit 547f04e734 changed pgbench so that it used the measurement result
of connection delays in its benchmark report only when -C/--connect option
is specified. But previously those delays were unnecessarily measured
even when that option is not specified. Which was a waste of cycles.
This commit improves pgbench so that it avoids such unnecessary measurement.
Back-patch to v14 where commit 547f04e734 first appeared.
Author: Yugo Nagata
Reviewed-by: Fabien COELHO, Asif Rehman, Fujii Masao
Discussion: https://postgr.es/m/20210614151155.a393bc7d8fed183e38c9f52a@sraoss.co.jp
This includes the detection of new patterns for various constraint
types, with the addition of USING INDEX for unique indexes of a table
on primary keys and unique constraints.
Author: Dagfinn Ilmari Mannsåker
Discussion: https://postgr.es/m/87bl6ehhpl.fsf@wibble.ilmari.org
Déjà vu of commit fc40ba1296, for another backslash command.
Strictly speaking this isn't a bug, but since all references to catalog
objects are schema-qualified, we might as well be consistent. The
omission first appeared in commit ad600bba04 and replicated in
a4d75c86bf15; backpatch to 14.
Author: Justin Pryzby <pryzbyj@telsasoft.com>
Discussion: https://postgr.es/m/20210827193151.GN26465@telsasoft.com
Strictly speaking this isn't a bug, but since all references to catalog
objects are schema-qualified, we might as well be consistent. The
omission first appeared in commit 1c5d9270e3, so backpatch to 12.
Author: Justin Pryzby <pryzbyj@telsasoft.com>
Discussion: https://postgr.es/m/20210827193151.GN26465@telsasoft.com
When looping over the resultset from a SQL query, extracting the field
number before the loop body to avoid repeated calls to PQfnumber is an
established pattern. On very wide tables this can have a performance
impact, but it wont be noticeable in the common case. This fixes a few
queries which were extracting the field number in the loop body.
Author: Hou Zhijie <houzj.fnst@fujitsu.com>
Reviewed-by: Nathan Bossart <bossartn@amazon.com>
Discussion: https://postgr.es/m/OS0PR01MB57164C392783F29F6D0ECA0B94139@OS0PR01MB5716.jpnprd01.prod.outlook.com
The previous coding relied on the PID file appearing and the query
starting "fast enough", which can fail on slow machines. Also, there
might have been an undocumented interference between alarm and
IPC::Run. This new coding doesn't rely on any of these concurrency
mechanisms. Instead, we wait unitl the PID file is complete before
proceeding, and then also wait until the sleep query is registered by
the server.
Discussion: https://www.postgresql.org/message-id/flat/E1mH14Q-0002gh-HS%40gemulon.postgresql.org
The current refresh behavior tries to just refresh added/dropped
publications but that leads to removing wrong tables from subscription. We
can't refresh just the dropped publication because it is quite possible
that some of the tables are removed from publication by that time and now
those will remain as part of the subscription. Also, there is a chance
that the tables that were part of the publication being dropped are also
part of another publication, so we can't remove those.
So, we decided that by default, add/drop commands will also act like
REFRESH PUBLICATION which means they will refresh all the publications. We
can keep the old behavior for "add publication" but it is better to be
consistent with "drop publication".
Author: Hou Zhijie
Reviewed-by: Masahiko Sawada, Amit Kapila
Backpatch-through: 14, where it was introduced
Discussion: https://postgr.es/m/OS0PR01MB5716935D4C2CC85A6143073F94EF9@OS0PR01MB5716.jpnprd01.prod.outlook.com
In a backup manifest, WAL-Ranges stores the range of WAL that is
required for the backup to be valid. pg_verifybackup would then
internally use pg_waldump for the checks based on this data.
When the timeline where the backup started was more than 1 with a
history file looked at for the manifest data generation, the calculation
of the WAL range for the first timeline to check was incorrect. The
previous logic used as start LSN the start position of the first
timeline, but it needs to use the start LSN of the backup. This would
cause failures with pg_verifybackup, or any tools making use of the
backup manifests.
This commit adds a test based on a logic using a self-promoted node,
making it rather cheap.
Author: Kyotaro Horiguchi
Discussion: https://postgr.es/m/20210818.143031.1867083699202617521.horikyota.ntt@gmail.com
Backpatch-through: 13
Using --quiet in combination with --no-strict-names didn't work as
documented, a warning message was still emitted. Since the --quiet
flag was working in an unconventional way to other utilities, fix
by removing the functionality instead.
Backpatch through 14 where pg_amcheck was introduced.
Bug: 17148
Reported-by: Chen Jiaoqian <chenjq.jy@fujitsu.com>
Reviewed-by: Julien Rouhaud <rjuju123@gmail.com>
Discussion: https://postgr.es/m/17148-b5087318e2b04fc6@postgresql.org
Backpatch-through: 14
The skip options set for all-visible and all-frozen were incorrect
as they used space rather than hyphen, causing a syntax error when
invoked. Also, the option for not skipping any pages at all, none,
was documented but not implemented.
Backpatch through 14 where pg_amcheck was introduced.
Bug: #17149
Reported-by: Chen Jiaoqian <chenjq.jy@fujitsu.com>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/17149-5918ea748da36b15@postgresql.org
Backpatch-through: 14
The initdb help message for --sync-only was a bit terse, and not
really self-explanatory. Make it clearer that initdb --sync-only
will exit after syncing, and expand the docs with a note on when
the option can be useful. Also align the help output with others
that exit immediately.
Author: Nathan Bossart, Gurjeet Singh
Discussion: https://postgr.es/m/CABwTF4U6hbNNE1bv=LxQdJybmUdZ5NJQ9rKY9tN82NXM8QH+iQ@mail.gmail.com
Neither is actually initialized as an auxiliary process, so it does not really
make sense to reserve a PGPROC etc for them.
This keeps checker mode implemented by exiting partway through bootstrap
mode. That might be worth changing at some point, perhaps if we ever extend
checker mode to be a more general tool.
Author: Andres Freund <andres@anarazel.de>
Reviewed-By: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-By: Robert Haas <robertmhaas@gmail.com>
Discussion: https://postgr.es/m/20210802164124.ufo5buo4apl6yuvs@alap3.anarazel.de
Up to now we did a PQconsumeInput() for each pipelined query, asking the OS
for more input - which it often won't have, as all results might already have
been sent. That turns out to have a noticeable performance impact.
Alvaro Herrera reviewed the idea to add the PQisBusy() check, but not this
concrete patch.
Author: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/20210720180039.23rivhdft3l4mayn@alap3.anarazel.de
Backpatch: 14, where libpq/pgbench pipelining was introduced.
This reverts commit 6a2c532. fairywren and bowerbird failed those tests
because of incorrect versions of ZLIB linked to, causing errors like
SIGBREAKs that stopped buildfarm runs or EACCES failures when writing
compressed WAL segments.
Andrew Dunstan has done all the investigation here, so he deserves all
the credit for being able to enable those tests on Windows.
Discussion: https://postgr.es/m/9040d5ed-6462-66a4-07ac-2923785ae563@dunslane.net
Those tests are not designed to fail, but if they do, like on some cases
for Windows because of ZLIB (?), they could remain stuck. Using
--no-loop makes the test fail immediately. The oldest test with
--endpos already did that.
Those tests have been added in ffc9dda.
Reviewed-by: Andrew Dunstan
Discussion: https://postgr.es/m/ec093ff1-a53c-0091-46a2-4537354b0dd4@dunslane.net
Certain versions of msys2/Windows have been observed to resolve symlinks
in perl2host rather than just follow them. This defeats using a
symlinked shorter path to a longer path, and makes certain tests fail.
We therefore call perl2host on the parent directory of the symlink and
thereafter just use that result.
Apply to release 14 where the problem has been observed.
There is only one constructor now for PostgresNode, with the idiomatic
name 'new'. The method is not exported by the class, and must be called
as "PostgresNode->new('name',[args])". All the TAP tests that use
PostgresNode are modified accordingly. Third party scripts will need
adjusting, which is a fairly mechanical process (I just used a sed
script).
The following changes are done:
- In pg_archivecleanup, the cleanup of older WAL segments would never
fail immediately.
- In pgbench, the initialization of a thread barrier would not fail
hard.
- In pg_recvlogical, a stat() failure never got the call.
- In pg_basebackup, two chmod() reported a failure without exit()'ing
when unpacking some tar data freshly received. It may be possible to
continue writing some data even after this failure, but that could be
confusing to the user at the end.
These are arguably bugs, but they would happen for code paths where a
failure is unlikely going to happen, so no backpatch is done.
Reviewed-by: Robert Haas, Fabien Coelho
Discussion: https://postgr.es/m/YQDMdB+B68yePFeT@paquier.xyz
pg_verifybackup needs by default pg_waldump to check after a range of
WAL segments required for a backup, except if --no-parse-wal is
specified. The code checked for the presence of the binary pg_waldump
in an installation and reported an error, but it forgot to properly
exit(). This could lead to confusing errors reported.
Reviewed-by: Robert Haas, Fabien Coelho
Discussion: https://postgr.es/m/YQDMdB+B68yePFeT@paquier.xyz
Backpatch-through: 13
The logic used to support a change of access method for a table is
similar to changes for tablespace or relation persistence, requiring a
table rewrite with an exclusive lock of the relation changed. Table
rewrites done in ALTER TABLE already go through the table AM layer when
scanning tuples from the old relation and inserting them into the new
one, making this implementation straight-forward.
Note that partitioned tables are not supported as these have no access
methods defined.
Author: Justin Pryzby, Jeff Davis
Reviewed-by: Michael Paquier, Vignesh C
Discussion: https://postgr.es/m/20210228222530.GD20769@telsasoft.com
Add pg_resetxlog -u option to set the oldest xid in pg_control.
Previously -x set this value be -2 billion less than the -x value.
However, this causes the server to immediately scan all relation's
relfrozenxid so it can advance pg_control's oldest xid to be inside the
autovacuum_freeze_max_age range, which is inefficient and might disrupt
diagnostic recovery. pg_upgrade will use this option to better create
the new cluster to match the old cluster.
Reported-by: Jason Harvey, Floris Van Nee
Discussion: https://postgr.es/m/20190615183759.GB239428@rfd.leadboat.com, 87da83168c644fd9aae38f546cc70295@opammb0562.comp.optiver.com
Author: Bertrand Drouvot
Backpatch-through: 9.6
A check in the ZLIB portion of the test to match the name of a
non-compressed partial segment with a completed compressed segment was
using m//, while a simple equality check is enough. This makes the test
a bit stricter without impacting its coverage.
Author: Kyotaro Horiguchi
Discussion: https://postgr.es/m/20210726.174622.826565852378770261.horikyota.ntt@gmail.com
strtoint(), via strtol(), would skip leading whitespaces but the same
rule was not applied for trailing whitespaces, leading to an
inconsistent behavior. Some tests are changed to cover more this area.
Author: Michael Paquier
Reviewed-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/YP5Pv0d13Ct+03ve@paquier.xyz
Commit ad600bba04 added psql command \dX listing extended statistics
objects, but it failed to consider search_path when selecting the
elements so some of the returned elements might be invisible.
The visibility was already considered for tab completion (added by
commit d99d58cdc8), so adding it to the query is fairly simple.
Reported and fix by Justin Pryzby, regression tests by me. Backpatch
to PostgreSQL 14, where \dX was introduced.
Batchpatch-through: 14
Author: Justin Pryzby
Reviewed-by: Tatsuro Yamada
Discussion: https://postgr.es/m/c027a541-5856-75a5-0868-341301e1624b%40nttcom.co.jp_1
These have been introduced by 7fbe0c8, and could happen for
pg_basebackup and pg_receivewal.
Per report from Coverity for the ones in walmethods.c, I have spotted
the ones in receivelog.c after more review.
Backpatch-through: 10
This fixes two compilation failures caused by 6f164e6. Interesting to
see that missing <limits.h> dies not fail in Linux or even Windows. On
MacOS, it fails, though.
Per various buildfarm members.
Most of the integer options for command-line binaries now make use of a
single routine able to do the job, fixing issues with the detection of
sloppy values caused for example by the use of atoi(), that fails on
strings beginning with numerical characters with junk trailing
characters.
This commit cuts down the number of strings requiring translation by 26
per my count, switching the code to have two error types for invalid and
out-of-range values instead.
Much more could be done here, with float or even int64 options, but
int32 was the most appealing case as it is possible to rely on strtol()
to do the job reliably. Note that there are some exceptions for now,
like pg_ctl or pg_upgrade that use their own logging logic. A couple of
negative TAP tests required some adjustments for the new errors
generated.
pg_dump and pg_restore tracked the maximum number of parallel jobs
within the option parsing. The code is refactored a bit to track that
in the code dedicated to parallelism instead.
Author: Kyotaro Horiguchi, Michael Paquier
Reviewed-by: David Rowley, Álvaro Herrera
Discussion: https://postgr.es/m/CALj2ACXqdG9WhqVoJ9zYf-iZt7sgK7Szv5USs=he6NnWQ2ofTA@mail.gmail.com
This reverts commit 91d395f, to avoid running those tests on Windows.
The tests are globally stable across all buildfarm members, except
fairywren (crash of pg_receivewal) and bowerdird (SIGBREAK preventing
the buildfarm run to complete). Those errors are rather strange, as
other hosts with very similar characteristics are able to run those
tests without breaking a sweat.
For now, disable those tests on Windows to turn back the buildfarm to
green.
Per discussion with Andrew Dunstan.
Discussion: https://postgr.es/m/9040d5ed-6462-66a4-07ac-2923785ae563@dunslane.net
This is a revert of 6cea447, that disabled those tests temporarily on
Windows due to failures with bowerbird where gzflush() would fail when
executed on a freshly-opened compressed and partial segment. This
problem should be taken care of now thanks to 7fbe0c8, so let's see what
the buildfarm has to say on Windows for those tests.
Discussion: https://postgr.es/m/YPDLz2x3o1aX2wRh@paquier.xyz
The logic handling the opening of new WAL segments was fuzzy when using
--compress if a partial, non-compressed, segment with the same base name
existed in the repository storing those files. In this case, using
--compress would cause the code to first check for the existence and the
size of a non-compressed segment, followed by the opening of a new
compressed, partial, segment. The code was accidentally working
correctly on most platforms as the buildfarm has proved, except
bowerbird where gzflush() could fail in this code path. It is wrong
anyway to take the code path used pre-padding when creating a new
partial, non-compressed, segment, so let's fix it.
Note that this issue exists when users mix successive runs of
pg_receivewal with or without compression, as discovered with the tests
introduced by ffc9dda.
While on it, this refactors the code so as code paths that need to know
about the ".gz" suffix are down from four to one in walmethods.c, easing
a bit the introduction of new compression methods. This addresses a
second issue where log messages generated for an unexpected failure
would not show the compressed segment name involved, which was
confusing, printing instead the name of the non-compressed equivalent.
Reported-by: Georgios Kokolatos
Discussion: https://postgr.es/m/YPDLz2x3o1aX2wRh@paquier.xyz
Backpatch-through: 10
Commit 3499df0d added a comment that incorrectly suggested that
--force-index-cleanup did not appear in the same major version as the
similar --no-index-cleanup option. In fact, both options are new to
PostgreSQL 14.
Backpatch: 14-, where both options were introduced.
Macs don't understand O_DIRECT, but they can disable caching with a
separate fcntl() call. Extend the file opening functions in fd.c to
handle this for us if the caller passes in PG_O_DIRECT.
For now, this affects only WAL data and even then only if you set:
max_wal_senders=0
wal_level=minimal
This is not expected to be very useful on its own, but later proposed
patches will make greater use of direct I/O, and it'll be useful for
testing if developers on Macs can see the effects.
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CA%2BhUKG%2BADiyyHe0cun2wfT%2BSVnFVqNYPxoO6J9zcZkVO7%2BNGig%40mail.gmail.com
pg_dump failed to preserve the 'enabled' flag (which can be not only
disabled, but also REPLICA or ALWAYS) for partitions which had it
changed from their respective parents. Attempt to handle that by
including a definition for such triggers in the dump, but replace the
standard CREATE TRIGGER line with an ALTER TRIGGER line.
Backpatch to 11, where these triggers can exist. In branches 11 and 12,
pick up a few test lines from commit b9b408c487 to verify that
pg_upgrade is okay with these arrangements.
Co-authored-by: Justin Pryzby <pryzby@telsasoft.com>
Co-authored-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/20200930223450.GA14848@telsasoft.com
As reported by buildfarm member bowerbird, those tests are unstable on
Windows. The failure produced there points to a problem with gzflush(),
that fails to sync a file freshly-opened, with a gzFile properly
opened. While testing this myself with MSVC, I bumped into a different
error where a file could simply not be opened, so this makes me rather
doubtful that testing this area on Windows is a good idea if this
finishes with random concurrency failures. This requires more
investigation, and keeping this buildfarm member red is not a good thing
in the long-term, so for now this just disables this set of tests on
Windows.
Discussion: https://postgr.es/m/YPDLz2x3o1aX2wRh@paquier.xyz
As of v14, pg_depend contains almost 7000 "pin" entries recording
the OIDs of built-in objects. This is a fair amount of bloat for
every database, and it adds time to pg_depend lookups as well as
initdb. We can get rid of all of those entries in favor of an OID
range check, i.e. "OIDs below FirstUnpinnedObjectId are pinned".
(template1 and the public schema are exceptions. Those exceptions
are now wired into IsPinnedObject() instead of initdb's code for
filling pg_depend, but it's the same amount of cruft either way.)
The contents of pg_shdepend are modified likewise.
Discussion: https://postgr.es/m/3737988.1618451008@sss.pgh.pa.us
The OpenBSD implementation of gzip considers only files suffixed by "Z",
"gz", "z", "tgz" or "taz" as valid targets, discarding anything else and
making a command using --test exit with an error code of 512 if anything
invalid is found. The test introduced in ffc9dda tested a WAL segment
suffixed as .gz.partial, enough to make the test fail.
Testing only a full segment is fine enough in terms of coverage, so
simplify the code by discarding the .gz.partial segment in this check.
This should be enough to make the test pass with OpenBSD environments.
Per report from curculio.
Discussion: https://postgr.es/m/YPAdf9r5aJbDoHoq@paquier.xyz
There is a non-trivial amount of code that handles ZLIB compression in
pg_receivewal, from basics like the format name, the calculation of the
start streaming position and of course the compression itself, but there
was no automated coverage for it.
This commit introduces a set of conditional tests (if the build supports
ZLIB) to cover the creation of ZLIB-compressed WAL segments, the
handling of the partial, compressed, WAL segments and the compression
operation in itself. Note that there is an extra phase checking the
validity of the generated files by using directly a gzip command, passed
down by the Makefile of pg_receivewal. This part is skipped if the
command cannot be found, something likely going to happen on Windows
with MSVC except if one sets the variable GZIP_PROGRAM in the
environment of the test.
This set of tests will become handy for upcoming patches that add more
options for the compression methods used by pg_receivewal, like LZ4, to
make sure that no existing facilities are broken.
Author: Georgios Kokolatos
Reviewed-by: Gilles Darold, Michael Paquier
Discussion: https://postgr.es/m/07BK3Mk5aEOsTwGaY77qBVyf9GjoEzn8TMgHLyPGfEFPIpTEmoQuP2P4c7teesjSg-LPeUafsp1flnPeQYINMSMB_UpggJDoduB5EDYBqaQ=@protonmail.com
Build farm animals running ancient HPUX and Solaris have a non-standard
sigwait() from draft versions of POSIX, so they didn't like commit
7c09d279. To avoid the problem in general, only try to use sigwait() if
it's declared by <signal.h> and matches the expected declaration. To
select the modern declaration on Solaris (even in non-threaded
programs), move -D_POSIX_PTHREAD_SEMANTICS into the right place to
affect all translation units.
Also fix the error checking. Modern sigwait() doesn't set errno.
Thanks to Tom Lane for help with this.
Discussion: https://postgr.es/m/3187588.1626136248%40sss.pgh.pa.us
Previously, we would send each line as a separate CopyData message.
That's pretty wasteful if the table is narrow, as each CopyData message
has 5 bytes of overhead. For efficiency, buffer up and pack 8 kB of
input data into each CopyData message.
The server also sends each line as a separate CopyData message in COPY TO
STDOUT, and that's similarly wasteful. But that's documented in the FE/BE
protocol description, so changing that would be a wire protocol break.
Reviewed-by: Aleksander Alekseev
Discussion: https://www.postgresql.org/message-id/40b2cec0-d0fb-3191-2ae1-9a3fe16a7e48%40iki.fi
To add support for streaming transactions at prepare time into the
built-in logical replication, we need to do the following things:
* Modify the output plugin (pgoutput) to implement the new two-phase API
callbacks, by leveraging the extended replication protocol.
* Modify the replication apply worker, to properly handle two-phase
transactions by replaying them on prepare.
* Add a new SUBSCRIPTION option "two_phase" to allow users to enable
two-phase transactions. We enable the two_phase once the initial data sync
is over.
We however must explicitly disable replication of two-phase transactions
during replication slot creation, even if the plugin supports it. We
don't need to replicate the changes accumulated during this phase,
and moreover, we don't have a replication connection open so we don't know
where to send the data anyway.
The streaming option is not allowed with this new two_phase option. This
can be done as a separate patch.
We don't allow to toggle two_phase option of a subscription because it can
lead to an inconsistent replica. For the same reason, we don't allow to
refresh the publication once the two_phase is enabled for a subscription
unless copy_data option is false.
Author: Peter Smith, Ajin Cherian and Amit Kapila based on previous work by Nikhil Sontakke and Stas Kelvich
Reviewed-by: Amit Kapila, Sawada Masahiko, Vignesh C, Dilip Kumar, Takamichi Osumi, Greg Nancarrow
Tested-By: Haiying Tang
Discussion: https://postgr.es/m/02DA5F5E-CECE-4D9C-8B4B-418077E2C010@postgrespro.ru
Discussion: https://postgr.es/m/CAA4eK1+opiV4aFTmWWUF9h_32=HfPOW9vZASHarT0UA5oBrtGw@mail.gmail.com
Allow a pager to be used by the \watch command. This works but isn't
very useful with traditional pagers like "less", so use a different
environment variable. The popular open source tool "pspg" (also by
Pavel) knows how to display the output if you set PSQL_WATCH_PAGER="pspg
--stream".
To make \watch react quickly when the user quits the pager or presses
^C, and also to increase the accuracy of its timing and decrease the
rate of useless context switches, change the main loop of the \watch
command to use sigwait() rather than a sleeping/polling loop, on Unix.
Supported on Unix only for now (like pspg).
Author: Pavel Stehule <pavel.stehule@gmail.com>
Author: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://postgr.es/m/CAFj8pRBfzUUPz-3gN5oAzto9SDuRSq-TQPfXU_P6h0L7hO%2BEhg%40mail.gmail.com
Commit 547f04e changed pgbench to use microsecond accounting, but
introduced a couple of logging and aggregation bugs:
1. We print Unix epoch timestamps so that you can correlate them with
other logs, but these were inadvertently changed to use a
system-dependent reference epoch. Compute Unix timestamps, and begin
aggregation intervals on the boundaries of whole Unix epoch seconds, as
before.
2. The user-supplied aggregation interval needed to be scaled.
Back-patch to 14.
Author: Fabien COELHO <coelho@cri.ensmp.fr>
Author: Yugo NAGATA <nagata@sraoss.co.jp>
Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reported-by: YoungHwan Joo <rulyox@gmail.com>
Reported-by: Gregory Smith <gregsmithpgsql@gmail.com>
Discussion: https://postgr.es/m/CAF7igB1r6wRfSCEAB-iZBKxkowWY6%2BdFF2jObSdd9%2BiVK%2BvHJg%40mail.gmail.com
Discussion: https://postgr.es/m/CAHLJuCW_8Vpcr0%3Dt6O_gozrg3wXXWXZXDioYJd3NhvKriqgpfQ%40mail.gmail.com
This commit removes a dependency to the central logging facilities in
the JSON parsing routines of src/common/, which existed to log errors
when seeing error codes that do not match any existing values in
JsonParseErrorType, which is not something that should never happen.
The routine providing a detailed error message based on the error code
is made backend-only, the existing code being unsafe to use in the
frontend as the error message may finish by being palloc'd or point to a
static string, so there is no way to know if the memory of the message
should be pfree'd or not. The only user of this routine in the frontend
was pg_verifybackup, that is changed to use a more generic error message
on parsing failure.
Note that making this code more resilient to OOM failures if used in
shared libraries would require much more work as a lot of code paths
still rely on palloc() & friends, but we are not sure yet if we need to
go down to that. Still, removing the dependency to logging is a step
toward more portability.
This cleans up the handling of check_stack_depth() while on it, as it
exists only in the backend.
Per discussion with Jacob Champion and Tom Lane.
Discussion: https://postgr.es/m/YNwL7kXwn3Cckbd6@paquier.xyz
Commit 4656e3d66 replaced the "#define CLOBBER_CACHE_ALWAYS"
testing mechanism with a GUC, which has been a great help for
doing cache-clobber testing in more efficient ways; but there
is a gap in the implementation. The only way to do cache-clobber
testing during an initdb run is to use the old method with #define,
because one can't set the GUC from outside. Improve this by
adding a switch to initdb for the purpose.
(Perhaps someday we should let initdb pass through arbitrary
"-c NAME=VALUE" switches. Quoting difficulties dissuaded me
from attempting that right now, though.)
Back-patch to v14 where 4656e3d66 came in.
Discussion: https://postgr.es/m/1582507.1624227029@sss.pgh.pa.us
Extend the replication command CREATE_REPLICATION_SLOT to support the
TWO_PHASE option. This will allow decoding commands like PREPARE
TRANSACTION, COMMIT PREPARED and ROLLBACK PREPARED for slots created with
this option. The decoding of the transaction happens at prepare command.
This patch also adds support of two-phase in pg_recvlogical via a new
option --two-phase.
This option will also be used by future patches that allow streaming of
transactions at prepare time for built-in logical replication. With this,
the out-of-core logical replication solutions can enable replication of
two-phase transactions via replication protocol.
Author: Ajin Cherian
Reviewed-By: Jeff Davis, Vignesh C, Amit Kapila
Discussion:
https://postgr.es/m/02DA5F5E-CECE-4D9C-8B4B-418077E2C010@postgrespro.ruhttps://postgr.es/m/64b9f783c6e125f18f88fbc0c0234e34e71d8639.camel@j-davis.com
This commit prevents pg_checksums to do a rewrite of a block if it has
no need to, in the case where the computed checksum matches with what's
already stored in the block read. This is helpful to accelerate
successive runs of the tool when the previous ones got interrupted, for
example.
The number of blocks and files written is tracked and reported by the
tool once finished. Note that the final flush of the data folder
happens even if no blocks are written, as it could be possible that a
previous interrupted run got stopped while doing a flush.
Author: Greg Sabino Mullane
Reviewed-by: Paquier Michael, Julien Rouhaud
Discussion: https://postgr.es/m/CAKAnmmL+k6goxmVzQJB+0bAR0PN1sgo6GDUXJhyhUmVMze1QAw@mail.gmail.com
The logic is implemented so as there can be a choice in the compression
used when building a WAL record, and an extra per-record bit is used to
track down if a block is compressed with PGLZ, LZ4 or nothing.
wal_compression, the existing parameter, is changed to an enum with
support for the following backward-compatible values:
- "off", the default, to not use compression.
- "pglz" or "on", to compress FPWs with PGLZ.
- "lz4", the new mode, to compress FPWs with LZ4.
Benchmarking has showed that LZ4 outclasses easily PGLZ. ZSTD would be
also an interesting choice, but going just with LZ4 for now makes the
patch minimalistic as toast compression is already able to use LZ4, so
there is no need to worry about any build-related needs for this
implementation.
Author: Andrey Borodin, Justin Pryzby
Reviewed-by: Dilip Kumar, Michael Paquier
Discussion: https://postgr.es/m/3037310D-ECB7-4BF1-AF20-01C10BB33A33@yandex-team.ru
As we do for other attributes of the public schema, omit the COMMENT
command when its payload would match what initdb had installed. For
dumps that do carry this new COMMENT command, non-superusers restoring
them are likely to get an error.
Reviewed by Asif Rehman.
Discussion: https://postgr.es/m/ab48a34c-60f6-e388-502a-3e5fe46a2dae@postgresfriends.org
Instead use the common/int.h functions to check for integer overflow
in a more C-standard-compliant fashion. This is motivated by recent
failures on buildfarm member moonjelly, where it appears that
development-tip gcc is optimizing without regard to the -fwrapv
switch. Presumably that's a gcc bug that will be fixed soon, but
we might as well install cleaner coding here rather than wait.
(This does not address the question of whether we'll ever be able
to get rid of using -fwrapv. Testing shows that this spot is the
only place where doing so creates visible regression test failures,
but unfortunately that proves very little.)
Back-patch to v12. The common/int.h functions exist in v11, but
that branch doesn't use them in any client-side code. I judge
that this case isn't interesting enough in the real world to take
even a small risk of issues from being the first such use.
Tom Lane and Fabien Coelho
Discussion: https://postgr.es/m/73927.1624815543@sss.pgh.pa.us
002_pgbench_no_server was printing some array pointers instead of the
actual contents of those arrays for the expected outputs of stdout and
stderr for a tested command. This does not add any new information that
can help with debugging as the test names allow to track failure
locations, if any.
This commit simply removes those logs as the rest of the printed
information is redundant with command_checks_all().
Per discussion with Andrew Dunstan and Álvaro Herrera.
Discussion: https://postgr.es/m/YNXNFaG7IgkzZanD@paquier.xyz
Backpatch-through: 11
This fixes a couple of problems within the so-said code of this commit
subject:
- Replace the use of open() with slurp_file(), fixing an issue reported
by buildfarm member fairywren whose perl installation keep around CRLF
characters, causing the matching patterns for the logs to fail.
- Remove the eval block, which is not really necessary.
This set of issues has come into light after fixing a different issue
with c13585fe, and this is wrong since this code has been introduced.
Reported-by: Andrew Dunstan, and buildfarm member fairywren
Author: Michael Paquier
Reviewed-by: Andrew Dunstan
Discussion: https://postgr.es/m/0f49303e-7784-b3ee-200b-cbf67be2eb9e@dunslane.net
Backpatch-through: 11
ALTER SUBSCRIPTION DROP PUBLICATION does not actually support
copy_data option, so remove it from tab completion.
Also, reword the error message that is thrown when all the
publications from a subscription are specified to be dropped.
Also, made few doc and cosmetic adjustments.
Author: Vignesh C <vignesh21@gmail.com>
Reviewed-by: Bharath Rupireddy <bharath.rupireddy@enterprisedb.com>
Reviewed-by: Japin Li <japinli@hotmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CALDaNm21RwsDzs4xj14ApteAF7auyyomHNnp+NEL-sH8m-jMvQ@mail.gmail.com
fairywren is not happy with the pattern checks introduced by c13585f.
I am not sure if this outlines a bug in pgbench or if the regex patterns
used in the tests are too restrictive for this buildfarm member's
environment. This adds more debugging information to show the log
entries that do not match with the expected pattern, to help in finding
out what's happening. That seems like a good addition in the long-term
anyway as that may not be the only issue in this area.
Discussion: https://postgr.es/m/YNUad2HvgW+6eXyo@paquier.xyz
The logic checking for the format of per-thread logs used grep() with
directly "$re", which would cause the test to consider all the logs as
a match without caring about their format at all. Using "/$re/" makes
grep() perform a regex test, which is what we want here.
While on it, improve some of the tests to be more picky with the
patterns expected and add more comments to describe the tests.
Issue discovered while digging into a separate patch.
Author: Fabien Coelho, Michael Paquier
Discussion: https://postgr.es/m/YNPsPAUoVDCpPOGk@paquier.xyz
Backpatch-through: 11
Generalize the INDEX_CLEANUP VACUUM parameter (and the corresponding
reloption): make it into a ternary style boolean parameter. It now
exposes a third option, "auto". The "auto" option (which is now the
default) enables the "bypass index vacuuming" optimization added by
commit 1e55e7d1.
"VACUUM (INDEX_CLEANUP TRUE)" is redefined to once again make VACUUM
simply do any required index vacuuming, regardless of how few dead
tuples are encountered during the first scan of the target heap relation
(unless there are exactly zero). This gives users a way of opting out
of the "bypass index vacuuming" optimization, if for whatever reason
that proves necessary. It is also expected to be used by PostgreSQL
developers as a testing option from time to time.
"VACUUM (INDEX_CLEANUP FALSE)" does the same thing as it always has: it
forcibly disables both index vacuuming and index cleanup. It's not
expected to be used much in PostgreSQL 14. The failsafe mechanism added
by commit 1e55e7d1 addresses the same problem in a simpler way.
INDEX_CLEANUP can now be thought of as a testing and compatibility
option.
Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: Masahiko Sawada <sawada.mshk@gmail.com>
Reviewed-By: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/CAH2-WznrBoCST4_Gxh_G9hA8NzGUbeBGnOUC8FcXcrhqsv6OHQ@mail.gmail.com
Commit 547f04e73 caused pgbench to start printing its version number,
which seems like a fine idea, but it needs a bit more work:
* Print the server version number too, when different.
* Print the PG_VERSION string, not some reconstructed approximation.
This patch copies psql's well-tested code for the same purpose.
Discussion: https://postgr.es/m/1226654.1624036821@sss.pgh.pa.us
Recent glibc versions have made mktime() fail if tm_isdst is
inconsistent with the prevailing timezone; in particular it fails for
tm_isdst = 1 when the zone is UTC. (This seems wildly inconsistent
with the POSIX-mandated treatment of "incorrect" values for the other
fields of struct tm, so if you ask me it's a bug, but I bet they'll
say it's intentional.) This has been observed to cause cosmetic
problems when pg_restore'ing an archive created in a different
timezone.
To fix, do mktime() using the field values from the archive, and if
that fails try again with tm_isdst = -1. This will give a result
that's off by the UTC-offset difference from the original zone, but
that was true before, too. It's not terribly critical since we don't
do anything with the result except possibly print it. (Someday we
should flush this entire bit of logic and record a standard-format
timestamp in the archive instead. That's not okay for a back-patched
bug fix, though.)
Also, guard our only other use of mktime() by having initdb's
build_time_t() set tm_isdst = -1 not 0. This case could only have
an issue in zones that are DST year-round; but I think some do exist,
or could in future.
Per report from Wells Oliver. Back-patch to all supported
versions, since any of them might need to run with a newer glibc.
Discussion: https://postgr.es/m/CAOC+FBWDhDHO7G-i1_n_hjRzCnUeFO+H-Czi1y10mFhRWpBrew@mail.gmail.com
We've accumulated quite a mix of instances of "an SQL" and "a SQL" in the
documents. It would be good to be a bit more consistent with these.
The most recent version of the SQL standard I looked at seems to prefer
"an SQL". That seems like a good lead to follow, so here we change all
instances of "a SQL" to become "an SQL". Most instances correctly use
"an SQL" already, so it also makes sense to use the dominant variation in
order to minimise churn.
Additionally, there were some other abbreviations that needed to be
adjusted. FSM, SSPI, SRF and a few others. Also fix some pronounceable,
abbreviations to use "a" instead of "an". For example, "a SASL" instead
of "an SASL".
Here I've only adjusted the documents and error messages. Many others
still exist in source code comments. Translator hint comments seem to be
the biggest culprit. It currently does not seem worth the churn to change
these.
Discussion: https://postgr.es/m/CAApHDvpML27UqFXnrYO1MJddsKVMQoiZisPvsAGhKE_tsKXquw%40mail.gmail.com
Commit 2453ea142 redefined pg_proc.proargtypes to include the types of
OUT parameters, for procedures only. While that had some advantages
for implementing the SQL-spec behavior of DROP PROCEDURE, it was pretty
disastrous from a number of other perspectives. Notably, since the
primary key of pg_proc is name + proargtypes, this made it possible to
have multiple procedures with identical names + input arguments and
differing output argument types. That would make it impossible to call
any one of the procedures by writing just NULL (or "?", or any other
data-type-free notation) for the output argument(s). The change also
seems likely to cause grave confusion for client applications that
examine pg_proc and expect the traditional definition of proargtypes.
Hence, revert the definition of proargtypes to what it was, and
undo a number of complications that had been added to support that.
To support the SQL-spec behavior of DROP PROCEDURE, when there are
no argmode markers in the command's parameter list, we perform the
lookup both ways (that is, matching against both proargtypes and
proallargtypes), succeeding if we get just one unique match.
In principle this could result in ambiguous-function failures
that would not happen when using only one of the two rules.
However, overloading of procedure names is thought to be a pretty
rare usage, so this shouldn't cause many problems in practice.
Postgres-specific code such as pg_dump can defend against any
possibility of such failures by being careful to specify argmodes
for all procedure arguments.
This also fixes a few other bugs in the area of CALL statements
with named parameters, and improves the documentation a little.
catversion bump forced because the representation of procedures
with OUT arguments changes.
Discussion: https://postgr.es/m/3742981.1621533210@sss.pgh.pa.us
The set of subcommands supported by \dAp, \do and \dy was described
incorrectly in psql's --help. The documentation was already consistent
with the code.
Reported-by: inoas, from IRC
Author: Matthijs van der Vleuten
Reviewed-by: Neil Chen
Discussion: https://postgr.es/m/6a984e24-2171-4039-9050-92d55e7b23fe@www.fastmail.com
Backpatch-through: 9.6
An incorrectly-encoded multibyte character near the end of a string
could cause various processing loops to run past the string's
terminating NUL, with results ranging from no detectable issue to
a program crash, depending on what happens to be in the following
memory.
This isn't an issue in the server, because we take care to verify
the encoding of strings before doing any interesting processing
on them. However, that lack of care leaked into client-side code
which shouldn't assume that anyone has validated the encoding of
its input.
Although this is certainly a bug worth fixing, the PG security team
elected not to regard it as a security issue, primarily because
any untrusted text should be sanitized by PQescapeLiteral or
the like before being incorporated into a SQL or psql command.
(If an app fails to do so, the same technique can be used to
cause SQL injection, with probably much more dire consequences
than a mere client-program crash.) Those functions were already
made proof against this class of problem, cf CVE-2006-2313.
To fix, invent PQmblenBounded() which is like PQmblen() except it
won't return more than the number of bytes remaining in the string.
In HEAD we can make this a new libpq function, as PQmblen() is.
It seems imprudent to change libpq's API in stable branches though,
so in the back branches define PQmblenBounded as a macro in the files
that need it. (Note that just changing PQmblen's behavior would not
be a good idea; notably, it would completely break the escaping
functions' defense against this exact problem. So we just want a
version for those callers that don't have any better way of handling
this issue.)
Per private report from houjingyi. Back-patch to all supported branches.
The previous arrangement was just one big list, and the internal order
was not all consistent either. Now arrange the options by group and
sort them, the way it's already done in the --help output and one
other reference pages. Also fix some ordering in the --help output.
Fix a few places that were using appendStringInfo() when they should have
been using appendStringInfoString(). Also some cases of
appendPQExpBuffer() that would have been better suited to use
appendPQExpBufferChar(), and finally, some places that used
appendPQExpBuffer() when appendPQExpBufferStr() would have suited better.
There are no bugs are being fixed here. The aim is just to make the code
use the most optimal function for the job.
All the code being changed here is new to PG14. It makes sense to fix
these before we branch for PG15. There are a few other places that we
could fix, but those cases are older code so fixing those seems less
worthwhile as it may cause unnecessary back-patching pain in the future.
Author: Hou Zhijie
Discussion: https://postgr.es/m/OS0PR01MB5716732158B1C4142C6FE375943D9@OS0PR01MB5716.jpnprd01.prod.outlook.com
Redefine '\0' (InvalidCompressionMethod) as meaning "if we need to
compress, use the current setting of default_toast_compression".
This allows '\0' to be a suitable default choice regardless of
datatype, greatly simplifying code paths that initialize tupledescs
and the like. It seems like a more user-friendly approach as well,
because now the default compression choice doesn't migrate into table
definitions, meaning that changing default_toast_compression is
usually sufficient to flip an installation's behavior; one needn't
tediously issue per-column ALTER SET COMPRESSION commands.
Along the way, fix a few minor bugs and documentation issues
with the per-column-compression feature. Adopt more robust
APIs for SetIndexStorageProperties and GetAttributeCompression.
Bump catversion because typical contents of attcompression will now
be different. We could get away without doing that, but it seems
better to ensure v14 installations all agree on this. (We already
forced initdb for beta2, anyway.)
Discussion: https://postgr.es/m/626613.1621787110@sss.pgh.pa.us
The error messages, docs, and one of the options were using
'parallel degree' to indicate parallelism used by vacuum command. We
normally use 'parallel workers' at other places so change it for parallel
vacuum accordingly.
Author: Bharath Rupireddy
Reviewed-by: Dilip Kumar, Amit Kapila
Backpatch-through: 13
Discussion: https://postgr.es/m/CALj2ACWz=PYrrFXVsEKb9J1aiX4raA+UBe02hdRp_zqDkrWUiw@mail.gmail.com
"typename" is a C++ keyword, so pg_upgrade.h fails to compile in C++.
Fortunately, there seems no likely reason for somebody to need to
do that. Nonetheless, it's project policy that all .h files should
pass cpluspluscheck, so rename the argument to fix that.
Oversight in 57c081de0; back-patch as that was. (The policy requiring
pg_upgrade.h to pass cpluspluscheck only goes back to v12, but it
seems best to keep this code looking the same in all branches.)
One of the tests for the pgbench permute() function added by
6b258e3d68 fails on some 32-bit platforms, due to variations in the
floating point computations in getrand(). The remaining tests give
sufficient coverage, so just remove the failing test.
Reported by Christoph Berg. Analysis by Thomas Munro and Tom Lane.
Based on patch by Fabien Coelho.
Discussion: https://postgr.es/m/YKQnUoYV63GRJBDD@msg.df7cb.de
This is an oversight from bbe0a81d, where the equivalent option exists
in pg_dump. This is useful to be able to reset the compression methods
cluster-wide when restoring the data based on default_toast_compression.
Reviewed-by: Daniel Gustafsson, Tom Lane
Discussion: https://postgr.es/m/YKHC+qCJvzCRVCpY@paquier.xyz
When specified directly as DML queries, INSERT was not getting always
completed to "INSERT INTO", same for DELETE with "DELETE FROM". This
makes the completion behavior more consistent for both commands, saving
a few keystrokes.
Commands on policies, triggers, grant/revoke, etc. require only DELETE
as completion keyword.
Author: Haiying Tang
Reviewed-by: Dilip Kumar, Julien Rouhaud
Discussion: https://postgr.es/m/OS0PR01MB61135AE2B07CCD1AB8C6A0F6FB549@OS0PR01MB6113.jpnprd01.prod.outlook.com
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.
This set of commits has some bugs with known fixes, but at this late
stage in the release cycle it seems best to revert and resubmit next
time, along with some new automated test coverage for this whole area.
Commits reverted:
dc88460c: Doc: Review for "Optionally prefetch referenced data in recovery."
1d257577: Optionally prefetch referenced data in recovery.
f003d9f8: Add circular WAL decoding buffer.
323cbe7c: Remove read_page callback from XLogReader.
Remove the new GUC group WAL_RECOVERY recently added by a55a9847, as the
corresponding section of config.sgml is now reverted.
Discussion: https://postgr.es/m/CAOuzzgrn7iKnFRsB4MHp3UisEQAGgZMbk_ViTN4HV4-Ksq8zCg%40mail.gmail.com
Some relations with LZ4 used as the toast compression methods have been
left around in the main regression test suite to stress pg_upgrade, but
pg_dump, that includes tests much more picky in terms of output
generated, had no actual coverage with this feature.
Similarly to collations, tests only working with LZ4 are tracked with an
additional flag, and this uses TestLib::check_pg_config() to check if
the build supports LZ4 or not. This stresses more scenarios with
tables, materialized views and pg_dump --no-toast-compression.
Author: Dilip Kumar
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/CAFiTN-twgPmohG7qj1HXhySq16h123y5OowsQR+5h1YeZE9fmQ@mail.gmail.com
Design problems were discovered in the handling of composite types and
record types that would cause some relevant versions not to be recorded.
Misgivings were also expressed about the use of the pg_depend catalog
for this purpose. We're out of time for this release so we'll revert
and try again.
Commits reverted:
1bf946bd: Doc: Document known problem with Windows collation versions.
cf002008: Remove no-longer-relevant test case.
ef387bed: Fix bogus collation-version-recording logic.
0fb0a050: Hide internal error for pg_collation_actual_version(<bad OID>).
ff942057: Suppress "warning: variable 'collcollate' set but not used".
d50e3b1f: Fix assertion in collation version lookup.
f24b1569: Rethink extraction of collation dependencies.
257836a7: Track collation versions for indexes.
cd6f479e: Add pg_depend.refobjversion.
7d1297df: Remove pg_collation.collversion.
Discussion: https://postgr.es/m/CA%2BhUKGLhj5t1fcjqAu8iD9B3ixJtsTNqyCCD4V0aTO9kAKAjjA%40mail.gmail.com
Don't advocate dropping a whole table when dropping a column would
serve. While at it, try to make the layout of these messages a
bit cleaner and more consistent.
Per suggestion from Daniel Gustafsson. No back-patch, as we generally
don't like to churn translatable messages in released branches.
Discussion: https://postgr.es/m/2798740.1619622555@sss.pgh.pa.us
Commits 29aeda6e4 et al closed up some oversights involving not checking
for non-upgradable types within container types, such as arrays and
ranges. However, I only looked at version.c, failing to notice that
there were substantially-equivalent tests in check.c. (The division
of responsibility between those files is less than clear...)
In addition, because genbki.pl does not guarantee that auto-generated
rowtype OIDs will hold still across versions, we need to consider that
the composite type associated with a system catalog or view is
non-upgradable. It seems unlikely that someone would have a user
column declared that way, but if they did, trying to read it in another
PG version would likely draw "no such pg_type OID" failures, thanks
to the type OID embedded in composite Datums.
To support the composite and reg*-type cases, extend the recursive
query that does the search to allow any base query that returns
a column of pg_type OIDs, rather than limiting it to exactly one
starting type.
As before, back-patch to all supported branches.
Discussion: https://postgr.es/m/2798740.1619622555@sss.pgh.pa.us
The verification of permissions doesn't succeed on Cygwin, because the
required feature is not implemented for Cygwin at the moment. So skip
this part of the test, like MinGW already does.
This will install amcheck in the database if not present. The default
schema is for the extension is pg_catalog, but this can be overridden by
providing a value for the option.
Mark Dilger, slightly editorialized by me.
(rather divergent)
Discussion: https://postgr.es/m/bdc0f7c2-09e3-ee57-8471-569dfb509234@dunslane.net
Instead, put them in via a format placeholder. This reduces the
number of distinct translatable messages and also reduces the chances
of typos during translation. We already did this for the system call
arguments in a number of cases, so this is just the same thing taken a
bit further.
Discussion: https://www.postgresql.org/message-id/flat/92d6f545-5102-65d8-3c87-489f71ea0a37%40enterprisedb.com
A couple of tests have been using 0 as magic constant while SEEK_SET can
be used instead. This makes the code easier to understand, and more
consistent with the changes done in 3c5b068.
Per discussion with Andrew Dunstan.
Discussion: https://postgr.es/m/YHrc24AgJQ6tQ1q0@paquier.xyz
Invent system_functions.sql to carry the function definitions that
were formerly in system_views.sql. The function definitions were
already a quarter of the file and are about to be more, so it seems
appropriate to give them their own home.
In passing, fix an oversight in dfb75e478: it neglected to call
check_input() for system_constraints.sql.
Discussion: https://postgr.es/m/3956760.1618529139@sss.pgh.pa.us
Commit e717a9a18 changed the longstanding rule that prosrc is NOT NULL
because when a SQL-language function is written in SQL-standard style,
we don't currently have anything useful to put there. This seems a poor
decision though, as it could easily have negative impacts on external
PLs (opening them to crashes they didn't use to have, for instance).
SQL-function-related code can just as easily test "is prosqlbody not
null" as "is prosrc null", so there's no real gain there either.
Hence, revert the NOT NULL marking removal and adjust related logic.
For now, we just put an empty string into prosrc for SQL-standard
functions. Maybe we'll have a better idea later, although the
history of things like pg_attrdef.adsrc suggests that it's not
easy to maintain a string equivalent of a node tree.
This also adds an assertion that queryDesc->sourceText != NULL
to standard_ExecutorStart. We'd been silently relying on that
for awhile, so let's make it less silent.
Also fix some overlooked documentation and test cases.
Discussion: https://postgr.es/m/2197698.1617984583@sss.pgh.pa.us
6568cef, that introduced the option, had an inconsistent behavior when
it comes to configuration tables set up by pg_extension_config_dump, as
the data of all configuration tables would included in a dump even for
extensions not listed by a set of --extension switches.
The contents dumped changed depending on the schema where an extension
was installed when an extension was not listed. For example, an
extension installed under the public schema would have its configuration
data not dumped even when not listed with --extension, which was
inconsistent with the case of an extension installed on a non-public
schema, where the configuration would be dumped.
Per discussion with Noah, we have settled down to the simple rule of
dumping configuration data of an extension if it is listed in
--extension (default is unchanged and backward-compatible, to dump
everything on sight if there are no extensions directly listed). This
avoids some weird cases where the dumps depended on a --schema for one.
More tests are added to cover the gap, where we cross-check more
behaviors depending on --schema when an extension is not listed.
Reported-by: Noah Misch
Reviewed-by: Noah Misch
Discussion: https://postgr.es/m/20210404220802.GA728316@rfd.leadboat.com
We don't need to mention the attribute number in these messages, because
there's a dedicated column for that, but we should mention the toast
value ID, because that's really useful for any follow-up troubleshooting
the user wants to do. This also rewords some of the messages to hopefully
read a little better.
Also, use VARATT_EXTERNAL_GET_POINTER in case we're accessing a TOAST
pointer that isn't aligned on a platform that's fussy about alignment,
so that we don't crash while corruption-checking the user's data.
Mark Dilger, reviewed by me.
Discussion: http://postgr.es/m/7D3B9BF6-50D0-4C30-8506-1C1851C7F96F@enterprisedb.com
Commit 8ff1c94649 extended TRUNCATE command so that it can also truncate
foreign tables. But it forgot to support tab-complete for TRUNCATE on
foreign tables. That is, previously tab-complete for TRUNCATE displayed
only the names of regular tables.
This commit improves tab-complete for TRUNCATE so that it displays also
the names of foreign tables.
Author: Fujii Masao
Reviewed-by: Bharath Rupireddy
Discussion: https://postgr.es/m/551ed8c1-f531-818b-664a-2cecdab99cd8@oss.nttdata.com
Comment fixes are applied on HEAD, and documentation improvements are
applied on back-branches where needed.
Author: Justin Pryzby
Discussion: https://postgr.es/m/20210408164008.GJ6592@telsasoft.com
Backpatch-through: 9.6
Teach xlogreader.c to decode its output into a circular buffer, to
support optimizations based on looking ahead.
* XLogReadRecord() works as before, consuming records one by one, and
allowing them to be examined via the traditional XLogRecGetXXX()
macros.
* An alternative new interface XLogNextRecord() is added that returns
pointers to DecodedXLogRecord structs that can be examined directly.
* XLogReadAhead() provides a second cursor that lets you see
further ahead, as long as data is available and there is enough space
in the decoding buffer. This returns DecodedXLogRecord pointers to the
caller, but also adds them to a queue of records that will later be
consumed by XLogNextRecord()/XLogReadRecord().
The buffer's size is controlled with wal_decode_buffer_size. The buffer
could potentially be placed into shared memory, for future projects.
Large records that don't fit in the circular buffer are called
"oversized" and allocated separately with palloc().
Discussion: https://postgr.es/m/CA+hUKGJ4VJN8ttxScUFM8dOKX0BrBiboo5uz1cq=AovOddfHpA@mail.gmail.com
Previously, the XLogReader module would fetch new input data using a
callback function. Redesign the interface so that it tells the caller
to insert more data with a special return value instead. This API suits
later patches for prefetching, encryption and maybe other future
projects that would otherwise require continually extending the callback
interface.
As incidental cleanup work, move global variables readOff, readLen and
readSegNo inside XlogReaderState.
Author: Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>
Author: Heikki Linnakangas <hlinnaka@iki.fi> (parts of earlier version)
Reviewed-by: Antonin Houska <ah@cybertec.at>
Reviewed-by: Alvaro Herrera <alvherre@2ndquadrant.com>
Reviewed-by: Takashi Menjo <takashi.menjo@gmail.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://postgr.es/m/20190418.210257.43726183.horiguchi.kyotaro%40lab.ntt.co.jp
When dealing with overloaded function or operator names, having
to look through a long list of matches is tedious. Let's extend
these commands to allow specification of (input) argument types
to let such results be trimmed down. Each additional argument
is treated the same as the pattern argument of \dT and matched
against the appropriate argument's type name.
While at it, fix \dT (and these new options) to recognize the
usual notation of "foo[]" for "the array type over foo", and
to handle the special abbreviations allowed by the backend
grammar, such as "int" for "integer".
Greg Sabino Mullane, revised rather significantly by me
Discussion: https://postgr.es/m/CAKAnmmLF9Hhu02N+s7uAyLc5J1xZReg72HQUoiKhNiJV3_jACQ@mail.gmail.com
This adds support for writing CREATE FUNCTION and CREATE PROCEDURE
statements for language SQL with a function body that conforms to the
SQL standard and is portable to other implementations.
Instead of the PostgreSQL-specific AS $$ string literal $$ syntax,
this allows writing out the SQL statements making up the body
unquoted, either as a single statement:
CREATE FUNCTION add(a integer, b integer) RETURNS integer
LANGUAGE SQL
RETURN a + b;
or as a block
CREATE PROCEDURE insert_data(a integer, b integer)
LANGUAGE SQL
BEGIN ATOMIC
INSERT INTO tbl VALUES (a);
INSERT INTO tbl VALUES (b);
END;
The function body is parsed at function definition time and stored as
expression nodes in a new pg_proc column prosqlbody. So at run time,
no further parsing is required.
However, this form does not support polymorphic arguments, because
there is no more parse analysis done at call time.
Dependencies between the function and the objects it uses are fully
tracked.
A new RETURN statement is introduced. This can only be used inside
function bodies. Internally, it is treated much like a SELECT
statement.
psql needs some new intelligence to keep track of function body
boundaries so that it doesn't send off statements when it sees
semicolons that are inside a function body.
Tested-by: Jaime Casanova <jcasanov@systemguards.com.ec>
Reviewed-by: Julien Rouhaud <rjuju123@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/1c11f1eb-f00c-43b7-799d-2d44132c02d7@2ndquadrant.com
Documentation and comments in code and tests have been using the terms
sensitive/insensitive cursor incorrectly relative to the SQL standard.
(Cursor sensitivity is only relevant for changes made in the same
transaction as the cursor, not for concurrent changes in other
sessions.) Moreover, some of the behavior of PostgreSQL is incorrect
according to the SQL standard, confusing the issue further. (WHERE
CURRENT OF changes are not visible in insensitive cursors, but they
should be.)
This change corrects the terminology and removes the claim that
sensitive cursors are supported. It also adds a test case that checks
the insensitive behavior in a "correct" way, using a change command
not using WHERE CURRENT OF. Finally, it adds the ASENSITIVE cursor
option to select the default asensitive behavior, per SQL standard.
There are no changes to cursor behavior in this patch.
Discussion: https://www.postgresql.org/message-id/flat/96ee8b30-9889-9e1b-b053-90e10c050e85%40enterprisedb.com
Previously, psql printed only the last result if a command string
returned multiple result sets. Now it prints all of them. The
previous behavior can be obtained by setting the psql variable
SHOW_ALL_RESULTS to off.
Author: Fabien COELHO <coelho@cri.ensmp.fr>
Reviewed-by: "Iwata, Aya" <iwata.aya@jp.fujitsu.com>
Reviewed-by: Daniel Verite <daniel@manitou-mail.org>
Reviewed-by: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-by: vignesh C <vignesh21@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/alpine.DEB.2.21.1904132231510.8961@lancre
This adds a new function, permute(), that generates pseudorandom
permutations of arbitrary sizes. This can be used to randomly shuffle
a set of values to remove unwanted correlations. For example,
permuting the output from a non-uniform random distribution so that
all the most common values aren't collocated, allowing more realistic
tests to be performed.
Formerly, hash() was recommended for this purpose, but that suffers
from collisions that might alter the distribution, so recommend
permute() for this purpose instead.
Fabien Coelho and Hironobu Suzuki, with additional hacking be me.
Reviewed by Thomas Munro, Alvaro Herrera and Muhammad Usama.
Discussion: https://postgr.es/m/alpine.DEB.2.21.1807280944370.5142@lancre
At present, if we want to update publications in a subscription, we
can use SET PUBLICATION. However, it requires supplying all
publications that exists and the new publications. If we want to add
new publications, it's inconvenient. The new syntax only supplies the
new publications. When the refresh is true, it only refreshes the new
publications.
Author: Japin Li <japinli@hotmail.com>
Author: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/MEYP282MB166939D0D6C480B7FBE7EFFBB6BC0@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM
When editing the previous query buffer, if the editor is exited
without modifying the temp file then clear the query buffer,
rather than re-loading (and probably re-executing) the previous
query buffer. This reduces the probability of accidentally
re-executing something you didn't intend to.
Similarly, in "\e file", if the file isn't actually modified
then don't load it into the query buffer. And in "\ef" and
"\ev", if no changes are made then clear the query buffer
instead of loading the function or view definition into it.
Cases where we fail to invoke the editor at all, or it returns
a nonzero status, are treated like the no-file-modification case.
Laurenz Albe, reviewed by Jacob Champion
Discussion: https://postgr.es/m/0ba3f2a658bac6546d9934ab6ba63a805d46a49b.camel@cybertec.at
pg_checksums uses two counters, total size and current size,
to calculate the progress. Previously the progress that
pg_checksums reported could not reach 100% at the end.
The cause of this issue was that the sizes of only pages excluding
new ones in each file were counted as the current size
while the size of each file is counted as the total size.
That is, the total size of all new pages could be reported
as the difference between the total size and current size.
This commit fixes this issue by making pg_checksums count
the sizes of all pages including new ones in each file as
the current size.
Back-patch to v12 where progress reporting was added to pg_checksums.
Reported-by: Shinya Kato
Author: Shinya Kato
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/TYAPR01MB289656B1ACA0A5E7CAD07BE3C47A9@TYAPR01MB2896.jpnprd01.prod.outlook.com
Despite the clear comments pointing out that the duplicative code
segments in ReadHead() and _discoverArchiveFormat() needed to be
in sync, they were not: the latter did not bother to apply any of
the sanity checks in the former. We'd missed noticing this partly
because none of those checks would fail in scenarios we customarily
test, and partly because the oversight would be masked if both
segments execute, which they would in cases other than needing to
autodetect the format of a non-seekable stdin source. However,
in a case meeting all these requirements --- for example, trying
to read a newer-than-supported archive format from non-seekable
stdin --- pg_restore missed applying the version check and would
likely dump core or otherwise misbehave.
The whole thing is silly anyway, because there seems little reason
to duplicate the logic beyond the one-line verification that the
file starts with "PGDMP". There seems to have been an undocumented
assumption that multiple major formats (major enough to require
separate reader modules) would nonetheless share the first half-dozen
fields of the custom-format header. This seems unlikely, so let's
fix it by just nuking the duplicate logic in _discoverArchiveFormat().
Also get rid of the pointless attempt to seek back to the start of
the file after successful autodetection. That wastes cycles and
it means we have four behaviors to verify not two.
Per bug #16951 from Sergey Koposov. This has been broken for
decades, so back-patch to all supported versions.
Discussion: https://postgr.es/m/16951-a4dd68cf0de23048@postgresql.org
With the 'noError' argument, you can try to convert a buffer without
knowing the character boundaries beforehand. The functions now need to
return the number of input bytes successfully converted.
This is is a backwards-incompatible change, if you have created a custom
encoding conversion with CREATE CONVERSION. This adds a check to
pg_upgrade for that, refusing the upgrade if there are any user-defined
encoding conversions. Custom conversions are very rare, there are no
commonly used extensions that I know of that uses that feature. No other
objects can depend on conversions, so if you do have one, you can fairly
easily drop it before upgrading, and recreate it after the upgrade with
an updated version.
Add regression tests for built-in encoding conversions. This doesn't cover
every conversion, but it covers all the internal functions in conv.c that
are used to implement the conversions.
Reviewed-by: John Naylor
Discussion: https://www.postgresql.org/message-id/e7861509-3960-538a-9025-b75a61188e01%40iki.fi
When specified, only extensions matching the given pattern are included
in dumps. Similarly to --table and --schema, when --strict-names is
used, a perfect match is required. Also, like the two other options,
this new option offers no guarantee that dependent objects have been
dumped, so a restore may fail on a clean database.
Tests are added in test_pg_dump/, checking after a set of positive and
negative cases, with or without an extension's contents added to the
dump generated.
Author: Guillaume Lelarge
Reviewed-by: David Fetter, Tom Lane, Michael Paquier, Asif Rehman,
Julien Rouhaud
Discussion: https://postgr.es/m/CAECtzeXOt4cnMU5+XMZzxBPJ_wu76pNy6HZKPRBL-j7yj1E4+g@mail.gmail.com
As committed in bbe0a81db, pg_dump from a pre-v14 server effectively
acts as though you'd said --no-toast-compression. I think the right
thing is for it to act as though default_toast_compression is set to
"pglz", instead, so that the tables' toast compression behavior is
preserved. You can always get the other behavior, if you want that,
by giving the switch.
Discussion: https://postgr.es/m/1112852.1616609702@sss.pgh.pa.us
Allow defining extended statistics on expressions, not just just on
simple column references. With this commit, expressions are supported
by all existing extended statistics kinds, improving the same types of
estimates. A simple example may look like this:
CREATE TABLE t (a int);
CREATE STATISTICS s ON mod(a,10), mod(a,20) FROM t;
ANALYZE t;
The collected statistics are useful e.g. to estimate queries with those
expressions in WHERE or GROUP BY clauses:
SELECT * FROM t WHERE mod(a,10) = 0 AND mod(a,20) = 0;
SELECT 1 FROM t GROUP BY mod(a,10), mod(a,20);
This introduces new internal statistics kind 'e' (expressions) which is
built automatically when the statistics object definition includes any
expressions. This represents single-expression statistics, as if there
was an expression index (but without the index maintenance overhead).
The statistics is stored in pg_statistics_ext_data as an array of
composite types, which is possible thanks to 79f6a942bd.
CREATE STATISTICS allows building statistics on a single expression, in
which case in which case it's not possible to specify statistics kinds.
A new system view pg_stats_ext_exprs can be used to display expression
statistics, similarly to pg_stats and pg_stats_ext views.
ALTER TABLE ... ALTER COLUMN ... TYPE now treats indexes the same way it
treats indexes, i.e. it drops and recreates the statistics. This means
all statistics are reset, and we no longer try to preserve at least the
functional dependencies. This should not be a major issue in practice,
as the functional dependencies actually rely on per-column statistics,
which were always reset anyway.
Author: Tomas Vondra
Reviewed-by: Justin Pryzby, Dean Rasheed, Zhihong Yu
Discussion: https://postgr.es/m/ad7891d2-e90c-b446-9fe2-7419143847d7%40enterprisedb.com
Membership consists, implicitly, of the current database owner. Expect
use in template databases. Once pg_database_owner has rights within a
template, each owner of a database instantiated from that template will
exercise those rights.
Reviewed by John Naylor.
Discussion: https://postgr.es/m/20201228043148.GA1053024@rfd.leadboat.com
Allow a partition be detached from its partitioned table without
blocking concurrent queries, by running in two transactions and only
requiring ShareUpdateExclusive in the partitioned table.
Because it runs in two transactions, it cannot be used in a transaction
block. This is the main reason to use dedicated syntax: so that users
can choose to use the original mode if they need it. But also, it
doesn't work when a default partition exists (because an exclusive lock
would still need to be obtained on it, in order to change its partition
constraint.)
In case the second transaction is cancelled or a crash occurs, there's
ALTER TABLE .. DETACH PARTITION .. FINALIZE, which executes the final
steps.
The main trick to make this work is the addition of column
pg_inherits.inhdetachpending, initially false; can only be set true in
the first part of this command. Once that is committed, concurrent
transactions that use a PartitionDirectory will include or ignore
partitions so marked: in optimizer they are ignored if the row is marked
committed for the snapshot; in executor they are always included. As a
result, and because of the way PartitionDirectory caches partition
descriptors, queries that were planned before the detach will see the
rows in the detached partition and queries that are planned after the
detach, won't.
A CHECK constraint is created that duplicates the partition constraint.
This is probably not strictly necessary, and some users will prefer to
remove it afterwards, but if the partition is re-attached to a
partitioned table, the constraint needn't be rechecked.
Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: Amit Langote <amitlangote09@gmail.com>
Reviewed-by: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/20200803234854.GA24158@alvherre.pgsql
To allow inserts in parallel-mode this feature has to ensure that all the
constraints, triggers, etc. are parallel-safe for the partition hierarchy
which is costly and we need to find a better way to do that. Additionally,
we could have used existing cached information in some cases like indexes,
domains, etc. to determine the parallel-safety.
List of commits reverted, in reverse chronological order:
ed62d3737c Doc: Update description for parallel insert reloption.
c8f78b6161 Add a new GUC and a reloption to enable inserts in parallel-mode.
c5be48f092 Improve FK trigger parallel-safety check added by 05c8482f7f.
e2cda3c20a Fix use of relcache TriggerDesc field introduced by commit 05c8482f7f.
e4e87a32cc Fix valgrind issue in commit 05c8482f7f.
05c8482f7f Enable parallel SELECT for "INSERT INTO ... SELECT ...".
Discussion: https://postgr.es/m/E1lMiB9-0001c3-SY@gemulon.postgresql.org
Disable autovacuum, because we don't want it to run against
intentionally corrupted tables. Also, before corrupting the tables,
run pg_amcheck and ensure that it passes. Otherwise, if something
unexpected happens when we check the corrupted tables, it's not so
clear whether it would have also happened before we corrupted
them.
Mark Dilger
Discussion: http://postgr.es/m/AA5506CE-7D2A-42E4-A51D-358635E3722D@enterprisedb.com
Jasen Betts reported yet another unintended side effect of commit
85c54287a: reconnecting with "\c service=whatever" did not have the
expected results. The reason is that starting from the output of
PQconndefaults() effectively allows environment variables (such
as PGPORT) to override entries in the service file, whereas the
normal priority is the other way around.
Not using PQconndefaults at all would require yet a third main code
path in do_connect's parameter setup, so I don't really want to fix
it that way. But we can have the logic effectively ignore all the
default values for just a couple more lines of code.
This patch doesn't change the behavior for "\c -reuse-previous=on
service=whatever". That remains significantly different from before
85c54287a, because many more parameters will be re-used, and thus
not be possible for service entries to replace. But I think this
is (mostly?) intentional. In any case, since libpq does not report
where it got parameter values from, it's hard to do differently.
Per bug #16936 from Jasen Betts. As with the previous patches,
back-patch to all supported branches. (9.5 is unfortunately now
out of support, so this won't get fixed there.)
Discussion: https://postgr.es/m/16936-3f524322a53a29f0@postgresql.org
pg_waldump --stats=record identifies a record by a combination
of the RmgrId and the four bits of the xl_info field of the record.
But XACT records use the first bit of those four bits for an optional
flag variable, and the following three bits for the opcode to
identify a record. So previously the same type of XACT record
could have different four bits (three bits are the same but the
first one bit is different), and which could cause
pg_waldump --stats=record to show two lines of per-record statistics
for the same XACT record. This is a bug.
This commit changes pg_waldump --stats=record so that it processes
only XACT record differently, i.e., filters the opcode out of xl_info
and uses a combination of the RmgrId and those three bits as
the identifier of a record, only for XACT record. For other records,
the four bits of the xl_info field are still used.
Back-patch to all supported branches.
Author: Kyotaro Horiguchi
Reviewed-by: Shinya Kato, Fujii Masao
Discussion: https://postgr.es/m/2020100913412132258847@highgo.ca
This commit improves pgbench \sleep command so that it handles
the following three cases more properly.
(1) When only one argument was specified in \sleep command and
it's not a number, previously pgbench reported a confusing error
message like "unrecognized time unit, must be us, ms or s".
This commit fixes this so that more proper error message like
"invalid sleep time, must be an integer" is reported.
(2) When two arguments were specified in \sleep command and
the first argument was not a number, previously pgbench treated
that argument as the sleep time 0. No error was reported in this
case. This commit fixes this so that an error is thrown in this
case.
(3) When a variable was specified as the first argument in \sleep
command and the variable stored non-digit value, previously
pgbench treated that argument as the sleep time 0. No error
was reported in this case. This commit fixes this so that
an error is thrown in this case.
Author: Kota Miyake
Reviewed-by: Hayato Kuroda, Alvaro Herrera, Fujii Masao
Discussion: https://postgr.es/m/23b254daf20cec4332a2d9168505dbc9@oss.nttdata.com
The approach used in commit bbe0a81db would've been disastrous for
portability of dumps. Instead handle non-default compression options
in separate ALTER TABLE commands. This reduces chatter for the
common case where most columns are compressed the same way, and it
makes it possible to restore the dump to a server that lacks any
knowledge of per-attribute compression options (so long as you're
willing to ignore syntax errors from the ALTER TABLE commands).
There's a whole lot left to do to mop up after bbe0a81db, but
I'm fast-tracking this part because we need to see if it's
enough to make the buildfarm's cross-version-upgrade tests happy.
Justin Pryzby and Tom Lane
Discussion: https://postgr.es/m/20210119190720.GL8560@telsasoft.com
There is now a per-column COMPRESSION option which can be set to pglz
(the default, and the only option in up until now) or lz4. Or, if you
like, you can set the new default_toast_compression GUC to lz4, and
then that will be the default for new table columns for which no value
is specified. We don't have lz4 support in the PostgreSQL code, so
to use lz4 compression, PostgreSQL must be built --with-lz4.
In general, TOAST compression means compression of individual column
values, not the whole tuple, and those values can either be compressed
inline within the tuple or compressed and then stored externally in
the TOAST table, so those properties also apply to this feature.
Prior to this commit, a TOAST pointer has two unused bits as part of
the va_extsize field, and a compessed datum has two unused bits as
part of the va_rawsize field. These bits are unused because the length
of a varlena is limited to 1GB; we now use them to indicate the
compression type that was used. This means we only have bit space for
2 more built-in compresison types, but we could work around that
problem, if necessary, by introducing a new vartag_external value for
any further types we end up wanting to add. Hopefully, it won't be
too important to offer a wide selection of algorithms here, since
each one we add not only takes more coding but also adds a build
dependency for every packager. Nevertheless, it seems worth doing
at least this much, because LZ4 gets better compression than PGLZ
with less CPU usage.
It's possible for LZ4-compressed datums to leak into composite type
values stored on disk, just as it is for PGLZ. It's also possible for
LZ4-compressed attributes to be copied into a different table via SQL
commands such as CREATE TABLE AS or INSERT .. SELECT. It would be
expensive to force such values to be decompressed, so PostgreSQL has
never done so. For the same reasons, we also don't force recompression
of already-compressed values even if the target table prefers a
different compression method than was used for the source data. These
architectural decisions are perhaps arguable but revisiting them is
well beyond the scope of what seemed possible to do as part of this
project. However, it's relatively cheap to recompress as part of
VACUUM FULL or CLUSTER, so this commit adjusts those commands to do
so, if the configured compression method of the table happens not to
match what was used for some column value stored therein.
Dilip Kumar. The original patches on which this work was based were
written by Ildus Kurbangaliev, and those were patches were based on
even earlier work by Nikita Glukhov, but the design has since changed
very substantially, since allow a potentially large number of
compression methods that could be added and dropped on a running
system proved too problematic given some of the architectural issues
mentioned above; the choice of which specific compression method to
add first is now different; and a lot of the code has been heavily
refactored. More recently, Justin Przyby helped quite a bit with
testing and reviewing and this version also includes some code
contributions from him. Other design input and review from Tomas
Vondra, Álvaro Herrera, Andres Freund, Oleg Bartunov, Alexander
Korotkov, and me.
Discussion: http://postgr.es/m/20170907194236.4cefce96%40wp.localdomain
Discussion: http://postgr.es/m/CAFiTN-uUpX3ck%3DK0mLEk-G_kUQY%3DSNOTeqdaNRR9FMdQrHKebw%40mail.gmail.com
Buildfarm member fairywren doesn't like the test case I added
in commit 081876d75. I'm guessing the reason is that I shouldn't
be using a perl2host-ified path in the tar command line.
Only "IMPORT" was showing as result of the completion, while IMPORT
FOREIGN SCHEMA is the only command using this keyword in first
position. This changes the completion to show the full command name
instead of just "IMPORT".
Reviewed-by: Georgios Kokolatos, Julien Rouhaud
Discussion: https://postgr.es/m/YFL6JneBiuMWYyoh@paquier.xyz
Commit 05c8482f7f added the implementation of parallel SELECT for
"INSERT INTO ... SELECT ..." which may incur non-negligible overhead in
the additional parallel-safety checks that it performs, even when, in the
end, those checks determine that parallelism can't be used. This is
normally only ever a problem in the case of when the target table has a
large number of partitions.
A new GUC option "enable_parallel_insert" is added, to allow insert in
parallel-mode. The default is on.
In addition to the GUC option, the user may want a mechanism to allow
inserts in parallel-mode with finer granularity at table level. The new
table option "parallel_insert_enabled" allows this. The default is true.
Author: "Hou, Zhijie"
Reviewed-by: Greg Nancarrow, Amit Langote, Takayuki Tsunakawa, Amit Kapila
Discussion: https://postgr.es/m/CAA4eK1K-cW7svLC2D7DHoGHxdAdg3P37BLgebqBOC2ZLc9a6QQ%40mail.gmail.com
Discussion: https://postgr.es/m/CAJcOf-cXnB5cnMKqWEp2E2z7Mvcd04iLVmV=qpFJrR3AcrTS3g@mail.gmail.com
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.
Don't complain about the last TOAST chunk number being different
from what we expected if there are no TOAST chunks at all.
In such a case, saying that the final chunk number is 0 is not
really accurate, and the fact the value is missing from the
TOAST table is reported separately anyway.
Mark Dilger
Discussion: http://postgr.es/m/AA5506CE-7D2A-42E4-A51D-358635E3722D@enterprisedb.com
Since commit ba79cb5dc, values of bind parameters have been logged
during errors in extended query mode. However, we only did that after
we'd collected and converted all the parameter values, thus failing to
offer any useful localization of invalid-parameter problems. Add a
separate callback that's used during parameter collection, and have it
print the parameter number, along with the input string if text input
format is used.
Justin Pryzby and Tom Lane
Discussion: https://postgr.es/m/20210104170939.GH9712@telsasoft.com
Discussion: https://postgr.es/m/CANfkH5k-6nNt-4cSv1vPB80nq2BZCzhFVR5O4VznYbsX0wZmow@mail.gmail.com
Pipeline mode in libpq lets an application avoid the Sync messages in
the FE/BE protocol that are implicit in the old libpq API after each
query. The application can then insert Sync at its leisure with a new
libpq function PQpipelineSync. This can lead to substantial reductions
in query latency.
Co-authored-by: Craig Ringer <craig.ringer@enterprisedb.com>
Co-authored-by: Matthieu Garrigues <matthieu.garrigues@gmail.com>
Co-authored-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Aya Iwata <iwata.aya@jp.fujitsu.com>
Reviewed-by: Daniel Vérité <daniel@manitou-mail.org>
Reviewed-by: David G. Johnston <david.g.johnston@gmail.com>
Reviewed-by: Justin Pryzby <pryzby@telsasoft.com>
Reviewed-by: Kirk Jamison <k.jamison@fujitsu.com>
Reviewed-by: Michael Paquier <michael.paquier@gmail.com>
Reviewed-by: Nikhil Sontakke <nikhils@2ndquadrant.com>
Reviewed-by: Vaishnavi Prabakaran <VaishnaviP@fast.au.fujitsu.com>
Reviewed-by: Zhihong Yu <zyu@yugabyte.com>
Discussion: https://postgr.es/m/CAMsr+YFUjJytRyV4J-16bEoiZyH=4nj+sQ7JP9ajwz=B4dMMZw@mail.gmail.com
Discussion: https://postgr.es/m/CAJkzx4T5E-2cQe3dtv2R78dYFvz+in8PY7A8MArvLhs_pg75gg@mail.gmail.com
It's not immediately obvious what you have to do to get "make
installcheck" to work here, so document that along the same lines
as we've used elsewhere.
It does not work on all versions of perl across all platforms.
To avoid endian-ness issues, pick a new value for column a
that has the same upper 4 bytes as lower 4 bytes. Try to
make it something that isn't likely to occur anywhere nearby
in the page.
Discussion: http://postgr.es/m/29DA079B-0658-4E66-BDAA-0EFD7B64D9C6@enterprisedb.com
Commit 24189277f6 managed to remove
one of the two places where we were checking for a "no such user"
error while leaving the other one right next to it. So remove that
too. In fact, remove the entire test, because the whole point of
this test was to see which message we got on a failure.
It's hard to believe, but buildfarm results from the new pg_amcheck
suggest that command_checks_all() perform shell expansion on some
machines but not others, apparently due to an underlying behavior
difference in IPC::Run. Let's see if we can work around that - and
confirm that it is the real problem - by passing '-S*' as a single
argument rather than '-S' and '*' as two separate ones.
Failures were observed on jacana and hoverfly.
Mark Dilger
Discussion: http://postgr.es/m/9E76E46A-48B2-4869-BD0C-422204C1F767@enterprisedb.com
Test #12 overwrote a 1-byte varlena header to make it look like the
initial byte of a 4-byte varlena header, but the results were
endian-dependent. Also, the byte "abc" that followed the overwritten
byte would be interpreted differently depending on endian-ness.
Overwrite 4 bytes instead, in an endian-aware manner.
Test #13 accidentally managed to depend on TOAST_MAX_CHUNK_SIZE,
which varies slightly depending on MAXIMUM_ALIGNOF. That's not
the point anyway, so make the regexp insensitive to the expected
number of chunks.
Mark Dilger
Discussion: http://postgr.es/m/A80D68F6-E38F-482D-9522-E2FB6AAFE8A1@enterprisedb.com
psql's editing commands decide whether the user has edited the file
by checking for change of modification timestamp. This is probably
fine for a pre-existing file, but with a temporary file that is
created within the command, it's possible for a fast typist to
save-and-exit in less than the one-second granularity of stat(2)
timestamps. On Windows FAT filesystems the granularity is even
worse, 2 seconds, making the race a bit easier to hit.
To fix, try to set the temp file's mod time to be two seconds ago.
It's unlikely this would fail, but then again the race condition
itself is unlikely, so just ignore any error.
Also, we might as well check the file size as well as its mod time.
While this is a difficult bug to hit, it still seems worth
back-patching, to ensure that users' edits aren't lost.
Laurenz Albe, per gripe from Jacob Champion; based on fix suggestions
from Jacob and myself
Discussion: https://postgr.es/m/0ba3f2a658bac6546d9934ab6ba63a805d46a49b.camel@cybertec.at
Create a wrapper object, ParallelSlotArray, to encapsulate the
number of slots and the slot array itself, plus some other relevant
bits of information. This reduces the number of parameters we have
to pass around all over the place.
Allow for a ParallelSlotArray to contain slots connected to
different databases within a single cluster. The current clients
of this mechanism don't need this, but it is expected to be used
by future patches.
Defer connecting to databases until we actually need the connection
for something. This is a slight behavior change for vacuumdb and
reindexdb. If you specify a number of jobs that is larger than the
number of objects, the extra connections will now not be used.
But, on the other hand, if you specify a number of jobs that is
so large that it's going to fail, the failure would previously have
happened before any operations were actually started, and now it
won't.
Mark Dilger, reviewed by me.
Discussion: http://postgr.es/m/12ED3DA8-25F0-4B68-937D-D907CFBF08E7@enterprisedb.com
Discussion: http://postgr.es/m/BA592F2D-F928-46FF-9516-2B827F067F57@enterprisedb.com
Remove the entire idea of "stale stats" within nbtree VACUUM (stop
caring about stats involving the number of inserted tuples). Also
remove the vacuum_cleanup_index_scale_factor GUC/param on the master
branch (though just disable them on postgres 13).
The vacuum_cleanup_index_scale_factor/stats interface made the nbtree AM
partially responsible for deciding when pg_class.reltuples stats needed
to be updated. This seems contrary to the spirit of the index AM API,
though -- it is not actually necessary for an index AM's bulk delete and
cleanup callbacks to provide accurate stats when it happens to be
inconvenient. The core code owns that. (Index AMs have the authority
to perform or not perform certain kinds of deferred cleanup based on
their own considerations, such as page deletion and recycling, but that
has little to do with pg_class.reltuples/num_index_tuples.)
This issue was fairly harmless until the introduction of the
autovacuum_vacuum_insert_threshold feature by commit b07642db, which had
an undesirable interaction with the vacuum_cleanup_index_scale_factor
mechanism: it made insert-driven autovacuums perform full index scans,
even though there is no real benefit to doing so. This has been tied to
a regression with an append-only insert benchmark [1].
Also have remaining cases that perform a full scan of an index during a
cleanup-only nbtree VACUUM indicate that the final tuple count is only
an estimate. This prevents vacuumlazy.c from setting the index's
pg_class.reltuples in those cases (it will now only update pg_class when
vacuumlazy.c had TIDs for nbtree to bulk delete). This arguably fixes
an oversight in deduplication-related bugfix commit 48e12913.
[1] https://smalldatum.blogspot.com/2021/01/insert-benchmark-postgres-is-still.html
Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/CAD21AoA4WHthN5uU6+WScZ7+J_RcEjmcuH94qcoUPuB42ShXzg@mail.gmail.com
Backpatch: 13-, where autovacuum_vacuum_insert_threshold was added.
Commit 547f04e7 produced errors on AIX/xlc while building plpython. The
new code appears to be incompatible with the hack installed by commit
a11cf433. Without access to an AIX system to check, my guess is that
_POSIX_C_SOURCE may be required for <time.h> to declare the things the
header needs to see, but plpython.h undefines it.
For now, to unbreak build farm animal hoverfly, just move the new
pg_time_usec_t support into pgbench.c. Perhaps later we could figure
out what to rearrange to put it back into a header for wider use.
Discussion: https://postgr.es/m/CA%2BhUKG%2BP%2BjcD%3Dx9%2BagyTdWtjpOT64MYiGic%2Bcbu_TD8CV%3D6A3w%40mail.gmail.com
1. pg_time_usec_t needs to be printed with INT64_FORMAT, not %ld, or 32
bit systems complain, per lapwing.
2. Some Windows compilers didn't like a thread function not marked with
__stdcall, per whelk; let's see if this fixes the problem.
Moving this logic into pg_regress fixes a potential failure with
parallel tests when pg_upgrade and the main regression test suite both
trigger the makefile rule that cleaned up testtablespace/ under
src/test/regress. Even if pg_upgrade was triggering this rule, it has
no need to do so as it uses a different tablespace path. So if
pg_upgrade triggered the makefile rule for the tablespace setup while
the main regression test suite ran the tablespace cases, it would fail.
61be85a was a similar attempt at achieving that, but that broke cases
where the regression tests require to run under an Administrator
account, like with Appveyor.
Reported-by: Andres Freund, Kyotaro Horiguchi
Reviewed-by: Peter Eisentraut
Discussion: https://postgr.es/m/20201209012911.uk4d6nxcnkp7ehrx@alap3.anarazel.de
Wait until all pgbench threads are connected before benchmarking begins.
This fixes a problem where some connections could take a very long time
to be established because of lock contention from earlier connections,
making results unstable and bogus with high connection counts.
Author: Andres Freund <andres@anarazel.de>
Author: Fabien COELHO <coelho@cri.ensmp.fr>
Reviewed-by: Marina Polyakova <m.polyakova@postgrespro.ru>
Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/20200227180100.zyvjwzcpiokfsqm2%40alap3.anarazel.de
Instead of instr_time (struct timespec) and the INSTR_XXX macros,
introduce pg_time_usec_t and use integer arithmetic. Don't include the
connection time in TPS unless using -C mode, but report it separately.
Author: Fabien COELHO <coelho@cri.ensmp.fr>
Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Discussion: https://postgr.es/m/20200227180100.zyvjwzcpiokfsqm2%40alap3.anarazel.de
Instead of maintaining an incomplete emulation of POSIX threads for
Windows, let's use an extremely minimalist macro-based abstraction for
now. A later patch will extend this, without the need to supply more
complicated pthread emulation code. (There may be a need for a more
serious portable thread abstraction in later projects, but this is not
it.)
Minor incidental problems fixed: it wasn't OK to use (pthread_t) 0 as a
special value, it wasn't OK to compare thread_t values with ==, and we
incorrectly assumed that pthread functions set errno.
Discussion: https://postgr.es/m/20200227180100.zyvjwzcpiokfsqm2%40alap3.anarazel.de
This partially reverts 096bbf7 and 9d2d457, undoing the libpq changes as
it could cause breakages in distributions that share one single libpq
version across multiple major versions of Postgres for extensions and
applications linking to that.
Note that the backend is unchanged here, and it still disables SSL
compression while simplifying the underlying catalogs that tracked if
compression was enabled or not for a SSL connection.
Per discussion with Tom Lane and Daniel Gustafsson.
Discussion: https://postgr.es/m/YEbq15JKJwIX+S6m@paquier.xyz
PostgreSQL disabled compression as of e3bdb2d and the documentation
recommends against using it since. Additionally, SSL compression has
been disabled in OpenSSL since version 1.1.0, and was disabled in many
distributions long before that. The most recent TLS version, TLSv1.3,
disallows compression at the protocol level.
This commit removes the feature itself, removing support for the libpq
parameter sslcompression (parameter still listed for compatibility
reasons with existing connection strings, just ignored), and removes
the equivalent field in pg_stat_ssl and de facto PgBackendSSLStatus.
Note that, on top of removing the ability to activate compression by
configuration, compression is actively disabled in both frontend and
backend to avoid overrides from local configurations.
A TAP test is added for deprecated SSL parameters to check after
backwards compatibility.
Bump catalog version.
Author: Daniel Gustafsson
Reviewed-by: Peter Eisentraut, Magnus Hagander, Michael Paquier
Discussion: https://postgr.es/m/7E384D48-11C5-441B-9EC3-F7DB1F8518F6@yesql.se
Using pgbench in an environment with both PGPORT and PGUSER set would
have caused the generation of a debug log with an incorrect database
name due to an oversight in 412893b. Not specifying user, port and/or
database using the option switches, without their respective environment
variables, generated a log entry with empty strings, which was
rather useless.
This commit fixes this set of issues by simplifying the logic grabbing
the connection information, removing a set of getenv() calls that
emulated what libpq already does. The faulty debug log now directly
uses the information from the libpq connection, and it gets generated
after the connection to the backend is completed, not before it (in the
event of a failure libpq would complain with more information about the
connection attempt so the log is not really useful before anyway).
Author: Kota Miyake
Reviewed-by: Fujii Masao, Michael Paquier
Discussion: https://postgr.es/m/026b3ae6fc339a18394d053c32a4463d@oss.nttdata.com
Mistake in f06b1c598254f8adb2b7f51d6a7685618a7fb121: We should only
check the version of the binaries in the target installation. The
source installation can of course be of a different version.
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://www.postgresql.org/message-id/flat/E1lHNKN-0005IC-V6%40gemulon.postgresql.org
Protocol version 3 was introduced in PostgreSQL 7.4. There shouldn't be
many clients or servers left out there without version 3 support. But as
a courtesy, I kept just enough of the old protocol support that we can
still send the "unsupported protocol version" error in v2 format, so that
old clients can display the message properly. Likewise, libpq still
understands v2 ErrorResponse messages when establishing a connection.
The impetus to do this now is that I'm working on a patch to COPY
FROM, to always prefetch some data. We cannot do that safely with the
old protocol, because it requires parsing the input one byte at a time
to detect the end-of-copy marker.
Reviewed-by: Tom Lane, Alvaro Herrera, John Naylor
Discussion: https://www.postgresql.org/message-id/9ec25819-0a8a-d51a-17dc-4150bb3cca3b%40iki.fi
This expands the binary validation in pg_upgrade with a version
check per binary to ensure that the target cluster installation
only contains binaries from the target version.
In order to reduce duplication, validate_exec is exported from
port.h and the local copy in pg_upgrade is removed.
Author: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://www.postgresql.org/message-id/flat/9328.1552952117@sss.pgh.pa.us
This option provides REINDEX (TABLESPACE) for reindexdb, applying the
tablespace value given by the caller to all the REINDEX queries
generated.
While on it, this commit adds some tests for REINDEX TABLESPACE, with
and without CONCURRENTLY, when run on toast indexes and tables. Such
operations are not allowed, and toast relation names are not stable
enough to be part of the main regression test suite (even if using a PL
function with a TRY/CATCH logic, as CONCURRENTLY could not be tested).
Author: Michael Paquier
Reviewed-by: Mark Dilger, Daniel Gustafsson
Discussion: https://postgr.es/m/YDiaDMnzLICqeukl@paquier.xyz
On Windows, CMD.EXE allegedly does not run a command that uses forward slashes,
so let's convert the path to use backslashes instead.
Backpatch to 10.
Author: Nitin Jadhav <nitinjadhavpostgres@gmail.com>
Reviewed-by: Juan José Santamaría Flecha <juanjo.santamaria@gmail.com>
Discussion: https://postgr.es/m/CAMm1aWaNDuaPYFYMAqDeJrZmPtNvLcJRS++CcZWY8LT6KcoBZw@mail.gmail.com
The same test for REINDEX (VERBOSE) was done twice, while it is clear
that the second test should use --concurrently. Issue introduced in
5dc92b8, for what looks like a copy-paste mistake.
Reviewed-by: Mark Dilger
Discussion: https://postgr.es/m/A7AE97EA-F4B0-4CAB-8FFF-3FECD31F9D63@enterprisedb.com
Backpatch-through: 12
When ON_ERROR_ROLLBACK is enabled, psql releases a temporary savepoint
if it's idle in a valid transaction block after executing a query. But psql
doesn't do that after RELEASE or ROLLBACK is executed because a temporary
savepoint has already been destroyed in that case.
This commit changes psql's ON_ERROR_ROLLBACK so that it doesn't release
a temporary savepoint also when COMMIT AND CHAIN is executed. A temporary
savepoint doesn't need to be released in that case because
COMMIT AND CHAIN also destroys any savepoints defined within the transaction
to commit. Otherwise psql tries to release the savepoint that
COMMIT AND CHAIN has already destroyed and cause an error
"ERROR: savepoint "pg_psql_temporary_savepoint" does not exist".
Back-patch to v12 where transaction chaining was added.
Reported-by: Arthur Nascimento
Author: Arthur Nascimento
Reviewed-by: Fujii Masao, Vik Fearing
Discussion: https://postgr.es/m/16867-3475744069228158@postgresql.org
ALTER INDEX was able to handle that already. This adds tab completion
for all the remaining commands that support this grammar:
- ALTER FUNCTION
- ALTER PROCEDURE
- ALTER ROUTINE
- ALTER TRIGGER
- ALTER MATERIALIZED VIEW
Author: Ian Lawrence Barwick
Discussion: https://postgr.es/m/CAB8KJ=iypYudXuMOAMOP4BpkaYbXxk=a2cdJppX0e9mJXWtuig@mail.gmail.com
With its current design, a careless use of pg_cryptohash_final() could
would result in an out-of-bound write in memory as the size of the
destination buffer to store the result digest is not known to the
cryptohash internals, without the caller knowing about that. This
commit adds a new argument to pg_cryptohash_final() to allow such sanity
checks, and implements such defenses.
The internals of SCRAM for HMAC could be tightened a bit more, but as
everything is based on SCRAM_KEY_LEN with uses particular to this code
there is no need to complicate its interface more than necessary, and
this comes back to the refactoring of HMAC in core. Except that, this
minimizes the uses of the existing DIGEST_LENGTH variables, relying
instead on sizeof() for the result sizes. In ossp-uuid, this also makes
the code more defensive, as it already relied on dce_uuid_t being at
least the size of a MD5 digest.
This is in philosophy similar to cfc40d3 for base64.c and aef8948 for
hex.c.
Reported-by: Ranier Vilela
Author: Michael Paquier, Ranier Vilela
Reviewed-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/CAEudQAoqEGmcff3J4sTSV-R_16Monuz-UpJFbf_dnVH=APr02Q@mail.gmail.com
This option controls if toast tables associated with a relation are
vacuumed or not when running a manual VACUUM. It was already possible
to trigger a manual VACUUM on a toast relation without processing its
main relation, but a manual vacuum on a main relation always forced a
vacuum on its toast table. This is useful in scenarios where the level
of bloat or transaction age of the main and toast relations differs a
lot.
This option is an extension of the existing VACOPT_SKIPTOAST that was
used by autovacuum to control if toast relations should be skipped or
not. This internal flag is renamed to VACOPT_PROCESS_TOAST for
consistency with the new option.
A new option switch, called --no-process-toast, is added to vacuumdb.
Author: Nathan Bossart
Reviewed-by: Kirk Jamison, Michael Paquier, Justin Pryzby
Discussion: https://postgr.es/m/BA8951E9-1524-48C5-94AF-73B1F0D7857F@amazon.com
The parallel slots infrastructure (which implements client-side
multiplexing of server connections doing similar things, not
threading or multiple processes or anything like that) are moved from
src/bin/scripts/scripts_parallel.c to src/fe_utils/parallel_slot.c.
The functions consumeQueryResult() and processQueryResult() which were
previously part of src/bin/scripts/common.c are now moved into that
file as well, becoming static helper functions. This might need to be
changed in the future, but currently they're not used for anything
else.
Some other functions from src/bin/scripts/common.c are moved to to
src/fe_utils and are split up among several files. connectDatabase(),
connectMaintenanceDatabase(), and disconnectDatabase() are moved to
connect_utils.c. executeQuery(), executeCommand(), and
executeMaintenanceCommand() are move to query_utils.c.
handle_help_version_opts() is moved to option_utils.c.
Mark Dilger, reviewed by me. The larger patch series of which this is
a part has also had review from Peter Geoghegan, Andres Freund, Álvaro
Herrera, Michael Paquier, and Amul Sul, but I don't know whether any
of them have reviewed this bit specifically.
Discussion: http://postgr.es/m/12ED3DA8-25F0-4B68-937D-D907CFBF08E7@enterprisedb.com
Discussion: http://postgr.es/m/5F743835-3399-419C-8324-2D424237E999@enterprisedb.com
Discussion: http://postgr.es/m/70655DF3-33CE-4527-9A4D-DDEB582B6BA0@enterprisedb.com
This patch adds the possibility to move indexes to a new tablespace
while rebuilding them. Both the concurrent and the non-concurrent cases
are supported, and the following set of restrictions apply:
- When using TABLESPACE with a REINDEX command that targets a
partitioned table or index, all the indexes of the leaf partitions are
moved to the new tablespace. The tablespace references of the non-leaf,
partitioned tables in pg_class.reltablespace are not changed. This
requires an extra ALTER TABLE SET TABLESPACE.
- Any index on a toast table rebuilt as part of a parent table is kept
in its original tablespace.
- The operation is forbidden on system catalogs, including trying to
directly move a toast relation with REINDEX. This results in an error
if doing REINDEX on a single object. REINDEX SCHEMA, DATABASE and
SYSTEM skip system relations when TABLESPACE is used.
Author: Alexey Kondratov, Michael Paquier, Justin Pryzby
Reviewed-by: Álvaro Herrera, Michael Paquier
Discussion: https://postgr.es/m/8a8f5f73-00d3-55f8-7583-1375ca8f6a91@postgrespro.ru
Generation expressions of generated columns are always inherited, so
there is no need to set them separately in child tables, and there is
no syntax to do so either. The code previously used the code paths
for the handling of default values, for which different rules apply;
in particular it might want to set a default value explicitly for an
inherited column. This resulted in unrestorable dumps. For generated
columns, just skip them in inherited tables.
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/15830.1575468847%40sss.pgh.pa.us
For those system catalogs that have a unique indexes, make a primary
key and unique constraint, using ALTER TABLE ... PRIMARY KEY/UNIQUE
USING INDEX.
This can be helpful for GUI tools that look for a primary key, and it
might in the future allow declaring foreign keys, for making schema
diagrams.
The constraint creation statements are automatically created by
genbki.pl from DECLARE_UNIQUE_INDEX directives. To specify which one
of the available unique indexes is the primary key, use the new
directive DECLARE_UNIQUE_INDEX_PKEY instead. By convention, we
usually make a catalog's OID column its primary key, if it has one.
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/dc5f44d9-5ec1-a596-0251-dadadcdede98@2ndquadrant.com
doConnect() never returns connections in state CONNECTION_BAD, so
checking for that is pointless. Remove the code that does.
This code has been dead since ba708ea3dc, 20 years ago.
Discussion: https://postgr.es/m/20210126195224.GA20361@alvherre.pgsql
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
CREATE TABLE AS has been preferred over SELECT INTO (outside of ecpg
and PL/pgSQL) for a long time. There were still a few uses of SELECT
INTO in tests and documentation, some old, some more recent. This
changes them to CREATE TABLE AS. Some occurrences in the tests remain
where they are specifically testing SELECT INTO parsing or similar.
Discussion: https://www.postgresql.org/message-id/flat/96dc0df3-e13a-a85d-d045-d6e2c85218da%40enterprisedb.com
The loops to identify word boundaries could access past the end of
the input string. Likely that would never result in an actual
crash, but it makes valgrind unhappy.
The logic to try different numbers of words didn't work when the
input has two words but we only have a match to the first, eg
"\h with select". (We must "continue" the pass loop, not "break".)
The logic to compute nl_count was bizarrely managed, and in at
least two code paths could end up calling PageOutput with
nl_count = 0, resulting in failing to paginate output that should
have been fed to the pager. Also, in v12 and up, the nl_count
calculation hadn't been updated to account for the addition of a URL.
The PQExpBuffer holding the command syntax details wasn't freed,
resulting in a session-lifespan memory leak.
While here, improve some comments, choose a more descriptive name
for a variable, fix inconsistent datatype choice for another variable.
Per bug #16837 from Alexander Lakhin. This code is very old,
so back-patch to all supported branches.
Kyotaro Horiguchi and Tom Lane
Discussion: https://postgr.es/m/16837-479bcd56040c71b3@postgresql.org
libpq's error messages for connection failures pretty well stand on
their own, especially since commits 52a10224e/27a48e5a1. Prefixing
them with 'could not connect to database "foo"' or the like is just
redundant, and perhaps even misleading if the specific database name
isn't relevant to the failure. (When it is, we trust that the
backend's error message will include the DB name.) Indeed, psql
hasn't used any such prefix in a long time. So, make all our other
programs and documentation examples agree with psql's practice.
Discussion: https://postgr.es/m/1094524.1611266589@sss.pgh.pa.us
"connection to server so-and-so failed:" seems clearer than the
previous wording "could not connect to so-and-so:" (introduced by
52a10224e), because the latter suggests a network-level connection
failure. We're now prefixing this string to all types of connection
failures, for instance authentication failures; so we need wording
that doesn't imply a low-level error.
Per discussion with Robert Haas.
Discussion: https://postgr.es/m/CA+TgmobssJ6rS22dspWnu-oDxXevGmhMD8VcRBjmj-b9UDqRjw@mail.gmail.com
The new command lists extended statistics objects. All past releases
with extended statistics are supported.
This is a simplified version of commit 891a1d0bca, which had to be
reverted due to not considering pg_statistic_ext_data is not accessible
by regular users. Fields requiring access to this catalog were removed.
It's possible to add them, but it'll require changes to core.
Author: Tatsuro Yamada
Reviewed-by: Julien Rouhaud, Alvaro Herrera, Tomas Vondra, Noriyoshi Shinoda
Discussion: https://postgr.es/m/c027a541-5856-75a5-0868-341301e1624b%40nttcom.co.jp_1
Specifying this parameter removes the informational messages about how
to start the server. This is intended for use by wrappers in different
packaging systems, where those instructions would most likely be wrong
anyway, but the other output from initdb would still be useful (and thus
just redirecting everything to /dev/null would be bad).
Author: Magnus Hagander
Reviewed-By: Peter Eisentraut
Discusion: https://postgr.es/m/CABUevEzo4t5bmTXF0_B9WzmuWpVbMpkNZZiGvzV8NZa-=fPqeQ@mail.gmail.com
The new command lists extended statistics objects, possibly with their
sizes. All past releases with extended statistics are supported.
Author: Tatsuro Yamada
Reviewed-by: Julien Rouhaud, Alvaro Herrera, Tomas Vondra
Discussion: https://postgr.es/m/c027a541-5856-75a5-0868-341301e1624b%40nttcom.co.jp_1
The context is an object that no longer bears some aclitem that it bore
initially. (A user issued REVOKE or GRANT statements upon the object.)
pg_dump is forming SQL to reproduce the object ACL. Since initdb
creates no ACL bearing GRANT OPTION, reaching this bug requires an
extension where the creation script establishes such an ACL. No PGXN
extension does that. If an installation did reach the bug, pg_dump
would have omitted a semicolon, causing a REVOKE and the next SQL
statement to fail. Separately, since the affected code exists to
eliminate an entire aclitem, it wants plain REVOKE, not REVOKE GRANT
OPTION FOR. Back-patch to 9.6, where commit
23f34fa4ba first appeared.
Discussion: https://postgr.es/m/20210109102423.GA160022@rfd.leadboat.com
This is the same fix as commit 9eabfe300 applied to INDEX ATTACH
entries, but for table-to-publication attachments. As in that
case, even though the backend doesn't record "ownership" of the
attachment, we still ought to label it in the dump archive with
the role name that should run the ALTER PUBLICATION command.
The existing behavior causes the ALTER to be done by the original
role that started the restore; that will usually work fine, but
there may be corner cases where it fails.
The bulk of the patch is concerned with changing struct
PublicationRelInfo to include a pointer to the associated
PublicationInfo object, so that we can get the owner's name
out of that when the time comes. While at it, I rewrote
getPublicationTables() to do just one query of pg_publication_rel,
not one per table.
Back-patch to v10 where this code was introduced.
Discussion: https://postgr.es/m/1165710.1610473242@sss.pgh.pa.us
This commit makes CLOSE, FETCH and MOVE commands tab-complete the list of
cursors. Also this commit makes DECLARE command tab-complete the options.
Author: Shinya Kato, Sawada Masahiko, tweaked by Fujii Masao
Reviewed-by: Shinya Kato, Sawada Masahiko, Fujii Masao
Discussion: https://postgr.es/m/b0e4c5c53ef84c5395524f5056fc71f0@MP-MSGSS-MBX001.msg.nttdata.co.jp
The point of this restriction is to avoid trying to substitute variables
into timestamp literal values, which may contain strings like '12:34'.
There is a good deal more that should be done to reduce pgbench's
tendency to substitute where it shouldn't. But this is sufficient to
solve the case complained of by Jaime Soler, and it's simple enough
to back-patch.
Back-patch to v11; before commit 9d36a3866, pgbench had a slightly
different definition of what a variable name is, and anyway it seems
unwise to change long-stable branches for this.
Fabien Coelho
Discussion: https://postgr.es/m/alpine.DEB.2.22.394.2006291740420.805678@pseudo
Although a partitioned index's attachment to its parent doesn't
have separate ownership, the ArchiveEntry for it needs to be
marked with an owner anyway, to ensure that the ALTER command
is run by the appropriate role when restoring with
--use-set-session-authorization. Without this, the ALTER will
be run by the role that started the restore session, which will
usually work but it's formally the wrong thing.
Back-patch to v11 where this type of ArchiveEntry was added.
In HEAD, add equivalent commentary to the just-added TABLE ATTACH
case, which I'd made do the right thing already.
Discussion: https://postgr.es/m/1094034.1610418498@sss.pgh.pa.us
Previously, we emitted the ATTACH PARTITION command as part of
the child table's ArchiveEntry. This was a poor choice since it
complicates restoring the partition as a standalone table; you have
to ignore the error from the ATTACH, which isn't even an option when
restoring direct-to-database with pg_restore. (pg_restore will issue
the whole ArchiveEntry as one PQexec, so that any error rolls back
the table creation as well.) Hence, separate it out as its own
ArchiveEntry, as indeed we already did for index ATTACH PARTITION
commands.
Justin Pryzby
Discussion: https://postgr.es/m/20201023052940.GE9241@telsasoft.com
Wedging a new object type into this table has historically required
manually renumbering a lot of existing entries. (Although it appears
that some people got lazy and re-used the priority level of an
existing object type, even if it wasn't particularly related.)
We can let the compiler do the counting by inventing an enum type that
lists the desired priority levels in order. Now, if you want to add
or remove a priority level, that's a one-liner.
This patch is not purely cosmetic, because I split apart the priorities
of DO_COLLATION and DO_TRANSFORM, as well as those of DO_ACCESS_METHOD
and DO_OPERATOR, which look to me to have been merged out of expediency
rather than because it was a good idea. Shell types continue to be
sorted interchangeably with full types, and opclasses interchangeably
with opfamilies.
Prefix "could not connect to host-or-socket-path:" to all connection
failure cases that occur after the socket() call, and remove the
ad-hoc server identity data that was appended to a few of these
messages. This should produce much more intelligible error reports
in multiple-target-host situations, especially for error cases that
are off the beaten track to any degree (because none of those provided
any server identity info).
As an example of the change, formerly a connection attempt with a bad
port number such as "psql -p 12345 -h localhost,/tmp" might produce
psql: error: could not connect to server: Connection refused
Is the server running on host "localhost" (::1) and accepting
TCP/IP connections on port 12345?
could not connect to server: Connection refused
Is the server running on host "localhost" (127.0.0.1) and accepting
TCP/IP connections on port 12345?
could not connect to server: No such file or directory
Is the server running locally and accepting
connections on Unix domain socket "/tmp/.s.PGSQL.12345"?
Now it looks like
psql: error: could not connect to host "localhost" (::1), port 12345: Connection refused
Is the server running on that host and accepting TCP/IP connections?
could not connect to host "localhost" (127.0.0.1), port 12345: Connection refused
Is the server running on that host and accepting TCP/IP connections?
could not connect to socket "/tmp/.s.PGSQL.12345": No such file or directory
Is the server running locally and accepting connections on that socket?
This requires adjusting a couple of regression tests to allow for
variation in the contents of a connection failure message.
Discussion: https://postgr.es/m/BN6PR05MB3492948E4FD76C156E747E8BC9160@BN6PR05MB3492.namprd05.prod.outlook.com
We found last February that the error-case tests added by commit
008cf0409 failed on OpenBSD, because that platform doesn't really
check locale names. At the time it seemed that that was only an issue
for LC_CTYPE, but testing on a more recent version of OpenBSD shows
that it's now equally lax about LC_COLLATE.
Rather than dropping the LC_COLLATE test too, put back LC_CTYPE
(reverting c4b0edb07), and adjust these tests to accept the different
error message that we get if setlocale() doesn't reject a bogus locale
name. The point of these tests is not really what the backend does
with the locale name, but to show that createdb quotes funny locale
names safely; so we're not losing test reliability this way.
Back-patch as appropriate.
Discussion: https://postgr.es/m/231373.1610058324@sss.pgh.pa.us
Formerly, TOAST objects were unconditionally suppressed, but since
\d is able to print them it's not very clear why these variants
should not. Instead, use the same rules as for system catalogs:
they can be seen if you write the 'S' modifier or a table name
pattern. (In practice, since hardly anybody would keep pg_toast
in their search_path, it's really down to whether you use a pattern
that can match pg_toast.*.)
No docs change seems necessary because the docs already say that
this happens for "system objects"; we're just classifying TOAST
tables as being that.
Justin Pryzby, reviewed by Laurenz Albe
Discussion: https://postgr.es/m/20201130165436.GX24052@telsasoft.com
Commit 7ca37fb04 removed regress_putenv from the regress.so library,
so reloading a SQL function dependent on that would not work.
Fix similarly to 52202bb39.
Per buildfarm.
Since at least 2001 we've used putenv() and avoided setenv(), on the
grounds that the latter was unportable and not in POSIX. However,
POSIX added it that same year, and by now the situation has reversed:
setenv() is probably more portable than putenv(), since POSIX now
treats the latter as not being a core function. And setenv() has
cleaner semantics too. So, let's reverse that old policy.
This commit adds a simple src/port/ implementation of setenv() for
any stragglers (we have one in the buildfarm, but I'd not be surprised
if that code is never used in the field). More importantly, extend
win32env.c to also support setenv(). Then, replace usages of putenv()
with setenv(), and get rid of some ad-hoc implementations of setenv()
wannabees.
Also, adjust our src/port/ implementation of unsetenv() to follow the
POSIX spec that it returns an error indicator, rather than returning
void as per the ancient BSD convention. I don't feel a need to make
all the call sites check for errors, but the portability stub ought
to match real-world practice.
Discussion: https://postgr.es/m/2065122.1609212051@sss.pgh.pa.us
The patch needs test cases, reorganization, and cfbot testing.
Technically reverts commits 5c31afc49d..e35b2bad1a (exclusive/inclusive)
and 08db7c63f3..ccbe34139b.
Reported-by: Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/E1ktAAG-0002V2-VB@gemulon.postgresql.org
This adds a key management system that stores (currently) two data
encryption keys of length 128, 192, or 256 bits. The data keys are
AES256 encrypted using a key encryption key, and validated via GCM
cipher mode. A command to obtain the key encryption key must be
specified at initdb time, and will be run at every database server
start. New parameters allow a file descriptor open to the terminal to
be passed. pg_upgrade support has also been added.
Discussion: https://postgr.es/m/CA+fd4k7q5o6Nc_AaX6BcYM9yqTbC6_pnH-6nSD=54Zp6NBQTCQ@mail.gmail.com
Discussion: https://postgr.es/m/20201202213814.GG20285@momjian.us
Author: Masahiko Sawada, me, Stephen Frost
Buildfarm members pogona and petalura have shown a failure when
pg_ctl/t/004_logrotate.pl starts just before local midnight.
The default rotate-at-midnight behavior occurs just before the
Perl script examines current_logfiles, so it figures that the
rotation it's already requested has occurred ... but in reality,
that rotation happens just after it looks, so the expected new
log data goes into a different file than the one it's examining.
In HEAD, src/test/kerberos/t/001_auth.pl has acquired similar code
that evidently has a related failure mode. Besides being quite new,
few buildfarm critters run that test, so it's unsurprising that
we've not yet seen a failure there.
Fix both cases by setting log_rotation_age = 0 so that no time-based
rotation can occur. Also absorb 004_logrotate.pl's decision to
set lc_messages = 'C' into the kerberos test, in hopes that it will
work in non-English prevailing locales.
Report: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=pogona&dt=2020-12-24%2022%3A10%3A04
Report: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=petalura&dt=2020-02-01%2022%3A20%3A04
Multiranges are basically sorted arrays of non-overlapping ranges with
set-theoretic operations defined over them.
Since v14, each range type automatically gets a corresponding multirange
datatype. There are both manual and automatic mechanisms for naming multirange
types. Once can specify multirange type name using multirange_type_name
attribute in CREATE TYPE. Otherwise, a multirange type name is generated
automatically. If the range type name contains "range" then we change that to
"multirange". Otherwise, we add "_multirange" to the end.
Implementation of multiranges comes with a space-efficient internal
representation format, which evades extra paddings and duplicated storage of
oids. Altogether this format allows fetching a particular range by its index
in O(n).
Statistic gathering and selectivity estimation are implemented for multiranges.
For this purpose, stored multirange is approximated as union range without gaps.
This field will likely need improvements in the future.
Catversion is bumped.
Discussion: https://postgr.es/m/CALNJ-vSUpQ_Y%3DjXvTxt1VYFztaBSsWVXeF1y6gTYQ4bOiWDLgQ%40mail.gmail.com
Discussion: https://postgr.es/m/a0b8026459d1e6167933be2104a6174e7d40d0ab.camel%40j-davis.com#fe7218c83b08068bfffb0c5293eceda0
Author: Paul Jungwirth, revised by me
Reviewed-by: David Fetter, Corey Huinker, Jeff Davis, Pavel Stehule
Reviewed-by: Alvaro Herrera, Tom Lane, Isaac Morland, David G. Johnston
Reviewed-by: Zhihong Yu, Alexander Korotkov
This patch generalizes the subscripting infrastructure so that any
data type can be subscripted, if it provides a handler function to
define what that means. Traditional variable-length (varlena) arrays
all use array_subscript_handler(), while the existing fixed-length
types that support subscripting use raw_array_subscript_handler().
It's expected that other types that want to use subscripting notation
will define their own handlers. (This patch provides no such new
features, though; it only lays the foundation for them.)
To do this, move the parser's semantic processing of subscripts
(including coercion to whatever data type is required) into a
method callback supplied by the handler. On the execution side,
replace the ExecEvalSubscriptingRef* layer of functions with direct
calls to callback-supplied execution routines. (Thus, essentially
no new run-time overhead should be caused by this patch. Indeed,
there is room to remove some overhead by supplying specialized
execution routines. This patch does a little bit in that line,
but more could be done.)
Additional work is required here and there to remove formerly
hard-wired assumptions about the result type, collation, etc
of a SubscriptingRef expression node; and to remove assumptions
that the subscript values must be integers.
One useful side-effect of this is that we now have a less squishy
mechanism for identifying whether a data type is a "true" array:
instead of wiring in weird rules about typlen, we can look to see
if pg_type.typsubscript == F_ARRAY_SUBSCRIPT_HANDLER. For this
to be bulletproof, we have to forbid user-defined types from using
that handler directly; but there seems no good reason for them to
do so.
This patch also removes assumptions that the number of subscripts
is limited to MAXDIM (6), or indeed has any hard-wired limit.
That limit still applies to types handled by array_subscript_handler
or raw_array_subscript_handler, but to discourage other dependencies
on this constant, I've moved it from c.h to utils/array.h.
Dmitry Dolgov, reviewed at various times by Tom Lane, Arthur Zakirov,
Peter Eisentraut, Pavel Stehule
Discussion: https://postgr.es/m/CA+q6zcVDuGBv=M0FqBYX8DPebS3F_0KQ6OVFobGJPM507_SZ_w@mail.gmail.com
Discussion: https://postgr.es/m/CA+q6zcVovR+XY4mfk-7oNk-rF91gH0PebnNfuUjuuDsyHjOcVA@mail.gmail.com
pg_rewind looks at the control file to check what timeline a server is on.
But promotion doesn't immediately write a checkpoint, it merely writes
an end-of-recovery WAL record. If pg_rewind runs immediately after
promotion, before the checkpoint has completed, it will think think that
the server is still on the earlier timeline. We ran into this issue a long
time ago already, see commit 484a848a73.
It's a bit bogus that pg_rewind doesn't determine the timeline correctly
until the end-of-recovery checkpoint has completed. We probably should
fix that. But for now work around it by waiting for the checkpoint
to complete before running pg_rewind, like we did in commit 484a848a73.
In the passing, tidy up the new test a little bit. Rerder the INSERTs so
that the comments make more sense, remove a spurious CHECKPOINT call after
pg_rewind has already run, and add --debug option, so that if this fails
again, we'll have more data.
Per buildfarm failure at https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=rorqual&dt=2020-12-06%2018%3A32%3A19&stg=pg_rewind-check.
Backpatch to all supported versions.
Discussion: https://www.postgresql.org/message-id/1713707e-e318-761c-d287-5b6a4aa807e8@iki.fi
Along the same lines as ed2c7f65b and daa9fe8a5, reduce code duplication
by having just one copy of the parts of the query that are the same
across all server versions; and make the conditionals control the
smallest possible amount of code. This is in preparation for adding
another dumpable field to pg_type.
An error code path newly-introduced by 87ae969 forgot to close a file
descriptor when verifying a file's checksum.
Per report from Coverity, via Tom Lane.
Buildfarm has been failing sporadically on the new test. I was able to
reproduce this by adding a random 0-10 s delay in the walreceiver, just
before it connects to the primary. There's a race condition where node_3
is promoted before it has fully caught up with node_1, leading to diverged
timelines. When node_1 is later reconfigured as standby following node_3,
it fails to catch up:
LOG: primary server contains no more WAL on requested timeline 1
LOG: new timeline 2 forked off current database system timeline 1 before current recovery point 0/30000A0
That's the situation where you'd need to use pg_rewind, but in this case
it happens already when we are just setting up the actual pg_rewind
scenario we want to test, so change the test so that it waits until
node_3 is connected and fully caught up before promoting it, so that you
get a clean, controlled failover.
Also rewrite some of the comments, for clarity. The existing comments
detailed what each step in the test did, but didn't give a good overview
of the situation the steps were trying to create.
For reasons I don't understand, the test setup had to be written slightly
differently in 9.6 and 9.5 than in later versions. The 9.5/9.6 version
needed node 1 to be reinitialized from backup, whereas in later versions
it could be shut down and reconfigured to be a standby. But even 9.5 should
support "clean switchover", where primary makes sure that pending WAL is
replicated to standby on shutdown. It would be nice to figure out what's
going on there, but that's independent of pg_rewind and the scenario that
this test tests.
Discussion: https://www.postgresql.org/message-id/b0a3b95b-82d2-6089-6892-40570f8c5e60%40iki.fi
If the target is a standby server, its WAL doesn't end at the last
checkpoint record, but at minRecoveryPoint. We must scan all the
WAL from the last common checkpoint all the way up to minRecoveryPoint
for modified pages, and also consider that portion when determining
whether the server needs rewinding.
Backpatch to all supported versions.
Author: Ian Barwick and me
Discussion: https://www.postgresql.org/message-id/CABvVfJU-LDWvoz4-Yow3Ay5LZYTuPD7eSjjE4kGyNZpXC6FrVQ%40mail.gmail.com
This changes CLUSTER and REINDEX so as a parenthesized grammar becomes
possible for options, while unifying the grammar parsing rules for
option lists with the existing ones.
This is a follow-up of the work done in 873ea9e for VACUUM, ANALYZE and
EXPLAIN. This benefits REINDEX for a potential backend-side filtering
for collatable-sensitive indexes and TABLESPACE, while CLUSTER would
benefit from the latter.
Author: Alexey Kondratov, Justin Pryzby
Discussion: https://postgr.es/m/8a8f5f73-00d3-55f8-7583-1375ca8f6a91@postgrespro.ru
Two new routines to allocate a hash context and to free it are created,
as these become necessary for the goal behind this refactoring: switch
the all cryptohash implementations for OpenSSL to use EVP (for FIPS and
also because upstream does not recommend the use of low-level cryptohash
functions for 20 years). Note that OpenSSL hides the internals of
cryptohash contexts since 1.1.0, so it is necessary to leave the
allocation to OpenSSL itself, explaining the need for those two new
routines. This part is going to require more work to properly track
hash contexts with resource owners, but this not introduced here.
Still, this refactoring makes the move possible.
This reduces the number of routines for all SHA2 implementations from
twelve (SHA{224,256,386,512} with init, update and final calls) to five
(create, free, init, update and final calls) by incorporating the hash
type directly into the hash context data.
The new cryptohash routines are moved to a new file, called cryptohash.c
for the fallback implementations, with SHA2 specifics becoming a part
internal to src/common/. OpenSSL specifics are part of
cryptohash_openssl.c. This infrastructure is usable for more hash
types, like MD5 or HMAC.
Any code paths using the internal SHA2 routines are adapted to report
correctly errors, which are most of the changes of this commit. The
zones mostly impacted are checksum manifests, libpq and SCRAM.
Note that e21cbb4 was a first attempt to switch SHA2 to EVP, but it
lacked the refactoring needed for libpq, as done here.
This patch has been tested on Linux and Windows, with and without
OpenSSL, and down to 1.0.1, the oldest version supported on HEAD.
Author: Michael Paquier
Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/20200924025314.GE7405@paquier.xyz
Through my misreading of what the existing code actually did,
commits 85c54287a et al. broke psql's behavior for the case where
"\c connstring" provides a password in the connstring. We should
use that password in such a case, but as of 85c54287a we ignored it
(and instead, prompted for a password).
Commit 94929f1cf fixed that in HEAD, but since I thought it was
cleaning up a longstanding misbehavior and not one I'd just created,
I didn't back-patch it.
Hence, back-patch the portions of 94929f1cf having to do with
password management. In addition to fixing the introduced bug,
this means that "\c -reuse-previous=on connstring" will allow
re-use of an existing connection's password if the connstring
doesn't change user/host/port. That didn't happen before, but
it seems like a bug fix, and anyway I'm loath to have significant
differences in this code across versions.
Also fix an error with the same root cause about whether or not to
override a connstring's setting of client_encoding. As of 85c54287a
we always did so; restore the previous behavior of overriding only
when stdin/stdout are a terminal and there's no environment setting
of PGCLIENTENCODING. (I find that definition a bit surprising, but
right now doesn't seem like the time to revisit it.)
Per bug #16746 from Krzysztof Gradek. As with the previous patch,
back-patch to all supported branches.
Discussion: https://postgr.es/m/16746-44b30e2edf4335d4@postgresql.org
Buildfarm member topminnow failed when the test script attempted this
before the syslogger would have created the file. Back-patch to v12,
which introduced the test.
Historically, psql has truncated the text of a column's default
expression at 128 characters. This is unlike any other behavior
in describe.c, and it's become particularly confusing now that
the limit is only applied to the expression proper and not to
the "generated always as (...) stored" text that may get wrapped
around it.
Excavation in our git history suggests that the original motivation
for this limit was not really to limit the display width (as I'd long
supposed), but to make it safe to use a fixed-width output buffer to
store the result. That implementation restriction is long gone of
course, but the limit remained. Let's just get rid of it.
While here, rearrange the logic about when to free the output string
so that it's not so dependent on unstated assumptions about the
possible values of attidentity and attgenerated.
Per bug #16743 from David Turon. Back-patch to v12 where GENERATED
came in. (Arguably we could take it back further, but I'm hesitant
to change the behavior of long-stable branches for this.)
Discussion: https://postgr.es/m/16743-7b1bacc4af76e7ad@postgresql.org
This is a variant of the normal Unix-domain sockets that don't use the
file system but a separate "abstract" namespace. At the user
interface, such sockets are represented by names starting with "@".
Supported on Linux and Windows right now.
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/flat/6dee8574-b0ad-fc49-9c8c-2edc796f0033@2ndquadrant.com
Similarly to 3636efa, the checks done in pg_dump when parsing array
values from catalogs have been too lax. Under memory pressure, it could
be possible, though very unlikely, to finish with dumps that miss some
data like:
- Statistics for indexes
- Run-time configuration of functions
- Configuration of extensions
- Publication list for a subscription
No backpatch is done as this is not going to be a problem in practice.
For example, if an OOM causes an array parsing to fail, a follow-up code
path of pg_dump would most likely complain with an allocation failure
due to the memory pressure.
Author: Michael Paquier
Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/20201111061319.GE2276@paquier.xyz
The test inserts a row in primary server, waits until the insertion has
been replicated to a cascaded standby, and checks that it's visible there
by querying the cascaded standby. In order for that to work reliably, the
test needs to wait until the insertion WAL record has been fully replayed.
This should fix the occasional buildfarm failures. Diagnosis by Tom Lane.
Discussion: https://www.postgresql.org/message-id/606796.1605424022@sss.pgh.pa.us
On Windows, a filename cannot contain backslashes, because a backslash
is used directory separator. Remove tests I added in commit 9c4f5192f
that tried to do that. We could perhaps use a SKIP block to only skip
them on Windows, but I'm not sure how exactly to formulate that, so just
remove the tests to make the buildfarm green again.
Per buildfarm.
Using a hot standby server as the source has not been possible, because
pg_rewind creates a temporary table in the source system, to hold the
list of file ranges that need to be fetched. Refactor it to queue up the
file fetch requests in pg_rewind's memory, so that the temporary table
is no longer needed.
Also update the logic to compute 'minRecoveryPoint' correctly, when the
source is a standby server.
Reviewed-by: Kyotaro Horiguchi, Soumyadeep Chakraborty
Discussion: https://www.postgresql.org/message-id/0c5b3783-af52-3ee5-f8fa-6e794061f70d%40iki.fi
It's entirely possible to see WAL for a relation that doesn't exist in
the target anymore. That happens when the relation was dropped later.
The refactoring in commit eb00f1d4b broke that case, by sanity-checking
the file type in the target before checking the flag forwhether it
exists there at all.
I noticed this during manual testing. Modify the 001_basic.pl test so
that it covers this case.
If an interactive psql session used \gset when querying a compromised
server, the attacker could execute arbitrary code as the operating
system account running psql. Using a prefix not found among specially
treated variables, e.g. every lowercase string, precluded the attack.
Fix by issuing a warning and setting no variable for the column in
question. Users wanting the old behavior can use a prefix and then a
meta-command like "\set HISTSIZE :prefix_HISTSIZE". Back-patch to 9.5
(all supported versions).
Reviewed by Robert Haas. Reported by Nick Cleaton.
Security: CVE-2020-25696
Coverity complained about a defect in commit 257836a7:
Calling "parsePGArray" without checking return value (as is
done elsewhere 11 out of 13 times).
Fix, and also check for empty strings explicitly (NULL as represented by
PQgetvalue()). That worked correctly before only because parsePGArray()
happens to set *nitems = 0 when it fails on an empty string. Also
convert a sanity check assertion to an error to be more paranoid, and
pgindent a nearby line.
Reported-by: Michael Paquier <michael@paquier.xyz>
This should have been done in the initial commit that made
unix_socket_directories a list as of c9b0cbe. This change allows to
support correctly the case of ALTER SYSTEM, where it is possible to
specify multiple paths as a list, like the following pattern where
flattening is applied to each item:
ALTER SYSTEM SET unix_socket_directories = '/path1', '/path2';
Any parameters specified in postgresql.conf are parsed the same way, so
there is no compatibility change. pg_dump has a hardcoded list of
parameters marked with GUC_LIST_QUOTE, that gets its routine update.
These are reordered alphabetically for clarity.
Author: Ian Lawrence Barwick
Reviewed-by: Peter Eisentraunt, Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/CAB8KJ=iMOtNY6_sUwV=LQVCJ2zgYHBDyNzVfvE5GN3WQ3v9kQg@mail.gmail.com
Revert 403a3d91c, as well as the followup fix 7f4235032, in all
branches. We need to think a bit harder about what the behavior
of LOCK TABLE on views should be, and there's no time for that
before next week's releases. We'll take another crack at this
later.
Discussion: https://postgr.es/m/16703-e348f58aab3cf6cc@postgresql.org
This makes the abstraction of a "source" server more clear, by introducing
a common abstract class, borrowing the object-oriented programming term,
that represents all the operations that can be done on the source server.
There are two implementations of it, one for fetching via libpq, and
another to fetch from a local directory. This adds some code, but makes it
easier to understand what's going on.
The copy_executeFileMap() and libpq_executeFileMap() functions contained
basically the same logic, just calling different functions to fetch the
source files. Refactor so that the common logic is in one place, in a new
function called perform_rewind().
Reviewed-by: Kyotaro Horiguchi, Soumyadeep Chakraborty
Discussion: https://www.postgresql.org/message-id/0c5b3783-af52-3ee5-f8fa-6e794061f70d%40iki.fi
Deciding what to do with each file is now a separate step after all the
necessary information has been gathered. It is more clear that way.
Previously, the decision-making was divided between process_source_file()
and process_target_file(), and it was a bit hard to piece together what
the overall rules were.
Reviewed-by: Kyotaro Horiguchi, Soumyadeep Chakraborty
Discussion: https://www.postgresql.org/message-id/0c5b3783-af52-3ee5-f8fa-6e794061f70d%40iki.fi
Record the current version of dependent collations in pg_depend when
creating or rebuilding an index. When accessing the index later, warn
that the index may be corrupted if the current version doesn't match.
Thanks to Douglas Doole, Peter Eisentraut, Christoph Berg, Laurenz Albe,
Michael Paquier, Robert Haas, Tom Lane and others for very helpful
discussion.
Author: Thomas Munro <thomas.munro@gmail.com>
Author: Julien Rouhaud <rjuju123@gmail.com>
Reviewed-by: Peter Eisentraut <peter.eisentraut@2ndquadrant.com> (earlier versions)
Discussion: https://postgr.es/m/CAEepm%3D0uEQCpfq_%2BLYFBdArCe4Ot98t1aR4eYiYTe%3DyavQygiQ%40mail.gmail.com
This model couldn't be extended to cover the default collation, and
didn't have any information about the affected database objects when the
version changed. Remove, in preparation for a follow-up commit that
will add a new mechanism.
Author: Thomas Munro <thomas.munro@gmail.com>
Reviewed-by: Julien Rouhaud <rjuju123@gmail.com>
Reviewed-by: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Discussion: https://postgr.es/m/CAEepm%3D0uEQCpfq_%2BLYFBdArCe4Ot98t1aR4eYiYTe%3DyavQygiQ%40mail.gmail.com
Although error results received from the backend should always have
a SQLSTATE field, ones generated by libpq won't, making this code
vulnerable to a crash after, say, untimely loss of connection.
Noted by Coverity.
Oversight in commit 403a3d91c. Back-patch to 9.5, as that was.
In almost all other places, we use plain "r" or "w" mode in popen()
calls (the exceptions being for COPY data). This one has been
overlooked (possibly because it's buried in a ".l" flex file?),
but it's using PG_BINARY_R.
Kensuke Okamura complained in bug #16688 that we fail to strip \r
when stripping the trailing newline from a backtick result string.
That's true enough, but we'd also fail to convert embedded \r\n
cleanly, which also seems undesirable. Fixing the popen() mode
seems like the best way to deal with this.
It's been like this for a long time, so back-patch to all supported
branches.
Discussion: https://postgr.es/m/16688-c649c7b69cd7e6f8@postgresql.org
Now that LOCK TABLE can take any relation type, acquire lock on all
relations that are to be dumped. This prevents schema changes or
deadlock errors that could cause a dump to fail after expending much
effort. The server is tested to have the capability and the feature
disabled if it doesn't, so that a patched pg_dump doesn't fail when
connecting to an unpatched server.
Backpatch to 9.5.
Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reported-by: Wells Oliver <wells.oliver@gmail.com>
Discussion: https://postgr.es/m/20201021200659.GA32358@alvherre.pgsql
Instead of immediately PQfinish'ing a dead connection, save it aside
so that we can still extract its parameters for \connect attempts.
(This works because PQconninfo doesn't care whether the PGconn is in
CONNECTION_BAD state.) This allows developers to reconnect with
just \c after a database crash and restart.
It's tempting to use the same approach instead of closing the old
connection after a failed non-interactive \connect command. However,
that would not be very safe: consider a script containing
\c db1 user1 live_server
\c db2 user2 dead_server
\c db3
The script would be expecting to connect to db3 at dead_server, but
if we re-use parameters from the first connection then it might
successfully connect to db3 at live_server. This'd defeat the goal
of not letting a script accidentally execute commands against the
wrong database.
Discussion: https://postgr.es/m/38464.1603394584@sss.pgh.pa.us
The check for whether to complain about not having an old connection
to get parameters from was seriously out of date: it had not been
rethought when we invented connstrings, nor when we invented the
-reuse-previous option. Replace it with a check that throws an
error if reuse-previous is active and we lack an old connection to
reuse. While that doesn't move the goalposts very far in terms of
easing reconnection after a server crash, at least it's consistent.
If the user specifies a connstring plus additional parameters
(which is invalid per the documentation), the extra parameters were
silently ignored. That seems like it could be really confusing,
so let's throw a syntax error instead.
Teach the connstring code path to re-use the old connection's password
in the same cases as the old-style-syntax code path would, ie if we
are reusing parameters and the values of username, host/hostaddr, and
port are not being changed. Document this behavior, too, since it was
unmentioned before. Also simplify the implementation a bit, giving
rise to two new and useful properties: if there's a "password=xxx" in
the connstring, we'll use it not ignore it, and by default (i.e.,
except with --no-password) we will prompt for a password if the
re-used password or connstring password doesn't work. The previous
code just failed if the re-used password didn't work.
Given the paucity of field complaints about these issues, I don't
think that they rise to the level of back-patchable bug fixes,
and in any case they might represent undesirable behavior changes
in minor releases. So no back-patch.
Discussion: https://postgr.es/m/235210.1603321144@sss.pgh.pa.us
psql's \connect claims to be able to re-use previous connection
parameters, but in fact it only re-uses the database name, user name,
host name (and possibly hostaddr, depending on version), and port.
This is problematic for assorted use cases. Notably, pg_dump[all]
emits "\connect databasename" commands which we would like to have
re-use all other parameters. If such a script is loaded in a psql run
that initially had "-d connstring" with some non-default parameters,
those other parameters would be lost, potentially causing connection
failure. (Thus, this is the same kind of bug addressed in commits
a45bc8a4f and 8e5793ab6, although the details are much different.)
To fix, redesign do_connect() so that it pulls out all properties
of the old PGconn using PQconninfo(), and then replaces individual
properties in that array. In the case where we don't wish to re-use
anything, get libpq's default settings using PQconndefaults() and
replace entries in that, so that we don't need different code paths
for the two cases.
This does result in an additional behavioral change for cases where
the original connection parameters allowed multiple hosts, say
"psql -h host1,host2", and the \connect request allows re-use of the
host setting. Because the previous coding relied on PQhost(), it
would only permit reconnection to the same host originally selected.
Although one can think of scenarios where that's a good thing, there
are others where it is not. Moreover, that behavior doesn't seem to
meet the principle of least surprise, nor was it documented; nor is
it even clear it was intended, since that coding long pre-dates the
addition of multi-host support to libpq. Hence, this patch is content
to drop it and re-use the host list as given.
Per Peter Eisentraut's comments on bug #16604. Back-patch to all
supported branches.
Discussion: https://postgr.es/m/16604-933f4b8791227b15@postgresql.org
After de8feb1f3a, some warnings remained
that were only visible when using GCC on Windows. Fix those as well.
Note that the ecpg test source files don't use the full pg_config.h,
so we can't use pg_funcptr_t there but have to do it the long way.
When told to process all databases, clusterdb, reindexdb, and vacuumdb
would reconnect by replacing their --maintenance-db parameter with the
name of the target database. If that parameter is a connstring (which
has been allowed for a long time, though we failed to document that
before this patch), we'd lose any other options it might specify, for
example SSL or GSS parameters, possibly resulting in failure to connect.
Thus, this is the same bug as commit a45bc8a4f fixed in pg_dump and
pg_restore. We can fix it in the same way, by using libpq's rules for
handling multiple "dbname" parameters to add the target database name
separately. I chose to apply the same refactoring approach as in that
patch, with a struct to handle the command line parameters that need to
be passed through to connectDatabase. (Maybe someday we can unify the
very similar functions here and in pg_dump/pg_restore.)
Per Peter Eisentraut's comments on bug #16604. Back-patch to all
supported branches.
Discussion: https://postgr.es/m/16604-933f4b8791227b15@postgresql.org
Failure to write data to a large object during restore led to an ugly
and uninformative error message. To add insult to injury, it then
fatal'd out, where other SQL-level errors usually result in pressing on.
Report the underlying error condition, rather than just giving not-very-
useful byte counts, and use warn_or_exit_horribly() so as to adhere to
pg_restore's general policy about whether to continue or not.
Also recognize that lo_write() returns int not size_t.
Per report from Justin Pryzby, though I didn't use his patch.
Given the lack of comparable complaints, I'm not sure this is
worth back-patching.
Discussion: https://postgr.es/m/20201018010232.GF9241@telsasoft.com
The Windows documentation insists that every WSAStartup call should
have a matching WSACleanup call. However, if that ever had actual
relevance, it wasn't in this century. Every remotely-modern Windows
kernel is capable of cleaning up when a process exits without doing
that, and must be so to avoid resource leaks in case of a process
crash. Moreover, Postgres backends have done WSAStartup without
WSACleanup since commit 4cdf51e64 in 2004, and we've never seen any
indication of a problem with that.
libpq's habit of doing WSAStartup during connection start and
WSACleanup during shutdown is also rather inefficient, since a
series of non-overlapping connection requests leads to repeated,
quite expensive DLL unload/reload cycles. We document a workaround
for that (having the application call WSAStartup for itself), but
that's just a kluge. It's also worth noting that it's far from
uncommon for applications to exit without doing PQfinish, and
we've not heard reports of trouble from that either.
However, the real reason for acting on this is that recent
experiments by Alexander Lakhin suggest that calling WSACleanup
during PQfinish might be triggering the symptom we occasionally see
that a process using libpq fails to emit expected stdio output.
Therefore, let's change libpq so that it calls WSAStartup only
once per process, during the first connection attempt, and never
calls WSACleanup at all.
While at it, get rid of the only other WSACleanup call in our code
tree, in pg_dump/parallel.c; that presumably is equally useless.
If this proves to suppress the fairly-common ecpg test failures
we see on Windows, I'll back-patch, but for now let's just do it
in HEAD and see what happens.
Discussion: https://postgr.es/m/ac976d8c-03df-d6b8-025c-15a2de8d9af1@postgrespro.ru
This commit required support for inline variable definition, which is
not a requirement.
RELEASE NOTE AUTHOR: the author of commit 3c0471b5fd
(pg_upgrade/tablespaces) was Justin Pryzby, not me.
Reported-by: Andres Freund
Discussion: https://postgr.es/m/20201016001959.h24fkywfubkv2pc5@alap3.anarazel.de
Backpatch-through: 9.5
Previously, if pg_upgrade failed, and the user recreated the cluster but
did not remove the new cluster tablespace directory, a later pg_upgrade
would fail since the new tablespace directory would already exists.
This adds error reporting for this during check.
Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/20200925005531.GJ23631@telsasoft.com
Backpatch-through: 9.5
A number of places were using appendStringInfo() when they could have been
using appendStringInfoString() instead. While there's no functionality
change there, it's just more efficient to use appendStringInfoString()
when no formatting is required. Likewise for some
appendStringInfoString() calls which were just appending a single char.
We can just use appendStringInfoChar() for that.
Additionally, many places were using appendPQExpBuffer() when they could
have used appendPQExpBufferStr(). Change those too.
Patch by Zhijie Hou, but further searching by me found significantly more
places that deserved the same treatment.
Author: Zhijie Hou, David Rowley
Discussion: https://postgr.es/m/cb172cf4361e4c7ba7167429070979d4@G08CNEXMBPEKD05.g08.fujitsu.local
Commit fe27009cb tried to make parallel.c's Windows implementation of
piperead() translate Windows socket errors to Unix, but that didn't
actually work because TranslateSocketError() is backend-internal code
(and not even public there). But on closer inspection, the sole
caller of this function doesn't actually care whether the result is
zero or negative, much less inspect the errno. So the whole exercise
is totally useless, and has been since this code was introduced.
Rip it out and just call recv() directly.
Per buildfarm.
Discussion: https://postgr.es/m/2621622.1602184554@sss.pgh.pa.us
Up to now, only ECONNRESET (and EPIPE, in most but not quite all places)
received special treatment in our error handling logic. This patch
changes things so that related error codes such as ECONNABORTED are
also recognized as indicating that the connection's dead and unlikely
to come back.
We continue to think, however, that only ECONNRESET and EPIPE should be
reported as probable server crashes; the other cases indicate network
connectivity problems but prove little about the server's state. Thus,
there's no change in the error message texts that are output for such
cases. The key practical effect is that errcode_for_socket_access()
will report ERRCODE_CONNECTION_FAILURE rather than
ERRCODE_INTERNAL_ERROR for a network failure. It's expected that this
will fix buildfarm member lorikeet's failures since commit 32a9c0bdf,
as that seems to be due to not treating ECONNABORTED equivalently to
ECONNRESET.
The set of errnos treated this way now includes ECONNABORTED, EHOSTDOWN,
EHOSTUNREACH, ENETDOWN, ENETRESET, and ENETUNREACH. Several of these
were second-class citizens in terms of their handling in places like
get_errno_symbol(), so upgrade the infrastructure where necessary.
As committed, this patch assumes that all these symbols are defined
everywhere. POSIX specifies all of them except EHOSTDOWN, but that
seems to exist on all platforms of interest; we'll see what the
buildfarm says about that.
Probably this should be back-patched, but let's see what the buildfarm
thinks of it first.
Fujii Masao and Tom Lane
Discussion: https://postgr.es/m/2621622.1602184554@sss.pgh.pa.us
Commit 3eb3d3e78 was a few bricks shy of a load: while it correctly
set the table's "interesting" flag when deciding to dump the data of
an extension config table, it was not correct to clear that flag
if we concluded we shouldn't dump the data. This led to the crash
reported in bug #16655, because in fact we'll traverse dumpTableSchema
anyway for all extension tables (to see if they have user-added
seclabels or RLS policies).
The right thing to do is to force "interesting" true in makeTableDataInfo,
and otherwise leave the flag alone. (Doing it there is more future-proof
in case additional calls are added, and it also avoids setting the flag
unnecessarily if that function decides the table is non-dumpable.)
This investigation also showed that while only the --inserts code path
had an obvious failure in the case considered by 3eb3d3e78, the COPY
code path also has a problem with not having loaded table subsidiary
data. That causes fmtCopyColumnList to silently return an empty string
instead of the correct column list. That accidentally mostly works,
which perhaps is why we didn't notice this before. It would only fail
if the restore column order is different from the dump column order,
which only happens in weird inheritance cases, so it's not surprising
nobody had hit the case with an extension config table. Nonetheless,
it's a bug, and it goes a long way back, not just to v12 where the
--inserts code path started to have a problem with this.
In hopes of catching such cases a bit sooner in future, add some
Asserts that "interesting" has been set in both dumpTableData and
dumpTableSchema. Adjust the test case added by 3eb3d3e78 so that it
checks the COPY rather than INSERT form of that bug, allowing it to
detect the longer-standing symptom.
Per bug #16655 from Cameron Daniel. Back-patch to all supported
branches.
Discussion: https://postgr.es/m/16655-5c92d6b3a9438137@postgresql.org
Discussion: https://postgr.es/m/18048b44-3414-b983-8c7c-9165b177900d@2ndQuadrant.com
The BKI file's string quoting conventions were previously quite weird,
perhaps as a result of repurposing a function built to scan
single-quoted strings to scan double-quoted ones. Change to use the
same rules as we use in GUC files, allowing some simplifications in
genbki.pl and initdb.c.
While at it, completely remove the backend's scanstr() function, which
was essentially a duplicate of the string dequoting code in guc-file.l.
Instead export that one (under a less generic name than it had) and let
bootscanner.l use it. Now we can clarify that scansup.c exists only to
support the main lexer. We could alternatively have removed GUC_scanstr,
but this way seems better since the previous arrangement could mislead
a reader into thinking that scanstr() had something to do with the main
lexer's handling of string literals. Maybe it did once, but if so it
was a long time ago.
This patch does not bump catversion, since the initially-installed
catalog contents don't change. Note however that successful initdb
after applying this patch will require up-to-date postgres.bki as well
as postgres and initdb executables.
In passing, remove a bunch of very-long-obsolete #include's in
bootparse.y and bootscanner.l.
John Naylor
Discussion: https://postgr.es/m/CACPNZCtDpd18T0KATTmCggO2GdVC4ow86ypiq5ENff1VnauL8g@mail.gmail.com
Both tools never had safeguard checks for the options provided, and it
was possible to make pg_test_fsync run an infinite amount of time or
pass down buggy values to pg_test_timing.
These behaviors have existed for a long time, with no actual complaints,
so no backpatch is done. Basic TAP tests are introduced for both tools.
Author: Michael Paquier
Reviewed-by: Peter Eisentraut
Discussion: https://postgr.es/m/20200806062759.GE16470@paquier.xyz
Parallel pg_dump failed if its -d parameter was a connection string
containing any essential information other than host, port, or username.
The same was true for pg_restore with --create.
The reason is that these scenarios failed to preserve the connection
string from the command line; the code felt free to replace that with
just the database name when reconnecting from a pg_dump parallel worker
or after creating the target database. By chance, parallel pg_restore
did not suffer this defect, as long as you didn't say --create.
In practice it seems that the error would be obvious only if the
connstring included essential, non-default SSL or GSS parameters.
This may explain why it took us so long to notice. (It also makes
it very difficult to craft a regression test case illustrating the
problem, since the test would fail in builds without those options.)
Fix by refactoring so that ConnectDatabase always receives all the
relevant options directly from the command line, rather than
reconstructed values. Inject a different database name, when necessary,
by relying on libpq's rules for handling multiple "dbname" parameters.
While here, let's get rid of the essentially duplicate _connectDB
function, as well as some obsolete nearby cruft.
Per bug #16604 from Zsolt Ero. Back-patch to all supported branches.
Discussion: https://postgr.es/m/16604-933f4b8791227b15@postgresql.org
Existing code used various inconsistent ways to printf struct stat's
st_size member. The type of that is off_t, which is in most cases a
signed 64-bit integer, so use the long long int format for it.
pg_restore previously coped with overlength TOC-file lines using some
complicated logic to ignore additional bufferloads. While this isn't
wrong, since we don't expect that the interesting part of a line would
run to more than a dozen or so bytes, it's more complex than it needs
to be. Use a StringInfo instead of a fixed-size buffer so that we can
process long lines as single entities and thus not need the extra
logic.
Daniel Gustafsson
Discussion: https://postgr.es/m/48A4FA71-524E-41B9-953A-FD04EF36E2E7@yesql.se
Further experience says that the appending behavior offered by
pg_get_line_append is useful to only a very small minority of callers.
For most, the requirement to reset the buffer after each line is just
an error-prone nuisance. Hence, invent another alternative call
pg_get_line_buf, which takes care of that detail.
Noted while reviewing a patch from Daniel Gustafsson.
Discussion: https://postgr.es/m/48A4FA71-524E-41B9-953A-FD04EF36E2E7@yesql.se
This feature has been a thorn in our sides for a long time, causing
many grammatical ambiguity problems. It doesn't seem worth the
pain to continue to support it, so remove it.
There are some follow-on improvements we can make in the grammar,
but this commit only removes the bare minimum number of productions,
plus assorted backend support code.
Note that pg_dump and psql continue to have full support, since
they may be used against older servers. However, pg_dump warns
about postfix operators. There is also a check in pg_upgrade.
Documentation-wise, I (tgl) largely removed the "left unary"
terminology in favor of saying "prefix operator", which is
a more standard and IMO less confusing term.
I included a catversion bump, although no initial catalog data
changes here, to mark the boundary at which oprkind = 'r'
stopped being valid in pg_operator.
Mark Dilger, based on work by myself and Robert Haas;
review by John Naylor
Discussion: https://postgr.es/m/38ca86db-42ab-9b48-2902-337a0d6b8311@2ndquadrant.com
Instead of hard-wiring specific verbosity levels into the option
processing of client applications, invent pg_logging_increase_verbosity()
and encourage clients to implement --verbose by calling that. Then,
the common convention that more -v's gets you more verbosity just works.
In particular, this allows resurrection of the debug-grade messages that
have long existed in pg_dump and its siblings. They were unreachable
before this commit due to lack of a way to select PG_LOG_DEBUG logging
level. (It appears that they may have been unreachable for some time
before common/logging.c was introduced, too, so I'm not specifically
blaming cc8d41511 for the oversight. One reason for thinking that is
that it's now apparent that _allocAH()'s message needs a null-pointer
guard. Testing might have failed to reveal that before 96bf88d52.)
Discussion: https://postgr.es/m/1173106.1600116625@sss.pgh.pa.us
It is not possible to get a list of foreign schemas as the server is not
known, so this provides instead a list of local schemas, which is more
useful than nothing if using a loopback server or having schema names
matching in the local and remote servers.
Author: Jeff Janes
Reviewed-by: Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/CAMkU=1wr7Roj41q-XiJs=Uyc2xCmHhcGGy7J-peJQK-e+w=ghw@mail.gmail.com
Adjust the whitespace in the emitted files so that it matches
what pgindent would do. This makes the generated files look
like they match project style, and avoids confusion if someone
does run pgindent on the generated files.
Also, add probes.h to pgindent's exclusion list, because it can
confuse pgindent, plus there's not much point in processing it.
Daniel Gustafsson, additional fixes by me
Discussion: https://postgr.es/m/79ed5348-be7a-b647-dd40-742207186a22@2ndquadrant.com
Those extra queries are not necessary when doing a data-only dump. With
this change, this means that the dependencies between CHECK/DEFAULT and
the parent table are not tracked anymore for a data-only dump. However,
these dependencies are only used for the schema generation and we have
never guaranteed that a dump can be reloaded if a CHECK constraint uses
a custom function whose behavior changes when loading the data, like
when using cross-table references in the CHECK function.
Author: Julien Rouhaud
Reviewed-by: Daniel Gustafsson, Michael Paquier
Discussion: https://postgr.es/m/20200712054850.GA92357@nol
If there are no objects of a certain type, there is no need to do an
allocation for a set of DumpableObject items. The previous coding did
an allocation of 1 byte instead as per the fallback of pg_malloc() in
the event of an allocation size of zero. This assigns NULL instead for
a set of dumpable objects.
A similar rule already applied to findObjectByOid(), so this makes the
code more defensive as we would just fail with a pointer dereference
instead of attempting to use some incorrect data if a non-existing,
positive, OID is given by a caller of this function.
Author: Daniel Gustafsson
Reviewed-by: Julien Rouhaud, Ranier Vilela
Discussion: https://postgr.es/m/26C43E58-BDD0-4F1A-97CC-4A07B52E32C5@yesql.se
The stats target can be set since commit d06215d03, but wasn't shown by
psql.
Author: Justin Pryzby <justin@telsasoft.com>
Discussion: https://postgr.es/m/20200831050047.GG5450@telsasoft.com
Reviewed-by: Georgios Kokolatos <gkokolatos@protonmail.com>
Reviewed-by: Tatsuro Yamada <tatsuro.yamada.tf@nttcom.co.jp>
Peter Eisentraut
Robert Haas
Michael Paquier
Andrew Dunstan
Andres Freund
Tom Lane
Jeff Davis
Fujii Masao
Thomas Munro
Bruce Momjian
Alvaro Herrera
Amit Kapila
Tomas Vondra
Magnus Hagander
Noah Misch
Daniel Gustafsson
Peter Geoghegan
Alexander Korotkov
Tatsuo Ishii
Heikki Linnakangas
David Rowley
Dean Rasheed