Correctifs appliqués

Tom Lane pushed:

  • Refer to OS X as "macOS", except for the port name which is still "darwin". We weren't terribly consistent about whether to call Apple's OS "OS X" or "Mac OS X", and the former is probably confusing to people who aren't Apple users. Now that Apple has rebranded it "macOS", follow their lead to establish a consistent naming pattern. Also, avoid the use of the ancient project name "Darwin", except as the port code name which does not seem desirable to change. (In short, this patch touches documentation and comments, but no actual code.) I didn't touch contrib/start-scripts/osx/, either. I suspect those are obsolete and due for a rewrite, anyway. I dithered about whether to apply this edit to old release notes, but those were responsible for quite a lot of the inconsistencies, so I ended up changing them too. Anyway, Apple's being ahistorical about this, so why shouldn't we be?
  • Document has_type_privilege(). Evidently an oversight in commit 729205571. Back-patch to 9.2 where privileges for types were introduced. Report: <>
  • Replace the built-in GIN array opclasses with a single polymorphic opclass. We had thirty different GIN array opclasses sharing the same operators and support functions. That still didn't cover all the built-in types, nor did it cover arrays of extension-added types. What we want is a single polymorphic opclass for "anyarray". There were two missing features needed to make this possible: 1. We have to be able to declare the index storage type as ANYELEMENT when the opclass is declared to index ANYARRAY. This just takes a few more lines in index_create(). Although this currently seems of use only for GIN, there's no reason to make index_create() restrict it to that. 2. We have to be able to identify the proper GIN compare function for the index storage type. This patch proceeds by making the compare function optional in GIN opclass definitions, and specifying that the default btree comparison function for the index storage type will be looked up when the opclass omits it. Again, that seems pretty generically useful. Since the comparison function lookup is done in initGinState(), making use of the second feature adds an additional cache lookup to GIN index access setup. It seems unlikely that that would be very noticeable given the other costs involved, but maybe at some point we should consider making GinState data persist longer than it now does --- we could keep it in the index relcache entry, perhaps. Rather fortuitously, we don't seem to need to do anything to get this change to play nice with dump/reload or pg_upgrade scenarios: the new opclass definition is automatically selected to replace existing index definitions, and the on-disk data remains compatible. Also, if a user has created a custom opclass definition for a non-builtin type, this doesn't break that, since CREATE INDEX will prefer an exact match to opcintype over a match to ANYARRAY. However, if there's anyone out there with handwritten DDL that explicitly specifies _bool_ops or one of the other replaced opclass names, they'll need to adjust that. Tom Lane, reviewed by Enrique Meneses Discussion: <>
  • Fix newly-introduced issues in pgbench. The result of FD_ISSET() doesn't necessarily fit in a bool, though assigning it to one might accidentally work depending on platform and which socket FD number is being inquired of. Rewrite to test it with if(), rather than making any specific assumption about the result width, to match the way every other such call in PG is written. Don't break out of the input_mask-filling loop after finding the first client that we're waiting for results from. That mostly breaks parallel query management. Also, if we choose not to call select(), be sure to clear out any bits the mask-filling loop might have set, so that we don't accidentally call doCustom for clients we don't know have input. Doing so would likely be harmless, but it's a waste of cycles and doesn't seem to be intended. Make this_usec wide enough. (Yeah, the value would usually fit in an int, but then why are we using int64 everywhere else?) Minor cosmetic adjustments, mostly comment improvements. Problems introduced by commit 12788ae49. The first issue was discovered by buildfarm testing, the others by code review.
  • Improve contrib/cube's handling of zero-D cubes, infinities, and NaNs. It's always been possible to create a zero-dimensional cube by converting from a zero-length float8 array, but cube_in failed to accept the '()' representation that cube_out produced for that case, resulting in a dump/reload hazard. Make it accept the case. Also fix a couple of other places that didn't behave sanely for zero-dimensional cubes: cube_size would produce 1.0 when surely the answer should be 0.0, and g_cube_distance risked a divide-by-zero failure. Likewise, it's always been possible to create cubes containing float8 infinity or NaN coordinate values, but cube_in couldn't parse such input, and cube_out produced platform-dependent spellings of the values. Convert them to use float8in_internal and float8out_internal so that the behavior will be the same as for float8, as we recently did for the core geometric types (cf commit 50861cd68). As in that commit, I don't pretend that this patch fixes all insane corner-case behaviors that may exist for NaNs, but it's a step forward. (This change allows removal of the separate cube_1.out and cube_3.out expected-files, as the platform dependency that previously required them is now gone: an underflowing coordinate value will now produce an error not plus or minus zero.) Make errors from cube_in follow project conventions as to spelling ("invalid input syntax for cube" not "bad cube representation") and errcode (INVALID_TEXT_REPRESENTATION not SYNTAX_ERROR). Also a few marginal code cleanups and comment improvements. Tom Lane, reviewed by Amul Sul Discussion: <>
  • Redesign parallel dump/restore's wait-for-workers logic. The ListenToWorkers/ReapWorkerStatus APIs were messy and hard to use. Instead, make DispatchJobForTocEntry register a callback function that will take care of state cleanup, doing whatever had been done by the caller of ReapWorkerStatus in the old design. (This callback is essentially just the old mark_work_done function in the restore case, and a trivial test for worker failure in the dump case.) Then we can have ListenToWorkers call the callback immediately on receipt of a status message, and return the worker to WRKR_IDLE state; so the WRKR_FINISHED state goes away. This allows us to design a unified wait-for-worker-messages loop: WaitForWorkers replaces EnsureIdleWorker and EnsureWorkersFinished as well as the mess in restore_toc_entries_parallel. Also, we no longer need the fragile API spec that the caller of DispatchJobForTocEntry is responsible for ensuring there's an idle worker, since DispatchJobForTocEntry can just wait until there is one. In passing, I got rid of the ParallelArgs struct, which was a net negative in terms of notational verboseness, and didn't seem to be providing any noticeable amount of abstraction either. Tom Lane, reviewed by Kevin Grittner Discussion: <>
  • Rationalize parallel dump/restore's handling of worker cmd/status messages. The existing APIs for creating and parsing command and status messages are rather messy; for example, archive-format modules have to provide code for constructing command messages, which is entirely pointless since the code to read them is hard-wired in WaitForCommands() and hence no format-specific variation is actually possible. But there's little foreseeable reason to need format-specific variation anyway. The situation for status messages is no better; at least those are both constructed and parsed by format-specific code, but said code is quite redundant since there's no actual need for format-specific variation. To add insult to injury, the first API involves returning pointers to static buffers, which is bad, while the second involves returning pointers to malloc'd strings, which is safer but randomly inconsistent. Hence, get rid of the MasterStartParallelItem and MasterEndParallelItem APIs, and instead write centralized functions that construct and parse command and status messages. If we ever do need more flexibility, these functions can be the standard implementations of format-specific callback methods, but that's a long way off if it ever happens. Tom Lane, reviewed by Kevin Grittner Discussion: <>
  • Make struct ParallelSlot private within pg_dump/parallel.c. The only field of this struct that other files have any need to touch is the pointer to the TocEntry a worker is working on. (Well, pg_backup_archiver.c is actually looking at workerStatus too, but that can be finessed by specifying that the TocEntry pointer is NULL for a non-busy worker.) Hence, move out the TocEntry pointers to a separate array within struct ParallelState, and then we can make struct ParallelSlot private. I noted the possibility of this previously, but hadn't got round to actually doing it. Discussion: <>
  • Disallow pushing volatile quals past set-returning functions. Pushing an upper-level restriction clause into an unflattened subquery-in-FROM is okay when the subquery contains no SRFs in its targetlist, or when it does but the SRFs are unreferenced by the clause *and the clause is not volatile*. Otherwise, we're changing the number of times the clause is evaluated, which is bad for volatile quals, and possibly changing the result, since a volatile qual might succeed for some SRF output rows and not others despite not referencing any of the changing columns. (Indeed, if the clause is something like "random() > 0.5", the user is probably expecting exactly that behavior.) We had most of these restrictions down, but not the one about the upper clause not being volatile. Fix that, and add a regression test to illustrate the expected behavior. Although this is definitely a bug, it doesn't seem like back-patch material, since possibly some users don't realize that the broken behavior is broken and are relying on what happens now. Also, while the added test is quite cheap in the wake of commit a4c35ea1c, it would be much more expensive (or else messier) in older branches. Per report from Tom van Tilburg. Discussion: <>
  • Make to_timestamp() and to_date() range-check fields of their input. Historically, something like to_date('2009-06-40','YYYY-MM-DD') would return '2009-07-10' because there was no prohibition on out-of-range month or day numbers. This has been widely panned, and it also turns out that Oracle throws an error in such cases. Since these functions are nominally Oracle-compatibility features, let's change that. There's no particular restriction on year (modulo the fact that the scanner may not believe that more than 4 digits are year digits, a matter to be addressed separately if at all). But we now check month, day, hour, minute, second, and fractional-second fields, as well as day-of-year and second-of-day fields if those are used. Currently, no checks are made on ISO-8601-style week numbers or day numbers; it's not very clear what the appropriate rules would be there, and they're probably so little used that it's not worth sweating over. Artur Zakirov, reviewed by Amul Sul, further adjustments by me Discussion: <> See-Also: <>
  • Rationalize format-picture caching logic in formatting.c. Add a validity flag to DCHCacheEntry and NUMCacheEntry entries, and do not set it true until after we've parsed the supplied format string. This allows dealing with possible errors while parsing the format without the baroque hack that was there before (which only covered errors within NUMDesc_prepare, anyway). We can get rid of the PG_TRY in NUMDesc_prepare, as well as last_NUMCacheEntry and NUM_cache_remove. (Essentially, this reverts commit ff783fbae in favor of a less fragile solution; the problems with that approach are well illustrated by later hacking such as 55f927a46.) In passing, define the size of these caches as DCH_CACHE_ENTRIES not DCH_CACHE_FIELDS + 1 (whoever thought that was a good definition?) and likewise for the NUM cache. Also const-ify format string parameters where convenient, and merge duplicated cache lookup logic. This is primarily driven by a proposed patch from Artur Zakirov, which introduced some ereport's into format string parsing for the datetime case. He proposed preventing the creation of invalid cache entries by parsing the format string first into a local-variable array, and then copying that to a cache entry. That seemed a bit ugly to me, and anyway randomly different from the way the identical problem had been solved for the numeric case. Let's make the two sets of code more similar not less so. I'm not sure whether we'll adopt the new error conditions Artur proposes, but this patch seems like good code cleanup and future-proofing in any case. The existing code is critically (and undocumented-ly) dependent on no elog being thrown out of several nontrivial functions, which is trouble waiting to happen, though it doesn't seem to be actively broken today. Discussion: <>
  • Allow contrib/file_fdw to read from a program, like COPY FROM PROGRAM. This patch just exposes COPY's FROM PROGRAM option in contrib/file_fdw. There don't seem to be any security issues with that that are any worse than what already exist with file_fdw and COPY; as in the existing cases, only superusers are allowed to control what gets executed. A regression test case might be nice here, but choosing a 100% portable command to run is hard. (We haven't got a test for COPY FROM PROGRAM itself, either.) Corey Huinker and Adam Gomaa, reviewed by Amit Langote Discussion: <>
  • Fix multiple portability issues in pg_upgrade's rewriteVisibilityMap(). This is new code in 9.6, and evidently we missed out testing it as thoroughly as it should have been. Bugs fixed here: 1. Use binary not text mode to open the files on Windows. Before, if the visibility map chanced to contain two bytes that looked like \r\n, Windows' read() would convert that to \n, which both corrupts the map data and causes the file to look shorter than it should. Unless you were *very* unlucky and had an exact multiple of 8K such occurrences in each VM file, this would cause pg_upgrade to report a failure, though with a rather obscure error message. 2. The code for copying rebuilt bytes into the output was simply wrong. It chanced to work okay on little-endian machines but would emit the bytes in the wrong order on big-endian, leading to silent corruption of the visibility map data. 3. The code was careless about alignment of the working buffers. Given all three of an alignment-picky architecture, a compiler that chooses to put the new_vmbuf[] local variable at an odd starting address, and a checksum-enabled database, pg_upgrade would dump core. Point one was reported by Thomas Kellerer, the other two detected by code-reading. Point two is much the nastiest of these issues from an impact standpoint, though fortunately it affects only a minority of users. The Windows issue will definitely bite people, but it seems quite unlikely that there would be undetected corruption from that. In addition, I failed to resist the temptation to do some minor cosmetic adjustments, mostly improving the comments. It would be a good idea to try to improve the error reporting here, but that seems like material for a separate patch. -------
  • Improve error reporting in pg_upgrade's file copying/linking/rewriting. The previous design for this had copyFile(), linkFile(), and rewriteVisibilityMap() returning strerror strings, with the caller producing one-size-fits-all error messages based on that. This made it impossible to produce messages that described the failures with any degree of precision, especially not short-read problems since those don't set errno at all. Since pg_upgrade has no intention of continuing after any error in this area, let's fix this by just letting these functions call pg_fatal() for themselves, making it easy for each point of failure to have a suitable error message. Taking this approach also allows dropping cleanup code that was unnecessary and was often rather sloppy about preserving errno. To not lose relevant info that was reported before, pass in the schema name and table name of the current table so that they can be included in the error reports. An additional problem was the use of getErrorText(), which was flat out wrong for all but a couple of call sites, because it unconditionally did "_dosmaperr(GetLastError())" on Windows. That's only appropriate when reporting an error from a Windows-native API, which only a couple of the callers were actually doing. Thus, even the reported strerror string would be unrelated to the actual failure in many cases on Windows. To fix, get rid of getErrorText() altogether, and just have call sites do strerror(errno) instead, since that's the way all the rest of our frontend programs do it. Add back the _dosmaperr() calls in the two places where that's actually appropriate. In passing, make assorted messages hew more closely to project style guidelines, notably by removing initial capitals in not-complete-sentence primary error messages. (I didn't make any effort to clean up places I didn't have another reason to touch, though.) Per discussion of a report from Thomas Kellerer. Back-patch to 9.6, but no further; given the relative infrequency of reports of problems here, it's not clear it's worth adapting the patch to older branches. Patch by me, but with credit to Alvaro Herrera for spotting the issue with getErrorText's misuse of _dosmaperr(). Discussion: <nsjrbh$8li$>
  • Fix misplacement of submake-generated-headers prerequisites. The sequence "configure; cd src/pl/plpython; make -j" failed due to trying to compile plpython's .o files before the generated headers finished building. (This is an important real-world case, since it's the typical second step when building both plpython2 and plpython3.) This happens because the submake-generated-headers target is not placed in a way to make it a prerequisite to compiling the .o files. Fix that. Checking other uses of submake-generated-headers, I noted that the one attached to pg_regress was similarly misplaced; but it's actually not needed at all for pg_regress.o, rather regress.o, so move it to be a prerequisite of that. Back-patch to 9.6 where submake-generated-headers was introduced (by commit 548af97fc). It's not immediately clear to me why the previous coding didn't have the same issue; but since we've not had field reports of plpython make failing, leave it alone in the older branches. Pavel Raiskup and Tom Lane Discussion: <>
  • Fix misstatement in comment in Makefile.shlib. There is no need for "all: all-lib" to be placed before inclusion of Makefile.shlib. is what ensures that "all" is the default target, and we already document that that has to be included first. Per comment from Pavel Raiskup. Discussion: <>
  • Copy-editing for contrib/pg_visibility documentation. Add omitted names for some function parameters. Fix some minor grammatical issues.
  • Fix bugs in contrib/pg_visibility. collect_corrupt_items() failed to initialize tuple.t_self. While HeapTupleSatisfiesVacuum() doesn't actually use that value, it does Assert that it's valid, so that the code would dump core if ip_posid chanced to be zero. (That's somewhat unlikely, which probably explains how this got missed. In any case it wouldn't matter for field use.) Also, collect_corrupt_items was returning the wrong TIDs, that is the contents of t_ctid rather than the tuple's own location. This would be the same thing in simple cases, but it could be wrong if, for example, a past update attempt had been rolled back, leaving a live tuple whose t_ctid doesn't point at itself. Also, in pg_visibility(), guard against trying to read a page past the end of the rel. The VM code handles inquiries beyond the end of the map by silently returning zeroes, and it seems like we should do the same thing here. I ran into the assertion failure while using pg_visibility to check pg_upgrade's behavior, and then noted the other problems while reading the code. Report: <>
  • Do ClosePostmasterPorts() earlier in SubPostmasterMain(). In standard Unix builds, postmaster child processes do ClosePostmasterPorts immediately after InitPostmasterChild, that is almost immediately after being spawned. This is important because we don't want children holding open the postmaster's end of the postmaster death watch pipe. However, in EXEC_BACKEND builds, SubPostmasterMain was postponing this responsibility significantly, in order to make it slightly more convenient to pass the right flag value to ClosePostmasterPorts. This is bad, particularly seeing that process_shared_preload_libraries() might invoke nearly-arbitrary code. Rearrange so that we do it as soon as we've fetched the socket FDs via read_backend_variables(). Also move the comment explaining about randomize_va_space to before the call of PGSharedMemoryReAttach, which is where it's relevant. The old placement was appropriate when the reattach happened inside CreateSharedMemoryAndSemaphores, but that was a long time ago. Back-patch to 9.3; the patch doesn't apply cleanly before that, and it doesn't seem worth a lot of effort given that we've had no actual field complaints traceable to this. Discussion: <>
  • Avoid leaking FDs after an fsync failure. Fixes errors introduced in commit bc34223bc, as detected by Coverity. In passing, report ENOSPC for a short write while padding a new wal file in open_walfile, make certain that close_walfile closes walfile in all cases, and improve a couple of comments. Michael Paquier and Tom Lane
  • Add ALTER EXTENSION ADD/DROP ACCESS METHOD, and use it in pg_upgrade. Without this, an extension containing an access method is not properly dumped/restored during pg_upgrade --- the AM ends up not being a member of the extension after upgrading. Another oversight in commit 473b93287, reported by Andrew Dunstan. Report: <>

