1. 21 Nov, 2019 5 commits
  2. 20 Nov, 2019 9 commits
  3. 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
  4. 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
  5. 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
  6. 16 Nov, 2019 4 commits
  7. 15 Nov, 2019 1 commit
  8. 14 Nov, 2019 4 commits
  9. 13 Nov, 2019 6 commits
    • Andres Freund's avatar
      Remove unused code from tuplesort. · 7d962eaf
      Andres Freund authored
      copytup_index() is unused, as tuplesort_putindextuplevalues() doesn't
      use COPYTUP(). Replace function body with an elog(ERROR), as already
      done e.g. for copytup_datum().
      
      Author: Andres Freund
      Discussion: https://postgr.es/m/20191013144153.ooxrfglvnaocsrx2@alap3.anarazel.de
      7d962eaf
    • Andres Freund's avatar
      Add tests for tuplesort.c. · 4a252996
      Andres Freund authored
      Previously significant parts of tuplesort.c were untested. This
      commit, while not testing every path, significantly increases
      coverage.  In particular, this adds tests for abbreviated key logic,
      forward/backward scans & scrolling and mark/restore.
      
      I tried to keep the table sizes reasonable, and stress the on-disk
      paths by setting work_mem to low values for specific tests. The
      buildfarm will tell whether more attention to test time is needed.
      
      Author: Andres Freund
      Discussion: https://postgr.es/m/20191013144153.ooxrfglvnaocsrx2@alap3.anarazel.de
      4a252996
    • Tom Lane's avatar
      Add missing check_collation_set call to bpcharne(). · d57d6153
      Tom Lane authored
      We should throw an error for indeterminate collation, but bpcharne()
      was missing that logic, resulting in a much less user-friendly error
      (either an assertion failure or "cache lookup failed for collation 0").
      
      Per report from Manuel Rigger.  Back-patch to v12 where the mistake
      came in, evidently in commit 5e1963fb.  (Before non-deterministic
      collations, this function wasn't collation sensitive.)
      
      Discussion: https://postgr.es/m/CA+u7OA4HOjtymxAbuGNh4-X_2R0Lw5n01tzvP8E5-i-2gQXYWA@mail.gmail.com
      d57d6153
    • Tom Lane's avatar
      Fix silly initializations (cosmetic only). · 0cafdd03
      Tom Lane authored
      Initializing a pointer to "false" isn't per project style,
      and reportedly some compilers warn about it (though I've
      not seen any such warnings in the buildfarm).
      
      Seems to have come in with commit ff11e7f4, so back-patch
      to v12 where that was added.
      
      Didier Gautheron
      
      Discussion: https://postgr.es/m/CAJRYxu+XQuM0qnSqt1Ujztu6fBPzMMAT3VEn6W32rgKG6A2Fsw@mail.gmail.com
      0cafdd03
    • Tom Lane's avatar
      Avoid using SplitIdentifierString to parse ListenAddresses, too. · 7bf40ea0
      Tom Lane authored
      This gets rid of our former behavior of forcibly downcasing
      the postmaster's hostname list and truncating the elements to
      NAMEDATALEN.  In principle, DNS hostnames are case-insensitive
      so the first behavior should be harmless, and server hostnames
      are seldom long enough for the second behavior to be an issue.
      But it's still dubious, and an easy fix is available: just use
      SplitGUCList instead.
      
      AFAICT, all other SplitIdentifierString calls in the backend are
      OK: either the items actually are SQL identifiers, or they are
      keywords that are short and case-insensitive.
      
      Per thinking about bug #16106.  While this has been wrong for
      a very long time, the lack of field complaints means there's
      little reason to back-patch.
      
      Discussion: https://postgr.es/m/16106-7d319e4295d08e70@postgresql.org
      7bf40ea0
    • Tom Lane's avatar
      Avoid downcasing/truncation of RADIUS authentication parameters. · 7618eaf5
      Tom Lane authored
      Commit 6b76f1bb changed all the RADIUS auth parameters to be lists
      rather than single values.  But its use of SplitIdentifierString
      to parse the list format was not very carefully thought through,
      because that function thinks it's parsing SQL identifiers, which
      means it will (a) downcase the strings and (b) truncate them to
      be shorter than NAMEDATALEN.  While downcasing should be harmless
      for the server names and ports, it's just wrong for the shared
      secrets, and probably for the NAS Identifier strings as well.
      The truncation aspect is at least potentially a problem too,
      though typical values for these parameters would fit in 63 bytes.
      
      Fortunately, we now have a function SplitGUCList that is exactly
      the same except for not doing the two unwanted things, so fixing
      this is a trivial matter of calling that function instead.
      
      While here, improve the documentation to show how to double-quote
      the parameter values.  I failed to resist the temptation to do
      some copy-editing as well.
      
      Report and patch from Marcos David (bug #16106); doc changes by me.
      Back-patch to v10 where the aforesaid commit came in, since this is
      arguably a regression from our previous behavior with RADIUS auth.
      
      Discussion: https://postgr.es/m/16106-7d319e4295d08e70@postgresql.org
      7618eaf5