33 Commits (9fee3232a1d3661981c3cbf87227bfd02ee36c7b)

Author	SHA1	Message	Date
Peter Eisentraut	2631ebab7b	Message wording improvements	2 years ago
Tom Lane	0245f8db36	Pre-beta mechanical code beautification. ... Run pgindent, pgperltidy, and reformat-dat-files. This set of diffs is a bit larger than typical. We've updated to pg_bsd_indent 2.1.2, which properly indents variable declarations that have multi-line initialization expressions (the continuation lines are now indented one tab stop). We've also updated to perltidy version 20230309 and changed some of its settings, which reduces its desire to add whitespace to lines to make assignments etc. line up. Going forward, that should make for fewer random-seeming changes to existing code. Discussion: https://postgr.es/m/20230428092545.qfb3y5wcu4cm75ur@alvherre.pgsql	2 years ago
David Rowley	3f58a4e296	Fix various typos and incorrect/outdated name references ... Author: Alexander Lakhin Discussion: https://postgr.es/m/699beab4-a6ca-92c9-f152-f559caf6dc25@gmail.com	2 years ago
David Rowley	b4dbf3e924	Fix various typos ... This fixes many spelling mistakes in comments, but a few references to invalid parameter names, function names and option names too in comments and also some in string constants Also, fix an #undef that was undefining the incorrect definition Author: Alexander Lakhin Reviewed-by: Justin Pryzby Discussion: https://postgr.es/m/d5f68d19-c0fc-91a9-118d-7c6a5a3f5fad@gmail.com	2 years ago
Tomas Vondra	2820adf775	Support long distance matching for zstd compression ... zstd compression supports a special mode for finding matched in distant past, which may result in better compression ratio, at the expense of using more memory (the window size is 128MB). To enable this optional mode, use the "long" keyword when specifying the compression method (--compress=zstd:long). Author: Justin Pryzby Reviewed-by: Tomas Vondra, Jacob Champion Discussion: https://postgr.es/m/20230224191840.GD1653@telsasoft.com Discussion: https://postgr.es/m/20220327205020.GM28503@telsasoft.com	2 years ago
Peter Eisentraut	0d15afc875	Simplify useless 0L constants ... In ancient times, these belonged to arguments or fields that were actually of type long, but now they are not anymore, so this "L" decoration is just confusing. (Some other 0L and other "L" constants remain, where they are actually associated with a long type.)	2 years ago
Peter Eisentraut	de4d456b40	Improve several permission-related error messages. ... Mainly move some detail from errmsg to errdetail, remove explicit mention of superuser where appropriate, since that is implied in most permission checks, and make messages more uniform. Author: Nathan Bossart <nathandbossart@gmail.com> Discussion: https://www.postgresql.org/message-id/20230316234701.GA903298@nathanxps13	3 years ago
Robert Haas	e76cbb6cd6	Reword overly-optimistic comment about backup checksum verification. ... The comment implies that a single retry is sufficient to avoid spurious checksum failures, but in fact no number of retries is sufficient for that purpose. Update the comment accordingly. Patch by me, reviewed by Michael Paquier. Discussion: http://postgr.es/m/CA+TgmoZ_fFAoU6mrHt9QBs+dcYhN6yXenGTTMRebZNhtwPwHyg@mail.gmail.com	3 years ago
Robert Haas	f3948b5c91	Remove an old comment that doesn't seem especially useful. ... The functions that follow are concerned with various things, of which the tar format is only one, so this comment doesn't really seem helpful. The file isn't really divided into sections in the way that this comment seems to contemplate -- or at least, not any more. Patch by me, reviewed by Michael Paquier. Discussion: http://postgr.es/m/CA+TgmoZ_fFAoU6mrHt9QBs+dcYhN6yXenGTTMRebZNhtwPwHyg@mail.gmail.com	3 years ago
Robert Haas	33352b9279	In basebackup.c, perform end-of-file test after checksum validation. ... We read blocks of data from files that we're backing up in chunks, some multiple of BLCKSZ for each read. If checksum verification fails, we then try rereading just the one block for which validation failed. If that block happened to be the first block of the chunk, and if the file was concurrently truncated to remove that block, then we'd reach a call to bbsink_archive_contents() with a buffer length of 0. That causes an assertion failure. As far as I can see, there are no particularly bad consequences if this happens in a non-assert build, and it's pretty unlikely to happen in the first place because it requires a series of somewhat unlikely things to happen in very quick succession. However, assertion failures are bad, so rearrange the code to avoid that possibility. Patch by me, reviewed by Michael Paquier. Discussion: http://postgr.es/m/CA+TgmoZ_fFAoU6mrHt9QBs+dcYhN6yXenGTTMRebZNhtwPwHyg@mail.gmail.com	3 years ago
Tom Lane	3a28d78089	Improve TimestampDifferenceMilliseconds to cope with overflow sanely. ... We'd like to use TimestampDifferenceMilliseconds with the stop_time possibly being TIMESTAMP_INFINITY, but up to now it's disclaimed responsibility for overflow cases. Define it to clamp its output to the range [0, INT_MAX], handling overflow correctly. (INT_MAX rather than LONG_MAX seems appropriate, because the function is already described as being intended for calculating wait times for WaitLatch et al, and that infrastructure only handles waits up to INT_MAX. Also, this choice gets rid of cross-platform behavioral differences.) Having done that, we can replace some ad-hoc code in walreceiver.c with a simple call to TimestampDifferenceMilliseconds. While at it, fix some buglets in existing callers of TimestampDifferenceMilliseconds: basebackup_copy.c had not read the memo about TimestampDifferenceMilliseconds never returning a negative value, and postmaster.c had not read the memo about Min() and Max() being macros with multiple-evaluation hazards. Neither of these quite seem worth back-patching. Patch by me; thanks to Nathan Bossart for review. Discussion: https://postgr.es/m/3126727.1674759248@sss.pgh.pa.us	3 years ago
Peter Eisentraut	20428d344a	Add BufFileRead variants with short read and EOF detection ... Most callers of BufFileRead() want to check whether they read the full specified length. Checking this at every call site is very tedious. This patch provides additional variants BufFileReadExact() and BufFileReadMaybeEOF() that include the length checks. I considered changing BufFileRead() itself, but this function is also used in extensions, and so changing the behavior like this would create a lot of problems there. The new names are analogous to the existing LogicalTapeReadExact(). Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/f3501945-c591-8cc3-5ef0-b72a2e0eaa9c@enterprisedb.com	3 years ago
Peter Eisentraut	1561612e3b	Fix some BufFileRead() error reporting ... Remove "%m" from error messages where errno would be bogus. Add short read byte counts where appropriate. This is equivalent to what was done in 7897e3bb90, but some code was apparently developed concurrently to that and not updated accordingly. Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/f3501945-c591-8cc3-5ef0-b72a2e0eaa9c@enterprisedb.com	3 years ago
Bruce Momjian	c8e1ba736b	Update copyright for 2023 ... Backpatch-through: 11	3 years ago
Peter Eisentraut	faf3750657	Add const to BufFileWrite ... Make data buffer argument to BufFileWrite a const pointer and bubble this up to various callers and related APIs. This makes the APIs clearer and more consistent. Discussion: https://www.postgresql.org/message-id/flat/11dda853-bb5b-59ba-a746-e168b1ce4bdb%40enterprisedb.com	3 years ago
Andrew Dunstan	8284cf5f74	Add copyright notices to meson files ... Discussion: https://postgr.es/m/222b43a5-2fb3-2c1b-9cd0-375d376c8246@dunslane.net	3 years ago
Peter Eisentraut	75f49221c2	Static assertions cleanup ... Because we added StaticAssertStmt() first before StaticAssertDecl(), some uses as well as the instructions in c.h are now a bit backwards from the "native" way static assertions are meant to be used in C. This updates the guidance and moves some static assertions to better places. Specifically, since the addition of StaticAssertDecl(), we can put static assertions at the file level. This moves a number of static assertions out of function bodies, where they might have been stuck out of necessity, to perhaps better places at the file level or in header files. Also, when the static assertion appears in a position where a declaration is allowed, then using StaticAssertDecl() is more native than StaticAssertStmt(). Reviewed-by: John Naylor <john.naylor@enterprisedb.com> Discussion: https://www.postgresql.org/message-id/flat/941a04e7-dd6f-c0e4-8cdf-a33b3338cbda%40enterprisedb.com	3 years ago
Robert Haas	45f5c81ad2	Fix failure to advance content pointer in sendFileWithContent. ... If sendFileWithContent were used to send a file larger than the bbsink buffer size, this would result in corruption. The only files that are sent via sendFileWithContent are the backup label file, the tablespace map file, and .done files for WAL segments included in the backup. Of these, it seems that only the tablespace_map file can become large enough to cause a problem, and then only if you have a lot of tablespaces. If you do have that situation, you might end up with a corrupted tablespace_map file, which would be bad. My commit bef47ff85d introduced this problem. Report and patch by Antonin Houska. Discussion: http://postgr.es/m/15764.1670528645@antos	3 years ago
David Rowley	2d0bbedda7	Rename shadowed local variables ... In a similar effort to f01592f91, here we mostly rename shadowed local variables to remove the warnings produced when compiling with -Wshadow=compatible-local. This fixes 63 warnings and leaves just 5. Author: Justin Pryzby, David Rowley Reviewed-by: Justin Pryzby Discussion https://postgr.es/m/20220817145434.GC26426%40telsasoft.com	3 years ago
Thomas Munro	b6d8a60aba	Restore pg_pread and friends. ... Commits cf112c12 and a0dc8271 were a little too hasty in getting rid of the pg_ prefixes where we use pread(), pwrite() and vectored variants. We dropped support for ancient Unixes where we needed to use lseek() to implement replacements for those, but it turns out that Windows also changes the current position even when you pass in an offset to ReadFile() and WriteFile() if the file handle is synchronous, despite its documentation saying otherwise. Switching to asynchronous file handles would fix that, but have other complications. For now let's just put back the pg_ prefix and add some comments to highlight the non-standard side-effect, which we can now describe as Windows-only. Reported-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> Discussion: https://postgr.es/m/20220923202439.GA1156054%40nathanxps13	3 years ago
Robert Haas	a448e49bcb	Revert 56-bit relfilenode change and follow-up commits. ... There are still some alignment-related failures in the buildfarm, which might or might not be able to be fixed quickly, but I've also just realized that it increased the size of many WAL records by 4 bytes because a block reference contains a RelFileLocator. The effect of that hasn't been studied or discussed, so revert for now.	3 years ago
Robert Haas	05d4cbf9b6	Increase width of RelFileNumbers from 32 bits to 56 bits. ... RelFileNumbers are now assigned using a separate counter, instead of being assigned from the OID counter. This counter never wraps around: if all 2^56 possible RelFileNumbers are used, an internal error occurs. As the cluster is limited to 2^64 total bytes of WAL, this limitation should not cause a problem in practice. If the counter were 64 bits wide rather than 56 bits wide, we would need to increase the width of the BufferTag, which might adversely impact buffer lookup performance. Also, this lets us use bigint for pg_class.relfilenode and other places where these values are exposed at the SQL level without worrying about overflow. This should remove the need to keep "tombstone" files around until the next checkpoint when relations are removed. We do that to keep RelFileNumbers from being recycled, but now that won't happen anyway. However, this patch doesn't actually change anything in this area; it just makes it possible for a future patch to do so. Dilip Kumar, based on an idea from Andres Freund, who also reviewed some earlier versions of the patch. Further review and some wordsmithing by me. Also reviewed at various points by Ashutosh Sharma, Vignesh C, Amul Sul, Álvaro Herrera, and Tom Lane. Discussion: http://postgr.es/m/CA+Tgmobp7+7kmi4gkq7Y+4AM9fTvL+O1oQ4-5gFTT+6Ng-dQ=g@mail.gmail.com	3 years ago
Michael Paquier	e1e6f8f3df	Remove dependency to StringInfo in xlogbackup.{c.h} ... This was used as the returned result type of the generated contents for the backup_label and backup history files. This is replaced by a simple string, reducing the cleanup burden of all the callers of build_backup_content(). Reviewed-by: Bharath Rupireddy Discussion: https://postgr.es/m/YzERvNPaZivHEKZJ@paquier.xyz	3 years ago
Michael Paquier	7d708093b7	Refactor creation of backup_label and backup history files ... This change simplifies some of the logic related to the generation and creation of the backup_label and backup history files, which has become unnecessarily complicated since the removal of the exclusive backup mode in commit 39969e2. The code was previously generating the contents of these files as a string (start phase for the backup_label and stop phase for the backup history file), one problem being that the contents of the backup_label string were scanned to grab some of its internal contents at the stop phase. This commit changes the logic so as we store the data required to build these files in an intermediate structure named BackupState. The backup_label file and backup history file strings are generated when they are ready to be sent back to the client. Both files are now generated with the same code path. While on it, this commit renames some variables for clarity. Two new files named xlogbackup.{c,h} are introduced in this commit, to remove from xlog.c some of the logic around base backups. Note that more could be moved to this new set of files. Author: Bharath Rupireddy, Michael Paquier Reviewed-by: Fujii Masao Discussion: https://postgr.es/m/CALj2ACXWwTDgJqCjdaPyfR7djwm6SrybGcrZyrvojzcsmt4FFw@mail.gmail.com	3 years ago
Peter Eisentraut	26f7802beb	Message style improvements	3 years ago
Andres Freund	e6927270cd	meson: Add initial version of meson based build system ... Autoconf is showing its age, fewer and fewer contributors know how to wrangle it. Recursive make has a lot of hard to resolve dependency issues and slow incremental rebuilds. Our home-grown MSVC build system is hard to maintain for developers not using Windows and runs tests serially. While these and other issues could individually be addressed with incremental improvements, together they seem best addressed by moving to a more modern build system. After evaluating different build system choices, we chose to use meson, to a good degree based on the adoption by other open source projects. We decided that it's more realistic to commit a relatively early version of the new build system and mature it in tree. This commit adds an initial version of a meson based build system. It supports building postgres on at least AIX, FreeBSD, Linux, macOS, NetBSD, OpenBSD, Solaris and Windows (however only gcc is supported on aix, solaris). For Windows/MSVC postgres can now be built with ninja (faster, particularly for incremental builds) and msbuild (supporting the visual studio GUI, but building slower). Several aspects (e.g. Windows rc file generation, PGXS compatibility, LLVM bitcode generation, documentation adjustments) are done in subsequent commits requiring further review. Other aspects (e.g. not installing test-only extensions) are not yet addressed. When building on Windows with msbuild, builds are slower when using a visual studio version older than 2019, because those versions do not support MultiToolTask, required by meson for intra-target parallelism. The plan is to remove the MSVC specific build system in src/tools/msvc soon after reaching feature parity. However, we're not planning to remove the autoconf/make build system in the near future. Likely we're going to keep at least the parts required for PGXS to keep working around until all supported versions build with meson. Some initial help for postgres developers is at https://wiki.postgresql.org/wiki/Meson With contributions from Thomas Munro, John Naylor, Stone Tickle and others. Author: Andres Freund <andres@anarazel.de> Author: Nazir Bilal Yavuz <byavuz81@gmail.com> Author: Peter Eisentraut <peter@eisentraut.org> Reviewed-By: Peter Eisentraut <peter.eisentraut@enterprisedb.com> Discussion: https://postgr.es/m/20211012083721.hvixq4pnh2pixr3j@alap3.anarazel.de	3 years ago
Peter Geoghegan	a601366a46	Harmonize more parameter names in bulk. ... Make sure that function declarations use names that exactly match the corresponding names from function definitions in optimizer, parser, utility, libpq, and "commands" code, as well as in remaining library code. Do the same for all code related to frontend programs (with the exception of pg_dump/pg_dumpall related code). Like other recent commits that cleaned up function parameter names, this commit was written with help from clang-tidy. Later commits will handle ecpg and pg_dump/pg_dumpall. Author: Peter Geoghegan <pg@bowt.ie> Reviewed-By: David Rowley <dgrowleyml@gmail.com> Discussion: https://postgr.es/m/CAH2-WznJt9CMM9KJTMjJh_zbL5hD9oX44qdJ4aqZtjFi-zA3Tg@mail.gmail.com	3 years ago
Peter Eisentraut	a0b65155d0	Message style improvements	3 years ago
Michael Paquier	f352e2d08a	Simplify handling of compression level with compression specifications ... PG_COMPRESSION_OPTION_LEVEL is removed from the compression specification logic, and instead the compression level is always assigned with each library's default if nothing is directly given. This centralizes the checks on the compression methods supported by a given build, and always assigns a default compression level when parsing a compression specification. This results in complaining at an earlier stage than previously if a build supports a compression method or not, aka when parsing a specification in the backend or the frontend, and not when processing it. zstd, lz4 and zlib are able to handle in their respective routines setting up the compression level the case of a default value, hence the backend or frontend code (pg_receivewal or pg_basebackup) has now no need to know what the default compression level should be if nothing is specified: the logic is now done so as the specification parsing assigns it. It can also be enforced by passing down a "level" set to the default value, that the backend will accept (the replication protocol is for example able to handle a command like BASE_BACKUP (COMPRESSION_DETAIL 'gzip:level=-1')). This code simplification fixes an issue with pg_basebackup --gzip introduced by ffd5365, where the tarball of the streamed WAL segments would be created as of pg_wal.tar.gz with uncompressed contents, while the intention is to compress the segments with gzip at a default level. The origin of the confusion comes from the handling of the default compression level of gzip (-1 or Z_DEFAULT_COMPRESSION) and the value of 0 was getting assigned, which is what walmethods.c would consider as equivalent to no compression when streaming WAL segments with its tar methods. Assigning always the compression level removes the confusion of some code paths considering a value of 0 set in a specification as either no compression or a default compression level. Note that 010_pg_basebackup.pl has to be adjusted to skip a few tests where the shape of the compression detail string for client and server-side compression was checked using gzip. This is a result of the code simplification, as gzip specifications cannot be used if a build does not support it. Reported-by: Tom Lane Reviewed-by: Tom Lane Discussion: https://postgr.es/m/1400032.1662217889@sss.pgh.pa.us Backpatch-through: 15	3 years ago
Tom Lane	0a20ff54f5	Split up guc.c for better build speed and ease of maintenance. ... guc.c has grown to be one of our largest .c files, making it a bottleneck for compilation. It's also acquired a bunch of knowledge that'd be better kept elsewhere, because of our not very good habit of putting variable-specific check hooks here. Hence, split it up along these lines: * guc.c itself retains just the core GUC housekeeping mechanisms. * New file guc_funcs.c contains the SET/SHOW interfaces and some SQL-accessible functions for GUC manipulation. * New file guc_tables.c contains the data arrays that define the built-in GUC variables, along with some already-exported constant tables. * GUC check/assign/show hook functions are moved to the variable's home module, whenever that's clearly identifiable. A few hard- to-classify hooks ended up in commands/variable.c, which was already a home for miscellaneous GUC hook functions. To avoid cluttering a lot more header files with #include "guc.h", I also invented a new header file utils/guc_hooks.h and put all the GUC hook functions' declarations there, regardless of their originating module. That allowed removal of #include "guc.h" from some existing headers. The fallout from that (hopefully all caught here) demonstrates clearly why such inclusions are best minimized: there are a lot of files that, for example, were getting array.h at two or more levels of remove, despite not having any connection at all to GUCs in themselves. There is some very minor code beautification here, such as renaming a couple of inconsistently-named hook functions and improving some comments. But mostly this just moves code from point A to point B and deals with the ensuing needs for #include adjustments and exporting a few functions that previously weren't exported. Patch by me, per a suggestion from Andres Freund; thanks also to Michael Paquier for the idea to invent guc_funcs.c. Discussion: https://postgr.es/m/587607.1662836699@sss.pgh.pa.us	3 years ago
Peter Eisentraut	ab9717847a	Remove obsolete comment ... The comment in basebackup.c updated by 33bd4698c1 was actually obsolete to begin with, since the symbols it was referring to haven't existed in that header file for quite some time. The header file is still needed for other reasons, though, so keep the #include, just drop the comment.	3 years ago
David Rowley	f01592f915	Remove shadowed local variables that are new in v15 ... Compiling with -Wshadow=compatible-local yields quite a few warnings about local variables being shadowed by compatible local variables in an inner scope. Of course, this is perfectly valid in C, but we have had bugs in the past as a result of developers failing to notice this. af7d270dd is a recent example. Here we do a cleanup of warnings we receive from -Wshadow=compatible-local for code which is new to PostgreSQL 15. We've yet to have the discussion about if we actually ever want to run that as a standard compilation flag. We'll need to at least get the number of warnings down to something easier to manage before we can realistically consider if we want this or not. This commit is the first step towards reducing the warnings. The changes being made here are all fairly trivial. Because of that, and the fact that v15 is still in beta, this is being back-patched into 15. It seems more risky not to do this as the risk of future bugs is increased by the additional conflicts that this commit could cause for any future bug fixes touching the same areas as this commit. Author: Justin Pryzby Discussion: https://postgr.es/m/20220817145434.GC26426%40telsasoft.com Backpatch-through: 15	3 years ago
Robert Haas	a8c0128697	Move basebackup code to new directory src/backend/backup ... Reviewed by David Steele and Justin Pryzby Discussion: http://postgr.es/m/CA+TgmoafqboATDSoXHz8VLrSwK_MDhjthK4hEpYjqf9_1Fmczw%40mail.gmail.com	3 years ago