1. 06 Feb, 2020 5 commits
    • Jeff Davis's avatar
      Refactor hash_agg_entry_size(). · 7d4395d0
      Jeff Davis authored
      Consolidate the calculations for hash table size estimation. This will
      help with upcoming Hash Aggregation work that will add additional call
      sites.
      7d4395d0
    • Jeff Davis's avatar
      Logical Tape Set: use min heap for freelist. · c02fdc92
      Jeff Davis authored
      Previously, the freelist of blocks was tracked as an
      occasionally-sorted array. A min heap is more resilient to larger
      freelists or more frequent changes between reading and writing.
      
      Discussion: https://postgr.es/m/97c46a59c27f3c38e486ca170fcbc618d97ab049.camel%40j-davis.com
      c02fdc92
    • Amit Kapila's avatar
      Fix typo. · cac8ce4a
      Amit Kapila authored
      Reported-by: Amit Langote
      Author: Amit Langote
      Backpatch-through: 9.6, where it was introduced
      Discussion: https://postgr.es/m/CA+HiwqFNADeukaaGRmTqANbed9Fd81gLi08AWe_F86_942Gspw@mail.gmail.com
      cac8ce4a
    • Fujii Masao's avatar
      Fix bug in LWLock statistics mechanism. · 3ccc66da
      Fujii Masao authored
      Previously PostgreSQL built with -DLWLOCK_STATS could report
      more than one LWLock statistics entries for the same backend
      process and the same LWLock. This is strange and only one
      statistics should be output in that case, instead.
      
      The cause of this issue is that the key variable used for
      LWLock stats hash table was not fully initialized. The key
      consists of two fields and they were initialized. But
      the following 4 bytes allocated in the key variable for
      the alignment was not initialized. So even if the same key
      was specified, hash_search(HASH_ENTER) could not find
      the existing entry for that key and created new one.
      
      This commit fixes this issue by initializing the key
      variable with zero. As the side effect of this commit,
      the volume of LWLock statistics output would be reduced
      very much.
      
      Back-patch to v10, where commit 3761fe3c introduced the issue.
      
      Author: Fujii Masao
      Reviewed-by: Julien Rouhaud, Kyotaro Horiguchi
      Discussion: https://postgr.es/m/26359edb-798a-568f-d93a-6aafac49752d@oss.nttdata.com
      3ccc66da
    • Michael Paquier's avatar
      Add leader_pid to pg_stat_activity · b025f32e
      Michael Paquier authored
      This new field tracks the PID of the group leader used with parallel
      query.  For parallel workers and the leader, the value is set to the
      PID of the group leader.  So, for the group leader, the value is the
      same as its own PID.  Note that this reflects what PGPROC stores in
      shared memory, so as leader_pid is NULL if a backend has never been
      involved in parallel query.  If the backend is using parallel query or
      has used it at least once, the value is set until the backend exits.
      
      Author: Julien Rouhaud
      Reviewed-by: Sergei Kornilov, Guillaume Lelarge, Michael Paquier, Tomas
      Vondra
      Discussion: https://postgr.es/m/CAOBaU_Yy5bt0vTPZ2_LUM6cUcGeqmYNoJ8-Rgto+c2+w3defYA@mail.gmail.com
      b025f32e
  2. 05 Feb, 2020 5 commits
  3. 04 Feb, 2020 3 commits
    • Thomas Munro's avatar
      Handle lack of DSM slots in parallel btree build, take 2. · d9fe702a
      Thomas Munro authored
      Commit 74618e77 added a new check intended to fix a bug, but put
      it in the wrong place so that parallel btree build was always
      disabled.  Do the check after we've actually tried to create
      a DSM segment.  Back-patch to 11, like the earlier commit.
      
      Reviewed-by: Peter Geoghegan
      Discussion: https://postgr.es/m/CAH2-WzmDABkJzrNnvf%2BOULK-_A_j9gkYg_Dz-H62jzNv4eKQTw%40mail.gmail.com
      d9fe702a
    • Tom Lane's avatar
      Fix handling of "Subplans Removed" field in EXPLAIN output. · 7d91b604
      Tom Lane authored
      Commit 499be013 added this field in a rather poorly-thought-through
      manner, with the result being that rather than being a field of the
      Append or MergeAppend plan node as intended (and as it seems to be,
      in text format), it was actually an element of the "Plans" subgroup.
      At least in JSON format, that's flat out invalid syntax, because
      "Plans" is an array not an object.
      
      While it's not hard to move the generation of the field so that it
      appears where it's supposed to, this does result in a visible change
      in field order in text format, in cases where a Append or MergeAppend
      plan node has any InitPlans attached.  That's slightly annoying to
      do in stable branches; but the alternative of continuing to emit
      broken non-text formats seems worse.
      
      Also, since the set of fields emitted is not supposed to be
      data-dependent in non-text formats, make sure that "Subplans Removed"
      appears in Append and MergeAppend nodes even when it's zero, in those
      formats.  (The previous coding made it look like it could appear in
      some other node types such as BitmapAnd, but we don't actually support
      runtime pruning there, so don't emit it in those cases.)
      
      Per bug #16171 from Mahadevan Ramachandran.  Fix by Daniel Gustafsson
      and Tom Lane, reviewed by Hamid Akhtar.  Back-patch to v11 where this
      code came in.
      
      Discussion: https://postgr.es/m/16171-b72259ab75505fa2@postgresql.org
      7d91b604
    • Michael Paquier's avatar
      Fix fuzzy error handling in pg_basebackup when opening gzFile · 177be9ed
      Michael Paquier authored
      First, this code did not bother checking for a failure when calling
      dup().  Then, per zlib, gzerror() returns NULL for a NULL input, which
      can happen if passing down to gzdopen() an invalid file descriptor or if
      there was an allocation failure.
      
      No back-patch is done as this would unlikely be a problem in the field.
      
      Per Coverity.
      
      Reported-by: Tom Lane
      177be9ed
  4. 03 Feb, 2020 2 commits
  5. 02 Feb, 2020 1 commit
    • Tom Lane's avatar
      Fix assorted error-cleanup bugs in SSL min/max protocol version code. · 6148e2b9
      Tom Lane authored
      The error exits added to initialize_SSL() failed to clean up the
      partially-built SSL_context, and some of them also leaked the
      result of SSLerrmessage().  Make them match other error-handling
      cases in that function.
      
      The error exits added to connectOptions2() failed to set conn->status
      like every other error exit in that function.
      
      In passing, make the SSL_get_peer_certificate() error exit look more
      like all the other calls of SSLerrmessage().
      
      Oversights in commit ff8ca5fa.  Coverity whined about leakage of the
      SSLerrmessage() results; I noted the rest in manual code review.
      6148e2b9
  6. 01 Feb, 2020 3 commits
  7. 31 Jan, 2020 8 commits
    • Tom Lane's avatar
      Fix not-quite-right string comparison in parse_jsonb_index_flags(). · 870ad6a5
      Tom Lane authored
      This code would accept "strinX", where X is any 1-byte character,
      as meaning "string".  Clearly it wasn't meant to do that.
      
      No back-patch, since this doesn't affect correct queries and
      there's some tiny chance we'd break somebody's incorrect query
      in a minor release.
      
      Report and patch by Dominik Czarnota.
      
      Discussion: https://postgr.es/m/CABEVAa1dU0mDCAfaT8WF2adVXTDsLVJy_izotg6ze_hh-cn8qQ@mail.gmail.com
      870ad6a5
    • Tom Lane's avatar
      Fix CheckAttributeType's handling of collations for ranges. · 74b35eb4
      Tom Lane authored
      Commit fc769589 changed CheckAttributeType to recurse into ranges,
      but made it pass down the wrong collation (always InvalidOid, since
      ranges as such have no collation).  This would result in guaranteed
      failure when considering a range type whose subtype is collatable.
      
      Embarrassingly, we lack any regression tests that would expose such
      a problem (but fortunately, somebody noticed before we shipped this
      bug in any release).
      
      Fix it to pass down the range's subtype collation property instead,
      and add some regression test cases to exercise collatable-subtype
      ranges a bit more.  Back-patch to all supported branches, as the
      previous patch was.
      
      Report and patch by Julien Rouhaud, test cases tweaked by me
      
      Discussion: https://postgr.es/m/CAOBaU_aBWqNweiGUFX0guzBKkcfJ8mnnyyGC_KBQmO12Mj5f_A@mail.gmail.com
      74b35eb4
    • Tom Lane's avatar
      Fix parallel pg_dump/pg_restore for failure to create worker processes. · 2425f8f7
      Tom Lane authored
      If we failed to fork a worker process, or create a communication pipe
      for one, WaitForTerminatingWorkers would suffer an assertion failure
      if assert-enabled, otherwise crash or go into an infinite loop.  This
      was a consequence of not accounting for the startup condition where
      we've not yet forked all the workers.
      
      The original bug was that ParallelBackupStart would set workerStatus to
      WRKR_IDLE before it had successfully forked a worker.  I made things
      worse in commit b7b8cc0c by not understanding the undocumented fact
      that the WRKR_TERMINATED state was also meant to represent the case
      where a worker hadn't been started yet: I changed enum T_WorkerStatus
      so that *all* the worker slots were initially in WRKR_IDLE state.  But
      this wasn't any more broken in practice, since even one slot in the
      wrong state would keep WaitForTerminatingWorkers from terminating.
      
      In v10 and later, introduce an explicit T_WorkerStatus value for
      worker-not-started, in hopes of preventing future oversights of the
      same ilk.  Before that, just document that WRKR_TERMINATED is supposed
      to cover that case (partly because it wasn't actively broken, and
      partly because the enum is exposed outside parallel.c in those branches,
      so there's microscopically more risk involved in changing it).
      In all branches, introduce a WORKER_IS_RUNNING status test macro
      to hide which T_WorkerStatus values mean that, and be more careful
      not to access ParallelSlot fields till we're sure they're valid.
      
      Per report from Vignesh C, though this is my patch not his.
      Back-patch to all supported branches.
      
      Discussion: https://postgr.es/m/CALDaNm1Luv-E3sarR+-unz-BjchquHHyfP+YC+2FS2pt_J+wxg@mail.gmail.com
      2425f8f7
    • Peter Eisentraut's avatar
      Allow building without default socket directory · a9cff89f
      Peter Eisentraut authored
      We have code paths for Unix socket support and no Unix socket support.
      Now add a third variant: Unix socket support but do not use a Unix
      socket by default in the client or the server, only if you explicitly
      specify one.  This will be useful when we enable Unix socket support
      on Windows.
      
      To implement this, tweak things so that setting DEFAULT_PGSOCKET_DIR
      to "" has the desired effect.  This mostly already worked like that;
      only a few places needed to be adjusted.  Notably, the reference to
      DEFAULT_PGSOCKET_DIR in UNIXSOCK_PATH() could be removed because all
      callers already resolve an empty socket directory setting with a
      default if appropriate.
      Reviewed-by: default avatarTom Lane <tgl@sss.pgh.pa.us>
      Discussion: https://www.postgresql.org/message-id/75f72249-8ae6-322a-63df-4fe03eeccb9f@2ndquadrant.com
      a9cff89f
    • Peter Eisentraut's avatar
      Sprinkle some const decorations · 7c23bfd2
      Peter Eisentraut authored
      This might help clarify the API a bit.
      7c23bfd2
    • Michael Paquier's avatar
      Fix typo in recently-added TAP test for replication slots · 7ca8c970
      Michael Paquier authored
      Oversight in commit b0afdcad.
      7ca8c970
    • Thomas Munro's avatar
      Report time spent in posix_fallocate() as a wait event. · ef02fb15
      Thomas Munro authored
      When allocating DSM segments with posix_fallocate() on Linux (see commit
      899bd785), report this activity as a wait event exactly as we would if
      we were using file-backed DSM rather than shm_open()-backed DSM.
      
      Author: Thomas Munro
      Discussion: https://postgr.es/m/CA%2BhUKGKCSh4GARZrJrQZwqs5SYp0xDMRr9Bvb%2BHQzJKvRgL6ZA%40mail.gmail.com
      ef02fb15
    • Thomas Munro's avatar
      Adjust DSM and DSA slot usage constants. · d061ea21
      Thomas Munro authored
      When running a lot of large parallel queries concurrently, or a plan with
      a lot of separate Gather nodes, it is possible to run out of DSM slots.
      There are better solutions to these problems requiring architectural
      redesign work, but for now, let's adjust the constants so that it's more
      difficult to hit the limit.
      
      1.  Previously, a DSA area would create up to four segments at each size
      before doubling the size.  After this commit, it will create only two at
      each size, so it ramps up faster and therefore needs fewer slots.
      
      2.  Previously, the total limit on DSM slots allowed for 2 per connection.
      Switch to 5 per connection.
      
      Also remove an obsolete nearby comment.
      
      Author: Thomas Munro
      Reviewed-by: Robert Haas, Andres Freund
      Discussion: https://postre.es/m/CA%2BhUKGL6H2BpGbiF7Lj6QiTjTGyTLW_vLR%3DSn2tEBeTcYXiMKw%40mail.gmail.com
      d061ea21
  8. 30 Jan, 2020 8 commits
  9. 29 Jan, 2020 5 commits
    • Tom Lane's avatar
      Invent "trusted" extensions, and remove the pg_pltemplate catalog. · 50fc694e
      Tom Lane authored
      This patch creates a new extension property, "trusted".  An extension
      that's marked that way in its control file can be installed by a
      non-superuser who has the CREATE privilege on the current database,
      even if the extension contains objects that normally would have to be
      created by a superuser.  The objects within the extension will (by
      default) be owned by the bootstrap superuser, but the extension itself
      will be owned by the calling user.  This allows replicating the old
      behavior around trusted procedural languages, without all the
      special-case logic in CREATE LANGUAGE.  We have, however, chosen to
      loosen the rules slightly: formerly, only a database owner could take
      advantage of the special case that allowed installation of a trusted
      language, but now anyone who has CREATE privilege can do so.
      
      Having done that, we can delete the pg_pltemplate catalog, moving the
      knowledge it contained into the extension script files for the various
      PLs.  This ends up being no change at all for the in-core PLs, but it is
      a large step forward for external PLs: they can now have the same ease
      of installation as core PLs do.  The old "trusted PL" behavior was only
      available to PLs that had entries in pg_pltemplate, but now any
      extension can be marked trusted if appropriate.
      
      This also removes one of the stumbling blocks for our Python 2 -> 3
      migration, since the association of "plpythonu" with Python 2 is no
      longer hard-wired into pg_pltemplate's initial contents.  Exactly where
      we go from here on that front remains to be settled, but one problem
      is fixed.
      
      Patch by me, reviewed by Peter Eisentraut, Stephen Frost, and others.
      
      Discussion: https://postgr.es/m/5889.1566415762@sss.pgh.pa.us
      50fc694e
    • Tom Lane's avatar
      Teach plpgsql's "make clean" to remove generated test files. · 166ab9c8
      Tom Lane authored
      Copy the rules that src/test/regress/GNUmakefile uses for this purpose.
      Since these files are .gitignore'd, the mistake wasn't obvious unless
      you happened to look at "git status --ignored" in an allegedly clean
      tree.
      
      Oversight in commit 1858b105.  No need for back-patch since that's
      not in the back branches.
      166ab9c8
    • Robert Haas's avatar
      Add jsonapi.c to Mkvcbuild.pm's @pgcommonallfiles. · 006b9dca
      Robert Haas authored
      My recent commit beb46990 caused
      some buildfarm breakage, as reported by Tom Lane. Try to repair.
      
      This fix is extracted from a larger patch by Andrew Dunstan.
      
      Discussion: http://postgr.es/m/8440ddc9-8347-ca64-1405-845d10e054cd@2ndQuadrant.com
      Discussion: http://postgr.es/m/14178.1580312751@sss.pgh.pa.us
      006b9dca
    • Robert Haas's avatar
      Move jsonapi.c and jsonapi.h to src/common. · beb46990
      Robert Haas authored
      To make this work, (1) makeJsonLexContextCstringLen now takes the
      encoding to be used as an argument; (2) check_stack_depth() is made to
      do nothing in frontend code, and (3) elog(ERROR, ...) is changed to
      pg_log_fatal + exit in frontend code.
      
      Mark Dilger, reviewed and slightly revised by me.
      
      Discussion: http://postgr.es/m/CA+TgmoYfOXhd27MUDGioVh6QtpD0C1K-f6ObSA10AWiHBAL5bA@mail.gmail.com
      beb46990
    • Peter Eisentraut's avatar
      Fail if recovery target is not reached · dc788668
      Peter Eisentraut authored
      Before, if a recovery target is configured, but the archive ended
      before the target was reached, recovery would end and the server would
      promote without further notice.  That was deemed to be pretty wrong.
      With this change, if the recovery target is not reached, it is a fatal
      error.
      Based-on-patch-by: default avatarLeif Gunnar Erlandsen <leif@lako.no>
      Reviewed-by: default avatarKyotaro Horiguchi <horikyota.ntt@gmail.com>
      Discussion: https://www.postgresql.org/message-id/flat/993736dd3f1713ec1f63fc3b653839f5@lako.no
      dc788668