1. 19 Feb, 2020 9 commits
    • Peter Eisentraut's avatar
      Fix typo · 2f9c46a3
      Peter Eisentraut authored
      Reported-by: default avatarDaniel Verite <daniel@manitou-mail.org>
      2f9c46a3
    • Tom Lane's avatar
      Fix confusion about event trigger vs. plain function in plpgsql. · 761a5688
      Tom Lane authored
      The function hash table keys made by compute_function_hashkey() failed
      to distinguish event-trigger call context from regular call context.
      This meant that once we'd successfully made a hash entry for an event
      trigger (either by validation, or by normal use as an event trigger),
      an attempt to call the trigger function as a plain function would
      find this hash entry and thereby bypass the you-can't-do-that check in
      do_compile().  Thus we'd attempt to execute the function, leading to
      strange errors or even crashes, depending on function contents and
      server version.
      
      To fix, add an isEventTrigger field to PLpgSQL_func_hashkey,
      paralleling the longstanding infrastructure for regular triggers.
      This fits into what had been pad space, so there's no risk of an ABI
      break, even assuming that any third-party code is looking at these
      hash keys.  (I considered replacing isTrigger with a PLpgSQL_trigtype
      enum field, but felt that that carried some API/ABI risk.  Maybe we
      should change it in HEAD though.)
      
      Per bug #16266 from Alexander Lakhin.  This has been broken since
      event triggers were invented, so back-patch to all supported branches.
      
      Discussion: https://postgr.es/m/16266-fcd7f838e97ba5d4@postgresql.org
      761a5688
    • Peter Eisentraut's avatar
      Set gen_random_uuid() to volatile · 2ed19a48
      Peter Eisentraut authored
      It was set to immutable.  This was a mistake in the initial
      commit (5925e554).
      Reported-by: default avatarhubert depesz lubaczewski <depesz@depesz.com>
      Discussion: https://www.postgresql.org/message-id/flat/20200218185452.GA8710%40depesz.com
      2ed19a48
    • Jeff Davis's avatar
      Minor refactor of nodeAgg.c. · 5b618e1f
      Jeff Davis authored
        * Separate calculation of hash value from the lookup.
        * Split build_hash_table() into two functions.
        * Change lookup_hash_entry() to return AggStatePerGroup. That's all
          the caller needed, anyway.
      
      These changes are to support the upcoming Disk-based Hash Aggregation
      work.
      
      Discussion: https://postgr.es/m/31f5ab871a3ad5a1a91a7a797651f20e77ac7ce3.camel%40j-davis.com
      5b618e1f
    • Jeff Davis's avatar
      logtape.c: allocate read buffer even for an empty tape. · 8021985d
      Jeff Davis authored
      Prior to this commit, the read buffer was allocated at the time the tape
      was rewound; but as an optimization, would not be allocated at all if
      the tape was empty.
      
      That optimization meant that it was valid to have a rewound tape with
      the buffer set to NULL, but only if a number of conditions were met
      and only if the API was used properly. After 7fdd919a refactored the
      code to support lazily-allocating the buffer, Coverity started
      complaining.
      
      The optimization for empty tapes doesn't seem important, so just
      allocate the buffer whether the tape has any data or not.
      
      Discussion: https://postgr.es/m/20351.1581868306%40sss.pgh.pa.us
      8021985d
    • Fujii Masao's avatar
      Fix mesurement of elapsed time during truncating heap in VACUUM. · 00749197
      Fujii Masao authored
      VACUUM may truncate heap in several batches. The activity report
      is logged for each batch, and contains the number of pages in the table
      before and after the truncation, and also the elapsed time during
      the truncation. Previously the elapsed time reported in each batch was
      the total elapsed time since starting the truncation until finishing
      each batch. For example, if the truncation was processed dividing into
      three batches, the second batch reported the accumulated time elapsed
      during both first and second batches. This is strange and confusing
      because the number of pages in the table reported together is not
      total. Instead, each batch should report the time elapsed during
      only that batch.
      
      The cause of this issue was that the resource usage snapshot was
      initialized only at the beginning of the truncation and was never
      reset later. This commit fixes the issue by changing VACUUM so that
      the resource usage snapshot is reset at each batch.
      
      Back-patch to all supported branches.
      
      Reported-by: Tatsuhito Kasahara
      Author: Tatsuhito Kasahara
      Reviewed-by: Masahiko Sawada, Fujii Masao
      Discussion: https://postgr.es/m/CAP0=ZVJsf=NvQuy+QXQZ7B=ZVLoDV_JzsVC1FRsF1G18i3zMGg@mail.gmail.com
      00749197
    • Michael Paquier's avatar
      Clean up some code, comments and docs referring to Windows 2000 and older · e2e02191
      Michael Paquier authored
      This fixes and updates a couple of comments related to outdated Windows
      versions.  Particularly, src/common/exec.c had a fallback implementation
      to read a file's line from a pipe because stdin/stdout/stderr does not
      exist in Windows 2000 that is removed to simplify src/common/ as there
      are unlikely versions of Postgres running on such platforms.
      
      Author: Michael Paquier
      Reviewed-by: Kyotaro Horiguchi, Juan José Santamaría Flecha
      Discussion: https://postgr.es/m/20191219021526.GC4202@paquier.xyz
      e2e02191
    • Amit Kapila's avatar
      Stop demanding that top xact must be seen before subxact in decoding. · e3ff789a
      Amit Kapila authored
      Manifested as
      
      ERROR:  subtransaction logged without previous top-level txn record
      
      this check forbids legit behaviours like
       - First xl_xact_assignment record is beyond reading, i.e. earlier
         restart_lsn.
       - After restart_lsn there is some change of a subxact.
       - After that, there is second xl_xact_assignment (for another subxact)
         revealing the relationship between top and first subxact.
      
      Such a transaction won't be streamed anyway because we hadn't seen it in
      full.  Saying for sure whether xact of some record encountered after
      the snapshot was deserialized can be streamed or not requires to know
      whether it wrote something before deserialization point --if yes, it
      hasn't been seen in full and can't be decoded. Snapshot doesn't have such
      info, so there is no easy way to relax the check.
      
      Reported-by: Hsu, John
      Diagnosed-by: Arseny Sher
      Author: Arseny Sher, Amit Kapila
      Reviewed-by: Amit Kapila, Dilip Kumar
      Backpatch-through: 9.5
      Discussion: https://postgr.es/m/AB5978B2-1772-4FEE-A245-74C91704ECB0@amazon.com
      e3ff789a
    • Peter Geoghegan's avatar
      Remove obsolete _bt_compare() comment. · fe9b9285
      Peter Geoghegan authored
      btbuild() has nothing to say about how NULL values compare in nbtree.
      Besides, there are _bt_compare() header comments that describe how NULL
      values are handled.
      fe9b9285
  2. 18 Feb, 2020 3 commits
  3. 17 Feb, 2020 5 commits
  4. 16 Feb, 2020 1 commit
    • Tom Lane's avatar
      Try again to work around Windows' ERROR_SHARING_VIOLATION in pg_ctl. · e02ea141
      Tom Lane authored
      Commit 0da33c76 introduced an unfortunate regression in pg_ctl on
      Windows: if the log file specified with -l doesn't exist yet, and
      pg_ctl is running with Administrator privileges, then the log file
      might get created with permissions that prevent the postmaster from
      writing on it.  (It seems that whether this happens depends on whether
      the log file is inside the user's home directory or not, and perhaps
      on other phase-of-the-moon conditions, which may explain why we failed
      to notice it sooner.)
      
      To fix, just don't create the log file if it doesn't exist yet.  The
      case where we need to wait obviously only occurs with a pre-existing
      log file.
      
      In passing, switch from using fopen() to plain open(), saving a few
      cycles.
      
      Per bug #16259 from Jonathan Katz and Heath Lord.  Back-patch to v12,
      as the faulty commit was.
      
      Alexander Lakhin
      
      Discussion: https://postgr.es/m/16259-c5ebed32a262a8b1@postgresql.org
      e02ea141
  5. 15 Feb, 2020 5 commits
    • Tom Lane's avatar
      Update obsolete comment. · faade5d4
      Tom Lane authored
      Noted by Justin Pryzby, though I chose to just rip out the stale text,
      as it's in no way relevant to this particular function.
      
      Discussion: https://postgr.es/m/20200212182337.GZ1412@telsasoft.com
      faade5d4
    • Tom Lane's avatar
      Clarify coding in Catalog::AddDefaultValues. · 9d1ec5a8
      Tom Lane authored
      Make it a bit shorter and better-commented; no functional change.
      
      Alvaro Herrera and Tom Lane
      
      Discussion: https://postgr.es/m/20200212182337.GZ1412@telsasoft.com
      9d1ec5a8
    • Tom Lane's avatar
      Run "make reformat-dat-files". · b78542b9
      Tom Lane authored
      Mostly to make sure the previous commit didn't break this.
      
      Discussion: https://postgr.es/m/20200212182337.GZ1412@telsasoft.com
      b78542b9
    • Tom Lane's avatar
      Don't require pg_class.dat to contain correct relnatts values. · 86ff085e
      Tom Lane authored
      Practically everybody who's ever added a column to one of the bootstrap
      catalogs has been burnt by the need to update the relnatts field in the
      initial pg_class data to match.  Now that we use Perl scripts to
      generate postgres.bki, we can have the machines take care of that,
      by filling the field during genbki.pl.
      
      While at it, use the BKI_DEFAULTS mechanism to eliminate repetitive
      specifications of other column values in pg_class.dat, too.  They
      weren't particularly a maintenance problem, but this way is prettier
      (certainly the spotty previous usage of BKI_DEFAULTS wasn't pretty).
      
      No catversion bump needed, since this doesn't actually change the
      contents of postgres.bki.
      
      Per gripe from Justin Pryzby, though this is quite different from
      his originally proposed solution.
      
      Amit Langote, John Naylor, Tom Lane
      
      Discussion: https://postgr.es/m/20200212182337.GZ1412@telsasoft.com
      86ff085e
    • Peter Geoghegan's avatar
      Recreate website's formatting for "website" doc builds. · 317906f2
      Peter Geoghegan authored
      The stylesheets used for the HTML documentation rendered on
      postgresql.org have shifted, and no longer matched what was expected by
      "make STYLE=website html" builds performed locally.  Local doc builds
      did not reflect other aspects of the website, including font and
      margins.
      
      This patch updates the references to use the current set of stylesheets
      that are used by the documentation on postgresql.org. This also wraps
      the documentation preview in a HTML container so it can keep the content
      within similar margins to those found on the website.
      
      The documentation on building the docs is updated to reflect this
      change, and to let the documentation builder know that an external
      network connection is required to properly preview documentation built
      with "make STYLE=website html" (which was true prior to this patch too,
      but not mentioned).
      
      Author: Jonathan Katz
      Reported-By: Tom Lane
      Discussion: https://postgr.es/m/1375.1581446233@sss.pgh.pa.us
      317906f2
  6. 14 Feb, 2020 2 commits
  7. 13 Feb, 2020 3 commits
    • Tom Lane's avatar
      Mark some contrib modules as "trusted". · eb67623c
      Tom Lane authored
      This allows these modules to be installed into a database without
      superuser privileges (assuming that the DBA or sysadmin has installed
      the module's files in the expected place).  You only need CREATE
      privilege on the current database, which by default would be
      available to the database owner.
      
      The following modules are marked trusted:
      
      btree_gin
      btree_gist
      citext
      cube
      dict_int
      earthdistance
      fuzzystrmatch
      hstore
      hstore_plperl
      intarray
      isn
      jsonb_plperl
      lo
      ltree
      pg_trgm
      pgcrypto
      seg
      tablefunc
      tcn
      tsm_system_rows
      tsm_system_time
      unaccent
      uuid-ossp
      
      In the future we might mark some more modules trusted, but there
      seems to be no debate about these, and on the whole it seems wise
      to be conservative with use of this feature to start out with.
      
      Discussion: https://postgr.es/m/32315.1580326876@sss.pgh.pa.us
      eb67623c
    • Jeff Davis's avatar
      Logical Tape Set: lazily allocate read buffer. · 7fdd919a
      Jeff Davis authored
      The write buffer was already lazily-allocated, so this is more
      symmetric. It also means that a freshly-rewound tape (whether for
      reading or writing) is not consuming memory for the buffer.
      
      Discussion: https://postgr.es/m/97c46a59c27f3c38e486ca170fcbc618d97ab049.camel%40j-davis.com
      7fdd919a
    • Tom Lane's avatar
      Avoid a performance regression in float overflow/underflow detection. · 607f8ce7
      Tom Lane authored
      Commit 6bf0bc84 replaced float.c's CHECKFLOATVAL() macro with static
      inline subroutines, but that wasn't too well thought out.  In the original
      coding, the unlikely condition (isinf(result) or result == 0) was checked
      first, and the inf_is_valid or zero_is_valid condition only afterwards.
      The inline-subroutine coding caused that to be swapped around, which is
      pretty horrid for performance because (a) in common cases the is_valid
      condition is twice as expensive to evaluate (e.g., requiring two isinf()
      calls not one) and (b) in common cases the is_valid condition is false,
      requiring us to perform the unlikely-condition check anyway.  Net result
      is that one isinf() call becomes two or three, resulting in visible
      performance loss as reported by Keisuke Kuroda.
      
      The original fix proposal was to revert the replacement of the macro,
      but on second thought, that macro was just a bad idea from the beginning:
      if anything it's a net negative for readability of the code.  So instead,
      let's just open-code all the overflow/underflow tests, being careful to
      test the unlikely condition first (and mark it unlikely() to help the
      compiler get the point).
      
      Also, rather than having N copies of the actual ereport() calls, collapse
      those into out-of-line error subroutines to save some code space.  This
      does mean that the error file/line numbers won't be very helpful for
      figuring out where the issue really is --- but we'd already burned that
      bridge by putting the ereports into static inlines.
      
      In HEAD, check_float[48]_val() are gone altogether.  In v12, leave them
      present in float.h but unused in the core code, just in case some
      extension is depending on them.
      
      Emre Hasegeli, with some kibitzing from me and Andres Freund
      
      Discussion: https://postgr.es/m/CANDwggLe1Gc1OrRqvPfGE=kM9K0FSfia0hbeFCEmwabhLz95AA@mail.gmail.com
      607f8ce7
  8. 12 Feb, 2020 7 commits
    • Peter Geoghegan's avatar
      Doc: Restructure B-Tree support routine docs. · caba0910
      Peter Geoghegan authored
      Use a top-level "variablelist", with one item per B-Tree support
      function.  This structure matches the structure used by various
      "Extensibility" sections in other documentation chapters for other index
      access methods.
      
      An explicit list makes it much clearer where each item begins and ends.
      This wasn't really a problem before now, but an upcoming patch that adds
      deduplication to nbtree will need to have its own new B-Tree support
      function.  Ease the burden of translators by tidying up btree.sgml ahead
      of committing the deduplication patch.
      caba0910
    • Tom Lane's avatar
      Remove long-dead comments. · 0973f560
      Tom Lane authored
      These should've been dropped by a8bb8eb5, but evidently were
      missed.  Not important enough to back-patch.
      0973f560
    • Tom Lane's avatar
      Doc: fix old oversights in GRANT/REVOKE documentation. · dce98814
      Tom Lane authored
      The GRANTED BY clause in GRANT/REVOKE ROLE has been there since 2005
      but was never documented.  I'm not sure now whether that was just an
      oversight or was intentional (given the limited capability of the
      option).  But seeing that pg_dumpall does emit code that uses this
      option, it seems like not documenting it at all is a bad idea.
      
      Also, when we upgraded the syntax to allow CURRENT_USER/SESSION_USER
      as the privilege recipient, the role form of GRANT was incorrectly
      not modified to show that, and REVOKE's docs weren't touched at all.
      
      Although I'm not that excited about GRANTED BY, the other oversight
      seems serious enough to justify a back-patch.
      
      Discussion: https://postgr.es/m/3070.1581526786@sss.pgh.pa.us
      dce98814
    • Andres Freund's avatar
      Try to harden insert-conflict-specconflict against autovacuum. · 997563df
      Andres Freund authored
      Looks like guaibasaurus had a autovacuum running during the
      controller_print_speculative_locks step (just added in
      43e08419). Which does indeed seem quite possible.
      
      Avoid the problem by only looking for the backends participating in
      the test.
      997563df
    • Michael Paquier's avatar
      Add %x to default PROMPT1 and PROMPT2 in psql · dcdbb5a5
      Michael Paquier authored
      %d can be used to track if the current connection is in a transaction
      block or not, and adding it by default to the prompt has the advantage
      to not need a modification of .psqlrc, something not possible depending
      on the environment.
      
      This discussion has happened across various sources, and there was a
      strong consensus in favor of this change.
      
      Author: Vik Fearing
      Reviewed-by: Fabien Coelho
      Discussion: https://postgr.es/m/09502c40-cfe1-bb29-10f9-4b3fa7b2bbb2@2ndquadrant.com
      dcdbb5a5
    • Andres Freund's avatar
      Test additional speculative conflict scenarios. · 43e08419
      Andres Freund authored
      Previously, the speculative insert tests did not cover the case when a
      tuple t is inserted into a table with a unique index on a column but
      before it can insert into the index, a concurrent transaction has
      inserted a conflicting value into the index and the insertion of tuple t
      must be aborted.
      
      The basic permutation is one session successfully inserts into the table
      and an associated unique index while a concurrent session successfully
      inserts into the table but discovers a conflict before inserting into
      the index and must abort the insertion.
      
      Several variants on this include:
      - swap which session is successful
      - first session insert transaction does not commit, so second session
      must wait on a transaction lock
      - first session insert does not "complete", so second session must wait
      on a speculative insertion lock
      
      Also, refactor the existing TOAST table upsert test to be in the same
      spec and reuse the steps.
      
      Author: Melanie Plageman, Ashwin Agrawal, Andres Freund
      Reviewed-by: Andres Freund, Taylor Vesely
      Discussion: https://postgr.es/m/CAAKRu_ZRmxy_OEryfY3G8Zp01ouhgw59_-_Cm8n7LzRH5BAvng@mail.gmail.com
      43e08419
    • Fujii Masao's avatar
      Fix bug in pg_basebackup -F plain -R. · be6221e9
      Fujii Masao authored
      Commit caba97a9 changed pg_basebackup -F plain -R so that
      it overwrote postgresql.auto.conf in the backup, with new connection
      setting. This could cause the existing postgresql.auto.conf settings
      in the server to get lost unexpectedly. This is a bug.
      
      This commit fixes the bug by making pg_basebackup -F plain -R
      append the connection setting into postgresql.auto.conf in the backup.
      
      Author: Fujii Masao
      Reviewed-by: Sergei Kornilov
      Discussion: https://postgr.es/m/250dcf2a-94e7-c05e-824a-73cfb38a48a4@oss.nttdata.com
      be6221e9
  9. 11 Feb, 2020 2 commits
  10. 10 Feb, 2020 3 commits