1. 09 May, 2019 3 commits
    • Peter Eisentraut's avatar
      Fix grammar in error message · 02daece4
      Peter Eisentraut authored
      02daece4
    • Tom Lane's avatar
      Clean up the behavior and API of catalog.c's is-catalog-relation tests. · 2d7d946c
      Tom Lane authored
      The right way for IsCatalogRelation/Class to behave is to return true
      for OIDs less than FirstBootstrapObjectId (not FirstNormalObjectId),
      without any of the ad-hoc fooling around with schema membership.
      
      The previous code was wrong because (1) it claimed that
      information_schema tables were not catalog relations but their toast
      tables were, which is silly; and (2) if you dropped and recreated
      information_schema, which is a supported operation, the behavior
      changed.  That's even sillier.  With this definition, "catalog
      relations" are exactly the ones traceable to the postgres.bki data,
      which seems like what we want.
      
      With this simplification, we don't actually need access to the pg_class
      tuple to identify a catalog relation; we only need its OID.  Hence,
      replace IsCatalogClass with "IsCatalogRelationOid(oid)".  But keep
      IsCatalogRelation as a convenience function.
      
      This allows fixing some arguably-wrong semantics in contrib/sepgsql and
      ReindexRelationConcurrently, which were using an IsSystemNamespace test
      where what they really should be using is IsCatalogRelationOid.  The
      previous coding failed to protect toast tables of system catalogs, and
      also was not on board with the general principle that user-created tables
      do not become catalogs just by virtue of being renamed into pg_catalog.
      We can also get rid of a messy hack in ReindexMultipleTables.
      
      While we're at it, also rename IsSystemNamespace to IsCatalogNamespace,
      because the previous name invited confusion with the more expansive
      semantics used by IsSystemRelation/Class.
      
      Also improve the comments in catalog.c.
      
      There are a few remaining places in replication-related code that are
      special-casing OIDs below FirstNormalObjectId.  I'm inclined to think
      those are wrong too, and if there should be any special case it should
      just extend to FirstBootstrapObjectId.  But first we need to debate
      whether a FOR ALL TABLES publication should include information_schema.
      
      Discussion: https://postgr.es/m/21697.1557092753@sss.pgh.pa.us
      Discussion: https://postgr.es/m/15150.1557257111@sss.pgh.pa.us
      2d7d946c
    • Michael Paquier's avatar
      Fix error status of vacuumdb when multiple jobs are used · 3ae3c18b
      Michael Paquier authored
      When running a batch of VACUUM or ANALYZE commands on a given database,
      there were cases where it is possible to have vacuumdb not report an
      error where it actually should, leading to incorrect status results.
      
      Author: Julien Rouhaud
      Reviewed-by: Amit Kapila, Michael Paquier
      Discussion: https://postgr.es/m/CAOBaU_ZuTwz7CtqLYJ1Ouuh272bTQPLN8b1bAPk0bCBm4PDMTQ@mail.gmail.com
      Backpatch-through: 9.5
      3ae3c18b
  2. 08 May, 2019 9 commits
  3. 07 May, 2019 9 commits
  4. 06 May, 2019 5 commits
    • Bruce Momjian's avatar
      docs: fist draft version of the PG 12 release notes · bdf595ad
      Bruce Momjian authored
      Still needs text markup, links, word wrap, and indenting.
      bdf595ad
    • Alvaro Herrera's avatar
      Revert "Make pg_dump emit ATTACH PARTITION instead of PARTITION OF" · a1ec7402
      Alvaro Herrera authored
      ... and fallout (from branches 10, 11 and master).  The change was
      ill-considered, and it broke a few normal use cases; since we don't have
      time to fix it, we'll try again after this week's minor releases.
      
      Reported-by: Rushabh Lathia
      Discussion: https://postgr.es/m/CAGPqQf0iQV=PPOv2Btog9J9AwOQp6HmuVd6SbGTR_v3Zp2XT1w@mail.gmail.com
      a1ec7402
    • Michael Paquier's avatar
      Add tests for error message generation in partition tuple routing · 91248608
      Michael Paquier authored
      This adds extra tests for the error message generated for partition
      tuple routing in the executor, using more than three levels of
      partitioning including partitioned tables with no partitions.  These
      tests have been added to fix CVE-2019-10129 on REL_11_STABLE.  HEAD has
      no active bugs in this area, but it lacked coverage.
      
      Author: Michael Paquier
      Reviewed-by: Noah Misch
      Security: CVE-2019-10129
      91248608
    • Dean Rasheed's avatar
      Use checkAsUser for selectivity estimator checks, if it's set. · a0905056
      Dean Rasheed authored
      In examine_variable() and examine_simple_variable(), when checking the
      user's table and column privileges to determine whether to grant
      access to the pg_statistic data, use checkAsUser for the privilege
      checks, if it's set. This will be the case if we're accessing the
      table via a view, to indicate that we should perform privilege checks
      as the view owner rather than the current user.
      
      This change makes this planner check consistent with the check in the
      executor, so the planner will be able to make use of statistics if the
      table is accessible via the view. This fixes a performance regression
      introduced by commit e2d4ef8d, which affects queries against
      non-security barrier views in the case where the user doesn't have
      privileges on the underlying table, but the view owner does.
      
      Note that it continues to provide the same safeguards controlling
      access to pg_statistic for direct table access (in which case
      checkAsUser won't be set) and for security barrier views, because of
      the nearby checks on rte->security_barrier and rte->securityQuals.
      
      Back-patch to all supported branches because e2d4ef8d was.
      
      Dean Rasheed, reviewed by Jonathan Katz and Stephen Frost.
      a0905056
    • Dean Rasheed's avatar
      Fix security checks for selectivity estimation functions with RLS. · 1aebfbea
      Dean Rasheed authored
      In commit e2d4ef8d, security checks were added to prevent
      user-supplied operators from running over data from pg_statistic
      unless the user has table or column privileges on the table, or the
      operator is leakproof. For a table with RLS, however, checking for
      table or column privileges is insufficient, since that does not
      guarantee that the user has permission to view all of the column's
      data.
      
      Fix this by also checking for securityQuals on the RTE, and insisting
      that the operator be leakproof if there are any. Thus the
      leakproofness check will only be skipped if there are no securityQuals
      and the user has table or column privileges on the table -- i.e., only
      if we know that the user has access to all the data in the column.
      
      Back-patch to 9.5 where RLS was added.
      
      Dean Rasheed, reviewed by Jonathan Katz and Stephen Frost.
      
      Security: CVE-2019-10130
      1aebfbea
  5. 05 May, 2019 3 commits
    • Tom Lane's avatar
      Bring pg_nextoid()'s error messages into line with message style guide. · bd5e8b62
      Tom Lane authored
      Noticed while reviewing nearby code.  Given all the disclaimers about
      this not being meant as user-facing code, I wonder whether we should
      make these non-translatable?  But in any case there's little excuse
      for them not to be good English.
      bd5e8b62
    • Tom Lane's avatar
      Fix style violations in syscache lookups. · 9691aa72
      Tom Lane authored
      Project style is to check the success of SearchSysCacheN and friends
      by applying HeapTupleIsValid to the result.  A tiny minority of calls
      creatively did it differently.  Bring them into line with the rest.
      
      This is just cosmetic, since HeapTupleIsValid is indeed just a null
      check at the moment ... but that may not be true forever, and in any
      case it puts a mental burden on readers who may wonder why these
      call sites are not like the rest.
      
      Back-patch to v11 just to keep the branches in sync.  (The bulk of these
      errors seem to have originated in v11 or v12, though a few are old.)
      
      Per searching to see if anyplace else had made the same error
      repaired in 62148c35.
      9691aa72
    • Tom Lane's avatar
      Add check for syscache lookup failure in update_relispartition(). · 62148c35
      Tom Lane authored
      Omitted in commit 05b38c7e (though it looks like the original blame
      belongs to 9e9befac).  A failure is admittedly unlikely, but if it
      did happen, SIGSEGV is not the approved method of reporting it.
      
      Per Coverity.  Back-patch to v11 where the broken code originated.
      62148c35
  6. 04 May, 2019 3 commits
  7. 03 May, 2019 3 commits
  8. 02 May, 2019 4 commits
    • Tom Lane's avatar
      Fix reindexing of pg_class indexes some more. · f912d7de
      Tom Lane authored
      Commits 3dbb317d et al failed under CLOBBER_CACHE_ALWAYS testing.
      Investigation showed that to reindex pg_class_oid_index, we must
      suppress accesses to the index (via SetReindexProcessing) before we call
      RelationSetNewRelfilenode, or at least before we do CommandCounterIncrement
      therein; otherwise, relcache reloads happening within the CCI may try to
      fetch pg_class rows using the index's new relfilenode value, which is as
      yet an empty file.
      
      Of course, the point of 3dbb317d was that that ordering didn't work
      either, because then RelationSetNewRelfilenode's own update of the index's
      pg_class row cannot access the index, should it need to.
      
      There are various ways we might have got around that, but Andres Freund
      came up with a brilliant solution: for a mapped index, we can really just
      skip the pg_class update altogether.  The only fields it was actually
      changing were relpages etc, but it was just setting them to zeroes which
      is useless make-work.  (Correct new values will be installed at the end
      of index build.)  All pg_class indexes are mapped and probably always will
      be, so this eliminates the problem by removing work rather than adding it,
      always a pleasant outcome.  Having taught RelationSetNewRelfilenode to do
      it that way, we can revert the code reordering in reindex_index.  (But
      I left the moved setup code where it was; there seems no reason why it
      has to run without use of the old index.  If you're trying to fix a
      busted pg_class index, you'll have had to disable system index use
      altogether to get this far.)
      
      Moreover, this means we don't need RelationSetIndexList at all, because
      reindex_relation's hacking to make "REINDEX TABLE pg_class" work is
      likewise now unnecessary.  We'll leave that code in place in the back
      branches, but a follow-on patch will remove it in HEAD.
      
      In passing, do some minor cleanup for commit 5c156060 (in HEAD only),
      notably removing a duplicate newrnode assignment.
      
      Patch by me, using a core idea due to Andres Freund.  Back-patch to all
      supported branches, as 3dbb317d was.
      
      Discussion: https://postgr.es/m/28926.1556664156@sss.pgh.pa.us
      f912d7de
    • Alvaro Herrera's avatar
      heap_prepare_freeze_tuple: Simplify coding · 2bf372a4
      Alvaro Herrera authored
      Commit d2599ecf introduced some contorted, confused code around:
      readers would think that it's possible for HeapTupleHeaderGetXmin return
      a non-frozen value for some frozen tuples, which would be disastrous.
      There's no actual bug, but it seems better to make it clearer.
      
      Per gripe from Tom Lane and Andres Freund.
      Discussion: https://postgr.es/m/30116.1555430496@sss.pgh.pa.us
      2bf372a4
    • Peter Geoghegan's avatar
      Fix nbtsort.c's page space accounting. · 6dd86c26
      Peter Geoghegan authored
      Commit dd299df8, which made heap TID a tiebreaker nbtree index
      column, introduced new rules on page space management to make suffix
      truncation safe.  In general, suffix truncation needs to have a small
      amount of extra space available on the new left page when splitting a
      leaf page.  This is needed in case it turns out that truncation cannot
      even "truncate away the heap TID column", resulting in a
      larger-than-firstright leaf high key with an explicit heap TID
      representation.
      
      Despite all this, CREATE INDEX/nbtsort.c did not account for the
      possible need for extra heap TID space on leaf pages when deciding
      whether or not a new item could fit on current page.  This could lead to
      "failed to add item to the index page" errors when CREATE
      INDEX/nbtsort.c tried to finish off a leaf page that lacked space for a
      larger-than-firstright leaf high key (it only had space for firstright
      tuple, which was just short of what was needed following "truncation").
      
      Several conditions needed to be met all at once for CREATE INDEX to
      fail.  The problem was in the hard limit on what will fit on a page,
      which tends to be masked by the soft fillfactor-wise limit.  The easiest
      way to recreate the problem seems to be a CREATE INDEX on a low
      cardinality text column, with tuples that are of non-uniform width,
      using a fillfactor of 100.
      
      To fix, bring nbtsort.c in line with nbtsplitloc.c, which already
      pessimistically assumes that all leaf page splits will have high keys
      that have a heap TID appended.
      
      Reported-By: Andreas Joseph Krogh
      Discussion: https://postgr.es/m/VisenaEmail.c5.3ee7fe277d514162.16a6d785bea@tc7-visena
      6dd86c26
    • Robert Haas's avatar
      Fix some problems with VACUUM (INDEX_CLEANUP FALSE). · dd695979
      Robert Haas authored
      The new nleft_dead_tuples and nleft_dead_itemids fields are confusing
      and do not seem like the correct way forward.  One of them is tested
      via an assertion that can fail, as it has already done on buildfarm
      member topminnow.  Remove the assertion and the fields.
      
      Change the logic for the case where a tuple is not initially pruned
      by heap_page_prune but later diagnosed HEAPTUPLE_DEAD by
      HeapTupleSatisfiesVacuum.  Previously, tupgone = true was set in
      that case, which leads to treating the tuple as one that will be
      removed.  In a normal vacuum, that's OK, because we'll remove
      index entries for it and then the second heap pass will remove the
      tuple itself, but when index cleanup is disabled, those things
      don't happen, so we must instead treat it as a recently-dead
      tuple that we have voluntarily chosen to keep.
      
      Report and analysis by Tom Lane.  This patch loosely based on one
      from Masahiko Sawada, but I changed most of it.
      dd695979
  9. 01 May, 2019 1 commit