Skip to content

P
Postgres FD Implementation
Switch branch/tag
  1. 28 Aug, 2021 2 commits
    • Alvaro Herrera's avatar
      psql \dP: reference regclass with "pg_catalog." prefix · a32860b8
      Alvaro Herrera authored 
      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 1c5d9270, so backpatch to 12.

Author: Justin Pryzby <pryzbyj@telsasoft.com>
Discussion: https://postgr.es/m/20210827193151.GN26465@telsasoft.com
      a32860b8
    • Noah Misch's avatar
      Fix data loss in wal_level=minimal crash recovery of CREATE TABLESPACE. · 5513c09c
      Noah Misch authored 
      If the system crashed between CREATE TABLESPACE and the next checkpoint,
the result could be some files in the tablespace unexpectedly containing
no rows.  Affected files would be those for which the system did not
write WAL; see the wal_skip_threshold documentation.  Before v13, a
different set of conditions governed the writing of WAL; see v12's
<sect2 id="populate-pitr">.  (The v12 conditions were broader in some
ways and narrower in others.)  Users may want to audit non-default
tablespaces for unexpected short files.  The bug could have truncated an
index without affecting the associated table, and reindexing the index
would fix that particular problem.

This fixes the bug by making create_tablespace_directories() more like
TablespaceCreateDbspace().  create_tablespace_directories() was
recursively removing tablespace contents, reasoning that WAL redo would
recreate everything removed that way.  That assumption holds for other
wal_level values.  Under wal_level=minimal, the old approach could
delete files for which no other copy existed.  Back-patch to 9.6 (all
supported versions).

Reviewed by Robert Haas and Prabhat Sahu.  Reported by Robert Haas.

Discussion: https://postgr.es/m/CA+TgmoaLO9ncuwvr2nN-J4VEP5XyAcy=zKiHxQzBbFRxxGxm0w@mail.gmail.com
      5513c09c
  3. 27 Aug, 2021 8 commits
  5. 25 Aug, 2021 9 commits
  7. 24 Aug, 2021 2 commits
    • Tom Lane's avatar
      Fix regexp misbehavior with capturing parens inside "{0}". · 244dd799
      Tom Lane authored 
      Regexps like "(.){0}...\1" drew an "invalid backreference number".
That's not unreasonable on its face, since the capture group will
never be matched if it's iterated zero times.  However, other engines
such as Perl's don't complain about this, nor do we throw an error for
related cases such as "(.)|\1", even though that backref can never
succeed either.  Also, if the zero-iterations case happens at runtime
rather than compile time --- say, "(x)*...\1" when there's no "x" to
be found --- that's not an error, we just deem the backref to not
match.  Making this even less defensible, no error was thrown for
nested cases such as "((.)){0}...\2"; and to add insult to injury,
those cases could result in assertion failures instead.  (It seems
that nothing especially bad happened in non-assert builds, though.)

Let's just fix it so that no error is thrown and instead the backref
is deemed to never match, so that compile-time detection of no
iterations behaves the same as run-time detection.

Per report from Mark Dilger.  This appears to be an aboriginal error
in Spencer's library, so back-patch to all supported versions.

Pre-v14, it turns out to also be necessary to back-patch one aspect of
commits cb76fbd7e/00116dee5, namely to create capture-node subREs with
the begin/end states of their subexpressions, not the current lp/rp
of the outer parseqatom invocation.  Otherwise delsub complains that
we're trying to disconnect a state from itself.  This is a bit scary
but code examination shows that it's safe: in the pre-v14 code, if we
want to wrap iteration around the subexpression, the first thing we do
is overwrite the atom's begin/end fields with new states.  So the
bogus values didn't survive long enough to be used for anything, except
if no iteration is required, in which case it doesn't matter.