Peter Eisentraut pushed:

Ãlvaro Herrera pushed:

Heikki Linnakangas pushed:

  • Refactor script execution state machine in pgbench. The doCustom() function had grown into quite a mess. Rewrite it, in a more explicit state machine style, for readability. This also fixes one minor bug: if a script consisted entirely of meta commands, doCustom() never returned to the caller, so progress reports with the -P option were not printed. I don't want to backpatch this refactoring, and the bug is quite insignificant, so only commit this to master, and leave the bug unfixed in back-branches. Review and original bug report by Fabien Coelho. Discussion: <alpine.DEB.2.20.1607090850120.3412@sto>
  • Turn password_encryption GUC into an enum. This makes the parameter easier to extend, to support other password-based authentication protocols than MD5. (SCRAM is being worked on.) The GUC still accepts on/off as aliases for "md5" and "plain", although we may want to remove those once we actually add support for another password hash type. Michael Paquier, reviewed by David Steele, with some further edits by me. Discussion: <>
  • Don't bother to lock bufmgr partitions in pg_buffercache. That makes the view a lot less disruptive to use on a production system. Without the locks, you don't get a consistent snapshot across all buffers, but that's OK. It wasn't a very useful guarantee in practice. Ivan Kartyshov, reviewed by Tomas Vondra and Robert Haas. Discusssion: <>

