1. 06 Oct, 2011 1 commit
    • Tom Lane's avatar
      Improve and simplify CREATE EXTENSION's management of GUC variables. · ba6f6293
      Tom Lane authored
      CREATE EXTENSION needs to transiently set search_path, as well as
      client_min_messages and log_min_messages.  We were doing this by the
      expedient of saving the current string value of each variable, doing a
      SET LOCAL, and then doing another SET LOCAL with the previous value at
      the end of the command.  This is a bit expensive though, and it also fails
      badly if there is anything funny about the existing search_path value,
      as seen in a recent report from Roger Niederland.  Fortunately, there's a
      much better way, which is to piggyback on the GUC infrastructure previously
      developed for functions with SET options.  We just open a new GUC nesting
      level, do our assignments with GUC_ACTION_SAVE, and then close the nesting
      level when done.  This automatically restores the prior settings without a
      re-parsing pass, so (in principle anyway) there can't be an error.  And
      guc.c still takes care of cleanup in event of an error abort.
      
      The CREATE EXTENSION code for this was modeled on some much older code in
      ri_triggers.c, which I also changed to use the better method, even though
      there wasn't really much risk of failure there.  Also improve the comments
      in guc.c to reflect this additional usage.
      ba6f6293
  2. 05 Oct, 2011 2 commits
  3. 04 Oct, 2011 6 commits
    • Tom Lane's avatar
      Improve define_custom_variable's handling of pre-existing settings. · 41e461d3
      Tom Lane authored
      Arrange for any problems with pre-existing settings to be reported as
      WARNING not ERROR, so that we don't undesirably abort the loading of the
      incoming add-on module.  The bad setting is just discarded, as though it
      had never been applied at all.  (This requires a change in the API of
      set_config_option.  After some thought I decided the most potentially
      useful addition was to allow callers to just pass in a desired elevel.)
      
      Arrange to restore the complete stacked state of the variable, rather than
      cheesily reinstalling only the active value.  This ensures that custom GUCs
      will behave unsurprisingly even when the module loading operation occurs
      within nested subtransactions that have changed the active value.  Since a
      module load could occur as a result of, eg, a PL function call, this is not
      an unlikely scenario.
      41e461d3
    • Tom Lane's avatar
      Fix uninitialized-variable bug. · fa56a0c3
      Tom Lane authored
      fa56a0c3
    • Tom Lane's avatar
      Add sourcefile/sourceline data to EXEC_BACKEND GUC transmission files. · 4bcb82a7
      Tom Lane authored
      This oversight meant that on Windows, the pg_settings view would not
      display source file or line number information for values coming from
      postgresql.conf, unless the backend had received a SIGHUP since starting.
      
      In passing, also make the error detection in read_nondefault_variables a
      tad more thorough, and fix it to not lose precision on float GUCs (these
      changes are already in HEAD as of my previous commit).
      4bcb82a7
    • Tom Lane's avatar
      Remember the source GucContext for each GUC parameter. · 9f5836d2
      Tom Lane authored
      We used to just remember the GucSource, but saving GucContext too provides
      a little more information --- notably, whether a SET was done by a
      superuser or regular user.  This allows us to rip out the fairly dodgy code
      that define_custom_variable used to use to try to infer the context to
      re-install a pre-existing setting with.  In particular, it now works for
      a superuser to SET a extension's SUSET custom variable before loading the
      associated extension, because GUC can remember whether the SET was done as
      a superuser or not.  The plperl regression tests contain an example where
      this is useful.
      9f5836d2
    • Alvaro Herrera's avatar
      Use callbacks in SlruScanDirectory for the actual action · 09e196e4
      Alvaro Herrera authored
      Previously, the code assumed that the only possible action to take was
      to delete files behind a certain cutoff point.  The async notify code
      was already a crock: it used a different "pagePrecedes" function for
      truncation than for regular operation.  By allowing it to pass a
      callback to SlruScanDirectory it can do cleanly exactly what it needs to
      do.
      
      The clog.c code also had its own use for SlruScanDirectory, which is
      made a bit simpler with this.
      09e196e4
    • Tom Lane's avatar
      Remove the custom_variable_classes parameter. · 1a00c0ef
      Tom Lane authored
      This variable provides only marginal error-prevention capability (since
      it can only check the prefix of a qualified GUC name), and the consensus
      is that that isn't worth the amount of hassle that maintaining the setting
      creates for DBAs.  So, let's just remove it.
      
      With this commit, the system will silently accept a value for any qualified
      GUC name at all, whether it has anything to do with any known extension or
      not.  (Unqualified names still have to match known built-in settings,
      though; and you will get a WARNING at extension load time if there's an
      unrecognized setting with that extension's prefix.)
      
      There's still some discussion ongoing about whether to tighten that up and
      if so how; but if we do come up with a solution, it's not likely to look
      anything like custom_variable_classes.
      1a00c0ef
  4. 03 Oct, 2011 1 commit
  5. 02 Oct, 2011 1 commit
    • Tom Lane's avatar
      Restructure error handling in reading of postgresql.conf. · d56b3afc
      Tom Lane authored
      This patch has two distinct purposes: to report multiple problems in
      postgresql.conf rather than always bailing out after the first one,
      and to change the policy for whether changes are applied when there are
      unrelated errors in postgresql.conf.
      
      Formerly the policy was to apply no changes if any errors could be
      detected, but that had a significant consistency problem, because in some
      cases specific values might be seen as valid by some processes but invalid
      by others.  This meant that the latter processes would fail to adopt
      changes in other parameters even though the former processes had done so.
      
      The new policy is that during SIGHUP, the file is rejected as a whole
      if there are any errors in the "name = value" syntax, or if any lines
      attempt to set nonexistent built-in parameters, or if any lines attempt
      to set custom parameters whose prefix is not listed in (the new value of)
      custom_variable_classes.  These tests should always give the same results
      in all processes, and provide what seems a reasonably robust defense
      against loading values from badly corrupted config files.  If these tests
      pass, all processes will apply all settings that they individually see as
      good, ignoring (but logging) any they don't.
      
      In addition, the postmaster does not abandon reading a configuration file
      after the first syntax error, but continues to read the file and report
      syntax errors (up to a maximum of 100 syntax errors per file).
      
      The postmaster will still refuse to start up if the configuration file
      contains any errors at startup time, but these changes allow multiple
      errors to be detected and reported before quitting.
      
      Alexey Klyukin, reviewed by Andy Colson and av (Alexander ?)
      with some additional hacking by Tom Lane
      d56b3afc
  6. 01 Oct, 2011 3 commits
    • Tom Lane's avatar
      Improve generated column names for cases involving sub-SELECTs. · 5ec6b7f1
      Tom Lane authored
      We'll now use "exists" for EXISTS(SELECT ...), "array" for ARRAY(SELECT
      ...), or the sub-select's own result column name for a simple expression
      sub-select.  Previously, you usually got "?column?" in such cases.
      
      Marti Raudsepp, reviewed by Kyotaro Horiugchi
      5ec6b7f1
    • Bruce Momjian's avatar
      878b74e0
    • Tom Lane's avatar
      Cache the result of makesign() across calls of gtrgm_penalty(). · 0a5d5a49
      Tom Lane authored
      Since gtrgm_penalty() is usually called many times in a row with the same
      "newval" (to determine which item on an index page newval fits into best),
      the makesign() calculation is repetitious.  It's expensive enough to make
      it worth caching the result, so do so.  On my machine this is good for
      more than a 40% savings in the time needed to build a trigram index on
      /usr/share/dict/words.  This is all per a suggestion of Heikki's.
      
      In passing, make some mostly-cosmetic improvements in the caching logic in
      the other functions in this file that rely on caching info in fn_extra.
      0a5d5a49
  7. 30 Sep, 2011 1 commit
    • Tom Lane's avatar
      Support GiST index support functions that want to cache data across calls. · d22a09dc
      Tom Lane authored
      pg_trgm was already doing this unofficially, but the implementation hadn't
      been thought through very well and leaked memory.  Restructure the core
      GiST code so that it actually works, and document it.  Ordinarily this
      would have required an extra memory context creation/destruction for each
      GiST index search, but I was able to avoid that in the normal case of a
      non-rescanned search by finessing the handling of the RBTree.  It used to
      have its own context always, but now shares a context with the
      scan-lifespan data structures, unless there is more than one rescan call.
      This should make the added overhead unnoticeable in typical cases.
      d22a09dc
  8. 29 Sep, 2011 4 commits
    • Tom Lane's avatar
      Fix recursion into previously planned sub-query in examine_simple_variable. · 79edb2b1
      Tom Lane authored
      This code was looking at the sub-Query tree as seen in the parent query's
      RangeTblEntry; but that's the pristine parser output, and what we need to
      look at is the tree as it stands at the completion of planning.  Otherwise
      we might pick up a Var that references a subquery that got flattened and
      hence has no RelOptInfo in the subroot.  Per report from Peter Geoghegan.
      79edb2b1
    • Bruce Momjian's avatar
      Fix pg_upgrade for EXEC_BACKEND builds (e.g. Windows) by properly · 054219c9
      Bruce Momjian authored
      passing the -b/binary-upgrade flag.
      
      Backpatch to 9.1.X.
      054219c9
    • Tom Lane's avatar
      Fix index matching for operators with mixed collatable/noncollatable inputs. · cb37c291
      Tom Lane authored
      If an indexable operator for a non-collatable indexed datatype has a
      collatable right-hand input type, any OpExpr for it will be marked with a
      nonzero inputcollid (since having one collatable input is sufficient to
      make that happen).  However, an index on a non-collatable column certainly
      doesn't have any collation.  This caused us to fail to match such operators
      to their indexes, because indxpath.c required an exact match of index
      collation and clause collation.  It seems correct to allow a match when the
      index is collation-less regardless of the clause's inputcollid: an operator
      with both noncollatable and collatable inputs could perhaps depend on the
      collation of the collatable input, but it could hardly expect the index for
      the noncollatable input to have that same collation.
      
      Per bug #6232 from Pierre Ducroquet.  His example is specifically about
      "hstore ? text" but the problem seems quite generic.
      cb37c291
    • Bruce Momjian's avatar
      In pg_upgrade, because toast table names can be mismatched with the heap · 5e595842
      Bruce Momjian authored
      oid on 8.4, modify the toast name comparison test to only apply to old
      9.0+ servers.  (The test was previously 8.4+.)
      
      Backpatch to 9.1.X.
      5e595842
  9. 28 Sep, 2011 2 commits
    • Tom Lane's avatar
      Update and extend the EXPLAIN-related documentation. · a32dd164
      Tom Lane authored
      I've made a significant effort at filling in the "Using EXPLAIN" section
      to be reasonably complete about mentioning everything that EXPLAIN can
      output, including the "Rows Removed" outputs that were added by Marko
      Tiikkaja's recent documentation-free patch.  I also updated the examples to
      be consistent with current behavior; several of them were not close to what
      the current code will do.  No doubt there's more that can be done here, but
      I'm out of patience for today.
      a32dd164
    • Tom Lane's avatar
      Take sepgsql regression tests out of the regular regression test mechanism. · cc4ff874
      Tom Lane authored
      Because these tests require root privileges, not to mention invasive
      changes to the security configuration of the host system, it's not
      reasonable for them to be invoked by a regular "make check" or "make
      installcheck".  Instead, dike out the Makefile's knowledge of the tests,
      and change chkselinuxenv (now renamed "test_sepgsql") into a script that
      verifies the environment is workable and then runs the tests.  It's
      expected that test_sepgsql will only be run manually.
      
      While at it, do some cleanup in the error checking in the script, and
      do some wordsmithing in the documentation.
      cc4ff874
  10. 27 Sep, 2011 7 commits
    • Alvaro Herrera's avatar
      Remove dependency on error ordering in isolation tests · bbd38af3
      Alvaro Herrera authored
      We now report errors reported by the just-unblocked and unblocking
      transactions identically; this should fix relatively common buildfarm
      failures reported by animals that are failing the "wrong" session.
      bbd38af3
    • Alvaro Herrera's avatar
      Fix typo · 17349927
      Alvaro Herrera authored
      17349927
    • Robert Haas's avatar
      Update comments related to the crash-safety of the visibility map. · f70648d5
      Robert Haas authored
      In hio.c, document how we avoid deadlock with respect to visibility map
      buffer locks.  In visibilitymap.c, update the LOCKING section of the
      file header comment.
      
      Both oversights noted by Heikki Linnakangas.
      f70648d5
    • Robert Haas's avatar
      sepgsql uavc comment improvements. · a9d845be
      Robert Haas authored
      Robert Haas and KaiGai Kohei
      a9d845be
    • Robert Haas's avatar
      heap_update() must recheck tuple after unlocking and relocking buffer. · 624f155f
      Robert Haas authored
      Bug found by Alvaro Herrera, fix suggested by Heikki Linnakangas
      and reviewed by Tom Lane.
      624f155f
    • Tom Lane's avatar
      Fix window functions that sort by expressions involving aggregates. · 269c5dd2
      Tom Lane authored
      In commit c1d9579d, I changed things so
      that the output of the Agg node that feeds the window functions would not
      list any ungrouped Vars directly.  Formerly, for example, the Agg tlist
      might have included both "x" and "sum(x)", which is not really valid if
      "x" isn't a grouping column.  If we then had a window function ordering on
      something like "sum(x) + 1", prepare_sort_from_pathkeys would find no exact
      match for this in the Agg tlist, and would conclude that it must recompute
      the expression.  But it would break the expression down to just the Var
      "x", which it would find in the tlist, and then rebuild the ORDER BY
      expression using a reference to the subplan's "x" output.  Now, after the
      above-referenced changes, "x" isn't in the Agg tlist if it's not a grouping
      column, so that prepare_sort_from_pathkeys fails with "could not find
      pathkey item to sort", as reported by Bricklen Anderson.
      
      The fix is to not break down Aggrefs into their component parts, but just
      treat them as irreducible expressions to be sought in the subplan tlist.
      This is definitely OK for the use with respect to window functions in
      grouping_planner, since it just built the tlist being used on the same
      basis.  AFAICT it is safe for other uses too; most of the other call sites
      couldn't encounter Aggrefs anyway.
      269c5dd2
    • Tom Lane's avatar
      Allow snapshot references to still work during transaction abort. · 57eb0090
      Tom Lane authored
      In REPEATABLE READ (nee SERIALIZABLE) mode, an attempt to do
      GetTransactionSnapshot() between AbortTransaction and CleanupTransaction
      failed, because GetTransactionSnapshot would recompute the transaction
      snapshot (which is already wrong, given the isolation mode) and then
      re-register it in the TopTransactionResourceOwner, leading to an Assert
      because the TopTransactionResourceOwner should be empty of resources after
      AbortTransaction.  This is the root cause of bug #6218 from Yamamoto
      Takashi.  While changing plancache.c to avoid requesting a snapshot when
      handling a ROLLBACK masks the problem, I think this is really a snapmgr.c
      bug: it's lower-level than the resource manager mechanism and should not be
      shutting itself down before we unwind resource manager resources.  However,
      just postponing the release of the transaction snapshot until cleanup time
      didn't work because of the circular dependency with
      TopTransactionResourceOwner.  Fix by managing the internal reference to
      that snapshot manually instead of depending on TopTransactionResourceOwner.
      This saves a few cycles as well as making the module layering more
      straightforward.  predicate.c's dependencies on TopTransactionResourceOwner
      go away too.
      
      I think this is a longstanding bug, but there's no evidence that it's more
      than a latent bug, so it doesn't seem worth any risk of back-patching.
      57eb0090
  11. 26 Sep, 2011 3 commits
  12. 25 Sep, 2011 4 commits
    • Tom Lane's avatar
      Fully const-ify PQconnectdbParams, PQconnectStartParams, and PQpingParams. · 2a571bc2
      Tom Lane authored
      The keywords and values arguments of these functions are more properly
      declared "const char * const *" than just "const char **".
      
      Lionel Elie Mamane, reviewed by Craig Ringer
      2a571bc2
    • Tom Lane's avatar
      Avoid unnecessary snapshot-acquisitions in BuildCachedPlan. · d5aa7a9f
      Tom Lane authored
      I had copied-and-pasted a claim that we couldn't reach this point when
      dealing with utility statements, but that was a leftover from when the
      caller was required to supply a plan to start with.  We now will go
      through here at least once when handling a utility statement, so it
      seems worth a check to see whether a snapshot is actually needed.
      (Note that analyze_requires_snapshot is quite a cheap test.)
      
      Per suggestion from Yamamoto Takashi.  I don't think I believe that this
      resolves his reported assertion failure; but it's worth changing anyway,
      just to save a cycle or two.
      d5aa7a9f
    • Bruce Momjian's avatar
      In pgrminclude, document requirement to use pgcompinclude, and sort · 2594ad74
      Bruce Momjian authored
      files so include removal is more predictable.
      2594ad74
    • Tom Lane's avatar
      Un-break compression of plain-text output format in pg_dump. · 23fe7a74
      Tom Lane authored
      pg_dump has historically understood -Z with no -F switch to mean that
      it should emit a gzip-compressed version of its plain text output.
      This got broken through a misunderstanding in the 9.1 patch that added
      directory output format.  Restore the former behavior.
      
      Per complaint from Roger Niederland and diagnosis by Adrian Klaver.
      23fe7a74
  13. 24 Sep, 2011 5 commits
    • Bruce Momjian's avatar
      Document pgrminclude limitations. · 84bbe57d
      Bruce Momjian authored
      84bbe57d
    • Tom Lane's avatar
      Recognize self-contradictory restriction clauses for non-table relations. · 7741dd65
      Tom Lane authored
      The constraint exclusion feature checks for contradictions among scan
      restriction clauses, as well as contradictions between those clauses and a
      table's CHECK constraints.  The first aspect of this testing can be useful
      for non-table relations (such as subqueries or functions-in-FROM), but the
      feature was coded with only the CHECK case in mind so we were applying it
      only to plain-table RTEs.  Move the relation_excluded_by_constraints call
      so that it is applied to all RTEs not just plain tables.  With the default
      setting of constraint_exclusion this results in no extra work, but with
      constraint_exclusion = ON we will detect optimizations that we missed
      before (at the cost of more planner cycles than we expended before).
      
      Per a gripe from Gunnlaugur Þór Briem.  Experimentation with
      his example also showed we were not being very bright about the case where
      constraint exclusion is proven within a subquery within UNION ALL, so tweak
      the code to allow set_append_rel_pathlist to recognize such cases.
      7741dd65
    • Bruce Momjian's avatar
    • Magnus Hagander's avatar
      Fix typo · 0126db2a
      Magnus Hagander authored
      0126db2a
    • Magnus Hagander's avatar
      Note that sslmode=require verifies the CA if root cert is present · 33e81fdf
      Magnus Hagander authored
      This mode still exists for backwards compatibility, making
      sslmode=require the same as sslmode=verify-ca when the file is present,
      but not causing an error when it isn't.
      
      Per bug 6189, reported by Srinivas Aji
      33e81fdf