1. 12 Feb, 2016 15 commits
    • Bruce Momjian's avatar
      pg_upgrade: Add C comment about NextXID delimiter · 13a6fa36
      Bruce Momjian authored
      We don't test the catversion for the NextXID delimiter change, we just
      test the string contents;  explain why.
      
      Reported-by: Michael Paquier
      13a6fa36
    • Joe Conway's avatar
      Change delimiter used for display of NextXID · 59a884e9
      Joe Conway authored
      NextXID has been rendered in the form of a pg_lsn even though it
      really is not. This can cause confusion, so change the format from
      %u/%u to %u:%u, per discussion on hackers.
      
      Complaint by me, patch by me and Bruce, reviewed by Michael Paquier
      and Alvaro. Applied to HEAD only.
      
      Author: Joe Conway, Bruce Momjian
      Reviewed-by: Michael Paquier, Alvaro Herrera
      Backpatch-through: master
      59a884e9
    • Tom Lane's avatar
      Increase deadlock_timeout some more in the deadlock-hard isolation test. · e84e06d2
      Tom Lane authored
      The previous value of 5s is inadequate for the buildfarm's
      CLOBBER_CACHE_ALWAYS animals: they take long enough to do the is-it-waiting
      queries that the timeout expires, allowing the database state to change,
      before isolationtester is done looking.  Perhaps 10s will be enough.
      (If it isn't, I'm inclined to reduce the number of sessions involved.)
      e84e06d2
    • Tom Lane's avatar
      Revert "isolationtester: don't repeat the is-it-waiting query when retrying a step." · dca36932
      Tom Lane authored
      This mostly reverts commit 9c9782f0.
      I left in the parts that rearranged removal of completed waiting steps;
      but the idea of not rechecking a step's blocked-ness isn't working.
      dca36932
    • Tom Lane's avatar
      Revert "Still further tweaking of deadlock isolation tests." · 3992188c
      Tom Lane authored
      This reverts commit d03130d3.
      That was dependent on an isolationtester.c change that now proves
      to be broken; we will need to find another solution.
      3992188c
    • Alvaro Herrera's avatar
      pgbench: cleanup use of a "logfile" parameter · 34f13cc4
      Alvaro Herrera authored
      There is no reason to have the per-thread logfile file pointer as a
      separate parameter in various functions: it's much simpler to put it in
      the per-thread state struct instead, which is already being passed to
      all functions that need the log file anyway.  Change the callsites in
      which it was used as a boolean to test whether logging is active, so
      that they use the use_log global variable instead.
      
      No backpatch, even though this exists since commit a887c486 of March
      2010, because this is just for cleanliness' sake and the surrounding
      code has been modified a lot recently anyway.
      34f13cc4
    • Alvaro Herrera's avatar
      pgbench: fix segfault with empty sql file · db94419f
      Alvaro Herrera authored
      Commit 1d0c3b3f introduced a bug that causes pgbench to crash if an
      empty script file is specified.  Fix it by rejecting such files at
      startup, which is the historical and intended behavior.
      
      Reported-By: Jeff Janes
      Discussion: https://www.postgresql.org/message-id/CAMkU=1zxKUbLPOt9hQWFp14pTc=V0cGo2GQBbn2GsK2Pu+8ZfA@mail.gmail.com
      db94419f
    • Tom Lane's avatar
      Still further tweaking of deadlock isolation tests. · d03130d3
      Tom Lane authored
      It turns out that there is a second race condition in the new deadlock-hard
      test: once the deadlock detector fires, it's uncertain whether step s7a8 or
      step s8a1 will report first, because killing s8's transaction unblocks s7.
      So far, s7 has only been seen to report first in CLOBBER_CACHE_ALWAYS
      builds, but it's pretty reproducible there, and in theory it should
      sometimes occur in normal builds too.  If s7 were a bit slower than usual,
      that could also break the test, since the existing expected-file assumes
      that we'll see s7a8 report the first time we check it after s8a1 completes.
      To fix, add a post-lock delay to s7a8.
      d03130d3
    • Tom Lane's avatar
      isolationtester: don't repeat the is-it-waiting query when retrying a step. · 9c9782f0
      Tom Lane authored
      If we're retrying a step, then we already decided it was blocked on a lock,
      and there's no need to recheck that.  The original coding of commit
      38f8bdca resulted in a large number of
      is-it-waiting queries when dealing with multiple concurrently-blocked
      sessions, which is fairly pointless and also results in test failures in
      CLOBBER_CACHE_ALWAYS builds, where the is-it-waiting query is quite slow.
      
      This definition also permits appending pg_sleep() calls to steps where it's
      needed to control the order of finish of concurrent steps.  Before, that
      did not work nicely because we'd decide that a step performing a sleep was
      not blocked and hang up waiting for it to finish, rather than noticing the
      completion of the concurrent step we're supposed to notice first.
      
      In passing, revise handling of removal of completed waiting steps
      to make it a bit less messy.
      9c9782f0
    • Tom Lane's avatar
      Re-pgindent isolationtester.c. · a3614908
      Tom Lane authored
      Need to do some more hacking on this, and got annoyed that it's not
      indent clean.
      a3614908
    • Peter Eisentraut's avatar
      Fix whitespace · 29b4b7bd
      Peter Eisentraut authored
      29b4b7bd
    • Tom Lane's avatar
      Add missing "static" qualifier. · 99a9d6d5
      Tom Lane authored
      Per buildfarm member pademelon.
      99a9d6d5
    • Robert Haas's avatar
      Introduce extensible node types. · bcac23de
      Robert Haas authored
      An extensible node is always tagged T_Extensible, but the extnodename
      field identifies it more specifically; it may also include arbitrary
      private data.  Extensible nodes can be copied, tested for equality,
      serialized, and deserialized, but the core system doesn't know
      anything about them otherwise.  Some extensions may find it useful to
      include these nodes in fdw_private or custom_private lists in lieu of
      arm-wrestling their data into a format that the core code can
      understand.
      
      Along the way, so as not to burden the authors of such extensible
      node types too much, expose the functions for writing serialized
      tokens, and for serializing and deserializing bitmapsets.
      
      KaiGai Kohei, per a design suggested by me.  Reviewed by Andres Freund
      and by me, and further edited by me.
      bcac23de
    • Robert Haas's avatar
      Make builtin lwlock tranche names consistent. · 63461a63
      Robert Haas authored
      Previously, we had a mix of styles.
      
      Amit Kapila
      63461a63
    • Tom Lane's avatar
      Further tweaking of deadlock isolation tests. · caefc11e
      Tom Lane authored
      The new deadlock-soft-2 test has a timing dependency too: it supposes
      that isolationtester will detect step s1b as waiting before the deadlock
      detector runs and grants it the lock.  Adjust deadlock_timeout to ensure
      that that's true even in CLOBBER_CACHE_ALWAYS builds, where the wait
      detection query is quite slow.  Per buildfarm member jaguarundi.
      caefc11e
  2. 11 Feb, 2016 16 commits
    • Tom Lane's avatar
      Refactor check_functional_grouping() to use get_primary_key_attnos(). · f144f732
      Tom Lane authored
      If we ever get around to allowing functional dependency to be proven
      from other things besides simple primary keys, this code will need to
      be rethought, but that was true anyway.  In the meantime, we might as
      well not have two very-similar routines for scanning pg_constraint.
      
      David Rowley, reviewed by Julien Rouhaud
      f144f732
    • Tom Lane's avatar
      Remove GROUP BY columns that are functionally dependent on other columns. · d4c3a156
      Tom Lane authored
      If a GROUP BY clause includes all columns of a non-deferred primary key,
      as well as other columns of the same relation, those other columns are
      redundant and can be dropped from the grouping; the pkey is enough to
      ensure that each row of the table corresponds to a separate group.
      Getting rid of the excess columns will reduce the cost of the sorting or
      hashing needed to implement GROUP BY, and can indeed remove the need for
      a sort step altogether.
      
      This seems worth testing for since many query authors are not aware of
      the GROUP-BY-primary-key exception to the rule about queries not being
      allowed to reference non-grouped-by columns in their targetlists or
      HAVING clauses.  Thus, redundant GROUP BY items are not uncommon.  Also,
      we can make the test pretty cheap in most queries where it won't help
      by not looking up a rel's primary key until we've found that at least
      two of its columns are in GROUP BY.
      
      David Rowley, reviewed by Julien Rouhaud
      d4c3a156
    • Tom Lane's avatar
      Move pg_constraint.h function declarations to new file pg_constraint_fn.h. · 72eee410
      Tom Lane authored
      A pending patch requires exporting a function returning Bitmapset from
      catalog/pg_constraint.c.  As things stand, that would mean including
      nodes/bitmapset.h in pg_constraint.h, which might be hazardous for the
      client-side includability of that header.  It's not entirely clear whether
      any client-side code needs to include pg_constraint.h, but it seems prudent
      to assume that there is some such code somewhere.  Therefore, split off the
      function definitions into a new file pg_constraint_fn.h, similarly to what
      we've done for some other catalog header files.
      72eee410
    • Tom Lane's avatar
      Fix typo in comment. · 2564be36
      Tom Lane authored
      2564be36
    • Tom Lane's avatar
      Shift the responsibility for emitting "database system is shut down". · d18643c4
      Tom Lane authored
      Historically this message has been emitted at the end of ShutdownXLOG().
      That's not an insane place for it in a standalone backend, but in the
      postmaster environment we've grown a fair amount of stuff that happens
      later, including archiver/walsender shutdown, stats collector shutdown,
      etc.  Recent buildfarm experimentation showed that on slower machines
      there could be many seconds' delay between finishing ShutdownXLOG() and
      actual postmaster exit.  That's fairly confusing, both for testing
      purposes and for DBAs.  Hence, move the code that prints this message
      into UnlinkLockFiles(), so that it comes out just after we remove the
      postmaster's pidfile.  That is a more appropriate definition of "is shut
      down" from the point of view of "pg_ctl stop", for example.  In general,
      removing the pidfile should be the last externally-visible action of
      either a postmaster or a standalone backend; compare commit
      d73d14c2 for instance.  So this seems
      like a reasonably future-proof approach.
      d18643c4
    • Robert Haas's avatar
      Use separate lwlock tranches for buffer, lock, and predicate lock managers. · c319991b
      Robert Haas authored
      This finishes the work - spread across many commits over the last
      several months - of putting each type of lock other than the named
      individual locks into a separate tranche.
      
      Amit Kapila
      c319991b
    • Tom Lane's avatar
      Make new deadlock isolation test more reproducible. · b11d07b6
      Tom Lane authored
      The original formulation of 4c9864b9
      was extremely timing-sensitive, because it arranged for the deadlock
      detector to be running (and possibly unblocking the current query)
      at almost exactly the same time as isolationtester would be probing
      to see if the query is blocked.  The committed expected-file assumed
      that the deadlock detection would finish first, but we see the opposite
      on both fast and slow buildfarm animals.  Adjust the deadlock timeout
      settings to make it predictable that isolationtester *will* see the
      query as waiting before deadlock detection unblocks it.
      
      I used a 5s timeout for the same reasons mentioned in
      a7921f71.
      b11d07b6
    • Tom Lane's avatar
      Code review for isolationtester changes. · d9dc2b41
      Tom Lane authored
      Fix a few oversights in 38f8bdca:
      don't leak memory in run_permutation(), remember when we've issued
      a cancel rather than issuing another one every 10ms,
      fix some typos in comments.
      d9dc2b41
    • Teodor Sigaev's avatar
      Improve error reporting in format() · 07d25a96
      Teodor Sigaev authored
      Clarify invalid format conversion type error message and add hint.
      
      Author: Jim Nasby
      07d25a96
    • Robert Haas's avatar
      Rename PGPROC fields related to group XID clearing again. · a455878d
      Robert Haas authored
      Commit 0e141c0f introduced a new
      facility to reduce ProcArrayLock contention by clearing several XIDs
      from the ProcArray under a single lock acquisition.  The names
      initially chosen were deemed not to be very good choices, so commit
      4aec4989 renamed them.  But now it
      seems like we still didn't get it right.  A pending patch wants to
      add similar infrastructure for batching CLOG updates, so the names
      need to be clear enough to allow a new set of structure members with
      a related purpose.
      
      Amit Kapila
      a455878d
    • Robert Haas's avatar
      Add some isolation tests for deadlock detection and resolution. · 4c9864b9
      Robert Haas authored
      Previously, we had no test coverage for the deadlock detector.
      4c9864b9
    • Robert Haas's avatar
      Modify the isolation tester so that multiple sessions can wait. · 38f8bdca
      Robert Haas authored
      This allows testing of deadlock scenarios.  Scenarios that would
      previously have been considered invalid are now simply taken as a
      scenario in which more than one backend will wait.
      38f8bdca
    • Robert Haas's avatar
      Specify permutations for isolation tests with "invalid" permutations. · c9882c60
      Robert Haas authored
      This is a necessary prerequisite for forthcoming changes to allow deadlock
      scenarios to be tested by the isolation tester.  It is also a good idea on
      general principle, since these scenarios add no useful test coverage not
      provided by other scenarios, but do to take time to execute.
      c9882c60
    • Noah Misch's avatar
      In pg_rewind test suite, triple promote timeout to 90s. · 64d89a93
      Noah Misch authored
      Thirty seconds was not consistently enough for promotion to complete on
      buildfarm members sungazer and tern.  Experiments suggest 43s would have
      been enough.  Back-patch to 9.5, where pg_rewind was introduced.
      64d89a93
    • Noah Misch's avatar
      Accept pg_ctl timeout from the PGCTLTIMEOUT environment variable. · 2ffa8696
      Noah Misch authored
      Many automated test suites call pg_ctl.  Buildfarm members axolotl,
      hornet, mandrill, shearwater, sungazer and tern have failed when server
      shutdown took longer than the pg_ctl default 60s timeout.  This addition
      permits slow hosts to easily raise the timeout without us editing a
      --timeout argument into every test suite pg_ctl call.  Back-patch to 9.1
      (all supported versions) for the sake of automated testing.
      
      Reviewed by Tom Lane.
      2ffa8696
    • Tom Lane's avatar
      Avoid use of sscanf() to parse ispell dictionary files. · 51e78ab4
      Tom Lane authored
      It turns out that on FreeBSD-derived platforms (including OS X), the
      *scanf() family of functions is pretty much brain-dead about multibyte
      characters.  In particular it will apply isspace() to individual bytes
      of input even when those bytes are part of a multibyte character, thus
      allowing false recognition of a field-terminating space.
      
      We appear to have little alternative other than instituting a coding
      rule that *scanf() is not to be used if the input string might contain
      multibyte characters.  (There was some discussion of relying on "%ls",
      but that probably just moves the portability problem somewhere else,
      and besides it doesn't fully prevent BSD *scanf() from using isspace().)
      
      This patch is a down payment on that: it gets rid of use of sscanf()
      to parse ispell dictionary files, which are certainly at great risk
      of having a problem.  The code is cleaner this way anyway, though
      a bit longer.
      
      In passing, improve a few comments.
      
      Report and patch by Artur Zakirov, reviewed and somewhat tweaked by me.
      Back-patch to all supported branches.
      51e78ab4
  3. 10 Feb, 2016 4 commits
  4. 09 Feb, 2016 3 commits
    • Robert Haas's avatar
      postgres_fdw: Remove unstable regression test. · bb4df42e
      Robert Haas authored
      Per Tom Lane and the buildfarm.
      bb4df42e
    • Robert Haas's avatar
      postgres_fdw: Push down joins to remote servers. · e4106b25
      Robert Haas authored
      If we've got a relatively straightforward join between two tables,
      this pushes that join down to the remote server instead of fetching
      the rows for each table and performing the join locally.  Some cases
      are not handled yet, such as SEMI and ANTI joins.  Also, we don't
      yet attempt to create presorted join paths or parameterized join
      paths even though these options do get tried for a base relation
      scan.  Nevertheless, this seems likely to be a very significant win
      in many practical cases.
      
      Shigeru Hanada and Ashutosh Bapat, reviewed by Robert Haas, with
      additional review at various points by Tom Lane, Etsuro Fujita,
      KaiGai Kohei, and Jeevan Chalke.
      e4106b25
    • Tom Lane's avatar
      Add more chattiness in server shutdown. · 7351e182
      Tom Lane authored
      Early returns from the buildfarm show that there's a bit of a gap in the
      logging I added in 3971f648: the portion of CreateCheckPoint()
      after CheckPointGuts() can take a fair amount of time.  Add a few more
      log messages in that section of code.  This too shall be reverted later.
      7351e182
  5. 08 Feb, 2016 2 commits
    • Tom Lane's avatar
      Temporarily make pg_ctl and server shutdown a whole lot chattier. · 3971f648
      Tom Lane authored
      This is a quick hack, due to be reverted when its purpose has been served,
      to try to gather information about why some of the buildfarm critters
      regularly fail with "postmaster does not shut down" complaints.  Maybe they
      are just really overloaded, but maybe something else is going on.  Hence,
      instrument pg_ctl to print the current time when it starts waiting for
      postmaster shutdown and when it gives up, and add a lot of logging of the
      current time in the server's checkpoint and shutdown code paths.
      
      No attempt has been made to make this pretty.  I'm not even totally sure
      if it will build on Windows, but we'll soon find out.
      3971f648
    • Tom Lane's avatar
      Re-pgindent varlena.c. · 0231f838
      Tom Lane authored
      Just to make sure previous commit worked ...
      0231f838