aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
* Be smarter about freeing tuples during tuplesortsDavid Rowley2022-09-01
| | | | | | | | | | | | | | | | | | | | | | During dumptuples() the call to writetuple() would pfree any non-null tuple. This was quite wasteful as this happens just before we perform a reset of the context which stores all of those tuples. It seems to make sense to do a bit of a code refactor to make this work, so here we just get rid of the writetuple function and adjust the WRITETUP macro to call the state's writetup function. The WRITETUP usage in mergeonerun() always has state->slabAllocatorUsed == true, so writetuple() would never free the tuple or do any memory accounting. The only call path that needs memory accounting done is in dumptuples(), so let's just do it manually there. In passing, let's get rid of the state->memtupcount-- code that counts the memtupcount down to 0 one tuple at a time inside the loop. That seems to be a rather inefficient way to set memtupcount to 0, so let's just zero it after the loop instead. Author: David Rowley Discussion: https://postgr.es/m/CAApHDvqZXoDCyrfCzZJR0-xH+7_q+GgitcQiYXUjRani7h4j8Q@mail.gmail.com
* Prevent long-term memory leakage in autovacuum launcher.Tom Lane2022-08-31
| | | | | | | | | | | | | | | | | | | | | get_database_list() failed to restore the caller's memory context, instead leaving current context set to TopMemoryContext which is how CommitTransactionCommand() leaves it. The callers both think they are using short-lived contexts, for the express purpose of not having to worry about cleaning up individual allocations. The net effect therefore is that supposedly short-lived allocations could accumulate indefinitely in the launcher's TopMemoryContext. Although this has been broken for a long time, it seems we didn't have any obvious memory leak here until v15's rearrangement of the stats logic. I (tgl) am not entirely convinced that there's no other leak at all, though, and we're surely at risk of adding one in future back-patched fixes. So back-patch to all supported branches, even though this may be only a latent bug in pre-v15. Reid Thompson Discussion: https://postgr.es/m/972a4e12b68b0f96db514777a150ceef7dcd2e0f.camel@crunchydata.com
* Derive freeze cutoff from nextXID, not OldestXmin.Peter Geoghegan2022-08-31
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Before now, the cutoffs that VACUUM used to determine which XIDs/MXIDs to freeze were determined at the start of each VACUUM by taking related cutoffs that represent which XIDs/MXIDs VACUUM should treat as still running, and subtracting an XID/MXID age based value controlled by GUCs like vacuum_freeze_min_age. The FreezeLimit cutoff (XID freeze cutoff) was derived by subtracting an XID age value from OldestXmin, while the MultiXactCutoff cutoff (MXID freeze cutoff) was derived by subtracting an MXID age value from OldestMxact. This approach didn't match the approach used nearby to determine whether this VACUUM operation should be an aggressive VACUUM or not. VACUUM now uses the standard approach instead: it subtracts the same age-based values from next XID/next MXID (rather than subtracting from OldestXmin/OldestMxact). This approach is simpler and more uniform. Most of the time it will have only a negligible impact on how and when VACUUM freezes. It will occasionally make VACUUM more robust in the event of problems caused by long running transaction. These are cases where OldestXmin and OldestMxact are held back by so much that they attain an age that is a significant fraction of the value of age-based settings like vacuum_freeze_min_age. There is no principled reason why freezing should be affected in any way by the presence of a long-running transaction -- at least not before the point that the OldestXmin and OldestMxact limits used by each VACUUM operation attain an age that makes it unsafe to freeze some of the XIDs/MXIDs whose age exceeds the value of the relevant age-based settings. The new approach should at least make freezing degrade more gracefully than before, even in the most extreme cases. Author: Peter Geoghegan <pg@bowt.ie> Reviewed-By: Nathan Bossart <nathandbossart@gmail.com> Reviewed-By: Matthias van de Meent <boekewurm+postgres@gmail.com> Discussion: https://postgr.es/m/CAH2-WzkOv5CEeyOO=c91XnT5WBR_0gii0Wn5UbZhJ=4TTykDYg@mail.gmail.com
* Fix MSVC warning in compat_informix/rnull.pgcAndres Freund2022-08-31
| | | | | | | | | | | Building the ecpg tests with MSVC, with warnings enabled, results in the following warning: src/interfaces/ecpg/test/compat_informix/rnull.pgc(19,1): warning C4305: 'initializing': truncation from 'double' to 'float' The more obvious fix would be an 'f' suffix, but ecpg can't parse that. Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com> Discussion: https://postgr.es/m/2180a97c-c026-1b6c-cec8-d6e499f97017@enterprisedb.com
* In the Snowball dictionary, don't try to stem excessively-long words.Tom Lane2022-08-31
| | | | | | | | | | | | | | | | | | | | | If the input word exceeds 1000 bytes, don't pass it to the stemmer; just return it as-is after case folding. Such an input is surely not a word in any human language, so whatever the stemmer might do to it would be pretty dubious in the first place. Adding this restriction protects us against a known recursion-to-stack-overflow problem in the Turkish stemmer, and it seems like good insurance against any other safety or performance issues that may exist in the Snowball stemmers. (I note, for example, that they contain no CHECK_FOR_INTERRUPTS calls, so we really don't want them running for a long time.) The threshold of 1000 bytes is arbitrary. An alternative definition could have been to treat such words as stopwords, but that seems like a bigger break from the old behavior. Per report from Egor Chindyaskin and Alexander Lakhin. Thanks to Olly Betts for the recommendation to fix it this way. Discussion: https://postgr.es/m/1661334672.728714027@f473.i.mail.ru
* Fix a bug in roles_is_member_of.Robert Haas2022-08-31
| | | | | | | | | | | | | | Commit e3ce2de09d814f8770b2e3b3c152b7671bcdb83f rearranged this function to be able to identify which inherited role had admin option on the target role, but it got the order of operations wrong, causing the function to return wrong answers in the presence of non-inherited grants. Fix that, and add a test case that verifies the correct behavior. Patch by me, reviewed by Nathan Bossart Discussion: http://postgr.es/m/CA+TgmoYamnu-xt-u7CqjYWnRiJ6BQaSpYOHXP=r4QGTfd1N_EA@mail.gmail.com
* Refactor check_ functions to use filehandle for statusDaniel Gustafsson2022-08-31
| | | | | | | | | | | | | | | When reporting failure in check_ functions there is (typically) a text- file mentioned in the error report which contains further details. Some check_ functions kept a separate flag variable to indicate failure, and some just checked the state of the filehandle as it's guaranteed to be open when the check failed. This refactors the functions to consistently do the same check on error reporting. As the error report contains the filepath, it makes more sense to check the filehandle state and skip the flag variable. Reviewed-by: Nathan Bossart <nathandbossart@gmail.com> Reviewed-by: Bruce Momjian <bruce@momjian.us> Discussion: https://postgr.es/m/595759F6-625B-4ED7-8125-91AF00437F83@yesql.se
* plpython: Don't create pgxsdir subdirectory in installdir targetPeter Eisentraut2022-08-31
| | | | | | As of db23464715f4792298c639153dda7bfd9ad9d602, we don't install anything there anymore from plpython, so we don't need to create the installation directory anymore.
* On NetBSD, force dynamic symbol resolution at postmaster start.Tom Lane2022-08-30
| | | | | | | | | | | | | | | | | | | | | | | | | | | | The default of lazy symbol resolution means that when the postmaster first reaches the select() call in ServerLoop, it'll need to resolve the link to that libc entry point. NetBSD's dynamic loader takes an internal lock while doing that, and if a signal interrupts the operation then there is a risk of self-deadlock should the signal handler do anything that requires that lock, as several of the postmaster signal handlers do. The window for this is pretty narrow, and timing considerations make it unlikely that a signal would arrive right then anyway. But it's semi-repeatable on slow single-CPU machines, and in principle the race could happen with any hardware. The least messy solution to this is to force binding of dynamic symbols at postmaster start, using the "-z now" linker option. While we're at it, also use "-z relro" so as to provide a small security gain. It's not entirely clear whether any other platforms share this issue, but for now we'll assume it's NetBSD-specific. (We might later try to use "-z now" on more platforms for performance reasons, but that would not likely be something to back-patch.) Report and patch by me; the idea to fix it this way is from Andres Freund. Discussion: https://postgr.es/m/3384826.1661802235@sss.pgh.pa.us
* Various cleanups of the new memory context header codeDavid Rowley2022-08-31
| | | | | | | | | | | | | | | | | | | | | | | Robert Haas reported that his older clang compiler didn't like the two Asserts which were verifying that the given MemoryContextMethodID was <= MEMORY_CONTEXT_METHODID_MASK when building with -Wtautological-constant-out-of-range-compare. In my (David's) opinion, the compiler is wrong to warn about that. Newer versions of clang don't warn about the out of range enum value, so perhaps this was a bug that has now been fixed. To keep older clang versions happy, let's just cast the enum value to int to stop the compiler complaining. The main reason for the Asserts mentioned above to exist are to inform future developers which are adding new MemoryContexts if they run out of bit space in MemoryChunk to store the MemoryContextMethodID. As pointed out by Tom Lane, it seems wise to also add a comment to the header for that enum to document the restriction on these enum values. Additionally, also fix an incorrect usage of UINT64CONST() which was introduced in c6e0fe1f2. Author: Robert Haas, David Rowley Discussion: https://postgr.es/m/CA+TgmoYGG2C7Vbw1cjkQRRBL3zOk8SmhrQnsJgzscX=N9AwPrw@mail.gmail.com
* Revert "Add missing padding from MemoryChunk struct"David Rowley2022-08-31
| | | | | | | | | | | This reverts commit df0f4feef. It turns out the problem which was causing the 32-bit ARM and PPC animals to fail was due to a MAXALIGN problem in slab.c. This was fixed by d5ee4db0e. The padding that was added in df0f4feef would only do anything on machines where uint64 was not aligned to 8 bytes. The 32-bit machines which were failing are not in that category, so revert this commit. Discussion: https://postgr.es/m/3209100.1661787561@sss.pgh.pa.us
* Update the comment in rmgrlist.h to match it to the code.Amit Kapila2022-08-30
| | | | | | Author: Hayato Kuroda Reviwed-by: Amit Kapila Discussion: https://postgr.es/m/TYAPR01MB58665F20F412EDF27B0759CFF5769@TYAPR01MB5866.jpnprd01.prod.outlook.com
* Drop replication origin slots before tablesync worker exits.Amit Kapila2022-08-30
| | | | | | | | | | | | | | | | | | | Currently, the replication origin tracking of the tablesync worker is dropped by the apply worker. So, there will be a small lag between the tablesync worker exit and its origin tracking got removed. In the meantime, new tablesync workers can be launched and will try to set up a new origin tracking. This can lead the system to reach max configured limit (max_replication_slots) even if the user has configured the max limit considering the number of tablesync workers required in the system. We decided not to back-patch as this can occur in very narrow circumstances and users have to option to increase the configured limit by increasing max_replication_slots. Reported-by: Hubert Depesz Lubaczewski Author: Ajin Cherian Reviwed-by: Masahiko Sawada, Peter Smith, Hou Zhijie, Amit Kapila Discussion: https://postgr.es/m/20220714115155.GA5439@depesz.com
* Further code review of port/simd.hJohn Naylor2022-08-30
| | | | | | | | | Add missing declaration per existing style, and fix a couple typos. Nathan Bossart and Julien Rouhaud Discussion: https://www.postgresql.org/message-id/20220829171712.GA509233%40nathanxps13 Discussion: https://www.postgresql.org/message-id/20220830022636.qrcbcecmhztbxrwa%40jrouhaud
* Adjust comments that called MultiXactIds "XMIDs".Peter Geoghegan2022-08-29
| | | | Oversights in commits 0b018fab and f3c15cbe.
* Use MAXALIGN() in calculations using sizeof(SlabBlock)David Rowley2022-08-30
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | c6e0fe1f2 added a new pointer field to SlabBlock to make it 4 bytes larger on 32-bit machines. Prior to that commit, the size of that struct was a multiple of 8, which meant that MAXALIGN(sizeof(SlabBlock)) was the same as sizeof(SlabBlock), however, after c6e0fe1f2, due to the addition of the new pointer field to store a pointer to the owning context, that was no longer true on builds with sizeof(void *) == 4. This problem was highlighted by an Assert failure which was checking that the pointer given to pfree() was MAXALIGNED. Various 32-bit ARM buildfarm animals were failing. These have MAXIMUM_ALIGNOF of 8. The only 32-bit testing I'd managed to do on c6e0fe1f2 had been on x86, which has a MAXIMUM_ALIGNOF of 4, therefore did not exhibit this issue. Here we define Slab_BLOCKHDRSZ and copy what is being done in aset.c and generation.c for doing calculations based on the size of the context's block type. This means that SlabAlloc() will now always return a MAXALIGNed pointer. This also fixes an incorrect sentinel_ok() check in SlabCheck() which was incorrectly checking the wrong sentinel byte. This must have previously not caused any issues due to the fullChunkSize never being large enough to store the sentinel byte. Diagnosed-by: Tomas Vondra, Tom Lane Author: Tomas Vondra, David Rowley Discussion: https://postgr.es/m/CAA4eK1%2B1JyW5TiL%3DyV-3Uq1CrfnTyn0Xrk5uArt31Z%3D8rgPhXQ%40mail.gmail.com
* Cleanup more code and comments related to Windows NT4 (XP days)Michael Paquier2022-08-30
| | | | | | | | | | | All the code and comments cleaned up here is irrelevant since 495ed0e. Note that this removes an assumption that CreateRestrictedToken() may not exist, something that could have happened when running under Windows NT as the code stated. Rather than assuming that it may not exist, this causes pg_ctl to fail hard if the function cannot be loaded. Reported-by: Justin Pryzby Discussion: https://postgr.es/m/20220826112637.GD2342@telsasoft.com
* Clean up inconsistent use of fflush().Tom Lane2022-08-29
| | | | | | | | | | | | | | | | | | | | | | More than twenty years ago (79fcde48b), we hacked the postmaster to avoid a core-dump on systems that didn't support fflush(NULL). We've mostly, though not completely, hewed to that rule ever since. But such systems are surely gone in the wild, so in the spirit of cleaning out no-longer-needed portability hacks let's get rid of multiple per-file fflush() calls in favor of using fflush(NULL). Also, we were fairly inconsistent about whether to fflush() before popen() and system() calls. While we've received no bug reports about that, it seems likely that at least some of these call sites are at risk of odd behavior, such as error messages appearing in an unexpected order. Rather than expend a lot of brain cells figuring out which places are at hazard, let's just establish a uniform coding rule that we should fflush(NULL) before these calls. A no-op fflush() is surely of trivial cost compared to launching a sub-process via a shell; while if it's not a no-op then we likely need it. Discussion: https://postgr.es/m/2923412.1661722825@sss.pgh.pa.us
* Prevent WAL corruption after a standby promotion.Robert Haas2022-08-29
| | | | | | | | | | | | | | | | | | | | | | | | | | | When a PostgreSQL instance performing archive recovery but not using standby mode is promoted, and the last WAL segment that it attempted to read ended in a partial record, the previous code would create invalid WAL on the new timeline. The WAL from the previously timeline would be copied to the new timeline up until the end of the last valid record, but instead of beginning to write WAL at immediately afterwards, the promoted server would write an overwrite contrecord at the beginning of the next segment. The end of the previous segment would be left as all-zeroes, resulting in failures if anything tried to read WAL from that file. The root of the issue is that ReadRecord() decides whether to set abortedRecPtr and missingContrecPtr based on the value of StandbyMode, but ReadRecord() switches to a new timeline based on the value of ArchiveRecoveryRequested. We shouldn't try to write an overwrite contrecord if we're switching to a new timeline, so change the test in ReadRecod() to check ArchiveRecoveryRequested instead. Code fix by Dilip Kumar. Comments by me incorporating suggested language from Álvaro Herrera. Further review from Kyotaro Horiguchi and Sami Imseih. Discussion: http://postgr.es/m/CAFiTN-t7umki=PK8dT1tcPV=mOUe2vNhHML6b3T7W7qqvvajjg@mail.gmail.com Discussion: http://postgr.es/m/FB0DEA0B-E14E-43A0-811F-C1AE93D00FF3%40amazon.com
* Add missing padding from MemoryChunk structDavid Rowley2022-08-29
| | | | | | | | | | | | | | | Buildfarm animals skate, grison and mamba are Assert failing on the pointer being given to repalloc not being MAXALIGNED. c6e0fe1f2a made changes in that area. All of these animals are 32-bit with a MAXIMUM_ALIGNOF of 8 and a SIZEOF_VOID_P of 4. I suspect that the pointer is not properly aligned due to the lack of padding in the MemoryChunk struct. Here we add the same type of padding that was previously used in AllocChunkData and GenerationChunk that c6e0fe1f2a neglected to add. Discussion: https://postgr.es/m/CAA4eK1%2B1JyW5TiL%3DyV-3Uq1CrfnTyn0Xrk5uArt31Z%3D8rgPhXQ%40mail.gmail.com
* Fix broken cast on MSVCJohn Naylor2022-08-29
| | | | | | | | Per buildfarm animal drongo, casting a vector type to the same type causes a compile error. We still need the cast on ARM64, so invent a wrapper function that does the casting only where necessary. Discussion: https://www.postgresql.org/message-id/CAFBsxsEouaTwbmpqV%2BEW2%3DwFbhw2vHRe26NQTRcd0%3DNaOFDy7A%40mail.gmail.com
* Use ARM Advanced SIMD (NEON) intrinsics where availableJohn Naylor2022-08-29
| | | | | | | | | | | | | | NEON support is required on the Aarch64 architecture for standard implementations. Hardware designers for specialized markets can choose not to support it, but that's true of floating point as well, which we assume is supported. As with x86, some SIMD support is available on 32-bit platforms, but those are not interesting from a performance standpoint and would require an inconvenient runtime check. Nathan Bossart Reviewed by John Naylor, Andres Freund, Thomas Munro, and Tom Lane Discussion: https://www.postgresql.org/message-id/flat/CAFBsxsEyR9JkfbPcDXBRYEfdfC__OkwVGdwEAgY4Rv0cvw35EA%40mail.gmail.com#aba7a64b11503494ffd8dd27067626a9
* Abstract some more architecture-specific details away from SIMD functionalityJohn Naylor2022-08-29
| | | | | | | | | | | | Add a typedef to represent vectors containing four 32-bit integers, and add functions operating on them. Also separate out saturating subtraction into its own function. The motivation for this is to prepare for a future commit to add ARM NEON support. Nathan Bossart Reviewed by John Naylor and Tom Lane Discussion: https://www.postgresql.org/message-id/flat/CAFBsxsEyR9JkfbPcDXBRYEfdfC__OkwVGdwEAgY4Rv0cvw35EA%40mail.gmail.com#aba7a64b11503494ffd8dd27067626a9
* Improve performance of and reduce overheads of memory managementDavid Rowley2022-08-29
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Whenever we palloc a chunk of memory, traditionally, we prefix the returned pointer with a pointer to the memory context to which the chunk belongs. This is required so that we're able to easily determine the owning context when performing operations such as pfree() and repalloc(). For the AllocSet context, prior to this commit we additionally prefixed the pointer to the owning context with the size of the chunk. This made the header 16 bytes in size. This 16-byte overhead was required for all AllocSet allocations regardless of the allocation size. For the generation context, the problem was worse; in addition to the pointer to the owning context and chunk size, we also stored a pointer to the owning block so that we could track the number of freed chunks on a block. The slab allocator had a 16-byte chunk header. The changes being made here reduce the chunk header size down to just 8 bytes for all 3 of our memory context types. For small to medium sized allocations, this significantly increases the number of chunks that we can fit on a given block which results in much more efficient use of memory. Additionally, this commit completely changes the rule that pointers to palloc'd memory must be directly prefixed by a pointer to the owning memory context and instead, we now insist that they're directly prefixed by an 8-byte value where the least significant 3-bits are set to a value to indicate which type of memory context the pointer belongs to. Using those 3 bits as an index (known as MemoryContextMethodID) to a new array which stores the methods for each memory context type, we're now able to pass the pointer given to functions such as pfree() and repalloc() to the function specific to that context implementation to allow them to devise their own methods of finding the memory context which owns the given allocated chunk of memory. The reason we're able to reduce the chunk header down to just 8 bytes is because of the way we make use of the remaining 61 bits of the required 8-byte chunk header. Here we also implement a general-purpose MemoryChunk struct which makes use of those 61 remaining bits to allow the storage of a 30-bit value which the MemoryContext is free to use as it pleases, and also the number of bytes which must be subtracted from the chunk to get a reference to the block that the chunk is stored on (also 30 bits). The 1 additional remaining bit is to denote if the chunk is an "external" chunk or not. External here means that the chunk header does not store the 30-bit value or the block offset. The MemoryContext can use these external chunks at any time, but must use them if any of the two 30-bit fields are not large enough for the value(s) that need to be stored in them. When the chunk is marked as external, it is up to the MemoryContext to devise its own means to determine the block offset. Using 3-bits for the MemoryContextMethodID does mean we're limiting ourselves to only having a maximum of 8 different memory context types. We could reduce the bit space for the 30-bit value a little to make way for more than 3 bits, but it seems like it might be better to do that only if we ever need more than 8 context types. This would only be a problem if some future memory context type which does not use MemoryChunk really couldn't give up any of the 61 remaining bits in the chunk header. With this MemoryChunk, each of our 3 memory context types can quickly obtain a reference to the block any given chunk is located on. AllocSet is able to find the context to which the chunk is owned, by first obtaining a reference to the block by subtracting the block offset as is stored in the 'hdrmask' field and then referencing the block's 'aset' field. The Generation context uses the same method, but GenerationBlock did not have a field pointing back to the owning context, so one is added by this commit. In aset.c and generation.c, all allocations larger than allocChunkLimit are stored on dedicated blocks. When there's just a single chunk on a block like this, it's easy to find the block from the chunk, we just subtract the size of the block header from the chunk pointer. The size of these chunks is also known as we store the endptr on the block, so we can just subtract the pointer to the allocated memory from that. Because we can easily find the owning block and the size of the chunk for these dedicated blocks, we just always use external chunks for allocation sizes larger than allocChunkLimit. For generation.c, this sidesteps the problem of non-external MemoryChunks being unable to represent chunk sizes >= 1GB. This is less of a problem for aset.c as we store the free list index in the MemoryChunk's spare 30-bit field (the value of which will never be close to using all 30-bits). We can easily reverse engineer the chunk size from this when needed. Storing this saves AllocSetFree() from having to make a call to AllocSetFreeIndex() to determine which free list to put the newly freed chunk on. For the slab allocator, this commit adds a new restriction that slab chunks cannot be >= 1GB in size. If there happened to be any users of slab.c which used chunk sizes this large, they really should be using AllocSet instead. Here we also add a restriction that normal non-dedicated blocks cannot be 1GB or larger. It's now not possible to pass a 'maxBlockSize' >= 1GB during the creation of an AllocSet or Generation context. Allocations can still be larger than 1GB, it's just these will always be on dedicated blocks (which do not have the 1GB restriction). Author: Andres Freund, David Rowley Discussion: https://postgr.es/m/CAApHDvpjauCRXcgcaL6+e3eqecEHoeRm9D-kcbuvBitgPnW=vw@mail.gmail.com
* Fix the incorrect assertion introduced in commit 7f13ac8123.Amit Kapila2022-08-29
| | | | | | | | | | | | | It has been incorrectly assumed in commit 7f13ac8123 that we can either purge all or none in the catalog modifying xids list retrieved from a serialized snapshot. It is quite possible that some of the xids in that array are old enough to be pruned but not others. As per buildfarm Author: Amit Kapila and Masahiko Sawada Reviwed-by: Masahiko Sawada Discussion: https://postgr.es/m/CAA4eK1LBtv6ayE+TvCcPmC-xse=DVg=SmbyQD1nv_AaqcpUJEg@mail.gmail.com
* Add more detail why repalloc and pfree do not accept NULL pointersPeter Eisentraut2022-08-28
| | | | | | | Per discussion, we choose not to change this. This just gives a little bit more information. Discussion: https://www.postgresql.org/message-id/flat/cf26e970-8e92-59f1-247a-aa265235075b%40enterprisedb.com
* Enable RandomizedBaseAddress (ASLR) on Windows with MSVC buildsMichael Paquier2022-08-28
| | | | | | | | | | | | | | | | | | | | | | | | This has as effect to add /DYNAMICBASE to the .dll and .exe files generated by the builds, undoing 7f3e17b. Note that ASLR was already enabled in MinGW as we have never added --disable-dynamicbase there. This change will ease a bit the integration of arm64 with MSVC, as ASLR support is mandatory in this case. So, thanks to this commit, we have no need to make ASLR conditional depending on the architecture used for the build. Andres Freund has done a lot of testing with this option while working on meson, without seeing /DYNAMICBASE as being a problem in the Windows builds of the CI. Personally, not supporting anything older than Windows 10 on HEAD makes me feel safer about this change, as we have seen ASLR with being a problem in process invocation particularly with Windows 8 and server 2012 back in 2014, even if Windows 10 was not really a thing back then. 45e004f is also something that can help in making the process invocation more stable. We are very early in the development of Postgres 16, giving a lot of room to detect stability issues if any. Discussion: https://postgr.es/m/20220826012907.gjw3jdqdgsts5y65@awork3.anarazel.de
* Doc: add comment about bug fixed in back branches as of 3f7323cbb.Tom Lane2022-08-27
| | | | | | | | | While the bug I just fixed in the back branches doesn't exist in HEAD, the requirement that MULTIEXPR SubPlans not share output parameters still does. Add a comment to memorialize that, because perhaps it could be an issue again someday. Discussion: https://postgr.es/m/17596-c5357f61427a81dc@postgresql.org
* Fix typo in comment for writetuple() functionAlexander Korotkov2022-08-27
| | | | | Reported-by: David Rowley Discussion: https://postgr.es/m/CAApHDvrZ9Ky2LcWwcKsbdYChA850JE5qS%3DkGJiTNWS8mbBXZHw%40mail.gmail.com
* Be more careful to avoid including system headers after perl.hJohn Naylor2022-08-27
| | | | | | | | | | | | | | | | Commit 121d2d3d70 included simd.h into pg_wchar.h. This caused a problem on Windows, since Perl has "#define free" (referring to globals), which breaks the Windows' header. To fix, move the static inline function definitions from plperl_helpers.h, into plperl.h, where we already document the necessary inclusion order. Since those functions were the only reason for the existence of plperl_helpers.h, remove it. First reported by Justin Pryzby Diagnosis and review by Andres Freund, patch by myself per suggestion from Tom Lane Discussion: https://www.postgresql.org/message-id/20220826115546.GE2342%40telsasoft.com
* Use correct connection for cancellation in frontend's parallel slotsMichael Paquier2022-08-27
| | | | | | | | | | | | | | While waiting for slots to become available in wait_on_slots() in parallel_slot.c, the cancellation always relied on the first connection in the set to do the job. This could cause problems when this slot's socket is gone as PQgetCancel() would return NULL in this case. Rather than always using the first connection, this changes the logic to use the first valid connection for the cancellation. Author: Ranier Vilela Reviewed-by: Justin Pryzby Discussion: https://postgr.es/m/CAEudQAokk1h_pUwGXsYS4oVOuf35s1O2o3TXGHpV8=AWikvgHA@mail.gmail.com Backpatch-through: 14
* Remove unneeded null pointer checks before PQfreemem()Peter Eisentraut2022-08-26
| | | | | | | | PQfreemem() just calls free(), and the latter already checks for null pointers. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/cf26e970-8e92-59f1-247a-aa265235075b%40enterprisedb.com
* Remove unnecessary casts in free() and pfree()Peter Eisentraut2022-08-26
| | | | | Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/cf26e970-8e92-59f1-247a-aa265235075b%40enterprisedb.com
* Use SSE2 in is_valid_ascii() where available.John Naylor2022-08-26
| | | | | | | | | | | | | Per flame graph from Jelte Fennema, COPY FROM ... USING BINARY shows input validation taking at least 5% of the profile, so it's worth trying to be more efficient here. With this change, validation of pure ASCII is nearly 40% faster on contemporary Intel hardware. To make this change legible and easier to adopt to additional architectures, use helper functions to abstract the platform details away. Reviewed by Nathan Bossart Discussion: https://www.postgresql.org/message-id/CAFBsxsG%3Dk8t%3DC457FXnoBXb%3D8iA4OaZkbFogFMachWif7mNnww%40mail.gmail.com
* Remove obsolete commentPeter Eisentraut2022-08-26
| | | | | | | | The comment in basebackup.c updated by 33bd4698c11 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.
* Fix typo in comment.Etsuro Fujita2022-08-26
|
* Add optimized functions for linear search within byte arraysJohn Naylor2022-08-26
| | | | | | | | | | | In similar vein to b6ef167564, add pg_lfind8() and pg_lfind8_le() to search for bytes equal or less-than-or-equal to a given byte, respectively. To abstract away platform details, add helper functions and typedefs to simd.h. John Naylor and Nathan Bossart, per suggestion from Andres Freund Discussion: https://www.postgresql.org/message-id/CAFBsxsGzaaGLF%3DNuq61iRXTyspbO9rOjhSqFN%3DV6ozzmta5mXg%40mail.gmail.com
* Remove configure probe for sockaddr_in6 and require AF_INET6.Thomas Munro2022-08-26
| | | | | | | | | | | | | | | | | SUSv3 <netinet/in.h> defines struct sockaddr_in6, and all targeted Unix systems have it. Windows has it in <ws2ipdef.h>. Remove the configure probe, the macro and a small amount of dead code. Also remove a mention of IPv6-less builds from the documentation, since there aren't any. This is similar to commits f5580882 and 077bf2f2 for Unix sockets. Even though AF_INET6 is an "optional" component of SUSv3, there are no known modern operating system without it, and it seems even less likely to be omitted from future systems than AF_UNIX. Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/CA+hUKGKErNfhmvb_H0UprEmp4LPzGN06yR2_0tYikjzB-2ECMw@mail.gmail.com
* libpq code should use libpq_gettext(), not _()Peter Eisentraut2022-08-25
| | | | Fix some wrong use and install a safeguard against future mistakes.
* Small refactor to get rid of -Wshadow=compatible-local warningDavid Rowley2022-08-26
| | | | | | | | | | Further reduce -Wshadow=compatible-local warnings by 1 by refactoring the code in gistRelocateBuildBuffersOnSplit() to make use of foreach_current_index() instead of manually incrementing a variable on each loop. Author: David Rowley Discussion: https://postgr.es/m/CAApHDvpGZX-X=Bn4moyXgfFa0CdSUwoa04d3isit3=1qo8F8Bw@mail.gmail.com
* More -Wshadow=compatible-local warning fixesDavid Rowley2022-08-26
| | | | | | | | | | | | In a similar effort to f01592f91, here we're targetting fixing the warnings where we've deemed the shadowing variable to serve a close enough purpose to the shadowed variable just to reuse the shadowed version and not declare the shadowing variable at all. By my count, this takes the warning count from 106 down to 71. Author: Justin Pryzby Discussion: https://postgr.es/m/20220825020839.GT2342@telsasoft.com
* Allow grant-level control of role inheritance behavior.Robert Haas2022-08-25
| | | | | | | | | | | | | | | | | | | | The GRANT statement can now specify WITH INHERIT TRUE or WITH INHERIT FALSE to control whether the member inherits the granted role's permissions. For symmetry, you can now likewise write WITH ADMIN TRUE or WITH ADMIN FALSE to turn ADMIN OPTION on or off. If a GRANT does not specify WITH INHERIT, the behavior based on whether the member role is marked INHERIT or NOINHERIT. This means that if all roles are marked INHERIT or NOINHERIT before any role grants are performed, the behavior is identical to what we had before; otherwise, it's different, because ALTER ROLE [NO]INHERIT now only changes the default behavior of future grants, and has no effect on existing ones. Patch by me. Reviewed and testing by Nathan Bossart and Tushar Ahuja, with design-level comments from various others. Discussion: http://postgr.es/m/CA+Tgmoa5Sf4PiWrfxA=sGzDKg0Ojo3dADw=wAHOhR9dggV=RmQ@mail.gmail.com
* Move NON_EXEC_STATIC from c.h to postgres.hPeter Eisentraut2022-08-25
| | | | | | | It is not needed at the scope of c.h, only in backend code. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/a6a6b48e-ca0a-b58d-18de-98e40d94b842%40enterprisedb.com
* Update another comment still referring to pg_start/stop_backup()Peter Eisentraut2022-08-25
|
* Fix typo in MVCC test commentDaniel Gustafsson2022-08-25
| | | | | | | | The optimization is named kill_prior_tuple but was accidentally spelled kill_prio_tuple in the test. Author: Mingli Zhang <avamingli@gmail.com> Discussion: https://postgr.es/m/82d3e66a-d8ae-4bfa-943e-29c5add0743f@Spark
* Remove unused symbol __aarch64John Naylor2022-08-25
| | | | | | | | | This was added as a possible variant of __aarch64__ back when 64-bit ARM was vaporware. It hasn't shown up in the wild since then, so remove. Nathan Bossart Discussion: https://www.postgresql.org/message-id/CAFBsxsEN5nW3uRh%3Djrs-QexDrC1btu0ZfriD3FFfb%3D3J6tAngg%40mail.gmail.com
* pg_dump: Fix new ICU testsPeter Eisentraut2022-08-25
| | | | | ICU doesn't support some server encodings, so we need to exclude them if a non-supported encoding was set up.
* aix: Fix SHLIB_EXPORTS reference in VPATH buildsAndres Freund2022-08-24
| | | | | | | | | The dependencies here aren't quite right independent of vpath builds or not, but this at least makes vpath builds succeed. And it's pretty rare to change the exports.txt file anyway... The referenced thread has a patch that will clean that up further. Discussion: https://postgr.es/m/20220820174213.d574qde4ptwdzoqz@awork3.anarazel.de
* Remove SUBSYS.o rule in common.mk, hasn't been used in a long timeAndres Freund2022-08-24
| | | | | | | | Apparently I missed that this SUBSYS.o rule isn't needed anymore in a4ebbd27527, likely because there still is a reference to it due to AIX - but that's self contained in src/backend/Makefile Discussion: https://postgr.es/m/20220820174213.d574qde4ptwdzoqz@awork3.anarazel.de
* Remove rule to generate postgres.o, not needed for 20+ yearsAndres Freund2022-08-24
| | | | Discussion: https://postgr.es/m/20220820174213.d574qde4ptwdzoqz@awork3.anarazel.de