1. 06 Jul, 2020 6 commits
  2. 05 Jul, 2020 4 commits
  3. 04 Jul, 2020 2 commits
    • Joe Conway's avatar
      Fix "ignoring return value" complaints from commit 96d1f423 · 1d05627f
      Joe Conway authored
      The cfbot and some BF animals are complaining about the previous
      read_binary_file commit because of ignoring return value of ‘fread’.
      So let's make everyone happy by testing the return value even though
      not strictly needed.
      
      Reported by Justin Pryzby, and suggested patch by Tom Lane. Backpatched
      to v11 same as the previous commit.
      
      Reported-By: Justin Pryzby
      Reviewed-By: Tom Lane
      Discussion: https://postgr.es/m/flat/969b8d82-5bb2-5fa8-4eb1-f0e685c5d736%40joeconway.com
      Backpatch-through: 11
      1d05627f
    • Joe Conway's avatar
      Read until EOF vice stat-reported size in read_binary_file · 96d1f423
      Joe Conway authored
      read_binary_file(), used by SQL functions pg_read_file() and friends,
      uses stat to determine file length to read, when not passed an explicit
      length as an argument. This is problematic, for example, if the file
      being read is a virtual file with a stat-reported length of zero.
      Arrange to read until EOF, or StringInfo data string lenth limit, is
      reached instead.
      
      Original complaint and patch by me, with significant review, corrections,
      advice, and code optimizations by Tom Lane. Backpatched to v11. Prior to
      that only paths relative to the data and log dirs were allowed for files,
      so no "zero length" files were reachable anyway.
      
      Reviewed-By: Tom Lane
      Discussion: https://postgr.es/m/flat/969b8d82-5bb2-5fa8-4eb1-f0e685c5d736%40joeconway.com
      Backpatch-through: 11
      96d1f423
  4. 03 Jul, 2020 6 commits
    • Tom Lane's avatar
      Clamp total-tuples estimates for foreign tables to ensure planner sanity. · ca5e93f7
      Tom Lane authored
      After running GetForeignRelSize for a foreign table, adjust rel->tuples
      to be at least as large as rel->rows.  This prevents bizarre behavior
      in estimate_num_groups() and perhaps other places, especially in the
      scenario where rel->tuples is zero because pg_class.reltuples is
      (suggesting that ANALYZE has never been run for the table).  As things
      stood, we'd end up estimating one group out of any GROUP BY on such a
      table, whereas the default group-count estimate is more likely to result
      in a sane plan.
      
      Also, clarify in the documentation that GetForeignRelSize has the option
      to override the rel->tuples value if it has a better idea of what to use
      than what is in pg_class.reltuples.
      
      Per report from Jeff Janes.  Back-patch to all supported branches.
      
      Patch by me; thanks to Etsuro Fujita for review
      
      Discussion: https://postgr.es/m/CAMkU=1xNo9cnan+Npxgz0eK7394xmjmKg-QEm8wYG9P5-CcaqQ@mail.gmail.com
      ca5e93f7
    • Tom Lane's avatar
      Fix temporary tablespaces for shared filesets some more. · f7b5988c
      Tom Lane authored
      Commit ecd9e9f0 fixed the problem in the wrong place, causing unwanted
      side-effects on the behavior of GetNextTempTableSpace().  Instead,
      let's make SharedFileSetInit() responsible for subbing in the value
      of MyDatabaseTableSpace when the default tablespace is called for.
      
      The convention about what is in the tempTableSpaces[] array is
      evidently insufficiently documented, so try to improve that.
      
      It also looks like SharedFileSetInit() is doing the wrong thing in the
      case where temp_tablespaces is empty.  It was hard-wiring use of the
      pg_default tablespace, but it seems like using MyDatabaseTableSpace
      is more consistent with what happens for other temp files.
      
      Back-patch the reversion of PrepareTempTablespaces()'s behavior to
      9.5, as ecd9e9f0 was.  The changes in SharedFileSetInit() go back
      to v11 where that was introduced.  (Note there is net zero code change
      before v11 from these two patch sets, so nothing to release-note.)
      
      Magnus Hagander and Tom Lane
      
      Discussion: https://postgr.es/m/CABUevExg5YEsOvqMxrjoNvb3ApVyH+9jggWGKwTDFyFCVWczGQ@mail.gmail.com
      f7b5988c
    • Tom Lane's avatar
      Inline plpgsql's exec_stmt() into exec_stmts(). · 1f902d49
      Tom Lane authored
      This saves one level of C function call per plpgsql statement executed,
      and permits a tiny additional optimization of not saving and restoring
      estate->err_stmt for each statement in a block.  The net effect seems
      nearly un-measurable on x86_64, but there's a clear win on aarch64,
      amounting to two or three percent in a loop over a few simple plpgsql
      statements.
      
      To do this, we have to get rid of the other existing call sites for
      exec_stmt().  Replace them with exec_toplevel_block(), which is just
      defined to do what exec_stmts() does, but for a single
      PLpgSQL_stmt_block statement.  Hard-wiring the expectation of which
      statement type applies here allows us to skip the dispatch switch,
      making this not much uglier than the previous factorization.
      
      Amit Khandekar, tweaked a bit by me
      
      Discussion: https://postgr.es/m/CAJ3gD9eBNrmUD7WBBLG8ohaZ485H9y+4eihQTgr+K8Lhka3vcQ@mail.gmail.com
      1f902d49
    • Magnus Hagander's avatar
      Fix temporary tablespaces for shared filesets · ecd9e9f0
      Magnus Hagander authored
      A likely copy/paste error in 98e8b480 from  back in 2004 would
      cause temp tablespace to be reset to InvalidOid if temp_tablespaces
      was set to the same value as the primary tablespace in the database.
      This would cause shared filesets (such as for parallel hash joins)
      to ignore them, putting the temporary files in the default tablespace
      instead of the configured one. The bug is in the old code, but it
      appears to have been exposed only once we had shared filesets.
      
      Reviewed-By: Daniel Gustafsson
      Discussion: https://postgr.es/m/CABUevExg5YEsOvqMxrjoNvb3ApVyH+9jggWGKwTDFyFCVWczGQ@mail.gmail.com
      Backpatch-through: 9.5
      ecd9e9f0
    • Fujii Masao's avatar
      doc: Correct description of restart_lsn in pg_replication_slots · 8f9b6d40
      Fujii Masao authored
      Previously the document explained that restart_lsn indicates the LSN of
      oldest WAL won't be automatically removed during checkpoints. But
      since v13 this was no longer true thanks to max_slot_wal_keep_size.
      
      Back-patch to v13 where max_slot_wal_keep_size was added.
      
      Author: Fujii Masao
      Discussion: https://postgr.es/m/6497f1e9-3148-c5da-7e49-b2fddad9a42f@oss.nttdata.com
      8f9b6d40
    • Fujii Masao's avatar
      Change default of pg_stat_statements.track_planning to off. · d1763ea8
      Fujii Masao authored
      Since v13 pg_stat_statements is allowed to track the planning time of
      statements when track_planning option is enabled. Its default was on.
      
      But this feature could cause more terrible spinlock contentions in
      pg_stat_statements. As a result of this, Robins Tharakan reported that
      v13 beta1 showed ~45% performance drop at high DB connection counts
      (when compared with v12.3) during fully-cached SELECT-only test using
      pgbench.
      
      To avoid this performance regression by the default setting,
      this commit changes default of pg_stat_statements.track_planning to off.
      
      Back-patch to v13 where pg_stat_statements.track_planning was introduced.
      
      Reported-by: Robins Tharakan
      Author: Fujii Masao
      Reviewed-by: Julien Rouhaud
      Discussion: https://postgr.es/m/2895b53b033c47ccb22972b589050dd9@EX13D05UWC001.ant.amazon.com
      d1763ea8
  5. 02 Jul, 2020 3 commits
    • Peter Geoghegan's avatar
      Initialize work_mem using current guc.c default. · 947456a8
      Peter Geoghegan authored
      Do the same for the maintenance_work_mem global variable.
      
      Oversight in commit 848ae330, which increased the previous defaults
      for work_mem and maintenance_work_mem by 4X.
      947456a8
    • Peter Geoghegan's avatar
      nbtree: Rename _bt_search() variables. · e25d462a
      Peter Geoghegan authored
      Make some of the variable names in _bt_search() consistent with
      corresponding variables within _bt_getstackbuf().  This naming scheme is
      clearer because the variable names always express a relationship between
      the currently locked buffer/page and some other page.
      e25d462a
    • Michael Paquier's avatar
      Move description of libpqwalreceiver hooks out of the replication's README · 641dd167
      Michael Paquier authored
      src/backend/replication/README includes since 32bc08b1 a basic
      description of the WAL receiver hooks available in walreceiver.h for a
      module like libpqwalreceiver, but the README has never been updated to
      reflect changes done to the hooks, so the contents of the README have
      rotten with the time.  This commit moves the description from the README
      to walreceiver.h, where it will be hard to miss that a description
      update or addition is needed depending on the modifications done to the
      hooks.
      
      Each hook now includes a description of what it does in walreceiver.h,
      and the replication's README mentions walreceiver.h.
      
      Thanks also to Amit Kapila for the discussion.
      
      Author: Michael Paquier
      Reviewed-by: Peter Eisentraut
      Discussion: https://postgr.es/m/20200502024606.GA471944@paquier.xyz
      641dd167
  6. 01 Jul, 2020 5 commits
    • Michael Paquier's avatar
      Refactor ObjectAddress field assignments in more places · 4315e8c2
      Michael Paquier authored
      This is a follow-up commit similar to 68de1440, with more places in the
      backend code simplified with the macros able to assign values to the
      fields of ObjectAddress.  The code paths changed here could be
      transitioned later into using more grouping when inserting dependency
      records, simplifying this future work.
      
      Author: Daniel Gustafsson, Michael Paquier
      Discussion: https://postgr.es/m/20190213182737.mxn6hkdxwrzgxk35@alap3.anarazel.de
      4315e8c2
    • Amit Kapila's avatar
      Improve vacuum error context handling. · a69e041d
      Amit Kapila authored
      Use separate functions to save and restore error context information as
      that made code easier to understand.  Also, make it clear that the index
      information required for error context is sane.
      
      Author: Andres Freund, Justin Pryzby, Amit Kapila
      Backpatch-through: 13, where it was introduced
      Discussion: https://postgr.es/m/CAA4eK1LWo+v1OWu=Sky27GTGSCuOmr7iaURNbc5xz6jO+SaPeA@mail.gmail.com
      a69e041d
    • Michael Paquier's avatar
      Refactor creation of normal dependency records when creating extension · 684b4f29
      Michael Paquier authored
      When creating an extension, the same type of dependency is used when
      registering a dependency to a schema and required extensions.  This
      improves the code so as those dependencies are not recorded one-by-one,
      but grouped together.  Note that this has as side effect to remove
      duplicate dependency entries, even if it should not happen in practice
      as extensions listed as required in a control file should be listed only
      once.
      
      Extracted from a larger patch by the same author.
      
      Author: Daniel Dustafsson
      Discussion: https://postgr.es/m/20200629065535.GA183079@paquier.xyz
      684b4f29
    • Michael Paquier's avatar
      Fix removal of files generated by TAP tests for SSL · c4342c93
      Michael Paquier authored
      001_ssltests.pl and 002_scram.pl both generated an extra file for a
      client key used in the tests that were not removed.  In Debian, this
      causes repeated builds to fail.
      
      The code refactoring done in 4dc63552 broke the cleanup done in
      001_ssltests.pl, and the new tests added in 002_scram.pl via d6e612f8
      forgot the removal of one file.  While on it, fix a second issue
      introduced in 002_scram.pl where we use the same file name in 001 and
      002 for the temporary client key whose permissions are changed in the
      test, as using the same file name in both tests could cause failures
      with parallel jobs of src/test/ssl/ if one test removes a file still
      needed by the second test.
      
      Reported-by: Felix Lechner
      Author: Daniel Gustafsson, Felix Lechner
      Reviewed-by: Tom Lane, Michael Paquier
      Discussion: https://postgr.es/m/CAFHYt543sjX=Cm_aEeoejStyP47C+Y3+Wh6WbirLXsgUMaw7iw@mail.gmail.com
      Backpatch-through: 13
      c4342c93
    • David Rowley's avatar
      Further adjustments to Hashagg EXPLAIN ANALYZE output · 40efbf87
      David Rowley authored
      The "Disk Usage" and "HashAgg Batches" properties in the EXPLAIN ANALYZE
      output for HashAgg were previously only shown if the number of batches
      was greater than 0.  Here we change this so that these properties are
      always shown for EXPLAIN ANALYZE formats other than "text".  The idea here
      is that since the HashAgg could have spilled to disk if there had been
      more data or groups to aggregate, then it's relevant that we're clear in
      the EXPLAIN ANALYZE output when no spilling occurred in this particular
      execution of the given plan.
      
      For the "text" EXPLAIN format, we still hide these properties when no
      spilling occurs.  This EXPLAIN format is designed to be easy for humans
      to read.  To maintain the readability we have a higher threshold for which
      properties we display for this format.
      
      Discussion: https://postgr.es/m/CAApHDvo_dmNozQQTmN-2jGp1vT%3Ddxx7Q0vd%2BMvD1cGpv2HU%3DSg%40mail.gmail.com
      Backpatch-through: 13, where the hashagg spilling code was added.
      40efbf87
  7. 30 Jun, 2020 5 commits
  8. 29 Jun, 2020 8 commits
    • Peter Eisentraut's avatar
      pgstattuple: Have pgstattuple_approx accept TOAST tables · ee0202d5
      Peter Eisentraut authored
      TOAST tables have a visibility map and a free space map, so they can
      be supported by pgstattuple_approx just fine.
      
      Add test cases to show how various pgstattuple functions accept TOAST
      tables.  Also add similar tests to pg_visibility, which already
      accepted TOAST tables correctly but had no test coverage for them.
      Reviewed-by: default avatarLaurenz Albe <laurenz.albe@cybertec.at>
      Discussion: https://www.postgresql.org/message-id/flat/27c4496a-02b9-dc87-8f6f-bddbef54e0fe@2ndquadrant.com
      ee0202d5
    • Tom Lane's avatar
      Remove support for timezone "posixrules" file. · ea57e531
      Tom Lane authored
      The IANA tzcode library has a feature to read a time zone file named
      "posixrules" and apply the daylight-savings transition dates and times
      therein, when it is given a POSIX-style time zone specification that
      lacks an explicit transition rule.  However, there's a problem with
      that code: it doesn't work for dates past the Y2038 time_t rollover.
      (Effectively, all times beyond that point are treated as standard
      time.)  The IANA crew regard this feature as legacy, so their plan is
      to remove it not fix it.  The time frame in which that will happen
      is unclear, but presumably it'll happen well before 2038.
      
      Moreover, effective with the next IANA data update (probably this
      fall), the recommended default will be to not install a "posixrules"
      file in the first place.  The time frame in which tzdata packagers
      might adopt that suggestion is likewise unclear, but at least some
      platforms will probably do it in the next year or so.  While we could
      ignore that recommendation so far as PG-supplied tzdata trees are
      concerned, builds using --with-system-tzdata will be subject to
      whatever the platform's tzdata packager decides to do.
      
      Thus, whether or not we do anything, some increasing fraction of
      Postgres users will be exposed to the behavior observed when there
      is no "posixrules" file; and if we do nothing, we'll have essentially
      no control over the timing of that change.
      
      The best thing to do to ameliorate the uncertainty seems to be to
      proactively remove the posixrules-reading feature.  If we do that in
      a scheduled release then at least we can release-note the behavioral
      change, rather than having users be surprised by it after a routine
      tzdata update.
      
      The change in question is fairly minor anyway: to be affected,
      you have to be using a POSIX-style timezone spec, it has to not
      have an explicit rule, and it has to not be one of the four traditional
      continental-USA zone names (EST5EDT, CST6CDT, MST7MDT, or PST8PDT),
      as those are special-cased.  Since the default "posixrules" file
      provides USA DST rules, the number of people who are likely to find
      such a zone spec useful is probably quite small.  Moreover, the
      fallback behavior with no explicit rule and no "posixrules" file is to
      apply current USA rules, so the only thing that really breaks is the
      DST transitions in years before 2007 (and you get the countervailing
      fix that transitions after 2038 will be applied).
      
      Now, some installations might have replaced the "posixrules" file,
      allowing e.g. EU rules to be applied to a POSIX-style timezone spec.
      That won't work anymore.  But it's not exactly clear why this solution
      would be preferable to using a regular named zone.  In any case, given
      the Y2038 issue, we need to be pushing users to stop depending on this.
      
      Back-patch into v13; it hasn't been released yet, so it seems OK to
      change its behavior.  (Personally I think we ought to back-patch
      further, but I've been outvoted.)
      
      Discussion: https://postgr.es/m/1390.1562258309@sss.pgh.pa.us
      Discussion: https://postgr.es/m/20200621211855.6211-1-eggert@cs.ucla.edu
      ea57e531
    • Tom Lane's avatar
      Mop up some no-longer-necessary hacks around printf %.*s format. · c410af09
      Tom Lane authored
      Commit 54cd4f04 added some kluges to work around an old glibc bug,
      namely that %.*s could misbehave if glibc thought any characters in
      the supplied string were incorrectly encoded.  Now that we use our
      own snprintf.c implementation, we need not worry about that bug (even
      if it still exists in the wild).  Revert a couple of particularly
      ugly hacks, and remove or improve assorted comments.
      
      Note that there can still be encoding-related hazards here: blindly
      clipping at a fixed length risks producing wrongly-encoded output
      if the clip splits a multibyte character.  However, code that's
      doing correct multibyte-aware clipping doesn't really need a comment
      about that, while code that isn't needs an explanation why not,
      rather than a red-herring comment about an obsolete bug.
      
      Discussion: https://postgr.es/m/279428.1593373684@sss.pgh.pa.us
      c410af09
    • Peter Geoghegan's avatar
      nbtree: Correct inaccurate split location comment. · f7a476f0
      Peter Geoghegan authored
      Minor oversight in commit fab25024.
      f7a476f0
    • Tom Lane's avatar
      Avoid using %c printf format for potentially non-ASCII characters. · 16e3ad5d
      Tom Lane authored
      Since %c only passes a C "char" to printf, it's incapable of dealing
      with multibyte characters.  Passing just the first byte of such a
      character leads to an output string that is visibly not correctly
      encoded, resulting in undesirable behavior such as encoding conversion
      failures while sending error messages to clients.
      
      We've lived with this issue for a long time because it was inconvenient
      to avoid in a portable fashion.  However, now that we always use our own
      snprintf code, it's reasonable to use the %.*s format to print just one
      possibly-multibyte character in a string.  (We previously avoided that
      obvious-looking answer in order to work around glibc's bug #6530, cf
      commits 54cd4f04 and ed437e2b.)
      
      Hence, run around and fix a bunch of places that used %c to report
      a character found in a user-supplied string.  For simplicity, I did
      not touch places that were emitting non-user-facing debug messages,
      or reporting catalog data that should always be ASCII.  (It's also
      unclear how useful this approach could be in frontend code, where
      it's less certain that we know what encoding we're dealing with.)
      
      In passing, improve a couple of poorly-written error messages in
      pageinspect/heapfuncs.c.
      
      This is a longstanding issue, but I'm hesitant to back-patch because
      of the impact on translatable message strings.  In any case this fix
      would not work reliably before v12.
      
      Tom Lane and Quan Zongliang
      
      Discussion: https://postgr.es/m/a120087c-4c88-d9d4-1ec5-808d7a7f133d@gmail.com
      16e3ad5d
    • Peter Eisentraut's avatar
      Add current substring regular expression syntax · 78c88767
      Peter Eisentraut authored
      SQL:1999 had syntax
      
          SUBSTRING(text FROM pattern FOR escapechar)
      
      but this was replaced in SQL:2003 by the more clear
      
          SUBSTRING(text SIMILAR pattern ESCAPE escapechar)
      
      but this was never implemented in PostgreSQL.  This patch adds that
      new syntax as an alternative in the parser, and updates documentation
      and tests to indicate that this is the preferred alternative now.
      Reviewed-by: default avatarPavel Stehule <pavel.stehule@gmail.com>
      Reviewed-by: default avatarVik Fearing <vik@postgresfriends.org>
      Reviewed-by: default avatarFabien COELHO <coelho@cri.ensmp.fr>
      Discussion: https://www.postgresql.org/message-id/flat/a15db31c-d0f8-8ce0-9039-578a31758adb%402ndquadrant.com
      78c88767
    • Peter Eisentraut's avatar
      Clean up grammar a bit · aafefb4d
      Peter Eisentraut authored
      Simplify the grammar specification of substring() and overlay() a bit,
      simplify and update some comments.
      Reviewed-by: default avatarPavel Stehule <pavel.stehule@gmail.com>
      Reviewed-by: default avatarVik Fearing <vik@postgresfriends.org>
      Reviewed-by: default avatarFabien COELHO <coelho@cri.ensmp.fr>
      Discussion: https://www.postgresql.org/message-id/flat/a15db31c-d0f8-8ce0-9039-578a31758adb%402ndquadrant.com
      aafefb4d
    • Michael Paquier's avatar
      Refactor ObjectAddress field assignments for type dependencies · 68de1440
      Michael Paquier authored
      The logic used to build the set of dependencies needed for a type is
      rather repetitive with direct assignments for each ObjectAddress field.
      This refactors the code to use the macro ObjectAddressSet() instead, to
      do the same work.  There are more areas of the backend code that could
      use this macro, but these are left for a follow-up patch that will
      partially rework the way dependencies are recorded as well.  Type
      dependencies are left out of the follow-up patch, so they are refactored
      separately here.
      
      Extracted from a larger patch by the same author.
      
      Author: Daniel Gustafsson
      Discussion: https://potgr.es/m/20190213182737.mxn6hkdxwrzgxk35@alap3.anarazel.de
      68de1440
  9. 28 Jun, 2020 1 commit
    • Noah Misch's avatar
      Fix documentation of "must be vacuumed within" warning. · 96879a0e
      Noah Misch authored
      Warnings start 10M transactions before xidStopLimit, which is 11M
      transactions before wraparound.  The sample WARNING output showed a
      value greater than 11M, and its HINT message predated commit
      25ec228e.  Hence, the sample was
      impossible.  Back-patch to 9.5 (all supported versions).
      96879a0e