Skip to content
Snippets Groups Projects
  1. Jul 17, 2012
    • Tom Lane's avatar
      Improve coding around the fsync request queue. · 4abcce8c
      Tom Lane authored
      In all branches back to 8.3, this patch fixes a questionable assumption in
      CompactCheckpointerRequestQueue/CompactBgwriterRequestQueue that there are
      no uninitialized pad bytes in the request queue structs.  This would only
      cause trouble if (a) there were such pad bytes, which could happen in 8.4
      and up if the compiler makes enum ForkNumber narrower than 32 bits, but
      otherwise would require not-currently-planned changes in the widths of
      other typedefs; and (b) the kernel has not uniformly initialized the
      contents of shared memory to zeroes.  Still, it seems a tad risky, and we
      can easily remove any risk by pre-zeroing the request array for ourselves.
      In addition to that, we need to establish a coding rule that struct
      RelFileNode can't contain any padding bytes, since such structs are copied
      into the request array verbatim.  (There are other places that are assuming
      this anyway, it turns out.)
      
      In 9.1 and up, the risk was a bit larger because we were also effectively
      assuming that struct RelFileNodeBackend contained no pad bytes, and with
      fields of different types in there, that would be much easier to break.
      However, there is no good reason to ever transmit fsync or delete requests
      for temp files to the bgwriter/checkpointer, so we can revert the request
      structs to plain RelFileNode, getting rid of the padding risk and saving
      some marginal number of bytes and cycles in fsync queue manipulation while
      we are at it.  The savings might be more than marginal during deletion of
      a temp relation, because the old code transmitted an entirely useless but
      nonetheless expensive-to-process ForgetRelationFsync request to the
      background process, and also had the background process perform the file
      deletion even though that can safely be done immediately.
      
      In addition, make some cleanup of nearby comments and small improvements to
      the code in CompactCheckpointerRequestQueue/CompactBgwriterRequestQueue.
      4abcce8c
  2. Jul 16, 2012
    • Tom Lane's avatar
      Avoid pre-determining index names during CREATE TABLE LIKE parsing. · 3727240d
      Tom Lane authored
      Formerly, when trying to copy both indexes and comments, CREATE TABLE LIKE
      had to pre-assign names to indexes that had comments, because it made up an
      explicit CommentStmt command to apply the comment and so it had to know the
      name for the index.  This creates bad interactions with other indexes, as
      shown in bug #6734 from Daniele Varrazzo: the preassignment logic couldn't
      take any other indexes into account so it could choose a conflicting name.
      
      To fix, add a field to IndexStmt that allows it to carry a comment to be
      assigned to the new index.  (This isn't a user-exposed feature of CREATE
      INDEX, only an internal option.)  Now we don't need preassignment of index
      names in any situation.
      
      I also took the opportunity to refactor DefineIndex to accept the IndexStmt
      as such, rather than passing all its fields individually in a mile-long
      parameter list.
      
      Back-patch to 9.2, but no further, because it seems too dangerous to change
      IndexStmt or DefineIndex's API in released branches.  The bug exists back
      to 9.0 where CREATE TABLE LIKE grew the ability to copy comments, but given
      the lack of prior complaints we'll just let it go unfixed before 9.2.
      3727240d
  3. Jul 15, 2012
    • Tom Lane's avatar
      Prevent corner-case core dump in rfree(). · 1116c9d1
      Tom Lane authored
      rfree() failed to cope with the case that pg_regcomp() had initialized the
      regex_t struct but then failed to allocate any memory for re->re_guts (ie,
      the first malloc call in pg_regcomp() failed).  It would try to touch the
      guts struct anyway, and thus dump core.  This is a sufficiently narrow
      corner case that it's not surprising it's never been seen in the field;
      but still a bug is a bug, so patch all active branches.
      
      Noted while investigating whether we need to call pg_regfree after a
      failure return from pg_regcomp.  Other than this bug, it turns out we
      don't, so adjust comments appropriately.
      1116c9d1
  4. Jul 12, 2012
    • Tom Lane's avatar
      Fix walsender processes to establish a SIGALRM handler. · 0bf8eb2a
      Tom Lane authored
      Walsenders must have working SIGALRM handling during InitPostgres,
      but they set the handler to SIG_IGN so that nothing would happen
      if a timeout was reached.  This could result in two failure modes:
      
      * If a walsender participated in a deadlock during its authentication
      transaction, and was the last to wait in the deadly embrace, the deadlock
      would not get cleared automatically.  This would require somebody to be
      trying to take out AccessExclusiveLock on multiple system catalogs, so
      it's not very probable.
      
      * If a client failed to respond to a walsender's authentication challenge,
      the intended disconnect after AuthenticationTimeout wouldn't happen, and
      the walsender would wait indefinitely for the client.
      
      For the moment, fix in back branches only, since this is fixed in a
      different way in the timeout-infrastructure patch that's awaiting
      application to HEAD.  If we choose not to apply that, then we'll need
      to do this in HEAD as well.
      0bf8eb2a
  5. Jul 11, 2012
  6. Jul 10, 2012
    • Tom Lane's avatar
      Back-patch addition of pg_wchar-to-multibyte conversion functionality. · f12960d8
      Tom Lane authored
      Back-patch of commits 72dd6291,
      f6a05fd9, and
      60e9c224.
      
      This is needed to support fixing the regex prefix extraction bug in
      back branches.
      f12960d8
    • Tom Lane's avatar
      Refactor pattern_fixed_prefix() to avoid dealing in incomplete patterns. · 8fc7b07b
      Tom Lane authored
      Previously, pattern_fixed_prefix() was defined to return whatever fixed
      prefix it could extract from the pattern, plus the "rest" of the pattern.
      That definition was sensible for LIKE patterns, but not so much for
      regexes, where reconstituting a valid pattern minus the prefix could be
      quite tricky (certainly the existing code wasn't doing that correctly).
      Since the only thing that callers ever did with the "rest" of the pattern
      was to pass it to like_selectivity() or regex_selectivity(), let's cut out
      the middle-man and just have pattern_fixed_prefix's subroutines do this
      directly.  Then pattern_fixed_prefix can return a simple selectivity
      number, and the question of how to cope with partial patterns is removed
      from its API specification.
      
      While at it, adjust the API spec so that callers who don't actually care
      about the pattern's selectivity (which is a lot of them) can pass NULL for
      the selectivity pointer to skip doing the work of computing a selectivity
      estimate.
      
      This patch is only an API refactoring that doesn't actually change any
      processing, other than allowing a little bit of useless work to be skipped.
      However, it's necessary infrastructure for my upcoming fix to regex prefix
      extraction, because after that change there won't be any simple way to
      identify the "rest" of the regex, not even to the low level of fidelity
      needed by regex_selectivity.  We can cope with that if regex_fixed_prefix
      and regex_selectivity communicate directly, but not if we have to work
      within the old API.  Hence, back-patch to all active branches.
      8fc7b07b
  7. Jul 09, 2012
    • Tom Lane's avatar
      Fix planner to pass correct collation to operator selectivity estimators. · eb1b4881
      Tom Lane authored
      We can do this without creating an API break for estimation functions
      by passing the collation using the existing fmgr functionality for
      passing an input collation as a hidden parameter.
      
      The need for this was foreseen at the outset, but we didn't get around to
      making it happen in 9.1 because of the decision to sort all pg_statistic
      histograms according to the database's default collation.  That meant that
      selectivity estimators generally need to use the default collation too,
      even if they're estimating for an operator that will do something
      different.  The reason it's suddenly become more interesting is that
      regexp interpretation also uses a collation (for its LC_TYPE not LC_COLLATE
      property), and we no longer want to use the wrong collation when examining
      regexps during planning.  It's not that the selectivity estimate is likely
      to change much from this; rather that we are thinking of caching compiled
      regexps during planner estimation, and we won't get the intended benefit
      if we cache them with a different collation than the executor will use.
      
      Back-patch to 9.1, both because the regexp change is likely to get
      back-patched and because we might as well get this right in all
      collation-supporting branches, in case any third-party code wants to
      rely on getting the collation.  The patch turns out to be minuscule
      now that I've done it ...
      eb1b4881
  8. Jul 06, 2012
  9. Jul 05, 2012
  10. Jul 04, 2012
  11. Jul 03, 2012
    • Alvaro Herrera's avatar
      c602b429
    • Alvaro Herrera's avatar
      Have REASSIGN OWNED work on extensions, too · 6416895c
      Alvaro Herrera authored
      Per bug #6593, REASSIGN OWNED fails when the affected role has created
      an extension.  Even though the user related to the extension is not
      nominally the owner, its OID appears on pg_shdepend and thus causes
      problems when the user is to be dropped.
      
      This commit adds code to change the "ownership" of the extension itself,
      not of the contained objects.  This is fine because it's currently only
      called from REASSIGN OWNED, which would also modify the ownership of the
      contained objects.  However, this is not sufficient for a working ALTER
      OWNER implementation extension.
      
      Back-patch to 9.1, where extensions were introduced.
      
      Bug #6593 reported by Emiliano Leporati.
      6416895c
  12. Jul 02, 2012
  13. Jul 01, 2012
    • Tom Lane's avatar
      Fix race condition in enum value comparisons. · 972e0666
      Tom Lane authored
      When (re) loading the typcache comparison cache for an enum type's values,
      use an up-to-date MVCC snapshot, not the transaction's existing snapshot.
      This avoids problems if we encounter an enum OID that was created since our
      transaction started.  Per report from Andres Freund and diagnosis by Robert
      Haas.
      
      To ensure this is safe even if enum comparison manages to get invoked
      before we've set a transaction snapshot, tweak GetLatestSnapshot to
      redirect to GetTransactionSnapshot instead of throwing error when
      FirstSnapshotSet is false.  The existing uses of GetLatestSnapshot (in
      ri_triggers.c) don't care since they couldn't be invoked except in a
      transaction that's already done some work --- but it seems just conceivable
      that this might not be true of enums, especially if we ever choose to use
      enums in system catalogs.
      
      Note that the comparable coding in enum_endpoint and enum_range_internal
      remains GetTransactionSnapshot; this is perhaps debatable, but if we
      changed it those functions would have to be marked volatile, which doesn't
      seem attractive.
      
      Back-patch to 9.1 where ALTER TYPE ADD VALUE was added.
      972e0666
  14. Jun 30, 2012
    • Tom Lane's avatar
      Remove inappropriate semicolons after function definitions. · 4f1e0e40
      Tom Lane authored
      Solaris Studio warns about this, and some compilers might think it's an
      outright syntax error.
      4f1e0e40
    • Tom Lane's avatar
      Prevent CREATE TABLE LIKE/INHERITS from (mis) copying whole-row Vars. · 52336a47
      Tom Lane authored
      If a CHECK constraint or index definition contained a whole-row Var (that
      is, "table.*"), an attempt to copy that definition via CREATE TABLE LIKE or
      table inheritance produced incorrect results: the copied Var still claimed
      to have the rowtype of the source table, rather than the created table.
      
      For the LIKE case, it seems reasonable to just throw error for this
      situation, since the point of LIKE is that the new table is not permanently
      coupled to the old, so there's no reason to assume its rowtype will stay
      compatible.  In the inheritance case, we should ideally allow such
      constraints, but doing so will require nontrivial refactoring of CREATE
      TABLE processing (because we'd need to know the OID of the new table's
      rowtype before we adjust inherited CHECK constraints).  In view of the lack
      of previous complaints, that doesn't seem worth the risk in a back-patched
      bug fix, so just make it throw error for the inheritance case as well.
      
      Along the way, replace change_varattnos_of_a_node() with a more robust
      function map_variable_attnos(), which is capable of being extended to
      handle insertion of ConvertRowtypeExpr whenever we get around to fixing
      the inheritance case nicely, and in the meantime it returns a failure
      indication to the caller so that a helpful message with some context can be
      thrown.  Also, this code will do the right thing with subselects (if we
      ever allow them in CHECK or indexes), and it range-checks varattnos before
      using them to index into the map array.
      
      Per report from Sergey Konoplev.  Back-patch to all supported branches.
      52336a47
  15. Jun 29, 2012
    • Heikki Linnakangas's avatar
      Initialize shared memory copy of ckptXidEpoch correctly when not in recovery. · 214fa27e
      Heikki Linnakangas authored
      This bug was introduced by commit 20d98ab6,
      so backpatch this to 9.0-9.2 like that one.
      
      This fixes bug #6710, reported by Tarvi Pillessaar
      214fa27e
    • Tom Lane's avatar
      Fix NOTIFY to cope with I/O problems, such as out-of-disk-space. · f374098e
      Tom Lane authored
      The LISTEN/NOTIFY subsystem got confused if SimpleLruZeroPage failed,
      which would typically happen as a result of a write() failure while
      attempting to dump a dirty pg_notify page out of memory.  Subsequently,
      all attempts to send more NOTIFY messages would fail with messages like
      "Could not read from file "pg_notify/nnnn" at offset nnnnn: Success".
      Only restarting the server would clear this condition.  Per reports from
      Kevin Grittner and Christoph Berg.
      
      Back-patch to 9.0, where the problem was introduced during the
      LISTEN/NOTIFY rewrite.
      f374098e
  16. Jun 28, 2012
    • Tom Lane's avatar
      Make UtilityContainsQuery recurse until it finds a non-utility Query. · c8967e38
      Tom Lane authored
      The callers of UtilityContainsQuery want it to return a non-utility Query
      if it returns anything at all.  However, since we made CREATE TABLE
      AS/SELECT INTO into a utility command instead of a variant of SELECT,
      a command like "EXPLAIN SELECT INTO" results in two nested utility
      statements.  So what we need UtilityContainsQuery to do is drill down
      to the bottom non-utility Query.
      
      I had thought of this possibility in setrefs.c, and fixed it there by
      looping around the UtilityContainsQuery call; but overlooked that the call
      sites in plancache.c have a similar issue.  In those cases it's
      notationally inconvenient to provide an external loop, so let's redefine
      UtilityContainsQuery as recursing down to a non-utility Query instead.
      
      Noted by Rushabh Lathia.  This is a somewhat cleaned-up version of his
      proposed patch.
      c8967e38
  17. Jun 27, 2012
  18. Jun 26, 2012
    • Tom Lane's avatar
      Cope with smaller-than-normal BLCKSZ setting in SPGiST indexes on text. · 00e58445
      Tom Lane authored
      The original coding failed miserably for BLCKSZ of 4K or less, as reported
      by Josh Kupershmidt.  With the present design for text indexes, a given
      inner tuple could have up to 256 labels (requiring either 3K or 4K bytes
      depending on MAXALIGN), which means that we can't positively guarantee no
      failures for smaller blocksizes.  But we can at least make it behave sanely
      so long as there are few enough labels to fit on a page.  Considering that
      btree is also more prone to "index tuple too large" failures when BLCKSZ is
      small, it's not clear that we should expend more work than this on this
      case.
      00e58445
  19. Jun 21, 2012
    • Tom Lane's avatar
      Fix memory leak in ARRAY(SELECT ...) subqueries. · 66567ab2
      Tom Lane authored
      Repeated execution of an uncorrelated ARRAY_SUBLINK sub-select (which
      I think can only happen if the sub-select is embedded in a larger,
      correlated subquery) would leak memory for the duration of the query,
      due to not reclaiming the array generated in the previous execution.
      Per bug #6698 from Armando Miraglia.  Diagnosis and fix idea by Heikki,
      patch itself by me.
      
      This has been like this all along, so back-patch to all supported versions.
      66567ab2
  20. Jun 18, 2012
  21. Jun 15, 2012
    • Peter Eisentraut's avatar
      Improve reporting of permission errors for array types · 0b847cba
      Peter Eisentraut authored
      Because permissions are assigned to element types, not array types,
      complaining about permission denied on an array type would be
      misleading to users.  So adjust the reporting to refer to the element
      type instead.
      
      In order not to duplicate the required logic in two dozen places,
      refactor the permission denied reporting for types a bit.
      
      pointed out by Yeb Havinga during the review of the type privilege
      feature
      0b847cba
    • Peter Eisentraut's avatar
      Add more message pluralization · ccc65b71
      Peter Eisentraut authored
      Even though we can't do much about the case with multiple plurals in
      one sentence, we can fix the other cases.
      ccc65b71
  22. Jun 14, 2012
    • Tom Lane's avatar
      Revisit error message details for JSON input parsing. · 80edfd76
      Tom Lane authored
      Instead of identifying error locations only by line number (which could
      be entirely unhelpful with long input lines), provide a fragment of the
      input text too, placing this info in a new CONTEXT entry.  Make the
      error detail messages conform more closely to style guidelines, fix
      failure to expose some of them for translation, ensure compiler can
      check formats against supplied parameters.
      80edfd76
    • Tom Lane's avatar
      Revert "Reduce checkpoints and WAL traffic on low activity database server" · b8b69d89
      Tom Lane authored
      This reverts commit 18fb9d8d.  Per
      discussion, it does not seem like a good idea to allow committed changes to
      go un-checkpointed indefinitely, as could happen in a low-traffic server;
      that makes us entirely reliant on the WAL stream with no redundancy that
      might aid data recovery in case of disk failure.
      
      This re-introduces the original problem of hot-standby setups generating a
      small continuing stream of WAL traffic even when idle, but there are other
      ways to address that without compromising crash recovery, so we'll revisit
      that issue in a future release cycle.
      b8b69d89
  23. Jun 13, 2012
    • Tom Lane's avatar
      Deprecate use of GLOBAL and LOCAL in temp table creation. · c3bc76bd
      Tom Lane authored
      Aside from adjusting the documentation to say that these are deprecated,
      we now report a warning (not an error) for use of GLOBAL, since it seems
      fairly likely that we might change that to request SQL-spec-compliant temp
      table behavior in the foreseeable future.  Although our handling of LOCAL
      is equally nonstandard, there is no evident interest in ever implementing
      SQL modules, and furthermore some other products interpret LOCAL as
      behaving the same way we do.  So no expectation of change and no warning
      for LOCAL; but it still seems a good idea to deprecate writing it.
      
      Noah Misch
      c3bc76bd
    • Tom Lane's avatar
      Support Linux's oom_score_adj API as well as the older oom_adj API. · 93f4d7f8
      Tom Lane authored
      The simplest way to handle this is just to copy-and-paste the relevant
      code block in fork_process.c, so that's what I did. (It's possible that
      something more complicated would be useful to packagers who want to work
      with either the old or the new API; but at this point the number of such
      people is rapidly approaching zero, so let's just get the minimal thing
      done.)  Update relevant documentation as well.
      93f4d7f8
    • Peter Eisentraut's avatar
      Improve documentation of postgres -C option · c0a6f9c8
      Peter Eisentraut authored
      Clarify help (s/return/print/), and explain that this option is for
      use by other programs, not for user-facing use (it does not print
      units).
      c0a6f9c8
  24. Jun 12, 2012
  25. Jun 11, 2012
    • Magnus Hagander's avatar
      Prevent non-streaming replication connections from being selected sync slave · 3595a71e
      Magnus Hagander authored
      This prevents a pg_basebackup backup session that just does a base
      backup (no xlog involved at all) from becoming the synchronous slave
      and thus blocking all access while it runs.
      
      Also fixes the problem when a higher priority slave shows up it would
      become the sync standby before it has reached the STREAMING state, by
      making sure we can only switch to a walsender that's actually STREAMING.
      
      Fujii Masao
      3595a71e
  26. Jun 10, 2012
  27. Jun 09, 2012
  28. Jun 08, 2012
    • Simon Riggs's avatar
      Fix bug in early startup of Hot Standby with subtransactions. · 37255705
      Simon Riggs authored
      When HS startup is deferred because of overflowed subtransactions, ensure
      that we re-initialize KnownAssignedXids for when both existing and incoming
      snapshots have non-zero qualifying xids.
      
      Fixes bug #6661 reported by Valentine Gogichashvili.
      
      Analysis and fix by Andres Freund
      37255705
Loading