- 28 Jan, 2021 3 commits
-
-
Andrew Gierth authored
When building aggregate expression steps, strict checks need a bailout jump for when a null value is encountered, so there is a list of steps that require later adjustment. Adding entries to that list for steps that aren't actually strict would be harmless, except that there is an Assert which catches them. This leads to spurious errors on asserts builds, for data sets that trigger parallel aggregation of an aggregate with a non-strict deserialization function (no such aggregates exist in the core system). Repair by not adding the adjustment entry when it's not needed. Backpatch back to 11 where the code was introduced. Per a report from Darafei (Komzpa) of the PostGIS project; analysis and patch by me. Discussion: https://postgr.es/m/87mty7peb3.fsf@news-spur.riddles.org.uk
-
Michael Paquier authored
Other code paths are already protected against this case, and _PG_init() warns about that in pg_stat_statements.c. While on it, I have checked the other extensions of the tree but did not notice any holes. Oversight in 9fbc3f31. Author: Jaime Casanova Reviewed-by: Julien Rouhaud Discussion: https://postgr.es/m/CAJKUy5gF4=_=qhJ1VX_tSGFfjKHb9BvzhRYWSApJD=Bfwp2SBw@mail.gmail.com
-
Michael Paquier authored
The same code pattern was repeated four times when compiling a SHA-2 hash. This refactoring has the advantage to issue a compilation warning if a new value is added to pg_cryptohash_type, so as anybody doing an addition in this area would need to consider if support for a new SQL function is needed or not. Author: Sehrope Sarkuni, Michael Paquier Discussion: https://postgr.es/m/YA7DvLRn2xnTgsMc@paquier.xyz
-
- 27 Jan, 2021 8 commits
-
-
Peter Geoghegan authored
When commit f425b605 introduced cost based vacuum delays back in 2004, the defaults reflected then-current trends in hardware, as well as certain historical limitations in PostgreSQL. There have been enormous improvements in both areas since that time. The cost limit GUC defaults finally became much more representative of current trends following commit cbccac37, which decreased autovacuum_vacuum_cost_delay's default by 10x for PostgreSQL 12 (it went from 20ms to only 2ms). The relative costs have shifted too. This should also be accounted for by the defaults. More specifically, the relative importance of avoiding dirtying pages within VACUUM has greatly increased, primarily due to main memory capacity scaling and trends in flash storage. Within Postgres itself, improvements like sequential access during index vacuuming (at least in nbtree and GiST indexes) have also been contributing factors. To reflect all this, decrease the default of vacuum_cost_page_miss to 2. Since the default of vacuum_cost_page_dirty remains 20, dirtying a page is now considered 10x "costlier" than a page miss by default. Author: Peter Geoghegan <pg@bowt.ie> Discussion: https://postgr.es/m/CAH2-WzmLPFnkWT8xMjmcsm7YS3+_Qi3iRWAb2+_Bc8UhVyHfuA@mail.gmail.com
-
Robert Haas authored
Since the CLOG page number is not recorded directly in the checkpoint record, we have to use ShmemVariableCache->nextXid to figure out the latest CLOG page number at the start of recovery. However, as recovery progresses, replay of CLOG/EXTEND records will update our notion of the latest page number, and we should rely on that being accurate rather than recomputing the value based on an updated notion of nextXid. ShmemVariableCache->nextXid is only an approximation during recovery anyway, whereas CLOG/EXTEND records are an authoritative representation of how the SLRU has been updated. Commit 0fcc2dec makes this simplification possible, as before that change clog_redo() might have injected a bogus value here, and we'd want to get rid of that before entering normal running. Patch by me, reviewed by Heikki Linnakangas. Discussion: http://postgr.es/m/CA+TgmoZYig9+AQodhF5sRXuKkJ=RgFDugLr3XX_dz_F-p=TwTg@mail.gmail.com
-
Robert Haas authored
The comment is no longer accurate, and hasn't been entirely accurate since Hot Standby was introduced. The original idea here was that StartupCLOG() wouldn't be called until the end of recovery and therefore this value would be uninitialized when this code is reached, but Hot Standby made that true only when hot_standby=off, and commit 1f113abd means that this value is now always initialized before replay even starts. The original purpose of this code was to bypass the sanity check in SimpleLruTruncate(), which will no longer occur: now, if something is wrong, that sanity check might trip during recovery. That's probably a good thing, because in the current code base latest_page_number should always be initialized and therefore we expect that the sanity check should pass. If it doesn't, something has gone wrong, and complaining about it is appropriate. Patch by me, reviewed by Heikki Linnakangas. Discussion: http://postgr.es/m/CA+TgmoZYig9+AQodhF5sRXuKkJ=RgFDugLr3XX_dz_F-p=TwTg@mail.gmail.com
-
Tom Lane authored
Per a user question, spell out that UNNEST() returns array elements in storage order; also provide an example to clarify the behavior for multi-dimensional arrays. While here, also clarify the SELECT reference page's description of WITH ORDINALITY. These details were already given in 7.2.1.4, but a reference page should not omit details. Back-patch to v13; there's not room in the table in older versions. Discussion: https://postgr.es/m/FF1FB31F-0507-4F18-9559-2DE6E07E3B43@gmail.com
-
Robert Haas authored
Previously, the hot_standby=off code path did this at end of recovery, while the hot_standby=on code path did it at the beginning of recovery. It's better to do this in only one place because (a) it's simpler, (b) StartupCLOG() is trivial so trying to postpone the work isn't useful, and (c) this will make it possible to simplify some other logic. Patch by me, reviewed by Heikki Linnakangas. Discussion: http://postgr.es/m/CA+TgmoZYig9+AQodhF5sRXuKkJ=RgFDugLr3XX_dz_F-p=TwTg@mail.gmail.com
-
Peter Geoghegan authored
Avoid calling heap_index_delete_tuples() with an empty deltids array to avoid an assertion failure. This issue was arguably an oversight in commit b5f58cf2, though the failing assert itself was added by my recent commit d168b666. No backpatch, though, since the oversight is harmless in the back branches. Author: Peter Geoghegan <pg@bowt.ie> Reported-By: Jaime Casanova <jcasanov@systemguards.com.ec> Discussion: https://postgr.es/m/CAJKUy5jscES84n3puE=sYngyF+zpb4wv8UMtuLnLPv5z=6yyNw@mail.gmail.com
-
Michael Paquier authored
The page about privilege rights mentioned that TRUNCATE could be applied to views or even other relation types. This is confusing as this command can be used only on tables and on partitioned tables. Oversight in afc4a78a. Reported-by: Harisai Hari Reviewed-by: Laurenz Albe Discussion: https://postgr.es/m/161157636877.14625.15340884663716426087@wrigleys.postgresql.org Backpatch-through: 12
-
Michael Paquier authored
Two code paths of tablecmds.c (for relations with storage and without storage) use the same logic to check if the move of a relation to a new tablespace is allowed or not and to update pg_class.reltablespace and pg_class.relfilenode. A potential TABLESPACE clause for REINDEX, CLUSTER and VACUUM FULL needs similar checks to make sure that nothing is moved around in illegal ways (no mapped relations, shared relations only in pg_global, no move of temp tables owned by other backends). This reorganizes the existing code of ALTER TABLE so as all this logic is controlled by two new routines that can be reused for the other commands able to move relations across tablespaces, limiting the number of code paths in need of the same protections. This also removes some code that was duplicated for tables with and without storage for ALTER TABLE. Author: Alexey Kondratov, Michael Paquier Discussion: https://postgr.es/m/YA+9mAMWYLXJMVPL@paquier.xyz
-
- 26 Jan, 2021 8 commits
-
-
Tom Lane authored
SPI_execute_with_receiver and SPI_cursor_parse_open_with_paramlist are new in v14 (cf. commit 2f48ede0). Before they can get out the door, let's change their APIs to follow the practice recently established by SPI_prepare_extended etc: shove all optional arguments into a struct that callers are supposed to pre-zero. The hope is to allow future addition of more options without either API breakage or a continuing proliferation of new SPI entry points. With that in mind, choose slightly more generic names for them: SPI_execute_extended and SPI_cursor_parse_open respectively. Discussion: https://postgr.es/m/CAFj8pRCLPdDAETvR7Po7gC5y_ibkn_-bOzbeJb39WHms01194Q@mail.gmail.com
-
Tom Lane authored
For obscure reasons, some buildfarm members are now generating complaints about plpgsql_call_handler's "retval" variable possibly being used uninitialized. It seems no less safe than it was before that commit, but these complaints are (mostly?) new. I trust that initializing the variable where it's declared will be enough to shut that up. I also notice that some compilers are warning about setjmp clobber of the same variable, which is maybe a bit more defensible. Mark it volatile to silence that. Also, rearrange the logic to give procedure_resowner a single point of initialization, in hopes of silencing some setjmp-clobber warnings about that. (Marking it volatile would serve too, but its sibling variables are depending on single assignment, so let's stick with that method.) Discussion: https://postgr.es/m/E1l4F1z-0000cN-Lx@gemulon.postgresql.org
-
Tom Lane authored
The loops to identify word boundaries could access past the end of the input string. Likely that would never result in an actual crash, but it makes valgrind unhappy. The logic to try different numbers of words didn't work when the input has two words but we only have a match to the first, eg "\h with select". (We must "continue" the pass loop, not "break".) The logic to compute nl_count was bizarrely managed, and in at least two code paths could end up calling PageOutput with nl_count = 0, resulting in failing to paginate output that should have been fed to the pager. Also, in v12 and up, the nl_count calculation hadn't been updated to account for the addition of a URL. The PQExpBuffer holding the command syntax details wasn't freed, resulting in a session-lifespan memory leak. While here, improve some comments, choose a more descriptive name for a variable, fix inconsistent datatype choice for another variable. Per bug #16837 from Alexander Lakhin. This code is very old, so back-patch to all supported branches. Kyotaro Horiguchi and Tom Lane Discussion: https://postgr.es/m/16837-479bcd56040c71b3@postgresql.org
-
Michael Paquier authored
The leak is minor, so no backpatch is done. Oversight in 21734d2f. Reported-by: Tom Lane
-
Fujii Masao authored
The roles created by regression test should have names starting with "regress_", and the test introduced in commit 411ae649 did not do that. Per buildfarm member longfin. Discussion: https://postgr.es/m/73fc5ae4-3c54-1262-4533-f8c547de2e60@oss.nttdata.com
-
Fujii Masao authored
The regression test added in commit 411ae649 caused buildfarm failures. The cause of them was that the order of warning messages output in the test was not stable. To fix this, this commit sets client_min_messages to ERROR temporarily when performing the test generating those warnings. Per buildfarm failures. Discussion: https://postgr.es/m/2147113.1611644754@sss.pgh.pa.us
-
Fujii Masao authored
This commit introduces two new functions postgres_fdw_disconnect() and postgres_fdw_disconnect_all(). The former function discards the cached connections to the specified foreign server. The latter discards all the cached connections. If the connection is used in the current transaction, it's not closed and a warning message is emitted. For example, these functions are useful when users want to explicitly close the foreign server connections that are no longer necessary and then to prevent them from eating up the foreign servers connections capacity. Author: Bharath Rupireddy, tweaked a bit by Fujii Masao Reviewed-by: Alexey Kondratov, Zhijie Hou, Zhihong Yu, Fujii Masao Discussion: https://postgr.es/m/CALj2ACVvrp5=AVp2PupEm+nAC8S4buqR3fJMmaCoc7ftT0aD2A@mail.gmail.com
-
Tom Lane authored
This patch essentially is cleaning up technical debt left behind by the original implementation of plpgsql procedures, particularly commit d92bc83c. That patch (or more precisely, follow-on patches fixing its worst bugs) forced us to re-plan CALL and DO statements each time through, if we're in a non-atomic context. That wasn't for any fundamental reason, but just because use of a saved plan requires having a ResourceOwner to hold a reference count for the plan, and we had no suitable resowner at hand, nor would the available APIs support using one if we did. While it's not that expensive to create a "plan" for CALL/DO, the cycles do add up in repeated executions. This patch therefore makes the following API changes: * GetCachedPlan/ReleaseCachedPlan are modified to let the caller specify which resowner to use to pin the plan, rather than forcing use of CurrentResourceOwner. * spi.c gains a "SPI_execute_plan_extended" entry point that lets callers say which resowner to use to pin the plan. This borrows the idea of an options struct from the recently added SPI_prepare_extended, hopefully allowing future options to be added without more API breaks. This supersedes SPI_execute_plan_with_paramlist (which I've marked deprecated) as well as SPI_execute_plan_with_receiver (which is new in v14, so I just took it out altogether). * I also took the opportunity to remove the crude hack of letting plpgsql reach into SPI private data structures to mark SPI plans as "no_snapshot". It's better to treat that as an option of SPI_prepare_extended. Now, when running a non-atomic procedure or DO block that contains any CALL or DO commands, plpgsql creates a ResourceOwner that will be used to pin the plans of the CALL/DO commands. (In an atomic context, we just use CurrentResourceOwner, as before.) Having done this, we can just save CALL/DO plans normally, whether or not they are used across transaction boundaries. This seems to be good for something like 2X speedup of a CALL of a trivial procedure with a few simple argument expressions. By restricting the creation of an extra ResourceOwner like this, there's essentially zero penalty in cases that can't benefit. Pavel Stehule, with some further hacking by me Discussion: https://postgr.es/m/CAFj8pRCLPdDAETvR7Po7gC5y_ibkn_-bOzbeJb39WHms01194Q@mail.gmail.com
-
- 25 Jan, 2021 8 commits
-
-
Andres Freund authored
Reported-by: Heikki Linnakangas <hlinnaka@iki.fi> Discussion: https://postgr.es/m/c94be044-818f-15e3-1ad3-7a7ae2dfed0a@iki.fi
-
Tom Lane authored
Embarrassing oversight in this test script, which fortunately is not run by default. Report and patch by Jacob Champion. Discussion: https://postgr.es/m/1fcb175bafef6560f47a8c31229fa7c938486b8d.camel@vmware.com
-
Tom Lane authored
I chanced to notice that this dumped core due to a faulty Assert. To add insult to injury, the output has been misformatted since v11. Obviously we need some regression testing here. Discussion: https://postgr.es/m/d1cc628c-3953-4209-957b-29427acc38c8@www.fastmail.com
-
Robert Haas authored
Up until now, we've held this lock when performing a checkpoint or restartpoint, but commit 076a055a back in 2004 and commit 7e48b77b from 2009, taken together, have removed all need for this. In the present code, there's only ever one process entitled to attempt a checkpoint: either the checkpointer, during normal operation, or the postmaster, during single-user operation. So, we don't need the lock. One possible concern in making this change is that it means that a substantial amount of code where HOLD_INTERRUPTS() was previously in effect due to the preceding LWLockAcquire() will now be running without that. This could mean that ProcessInterrupts() gets called in places from which it didn't before. However, this seems unlikely to do very much, because the checkpointer doesn't have any signal mapped to die(), so it's not clear how, for example, ProcDiePending = true could happen in the first place. Similarly with ClientConnectionLost and recovery conflicts. Also, if there are any such problems, we might want to fix them rather than reverting this, since running lots of code with interrupt handling suspended is generally bad. Patch by me, per an inquiry by Amul Sul. Review by Tom Lane and Michael Paquier. Discussion: http://postgr.es/m/CAAJ_b97XnBBfYeSREDJorFsyoD1sHgqnNuCi=02mNQBUMnA=FA@mail.gmail.com
-
Tom Lane authored
Add a "references" link pointing to pg_type, as we have for other arrays of type OIDs. Wordsmith the explanation a bit. Joel Jacobson, additional editing by me Discussion: https://postgr.es/m/d1cc628c-3953-4209-957b-29427acc38c8@www.fastmail.com
-
David Rowley authored
Both heapgettup() and heapgettup_pagemode() incorrectly set the first page to scan in a backward scan in which the number of pages to scan was specified by heap_setscanlimits(). The code incorrectly started the scan at the end of the relation when startBlk was 0, or otherwise at startBlk - 1, neither of which is correct when only scanning a subset of pages. The fix here checks if heap_setscanlimits() has changed the number of pages to scan and if so we set the first page to scan as the final page in the specified range during backward scans. Proper adjustment of this code was forgotten when heap_setscanlimits() was added in 7516f525 back in 9.5. However, practice, nowhere in core code performs backward scans after having used heap_setscanlimits(), yet, it is possible an extension uses the heap functions in this way, hence backpatch. An upcoming patch does use heap_setscanlimits() with backward scans, so this must be fixed before that can go in. Author: David Rowley Discussion: https://postgr.es/m/CAApHDvpGc9h0_oVD2CtgBcxCS1N-qDYZSeBRnUh+0CWJA9cMaA@mail.gmail.com Backpatch-through: 9.5, all supported versions
-
Amit Kapila authored
Commit 69bd6067 fixed the initialization of streamed transactions for RelationSyncEntry. It forgot to initialize the publication actions while invalidating the RelationSyncEntry due to which even though the relation is dropped from a particular publication we still publish its changes. Fix it by initializing pubactions when entry got invalidated. Author: Japin Li and Bharath Rupireddy Reviewed-by: Amit Kapila Discussion: https://postgr.es/m/CALj2ACV+0UFpcZs5czYgBpujM9p0Hg1qdOZai_43OU7bqHU_xw@mail.gmail.com
-
- 24 Jan, 2021 6 commits
-
-
Tom Lane authored
DST law changes in Russia (Volgograd zone) and South Sudan. Historical corrections for Australia, Bahamas, Belize, Bermuda, Ghana, Israel, Kenya, Nigeria, Palestine, Seychelles, and Vanuatu. Notably, the Australia/Currie zone has been corrected to the point where it is identical to Australia/Hobart.
-
Tom Lane authored
This module formerly had zero test coverage. Discussion: https://postgr.es/m/1445881.1611441692@sss.pgh.pa.us
-
Magnus Hagander authored
These are mostly obsoleted by the switch to git, and it's easier to remove them than to update the incorrect documentation. Discussion: https://postgr.es/m/CABUevEwmASMn4WRJ6RagBx43sj10ctfMHcMA_-7KA3pDYmwpJw@mail.gmail.com
-
Tom Lane authored
I came to fix the overwidth-PDF-page warnings seen in the buildfarm, but stayed long enough to copy-edit some nearby text.
-
Tomas Vondra authored
This adds code omitted from commit 7db0cd21 by accident, which had two consequences. Firstly, only rows inserted by heap_multi_insert were frozen as expected when running COPY FREEZE, while heap_insert left rows unfrozen. That however includes rows in TOAST tables, so a lot of data might have been left unfrozen. Secondly, page might have been left partially empty after relcache invalidation. This addresses both of those issues. Discussion: https://postgr.es/m/CABOikdN-ptGv0mZntrK2Q8OtfUuAjqaYMGmkdU1dCKFtUxVLrg@mail.gmail.com
- 23 Jan, 2021 4 commits
-
-
Tom Lane authored
I should have updated this in commits 52a10224 and follow-ons, but I missed it because it's not run by default, and none of the buildfarm runs it either. Maybe we should try to improve that situation. Discussion: https://postgr.es/m/CAH2-Wz=j9SRW=s5BV4-3k+=tr4N3A03in+gTuVA09vNF+-iHjA@mail.gmail.com
-
Michael Paquier authored
With this commit, SHA1 goes through the implementation provided by OpenSSL via EVP when building the backend with it, and uses as fallback implementation KAME which was located in pgcrypto and already shaped for an integration with a set of init, update and final routines. Structures and routines have been renamed to make things consistent with the fallback implementations of MD5 and SHA2. uuid-ossp has used for ages a shortcut with pgcrypto to fetch a copy of SHA1 if needed. This was built depending on the build options within ./configure, so this cleans up some code and removes the build dependency between pgcrypto and uuid-ossp. Note that this will help with the refactoring of HMAC, as pgcrypto offers the option to use MD5, SHA1 or SHA2, so only the second option was missing to make that possible. Author: Michael Paquier Reviewed-by: Heikki Linnakangas Discussion: https://postgr.es/m/X9HXKTgrvJvYO7Oh@paquier.xyz
-
Tom Lane authored
opt_distinct_clause is only used in PLpgSQL_Expr, which ecpg ignores, so it needs to ignore opt_distinct_clause too. My oversight in 7cd9765f; reported by Bruce Momjian. Discussion: https://postgr.es/m/E1l33wr-0005sJ-9n@gemulon.postgresql.org
- 22 Jan, 2021 3 commits
-
-
Tom Lane authored
In light of recent discussions, we should instruct people to install Apple's command line tools; installing Xcode is secondary. Also, fix sample command for finding out the default sysroot, as we now know that the command originally recommended can give a result that doesn't match your OS version. Also document the workaround to use if you really don't want configure to select a sysroot at all. Discussion: https://postgr.es/m/20210119111625.20435-1-james.hilliard1@gmail.com
-
Tom Lane authored
libpq's error messages for connection failures pretty well stand on their own, especially since commits 52a10224/27a48e5a. Prefixing them with 'could not connect to database "foo"' or the like is just redundant, and perhaps even misleading if the specific database name isn't relevant to the failure. (When it is, we trust that the backend's error message will include the DB name.) Indeed, psql hasn't used any such prefix in a long time. So, make all our other programs and documentation examples agree with psql's practice. Discussion: https://postgr.es/m/1094524.1611266589@sss.pgh.pa.us
-
Tom Lane authored
I'd omitted this from the grammar in commit c9d52984, figuring that it wasn't worth supporting. However we already have one complaint, so it seems that judgment was wrong. It doesn't require a huge amount of code, so add it back. (I'm still drawing the line at UNION/INTERSECT/EXCEPT though: those'd require an unreasonable amount of grammar refactoring, and the single-result-row restriction makes them near useless anyway.) Also rethink the documentation: this behavior is a property of all pl/pgsql expressions, not just assignments. Discussion: https://postgr.es/m/20210122134106.e94c5cd7@mail.verfriemelt.org
-