1. 11 Dec, 2019 5 commits
  2. 10 Dec, 2019 5 commits
    • Tom Lane's avatar
      Fix tuple column count in pg_control_init(). · 8729fa72
      Tom Lane authored
      Oversight in commit 2e4db241.
      
      Nathan Bossart
      
      Discussion: https://postgr.es/m/1B616360-396A-4482-AA28-375566C86160@amazon.com
      8729fa72
    • Peter Eisentraut's avatar
      Cosmetic cleaning of pg_config.h.win32 · 877b61e9
      Peter Eisentraut authored
      Clean up some comments (some generated by old versions of autoconf)
      and some random ordering differences, so it's easier to diff this
      against the default pg_config.h or pg_config.h.in.  Remove LOCALEDIR
      handling from pg_config.h.win32 altogether because it's already in
      pg_config_paths.h.
      877b61e9
    • Alvaro Herrera's avatar
      Add backend-only appendStringInfoStringQuoted · 6cafde1b
      Alvaro Herrera authored
      This provides a mechanism to emit literal values in informative
      messages, such as query parameters.  The new code is more complex than
      what it replaces, primarily because it wants to be more efficient.
      It also has the (currently unused) additional optional capability of
      specifying a maximum size to print.
      
      The new function lives out of common/stringinfo.c so that frontend users
      of that file need not pull in unnecessary multibyte-encoding support
      code.
      
      Author: Álvaro Herrera and Alexey Bashtanov, after a suggestion from Andres Freund
      Reviewed-by: Tom Lane
      Discussion: https://postgr.es/m/20190920203905.xkv5udsd5dxfs6tr@alap3.anarazel.de
      6cafde1b
    • Tom Lane's avatar
      In pg_ctl, work around ERROR_SHARING_VIOLATION on the postmaster log file. · 0da33c76
      Tom Lane authored
      On Windows, we use CMD.EXE to redirect the postmaster's stdout/stderr
      into a log file.  CMD.EXE will open that file with non-sharing-friendly
      parameters, and the file will remain open for a short time after the
      postmaster has removed postmaster.pid.  This can result in an
      ERROR_SHARING_VIOLATION failure if we attempt to start a new postmaster
      immediately with the same log file (e.g. during "pg_ctl restart").
      This seems to explain intermittent buildfarm failures we've been seeing
      on Windows machines.
      
      To fix, just open and close the log file using our own pgwin32_open(),
      which will wait if necessary to avoid the failure.  (Perhaps someday
      we should stop using CMD.EXE, but that would be a far more complex
      patch, and it doesn't seem worth the trouble ... yet.)
      
      Back-patch to v12.  This only solves the problem when frontend fopen()
      is redirected to pgwin32_fopen(), which has only been true since commit
      0ba06e0b.  Hence, no point in back-patching further, unless we care
      to back-patch that change too.
      
      Diagnosis and patch by Alexander Lakhin (bug #16154).
      
      Discussion: https://postgr.es/m/16154-1ccf0b537b24d5e0@postgresql.org
      0da33c76
    • Etsuro Fujita's avatar
      Fix handling of multiple AFTER ROW triggers on a foreign table. · 5a20b021
      Etsuro Fujita authored
      AfterTriggerExecute() retrieves a fresh tuple or pair of tuples from a
      tuplestore and then stores the tuple(s) in the passed-in slot(s) if
      AFTER_TRIGGER_FDW_FETCH, while it uses the most-recently-retrieved
      tuple(s) stored in the slot(s) if AFTER_TRIGGER_FDW_REUSE.  This was
      done correctly before 12, but commit ff11e7f4 broke it by mistakenly
      clearing the tuple(s) stored in the slot(s) in that function, leading to
      an assertion failure as reported in bug #16139 from Alexander Lakhin.
      
      Also, fix some other issues with the aforementioned commit in passing:
      
      * For tg_newslot, which is a slot added to the TriggerData struct by the
        commit to store new updated tuples, it didn't ensure the slot was NULL
        if there was no such tuple.
      * The commit failed to update the documentation about the trigger
        interface.
      
      Author: Etsuro Fujita
      Backpatch-through: 12
      Discussion: https://postgr.es/m/16139-94f9ccf0db6119ec%40postgresql.org
      5a20b021
  3. 09 Dec, 2019 3 commits
    • Tom Lane's avatar
      Fix race condition in our Windows signal emulation. · 28e6a2fd
      Tom Lane authored
      pg_signal_dispatch_thread() responded to the client (signal sender)
      and disconnected the pipe before actually setting the shared variables
      that make the signal visible to the backend process's main thread.
      In the worst case, it seems, effective delivery of the signal could be
      postponed for as long as the machine has any other work to do.
      
      To fix, just move the pg_queue_signal() call so that we do it before
      responding to the client.  This essentially makes pgkill() synchronous,
      which is a stronger guarantee than we have on Unix.  That may be
      overkill, but on the other hand we have not seen comparable timing bugs
      on any Unix platform.
      
      While at it, add some comments to this sadly underdocumented code.
      
      Problem diagnosis and fix by Amit Kapila; I just added the comments.
      
      Back-patch to all supported versions, as it appears that this can cause
      visible NOTIFY timing oddities on all of them, and there might be
      other misbehavior due to slow delivery of other signals.
      
      Discussion: https://postgr.es/m/32745.1575303812@sss.pgh.pa.us
      28e6a2fd
    • Tom Lane's avatar
      Improve isolationtester's timeout management. · 99351a8b
      Tom Lane authored
      isolationtester.c had a hard-wired limit of 3 minutes per test step.
      It now emerges that this isn't quite enough for some of the slowest
      buildfarm animals.  This isn't the first time we've had to raise
      this limit (cf. 1db439ad), so let's make it configurable.  This
      patch raises the default to 5 minutes, and introduces an environment
      variable PGISOLATIONTIMEOUT that can be set if more time is needed,
      following the precedent of PGCTLTIMEOUT.
      
      Also, modify isolationtester so that when the timeout is hit,
      it explicitly reports having sent a cancel.  This makes the regression
      failure log considerably more intelligible.  (In the worst case, a
      timed-out test might actually be reported as "passing" without this
      extra output, so arguably this is a bug fix in itself.)
      
      In passing, update the README file, which had apparently not gotten
      touched when we added "make check" support here.
      
      Back-patch to 9.6; older versions don't have comparable timeout logic.
      
      Discussion: https://postgr.es/m/22964.1575842935@sss.pgh.pa.us
      99351a8b
    • Amit Kapila's avatar
      Fix typos in miscinit.c. · 2d0fdfac
      Amit Kapila authored
      Commit f13ea95f moved the description of postmaster.pid file contents
      from miscadmin.h to pidfile.h, but missed to update the comments in
      miscinit.c.
      
      Author: Hadi Moshayedi
      Reviewed-by: Amit Kapila
      Backpatch-through: 10
      Discussion: https://postgr.es/m/CAK=1=WpYEM9x3LGkaxgXaxeYQjnkdW8XLsxrYRTE2Gq-H83FMw@mail.gmail.com
      2d0fdfac
  4. 08 Dec, 2019 2 commits
  5. 07 Dec, 2019 1 commit
  6. 06 Dec, 2019 6 commits
    • Tom Lane's avatar
      Improve test coverage of ruleutils.c. · 830d1c73
      Tom Lane authored
      While fooling around with the EXPLAIN improvements I've been working
      on, I noticed that there were some large gaps in our test coverage
      of ruleutils.c, according to the code coverage report.  This commit
      just adds a few test cases to improve coverage of:
      get_name_for_var_field()
      get_update_query_targetlist_def()
      isSimpleNode()
      get_sublink_expr()
      830d1c73
    • Jeff Davis's avatar
      Fix comments in execGrouping.c · 30d47723
      Jeff Davis authored
      Commit 5dfc1981 missed updating some comments.
      
      Also, fix a comment typo found in passing.
      
      Author: Jeff Davis
      Discussion: https://postgr.es/m/9723131d247b919f94699152647fa87ee0bc02c2.camel%40j-davis.com
      30d47723
    • Tom Lane's avatar
      Disallow non-default collation in ADD PRIMARY KEY/UNIQUE USING INDEX. · fbbf6809
      Tom Lane authored
      When creating a uniqueness constraint using a pre-existing index,
      we have always required that the index have the same properties you'd
      get if you just let a new index get built.  However, when collations
      were added, we forgot to add the index's collation to that check.
      
      It's hard to trip over this without intentionally trying to break it:
      you'd have to explicitly specify a different collation in CREATE
      INDEX, then convert it to a pkey or unique constraint.  Still, if you
      did that, pg_dump would emit a script that fails to reproduce the
      index's collation.  The main practical problem is that after a
      pg_upgrade the index would be corrupt, because its actual physical
      order wouldn't match what pg_index says.  A more theoretical issue,
      which is new as of v12, is that if you create the index with a
      nondeterministic collation then it wouldn't be enforcing the normal
      notion of uniqueness, causing the constraint to mean something
      different from a normally-created constraint.
      
      To fix, just add collation to the conditions checked for index
      acceptability in ADD PRIMARY KEY/UNIQUE USING INDEX.  We won't try
      to clean up after anybody who's already created such a situation;
      it seems improbable enough to not be worth the effort involved.
      (If you do get into trouble, a REINDEX should be enough to fix it.)
      
      In principle this is a long-standing bug, but I chose not to
      back-patch --- the odds of causing trouble seem about as great
      as the odds of preventing it, and both risks are very low anyway.
      
      Per report from Alexey Bashtanov, though this is not his preferred
      fix.
      
      Discussion: https://postgr.es/m/b05ce36a-cefb-ca5e-b386-a400535b1c0b@imap.cc
      fbbf6809
    • Michael Paquier's avatar
      Fix handling of OpenSSL's SSL_clear_options · 7d0bcb04
      Michael Paquier authored
      This function is supported down to OpenSSL 0.9.8, which is the oldest
      version supported since 593d4e47 (from Postgres 10 onwards), and is used
      since e3bdb2d9 (from 11 onwards).  It is defined as a macro from OpenSSL
      0.9.8 to 1.0.2, and as a function in 1.1.0 and newer versions.  However,
      the configure check present is only adapted for functions.  So, even if
      the code would be able to compile, configure fails to detect the macro,
      causing it to be ignored when compiling the code with OpenSSL from 0.9.8
      to 1.0.2.
      
      The code needs a configure check as per a364dfa4, which has fixed a
      compilation issue with a past version of LibreSSL in NetBSD 5.1.  On
      HEAD, just remove the configure check as the last release of NetBSD 5 is
      from 2014 (and we have no more buildfarm members for it).  In 11 and 12,
      improve the configure logic so as both macros and functions are
      correctly detected.  This makes NetBSD 5 still work on already-released
      branches, but not for 13 onwards.
      
      The patch for HEAD is from me, and Daniel has written the version to use
      for the back-branches.
      
      Author: Michael Paquier, Daniel Gustaffson
      Reviewed-by: Tom Lane
      Discussion: https://postgr.es/m/20191205083252.GE5064@paquier.xyz
      Discussion: https://postgr.es/m/98F7F99E-1129-41D8-B86B-FE3B1E286881@yesql.se
      Backpatch-through: 11
      7d0bcb04
    • Michael Paquier's avatar
      Improve some comments in pg_upgrade.c · 690c8802
      Michael Paquier authored
      When restoring database schemas on a new cluster, database "template1"
      is processed first, followed by all other databases in parallel,
      including "postgres".  Both "postgres" and "template1" have some extra
      handling to propagate each one's properties, but comments were confusing
      regarding which one is processed where.
      
      Author: Julien Rouhaud
      Reviewed-by: Daniel Gustafsson
      Discussion: https://postgr.es/m/CAOBaU_a2iviTG7FE10yO_gcW+zQCHNFhRA_NDiktf3UR65BHdw@mail.gmail.com
      690c8802
    • Michael Paquier's avatar
      Remove configure check for OpenSSL's SSL_get_current_compression() · 28f4bba6
      Michael Paquier authored
      This function has been added in OpenSSL 0.9.8, which is the oldest
      version supported on HEAD, so checking for it at configure time is
      useless.  Both the frontend and backend code did not even bother to use
      it.
      
      Reported-by: Daniel Gustafsson
      Author: Michael Paquier
      Reviewed-by: Daniel Gustafsson, Tom Lane
      Discussion: https://postgr.es/m/20191205083252.GE5064@paquier.xyz
      Discussion: https://postgr.es/m/98F7F99E-1129-41D8-B86B-FE3B1E286881@yesql.se
      28f4bba6
  7. 05 Dec, 2019 2 commits
  8. 04 Dec, 2019 6 commits
  9. 03 Dec, 2019 7 commits
    • Tomas Vondra's avatar
      Ensure maxlen is at leat 1 in dict_int · b5273943
      Tomas Vondra authored
      The dict_int text search dictionary template accepts maxlen parameter,
      which is then used to cap the length of input strings. The value was
      not properly checked, and the code simply does
      
          txt[d->maxlen] = '\0';
      
      to insert a terminator, leading to segfaults with negative values.
      
      This commit simply rejects values less than 1. The issue was there since
      dct_int was introduced in 9.3, so backpatch all the way back to 9.4
      which is the oldest supported version.
      
      Reported-by: cili
      Discussion: https://postgr.es/m/16144-a36a5bef7657047d@postgresql.org
      Backpatch-through: 9.4
      b5273943
    • Tom Lane's avatar
      Further sync postgres_fdw's "Relations" output with the rest of EXPLAIN. · bf39b3af
      Tom Lane authored
      EXPLAIN generally only adds schema qualifications to table names when
      VERBOSE is specified.  In postgres_fdw's "Relations" output, table
      names were always so qualified, but that was an implementation
      restriction: in the original coding, we didn't have access to the
      verbose flag at the time the string was generated.  After the code
      rearrangement of commit 4526951d, we do have that info available
      at the right time, so make this output follow the normal rule.
      
      Discussion: https://postgr.es/m/12424.1575168015@sss.pgh.pa.us
      bf39b3af
    • Michael Paquier's avatar
      Fix thinkos from commit 9989d37d · 68ab9829
      Michael Paquier authored
      Error messages referring to incorrect WAL segment names could have been
      generated for a fsync() failure or when creating a new segment at the
      end of recovery.
      68ab9829
    • Peter Eisentraut's avatar
      Fix alter_system_table test · 88d45ac7
      Peter Eisentraut authored
      Add workaround for disabling ENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS
      warning for the test that tries to create a tablespace with a reserved
      name.
      
      Discussion: https://www.postgresql.org/message-id/flat/E1iacW7-0003h6-6U%40gemulon.postgresql.org
      88d45ac7
    • Michael Paquier's avatar
      Remove XLogFileNameP() from the tree · 9989d37d
      Michael Paquier authored
      XLogFileNameP() is a wrapper routine able to build a palloc'd string for
      a WAL segment name, which is used for error string generation.  There
      were several code paths where it gets called in a critical section,
      where memory allocation is not allowed.  This results in triggering
      an assertion failure instead of generating the wanted error message.
      
      Another, more annoying, problem is that if the allocation to generate
      the WAL segment name fails on OOM, then the failure would be escalated
      to a PANIC.
      
      This removes the routine and all its callers are replaced with a logic
      using a fixed-size buffer.  This way, all the existing mistakes are
      fixed and future ones are prevented.
      
      Author: Masahiko Sawada
      Reviewed-by: Michael Paquier, Álvaro Herrera
      Discussion: https://postgr.es/m/CA+fd4k5gC9H4uoWMLg9K_QfNrnkkdEw+-AFveob9YX7z8JnKTA@mail.gmail.com
      9989d37d
    • Michael Paquier's avatar
      Fix failures with TAP tests of pg_ctl on Windows · e5532f19
      Michael Paquier authored
      On Windows, all the hosts spawned by the TAP tests bind to 127.0.0.1.
      Hence, if there is a port conflict, starting a cluster would immediately
      fail.  One of the test scripts of pg_ctl initializes a node without
      PostgresNode.pm, using the default port 5432.  This could cause
      unexpected startup failures in the tests if an independent server was up
      and running on the same host (the reverse is also possible, though more
      unlikely).  Fix this issue by assigning properly a free port to the node
      configured, in the same range used as for the other nodes part of the
      tests.
      
      Author: Michael Paquier
      Reviewed-by: Andrew Dunstan
      Discussion: https://postgr.es/m/20191202031444.GC1696@paquier.xyz
      Backpatch-through: 11
      e5532f19
    • Tom Lane's avatar
      Fix EXPLAIN's column alias output for mismatched child tables. · 55a1954d
      Tom Lane authored
      If an inheritance/partitioning parent table is assigned some column
      alias names in the query, EXPLAIN mapped those aliases onto the
      child tables' columns by physical position, resulting in bogus output
      if a child table's columns aren't one-for-one with the parent's.
      
      To fix, make expand_single_inheritance_child() generate a correctly
      re-mapped column alias list, rather than just copying the parent
      RTE's alias node.  (We have to fill the alias field, not just
      adjust the eref field, because ruleutils.c will ignore eref in
      favor of looking at the real column names.)
      
      This means that child tables will now always have alias fields in
      plan rtables, where before they might not have.  That results in
      a rather substantial set of regression test output changes:
      EXPLAIN will now always show child tables with aliases that match
      the parent table (usually with "_N" appended for uniqueness).
      But that seems like a net positive for understandability, since
      the parent alias corresponds to something that actually appeared
      in the original query, while the child table names didn't.
      (Note that this does not change anything for cases where an explicit
      table alias was written in the query for the parent table; it
      just makes cases without such aliases behave similarly to that.)
      Hence, while we could avoid these subsidiary changes if we made
      inherit.c more complicated, we choose not to.
      
      Discussion: https://postgr.es/m/12424.1575168015@sss.pgh.pa.us
      55a1954d
  10. 02 Dec, 2019 3 commits
    • Tom Lane's avatar
      Add a reverse-translation column number array to struct AppendRelInfo. · ce76c0ba
      Tom Lane authored
      This provides for cheaper mapping of child columns back to parent
      columns.  The one existing use-case in examine_simple_variable()
      would hardly justify this by itself; but an upcoming bug fix will
      make use of this array in a mainstream code path, and it seems
      likely that we'll find other uses for it as we continue to build
      out the partitioning infrastructure.
      
      Discussion: https://postgr.es/m/12424.1575168015@sss.pgh.pa.us
      ce76c0ba
    • Tom Lane's avatar
      Make postgres_fdw's "Relations" output agree with the rest of EXPLAIN. · 4526951d
      Tom Lane authored
      The relation aliases shown in the "Relations" line for a foreign scan
      didn't always agree with those used in the rest of EXPLAIN's output.
      The regression test result changes appearing here provide examples.
      
      It's really impossible for postgres_fdw to duplicate EXPLAIN's alias
      assignment logic during postgresGetForeignRelSize(), because of the
      de-duplication that EXPLAIN does on a global basis --- and anyway,
      trying to duplicate that would be unmaintainable.  Instead, just put
      numeric rangetable indexes into the string, and convert those to
      table names/aliases in postgresExplainForeignScan, which does have
      access to the results of ruleutils.c's alias assignment logic.
      Aside from being more reliable, this shifts some work from planning
      to EXPLAIN, which is a good tradeoff for performance.  (I also
      changed from using StringInfo to using psprintf, which makes the
      code slightly simpler and reduces its memory consumption.)
      
      A kluge required by this solution is that we have to reverse-engineer
      the rtoffset applied by setrefs.c.  If that logic ever fails
      (presumably because the member tables of a join got offset by
      different amounts), we'll need some more cooperation with setrefs.c
      to keep things straight.  But for now, there's no need for that.
      
      Arguably this is a back-patchable bug fix, but since this is a mostly
      cosmetic issue and there have been no field complaints, I'll refrain
      for now.
      
      Discussion: https://postgr.es/m/12424.1575168015@sss.pgh.pa.us
      4526951d
    • Michael Paquier's avatar
      Add query cancellation capabilities in pgbench init phase · 1d468b9a
      Michael Paquier authored
      This can be useful to stop data generation happening on the server for
      long-running queries caused by large scale factors.  This cannot happen
      by default as data is generated on the client, but it is possible to
      control the initialization steps of pgbench to do that.
      
      Reported-by: Fujii Masao
      Author: Fabien Coelho
      Discussion: https://postgr.es/m/alpine.DEB.2.21.1910311939430.27369@lancre
      Discussion: https://postgr.es/m/CAHGQGwHWEyTXxZh46qgFY8a2bDF_EYeUdp3+_Hy=qLZSzwVPKg@mail.gmail.com
      1d468b9a