1. 23 Nov, 2019 3 commits
  2. 22 Nov, 2019 6 commits
    • Tom Lane's avatar
      Make psql redisplay the query buffer after \e. · d1c866e5
      Tom Lane authored
      Up to now, whatever you'd edited was put back into the query buffer
      but not redisplayed, which is less than user-friendly.  But we can
      improve that just by printing the text along with a prompt, if we
      enforce that the editing result ends with a newline (which it
      typically would anyway).  You then continue typing more lines if
      you want, or you can type ";" or do \g or \r or another \e.
      
      This is intentionally divorced from readline's processing,
      for simplicity and so that it works the same with or without
      readline enabled.  We discussed possibly integrating things
      more closely with readline; but that seems difficult, uncertainly
      portable across different readline and libedit versions, and
      of limited real benefit anyway.  Let's try the simple way and
      see if it's good enough.
      
      Patch by me, thanks to Fabien Coelho and Laurenz Albe for review
      
      Discussion: https://postgr.es/m/13192.1572318028@sss.pgh.pa.us
      d1c866e5
    • Tom Lane's avatar
      Avoid taking a new snapshot for an immutable simple expression in plpgsql. · 73b06cf8
      Tom Lane authored
      We already used this optimization if the plpgsql function is read-only.
      But it seems okay to do it even in a read-write function, if the
      expression contains only immutable functions/operators.  There would
      only be a change of behavior if an "immutable" called function depends
      on seeing database updates made during the current plpgsql function.
      That's enough of a violation of the promise of immutability that anyone
      who complains won't have much of a case.
      
      The benefits are significant --- for simple cases like
      
        while i < 10000000
        loop
          i := i + 1;
        end loop;
      
      I see net performance improvements around 45%.  Of course, real-world
      cases won't get that much faster, but it ought to be noticeable.
      At the very least, this removes much of the performance penalty that
      used to exist for forgetting to mark a plpgsql function non-volatile.
      
      Konstantin Knizhnik, reviewed by Pavel Stehule, cosmetic changes by me
      
      Discussion: https://postgr.es/m/ed9da20e-01aa-d04b-d085-e6c16b14b9d7@postgrespro.ru
      73b06cf8
    • Tom Lane's avatar
      Add test coverage for "unchanged toast column" replication code path. · f67b1737
      Tom Lane authored
      It seems pretty unacceptable to have no regression test coverage
      for this aspect of the logical replication protocol, especially
      given the bugs we've found in related code.
      
      Discussion: https://postgr.es/m/16129-a0c0f48e71741e5f@postgresql.org
      f67b1737
    • Tom Lane's avatar
      Fix bogus tuple-slot management in logical replication UPDATE handling. · 4d9ceb00
      Tom Lane authored
      slot_modify_cstrings seriously abused the TupleTableSlot API by relying
      on a slot's underlying data to stay valid across ExecClearTuple.  Since
      this abuse was also quite undocumented, it's little surprise that the
      case got broken during the v12 slot rewrites.  As reported in bug #16129
      from Ondřej Jirman, this could lead to crashes or data corruption when
      a logical replication subscriber processes a row update.  Problems would
      only arise if the subscriber's table contained columns of pass-by-ref
      types that were not being copied from the publisher.
      
      Fix by explicitly copying the datum/isnull arrays from the source slot
      that the old row was in already.  This ends up being about the same
      thing that happened pre-v12, but hopefully in a less opaque and
      fragile way.
      
      We might've caught the problem sooner if there were any test cases
      dealing with updates involving non-replicated or dropped columns.
      Now there are.
      
      Back-patch to v10 where this code came in.  Even though the failure
      does not manifest before v12, IMO this code is too fragile to leave
      as-is.  In any case we certainly want the additional test coverage.
      
      Patch by me; thanks to Tomas Vondra for initial investigation.
      
      Discussion: https://postgr.es/m/16129-a0c0f48e71741e5f@postgresql.org
      4d9ceb00
    • Michael Paquier's avatar
      Add .gitignore to src/tutorial/ · 8959a5e0
      Michael Paquier authored
      A set of SQL files are generated for the tutorial when compiling the
      code, so let's avoid any risk to have them added in the tree in the
      future.
      8959a5e0
    • Michael Paquier's avatar
      Remove traces of version-0 calling convention in src/tutorial/ · a9d5157a
      Michael Paquier authored
      Support has been removed as of 5ded4bd2, but code related to the tutorial
      still used it.  Functions using version-1 are already present for some
      time in the tutorial, and the documentation mentions them, so just
      replace the old version with the new one.
      
      Reported-by: Pavel Stehule
      Analyzed-by: Euler Taveira
      Author: Michael Paquier
      Reviewed-by: Tom Lane, Pavel Stehule
      Discussion: https://postgr.es/m/CAFj8pRCgC2uDzrw-vvanXu6Z3ofyviEOQPEpH6_aL4OCe7JRag@mail.gmail.com
      a9d5157a
  3. 21 Nov, 2019 8 commits
  4. 20 Nov, 2019 9 commits
  5. 19 Nov, 2019 9 commits
    • Tom Lane's avatar
      Fix corner-case failure in match_pattern_prefix(). · b3c265d7
      Tom Lane authored
      The planner's optimization code for LIKE and regex operators could
      error out with a complaint like "no = operator for opfamily NNN"
      if someone created a binary-compatible index (for example, a
      bpchar_ops index on a text column) on the LIKE's left argument.
      
      This is a consequence of careless refactoring in commit 74dfe58a.
      The old code in match_special_index_operator only accepted specific
      combinations of the pattern operator and the index opclass, thereby
      indirectly guaranteeing that the opclass would have a comparison
      operator with the same LHS input type as the pattern operator.
      While moving the logic out to a planner support function, I simplified
      that test in a way that no longer guarantees that.  Really though we'd
      like an altogether weaker dependency on the opclass, so rather than
      put back exactly the old code, just allow lookup failure.  I have in
      mind now to rewrite this logic completely, but this is the minimum
      change needed to fix the bug in v12.
      
      Per report from Manuel Rigger.  Back-patch to v12 where the mistake
      came in.
      
      Discussion: https://postgr.es/m/CA+u7OA7nnGYy8rY0vdTe811NuA+Frr9nbcBO9u2Z+JxqNaud+g@mail.gmail.com
      b3c265d7
    • Alexander Korotkov's avatar
      Fix page modification outside of critical section in GIN · b1071408
      Alexander Korotkov authored
      By oversight 52ac6cd2 makes ginDeletePage() sets pd_prune_xid of page to be
      deleted before entering the critical section.  It appears that only versions 11
      and later were affected by this oversight.
      
      Backpatch-through: 11
      b1071408
    • Alexander Korotkov's avatar
      Revise GIN README · 32ca32d0
      Alexander Korotkov authored
      We find GIN concurrency bugs from time to time.  One of the problems here is
      that concurrency of GIN isn't well-documented in README.  So, it might be even
      hard to distinguish design bugs from implementation bugs.
      
      This commit revised concurrency section in GIN README providing more details.
      Some examples are illustrated in ASCII art.
      
      Also, this commit add the explanation of how is tuple layout in internal GIN
      B-tree page different in comparison with nbtree.
      
      Discussion: https://postgr.es/m/CAPpHfduXR_ywyaVN4%2BOYEGaw%3DcPLzWX6RxYLBncKw8de9vOkqw%40mail.gmail.com
      Author: Alexander Korotkov
      Reviewed-by: Peter Geoghegan
      Backpatch-through: 9.4
      32ca32d0
    • Alexander Korotkov's avatar
      Fix traversing to the deleted GIN page via downlink · d5ad7a09
      Alexander Korotkov authored
      Current GIN code appears to don't handle traversing to the deleted page via
      downlink.  This commit fixes that by stepping right from the delete page like
      we do in nbtree.
      
      This commit also fixes setting 'deleted' flag to the GIN pages.  Now other page
      flags are not erased once page is deleted.  That helps to keep our assertions
      true if we arrive deleted page via downlink.
      
      Discussion: https://postgr.es/m/CAPpHfdvMvsw-NcE5bRS7R1BbvA4BxoDnVVjkXC5W0Czvy9LVrg%40mail.gmail.com
      Author: Alexander Korotkov
      Reviewed-by: Peter Geoghegan
      Backpatch-through: 9.4
      d5ad7a09
    • Alexander Korotkov's avatar
      Fix deadlock between ginDeletePage() and ginStepRight() · e1464119
      Alexander Korotkov authored
      When ginDeletePage() is about to delete page it locks its left sibling to revise
      the rightlink.  So, it locks pages in right to left manner.  Int he same time
      ginStepRight() locks pages in left to right manner, and that could cause a
      deadlock.
      
      This commit makes ginScanToDelete() keep exclusive lock on left siblings of
      currently investigated path.  That elimites need to relock left sibling in
      ginDeletePage().  Thus, deadlock with ginStepRight() can't happen anymore.
      
      Reported-by: Chen Huajun
      Discussion: https://postgr.es/m/5c332bd1.87b6.16d7c17aa98.Coremail.chjischj%40163.com
      Author: Alexander Korotkov
      Reviewed-by: Peter Geoghegan
      Backpatch-through: 10
      e1464119
    • Tom Lane's avatar
      Doc: clarify use of RECURSIVE in WITH. · 5b805886
      Tom Lane authored
      Apparently some people misinterpreted the syntax as being that
      RECURSIVE is a prefix of individual WITH queries.  It's a modifier
      for the WITH clause as a whole, so state that more clearly.
      
      Discussion: https://postgr.es/m/ca53c6ce-a0c6-b14a-a8e3-162f0b2cc119@a-kretschmer.de
      5b805886
    • Tom Lane's avatar
      Doc: clarify behavior of ALTER DEFAULT PRIVILEGES ... IN SCHEMA. · 787b3fd3
      Tom Lane authored
      The existing text stated that "Default privileges that are specified
      per-schema are added to whatever the global default privileges are for
      the particular object type".  However, that bare-bones observation is
      not quite clear enough, as demonstrated by the complaint in bug #16124.
      Flesh it out by stating explicitly that you can't revoke built-in
      default privileges this way, and by providing an example to drive
      the point home.
      
      Back-patch to all supported branches, since it's been like this
      from the beginning.
      
      Discussion: https://postgr.es/m/16124-423d8ee4358421bc@postgresql.org
      787b3fd3
    • Thomas Munro's avatar
      Allow invisible PROMPT2 in psql. · 7f338369
      Thomas Munro authored
      Keep track of the visible width of PROMPT1, and provide %w as a way
      for PROMPT2 to generate the same number of spaces.
      
      Author: Thomas Munro, with ideas from others
      Reviewed-by: Tom Lane (earlier version)
      Discussion: https://postgr.es/m/CA%2BhUKG%2BzGd7RigjWbxwhzGW59gUpf76ydQECeGdEdodH6nd__A%40mail.gmail.com
      7f338369
    • Amit Kapila's avatar
      Add logical_decoding_work_mem to limit ReorderBuffer memory usage. · cec2edfa
      Amit Kapila authored
      Instead of deciding to serialize a transaction merely based on the
      number of changes in that xact (toplevel or subxact), this makes
      the decisions based on amount of memory consumed by the changes.
      
      The memory limit is defined by a new logical_decoding_work_mem GUC,
      so for example we can do this
      
          SET logical_decoding_work_mem = '128kB'
      
      to reduce the memory usage of walsenders or set the higher value to
      reduce disk writes. The minimum value is 64kB.
      
      When adding a change to a transaction, we account for the size in
      two places. Firstly, in the ReorderBuffer, which is then used to
      decide if we reached the total memory limit. And secondly in the
      transaction the change belongs to, so that we can pick the largest
      transaction to evict (and serialize to disk).
      
      We still use max_changes_in_memory when loading changes serialized
      to disk. The trouble is we can't use the memory limit directly as
      there might be multiple subxact serialized, we need to read all of
      them but we don't know how many are there (and which subxact to
      read first).
      
      We do not serialize the ReorderBufferTXN entries, so if there is a
      transaction with many subxacts, most memory may be in this type of
      objects. Those records are not included in the memory accounting.
      
      We also do not account for INTERNAL_TUPLECID changes, which are
      kept in a separate list and not evicted from memory. Transactions
      with many CTID changes may consume significant amounts of memory,
      but we can't really do much about that.
      
      The current eviction algorithm is very simple - the transaction is
      picked merely by size, while it might be useful to also consider age
      (LSN) of the changes for example. With the new Generational memory
      allocator, evicting the oldest changes would make it more likely
      the memory gets actually pfreed.
      
      The logical_decoding_work_mem can be set in postgresql.conf, in which
      case it serves as the default for all publishers on that instance.
      
      Author: Tomas Vondra, with changes by Dilip Kumar and Amit Kapila
      Reviewed-by: Dilip Kumar and Amit Kapila
      Tested-By: Vignesh C
      Discussion: https://postgr.es/m/688b0b7f-2f6c-d827-c27b-216a8e3ea700@2ndquadrant.com
      cec2edfa
  6. 18 Nov, 2019 1 commit
    • Peter Geoghegan's avatar
      nbtree: Tweak _bt_pgaddtup() comments. · 2110f716
      Peter Geoghegan authored
      Make it clear that _bt_pgaddtup() truncates the first data item on an
      internal page because its key is supposed to be treated as minus
      infinity within _bt_compare().
      2110f716
  7. 17 Nov, 2019 1 commit
    • Tom Lane's avatar
      Further fix dumping of views that contain just VALUES(...). · bf2efc55
      Tom Lane authored
      It turns out that commit e9f1c01b missed a case: we must print a
      VALUES clause in long format if get_query_def is given a resultDesc
      that would require the query's output column name(s) to be different
      from what the bare VALUES clause would produce.
      
      This applies in case an ALTER ... RENAME COLUMN has been done to
      a view that formerly could be printed in simple format, as shown
      in the added regression test case.  It also explains bug #16119
      from Dmitry Telpt, because it turns out that (unlike CREATE VIEW)
      CREATE MATERIALIZED VIEW fails to apply any column aliases it's
      given to the stored ON SELECT rule.  So to get them to be printed,
      we have to account for the resultDesc renaming.  It might be worth
      changing the matview code so that it creates the ON SELECT rule
      with the correct aliases; but we'd still need these messy checks in
      get_simple_values_rte to handle the case of a subsequent column
      rename, so any such change would be just neatnik-ism not a bug fix.
      
      Like the previous patch, back-patch to all supported branches.
      
      Discussion: https://postgr.es/m/16119-e64823f30a45a754@postgresql.org
      bf2efc55
  8. 16 Nov, 2019 3 commits