1. 18 Jul, 2020 8 commits
  2. 17 Jul, 2020 9 commits
    • Tom Lane's avatar
      Cope with data-offset-less archive files during out-of-order restores. · f009591d
      Tom Lane authored
      pg_dump produces custom-format archive files that lack data offsets
      when it is unable to seek its output.  Up to now that's been a hazard
      for pg_restore.  But if pg_restore is able to seek in the archive
      file, there is no reason to throw up our hands when asked to restore
      data blocks out of order.  Instead, whenever we are searching for a
      data block, record the locations of the blocks we passed over (that
      is, fill in the missing data-offset fields in our in-memory copy of
      the TOC data).  Then, when we hit a case that requires going
      backwards, we can just seek back.
      
      Also track the furthest point that we've searched to, and seek back
      to there when beginning a search for a new data block.  This avoids
      possible O(N^2) time consumption, by ensuring that each data block
      is examined at most twice.  (On Unix systems, that's at most twice
      per parallel-restore job; but since Windows uses threads here, the
      threads can share block location knowledge, reducing the amount of
      duplicated work.)
      
      We can also improve the code a bit by using fseeko() to skip over
      data blocks during the search.
      
      This is all of some use even in simple restores, but it's really
      significant for parallel pg_restore.  In that case, we require
      seekability of the input already, and we will very probably need
      to do out-of-order restores.
      
      Back-patch to v12, as this fixes a regression introduced by commit
      548e5097.  Before that, parallel restore avoided requesting
      out-of-order restores, so it would work on a data-offset-less
      archive.  Now it will again.
      
      Ideally this patch would include some test coverage, but there are
      other open bugs that need to be fixed before we can extend our
      coverage of parallel restore very much.  Plan to revisit that later.
      
      David Gilman and Tom Lane; reviewed by Justin Pryzby
      
      Discussion: https://postgr.es/m/CALBH9DDuJ+scZc4MEvw5uO-=vRyR2=QF9+Yh=3hPEnKHWfS81A@mail.gmail.com
      f009591d
    • Tom Lane's avatar
      Remove manual tracking of file position in pg_dump/pg_backup_custom.c. · a8d0732a
      Tom Lane authored
      We do not really need to track the file position by hand.  We were
      already relying on ftello() whenever the archive file is seekable,
      while if it's not seekable we don't need the file position info
      anyway because we're not going to be able to re-write the TOC.
      
      Moreover, that tracking was buggy since it failed to account for
      the effects of fseeko().  Somewhat remarkably, that seems not to
      have made for any live bugs up to now.  We could fix the oversights,
      but it seems better to just get rid of the whole error-prone mess.
      
      In itself this is merely code cleanup.  However, it's necessary
      infrastructure for an upcoming bug-fix patch (because that code
      *does* need valid file position after fseeko).  The bug fix
      needs to go back as far as v12; hence, back-patch that far.
      
      Discussion: https://postgr.es/m/CALBH9DDuJ+scZc4MEvw5uO-=vRyR2=QF9+Yh=3hPEnKHWfS81A@mail.gmail.com
      a8d0732a
    • Peter Geoghegan's avatar
      Avoid CREATE INDEX unique index deduplication. · 5da8bf8b
      Peter Geoghegan authored
      There is no advantage to attempting deduplication for a unique index
      during CREATE INDEX, since there cannot possibly be any duplicates.
      Doing so wastes cycles due to unnecessary copying.  Make sure that we
      avoid it consistently.
      
      We already avoided unique index deduplication in the case where there
      were some spool2 tuples to merge.  That didn't account for the fact that
      spool2 is removed early/unset in the common case where it has no tuples
      that need to be merged (i.e. it failed to account for the "spool2 turns
      out to be unnecessary" optimization in _bt_spools_heapscan()).
      
      Oversight in commit 0d861bbb, which added nbtree deduplication
      
      Backpatch: 13-, where nbtree deduplication was introduced.
      5da8bf8b
    • Tom Lane's avatar
      Ensure that distributed timezone abbreviation files are plain ASCII. · 7fe3083f
      Tom Lane authored
      We had two occurrences of "Mitteleuropäische Zeit" in Europe.txt,
      though the corresponding entries in Default were spelled
      "Mitteleuropaeische Zeit".  Standardize on the latter spelling to
      avoid questions of which encoding to use.
      
      While here, correct a couple of other trivial inconsistencies between
      the Default file and the supposedly-matching entries in the *.txt
      files, as exposed by some checking with comm(1).  Also, add BDST to
      the Europe.txt file; it previously was only listed in Default.
      None of this has any direct functional effect.
      
      Per complaint from Christoph Berg.  As usual for timezone data patches,
      apply to all branches.
      
      Discussion: https://postgr.es/m/20200716100743.GE3534683@msg.df7cb.de
      7fe3083f
    • Peter Eisentraut's avatar
      Fix whitespace · 20ef3551
      Peter Eisentraut authored
      20ef3551
    • Peter Eisentraut's avatar
      Resolve gratuitous tabs in SQL file · 44f34365
      Peter Eisentraut authored
      44f34365
    • Amit Kapila's avatar
      Fix signal handler setup for SIGHUP in the apply launcher process. · 01160a3d
      Amit Kapila authored
      Commit 1e53fe0e has unified the usage of the config-file reload flag by
      using the same signal handler function for the SIGHUP signal at many places
      in the code.  By mistake, it used the wrong SIGNAL in apply launcher
      process for the SIGHUP signal handler function.
      
      Author: Bharath Rupireddy
      Reviewed-by: Dilip Kumar
      Backpatch-through: 13, where it was introduced
      Discussion: https://postgr.es/m/CALj2ACVzHCRnS20bOiEHaLtP5PVBENZQn4khdsSJQgOv_GM-LA@mail.gmail.com
      01160a3d
    • Thomas Munro's avatar
      Use MinimalTuple for tuple queues. · cdc71695
      Thomas Munro authored
      This representation saves 8 bytes per tuple compared to HeapTuple, and
      avoids the need to allocate, copy and free on the receiving side.
      
      Gather can emit the returned MinimalTuple directly, but GatherMerge now
      needs to make an explicit copy because it buffers multiple tuples at a
      time.  That should be no worse than before.
      Reviewed-by: default avatarSoumyadeep Chakraborty <soumyadeep2007@gmail.com>
      Discussion: https://postgr.es/m/CA%2BhUKG%2B8T_ggoUTAE-U%3DA%2BOcPc4%3DB0nPPHcSfffuQhvXXjML6w%40mail.gmail.com
      cdc71695
    • Thomas Munro's avatar
      Add huge_page_size setting for use on Linux. · d2bddc25
      Thomas Munro authored
      This allows the huge page size to be set explicitly.  The default is 0,
      meaning it will use the system default, as before.
      
      Author: Odin Ugedal <odin@ugedal.com>
      Discussion: https://postgr.es/m/20200608154639.20254-1-odin%40ugedal.com
      d2bddc25
  3. 16 Jul, 2020 2 commits
    • Andrew Dunstan's avatar
      Enable almost all TAP tests involving symlinks on Windows · d66b23b0
      Andrew Dunstan authored
      Windows has junction points which function as symbolic links for
      directories. This patch introduces a new function TestLib::dir_symlink()
      which creates a junction point on Windows and a standard Unix type
      symbolic link elsewhere.
      
      The function TestLib::perl2host is also modified, first to use cygpath
      where it's available (e.g. msys2) and second to allow it to succeed if
      the gandparent directory exists but the parent does not.
      
      Given these changes the only symlink tests that need to be skipped on
      Windows are those related to permissions or to use of readlink. The
      relevant tests for pg_basebackup and pg_rewind are therefore adjusted
      accordingly.
      
      Andrew Dunstan, reviewed by Peter Eisentraut and Michael Paquier.
      
      Discussion: https://postgr.es/m/c50a646c-d9bb-7c62-a4bf-8256ff6ff338@2ndquadrant.com
      d66b23b0
    • Michael Paquier's avatar
      Switch pg_test_fsync to use binary mode on Windows · 932f9fb5
      Michael Paquier authored
      pg_test_fsync has always opened files using the text mode on Windows, as
      this is the default mode used if not enforced by _setmode().
      
      This fixes a failure when running pg_test_fsync down to 12 because
      O_DSYNC and the text mode are not able to work together nicely.  We
      fixed the handling of O_DSYNC in 12~ for the tool by switching to the
      concurrent-safe version of fopen() in src/port/ with 0ba06e0b.  And
      40cfe860, by enforcing the text mode for compatibility reasons if O_TEXT
      or O_BINARY are not specified by the caller, broke pg_test_fsync.  For
      all versions, this avoids any translation overhead, and pg_test_fsync
      should test binary writes, so it is a gain in all cases.
      
      Note that O_DSYNC is still not handled correctly in ~11, leading to
      pg_test_fsync to show insanely high numbers for open_datasync() (using
      this property it is easy to notice that the binary mode is much
      faster).  This would require a backpatch of 0ba06e0b and 40cfe860, which
      could potentially break existing applications, so this is left out.
      
      There are no TAP tests for this tool yet, so I have checked all builds
      manually using MSVC.  We could invent a new option to run a single
      transaction instead of using a duration of 1s to make the tests a
      maximum short, but this is left as future work.
      
      Thanks to Bruce Momjian for the discussion.
      
      Reported-by: Jeff Janes
      Author: Michael Paquier
      Discussion: https://postgr.es/m/16526-279ded30a230d275@postgresql.org
      Backpatch-through: 9.5
      932f9fb5
  4. 15 Jul, 2020 4 commits
    • Peter Eisentraut's avatar
      pg_dump: Reorganize dumpFunc() and dumpAgg() · ed2c7f65
      Peter Eisentraut authored
      Similar to daa9fe8a, instead of
      repeating the almost same large query in each version branch, use one
      query and add a few columns to the SELECT list depending on the
      version.  This saves a lot of duplication.
      Reviewed-by: default avatarFabien COELHO <coelho@cri.ensmp.fr>
      Discussion: https://www.postgresql.org/message-id/flat/6594334b-40fd-14f1-6bc5-877afa3feed5@2ndquadrant.com
      ed2c7f65
    • Michael Paquier's avatar
      Fix handling of missing files when using pg_rewind with online source · 1d09fb1f
      Michael Paquier authored
      When working with an online source cluster, pg_rewind gets a list of all
      the files in the source data directory using a WITH RECURSIVE query,
      returning a NULL result for a file's metadata if it gets removed between
      the moment it is listed in a directory and the moment its metadata is
      obtained with pg_stat_file() (say a recycled WAL segment).  The query
      result was processed in such a way that for each tuple we checked only
      that the first file's metadata was NULL.  This could have two
      consequences, both resulting in a failure of the rewind:
      - If the first tuple referred to a removed file, all files from the
      source would be ignored.
      - Any file actually missing would not be considered as such.
      
      While on it, rework slightly the code so as no values are saved if we
      know that a file is going to be skipped.
      
      Issue introduced by b36805f3, so backpatch down to 9.5.
      
      Author: Justin Pryzby, Michael Paquier
      Reviewed-by: Daniel Gustafsson, Masahiko Sawada
      Discussion: https://postgr.es/m/20200713061010.GC23581@telsasoft.com
      Backpatch-through: 9.5
      1d09fb1f
    • Michael Paquier's avatar
      Fix compilation failure with sepgsql · e9491373
      Michael Paquier authored
      One change for getObjectIdentity() has been missed in 2a10fdc4, causing
      the module to not compile properly.  This was actually the only problem,
      and it happens that it is easy enough to check the compilation of the
      module on Debian after installing libselinux1-dev.
      
      Per buildfarm member rhinoceros.
      e9491373
    • Michael Paquier's avatar
      Eliminate cache lookup errors in SQL functions for object addresses · 2a10fdc4
      Michael Paquier authored
      When using the following functions, users could see various types of
      errors of the type "cache lookup failed for OID XXX" with elog(), that
      can only be used for internal errors:
      * pg_describe_object()
      * pg_identify_object()
      * pg_identify_object_as_address()
      
      The set of APIs managing object addresses for all object types are made
      smarter by gaining a new argument "missing_ok" that allows any caller to
      control if an error is raised or not on an undefined object.  The SQL
      functions listed above are changed to handle the case where an object is
      missing.
      
      Regression tests are added for all object types for the cases where
      these are undefined.  Before this commit, these cases failed with cache
      lookup errors, and now they basically return NULL (minus the name of the
      object type requested).
      
      Author: Michael Paquier
      Reviewed-by: Aleksander Alekseev, Dmitry Dolgov, Daniel Gustafsson,
      Álvaro Herrera, Kyotaro Horiguchi
      Discussion: https://postgr.es/m/CAB7nPqSZxrSmdHK-rny7z8mi=EAFXJ5J-0RbzDw6aus=wB5azQ@mail.gmail.com
      2a10fdc4
  5. 14 Jul, 2020 7 commits
    • Tom Lane's avatar
      Fix bitmap AND/OR scans on the inside of a nestloop partition-wise join. · 689696c7
      Tom Lane authored
      reparameterize_path_by_child() failed to reparameterize BitmapAnd
      and BitmapOr paths.  This matters only if such a path is chosen as
      the inside of a nestloop partition-wise join, where we have to pass
      in parameters from the outside of the nestloop.  If that did happen,
      we generated a bad plan that would likely lead to crashes at execution.
      
      This is not entirely reparameterize_path_by_child()'s fault though;
      it's the victim of an ancient decision (my ancient decision, I think)
      to not bother filling in param_info in BitmapAnd/Or path nodes.  That
      caused the function to believe that such nodes and their children
      contain no parameter references and so need not be processed.
      
      In hindsight that decision looks pretty penny-wise and pound-foolish:
      while it saves a few cycles during path node setup, we do commonly
      need the information later.  In particular, by reversing the decision
      and requiring valid param_info data in all nodes of a bitmap path
      tree, we can get rid of indxpath.c's get_bitmap_tree_required_outer()
      function, which computed the data on-demand.  It's not unlikely that
      that nets out as a savings of cycles in many scenarios.  A couple
      of other things in indxpath.c can be simplified as well.
      
      While here, get rid of some cases in reparameterize_path_by_child()
      that are visibly dead or useless, given that we only care about
      reparameterizing paths that can be on the inside of a parameterized
      nestloop.  This case reminds one of the maxim that untested code
      probably does not work, so I'm unwilling to leave unreachable code
      in this function.  (I did leave the T_Gather case in place even
      though it's not reached in the regression tests.  It's not very
      clear to me when the planner might prefer to put Gather below
      rather than above a nestloop, but at least in principle the case
      might be interesting.)
      
      Per bug #16536, originally from Arne Roland but with a test case
      by Andrew Gierth.  Back-patch to v11 where this code came in.
      
      Discussion: https://postgr.es/m/16536-2213ee0b3aad41fd@postgresql.org
      689696c7
    • Peter Eisentraut's avatar
      Fix -Wcast-function-type warnings · de8feb1f
      Peter Eisentraut authored
      Three groups of issues needed to be addressed:
      
      load_external_function() and related functions returned PGFunction,
      even though not necessarily all callers are looking for a function of
      type PGFunction.  Since these functions are really just wrappers
      around dlsym(), change to return void * just like dlsym().
      
      In dynahash.c, we are using strlcpy() where a function with a
      signature like memcpy() is expected.  This should be safe, as the new
      comment there explains, but the cast needs to be augmented to avoid
      the warning.
      
      In PL/Python, methods all need to be cast to PyCFunction, per Python
      API, but this now runs afoul of these warnings.  (This issue also
      exists in core CPython.)
      
      To fix the second and third case, we add a new type pg_funcptr_t that
      is defined specifically so that gcc accepts it as a special function
      pointer that can be cast to any other function pointer without the
      warning.
      
      Also add -Wcast-function-type to the standard warning flags, subject
      to configure check.
      Reviewed-by: default avatarTom Lane <tgl@sss.pgh.pa.us>
      Discussion: https://www.postgresql.org/message-id/flat/1e97628e-6447-b4fd-e230-d109cec2d584%402ndquadrant.com
      de8feb1f
    • David Rowley's avatar
      Add comment to explain an unused function parameter · 101f903e
      David Rowley authored
      Removing the unused 'miinfo' parameter has been raised a couple of times
      now.  It was decided in the 2nd discussion below that we're going to leave
      it alone.  It seems like it might be useful to add a comment to mention
      this fact so that nobody wastes any time in the future proposing its
      removal again.
      
      Discussion: https://postgr.es/m/CAApHDvpCf-qR5HC1rXskUM4ToV+3YDb4-n1meY=vpAHsRS_1PA@mail.gmail.com
      Discussion: https://postgr.es/m/CAE9k0P%3DFvcDswnSVtRpSyZMpcAWC%3DGp%3DifZ0HdfPaRQ%3D__LBtw%40mail.gmail.com
      101f903e
    • David Rowley's avatar
      Fix timing issue with ALTER TABLE's validate constraint · f1fcf2d3
      David Rowley authored
      An ALTER TABLE to validate a foreign key in which another subcommand
      already caused a pending table rewrite could fail due to ALTER TABLE
      attempting to validate the foreign key before the actual table rewrite
      takes place.  This situation could result in an error such as:
      
      ERROR:  could not read block 0 in file "base/nnnnn/nnnnn": read only 0 of 8192 bytes
      
      The failure here was due to the SPI call which validates the foreign key
      trying to access an index which is yet to be rebuilt.
      
      Similarly, we also incorrectly tried to validate CHECK constraints before
      the heap had been rewritten.
      
      The fix for both is to delay constraint validation until phase 3, after
      the table has been rewritten.  For CHECK constraints this means a slight
      behavioral change.  Previously ALTER TABLE VALIDATE CONSTRAINT on
      inheritance tables would be validated from the bottom up.  This was
      different from the order of evaluation when a new CHECK constraint was
      added.  The changes made here aligns the VALIDATE CONSTRAINT evaluation
      order for inheritance tables to be the same as ADD CONSTRAINT, which is
      generally top-down.
      
      Reported-by: Nazli Ugur Koyluoglu, using SQLancer
      Discussion: https://postgr.es/m/CAApHDvp%3DZXv8wiRyk_0rWr00skhGkt8vXDrHJYXRMft3TjkxCA%40mail.gmail.com
      Backpatch-through: 9.5 (all supported versions)
      f1fcf2d3
    • Michael Paquier's avatar
      Fix some header identifications · b8401c32
      Michael Paquier authored
      The following header files missed the shot:
      - jsonfuncs.h, as of ce0425b1.
      - jsonapi.h, as of beb46990.
      - llvmjit_emit.h as of 7ec0d80c.
      - partdesc.h, as of 1bb5e782.
      
      Author: Jesse Zhang
      Discussion: https://postgr.es/m/CAGf+fX4-8xULEOz09DE2dZGjT+q8VJ--rqfTpvcFwc+A4fc-3Q@mail.gmail.com
      b8401c32
    • Michael Paquier's avatar
      Fix comments related to table AMs · 9168793d
      Michael Paquier authored
      Incorrect function names were referenced.  As this fixes some portions
      of tableam.h, that is mentioned in the docs as something to look at when
      implementing a table AM, backpatch down to 12 where this has been
      introduced.
      
      Author: Hironobu Suzuki
      Discussion: https://postgr.es/m/8fe6d672-28dd-3f1d-7aed-ac2f6d599d3f@interdb.jp
      Backpatch-through: 12
      9168793d
    • Tom Lane's avatar
      Cope with lateral references in the quals of a subquery RTE. · a742ecf9
      Tom Lane authored
      The qual pushdown logic assumed that all Vars in a restriction clause
      must be Vars referencing subquery outputs; but since we introduced
      LATERAL, it's possible for such a Var to be a lateral reference instead.
      This led to an assertion failure in debug builds.  In a non-debug
      build, there might be no ill effects (if qual_is_pushdown_safe decided
      the qual was unsafe anyway), or we could get failures later due to
      construction of an invalid plan.  I've not gone to much length to
      characterize the possible failures, but at least segfaults in the
      executor have been observed.
      
      Given that this has been busted since 9.3 and it took this long for
      anybody to notice, I judge that the case isn't worth going to great
      lengths to optimize.  Hence, fix by just teaching qual_is_pushdown_safe
      that such quals are unsafe to push down, matching the previous behavior
      when it accidentally didn't fail.
      
      Per report from Tom Ellis.  Back-patch to all supported branches.
      
      Discussion: https://postgr.es/m/20200713175124.GQ8220@cloudinit-builder
      a742ecf9
  6. 13 Jul, 2020 7 commits
  7. 12 Jul, 2020 2 commits
    • Michael Paquier's avatar
      Fix test failure with -DENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS · ea3e15d1
      Michael Paquier authored
      Replication origins created by regression tests should have names
      starting with "regress_", and the test introduced in b1e48bbe for commit
      timestamps did not do that.
      
      Per buildfarm member longfin.
      
      Discussion: https://postgr.es/m/20200712122507.GD21680@paquier.xyz
      ea3e15d1
    • Michael Paquier's avatar
      Include replication origins in SQL functions for commit timestamp · b1e48bbe
      Michael Paquier authored
      This includes two changes:
      - Addition of a new function pg_xact_commit_timestamp_origin() able, for
      a given transaction ID, to return the commit timestamp and replication
      origin of this transaction.  An equivalent function existed in
      pglogical.
      - Addition of the replication origin to pg_last_committed_xact().
      
      The commit timestamp manager includes already APIs able to return the
      replication origin of a transaction on top of its commit timestamp, but
      the code paths for replication origins were never stressed as those
      functions have never looked for a replication origin, and the SQL
      functions available have never included this information since their
      introduction in 73c986ad.
      
      While on it, refactor a test of modules/commit_ts/ to use tstzrange() to
      check that a transaction timestamp is within the wanted range, making
      the test a bit easier to read.
      
      Bump catalog version.
      
      Author: Movead Li
      Reviewed-by: Madan Kumar, Michael Paquier
      Discussion: https://postgr.es/m/2020051116430836450630@highgo.ca
      b1e48bbe
  8. 11 Jul, 2020 1 commit