1. 26 Feb, 2019 2 commits
    • Peter Geoghegan's avatar
      Remove unneeded argument from _bt_getstackbuf(). · 2ab23445
      Peter Geoghegan authored
      _bt_getstackbuf() is called at exactly two points following commit
      efada2b8 (one call site is concerned with page splits, while the
      other is concerned with page deletion).  The parent buffer returned by
      _bt_getstackbuf() is write-locked in both cases.  Remove the 'access'
      argument and make _bt_getstackbuf() assume that callers require a
      write-lock.
      2ab23445
    • Peter Geoghegan's avatar
      Correct obsolete nbtree page deletion comment. · 067786ce
      Peter Geoghegan authored
      Commit efada2b8, which made the nbtree page deletion algorithm more
      robust, removed _bt_getstackbuf() calls from _bt_pagedel().  It failed
      to update a comment that referenced the earlier approach.  Update the
      comment to explain that the _bt_getstackbuf() page deletion call site
      mirrors the only other remaining _bt_getstackbuf() call site, which is
      reached during page splits.
      067786ce
  2. 25 Feb, 2019 3 commits
    • Peter Eisentraut's avatar
      psql: Remove obsolete code · b6926dee
      Peter Eisentraut authored
      The check in create_help.pl for a null end tag (</>) has been obsolete
      since the conversion from SGML to XML, since XML does not allow that
      anymore.
      b6926dee
    • Peter Eisentraut's avatar
      Remove unnecessary use of PROCEDURAL · bc09d5e4
      Peter Eisentraut authored
      Remove some unnecessary, legacy-looking use of the PROCEDURAL keyword
      before LANGUAGE.  We mostly don't use this anymore, so some of these
      look a bit old.
      
      There is still some use in pg_dump, which is harder to remove because
      it's baked into the archive format, so I'm not touching that.
      
      Discussion: https://www.postgresql.org/message-id/2330919b-62d9-29ac-8de3-58c024fdcb96@2ndquadrant.com
      bc09d5e4
    • Michael Paquier's avatar
      Make release of 2PC identifier and locks consistent in COMMIT PREPARED · effe7d95
      Michael Paquier authored
      When preparing a transaction in two-phase commit, a dummy PGPROC entry
      holding the GID used for the transaction is registered, which gets
      released once COMMIT PREPARED is run.  Prior releasing its shared memory
      state, all the locks taken in the prepared transaction are released
      using a dedicated set of callbacks (pgstat and multixact having similar
      callbacks), which may cause the locks to be released before the GID is
      set free.
      
      Hence, there is a small window where lock conflicts could happen, for
      example:
      - Transaction A releases its locks, still holding its GID in shared
      memory.
      - Transaction B held a lock which conflicted with locks of transaction
      A.
      - Transaction B continues its processing, reusing the same GID as
      transaction A.
      - Transaction B fails because of a conflicting GID, already in use by
      transaction A.
      
      This commit changes the shared memory state release so as post-commit
      callbacks and predicate lock cleanup happen consistently with the shared
      memory state cleanup for the dummy PGPROC entry.  The race window is
      small and 2PC had this issue from the start, so no backpatch is done.
      On top if that fixes discussed involved ABI breakages, which are not
      welcome in stable branches.
      
      Reported-by: Oleksii Kliukin, Ildar Musin
      Diagnosed-by: Oleksii Kliukin, Ildar Musin
      Author: Michael Paquier
      Reviewed-by: Masahiko Sawada, Oleksii Kliukin
      Discussion: https://postgr.es/m/BF9B38A4-2BFF-46E8-BA87-A2D00A8047A6@hintbits.com
      effe7d95
  3. 24 Feb, 2019 4 commits
    • Thomas Munro's avatar
      Fix inconsistent out-of-memory error reporting in dsa.c. · 29ddb548
      Thomas Munro authored
      Commit 16be2fd1 introduced the flag DSA_ALLOC_NO_OOM to control whether
      the DSA allocator would raise an error or return InvalidDsaPointer on
      failure to allocate.  One edge case was not handled correctly: if we
      fail to allocate an internal "span" object for a large allocation, we
      would always return InvalidDsaPointer regardless of the flag; a caller
      not expecting that could then dereference a null pointer.
      
      This is a plausible explanation for a one-off report of a segfault.
      
      Remove a redundant pair of braces so that all three stanzas that handle
      DSA_ALLOC_NO_OOM match in style, for visual consistency.
      
      While fixing inconsistencies, if FreePageManagerGet() can't supply the
      pages that our book-keeping says it should be able to supply, then we
      should always report a FATAL error.  Previously we treated that as a
      regular allocation failure in one code path, but as a FATAL condition
      in another.
      
      Back-patch to 10, where dsa.c landed.
      
      Author: Thomas Munro
      Reported-by: Jakub Glapa
      Discussion: https://postgr.es/m/CAEepm=2oPqXxyWQ-1o60tpOLrwkw=VpgNXqqF1VN2EyO9zKGQw@mail.gmail.com
      29ddb548
    • Tom Lane's avatar
      Fix ecpg bugs caused by missing semicolons in the backend grammar. · 9e138a40
      Tom Lane authored
      The Bison documentation clearly states that a semicolon is required
      after every grammar rule, and our scripts that generate ecpg's
      grammar from the backend's implicitly assumed this is true.  But it
      turns out that only ancient versions of Bison actually enforce that.
      There have been a couple of rules without trailing semicolons in
      gram.y for some time, and as a consequence, ecpg's grammar was faulty
      and produced wrong output for the affected statements.
      
      To fix, add the missing semis, and add some cross-checks to ecpg's
      scripts so that they'll bleat if we mess this up again.
      
      The cases that were broken were:
      * "SET variable = DEFAULT" (but not "SET variable TO DEFAULT"),
        as well as allied syntaxes such as ALTER SYSTEM SET ... DEFAULT.
        These produced syntactically invalid output that the server
        would reject.
      * Multiple type names in DROP TYPE/DOMAIN commands.  Only the
        first type name would be listed in the emitted command.
      
      Per report from Daisuke Higuchi.  Back-patch to all supported versions.
      
      Discussion: https://postgr.es/m/1803D792815FC24D871C00D17AE95905DB51CE@g01jpexmbkw24
      9e138a40
    • Thomas Munro's avatar
      Tolerate EINVAL when calling fsync() on a directory. · f16735d8
      Thomas Munro authored
      Previously, we tolerated EBADF as a way for the operating system to
      indicate that it doesn't support fsync() on a directory.  Tolerate
      EINVAL too, for older versions of Linux CIFS.
      
      Bug #15636.  Back-patch all the way.
      
      Reported-by: John Klann
      Discussion: https://postgr.es/m/15636-d380890dafd78fc6@postgresql.org
      f16735d8
    • Thomas Munro's avatar
      Tolerate ENOSYS failure from sync_file_range(). · 483520ec
      Thomas Munro authored
      One unintended consequence of commit 9ccdd7f6 was that Windows WSL
      users started getting a panic whenever we tried to initiate data
      flushing with sync_file_range(), because WSL does not implement that
      system call.  Previously, they got a stream of periodic warnings,
      which was also undesirable but at least ignorable.
      
      Prevent the panic by handling ENOSYS specially and skipping the panic
      promotion with data_sync_elevel().  Also suppress future attempts
      after the first such failure so that the pre-existing problem of
      noisy warnings is improved.
      
      Back-patch to 9.6 (older branches were not affected in this way by
      9ccdd7f6).
      
      Author: Thomas Munro and James Sewell
      Tested-by: James Sewell
      Reported-by: Bruce Klein
      Discussion: https://postgr.es/m/CA+mCpegfOUph2U4ZADtQT16dfbkjjYNJL1bSTWErsazaFjQW9A@mail.gmail.com
      483520ec
  4. 23 Feb, 2019 1 commit
  5. 22 Feb, 2019 6 commits
  6. 21 Feb, 2019 9 commits
  7. 20 Feb, 2019 7 commits
    • Andrew Gierth's avatar
      Use an unsigned char for bool if we don't use the native bool. · d26a810e
      Andrew Gierth authored
      On (rare) platforms where sizeof(bool) > 1, we need to use our own
      bool, but imported c99 code (such as Ryu) may want to use bool values
      as array subscripts, which elicits warnings if bool is defined as
      char. Using unsigned char instead should work just as well for our
      purposes, and avoid such warnings.
      
      Per buildfarm members prariedog and locust.
      d26a810e
    • Tom Lane's avatar
      Improve planner's understanding of strictness of type coercions. · e04a3905
      Tom Lane authored
      PG type coercions are generally strict, ie a NULL input must produce
      a NULL output (or, in domain cases, possibly an error).  The planner's
      understanding of that was a bit incomplete though, so improve it:
      
      * Teach contain_nonstrict_functions() that CoerceViaIO can always be
      considered strict.  Previously it believed that only if the underlying
      I/O functions were marked strict, which is often but not always true.
      
      * Teach clause_is_strict_for() that CoerceViaIO, ArrayCoerceExpr,
      ConvertRowtypeExpr, CoerceToDomain can all be considered strict.
      Previously it knew nothing about any of them.
      
      The main user-visible impact of this is that IS NOT NULL predicates
      can be proven to hold from expressions involving casts in more cases
      than before, allowing partial indexes with such predicates to be used
      without extra pushups.  This reduces the surprise factor for users,
      who may well be used to ordinary (function-call-based) casts being
      known to be strict.
      
      Per a gripe from Samuel Williams.  This doesn't rise to the level of
      a bug, IMO, so no back-patch.
      
      Discussion: https://postgr.es/m/27571.1550617881@sss.pgh.pa.us
      e04a3905
    • Tom Lane's avatar
      Fix incorrect strictness test for ArrayCoerceExpr expressions. · 1571bc0f
      Tom Lane authored
      The recursion in contain_nonstrict_functions_walker() was done wrong,
      causing the strictness check to be bypassed for a parse node that
      is the immediate input of an ArrayCoerceExpr node.  This could allow,
      for example, incorrect decisions about whether a strict SQL function
      can be inlined.
      
      I didn't add a regression test, because (a) the bug is so narrow
      and (b) I couldn't think of a test case that wasn't dependent on a
      large number of other behaviors, to the point where it would likely
      soon rot to the point of not testing what it was intended to.
      
      I broke this in commit c12d570f, so back-patch to v11.
      
      Discussion: https://postgr.es/m/27571.1550617881@sss.pgh.pa.us
      1571bc0f
    • Alvaro Herrera's avatar
      Make object address handling more robust · 5721b9b3
      Alvaro Herrera authored
      pg_identify_object_as_address crashes when passed certain tuples from
      inconsistent system catalogs.  Make it more defensive.
      
      Author: Álvaro Herrera
      Reviewed-by: Michaël Paquier
      Discussion: https://postgr.es/m/20190218202743.GA12392@alvherre.pgsql
      5721b9b3
    • Amit Kapila's avatar
      Doc: Update the documentation for FSM behavior for small tables. · 29d108cd
      Amit Kapila authored
      In commit b0eaa4c5, we have avoided the creation of FSM for small tables.
      So the functions that use FSM to compute the free space can return zero for
      such tables.  This was previously also possible for the cases where the
      vacuum has not been triggered to update FSM.
      
      This commit updates the comments in code and documentation to reflect this
      behavior.
      
      Author: John Naylor
      Reviewed-by: Amit Kapila
      Discussion: https://postgr.es/m/CACPNZCtba-3m1q3A8gxA_vxg=T7gQf7gMbpR4Ciy5LntY-j+0Q@mail.gmail.com
      29d108cd
    • Dean Rasheed's avatar
      Fix DEFAULT-handling in multi-row VALUES lists for updatable views. · 41531e42
      Dean Rasheed authored
      INSERT ... VALUES for a single VALUES row is implemented differently
      from a multi-row VALUES list, which causes inconsistent behaviour in
      the way that DEFAULT items are handled. In particular, when inserting
      into an auto-updatable view on top of a table with a column default, a
      DEFAULT item in a single VALUES row gets correctly replaced with the
      table column's default, but for a multi-row VALUES list it is replaced
      with NULL.
      
      Fix this by allowing rewriteValuesRTE() to leave DEFAULT items in the
      VALUES list untouched if the target relation is an auto-updatable view
      and has no column default, deferring DEFAULT-expansion until the query
      against the base relation is rewritten. For all other types of target
      relation, including tables and trigger- and rule-updatable views, we
      must continue to replace DEFAULT items with NULL in the absence of a
      column default.
      
      This is somewhat complicated by the fact that if an auto-updatable
      view has DO ALSO rules attached, the VALUES lists for the product
      queries need to be handled differently from the original query, since
      the product queries need to act like rule-updatable views whereas the
      original query has auto-updatable view semantics.
      
      Back-patch to all supported versions.
      
      Reported by Roger Curley (bug #15623). Patch by Amit Langote and me.
      
      Discussion: https://postgr.es/m/15623-5d67a46788ec8b7f@postgresql.org
      41531e42
    • Michael Paquier's avatar
      Mark correctly initial slot snapshots with MVCC type when built · 56fadbed
      Michael Paquier authored
      When building an initial slot snapshot, snapshots are marked with
      historic MVCC snapshots as type with the marker field being set in
      SnapBuildBuildSnapshot() but not overriden in SnapBuildInitialSnapshot().
      Existing callers of SnapBuildBuildSnapshot() do not care about the type
      of snapshot used, but extensions calling it actually may, as reported.
      
      While on it, mark correctly the snapshot type when importing one.  This
      is cosmetic as the field is enforced to 0.
      
      Author: Antonin Houska
      Reviewed-by: Álvaro Herrera, Michael Paquier
      Discussion: https://postgr.es/m/23215.1527665193@localhost
      Backpatch-through: 9.4
      56fadbed
  8. 19 Feb, 2019 2 commits
  9. 18 Feb, 2019 6 commits