1. 16 Mar, 2020 3 commits
    • Thomas Munro's avatar
      Introduce a maintenance_io_concurrency setting. · fc34b0d9
      Thomas Munro authored
      Introduce a GUC and a tablespace option to control I/O prefetching, much
      like effective_io_concurrency, but for work that is done on behalf of
      many client sessions.
      
      Use the new setting in heapam.c instead of the hard-coded formula
      effective_io_concurrency + 10 introduced by commit 558a9165.  Go with
      a default value of 10 for now, because it's a round number pretty close
      to the value used for that existing case.
      
      Discussion: https://postgr.es/m/CA%2BhUKGJUw08dPs_3EUcdO6M90GnjofPYrWp4YSLaBkgYwS-AqA%40mail.gmail.com
      fc34b0d9
    • Thomas Munro's avatar
      Simplify the effective_io_concurrency setting. · b09ff536
      Thomas Munro authored
      The effective_io_concurrency GUC and equivalent tablespace option were
      previously passed through a formula based on a theory about RAID
      spindles and probabilities, to arrive at the number of pages to prefetch
      in bitmap heap scans.  Tomas Vondra, Andres Freund and others argued
      that it was anachronistic and hard to justify, and commit 558a9165
      already started down the path of bypassing it in new code.  We agreed to
      drop that logic and use the value directly.
      
      For the default setting of 1, there is no change in effect.  Higher
      settings can be converted from the old meaning to the new with:
      
        select round(sum(OLD / n::float)) from generate_series(1, OLD) s(n);
      
      We might want to consider renaming the GUC before the next release given
      the change in meaning, but it's not clear that many users had set it
      very carefully anyway.  That decision is deferred for now.
      
      Discussion: https://postgr.es/m/CA%2BhUKGJUw08dPs_3EUcdO6M90GnjofPYrWp4YSLaBkgYwS-AqA%40mail.gmail.com
      b09ff536
    • Peter Geoghegan's avatar
      nbtree: Reorder nbtinsert.c prototypes. · f207bb0b
      Peter Geoghegan authored
      Relocate _bt_newroot() prototype, so that the order that prototypes
      appear in matches the order that the functions are defined in.
      f207bb0b
  2. 15 Mar, 2020 1 commit
  3. 14 Mar, 2020 6 commits
    • Tomas Vondra's avatar
      Improve test coverage for multi-column MCV lists · d8cfa82d
      Tomas Vondra authored
      The regression tests for extended statistics were not testing a couple
      of important cases for the MCV lists:
      
        * IS NOT NULL clauses - We did have queries with IS NULL clauses, but
          not the negative case.
      
        * clauses with variable on the right - All the clauses had the Var on
          the left, i.e. (Var op Const), so this adds (Const op Var) too.
      
        * columns with fixed-length types passed by reference - All columns
          were using either by-value or varlena types, so add a test with
          UUID columns too. This matters for (de)serialization.
      
        * NULL-only dimension - When one of the columns contains only NULL
          values, we treat it a a special case during (de)serialization.
      
        * arrays containing NULL - When the constant parameter contains NULL
          value, we need to handle it correctly during estimation, for all
          IN, ANY and ALL clauses.
      
      Discussion: https://www.postgresql.org/message-id/flat/20200113230008.g67iyk4cs3xbnjju@development
      Author: Tomas Vondra
      d8cfa82d
    • Tomas Vondra's avatar
      Improve test coverage for functional dependencies · f9696782
      Tomas Vondra authored
      The regression tests for functional dependencies were only using clauses
      of the form (Var op Const), i.e. with Var on the left side. This adds
      a couple of queries with Var on the right, to test other code paths.
      
      It also prints one of the functional dependencies, to test the data type
      output function. The functional dependencies are "perfect" with degree
      of 1.0 so this should be stable.
      
      Discussion: https://www.postgresql.org/message-id/flat/20200113230008.g67iyk4cs3xbnjju@development
      Author: Tomas Vondra
      f9696782
    • Tom Lane's avatar
      Rearrange pseudotypes.c to get rid of duplicative code. · 87c9c257
      Tom Lane authored
      Commit a5954de1 replaced a lot of manually-coded stub I/O routines
      with code generated by macros.  That was a good idea but it didn't
      go far enough, because there were still manually-coded stub input
      routines for types that had live output routines.  Refactor the
      macro so that we can generate just a stub input routine at need.
      
      Also create similar macros to generate stub binary I/O routines,
      since we have some of those now.  The only stub functions that remain
      hand-coded are shell_in() and shell_out(), which need to be separate
      because they use different error messages.
      
      While here, rearrange the commentary to discuss each type not each
      function.  This provides a better way to explain the *why* of which
      types need which support, rather than just duplicatively annotating
      the functions.
      
      Discussion: https://postgr.es/m/24137.1584139352@sss.pgh.pa.us
      87c9c257
    • Tom Lane's avatar
      Restructure polymorphic-type resolution in funcapi.c. · 4dbcb3f8
      Tom Lane authored
      resolve_polymorphic_tupdesc() and resolve_polymorphic_argtypes() failed to
      cover the case of having to resolve anyarray given only an anyrange input.
      The bug was masked if anyelement was also used (as either input or
      output), which probably helps account for our not having noticed.
      
      While looking at this I noticed that resolve_generic_type() would produce
      the wrong answer if asked to make that same resolution.  ISTM that
      resolve_generic_type() is confusingly defined and overly complex, so
      rather than fix it, let's just make funcapi.c do the specific lookups
      it requires for itself.
      
      With this change, resolve_generic_type() is not used anywhere, so remove
      it in HEAD.  In the back branches, leave it alone (complete with bug)
      just in case any external code is using it.
      
      While we're here, make some other refactoring adjustments in funcapi.c
      with an eye to upcoming future expansion of the set of polymorphic types:
      
      * Simplify quick-exit tests by adding an overall have_polymorphic_result
      flag.  This is about a wash now but will be a win when there are more
      flags.
      
      * Reduce duplication of code between resolve_polymorphic_tupdesc() and
      resolve_polymorphic_argtypes().
      
      * Don't bother to validate correct matching of anynonarray or anyenum;
      the parser should have done that, and even if it didn't, just doing
      "return false" here would lead to a very confusing, off-point error
      message.  (Really, "return false" in these two functions should only
      occur if the call_expr isn't supplied or we can't obtain data type
      info from it.)
      
      * For the same reason, throw an elog rather than "return false" if
      we fail to resolve a polymorphic type.
      
      The bug's been there since we added anyrange, so back-patch to
      all supported branches.
      
      Discussion: https://postgr.es/m/6093.1584202130@sss.pgh.pa.us
      4dbcb3f8
    • Tomas Vondra's avatar
      Use multi-variate MCV lists to estimate ScalarArrayOpExpr · e83daa7e
      Tomas Vondra authored
      Commit 8f321bd1 added support for estimating ScalarArrayOpExpr clauses
      (IN/ANY) clauses using functional dependencies. There's no good reason
      not to support estimation of these clauses using multi-variate MCV lists
      too, so this commits implements that. That makes the behavior consistent
      and MCV lists can estimate all variants (ANY/ALL, inequalities, ...).
      
      Author: Tomas Vondra
      Review: Dean Rasheed
      Discussion: https://www.postgresql.org/message-id/flat/13902317.Eha0YfKkKy%40pierred-pdoc
      e83daa7e
    • Tomas Vondra's avatar
      Use functional dependencies to estimate ScalarArrayOpExpr · 8f321bd1
      Tomas Vondra authored
      Until now functional dependencies supported only simple equality clauses
      and clauses that can be trivially translated to equalities. This commit
      allows estimation of some ScalarArrayOpExpr (IN/ANY) clauses.
      
      For IN clauses we can do this thanks to using operator with equality
      semantics, which means an IN clause
      
          WHERE c IN (1, 2, ..., N)
      
      can be translated to
      
          WHERE (c = 1 OR c = 2 OR ... OR c = N)
      
      IN clauses are now considered compatible with functional dependencies,
      and rely on the same assumption of consistency of queries with data
      (which is an assumption we already used for simple equality clauses).
      This applies also to ALL clauses with an equality operator, which can be
      considered equivalent to IN clause.
      
      ALL clauses are still considered incompatible, although there's some
      discussion about maybe relaxing this in the future.
      
      Author: Pierre Ducroquet
      Reviewed-by: Tomas Vondra, Dean Rasheed
      Discussion: https://www.postgresql.org/message-id/flat/13902317.Eha0YfKkKy%40pierred-pdoc
      8f321bd1
  4. 13 Mar, 2020 6 commits
  5. 11 Mar, 2020 10 commits
    • Tom Lane's avatar
      Fix test case instability introduced in 085b6b66. · a029a064
      Tom Lane authored
      I forgot that the WAL directory might hold other files besides WAL
      segments, notably including new segments still being filled.
      That means a blind test for the first file's size being 16MB can
      fail.  Restrict based on file name length to make it more robust.
      
      Per buildfarm.
      a029a064
    • Alvaro Herrera's avatar
      Add pg_dump support for ALTER obj DEPENDS ON EXTENSION · b08dee24
      Alvaro Herrera authored
      pg_dump is oblivious to this kind of dependency, so they're lost on
      dump/restores (and pg_upgrade).  Have pg_dump emit ALTER lines so that
      they're preserved.  Add some pg_dump tests for the whole thing, also.
      
      Reviewed-by: Tom Lane (offlist)
      Reviewed-by: Ibrar Ahmed
      Reviewed-by: Ahsan Hadi (who also reviewed commit 899a04f5)
      Discussion: https://postgr.es/m/20200217225333.GA30974@alvherre.pgsql
      b08dee24
    • Tom Lane's avatar
      Avoid holding a directory FD open across pg_ls_dir_files() calls. · 085b6b66
      Tom Lane authored
      This coding technique is undesirable because (a) it leaks the FD for
      the rest of the transaction if the SRF is not run to completion, and
      (b) allocated FDs are a scarce resource, but multiple interleaved
      uses of the relevant functions could eat many such FDs.
      
      In v11 and later, a query such as "SELECT pg_ls_waldir() LIMIT 1"
      yields a warning about the leaked FD, and the only reason there's
      no warning in earlier branches is that fd.c didn't whine about such
      leaks before commit 9cb7db3f.  Even disregarding the warning, it
      wouldn't be too hard to run a backend out of FDs with careless use
      of these SQL functions.
      
      Hence, rewrite the function so that it reads the directory within
      a single call, returning the results as a tuplestore rather than
      via value-per-call mode.
      
      There are half a dozen other built-in SRFs with similar problems,
      but let's fix this one to start with, just to see if the buildfarm
      finds anything wrong with the code.
      
      In passing, fix bogus error report for stat() failure: it was
      whining about the directory when it should be fingering the
      individual file.  Doubtless a copy-and-paste error.
      
      Back-patch to v10 where this function was added.
      
      Justin Pryzby, with cosmetic tweaks and test cases by me
      
      Discussion: https://postgr.es/m/20200308173103.GC1357@telsasoft.com
      085b6b66
    • Peter Eisentraut's avatar
      Refactor ps_status.c API · bf68b79e
      Peter Eisentraut authored
      The init_ps_display() arguments were mostly lies by now, so to match
      typical usage, just use one argument and let the caller assemble it
      from multiple sources if necessary.  The only user of the additional
      arguments is BackendInitialize(), which was already doing string
      assembly on the caller side anyway.
      
      Remove the second argument of set_ps_display() ("force") and just
      handle that in init_ps_display() internally.
      
      BackendInitialize() also used to set the initial status as
      "authentication", but that was very far from where authentication
      actually happened.  So now it's set to "initializing" and then
      "authentication" just before the actual call to
      ClientAuthentication().
      Reviewed-by: default avatarJulien Rouhaud <rjuju123@gmail.com>
      Reviewed-by: default avatarKuntal Ghosh <kuntalghosh.2007@gmail.com>
      Reviewed-by: default avatarAlvaro Herrera <alvherre@2ndquadrant.com>
      Discussion: https://www.postgresql.org/message-id/flat/c65e5196-4f04-4ead-9353-6088c19615a3@2ndquadrant.com
      bf68b79e
    • Alvaro Herrera's avatar
      Avoid duplicates in ALTER ... DEPENDS ON EXTENSION · 899a04f5
      Alvaro Herrera authored
      If the command is attempted for an extension that the object already
      depends on, silently do nothing.
      
      In particular, this means that if a database containing multiple such
      entries is dumped, the restore will silently do the right thing and
      record just the first one.  (At least, in a world where pg_dump does
      dump such entries -- which it doesn't currently, but it will.)
      
      Backpatch to 9.6, where this kind of dependency was introduced.
      
      Reviewed-by: Ibrar Ahmed, Tom Lane (offlist)
      Discussion: https://postgr.es/m/20200217225333.GA30974@alvherre.pgsql
      899a04f5
    • Peter Eisentraut's avatar
      Clean up order in miscinit.c a bit · 1c918381
      Peter Eisentraut authored
      The code around InitPostmasterChild() from commit 31c45316 somehow
      ended up in the middle of a block of code related to "User ID state".
      Move it into its own block instead.
      1c918381
    • Peter Eisentraut's avatar
      Remove HAVE_WORKING_LINK · aaa3aedd
      Peter Eisentraut authored
      Previously, hard links were not used on Windows and Cygwin, but they
      support them just fine in currently supported OS versions, so we can
      use them there as well.
      
      Since all supported platforms now support hard links, we can remove
      the alternative code paths.
      
      Rename durable_link_or_rename() to durable_rename_excl() to make the
      purpose more clear without referencing the implementation details.
      
      Discussion: https://www.postgresql.org/message-id/flat/72fff73f-dc9c-4ef4-83e8-d2e60c98df48%402ndquadrant.com
      aaa3aedd
    • Alexander Korotkov's avatar
      Improve checking of child pages in contrib/amcheck. · d114cc53
      Alexander Korotkov authored
      This commit eliminates lossiness in check for missing parent downlinks in
      B-tree.  Instead of collecting lossy bitmap, we check for missing downlinks
      while visiting child pages referenced by downlinks of target level.  We
      traverse from previous child page to the subsequent child page by right links.
      Intermediate pages are candidates to have lost parent downlinks.
      
      Also this commit introduces matching of child high key to the pivot key of
      it's parent.
      
      Discussion: https://postgr.es/m/CAPpHfduoF-c4RhOyOm%3D4-Y367%2B8txq9Q6iM_ty0OYc8si1Abww%40mail.gmail.com
      Author: Alexander Korotkov
      Reviewed-by: Peter Geoghegan
      d114cc53
    • Peter Geoghegan's avatar
      Remove stray parenthesis in nbtree.h. · a88a285c
      Peter Geoghegan authored
      Oversight in commit 0d861bbb.
      a88a285c
    • Peter Geoghegan's avatar
      nbtree: Move fastpath NULL descent stack assertion. · 39eabec9
      Peter Geoghegan authored
      Commit 074251db added an assertion that verified the fastpath/rightmost
      page insert optimization's assumption about free space: There should
      always be enough free space on the page to insert the new item without
      splitting the page.  Otherwise, we end up using the "concurrent root
      page split" phony/fake stack path in _bt_insert_parent().  This does not
      lead to incorrect behavior, but it is likely to be far slower than
      simply using the regular _bt_search() path.  The assertion catches
      serious performance bugs that would probably take a long time to detect
      any other way.
      
      It seems much more natural to make this assertion just before the point
      that we generate a fake/phony descent stack.  Move the assert there.
      This also makes _bt_insertonpg() a bit more readable.
      39eabec9
  6. 10 Mar, 2020 11 commits
  7. 09 Mar, 2020 3 commits
    • Tom Lane's avatar
      Fix pg_dump/pg_restore to restore event triggers later. · 8728b2c7
      Tom Lane authored
      Previously, event triggers were restored just after regular triggers
      (and FK constraints, which are basically triggers).  This is risky
      since an event trigger, once installed, could interfere with subsequent
      restore commands.  Worse, because event triggers don't have any
      particular dependencies on any post-data objects, a parallel restore
      would consider them eligible to be restored the moment the post-data
      phase starts, allowing them to also interfere with restoration of a
      whole bunch of objects that would have been restored before them in
      a serial restore.  There's no way to completely remove the risk of a
      misguided event trigger breaking the restore, since if nothing else
      it could break other event triggers.  But we can certainly push them
      to later in the process to minimize the hazard.
      
      To fix, tweak the RestorePass mechanism introduced by commit 3eb9a5e7
      so that event triggers are handled as part of the post-ACL processing
      pass (renaming the "REFRESH" pass to "POST_ACL" to reflect its more
      general use).  This will cause them to restore after everything except
      matview refreshes, which seems OK since matview refreshes really ought
      to run in the post-restore state of the database.  In a parallel
      restore, event triggers and matview refreshes might be intermixed,
      but that seems all right as well.
      
      Also update the code and comments in pg_dump_sort.c so that its idea
      of how things are sorted agrees with what actually happens due to
      the RestorePass mechanism.  This is mostly cosmetic: it'll affect the
      order of objects in a dump's TOC, but not the actual restore order.
      But not changing that would be quite confusing to somebody reading
      the code.
      
      Back-patch to all supported branches.
      
      Fabrízio de Royes Mello, tweaked a bit by me
      
      Discussion: https://postgr.es/m/CAFcNs+ow1hmFox8P--3GSdtwz-S3Binb6ZmoP6Vk+Xg=K6eZNA@mail.gmail.com
      8728b2c7
    • Jeff Davis's avatar
      Introduce LogicalTapeSetExtend(). · 24d85952
      Jeff Davis authored
      Increases the number of tapes in a logical tape set. This will be
      important for disk-based hash aggregation, because the maximum number
      of tapes is not known ahead of time.
      
      While discussing this change, it was observed to regress the
      performance of Sort for at least one test case. The performance
      regression was because some versions of GCC switch to an inlined
      version of memcpy() in LogicalTapeWrite() after this change. No
      performance regression for clang was observed.
      
      Because the regression is due to an arbitrary decision by the
      compiler, I decided it shouldn't hold up this change. If it needs to
      be fixed, we can find a workaround.
      
      Author: Adam Lee, Jeff Davis
      Discussion: https://postgr.es/m/e54bfec11c59689890f277722aaaabd05f78e22c.camel%40j-davis.com
      24d85952
    • Fujii Masao's avatar
      Fix bug that causes to report waiting in PS display twice, in hot standby. · 17d3fcdc
      Fujii Masao authored
      Previously "waiting" could appear twice via PS in case of lock conflict
      in hot standby mode. Specifically this issue happend when the delay
      in WAL application determined by max_standby_archive_delay and
      max_standby_streaming_delay had passed but it took more than 500 msec
      to cancel all the conflicting transactions. Especially we can observe this
      easily by setting those delay parameters to -1.
      
      The cause of this issue was that WaitOnLock() and
      ResolveRecoveryConflictWithVirtualXIDs() added "waiting" to
      the process title in that case. This commit prevents
      ResolveRecoveryConflictWithVirtualXIDs() from reporting waiting
      in case of lock conflict, to fix the bug.
      
      Back-patch to all back branches.
      
      Author: Masahiko Sawada
      Reviewed-by: Fujii Masao
      Discussion: https://postgr.es/m/CA+fd4k4mXWTwfQLS3RPwGr4xnfAEs1ysFfgYHvmmoUgv6Zxvmg@mail.gmail.com
      17d3fcdc