aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* 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
* Remove unnecessary test dependency on the contents of pg_pltemplate.Tom Lane2019-08-21
| | | | | | | | | Using pg_pltemplate as test data was probably not very forward-looking, considering we've had many discussions around removing that catalog altogether. Use a nearby temp table instead, to make these two test scripts more self-contained. This is a better test case anyway, since it exercises the scenario where the entries in the anyarray column actually vary in type intra-query.
* Remove master/slave usage from plpgsql testsPeter Eisentraut2019-08-21
| | | | | Author: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> Discussion: https://www.postgresql.org/message-id/flat/E393EC88-377F-4C59-A67A-69F2A38D17C7@yesql.se
* 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
* Fix compilation failure of vacuumdb and reindexdb with OpenBSDMichael Paquier2019-08-20
| | | | | | | | | | | | | | | | | | | | | | | FD_SETSIZE is included in sys/select.h per POSIX, and this header inclusion has been moved to scripts_parallel.c as of 5f38403 without moving the variable, causing a compilation failure on recent versions of OpenBSD (6.6 was the version used in the report). In order to take care of the failure, move FD_SETSIZE directly to scripts_parallel.c with a wrapper controlling the maximum number of parallel slots supported, based on a suggestion by Andres Freund. While on it, reduce the maximum number to be less than FD_SETSIZE, leaving some room for stdin, stdout and such as they consume some file descriptors. The buildfarm did not complain about that, as it happens to only be an issue on recent versions of OpenBSD and there is no coverage in this area. 51c3e9f fixed a similar set of issues. Bug: #15964 Reported-by: Sean Farrell Discussion: https://postgr.es/m/15964-c1753bdfed722e04@postgresql.org
* 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
* Add fmgr.h include to selfuncs.h.Andres Freund2019-08-19
| | | | | | | | | Necessary after fb3b098f. That previously escaped notice, because all including sites already include fmgr.h some other way. Reported-By: Tom Lane Author: Andres Freund Discussion: https://postgr.es/m/17463.1566153454@sss.pgh.pa.us
* Add "headerscheck" script to test header-file compilability under C.Tom Lane2019-08-19
| | | | | | | | | | | | | | | We already had "cpluspluscheck", which served the dual purposes of verifying that headers compile standalone and that they compile as C++. However, C++ compilers don't have the exact same set of error conditions as C compilers, so this doesn't really prove that a header will compile standalone as C. Hence, add a second script that's largely similar but runs the C compiler not C++. Also add a bit more documentation than the none-at-all we had before. Discussion: https://postgr.es/m/14803.1566175851@sss.pgh.pa.us
* Use zic's new "-b slim" option to generate smaller timezone files.Tom Lane2019-08-19
| | | | | | | | | | | | | | | | | | | | | | | | | IANA tzcode release 2019b adds an option that tells zic not to emit the old 32-bit section of the timezone files, and to skip some other space-wasting hacks needed for compatibility with old timezone client libraries. Since we only expect our own code to use the timezone data we install, and our code is up-to-date with 2019b, there's no apparent reason not to generate the smallest possible files. Unfortunately, while the individual zone files do get significantly smaller in many cases, they were not that big to begin with; which means that no real space savings ensues on filesystems that don't optimize small files. (For instance, on ext4 with 4K block size, "du" says the installed timezone tree is the same size as before.) Still, it seems worth making the change, if only because this is presumably the wave of the future. At the very least, we'll save some cycles while reading a zone file. But given the marginal value and the fact that this is a new code path, it doesn't seem worth the risk of back-patching this change into stable branches. Hence, unlike most of our timezone-related changes, apply to HEAD only. Discussion: https://postgr.es/m/24998.1563403327@sss.pgh.pa.us
* Fix tab completion for CREATE TYPE in psqlMichael Paquier2019-08-19
| | | | | | | Oversight in 7bdc655. Author: Alexander Lakhin Discussion: https://postgr.es/m/5da8e325-c665-da95-21e0-c8a99ea61fbf@gmail.com
* 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
* Fix failure-to-compile-standalone in scripts_parallel.h.Tom Lane2019-08-18
| | | | | | Needs libpq-fe.h for references to PGConn. Discussion: https://postgr.es/m/17463.1566153454@sss.pgh.pa.us
* Fix failure-to-compile-standalone in ecpg's dt.h.Tom Lane2019-08-18
| | | | | | | | | This has to have <time.h>, or the references to "struct tm" don't mean what they should. We have some other recently-introduced issues of the same ilk, but this one seems old. No backpatch though, as it's only a latent problem for most purposes.
* 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
* Make deadlock-parallel isolation test more robust.Tom Lane2019-08-17
| | | | | | | | | | | | | | | | | | | | | | | This test failed fairly reproducibly on some CLOBBER_CACHE_ALWAYS buildfarm animals. The cause seems to be that if a parallel worker is slow enough to reach its lock wait, it may not be released by the first deadlock check run, and then later deadlock checks might decide to unblock the d2 session instead of the d1 session, leaving us in an undetected deadlock state (since the isolationtester client is waiting for d1 to complete first). Fix by introducing an additional lock wait at the end of the d2a1 step, ensuring that the deadlock checker will recognize that d1 has to be unblocked before d2a1 completes. Also reduce max_parallel_workers_per_gather to 3 in this test. With the default max_worker_processes value, we were only getting one parallel worker for the d2a1 step, which is not the case I hoped to test. We should get 3 for d1a2 and 2 for d2a1, as the code stands; and maybe 3 for d2a1 if somebody figures out why the last parallel worker slot isn't free already. Discussion: https://postgr.es/m/22195.1566077308@sss.pgh.pa.us
* Improve Assert outputPeter Eisentraut2019-08-17
| | | | | | | | | | | | | If an assertion expression contained a macro, the failed assertion message would print the expanded macro, which is usually unhelpful and confusing. Restructure the Assert macros to not expand any macros when constructing the failure message. This also fixes that the existing output for Assert et al. shows the *inverted* condition, which is also confusing and not how assertions usually work. Discussion: https://www.postgresql.org/message-id/flat/6c68efe3-117a-dcc1-73d4-18ba1ec532e2%402ndquadrant.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
* Add missing fmgr.h include.Andres Freund2019-08-16
|
* 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 redundant prototypes for SQL callable functions.Andres Freund2019-08-16
| | | | | | | | These aren't needed after 352a24a1f9d6. The remaining prototypes are not defined on the SQL level. 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
* 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
* 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
* 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
* 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.
* 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
* 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 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
* 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
* Partially revert "Insert temporary debugging output in regression tests."Tom Lane2019-08-11
| | | | | | | | | | | | This reverts much of commit f03a9ca4366d064d89b7cf7ed75d4e43f2ed0667, but leaves the relpages/reltuples probe in select_parallel.sql. The pg_stat_all_tables probes are unstable enough to be annoying, and it no longer seems likely that they will teach us anything more about the underlying problem. I'd still like some more confirmation though that the observed plan instability is caused by VACUUM leaving relpages/reltuples as zero for one of these tables. Discussion: https://postgr.es/m/CA+hUKG+0CxrKRWRMf5ymN3gm+BECHna2B-q1w8onKBep4HasUw@mail.gmail.com
* 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