Skip to main content

Browse the archive

https://github.com/postgres/postgres
07 April 2024, 13:56:23 UTC
sort by:
Revision Author Date Message Commit Date
0b09804 Tom Lane 05 November 2018, 21:49:29 UTC Stamp 9.5.15. 05 November 2018, 21:49:29 UTC
a0e450e Andres Freund 05 November 2018, 20:02:25 UTC Fix copy-paste error in errhint() introduced in 691d79a07933. Reported-By: Petr Jelinek Discussion: https://postgr.es/m/c95a620b-34f0-7930-aeb5-f7ab804f26cb@2ndquadrant.com Backpatch: 9.4-, like the previous commit 05 November 2018, 20:05:40 UTC
9096bf3 Peter Eisentraut 05 November 2018, 14:10:54 UTC Translation updates Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git Source-Git-Hash: fa2a38c6d1759866a6840952dd2fbd71b9a69955 05 November 2018, 14:10:54 UTC
1d00f27 Tom Lane 04 November 2018, 21:57:14 UTC Release notes for 11.1, 10.6, 9.6.11, 9.5.15, 9.4.20, 9.3.25. 04 November 2018, 21:57:14 UTC
6e60929 Tom Lane 03 November 2018, 17:56:10 UTC Make ts_locale.c's character-type functions cope with UTF-16. On Windows, in UTF8 database encoding, what char2wchar() produces is UTF16 not UTF32, ie, characters above U+FFFF will be represented by surrogate pairs. t_isdigit() and siblings did not account for this and failed to provide a large enough result buffer. That in turn led to bogus "invalid multibyte character for locale" errors, because contrary to what you might think from char2wchar()'s documentation, its Windows code path doesn't cope sanely with buffer overflow. The solution for t_isdigit() and siblings is pretty clear: provide a 3-wchar_t result buffer not 2. char2wchar() also needs some work to provide more consistent, and more accurately documented, buffer overrun behavior. But that's a bigger job and it doesn't actually have any immediate payoff, so leave it for later. Per bug #15476 from Kenji Uno, who deserves credit for identifying the cause of the problem. Back-patch to all active branches. Discussion: https://postgr.es/m/15476-4314f480acf0f114@postgresql.org 03 November 2018, 17:56:10 UTC
94ea1cf Tom Lane 02 November 2018, 22:54:00 UTC Yet further rethinking of build changes for macOS Mojave. The solution arrived at in commit e74dd00f5 presumes that the compiler has a suitable default -isysroot setting ... but further experience shows that in many combinations of macOS version, XCode version, Xcode command line tools version, and phase of the moon, Apple's compiler will *not* supply a default -isysroot value. We could potentially go back to the approach used in commit 68fc227dd, but I don't have a lot of faith in the reliability or life expectancy of that either. Let's just revert to the approach already shipped in 11.0, namely specifying an -isysroot switch globally. As a partial response to the concerns raised by Jakob Egger, adjust the contents of Makefile.global to look like CPPFLAGS = -isysroot $(PG_SYSROOT) ... PG_SYSROOT = /path/to/sysroot This allows overriding the sysroot path at build time in a relatively painless way. Add documentation to installation.sgml about how to use the PG_SYSROOT option. I also took the opportunity to document how to work around macOS's "System Integrity Protection" feature. As before, back-patch to all supported versions. Discussion: https://postgr.es/m/20840.1537850987@sss.pgh.pa.us 02 November 2018, 22:54:00 UTC
d403ed2 Bruce Momjian 02 November 2018, 17:05:30 UTC docs: adjust simpler language for NULL return from ANY/ALL Adjustment to commit 8610c973ddf1cbf0befc1369d2cf0d56c0efcd0a. Reported-by: Tom Lane Discussion: https://postgr.es/m/17406.1541168421@sss.pgh.pa.us Backpatch-through: 9.3 02 November 2018, 17:05:30 UTC
75f1abf Bruce Momjian 02 November 2018, 13:10:59 UTC GUC: adjust effective_cache_size docs and SQL description Clarify that effective_cache_size is both kernel buffers and shared buffers. Reported-by: nat@makarevitch.org Discussion: https://postgr.es/m/153685164808.22334.15432535018443165207@wrigleys.postgresql.org Backpatch-through: 9.3 02 November 2018, 13:10:59 UTC
51afe76 Magnus Hagander 02 November 2018, 12:55:57 UTC Fix some spelling errors in the documentation Author: Daniel Gustafsson <daniel@yesql.se> 02 November 2018, 12:59:10 UTC
eff71cd Bruce Momjian 02 November 2018, 12:54:33 UTC doc: use simpler language for NULL return from ANY/ALL Previously the combination of "does not return" and "any row" caused ambiguity. Reported-by: KES <kes-kes@yandex.ru> Discussion: https://postgr.es/m/153701242703.22334.1476830122267077397@wrigleys.postgresql.org Reviewed-by: David G. Johnston Backpatch-through: 9.3 02 November 2018, 12:54:33 UTC
d554e33 Andres Freund 01 November 2018, 17:43:54 UTC Fix error message typo introduced 691d79a07933. Reported-By: Michael Paquier Discussion: https://postgr.es/m/20181101003405.GB1727@paquier.xyz Backpatch: 9.4-, like the previous commit 01 November 2018, 17:43:54 UTC
679cb44 Andres Freund 31 October 2018, 21:47:41 UTC Disallow starting server with insufficient wal_level for existing slot. Previously it was possible to create a slot, change wal_level, and restart, even if the new wal_level was insufficient for the slot. That's a problem for both logical and physical slots, because the necessary WAL records are not generated. This removes a few tests in newer versions that, somewhat inexplicably, whether restarting with a too low wal_level worked (a buggy behaviour!). Reported-By: Joshua D. Drake Author: Andres Freund Discussion: https://postgr.es/m/20181029191304.lbsmhshkyymhw22w@alap3.anarazel.de Backpatch: 9.4-, where replication slots where introduced 31 October 2018, 22:46:40 UTC
156a737 Tom Lane 31 October 2018, 21:04:43 UTC Fix memory leak in repeated SPGIST index scans. spgendscan neglected to pfree all the memory allocated by spgbeginscan. It's possible to get away with that in most normal queries, since the memory is allocated in the executor's per-query context which is about to get deleted anyway; but it causes severe memory leakage during creation or filling of large exclusion-constraint indexes. Also, document that amendscan is supposed to free what ambeginscan allocates. The docs' lack of clarity on that point probably caused this bug to begin with. (There is discussion of changing that API spec going forward, but I don't think it'd be appropriate for the back branches.) Per report from Bruno Wolff. It's been like this since the beginning, so back-patch to all active branches. In HEAD, also fix an independent leak caused by commit 2a6368343 (allocating memory during spgrescan instead of spgbeginscan, which might be all right if it got cleaned up, but it didn't). And do a bit of code beautification on that commit, too. Discussion: https://postgr.es/m/20181024012314.GA27428@wolff.to 31 October 2018, 21:04:43 UTC
5cdb330 Tom Lane 31 October 2018, 13:47:53 UTC Sync our copy of the timezone library with IANA release tzcode2018g. This patch absorbs an upstream fix to "zic" for a recently-introduced bug that made it output data that some 32-bit clients couldn't read. Given the current source data, the bug only manifests in zones with leap seconds, which we don't generate, so that there's no actual change in our installed timezone data files from this. Still, in case somebody uses our copy of "zic" to do something else, it seems best to apply the fix promptly. Also, update the README's notes about converting upstream code to our conventions. 31 October 2018, 13:48:19 UTC
811d8cb Tom Lane 31 October 2018, 12:35:50 UTC Update time zone data files to tzdata release 2018g. DST law changes in Morocco (with, effectively, zero notice). Historical corrections for Hawaii. 31 October 2018, 12:36:29 UTC
ba5644d Magnus Hagander 29 October 2018, 11:34:49 UTC Fix missing whitespace in pg_dump ref page Author: Daniel Gustafsson <daniel@yesql.se> 29 October 2018, 11:36:27 UTC
ba103dc Andrew Dunstan 28 October 2018, 16:22:32 UTC Fix perl searchpath for modern perl for MSVC tools Modern versions of perl no longer include the current directory in the perl searchpath, as it's insecure. Instead of adding the current directory, we get around the problem by adding the directory where the script lives. Problem noted by Victor Wagner. Solution adapted from buildfarm client code. Backpatch to all live versions. 28 October 2018, 16:26:05 UTC
cc02db8 Andrew Dunstan 20 October 2018, 13:02:36 UTC Lower privilege level of programs calling regression_main On Windows this mean that the regression tests can now safely and successfully run as Administrator, which is useful in situations like Appveyor. Elsewhere it's a no-op. Backpatch to 9.5 - this is harder in earlier branches and not worth the trouble. Discussion: https://postgr.es/m/650b0c29-9578-8571-b1d2-550d7f89f307@2ndQuadrant.com 20 October 2018, 13:11:18 UTC
ac3be11 Tom Lane 20 October 2018, 02:22:57 UTC Client-side fixes for delayed NOTIFY receipt. PQnotifies() is defined to just process already-read data, not try to read any more from the socket. (This is a debatable decision, perhaps, but I'm hesitant to change longstanding library behavior.) The documentation has long recommended calling PQconsumeInput() before PQnotifies() to ensure that any already-arrived message would get absorbed and processed. However, psql did not get that memo, which explains why it's not very reliable about reporting notifications promptly. Also, most (not quite all) callers called PQconsumeInput() just once before a PQnotifies() loop. Taking this recommendation seriously implies that we should do PQconsumeInput() before each call. This is more important now that we have "payload" strings in notification messages than it was before; that increases the probability of having more than one packet's worth of notify messages. Hence, adjust code as well as documentation examples to do it like that. Back-patch to 9.5 to match related server fixes. In principle we could probably go back further with these changes, but given lack of field complaints I doubt it's worthwhile. Discussion: https://postgr.es/m/CAOYf6ec-TmRYjKBXLLaGaB-jrd=mjG1Hzn1a1wufUAR39PQYhw@mail.gmail.com 20 October 2018, 02:22:57 UTC
f494166 Tom Lane 20 October 2018, 01:39:22 UTC Server-side fix for delayed NOTIFY and SIGTERM processing. Commit 4f85fde8e introduced some code that was meant to ensure that we'd process cancel, die, sinval catchup, and notify interrupts while waiting for client input. But there was a flaw: it supposed that the process latch would be set upon arrival at secure_read() if any such interrupt was pending. In reality, we might well have cleared the process latch at some earlier point while those flags remained set -- particularly notifyInterruptPending, which can't be handled as long as we're within a transaction. To fix the NOTIFY case, also attempt to process signals (except ProcDiePending) before trying to read. Also, if we see that ProcDiePending is set before we read, forcibly set the process latch to ensure that we will handle that signal promptly if no data is available. I also made it set the process latch on the way out, in case there is similar logic elsewhere. (It remains true that we won't service ProcDiePending here unless we need to wait for input.) The code for handling ProcDiePending during a write needs those changes, too. Also be a little more careful about when to reset whereToSendOutput, and improve related comments. Back-patch to 9.5 where this code was added. I'm not entirely convinced that older branches don't have similar issues, but the complaint at hand is just about the >= 9.5 code. Jeff Janes and Tom Lane Discussion: https://postgr.es/m/CAOYf6ec-TmRYjKBXLLaGaB-jrd=mjG1Hzn1a1wufUAR39PQYhw@mail.gmail.com 20 October 2018, 01:39:22 UTC
eda69c6 Tom Lane 19 October 2018, 23:36:34 UTC Sync our copy of the timezone library with IANA release tzcode2018f. About half of this is purely cosmetic changes to reduce the diff between our code and theirs, like inserting "const" markers where they have them. The other half is tracking actual code changes in zic.c and localtime.c. I don't think any of these represent near-term compatibility hazards, but it seems best to stay up to date. I also fixed longstanding bugs in our code for producing the known_abbrevs.txt list, which by chance hadn't been exposed before, but which resulted in some garbage output after applying the upstream changes in zic.c. Notably, because upstream removed their old phony transitions at the Big Bang, it's now necessary to cope with TZif files containing no DST transition times at all. 19 October 2018, 23:36:34 UTC
5617060 Tom Lane 19 October 2018, 21:01:34 UTC Update time zone data files to tzdata release 2018f. DST law changes in Chile, Fiji, and Russia (Volgograd). Historical corrections for China, Japan, Macau, and North Korea. Note: like the previous tzdata update, this involves a depressingly large amount of semantically-meaningless churn in tzdata.zi. That is a consequence of upstream's data compression method assigning unstable abbreviations to DST rulesets. I complained about that to them last time, and this version now uses an assignment method that pays some heed to not changing abbreviations unnecessarily. So hopefully, that'll be better going forward. 19 October 2018, 21:02:12 UTC
021b355 Tom Lane 18 October 2018, 18:55:23 UTC Still further rethinking of build changes for macOS Mojave. To avoid the sorts of problems complained of by Jakob Egger, it'd be best if configure didn't emit any references to the sysroot path at all. In the case of PL/Tcl, we can do that just by keeping our hands off the TCL_INCLUDE_SPEC string altogether. In the case of PL/Perl, we need to substitute -iwithsysroot for -I in the compile commands, which is easily handled if we change to using a configure output variable that includes the switch not only the directory name. Since PL/Tcl and PL/Python already do it like that, this seems like good consistency cleanup anyway. Hence, this replaces the advice given to Perl-related extensions in commit 5e2217131; instead of writing "-I$(perl_archlibexp)/CORE", they should just write "$(perl_includespec)". (The old way continues to work, but not on recent macOS.) It's still the case that configure needs to be aware of the sysroot path internally, but that's cleaner than what we had before. As before, back-patch to all supported versions. Discussion: https://postgr.es/m/20840.1537850987@sss.pgh.pa.us 18 October 2018, 18:55:23 UTC
7a1e7b2 Tom Lane 17 October 2018, 19:06:38 UTC Fix minor bug in isolationtester. If the lock wait query failed, isolationtester would report the PQerrorMessage from some other connection, meaning there would be no message or an unrelated one. This seems like a pretty unlikely occurrence, but if it did happen, this bug could make it really difficult/confusing to figure out what happened. That seems to justify patching all the way back. In passing, clean up another place where the "wrong" conn was used for an error report. That one's not actually buggy because it's a different alias for the same connection, but it's still confusing to the reader. 17 October 2018, 19:06:38 UTC
fb38d9a Tom Lane 17 October 2018, 16:26:48 UTC Improve tzparse's handling of TZDEFRULES ("posixrules") zone data. In the IANA timezone code, tzparse() always tries to load the zone file named by TZDEFRULES ("posixrules"). Previously, we'd hacked that logic to skip the load in the "lastditch" code path, which we use only to initialize the default "GMT" zone during GUC initialization. That's critical for a couple of reasons: since we do not support leap seconds, we *must not* allow "GMT" to have leap seconds, and since this case runs before the GUC subsystem is fully alive, we'd really rather not take the risk of pg_open_tzfile throwing any errors. However, that still left the code reading TZDEFRULES on every other call, something we'd noticed to the extent of having added code to cache the result so it was only done once per process not a lot of times. Andres Freund complained about the static data space used up for the cache; but as long as the logic was like this, there was no point in trying to get rid of that space. We can improve matters by looking a bit more closely at what the IANA code actually needs the TZDEFRULES data for. One thing it does is that if "posixrules" is a leap-second-aware zone, the leap-second behavior will be absorbed into every POSIX-style zone specification. However, that's a behavior we'd really prefer to do without, since for our purposes the end effect is to render every POSIX-style zone name unsupported. Otherwise, the TZDEFRULES data is used only if the POSIX zone name specifies DST but doesn't include a transition date rule (e.g., "EST5EDT" rather than "EST5EDT,M3.2.0,M11.1.0"). That is a minority case for our purposes --- in particular, it never happens when tzload() invokes tzparse() to interpret a transition date rule string found in a tzdata zone file. Hence, if we legislate that we're going to ignore leap-second data from "posixrules", we can postpone the TZDEFRULES load into the path where we actually need to substitute for a missing date rule string. That means it will never happen at all in common scenarios, making it reasonable to dynamically allocate the cache space when it does happen. Even when the data is already loaded, this saves some cycles in the common code path since we avoid a memcpy of 23KB or so. And, IMO at least, this is a less ugly hack on the IANA logic than what we had before, since it's not messing with the lastditch-vs-regular code paths. Back-patch to all supported branches, not so much because this is a critical change as that I want to keep all our copies of the IANA timezone code in sync. Discussion: https://postgr.es/m/20181015200754.7y7zfuzsoux2c4ya@alap3.anarazel.de 17 October 2018, 16:26:48 UTC
d0ab588 Tom Lane 16 October 2018, 20:27:15 UTC Back off using -isysroot on Darwin. Rethink the solution applied in commit 5e2217131 to get PL/Tcl to build on macOS Mojave. I feared that adding -isysroot globally might have undesirable consequences, and sure enough Jakob Egger reported one: it complicates building extensions with a different Xcode version than was used for the core server. (I find that a risky proposition in general, but apparently it works most of the time, so we shouldn't break it if we don't have to.) We'd already adopted the solution for PL/Perl of inserting the sysroot path directly into the -I switches used to find Perl's headers, and we can do the same thing for PL/Tcl by changing the -iwithsysroot switch that Apple's tclConfig.sh reports. This restricts the risks to PL/Perl and PL/Tcl themselves and directly-dependent extensions, which is a lot more pleasing in general than a global -isysroot switch. Along the way, tighten the test to see if we need to inject the sysroot path into $perl_includedir, as I'd speculated about upthread but not gotten round to doing. As before, back-patch to all supported versions. Discussion: https://postgr.es/m/20840.1537850987@sss.pgh.pa.us 16 October 2018, 20:27:15 UTC
7249d1d Tom Lane 16 October 2018, 17:56:58 UTC Avoid rare race condition in privileges.sql regression test. We created a temp table, then switched to a new session, leaving the old session to clean up its temp objects in background. If that took long enough, the eventual attempt to drop the user that owns the temp table could fail, as exhibited today by sidewinder. Fix by dropping the temp table explicitly when we're done with it. It's been like this for quite some time, so back-patch to all supported branches. Report: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=sidewinder&dt=2018-10-16%2014%3A45%3A00 16 October 2018, 17:56:58 UTC
94ccffb Tom Lane 16 October 2018, 16:32:28 UTC Make PostgresNode.pm's poll_query_until() more chatty about failures. Reporting only the stderr is unhelpful when the problem is that the server output we're getting doesn't match what was expected. So we should report the query output too; and just for good measure, let's print the query we used and the output we expected. Back-patch to 9.5 where poll_query_until was introduced. Discussion: https://postgr.es/m/17913.1539634756@sss.pgh.pa.us 16 October 2018, 16:32:28 UTC
4f6a77e Tom Lane 16 October 2018, 16:01:19 UTC Improve stability of recently-added regression test case. Commit b5febc1d1 added a contrib/btree_gist test case that has been observed to fail in the buildfarm as a result of background auto-analyze updating stats and changing the selected plan. Forestall that by forcibly analyzing in foreground, instead. The new plan choice is just as good for our purposes, since we really only care that an index-only plan does not get selected. Back-patch to 9.5, like the previous patch. Discussion: https://postgr.es/m/14643.1539629304@sss.pgh.pa.us 16 October 2018, 16:01:19 UTC
18c7254 Tom Lane 16 October 2018, 15:50:19 UTC Avoid statically allocating gmtsub()'s timezone workspace. localtime.c's "struct state" is a rather large object, ~23KB. We were statically allocating one for gmtsub() to use to represent the GMT timezone, even though that function is not at all heavily used and is never reached in most backends. Let's malloc it on-demand, instead. This does pose the question of how to handle a malloc failure, but there's already a well-defined error report convention here, ie set errno and return NULL. We have but one caller of pg_gmtime in HEAD, and two in back branches, neither of which were troubling to check for error. Make them do so. The possible errors are sufficiently unlikely (out-of-range timestamp, and now malloc failure) that I think elog() is adequate. Back-patch to all supported branches to keep our copies of the IANA timezone code in sync. This particular change is in a stanza that already differs from upstream, so it's a wash for maintenance purposes --- but only as long as we keep the branches the same. Discussion: https://postgr.es/m/20181015200754.7y7zfuzsoux2c4ya@alap3.anarazel.de 16 October 2018, 15:50:19 UTC
10412ce Tom Lane 15 October 2018, 18:01:38 UTC Check for stack overrun in standard_ProcessUtility(). ProcessUtility can recurse, and indeed can be driven to infinite recursion, so it ought to have a check_stack_depth() call. This covers the reported bug (portal trying to execute itself) and a bunch of other cases that could perhaps arise somewhere. Per bug #15428 from Malthe Borch. Back-patch to all supported branches. Discussion: https://postgr.es/m/15428-b3c2915ec470b033@postgresql.org 15 October 2018, 18:01:38 UTC
d83dac3 Michael Paquier 14 October 2018, 13:23:48 UTC Avoid duplicate XIDs at recovery when building initial snapshot On a primary, sets of XLOG_RUNNING_XACTS records are generated on a periodic basis to allow recovery to build the initial state of transactions for a hot standby. The set of transaction IDs is created by scanning all the entries in ProcArray. However it happens that its logic never counted on the fact that two-phase transactions finishing to prepare can put ProcArray in a state where there are two entries with the same transaction ID, one for the initial transaction which gets cleared when prepare finishes, and a second, dummy, entry to track that the transaction is still running after prepare finishes. This way ensures a continuous presence of the transaction so as callers of for example TransactionIdIsInProgress() are always able to see it as alive. So, if a XLOG_RUNNING_XACTS takes a standby snapshot while a two-phase transaction finishes to prepare, the record can finish with duplicated XIDs, which is a state expected by design. If this record gets applied on a standby to initial its recovery state, then it would simply fail, so the odds of facing this failure are very low in practice. It would be tempting to change the generation of XLOG_RUNNING_XACTS so as duplicates are removed on the source, but this requires to hold on ProcArrayLock for longer and this would impact all workloads, particularly those using heavily two-phase transactions. XLOG_RUNNING_XACTS is also actually used only to initialize the standby state at recovery, so instead the solution is taken to discard duplicates when applying the initial snapshot. Diagnosed-by: Konstantin Knizhnik Author: Michael Paquier Discussion: https://postgr.es/m/0c96b653-4696-d4b4-6b5d-78143175d113@postgrespro.ru Backpatch-through: 9.3 14 October 2018, 13:23:48 UTC
43cc4e4 Tom Lane 12 October 2018, 23:33:56 UTC Remove abstime, reltime, tinterval tables from old regression databases. In the back branches, drop these tables after the regression tests are done with them. This fixes failures of cross-branch pg_upgrade testing caused by these types having been removed in v12. We do lose the ability to test dump/restore behavior with these types in the back branches, but the actual loss of code coverage seems to be nil given that there's nothing very special about these types. Discussion: https://postgr.es/m/20181009192237.34wjp3nmw7oynmmr@alap3.anarazel.de 12 October 2018, 23:33:56 UTC
1245561 Tom Lane 12 October 2018, 18:49:33 UTC Back-patch addition of the ALLOCSET_FOO_SIZES macros. These macros were originally added in commit ea268cdc9, and back-patched into 9.6 before 9.6.0. However, some extensions would like to use them in older branches, and there seems no harm in providing them. So add them to all supported branches. Per suggestions from Christoph Berg and Andres Freund. Discussion: https://postgr.es/m/20181012170355.bhxi273skjt6sag4@alap3.anarazel.de 12 October 2018, 18:49:33 UTC
0a0c255 Andres Freund 10 October 2018, 20:53:03 UTC Fix logical decoding error when system table w/ toast is repeatedly rewritten. Repeatedly rewriting a mapped catalog table with VACUUM FULL or CLUSTER could cause logical decoding to fail with: ERROR, "could not map filenode \"%s\" to relation OID" To trigger the problem the rewritten catalog had to have live tuples with toasted columns. The problem was triggered as during catalog table rewrites the heap_insert() check that prevents logical decoding information to be emitted for system catalogs, failed to treat the new heap's toast table as a system catalog (because the new heap is not recognized as a catalog table via RelationIsLogicallyLogged()). The relmapper, in contrast to the normal catalog contents, does not contain historical information. After a single rewrite of a mapped table the new relation is known to the relmapper, but if the table is rewritten twice before logical decoding occurs, the relfilenode cannot be mapped to a relation anymore. Which then leads us to error out. This only happens for toast tables, because the main table contents aren't re-inserted with heap_insert(). The fix is simple, add a new heap_insert() flag that prevents logical decoding information from being emitted, and accept during decoding that there might not be tuple data for toast tables. Unfortunately that does not fix pre-existing logical decoding errors. Doing so would require not throwing an error when a filenode cannot be mapped to a relation during decoding, and that seems too likely to hide bugs. If it's crucial to fix decoding for an existing slot, temporarily changing the ERROR in ReorderBufferCommit() to a WARNING appears to be the best fix. Author: Andres Freund Discussion: https://postgr.es/m/20180914021046.oi7dm4ra3ot2g2kt@alap3.anarazel.de Backpatch: 9.4-, where logical decoding was introduced 10 October 2018, 20:53:03 UTC
fd66ca0 Alvaro Herrera 08 October 2018, 13:37:21 UTC Silence compiler warning in Assert() gcc 6.3 does not whine about this mistake I made in 39808e8868c8 but evidently lots of other compilers do, according to Michael Paquier, Peter Eisentraut, Arthur Zakirov, Tomas Vondra. Discussion: too many to list 08 October 2018, 13:37:21 UTC
a2a5159 Alvaro Herrera 06 October 2018, 22:17:46 UTC Fix event triggers for partitioned tables Index DDL cascading on partitioned tables introduced a way for ALTER TABLE to be called reentrantly. This caused an an important deficiency in event trigger support to be exposed: on exiting the reentrant call, the alter table state object was clobbered, causing a crash when the outer alter table tries to finalize its processing. Fix the crash by creating a stack of event trigger state objects. There are still ways to cause things to misbehave (and probably other crashers) with more elaborate tricks, but at least it now doesn't crash in the obvious scenario. Backpatch to 9.5, where DDL deparsing of event triggers was introduced. Reported-by: Marco Slot Authors: Michaël Paquier, Álvaro Herrera Discussion: https://postgr.es/m/CANNhMLCpi+HQ7M36uPfGbJZEQLyTy7XvX=5EFkpR-b1bo0uJew@mail.gmail.com 06 October 2018, 22:17:46 UTC
3c9dd96 Tom Lane 06 October 2018, 16:00:10 UTC Propagate xactStartTimestamp and stmtStartTimestamp to parallel workers. Previously, a worker process would establish values for these based on its own start time. In v10 and up, this can trivially be shown to cause misbehavior of transaction_timestamp(), timestamp_in(), and related functions which are (perhaps unwisely?) marked parallel-safe. It seems likely that other behaviors might diverge from what happens in the parent as well. It's not as trivial to demonstrate problems in 9.6 or 9.5, but I'm sure it's still possible, so back-patch to all branches containing parallel worker infrastructure. In HEAD only, mark now() and statement_timestamp() as parallel-safe (other affected functions already were). While in theory we could still squeeze that change into v11, it doesn't seem important enough to force a last-minute catversion bump. Konstantin Knizhnik, whacked around a bit by me Discussion: https://postgr.es/m/6406dbd2-5d37-4cb6-6eb2-9c44172c7e7c@postgrespro.ru 06 October 2018, 16:00:10 UTC
0dc6bf6 Tom Lane 05 October 2018, 20:01:30 UTC Allow btree comparison functions to return INT_MIN. Historically we forbade datatype-specific comparison functions from returning INT_MIN, so that it would be safe to invert the sort order just by negating the comparison result. However, this was never really safe for comparison functions that directly return the result of memcmp(), strcmp(), etc, as POSIX doesn't place any such restriction on those library functions. Buildfarm results show that at least on recent Linux on s390x, memcmp() actually does return INT_MIN sometimes, causing sort failures. The agreed-on answer is to remove this restriction and fix relevant call sites to not make such an assumption; code such as "res = -res" should be replaced by "INVERT_COMPARE_RESULT(res)". The same is needed in a few places that just directly negated the result of memcmp or strcmp. To help find places having this problem, I've also added a compile option to nbtcompare.c that causes some of the commonly used comparators to return INT_MIN/INT_MAX instead of their usual -1/+1. It'd likely be a good idea to have at least one buildfarm member running with "-DSTRESS_SORT_INT_MIN". That's far from a complete test of course, but it should help to prevent fresh introductions of such bugs. This is a longstanding portability hazard, so back-patch to all supported branches. Discussion: https://postgr.es/m/20180928185215.ffoq2xrq5d3pafna@alap3.anarazel.de 05 October 2018, 20:01:30 UTC
dee8488 Tom Lane 02 October 2018, 16:41:28 UTC Set snprintf.c's maximum number of NL arguments to be 31. Previously, we used the platform's NL_ARGMAX if any, otherwise 16. The trouble with this is that the platform value is hugely variable, ranging from the POSIX-minimum 9 to as much as 64K on recent FreeBSD. Values of more than a dozen or two have no practical use and slow down the initialization of the argtypes array. Worse, they cause snprintf.c to consume far more stack space than was the design intention, possibly resulting in stack-overflow crashes. Standardize on 31, which is comfortably more than we need (it looks like no existing translatable message has more than about 10 parameters). I chose that, not 32, to make the array sizes powers of 2, for some possible small gain in speed of the memset. The lack of reported crashes suggests that the set of platforms we use snprintf.c on (in released branches) may have no overlap with the set where NL_ARGMAX has unreasonably large values. But that's not entirely clear, so back-patch to all supported branches. Per report from Mateusz Guzik (via Thomas Munro). Discussion: https://postgr.es/m/CAEepm=3VF=PUp2f8gU8fgZB22yPE_KBS0+e1AHAtQ=09schTHg@mail.gmail.com 02 October 2018, 16:41:28 UTC
dad4df0 Tom Lane 02 October 2018, 15:54:13 UTC Fix corner-case failures in has_foo_privilege() family of functions. The variants of these functions that take numeric inputs (OIDs or column numbers) are supposed to return NULL rather than failing on bad input; this rule reduces problems with snapshot skew when queries apply the functions to all rows of a catalog. has_column_privilege() had careless handling of the case where the table OID didn't exist. You might get something like this: select has_column_privilege(9999,'nosuchcol','select'); ERROR: column "nosuchcol" of relation "(null)" does not exist or you might get a crash, depending on the platform's printf's response to a null string pointer. In addition, while applying the column-number variant to a dropped column returned NULL as desired, applying the column-name variant did not: select has_column_privilege('mytable','........pg.dropped.2........','select'); ERROR: column "........pg.dropped.2........" of relation "mytable" does not exist It seems better to make this case return NULL as well. Also, the OID-accepting variants of has_foreign_data_wrapper_privilege, has_server_privilege, and has_tablespace_privilege didn't follow the principle of returning NULL for nonexistent OIDs. Superusers got TRUE, everybody else got an error. Per investigation of Jaime Casanova's report of a new crash in HEAD. These behaviors have been like this for a long time, so back-patch to all supported branches. Patch by me; thanks to Stephen Frost for discussion and review Discussion: https://postgr.es/m/CAJGNTeP=-6Gyqq5TN9OvYEydi7Fv1oGyYj650LGTnW44oAzYCg@mail.gmail.com 02 October 2018, 15:54:13 UTC
fa7039d Michael Paquier 02 October 2018, 07:36:39 UTC Fix documentation of pgrowlocks using "lock_type" instead of "modes" The example used in the documentation is outdated as well. This is an oversight from 0ac5ad5, which bumped up pgrowlocks but forgot some bits of the documentation. Reported-by: Chris Wilson Discussion: https://postgr.es/m/153838692816.2950.12001142346234155699@wrigleys.postgresql.org Backpatch-through: 9.3 02 October 2018, 07:36:39 UTC
0031e9d Tom Lane 01 October 2018, 15:39:14 UTC Fix ALTER COLUMN TYPE to not open a relation without any lock. If the column being modified is referenced by a foreign key constraint of another table, ALTER TABLE would open the other table (to re-parse the constraint's definition) without having first obtained a lock on it. This was evidently intentional, but that doesn't mean it's really safe. It's especially not safe in 9.3, which pre-dates use of MVCC scans for catalog reads, but even in current releases it doesn't seem like a good idea. We know we'll need AccessExclusiveLock shortly to drop the obsoleted constraint, so just get that a little sooner to close the hole. Per testing with a patch that complains if we open a relation without holding any lock on it. I don't plan to back-patch that patch, but we should close the holes it identifies in all supported branches. Discussion: https://postgr.es/m/2038.1538335244@sss.pgh.pa.us 01 October 2018, 15:39:14 UTC
8b36dc5 Tom Lane 30 September 2018, 20:24:56 UTC Fix detection of the result type of strerror_r(). The method we've traditionally used, of redeclaring strerror_r() to see if the compiler complains of inconsistent declarations, turns out not to work reliably because some compilers only report a warning, not an error. Amazingly, this has gone undetected for years, even though it certainly breaks our detection of whether strerror_r succeeded. Let's instead test whether the compiler will take the result of strerror_r() as a switch() argument. It's possible this won't work universally either, but it's the best idea I could come up with on the spur of the moment. Back-patch of commit 751f532b9. Buildfarm results indicate that only icc-on-Linux actually has an issue here; perhaps the lack of field reports indicates that people don't build PG for production that way. Discussion: https://postgr.es/m/10877.1537993279@sss.pgh.pa.us 30 September 2018, 20:24:56 UTC
85cc9c4 Amit Kapila 28 September 2018, 11:24:40 UTC Fix assertion failure when updating full_page_writes for checkpointer. When the checkpointer receives a SIGHUP signal to update its configuration, it may need to update the shared memory for full_page_writes and need to write a WAL record for it. Now, it is quite possible that the XLOG machinery has not been initialized by that time and it will lead to assertion failure while doing that. Fix is to allow the initialization of the XLOG machinery outside critical section. This bug has been introduced by the commit 2c03216d83 which added the XLOG machinery initialization in RecoveryInProgress code path. Reported-by: Dilip Kumar Author: Dilip Kumar Reviewed-by: Michael Paquier and Amit Kapila Backpatch-through: 9.5 Discussion: https://postgr.es/m/CAFiTN-u4BA8KXcQUWDPNgaKAjDXC=C2whnzBM8TAcv=stckYUw@mail.gmail.com 28 September 2018, 11:24:40 UTC
ed9d6d6 Michael Paquier 28 September 2018, 02:56:11 UTC Fix WAL recycling on standbys depending on archive_mode A restart point or a checkpoint recycling WAL segments treats segments marked with neither ".done" (archiving is done) or ".ready" (segment is ready to be archived) in archive_status the same way for archive_mode being "on" or "always". While for a primary this is fine, a standby running a restart point with archive_mode = on would try to mark such a segment as ready for archiving, which is something that will never happen except after the standby is promoted. Note that this problem applies only to WAL segments coming from the local pg_wal the first time archive recovery is run. Segments part of a self-contained base backup are the most common case where this could happen, however even in this case normally the .done markers would be most likely part of the backup. Segments recovered from an archive are marked as .ready or .done by the startup process, and segments finished streaming are marked as such by the WAL receiver, so they are handled already. Reported-by: Haruka Takatsuka Author: Michael Paquier Discussion: https://postgr.es/m/15402-a453c90ed4cf88b2@postgresql.org Backpatch-through: 9.5, where archive_mode = always has been added. 28 September 2018, 02:56:11 UTC
992f854 Peter Eisentraut 15 June 2018, 03:22:14 UTC Recurse to sequences on ownership change for all relkinds When a table ownership is changed, we must apply that also to any owned sequences. (Otherwise, it would result in a situation that cannot be restored, because linked sequences must have the same owner as the table.) But this was previously only applied to regular tables and materialized views. But it should also apply to at least foreign tables. This patch removes the relkind check altogether, because it doesn't save very much and just introduces the possibility of similar omissions. Bug: #15238 Reported-by: Christoph Berg <christoph.berg@credativ.de> 26 September 2018, 18:20:59 UTC
69a5686 Michael Paquier 26 September 2018, 01:30:38 UTC Rework activation of commit timestamps during recovery The activation and deactivation of commit timestamp tracking has not been handled consistently for a primary or standbys at recovery. The facility can be activated at three different moments of recovery: - The beginning, where a primary would use the GUC value for the decision-making, and where a standby relies on the contents of the control file. - When replaying a XLOG_PARAMETER_CHANGE record at redo. - The end, where both primary and standby rely on the GUC value. Using the GUC value for a primary at the beginning of recovery causes problems with commit timestamp access when doing crash recovery. Particularly, when replaying transaction commits, it could be possible that an attempt to read commit timestamps is done for a transaction which committed at a moment when track_commit_timestamp was disabled. A test case is added to reproduce the failure. The test works down to v11 as it takes advantage of transaction commits within procedures. Reported-by: Hailong Li Author: Masahiko Sawasa, Michael Paquier Reviewed-by: Kyotaro Horiguchi Discussion: https://postgr.es/m/11224478-a782-203b-1f17-e4797b39bdf0@qunar.com Backpatch-through: 9.5, where commit timestamps have been introduced. 26 September 2018, 01:30:38 UTC
6dc28d2 Tom Lane 25 September 2018, 17:23:29 UTC Make some fixes to allow building Postgres on macOS 10.14 ("Mojave"). Apple's latest rearrangements of the system-supplied headers have broken building of PL/Perl and PL/Tcl. The only practical way to fix PL/Tcl is to start using the "-isysroot" compiler flag to point to SDK-supplied headers, as Apple expects. We must also start distinguishing where to find Perl's headers from where to find its shared library; but that seems like good cleanup anyway. Extensions that formerly did something like -I$(perl_archlibexp)/CORE should now do -I$(perl_includedir)/CORE instead. perl_archlibexp is still the place to look for libperl.so, though. If for some reason you don't like the default -isysroot setting, you can override that by setting PG_SYSROOT in configure's arguments. I don't currently think people would need to do so, unless maybe for cross-version build purposes. In addition, teach configure where to find tclConfig.sh. Our traditional method of searching $auto_path hasn't worked for the last couple of macOS releases, and it now seems clear that Apple's not going to change that. The workaround of manually specifying --with-tclconfig was annoying already, but Mojave's made it a lot more so because the sysroot path now has to be included as well. Let's just wire the knowledge into configure instead. To avoid breaking builds against non-default Tcl installations (e.g. MacPorts) wherein the $auto_path method probably still works, arrange to try the additional case only after all else has failed. Back-patch to all supported versions, since at least the buildfarm cares about that. The changes are set up to not do anything on macOS releases that are old enough to not have functional sysroot trees. 25 September 2018, 17:23:29 UTC
6ed095e Tom Lane 24 September 2018, 15:30:51 UTC Fix over-allocation of space for array_out()'s result string. array_out overestimated the space needed for its output, possibly by a very substantial amount if the array is multi-dimensional, because of wrong order of operations in the loop that counts the number of curly-brace pairs needed. While the output string is normally short-lived, this could still cause problems in extreme cases. An additional minor error was that it counted one more delimiter than is actually needed. Repair those errors, add an Assert that the space is now correctly calculated, and make some minor improvements in the comments. I also failed to resist the temptation to get rid of an integer modulus operation per array element; a simple comparison is sufficient. This bug dates clear back to Berkeley days, so back-patch to all supported versions. Keiichi Hirobe, minor additional work by me Discussion: https://postgr.es/m/CAH=EFxE9W0tRvQkixR2XJRRCToUYUEDkJZk6tnADXugPBRdcdg@mail.gmail.com 24 September 2018, 15:30:51 UTC
d68d5ad Noah Misch 24 September 2018, 05:56:39 UTC Initialize random() in bootstrap/stand-alone postgres and in initdb. This removes a difference between the standard IsUnderPostmaster execution environment and that of --boot and --single. In a stand-alone backend, "SELECT random()" always started at the same seed. On a system capable of using posix shared memory, initdb could still conclude "selecting dynamic shared memory implementation ... sysv". Crashed --boot or --single postgres processes orphaned shared memory objects having names that collided with the not-actually-random names that initdb probed. The sysv fallback appeared after ten crashes of --boot or --single postgres. Since --boot and --single are rare in production use, systems used for PostgreSQL development are the principal candidate to notice this symptom. Back-patch to 9.3 (all supported versions). PostgreSQL 9.4 introduced dynamic shared memory, but 9.3 does share the "SELECT random()" problem. Reviewed by Tom Lane and Kyotaro HORIGUCHI. Discussion: https://postgr.es/m/20180915221546.GA3159382@rfd.leadboat.com 24 September 2018, 05:56:43 UTC
c8a978b Tom Lane 23 September 2018, 20:05:45 UTC Fix failure in WHERE CURRENT OF after rewinding the referenced cursor. In a case where we have multiple relation-scan nodes in a cursor plan, such as a scan of an inheritance tree, it's possible to fetch from a given scan node, then rewind the cursor and fetch some row from an earlier scan node. In such a case, execCurrent.c mistakenly thought that the later scan node was still active, because ExecReScan hadn't done anything to make it look not-active. We'd get some sort of failure in the case of a SeqScan node, because the node's scan tuple slot would be pointing at a HeapTuple whose t_self gets reset to invalid by heapam.c. But it seems possible that for other relation scan node types we'd actually return a valid tuple TID to the caller, resulting in updating or deleting a tuple that shouldn't have been considered current. To fix, forcibly clear the ScanTupleSlot in ExecScanReScan. Another issue here, which seems only latent at the moment but could easily become a live bug in future, is that rewinding a cursor does not necessarily lead to *immediately* applying ExecReScan to every scan-level node in the plan tree. Upper-level nodes will think that they can postpone that call if their child node is already marked with chgParam flags. I don't see a way for that to happen today in a plan tree that's simple enough for execCurrent.c's search_plan_tree to understand, but that's one heck of a fragile assumption. So, add some logic in search_plan_tree to detect chgParam flags being set on nodes that it descended to/through, and assume that that means we should consider lower scan nodes to be logically reset even if their ReScan call hasn't actually happened yet. Per bug #15395 from Matvey Arye. This has been broken for a long time, so back-patch to all supported branches. Discussion: https://postgr.es/m/153764171023.14986.280404050547008575@wrigleys.postgresql.org 23 September 2018, 20:05:45 UTC
ed6755b Bruce Momjian 21 September 2018, 23:55:06 UTC docs: remove use of escape strings and use bytea hex output standard_conforming_strings defaulted to 'on' in PG 9.1. bytea_output defaulted to 'hex' in PG 9.0. Reported-by: André Hänsel Discussion: https://postgr.es/m/12e601d447ac$345994a0$9d0cbde0$@webkr.de Backpatch-through: 9.3 21 September 2018, 23:55:06 UTC
dbbc98a Andres Freund 21 September 2018, 01:11:04 UTC Error out for clang on x86-32 without SSE2 support, no -fexcess-precision. As clang currently doesn't support -fexcess-precision=standard, compiling x86-32 code with SSE2 disabled, can lead to problems with floating point overflow checks and the like. This issue was noticed because clang, on at least some BSDs, defaults to i386 compatibility, whereas it defaults to pentium4 on Linux. Our forced usage of __builtin_isinf() lead to some overflow checks not triggering when compiling for i386, e.g. when the result of the calculation didn't overflow in 80bit registers, but did so in 64bit. While we could just fall back to a non-builtin isinf, it seems likely that the use of 80bit registers leads to other problems (which is why we force the flag for GCC already). Therefore error out when detecting clang in that situation. Reported-By: Victor Wagner Analyzed-By: Andrew Gierth and Andres Freund Author: Andres Freund Discussion: https://postgr.es/m/20180905005130.ewk4xcs5dgyzcy45@alap3.anarazel.de Backpatch: 9.3-, all supported versions are affected 21 September 2018, 01:11:04 UTC
fb38949 Thomas Munro 18 September 2018, 10:56:36 UTC Allow DSM allocation to be interrupted. Chris Travers reported that the startup process can repeatedly try to cancel a backend that is in a posix_fallocate()/EINTR loop and cause it to loop forever. Teach the retry loop to give up if an interrupt is pending. Don't actually check for interrupts in that loop though, because a non-local exit would skip some clean-up code in the caller. Back-patch to 9.4 where DSM was added (and posix_fallocate() was later back-patched). Author: Chris Travers Reviewed-by: Ildar Musin, Murat Kabilov, Oleksii Kliukin Tested-by: Oleksii Kliukin Discussion: https://postgr.es/m/CAN-RpxB-oeZve_J3SM_6%3DHXPmvEG%3DHX%2B9V9pi8g2YR7YW0rBBg%40mail.gmail.com 18 September 2018, 11:19:22 UTC
9b14bbd Tom Lane 15 September 2018, 17:42:34 UTC Fix failure with initplans used conditionally during EvalPlanQual rechecks. The EvalPlanQual machinery assumes that any initplans (that is, uncorrelated sub-selects) used during an EPQ recheck would have already been evaluated during the main query; this is implicit in the fact that execPlan pointers are not copied into the EPQ estate's es_param_exec_vals. But it's possible for that assumption to fail, if the initplan is only reached conditionally. For example, a sub-select inside a CASE expression could be reached during a recheck when it had not been previously, if the CASE test depends on a column that was just updated. This bug is old, appearing to date back to my rewrite of EvalPlanQual in commit 9f2ee8f28, but was not detected until Kyle Samson reported a case. To fix, force all not-yet-evaluated initplans used within the EPQ plan subtree to be evaluated at the start of the recheck, before entering the EPQ environment. This could be inefficient, if such an initplan is expensive and goes unused again during the recheck --- but that's piling one layer of improbability atop another. It doesn't seem worth adding more complexity to prevent that, at least not in the back branches. It was convenient to use the new-in-v11 ExecEvalParamExecParams function to implement this, but I didn't like either its name or the specifics of its API, so revise that. Back-patch all the way. Rather than rewrite the patch to avoid depending on bms_next_member() in the oldest branches, I chose to back-patch that function into 9.4 and 9.3. (This isn't the first time back-patches have needed that, and it exhausted my patience.) I also chose to back-patch some test cases added by commits 71404af2a and 342a1ffa2 into 9.4 and 9.3, so that the 9.x versions of eval-plan-qual.spec are all the same. Andrew Gierth diagnosed the problem and contributed the added test cases, though the actual code changes are by me. Discussion: https://postgr.es/m/A033A40A-B234-4324-BE37-272279F7B627@tripadvisor.com 15 September 2018, 17:42:34 UTC
47a589c Amit Kapila 13 September 2018, 10:40:59 UTC Attach FPI to the first record after full_page_writes is turned on. XLogInsert fails to attach a required FPI to the first record after full_page_writes is turned on by the last checkpoint. This bug got introduced in 9.5 due to code rearrangement in commits 2c03216d83 and 2076db2aea. Fix it by ensuring that XLogInsertRecord performs a recomputation when the given record is generated with FPW as off but found that the flag has been turned on while actually inserting the record. Reported-by: Kyotaro Horiguchi Author: Kyotaro Horiguchi Reviewed-by: Amit Kapila Backpatch-through: 9.5 where this problem was introduced Discussion: https://postgr.es/m/20180420.151043.74298611.horiguchi.kyotaro@lab.ntt.co.jp 13 September 2018, 10:40:59 UTC
93a508e Peter Eisentraut 14 August 2018, 20:54:52 UTC doc: Update broken links Discussion: https://www.postgresql.org/message-id/flat/153044458767.13254.16049977382403131287%40wrigleys.postgresql.org 13 September 2018, 06:51:51 UTC
77c2663 Andrew Gierth 12 September 2018, 18:31:06 UTC Repair bug in regexp split performance improvements. Commit c8ea87e4b introduced a temporary conversion buffer for substrings extracted during regexp splits. Unfortunately the code that sized it was failing to ignore the effects of ignored degenerate regexp matches, so for regexp_split_* calls it could under-size the buffer in such cases. Fix, and add some regression test cases (though those will only catch the bug if run in a multibyte encoding). Backpatch to 9.3 as the faulty code was. Thanks to the PostGIS project, Regina Obe and Paul Ramsey for the report (via IRC) and assistance in analysis. Patch by me. 12 September 2018, 18:52:10 UTC
f6c268c Andrew Dunstan 25 September 2017, 12:03:05 UTC Support building with Visual Studio 2017 Haribabu Kommi, reviewed by Takeshi Ideriha and Christian Ullrich Backpatch to 9.6 11 September 2018, 20:09:46 UTC
e950c6c Alexander Korotkov 09 September 2018, 18:19:29 UTC Fix past pd_upper write in ginRedoRecompress() ginRedoRecompress() replays actions over compressed segments of posting list in-place. However, it might lead to write past pg_upper, because intermediate state during playing the changes can take more space than both original state and final state. This commit fixes that by refuse from in-place modification. Instead page tail is copied once modification is started, and then it's used as the source of original segments. Backpatch to 9.4 where posting list compression was introduced. Reported-by: Sivasubramanian Ramasubramanian Discussion: https://postgr.es/m/1536091151804.6588%40amazon.com Author: Alexander Korotkov based on patch from and ideas by Sivasubramanian Ramasubramanian Review: Sivasubramanian Ramasubramanian Backpatch-through: 9.4 09 September 2018, 18:44:58 UTC
0254aa8 Tom Lane 08 September 2018, 00:09:57 UTC Save/restore SPI's global variables in SPI_connect() and SPI_finish(). This patch removes two sources of interference between nominally independent functions when one SPI-using function calls another, perhaps without knowing that it does so. Chapman Flack pointed out that xml.c's query_to_xml_internal() expects SPI_tuptable and SPI_processed to stay valid across datatype output function calls; but it's possible that such a call could involve re-entrant use of SPI. It seems likely that there are similar hazards elsewhere, if not in the core code then in third-party SPI users. Previously SPI_finish() reset SPI's API globals to zeroes/nulls, which would typically make for a crash in such a situation. Restoring them to the values they had at SPI_connect() seems like a considerably more useful behavior, and it still meets the design goal of not leaving any dangling pointers to tuple tables of the function being exited. Also, cause SPI_connect() to reset these variables to zeroes/nulls after saving them. This prevents interference in the opposite direction: it's possible that a SPI-using function that's only ever been tested standalone contains assumptions that these variables start out as zeroes. That was the case as long as you were the outermost SPI user, but not so much for an inner user. Now it's consistent. Report and fix suggestion by Chapman Flack, actual patch by me. Back-patch to all supported branches. Discussion: https://postgr.es/m/9fa25bef-2e4f-1c32-22a4-3ad0723c4a17@anastigmatix.net 08 September 2018, 00:09:57 UTC
cc4e995 Tom Lane 07 September 2018, 22:13:29 UTC Limit depth of forced recursion for CLOBBER_CACHE_RECURSIVELY. It's somewhat surprising that we got away with this before. (Actually, since nobody tests this routinely AFAIK, it might've been broken for awhile. But it's definitely broken in the wake of commit f868a8143.) It seems sufficient to limit the forced recursion to a small number of levels. Back-patch to all supported branches, like the preceding patch. Discussion: https://postgr.es/m/12259.1532117714@sss.pgh.pa.us 07 September 2018, 22:14:30 UTC
66321ae Tom Lane 07 September 2018, 22:04:38 UTC Fix longstanding recursion hazard in sinval message processing. LockRelationOid and sibling routines supposed that, if our session already holds the lock they were asked to acquire, they could skip calling AcceptInvalidationMessages on the grounds that we must have already read any remote sinval messages issued against the relation being locked. This is normally true, but there's a critical special case where it's not: processing inside AcceptInvalidationMessages might attempt to access system relations, resulting in a recursive call to acquire a relation lock. Hence, if the outer call had acquired that same system catalog lock, we'd fall through, despite the possibility that there's an as-yet-unread sinval message for that system catalog. This could, for example, result in failure to access a system catalog or index that had just been processed by VACUUM FULL. This is the explanation for buildfarm failures we've been seeing intermittently for the past three months. The bug is far older than that, but commits a54e1f158 et al added a new recursion case within AcceptInvalidationMessages that is apparently easier to hit than any previous case. To fix this, we must not skip calling AcceptInvalidationMessages until we have *finished* a call to it since acquiring a relation lock, not merely acquired the lock. (There's already adequate logic inside AcceptInvalidationMessages to deal with being called recursively.) Fortunately, we can implement that at trivial cost, by adding a flag to LOCALLOCK hashtable entries that tracks whether we know we have completed such a call. There is an API hazard added by this patch for external callers of LockAcquire: if anything is testing for LOCKACQUIRE_ALREADY_HELD, it might be fooled by the new return code LOCKACQUIRE_ALREADY_CLEAR into thinking the lock wasn't already held. This should be a fail-soft condition, though, unless something very bizarre is being done in response to the test. Also, I added an additional output argument to LockAcquireExtended, assuming that that probably isn't called by any outside code given the very limited usefulness of its additional functionality. Back-patch to all supported branches. Discussion: https://postgr.es/m/12259.1532117714@sss.pgh.pa.us 07 September 2018, 22:04:38 UTC
c79b39f Tom Lane 06 September 2018, 14:49:45 UTC Make contrib/unaccent's unaccent() function work when not in search path. Since the fixes for CVE-2018-1058, we've advised people to schema-qualify function references in order to fix failures in code that executes under a minimal search_path setting. However, that's insufficient to make the single-argument form of unaccent() work, because it looks up the "unaccent" text search dictionary using the search path. The most expedient answer seems to be to remove the search_path dependency by making it look in the same schema that the unaccent() function itself is declared in. This will definitely work for the normal usage of this function with the unaccent dictionary provided by the extension. It's barely possible that there are people who were relying on the search-path-dependent behavior to select other dictionaries with the same name; but if there are any such people at all, they can still get that behavior by writing unaccent('unaccent', ...), or possibly unaccent('unaccent'::text::regdictionary, ...) if the lookup has to be postponed to runtime. Per complaint from Gunnlaugur Thor Briem. Back-patch to all supported branches. Discussion: https://postgr.es/m/CAPs+M8LCex6d=DeneofdsoJVijaG59m9V0ggbb3pOH7hZO4+cQ@mail.gmail.com 06 September 2018, 14:49:45 UTC
ccd9a4d Tom Lane 05 September 2018, 17:47:16 UTC Make argument names of pg_get_object_address consistent, and fix docs. pg_get_object_address and pg_identify_object_as_address are supposed to be inverses, but they disagreed as to the names of the arguments representing the textual form of an object address. Moreover, the documented argument names didn't agree with reality at all, either for these functions or pg_identify_object. In HEAD and v11, I think we can get away with renaming the input arguments of pg_get_object_address to match the outputs of pg_identify_object_as_address. In theory that might break queries using named-argument notation to call pg_get_object_address, but it seems really unlikely that anybody is doing that, or that they'd have much trouble adjusting if they were. In older branches, we'll just live with the lack of consistency. Aside from fixing the documentation of these functions to match reality, I couldn't resist the temptation to do some copy-editing. Per complaint from Jean-Pierre Pelletier. Back-patch to 9.5 where these functions were introduced. (Before v11, this is a documentation change only.) Discussion: https://postgr.es/m/CANGqjDnWH8wsTY_GzDUxbt4i=y-85SJreZin4Hm8uOqv1vzRQA@mail.gmail.com 05 September 2018, 17:47:16 UTC
4693ea6 Bruce Momjian 05 September 2018, 02:34:07 UTC docs: improve AT TIME ZONE description The previous description was unclear. Also add a third example, change use of time zone acronyms to more verbose descriptions, and add a mention that using 'time' with AT TIME ZONE uses the current time zone rules. Backpatch-through: 9.3 05 September 2018, 02:34:07 UTC
02b1b01 Michael Paquier 02 September 2018, 19:40:58 UTC Fix initial sync of slot parent directory when restoring status At the beginning of recovery, information from replication slots is recovered from disk to memory. In order to ensure the durability of the information, the status file as well as its parent directory are synced. It happens that the sync on the parent directory was done directly using the status file path, which is logically incorrect, and the current code has been doing a sync on the same object twice in a row. Reported-by: Konstantin Knizhnik Diagnosed-by: Konstantin Knizhnik Author: Michael Paquier Discussion: https://postgr.es/m/9eb1a6d5-b66f-2640-598d-c5ea46b8f68a@postgrespro.ru Backpatch-through: 9.4- 02 September 2018, 19:40:58 UTC
d368c6c Tom Lane 01 September 2018, 20:02:47 UTC Doc: fix oversights in "Client/Server Character Set Conversions" table. This table claimed that JOHAB could be used as a server encoding, which was true originally but hasn't been true since 8.3. It also lacked entries for EUC_JIS_2004 and SHIFT_JIS_2004. JOHAB problem noted by Lars Kanis, the others by me. Discussion: https://postgr.es/m/c0f514a1-b7a9-b9ea-1c02-c34aead56c06@greiz-reinsdorf.de 01 September 2018, 20:02:47 UTC
03ffe55 Tom Lane 01 September 2018, 19:27:13 UTC Avoid using potentially-under-aligned page buffers. There's a project policy against using plain "char buf[BLCKSZ]" local or static variables as page buffers; preferred style is to palloc or malloc each buffer to ensure it is MAXALIGN'd. However, that policy's been ignored in an increasing number of places. We've apparently got away with it so far, probably because (a) relatively few people use platforms on which misalignment causes core dumps and/or (b) the variables chance to be sufficiently aligned anyway. But this is not something to rely on. Moreover, even if we don't get a core dump, we might be paying a lot of cycles for misaligned accesses. To fix, invent new union types PGAlignedBlock and PGAlignedXLogBlock that the compiler must allocate with sufficient alignment, and use those in place of plain char arrays. I used these types even for variables where there's no risk of a misaligned access, since ensuring proper alignment should make kernel data transfers faster. I also changed some places where we had been palloc'ing short-lived buffers, for coding style uniformity and to save palloc/pfree overhead. Since this seems to be a live portability hazard (despite the lack of field reports), back-patch to all supported versions. Patch by me; thanks to Michael Paquier for review. Discussion: https://postgr.es/m/1535618100.1286.3.camel@credativ.de 01 September 2018, 19:27:13 UTC
e3eca93 Noah Misch 01 September 2018, 05:59:58 UTC Ignore server-side delays when enforcing wal_sender_timeout. Healthy clients of servers having poor I/O performance, such as buildfarm members hamster and tern, saw unexpected timeouts. That disagreed with documentation. This fix adds one gettimeofday() call whenever ProcessRepliesIfAny() finds no client reply messages. Back-patch to 9.4; the bug's symptom is rare and mild, and the code all moved between 9.3 and 9.4. Discussion: https://postgr.es/m/20180826034600.GA1105084@rfd.leadboat.com 01 September 2018, 06:00:02 UTC
f3520ff Michael Paquier 31 August 2018, 18:04:46 UTC Ensure correct minimum consistent point on standbys Startup process has improved its calculation of incorrect minimum consistent point in 8d68ee6, which ensures that all WAL available gets replayed when doing crash recovery, and has introduced an incorrect calculation of the minimum recovery point for non-startup processes, which can cause incorrect page references on a standby when for example the background writer flushed a couple of pages on-disk but was not updating the control file to let a subsequent crash recovery replay to where it should have. The only case where this has been reported to be a problem is when a standby needs to calculate the latest removed xid when replaying a btree deletion record, so one would need connections on a standby that happen just after recovery has thought it reached a consistent point. Using a background worker which is started after the consistent point is reached would be the easiest way to get into problems if it connects to a database. Having clients which attempt to connect periodically could also be a problem, but the odds of seeing this problem are much lower. The fix used is pretty simple, as the idea is to give access to the minimum recovery point written in the control file to non-startup processes so as they use a reference, while the startup process still initializes its own references of the minimum consistent point so as the original problem with incorrect page references happening post-promotion with a crash do not show up. Reported-by: Alexander Kukushkin Diagnosed-by: Alexander Kukushkin Author: Michael Paquier Reviewed-by: Kyotaro Horiguchi, Alexander Kukushkin Discussion: https://postgr.es/m/153492341830.1368.3936905691758473953@wrigleys.postgresql.org Backpatch-through: 9.3 31 August 2018, 18:04:46 UTC
b187dae Alexander Korotkov 30 August 2018, 11:18:53 UTC Enforce cube dimension limit in all cube construction functions contrib/cube has a limit to 100 dimensions for cube datatype. However, it's not enforced everywhere, and one can actually construct cube with more than 100 dimensions having then trouble with dump/restore. This commit add checks for dimensions limit in all functions responsible for cube construction. Backpatch to all supported versions. Reported-by: Andrew Gierth Discussion: https://postgr.es/m/87va7uybt4.fsf%40news-spur.riddles.org.uk Author: Andrey Borodin with small additions by me Review: Tom Lane Backpatch-through: 9.3 31 August 2018, 17:06:49 UTC
26cbd23 Alexander Korotkov 30 August 2018, 11:09:25 UTC Split contrib/cube platform-depended checks into separate test We're currently maintaining two outputs for cube regression test. But that appears to be unsuitable, because these outputs are different in out few checks involving scientific notation. So, split checks involving scientific notation into separate test, making contrib/cube easier to maintain. Backpatch to all supported versions in order to make further backpatching easier. Discussion: https://postgr.es/m/CAPpHfdvJgWjxHsJTtT%2Bo1tz3OR8EFHcLQjhp-d3%2BUcmJLh-fQA%40mail.gmail.com Author: Alexander Korotkov Backpatch-through: 9.3 31 August 2018, 17:06:32 UTC
853af99 Tom Lane 31 August 2018, 16:26:20 UTC Make checksum_impl.h safe to compile with -fstrict-aliasing. In general, Postgres requires -fno-strict-aliasing with compilers that implement C99 strict aliasing rules. There's little hope of getting rid of that overall. But it seems like it would be a good idea if storage/checksum_impl.h in particular didn't depend on it, because that header is explicitly intended to be included by external programs. We don't have a lot of control over the compiler switches that an external program might use, as shown by Michael Banck's report of failure in a privately-modified version of pg_verify_checksums. Hence, switch to using a union in place of willy-nilly pointer casting inside this file. I think this makes the code a bit more readable anyway. checksum_impl.h hasn't changed since it was introduced in 9.3, so back-patch all the way. Discussion: https://postgr.es/m/1535618100.1286.3.camel@credativ.de 31 August 2018, 16:27:07 UTC
32f2792 Michael Paquier 30 August 2018, 00:11:40 UTC Stop bgworkers during fast shutdown with postmaster in startup phase When a postmaster gets into its phase PM_STARTUP, it would start background workers using BgWorkerStart_PostmasterStart mode immediately, which would cause problems for a fast shutdown as the postmaster forgets to send SIGTERM to already-started background workers. With smart and immediate shutdowns, this correctly happened, and fast shutdown is the only mode missing the shot. Author: Alexander Kukushkin Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/CAFh8B=mvnD8+DZUfzpi50DoaDfZRDfd7S=gwj5vU9GYn8UvHkA@mail.gmail.com Backpatch-through: 9.5 30 August 2018, 00:11:40 UTC
aac21f1 Tom Lane 28 August 2018, 23:46:59 UTC Make pg_restore's identify_locking_dependencies() more bulletproof. This function had a blacklist of dump object types that it believed needed exclusive lock ... but we hadn't maintained that, so that it was missing ROW SECURITY, POLICY, and INDEX ATTACH items, all of which need (or should be treated as needing) exclusive lock. Since the same oversight seems likely in future, let's reverse the sense of the test so that the code has a whitelist of safe object types; better to wrongly assume a command can't be run in parallel than the opposite. Currently the only POST_DATA object type that's safe is CREATE INDEX ... and that list hasn't changed in a long time. Back-patch to 9.5 where RLS came in. Discussion: https://postgr.es/m/11450.1535483506@sss.pgh.pa.us 28 August 2018, 23:46:59 UTC
41cfae1 Andrew Gierth 28 August 2018, 08:52:25 UTC Avoid quadratic slowdown in regexp match/split functions. regexp_matches, regexp_split_to_table and regexp_split_to_array all work by compiling a list of match positions as character offsets (NOT byte positions) in the source string. Formerly, they then used text_substr to extract the matched text; but in a multi-byte encoding, that counts the characters in the string, and the characters needed to reach the starting byte position, on every call. Accordingly, the performance degraded as the product of the input string length and the number of match positions, such that splitting a string of a few hundred kbytes could take many minutes. Repair by keeping the wide-character copy of the input string available (only in the case where encoding_max_length is not 1) after performing the match operation, and extracting substrings from that instead. This reduces the complexity to being linear in the number of result bytes, discounting the actual regexp match itself (which is not affected by this patch). In passing, remove cleanup using retail pfree() which was obsoleted by commit ff428cded (Feb 2008) which made cleanup of SRF multi-call contexts automatic. Also increase (to ~134 million) the maximum number of matches and provide an error message when it is reached. Backpatch all the way because this has been wrong forever. Analysis and patch by me; review by Kaiting Chen. Discussion: https://postgr.es/m/87pnyn55qh.fsf@news-spur.riddles.org.uk see also https://postgr.es/m/87lg996g4r.fsf@news-spur.riddles.org.uk 28 August 2018, 10:51:06 UTC
3998e55 Tom Lane 27 August 2018, 19:11:12 UTC Fix missing dependency for pg_dump's ENABLE ROW LEVEL SECURITY items. The archive should show a dependency on the item's table, but it failed to include one. This could cause failures in parallel restore due to emitting ALTER TABLE ... ENABLE ROW LEVEL SECURITY before restoring the table's data. In practice the odds of a problem seem low, since you would typically need to have set FORCE ROW LEVEL SECURITY as well, and you'd also need a very high --jobs count to have any chance of this happening. That probably explains the lack of field reports. Still, it's a bug, so back-patch to 9.5 where RLS was introduced. Discussion: https://postgr.es/m/19784.1535390902@sss.pgh.pa.us 27 August 2018, 19:11:12 UTC
8895daf Tom Lane 26 August 2018, 18:21:55 UTC Make syslogger more robust against failures in opening CSV log files. The previous coding figured it'd be good enough to postpone opening the first CSV log file until we got a message we needed to write there. This is unsafe, though, because if the open fails we end up in infinite recursion trying to report the failure. Instead make the CSV log file management code look as nearly as possible like the longstanding logic for the stderr log file. In particular, open it immediately at postmaster startup (if enabled), or when we get a SIGHUP in which we find that log_destination has been changed to enable CSV logging. It seems OK to fail if a postmaster-start-time open attempt fails, as we've long done for the stderr log file. But we can't die if we fail to open a CSV log file during SIGHUP, so we're still left with a problem. In that case, write any output meant for the CSV log file to the stderr log file. (This will also cover race-condition cases in which backends send CSV log data before or after we have the CSV log file open.) This patch also fixes an ancient oversight that, if CSV logging was turned off during a SIGHUP, we never actually closed the last CSV log file. In passing, remember to reset whereToSendOutput = DestNone during syslogger start, since (unlike all other postmaster children) it's forked before the postmaster has done that. This made for a platform-dependent difference in error reporting behavior between the syslogger and other children: except on Windows, it'd report problems to the original postmaster stderr as well as the normal error log file(s). It's barely possible that that was intentional at some point; but it doesn't seem likely to be desirable in production, and the platform dependency definitely isn't desirable. Per report from Alexander Kukushkin. It's been like this for a long time, so back-patch to all supported branches. Discussion: https://postgr.es/m/CAFh8B==iLUD_gqC-dAENS0V+kVrCeGiKujtKqSQ7++S-caaChw@mail.gmail.com 26 August 2018, 18:21:55 UTC
3e83829 Bruce Momjian 25 August 2018, 17:35:14 UTC doc: "Latest checkpoint location" will not match in pg_upgrade Mention that "Latest checkpoint location" will not match in pg_upgrade if the standby server is still running during the upgrade, which is possible. "Match" text first appeared in PG 9.5. Reported-by: Paul Bonaud Discussion: https://postgr.es/m/c7268794-edb4-1772-3bfd-04c54585c24e@trainline.com Backpatch-through: 9.5 25 August 2018, 17:35:14 UTC
56c1d75 Bruce Momjian 25 August 2018, 17:01:24 UTC doc: add doc link for 'applicable_roles' Reported-by: Ashutosh Sharma Discussion: https://postgr.es/m/CAE9k0PnhnL6MNDLuvkk8USzOa_DpzDzFQPAM_uaGuXbh9HMKYw@mail.gmail.com Author: Ashutosh Sharma Backpatch-through: 9.3 25 August 2018, 17:01:24 UTC
028f855 Bruce Momjian 25 August 2018, 15:52:29 UTC docs: clarify plpython SD and GD dictionary behavior Reported-by: Adam Bielański Discussion: https://postgr.es/m/153484305538.1370.7605856225879294548@wrigleys.postgresql.org Backpatch-through: 9.3 25 August 2018, 15:52:29 UTC
af988d1 Andrew Gierth 23 August 2018, 17:29:18 UTC Fix lexing of standard multi-character operators in edge cases. Commits c6b3c939b (which fixed the precedence of >=, <=, <> operators) and 865f14a2d (which added support for the standard => notation for named arguments) created a class of lexer tokens which look like multi-character operators but which have their own token IDs distinct from Op. However, longest-match rules meant that following any of these tokens with another operator character, as in (1<>-1), would cause them to be incorrectly returned as Op. The error here isn't immediately obvious, because the parser would usually still find the correct operator via the Op token, but there were more subtle problems: 1. If immediately followed by a comment or +-, >= <= <> would be given the old precedence of Op rather than the correct new precedence; 2. If followed by a comment, != would be returned as Op rather than as NOT_EQUAL, causing it not to be found at all; 3. If followed by a comment or +-, the => token for named arguments would be lexed as Op, causing the argument to be mis-parsed as a simple expression, usually causing an error. Fix by explicitly checking for the operators in the {operator} code block in addition to all the existing special cases there. Backpatch to 9.5 where the problem was introduced. Analysis and patch by me; review by Tom Lane. Discussion: https://postgr.es/m/87va851ppl.fsf@news-spur.riddles.org.uk 23 August 2018, 20:35:53 UTC
ad871a9 Andrew Gierth 23 August 2018, 19:00:12 UTC Reduce an unnecessary O(N^3) loop in lexer. The lexer's handling of operators contained an O(N^3) hazard when dealing with long strings of + or - characters; it seems hard to prevent this case from being O(N^2), but the additional N multiplier was not needed. Backpatch all the way since this has been there since 7.x, and it presents at least a mild hazard in that trying to do Bind, PREPARE or EXPLAIN on a hostile query could take excessive time (without honouring cancels or timeouts) even if the query was never executed. 23 August 2018, 20:33:55 UTC
6120f22 Michael Paquier 21 August 2018, 06:18:14 UTC Fix set of NLS translation issues While monitoring the code, a couple of issues related to string translation has showed up: - Some routines for auto-updatable views return an error string, which sometimes missed the shot. A comment regarding string translation is added for each routine to help with future features. - GSSAPI authentication missed two translations. - vacuumdb handles non-translated strings. Reported-by: Kyotaro Horiguchi Author: Kyotaro Horiguchi Reviewed-by: Michael Paquier, Tom Lane Discussion: https://postgr.es/m/20180810.152131.31921918.horiguchi.kyotaro@lab.ntt.co.jp Backpatch-through: 9.3 21 August 2018, 06:18:14 UTC
3998dfe Tom Lane 17 August 2018, 21:12:21 UTC Ensure schema qualification in pg_restore DISABLE/ENABLE TRIGGER commands. Previously, this code blindly followed the common coding pattern of passing PQserverVersion(AH->connection) as the server-version parameter of fmtQualifiedId. That works as long as we have a connection; but in pg_restore with text output, we don't. Instead we got a zero from PQserverVersion, which fmtQualifiedId interpreted as "server is too old to have schemas", and so the name went unqualified. That still accidentally managed to work in many cases, which is probably why this ancient bug went undetected for so long. It only became obvious in the wake of the changes to force dump/restore to execute with restricted search_path. In HEAD/v11, let's deal with this by ripping out fmtQualifiedId's server- version behavioral dependency, and just making it schema-qualify all the time. We no longer support pg_dump from servers old enough to need the ability to omit schema name, let alone restoring to them. (Also, the few callers outside pg_dump already didn't work with pre-schema servers.) In older branches, that's not an acceptable solution, so instead just tweak the DISABLE/ENABLE TRIGGER logic to ensure it will schema-qualify its output regardless of server version. Per bug #15338 from Oleg somebody. Back-patch to all supported branches. Discussion: https://postgr.es/m/153452458706.1316.5328079417086507743@wrigleys.postgresql.org 17 August 2018, 21:12:21 UTC
d2ecc27 Andrew Gierth 17 August 2018, 14:04:26 UTC Set scan direction appropriately for SubPlans (bug #15336) When executing a SubPlan in an expression, the EState's direction field was left alone, resulting in an attempt to execute the subplan backwards if it was encountered during a backwards scan of a cursor. Also, though much less likely, it was possible to reach the execution of an InitPlan while in backwards-scan state. Repair by saving/restoring estate->es_direction and forcing forward scan mode in the relevant places. Backpatch all the way, since this has been broken since 8.3 (prior to commit c7ff7663e, SubPlans had their own EStates rather than sharing the parent plan's, so there was no confusion over scan direction). Per bug #15336 reported by Vladimir Baranoff; analysis and patch by me, review by Tom Lane. Discussion: https://postgr.es/m/153449812167.1304.1741624125628126322@wrigleys.postgresql.org 17 August 2018, 15:20:04 UTC
30f4801 Bruce Momjian 17 August 2018, 14:25:48 UTC pg_upgrade: issue helpful error message for use on standbys Commit 777e6ddf1723306bd2bf8fe6f804863f459b0323 checked for a shut down message from a standby and allowed it to continue. This patch reports a helpful error message in these cases, suggesting to use rsync as documented. Diagnosed-by: Martín Marqués Discussion: https://postgr.es/m/CAPdiE1xYCow-reLjrhJ9DqrMu-ppNq0ChUUEvVdxhdjGRD5_eA@mail.gmail.com Backpatch-through: 9.3 17 August 2018, 14:25:48 UTC
82784f0 Michael Paquier 17 August 2018, 02:29:15 UTC Mention ownership requirements for REFRESH MATERIALIZED VIEW in docs Author: Dian Fay Discussion: https://postgr.es/m/745abbd2-a1a0-ead8-2cb2-768c16747d97@gmail.com Backpatch-through: 9.3 17 August 2018, 02:32:32 UTC
864ecd7 Tomas Vondra 16 August 2018, 14:49:10 UTC Close the file descriptor in ApplyLogicalMappingFile The function was forgetting to close the file descriptor, resulting in failures like this: ERROR: 53000: exceeded maxAllocatedDescs (492) while trying to open file "pg_logical/mappings/map-4000-4eb-1_60DE1E08-5376b5-537c6b" LOCATION: OpenTransientFile, fd.c:2161 Simply close the file at the end, and backpatch to 9.4 (where logical decoding was introduced). While at it, fix a nearby typo. Discussion: https://www.postgresql.org/message-id/flat/738a590a-2ce5-9394-2bef-7b1caad89b37%402ndquadrant.com 16 August 2018, 14:51:46 UTC
8e9f229 Tom Lane 15 August 2018, 21:25:23 UTC Make snprintf.c follow the C99 standard for snprintf's result value. C99 says that the result should be the number of bytes that would have been emitted given a large enough buffer, not the number we actually were able to put in the buffer. It's time to make our substitute implementation comply with that. Not doing so results in inefficiency in buffer-enlargement cases, and also poses a portability hazard for third-party code that might expect C99-compliant snprintf behavior within Postgres. In passing, remove useless tests for str == NULL; neither C99 nor predecessor standards ever allowed that except when count == 0, so I see no reason to expend cycles on making that a non-crash case for this implementation. Also, don't waste a byte in pg_vfprintf's local I/O buffer; this might have performance benefits by allowing aligned writes during flushbuffer calls. Back-patch of commit 805889d7d. There was some concern about this possibly breaking code that assumes pre-C99 behavior, but there is much more risk (and reality, in our own code) of code that assumes C99 behavior and hence fails to detect buffer overrun without this. Discussion: https://postgr.es/m/17245.1534289329@sss.pgh.pa.us 15 August 2018, 21:25:23 UTC
c81062e Tom Lane 15 August 2018, 20:29:32 UTC Clean up assorted misuses of snprintf()'s result value. Fix a small number of places that were testing the result of snprintf() but doing so incorrectly. The right test for buffer overrun, per C99, is "result >= bufsize" not "result > bufsize". Some places were also checking for failure with "result == -1", but the standard only says that a negative value is delivered on failure. (Note that this only makes these places correct if snprintf() delivers C99-compliant results. But at least now these places are consistent with all the other places where we assume that.) Also, make psql_start_test() and isolation_start_test() check for buffer overrun while constructing their shell commands. There seems like a higher risk of overrun, with more severe consequences, here than there is for the individual file paths that are made elsewhere in the same functions, so this seemed like a worthwhile change. Also fix guc.c's do_serialize() to initialize errno = 0 before calling vsnprintf. In principle, this should be unnecessary because vsnprintf should have set errno if it returns a failure indication ... but the other two places this coding pattern is cribbed from don't assume that, so let's be consistent. These errors are all very old, so back-patch as appropriate. I think that only the shell command overrun cases are even theoretically reachable in practice, but there's not much point in erroneous error checks. Discussion: https://postgr.es/m/17245.1534289329@sss.pgh.pa.us 15 August 2018, 20:29:32 UTC
dcca996 Bruce Momjian 14 August 2018, 21:19:02 UTC pg_upgrade: fix shutdown check for standby servers Commit 244142d32afd02e7408a2ef1f249b00393983822 only tested for the pg_controldata output for primary servers, but standby servers have different "Database cluster state" output, so check for that too. Diagnosed-by: Michael Paquier Discussion: https://postgr.es/m/20180810164240.GM13638@paquier.xyz Backpatch-through: 9.3 14 August 2018, 21:19:02 UTC
69467eb Bruce Momjian 09 August 2018, 14:13:15 UTC docs: Only first instance of a PREPARE parameter sets data type If the first reference to $1 is "($1 = col) or ($1 is null)", the data type can be determined, but not for "($1 is null) or ($1 = col)". This change documents this. Reported-by: Morgan Owens Discussion: https://postgr.es/m/153233728858.1404.15268121695358514937@wrigleys.postgresql.org Backpatch-through: 9.3 09 August 2018, 14:13:15 UTC
f318f7f Heikki Linnakangas 08 August 2018, 16:08:10 UTC Don't run atexit callbacks in quickdie signal handlers. exit() is not async-signal safe. Even if the libc implementation is, 3rd party libraries might have installed unsafe atexit() callbacks. After receiving SIGQUIT, we really just want to exit as quickly as possible, so we don't really want to run the atexit() callbacks anyway. The original report by Jimmy Yih was a self-deadlock in startup_die(). However, this patch doesn't address that scenario; the signal handling while waiting for the startup packet is more complicated. But at least this alleviates similar problems in the SIGQUIT handlers, like that reported by Asim R P later in the same thread. Backpatch to 9.3 (all supported versions). Discussion: https://www.postgresql.org/message-id/CAOMx_OAuRUHiAuCg2YgicZLzPVv5d9_H4KrL_OFsFP%3DVPekigA%40mail.gmail.com 08 August 2018, 16:09:35 UTC
74c877e Tom Lane 07 August 2018, 20:32:50 UTC Don't record FDW user mappings as members of extensions. CreateUserMapping has a recordDependencyOnCurrentExtension call that's been there since extensions were introduced (very possibly my fault). However, there's no support anywhere else for user mappings as members of extensions, nor are they listed as a possible member object type in the documentation. Nor does it really seem like a good idea for user mappings to belong to extensions when roles don't. Hence, remove the bogus call. (As we saw in bug #15310, the lack of any pg_dump support for this case ensures that any such membership record would silently disappear during pg_upgrade. So there's probably no need for us to do anything else about cleaning up after this mistake.) Discussion: https://postgr.es/m/27952.1533667213@sss.pgh.pa.us 07 August 2018, 20:33:08 UTC
0667d88 Tom Lane 07 August 2018, 20:00:44 UTC Fix incorrect initialization of BackendActivityBuffer. Since commit c8e8b5a6e, this has been zeroed out using the wrong length. In practice the length would always be too small, leading to not zeroing the whole buffer rather than clobbering additional memory; and that's pretty harmless, both because shmem would likely start out as zeroes and because we'd reinitialize any given entry before use. Still, it's bogus, so fix it. Reported by Petru-Florin Mihancea (bug #15312) Discussion: https://postgr.es/m/153363913073.1303.6518849192351268091@wrigleys.postgresql.org 07 August 2018, 20:01:09 UTC
91f6ec2 Tom Lane 07 August 2018, 19:43:49 UTC Fix pg_upgrade to handle event triggers in extensions correctly. pg_dump with --binary-upgrade must emit ALTER EXTENSION ADD commands for all objects that are members of extensions. It forgot to do so for event triggers, as per bug #15310 from Nick Barnes. Back-patch to 9.3 where event triggers were introduced. Haribabu Kommi Discussion: https://postgr.es/m/153360083872.1395.4593932457718151600@wrigleys.postgresql.org 07 August 2018, 19:43:49 UTC
f3f6558 Tom Lane 07 August 2018, 17:13:42 UTC Ensure pg_dump_sort.c sorts null vs non-null namespace consistently. The original coding here (which is, I believe, my fault) supposed that it didn't need to concern itself with the possibility that one object of a given type-priority has a namespace while another doesn't. But that's not reliably true anymore, if it ever was; and if it does happen then it's possible that DOTypeNameCompare returns self-inconsistent comparison results. That leads to unspecified behavior in qsort() and a resultant weird output order from pg_dump. This should end up being only a cosmetic problem, because any ordering constraints that actually matter should be enforced by the later dependency-based sort. Still, it's a bug, so back-patch. Report and fix by Jacob Champion, though I editorialized on his patch to the extent of making NULL sort after non-NULL, for consistency with our usual sorting definitions. Discussion: https://postgr.es/m/CABAq_6Hw+V-Kj7PNfD5tgOaWT_-qaYkc+SRmJkPLeUjYXLdxwQ@mail.gmail.com 07 August 2018, 17:13:42 UTC
back to top