- 08 Jan, 2021 4 commits
-
-
Tom Lane authored
Commit c9d52984 resulted in having a couple more places where the error context stack for a failure varies depending on debug_invalidate_system_caches_always (nee CLOBBER_CACHE_ALWAYS). This is not very surprising, since we have to re-parse cached plans if the plan cache is clobbered. Stabilize the expected test output by hiding the context stack in these places, as we've done elsewhere in this test script. (Another idea worth considering, now that we have debug_invalidate_system_caches_always, is to force it to zero for these test cases. That seems like it'd risk reducing the coverage of cache-clobber testing, which might or might not be worth being able to verify that we get the expected error output in normal cases. For the moment I just stuck with the existing technique.) In passing, update comments that referred to CLOBBER_CACHE_ALWAYS. Per buildfarm member hyrax.
-
Tom Lane authored
brenext(), when parsing a '*' quantifier, forgot to return any "value" for the token; per the equivalent case in next(), it should return value 1 to indicate that greedy rather than non-greedy behavior is wanted. The result is that the compiled regexp could behave like 'x*?' rather than the intended 'x*', if we were unlucky enough to have a zero in v->nextvalue at this point. That seems to happen with some reliability if we have '.*' at the beginning of a BRE-mode regexp, although that depends on the initial contents of a stack-allocated struct, so it's not guaranteed to fail. Found by Alexander Lakhin using valgrind testing. This bug seems to be aboriginal in Spencer's code, so back-patch all the way. Discussion: https://postgr.es/m/16814-6c5e3edd2bdf0d50@postgresql.org
-
Michael Paquier authored
This commit addresses two issues: - In pgcrypto, MD5 computation called pg_cryptohash_{init,update,final} without checking for the result status. - Simplify pg_checksum_raw_context to use only one variable for all the SHA2 options available in checksum manifests. Reported-by: Heikki Linnakangas Discussion: https://postgr.es/m/f62f26bb-47a5-8411-46e5-4350823e06a5@iki.fi
-
Tom Lane authored
We found last February that the error-case tests added by commit 008cf040 failed on OpenBSD, because that platform doesn't really check locale names. At the time it seemed that that was only an issue for LC_CTYPE, but testing on a more recent version of OpenBSD shows that it's now equally lax about LC_COLLATE. Rather than dropping the LC_COLLATE test too, put back LC_CTYPE (reverting c4b0edb0), and adjust these tests to accept the different error message that we get if setlocale() doesn't reject a bogus locale name. The point of these tests is not really what the backend does with the locale name, but to show that createdb quotes funny locale names safely; so we're not losing test reliability this way. Back-patch as appropriate. Discussion: https://postgr.es/m/231373.1610058324@sss.pgh.pa.us
-
- 07 Jan, 2021 6 commits
-
-
Tom Lane authored
On reflection, the order of operations in PostgresMain() is wrong. These timeouts ought to be shut down before, not after, we do the post-command-read CHECK_FOR_INTERRUPTS, to guarantee that any timeout error will be detected there rather than at some ill-defined later point (possibly after having wasted a lot of work). This is really an error in the original idle_in_transaction_timeout patch, so back-patch to 9.6 where that was introduced.
-
Tomas Vondra authored
Author: Justin Pryzby Discussion: https://postgr.es/m/CAFp7Qwr6_FmRM6pCO0x_a0mymOfX_Gg+FEKet4XaTGSW=LitKQ@mail.gmail.com
-
Fujii Masao authored
This commit adds GUC log_recovery_conflict_waits that controls whether a log message is produced when the startup process is waiting longer than deadlock_timeout for recovery conflicts. This is useful in determining if recovery conflicts prevent the recovery from applying WAL. Note that currently a log message is produced only when recovery conflict has not been resolved yet even after deadlock_timeout passes, i.e., only when the startup process is still waiting for recovery conflict even after deadlock_timeout. Author: Bertrand Drouvot, Masahiko Sawada Reviewed-by: Alvaro Herrera, Kyotaro Horiguchi, Fujii Masao Discussion: https://postgr.es/m/9a60178c-a853-1440-2cdc-c3af916cff59@amazon.com
-
Tom Lane authored
On re-reading I realized that I'd missed one race condition in the new timeout code. It's safe, but add a comment explaining it. Discussion: https://postgr.es/m/CA+hUKG+o6pbuHBJSGnud=TadsuXySWA7CCcPgCt2QE9F6_4iHQ@mail.gmail.com
-
Michael Paquier authored
The allocation of the cryptohash context data when building with OpenSSL was happening in the memory context of the caller of pg_cryptohash_create(), which could lead to issues with resowner cleanup if cascading resources are cleaned up on an error. Like other facilities using resowners, move the base allocation to TopMemoryContext to ensure a correct cleanup on failure. The resulting code gets simpler with this commit as the context data is now hold by a unique opaque pointer, so as there is only one single allocation done in TopMemoryContext. After discussion, also change the cryptohash subroutines to return an error if the caller provides NULL for the context data to ease error detection on OOM. Author: Heikki Linnakangas Discussion: https://postgr.es/m/X9xbuEoiU3dlImfa@paquier.xyz
-
- 06 Jan, 2021 10 commits
-
-
Tom Lane authored
This GUC variable works much like idle_in_transaction_session_timeout, in that it kills sessions that have waited too long for a new client query. But it applies when we're not in a transaction, rather than when we are. Li Japin, reviewed by David Johnston and Hayato Kuroda, some fixes by me Discussion: https://postgr.es/m/763A0689-F189-459E-946F-F0EC4458980B@hotmail.com
-
Tom Lane authored
A very common usage pattern is that we set a timeout that we don't expect to reach, cancel it after a little bit, and later repeat. With the original implementation of timeout.c, this results in one setitimer() call per timeout set or cancel. We can do a lot better by being lazy about changing the timeout interrupt request, namely: (1) never cancel the outstanding interrupt, even when we have no active timeout events; (2) if we need to set an interrupt, but there already is one pending at or before the required time, leave it alone. When the interrupt happens, the signal handler will reschedule it at whatever time is then needed. For example, with a one-second setting for statement_timeout, this method results in having to interact with the kernel only a little more than once a second, no matter how many statements we execute in between. The mainline code might never call setitimer() at all after the first time, while each time the signal handler fires, it sees that the then-pending request is most of a second away, and that's when it sets the next interrupt request for. Each mainline timeout-set request after that will observe that the time it wants is past the pending interrupt request time, and do nothing. This also works pretty well for cases where a few different timeout lengths are in use, as long as none of them are very short. But that describes our usage well. Idea and original patch by Thomas Munro; I fixed a race condition and improved the comments. Discussion: https://postgr.es/m/CA+hUKG+o6pbuHBJSGnud=TadsuXySWA7CCcPgCt2QE9F6_4iHQ@mail.gmail.com
-
Tomas Vondra authored
This commit introduces a view pg_stat_progress_copy, reporting progress of COPY commands. This allows rough estimates how far a running COPY progressed, with the caveat that the total number of bytes may not be available in some cases (e.g. when the input comes from the client). Author: Josef Šimánek Reviewed-by: Fujii Masao, Bharath Rupireddy, Vignesh C, Matthias van de Meent Discussion: https://postgr.es/m/CAFp7QwqMGEi4OyyaLEK9DR0+E+oK3UtA4bEjDVCa4bNkwUY2PQ@mail.gmail.com Discussion: https://postgr.es/m/CAFp7Qwr6_FmRM6pCO0x_a0mymOfX_Gg+FEKet4XaTGSW=LitKQ@mail.gmail.com
-
Tom Lane authored
This module provides a function test_regex() that is functionally rather like regexp_matches(), but with additional debugging-oriented options and additional output. The debug options are somewhat obscure; they are chosen to match the API of the test harness that Henry Spencer wrote way-back-when for use in Tcl. With this, we can import all the test cases that Spencer wrote originally, even for regex functionality that we don't currently expose in Postgres. This seems necessary because we can no longer rely on Tcl to act as upstream and verify any fixes or improvements that we make. In addition to Spencer's tests, I added a few for lookbehind constraints (which we added in 2015, and Tcl still hasn't absorbed) that are modeled on his tests for lookahead constraints. After looking at code coverage reports, I also threw in a couple of tests to more fully exercise our "high colormap" logic. According to my testing, this brings the check-world coverage for src/backend/regex/ from 71.1% to 86.7% of lines. (coverage.postgresql.org shows a slightly different number, which I think is because it measures a non-assert build.) Discussion: https://postgr.es/m/2873268.1609732164@sss.pgh.pa.us
-
Peter Eisentraut authored
Forced cache invalidation (CLOBBER_CACHE_ALWAYS) has been impractical to use for testing in PostgreSQL because it's so slow and because it's toggled on/off only at build time. It is helpful when hunting bugs in any code that uses the sycache/relcache because causes cache invalidations to be injected whenever it would be possible for an invalidation to occur, whether or not one was really pending. Address this by providing run-time control over cache clobber behaviour using the new debug_invalidate_system_caches_always GUC. Support is not compiled in at all unless assertions are enabled or CLOBBER_CACHE_ENABLED is explicitly defined at compile time. It defaults to 0 if compiled in, so it has negligible effect on assert build performance by default. When support is compiled in, test code can now set debug_invalidate_system_caches_always=1 locally to a backend to test specific queries, functions, extensions, etc. Or tests can toggle it globally for a specific test case while retaining normal performance during test setup and teardown. For backwards compatibility with existing test harnesses and scripts, debug_invalidate_system_caches_always defaults to 1 if CLOBBER_CACHE_ALWAYS is defined, and to 3 if CLOBBER_CACHE_RECURSIVE is defined. CLOBBER_CACHE_ENABLED is now visible in pg_config_manual.h, as is the related RECOVER_RELATION_BUILD_MEMORY setting for the relcache. Author: Craig Ringer <craig.ringer@2ndquadrant.com> Discussion: https://www.postgresql.org/message-id/flat/CAMsr+YF=+ctXBZj3ywmvKNUjWpxmuTuUKuv-rgbHGX5i5pLstQ@mail.gmail.com
-
Fujii Masao authored
The deadlocks that the recovery conflict on lock is involved in can happen between hot-standby backends and the startup process. If a backend takes an access exclusive lock on the table and which finally triggers the deadlock, that deadlock can be detected as expected. On the other hand, previously, if the startup process took an access exclusive lock and which finally triggered the deadlock, that deadlock could not be detected and could remain even after deadlock_timeout passed. This is a bug. The cause of this bug was that the code for handling the recovery conflict on lock didn't take care of deadlock case at all. It assumed that deadlocks involving the startup process and backends were able to be detected by the deadlock detector invoked within backends. But this assumption was incorrect. The startup process also should have invoked the deadlock detector if necessary. To fix this bug, this commit makes the startup process invoke the deadlock detector if deadlock_timeout is reached while handling the recovery conflict on lock. Specifically, in that case, the startup process requests all the backends holding the conflicting locks to check themselves for deadlocks. Back-patch to v9.6. v9.5 has also this bug, but per discussion we decided not to back-patch the fix to v9.5. Because v9.5 doesn't have some infrastructure codes (e.g., 37c54863) that this bug fix patch depends on. We can apply those codes for the back-patch, but since the next minor version release is the final one for v9.5, it's risky to do that. If we unexpectedly introduce new bug to v9.5 by the back-patch, there is no chance to fix that. We determined that the back-patch to v9.5 would give more risk than gain. Author: Fujii Masao Reviewed-by: Bertrand Drouvot, Masahiko Sawada, Kyotaro Horiguchi Discussion: https://postgr.es/m/4041d6b6-cf24-a120-36fa-1294220f8243@oss.nttdata.com
-
Amit Kapila authored
Per report by Ajin Cherian in email: https://postgr.es/m/CAFPTHDYnRKDvzgDxoMn_CKqXA-D0MtrbyJvfvjBsO4G=UHDXkg@mail.gmail.com
-
Fujii Masao authored
The default value of recovery_target_timeline was changed in v12, but the description about the default behavior of that was not updated. Back-patch to v12 where the default behavior of recovery_target_timeline was changed. Author: Benoit Lobréau Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/CAPE8EZ7c3aruEmM24GYkj8y8WmHKD1m9TtPtgCF0nQ3zw4LCkQ@mail.gmail.com
-
Michael Paquier authored
This was previously part of the section dedicated to less common options, but it is an option commonly used these days. Author: Michael Banck Reviewed-by: Stephen Frost, Michael Paquier Discussion: https://postgr.es/m/d7938aca4d4ea8e8c72c33bd75efe9f8218fe390.camel@credativ.de
-
Tom Lane authored
I momentarily forgot that the "owner" column wouldn't be stable in the buildfarm. Oh well, these tests weren't very valuable anyway. Discussion: https://postgr.es/m/20201130165436.GX24052@telsasoft.com
-
- 05 Jan, 2021 12 commits
-
-
Tom Lane authored
Formerly, TOAST objects were unconditionally suppressed, but since \d is able to print them it's not very clear why these variants should not. Instead, use the same rules as for system catalogs: they can be seen if you write the 'S' modifier or a table name pattern. (In practice, since hardly anybody would keep pg_toast in their search_path, it's really down to whether you use a pattern that can match pg_toast.*.) No docs change seems necessary because the docs already say that this happens for "system objects"; we're just classifying TOAST tables as being that. Justin Pryzby, reviewed by Laurenz Albe Discussion: https://postgr.es/m/20201130165436.GX24052@telsasoft.com
-
Tom Lane authored
Aside from being queriable via SHOW, this value is sent to the client immediately at session startup, and again later on if the server gets promoted to primary during the session. The immediate report will be used in an upcoming patch to avoid an extra round trip when trying to connect to a primary server. Haribabu Kommi, Greg Nancarrow, Tom Lane; reviewed at various times by Laurenz Albe, Takayuki Tsunakawa, Peter Smith. Discussion: https://postgr.es/m/CAF3+xM+8-ztOkaV9gHiJ3wfgENTq97QcjXQt+rbFQ6F7oNzt9A@mail.gmail.com
-
Bruce Momjian authored
Reported-by: "Tang, Haiying" Discussion: https://postgr.es/m/bbbccf7a3c2d436e85d45869d612fd6b@G08CNEXMBPEKD05.g08.fujitsu.local Author: "Tang, Haiying" Backpatch-through: 9.5
-
Dean Rasheed authored
Commit bc43b7c2 used fabs() directly on an int variable, which apparently requires an explicit cast on some platforms. Per buildfarm.
-
Dean Rasheed authored
In power_var_int(), the computation of the number of significant digits to use in the computation used log(Abs(exp)), which isn't safe because Abs(exp) returns INT_MIN when exp is INT_MIN. Use fabs() instead of Abs(), so that the exponent is cast to a double before the absolute value is taken. Back-patch to 9.6, where this was introduced (by 7d9a4737). Discussion: https://postgr.es/m/CAEZATCVd6pMkz=BrZEgBKyqqJrt2xghr=fNc8+Z=5xC6cgWrWA@mail.gmail.com
-
Amit Kapila authored
Commit a271a1b5 added the capability to allow decoding at prepare time. This adds an isolation testcase to test that decoding happens at commit time when the consistent snapshot state is reached after prepare but before commit prepared. Author: Ajin Cherian Reviewed-by: Amit Kapila Discussion: https://postgr.es/m/02DA5F5E-CECE-4D9C-8B4B-418077E2C010@postgrespro.ru https://postgr.es/m/CAMGcDxeqEpWj3fTXwqhSwBdXd2RS9jzwWscO-XbeCfso6ts3+Q@mail.gmail.com
-
Peter Eisentraut authored
Author: Craig Ringer <craig.ringer@2ndquadrant.com> Discussion: https://www.postgresql.org/message-id/flat/CAMsr+YF=+ctXBZj3ywmvKNUjWpxmuTuUKuv-rgbHGX5i5pLstQ@mail.gmail.com
-
Peter Geoghegan authored
Bring heap and hash rmgr desc output in line with nbtree and GiST desc output by using the name latestRemovedXid for all fields that output the contents of the latestRemovedXid field from the WAL record's C struct (stop using local variants). This seems like a clear improvement because latestRemovedXid is a symbol name that already appears across many different source files, and so is probably much more recognizable. Discussion: https://postgr.es/m/CAH2-Wzkt_Rs4VqPSCk87nyjPAAEmWL8STU9zgET_83EF5YfrLw@mail.gmail.com
-
Amit Kapila authored
Author: Peter Smith Discussion: https://postgr.es/m/CAHut+PsReyuvww_Fn1NN_Vsv0wBP1bnzuhzRFr_2=y1nNZrG7w@mail.gmail.com
-
Amit Kapila authored
Author: Zhijie Hou Reviewed-by: Sawada Masahiko Discussion: https://postgr.es/m/ba88bb58aaf14284abca16aec04bf279@G08CNEXMBPEKD05.g08.fujitsu.local
-
Thomas Munro authored
Instead describe the action that the list effects, or just use "list" where the meaning is obvious from context. Author: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> Discussion: https://postgr.es/m/20200615182235.x7lch5n6kcjq4aue%40alap3.anarazel.de
-
Thomas Munro authored
Author: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> Discussion: https://postgr.es/m/20200615182235.x7lch5n6kcjq4aue%40alap3.anarazel.de
-
- 04 Jan, 2021 8 commits
-
-
Thomas Munro authored
We agreed to remove this terminology and use something more descriptive. Discussion: https://postgr.es/m/20200615182235.x7lch5n6kcjq4aue%40alap3.anarazel.de
-
Tom Lane authored
If the substring start index and length overflow when added together, substring() misbehaved, either throwing a bogus "negative substring length" error on a case that should succeed, or failing to complain that a negative length is negative (and instead returning the whole string, in most cases). Unsurprisingly, the text, bytea, and bit variants of the function all had this issue. Rearrange the logic to ensure that negative lengths are always rejected, and add an overflow check to handle the other case. Also install similar guards into detoast_attr_slice() (nee heap_tuple_untoast_attr_slice()), since it's far from clear that no other code paths leading to that function could pass it values that would overflow. Patch by myself and Pavel Stehule, per bug #16804 from Rafi Shamim. Back-patch to v11. While these bugs are old, the common/int.h infrastructure for overflow-detecting arithmetic didn't exist before commit 4d6ad312, and it doesn't seem like these misbehaviors are bad enough to justify developing a standalone fix for the older branches. Discussion: https://postgr.es/m/16804-f4eeeb6c11ba71d4@postgresql.org
-
Thomas Munro authored
Cleanup for commit dee663f7. Reported-by: Tomas Vondra <tomas.vondra@enterprisedb.com> Discussion: https://postgr.es/m/CA+hUKGLJ=84YT+NvhkEEDAuUtVHMfQ9i-N7k_o50JmQ6Rpj_OQ@mail.gmail.com
-
Tom Lane authored
Performance issues with the preceding patch to re-implement array element assignment within pl/pgsql led me to realize that the read/write parameter mechanism is misdesigned. Instead of requiring the assignment source expression to be such that *all* its references to the target variable could be passed as R/W, we really want to identify *one* reference to the target variable to be passed as R/W, allowing any other ones to be passed read/only as they would be by default. As long as the R/W reference is a direct argument to the top-level (hence last to be executed) function in the expression, there is no harm in R/O references being passed to other lower parts of the expression. Nor is there any use-case for more than one argument of the top-level function being R/W. Hence, rewrite that logic to identify one single Param that references the target variable, and make only that Param pass a read/write reference, not any other Params referencing the target variable. Discussion: https://postgr.es/m/4165684.1607707277@sss.pgh.pa.us
-
Tom Lane authored
In the wake of the previous commit, we don't really need this anymore, since array assignment is primarily handled by the core code. The only way that that code could still be reached is that a GET DIAGNOSTICS target variable could be an array element. But that doesn't seem like a particularly essential feature. I'd added it in commit 55caaaeb, but just because it was easy not because anyone had actually asked for it. Hence, revert that patch and then remove the now-unreachable stuff. (If we really had to, we could probably reimplement GET DIAGNOSTICS using the new assignment machinery; but the cost/benefit ratio looks very poor, and it'd likely be a bit slower.) Note that PLPGSQL_DTYPE_RECFIELD remains. It's possible that we could get rid of that too, but maintaining the existing behaviors for RECORD-type variables seems like it might be difficult. Since there's not any functional limitation in those code paths as there was in the ARRAYELEM code, I've not pursued the idea. Discussion: https://postgr.es/m/4165684.1607707277@sss.pgh.pa.us
-
Tom Lane authored
Invent new RawParseModes that allow the core grammar to handle pl/pgsql expressions and assignments directly, and thereby get rid of a lot of hackery in pl/pgsql's parser. This moves a good deal of knowledge about pl/pgsql into the core code: notably, we have to invent a CoercionContext that matches pl/pgsql's (rather dubious) historical behavior for assignment coercions. That's getting away from the original idea of pl/pgsql as an arm's-length extension of the core, but really we crossed that bridge a long time ago. The main advantage of doing this is that we can now use the core parser to generate FieldStore and/or SubscriptingRef nodes to handle assignments to pl/pgsql variables that are records or arrays. That fixes a number of cases that had never been implemented in pl/pgsql assignment, such as nested records and array slicing, and it allows pl/pgsql assignment to support the datatype-specific subscripting behaviors introduced in commit c7aba7c1. There are cosmetic benefits too: when a syntax error occurs in a pl/pgsql expression, the error report no longer includes the confusing "SELECT" keyword that used to get prefixed to the expression text. Also, there seem to be some small speed gains. Discussion: https://postgr.es/m/4165684.1607707277@sss.pgh.pa.us
-
Tom Lane authored
This patch essentially allows gram.y to implement a family of related syntax trees, rather than necessarily always parsing a list of SQL statements. raw_parser() gains a new argument, enum RawParseMode, to say what to do. As proof of concept, add a mode that just parses a TypeName without any other decoration, and use that to greatly simplify typeStringToTypeName(). In addition, invent a new SPI entry point SPI_prepare_extended() to allow SPI users (particularly plpgsql) to get at this new functionality. In hopes of making this the last variant of SPI_prepare(), set up its additional arguments as a struct rather than direct arguments, and promise that future additions to the struct can default to zero. SPI_prepare_cursor() and SPI_prepare_params() can perhaps go away at some point. Discussion: https://postgr.es/m/4165684.1607707277@sss.pgh.pa.us
-
Michael Paquier authored
Author: Justin Pryzby Discussion: https://postgr.es/m/X/Ff7jfnvJUab013@paquier.xyz
-