1. 22 Mar, 2019 18 commits
  2. 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
  3. 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
  4. 19 Mar, 2019 6 commits
    • 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
    • Alexander Korotkov's avatar
      Rename typedef in jsonpath_gram.y from "string" to "JsonPathString" · 5e28b778
      Alexander Korotkov authored
      Reason is the same as in 75c57058.
      5e28b778
    • Peter Geoghegan's avatar
      Tweak nbtsearch.c function prototype order. · 1009920a
      Peter Geoghegan authored
      nbtsearch.c's static function prototypes were slightly out of order.
      Make the order consistent with static function definition order.
      1009920a
    • Tom Lane's avatar
      Make checkpoint requests more robust. · 0dfe3d0e
      Tom Lane authored
      Commit 6f6a6d8b introduced a delay of up to 2 seconds if we're trying
      to request a checkpoint but the checkpointer hasn't started yet (or,
      much less likely, our kill() call fails).  However buildfarm experience
      shows that that's not quite enough for slow or heavily-loaded machines.
      There's no good reason to assume that the checkpointer won't start
      eventually, so we may as well make the timeout much longer, say 60 sec.
      
      However, if the caller didn't say CHECKPOINT_WAIT, it seems like a bad
      idea to be waiting at all, much less for as long as 60 sec.  We can
      remove the need for that, and make this whole thing more robust, by
      adjusting the code so that the existence of a pending checkpoint
      request is clear from the contents of shared memory, and making sure
      that the checkpointer process will notice it at startup even if it did
      not get a signal.  In this way there's no need for a non-CHECKPOINT_WAIT
      call to wait at all; if it can't send the signal, it can nonetheless
      assume that the checkpointer will eventually service the request.
      
      A potential downside of this change is that "kill -INT" on the checkpointer
      process is no longer enough to trigger a checkpoint, should anyone be
      relying on something so hacky.  But there's no obvious reason to do it
      like that rather than issuing a plain old CHECKPOINT command, so we'll
      assume that nobody is.  There doesn't seem to be a way to preserve this
      undocumented quasi-feature without introducing race conditions.
      
      Since a principal reason for messing with this is to prevent intermittent
      buildfarm failures, back-patch to all supported branches.
      
      Discussion: https://postgr.es/m/27830.1552752475@sss.pgh.pa.us
      0dfe3d0e
    • Peter Eisentraut's avatar
      Reorder LOCALLOCK structure members to compact the size · 28988a84
      Peter Eisentraut authored
      Save 8 bytes (on x86-64) by filling up padding holes.
      
      Author: Takayuki Tsunakawa <tsunakawa.takay@jp.fujitsu.com>
      Discussion: https://www.postgresql.org/message-id/20190219001639.ft7kxir2iz644alf@alap3.anarazel.de
      28988a84
    • Alexander Korotkov's avatar
      Rename typedef in jsonpath_scan.l from "keyword" to "JsonPathKeyword" · 75c57058
      Alexander Korotkov authored
      Typedef name should be both unique and non-intersect with variable names
      across all the sources.  That makes both pg_indent and debuggers happy.
      
      Discussion: https://postgr.es/m/23865.1552936099%40sss.pgh.pa.us
      75c57058