1. 27 Apr, 2016 2 commits
  2. 26 Apr, 2016 6 commits
    • Tom Lane's avatar
      Add a --brief option to git_changelog. · 8067c8f8
      Tom Lane authored
      In commit c0b05019, Andres introduced the idea of including one-line
      commit references in our major release notes.  Teach git_changelog to
      emit a (lightly adapted) version of that format, so that we don't
      have to laboriously add it to the notes after the fact.  The default
      output isn't changed, since I anticipate still using that for minor
      release notes.
      8067c8f8
    • Teodor Sigaev's avatar
      Fix tsearch docs · f1e3c760
      Teodor Sigaev authored
      Remove mention of setweight(tsquery) which wasn't included in 9.6. Also
      replace old forgotten phrase operator to new one.
      
      Dmitry Ivanov
      f1e3c760
    • Tom Lane's avatar
      Fix order of shutdown cleanup operations in PostgresNode.pm. · 08af9219
      Tom Lane authored
      Previously, database clusters created by a TAP test were shut down by
      DESTROY methods attached to the PostgresNode objects representing them.
      The trouble with that is that if the objects survive into the final global
      destruction phase (which they do), Perl executes the DESTROY methods in an
      unspecified order.  Thus, the order of shutdown of multiple clusters was
      indeterminate, which might lead to not-very-reproducible errors getting
      logged (eg from a slave whose master might or might not get killed first).
      Worse, the File::Temp objects representing the temporary PGDATA directories
      might get destroyed before the PostgresNode objects, resulting in attempts
      to delete PGDATA directories that still have live servers in them.  On
      Windows, this would lead to directory deletion failures; on Unix, it
      usually had no effects worse than erratic "could not open temporary
      statistics file "pg_stat/global.tmp": No such file or directory" log
      messages.
      
      While none of this would affect the reported result of the TAP test, which
      is already determined, it could be very confusing when one is trying to
      understand from the logs what went wrong with a failed test.
      
      To fix, do the postmaster shutdowns in an END block rather than at object
      destruction time.  The END block will execute at a well-defined (and
      reasonable) time during script termination, and it will stop the
      postmasters in order of PostgresNode object creation.  (Perhaps we should
      change that to be reverse order of creation, but the main point here is
      that we now have control which we did not before.)  Use "pg_ctl stop", not
      an asynchronous kill(SIGQUIT), so that we wait for the postmasters to shut
      down before proceeding with directory deletion.
      
      Deletion of temporary directories still happens in an unspecified order
      during global destruction, but I can see no reason to care about that
      once the postmasters are stopped.
      08af9219
    • Tom Lane's avatar
      Yet more portability hacking for degree-based trig functions. · 82311bcd
      Tom Lane authored
      The true explanation for Peter Eisentraut's report of inexact asind results
      seems to be that (a) he's compiling into x87 instruction set, which uses
      wider-than-double float registers, plus (b) the library function asin() on
      his platform returns a result that is wider than double and is not rounded
      to double width.  To fix, we have to force the function's result to be
      rounded comparably to what happened to the scaling constant asin_0_5.
      Experimentation suggests that storing it into a volatile local variable is
      the least ugly way of making that happen.  Although only asin() is known to
      exhibit an observable inexact result, we'd better do this in all the places
      where we're hoping to get an exact result by scaling.
      82311bcd
    • Robert Haas's avatar
      Enable parallel query by default. · 77cd477c
      Robert Haas authored
      Change max_parallel_degree default from 0 to 2.  It is possible that
      this is not a good idea, or that we should go with 1 worker rather
      than 2, but we won't find out without trying it.  Along the way,
      reword the documentation for max_parallel_degree a little bit to
      hopefully make it more clear.
      
      Discussion: 20160420174631.3qjjhpwsvvx5bau5@alap3.anarazel.de
      77cd477c
    • Magnus Hagander's avatar
      Fix typo in comment · b7351ced
      Magnus Hagander authored
      Author: Daniel Gustafsson
      b7351ced
  3. 25 Apr, 2016 6 commits
    • Peter Eisentraut's avatar
      pg_dump: Message style improvements · b6dacc17
      Peter Eisentraut authored
      b6dacc17
    • Kevin Grittner's avatar
      Fix C comment typo and redundant test · e65953be
      Kevin Grittner authored
      e65953be
    • Tom Lane's avatar
      New method for preventing compile-time calculation of degree constants. · 6b1a213b
      Tom Lane authored
      Commit 65abaab5 tried to prevent the scaling constants used in
      the degree-based trig functions from being precomputed at compile time,
      because some compilers do that with functions that don't yield results
      identical-to-the-last-bit to what you get at runtime.  A report from
      Peter Eisentraut suggests that some recent compilers are smart enough
      to see through that trick, though.  Instead, let's put the inputs to
      these calculations into non-const global variables, which should be a
      more reliable way of convincing the compiler that it can't assume that
      they are compile-time constants.  (If we really get desperate, we could
      mark these variables "volatile", but I do not believe we should have to.)
      6b1a213b
    • Tom Lane's avatar
      Try harder to detect a port conflict in PostgresNode.pm. · 40e89e2a
      Tom Lane authored
      Commit fab84c77 tried to get away without doing an actual bind(),
      but buildfarm results show that that doesn't get the job done.  So we must
      really bind to the target port --- and at least on my Linux box, we need a
      listen() as well, or conflicts won't be detected.  We rely on SO_REUSEADDR
      to prevent problems from starting a postmaster on the socket immediately
      after we've bound to it in the test code.  (There may be platforms where
      that doesn't work too well.  But fortunately, we only really care whether
      this works on Windows, and there the default behavior should be OK.)
      40e89e2a
    • Peter Eisentraut's avatar
    • Peter Eisentraut's avatar
      doc: Fix typo · 96687497
      Peter Eisentraut authored
      From: Andreas Seltenreich <andreas.seltenreich@credativ.de>
      96687497
  4. 24 Apr, 2016 2 commits
    • Tom Lane's avatar
      Improve PostgresNode.pm's logic for detecting already-in-use ports. · fab84c77
      Tom Lane authored
      Buildfarm members bowerbird and jacana have shown intermittent "could not
      bind IPv4 socket" failures in the BinInstallCheck stage since mid-December,
      shortly after commits 1caef31d and 9821492e changed the
      logic for selecting which port to use in temporary installations.  One
      plausible explanation is that we are randomly selecting ports that are
      already in use for some non-Postgres purpose.  Although the code tried
      to defend against already-in-use ports, it used pg_isready to probe
      the port which is quite unhelpful: if some non-Postgres server responds
      at the given address, pg_isready will generally say "no response",
      leading to exactly the wrong conclusion about whether the port is free.
      
      Instead, let's use a simple TCP connect() call to see if anything answers
      without making assumptions about what it is.  Note that this means there's
      no direct check for a conflicting Unix socket, but that should be okay
      because there should be no other Unix sockets in use in the temporary
      socket directory created for a test run.
      
      This is only a partial solution for the TCP case, since if the port number
      is in use for an outgoing connection rather than a listening socket, we'll
      fail to detect that.  We could try to bind() to the proposed port as a
      means of detecting that case, but that would introduce its own failure
      modes, since the system might consider the address to remain reserved for
      some period of time after we drop the bound socket.  Close study of the
      errors returned by bowerbird and jacana suggests that what we're seeing
      there may be conflicts with listening not outgoing sockets, so let's try
      this and see if it improves matters.  It's certainly better than what's
      there now, in any case.
      
      Michael Paquier, adjusted by me to work on non-Windows as well as Windows
      fab84c77
    • Andres Freund's avatar
      Fix documentation & config inconsistencies around 428b1d6b. · 8f91d87d
      Andres Freund authored
      Several issues:
      1) checkpoint_flush_after doc and code disagreed about the default
      2) new GUCs were missing from postgresql.conf.sample
      3) Outdated source-code comment about bgwriter_flush_after's default
      4) Sub-optimal categories assigned to new GUCs
      5) Docs suggested backend_flush_after is PGC_SIGHUP, but it's PGC_USERSET.
      6) Spell out int as integer in the docs, as done elsewhere
      
      Reported-By: Magnus Hagander, Fujii Masao
      Discussion: CAHGQGwETyTG5VYQQ5C_srwxWX7RXvFcD3dKROhvAWWhoSBdmZw@mail.gmail.com
      8f91d87d
  5. 23 Apr, 2016 3 commits
    • Tom Lane's avatar
      Rename strtoi() to strtoint(). · 0ab3595e
      Tom Lane authored
      NetBSD has seen fit to invent a libc function named strtoi(), which
      conflicts with the long-established static functions of the same name in
      datetime.c and ecpg's interval.c.  While muttering darkly about intrusions
      on application namespace, we'll rename our functions to avoid the conflict.
      
      Back-patch to all supported branches, since this would affect attempts
      to build any of them on recent NetBSD.
      
      Thomas Munro
      0ab3595e
    • Peter Eisentraut's avatar
      doc: Fix typos · b87b2f4b
      Peter Eisentraut authored
      From: Erik Rijkers <er@xs4all.nl>
      b87b2f4b
    • Bruce Momjian's avatar
      Properly mark initRectBox() as taking 'void' args · 915cee45
      Bruce Momjian authored
      Was part of box type in SP-GiST index patch.
      
      Reported-by: Emre Hasegeli
      915cee45
  6. 22 Apr, 2016 4 commits
    • Tom Lane's avatar
      Convert contrib/seg's bool-returning SQL functions to V1 call convention. · c8e81afc
      Tom Lane authored
      It appears that we can no longer get away with using V0 call convention
      for bool-returning functions in newer versions of MSVC.  The compiler
      seems to generate code that doesn't clear the higher-order bits of the
      result register, causing the bool result Datum to often read as "true"
      when "false" was intended.  This is not very surprising, since the
      function thinks it's returning a bool-width result but fmgr_oldstyle
      assumes that V0 functions return "char *"; what's surprising is that
      that hack worked for so long on so many platforms.
      
      The only functions of this description in core+contrib are in contrib/seg,
      which we'd intentionally left mostly in V0 style to serve as a warning
      canary if V0 call convention breaks.  We could imagine hacking things
      so that they're still V0 (we'd have to redeclare the bool-returning
      functions as returning some suitably wide integer type, like size_t,
      at the C level).  But on the whole it seems better to convert 'em to V1.
      We can still leave the pointer- and int-returning functions in V0 style,
      so that the test coverage isn't gone entirely.
      
      Back-patch to 9.5, since our intention is to support VS2015 in 9.5
      and later.  There's no SQL-level change in the functions' behavior
      so back-patching should be safe enough.
      
      Discussion: <22094.1461273324@sss.pgh.pa.us>
      
      Michael Paquier, adjusted some by me
      c8e81afc
    • Magnus Hagander's avatar
      Add putenv support for msvcrt from Visual Studio 2013 · 9f633b40
      Magnus Hagander authored
      This was missed when VS 2013 support was added.
      
      Michael Paquier
      9f633b40
    • Tom Lane's avatar
      Fix unexpected side-effects of operator_precedence_warning. · abb16465
      Tom Lane authored
      The implementation of that feature involves injecting nodes into the
      raw parsetree where explicit parentheses appear.  Various places in
      parse_expr.c that test to see "is this child node of type Foo" need to
      look through such nodes, else we'll get different behavior when
      operator_precedence_warning is on than when it is off.  Note that we only
      need to handle this when testing untransformed child nodes, since the
      AEXPR_PAREN nodes will be gone anyway after transformExprRecurse.
      
      Per report from Scott Ribe and additional code-reading.  Back-patch
      to 9.5 where this feature was added.
      
      Report: <ED37E303-1B0A-4CD8-8E1E-B9C4C2DD9A17@elevated-dev.com>
      abb16465
    • Tom Lane's avatar
      Fix planner failure with full join in RHS of left join. · 80f66a9a
      Tom Lane authored
      Given a left join containing a full join in its righthand side, with
      the left join's joinclause referencing only one side of the full join
      (in a non-strict fashion, so that the full join doesn't get simplified),
      the planner could fail with "failed to build any N-way joins" or related
      errors.  This happened because the full join was seen as overlapping the
      left join's RHS, and then recent changes within join_is_legal() caused
      that function to conclude that the full join couldn't validly be formed.
      Rather than try to rejigger join_is_legal() yet more to allow this,
      I think it's better to fix initsplan.c so that the required join order
      is explicit in the SpecialJoinInfo data structure.  The previous coding
      there essentially ignored full joins, relying on the fact that we don't
      flatten them in the joinlist data structure to preserve their ordering.
      That's sufficient to prevent a wrong plan from being formed, but as this
      example shows, it's not sufficient to ensure that the right plan will
      be formed.  We need to work a bit harder to ensure that the right plan
      looks sane according to the SpecialJoinInfos.
      
      Per bug #14105 from Vojtech Rylko.  This was apparently induced by
      commit 8703059c (though now that I've seen it, I wonder whether there
      are related cases that could have failed before that); so back-patch
      to all active branches.  Unfortunately, that patch also went into 9.0,
      so this bug is a regression that won't be fixed in that branch.
      80f66a9a
  7. 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
  8. 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