Skip to content

P
Postgres FD Implementation
Switch branch/tag
  1. 24 Jul, 2018 3 commits
  3. 23 Jul, 2018 8 commits
  5. 22 Jul, 2018 2 commits
  7. 21 Jul, 2018 2 commits
    • Tom Lane's avatar
      Further portability hacking in pg_upgrade's test script. · 04269320
      Tom Lane authored 
      I blew the dust off a Bourne shell (file date 1996, yea verily) and
tried to run test.sh with it.  It mostly worked, but I found that the
temp-directory creation code introduced by commit be76a6d3 was not
compatible, for a couple of reasons: this shell thinks "set -e" should
force an exit if a command within backticks fails, and it also thinks code
within braces should be executed by a sub-shell, meaning that variable
settings don't propagate back up to the parent shell.  In view of Victor
Wagner's report that Solaris is still using pre-POSIX shells, seems like
we oughta make this case work.  It's not like the code is any less
idiomatic this way; the prior coding technique appeared nowhere else.

(There is a remaining bash-ism here, which is that $RANDOM doesn't do
what the code hopes in non-bash shells.  But the use of $$ elsewhere in
that path should be enough to ensure uniqueness and some amount of
randomness, so I think it's okay as-is.)

Back-patch to all supported branches, as the previous commit was.

Discussion: https://postgr.es/m/20180720153820.69e9ae6c@fafnir.local.vm
      04269320
    • Tom Lane's avatar
      Be more paranoid about quoting in pg_upgrade's test script. · 0c0908d9
      Tom Lane authored 
      Double-quote $PGDATA in "find" commands introduced by commit da9b580d,
in case that path contains spaces or other special characters.

Adjust a few other places so that quoting is done more consistently.
None of the others are actual bugs AFAICS, but it's confusing to readers
if the same thing is done differently in different places.

Noted by Tels.

Discussion: https://postgr.es/m/c96303c04c360bbedaa04f90f515745b.squirrel@sm.webmail.pair.com
      0c0908d9
  9. 20 Jul, 2018 3 commits
  11. 19 Jul, 2018 12 commits
  13. 18 Jul, 2018 6 commits
    • Tom Lane's avatar
      Remove race-prone hot_standby_feedback test cases in 001_stream_rep.pl. · a360f952
      Tom Lane authored 
      This script supposed that if it turned hot_standby_feedback on and then
shut down the standby server, at least one feedback message would be
guaranteed to be sent before the standby stops.  But there is no such
guarantee, if the standby's walreceiver process is slow enough --- and
we've seen multiple failures in the buildfarm showing that that does
happen in practice.  While we could rearrange the walreceiver logic to
make it less likely, it seems probably impossible to create a really
bulletproof guarantee of that sort; and if we tried, we might create
situations where the walreceiver wouldn't react in a timely manner to
shutdown commands.  It seems better instead to remove the script's
assumption that feedback will occur before shutdown.

But once we do that, these last few tests seem quite redundant with
the earlier tests in the script.  So let's just drop them altogether
and save some buildfarm cycles.

Backpatch to v10 where these tests were added.

Discussion: https://postgr.es/m/1922.1531592205@sss.pgh.pa.us
      a360f952
    • Tom Lane's avatar
      Drop the rule against included index columns duplicating key columns. · 701fd0bb
      Tom Lane authored 
      The initial version of the included-index-column feature stated that
included columns couldn't be the same as any key column of the index.
While it'd be pretty silly to do that, since the included column would be
entirely redundant, we've never prohibited redundant index columns before
so it's not very consistent to do so here.  Moreover, the prohibition
was itself badly implemented, so that it failed to reject columns that
were effectively identical but not spelled quite alike, as reported by
Aditya Toshniwal.

(Moreover, it's not hard to imagine that for some non-btree index types,
such cases would be non-silly anyhow: the index might use a lossy
representation for key columns but be able to support retrieval of the
original form of included columns.)

Hence, let's just drop the prohibition.

In passing, do some copy-editing on the documentation for the
included-column feature.

Yugo Nagata; documentation and test corrections by me

Discussion: https://postgr.es/m/CAM9w-_mhBCys4fQNfaiQKTRrVWtoFrZ-wXmDuE9Nj5y-Y7aDKQ@mail.gmail.com
      701fd0bb
    • Tom Lane's avatar
      Use a ResourceOwner to track buffer pins in all cases. · 3cb64626
      Tom Lane authored 
      Historically, we've allowed auxiliary processes to take buffer pins without
tracking them in a ResourceOwner.  However, that creates problems for error
recovery.  In particular, we've seen multiple reports of assertion crashes
in the startup process when it gets an error while holding a buffer pin,
as for example if it gets ENOSPC during a write.  In a non-assert build,
the process would simply exit without releasing the pin at all.  We've
gotten away with that so far just because a failure exit of the startup
process translates to a database crash anyhow; but any similar behavior
in other aux processes could result in stuck pins and subsequent problems
in vacuum.

To improve this, institute a policy that we must *always* have a resowner
backing any attempt to pin a buffer, which we can enforce just by removing
the previous special-case code in resowner.c.  Add infrastructure to make
it easy to create a process-lifespan AuxProcessResourceOwner and clear
out its contents at appropriate times.  Replace existing ad-hoc resowner
management in bgwriter.c and other aux processes with that.  (Thus, while
the startup process gains a resowner where it had none at all before, some
other aux process types are replacing an ad-hoc resowner with this code.)
Also use the AuxProcessResourceOwner to manage buffer pins taken during
StartupXLOG and ShutdownXLOG, even when those are being run in a bootstrap
process or a standalone backend rather than a true auxiliary process.

In passing, remove some other ad-hoc resource owner creations that had
gotten cargo-culted into various other places.  As far as I can tell
that was all unnecessary, and if it had been necessary it was incomplete,
due to lacking any provision for clearing those resowners later.
(Also worth noting in this connection is that a process that hasn't called
InitBufferPoolBackend has no business accessing buffers; so there's more
to do than just add the resowner if we want to touch buffers in processes
not covered by this patch.)

Although this fixes a very old bug, no back-patch, because there's no
evidence of any significant problem in non-assert builds.

Patch by me, pursuant to a report from Justin Pryzby.  Thanks to
Robert Haas and Kyotaro Horiguchi for reviews.

Discussion: https://postgr.es/m/20180627233939.GA10276@telsasoft.com
      3cb64626
    • Heikki Linnakangas's avatar
      Fix misc typos, mostly in comments. · 6b387179
      Heikki Linnakangas authored 
      A collection of typos I happened to spot while reading code, as well as
grepping for common mistakes.

Backpatch to all supported versions, as applicable, to avoid conflicts
when backporting other commits in the future.
      6b387179
    • Michael Paquier's avatar
      Fix more portability issues with casts to Size when using off_t · 94019c87
      Michael Paquier authored 
      This should tame the beast, as there are no other places where off_t is
used in the new error messages.

Reported again by longfin, which complained about walsender.c while I
spotted the other two ones while double-checking.
      94019c87
    • Michael Paquier's avatar
      Fix casting in error message for two-phase file · 8bd064f0
      Michael Paquier authored 
      This error from from 811b6e36, which causes compilation warnings with OSX
10.3 and clang.

Reported by Tom Lane, via buildfarm member longfin.
      8bd064f0
  15. 17 Jul, 2018 4 commits
    • Michael Paquier's avatar
      Rework error messages around file handling · 811b6e36
      Michael Paquier authored 
      Some error messages related to file handling are using the code path
context to define their state.  For example, 2PC-related errors are
referring to "two-phase status files", or "relation mapping file" is
used for catalog-to-filenode mapping, however those prove to be
difficult to translate, and are not more helpful than just referring to
the path of the file being worked on.  So simplify all those error
messages by just referring to files with their path used.  In some
cases, like the manipulation of WAL segments, the context is actually
helpful so those are kept.

Calls to the system function read() have also been rather inconsistent
with their error handling sometimes not reporting the number of bytes
read, and some other code paths trying to use an errno which has not
been set.  The in-core functions are using a more consistent pattern
with this patch, which checks for both errno if set or if an
inconsistent read is happening.

So as to care about pluralization when reading an unexpected number of
byte(s), "could not read: read %d of %zu" is used as error message, with
%d field being the output result of read() and %zu the expected size.
This simplifies the work of translators with less variations of the same
message.

Author: Michael Paquier
Reviewed-by: Álvaro Herrera
Discussion: https://postgr.es/m/20180520000522.GB1603@paquier.xyz
      811b6e36
    • Alvaro Herrera's avatar
      doc: move PARTITION OF stanza to just below PARTITION BY · c6736ff7
      Alvaro Herrera authored 
      It's more logical this way, since the new ordering matches the way the
tables are created; but in any case, the previous location of PARTITION OF
did not appear carefully chosen anyway (since it didn't match the
location in which it appears in the synopsys either, which is what we
normally do.)

In the PARTITION BY stanza, add a link to the partitioning section in
the DDL chapter, too.

Suggested-by: David G. Johnston
Discussion: https://postgr.es/m/CAKFQuwY4Ld7ecxL_KAmaxwt0FUu5VcPPN2L4dh+3BeYbrdBa5g@mail.gmail.com
      c6736ff7
    • Alvaro Herrera's avatar
      Revise BuildIndexValueDescription to simplify it · 1c04d4be
      Alvaro Herrera authored 
      Getting a pg_index tuple from syscache when the open index relation is
available is pointless -- just use the one from relcache.

Noticed while reviewing code for cb9db2ab.

No backpatch.
      1c04d4be
    • Alvaro Herrera's avatar
      Fix ALTER TABLE...SET STATS error message for included columns · cb9db2ab
      Alvaro Herrera authored 
      The existing error message was complaining that the column is not an
expression, which is not correct.  Introduce a suitable wording
variation and a test.
Co-authored-by: default avatarYugo Nagata <nagata@sraoss.co.jp>
Discussion: https://postgr.es/m/20180628182803.e4632d5a.nagata@sraoss.co.jpReviewed-by: default avatarÁlvaro Herrera <alvherre@alvh.no-ip.org>
      cb9db2ab