1. 09 May, 2019 7 commits
    • Etsuro Fujita's avatar
      Doc: Update FDW documentation about GetForeignUpperPaths(). · a0be05ba
      Etsuro Fujita authored
      In commit d50d172e, which added support for LIMIT/OFFSET pushdown in
      postgres_fdw, a new struct was introduced as the extra parameter of
      GetForeignUpperPaths() set for UPPERREL_FINAL, but I forgot to update
      the documentation to mention that.
      
      Author: Etsuro Fujita
      Discussion: https://postgr.es/m/CAPmGK17uSXQDe31oRb-z1nYyT6vVzkstZkA3_Wbq38U92b9BmQ%40mail.gmail.com
      a0be05ba
    • Etsuro Fujita's avatar
      postgres_fdw: Fix cost estimation for aggregate pushdown. · edbcbe27
      Etsuro Fujita authored
      In commit 7012b132, which added support for aggregate pushdown in
      postgres_fdw, the expense of evaluating the final scan/join target
      computed by make_group_input_target() was not accounted for at all in
      costing aggregate pushdown paths with local statistics.  The right fix
      for this would be to have a separate upper stage to adjust the final
      scan/join relation (see comments for apply_scanjoin_target_to_paths());
      but for now, fix by adding the tlist eval cost when costing aggregate
      pushdown paths with local statistics.
      
      Apply this to HEAD only to avoid destabilizing existing plan choices.
      
      Author: Etsuro Fujita
      Reviewed-By: Antonin Houska
      Discussion: https://postgr.es/m/5C66A056.60007%40lab.ntt.co.jp
      edbcbe27
    • Thomas Munro's avatar
      Fix SxactGlobalXmin tracking. · 47a338cf
      Thomas Munro authored
      Commit bb16aba5 broke the code that maintains SxactGlobalXmin.  It
      could get stuck when a well-timed READ ONLY transaction runs.  If
      SxactGlobalXmin stops advancing, transactions on the
      FinishedSerializableTransactions queue are never cleaned up, so
      resources are effectively leaked.  Revert that hunk of the commit.
      
      Also revert another similar hunk that was probably harmless, but
      unnecessary and unjustified, relating to the DOOMED flag in case of
      RO_SAFE early release.
      
      Author: Thomas Munro
      Reported-by: Tom Lane
      Discussion: https://postgr.es/m/16170.1557251214%40sss.pgh.pa.us
      47a338cf
    • Peter Eisentraut's avatar
      pg_controldata: Add common gettext flags · cd805f46
      Peter Eisentraut authored
      So it picks up strings in pg_log_* calls.  This was forgotten when it
      was added to all other relevant subdirectories.
      cd805f46
    • 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 1 commit
    • 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