- 02 Jan, 2020 10 commits
-
-
Alvaro Herrera authored
When row triggers exist in partitioned partitions that are not either part of FKs or deferred unique constraints, they are not correctly cloned to their partitions. That's because they are marked "internal", and those are purposefully skipped when doing the clone triggers dance. Fix by relaxing the condition on which internal triggers are skipped. Amit Langote initially diagnosed the problem and proposed a fix, but I used a different approach. Reported-by: Petr Fedorov Discussion: https://postgr.es/m/6b3f0646-ba8c-b3a9-c62d-1c6651a1920f@phystech.edu
-
Tom Lane authored
Up to now, psql's tab-complete.c has had exactly no regression test coverage. This patch is an experimental attempt to add some. This needs Perl's IO::Pty module, which isn't installed everywhere, so the test script just skips all tests if that's not present. There may be other portability gotchas too, so I await buildfarm results with interest. So far this just covers a few very basic keyword-completion and query-driven-completion scenarios, which should be enough to let us get a feel for whether this is practical at all from a portability standpoint. If it is, there's lots more that can be done. Discussion: https://postgr.es/m/10967.1577562752@sss.pgh.pa.us
-
Peter Geoghegan authored
Make the function prototype order consistent with the definition order in nbtpage.c.
-
Tom Lane authored
One code path in addRangeTableEntryForFunction() neglected to assign a collation to the tupdesc entry it constructs (which is a bit odd considering the other path did do so). This didn't matter before commit 5815696b, because nothing would look at the type data in this tupdesc; but now it does. While at it, make sure we assign the correct typmod as well. Most function expressions don't have a determinate typmod, but some do. Per buildfarm, which showed failures in non-C collations, a case I'd not thought to test for this patch :-(
-
Tom Lane authored
When I added the ParseNamespaceItem data structure (in commit 5ebaaa49), it wasn't very tightly integrated into the parser's APIs. In the wake of adding p_rtindex to that struct (commit b541e9ac), there is a good reason to make more use of it: by passing around ParseNamespaceItem pointers instead of bare RTE pointers, we can get rid of various messy methods for passing back or deducing the rangetable index of an RTE during parsing. Hence, refactor the addRangeTableEntryXXX functions to build and return a ParseNamespaceItem struct, not just the RTE proper; and replace addRTEtoQuery with addNSItemToQuery, which is passed a ParseNamespaceItem rather than building one internally. Also, add per-column data (a ParseNamespaceColumn array) to each ParseNamespaceItem. These arrays are built during addRangeTableEntryXXX, where we have column type data at hand so that it's nearly free to fill the data structure. Later, when we need to build Vars referencing RTEs, we can use the ParseNamespaceColumn info to avoid the rather expensive operations done in get_rte_attribute_type() or expandRTE(). get_rte_attribute_type() is indeed dead code now, so I've removed it. This makes for a useful improvement in parse analysis speed, around 20% in one moderately-complex test query. The ParseNamespaceColumn structs also include Var identity information (varno/varattno). That info isn't actually being used in this patch, except that p_varno == 0 is a handy test for a dropped column. A follow-on patch will make more use of it. Discussion: https://postgr.es/m/2461.1577764221@sss.pgh.pa.us
-
Peter Eisentraut authored
The comment was apparently copy-and-pasted and did not reflect the actual test outcome.
-
Amit Kapila authored
Currently while decoding changes, if the number of changes exceeds a certain threshold, we spill those to disk. And this happens for each (sub)transaction. Now, while reading all these files, we don't close them until we read all the files. While reading these files, if the number of such files exceeds the maximum number of file descriptors, the operation errors out. Use PathNameOpenFile interface to open these files as that internally has the mechanism to release kernel FDs as needed to get us under the max_safe_fds limit. Reported-by: Amit Khandekar Author: Amit Khandekar Reviewed-by: Amit Kapila Backpatch-through: 9.4 Discussion: https://postgr.es/m/CAJ3gD9c-sECEn79zXw4yBnBdOttacoE-6gAyP0oy60nfs_sabQ@mail.gmail.com
-
Peter Geoghegan authored
_bt_delitems_vacuum() comments claimed that it isn't worth another scan of the page to avoid falsely unsetting the BTP_HAS_GARBAGE page flag hint (this happens to be the same wording that was removed from _bt_delitems_delete() by my recent commit fe97c61c). The comments made little sense, though. The issue can't have much to do with performing a second scan of the target leaf page, since an LP_DEAD test could easily be performed in the first scan of the page anyway (the scan that takes place in btvacuumpage() caller). Revise the explanation. It makes much more sense to frame this as an issue about recovery conflicts. _bt_delitems_vacuum() cannot easily generate an XID cutoff in the same way that _bt_delitems_delete() is designed to. Falsely unsetting the page flag is not ideal, and is likely to happen more often than was supposed by the original comments. Explain why it usually isn't a problem in practice. There may be an argument for _bt_delitems_vacuum() not clearing the BTP_HAS_GARBAGE bit, removing the question of it being falsely unset by VACUUM (there may even be an argument for not using a page level hint at all). This can be revisited later.
-
Tom Lane authored
If we have, say, an int column that is left-joined to a bigint column with USING, the merged column is the int column promoted to bigint. GROUP BY's tests for whether grouping on the merged column allows a reference to the underlying column, or vice versa, should know about that relationship --- and they do. But I nearly broke this case with an ill-advised optimization, so the lack of any test coverage for it seems like a bad idea.
-
- 01 Jan, 2020 2 commits
-
-
Peter Geoghegan authored
Commit fe97c61c updated LP_DEAD item deletion comments, but missed a minor discrepancy on the REDO side. Fix it now. In passing, don't talk about the btree_xlog_vacuum() behavior within btree_xlog_delete(). The reliance on XLOG_HEAP2_CLEANUP_INFO records for recovery conflicts is already discussed within btvacuumpage() and mentioned again in passing above btree_xlog_vacuum(), which seems sufficient.
-
Bruce Momjian authored
Backpatch-through: update all files in master, backpatch legal files through 9.4
-
- 31 Dec, 2019 1 commit
-
-
Peter Eisentraut authored
Change the exception syntax used in the documentation to use the more current except Exception as ex: rather than the old except Exception, ex: We keep the old syntax in the test code since Python <2.6 is still supported there, but the documentation might as well use the modern syntax.
-
- 28 Dec, 2019 1 commit
-
-
Tom Lane authored
Use __builtin_clz() where available. Where it isn't, we can still win a little by using the pg_leftmost_one_pos[] lookup table instead of having a private table. Also drop the initial right shift by ALLOC_MINBITS in favor of subtracting ALLOC_MINBITS from the leftmost-one-pos result. This is a win because the compiler can fold that adjustment into other constants it'd have to add anyway, making the shift-removal free. Also, we can explain this coding as an unrolled form of pg_leftmost_one_pos32(), even though that's a bit ahistorical since it long predates pg_bitutils.h. John Naylor, with some cosmetic adjustments by me Discussion: https://postgr.es/m/CACPNZCuNUGMxjK7WTn_=WZnRbfASDdBxmjsVf2+m9MdmeNw_sg@mail.gmail.com
-
- 27 Dec, 2019 4 commits
-
-
Alvaro Herrera authored
This currently works, but add this test to ensure it continues to work. Lack of this test became evident after a recent bugfix submission that would have inadvertently broken it, in https://postgr.es/m/CA+HiwqFM2=i+uHB9o4OkLbE2S3sjPHoVe2wXuAD1GLJ4+Pk9eg@mail.gmail.com
-
Bruce Momjian authored
Unique expression indexes can constrain data in creative ways, so show two examples. Reported-by: Tuomas Leikola Discussion: https://postgr.es/m/156760275564.1127.12321702656456074572@wrigleys.postgresql.org Backpatch-through: 9.4
-
Bruce Momjian authored
The previous docs referenced these distinct ideas confusingly. Reported-by: Eugen Konkov Discussion: https://postgr.es/m/376945611.20191026161529@yandex.ru Backpatch-through: 9.4
-
Michael Paquier authored
This operation was possible for the owner of the schema or a superuser. Down to 9.4, doing this operation would cause inconsistencies in a session whose temporary schema was dropped, particularly if trying to create new temporary objects after the drop. A more annoying consequence is a crash of autovacuum on an assertion failure when logging information about an orphaned temp table dropped. Note that because of 246a6c8f (present in v11~), which has made the removal of orphaned temporary tables more aggressive, the failure could be triggered more easily, but it is possible to reproduce down to 9.4. Reported-by: Mahendra Singh, Prabhat Sahu Author: Michael Paquier Reviewed-by: Kyotaro Horiguchi, Mahendra Singh Discussion: https://postgr.es/m/CAKYtNAr9Zq=1-ww4etHo-VCC-k120YxZy5OS01VkaLPaDbv2tg@mail.gmail.com Backpatch-through: 9.4
-
- 26 Dec, 2019 7 commits
-
-
Michael Paquier authored
This follows multiple complains from Peter Geoghegan, Andres Freund and Alvaro Herrera that this issue ought to be dug more before actually happening, if it happens. Discussion: https://postgr.es/m/20191226144606.GA5659@alvherre.pgsql
-
Tom Lane authored
When revalidate_rectypeid() acts to update a stale record type OID in plpgsql's data structures, it fixes the active PLpgSQL_rec struct as well as the PLpgSQL_type struct it references. However, the latter is shared across function executions while the former is not. In a later function execution, the PLpgSQL_rec struct would be reinitialized by copy_plpgsql_datums and would then contain a stale type OID, typically leading to "could not open relation with OID NNNN" errors. revalidate_rectypeid() can easily fix this, fortunately, just by treating typ->typoid as authoritative. Per report and diagnosis from Ashutosh Sharma, though this is not his suggested fix. Back-patch to v11 where this code came in. Discussion: https://postgr.es/m/CAE9k0Pkd4dZwt9J5pS9xhJFWpUtqs05C9xk_GEwPzYdV=GxwWg@mail.gmail.com
-
Tom Lane authored
Mark the fields that should be accessed via partitioning-related functions, as we already did for some other fields. Amit Langote Discussion: https://postgr.es/m/CA+HiwqFnK6LbVMACMCaqwWrvoSFTecZzufKRahg2qGvLPYMX=g@mail.gmail.com
-
Tom Lane authored
Instead of passing around a pointer to the RangeTblEntry that provides the desired column, pass a pointer to the associated ParseNamespaceItem. The RTE is trivially reachable from the nsitem, and having the ParseNamespaceItem allows access to additional information. As proof of concept for that, add the rangetable index to ParseNamespaceItem, and use that to get rid of RTERangeTablePosn searches. (I have in mind to teach the parser to generate some different representation for Vars that are nullable by outer joins, and keeping the necessary information in ParseNamespaceItems seems like a reasonable approach to that. But whether that ever happens or not, this seems like good cleanup.) Also refactor the code around scanRTEForColumn so that the "fuzzy match" stuff does not leak out of parse_relation.c. Discussion: https://postgr.es/m/26144.1576858373@sss.pgh.pa.us
-
Michael Paquier authored
confirmed_flush is part of a replication slot's information, but not confirmed_lsn. Author: Kyotaro Horiguchi Discussion: https://postgr.es/m/20191226.175919.17237335658671970.horikyota.ntt@gmail.com Backpatch-through: 11
-
Michael Paquier authored
The part in charge of doing the vacuum on all the indexes of a relation was duplicated, with the same handling for progress reporting done. While on it, update the progress reporting for heap vacuuming in the subroutine doing the actual work, keeping the status update local. This way, any future caller of lazy_vacuum_heap() does not have to worry about doing any progress reporting update. Author: Justin Pryzby, Michael Paquier Discussion: https://postgr.es/m/20191120210600.GC30362@telsasoft.com
-
Fujii Masao authored
Column defaults may be specified separately for each partition. But INSERT via a partitioned table ignores those partition's default values. The former is documented, but the latter restriction not. This commit adds the note about that restriction into the document. Author: Fujii Masao Reviewed-by: Amit Langote Discussion: https://postgr.es/m/CAHGQGwEs-59omrfGF7hOHz9iMME3RbKy5ny+iftDx3LHTEn9sA@mail.gmail.com
-
- 25 Dec, 2019 4 commits
-
-
Tom Lane authored
In the wake of commit 5b931237, there's no particular reason for this restriction (previously, it was problematic because of the implied rowtype reference). A simple constraint on a whole-row Var probably isn't that useful, but conceivably somebody would want to pass one to a function that extracts a partitioning key. Besides which, we're expending much more code to enforce the restriction than we save by having it, since the latter quantity is now zero. So drop the restriction. Amit Langote Discussion: https://postgr.es/m/CA+HiwqFUzjfj9HEsJtYWcr1SgQ_=iCAvQ=O2Sx6aQxoDu4OiHw@mail.gmail.com
-
Tom Lane authored
This is dead code in the wake of the previous commit. We can always add it back if we need it again someday. Discussion: https://postgr.es/m/CA+HiwqFUzjfj9HEsJtYWcr1SgQ_=iCAvQ=O2Sx6aQxoDu4OiHw@mail.gmail.com
-
Tom Lane authored
Formerly the rd_partkey and rd_partdesc data structures were always populated immediately when a relcache entry was built or rebuilt. This patch changes things so that they are populated only when they are first requested. (Hence, callers *must* now always use RelationGetPartitionKey or RelationGetPartitionDesc; just fetching the pointer directly is no longer acceptable.) This seems to have some performance benefits, but the main reason to do it is that it eliminates a recursive-reload failure that occurs if the partkey or partdesc expressions contain any references to the relation's rowtype (as discovered by Amit Langote). In retrospect, since loading these data structures might result in execution of nearly-arbitrary code via eval_const_expressions, it was a dumb idea to require that to happen during relcache entry rebuild. Also, fix things so that old copies of a relcache partition descriptor will be dropped when the cache entry's refcount goes to zero. In the previous coding it was possible for such copies to survive for the lifetime of the session, as I'd complained of in a previous discussion. (This management technique still isn't perfect, but it's better than before.) Improve the commentary explaining how that works and why it's safe to hand out direct pointers to these relcache substructures. In passing, improve RelationBuildPartitionDesc by using the same memory-context-parent-swap approach used by RelationBuildPartitionKey, thereby making it less dependent on strong assumptions about what partition_bounds_copy does. Avoid doing get_rel_relkind in the critical section, too. Patch by Amit Langote and Tom Lane; Robert Haas deserves some credit for prior work in the area, too. Although this is a pre-existing problem, no back-patch: the patch seems too invasive to be safe to back-patch, and the bug it fixes is a corner case that seems relatively unlikely to cause problems in the field. Discussion: https://postgr.es/m/CA+HiwqFUzjfj9HEsJtYWcr1SgQ_=iCAvQ=O2Sx6aQxoDu4OiHw@mail.gmail.com Discussion: https://postgr.es/m/CA+TgmoY3bRmGB6-DUnoVy5fJoreiBJ43rwMrQRCdPXuKt4Ykaw@mail.gmail.com
-
Michael Paquier authored
The following renaming is done so as source files related to index access methods are more consistent with table access methods (the original names used for index AMs ware too generic, and could be confused as including features related to table AMs): - amapi.h -> indexam.h. - amapi.c -> indexamapi.c. Here we have an equivalent with backend/access/table/tableamapi.c. - amvalidate.c -> indexamvalidate.c. - amvalidate.h -> indexamvalidate.h. - genam.c -> indexgenam.c. - genam.h -> indexgenam.h. This has been discussed during the development of v12 when table AM was worked on, but the renaming never happened. Author: Michael Paquier Reviewed-by: Fabien Coelho, Julien Rouhaud Discussion: https://postgr.es/m/20191223053434.GF34339@paquier.xyz
-
- 24 Dec, 2019 3 commits
-
-
Alvaro Herrera authored
Using \ is unnecessary and ugly, so remove that. While at it, stitch the literals back into a single line: we've long discouraged splitting error message literals even when they go past the 80 chars line limit, to improve greppability. Leave contrib/tablefunc alone. Discussion: https://postgr.es/m/20191223195156.GA12271@alvherre.pgsql
-
Michael Paquier authored
Since d6c55de1, src/port/snprintf.c is able to use %m instead of strerror(). A couple of utilities in src/bin/ have already done the switch, and do it now for pg_waldump as this reduces the workload for translators. Note that more could be done, particularly with pgbench. Thanks to Kyotaro Horiguchi for the discussion. Discussion: https://postgr.es/m/20191129065115.GM2505@paquier.xyz
-
Thomas Munro authored
Our algorithm for choosing batch numbers turned out not to work effectively for multi-billion key inner relations. We would use more hash bits than we have, and effectively concentrate all tuples into a smaller number of batches than we intended. While ideally we should switch to wider hashes, for now, change the algorithm to one that effectively gives up bits from the bucket number when we don't have enough bits. That means we'll finish up with longer bucket chains than would be ideal, but that's better than having batches that don't fit in work_mem and can't be divided. Batch-patch to all supported releases. Author: Thomas Munro Reviewed-by: Tom Lane, thanks also to Tomas Vondra, Alvaro Herrera, Andres Freund for testing and discussion Reported-by: James Coleman Discussion: https://postgr.es/m/16104-dc11ed911f1ab9df%40postgresql.org
-
- 23 Dec, 2019 6 commits
-
-
Joe Conway authored
While building a hash map of categories in load_categories_hash, resulting category names have not thus far been checked to ensure they are not null. Prior to pg12 null category names worked to the extent that they did not crash on some platforms. This is because those system libraries have an snprintf which can deal with being passed a null pointer argument for a string. But even in those cases null categories did nothing useful. And on some platforms it crashed. As of pg12, our own version of snprintf gets called, and it does not deal with null pointer arguments at all, and crashes consistently. Fix that by disallowing null categories. They never worked usefully, and no one has ever asked for them to work previously. Back-patch to all supported branches. Reported-By: Ireneusz Pluta Discussion: https://postgr.es/m/16176-7489719b05e4303c@postgresql.org
-
Tom Lane authored
This wasn't checked originally, but it should have been, because in general pseudo-types can't be stored to and retrieved from disk. Notably, partition bound values of type "record" would not be interpretable by another session. In v12 and HEAD, add another flag to CheckAttributeType's repertoire so that it can produce a specific error message for this case. That's infeasible in older branches without an ABI break, so fall back to a slightly-less-nicely-worded error message in v10 and v11. Problem noted by Amit Langote, though this patch is not his initial solution. Back-patch to v10 where partitioning was introduced. Discussion: https://postgr.es/m/CA+HiwqFUzjfj9HEsJtYWcr1SgQ_=iCAvQ=O2Sx6aQxoDu4OiHw@mail.gmail.com
-
Tom Lane authored
We probably should have thought of this case when ranges were added, but we didn't. (It's not the fault of commit eb51af71, because ranges didn't exist then.) It's an old bug, so back-patch to all supported branches. Discussion: https://postgr.es/m/7782.1577051475@sss.pgh.pa.us
-
Alvaro Herrera authored
Use get_publication_oid() instead of reimplementing it. Discussion: https://postgr.es/m/20191220201017.GA17292@alvherre.pgsql
-
Peter Geoghegan authored
Make a function prototype argument's name match the function definition's argument name.
-
Peter Geoghegan authored
Comments about the consequences of clearing the BTP_HAS_GARBAGE page flag bit that apply only to VACUUM were added to code that deals with opportunistic deletion of LP_DEAD items by commit a760893d. The same comment block was added to both _bt_delitems_vacuum() and _bt_delitems_delete(). Correct _bt_delitems_delete()'s copy of the comment block. _bt_delitems_delete() reliably deletes items that were found by caller to have their LP_DEAD bit set. There is no question about whether or not unsetting the BTP_HAS_GARBAGE bit can miss some LP_DEAD items that were set recently. Also tweak a related section of the nbtree README.
-
- 22 Dec, 2019 2 commits
-
-
Tom Lane authored
If the first transaction block in these tests were entered exactly at midnight (California time), they'd report a bogus failure due to 'now' and 'midnight' having the same values. Commit 8c2ac75c had dismissed this as being of negligible probability, but we've now seen it happen in the buildfarm, so let's prevent it. We can get pretty much the same test coverage without an it's-not-midnight assumption by moving the does-'now'-work cases into their own test step. While here, apply commit 47169c25's s/DELETE/TRUNCATE/ change to timestamptz as well as timestamp (not sure why that didn't occur to me at the time; the risk of failure is the same). Back-patch to all supported branches, since the main point is to get rid of potential buildfarm failures. Discussion: https://postgr.es/m/14821.1577031117@sss.pgh.pa.us
-
Peter Eisentraut authored
_GNU_SOURCE is required to get the prototype, so just define that globally, as was already done in the linux template. Discussion: https://www.postgresql.org/message-id/flat/6b467edc-4018-521f-ab18-171f098557ca%402ndquadrant.com
-