1. 23 Mar, 2019 5 commits
    • Peter Geoghegan's avatar
      Suppress DETAIL output from an event_trigger test. · 05f110cc
      Peter Geoghegan authored
      Suppress 3 lines of unstable DETAIL output from a DROP ROLE statement in
      event_trigger.sql.  This is further cleanup for commit dd299df8.
      
      Note that the event_trigger test instability issue is very similar to
      the recently suppressed foreign_data test instability issue.  Both
      issues involve DETAIL output for a DROP ROLE statement that needed to be
      changed as part of dd299df8.
      
      Per buildfarm member macaque.
      05f110cc
    • Peter Geoghegan's avatar
      Add nbtree high key "continuescan" optimization. · 29b64d1d
      Peter Geoghegan authored
      Teach nbtree forward index scans to check the high key before moving to
      the right sibling page in the hope of finding that it isn't actually
      necessary to do so.  The new check may indicate that the scan definitely
      cannot find matching tuples to the right, ending the scan immediately.
      We already opportunistically force a similar "continuescan orientated"
      key check of the final non-pivot tuple when it's clear that it cannot be
      returned to the scan due to being dead-to-all.  The new high key check
      is complementary.
      
      The new approach for forward scans is more effective than checking the
      final non-pivot tuple, especially with composite indexes and non-unique
      indexes.  The improvements to the logic for picking a split point added
      by commit fab25024 make it likely that relatively dissimilar high keys
      will appear on a page.  A distinguishing key value that can only appear
      on non-pivot tuples on the right sibling page will often be present in
      leaf page high keys.
      
      Since forcing the final item to be key checked no longer makes any
      difference in the case of forward scans, the existing extra key check is
      now only used for backwards scans.  Backward scans continue to
      opportunistically check the final non-pivot tuple, which is actually the
      first non-pivot tuple on the page (not the last).
      
      Note that even pg_upgrade'd v3 indexes make use of this optimization.
      
      Author: Peter Geoghegan, Heikki Linnakangas
      Reviewed-By: Heikki Linnakangas
      Discussion: https://postgr.es/m/CAH2-WzkOmUduME31QnuTFpimejuQoiZ-HOf0pOWeFZNhTMctvA@mail.gmail.com
      29b64d1d
    • Michael Paquier's avatar
      Improve format of code and some error messages in pg_checksums · 4ba96d1b
      Michael Paquier authored
      This makes the code more consistent with the surroundings.
      
      Author: Fabrízio de Royes Mello
      Discussion: https://postgr.es/m/CAFcNs+pXb_35r5feMU3-dWsWxXU=Yjq+spUsthFyGFbT0QcaKg@mail.gmail.com
      4ba96d1b
    • Tom Lane's avatar
      Add unreachable "break" to satisfy -Wimplicit-fallthrough. · fb50d3f0
      Tom Lane authored
      gcc is a bit pickier about this than perhaps it should be.
      
      Discussion: https://postgr.es/m/E1h6zzT-0003ft-DD@gemulon.postgresql.org
      fb50d3f0
    • Andres Freund's avatar
      Expand EPQ tests for UPDATEs and DELETEs · cdcffe22
      Andres Freund authored
      Previously there was basically no coverage for UPDATEs encountering
      deleted rows, and no coverage for DELETE having to perform EPQ. That's
      problematic for an upcoming commit in which EPQ is tought to integrate
      with tableams.  Also, there was no test for UPDATE to encounter a row
      UPDATEd into another partition.
      
      Author: Andres Freund
      cdcffe22
  2. 22 Mar, 2019 18 commits
  3. 21 Mar, 2019 5 commits
    • Peter Geoghegan's avatar
      Revert "Suppress DETAIL output from a foreign_data test." · fff518d0
      Peter Geoghegan authored
      This should be superseded by commit 8aa9dd74.
      fff518d0
    • Alvaro Herrera's avatar
    • Alvaro Herrera's avatar
      Fix dependency recording bug for partitioned PKs · 7e7c57bb
      Alvaro Herrera authored
      When DefineIndex recurses to create constraints on partitions, it needs
      to use the value returned by index_constraint_create to set up partition
      dependencies.  However, in the course of fixing the DEPENDENCY_INTERNAL_AUTO
      mess, commit 1d92a0c9 introduced some code to that function that
      clobbered the return value, causing the recorded OID to be of the wrong
      object.  Close examination of pg_depend after creating the tables leads
      to indescribable objects :-( My sin (in commit bdc3d7fa, while
      preparing for DDL deparsing in event triggers) was to use a variable
      name for the return value that's typically used for throwaway objects in
      dependency-setting calls ("referenced").  Fix by changing the variable
      names to match extended practice (the return value is "myself" rather
      than "referenced".)
      
      The pg_upgrade test notices the problem (in an indirect way: the pg_dump
      outputs are in different order), but only if you create the objects in a
      specific way that wasn't being used in the existing tests.  Add a stanza
      to leave some objects around that shows the bug.
      
      Catversion bump because preexisting databases might have bogus pg_depend
      entries.
      
      Discussion: https://postgr.es/m/20190318204235.GA30360@alvherre.pgsql
      7e7c57bb
    • Tom Lane's avatar
      Improve error reporting for DROP FUNCTION/PROCEDURE/AGGREGATE/ROUTINE. · bfb456c1
      Tom Lane authored
      These commands allow the argument type list to be omitted if there is
      just one object that matches by name.  However, if that syntax was
      used with DROP IF EXISTS and there was more than one match, you got
      a "function ... does not exist, skipping" notice message rather than a
      truthful complaint about the ambiguity.  This was basically due to
      poor factorization and a rats-nest of logic, so refactor the relevant
      lookup code to make it cleaner.
      
      Note that this amounts to narrowing the scope of which sorts of
      error conditions IF EXISTS will bypass.  Per discussion, we only
      intend it to skip no-such-object cases, not multiple-possible-matches
      cases.
      
      Per bug #15572 from Ash Marath.  Although this definitely seems like
      a bug, it's not clear that people would thank us for changing the
      behavior in minor releases, so no back-patch.
      
      David Rowley, reviewed by Julien Rouhaud and Pavel Stehule
      
      Discussion: https://postgr.es/m/15572-ed1b9ed09503de8a@postgresql.org
      bfb456c1
    • Thomas Munro's avatar
      Add DNS SRV support for LDAP server discovery. · 0f086f84
      Thomas Munro authored
      LDAP servers can be advertised on a network with RFC 2782 DNS SRV
      records.  The OpenLDAP command-line tools automatically try to find
      servers that way, if no server name is provided by the user.  Teach
      PostgreSQL to do the same using OpenLDAP's support functions, when
      building with OpenLDAP.
      
      For now, we assume that HAVE_LDAP_INITIALIZE (an OpenLDAP extension
      available since OpenLDAP 2.0 and also present in Apple LDAP) implies
      that you also have ldap_domain2hostlist() (which arrived in the same
      OpenLDAP version and is also present in Apple LDAP).
      
      Author: Thomas Munro
      Reviewed-by: Daniel Gustafsson
      Discussion: https://postgr.es/m/CAEepm=2hAnSfhdsd6vXsM6VZVN0br-FbAZ-O+Swk18S5HkCP=A@mail.gmail.com
      0f086f84
  4. 20 Mar, 2019 11 commits
    • Tom Lane's avatar
      Sort the dependent objects before deletion in DROP OWNED BY. · 8aa9dd74
      Tom Lane authored
      This finishes a task we left undone in commit f1ad067f, by extending
      the delete-in-descending-OID-order rule to deletions triggered by
      DROP OWNED BY.  We've coped with machine-dependent deletion orders
      one time too many, and the new issues caused by Peter G's recent
      nbtree hacking seem like the last straw.
      
      Discussion: https://postgr.es/m/E1h6eep-0001Mw-Vd@gemulon.postgresql.org
      8aa9dd74
    • Alvaro Herrera's avatar
      Add index_get_partition convenience function · a6da0047
      Alvaro Herrera authored
      This new function simplifies some existing coding, as well as supports
      future patches.
      
      Discussion: https://postgr.es/m/201901222145.t6wws6t6vrcu@alvherre.pgsql
      Reviewed-by: Amit Langote, Jesper Pedersen
      a6da0047
    • Peter Geoghegan's avatar
      Fix spurious compiler warning in nbtxlog.c. · 3d0dcc5c
      Peter Geoghegan authored
      Cleanup from commit dd299df8.
      
      Per complaint from Tom Lane.
      3d0dcc5c
    • Peter Geoghegan's avatar
      Suppress DETAIL output from a foreign_data test. · 7d3bf73a
      Peter Geoghegan authored
      Unstable sort order related to changes to nbtree from commit dd299df8
      can cause two lines of DETAIL output to be in opposite-of-expected
      order.  Suppress the output using the same VERBOSITY hack that is used
      elsewhere in the foreign_data tests.
      
      Note that the same foreign_data.out DETAIL output was mechanically
      updated by commit dd299df8.  Only a few such changes were required,
      though.
      
      Per buildfarm member batfish.
      
      Discussion: https://postgr.es/m/CAH2-WzkCQ_MtKeOpzozj7QhhgP1unXsK8o9DMAFvDqQFEPpkYQ@mail.gmail.com
      7d3bf73a
    • Alvaro Herrera's avatar
      Restore RI trigger sanity check · 815b20ae
      Alvaro Herrera authored
      I unnecessarily removed this check in 3de241db because I
      misunderstood what the final representation of constraints across a
      partitioning hierarchy was to be.  Put it back (in both branches).
      
      Discussion: https://postgr.es/m/201901222145.t6wws6t6vrcu@alvherre.pgsql
      815b20ae
    • Peter Geoghegan's avatar
      Allow amcheck to re-find tuples using new search. · c1afd175
      Peter Geoghegan authored
      Teach contrib/amcheck's bt_index_parent_check() function to take
      advantage of the uniqueness property of heapkeyspace indexes in support
      of a new verification option: non-pivot tuples (non-highkey tuples on
      the leaf level) can optionally be re-found using a new search for each,
      that starts from the root page.  If a tuple cannot be re-found, report
      that the index is corrupt.
      
      The new "rootdescend" verification option is exhaustive, and can
      therefore make a call to bt_index_parent_check() take a lot longer.
      Re-finding tuples during verification is mostly intended as an option
      for backend developers, since the corruption scenarios that it alone is
      uniquely capable of detecting seem fairly far-fetched.
      
      For example, "rootdescend" verification is much more likely to detect
      corruption of the least significant byte of a key from a pivot tuple in
      the root page of a B-Tree that already has at least three levels.
      Typically, only a few tuples on a cousin leaf page are at risk of
      "getting overlooked" by index scans in this scenario.  The corrupt key
      in the root page is only slightly corrupt: corrupt enough to give wrong
      answers to some queries, and yet not corrupt enough to allow the problem
      to be detected without verifying agreement between the leaf page and the
      root page, skipping at least one internal page level.  The existing
      bt_index_parent_check() checks never cross more than a single level.
      
      Author: Peter Geoghegan
      Reviewed-By: Heikki Linnakangas
      Discussion: https://postgr.es/m/CAH2-Wz=yTWnVu+HeHGKb2AGiADL9eprn-cKYAto4MkKOuiGtRQ@mail.gmail.com
      c1afd175
    • Peter Geoghegan's avatar
      Consider secondary factors during nbtree splits. · fab25024
      Peter Geoghegan authored
      Teach nbtree to give some consideration to how "distinguishing"
      candidate leaf page split points are.  This should not noticeably affect
      the balance of free space within each half of the split, while still
      making suffix truncation truncate away significantly more attributes on
      average.
      
      The logic for choosing a leaf split point now uses a fallback mode in
      the case where the page is full of duplicates and it isn't possible to
      find even a minimally distinguishing split point.  When the page is full
      of duplicates, the split should pack the left half very tightly, while
      leaving the right half mostly empty.  Our assumption is that logical
      duplicates will almost always be inserted in ascending heap TID order
      with v4 indexes.  This strategy leaves most of the free space on the
      half of the split that will likely be where future logical duplicates of
      the same value need to be placed.
      
      The number of cycles added is not very noticeable.  This is important
      because deciding on a split point takes place while at least one
      exclusive buffer lock is held.  We avoid using authoritative insertion
      scankey comparisons to save cycles, unlike suffix truncation proper.  We
      use a faster binary comparison instead.
      
      Note that even pg_upgrade'd v3 indexes make use of these optimizations.
      Benchmarking has shown that even v3 indexes benefit, despite the fact
      that suffix truncation will only truncate non-key attributes in INCLUDE
      indexes.  Grouping relatively similar tuples together is beneficial in
      and of itself, since it reduces the number of leaf pages that must be
      accessed by subsequent index scans.
      
      Author: Peter Geoghegan
      Reviewed-By: Heikki Linnakangas
      Discussion: https://postgr.es/m/CAH2-WzmmoLNQOj9mAD78iQHfWLJDszHEDrAzGTUMG3mVh5xWPw@mail.gmail.com
      fab25024
    • Peter Geoghegan's avatar
      Make heap TID a tiebreaker nbtree index column. · dd299df8
      Peter Geoghegan authored
      Make nbtree treat all index tuples as having a heap TID attribute.
      Index searches can distinguish duplicates by heap TID, since heap TID is
      always guaranteed to be unique.  This general approach has numerous
      benefits for performance, and is prerequisite to teaching VACUUM to
      perform "retail index tuple deletion".
      
      Naively adding a new attribute to every pivot tuple has unacceptable
      overhead (it bloats internal pages), so suffix truncation of pivot
      tuples is added.  This will usually truncate away the "extra" heap TID
      attribute from pivot tuples during a leaf page split, and may also
      truncate away additional user attributes.  This can increase fan-out,
      especially in a multi-column index.  Truncation can only occur at the
      attribute granularity, which isn't particularly effective, but works
      well enough for now.  A future patch may add support for truncating
      "within" text attributes by generating truncated key values using new
      opclass infrastructure.
      
      Only new indexes (BTREE_VERSION 4 indexes) will have insertions that
      treat heap TID as a tiebreaker attribute, or will have pivot tuples
      undergo suffix truncation during a leaf page split (on-disk
      compatibility with versions 2 and 3 is preserved).  Upgrades to version
      4 cannot be performed on-the-fly, unlike upgrades from version 2 to
      version 3.  contrib/amcheck continues to work with version 2 and 3
      indexes, while also enforcing stricter invariants when verifying version
      4 indexes.  These stricter invariants are the same invariants described
      by "3.1.12 Sequencing" from the Lehman and Yao paper.
      
      A later patch will enhance the logic used by nbtree to pick a split
      point.  This patch is likely to negatively impact performance without
      smarter choices around the precise point to split leaf pages at.  Making
      these two mostly-distinct sets of enhancements into distinct commits
      seems like it might clarify their design, even though neither commit is
      particularly useful on its own.
      
      The maximum allowed size of new tuples is reduced by an amount equal to
      the space required to store an extra MAXALIGN()'d TID in a new high key
      during leaf page splits.  The user-facing definition of the "1/3 of a
      page" restriction is already imprecise, and so does not need to be
      revised.  However, there should be a compatibility note in the v12
      release notes.
      
      Author: Peter Geoghegan
      Reviewed-By: Heikki Linnakangas, Alexander Korotkov
      Discussion: https://postgr.es/m/CAH2-WzkVb0Kom=R+88fDFb=JSxZMFvbHVC6Mn9LJ2n=X=kS-Uw@mail.gmail.com
      dd299df8
    • Peter Geoghegan's avatar
      Refactor nbtree insertion scankeys. · e5adcb78
      Peter Geoghegan authored
      Use dedicated struct to represent nbtree insertion scan keys.  Having a
      dedicated struct makes the difference between search type scankeys and
      insertion scankeys a lot clearer, and simplifies the signature of
      several related functions.  This is based on a suggestion by Andrey
      Lepikhov.
      
      Streamline how unique index insertions cache binary search progress.
      Cache the state of in-progress binary searches within _bt_check_unique()
      for later instead of having callers avoid repeating the binary search in
      an ad-hoc manner.  This makes it easy to add a new optimization:
      _bt_check_unique() now falls out of its loop immediately in the common
      case where it's already clear that there couldn't possibly be a
      duplicate.
      
      The new _bt_check_unique() scheme makes it a lot easier to manage cached
      binary search effort afterwards, from within _bt_findinsertloc().  This
      is needed for the upcoming patch to make nbtree tuples unique by
      treating heap TID as a final tiebreaker column.  Unique key binary
      searches need to restore lower and upper bounds.  They cannot simply
      continue to use the >= lower bound as the offset to insert at, because
      the heap TID tiebreaker column must be used in comparisons for the
      restored binary search (unlike the original _bt_check_unique() binary
      search, where scankey's heap TID column must be omitted).
      
      Author: Peter Geoghegan, Heikki Linnakangas
      Reviewed-By: Heikki Linnakangas, Andrey Lepikhov
      Discussion: https://postgr.es/m/CAH2-WzmE6AhUdk9NdWBf4K3HjWXZBX3+umC7mH7+WDrKcRtsOw@mail.gmail.com
      e5adcb78
    • Alexander Korotkov's avatar
      Get rid of jsonpath_gram.h and jsonpath_scanner.h · 550b9d26
      Alexander Korotkov authored
      Jsonpath grammar and scanner are both quite small.  It doesn't worth complexity
      to compile them separately.  This commit makes grammar and scanner be compiled
      at once.  Therefore, jsonpath_gram.h and jsonpath_gram.h are no longer needed.
      This commit also does some reorganization of code in jsonpath_gram.y.
      
      Discussion: https://postgr.es/m/d47b2023-3ecb-5f04-d253-d557547cf74f%402ndQuadrant.com
      550b9d26
    • Alexander Korotkov's avatar
      Remove ambiguity for jsonb_path_match() and jsonb_path_exists() · 641fde25
      Alexander Korotkov authored
      There are 2-arguments and 4-arguments versions of jsonb_path_match() and
      jsonb_path_exists().  But 4-arguments versions have optional 3rd and 4th
      arguments, that leads to ambiguity.  In the same time 2-arguments versions are
      needed only for @@ and @? operators.  So, rename 2-arguments versions to
      remove the ambiguity.
      
      Catversion is bumped.
      641fde25
  5. 19 Mar, 2019 1 commit
    • Tom Lane's avatar
      Restructure libpq's handling of send failures. · 1f39a1c0
      Tom Lane authored
      Originally, if libpq got a failure (e.g., ECONNRESET) while trying to
      send data to the server, it would just report that and wash its hands
      of the matter.  It was soon found that that wasn't a very pleasant way
      of coping with server-initiated disconnections, so we introduced a hack
      (pqHandleSendFailure) in the code that sends queries to make it peek
      ahead for server error reports before reporting the send failure.
      
      It now emerges that related cases can occur during connection setup;
      in particular, as of TLS 1.3 it's unsafe to assume that SSL connection
      failures will be reported by SSL_connect rather than during our first
      send attempt.  We could have fixed that in a hacky way by applying
      pqHandleSendFailure after a startup packet send failure, but
      (a) pqHandleSendFailure explicitly disclaims suitability for use in any
      state except query startup, and (b) the problem still potentially exists
      for other send attempts in libpq.
      
      Instead, let's fix this in a more general fashion by eliminating
      pqHandleSendFailure altogether, and instead arranging to postpone
      all reports of send failures in libpq until after we've made an
      attempt to read and process server messages.  The send failure won't
      be reported at all if we find a server message or detect input EOF.
      
      (Note: this removes one of the reasons why libpq typically overwrites,
      rather than appending to, conn->errorMessage: pqHandleSendFailure needed
      that behavior so that the send failure report would be replaced if we
      got a server message or read failure report.  Eventually I'd like to get
      rid of that overwrite behavior altogether, but today is not that day.
      For the moment, pqSendSome is assuming that its callees will overwrite
      not append to conn->errorMessage.)
      
      Possibly this change should get back-patched someday; but it needs
      testing first, so let's not consider that till after v12 beta.
      
      Discussion: https://postgr.es/m/CAEepm=2n6Nv+5tFfe8YnkUm1fXgvxR0Mm1FoD+QKG-vLNGLyKg@mail.gmail.com
      1f39a1c0