Skip to content

P
Postgres FD Implementation
Switch branch/tag
  1. 27 Aug, 2018 1 commit
    • Michael Paquier's avatar
      Improve VACUUM and ANALYZE by avoiding early lock queue · a556549d
      Michael Paquier authored 
      A caller of VACUUM can perform early lookup obtention which can cause
other sessions to block on the request done, causing potentially DOS
attacks as even a non-privileged user can attempt a vacuum fill of a
critical catalog table to block even all incoming connection attempts.

Contrary to TRUNCATE, a client could attempt a system-wide VACUUM after
building the list of relations to VACUUM, which can cause vacuum_rel()
or analyze_rel() to try to lock the relation but the operation would
just block.  When the client specifies a list of relations and the
relation needs to be skipped, ownership checks are done when building
the list of relations to work on, preventing a later lock attempt.

vacuum_rel() already had the sanity checks needed, except that those
were applied too late.  This commit refactors the code so as relation
skips are checked beforehand, making it safer to avoid too early locks,
for both manual VACUUM with and without a list of relations specified.

An isolation test is added emulating the fact that early locks do not
happen anymore, issuing a WARNING message earlier if the user calling
VACUUM is not a relation owner.

When a partitioned table is listed in a manual VACUUM or ANALYZE
command, its full list of partitions is fetched, all partitions get
added to the list to work on, and then each one of them is processed one
by one, with ownership checks happening at the later phase of
vacuum_rel() or analyze_rel().  Trying to do early ownership checks for
each partition is proving to be tedious as this would result in deadlock
risks with lock upgrades, and skipping all partitions if the listed
partitioned table is not owned would result in a behavior change
compared to how Postgres 10 has implemented vacuum for partitioned
tables.  The original problem reported related to early lock queue for
critical relations is fixed anyway, so priority is given to avoiding a
backward-incompatible behavior.

Reported-by: Lloyd Albin, Jeremy Schneider
Author: Michael Paquier
Reviewed by: Nathan Bossart, Kyotaro Horiguchi
Discussion: https://postgr.es/m/152512087100.19803.12733865831237526317@wrigleys.postgresql.org
Discussion: https://postgr.es/m/20180812222142.GA6097@paquier.xyz
      a556549d
  3. 26 Aug, 2018 3 commits
    • Thomas Munro's avatar
      Fix typos. · 18e58674
      Thomas Munro authored 
      Author: David Rowley
Discussion: https://postgr.es/m/CAKJS1f8du35u5DprpykWvgNEScxapbWYJdHq%2Bz06Wj3Y2KFPbw%40mail.gmail.com
      18e58674
    • Tom Lane's avatar
      Make syslogger more robust against failures in opening CSV log files. · bff84a54
      Tom Lane authored 
      The previous coding figured it'd be good enough to postpone opening
the first CSV log file until we got a message we needed to write there.
This is unsafe, though, because if the open fails we end up in infinite
recursion trying to report the failure.  Instead make the CSV log file
management code look as nearly as possible like the longstanding logic
for the stderr log file.  In particular, open it immediately at postmaster
startup (if enabled), or when we get a SIGHUP in which we find that
log_destination has been changed to enable CSV logging.

It seems OK to fail if a postmaster-start-time open attempt fails, as
we've long done for the stderr log file.  But we can't die if we fail
to open a CSV log file during SIGHUP, so we're still left with a problem.
In that case, write any output meant for the CSV log file to the stderr
log file.  (This will also cover race-condition cases in which backends
send CSV log data before or after we have the CSV log file open.)

This patch also fixes an ancient oversight that, if CSV logging was
turned off during a SIGHUP, we never actually closed the last CSV
log file.

In passing, remember to reset whereToSendOutput = DestNone during syslogger
start, since (unlike all other postmaster children) it's forked before the
postmaster has done that.  This made for a platform-dependent difference
in error reporting behavior between the syslogger and other children:
except on Windows, it'd report problems to the original postmaster stderr
as well as the normal error log file(s).  It's barely possible that that
was intentional at some point; but it doesn't seem likely to be desirable
in production, and the platform dependency definitely isn't desirable.

Per report from Alexander Kukushkin.  It's been like this for a long time,
so back-patch to all supported branches.

Discussion: https://postgr.es/m/CAFh8B==iLUD_gqC-dAENS0V+kVrCeGiKujtKqSQ7++S-caaChw@mail.gmail.com
      bff84a54
    • Jeff Davis's avatar
      Reconsider new file extension in commit 91f26d5f. · ba9d35b8
      Jeff Davis authored 
      Andres and Tom objected to the choice of the ".tmp"
extension. Changing to Andres's suggestion of ".spill".

Discussion: https://postgr.es/m/88092095-3348-49D8-8746-EB574B1D30EA%40anarazel.de
      ba9d35b8
  5. 25 Aug, 2018 6 commits
  7. 24 Aug, 2018 7 commits
  9. 23 Aug, 2018 7 commits
    • Andres Freund's avatar
      Deduplicate code between slot_getallattrs() and slot_getsomeattrs(). · 88ebd62f
      Andres Freund authored 
      Code in slot_getallattrs() is the same as if slot_getsomeattrs() is
