1. 03 Apr, 2016 6 commits
    • Tom Lane's avatar
      Clean up dubious code in contrib/seg. · a75a418d
      Tom Lane authored
      The restore() function assumed that the result of sprintf() with %e format
      would necessarily contain an 'e', which is false: what if the supplied
      number is an infinity or NaN?  If that did happen, we'd get a
      null-pointer-dereference core dump.  The case appears impossible currently,
      because seg_in() does not accept such values, and there are no seg-creating
      functions that would create one.  But it seems unwise to rely on it never
      happening in future.
      
      Quite aside from that, the code was pretty ugly: it relied on modifying a
      static format string when it could use a "*" precision argument, and it
      used strtok() entirely gratuitously, and it stripped off trailing spaces
      by hand instead of just not asking for them to begin with.
      
      Coverity noticed the potential null pointer dereference (though I wonder
      why it didn't complain years ago, since this code is ancient).
      
      Since this is just code cleanup and forestalling a hypothetical future
      bug, there seems no need for back-patching.
      a75a418d
    • Tom Lane's avatar
      Fix contrib/bloom to not fail under CLOBBER_CACHE_ALWAYS. · 8f75fd1f
      Tom Lane authored
      The code was supposing that rd_amcache wouldn't disappear from under it
      during a scan; which is wrong.  Copy the data out of the relcache rather
      than trying to reference it there.
      8f75fd1f
    • Tom Lane's avatar
      Clean up some stuff in new contrib/bloom module. · a9284849
      Tom Lane authored
      Coverity complained about implicit sign-extension in the
      BloomPageGetFreeSpace macro, probably because sizeOfBloomTuple isn't wide
      enough for size calculations.  No overflow is really possible as long as
      maxoff and sizeOfBloomTuple are small enough to represent a realistic
      situation, but it seems like a good idea to declare sizeOfBloomTuple as
      Size not int32.
      
      Add missing check on BloomPageAddItem() result, again from Coverity.
      
      Avoid core dump due to not allocating so->sign array when
      scan->numberOfKeys is zero.  Also thanks to Coverity.
      
      Use FLEXIBLE_ARRAY_MEMBER rather than declaring an array as size 1
      when it isn't necessarily.
      
      Very minor beautification of related code.
      
      Unfortunately, none of the Coverity-detected mistakes look like they
      could account for the remaining buildfarm unhappiness with this
      module.  It's barely possible that the FLEXIBLE_ARRAY_MEMBER mistake
      does account for that, if it's enabling bogus compiler optimizations;
      but I'm not terribly optimistic.  We probably still have bugs to
      find here.
      a9284849
    • Simon Riggs's avatar
      Avoid pin scan for replay of XLOG_BTREE_VACUUM in all cases · 3e4b7d87
      Simon Riggs authored
      Replay of XLOG_BTREE_VACUUM during Hot Standby was previously thought to require
      complex interlocking that matched the requirements on the master. This required
      an O(N) operation that became a significant problem with large indexes, causing
      replication delays of seconds or in some cases minutes while the
      XLOG_BTREE_VACUUM was replayed.
      
      This commit skips the pin scan that was previously required, by observing in
      detail when and how it is safe to do so, with full documentation. The pin
      scan is skipped only in replay; the VACUUM code path on master is not
      touched here and WAL is identical.
      
      The current commit applies in all cases, effectively replacing commit
      687f2cd7.
      3e4b7d87
    • Tom Lane's avatar
      Add psql \errverbose command to see last server error at full verbosity. · 3cc38ca7
      Tom Lane authored
      Often, upon getting an unexpected error in psql, one's first wish is that
      the verbosity setting had been higher; for example, to be able to see the
      schema-name field or the server code location info.  Up to now the only way
      has been to adjust the VERBOSITY variable and repeat the failing query.
      That's a pain, and it doesn't work if the error isn't reproducible.
      
      This commit adds a psql feature that redisplays the most recent server
      error at full verbosity, without needing to make any variable changes or
      re-execute the failed command.  We just need to hang onto the latest error
      PGresult in case the user executes \errverbose, and then apply libpq's
      new PQresultVerboseErrorMessage() function to it.  This will consume
      some trivial amount of psql memory, but otherwise the cost when the
      feature isn't used should be negligible.
      
      Alex Shulgin, reviewed by Daniel Vérité, some improvements by me
      3cc38ca7
    • Tom Lane's avatar
      Add libpq support for recreating an error message with different verbosity. · e3161b23
      Tom Lane authored
      Often, upon getting an unexpected error in psql, one's first wish is that
      the verbosity setting had been higher; for example, to be able to see the
      schema-name field or the server code location info.  Up to now the only way
      has been to adjust the VERBOSITY variable and repeat the failing query.
      That's a pain, and it doesn't work if the error isn't reproducible.
      
      This commit adds support in libpq for regenerating the error message for
      an existing error PGresult at any desired verbosity level.  This is almost
      just a matter of refactoring the existing code into a subroutine, but there
      is one bit of possibly-needed information that was not getting put into
      PGresults: the text of the last query sent to the server.  We must add that
      string to the contents of an error PGresult.  But we only need to save it
      if it might be used, which with the existing error-formatting code only
      happens if there is a PG_DIAG_STATEMENT_POSITION error field, which is
      probably pretty rare for errors in production situations.  So really the
      overhead when the feature isn't used should be negligible.
      
      Alex Shulgin, reviewed by Daniel Vérité, some improvements by me
      e3161b23
  2. 02 Apr, 2016 9 commits
  3. 01 Apr, 2016 13 commits
    • Alvaro Herrera's avatar
      f07d18b6
    • Tom Lane's avatar
      Omit null rows when setting the threshold for what's a most-common value. · 3d3bf62f
      Tom Lane authored
      As with the previous patch, large numbers of null rows could skew this
      calculation unfavorably, causing us to discard values that have a
      legitimate claim to be MCVs, since our definition of MCV is that it's
      most common among the non-null population of the column.  Hence, make
      the numerator of avgcount be the number of non-null sample values not
      the number of sample rows; likewise for maxmincount in the
      compute_scalar_stats variant.
      
      Also, make the denominator be the number of distinct values actually
      observed in the sample, rather than reversing it back out of the computed
      stadistinct.  This avoids depending on the accuracy of the Haas-Stokes
      approximation, and really it's what we want anyway; the threshold should
      depend only on what we see in the sample, not on what we extrapolate
      about the contents of the whole column.
      
      Alex Shulgin, reviewed by Tomas Vondra and myself
      3d3bf62f
    • Alvaro Herrera's avatar
      pgbench: Remove unused parameter · 5cb88267
      Alvaro Herrera authored
      For some reason this parameter was introduced as unused in 3da0dfb4,
      and has never been used for anything.  Remove it.
      
      Author: Fabien Coelho
      5cb88267
    • Tom Lane's avatar
      Omit null rows when applying the Haas-Stokes estimator for ndistinct. · be4b4dc7
      Tom Lane authored
      Previously, we included null rows in the values of n and N that went
      into the formula, which amounts to considering null as a value in its
      own right; but the d and f1 values do not include nulls.  This is
      inconsistent, and it contributes to significant underestimation of
      ndistinct when the column is mostly nulls.  In any case stadistinct
      is defined as the number of distinct non-null values, so we should
      exclude nulls when doing this computation.
      
      This is an aboriginal bug in our application of the Haas-Stokes formula,
      but we'll refrain from back-patching for fear of destabilizing plan
      choices in released branches.
      
      While at it, make the code a bit more readable by omitting unnecessary
      casts and intermediate variables.
      
      Observation and original patch by Tomas Vondra, adjusted to fix both
      uses of the formula by Alex Shulgin, cosmetic improvements by me
      be4b4dc7
    • Alvaro Herrera's avatar
      Fix logical_decoding_timelines test crashes · 82c83b33
      Alvaro Herrera authored
      In the test_slot_timelines test module, we were abusing passing NULL
      values which was received as zeroes in x86, but this breaks in ARM
      (buildfarm member hamster) by crashing instead.  Fix the breakage by
      marking these functions as STRICT; the InvalidXid value that was
      previously implicit in NULL values (on x86 at least) can now be passed
      as 0.  Failing to follow the fmgr protocol to check for NULLs beforehand
      was causing ARM to fail, as evidenced by segmentation faults in
      buildfarm member hamster.
      
      In order to use the new functionality in the test script, use COALESCE
      in the right spot to avoid forwarding NULL values.
      
      This was diagnosed from the hamster crash by Craig Ringer, who also
      proposed a different patch (checking for NULL values explicitely in the
      C function code, and keeping the non-strictness in the C functions).
      I decided to go with this approach instead.
      82c83b33
    • Teodor Sigaev's avatar
      Fixes in bloom contrib module missed during review · 27f3bbfa
      Teodor Sigaev authored
      - macroses llike (var & FLAG) are changed to ((var & FLAG) != 0)
      - do not copy uninitialized part of notFullPage array to page
      27f3bbfa
    • Alvaro Herrera's avatar
      Type names should not be quoted · f402b995
      Alvaro Herrera authored
      Our actual convention, contrary to what I said in 59a2111b, is not to
      quote type names, as evidenced by unquoted use of format_type_be()
      result value in error messages.  Remove quotes from recently tweaked
      messages accordingly.
      
      Per note from Tom Lane
      f402b995
    • Tom Lane's avatar
      Get rid of minus zero in box regression test. · a067b504
      Tom Lane authored
      Commit acdf2a8b added a test case involving minus zero as a box endpoint.
      This is not very portable, as evidenced by the several older buildfarm
      members that are failing on the test because they print minus zero as
      just "0".  If there were any significant reason to test this behavior,
      we could consider carrying a separate expected-file; but it doesn't look
      to me like there's adequate justification to accept such a maintenance
      burden.  Just change the test to use plain zero, instead.
      a067b504
    • Tom Lane's avatar
      Fix oversight in getParamDescriptions(), and improve comments. · 23066960
      Tom Lane authored
      When getParamDescriptions was changed to handle out-of-memory better
      by cribbing error recovery logic from getRowDescriptions/getAnotherTuple,
      somebody omitted to copy the stanza about checking for excess data in
      the message.  But you need to do that, since continue'ing out of the
      switch in pqParseInput3 means no such check gets applied there anymore.
      Noted while looking at Michael Paquier's patch that made yet another
      copy of this advance_and_error logic.
      
      (This whole business desperately needs refactoring, because I sure don't
      want to see a dozen copies of this code, but that's where we seem to be
      headed.  What's more, the "suspend parsing on EOF return" convention is a
      holdover from protocol 2 and shouldn't exist at all in protocol 3, because
      we don't process partial messages anymore.  But for now, just fix the
      obvious bug.)
      
      Also, fix some wrong/missing comments about what the API spec is
      for these three functions.
      
      This doesn't seem worthy of back-patching, even though it's a bug;
      the case shouldn't ever arise in the field.
      23066960
    • Teodor Sigaev's avatar
      Fix English in bloom module documentation · a361c22e
      Teodor Sigaev authored
      Author: Erik Rijkers
      a361c22e
    • Teodor Sigaev's avatar
      Bloom index contrib module · 9ee014fc
      Teodor Sigaev authored
      Module provides new access method. It is actually a simple Bloom filter
      implemented as pgsql's index. It could give some benefits on search
      with large number of columns.
      
      Module is a single way to test generic WAL interface committed earlier.
      
      Author: Teodor Sigaev, Alexander Korotkov
      Reviewers: Aleksander Alekseev, Michael Paquier, Jim Nasby
      9ee014fc
    • Teodor Sigaev's avatar
      Fix typo in generic wal docs · 4e56e5a6
      Teodor Sigaev authored
      Markus Nullmeier
      4e56e5a6
    • Teodor Sigaev's avatar
      Add Generic WAL interface · 65578341
      Teodor Sigaev authored
      This interface is designed to give an access to WAL for extensions which
      could implement new access method, for example. Previously it was
      impossible because restoring from custom WAL would need to access system
      catalog to find a redo custom function. This patch suggests generic way
      to describe changes on page with standart layout.
      
      Bump XLOG_PAGE_MAGIC because of new record type.
      
      Author: Alexander Korotkov with a help of Petr Jelinek, Markus Nullmeier and
      	minor editorization by my
      Reviewers: Petr Jelinek, Alvaro Herrera, Teodor Sigaev, Jim Nasby,
      	Michael Paquier
      65578341
  4. 31 Mar, 2016 6 commits
    • Tom Lane's avatar
      Another zic portability fix. · c202ecf9
      Tom Lane authored
      I should have remembered that we can't use INT64_MODIFIER with sscanf():
      configure chooses that to work with snprintf(), but it might be for our
      src/port/snprintf.c implementation and so not compatible with the
      platform's sscanf().  This appears to be the explanation for buildfarm
      member frogmouth's continuing unhappiness with the tzcode update.
      
      Fortunately, in all of the places where zic is attempting to read into
      an int64 variable, it's reading a year which certainly will fit just fine
      into an int.  So make it read into an int with %d, and then cast or copy
      as necessary.
      c202ecf9
    • Alvaro Herrera's avatar
      Fix recovery_min_apply_delay test · 61608d38
      Alvaro Herrera authored
      Previously this test was relying too much on WAL replay to occur in the
      exact configured interval, which was unreliable on slow or overly busy
      servers.  Use a custom loop instead of poll_query_until, which is
      hopefully more reliable.
      
      Per continued failures on buildfarm member hamster (which is probably
      the only one running this test suite)
      
      Author: Michaël Paquier
      61608d38
    • Tom Lane's avatar
      Support using index-only scans with partial indexes in more cases. · f9aefcb9
      Tom Lane authored
      Previously, the planner would reject an index-only scan if any restriction
      clause for its table used a column not available from the index, even
      if that restriction clause would later be dropped from the plan entirely
      because it's implied by the index's predicate.  This is a fairly common
      situation for partial indexes because predicates using columns not included
      in the index are often the most useful kind of predicate, and we have to
      duplicate (or at least imply) the predicate in the WHERE clause in order
      to get the index to be considered at all.  So index-only scans were
      essentially unavailable with such partial indexes.
      
      To fix, we have to do detection of implied-by-predicate clauses much
      earlier in the planner.  This patch puts it in check_index_predicates
      (nee check_partial_indexes), meaning it gets done for every partial index,
      whereas we previously only considered this issue at createplan time,
      so that the work was only done for an index actually selected for use.
      That could result in a noticeable planning slowdown for queries against
      tables with many partial indexes.  However, testing suggested that there
      isn't really a significant cost, especially not with reasonable numbers
      of partial indexes.  We do get a small additional benefit, which is that
      cost_index is more accurate since it correctly discounts the evaluation
      cost of clauses that will be removed.  We can also avoid considering such
      clauses as potential indexquals, which saves useless matching cycles in
      the case where the predicate columns aren't in the index, and prevents
      generating bogus plans that double-count the clause's selectivity when
      the columns are in the index.
      
      Tomas Vondra and Kyotaro Horiguchi, reviewed by Kevin Grittner and
      Konstantin Knizhnik, and whacked around a little by me
      f9aefcb9
    • Alvaro Herrera's avatar
      Fix broken variable declaration · 3501f71c
      Alvaro Herrera authored
      Author: Konstantin Knizhnik
      3501f71c
    • Alvaro Herrera's avatar
      Blind attempt at fixing Win32 issue on 24c5f1a1 · 3dd0792a
      Alvaro Herrera authored
      As best as I can tell, MyReplicationSlot needs to be PGDLLIMPORT in
      order for the new test_slot_timelines test module to compile.
      
      Per buildfarm
      3dd0792a
    • Fujii Masao's avatar
      Use proper format specifier %X/%X for LSN. · cee31f5f
      Fujii Masao authored
      cee31f5f
  5. 30 Mar, 2016 6 commits
    • Alvaro Herrera's avatar
      I forgot the alternate expected file in previous commit · 3a3b3090
      Alvaro Herrera authored
      Without this, the test_slot_timelines modules fails "make installcheck"
      because the required feature is not enabled in a stock server.
      
      Per buildfarm
      3a3b3090
    • Alvaro Herrera's avatar
      Enable logical slots to follow timeline switches · 24c5f1a1
      Alvaro Herrera authored
      When decoding from a logical slot, it's necessary for xlog reading to be
      able to read xlog from historical (i.e. not current) timelines;
      otherwise, decoding fails after failover, because the archives are in
      the historical timeline.  This is required to make "failover logical
      slots" possible; it currently has no other use, although theoretically
      it could be used by an extension that creates a slot on a standby and
      continues to replay from the slot when the standby is promoted.
      
      This commit includes a module in src/test/modules with functions to
      manipulate the slots (which is not otherwise possible in SQL code) in
      order to enable testing, and a new test in src/test/recovery to ensure
      that the behavior is as expected.
      
      Author: Craig Ringer
      Reviewed-By: Oleksii Kliukin, Andres Freund, Petr Jelínek
      24c5f1a1
    • Alvaro Herrera's avatar
      XLogReader general code cleanup · 3b02ea4f
      Alvaro Herrera authored
      Some minor tweaks and comment additions, for cleanliness sake and to
      avoid having the upcoming timeline-following patch be polluted with
      unrelated cleanup.
      
      Extracted from a larger patch by Craig Ringer, reviewed by Andres
      Freund, with some additions by myself.
      3b02ea4f
    • Tom Lane's avatar
      Improve portability of I/O behavior for the geometric types. · 50861cd6
      Tom Lane authored
      Formerly, the geometric I/O routines such as box_in and point_out relied
      directly on strtod() and sprintf() for conversion of the float8 component
      values of their data types.  However, the behavior of those functions is
      pretty platform-dependent, especially for edge-case values such as
      infinities and NaNs.  This was exposed by commit acdf2a8b, which
      added test cases involving boxes with infinity endpoints, and immediately
      failed on Windows and AIX buildfarm members.  We solved these problems
      years ago in the main float8in and float8out functions, so let's fix it
      by making the geometric types use that code instead of depending directly
      on the platform-supplied functions.
      
      To do this, refactor the float8in code so that it can be used to parse
      just part of a string, and as a convenience make the guts of float8out
      usable without going through DirectFunctionCall.
      
      While at it, get rid of geo_ops.c's fairly shaky assumptions about the
      maximum output string length for a double, by having it build results in
      StringInfo buffers instead of fixed-length strings.
      
      In passing, convert all the "invalid input syntax for type foo" messages
      in this area of the code into "invalid input syntax for type %s" to reduce
      the number of distinct translatable strings, per recent discussion.
      We would have needed a fair number of the latter anyway for code-sharing
      reasons, so we might as well just go whole hog.
      
      Note: this patch is by no means intended to guarantee that the geometric
      types uniformly behave sanely for infinity or NaN component values.
      But any bugs we have in that line were there all along, they were just
      harder to reach in a platform-independent way.
      50861cd6
    • Tom Lane's avatar
      Suppress uninitialized-variable warnings. · 818e5937
      Tom Lane authored
      My compiler doesn't like the lack of initialization of "flag", and
      I think it's right: if there were zero keys we'd have an undefined
      result.  The AND of zero items is TRUE, so initialize to TRUE.
      818e5937
    • Teodor Sigaev's avatar
      Bump catalog version, forget in acdf2a8b · 2d02a856
      Teodor Sigaev authored
      2d02a856