1. 15 Nov, 2018 1 commit
    • Thomas Munro's avatar
      Use 64 bit type for BufFileSize(). · aa551830
      Thomas Munro authored
      BufFileSize() can't use off_t, because it's only 32 bits wide on
      some systems.  BufFile objects can have many 1GB segments so the
      total size can exceed 2^31.  The only known client of the function
      is parallel CREATE INDEX, which was reported to fail when building
      large indexes on Windows.
      
      Though this is technically an ABI break on platforms with a 32 bit
      off_t and we might normally avoid back-patching it, the function is
      brand new and thus unlikely to have been discovered by extension
      authors yet, and it's fairly thoroughly broken on those platforms
      anyway, so just fix it.
      
      Defect in 9da0cc35.  Bug #15460.  Back-patch to 11, where this
      function landed.
      
      Author: Thomas Munro
      Reported-by: Paul van der Linden, Pavel Oskin
      Reviewed-by: Peter Geoghegan
      Discussion: https://postgr.es/m/15460-b6db80de822fa0ad%40postgresql.org
      Discussion: https://postgr.es/m/CAHDGBJP_GsESbTt4P3FZA8kMUKuYxjg57XHF7NRBoKnR%3DCAR-g%40mail.gmail.com
      aa551830
  2. 14 Nov, 2018 8 commits
    • Tom Lane's avatar
      Make psql's "\pset format" command reject non-unique abbreviations. · eaf746a5
      Tom Lane authored
      The previous behavior of preferring the oldest match had the advantage
      of not breaking existing scripts when we add a conflicting format name;
      but that behavior was undocumented and fragile (it seems just luck that
      commit add9182e didn't break it).  Let's go over to the less mistake-
      prone approach of complaining when there are multiple matches.
      
      Since this is a small compatibility break, no back-patch.
      
      Daniel Vérité
      
      Discussion: https://postgr.es/m/cb7e1caf-3ea6-450d-af28-f524903a030c@manitou-mail.org
      eaf746a5
    • Tom Lane's avatar
      Doc: remove claim that all \pset format options are unique in 1 letter. · 51eaaafb
      Tom Lane authored
      This hasn't been correct since 9.3 added "latex-longtable".
      
      I left the phraseology "Unique abbreviations are allowed" alone.
      It's correct as far as it goes, and we are studiously refraining
      from specifying exactly what happens if you give a non-unique
      abbreviation.  (The answer in the back branches is "you get a
      backwards-compatible choice", and the answer in HEAD will shortly
      be "you get an error", but there seems no need to mention such
      details here.)
      
      Daniel Vérité
      
      Discussion: https://postgr.es/m/cb7e1caf-3ea6-450d-af28-f524903a030c@manitou-mail.org
      51eaaafb
    • Tom Lane's avatar
      Add a timezone-specific variant of date_trunc(). · 600b04d6
      Tom Lane authored
      date_trunc(field, timestamptz, zone_name) performs truncation using
      the named time zone as reference, rather than working in the session
      time zone as is the default behavior.  It's equivalent to
      
      date_trunc(field, timestamptz at time zone zone_name) at time zone zone_name
      
      but it's faster, easier to type, and arguably easier to understand.
      
      Vik Fearing and Tom Lane
      
      Discussion: https://postgr.es/m/6249ffc4-2b22-4c1b-4e7d-7af84fedd7c6@2ndquadrant.com
      600b04d6
    • Tom Lane's avatar
      Second try at fixing numeric data passed through an ECPG SQLDA. · 06c72344
      Tom Lane authored
      In commit ecfd5579, I removed sqlda.c's checks for ndigits != 0 on the
      grounds that we should duplicate the state of the numeric value's digit
      buffer even when all the digits are zeroes.  However, that still isn't
      quite right, because another possible state of the digit buffer is
      buf == digits == NULL (this occurs for a NaN).  As the code now stands,
      it'll invoke memcpy with a NULL source address and zero bytecount,
      which we know a few platforms crash on.  Hence, reinstate the no-copy
      short-circuit, but make it test specifically for buf != NULL rather than
      some other condition.  In hindsight, the ndigits test (added by commit
      f2ae9f9c) was almost certainly meant to fix the NaN case not the
      all-zeroes case as the associated thread alleged.
      
      As before, back-patch to all supported versions.
      
      Discussion: https://postgr.es/m/1803D792815FC24D871C00D17AE95905C71161@g01jpexmbkw24
      06c72344
    • Peter Eisentraut's avatar
      Lower lock level for renaming indexes · 1b5d797c
      Peter Eisentraut authored
      Change lock level for renaming index (either ALTER INDEX or implicitly
      via some other commands) from AccessExclusiveLock to
      ShareUpdateExclusiveLock.
      
      One reason we need a strong lock for relation renaming is that the
      name change causes a rebuild of the relcache entry.  Concurrent
      sessions that have the relation open might not be able to handle the
      relcache entry changing underneath them.  Therefore, we need to lock
      the relation in a way that no one can have the relation open
      concurrently.  But for indexes, the relcache handles reloads specially
      in RelationReloadIndexInfo() in a way that keeps changes in the
      relcache entry to a minimum.  As long as no one keeps pointers to
      rd_amcache and rd_options around across possible relcache flushes,
      which is the case, this ought to be safe.
      
      We also want to use a self-exclusive lock for correctness, so that
      concurrent DDL doesn't overwrite the rename if they start updating
      while still seeing the old version.  Therefore, we use
      ShareUpdateExclusiveLock, which is already used by other DDL commands
      that want to operate in a concurrent manner.
      
      The reason this is interesting at all is that renaming an index is a
      typical part of a concurrent reindexing workflow (CREATE INDEX
      CONCURRENTLY new + DROP INDEX CONCURRENTLY old + rename back).  And
      indeed a future built-in REINDEX CONCURRENTLY might rely on the ability
      to do concurrent renames as well.
      Reviewed-by: default avatarAndrey Klychkov <aaklychkov@mail.ru>
      Reviewed-by: default avatarFabrízio de Royes Mello <fabriziomello@gmail.com>
      Discussion: https://www.postgresql.org/message-id/flat/1531767486.432607658@f357.i.mail.ru
      1b5d797c
    • Michael Paquier's avatar
      Initialize TransactionState and user ID consistently at transaction start · b4721f39
      Michael Paquier authored
      If a failure happens when a transaction is starting between the moment
      the transaction status is changed from TRANS_DEFAULT to TRANS_START and
      the moment the current user ID and security context flags are fetched
      via GetUserIdAndSecContext(), or before initializing its basic fields,
      then those may get reset to incorrect values when the transaction
      aborts, leaving the session in an inconsistent state.
      
      One problem reported is that failing a starting transaction at the first
      query of a session could cause several kinds of system crashes on the
      follow-up queries.
      
      In order to solve that, move the initialization of the transaction state
      fields and the call of GetUserIdAndSecContext() in charge of fetching
      the current user ID close to the point where the transaction status is
      switched to TRANS_START, where there cannot be any error triggered
      in-between, per an idea of Tom Lane.  This properly ensures that the
      current user ID, the security context flags and that the basic fields of
      TransactionState remain consistent even if the transaction fails while
      starting.
      
      Reported-by: Richard Guo
      Diagnosed-By: Richard Guo
      Author: Michael Paquier
      Reviewed-by: Tom Lane
      Discussion: https://postgr.es/m/CAN_9JTxECSb=pEPcb0a8d+6J+bDcOZ4=DgRo_B7Y5gRHJUM=Rw@mail.gmail.com
      Backpatch-through: 9.4
      b4721f39
    • Michael Paquier's avatar
      Add flag values in WAL description to all heap records · 3be97b97
      Michael Paquier authored
      Hexadecimal is consistently used as format to not bloat too much the
      output but keep it readable.  This information is useful mainly for
      debugging purposes with for example pg_waldump.
      
      Author: Michael Paquier
      Reviewed-by: Nathan Bossart, Dmitry Dolgov, Andres Freund, Álvaro
      Herrera
      Discussion: https://postgr.es/m/20180413034734.GE1552@paquier.xyz
      3be97b97
    • Michael Paquier's avatar
      Refactor code creating PartitionBoundInfo · b52b7dc2
      Michael Paquier authored
      The code building PartitionBoundInfo based on the constituent partition
      data read from catalogs has been located in partcache.c, with a specific
      set of routines dedicated to bound types, like sorting or bound data
      creation.  All this logic is moved to partbounds.c and relocates all the
      bound-specific logistic into it, with partition_bounds_create() as
      principal entry point.
      
      Author: Amit Langote
      Reviewed-by: Michael Paquier, Álvaro Herrera
      Discussion: https://postgr.es/m/3f289da8-6d10-75fe-814a-635e8b191d43@lab.ntt.co.jp
      b52b7dc2
  3. 13 Nov, 2018 11 commits
  4. 12 Nov, 2018 6 commits
    • Michael Paquier's avatar
      Remove CommandCounterIncrement() after processing ON COMMIT DELETE · 52b70b1c
      Michael Paquier authored
      This comes from f9b5b41e, which is part of one the original commits that
      implemented ON COMMIT actions.  By looking at the truncation code, any
      CCI needed happens locally when rebuilding indexes, so it looks safe to
      just remove this final incrementation.
      
      Author: Michael Paquier
      Reviewed-by: Álvaro Herrera
      Discussion: https://postgr.es/m/20181109024731.GF2652@paquier.xyz
      52b70b1c
    • Tom Lane's avatar
      Simplify null-element handling in extension_config_remove(). · 3de49148
      Tom Lane authored
      There's no point in asking deconstruct_array() for a null-flags
      array when we already checked the array has no nulls, and aren't
      going to examine the output anyhow.  Not asking for this output
      should make the code marginally faster, and it's also more
      robust since if there somehow were nulls, deconstruct_array()
      would throw an error.
      
      Daniel Gustafsson
      
      Discussion: https://postgr.es/m/289FFB8B-7AAB-48B5-A497-6E0D41D7BA47@yesql.se
      3de49148
    • Tom Lane's avatar
      Limit the number of index clauses considered in choose_bitmap_and(). · e3f005d9
      Tom Lane authored
      classify_index_clause_usage() is O(N^2) in the number of distinct index
      qual clauses it considers, because of its use of a simple search list to
      store them.  For nearly all queries, that's fine because only a few clauses
      will be considered.  But Alexander Kuzmenkov reported a machine-generated
      query with 80000 (!) index qual clauses, which caused this code to take
      forever.  Somewhat remarkably, this is the only O(N^2) behavior we now
      have for such a query, so let's fix it.
      
      We can get rid of the O(N^2) runtime for cases like this without much
      damage to the functionality of choose_bitmap_and() by separating out
      paths with "too many" qual or pred clauses, and deeming them to always
      be nonredundant with other paths.  Then their clauses needn't go into
      the search list, so it doesn't get too long, but we don't lose the
      ability to consider bitmap AND plans altogether.  I set the threshold
      for "too many" to be 100 clauses per path, which should be plenty to
      ensure no change in planning behavior for normal queries.
      
      There are other things we could do to make this go faster, but it's not
      clear that it's worth any additional effort.  80000 qual clauses require
      a whole lot of work in many other places, too.
      
      The code's been like this for a long time, so back-patch to all supported
      branches.  The troublesome query only works back to 9.5 (in 9.4 it fails
      with stack overflow in the parser); so I'm not sure that fixing this in
      9.4 has any real-world benefit, but perhaps it does.
      
      Discussion: https://postgr.es/m/90c5bdfa-d633-dabe-9889-3cf3e1acd443@postgrespro.ru
      e3f005d9
    • Michael Paquier's avatar
      ffb68980
    • Peter Eisentraut's avatar
      doc: Small punctuation improvement · fc151211
      Peter Eisentraut authored
      fc151211
    • Peter Eisentraut's avatar
      doc: Small run-time pruning doc fix · 253b3f67
      Peter Eisentraut authored
      A note in ddl.sgml used to mention that run-time pruning was only
      implemented for Append.  When we got MergeAppend support, this was
      updated to mention that MergeAppend is supported too.  This is
      slightly weird as it's not all that obvious what exactly isn't
      supported when we mention:
      
          <para>
           Both of these behaviors are likely to be changed in a future release
           of <productname>PostgreSQL</productname>.
          </para>
      
      This patch updates this to mention that ModifyTable is unsupported,
      which makes the above fragment make sense again.
      
      Author: David Rowley <david.rowley@2ndquadrant.com>
      253b3f67
  5. 11 Nov, 2018 2 commits
  6. 10 Nov, 2018 8 commits
    • Peter Eisentraut's avatar
      Apply RI trigger skipping tests also for DELETE · 69ee2ff9
      Peter Eisentraut authored
      The tests added in cfa0f425 to skip
      firing an RI trigger if any old key value is NULL can also be applied
      for DELETE.  This should give a performance gain in those cases, and it
      also saves a lot of duplicate code in the actual RI triggers.  (That
      code was already dead code for the UPDATE cases.)
      Reviewed-by: default avatarDaniel Gustafsson <daniel@yesql.se>
      69ee2ff9
    • Peter Eisentraut's avatar
      Remove dead foreign key optimization code · 34479d9a
      Peter Eisentraut authored
      The ri_KeysEqual() calls in the foreign-key trigger functions to
      optimize away some updates are useless because since
      adfeef55 those triggers are not enqueued
      at all.  (It's also not useful to keep these checks as some kind of
      backstop, since it's also semantically correct to just run the full
      check even with equal keys.)
      Reviewed-by: default avatarDaniel Gustafsson <daniel@yesql.se>
      34479d9a
    • Andres Freund's avatar
      Combine two flag tests in GetSnapshotData(). · 5fde047f
      Andres Freund authored
      Previously the code checked PROC_IN_LOGICAL_DECODING and
      PROC_IN_VACUUM separately. As the relevant variable is marked as
      volatile, the compiler cannot combine the two tests.  As
      GetSnapshotData() is pretty hot in a number of workloads, it's
      worthwhile to fix that.
      
      It'd also be a good idea to get rid of the volatiles altogether. But
      for one that's a larger patch, and for another, the code after this
      change still seems at least as easy to read as before.
      
      Author: Andres Freund
      Discussion: https://postgr.es/m/20181005172955.wyjb4fzcdzqtaxjq@alap3.anarazel.de
      5fde047f
    • Andres Freund's avatar
      docs: Adapt wal_segment_size docs to fc49e24f. · 5fc1670b
      Andres Freund authored
      Before this change the docs weren't adapted to the fact that
      wal_segment_size is now measured in bytes, rather than multiples of
      wal_block_size.
      
      Author: David Steele
      Discussion: https://postgr.es/m/68ea97d6-2ed9-f339-e57d-ab3a33caf3b1@pgmasters.net
      Backpatch: 11-, like fc49e24f itself.
      5fc1670b
    • Tom Lane's avatar
      Fix error-cleanup mistakes in exec_stmt_call(). · f26c06a4
      Tom Lane authored
      Commit 15c72934 was a couple bricks shy of a load: we need to
      ensure that expr->plan gets reset to NULL on any error exit,
      if it's not supposed to be saved.  Also ensure that the
      stmt->target calculation gets redone if needed.
      
      The easy way to exhibit a problem is to set up code that
      violates the writable-argument restriction and then execute
      it twice.  But error exits out of, eg, setup_param_list()
      could also break it.  Make the existing PG_TRY block cover
      all of that code to be sure.
      
      Per report from Pavel Stehule.
      
      Discussion: https://postgr.es/m/CAFj8pRAeXNTO43W2Y0Cn0YOVFPv1WpYyOqQrrzUiN6s=dn7gCg@mail.gmail.com
      f26c06a4
    • Tom Lane's avatar
      Fix missing role dependencies for some schema and type ACLs. · fa2952d8
      Tom Lane authored
      This patch fixes several related cases in which pg_shdepend entries were
      never made, or were lost, for references to roles appearing in the ACLs of
      schemas and/or types.  While that did no immediate harm, if a referenced
      role were later dropped, the drop would be allowed and would leave a
      dangling reference in the object's ACL.  That still wasn't a big problem
      for normal database usage, but it would cause obscure failures in
      subsequent dump/reload or pg_upgrade attempts, taking the form of
      attempts to grant privileges to all-numeric role names.  (I think I've
      seen field reports matching that symptom, but can't find any right now.)
      
      Several cases are fixed here:
      
      1. ALTER DOMAIN SET/DROP DEFAULT would lose the dependencies for any
      existing ACL entries for the domain.  This case is ancient, dating
      back as far as we've had pg_shdepend tracking at all.
      
      2. If a default type privilege applies, CREATE TYPE recorded the
      ACL properly but forgot to install dependency entries for it.
      This dates to the addition of default privileges for types in 9.2.
      
      3. If a default schema privilege applies, CREATE SCHEMA recorded the
      ACL properly but forgot to install dependency entries for it.
      This dates to the addition of default privileges for schemas in v10
      (commit ab89e465).
      
      Another somewhat-related problem is that when creating a relation
      rowtype or implicit array type, TypeCreate would apply any available
      default type privileges to that type, which we don't really want
      since such an object isn't supposed to have privileges of its own.
      (You can't, for example, drop such privileges once they've been added
      to an array type.)
      
      ab89e465 is also to blame for a race condition in the regression tests:
      privileges.sql transiently installed globally-applicable default
      privileges on schemas, which sometimes got absorbed into the ACLs of
      schemas created by concurrent test scripts.  This should have resulted
      in failures when privileges.sql tried to drop the role holding such
      privileges; but thanks to the bug fixed here, it instead led to dangling
      ACLs in the final state of the regression database.  We'd managed not to
      notice that, but it became obvious in the wake of commit da906766, which
      allowed the race condition to occur in pg_upgrade tests.
      
      To fix, add a function recordDependencyOnNewAcl to encapsulate what
      callers of get_user_default_acl need to do; while the original call
      sites got that right via ad-hoc code, none of the later-added ones
      have.  Also change GenerateTypeDependencies to generate these
      dependencies, which requires adding the typacl to its parameter list.
      (That might be annoying if there are any extensions calling that
      function directly; but if there are, they're most likely buggy in the
      same way as the core callers were, so they need work anyway.)  While
      I was at it, I changed GenerateTypeDependencies to accept most of its
      parameters in the form of a Form_pg_type pointer, making its parameter
      list a bit less unwieldy and mistake-prone.
      
      The test race condition is fixed just by wrapping the addition and
      removal of default privileges into a single transaction, so that that
      state is never visible externally.  We might eventually prefer to
      separate out tests of default privileges into a script that runs by
      itself, but that would be a bigger change and would make the tests
      run slower overall.
      
      Back-patch relevant parts to all supported branches.
      
      Discussion: https://postgr.es/m/15719.1541725287@sss.pgh.pa.us
      fa2952d8
    • Andres Freund's avatar
      Remove ineffective check against dropped columns from slot_getattr(). · c670d0fa
      Andres Freund authored
      Before this commit slot_getattr() checked for dropped
      columns (returning NULL in that case), but only after checking for
      previously deformed columns. As slot_deform_tuple() does not contain
      such a check, the check in slot_getattr() would often not have been
      reached, depending on previous use of the slot.
      
      These days locking and plan invalidation ought to ensure that dropped
      columns are not accessed in query plans. Therefore this commit just
      drops the insufficient check in slot_getattr().  It's possible that
      we'll find some holes againt use of dropped columns, but if so, those
      need to be addressed independent of slot_getattr(), as most accesses
      don't go through that function anyway.
      
      Author: Andres Freund
      Discussion: https://postgr.es/m/20181107174403.zai7fedgcjoqx44p@alap3.anarazel.de
      c670d0fa
    • Andres Freund's avatar
      Don't require return slots for nodes without projection. · 1ef6bd29
      Andres Freund authored
      In a lot of nodes the return slot is not required. That can either be
      because the node doesn't do any projection (say an Append node), or
      because the node does perform projections but the projection is
      optimized away because the projection would yield an identical row.
      
      Slots aren't that small, especially for wide rows, so it's worthwhile
      to avoid creating them.  It's not possible to just skip creating the
      slot - it's currently used to determine the tuple descriptor returned
      by ExecGetResultType().  So separate the determination of the result
      type from the slot creation.  The work previously done internally
      ExecInitResultTupleSlotTL() can now also be done separately with
      ExecInitResultTypeTL() and ExecInitResultSlot().  That way nodes that
      aren't guaranteed to need a result slot, can use
      ExecInitResultTypeTL() to determine the result type of the node, and
      ExecAssignScanProjectionInfo() (via
      ExecConditionalAssignProjectionInfo()) determines that a result slot
      is needed, it is created with ExecInitResultSlot().
      
      Besides the advantage of avoiding to create slots that then are
      unused, this is necessary preparation for later patches around tuple
      table slot abstraction. In particular separating the return descriptor
      and slot is a prerequisite to allow JITing of tuple deforming with
      knowledge of the underlying tuple format, and to avoid unnecessarily
      creating JITed tuple deforming for virtual slots.
      
      This commit removes a redundant argument from
      ExecInitResultTupleSlotTL(). While this commit touches a lot of the
      relevant lines anyway, it'd normally still not worthwhile to cause
      breakage, except that aforementioned later commits will touch *all*
      ExecInitResultTupleSlotTL() callers anyway (but fits worse
      thematically).
      
      Author: Andres Freund
      Discussion: https://postgr.es/m/20181105210039.hh4vvi4vwoq5ba2q@alap3.anarazel.de
      1ef6bd29
  7. 09 Nov, 2018 3 commits
    • Michael Paquier's avatar
      Fix incorrect routine name in xlog_heapam.h · 3ce12018
      Michael Paquier authored
      s/xl_heap_delete/xl_heap_truncate/ in a comment block referring to flags
      for truncation.
      
      Discussion: https://postgr.es/m/20180413034734.GE1552@paquier.xyz
      3ce12018
    • Alvaro Herrera's avatar
      Indicate session name in isolationtester notices · a28e10e8
      Alvaro Herrera authored
      When a session under isolationtester produces printable notices (NOTICE,
      WARNING) we were just printing them unadorned, which can be confusing
      when debugging.  Prefix them with the session name, which makes things
      clearer.
      
      Author: Álvaro Herrera
      Reviewed-by: Hari Babu Kommi
      Discussion: https://postgr.es/m/20181024213451.75nh3f3dctmcdbfq@alvherre.pgsql
      a28e10e8
    • Michael Paquier's avatar
      Fix dependency handling of partitions and inheritance for ON COMMIT · 319a8101
      Michael Paquier authored
      This commit fixes a set of issues with ON COMMIT actions when used on
      partitioned tables and tables with inheritance children:
      - Applying ON COMMIT DROP on a partitioned table with partitions or on a
      table with inheritance children caused a failure at commit time, with
      complains about the children being already dropped as all relations are
      dropped one at the same time.
      - Applying ON COMMIT DELETE on a partition relying on a partitioned
      table which uses ON COMMIT DROP would cause the partition truncation to
      fail as the parent is removed first.
      
      The solution to the first problem is to handle the removal of all the
      dependencies in one go instead of dropping relations one-by-one, based
      on a suggestion from Álvaro Herrera.  So instead all the relation OIDs
      to remove are gathered and then processed in one round of multiple
      deletions.
      
      The solution to the second problem is to reorder the actions, with
      truncation happening first and relation drop done after.  Even if it
      means that a partition could be first truncated, then immediately
      dropped if its partitioned table is dropped, this has the merit to keep
      the code simple as there is no need to do existence checks on the
      relations to drop.
      
      Contrary to a manual TRUNCATE on a partitioned table, ON COMMIT DELETE
      does not cascade to its partitions.  The ON COMMIT action defined on
      each partition gets the priority.
      
      Author: Michael Paquier
      Reviewed-by: Amit Langote, Álvaro Herrera, Robert Haas
      Discussion: https://postgr.es/m/68f17907-ec98-1192-f99f-8011400517f5@lab.ntt.co.jp
      Backpatch-through: 10
      319a8101
  8. 08 Nov, 2018 1 commit
    • Tom Lane's avatar
      Disallow setting client_min_messages higher than ERROR. · 3d360e20
      Tom Lane authored
      Previously it was possible to set client_min_messages to FATAL or PANIC,
      which had the effect of suppressing transmission of regular ERROR messages
      to the client.  Perhaps that seemed like a useful option in the past, but
      the trouble with it is that it breaks guarantees that are explicitly made
      in our FE/BE protocol spec about how a query cycle can end.  While libpq
      and psql manage to cope with the omission, that's mostly because they
      are not very bright; client libraries that have more semantic knowledge
      are likely to get confused.  Notably, pgODBC doesn't behave very sanely.
      Let's fix this by getting rid of the ability to set client_min_messages
      above ERROR.
      
      In HEAD, just remove the FATAL and PANIC options from the set of allowed
      enum values for client_min_messages.  (This change also affects
      trace_recovery_messages, but that's OK since these aren't useful values
      for that variable either.)
      
      In the back branches, there was concern that rejecting these values might
      break applications that are explicitly setting things that way.  I'm
      pretty skeptical of that argument, but accommodate it by accepting these
      values and then internally setting the variable to ERROR anyway.
      
      In all branches, this allows a couple of tiny simplifications in the
      logic in elog.c, so do that.
      
      Also respond to the point that was made that client_min_messages has
      exactly nothing to do with the server's logging behavior, and therefore
      does not belong in the "When To Log" subsection of the documentation.
      The "Statement Behavior" subsection is a better match, so move it there.
      
      Jonah Harris and Tom Lane
      
      Discussion: https://postgr.es/m/7809.1541521180@sss.pgh.pa.us
      Discussion: https://postgr.es/m/15479-ef0f4cc2fd995ca2@postgresql.org
      3d360e20