Previously, invoking pg_terminate_backend() or pg_cancel_backend()
with the postmaster PID produced a "PID XXXX is not a PostgresSQL
server process" warning, which does not make sense. Change to
"backend process" to make the message more exact.
Nathan Bossart, based on an idea from Bharath Rupireddy with
input from Tom Lane and Euler Taveira
Discussion: https://www.postgresql.org/message-id/flat/CALj2ACW7Rr-R7mBcBQiXWPp=JV5chajjTdudLiF5YcpW-BmHhg@mail.gmail.com
Experimenting with FIPS mode enabled, I saw
regression=# \password joe
Enter new password for user "joe":
Enter it again:
could not encrypt password: disabled for FIPS
out of memory
because PQencryptPasswordConn was still of the opinion that "out of
memory" is always appropriate to print.
Minor oversight in b69aba745. Like that one, back-patch to v14.
Previously pg_log_backend_memory_contexts() could request to
log the memory contexts of backends, but not of auxiliary processes
such as checkpointer. This commit enhances the function so that
it can also send the request to auxiliary processes. It's useful to
look at the memory contexts of those processes for debugging purpose
and better understanding of the memory usage pattern of them.
Note that pg_log_backend_memory_contexts() cannot send the request
to logger or statistics collector. Because this logging request
mechanism is based on shared memory but those processes aren't
connected to that.
Author: Bharath Rupireddy
Reviewed-by: Vignesh C, Kyotaro Horiguchi, Fujii Masao
Discussion: https://postgr.es/m/CALj2ACU1nBzpacOK2q=a65S_4+Oaz_rLTsU1Ri0gf7YUmnmhfQ@mail.gmail.com
The existing cryptohash facility was causing problems in some code paths
related to MD5 (frontend and backend) that relied on the fact that the
only type of error that could happen would be an OOM, as the MD5
implementation used in PostgreSQL ~13 (the in-core implementation is
used when compiling with or without OpenSSL in those older versions),
could fail only under this circumstance.
The new cryptohash facilities can fail for reasons other than OOMs, like
attempting MD5 when FIPS is enabled (upstream OpenSSL allows that up to
1.0.2, Fedora and Photon patch OpenSSL 1.1.1 to allow that), so this
would cause incorrect reports to show up.
This commit extends the cryptohash APIs so as callers of those routines
can fetch more context when an error happens, by using a new routine
called pg_cryptohash_error(). The error states are stored within each
implementation's internal context data, so as it is possible to extend
the logic depending on what's suited for an implementation. The default
implementation requires few error states, but OpenSSL could report
various issues depending on its internal state so more is needed in
cryptohash_openssl.c, and the code is shaped so as we are always able to
grab the necessary information.
The core code is changed to adapt to the new error routine, painting
more "const" across the call stack where the static errors are stored,
particularly in authentication code paths on variables that provide
log details. This way, any future changes would warn if attempting to
free these strings. The MD5 authentication code was also a bit blurry
about the handling of "logdetail" (LOG sent to the postmaster), so
improve the comments related that, while on it.
The origin of the problem is 87ae969, that introduced the centralized
cryptohash facility. Extra changes are done for pgcrypto in v14 for the
non-OpenSSL code path to cope with the improvements done by this
commit.
Reported-by: Michael Mühlbeyer
Author: Michael Paquier
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/89B7F072-5BBE-4C92-903E-D83E865D9367@trivadis.com
Backpatch-through: 14
I had a brain fade in commit d32899157, and used 2:30AM as the
example timestamp for both spring-forward and fall-back cases.
But it's not actually ambiguous at all in the fall-back case,
because that transition is from 2AM to 1AM under USA rules.
Fix the example to use 1:30AM, which *is* ambiguous.
Noted while answering a question from Aleksander Alekseev.
Back-patch to all supported branches.
Discussion: https://postgr.es/m/2191355.1641828552@sss.pgh.pa.us
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
Since this function is defined to accept pg_node_tree values, it could
get applied to any nodetree that can appear in a cataloged pg_node_tree
column. Some such cases can't be supported --- for example, its API
doesn't allow providing referents for more than one relation --- but
we should try to throw a user-facing error rather than an internal
error when encountering such a case.
In support of this, extend expression_tree_walker/mutator to be sure
they'll work on any such node tree (which basically means adding
support for relpartbound node types). That allows us to run pull_varnos
and check for the case of multiple relations before we start processing
the tree. The alternative of changing the low-level error thrown for an
out-of-range varno isn't appealing, because that could mask actual bugs
in other usages of ruleutils.
Per report from Justin Pryzby. This is basically cosmetic, so no
back-patch.
Discussion: https://postgr.es/m/20211219205422.GT17618@telsasoft.com
If contrib/btree_gist is used to make a GIST index on a char(N)
(bpchar) column, and that column is retrieved via an index-only
scan, what came out had all trailing spaces removed. Since
that doesn't happen in any other kind of table scan, this is
clearly a bug. The cause is that gbt_bpchar_compress() strips
trailing spaces (using rtrim1) before a new index entry is made.
That was probably a good idea when this code was first written,
but since we invented index-only scans, it's not so good.
One answer could be to mark this opclass as incapable of index-only
scans. But to do so, we'd need an extension module version bump,
followed by manual action by DBAs to install the updated version
of btree_gist. And it's not really a desirable place to end up,
anyway.
Instead, let's fix the code by removing the unwanted space-stripping
action and adjusting the opclass's comparison logic to ignore
trailing spaces as bpchar normally does. This will not hinder
cases that work today, since index searches with this logic will
act the same whether trailing spaces are stored or not. It will
not by itself fix the problem of getting space-stripped results
from index-only scans, of course. Users who care about that can
REINDEX affected indexes after installing this update, to immediately
replace all improperly-truncated index entries. Otherwise, it can
be expected that the index's behavior will change incrementally as
old entries are replaced by new ones.
Per report from Alexander Lakhin. Back-patch to all supported branches.
Discussion: https://postgr.es/m/696c995b-b37f-5526-f45d-04abe713179f@gmail.com
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
Prevent logical replication workers from performing insert, update,
delete, truncate, or copy commands on tables unless the subscription
owner has permission to do so.
Prevent subscription owners from circumventing row-level security by
forbidding replication into tables with row-level security policies
which the subscription owner is subject to, without regard to whether
the policy would ordinarily allow the INSERT, UPDATE, DELETE or
TRUNCATE which is being replicated. This seems sufficient for now, as
superusers, roles with bypassrls, and target table owners should still
be able to replicate despite RLS policies. We can revisit the
question of applying row-level security policies on a per-row basis if
this restriction proves too severe in practice.
Author: Mark Dilger
Reviewed-by: Jeff Davis, Andrew Dunstan, Ronan Dunklau
Discussion: https://postgr.es/m/9DFC88D3-1300-4DE8-ACBC-4CEF84399A53%40enterprisedb.com
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
Instead of using a hardcoded or default path to the perl file the .bat
file is a wrapper for, we use a path that means the file is found in
the same directory as the .bat file.
Patch by Anton Voloshin, slightly tweaked by me.
Backpatch to all live branches
Discussion: https://postgr.es/m/2b7a674b-5fb0-d264-75ef-ecc7a31e54f8@postgrespro.ru
Commit db7d1a7b05 missed a couple of places that needed adjustment now
we are not building pgcrypto when openssl is not configured, causing
contrib installcheck to fail in such a case.
Commit 449ab63505 added postgres_fdw.application_name GUC that specifies
a value for application_name configuration parameter used when postgres_fdw
establishes a connection to a foreign server. Also commit 6e0cb3dec1
allowed it to include escape sequences. Both commits added the regression
tests for the GUC, but those tests were reverted by commits 98dbef90eb and
5e64ad3697 because they were unstable and caused some buildfarm members
to report the failure.
This is the third try to add the regression test for
postgres_fdw.application_name GUC.
One of issues to make the test unstable was to have used
postgres_fdw_disconnect_all() to close the existing remote connections.
The test expected that the remote connection and its corresponding backend
at the remote server disappeared just after postgres_fdw_disconnect_all()
was executed, but it could take a bit time for them to disappear.
To make sure that they exit, this commit makes the test use
pg_terminate_backend() with the timeout at the remote server, instead.
If the timeout is set to greater than zero, this function waits until
they are actually terminated (or until the given time has passed).
Another issue was that the test didn't take into consideration the case
where postgres_fdw.application_name containing some escape sequences was
converted to the string larger than NAMEDATALEN. In this case it was
truncated to less than NAMEDATALEN when it's passed to the remote server,
but the test expected wrongly that full string of application_name was
always viewable. This commit changes the test so that it can handle that case.
Author: Fujii Masao
Reviewed-by: Masahiko Sawada, Hayato Kuroda, Kyotaro Horiguchi
Discussion: https://postgr.es/m/3220909.1631054766@sss.pgh.pa.us
Discussion: https://postgr.es/m/20211224.180006.2247635208768233073.horikyota.ntt@gmail.com
Discussion: https://postgr.es/m/e7b61420-a97b-8246-77c4-a0d48fba5a45@oss.nttdata.com
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
We disallow altering a column datatype within a regular table,
if the table's rowtype is used as a column type elsewhere,
because we lack code to go around and rewrite the other tables.
This restriction should apply to partitioned tables as well, but it
was not checked because ATRewriteTables and ATPrepAlterColumnType
were not on the same page about who should do it for which relkinds.
Per bug #17351 from Alexander Lakhin. Back-patch to all supported
branches.
Discussion: https://postgr.es/m/17351-6db1870f3f4f612a@postgresql.org
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
The link used in the documentation is dead, and the only options to have
an access to this part of the SQL specification are not free. Like any
other books referred, just remove the link to keep some neutrality but
keep its reference.
Reported-by: Erik Rijkers
Discussion: https://postgr.es/m/989abd7d-af30-ab52-1201-bf0b4f33b872@xs4all.nl
Backpatch-through: 12
The code-coverage report says that this test doesn't increase
coverage by one single line, which I now realize is because
I made src/test/modules/test_regex/sql/test_regex_utf8.sql
to cover all the code that this would. So really it's pointless
and we should just drop it.
Up to now this has just sat there as a test you could invoke via
EXTRA_TESTS, which of course nobody does. I'm feeling encouraged
because c2e8bd275 hasn't yet broke anything, so let's try making this
run with a suitable guard condition (similar to collate.linux.utf8).
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
These test cases have been commented out since citext was invented,
because at the time we had no nice way to deal with tests that
have restrictions such as requiring UTF8 encoding. But now we do
have a convention for that, ie put them into a separate test file
with an early-exit path. So let's enable these tests to run when
their prerequisites are satisfied.
(We may have to tighten the prerequisites beyond the "encoding = UTF8
and locale != C" checks made here. But let's put it on the buildfarm
and see what blows up.)
Dag Lem
Discussion: https://postgr.es/m/ygezgoacs4e.fsf_-_@sid.nimrod.no
get_rel_sync_entry(), which is called each time a change needs to be
logically replicated, is a rather hot code path in the WAL sender
sending logical changes. This code path was doing a relcache access on
relkind and relpartition for each logical change, but we only need to
know this information when building or re-building the cached
information for a relation.
Some measurements prove that this is noticeable in perf profiles,
particularly when attempting to replicate changes from relations that
are not published as these cause less overhead in the WAL sender,
delaying further the replication of changes for relations that are
published.
Issue introduced in 83fd453.
Author: Hou Zhijie
Reviewed-by: Kyotaro Horiguchi, Euler Taveira
Discussion: https://postgr.es/m/OS0PR01MB5716E863AA9E591C1F010F7A947D9@OS0PR01MB5716.jpnprd01.prod.outlook.com
Backpatch-through: 13
Under concurrency, it is possible for two sessions to be merrily locking
and releasing a tuple and marking it again as HEAP_XMAX_INVALID all the
while a third session attempts to lock it, miserably fails at it, and
then contemplates life, the universe and everything only to eventually
fail an assertion that said bit is not set. Before SKIP LOCKED that was
indeed a reasonable expectation, but alas! commit df630b0dd5 falsified
it.
This bug is as old as time itself, and even older, if you think time
begins with the oldest supported branch. Therefore, backpatch to all
supported branches.
Author: Simon Riggs <simon.riggs@enterprisedb.com>
Discussion: https://postgr.es/m/CANbhV-FeEwMnN8yuMyss7if1ZKjOKfjcgqB26n8pqu1e=q0ebg@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
We can revert the code changes of commit b5febc1d1 now, because
commit 9a3ddeb51 installed a real solution for the difficulty
that b5febc1d1 just dodged, namely that the planner might pick
the wrong one of several index columns nominally containing the
same value. It only matters which one we pick if we pick one
that's not returnable, and that mistake is now foreclosed.
Although both of the aforementioned commits were back-patched,
I don't feel a need to take any risk by back-patching this one.
The cases that it improves are very corner-ish.
Discussion: https://postgr.es/m/3179992.1641150853@sss.pgh.pa.us
Commit 4ace45677 failed to fix the problem fully, because the
same issue of attempting to fetch a non-returnable index column
can occur when rechecking the indexqual after using a lossy index
operator. Moreover, it broke EXPLAIN for such indexquals (which
indicates a gap in our test cases :-().
Revert the code changes of 4ace45677 in favor of adding a new field
to struct IndexOnlyScan, containing a version of the indexqual that
can be executed against the index-returned tuple without using any
non-returnable columns. (The restrictions imposed by check_index_only
guarantee this is possible, although we may have to recompute indexed
expressions.) Support construction of that during setrefs.c
processing by marking IndexOnlyScan.indextlist entries as resjunk
if they can't be returned, rather than removing them entirely.
(We could alternatively require setrefs.c to look up the IndexOptInfo
again, but abusing resjunk this way seems like a reasonably safe way
to avoid needing to do that.)
This solution isn't great from an API-stability standpoint: if there
are any extensions out there that build IndexOnlyScan structs directly,
they'll be broken in the next minor releases. However, only a very
invasive extension would be likely to do such a thing. There's no
change in the Path representation, so typical planner extensions
shouldn't have a problem.
As before, back-patch to all supported branches.
Discussion: https://postgr.es/m/3179992.1641150853@sss.pgh.pa.us
Discussion: https://postgr.es/m/17350-b5bdcf476e5badbb@postgresql.org
Adjust the error texts used for unrecognized/unsupported datetime
units so that there are just two strings to translate, not two
per datatype. Along the way, follow our usual error message style
of not double-quoting type names, and instead making sure that we
say the name is a type. Fix a couple of places in date.c that
were using the wrong one of "unrecognized" and "unsupported".
Nikhil Benesch, with a bit more editing by me
Discussion: https://postgr.es/m/CAPWqQZTURGixmbMH2_Z3ZtWGA0ANjUb9bwtkkxSxSfDeFHuM6Q@mail.gmail.com
Since 4f0b096, pgss_store() does nothing if compute_query_id is disabled
or if no other module computed a query identifier, but the top comment
of this function did not reflect that. This behavior is already
documented in its own code path, and this just removes the inconsistent
comment.
Author: Kyotaro Horiguchi
Reviewed-by: Julien Rouhaud
Discussion: https://postgr.es/m/20211122.153823.1325120762360533122.horikyota.ntt@gmail.com
Backpatch-through: 14
If an index has both returnable and non-returnable columns, and one of
the non-returnable columns is an expression using a Var that is in a
returnable column, then a query returning that expression could result
in an index-only scan plan that attempts to read the non-returnable
column, instead of recomputing the expression from the returnable
column as intended.
To fix, redefine the "indextlist" list of an IndexOnlyScan plan node
as containing null Consts in place of any non-returnable columns.
This solves the problem by preventing setrefs.c from falsely matching
to such entries. The executor is happy since it only cares about the
exposed types of the entries, and ruleutils.c doesn't care because a
correct plan won't reference those entries. I considered some other
ways to prevent setrefs.c from doing the wrong thing, but this way
seems good since (a) it allows a very localized fix, (b) it makes
the indextlist structure more compact in many cases, and (c) the
indextlist is now a more faithful representation of what the index AM
will actually produce, viz. nulls for any non-returnable columns.
This is easier to hit since we introduced included columns, but it's
possible to construct failing examples without that, as per the
added regression test. Hence, back-patch to all supported branches.
Per bug #17350 from Louis Jachiet.
Discussion: https://postgr.es/m/17350-b5bdcf476e5badbb@postgresql.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
Currently FreeBSD, Linux, macOS and Windows (Visual Studio) are tested.
The main goal of this integration is to make it easier to test in-development
patches across multiple platforms. This includes improving the testing done
automatically by cfbot [1] for commitfest entries. It is *not* the goal to
supersede the buildfarm.
cirrus-ci [2] was chosen because it was already in use for cfbot, allows using
full VMs, has good OS coverage and allows accessing the full test results
without authentication (like a github account). It might be worth adding
support for further CI providers, particularly ones supporting other git
forges, in the future.
To keep CI times tolerable, most platforms use pre-generated images. Some
platforms use containers, others use full VMs.
For instructions on how to enable the CI integration in a repository and
further details, see src/tools/ci/README
[1] http://cfbot.cputube.org/
[2] https://cirrus-ci.org/
Author: Andres Freund <andres@anarazel.de>
Author: Thomas Munro <tmunro@postgresql.org>
Author: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-By: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-By: Justin Pryzby <pryzby@telsasoft.com>
Reviewed-By: Thomas Munro <tmunro@postgresql.org>
Reviewed-By: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Discussion: https://postgr.es/m/20211001222752.wrz7erzh4cajvgp6@alap3.anarazel.de
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.
John Naylor
Tom Lane
Fujii Masao
Amit Kapila
Michael Paquier
Andrew Dunstan
Thomas Munro
Peter Eisentraut
Jeff Davis
Bruce Momjian
Alvaro Herrera
Magnus Hagander
Andres Freund