1. 30 Aug, 2016 3 commits
    • Tom Lane's avatar
      Fix a bunch of places that called malloc and friends with no NULL check. · 052cc223
      Tom Lane authored
      Where possible, use palloc or pg_malloc instead; otherwise, insert
      explicit NULL checks.
      
      Generally speaking, these are places where an actual OOM is quite
      unlikely, either because they're in client programs that don't
      allocate all that much, or they're very early in process startup
      so that we'd likely have had a fork() failure instead.  Hence,
      no back-patch, even though this is nominally a bug fix.
      
      Michael Paquier, with some adjustments by me
      
      Discussion: <CAB7nPqRu07Ot6iht9i9KRfYLpDaF2ZuUv5y_+72uP23ZAGysRg@mail.gmail.com>
      052cc223
    • Tom Lane's avatar
      Simplify correct use of simple_prompt(). · 9daec77e
      Tom Lane authored
      The previous API for this function had it returning a malloc'd string.
      That meant that callers had to check for NULL return, which few of them
      were doing, and it also meant that callers had to remember to free()
      the string later, which required extra logic in most cases.
      
      Instead, make simple_prompt() write into a buffer supplied by the caller.
      Anywhere that the maximum required input length is reasonably small,
      which is almost all of the callers, we can just use a local or static
      array as the buffer instead of dealing with malloc/free.
      
      A fair number of callers used "pointer == NULL" as a proxy for "haven't
      requested the password yet".  Maintaining the same behavior requires
      adding a separate boolean flag for that, which adds back some of the
      complexity we save by removing free()s.  Nonetheless, this nets out
      at a small reduction in overall code size, and considerably less code
      than we would have had if we'd added the missing NULL-return checks
      everywhere they were needed.
      
      In passing, clean up the API comment for simple_prompt() and get rid
      of a very-unnecessary malloc/free in its Windows code path.
      
      This is nominally a bug fix, but it does not seem worth back-patching,
      because the actual risk of an OOM failure in any of these places seems
      pretty tiny, and all of them are client-side not server-side anyway.
      
      This patch is by me, but it owes a great deal to Michael Paquier
      who identified the problem and drafted a patch for fixing it the
      other way.
      
      Discussion: <CAB7nPqRu07Ot6iht9i9KRfYLpDaF2ZuUv5y_+72uP23ZAGysRg@mail.gmail.com>
      9daec77e
    • Tom Lane's avatar
      Fix initdb misbehavior when user mis-enters superuser password. · 37f6fd1e
      Tom Lane authored
      While testing simple_prompt() revisions, I happened to notice that
      current initdb behaves rather badly when --pwprompt is specified and
      the user miskeys the second password.  It complains about the mismatch,
      does "rm -rf" on the data directory, and exits.  The problem is that
      since commit c4a8812c, there's a standalone backend sitting waiting
      for commands at that point.  It gets unhappy about its datadir having
      gone away, and spews a PANIC message at the user, which is not nice.
      (And the shell then adds to the mess with meaningless bleating about a
      core dump...)  We don't really want that sort of thing to happen unless
      there's an internal failure in initdb, which this surely is not.
      
      The best fix seems to be to move the collection of the password
      earlier, so that it's done essentially as part of argument collection,
      rather than at the rather ad-hoc time it was done before.
      
      Back-patch to 9.6 where the problem was introduced.
      37f6fd1e
  2. 29 Aug, 2016 6 commits
    • Alvaro Herrera's avatar
      Split hash.h → hash_xlog.h · 8e1e3f95
      Alvaro Herrera authored
      Since the hash AM is going to be revamped to have WAL, this is a good
      opportunity to clean up the include file a little bit to avoid including
      a lot of extra stuff in the future.
      
      Author: Amit Kapila
      8e1e3f95
    • Heikki Linnakangas's avatar
      Remove support for OpenSSL versions older than 0.9.8. · 9b7cd59a
      Heikki Linnakangas authored
      OpenSSL officially only supports 1.0.1 and newer. Some OS distributions
      still provide patches for 0.9.8, but anything older than that is not
      interesting anymore. Let's simplify things by removing compatibility code.
      
      Andreas Karlsson, with small changes by me.
      9b7cd59a
    • Tom Lane's avatar
      Make AllocSetContextCreate throw an error for bad context-size parameters. · cf34fdbb
      Tom Lane authored
      The previous behavior was to silently change them to something valid.
      That obscured the bugs fixed in commit ea268cdc, and generally seems
      less useful than complaining.  Unlike the previous commit, though,
      we'll do this in HEAD only --- it's a bit too late to be possibly
      breaking third-party code in 9.6.
      
      Discussion: <CA+TgmobNcELVd3QmLD3tx=w7+CokRQiC4_U0txjz=WHpfdkU=w@mail.gmail.com>
      cf34fdbb
    • Simon Riggs's avatar
      Fix pg_receivexlog --synchronous · 49340627
      Simon Riggs authored
      Make pg_receivexlog work correctly with --synchronous without slots
      
      Backpatch to 9.5
      
      Gabriele Bartolini, reviewed by Michael Paquier and Simon Riggs
      49340627
    • Fujii Masao's avatar
      Fix typos in comments. · bd082231
      Fujii Masao authored
      bd082231
    • Fujii Masao's avatar
      Fix pg_xlogdump so that it handles cross-page XLP_FIRST_IS_CONTRECORD record. · bab7823a
      Fujii Masao authored
      Previously pg_xlogdump failed to dump the contents of the WAL file
      if the file starts with the continuation WAL record which spans
      more than one pages. Since pg_xlogdump assumed that the continuation
      record always fits on a page, it could not find the valid WAL record to
      start reading from in that case.
      
      This patch changes pg_xlogdump so that it can handle a continuation
      WAL record which crosses a page boundary and find the valid record
      to start reading from.
      
      Back-patch to 9.3 where pg_xlogdump was introduced.
      
      Author: Pavan Deolasee
      Reviewed-By: Michael Paquier and Craig Ringer
      Discussion: CABOikdPsPByMiG6J01DKq6om2+BNkxHTPkOyqHM2a4oYwGKsqQ@mail.gmail.com
      bab7823a
  3. 28 Aug, 2016 3 commits
  4. 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
  5. 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
  6. 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
  7. 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
  8. 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
  9. 22 Aug, 2016 2 commits
    • Bruce Momjian's avatar
      doc: fix typo in recent patch · f2e016f8
      Bruce Momjian authored
      Reported-by: Jeff Janes
      
      Backpatch-through: 9.6
      f2e016f8
    • Robert Haas's avatar
      Fix possible sorting error when aborting use of abbreviated keys. · 008c4135
      Robert Haas authored
      Due to an error in the abbreviated key abort logic, the most recently
      processed SortTuple could be incorrectly marked NULL, resulting in an
      incorrect final sort order.
      
      In the worst case, this could result in a corrupt btree index, which
      would need to be rebuild using REINDEX.  However, abbrevation doesn't
      abort very often, not all data types use it, and only one tuple would
      end up in the wrong place, so the practical impact of this mistake may
      be somewhat limited.
      
      Report and patch by Peter Geoghegan.
      008c4135