1. 21 Apr, 2016 13 commits
    • Tom Lane's avatar
      Improve TranslateSocketError() to handle more Windows error codes. · 125ad539
      Tom Lane authored
      The coverage was rather lean for cases that bind() or listen() might
      return.  Add entries for everything that there's a direct equivalent
      for in the set of Unix errnos that elog.c has heard of.
      125ad539
    • Tom Lane's avatar
      Remove dead code in win32.h. · e5452815
      Tom Lane authored
      There's no longer a need for the MSVC-version-specific code stanza that
      forcibly redefines errno code symbols, because since commit 73838b52 we're
      unconditionally redefining them in the stanza before this one anyway.
      Now it's merely confusing and ugly, so get rid of it; and improve the
      comment that explains what's going on here.
      
      Although this is just cosmetic, back-patch anyway since I'm intending
      to back-patch some less-cosmetic changes in this same hunk of code.
      e5452815
    • Tom Lane's avatar
      PGDLLIMPORT-ify old_snapshot_threshold. · 14216649
      Tom Lane authored
      Revert commit 7cb1db1d, which represented
      a misunderstanding of the problem (if snapmgr.h weren't already included
      in bufmgr.h, things wouldn't compile anywhere).  Instead install what
      I think is the real fix.
      14216649
    • Tom Lane's avatar
      Fix ruleutils.c's dumping of ScalarArrayOpExpr containing an EXPR_SUBLINK. · 1f7c85b8
      Tom Lane authored
      When we shoehorned "x op ANY (array)" into the SQL syntax, we created a
      fundamental ambiguity as to the proper treatment of a sub-SELECT on the
      righthand side: perhaps what's meant is to compare x against each row of
      the sub-SELECT's result, or perhaps the sub-SELECT is meant as a scalar
      sub-SELECT that delivers a single array value whose members should be
      compared against x.  The grammar resolves it as the former case whenever
      the RHS is a select_with_parens, making the latter case hard to reach ---
      but you can get at it, with tricks such as attaching a no-op cast to the
      sub-SELECT.  Parse analysis would throw away the no-op cast, leaving a
      parsetree with an EXPR_SUBLINK SubLink directly under a ScalarArrayOpExpr.
      ruleutils.c was not clued in on this fine point, and would naively emit
      "x op ANY ((SELECT ...))", which would be parsed as the first alternative,
      typically leading to errors like "operator does not exist: text = text[]"
      during dump/reload of a view or rule containing such a construct.  To fix,
      emit a no-op cast when dumping such a parsetree.  This might well be
      exactly what the user wrote to get the construct accepted in the first
      place; and even if she got there with some other dodge, it is a valid
      representation of the parsetree.
      
      Per report from Karl Czajkowski.  He mentioned only a case involving
      RLS policies, but actually the problem is very old, so back-patch to
      all supported branches.
      
      Report: <20160421001832.GB7976@moraine.isi.edu>
      1f7c85b8
    • Robert Haas's avatar
      Prevent possible crash reading pg_stat_activity. · c4a586c4
      Robert Haas authored
      Also, avoid reading PGPROC's wait_event field twice, once for the wait
      event and again for the wait_event_type, because the value might change
      in the middle.
      
      Petr Jelinek and Robert Haas
      c4a586c4
    • Robert Haas's avatar
      Comment improvements for ForeignPath. · 36f69fae
      Robert Haas authored
      It's not necessarily just scanning a base relation any more.
      
      Amit Langote and Etsuro Fujita
      36f69fae
    • Robert Haas's avatar
      Fix assorted defects in 09adc9a8. · 9f84280a
      Robert Haas authored
      That commit increased all shared memory allocations to the next higher
      multiple of PG_CACHE_LINE_SIZE, but it didn't ensure that allocation
      started on a cache line boundary.  It also failed to remove a couple
      other pieces of now-useless code.
      
      BUFFERALIGN() is perhaps obsolete at this point, and likely should be
      removed at some point, too, but that seems like it can be left to a
      future cleanup.
      
      Mistakes all pointed out by Andres Freund.  The patch is mine, with
      a few extra assertions which I adopted from his version of this fix.
      9f84280a
    • Kevin Grittner's avatar
      Include snapmgr.h in blscan.c · 7cb1db1d
      Kevin Grittner authored
      Windows builds on buildfarm are failing because
      old_snapshot_threshold is not found in the bloom filter contrib
      module.
      7cb1db1d
    • Robert Haas's avatar
      Allow queries submitted by postgres_fdw to be canceled. · f039eaac
      Robert Haas authored
      This fixes a problem which is not new, but with the advent of direct
      foreign table modification in 0bf3ae88,
      it's somewhat more likely to be annoying than previously.  So,
      arrange for a local query cancelation to propagate to the remote side.
      
      Michael Paquier, reviewed by Etsuro Fujita.	 Original report by
      Thom Brown.
      f039eaac
    • Kevin Grittner's avatar
      Inline initial comparisons in TestForOldSnapshot() · 11e178d0
      Kevin Grittner authored
      Even with old_snapshot_threshold = -1 (which disables the "snapshot
      too old" feature), performance regressions were seen at moderate to
      high concurrency.  For example, a one-socket, four-core system
      running 200 connections at saturation could see up to a 2.3%
      regression, with larger regressions possible on NUMA machines.
      By inlining the early (smaller, faster) tests in the
      TestForOldSnapshot() function, the i7 case dropped to a 0.2%
      regression, which could easily just be noise, and is clearly an
      improvement.  Further testing will show whether more is needed.
      11e178d0
    • Robert Haas's avatar
      postgres_fdw: Don't push down certain full joins. · 5b1f9ce1
      Robert Haas authored
      If there's a filter condition on either side of a full outer join,
      it is neither correct to attach it to the join's ON clause nor to
      throw it into the toplevel WHERE clause.  Just don't push down the
      join in that case.
      
      To maximize the number of cases where we can still push down full
      joins, push inner join conditions into the ON clause at the first
      opportunity rather than postponing them to the top-level WHERE
      clause.  This produces nicer SQL, anyway.
      
      This bug was introduced in e4106b25.
      
      Ashutosh Bapat, per report from Rajkumar Raghuwanshi.
      5b1f9ce1
    • Tom Lane's avatar
      Honor PGCTLTIMEOUT environment variable for pg_regress' startup wait. · cbabb70f
      Tom Lane authored
      In commit 2ffa8696 we made pg_ctl recognize an environment variable
      PGCTLTIMEOUT to set the default timeout for starting and stopping the
      postmaster.  However, pg_regress uses pg_ctl only for the "stop" end of
      that; it has bespoke code for starting the postmaster, and that code has
      historically had a hard-wired 60-second timeout.  Further buildfarm
      experience says it'd be a good idea if that timeout were also controlled
      by PGCTLTIMEOUT, so let's make it so.  Like the previous patch, back-patch
      to all active branches.
      
      Discussion: <13969.1461191936@sss.pgh.pa.us>
      cbabb70f
    • Robert Haas's avatar
      Add pg_dump support for the new PARALLEL option for aggregates. · b4e0f183
      Robert Haas authored
      This was an oversight in commit 41ea0c23.
      
      Fabrízio de Royes Mello, per a report from Tushar Ahuja
      b4e0f183
  2. 20 Apr, 2016 4 commits
    • Robert Haas's avatar
      Forbid parallel Hash Right Join or Hash Full Join. · 9c75e1a3
      Robert Haas authored
      That won't work.  You'll get bogus null-extended rows.
      
      Mithun Cy
      9c75e1a3
    • Magnus Hagander's avatar
      Update backup documentation for new APIs · cfb863f2
      Magnus Hagander authored
      This includes the rest of the documentation that was not included
      in 71176854. A larger restructure would still be wanted, but with
      this commit the documentation of the new features is complete.
      cfb863f2
    • Tom Lane's avatar
      Fix memory leak and other bugs in ginPlaceToPage() & subroutines. · bde361fe
      Tom Lane authored
      Commit 36a35c55 turned the interface between ginPlaceToPage and
      its subroutines in gindatapage.c and ginentrypage.c into a royal mess:
      page-update critical sections were started in one place and finished in
      another place not even in the same file, and the very same subroutine
      might return having started a critical section or not.  Subsequent patches
      band-aided over some of the problems with this design by making things
      even messier.
      
      One user-visible resulting problem is memory leaks caused by the need for
      the subroutines to allocate storage that would survive until ginPlaceToPage
      calls XLogInsert (as reported by Julien Rouhaud).  This would not typically
      be noticeable during retail index updates.  It could be visible in a GIN
      index build, in the form of memory consumption swelling to several times
      the commanded maintenance_work_mem.
      
      Another rather nasty problem is that in the internal-page-splitting code
      path, we would clear the child page's GIN_INCOMPLETE_SPLIT flag well before
      entering the critical section that it's supposed to be cleared in; a
      failure in between would leave the index in a corrupt state.  There were
      also assorted coding-rule violations with little immediate consequence but
      possible long-term hazards, such as beginning an XLogInsert sequence before
      entering a critical section, or calling elog(DEBUG) inside a critical
      section.
      
      To fix, redefine the API between ginPlaceToPage() and its subroutines
      by splitting the subroutines into two parts.  The "beginPlaceToPage"
      subroutine does what can be done outside a critical section, including
      full computation of the result pages into temporary storage when we're
      going to split the target page.  The "execPlaceToPage" subroutine is called
      within a critical section established by ginPlaceToPage(), and it handles
      the actual page update in the non-split code path.  The critical section,
      as well as the XLOG insertion call sequence, are both now always started
      and finished in ginPlaceToPage().  Also, make ginPlaceToPage() create and
      work in a short-lived memory context to eliminate the leakage problem.
      (Since a short-lived memory context had been getting created in the most
      common code path in the subroutines, this shouldn't cause any noticeable
      performance penalty; we're just moving the overhead up one call level.)
      
      In passing, fix a bunch of comments that had gone unmaintained throughout
      all this klugery.
      
      Report: <571276DD.5050303@dalibo.com>
      bde361fe
    • Kevin Grittner's avatar
      Revert no-op changes to BufferGetPage() · a343e223
      Kevin Grittner authored
      The reverted changes were intended to force a choice of whether any
      newly-added BufferGetPage() calls needed to be accompanied by a
      test of the snapshot age, to support the "snapshot too old"
      feature.  Such an accompanying test is needed in about 7% of the
      cases, where the page is being used as part of a scan rather than
      positioning for other purposes (such as DML or vacuuming).  The
      additional effort required for back-patching, and the doubt whether
      the intended benefit would really be there, have indicated it is
      best just to rely on developers to do the right thing based on
      comments and existing usage, as we do with many other conventions.
      
      This change should have little or no effect on generated executable
      code.
      
      Motivated by the back-patching pain of Tom Lane and Robert Haas
      a343e223
  3. 19 Apr, 2016 1 commit
    • Tom Lane's avatar
      Improve regression tests for degree-based trigonometric functions. · 4db0d2d2
      Tom Lane authored
      Print the actual value of each function result that's expected to be exact,
      rather than merely emitting a NULL if it's not right.  Although we print
      these with extra_float_digits = 3, we should not trust that the platform
      will produce a result visibly different from the expected value if it's off
      only in the last place; hence, also include comparisons against the exact
      values as before.  This is a bit bulkier and uglier than the previous
      printout, but it will provide more information and be easier to interpret
      if there's a test failure.
      
      Discussion: <18241.1461073100@sss.pgh.pa.us>
      4db0d2d2
  4. 18 Apr, 2016 4 commits
    • Tom Lane's avatar
      Make partition-lock-release coding more transparent in BufferAlloc(). · a0382e2d
      Tom Lane authored
      Coverity complained that oldPartitionLock was possibly dereferenced after
      having been set to NULL.  That actually can't happen, because we'd only use
      it if (oldFlags & BM_TAG_VALID) is true.  But nonetheless Coverity is
      justified in complaining, because at line 1275 we actually overwrite
      oldFlags, and then still expect its BM_TAG_VALID bit to be a safe guide to
      whether to release the oldPartitionLock.  Thus, the code would be incorrect
      if someone else had changed the buffer's BM_TAG_VALID flag meanwhile.
      That should not happen, since we hold pin on the buffer throughout this
      sequence, but it's starting to look like a rather shaky chain of logic.
      And there's no need for such assumptions, because we can simply replace
      the (oldFlags & BM_TAG_VALID) tests with (oldPartitionLock != NULL),
      which has identical results and makes it plain to all comers that we don't
      dereference a null pointer.  A small side benefit is that the range of
      liveness of oldFlags is greatly reduced, possibly allowing the compiler
      to save a register.
      
      This is just cleanup, not an actual bug fix, so there seems no need
      for a back-patch.
      a0382e2d
    • Tom Lane's avatar
      Further reduce the number of semaphores used under --disable-spinlocks. · 75c24d0f
      Tom Lane authored
      Per discussion, there doesn't seem to be much value in having
      NUM_SPINLOCK_SEMAPHORES set to 1024: under any scenario where you are
      running more than a few backends concurrently, you really had better have a
      real spinlock implementation if you want tolerable performance.  And 1024
      semaphores is a sizable fraction of the system-wide SysV semaphore limit
      on many platforms.  Therefore, reduce this setting's default value to 128
      to make it less likely to cause out-of-semaphores problems.
      75c24d0f
    • Fujii Masao's avatar
      Fix typo in docs. · 8ce8307b
      Fujii Masao authored
      Artur Zakirov
      8ce8307b
    • Peter Eisentraut's avatar
      doc: Document that sequences can also be extension configuration tables · d460c7cc
      Peter Eisentraut authored
      From: Michael Paquier <michael.paquier@gmail.com>
      d460c7cc
  5. 17 Apr, 2016 1 commit
    • Tom Lane's avatar
      Avoid code duplication in \crosstabview. · 9603a325
      Tom Lane authored
      In commit 6f0d6a50 I added a duplicate copy of psqlscanslash's identifier
      downcasing code, but actually it's not hard to split that out as a callable
      subroutine and avoid the duplication.
      9603a325
  6. 16 Apr, 2016 7 commits
  7. 15 Apr, 2016 10 commits
    • Tom Lane's avatar
      Use less-generic names in matview.sql. · 4447f0bc
      Tom Lane authored
      The original coding of this test used table and view names like "t",
      "tv", "foo", etc.  This tended to interfere with doing simple manual
      tests in the regression database; not to mention that it posed a
      considerable risk of conflict with other regression test scripts.
      Prefix these names with "mvtest_" to avoid such conflicts.
      
      Also, change transiently-created role name to be "regress_xxx" per
      discussions about being careful with regression-test role creation.
      4447f0bc
    • Tom Lane's avatar
      Fix possible crash in ALTER TABLE ... REPLICA IDENTITY USING INDEX. · 8f1911d5
      Tom Lane authored
      Careless coding added by commit 07cacba9 could result in a crash
      or a bizarre error message if someone tried to select an index on the
      OID column as the replica identity index for a table.  Back-patch to 9.4
      where the feature was introduced.
      
      Discussion: CAKJS1f8TQYgTRDyF1_u9PVCKWRWz+DkieH=U7954HeHVPJKaKg@mail.gmail.com
      
      David Rowley
      8f1911d5
    • Robert Haas's avatar
      postgres_fdw: Clean up handling of system columns. · da7d44b6
      Robert Haas authored
      Previously, querying the xmin column of a single postgres_fdw foreign
      table fetched the tuple length, xmax the typmod, and cmin or cmax the
      composite type OID of the tuple.  However, when you queried several
      such tables and the join got shipped to the remote side, these columns
      ended up containing the remote values of the corresponding columns.
      Both behaviors are rather unprincipled, the former for obvious reasons
      and the latter because the remote values of these columns don't have
      any local significance; our transaction IDs are in a different space
      than those of the remote machine.  Clean this up by setting all of
      these fields to 0 in both cases.  Also fix the handling of tableoid
      to be sane.
      
      Robert Haas and Ashutosh Bapat, reviewed by Etsuro Fujita.
      da7d44b6
    • Robert Haas's avatar
      Tweak EXPLAIN for parallel query to show workers launched. · 5702277c
      Robert Haas authored
      The previous display was sort of confusing, because it didn't
      distinguish between the number of workers that we planned to launch
      and the number that actually got launched.  This has already confused
      several people, so display both numbers and label them clearly.
      
      Julien Rouhaud, reviewed by me.
      5702277c
    • Tom Lane's avatar
      Fix portability problem induced by commit a6f6b781. · 6b85d4ba
      Tom Lane authored
      pg_xlogdump includes bufmgr.h.  With a compiler that emits code for
      static inline functions even when they're unreferenced, that leads
      to unresolved external references in the new static-inline version
      of BufferGetPage().  So hide it with #ifndef FRONTEND, as we've done
      for similar issues elsewhere.  Per buildfarm member pademelon.
      6b85d4ba
    • Magnus Hagander's avatar
      Fix typo in comment · ba8fe38f
      Magnus Hagander authored
      ba8fe38f
    • Magnus Hagander's avatar
      Update helptext for vcregress.pl · cf086b1c
      Magnus Hagander authored
      This has clearly not been tracking the code changse for quite some time.
      
      Michael Paquier, problem spotted by Kyotaro HORIGUCHI
      cf086b1c
    • Fujii Masao's avatar
      Make regression test for multiple synchronous standbys more stable. · 36c1c916
      Fujii Masao authored
      The regression test checks whether the output of pg_stat_replication is
      expected or not after changing synchronous_standby_names and reloading
      the configuration file. Regarding this test logic, previously there was
      a timing issue which made the test result unstable. That is,
      pg_stat_replication could return unexpected result during small window
      after the configuration file was reloaded before new setting value
      took effect, and which made the test fail.
      
      This commit changes the test logic so that it uses a loop with a timeout
      to give some room for the test to pass. Now the test fails only when
      pg_stat_replication keeps returning unexpected result for 30 seconds.
      
      Michael Paquier
      36c1c916
    • Tom Lane's avatar
      Fix memory leak in GIN index scans. · f0e766bd
      Tom Lane authored
      The code had a query-lifespan memory leak when encountering GIN entries
      that have posting lists (rather than posting trees, ie, there are a
      relatively small number of heap tuples containing this index key value).
      With a suitable data distribution this could add up to a lot of leakage.
      Problem seems to have been introduced by commit 36a35c55, so back-patch
      to 9.4.
      
      Julien Rouhaud
      f0e766bd
    • Tom Lane's avatar
      Rethink \crosstabview's argument parsing logic. · 6f0d6a50
      Tom Lane authored
      \crosstabview interpreted its arguments in an unusual way, including
      doing case-insensitive matching of unquoted column names, which is
      surely not the right thing.  Rip that out in favor of doing something
      equivalent to the dequoting/case-folding rules used by other psql
      commands.  To keep it simple, change the syntax so that the optional
      sort column is specified as a separate argument, instead of the
      also-quite-unusual syntax that attached it to the colH argument with
      a colon.
      
      Also, rework the error messages to be closer to project style.
      6f0d6a50