1. 15 Jul, 2016 8 commits
    • Andres Freund's avatar
      Make HEAP_LOCK/HEAP2_LOCK_UPDATED replay reset HEAP_XMAX_INVALID. · a4d357bf
      Andres Freund authored
      0ac5ad51 started to compress infomask bits in WAL records. Unfortunately
      the replay routines for XLOG_HEAP_LOCK/XLOG_HEAP2_LOCK_UPDATED forgot to
      reset the HEAP_XMAX_INVALID (and some other) hint bits.
      
      Luckily that's not problematic in the majority of cases, because after a
      crash/on a standby row locks aren't meaningful. Unfortunately that does
      not hold true in the presence of prepared transactions. This means that
      after a crash, or after promotion, row level locks held by a prepared,
      but not yet committed, prepared transaction might not be enforced.
      
      Discussion: 20160715192319.ubfuzim4zv3rqnxv@alap3.anarazel.de
      Backpatch: 9.3, the oldest branch on which 0ac5ad51 is present.
      a4d357bf
    • Tom Lane's avatar
      Avoid invalidating all foreign-join cached plans when user mappings change. · 45639a05
      Tom Lane authored
      We must not push down a foreign join when the foreign tables involved
      should be accessed under different user mappings.  Previously we tried
      to enforce that rule literally during planning, but that meant that the
      resulting plans were dependent on the current contents of the
      pg_user_mapping catalog, and we had to blow away all cached plans
      containing any remote join when anything at all changed in pg_user_mapping.
      This could have been improved somewhat, but the fact that a syscache inval
      callback has very limited info about what changed made it hard to do better
      within that design.  Instead, let's change the planner to not consider user
      mappings per se, but to allow a foreign join if both RTEs have the same
      checkAsUser value.  If they do, then they necessarily will use the same
      user mapping at runtime, and we don't need to know specifically which one
      that is.  Post-plan-time changes in pg_user_mapping no longer require any
      plan invalidation.
      
      This rule does give up some optimization ability, to wit where two foreign
      table references come from views with different owners or one's from a view
      and one's directly in the query, but nonetheless the same user mapping
      would have applied.  We'll sacrifice the first case, but to not regress
      more than we have to in the second case, allow a foreign join involving
      both zero and nonzero checkAsUser values if the nonzero one is the same as
      the prevailing effective userID.  In that case, mark the plan as only
      runnable by that userID.
      
      The plancache code already had a notion of plans being userID-specific,
      in order to support RLS.  It was a little confused though, in particular
      lacking clarity of thought as to whether it was the rewritten query or just
      the finished plan that's dependent on the userID.  Rearrange that code so
      that it's clearer what depends on which, and so that the same logic applies
      to both RLS-injected role dependency and foreign-join-injected role
      dependency.
      
      Note that this patch doesn't remove the other issue mentioned in the
      original complaint, which is that while we'll reliably stop using a foreign
      join if it's disallowed in a new context, we might fail to start using a
      foreign join if it's now allowed, but we previously created a generic
      cached plan that didn't use one.  It was agreed that the chance of winning
      that way was not high enough to justify the much larger number of plan
      invalidations that would have to occur if we tried to cause it to happen.
      
      In passing, clean up randomly-varying spelling of EXPLAIN commands in
      postgres_fdw.sql, and fix a COSTS ON example that had been allowed to
      leak into the committed tests.
      
      This reverts most of commits fbe5a3fb and 5d4171d1, which were the
      previous attempt at ensuring we wouldn't push down foreign joins that
      span permissions contexts.
      
      Etsuro Fujita and Tom Lane
      
      Discussion: <d49c1e5b-f059-20f4-c132-e9752ee0113e@lab.ntt.co.jp>
      45639a05
    • Alvaro Herrera's avatar
      Avoid serializability errors when locking a tuple with a committed update · 533e9c6b
      Alvaro Herrera authored
      When key-share locking a tuple that has been not-key-updated, and the
      update is a committed transaction, in some cases we raised
      serializability errors:
          ERROR:  could not serialize access due to concurrent update
      
      Because the key-share doesn't conflict with the update, the error is
      unnecessary and inconsistent with the case that the update hasn't
      committed yet.  This causes problems for some usage patterns, even if it
      can be claimed that it's sufficient to retry the aborted transaction:
      given a steady stream of updating transactions and a long locking
      transaction, the long transaction can be starved indefinitely despite
      multiple retries.
      
      To fix, we recognize that HeapTupleSatisfiesUpdate can return
      HeapTupleUpdated when an updating transaction has committed, and that we
      need to deal with that case exactly as if it were a non-committed
      update: verify whether the two operations conflict, and if not, carry on
      normally.  If they do conflict, however, there is a difference: in the
      HeapTupleBeingUpdated case we can just sleep until the concurrent
      transaction is gone, while in the HeapTupleUpdated case this is not
      possible and we must raise an error instead.
      
      Per trouble report from Olivier Dony.
      
      In addition to a couple of test cases that verify the changed behavior,
      I added a test case to verify the behavior that remains unchanged,
      namely that errors are raised when a update that modifies the key is
      used.  That must still generate serializability errors.  One
      pre-existing test case changes behavior; per discussion, the new
      behavior is actually the desired one.
      
      Discussion: https://www.postgresql.org/message-id/560AA479.4080807@odoo.com
        https://www.postgresql.org/message-id/20151014164844.3019.25750@wrigleys.postgresql.org
      
      Backpatch to 9.3, where the problem appeared.
      533e9c6b
    • Teodor Sigaev's avatar
      Fix parsing NOT sequence in tsquery · 00f304ce
      Teodor Sigaev authored
      Digging around bug #14245 I found that commit
      6734a1ca missed that NOT operation is
      right associative in opposite to all other. This miss is resposible for
      tsquery parser fail on sequence of NOT operations
      00f304ce
    • Teodor Sigaev's avatar
      Fix nested NOT operation cleanup in tsquery. · 19d29015
      Teodor Sigaev authored
      During normalization of tsquery tree it tries to simplify nested NOT
      operations but there it's obvioulsy missed that subsequent node could be
      a leaf node (value node)
      
      Bug #14245: Segfault on weird to_tsquery
      Reported by David Kellum.
      19d29015
    • Tom Lane's avatar
      Improve documentation about search_path for SECURITY DEFINER functions. · ce150e7e
      Tom Lane authored
      Clarify that the reason for recommending that pg_temp be put last is to
      prevent temporary tables from capturing unqualified table names.  Per
      discussion with Albe Laurenz.
      
      Discussion: <A737B7A37273E048B164557ADEF4A58B5386C6E1@ntex2010i.host.magwien.gv.at>
      ce150e7e
    • Peter Eisentraut's avatar
      Adjust spellings of forms of "cancel" · 63cfdb8d
      Peter Eisentraut authored
      63cfdb8d
    • Peter Eisentraut's avatar
      doc: Fix typos · 3aed52a6
      Peter Eisentraut authored
      From: Alexander Law <exclusion@gmail.com>
      3aed52a6
  2. 14 Jul, 2016 2 commits
    • Tom Lane's avatar
      Fix GiST index build for NaN values in geometric types. · 1acf7572
      Tom Lane authored
      GiST index build could go into an infinite loop when presented with boxes
      (or points, circles or polygons) containing NaN component values.  This
      happened essentially because the code assumed that x == x is true for any
      "double" value x; but it's not true for NaNs.  The looping behavior was not
      the only problem though: we also attempted to sort the items using simple
      double comparisons.  Since NaNs violate the trichotomy law, qsort could
      (in principle at least) get arbitrarily confused and mess up the sorting of
      ordinary values as well as NaNs.  And we based splitting choices on box size
      calculations that could produce NaNs, again resulting in undesirable
      behavior.
      
      To fix, replace all comparisons of doubles in this logic with
      float8_cmp_internal, which is NaN-aware and is careful to sort NaNs
      consistently, higher than any non-NaN.  Also rearrange the box size
      calculation to not produce NaNs; instead it should produce an infinity
      for a box with NaN on one side and not-NaN on the other.
      
      I don't by any means claim that this solves all problems with NaNs in
      geometric values, but it should at least make GiST index insertion work
      reliably with such data.  It's likely that the index search side of things
      still needs some work, and probably regular geometric operations too.
      But with this patch we're laying down a convention for how such cases
      ought to behave.
      
      Per bug #14238 from Guang-Dih Lei.  Back-patch to 9.2; the code used before
      commit 7f3bd868 is quite different and doesn't lock up on my simple
      test case, nor on the submitter's dataset.
      
      Report: <20160708151747.1426.60150@wrigleys.postgresql.org>
      Discussion: <28685.1468246504@sss.pgh.pa.us>
      1acf7572
    • Magnus Hagander's avatar
      Remove reference to range mode in pg_xlogdump error · 00e0b67a
      Magnus Hagander authored
      pg_xlogdump doesn't have any other mode, so it's just confusing to
      include this in the error message as it indicates there might be another
      mode.
      00e0b67a
  3. 13 Jul, 2016 4 commits
    • Tom Lane's avatar
      Minor test adjustment. · 56a99741
      Tom Lane authored
      Dept of second thoughts: given the RESET SESSION AUTHORIZATION that
      was just added by commit cec55013, we don't need the reconnection
      that used to be here.  Might as well buy back a few microseconds.
      56a99741
    • Tom Lane's avatar
      Add a regression test case to improve code coverage for tuplesort. · cec55013
      Tom Lane authored
      Test the external-sort code path in CLUSTER for two different scenarios:
      multiple-pass external sorting, and the best case for replacement
      selection, where only one run is produced, so that no merge is required.
      This test would have caught the bug fixed in commit 1b0fc850, at
      least when run with valgrind enabled.
      
      In passing, add a short-circuit test in plan_cluster_use_sort() to make
      dead certain that it selects sorting when enable_indexscan is off.  As
      things stand, that would happen anyway, but it seems like good future
      proofing for this test.
      
      Peter Geoghegan
      
      Discussion: <CAM3SWZSgxehDkDMq1FdiW2A0Dxc79wH0hz1x-TnGy=1BXEL+nw@mail.gmail.com>
      cec55013
    • Tom Lane's avatar
      Fix obsolete header-file reference in pg_buffercache docs. · 91c0eb50
      Tom Lane authored
      Commit 2d001904 moved enum ForkNumber from relfilenode.h into relpath.h,
      but missed updating this documentation reference.
      
      Alexander Law
      91c0eb50
    • Stephen Frost's avatar
      Add missing hyphen · 42ec6c2d
      Stephen Frost authored
      Pointed out by Alexander Law
      42ec6c2d
  4. 12 Jul, 2016 4 commits
  5. 11 Jul, 2016 5 commits
    • Tom Lane's avatar
      Print a given subplan only once in EXPLAIN. · 4d042999
      Tom Lane authored
      We have, for a very long time, allowed the same subplan (same member of the
      PlannedStmt.subplans list) to be referenced by more than one SubPlan node;
      this avoids problems for cases such as subplans within an IndexScan's
      indxqual and indxqualorig fields.  However, EXPLAIN had not gotten the memo
      and would print each reference as though it were an independent identical
      subplan.  To fix, track plan_ids of subplans we've printed and don't print
      the same plan_id twice.  Per report from Pavel Stehule.
      
      BTW: the particular case of IndexScan didn't cause visible duplication
      in a plain EXPLAIN, only EXPLAIN ANALYZE, because in the former case we
      short-circuit executor startup before the indxqual field is processed by
      ExecInitExpr.  That seems like it could easily lead to other EXPLAIN
      problems in future, but it's not clear how to avoid it without breaking
      the "EXPLAIN a plan using hypothetical indexes" use-case.  For now I've
      left that issue alone.
      
      Although this is a longstanding bug, it's purely cosmetic (no great harm
      is done by the repeat printout) and we haven't had field complaints before.
      So I'm hesitant to back-patch it, especially since there is some small risk
      of ABI problems due to the need to add a new field to ExplainState.
      
      In passing, rearrange order of fields in ExplainState to be less random,
      and update some obsolete comments about when/where to initialize them.
      
      Report: <CAFj8pRAimq+NK-menjt+3J4-LFoodDD8Or6=Lc_stcFD+eD4DA@mail.gmail.com>
      4d042999
    • Tom Lane's avatar
      Improve output of psql's \df+ command. · a670c24c
      Tom Lane authored
      Add display of proparallel (parallel-safety) when the server is >= 9.6,
      and display of proacl (access privileges) for all server versions.
      Minor tweak of column ordering to keep related columns together.
      
      Michael Paquier
      
      Discussion: <CAB7nPqTR3Vu3xKOZOYqSm-+bSZV0kqgeGAXD6w5GLbkbfd5Q6w@mail.gmail.com>
      a670c24c
    • Peter Eisentraut's avatar
      doc: Update URL for PL/PHP · 740bf396
      Peter Eisentraut authored
      740bf396
    • Magnus Hagander's avatar
      Add missing newline in error message · ae7d78c3
      Magnus Hagander authored
      ae7d78c3
    • Magnus Hagander's avatar
      Fix start WAL filename for concurrent backups from standby · 87d84d67
      Magnus Hagander authored
      On a standby, ThisTimelineID is always 0, so we would generate a
      filename in timeline 0 even for other timelines. Instead, use starttli
      which we have retreived from the controlfile.
      
      Report by: Francesco Canovai in bug #14230
      Author: Marco Nenciarini
      Reviewed by: Michael Paquier and Amit Kapila
      87d84d67
  6. 10 Jul, 2016 1 commit
  7. 09 Jul, 2016 2 commits
    • Tom Lane's avatar
      Fix TAP tests and MSVC scripts for pathnames with spaces. · 30b2731b
      Tom Lane authored
      Change assorted places in our Perl code that did things like
      	system("prog $path/file");
      to do it more like
      	system('prog', "$path/file");
      which is safe against spaces and other special characters in the path
      variable.  The latter was already the prevailing style, but a few bits
      of code hadn't gotten this memo.  Back-patch to 9.4 as relevant.
      
      Michael Paquier, Kyotaro Horiguchi
      
      Discussion: <20160704.160213.111134711.horiguchi.kyotaro@lab.ntt.co.jp>
      30b2731b
    • Tom Lane's avatar
      Improve recording of IA64 stack data. · c5756272
      Tom Lane authored
      Examination of the results from anole and gharial suggests that we're
      only managing to track the size of one of the two stacks of IA64 machines.
      Some googling gave the answer: on HPUX11, the register stack is reported
      as a page type I don't see in pstat.h on my HPUX10 box.  Let's try
      testing for that.
      c5756272
  8. 08 Jul, 2016 7 commits
  9. 07 Jul, 2016 7 commits
    • Robert Haas's avatar
      Fix typo in comment. · 3edcdbcc
      Robert Haas authored
      Amit Langote
      3edcdbcc
    • Robert Haas's avatar
      Properly adjust pointers when tuples are moved during CLUSTER. · 1b0fc850
      Robert Haas authored
      Otherwise, when we abandon incremental memory accounting and use
      batch allocation for the final merge pass, we might crash.  This
      has been broken since 0011c009.
      
      Peter Geoghegan, tested by Noah Misch
      1b0fc850
    • Robert Haas's avatar
      Fix a prototype which is inconsistent with the function definition. · b22934dc
      Robert Haas authored
      Peter Geoghegan
      b22934dc
    • Robert Haas's avatar
      Clarify resource utilization of parallel query. · d1f822e5
      Robert Haas authored
      temp_file_limit is a per-process limit, not a per-session limit across
      all cooperating parallel processes; change wording accordingly, per a
      suggestion from Tom Lane.
      
      Also, document under max_parallel_workers_per_gather the fact that each
      process involved in a parallel query may use as many resources as a
      separate session.  Caveat emptor.
      
      Per a complaint from Peter Geoghegan.
      d1f822e5
    • Tom Lane's avatar
      Reduce stack space consumption in tzload(). · 62c8421e
      Tom Lane authored
      While syncing our timezone code with IANA's updates in commit 1c1a7cbd,
      I'd chosen not to adopt the code they conditionally compile under #ifdef
      ALL_STATE.  The main thing that that drives is that the space for gmtime
      and localtime timezone definitions isn't statically allocated, but is
      malloc'd on first use.  I reasoned we didn't need that logic: we don't have
      localtime() at all, and we always initialize TimeZone to GMT so we always
      need that one.  But there is one other thing ALL_STATE does, which is to
      make tzload() malloc its transient workspace instead of just declaring it
      as a local variable.  It turns out that that local variable occupies 78K.
      Even worse is that, at least for common US timezone settings, there's a
      recursive call to parse the "posixrules" zone name, making peak stack
      consumption to select a time zone upwards of 150K.  That's an uncomfortably
      large fraction of our STACK_DEPTH_SLOP safety margin, and could result in
      outright crashes if we try to reduce STACK_DEPTH_SLOP as has been discussed
      recently.  Furthermore, this means that the postmaster's peak stack
      consumption is several times that of a backend running typical queries
      (since, except on Windows, backends inherit the timezone GUC values and
      don't ever run this code themselves unless you do SET TIMEZONE).  That's
      completely backwards from a safety perspective.
      
      Hence, adopt the ALL_STATE rather than non-ALL_STATE variant of tzload(),
      while not changing the other code aspects that symbol controls.  The
      risk of an ENOMEM error from malloc() seems less than that of a SIGSEGV
      from stack overrun.
      
      This should probably get back-patched along with 1c1a7cbd and followon
      fixes, whenever we decide we have enough confidence in the updates to do
      that.
      62c8421e
    • Fujii Masao's avatar
      Rename pg_stat_wal_receiver.conn_info to conninfo. · 60d50769
      Fujii Masao authored
      Per discussion on pgsql-hackers, conninfo is better as the column name
      because it's more commonly used in PostgreSQL.
      
      Catalog version bumped due to the change of pg_proc.
      
      Author: Michael Paquier
      60d50769
    • Peter Eisentraut's avatar
      Fix typos · 397bf6ee
      Peter Eisentraut authored
      397bf6ee