1. 25 Nov, 2017 6 commits
    • Tom Lane's avatar
      Avoid formally-undefined use of memcpy() in hstoreUniquePairs(). · d3f4e8a8
      Tom Lane authored
      hstoreUniquePairs() often called memcpy with equal source and destination
      pointers.  Although this is almost surely harmless in practice, it's
      undefined according to the letter of the C standard.  Some versions of
      valgrind will complain about it, and some versions of libc as well
      (cf. commit ad520ec4).  Tweak the code to avoid doing that.
      
      Noted by Tomas Vondra.  Back-patch to all supported versions because
      of the hazard of libc assertions.
      
      Discussion: https://postgr.es/m/bf84d940-90d4-de91-19dd-612e011007f4@fuzzy.cz
      d3f4e8a8
    • Tom Lane's avatar
      Repair failure with SubPlans in multi-row VALUES lists. · 9b63c13f
      Tom Lane authored
      When nodeValuesscan.c was written, it was impossible to have a SubPlan in
      VALUES --- any sub-SELECT there would have to be uncorrelated and thereby
      would produce an InitPlan instead.  We therefore took a shortcut in the
      logic that throws away a ValuesScan's per-row expression evaluation data
      structures.  This was broken by the introduction of LATERAL however; a
      sub-SELECT containing a lateral reference produces a correlated SubPlan.
      
      The cleanest fix for this would be to give up the optimization of
      discarding the expression eval state.  But that still seems pretty
      unappetizing for long VALUES lists.  It seems to work to just prevent
      the subexpressions from hooking into the ValuesScan node's subPlan
      list, so let's do that and see how well it works.  (If this breaks,
      due to additional connections between the subexpressions and the outer
      query structures, we might consider compromises like throwing away data
      only for VALUES rows not containing SubPlans.)
      
      Per bug #14924 from Christian Duta.  Back-patch to 9.3 where LATERAL
      was introduced.
      
      Discussion: https://postgr.es/m/20171124120836.1463.5310@wrigleys.postgresql.org
      9b63c13f
    • Tom Lane's avatar
      Update buffile.h/.c comments for removal of non-temp option. · ab97aaac
      Tom Lane authored
      Commit 11e26451 removed BufFile's isTemp flag, thereby eliminating
      the possibility of resurrecting BufFileCreate().  But it left that
      function in place, as well as a bunch of comments describing how things
      worked for the non-temp-file case.  At best, that's now a source of
      confusion.  So remove the long-since-commented-out function and change
      relevant comments.
      
      I (tgl) wanted to rename BufFileCreateTemp() to BufFileCreate(), but
      that seems not to be the consensus position, so leave it as-is.
      
      In passing, fix commit f0828b2f's failure to update BufFileSeek's
      comment to match the change of its argument type from long to off_t.
      (I think that might actually have been intentional at the time, but
      now that 64-bit off_t is nearly universal, it looks anachronistic.)
      
      Thomas Munro and Tom Lane
      
      Discussion: https://postgr.es/m/E1eFVyl-0008J1-RO@gemulon.postgresql.org
      ab97aaac
    • Tom Lane's avatar
      Improve planner's handling of set-returning functions in grouping columns. · df3a66e2
      Tom Lane authored
      Improve query_is_distinct_for() to accept SRFs in the targetlist when
      we can prove distinctness from a DISTINCT clause.  In that case the
      de-duplication will surely happen after SRF expansion, so the proof
      still works.  Continue to punt in the case where we'd try to prove
      distinctness from GROUP BY (or, in the future, source relations).
      To do that, we'd have to determine whether the SRFs were in the
      grouping columns or elsewhere in the tlist, and it still doesn't
      seem worth the trouble.  But this trivial change allows us to
      recognize that "SELECT DISTINCT unnest(foo) FROM ..." produces
      unique-ified output, which seems worth having.
      
      Also, fix estimate_num_groups() to consider the possibility of SRFs in
      the grouping columns.  Its failure to do so was masked before v10 because
      grouping_planner() scaled up plan rowcount estimates by the estimated SRF
      multiplier after performing grouping.  That doesn't happen anymore, which
      is more correct, but it means we need an adjustment in the estimate for
      the number of groups.  Failure to do this leads to an underestimate for
      the number of output rows of subqueries like "SELECT DISTINCT unnest(foo)"
      compared to what 9.6 and earlier estimated, thus breaking plan choices
      in some cases.
      
      Per report from Dmitry Shalashov.  Back-patch to v10 to avoid degraded
      plan choices compared to previous releases.
      
      Discussion: https://postgr.es/m/CAKPeCUGAeHgoh5O=SvcQxREVkoX7UdeJUMj1F5=aBNvoTa+O8w@mail.gmail.com
      df3a66e2
    • Robert Haas's avatar
      Avoid projecting tuples unnecessarily in Gather and Gather Merge. · b10967ed
      Robert Haas authored
      It's most often the case that the target list for the Gather (Merge)
      node matches the target list supplied by the underlying plan node;
      when this is so, we can avoid the overhead of projecting.
      
      This depends on commit f455e112 for
      proper functioning.
      
      Idea by Andres Freund.  Patch by me.  Review by Amit Kapila.
      
      Discussion: http://postgr.es/m/CA+TgmoZ0ZL=cesZFq8c9NnfK6bqy-wwUd3_74iYGodYrSoQ7Fw@mail.gmail.com
      b10967ed
    • Tom Lane's avatar
      Improve valgrind logic in aset.c, and fix multiple issues in generation.c. · 0f2458ff
      Tom Lane authored
      Revise aset.c so that all the "private" fields of chunk headers are
      marked NOACCESS when outside the module, improving on the previous
      coding which protected only requested_size.  Fix a couple of corner
      case bugs, such as failing to re-protect the header during a failure
      exit from AllocSetRealloc, and wrong padding-size calculation for an
      oversize allocation request.
      
      Apply the same design to generation.c, and also fix several bugs therein
      that I found by dint of hacking the code to use generation.c as the
      standard allocator and then running the core regression tests with it.
      Notably, we have to track the actual size of each block, else the
      wipe_mem call in GenerationReset clears the wrong amount of memory for
      an oversize-chunk block; and GenerationCheck needs a way of identifying
      freed chunks that isn't fooled by palloc(0).  I chose to fix the latter
      by resetting the context pointer to NULL in a freed chunk, roughly like
      what happens in a freed aset.c chunk.
      
      Discussion: https://postgr.es/m/E1eHa4J-0006hI-Q8@gemulon.postgresql.org
      0f2458ff
  2. 24 Nov, 2017 8 commits
  3. 23 Nov, 2017 4 commits
  4. 22 Nov, 2017 6 commits
  5. 21 Nov, 2017 5 commits
    • Tom Lane's avatar
      pgbench: fix stats reporting when some transactions are skipped. · 16827d44
      Tom Lane authored
      pgbench can skip some transactions when both -R and -L options are used.
      Previously, this resulted in slightly silly statistics both in progress
      reports and final output, because the skipped transactions were counted
      as executed for TPS and related stats.  Discount skipped xacts in TPS
      numbers, and also when figuring the percentage of xacts exceeding the
      latency limit.
      
      Also, don't print per-script skipped-transaction counts when there is
      only one script.  That's redundant with the overall count, and it's
      inconsistent with the fact that we don't print other per-script stats
      when there's only one script.  Clean up some unnecessary interactions
      between what should be independent options that were due to that
      decision.
      
      While at it, avoid division-by-zero in cases where no transactions were
      executed.  While on modern platforms this would generally result in
      printing "NaN" rather than a crash, that isn't spelled consistently
      across platforms and it would confuse many people.  Skip the relevant
      output entirely when practical, else print zeroes.
      
      Fabien Coelho, reviewed by Steve Singer, additional hacking by me
      
      Discussion: https://postgr.es/m/26654.1505232433@sss.pgh.pa.us
      16827d44
    • Tom Lane's avatar
      Doc: fix broken markup. · 41761265
      Tom Lane authored
      41761265
    • Robert Haas's avatar
      Provide for forward compatibility with future minor protocol versions. · ae65f606
      Robert Haas authored
      Previously, any attempt to request a 3.x protocol version other than
      3.0 would lead to a hard connection failure, which made the minor
      protocol version really no different from the major protocol version
      and precluded gentle protocol version breaks.  Instead, when the
      client requests a 3.x protocol version where x is greater than 0, send
      the new NegotiateProtocolVersion message to convey that we support
      only 3.0.  This makes it possible to introduce new minor protocol
      versions without requiring a connection retry when the server is
      older.
      
      In addition, if the startup packet includes name/value pairs where
      the name starts with "_pq_.", assume that those are protocol options,
      not GUCs.  Include those we don't support (i.e. all of them, at
      present) in the NegotiateProtocolVersion message so that the client
      knows they were not understood.  This makes it possible for the
      client to request previously-unsupported features without bumping
      the protocol version at all; the client can tell from the server's
      response whether the option was understood.
      
      It will take some time before servers that support these new
      facilities become common in the wild; to speed things up and make
      things easier for a future 3.1 protocol version, back-patch to all
      supported releases.
      
      Robert Haas and Badrul Chowdhury
      
      Discussion: http://postgr.es/m/BN6PR21MB0772FFA0CBD298B76017744CD1730@BN6PR21MB0772.namprd21.prod.outlook.com
      Discussion: http://postgr.es/m/30788.1498672033@sss.pgh.pa.us
      ae65f606
    • Robert Haas's avatar
      Fix multiple problems with satisfies_hash_partition. · f3b0897a
      Robert Haas authored
      Fix the function header comment to describe the actual behavior.
      Check that table OID, modulus, and remainder arguments are not NULL
      before accessing them.  Check that the modulus and remainder are
      sensible.  If the table OID doesn't exist, return NULL instead of
      emitting an internal error, similar to what we do elsewhere.  Check
      that the actual argument types match, or at least are binary coercible
      to, the expected argument types.  Correctly handle invocation of this
      function using the VARIADIC syntax.  Add regression tests.
      
      Robert Haas and Amul Sul, per a report by Andreas Seltenreich and
      subsequent followup investigation.
      
      Discussion: http://postgr.es/m/871sl4sdrv.fsf@ansel.ydns.eu
      f3b0897a
    • Tom Lane's avatar
      Support index-only scans in contrib/cube and contrib/seg GiST indexes. · de1d042f
      Tom Lane authored
      To do this, we only have to remove the compress and decompress support
      functions, which have never done anything more than detoasting.
      In the wake of commit d3a4f89d, this results in automatically enabling
      index-only scans, since the core code will now know that the stored
      representation is the same as the original data (up to detoasting).
      
      The only exciting part of this is that ALTER OPERATOR FAMILY lacks
      a way to drop a support function that was declared as being part of
      an opclass rather than being loose in the family.  For the moment,
      we'll hack our way to a solution with a manual update of the pg_depend
      entry type, which is what distinguishes the two cases.  Perhaps
      someday it'll be worth providing a cleaner way to do that, but for
      now it seems like a very niche problem.
      
      Note that the underlying C functions remain, to support use of the shared
      libraries with older versions of the modules' SQL declarations.  Someday
      we may be able to remove them, but not soon.
      
      Andrey Borodin, reviewed by me
      
      Discussion: https://postgr.es/m/D0F53A05-4F4A-4DEC-8339-3C069FA0EE11@yandex-team.ru
      de1d042f
  6. 20 Nov, 2017 6 commits
  7. 19 Nov, 2017 1 commit
  8. 18 Nov, 2017 4 commits
    • Tom Lane's avatar
      Fix compiler warning in rangetypes_spgist.c. · 52f63bd9
      Tom Lane authored
      On gcc 7.2.0, comparing pointer to (Datum) 0 produces a warning.
      Treat it as a simple pointer to avoid that; this is more consistent
      with comparable code elsewhere, anyway.
      
      Tomas Vondra
      
      Discussion: https://postgr.es/m/99410021-61ef-9a9a-9bc8-f733ece637ee@2ndquadrant.com
      52f63bd9
    • Tom Lane's avatar
      Merge near-duplicate code in RI triggers. · 4797f9b5
      Tom Lane authored
      Merge ri_restrict_del and ri_restrict_upd into one function ri_restrict.
      Create a function ri_setnull that is the common implementation of
      RI_FKey_setnull_del and RI_FKey_setnull_upd.  Likewise create a function
      ri_setdefault that is the common implementation of RI_FKey_setdefault_del
      and RI_FKey_setdefault_upd.  All of these pairs of functions were identical
      except for needing to check for no-actual-key-change in the UPDATE cases;
      the one extra if-test is a small price to pay for saving so much code.
      
      Aside from removing about 400 lines of essentially duplicate code, this
      allows us to recognize that we were uselessly caching two identical plans
      whenever there were pairs of triggers using these duplicated functions
      (which is likely very common).
      
      Ildar Musin, reviewed by Ildus Kurbangaliev
      
      Discussion: https://postgr.es/m/ca7064a7-6adc-6f22-ca47-8615ba9425a5@postgrespro.ru
      4797f9b5
    • Peter Eisentraut's avatar
      Consistently catch errors from Python _New() functions · d0aa965c
      Peter Eisentraut authored
      Python Py*_New() functions can fail and return NULL in out-of-memory
      conditions.  The previous code handled that inconsistently or not at
      all.  This change organizes that better.  If we are in a function that
      is called from Python, we just check for failure and return NULL
      ourselves, which will cause any exception information to be passed up.
      If we are called from PostgreSQL, we consistently create an "out of
      memory" error.
      Reviewed-by: default avatarTom Lane <tgl@sss.pgh.pa.us>
      d0aa965c
    • Tom Lane's avatar
      Improve to_date/to_number/to_timestamp behavior with multibyte characters. · 976a1a48
      Tom Lane authored
      The documentation says that these functions skip one input character
      per literal (non-pattern) format character.  Actually, though, they
      skipped one input *byte* per literal *byte*, which could be hugely
      confusing if either data or format contained multibyte characters.
      
      To fix, adjust the FormatNode representation and parse_format() so
      that multibyte format characters are stored as one FormatNode not
      several, and adjust the data-skipping bits to advance by pg_mblen()
      not necessarily one byte.  There's no user-visible behavior change
      on the to_char() side, although the internal representation changes.
      
      Commit e87d4965 had already fixed most places where we skip characters
      on the basis of non-literal format patterns to advance by characters
      not bytes, but this gets one more place, the SKIP_THth macro.  I think
      everything in formatting.c gets that right now.
      
      It'd be nice to have some regression test cases covering this behavior;
      but of course there's no way to do so in an encoding-agnostic way, and
      many of the interesting aspects would also require unportable locale
      selections.  So I've not bothered here.
      
      Discussion: https://postgr.es/m/28186.1510957703@sss.pgh.pa.us
      976a1a48