Discussion: https://postgr.es/m/A099E4A8-4377-4C64-A98C-3DEDDC075502@enterprisedb.com
      244dd799
    • Amit Kapila's avatar
      Fix Alter Subscription's Add/Drop Publication behavior. · 5cfcd46e
      Amit Kapila authored 
      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
      5cfcd46e
  9. 23 Aug, 2021 3 commits
    • Tom Lane's avatar
      Prevent regexp back-refs from sometimes matching when they shouldn't. · 779557bd
      Tom Lane authored 
      The recursion in cdissect() was careless about clearing match data
for capturing parentheses after rejecting a partial match.  This
could allow a later back-reference to succeed when by rights it
should fail for lack of a defined referent.

To fix, think a little more rigorously about what the contract
between different levels of cdissect's recursion needs to be.
With the right spec, we can fix this using fewer rather than more
resets of the match data; the key decision being that a failed
sub-match is now explicitly responsible for clearing any matches
it may have set.

There are enough other cross-checks and optimizations in the code
that it's not especially easy to exhibit this problem; usually, the
match will fail as-expected.  Plus, regexps that are even potentially
vulnerable are most likely user errors, since there's just not much
point in writing a back-ref that doesn't always have a referent.
These facts perhaps explain why the issue hasn't been detected,
even though it's almost certainly a couple of decades old.

Discussion: https://postgr.es/m/151435.1629733387@sss.pgh.pa.us
      779557bd
    • Alvaro Herrera's avatar
      Avoid creating archive status ".ready" files too early · e3fb6170
      Alvaro Herrera authored 
      WAL records may span multiple segments, but XLogWrite() does not
wait for the entire record to be written out to disk before
creating archive status files.  Instead, as soon as the last WAL page of
the segment is written, the archive status file is created, and the
archiver may process it.  If PostgreSQL crashes before it is able to
write and flush the rest of the record (in the next WAL segment), the
wrong version of the first segment file lingers in the archive, which
causes operations such as point-in-time restores to fail.

To fix this, keep track of records that span across segments and ensure
that segments are only marked ready-for-archival once such records have
been completely written to disk.

This has always been wrong, so backpatch all the way back.

Author: Nathan Bossart <bossartn@amazon.com>
Reviewed-by: default avatarKyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-by: default avatarRyo Matsumura <matsumura.ryo@fujitsu.com>
Reviewed-by: default avatarAndrey Borodin <x4mmm@yandex-team.ru>
Discussion: https://postgr.es/m/CBDDFA01-6E40-46BB-9F98-9340F4379505@amazon.com
      e3fb6170
    • Michael Paquier's avatar
      Fix backup manifests to generate correct WAL-Ranges across timelines · 65b649fe
      Michael Paquier authored 
      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
      65b649fe
  11. 21 Aug, 2021 1 commit
    • Tom Lane's avatar
      Improve error messages about misuse of SELECT INTO. · 8f6a5219
      Tom Lane authored 
      Improve two places in plpgsql, and one in spi.c, where an error
message would confusingly tell you that you couldn't use a SELECT
query, when what you had written *was* a SELECT query.  The actual
problem is that you can't use SELECT ... INTO in these contexts,
but the messages failed to make that apparent.  Special-case
SELECT INTO to make these errors more helpful.

Also, fix the same spots in plpgsql, as well as several messages
in exec_eval_expr(), to not quote the entire complained-of query or
expression in the primary error message.  That behavior very easily
led to violating our message style guideline about keeping the primary
error message short and single-line.  Also, since the important part
of the message was after the inserted text, it could make the real
problem very hard to see.  We can report the query or expression as
the first line of errcontext instead.

Per complaint from Roger Mason.  Back-patch to v14, since (a) some
of these messages are new in v14 and (b) v14's translatable strings
are still somewhat in flux.  The problem's older than that of
course, but I'm hesitant to change the behavior further back.

