aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
* Fix race in dsm_unpin_segment() when handles are reused.Thomas Munro2019-02-18
| | | | | | | | | | | | | | | | | | | Teach dsm_unpin_segment() to skip segments that are in the process of being destroyed by another backend, when searching for a handle. Such a segment cannot possibly be the one we are looking for, even if its handle matches. Another slot might hold a recently created segment that has the same handle value by coincidence, and we need to keep searching for that one. The bug caused rare "cannot unpin a segment that is not pinned" errors on 10 and 11. Similar to commit 6c0fb941 for dsm_attach(). Back-patch to 10, where dsm_unpin_segment() landed. Author: Thomas Munro Reported-by: Justin Pryzby Tested-by: Justin Pryzby (along with other recent DSA/DSM fixes) Discussion: https://postgr.es/m/20190216023854.GF30291@telsasoft.com
* Fix CREATE VIEW to allow zero-column views.Tom Lane2019-02-17
| | | | | | | | | | | | | | | | | | | | | We should logically have allowed this case when we allowed zero-column tables, but it was overlooked. Although this might be thought a feature addition, it's really a bug fix, because it was possible to create a zero-column view via the convert-table-to-view code path, and then you'd have a situation where dump/reload would fail. Hence, back-patch to all supported branches. Arrange the added test cases to provide coverage of the related pg_dump code paths (since these views will be dumped and reloaded during the pg_upgrade regression test). I also made them test the case where pg_dump has to postpone the view rule into post-data, which disturbingly had no regression coverage before. Report and patch by Ashutosh Sharma (test case by me) Discussion: https://postgr.es/m/CAE9k0PkmHdeSaeZt2ujnb_cKucmK3sDDceDzw7+d5UZoNJPYOg@mail.gmail.com
* Fix support for CREATE TABLE IF NOT EXISTS AS EXECUTEMichael Paquier2019-02-15
| | | | | | | | | | | The grammar IF NOT EXISTS for CTAS is supported since 9.5 and documented as such, however the case of using EXECUTE as query has never been covered as EXECUTE CTAS statements and normal CTAS statements are parsed separately. Author: Andreas Karlsson Discussion: https://postgr.es/m/2ddcc188-e37c-a0be-32bf-a56b07c3559e@proxel.se Backpatch-through: 9.5
* Fix race in dsm_attach() when handles are reused.Thomas Munro2019-02-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | DSM handle values can be reused as soon as the underlying shared memory object has been destroyed. That means that for a brief moment we might have two DSM slots with the same handle. While trying to attach, if we encounter a slot with refcnt == 1, meaning that it is currently being destroyed, we should continue our search in case the same handle exists in another slot. The race manifested as a rare "dsa_area could not attach to segment" error, and was more likely in 10 and 11 due to the lack of distinct seed for random() in parallel workers. It was made very unlikely in in master by commit 197e4af9, and older releases don't usually create new DSM segments in background workers so it was also unlikely there. This fixes the root cause of bug report #15585, in which the error could also sometimes result in a self-deadlock in the error path. It's not yet clear if further changes are needed to avoid that failure mode. Back-patch to 9.4, where dsm.c arrived. Author: Thomas Munro Reported-by: Justin Pryzby, Sergei Kornilov Discussion: https://postgr.es/m/20190207014719.GJ29720@telsasoft.com Discussion: https://postgr.es/m/15585-324ff6a93a18da46@postgresql.org
* Fix rare dsa_allocate() failures due to freepage.c corruption.Thomas Munro2019-02-13
| | | | | | | | | | | | | | | | | | In a corner case, a btree page was allocated during a clean-up operation that could cause the tracking of the largest contiguous span of free space to get out of whack. That was supposed to be prevented by the use of the "soft" flag to avoid allocating internal pages during incidental clean-up work, but the flag was ignored in the case where the FPM was promoted from singleton format to btree format. Repair. Remove an obsolete comment in passing. Back-patch to 10, where freepage.c arrived (as support for dsa.c). Author: Robert Haas Diagnosed-by: Thomas Munro and Robert Haas Reported-by: Justin Pryzby, Rick Otten, Sand Stone, Arne Roland and others Discussion: https://postgr.es/m/CAMAYy4%2Bw3NTBM5JLWFi8twhWK4%3Dk_5L4nV5%2BbYDSPu8r4b97Zg%40mail.gmail.com
* Clean up planner confusion between ncolumns and nkeycolumns.Tom Lane2019-02-12
| | | | | | | | | | | | | | | | | | | | | | | | | We're only going to consider key columns when creating indexquals, so there is no point in having the outer loops in indxpath.c iterate further than nkeycolumns. Doing so in match_pathkeys_to_index() is actually wrong, and would have caused crashes by now, except that we have no index AMs supporting both amcanorderbyop and amcaninclude. It's also wrong in relation_has_unique_index_for(). The effect there is to fail to prove uniqueness even when the index does prove it, if there are extra columns. Also future-proof examine_variable() for the day when extra columns can be expressions, and fix what's either a thinko or just an oversight in btcostestimate(): we should consider the number of key columns, not the total, when deciding whether to derate correlation. None of these things seemed important enough to risk changing in a just-before-wrap patch, but since we're past the release wrap window, time to fix 'em. Discussion: https://postgr.es/m/25526.1549847928@sss.pgh.pa.us
* Relax overly strict assertionAlvaro Herrera2019-02-12
| | | | | | | | | | | | | | | Ever since its birth, ReorderBufferBuildTupleCidHash() has contained an assertion that a catalog tuple cannot change Cmax after acquiring one. But that's wrong: if a subtransaction executes DDL that affects that catalog tuple, and later aborts and another DDL affects the same tuple, it will change Cmax. Relax the assertion to merely verify that the Cmax remains valid and monotonically increasing, instead. Add a test that tickles the relevant code. Diagnosed by, and initial patch submitted by: Arseny Sher Co-authored-by: Arseny Sher Discussion: https://postgr.es/m/874l9p8hyw.fsf@ars-thinkpad
* Fix erroneous error reports in snapbuild.c.Tom Lane2019-02-12
| | | | | | | | | | | | | | | | It's pretty unhelpful to report the wrong file name in a complaint about syscall failure, but SnapBuildSerialize managed to do that twice in a span of 50 lines. Also fix half a dozen missing or poorly-chosen errcode assignments; that's mostly cosmetic, but still wrong. Noted while studying recent failures on buildfarm member nightjar. I'm not sure whether those reports are actually giving the wrong filename, because there are two places here with identically spelled error messages. The other one is specifically coded not to report ENOENT, but if it's this one, how could we be getting ENOENT from open() with O_CREAT? Need to sit back and await results. However, these ereports are clearly broken from birth, so back-patch.
* Translation updatesPeter Eisentraut2019-02-11
| | | | | Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git Source-Git-Hash: 2cd47eeb832ed1bb1cbfff285cfc921ca4d07a9d
* Adjust error messagePeter Eisentraut2019-02-11
| | | | | | | | | We usually don't use "namespace" in user-facing error messages. Also, in master this was replaced by another error message referring to "temporary objects", so we might as well use that here to avoid introducing too many variants. Discussion: https://www.postgresql.org/message-id/bbd3f8d9-e3d5-e5aa-4305-7f0121c3fa94@2ndquadrant.com
* Fix indexable-row-comparison logic to account for covering indexes.Tom Lane2019-02-10
| | | | | | | | | | | | | | | | | | indxpath.c needs a good deal more attention for covering indexes than it's gotten. But so far as I can tell, the only really awful breakage is in expand_indexqual_rowcompare (nee adjust_rowcompare_for_index), which was only half fixed in c266ed31a. The other problems aren't bad enough to take the risk of a just-before-wrap fix. The problem here is that if the leading column of a row comparison matches an index (allowing this code to be reached), and some later column doesn't match the index, it'll nonetheless believe that that column matches the first included index column. Typically that'll lead to an error like "operator M is not a member of opfamily N" as a result of fetching a garbage opfamily OID. But with enough bad luck, maybe a broken plan would be generated. Discussion: https://postgr.es/m/25526.1549847928@sss.pgh.pa.us
* Fix trigger drop procedureAlvaro Herrera2019-02-10
| | | | | | | | | | | After commit 123cc697a8eb, we remove redundant FK action triggers during partition ATTACH by merely deleting the catalog tuple, but that's wrong: it should use performDeletion() instead. Repair, and make the comments more explicit. Per code review from Tom Lane. Discussion: https://postgr.es/m/18885.1549642539@sss.pgh.pa.us
* Call set_rel_pathlist_hook before generate_gather_paths, not after.Tom Lane2019-02-09
| | | | | | | | | | | | | | | | | | | | | | | | The previous ordering of these steps satisfied the nominal requirement that set_rel_pathlist_hook could editorialize on the whole set of Paths constructed for a base relation. In practice, though, trying to change the set of partial paths was impossible. Adding one didn't work because (a) it was too late to be included in Gather paths made by the core code, and (b) calling add_partial_path after generate_gather_paths is unsafe, because it might try to delete a path it thinks is dominated, but that is already embedded in some Gather path(s). Nor could the hook safely remove partial paths, for the same reason that they might already be embedded in Gathers. Better to call extensions first, let them add partial paths as desired, and then gather. In v11 and up, we already doubled down on that ordering by postponing gathering even further for single-relation queries; so even if the hook wished to editorialize on Gather path construction, it could not. Report and patch by KaiGai Kohei. Back-patch to 9.6 where Gather paths were added. Discussion: https://postgr.es/m/CAOP8fzahwpKJRTVVTqo2AE=mDTz_efVzV6Get_0=U3SO+-ha1A@mail.gmail.com
* For 11 only, put back heap_expand_tuple to GetTupleForTrigger().Andres Freund2019-02-09
| | | | | | | | | | | | This is not necessary anymore after 297d627e, but extensions that have not been recompiled after the fix will not use the new definition of heap_getattr(). While recompiling those extensions is obviously the suggested course, it's cheap enough to retain the expansion in GetTupleForTrigger(). Per suggestion from Andrew Gierth. Discussion: 87va1x43ot.fsf@news-spur.riddles.org.uk
* Reset, not recreate, execGrouping.c style hashtables.Andres Freund2019-02-09
| | | | | | | | | | | | | | | | This uses the facility added in the preceding commit to fix performance issues caused by rebuilding the hashtable (with its comparator expression being the most expensive bit), after every reset. That's especially important when the comparator is JIT compiled. Bug: #15592 #15486 Reported-By: Jakub Janeček, Dmitry Marakasov Author: Andres Freund Discussion: https://postgr.es/m/15486-05850f065da42931@postgresql.org https://postgr.es/m/20190114180423.ywhdg2iagzvh43we@alap3.anarazel.de Backpatch: 11, where I broke this in bf6c614a2f2c5
* Allow to reset execGrouping.c style tuple hashtables.Andres Freund2019-02-09
| | | | | | | | | | | | | | | | | | | | | | | | | | This has the advantage that the comparator expression, the table's slot, etc do not have to be rebuilt. Additionally the simplehash.h hashtable within the tuple hashtable now keeps its previous size and doesn't need to be reallocated. That both reduces allocator overhead, and improves performance in cases where the input estimation was off by a significant factor. To avoid an API/ABI break, the new parameter is exposed via the new BuildTupleHashTableExt(), and BuildTupleHashTable() now is a wrapper around the former, that continues to allocate the table itself in the tablecxt. Using this fixes performance issues discovered in the two bugs referenced. This commit however has not converted the callers, that's done in a separate commit. Bug: #15592 #15486 Reported-By: Jakub Janeček, Dmitry Marakasov Author: Andres Freund Discussion: https://postgr.es/m/15486-05850f065da42931@postgresql.org https://postgr.es/m/20190114180423.ywhdg2iagzvh43we@alap3.anarazel.de Backpatch: 11, this is a prerequisite for other fixes
* Plug leak in BuildTupleHashTable by creating ExprContext in correct context.Andres Freund2019-02-09
| | | | | | | | | | | | | | | | | | | | In bf6c614a2f2c5 I added a expr context to evaluate the grouping expression. Unfortunately the code I added initialized them while in the calling context, rather the table context. Additionally, I used CreateExprContext() rather than CreateStandaloneExprContext(), which creates the econtext in the estate's query context. Fix that by using CreateStandaloneExprContext when in the table's tablecxt. As we rely on the memory being freed by a memory context reset that means that the econtext's shutdown callbacks aren't being called, but that seems ok as the expressions are tightly controlled due to ExecBuildGroupingEqual(). Bug: #15592 Reported-By: Dmitry Marakasov Author: Andres Freund Discussion: https://postgr.es/m/20190114222838.h6r3fuyxjxkykf6t@alap3.anarazel.de Backpatch: 11, where I broke this in bf6c614a2f2c5
* Defend against null error message reported by libxml2.Tom Lane2019-02-08
| | | | | | | | | | | | | | While this isn't really supposed to happen, it can occur in OOM situations and perhaps others. Instead of crashing, substitute "(no message provided)". I didn't worry about localizing this text, since we aren't localizing anything else here; besides, if we're on the edge of OOM, it's unlikely gettext() would work. Report and fix by Sergio Conde Gómez in bug #15624. Discussion: https://postgr.es/m/15624-4dea54091a2864e6@postgresql.org
* Ensure that foreign scans with lateral refs are planned correctly.Tom Lane2019-02-07
| | | | | | | | | | | | | | | | | | | | | | | | | As reported in bug #15613 from Srinivasan S A, file_fdw and postgres_fdw neglected to mark plain baserel foreign paths as parameterized when the relation has lateral_relids. Other FDWs have surely copied this mistake, so rather than just patching those two modules, install a band-aid fix in create_foreignscan_path to rectify the mistake centrally. Although the band-aid is enough to fix the visible symptom, correct the calls in file_fdw and postgres_fdw anyway, so that they are valid examples for external FDWs. Also, since the band-aid isn't enough to make this work for parameterized foreign joins, throw an elog(ERROR) if such a case is passed to create_foreignscan_path. This shouldn't pose much of a problem for existing external FDWs, since it's likely they aren't trying to make such paths anyway (though some of them may need a defense against joins with lateral_relids, similar to the one this patch installs into postgres_fdw). Add some assertions in relnode.c to catch future occurrences of the same error --- in particular, as backstop against core-code mistakes like the one fixed by commit bdd9a99aa. Discussion: https://postgr.es/m/15613-092be1be9576c728@postgresql.org
* Add collation assignment to CALL statementPeter Eisentraut2019-02-07
| | | | | | | | | Otherwise functions that require collation information will not have it if they are called in arguments to a CALL statement. Reported-by: Jean-Marc Voillequin <Jean-Marc.Voillequin@moodys.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/1EC8157EB499BF459A516ADCF135ADCE39FFAC54%40LON-WGMSX712.ad.moodys.net
* Propagate lateral-reference information to indirect descendant relations.Tom Lane2019-02-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | create_lateral_join_info() computes a bunch of information about lateral references between base relations, and then attempts to propagate those markings to appendrel children of the original base relations. But the original coding neglected the possibility of indirect descendants (grandchildren etc). During v11 development we noticed that this was wrong for partitioned-table cases, but failed to realize that it was just as wrong for any appendrel. While the case can't arise for appendrels derived from traditional table inheritance (because we make a flat appendrel for that), nested appendrels can arise from nested UNION ALL subqueries. Failure to mark the lower-level relations as having lateral references leads to confusion in add_paths_to_append_rel about whether unparameterized paths can be built. It's not very clear whether that leads to any user-visible misbehavior; the lack of field reports suggests that it may cause nothing worse than minor cost misestimation. Still, it's a bug, and it leads to failures of Asserts that I intend to add later. To fix, we need to propagate information from all appendrel parents, not just those that are RELOPT_BASERELs. We can still do it in one pass, if we rely on the append_rel_list to be ordered with ancestor relationships before descendant ones; add assertions checking that. While fixing this, we can make a small performance improvement by traversing the append_rel_list just once instead of separately for each appendrel parent relation. Noted while investigating bug #15613, though this patch does not fix that (which is why I'm not committing the related Asserts yet). Discussion: https://postgr.es/m/3951.1549403812@sss.pgh.pa.us
* Fix heap_getattr() handling of fast defaults.Andres Freund2019-02-06
| | | | | | | | | | | | | | | | | | | | | | | Previously heap_getattr() returned NULL for attributes with a fast default value (c.f. 16828d5c0273), as it had no handling whatsoever for that case. A previous fix, 7636e5c60f, attempted to fix issues caused by this oversight, but just expanding OLD tuples for triggers doesn't actually solve the underlying issue. One known consequence of this bug is that the check for HOT updates can return the wrong result, when a previously fast-default'ed column is set to NULL. Which in turn means that an index over a column with fast default'ed columns might be corrupt if the underlying column(s) allow NULLs. Fix by handling fast default columns in heap_getattr(), remove now superfluous expansion in GetTupleForTrigger(). Author: Andres Freund Discussion: https://postgr.es/m/20190201162404.onngi77f26baem4g@alap3.anarazel.de Backpatch: 11, where fast defaults were introduced
* Keep perl style checker happyAndrew Dunstan2019-02-05
| | | | It doesn't like code before "use strict;".
* Fix searchpath for modern Perl for genbki.plAndrew Dunstan2019-02-05
| | | | | | | This was fixed for MSVC tools by commit 1df92eeafefac4, but per buildfarm member bowerbird genbki.pl needs the same treatment. Backpatch to all live branches.
* Avoid possible deadlock while locking multiple heap pages.Amit Kapila2019-02-02
| | | | | | | | | | | | | | | | | To avoid deadlock, backend acquires a lock on heap pages in block number order. In certain cases, lock on heap pages is dropped and reacquired. In this case, the locks are dropped for reading in corresponding VM page/s. The issue is we re-acquire locks in bufferId order whereas the intention was to acquire in blockid order. This commit ensures that we will always acquire locks on heap pages in blockid order. Reported-by: Nishant Fnu Author: Nishant Fnu Reviewed-by: Amit Kapila and Robert Haas Backpatch-through: 9.4 Discussion: https://postgr.es/m/5883C831-2ED1-47C8-BFAC-2D5BAE5A8CAE@amazon.com
* Fix use of dangling pointer in heap_delete() when logging replica identityMichael Paquier2019-02-01
| | | | | | | | | | | | | | When logging the replica identity of a deleted tuple, XLOG_HEAP_DELETE records include references of the old tuple. Its data is stored in an intermediate variable used to register this information for the WAL record, but this variable gets away from the stack when the record gets actually inserted. Spotted by clang's AddressSanitizer. Author: Stas Kelvish Discussion: https://postgr.es/m/085C8825-AD86-4E93-AF80-E26CDF03D1EA@postgrespro.ru Backpatch-through: 9.4
* Fix a crash in logical replicationPeter Eisentraut2019-01-30
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The bug was that determining which columns are part of the replica identity index using RelationGetIndexAttrBitmap() would run eval_const_expressions() on index expressions and predicates across all indexes of the table, which in turn might require a snapshot, but there wasn't one set, so it crashes. There were actually two separate bugs, one on the publisher and one on the subscriber. To trigger the bug, a table that is part of a publication or subscription needs to have an index with a predicate or expression that lends itself to constant expressions simplification. The fix is to avoid the constant expressions simplification in RelationGetIndexAttrBitmap(), so that it becomes safe to call in these contexts. The constant expressions simplification comes from the calls to RelationGetIndexExpressions()/RelationGetIndexPredicate() via BuildIndexInfo(). But RelationGetIndexAttrBitmap() calling BuildIndexInfo() is overkill. The latter just takes pg_index catalog information, packs it into the IndexInfo structure, which former then just unpacks again and throws away. We can just do this directly with less overhead and skip the troublesome calls to eval_const_expressions(). This also removes the awkward cross-dependency between relcache.c and index.c. Bug: #15114 Reported-by: Петър Славов <pet.slavov@gmail.com> Reviewed-by: Noah Misch <noah@leadboat.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://www.postgresql.org/message-id/flat/152110589574.1223.17983600132321618383@wrigleys.postgresql.org/
* Allow UNLISTEN in hot-standby mode.Tom Lane2019-01-25
| | | | | | | | | | | | | | | Since LISTEN is (still) disallowed, UNLISTEN must be a no-op in a hot-standby session, and so there's no harm in allowing it. This change allows client code to not worry about whether it's connected to a primary or standby server when performing session-state-reset type activities. (Note that DISCARD ALL, which includes UNLISTEN, was already allowed, making it inconsistent to reject UNLISTEN.) Per discussion, back-patch to all supported versions. Shay Rojansky, reviewed by Mi Tar Discussion: https://postgr.es/m/CADT4RqCf2gA_TJtPAjnGzkC3ZiexfBZiLmA-mV66e4UyuVv8bA@mail.gmail.com
* Fix droppability of constraints upon partition detachAlvaro Herrera2019-01-24
| | | | | | | | | | | | | | | | | | | | | | | | We were failing to set conislocal correctly for constraints in partitions after partition detach, leading to those constraints becoming undroppable. Fix by setting the flag correctly. Existing databases might contain constraints with the conislocal wrongly set to false, for partitions that were detached; this situation should be fixable by applying an UPDATE on pg_constraint to set conislocal true. This problem should otherwise be innocuous and should disappear across a dump/restore or pg_upgrade. Secondarily, when constraint drop was attempted in a partitioned table, ATExecDropConstraint would try to recurse to partitions after doing performDeletion() of the constraint in the partitioned table itself; but since the constraint in the partitions are dropped by the initial call of performDeletion() (because of following dependencies), the recursion step would fail since it would not find the constraint, causing the whole operation to fail. Fix by preventing recursion. Reported-by: Amit Langote Diagnosed-by: Amit Langote Author: Amit Langote, Álvaro Herrera Discussion: https://postgr.es/m/f2b8ead5-4131-d5a8-8016-2ea0a31250af@lab.ntt.co.jp
* Simplify coding to detach constraints when detaching partitionAlvaro Herrera2019-01-24
| | | | | | | The original coding was too baroque and led to an use-after-release mistake, noticed by buildfarm member prion. Discussion: https://postgr.es/m/21693.1548305934@sss.pgh.pa.us
* Detach constraints when partitions are detachedAlvaro Herrera2019-01-23
| | | | | | | | | I (Álvaro) forgot to do this in eb7ed3f30634, leading to undroppable constraints after partitions are detached. Repair. Reported-by: Amit Langote Author: Amit Langote Discussion: https://postgr.es/m/c1c9b688-b886-84f7-4048-1e4ebe9b1d06@lab.ntt.co.jp
* Fix misc typos in comments.Heikki Linnakangas2019-01-23
| | | | | | Spotted mostly by Fabien Coelho. Discussion: https://www.postgresql.org/message-id/alpine.DEB.2.21.1901230947050.16643@lancre
* llvm: Fix file-ending in IDENTIFICATION comments.Andres Freund2019-01-22
| | | | | | Author: Amit Langote Discussion: https://postgr.es/m/9a54dcef-c799-ce89-2e47-0a7fc12d5fc2@lab.ntt.co.jp Backpatch: 11-, where llvm was introduced.
* Remove useless bms_copy step in RelationGetIndexAttrBitmap.Tom Lane2019-01-21
| | | | | | | | | | Seems to be from a bad case of copy-and-paste-itis in commit 665d1fad9. It wouldn't be quite so annoying if it didn't contradict the comment half a dozen lines above. David Rowley Discussion: https://postgr.es/m/CAKJS1f95Dyf8Qkdz4W+PbCmT-HTb54tkqUCC8isa2RVgSJ_pXQ@mail.gmail.com
* Create action triggers when partitions are detachedAlvaro Herrera2019-01-21
| | | | | | | | | | | | | | | | Detaching a partition from a partitioned table that's constrained by foreign keys requires additional action triggers on the referenced side; otherwise, DELETE/UPDATE actions there fail to notice rows in the table that was partition, and so are incorrectly allowed through. With this commit, those triggers are now created. Conversely, when a table that has a foreign key is attached as a partition to a table that also has the same foreign key, those action triggers are no longer needed, so we remove them. Add a minimal test case verifying (part of) this. Authors: Amit Langote, Álvaro Herrera Discussion: https://postgr.es/m/f2b8ead5-4131-d5a8-8016-2ea0a31250af@lab.ntt.co.jp
* Flush relcache entries when their FKs are meddled withAlvaro Herrera2019-01-21
| | | | | | | | | | | | | | | Back in commit 100340e2dcd0, we made relcache entries keep lists of the foreign keys applying to the relation -- but we forgot to update CacheInvalidateHeapTuple to flush those entries when new FKs got created or existing ones updated/deleted. No bugs appear to have been reported that would be explained by this ommission, but I noticed the problem while working on an unrelated bugfix which clearly showed it. Fix by adding relcache flush on relevant foreign key changes. Backpatch to 9.6, like the aforementioned commit. Discussion: https://postgr.es/m/201901211927.7mmhschxlejh@alvherre.pgsql Reviewed-by: Tom Lane
* Postpone generating tlists and EC members for inheritance dummy children.Etsuro Fujita2019-01-21
| | | | | | | | | | | | | | | | | | | | | Previously, in set_append_rel_size(), we generated tlists and EC members for dummy children for possible use by partition-wise join, even if partition-wise join was disabled or the top parent was not a partitioned table, but adding such EC members causes noticeable planning speed degradation for queries with certain kinds of join quals like "(foo.x + bar.y) = constant" where foo and bar are partitioned tables in cases where there are lots of dummy children, as the EC members lists grow huge, especially for the ECs derived from such join quals, which makes the search for the parent EC members in add_child_rel_equivalences() very time-consuming. Postpone the work until such children are actually involved in a partition-wise join. Reported-by: Sanyo Capobiango Analyzed-by: Justin Pryzby and Alvaro Herrera Author: Amit Langote, with a few additional changes by me Reviewed-by: Ashutosh Bapat Backpatch-through: v11 where partition-wise join was added Discussion: https://postgr.es/m/CAO698qZnrxoZu7MEtfiJmpmUtz3AVYFVnwzR%2BpqjF%3DrmKBTgpw%40mail.gmail.com
* Fix outdated commentPeter Eisentraut2019-01-19
| | | | | The issue the comment is referring to was fixed by 08859bb5c2cebc132629ca838113d27bb31b990c.
* Fix creation of duplicate foreign keys on partitionsAlvaro Herrera2019-01-18
| | | | | | | | | | | | | | | | | | When creating a foreign key in a partitioned table, if some partitions already have equivalent constraints, we wastefully create duplicates of the constraints instead of attaching to the existing ones. That's inconsistent with the de-duplication that is applied when a table is attached as a partition. To fix, reuse the FK-cloning code instead of having a separate code path. Backpatch to Postgres 11. This is a subtle behavior change, but surely a welcome one since there's no use in having duplicate foreign keys. Discovered by Álvaro Herrera while thinking about a different problem reported by Jesper Pedersen (bug #15587). Author: Álvaro Herrera Discussion: https://postgr.es/m/201901151935.zfadrzvyof4k@alvherre.pgsql
* Move CloneForeignKeyConstraints to tablecmds.cAlvaro Herrera2019-01-18
| | | | | | | | | | | | | | | My commit 3de241dba86f introduced some code to create a clone of a foreign key to a partition, but I put it in pg_constraint.c because it was too close to the contents of the pg_constraint row. With the previous commit that split out the constraint tuple deconstruction into its own routine, it makes more sense to have the FK-cloning function in tablecmds.c, mostly because its static subroutine can then be used by a future bugfix. My initial posting of this patch had this routine as static in tablecmds.c, but sadly this function is already part of the Postgres 11 ABI as exported from pg_constraint.c, so keep it as exported also just to avoid breaking any possible users of it.
* Refactor duplicate code into DeconstructFkConstraintRowAlvaro Herrera2019-01-18
| | | | | | | | | | | My commit 3de241dba86f introduced some code (in tablecmds.c) to obtain data from a pg_constraint row for a foreign key, that already existed in ri_triggers.c. Split it out into its own routine in pg_constraint.c, where it naturally belongs. No functional code changes, only code movement. Backpatch to pg11, because a future bugfix is simpler after this.
* Restrict the use of temporary namespace in two-phase transactionsMichael Paquier2019-01-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Attempting to use a temporary table within a two-phase transaction is forbidden for ages. However, there have been uncovered grounds for a couple of other object types and commands which work on temporary objects with two-phase commit. In short, trying to create, lock or drop an object on a temporary schema should not be authorized within a two-phase transaction, as it would cause its state to create dependencies with other sessions, causing all sorts of side effects with the existing session or other sessions spawned later on trying to use the same temporary schema name. Regression tests are added to cover all the grounds found, the original report mentioned function creation, but monitoring closer there are many other patterns with LOCK, DROP or CREATE EXTENSION which are involved. One of the symptoms resulting in combining both is that the session which used the temporary schema is not able to shut down completely, waiting for being able to drop the temporary schema, something that it cannot complete because of the two-phase transaction involved with temporary objects. In this case the client is able to disconnect but the session remains alive on the backend-side, potentially blocking connection backend slots from being used. Other problems reported could also involve server crashes. This is back-patched down to v10, which is where 9b013dc has introduced MyXactFlags, something that this patch relies on. Reported-by: Alexey Bashtanov Author: Michael Paquier Reviewed-by: Masahiko Sawada Discussion: https://postgr.es/m/5d910e2e-0db8-ec06-dd5f-baec420513c3@imap.cc Backpatch-through: 10
* Postpone aggregate checks until after collation is assigned.Andrew Gierth2019-01-17
| | | | | | | | | | | | | | | | | | | | | | | Previously, parseCheckAggregates was run before assign_query_collations, but this causes problems if any expression has already had a collation assigned by some transform function (e.g. transformCaseExpr) before parseCheckAggregates runs. The differing collations would cause expressions not to be recognized as equal to the ones in the GROUP BY clause, leading to spurious errors about unaggregated column references. The result was that CASE expr WHEN val ... would fail when "expr" contained a GROUPING() expression or matched one of the group by expressions, and where collatable types were involved; whereas the supposedly identical CASE WHEN expr = val ... would succeed. Backpatch all the way; this appears to have been wrong ever since collations were introduced. Per report from Guillaume Lelarge, analysis and patch by me. Discussion: https://postgr.es/m/CAECtzeVSO_US8C2Khgfv54ZMUOBR4sWq+6_bLrETnWExHT=rFg@mail.gmail.com Discussion: https://postgr.es/m/87muo0k0c7.fsf@news-spur.riddles.org.uk
* Fix typos in documentation and for one wait eventMichael Paquier2019-01-15
| | | | | | | | | These have been found while cross-checking for the use of unique words in the documentation, and a wait event was not getting generated in a way consistent to what the documentation provided. Author: Alexander Lakhin Discussion: https://postgr.es/m/9b5a3a85-899a-ae62-dbab-1e7943aa5ab1@gmail.com
* Fix unique INCLUDE indexes on partitioned tablesAlvaro Herrera2019-01-14
| | | | | | | | | | We were considering the INCLUDE columns as part of the key, allowing unicity-violating rows to be inserted in different partitions. Concurrent development conflict in eb7ed3f30634 and 8224de4f42cc. Reported-by: Justin Pryzby Discussion: https://postgr.es/m/20190109065109.GA4285@telsasoft.com
* Free pre-modification HeapTuple in ALTER TABLE ... TYPE ...Andrew Dunstan2019-01-11
| | | | | | | | This was an oversight in commit 3b174b1a3. Per offline gripe from Alvaro Herrera Backpatch to release 11.
* Avoid sharing PARAM_EXEC slots between different levels of NestLoop.Tom Lane2019-01-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Up to now, createplan.c attempted to share PARAM_EXEC slots for NestLoopParams across different plan levels, if the same underlying Var was being fed down to different righthand-side subplan trees by different NestLoops. This was, I think, more of an artifact of using subselect.c's PlannerParamItem infrastructure than an explicit design goal, but anyway that was the end result. This works well enough as long as the plan tree is executing synchronously, but the feature whereby Gather can execute the parallelized subplan locally breaks it. An upper NestLoop node might execute for a row retrieved from a parallel worker, and assign a value for a PARAM_EXEC slot from that row, while the leader's copy of the parallelized subplan is suspended with a different active value of the row the Var comes from. When control eventually returns to the leader's subplan, it gets the wrong answers if the same PARAM_EXEC slot is being used within the subplan, as reported in bug #15577 from Bartosz Polnik. This is pretty reminiscent of the problem fixed in commit 46c508fbc, and the proper fix seems to be the same: don't try to share PARAM_EXEC slots across different levels of controlling NestLoop nodes. This requires decoupling NestLoopParam handling from PlannerParamItem handling, although the logic remains somewhat similar. To avoid bizarre division of labor between subselect.c and createplan.c, I decided to move all the param-slot-assignment logic for both cases out of those files and put it into a new file paramassign.c. Hopefully it's a bit better documented now, too. A regression test case for this might be nice, but we don't know a test case that triggers the problem with a suitably small amount of data. Back-patch to 9.6 where we added Gather nodes. It's conceivable that related problems exist in older branches; but without some evidence for that, I'll leave the older branches alone. Discussion: https://postgr.es/m/15577-ca61ab18904af852@postgresql.org
* Fix missing values when doing ALTER TABLE ALTER COLUMN TYPEAndrew Dunstan2019-01-10
| | | | | | | | | | | | | | This was an oversight in commit 16828d5c. If the table is going to be rewritten, we simply clear all the missing values from all the table's attributes, since there will no longer be any rows with the attributes missing. Otherwise, we repackage the missing value in an array constructed with the new type specifications. Backpatch to release 11. This fixes bug #15446, reported by Dmitry Molotkov Reviewed by Dean Rasheed
* Improve ANALYZE's handling of concurrent-update scenarios.Tom Lane2019-01-03
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch changes the rule for whether or not a tuple seen by ANALYZE should be included in its sample. When we last touched this logic, in commit 51e1445f1, we weren't thinking very hard about tuples being UPDATEd by a long-running concurrent transaction. In such a case, we might see the pre-image as either LIVE or DELETE_IN_PROGRESS depending on timing; and we might see the post-image not at all, or as INSERT_IN_PROGRESS. Since the existing code will not sample either DELETE_IN_PROGRESS or INSERT_IN_PROGRESS tuples, this leads to concurrently-updated rows being omitted from the sample entirely. That's not very helpful, and it's especially the wrong thing if the concurrent transaction ends up rolling back. The right thing seems to be to sample DELETE_IN_PROGRESS rows just as if they were live. This makes the "sample it" and "count it" decisions the same, which seems good for consistency. It's clearly the right thing if the concurrent transaction ends up rolling back; in effect, we are sampling as though IN_PROGRESS transactions haven't happened yet. Also, this combination of choices ensures maximum robustness against the different combinations of whether and in which state we might see the pre- and post-images of an update. It's slightly annoying that we end up recording immediately-out-of-date stats in the case where the transaction does commit, but on the other hand the stats are fine for columns that didn't change in the update. And the alternative of sampling INSERT_IN_PROGRESS rows instead seems like a bad idea, because then the sampling would be inconsistent with the way rows are counted for the stats report. Per report from Mark Chambers; thanks to Jeff Janes for diagnosing what was happening. Back-patch to all supported versions. Discussion: https://postgr.es/m/CAFh58O_Myr6G3tcH3gcGrF-=OExB08PJdWZcSBcEcovaiPsrHA@mail.gmail.com
* Don't believe MinMaxExpr is leakproof without checking.Tom Lane2019-01-02
| | | | | | | | | | | | | | | | MinMaxExpr invokes the btree comparison function for its input datatype, so it's only leakproof if that function is. Many such functions are indeed leakproof, but others are not, and we should not just assume that they are. Hence, adjust contain_leaked_vars to verify the leakproofness of the referenced function explicitly. I didn't add a regression test because it would need to depend on some particular comparison function being leaky, and that's a moving target, per discussion. This has been wrong all along, so back-patch to supported branches. Discussion: https://postgr.es/m/31042.1546194242@sss.pgh.pa.us
generated by cgit v1.2.3 (git 2.25.1) at 2025-08-05 14:40:54 +0000