1. 12 Feb, 2016 3 commits
    • 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 8 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
    • Tom Lane's avatar
      Rename typedef "string" to "VarString". · 58e79721
      Tom Lane authored
      Since pgindent treats typedef names as global, the original coding of
      b47b4dbf would have had rather nasty effects on the formatting
      of other files in which "string" is used as a variable or field name.
      Use a less generic name for this typedef, and rename some other
      identifiers to match.
      
      Peter Geoghegan, per gripe from me
      58e79721
    • Tom Lane's avatar
      Use %u not %d to print OIDs. · 63828969
      Tom Lane authored
      Oversight in commit 96198d94.
      
      Etsuro Fujita
      63828969
    • Tom Lane's avatar
      Last-minute updates for release notes. · 02292845
      Tom Lane authored
      Security: CVE-2016-0773
      02292845
    • Tom Lane's avatar
      Fix some regex issues with out-of-range characters and large char ranges. · 3bb3f42f
      Tom Lane authored
      Previously, our regex code defined CHR_MAX as 0xfffffffe, which is a
      bad choice because it is outside the range of type "celt" (int32).
      Characters approaching that limit could lead to infinite loops in logic
      such as "for (c = a; c <= b; c++)" where c is of type celt but the
      range bounds are chr.  Such loops will work safely only if CHR_MAX+1
      is representable in celt, since c must advance to beyond b before the
      loop will exit.
      
      Fortunately, there seems no reason not to restrict CHR_MAX to 0x7ffffffe.
      It's highly unlikely that Unicode will ever assign codes that high, and
      none of our other backend encodings need characters beyond that either.
      
      In addition to modifying the macro, we have to explicitly enforce character
      range restrictions on the values of \u, \U, and \x escape sequences, else
      the limit is trivially bypassed.
      
      Also, the code for expanding case-independent character ranges in bracket
      expressions had a potential integer overflow in its calculation of the
      number of characters it could generate, which could lead to allocating too
      small a character vector and then overwriting memory.  An attacker with the
      ability to supply arbitrary regex patterns could easily cause transient DOS
      via server crashes, and the possibility for privilege escalation has not
      been ruled out.
      
      Quite aside from the integer-overflow problem, the range expansion code was
      unnecessarily inefficient in that it always produced a result consisting of
      individual characters, abandoning the knowledge that we had a range to
      start with.  If the input range is large, this requires excessive memory.
      Change it so that the original range is reported as-is, and then we add on
      any case-equivalent characters that are outside that range.  With this
      approach, we can bound the number of individual characters allowed without
      sacrificing much.  This patch allows at most 100000 individual characters,
      which I believe to be more than the number of case pairs existing in
      Unicode, so that the restriction will never be hit in practice.
      
      It's still possible for range() to take awhile given a large character code
      range, so also add statement-cancel detection to its loop.  The downstream
      function dovec() also lacked cancel detection, and could take a long time
      given a large output from range().
      
      Per fuzz testing by Greg Stark.  Back-patch to all supported branches.
      
      Security: CVE-2016-0773
      3bb3f42f
    • Fujii Masao's avatar
      Make GIN regression test stable. · f8a1c1d5
      Fujii Masao authored
      Commit 7f46eaf0 added the regression test which checks that
      gin_clean_pending_list() cleans up the GIN pending list and returns >0.
      This usually works fine. But if autovacuum comes along and cleans
      the list before gin_clean_pending_list() starts, the function may
      return 0, and then the regression test may fail.
      
      To fix the problem, this commit disables autovacuum on the target
      index of gin_clean_pending_list() by setting autovacuum_enabled
      reloption to off when creating the table.
      
      Also this commit sets gin_pending_list_limit reloption to 4MB on
      the target index. Otherwise when running "make installcheck" with
      small gin_pending_list_limit GUC, insertions of data may trigger
      the cleanup of pending list before gin_clean_pending_list() starts
      and the function may return 0. This could cause the regression test
      to fail.
      
      Per buildfarm member spoonbill.
      
      Reported-By: Tom Lane
      f8a1c1d5
    • Andres Freund's avatar
      Fix overeager pushdown of HAVING clauses when grouping sets are used. · a6897efa
      Andres Freund authored
      In 61444bfb we started to allow HAVING clauses to be fully pushed down
      into WHERE, even when grouping sets are in use. That turns out not to
      work correctly, because grouping sets can "produce" NULLs, meaning that
      filtering in WHERE and HAVING can have different results, even when no
      aggregates or volatile functions are involved.
      
      Instead only allow pushdown of empty grouping sets.
      
      It'd be nice to do better, but the exact mechanics of deciding which
      cases are safe are still being debated. It's important to give correct
      results till we find a good solution, and such a solution might not be
      appropriate for backpatching anyway.
      
      Bug: #13863
      Reported-By: 'wrb'
      Diagnosed-By: Dean Rasheed
      Author: Andrew Gierth
      Reviewed-By: Dean Rasheed and Andres Freund
      Discussion: 20160113183558.12989.56904@wrigleys.postgresql.org
      Backpatch: 9.5, where grouping sets were introduced
      a6897efa
  6. 07 Feb, 2016 6 commits
    • Tom Lane's avatar
      Improve documentation about PRIMARY KEY constraints. · c477e84f
      Tom Lane authored
      Get rid of the false implication that PRIMARY KEY is exactly equivalent to
      UNIQUE + NOT NULL.  That was more-or-less true at one time in our
      implementation, but the standard doesn't say that, and we've grown various
      features (many of them required by spec) that treat a pkey differently from
      less-formal constraints.  Per recent discussion on pgsql-general.
      
      I failed to resist the temptation to do some other wordsmithing in the
      same area.
      c477e84f
    • Tom Lane's avatar
      Fix deparsing of ON CONFLICT arbiter WHERE clauses. · cc2ca931
      Tom Lane authored
      The parser doesn't allow qualification of column names appearing in
      these clauses, but ruleutils.c would sometimes qualify them, leading
      to dump/reload failures.  Per bug #13891 from Onder Kalaci.
      
      (In passing, make stanzas in ruleutils.c that save/restore varprefix
      more consistent.)
      
      Peter Geoghegan
      cc2ca931
    • Tom Lane's avatar
      1d76c972
    • Tom Lane's avatar
      ExecHashRemoveNextSkewBucket must physically copy tuples to main hashtable. · f867ce55
      Tom Lane authored
      Commit 45f6240a added an assumption in ExecHashIncreaseNumBatches
      and ExecHashIncreaseNumBuckets that they could find all tuples in the main
      hash table by iterating over the "dense storage" introduced by that patch.
      However, ExecHashRemoveNextSkewBucket continued its old practice of simply
      re-linking deleted skew tuples into the main table's hashchains.  Hence,
      such tuples got lost during any subsequent increase in nbatch or nbuckets,
      and would never get joined, as reported in bug #13908 from Seth P.
      
      I (tgl) think that the aforesaid commit has got multiple design issues
      and should be reworked rather completely; but there is no time for that
      right now, so band-aid the problem by making ExecHashRemoveNextSkewBucket
      physically copy deleted skew tuples into the "dense storage" arena.
      
      The added test case is able to exhibit the problem by means of fooling the
      planner with a WHERE condition that it will underestimate the selectivity
      of, causing the initial nbatch estimate to be too small.
      
      Tomas Vondra and Tom Lane.  Thanks to David Johnston for initial
      investigation into the bug report.
      f867ce55
    • Robert Haas's avatar
      Fix parallel-safety markings for pg_upgrade functions. · d89f06f0
      Robert Haas authored
      These establish backend-local state which will not be copied to
      parallel workers, so they must be marked parallel-restricted, not
      parallel-safe.
      d89f06f0
    • Robert Haas's avatar
      Introduce a new GUC force_parallel_mode for testing purposes. · 7c944bd9
      Robert Haas authored
      When force_parallel_mode = true, we enable the parallel mode restrictions
      for all queries for which this is believed to be safe.  For the subset of
      those queries believed to be safe to run entirely within a worker, we spin
      up a worker and run the query there instead of running it in the
      original process.  When force_parallel_mode = regress, make additional
      changes to allow the regression tests to run cleanly even though parallel
      workers have been injected under the hood.
      
      Taken together, this facilitates both better user testing and better
      regression testing of the parallelism code.
      
      Robert Haas, with help from Amit Kapila and Rushabh Lathia.
      7c944bd9