Discussion: https://postgr.es/m/1914708.1629474624@sss.pgh.pa.us
      8f6a5219
  13. 20 Aug, 2021 3 commits
  15. 19 Aug, 2021 4 commits
    • Tom Lane's avatar
      Avoid trying to lock OLD/NEW in a rule with FOR UPDATE. · 46490039
      Tom Lane authored 
      transformLockingClause neglected to exclude the pseudo-RTEs for
OLD/NEW when processing a rule's query.  This led to odd errors
or even crashes later on.  This bug is very ancient, but it's
not terribly surprising that nobody noticed, since the use-case
for SELECT FOR UPDATE in a non-view rule is somewhere between
thin and non-existent.  Still, crashing is not OK.

Per bug #17151 from Zhiyong Wu.  Thanks to Masahiko Sawada
for analysis of the problem.

Discussion: https://postgr.es/m/17151-c03a3e6e4ec9aadb@postgresql.org
      46490039
    • Andres Freund's avatar
      Unset MyBEEntry, making elog.c's call to pgstat_get_my_query_id() safe. · 18914f24
      Andres Freund authored 
      Previously log messages late during shutdown could end up using either another
backend's PgBackendStatus (multi user) or segfault (single user) because
pgstat_get_my_query_id()'s check for !MyBEEntry didn't filter out use after
pgstat_beshutdown_hook().

This became a bug in 4f0b0966, but was a bit fishy before. But given
there's no known problematic cases before 14, it doesn't seem worth
backpatching further.

Also fixes a wrong filename in a comment, introduced in e1025044.
Reported-By: default avatarAndres Freund <andres@anarazel.de>
Reviewed-By: default avatarJulien Rouhaud <rjuju123@gmail.com>
Discussion: https://postgr.es/m/Julien Rouhaud <rjuju123@gmail.com>
Backpatch: 14-
      18914f24
    • Amit Kapila's avatar
      Fix typo in protocol.sgml. · e1915646
      Amit Kapila authored 
      The 'Stream Stop' message is misspelled as 'Stream End' in the docs.

Author: Masahiko Sawada
Backpatch-through: 14, where it was introduced
Discussion: https://postgr.es/m/CAD21AoDeScrsHhLyEPYqN3sydg6PxAPVBboK=30xJfUVihNZDA@mail.gmail.com
      e1915646
    • Michael Paquier's avatar
      Revert refactoring of hex code to src/common/ · 1900c140
      Michael Paquier authored 
      This is a combined revert of the following commits:
- c3826f83, a refactoring piece that moved the hex decoding code to
src/common/.  This code was cleaned up by aef8948f, as it originally
included no overflow checks in the same way as the base64 routines in
src/common/ used by SCRAM, making it unsafe for its purpose.
- aef8948f, a more advanced refactoring of the hex encoding/decoding code
to src/common/ that added sanity checks on the result buffer for hex
decoding and encoding.  As reported by Hans Buschmann, those overflow
checks are expensive, and it is possible to see a performance drop in
the decoding/encoding of bytea or LOs the longer they are.  Simple SQLs
working on large bytea values show a clear difference in perf profile.
- ccf4e277, a cleanup made possible by aef8948f.

The reverts of all those commits bring back the performance of hex
decoding and encoding back to what it was in ~13.  Fow now and
post-beta3, this is the simplest option.

Reported-by: Hans Buschmann
Discussion: https://postgr.es/m/1629039545467.80333@nidsa.net
Backpatch-through: 14
      1900c140
  17. 18 Aug, 2021 2 commits
    • Tom Lane's avatar
      Fix check_agg_arguments' examination of aggregate FILTER clauses. · 61f9dae2
      Tom Lane authored 
      Recursion into the FILTER clause was mis-implemented, such that a
