1. 14 Apr, 2017 6 commits
  2. 13 Apr, 2017 17 commits
  3. 12 Apr, 2017 9 commits
    • Tom Lane's avatar
      Speed up hash_index regression test. · 4a8bc39b
      Tom Lane authored
      Commit f5ab0a14 made this test take substantially longer than it used
      to.  With a bit more care, we can get the runtime back down while
      achieving the same, or even a bit better, code coverage.
      
      Mithun Cy
      
      Discussion: https://postgr.es/m/CAD__Ouh-qaEb+rD7Uy-4g3xQYOrhPzHs-a_TrFAjiQ5azAW5+w@mail.gmail.com
      4a8bc39b
    • Tom Lane's avatar
      Avoid transferring parallel-unsafe subplans to parallel workers. · 16ebab68
      Tom Lane authored
      Commit 5e6d8d2b allowed parallel workers to execute parallel-safe
      subplans, but it transmitted the query's entire list of subplans to
      the worker(s).  Since execMain.c blindly does ExecInitNode and later
      ExecEndNode on every list element, this resulted in parallel-unsafe plan
      nodes nonetheless getting started up and shut down in parallel workers.
      That seems mostly harmless as far as core plan node types go (but
      maybe not so much for Gather?).  But it resulted in postgres_fdw
      opening and then closing extra remote connections, and it's likely
      that other non-parallel-safe FDWs or custom scan providers would have
      worse reactions.
      
      To fix, just make ExecSerializePlan replace parallel-unsafe subplans
      with NULLs in the cut-down plan tree that it transmits to workers.
      This relies on ExecInitNode and ExecEndNode to do nothing on NULL
      input, but they do anyway.  If anything else is touching the dropped
      subplans in a parallel worker, that would be a bug to be fixed.
      (This thus provides a strong guarantee that we won't try to do
      something with a parallel-unsafe subplan in a worker.)
      
      This is, I think, the last fix directly occasioned by Andreas Seltenreich's
      bug report of a few days ago.
      
      Tom Lane and Amit Kapila
      
      Discussion: https://postgr.es/m/87tw5x4vcu.fsf@credativ.de
      16ebab68
    • Peter Eisentraut's avatar
      doc: Tweak CSS · f6f9f8a2
      Peter Eisentraut authored
      Tweak CSS a bit to match latest similar changes to web site style.  Also
      move some CSS out of the HTML to the stylesheet so that the web site
      stylesheet can override it.  This should ensure that notes and such are
      back to being centered.
      f6f9f8a2
    • Bruce Momjian's avatar
      git_changelog: improve instructions for finding branch commits · 85485401
      Bruce Momjian authored
      Specifically, use '--summary' with 'git show'.
      85485401
    • Tom Lane's avatar
      Mark finished Plan nodes with parallel_safe flags. · 003d80f3
      Tom Lane authored
      We'd managed to avoid doing this so far, but it seems pretty obvious
      that it would be forced on us some day, and this is much the cleanest
      way of approaching the open problem that parallel-unsafe subplans are
      being transmitted to parallel workers.  Anyway there's no space cost
      due to alignment considerations, and the time cost is pretty minimal
      since we're just copying the flag from the corresponding Path node.
      (At least in most cases ... some of the klugier spots in createplan.c
      have to work a bit harder.)
      
      In principle we could perhaps get rid of SubPlan.parallel_safe,
      but I thought it better to keep that in case there are reasons to
      consider a SubPlan unsafe even when its child plan is parallel-safe.
      
      This patch doesn't actually do anything with the new flags, but
      I thought I'd commit it separately anyway.
      
      Note: although this touches outfuncs/readfuncs, there's no need for
      a catversion bump because Plan trees aren't stored on disk.
      
      Discussion: https://postgr.es/m/87tw5x4vcu.fsf@credativ.de
      003d80f3
    • Peter Eisentraut's avatar
      Remove some tabs in SQL code in C string literals · 35b5f7b6
      Peter Eisentraut authored
      This is not handled uniformly throughout the code, but at least nearby
      code can be consistent.
      35b5f7b6
    • Robert Haas's avatar
      Fix pgstattuple's handling of unused hash pages. · 9cc27566
      Robert Haas authored
      Hash indexes can contain both pages which are all-zeroes (i.e.
      PageIsNew()) and pages which have been initialized but currently
      aren't used.  The latter category can happen either when a page
      has been reserved but not yet used or when it is used for a time
      and then freed.  pgstattuple was only prepared to deal with the
      pages that are actually-zeroes, which it called zero_pages.
      Rename the column to unused_pages (extension version 1.5 is
      as-yet-unreleased) and make it count both kinds of unused pages.
      
      Along the way, slightly tidy up the way we test for pages of
      various types.
      
      Robert Haas and Ashutosh Sharma, reviewed by Amit Kapila
      
      Discussion: http://postgr.es/m/CAE9k0PkTtKFB3YndOyQMjwuHx+-FtUP1ynK8E-nHtetoow3NtQ@mail.gmail.com
      9cc27566
    • Robert Haas's avatar
      Code review for c94e6942. · 1d5fede4
      Robert Haas authored
      validateCheckConstraint() shouldn't try to access the storage for
      a partitioned table, because it no longer has any.  Creating a
      _RETURN table on a partitioned table shouldn't be allowed, both
      because there's no value in it and because trying to do so would
      involve a validation scan against its nonexistent storage.
      
      Amit Langote, reviewed by Tom Lane.  Regression test outputs
      updated to pass by me.
      
      Discussion: http://postgr.es/m/e5c3cbd3-1551-d6f8-c9e2-51777d632fd2@lab.ntt.co.jp
      1d5fede4
    • Magnus Hagander's avatar
      Fix reversed check of return value from sync · b935eb7d
      Magnus Hagander authored
      While at it also update the comments in walmethods.h to make it less
      likely for mistakes like this to appear in the future (thanks to Tom for
      improvements to the comments).
      
      And finally, in passing change the return type of walmethod.getlasterror
      to being const, also per suggestion from Tom.
      b935eb7d
  4. 11 Apr, 2017 8 commits
    • Tom Lane's avatar
      Remove bogus redefinition of _MSC_VER. · 587d62d8
      Tom Lane authored
      Commit a4777f35 was a shade too mechanical: we don't want to override
      MSVC's own definition of _MSC_VER, as that breaks tests on its numerical
      value.  Per buildfarm.
      587d62d8
    • Tom Lane's avatar
      Simplify handling of remote-qual pass-forward in postgres_fdw. · 88e902b7
      Tom Lane authored
      Commit 0bf3ae88 encountered a need to pass the finally chosen remote qual
      conditions forward from postgresGetForeignPlan to postgresPlanDirectModify.
      It solved that by sticking them into the plan node's fdw_private list,
      which in hindsight was a pretty bad idea.  In the first place, there's no
      use for those qual trees either in EXPLAIN or execution; indeed they could
      never safely be used for any post-planning purposes, because they would not
      get processed by setrefs.c.  So they're just dead weight to carry around in
      the finished plan tree, plus being an attractive nuisance for somebody who
      might get the idea that they could be used that way.  Secondly, because
      those qual trees (sometimes) contained RestrictInfos, they created a
      plan-transmission hazard for parallel query, which is how come we noticed a
      problem.  We dealt with that symptom in commit 28b04787, but really a more
      straightforward and more efficient fix is to pass the data through in a new
      field of struct PgFdwRelationInfo.  So do it that way.  (There's no need
      to revert 28b04787, as it has sufficient reason to live anyway.)
      
      Per fuzz testing by Andreas Seltenreich.
      
      Discussion: https://postgr.es/m/87tw5x4vcu.fsf@credativ.de
      88e902b7
    • Robert Haas's avatar
      Allow a rule on partitioned table to be renamed. · 02af7857
      Robert Haas authored
      Commit f0e44751 should have updated
      this code, but did not.
      
      Amit Langote
      
      Discussion: http://postgr.es/m/52d9c443-ec78-5c8a-7a77-0f34aad12b82@lab.ntt.co.jp
      02af7857
    • Robert Haas's avatar
      Add an Assert() to max_parallel_workers enforcement. · 6599c9ac
      Robert Haas authored
      To prevent future bugs along the lines of the one corrected by commit
      8ff51869, or find any that remain
      in the current code, add an Assert() that the difference between
      parallel_register_count and parallel_terminate_count is in a sane
      range.
      
      Kuntal Ghosh, with considerable tidying-up by me, per a suggestion
      from Neha Khatri.  Reviewed by Tomas Vondra.
      
      Discussion: http://postgr.es/m/CAFO0U+-E8yzchwVnvn5BeRDPgX2z9vZUxQ8dxx9c0XFGBC7N1Q@mail.gmail.com
      6599c9ac
    • Robert Haas's avatar
      Fix confusion of max_parallel_workers mechanism following crash. · 8ff51869
      Robert Haas authored
      Commit b460f5d6 failed to contemplate
      the possibilit that a parallel worker registered before a crash would
      be unregistered only after the crash; if that happened, we'd end up
      with parallel_terminate_count > parallel_register_count and the
      system would refuse to launch any more parallel workers.
      
      The easiest way to fix that seems to be to forget BGW_NEVER_RESTART
      workers in ResetBackgroundWorkerCrashTimes() rather than leaving them
      around to be cleaned up after the conclusion of the restart, so that
      they go away before rather than after shared memory is reset.
      
      To make sure that this fix is water-tight, don't allow parallel
      workers to be anything other than BGW_NEVER_RESTART, so that after
      recovering from a crash, 0 is guaranteed to be the correct starting
      value for parallel_register_count.  The core code wouldn't do this
      anyway, but somebody might try to do it in extension code.
      
      Report by Thomas Vondra.  Patch by me, reviewed by Kuntal Ghosh.
      
      Discussion: http://postgr.es/m/CAGz5QC+AVEVS+3rBKRq83AxkJLMZ1peMt4nnrQwczxOrmo3CNw@mail.gmail.com
      8ff51869
    • Bruce Momjian's avatar
      doc: clearify pg_upgrade default copy behavior · 1e298b8d
      Bruce Momjian authored
      Reported-by: default avatarMarek <marek.cvoren@gmail.com>
      
      Discussion: 20170328110253.2695.62609@wrigleys.postgresql.org
      1e298b8d
    • Robert Haas's avatar
      Fix failure when a shared tidbitmap has only one page. · 4c3b59ab
      Robert Haas authored
      Commit 98e6e890 made inadequate
      provision for the case of a single-page shared tidbitmap.  It
      allocate space for a shared PagetableEntry, but failed to
      initialize it.
      
      Report by Thomas Munro.  Patch by Dilip Kumar, with some comment
      changes by me.
      
      Discussion: http://postgr.es/m/CAEepm=19Cmnfbi-j2Bw-a6yGPeHE1OVhKvvKz9bRBTJGKfGHMA@mail.gmail.com
      4c3b59ab
    • Tom Lane's avatar
      Handle restriction clause lists more uniformly in postgres_fdw. · 28b04787
      Tom Lane authored
      Clauses in the lists retained by postgres_fdw during planning were
      sometimes bare boolean clauses, sometimes RestrictInfos, and sometimes
      a mixture of the two in the same list.  The comment about that situation
      didn't come close to telling the full truth, either.  Aside from being
      confusing, this had a couple of bad practical consequences:
      * waste of planning cycles due to inability to cache per-clause selectivity
      and cost estimates;
      * sometimes, RestrictInfos would sneak into the fdw_private list of a
      finished Plan node, causing failures if, for example, we tried to ship
      the Plan tree to a parallel worker.
      (It may well be that it's a bug in the parallel-query logic that we
      would ever try to ship such a plan to a parallel worker, but in any
      case this deserves to be cleaned up.)
      
      To fix, rearrange so that clause lists in PgFdwRelationInfo are always
      lists of RestrictInfos, and then strip the RestrictInfos at the last
      minute when making a Plan node.  In passing do a bit of refactoring and
      comment cleanup in postgresGetForeignPlan and foreign_join_ok.
      
      Although the messiness here dates back at least to 9.6, there's no evidence
      that it causes anything worse than wasted planning cycles in 9.6, so no
      back-patch for now.
      
      Per fuzz testing by Andreas Seltenreich.
      
      Tom Lane and Ashutosh Bapat
      
      Discussion: https://postgr.es/m/87tw5x4vcu.fsf@credativ.de
      28b04787