Robert Haas pushed:

Stephen Frost pushed:

  • Remove superuser checks in pgstattuple. Now that we track initial privileges on extension objects and changes to those permissions, we can drop the superuser() checks from the various functions which are part of the pgstattuple extension and rely on the GRANT system to control access to those functions. Since a pg_upgrade will preserve the version of the extension which existed prior to the upgrade, we can't simply modify the existing functions but instead need to create new functions which remove the checks and update the SQL-level functions to use the new functions (and to REVOKE EXECUTE rights on those functions from PUBLIC). Thanks to Tom and Andres for adding support for extensions to follow update paths (see: 40b449a), allowing this patch to be much smaller since no new base version script needed to be included. Approach suggested by Noah. Reviewed by Michael Paquier.

Magnus Hagander pushed:

Correctifs en attente

Takayuki Tsunakawa sent in two revisions of a patch to implement huge_pages on Windows.

Michaël Paquier and Heikki Linnakangas traded patch flocks to implement SCRAM password auth.

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

Pavel Stěhule sent in another revision of a patch to add xmltable().

Vitaly Burovoy sent in a patch to detect supported SET parameters when pg_restore is run.

Vinayak Pokale support prepared transactions with foreign servers.

Peter Eisentraut and Michaël Paquier traded patches to fix CRC check handling in get_controlfile.