relevant Var or Aggref at the very top of the FILTER clause would
be ignored.  (Of course, that'd have to be a plain boolean Var or
boolean-returning aggregate.)  The consequence would be
mis-identification of the correct semantic level of the aggregate,
which could lead to not-per-spec query behavior.  If the FILTER
expression is an aggregate, this could also lead to failure to issue
an expected "aggregate function calls cannot be nested" error, which
would likely result in a core dump later on, since the planner and
executor aren't expecting such cases to appear.

The root cause is that commit b560ec1b blindly copied some code
that assumed it's recursing into a List, and thus didn't examine the
top-level node.  To forestall questions about why this call doesn't
look like the others, as well as possible future copy-and-paste
mistakes, let's change all three check_agg_arguments_walker calls in
check_agg_arguments, even though only the one for the filter clause
is really broken.

Per bug #17152 from Zhiyong Wu.  This has been wrong since we
implemented FILTER, so back-patch to all supported versions.
(Testing suggests that pre-v11 branches manage to avoid crashing
in the bad-Aggref case, thanks to "redundant" checks in ExecInitAgg.
But I'm not sure how thorough that protection is, and anyway the
wrong-behavior issue remains, so fix 9.6 and 10 too.)

Discussion: https://postgr.es/m/17152-c7f906cc1a88e61b@postgresql.org
      61f9dae2
    • Daniel Gustafsson's avatar
      Fix pg_amcheck --skip option parameter handling · 5310c61e
      Daniel Gustafsson authored 
      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: default avatarChen Jiaoqian <chenjq.jy@fujitsu.com>
Reviewed-by: default avatarMasahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/17149-5918ea748da36b15@postgresql.org
Backpatch-through: 14
      5310c61e
  19. 17 Aug, 2021 4 commits
    • Tom Lane's avatar
      Prevent ALTER TYPE/DOMAIN/OPERATOR from changing extension membership. · 8f51ee63
      Tom Lane authored 
      If recordDependencyOnCurrentExtension is invoked on a pre-existing,
free-standing object during an extension update script, that object
will become owned by the extension.  In our current code this is
possible in three cases:

* Replacing a "shell" type or operator.
* CREATE OR REPLACE overwriting an existing object.
* ALTER TYPE SET, ALTER DOMAIN SET, and ALTER OPERATOR SET.

The first of these cases is intentional behavior, as noted by the
existing comments for GenerateTypeDependencies.  It seems like
appropriate behavior for CREATE OR REPLACE too; at least, the obvious
alternatives are not better.  However, the fact that it happens during
ALTER is an artifact of trying to share code (GenerateTypeDependencies
and makeOperatorDependencies) between the CREATE and ALTER cases.
Since an extension script would be unlikely to ALTER an object that
didn't already belong to the extension, this behavior is not very
troubling for the direct target object ... but ALTER TYPE SET will
recurse to dependent domains, and it is very uncool for those to
become owned by the extension if they were not already.

Let's fix this by redefining the ALTER cases to never change extension
membership, full stop.  We could minimize the behavioral change by
only changing the behavior when ALTER TYPE SET is recursing to a
domain, but that would complicate the code and it does not seem like
a better definition.

Per bug #17144 from Alex Kozhemyakin.  Back-patch to v13 where ALTER
TYPE SET was added.  (The other cases are older, but since they only
affect the directly-named object, there's not enough of a problem to
justify changing the behavior further back.)

Discussion: https://postgr.es/m/17144-e67d7a8f049de9af@postgresql.org
      8f51ee63
    • Michael Meskes's avatar
      Improved ECPG warning as suggested by Michael Paquier and removed test case · e2d6da07
      Michael Meskes authored 
      that triggers the warning during regression tests.
      e2d6da07
    • Daniel Gustafsson's avatar
      Set type identifier on BIO · b88377ad
      Daniel Gustafsson authored 
      In OpenSSL there are two types of BIO's (I/O abstractions):
