1. 21 Jul, 2020 9 commits
    • Tom Lane's avatar
      neqjoinsel must now pass through collation to eqjoinsel. · bd0d893a
      Tom Lane authored
      Since commit 044c99bc, eqjoinsel passes the passed-in collation
      to any operators it invokes.  However, neqjoinsel failed to pass
      on whatever collation it got, so that if we invoked a
      collation-dependent operator via that code path, we'd get "could not
      determine which collation to use for string comparison" or the like.
      
      Per report from Justin Pryzby.  Back-patch to v12, like the previous
      commit.
      
      Discussion: https://postgr.es/m/20200721191606.GL5748@telsasoft.com
      bd0d893a
    • Peter Geoghegan's avatar
      Add nbtree Valgrind buffer lock checks. · 4a70f829
      Peter Geoghegan authored
      Holding just a buffer pin (with no buffer lock) on an nbtree buffer/page
      provides very weak guarantees, especially compared to heapam, where it's
      often safe to read a page while only holding a buffer pin.  This commit
      has Valgrind enforce the following rule: it is never okay to access an
      nbtree buffer without holding both a pin and a lock on the buffer.
      
      A draft version of this patch detected questionable code that was
      cleaned up by commits fa7ff642 and 7154aa16.  The code in question used
      to access an nbtree buffer page's special/opaque area with no buffer
      lock (only a buffer pin).  This practice (which isn't obviously unsafe)
      is hereby formally disallowed in nbtree.  There doesn't seem to be any
      reason to allow it, and banning it keeps things simple for Valgrind.
      
      The new checks are implemented by adding custom nbtree client requests
      (located in LockBuffer() wrapper functions); these requests are
      "superimposed" on top of the generic bufmgr.c Valgrind client requests
      added by commit 1e0dfd16.  No custom resource management cleanup code is
      needed to undo the effects of marking buffers as non-accessible under
      this scheme.
      
      Author: Peter Geoghegan
      Reviewed-By: Anastasia Lubennikova, Georgios Kokolatos
      Discussion: https://postgr.es/m/CAH2-WzkLgyN3zBvRZ1pkNJThC=xi_0gpWRUb_45eexLH1+k2_Q@mail.gmail.com
      4a70f829
    • Tom Lane's avatar
      Weaken type-OID-matching checks in array_recv and record_recv. · 670c0a1d
      Tom Lane authored
      Rather than always insisting on an exact match of the type OID in the
      data to the element type or column type we expect, complain only when
      both OIDs fall within the manually-assigned range.  This acknowledges
      the reality that user-defined types don't have stable OIDs, while
      still preserving some of the mistake-detection value of the old test.
      
      (It's not entirely clear whether to error if one OID is manually
      assigned and the other isn't.  But perhaps that case could arise in
      cross-version cases where a former extension type has been imported
      into core, so I let it pass.)
      
      This change allows us to remove the prohibition on binary transfer
      of user-defined arrays and composites in the recently-landed support
      for binary logical replication (commit 9de77b54).  We can just
      unconditionally drop that check, since if the client has asked for
      binary transfer it must be >= v14 and must have this change.
      
      Discussion: https://postgr.es/m/CADK3HH+R3xMn=8t3Ct+uD+qJ1KD=Hbif5NFMJ+d5DkoCzp6Vgw@mail.gmail.com
      670c0a1d
    • Alvaro Herrera's avatar
      Glossary: Add term "base backup" · 606c3845
      Alvaro Herrera authored
      Author: Jürgen Purtz <juergen@purtz.de>
      Discussion: https://postgr.es/m/95f90a5d-7692-701d-2c0c-0c88eb5cea7d@purtz.de
      606c3845
    • Alvaro Herrera's avatar
      Minor glossary tweaks · a0b2d583
      Alvaro Herrera authored
      Add "(process)" qualifier to two terms, remove self-reference in one
      term.
      
      Author: Jürgen Purtz <juergen@purtz.de>
      Discussion: https://postgr.es/m/95f90a5d-7692-701d-2c0c-0c88eb5cea7d@purtz.de
      a0b2d583
    • Tom Lane's avatar
      Be more careful about marking catalog columns NOT NULL by default. · fc032bed
      Tom Lane authored
      The bug fixed in commit 72eab84a would not have occurred if initdb
      had a less surprising rule about which columns should be marked
      NOT NULL by default.  Let's make that rule be strictly that the
      column must be fixed-width and its predecessors must be fixed-width
      and NOT NULL, removing the hacky and unsafe exceptions for oidvector
      and int2vector.
      
      Since we do still want all existing oidvector and int2vector columns
      to be marked NOT NULL, we have to put BKI_FORCE_NOT_NULL labels on
      them.  But making this less magic and more documented seems like a
      good idea, even if it's a shade more verbose.
      
      I didn't bump catversion since the initial catalog contents are
      not actually changed by this patch.  Note however that the
      contents of postgres.bki do change, and feeding an old copy of
      that to a new backend will produce wrong results.
      
      Discussion: https://postgr.es/m/204760.1595181800@sss.pgh.pa.us
      fc032bed
    • Tom Lane's avatar
      Assert that we don't insert nulls into attnotnull catalog columns. · 3e66019f
      Tom Lane authored
      The executor checks for this error, and so does the bootstrap catalog
      loader, but we never checked for it in retail catalog manipulations.
      The folly of that has now been exposed, so let's add assertions
      checking it.  Checking in CatalogTupleInsert[WithInfo] and
      CatalogTupleUpdate[WithInfo] should be enough to cover this.
      
      Back-patch to v10; the aforesaid functions didn't exist before that,
      and it didn't seem worth adapting the patch to the oldest branches.
      But given the risk of JIT crashes, I think we certainly need this
      as far back as v11.
      
      Pre-v13, we have to explicitly exclude pg_subscription.subslotname
      and pg_subscription_rel.srsublsn from the checks, since they are
      mismarked.  (Even if we change our mind about applying BKI_FORCE_NULL
      in the branch tips, it doesn't seem wise to have assertions that
      would fire in existing databases.)
      
      Discussion: https://postgr.es/m/298837.1595196283@sss.pgh.pa.us
      3e66019f
    • Michael Paquier's avatar
      Rework tab completion of COPY and \copy in psql · c273d9d8
      Michael Paquier authored
      This corrects and simplifies $subject in a number of ways:
      - Remove from the completion the pre-9.0 grammar still supported for
      compatibility purposes.  This simplifies the code, and allows to extend
      it more easily with new patterns.
      - Add completion for the options of FORMAT within a WITH clause.
      - Complete WHERE and WITH clauses correctly depending on if TO or FROM
      are used, WHERE being only available with COPY FROM.
      
      Author: Vignesh C, Michael Paquier
      Reviewed-by: Ahsan Hadi
      Discussion: https://postgr.es/m/CALDaNm3zWr=OmxeNqOqfT=uZTSdam_j-gkX94CL8eTNfgUtf6A@mail.gmail.com
      c273d9d8
    • Tom Lane's avatar
      Fix some corner cases for window ranges with infinite offsets. · a4faef8f
      Tom Lane authored
      Many situations where the offset is infinity were not handled sanely.
      We should generally allow the val versus base +/- offset comparison to
      proceed according to the normal rules of IEEE arithmetic; however, we
      must do something special for the corner cases where base +/- offset
      would produce NaN due to subtracting two like-signed infinities.
      That corresponds to asking which values infinitely precede +inf or
      infinitely follow -inf, which should certainly be true of any finite
      value or of the opposite-signed infinity.  After some discussion it
      seems that the best decision is to make it true of the same-signed
      infinity as well, ie, just return constant TRUE if the calculation
      would produce a NaN.
      
      (We could write this with a bit less code by subtracting anyway,
      and then checking for a NaN result.  However, I prefer this
      formulation because it'll be easier to transpose into numeric.c.)
      
      Although this seems like clearly a bug fix with respect to finite
      values, it is less obviously correct for infinite values.  Between
      that and the fact that the whole issue only arises for very strange
      window specifications (e.g. RANGE BETWEEN 'inf' PRECEDING AND 'inf'
      PRECEDING), I'll desist from back-patching.
      
      Noted by Dean Rasheed.
      
      Discussion: https://postgr.es/m/3393130.1594925893@sss.pgh.pa.us
      a4faef8f
  2. 20 Jul, 2020 9 commits
  3. 19 Jul, 2020 4 commits
    • Peter Geoghegan's avatar
      Avoid harmless Valgrind no-buffer-pin errors. · a766d6ca
      Peter Geoghegan authored
      Valgrind builds with assertions enabled sometimes perform a
      theoretically unsafe page access inside an assertion in
      heapam_tuple_lock().  This happened when the eval-plan-qual isolation
      test ran one of the permutations added by commit a2418f9e.
      
      Avoid complaints from Valgrind by moving the assertion ever so slightly.
      This is minor cleanup for commit 1e0dfd16, which added Valgrind buffer
      access instrumentation.
      
      No backpatch, since this only happens within an assertion, and seems
      very unlikely to cause any real problems even with assert-enabled
      builds.
      a766d6ca
    • Peter Geoghegan's avatar
      Mark buffers as defined to Valgrind consistently. · 46ef520b
      Peter Geoghegan authored
      Make PinBuffer() mark buffers as defined to Valgrind unconditionally,
      including when the buffer header spinlock must be acquired.  Failure to
      handle that case could lead to false positive reports from Valgrind.
      
      This theoretically creates a risk that we'll mark buffers defined even
      when external callers don't end up with a buffer pin.  That seems
      perfectly acceptable, though, since in general we make no guarantees
      about buffers that are unsafe to access being reliably marked as unsafe.
      
      Oversight in commit 1e0dfd16, which added valgrind buffer access
      instrumentation.
      46ef520b
    • Tom Lane's avatar
      Correctly mark pg_subscription.subslotname as nullable. · 72eab84a
      Tom Lane authored
      Due to the layout of this catalog, subslotname has to be explicitly
      marked BKI_FORCE_NULL, else initdb will default to the assumption
      that it's non-nullable.  Since, in fact, CREATE/ALTER SUBSCRIPTION
      will store null values there, the existing marking is just wrong,
      and has been since this catalog was invented.
      
      We haven't noticed because not much in the system actually depends
      on attnotnull being truthful.  However, JIT'ed tuple deconstruction
      does depend on that in some cases, allowing crashes or wrong answers
      in queries that inspect pg_subscription.  Commit 9de77b54 quite
      accidentally exposed this on the buildfarm members that force JIT
      activation.
      
      Back-patch to v13.  The problem goes further back, but we cannot
      force initdb in released branches, so some klugier solution will
      be needed there.  Before working on that, push this simple fix
      to try to get the buildfarm back to green.
      
      Discussion: https://postgr.es/m/4118109.1595096139@sss.pgh.pa.us
      72eab84a
    • Peter Eisentraut's avatar
      Define OPENSSL_API_COMPAT · 4d3db136
      Peter Eisentraut authored
      This avoids deprecation warnings from newer OpenSSL versions (3.0.0 in
      particular).
      
      Discussion: https://www.postgresql.org/message-id/flat/FEF81714-D479-4512-839B-C769D2605F8A%40yesql.se
      4d3db136
  4. 18 Jul, 2020 8 commits
  5. 17 Jul, 2020 9 commits
    • Tom Lane's avatar
      Cope with data-offset-less archive files during out-of-order restores. · f009591d
      Tom Lane authored
      pg_dump produces custom-format archive files that lack data offsets
      when it is unable to seek its output.  Up to now that's been a hazard
      for pg_restore.  But if pg_restore is able to seek in the archive
      file, there is no reason to throw up our hands when asked to restore
      data blocks out of order.  Instead, whenever we are searching for a
      data block, record the locations of the blocks we passed over (that
      is, fill in the missing data-offset fields in our in-memory copy of
      the TOC data).  Then, when we hit a case that requires going
      backwards, we can just seek back.
      
      Also track the furthest point that we've searched to, and seek back
      to there when beginning a search for a new data block.  This avoids
      possible O(N^2) time consumption, by ensuring that each data block
      is examined at most twice.  (On Unix systems, that's at most twice
      per parallel-restore job; but since Windows uses threads here, the
      threads can share block location knowledge, reducing the amount of
      duplicated work.)
      
      We can also improve the code a bit by using fseeko() to skip over
      data blocks during the search.
      
      This is all of some use even in simple restores, but it's really
      significant for parallel pg_restore.  In that case, we require
      seekability of the input already, and we will very probably need
      to do out-of-order restores.
      
      Back-patch to v12, as this fixes a regression introduced by commit
      548e5097.  Before that, parallel restore avoided requesting
      out-of-order restores, so it would work on a data-offset-less
      archive.  Now it will again.
      
      Ideally this patch would include some test coverage, but there are
      other open bugs that need to be fixed before we can extend our
      coverage of parallel restore very much.  Plan to revisit that later.
      
      David Gilman and Tom Lane; reviewed by Justin Pryzby
      
      Discussion: https://postgr.es/m/CALBH9DDuJ+scZc4MEvw5uO-=vRyR2=QF9+Yh=3hPEnKHWfS81A@mail.gmail.com
      f009591d
    • Tom Lane's avatar
      Remove manual tracking of file position in pg_dump/pg_backup_custom.c. · a8d0732a
      Tom Lane authored
      We do not really need to track the file position by hand.  We were
      already relying on ftello() whenever the archive file is seekable,
      while if it's not seekable we don't need the file position info
      anyway because we're not going to be able to re-write the TOC.
      
      Moreover, that tracking was buggy since it failed to account for
      the effects of fseeko().  Somewhat remarkably, that seems not to
      have made for any live bugs up to now.  We could fix the oversights,
      but it seems better to just get rid of the whole error-prone mess.
      
      In itself this is merely code cleanup.  However, it's necessary
      infrastructure for an upcoming bug-fix patch (because that code
      *does* need valid file position after fseeko).  The bug fix
      needs to go back as far as v12; hence, back-patch that far.
      
      Discussion: https://postgr.es/m/CALBH9DDuJ+scZc4MEvw5uO-=vRyR2=QF9+Yh=3hPEnKHWfS81A@mail.gmail.com
      a8d0732a
    • Peter Geoghegan's avatar
      Avoid CREATE INDEX unique index deduplication. · 5da8bf8b
      Peter Geoghegan authored
      There is no advantage to attempting deduplication for a unique index
      during CREATE INDEX, since there cannot possibly be any duplicates.
      Doing so wastes cycles due to unnecessary copying.  Make sure that we
      avoid it consistently.
      
      We already avoided unique index deduplication in the case where there
      were some spool2 tuples to merge.  That didn't account for the fact that
      spool2 is removed early/unset in the common case where it has no tuples
      that need to be merged (i.e. it failed to account for the "spool2 turns
      out to be unnecessary" optimization in _bt_spools_heapscan()).
      
      Oversight in commit 0d861bbb, which added nbtree deduplication
      
      Backpatch: 13-, where nbtree deduplication was introduced.
      5da8bf8b
    • Tom Lane's avatar
      Ensure that distributed timezone abbreviation files are plain ASCII. · 7fe3083f
      Tom Lane authored
      We had two occurrences of "Mitteleuropäische Zeit" in Europe.txt,
      though the corresponding entries in Default were spelled
      "Mitteleuropaeische Zeit".  Standardize on the latter spelling to
      avoid questions of which encoding to use.
      
      While here, correct a couple of other trivial inconsistencies between
      the Default file and the supposedly-matching entries in the *.txt
      files, as exposed by some checking with comm(1).  Also, add BDST to
      the Europe.txt file; it previously was only listed in Default.
      None of this has any direct functional effect.
      
      Per complaint from Christoph Berg.  As usual for timezone data patches,
      apply to all branches.
      
      Discussion: https://postgr.es/m/20200716100743.GE3534683@msg.df7cb.de
      7fe3083f
    • Peter Eisentraut's avatar
      Fix whitespace · 20ef3551
      Peter Eisentraut authored
      20ef3551
    • Peter Eisentraut's avatar
      Resolve gratuitous tabs in SQL file · 44f34365
      Peter Eisentraut authored
      44f34365
    • Amit Kapila's avatar
      Fix signal handler setup for SIGHUP in the apply launcher process. · 01160a3d
      Amit Kapila authored
      Commit 1e53fe0e has unified the usage of the config-file reload flag by
      using the same signal handler function for the SIGHUP signal at many places
      in the code.  By mistake, it used the wrong SIGNAL in apply launcher
      process for the SIGHUP signal handler function.
      
      Author: Bharath Rupireddy
      Reviewed-by: Dilip Kumar
      Backpatch-through: 13, where it was introduced
      Discussion: https://postgr.es/m/CALj2ACVzHCRnS20bOiEHaLtP5PVBENZQn4khdsSJQgOv_GM-LA@mail.gmail.com
      01160a3d
    • Thomas Munro's avatar
      Use MinimalTuple for tuple queues. · cdc71695
      Thomas Munro authored
      This representation saves 8 bytes per tuple compared to HeapTuple, and
      avoids the need to allocate, copy and free on the receiving side.
      
      Gather can emit the returned MinimalTuple directly, but GatherMerge now
      needs to make an explicit copy because it buffers multiple tuples at a
      time.  That should be no worse than before.
      Reviewed-by: default avatarSoumyadeep Chakraborty <soumyadeep2007@gmail.com>
      Discussion: https://postgr.es/m/CA%2BhUKG%2B8T_ggoUTAE-U%3DA%2BOcPc4%3DB0nPPHcSfffuQhvXXjML6w%40mail.gmail.com
      cdc71695
    • Thomas Munro's avatar
      Add huge_page_size setting for use on Linux. · d2bddc25
      Thomas Munro authored
      This allows the huge page size to be set explicitly.  The default is 0,
      meaning it will use the system default, as before.
      
      Author: Odin Ugedal <odin@ugedal.com>
      Discussion: https://postgr.es/m/20200608154639.20254-1-odin%40ugedal.com
      d2bddc25
  6. 16 Jul, 2020 1 commit
    • Andrew Dunstan's avatar
      Enable almost all TAP tests involving symlinks on Windows · d66b23b0
      Andrew Dunstan authored
      Windows has junction points which function as symbolic links for
      directories. This patch introduces a new function TestLib::dir_symlink()
      which creates a junction point on Windows and a standard Unix type
      symbolic link elsewhere.
      
      The function TestLib::perl2host is also modified, first to use cygpath
      where it's available (e.g. msys2) and second to allow it to succeed if
      the gandparent directory exists but the parent does not.
      
      Given these changes the only symlink tests that need to be skipped on
      Windows are those related to permissions or to use of readlink. The
      relevant tests for pg_basebackup and pg_rewind are therefore adjusted
      accordingly.
      
      Andrew Dunstan, reviewed by Peter Eisentraut and Michael Paquier.
      
      Discussion: https://postgr.es/m/c50a646c-d9bb-7c62-a4bf-8256ff6ff338@2ndquadrant.com
      d66b23b0