Stephen Frost and Jeevan Chalke traded patches to add support for restrictive RLS policies.

Jesper Pedersen and Peter Eisentraut traded patches to add hash index support to pageinspect.

Kyotaro HORIGUCHI and Michaël Paquier traded patches to track LSN on lightly loaded systems to reduce the number of checkpoints.

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

Fabien COELHO sent in three more revisions of a patch to add more operators and functions to pgbench.

Fabien COELHO sent in another revision of a patch to enable storing select results into variables.

Rajkumar Raghuwanshi sent in a patch to add test cases for partition-wise joins on declarative partitions.

Daniel Gustafsson sent in a patch to make flex/bison checks stricter in Git trees.

Mark Dilger sent in a patch to stop casting away const in comparators.

Mark Dilger sent in a patch to stop casting away const in regex.

Mark Dilger sent in a patch to avoid casting away const when using the FileName typedef.

Takayuki Tsunakawa sent in a patch to allow SIGQUIT to prompt a fast shutdown of the stats collector without leaving partly-written stats files behind.

Mark Dilger sent in a WIP patch to const-ify xlog_outdesc() and rm_redo_error_callback().

David Steele and Michaël Paquier traded patches to exclude additional directories in pg_basebackup.

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

Michaël Paquier sent in four more revisions of a patch to track wait event for latches.

