Skip to content
Snippets Groups Projects
  1. Nov 07, 2012
  2. Nov 06, 2012
  3. Nov 05, 2012
    • Tom Lane's avatar
      Fix handling of inherited check constraints in ALTER COLUMN TYPE. · 329057fd
      Tom Lane authored
      This case got broken in 8.4 by the addition of an error check that
      complains if ALTER TABLE ONLY is used on a table that has children.
      We do use ONLY for this situation, but it's okay because the necessary
      recursion occurs at a higher level.  So we need to have a separate
      flag to suppress recursion without making the error check.
      
      Reported and patched by Pavan Deolasee, with some editorial adjustments by
      me.  Back-patch to 8.4, since this is a regression of functionality that
      worked in earlier branches.
      329057fd
  4. Nov 02, 2012
  5. Nov 01, 2012
    • Tom Lane's avatar
      Limit the number of rel sets considered in consider_index_join_outer_rels. · ca2d6a6c
      Tom Lane authored
      In bug #7626, Brian Dunavant exposes a performance problem created by
      commit 3b8968f2: that commit attempted to
      consider *all* possible combinations of indexable join clauses, but if said
      clauses join to enough different relations, there's an exponential increase
      in the number of outer-relation sets considered.
      
      In Brian's example, all the clauses come from the same equivalence class,
      which means it's redundant to use more than one of them in an indexscan
      anyway.  So we can prevent the problem in this class of cases (which is
      probably the majority of real examples) by rejecting combinations that
      would only serve to add a known-redundant clause.
      
      But that still leaves us exposed to exponential growth of planning time
      when the query has a lot of non-equivalence join clauses that are usable
      with the same index.  I chose to prevent such cases by setting an upper
      limit on the number of relation sets considered, equal to ten times the
      number of index clauses considered so far.  (This sliding limit still
      allows new relsets to be added on as we move to additional index columns,
      which is probably more important than considering even more combinations of
      clauses for the previous column.)  This should keep the amount of work done
      roughly linear rather than exponential in the apparent query complexity.
      This part of the fix is pretty ad-hoc; but without a clearer idea of
      real-world cases for which this would result in markedly inferior plans,
      it's hard to see how to do better.
      ca2d6a6c
  6. Oct 31, 2012
    • Tom Lane's avatar
      Document that TCP keepalive settings read as 0 on Unix-socket connections. · ec397c90
      Tom Lane authored
      Per bug #7631 from Rob Johnson.  The code is operating as designed, but the
      docs didn't explain it.
      ec397c90
    • Alvaro Herrera's avatar
      Fix ALTER EXTENSION / SET SCHEMA · fb3590dd
      Alvaro Herrera authored
      In its original conception, it was leaving some objects into the old
      schema, but without their proper pg_depend entries; this meant that the
      old schema could be dropped, causing future pg_dump calls to fail on the
      affected database.  This was originally reported by Jeff Frost as #6704;
      there have been other complaints elsewhere that can probably be traced
      to this bug.
      
      To fix, be more consistent about altering a table's subsidiary objects
      along the table itself; this requires some restructuring in how tables
      are relocated when altering an extension -- hence the new
      AlterTableNamespaceInternal routine which encapsulates it for both the
      ALTER TABLE and the ALTER EXTENSION cases.
      
      There was another bug lurking here, which was unmasked after fixing the
      previous one: certain objects would be reached twice via the dependency
      graph, and the second attempt to move them would cause the entire
      operation to fail.  Per discussion, it seems the best fix for this is to
      do more careful tracking of objects already moved: we now maintain a
      list of moved objects, to avoid attempting to do it twice for the same
      object.
      
      Authors: Alvaro Herrera, Dimitri Fontaine
      Reviewed by Tom Lane
      fb3590dd
  7. Oct 26, 2012
    • Tom Lane's avatar
      Prefer actual constants to pseudo-constants in equivalence class machinery. · 7e951ba6
      Tom Lane authored
      generate_base_implied_equalities_const() should prefer plain Consts over
      other em_is_const eclass members when choosing the "pivot" value that
      all the other members will be equated to.  This makes it more likely that
      the generated equalities will be useful in constraint-exclusion proofs.
      Per report from Rushabh Lathia.
      7e951ba6
    • Tom Lane's avatar
      In pg_dump, dump SEQUENCE SET items in the data not pre-data section. · 725fa25e
      Tom Lane authored
      Represent a sequence's current value as a separate TableDataInfo dumpable
      object, so that it can be dumped within the data section of the archive
      rather than in pre-data.  This fixes an undesirable inconsistency between
      the meanings of "--data-only" and "--section=data", and also fixes dumping
      of sequences that are marked as extension configuration tables, as per a
      report from Marko Kreen back in July.  The main cost is that we do one more
      SQL query per sequence, but that's probably not very meaningful in most
      databases.
      
      Back-patch to 9.1, since it has the extension configuration issue even
      though not the --section switch.
      725fa25e
  8. Oct 24, 2012
    • Tom Lane's avatar
      Prevent parser from believing that views have system columns. · 1dec7c7c
      Tom Lane authored
      Views should not have any pg_attribute entries for system columns.
      However, we forgot to remove such entries when converting a table to a
      view.  This could lead to crashes later on, if someone attempted to
      reference such a column, as reported by Kohei KaiGai.
      
      This problem is corrected properly in HEAD (by removing the pg_attribute
      entries during conversion), but in the back branches we need to defend
      against existing mis-converted views.  This fix costs us an extra syscache
      lookup per system column reference, which is annoying but probably not
      really measurable in the big scheme of things.
      1dec7c7c
  9. Oct 22, 2012
    • Kevin Grittner's avatar
      Correct predicate locking for DROP INDEX CONCURRENTLY. · 523ecaf4
      Kevin Grittner authored
      For the non-concurrent case there is an AccessExclusiveLock lock
      on both the index and the heap at a time during which no other
      process is using either, before which the index is maintained and
      used for scans, and after which the index is no longer used or
      maintained.  Predicate locks can safely be moved from the index to
      the related heap relation under the protection of these locks.
      This was done prior to the introductin of DROP INDEX CONCURRENTLY
      and continues to be done for non-concurrent index drops.
      
      For concurrent index drops, the predicate locks must be moved when
      there are no index scans in progress on that index and no more can
      subsequently start, and before heap inserts stop maintaining the
      index.  As long as these conditions are guaranteed when the
      TransferPredicateLocksToHeapRelation() function is called,
      stronger locks are not needed for correctness.
      
      Kevin Grittner based on questions by Tom Lane in reviewing the
      DROP INDEX CONCURRENTLY patch and in cooperation with Andres
      Freund and Simon Riggs.
      
      Back-patch of commit 4c9d0901
      523ecaf4
  10. Oct 20, 2012
    • Tom Lane's avatar
      Fix pg_dump's handling of DROP DATABASE commands in --clean mode. · a4ef1f09
      Tom Lane authored
      In commit 4317e024, I accidentally broke
      this behavior while rearranging code to ensure that --create wouldn't
      affect whether a DATABASE entry gets put into archive-format output.
      Thus, 9.2 would issue a DROP DATABASE command in --clean mode, which is
      either useless or dangerous depending on the usage scenario.
      It should not do that, and no longer does.
      
      A bright spot is that this refactoring makes it easy to allow the
      combination of --clean and --create to work sensibly, ie, emit DROP
      DATABASE then CREATE DATABASE before reconnecting.  Ordinarily we'd
      consider that a feature addition and not back-patch it, but it seems
      silly to not include the extra couple of lines required in the 9.2
      version of the code.
      
      Per report from Guillaume Lelarge, though this is slightly more extensive
      than his proposed patch.
      a4ef1f09
    • Tom Lane's avatar
      Fix UtilityContainsQuery() to handle CREATE TABLE AS EXECUTE correctly. · 12b721a7
      Tom Lane authored
      The code seems to have been written to handle the pre-parse-analysis
      representation, where an ExecuteStmt would appear directly under
      CreateTableAsStmt.  But in reality the function is only run on
      already-parse-analyzed statements, so there will be a Query node in
      between.  We'd not noticed the bug because the function is generally
      not used at all except in extended query protocol.
      
      Per report from Robert Haas and Rushabh Lathia.
      12b721a7
  11. Oct 19, 2012
    • Tom Lane's avatar
      Fix hash_search to avoid corruption of the hash table on out-of-memory. · 8a3249f1
      Tom Lane authored
      An out-of-memory error during expand_table() on a palloc-based hash table
      would leave a partially-initialized entry in the table.  This would not be
      harmful for transient hash tables, since they'd get thrown away anyway at
      transaction abort.  But for long-lived hash tables, such as the relcache
      hash, this would effectively corrupt the table, leading to crash or other
      misbehavior later.
      
      To fix, rearrange the order of operations so that table enlargement is
      attempted before we insert a new entry, rather than after adding it
      to the hash table.
      
      Problem discovered by Hitoshi Harada, though this is a bit different
      from his proposed patch.
      8a3249f1
    • Tom Lane's avatar
      Fix ruleutils to print "INSERT INTO foo DEFAULT VALUES" correctly. · 645984e4
      Tom Lane authored
      Per bug #7615 from Marko Tiikkaja.  Apparently nobody ever tried this
      case before ...
      645984e4
    • Simon Riggs's avatar
      Fix orphan on cancel of drop index concurrently. · c5675357
      Simon Riggs authored
      Canceling DROP INDEX CONCURRENTLY during
      wait could allow an orphaned index to be
      left behind which could not be dropped.
      
      Backpatch to 9.2
      
      Andres Freund, tested by Abhijit Menon-Sen
      c5675357
  12. Oct 18, 2012
    • Andrew Dunstan's avatar
      Use a more portable platform test. · f61013a4
      Andrew Dunstan authored
      f61013a4
    • Heikki Linnakangas's avatar
      Further tweaking of the readfile() function in pg_ctl. · e7bab081
      Heikki Linnakangas authored
      Don't leak a file descriptor if the file is empty or we can't read its size.
      
      Expect there to be a newline at the end of the last line, too. If there
      isn't, ignore anything after the last newline. This makes it a tiny bit
      more robust in case the file is appended to concurrently, so that we don't
      return the last line if it hasn't been fully written yet. And this makes
      the code a bit less obscure, anyway. Per Tom Lane's suggestion.
      
      Backpatch to all supported branches.
      e7bab081
    • Simon Riggs's avatar
      Isolation test for DROP INDEX CONCURRENTLY · 623e49c0
      Simon Riggs authored
      for recent concurrent changes.
      
      Abhijit Menon-Sen
      623e49c0
    • Simon Riggs's avatar
      Re-think guts of DROP INDEX CONCURRENTLY. · 5da1c4b7
      Simon Riggs authored
      Concurrent behaviour was flawed when using
      a two-step process, so add an additional
      phase of processing to ensure concurrency
      for both SELECTs and INSERT/UPDATE/DELETEs.
      
      Backpatch to 9.2
      
      Andres Freund, tweaked by me
      5da1c4b7
    • Tom Lane's avatar
      Fix planning of non-strict equivalence clauses above outer joins. · 0237b394
      Tom Lane authored
      If a potential equivalence clause references a variable from the nullable
      side of an outer join, the planner needs to take care that derived clauses
      are not pushed to below the outer join; else they may use the wrong value
      for the variable.  (The problem arises only with non-strict clauses, since
      if an upper clause can be proven strict then the outer join will get
      simplified to a plain join.)  The planner attempted to prevent this type
      of error by checking that potential equivalence clauses aren't
      outerjoin-delayed as a whole, but actually we have to check each side
      separately, since the two sides of the clause will get moved around
      separately if it's treated as an equivalence.  Bugs of this type can be
      demonstrated as far back as 7.4, even though releases before 8.3 had only
      a very ad-hoc notion of equivalence clauses.
      
      In addition, we neglected to account for the possibility that such clauses
      might have nonempty nullable_relids even when not outerjoin-delayed; so the
      equivalence-class machinery lacked logic to compute correct nullable_relids
      values for clauses it constructs.  This oversight was harmless before 9.2
      because we were only using RestrictInfo.nullable_relids for OR clauses;
      but as of 9.2 it could result in pushing constructed equivalence clauses
      to incorrect places.  (This accounts for bug #7604 from Bill MacArthur.)
      
      Fix the first problem by adding a new test check_equivalence_delay() in
      distribute_qual_to_rels, and fix the second one by adding code in
      equivclass.c and called functions to set correct nullable_relids for
      generated clauses.  Although I believe the second part of this is not
      currently necessary before 9.2, I chose to back-patch it anyway, partly to
      keep the logic similar across branches and partly because it seems possible
      we might find other reasons why we need valid values of nullable_relids in
      the older branches.
      
      Add regression tests illustrating these problems.  In 9.0 and up, also
      add test cases checking that we can push constants through outer joins,
      since we've broken that optimization before and I nearly broke it again
      with an overly simplistic patch for this problem.
      0237b394
    • Simon Riggs's avatar
      Revert tests for drop index concurrently. · 12609127
      Simon Riggs authored
      12609127
    • Simon Riggs's avatar
      Add isolation tests for DROP INDEX CONCURRENTLY. · d4412fa0
      Simon Riggs authored
      Backpatch to 9.2 to ensure bugs are fixed.
      
      Abhijit Menon-Sen
      d4412fa0
  13. Oct 17, 2012
    • Tom Lane's avatar
      Close un-owned SMgrRelations at transaction end. · d7598aee
      Tom Lane authored
      If an SMgrRelation is not "owned" by a relcache entry, don't allow it to
      live past transaction end.  This design allows the same SMgrRelation to be
      used for blind writes of multiple blocks during a transaction, but ensures
      that we don't hold onto such an SMgrRelation indefinitely.  Because an
      SMgrRelation typically corresponds to open file descriptors at the fd.c
      level, leaving it open when there's no corresponding relcache entry can
      mean that we prevent the kernel from reclaiming deleted disk space.
      (While CacheInvalidateSmgr messages usually fix that, there are cases
      where they're not issued, such as DROP DATABASE.  We might want to add
      some more sinval messaging for that, but I'd be inclined to keep this
      type of logic anyway, since allowing VFDs to accumulate indefinitely
      for blind-written relations doesn't seem like a good idea.)
      
      This code replaces a previous attempt towards the same goal that proved
      to be unreliable.  Back-patch to 9.1 where the previous patch was added.
      d7598aee
    • Tom Lane's avatar
      Revert "Use "transient" files for blind writes, take 2". · a1f064fc
      Tom Lane authored
      This reverts commit fba105b1.
      That approach had problems with the smgr-level state not tracking what
      we really want to happen, and with the VFD-level state not tracking the
      smgr-level state very well either.  In consequence, it was still possible
      to hold kernel file descriptors open for long-gone tables (as in recent
      report from Tore Halset), and yet there were also cases of FDs being closed
      undesirably soon.  A replacement implementation will follow.
      a1f064fc
    • Simon Riggs's avatar
      Fix typo in previous commit · 4be1d015
      Simon Riggs authored
      4be1d015
    • Simon Riggs's avatar
  14. Oct 15, 2012
    • Heikki Linnakangas's avatar
      Fix race condition in pg_ctl reading postmaster.pid. · 54037476
      Heikki Linnakangas authored
      If postmaster changed postmaster.pid while pg_ctl was reading it, pg_ctl
      could overrun the buffer it allocated for the file. Fix by reading the
      whole file to memory with one read() call.
      
      initdb contains an identical copy of the readfile() function, but the files
      that initdb reads are static, not modified concurrently. Nevertheless, add
      a simple bounds-check there, if only to silence static analysis tools.
      
      Per report from Dave Vitek. Backpatch to all supported branches.
      54037476
    • Tom Lane's avatar
      Split up process latch initialization for more-fail-soft behavior. · 4d4005cb
      Tom Lane authored
      In the previous coding, new backend processes would attempt to create their
      self-pipe during the OwnLatch call in InitProcess.  However, pipe creation
      could fail if the kernel is short of resources; and the system does not
      recover gracefully from a FATAL error right there, since we have armed the
      dead-man switch for this process and not yet set up the on_shmem_exit
      callback that would disarm it.  The postmaster then forces an unnecessary
      database-wide crash and restart, as reported by Sean Chittenden.
      
      There are various ways we could rearrange the code to fix this, but the
      simplest and sanest seems to be to split out creation of the self-pipe into
      a new function InitializeLatchSupport, which must be called from a place
      where failure is allowed.  For most processes that gets called in
      InitProcess or InitAuxiliaryProcess, but processes that don't call either
      but still use latches need their own calls.
      
      Back-patch to 9.1, which has only a part of the latch logic that 9.2 and
      HEAD have, but nonetheless includes this bug.
      4d4005cb
  15. Oct 12, 2012
    • Tom Lane's avatar
      Make equal() ignore CoercionForm fields for better planning with casts. · 0fbd4438
      Tom Lane authored
      This change ensures that the planner will see implicit and explicit casts
      as equivalent for all purposes, except in the minority of cases where
      there's actually a semantic difference (as reflected by having a 3-argument
      cast function).  In particular, this fixes cases where the EquivalenceClass
      machinery failed to consider two references to a varchar column as
      equivalent if one was implicitly cast to text but the other was explicitly
      cast to text, as seen in bug #7598 from Vaclav Juza.  We have had similar
      bugs before in other parts of the planner, so I think it's time to fix this
      problem at the core instead of continuing to band-aid around it.
      
      Remove set_coercionform_dontcare(), which represents the band-aid
      previously in use for allowing matching of index and constraint expressions
      with inconsistent cast labeling.  (We can probably get rid of
      COERCE_DONTCARE altogether, but I don't think removing that enum value in
      back branches would be wise; it's possible there's third party code
      referring to it.)
      
      Back-patch to 9.2.  We could go back further, and might want to once this
      has been tested more; but for the moment I won't risk destabilizing plan
      choices in long-since-stable branches.
      0fbd4438
  16. Oct 11, 2012
    • Tom Lane's avatar
      Fix cross-type case in partial row matching for hashed subplans. · a963c8b8
      Tom Lane authored
      When hashing a subplan like "WHERE (a, b) NOT IN (SELECT x, y FROM ...)",
      findPartialMatch() attempted to match rows using the hashtable's internal
      equality operators, which of course are for x and y's datatypes.  What we
      need to use are the potentially cross-type operators for a=x, b=y, etc.
      Failure to do that leads to wrong answers or even crashes.  The scope for
      problems is limited to cases where we have different types with compatible
      hash functions (else we'd not be using a hashed subplan), but for example
      int4 vs int8 can cause the problem.
      
      Per bug #7597 from Bo Jensen.  This has been wrong since the hashed-subplan
      code was written, so patch all the way back.
      a963c8b8
  17. Oct 10, 2012
  18. Oct 09, 2012
    • Tom Lane's avatar
      Fix lo_import and lo_export to return useful error messages more often. · c5b8daa0
      Tom Lane authored
      I found that these functions tend to return -1 while leaving an empty error
      message string in the PGconn, if they suffer some kind of I/O error on the
      file.  The reason is that lo_close, which thinks it's executed a perfectly
      fine SQL command, clears the errorMessage.  The minimum-change workaround
      is to reorder operations here so that we don't fill the errorMessage until
      after lo_close.
      c5b8daa0
    • Tom Lane's avatar
      Fix lo_export usage in example programs. · cf9d9929
      Tom Lane authored
      lo_export returns -1, not zero, on failure.
      cf9d9929
  19. Oct 08, 2012
  20. Oct 05, 2012
  21. Oct 04, 2012
    • Tom Lane's avatar
      Fix permissions explanations in CREATE DATABASE and CREATE SCHEMA docs. · cb4083df
      Tom Lane authored
      These reference pages still claimed that you have to be superuser to create
      a database or schema owned by a different role.  That was true before 8.1,
      but it was changed in commits aa111062 and
      f91370cd to allow assignment of ownership
      to any role you are a member of.  However, at the time we were thinking of
      that primarily as a change to the ALTER OWNER rules, so the need to touch
      these two CREATE ref pages got missed.
      cb4083df
Loading