1. 04 Sep, 2020 3 commits
    • Bruce Momjian's avatar
      remove redundant initializations · e36e936e
      Bruce Momjian authored
      Reported-by: Ranier Vilela
      
      Discussion: https://postgr.es/m/CAEudQAo1+AcGppxDSg8k+zF4+Kv+eJyqzEDdbpDg58-=MQcerQ@mail.gmail.com
      
      Author: Ranier Vilela
      
      Backpatch-through: master
      e36e936e
    • Michael Paquier's avatar
      Remove variable "concurrent" from ReindexStmt · 844c05ab
      Michael Paquier authored
      This node already handles multiple options using a bitmask, so having a
      separate boolean flag is not necessary.  This simplifies the code a bit
      with less arguments to give to the reindex routines, by replacing the
      boolean with an equivalent bitmask value.
      
      Reviewed-by: Julien Rouhaud
      Discussion: https://postgr.es/m/20200902110326.GA14963@paquier.xyz
      844c05ab
    • Tom Lane's avatar
      Remove arbitrary restrictions on password length. · 67a472d7
      Tom Lane authored
      This patch started out with the goal of harmonizing various arbitrary
      limits on password length, but after awhile a better idea emerged:
      let's just get rid of those fixed limits.
      
      recv_password_packet() has an arbitrary limit on the packet size,
      which we don't really need, so just drop it.  (Note that this doesn't
      really affect anything for MD5 or SCRAM password verification, since
      those will hash the user's password to something shorter anyway.
      It does matter for auth methods that require a cleartext password.)
      
      Likewise remove the arbitrary error condition in pg_saslprep().
      
      The remaining limits are mostly in client-side code that prompts
      for passwords.  To improve those, refactor simple_prompt() so that
      it allocates its own result buffer that can be made as big as
      necessary.  Actually, it proves best to make a separate routine
      pg_get_line() that has essentially the semantics of fgets(), except
      that it allocates a suitable result buffer and hence will never
      return a truncated line.  (pg_get_line has a lot of potential
      applications to replace randomly-sized fgets buffers elsewhere,
      but I'll leave that for another patch.)
      
      I built pg_get_line() atop stringinfo.c, which requires moving
      that code to src/common/; but that seems fine since it was a poor
      fit for src/port/ anyway.
      
      This patch is mostly mine, but it owes a good deal to Nathan Bossart
      who pressed for a solution to the password length problem and
      created a predecessor patch.  Also thanks to Peter Eisentraut and
      Stephen Frost for ideas and discussion.
      
      Discussion: https://postgr.es/m/09512C4F-8CB9-4021-B455-EF4C4F0D55A0@amazon.com
      67a472d7
  2. 03 Sep, 2020 7 commits
  3. 02 Sep, 2020 7 commits
  4. 01 Sep, 2020 6 commits
    • Tom Lane's avatar
      Improve test coverage of ginvacuum.c. · 4c51a2d1
      Tom Lane authored
      Add a test case that exercises vacuum's deletion of empty GIN
      posting pages.  Since this is a temp table, it should now work
      reliably to delete a bunch of rows and immediately VACUUM.
      Before the preceding commit, this would not have had the desired
      effect, at least not in parallel regression tests.
      
      Discussion: https://postgr.es/m/3490536.1598629609@sss.pgh.pa.us
      4c51a2d1
    • Tom Lane's avatar
      Set cutoff xmin more aggressively when vacuuming a temporary table. · a7212be8
      Tom Lane authored
      Since other sessions aren't allowed to look into a temporary table
      of our own session, we do not need to worry about the global xmin
      horizon when setting the vacuum XID cutoff.  Indeed, if we're not
      inside a transaction block, we may set oldestXmin to be the next
      XID, because there cannot be any in-doubt tuples in a temp table,
      nor any tuples that are dead but still visible to some snapshot of
      our transaction.  (VACUUM, of course, is never inside a transaction
      block; but we need to test that because CLUSTER shares the same code.)
      
      This approach allows us to always clean out a temp table completely
      during VACUUM, independently of concurrent activity.  Aside from
      being useful in its own right, that simplifies building reproducible
      test cases.
      
      Discussion: https://postgr.es/m/3490536.1598629609@sss.pgh.pa.us
      a7212be8
    • Bruce Momjian's avatar
      doc: clarify that max_wal_size is "during" checkpoints · db864c3c
      Bruce Momjian authored
      Previous wording was "between".
      
      Reported-by: Pavel Luzanov
      
      Discussion: https://postgr.es/m/26906a54-d7cb-2f8e-eed7-e31660024694@postgrespro.ru
      
      Backpatch-through: 9.5
      db864c3c
    • Alvaro Herrera's avatar
      Raise error on concurrent drop of partitioned index · afc7e0ad
      Alvaro Herrera authored
      We were already raising an error for DROP INDEX CONCURRENTLY on a
      partitioned table, albeit a different and confusing one:
        ERROR:  DROP INDEX CONCURRENTLY must be first action in transaction
      
      Change that to throw a more comprehensible error:
        ERROR:  cannot drop partitioned index \"%s\" concurrently
      
      Michael Paquier authored the test case for indexes on temporary
      partitioned tables.
      
      Backpatch to 11, where indexes on partitioned tables were added.
      Reported-by: default avatarJan Mussler <jan.mussler@zalando.de>
      Reviewed-by: default avatarMichael Paquier <michael@paquier.xyz>
      Discussion: https://postgr.es/m/16594-d2956ca909585067@postgresql.org
      afc7e0ad
    • Tom Lane's avatar
      Teach libpq to handle arbitrary-length lines in .pgpass files. · b55b4dad
      Tom Lane authored
      Historically there's been a hard-wired assumption here that no line of
      a .pgpass file could be as long as NAMEDATALEN*5 bytes.  That's a bit
      shaky to start off with, because (a) there's no reason to suppose that
      host names fit in NAMEDATALEN, and (b) this figure fails to allow for
      backslash escape characters.  However, it fails completely if someone
      wants to use a very long password, and we're now hearing reports of
      people wanting to use "security tokens" that can run up to several
      hundred bytes.  Another angle is that the file is specified to allow
      comment lines, but there's no reason to assume that long comment lines
      aren't possible.
      
      Rather than guessing at what might be a more suitable limit, let's
      replace the fixed-size buffer with an expansible PQExpBuffer.  That
      adds one malloc/free cycle to the typical use-case, but that's surely
      pretty cheap relative to the I/O this code has to do.
      
      Also, add TAP test cases to exercise this code, because there was no
      test coverage before.
      
      This reverts most of commit 2eb3bc58, as there's no longer a need for
      a warning message about overlength .pgpass lines.  (I kept the explicit
      check for comment lines, though.)
      
      In HEAD and v13, this also fixes an oversight in 74a308cf: there's not
      much point in explicit_bzero'ing the line buffer if we only do so in two
      of the three exit paths.
      
      Back-patch to all supported branches, except that the test case only
      goes back to v10 where src/test/authentication/ was added.
      
      Discussion: https://postgr.es/m/4187382.1598909041@sss.pgh.pa.us
      b55b4dad
    • Amit Kapila's avatar
      Fix the SharedFileSetUnregister API. · 4ab77697
      Amit Kapila authored
      Commit 808e13b2 introduced a few APIs to extend the existing Buffile
      interface. In SharedFileSetDeleteOnProcExit, it tries to delete the list
      element while traversing the list with 'foreach' construct which makes the
      behavior of list traversal unpredictable.
      
      Author: Amit Kapila
      Reviewed-by: Dilip Kumar
      Tested-by: Dilip Kumar and Neha Sharma
      Discussion: https://postgr.es/m/CAA4eK1JhLatVcQ2OvwA_3s0ih6Hx9+kZbq107cXVsSWWukH7vA@mail.gmail.com
      4ab77697
  5. 31 Aug, 2020 13 commits
  6. 30 Aug, 2020 3 commits
    • Tom Lane's avatar
      Mark factorial operator, and postfix operators in general, as deprecated. · 6ca547cf
      Tom Lane authored
      Per discussion, we're planning to remove parser support for postfix
      operators in order to simplify the grammar.  So it behooves us to
      put out a deprecation notice at least one release before that.
      
      There is only one built-in postfix operator, ! for factorial.
      Label it deprecated in the docs and in pg_description, and adjust
      some examples that formerly relied on it.  (The sister prefix
      operator !! is also deprecated.  We don't really have to remove
      that one, but since we're suggesting that people use factorial()
      instead, it seems better to remove both operators.)
      
      Also state in the CREATE OPERATOR ref page that postfix operators
      in general are going away.
      
      Although this changes the initial contents of pg_description,
      I did not force a catversion bump; it doesn't seem essential.
      
      In v13, also back-patch 4c5cf543, so that there's someplace for
      the <link>s to point to.
      
      Mark Dilger and John Naylor, with some adjustments by me
      
      Discussion: https://postgr.es/m/BE2DF53D-251A-4E26-972F-930E523580E9@enterprisedb.com
      6ca547cf
    • Tom Lane's avatar
      Redefine pg_class.reltuples to be -1 before the first VACUUM or ANALYZE. · 3d351d91
      Tom Lane authored
      Historically, we've considered the state with relpages and reltuples
      both zero as indicating that we do not know the table's tuple density.
      This is problematic because it's impossible to distinguish "never yet
      vacuumed" from "vacuumed and seen to be empty".  In particular, a user
      cannot use VACUUM or ANALYZE to override the planner's normal heuristic
      that an empty table should not be believed to be empty because it is
      probably about to get populated.  That heuristic is a good safety
      measure, so I don't care to abandon it, but there should be a way to
      override it if the table is indeed intended to stay empty.
      
      Hence, represent the initial state of ignorance by setting reltuples
      to -1 (relpages is still set to zero), and apply the minimum-ten-pages
      heuristic only when reltuples is still -1.  If the table is empty,
      VACUUM or ANALYZE (but not CREATE INDEX) will override that to
      reltuples = relpages = 0, and then we'll plan on that basis.
      
      This requires a bunch of fiddly little changes, but we can get rid of
      some ugly kluges that were formerly needed to maintain the old definition.
      
      One notable point is that FDWs' GetForeignRelSize methods will see
      baserel->tuples = -1 when no ANALYZE has been done on the foreign table.
      That seems like a net improvement, since those methods were formerly
      also in the dark about what baserel->tuples = 0 really meant.  Still,
      it is an API change.
      
      I bumped catversion because code predating this change would get confused
      by seeing reltuples = -1.
      
      Discussion: https://postgr.es/m/F02298E0-6EF4-49A1-BCB6-C484794D9ACC@thebuild.com
      3d351d91
    • Michael Paquier's avatar
      Reset indisreplident for an invalid index in DROP INDEX CONCURRENTLY · 9511fb37
      Michael Paquier authored
      A failure when dropping concurrently an index used in a replica identity
      could leave in pg_index an index marked as !indisvalid and
      indisreplident.  Reindexing this index would switch back indisvalid to
      true, and if the replica identity of the parent relation was switched to
      use a different index, it would be possible to finish with more than one
      index marked as indisreplident.  If that were to happen, this could mess
      up with the relation cache as an incorrect index could be used for the
      replica identity.
      
      Indexes marked as invalid are discarded as candidates for the replica
      identity, as of RelationGetIndexList(), so similarly to what is done
      with indisclustered, resetting indisreplident when the index is marked
      as invalid keeps things consistent.  REINDEX CONCURRENTLY's swapping
      already resets the flag for the old index, while the new index inherits
      the value of the old index to-be-dropped, so only DROP INDEX was an
      issue.
      
      Even if this is a bug, the sequence able to reproduce a problem requires
      a failure while running DROP INDEX CONCURRENTLY, something unlikely
      going to happen in the field, so no backpatch is done.
      
      Author: Michael Paquier
      Reviewed-by: Dmitry Dolgov
      Discussion: https://postgr.es/m/20200827025721.GN2017@paquier.xyz
      9511fb37
  7. 28 Aug, 2020 1 commit
    • Michael Paquier's avatar
      doc: Rework tables for built-in operator classes of index AMs · 7a1cd526
      Michael Paquier authored
      The tables listing all the operator classes available for BRIN, GIN,
      GiST and SP-GiST had a confusing format where the same operator could be
      listed multiple times, for different data types.  This improves the
      shape of these tables by adding the types associated to each operator,
      for their associated operator class.
      
      Each table included previously the data type that could be used for an
      operator class in an extra column.  This is removed to reduce the width
      of the tables as this is now described within each operator.  This also
      makes the tables fit better in the PDF documentation.
      
      Reported-by: osdba
      Author: Michael Paquier
      Reviewed-by: Álvaro Herrera, Tom Lane, Bruce Momjian
      Discussion: https://postgr.es/m/38d55061.9604.173b32c60ec.Coremail.mailtch@163.com
      7a1cd526