aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* Fix overflow check and comment in GIN posting list encoding.Heikki Linnakangas2019-08-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Avoid catalog lookups in RelationAllowsEarlyPruning().Thomas Munro2019-08-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | RelationAllowsEarlyPruning() performed a catalog scan, but is used in two contexts where that was a bad idea: 1. In heap_page_prune_opt(), which runs very frequently in some large scans. This caused major performance problems in a field report that was easy to reproduce. 2. In TestForOldSnapshot(), which runs while we hold a buffer content lock. It's not clear if this was guaranteed to be free of buffer deadlock risk. The check was introduced in commit 2cc41acd8 and defended against a real problem: 9.6's hash indexes have no page LSN and so we can't allow early pruning (ie the snapshot-too-old feature). We can remove the check from all later releases though: hash indexes are now logged, and there is no way to create UNLOGGED indexes on regular logged tables. If a future release allows such a combination, it might need to put a similar check in place, but it'll need some more thought. Back-patch to 10. Author: Thomas Munro Reviewed-by: Tom Lane, who spotted the second problem Discussion: https://postgr.es/m/CA%2BhUKGKT8oTkp5jw_U4p0S-7UG9zsvtw_M47Y285bER6a2gD%2Bg%40mail.gmail.com Discussion: https://postgr.es/m/CAA4eK1%2BWy%2BN4eE5zPm765h68LrkWc3Biu_8rzzi%2BOYX4j%2BiHRw%40mail.gmail.com
* Disable timeouts when running pg_rewind with online source clusterMichael Paquier2019-08-28
| | | | | | | | | | | | | | | | | | | | | | 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
* Improve what pg_strsignal prints if we haven't got strsignal(3).Tom Lane2019-08-27
| | | | | | | | | | Turns out that returning "unrecognized signal" is confusing. Make it explicit that the platform lacks any support for signal names. (At least of the machines in the buildfarm, only HPUX lacks it.) Back-patch to v12 where we invented this function. Discussion: https://postgr.es/m/3067.1566870481@sss.pgh.pa.us
* Reject empty names and recursion in config-file include directives.Tom Lane2019-08-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Fix failure of --jobs with vacuumdb on WindowsMichael Paquier2019-08-27
| | | | | | | | | | | | | | | | | | 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. 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
* Fix gettext triggers specificationPeter Eisentraut2019-08-26
| | | | | In cc8d41511721d25d557fc02a46c053c0a602fed0, the arguments of warn_or_exit_horribly() were changed but this was not updated.
* Adjust to latest Msys2 kernel release numberAndrew Dunstan2019-08-26
| | | | | | | | | Previously 'uname -r' on Msys2 reported a kernele release starting with 2. The latest version starts with 3. In commit 1638623f we specifically looked for one starting with 2. This is now changed to look for any digit between 2 and 9. backpatch to release 10.
* Treat MINGW and MSYS the same in pg_upgrade test scriptAndrew Dunstan2019-08-26
| | | | | | | | 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.
* Fix error handling of vacuumdb when running out of fdsMichael Paquier2019-08-26
| | | | | | | | | | | | | | | | | | | | 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
* Avoid platform-specific null pointer dereference in psql.Tom Lane2019-08-25
| | | | | | | | | | | | | | 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
* Back off output precision in circle.sql regression test.Tom Lane2019-08-25
| | | | | | | | | | | | | | | | We were setting extra_float_digits = 0 to avoid platform-dependent output in this test, but that's still able to expose platform-specific roundoff behavior in some new test cases added by commit a3d284485, as reported by Peter Eisentraut. Reduce it to -1 to hide that. (Over in geometry.sql, we're using -3, which is an ancient decision dating to 337f73b1b. I wonder whether that's overkill now. But there's probably little value in trying to change it.) Back-patch to v12 where a3d284485 came in; there's no evidence that we have any platform-dependent issues here before that. Discussion: https://postgr.es/m/15551268-e224-aa46-084a-124b64095ee3@2ndquadrant.com
* Don't rely on llvm::make_unique.Thomas Munro2019-08-25
| | | | | | | | | | | | Bleeding-edge LLVM has stopped supplying replacements for various C++14 library features, for people on older C++ versions. Since we're not ready to require C++14 yet, just use plain old new instead of make_unique. As revealed by buildfarm animal seawasp. Back-patch to 11. Reviewed-by: Andres Freund Discussion: https://postgr.es/m/CA%2BhUKGJWG7unNqmkxg7nC5o3o-0p2XP6co4r%3D9epqYMm8UY4Mw%40mail.gmail.com
* Make SQL/JSON error code names match SQL standardPeter Eisentraut2019-08-22
| | | | | | There were some minor differences that didn't seem necessary. Discussion: https://www.postgresql.org/message-id/flat/86b67eef-bb26-c97d-3e35-64f1fbd4f9fe%402ndquadrant.com
* Fix typoAlvaro Herrera2019-08-21
| | | | | | | | 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
* Fix bogus commentAlvaro Herrera2019-08-20
| | | | | Author: Alexander Lakhin Discussion: https://postgr.es/m/20190819072244.GE18166@paquier.xyz
* Restore json{b}_populate_record{set}'s ability to take type info from AS.Tom Lane2019-08-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If the record argument is NULL and has no declared type more concrete than RECORD, we can't extract useful information about the desired rowtype from it. In this case, see if we're in FROM with an AS clause, and if so extract the needed rowtype info from AS. It worked like this before v11, but commit 37a795a60 removed the behavior, reasoning that it was undocumented, inefficient, and utterly not self-consistent. If you want to take type info from an AS clause, you should be using the json_to_record() family of functions not the json_populate_record() family. Also, it was already the case that the "populate" functions would fail for a null-valued RECORD input (with an unfriendly "record type has not been registered" error) when there wasn't an AS clause at hand, and it wasn't obvious that that behavior wasn't OK when there was one. However, it emerges that some people were depending on this to work, and indeed the rather off-point error message you got if you left off AS encouraged slapping on AS without switching to the json_to_record() family. Hence, put back the fallback behavior of looking for AS. While at it, improve the run-time error you get when there's no place to obtain type info; we can do a lot better than "record type has not been registered". (We can't, unfortunately, easily improve the parse-time error message that leads people down this path in the first place.) While at it, I refactored the code a bit to avoid duplicating the same logic in several different places. Per bug #15940 from Jaroslav Sivy. Back-patch to v11 where the current coding came in. (The pre-v11 deficiencies in this area aren't regressions, so we'll leave those branches alone.) Patch by me, based on preliminary analysis by Dmitry Dolgov. Discussion: https://postgr.es/m/15940-2ab76dc58ffb85b6@postgresql.org
* Disallow changing an inherited column's type if not all parents changed.Tom Lane2019-08-18
| | | | | | | | | | | | | | | | | | | | | | | | 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
* Add default_table_access_method to postgresql.conf.sample.Andres Freund2019-08-16
| | | | | | | Reported-By: Heikki Linnakangas Author: Michael Paquier Discussion: https://postgr.es/m/d6ffbebb-a0d2-181c-811d-b029b2225ed7@iki.fi Backpatch: 12-, where pluggable table access methods were introduced
* Prevent possible double-free when update trigger returns old tuple.Tom Lane2019-08-15
| | | | | | | | | | | | | | | | | | | | | | | | 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
* Fix plpgsql to re-look-up composite type names at need.Tom Lane2019-08-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 4b93f5799 rearranged things in plpgsql to make it cope better with composite types changing underneath it intra-session. However, I failed to consider the case of a composite type being dropped and recreated entirely. In my defense, the previous coding didn't consider that possibility at all either --- but it would accidentally work so long as you didn't change the type's field list, because the built-at-compile-time list of component variables would then still match the type's new definition. The new coding, however, occasionally tries to re-look-up the type by OID, and then fails to find the dropped type. To fix this, we need to save the TypeName struct, and then redo the type OID lookup from that. Of course that's expensive, so we don't want to do it every time we need the type OID. This can be fixed in the same way that 4b93f5799 dealt with changes to composite types' definitions: keep an eye on the type's typcache entry to see if its tupledesc has been invalidated. (Perhaps, at some point, this mechanism should be generalized so it can work for non-composite types too; but for now, plpgsql only tries to cope with intra-session redefinitions of composites.) I'm slightly hesitant to back-patch this into v11, because it changes the contents of struct PLpgSQL_type as well as the signature of plpgsql_build_datatype(), so in principle it could break code that is poking into the innards of plpgsql. However, the only popular extension of that ilk is pldebugger, and it doesn't seem to be affected. Since this is a regression for people who were relying on the old behavior, it seems worth taking the small risk of causing compatibility issues. Per bug #15913 from Daniel Fiori. Back-patch to v11 where 4b93f5799 came in. Discussion: https://postgr.es/m/15913-a7e112e16dedcffc@postgresql.org
* Fix ALTER SYSTEM to cope with duplicate entries in postgresql.auto.conf.Tom Lane2019-08-14
| | | | | | | | | | | | | | | | | | | | | | | | | 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
* Fix random regression failure in test case "collate.icu.utf8"Michael Paquier2019-08-14
| | | | | | | | | | | | | | This is a fix similar to 2d7d67cc, where slight plan alteration can cause a random failure of this regression test because of an incorect tuple ordering, except that this one involves lookups of pg_type. Similarly to the other case, add ORDER BY clauses to ensure the output order. The failure has been seen at least once on buildfarm member skink. Reported-by: Thomas Munro Discussion: https://postgr.es/m/CA+hUKGLjR9ZBvhXcr9b-NSBHPw9aRgbjyzGE+kqLsT4vwX+nkQ@mail.gmail.com Backpatch-through: 12
* Un-break pg_dump for pre-8.3 source servers.Tom Lane2019-08-13
| | | | | | | | | | | | 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
* Fix random regression failure in test case "temp"Michael Paquier2019-08-13
| | | | | | | | | | | | | | | | | | | | | | This test case could fail because of an incorrect result ordering when looking up at pg_class entries. This commit adds an ORDER BY to the culprit query. The cause of the failure was likely caused by a plan switch. By default, the planner would likely choose an index-only scan or an index scan, but even a small change in the startup cost could have caused a bitmap heap scan to be chosen, causing the failure. While on it, switch some filtering quals to a regular expression as per an idea of Tom Lane. As previously shaped, the quals would have selected any relations whose name begins with "temp". And that could cause failures if another test running in parallel began to use similar relation names. Per report from buildfarm member anole, though the failure was very rare. This test has been introduced by 319a810, so backpatch down to v10. Discussion: https://postgr.es/m/20190807132422.GC15695@paquier.xyz Backpatch-through: 10
* Fix planner's test for case-foldable characters in ILIKE with ICU.Tom Lane2019-08-12
| | | | | | | | | | | | | As coded, the ICU-collation path in pattern_char_isalpha() failed to consider regular ASCII letters to be case-varying. This led to like_fixed_prefix treating too much of an ILIKE pattern as being a fixed prefix, so that indexscans derived from an ILIKE clause might miss entries that they should find. Per bug #15892 from James Inform. This is an oversight in the original ICU patch (commit eccfef81e), so back-patch to v10 where that came in. Discussion: https://postgr.es/m/15892-e5d2bea3e8a04a1b@postgresql.org
* Fix string comparison in jsonpathAlexander Korotkov2019-08-12
| | | | | | | | | Take into account pg_server_to_any() may return input string "as is". Reported-by: Andrew Dunstan, Thomas Munro Discussion: https://postgr.es/m/0ed83a33-d900-466a-880a-70ef456c721f%402ndQuadrant.com Author: Alexander Korotkov, Thomas Munro Backpatch-through: 12
* Adjust string comparison in jsonpathAlexander Korotkov2019-08-11
| | | | | | | | | | | | | | | | | | | | We have implemented jsonpath string comparison using default database locale. However, standard requires us to compare Unicode codepoints. This commit implements that, but for performance reasons we still use per-byte comparison for "==" operator. Thus, for consistency other comparison operators do per-byte comparison if Unicode codepoints appear to be equal. In some edge cases, when same Unicode codepoints have different binary representations in database encoding, we diverge standard to achieve better performance of "==" operator. In future to implement strict standard conformance, we can do normalization of input JSON strings. Original patch was written by Nikita Glukhov, rewritten by me. Reported-by: Markus Winand Discussion: https://postgr.es/m/8B7FA3B4-328D-43D7-95A8-37B8891B8C78%40winand.at Author: Nikita Glukhov, Alexander Korotkov Backpatch-through: 12
* Fix "ANALYZE t, t" inside a transaction block.Tom Lane2019-08-10
| | | | | | | | | | | | | | | | | | | | | | | | | | This failed with either "tuple already updated by self" or "duplicate key value violates unique constraint", depending on whether the table had previously been analyzed or not. The reason is that ANALYZE tried to insert or update the same pg_statistic rows twice, and there was no CommandCounterIncrement between. So add one. The same case works fine outside a transaction block, because then there's a whole transaction boundary between, as a consequence of the way VACUUM works. This issue has been latent all along, but the problem was unreachable before commit 11d8d72c2 added the ability to specify multiple tables in ANALYZE. We could, perhaps, alternatively fix it by adding code to de-duplicate the list of VacuumRelations --- but that would add a lot of overhead to work around dumb commands, so it's not attractive. Per bug #15946 from Yaroslav Schekin. Back-patch to v11. (Note: in v11 I also back-patched the test added by commit 23224563d; otherwise the problem doesn't manifest in the test I added, because "vactst" is empty when the tests for multiple ANALYZE targets are reached. That seems like not a very good thing anyway, so I did this rather than rethinking the choice of test case.) Discussion: https://postgr.es/m/15946-5c7570a2884a26cf@postgresql.org
* Fix SIGSEGV in pruning for ScalarArrayOp with constant-null array.Tom Lane2019-08-09
| | | | | | | | | | Not much to be said here: commit 9fdb675fc should have checked constisnull, didn't. Per report from Piotr Włodarczyk. Back-patch to v11 where bug was introduced. Discussion: https://postgr.es/m/CAP-dhMr+vRpwizEYjUjsiZ1vwqpohTm+3Pbdt6Pr7FEgPq9R0Q@mail.gmail.com
* Fix certificate subjects in ldap testAndrew Dunstan2019-08-08
| | | | | | | openssl doesn't like lower case subject attribute names. Error observed in buildfarm results. Backpatch to release 11.
* Fix typos in comments.Etsuro Fujita2019-08-07
|
* Fix predicate-locking of HOT updated rows.Heikki Linnakangas2019-08-07
| | | | | | | | | | | | | | | | | | | | | | | 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
* Fix some incorrect parsing of time with time zone stringsMichael Paquier2019-08-07
| | | | | | | | | | | | | | | | | | | 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
* Save Kerberos and LDAP daemon logs where the buildfarm can find them.Tom Lane2019-08-06
| | | | | | | | | | | | | src/test/kerberos and src/test/ldap try to run private authentication servers, which of course might fail. The logs from these servers were being dropped into the tmp_check/ subdirectory, but they should be put in tmp_check/log/, because the buildfarm will only capture log files in that subdirectory. Without the log output there's little hope of diagnosing buildfarm failures related to these servers. Backpatch to v11 where these test suites were added. Discussion: https://postgr.es/m/16017.1565047605@sss.pgh.pa.us
* Stamp 12beta3.REL_12_BETA3Tom Lane2019-08-05
|
* Fix choice of comparison operators for cross-type hashed subplans.Tom Lane2019-08-05
| | | | | | | | | | | | | | | | | | | | | | Commit bf6c614a2 rearranged the lookup of the comparison operators needed in a hashed subplan, and in so doing, broke the cross-type case: it caused the original LHS-vs-RHS operator to be used to compare hash table entries too (which of course are all of the RHS type). This leads to C functions being passed a Datum that is not of the type they expect, with the usual hazards of crashes and unauthorized server memory disclosure. For the set of hashable cross-type operators present in v11 core Postgres, this bug is nearly harmless on 64-bit machines, which may explain why it escaped earlier detection. But it is a live security hazard on 32-bit machines; and of course there may be extensions that add more hashable cross-type operators, which would increase the risk. Reported by Andreas Seltenreich. Back-patch to v11 where the problem came in. Security: CVE-2019-10209
* Require the schema qualification in pg_temp.type_name(arg).Noah Misch2019-08-05
| | | | | | | | | | | | 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
* Translation updatesPeter Eisentraut2019-08-05
| | | | | Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git Source-Git-Hash: e255bc8b15d0f173f9de9048d3d6ad6e40085a48
* Fix tab completion for ALTER LANGUAGE in psqlMichael Paquier2019-08-05
| | | | | | | | | | | 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
* Revert "Add log_statement_sample_rate parameter"Tomas Vondra2019-08-04
| | | | | | | | | | | | | | This reverts commit 88bdbd3f746049834ae3cc972e6e650586ec3c9d. As committed, statement sampling used the existing duration threshold (log_min_duration_statement) when decide which statements to sample. The issue is that even the longest statements are subject to sampling, and so may not end up logged. An improvement was proposed, introducing a second duration threshold, but it would not be backwards compatible. So we've decided to revert this feature - the separate threshold should be part of the feature itself. Discussion: https://postgr.es/m/CAFj8pRDS8tQ3Wviw9%3DAvODyUciPSrGeMhJi_WPE%2BEB8%2B4gLL-Q%40mail.gmail.com
* Revert "Silence compiler warning"Tomas Vondra2019-08-04
| | | | | | | | | | | | | | This reverts commit 9dc122585551516309c9362e673effdbf3bd79bd. As committed, statement sampling used the existing duration threshold (log_min_duration_statement) when decide which statements to sample. The issue is that even the longest statements are subject to sampling, and so may not end up logged. An improvement was proposed, introducing a second duration threshold, but it would not be backwards compatible. So we've decided to revert this feature - the separate threshold should be part of the feature itself. Discussion: https://postgr.es/m/CAFj8pRDS8tQ3Wviw9%3DAvODyUciPSrGeMhJi_WPE%2BEB8%2B4gLL-Q%40mail.gmail.com
* Avoid picking already-bound TCP ports in kerberos and ldap test suites.Tom Lane2019-08-04
| | | | | | | | | | | | | | | | | | | | | | | src/test/kerberos and src/test/ldap need to run a private authentication server of the relevant type, for which they need a free TCP port. They were just picking a random port number in 48K-64K, which works except when something's already using the particular port. Notably, the probability of failure rises dramatically if one simply runs those tests in a tight loop, because each test cycle leaves behind a bunch of high ports that are transiently in TIME_WAIT state. To fix, split out the code that PostgresNode.pm already had for identifying a free TCP port number, so that it can be invoked to choose a port for the KDC or LDAP server. This isn't 100% bulletproof, since conceivably something else on the machine could grab the port between the time we check and the time we actually start the server. But that's a pretty short window, so in practice this should be good enough. Back-patch to v11 where these test suites were added. Patch by me, reviewed by Andrew Dunstan. Discussion: https://postgr.es/m/3397.1564872168@sss.pgh.pa.us
* Improve pruning of a default partitionAlvaro Herrera2019-08-04
| | | | | | | | | | | | | | | | | | | When querying a partitioned table containing a default partition, we were wrongly deciding to include it in the scan too early in the process, failing to exclude it in some cases. If we reinterpret the PruneStepResult.scan_default flag slightly, we can do a better job at detecting that it can be excluded. The change is that we avoid setting the flag for that pruning step unless the step absolutely requires the default partition to be scanned (in contrast with the previous arrangement, which was to set it unless the step was able to prune it). So get_matching_partitions() must explicitly check the partition that each returned bound value corresponds to in order to determine whether the default one needs to be included, rather than relying on the flag from the final step result. Author: Yuzuko Hosoya <hosoya.yuzuko@lab.ntt.co.jp> Reviewed-by: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> Discussion: https://postgr.es/m/00e601d4ca86$932b8bc0$b982a340$@lab.ntt.co.jp
* Fix representation of hash keys in Hash/HashJoin nodes.Andres Freund2019-08-02
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In 5f32b29c1819 I changed the creation of HashState.hashkeys to actually use HashState as the parent (instead of HashJoinState, which was incorrect, as they were executed below HashState), to fix the problem of hashkeys expressions otherwise relying on slot types appropriate for HashJoinState, rather than HashState as would be correct. That reliance was only introduced in 12, which is why it previously worked to use HashJoinState as the parent (although I'd be unsurprised if there were problematic cases). Unfortunately that's not a sufficient solution, because before this commit, the to-be-hashed expressions referenced inner/outer as appropriate for the HashJoin, not Hash. That didn't have obvious bad consequences, because the slots containing the tuples were put into ecxt_innertuple when hashing a tuple for HashState (even though Hash doesn't have an inner plan). There are less common cases where this can cause visible problems however (rather than just confusion when inspecting such executor trees). E.g. "ERROR: bogus varno: 65000", when explaining queries containing a HashJoin where the subsidiary Hash node's hash keys reference a subplan. While normally hashkeys aren't displayed by EXPLAIN, if one of those expressions references a subplan, that subplan may be printed as part of the Hash node - which then failed because an inner plan was referenced, and Hash doesn't have that. It seems quite possible that there's other broken cases, too. Fix the problem by properly splitting the expression for the HashJoin and Hash nodes at plan time, and have them reference the proper subsidiary node. While other workarounds are possible, fixing this correctly seems easy enough. It was a pretty ugly hack to have ExecInitHashJoin put the expression into the already initialized HashState, in the first place. I decided to not just split inner/outer hashkeys inside make_hashjoin(), but also to separate out hashoperators and hashcollations at plan time. Otherwise we would have ended up having two very similar loops, one at plan time and the other during executor startup. The work seems to more appropriately belong to plan time, anyway. Reported-By: Nikita Glukhov, Alexander Korotkov Author: Andres Freund Reviewed-By: Tom Lane, in an earlier version Discussion: https://postgr.es/m/CAPpHfdvGVegF_TKKRiBrSmatJL2dR9uwFCuR+teQ_8tEXU8mxg@mail.gmail.com Backpatch: 12-
* Fix pg_dump's handling of dependencies for custom opclasses.Tom Lane2019-07-31
| | | | | | | | | | | | | | | | | | | | | | | | 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
* Remove superfluous newlines in function prototypes.Andres Freund2019-07-31
| | | | | | | | | | | | | | | These were introduced by pgindent due to fixe to broken indentation (c.f. 8255c7a5eeba8). Previously the mis-indentation of function prototypes was creatively used to reduce indentation in a few places. As that formatting only exists in master and REL_12_STABLE, it seems better to fix it in both, rather than having some odd indentation in v12 that somebody might copy for future patches or such. Author: Andres Freund Discussion: https://postgr.es/m/20190728013754.jwcbe5nfyt3533vx@alap3.anarazel.de Backpatch: 12-
* Allow table AM's to use rd_amcache, too.Heikki Linnakangas2019-07-30
| | | | | | | | | | | The rd_amcache allows an index AM to cache arbitrary information in a relcache entry. This commit moves the cleanup of rd_amcache so that it can also be used by table AMs. Nothing takes advantage of that yet, but I'm sure it'll come handy for anyone writing new table AMs. Backpatch to v12, where table AM interface was introduced. Reviewed-by: Julien Rouhaud
* Print WAL position correctly in pg_rewind error message.Heikki Linnakangas2019-07-30
| | | | | | | | | | 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.
* Don't build extended statistics on inheritance treesTomas Vondra2019-07-30
| | | | | | | | | | | | | | | | | | | | | | | | | | When performing ANALYZE on inheritance trees, we collect two samples for each relation - one for the relation alone, and one for the inheritance subtree (relation and its child relations). And then we build statistics on each sample, so for each relation we get two sets of statistics. For regular (per-column) statistics this works fine, because the catalog includes a flag differentiating statistics built from those two samples. But we don't have such flag in the extended statistics catalogs, and we ended up updating the same row twice, triggering this error: ERROR: tuple already updated by self The simplest solution is to disable extended statistics on inheritance trees, which is what this commit is doing. In the future we may need to do something similar to per-column statistics, but that requires adding a flag to the catalog - and that's not backpatchable. Moreover, the current selectivity estimation code only works with individual relations, so building statistics on inheritance trees would be pointless anyway. Author: Tomas Vondra Backpatch-to: 10- Discussion: https://postgr.es/m/20190618231233.GA27470@telsasoft.com Reported-by: Justin Pryzby