Victor Wagner sent in two more revisions of a patch to implement failover at the libpq connect level.

Mark Dilger sent in a patch to tidy up some handling of pointers in rangetypes_spgist.c, tsgistidx.c, and tsrank.c.

David Cramer and Heikki Linnakangas traded patches to add support for multi-dimensional arrays to PL/PythonU.

Mithun Cy sent in another revision of a patch to cache hash index meta pages.

Emre Hasegeli sent in another revision of a patch to fix floating point comparison inconsistencies in the geometric types.

Ivan Kartyshov sent in another revision of a patch to make pg_buffercache more efficient with large shared memory.

Etsuro Fujita sent in two more revisions of a patch to push down more full joins to the PostgreSQL FDW.

David Fetter and Thomas Munro traded patches to add a hook and corresponding GUCs to disable UPDATEs and DELETEs that lack a WHERE clause.

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

Heikki Linnakangas sent in two more revisions of a patch to change the way use memory for larger read buffers in logtape.c rather than pre-reading tuples into SortTuple slots during merge.

Magnus Hagander sent in two more revisions of a patch to enable pg_basebackup to stream xlog to tar.

Kyotaro HORIGUCHI sent in another revision of a patch to fix wal level minimal.

Ashutosh Bapat sent in another revision of a patch to fix an issue where altering foreign table was not invalidating prepare statement execution plans that it should have.

Jeevan Chalke sent in another revision of a patch to enable pushing aggregates to a FDW.

Aleksander Alekseev sent in a patch to rename md5Salt to pwsalt.

Corey Huinker sent in a PoC patch to make a copy() set-returning function.

Stephen Frost sent in another revision of a patch to fix some infelicities between COPY and RLS.

Andres Freund sent in another revision of a patch to add a macro-customizable hashtable for bitmapscan and aggregation performance.

Daniel Vérité sent in another revision of a patch to fix some pg_dump / copy bugs with long lines.

Dmitry Dolgov sent in another revision of a patch to add a generic type subscription.

Emre Hasegeli sent in a patch to add <@, @>, <<@, and @>> operator symbols for the inet datatype to replace <<=, >>=, <<, and >>.

Christoph Berg sent in a patch to ensure that the default log_line prefix is not empty.