called with number of attributes specified in the tuple
descriptor. Implement it that way instead of duplicating the code
between those two functions.

This is part of a patchseries abstracting TupleTableSlots so they can
store arbitrary forms of tuples, but is a nice enough cleanup on its
own.

Author: Ashutosh Bapat
Reviewed-By: Andres Freund
Discussion: https://postgr.es/m/20180220224318.gw4oe5jadhpmcdnm@alap3.anarazel.de
      88ebd62f
    • Andrew Gierth's avatar
      Fix lexing of standard multi-character operators in edge cases. · a40631a9
      Andrew Gierth authored 
      Commits c6b3c939 (which fixed the precedence of >=, <=, <> operators)
and 865f14a2 (which added support for the standard => notation for
named arguments) created a class of lexer tokens which look like
multi-character operators but which have their own token IDs distinct
from Op. However, longest-match rules meant that following any of
these tokens with another operator character, as in (1<>-1), would
cause them to be incorrectly returned as Op.

The error here isn't immediately obvious, because the parser would
usually still find the correct operator via the Op token, but there
were more subtle problems:

1. If immediately followed by a comment or +-, >= <= <> would be given
   the old precedence of Op rather than the correct new precedence;

2. If followed by a comment, != would be returned as Op rather than as
   NOT_EQUAL, causing it not to be found at all;

3. If followed by a comment or +-, the => token for named arguments
   would be lexed as Op, causing the argument to be mis-parsed as a
   simple expression, usually causing an error.

Fix by explicitly checking for the operators in the {operator} code
block in addition to all the existing special cases there.

Backpatch to 9.5 where the problem was introduced.

Analysis and patch by me; review by Tom Lane.
Discussion: https://postgr.es/m/87va851ppl.fsf@news-spur.riddles.org.uk
      a40631a9
    • Andrew Gierth's avatar
      Reduce an unnecessary O(N^3) loop in lexer. · d4a63f82
      Andrew Gierth authored 
      The lexer's handling of operators contained an O(N^3) hazard when
dealing with long strings of + or - characters; it seems hard to
prevent this case from being O(N^2), but the additional N multiplier
was not needed.

Backpatch all the way since this has been there since 7.x, and it
presents at least a mild hazard in that trying to do Bind, PREPARE or
EXPLAIN on a hostile query could take excessive time (without
honouring cancels or timeouts) even if the query was never executed.
      d4a63f82
    • Tom Lane's avatar
      In libpq, don't look up all the hostnames at once. · 5ca00774
      Tom Lane authored 
      Historically, we looked up the target hostname in connectDBStart, so that
PQconnectPoll did not need to do DNS name resolution.  The patches that
added multiple-target-host support to libpq preserved this division of
labor; but it's really nonsensical now, because it means that if any one
of the target hosts fails to resolve in DNS, the connection fails.  That
negates the no-single-point-of-failure goal of the feature.  Additionally,
DNS lookups aren't exactly cheap, but the code did them all even if the
first connection attempt succeeds.

Hence, rearrange so that PQconnectPoll does the lookups, and only looks
up a hostname when it's time to try that host.  This does mean that
PQconnectPoll could block on a DNS lookup --- but if you wanted to avoid
that, you should be using hostaddr, as the documentation has always
specified.  It seems fairly unlikely that any applications would really
care whether the lookup occurs inside PQconnectStart or PQconnectPoll.

In addition to calling out that fact explicitly, do some other minor
wordsmithing in the docs around the multiple-target-host feature.

Since this seems like a bug in the multiple-target-host feature,
backpatch to v10 where that was introduced.  In the back branches,
avoid moving any existing fields of struct pg_conn, just in case
any third-party code is looking into that struct.

Tom Lane, reviewed by Fabien Coelho

Discussion: https://postgr.es/m/4913.1533827102@sss.pgh.pa.us
      5ca00774
    • Peter Eisentraut's avatar
      Copy-editing of pg_verify_checksums help and ref page · 2d41d914
      Peter Eisentraut authored 
      Reformat synopsis, put options into better order, make the desciption
line a bit shorter, and put more details into the description.
      2d41d914
    • Peter Eisentraut's avatar
      PL/pgSQL: Extend test case · d2cc897b
      Peter Eisentraut authored 
      This test was supposed to check the interaction of INOUT and default
parameters in a procedure call, but it only checked the case where the
parameter was not supplied.  Now it also checks the case where the
parameter was supplied.  It was already working correctly, so no code
changes required.
      d2cc897b
    • Alvaro Herrera's avatar
      Return type of txid_status is text, not txid_status · d10f7741
      Alvaro Herrera authored 
      Thinko in commit 857ee8e3.

Discovered-by: Gianni Ciolli
      d10f7741
  11. 22 Aug, 2018 8 commits
  13. 21 Aug, 2018 5 commits
  15. 20 Aug, 2018 1 commit
  17. 19 Aug, 2018 2 commits