1. 02 Jan, 2020 11 commits
    • Peter Geoghegan's avatar
      Correct _bt_delitems_vacuum() lock comments. · 44e44bd2
      Peter Geoghegan authored
      The expectation within _bt_delitems_vacuum() is that caller has a
      super-exclusive/cleanup buffer lock (not just a pin and a write lock).
      44e44bd2
    • Alvaro Herrera's avatar
      Fix cloning of row triggers to sub-partitions · 1fa846f1
      Alvaro Herrera authored
      When row triggers exist in partitioned partitions that are not either
      part of FKs or deferred unique constraints, they are not correctly
      cloned to their partitions.  That's because they are marked "internal",
      and those are purposefully skipped when doing the clone triggers dance.
      Fix by relaxing the condition on which internal triggers are skipped.
      
      Amit Langote initially diagnosed the problem and proposed a fix, but I
      used a different approach.
      
      Reported-by: Petr Fedorov
      Discussion: https://postgr.es/m/6b3f0646-ba8c-b3a9-c62d-1c6651a1920f@phystech.edu
      1fa846f1
    • Tom Lane's avatar
      Add basic TAP tests for psql's tab-completion logic. · 7c015045
      Tom Lane authored
      Up to now, psql's tab-complete.c has had exactly no regression test
      coverage.  This patch is an experimental attempt to add some.
      
      This needs Perl's IO::Pty module, which isn't installed everywhere,
      so the test script just skips all tests if that's not present.
      There may be other portability gotchas too, so I await buildfarm
      results with interest.
      
      So far this just covers a few very basic keyword-completion and
      query-driven-completion scenarios, which should be enough to let us
      get a feel for whether this is practical at all from a portability
      standpoint.  If it is, there's lots more that can be done.
      
      Discussion: https://postgr.es/m/10967.1577562752@sss.pgh.pa.us
      7c015045
    • Tom Lane's avatar
      Fix typmod exposed for scalar function in FROM, too. · 915c04f0
      Tom Lane authored
      On further reflection about commit 4d02eb01, it occurs to me that
      expandRTE() had better agree with what addRangeTableEntryForFunction()
      is doing.  So teach that about functions possibly having typmods, too.
      915c04f0
    • Peter Geoghegan's avatar
      Reorder two nbtree.h function prototypes. · a412f469
      Peter Geoghegan authored
      Make the function prototype order consistent with the definition order
      in nbtpage.c.
      a412f469
    • Tom Lane's avatar
      Fix collation exposed for scalar function in FROM. · 4d02eb01
      Tom Lane authored
      One code path in addRangeTableEntryForFunction() neglected to assign
      a collation to the tupdesc entry it constructs (which is a bit odd
      considering the other path did do so).  This didn't matter before commit
      5815696b, because nothing would look at the type data in this tupdesc;
      but now it does.
      
      While at it, make sure we assign the correct typmod as well.  Most
      function expressions don't have a determinate typmod, but some do.
      
      Per buildfarm, which showed failures in non-C collations, a case
      I'd not thought to test for this patch :-(
      4d02eb01
    • Tom Lane's avatar
      Make parser rely more heavily on the ParseNamespaceItem data structure. · 5815696b
      Tom Lane authored
      When I added the ParseNamespaceItem data structure (in commit 5ebaaa49),
      it wasn't very tightly integrated into the parser's APIs.  In the wake of
      adding p_rtindex to that struct (commit b541e9ac), there is a good reason
      to make more use of it: by passing around ParseNamespaceItem pointers
      instead of bare RTE pointers, we can get rid of various messy methods for
      passing back or deducing the rangetable index of an RTE during parsing.
      Hence, refactor the addRangeTableEntryXXX functions to build and return
      a ParseNamespaceItem struct, not just the RTE proper; and replace
      addRTEtoQuery with addNSItemToQuery, which is passed a ParseNamespaceItem
      rather than building one internally.
      
      Also, add per-column data (a ParseNamespaceColumn array) to each
      ParseNamespaceItem.  These arrays are built during addRangeTableEntryXXX,
      where we have column type data at hand so that it's nearly free to fill
      the data structure.  Later, when we need to build Vars referencing RTEs,
      we can use the ParseNamespaceColumn info to avoid the rather expensive
      operations done in get_rte_attribute_type() or expandRTE().
      get_rte_attribute_type() is indeed dead code now, so I've removed it.
      This makes for a useful improvement in parse analysis speed, around 20%
      in one moderately-complex test query.
      
      The ParseNamespaceColumn structs also include Var identity information
      (varno/varattno).  That info isn't actually being used in this patch,
      except that p_varno == 0 is a handy test for a dropped column.
      A follow-on patch will make more use of it.
      
      Discussion: https://postgr.es/m/2461.1577764221@sss.pgh.pa.us
      5815696b
    • Peter Eisentraut's avatar
      Fix comment in test · 198c7153
      Peter Eisentraut authored
      The comment was apparently copy-and-pasted and did not reflect the
      actual test outcome.
      198c7153
    • Amit Kapila's avatar
      Fix running out of file descriptors for spill files. · d2070380
      Amit Kapila authored
      Currently while decoding changes, if the number of changes exceeds a
      certain threshold, we spill those to disk.  And this happens for each
      (sub)transaction.  Now, while reading all these files, we don't close them
      until we read all the files.  While reading these files, if the number of
      such files exceeds the maximum number of file descriptors, the operation
      errors out.
      
      Use PathNameOpenFile interface to open these files as that internally has
      the mechanism to release kernel FDs as needed to get us under the
      max_safe_fds limit.
      
      Reported-by: Amit Khandekar
      Author: Amit Khandekar
      Reviewed-by: Amit Kapila
      Backpatch-through: 9.4
      Discussion: https://postgr.es/m/CAJ3gD9c-sECEn79zXw4yBnBdOttacoE-6gAyP0oy60nfs_sabQ@mail.gmail.com
      d2070380
    • Peter Geoghegan's avatar
      Revise BTP_HAS_GARBAGE nbtree VACUUM comments. · 4b25f5d0
      Peter Geoghegan authored
      _bt_delitems_vacuum() comments claimed that it isn't worth another scan
      of the page to avoid falsely unsetting the BTP_HAS_GARBAGE page flag
      hint (this happens to be the same wording that was removed from
      _bt_delitems_delete() by my recent commit fe97c61c).  The comments made
      little sense, though.  The issue can't have much to do with performing a
      second scan of the target leaf page, since an LP_DEAD test could easily
      be performed in the first scan of the page anyway (the scan that takes
      place in btvacuumpage() caller).
      
      Revise the explanation.  It makes much more sense to frame this as an
      issue about recovery conflicts.  _bt_delitems_vacuum() cannot easily
      generate an XID cutoff in the same way that _bt_delitems_delete() is
      designed to.
      
      Falsely unsetting the page flag is not ideal, and is likely to happen
      more often than was supposed by the original comments.  Explain why it
      usually isn't a problem in practice.  There may be an argument for
      _bt_delitems_vacuum() not clearing the BTP_HAS_GARBAGE bit, removing the
      question of it being falsely unset by VACUUM (there may even be an
      argument for not using a page level hint at all).  This can be revisited
      later.
      4b25f5d0
    • Tom Lane's avatar
      Test GROUP BY matching of join columns that are type-coerced by USING. · 823e739d
      Tom Lane authored
      If we have, say, an int column that is left-joined to a bigint column
      with USING, the merged column is the int column promoted to bigint.
      GROUP BY's tests for whether grouping on the merged column allows a
      reference to the underlying column, or vice versa, should know about
      that relationship --- and they do.  But I nearly broke this case with
      an ill-advised optimization, so the lack of any test coverage for it
      seems like a bad idea.
      823e739d
  2. 01 Jan, 2020 2 commits
    • Peter Geoghegan's avatar
      Update btree_xlog_delete() comments. · c5f3b53b
      Peter Geoghegan authored
      Commit fe97c61c updated LP_DEAD item deletion comments, but missed a
      minor discrepancy on the REDO side.  Fix it now.
      
      In passing, don't talk about the btree_xlog_vacuum() behavior within
      btree_xlog_delete().  The reliance on XLOG_HEAP2_CLEANUP_INFO records
      for recovery conflicts is already discussed within btvacuumpage() and
      mentioned again in passing above btree_xlog_vacuum(), which seems
      sufficient.
      c5f3b53b
    • Bruce Momjian's avatar
      Update copyrights for 2020 · 7559d8eb
      Bruce Momjian authored
      Backpatch-through: update all files in master, backpatch legal files through 9.4
      7559d8eb
  3. 31 Dec, 2019 1 commit
    • Peter Eisentraut's avatar
      Modernize Python exception syntax in documentation · b55413d7
      Peter Eisentraut authored
      Change the exception syntax used in the documentation to use the more
      current
      
          except Exception as ex:
      
      rather than the old
      
          except Exception, ex:
      
      We keep the old syntax in the test code since Python <2.6 is still
      supported there, but the documentation might as well use the modern
      syntax.
      b55413d7
  4. 28 Dec, 2019 1 commit
    • Tom Lane's avatar
      Micro-optimize AllocSetFreeIndex() by reference to pg_bitutils code. · 0ce38730
      Tom Lane authored
      Use __builtin_clz() where available.  Where it isn't, we can still win
      a little by using the pg_leftmost_one_pos[] lookup table instead of
      having a private table.
      
      Also drop the initial right shift by ALLOC_MINBITS in favor of
      subtracting ALLOC_MINBITS from the leftmost-one-pos result.  This
      is a win because the compiler can fold that adjustment into other
      constants it'd have to add anyway, making the shift-removal free.
      
      Also, we can explain this coding as an unrolled form of
      pg_leftmost_one_pos32(), even though that's a bit ahistorical
      since it long predates pg_bitutils.h.
      
      John Naylor, with some cosmetic adjustments by me
      
      Discussion: https://postgr.es/m/CACPNZCuNUGMxjK7WTn_=WZnRbfASDdBxmjsVf2+m9MdmeNw_sg@mail.gmail.com
      0ce38730
  5. 27 Dec, 2019 4 commits
  6. 26 Dec, 2019 7 commits
  7. 25 Dec, 2019 4 commits
    • Tom Lane's avatar
      Allow whole-row Vars to be used in partitioning expressions. · bb4114a4
      Tom Lane authored
      In the wake of commit 5b931237, there's no particular reason
      for this restriction (previously, it was problematic because of
      the implied rowtype reference).  A simple constraint on a whole-row
      Var probably isn't that useful, but conceivably somebody would want
      to pass one to a function that extracts a partitioning key.  Besides
      which, we're expending much more code to enforce the restriction than
      we save by having it, since the latter quantity is now zero.
      So drop the restriction.
      
      Amit Langote
      
      Discussion: https://postgr.es/m/CA+HiwqFUzjfj9HEsJtYWcr1SgQ_=iCAvQ=O2Sx6aQxoDu4OiHw@mail.gmail.com
      bb4114a4
    • Tom Lane's avatar
      Remove equalPartitionDescs(). · 42f74f49
      Tom Lane authored
      This is dead code in the wake of the previous commit.
      We can always add it back if we need it again someday.
      
      Discussion: https://postgr.es/m/CA+HiwqFUzjfj9HEsJtYWcr1SgQ_=iCAvQ=O2Sx6aQxoDu4OiHw@mail.gmail.com
      42f74f49
    • Tom Lane's avatar
      Load relcache entries' partitioning data on-demand, not immediately. · 5b931237
      Tom Lane authored
      Formerly the rd_partkey and rd_partdesc data structures were always
      populated immediately when a relcache entry was built or rebuilt.
      This patch changes things so that they are populated only when they
      are first requested.  (Hence, callers *must* now always use
      RelationGetPartitionKey or RelationGetPartitionDesc; just fetching
      the pointer directly is no longer acceptable.)
      
      This seems to have some performance benefits, but the main reason to do
      it is that it eliminates a recursive-reload failure that occurs if the
      partkey or partdesc expressions contain any references to the relation's
      rowtype (as discovered by Amit Langote).  In retrospect, since loading
      these data structures might result in execution of nearly-arbitrary code
      via eval_const_expressions, it was a dumb idea to require that to happen
      during relcache entry rebuild.
      
      Also, fix things so that old copies of a relcache partition descriptor
      will be dropped when the cache entry's refcount goes to zero.  In the
      previous coding it was possible for such copies to survive for the
      lifetime of the session, as I'd complained of in a previous discussion.
      (This management technique still isn't perfect, but it's better than
      before.)  Improve the commentary explaining how that works and why
      it's safe to hand out direct pointers to these relcache substructures.
      
      In passing, improve RelationBuildPartitionDesc by using the same
      memory-context-parent-swap approach used by RelationBuildPartitionKey,
      thereby making it less dependent on strong assumptions about what
      partition_bounds_copy does.  Avoid doing get_rel_relkind in the
      critical section, too.
      
      Patch by Amit Langote and Tom Lane; Robert Haas deserves some credit
      for prior work in the area, too.  Although this is a pre-existing
      problem, no back-patch: the patch seems too invasive to be safe to
      back-patch, and the bug it fixes is a corner case that seems
      relatively unlikely to cause problems in the field.
      
      Discussion: https://postgr.es/m/CA+HiwqFUzjfj9HEsJtYWcr1SgQ_=iCAvQ=O2Sx6aQxoDu4OiHw@mail.gmail.com
      Discussion: https://postgr.es/m/CA+TgmoY3bRmGB6-DUnoVy5fJoreiBJ43rwMrQRCdPXuKt4Ykaw@mail.gmail.com
      5b931237
    • Michael Paquier's avatar
      Rename files and headers related to index AM · 8ce3aa9b
      Michael Paquier authored
      The following renaming is done so as source files related to index
      access methods are more consistent with table access methods (the
      original names used for index AMs ware too generic, and could be
      confused as including features related to table AMs):
      - amapi.h -> indexam.h.
      - amapi.c -> indexamapi.c.  Here we have an equivalent with
      backend/access/table/tableamapi.c.
      - amvalidate.c -> indexamvalidate.c.
      - amvalidate.h -> indexamvalidate.h.
      - genam.c -> indexgenam.c.
      - genam.h -> indexgenam.h.
      
      This has been discussed during the development of v12 when table AM was
      worked on, but the renaming never happened.
      
      Author: Michael Paquier
      Reviewed-by: Fabien Coelho, Julien Rouhaud
      Discussion: https://postgr.es/m/20191223053434.GF34339@paquier.xyz
      8ce3aa9b
  8. 24 Dec, 2019 3 commits
    • Alvaro Herrera's avatar
      Avoid splitting C string literals with \-newline · c4dcd914
      Alvaro Herrera authored
      Using \ is unnecessary and ugly, so remove that.  While at it, stitch
      the literals back into a single line: we've long discouraged splitting
      error message literals even when they go past the 80 chars line limit,
      to improve greppability.
      
      Leave contrib/tablefunc alone.
      
      Discussion: https://postgr.es/m/20191223195156.GA12271@alvherre.pgsql
      c4dcd914
    • Michael Paquier's avatar
      Replace use of strerror() with %s by %m in pg_waldump · cce64a51
      Michael Paquier authored
      Since d6c55de1, src/port/snprintf.c is able to use %m instead of
      strerror().  A couple of utilities in src/bin/ have already done the
      switch, and do it now for pg_waldump as this reduces the workload for
      translators.
      
      Note that more could be done, particularly with pgbench.  Thanks to
      Kyotaro Horiguchi for the discussion.
      
      Discussion: https://postgr.es/m/20191129065115.GM2505@paquier.xyz
      cce64a51
    • Thomas Munro's avatar
      Rotate instead of shifting hash join batch number. · e69d6445
      Thomas Munro authored
      Our algorithm for choosing batch numbers turned out not to work
      effectively for multi-billion key inner relations.  We would use
      more hash bits than we have, and effectively concentrate all tuples
      into a smaller number of batches than we intended.  While ideally
      we should switch to wider hashes, for now, change the algorithm to
      one that effectively gives up bits from the bucket number when we
      don't have enough bits.  That means we'll finish up with longer
      bucket chains than would be ideal, but that's better than having
      batches that don't fit in work_mem and can't be divided.
      
      Batch-patch to all supported releases.
      
      Author: Thomas Munro
      Reviewed-by: Tom Lane, thanks also to Tomas Vondra, Alvaro Herrera, Andres Freund for testing and discussion
      Reported-by: James Coleman
      Discussion: https://postgr.es/m/16104-dc11ed911f1ab9df%40postgresql.org
      e69d6445
  9. 23 Dec, 2019 6 commits
    • Joe Conway's avatar
      Disallow null category in crosstab_hash · d5b9c2ba
      Joe Conway authored
      While building a hash map of categories in load_categories_hash,
      resulting category names have not thus far been checked to ensure
      they are not null. Prior to pg12 null category names worked to the
      extent that they did not crash on some platforms. This is because
      those system libraries have an snprintf which can deal with being
      passed a null pointer argument for a string. But even in those cases
      null categories did nothing useful. And on some platforms it crashed.
      As of pg12, our own version of snprintf gets called, and it does
      not deal with null pointer arguments at all, and crashes consistently.
      
      Fix that by disallowing null categories. They never worked usefully,
      and no one has ever asked for them to work previously. Back-patch to
      all supported branches.
      
      Reported-By: Ireneusz Pluta
      Discussion: https://postgr.es/m/16176-7489719b05e4303c@postgresql.org
      d5b9c2ba
    • Tom Lane's avatar
      Disallow partition key expressions that return pseudo-types. · 39ebb943
      Tom Lane authored
      This wasn't checked originally, but it should have been, because
      in general pseudo-types can't be stored to and retrieved from disk.
      Notably, partition bound values of type "record" would not be
      interpretable by another session.
      
      In v12 and HEAD, add another flag to CheckAttributeType's repertoire
      so that it can produce a specific error message for this case.  That's
      infeasible in older branches without an ABI break, so fall back to
      a slightly-less-nicely-worded error message in v10 and v11.
      
      Problem noted by Amit Langote, though this patch is not his initial
      solution.  Back-patch to v10 where partitioning was introduced.
      
      Discussion: https://postgr.es/m/CA+HiwqFUzjfj9HEsJtYWcr1SgQ_=iCAvQ=O2Sx6aQxoDu4OiHw@mail.gmail.com
      39ebb943
    • Tom Lane's avatar
      Prevent a rowtype from being included in itself via a range. · fc769589
      Tom Lane authored
      We probably should have thought of this case when ranges were added,
      but we didn't.  (It's not the fault of commit eb51af71, because
      ranges didn't exist then.)
      
      It's an old bug, so back-patch to all supported branches.
      
      Discussion: https://postgr.es/m/7782.1577051475@sss.pgh.pa.us
      fc769589
    • Alvaro Herrera's avatar
      GetPublicationByName: Don't repeat ourselves · 0fd8cfb2
      Alvaro Herrera authored
      Use get_publication_oid() instead of reimplementing it.
      
      Discussion: https://postgr.es/m/20191220201017.GA17292@alvherre.pgsql
      0fd8cfb2
    • Peter Geoghegan's avatar
      Normalize _bt_finish_split() argument names. · 696cc3a0
      Peter Geoghegan authored
      Make a function prototype argument's name match the function
      definition's argument name.
      696cc3a0
    • Peter Geoghegan's avatar
      Update nbtree LP_DEAD item deletion comments. · fe97c61c
      Peter Geoghegan authored
      Comments about the consequences of clearing the BTP_HAS_GARBAGE page
      flag bit that apply only to VACUUM were added to code that deals with
      opportunistic deletion of LP_DEAD items by commit a760893d.  The same
      comment block was added to both _bt_delitems_vacuum() and
      _bt_delitems_delete().  Correct _bt_delitems_delete()'s copy of the
      comment block.
      
      _bt_delitems_delete() reliably deletes items that were found by caller
      to have their LP_DEAD bit set.  There is no question about whether or
      not unsetting the BTP_HAS_GARBAGE bit can miss some LP_DEAD items that
      were set recently.
      
      Also tweak a related section of the nbtree README.
      fe97c61c
  10. 22 Dec, 2019 1 commit
    • Tom Lane's avatar
      Avoid low-probability regression test failures in timestamp[tz] tests. · b265aa1f
      Tom Lane authored
      If the first transaction block in these tests were entered exactly
      at midnight (California time), they'd report a bogus failure due
      to 'now' and 'midnight' having the same values.  Commit 8c2ac75c
      had dismissed this as being of negligible probability, but we've
      now seen it happen in the buildfarm, so let's prevent it.  We can
      get pretty much the same test coverage without an it's-not-midnight
      assumption by moving the does-'now'-work cases into their own test step.
      
      While here, apply commit 47169c25's s/DELETE/TRUNCATE/ change to
      timestamptz as well as timestamp (not sure why that didn't
      occur to me at the time; the risk of failure is the same).
      
      Back-patch to all supported branches, since the main point is
      to get rid of potential buildfarm failures.
      
      Discussion: https://postgr.es/m/14821.1577031117@sss.pgh.pa.us
      b265aa1f