summaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2019-09-19Fix oversight in backpatch of 6cae9d2c10Alexander Korotkov
During backpatch of 6cae9d2c10 Float8GetDatum() was accidentally removed. This commit turns it back. Reported-by: Erik Rijkers Discussion: https://postgr.es/m/6d51305e1159241cabee132f7efc7eff%40xs4all.nl Author: Tom Lane Backpatch-through: from 11 to 9.5
2019-09-19Improve handling of NULLs in KNN-GiST and KNN-SP-GiSTAlexander Korotkov
This commit improves subject in two ways: * It removes ugliness of 02f90879e7, which stores distance values and null flags in two separate arrays after GISTSearchItem struct. Instead we pack both distance value and null flag in IndexOrderByDistance struct. Alignment overhead should be negligible, because we typically deal with at most few "col op const" expressions in ORDER BY clause. * It fixes handling of "col op NULL" expression in KNN-SP-GiST. Now, these expression are not passed to support functions, which can't deal with them. Instead, NULL result is implicitly assumed. It future we may decide to teach support functions to deal with NULL arguments, but current solution is bugfix suitable for backpatch. Reported-by: Nikita Glukhov Discussion: https://postgr.es/m/826f57ee-afc7-8977-c44c-6111d18b02ec%40postgrespro.ru Author: Nikita Glukhov Reviewed-by: Alexander Korotkov Backpatch-through: 9.4
2019-09-18pg_upgrade/test.sh: Quote sed(1) argumentAlvaro Herrera
Lack of quotes results in failure to run the test under older Solaris. Author: Marina Polyakova, Victor Wagner Discussion: https://postgr.es/m/feba89f89e8925b3535cb7d72b9e05e1@postgrespro.ru
2019-09-13Replace xlc __fetch_and_add() with inline asm.Noah Misch
PostgreSQL has been unusable when built with xlc 13 and newer, which are incompatible with our use of __fetch_and_add(). Back-patch to 9.5, which introduced pg_atomic_fetch_add_u32(). Reviewed by Tom Lane. Discussion: https://postgr.es/m/20190831071157.GA3251746@rfd.leadboat.com
2019-09-13Test pg_atomic_fetch_add_ with variable addend and 16-bit edge cases.Noah Misch
Back-patch to 9.5, which introduced these functions. Reviewed by Tom Lane. Discussion: https://postgr.es/m/20190831071157.GA3251746@rfd.leadboat.com
2019-09-13logical decoding: process ASSIGNMENT during snapshot buildAlvaro Herrera
Most WAL records are ignored in early SnapBuild snapshot build phases. But it's critical to process some of them, so that later messages have the correct transaction state after the snapshot is completely built; in particular, XLOG_XACT_ASSIGNMENT messages are critical in order for sub-transactions to be correctly assigned to their parent transactions, or at least one assert misbehaves, as reported by Ildar Musin. Diagnosed-by: Masahiko Sawada Author: Masahiko Sawada Discussion: https://postgr.es/m/CAONYFtOv+Er1p3WAuwUsy1zsCFrSYvpHLhapC_fMD-zNaRWxYg@mail.gmail.com
2019-09-13Fix under-parenthesized macro definitionsAlvaro Herrera
Lack of parens in the definitions could cause a statement using these macros to have unexpected semantics. In current code no bug is apparent, but best to fix the definitions to avoid problems down the line. Reported-by: Tom Lane Discussion: https://postgr.es/m/19795.1568400476@sss.pgh.pa.us
2019-09-12Fix nbtree page split rmgr desc routine.Peter Geoghegan
Include newitemoff in rmgr desc output for nbtree page split records. In passing, correct an obsolete comment that claimed that newitemoff is only logged for _L variant nbtree page split WAL records. Both issues were oversights in commit 2c03216d831, which revamped the WAL format. Author: Peter Geoghegan Backpatch: 9.5-, where the WAL format was revamped.
2019-09-12Fix usage of whole-row variables in WCO and RLS policy expressions.Tom Lane
Since WITH CHECK OPTION was introduced, ExecInitModifyTable has initialized WCO expressions with the wrong plan node as parent -- that is, it passed its input subplan not the ModifyTable node itself. Up to now we thought this was harmless, but bug #16006 from Vinay Banakar shows it's not: if the input node is a SubqueryScan then ExecInitWholeRowVar can get confused into doing the wrong thing. (The fact that ExecInitWholeRowVar contains such logic is certainly a horrid kluge that doesn't deserve to live, but figuring out another way to do that is a task for some other day.) Andres had already noticed the wrong-parent mistake and fixed it in commit 148e632c0, but not being aware of any user-visible consequences, he quite reasonably didn't back-patch. This patch is simply a back-patch of 148e632c0, plus addition of a test case based on bug #16006. I also added the test case to v12/HEAD, even though the bug is already fixed there. Back-patch to all supported branches. 9.4 lacks RLS policies so the new test case doesn't work there, but I'm pretty sure a test could be devised based on using a whole-row Var in a plain WITH CHECK OPTION condition. (I lack the cycles to do so myself, though.) Andres Freund and Tom Lane Discussion: https://postgr.es/m/16006-99290d2e4642cbd5@postgresql.org Discussion: https://postgr.es/m/20181205225213.hiwa3kgoxeybqcqv@alap3.anarazel.de
2019-09-11Expand properly list of TAP tests used for prove in vcregress.plMichael Paquier
Depending on the system used, t/*.pl may not be expanded into a list of tests which can be consumed by prove when attempting to run TAP tests on a given path. Fix that by using glob() directly in the script, to make sure that a complete list of tests is provided. This has not proved to be an issue with MSVC as the list was properly expanded, but it is on Linux with perl's system(). This is extracted from a larger patch. Author: Tom Lane Discussion: https://postgr.es/m/6628.1567958876@sss.pgh.pa.us Backpatch-through: 9.4
2019-09-10Sync isolationtester's handling of notice/warning messages with HEAD.Tom Lane
Back-patch relevant parts of these commits: 30717637c Fix isolationtester race condition for notices sent before blocking ebd499282 Don't drop NOTICE messages in isolation tests a28e10e82 Indicate session name in isolationtester notices This ensures that older versions of the isolationtester will handle NOTICE/WARNING messages the same way as HEAD and v12 do. While this isn't fixing any critical problem right now, it seems like a prudent change to prevent surprises (like we had yesterday...) with back-patches of future isolation test changes. Back-patch as far as 9.6. Due to the significant changes we made in isolationtester in 9.6, back-patching isolation tests further than that is going to be risky anyway; besides, this patch doesn't apply cleanly before that. Discussion: https://postgr.es/m/E1i7IqC-0000Uc-5H@gemulon.postgresql.org
2019-09-09Prevent msys2 conversion of "cmd /c" switch to a file pathAndrew Dunstan
Modern versions of msys2 have changed the treatment of "cmd /c" so that the runtime will try to convert the switch to a native file path. This patch adds a setting to inhibit that behaviour. Discussion: https://postgr.es/m/3227042f-cfcc-745a-57dd-fb8c471f8ddf@2ndQuadrant.com Backpatch to all live branches.
2019-09-09Always skip recovery SysV shared memory tests on WindowsAndrew Dunstan
These tests were disabled on git master in commit 8e5ce1c3f8. This does the same thing on the back branches. Discussion: https://postgr.es/m/a1c40fab-a38c-cb42-6879-125f834e837b@2ndQuadrant.com
2019-09-08Fix RelationIdGetRelation calls that weren't bothering with error checks.Tom Lane
Some of these are quite old, but that doesn't make them not bugs. We'd rather report a failure via elog than SIGSEGV. While at it, uniformly spell the error check as !RelationIsValid(rel) rather than a bare rel == NULL test. The machine code is the same but it seems better to be consistent. Coverity complained about this today, not sure why, because the mistake is in fact old.
2019-09-08Fix handling of NULL distances in KNN-GiSTAlexander Korotkov
In order to implement NULL LAST semantic GiST previously assumed distance to the NULL value to be Inf. However, our distance functions can return Inf and NaN for non-null values. In such cases, NULL LAST semantic appears to be broken. This commit fixes that by introducing separate array of null flags for distances. Backpatch to all supported versions. Discussion: https://postgr.es/m/CAPpHfdsNvNdA0DBS%2BwMpFrgwT6C3-q50sFVGLSiuWnV3FqOJuQ%40mail.gmail.com Author: Alexander Korotkov Backpatch-through: 9.4
2019-09-08Fix handling Inf and Nan values in GiST pairing heap comparatorAlexander Korotkov
Previously plain float comparison was used in GiST pairing heap. Such comparison doesn't provide proper ordering for value sets containing Inf and Nan values. This commit fixes that by usage of float8_cmp_internal(). Note, there is remaining problem with NULL distances, which are represented as Inf in pairing heap. It would be fixes in subsequent commit. Backpatch to all supported versions. Reported-by: Andrey Borodin Discussion: https://postgr.es/m/CAPpHfdsNvNdA0DBS%2BwMpFrgwT6C3-q50sFVGLSiuWnV3FqOJuQ%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Heikki Linnakangas Backpatch-through: 9.4
2019-09-06When performing a base backup, check for read errors.Robert Haas
The old code didn't differentiate between a read error and a concurrent truncation. fread reports both of these by returning 0; you have to use feof() or ferror() to distinguish between them, which this code did not do. It might be a better idea to use read() rather than fread() here, so that we can display a less-generic error message, but I'm not sure that would qualify as a back-patchable bug fix, so just do this much for now. Jeevan Chalke, reviewed by Jeevan Ladhe and by me. Discussion: http://postgr.es/m/CA+TgmobG4ywMzL5oQq2a8YKp8x2p3p1LOMMcGqpS7aekT9+ETA@mail.gmail.com
2019-09-04Fix thinko when ending progress report for a backendMichael Paquier
The logic ending progress reporting for a backend entry introduced by b6fb647 causes callers of pgstat_progress_end_command() to do some extra work when track_activities is enabled as the process fields are reset in the backend entry even if no command were started for reporting. This resets the fields only if a command is registered for progress reporting, and only if track_activities is enabled. Author: Masahiho Sawada Discussion: https://postgr.es/m/CAD21AoCry_vJ0E-m5oxJXGL3pnos-xYGCzF95rK5Bbi3Uf-rpA@mail.gmail.com Backpatch-through: 9.6
2019-09-02Handle corner cases correctly in psql's reconnection logic.Tom Lane
After an unexpected connection loss and successful reconnection, psql neglected to resynchronize its internal state about the server, such as server version. Ordinarily we'd be reconnecting to the same server and so this isn't really necessary, but there are scenarios where we do need to update --- one example is where we have a list of possible connection targets and they're not all alike. Define "resynchronize" as including connection_warnings(), so that this case acts the same as \connect. This seems useful; for example, if the server version did change, the user might wish to know that. An attuned user might also notice that the new connection isn't SSL-encrypted, for example, though this approach isn't especially in-your-face about such changes. Although this part is a behavioral change, it only affects interactive sessions, so it should not break any applications. Also, in do_connect, make sure that we desynchronize correctly when abandoning an old connection in non-interactive mode. These problems evidently are the result of people patching only one of the two places where psql deals with connection changes, so insert some cross-referencing comments in hopes of forestalling future bugs of the same ilk. Lastly, in Windows builds, issue codepage mismatch warnings only at startup, not during reconnections. psql's codepage can't change during a reconnect, so complaining about it again seems like useless noise. Peter Billen and Tom Lane. Back-patch to all supported branches. Discussion: https://postgr.es/m/CAMTXbE8e6U=EBQfNSe01Ej17CBStGiudMAGSOPaw-ALxM-5jXg@mail.gmail.com
2019-08-28Fix overflow check and comment in GIN posting list encoding.Heikki Linnakangas
The comment did not match what the code actually did for integers with the 43rd bit set. You get an integer like that, if you have a posting list with two adjacent TIDs that are more than 2^31 blocks apart. According to the comment, we would store that in 6 bytes, with no continuation bit on the 6th byte, but in reality, the code encodes it using 7 bytes, with a continuation bit on the 6th byte as normal. The decoding routine also handled these 7-byte integers correctly, except for an overflow check that assumed that one integer needs at most 6 bytes. Fix the overflow check, and fix the comment to match what the code actually does. Also fix the comment that claimed that there are 17 unused bits in the 64-bit representation of an item pointer. In reality, there are 64-32-11=21. Fitting any item pointer into max 6 bytes was an important property when this was written, because in the old pre-9.4 format, item pointers were stored as plain arrays, with 6 bytes for every item pointer. The maximum of 6 bytes per integer in the new format guaranteed that we could convert any page from the old format to the new format after upgrade, so that the new format was never larger than the old format. But we hardly need to worry about that anymore, and running into that problem during upgrade, where an item pointer is expanded from 6 to 7 bytes such that the data doesn't fit on a page anymore, is implausible in practice anyway. Backpatch to all supported versions. This also includes a little test module to test these large distances between item pointers, without requiring a 16 TB table. It is not backpatched, I'm including it more for the benefit of future development of new posting list formats. Discussion: https://www.postgresql.org/message-id/33bfc20a-5c86-f50c-f5a5-58e9925d05ff%40iki.fi Reviewed-by: Masahiko Sawada, Alexander Korotkov
2019-08-28Disable timeouts when running pg_rewind with online source clusterMichael Paquier
In this case, the transfer uses a libpq connection, which is subject to the timeout parameters set at system level, and this can make the rewind operation suddenly canceled which is not good for automation. One workaround to such issues would be to use PGOPTIONS to enforce the wanted timeout parameters, but that's annoying, and for example pg_dump, which can run potentially long-running queries disables all types of timeouts. lock_timeout and statement_timeout are the ones which can cause problems now. Note that pg_rewind does not use transactions, so disabling idle_in_transaction_session_timeout is optional, but it feels safer to do so for the future. This is back-patched down to 9.5. idle_in_transaction_session_timeout is only present since 9.6. Author: Alexander Kukushkin Discussion: https://postgr.es/m/CAFh8B=krcVXksxiwVQh1SoY+ziJ-JC=6FcuoBL3yce_40Es5_g@mail.gmail.com Backpatch-through: 9.5
2019-08-27Reject empty names and recursion in config-file include directives.Tom Lane
An empty file name or subdirectory name leads join_path_components() to just produce the parent directory name, which leads to weird failures or recursive inclusions. Let's throw a specific error for that. It takes only slightly more code to detect all-blank names, so do so. Also, detect direct recursion, ie a file calling itself. As coded this will also detect recursion via "include_dir '.'", which is perhaps more likely than explicitly including the file itself. Detecting indirect recursion would require API changes for guc-file.l functions, which seems not worth it since extensions might call them. The nesting depth limit will catch such cases eventually, just not with such an on-point error message. In passing, adjust the example usages in postgresql.conf.sample to perhaps eliminate the problem at the source: there's no reason for the examples to suggest that an empty value is valid. Per a trouble report from Brent Bates. Back-patch to 9.5; the issue is old, but the code in 9.4 is enough different that the patch doesn't apply easily, and it doesn't seem worth the trouble to fix there. Ian Barwick and Tom Lane Discussion: https://postgr.es/m/8c8bcbca-3bd9-dc6e-8986-04a5abdef142@2ndquadrant.com
2019-08-27Fix failure of --jobs with vacuumdb on WindowsMichael Paquier
FD_SETSIZE needs to be declared before winsock2.h, or it is possible to run into buffer overflow issues when using --jobs. This is similar to pgbench's solution done in a23c641. This has been introduced by 71d84ef, and older versions have been using the default value of FD_SETSIZE, defined at 64. While on it, add a missing newline to the previously-added error message. Per buildfarm member jacana, but this impacts all Windows animals running the TAP tests. I have reproduced the failure locally to check the patch. Author: Michael Paquier Reviewed-by: Andrew Dunstan Discussion: https://postgr.es/m/20190826054000.GE7005@paquier.xyz Backpatch-through: 9.5
2019-08-26Treat MINGW and MSYS the same in pg_upgrade test scriptAndrew Dunstan
On msys2, 'uname -s' reports a string starting MSYS instead on MINGW as happens on msys1. Treat these both the same way. This reverts 608a710195a4b in favor of a more general solution. Backpatch to all live branches.
2019-08-26Fix error handling of vacuumdb when running out of fdsMichael Paquier
When trying to use a high number of jobs, vacuumdb has only checked for a maximum number of jobs used, causing confusing failures when running out of file descriptors when the jobs open connections to Postgres. This commit changes the error handling so as we do not check anymore for a maximum number of allowed jobs when parsing the option value with FD_SETSIZE, but check instead if a file descriptor is within the supported range when opening the connections for the jobs so as this is detected at the earliest time possible. Also, improve the error message to give a hint about the number of jobs recommended, using a wording given by the reviewers of the patch. Reported-by: Andres Freund Author: Michael Paquier Reviewed-by: Andres Freund, Álvaro Herrera, Tom Lane Discussion: https://postgr.es/m/20190818001858.ho3ev4z57fqhs7a5@alap3.anarazel.de Backpatch-through: 9.5
2019-08-25Avoid platform-specific null pointer dereference in psql.Tom Lane
POSIX permits getopt() to advance optind beyond argc when the last argv entry is an option that requires an argument and hasn't got one. It seems that no major platforms actually do that, but musl does, so that something like "psql -f" would crash with that libc. Add a check that optind is in range before trying to look at the possibly-bogus option. Report and fix by Quentin Rameau. Back-patch to all supported branches. Discussion: https://postgr.es/m/20190825100617.GA6087@fifth.space
2019-08-21Fix typoAlvaro Herrera
In early development patches, "replication origins" were called "identifiers"; almost everything was renamed, but these references to the old terminology went unnoticed. Reported-by: Craig Ringer
2019-08-20Fix bogus commentAlvaro Herrera
Author: Alexander Lakhin Discussion: https://postgr.es/m/20190819072244.GE18166@paquier.xyz
2019-08-18Disallow changing an inherited column's type if not all parents changed.Tom Lane
If a table inherits from multiple unrelated parents, we must disallow changing the type of a column inherited from multiple such parents, else it would be out of step with the other parents. However, it's possible for the column to ultimately be inherited from just one common ancestor, in which case a change starting from that ancestor should still be allowed. (I would not be excited about preserving that option, were it not that we have regression test cases exercising it already ...) It's slightly annoying that this patch looks different from the logic with the same end goal in renameatt(), and more annoying that it requires an extra syscache lookup to make the test. However, the recursion logic is quite different in the two functions, and a back-patched bug fix is no place to be trying to unify them. Per report from Manuel Rigger. Back-patch to 9.5. The bug exists in 9.4 too (and doubtless much further back); but the way the recursion is done in 9.4 is a good bit different, so that substantial refactoring would be needed to fix it in 9.4. I'm disinclined to do that, or risk introducing new bugs, for a bug that has escaped notice for this long. Discussion: https://postgr.es/m/CA+u7OA4qogDv9rz1HAb-ADxttXYPqQdUdPY_yd4kCzywNxRQXA@mail.gmail.com
2019-08-15Prevent possible double-free when update trigger returns old tuple.Tom Lane
This is a variant of the problem fixed in commit 25b692568, which unfortunately we failed to detect at the time. If an update trigger returns the "old" tuple, as it's entitled to do, then a subsequent iteration of the loop in ExecBRUpdateTriggers would have "oldtuple" equal to "trigtuple" and would fail to notice that it shouldn't free that. In addition to fixing the code, extend the test case added by 25b692568 so that it covers multiple-trigger-iterations cases. This problem does not manifest in v12/HEAD, as a result of the relevant code having been largely rewritten for slotification. However, include the test case into v12/HEAD anyway, since this is clearly an area that someone could break again in future. Per report from Piotr Gabriel Kosinski. Back-patch into all supported branches, since the bug seems quite old. Diagnosis and code fix by Thomas Munro, test case by me. Discussion: https://postgr.es/m/CAFMLSdP0rd7LqC3j-H6Fh51FYSt5A10DDh-3=W4PPc4LLUQ8YQ@mail.gmail.com
2019-08-14Fix ALTER SYSTEM to cope with duplicate entries in postgresql.auto.conf.Tom Lane
ALTER SYSTEM itself normally won't make duplicate entries (although up till this patch, it was possible to confuse it by writing case variants of a GUC's name). However, if some external tool has appended entries to the file, that could result in duplicate entries for a single GUC name. In such a situation, ALTER SYSTEM did exactly the wrong thing, because it replaced or removed only the first matching entry, leaving the later one(s) still there and hence still determining the active value. This patch fixes that by making ALTER SYSTEM sweep through the file and remove all matching entries, then (if not ALTER SYSTEM RESET) append the new setting to the end. This means entries will be in order of last setting rather than first setting, but that shouldn't hurt anything. Also, make the comparisons case-insensitive so that the right things happen if you do, say, ALTER SYSTEM SET "TimeZone" = 'whatever'. This has been broken since ALTER SYSTEM was invented, so back-patch to all supported branches. Ian Barwick, with minor mods by me Discussion: https://postgr.es/m/aed6cc9f-98f3-2693-ac81-52bb0052307e@2ndquadrant.com
2019-08-13Un-break pg_dump for pre-8.3 source servers.Tom Lane
Commit 07b39083c inserted an unconditional reference to pg_opfamily, which of course fails on servers predating that catalog. Fortunately, the case it's trying to solve can't occur on such old servers (AFAIK). Hence, just skip the additional code when the source predates 8.3. Per bug #15955 from sly. Back-patch to all supported branches, like the previous patch. Discussion: https://postgr.es/m/15955-1daa2e676e903d87@postgresql.org
2019-08-12Fix compiler warningPeter Eisentraut
With some newer gcc versions (8 and 9) you get a -Wformat-overflow warning here. In PG11 and later this was already fixed. Since it's trivial, backport it to get the older branches building without warnings.
2019-08-07Fix predicate-locking of HOT updated rows.Heikki Linnakangas
In serializable mode, heap_hot_search_buffer() incorrectly acquired a predicate lock on the root tuple, not the returned tuple that satisfied the visibility checks. As explained in README-SSI, the predicate lock does not need to be copied or extended to other tuple versions, but for that to work, the correct, visible, tuple version must be locked in the first place. The original SSI commit had this bug in it, but it was fixed back in 2013, in commit 81fbbfe335. But unfortunately, it was reintroduced a few months later in commit b89e151054. Wising up from that, add a regression test to cover this, so that it doesn't get reintroduced again. Also, move the code that sets 't_self', so that it happens at the same time that the other HeapTuple fields are set, to make it more clear that all the code in the loop operate on the "current" tuple in the chain, not the root tuple. Bug spotted by Andres Freund, analysis and original fix by Thomas Munro, test case and some additional changes to the fix by Heikki Linnakangas. Backpatch to all supported versions (9.4). Discussion: https://www.postgresql.org/message-id/20190731210630.nqhszuktygwftjty%40alap3.anarazel.de
2019-08-07Fix some incorrect parsing of time with time zone stringsMichael Paquier
When parsing a timetz string with a dynamic timezone abbreviation or a timezone not specified, it was possible to generate incorrect timestamps based on a date which uses some non-initialized variables if the input string did not specify fully a date to parse. This is already checked when a full timezone spec is included in the input string, but the two other cases mentioned above missed the same checks. This gets fixed by generating an error as this input is invalid, or in short when a date is not fully specified. Valgrind was complaining about this problem. Bug: #15910 Author: Alexander Lakhin Discussion: https://postgr.es/m/15910-2eba5106b9aa0c61@postgresql.org Backpatch-through: 9.4
2019-08-05Stamp 9.6.15.REL9_6_15Tom Lane
2019-08-05Require the schema qualification in pg_temp.type_name(arg).Noah Misch
Commit aa27977fe21a7dfa4da4376ad66ae37cb8f0d0b5 introduced this restriction for pg_temp.function_name(arg); do likewise for types created in temporary schemas. Programs that this breaks should add "pg_temp." schema qualification or switch to arg::type_name syntax. Back-patch to 9.4 (all supported versions). Reviewed by Tom Lane. Reported by Tom Lane. Security: CVE-2019-10208
2019-08-05Translation updatesPeter Eisentraut
Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git Source-Git-Hash: 9e5626e1043405479299c3e23754614b43ad4d0f
2019-08-05Fix tab completion for ALTER LANGUAGE in psqlMichael Paquier
OWNER_TO was used for the completion, which is not a supported grammar, but OWNER TO is. This error has been introduced by d37b816, so backpatch down to 9.6. Author: Alexander Lakhin Discussion: https://postgr.es/m/7ab243e0-116d-3e44-d120-76b3df7abefd@gmail.com Backpatch-through: 9.6
2019-07-31Fix pg_dump's handling of dependencies for custom opclasses.Tom Lane
Since pg_dump doesn't treat the member operators and functions of operator classes/families (that is, the pg_amop and pg_amproc entries, not the underlying operators/functions) as separate dumpable objects, it missed their dependency information. I think this was safe when the code was designed, because the default object sorting rule emits operators and functions before opclasses, and there were no dependency types that could mess that up. However, the introduction of range types in 9.2 broke it: now a type can have a dependency on an opclass, allowing dependency rules to push the opclass before the type and hence before custom operators. Lacking any information showing that it shouldn't do so, pg_dump emitted the objects in the wrong order. Fix by teaching getDependencies() to translate pg_depend entries for pg_amop/amproc rows to look like dependencies for their parent opfamily. I added a regression test for this in HEAD/v12, but not further back; life is too short to fight with 002_pg_dump.pl. Per bug #15934 from Tom Gottfried. Back-patch to all supported branches. Discussion: https://postgr.es/m/15934-58b8c8ab7a09ea15@postgresql.org
2019-07-30Print WAL position correctly in pg_rewind error message.Heikki Linnakangas
This has been wrong ever since pg_rewind was added. The if-branch just above this, where we print the same error with an extra message supplied by XLogReadRecord() got this right, but the variable name was wrong in the else-branch. As a consequence, the error printed the WAL position as 0/0 if there was an error reading a WAL file. Backpatch to 9.5, where pg_rewind was added.
2019-07-29Fix busted logic for parallel lock grouping in TopoSort().Tom Lane
A "break" statement erroneously left behind by commit a1c1af2a1 caused TopoSort to do the wrong thing if a lock's wait list contained multiple members of the same locking group. Because parallel workers don't normally need any locks not already taken by their leader, this is very hard --- maybe impossible --- to hit in production. Still, if it did happen, the queries involved in an otherwise-resolvable deadlock would block until canceled. In addition to removing the bogus "break", add an Assert showing that the conflicting uses of the beforeConstraints[] array (for both counts and flags) don't overlap, and add some commentary explaining why not; because it's not obvious without explanation, IMHO. Original report and patch from Rui Hai Jiang; additional assert and commentary by me. Back-patch to 9.6 where the bug came in. Discussion: https://postgr.es/m/CAEri+mLd3bpHLyW+a9pSe1y=aEkeuJpwBSwvo-+m4n7-ceRmXw@mail.gmail.com
2019-07-26Don't uselessly escape a string that doesn't need escapingAlvaro Herrera
Per gripe from Ian Barwick Co-authored-by: Ian Barwick <ian@2ndquadrant.com> Discussion: https://postgr.es/m/CABvVfJWNnNKb8cHsTLhkTsvL1+G6BVcV+57+w1JZ61p8YGPdWQ@mail.gmail.com
2019-07-26Tweak our special-case logic for the IANA "Factory" timezone.Tom Lane
pg_timezone_names() tries to avoid showing the "Factory" zone in the view, mainly because that has traditionally had a very long "abbreviation" such as "Local time zone must be set--see zic manual page", so that showing it messes up psql's formatting of the whole view. Since tzdb version 2016g, IANA instead uses the abbreviation "-00", which is sane enough that there's no reason to discriminate against it. On the other hand, it emerges that FreeBSD and possibly other packagers are so wedded to backwards compatibility that they hack the IANA data to keep the old spelling --- and not just that old spelling, but even older spellings that IANA used back in the stone age. This caused the filter logic to fail to suppress "Factory" at all on such platforms, though the formatting problem is definitely real in that case. To solve both problems, get rid of the hard-wired assumption about exactly what Factory's abbreviation is, and instead reject abbreviations exceeding 31 characters. This will allow Factory to appear in the view if and only if it's using the modern abbreviation. In passing, simplify the code we add to zic.c to support "zic -P" to remove its now-obsolete hacks to not print the Factory zone's abbreviation. Unlike pg_timezone_names(), there's no reason for that code to support old/nonstandard timezone data. Since we generally prefer to keep timezone-related behavior the same in all branches, and since this is arguably a bug fix, back-patch to all supported branches. Discussion: https://postgr.es/m/3961.1564086915@sss.pgh.pa.us
2019-07-26Avoid choosing "localtime" or "posixrules" as TimeZone during initdb.Tom Lane
Some platforms create a file named "localtime" in the system timezone directory, making it a copy or link to the active time zone file. If Postgres is built with --with-system-tzdata, initdb will see that file as an exact match to localtime(3)'s behavior, and it may decide that "localtime" is the most preferred spelling of the active zone. That's a very bad choice though, because it's neither informative, nor portable, nor stable if someone changes the system timezone setting. Extend the preference logic added by commit e3846a00c so that we will prefer any other zone file that matches localtime's behavior over "localtime". On the same logic, also discriminate against "posixrules", which is another not-really-a-zone file that is often present in the timezone directory. (Since we install "posixrules" but not "localtime", this change can affect the behavior of Postgres with or without --with-system-tzdata.) Note that this change doesn't prevent anyone from choosing these pseudo-zones if they really want to (i.e., by setting TZ for initdb, or modifying the timezone GUC later on). It just prevents initdb from preferring these zone names when there are multiple matches to localtime's behavior. Since we generally prefer to keep timezone-related behavior the same in all branches, and since this is arguably a bug fix, back-patch to all supported branches. Discussion: https://postgr.es/m/CADT4RqCCnj6FKLisvT8tTPfTP4azPhhDFJqDF1JfBbOH5w4oyQ@mail.gmail.com Discussion: https://postgr.es/m/27991.1560984458@sss.pgh.pa.us
2019-07-26Fix loss of fractional digits for large values in cash_numeric().Tom Lane
Money values exceeding about 18 digits (depending on lc_monetary) could be inaccurately converted to numeric, due to select_div_scale() deciding it didn't need to compute any fractional digits. Force its hand by setting the dscale of one division input to equal the number of fractional digits we need. In passing, rearrange the logic to not do useless work in locales where money values are considered integral. Per bug #15925 from Slawomir Chodnicki. Back-patch to all supported branches. Discussion: https://postgr.es/m/15925-da9953e2674bb5c8@postgresql.org
2019-07-25Fix syntax error in commit 20e99cddd.Tom Lane
Per buildfarm.
2019-07-25Fix failures to ignore \r when reading Windows-style newlines.Tom Lane
libpq failed to ignore Windows-style newlines in connection service files. This normally wasn't a problem on Windows itself, because fgets() would convert \r\n to just \n. But if libpq were running inside a program that changes the default fopen mode to binary, it would see the \r's and think they were data. In any case, it's project policy to ignore \r in text files unconditionally, because people sometimes try to use files with DOS-style newlines on Unix machines, where the C library won't hide that from us. Hence, adjust parseServiceFile() to ignore \r as well as \n at the end of the line. In HEAD, go a little further and make it ignore all trailing whitespace, to match what it's always done with leading whitespace. In HEAD, also run around and fix up everyplace where we have newline-chomping code to make all those places look consistent and uniformly drop \r. It is not clear whether any of those changes are fixing live bugs. Most of the non-cosmetic changes are in places that are reading popen output, and the jury is still out as to whether popen on Windows can return \r\n. (The Windows-specific code in pipe_read_line seems to think so, but our lack of support for this elsewhere suggests maybe it's not a problem in practice.) Hence, I desisted from applying those changes to back branches, except in run_ssl_passphrase_command() which is new enough and little-tested enough that we'd probably not have heard about any problems there. Tom Lane and Michael Paquier, per bug #15827 from Jorge Gustavo Rocha. Back-patch the parseServiceFile() change to all supported branches, and the run_ssl_passphrase_command() change to v11 where that was added. Discussion: https://postgr.es/m/15827-e6ba53a3a7ed543c@postgresql.org
2019-07-25Honor MSVC WindowsSDKVersion if setAndrew Dunstan
Add a line to the project file setting the target SDK. Otherwise, in for example VS2017, if the default but optional 8.1 SDK is not installed the build will fail. Patch from Peifeng Qiu, slightly edited by me. Discussion: https://postgr.es/m/CABmtVJhw1boP_bd4=b3Qv5YnqEdL696NtHFi2ruiyQ6mFHkeQQ@mail.gmail.com Backpatch to all live branches.
2019-07-25Fix failure with pgperlcritic from the TAP test of synchronous replicationMichael Paquier
Oversight in 7d81bdc, which introduced a new routine in perl lacking a return clause. Per buildfarm member crake. Backpatch down to 9.6 like its parent. Reported-by: Andrew Dunstan Discussion: https://postgr.es/m/16da29fa-d504-1380-7095-40de586dc038@2ndQuadrant.com Backpatch-through: 9.6