1. 28 Jun, 2014 1 commit
    • Tom Lane's avatar
      Allow pushdown of WHERE quals into subqueries with window functions. · d222585a
      Tom Lane authored
      We can allow this even without any specific knowledge of the semantics
      of the window function, so long as pushed-down quals will either accept
      every row in a given window partition, or reject every such row.  Because
      window functions act only within a partition, such a case can't result
      in changing the window functions' outputs for any surviving row.
      Eliminating entire partitions in this way obviously can reduce the cost
      of the window-function computations substantially.
      
      The fly in the ointment is that it's hard to be entirely sure whether
      this is true for an arbitrary qual condition.  This patch allows pushdown
      if (a) the qual references only partitioning columns, and (b) the qual
      contains no volatile functions.  We are at risk of incorrect results if
      the qual can produce different answers for values that the partitioning
      equality operator sees as equal.  While it's not hard to invent cases
      for which that can happen, it seems to seldom be a problem in practice,
      since no one has complained about a similar assumption that we've had
      for many years with respect to DISTINCT.  The potential performance
      gains seem to be worth the risk.
      
      David Rowley, reviewed by Vik Fearing; some credit is due also to
      Thomas Mayer who did considerable preliminary investigation.
      d222585a
  2. 27 Jun, 2014 4 commits
    • Alvaro Herrera's avatar
      Have multixact be truncated by checkpoint, not vacuum · f741300c
      Alvaro Herrera authored
      Instead of truncating pg_multixact at vacuum time, do it only at
      checkpoint time.  The reason for doing it this way is twofold: first, we
      want it to delete only segments that we're certain will not be required
      if there's a crash immediately after the removal; and second, we want to
      do it relatively often so that older files are not left behind if
      there's an untimely crash.
      
      Per my proposal in
      http://www.postgresql.org/message-id/20140626044519.GJ7340@eldon.alvh.no-ip.org
      we now execute the truncation in the checkpointer process rather than as
      part of vacuum.  Vacuum is in only charge of maintaining in shared
      memory the value to which it's possible to truncate the files; that
      value is stored as part of checkpoints also, and so upon recovery we can
      reuse the same value to re-execute truncate and reset the
      oldest-value-still-safe-to-use to one known to remain after truncation.
      
      Per bug reported by Jeff Janes in the course of his tests involving
      bug #8673.
      
      While at it, update some comments that hadn't been updated since
      multixacts were changed.
      
      Backpatch to 9.3, where persistency of pg_multixact files was
      introduced by commit 0ac5ad51.
      f741300c
    • Alvaro Herrera's avatar
      Don't allow relminmxid to go backwards during VACUUM FULL · b7e51d9c
      Alvaro Herrera authored
      We were allowing a table's pg_class.relminmxid value to move backwards
      when heaps were swapped by VACUUM FULL or CLUSTER.  There is a
      similar protection against relfrozenxid going backwards, which we
      neglected to clone when the multixact stuff was rejiggered by commit
      0ac5ad51.
      
      Backpatch to 9.3, where relminmxid was introduced.
      
      As reported by Heikki in
      http://www.postgresql.org/message-id/52401AEA.9000608@vmware.com
      b7e51d9c
    • Alvaro Herrera's avatar
      Fix broken Assert() introduced by 8e9a16ab8f7f0e58 · b2770576
      Alvaro Herrera authored
      Don't assert MultiXactIdIsRunning if the multi came from a tuple that
      had been share-locked and later copied over to the new cluster by
      pg_upgrade.  Doing that causes an error to be raised unnecessarily:
      MultiXactIdIsRunning is not open to the possibility that its argument
      came from a pg_upgraded tuple, and all its other callers are already
      checking; but such multis cannot, obviously, have transactions still
      running, so the assert is pointless.
      
      Noticed while investigating the bogus pg_multixact/offsets/0000 file
      left over by pg_upgrade, as reported by Andres Freund in
      http://www.postgresql.org/message-id/20140530121631.GE25431@alap3.anarazel.de
      
      Backpatch to 9.3, as the commit that introduced the buglet.
      b2770576
    • Tom Lane's avatar
      Disallow pushing volatile qual expressions down into DISTINCT subqueries. · 11470352
      Tom Lane authored
      A WHERE clause applied to the output of a subquery with DISTINCT should
      theoretically be applied only once per distinct row; but if we push it
      into the subquery then it will be evaluated at each row before duplicate
      elimination occurs.  If the qual is volatile this can give rise to
      observably wrong results, so don't do that.
      
      While at it, refactor a little bit to allow subquery_is_pushdown_safe
      to report more than one kind of restrictive condition without indefinitely
      expanding its argument list.
      
      Although this is a bug fix, it seems unwise to back-patch it into released
      branches, since it might de-optimize plans for queries that aren't giving
      any trouble in practice.  So apply to 9.4 but not further back.
      11470352
  3. 26 Jun, 2014 3 commits
    • Tom Lane's avatar
      Get rid of bogus separate pg_proc entries for json_extract_path operators. · f71136ee
      Tom Lane authored
      These should not have existed to begin with, but there was apparently some
      misunderstanding of the purpose of the opr_sanity regression test item
      that checks for operator implementation functions with their own comments.
      The idea there is to check for unintentional violations of the rule that
      operator implementation functions shouldn't be documented separately
      .... but for these functions, that is in fact what we want, since the
      variadic option is useful and not accessible via the operator syntax.
      Get rid of the extra pg_proc entries and fix the regression test and
      documentation to be explicit about what we're doing here.
      f71136ee
    • Tom Lane's avatar
      Forward-patch regression test for "could not find pathkey item to sort". · 344eed91
      Tom Lane authored
      Commit a87c7291 already fixed the bug this
      is checking for, but the regression test case it added didn't cover this
      scenario.  Since we managed to miss the fact that there was a bug at all,
      it seems like a good idea to propagate the extra test case forward to HEAD.
      344eed91
    • Fujii Masao's avatar
      Remove obsolete example of CSV log file name from log_filename document. · de42ed40
      Fujii Masao authored
      7380b638 changed log_filename so that epoch was not appended to it
      when no format specifier is given. But the example of CSV log file name
      with epoch still left in log_filename document. This commit removes
      such obsolete example.
      
      This commit also documents the defaults of log_directory and
      log_filename.
      
      Backpatch to all supported versions.
      
      Christoph Berg
      de42ed40
  4. 25 Jun, 2014 3 commits
    • Tom Lane's avatar
      Rationalize error messages within jsonfuncs.c. · 798e2357
      Tom Lane authored
      I noticed that the functions in jsonfuncs.c sometimes printed error
      messages that claimed I'd called some other function.  Investigation showed
      that this was from repurposing code into "worker" functions without taking
      much care as to whether it would mention the right SQL-level function if it
      threw an error.  Moreover, there was a weird mismash of messages that
      contained a fixed function name, messages that used %s for a function name,
      and messages that constructed a function name out of spare parts, like
      "json%s_populate_record" (which, quite aside from being ugly as sin, wasn't
      even sufficient to cover all the cases).  This would put an undue burden on
      our long-suffering translators.  Standardize on inserting the SQL function
      name with %s so as to reduce the number of translatable strings, and pass
      function names around as needed to make sure we can report the right one.
      Fix up some gratuitous variations in wording, too.
      798e2357
    • Tom Lane's avatar
      Cosmetic improvements in jsonfuncs.c. · 8d2d7ad5
      Tom Lane authored
      Re-pgindent, remove a lot of random vertical whitespace, remove useless
      (if not counterproductive) inline markings, get rid of unnecessary
      zero-padding of strings for hashtable searches.  No functional changes.
      8d2d7ad5
    • Tom Lane's avatar
      Fix handling of nested JSON objects in json_populate_recordset and friends. · 57d8c127
      Tom Lane authored
      populate_recordset_object_start() improperly created a new hash table
      (overwriting the link to the existing one) if called at nest levels
      greater than one.  This resulted in previous fields not appearing in
      the final output, as reported by Matti Hameister in bug #10728.
      In 9.4 the problem also affects json_to_recordset.
      
      This perhaps missed detection earlier because the default behavior is to
      throw an error for nested objects: you have to pass use_json_as_text = true
      to see the problem.
      
      In addition, fix query-lifespan leakage of the hashtable created by
      json_populate_record().  This is pretty much the same problem recently
      fixed in dblink: creating an intended-to-be-temporary context underneath
      the executor's per-tuple context isn't enough to make it go away at the
      end of the tuple cycle, because MemoryContextReset is not
      MemoryContextResetAndDeleteChildren.
      
      Michael Paquier and Tom Lane
      57d8c127
  5. 24 Jun, 2014 3 commits
    • Bruce Momjian's avatar
      pg_upgrade: remove pg_multixact files left by initdb · 0f748273
      Bruce Momjian authored
      This fixes a bug that caused vacuum to fail when the '0000' files left
      by initdb were accessed as part of vacuum's cleanup of old pg_multixact
      files.
      
      Backpatch through 9.3
      0f748273
    • Heikki Linnakangas's avatar
      Don't allow foreign tables with OIDs. · a87a7dc8
      Heikki Linnakangas authored
      The syntax doesn't let you specify "WITH OIDS" for foreign tables, but it
      was still possible with default_with_oids=true. But the rest of the system,
      including pg_dump, isn't prepared to handle foreign tables with OIDs
      properly.
      
      Backpatch down to 9.1, where foreign tables were introduced. It's possible
      that there are databases out there that already have foreign tables with
      OIDs. There isn't much we can do about that, but at least we can prevent
      them from being created in the future.
      
      Patch by Etsuro Fujita, reviewed by Hadi Moshayedi.
      a87a7dc8
    • Robert Haas's avatar
      Check for interrupts during tuple-insertion loops. · c922353b
      Robert Haas authored
      Normally, this won't matter too much; but if I/O is really slow, for
      example because the system is overloaded, we might write many pages
      before checking for interrupts.  A single toast insertion might
      write up to 1GB of data, and a multi-insert could write hundreds
      of tuples (and their corresponding TOAST data).
      c922353b
  6. 23 Jun, 2014 4 commits
  7. 21 Jun, 2014 3 commits
  8. 20 Jun, 2014 7 commits
    • Tom Lane's avatar
      Add Asserts to verify that catalog cache keys are unique and not null. · 8b38a538
      Tom Lane authored
      The catcache code is effectively assuming this already, so let's insist
      that the catalog and index are actually declared that way.
      
      Having done that, the comments in indexing.h about non-unique indexes
      not being used for catcaches are completely redundant not just mostly so;
      and we didn't have such a comment for every such index anyway.  So let's
      get rid of them.
      
      Per discussion of whether we should identify primary keys for catalogs.
      We might or might not take that further step, but this change in itself
      will allow quicker detection of misdeclared catcaches, so it seems worth
      doing in any case.
      8b38a538
    • Joe Conway's avatar
      Clean up data conversion short-lived memory context. · 1dde5782
      Joe Conway authored
      dblink uses a short-lived data conversion memory context. However it
      was not deleted when no longer needed, leading to a noticeable memory
      leak under some circumstances. Plug the hole, along with minor
      refactoring. Backpatch to 9.2 where the leak was introduced.
      
      Report and initial patch by MauMau. Reviewed/modified slightly by
      Tom Lane and me.
      1dde5782
    • Andres Freund's avatar
      Do all-visible handling in lazy_vacuum_page() outside its critical section. · ecac0e2b
      Andres Freund authored
      Since fdf9e211 lazy_vacuum_page() rechecks the all-visible status
      of pages in the second pass over the heap. It does so inside a
      critical section, but both visibilitymap_test() and
      heap_page_is_all_visible() perform operations that should not happen
      inside one. The former potentially performs IO and both potentially do
      memory allocations.
      
      To fix, simply move all the all-visible handling outside the critical
      section. Doing so means that the PD_ALL_VISIBLE on the page won't be
      included in the full page image of the HEAP2_CLEAN record anymore. But
      that's fine, the flag will be set by the HEAP2_VISIBLE logged later.
      
      Backpatch to 9.3 where the problem was introduced. The bug only came
      to light due to the assertion added in 4a170ee9 and isn't likely to
      cause problems in production scenarios. The worst outcome is a
      avoidable PANIC restart.
      
      This also gets rid of the difference in the order of operations
      between master and standby mentioned in 2a8e1ac5.
      
      Per reports from David Leverton and Keith Fiske in bug #10533.
      ecac0e2b
    • Andres Freund's avatar
      Don't allow to disable backend assertions via the debug_assertions GUC. · 3bdcf6a5
      Andres Freund authored
      The existance of the assert_enabled variable (backing the
      debug_assertions GUC) reduced the amount of knowledge some static code
      checkers (like coverity and various compilers) could infer from the
      existance of the assertion. That could have been solved by optionally
      removing the assertion_enabled variable from the Assert() et al macros
      at compile time when some special macro is defined, but the resulting
      complication doesn't seem to be worth the gain from having
      debug_assertions. Recompiling is fast enough.
      
      The debug_assertions GUC is still available, but readonly, as it's
      useful when diagnosing problems. The commandline/client startup option
      -A, which previously also allowed to enable/disable assertions, has
      been removed as it doesn't serve a purpose anymore.
      
      While at it, reduce code duplication in bufmgr.c and localbuf.c
      assertions checking for spurious buffer pins. That code had to be
      reindented anyway to cope with the assert_enabled removal.
      3bdcf6a5
    • Tom Lane's avatar
      Avoid leaking memory while evaluating arguments for a table function. · 45b0f357
      Tom Lane authored
      ExecMakeTableFunctionResult evaluated the arguments for a function-in-FROM
      in the query-lifespan memory context.  This is insignificant in simple
      cases where the function relation is scanned only once; but if the function
      is in a sub-SELECT or is on the inside of a nested loop, any memory
      consumed during argument evaluation can add up quickly.  (The potential for
      trouble here had been foreseen long ago, per existing comments; but we'd
      not previously seen a complaint from the field about it.)  To fix, create
      an additional temporary context just for this purpose.
      
      Per an example from MauMau.  Back-patch to all active branches.
      45b0f357
    • Noah Misch's avatar
      Fix contrib/pg_upgrade/test.sh for $PWD containing spaces. · 686f362b
      Noah Misch authored
      Most of the necessary quoting was in place; this catches the exceptions.
      686f362b
    • Noah Misch's avatar
      Let installcheck-world pass against a server requiring a password. · c82725ed
      Noah Misch authored
      Give passwords to each user created in support of an ECPG connection
      test case.  Use SET SESSION AUTHORIZATION, not a fresh connection, to
      reduce privileges during a dblink test case.
      
      To test against such a server, both the "make installcheck-world"
      environment and the postmaster environment must provide the default
      user's password; $PGPASSFILE is the principal way to do so.  (The
      postmaster environment needs it for dblink and postgres_fdw tests.)
      c82725ed
  9. 19 Jun, 2014 4 commits
    • Tom Lane's avatar
      Document SQL functions' behavior of parsing the whole function at once. · f28d9b10
      Tom Lane authored
      Haribabu Kommi, somewhat rewritten by me
      f28d9b10
    • Kevin Grittner's avatar
      Fix calculation of PREDICATELOCK_MANAGER_LWLOCK_OFFSET. · bfaa8c66
      Kevin Grittner authored
      Commit ea9df812 failed to include
      NUM_BUFFER_PARTITIONS in this offset, resulting in a bad offset.
      Ultimately this threw off NUM_FIXED_LWLOCKS which is based on
      earlier offsets, leading to memory allocation problems.  It seems
      likely to have also caused increased LWLOCK contention when
      serializable transactions were used, because lightweight locks used
      for that overlapped others.
      
      Reported by Amit Kapila with analysis and fix.
      Backpatch to 9.4, where the bug was introduced.
      bfaa8c66
    • Fujii Masao's avatar
      Don't allow data_directory to be set in postgresql.auto.conf by ALTER SYSTEM. · 9ba78fb0
      Fujii Masao authored
      data_directory could be set both in postgresql.conf and postgresql.auto.conf so far.
      This could cause some problematic situations like circular definition. To avoid such
      situations, this commit forbids a user to set data_directory in postgresql.auto.conf.
      
      Backpatch this to 9.4 where ALTER SYSTEM command was introduced.
      
      Amit Kapila, reviewed by Abhijit Menon-Sen, with minor adjustments by me.
      9ba78fb0
    • Tom Lane's avatar
      Improve our mechanism for controlling the Linux out-of-memory killer. · df8b7bc9
      Tom Lane authored
      Arrange for postmaster child processes to respond to two environment
      variables, PG_OOM_ADJUST_FILE and PG_OOM_ADJUST_VALUE, to determine whether
      they reset their OOM score adjustments and if so to what.  This is superior
      to the previous design involving #ifdef's in several ways.  The behavior is
      now available in a default build, and both ends of the adjustment --- the
      original adjustment of the postmaster's level and the subsequent
      readjustment by child processes --- can now be controlled in one place,
      namely the postmaster launch script.  So it's no longer necessary for the
      launch script to act on faith that the server was compiled with the
      appropriate options.  In addition, if someone wants to use an OOM score
      other than zero for the child processes, that doesn't take a recompile
      anymore; and we no longer have to cater separately to the two different
      historical kernel APIs for this adjustment.
      
      Gurjeet Singh, somewhat revised by me
      df8b7bc9
  10. 18 Jun, 2014 5 commits
    • Andrew Dunstan's avatar
      Remove unnecessary check for jbvBinary in convertJsonbValue. · 96066198
      Andrew Dunstan authored
      The check was confusing and is a condition that should never in fact
      happen.
      
      Per gripe from Dmitry Dolgov.
      96066198
    • Tom Lane's avatar
      Fix weird spacing in error message. · 66802246
      Tom Lane authored
      Seems to have been introduced in 1a3458b6.
      66802246
    • Andrew Dunstan's avatar
    • Tom Lane's avatar
      Implement UPDATE tab SET (col1,col2,...) = (SELECT ...), ... · 8f889b10
      Tom Lane authored
      This SQL-standard feature allows a sub-SELECT yielding multiple columns
      (but only one row) to be used to compute the new values of several columns
      to be updated.  While the same results can be had with an independent
      sub-SELECT per column, such a workaround can require a great deal of
      duplicated computation.
      
      The standard actually says that the source for a multi-column assignment
      could be any row-valued expression.  The implementation used here is
      tightly tied to our existing sub-SELECT support and can't handle other
      cases; the Bison grammar would have some issues with them too.  However,
      I don't feel too bad about this since other cases can be converted into
      sub-SELECTs.  For instance, "SET (a,b,c) = row_valued_function(x)" could
      be written "SET (a,b,c) = (SELECT * FROM row_valued_function(x))".
      8f889b10
    • Noah Misch's avatar
      Fix the MSVC build process for uuid-ossp. · 230ba02d
      Noah Misch authored
      Catch up with commit b8cc8f94's
      introduction of the HAVE_UUID_OSSP symbol to the principal build
      process.  Back-patch to 9.4, where that commit appeared.
      230ba02d
  11. 17 Jun, 2014 2 commits
  12. 16 Jun, 2014 1 commit
    • Tom Lane's avatar
      Avoid recursion when processing simple lists of AND'ed or OR'ed clauses. · 2146f134
      Tom Lane authored
      Since most of the system thinks AND and OR are N-argument expressions
      anyway, let's have the grammar generate a representation of that form when
      dealing with input like "x AND y AND z AND ...", rather than generating
      a deeply-nested binary tree that just has to be flattened later by the
      planner.  This avoids stack overflow in parse analysis when dealing with
      queries having more than a few thousand such clauses; and in any case it
      removes some rather unsightly inconsistencies, since some parts of parse
      analysis were generating N-argument ANDs/ORs already.
      
      It's still possible to get a stack overflow with weirdly parenthesized
      input, such as "x AND (y AND (z AND ( ... )))", but such cases are not
      mainstream usage.  The maximum depth of parenthesization is already
      limited by Bison's stack in such cases, anyway, so that the limit is
      probably fairly platform-independent.
      
      Patch originally by Gurjeet Singh, heavily revised by me
      2146f134