1. 15 May, 2017 2 commits
    • Tom Lane's avatar
      Make stats regression test more robust in the face of parallel query. · 7606bbb3
      Tom Lane authored
      Commit 60690a6f attempted to fix the wait_for_stats() function in this
      test so that it would wait properly if the tenk2 scans were done in
      parallel workers instead of the main session (typically as a consequence of
      force_parallel_mode being turned on).  However, we made it test for whether
      the main session's actions had been reported by looking for inserts on
      'trunc_stats_test'.  This is the Wrong Thing, because those aren't the last
      updates we expect the main session to do.  As shown by recent failures on
      buildfarm member frogmouth, it's entirely likely that the trunc_stats_test
      updates will be reported in a separate message from later updates, which
      means there can be a window in which wait_for_stats() will exit but not all
      the updates we are expecting to see will have arrived.  We should test for
      the last updates we're expecting, namely those on 'trunc_stats_test4'.
      
      Unfortunately, I doubt that this explains frogmouth's failures, because
      there's no reason to believe that it's running the tenk2 queries in
      parallel.  Still, the test is wrong on its own terms, so fix and back-patch
      to 9.6 where parallel query came in.
      7606bbb3
    • Robert Haas's avatar
      Attempt to fix compiler warning. · edbe2a29
      Robert Haas authored
      Per a report from Tom Lane, newer versions of gcc apparently think
      that partexprs_item_saved can be used uninitialized.  Try to convince
      them otherwise.
      edbe2a29
  2. 14 May, 2017 7 commits
    • Tom Lane's avatar
      Edit SGML documentation related to extended statistics. · 93ece9cc
      Tom Lane authored
      Use the "statistics object" terminology uniformly here too.  Assorted
      copy-editing.  Put new catalogs.sgml sections into alphabetical order.
      93ece9cc
    • Tom Lane's avatar
      Fix maintenance hazards caused by ill-considered use of default: cases. · e84c0195
      Tom Lane authored
      Remove default cases from assorted switches over ObjectClass and some
      related enum types, so that we'll get compiler warnings when someone
      adds a new enum value without accounting for it in all these places.
      
      In passing, re-order some switch cases as needed to match the declaration
      of enum ObjectClass.  OK, that's just neatnik-ism, but I dislike code
      that looks like it was assembled with the help of a dartboard.
      
      Discussion: https://postgr.es/m/20170512221010.nglatgt5azzdxjlj@alvherre.pgsql
      e84c0195
    • Tom Lane's avatar
      Fix handling of extended statistics during ALTER COLUMN TYPE. · b5b0db19
      Tom Lane authored
      ALTER COLUMN TYPE on a column used by a statistics object fails since
      commit 928c4de3, because the relevant switch in ATExecAlterColumnType
      is unprepared for columns to have dependencies from OCLASS_STATISTIC_EXT
      objects.
      
      Although the existing types of extended statistics don't actually need us
      to do any work for a column type change, it seems completely indefensible
      that that assumption is hidden behind the failure of an unrelated module
      to contain any code for the case.  Hence, create and call an API function
      in statscmds.c where the assumption can be explained, and where we could
      add code to deal with the problem when it inevitably becomes real.
      
      Also, the reason this wasn't handled before, neither for extended stats
      nor for the last half-dozen new OCLASS kinds :-(, is that the default:
      in that switch suppresses compiler warnings, allowing people to miss the
      need to consider it when adding an OCLASS.  We don't really need a default
      because surely getObjectClass should only return valid values of the enum;
      so remove it, and add the missed OCLASS entries where they should be.
      
      Discussion: https://postgr.es/m/20170512221010.nglatgt5azzdxjlj@alvherre.pgsql
      b5b0db19
    • Peter Eisentraut's avatar
      Update config.guess and config.sub · 65b655b5
      Peter Eisentraut authored
      65b655b5
    • Tom Lane's avatar
      Remove no-longer-needed fields of Hash plan nodes. · f6747434
      Tom Lane authored
      skewColType/skewColTypmod are no longer used in the wake of commit
      9aab83fc, and seem unlikely to be wanted in future, so let's drop 'em.
      
      Discussion: https://postgr.es/m/16364.1494520862@sss.pgh.pa.us
      f6747434
    • Tom Lane's avatar
      Standardize terminology for pg_statistic_ext entries. · f04c9a61
      Tom Lane authored
      Consistently refer to such an entry as a "statistics object", not just
      "statistics" or "extended statistics".  Previously we had a mismash of
      terms, accompanied by utter confusion as to whether the term was
      singular or plural.  That's not only grating (at least to the ear of
      a native English speaker) but could be outright misleading, eg in error
      messages that seemed to be referring to multiple objects where only one
      could be meant.
      
      This commit fixes the code and a lot of comments (though I may have
      missed a few).  I also renamed two new SQL functions,
      pg_get_statisticsextdef -> pg_get_statisticsobjdef
      pg_statistic_ext_is_visible -> pg_statistics_obj_is_visible
      to conform better with this terminology.
      
      I have not touched the SGML docs other than fixing those function
      names; the docs certainly need work but it seems like a separable task.
      
      Discussion: https://postgr.es/m/22676.1494557205@sss.pgh.pa.us
      f04c9a61
    • Andrew Dunstan's avatar
      Suppress indentation from Data::Dumper in regression tests · 12ad38b3
      Andrew Dunstan authored
      Ultra-modern versions of the perl Data::Dumper module have apparently
      changed how they indent output. Instead of trying to keep up we choose
      to tell it to supporess all indentation in the hstore_plperl regression
      tests.
      
      Backpatch to 9.5 where this feature was introduced.
      12ad38b3
  3. 13 May, 2017 9 commits
    • Andres Freund's avatar
      Specify --outputdir for isolation install check, not just plain check. · 29c7d5e4
      Andres Freund authored
      This should probably have been part of 60f826c5.
      
      Reported-By: Andrew Gierth
      29c7d5e4
    • Andres Freund's avatar
      Avoid superfluous work for commits during logical slot creation. · 524dbc14
      Andres Freund authored
      Before 955a684e logical decoding snapshot maintenance needed to
      cope with transactions it might not have seen in their entirety. For
      such transactions we'd to assume they modified the catalog (could have
      happened before we were watching), and thus a new snapshot had to be
      built, and distributed to concurrently running transactions.
      
      That's problematic because building a new snapshot isn't that cheap ,
      especially as the the array of committed transactions needs to be
      sorted.  When creating a slot on a server with a lot of transactions,
      this could make logical slot creation infeasibly expensive.
      
      After 955a684e there's no need to deal with transaction that
      aren't guaranteed to be fully observable.  That allows to avoid
      building snapshots for transactions that haven't modified catalog,
      even before reaching consistency.
      
      While this isn't necessarily a bugfix, slot creation being impossible
      in some production workloads, is severe enough to warrant
      backpatching.
      
      Author: Andres Freund, based on a quite different patch from Petr Jelinek
      Analyzed-By: Petr Jelinek
      Reviewed-By: Petr Jelinek
      Discussion: https://postgr.es/m/f37e975c-908f-858e-707f-058d3b1eb214@2ndquadrant.com
      Backpatch: 9.4-, where logical decoding has been introduced
      524dbc14
    • Andres Freund's avatar
      Fix race condition leading to hanging logical slot creation. · 955a684e
      Andres Freund authored
      The snapshot assembly during the creation of logical slots relied
      waiting for transactions in xl_running_xacts to end, by checking for
      their commit/abort records.  Unfortunately, despite locking, it is
      possible to see an xl_running_xact record listing transactions as
      ready, that have already WAL-logged an commit/abort record, as the
      locking just prevents the ProcArray to be adjusted, and the commit
      record has to be logged first.
      
      That lead to either delayed or hanging snapshot creation, because
      snapbuild.c would wait "forever" to see commit/abort records for some
      transactions.  That hang resolved only if a xl_running_xacts record
      without any running transactions happened to be logged, far from
      certain on a busy server.
      
      It's impractical to prevent that via more heavyweight locking, the
      likelihood of deadlocks and significantly increased contention would
      be too big.
      
      Instead change the initial snapshot creation to be solely based on
      tracking the oldest running transaction via
      xl_running_xacts->oldestRunningXid - that actually ends up
      significantly simplifying the code.  That has two disadvantages:
      1) Because we cannot fully "trust" the contents of xl_running_xacts,
         we cannot use it to build the initial snapshot.  Instead we have to
         wait twice for all running transactions to finish.
      2) Previously a slot, unless the race occurred, could be created when
         the all transaction perceived as running based on commit/abort
         records, now we have to wait for the next xl_running_xacts record.
      To address that, trigger logging new xl_running_xacts record from
      within snapbuild.c exactly when necessary.
      
      Unfortunately snabuild.c's SnapBuild is stored on disk, one of the
      stupider ideas of a certain Mr Freund, so we can't change it in a
      minor release.  As this is going to be backpatched, we have to hack
      around a bit to keep on-disk compatibility.  A later commit will
      rejigger that on master.
      
      Author: Andres Freund, based on a quite different patch from Petr Jelinek
      Analyzed-By: Petr Jelinek
      Reviewed-By: Petr Jelinek
      Discussion: https://postgr.es/m/f37e975c-908f-858e-707f-058d3b1eb214@2ndquadrant.com
      Backpatch: 9.4-, where logical decoding has been introduced
      955a684e
    • Tom Lane's avatar
      Redesign get_attstatsslot()/free_attstatsslot() for more safety and speed. · 9aab83fc
      Tom Lane authored
      The mess cleaned up in commit da075960 is clear evidence that it's a
      bug hazard to expect the caller of get_attstatsslot()/free_attstatsslot()
      to provide the correct type OID for the array elements in the slot.
      Moreover, we weren't even getting any performance benefit from that,
      since get_attstatsslot() was extracting the real type OID from the array
      anyway.  So we ought to get rid of that requirement; indeed, it would
      make more sense for get_attstatsslot() to pass back the type OID it found,
      in case the caller isn't sure what to expect, which is likely in binary-
      compatible-operator cases.
      
      Another problem with the current implementation is that if the stats array
      element type is pass-by-reference, we incur a palloc/memcpy/pfree cycle
      for each element.  That seemed acceptable when the code was written because
      we were targeting O(10) array sizes --- but these days, stats arrays are
      almost always bigger than that, sometimes much bigger.  We can save a
      significant number of cycles by doing one palloc/memcpy/pfree of the whole
      array.  Indeed, in the now-probably-common case where the array is toasted,
      that happens anyway so this method is basically free.  (Note: although the
      catcache code will inline any out-of-line toasted values, it doesn't
      decompress them.  At the other end of the size range, it doesn't expand
      short-header datums either.  In either case, DatumGetArrayTypeP would have
      to make a copy.  We do end up using an extra array copy step if the element
      type is pass-by-value and the array length is neither small enough for a
      short header nor large enough to have suffered compression.  But that
      seems like a very acceptable price for winning in pass-by-ref cases.)
      
      Hence, redesign to take these insights into account.  While at it,
      convert to an API in which we fill a struct rather than passing a bunch
      of pointers to individual output arguments.  That will make it less
      painful if we ever want further expansion of what get_attstatsslot can
      pass back.
      
      It's certainly arguable that this is new development and not something to
      push post-feature-freeze.  However, I view it as primarily bug-proofing
      and therefore something that's better to have sooner not later.  Since
      we aren't quite at beta phase yet, let's put it in.
      
      Discussion: https://postgr.es/m/16364.1494520862@sss.pgh.pa.us
      9aab83fc
    • Robert Haas's avatar
      Teach \d+ to show partitioning constraints. · 1848b73d
      Robert Haas authored
      The fact that we didn't have this in the first place is likely why
      the problem fixed by f8bffe9e
      escaped detection.
      
      Patch by Amit Langote, reviewed and slightly adjusted by me.
      
      Discussion: http://postgr.es/m/CA+TgmoYWnV2GMnYLG-Czsix-E1WGAbo4D+0tx7t9NdfYBDMFsA@mail.gmail.com
      1848b73d
    • Robert Haas's avatar
      Fix multi-column range partitioning constraints. · f8bffe9e
      Robert Haas authored
      The old logic was just plain wrong.
      
      Report by Olaf Gawenda.  Patch by Amit Langote, reviewed by
      Beena Emerson and by me.  Minor adjustments by me also.
      f8bffe9e
    • Tom Lane's avatar
      Avoid hard-wired sleep delays in stats regression test. · 4e37b3e1
      Tom Lane authored
      On faster machines, the overall runtime for running the core regression
      tests is under twenty seconds these days, of which the hard-wired delays
      in the stats test are a significant fraction.  But on closer inspection,
      it seems like we shouldn't need those.
      
      The initial 2-second delay is there only to reduce the risk of the test's
      stats messages not getting sent due to contention.  But analysis of the
      last ten years' worth of buildfarm runs shows no evidence that such
      failures actually occur.  (We do see failures that look like stats
      messages not getting sent, particularly on Windows; but there is little
      reason to believe that the initial delay reduces their frequency.)
      
      The later 1-second delay is there to ensure that our session's stats
      will have gotten sent.  But we could also do that by starting a fresh
      session, which takes well under 1 second even on very slow machines.
      
      Hence, let's remove both delays and see what happens.  The first delay
      was the only test of pg_sleep_for() in the regression tests, but we can
      move that responsibility into wait_for_stats().
      
      Discussion: https://postgr.es/m/17795.1493869423@sss.pgh.pa.us
      4e37b3e1
    • Andrew Dunstan's avatar
      Use a better way of skipping all subscription tests on Windows · 8d9f0609
      Andrew Dunstan authored
      This way we only need to specify the number of tests in one place, and
      the output is also less verbose.
      8d9f0609
    • Alvaro Herrera's avatar
      Complete tab completion for DROP STATISTICS · d99d58cd
      Alvaro Herrera authored
      Tab-completing DROP STATISTICS would only work if you started writing
      the schema name containing the statistics object, because the visibility
      clause was missing.  To add it, we need to add SQL-callable support for
      testing visibility of a statistics object, like all other object types
      already have.
      
      Discussion: https://postgr.es/m/22676.1494557205@sss.pgh.pa.us
      d99d58cd
  4. 12 May, 2017 16 commits
    • Tom Lane's avatar
      Avoid searching for callback functions in CallSyscacheCallbacks(). · 2df5d465
      Tom Lane authored
      We have now grown enough registerable syscache-invalidation callback
      functions that the original assumption that there would be few of them
      is causing performance problems.  In particular, let's fix things so that
      CallSyscacheCallbacks doesn't have to search the whole array to find
      which callback(s) to invoke for a given cache ID.  Preserve the original
      behavior that callbacks are called in order of registration, just in
      case there's someplace that depends on that (which I doubt).
      
      In support of this, export the number of syscaches from syscache.h.
      People could have found that out anyway from the enum, but adding a
      #define makes that much safer.
      
      This provides a useful additional speedup in Mathieu Fenniak's
      logical-decoding test case, although we're reaching the point of
      diminishing returns there.  I think any further improvement will have
      to come from reducing the number of cache invalidations that are
      triggered in the first place.  Still, we can hope that this change
      gives some incremental benefit for all invalidation scenarios.
      
      Back-patch to 9.4 where logical decoding was introduced.
      
      Discussion: https://postgr.es/m/CAHoiPjzea6N0zuCi=+f9v_j94nfsy6y8SU7-=bp4=7qw6_i=Rg@mail.gmail.com
      2df5d465
    • Bruce Momjian's avatar
      doc: update markup for release note "release date" block · 9ed74fd4
      Bruce Momjian authored
      This has to be backpatched to all supported releases so release markup
      added to HEAD and copied to back branches matches the existing markup.
      
      Reported-by: Peter Eisentraut
      
      Discussion: 2b8a2552-fffa-f7c8-97c5-14db47a87731@2ndquadrant.com
      
      Author: initial patch and sample markup by Peter Eisentraut
      
      Backpatch-through: 9.2
      9ed74fd4
    • Tom Lane's avatar
      Reduce initial size of RelfilenodeMapHash. · 8085a4f7
      Tom Lane authored
      A test case provided by Mathieu Fenniak shows that hash_seq_search'ing
      this hashtable can consume a very significant amount of overhead during
      logical decoding, which triggers frequent cache invalidation.  Testing
      suggests that the actual population of the hashtable is often no more
      than a few dozen entries, so we can cut the overhead just by dropping
      the initial number of buckets down from 1024 --- I chose to cut it to 64.
      (In situations where we do have a significant number of entries, we
      shouldn't get any real penalty from doing this, as the dynahash.c code
      will resize the hashtable automatically.)
      
      This gives a further factor-of-two savings in Mathieu's test case.
      That may be overly optimistic for real-world benefit, as real cases
      may have larger average table populations, but it's hard to see it
      turning into a net negative for any workload.
      
      Back-patch to 9.4 where relfilenodemap.c was introduced.
      
      Discussion: https://postgr.es/m/CAHoiPjzea6N0zuCi=+f9v_j94nfsy6y8SU7-=bp4=7qw6_i=Rg@mail.gmail.com
      8085a4f7
    • Alvaro Herrera's avatar
      getObjectDescription: support extended statistics · 5e2af609
      Alvaro Herrera authored
      This was missed in 7b504eb2.
      
      Remove the "default:" clause in the switch, to avoid this problem in the
      future.  Other switches involving the same enum should probably be
      changed in the same way, but are not touched by this patch.
      
      Discussion: https://postgr.es/m/20170512204800.iqt2uwyx3c32j45r@alvherre.pgsql
      5e2af609
    • Tom Lane's avatar
      Avoid searching for the target catcache in CatalogCacheIdInvalidate. · 50ee1c74
      Tom Lane authored
      A test case provided by Mathieu Fenniak shows that the initial search for
      the target catcache in CatalogCacheIdInvalidate consumes a very significant
      amount of overhead in cases where cache invalidation is triggered but has
      little useful work to do.  There is no good reason for that search to exist
      at all, as the index array maintained by syscache.c allows direct lookup of
      the catcache from its ID.  We just need a frontend function in syscache.c,
      matching the division of labor for most other cache-accessing operations.
      
      While there's more that can be done in this area, this patch alone reduces
      the runtime of Mathieu's example by 2X.  We can hope that it offers some
      useful benefit in other cases too, although usually cache invalidation
      overhead is not such a striking fraction of the total runtime.
      
      Back-patch to 9.4 where logical decoding was introduced.  It might be
      worth going further back, but presently the only case we know of where
      cache invalidation is really a significant burden is in logical decoding.
      Also, older branches have fewer catcaches, reducing the possible benefit.
      
      (Note: although this nominally changes catcache's API, we have always
      documented CatalogCacheIdInvalidate as a private function, so I would
      have little sympathy for an external module calling it directly.  So
      backpatching should be fine.)
      
      Discussion: https://postgr.es/m/CAHoiPjzea6N0zuCi=+f9v_j94nfsy6y8SU7-=bp4=7qw6_i=Rg@mail.gmail.com
      50ee1c74
    • Tom Lane's avatar
      Fix dependencies for extended statistics objects. · 928c4de3
      Tom Lane authored
      A stats object ought to have a dependency on each individual column
      it reads, not the entire table.  Doing this honestly lets us get rid
      of the hard-wired logic in RemoveStatisticsExt, which seems to have
      been misguidedly modeled on RemoveStatistics; and it will be far easier
      to extend to multiple tables later.
      
      Also, add overlooked dependency on owner, and make the dependency on
      schema be NORMAL like every other such dependency.
      
      There remains some unfinished work here, which is to allow statistics
      objects to be extension members.  That takes more effort than just
      adding the dependency call, though, so I left it out for now.
      
      initdb forced because this changes the set of pg_depend records that
      should exist for a statistics object.
      
      Discussion: https://postgr.es/m/22676.1494557205@sss.pgh.pa.us
      928c4de3
    • Alvaro Herrera's avatar
      Change CREATE STATISTICS syntax · bc085205
      Alvaro Herrera authored
      Previously, we had the WITH clause in the middle of the command, where
      you'd specify both generic options as well as statistic types.  Few
      people liked this, so this commit changes it to remove the WITH keyword
      from that clause and makes it accept statistic types only.  (We
      currently don't have any generic options, but if we invent in the
      future, we will gain a new WITH clause, probably at the end of the
      command).
      
      Also, the column list is now specified without parens, which makes the
      whole command look more similar to a SELECT command.  This change will
      let us expand the command to supporting expressions (not just columns
      names) as well as multiple tables and their join conditions.
      
      Tom added lots of code comments and fixed some parts of the CREATE
      STATISTICS reference page, too; more changes in this area are
      forthcoming.  He also fixed a potential problem in the alter_generic
      regression test, reducing verbosity on a cascaded drop to avoid
      dependency on message ordering, as we do in other tests.
      
      Tom also closed a security bug: we documented that table ownership was
      required in order to create a statistics object on it, but didn't
      actually implement it.
      
      Implement tab-completion for statistics objects.  This can stand some
      more improvement.
      
      Authors: Alvaro Herrera, with lots of cleanup by Tom Lane
      Discussion: https://postgr.es/m/20170420212426.ltvgyhnefvhixm6i@alvherre.pgsql
      bc085205
    • Peter Eisentraut's avatar
    • Peter Eisentraut's avatar
      Standardize "WAL location" terminology · d496a657
      Peter Eisentraut authored
      Other previously used terms were "WAL position" or "log position".
      d496a657
    • Peter Eisentraut's avatar
      Replace "transaction log" with "write-ahead log" · c1a7f64b
      Peter Eisentraut authored
      This makes documentation and error messages match the renaming of "xlog"
      to "wal" in APIs and file naming.
      c1a7f64b
    • Andrew Dunstan's avatar
      Honor PROVE_FLAGS environment setting · 56b6ef89
      Andrew Dunstan authored
      On MSVC builds and on back branches that means removing the hardcoded
      --verbose setting. On master for Unix that means removing the empty
      setting in the global Makefile so that the value can be acquired from
      the environment as well as from the make arguments.
      
      Backpatch to 9.4 where we introduced TAP tests
      56b6ef89
    • Andrew Dunstan's avatar
      Add libxml2 include path for MSVC builds · b757e01f
      Andrew Dunstan authored
      On Unix this path is detected via the use of xml2-config, but that's not
      available on Windows. This means that users building with libxml2 will
      no longer need to move things around from the standard libxml2
      installation for MSVC builds.
      
      Backpatch to all live branches.
      b757e01f
    • Peter Eisentraut's avatar
      pg_dump: Add --no-publications option · 96e1cb4c
      Peter Eisentraut authored
      Author: Michael Paquier <michael.paquier@gmail.com>
      96e1cb4c
    • Peter Eisentraut's avatar
      Rework the options syntax for logical replication commands · b807f598
      Peter Eisentraut authored
      For CREATE/ALTER PUBLICATION/SUBSCRIPTION, use similar option style as
      other statements that use a WITH clause for options.
      
      Author: Petr Jelinek <petr.jelinek@2ndquadrant.com>
      b807f598
    • Andrew Dunstan's avatar
      Avoid tests which crash the calling process on Windows · 734cb4c2
      Andrew Dunstan authored
      Certain recovery tests use the Perl IPC::Run module's start/kill_kill
      method of processing. On at least some versions of perl this causes the
      whole process and its caller to crash. If we ever find a better way of
      doing these tests they can be re-enabled on this platform. This does not
      affect Mingw or Cygwin builds, which use a different perl and a
      different shell and so are not affected.
      734cb4c2
    • Simon Riggs's avatar
      Lag tracking for logical replication · 024711bb
      Simon Riggs authored
      Lag tracking is called for each commit, but we introduce
      a pacing delay to ensure we don't swamp the lag tracker.
      
      Author: Petr Jelinek, with minor pacing delay code from me
      024711bb
  5. 11 May, 2017 3 commits
  6. 10 May, 2017 3 commits