1. 28 Aug, 2016 2 commits
  2. 27 Aug, 2016 1 commit
    • Tom Lane's avatar
      Add macros to make AllocSetContextCreate() calls simpler and safer. · ea268cdc
      Tom Lane authored
      I found that half a dozen (nearly 5%) of our AllocSetContextCreate calls
      had typos in the context-sizing parameters.  While none of these led to
      especially significant problems, they did create minor inefficiencies,
      and it's now clear that expecting people to copy-and-paste those calls
      accurately is not a great idea.  Let's reduce the risk of future errors
      by introducing single macros that encapsulate the common use-cases.
      Three such macros are enough to cover all but two special-purpose contexts;
      those two calls can be left as-is, I think.
      
      While this patch doesn't in itself improve matters for third-party
      extensions, it doesn't break anything for them either, and they can
      gradually adopt the simplified notation over time.
      
      In passing, change TopMemoryContext to use the default allocation
      parameters.  Formerly it could only be extended 8K at a time.  That was
      probably reasonable when this code was written; but nowadays we create
      many more contexts than we did then, so that it's not unusual to have a
      couple hundred K in TopMemoryContext, even without considering various
      dubious code that sticks other things there.  There seems no good reason
      not to let it use growing blocks like most other contexts.
      
      Back-patch to 9.6, mostly because that's still close enough to HEAD that
      it's easy to do so, and keeping the branches in sync can be expected to
      avoid some future back-patching pain.  The bugs fixed by these changes
      don't seem to be significant enough to justify fixing them further back.
      
      Discussion: <21072.1472321324@sss.pgh.pa.us>
      ea268cdc
  3. 26 Aug, 2016 6 commits
    • Tom Lane's avatar
      Add a nonlocalized version of the severity field to client error messages. · 26fa446d
      Tom Lane authored
      This has been requested a few times, but the use-case for it was never
      entirely clear.  The reason for adding it now is that transmission of
      error reports from parallel workers fails when NLS is active, because
      pq_parse_errornotice() wrongly assumes that the existing severity field
      is nonlocalized.  There are other ways we could have fixed that, but the
      other options were basically kluges, whereas this way provides something
      that's at least arguably a useful feature along with the bug fix.
      
      Per report from Jakob Egger.  Back-patch into 9.6, because otherwise
      parallel query is essentially unusable in non-English locales.  The
      problem exists in 9.5 as well, but we don't want to risk changing
      on-the-wire behavior in 9.5 (even though the possibility of new error
      fields is specifically called out in the protocol document).  It may
      be sufficient to leave the issue unfixed in 9.5, given the very limited
      usefulness of pq_parse_errornotice in that version.
      
      Discussion: <A88E0006-13CB-49C6-95CC-1A77D717213C@eggerapps.at>
      26fa446d
    • Tom Lane's avatar
      Fix potential memory leakage from HandleParallelMessages(). · 78dcd027
      Tom Lane authored
      HandleParallelMessages leaked memory into the caller's context.  Since it's
      called from ProcessInterrupts, there is basically zero certainty as to what
      CurrentMemoryContext is, which means we could be leaking into long-lived
      contexts.  Over the processing of many worker messages that would grow to
      be a problem.  Things could be even worse than just a leak, if we happened
      to service the interrupt while ErrorContext is current: elog.c thinks it
      can reset that on its own whim, possibly yanking storage out from under
      HandleParallelMessages.
      
      Give HandleParallelMessages its own dedicated context instead, which we can
      reset during each call to ensure there's no accumulation of wasted memory.
      
      Discussion: <16610.1472222135@sss.pgh.pa.us>
      78dcd027
    • Tom Lane's avatar
      Put static forward declarations in elog.c back into same order as code. · 45a36e68
      Tom Lane authored
      The guiding principle for the last few patches in this area apparently
      involved throwing darts.
      
      Cosmetic only, but back-patch to 9.6 because there is no reason for
      9.6 and HEAD to diverge yet in this file.
      45a36e68
    • Tom Lane's avatar
      Fix assorted small bugs in ThrowErrorData(). · 8529036b
      Tom Lane authored
      Copy the palloc'd strings into the correct context, ie ErrorContext
      not wherever the source ErrorData is.  This would be a large bug,
      except that it appears that all catchers of thrown errors do either
      EmitErrorReport or CopyErrorData before doing anything that would
      cause transient memory contexts to be cleaned up.  Still, it's wrong
      and it will bite somebody someday.
      
      Fix failure to copy cursorpos and internalpos.
      
      Utter the appropriate incantations involving recursion_depth, so that
      we'll behave sanely if we get an error inside pstrdup.  (In general,
      the body of this function ought to act like, eg, errdetail().)
      
      Per code reading induced by Jakob Egger's report.
      8529036b
    • Tom Lane's avatar
      Fix logic for adding "parallel worker" context line to worker errors. · fbf28b6b
      Tom Lane authored
      The previous coding here was capable of adding a "parallel worker" context
      line to errors that were not, in fact, returned from a parallel worker.
      Instead of using an errcontext callback to add that annotation, just paste
      it onto the message by hand; this looks uglier but is more reliable.
      
      Discussion: <19757.1472151987@sss.pgh.pa.us>
      fbf28b6b
    • Heikki Linnakangas's avatar
      Support OID system column in postgres_fdw. · ae025a15
      Heikki Linnakangas authored
      You can use ALTER FOREIGN TABLE SET WITH OIDS on a foreign table, but the
      oid column read out as zeros, because the postgres_fdw didn't know about
      it. Teach postgres_fdw how to fetch it.
      
      Etsuro Fujita, with an additional test case by me.
      
      Discussion: <56E90A76.5000503@lab.ntt.co.jp>
      ae025a15
  4. 25 Aug, 2016 3 commits
    • Tom Lane's avatar
      Fix instability in parallel regression tests. · 2533ff0a
      Tom Lane authored
      Commit f0c7b789 added a test case in case.sql that creates and then drops
      both an '=' operator and the type it's for.  Given the right timing, that
      can cause a "cache lookup failed for type" failure in concurrent sessions,
      which see the '=' operator as a potential match for '=' in a query, but
      then the type is gone by the time they inquire into its properties.
      It might be nice to make that behavior more robust someday, but as a
      back-patchable solution, adjust the new test case so that the operator
      is never visible to other sessions.  Like the previous commit, back-patch
      to all supported branches.
      
      Discussion: <5983.1471371667@sss.pgh.pa.us>
      2533ff0a
    • Tom Lane's avatar
      Fix small query-lifespan memory leak in bulk updates. · ae4760d6
      Tom Lane authored
      When there is an identifiable REPLICA IDENTITY index on the target table,
      heap_update leaks the id_attrs bitmapset.  That's not many bytes, but it
      adds up over enough rows, since the code typically runs in a query-lifespan
      context.  Bug introduced in commit e55704d8, which did a rather poor job
      of cloning the existing use-pattern for RelationGetIndexAttrBitmap().
      
      Per bug #14293 from Zhou Digoal.  Back-patch to 9.4 where the bug was
      introduced.
      
      Report: <20160824114320.15676.45171@wrigleys.postgresql.org>
      ae4760d6
    • Bruce Momjian's avatar
      doc: more replacement of <literal> with something better · ca9cb940
      Bruce Momjian authored
      Reported-by: Alexander Law
      
      Author: Alexander Law
      
      Backpatch-through: 9.6
      ca9cb940
  5. 24 Aug, 2016 7 commits
    • Robert Haas's avatar
      postgres_fdw: Cosmetic cleanup. · dcb7a54b
      Robert Haas authored
      Etsuro Fujita
      dcb7a54b
    • Tom Lane's avatar
      Fix improper repetition of previous results from a hashed aggregate. · 2c00fad2
      Tom Lane authored
      ExecReScanAgg's check for whether it could re-use a previously calculated
      hashtable neglected the possibility that the Agg node might reference
      PARAM_EXEC Params that are not referenced by its input plan node.  That's
      okay if the Params are in upper tlist or qual expressions; but if one
      appears in aggregate input expressions, then the hashtable contents need
      to be recomputed when the Param's value changes.
      
      To avoid unnecessary performance degradation in the case of a Param that
      isn't within an aggregate input, add logic to the planner to determine
      which Params are within aggregate inputs.  This requires a new field in
      struct Agg, but fortunately we never write plans to disk, so this isn't
      an initdb-forcing change.
      
      Per report from Jeevan Chalke.  This has been broken since forever,
      so back-patch to all supported branches.
      
      Andrew Gierth, with minor adjustments by me
      
      Report: <CAM2+6=VY8ykfLT5Q8vb9B6EbeBk-NGuLbT6seaQ+Fq4zXvrDcA@mail.gmail.com>
      2c00fad2
    • Kevin Grittner's avatar
      Remove unnecessary #include. · 5cd38640
      Kevin Grittner authored
      Accidentally added in 8b65cf4c.
      
      Pointed out by Álvaro Herrera
      5cd38640
    • Peter Eisentraut's avatar
      doc: Fix XSLT speedup with older upstream stylesheet versions · 0e4cc1fc
      Peter Eisentraut authored
      From: Alexander Law <exclusion@gmail.com>
      0e4cc1fc
    • Noah Misch's avatar
      Build libpgfeutils before src/bin/pg_basebackup programs. · 03951987
      Noah Misch authored
      Oversight in commit 9132c014.
      03951987
    • Noah Misch's avatar
      Build libpgfeutils before pg_isready. · b6418a09
      Noah Misch authored
      Every program having -lpgfeutils in LDFLAGS must have this dependency,
      whether or not the program uses a libpgfeutils symbol.  Back-patch to
      9.6, where libpgfeutils was introduced.
      b6418a09
    • Tom Lane's avatar
      Suppress compiler warnings in non-cassert builds. · 71e006f0
      Tom Lane authored
      With Asserts off, these variables are set but never used, resulting
      in warnings from pickier compilers.  Fix that with our standard solution.
      Per report from Jeff Janes.
      71e006f0
  6. 23 Aug, 2016 9 commits
    • Tom Lane's avatar
      Fix network_spgist.c build failures from missing AF_INET definition. · 32909a57
      Tom Lane authored
      AF_INET is apparently defined in something that's pulled in automatically
      on Linux, but the buildfarm says that's not true everywhere.  Comparing
      to network_gist.c suggests that including <sys/socket.h> ought to fix it,
      and the POSIX standard concurs.
      32909a57
    • Tom Lane's avatar
      Create an SP-GiST opclass for inet/cidr. · 77e29068
      Tom Lane authored
      This seems to offer significantly better search performance than the
      existing GiST opclass for inet/cidr, at least on data with a wide mix
      of network mask lengths.  (That may suggest that the data splitting
      heuristics in the GiST opclass could be improved.)
      
      Emre Hasegeli, with mostly-cosmetic adjustments by me
      
      Discussion: <CAE2gYzxtth9qatW_OAqdOjykS0bxq7AYHLuyAQLPgT7H9ZU0Cw@mail.gmail.com>
      77e29068
    • Robert Haas's avatar
      Extend dsm API with a new function dsm_unpin_segment. · 0fda682e
      Robert Haas authored
      If you have previously pinned a segment and decide that you don't
      actually want to keep it around until shutdown, this new API lets you
      remove the pin.  This is pretty trivial except on Windows, where it
      requires closing the duplicate handle that was used to implement the
      pin.
      
      Thomas Munro and Amit Kapila, reviewed by Amit Kapila and by me.
      0fda682e
    • Robert Haas's avatar
      Remove duplicate function prototype. · 19998730
      Robert Haas authored
      Kyotaro Horiguchi
      19998730
    • Bruce Momjian's avatar
      doc: fix incorrect 'literal' tags · ff066481
      Bruce Momjian authored
      Discussion: dcc4113d-1eda-4f60-d1c5-f50eee160bad@gmail.com
      
      Author: Alexander Law <exclusion@gmail.com>
      
      Backpatch-through: 9.6
      ff066481
    • Tom Lane's avatar
      Improve SP-GiST opclass API to better support unlabeled nodes. · d2ddee63
      Tom Lane authored
      Previously, the spgSplitTuple action could only create a new upper tuple
      containing a single labeled node.  This made it useless for opclasses
      that prefer to work with fixed sets of nodes (labeled or otherwise),
      which meant that restrictive prefixes could not be used with such
      node definitions.  Change the output field set for the choose() method
      to allow it to specify any valid node set for the new upper tuple,
      and to specify which of these nodes to place the modified lower tuple in.
      
      In addition to its primary use for fixed node sets, this feature could
      allow existing opclasses that use variable node sets to skip a separate
      spgAddNode action when splitting a tuple, by setting up the node needed
      for the incoming value as part of the spgSplitTuple action.  However, care
      would have to be taken to add the extra node only when it would not make
      the tuple bigger than before.  (spgAddNode can enlarge the tuple,
      spgSplitTuple can't.)
      
      This is a prerequisite for an upcoming SP-GiST inet opclass, but is
      being committed separately to increase the visibility of the API change.
      
      In passing, improve the documentation about the traverse-values feature
      that was added by commit ccd6eb49.
      
      Emre Hasegeli, with cosmetic adjustments and documentation rework by me
      
      Discussion: <CAE2gYzxtth9qatW_OAqdOjykS0bxq7AYHLuyAQLPgT7H9ZU0Cw@mail.gmail.com>
      d2ddee63
    • Robert Haas's avatar
      Add txid_current_ifassigned(). · 86f31695
      Robert Haas authored
      Add a variant of txid_current() that returns NULL if no transaction ID
      is assigned.  This version can be used even on a standby server,
      although it will always return NULL since no transaction IDs can be
      assigned during recovery.
      
      Craig Ringer, per suggestion from Jim Nasby.  Reviewed by Petr Jelinek
      and by me.
      86f31695
    • Robert Haas's avatar
      Remove duplicate word from comment. · ff36700c
      Robert Haas authored
      Erik Rijkers
      ff36700c
    • Tom Lane's avatar
      Refactor some network.c code to create cidr_set_masklen_internal(). · 7b405b3e
      Tom Lane authored
      Merge several copies of "copy an inet value and adjust the mask length"
      code to create a single, conveniently C-callable function.  This function
      is exported for future use by inet SPGiST support, but it's good cleanup
      anyway since we had three slightly-different-for-no-good-reason copies.
      
      (Extracted from a larger patch, to separate new code from refactoring
      of old code)
      
      Emre Hasegeli
      7b405b3e
  7. 22 Aug, 2016 6 commits
  8. 20 Aug, 2016 2 commits
    • Tom Lane's avatar
      initdb now needs to reference libpq include files in MSVC builds. · 04164deb
      Tom Lane authored
      Fallout from commit a00c5831.  Per buildfarm.
      04164deb
    • Tom Lane's avatar
      Make initdb's suggested "pg_ctl start" command line more reliable. · a00c5831
      Tom Lane authored
      The original coding here was not nearly careful enough about quoting
      special characters, and it didn't get corner cases right for constructing
      the pg_ctl path either.  Use join_path_components() and appendShellString()
      to do it honestly, so that the string will more likely work if blindly
      copied-and-pasted.
      
      While at it, teach appendShellString() not to quote strings that clearly
      don't need it, so that the output from initdb doesn't become uglier than
      it was before in typical cases where quoting is not needed.
      
      Ryan Murphy, reviewed by Michael Paquier and myself
      
      Discussion: <CAHeEsBeAe1FeBypT3E8R1ZVZU0e8xv3A-7BHg6bEOi=jZny2Uw@mail.gmail.com>
      a00c5831
  9. 19 Aug, 2016 4 commits
    • Tom Lane's avatar
      Allow empty queries in pgbench. · 64710452
      Tom Lane authored
      This might have been too much of a foot-gun before 9.6, but with the
      new commands-end-at-semicolons parsing rule, the only way to get an
      empty query into a script is to explicitly write an extra ";".
      So we may as well allow the case.
      
      Fabien Coelho
      
      Patch: <alpine.DEB.2.20.1607090922170.3412@sto>
      64710452
    • Tom Lane's avatar
      Use LEFT JOINs in some system views in case referenced row doesn't exist. · 8299471c
      Tom Lane authored
      In particular, left join to pg_authid so that rows in pg_stat_activity
      don't disappear if the session's owning user has been dropped.
      Also convert a few joins to pg_database to left joins, in the same spirit,
      though that case might be harder to hit.  We were doing this in other
      views already, so it was a bit inconsistent that these views didn't.
      
      Oskari Saarenmaa, with some further tweaking by me
      
      Discussion: <56E87CD8.60007@ohmu.fi>
      8299471c
    • Tom Lane's avatar
      Guard against parallel-restricted functions in VALUES expressions. · 65a603e9
      Tom Lane authored
      Obvious brain fade in set_rel_consider_parallel().  Noticed it while
      adjusting the adjacent RTE_FUNCTION case.
      
      In 9.6, also make the code look more like what I just did in HEAD
      by removing the unnecessary function_rte_parallel_ok subroutine
      (it does nothing that expression_tree_walker wouldn't do).
      65a603e9
    • Tom Lane's avatar
      Speed up planner's scanning for parallel-query hazards. · da1c9163
      Tom Lane authored
      We need to scan the whole parse tree for parallel-unsafe functions.
      If there are none, we'll later need to determine whether particular
      subtrees contain any parallel-restricted functions.  The previous coding
      retained no knowledge from the first scan, even though this is very
      wasteful in the common case where the query contains only parallel-safe
      functions.  We can bypass all of the later scans by remembering that fact.
      This provides a small but measurable speed improvement when the case
      applies, and shouldn't cost anything when it doesn't.
      
      Patch by me, reviewed by Robert Haas
      
      Discussion: <3740.1471538387@sss.pgh.pa.us>
      da1c9163