aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
* Update SQL conformance informationPeter Eisentraut2019-08-22
| | | | | T612 has been fully supported since the major window function enhancements in PostgreSQL 11, but it wasn't updated at the time.
* 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
* Update comments on nbtree stack struct.Peter Geoghegan2019-08-21
| | | | | | | | Adjust the struct comment that describes how page splits use their descent stack to cascade up the tree from the leaf level. In passing, fix up some unrelated nbtree comments that had typos or were obsolete.
* Remove configure detection of crypt()Peter Eisentraut2019-08-21
| | | | | | | | crypt() hasn't been needed since crypt detection was removed from PostgreSQL, so these configure checks are not necessary. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/21f88934-f00c-27f6-a9d8-7ea06d317781%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
* Clean up some SCRAM attribute processingPeter Eisentraut2019-08-20
| | | | | | | | | Correct the comment for read_any_attr(). Give a clearer error message when parsing at the end of the string, when the client-final-message does not contain a "p" attribute (for some reason). Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://www.postgresql.org/message-id/flat/2fb8a15b-de35-682d-a77b-edcc9c52fa12%402ndquadrant.com
* 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
* Fix inconsistencies and typos in the tree, take 11Michael Paquier2019-08-19
| | | | | | | | This fixes various typos in docs and comments, and removes some orphaned definitions. Author: Alexander Lakhin Discussion: https://postgr.es/m/5da8e325-c665-da95-21e0-c8a99ea61fbf@gmail.com
* Avoid conflicts with library versions of inet_net_ntop() and friends.Tom Lane2019-08-18
| | | | | | | | | | | | | | Prefix inet_net_ntop and sibling routines with "pg_" to ensure that they aren't mistaken for C-library functions. This fixes warnings from cpluspluscheck on some platforms, and should help reduce reader confusion everywhere, since our functions aren't exactly interchangeable with the library versions (they may have different ideas about address family codes). This shouldn't be fixing any actual bugs, unless somebody's linker is misbehaving, so no need to back-patch. Discussion: https://postgr.es/m/20518.1559494394@sss.pgh.pa.us
* Fix incidental warnings from cpluspluscheck.Tom Lane2019-08-18
| | | | | | | | | | Remove use of "register" keyword in hashfn.c. It's obsolescent according to recent C++ compilers, and no modern C compiler pays much attention to it either. Also fix one cosmetic warning about signed vs unsigned comparison. Discussion: https://postgr.es/m/20518.1559494394@sss.pgh.pa.us
* 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
* Remove fmgr.h includes from headers that don't really need it.Andres Freund2019-08-16
| | | | | | | | | Most of the fmgr.h includes were obsoleted by 352a24a1f9d6f7d4abb1. A few others can be obsoleted using the underlying struct type in an implementation detail. Author: Andres Freund Discussion: https://postgr.es/m/20190803193733.g3l3x3o42uv4qj7l@alap3.anarazel.de
* Don't include utils/array.h from acl.h.Andres Freund2019-08-16
| | | | | | | | | | | | | | | | | | For most uses of acl.h the details of how "Acl" internally looks like are irrelevant. It might make sense to move a lot of the implementation details into a separate header at a later point. The main motivation of this change is to avoid including fmgr.h (via array.h, which needs it for exposed structs) in a lot of files that otherwise don't need it. A subsequent commit will remove the fmgr.h include from a lot of files. Directly include utils/array.h and utils/expandeddatum.h from the files that need them, but previously included them indirectly, via acl.h. Author: Andres Freund Discussion: https://postgr.es/m/20190803193733.g3l3x3o42uv4qj7l@alap3.anarazel.de
* Remove useless bms_free() calls in build_child_join_rel().Etsuro Fujita2019-08-16
| | | | | | | These seem to be leftovers from the original partitionwise-join patch, perhaps. Discussion: https://postgr.es/m/CAPmGK145YiMTPRnvev1dLz8na_-0aZ=Xyqn8f2QsJFBUTObNow@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
* Use a hash table to de-duplicate NOTIFY events faster.Tom Lane2019-08-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, async.c got rid of duplicate notifications by scanning the list of pending events to compare each one to the proposed new event. This works okay for very small numbers of distinct events, but degrades as O(N^2) for many events. We can improve matters by using a hash table to probe for duplicates. So as not to add a lot of overhead for the simple cases that the code did handle well before, create the hash table only once a (sub)transaction has queued more than 16 distinct notify events. A downside is that we now have to do per-event work to propagate a successful subtransaction's notify events up to its parent. (But this isn't significant unless the subtransaction had many events, in which case the O(N^2) behavior would have been in play already, so we still come out ahead.) We can make some lemonade out of this lemon, though: since we must examine each event anyway, it's now possible to de-duplicate events fully, rather than skipping that for events merged up from subtransactions. Hence, remove the old weasel wording in notify.sgml about whether de-duplication happens or not, and adjust the test case in async-notify.spec that exhibited the old behavior. While at it, rearrange the definition of struct Notification to make it more compact and require just one palloc per event, rather than two or three. This saves space when there are a lot of events, in fact more than enough to buy back the space needed for the hash table. Patch by me, based on discussions around a different patch submitted by Filip Rembiałkowski. Discussion: https://postgr.es/m/17822.1564186806@sss.pgh.pa.us
* 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
* Remove block number field from nbtree stack.Peter Geoghegan2019-08-14
| | | | | | | | | | | | | | The initial value of the nbtree stack downlink block number field recorded during an initial descent of the tree wasn't actually used. Both _bt_getstackbuf() callers overwrote the value with their own value. Remove the block number field from the stack struct, and add a child block number argument to _bt_getstackbuf() in its place. This makes the overall design of _bt_getstackbuf() clearer. Author: Peter Geoghegan Reviewed-By: Anastasia Lubennikova Discussion: https://postgr.es/m/CAH2-Wzmx+UbXt2YNOUCZ-a04VdXU=S=OHuAuD7Z8uQq-PXTYUg@mail.gmail.com
* initdb: Remove obsolete locale handlingPeter Eisentraut2019-08-14
| | | | | | | | | The method of passing LC_COLLATE and LC_CTYPE to the backend during initdb is obsolete as of 61d967498802ab86d8897cb3c61740d7e9d712f6. This can all be removed. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/eeaf2f99-a1a6-8aca-3f43-9ab0b2fb112a%402ndquadrant.com
* Remove obsolete nbtree README commentary.Peter Geoghegan2019-08-13
| | | | | | | | | | | | | Commit d2086b08b02 removed almost all cases where nbtree must release a read buffer lock and acquire a write buffer lock instead, so remaining cases in which that's still necessary are not notable enough to appear in the nbtree README. More importantly, holding on to a buffer pin in cases where nbtree must trade a read lock for a write lock is very unlikely to save any I/O. This seems to have been a long overlooked throwback to a time when nbtree cared about write-ordering dependencies, and performed synchronous buffer writes. It hasn't worked that way in many years.
* Use PageIndexTupleOverwrite() within nbtree.Peter Geoghegan2019-08-13
| | | | | | | | | | | | Use the PageIndexTupleOverwrite() bufpage.c routine within nbtree instead of deleting a tuple and re-inserting its replacement. This makes the intent of affected code slightly clearer. It also makes CREATE INDEX slightly faster, since there is no longer a need to shift every leaf page's line pointer array back and forth during index builds. Author: Peter Geoghegan, Anastasia Lubennikova Reviewed-By: Anastasia Lubennikova Discussion: https://postgr.es/m/CAH2-Wz=Zk=B9+Vwm376WuO7YTjFc2SSskifQm4Nme3RRRPtOSQ@mail.gmail.com
* Don't constraint-exclude partitioned tables as muchAlvaro Herrera2019-08-13
| | | | | | | | | | | | | | | | | We only need to invoke constraint exclusion on partitioned tables when they are a partition, and they themselves contain a default partition; it's not necessary otherwise, and it's expensive, so avoid it. Also, we were trying once for each clause separately, but we can do it for all the clauses at once. While at it, centralize setting of RelOptInfo->partition_qual instead of computing it in slightly different ways in different places. Per complaints from Simon Riggs about 4e85642d935e; reviewed by Yuzuko Hosoya, Kyotaro Horiguchi. Author: Amit Langote. I (Álvaro) again mangled the patch somewhat. Discussion: https://postgr.es/m/CANP8+j+tMCY=nEcQeqQam85=uopLBtX-2vHiLD2bbp7iQQUKpA@mail.gmail.com
* Fix inconsistencies and typos in the tree, take 10Michael Paquier2019-08-13
| | | | | | | | | This addresses some issues with unnecessary code comments, fixes various typos in docs and comments, and removes some orphaned structures and definitions. Author: Alexander Lakhin Discussion: https://postgr.es/m/9aabc775-5494-b372-8bcb-4dfc0bd37c68@gmail.com
* 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
* Remove EState.es_range_table_array.Tom Lane2019-08-12
| | | | | | | | | | Now that list_nth is O(1), there's no good reason to maintain a separate array of RTE pointers rather than indexing into estate->es_range_table. Deleting the array doesn't save all that much either; but just on cleanliness grounds, it's better not to have duplicate representations of the identical information. Discussion: https://postgr.es/m/14960.1565384592@sss.pgh.pa.us
* Rationalize use of list_concat + list_copy combinations.Tom Lane2019-08-12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | In the wake of commit 1cff1b95a, the result of list_concat no longer shares the ListCells of the second input. Therefore, we can replace "list_concat(x, list_copy(y))" with just "list_concat(x, y)". To improve call sites that were list_copy'ing the first argument, or both arguments, invent "list_concat_copy()" which produces a new list sharing no ListCells with either input. (This is a bit faster than "list_concat(list_copy(x), y)" because it makes the result list the right size to start with.) In call sites that were not list_copy'ing the second argument, the new semantics mean that we are usually leaking the second List's storage, since typically there is no remaining pointer to it. We considered inventing another list_copy variant that would list_free the second input, but concluded that for most call sites it isn't worth worrying about, given the relative compactness of the new List representation. (Note that in cases where such leakage would happen, the old code already leaked the second List's header; so we're only discussing the size of the leak not whether there is one. I did adjust two or three places that had been troubling to free that header so that they manually free the whole second List.) Patch by me; thanks to David Rowley for review. Discussion: https://postgr.es/m/11587.1550975080@sss.pgh.pa.us
* 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
* Rename tuplesort.c's SortTuple.tupindex field.Peter Geoghegan2019-08-09
| | | | | | | | Rename the "tupindex" field from tuplesort.c's SortTuple struct to "srctape", since it can only ever be used to store a source/input tape number when merging external sort runs. This has been the case since commit 8b304b8b72b, which removed replacement selection sort from tuplesort.c.
* 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
* Cosmetic improvements in setup of planner's per-RTE arrays.Tom Lane2019-08-09
| | | | | | | | | | | | | Merge setup_append_rel_array into setup_simple_rel_arrays. There's no particularly good reason to keep them separate, and it's inconsistent with the lack of separation in expand_planner_arrays. The only apparent benefit was that the fast path for trivial queries in query_planner() doesn't need to set up the append_rel_array; but all we're saving there is an if-test and NULL assignment, which surely ought to be negligible. Also improve some obsolete comments. Discussion: https://postgr.es/m/17220.1565301350@sss.pgh.pa.us
* Refactor logic to remove trailing CR/LF characters from stringsMichael Paquier2019-08-09
| | | | | | | | | | b654714 has reworked the way trailing CR/LF characters are removed from strings. This commit introduces a new routine in common/string.c and refactors the code so as the logic is in a single place, mostly. Author: Michael Paquier Reviewed-by: Bruce Momjian Discussion: https://postgr.es/m/20190801031820.GF29334@paquier.xyz
* Update obsolete tuplesort READTUP() comment.Peter Geoghegan2019-08-08
| | | | | | | | | READTUP() routines do not and cannot use the resettable "tuplecontext" memory context, since it is deleted when merging begins. Update an obsolete comment that claimed otherwise. This was an oversight in commit e94568ecc10. In passing, fix an unrelated tuplesort typo.
* Remove unnecessary #include <limits.h>Alvaro Herrera2019-08-07
| | | | | | | | This include was probably copied from tuplestore.c, but it's not needed. Extracted from a larger patch submitted by vignesh C <vignesh21@gmail.com> Discussion: https://postgr.es/m/CALDaNm1B9naPDTm3ox1m_yZvOm3KA5S4kZQSWWAeLHAQ=3gV1Q@mail.gmail.com
* Add comment on no default partition with hash partitioningAlvaro Herrera2019-08-07
| | | | Discussion: https://postgr.es/m/20190806222735.GA9535@alvherre.pgsql
* Apply constraint exclusion more generally in partitioningAlvaro Herrera2019-08-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | We were applying constraint exclusion on the partition constraint when generating pruning steps for a clause, but only for the rather restricted situation of them being boolean OR operators; however it is possible to have differently shaped clauses that also benefit from constraint exclusion. This applies particularly to the default partition since their constraints are in essence a long list of OR'ed subclauses ... but it applies to other cases too. So in certain cases we're scanning partitions that we don't need to. Remove the specialized code in OR clauses, and add a generally applicable test of the clause refuting the partition constraint; mark the whole pruning operation as contradictory if it hits. This has the unwanted side-effect of testing some (most? all?) constraints more than once if constraint_exclusion=on. That seems unavoidable as far as I can tell without some additional work, but that's not the recommended setting for that parameter anyway. However, because this imposes additional processing cost for all queries using partitioned tables, I decided not to backpatch this change. Author: Amit Langote, Yuzuko Hosoya, Álvaro Herrera Reviewers: Shawn Wang, Thibaut Madeleine, Yoshikazu Imai, Kyotaro Horiguchi; they were also uncredited reviewers for commit 489247b0e615. Discussion: https://postgr.es/m/9bb31dfe-b0d0-53f3-3ea6-e64b811424cf@lab.ntt.co.jp
* 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
* Adjust tuple data lookup logic in multi-insert logical decodingMichael Paquier2019-08-07
| | | | | | | | | | | | | | | | | | | | As of now, logical decoding of a multi-insert has been scanning all xl_multi_insert_tuple entries only if XLH_INSERT_CONTAINS_NEW_TUPLE was getting set in the record. This is not an issue on HEAD as multi-insert records are not used for system catalogs, but the logical decoding logic includes all the code necessary to handle that properly, except that the code missed to iterate correctly over all xl_multi_insert_tuple entries when the flag is not set. Hence, when trying to use multi-insert for system catalogs, an assertion would be triggered. An upcoming patch is going to make use of multi-insert for system catalogs, and this fixes the logic to make sure that all entries are scanned correctly without softening the existing assertions. Reported-by: Daniel Gustafsson Author: Michael Paquier Reviewed-by: Daniel Gustafsson Discussion: https://postgr.es/m/CBFFD532-C033-49EB-9A5A-F67EAEE9EB0B@yesql.se
* Fix typo in pathnode.cMichael Paquier2019-08-06
| | | | | Author: Amit Langote Discussion: https://postgr.es/m/CA+HiwqFhZ6ABoz-i=JZ5wMMyz-orx4asjR0og9qBtgEwOww6Yg@mail.gmail.com
* 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
* Add safeguards in LSN, numeric and float calculation for custom errorsMichael Paquier2019-08-05
| | | | | | | | | | | | | | | | | | | | | Those data types use parsing and/or calculation wrapper routines which can generate some generic error messages in the event of a failure. The caller of these routines can also pass a pointer variable settable by the routine to track if an error has happened, letting the caller decide what to do in the event of an error and what error message to generate. Those routines have been slacking the initialization of the tracking flag, which can be confusing when reading the code, so add some safeguards against calls of these parsing routines which could lead to a dubious result. The LSN parsing gains an assertion to make sure that the tracking flag is set, while numeric and float paths initialize the flag to a saner state. Author: Jeevan Ladhe Reviewed-by: Álvaro Herrera, Michael Paquier Discussion: https://postgr.es/m/CAOgcT0NOM9oR0Hag_3VpyW0uF3iCU=BDUFSPfk9JrWXRcWQHqw@mail.gmail.com
* Fix inconsistencies and typos in the tree, take 9Michael Paquier2019-08-05
| | | | | | | | This addresses more issues with code comments, variable names and unreferenced variables. Author: Alexander Lakhin Discussion: https://postgr.es/m/7ab243e0-116d-3e44-d120-76b3df7abefd@gmail.com
* 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