1. 04 Jun, 2015 4 commits
    • Tom Lane's avatar
      Stabilize query plans in rowsecurity regression test. · 5cdf25e1
      Tom Lane authored
      Some recent buildfarm failures can be explained by supposing that
      autovacuum or autoanalyze fired on the tables created by this test,
      resulting in plan changes.  Do a proactive VACUUM ANALYZE on the
      test's principal tables to try to forestall such changes.
      5cdf25e1
    • Fujii Masao's avatar
      Remove -i/--ignore-version option from pg_dump, pg_dumpall and pg_restore. · 232cd63b
      Fujii Masao authored
      The commit c22ed3d5 turned
      the -i/--ignore-version options into no-ops and marked as deprecated.
      Considering we shipped that in 8.4, it's time to remove all trace of
      those switches, per discussion. We'd still have to wait a couple releases
      before it'd be safe to use -i for something else, but it'd be a start.
      232cd63b
    • Fujii Masao's avatar
      Fix some issues in pg_class.relminmxid and pg_database.datminmxid documentation. · 38d500ac
      Fujii Masao authored
      - Correct the name of directory which those catalog columns allow to be shrunk.
      - Correct the name of symbol which is used as the value of pg_class.relminmxid
        when the relation is not a table.
      - Fix "ID ID" typo.
      
      Backpatch to 9.3 where those cataog columns were introduced.
      38d500ac
    • Peter Eisentraut's avatar
      doc: Fix PDF build with FOP · afae1f78
      Peter Eisentraut authored
      Because of a bug in the DocBook XSL FO style sheet, an xref to a
      varlistentry whose term includes an indexterm fails to build.  One such
      instance was introduced in commit
      5086dfce.  Fix by adding the upstream
      bug fix to our customization layer.
      afae1f78
  2. 03 Jun, 2015 3 commits
    • Tom Lane's avatar
      Fix some questionable edge-case behaviors in add_path() and friends. · 3b0f7760
      Tom Lane authored
      add_path_precheck was doing exact comparisons of path costs, but it really
      needs to do them fuzzily to be sure it won't reject paths that could
      survive add_path's comparisons.  (This can only matter if the initial cost
      estimate is very close to the final one, but that turns out to often be
      true.)
      
      Also, it should ignore startup cost for this purpose if and only if
      compare_path_costs_fuzzily would do so.  The previous coding always ignored
      startup cost for parameterized paths, which is wrong as of commit
      3f59be83; it could result in improper early rejection of paths that
      we care about for SEMI/ANTI joins.  It also always considered startup cost
      for unparameterized paths, which is just as wrong though the only effect is
      to waste planner cycles on paths that can't survive.  Instead, it should
      consider startup cost only when directed to by the consider_startup/
      consider_param_startup relation flags.
      
      Likewise, compare_path_costs_fuzzily should have symmetrical behavior
      for parameterized and unparameterized paths.  In this case, the best
      answer seems to be that after establishing that total costs are fuzzily
      equal, we should compare startup costs whether or not the consider_xxx
      flags are on.  That is what it's always done for unparameterized paths,
      so let's make the behavior for parameterized  paths match.
      
      These issues were noted while developing the SEMI/ANTI join costing fix
      of commit 3f59be83, but we chose not to back-patch these fixes,
      because they can cause changes in the planner's choices among
      nearly-same-cost plans.  (There is in fact one minor change in plan choice
      within the core regression tests.)  Destabilizing plan choices in back
      branches without very clear improvements is frowned on, so we'll just fix
      this in HEAD.
      3b0f7760
    • Tom Lane's avatar
      Fix planner's cost estimation for SEMI/ANTI joins with inner indexscans. · 3f59be83
      Tom Lane authored
      When the inner side of a nestloop SEMI or ANTI join is an indexscan that
      uses all the join clauses as indexquals, it can be presumed that both
      matched and unmatched outer rows will be processed very quickly: for
      matched rows, we'll stop after fetching one row from the indexscan, while
      for unmatched rows we'll have an indexscan that finds no matching index
      entries, which should also be quick.  The planner already knew about this,
      but it was nonetheless charging for at least one full run of the inner
      indexscan, as a consequence of concerns about the behavior of materialized
      inner scans --- but those concerns don't apply in the fast case.  If the
      inner side has low cardinality (many matching rows) this could make an
      indexscan plan look far more expensive than it actually is.  To fix,
      rearrange the work in initial_cost_nestloop/final_cost_nestloop so that we
      don't add the inner scan cost until we've inspected the indexquals, and
      then we can add either the full-run cost or just the first tuple's cost as
      appropriate.
      
      Experimentation with this fix uncovered another problem: add_path and
      friends were coded to disregard cheap startup cost when considering
      parameterized paths.  That's usually okay (and desirable, because it thins
      the path herd faster); but in this fast case for SEMI/ANTI joins, it could
      result in throwing away the desired plain indexscan path in favor of a
      bitmap scan path before we ever get to the join costing logic.  In the
      many-matching-rows cases of interest here, a bitmap scan will do a lot more
      work than required, so this is a problem.  To fix, add a per-relation flag
      consider_param_startup that works like the existing consider_startup flag,
      but applies to parameterized paths, and set it for relations that are the
      inside of a SEMI or ANTI join.
      
      To make this patch reasonably safe to back-patch, care has been taken to
      avoid changing the planner's behavior except in the very narrow case of
      SEMI/ANTI joins with inner indexscans.  There are places in
      compare_path_costs_fuzzily and add_path_precheck that are not terribly
      consistent with the new approach, but changing them will affect planner
      decisions at the margins in other cases, so we'll leave that for a
      HEAD-only fix.
      
      Back-patch to 9.3; before that, the consider_startup flag didn't exist,
      meaning that the second aspect of the patch would be too invasive.
      
      Per a complaint from Peter Holzer and analysis by Tomas Vondra.
      3f59be83
    • Fujii Masao's avatar
      Minor improvement to txid_current() documentation. · 37013621
      Fujii Masao authored
      Michael Paquier, reviewed by Christoph Berg and Naoya Anzai
      37013621
  3. 01 Jun, 2015 5 commits
  4. 31 May, 2015 1 commit
    • Peter Eisentraut's avatar
      Make Python tests more portable · 75f9d176
      Peter Eisentraut authored
      Newer Python versions randomize the hash seed for dictionaries,
      resulting in a random output order, which messes up the regression test
      diffs.
      
      Instead, use Python assert to compare the dictionaries with their
      expected value.
      75f9d176
  5. 29 May, 2015 6 commits
    • Bruce Momjian's avatar
      ac6f2295
    • Tom Lane's avatar
      initdb -S should now have an explicit check that $PGDATA is valid. · 1943c000
      Tom Lane authored
      The fsync code from the backend essentially assumes that somebody's already
      validated PGDATA, at least to the extent of it being a readable directory.
      That's safe enough for initdb's normal code path too, but "initdb -S"
      doesn't have any other processing at all that touches the target directory.
      To have reasonable error-case behavior, add a pg_check_dir call.
      Per gripe from Peter E.
      1943c000
    • Tom Lane's avatar
      Remove special cases for ETXTBSY from new fsync'ing logic. · 57e1138b
      Tom Lane authored
      The argument that this is a sufficiently-expected case to be silently
      ignored seems pretty thin.  Andres had brought it up back when we were
      still considering that most fsync failures should be hard errors, and it
      probably would be legit not to fail hard for ETXTBSY --- but the same is
      true for EROFS and other cases, which is why we gave up on hard failures.
      ETXTBSY is surely not a normal case, so logging the failure seems fine
      from here.
      57e1138b
    • Tom Lane's avatar
      Check that all aliases of a built-in function have same leakproof property. · 1c8c656b
      Tom Lane authored
      opr_sanity.sql has a test checking that relevant properties of built-in
      functions match when the same C function is referenced by multiple pg_proc
      entries.  The test neglected to check proleakproof, though, and when
      I added that condition it exposed that xideqint4 hadn't been updated to
      match xideq.  So fix that as well, and in consequence bump catversion.
      
      This isn't very critical, so no need to worry about fixing back branches.
      1c8c656b
    • Tom Lane's avatar
      Adjust initdb to also not consider fsync'ing failures fatal. · c07d8c96
      Tom Lane authored
      Make initdb's version of this logic look as much like the backend's
      as possible.  This is much less critical than in the backend since not
      so many people use "initdb -S", but we want the same corner-case error
      handling in both cases.
      
      Back-patch to 9.3 where initdb -S option was introduced.  Before that,
      initdb only had to deal with freshly-created data directories, wherein
      no failures should be expected.
      
      Abhijit Menon-Sen
      c07d8c96
    • Tom Lane's avatar
      Revert exporting of internal GUC variable "data_directory". · da33a389
      Tom Lane authored
      This undoes a poorly-thought-out choice in commit 970a1868, namely
      to export guc.c's internal variable data_directory.  The authoritative
      variable so far as C code is concerned is DataDir; there is no reason for
      anything except specific bits of GUC code to look at the GUC variable.
      
      After yesterday's commits fixing the fsync-on-restart patch, the only
      remaining misuse of data_directory was in AlterSystemSetConfigFile(),
      which would be much better off just using a relative path anyhow: it's
      less code and it doesn't break if the DBA moves the data directory of a
      running system, which is a case we've taken some pains over in the past.
      
      This is mostly cosmetic, so no need for a back-patch (and I'd be hesitant
      to remove a global variable in stable branches anyway).
      da33a389
  6. 28 May, 2015 7 commits
    • Tom Lane's avatar
      Fix fsync-at-startup code to not treat errors as fatal. · d8179b00
      Tom Lane authored
      Commit 2ce439f3 introduced a rather serious
      regression, namely that if its scan of the data directory came across any
      un-fsync-able files, it would fail and thereby prevent database startup.
      Worse yet, symlinks to such files also caused the problem, which meant that
      crash restart was guaranteed to fail on certain common installations such
      as older Debian.
      
      After discussion, we agreed that (1) failure to start is worse than any
      consequence of not fsync'ing is likely to be, therefore treat all errors
      in this code as nonfatal; (2) we should not chase symlinks other than
      those that are expected to exist, namely pg_xlog/ and tablespace links
      under pg_tblspc/.  The latter restriction avoids possibly fsync'ing a
      much larger part of the filesystem than intended, if the user has left
      random symlinks hanging about in the data directory.
      
      This commit takes care of that and also does some code beautification,
      mainly moving the relevant code into fd.c, which seems a much better place
      for it than xlog.c, and making sure that the conditional compilation for
      the pre_sync_fname pass has something to do with whether pg_flush_data
      works.
      
      I also relocated the call site in xlog.c down a few lines; it seems a
      bit silly to be doing this before ValidateXLOGDirectoryStructure().
      
      The similar logic in initdb.c ought to be made to match this, but that
      change is noncritical and will be dealt with separately.
      
      Back-patch to all active branches, like the prior commit.
      
      Abhijit Menon-Sen and Tom Lane
      d8179b00
    • Stephen Frost's avatar
      Remove *pgaudit* references also. · d5442cb2
      Stephen Frost authored
      Fixes the docs build.
      d5442cb2
    • Stephen Frost's avatar
      Finish removing pg_audit · cde9cf17
      Stephen Frost authored
      cde9cf17
    • Tom Lane's avatar
      Fix pg_rewind's handling of top-level symlinks. · 0381fefa
      Tom Lane authored
      The previous coding suffered a null-pointer dereference if it found any
      symlink at the top level of $PGDATA.  Fix that, and teach it to recurse
      into a symlink for pg_xlog, but not anything else.
      
      Per note from Abhijit Menon-Sen.
      0381fefa
    • Stephen Frost's avatar
      Remove pg_audit · e5f1a4f1
      Stephen Frost authored
      This removes pg_audit, per discussion:
      
      20150528082038.GU26667@tamriel.snowman.net
      e5f1a4f1
    • Tom Lane's avatar
      Fix assorted inconsistencies in our calls of readlink(). · 32f628be
      Tom Lane authored
      Ensure that we null-terminate the result string (one place in pg_rewind).
      Be paranoid about out-of-range results from readlink() (should not happen,
      but there is no good reason for some call sites to be careful about it and
      others not).  Consistently use the whole buffer, not sometimes one byte
      less.  Ensure we emit an appropriate errcode() in all cases.  Spell the
      error messages the same way.
      
      The only serious bug here is the missing null-termination in pg_rewind,
      which is new code, so no need for a back-patch.
      
      Abhijit Menon-Sen and Tom Lane
      32f628be
    • Tom Lane's avatar
      Fix pg_get_functiondef() to print a function's LEAKPROOF property. · f46edf47
      Tom Lane authored
      Seems to have been an oversight in the original leakproofness patch.
      Per report and patch from Jeevan Chalke.
      
      In passing, prettify some awkward leakproof-related code in AlterFunction.
      f46edf47
  7. 27 May, 2015 4 commits
    • Tom Lane's avatar
      Fix portability issue in isolationtester grammar. · aa9eac45
      Tom Lane authored
      specparse.y and specscanner.l used "string" as a token name.  Now, bison
      likes to define each token name as a macro for the token code it assigns,
      which means those names are basically off-limits for any other use within
      the grammar file or included headers.  So names as generic as "string" are
      dangerous.  This is what was causing the recent failures on protosciurus:
      some versions of Solaris' sys/kstat.h use "string" as a field name.
      With late-model bison we don't see this problem because the token macros
      aren't defined till later (that is why castoroides didn't show the problem
      even though it's on the same machine).  But protosciurus uses bison 1.875
      which defines the token macros up front.
      
      This land mine has been there from day one; we'd have found it sooner
      except that protosciurus wasn't trying to run the isolation tests till
      recently.
      
      To fix, rename the token to "string_literal" which is hopefully less
      likely to collide with names used by system headers.  Back-patch to
      all branches containing the isolation tests.
      aa9eac45
    • Andrew Dunstan's avatar
      Revert "Add all structured objects passed to pushJsonbValue piecewise." · f41042ce
      Andrew Dunstan authored
      This reverts commit 54547bd8.
      
      This appears to have been a thinko on my part. I will try to come up
      wioth a better solution.
      f41042ce
    • Andrew Dunstan's avatar
      Revert "Simplify addJsonbToParseState()" · 956cc443
      Andrew Dunstan authored
      This reverts commit fba12c8c.
      
      This relied on a commit that is also being reverted.
      956cc443
    • Tom Lane's avatar
      Remove configure check prohibiting threaded libpython on OpenBSD. · 86832eb8
      Tom Lane authored
      According to recent tests, this case now works fine, so there's no reason
      to reject it anymore.  (Even if there are still some OpenBSD platforms
      in the wild where it doesn't work, removing the check won't break any case
      that worked before.)
      
      We can actually remove the entire test that discovers whether libpython
      is threaded, since without the OpenBSD case there's no need to know that
      at all.
      
      Per report from Davin Potts.  Back-patch to all active branches.
      86832eb8
  8. 26 May, 2015 4 commits
    • Tom Lane's avatar
      Suppress occasional failures in brin regression test. · 1f303fd1
      Tom Lane authored
      brin.sql included a call of brin_summarize_new_values(), and expected
      it to always report exactly 5 summarization events.  This failed sometimes
      during parallel regression tests, as a consequence of the database-wide
      VACUUM in gist.sql getting there first.  The most future-proof way
      to avoid variation in the test results is to forget about using
      brin_summarize_new_values() and just do a plain "VACUUM brintest",
      which will exercise the same code anyway.
      
      Having done that, there's no need for preventing autovacuum on brintest;
      doing so just reduces the scope of test coverage, so let's not.
      1f303fd1
    • Andrew Dunstan's avatar
      Simplify addJsonbToParseState() · fba12c8c
      Andrew Dunstan authored
      This function no longer needs to walk non-scalar structures passed to
      it, following commit 54547bd8.
      fba12c8c
    • Andrew Dunstan's avatar
      Add all structured objects passed to pushJsonbValue piecewise. · 54547bd8
      Andrew Dunstan authored
      Commit 9b74f32cdbff8b9be47fc69164eae552050509ff did this for objects of
      type jbvBinary, but in trying further to simplify some of the new jsonb
      code I discovered that objects of type jbvObject or jbvArray passed as
      WJB_ELEM or WJB_VALUE also caused problems. These too are now added
      component by component.
      
      Backpatch to 9.4.
      54547bd8
    • Tom Lane's avatar
      Fix valgrind's "unaddressable bytes" whining about BRIN code. · 79f2b5d5
      Tom Lane authored
      brin_form_tuple calculated an exact tuple size, then palloc'd and
      filled just that much.  Later, brin_doinsert or brin_doupdate would
      MAXALIGN the tuple size and tell PageAddItem that that was the size
      of the tuple to insert.  If the original tuple size wasn't a multiple
      of MAXALIGN, the net result would be that PageAddItem would memcpy
      a few more bytes than the palloc request had been for.
      
      AFAICS, this is totally harmless in the real world: the error is a
      read overrun not a write overrun, and palloc would certainly have
      rounded the request up to a MAXALIGN multiple internally, so there's
      no chance of the memcpy fetching off the end of memory.  Valgrind,
      however, is picky to the byte level not the MAXALIGN level.
      
      Fix it by pushing the MAXALIGN step back to brin_form_tuple.  (The other
      possible source of tuples in this code, brin_form_placeholder_tuple,
      was already producing a MAXALIGN'd result.)
      
      In passing, be a bit more paranoid about internal allocations in
      brin_form_tuple.
      79f2b5d5
  9. 25 May, 2015 6 commits