1. 21 Feb, 2020 7 commits
    • Tom Lane's avatar
      Assume that we have isinf(). · 7fde892b
      Tom Lane authored
      Windows has this, and so do all other live platforms according to the
      buildfarm, so remove the configure probe and src/port/ substitution.
      
      This also lets us get rid of some configure probes that existed only
      to support src/port/isinf.c.  I kept the port.h hack to force using
      __builtin_isinf() on clang, though.
      
      This is part of a series of commits to get rid of no-longer-relevant
      configure checks and dead src/port/ code.  I'm committing them separately
      to make it easier to back out individual changes if they prove less
      portable than I expect.
      
      Discussion: https://postgr.es/m/15379.1582221614@sss.pgh.pa.us
      7fde892b
    • Tom Lane's avatar
      Assume that we have functional, 64-bit fseeko()/ftello(). · 799d2246
      Tom Lane authored
      Windows has this, and so do all other live platforms according to the
      buildfarm, so remove the configure probe and src/port/ substitution.
      
      Keep the probe that detects whether _LARGEFILE_SOURCE has to be
      defined to get that, though ... that seems to be still relevant in
      some places.
      
      This is part of a series of commits to get rid of no-longer-relevant
      configure checks and dead src/port/ code.  I'm committing them separately
      to make it easier to back out individual changes if they prove less
      portable than I expect.
      
      Discussion: https://postgr.es/m/15379.1582221614@sss.pgh.pa.us
      799d2246
    • Peter Eisentraut's avatar
      Fix compiler warnings on 64-bit Windows · 3f9c1697
      Peter Eisentraut authored
      GCC reports various instances of
      
      warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
      warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
      
      and MSVC equivalently
      
      warning C4312: 'type cast': conversion from 'int' to 'void *' of greater size
      warning C4311: 'type cast': pointer truncation from 'void *' to 'long'
      
      in ECPG test files.  This is because void* and long are cast back and
      forth, but on 64-bit Windows, these have different sizes.  Fix by
      using intptr_t instead.
      
      The code actually worked fine because the integer values in use are
      all small.  So this is just to get the test code to compile warning-free.
      
      This change is simplified by having made stdint.h required (commit
      95733841).  Before this it would have
      been more complicated because the ecpg test source files don't use the
      full pg_config.h.
      Reviewed-by: default avatarTom Lane <tgl@sss.pgh.pa.us>
      Discussion: https://www.postgresql.org/message-id/flat/5d398bbb-262a-5fed-d839-d0e5cff3c0d7%402ndquadrant.com
      3f9c1697
    • Jeff Davis's avatar
      Fixup for nodeAgg.c refactor. · b7fabe80
      Jeff Davis authored
      Commit 5b618e1f made an unintended behavior change.
      b7fabe80
    • Etsuro Fujita's avatar
      Avoid redundant checks in partition_bounds_copy(). · 032f9ae0
      Etsuro Fujita authored
      Previously, partition_bounds_copy() checked whether the strategy for the
      given partition bounds was hash or not, and then determined the number of
      elements in the datums in the datums array for the partition bounds, on
      each iteration of the loop for copying the datums array, but there is no
      need to do that.  Perform the checks only once before the loop iteration.
      
      Author: Etsuro Fujita
      Reported-by: Amit Langote and Julien Rouhaud
      Discussion: https://postgr.es/m/CAPmGK14Rvxrm8DHWvCjdoks6nwZuHBPvMnWZ6rkEx2KhFeEoPQ@mail.gmail.com
      032f9ae0
    • Peter Eisentraut's avatar
      Require stdint.h · 95733841
      Peter Eisentraut authored
      stdint.h belongs to the compiler (as opposed to inttypes.h), so by
      requiring a C99 compiler we can also require stdint.h
      unconditionally.  Remove configure checks and other workarounds for
      it.
      
      This also removes a few steps in the required portability adjustments
      to the imported time zone code, which can be applied on the next
      import.
      
      When using GCC on a platform that is otherwise pre-C99, this will now
      require at least GCC 4.5, which is the first release that supplied a
      standard-conforming stdint.h if the native platform didn't have it.
      Reviewed-by: default avatarTom Lane <tgl@sss.pgh.pa.us>
      Discussion: https://www.postgresql.org/message-id/flat/5d398bbb-262a-5fed-d839-d0e5cff3c0d7%402ndquadrant.com
      95733841
    • Michael Paquier's avatar
      Doc: Fix instructions to control build environment with MSVC · dca3911a
      Michael Paquier authored
      The documentation included some outdated instructions to change the
      architecture, build type or target OS of a build done with MSVC.  This
      commit updates the documentation to include the modern options
      available, down to Visual Studio 2013.
      
      Reported-by: Kyotaro Horiguchi
      Author: Juan José Santamaría Flecha
      Discussion: https://postgr.es/m/CAC+AXB0J7tAqW_2F1fCE4Dh2=Ccz96TcLpsGXOCvka7VvWG9Qw@mail.gmail.com
      Backpatch-through: 12
      dca3911a
  2. 20 Feb, 2020 3 commits
  3. 19 Feb, 2020 11 commits
    • Tom Lane's avatar
      Doc: discourage use of partial indexes for poor-man's-partitioning. · 6a8e5605
      Tom Lane authored
      Creating a bunch of non-overlapping partial indexes is generally
      a bad idea, so add an example saying not to do that.
      
      Back-patch to v10.  Before that, the alternative of using (real)
      partitioning wasn't available, so that the tradeoff isn't quite
      so clear cut.
      
      Discussion: https://postgr.es/m/CAKVFrvFY-f7kgwMRMiPLbPYMmgjc8Y2jjUGK_Y0HVcYAmU6ymg@mail.gmail.com
      6a8e5605
    • Tom Lane's avatar
      Remove support for upgrading extensions from "unpackaged" state. · 70a77320
      Tom Lane authored
      Andres Freund pointed out that allowing non-superusers to run
      "CREATE EXTENSION ... FROM unpackaged" has security risks, since
      the unpackaged-to-1.0 scripts don't try to verify that the existing
      objects they're modifying are what they expect.  Just attaching such
      objects to an extension doesn't seem too dangerous, but some of them
      do more than that.
      
      We could have resolved this, perhaps, by still requiring superuser
      privilege to use the FROM option.  However, it's fair to ask just what
      we're accomplishing by continuing to lug the unpackaged-to-1.0 scripts
      forward.  None of them have received any real testing since 9.1 days,
      so they may not even work anymore (even assuming that one could still
      load the previous "loose" object definitions into a v13 database).
      And an installation that's trying to go from pre-9.1 to v13 or later
      in one jump is going to have worse compatibility problems than whether
      there's a trivial way to convert their contrib modules into extension
      style.
      
      Hence, let's just drop both those scripts and the core-code support
      for "CREATE EXTENSION ... FROM".
      
      Discussion: https://postgr.es/m/20200213233015.r6rnubcvl4egdh5r@alap3.anarazel.de
      70a77320
    • 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
  4. 18 Feb, 2020 3 commits
  5. 17 Feb, 2020 5 commits
  6. 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
  7. 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
  8. 14 Feb, 2020 2 commits
  9. 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