1. 03 Aug, 2015 6 commits
    • Heikki Linnakangas's avatar
      Fix pg_rewind when pg_xlog is a symlink. · 0e42397f
      Heikki Linnakangas authored
      pg_xlog is often a symlink, typically to a different filesystem. Don't
      get confused and comlain about by that, and just always pretend that it's a
      normal directory, even if it's really a symlink.
      
      Also add a test case for this.
      
      Backpatch to 9.5.
      0e42397f
    • Heikki Linnakangas's avatar
      Clean up pg_rewind regression test script. · 69b7a35c
      Heikki Linnakangas authored
      Since commit 01f6bb4b, TestLib.pm has exported path to tmp_check directory,
      so let's use that also for the pg_rewind test clusters etc.
      
      Also, in master, the $tempdir_short variable has not been used since commit
      13d856e1, which moved the initdb-running code to TestLib.pm.
      
      Backpatch to 9.5.
      69b7a35c
    • Tom Lane's avatar
      Make modules/test_ddl_deparse/.gitignore match its siblings. · e2b49db0
      Tom Lane authored
      Not sure why /tmp_check/ was omitted from this one, but even if it
      isn't really needed right now, it's inconsistent not to include it.
      e2b49db0
    • Tom Lane's avatar
      contrib/isn now needs a .gitignore file. · fd7ed26c
      Tom Lane authored
      Oversight in commit cb3384a0.
      Back-patch to 9.1, like that commit.
      fd7ed26c
    • Tom Lane's avatar
      Fix a number of places that produced XX000 errors in the regression tests. · 09cecdf2
      Tom Lane authored
      It's against project policy to use elog() for user-facing errors, or to
      omit an errcode() selection for errors that aren't supposed to be "can't
      happen" cases.  Fix all the violations of this policy that result in
      ERRCODE_INTERNAL_ERROR log entries during the standard regression tests,
      as errors that can reliably be triggered from SQL surely should be
      considered user-facing.
      
      I also looked through all the files touched by this commit and fixed
      other nearby problems of the same ilk.  I do not claim to have fixed
      all violations of the policy, just the ones in these files.
      
      In a few places I also changed existing ERRCODE choices that didn't
      seem particularly appropriate; mainly replacing ERRCODE_SYNTAX_ERROR
      by something more specific.
      
      Back-patch to 9.5, but no further; changing ERRCODE assignments in
      stable branches doesn't seem like a good idea.
      09cecdf2
    • Andrew Dunstan's avatar
      Allow TAP tests to run under Msys · 690ed2b7
      Andrew Dunstan authored
      The Msys DTK perl, which is required to run TAP tests under Msys as a
      native perl won't recognize the correct virtual paths, has its osname
      recorded in the Config module as 'msys' instead of 'MSWin32'. To avoid
      having to repeat the test a variable is created that is true iff the
      osname is either of these values, and is then used everywhere that
      matters.
      690ed2b7
  2. 02 Aug, 2015 7 commits
    • Tom Lane's avatar
      Avoid calling memcpy() with a NULL source pointer and count == 0. · 13bba022
      Tom Lane authored
      As in commit 0a52d378, avoid doing something that has undefined
      results according to the C standard, even though in practice there does
      not seem to be any problem with it.
      
      This fixes two places in numeric.c that demonstrably could call memcpy()
      with such arguments.  I looked through that file and didn't see any other
      places with similar hazards; this is not to claim that there are not such
      places in other files.
      
      Per report from Piotr Stefaniak.  Back-patch to 9.5 which is where the
      previous commit was added.  We're more or less setting a precedent that
      we will not worry about this type of issue in pre-9.5 branches unless
      someone demonstrates a problem in the field.
      13bba022
    • Heikki Linnakangas's avatar
      Fix output of ISBN-13 numbers beginning with 979. · cb3384a0
      Heikki Linnakangas authored
      An EAN beginning with 979 (but not 9790 - those are ISMN's) are accepted
      as ISBN numbers, but they cannot be represented in the old, 10-digit ISBN
      format. They must be output in the new 13-digit ISBN-13 format. We printed
      out an incorrect value for those.
      
      Also add a regression test, to test this and some other basic functionality
      of the module.
      
      Patch by Fabien Coelho. This fixes bug #13442, reported by B.Z. Backpatch
      to 9.1, where we started to recognize ISBN-13 numbers.
      cb3384a0
    • Tom Lane's avatar
      Fix incorrect order of lock file removal and failure to close() sockets. · d73d14c2
      Tom Lane authored
      Commit c9b0cbe9 accidentally broke the
      order of operations during postmaster shutdown: it resulted in removing
      the per-socket lockfiles after, not before, postmaster.pid.  This creates
      a race-condition hazard for a new postmaster that's started immediately
      after observing that postmaster.pid has disappeared; if it sees the
      socket lockfile still present, it will quite properly refuse to start.
      This error appears to be the explanation for at least some of the
      intermittent buildfarm failures we've seen in the pg_upgrade test.
      
      Another problem, which has been there all along, is that the postmaster
      has never bothered to close() its listen sockets, but has just allowed them
      to close at process death.  This creates a different race condition for an
      incoming postmaster: it might be unable to bind to the desired listen
      address because the old postmaster is still incumbent.  This might explain
      some odd failures we've seen in the past, too.  (Note: this is not related
      to the fact that individual backends don't close their client communication
      sockets.  That behavior is intentional and is not changed by this patch.)
      
      Fix by adding an on_proc_exit function that closes the postmaster's ports
      explicitly, and (in 9.3 and up) reshuffling the responsibility for where
      to unlink the Unix socket files.  Lock file unlinking can stay where it
      is, but teach it to unlink the lock files in reverse order of creation.
      d73d14c2
    • Heikki Linnakangas's avatar
      Fix race condition that lead to WALInsertLock deadlock with commit_delay. · 358cde32
      Heikki Linnakangas authored
      If a call to WaitForXLogInsertionsToFinish() returned a value in the middle
      of a page, and another backend then started to insert a record to the same
      page, and then you called WaitXLogInsertionsToFinish() again, the second
      call might return a smaller value than the first call. The problem was in
      GetXLogBuffer(), which always updated the insertingAt value to the
      beginning of the requested page, not the actual requested location. Because
      of that, the second call might return a xlog pointer to the beginning of
      the page, while the first one returned a later position on the same page.
      XLogFlush() performs two calls to WaitXLogInsertionsToFinish() in
      succession, and holds WALWriteLock on the second call, which can deadlock
      if the second call to WaitXLogInsertionsToFinish() blocks.
      
      Reported by Spiros Ioannou. Backpatch to 9.4, where the more scalable
      WALInsertLock mechanism, and this bug, was introduced.
      358cde32
    • Andres Freund's avatar
      Micro optimize LWLockAttemptLock() a bit. · a4b09af3
      Andres Freund authored
      LWLockAttemptLock pointlessly read the lock's state in every loop
      iteration, even though pg_atomic_compare_exchange_u32() returns the old
      value. Instead do that only once before the loop iteration.
      
      Additionally there's no need to have the expected_state variable,
      old_state mostly had the same value anyway.
      
      Noticed-By: Heikki Linnakangas
      Backpatch: 9.5, no reason to let the branches diverge at this point
      a4b09af3
    • Andres Freund's avatar
      Fix issues around the "variable" support in the lwlock infrastructure. · 70397601
      Andres Freund authored
      The lwlock scalability work introduced two race conditions into the
      lwlock variable support provided for xlog.c. First, and harmlessly on
      most platforms, it set/read the variable without the spinlock in some
      places. Secondly, due to the removal of the spinlock, it was possible
      that a backend missed changes to the variable's state if it changed in
      the wrong moment because checking the lock's state, the variable's state
      and the queuing are not protected by a single spinlock acquisition
      anymore.
      
      To fix first move resetting the variable's from LWLockAcquireWithVar to
      WALInsertLockRelease, via a new function LWLockReleaseClearVar. That
      prevents issues around waiting for a variable's value to change when a
      new locker has acquired the lock, but not yet set the value. Secondly
      re-check that the variable hasn't changed after enqueing, that prevents
      the issue that the lock has been released and already re-acquired by the
      time the woken up backend checks for the lock's state.
      
      Reported-By: Jeff Janes
      Analyzed-By: Heikki Linnakangas
      Reviewed-By: Heikki Linnakangas
      Discussion: 5592DB35.2060401@iki.fi
      Backpatch: 9.5, where the lwlock scalability went in
      70397601
    • Tom Lane's avatar
      Fix some planner issues with degenerate outer join clauses. · f69b4b94
      Tom Lane authored
      An outer join clause that didn't actually reference the RHS (perhaps only
      after constant-folding) could confuse the join order enforcement logic,
      leading to wrong query results.  Also, nested occurrences of such things
      could trigger an Assertion that on reflection seems incorrect.
      
      Per fuzz testing by Andreas Seltenreich.  The practical use of such cases
      seems thin enough that it's not too surprising we've not heard field
      reports about it.
      
      This has been broken for a long time, so back-patch to all active branches.
      f69b4b94
  3. 01 Aug, 2015 1 commit
    • Tom Lane's avatar
      Teach predtest.c that "foo" implies "foo IS NOT NULL". · dea1491f
      Tom Lane authored
      Per complaint from Peter Holzer.  It's useful to cover this special case,
      since for a boolean variable "foo", earlier parts of the planner will have
      reduced variants like "foo = true" to just "foo", and thus we may fail
      to recognize the applicability of a partial index with predicate
      "foo IS NOT NULL".
      
      Back-patch to 9.5, but not further; given the lack of previous complaints
      this doesn't seem like behavior to change in stable branches.
      dea1491f
  4. 31 Jul, 2015 3 commits
    • Tom Lane's avatar
      Fix an oversight in checking whether a join with LATERAL refs is legal. · a6492ff8
      Tom Lane authored
      In many cases, we can implement a semijoin as a plain innerjoin by first
      passing the righthand-side relation through a unique-ification step.
      However, one of the cases where this does NOT work is where the RHS has
      a LATERAL reference to the LHS; that makes the RHS dependent on the LHS
      so that unique-ification is meaningless.  joinpath.c understood this,
      and so would not generate any join paths of this kind ... but join_is_legal
      neglected to check for the case, so it would think that we could do it.
      The upshot would be a "could not devise a query plan for the given query"
      failure once we had failed to generate any join paths at all for the bogus
      join pair.
      
      Back-patch to 9.3 where LATERAL was added.
      a6492ff8
    • Noah Misch's avatar
      Clean up Makefile.win32 "-I" flag additions. · 16c4e6d8
      Noah Misch authored
      The PGXS-case directory does not exist in the non-PGXS case, and vice
      versa.  Add one or the other, not both.  This is essentially cosmetic.
      It makes Makefile.win32 more like the similar Makefile.global code.
      16c4e6d8
    • Noah Misch's avatar
      Consolidate makefile code for setting top_srcdir, srcdir and VPATH. · 5da944fb
      Noah Misch authored
      Responsibility was formerly split between Makefile.global and pgxs.mk.
      As a result of commit b58233c7, in the
      PGXS case, these variables were unset while parsing Makefile.global and
      callees.  Inclusion of Makefile.custom did not work from PGXS, and the
      subtle difference seemed like a recipe for future bugs.  Back-patch to
      9.4, where that commit first appeared.
      5da944fb
  5. 30 Jul, 2015 13 commits
    • Alvaro Herrera's avatar
      Fix volatility marking of commit timestamp functions · e8e86fbc
      Alvaro Herrera authored
      They are marked stable, but since they act on instantaneous state and it
      is possible to consult state of transactions as they commit, the results
      could change mid-query.  They need to be marked volatile, and this
      commit does so.
      
      There would normally be a catversion bump here, but this is so much a
      niche feature and I don't believe there's real damage from the incorrect
      marking, that I refrained.
      
      Backpatch to 9.5, where commit timestamps where introduced.
      
      Per note from Fujii Masao.
      e8e86fbc
    • Alvaro Herrera's avatar
      Fix broken assertion in BRIN code · c8127624
      Alvaro Herrera authored
      The code was assuming that any NULL value in scan keys was due to IS
      NULL or IS NOT NULL, but it turns out to be possible to get them with
      other operators too, if they are used in contrived-enough ways.  Easiest
      way out of the problem seems to check explicitely for the IS NOT NULL
      flag, instead of assuming it must be set if the IS NULL flag is not set,
      when a null scan key is found; if neither flag is set, follow the lead
      of other index AMs and assume that all indexable operators must be
      strict, and thus the query is never satisfiable.
      
      Also, add a comment to try and lure some future hacker into improving
      analysis of scan keys in brin.
      
      Per report from Andreas Seltenreich; diagnosis by Tom Lane.
      Backpatch to 9.5.
      
      Discussion: http://www.postgresql.org/message-id/20646.1437919632@sss.pgh.pa.us
      c8127624
    • Joe Conway's avatar
      Improve CREATE FUNCTION doc WRT to LEAKPROOF RLS interaction. · d6314b20
      Joe Conway authored
      Patch by Dean Rasheed. Back-patched to 9.5 where RLS was introduced.
      d6314b20
    • Joe Conway's avatar
      Use appropriate command type when retrieving relation's policies. · 1e15b212
      Joe Conway authored
      When retrieving policies, if not working on the root target relation,
      we actually want the relation's SELECT policies, regardless of
      the top level query command type. For example in UPDATE t1...FROM t2
      we need to apply t1's UPDATE policies and t2's SELECT policies.
      Previously top level query command type was applied to all relations,
      which was wrong. Add some regression coverage to ensure we don't
      violate this principle in the future.
      
      Report and patch by Dean Rasheed. Cherry picked from larger refactoring
      patch and tweaked by me. Back-patched to 9.5 where RLS was introduced.
      1e15b212
    • Tom Lane's avatar
      Avoid some zero-divide hazards in the planner. · 8693ebe3
      Tom Lane authored
      Although I think on all modern machines floating division by zero
      results in Infinity not SIGFPE, we still don't want infinities
      running around in the planner's costing estimates; too much risk
      of that leading to insane behavior.
      
      grouping_planner() failed to consider the possibility that final_rel
      might be known dummy and hence have zero rowcount.  (I wonder if it
      would be better to set a rows estimate of 1 for dummy relations?
      But at least in the back branches, changing this convention seems
      like a bad idea, so I'll leave that for another day.)
      
      Make certain that get_variable_numdistinct() produces a nonzero result.
      The case that can be shown to be broken is with stadistinct < 0.0 and
      small ntuples; we did not prevent the result from rounding to zero.
      For good luck I applied clamp_row_est() to all the nonconstant return
      values.
      
      In ExecChooseHashTableSize(), Assert that we compute positive nbuckets
      and nbatch.  I know of no reason to think this isn't the case, but it
      seems like a good safety check.
      
      Per reports from Piotr Stefaniak.  Back-patch to all active branches.
      8693ebe3
    • Heikki Linnakangas's avatar
      Fix calculation of latency of pgbench backslash commands. · 5515ec0b
      Heikki Linnakangas authored
      When we loop back to the top of doCustom after processing a backslash
      command, we must reset the "now" timestamp, because that's used to
      calculate the time spent executing the previous command.
      
      Report and fix by Fabien Coelho. Backpatch to 9.5, where this was broken.
      5515ec0b
    • Heikki Linnakangas's avatar
      Update ax_pthread.m4 to an experimental draft version from upstream. · a2932283
      Heikki Linnakangas authored
      The current version is adding a spurious -pthread option on some Darwin
      systems that don't need it, which leads to a bunch of "unrecognized option
      '-pthread'" warnings. There is a proposed fix for that in the upstream
      autoconf archive's bug tracker, see https://savannah.gnu.org/patch/?8186.
      This commit updates our version of ax_pthread.m4 to the "draft2" version
      proposed there by Daniel Richard G. I'm using our buildfarm to help Daniel
      to test this, before he commits this to the upstream repository.
      a2932283
    • Noah Misch's avatar
      Blacklist xlc 32-bit inlining. · c53f7387
      Noah Misch authored
      Per a suggestion from Tom Lane.  Back-patch to 9.0 (all supported
      versions).  While only 9.4 and up have code known to elicit this
      compiler bug, we were disabling inlining by accident until commit
      43d89a23.
      c53f7387
    • Noah Misch's avatar
      Remove redundant "make install" from pg_upgrade test suite. · 021a5698
      Noah Misch authored
      A top-level "make install" includes pg_upgrade since commit
      9fa8b0ee.  Back-patch to 9.5, where that
      commit first appeared.
      021a5698
    • Noah Misch's avatar
      MSVC: Revert most 9.5 changes to pre-9.5 vcregress.pl tests. · e6ea46c3
      Noah Misch authored
      The reverted changes did not narrow the semantic gap between the MSVC
      build system and the GNU make build system.  For targets old and new
      that run multiple suites (contribcheck, modulescheck, tapcheck), restore
      vcregress.pl to mimicking "make -k" rather than the "make -S" default.
      Lack of "-k" would be more burdensome than lack of "-S".  Keep changes
      reflecting contemporary changes to the GNU make build system, and keep
      updates to Makefile parsing.  Keep the loss of --psqldir in "check" and
      "ecpgcheck" targets; it had been a no-op when used alongside
      --temp-install.  No log message mentioned any of the reverted changes.
      Based on a germ by Michael Paquier.  Back-patch to 9.5.
      e6ea46c3
    • Noah Misch's avatar
      MSVC: Remove duplicate PATH entry in test harness. · d6ab1467
      Noah Misch authored
      Back-patch to 9.5, where commit 4cb7d671
      introduced it.
      d6ab1467
    • Noah Misch's avatar
      MSVC: Future-proof installation file skip logic. · d6925228
      Noah Misch authored
      This code relied on knowing exactly where in the source tree temporary
      installations might appear.  A reasonable hacker may not think to update
      this code when adding use of a temporary installation, making it
      fragile.  Observe that commit 9fa8b0ee
      broke it unnoticed, and commit dcae5fac
      fixed it unnoticed.  Back-patch to 9.5 only; use of temporary
      installations is unlikely to change in released versions.
      d6925228
    • Andrew Dunstan's avatar
      Add IF NOT EXISTS processing to ALTER TABLE ADD COLUMN · 2cd40adb
      Andrew Dunstan authored
      Fabrízio de Royes Mello, reviewed by Payal Singh, Alvaro Herrera and
      Michael Paquier.
      2cd40adb
  6. 29 Jul, 2015 10 commits
    • Joe Conway's avatar
      Create new ParseExprKind for use by policy expressions. · 632cd9f8
      Joe Conway authored
      Policy USING and WITH CHECK expressions were using EXPR_KIND_WHERE for
      parse analysis, which results in inappropriate ERROR messages when
      the expression contains unsupported constructs such as aggregates.
      Create a new ParseExprKind called EXPR_KIND_POLICY and tailor the
      related messages to fit.
      
      Reported by Noah Misch. Reviewed by Dean Rasheed, Alvaro Herrera,
      and Robert Haas. Back-patch to 9.5 where RLS was introduced.
      632cd9f8
    • Robert Haas's avatar
      Fix incorrect comment. · f04ce314
      Robert Haas authored
      Amit Langote
      f04ce314
    • Robert Haas's avatar
      Flesh out the background worker documentation. · 38d4ce6b
      Robert Haas authored
      Make it more clear that bgw_main is usually not what you want.  Put the
      background worker flags in a variablelist rather than having them as
      part of a paragraph.  Explain important limits on how bgw_main_arg can
      be used.
      
      Craig Ringer, substantially revised by me.
      38d4ce6b
    • Heikki Linnakangas's avatar
      Fix TAP tests with "make installcheck". · c7f0b28c
      Heikki Linnakangas authored
      I neglected that the prove_installcheck rule also needs to also define
      PG_REGRESS, like prove_check does.
      c7f0b28c
    • Tom Lane's avatar
      Add some test coverage of EvalPlanQual with non-locked tables. · 342a1ffa
      Tom Lane authored
      A Salesforce colleague of mine griped that the regression tests don't
      exercise EvalPlanQualFetchRowMarks() and allied routines.  Which is
      a fair complaint.  Add test cases that go through the REFERENCE and COPY
      code paths.  Unfortunately we don't have sufficient infrastructure right
      now to exercise the FDW code path in the isolation tests, but this is
      surely better than before.
      342a1ffa
    • Joe Conway's avatar
      Add missing post create and alter hooks to policy objects. · efe72a82
      Joe Conway authored
      AlterPolicy() and CreatePolicy() lacked their respective hook invocations.
      Noted by Noah Misch, review by Dean Rasheed. Back-patch to 9.5 where
      RLS was introduced.
      efe72a82
    • Heikki Linnakangas's avatar
      Make TAP tests work on Windows. · 13d856e1
      Heikki Linnakangas authored
      On Windows, use listen_address=127.0.0.1 to allow TCP connections. We were
      already using "pg_regress --config-auth" to set up HBA appropriately. The
      standard_initdb helper function now sets up the server's
      unix_socket_directories or listen_addresses in the config file, so that
      they don't need to be specified in the pg_ctl command line anymore. That
      way, the pg_ctl invocations in test programs don't need to differ between
      Windows and Unix.
      
      Add another helper function to configure the server's pg_hba.conf to allow
      replication connections. The configuration is done similarly to "pg_regress
      --config-auth": trust on domain sockets on Unix, and SSPI authentication on
      Windows.
      
      Replace calls to "cat" and "touch" programs with built-in perl code, as
      those programs don't normally exist on Windows.
      
      Add instructions in the docs on how to install IPC::Run on Windows. Adjust
      vcregress.pl to not replace PERL5LIB completely in vcregress.pl, because
      otherwise cannot install IPC::Run in a non-standard location easily.
      
      Michael Paquier, reviewed by Noah Misch, some additional tweaking by me.
      13d856e1
    • Robert Haas's avatar
      Document how to build the docs using the website style. · 5f106607
      Robert Haas authored
      Craig Ringer
      5f106607
    • Andres Freund's avatar
      Remove outdated comment in LWLockDequeueSelf's header. · 3bc9356d
      Andres Freund authored
      Noticed-By: Robert Haas
      Backpatch: 9.5, where the function was added
      3bc9356d
    • Heikki Linnakangas's avatar
      Fix typo in comment. · a309ebd6
      Heikki Linnakangas authored
      Amit Langote
      a309ebd6