1. 29 Nov, 2017 8 commits
  2. 28 Nov, 2017 10 commits
  3. 27 Nov, 2017 4 commits
    • Tom Lane's avatar
      Fix creation of resjunk tlist entries for inherited mixed UPDATE/DELETE. · 9a785ad5
      Tom Lane authored
      rewriteTargetListUD's processing is dependent on the relkind of the query's
      target table.  That was fine at the time it was made to act that way, even
      for queries on inheritance trees, because all tables in an inheritance tree
      would necessarily be plain tables.  However, the 9.5 feature addition
      allowing some members of an inheritance tree to be foreign tables broke the
      assumption that rewriteTargetListUD's output tlist could be applied to all
      child tables with nothing more than column-number mapping.  This led to
      visible failures if foreign child tables had row-level triggers, and would
      also break in cases where child tables belonged to FDWs that used methods
      other than CTID for row identification.
      
      To fix, delay running rewriteTargetListUD until after the planner has
      expanded inheritance, so that it is applied separately to the (already
      mapped) tlist for each child table.  We can conveniently call it from
      preprocess_targetlist.  Refactor associated code slightly to avoid the
      need to heap_open the target relation multiple times during
      preprocess_targetlist.  (The APIs remain a bit ugly, particularly around
      the point of which steps scribble on parse->targetList and which don't.
      But avoiding such scribbling would require a change in FDW callback APIs,
      which is more pain than it's worth.)
      
      Also fix ExecModifyTable to ensure that "tupleid" is reset to NULL when
      we transition from rows providing a CTID to rows that don't.  (That's
      really an independent bug, but it manifests in much the same cases.)
      
      Add a regression test checking one manifestation of this problem, which
      was that row-level triggers on a foreign child table did not work right.
      
      Back-patch to 9.5 where the problem was introduced.
      
      Etsuro Fujita, reviewed by Ildus Kurbangaliev and Ashutosh Bapat
      
      Discussion: https://postgr.es/m/20170514150525.0346ba72@postgrespro.ru
      9a785ad5
    • Simon Riggs's avatar
      11746900
    • Simon Riggs's avatar
      Pad XLogReaderState's per-buffer data_bufsz more aggressively. · 59af8d43
      Simon Riggs authored
      Originally, we palloc'd this buffer just barely big enough to hold the
      largest xlog backup block seen so far. We now MAXALIGN the palloc size.
      
      The original coding could result in many repeated palloc cycles, in the
      worst case where we see a series ofgradually larger xlog records.  We
      ameliorate that similarly to 8735978e
      by imposing a minimum buffer size of BLCKSZ.
      
      Discussion: https://postgr.es/m/E1eHa4J-0006hI-Q8@gemulon.postgresql.org
      59af8d43
    • Magnus Hagander's avatar
      Fix typo in comment · d5f965c2
      Magnus Hagander authored
      Andreas Karlsson
      d5f965c2
  4. 26 Nov, 2017 2 commits
    • Tom Lane's avatar
      Pad XLogReaderState's main_data buffer more aggressively. · 8735978e
      Tom Lane authored
      Originally, we palloc'd this buffer just barely big enough to hold the
      largest xlog record seen so far.  It turns out that that can result in
      valgrind complaints, because some compilers will emit code that assumes
      it can safely fetch padding bytes at the end of a struct, and those
      padding bytes were unallocated so far as aset.c was concerned.  We can
      fix that by MAXALIGN'ing the palloc request size, ensuring that it is big
      enough to include any possible padding that might've been omitted from
      the on-disk record.
      
      An additional objection to the original coding is that it could result in
      many repeated palloc cycles, in the worst case where we see a series of
      gradually larger xlog records.  We can ameliorate that cheaply by
      imposing a minimum buffer size that's large enough for most xlog records.
      BLCKSZ/2 was chosen after a bit of discussion.
      
      In passing, remove an obsolete comment in struct xl_heap_new_cid that the
      combocid field is free due to alignment considerations.  Perhaps that was
      true at some point, but it's not now.
      
      Back-patch to 9.5 where this code came in.
      
      Discussion: https://postgr.es/m/E1eHa4J-0006hI-Q8@gemulon.postgresql.org
      8735978e
    • Joe Conway's avatar
      Make has_sequence_privilege support WITH GRANT OPTION · 752714dd
      Joe Conway authored
      The various has_*_privilege() functions all support an optional
      WITH GRANT OPTION added to the supported privilege types to test
      whether the privilege is held with grant option. That is, all except
      has_sequence_privilege() variations. Fix that.
      
      Back-patch to all supported branches.
      
      Discussion: https://postgr.es/m/005147f6-8280-42e9-5a03-dd2c1e4397ef@joeconway.com
      752714dd
  5. 25 Nov, 2017 8 commits
    • Tom Lane's avatar
      Update MSVC build process for new timezone data. · 6d4ae6a8
      Tom Lane authored
      Missed this dependency in commits 7cce222c et al.
      6d4ae6a8
    • Tom Lane's avatar
      Replace raw timezone source data with IANA's new compact format. · 7cce222c
      Tom Lane authored
      Traditionally IANA has distributed their timezone data in pure source
      form, replete with extensive historical comments.  As of release 2017c,
      they've added a compact single-file format that omits comments and
      abbreviates command keywords.  This form is way shorter than the pure
      source, even before considering its allegedly better compressibility.
      Hence, let's distribute the data in that form rather than pure source.
      
      I'm pushing this now, rather than at the next timezone database update,
      so that it's easy to confirm that this data file produces compiled zic
      output that's identical to what we were getting before.
      
      Discussion: https://postgr.es/m/1915.1511210334@sss.pgh.pa.us
      7cce222c
    • 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
  6. 24 Nov, 2017 8 commits