1. 06 Dec, 2017 4 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
    • Tom Lane's avatar
      Adjust regression test cases added by commit ab727167. · 979a36c3
      Tom Lane authored
      I suppose it is a copy-and-paste error that this test doesn't actually
      test the "Parallel Append with both partial and non-partial subplans"
      case (EXPLAIN alone surely doesn't qualify as a test of executor
      behavior).  Fix that.
      
      Also, add cosmetic aliases to make it possible to tell apart these
      otherwise-identical test cases in log_statement output.
      979a36c3
    • Peter Eisentraut's avatar
      doc: Flex is not a GNU package · 51cff91c
      Peter Eisentraut authored
      Remove the designation that Flex is a GNU package.  Even though Bison is
      a GNU package, leave out the designation to not make the sentence
      unnecessarily complicated.
      
      Author: Pavan Maddamsetti <pavan.maddamsetti@gmail.com>
      51cff91c
  2. 05 Dec, 2017 11 commits
    • Tom Lane's avatar
      Fix broken markup. · 7404704a
      Tom Lane authored
      7404704a
    • Robert Haas's avatar
      Support Parallel Append plan nodes. · ab727167
      Robert Haas authored
      When we create an Append node, we can spread out the workers over the
      subplans instead of piling on to each subplan one at a time, which
      should typically be a bit more efficient, both because the startup
      cost of any plan executed entirely by one worker is paid only once and
      also because of reduced contention.  We can also construct Append
      plans using a mix of partial and non-partial subplans, which may allow
      for parallelism in places that otherwise couldn't support it.
      Unfortunately, this patch doesn't handle the important case of
      parallelizing UNION ALL by running each branch in a separate worker;
      the executor infrastructure is added here, but more planner work is
      needed.
      
      Amit Khandekar, Robert Haas, Amul Sul, reviewed and tested by
      Ashutosh Bapat, Amit Langote, Rafia Sabih, Amit Kapila, and
      Rajkumar Raghuwanshi.
      
      Discussion: http://postgr.es/m/CAJ3gD9dy0K_E8r727heqXoBmWZ83HwLFwdcaSSmBQ1+S+vRuUQ@mail.gmail.com
      ab727167
    • Peter Eisentraut's avatar
      doc: Update memory requirements for FOP · 8097d189
      Peter Eisentraut authored
      Reported-by: default avatarDave Page <dpage@pgadmin.org>
      8097d189
    • Robert Haas's avatar
      Fix accumulation of parallel worker instrumentation. · 2c09a5c1
      Robert Haas authored
      When a Gather or Gather Merge node is started and stopped multiple
      times, the old code wouldn't reset the shared state between executions,
      potentially resulting in dramatically inflated instrumentation data
      for nodes beneath it.  (The per-worker instrumentation ended up OK,
      I think, but the overall totals were inflated.)
      
      Report by hubert depesz lubaczewski.  Analysis and fix by Amit Kapila,
      reviewed and tweaked a bit by me.
      
      Discussion: http://postgr.es/m/20171127175631.GA405@depesz.com
      2c09a5c1
    • Andres Freund's avatar
      Fix EXPLAIN ANALYZE of hash join when the leader doesn't participate. · 5bcf389e
      Andres Freund authored
      If a hash join appears in a parallel query, there may be no hash table
      available for explain.c to inspect even though a hash table may have
      been built in other processes.  This could happen either because
      parallel_leader_participation was set to off or because the leader
      happened to hit the end of the outer relation immediately (even though
      the complete relation is not empty) and decided not to build the hash
      table.
      
      Commit bf11e7ee introduced a way for workers to exchange
      instrumentation via the DSM segment for Sort nodes even though they
      are not parallel-aware.  This commit does the same for Hash nodes, so
      that explain.c has a way to find instrumentation data from an
      arbitrary participant that actually built the hash table.
      
      Author: Thomas Munro
      Reviewed-By: Andres Freund
      Discussion: https://postgr.es/m/CAEepm%3D3DUQC2-z252N55eOcZBer6DPdM%3DFzrxH9dZc5vYLsjaA%40mail.gmail.com
      5bcf389e
    • Robert Haas's avatar
      postgres_fdw: Fix failing regression test. · 82c5c533
      Robert Haas authored
      Commit ab3f008a broke this.
      
      Report by Stephen Frost.
      
      Discussion: http://postgr.es/m/20171205180342.GO4628@tamriel.snowman.net
      82c5c533
    • Robert Haas's avatar
      postgres_fdw: Judge password use by run-as user, not session user. · ab3f008a
      Robert Haas authored
      This is a backward incompatibility which should be noted in the
      release notes for PostgreSQL 11.
      
      For security reasons, we require that a postgres_fdw foreign table use
      password authentication when accessing a remote server, so that an
      unprivileged user cannot usurp the server's credentials.  Superusers
      are exempt from this requirement, because we assume they are entitled
      to usurp the server's credentials or, at least, can find some other
      way to do it.
      
      But what should happen when the foreign table is accessed by a view
      owned by a user different from the session user?  Is it the view owner
      that must be a superuser in order to avoid the requirement of using a
      password, or the session user?  Historically it was the latter, but
      this requirement makes it the former instead.  This allows superusers
      to delegate to other users the right to select from a foreign table
      that doesn't use password authentication by creating a view over the
      foreign table and handing out rights to the view.  It is also more
      consistent with the idea that access to a view should use the view
      owner's privileges rather than the session user's privileges.
      
      The upshot of this change is that a superuser selecting from a view
      created by a non-superuser may now get an error complaining that no
      password was used, while a non-superuser selecting from a view
      created by a superuser will no longer receive such an error.
      
      No documentation changes are present in this patch because the
      wording of the documentation already suggests that it works this
      way.  We should perhaps adjust the documentation in the back-branches,
      but that's a task for another patch.
      
      Originally proposed by Jeff Janes, but with different semantics;
      adjusted to work like this by me per discussion.
      
      Discussion: http://postgr.es/m/CA+TgmoaY4HsVZJv5SqEjCKLDwtCTSwXzKpRftgj50wmMMBwciA@mail.gmail.com
      ab3f008a
    • Robert Haas's avatar
      Mark assorted variables PGDLLIMPORT. · c572599c
      Robert Haas authored
      This makes life easier for extension authors who wish to support
      Windows.
      
      Brian Cloutier, slightly amended by me.
      
      Discussion: http://postgr.es/m/CAJCy68fscdNhmzFPS4kyO00CADkvXvEa-28H-OtENk-pa2OTWw@mail.gmail.com
      c572599c
    • Peter Eisentraut's avatar
      doc: Turn on generate.consistent.ids parameter · 28f8896a
      Peter Eisentraut authored
      This ensures that automatically generated HTML anchors don't change in
      every build.
      28f8896a
    • Tom Lane's avatar
      Treat directory open failures as hard errors in ResetUnloggedRelations(). · 8dc3c971
      Tom Lane authored
      Previously, this code just reported such problems at LOG level and kept
      going.  The problem with this approach is that transient failures (e.g.,
      ENFILE) could prevent us from resetting unlogged relations to empty,
      yet allow recovery to appear to complete successfully.  That seems like
      a data corruption hazard large enough to treat such problems as reasons
      to fail startup.
      
      For the same reason, treat unlink failures for unlogged files as hard
      errors not just LOG messages.  It's a little odd that we did it like that
      when file-level errors in other steps (copy_file, fsync_fname) are ERRORs.
      
      The sole case that I left alone is that ENOENT failure on a tablespace
      (not database) directory is not an error, though it will now be logged
      rather than just silently ignored.  This is to cover the scenario where
      a previous DROP TABLESPACE removed the tablespace directory but failed
      before removing the pg_tblspc symlink.  I'm not sure that that's very
      likely in practice, but that seems like the only real excuse for the
      old behavior here, so let's allow for it.  (As coded, this will also
      allow ENOENT on $PGDATA/base/.  But since we'll fail soon enough if
      that's gone, I don't think we need to complicate this code by
      distinguishing that from a true tablespace case.)
      
      Discussion: https://postgr.es/m/21040.1512418508@sss.pgh.pa.us
      8dc3c971
    • Peter Eisentraut's avatar
      Fix warnings from cpluspluscheck · e7cfb26f
      Peter Eisentraut authored
      Fix warnings about "comparison between signed and unsigned integer
      expressions" in inline functions in header files by adding some casts.
      e7cfb26f
  3. 04 Dec, 2017 6 commits
    • Tom Lane's avatar
      Simplify do_pg_start_backup's API by opening pg_tblspc internally. · 066bc21c
      Tom Lane authored
      do_pg_start_backup() expects its callers to pass in an open DIR pointer
      for the pg_tblspc directory, but there's no apparent advantage in that.
      It complicates the callers without adding any flexibility, and there's no
      robustness advantage, since we surely have to be prepared for errors during
      the scan of pg_tblspc anyway.  In fact, by holding an extra kernel resource
      during operations like the preliminary checkpoint, we might be making
      things a fraction more failure-prone not less.  Hence, remove that argument
      and open the directory just for the duration of the actual scan.
      
      Discussion: https://postgr.es/m/28752.1512413887@sss.pgh.pa.us
      066bc21c
    • Tom Lane's avatar
      Improve error handling in RemovePgTempFiles(). · 561885db
      Tom Lane authored
      Modify this function and its subsidiaries so that syscall failures are
      reported via ereport(LOG), rather than silently ignored as before.
      We don't want to throw a hard ERROR, as that would prevent database
      startup, and getting rid of leftover temporary files is not important
      enough for that.  On the other hand, not reporting trouble at all
      seems like an odd choice not in line with current project norms,
      especially since any failure here is quite unexpected.
      
      On the same reasoning, adjust these functions' AllocateDir/ReadDir calls
      so that failure to scan a directory results in LOG not ERROR.  I also
      removed the previous practice of silently ignoring ENOENT failures during
      directory opens --- there are some corner cases where that could happen
      given a previous database crash, but that seems like a bad excuse for
      ignoring a condition that isn't expected in most cases.  A LOG message
      during postmaster start seems OK in such situations, and better than
      no output at all.
      
      In passing, make RemovePgTempRelationFiles' test for "is the file name
      all digits" look more like the way it's done elsewhere.
      
      Discussion: https://postgr.es/m/19907.1512402254@sss.pgh.pa.us
      561885db
    • Tom Lane's avatar
      Clean up assorted messiness around AllocateDir() usage. · 2069e6fa
      Tom Lane authored
      This patch fixes a couple of low-probability bugs that could lead to
      reporting an irrelevant errno value (and hence possibly a wrong SQLSTATE)
      concerning directory-open or file-open failures.  It also fixes places
      where we took shortcuts in reporting such errors, either by using elog
      instead of ereport or by using ereport but forgetting to specify an
      errcode.  And it eliminates a lot of just plain redundant error-handling
      code.
      
      In service of all this, export fd.c's formerly-static function
      ReadDirExtended, so that external callers can make use of the coding
      pattern
      
      	dir = AllocateDir(path);
      	while ((de = ReadDirExtended(dir, path, LOG)) != NULL)
      
      if they'd like to treat directory-open failures as mere LOG conditions
      rather than errors.  Also fix FreeDir to be a no-op if we reach it
      with dir == NULL, as such a coding pattern would cause.
      
      Then, remove code at many call sites that was throwing an error or log
      message for AllocateDir failure, as ReadDir or ReadDirExtended can handle
      that job just fine.  Aside from being a net code savings, this gets rid of
      a lot of not-quite-up-to-snuff reports, as mentioned above.  (In some
      places these changes result in replacing a custom error message such as
      "could not open tablespace directory" with more generic wording "could not
      open directory", but it was agreed that the custom wording buys little as
      long as we report the directory name.)  In some other call sites where we
      can't just remove code, change the error reports to be fully
      project-style-compliant.
      
      Also reorder code in restoreTwoPhaseData that was acquiring a lock
      between AllocateDir and ReadDir; in the unlikely but surely not
      impossible case that LWLockAcquire changes errno, AllocateDir failures
      would be misreported.  There is no great value in opening the directory
      before acquiring TwoPhaseStateLock, so just do it in the other order.
      
      Also fix CheckXLogRemoved to guarantee that it preserves errno,
      as quite a number of call sites are implicitly assuming.  (Again,
      it's unlikely but I think not impossible that errno could change
      during a SpinLockAcquire.  If so, this function was broken for its
      own purposes as well as breaking callers.)
      
      And change a few places that were using not-per-project-style messages,
      such as "could not read directory" when "could not open directory" is
      more correct.
      
      Back-patch the exporting of ReadDirExtended, in case we have occasion
      to back-patch some fix that makes use of it; it's not needed right now
      but surely making it global is pretty harmless.  Also back-patch the
      restoreTwoPhaseData and CheckXLogRemoved fixes.  The rest of this is
      essentially cosmetic and need not get back-patched.
      
      Michael Paquier, with a bit of additional work by me
      
      Discussion: https://postgr.es/m/CAB7nPqRpOCxjiirHmebEFhXVTK7V5Jvw4bz82p7Oimtsm3TyZA@mail.gmail.com
      2069e6fa
    • Robert Haas's avatar
      When VACUUM or ANALYZE skips a concurrently dropped table, log it. · ab6eaee8
      Robert Haas authored
      Hopefully, the additional logging will help avoid confusion that
      could otherwise result.
      
      Nathan Bossart, reviewed by Michael Paquier, Fabrízio Mello, and me
      ab6eaee8
    • Tom Lane's avatar
      Support boolean columns in functional-dependency statistics. · ecc27d55
      Tom Lane authored
      There's no good reason that the multicolumn stats stuff shouldn't work on
      booleans.  But it looked only for "Var = pseudoconstant" clauses, and it
      will seldom find those for boolean Vars, since earlier phases of planning
      will fold "boolvar = true" or "boolvar = false" to just "boolvar" or
      "NOT boolvar" respectively.  Improve dependencies_clauselist_selectivity()
      to recognize such clauses as equivalent to equality restrictions.
      
      This fixes a failure of the extended stats mechanism to apply in a case
      reported by Vitaliy Garnashevich.  It's not a complete solution to his
      problem because the bitmap-scan costing code isn't consulting extended
      stats where it should, but that's surely an independent issue.
      
      In passing, improve some comments, get rid of a NumRelids() test that's
      redundant with the preceding bms_membership() test, and fix
      dependencies_clauselist_selectivity() so that estimatedclauses actually
      is a pure output argument as stated by its API contract.
      
      Back-patch to v10 where this code was introduced.
      
      Discussion: https://postgr.es/m/73a4936d-2814-dc08-ed0c-978f76f435b0@gmail.com
      ecc27d55
    • Robert Haas's avatar
      Remove memory leak protection from Gather and Gather Merge nodes. · 9f4992e2
      Robert Haas authored
      Before commit 6b65a7fe, tqueue.c could
      perform tuple remapping and thus leak memory, which is why commit
      af330393 made TupleQueueReaderNext
      run in a short-lived context.  Now, however, tqueue.c has been reduced
      to a shadow of its former self, and there shouldn't be any chance of
      leaks any more.  Accordingly, remove some tuple copying and memory
      context manipulation to speed up processing.
      
      Patch by me, reviewed by Amit Kapila.  Some testing by Rafia Sabih.
      
      Discussion: http://postgr.es/m/CAA4eK1LSDydwrNjmYSNkfJ3ZivGSWH9SVswh6QpNzsMdj_oOQA@mail.gmail.com
      9f4992e2
  4. 03 Dec, 2017 1 commit
  5. 02 Dec, 2017 2 commits
  6. 01 Dec, 2017 7 commits
  7. 30 Nov, 2017 7 commits
    • Robert Haas's avatar
      Remove extra word from comment. · 06ae669c
      Robert Haas authored
      David Rowley, who also was the primary author of the patch that
      added this function; the attribution in my previous commit,
      84940644, was incorrect due to
      sloppiness on my part.
      
      Discussion: http://postgr.es/m/CAKJS1f_0iSiLQsf_c06AzOWAc3eS6ePjjVQFpcFv3W-O5aktnQ@mail.gmail.com
      06ae669c
    • Peter Eisentraut's avatar
      SQL procedures · e4128ee7
      Peter Eisentraut authored
      This adds a new object type "procedure" that is similar to a function
      but does not have a return type and is invoked by the new CALL statement
      instead of SELECT or similar.  This implementation is aligned with the
      SQL standard and compatible with or similar to other SQL implementations.
      
      This commit adds new commands CALL, CREATE/ALTER/DROP PROCEDURE, as well
      as ALTER/DROP ROUTINE that can refer to either a function or a
      procedure (or an aggregate function, as an extension to SQL).  There is
      also support for procedures in various utility commands such as COMMENT
      and GRANT, as well as support in pg_dump and psql.  Support for defining
      procedures is available in all the languages supplied by the core
      distribution.
      
      While this commit is mainly syntax sugar around existing functionality,
      future features will rely on having procedures as a separate object
      type.
      Reviewed-by: default avatarAndrew Dunstan <andrew.dunstan@2ndquadrant.com>
      e4128ee7
    • Robert Haas's avatar
      Make create_unique_path manage memory like mark_dummy_rel. · 1761653b
      Robert Haas authored
      Put the unique path in the same context as the owning RelOptInfo, rather
      than the toplevel planner context.  This is how this function worked
      originally, but commit f41803bb
      changed it without explanation.  mark_dummy_rel adopted the older (or
      newer?) technique in commit eca75a12,
      which also featured a much better explanation of why it is correct.
      So, switch back to that technique here, with the same explanation
      given there.
      
      Although this fixes a possible memory leak when GEQO is in use, the
      leak is minor and probably nobody cares, so no back-patch.
      
      Ashutosh Bapat, reviewed by Tom Lane and by me
      
      Discussion: http://postgr.es/m/CAFjFpRcXkHHrXyD9BCvkgGJV4TnHG2SWJ0PhJfrDu3NAcQvh7g@mail.gmail.com
      1761653b
    • Noah Misch's avatar
      Fix non-GNU makefiles for AIX make. · e21a556e
      Noah Misch authored
      Invoking the Makefile without an explicit target was building every
      possible target instead of just the "all" target.  Back-patch to 9.3
      (all supported versions).
      e21a556e
    • Tom Lane's avatar
      Fix neqjoinsel's behavior for semi/anti join cases. · 7ca25b7d
      Tom Lane authored
      Previously, this function estimated the selectivity as 1 minus eqjoinsel()
      for the negator equality operator, regardless of join type (I think there
      was an expectation that eqjoinsel would handle the join type).  But
      actually this is completely wrong for semijoin cases: the fraction of the
      LHS that has a non-matching row is not one minus the fraction of the LHS
      that has a matching row.  In reality a semijoin with <> will nearly always
      succeed: it can only fail when the RHS is empty, or it contains a single
      distinct value that is equal to the particular LHS value, or the LHS value
      is null.  The only one of those things we should have much confidence in
      estimating is the fraction of LHS values that are null, so let's just take
      the selectivity as 1 minus outer nullfrac.
      
      Per coding convention, antijoin should be estimated the same as semijoin.
      
      Arguably this is a bug fix, but in view of the lack of field complaints
      and the risk of destabilizing plans, no back-patch.
      
      Thomas Munro, reviewed by Ashutosh Bapat
      
      Discussion: https://postgr.es/m/CAEepm=270ze2hVxWkJw-5eKzc3AB4C9KpH3L2kih75R5pdSogg@mail.gmail.com
      7ca25b7d
    • Andres Freund's avatar
      Add a barrier primitive for synchronizing backends. · 1145acc7
      Andres Freund authored
      Provide support for dynamic or static parties of processes to wait for
      all processes to reach point in the code before continuing.
      
      This is similar to the mechanism of the same name in POSIX threads and
      MPI, though has explicit phasing and dynamic party support like the
      Java core library's Phaser.
      
      This will be used by an upcoming patch adding support for parallel
      hash joins.
      
      Author: Thomas Munro
      Reviewed-By: Andres Freund
      Discussion: https://postgr.es/m/CAEepm=2_y7oi01OjA_wLvYcWMc9_d=LaoxrY3eiROCZkB_qakA@mail.gmail.com
      1145acc7
    • Andres Freund's avatar
      Add some regression tests that exercise hash join code. · fa330f9a
      Andres Freund authored
      Although hash joins are already tested by many queries, these tests
      systematically cover the four different states we can reach as part of
      the strategy for respecting work_mem.
      
      Author: Thomas Munro
      Reviewed-By: Andres Freund
      fa330f9a
  8. 29 Nov, 2017 2 commits
    • Robert Haas's avatar
      New C function: bms_add_range · 84940644
      Robert Haas authored
      This will be used by pending patches to improve partition pruning.
      
      Amit Langote and Kyotaro Horiguchi, per a suggestion from David
      Rowley.  Review and testing of the larger patch set of which this is a
      part by Ashutosh Bapat, David Rowley, Dilip Kumar, Jesper Pedersen,
      Rajkumar Raghuwanshi, Beena Emerson, Amul Sul, and Kyotaro Horiguchi.
      
      Discussion: http://postgr.es/m/098b9c71-1915-1a2a-8d52-1a7a50ce79e8@lab.ntt.co.jp
      84940644
    • Robert Haas's avatar
      Add extensive tests for partition pruning. · 8d4e70a6
      Robert Haas authored
      Currently, partition pruning happens via constraint exclusion, but
      there are pending places to replace that with a different and
      hopefully faster mechanism.  To be sure that we don't change behavior
      without realizing it, add extensive test coverage.
      
      Note that not all of these behaviors are optimal; in some cases,
      partitions are not pruned even though it would be safe to do so.
      These tests therefore serve to memorialize the current state rather
      than the ideal state.  Patches that improve things can update the test
      results as appropriate.
      
      Amit Langote, adjusted by me.  Review and testing of the larger patch
      set of which this is a part by Ashutosh Bapat, David Rowley, Dilip
      Kumar, Jesper Pedersen, Rajkumar Raghuwanshi, Beena Emerson, Amul Sul,
      and Kyotaro Horiguchi.
      
      Discussion: http://postgr.es/m/098b9c71-1915-1a2a-8d52-1a7a50ce79e8@lab.ntt.co.jp
      8d4e70a6