- 18 Aug, 2019 5 commits
-
-
Tom Lane authored
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
-
Tom Lane authored
Needs libpq-fe.h for references to PGConn. Discussion: https://postgr.es/m/17463.1566153454@sss.pgh.pa.us
-
Tom Lane authored
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.
-
Tom Lane authored
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
-
Peter Eisentraut authored
-
- 17 Aug, 2019 2 commits
-
-
Tom Lane authored
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
-
Peter Eisentraut authored
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
-
- 16 Aug, 2019 7 commits
-
-
Andres Freund authored
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
-
Andres Freund authored
-
Andres Freund authored
Most of the fmgr.h includes were obsoleted by 352a24a1. 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
-
Andres Freund authored
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
-
Andres Freund authored
These aren't needed after 352a24a1. The remaining prototypes are not defined on the SQL level. Author: Andres Freund Discussion: https://postgr.es/m/20190803193733.g3l3x3o42uv4qj7l@alap3.anarazel.de
-
Etsuro Fujita authored
These seem to be leftovers from the original partitionwise-join patch, perhaps. Discussion: https://postgr.es/m/CAPmGK145YiMTPRnvev1dLz8na_-0aZ=Xyqn8f2QsJFBUTObNow@mail.gmail.com
-
Tom Lane authored
This is a variant of the problem fixed in commit 25b69256, 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 25b69256 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
-
- 15 Aug, 2019 3 commits
-
-
Tom Lane authored
Commit 4b93f579 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 4b93f579 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 4b93f579 came in. Discussion: https://postgr.es/m/15913-a7e112e16dedcffc@postgresql.org
-
Tom Lane authored
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
-
Tom Lane authored
Clarify what external tools can do to this file, and add a bit of detail about what ALTER SYSTEM itself does. Discussion: https://postgr.es/m/aed6cc9f-98f3-2693-ac81-52bb0052307e@2ndquadrant.com
-
- 14 Aug, 2019 5 commits
-
-
Tom Lane authored
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
-
Peter Geoghegan authored
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
-
Peter Eisentraut authored
The method of passing LC_COLLATE and LC_CTYPE to the backend during initdb is obsolete as of 61d96749. 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
-
Michael Paquier authored
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
-
Peter Geoghegan authored
Commit d2086b08 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.
-
- 13 Aug, 2019 6 commits
-
-
Tom Lane authored
Commit 07b39083 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
-
Peter Geoghegan authored
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
-
Alvaro Herrera authored
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 4e85642d; 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
-
Peter Eisentraut authored
This moves us to the latest minor version of DocBook 4. It requires no markup changes.
-
Michael Paquier authored
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
-
Michael Paquier authored
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 319a8101, so backpatch down to v10. Discussion: https://postgr.es/m/20190807132422.GC15695@paquier.xyz Backpatch-through: 10
-
- 12 Aug, 2019 5 commits
-
-
Peter Geoghegan authored
contrib/amcheck failed to consider the possibility that unlogged relations will not have any main relation fork files when running in hot standby mode. This led to low-level "can't happen" errors that complain about the absence of a relfilenode file. To fix, simply skip verification of unlogged index relations during recovery. In passing, add a direct check for the presence of a main fork just before verification proper begins, so that we cleanly verify the presence of the main relation fork file. Author: Andrey Borodin, Peter Geoghegan Reported-By: Andrey Borodin Diagnosed-By: Andrey Borodin Discussion: https://postgr.es/m/DA9B33AC-53CB-4643-96D4-7A0BBC037FA1@yandex-team.ru Backpatch: 10-, where amcheck was introduced.
-
Tom Lane authored
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 eccfef81), so back-patch to v10 where that came in. Discussion: https://postgr.es/m/15892-e5d2bea3e8a04a1b@postgresql.org
-
Tom Lane authored
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
-
Tom Lane authored
In the wake of commit 1cff1b95, 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
-
Alexander Korotkov authored
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
-
- 11 Aug, 2019 2 commits
-
-
Tom Lane authored
This reverts much of commit f03a9ca4, 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
-
Alexander Korotkov authored
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
-
- 10 Aug, 2019 2 commits
-
-
Tom Lane authored
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 11d8d72c 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 23224563; 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
-
Peter Geoghegan authored
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 8b304b8b, which removed replacement selection sort from tuplesort.c.
-
- 09 Aug, 2019 3 commits
-
-
Tom Lane authored
Not much to be said here: commit 9fdb675f 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
-
Tom Lane authored
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
-
Michael Paquier authored
b654714f 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
-