aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
* Remove unneeded Python includesPeter Eisentraut2021-11-25
| | | | | | | | | Inluding <compile.h> and <eval.h> has not been necessary since Python 2.4, since they are included via <Python.h>. Morever, <eval.h> is being removed in Python 3.11. So remove these includes. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/84884.1637723223%40sss.pgh.pa.us
* Block ALTER TABLE .. DROP NOT NULL on columns in replica identity indexMichael Paquier2021-11-25
| | | | | | | | | | | | | | | | | Replica identities that depend directly on an index rely on a set of properties, one of them being that all the columns defined in this index have to be marked as NOT NULL. There was a hole in the logic with ALTER TABLE DROP NOT NULL, where it was possible to remove the NOT NULL property of a column part of an index used as replica identity, so block it to avoid problems with logical decoding down the road. The same check was already done columns part of a primary key, so the fix is straight-forward. Author: Haiying Tang, Hou Zhijie Reviewed-by: Dilip Kumar, Michael Paquier Discussion: https://postgr.es/m/OS0PR01MB6113338C102BEE8B2FFC5BD9FB619@OS0PR01MB6113.jpnprd01.prod.outlook.com Backpatch-through: 10
* Fix fstat() emulation on Windows with standard streamsMichael Paquier2021-11-25
| | | | | | | | | | | | | | | | | | | | | | | | | The emulation of fstat() in win32stat.c caused two issues with the existing in-core callers, failing on EINVAL when using a stream as argument: - psql's \copy would crash when using a stream. - pg_recvlogical would fail with -f -. The tests in copyselect.sql from the main test suite covers the first case, and there is a TAP test for the second case. However, in both cases, as the standard streams are always redirected, automated tests did not notice those issues, requiring a terminal on Windows to be reproducible. This issue has been introduced in bed9075, and the origin of the problem is that GetFileInformationByHandle() does not work directly on streams, so this commit adds an extra code path to emulate and return a set of stats that match best with the reality. Note that redirected streams rely on handles that can be queried with GetFileInformationByHandle(), but we can rely on GetFinalPathNameByHandleA() to detect this case. Author: Dmitry Koval, Juan José Santamaría Flecha Discussion: https://postgr.es/m/17288-6b58a91025a8a8a3@postgresql.org Backpatch-through: 14
* Replace straggling uses of ReadRecPtr/EndRecPtr.Andres Freund2021-11-24
| | | | | | | d2ddfa681db removed ReadRecPtr/EndRecPtr, but two uses within an #ifdef WAL_DEBUG escaped. Discussion: https://postgr.es/m/20211124231206.gbadj5bblcljb6d5@alap3.anarazel.de
* xlog.c: Remove global variables ReadRecPtr and EndRecPtr.Robert Haas2021-11-24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In most places, the variables necessarily store the same value as the eponymous members of the XLogReaderState that we use during WAL replay, because ReadRecord() assigns the values from the structure members to the global variables just after XLogReadRecord() returns. However, XLogBeginRead() adjusts the structure members but not the global variables, so after XLogBeginRead() and before the completion of XLogReadRecord() the values can differ. Otherwise, they must be identical. According to my analysis, the only place where either variable is referenced at a point where it might not have the same value as the structure member is the refrence to EndRecPtr within XLogPageRead. Therefore, at every other place where we are using the global variable, we can just switch to using the structure member instead, and remove the global variable. However, we can, and in fact should, do this in XLogPageRead() as well, because at that point in the code, the global variable will actually store the start of the record we want to read - either because it's where the last WAL record ended, or because the read position has been changed using XLogBeginRead since the last record was read. The structure member, on the other hand, will already have been updated to point to the end of the record we just read. Elsewhere, the latter is what we use as an argument to emode_for_corrupt_record(), so we should do the same here. This part of the patch is perhaps a bug fix, but I don't think it has any important consequences, so no back-patch. The point here is just to continue to whittle down the entirely excessive use of global variables in xlog.c. Discussion: http://postgr.es/m/CA+Tgmoao96EuNeSPd+hspRKcsCddu=b1h-QNRuKfY8VmfNQdfg@mail.gmail.com
* Fix corner-case failure to detect improper timeline switch.Robert Haas2021-11-24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | rescanLatestTimeLine() contains a guard against switching to a timeline that forked off from the current one prior to the current recovery point, but that guard does not work if the timeline switch occurs before the first WAL recod (which must be the checkpoint record) is read. Without this patch, an improper timeline switch is therefore possible in such cases. This happens because rescanLatestTimeLine() relies on the global variable EndRecPtr to understand the current position of WAL replay. However, EndRecPtr at this point in the code contains the endpoint of the last-replayed record, not the startpoint or endpoint of the record being replayed now. Thus, before any records have been replayed, it's zero, which causes the sanity check to always pass. To fix, pass down the correct timeline explicitly. The EndRecPtr value we want is the one from the xlogreader, which will be the starting position of the record we're about to try to read, rather than the global variable, which is the ending position of the last record we successfully read. They're usually the same, but not in the corner case described here. No back-patch, because in v14 and earlier branhes, we were using the wrong TLI here as well as the wrong LSN. In master, that was fixed by commit 4a92a1c3d1c361ffb031ed05bf65b801241d7cdd, but that and it's prerequisite patches are too invasive to back-patch for such a minor issue. Patch by me, reviewed by Amul Sul. Discussion: http://postgr.es/m/CA+Tgmoao96EuNeSPd+hspRKcsCddu=b1h-QNRuKfY8VmfNQdfg@mail.gmail.com
* Remove useless LZ4 system call on failure when writing file headerMichael Paquier2021-11-24
| | | | | | | | | | | | If an error occurs when writing the LZ4 file header, LZ4F_compressEnd() was called in the error code path of write(), followed by LZ4F_freeCompressionContext() to finish the cleanup. The code as-is was not broken, but the LZ4F_compressEnd() proves to not be necessary as there are no contents to flush at this stage, so remove it. Per gripe from Jeevan Ladhe and Robert Haas. Discussion: https://postgr.es/m/CAOgcT0PE33wbD7giAT1OSkNJt=p-vu8huq++qh=ny9O=SCP5aA@mail.gmail.com
* Flush Memoize cache when non-key parameters change, take 2David Rowley2021-11-24
| | | | | | | | | | | | | | | | It's possible that a subplan below a Memoize node contains a parameter from above the Memoize node. If this parameter changes then cache entries may become out-dated due to the new parameter value. Previously Memoize was mistakenly not aware of this. We fix this here by flushing the cache whenever a parameter that's not part of the cache key changes. Bug: #17213 Reported by: Elvis Pranskevichus Author: David Rowley Discussion: https://postgr.es/m/17213-988ed34b225a2862@postgresql.org Backpatch-through: 14, where Memoize was added
* Add support for Visual Studio 2022 in build scriptsMichael Paquier2021-11-24
| | | | | | | | | | | | | | Documentation and any code paths related to VS are updated to keep the whole consistent. Similarly to 2017 and 2019, the version of VS and the version of nmake that we use to determine which code paths to use for the build are still inconsistent in their own way. Backpatch down to 10, so as buildfarm members are able to use this new version of Visual Studio on all the stable branches supported. Author: Hans Buschmann Discussion: https://postgr.es/m/1633101364685.39218@nidsa.net Backpatch-through: 10
* Rename SnapBuild* macros in slot.c.Amit Kapila2021-11-24
| | | | | | | | | | | | Same macro names for SnapBuildOnDiskNotChecksummedSize and SnapBuildOnDiskChecksummedSize are being used in slot.c and snapbuild.c. This patch renames them, in slot.c, to ReplicationSlotOnDiskNotChecksummedSize and ReplicationSlotOnDiskChecksummedSize similar to the other macros. This makes all macro names look consistent in slot.c. Author: Bharath Rupireddy Discussion: https://postgr.es/m/CALj2ACVZo-piDGzBOJRY4ob=_goFR6t9DhZMDMjJWN7LQs34Aw@mail.gmail.com
* Revert "Flush Memoize cache when non-key parameters change"David Rowley2021-11-24
| | | | This reverts commit 1050048a315790a505465bfcceb26eaf8dbc7e2e.
* Flush Memoize cache when non-key parameters changeDavid Rowley2021-11-24
| | | | | | | | | | | | | | | | It's possible that a subplan below a Memoize node contains a parameter from above the Memoize node. If this parameter changes then cache entries may become out-dated due to the new parameter value. Previously Memoize was mistakenly not aware of this. We fix this here by flushing the cache whenever a parameter that's not part of the cache key changes. Bug: #17213 Reported by: Elvis Pranskevichus Author: David Rowley Discussion: https://postgr.es/m/17213-988ed34b225a2862@postgresql.org Backpatch-through: 14, where Memoize was added
* Allow Memoize to operate in binary comparison modeDavid Rowley2021-11-24
| | | | | | | | | | | | | | | | | | | | | | Memoize would always use the hash equality operator for the cache key types to determine if the current set of parameters were the same as some previously cached set. Certain types such as floating points where -0.0 and +0.0 differ in their binary representation but are classed as equal by the hash equality operator may cause problems as unless the join uses the same operator it's possible that whichever join operator is being used would be able to distinguish the two values. In which case we may accidentally return in the incorrect rows out of the cache. To fix this here we add a binary mode to Memoize to allow it to the current set of parameters to previously cached values by comparing bit-by-bit rather than logically using the hash equality operator. This binary mode is always used for LATERAL joins and it's used for normal joins when any of the join operators are not hashable. Reported-by: Tom Lane Author: David Rowley Discussion: https://postgr.es/m/3004308.1632952496@sss.pgh.pa.us Backpatch-through: 14, where Memoize was added
* Add SQL functions to monitor the directory contents of replication slotsMichael Paquier2021-11-23
| | | | | | | | | | | | | | | | | | This commit adds a set of functions able to look at the contents of various paths related to replication slots: - pg_ls_logicalsnapdir, for pg_logical/snapshots/ - pg_ls_logicalmapdir, for pg_logical/mappings/ - pg_ls_replslotdir, for pg_replslot/<slot_name>/ These are intended to be used by monitoring tools. Unlike pg_ls_dir(), execution permission can be granted to non-superusers. Roles members of pg_monitor gain have access to those functions. Bump catalog version. Author: Bharath Rupireddy Reviewed-by: Nathan Bossart, Justin Pryzby Discussion: https://postgr.es/m/CALj2ACWsfizZjMN6bzzdxOk1ADQQeSw8HhEjhmVXn_Pu+7VzLw@mail.gmail.com
* Adjust pg_dump's priority ordering for casts.Tom Lane2021-11-22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When a stored expression depends on a user-defined cast, the backend records the dependency as being on the cast's implementation function --- or indeed, if there's no cast function involved but just RelabelType or CoerceViaIO, no dependency is recorded at all. This is problematic for pg_dump, which is at risk of dumping things in the wrong order leading to restore failures. Given the lack of previous reports, the risk isn't that high, but it can be demonstrated if the cast is used in some view whose rowtype is then used as an input or result type for some other function. (That results in the view getting hoisted into the functions portion of the dump, ahead of the cast.) A logically bulletproof fix for this would require including the cast's OID in the parsed form of the expression, whence it could be extracted by dependency.c, and then the stored dependency would force pg_dump to do the right thing. Such a change would be fairly invasive, and certainly not back-patchable. Moreover, since we'd prefer that an expression using cast syntax be equal() to one doing the same thing by explicit function call, the cast OID field would have to have special ignored-by-comparisons semantics, making things messy. So, let's instead fix this by a very simple hack in pg_dump: change the object-type priority order so that casts are initially sorted before functions, immediately after types. This fixes the problem in a fairly direct way for casts that have no implementation function. For those that do, the implementation function will be hoisted to just before the cast by the dependency sorting step, so that we still have a valid dump order. (I'm not sure that this provides a full guarantee of no problems; but since it's been like this for many years without any previous reports, this is probably enough to fix it in practice.) Per report from Дмитрий Иванов. Back-patch to all supported branches. Discussion: https://postgr.es/m/CAPL5KHoGa3uvyKp6z6m48LwCnTsK+LRQ_mcA4uKGfqAVSEjV_A@mail.gmail.com
* Fix pg_dump --inserts mode for generated columns with dropped columns.Tom Lane2021-11-22
| | | | | | | | | | | | | | | | | | | | If a table contains a generated column that's preceded by a dropped column, dumpTableData_insert failed to account for the dropped column, and would emit DEFAULT placeholder(s) in the wrong column(s). This resulted in failures at restore time. The default COPY code path did not have this bug, likely explaining why it wasn't noticed sooner. While we're fixing this, we can be a little smarter about the situation: (1) avoid unnecessarily fetching the values of generated columns, (2) omit generated columns from the output, too, if we're using --column-inserts. While these modes aren't expected to be as high-performance as the COPY path, we might as well be as efficient as we can; it doesn't add much complexity. Per report from Дмитрий Иванов. Back-patch to v12 where generated columns came in. Discussion: https://postgr.es/m/CAPL5KHrkBniyQt5e1rafm5DdXvbgiiqfEQEJ9GjtVzN71Jj5pA@mail.gmail.com
* Be more specific about OOM in XLogReaderAllocateAlvaro Herrera2021-11-22
| | | | | | | | | | | A couple of spots can benefit from an added errdetail(), which matches what we were already doing in other places; and those that cannot withstand errdetail() can get a more descriptive primary message. Author: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Reviewed-by: Julien Rouhaud <rjuju123@gmail.com> Discussion: https://postgr.es/m/CALj2ACV+cX1eM03GfcA=ZMLXh5fSn1X1auJLz3yuS1duPSb9QA@mail.gmail.com
* autovacuum: Improve wording in a couple placesAlvaro Herrera2021-11-22
| | | | | | | | | | | | | | | | | A few strings (one WARNING and some memory context names) in the autovacuum code were written in a world where "worker" had no other possible meaning than "autovacuum worker", but that's long time gone. Be more specific about it. Also, change the WARNING from elog() to ereport(), to add translability. Author: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> Reviewed-by: Nathan Bossart <bossartn@amazon.com> Reviewed-by: Justin Pryzby <pryzby@telsasoft.com> Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Reviewed-by: Dilip Kumar <dilipbalaut@gmail.com> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Discussion: https://postgr.es/m/CALj2ACX2UHp76dqdoZq92a7v4APFuV5wJQ+AUrb+2HURrKN=NQ@mail.gmail.com
* Add missing words in commentAlvaro Herrera2021-11-22
| | | | | | Reported by Zhihong Yu. Discussion: https://postgr.es/m/CALNJ-vR6uZivg_XkB1zKjEXeyZDEgoYanFXB-++1kBT9yZQoUw@mail.gmail.com
* Add ABI extra field to fmgr magic blockPeter Eisentraut2021-11-22
| | | | | | | This allows derived products to intentionally make their fmgr ABI incompatible, with a clean error message. Discussion: https://www.postgresql.org/message-id/flat/55215fda-db31-a045-d6b7-d6f2d2dc9920%40enterprisedb.com
* Report wait events for local shell commands like archive_command.Fujii Masao2021-11-22
| | | | | | | | | This commit introduces new wait events for archive_command, archive_cleanup_command, restore_command and recovery_end_command. Author: Fujii Masao Reviewed-by: Bharath Rupireddy, Michael Paquier Discussion: https://postgr.es/m/4ca4f920-6b48-638d-08b2-93598356f5d3@oss.nttdata.com
* Remove lazy_scan_heap parallel VACUUM comment block.Peter Geoghegan2021-11-21
| | | | | | This doesn't belong next to very high level discussion of the tasks that lazy_scan_heap performs. There is already a similar, longer comment block at the top of vacuumlazy.c that mentions lazy_scan_heap directly.
* pg_receivewal, pg_recvlogical: allow canceling initial password prompt.Tom Lane2021-11-21
| | | | | | | | | | | | | | | | | | | | | Previously it was impossible to terminate these programs via control-C while they were prompting for a password. We can fix that trivially for their initial password prompts, by moving setup of the SIGINT handler from just before to just after their initial GetConnection() calls. This fix doesn't permit escaping out of later re-prompts, but those should be exceedingly rare, since the user's password or the server's authentication setup would have to have changed meanwhile. We considered applying a fix similar to commit 46d665bc2, but that seemed more complicated than it'd be worth. Moreover, this way is back-patchable, which that wasn't. The misbehavior exists in all supported versions, so back-patch to all. Tom Lane and Nathan Bossart Discussion: https://postgr.es/m/747443.1635536754@sss.pgh.pa.us
* Doc: update some things relevant to minimum Test::More version.Tom Lane2021-11-21
| | | | | | | Oversights in commit 405f32fc4. Also, add a tip (discovered the hard way) about getting Test::More 0.98 to pass its regression tests on recent Linux platforms.
* Require version 0.98 of Test::More for TAP testsAndrew Dunstan2021-11-20
| | | | | | | | | This means that the subtest feature will be available for use. We expect that this change will make prairiedog go red until it is updated, but other buildfarm animals should be fine. Discussion: https://postgr.es/m/f5e1d308-4e33-37a7-bdf1-f6e0c75119de@dunslane.net
* Fix SP-GiST scan initialization logic for binary-compatible cases.Tom Lane2021-11-20
| | | | | | | | | | | | | | | | | | | | | | | Commit ac9099fc1 rearranged the logic in spgGetCache() that determines the index's attType (nominal input data type) and leafType (actual type stored in leaf index tuples). Turns out this broke things for the case where (a) the actual input data type is different from the nominal type, (b) the opclass's config function leaves leafType defaulted, and (c) the opclass has no "compress" function. (b) caused us to assign the actual input data type as leafType, and then since that's not attType, we complained that a "compress" function is required. For non-polymorphic opclasses, condition (a) arises in binary-compatible cases, such as using SP-GiST text_ops for a varchar column, or using any opclass on a domain over its nominal input type. To fix, use attType for leafType when the index's declared column type is different from but binary-compatible with attType. Do this only in the defaulted-leafType case, to avoid overriding any explicit selection made by the opclass. Per bug #17294 from Ilya Anfimov. Back-patch to v14. Discussion: https://postgr.es/m/17294-8f6c7962ce877edc@postgresql.org
* Allow psql's other uses of simple_prompt() to be interrupted by ^C.Tom Lane2021-11-19
| | | | | | | | | | | | This fills in the work left un-done by 5f1148224. \prompt can be canceled out of now, and so can password prompts issued during \connect. (We don't need to do anything for password prompts issued during startup, because we aren't yet trapping SIGINT at that point.) Nathan Bossart Discussion: https://postgr.es/m/747443.1635536754@sss.pgh.pa.us
* Initialize backend status reporting during bootstrap.Andres Freund2021-11-19
| | | | | | | | | | This allows a later commit to reduce the number of branches in performance sensitive functions during normal running, compared to a very minor saving during bootstrapping. Author: Melanie Plageman <melanieplageman@gmail.com> Reviewed-By: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/CAAKRu_Yeg+vh6SHNEo1+=O7e-BPX35cU0XQM=YwQRnkFyv_y+w@mail.gmail.com
* Fix parallel operations that prevent oldest xmin from advancing.Amit Kapila2021-11-19
| | | | | | | | | | | | | | | | | While determining xid horizons, we skip over backends that are running Vacuum. We also ignore Create Index Concurrently, or Reindex Concurrently for the purposes of computing Xmin for Vacuum. But we were not setting the flags corresponding to these operations when they are performed in parallel which was preventing Xid horizon from advancing. The optimization related to skipping Create Index Concurrently, or Reindex Concurrently operations was implemented in PG-14 but the fix is the same for the Parallel Vacuum as well so back-patched till PG-13. Author: Masahiko Sawada Reviewed-by: Amit Kapila Backpatch-through: 13 Discussion: https://postgr.es/m/CAD21AoCLQqgM1sXh9BrDFq0uzd3RBFKi=Vfo6cjjKODm0Onr5w@mail.gmail.com
* Improve psql tab completion for transforms, domains and sequencesMichael Paquier2021-11-19
| | | | | | | | | | | The following improvements are done: - Addition of some tab completion for CREATE DOMAIN. - Addition of some tab completion for CREATE TRANSFORM. - Addition of type completion for CREATE SEQUENCE AS. Author: Ken Kato Reviewed-by: Kyotaro Horiguchi, Michael Paquier Discussion: https://postgr.es/m/8d370135aef066659eef8e8fbfa6315b@oss.nttdata.com
* Fix quoting of ACL item in table for upgrade binary compatibility checksMichael Paquier2021-11-18
| | | | | | | | Per buildfarm member prion, that runs the regression tests under a role name that uses a hyphen. Issue introduced by 835bcba. Discussion: https://postgr.es/m/YZW4MvzCZ+hQ34vw@paquier.xyz Backpatch-through: 12
* Add table to regression tests for binary-compatibility checks in pg_upgradeMichael Paquier2021-11-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | This commit adds to the main regression test suite a table with all the in-core data types (some exceptions apply). This table is not dropped, so as pg_upgrade would be able to check the binary compatibility of the types tracked in the table. If a new type is added in core, this part of the tests would need a refresh but the tests are designed to fail if that were to happen. As this is useful for upgrades and that these rely on the objects created in the regression test suite of the old version upgraded from, a backpatch down to 12 is done, which is the last point where a binary incompatible change has been done (7c15cef). This will hopefully be enough to find out if something gets broken during the development of a new version of Postgres, so as it is possible to take actions in pg_upgrade itself in this case (like 0ccfc28 for sql_identifier). An area that is not covered yet is related to external modules, which may create their own types. The testing infrastructure of pg_upgrade is not integrated yet with the external modules stored in core (src/test/modules/ or contrib/, all use the same database name for their tests so there would be an overlap). This could be improved in the future. Author: Justin Pryzby Reviewed-by: Jacob Champion, Peter Eisentraut, Tom Lane, Michael Paquier Discussion: https://postgr.es/m/20201206180248.GI24052@telsasoft.com Backpatch-through: 12
* Provide a variant of simple_prompt() that can be interrupted by ^C.Tom Lane2021-11-17
| | | | | | | | | | | | | | | | | | | | | Up to now, you couldn't escape out of psql's \password command by typing control-C (or other local spelling of SIGINT). This is pretty user-unfriendly, so improve it. To do so, we have to modify the functions provided by pg_get_line.c; but we don't want to mess with psql's SIGINT handler setup, so provide an API that lets that handler cause the cancel to occur. This relies on the assumption that we won't do any major harm by longjmp'ing out of fgets(). While that's obviously a little shaky, we've long had the same assumption in the main input loop, and few issues have been reported. psql has some other simple_prompt() calls that could usefully be improved the same way; for now, just deal with \password. Nathan Bossart, minor tweaks by me Discussion: https://postgr.es/m/747443.1635536754@sss.pgh.pa.us
* Add a planner support function for starts_with().Tom Lane2021-11-17
| | | | | | | | | | | | | | | | | | | | | | This fills in some gaps in planner support for starts_with() and the equivalent ^@ operator: * A condition such as "textcol ^@ constant" can now use a regular btree index, not only an SP-GiST index, so long as the index's collation is C. (This works just like "textcol LIKE 'foo%'".) * "starts_with(textcol, constant)" can be optimized the same as "textcol ^@ constant". * Fixed-prefix LIKE and regex patterns are now more like starts_with() in another way: if you apply one to an SPGiST-indexed column, you'll get an index condition using ^@ rather than two index conditions with >= and <. Per a complaint from Shay Rojansky. Patch by me; thanks to Nathan Bossart for review. Discussion: https://postgr.es/m/232599.1633800229@sss.pgh.pa.us
* Clean up error handling in pg_basebackup's walmethods.c.Tom Lane2021-11-17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The error handling here was a mess, as a result of a fundamentally bad design (relying on errno to keep its value much longer than is safe to assume) as well as a lot of just plain sloppiness, both as to noticing errors at all and as to reporting the correct errno. Moreover, the recent addition of LZ4 compression broke things completely, because liblz4 doesn't use errno to report errors. To improve matters, keep the error state in the DirectoryMethodData or TarMethodData struct, and add a string field so we can handle cases that don't set errno. (The tar methods already had a version of this, but it can be done more efficiently since all these cases use a constant error string.) Make the dir and tar methods handle errors in basically identical ways, which they didn't before. This requires copying errno into the state struct in a lot of places, which is a bit tedious, but it has the virtue that we can get rid of ad-hoc code to save and restore errno in a number of places ... not to mention that it fixes other places that should've saved/restored errno but neglected to. In passing, fix some pointlessly static buffers to be ordinary local variables. There remains an issue about exactly how to handle errors from fsync(), but that seems like material for its own patch. While the LZ4 problems are new, all the rest of this is fixes for old bugs, so backpatch to v10 where walmethods.c was introduced. Patch by me; thanks to Michael Paquier for review. Discussion: https://postgr.es/m/1343113.1636489231@sss.pgh.pa.us
* Handle close() failures more robustly in pg_dump and pg_basebackup.Tom Lane2021-11-17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Coverity complained that applying get_gz_error after a failed gzclose, as we did in one place in pg_basebackup, is unsafe. I think it's right: it's entirely likely that the call is touching freed memory. Change that to inspect errno, as we do for other gzclose calls. Also, be careful to initialize errno to zero immediately before any gzclose() call where we care about the error status. (There are some calls where we don't, because we already failed at some previous step.) This ensures that we don't get a misleadingly irrelevant error code if gzclose() fails in a way that doesn't set errno. We could work harder at that, but it looks to me like all such cases are basically can't-happen if we're not misusing zlib, so it's not worth the extra notational cruft that would be required. Also, fix several places that simply failed to check for close-time errors at all, mostly at some remove from the close or gzclose itself; and one place that did check but didn't bother to report the errno. Back-patch to v12. These mistakes are older than that, but between the frontend logging API changes that happened in v12 and the fact that frontend code can't rely on %m before that, the patch would need substantial revision to work in older branches. It doesn't quite seem worth the trouble given the lack of related field complaints. Patch by me; thanks to Michael Paquier for review. Discussion: https://postgr.es/m/1343113.1636489231@sss.pgh.pa.us
* Fix display of SQL-standard function's arguments in INSERT/SELECT.Tom Lane2021-11-17
| | | | | | | | | | | | | | | | | | | | | | | | | | | If a SQL-standard function body contains an INSERT ... SELECT statement, any function parameters referenced within the SELECT were always printed in $N style, rather than using the parameter name if any. While not strictly incorrect, this wasn't the intention, and it's inconsistent with the way that such parameters would be printed in any other kind of statement. The cause is that the recursion to get_query_def from get_insert_query_def neglected to pass down the context->namespaces list, passing constant NIL instead. This is a very ancient oversight, but AFAICT it had no visible consequences before commit e717a9a18 added an outermost namespace with function parameters. We don't allow INSERT ... SELECT as a sub-query, except in a top-level WITH clause, where it couldn't contain any outer references that might need to access upper namespaces. So although that's arguably a bug, I don't see any point in changing it before v14. In passing, harden the code added to get_parameter by e717a9a18 so that it won't crash if a PARAM_EXTERN Param appears in an unexpected place. Per report from Erki Eessaar. Code fix by me, regression test case by Masahiko Sawada. Discussion: https://postgr.es/m/AM9PR01MB8268347BED344848555167FAFE949@AM9PR01MB8268.eurprd01.prod.exchangelabs.com
* Improve publication error messagesDaniel Gustafsson2021-11-17
| | | | | | | | | | | | | Commit 81d5995b4b introduced more fine-grained errormessages for incorrect relkinds for publication, while unlogged and temporary tables were reported with using the same message. This provides separate error messages for these types of relpersistence. Author: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com> Reviewed-by: Jeevan Ladhe <jeevan.ladhe@enterprisedb.com> Reviewed-by: Euler Taveira <euler@eulerto.com> Discussion: https://postgr.es/m/CALj2ACW9S=AswyQHjtO6WMcsergMkCBTtzXGrM8DX26DzfeTLQ@mail.gmail.com
* Fix incorrect format placeholdersPeter Eisentraut2021-11-17
|
* Remove global variable "LastRec" in xlog.cMichael Paquier2021-11-17
| | | | | | | | | This variable is used only by StartupXLOG() now, so let's make it local to simplify the code. Author: Amul Sul Reviewed-by: Tom Lane, Michael Paquier Discussion: https://postgr.es/m/CAAJ_b96Qd023itERBRN9Z7P2saNDT3CYvGuMO8RXwndVNN6z7g@mail.gmail.com
* Fix headerscheck failure in replication/worker_internal.hAlvaro Herrera2021-11-16
| | | | Broken by 31c389d8de91
* Move InitXLogInsert() call from InitXLOGAccess() to BaseInit().Robert Haas2021-11-16
| | | | | | | | | | | | | | | | | At present, there is an undocumented coding rule that you must call RecoveryInProgress(), or do something else that results in a call to InitXLogInsert(), before trying to write WAL. Otherwise, the WAL construction buffers won't be initialized, resulting in failures. Since it's not good to rely on a status inquiry function like RecoveryInProgress() having the side effect of initializing critical data structures, instead do the initialization eariler, when the backend first starts up. Patch by me. Reviewed by Nathan Bossart and Michael Paquier. Discussion: http://postgr.es/m/CA+TgmoY7b65qRjzHN_tWUk8B4sJqk1vj1d31uepVzmgPnZKeLg@mail.gmail.com
* Invalidate relcache when changing REPLICA IDENTITY index.Amit Kapila2021-11-16
| | | | | | | | | | | | When changing REPLICA IDENTITY INDEX to another one, the target table's relcache was not being invalidated. This leads to skipping update/delete operations during apply on the subscriber side as the columns required to search corresponding rows won't get logged. Author: Tang Haiying, Hou Zhijie Reviewed-by: Euler Taveira, Amit Kapila Backpatch-through: 10 Discussion: https://postgr.es/m/OS0PR01MB61133CA11630DAE45BC6AD95FB939@OS0PR01MB6113.jpnprd01.prod.outlook.com
* Fix thinko in bbsink_throttle_manifest_contents.Robert Haas2021-11-15
| | | | | | Report and diagnosis by Dmitry Dolgov. Discussion: http://postgr.es/m/20211115162641.dmo6l32fklh64gnw@localhost
* Explain pruning pgstats accounting subtleties.Peter Geoghegan2021-11-12
| | | | | | | | | | | | | | | | Add a comment explaining why the pgstats accounting used during opportunistic heap pruning operations (to maintain the current number of dead tuples in the relation) needs to compensate by subtracting away the number of new LP_DEAD items. This is needed so it can avoid completely forgetting about tuples that become LP_DEAD items during pruning -- they should still count. It seems more natural to discuss this issue at the only relevant call site (opportunistic pruning), since the same issue does not apply to the only other caller (the VACUUM call site). Move everything there too. Author: Peter Geoghegan <pg@bowt.ie> Discussion: https://postgr.es/m/CAH2-Wzm7f+A6ej650gi_ifTgbhsadVW5cujAL3punpupHff5Yg@mail.gmail.com
* Document PG_TEST_NOCLEAN in TAP test READMEDaniel Gustafsson2021-11-12
| | | | | | | | | | | Commit 90627cf98 added support for retaining the data directory even on successful tests, but failed to document the environment variable which controls retention. This adds a small note to the TAP test README about PG_TEST_NOCLEAN which when set skips removing the data directories from successful tests. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/2B02C1B3-3F41-4E14-92B9-005D83623A0B@yesql.se
* Make psql's \password default to CURRENT_USER, not PQuser(conn).Tom Lane2021-11-12
| | | | | | | | | | | | | | | | | | | | | The documentation says plainly that \password acts on "the current user" by default. What it actually acted on, or tried to, was the username used to log into the current session. This is not the same thing if one has since done SET ROLE or SET SESSION AUTHENTICATION. Aside from the possible surprise factor, it's quite likely that the current role doesn't have permissions to set the password of the original role. To fix, use "SELECT CURRENT_USER" to get the role name to act on. (This syntax works with servers at least back to 7.0.) Also, in hopes of reducing confusion, include the role name that will be acted on in the password prompt. The discrepancy from the documentation makes this a bug, so back-patch to all supported branches. Patch by me; thanks to Nathan Bossart for review. Discussion: https://postgr.es/m/747443.1635536754@sss.pgh.pa.us
* Fix memory overrun when querying pg_stat_slruMichael Paquier2021-11-12
| | | | | | | | | | | | | | | | pg_stat_get_slru() in pgstatfuncs.c would point to one element after the end of the array PgStat_SLRUStats when finishing to scan its entries. This had no direct consequences as no data from the extra memory area was read, but static analyzers would rightfully complain here. So let's be clean. While on it, this adds one regression test in the area reserved for system views. Reported-by: Alexander Kozhemyakin, via AddressSanitizer Author: Kyotaro Horiguchi Discussion: https://postgr.es/m/17280-37da556e86032070@postgresql.org Backpatch-through: 13
* Report any XLogReadRecord() error in XlogReadTwoPhaseData().Noah Misch2021-11-11
| | | | | | | | | | Buildfarm members kittiwake and tadarida have witnessed errors at this site. The site discarded key facts. Back-patch to v10 (all supported versions). Reviewed by Michael Paquier and Tom Lane. Discussion: https://postgr.es/m/20211107013157.GB790288@rfd.leadboat.com
* Update heap_page_prune() free space map comments.Peter Geoghegan2021-11-11
| | | | | | | | It is up to the heap_page_prune() caller to decide what to do about updating the FSM for a page following pruning. Update old comments that address what we might want to do as if it was the responsibility of heap_page_prune() itself. heap_page_prune() doesn't have enough high-level context to make a sensible choice.