Correctifs appliqués

Tom Lane pushed:

  • Enforce a specific order for probing library loadability in pg_upgrade. pg_upgrade checks whether all the shared libraries used in the old cluster are also available in the new one by issuing LOAD for each library name. Previously, it cared not what order it did the LOADs in. Ideally it should not have to care, but currently the transform modules in contrib fail unless both the language and datatype modules they depend on are loaded first. A backend-side solution for that looks possible but probably not back-patchable, so as a stopgap measure, let's do the LOAD tests in order by library name length. That should fix the problem for reasonably-named transform modules, eg "hstore_plpython" will be loaded after both "hstore" and "plpython". (Yeah, it's a hack.) In a larger sense, having a predictable order of these probes is a good thing, since it will make upgrades predictably work or not work in the face of inter-library dependencies. Also, this patch replaces O(N^2) de-duplication logic with O(N log N) logic, which could matter in installations with very many databases. So I don't foresee reverting this even after we have a proper fix for the library-dependency problem. In passing, improve a couple of SQL queries used here. Per complaint from Andrew Dunstan that pg_upgrade'ing the transform contrib modules failed. Back-patch to 9.5 where transform modules were introduced. Discussion: <>
  • Show a sensible value in pg_settings.unit for GUC_UNIT_XSEGS variables. Commit 88e982302 invented GUC_UNIT_XSEGS for min_wal_size and max_wal_size, but neglected to make it display sensibly in pg_settings.unit (by adding a case to the switch in GetConfigOptionByNum). Fix that, and adjust said switch to throw a run-time error the next time somebody forgets. In passing, avoid using a static buffer for the output string --- the rest of this function pstrdup's from a local buffer, and I see no very good reason why the units code should do it differently and less safely. Per report from Otar Shavadze. Back-patch to 9.5 where the new unit type was added. Report: <>
  • Convert contrib/hstore_plpython to not use direct linking to other modules. Previously, on most platforms, we allowed hstore_plpython's references to hstore and plpython to be unresolved symbols at link time, trusting the dynamic linker to resolve them when the module is loaded. This has a number of problems, the worst being that the dynamic linker does not know where the references come from and can do nothing but fail if those other modules haven't been loaded. We've more or less gotten away with that for the limited use-case of datatype transform modules, but even there, it requires some awkward hacks, most recently commit 83c249200. Instead, let's not treat these references as linker-resolvable at all, but use function pointers that are manually filled in by the module's _PG_init function. There are few enough contact points that this doesn't seem unmaintainable, at least for these use-cases. (Note that the same technique wouldn't work at all for decoupling from libpython itself, but fortunately that's just a standard shared library and can be linked to normally.) This is an initial patch that just converts hstore_plpython. If the buildfarm doesn't find any fatal problems, I'll work on the other transform modules soon. Tom Lane, per an idea of Andres Freund's. Discussion: <>
  • Fix hstore_plpython for Python 3. In commit d51924be8, I overlooked the need to provide linkage for PLyUnicode_FromStringAndSize, because that's only used (and indeed only exists) in Python 3 builds. In light of the need to #if this item, rearrange the ordering of the code related to each function pointer, so as not to need more #if's than absolutely necessary. Per buildfarm.
  • Improve (I hope) our autolocation of the Python shared library. Older versions of Python produce garbage (or at least useless) values of get_config_vars('LDLIBRARY'). Newer versions produce garbage (or at least useless) values of get_config_vars('SO'), which was defeating our configure logic that attempted to identify where the Python shlib really is. The net result, at least with a stock Python 3.5 installation on macOS, was that we were linking against a static library in the mistaken belief that it was a shared library. This managed to work, if you count statically absorbing libpython into as working. But it no longer works as of commit d51924be8, because now we get separate static copies of libpython in and, and those can't interoperate on the same data. There are some other infelicities like assuming that nobody ever installs a private version of Python on a macOS machine. Hence, forget about looking in $python_configdir for the Python shlib; as far as I can tell no version of Python has ever put one there, and certainly no currently-supported version does. Also, rather than relying on get_config_vars('SO'), just try all the possibilities for shlib extensions. Also, rather than trusting Py_ENABLE_SHARED, believe we've found a shlib only if it has a recognized extension. Last, explicitly cope with the possibility that the shlib is really in /usr/lib and $python_libdir is a red herring --- this is the actual situation on older macOS, but we were only accidentally working with it. Discussion: <>
  • Avoid direct cross-module links in hstore_plperl and ltree_plpython, too. Just turning the crank on the project started in commit d51924be8. These cases turn out to be exact subsets of the boilerplate needed for hstore_plpython. Discussion: <>
  • Huh, we do need to look in $python_configdir for the Python shlib. Debian does it that way, for no doubt what seems to them a good reason. Thanks to Aidan Van Dyk for confirmation.
  • In python shlib probe, cater for OpenBSD, which omits the .so symlink. Most Unix-oid platforms provide a symlink "" -> "" to allow the linker to resolve a reference "-lfoo" to a version-numbered shared library. OpenBSD has apparently hacked ld(1) to do this internally, because there are no such symlinks to be found in their library directories. Tweak the new code in PGAC_CHECK_PYTHON_EMBED_SETUP to cope. Per buildfarm member curculio.
  • Try to fix python shlib probe for MinGW. Per discussion with Andrew Dunstan, it seems there are three peculiarities of the Python installation on MinGW (or at least, of the instance on buildfarm member frogmouth). First, the library name doesn't contain "2.7" but just "27". It looks like we can deal with that by consulting get_config_vars('VERSION') to see whether a dot should be used or not. Second, the library might be in c:/Windows/System32, analogously to it possibly being in /usr/lib on Unix-oid platforms. Third, it's apparently not standard to use the prefix "lib" on the file name. This patch will accept files with or without "lib", but the first part of that may well be dead code.
  • Remove -Wl,-undefined,dynamic_lookup in macOS build. We don't need this anymore, and it prevents build-time error checking that's usually good to have, so remove it. Undoes one change of commit cac765820. Unfortunately, it's much harder to get a similar effect on other common platforms, because we don't want the linker to throw errors for symbols that will be resolved in the core backend. Only macOS and AIX expect the core backend executable to be available while linking loadable modules, so only these platforms can usefully throw errors for unresolved symbols at link time. Discussion: <>
  • Fix pg_dump to work against pre-9.0 servers again. getBlobs' queries for pre-9.0 servers were broken in two ways: the 7.x/8.x query uses DISTINCT so it can't have unspecified-type NULLs in the target list, and both that query and the 7.0 one failed to provide the correct output column labels, so that the subsequent code to extract data from the PGresult would fail. Back-patch to 9.6 where the breakage was introduced (by commit 23f34fa4b). Amit Langote and Tom Lane Discussion: <>
  • Fix python shlib probe for Cygwin. On buildfarm member cockatiel, that library is in /usr/bin. (Possibly we should look at $PATH on this platform?) Per off-list report from Andrew Dunstan.
  • libpqwalreceiver needs to link with libintl when using --enable-nls. The need for this was previously obscured even on picky platforms by the hack we used to support direct cross-module references in the transforms contrib modules. Now that that hack is gone, the undefined symbol is exposed, as reported by Robert Haas. Back-patch to 9.5 where we started to use -Wl,-undefined,dynamic_lookup. I'm a bit surprised that the older branches don't seem to contain any gettext references in this module, but since they don't fail at build time, they must not. (We might be able to get away with leaving this alone in 9.5/9.6, but I think it's cleaner if the reference gets resolved at link time.) Report: <>
  • Fix two bugs in merging of inherited CHECK constraints. Historically, we've allowed users to add a CHECK constraint to a child table and then add an identical CHECK constraint to the parent. This results in "merging" the two constraints so that the pre-existing child constraint ends up with both conislocal = true and coninhcount > 0. However, if you tried to do it in the other order, you got a duplicate constraint error. This is problematic for pg_dump, which needs to issue separated ADD CONSTRAINT commands in some cases, but has no good way to ensure that the constraints will be added in the required order. And it's more than a bit arbitrary, too. The goal of complaining about duplicated ADD CONSTRAINT commands can be served if we reject the case of adding a constraint when the existing one already has conislocal = true; but if it has conislocal = false, let's just make the ADD CONSTRAINT set conislocal = true. In this way, either order of adding the constraints has the same end result. Another problem was that the code allowed creation of a parent constraint marked convalidated that is merged with a child constraint that is !convalidated. In this case, an inheritance scan of the parent table could emit some rows violating the constraint condition, which would be an unexpected result given the marking of the parent constraint as validated. Hence, forbid merging of constraints in this case. (Note: valid child and not-valid parent seems fine, so continue to allow that.) Per report from Benedikt Grundmann. Back-patch to 9.2 where we introduced possibly-not-valid check constraints. The second bug obviously doesn't apply before that, and I think the first doesn't either, because pg_dump only gets into this situation when dealing with not-valid constraints. Report: <> Discussion: <>
  • Fix incorrect handling of polymorphic aggregates used as window functions. The transfunction was told that its first argument and result were of the window function output type, not the aggregate state type. This'd only matter if the transfunction consults get_fn_expr_argtype, which typically only polymorphic functions would do. Although we have several regression tests around polymorphic aggs, none of them detected this mistake --- in fact, they still didn't fail when I injected the same mistake into nodeAgg.c. So add some more tests covering both plain agg and window-function-agg cases. Per report from Sebastian Luque. Back-patch to 9.6 where the error was introduced (by sloppy refactoring in commit 804163bc2, looks like). Report: <>
  • Use unnamed POSIX semaphores, if available, on Linux and FreeBSD. We've had support for using unnamed POSIX semaphores instead of System V semaphores for quite some time, but it was not used by default on any platform. Since many systems have rather small limits on the number of SysV semaphores allowed, it seems desirable to switch to POSIX semaphores where they're available and don't create performance or kernel resource problems. Experimentation by me shows that unnamed POSIX semaphores are at least as good as SysV semaphores on Linux, and we previously had a report from Maksym Sobolyev that FreeBSD is significantly worse with SysV semaphores than POSIX ones. So adjust those two platforms to use unnamed POSIX semaphores, if configure can find the necessary library functions. If this goes well, we may switch other platforms as well, but it would be advisable to test them individually first. It's not currently contemplated that we'd encourage users to select a semaphore API for themselves, but anyone who wants to experiment can add PREFERRED_SEMAPHORES=UNNAMED_POSIX (or NAMED_POSIX, or SYSV) to their configure command line to do so. I also tweaked configure to report which API it's selected, mainly so that we can tell that from buildfarm reports. I did not touch the user documentation's discussion about semaphores; that will need some adjustment once the dust settles. Discussion: <>

