- 01 Dec, 2019 1 commit
-
-
Tom Lane authored
We implement ON COMMIT DELETE ROWS by truncating tables marked that way, which requires also truncating/rebuilding their indexes. But RelationTruncateIndexes asks the relcache for up-to-date copies of any index expressions, which may cause execution of eval_const_expressions on them, which can result in actual execution of subexpressions. This is a bad thing to have happening during ON COMMIT. Manuel Rigger reported that use of a SQL function resulted in crashes due to expectations that ActiveSnapshot would be set, which it isn't. The most obvious fix perhaps would be to push a snapshot during PreCommit_on_commit_actions, but I think that would just open the door to more problems: CommitTransaction explicitly expects that no user-defined code can be running at this point. Fortunately, since we know that no tuples exist to be indexed, there seems no need to use the real index expressions or predicates during RelationTruncateIndexes. We can set up dummy index expressions instead (we do need something that will expose the right data type, as there are places that build index tupdescs based on this), and just ignore predicates and exclusion constraints. In a green field it'd likely be better to reimplement ON COMMIT DELETE ROWS using the same "init fork" infrastructure used for unlogged relations. That seems impractical without catalog changes though, and even without that it'd be too big a change to back-patch. So for now do it like this. Per private report from Manuel Rigger. This has been broken forever, so back-patch to all supported branches.
-
- 30 Nov, 2019 3 commits
-
-
Andrew Dunstan authored
This patch providies for support for password protected SSL client keys in libpq, and for DER format keys, both encrypted and unencrypted. There is a new connection parameter sslpassword, which is supplied to the OpenSSL libraries via a callback function. The callback function can also be set by an application by calling PQgetSSLKeyPassHook(). There is also a function to retreive the connection setting, PQsslpassword(). Craig Ringer and Andrew Dunstan Reviewed by: Greg Nancarrow Discussion: https://postgr.es/m/f7ee88ed-95c4-95c1-d4bf-7b415363ab62@2ndQuadrant.com
-
Tomas Vondra authored
When using %b or %B patterns to format a date, the code was simply using tm_mon as an index into array of month names. But that is wrong, because tm_mon is 1-based, while array indexes are 0-based. The result is we either use name of the next month, or a segfault (for December). Fix by subtracting 1 from tm_mon for both patterns, and add a regression test triggering the issue. Backpatch to all supported versions (the bug is there far longer, since at least 2003). Reported-by: Paul Spencer Backpatch-through: 9.4 Discussion: https://postgr.es/m/16143-0d861eb8688d3fef%40postgresql.org
-
Amit Kapila authored
This commit revert the commits to add a test case that tests the 'force' option when there is an active backend connected to the database being dropped. This feature internally sends SIGTERM to all the backends connected to the database being dropped and then the same is reported to the client. We found that on Windows, the client end of the socket is not able to read the data once we close the socket in the server which leads to loss of error message which is not what we expect. We also observed similar behavior in other cases like pg_terminate_backend(), pg_ctl kill TERM <pid>. There are probably a few others like that. The fix for this requires further study. Discussion: https://postgr.es/m/E1iaD8h-0004us-K9@gemulon.postgresql.org
-
- 29 Nov, 2019 5 commits
-
-
Peter Eisentraut authored
FLOAT8PASSBYVAL can be used instead of USE_FLOAT8_BYVAL here.
-
Peter Eisentraut authored
Add a regression test file that exercises the kinds of commands that allow_system_table_mods allows. This is put in the "unsafe_tests" suite, so it won't accidentally create a mess if someone runs the normal regression tests against an instance that they care about. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/8b00ea5e-28a7-88ba-e848-21528b632354%402ndquadrant.com
-
Peter Eisentraut authored
Make allow_system_table_mods settable at run time by superusers. It was previously postmaster start only. We don't want to make system catalog DDL wide-open, but there are occasionally useful things to do like setting reloptions or statistics on a busy system table, and blocking those doesn't help anyone. Also, this enables the possibility of writing a test suite for this setting. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/8b00ea5e-28a7-88ba-e848-21528b632354%402ndquadrant.com
-
Peter Eisentraut authored
Previously, allow_system_table_mods allowed a non-superuser to do DML on a system table without further permission checks. This has been removed, as it was quite inconsistent with the rest of the meaning of this setting. (Since allow_system_table_mods was previously only accessible with a server restart, it is unlikely that anyone was using this possibility.) Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/8b00ea5e-28a7-88ba-e848-21528b632354%402ndquadrant.com
-
Peter Eisentraut authored
Reviewed-by: Pavel Stehule <pavel.stehule@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/6e7aa4a1-be6a-1a75-b1f9-83a678e5184a@2ndquadrant.com
-
- 28 Nov, 2019 8 commits
-
-
Tomas Vondra authored
The byte loop used in pglz_decompress() because of possible overlap may be quite inefficient, so this commit replaces it with memcpy. The gains do depend on the data (compressibility) and hardware, but seem to be quite significant. Author: Andrey Borodin Reviewed-by: Michael Paquier, Konstantin Knizhnik, Tels Discussion: https://postgr.es/m/469C9ED9-348C-4FE7-A7A7-B0FA671BEE4C@yandex-team.ru
-
Tomas Vondra authored
Commit c676e659 reworked how choose_best_statistics() picks the best extended statistics, but failed to remove clauses_attnums which is now unnecessary. So get rid of it and backpatch to 12, same as c676e659. Author: Tomas Vondra Discussion: https://postgr.es/m/CA+u7OA7H5rcE2=8f263w4NZD6ipO_XOrYB816nuLXbmSTH9pQQ@mail.gmail.com Backpatch-through: 12
-
Tomas Vondra authored
When picking the best extended statistics object for a list of clauses, it's not enough to look at attnums extracted from the clause list as a whole. Consider for example this query with OR clauses: SELECT * FROM t WHERE (t.a = 1) OR (t.b = 1) OR (t.c = 1) with a statistics defined on columns (a,b). Relying on attnums extracted from the whole OR clause, we'd consider the statistics usable. That does not work, as we see the conditions as a single OR-clause, referencing an attribute not covered by the statistic, leading to empty list of clauses to be estimated using the statistics and an assert failure. This changes choose_best_statistics to check which clauses are actually covered, and only using attributes from the fully covered ones. For the previous example this means the statistics object will not be considered as compatible with the OR-clause. Backpatch to 12, where MCVs were introduced. The issue does not affect older versions because functional dependencies don't handle OR clauses. Author: Tomas Vondra Reviewed-by: Dean Rasheed Reported-By: Manuel Rigger Discussion: https://postgr.es/m/CA+u7OA7H5rcE2=8f263w4NZD6ipO_XOrYB816nuLXbmSTH9pQQ@mail.gmail.com Backpatch-through: 12
-
Alvaro Herrera authored
Discussion: https://postgr.es/m/20191128144653.GA27883@alvherre.pgsql
-
Amit Kapila authored
This will test that the force option works when there is an active backend connected to the database being dropped. Author: Pavel Stehule and Vignesh C Reviewed-by: Amit Kapila and Vignesh C Discussion: https://postgr.es/m/CAP_rwwmLJJbn70vLOZFpxGw3XD7nLB_7+NKz46H5EOO2k5H7OQ@mail.gmail.com
-
Amit Kapila authored
The subroutine pump_until provides the functionality to poll until the given string is matched, or a timeout occurs. This can be used from other places as well, so moving it to TestLib.pm. The immediate need is for an upcoming regression test patch for dropdb utility. Author: Vignesh C Reviewed-by: Amit Kapila Discussion: https://postgr.es/m/CAP_rwwmLJJbn70vLOZFpxGw3XD7nLB_7+NKz46H5EOO2k5H7OQ@mail.gmail.com
-
Bruce Momjian authored
Previously these error paragraphs were too wide. Backpatch-through: 13
-
Bruce Momjian authored
This clarifies instructions on how to dump/reload databases that use incompatible isn versions. Reported-by: Alvaro Herrera Discussion: https://postgr.es/m/20191114190652.GA23586@alvherre.pgsqlReviewed-by: Daniel Gustafsson <daniel@yesql.se> Backpatch-through: 13
-
- 27 Nov, 2019 4 commits
-
-
Andrew Dunstan authored
Commit 114541d5 has upset some buildfarm members running Msys, that weren't previously having problems, so the use of native Windows methods to open files is restricted by this patch to only MSVC builds.
-
Andrew Dunstan authored
This reverts commits 9af34f3c and 792dba73. The code has been found not to be portable. Discussion: https://postgr.es/m/2658c496-f885-02db-13bb-228423782524@2ndQuadrant.com
-
Peter Eisentraut authored
This build option was once useful to maintain compatibility with version-0 functions, but those are no longer supported, so this option is no longer useful for end users. We keep the option available to developers in pg_config_manual.h so that it is easy to test the pass-by-reference code paths without having to fire up a 32-bit machine. Discussion: https://www.postgresql.org/message-id/flat/f3e1e576-2749-bbd7-2d57-3f9dcf75255a@2ndquadrant.com
-
Etsuro Fujita authored
-
- 26 Nov, 2019 4 commits
-
-
Andrew Dunstan authored
Commit 9af34f3c naively assumed that all non-windows platforms would have pseudoterminals and that perl would have IO::Pty. Such is not the case. Instead of assumung anything, test for the presence of IO::Pty and only use code that might depend on it if it's present. The test result is exposed in $TestLib::have_io_pty so that it can be conveniently used in SKIP tests. Discussion: https://postgr.es/m/20191126044110.GB5435@paquier.xyz
-
Tom Lane authored
The fix for CVE-2017-7484 disallowed use of pg_statistic data for planning purposes if the user would not be able to select the associated column and a non-leakproof function is to be applied to the statistics values. That turns out to disable use of pg_statistic data in some common cases involving inheritance/partitioning, where the user does have permission to select from the parent table that was actually named in the query, but not from a child table whose stats are needed. Since, in non-corner cases, the user *can* select the child table's data via the parent, this restriction is not actually useful from a security standpoint. Improve the logic so that we also check the permissions of the originally-named table, and allow access if select permission exists for that. When checking access to stats for a simple child column, we can map the child column number back to the parent, and perform this test exactly (including not allowing access if the child column isn't exposed by the parent). For expression indexes, the current logic just insists on whole-table select access, and this patch allows access if the user can select the whole parent table. In principle, if the child table has extra columns, this might allow access to stats on columns the user can't read. In practice, it's unlikely that the planner is going to do any stats calculations involving expressions that are not visible to the query, so we'll ignore that fine point for now. Perhaps someday we'll improve that logic to detect exactly which columns are used by an expression index ... but today is not that day. Back-patch to v11. The issue was created in 9.2 and up by the CVE-2017-7484 fix, but this patch depends on the append_rel_array[] planner data structure which only exists in v11 and up. In practice the issue is most urgent with partitioned tables, so fixing v11 and later should satisfy much of the practical need. Dilip Kumar and Amit Langote, with some kibitzing by me Discussion: https://postgr.es/m/3876.1531261875@sss.pgh.pa.us
-
Michael Paquier authored
On some platforms, fsync() returns EBADFD when opening a file descriptor with O_RDONLY (read-only), leading ultimately now to a PANIC to prevent data corruption. This commit adds a new sanity check in pg_fsync() based on fcntl() to make sure that we don't repeat again mistakes with incorrectly-set file descriptors so as problems are detected at an early stage. Without that, such errors could only be detected after running Postgres on a specific supported platform for the culprit code path, which could take some time before being found. b8e19b93 was a fix for such a problem, which got undetected for more than 5 years, and a586cc4b fixed another similar issue. Note that the new check added works as well when fsync=off is configured, so as all regression tests would detect problems as long as assertions are enabled. fcntl() being not available on Windows, the new checks do not happen there. Author: Michael Paquier Reviewed-by: Mark Dilger Discussion: https://postgr.es/m/20191009062640.GB21379@paquier.xyz
-
Amit Kapila authored
Revert part of commit 19df1702f5. Early shutdown was added by that commit so that we could collect statistics from workers, but unfortunately, it interacted badly with rescans. The problem is that we ended up destroying the parallel context which is required for rescans. This leads to rescans of a Limit node over a Gather node to produce unpredictable results as it tries to access destroyed parallel context. By reverting the early shutdown code, we might lose statistics in some cases of Limit over Gather [Merge], but that will require further study to fix. Reported-by: Jerry Sievers Diagnosed-by: Thomas Munro Author: Amit Kapila, testcase by Vignesh C Backpatch-through: 9.6 Discussion: https://postgr.es/m/87ims2amh6.fsf@jsievers.enova.com
-
- 25 Nov, 2019 7 commits
-
-
Robert Haas authored
AuxiliaryProcessMain does ProcSignalInit, so one might expect that auxiliary processes would need to respond to SendProcSignal, but none of the auxiliary processes do that. Change them to use procsignal_sigusr1_handler instead of their own private handlers so that they do. Besides seeming more correct, this is also less code. It shouldn't make any functional difference right now because, as far as we know, there are no current cases where SendProcSignal targets an auxiliary process, but there are plans to change that in the future. Andres Freund Discussion: http://postgr.es/m/20181030051643.elbxjww5jjgnjaxg@alap3.anarazel.de
-
Andrew Dunstan authored
Where possible, do this using a pseudoterminal, so that things like openssl that want to open /dev/tty if stdin isn't a tty won't. Elsewhere, i.e. Windows, just close by providing an empty string using the standard IPC::Run pipe mechanism. Patch by Andrew Dunstan, based on an idea from Craig Ringer. Reviewed by Mark Dilger. Discussion: https://postgr.es/m/873ebb57-fc98-340d-949d-691b1810bf66@2ndQuadrant.com
-
Alvaro Herrera authored
XLogReader, walsender and pg_waldump all had their own routines to read data from WAL files to memory, with slightly different approaches according to the particular conditions of each environment. There's a lot of commonality, so we can refactor that into a single routine WALRead in XLogReader, and move the differences to a separate (simpler) callback that just opens the next WAL-segment. This results in a clearer (ahem) code flow. The error reporting needs are covered by filling in a new error-info struct, WALReadError, and it's the caller's responsibility to act on it. The backend has WALReadRaiseError() to do so. We no longer ever need to seek in this interface; switch to using pg_pread(). Author: Antonin Houska, with contributions from Álvaro Herrera Reviewed-by: Michaël Paquier, Kyotaro Horiguchi Discussion: https://postgr.es/m/14984.1554998742@spoje.net
-
Amit Kapila authored
Similar to commits 14aec035, 7e735035 and dddf4cdc, this commit makes the order of header file inclusion consistent in more places. Author: Vignesh C Reviewed-by: Amit Kapila Discussion: https://postgr.es/m/CALDaNm2Sznv8RR6Ex-iJO6xAdsxgWhCoETkaYX=+9DW3q0QCfA@mail.gmail.com
-
Michael Paquier authored
Both argument names were reversed in the declaration of the function. Author: Ranier Vilela Discussion: https://postgr.es/m/MN2PR18MB292755AEFF9A9144B220ABEEE34B0@MN2PR18MB2927.namprd18.prod.outlook.com
-
Michael Paquier authored
This reworks the reloption parsing and build of a couple of index AMs by creating new structures for each index AM's options. This split was already done for BRIN, GIN and GiST (which actually has a fillfactor parameter), but not for hash, B-tree and SPGiST which relied on StdRdOptions due to an overlap with the default option set. This saves a couple of bytes for rd_options in each relcache entry with indexes making use of relation options, and brings more consistency between all index AMs. While on it, add a couple of AssertMacro() calls to make sure that utility macros to grab values of reloptions are used with the expected index AM. Author: Nikolay Shaplov Reviewed-by: Amit Langote, Michael Paquier, Álvaro Herrera, Dent John Discussion: https://postgr.es/m/4127670.gFlpRb6XCm@x200m
-
- 24 Nov, 2019 7 commits
-
-
Andrew Dunstan authored
It is hoped that this will avoid some errors that we have seen before, but lately with greater frequency, in buildfarm animals. For now apply only to master. If this proves effective it can be backpatched. Discussion: https://postgr.es/m/13900.1572839580@sss.pgh.pa.us Author: Juan José Santamaría Flecha
-
Tom Lane authored
The user docs didn't really explain how to use LISTEN safely, so clarify that. Also clean up some fuzzy-headed explanations in comments. No code changes. Discussion: https://postgr.es/m/3ac7f397-4d5f-be8e-f354-440020675694@gmail.com
-
Tom Lane authored
If LISTEN is the only action in a serializable-mode transaction, and the session was not previously listening, and the notify queue is not empty, predicate.c reported an assertion failure. That happened because we'd acquire the transaction's initial snapshot during PreCommit_Notify, which was called *after* predicate.c expects any such snapshot to have been established. To fix, just swap the order of the PreCommit_Notify and PreCommit_CheckForSerializationFailure calls during CommitTransaction. This will imply holding the notify-insertion lock slightly longer, but the difference could only be meaningful in serializable mode, which is an expensive option anyway. It appears that this is just an assertion failure, with no consequences in non-assert builds. A snapshot used only to scan the notify queue could not have been involved in any serialization conflicts, so there would be nothing for PreCommit_CheckForSerializationFailure to do except assign it a prepareSeqNo and set the SXACT_FLAG_PREPARED flag. And given no conflicts, neither of those omissions affect the behavior of ReleasePredicateLocks. This admittedly once-over-lightly analysis is backed up by the lack of field reports of trouble. Per report from Mark Dilger. The bug is old, so back-patch to all supported branches; but the new test case only goes back to 9.6, for lack of adequate isolationtester infrastructure before that. Discussion: https://postgr.es/m/3ac7f397-4d5f-be8e-f354-440020675694@gmail.com Discussion: https://postgr.es/m/13881.1574557302@sss.pgh.pa.us
-
Thomas Munro authored
Back-patch to 10. Author: Andreas Karlsson Discussion: https://postgr.es/m/043acae2-a369-b7fa-be48-1933aa2e82d1%40proxel.se
-
Tom Lane authored
This patch ensures that, if any notify messages were received during a just-finished transaction, they get sent to the frontend just before not just after the ReadyForQuery message. With libpq and other client libraries that act similarly, this guarantees that the client will see the notify messages as available as soon as it thinks the transaction is done. This probably makes no difference in practice, since in realistic use-cases the application would have to cope with asynchronous arrival of notify events anyhow. However, it makes it a lot easier to build cross-session-notify test cases with stable behavior. I'm a bit surprised now that we've not seen any buildfarm instability with the test cases added by commit b10f40bf. Tests that I intend to add in an upcoming bug fix are definitely unstable without this. Back-patch to 9.6, which is as far back as we can do NOTIFY testing with the isolationtester infrastructure. Discussion: https://postgr.es/m/13881.1574557302@sss.pgh.pa.us
-
Tom Lane authored
This function wasn't touched in commit 51004c71, but that turns out to be a bad idea, because its results now include any dead space that exists in the NOTIFY queue on account of our being lazy about advancing the queue tail. Notably, the isolation tests now fail if run twice without a server restart between, because async-notify's first test of the function will already show a positive value. It seems likely that end users would be equally unhappy about the result's instability. To fix, just make the function call asyncQueueAdvanceTail before computing its result. That should end in producing the same value as before, and it's hard to believe that there's any practical use-case where pg_notification_queue_usage() is called so often as to create a performance degradation, especially compared to what we did before. Out of paranoia, also mark this function parallel-restricted (it was volatile, but parallel-safe by default, before). Although the code seems to work fine when run in a parallel worker, that's outside the design scope of async.c, and it's a bit scary to have intentional side-effects happening in a parallel worker. There seems no plausible use-case where it'd be important to try to parallelize this, so let's not take any risk of introducing new bugs. In passing, re-pgindent async.c and run reformat-dat-files on pg_proc.dat, just because I'm a neatnik. Discussion: https://postgr.es/m/13881.1574557302@sss.pgh.pa.us
-
Tom Lane authored
Commit abd9ca37 replaced a couple of while-loops in fmtfloat() with calls to dopr_outchmulti, but I (tgl) failed to notice that the new if-tests guarding those calls were really unnecessary, because they're inside a larger if-block checking the same thing. Ranier Vilela Discussion: https://postgr.es/m/MN2PR18MB2927850AB00CF39CC370D107E34B0@MN2PR18MB2927.namprd18.prod.outlook.com
-
- 23 Nov, 2019 1 commit
-
-
Alvaro Herrera authored
This Assert(false) was not supposed to be in the committed copy. Reported by: Tom Lane Discussion: https://postgr.es/m/26476.1574525468@sss.pgh.pa.us
-