- 24 Apr, 2020 7 commits
-
-
Tom Lane authored
This includes the usual amount of editorial cleanup, such as correcting wrong or less-helpful-than-they-could-be examples. I moved the two tsvector-updating triggers into "9.28 Trigger Functions", which seems like a better home for them. (I believe that section didn't exist when this text was originally written.)
-
Bruce Momjian authored
e.g., REL_12_STABLE
-
Robert Haas authored
Per report from Andres Freund, who also says that this fix works for him. Discussion: http://postgr.es/m/20200405193118.alprgmozhxcfabkw@alap3.anarazel.de
-
Tom Lane authored
Commit 32ff2691 introduced use of rank() into the triggers view to calculate the spec-mandated action_order column. As written, this prevents query constraints on the table-name column from being pushed below the window aggregate step. That's bad for performance of this typical usage pattern, since the view now has to be evaluated for all tables not just the one(s) the user wants to see. It's also the cause of some recent buildfarm failures, in which trying to evaluate the view rows for triggers in process of being dropped resulted in "cache lookup failed for function NNN" errors. Those rows aren't of interest to the test script doing the query, but the filter that would eliminate them is being applied too late. None of this happened before the rank() call was there, so it's a regression compared to v10 and before. We can improve matters by changing the rank() call so that instead of partitioning by OIDs, it partitions by nspname and relname, casting those to sql_identifier so that they match the respective view output columns exactly. The planner has enough intelligence to know that constraints on partitioning columns are safe to push down, so this eliminates the performance problem and the regression test failure risk. We could make the other partitioning columns match view outputs as well, but it'd be more complicated and the performance benefits are questionable. Side note: as this stands, the planner will push down constraints on event_object_table and trigger_schema, but not on event_object_schema, because it checks for ressortgroupref matches not expression equivalence. That might be worth improving someday, but it's not necessary to fix the immediate concern. Back-patch to v11 where the rank() call was added. Ordinarily we'd not change information_schema in released branches, but the test failure has been seen in v12 and presumably could happen in v11 as well, so we need to do this to keep the buildfarm happy. The change is harmless so far as users are concerned. Some might wish to apply it to existing installations if performance of this type of query is of concern, but those who don't are no worse off. I bumped catversion in HEAD as a pro forma matter (there's no catalog incompatibility that would really require a re-initdb). Obviously that can't be done in the back branches. Discussion: https://postgr.es/m/5891.1587594470@sss.pgh.pa.us
-
Tom Lane authored
DST law changes in Morocco and the Canadian Yukon. Historical corrections for Shanghai. The America/Godthab zone is renamed to America/Nuuk to reflect current English usage; however, the old name remains available as a compatibility link.
-
Peter Eisentraut authored
-
Michael Paquier authored
The test is proving to have timing issues when looking at archive status files on standbys after crash recovery, while other parts of the test rely on pg_stat_archiver as a wait point to make sure that a given state of the archiving is reached. The coverage is not heavily impacted by the removal those extra tests. Per reports from several buildfarm animals, like crake, piculet, culicidae and francolin. Discussion: https://postgr.es/m/20200424005929.GK33034@paquier.xyz Backpatch-through: 9.5
-
- 23 Apr, 2020 9 commits
-
-
Michael Paquier authored
78ea8b5d has fixed an issue related to the recycling of WAL segments on standbys depending on archive_mode. However, it has introduced a regression with the handling of WAL segments ready to be archived during crash recovery, causing those files to be recycled without getting archived. This commit fixes the regression by tracking in shared memory if a live cluster is either in crash recovery or archive recovery as the handling of WAL segments ready to be archived is different in both cases (those WAL segments should not be removed during crash recovery), and by using this new shared memory state to decide if a segment can be recycled or not. Previously, it was not possible to know if a cluster was in crash recovery or archive recovery as the shared state was able to track only if recovery was happening or not, leading to the problem. A set of TAP tests is added to close the gap here, making sure that WAL segments ready to be archived are correctly handled when a cluster is in archive or crash recovery with archive_mode set to "on" or "always", for both standby and primary. Reported-by: Benoît Lobréau Author: Jehan-Guillaume de Rorthais Reviewed-by: Kyotaro Horiguchi, Fujii Masao, Michael Paquier Discussion: https://postgr.es/m/20200331172229.40ee00dc@firost Backpatch-through: 9.5
-
Tom Lane authored
In the footsteps of aaf069aa, remove ACLDEBUG, which was the only other remaining undocumented symbol in pg_config_manual.h. The fact that nobody had bothered to document it in seventeen years is a good clue to its usefulness. In practice, none of the tracing logic it enabled would be of any value without additional effort. Discussion: https://postgr.es/m/6631.1587565046@sss.pgh.pa.us
-
Tom Lane authored
Nobody really uses this stuff, especially not since we created valgrind-based infrastructure that does the same thing better. It is thus unsurprising that the generation.c and slab.c versions were actually broken. Rather than fix 'em, let's just remove 'em. Alexander Lakhin Discussion: https://postgr.es/m/8936216c-3492-3f6e-634b-d638fddc5f91@gmail.com
-
Tom Lane authored
Also rearrange that page a bit for more consistency and less duplication. In passing, fix erroneous examples of the results of abbrev(cidr) in datatype.sgml, and do a bit of copy-editing there.
-
Robert Haas authored
The previous commit renamed the struct's typedef, but not the struct name itself.
-
Robert Haas authored
Function names declared "extern" now use BackupManifest in the name rather than just Manifest, and data types use backup_manifest rather than just manifest. Per note from Michael Paquier. Discussion: http://postgr.es/m/20200418125713.GG350229@paquier.xyz
-
Andres Freund authored
In a9c35cf8 I changed ExecMakeTableFunctionResult() to dynamically allocate the FunctionCallInfo used to call the SRF. Unfortunately I did not account for the fact that the surrounding memory context has query lifetime, leading to a leak till the end of the query. In most cases the leak is fairly inconsequential, but if the FunctionScan is done many times in the query, the leak can add up. This happens e.g. if the function scan is on the inner side of a nested loop, due to a lateral join. EXPLAIN SELECT sum(f) FROM generate_series(1, 100000000) g(i), generate_series(i, i+1) f; quickly shows the leak. Instead of explicitly freeing the FunctionCallInfo it seems better to make sure all the per-set temporary state in ExecMakeTableFunctionResult() is cleaned up wholesale. Currently that's probably just the FunctionCallInfo allocation, but since there's some initialization work, and since there's already an appropriate context, this seems like a more robust approach. Bug: #16112 Reported-By: Ben Cornett Author: Andres Freund Reviewed-By: Tom Lane Discussion: https://postgr.es/m/16112-4448bbf55a404189%40postgresql.org Backpatch: 12, a9c35cf8
-
Fujii Masao authored
This commit does: - get rid of the garbage code for unused --print-parse-wal option. - add help message for --quiet option into usage(). - fix typo of option name in help message. Author: Fujii Masao Reviewed-by: Robert Haas Discussion: https://postgr.es/m/ff4710f7-2331-4f6b-012e-d76da3275e91@oss.nttdata.com
-
Tom Lane authored
David Johnston reminded me that the per-point calculations being done by these operators are equivalent to complex multiplication/division. (Once I would've recognized that immediately, but it's been too long since I did any of that sort of math.) Also put in a footnote mentioning that "rotation" of a box doesn't do what you might expect, as I'd griped about in the referenced thread. Discussion: https://postgr.es/m/158110996889.1089.4224139874633222837@wrigleys.postgresql.org
-
- 22 Apr, 2020 7 commits
-
-
Peter Geoghegan authored
The mask was added by commit 8224de4f, which introduced INCLUDE nbtree indexes. The status bits really were reserved initially. We now use 2 out of 4 of the bits for additional tuple metadata, though. Rename the mask to BT_STATUS_OFFSET_MASK. Also consolidate related nbtree.h code comments about the format of pivot tuples and posting list tuples.
-
Tomas Vondra authored
When estimating the number of pre-sorted groups in cost_incremental_sort we must not pass Vars with varno 0 to estimate_num_groups, which would cause failues in find_base_rel. This may happen when sorting output of set operations, thanks to generate_append_tlist. Unlike recurse_set_operations we can't easily access the original target list, so if we find any Vars with varno 0, we fall back to the default estimate DEFAULT_NUM_DISTINCT. Reported-by: Justin Pryzby Discussion: https://postgr.es/m/20200411214639.GK2228%40telsasoft.com
-
Bruce Momjian authored
See https://mapscaping.com/blogs/geo-candy/what-is-the-difference-between-elevation-relief-and-altitude No patching of regression tests. Reported-by: taf1@cornell.edu Discussion: https://postgr.es/m/158506544539.679.2278386310645558048@wrigleys.postgresql.org Backpatch-through: 9.5
-
Tom Lane authored
Make header/trailer comments agree with the actual names of some macros. These seem like legit names in earlier iterations of respective patches (commit b779168f "Detect PG_PRINTF_ATTRIBUTE automatically." and commit 6869b4f2 "Add C++ support to configure.") but the macro had been renamed out of sync with the header / trailer comment in the final committed patch. Even more nitpickily, make the dashed underlines agree with the lengths of the macro names everyplace. There doesn't seem to have been any meeting of the minds previously on whether those should match or not, but at least some people have been trying to make 'em match. Jesse Zhang, Tom Lane Discussion: https://postgr.es/m/CAGf+fX7DDyq6WfCy6X_KtD28MkbNBE6NkRi26fSf25dfUwX0zw@mail.gmail.com
-
Tom Lane authored
This also makes an attempt to flesh out the docs for some of the more severely underdocumented geometric operators and functions. This effort exposed that the point <^ point (point_below) and point >^ point (point_above) operators are misnamed; they should be <<| and |>>, because they act like the other operators named that way and not like the other operators named <^ and >^. But I just documented them that way; fixing it is matter for another patch. The haphazard datatype coverage of many of the operators is also now depressingly obvious. Discussion: https://postgr.es/m/158110996889.1089.4224139874633222837@wrigleys.postgresql.org
-
David Rowley authored
This Assert was trying to ensure that the number of columns in the foreign key being cloned was the same number of attributes in the parentRel. Of course, it's perfectly valid to have columns in the table which are not part of the foreign key constraint. It appears that this Assert was misunderstanding that. Reported-by: Rajkumar Raghuwanshi Reviewed-by: amul sul Discussion: https://postgr.es/m/CAKcux6=z1dtiWw5BOpqDx-U6KTiq+zD0Y2m810zUtWL+giVXWA@mail.gmail.com
-
Peter Eisentraut authored
This has been broken since PostgreSQL 12 and was probably never really used. PostgreSQL 12 added an analogous HEAPAMSLOTDEBUGALL, which still works right now, but it's also not very useful, so remove that as well. Discussion: https://www.postgresql.org/message-id/flat/645c0646-4218-d4c3-409a-a7003a0c108d%402ndquadrant.com
-
- 21 Apr, 2020 13 commits
-
-
Michael Paquier authored
readOneRecord() is used now when looking for a checkpoint record to check if the target server is an ancestor of the source across multiple timelines, and using a restore_command if available improves the stability of the operation. This part was missed in a7e8ece4. Reported-by: Kyotaro Horiguchi Discussion: https://postgr.es/m/20200421.150830.1410714948345179794.horikyota.ntt@gmail.com
-
Alvaro Herrera authored
It's important to know that a trigger is cloned from a parent table, because of the behavior that the trigger is dropped on detach. Make psql's \d display it. We'd like to backpatch, but lack of the pg_trigger.tgparentid column makes it more difficult. Punt for now. If somebody wants to volunteer an implementation that reads pg_depend on older versions, that can probably be backpatched. Authors: Justin Pryzby, Amit Langote, Álvaro Herrera Discussion: https://postgr.es/m/20200419002206.GM26953@telsasoft.com
-
Michael Paquier authored
Checking if Subject Alternative Names (SANs) from a certificate match with the hostname connected to leaked memory after each lookup done. This is broken since acd08d76 that added support for SANs in SSL certificates, so backpatch down to 9.5. Author: Roman Peshkurov Reviewed-by: Hamid Akhtar, Michael Paquier, David Steele Discussion: https://postgr.es/m/CALLDf-pZ-E3mjxd5=bnHsDu9zHEOnpgPgdnO84E2RuwMCjjyPw@mail.gmail.com Backpatch-through: 9.5
-
Alvaro Herrera authored
Add a couple of lines to make it explicit that indexes, constraints, triggers are added, removed, or left alone. Backpatch to pg11. Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: Justin Pryzby <pryzby@telsasoft.com> Discussion: https://postgr.es/m/20200421162038.GA18628@alvherre.pgsql
-
Tom Lane authored
index.c supposed that it could just use a PG_TRY block to clean up the state associated with an active REINDEX operation. However, that code doesn't run if we do a FATAL exit --- for example, due to a SIGTERM shutdown signal --- while the REINDEX is happening. And that state does get consulted during catalog accesses, which makes it problematic if we do any catalog accesses during shutdown --- for example, to clean up any temp tables created in the session. If this combination of circumstances occurred, we could find ourselves trying to access already-freed memory. In debug builds that'd fairly reliably cause an assertion failure. In production we might often get away with it, but with some bad luck it could cause a core dump. Another possible bad outcome is an erroneous conclusion that an index-to-be-accessed is being reindexed; but it looks like that would be unlikely to have any consequences worse than failing to drop temp tables right away. (They'd still get dropped by the next session that uses that temp schema.) To fix, get rid of the use of PG_TRY here, and instead hook into the transaction abort mechanisms to clean up reindex state. Per bug #16378 from Alexander Lakhin. This has been wrong for a very long time, so back-patch to all supported branches. Discussion: https://postgr.es/m/16378-7a70ca41b3ec2009@postgresql.org
-
Tom Lane authored
Working on commit 1c455078 led me to check through FunctionCallInvoke call sites to see if every one was being honest about (a) making sure that fcinfo.isnull is initially false, and (b) checking its state after the call. Sure enough, I found some violations. The main one is that finalize_partialaggregate re-used serialfn_fcinfo without resetting isnull, even though it clearly intends to cater for serialfns that return NULL. There would only be an issue with a non-strict serialfn, since it's unlikely that a serialfn would return NULL for non-null input. We have no non-strict serialfns in core, and there may be none in the wild either, which would account for the lack of complaints. Still, it's clearly wrong, so back-patch that fix to 9.6 where finalize_partialaggregate was introduced. Also, arrayfuncs.c and rowtypes.c contained various callers that were not bothering to check for result nulls. While what's being called is a comparison or hash function that probably *shouldn't* return null, that's a lousy excuse for not having any check at all. There are existing places that just Assert(!fcinfo->isnull) in comparable situations, so I added that to the places that were calling btree comparison or hash support functions. In the places calling boolean-returning equality functions, it's quite cheap to have them treat isnull as FALSE, so make those places do that. Also remove some "locfcinfo->isnull = false" assignments that are unnecessary given the assumption that no previous call returned null. These changes seem like mostly neatnik-ism or debugging support, so I didn't back-patch.
-
Alvaro Herrera authored
When a partition is detached, any triggers that had been cloned from its parent were not properly disentangled from its parent triggers. This resulted in triggers that could not be dropped because they depended on the trigger in the trigger in the no-longer-parent table: ALTER TABLE t DETACH PARTITION t1; DROP TRIGGER trig ON t1; ERROR: cannot drop trigger trig on table t1 because trigger trig on table t requires it HINT: You can drop trigger trig on table t instead. Moreover the table can no longer be re-attached to its parent, because the trigger name is already taken: ALTER TABLE t ATTACH PARTITION t1 FOR VALUES FROM (1)TO(2); ERROR: trigger "trig" for relation "t1" already exists The former is a bug introduced in commit 86f57594. (The latter is not necessarily a bug, but it makes the bug more uncomfortable.) To avoid the complexity that would be needed to tell whether the trigger has a local definition that has to be merged with the one coming from the parent table, establish the behavior that the trigger is removed when the table is detached. Backpatch to pg11. Author: Justin Pryzby <pryzby@telsasoft.com> Reviewed-by: Amit Langote <amitlangote09@gmail.com> Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://www.postgresql.org/message-id/flat/20200408152412.GZ2228@telsasoft.com
-
Peter Geoghegan authored
Commit 0d861bbb, which introduced deduplication to nbtree, added some logic to take large posting list tuples into account when choosing a split point. We subtract firstright posting list overhead from the projected new high key size when calculating leftfree/rightfree values for an affected candidate split point. Posting list tuples aren't special to nbtsplitloc.c, but taking them into account like this makes a huge difference in practice. Posting list tuples are frequently tuple size outliers. However, commit 0d861bbb missed a closely related issue: split interval itself is calculated based on the assumption that tuples on the page being split are roughly equisized. That assumption was acceptable back when commit fab25024 taught the logic for choosing a split point about suffix truncation, but it's pretty questionable now that very large tuple sizes are common. This oversight led to unbalanced page splits in low cardinality multi-column indexes when deduplication was used: page splits that don't give sufficient weight to how unbalanced the split is when the interval happens to include some large posting list tuples (and when most other tuples on the page are not so large). Nail this down by calculating an initial split interval in a way that's attuned to the actual cost that we want to keep under control (not a fuzzy proxy for the cost): apply a leftfree + rightfree evenness test to each candidate split point that actually gets included in the split interval (for the default strategy). This replaces logic that used a percentage of all legal split points for the page as the basis of the initial split interval. Discussion: https://postgr.es/m/CAH2-WznJt5aT2uUB2Bs+JBLdwe0XTX67+xeLFcaNvCKxO=QBVQ@mail.gmail.com
-
Tom Lane authored
Although selfuncs.c will never call a target operator with null inputs, some functions might return null anyway. The existing coding will fail if that happens (since FunctionCall2Coll will punt), which seems undesirable given that matchingsel() has such a broad range of potential applicability --- in fact, we already have a problem because we apply it to jsonb_path_exists_opr, which can return null. Hence, rejigger the underlying functions mcv_selectivity and histogram_selectivity to cope, treating a null result as false. While we are at it, we can move the InitFunctionCallInfoData overhead out of the inner loops, which isn't a huge number of cycles but might save something considering we are likely calling functions as cheap as int4eq(). Plus, the number of loop cycles to be expected is much more than it was when this code was written, since typical settings of default_statistics_target are higher. In view of that consideration, let's apply the same change to var_eq_const, eqjoinsel_inner, and eqjoinsel_semi. We do not expect equality functions to ever return null for non-null inputs (and certainly that code has been that way a long time without complaints), but the cycle savings seem attractive, especially in the eqjoinsel loops where there's potentially an O(N^2) savings. Similar code exists in ineq_histogram_selectivity and get_variable_range, but I forebore from changing those for now. The performance argument for changing ineq_histogram_selectivity is really weak anyway, since that will only iterate log2(N) times. Nikita Glukhov and Tom Lane Discussion: https://postgr.es/m/9d3b0959-95d6-c37e-2c0b-287bcfe5c705@postgrespro.ru
-
Tom Lane authored
Older gcc versions don't like duplicate typedefs, so get rid of that in favor of doing it like we do it elsewhere, ie just use a "struct" declaration when trying to avoid importing a whole header file. Also, there seems no reason to include stringinfo.h here at all, so get rid of that addition too. Discussion: https://postgr.es/m/27239.1587415696@sss.pgh.pa.us
-
Fujii Masao authored
Previously in the "Standby Server Operation" section, pg_ctl promote and protmote_trigger_file were documented as a method to trigger standby promotion, but pg_promote() function not. This commit also adds parentheses into <function>pg_promote</function> in some docs to make it clearer that a function is being referred to. Author: Masahiro Ikeda Reviewed-by: Michael Paquier, Laurenz Albe, Tom Lane, Fujii Masao Discussion: https://postgr.es/m/de0068417a9f4046bac693cbcc00bdc9@oss.nttdata.com
-
Bruce Momjian authored
Reported-by: Jürgen Purtz Discussion: https://postgr.es/m/709d7809-d7f4-8175-47f3-4d131341bba8@purtz.de Author: Jürgen Purtz Backpatch-through: 9.5
-
- 20 Apr, 2020 4 commits
-
-
Tom Lane authored
Also some mop-up in section 9.9.
-
Robert Haas authored
basebackup.c is already a pretty big and complicated file, so it makes more sense to keep the backup manifest support routines in a separate file, for clarity and ease of maintenance. Discussion: http://postgr.es/m/CA+TgmoavRak5OdP76P8eJExDYhPEKWjMb0sxW7dF01dWFgE=uA@mail.gmail.com
-
Alvaro Herrera authored
... as added in the prior commit. (We'd like to have tab-completion for the other object types too, but they don't have sub-command completion yet.) Author: Ibrar Ahmed <ibrar.ahmad@gmail.com> Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/CALtqXTcogrFEVP9uou5vFtnGsn+vHZUu9+9a0inarfYVOHScYQ@mail.gmail.com
-
Alvaro Herrera authored
Commit f2fcad27 (9.6 era) added the ability to mark objects as dependent an extension, but forgot to add a way for such dependencies to be removed. This commit fixes that oversight. Strictly speaking this should be backpatched to 9.6, but due to lack of demand we're not doing so at this time. Discussion: https://postgr.es/m/20200217225333.GA30974@alvherre.pgsqlReviewed-by: ahsan hadi <ahsan.hadi@gmail.com> Reviewed-by: Ibrar Ahmed <ibrar.ahmad@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
-