Skip to content
Snippets Groups Projects
  1. Dec 13, 2017
  2. Dec 11, 2017
    • Peter Eisentraut's avatar
      Fix comment · c55253b7
      Peter Eisentraut authored
      
      Reported-by: default avatarNoah Misch <noah@leadboat.com>
      c55253b7
    • Tom Lane's avatar
      Fix corner-case coredump in _SPI_error_callback(). · e3d194f7
      Tom Lane authored
      I noticed that _SPI_execute_plan initially sets spierrcontext.arg = NULL,
      and only fills it in some time later.  If an error were to happen in
      between, _SPI_error_callback would try to dereference the null pointer.
      This is unlikely --- there's not much between those points except
      push-snapshot calls --- but it's clearly not impossible.  Tweak the
      callback to do nothing if the pointer isn't set yet.
      
      It's been like this for awhile, so back-patch to all supported branches.
      e3d194f7
  3. Dec 09, 2017
    • Magnus Hagander's avatar
      Fix typo · 22e71b3a
      Magnus Hagander authored
      Reported by Robins Tharakan
      22e71b3a
    • Noah Misch's avatar
      MSVC 2012+: Permit linking to 32-bit, MinGW-built libraries. · e2cc6505
      Noah Misch authored
      Notably, this permits linking to the 32-bit Perl binaries advertised on
      perl.org, namely Strawberry Perl and ActivePerl.  This has a side effect
      of permitting linking to binaries built with obsolete MSVC versions.
      
      By default, MSVC 2012 and later require a "safe exception handler table"
      in each binary.  MinGW-built, 32-bit DLLs lack the relevant exception
      handler metadata, so linking to them failed with error LNK2026.  Restore
      the semantics of MSVC 2010, which omits the table from a given binary if
      some linker input lacks metadata.  This has no effect on 64-bit builds
      or on MSVC 2010 and earlier.  Back-patch to 9.3 (all supported
      versions).
      
      Reported by Victor Wagner.
      
      Discussion: https://postgr.es/m/20160326154321.7754ab8f@wagner.wagner.home
      e2cc6505
    • Noah Misch's avatar
      MSVC: Test whether 32-bit Perl needs -D_USE_32BIT_TIME_T. · 9b5c9979
      Noah Misch authored
      Commits 5a5c2fec and
      b5178c5d introduced support for modern
      MSVC-built, 32-bit Perl, but they broke use of MinGW-built, 32-bit Perl
      distributions like Strawberry Perl and modern ActivePerl.  Perl has no
      robust means to report whether it expects a -D_USE_32BIT_TIME_T ABI, so
      test this.  Back-patch to 9.3 (all supported versions).
      
      The chief alternative was a heuristic of adding -D_USE_32BIT_TIME_T when
      $Config{gccversion} is nonempty.  That banks on every gcc-built Perl
      using the same ABI.  gcc could change its default ABI the way MSVC once
      did, and one could build Perl with gcc and the non-default ABI.
      
      The GNU make build system could benefit from a similar test, without
      which it does not support MSVC-built Perl.  For now, just add a comment.
      Most users taking the special step of building Perl with MSVC probably
      build PostgreSQL with MSVC.
      
      Discussion: https://postgr.es/m/20171130041441.GA3161526@rfd.leadboat.com
      9b5c9979
  4. Dec 08, 2017
  5. Dec 06, 2017
    • Robert Haas's avatar
      Report failure to start a background worker. · a8ef4e81
      Robert Haas authored
      When a worker is flagged as BGW_NEVER_RESTART and we fail to start it,
      or if it is not marked BGW_NEVER_RESTART but is terminated before
      startup succeeds, what BgwHandleStatus should be reported?  The
      previous code really hadn't considered this possibility (as indicated
      by the comments which ignore it completely) and would typically return
      BGWH_NOT_YET_STARTED, but that's not a good answer, because then
      there's no way for code using GetBackgroundWorkerPid() to tell the
      difference between a worker that has not started but will start
      later and a worker that has not started and will never be started.
      So, when this case happens, return BGWH_STOPPED instead.  Update the
      comments to reflect this.
      
      The preceding fix by itself is insufficient to fix the problem,
      because the old code also didn't send a notification to the process
      identified in bgw_notify_pid when startup failed.  That might've
      been technically correct under the theory that the status of the
      worker was BGWH_NOT_YET_STARTED, because the status would indeed not
      change when the worker failed to start, but now that we're more
      usefully reporting BGWH_STOPPED, a notification is needed.
      
      Without these fixes, code which starts background workers and then
      uses the recommended APIs to wait for those background workers to
      start would hang indefinitely if the postmaster failed to fork a
      worker.
      
      Amit Kapila and Robert Haas
      
      Discussion: http://postgr.es/m/CAA4eK1KDfKkvrjxsKJi3WPyceVi3dH1VCkbTJji2fuwKuB=3uw@mail.gmail.com
      a8ef4e81
  6. Dec 05, 2017
  7. Dec 04, 2017
    • Tom Lane's avatar
      Clean up assorted messiness around AllocateDir() usage. · 2a11b188
      Tom Lane authored
      This patch fixes a couple of low-probability bugs that could lead to
      reporting an irrelevant errno value (and hence possibly a wrong SQLSTATE)
      concerning directory-open or file-open failures.  It also fixes places
      where we took shortcuts in reporting such errors, either by using elog
      instead of ereport or by using ereport but forgetting to specify an
      errcode.  And it eliminates a lot of just plain redundant error-handling
      code.
      
      In service of all this, export fd.c's formerly-static function
      ReadDirExtended, so that external callers can make use of the coding
      pattern
      
      	dir = AllocateDir(path);
      	while ((de = ReadDirExtended(dir, path, LOG)) != NULL)
      
      if they'd like to treat directory-open failures as mere LOG conditions
      rather than errors.  Also fix FreeDir to be a no-op if we reach it
      with dir == NULL, as such a coding pattern would cause.
      
      Then, remove code at many call sites that was throwing an error or log
      message for AllocateDir failure, as ReadDir or ReadDirExtended can handle
      that job just fine.  Aside from being a net code savings, this gets rid of
      a lot of not-quite-up-to-snuff reports, as mentioned above.  (In some
      places these changes result in replacing a custom error message such as
      "could not open tablespace directory" with more generic wording "could not
      open directory", but it was agreed that the custom wording buys little as
      long as we report the directory name.)  In some other call sites where we
      can't just remove code, change the error reports to be fully
      project-style-compliant.
      
      Also reorder code in restoreTwoPhaseData that was acquiring a lock
      between AllocateDir and ReadDir; in the unlikely but surely not
      impossible case that LWLockAcquire changes errno, AllocateDir failures
      would be misreported.  There is no great value in opening the directory
      before acquiring TwoPhaseStateLock, so just do it in the other order.
      
      Also fix CheckXLogRemoved to guarantee that it preserves errno,
      as quite a number of call sites are implicitly assuming.  (Again,
      it's unlikely but I think not impossible that errno could change
      during a SpinLockAcquire.  If so, this function was broken for its
      own purposes as well as breaking callers.)
      
      And change a few places that were using not-per-project-style messages,
      such as "could not read directory" when "could not open directory" is
      more correct.
      
      Back-patch the exporting of ReadDirExtended, in case we have occasion
      to back-patch some fix that makes use of it; it's not needed right now
      but surely making it global is pretty harmless.  Also back-patch the
      restoreTwoPhaseData and CheckXLogRemoved fixes.  The rest of this is
      essentially cosmetic and need not get back-patched.
      
      Michael Paquier, with a bit of additional work by me
      
      Discussion: https://postgr.es/m/CAB7nPqRpOCxjiirHmebEFhXVTK7V5Jvw4bz82p7Oimtsm3TyZA@mail.gmail.com
      2a11b188
    • Tom Lane's avatar
      Support boolean columns in functional-dependency statistics. · bf2b317f
      Tom Lane authored
      There's no good reason that the multicolumn stats stuff shouldn't work on
      booleans.  But it looked only for "Var = pseudoconstant" clauses, and it
      will seldom find those for boolean Vars, since earlier phases of planning
      will fold "boolvar = true" or "boolvar = false" to just "boolvar" or
      "NOT boolvar" respectively.  Improve dependencies_clauselist_selectivity()
      to recognize such clauses as equivalent to equality restrictions.
      
      This fixes a failure of the extended stats mechanism to apply in a case
      reported by Vitaliy Garnashevich.  It's not a complete solution to his
      problem because the bitmap-scan costing code isn't consulting extended
      stats where it should, but that's surely an independent issue.
      
      In passing, improve some comments, get rid of a NumRelids() test that's
      redundant with the preceding bms_membership() test, and fix
      dependencies_clauselist_selectivity() so that estimatedclauses actually
      is a pure output argument as stated by its API contract.
      
      Back-patch to v10 where this code was introduced.
      
      Discussion: https://postgr.es/m/73a4936d-2814-dc08-ed0c-978f76f435b0@gmail.com
      bf2b317f
  8. Nov 30, 2017
    • Noah Misch's avatar
      Fix non-GNU makefiles for AIX make. · f8252b64
      Noah Misch authored
      Invoking the Makefile without an explicit target was building every
      possible target instead of just the "all" target.  Back-patch to 9.3
      (all supported versions).
      f8252b64
  9. Nov 29, 2017
  10. Nov 28, 2017
  11. Nov 27, 2017
    • Tom Lane's avatar
      Fix creation of resjunk tlist entries for inherited mixed UPDATE/DELETE. · a57aa430
      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
      a57aa430
    • Magnus Hagander's avatar
      Fix typo in comment · 4f2d0af1
      Magnus Hagander authored
      Andreas Karlsson
      4f2d0af1
  12. Nov 26, 2017
    • Tom Lane's avatar
      Pad XLogReaderState's main_data buffer more aggressively. · 94fd57df
      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
      94fd57df
    • Joe Conway's avatar
      Make has_sequence_privilege support WITH GRANT OPTION · 9e051b67
      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
      9e051b67
    • Tom Lane's avatar
      Update MSVC build process for new timezone data. · 7b0cb5ec
      Tom Lane authored
      Missed this dependency in commits 7cce222c et al.
      7b0cb5ec
  13. Nov 25, 2017
    • Tom Lane's avatar
      Replace raw timezone source data with IANA's new compact format. · 097b24ce
      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
      097b24ce
    • Tom Lane's avatar
      Avoid formally-undefined use of memcpy() in hstoreUniquePairs(). · ddba3200
      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
      ddba3200
    • Tom Lane's avatar
      Repair failure with SubPlans in multi-row VALUES lists. · 5dc7faa9
      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
      5dc7faa9
    • Tom Lane's avatar
      Improve planner's handling of set-returning functions in grouping columns. · b9fc2d0b
      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
      b9fc2d0b
  14. Nov 24, 2017
  15. Nov 23, 2017
Loading