1. 09 Nov, 2020 7 commits
    • Peter Geoghegan's avatar
      Remove ineffective heapam CHECK_FOR_INTERRUPTS(). · 180cf876
      Peter Geoghegan authored
      Remove a CHECK_FOR_INTERRUPTS() call that could never actually handle an
      interrupt.  We always have a heap page buffer lock at this point.
      Having a useless CHECK_FOR_INTERRUPTS() call is harmless but misleading.
      
      It is probably possible to work around the immediate problem by moving
      the CHECK_FOR_INTERRUPTS() to before the heap page buffer lock is
      acquired.  That isn't enough to make the function responsive to
      interrupts, though.  The index AM caller will still hold an exclusive
      buffer lock of its own.
      180cf876
    • Noah Misch's avatar
      Ignore attempts to \gset into specially treated variables. · 098fb007
      Noah Misch authored
      If an interactive psql session used \gset when querying a compromised
      server, the attacker could execute arbitrary code as the operating
      system account running psql.  Using a prefix not found among specially
      treated variables, e.g. every lowercase string, precluded the attack.
      Fix by issuing a warning and setting no variable for the column in
      question.  Users wanting the old behavior can use a prefix and then a
      meta-command like "\set HISTSIZE :prefix_HISTSIZE".  Back-patch to 9.5
      (all supported versions).
      
      Reviewed by Robert Haas.  Reported by Nick Cleaton.
      
      Security: CVE-2020-25696
      098fb007
    • Noah Misch's avatar
      In security-restricted operations, block enqueue of at-commit user code. · 0c3185e9
      Noah Misch authored
      Specifically, this blocks DECLARE ... WITH HOLD and firing of deferred
      triggers within index expressions and materialized view queries.  An
      attacker having permission to create non-temp objects in at least one
      schema could execute arbitrary SQL functions under the identity of the
      bootstrap superuser.  One can work around the vulnerability by disabling
      autovacuum and not manually running ANALYZE, CLUSTER, REINDEX, CREATE
      INDEX, VACUUM FULL, or REFRESH MATERIALIZED VIEW.  (Don't restore from
      pg_dump, since it runs some of those commands.)  Plain VACUUM (without
      FULL) is safe, and all commands are fine when a trusted user owns the
      target object.  Performance may degrade quickly under this workaround,
      however.  Back-patch to 9.5 (all supported versions).
      
      Reviewed by Robert Haas.  Reported by Etienne Stalmans.
      
      Security: CVE-2020-25695
      0c3185e9
    • Magnus Hagander's avatar
      Remove analyze_new_cluster script from pg_upgrade · 8f113698
      Magnus Hagander authored
      Since this script just runs vacuumdb anyway, remove the script and
      replace the instructions to run it with instructions to run vacuumdb
      directly.
      
      Reviewed-By: Michael Paquier
      Discussion: https://postgr.es/m/CABUevEwg5LDFzthhxzSj7sZGMiVsZe0VVNbzzwTQOHJ=rN7+5A@mail.gmail.com
      8f113698
    • Magnus Hagander's avatar
      Remove incorrect %s in string · 7e84dd21
      Magnus Hagander authored
      Appears to have been a copy/paste error in the original commit that
      moved the messages to fe_utils/.
      
      Author: Tang, Haiying <tanghy.fnst@cn.fujitsu.com>
      Backpatch-through: 13
      Discussion: https://postgr.es/m/3321cbcea76d4d2c8320a05c19b9304a@G08CNEXMBPEKD05.g08.fujitsu.local
      7e84dd21
    • Fujii Masao's avatar
      doc: Add note about pg_settings and customized options into catalogs.sgml. · ef60de67
      Fujii Masao authored
      The pg_settings view does not display customized options until
      the extension module that defines them has been loaded. This commit
      add the note about that behavior, into the docs.
      
      Author: John Naylor
      Reviewed-by: Tom Lane, Fujii Masao
      Discussion: https://postgr.es/m/CAFBsxsGsBZsG=cLM0Op5HFb2Ks6SzJrOc_eRO_jcKSNuqFRKnQ@mail.gmail.com
      ef60de67
    • Thomas Munro's avatar
      Fix parsePGArray() error checking in pg_dump. · 3636efa1
      Thomas Munro authored
      Coverity complained about a defect in commit 257836a7:
      
        Calling "parsePGArray" without checking return value (as is
        done elsewhere 11 out of 13 times).
      
      Fix, and also check for empty strings explicitly (NULL as represented by
      PQgetvalue()).  That worked correctly before only because parsePGArray()
      happens to set *nitems = 0 when it fails on an empty string.  Also
      convert a sanity check assertion to an error to be more paranoid, and
      pgindent a nearby line.
      Reported-by: default avatarMichael Paquier <michael@paquier.xyz>
      3636efa1
  2. 08 Nov, 2020 4 commits
    • Tom Lane's avatar
      In INSERT/UPDATE, use the table's real tuple descriptor as target. · 8b39345a
      Tom Lane authored
      This back-patches commit 20d3fe90 into the v12 and v13 branches.
      At the time I thought that commit was not fixing any observable
      bug, but Bertrand Drouvot showed otherwise: adding a dropped column
      to the previously-considered scenario crashes v12 and v13, unless the
      dropped column happens to be an integer.  That is, of course, because
      the tupdesc we derive from the plan output tlist fails to describe
      the dropped column accurately, so that we'll do the wrong thing with
      a tuple in which that column isn't NULL.
      
      There is no bug in pre-v12 branches because they already did use
      the table's real tuple descriptor for any trigger-returned tuple.
      It seems that this set of bugs can be blamed on the changes that
      removed es_trig_tuple_slot, though I've not attempted to pin that
      down precisely.
      
      Although there's no code change needed in HEAD, update the test case
      to include a dropped column there too.
      
      Discussion: https://postgr.es/m/db5d97c8-f48a-51e2-7b08-b73d5434d425@amazon.com
      Discussion: https://postgr.es/m/16644-5da7ef98a7ac4545@postgresql.org
      8b39345a
    • Thomas Munro's avatar
      Fix assertion in collation version lookup. · d50e3b1f
      Thomas Munro authored
      Commit 257836a7 included an assertion that a version lookup routine is
      not trying to look up "C" or "POSIX", but that case is reachable with
      the user-facing SQL function pg_collation_actual_version().  Remove the
      assertion.
      d50e3b1f
    • Peter Eisentraut's avatar
      Fix test for error message change · 8cff66d3
      Peter Eisentraut authored
      fix for 6be725e7
      8cff66d3
    • Peter Geoghegan's avatar
      Improve nbtree README's LP_DEAD section. · 5a2f154a
      Peter Geoghegan authored
      The description of how LP_DEAD bit setting by index scans works
      following commit 2ed5b87f was rather unclear.  Clean that up a bit.
      
      Also refer to LP_DEAD bit setting within _bt_check_unique() at the start
      of the same section.  This mechanism may actually be more important than
      the generic kill_prior_tuple mechanism that the section focuses on, so
      it at least deserves to be mentioned in passing.
      5a2f154a
  3. 07 Nov, 2020 9 commits
  4. 06 Nov, 2020 6 commits
  5. 05 Nov, 2020 4 commits
    • Peter Geoghegan's avatar
      Fix wal_consistency_checking nbtree bug. · efc5dcfd
      Peter Geoghegan authored
      wal_consistency_checking indicated an inconsistency in certain cases
      involving nbtree page deletion.  The underlying issue is that there was
      a minor difference between the page image produced after a REDO routine
      ran and the corresponding page image following original execution.
      
      This harmless inconsistency has been around forever.  We more or less
      expect total consistency among even deleted nbtree pages these days,
      though, so this won't do anymore.
      
      To fix, tweak the REDO routine to match original execution.
      
      Oversight in commit f47b5e13.
      efc5dcfd
    • Tom Lane's avatar
      Don't throw an error for LOCK TABLE on a self-referential view. · 5b7bfc39
      Tom Lane authored
      LOCK TABLE has complained about "infinite recursion" when applied
      to a self-referential view, ever since we made it recurse into views
      in v11.  However, that breaks pg_dump's new assumption that it's
      okay to lock every relation.  There doesn't seem to be any good
      reason to throw an error: if we just abandon the recursion, we've
      still satisfied the requirement of locking every referenced relation.
      
      Per bug #16703 from Andrew Bille (via Alexander Lakhin).
      
      Discussion: https://postgr.es/m/16703-e348f58aab3cf6cc@postgresql.org
      5b7bfc39
    • Peter Geoghegan's avatar
      Fix nbtree cleanup-only VACUUM stats inaccuracies. · 48e12913
      Peter Geoghegan authored
      Logic for counting heap TIDs from posting list tuples (added by commit
      0d861bbb) was faulty.  It didn't count any TIDs/index tuples in the
      event of no callback being set.  This meant that we incorrectly counted
      no index tuples in clean-up only VACUUMs, which could lead to
      pg_class.reltuples being spuriously set to 0 in affected indexes.
      
      To fix, go back to counting items from the page in cases where there is
      no callback.  This approach isn't very accurate, but it works well
      enough in practice while avoiding the expense of accessing every index
      tuple during cleanup-only VACUUMs.
      
      Author: Peter Geoghegan <pg@bowt.ie>
      Reported-By: default avatarJehan-Guillaume de Rorthais <jgdr@dalibo.com>
      https://postgr.es/m/20201023174451.69e358f1@firost
      Backpatch: 13-, where nbtree deduplication was introduced
      48e12913
    • Thomas Munro's avatar
      Fix unlinking of SLRU segments. · c732c3f8
      Thomas Munro authored
      Commit dee663f7 intended to drop any queued up fsync requests before
      unlinking segment files, but missed a code path.  Fix, by centralizing
      the forget-and-unlink code into a single function.
      Reported-by: default avatarTomas Vondra <tomas.vondra@2ndquadrant.com>
      Discussion: https://postgr.es/m/20201104013205.icogbi773przyny5%40development
      c732c3f8
  6. 04 Nov, 2020 10 commits
    • Tom Lane's avatar
      Remove underflow error in float division with infinite divisor. · fac83dbd
      Tom Lane authored
      float4_div and float8_div correctly produced zero for zero divided
      by infinity, but threw an underflow error for nonzero finite values
      divided by infinity.  This seems wrong; at the very least it's
      inconsistent with the behavior recently implemented for numeric
      infinities.  Remove the error and allow zero to be returned.
      
      This patch also removes a useless isinf() test from the overflow
      checks in these functions (non-Inf divided by Inf can't produce Inf).
      
      Extracted from a larger patch; this seems significant outside the
      context of geometric operators, so it deserves its own commit.
      
      Kyotaro Horiguchi
      
      Discussion: https://postgr.es/m/CAGf+fX70rWFOk5cd00uMfa__0yP+vtQg5ck7c2Onb-Yczp0URA@mail.gmail.com
      fac83dbd
    • Tom Lane's avatar
      Declare assorted array functions using anycompatible not anyelement. · 9e38c2bb
      Tom Lane authored
      Convert array_append, array_prepend, array_cat, array_position,
      array_positions, array_remove, array_replace, and width_bucket
      to use anycompatiblearray.  This is a simple extension of commit
      5c292e6b to hit some other places where there's a pretty obvious
      gain in usability from doing so.
      
      Ideally we'd also modify other functions taking multiple old-style
      polymorphic arguments.  But most of the remainder are tied into one
      or more operator classes, making any such change a much larger can of
      worms than I desire to open right now.
      
      Discussion: https://postgr.es/m/77675130-89da-dab1-51dd-492c93dcf5d1@postgresfriends.org
      9e38c2bb
    • Tom Lane's avatar
      Declare lead() and lag() using anycompatible not anyelement. · 5c292e6b
      Tom Lane authored
      This allows use of a "default" expression that doesn't slavishly
      match the data column's type.  Formerly you got something like
      "function lag(numeric, integer, integer) does not exist", which
      is not just unhelpful but actively misleading.
      
      The SQL spec suggests that the default should be coerced to the data
      column's type, but this implementation instead chooses the common
      supertype, which seems at least as reasonable.
      
      (Note: I took the opportunity to run "make reformat-dat-files" on
      pg_proc.dat, so this commit includes some cosmetic changes to
      recently-added entries that aren't related to lead/lag.)
      
      Vik Fearing
      
      Discussion: https://postgr.es/m/77675130-89da-dab1-51dd-492c93dcf5d1@postgresfriends.org
      5c292e6b
    • Tom Lane's avatar
      Improve our ability to regurgitate SQL-syntax function calls. · 40c24bfe
      Tom Lane authored
      The SQL spec calls out nonstandard syntax for certain function calls,
      for example substring() with numeric position info is supposed to be
      spelled "SUBSTRING(string FROM start FOR count)".  We accept many
      of these things, but up to now would not print them in the same format,
      instead simplifying down to "substring"(string, start, count).
      That's long annoyed me because it creates an interoperability
      problem: we're gratuitously injecting Postgres-specific syntax into
      what might otherwise be a perfectly spec-compliant view definition.
      However, the real reason for addressing it right now is to support
      a planned change in the semantics of EXTRACT() a/k/a date_part().
      When we switch that to returning numeric, we'll have the parser
      translate EXTRACT() to some new function name (might as well be
      "extract" if you ask me) and then teach ruleutils.c to reverse-list
      that per SQL spec.  In this way existing calls to date_part() will
      continue to have the old semantics.
      
      To implement this, invent a new CoercionForm value COERCE_SQL_SYNTAX,
      and make the parser insert that rather than COERCE_EXPLICIT_CALL when
      the input has SQL-spec decoration.  (But if the input has the form of
      a plain function call, continue to mark it COERCE_EXPLICIT_CALL, even
      if it's calling one of these functions.)  Then ruleutils.c recognizes
      COERCE_SQL_SYNTAX as a cue to emit SQL call syntax.  It can know
      which decoration to emit using hard-wired knowledge about the
      functions that could be called this way.  (While this solution isn't
      extensible without manual additions, neither is the grammar, so this
      doesn't seem unmaintainable.)  Notice that this solution will
      reverse-list a function call with SQL decoration only if it was
      entered that way; so dump-and-reload will not by itself produce any
      changes in the appearance of views.
      
      This requires adding a CoercionForm field to struct FuncCall.
      (I couldn't resist the temptation to rearrange that struct's
      field order a tad while I was at it.)  FuncCall doesn't appear
      in stored rules, so that change isn't a reason for a catversion
      bump, but I did one anyway because the new enum value for
      CoercionForm fields could confuse old backend code.
      
      Possible future work:
      
      * Perhaps CoercionForm should now be renamed to DisplayForm,
      or something like that, to reflect its more general meaning.
      This'd require touching a couple hundred places, so it's not
      clear it's worth the code churn.
      
      * The SQLValueFunction node type, which was invented partly for
      the same goal of improving SQL-compatibility of view output,
      could perhaps be replaced with regular function calls marked
      with COERCE_SQL_SYNTAX.  It's unclear if this would be a net
      code savings, however.
      
      Discussion: https://postgr.es/m/42b73d2d-da12-ba9f-570a-420e0cce19d9@phystech.edu
      40c24bfe
    • Tom Lane's avatar
      Remove useless entries for aggregate functions from fmgrtab.c. · f21636e5
      Tom Lane authored
      Gen_fmgrtab.pl treated aggregate functions the same as other built-in
      functions, which is wasteful because there is no real need to have
      entries for them in the fmgr_builtins[] table.  Suppressing those
      entries saves about 3KB in the compiled table on my machine; which
      is not a lot but it's not nothing either, considering that that
      table is pretty "hot".  The only outside code change needed is
      that ExecInitWindowAgg() can't be allowed to call fmgr_info_cxt()
      on a plain aggregate function.  But that saves a few cycles anyway.
      
      Having done that, the aggregate_dummy() function is unreferenced
      and might as well be dropped.  Using "aggregate_dummy" as the prosrc
      value for an aggregate is now just a documentation convention not
      something that matters.  There was some discussion of using NULL
      instead to save a few bytes in pg_proc, but we'd have to remove
      prosrc's BKI_FORCE_NOT_NULL marking which doesn't seem a great idea.
      Anyway, it's possible there's client-side code that expects to
      see "aggregate_dummy" there, so I'm loath to change it without a
      strong reason.
      
      Discussion: https://postgr.es/m/533989.1604263665@sss.pgh.pa.us
      f21636e5
    • Fujii Masao's avatar
      Fix segmentation fault that commit ac22929a caused. · 113d3591
      Fujii Masao authored
      Commit ac22929a changed recoveryWakeupLatch so that it's reset to
      NULL at the end of recovery. This change could cause a segmentation fault
      in the buildfarm member 'elver'.
      
      Previously the latch was reset to NULL after calling ShutdownWalRcv().
      But there could be a window between ShutdownWalRcv() and the actual
      exit of walreceiver. If walreceiver set the latch during that window,
      the segmentation fault could happen.
      
      To fix the issue, this commit changes walreceiver so that it sets
      the latch only when the latch has not been reset to NULL yet.
      
      Author: Fujii Masao
      Discussion: https://postgr.es/m/5c1f8a85-747c-7bf9-241e-dd467d8a3586@iki.fi
      113d3591
    • Peter Eisentraut's avatar
      Enable hash partitioning of text arrays · 560564d3
      Peter Eisentraut authored
      hash_array_extended() needs to pass PG_GET_COLLATION() to the hash
      function of the element type.  Otherwise, the hash function of a
      collation-aware data type such as text will error out, since the
      introduction of nondeterministic collation made hash functions require
      a collation, too.
      
      The consequence of this is that before this change, hash partitioning
      using an array over text in the partition key would not work.
      Reviewed-by: default avatarHeikki Linnakangas <hlinnaka@iki.fi>
      Reviewed-by: default avatarTom Lane <tgl@sss.pgh.pa.us>
      Reviewed-by: default avatarMichael Paquier <michael@paquier.xyz>
      Discussion: https://www.postgresql.org/message-id/flat/32c1fdae-95c6-5dc6-058a-a90330a3b621%40enterprisedb.com
      560564d3
    • Heikki Linnakangas's avatar
      pg_rewind: Refactor the abstraction to fetch from local/libpq source. · 37d2ff38
      Heikki Linnakangas authored
      This makes the abstraction of a "source" server more clear, by introducing
      a common abstract class, borrowing the object-oriented programming term,
      that represents all the operations that can be done on the source server.
      There are two implementations of it, one for fetching via libpq, and
      another to fetch from a local directory. This adds some code, but makes it
      easier to understand what's going on.
      
      The copy_executeFileMap() and libpq_executeFileMap() functions contained
      basically the same logic, just calling different functions to fetch the
      source files. Refactor so that the common logic is in one place, in a new
      function called perform_rewind().
      
      Reviewed-by: Kyotaro Horiguchi, Soumyadeep Chakraborty
      Discussion: https://www.postgresql.org/message-id/0c5b3783-af52-3ee5-f8fa-6e794061f70d%40iki.fi
      37d2ff38
    • Heikki Linnakangas's avatar
      pg_rewind: Replace the hybrid list+array data structure with simplehash. · f81e97d0
      Heikki Linnakangas authored
      Now that simplehash can be used in frontend code, let's make use of it.
      
      Reviewed-by: Kyotaro Horiguchi, Soumyadeep Chakraborty
      Discussion: https://www.postgresql.org/message-id/0c5b3783-af52-3ee5-f8fa-6e794061f70d%40iki.fi
      f81e97d0
    • Heikki Linnakangas's avatar
      Refactor pg_rewind for more clear decision making. · eb00f1d4
      Heikki Linnakangas authored
      Deciding what to do with each file is now a separate step after all the
      necessary information has been gathered. It is more clear that way.
      Previously, the decision-making was divided between process_source_file()
      and process_target_file(), and it was a bit hard to piece together what
      the overall rules were.
      
      Reviewed-by: Kyotaro Horiguchi, Soumyadeep Chakraborty
      Discussion: https://www.postgresql.org/message-id/0c5b3783-af52-3ee5-f8fa-6e794061f70d%40iki.fi
      eb00f1d4