summaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAgeFilesLines
* Stamp 9.5.24.REL9_5_24Tom Lane2020-11-096-21/+21
|
* Last-minute updates for release notes.Tom Lane2020-11-091-73/+143
| | | | Security: CVE-2020-25694, CVE-2020-25695, CVE-2020-25696
* Doc: clarify data type behavior of COALESCE and NULLIF.Tom Lane2020-11-092-6/+33
| | | | | | | After studying the code, NULLIF is a lot more subtle than you might have guessed. Discussion: https://postgr.es/m/160486028730.25500.15740897403028593550@wrigleys.postgresql.org
* Ignore attempts to \gset into specially treated variables.Noah Misch2020-11-095-0/+33
| | | | | | | | | | | | | | | If an interactive psql session used \gset when querying a compromised server, the attacker could execute arbitrary code as the operating system account running psql. Using a prefix not found among specially treated variables, e.g. every lowercase string, precluded the attack. Fix by issuing a warning and setting no variable for the column in question. Users wanting the old behavior can use a prefix and then a meta-command like "\set HISTSIZE :prefix_HISTSIZE". Back-patch to 9.5 (all supported versions). Reviewed by Robert Haas. Reported by Nick Cleaton. Security: CVE-2020-25696
* In security-restricted operations, block enqueue of at-commit user code.Noah Misch2020-11-096-6/+104
| | | | | | | | | | | | | | | | | | Specifically, this blocks DECLARE ... WITH HOLD and firing of deferred triggers within index expressions and materialized view queries. An attacker having permission to create non-temp objects in at least one schema could execute arbitrary SQL functions under the identity of the bootstrap superuser. One can work around the vulnerability by disabling autovacuum and not manually running ANALYZE, CLUSTER, REINDEX, CREATE INDEX, VACUUM FULL, or REFRESH MATERIALIZED VIEW. (Don't restore from pg_dump, since it runs some of those commands.) Plain VACUUM (without FULL) is safe, and all commands are fine when a trusted user owns the target object. Performance may degrade quickly under this workaround, however. Back-patch to 9.5 (all supported versions). Reviewed by Robert Haas. Reported by Etienne Stalmans. Security: CVE-2020-25695
* Translation updatesPeter Eisentraut2020-11-0957-7771/+46448
| | | | | Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git Source-Git-Hash: 8740d75e31771525e9bb55e71bfd343faae253b4
* Release notes for 13.1, 12.5, 11.10, 10.15, 9.6.20, 9.5.24.Tom Lane2020-11-081-0/+804
|
* Fix redundant error messages in client toolsPeter Eisentraut2020-11-072-4/+2
| | | | | | A few client tools duplicate error messages already provided by libpq. Discussion: https://www.postgresql.org/message-id/flat/3e937641-88a1-e697-612e-99bba4b8e5e4%40enterprisedb.com
* Properly detoast data in brin_form_tupleTomas Vondra2020-11-073-1/+172
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | brin_form_tuple failed to consider the values may be toasted, inserting the toast pointer into the index. This may easily result in index corruption, as the toast data may be deleted and cleaned up by vacuum. The cleanup however does not care about indexes, leaving invalid toast pointers behind, which triggers errors like this: ERROR: missing chunk number 0 for toast value 16433 in pg_toast_16426 A less severe consequence are inconsistent failures due to the index row being too large, depending on whether brin_form_tuple operated on plain or toasted version of the row. For example CREATE TABLE t (val TEXT); INSERT INTO t VALUES ('... long value ...') CREATE INDEX idx ON t USING brin (val); would likely succeed, as the row would likely include toast pointer. Switching the order of INSERT and CREATE INDEX would likely fail: ERROR: index row size 8712 exceeds maximum 8152 for index "idx" because this happens before the row values are toasted. The bug exists since PostgreSQL 9.5 where BRIN indexes were introduced. So backpatch all the way back. Author: Tomas Vondra Reviewed-by: Alvaro Herrera Backpatch-through: 9.5 Discussion: https://postgr.es/m/20201001184133.oq5uq75sb45pu3aw@development Discussion: https://postgr.es/m/20201104010544.zexj52mlldagzowv%40development
* Revert "Accept relations of any kind in LOCK TABLE".Tom Lane2020-11-064-44/+20
| | | | | | | | | | Revert 59ab4ac32, as well as the followup fix 33862cb9c, in all branches. We need to think a bit harder about what the behavior of LOCK TABLE on views should be, and there's no time for that before next week's releases. We'll take another crack at this later. Discussion: https://postgr.es/m/16703-e348f58aab3cf6cc@postgresql.org
* Revert "pg_dump: Lock all relations, not just plain tables".Tom Lane2020-11-064-79/+3
| | | | | | | | | | Revert 403a3d91c, as well as the followup fix 7f4235032, in all branches. We need to think a bit harder about what the behavior of LOCK TABLE on views should be, and there's no time for that before next week's releases. We'll take another crack at this later. Discussion: https://postgr.es/m/16703-e348f58aab3cf6cc@postgresql.org
* Doc: undo mistaken adjustment to LOCK TABLE docs in back branches.Tom Lane2020-11-061-2/+7
| | | | | | | | | | | | | Commits 59ab4ac32 et al mistakenly copied-and-pasted some text about how LOCK on a view recurses to referenced tables into pre-v11 branches, which in fact don't do that. Undo that, and instead state clearly that they don't. (I also chose to add a note that this behavior changed in v11. We usually don't back-patch such statements, but since it's easy to add the warning now, might as well.) Noted while considering followup fixes for bug #16703. Discussion: https://postgr.es/m/16703-e348f58aab3cf6cc@postgresql.org
* Allow users with BYPASSRLS to alter their own passwords.Tom Lane2020-11-033-8/+19
| | | | | | | | | | | | | | | | The intention in commit 491c029db was to require superuserness to change the BYPASSRLS property, but the actual effect of the coding in AlterRole() was to require superuserness to change anything at all about a BYPASSRLS role. Other properties of a BYPASSRLS role should be changeable under the same rules as for a normal role, though. Fix that, and also take care of some documentation omissions related to BYPASSRLS and REPLICATION role properties. Tom Lane and Stephen Frost, per bug report from Wolfgang Walther. Back-patch to all supported branches. Discussion: https://postgr.es/m/a5548a9f-89ee-3167-129d-162b5985fcf8@technowledgy.de
* Avoid null pointer dereference if error result lacks SQLSTATE.Tom Lane2020-11-011-6/+8
| | | | | | | | | Although error results received from the backend should always have a SQLSTATE field, ones generated by libpq won't, making this code vulnerable to a crash after, say, untimely loss of connection. Noted by Coverity. Oversight in commit 403a3d91c. Back-patch to 9.5, as that was.
* Use mode "r" for popen() in psql's evaluate_backtick().Tom Lane2020-10-281-2/+2
| | | | | | | | | | | | | | | | | | In almost all other places, we use plain "r" or "w" mode in popen() calls (the exceptions being for COPY data). This one has been overlooked (possibly because it's buried in a ".l" flex file?), but it's using PG_BINARY_R. Kensuke Okamura complained in bug #16688 that we fail to strip \r when stripping the trailing newline from a backtick result string. That's true enough, but we'd also fail to convert embedded \r\n cleanly, which also seems undesirable. Fixing the popen() mode seems like the best way to deal with this. It's been like this for a long time, so back-patch to all supported branches. Discussion: https://postgr.es/m/16688-c649c7b69cd7e6f8@postgresql.org
* Fix use-after-free bug with event triggers and ALTER TABLE.Tom Lane2020-10-271-0/+6
| | | | | | | | | | | | | | | | | EventTriggerAlterTableEnd neglected to make sure that it built its output list in the right context. In simple cases this was masked because the function is called in PortalContext which will be sufficiently long-lived anyway; but that doesn't make it not a bug. Commit ced138e8c fixed this in HEAD and v13, but mistakenly chose not to back-patch further. Back-patch the same code change all the way (I didn't bother with the test case though, as it would prove nothing in pre-v13 branches). Per report from Arseny Sher. Original fix by Jehan-Guillaume de Rorthais. Discussion: https://postgr.es/m/877drcyprb.fsf@ars-thinkpad Discussion: https://postgr.es/m/20200902193715.6e0269d4@firost
* Makefile comment: remove reference to tools/thread/thread_testBruce Momjian2020-10-271-1/+0
| | | | | | | | | | You can't compile thread_test alone anymore, and the location moved too. Reported-by: Tom Lane Discussion: https://postgr.es/m/1062278.1603819969@sss.pgh.pa.us Backpatch-through: 9.5
* pg_dump: Lock all relations, not just plain tablesAlvaro Herrera2020-10-274-3/+77
| | | | | | | | | | | | | | | | Now that LOCK TABLE can take any relation type, acquire lock on all relations that are to be dumped. This prevents schema changes or deadlock errors that could cause a dump to fail after expending much effort. The server is tested to have the capability and the feature disabled if it doesn't, so that a patched pg_dump doesn't fail when connecting to an unpatched server. Backpatch to 9.5. Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reported-by: Wells Oliver <wells.oliver@gmail.com> Discussion: https://postgr.es/m/20201021200659.GA32358@alvherre.pgsql
* Accept relations of any kind in LOCK TABLEAlvaro Herrera2020-10-274-21/+40
| | | | | | | | | | | | | | | The restriction that only tables and views can be locked by LOCK TABLE is quite arbitrary, since the underlying mechanism can lock any relation type. Drop the restriction so that programs such as pg_dump can lock all relations they're interested in, preventing schema changes that could cause a dump to fail after expending much effort. Backpatch to 9.5. Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reported-by: Wells Oliver <wells.oliver@gmail.com> Discussion: https://postgr.es/m/20201021200659.GA32358@alvherre.pgsql
* docs: remove reference to src/tools/threadBruce Momjian2020-10-271-8/+0
| | | | | | | | | | | This directory and the ability to build the thread test independently were removed in commit 8a2121185b. Reported-by: e.indrupskaya@postgrespro.ru Discussion: https://postgr.es/m/160379609706.24746.7506163279454026608@wrigleys.postgresql.org Backpatch-through: 9.5
* doc: simplify wording of function error affectsBruce Momjian2020-10-261-2/+2
| | | | | | | | Reported-by: bob.henkel@gmail.com Discussion: https://postgr.es/m/160324449781.693.8298142858847611071@wrigleys.postgresql.org Backpatch-through: 9.5
* Fix ancient bug in ecpg's pthread_once() emulation for Windows.Tom Lane2020-10-241-1/+1
| | | | | | | | | | | | | | | | We must not set the "done" flag until after we've executed the initialization function. Otherwise, other threads can fall through the initial unlocked test before initialization is really complete. This has been seen to cause rare failures of ecpg's thread/descriptor test, and it could presumably cause other sorts of misbehavior in threaded ECPG-using applications, since ecpglib relies on pthread_once() in several places. Diagnosis and patch by me, based on investigation by Alexander Lakhin. Back-patch to all supported branches (the bug dates to 2007). Discussion: https://postgr.es/m/16685-d6cd241872c101d3@postgresql.org
* Update time zone data files to tzdata release 2020d.Tom Lane2020-10-221-8/+12
| | | | | DST law changes in Palestine, with a whopping 120 hours' notice. Also some historical corrections for Palestine.
* Sync our copy of the timezone library with IANA release tzcode2020d.Tom Lane2020-10-222-3/+13
| | | | | | | | There's no functional change at all here, but I'm curious to see whether this change successfully shuts up Coverity's warning about a useless strcmp(), which appeared with the previous update. Discussion: http://mm.icann.org/pipermail/tz/2020-October/029370.html
* Fix connection string handling in psql's \connect command.Tom Lane2020-10-212-87/+229
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | psql's \connect claims to be able to re-use previous connection parameters, but in fact it only re-uses the database name, user name, host name (and possibly hostaddr, depending on version), and port. This is problematic for assorted use cases. Notably, pg_dump[all] emits "\connect databasename" commands which we would like to have re-use all other parameters. If such a script is loaded in a psql run that initially had "-d connstring" with some non-default parameters, those other parameters would be lost, potentially causing connection failure. (Thus, this is the same kind of bug addressed in commits a45bc8a4f and 8e5793ab6, although the details are much different.) To fix, redesign do_connect() so that it pulls out all properties of the old PGconn using PQconninfo(), and then replaces individual properties in that array. In the case where we don't wish to re-use anything, get libpq's default settings using PQconndefaults() and replace entries in that, so that we don't need different code paths for the two cases. This does result in an additional behavioral change for cases where the original connection parameters allowed multiple hosts, say "psql -h host1,host2", and the \connect request allows re-use of the host setting. Because the previous coding relied on PQhost(), it would only permit reconnection to the same host originally selected. Although one can think of scenarios where that's a good thing, there are others where it is not. Moreover, that behavior doesn't seem to meet the principle of least surprise, nor was it documented; nor is it even clear it was intended, since that coding long pre-dates the addition of multi-host support to libpq. Hence, this patch is content to drop it and re-use the host list as given. Per Peter Eisentraut's comments on bug #16604. Back-patch to all supported branches. Discussion: https://postgr.es/m/16604-933f4b8791227b15@postgresql.org
* Avoid invalid alloc size error in shm_mqPeter Eisentraut2020-10-201-0/+24
| | | | | | | | | | | | In shm_mq_receive(), a huge payload could trigger an unjustified "invalid memory alloc request size" error due to the way the buffer size is increased. Add error checks (documenting the upper limit) and avoid the error by limiting the allocation size to MaxAllocSize. Author: Markus Wanner <markus.wanner@2ndquadrant.com> Discussion: https://www.postgresql.org/message-id/flat/3bb363e7-ac04-0ac4-9fe8-db1148755bfa%402ndquadrant.com
* Fix connection string handling in src/bin/scripts/ programs.Tom Lane2020-10-1916-200/+274
| | | | | | | | | | | | | | | | | | | | | When told to process all databases, clusterdb, reindexdb, and vacuumdb would reconnect by replacing their --maintenance-db parameter with the name of the target database. If that parameter is a connstring (which has been allowed for a long time, though we failed to document that before this patch), we'd lose any other options it might specify, for example SSL or GSS parameters, possibly resulting in failure to connect. Thus, this is the same bug as commit a45bc8a4f fixed in pg_dump and pg_restore. We can fix it in the same way, by using libpq's rules for handling multiple "dbname" parameters to add the target database name separately. I chose to apply the same refactoring approach as in that patch, with a struct to handle the command line parameters that need to be passed through to connectDatabase. (Maybe someday we can unify the very similar functions here and in pg_dump/pg_restore.) Per Peter Eisentraut's comments on bug #16604. Back-patch to all supported branches. Discussion: https://postgr.es/m/16604-933f4b8791227b15@postgresql.org
* Misc documentation fixes.Heikki Linnakangas2020-10-1914-39/+39
| | | | | | | | | | | | | | | | | | | - Misc grammar and punctuation fixes. - Stylistic cleanup: use spaces between function arguments and JSON fields in examples. For example "foo(a,b)" -> "foo(a, b)". Add semicolon after last END in a few PL/pgSQL examples that were missing them. - Make sentence that talked about "..." and ".." operators more clear, by avoiding to end the sentence with "..". That makes it look the same as "..." - Fix syntax description for HAVING: HAVING conditions cannot be repeated Patch by Justin Pryzby, per Yaroslav Schekin's report. Backpatch to all supported versions, to the extent that the patch applies easily. Discussion: https://www.postgresql.org/message-id/20201005191922.GE17626%40telsasoft.com
* In libpq for Windows, call WSAStartup once and WSACleanup not at all.Tom Lane2020-10-193-44/+18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The Windows documentation insists that every WSAStartup call should have a matching WSACleanup call. However, if that ever had actual relevance, it wasn't in this century. Every remotely-modern Windows kernel is capable of cleaning up when a process exits without doing that, and must be so to avoid resource leaks in case of a process crash. Moreover, Postgres backends have done WSAStartup without WSACleanup since commit 4cdf51e64 in 2004, and we've never seen any indication of a problem with that. libpq's habit of doing WSAStartup during connection start and WSACleanup during shutdown is also rather inefficient, since a series of non-overlapping connection requests leads to repeated, quite expensive DLL unload/reload cycles. We document a workaround for that (having the application call WSAStartup for itself), but that's just a kluge. It's also worth noting that it's far from uncommon for applications to exit without doing PQfinish, and we've not heard reports of trouble from that either. However, the real reason for acting on this is that recent experiments by Alexander Lakhin show that calling WSACleanup during PQfinish is triggering the symptom we occasionally see that a process using libpq fails to emit expected stdio output. Therefore, let's change libpq so that it calls WSAStartup only once per process, during the first connection attempt, and never calls WSACleanup at all. While at it, get rid of the only other WSACleanup call in our code tree, in pg_dump/parallel.c; that presumably is equally useless. Back-patch of HEAD commit 7d00a6b2d. Discussion: https://postgr.es/m/ac976d8c-03df-d6b8-025c-15a2de8d9af1@postgrespro.ru
* Relax some asserts in merge join costing codeDavid Rowley2020-10-201-4/+6
| | | | | | | | | | | | | | | | | | | | | | | | In the planner, it was possible, given an extreme enough case containing a large number of joins for the number of estimated rows to become infinite. This could cause problems in initial_cost_mergejoin() where we perform some calculations based on those row estimates. A problem case, presented by Onder Kalaci showed an Assert failure from an Assert checking outerstartsel <= outerendsel. In his test case this was effectively NaN <= Inf, which is false. The NaN outerstartsel came from multiplying the infinite outer_path_rows by 0.0. In master, this problem was fixed by a90c950fc, however, that fix was too invasive for the backbranches. Here we just relax the Asserts to allow them to pass. The worst that appears to happen from this is that we show NaN cost values and infinite row estimates in EXPLAIN. add_path() would have had a hard time doing anything useful with such costs, but that does not really matter as if the row estimates were even close to accurate, such plan would not complete this side of the heat death of the universe. Reported-by: Onder Kalaci Backpatch: 9.5 to 13 Discussion: https://postgr.es/m/DM6PR21MB1211FF360183BCA901B27F04D80B0@DM6PR21MB1211.namprd21.prod.outlook.com
* Fix potential memory leak in pgcryptoMichael Paquier2020-10-191-0/+1
| | | | | | | | | | | | | | | | | | | | | When allocating a EVP context, it would have been possible to leak some memory allocated directly by OpenSSL, that PostgreSQL lost track of if the initialization of the context allocated failed. The cleanup can be done with EVP_MD_CTX_destroy(). Note that EVP APIs exist since OpenSSL 0.9.7 and we have in the tree equivalent implementations for older versions since ce9b75d (code removed with 9b7cd59a as of 10~). However, in 9.5 and 9.6, the existing code makes use of EVP_MD_CTX_destroy() and EVP_MD_CTX_create() without an equivalent implementation when building the tree with OpenSSL 0.9.6 or older, meaning that this code is in reality broken with such versions since it got introduced in e2838c5. As we have heard no complains about that, it does not seem worth bothering with in 9.5 and 9.6, so I have left that out for simplicity. Author: Michael Paquier Discussion: https://postgr.es/m/20201015072212.GC2305@paquier.xyz Backpatch-through: 9.5
* Doc: caution against misuse of 'now' and related datetime literals.Tom Lane2020-10-172-6/+22
| | | | | | | | | | | | | | Section 8.5.1.4, which defines these literals, made only a vague reference to the fact that they might be evaluated too soon to be safe in non-interactive contexts. Provide a more explicit caution against misuse. Also, generalize the wording in the related tip in section 9.9.4: while it clearly described this problem, it implied (or really, stated outright) that the problem only applies to table DEFAULT clauses. Per gripe from Tijs van Dam. Back-patch to all supported branches. Discussion: https://postgr.es/m/c2LuRv9BiRT3bqIo5mMQiVraEXey_25B4vUn0kDqVqilwOEu_iVF1tbtvLnyQK7yDG3PFaz_GxLLPil2SDkj1MCObNRVaac-7j1dVdFERk8=@thalex.com
* Update time zone data files to tzdata release 2020c.Tom Lane2020-10-161-40/+49
| | | | | | DST law changes in Morocco, Canadian Yukon, Fiji, Macquarie Island, Casey Station (Antarctica). Historical corrections for France, Hungary, Monaco.
* Sync our copy of the timezone library with IANA release tzcode2020c.Tom Lane2020-10-165-141/+80
| | | | | | | | | | | | | | | This changes zic's default output format from "-b fat" to "-b slim". We were already using "slim" in v13/HEAD, so those branches drop the explicit -b switch in the Makefiles. Instead, add an explicit "-b fat" in v12 and before, so that we don't change the output file format in those branches. (This is perhaps excessively conservative, but we decided not to do so in a12079109, and I'll stick with that.) Other non-cosmetic changes are to drop support for zic's long-obsolete "-y" switch, and to ensure that strftime() does not change errno unless it fails. As usual with tzcode changes, back-patch to all supported branches.
* Add missing error check in pgcrypto/crypt-md5.c.Tom Lane2020-10-161-1/+7
| | | | | | | | | | | | | | | In theory, the second px_find_digest call in px_crypt_md5 could fail even though the first one succeeded, since resource allocation is required. Don't skip testing for a failure. (If one did happen, the likely result would be a crash rather than clean recovery from an OOM failure.) The code's been like this all along, so back-patch to all supported branches. Daniel Gustafsson Discussion: https://postgr.es/m/AA8D6FE9-4AB2-41B4-98CB-AE64BA668C03@yesql.se
* pg_upgrade: remove C99 compiler req. from commit 3c0471b5fdBruce Momjian2020-10-151-1/+2
| | | | | | | | | | | | | | This commit required support for inline variable definition, which is not a requirement. RELEASE NOTE AUTHOR: the author of commit 3c0471b5fd (pg_upgrade/tablespaces) was Justin Pryzby, not me. Reported-by: Andres Freund Discussion: https://postgr.es/m/20201016001959.h24fkywfubkv2pc5@alap3.anarazel.de Backpatch-through: 9.5
* pg_upgrade: generate check error for left-over new tablespaceBruce Momjian2020-10-151-0/+40
| | | | | | | | | | | | | Previously, if pg_upgrade failed, and the user recreated the cluster but did not remove the new cluster tablespace directory, a later pg_upgrade would fail since the new tablespace directory would already exists. This adds error reporting for this during check. Reported-by: Justin Pryzby Discussion: https://postgr.es/m/20200925005531.GJ23631@telsasoft.com Backpatch-through: 9.5
* Fix memory leak when guc.c decides a setting can't be applied now.Tom Lane2020-10-121-2/+30
| | | | | | | | | | | | | | | | | | | | The prohibitValueChange code paths in set_config_option(), which are executed whenever we re-read a PGC_POSTMASTER variable from postgresql.conf, neglected to free anything before exiting. Thus we'd leak the proposed new value of a PGC_STRING variable, as noted by BoChen in bug #16666. For all variable types, if the check hook creates an "extra" chunk, we'd also leak that. These are malloc not palloc chunks, so there is no mechanism for recovering the leaks before process exit. Fortunately, the values are typically not very large, meaning you'd have to go through an awful lot of SIGHUP configuration-reload cycles to make the leakage amount to anything. Still, for a long-lived postmaster process it could potentially be a problem. Oversight in commit 2594cf0e8. Back-patch to all supported branches. Discussion: https://postgr.es/m/16666-2c41a4eec61b03e1@postgresql.org
* Fix optimization hazard in gram.y's makeOrderedSetArgs(), redux.Tom Lane2020-10-071-3/+3
| | | | | | | | | | | | | | | | It appears that commit cf63c641c, which intended to prevent misoptimization of the result-building step in makeOrderedSetArgs, didn't go far enough: buildfarm member hornet's version of xlc is now optimizing back to the old, broken behavior in which list_length(directargs) is fetched only after list_concat() has changed that value. I'm not entirely convinced whether that's an undeniable compiler bug or whether it can be justified by a sufficiently aggressive interpretation of C sequence points. So let's just change the code to make it harder to misinterpret. Back-patch to all supported versions, just in case. Discussion: https://postgr.es/m/1830491.1601944935@sss.pgh.pa.us
* Rethink recent fix for pg_dump's handling of extension config tables.Tom Lane2020-10-071-0/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 3eb3d3e78 was a few bricks shy of a load: while it correctly set the table's "interesting" flag when deciding to dump the data of an extension config table, it was not correct to clear that flag if we concluded we shouldn't dump the data. This led to the crash reported in bug #16655, because in fact we'll traverse dumpTableSchema anyway for all extension tables (to see if they have user-added seclabels or RLS policies). The right thing to do is to force "interesting" true in makeTableDataInfo, and otherwise leave the flag alone. (Doing it there is more future-proof in case additional calls are added, and it also avoids setting the flag unnecessarily if that function decides the table is non-dumpable.) This investigation also showed that while only the --inserts code path had an obvious failure in the case considered by 3eb3d3e78, the COPY code path also has a problem with not having loaded table subsidiary data. That causes fmtCopyColumnList to silently return an empty string instead of the correct column list. That accidentally mostly works, which perhaps is why we didn't notice this before. It would only fail if the restore column order is different from the dump column order, which only happens in weird inheritance cases, so it's not surprising nobody had hit the case with an extension config table. Nonetheless, it's a bug, and it goes a long way back, not just to v12 where the --inserts code path started to have a problem with this. In hopes of catching such cases a bit sooner in future, add some Asserts that "interesting" has been set in both dumpTableData and dumpTableSchema. Adjust the test case added by 3eb3d3e78 so that it checks the COPY rather than INSERT form of that bug, allowing it to detect the longer-standing symptom. Per bug #16655 from Cameron Daniel. Back-patch to all supported branches. Discussion: https://postgr.es/m/16655-5c92d6b3a9438137@postgresql.org Discussion: https://postgr.es/m/18048b44-3414-b983-8c7c-9165b177900d@2ndQuadrant.com
* pg_upgrade: remove pre-8.4 code and >= 8.4 checkBruce Momjian2020-10-062-32/+13
| | | | | | | | | | We only support upgrading from >= 8.4 so no need for this code or tests. Reported-by: Magnus Hagander Discussion: https://postgr.es/m/CABUevEx-D0PNVe00tkeQRGennZQwDtBJn=493MJt-x6sppbUxA@mail.gmail.com Backpatch-through: 9.5
* pg_upgrade; change major version comparisons to use <=, not <Bruce Momjian2020-10-064-7/+7
| | | | | | This makes checking for older major versions more consistent. Backpatch-through: 9.5
* doc: show functions returning record types and use of ROWS FROMBruce Momjian2020-10-051-1/+29
| | | | | | | | | | | | | Previously it was unclear exactly how ROWS FROM behaved and how to cast the data types of columns returned by FROM functions. Also document that only non-OUT record functions can have their columns cast to data types. Reported-by: guyren@gmail.com Discussion: https://postgr.es/m/158638264419.662.2482095087061084020@wrigleys.postgresql.org Backpatch-through: 9.5
* Fix two latent(?) bugs in equivclass.c.Tom Lane2020-10-051-6/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | get_eclass_for_sort_expr() computes expr_relids and nullable_relids early on, even though they won't be needed unless we make a new EquivalenceClass, which we often don't. Aside from the probably-minor inefficiency, there's a memory management problem: these bitmapsets will be built in the caller's context, leading to dangling pointers if that is shorter-lived than root->planner_cxt. This would be a live bug if get_eclass_for_sort_expr() could be called with create_it = true during GEQO join planning. So far as I can find, the core code never does that, but it's hard to be sure that no extensions do, especially since the comments make it clear that that's supposed to be a supported case. Fix by not computing these values until we've switched into planner_cxt to build the new EquivalenceClass. generate_join_implied_equalities() uses inner_rel->relids to look up relevant eclasses, but it ought to be using nominal_inner_relids. This is presently harmless because a child RelOptInfo will always have exactly the same eclass_indexes as its topmost parent; but that might not be true forever, and anyway it makes the code confusing. The first of these is old (introduced by me in f3b3b8d5b), so back-patch to all supported branches. The second only dates to v13, but we might as well back-patch it to keep the code looking similar across branches. Discussion: https://postgr.es/m/1508010.1601832581@sss.pgh.pa.us
* doc: libpq connection options can override command-line flagsBruce Momjian2020-10-0210-45/+50
| | | | | | | | Reported-by: Alexander Lakhin Discussion: https://postgr.es/m/16486-b9c93d71c02c4907@postgresql.org Backpatch-through: 9.5
* doc: clarify the use of ssh port forwardingBruce Momjian2020-10-021-17/+22
| | | | | | | | Reported-by: karimelghazouly@gmail.com Discussion: https://postgr.es/m/159854511172.24991.4373145230066586863@wrigleys.postgresql.org Backpatch-through: 9.5
* Fix handling of BC years in to_date/to_timestamp.Tom Lane2020-09-304-4/+67
| | | | | | | | | | | | | | | | | | | | | | | Previously, a conversion such as to_date('-44-02-01','YYYY-MM-DD') would result in '0045-02-01 BC', as the code attempted to interpret the negative year as BC, but failed to apply the correction needed for our internal handling of BC years. Fix the off-by-one problem. Also, arrange for the combination of a negative year and an explicit "BC" marker to cancel out and produce AD. This is how the negative-century case works, so it seems sane to do likewise. Continue to read "year 0000" as 1 BC. Oracle would throw an error, but we've accepted that case for a long time so I'm hesitant to change it in a back-patch. Per bug #16419 from Saeed Hubaishan. Back-patch to all supported branches. Dar Alathar-Yemen and Tom Lane Discussion: https://postgr.es/m/16419-d8d9db0a7553f01b@postgresql.org
* Archive timeline history files in standby if archive_mode is set to "always".Fujii Masao2020-09-292-1/+11
| | | | | | | | | | | | | | | | | | | | Previously the standby server didn't archive timeline history files streamed from the primary even when archive_mode is set to "always", while it archives the streamed WAL files. This could cause the PITR to fail because there was no required timeline history file in the archive. The cause of this issue was that walreceiver didn't mark those files as ready for archiving. This commit makes walreceiver mark those streamed timeline history files as ready for archiving if archive_mode=always. Then the archiver process archives the marked timeline history files. Back-patch to all supported versions. Reported-by: Grigory Smolkin Author: Grigory Smolkin, Fujii Masao Reviewed-by: David Zhang, Anastasia Lubennikova Discussion: https://postgr.es/m/54b059d4-2b48-13a4-6f43-95a087c92367@postgrespro.ru
* Revise RelationBuildRowSecurity() to avoid memory leaks.Tom Lane2020-09-261-109/+95
| | | | | | | | | | | | | | | | | | | | | | | | | | | This function leaked some memory while loading qual clauses for an RLS policy. While ordinarily negligible, that could build up in some repeated-reload cases, as reported by Konstantin Knizhnik. We can improve matters by borrowing the coding long used in RelationBuildRuleLock: build stringToNode's result directly in the target context, and remember to explicitly pfree the input string. This patch by no means completely guarantees zero leaks within this function, since we have no real guarantee that the catalog- reading subroutines it calls don't leak anything. However, practical tests suggest that this is enough to resolve the issue. In any case, any remaining leaks are similar to those risked by RelationBuildRuleLock and other relcache-loading subroutines. If we need to fix them, we should adopt a more global approach such as that used by the RECOVER_RELATION_BUILD_MEMORY hack. While here, let's remove the need for an expensive PG_TRY block by using MemoryContextSetParent to reparent an initially-short-lived context for the RLS data. Back-patch to all supported branches. Discussion: https://postgr.es/m/21356c12-8917-8249-b35f-1c447231922b@postgrespro.ru
* Fix handling of -d "connection string" in pg_dump/pg_restore.Tom Lane2020-09-246-296/+131
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Parallel pg_dump failed if its -d parameter was a connection string containing any essential information other than host, port, or username. The same was true for pg_restore with --create. The reason is that these scenarios failed to preserve the connection string from the command line; the code felt free to replace that with just the database name when reconnecting from a pg_dump parallel worker or after creating the target database. By chance, parallel pg_restore did not suffer this defect, as long as you didn't say --create. In practice it seems that the error would be obvious only if the connstring included essential, non-default SSL or GSS parameters. This may explain why it took us so long to notice. (It also makes it very difficult to craft a regression test case illustrating the problem, since the test would fail in builds without those options.) Fix by refactoring so that ConnectDatabase always receives all the relevant options directly from the command line, rather than reconstructed values. Inject a different database name, when necessary, by relying on libpq's rules for handling multiple "dbname" parameters. While here, let's get rid of the essentially duplicate _connectDB function, as well as some obsolete nearby cruft. Per bug #16604 from Zsolt Ero. Back-patch to all supported branches. Discussion: https://postgr.es/m/16604-933f4b8791227b15@postgresql.org