1. 10 Sep, 2019 4 commits
    • Tomas Vondra's avatar
      Allow setting statistics target for extended statistics · d06215d0
      Tomas Vondra authored
      When building statistics, we need to decide how many rows to sample and
      how accurate the resulting statistics should be. Until now, it was not
      possible to explicitly define statistics target for extended statistics
      objects, the value was always computed from the per-attribute targets
      with a fallback to the system-wide default statistics target.
      
      That's a bit inconvenient, as it ties together the statistics target set
      for per-column and extended statistics. In some cases it may be useful
      to require larger sample / higher accuracy for extended statics (or the
      other way around), but with this approach that's not possible.
      
      So this commit introduces a new command, allowing to specify statistics
      target for individual extended statistics objects, overriding the value
      derived from per-attribute targets (and the system default).
      
        ALTER STATISTICS stat_name SET STATISTICS target_value;
      
      When determining statistics target for an extended statistics object we
      first look at this explicitly set value. When this value is -1, we fall
      back to the old formula, looking at the per-attribute targets first and
      then the system default. This means the behavior is backwards compatible
      with older PostgreSQL releases.
      
      Author: Tomas Vondra
      Discussion: https://postgr.es/m/20190618213357.vli3i23vpkset2xd@development
      Reviewed-by: Kirk Jamison, Dean Rasheed
      d06215d0
    • Tom Lane's avatar
      Reduce overhead of scanning the backend[] array in LISTEN/NOTIFY. · bca6e643
      Tom Lane authored
      Up to now, async.c scanned its whole array of per-backend state
      whenever it needed to find listening backends.  That's expensive
      if MaxBackends is large, so extend the data structure with list
      links that thread the active entries together.
      
      A downside of this change is that asyncQueueUnregister (unregister
      a listening backend at backend exit) now requires exclusive not shared
      lock, and it can take awhile if there are many other listening
      backends.  We could improve the latter issue by using a doubly- not
      singly-linked list, but it's probably not worth the storage space;
      typical usage patterns for LISTEN/NOTIFY have fairly long-lived
      listeners.
      
      In return for that, Exec_ListenPreCommit (initially register a
      listening backend), SignalBackends, and asyncQueueAdvanceTail
      get significantly faster when MaxBackends is much larger than
      the number of listening backends.  If most of the potential
      backend slots are listening, we don't win, but that's a case
      where the actual interprocess-signal overhead is going to swamp
      these considerations anyway.
      
      Martijn van Oosterhout, hacked a bit more by me
      
      Discussion: https://postgr.es/m/CADWG95vtRBFDdrx1JdT1_9nhOFw48KaeTev6F_LtDQAFVpSPhA@mail.gmail.com
      bca6e643
    • Alvaro Herrera's avatar
      Fix unaccent generation script in Windows · 0afc0a78
      Alvaro Herrera authored
      As originally coded, the script would fail on Windows 10 and Python 3
      because stdout would not be switched to UTF-8 only for Python 2.  This
      patch makes that apply to both versions.
      
      Also add python 2 compatibility markers so that we know what to remove
      once we drop support for that.  Also use a "with" clause to ensure file
      descriptor is closed promptly.
      
      Author: Hugh Ranalli, Ramanarayana
      Reviewed-by: Kyotaro Horiguchi
      Discussion: https://postgr.es/m/CAKm4Xs7_61XMyOWmHs3n0mmkS0O4S0pvfWk=7cQ5P0gs177f7A@mail.gmail.com
      Discussion: https://postgr.es/m/15548-cef1b3f8de190d4f@postgresql.org
      0afc0a78
    • Alvaro Herrera's avatar
      Restructure libpq code to remove some duplicity · b438e7e7
      Alvaro Herrera authored
      There was some duplicate code to run SHOW transaction_read_only to
      determine whether the server is read-write or read-only.  Reduce it by
      adding another state to the state machine.
      
      Author: Hari Babu Kommi
      Reviewed-by: Takayuki Tsunakawa, Álvaro Herrera
      Discussion: https://postgr.es/m/CAJrrPGe_qgdbbN+yBgEVpd+YLHXXjTruzk6RmTMhqrFig+32ag@mail.gmail.com
      b438e7e7
  2. 09 Sep, 2019 6 commits
    • Peter Geoghegan's avatar
      Add _bt_binsrch() scantid assertion to nbtree. · 55d015bd
      Peter Geoghegan authored
      Assert that _bt_binsrch() binary searches with scantid set in insertion
      scankey cannot be performed on leaf pages.  Leaf-level binary searches
      where scantid is set must use _bt_binsrch_insert() instead.
      
      _bt_binsrch_insert() is likely to have additional responsibilities in
      the future, such as searching within GIN-style posting lists using
      scantid.  It seems like a good idea to tighten things up now.
      55d015bd
    • Tom Lane's avatar
      Be more careful about port selection in src/test/ldap/. · 3146f525
      Tom Lane authored
      Don't just assume that the next port is free; it might not be, or
      if we're really unlucky it might even be out of the TCP range.
      Do it honestly with two get_free_port() calls instead.
      
      This is surely a pretty low-probability problem, but I think it
      explains a buildfarm failure seen today, so let's fix it.
      
      Back-patch to v11 where this script was added.
      
      Discussion: https://postgr.es/m/25124.1568052346@sss.pgh.pa.us
      3146f525
    • Andrew Dunstan's avatar
      Prevent msys2 conversion of "cmd /c" switch to a file path · 73ff3a0a
      Andrew Dunstan authored
      Modern versions of msys2 have changed the treatment of "cmd /c" so that
      the runtime will try to convert the switch to a native file path. This
      patch adds a setting to inhibit that behaviour.
      
      Discussion: https://postgr.es/m/3227042f-cfcc-745a-57dd-fb8c471f8ddf@2ndQuadrant.com
      
      Backpatch to all live branches.
      73ff3a0a
    • Andres Freund's avatar
      Reorder EPQ work, to fix rowmark related bugs and improve efficiency. · 27cc7cd2
      Andres Freund authored
      In ad0bda5d I changed the EvalPlanQual machinery to store
      substitution tuples in slot, instead of using plain HeapTuples. The
      main motivation for that was that using HeapTuples will be inefficient
      for future tableams.  But it turns out that that conversion was buggy
      for non-locking rowmarks - the wrong tuple descriptor was used to
      create the slot.
      
      As a secondary issue 5db6df0c changed ExecLockRows() to begin EPQ
      earlier, to allow to fetch the locked rows directly into the EPQ
      slots, instead of having to copy tuples around. Unfortunately, as Tom
      complained, that forces some expensive initialization to happen
      earlier.
      
      As a third issue, the test coverage for EPQ was clearly insufficient.
      
      Fixing the first issue is unfortunately not trivial: Non-locked row
      marks were fetched at the start of EPQ, and we don't have the type
      information for the rowmarks available at that point. While we could
      change that, it's not easy. It might be worthwhile to change that at
      some point, but to fix this bug, it seems better to delay fetching
      non-locking rowmarks when they're actually needed, rather than
      eagerly. They're referenced at most once, and in cases where EPQ
      fails, might never be referenced. Fetching them when needed also
      increases locality a bit.
      
      To be able to fetch rowmarks during execution, rather than
      initialization, we need to be able to access the active EPQState, as
      that contains necessary data. To do so move EPQ related data from
      EState to EPQState, and, only for EStates creates as part of EPQ,
      reference the associated EPQState from EState.
      
      To fix the second issue, change EPQ initialization to allow use of
      EvalPlanQualSlot() to be used before EvalPlanQualBegin() (but
      obviously still requiring EvalPlanQualInit() to have been done).
      
      As these changes made struct EState harder to understand, e.g. by
      adding multiple EStates, significantly reorder the members, and add a
      lot more comments.
      
      Also add a few more EPQ tests, including one that fails for the first
      issue above. More is needed.
      
      Reported-By: yi huang
      Author: Andres Freund
      Reviewed-By: Tom Lane
      Discussion:
          https://postgr.es/m/CAHU7rYZo_C4ULsAx_LAj8az9zqgrD8WDd4hTegDTMM1LMqrBsg@mail.gmail.com
          https://postgr.es/m/24530.1562686693@sss.pgh.pa.us
      Backpatch: 12-, where the EPQ changes were introduced
      27cc7cd2
    • Alexander Korotkov's avatar
      Fix handling of non-key columns get_index_column_opclass() · 7e041603
      Alexander Korotkov authored
      f2e40380 introduces support of non-key attributes in GiST indexes.  Then if
      get_index_column_opclass() is asked by gistproperty() to get an opclass of
      non-key column, it returns garbage past oidvector value.  This commit fixes
      that by making get_index_column_opclass() return InvalidOid in this case.
      
      Discussion: https://postgr.es/m/20190902231948.GA5343%40alvherre.pgsql
      Author: Nikita Glukhov, Alexander Korotkov
      Backpatch-through: 12
      7e041603
    • Peter Eisentraut's avatar
      Improve new AND CHAIN tests · 89b160c3
      Peter Eisentraut authored
      Tweak the tests so that we're not just testing the default setting of
      transaction_read_only.
      Reported-by: default avatarfn ln <emuser20140816@gmail.com>
      89b160c3
  3. 08 Sep, 2019 6 commits
  4. 07 Sep, 2019 2 commits
    • Tom Lane's avatar
      Avoid using INFO elevel for what are fundamentally debug messages. · db438318
      Tom Lane authored
      Commit 6f6b99d1 stuck an INFO message into the fast path for
      checking partition constraints, for no very good reason except
      that it made it easy for the regression tests to verify that
      that path was taken.  Assorted later patches did likewise,
      increasing the unsuppressable-chatter level from ALTER TABLE
      even more.  This isn't good for the user experience, so let's
      drop these messages down to DEBUG1 where they belong.  So as
      not to have a loss of test coverage, create a TAP test that
      runs the relevant queries with client_min_messages = DEBUG1
      and greps for the expected messages.
      
      This testing method is a bit brute-force --- in particular,
      it duplicates the execution of a fair amount of the core
      create_table and alter_table tests.  We experimented with
      other solutions, but running any significant amount of
      standard testing with client_min_messages = DEBUG1 seems
      to have a lot of output-stability pitfalls, cf commits
      bbb96c37 and 5655565c.  Possibly at some point we'll look
      into whether we can reduce the amount of test duplication.
      
      Backpatch into v12, because some of these messages are new
      in v12 and we don't really want to ship it that way.
      
      Sergei Kornilov
      
      Discussion: https://postgr.es/m/81911511895540@web58j.yandex.ru
      Discussion: https://postgr.es/m/4859321552643736@myt5-02b80404fd9e.qloud-c.yandex.net
      db438318
    • Tom Lane's avatar
      Fix issues around strictness of SIMILAR TO. · ca70bdae
      Tom Lane authored
      As a result of some long-ago quick hacks, the SIMILAR TO operator
      and the corresponding flavor of substring() interpreted "ESCAPE NULL"
      as selecting the default escape character '\'.  This is both
      surprising and not per spec: the standard is clear that these
      functions should return NULL for NULL input.
      
      Additionally, because of inconsistency of the strictness markings
      of 3-argument substring() and similar_escape(), the planner could not
      inline the SQL definition of substring(), resulting in a substantial
      performance penalty compared to the underlying POSIX substring()
      function.
      
      The simplest fix for this would be to change the strictness marking
      of similar_escape(), but if we do that we risk breaking existing views
      that depend on that function.  Hence, leave similar_escape() as-is
      as a compatibility function, and instead invent a new function
      similar_to_escape() that comes in two strict variants.
      
      There are a couple of other behaviors in this area that are also
      not per spec, but they are documented and seem generally at least
      as sane as the spec's definition, so leave them alone.  But improve
      the documentation to describe them fully.
      
      Patch by me; thanks to Álvaro Herrera and Andrew Gierth for review
      and discussion.
      
      Discussion: https://postgr.es/m/14047.1557708214@sss.pgh.pa.us
      ca70bdae
  5. 06 Sep, 2019 10 commits
  6. 05 Sep, 2019 3 commits
    • Tom Lane's avatar
      Use data directory inode number, not port, to select SysV resource keys. · 7de19fbc
      Tom Lane authored
      This approach provides a much tighter binding between a data directory
      and the associated SysV shared memory block (and SysV or named-POSIX
      semaphores, if we're using those).  Key collisions are still possible,
      but only between data directories stored on different filesystems,
      so the situation should be negligible in practice.  More importantly,
      restarting the postmaster with a different port number no longer
      risks failing to identify a relevant shared memory block, even when
      postmaster.pid has been removed.  A standalone backend is likewise
      much more certain to detect conflicting leftover backends.
      
      (In the longer term, we might now think about deprecating the port as
      a cluster-wide value, so that one postmaster could support sockets
      with varying port numbers.  But that's for another day.)
      
      The hazards fixed here apply only on Unix systems; our Windows code
      paths already use identifiers derived from the data directory path
      name rather than the port.
      
      src/test/recovery/t/017_shm.pl, which intends to test key-collision
      cases, has been substantially rewritten since it can no longer use
      two postmasters with identical port numbers to trigger the case.
      Instead, use Perl's IPC::SharedMem module to create a conflicting
      shmem segment directly.  The test script will be skipped if that
      module is not available.  (This means that some older buildfarm
      members won't run it, but I don't think that that results in any
      meaningful coverage loss.)
      
      Patch by me; thanks to Noah Misch and Peter Eisentraut for discussion
      and review.
      
      Discussion: https://postgr.es/m/16908.1557521200@sss.pgh.pa.us
      7de19fbc
    • Robert Haas's avatar
      Split tuptoaster.c into three separate files. · 8b94dab0
      Robert Haas authored
      detoast.c/h contain functions required to detoast a datum, partially
      or completely, plus a few other utility functions for examining the
      size of toasted datums.
      
      toast_internals.c/h contain functions that are used internally to the
      TOAST subsystem but which (mostly) do not need to be accessed from
      outside.
      
      heaptoast.c/h contains code that is intrinsically specific to the
      heap AM, either because it operates on HeapTuples or is based on the
      layout of a heap page.
      
      detoast.c and toast_internals.c are placed in
      src/backend/access/common rather than src/backend/access/heap.  At
      present, both files still have dependencies on the heap, but that will
      be improved in a future commit.
      
      Patch by me, reviewed and tested by Prabhat Sabu, Thomas Munro,
      Andres Freund, and Álvaro Herrera.
      
      Discussion: http://postgr.es/m/CA+TgmoZv-=2iWM4jcw5ZhJeL18HF96+W1yJeYrnGMYdkFFnEpQ@mail.gmail.com
      8b94dab0
    • Peter Eisentraut's avatar
      Use explicit_bzero · 74a308cf
      Peter Eisentraut authored
      Use the explicit_bzero() function in places where it is important that
      security information such as passwords is cleared from memory.  There
      might be other places where it could be useful; this is just an
      initial collection.
      
      For platforms that don't have explicit_bzero(), provide various
      fallback implementations.  (explicit_bzero() itself isn't standard,
      but as Linux/glibc, FreeBSD, and OpenBSD have it, it's the most common
      spelling, so it makes sense to make that the invocation point.)
      
      Discussion: https://www.postgresql.org/message-id/flat/42d26bde-5d5b-c90d-87ae-6cab875f73be%402ndquadrant.com
      74a308cf
  7. 04 Sep, 2019 2 commits
    • Michael Paquier's avatar
      Fix thinko when ending progress report for a backend · ae060a52
      Michael Paquier authored
      The logic ending progress reporting for a backend entry introduced by
      b6fb6471 causes callers of pgstat_progress_end_command() to do some extra
      work when track_activities is enabled as the process fields are reset in
      the backend entry even if no command were started for reporting.
      
      This resets the fields only if a command is registered for progress
      reporting, and only if track_activities is enabled.
      
      Author: Masahiho Sawada
      Discussion: https://postgr.es/m/CAD21AoCry_vJ0E-m5oxJXGL3pnos-xYGCzF95rK5Bbi3Uf-rpA@mail.gmail.com
      Backpatch-through: 9.6
      ae060a52
    • Michael Paquier's avatar
      Delay fsyncs of pg_basebackup until the end of backup · 522baf14
      Michael Paquier authored
      Since the addition of fsync requests in bc34223b to make base backup data
      consistent on disk once pg_basebackup finishes, each tablespace tar file
      is individually flushed once completed, with an additional flush of the
      parent directory when the base backup finishes.  While holding a
      connection to the server, a fsync request taking a long time may cause a
      failure of the base backup, which is annoying for any integration.  A
      recent example of breakage can involve tcp_user_timeout, but
      wal_sender_timeout can cause similar problems.
      
      While reviewing the code, there was a second issue causing too many
      fsync requests to be done for the same WAL data.  As recursive fsyncs
      are done at the end of the backup for both the plain and tar formats
      from the base target directory where everything is written, it is fine
      to disable fsyncs when fetching or streaming WAL.
      
      Reported-by: Ryohei Takahashi
      Author: Michael Paquier
      Reviewed-by: Ryohei Takahashi
      Discussion: https://postgr.es/m/OSBPR01MB4550DAE2F8C9502894A45AAB82BE0@OSBPR01MB4550.jpnprd01.prod.outlook.com
      Backpatch-through: 10
      522baf14
  8. 03 Sep, 2019 7 commits