Stephen Frost pushed:

  • Fix RLS with COPY (col1, col2) FROM tab Attempting to COPY a subset of columns from a table with RLS enabled would fail due to an invalid query being constructed (using a single ColumnRef with the list of fields to exact in 'fields', but that's for the different levels of an indirection for a single column, not for specifying multiple columns). Correct by building a ColumnRef and then RestTarget for each column being requested and then adding those to the targetList for the select query. Include regression tests to hopefully catch if this is broken again in the future. Patch-By: Adam Brightwell Reviewed-By: Michael Paquier

Andres Freund pushed:

  • Correct logical decoding restore behaviour for subtransactions. Before initializing iteration over a subtransaction's changes, the last few changes were not spilled to disk. That's correct if the transaction didn't spill to disk, but otherwise... This bug can lead to missed or misorderd subtransaction contents when they were spilled to disk. Move spilling of the remaining in-memory changes to ReorderBufferIterTXNInit(), where it can easily be applied to the top transaction and, if present, subtransactions. Since this code had too many bugs already, noticeably increase test coverage. Fixes: #14319 Reported-By: Huan Ruan Discussion: <> Backport: 9,4-, where logical decoding was added
  • Fix fallback implementation of pg_atomic_write_u32(). I somehow had assumed that in the spinlock (in turn possibly using semaphores) based fallback atomics implementation 32 bit writes could be done without a lock. As far as the write goes that's correct, since postgres supports only platforms with single-copy atomicity for aligned 32bit writes. But writing without holding the spinlock breaks read-modify-write operations like pg_atomic_compare_exchange_u32(), since they'll potentially "miss" a concurrent write, which can't happen in actual hardware implementations. In 9.6+ when using the fallback atomics implementation this could lead to buffer header locks not being properly marked as released, and potentially some related state corruption. I don't see a related danger in 9.5 (earliest release with the API), because pg_atomic_write_u32() wasn't used in a concurrent manner there. The state variable of local buffers, before this change, were manipulated using pg_atomic_write_u32(), to avoid unnecessary synchronization overhead. As that'd not be the case anymore, introduce and use pg_atomic_unlocked_write_u32(), which does not correctly interact with RMW operations. This bug only caused issues when postgres is compiled on platforms without atomics support (i.e. no common new platform), or when compiled with --disable-atomics, which explains why this wasn't noticed in testing. Reported-By: Tom Lane Discussion: <> Backpatch: 9.5-, where the atomic operations API was introduced.

Heikki Linnakangas pushed:

  • Update comment. mergepreread()/mergeprereadone() don't exist anymore, the function that does roughly the same is now called mergereadnext().
  • Fix another outdated comment. Preloading is done by logtape.c now.
  • Change the way pre-reading in external sort's merge phase works. Don't pre-read tuples into SortTuple slots during merge. Instead, use the memory for larger read buffers in logtape.c. We're doing the same number of READTUP() calls either way, but managing the pre-read SortTuple slots is much more complicated. Also, the on-tape representation is more compact than SortTuples, so we can fit more pre-read tuples into the same amount of memory this way. And we have better cache-locality, when we use just a small number of SortTuple slots. Now that we only hold one tuple from each tape in the SortTuple slots, we can greatly simplify the "batch memory" management. We now maintain a small set of fixed-sized slots, to hold the tuples, and fall back to palloc() for larger tuples. We use this method during all merge phases, not just the final merge, and also when randomAccess is requested, and also in the TSS_SORTEDONTAPE case. In other words, it's used whenever we do an external sort. Reviewed by Peter Geoghegan and Claudio Freire. Discussion: <>
  • Fix excessive memory consumption in the new sort pre-reading code. LogicalTapeRewind() should not allocate large read buffer, if the tape is completely empty. The calling code relies on that, for its calculation of how much memory to allocate for the read buffers. That lead to massive overallocation of memory, if maxTapes was high, but only a few tapes were actually used. Reported by Tomas Vondra Discussion: <>
  • Disable synchronous commits in pg_rewind. If you point pg_rewind to a server that is using synchronous replication, with "pg_rewind --source-server=...", and the replication is not working for some reason, pg_rewind will get stuck because it creates a temporary table, which needs to be replicated. You could call broken replication a pilot error, but pg_rewind is often used in special circumstances, when there are changes to the replication setup. We don't do any "real" updates, and we don't care about fsyncing or replicating the operations on the temporary tables, so fix that by setting synchronous_commit off. Michael Banck, Michael Paquier. Backpatch to 9.5, where pg_rewind was introduced. Discussion: <>
  • Don't share SSL_CTX between libpq connections. There were several issues with the old coding: 1. There was a race condition, if two threads opened a connection at the same time. We used a mutex around SSL_CTX_* calls, but that was not enough, e.g. if one thread SSL_CTX_load_verify_locations() with one path, and another thread set it with a different path, before the first thread got to establish the connection. 2. Opening two different connections, with different sslrootcert settings, seemed to fail outright with "SSL error: block type is not 01". Not sure why. 3. We created the SSL object, before calling SSL_CTX_load_verify_locations and SSL_CTX_use_certificate_chain_file on the SSL context. That was wrong, because the options set on the SSL context are propagated to the SSL object, when the SSL object is created. If they are set after the SSL object has already been created, they won't take effect until the next connection. (This is bug #14329) At least some of these could've been fixed while still using a shared context, but it would've been more complicated and error-prone. To keep things simple, let's just use a separate SSL context for each connection, and accept the overhead. Backpatch to all supported versions. Report, analysis and test case by Kacper Zuk. Discussion: <>
  • Clear OpenSSL error queue after failed X509_STORE_load_locations() call. Leaving the error in the error queue used to be harmless, because the X509_STORE_load_locations() call used to be the last step in initialize_SSL(), and we would clear the queue before the next SSL_connect() call. But previous commit moved things around. The symptom was that if a CRL file was not found, and one of the subsequent initialization steps, like loading the client certificate or private key, failed, we would incorrectly print the "no such file" error message from the earlier X509_STORE_load_locations() call as the reason. Backpatch to all supported versions, like the previous patch.
  • Don't allow both --source-server and --source-target args to pg_rewind. They are supposed to be mutually exclusive, but there was no check for that. Michael Banck Discussion: <>
  • Make TAP test suites to work, when @INC does not contain current dir. Recent Perl and/or new Linux distributions are starting to remove "." from the @INC list by default. That breaks pg_rewind and ssl test suites, which use helper perl modules that reside in the same directory. To fix, add the current source directory explicitly to prove's include dir. The script probably also needs something like this, but I wasn't able to remove '.' from @INC on Windows to test this, and don't want to try doing that blindly. Discussion: <>
  • Remove bogus mapping from UTF-8 to SJIS conversion table. 0xc19c is not a valid UTF-8 byte sequence. It doesn't do any harm, AFAICS, but it's surely not intentional. No backpatching though, just to be sure. In the passing, also add a file header comment to the file, like the script would produce. (The file was originally created with, but has been modified by hand since then. That's questionable, but I'll leave fixing that for later.) Kyotaro Horiguchi Discussion: <>

Robert Haas pushed:

Correctifs en attente

Fabien COELHO sent in a patch to allow backslash continuations in \set expressions in pgbench.

Amit Langote sent in a patch to un-include access/heapam.h.

Michaël Paquier sent in another revision of a patch to rename pg_xlog to pg_wal and rename pg_clog to pg_transaction.

Anastasia Lubennikova sent in two more revisions of a patch to add covering + unique indexes.

Julien Rouhaud sent in another revision of a patch to add the parallel worker class and change max_worker_processes default to 16 and max_parallel_workers to 8.

Thomas Munro sent in another revision of a patch to add condition variables.

Thomas Munro sent in a patch to create a DSA area to provide work space for parallel execution.

Rushabh Lathia sent in a patch to implement Gather Merge.

Michaël Paquier sent in another revision of a patch to fix hot standby checkpoints.

Thomas Munro sent in two more revisions of a patch to add dynamic shared memory areas.

Tomas Vondra sent in another revision of a patch to add two slab-like memory allocators.

Takayuki Tsunakawa sent in another revision of a patch to close listen ports earlier in the shutdown process.

Thomas Munro sent in two revisions of a patch to use DSA memory so that hash tables can be shared by multiple backends and yet grow dynamically. Development name: "DHT".

Etsuro Fujita sent in three more revisions of a patch to fix invalidation for prepared statements with FDWs.

Amit Langote sent in three more revisions of a patch to implement declarative partitioning.

Fabien COELHO sent in another revision of a patch to add more operators and functions to pgbench.

Robert Haas sent in a patch to improve on pgrminclude / pgcompinclude.

Daniel Gustafsson sent in a WIP patch to add Secure Transport support as OpenSSL alternative on macOS.

Vitaly Burovoy sent in a patch to fix wording in the docs on parallel queries.

Franck Verrot sent in another revision of a patch to mntion column name in error messages.

Ashutosh Bapat sent in a patch to pass input_rel->relids to fetch_upper_rel() in create_grouping_path().

Heikki Linnakangas sent in a patch to always use 2048 bit DH parameters for OpenSSL ephemeral DH ciphers.

Dilip Kumar sent in a patch to implement parallel bitmap heap scan.

Pavan Deolasee sent in a patch to prevent certain types of FSM corruption.

Gilles Darold sent in another revision of a patch to implement a pg_current_logfile() function.

Peter Geoghegan sent in another revision of a patch to add parallel tuplesort for parallel B-Tree index creation.

Jeff Janes sent in a patch to add a pgbench transaction function.

Pavel Stěhule sent in a patch to add a parallel status to psql's \df+.

Pavel Stěhule sent in another revision of a patch to add \setfileref to psql.