aboutsummaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAge
* Fix up usage of krb_server_keyfile GUC parameter.Tom Lane2020-12-30
| | | | | | | | | | | | | | | | | | | | | | | | | | | secure_open_gssapi() installed the krb_server_keyfile setting as KRB5_KTNAME unconditionally, so long as it's not empty. However, pg_GSS_recvauth() only installed it if KRB5_KTNAME wasn't set already, leading to a troubling inconsistency: in theory, clients could see different sets of server principal names depending on whether they use GSSAPI encryption. Always using krb_server_keyfile seems like the right thing, so make both places do that. Also fix up secure_open_gssapi()'s lack of a check for setenv() failure --- it's unlikely, surely, but security-critical actions are no place to be sloppy. Also improve the associated documentation. This patch does nothing about secure_open_gssapi()'s use of setenv(), and indeed causes pg_GSS_recvauth() to use it too. That's nominally against project portability rules, but since this code is only built with --with-gssapi, I do not feel a need to do something about this in the back branches. A fix will be forthcoming for HEAD though. Back-patch to v12 where GSSAPI encryption was introduced. The dubious behavior in pg_GSS_recvauth() goes back further, but it didn't have anything to be inconsistent with, so let it be. Discussion: https://postgr.es/m/2187460.1609263156@sss.pgh.pa.us
* In pg_upgrade cross-version test, handle lack of oldstyle_length().Noah Misch2020-12-30
| | | | | This suffices for testing v12 -> v13; some other version pairs need more changes. Back-patch to v10, which removed the function.
* doc: Improve some grammar and sentencesMichael Paquier2020-12-29
| | | | | | | | | | 90fbf7c has taken care of that for HEAD. This includes the portion of the fixes that applies to the documentation, where needed depending on the branch. Author: Justin Pryzby Discussion: https://postgr.es/m/20201227202604.GC26311@telsasoft.com Backpatch-through: 9.5
* Improve log messages related to pg_hba.conf not matching a connection.Tom Lane2020-12-28
| | | | | | | | | | Include details on whether GSS encryption has been activated; since we added "hostgssenc" type HBA entries, that's relevant info. Kyotaro Horiguchi and Tom Lane. Back-patch to v12 where GSS encryption was introduced. Discussion: https://postgr.es/m/e5b0b6ed05764324a2f3fe7acfc766d5@smhi.se
* Fix assorted issues in backend's GSSAPI encryption support.Tom Lane2020-12-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Unrecoverable errors detected by GSSAPI encryption can't just be reported with elog(ERROR) or elog(FATAL), because attempting to send the error report to the client is likely to lead to infinite recursion or loss of protocol sync. Instead make this code do what the SSL encryption code has long done, which is to just report any such failure to the server log (with elevel COMMERROR), then pretend we've lost the connection by returning errno = ECONNRESET. Along the way, fix confusion about whether message translation is done by pg_GSS_error() or its callers (the latter should do it), and make the backend version of that function work more like the frontend version. Avoid allocating the port->gss struct until it's needed; we surely don't need to allocate it in the postmaster. Improve logging of "connection authorized" messages with GSS enabled. (As part of this, I back-patched the code changes from dc11f31a1.) Make BackendStatusShmemSize() account for the GSS-related space that will be allocated by CreateSharedBackendStatus(). This omission could possibly cause out-of-shared-memory problems with very high max_connections settings. Remove arbitrary, pointless restriction that only GSS authentication can be used on a GSS-encrypted connection. Improve documentation; notably, document the fact that libpq now prefers GSS encryption over SSL encryption if both are possible. Per report from Mikael Gustavsson. Back-patch to v12 where this code was introduced. Discussion: https://postgr.es/m/e5b0b6ed05764324a2f3fe7acfc766d5@smhi.se
* Fix bugs in libpq's GSSAPI encryption support.Tom Lane2020-12-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The critical issue fixed here is that if a GSSAPI-encrypted connection is successfully made, pqsecure_open_gss() cleared conn->allow_ssl_try, as an admittedly-hacky way of preventing us from then trying to tunnel SSL encryption over the already-encrypted connection. The problem with that is that if we abandon the GSSAPI connection because of a failure during authentication, we would not attempt SSL encryption in the next try with the same server. This can lead to unexpected connection failure, or silently getting a non-encrypted connection where an encrypted one is expected. Fortunately, we'd only manage to make a GSSAPI-encrypted connection if both client and server hold valid tickets in the same Kerberos infrastructure, which is a relatively uncommon environment. Nonetheless this is a very nasty bug with potential security consequences. To fix, don't reset the flag, instead adding a check for conn->gssenc being already true when deciding whether to try to initiate SSL. While here, fix some lesser issues in libpq's GSSAPI code: * Use the need_new_connection stanza when dropping an attempted GSSAPI connection, instead of partially duplicating that code. The consequences of this are pretty minor: AFAICS it could only lead to auth_req_received or password_needed remaining set when they shouldn't, which is not too harmful. * Fix pg_GSS_error() to not repeat the "mprefix" it's given multiple times, and to notice any failure return from gss_display_status(). * Avoid gratuitous dependency on NI_MAXHOST in pg_GSS_load_servicename(). Per report from Mikael Gustavsson. Back-patch to v12 where this code was introduced. Discussion: https://postgr.es/m/e5b0b6ed05764324a2f3fe7acfc766d5@smhi.se
* Expose the default for channel_binding in PQconndefaults().Tom Lane2020-12-28
| | | | | | | | | If there's a static default value for a connection option, it should be shown in the PQconninfoOptions array. Daniele Varrazzo Discussion: https://postgr.es/m/CA+mi_8Zo8Rgn7p+6ZRY7QdDu+23ukT9AvoHNyPbgKACxwgGhZA@mail.gmail.com
* Further fix thinko in plpgsql memory leak fix.Tom Lane2020-12-28
| | | | | | | | | | There's a second call of get_eval_mcontext() that should also be get_stmt_mcontext(). This is actually dead code, since no interesting allocations happen before switching back to the original context, but we should keep it in sync with the other call to forestall possible future bugs. Discussion: https://postgr.es/m/f075f7be-c654-9aa8-3ffc-e9214622f02a@enterprisedb.com
* Fix thinko in plpgsql memory leak fix.Tom Lane2020-12-28
| | | | | | | | | | | | | | | | Commit a6b1f5365 intended to place the transient "target" list of a CALL statement in the function's statement-lifespan context, but I fat-fingered that and used get_eval_mcontext() instead of get_stmt_mcontext(). The eval_mcontext belongs to the "simple expression" infrastructure, which is destroyed at transaction end. The net effect is that a CALL in a procedure to another procedure that has OUT or INOUT parameters would fail if the called procedure did a COMMIT. Per report from Peter Eisentraut. Back-patch to v11, like the prior patch. Discussion: https://postgr.es/m/f075f7be-c654-9aa8-3ffc-e9214622f02a@enterprisedb.com
* Fix inconsistent code with shared invalidations of snapshotsMichael Paquier2020-12-28
| | | | | | | | | | | | The code in charge of processing a single invalidation message has been using since 568d413 the structure for relation mapping messages. This had fortunately no consequence as both locate the database ID at the same location, but it could become a problem in the future if this area of the code changes. Author: Konstantin Knizhnik Discussion: https://postgr.es/m/8044c223-4d3a-2cdb-42bf-29940840ce94@postgrespro.ru Backpatch-through: 9.5
* postgres_fdw: Fix connection leak.Fujii Masao2020-12-28
| | | | | | | | | | | | | | | | | | | | | In postgres_fdw, the cached connections to foreign servers will not be closed until the local session exits if the user mappings or foreign servers that those connections depend on are dropped. Those connections can be leaked. To fix that connection leak issue, after a change to a pg_foreign_server or pg_user_mapping catalog entry, this commit makes postgres_fdw close the connections depending on that entry immediately if current transaction has not used those connections yet. Otherwise, mark those connections as invalid and then close them at the end of current transaction, since they cannot be closed in the midst of the transaction using them. Closed connections will be remade at the next opportunity if necessary. Back-patch to all supported branches. Author: Bharath Rupireddy Reviewed-by: Zhihong Yu, Zhijie Hou, Fujii Masao Discussion: https://postgr.es/m/CALj2ACVNcGH_6qLY-4_tXz8JLvA+4yeBThRfxMz7Oxbk1aHcpQ@mail.gmail.com
* Second attempt to stabilize 05c02589.Jeff Davis2020-12-27
| | | | | | Removing the EXPLAIN test to stabilize the buildfarm. The execution test should still be effective to catch the bug even if the plan is slightly different on different platforms.
* Stabilize test introduced in 05c02589, per buildfarm.Jeff Davis2020-12-27
| | | | | | In passing, make the capitalization match the rest of the file. Reported-by: Tom Lane
* Fix bug #16784 in Disk-based Hash Aggregation.Jeff Davis2020-12-26
| | | | | | | | | | | | | | | | | Before processing tuples, agg_refill_hash_table() was setting all pergroup pointers to NULL to signal to advance_aggregates() that it should not attempt to advance groups that had spilled. The problem was that it also set the pergroups for sorted grouping sets to NULL, which caused rescanning to fail. Instead, change agg_refill_hash_table() to only set the pergroups for hashed grouping sets to NULL; and when compiling the expression, pass doSort=false. Reported-by: Alexander Lakhin Discussion: https://postgr.es/m/16784-7ff169bf2c3d1588%40postgresql.org Backpatch-through: 13
* Invalidate acl.c caches when pg_authid changes.Noah Misch2020-12-25
| | | | | | | | | | This makes existing sessions reflect "ALTER ROLE ... [NO]INHERIT" as quickly as they have been reflecting "GRANT role_name". Back-patch to 9.5 (all supported versions). Reviewed by Nathan Bossart. Discussion: https://postgr.es/m/20201221095028.GB3777719@rfd.leadboat.com
* Avoid time-of-day-dependent failure in log rotation test.Tom Lane2020-12-24
| | | | | | | | | | | | | | | | | | | | | | | Buildfarm members pogona and petalura have shown a failure when pg_ctl/t/004_logrotate.pl starts just before local midnight. The default rotate-at-midnight behavior occurs just before the Perl script examines current_logfiles, so it figures that the rotation it's already requested has occurred ... but in reality, that rotation happens just after it looks, so the expected new log data goes into a different file than the one it's examining. In HEAD, src/test/kerberos/t/001_auth.pl has acquired similar code that evidently has a related failure mode. Besides being quite new, few buildfarm critters run that test, so it's unsurprising that we've not yet seen a failure there. Fix both cases by setting log_rotation_age = 0 so that no time-based rotation can occur. Also absorb 004_logrotate.pl's decision to set lc_messages = 'C' into the kerberos test, in hopes that it will work in non-English prevailing locales. Report: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=pogona&dt=2020-12-24%2022%3A10%3A04 Report: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=petalura&dt=2020-02-01%2022%3A20%3A04
* Fix race condition between shutdown and unstarted background workers.Tom Lane2020-12-24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If a database shutdown (smart or fast) is commanded between the time some process decides to request a new background worker and the time that the postmaster can launch that worker, then nothing happens because the postmaster won't launch any bgworkers once it's exited PM_RUN state. This is fine ... unless the requesting process is waiting for that worker to finish (or even for it to start); in that case the requestor is stuck, and only manual intervention will get us to the point of being able to shut down. To fix, cancel pending requests for workers when the postmaster sends shutdown (SIGTERM) signals, and similarly cancel any new requests that arrive after that point. (We can optimize things slightly by only doing the cancellation for workers that have waiters.) To fit within the existing bgworker APIs, the "cancel" is made to look like the worker was started and immediately stopped, causing deregistration of the bgworker entry. Waiting processes would have to deal with premature worker exit anyway, so this should introduce no bugs that weren't there before. We do have a side effect that registration records for restartable bgworkers might disappear when theoretically they should have remained in place; but since we're shutting down, that shouldn't matter. Back-patch to v10. There might be value in putting this into 9.6 as well, but the management of bgworkers is a bit different there (notably see 8ff518699) and I'm not convinced it's worth the effort to validate the patch for that branch. Discussion: https://postgr.es/m/661570.1608673226@sss.pgh.pa.us
* docs: document which server-side languages can create procsBruce Momjian2020-12-23
| | | | | | | | | | This was missed when the feature was added. Reported-by: Daniel Westermann Discussion: https://postgr.es/m/160624532969.25818.4767632047905006142@wrigleys.postgresql.org Backpatch-through: 11
* Fix portability issues with parsing of recovery_target_xidMichael Paquier2020-12-23
| | | | | | | | | | | | | | | | | | | | | | | | | | | The parsing of this parameter has been using strtoul(), which is not portable across platforms. On most Unix platforms, unsigned long has a size of 64 bits, while on Windows it is 32 bits. It is common in recovery scenarios to rely on the output of txid_current() or even the newer pg_current_xact_id() to get a transaction ID for setting up recovery_target_xid. The value returned by those functions includes the epoch in the computed result, which would cause strtoul() to fail where unsigned long has a size of 32 bits once the epoch is incremented. WAL records and 2PC data include only information about 32-bit XIDs and it is not possible to have XIDs across more than one epoch, so discarding the high bits from the transaction ID set has no impact on recovery. On the contrary, the use of strtoul() prevents a consistent behavior across platforms depending on the size of unsigned long. This commit changes the parsing of recovery_target_xid to use pg_strtouint64() instead, available down to 9.6. There is one TAP test stressing recovery with recovery_target_xid, where a tweak based on pg_reset{xlog,wal} is added to bump the XID epoch so as this change gets tested, as per an idea from Alexander Lakhin. Reported-by: Alexander Lakhin Discussion: https://postgr.es/m/16780-107fd0c0385b1035@postgresql.org Backpatch-through: 9.6
* Improve autoprewarm's handling of early-shutdown scenarios.Tom Lane2020-12-22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Bad things happen if the DBA issues "pg_ctl stop -m fast" before autoprewarm finishes loading its list of blocks to prewarm. The current worker process successfully terminates early, but (if this wasn't the last database with blocks to prewarm) the leader process will just try to launch another worker for the next database. Since the postmaster is now in PM_WAIT_BACKENDS state, it ignores the launch request, and the leader just sits until it's killed manually. This is mostly the fault of our half-baked design for launching background workers, but a proper fix for that is likely to be too invasive to be back-patchable. To ameliorate the situation, fix apw_load_buffers() to check whether SIGTERM has arrived just before trying to launch another worker. That leaves us with only a very narrow window in each worker launch where SIGTERM could occur between the launch request and successful worker start. Another issue is that if the leader process does manage to exit, it unconditionally rewrites autoprewarm.blocks with only the blocks currently in shared buffers, thus forgetting any blocks that we hadn't reached yet while prewarming. This seems quite unhelpful, since the next database start will then not have the expected prewarming benefit. Fix it to not modify the file if we shut down before the initial load attempt is complete. Per bug #16785 from John Thompson. Back-patch to v11 where the autoprewarm code was introduced. Discussion: https://postgr.es/m/16785-c0207d8c67fb5f25@postgresql.org
* Improve find_em_expr_usable_for_sorting_rel commentTomas Vondra2020-12-22
| | | | | | | | | | | Clarify the relationship between find_em_expr_usable_for_sorting_rel and prepare_sort_from_pathkeys, i.e. what restrictions need to be shared between those two places. Author: James Coleman Reviewed-by: Tomas Vondra Backpatch-through: 13 Discussion: https://postgr.es/m/CAAaqYe8cK3g5CfLC4w7bs%3DhC0mSksZC%3DH5M8LSchj5e5OxpTAg%40mail.gmail.com
* Don't search for volatile expr in find_em_expr_usable_for_sorting_relTomas Vondra2020-12-21
| | | | | | | | | | | | While prepare_sort_from_pathkeys has to be concerned about matching up a volatile expression to the proper tlist entry, we don't need to do that in find_em_expr_usable_for_sorting_rel becausee such a sort will have to be postponed anyway. Author: James Coleman Reviewed-by: Tomas Vondra Backpatch-through: 13 Discussion: https://postgr.es/m/CAAaqYe8cK3g5CfLC4w7bs%3DhC0mSksZC%3DH5M8LSchj5e5OxpTAg%40mail.gmail.com
* Disallow SRFs when considering sorts below Gather MergeTomas Vondra2020-12-21
| | | | | | | | | | | | | | | | | While we do allow SRFs in ORDER BY, scan/join processing should not consider such cases - such sorts should only happen via final Sort atop a ProjectSet. So make sure we don't try adding such sorts below Gather Merge, just like we do for expressions that are volatile and/or not parallel safe. Backpatch to PostgreSQL 13, where this code was introduced as part of the Incremental Sort patch. Author: James Coleman Reviewed-by: Tomas Vondra Backpatch-through: 13 Discussion: https://postgr.es/m/CAAaqYe8cK3g5CfLC4w7bs=hC0mSksZC=H5M8LSchj5e5OxpTAg@mail.gmail.com Discussion: https://postgr.es/m/295524.1606246314%40sss.pgh.pa.us
* Remove "invalid concatenation of jsonb objects" error case.Tom Lane2020-12-21
| | | | | | | | | | | | | | | The jsonb || jsonb operator arbitrarily rejected certain combinations of scalar and non-scalar inputs, while being willing to concatenate other combinations. This was of course quite undocumented. Rather than trying to document it, let's just remove the restriction, creating a uniform rule that unless we are handling an object-to-object concatenation, non-array inputs are converted to one-element arrays, resulting in an array-to-array concatenation. (This does not change the behavior for any case that didn't throw an error before.) Per complaint from Joel Jacobson. Back-patch to all supported branches. Discussion: https://postgr.es/m/163099.1608312033@sss.pgh.pa.us
* Check parallel safety in generate_useful_gather_pathsTomas Vondra2020-12-21
| | | | | | | | | | | | | | | | | | Commit ebb7ae839d ensured we ignore pathkeys with volatile expressions when considering adding a sort below a Gather Merge. Turns out we need to care about parallel safety of the pathkeys too, otherwise we might try sorting e.g. on results of a correlated subquery (as demonstrated by a report from Luis Roberto). Initial investigation by Tom Lane, patch by James Coleman. Backpatch to 13, where the code was instroduced (as part of Incremental Sort). Reported-by: Luis Roberto Author: James Coleman Reviewed-by: Tomas Vondra Backpatch-through: 13 Discussion: https://postgr.es/m/622580997.37108180.1604080457319.JavaMail.zimbra%40siscobra.com.br Discussion: https://postgr.es/m/CAAaqYe8cK3g5CfLC4w7bs=hC0mSksZC=H5M8LSchj5e5OxpTAg@mail.gmail.com
* Consider unsorted paths in generate_useful_gather_pathsTomas Vondra2020-12-21
| | | | | | | | | | | | | | generate_useful_gather_paths used to skip unsorted paths (without any pathkeys), but that is unnecessary - the later code actually can handle such paths just fine by adding a Sort node. This is clearly a thinko, preventing construction of useful plans. Backpatch to 13, where Incremental Sort was introduced. Author: James Coleman Reviewed-by: Tomas Vondra Backpatch-through: 13 Discussion: https://postgr.es/m/CAAaqYe8cK3g5CfLC4w7bs=hC0mSksZC=H5M8LSchj5e5OxpTAg@mail.gmail.com
* Doc: fix description of how to use src/tutorial files.Tom Lane2020-12-20
| | | | | | | | | | | | | The separate "cd" command before invoking psql made sense (or at least I thought so) when it was added in commit ed1939332. But 4e3a61635 removed the supporting text that explained when to use it, making it just confusing. So drop it. Also switch from four-dot to three-dot filler for the unsupplied part of the path, since at least one person has read the four-dot filler as a typo for "../..". And fix these/those inconsistency. Discussion: https://postgr.es/m/160837647714.673.5195186835607800484@wrigleys.postgresql.org
* Doc: improve description of pgbench script weights.Tom Lane2020-12-20
| | | | | | | | | Point out the workaround to be used if you want to write a script file name that includes "@". Clean up the text a little. Fabien Coelho, additional wordsmithing by me Discussion: https://postgr.es/m/1c4e81550d214741827a03292222db8d@G08CNEXMBPEKD06.g08.fujitsu.local
* Avoid memcpy() with same source and destination during relmapper init.Tom Lane2020-12-18
| | | | | | | | | | | | | | | | | A narrow reading of the C standard says that memcpy(x,x,n) is undefined, although it's hard to envision an implementation that would really misbehave. However, analysis tools such as valgrind might whine about this; accordingly, let's band-aid relmapper.c to not do it. See also 5b630501e, d3f4e8a8a, ad7b48ea0, and other similar fixes. Apparently, none of those folk tried valgrinding initdb? This has been like this for long enough that I'm surprised it hasn't been reported before. Back-patch, just in case anybody wants to use a back branch on a platform that complains about this; we back-patched those earlier fixes too. Discussion: https://postgr.es/m/161790.1608310142@sss.pgh.pa.us
* doc: Fix explanation related to pg_shmem_allocationsMichael Paquier2020-12-16
| | | | | | | | | Offsets are shown as NULL only for anonymous allocations. Author: Benoit Lobréau Reviewed-by: Kyotaro Horiguchi Discussion: https://postgr.es/m/CAPE8EZ5Lnoyqoz7aZpvQM0E8sW+hw+k6G2NULe+m4arFRrA1aA@mail.gmail.com Backpatch-through: 13
* doc: clarify COPY TO for partitioning/inheritanceBruce Momjian2020-12-15
| | | | | | | | | | | It was not clear how COPY TO behaved with partitioning/inheritance because the paragraphs were so far apart. Also reword to simplify. Discussion: https://postgr.es/m/20201203211723.GR24052@telsasoft.com Author: Justin Pryzby Backpatch-through: 10
* Revert "Cannot use WL_SOCKET_WRITEABLE without WL_SOCKET_READABLE."Jeff Davis2020-12-14
| | | | | | | | | | | | | This reverts commit 3a9e64aa0d96c8ffb6c682b082d0f72b1d373327. Commit 4bad60e3 fixed the root of the problem that 3a9e64aa worked around. This enables proper pipelining of commands after terminating replication, eliminating an undocumented limitation. Discussion: https://postgr.es/m/3d57bc29-4459-578b-79cb-7641baf53c57%40iki.fi Backpatch-through: 9.5
* initdb: complete getopt_long alphabetizationBruce Momjian2020-12-12
| | | | Backpatch-through: 9.5
* initdb: properly alphabetize getopt_long options in C stringBruce Momjian2020-12-12
| | | | Backpatch-through: 9.5
* Teach contain_leaked_vars that assignment SubscriptingRefs are leaky.Tom Lane2020-12-08
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | array_get_element and array_get_slice qualify as leakproof, since they will silently return NULL for bogus subscripts. But array_set_element and array_set_slice throw errors for such cases, making them clearly not leakproof. contain_leaked_vars was evidently written with only the former case in mind, as it gave the wrong answer for assignment SubscriptingRefs (nee ArrayRefs). This would be a live security bug, were it not that assignment SubscriptingRefs can only occur in INSERT and UPDATE target lists, while we only care about leakproofness for qual expressions; so the wrong answer can't occur in practice. Still, that's a rather shaky answer for a security-related question; and maybe in future somebody will want to ask about leakproofness of a tlist. So it seems wise to fix and even back-patch this correction. (We would need some change here anyway for the upcoming generic-subscripting patch, since extensions might make different tradeoffs about whether to throw errors. Commit 558d77f20 attempted to lay groundwork for that by asking check_functions_in_node whether a SubscriptingRef contains leaky functions; but that idea fails now that the implementation methods of a SubscriptingRef are not SQL-visible functions that could be marked leakproof or not.) Back-patch to 9.6. While 9.5 has the same issue, the code's a bit different. It seems quite unlikely that we'd introduce any actual bug in the short time 9.5 has left to live, so the work/risk/reward balance isn't attractive for changing 9.5. Discussion: https://postgr.es/m/3143742.1607368115@sss.pgh.pa.us
* Doc: clarify that CREATE TABLE discards redundant unique constraints.Tom Lane2020-12-08
| | | | | | | | | | | | | | | The SQL standard says that redundant unique constraints are disallowed, but we long ago decided that throwing an error would be too user-unfriendly, so we just drop redundant ones. The docs weren't very clear about that though, as this behavior was only explained for PRIMARY KEY vs UNIQUE, not UNIQUE vs UNIQUE. While here, I couldn't resist doing some copy-editing and markup-fixing on the adjacent text about INCLUDE options. Per bug #16767 from Matthias vd Meent. Discussion: https://postgr.es/m/16767-1714a2056ca516d0@postgresql.org
* Doc: explain that the string types can't store \0 (ASCII NUL).Tom Lane2020-12-08
| | | | | | | | | | This restriction was mentioned in connection with string literals, but it wasn't made clear that it's a general restriction not just a syntactic limitation in query strings. Per unsigned documentation comment. Discussion: https://postgr.es/m/160720552914.710.16625261471128631268@wrigleys.postgresql.org
* pgcrypto: Detect errors with EVP calls from OpenSSLMichael Paquier2020-12-08
| | | | | | | | | | | | | | | | | | | | | | | The following routines are called within pgcrypto when handling digests but there were no checks for failures: - EVP_MD_CTX_size (can fail with -1 as of 3.0.0) - EVP_MD_CTX_block_size (can fail with -1 as of 3.0.0) - EVP_DigestInit_ex - EVP_DigestUpdate - EVP_DigestFinal_ex A set of elog(ERROR) is added by this commit to detect such failures, that should never happen except in the event of a processing failure internal to OpenSSL. Note that it would be possible to use ERR_reason_error_string() to get more context about such errors, but these refer mainly to the internals of OpenSSL, so it is not really obvious how useful that would be. This is left out for simplicity. Per report from Coverity. Thanks to Tom Lane for the discussion. Backpatch-through: 9.5
* jit: Correct parameter type for generated expression evaluation functions.Andres Freund2020-12-07
| | | | | | | | | | | | | clang only uses the 'i1' type for scalar booleans, not for pointers to booleans (as the pointer might be pointing into a larger memory allocation). Therefore a pointer-to-bool needs to the "storage" boolean. There's no known case of wrong code generation due to this, but it seems quite possible that it could cause problems (see e.g. 72559438f92). Author: Andres Freund Discussion: https://postgr.es/m/20201207212142.wz5tnbk2jsaqzogb@alap3.anarazel.de Backpatch: 11-, where jit support was added
* jit: configure: Explicitly reference 'native' component.Andres Freund2020-12-07
| | | | | | | | | | | | | Until recently 'native' was implicitly included via 'orcjit', but a change included in LLVM 11 (not yet released) removed a number of such indirect component references. Reported-By: Fabien COELHO <coelho@cri.ensmp.fr> Reported-By: Andres Freund <andres@anarazel.de> Reported-By: Thomas Munro <thomas.munro@gmail.com> Author: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/20201201064949.mex6kvi2kygby3ni@alap3.anarazel.de Backpatch: 11-, where jit support was added
* backpatch "jit: Add support for LLVM 12."Andres Freund2020-12-07
| | | | | | | | | As there haven't been problem on the buildfarm due to this change, backpatch 6c57f2ed16e now. Author: Andres Freund Discussion: https://postgr.es/m/20201016011244.pmyvr3ee2gbzplq4@alap3.anarazel.de Backpatch: 11-, where jit support was added
* Fix more race conditions in the newly-added pg_rewind test.Heikki Linnakangas2020-12-07
| | | | | | | | | | | | | | | | | | | | | | | | pg_rewind looks at the control file to check what timeline a server is on. But promotion doesn't immediately write a checkpoint, it merely writes an end-of-recovery WAL record. If pg_rewind runs immediately after promotion, before the checkpoint has completed, it will think think that the server is still on the earlier timeline. We ran into this issue a long time ago already, see commit 484a848a73f. It's a bit bogus that pg_rewind doesn't determine the timeline correctly until the end-of-recovery checkpoint has completed. We probably should fix that. But for now work around it by waiting for the checkpoint to complete before running pg_rewind, like we did in commit 484a848a73f. In the passing, tidy up the new test a little bit. Rerder the INSERTs so that the comments make more sense, remove a spurious CHECKPOINT call after pg_rewind has already run, and add --debug option, so that if this fails again, we'll have more data. Per buildfarm failure at https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=rorqual&dt=2020-12-06%2018%3A32%3A19&stg=pg_rewind-check. Backpatch to all supported versions. Discussion: https://www.postgresql.org/message-id/1713707e-e318-761c-d287-5b6a4aa807e8@iki.fi
* Fix missed step in removal of useless RESULT RTEs in the planner.Tom Lane2020-12-05
| | | | | | | | | | | | Commit 4be058fe9 forgot that the append_rel_list would already be populated at the time we remove useless result RTEs, and it might contain PlaceHolderVars that need to be adjusted like the ones in the main parse tree. This could lead to "no relation entry for relid N" failures later on, when the planner tries to do something with an unadjusted PHV. Per report from Tom Ellis. Back-patch to v12 where the bug came in. Discussion: https://postgr.es/m/20201205173056.GF30712@cloudinit-builder
* Fix race conditions in newly-added test.Heikki Linnakangas2020-12-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Buildfarm has been failing sporadically on the new test. I was able to reproduce this by adding a random 0-10 s delay in the walreceiver, just before it connects to the primary. There's a race condition where node_3 is promoted before it has fully caught up with node_1, leading to diverged timelines. When node_1 is later reconfigured as standby following node_3, it fails to catch up: LOG: primary server contains no more WAL on requested timeline 1 LOG: new timeline 2 forked off current database system timeline 1 before current recovery point 0/30000A0 That's the situation where you'd need to use pg_rewind, but in this case it happens already when we are just setting up the actual pg_rewind scenario we want to test, so change the test so that it waits until node_3 is connected and fully caught up before promoting it, so that you get a clean, controlled failover. Also rewrite some of the comments, for clarity. The existing comments detailed what each step in the test did, but didn't give a good overview of the situation the steps were trying to create. For reasons I don't understand, the test setup had to be written slightly differently in 9.6 and 9.5 than in later versions. The 9.5/9.6 version needed node 1 to be reinitialized from backup, whereas in later versions it could be shut down and reconfigured to be a standby. But even 9.5 should support "clean switchover", where primary makes sure that pending WAL is replicated to standby on shutdown. It would be nice to figure out what's going on there, but that's independent of pg_rewind and the scenario that this test tests. Discussion: https://www.postgresql.org/message-id/b0a3b95b-82d2-6089-6892-40570f8c5e60%40iki.fi
* doc: remove unnecessary blank before command option textBruce Momjian2020-12-03
| | | | Backpatch-through: 11
* docs: list single-letter options first in command-line summaryBruce Momjian2020-12-03
| | | | | | | | | In a few places, the long-version options were listed before the single-letter ones in the command summary of a few commands. This didn't match other commands, and didn't match the option ordering later in the same reference page. Backpatch-through: 9.5
* Fix pg_rewind bugs when rewinding a standby server.Heikki Linnakangas2020-12-03
| | | | | | | | | | | | | If the target is a standby server, its WAL doesn't end at the last checkpoint record, but at minRecoveryPoint. We must scan all the WAL from the last common checkpoint all the way up to minRecoveryPoint for modified pages, and also consider that portion when determining whether the server needs rewinding. Backpatch to all supported versions. Author: Ian Barwick and me Discussion: https://www.postgresql.org/message-id/CABvVfJU-LDWvoz4-Yow3Ay5LZYTuPD7eSjjE4kGyNZpXC6FrVQ%40mail.gmail.com
* pg_checksums: data_checksum_version is unsigned so use %u not %dBruce Momjian2020-12-01
| | | | | | | While the previous behavior didn't generate a warning, we might as well use an accurate *printf specification. Backpatch-through: 12
* Ensure that expandTableLikeClause() re-examines the same table.Tom Lane2020-12-01
| | | | | | | | | | | | | | | | | | | | | | As it stood, expandTableLikeClause() re-did the same relation_openrv call that transformTableLikeClause() had done. However there are scenarios where this would not find the same table as expected. We hold lock on the LIKE source table, so it can't be renamed or dropped, but another table could appear before it in the search path. This explains the odd behavior reported in bug #16758 when cloning a table as a temp table of the same name. This case worked as expected before commit 502898192 introduced the need to open the source table twice, so we should fix it. To make really sure we get the same table, let's re-open it by OID not name. That requires adding an OID field to struct TableLikeClause, which is a little nervous-making from an ABI standpoint, but as long as it's at the end I don't think there's any serious risk. Per bug #16758 from Marc Boeren. Like the previous patch, back-patch to all supported branches. Discussion: https://postgr.es/m/16758-840e84a6cfab276d@postgresql.org
* Avoid memcpy() with a NULL source pointer and count == 0Alvaro Herrera2020-12-01
| | | | | | | | | | | | | | | | When memcpy() is called on a pointer, the compiler is entitled to assume that the pointer is not null, which can lead to optimizing nearby code in potentially undesirable ways. We still want such optimizations (gcc's -fdelete-null-pointer-checks) in cases where they're valid. Related: commit 13bba02271dc. Backpatch to pg11, where this particular instance appeared. Reported-by: Ranier Vilela <ranier.vf@gmail.com> Reported-by: Zhihong Yu <zyu@yugabyte.com> Discussion: https://postgr.es/m/CAEudQApUndmQkr5fLrCKXQ7+ib44i7S+Kk93pyVThS85PnG3bQ@mail.gmail.com Discussion: https://postgr.es/m/CALNJ-vSdhwSM5f4tnNn1cdLHvXMVe_S+V3nR5GwNrmCPNB2VtQ@mail.gmail.com