1. 07 Mar, 2016 6 commits
    • Tom Lane's avatar
      Make the upper part of the planner work by generating and comparing Paths. · 3fc6e2d7
      Tom Lane authored
      I've been saying we needed to do this for more than five years, and here it
      finally is.  This patch removes the ever-growing tangle of spaghetti logic
      that grouping_planner() used to use to try to identify the best plan for
      post-scan/join query steps.  Now, there is (nearly) independent
      consideration of each execution step, and entirely separate construction of
      Paths to represent each of the possible ways to do that step.  We choose
      the best Path or set of Paths using the same add_path() logic that's been
      used inside query_planner() for years.
      
      In addition, this patch removes the old restriction that subquery_planner()
      could return only a single Plan.  It now returns a RelOptInfo containing a
      set of Paths, just as query_planner() does, and the parent query level can
      use each of those Paths as the basis of a SubqueryScanPath at its level.
      This allows finding some optimizations that we missed before, wherein a
      subquery was capable of returning presorted data and thereby avoiding a
      sort in the parent level, making the overall cost cheaper even though
      delivering sorted output was not the cheapest plan for the subquery in
      isolation.  (A couple of regression test outputs change in consequence of
      that.  However, there is very little change in visible planner behavior
      overall, because the point of this patch is not to get immediate planning
      benefits but to create the infrastructure for future improvements.)
      
      There is a great deal left to do here.  This patch unblocks a lot of
      planner work that was basically impractical in the old code structure,
      such as allowing FDWs to implement remote aggregation, or rewriting
      plan_set_operations() to allow consideration of multiple implementation
      orders for set operations.  (The latter will likely require a full
      rewrite of plan_set_operations(); what I've done here is only to fix it
      to return Paths not Plans.)  I have also left unfinished some localized
      refactoring in createplan.c and planner.c, because it was not necessary
      to get this patch to a working state.
      
      Thanks to Robert Haas, David Rowley, and Amit Kapila for review.
      3fc6e2d7
    • Tom Lane's avatar
      Fix backwards test for Windows service-ness in pg_ctl. · b642e50a
      Tom Lane authored
      A thinko in a9676139 caused pg_ctl to get it exactly backwards when
      deciding whether to report problems to the Windows eventlog or to stderr.
      Per bug #14001 from Manuel Mathar, who also identified the fix.
      Like the previous patch, back-patch to all supported branches.
      b642e50a
    • Tom Lane's avatar
      Re-fix broken definition for function name in pgbench's exprscan.l. · 94f1adcc
      Tom Lane authored
      Wups, my first try wasn't quite right either.  Too focused on fixing
      the existing bug, not enough on not introducing new ones.
      94f1adcc
    • Tom Lane's avatar
      Fix broken definition for function name in pgbench's exprscan.l. · 3899caf7
      Tom Lane authored
      As written, this would accept e.g. 123e9 as a function name.  Aside
      from being mildly astonishing, that would come back to haunt us if
      we ever try to add float constants to the expression syntax.  Insist
      that function names start with letters (or at least non-digits).
      
      In passing reset yyline as well as yycol when starting a new expression.
      This variable is useless since it's used nowhere, but if we're going
      to have it we should have it act sanely.
      3899caf7
    • Andres Freund's avatar
      Fix wrong allocation size in c8f621c4. · fd45d16f
      Andres Freund authored
      In c8f621c4 I forgot to account for MAXALIGN when allocating a new
      tuplebuf in ReorderBufferGetTupleBuf(). That happens to currently not
      cause active problems on a number of platforms because the affected
      pointer is already aligned, but others, like ppc and hppa, trigger this
      in the regression test, due to a debug memset clearing memory.
      
      Fix that.
      
      Backpatch: 9.4, like the previous commit.
      fd45d16f
    • Tom Lane's avatar
      Fix not-terribly-safe coding in NIImportOOAffixes() and NIImportAffixes(). · b3e05097
      Tom Lane authored
      There were two places in spell.c that supposed that they could search
      for a location in a string produced by lowerstr() and then transpose
      the offset into the original string.  But this fails completely if
      lowerstr() transforms any characters into characters of different byte
      length, as can happen in Turkish UTF8 for instance.
      
      We'd added some comments about this coding in commit 51e78ab4,
      but failed to realize that it was not merely confusing but wrong.
      
      Coverity complained about this code years ago, but in such an opaque
      fashion that nobody understood what it was on about.  I'm not entirely
      sure that this issue *is* what it's on about, actually, but perhaps
      this patch will shut it up -- and in any case the problem is clear.
      
      Back-patch to all supported branches.
      b3e05097
  2. 06 Mar, 2016 5 commits
    • Tom Lane's avatar
      Fix unportable usage of <ctype.h> functions. · cb0ca0c9
      Tom Lane authored
      isdigit(), isspace(), etc are likely to give surprising results if passed a
      signed char.  We should always cast the argument to unsigned char to avoid
      that.  Error in commit d78a7d9c, found by buildfarm member gaur.
      cb0ca0c9
    • Magnus Hagander's avatar
      Fix typos · 2b46259b
      Magnus Hagander authored
      Author: Guillaume Lelarge
      2b46259b
    • Andres Freund's avatar
      logical decoding: Fix handling of large old tuples with replica identity full. · c8f621c4
      Andres Freund authored
      When decoding the old version of an UPDATE or DELETE change, and if that
      tuple was bigger than MaxHeapTupleSize, we either Assert'ed out, or
      failed in more subtle ways in non-assert builds.  Normally individual
      tuples aren't bigger than MaxHeapTupleSize, with big datums toasted.
      But that's not the case for the old version of a tuple for logical
      decoding; the replica identity is logged as one piece. With the default
      replica identity btree limits that to small tuples, but that's not the
      case for FULL.
      
      Change the tuple buffer infrastructure to separate allocate over-large
      tuples, instead of always going through the slab cache.
      
      This unfortunately requires changing the ReorderBufferTupleBuf
      definition, we need to store the allocated size someplace. To avoid
      requiring output plugins to recompile, don't store HeapTupleHeaderData
      directly after HeapTupleData, but point to it via t_data; that leaves
      rooms for the allocated size.  As there's no reason for an output plugin
      to look at ReorderBufferTupleBuf->t_data.header, remove the field. It
      was just a minor convenience having it directly accessible.
      
      Reported-By: Adam Dratwiński
      Discussion: CAKg6ypLd7773AOX4DiOGRwQk1TVOQKhNwjYiVjJnpq8Wo+i62Q@mail.gmail.com
      c8f621c4
    • Andres Freund's avatar
      logical decoding: old/newtuple in spooled UPDATE changes was switched around. · 0bda14d5
      Andres Freund authored
      Somehow I managed to flip the order of restoring old & new tuples when
      de-spooling a change in a large transaction from disk. This happens to
      only take effect when a change is spooled to disk which has old/new
      versions of the tuple. That only is the case for UPDATEs where he
      primary key changed or where replica identity is changed to FULL.
      
      The tests didn't catch this because either spooled updates, or updates
      that changed primary keys, were tested; not both at the same time.
      
      Found while adding tests for the following commit.
      
      Backpatch: 9.4, where logical decoding was added
      0bda14d5
    • Andres Freund's avatar
      logical decoding: Tell reorderbuffer about all xids. · d9e903f3
      Andres Freund authored
      Logical decoding's reorderbuffer keeps transactions in an LSN ordered
      list for efficiency. To make that's efficiently possible upper-level
      xids are forced to be logged before nested subtransaction xids.  That
      only works though if these records are all looked at: Unfortunately we
      didn't do so for e.g. row level locks, which are otherwise uninteresting
      for logical decoding.
      
      This could lead to errors like:
      "ERROR: subxact logged without previous toplevel record".
      
      It's not sufficient to just look at row locking records, the xid could
      appear first due to a lot of other types of records (which will trigger
      the transaction to be marked logged with MarkCurrentTransactionIdLoggedIfAny).
      So invent infrastructure to tell reorderbuffer about xids seen, when
      they'd otherwise not pass through reorderbuffer.c.
      
      Reported-By: Jarred Ward
      Bug: #13844
      Discussion: 20160105033249.1087.66040@wrigleys.postgresql.org
      Backpatch: 9.4, where logical decoding was added
      d9e903f3
  3. 05 Mar, 2016 2 commits
    • Joe Conway's avatar
      Expose control file data via SQL accessible functions. · dc7d70ea
      Joe Conway authored
      Add four new SQL accessible functions: pg_control_system(),
      pg_control_checkpoint(), pg_control_recovery(), and pg_control_init()
      which expose a subset of the control file data.
      
      Along the way move the code to read and validate the control file to
      src/common, where it can be shared by the new backend functions
      and the original pg_controldata frontend program.
      
      Patch by me, significant input, testing, and review by Michael Paquier.
      dc7d70ea
    • Fujii Masao's avatar
      Ignore recovery_min_apply_delay until recovery has reached consistent state · d34794f7
      Fujii Masao authored
      Previously recovery_min_apply_delay was applied even before recovery
      had reached consistency. This could cause us to wait a long time
      unexpectedly for read-only connections to be allowed. It's problematic
      because the standby was useless during that wait time.
      
      This patch changes recovery_min_apply_delay so that it's applied once
      the database has reached the consistent state. That is, even if the delay
      is set, the standby tries to replay WAL records as fast as possible until
      it has reached consistency.
      
      Author: Michael Paquier
      Reviewed-By: Julien Rouhaud
      Reported-By: Greg Clough
      Backpatch: 9.4, where recovery_min_apply_delay was added
      Bug: #13770
      Discussion: http://www.postgresql.org/message-id/20151111155006.2644.84564@wrigleys.postgresql.org
      d34794f7
  4. 04 Mar, 2016 16 commits
    • Tom Lane's avatar
      Make stats regression test robust in the face of parallel query. · 60690a6f
      Tom Lane authored
      Historically, the wait_for_stats() function in this test has simply checked
      for a report of an indexscan on tenk2, corresponding to the last command
      issued before we expect stats updates to appear.  However, with parallel
      query that indexscan could be done by a parallel worker that will emit
      its stats counters to the collector before the session's main backend does
      (a full second before, in fact, thanks to the "pg_sleep(1.0)" added by
      commit 957d08c8).  That leaves a sizable window in which an
      autovacuum-triggered write of the stats files would present a state in
      which the indexscan on tenk2 appears to have been done, but none of the
      write updates performed by the test have been.  This is evidently the
      explanation for intermittent failures seen by me and on buildfarm member
      mandrill.
      
      To fix, we should check separately for both the tenk2 seqscan and indexscan
      counts, since those might be reported by different processes that could be
      delayed arbitrarily on an overloaded test machine.  And we need to check
      for at least one update-related count.  If we ever allow parallel workers
      to do writes, this will get even more complicated ... but in view of all
      the other hard problems that will entail, I don't feel a need to solve this
      one today.
      
      Per research by Rahila Syed and myself; part of this patch is Rahila's.
      60690a6f
    • Robert Haas's avatar
      Fix typo in comment. · 708020eb
      Robert Haas authored
      Thomas Munro
      708020eb
    • Robert Haas's avatar
      Minor improvements to transaction manager README. · 6fcde8a5
      Robert Haas authored
      A simple SELECT is handled by PortalRunSelect, not ProcessQuery.  Also,
      the previous indentation was unclear: change it so that a deeper level
      of indentation indicates that the outer function calls the inner one.
      
      Stas Kelvich
      6fcde8a5
    • Robert Haas's avatar
      Fix SerializeSnapshot not to overrun the allocated space. · 17b124d3
      Robert Haas authored
      Rushabh Lathia
      17b124d3
    • Teodor Sigaev's avatar
      Fix Windows build broken by d78a7d9c · 0e7557dc
      Teodor Sigaev authored
      0e7557dc
    • Robert Haas's avatar
      Minor optimizations based on ParallelContext having nworkers_launched. · df4685fb
      Robert Haas authored
      Originally, we didn't have nworkers_launched, so code that used parallel
      contexts had to be preprared for the possibility that not all of the
      workers requested actually got launched.  But now we can count on knowing
      the number of workers that were successfully launched, which can shave
      off a few cycles and simplify some code slightly.
      
      Amit Kapila, reviewed by Haribabu Kommi, per a suggestion from Peter
      Geoghegan.
      df4685fb
    • Robert Haas's avatar
      Fix InitializeSessionUserId not to deference NULL rolename pointer. · 546cd0d7
      Robert Haas authored
      Dmitriy Sarafannikov, reviewed by Michael Paquier and Haribabu Kommi,
      with a minor fix by me.
      546cd0d7
    • Teodor Sigaev's avatar
      Improve support of Hunspell in ispell dictionary. · d78a7d9c
      Teodor Sigaev authored
      Now it's possible to load recent version of Hunspell for several languages.
      To handle these dictionaries Hunspell patch adds support for:
      * FLAG long - sets the double extended ASCII character flag type
      * FLAG num - sets the decimal number flag type (from 1 to 65535)
      * AF parameter - alias for flag's set
      
      Also it moves test dictionaries into separate directory.
      
      Author: Artur Zakirov with editorization by me
      d78a7d9c
    • Robert Haas's avatar
      Fix query-based tab completion for multibyte characters. · 9445db92
      Robert Haas authored
      The existing code confuses the byte length of the string (which is
      relevant when passing it to pg_strncasecmp) with the character length
      of the string (which is relevant when it is used with the SQL substring
      function).  Separate those two concepts.
      
      Report and patch by Kyotaro Horiguchi, reviewed by Thomas Munro and
      reviewed and further revised by me.
      9445db92
    • Robert Haas's avatar
      Fix the way GetExistingLocalJoinPath is documented. · 33b5eab7
      Robert Haas authored
      The old approach made it look like it was an FDW callback, which it
      is not.
      
      Per a gripe from Stephen Frost.  Patch by me, reviewed by Ashutosh
      Bapat.
      33b5eab7
    • Robert Haas's avatar
      postgres_fdw: When sending ORDER BY, always include NULLS FIRST/LAST. · 3bea3f88
      Robert Haas authored
      Previously, we included NULLS FIRST when appropriate but relied on the
      default behavior to be NULLS LAST.  This is, however, not true for a
      sort in descending order and seems like a fragile assumption anyway.
      
      Report by Rajkumar Raghuwanshi.  Patch by Ashutosh Bapat.  Review
      comments from Michael Paquier and Tom Lane.
      3bea3f88
    • Alvaro Herrera's avatar
      Add 'tap_tests' flag in config_default.pl · 52fe6f4e
      Alvaro Herrera authored
      This makes the flag more visible for testers using the default file as a
      template, increasing the likelyhood that the test suite will be run.
      Also have the flag be displayed in the fake "configure" output, if set.
      
      This patch is two new lines only, but perltidy decides to shift things
      around which makes it appear a bit bigger.
      
      Author: Michaël Paquier
      Reviewed-by: Craig Ringer
      Discussion: https://www.postgresql.org/message-id/CAB7nPqRet6UAP2APhZAZw%3DVhJ6w-Q-gGLdZkrOqFgd2vc9-ZDw%40mail.gmail.com
      52fe6f4e
    • Peter Eisentraut's avatar
      Add prerequisite for KOI8-U.TXT · 1fa2a6b1
      Peter Eisentraut authored
      This was missed when the encoding was added.
      1fa2a6b1
    • Peter Eisentraut's avatar
      Make some adjustments in variable assignments · b497abc6
      Peter Eisentraut authored
      These variables aren't really used for anything interesting, but it
      seems the existing grouping was somewhat nonsensical.
      b497abc6
    • Peter Eisentraut's avatar
      Add missing rules related to EUC_JIS_2004 and SHIFT_JIS_2004 encodings · 7a4a813c
      Peter Eisentraut authored
      This was apparently forgotten in commit
      75c6519f.
      7a4a813c
    • Andres Freund's avatar
      Force synchronous_commit=on in test_decoding's concurrent_ddl_dml.spec. · 1986c3c4
      Andres Freund authored
      Otherwise running installcheck-force on a server with
      synchronous_commit=off will result in the tests failing. All the other
      tests already do so...
      
      Backpatch: 9.4, where logical decoding was added
      1986c3c4
  5. 03 Mar, 2016 8 commits
    • Alvaro Herrera's avatar
      pgbench: accept unambiguous builtin prefixes for -b · d561f1ca
      Alvaro Herrera authored
      This makes it easier to use "-b se" instead of typing the full "-b
      select-only".
      
      Author: Fabien Coelho
      Reviewed-by: Michaël Paquier
      d561f1ca
    • Alvaro Herrera's avatar
      Rework PostgresNode's psql method · 2c83f435
      Alvaro Herrera authored
      This makes the psql() method much more capable: it captures both stdout
      and stderr; it now returns the psql exit code rather than stdout; a
      timeout can now be specified, as can ON_ERROR_STOP behavior; it gained a
      new "on_error_die" (defaulting to off) parameter to raise an exception
      if there's any problem.  Finally, additional parameters to psql can be
      passed if there's need for further tweaking.
      
      For convenience, a new safe_psql() method retains much of the old
      behavior of psql(), except that it uses on_error_die on, so that
      problems like syntax errors in SQL commands can be detected more easily.
      
      Many existing TAP test files now use safe_psql, which is what is really
      wanted.  A couple of ->psql() calls are now added in the commit_ts
      tests, which verify that the right thing is happening on certain errors.
      Some ->command_fails() calls in recovery tests that were verifying that
      psql failed also became ->psql() calls now.
      
      Author: Craig Ringer. Some tweaks by Álvaro Herrera
      Reviewed-By: Michaël Paquier
      2c83f435
    • Alvaro Herrera's avatar
      perltidy PostgresNode and SimpleTee · 7d9a4301
      Alvaro Herrera authored
      Also, mention in README that Perl files should be perltidy'ed.  This
      isn't really the best place (since we have Perl files elsewhere in the
      tree) and this is already in pgindent's README, but this subdir is
      likely to get hacked a whole lot more than the other Perl files, so it
      seems okay to spend two lines on this.
      
      Author: Craig Ringer
      7d9a4301
    • Alvaro Herrera's avatar
      Fix mistakes in recovery tests · 5bec1ad4
      Alvaro Herrera authored
      One test was relying on method remove_tree that isn't implemented in the
      oldest Perl we support; fix it by using the older rmtree instead.
      
      Another test had a typo in a SQL command, which isn't noticed because
      the PostgresNode->psql() method doesn't check that queries return
      correctly.  That's undesirable and will also be fixed later on, but for
      now let's make the test actually work.
      
      Author: Craig Ringer
      5bec1ad4
    • Simon Riggs's avatar
      Revert buggy optimization of index scans · c7111d11
      Simon Riggs authored
      606c0123 attempted to reduce cost of index scans using > and <
      strategies, though got that completely wrong in a few complex cases.
      
      Revert whole patch until we find a safe optimization.
      c7111d11
    • Magnus Hagander's avatar
      Add prefix to pl/pgsql global variables and functions · 6c90996a
      Magnus Hagander authored
      Rename pl/pgsql global variables to always have a plpgsql_ prefix,
      so they don't conflict with other shared libraries loaded.
      6c90996a
    • Andres Freund's avatar
      logical decoding: fix decoding of a commit's commit time. · 7c17aac6
      Andres Freund authored
      When adding replication origins in 5aa23504, I somehow managed to set
      the timestamp of decoded transactions to InvalidXLogRecptr when decoding
      one made without a replication origin. Fix that, and the wrong type of
      the new commit_time variable.
      
      This didn't trigger a regression test failure because we explicitly
      don't show commit timestamps in the regression tests, as they obviously
      are variable. Add a test that checks that a decoded commit's timestamp
      is within minutes of NOW() from before the commit.
      
      Reported-By: Weiping Qu
      Diagnosed-By: Artur Zakirov
      Discussion: 56D4197E.9050706@informatik.uni-kl.de,
          56D42918.1010108@postgrespro.ru
      Backpatch: 9.5, where 5aa23504 originates.
      7c17aac6
    • Tom Lane's avatar
      Fix json_to_record() bug with nested objects. · a9d199f6
      Tom Lane authored
      A thinko concerning nesting depth caused json_to_record() to produce bogus
      output if a field of its input object contained a sub-object with a field
      name matching one of the requested output column names.  Per bug #13996
      from Johann Visagie.
      
      I added a regression test case based on his example, plus parallel tests
      for json_to_recordset, jsonb_to_record, jsonb_to_recordset.  The latter
      three do not exhibit the same bug (which suggests that we may be missing
      some opportunities to share code...) but testing seems like a good idea
      in any case.
      
      Back-patch to 9.4 where these functions were introduced.
      a9d199f6
  6. 02 Mar, 2016 3 commits
    • Tom Lane's avatar
      Create stub functions to support pg_upgrade of old contrib/tsearch2. · eb43e851
      Tom Lane authored
      Commits 9ff60273 and dbe23289 adjusted the declarations
      of some core functions referenced by contrib/tsearch2's install script,
      forgetting that in a pg_upgrade situation, we'll be trying to restore
      operator class definitions that reference the old signatures.  We've
      hit this problem before; solve it in the same way as before, namely by
      installing stub functions that have the expected signature and just
      invoke the correct function.  Per report from Jeff Janes.
      
      (Someday we ought to stop supporting contrib/tsearch2, but I'm not
      sure today is that day.)
      eb43e851
    • Alvaro Herrera's avatar
      Prefix temp data dirs with the node name · cc6077d4
      Alvaro Herrera authored
      This makes it easier to relate the temporary data dirs to each node in
      a test script.
      
      Author: Kyotaro Horiguchi
      Reviewed-By: Craig Ringer, Alvaro Herrera
      cc6077d4
    • Tom Lane's avatar
      Fix PL/Tcl's encoding conversion logic. · c8c7c93d
      Tom Lane authored
      PL/Tcl appears to contain logic to convert strings between the database
      encoding and UTF8, which is the only encoding modern Tcl will deal with.
      However, that code has been disabled since commit 03489512, which
      made it "#if defined(UNICODE_CONVERSION)" and neglected to provide any way
      for that symbol to become defined.  That might have been all right back
      in 2001, but these days we take a dim view of allowing strings with
      incorrect encoding into the database.
      
      Remove the conditional compilation, fix warnings about signed/unsigned char
      conversions, clean up assorted places that didn't bother with conversions.
      (Notably, there were lots of assumptions that database table and field
      names didn't need conversion...)
      
      Add a regression test based on plpython_unicode.  It's not terribly
      thorough, but better than no test at all.
      c8c7c93d