aboutsummaryrefslogtreecommitdiff
path: root/src/backend/utils
Commit message (Collapse)AuthorAge
* 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
* 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
* Fix typo in comment for writetuple() functionAlexander Korotkov2022-08-27
| | | | | Reported-by: David Rowley Discussion: https://postgr.es/m/CAApHDvrZ9Ky2LcWwcKsbdYChA850JE5qS%3DkGJiTNWS8mbBXZHw%40mail.gmail.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
* 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
* 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
* Defend against stack overrun in a few more places.Tom Lane2022-08-24
| | | | | | | | | | | | | | | | | | | SplitToVariants() in the ispell code, lseg_inside_poly() in geo_ops.c, and regex_selectivity_sub() in selectivity estimation could recurse until stack overflow; fix by adding check_stack_depth() calls. So could next() in the regex compiler, but that case is better fixed by converting its tail recursion to a loop. (We probably get better code that way too, since next() can now be inlined into its sole caller.) There remains a reachable stack overrun in the Turkish stemmer, but we'll need some advice from the Snowball people about how to fix that. Per report from Egor Chindyaskin and Alexander Lakhin. These mistakes are old, so back-patch to all supported branches. Richard Guo and Tom Lane Discussion: https://postgr.es/m/1661334672.728714027@f473.i.mail.ru
* Further -Wshadow=compatible-local warning fixesDavid Rowley2022-08-24
| | | | | | | | | | | | | These should have been included in 421892a19 as these shadowed variable warnings can also be fixed by adjusting the scope of the shadowed variable to put the declaration for it in an inner scope. This is part of the same effort as f01592f91. By my count, this takes the warning count from 114 down to 106. Author: David Rowley and Justin Pryzby Discussion: https://postgr.es/m/CAApHDvrwLGBP%2BYw9vriayyf%3DXR4uPWP5jr6cQhP9au_kaDUhbA%40mail.gmail.com
* Allow parallel workers to retrieve some data from PortMichael Paquier2022-08-24
| | | | | | | | | | | | | | | | | | | | | | | | | This commit moves authn_id into a new global structure called ClientConnectionInfo (mapping to a MyClientConnectionInfo for each backend) which is intended to hold all the client information that should be shared between the backend and any of its parallel workers, access for extensions and triggers being the primary use case. There is no need to push all the data of Port to the workers, and authn_id is quite a generic concept so using a separate structure provides the best balance (the name of the structure has been suggested by Robert Haas). While on it, and per discussion as this would be useful for a potential SYSTEM_USER that can be accessed through parallel workers, a second field is added for the authentication method, copied directly from Port. ClientConnectionInfo is serialized and restored using a new parallel key and a structure tracks the length of the authn_id, making the addition of more fields straight-forward. Author: Jacob Champion Reviewed-by: Bertrand Drouvot, Stephen Frost, Robert Haas, Tom Lane, Michael Paquier, Julien Rouhaud Discussion: https://postgr.es/m/793d990837ae5c06a558d58d62de9378ab525d83.camel@vmware.com
* Add BackendType for standalone backendsAndres Freund2022-08-22
| | | | | | | | | | | | | All backends should have a BackendType to enable statistics reporting per BackendType. Add a new BackendType for standalone backends, B_STANDALONE_BACKEND (and alphabetize the BackendTypes). Both the bootstrap backend and single user mode backends will have BackendType B_STANDALONE_BACKEND. Author: Melanie Plageman <melanieplageman@gmail.com> Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://www.postgresql.org/message-id/CAAKRu_aaq33UnG4TXq3S-OSXGWj1QGf0sU%2BECH4tNwGFNERkZA%40mail.gmail.com
* pgstat: Acquire lock when reading variable-numbered statsAndres Freund2022-08-22
| | | | | | | | | | | | | | Somewhere during the development of the patch acquiring a lock during read access to variable-numbered stats got lost. The missing lock acquisition won't cause corruption, but can lead to reading torn values when accessing stats. Add the missing lock acquisitions. Reported-by: Greg Stark <stark@mit.edu> Reviewed-by: "Drouvot, Bertrand" <bdrouvot@amazon.com> Reviewed-by: Andres Freund <andres@anarazel.de> Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Discussion: https://postgr.es/m/CAM-w4HMYkM_DkYhWtUGV+qE_rrBxKOzOF0+5faozxO3vXrc9wA@mail.gmail.com Backpatch: 15-
* Make role grant system more consistent with other privileges.Robert Haas2022-08-22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, membership of role A in role B could be recorded in the catalog tables only once. This meant that a new grant of role A to role B would overwrite the previous grant. For other object types, a new grant of permission on an object - in this case role A - exists along side the existing grant provided that the grantor is different. Either grant can be revoked independently of the other, and permissions remain so long as at least one grant remains. Make role grants work similarly. Previously, when granting membership in a role, the superuser could specify any role whatsoever as the grantor, but for other object types, the grantor of record must be either the owner of the object, or a role that currently has privileges to perform a similar GRANT. Implement the same scheme for role grants, treating the bootstrap superuser as the role owner since roles do not have owners. This means that attempting to revoke a grant, or admin option on a grant, can now fail if there are dependent privileges, and that CASCADE can be used to revoke these. It also means that you can't grant ADMIN OPTION on a role back to a user who granted it directly or indirectly to you, similar to how you can't give WITH GRANT OPTION on a privilege back to a role which granted it directly or indirectly to you. Previously, only the superuser could specify GRANTED BY with a user other than the current user. Relax that rule to allow the grantor to be any role whose privileges the current user posseses. This doesn't improve compatibility with what we do for other object types, where support for GRANTED BY is entirely vestigial, but it makes this feature more usable and seems to make sense to change at the same time we're changing related behaviors. Along the way, fix "ALTER GROUP group_name ADD USER user_name" to require the same privileges as "GRANT group_name TO user_name". Previously, CREATEROLE privileges were sufficient for either, but only the former form was permissible with ADMIN OPTION on the role. Now, either CREATEROLE or ADMIN OPTION on the role suffices for either spelling. Patch by me, reviewed by Stephen Frost. Discussion: http://postgr.es/m/CA+TgmoaFr-RZeQ+WoQ5nKPv97oT9+aDgK_a5+qWHSgbDsMp1Vg@mail.gmail.com
* Remove shadowed local variables that are new in v15David Rowley2022-08-20
| | | | | | | | | | | | | | | | | | | | | | | | | 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
* Avoid using list_length() to test for empty list.Tom Lane2022-08-17
| | | | | | | | | | | | | | | | | | | | | | | | The standard way to check for list emptiness is to compare the List pointer to NIL; our list code goes out of its way to ensure that that is the only representation of an empty list. (An acceptable alternative is a plain boolean test for non-null pointer, but explicit mention of NIL is usually preferable.) Various places didn't get that memo and expressed the condition with list_length(), which might not be so bad except that there were such a variety of ways to check it exactly: equal to zero, less than or equal to zero, less than one, yadda yadda. In the name of code readability, let's standardize all those spellings as "list == NIL" or "list != NIL". (There's probably some microscopic efficiency gain too, though few of these look to be at all performance-critical.) A very small number of cases were left as-is because they seemed more consistent with other adjacent list_length tests that way. Peter Smith, with bikeshedding from a number of us Discussion: https://postgr.es/m/CAHut+PtQYe+ENX5KrONMfugf0q6NHg4hR5dAhqEXEc2eefFeig@mail.gmail.com
* Preserve memory context of VarStringSortSupport buffers.Tom Lane2022-08-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When enlarging the work buffers of a VarStringSortSupport object, varstrfastcmp_locale was careful to keep them in the ssup_cxt memory context; but varstr_abbrev_convert just used palloc(). The latter creates a hazard that the buffers could be freed out from under the VarStringSortSupport object, resulting in stomping on whatever gets allocated in that memory later. In practice, because we only use this code for ICU collations (cf. 3df9c374e), the problem is confined to use of ICU collations. I believe it may have been unreachable before the introduction of incremental sort, too, as traditional sorting usually just uses one context for the duration of the sort. We could fix this by making the broken stanzas in varstr_abbrev_convert match the non-broken ones in varstrfastcmp_locale. However, it seems like a better idea to dodge the issue altogether by replacing the pfree-and-allocate-anew coding with repalloc, which automatically preserves the chunk's memory context. This fix does add a few cycles because repalloc will copy the chunk's content, which the existing coding assumes is useless. However, we don't expect that these buffer enlargement operations are performance-critical. Besides that, it's far from obvious that copying the buffer contents isn't required, since these stanzas make no effort to mark the buffers invalid by resetting last_returned, cache_blob, etc. That seems to be safe upon examination, but it's fragile and could easily get broken in future, which wouldn't get revealed in testing with short-to-moderate-size strings. Per bug #17584 from James Inform. Whether or not the issue is reachable in the older branches, this code has been broken on its own terms from its introduction, so patch all the way back. Discussion: https://postgr.es/m/17584-95c79b4a7d771f44@postgresql.org
* Remove HAVE_UNIX_SOCKETS.Thomas Munro2022-08-14
| | | | | | | | | | | | | | | | | | | | | | | | | Since HAVE_UNIX_SOCKETS is now defined unconditionally, remove the macro and drop a small amount of dead code. The last known systems not to have them (as far as I know at least) were QNX, which we de-supported years ago, and Windows, which now has them. If a new OS ever shows up with the POSIX sockets API but without working AF_UNIX, it'll presumably still be able to compile the code, and fail at runtime with an unsupported address family error. We might want to consider adding a HINT that you should turn off the option to use it if your network stack doesn't support it at that point, but it doesn't seem worth making the relevant code conditional at compile time. Also adjust a couple of places in the docs and comments that referred to builds without Unix-domain sockets, since there aren't any. Windows still gets a special mention in those places, though, because we don't try to use them by default there yet. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Andres Freund <andres@anarazel.de> Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com> Discussion: https://postgr.es/m/CA%2BhUKG%2BL_3brvh%3D8e0BW_VfX9h7MtwgN%3DnFHP5o7X2oZucY9dg%40mail.gmail.com
* Optimize xid/subxid searches in XidInMVCCSnapshot().John Naylor2022-08-11
| | | | | | | | | | | | | | | | | As reported by Yura Sokolov, scanning the snapshot->xip array has noticeable impact on scalability when there are a large number of concurrent writers. Use the optimized (on x86-64) routine from b6ef16756 to speed up searches through the [sub]xip arrays. One benchmark showed a 5% increase in transaction throughput with 128 concurrent writers, and a 50% increase in a pathological case of 1024 writers. While a hash table would have scaled even better, it was ultimately rejected because of concerns around code complexity and memory allocation. Credit to Andres Freund for the idea to optimize linear search using SIMD instructions. Nathan Bossart Reviewed by: Andres Freund, John Naylor, Bharath Rupireddy, Masahiko Sawada Discussion: https://postgr.es/m/20220713170950.GA3116318%40nathanxps13
* Fix some inconsistencies with GUC categoriesMichael Paquier2022-08-09
| | | | | | | | | | | | | | | | | | | | This commit addresses a few things around GUCs: - The TCP-related parameters (the four tcp_keepalives_* and client_connection_check_interval are listed in postgresql.conf.sample in a subsection called "TCP settings" of "CONNECTIONS AND AUTHENTICATION", but they did not have their own group name in guc.c. - enable_group_by_reordering, stats_fetch_consistency and recovery_prefetch had an inconsistent description, missing a dot at the end. - In postgresql.conf.sample, "Process title" should not have a section of its own, but it should be a subsection of "REPORTING AND LOGGING". This impacts the contents of pg_settings, which could be seen as a compatibility break, so no backpatch is done. This is similar to the cleanup done in a55a984. Author: Shinya Kato Discussion: https://postgr.es/m/5e0c9c608624eafbba910c344282cb14@oss.nttdata.com
* Fix function-defined-but-not-used warning.Tom Lane2022-08-06
| | | | | | | | | | | | Buildfarm member jacana (MinGW) has been complaining that get_iso_localename is defined but not used. This is evidently fallout from the recent removal of VS2013 support in pg_locale.c. Rearrange the #ifs so that get_iso_localename and its subroutine search_locale_enum won't get built on MinGW. I also noticed that a comment in get_iso_localename cross- referenced a comment in IsoLocaleName that isn't there anymore. Put back what I think is the referenced material.
* Replace pgwin32_is_junction() with lstat().Thomas Munro2022-08-06
| | | | | | | | | | | | | | | | | | | Now that lstat() reports junction points with S_IFLNK/S_ISLINK(), and unlink() can unlink them, there is no need for conditional code for Windows in a few places. That was expressed by testing for WIN32 or S_ISLNK, which we can now constant-fold. The coding around pgwin32_is_junction() was a bit suspect anyway, as we never checked for errors, and we also know that errors can be spuriously reported because of transient sharing violations on this OS. The lstat()-based code has handling for that. This also reverts 4fc6b6ee on master only. That was done because lstat() didn't previously work for symlinks (junction points), but now it does. Tested-by: Andrew Dunstan <andrew@dunslane.net> Discussion: https://postgr.es/m/CA%2BhUKGLfOOeyZpm5ByVcAt7x5Pn-%3DxGRNCvgiUPVVzjFLtnY0w%40mail.gmail.com
* Use hba_file/ident_file GUCs rather than pg_hba.conf/pg_ident.conf in logsMichael Paquier2022-08-05
| | | | | | | | | | | | This is particularly useful when log_min_messages is set to FATAL, so as one can know which file was not getting loaded whether hba_file or ident_file are set to some non-default values. If using the default values of these GUC parameters, the same reports are generated. This commit changes the load (startup) and reload (SIGHUP) messages. Author: Julien Rouhaud Discussion: https://postgr.es/m/20220223045959.35ipdsvbxcstrhya@jrouhaud
* Remove dead pread and pwrite replacement code.Thomas Munro2022-08-05
| | | | | | | | | | | | | | | | | | | | | | | pread() and pwrite() are in SUSv2, and all targeted Unix systems have them. Previously, we defined pg_pread and pg_pwrite to emulate these function with lseek() on old Unixen. The names with a pg_ prefix were a reminder of a portability hazard: they might change the current file position. That hazard is gone, so we can drop the prefixes. Since the remaining replacement code is Windows-only, move it into src/port/win32p{read,write}.c, and move the declarations into src/include/port/win32_port.h. No need for vestigial HAVE_PREAD, HAVE_PWRITE macros as they were only used for declarations in port.h which have now moved into win32_port.h. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Greg Stark <stark@mit.edu> Reviewed-by: Robert Haas <robertmhaas@gmail.com> Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/CA+hUKGJ3LHeP9w5Fgzdr4G8AnEtJ=z=p6hGDEm4qYGEUX5B6fQ@mail.gmail.com
* Remove configure probes for symlink/readlink, and dead code.Thomas Munro2022-08-05
| | | | | | | | | | | | | | | | | | | symlink() and readlink() are in SUSv2 and all targeted Unix systems have them. We have partial emulation on Windows. Code that raised runtime errors on systems without it has been dead for years, so we can remove that and also references to such systems in the documentation. Define HAVE_READLINK and HAVE_SYMLINK macros on Unix. Our Windows replacement functions based on junction points can't be used for relative paths or for non-directories, so the macros can be used to check for full symlink support. The places that deal with tablespaces can just use symlink functions without checking the macros. (If they did check the macros, they'd need to provide an #else branch with a runtime or compile time error, and it'd be dead code.) Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/CA+hUKGJ3LHeP9w5Fgzdr4G8AnEtJ=z=p6hGDEm4qYGEUX5B6fQ@mail.gmail.com
* Remove configure probe for dlopen, and refactor.Thomas Munro2022-08-05
| | | | | | | | | | | | | | | | dlopen() is in SUSv2 and all targeted Unix systems have it. We still need replacement functions for Windows, but we don't need a configure probe for that. Since it's no longer needed by other operating systems, rename dlopen.c to win32dlopen.c and move the declarations into win32_port.h. Likewise, the macros RTLD_NOW and RTLD_GLOBAL now only need to be defined on Windows, since all targeted Unix systems have 'em. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/CA+hUKGJ3LHeP9w5Fgzdr4G8AnEtJ=z=p6hGDEm4qYGEUX5B6fQ@mail.gmail.com
* Change type "char"'s I/O format for non-ASCII characters.Tom Lane2022-08-02
| | | | | | | | | | | | | | | | | | | | | | | | | | Previously, a byte with the high bit set was just transmitted as-is by charin() and charout(). This is problematic if the database encoding is multibyte, because the result of charout() won't be validly encoded, which breaks various stuff that expects all text strings to be validly encoded. We've previously decided to enforce encoding validity rather than try to individually harden each place that might have a problem with such strings, so it's time to do something about "char". To fix, represent high-bit-set characters as \ooo (backslash and three octal digits), following the ancient "escape" format for bytea. charin() will continue to accept the old way as well, though that is only reachable in single-byte encodings. Add some test cases just so there is coverage for this code. We'll otherwise leave this question undocumented as it was before, because we don't really want to encourage end-user use of "char". For the moment, back-patch into v15 so that this change appears in 15beta3. If there's not great pushback we should consider absorbing this change into the older branches. Discussion: https://postgr.es/m/2318797.1638558730@sss.pgh.pa.us
* Move common catalog cache access routines to lsyscache.cAmit Kapila2022-08-02
| | | | | | | | | In passing, move pg_relation_is_publishable next to similar functions. Suggested-by: Alvaro Herrera Author: Amit Kapila Reviewed-by: Hou Zhijie Discussion: https://postgr.es/m/CAHut+PupQ5UW9A9ut0Yjt21J9tHhx958z5L0k8-9hTYf_NYqxA@mail.gmail.com
* Fix trim_array() for zero-dimensional array argument.Tom Lane2022-07-31
| | | | | | | | | | | | | | The code tried to access ARR_DIMS(v)[0] and ARR_LBOUND(v)[0] whether or not those values exist. This made the range check on the "n" argument unstable --- it might or might not fail, and if it did it would report garbage for the allowed upper limit. These bogus accesses would probably annoy Valgrind, and if you were very unlucky even lead to SIGSEGV. Report and fix by Martin Kalcher. Back-patch to v14 where this function was added. Discussion: https://postgr.es/m/baaeb413-b8a8-4656-5757-ef347e5ec11f@aboutsource.net
* Support pg_read_[binary_]file (filename, missing_ok).Tom Lane2022-07-29
| | | | | | | | | | | | | | | | | | There wasn't an especially nice way to read all of a file while passing missing_ok = true. Add an additional overloaded variant to support that use-case. While here, refactor the C code to avoid a rats-nest of PG_NARGS checks, instead handling the argument collection in the outer wrapper functions. It's a bit longer this way, but far more straightforward. (Upon looking at the code coverage report for genfile.c, I was impelled to also add a test case for pg_stat_file() -- tgl) Kyotaro Horiguchi Discussion: https://postgr.es/m/20220607.160520.1984541900138970018.horikyota.ntt@gmail.com
* Use TRUNCATE to preserve relfilenode for pg_largeobject + index.Robert Haas2022-07-28
| | | | | | | | | | | | | | | | | | | | | | Commit 9a974cbcba005256a19991203583a94b4f9a21a9 arranged to preserve the relfilenode of user tables across pg_upgrade, but failed to notice that pg_upgrade treats pg_largeobject as a user table and thus it needs the same treatment. Otherwise, large objects will appear to vanish after a pg_upgrade. Commit d498e052b4b84ae21b3b68d5b3fda6ead65d1d4d fixed this problem by teaching pg_dump to UPDATE pg_class.relfilenode for pg_largeobject and its index. However, because an UPDATE on the catalog rows doesn't change anything on disk, this can leave stray files behind in the new cluster. They will normally be empty, but it's a little bit untidy. Hence, this commit arranges to do the same thing using DDL. Specifically, it makes TRUNCATE work for the pg_largeobject catalog when in binary-upgrade mode, and it then uses that command in binary-upgrade dumps as a way of setting pg_class.relfilenode for pg_largeobject and its index. That way, the old files are removed from the new cluster. Discussion: http://postgr.es/m/CA+TgmoYYMXGUJO5GZk1-MByJGu_bB8CbOL6GJQC8=Bzt6x6vDg@mail.gmail.com
* Improve speed of hash index build.Tom Lane2022-07-28
| | | | | | | | | | | | | In the initial data sort, if the bucket numbers are the same then next sort on the hash value. Because index pages are kept in hash value order, this gains a little speed by allowing the eventual tuple insertions to be done sequentially, avoiding repeated data movement within PageAddItem. This seems to be good for overall speedup of 5%-9%, depending on the incoming data. Simon Riggs, reviewed by Amit Kapila Discussion: https://postgr.es/m/CANbhV-FG-1ZNMBuwhUF7AxxJz3u5137dYL-o6hchK1V_dMw86g@mail.gmail.com
* Fix read_relmap_file() concurrency on Windows.Robert Haas2022-07-27
| | | | | | | | | | | | Commit d8cd0c6c95c0120168df93aae095df4e0682a08a introduced a file rename that could fail on Windows, probably due to other backends having an open file handle to the old file of the same name. Re-arrange the locking slightly to prevent that, by making sure the open() and close() run while we hold the lock. Thomas Munro. I added an explanatory comment. Discussion: https://postgr.es/m/CA%2BhUKGLZtCTgp4NTWV-wGbR2Nyag71%3DEfYTKjDKnk%2BfkhuFMHw%40mail.gmail.com
* Split tuplesortvariants.c from tuplesort.cAlexander Korotkov2022-07-27
| | | | | | | | | | | | This commit puts the implementation of Tuple sort variants into the separate file tuplesortvariants.c. That gives better separation of the code and serves well as the demonstration that Tuple sort variant can be defined outside of tuplesort.c. Discussion: https://postgr.es/m/CAPpHfdvjix0Ahx-H3Jp1M2R%2B_74P-zKnGGygx4OWr%3DbUQ8BNdw%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Pavel Borisov, Maxim Orlov, Matthias van de Meent Reviewed-by: Andres Freund, John Naylor
* Split TuplesortPublic from TuplesortstateAlexander Korotkov2022-07-27
| | | | | | | | | | | | | The new TuplesortPublic data structure contains the definition of sort-variant-specific interface methods and the part of Tuple sort operation state required by their implementations. This will let define Tuple sort variants without knowledge of Tuplesortstate, that is without knowledge of generic sort implementation guts. Discussion: https://postgr.es/m/CAPpHfdvjix0Ahx-H3Jp1M2R%2B_74P-zKnGGygx4OWr%3DbUQ8BNdw%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Pavel Borisov, Maxim Orlov, Matthias van de Meent Reviewed-by: Andres Freund, John Naylor
* Move memory management away from writetup() and tuplesort_put*()Alexander Korotkov2022-07-27
| | | | | | | | | | | | This commit puts some generic work away from sort-variant-specific function. In particular, tuplesort_put*() now doesn't need to decrease available memory and switch to sort context before calling puttuple_common(). writetup() doesn't need to free SortTuple.tuple and increase available memory. Discussion: https://postgr.es/m/CAPpHfdvjix0Ahx-H3Jp1M2R%2B_74P-zKnGGygx4OWr%3DbUQ8BNdw%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Pavel Borisov, Maxim Orlov, Matthias van de Meent Reviewed-by: Andres Freund, John Naylor
* Put abbreviation logic into puttuple_common()Alexander Korotkov2022-07-27
| | | | | | | | | | | | Abbreviation code is very similar along tuplesort_put*() functions. This commit unifies that code and puts it into puttuple_common(). tuplesort_put*() functions differs in the abbreviation condition, so it has been added as an argument to the puttuple_common() function. Discussion: https://postgr.es/m/CAPpHfdvjix0Ahx-H3Jp1M2R%2B_74P-zKnGGygx4OWr%3DbUQ8BNdw%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Pavel Borisov, Maxim Orlov, Matthias van de Meent Reviewed-by: Andres Freund, John Naylor
* Add new Tuplesortstate.removeabbrev functionAlexander Korotkov2022-07-27
| | | | | | | | | | | | | This commit is the preparation to move abbreviation logic into puttuple_common(). The new removeabbrev function turns datum1 representation of SortTuple's from the abbreviated key to the first column value. Therefore, it encapsulates the differential part of abbreviation handling code in tuplesort_put*() functions, making these functions similar. Discussion: https://postgr.es/m/CAPpHfdvjix0Ahx-H3Jp1M2R%2B_74P-zKnGGygx4OWr%3DbUQ8BNdw%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Pavel Borisov, Maxim Orlov, Matthias van de Meent Reviewed-by: Andres Freund, John Naylor
* Remove Tuplesortstate.copytup functionAlexander Korotkov2022-07-27
| | | | | | | | | | | | | | It's currently unclear how do we split functionality between Tuplesortstate.copytup() function and tuplesort_put*() functions. For instance, copytup_index() and copytup_datum() return error while tuplesort_putindextuplevalues() and tuplesort_putdatum() do their work. This commit removes Tuplesortstate.copytup() altogether, putting the corresponding code into tuplesort_put*(). Discussion: https://postgr.es/m/CAPpHfdvjix0Ahx-H3Jp1M2R%2B_74P-zKnGGygx4OWr%3DbUQ8BNdw%40mail.gmail.com Author: Alexander Korotkov Reviewed-by: Pavel Borisov, Maxim Orlov, Matthias van de Meent Reviewed-by: Andres Freund, John Naylor
* Remove the restriction that the relmap must be 512 bytes.Robert Haas2022-07-26
| | | | | | | | | | | | | Instead of relying on the ability to atomically overwrite the entire relmap file in one shot, write a new one and durably rename it into place. Removing the struct padding and the calculation showing why the map is exactly 512 bytes, and change the maximum number of entries to a nearby round number. Patch by me, reviewed by Andres Freund and Dilip Kumar. Discussion: http://postgr.es/m/CA+TgmoZq5%3DLWDK7kHaUbmWXxcaTuw_QwafgG9dr-BaPym_U8WQ%40mail.gmail.com Discussion: http://postgr.es/m/CAFiTN-ttOXLX75k_WzRo9ar=VvxFhrHi+rJxns997F+yvkm==A@mail.gmail.com
* Fix path reference when parsing pg_ident.conf for pg_ident_file_mappingsMichael Paquier2022-07-26
| | | | | | | | | | | | | | | | | | | | | | | | Since a2c8499, HbaFileName (default pg_hba.conf) was getting used instead of IdentFileName (default pg_ident.conf) as the parent file to use as reference when parsing the contents of pg_ident.conf, with pg_ident.conf correctly opened, when feeding this information to pg_ident_file_mappings. This had two consequences: - On an I/O error when reading pg_ident.conf, the user would get an ERROR message referring to pg_hba.conf and not pg_ident.conf. - When reading an external file with a relative path using '@' in pg_ident.conf, the directory used to look at the file to load would be the base directory of pg_hba.conf rather than the one of pg_ident.conf, leading to errors in pg_ident_file_mappings inconsistent with what gets loaded at startup when pg_ident.conf and pg_hba.conf are located in different directories. This error only impacted the SQL view pg_ident_file_mappings that uses a logic new to v15 to fill the view with the parsed information, not the code paths loading these authentication files at startup. Author: Julien Rouhaud Discussion: https://postgr.es/m/20220726050402.vsr6fmz7rsgpmdz3@jrouhaud Backpatch-through: 15
* Process session_preload_libraries within InitPostgres's transaction.Tom Lane2022-07-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously we did this after InitPostgres, at a somewhat randomly chosen place within PostgresMain. However, since commit a0ffa885e doing this outside a transaction can cause a crash, if we need to check permissions while replacing a placeholder GUC. (Besides which, a preloaded library could itself want to do database access within _PG_init.) To avoid needing an additional transaction start/end in every session, move the process_session_preload_libraries call to within InitPostgres's transaction. That requires teaching the code not to call it when InitPostgres is called from somewhere other than PostgresMain, since we don't want session_preload_libraries to affect background workers. The most future-proof solution here seems to be to add an additional flag parameter to InitPostgres; fortunately, we're not yet very worried about API stability for v15. Doing this also exposed the fact that we're currently honoring session_preload_libraries in walsenders, even those not connected to any database. This seems, at minimum, a POLA violation: walsenders are not interactive sessions. Let's stop doing that. (All these comments also apply to local_preload_libraries, of course.) Per report from Gurjeet Singh (thanks also to Nathan Bossart and Kyotaro Horiguchi for review). Backpatch to v15 where a0ffa885e came in. Discussion: https://postgr.es/m/CABwTF4VEpwTHhRQ+q5MiC5ucngN-whN-PdcKeufX7eLSoAfbZA@mail.gmail.com
* Remove configure probe for wctype.h.Thomas Munro2022-07-23
| | | | | | | | | | | This header is present in SUSv2 and Windows. Also remove the inclusion of <wchar.h>, following clues that it was only included for the benefit of historical systems that didn't have <wctype.h>. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CA%2BhUKGKAmTgbg_hMiGG5T7pkpzOnY1cWFAHYtZXHCpqeC_hCkA%40mail.gmail.com
* Close old gap in dependency checks for functions returning composite.Tom Lane2022-07-22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The dependency logic failed to register a column-level dependency when a view or rule contains a reference to a specific column of the result of a function-returning-composite. That meant you could drop the column from the composite type, causing trouble for future executions of the view. We've known about this for years, but never summoned the energy to actually fix it, instead installing various low-level defenses to prevent crashing on references to dropped columns. We had to do that to plug the hole in stable branches, where there might be pre-existing broken references; but let's fix the root cause today. To do that, add some logic (borrowed from get_rte_attribute_is_dropped) to find_expr_references_walker, to check whether a Var referencing an RTE_FUNCTION RTE is referencing a column of a composite type, and if so add the proper dependency. However ... it seems mighty unwise to remove said low-level defenses, since there could be other bugs now or in the future that allow reaching them. By the same token, letting those defenses go untested seems unwise. Hence, rather than just dropping the associated test cases, hack them to continue working by the expedient of manually dropping the pg_depend entries that this fix installs. Back-patch into v15. I don't want to risk changing this behavior in stable branches, but it seems not too late for v15. (Since we have already forced initdb for beta3, we can be sure that all production v15 installations will have these added dependencies.) Discussion: https://postgr.es/m/182492.1658431155@sss.pgh.pa.us
* Remove fls(), use pg_leftmost_one_pos32() instead.Thomas Munro2022-07-22
| | | | | | | | | | | | | | Commit 4f658dc8 provided the traditional BSD fls() function in src/port/fls.c so it could be used in several places. Later we added a bunch of similar facilities in pg_bitutils.h, based on compiler builtins that map to hardware instructions. It's a bit confusing to have both 1-based and 0-based variants of this operation in use in different parts of the tree, and neither is blessed by a standard. Let's drop fls.c and the configure probe, and reuse the newer code. Reviewed-by: David Rowley <dgrowleyml@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CA%2BhUKG%2B7dSX1XF8yFGmYk-%3D48dbjH2kmzZj16XvhbrWP-9BzRg%40mail.gmail.com
* Fix ruleutils issues with dropped cols in functions-returning-composite.Tom Lane2022-07-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Due to lack of concern for the case in the dependency code, it's possible to drop a column of a composite type even though stored queries have references to the dropped column via functions-in-FROM that return the composite type. There are "soft" references, namely FROM-clause aliases for such columns, and "hard" references, that is actual Vars referring to them. The right fix for hard references is to add dependencies preventing the drop; something we've known for many years and not done (and this commit still doesn't address it). A "soft" reference shouldn't prevent a drop though. We've been around on this before (cf. 9b35ddce9, 2c4debbd0), but nobody had noticed that the current behavior can result in dump/reload failures, because ruleutils.c can print more column aliases than the underlying composite type now has. So we need to rejigger the column-alias-handling code to treat such columns as dropped and not print aliases for them. Rather than writing new code for this, I used expandRTE() which already knows how to figure out which function result columns are dropped. I'd initially thought maybe we could use expandRTE() in all cases, but that fails for EXPLAIN's purposes, because the planner strips a lot of RTE infrastructure that expandRTE() needs. So this patch just uses it for unplanned function RTEs and otherwise does things the old way. If there is a hard reference (Var), then removing the column alias causes us to fail to print the Var, since there's no longer a name to print. Failing seems less desirable than printing a made-up name, so I made it print "?dropped?column?" instead. Per report from Timo Stolz. Back-patch to all supported branches. Discussion: https://postgr.es/m/5c91267e-3b6d-5795-189c-d15a55d61dbb@nullachtvierzehn.de
* Make subquery aliases optional in the FROM clause.Dean Rasheed2022-07-20
| | | | | | | | | | | | | | This allows aliases for sub-SELECTs and VALUES clauses in the FROM clause to be omitted. This is an extension of the SQL standard, supported by some other database systems, and so eases the transition from such systems, as well as removing the minor inconvenience caused by requiring these aliases. Patch by me, reviewed by Tom Lane. Discussion: https://postgr.es/m/CAEZATCUCGCf82=hxd9N5n6xGHPyYpQnxW8HneeH+uP7yNALkWA@mail.gmail.com
* Tweak detail and hint messages to be consistent with project policyMichael Paquier2022-07-20
| | | | | | | | | | | Detail and hint messages should be full sentences and should end with a period, but some of the messages newly-introduced in v15 did not follow that. Author: Justin Pryzby Reviewed-by: Álvaro Herrera Discussion: https://postgr.es/m/20220719120948.GF12702@telsasoft.com Backpatch-through: 15
* Fix missed corner cases for grantable permissions on GUCs.Tom Lane2022-07-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | We allow users to set the values of not-yet-loaded extension GUCs, remembering those values in "placeholder" GUC entries. When/if the extension is loaded later in the session, we need to verify that the user had permissions to set the GUC. That was done correctly before commit a0ffa885e, but as of that commit, we'd check the permissions of the active role when the LOAD happens, not the role that had set the value. (This'd be a security bug if it had made it into a released version.) In principle this is simple enough to fix: we just need to remember the exact role OID that set each GUC value, and use that not GetUserID() when verifying permissions. Maintaining that data in the guc.c data structures is slightly tedious, but fortunately it's all basically just copy-n-paste of the logic for tracking the GucSource of each setting, as we were already doing. Another oversight is that validate_option_array_item() hadn't been taught to check for granted GUC privileges. This appears to manifest only in that ALTER ROLE/DATABASE RESET ALL will fail to reset settings that the user should be allowed to reset. Patch by myself and Nathan Bossart, per report from Nathan Bossart. Back-patch to v15 where the faulty code came in. Discussion: https://postgr.es/m/20220706224727.GA2158260@nathanxps13
* Clean up temp file from refactored dtrace rulePeter Eisentraut2022-07-19
| | | | related to eb6569fd0e24e2f0502ef7b496ba0d3125bd4f15
* Refactor dtrace postprocessing make rulesAndres Freund2022-07-18
| | | | | | | | | | | | This is in preparation for building postgres with meson / ninja. Move the dtrace postprocessing sed commands into a separate file so that it can be shared by meson. Also split the rule into two for proper dependency declaration. Reviewed-by: Andres Freund <andres@anarazel.de> Author: Peter Eisentraut <peter@eisentraut.org> Discussion: https://postgr.es/m/5e216522-ba3c-f0e6-7f97-5276d0270029@enterprisedb.com
generated by cgit v1.2.3 (git 2.25.1) at 2025-06-29 04:25:27 +0000