1. 16 Dec, 2017 1 commit
  2. 15 Dec, 2017 2 commits
    • Andres Freund's avatar
      Perform a lot more sanity checks when freezing tuples. · 699bf7d0
      Andres Freund authored
      The previous commit has shown that the sanity checks around freezing
      aren't strong enough. Strengthening them seems especially important
      because the existance of the bug has caused corruption that we don't
      want to make even worse during future vacuum cycles.
      
      The errors are emitted with ereport rather than elog, despite being
      "should never happen" messages, so a proper error code is emitted. To
      avoid superflous translations, mark messages as internal.
      
      Author: Andres Freund and Alvaro Herrera
      Reviewed-By: Alvaro Herrera, Michael Paquier
      Discussion: https://postgr.es/m/20171102112019.33wb7g5wp4zpjelu@alap3.anarazel.de
      Backpatch: 9.3-
      699bf7d0
    • Andres Freund's avatar
      Fix pruning of locked and updated tuples. · 9c2f0a6c
      Andres Freund authored
      Previously it was possible that a tuple was not pruned during vacuum,
      even though its update xmax (i.e. the updating xid in a multixact with
      both key share lockers and an updater) was below the cutoff horizon.
      
      As the freezing code assumed, rightly so, that that's not supposed to
      happen, xmax would be preserved (as a member of a new multixact or
      xmax directly). That causes two problems: For one the tuple is below
      the xmin horizon, which can cause problems if the clog is truncated or
      once there's an xid wraparound. The bigger problem is that that will
      break HOT chains, which in turn can lead two to breakages: First,
      failing index lookups, which in turn can e.g lead to constraints being
      violated. Second, future hot prunes / vacuums can end up making
      invisible tuples visible again. There's other harmful scenarios.
      
      Fix the problem by recognizing that tuples can be DEAD instead of
      RECENTLY_DEAD, even if the multixactid has alive members, if the
      update_xid is below the xmin horizon. That's safe because newer
      versions of the tuple will contain the locking xids.
      
      A followup commit will harden the code somewhat against future similar
      bugs and already corrupted data.
      
      Author: Andres Freund, with changes by Alvaro Herrera
      Reported-By: Daniel Wood
      Analyzed-By: Andres Freund, Alvaro Herrera, Robert Haas, Peter
         Geoghegan, Daniel Wood, Yi Wen Wong, Michael Paquier
      Reviewed-By: Alvaro Herrera, Robert Haas, Michael Paquier
      Discussion:
          https://postgr.es/m/E5711E62-8FDF-4DCA-A888-C200BF6B5742@amazon.com
          https://postgr.es/m/20171102112019.33wb7g5wp4zpjelu@alap3.anarazel.de
      Backpatch: 9.3-
      9c2f0a6c
  3. 14 Dec, 2017 4 commits
  4. 13 Dec, 2017 15 commits
    • Andres Freund's avatar
      Allow executor nodes to change their ExecProcNode function. · 538d114f
      Andres Freund authored
      In order for executor nodes to be able to change their ExecProcNode function
      after ExecInitNode() has finished, provide ExecSetExecProcNode().  This allows
      any wrappers functions that only execProcnode.c knows about to be reinstalled.
      The motivation for wanting to change ExecProcNode after ExecInitNode() has
      finished is that it is not known until later whether parallel query is
      available, so if a parallel variant is to be installed then ExecInitNode()
      is too soon to decide.
      
      Author: Thomas Munro
      Reviewed-By: Andres Freund
      Discussion: https://postgr.es/m/CAEepm=09rr65VN+cAV5FgyM_z=D77Xy8Fuc9CDDDYbq3pQUezg@mail.gmail.com
      538d114f
    • Andres Freund's avatar
      Add pg_attribute_always_inline. · dbb3d6f0
      Andres Freund authored
      Sometimes it is useful to be able to insist that the compiler inline a
      function that its normal cost analysis would not normally choose to inline.
      This can be useful for instantiating different variants of a function that
      remove branches of code by constant folding.
      
      Author: Thomas Munro
      Reviewed-By: Andres Freund
      Discussion: https://postgr.es/m/CAEepm=09rr65VN+cAV5FgyM_z=D77Xy8Fuc9CDDDYbq3pQUezg@mail.gmail.com
      dbb3d6f0
    • Andres Freund's avatar
      Add defenses against pre-crash files to BufFileOpenShared(). · 923e8dee
      Andres Freund authored
      Crash restarts currently don't clean up temporary files, as a debugging aid.
      If a left-over file happens to have the same name as a segment file we're
      trying to create, we'll just truncate and reuse it, but there is a problem:
      BufFileOpenShared() determines how many segment files exist by trying to open
      .0, .1, .2, ... until it finds no more files.  It might be confused by a junk
      file that has the next segment number.  To defend against that, make sure we
      always create a gap after the end file by unlinking the following name if it
      exists.  Also make it an error to try to open a BufFile that doesn't exist
      (has no segment 0), so as not to encourage the development of client code
      that depends on an interface that we can't reliably provide.
      
      Author: Thomas Munro
      Reviewed-By: Andres Freund
      Discussion: https://postgr.es/m/CAEepm%3D2jhCbC_GFQJaaDhWxLB4EXtT3vVd5czuRNaqF5CWSTog%40mail.gmail.com
      923e8dee
    • Robert Haas's avatar
      Fix parallel index scan hang with deleted or half-dead pages. · 884a6084
      Robert Haas authored
      The previous coding forgot to release the scan before seizing
      it again, leading to a lockup.
      
      Report by Patrick Hemmer.  Diagnosis by Thomas Munro.  Patch by
      Amit Kapila.
      
      Discussion: http://postgr.es/m/CAEepm=2xZUcOGP9V0O_G0=2P2wwXwPrkF=upWTCJSisUxMnuSg@mail.gmail.com
      884a6084
    • Robert Haas's avatar
      Revert "Fix accumulation of parallel worker instrumentation." · 1d6fb35a
      Robert Haas authored
      This reverts commit 2c09a5c1.  Per
      further discussion, that doesn't seem to be the best possible fix.
      
      Discussion: http://postgr.es/m/CAA4eK1LW2aFKzY3=vwvc=t-juzPPVWP2uT1bpx_MeyEqnM+p8g@mail.gmail.com
      1d6fb35a
    • Tom Lane's avatar
      Rethink MemoryContext creation to improve performance. · 9fa6f00b
      Tom Lane authored
      This patch makes a number of interrelated changes to reduce the overhead
      involved in creating/deleting memory contexts.  The key ideas are:
      
      * Include the AllocSetContext header of an aset.c context in its first
      malloc request, rather than allocating it separately in TopMemoryContext.
      This means that we now always create an initial or "keeper" block in an
      aset, even if it never receives any allocation requests.
      
      * Create freelists in which we can save and recycle recently-destroyed
      asets (this idea is due to Robert Haas).
      
      * In the common case where the name of a context is a constant string,
      just store a pointer to it in the context header, rather than copying
      the string.
      
      The first change eliminates a palloc/pfree cycle per context, and
      also avoids bloat in TopMemoryContext, at the price that creating
      a context now involves a malloc/free cycle even if the context never
      receives any allocations.  That would be a loser for some common
      usage patterns, but recycling short-lived contexts via the freelist
      eliminates that pain.
      
      Avoiding copying constant strings not only saves strlen() and strcpy()
      overhead, but is an essential part of the freelist optimization because
      it makes the context header size constant.  Currently we make no
      attempt to use the freelist for contexts with non-constant names.
      (Perhaps someday we'll need to think harder about that, but in current
      usage, most contexts with custom names are long-lived anyway.)
      
      The freelist management in this initial commit is pretty simplistic,
      and we might want to refine it later --- but in common workloads that
      will never matter because the freelists will never get full anyway.
      
      To create a context with a non-constant name, one is now required to
      call AllocSetContextCreateExtended and specify the MEMCONTEXT_COPY_NAME
      option.  AllocSetContextCreate becomes a wrapper macro, and it includes
      a test that will complain about non-string-literal context name
      parameters on gcc and similar compilers.
      
      An unfortunate side effect of making AllocSetContextCreate a macro is
      that one is now *required* to use the size parameter abstraction macros
      (ALLOCSET_DEFAULT_SIZES and friends) with it; the pre-9.6 habit of
      writing out individual size parameters no longer works unless you
      switch to AllocSetContextCreateExtended.
      
      Internally to the memory-context-related modules, the context creation
      APIs are simplified, removing the rather baroque original design whereby
      a context-type module called mcxt.c which then called back into the
      context-type module.  That saved a bit of code duplication, but not much,
      and it prevented context-type modules from exercising control over the
      allocation of context headers.
      
      In passing, I converted the test-and-elog validation of aset size
      parameters into Asserts to save a few more cycles.  The original thought
      was that callers might compute size parameters on the fly, but in practice
      nobody does that, so it's useless to expend cycles on checking those
      numbers in production builds.
      
      Also, mark the memory context method-pointer structs "const",
      just for cleanliness.
      
      Discussion: https://postgr.es/m/2264.1512870796@sss.pgh.pa.us
      9fa6f00b
    • Peter Eisentraut's avatar
      Start a separate test suite for plpgsql · 632b03da
      Peter Eisentraut authored
      The plpgsql.sql test file in the main regression tests is now by far the
      largest after numeric_big, making editing and managing the test cases
      very cumbersome.  The other PLs have their own test suites split up into
      smaller files by topic.  It would be nice to have that for plpgsql as
      well.  So, to get that started, set up test infrastructure in
      src/pl/plpgsql/src/ and split out the recently added procedure test
      cases into a new file there.  That file now mirrors the test cases added
      to the other PLs, making managing those matching tests a bit easier too.
      
      msvc build system changes with help from Michael Paquier
      632b03da
    • Peter Eisentraut's avatar
      Fix crash when using CALL on an aggregate · 3d887422
      Peter Eisentraut authored
      Author: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
      Reported-by: default avatarRushabh Lathia <rushabh.lathia@gmail.com>
      3d887422
    • Andres Freund's avatar
      Add float.h include to int8.c, for isnan(). · 8e211f53
      Andres Freund authored
      port.h redirects isnan() to _isnan() on windows, which in turn is
      provided by float.h rather than math.h. Therefore include the latter
      as well.
      
      Per buildfarm.
      8e211f53
    • Andres Freund's avatar
      Consistently use PG_INT(16|32|64)_(MIN|MAX). · f512a6e1
      Andres Freund authored
      Per buildfarm animal woodlouse.
      f512a6e1
    • Peter Eisentraut's avatar
      PL/Python: Fix potential NULL pointer dereference · 4c6744ed
      Peter Eisentraut authored
      After d0aa965c, one error path in
      PLy_spi_execute_fetch_result() could result in the variable "result"
      being dereferenced after being set to NULL.  Rearrange the code a bit to
      fix that.
      
      Also add another SPI_freetuptable() call so that that is cleared in all
      error paths.
      
      discovered by John Naylor <jcnaylor@gmail.com> via scan-build
      
      ideas and review by Tom Lane
      4c6744ed
    • Andres Freund's avatar
      Make PGAC_C_BUILTIN_OP_OVERFLOW link instead of just compiling. · 85abb5b2
      Andres Freund authored
      Otherwise the detection can spuriously detect symbol as available,
      because the compiler may just emits reference to non-existant symbol.
      85abb5b2
    • Andres Freund's avatar
      Use new overflow aware integer operations. · 101c7ee3
      Andres Freund authored
      A previous commit added inline functions that provide fast(er) and
      correct overflow checks for signed integer math. Use them in a
      significant portion of backend code.  There's more to touch in both
      backend and frontend code, but these were the easily identifiable
      cases.
      
      The old overflow checks are noticeable in integer heavy workloads.
      
      A secondary benefit is that getting rid of overflow checks that rely
      on signed integer overflow wrapping around, will allow us to get rid
      of -fwrapv in the future. Which in turn slows down other code.
      
      Author: Andres Freund
      Discussion: https://postgr.es/m/20171024103954.ztmatprlglz3rwke@alap3.anarazel.de
      101c7ee3
    • Andres Freund's avatar
      Provide overflow safe integer math inline functions. · 4d6ad312
      Andres Freund authored
      It's not easy to get signed integer overflow checks correct and
      fast. Therefore abstract the necessary infrastructure into a common
      header providing addition, subtraction and multiplication for 16, 32,
      64 bit signed integers.
      
      The new macros aren't yet used, but a followup commit will convert
      several open coded overflow checks.
      
      Author: Andres Freund, with some code stolen from Greg Stark
      Reviewed-By: Robert Haas
      Discussion: https://postgr.es/m/20171024103954.ztmatprlglz3rwke@alap3.anarazel.de
      4d6ad312
    • Robert Haas's avatar
      Remove obsolete comment. · 95b52351
      Robert Haas authored
      Commit 8b304b8b removed replacement
      selection, but left behind this comment text.  The optimization to
      which the comment refers is not relevant without replacement
      selection, because if we had so few tuples as to require only one
      tape, we would have just completed the sort in memory.
      
      Peter Geoghegan
      
      Discussion: http://postgr.es/m/CAH2-WznqupLA8CMjp+vqzoe0yXu0DYYbQSNZxmgN76tLnAOZ_w@mail.gmail.com
      95b52351
  5. 12 Dec, 2017 2 commits
  6. 11 Dec, 2017 3 commits
  7. 10 Dec, 2017 1 commit
    • Tom Lane's avatar
      Stabilize output of new regression test case. · 9edc97b7
      Tom Lane authored
      The test added by commit 390d5813 turns out to have different output
      in CLOBBER_CACHE_ALWAYS builds: there's an extra CONTEXT line in the
      error message as a result of detecting the error at a different place.
      Possibly we should do something to make that more consistent.  But as
      a stopgap measure to make the buildfarm green again, adjust the test
      to suppress CONTEXT entirely.  We can revert this if we do something
      in the backend to eliminate the inconsistency.
      
      Discussion: https://postgr.es/m/31545.1512924904@sss.pgh.pa.us
      9edc97b7
  8. 09 Dec, 2017 5 commits
    • Tom Lane's avatar
      Fix plpgsql to reinitialize record variables at block re-entry. · 390d5813
      Tom Lane authored
      If one exits and re-enters a DECLARE ... BEGIN ... END block within a
      single execution of a plpgsql function, perhaps due to a surrounding loop,
      the declared variables are supposed to get re-initialized to null (or
      whatever their initializer is).  But this failed to happen for variables
      of type "record", because while exec_stmt_block() expected such variables
      to be included in the block's initvarnos list, plpgsql_add_initdatums()
      only adds DTYPE_VAR variables to that list.  This bug appears to have
      been there since the aboriginal addition of plpgsql to our tree.
      
      Fix by teaching plpgsql_add_initdatums() to include DTYPE_REC variables
      as well.  (We don't need to consider other DTYPEs because they don't
      represent separately-stored values.)  I failed to resist the temptation
      to make some nearby cosmetic adjustments, too.
      
      No back-patch, because there have not been field complaints, and it
      seems possible that somewhere out there someone has code depending
      on the incorrect behavior.  In any case this change would have no
      impact on correctly-written code.
      
      Discussion: https://postgr.es/m/22994.1512800671@sss.pgh.pa.us
      390d5813
    • Magnus Hagander's avatar
      Fix regression test output · ce1468d0
      Magnus Hagander authored
      Missed this in the last commit.
      ce1468d0
    • Magnus Hagander's avatar
      Fix typo · d8f632ca
      Magnus Hagander authored
      Reported by Robins Tharakan
      d8f632ca
    • Noah Misch's avatar
      MSVC 2012+: Permit linking to 32-bit, MinGW-built libraries. · 7e0c574e
      Noah Misch authored
      Notably, this permits linking to the 32-bit Perl binaries advertised on
      perl.org, namely Strawberry Perl and ActivePerl.  This has a side effect
      of permitting linking to binaries built with obsolete MSVC versions.
      
      By default, MSVC 2012 and later require a "safe exception handler table"
      in each binary.  MinGW-built, 32-bit DLLs lack the relevant exception
      handler metadata, so linking to them failed with error LNK2026.  Restore
      the semantics of MSVC 2010, which omits the table from a given binary if
      some linker input lacks metadata.  This has no effect on 64-bit builds
      or on MSVC 2010 and earlier.  Back-patch to 9.3 (all supported
      versions).
      
      Reported by Victor Wagner.
      
      Discussion: https://postgr.es/m/20160326154321.7754ab8f@wagner.wagner.home
      7e0c574e
    • Noah Misch's avatar
      MSVC: Test whether 32-bit Perl needs -D_USE_32BIT_TIME_T. · 65a00f30
      Noah Misch authored
      Commits 5a5c2fec and
      b5178c5d introduced support for modern
      MSVC-built, 32-bit Perl, but they broke use of MinGW-built, 32-bit Perl
      distributions like Strawberry Perl and modern ActivePerl.  Perl has no
      robust means to report whether it expects a -D_USE_32BIT_TIME_T ABI, so
      test this.  Back-patch to 9.3 (all supported versions).
      
      The chief alternative was a heuristic of adding -D_USE_32BIT_TIME_T when
      $Config{gccversion} is nonempty.  That banks on every gcc-built Perl
      using the same ABI.  gcc could change its default ABI the way MSVC once
      did, and one could build Perl with gcc and the non-default ABI.
      
      The GNU make build system could benefit from a similar test, without
      which it does not support MSVC-built Perl.  For now, just add a comment.
      Most users taking the special step of building Perl with MSVC probably
      build PostgreSQL with MSVC.
      
      Discussion: https://postgr.es/m/20171130041441.GA3161526@rfd.leadboat.com
      65a00f30
  9. 08 Dec, 2017 4 commits
  10. 07 Dec, 2017 1 commit
  11. 06 Dec, 2017 2 commits
    • Robert Haas's avatar
      Report failure to start a background worker. · 28724fd9
      Robert Haas authored
      When a worker is flagged as BGW_NEVER_RESTART and we fail to start it,
      or if it is not marked BGW_NEVER_RESTART but is terminated before
      startup succeeds, what BgwHandleStatus should be reported?  The
      previous code really hadn't considered this possibility (as indicated
      by the comments which ignore it completely) and would typically return
      BGWH_NOT_YET_STARTED, but that's not a good answer, because then
      there's no way for code using GetBackgroundWorkerPid() to tell the
      difference between a worker that has not started but will start
      later and a worker that has not started and will never be started.
      So, when this case happens, return BGWH_STOPPED instead.  Update the
      comments to reflect this.
      
      The preceding fix by itself is insufficient to fix the problem,
      because the old code also didn't send a notification to the process
      identified in bgw_notify_pid when startup failed.  That might've
      been technically correct under the theory that the status of the
      worker was BGWH_NOT_YET_STARTED, because the status would indeed not
      change when the worker failed to start, but now that we're more
      usefully reporting BGWH_STOPPED, a notification is needed.
      
      Without these fixes, code which starts background workers and then
      uses the recommended APIs to wait for those background workers to
      start would hang indefinitely if the postmaster failed to fork a
      worker.
      
      Amit Kapila and Robert Haas
      
      Discussion: http://postgr.es/m/CAA4eK1KDfKkvrjxsKJi3WPyceVi3dH1VCkbTJji2fuwKuB=3uw@mail.gmail.com
      28724fd9
    • Robert Haas's avatar
      Fix Parallel Append crash. · 9c64ddd4
      Robert Haas authored
      Reported by Tom Lane and the buildfarm.
      
      Amul Sul and Amit Khandekar
      
      Discussion: http://postgr.es/m/17868.1512519318@sss.pgh.pa.us
      Discussion: http://postgr.es/m/CAJ3gD9cJQ4d-XhmZ6BqM9rMM2KDBfpkdgOAb4+psz56uBuMQ_A@mail.gmail.com
      9c64ddd4