1. 25 Jul, 2019 6 commits
    • Tom Lane's avatar
      Fix syntax error in commit 20e99cdd. · cb9bb157
      Tom Lane authored
      Per buildfarm.
      cb9bb157
    • Tom Lane's avatar
      Fix failures to ignore \r when reading Windows-style newlines. · b654714f
      Tom Lane authored
      libpq failed to ignore Windows-style newlines in connection service files.
      This normally wasn't a problem on Windows itself, because fgets() would
      convert \r\n to just \n.  But if libpq were running inside a program that
      changes the default fopen mode to binary, it would see the \r's and think
      they were data.  In any case, it's project policy to ignore \r in text
      files unconditionally, because people sometimes try to use files with
      DOS-style newlines on Unix machines, where the C library won't hide that
      from us.
      
      Hence, adjust parseServiceFile() to ignore \r as well as \n at the end of
      the line.  In HEAD, go a little further and make it ignore all trailing
      whitespace, to match what it's always done with leading whitespace.
      
      In HEAD, also run around and fix up everyplace where we have
      newline-chomping code to make all those places look consistent and
      uniformly drop \r.  It is not clear whether any of those changes are
      fixing live bugs.  Most of the non-cosmetic changes are in places that
      are reading popen output, and the jury is still out as to whether popen
      on Windows can return \r\n.  (The Windows-specific code in pipe_read_line
      seems to think so, but our lack of support for this elsewhere suggests
      maybe it's not a problem in practice.)  Hence, I desisted from applying
      those changes to back branches, except in run_ssl_passphrase_command()
      which is new enough and little-tested enough that we'd probably not have
      heard about any problems there.
      
      Tom Lane and Michael Paquier, per bug #15827 from Jorge Gustavo Rocha.
      Back-patch the parseServiceFile() change to all supported branches,
      and the run_ssl_passphrase_command() change to v11 where that was added.
      
      Discussion: https://postgr.es/m/15827-e6ba53a3a7ed543c@postgresql.org
      b654714f
    • Andrew Dunstan's avatar
      Honor MSVC WindowsSDKVersion if set · 20e99cdd
      Andrew Dunstan authored
      Add a line to the project file setting the target SDK. Otherwise, in for
      example VS2017, if the default but optional 8.1 SDK is not installed the
      build will fail.
      
      Patch from Peifeng Qiu, slightly edited by me.
      
      Discussion: https://postgr.es/m/CABmtVJhw1boP_bd4=b3Qv5YnqEdL696NtHFi2ruiyQ6mFHkeQQ@mail.gmail.com
      
      Backpatch to all live branches.
      20e99cdd
    • Tom Lane's avatar
      Fix contrib/sepgsql test policy to work with latest SELinux releases. · f5a4ab23
      Tom Lane authored
      As of Fedora 30, it seems that the system-provided macros for setting
      up user privileges in SELinux policies don't grant the ability to read
      /etc/passwd, as they formerly did.  This restriction breaks psql
      (which tries to use getpwuid() to obtain the user name it's running
      under) and thereby the contrib/sepgsql regression test.  Add explicit
      specifications that we need the right to read /etc/passwd.
      
      Mike Palmiotto, per a report from me.  Back-patch to all supported
      branches.
      
      Discussion: https://postgr.es/m/23856.1563381159@sss.pgh.pa.us
      f5a4ab23
    • Peter Eisentraut's avatar
      doc: Fix typo · 35a34e62
      Peter Eisentraut authored
      35a34e62
    • Andres Freund's avatar
      Fix system column accesses in ON CONFLICT ... RETURNING. · ecbdd009
      Andres Freund authored
      After 277cb789 ON CONFLICT ... SET ... RETURNING failed with
      ERROR:  virtual tuple table slot does not have system attributes
      when taking the update path, as the slot used to insert into the
      table (and then process RETURNING) was defined to be a virtual slot in
      that commit. Virtual slots don't support system columns except for
      tableoid and ctid, as the other system columns are AM dependent.
      
      Fix that by using a slot of the table's type. Add tests for system
      column accesses in ON CONFLICT ...  RETURNING.
      
      Reported-By: Roby, bisected to the relevant commit by Jeff Janes
      Author: Andres Freund
      Discussion: https://postgr.es/m/73436355-6432-49B1-92ED-1FE4F7E7E100@finefun.com.au
      Backpatch: 12-, where the bug was introduced in 277cb789
      ecbdd009
  2. 24 Jul, 2019 7 commits
  3. 23 Jul, 2019 5 commits
  4. 22 Jul, 2019 7 commits
  5. 21 Jul, 2019 4 commits
    • David Rowley's avatar
      Adjust overly strict Assert · e1a0f6a9
      David Rowley authored
      3373c715 changed how we determine EquivalenceClasses for relations and
      added an Assert to ensure all relations mentioned in each EC's ec_relids
      was a RELOPT_BASEREL.  However, the join removal code may remove a LEFT
      JOIN and since it does not clean up EC members belonging to the removed
      relations it can leave RELOPT_DEADREL rels in ec_relids.
      
      Fix this by adjusting the Assert to allow RELOPT_DEADREL rels too.
      
      Reported-by: sqlsmith via Andreas Seltenreich
      Discussion: https://postgr.es/m/87y30r8sls.fsf@ansel.ydns.eu
      e1a0f6a9
    • Tom Lane's avatar
      Remove no-longer-helpful reliance on fixed-size local array. · 330cafdf
      Tom Lane authored
      Coverity complained about this code, apparently because it uses a local
      array of size FUNC_MAX_ARGS without a guard that the input argument list
      is no longer than that.  (Not sure why it complained today, since this
      code's been the same for a long time; possibly it re-analyzed everything
      the List API change touched?)
      
      Rather than add a guard, though, let's just get rid of the local array
      altogether.  It was only there to avoid list_nth() calls, and those are
      no longer expensive.
      330cafdf
    • Michael Paquier's avatar
      Fix compilation warning of pg_basebackup with MinGW · 90317ab7
      Michael Paquier authored
      Several buildfarm members have been complaining about that with gcc,
      like jacana.  Weirdly enough, Visual Studio's compilers do not find this
      issue.
      
      Author: Michael Paquier
      Reviewed-by: Andrew Dunstan
      Discussion: https://postgr.es/m/20190719050830.GK1859@paquier.xyz
      90317ab7
    • David Rowley's avatar
      Speed up finding EquivalenceClasses for a given set of rels · 3373c715
      David Rowley authored
      Previously in order to determine which ECs a relation had members in, we
      had to loop over all ECs stored in PlannerInfo's eq_classes and check if
      ec_relids mentioned the relation.  For the most part, this was fine, as
      generally, unless queries were fairly complex, the overhead of performing
      the lookup would have not been that significant.  However, when queries
      contained large numbers of joins and ECs, the overhead to find the set of
      classes matching a given set of relations could become a significant
      portion of the overall planning effort.
      
      Here we allow a much more efficient method to access the ECs which match a
      given relation or set of relations.  A new Bitmapset field in RelOptInfo
      now exists to store the indexes into PlannerInfo's eq_classes list which
      each relation is mentioned in.  This allows very fast lookups to find all
      ECs belonging to a single relation.  When we need to lookup ECs belonging
      to a given pair of relations, we can simply bitwise-AND the Bitmapsets from
      each relation and use the result to perform the lookup.
      
      We also take the opportunity to write a new implementation of
      generate_join_implied_equalities which makes use of the new indexes.
      generate_join_implied_equalities_for_ecs must remain as is as it can be
      given a custom list of ECs, which we can't easily determine the indexes of.
      
      This was originally intended to fix the performance penalty of looking up
      foreign keys matching a join condition which was introduced by 100340e2.
      However, we're speeding up much more than just that here.
      
      Author: David Rowley, Tom Lane
      Reviewed-by: Tom Lane, Tomas Vondra
      Discussion: https://postgr.es/m/6970.1545327857@sss.pgh.pa.us
      3373c715
  6. 20 Jul, 2019 3 commits
    • Peter Geoghegan's avatar
      Don't rely on estimates for amcheck Bloom filters. · 894af78f
      Peter Geoghegan authored
      Solely relying on a relation's reltuples/relpages estimate to size the
      Bloom filters used by amcheck verification makes verification less
      effective when the estimates are very stale.  In extreme cases,
      verification options that use Bloom filters internally could be totally
      ineffective, without users receiving any clear indication that certain
      types of corruption might easily be missed.
      
      To fix, use RelationGetNumberOfBlocks() instead of relpages to size the
      downlink block Bloom filter.  Use the same RelationGetNumberOfBlocks()
      value to derive a minimum size for the heapallindexed Bloom filter,
      rather than completely trusting reltuples.  Verification will still be
      reasonably effective when the projected/estimated number of Bloom filter
      elements is at least 1/5 of the final number of elements, which is
      assured by the new sizing logic.
      
      Reported-By: Alexander Korotkov
      Discussion: https://postgr.es/m/CAH2-Wzk0ke2J42KrNYBKu0Xovjy-sU5ub7PWjgpbsKdAQcL4OA@mail.gmail.com
      Backpatch: 11-, where downlink/heapallindexed verification were added.
      894af78f
    • Tomas Vondra's avatar
      Use column collation for extended statistics · a63378a0
      Tomas Vondra authored
      The current extended statistics code was a bit confused which collation
      to use.  When building the statistics, the collations defined as default
      for the data types were used (since commit 5e092800).  The MCV code was
      however using the column collations for MCV serialization, and then
      DEFAULT_COLLATION_OID when computing estimates. So overall the code was
      using all three possible options, inconsistently.
      
      This uses the column colation everywhere - this makes it consistent with
      what 5e092800 did for regular stats.  We however do not track the
      collations in a catalog, because we can derive them from column-level
      information.  This may need to change in the future, e.g. after allowing
      statistics on expressions.
      
      Reviewed-by: Tom Lane
      Discussion: https://postgr.es/m/8736jdhbhc.fsf%40ansel.ydns.eu
      Backpatch-to: 12
      a63378a0
    • Tomas Vondra's avatar
      Rework examine_opclause_expression to use varonleft · e38a55ba
      Tomas Vondra authored
      The examine_opclause_expression function needs to return information on
      which side of the operator we found the Var, but the variable was called
      "isgt" which is rather misleading (it assumes the operator is either
      less-than or greater-than, but it may be equality or something else).
      Other places in the planner use a variable called "varonleft" for this
      purpose, so just adopt the same convention here.
      
      The code also assumed we don't care about this flag for equality, as
      (Var = Const) and (Const = Var) should be the same thing. But that does
      not work for cross-type operators, in which case we need to pass the
      parameters to the procedure in the right order. So just use the same
      code for all types of expressions.
      
      This means we don't need to care about the selectivity estimation
      function anymore, at least not in this code. We should only get the
      supported cases here (thanks to statext_is_compatible_clause).
      
      Reviewed-by: Tom Lane
      Discussion: https://postgr.es/m/8736jdhbhc.fsf%40ansel.ydns.eu
      Backpatch-to: 12
      e38a55ba
  7. 19 Jul, 2019 5 commits
  8. 18 Jul, 2019 3 commits
    • Michael Paquier's avatar
      Fix typo in mvdistinct.c · 70a33b21
      Michael Paquier authored
      Noticed while browsing the code.
      70a33b21
    • Jeff Davis's avatar
      Fix daterange canonicalization for +/- infinity. · e6feef57
      Jeff Davis authored
      The values 'infinity' and '-infinity' are a part of the DATE type
      itself, so a bound of the date 'infinity' is not the same as an
      unbounded/infinite range. However, it is still wrong to try to
      canonicalize such values, because adding or subtracting one has no
      effect. Fix by treating 'infinity' and '-infinity' the same as
      unbounded ranges for the purposes of canonicalization (but not other
      purposes).
      
      Backpatch to all versions because it is inconsistent with the
      documented behavior. Note that this could be an incompatibility for
      applications relying on the behavior contrary to the documentation.
      
      Author: Laurenz Albe
      Reviewed-by: Thomas Munro
      Discussion: https://postgr.es/m/77f24ea19ab802bc9bc60ddbb8977ee2d646aec1.camel%40cybertec.at
      Backpatch-through: 9.4
      e6feef57
    • Peter Geoghegan's avatar
      Fix nbtree metapage cache upgrade bug. · d004147e
      Peter Geoghegan authored
      Commit 857f9c36, which taught nbtree VACUUM to avoid unnecessary
      index scans, bumped the nbtree version number from 2 to 3, while adding
      the ability for nbtree indexes to be upgraded on-the-fly.  Various
      assertions that assumed that an nbtree index was always on version 2 had
      to be changed to accept any supported version (version 2 or 3 on
      Postgres 11).
      
      However, a few assertions were missed in the initial commit, all of
      which were in code paths that cache a local copy of the metapage
      metadata, where the index had been expected to be on the current version
      (no longer version 2) as a generic sanity check.  Rather than simply
      update the assertions, follow-up commit 0a64b451 intentionally made
      the metapage caching code update the per-backend cached metadata version
      without changing the on-disk version at the same time.  This could even
      happen when the planner needed to determine the height of a B-Tree for
      costing purposes.  The assertions only fail on Postgres v12 when
      upgrading from v10, because they were adjusted to use the authoritative
      shared memory metapage by v12's commit dd299df8.
      
      To fix, remove the cache-only upgrade mechanism entirely, and update the
      assertions themselves to accept any supported version (go back to using
      the cached version in v12).  The fix is almost a full revert of commit
      0a64b451 on the v11 branch.
      
      VACUUM only considers the authoritative metapage, and never bothers with
      a locally cached version, whereas everywhere else isn't interested in
      the metapage fields that were added by commit 857f9c36.  It seems
      unlikely that this bug has affected any user on v11.
      
      Reported-By: Christoph Berg
      Bug: #15896
      Discussion: https://postgr.es/m/15896-5b25e260fdb0b081%40postgresql.org
      Backpatch: 11-, where VACUUM was taught to avoid unnecessary index scans.
      d004147e