Skip to content
Snippets Groups Projects
  1. Feb 05, 2018
    • Tom Lane's avatar
      Fix RelationBuildPartitionKey's processing of partition key expressions. · fe921a36
      Tom Lane authored
      Failure to advance the list pointer while reading partition expressions
      from a list results in invoking an input function with inappropriate data,
      possibly leading to crashes or, with carefully crafted input, disclosure
      of arbitrary backend memory.
      
      Bug discovered independently by Álvaro Herrera and David Rowley.
      This patch is by Álvaro but owes something to David's proposed fix.
      Back-patch to v10 where the issue was introduced.
      
      Security: CVE-2018-1052
      fe921a36
  2. Jul 24, 2017
  3. Jun 21, 2017
    • Tom Lane's avatar
      Phase 3 of pgindent updates. · 382ceffd
      Tom Lane authored
      Don't move parenthesized lines to the left, even if that means they
      flow past the right margin.
      
      By default, BSD indent lines up statement continuation lines that are
      within parentheses so that they start just to the right of the preceding
      left parenthesis.  However, traditionally, if that resulted in the
      continuation line extending to the right of the desired right margin,
      then indent would push it left just far enough to not overrun the margin,
      if it could do so without making the continuation line start to the left of
      the current statement indent.  That makes for a weird mix of indentations
      unless one has been completely rigid about never violating the 80-column
      limit.
      
      This behavior has been pretty universally panned by Postgres developers.
      Hence, disable it with indent's new -lpl switch, so that parenthesized
      lines are always lined up with the preceding left paren.
      
      This patch is much less interesting than the first round of indent
      changes, but also bulkier, so I thought it best to separate the effects.
      
      Discussion: https://postgr.es/m/E1dAmxK-0006EE-1r@gemulon.postgresql.org
      Discussion: https://postgr.es/m/30527.1495162840@sss.pgh.pa.us
      382ceffd
    • Tom Lane's avatar
      Phase 2 of pgindent updates. · c7b8998e
      Tom Lane authored
      Change pg_bsd_indent to follow upstream rules for placement of comments
      to the right of code, and remove pgindent hack that caused comments
      following #endif to not obey the general rule.
      
      Commit e3860ffa wasn't actually using
      the published version of pg_bsd_indent, but a hacked-up version that
      tried to minimize the amount of movement of comments to the right of
      code.  The situation of interest is where such a comment has to be
      moved to the right of its default placement at column 33 because there's
      code there.  BSD indent has always moved right in units of tab stops
      in such cases --- but in the previous incarnation, indent was working
      in 8-space tab stops, while now it knows we use 4-space tabs.  So the
      net result is that in about half the cases, such comments are placed
      one tab stop left of before.  This is better all around: it leaves
      more room on the line for comment text, and it means that in such
      cases the comment uniformly starts at the next 4-space tab stop after
      the code, rather than sometimes one and sometimes two tabs after.
      
      Also, ensure that comments following #endif are indented the same
      as comments following other preprocessor commands such as #else.
      That inconsistency turns out to have been self-inflicted damage
      from a poorly-thought-through post-indent "fixup" in pgindent.
      
      This patch is much less interesting than the first round of indent
      changes, but also bulkier, so I thought it best to separate the effects.
      
      Discussion: https://postgr.es/m/E1dAmxK-0006EE-1r@gemulon.postgresql.org
      Discussion: https://postgr.es/m/30527.1495162840@sss.pgh.pa.us
      c7b8998e
  4. May 17, 2017
  5. May 14, 2017
    • Tom Lane's avatar
      Standardize terminology for pg_statistic_ext entries. · f04c9a61
      Tom Lane authored
      Consistently refer to such an entry as a "statistics object", not just
      "statistics" or "extended statistics".  Previously we had a mismash of
      terms, accompanied by utter confusion as to whether the term was
      singular or plural.  That's not only grating (at least to the ear of
      a native English speaker) but could be outright misleading, eg in error
      messages that seemed to be referring to multiple objects where only one
      could be meant.
      
      This commit fixes the code and a lot of comments (though I may have
      missed a few).  I also renamed two new SQL functions,
      pg_get_statisticsextdef -> pg_get_statisticsobjdef
      pg_statistic_ext_is_visible -> pg_statistics_obj_is_visible
      to conform better with this terminology.
      
      I have not touched the SGML docs other than fixing those function
      names; the docs certainly need work but it seems like a separable task.
      
      Discussion: https://postgr.es/m/22676.1494557205@sss.pgh.pa.us
      f04c9a61
  6. May 10, 2017
  7. Apr 28, 2017
  8. Apr 17, 2017
  9. Apr 06, 2017
    • Alvaro Herrera's avatar
      Comment fixes for extended statistics · b1fc51a3
      Alvaro Herrera authored
      Clean up some code comments in new extended statistics code, from
      7b504eb2.
      b1fc51a3
    • Peter Eisentraut's avatar
      Identity columns · 32173270
      Peter Eisentraut authored
      
      This is the SQL standard-conforming variant of PostgreSQL's serial
      columns.  It fixes a few usability issues that serial columns have:
      
      - CREATE TABLE / LIKE copies default but refers to same sequence
      - cannot add/drop serialness with ALTER TABLE
      - dropping default does not drop sequence
      - need to grant separate privileges to sequence
      - other slight weirdnesses because serial is some kind of special macro
      
      Reviewed-by: default avatarVitaly Burovoy <vitaly.burovoy@gmail.com>
      32173270
  10. Mar 29, 2017
    • Peter Eisentraut's avatar
      Cast result of copyObject() to correct type · 4cb82469
      Peter Eisentraut authored
      
      copyObject() is declared to return void *, which allows easily assigning
      the result independent of the input, but it loses all type checking.
      
      If the compiler supports typeof or something similar, cast the result to
      the input type.  This creates a greater amount of type safety.  In some
      cases, where the result is assigned to a generic type such as Node * or
      Expr *, new casts are now necessary, but in general casts are now
      unnecessary in the normal case and indicate that something unusual is
      happening.
      
      Reviewed-by: default avatarMark Dilger <hornschnorter@gmail.com>
      4cb82469
  11. Mar 26, 2017
    • Tom Lane's avatar
      Update some obsolete comments. · 244dd95c
      Tom Lane authored
      Fix a few stray references to expression eval functions that don't
      exist anymore or don't take the same input representation they used to.
      244dd95c
  12. Mar 24, 2017
    • Alvaro Herrera's avatar
      Implement multivariate n-distinct coefficients · 7b504eb2
      Alvaro Herrera authored
      Add support for explicitly declared statistic objects (CREATE
      STATISTICS), allowing collection of statistics on more complex
      combinations that individual table columns.  Companion commands DROP
      STATISTICS and ALTER STATISTICS ... OWNER TO / SET SCHEMA / RENAME are
      added too.  All this DDL has been designed so that more statistic types
      can be added later on, such as multivariate most-common-values and
      multivariate histograms between columns of a single table, leaving room
      for permitting columns on multiple tables, too, as well as expressions.
      
      This commit only adds support for collection of n-distinct coefficient
      on user-specified sets of columns in a single table.  This is useful to
      estimate number of distinct groups in GROUP BY and DISTINCT clauses;
      estimation errors there can cause over-allocation of memory in hashed
      aggregates, for instance, so it's a worthwhile problem to solve.  A new
      special pseudo-type pg_ndistinct is used.
      
      (num-distinct estimation was deemed sufficiently useful by itself that
      this is worthwhile even if no further statistic types are added
      immediately; so much so that another version of essentially the same
      functionality was submitted by Kyotaro Horiguchi:
      https://postgr.es/m/20150828.173334.114731693.horiguchi.kyotaro@lab.ntt.co.jp
      though this commit does not use that code.)
      
      Author: Tomas Vondra.  Some code rework by Álvaro.
      Reviewed-by: Dean Rasheed, David Rowley, Kyotaro Horiguchi, Jeff Janes,
          Ideriha Takeshi
      Discussion: https://postgr.es/m/543AFA15.4080608@fuzzy.cz
          https://postgr.es/m/20170320190220.ixlaueanxegqd5gr@alvherre.pgsql
      7b504eb2
  13. Mar 14, 2017
    • Robert Haas's avatar
      hash: Add write-ahead logging support. · c11453ce
      Robert Haas authored
      The warning about hash indexes not being write-ahead logged and their
      use being discouraged has been removed.  "snapshot too old" is now
      supported for tables with hash indexes.  Most importantly, barring
      bugs, hash indexes will now be crash-safe and usable on standbys.
      
      This commit doesn't yet add WAL consistency checking for hash
      indexes, as we now have for other index types; a separate patch has
      been submitted to cure that lack.
      
      Amit Kapila, reviewed and slightly modified by me.  The larger patch
      series of which this is a part has been reviewed and tested by Álvaro
      Herrera, Ashutosh Sharma, Mark Kirkwood, Jeff Janes, and Jesper
      Pedersen.
      
      Discussion: http://postgr.es/m/CAA4eK1JOBX=YU33631Qh-XivYXtPSALh514+jR8XeD7v+K3r_Q@mail.gmail.com
      c11453ce
  14. Feb 06, 2017
    • Tom Lane's avatar
      Avoid returning stale attribute bitmaps in RelationGetIndexAttrBitmap(). · 2aaec654
      Tom Lane authored
      The problem with the original coding here is that we might receive (and
      clear) a relcache invalidation signal for the target relation down inside
      one of the index_open calls we're doing.  Since the target is open, we
      would not drop the relcache entry, just reset its rd_indexvalid and
      rd_indexlist fields.  But RelationGetIndexAttrBitmap() kept going, and
      would eventually cache and return potentially-obsolete attribute bitmaps.
      
      The case where this matters is where the inval signal was from a CREATE
      INDEX CONCURRENTLY telling us about a new index on a formerly-unindexed
      column.  (In all other cases, the lock we hold on the target rel should
      prevent any concurrent change in index state.)  Even just returning the
      stale attribute bitmap is not such a problem, because it shouldn't matter
      during the transaction in which we receive the signal.  What hurts is
      caching the stale data, because it can survive into later transactions,
      breaking CREATE INDEX CONCURRENTLY's expectation that later transactions
      will not create new broken HOT chains.  The upshot is that there's a window
      for building corrupted indexes during CREATE INDEX CONCURRENTLY.
      
      This patch fixes the problem by rechecking that the set of index OIDs
      is still the same at the end of RelationGetIndexAttrBitmap() as it was
      at the start.  If not, we loop back and try again.  That's a little
      more than is strictly necessary to fix the bug --- in principle, we
      could return the stale data but not cache it --- but it seems like a
      bad idea on general principles for relcache to return data it knows
      is stale.
      
      There might be more hazards of the same ilk, or there might be a better
      way to fix this one, but this patch definitely improves matters and seems
      unlikely to make anything worse.  So let's push it into today's releases
      even as we continue to study the problem.
      
      Pavan Deolasee and myself
      
      Discussion: https://postgr.es/m/CABOikdM2MUq9cyZJi1KyLmmkCereyGp5JQ4fuwKoyKEde_mzkQ@mail.gmail.com
      2aaec654
    • Tom Lane's avatar
      Update comment in relcache.c. · a5931834
      Tom Lane authored
      Commit 665d1fad introduced rd_pkindex, and made RelationGetIndexList
      responsible for updating it, but didn't bother to fix
      RelationGetIndexList's header comment to say so.
      a5931834
    • Heikki Linnakangas's avatar
      Fix typos in comments. · 181bdb90
      Heikki Linnakangas authored
      Backpatch to all supported versions, where applicable, to make backpatching
      of future fixes go more smoothly.
      
      Josh Soref
      
      Discussion: https://www.postgresql.org/message-id/CACZqfqCf+5qRztLPgmmosr-B0Ye4srWzzw_mo4c_8_B_mtjmJQ@mail.gmail.com
      181bdb90
  15. Jan 31, 2017
    • Alvaro Herrera's avatar
      Tweak catalog indexing abstraction for upcoming WARM · 2f5c9d9c
      Alvaro Herrera authored
      Split the existing CatalogUpdateIndexes into two different routines,
      CatalogTupleInsert and CatalogTupleUpdate, which do both the heap
      insert/update plus the index update.  This removes over 300 lines of
      boilerplate code all over src/backend/catalog/ and src/backend/commands.
      The resulting code is much more pleasing to the eye.
      
      Also, by encapsulating what happens in detail during an UPDATE, this
      facilitates the upcoming WARM patch, which is going to add a few more
      lines to the update case making the boilerplate even more boring.
      
      The original CatalogUpdateIndexes is removed; there was only one use
      left, and since it's just three lines, we can as well expand it in place
      there.  We could keep it, but WARM is going to break all the UPDATE
      out-of-core callsites anyway, so there seems to be no benefit in doing
      so.
      
      Author: Pavan Deolasee
      Discussion: https://www.postgr.es/m/CABOikdOcFYSZ4vA2gYfs=M2cdXzXX4qGHeEiW3fu9PCfkHLa2A@mail.gmail.com
      2f5c9d9c
  16. Jan 20, 2017
  17. Jan 03, 2017
  18. Dec 07, 2016
    • Robert Haas's avatar
      Implement table partitioning. · f0e44751
      Robert Haas authored
      Table partitioning is like table inheritance and reuses much of the
      existing infrastructure, but there are some important differences.
      The parent is called a partitioned table and is always empty; it may
      not have indexes or non-inherited constraints, since those make no
      sense for a relation with no data of its own.  The children are called
      partitions and contain all of the actual data.  Each partition has an
      implicit partitioning constraint.  Multiple inheritance is not
      allowed, and partitioning and inheritance can't be mixed.  Partitions
      can't have extra columns and may not allow nulls unless the parent
      does.  Tuples inserted into the parent are automatically routed to the
      correct partition, so tuple-routing ON INSERT triggers are not needed.
      Tuple routing isn't yet supported for partitions which are foreign
      tables, and it doesn't handle updates that cross partition boundaries.
      
      Currently, tables can be range-partitioned or list-partitioned.  List
      partitioning is limited to a single column, but range partitioning can
      involve multiple columns.  A partitioning "column" can be an
      expression.
      
      Because table partitioning is less general than table inheritance, it
      is hoped that it will be easier to reason about properties of
      partitions, and therefore that this will serve as a better foundation
      for a variety of possible optimizations, including query planner
      optimizations.  The tuple routing based which this patch does based on
      the implicit partitioning constraints is an example of this, but it
      seems likely that many other useful optimizations are also possible.
      
      Amit Langote, reviewed and tested by Robert Haas, Ashutosh Bapat,
      Amit Kapila, Rajkumar Raghuwanshi, Corey Huinker, Jaime Casanova,
      Rushabh Lathia, Erik Rijkers, among others.  Minor revisions by me.
      f0e44751
  19. Aug 27, 2016
    • Tom Lane's avatar
      Add macros to make AllocSetContextCreate() calls simpler and safer. · ea268cdc
      Tom Lane authored
      I found that half a dozen (nearly 5%) of our AllocSetContextCreate calls
      had typos in the context-sizing parameters.  While none of these led to
      especially significant problems, they did create minor inefficiencies,
      and it's now clear that expecting people to copy-and-paste those calls
      accurately is not a great idea.  Let's reduce the risk of future errors
      by introducing single macros that encapsulate the common use-cases.
      Three such macros are enough to cover all but two special-purpose contexts;
      those two calls can be left as-is, I think.
      
      While this patch doesn't in itself improve matters for third-party
      extensions, it doesn't break anything for them either, and they can
      gradually adopt the simplified notation over time.
      
      In passing, change TopMemoryContext to use the default allocation
      parameters.  Formerly it could only be extended 8K at a time.  That was
      probably reasonable when this code was written; but nowadays we create
      many more contexts than we did then, so that it's not unusual to have a
      couple hundred K in TopMemoryContext, even without considering various
      dubious code that sticks other things there.  There seems no good reason
      not to let it use growing blocks like most other contexts.
      
      Back-patch to 9.6, mostly because that's still close enough to HEAD that
      it's easy to do so, and keeping the branches in sync can be expected to
      avoid some future back-patching pain.  The bugs fixed by these changes
      don't seem to be significant enough to justify fixing them further back.
      
      Discussion: <21072.1472321324@sss.pgh.pa.us>
      ea268cdc
  20. Jun 18, 2016
    • Tom Lane's avatar
      Restore foreign-key-aware estimation of join relation sizes. · 100340e2
      Tom Lane authored
      This patch provides a new implementation of the logic added by commit
      137805f8 and later removed by 77ba6108.  It differs from the original
      primarily in expending much less effort per joinrel in large queries,
      which it accomplishes by doing most of the matching work once per query not
      once per joinrel.  Hopefully, it's also less buggy and better commented.
      The never-documented enable_fkey_estimates GUC remains gone.
      
      There remains work to be done to make the selectivity estimates account
      for nulls in FK referencing columns; but that was true of the original
      patch as well.  We may be able to address this point later in beta.
      In the meantime, any error should be in the direction of overestimating
      rather than underestimating joinrel sizes, which seems like the direction
      we want to err in.
      
      Tomas Vondra and Tom Lane
      
      Discussion: <31041.1465069446@sss.pgh.pa.us>
      100340e2
  21. Jun 10, 2016
    • Tom Lane's avatar
      Refactor to reduce code duplication for function property checking. · 2f153ddf
      Tom Lane authored
      As noted by Andres Freund, we'd accumulated quite a few similar functions
      in clauses.c that examine all functions in an expression tree to see if
      they satisfy some boolean test.  Reduce the duplication by inventing a
      function check_functions_in_node() that applies a simple callback function
      to each SQL function OID appearing in a given expression node.  This also
      fixes some arguable oversights; for example, contain_mutable_functions()
      did not check aggregate or window functions for mutability.  I doubt that
      that represents a live bug at the moment, because we don't really consider
      mutability for aggregates; but it might someday be one.
      
      I chose to put check_functions_in_node() in nodeFuncs.c because it seemed
      like other modules might wish to use it in future.  That in turn forced
      moving set_opfuncid() et al into nodeFuncs.c, as the alternative was for
      nodeFuncs.c to depend on optimizer/setrefs.c which didn't seem very clean.
      
      In passing, teach contain_leaked_vars_walker() about a few more expression
      node types it can safely look through, and improve the rather messy and
      undercommented code in has_parallel_hazard_walker().
      
      Discussion: <20160527185853.ziol2os2zskahl7v@alap3.anarazel.de>
      2f153ddf
    • Tom Lane's avatar
      Improve the situation for parallel query versus temp relations. · cae1c788
      Tom Lane authored
      Transmit the leader's temp-namespace state to workers.  This is important
      because without it, the workers do not really have the same search path
      as the leader.  For example, there is no good reason (and no extant code
      either) to prevent a worker from executing a temp function that the
      leader created previously; but as things stood it would fail to find the
      temp function, and then either fail or execute the wrong function entirely.
      
      We still prohibit a worker from creating a temp namespace on its own.
      In effect, a worker can only see the session's temp namespace if the leader
      had created it before starting the worker, which seems like the right
      semantics.
      
      Also, transmit the leader's BackendId to workers, and arrange for workers
      to use that when determining the physical file path of a temp relation
      belonging to their session.  While the original intent was to prevent such
      accesses entirely, there were a number of holes in that, notably in places
      like dbsize.c which assume they can safely access temp rels of other
      sessions anyway.  We might as well get this right, as a small down payment
      on someday allowing workers to access the leader's temp tables.  (With
      this change, directly using "MyBackendId" as a relation or buffer backend
      ID is deprecated; you should use BackendIdForTempRelations() instead.
      I left a couple of such uses alone though, as they're not going to be
      reachable in parallel workers until we do something about localbuf.c.)
      
      Move the thou-shalt-not-access-thy-leader's-temp-tables prohibition down
      into localbuf.c, which is where it actually matters, instead of having it
      in relation_open().  This amounts to recognizing that access to temp
      tables' catalog entries is perfectly safe in a worker, it's only the data
      in local buffers that is problematic.
      
      Having done all that, we can get rid of the test in has_parallel_hazard()
      that says that use of a temp table's rowtype is unsafe in parallel workers.
      That test was unduly expensive, and if we really did need such a
      prohibition, that was not even close to being a bulletproof guard for it.
      (For example, any user-defined function executed in a parallel worker
      might have attempted such access.)
      cae1c788
    • Robert Haas's avatar
      pgindent run for 9.6 · 4bc424b9
      Robert Haas authored
      4bc424b9
  22. Jun 07, 2016
    • Tom Lane's avatar
      Revert "Use Foreign Key relationships to infer multi-column join selectivity". · 77ba6108
      Tom Lane authored
      This commit reverts 137805f8 as well as the associated commits 015e8894,
      5306df28, and 68d704ed.  We found multiple bugs in this feature, and
      there was concern about possible planner slowdown (though to be fair,
      exhibiting a very large slowdown proved difficult).  The way forward
      requires a considerable rewrite, which may or may not be possible to
      accomplish in time for beta2.  In my judgment reviewing the rewrite will
      be easier to accomplish starting from a clean slate, so let's temporarily
      revert what's there now.  This also leaves us in a safe state if it turns
      out to be necessary to postpone the rewrite to the next development cycle.
      
      Discussion: <20160429102531.GA13701@huehner.biz>
      77ba6108
  23. May 06, 2016
    • Kevin Grittner's avatar
      Fix hash index vs "snapshot too old" problemms · 2cc41acd
      Kevin Grittner authored
      Hash indexes are not WAL-logged, and so do not maintain the LSN of
      index pages.  Since the "snapshot too old" feature counts on
      detecting error conditions using the LSN of a table and all indexes
      on it, this makes it impossible to safely do early vacuuming on any
      table with a hash index, so add this to the tests for whether the
      xid used to vacuum a table can be adjusted based on
      old_snapshot_threshold.
      
      While at it, add a paragraph to the docs for old_snapshot_threshold
      which specifically mentions this and other aspects of the feature
      which may otherwise surprise users.
      
      Problem reported and patch reviewed by Amit Kapila
      2cc41acd
  24. Apr 08, 2016
    • Teodor Sigaev's avatar
      Revert CREATE INDEX ... INCLUDING ... · 8b99edef
      Teodor Sigaev authored
      It's not ready yet, revert two commits
      690c5435 - unstable test output
      386e3d76 - patch itself
      8b99edef
    • Teodor Sigaev's avatar
      CREATE INDEX ... INCLUDING (column[, ...]) · 386e3d76
      Teodor Sigaev authored
      Now indexes (but only B-tree for now) can contain "extra" column(s) which
      doesn't participate in index structure, they are just stored in leaf
      tuples. It allows to use index only scan by using single index instead
      of two or more indexes.
      
      Author: Anastasia Lubennikova with minor editorializing by me
      Reviewers: David Rowley, Peter Geoghegan, Jeff Janes
      386e3d76
  25. Apr 07, 2016
  26. Jan 18, 2016
    • Tom Lane's avatar
      Restructure index access method API to hide most of it at the C level. · 65c5fcd3
      Tom Lane authored
      This patch reduces pg_am to just two columns, a name and a handler
      function.  All the data formerly obtained from pg_am is now provided
      in a C struct returned by the handler function.  This is similar to
      the designs we've adopted for FDWs and tablesample methods.  There
      are multiple advantages.  For one, the index AM's support functions
      are now simple C functions, making them faster to call and much less
      error-prone, since the C compiler can now check function signatures.
      For another, this will make it far more practical to define index access
      methods in installable extensions.
      
      A disadvantage is that SQL-level code can no longer see attributes
      of index AMs; in particular, some of the crosschecks in the opr_sanity
      regression test are no longer possible from SQL.  We've addressed that
      by adding a facility for the index AM to perform such checks instead.
      (Much more could be done in that line, but for now we're content if the
      amvalidate functions more or less replace what opr_sanity used to do.)
      We might also want to expose some sort of reporting functionality, but
      this patch doesn't do that.
      
      Alexander Korotkov, reviewed by Petr Jelínek, and rather heavily
      editorialized on by me.
      65c5fcd3
  27. Jan 05, 2016
    • Alvaro Herrera's avatar
      Make pg_shseclabel available in early backend startup · efa318bc
      Alvaro Herrera authored
      While the in-core authentication mechanism doesn't need to access
      pg_shseclabel at all, it's reasonable to think that an authentication
      hook will want to look at the label for the role logging in, or for rows
      in other catalogs used during the authentication phase of startup.
      
      Catalog version bumped, because this changes the "is nailed" status for
      pg_shseclabel.
      
      Author: Adam Brightwell
      efa318bc
  28. Jan 02, 2016
  29. Nov 11, 2015
    • Tom Lane's avatar
      Be more noisy about "wrong number of nailed relations" initfile problems. · da3751c8
      Tom Lane authored
      In commit 5d1ff6bd I added some logic to
      relcache.c to try to ensure that the regression tests would fail if we
      made a mistake about which relations belong in the relcache init files.
      I'm quite sure I tested that, but I must have done so only for the
      non-shared-catalog case, because a report from Adam Brightwell showed that
      the regression tests still pass just fine if we bollix the shared-catalog
      init file in the way this code was supposed to catch.  The reason is that
      that file gets loaded before we do client authentication, so the WARNING
      is not sent to the client, only to the postmaster log, where it's far too
      easily missed.
      
      The least Rube Goldbergian answer to this is to put an Assert(false)
      after the elog(WARNING).  That will certainly get developers' attention,
      while not breaking production builds' ability to recover from corner
      cases with similar symptoms.
      
      Since this is only of interest to developers, there seems no need for
      a back-patch, even though the previous commit went into all branches.
      da3751c8
  30. Sep 15, 2015
    • Stephen Frost's avatar
      RLS refactoring · 22eaf35c
      Stephen Frost authored
      This refactors rewrite/rowsecurity.c to simplify the handling of the
      default deny case (reducing the number of places where we check for and
      add the default deny policy from three to one) by splitting up the
      retrival of the policies from the application of them.
      
      This also allowed us to do away with the policy_id field.  A policy_name
      field was added for WithCheckOption policies and is used in error
      reporting, when available.
      
      Patch by Dean Rasheed, with various mostly cosmetic changes by me.
      
      Back-patch to 9.5 where RLS was introduced to avoid unnecessary
      differences, since we're still in alpha, per discussion with Robert.
      22eaf35c
  31. Sep 04, 2015
    • Tom Lane's avatar
      Fix subtransaction cleanup after an outer-subtransaction portal fails. · c5454f99
      Tom Lane authored
      Formerly, we treated only portals created in the current subtransaction as
      having failed during subtransaction abort.  However, if the error occurred
      while running a portal created in an outer subtransaction (ie, a cursor
      declared before the last savepoint), that has to be considered broken too.
      
      To allow reliable detection of which ones those are, add a bookkeeping
      field to struct Portal that tracks the innermost subtransaction in which
      each portal has actually been executed.  (Without this, we'd end up
      failing portals containing functions that had called the subtransaction,
      thereby breaking plpgsql exception blocks completely.)
      
      In addition, when we fail an outer-subtransaction Portal, transfer its
      resources into the subtransaction's resource owner, so that they're
      released early in cleanup of the subxact.  This fixes a problem reported by
      Jim Nasby in which a function executed in an outer-subtransaction cursor
      could cause an Assert failure or crash by referencing a relation created
      within the inner subtransaction.
      
      The proximate cause of the Assert failure is that AtEOSubXact_RelationCache
      assumed it could blow away a relcache entry without first checking that the
      entry had zero refcount.  That was a bad idea on its own terms, so add such
      a check there, and to the similar coding in AtEOXact_RelationCache.  This
      provides an independent safety measure in case there are still ways to
      provoke the situation despite the Portal-level changes.
      
      This has been broken since subtransactions were invented, so back-patch
      to all supported branches.
      
      Tom Lane and Michael Paquier
      c5454f99
  32. Jun 25, 2015
    • Tom Lane's avatar
      Fix the logic for putting relations into the relcache init file. · 5d1ff6bd
      Tom Lane authored
      Commit f3b5565d was a couple of bricks shy
      of a load; specifically, it missed putting pg_trigger_tgrelid_tgname_index
      into the relcache init file, because that index is not used by any
      syscache.  However, we have historically nailed that index into cache for
      performance reasons.  The upshot was that load_relcache_init_file always
      decided that the init file was busted and silently ignored it, resulting
      in a significant hit to backend startup speed.
      
      To fix, reinstantiate RelationIdIsInInitFile() as a wrapper around
      RelationSupportsSysCache(), which can know about additional relations
      that should be in the init file despite being unknown to syscache.c.
      
      Also install some guards against future mistakes of this type: make
      write_relcache_init_file Assert that all nailed relations get written to
      the init file, and make load_relcache_init_file emit a WARNING if it takes
      the "wrong number of nailed relations" exit path.  Now that we remove the
      init files during postmaster startup, that case should never occur in the
      field, even if we are starting a minor-version update that added or removed
      rels from the nailed set.  So the warning shouldn't ever be seen by end
      users, but it will show up in the regression tests if somebody breaks this
      logic.
      
      Back-patch to all supported branches, like the previous commit.
      5d1ff6bd
Loading