1. 11 Apr, 2016 16 commits
    • Kevin Grittner's avatar
      Use static inline function for BufferGetPage() · a6f6b781
      Kevin Grittner authored
      I was initially concerned that the some of the hundreds of
      references to BufferGetPage() where the literal
      BGP_NO_SNAPSHOT_TEST were passed might not optimize as well as a
      macro, leading to some hard-to-find performance regressions in
      corner cases.  Inspection of disassembled code has shown identical
      code at all inspected locations, and the size difference doesn't
      amount to even one byte per such call.  So make it readable.
      
      Per gripes from Álvaro Herrera and Tom Lane
      a6f6b781
    • Kevin Grittner's avatar
      Make oldSnapshotControl a pointer to a volatile structure · 80647bf6
      Kevin Grittner authored
      It was incorrectly declared as a volatile pointer to a non-volatile
      structure.  Eliminate the OldSnapshotControl struct definition; it
      is really not needed.  Pointed out by Tom Lane.
      
      While at it, add OldSnapshotControlData to pgindent's list of
      structures.
      80647bf6
    • Peter Eisentraut's avatar
      Fix whitespace · d8ed83cd
      Peter Eisentraut authored
      d8ed83cd
    • Stephen Frost's avatar
      Prefix RLS regression test roles with 'regress_' · 6c7b0388
      Stephen Frost authored
      To avoid any possible overlap with existing roles on a system when
      doing a 'make installcheck', use role names which start with
      'regress_'.
      
      Pointed out by Tom.
      6c7b0388
    • Peter Eisentraut's avatar
      29ca231b
    • Tom Lane's avatar
      Fix missing "volatile" in PLy_output(). · 81ba9348
      Tom Lane authored
      Commit 5c3c3cd0 plastered "volatile" on a bunch of variables
      in PLy_output(), but removed the one that actually mattered, ie the
      one on "oldcontext".  This allows some versions of clang to generate
      code in which "oldcontext" has been trashed when control reaches the
      PG_CATCH block.  Per buildfarm member tick.
      81ba9348
    • Peter Eisentraut's avatar
      cpluspluscheck: Update include path · ee5dbc81
      Peter Eisentraut authored
      Some things in src/include/fe_utils require libpq headers, so add
      libpq's include path to the command line used here.
      ee5dbc81
    • Fujii Masao's avatar
    • Fujii Masao's avatar
      Use ereport(ERROR) instead of Assert() to emit syncrep_parser error. · 0038c1e2
      Fujii Masao authored
      The existing code would either Assert or generate an invalid
      SyncRepConfig variable, neither of which is desirable. A regular
      error should be thrown instead.
      
      This commit silences compiler warning in non assertion-enabled builds.
      
      Per report from Jeff Janes.
      Suggested fix by Tom Lane.
      0038c1e2
    • Tom Lane's avatar
      Fix poorly thought-through code from commit 5c3c3cd0. · f73b2bbb
      Tom Lane authored
      It's not entirely clear to me whether PyString_AsString can return
      null (looks like the answer might vary between Python 2 and 3).
      But in any case, this code's attempt to cope with the possibility
      was quite broken, because pstrdup() neither allows a null argument
      nor ever returns a null.
      
      Moreover, the code below this point assumes that "message" is a
      palloc'd string, which would not be the case for a dgettext result.
      
      Fix both problems by doing the pstrdup step separately.
      f73b2bbb
    • Tom Lane's avatar
      pg_dump: add missing "destroyPQExpBuffer(query)" in dumpForeignServer(). · 074050f1
      Tom Lane authored
      Coverity complained about this resource leak (why now, I don't know,
      since it's been like that a long time).  Our general policy in pg_dump
      is that PQExpBuffers are worth cleaning up, so do it here too.  But
      don't bother with a back-patch, because it seems unlikely that very
      many databases contain enough FOREIGN SERVER objects to notice.
      074050f1
    • Tom Lane's avatar
      Add comment about intentional fallthrough in switch. · 1630f5b9
      Tom Lane authored
      Coverity complained about an apparent missing "break" in a switch
      added by bb140506.  The human-readable comments are pretty
      clear that this is intentional, but add a standard /* FALL THRU */
      comment to make it clear to tools too.
      1630f5b9
    • Tom Lane's avatar
      Clean up foreign-key caching code in planner. · 5306df28
      Tom Lane authored
      Coverity complained that the code added by 015e8894 lacked an
      error check for SearchSysCache1 failures, which it should have.  But
      the code was pretty duff in other ways too, including failure to think
      about whether it could really cope with arrays of different lengths.
      5306df28
    • Tom Lane's avatar
      Fix access-to-already-freed-memory issue in plpython's error handling. · 7e3bb080
      Tom Lane authored
      PLy_elog() could attempt to access strings that Python had already freed,
      because the strings that PLy_get_spi_error_data() returns are simply
      pointers into storage associated with the error "val" PyObject.  That's
      fine at the instant PLy_get_spi_error_data() returns them, but just after
      that PLy_traceback() intentionally releases the only refcount on that
      object, allowing it to be freed --- so that the strings we pass to
      ereport() are dangling pointers.
      
      In principle this could result in garbage output or a coredump.  In
      practice, I think the risk is pretty low, because there are no Python
      operations between where we decrement that refcount and where we use the
      strings (and copy them into PG storage), and thus no reason for Python
      to recycle the storage.  Still, it's clearly hazardous, and it leads to
      Valgrind complaints when running under a Valgrind that hasn't been
      lobotomized to ignore Python memory allocations.
      
      The code was a mess anyway: we fetched the error data out of Python
      (clearing Python's error indicator) with PyErr_Fetch, examined it, pushed
      it back into Python with PyErr_Restore (re-setting the error indicator),
      then immediately pulled it back out with another PyErr_Fetch.  Just to
      confuse matters even more, there were some gratuitous-and-yet-hazardous
      PyErr_Clear calls in the "examine" step, and we didn't get around to doing
      PyErr_NormalizeException until after the second PyErr_Fetch, making it even
      less clear which object was being manipulated where and whether we still
      had a refcount on it.  (If PyErr_NormalizeException did substitute a
      different "val" object, it's possible that the problem could manifest for
      real, because then we'd be doing assorted Python stuff with no refcount
      on the object we have string pointers into.)
      
      So, rearrange all that into some semblance of sanity, and don't decrement
      the refcount on the Python error objects until the end of PLy_elog().
      In HEAD, I failed to resist the temptation to reformat some messy bits
      from 5c3c3cd0 along the way.
      
      Back-patch as far as 9.2, because the code is substantially the same
      that far back.  I believe that 9.1 has the bug as well; but the code
      around it is rather different and I don't want to take a chance on
      breaking something for what seems a low-probability problem.
      7e3bb080
    • Andres Freund's avatar
      Avoid the use of a separate spinlock to protect a LWLock's wait queue. · 008608b9
      Andres Freund authored
      Previously we used a spinlock, in adition to the atomically manipulated
      ->state field, to protect the wait queue. But it's pretty simple to
      instead perform the locking using a flag in state.
      
      Due to 6150a1b0 BufferDescs, on platforms (like PPC) with > 1 byte
      spinlocks, increased their size above 64byte. As 64 bytes are the size
      we pad allocated BufferDescs to, this can increase false sharing;
      causing performance problems in turn. Together with the previous commit
      this reduces the size to <= 64 bytes on all common platforms.
      
      Author: Andres Freund
      Discussion: CAA4eK1+ZeB8PMwwktf+3bRS0Pt4Ux6Rs6Aom0uip8c6shJWmyg@mail.gmail.com
          20160327121858.zrmrjegmji2ymnvr@alap3.anarazel.de
      008608b9
    • Andres Freund's avatar
      Allow Pin/UnpinBuffer to operate in a lockfree manner. · 48354581
      Andres Freund authored
      Pinning/Unpinning a buffer is a very frequent operation; especially in
      read-mostly cache resident workloads. Benchmarking shows that in various
      scenarios the spinlock protecting a buffer header's state becomes a
      significant bottleneck. The problem can be reproduced with pgbench -S on
      larger machines, but can be considerably worse for queries which touch
      the same buffers over and over at a high frequency (e.g. nested loops
      over a small inner table).
      
      To allow atomic operations to be used, cram BufferDesc's flags,
      usage_count, buf_hdr_lock, refcount into a single 32bit atomic variable;
      that allows to manipulate them together using 32bit compare-and-swap
      operations. This requires reducing MAX_BACKENDS to 2^18-1 (which could
      be lifted by using a 64bit field, but it's not a realistic configuration
      atm).
      
      As not all operations can easily implemented in a lockfree manner,
      implement the previous buf_hdr_lock via a flag bit in the atomic
      variable. That way we can continue to lock the header in places where
      it's needed, but can get away without acquiring it in the more frequent
      hot-paths.  There's some additional operations which can be done without
      the lock, but aren't in this patch; but the most important places are
      covered.
      
      As bufmgr.c now essentially re-implements spinlocks, abstract the delay
      logic from s_lock.c into something more generic. It now has already two
      users, and more are coming up; there's a follupw patch for lwlock.c at
      least.
      
      This patch is based on a proof-of-concept written by me, which Alexander
      Korotkov made into a fully working patch; the committed version is again
      revised by me.  Benchmarking and testing has, amongst others, been
      provided by Dilip Kumar, Alexander Korotkov, Robert Haas.
      
      On a large x86 system improvements for readonly pgbench, with a high
      client count, of a factor of 8 have been observed.
      
      Author: Alexander Korotkov and Andres Freund
      Discussion: 2400449.GjM57CE0Yg@dinodell
      48354581
  2. 10 Apr, 2016 3 commits
    • Tom Lane's avatar
      Improve contrib/bloom regression test using code coverage info. · cf223c3b
      Tom Lane authored
      Originally, this test created a 100000-row test table, which made it
      run rather slowly compared to other contrib tests.  Investigation with
      gcov showed that we got no further improvement in code coverage after
      the first 700 or so rows, making the large table 99% a waste of time.
      Cut it back to 2000 rows to fix the runtime problem and still leave
      some headroom for testing behaviors that may appear later.
      
      A closer look at the gcov results showed that the main coverage
      omissions in contrib/bloom occurred because the test never filled more
      than one entry in the notFullPage array; which is unsurprising because
      it exercised index cleanup only in the scenario of complete table
      deletion, allowing every page in the index to become deleted rather
      than not-full.  Add testing that allows the not-full path to be
      exercised as well.
      
      Also, test the amvalidate function, because blvalidate.c had zero
      coverage without that, and besides it's a good idea to check for
      mistakes in the bloom opclass definitions.
      cf223c3b
    • Alvaro Herrera's avatar
      Fix possible NULL dereference in ExecAlterObjectDependsStmt · bd905a0d
      Alvaro Herrera authored
      I used the wrong variable here.  Doesn't make a difference today because
      the only plausible caller passes a non-NULL variable, but someday it
      will be wrong, and even today's correctness is subtle: the caller that
      does pass a NULL is never invoked because of object type constraints.
      Surely not a condition to rely on.
      
      Noted by Coverity
      bd905a0d
    • Tom Lane's avatar
      Further minor improvement in generic_xlog.c: always say REGBUF_STANDARD. · 660d5fb8
      Tom Lane authored
      Since we're requiring pages handled by generic_xlog.c to be standard
      format, specify REGBUF_STANDARD when doing a full-page image, so that
      xloginsert.c can compress out the "hole" between pd_lower and pd_upper.
      Given the current API in which this path will be taken only for a newly
      initialized page, the hole is likely to be particularly large in such
      cases, so that this oversight could easily be performance-significant.
      I don't notice any particular change in the runtime of contrib/bloom's
      regression test, though.
      660d5fb8
  3. 09 Apr, 2016 9 commits
    • Tom Lane's avatar
      Micro-optimize GenericXLogFinish(). · 68689c66
      Tom Lane authored
      Make the inner comparison loops of computeDelta() as tight as possible by
      pulling considerations of valid and invalid ranges out of the inner loops,
      and extending a match or non-match detection as far as possible before
      deciding what to do next.  To keep this tractable, give up the possibility
      of merging fragments across the pd_lower to pd_upper gap.  The fraction of
      pages where that could happen (ie, there are 4 or fewer bytes in the gap,
      *and* data changes immediately adjacent to it on both sides) is too small
      to be worth spending cycles on.
      
      Also, avoid two BLCKSZ-length memcpy()s by computing the delta before
      moving data into the target buffer, instead of after.  This doesn't save
      nearly as many cycles as being tenser about computeDelta(), but it still
      seems worth doing.
      
      On my machine, this patch cuts a full 40% off the runtime of
      contrib/bloom's regression test.
      68689c66
    • Tom Lane's avatar
      Fix PL/Python ereport() test to work on Python 2.3. · c7a141a9
      Tom Lane authored
      Per buildfarm.
      
      Pavel Stehule
      c7a141a9
    • Tom Lane's avatar
      Get rid of GenericXLogUnregister(). · 08e78543
      Tom Lane authored
      This routine is unsafe as implemented, because it invalidates the page
      image pointers returned by previous GenericXLogRegister() calls.
      
      Rather than complicate the API or the implementation to avoid that,
      let's just get rid of it; the use-case for having it seems much
      too thin to justify a lot of work here.
      
      While at it, do some wordsmithing on the SGML docs for generic WAL.
      08e78543
    • Tom Lane's avatar
      Get rid of blinsert()'s use of GenericXLogUnregister(). · 80cf1891
      Tom Lane authored
      That routine is dangerous, and unnecessary once we get rid of this
      one caller.
      
      In passing, fix failure to clean up temp memory context, or switch
      back to caller's context, during slowest exit path.
      80cf1891
    • Tom Lane's avatar
      Code review/prettification for generic_xlog.c. · db03cf37
      Tom Lane authored
      Improve commentary, use more specific names for the delta fields,
      const-ify pointer arguments where possible, avoid assuming that
      initializing only the first element of a local array will guarantee
      that the remaining elements end up as we need them.  (I think that
      code in generic_redo actually worked, but only because InvalidBuffer
      is zero; this is a particularly ugly way of depending on that ...)
      db03cf37
    • Tom Lane's avatar
      Run pgindent on generic_xlog.c. · 2dd318d2
      Tom Lane authored
      This code desperately needs some micro-optimization, and I'd like it
      to be formatted a bit more nicely while I work on it.
      2dd318d2
    • Kevin Grittner's avatar
      Fix typo in C comment. · 381200be
      Kevin Grittner authored
      381200be
    • Kevin Grittner's avatar
      Turn special page pointer validation to static inline function · 56dffb5a
      Kevin Grittner authored
      Inclusion of multiple macros inside another macro was pushing MSVC
      past its size liimit.  Reported by buildfarm.
      56dffb5a
    • Alvaro Herrera's avatar
      Move \crosstabview regression tests to a separate file · 1ff3f420
      Alvaro Herrera authored
      It cannot run in the same parallel group as misc, because it creates a
      table which is unpredictably visible in that test.
      
      Per buildfarm member crake.
      1ff3f420
  4. 08 Apr, 2016 12 commits
    • Alvaro Herrera's avatar
      Support \crosstabview in psql · c09b18f2
      Alvaro Herrera authored
      \crosstabview is a completely different way to display results from a
      query: instead of a vertical display of rows, the data values are placed
      in a grid where the column and row headers come from the data itself,
      similar to a spreadsheet.
      
      The sort order of the horizontal header can be specified by using
      another column in the query, and the vertical header determines its
      ordering from the order in which they appear in the query.
      
      This only allows displaying a single value in each cell.  If more than
      one value correspond to the same cell, an error is thrown.  Merging of
      values can be done in the query itself, if necessary.  This may be
      revisited in the future.
      
      Author: Daniel Verité
      Reviewed-by: Pavel Stehule, Dean Rasheed
      c09b18f2
    • Kevin Grittner's avatar
      Add snapshot_too_old to NSVC @contrib_excludes · 279d86af
      Kevin Grittner authored
      The buildfarm showed failure for Windows MSVC builds due to this
      omission.  This might not be the only problem with the Makefile for
      this feature, but hopefully this will get it past the immediate
      problem.
      
      Fix suggested by Tom Lane
      279d86af
    • Andres Freund's avatar
      Expose more out/readfuncs support functions. · c1ddd236
      Andres Freund authored
      Previously bcac23de exposed a subset of support functions, namely the
      ones Kaigai found useful. In
      20160304193704.elq773pyg5fyl3mi@alap3.anarazel.de I mentioned that
      there's some functions missing to use the facility in an external
      project.
      
      To avoid having to add functions piecemeal, add all the functions which
      are used to define READ_* and WRITE_* macros; users of the extensible
      node functionality are likely to need these. Additionally expose
      outDatum(), which doesn't have it's own WRITE_ macro, as it needs
      information from the embedding struct.
      
      Discussion: 20160304193704.elq773pyg5fyl3mi@alap3.anarazel.de
      c1ddd236
    • Stephen Frost's avatar
      Create default roles · 7a542700
      Stephen Frost authored
      This creates an initial set of default roles which administrators may
      use to grant access to, historically, superuser-only functions.  Using
      these roles instead of granting superuser access reduces the number of
      superuser roles required for a system.  Documention for each of the
      default roles has been added to user-manag.sgml.
      
      Bump catversion to 201604082, as we had a commit that bumped it to
      201604081 and another that set it back to 201604071...
      
      Reviews by José Luis Tallón and Robert Haas
      7a542700
    • Stephen Frost's avatar
      Reserve the "pg_" namespace for roles · 29300789
      Stephen Frost authored
      This will prevent users from creating roles which begin with "pg_" and
      will check for those roles before allowing an upgrade using pg_upgrade.
      
      This will allow for default roles to be provided at initdb time.
      
      Reviews by José Luis Tallón and Robert Haas
      29300789
    • Stephen Frost's avatar
      Fix improper usage of 'dump' bitmap · fa6075e5
      Stephen Frost authored
      Now that 'dump' is a bitmap, we can't simply set it to 'true'.
      
      Noticed while debugging the prior issue.
      fa6075e5
    • Kevin Grittner's avatar
      Add the "snapshot too old" feature · 848ef42b
      Kevin Grittner authored
      This feature is controlled by a new old_snapshot_threshold GUC.  A
      value of -1 disables the feature, and that is the default.  The
      value of 0 is just intended for testing.  Above that it is the
      number of minutes a snapshot can reach before pruning and vacuum
      are allowed to remove dead tuples which the snapshot would
      otherwise protect.  The xmin associated with a transaction ID does
      still protect dead tuples.  A connection which is using an "old"
      snapshot does not get an error unless it accesses a page modified
      recently enough that it might not be able to produce accurate
      results.
      
      This is similar to the Oracle feature, and we use the same SQLSTATE
      and error message for compatibility.
      848ef42b
    • Kevin Grittner's avatar
      Modify BufferGetPage() to prepare for "snapshot too old" feature · 8b65cf4c
      Kevin Grittner authored
      This patch is a no-op patch which is intended to reduce the chances
      of failures of omission once the functional part of the "snapshot
      too old" patch goes in.  It adds parameters for snapshot, relation,
      and an enum to specify whether the snapshot age check needs to be
      done for the page at this point.  This initial patch passes NULL
      for the first two new parameters and BGP_NO_SNAPSHOT_TEST for the
      third.  The follow-on patch will change the places where the test
      needs to be made.
      8b65cf4c
    • Stephen Frost's avatar
      In dumpTable, re-instate the skipping logic · 689f9a05
      Stephen Frost authored
      Pretty sure I removed this based on some incorrect thinking that it was
      no longer possible to reach this point for a table which will not be
      dumped, but that's clearly wrong.
      
      Pointed out on IRC by Erik Rijkers.
      689f9a05
    • 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
    • Magnus Hagander's avatar
      Add authentication parameters compat_realm and upn_usename for SSPI · 35e2e357
      Magnus Hagander authored
      These parameters are available for SSPI authentication only, to make
      it possible to make it behave more like "normal gssapi", while
      making it possible to maintain compatibility.
      
      compat_realm is on by default, but can be turned off to make the
      authentication use the full Kerberos realm instead of the NetBIOS name.
      
      upn_username is off by default, and can be turned on to return the users
      Kerberos UPN rather than the SAM-compatible name (a user in Active
      Directory can have both a legacy SAM-compatible username and a new
      Kerberos one. Normally they are the same, but not always)
      
      Author: Christian Ullrich
      Reviewed by: Robbie Harwood, Alvaro Herrera, me
      35e2e357
    • Teodor Sigaev's avatar
      Fix possible use of uninitialised value in ts_headline() · cb0c8cbf
      Teodor Sigaev authored
      Found during investigation of failure of skink buildfarm member and its
      valgrind report.
      
      Backpatch to all supported branches
      cb0c8cbf