source/sink and filters. A source/sink BIO is a source and/or
sink of data, ie one acting on a socket or a file. A filter
BIO takes a stream of input from another BIO and transforms it.
In order for BIO_find_type() to be able to traverse the chain
of BIO's and correctly find all BIO's of a certain type they
shall have the type bit set accordingly, source/sink BIO's
(what PostgreSQL implements) use BIO_TYPE_SOURCE_SINK and
filter BIO's use BIO_TYPE_FILTER. In addition to these, file
descriptor based BIO's should have the descriptor bit set,
BIO_TYPE_DESCRIPTOR.

The PostgreSQL implementation didn't set the type bits, which
went unnoticed for a long time as it's only really relevant
for code auditing the OpenSSL installation, or doing similar
tasks. It is required by the API though, so this fixes it.

Backpatch through 9.6 as this has been wrong for a long time.

Author: Itamar Gafni
Discussion: https://postgr.es/m/SN6PR06MB39665EC10C34BB20956AE4578AF39@SN6PR06MB3966.namprd06.prod.outlook.com
Backpatch-through: 9.6
      b88377ad
    • Heikki Linnakangas's avatar
      doc: \123 and \x12 escapes in COPY are in database encoding. · f90c0595
      Heikki Linnakangas authored 
      The backslash sequences, including \123 and \x12 escapes, are interpreted
after encoding conversion. The docs failed to mention that.

Backpatch to all supported versions.

Reported-by: Andreas Grob
Discussion: https://www.postgresql.org/message-id/17142-9181542ca1df75ab%40postgresql.org
      f90c0595
  21. 16 Aug, 2021 2 commits
    • Alvaro Herrera's avatar
      Revert analyze support for partitioned tables · b3d24cc0
      Alvaro Herrera authored 
      This reverts the following commits:
1b5617eb Describe (auto-)analyze behavior for partitioned tables
0e69f705 Set pg_class.reltuples for partitioned tables
41badeab Document ANALYZE storage parameters for partitioned tables
0827e8af autovacuum: handle analyze for partitioned tables

There are efficiency issues in this code when handling databases with
large numbers of partitions, and it doesn't look like there isn't any
trivial way to handle those.  There are some other issues as well.  It's
now too late in the cycle for nontrivial fixes, so we'll have to let
Postgres 14 users continue to manually deal with ANALYZE their
partitioned tables, and hopefully we can fix the issues for Postgres 15.

I kept [most of] be280cda ("Don't reset relhasindex for partitioned
tables on ANALYZE") because while we added it due to 0827e8af, it is
a good bugfix in its own right, since it affects manual analyze as well
as autovacuum-induced analyze, and there's no reason to revert it.

I retained the addition of relkind 'p' to tables included by
pg_stat_user_tables, because reverting that would require a catversion
bump.
Also, in pg14 only, I keep a struct member that was added to
PgStat_TabStatEntry to avoid breaking compatibility with existing stat
files.

Backpatch to 14.

Discussion: https://postgr.es/m/20210722205458.f2bug3z6qzxzpx2s@alap3.anarazel.de
      b3d24cc0
    • Michael Paquier's avatar
      Refresh apply delay on reload of recovery_min_apply_delay at recovery · f83d80ea
      Michael Paquier authored 
      This commit ensures that the wait interval in the replay delay loop
waiting for an amount of time defined by recovery_min_apply_delay is
correctly handled on reload, recalculating the delay if this GUC value
is updated, based on the timestamp of the commit record being replayed.

The previous behavior would be problematic for example with replay
still waiting even if the delay got reduced or just cancelled.  If the
apply delay was increased to a larger value, the wait would have just
respected the old value set, finishing earlier.

Author: Soumyadeep Chakraborty, Ashwin Agrawal
Reviewed-by: Kyotaro Horiguchi, Michael Paquier
Discussion: https://postgr.es/m/CAE-ML+93zfr-HLN8OuxF0BjpWJ17O5dv1eMvSE5jsj9jpnAXZA@mail.gmail.com
Backpatch-through: 9.6
      f83d80ea