- 19 Jan, 2021 1 commit
-
-
Amit Kapila authored
This is a leftover from commit 0926e96c. Changing this separately because this file is being modified for upcoming patch logical replication of 2PC. Author: Peter Smith Discussion: https://postgr.es/m/CAHut+Ps+EgG8KzcmAyAgBUi_vuTps6o9ZA8DG6SdnO0-YuOhPQ@mail.gmail.com
-
- 18 Jan, 2021 10 commits
-
-
Bruce Momjian authored
Backpatch-through: 10
-
Tom Lane authored
execCurrent.c's search_plan_tree() assumed that ForeignScanStates and CustomScanStates necessarily have a valid ss_currentRelation. This is demonstrably untrue for postgres_fdw's remote join and remote aggregation plans, and non-leaf custom scans might not have an identifiable scan relation either. Avoid crashing by ignoring such nodes when the field is null. This solution will lead to errors like 'cursor "foo" is not a simply updatable scan of table "bar"' in cases where maybe we could have allowed WHERE CURRENT OF to work. That's not an issue for postgres_fdw's usages, since joins or aggregations would render WHERE CURRENT OF invalid anyway. But an otherwise-transparent upper level custom scan node might find this annoying. When and if someone cares to expend work on such a scenario, we could invent a custom-scan-provider callback to determine what's safe. Report and patch by David Geier, commentary by me. It's been like this for awhile, so back-patch to all supported branches. Discussion: https://postgr.es/m/0253344d-9bdd-11c4-7f0d-d88c02cd7991@swarm64.com
-
Tom Lane authored
This is better style and more symmetrical with the other if-branch. This likely should have been included in 9de77b54 (which created the opportunity), but it was overlooked. Japin Li Discussion: https://postgr.es/m/MEYP282MB16699FA4A7CD57EB250E871FB6A40@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM
-
Tom Lane authored
We had bytea btrim() already, but for some reason not the other two. Joel Jacobson Discussion: https://postgr.es/m/d10cd5cd-a901-42f1-b832-763ac6f7ff3a@www.fastmail.com
-
Robert Haas authored
Previously, the per-barrier-type functions tasked with absorbing them were expected to always succeed and never throw an error. However, that's a bit inconvenient. Further study has revealed that there are realistic cases where it might not be possible to absorb a ProcSignalBarrier without terminating the transaction, or even the whole backend. Similarly, for some barrier types, there might be other reasons where it's not reasonably possible to absorb the barrier at certain points in the code, so provide a way for a per-barrier-type function to reject absorbing the barrier. Unfortunately, there's still no committed code making use of this infrastructure; hopefully, we'll get there. :-( Patch by me, reviewed by Andres Freund and Amul Sul. Discussion: http://postgr.es/m/20200908182005.xya7wetdh3pndzim@alap3.anarazel.de Discussion: http://postgr.es/m/CA+Tgmob56Pk1-5aTJdVPCWFHon7me4M96ENpGe9n_R4JUjjhZA@mail.gmail.com
-
Magnus Hagander authored
This was missed in 960869da Reported-By: Laurenz Albe Discussion: https://postgr.es/m/4f0aacc5fe1b4bfafa32b36ecd97469fae526a75.camel@cybertec.at
-
Heikki Linnakangas authored
Per Coverity. BuildIndexValueDescription() cannot actually return NULL in this instance, because it only returns NULL if the user doesn't have the required privileges, and this function can only be used by superuser. But better safe than sorry.
-
Peter Eisentraut authored
When certain parameters are changed on a physical replication primary, this is communicated to standbys using the XLOG_PARAMETER_CHANGE WAL record. The standby then checks whether its own settings are at least as big as the ones on the primary. If not, the standby shuts down with a fatal error. This patch changes this behavior for hot standbys to pause recovery at that point instead. That allows read traffic on the standby to continue while database administrators figure out next steps. When recovery is unpaused, the server shuts down (as before). The idea is to fix the parameters while recovery is paused and then restart when there is a maintenance window. Reviewed-by: Sergei Kornilov <sk@zsrv.org> Discussion: https://www.postgresql.org/message-id/flat/4ad69a4c-cc9b-0dfe-0352-8b1b0cd36c7b@2ndquadrant.com
-
Fujii Masao authored
This commit adds function postgres_fdw_get_connections() to return the foreign server names of all the open connections that postgres_fdw established from the local session to the foreign servers. This function also returns whether each connection is valid or not. This function is useful when checking all the open foreign server connections. If we found some connection to drop, from the result of function, probably we can explicitly close them by the function that upcoming commit will add. This commit bumps the version of postgres_fdw to 1.1 since it adds new function. Author: Bharath Rupireddy, tweaked by Fujii Masao Reviewed-by: Zhijie Hou, Alexey Kondratov, Zhihong Yu, Fujii Masao Discussion: https://postgr.es/m/2d5cb0b3-a6e8-9bbb-953f-879f47128faa@oss.nttdata.com
-
Michael Paquier authored
This continues the work done in b5913f61. All the options of those commands are changed to use hex values rather than enums to reduce the risk of compatibility bugs when introducing new options. Each option set is moved into a new structure that can be extended with more non-boolean options (this was already the case of VACUUM). The code of REINDEX is restructured so as manual REINDEX commands go through a single routine from utility.c, like VACUUM, to ease the allocation handling of option parameters when a command needs to go through multiple transactions. This can be used as a base infrastructure for future patches related to those commands, including reindex filtering and tablespace support. Per discussion with people mentioned below, as well as Alvaro Herrera and Peter Eisentraut. Author: Michael Paquier, Justin Pryzby Reviewed-by: Alexey Kondratov, Justin Pryzby Discussion: https://postgr.es/m/X8riynBLwxAD9uKk@paquier.xyz
-
- 17 Jan, 2021 7 commits
-
-
Heikki Linnakangas authored
The gist_page_items() function opened the index relation on first call and closed it on the last call. But there's no guarantee that the function is run to completion, leading to a relcache leak and warning at the end of the transaction. To fix, refactor the function to return all the rows in one call, as a tuplestore. Reported-by: Tom Lane Discussion: https://www.postgresql.org/message-id/234863.1610916631%40sss.pgh.pa.us
-
Tomas Vondra authored
Make sure COPY FREEZE marks the pages as PD_ALL_VISIBLE and updates the visibility map. Until now we only marked individual tuples as frozen, but page-level flags were not updated, so the first VACUUM after the COPY FREEZE had to rewrite the whole table. This is a fairly old patch, and multiple people worked on it. The first version was written by Jeff Janes, and then reworked by Pavan Deolasee and Anastasia Lubennikova. Author: Anastasia Lubennikova, Pavan Deolasee, Jeff Janes Reviewed-by: Kuntal Ghosh, Jeff Janes, Tomas Vondra, Masahiko Sawada, Andres Freund, Ibrar Ahmed, Robert Haas, Tatsuro Ishii, Darafei Praliaskouski Discussion: https://postgr.es/m/CABOikdN-ptGv0mZntrK2Q8OtfUuAjqaYMGmkdU1dCKFtUxVLrg@mail.gmail.com Discussion: https://postgr.es/m/CAMkU%3D1w3osJJ2FneELhhNRLxfZitDgp9FPHee08NT2FQFmz_pQ%40mail.gmail.com
-
Tom Lane authored
The stanza to report a "partial" match could overrun the initially allocated output array, so it needs its own copy of the array-resizing logic that's in the main loop. I overlooked the need for this in ca8217c1. Per report from Alexander Lakhin. Discussion: https://postgr.es/m/3206aace-50db-e02a-bbea-76d5cdaa2cb6@gmail.com
-
Magnus Hagander authored
Data checksums did not have a longer discussion in the docs, this adds a short section with an overview. Extracted from the larger patch for on-line enabling of checksums, which has many more authors and reviewers. Author: Daniel Gustafsson Reviewed-By: Magnus Hagander, Michael Banck (and others through the big patch) Discussion: https://postgr.es/m/5ff49fa4.1c69fb81.658f3.04ac@mx.google.com
-
Tomas Vondra authored
Reverts 891a1d0b, because the new psql command \dX only worked for users users who can read pg_statistic_ext_data catalog, and most regular users lack that privilege (the catalog may contain sensitive user data). Reported-by: Noriyoshi Shinoda Discussion: https://postgr.es/m/c027a541-5856-75a5-0868-341301e1624b%40nttcom.co.jp_1
-
Magnus Hagander authored
Specifying this parameter removes the informational messages about how to start the server. This is intended for use by wrappers in different packaging systems, where those instructions would most likely be wrong anyway, but the other output from initdb would still be useful (and thus just redirecting everything to /dev/null would be bad). Author: Magnus Hagander Reviewed-By: Peter Eisentraut Discusion: https://postgr.es/m/CABUevEzo4t5bmTXF0_B9WzmuWpVbMpkNZZiGvzV8NZa-=fPqeQ@mail.gmail.com
-
Magnus Hagander authored
This add counters for number of sessions, the different kind of session termination types, and timers for how much time is spent in active vs idle in a database to pg_stat_database. Internally this also renames the parameter "force" to disconnect. This was the only use-case for the parameter before, so repurposing it to this mroe narrow usecase makes things cleaner than inventing something new. Author: Laurenz Albe Reviewed-By: Magnus Hagander, Soumyadeep Chakraborty, Masahiro Ikeda Discussion: https://postgr.es/m/b07e1f9953701b90c66ed368656f2aef40cac4fb.camel@cybertec.at
-
- 16 Jan, 2021 5 commits
-
-
Tomas Vondra authored
The new command lists extended statistics objects, possibly with their sizes. All past releases with extended statistics are supported. Author: Tatsuro Yamada Reviewed-by: Julien Rouhaud, Alvaro Herrera, Tomas Vondra Discussion: https://postgr.es/m/c027a541-5856-75a5-0868-341301e1624b%40nttcom.co.jp_1
-
Jeff Davis authored
There is no CopyResponse message; it should be CopyOutResponse. Also, if there is no WAL to stream, the server does not immediately send a CommandComplete; it's a historical timeline, so it will send a response tuple first. Discussion: https://postgr.es/m/0a2c985ebcaa1acd385350aeba561b6509187394.camel@j-davis.com
-
Noah Misch authored
The context is an object that no longer bears some aclitem that it bore initially. (A user issued REVOKE or GRANT statements upon the object.) pg_dump is forming SQL to reproduce the object ACL. Since initdb creates no ACL bearing GRANT OPTION, reaching this bug requires an extension where the creation script establishes such an ACL. No PGXN extension does that. If an installation did reach the bug, pg_dump would have omitted a semicolon, causing a REVOKE and the next SQL statement to fail. Separately, since the affected code exists to eliminate an entire aclitem, it wants plain REVOKE, not REVOKE GRANT OPTION FOR. Back-patch to 9.6, where commit 23f34fa4 first appeared. Discussion: https://postgr.es/m/20210109102423.GA160022@rfd.leadboat.com
-
Noah Misch authored
Every core SLRU wraps around. With the exception of pg_notify, the wrap point can fall in the middle of a page. Account for this in the PagePrecedes callback specification and in SimpleLruTruncate()'s use of said callback. Update each callback implementation to fit the new specification. This changes SerialPagePrecedesLogically() from the style of asyncQueuePagePrecedes() to the style of CLOGPagePrecedes(). (Whereas pg_clog and pg_serial share a key space, pg_serial is nothing like pg_notify.) The bug fixed here has the same symptoms and user followup steps as 592a589a04bd456410b853d86bd05faa9432cbbb. Back-patch to 9.5 (all supported versions). Reviewed by Andrey Borodin and (in earlier versions) by Tom Lane. Discussion: https://postgr.es/m/20190202083822.GC32531@gust.leadboat.com
-
Amit Kapila authored
The result of TextDatumGetCString is already palloc'ed so we don't need to allocate memory for it again. We decide not to backpatch it as there doesn't seem to be any case where it can create a meaningful leak. Author: Zhijie Hou Reviewed-by: Daniel Gustafsson Discussion: https://postgr.es/m/229fed2eb8c54c71a96ccb99e516eb12@G08CNEXMBPEKD05.g08.fujitsu.local
-
- 15 Jan, 2021 7 commits
-
-
Tomas Vondra authored
Add a check that CREATE STATISTICS does not add extended statistics on system catalogs, similarly to indexes etc. It can be overriden using the allow_system_table_mods GUC. This bug exists since 7b504eb2, adding the extended statistics, so backpatch all the way back to PostgreSQL 10. Author: Tomas Vondra Reported-by: Dean Rasheed Backpatch-through: 10 Discussion: https://postgr.es/m/CAEZATCXAPrrOKwEsyZKQ4uzzJQWBCt6QAvOcgqRGdWwT1zb%2BrQ%40mail.gmail.com
-
Tom Lane authored
In cases where Xcode is newer than the underlying macOS version, asking xcodebuild for the SDK path will produce a pointer to the SDK shipped with Xcode, which may end up building code that does not work on the underlying macOS version. It appears that in such cases, xcodebuild's answer also fails to match the default behavior of Apple's compiler: assuming one has installed Xcode's "command line tools", there will be an SDK for the OS's own version in /Library/Developer/CommandLineTools, and the compiler will default to using that. This is all pretty poorly documented, but experimentation suggests that "xcrun --show-sdk-path" gives the sysroot path that the compiler is actually using, at least in some cases. Hence, try that first, but revert to xcodebuild if xcrun fails (in very old Xcode, it is missing or lacks the --show-sdk-path switch). Also, "xcrun --show-sdk-path" may give a path that is valid but lacks any OS version identifier. We don't really want that, since most of the motivation for wiring -isysroot into the build flags at all is to ensure that all parts of a PG installation are built against the same SDK, even when considering extensions built later and/or on a different machine. Insist on finding "N.N" in the directory name before accepting the result. (Adding "--sdk macosx" to the xcrun call seems to produce the same answer as xcodebuild, but usually more quickly because it's cached, so we also try that as a fallback.) The core reason why we don't want to use Xcode's default SDK in cases like this is that Apple's technology for introducing new syscalls does not play nice with Autoconf: for example, configure will think that preadv/pwritev exist when using a Big Sur SDK, even when building on an older macOS version where they don't exist. It'd be nice to have a better solution to that problem, but this patch doesn't attempt to fix that. Per report from Sergey Shinderuk. Back-patch to all supported versions. Discussion: https://postgr.es/m/ed3b8e5d-0da8-6ebd-fd1c-e0ac80a4b204@postgrespro.ru
-
Alvaro Herrera authored
This is like commit c98763bf, but for REINDEX CONCURRENTLY. To wit: this flags indicates that the current process is safe to ignore for the purposes of waiting for other snapshots, when doing CREATE INDEX CONCURRENTLY and REINDEX CONCURRENTLY. This helps two processes doing either of those things not deadlock, and also avoids spurious waits. Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: Dmitry Dolgov <9erthalion6@gmail.com> Reviewed-by: Hamid Akhtar <hamid.akhtar@gmail.com> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Discussion: https://postgr.es/m/20201130195439.GA24598@alvherre.pgsql
-
Fujii Masao authored
Commit ac883ac4 refactored shm_toc_estimate() but changed its calculation of shared memory size for TOC incorrectly. Previously this could cause too large memory to be allocated. Back-patch to v11 where the bug was introduced. Author: Takayuki Tsunakawa Discussion: https://postgr.es/m/TYAPR01MB2990BFB73170E2C4921E2C4DFEA80@TYAPR01MB2990.jpnprd01.prod.outlook.com
-
Michael Paquier authored
The last reference to those variables has been removed in aef8948f, so this cleans up a bit the code. Discussion: https://postgr.es/m/X//ggAqmTtt+3t7X@paquier.xyz
-
Michael Paquier authored
The counter tracking the last segment number recycled was getting initialized when recycling one single segment, while it should be used across a full cycle of segments recycled to prevent useless checks related to entries already recycled. This performance issue has been introduced by b2a5545b, and it was first implemented in 61b86142. No backpatch is done per the lack of field complaints. Reported-by: Andres Freund, Thomas Munro Author: Michael Paquier Reviewed-By: Andres Freund Discussion: https://postgr.es/m/20170621211016.eln6cxxp3jrv7m4m@alap3.anarazel.de Discussion: https://postgr.es/m/CA+hUKG+DRiF9z1_MU4fWq+RfJMxP7zjoptfcmuCFPeO4JM2iVg@mail.gmail.com
-
Fujii Masao authored
The foreign server OID stored in the connection cache entry is used as a lookup key to directly get the server name. Previously since the connection cache entry did not have the server OID, postgres_fdw had to get the server OID at first from user mapping before getting the server name. So if the corresponding user mapping was dropped, postgres_fdw could raise the error "cache lookup failed for user mapping" while looking up user mapping and fail to get the server name even though the server had not been dropped yet. Author: Bharath Rupireddy Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/CALj2ACVRZPUB7ZwqLn-6DY8C_UmPs6084gSpHA92YBv++1AJXA@mail.gmail.com
-
- 14 Jan, 2021 7 commits
-
-
Tom Lane authored
This is the same fix as commit 9eabfe30 applied to INDEX ATTACH entries, but for table-to-publication attachments. As in that case, even though the backend doesn't record "ownership" of the attachment, we still ought to label it in the dump archive with the role name that should run the ALTER PUBLICATION command. The existing behavior causes the ALTER to be done by the original role that started the restore; that will usually work fine, but there may be corner cases where it fails. The bulk of the patch is concerned with changing struct PublicationRelInfo to include a pointer to the associated PublicationInfo object, so that we can get the owner's name out of that when the time comes. While at it, I rewrote getPublicationTables() to do just one query of pg_publication_rel, not one per table. Back-patch to v10 where this code was introduced. Discussion: https://postgr.es/m/1165710.1610473242@sss.pgh.pa.us
-
Alvaro Herrera authored
When a tablespace is used in a partitioned relation (per commits ca410302 in pg12 for tables and 33e6c34c3267 in pg11 for indexes), it is possible to drop the tablespace, potentially causing various problems. One such was reported in bug #16577, where a rewriting ALTER TABLE causes a server crash. Protect against this by using pg_shdepend to keep track of tablespaces when used for relations that don't keep physical files; we now abort a tablespace if we see that the tablespace is referenced from any partitioned relations. Backpatch this to 11, where this problem has been latent all along. We don't try to create pg_shdepend entries for existing partitioned indexes/tables, but any ones that are modified going forward will be protected. Note slight behavior change: when trying to drop a tablespace that contains both regular tables as well as partitioned ones, you'd previously get ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE and now you'll get ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST. Arguably, the latter is more correct. It is possible to add protecting pg_shdepend entries for existing tables/indexes, by doing ALTER TABLE ONLY some_partitioned_table SET TABLESPACE pg_default; ALTER TABLE ONLY some_partitioned_table SET TABLESPACE original_tablespace; for each partitioned table/index that is not in the database default tablespace. Because these partitioned objects do not have storage, no file needs to be actually moved, so it shouldn't take more time than what's required to acquire locks. This query can be used to search for such relations: SELECT ... FROM pg_class WHERE relkind IN ('p', 'I') AND reltablespace <> 0 Reported-by: Alexander Lakhin <exclusion@gmail.com> Discussion: https://postgr.es/m/16577-881633a9f9894fd5@postgresql.org Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: Michael Paquier <michael@paquier.xyz>
-
Fujii Masao authored
Commit fef5b47f added the regression test to check whether a standby is able to follow a primary on a newer timeline when WAL archiving is enabled. But the buildfarm member florican reported that this test failed because the requested WAL segment was removed and replication failed. This is a timing issue. Since neither replication slot is used nor wal_keep_size is set in the test, checkpoint could remove the WAL segment that's still necessary for replication. This commit stabilizes the test by setting wal_keep_size. Back-patch to v13 where the regression test that this commit stabilizes was added. Author: Fujii Masao Discussion: https://postgr.es/m/X//PsenxcC50jDzX@paquier.xyz
-
Fujii Masao authored
This commit makes CLOSE, FETCH and MOVE commands tab-complete the list of cursors. Also this commit makes DECLARE command tab-complete the options. Author: Shinya Kato, Sawada Masahiko, tweaked by Fujii Masao Reviewed-by: Shinya Kato, Sawada Masahiko, Fujii Masao Discussion: https://postgr.es/m/b0e4c5c53ef84c5395524f5056fc71f0@MP-MSGSS-MBX001.msg.nttdata.co.jp
-
Thomas Munro authored
Remove redundant function declaration and improve header comment in pg_iovec.h. Move the new declaration in fd.h next to a group of more similar functions.
-
Fujii Masao authored
Commit 709d003f refactored WAL-reading code, but accidentally caused WalSndSegmentOpen() to fail to follow a timeline switch while reading from a historic timeline. This issue caused a standby to fail to follow a primary on a newer timeline when WAL archiving is enabled. If there is a timeline switch within the segment, WalSndSegmentOpen() should read from the WAL segment belonging to the new timeline. But previously since it failed to follow a timeline switch, it tried to read the WAL segment with old timeline. When WAL archiving is enabled, that WAL segment with old timeline doesn't exist because it's renamed to .partial. This leads a primary to have tried to read non-existent WAL segment, and which caused replication to faill with the error "ERROR: requested WAL segment ... has already been removed". This commit fixes WalSndSegmentOpen() so that it's able to follow a timeline switch, to ensure that a standby is able to follow a primary on a newer timeline even when WAL archiving is enabled. This commit also adds the regression test to check whether a standby is able to follow a primary on a newer timeline when WAL archiving is enabled. Back-patch to v13 where the bug was introduced. Reported-by: Kyotaro Horiguchi Author: Kyotaro Horiguchi, tweaked by Fujii Masao Reviewed-by: Alvaro Herrera, Fujii Masao Discussion: https://postgr.es/m/20201209.174314.282492377848029776.horikyota.ntt@gmail.com
-
Michael Paquier authored
This commit addresses some issues with c3826f83 that moved the hex decoding routine to src/common/: - The decoding function lacked overflow checks, so when used for security-related features it was an open door to out-of-bound writes if not carefully used that could remain undetected. Like the base64 routines already in src/common/ used by SCRAM, this routine is reworked to check for overflows by having the size of the destination buffer passed as argument, with overflows checked before doing any writes. - The encoding routine was missing. This is moved to src/common/ and it gains the same overflow checks as the decoding part. On failure, the hex routines of src/common/ issue an error as per the discussion done to make them usable by frontend tools, but not by shared libraries. Note that this is why ECPG is left out of this commit, and it still includes a duplicated logic doing hex encoding and decoding. While on it, this commit uses better variable names for the source and destination buffers in the existing escape and base64 routines in encode.c and it makes them more robust to overflow detection. The previous core code issued a FATAL after doing out-of-bound writes if going through the SQL functions, which would be enough to detect problems when working on changes that impacted this area of the code. Instead, an error is issued before doing an out-of-bound write. The hex routines were being directly called for bytea conversions and backup manifests without such sanity checks. The current calls happen to not have any problems, but careless uses of such APIs could easily lead to CVE-class bugs. Author: Bruce Momjian, Michael Paquier Reviewed-by: Sehrope Sarkuni Discussion: https://postgr.es/m/20201231003557.GB22199@momjian.us
-
- 13 Jan, 2021 3 commits
-
-
Thomas Munro authored
macOS's ranlib issued a warning about an empty pread.o file with the previous arrangement, on systems new enough to require no replacement functions. Let's go back to using configure's AC_REPLACE_FUNCS system to build and include each .o in the library only if it's needed, which requires moving the *v() functions to their own files. Also move the _with_retry() wrapper to a more permanent home. Reported-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/1283127.1610554395%40sss.pgh.pa.us
-
Tom Lane authored
These need to be PR because they access the MyProcPort data structure, which doesn't get copied to parallel workers. The very similar functions inet_client_addr() and inet_client_port() are already marked PR, but somebody missed these. Although this is a pre-existing bug, we can't readily fix it in the back branches since we can't force initdb. Given the small usage of these two functions, and the even smaller likelihood that they'd get pushed to a parallel worker anyway, it doesn't seem worth the trouble to suggest that DBAs should fix it manually. Masahiko Sawada Discussion: https://postgr.es/m/CAD21AoAT4aHP0Uxq91qpD7NL009tnUYQe-b14R3MnSVOjtE71g@mail.gmail.com
-
Tom Lane authored
Things had gotten pretty messy here, apparently mostly but not entirely the fault of the multirange patch. No functional changes.
-