1. 22 Apr, 2020 7 commits
  2. 21 Apr, 2020 13 commits
    • Michael Paquier's avatar
      Fix single-record reads to use restore_command if available in pg_rewind · cd123234
      Michael Paquier authored
      readOneRecord() is used now when looking for a checkpoint record to
      check if the target server is an ancestor of the source across multiple
      timelines, and using a restore_command if available improves the
      stability of the operation.  This part was missed in a7e8ece4.
      
      Reported-by: Kyotaro Horiguchi
      Discussion: https://postgr.es/m/20200421.150830.1410714948345179794.horikyota.ntt@gmail.com
      cd123234
    • Alvaro Herrera's avatar
      psql \d: Display table where trigger is defined, if inherited · c33869cc
      Alvaro Herrera authored
      It's important to know that a trigger is cloned from a parent table,
      because of the behavior that the trigger is dropped on detach.  Make
      psql's \d display it.
      
      We'd like to backpatch, but lack of the pg_trigger.tgparentid column
      makes it more difficult.  Punt for now.  If somebody wants to volunteer
      an implementation that reads pg_depend on older versions, that can
      probably be backpatched.
      
      Authors: Justin Pryzby, Amit Langote, Álvaro Herrera
      Discussion: https://postgr.es/m/20200419002206.GM26953@telsasoft.com
      c33869cc
    • Michael Paquier's avatar
      Fix memory leak in libpq when using sslmode=verify-full · 27dbe1a1
      Michael Paquier authored
      Checking if Subject Alternative Names (SANs) from a certificate match
      with the hostname connected to leaked memory after each lookup done.
      
      This is broken since acd08d76 that added support for SANs in SSL
      certificates, so backpatch down to 9.5.
      
      Author: Roman Peshkurov
      Reviewed-by: Hamid Akhtar, Michael Paquier, David Steele
      Discussion: https://postgr.es/m/CALLDf-pZ-E3mjxd5=bnHsDu9zHEOnpgPgdnO84E2RuwMCjjyPw@mail.gmail.com
      Backpatch-through: 9.5
      27dbe1a1
    • Alvaro Herrera's avatar
      Document partitiong tables ancillary object handling some more · 8803506c
      Alvaro Herrera authored
      Add a couple of lines to make it explicit that indexes, constraints,
      triggers are added, removed, or left alone.
      
      Backpatch to pg11.
      
      Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
      Reviewed-by: default avatarJustin Pryzby <pryzby@telsasoft.com>
      Discussion: https://postgr.es/m/20200421162038.GA18628@alvherre.pgsql
      8803506c
    • Tom Lane's avatar
      Fix possible crash during FATAL exit from reindexing. · d12bdba7
      Tom Lane authored
      index.c supposed that it could just use a PG_TRY block to clean up the
      state associated with an active REINDEX operation.  However, that code
      doesn't run if we do a FATAL exit --- for example, due to a SIGTERM
      shutdown signal --- while the REINDEX is happening.  And that state does
      get consulted during catalog accesses, which makes it problematic if we
      do any catalog accesses during shutdown --- for example, to clean up any
      temp tables created in the session.
      
      If this combination of circumstances occurred, we could find ourselves
      trying to access already-freed memory.  In debug builds that'd fairly
      reliably cause an assertion failure.  In production we might often
      get away with it, but with some bad luck it could cause a core dump.
      
      Another possible bad outcome is an erroneous conclusion that an
      index-to-be-accessed is being reindexed; but it looks like that would
      be unlikely to have any consequences worse than failing to drop temp
      tables right away.  (They'd still get dropped by the next session that
      uses that temp schema.)
      
      To fix, get rid of the use of PG_TRY here, and instead hook into
      the transaction abort mechanisms to clean up reindex state.
      
      Per bug #16378 from Alexander Lakhin.  This has been wrong for a
      very long time, so back-patch to all supported branches.
      
      Discussion: https://postgr.es/m/16378-7a70ca41b3ec2009@postgresql.org
      d12bdba7
    • Tom Lane's avatar
      Fix minor violations of FunctionCallInvoke usage protocol. · 5836d326
      Tom Lane authored
      Working on commit 1c455078 led me to check through FunctionCallInvoke
      call sites to see if every one was being honest about (a) making sure
      that fcinfo.isnull is initially false, and (b) checking its state after
      the call.  Sure enough, I found some violations.
      
      The main one is that finalize_partialaggregate re-used serialfn_fcinfo
      without resetting isnull, even though it clearly intends to cater for
      serialfns that return NULL.  There would only be an issue with a
      non-strict serialfn, since it's unlikely that a serialfn would return
      NULL for non-null input.  We have no non-strict serialfns in core, and
      there may be none in the wild either, which would account for the lack
      of complaints.  Still, it's clearly wrong, so back-patch that fix to
      9.6 where finalize_partialaggregate was introduced.
      
      Also, arrayfuncs.c and rowtypes.c contained various callers that were
      not bothering to check for result nulls.  While what's being called is
      a comparison or hash function that probably *shouldn't* return null,
      that's a lousy excuse for not having any check at all.  There are
      existing places that just Assert(!fcinfo->isnull) in comparable
      situations, so I added that to the places that were calling btree
      comparison or hash support functions.  In the places calling
      boolean-returning equality functions, it's quite cheap to have them
      treat isnull as FALSE, so make those places do that.  Also remove some
      "locfcinfo->isnull = false" assignments that are unnecessary given the
      assumption that no previous call returned null.  These changes seem like
      mostly neatnik-ism or debugging support, so I didn't back-patch.
      5836d326
    • Alvaro Herrera's avatar
      Fix detaching partitions with cloned row triggers · afccd76f
      Alvaro Herrera authored
      When a partition is detached, any triggers that had been cloned from its
      parent were not properly disentangled from its parent triggers.
      This resulted in triggers that could not be dropped because they
      depended on the trigger in the trigger in the no-longer-parent table:
        ALTER TABLE t DETACH PARTITION t1;
        DROP TRIGGER trig ON t1;
          ERROR:  cannot drop trigger trig on table t1 because trigger trig on table t requires it
          HINT:  You can drop trigger trig on table t instead.
      
      Moreover the table can no longer be re-attached to its parent, because
      the trigger name is already taken:
        ALTER TABLE t ATTACH PARTITION t1 FOR VALUES FROM (1)TO(2);
          ERROR:  trigger "trig" for relation "t1" already exists
      
      The former is a bug introduced in commit 86f57594.  (The latter is
      not necessarily a bug, but it makes the bug more uncomfortable.)
      
      To avoid the complexity that would be needed to tell whether the trigger
      has a local definition that has to be merged with the one coming from
      the parent table, establish the behavior that the trigger is removed
      when the table is detached.
      
      Backpatch to pg11.
      
      Author: Justin Pryzby <pryzby@telsasoft.com>
      Reviewed-by: default avatarAmit Langote <amitlangote09@gmail.com>
      Reviewed-by: default avatarÁlvaro Herrera <alvherre@alvh.no-ip.org>
      Discussion: https://www.postgresql.org/message-id/flat/20200408152412.GZ2228@telsasoft.com
      afccd76f
    • Peter Geoghegan's avatar
      Consider outliers in split interval calculation. · 1542e16f
      Peter Geoghegan authored
      Commit 0d861bbb, which introduced deduplication to nbtree, added some
      logic to take large posting list tuples into account when choosing a
      split point.  We subtract firstright posting list overhead from the
      projected new high key size when calculating leftfree/rightfree values
      for an affected candidate split point.  Posting list tuples aren't
      special to nbtsplitloc.c, but taking them into account like this makes a
      huge difference in practice.  Posting list tuples are frequently tuple
      size outliers.
      
      However, commit 0d861bbb missed a closely related issue: split interval
      itself is calculated based on the assumption that tuples on the page
      being split are roughly equisized.  That assumption was acceptable back
      when commit fab25024 taught the logic for choosing a split point about
      suffix truncation, but it's pretty questionable now that very large
      tuple sizes are common.  This oversight led to unbalanced page splits in
      low cardinality multi-column indexes when deduplication was used: page
      splits that don't give sufficient weight to how unbalanced the split is
      when the interval happens to include some large posting list tuples (and
      when most other tuples on the page are not so large).
      
      Nail this down by calculating an initial split interval in a way that's
      attuned to the actual cost that we want to keep under control (not a
      fuzzy proxy for the cost): apply a leftfree + rightfree evenness test to
      each candidate split point that actually gets included in the split
      interval (for the default strategy).  This replaces logic that used a
      percentage of all legal split points for the page as the basis of the
      initial split interval.
      
      Discussion: https://postgr.es/m/CAH2-WznJt5aT2uUB2Bs+JBLdwe0XTX67+xeLFcaNvCKxO=QBVQ@mail.gmail.com
      1542e16f
    • Tom Lane's avatar
      Allow matchingsel() to be used with operators that might return NULL. · 1c455078
      Tom Lane authored
      Although selfuncs.c will never call a target operator with null inputs,
      some functions might return null anyway.  The existing coding will fail
      if that happens (since FunctionCall2Coll will punt), which seems
      undesirable given that matchingsel() has such a broad range of potential
      applicability --- in fact, we already have a problem because we apply it
      to jsonb_path_exists_opr, which can return null.  Hence, rejigger the
      underlying functions mcv_selectivity and histogram_selectivity to cope,
      treating a null result as false.
      
      While we are at it, we can move the InitFunctionCallInfoData overhead
      out of the inner loops, which isn't a huge number of cycles but might
      save something considering we are likely calling functions as cheap
      as int4eq().  Plus, the number of loop cycles to be expected is much
      more than it was when this code was written, since typical settings
      of default_statistics_target are higher.
      
      In view of that consideration, let's apply the same change to
      var_eq_const, eqjoinsel_inner, and eqjoinsel_semi.  We do not expect
      equality functions to ever return null for non-null inputs (and
      certainly that code has been that way a long time without complaints),
      but the cycle savings seem attractive, especially in the eqjoinsel loops
      where there's potentially an O(N^2) savings.
      
      Similar code exists in ineq_histogram_selectivity and
      get_variable_range, but I forebore from changing those for now.
      The performance argument for changing ineq_histogram_selectivity
      is really weak anyway, since that will only iterate log2(N) times.
      
      Nikita Glukhov and Tom Lane
      
      Discussion: https://postgr.es/m/9d3b0959-95d6-c37e-2c0b-287bcfe5c705@postgrespro.ru
      1c455078
    • Tom Lane's avatar
      Clean up cpluspluscheck violation. · 9d25e1aa
      Tom Lane authored
      "operator" is a reserved word in C++, so per project conventions,
      don't use it as an identifier in header files.
      
      My oversight in commit a8081860.
      9d25e1aa
    • Tom Lane's avatar
      Fix duplicate typedef from commit 0d8c9c12. · 2117c3cb
      Tom Lane authored
      Older gcc versions don't like duplicate typedefs, so get rid of
      that in favor of doing it like we do it elsewhere, ie just use
      a "struct" declaration when trying to avoid importing a whole
      header file.
      
      Also, there seems no reason to include stringinfo.h here at all,
      so get rid of that addition too.
      
      Discussion: https://postgr.es/m/27239.1587415696@sss.pgh.pa.us
      2117c3cb
    • Fujii Masao's avatar
      Mention pg_promote() as a method to trigger promotion in documentation. · 67f82e96
      Fujii Masao authored
      Previously in the "Standby Server Operation" section, pg_ctl promote and
      protmote_trigger_file were documented as a method to trigger standby
      promotion, but pg_promote() function not.
      
      This commit also adds parentheses into <function>pg_promote</function>
      in some docs to make it clearer that a function is being referred to.
      
      Author: Masahiro Ikeda
      Reviewed-by: Michael Paquier, Laurenz Albe, Tom Lane, Fujii Masao
      Discussion: https://postgr.es/m/de0068417a9f4046bac693cbcc00bdc9@oss.nttdata.com
      67f82e96
    • Bruce Momjian's avatar
      doc: change SGML markup "figure" to "example" · f192312d
      Bruce Momjian authored
      Reported-by: Jürgen Purtz
      
      Discussion: https://postgr.es/m/709d7809-d7f4-8175-47f3-4d131341bba8@purtz.de
      
      Author: Jürgen Purtz
      
      Backpatch-through: 9.5
      f192312d
  3. 20 Apr, 2020 6 commits
  4. 19 Apr, 2020 4 commits
    • Tom Lane's avatar
      Doc: update the rest of section 9.4 for new function table layout. · 9aece5cd
      Tom Lane authored
      Notably, this replaces the previous handwaving about these functions'
      behavior with "character"-type inputs with some actual facts.
      9aece5cd
    • Jeff Davis's avatar
      0cacb2b7
    • Tom Lane's avatar
      Doc: update sections 9.1-9.3 for new function table layout. · 81e83216
      Tom Lane authored
      I took the opportunity to do some copy-editing in this area as well,
      and to add some new material such as a note about BETWEEN's syntactical
      peculiarities.
      
      Of note is that quite a few of the examples of transcendental functions
      needed to be updated, because the displayed output no longer matched
      what you get on a modern server.  I believe some of these cases are
      side-effects of the new Ryu algorithm in float8out.  Others appear to be
      because the examples predate the addition of type numeric, and were
      expecting that float8 calculations would be done although the given
      syntax would actually lead to calling the numeric function nowadays.
      81e83216
    • Peter Eisentraut's avatar
      Fix update-unicode target · 73afabcd
      Peter Eisentraut authored
      The normalization-check target needs to be run last, after moving the
      newly generated files into place.  Also, we need an additional
      dependency so that unicode_norm.o is rebuilt first.  Otherwise,
      norm_test will still test the old files but against the new expected
      results, which will probably fail.
      73afabcd
  5. 18 Apr, 2020 4 commits
    • Tom Lane's avatar
      Doc: sync functableentry markup choices with website style. · 00f4aba4
      Tom Lane authored
      Jonathan Katz felt that slightly different indentation settings made
      for a better-looking result, so sync stylesheet-fo.xsl (for PDF) and
      stylesheet.css (for non-website-style HTML) with those choices.
      
      Discussion: https://postgr.es/m/31464.1587156281@sss.pgh.pa.us
      00f4aba4
    • Tom Lane's avatar
      Fix race conditions in synchronous standby management. · f332241a
      Tom Lane authored
      We have repeatedly seen the buildfarm reach the Assert(false) in
      SyncRepGetSyncStandbysPriority.  This apparently is due to failing to
      consider the possibility that the sync_standby_priority values in
      shared memory might be inconsistent; but they will be whenever only
      some of the walsenders have updated their values after a change in
      the synchronous_standby_names setting.  That function is vastly too
      complex for what it does, anyway, so rewriting it seems better than
      trying to apply a band-aid fix.
      
      Furthermore, the API of SyncRepGetSyncStandbys is broken by design:
      it returns a list of WalSnd array indexes, but there is nothing
      guaranteeing that the contents of the WalSnd array remain stable.
      Thus, if some walsender exits and then a new walsender process
      takes over that WalSnd array slot, a caller might make use of
      WAL position data that it should not, potentially leading to
      incorrect decisions about whether to release transactions that
      are waiting for synchronous commit.
      
      To fix, replace SyncRepGetSyncStandbys with a new function
      SyncRepGetCandidateStandbys that copies all the required data
      from shared memory while holding the relevant mutexes.  If the
      associated walsender process then exits, this data is still safe to
      make release decisions with, since we know that that much WAL *was*
      sent to a valid standby server.  This incidentally means that we no
      longer need to treat sync_standby_priority as protected by the
      SyncRepLock rather than the per-walsender mutex.
      
      SyncRepGetSyncStandbys is no longer used by the core code, so remove
      it entirely in HEAD.  However, it seems possible that external code is
      relying on that function, so do not remove it from the back branches.
      Instead, just remove the known-incorrect Assert.  When the bug occurs,
      the function will return a too-short list, which callers should treat
      as meaning there are not enough sync standbys, which seems like a
      reasonably safe fallback until the inconsistent state is resolved.
      Moreover it's bug-compatible with what has been happening in non-assert
      builds.  We cannot do anything about the walsender-replacement race
      condition without an API/ABI break.
      
      The bogus assertion exists back to 9.6, but 9.6 is sufficiently
      different from the later branches that the patch doesn't apply at all.
      I chose to just remove the bogus assertion in 9.6, feeling that the
      probability of a bad outcome from the walsender-replacement race
      condition is too low to justify rewriting the whole patch for 9.6.
      
      Discussion: https://postgr.es/m/21519.1585272409@sss.pgh.pa.us
      f332241a
    • David Rowley's avatar
      Fix possible crash with GENERATED ALWAYS columns · 3cb02e30
      David Rowley authored
      In some corner cases, this could also lead to corrupted values being
      included in the tuple.
      
      Users who are concerned that they are affected by this should first
      upgrade and then perform a base backup of their database and restore onto
      an off-line server. They should then query each table with generated
      columns to ensure there are no rows where the generated expression does
      not match a newly calculated version of the GENERATED ALWAYS expression.
      If no crashes occur and no rows are returned then you're not affected.
      
      Fixes bug #16369.
      
      Reported-by: Cameron Ezell
      Discussion: https://postgr.es/m/16369-5845a6f1bef59884@postgresql.org
      Backpatch-through: 12 (where GENERATED ALWAYS columns were added.)
      3cb02e30
    • Tom Lane's avatar
      Doc: revise formatting of function/operator tables. · 737d69ff
      Tom Lane authored
      The table layout ideas proposed in commit e894c618 were not as widely
      popular as I'd hoped.  After discussion, we've settled on a layout
      that's effectively a single-column table with cell contents much like a
      <varlistentry> description of the function or operator; though we're not
      actually using <varlistentry>, because it'd add way too much vertical
      space.  Instead the effect is accomplished using line-break processing
      instructions to separate the description and example(s), plus CSS or FO
      customizations to produce indentation of all but the first line in each
      cell.  While technically this is a bit grotty, it does have the
      advantage that we won't need to write nearly as much boilerplate markup.
      
      This patch updates tables 9.30, 9.31, and 9.33 (which were touched by
      the previous patch) to the revised style, and additionally converts
      table 9.10.  A lot of work still remains to do, but hopefully it won't
      be too controversial.
      
      Thanks to Andrew Dunstan, Pierre Giraud, Robert Haas, Alvaro Herrera,
      David Johnston, Jonathan Katz, Isaac Morland for valuable ideas.
      
      Discussion: https://postgr.es/m/8691.1586798003@sss.pgh.pa.us
      737d69ff
  6. 17 Apr, 2020 6 commits