aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
...
* Remove configure probes for sys/ipc.h, sys/sem.h, sys/shm.h.Thomas Munro2022-08-14
| | | | | | | | | These are in SUSv2 and every targeted Unix system has them. It's not hard to avoid including them on Windows system because they're mostly used in platform-specific translation units. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CA%2BhUKG%2BL_3brvh%3D8e0BW_VfX9h7MtwgN%3DnFHP5o7X2oZucY9dg%40mail.gmail.com
* Remove configure probe for sys/select.h.Thomas Munro2022-08-14
| | | | | | | | | <sys/select.h> is in SUSv3 and every targeted Unix system has it. Provide an empty header in src/include/port/win32 so that we can include it unguarded even on Windows. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CA%2BhUKG%2BL_3brvh%3D8e0BW_VfX9h7MtwgN%3DnFHP5o7X2oZucY9dg%40mail.gmail.com
* Add missing fields to _outConstraint()Peter Eisentraut2022-08-13
| | | | | | | | As of 897795240cfaaed724af2f53ed2c50c9862f951f, check constraints can be declared invalid. But that patch didn't update _outConstraint() to also show the relevant struct fields (which were only applicable to foreign keys before that). This currently only affects debugging output, so no impact in practice.
* Avoid using a fake relcache entry to own an SmgrRelation.Robert Haas2022-08-12
| | | | | | | | | | | | | | | | | | | | If an error occurs before we close the fake relcache entry, the the fake relcache entry will be destroyed by the SmgrRelation will survive until end of transaction. Its smgr_owner pointer ends up pointing to already-freed memory. The original reason for using a fake relcache entry here was to try to avoid reusing an SMgrRelation across a relevant invalidation. To avoid that problem, just call smgropen() again each time we need a reference to it. Hopefully someday we will come up with a more elegant approach, but accessing uninitialized memory is bad so let's do this for now. Dilip Kumar, reviewed by Andres Freund and Tom Lane. Report by Justin Pryzby. Discussion: http://postgr.es/m/20220802175043.GA13682@telsasoft.com Discussion: http://postgr.es/m/CAFiTN-vSFeE6_W9z698XNtFROOA_nSqUXWqLcG0emob_kJ+dEQ@mail.gmail.com
* Reject MERGE in CTEs and COPYAlvaro Herrera2022-08-12
| | | | | | | | | | | | The grammar added for MERGE inadvertently made it accepted syntax in places that were not prepared to deal with it -- namely COPY and inside CTEs, but invoking these things with MERGE currently causes assertion failures or weird misbehavior in non-assertion builds. Protect those places by checking for it explicitly until somebody decides to implement it. Reported-by: Alexey Borzov <borz_off@cs.msu.su> Discussion: https://postgr.es/m/17579-82482cd7b267b862@postgresql.org
* Fix _outConstraint() for "identity" constraintsPeter Eisentraut2022-08-12
| | | | | | | | | The set of fields printed by _outConstraint() in the CONSTR_IDENTITY case didn't match the set of fields actually used in that case. (The code was probably uncarefully copied from the CONSTR_DEFAULT case.) Fix that by using the right set of fields. Since there is no read support for this node type, this is really just for debugging output right now, so it doesn't affect anything important.
* Fix non-specific error message.Robert Haas2022-08-11
| | | | | | | "something has gone wrong" is not helpful. Make this elog() consistent with the other one in the same function. Discussion: http://postgr.es/m/CA+Tgmoa_AZ2jUWSA_noiqOqnxBaWDR+t3bHjSygZi6+wqDBCXQ@mail.gmail.com
* struct PQWalReceiverFunctions: use designated initializersAlvaro Herrera2022-08-11
| | | | | | | | | We now require that compilers support this, and it makes the code easier to trace, so change it. I'm fixated on this particular struct because I've had to navigate around it a number of times, but there are others elsewhere that could use the same treatment. Discussion: https://postgr.es/m/20220810140300.ixhbmm4svo5yypv6@alvherre.pgsql
* Add missing space in _outA_Const() outputPeter Eisentraut2022-08-11
| | | | Mistake introduced by 639a86e36aaecb84faaf941dcd0b183ba0aba9e9.
* Fix catalog lookup with the wrong snapshot during logical decoding.Amit Kapila2022-08-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, we relied on HEAP2_NEW_CID records and XACT_INVALIDATION records to know if the transaction has modified the catalog, and that information is not serialized to snapshot. Therefore, after the restart, if the logical decoding decodes only the commit record of the transaction that has actually modified a catalog, we will miss adding its XID to the snapshot. Thus, we will end up looking at catalogs with the wrong snapshot. To fix this problem, this change adds the list of transaction IDs and sub-transaction IDs, that have modified catalogs and are running during snapshot serialization, to the serialized snapshot. After restart or otherwise, when we restore from such a serialized snapshot, the corresponding list is restored in memory. Now, when decoding a COMMIT record, we check both the list and the ReorderBuffer to see if the transaction has modified catalogs. Since this adds additional information to the serialized snapshot, we cannot backpatch it. For back branches, we took another approach. We remember the last-running-xacts list of the decoded RUNNING_XACTS record after restoring the previously serialized snapshot. Then, we mark the transaction as containing catalog changes if it's in the list of initial running transactions and its commit record has XACT_XINFO_HAS_INVALS. This doesn't require any file format changes but the transaction will end up being added to the snapshot even if it has only relcache invalidations. But that won't be a problem since we use snapshot built during decoding only to read system catalogs. This commit bumps SNAPBUILD_VERSION because of a change in SnapBuild. Reported-by: Mike Oh Author: Masahiko Sawada Reviewed-by: Amit Kapila, Shi yu, Takamichi Osumi, Kyotaro Horiguchi, Bertrand Drouvot, Ahsan Hadi Backpatch-through: 10 Discussion: https://postgr.es/m/81D0D8B0-E7C4-4999-B616-1E5004DBDCD2%40amazon.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
* Move basebackup code to new directory src/backend/backupRobert Haas2022-08-10
| | | | | | Reviewed by David Steele and Justin Pryzby Discussion: http://postgr.es/m/CA+TgmoafqboATDSoXHz8VLrSwK_MDhjthK4hEpYjqf9_1Fmczw%40mail.gmail.com
* Fix handling of R/W expanded datums that are passed to SQL functions.Tom Lane2022-08-10
| | | | | | | | | | | | | | | | | | | fmgr_sql must make expanded-datum arguments read-only, because it's possible that the function body will pass the argument to more than one callee function. If one of those functions takes the datum's R/W property as license to scribble on it, then later callees will see an unexpected value, leading to wrong answers. From a performance standpoint, it'd be nice to skip this in the common case that the argument value is passed to only one callee. However, detecting that seems fairly hard, and certainly not something that I care to attempt in a back-patched bug fix. Per report from Adam Mackler. This has been broken since we invented expanded datums, so back-patch to all supported branches. Discussion: https://postgr.es/m/WScDU5qfoZ7PB2gXwNqwGGgDPmWzz08VdydcPFLhOwUKZcdWbblbo-0Lku-qhuEiZoXJ82jpiQU4hOjOcrevYEDeoAvz6nR0IU4IHhXnaCA=@mackler.email Discussion: https://postgr.es/m/187436.1660143060@sss.pgh.pa.us
* 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 obsolete comment in commit_ts.c.Thomas Munro2022-08-09
| | | | | | | | Commit 08aa89b removed COMMIT_TS_SETTS, but left a reference in a comment. Author: Nathan Bossart <nathandbossart@gmail.com> Discussion: https://postgr.es/m/20220726173343.GA154110%40nathanxps13
* Fix MSVC build script's check for obsolete node support functions.Tom Lane2022-08-08
| | | | | | | | | | | | | | | | Commit 964d01ae9 was a few bricks shy of a load here: the script checked whether gen_node_support.pl itself had been updated since it was last run, but not whether any of its input files had been updated. Fix that. While here, scrape the list of input files from the Makefiles rather than having a duplicate copy, as we do for most other lists of source files. In passing, improve gen_node_support.pl's error report for an incorrect file list. Per gripe from Amit Kapila. Discussion: https://postgr.es/m/CAA4eK1KQk4vP-3mTAz26h-PRUZaGu8Fc=q-ZKSajsAthH0A15w@mail.gmail.com
* In extensions, don't replace objects not belonging to the extension.Tom Lane2022-08-08
| | | | | | | | | | | | | | | | | | | | | | | Previously, if an extension script did CREATE OR REPLACE and there was an existing object not belonging to the extension, it would overwrite the object and adopt it into the extension. This is problematic, first because the overwrite is probably unintentional, and second because we didn't change the object's ownership. Thus a hostile user could create an object in advance of an expected CREATE EXTENSION command, and would then have ownership rights on an extension object, which could be modified for trojan-horse-type attacks. Hence, forbid CREATE OR REPLACE of an existing object unless it already belongs to the extension. (Note that we've always forbidden replacing an object that belongs to some other extension; only the behavior for previously-free-standing objects changes here.) For the same reason, also fail CREATE IF NOT EXISTS when there is an existing object that doesn't belong to the extension. Our thanks to Sven Klemm for reporting this problem. Security: CVE-2022-2625
* aix: fix misreading of condition in 8f12a4e7addAndres Freund2022-08-07
| | | | | | | | | This lead to choosing the aix4.1 specific way of building the export file for the backend, rather than the modern one. Per buildfarm member hoverfly. Discussion: https://postgr.es/m/20220807182707.gi7pirwbz5etprfo@awork3.anarazel.de
* aix: Remove checks for very old OS versionsAndres Freund2022-08-07
| | | | | Reviewed-By: Thomas Munro <thomas.munro@gmail.com> Discussion: https://postgr.es/m/20220807012914.ydz73yte6j3coulo@awork3.anarazel.de
* windows: Remove HAVE_MINIDUMP_TYPE testAndres Freund2022-08-07
| | | | | | | We've relied on it being present for msvc for ages... Reviewed-By: Thomas Munro <thomas.munro@gmail.com> Discussion: https://postgr.es/m/20220807012914.ydz73yte6j3coulo@awork3.anarazel.de
* Rely on __func__ being supportedAndres Freund2022-08-07
| | | | | | | | | | | | Previously we fell back to __FUNCTION__ and then NULL. As __func__ is in C99 that shouldn't be necessary anymore. Solution.pm defined HAVE_FUNCNAME__FUNCTION instead of HAVE_FUNCNAME__FUNC (originating in 4164e6636e2), as at some point in the past MSVC only supported __FUNCTION__. Our minimum version supports __func__. Reviewed-By: Thomas Munro <thomas.munro@gmail.com> Discussion: https://postgr.es/m/20220807012914.ydz73yte6j3coulo@awork3.anarazel.de
* 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.
* Fix data-corruption hazard in WAL-logged CREATE DATABASE.Tom Lane2022-08-06
| | | | | | | | | | | | | | RelationCopyStorageUsingBuffer thought it could skip copying empty pages, but of course that does not work at all, because subsequent blocks will be out of place. Also fix it to acquire share lock on the source buffer. It *might* be safe to not do that, but it's not very certain, and I don't think this code deserves any benefit of the doubt. Dilip Kumar, per complaint from me Discussion: https://postgr.es/m/3679800.1659654066@sss.pgh.pa.us
* 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
* Fix comments about deduplication updating page.Peter Geoghegan2022-08-05
| | | | | | | | | | | nbtree deduplication passes add tuples from the original/target page to a temp page, merging as necessary. The temp page is copied back to the target permanent page in the critical section. This is similar to the approach taken by nbtree page splits. Adjust comments that referred to updating the original page in-place as tuples were merged. These were left over from earlier versions of the deduplication patch that didn't yet use a temp page.
* Add missing parenthesis to max item size macro.Peter Geoghegan2022-08-05
| | | | Oversight in commit 92f37505, per buildfarm.
* Partially undo commit 94da73281.Tom Lane2022-08-05
| | | | | | | | On closer inspection, mcv.c isn't as broken for ScalarArrayOpExpr as I thought. The Var-on-right issue is real enough, but actually it does cope fine with a NULL array constant --- I was misled by an XXX comment suggesting it didn't. Undo that part of the code change, and replace the XXX comment with something less misleading.
* Fix handling of bare boolean expressions in mcv_get_match_bitmap.Tom Lane2022-08-05
| | | | | | | | | | | | | | | | | | | | Since v14, the extended stats machinery will try to estimate for otherwise-unsupported boolean expressions if they match an expression available from an extended stats object. mcv.c did not get the memo about this, and would spit up with "unknown clause type". Fortunately the case is easy to handle, since we can expect the expression yields boolean. While here, replace some not-terribly-on-point assertions with simpler runtime tests for lookup failure. That seems appropriate so that we get an elog not a crash if we somehow get to the new it-should-be-a-bool-expression code with a subexpression that doesn't match any stats column. Per report from Danny Shemesh. Thanks to Justin Pryzby for preliminary investigation. Discussion: https://postgr.es/m/CAFZC=QqD6=27wQPOW1pbRa98KPyuyn+7cL_Ay_Ck-roZV84vHg@mail.gmail.com
* Fix non-bulletproof ScalarArrayOpExpr code for extended statistics.Tom Lane2022-08-05
| | | | | | | | | | | | | | | | statext_is_compatible_clause_internal() checked that the arguments of a ScalarArrayOpExpr are one Var and one Const, but it would allow cases where the Const was on the left. Subsequent uses of the clause are not expecting that and would suffer assertion failures or core dumps. mcv.c also had not bothered to cope with the case of a NULL array constant, which seems really unacceptably sloppy of somebody. (Although our tools failed us there too, since AFAIK neither Coverity nor any compiler warned of the obvious use-of-uninitialized-variable condition.) It seems best to handle that by having statext_is_compatible_clause_internal() reject it. Noted while fixing bug #17570. Back-patch to v13 where the extended stats code grew some awareness of ScalarArrayOpExpr.
* Fix incorrect permissions-checking code for extended statistics.Tom Lane2022-08-05
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit a4d75c86b improved the extended-stats logic to allow extended stats to be collected on expressions not just bare Vars. To apply such stats, we first verify that the user has permissions to read all columns used in the stats. (If not, the query will likely fail at runtime, but the planner ought not do so.) That had to get extended to check permissions of columns appearing within such expressions, but the code for that was completely wrong: it applied pull_varattnos to the wrong pointer, leading to "unrecognized node type" failures. Furthermore, although you couldn't get to this because of that bug, it failed to account for the attnum offset applied by pull_varattnos. This escaped recognition so far because the code in question is not reached when the user has whole-table SELECT privilege (which is the common case), and because only subexpressions not specially handled by statext_is_compatible_clause_internal() are at risk. I think a large part of the reason for this bug is under-documentation of what statext_is_compatible_clause() is doing and what its arguments are, so do some work on the comments to try to improve that. Per bug #17570 from Alexander Kozhemyakin. Patch by Richard Guo; comments and other cosmetic improvements by me. (Thanks also to Japin Li for diagnosis.) Back-patch to v14 where the bug came in. Discussion: https://postgr.es/m/17570-f2f2e0f4bccf0965@postgresql.org
* BRIN: mask BRIN_EVACUATE_PAGE for WAL consistency checkingAlvaro Herrera2022-08-05
| | | | | | | | | | | | | | | | That bit is unlogged and therefore it's wrong to consider it in WAL page comparison. Add a test that tickles the case, as branch testing technology allows. This has been a problem ever since wal consistency checking was introduced (commit a507b86900f6 for pg10), so backpatch to all supported branches. Author: 王海洋 (Haiyang Wang) <wanghaiyang.001@bytedance.com> Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Discussion: https://postgr.es/m/CACciXAD2UvLMOhc4jX9VvOKt7DtYLr3OYRBhvOZ-jRxtzc_7Jg@mail.gmail.com Discussion: https://postgr.es/m/CACciXADOfErX9Bx0nzE_SkdfXr6Bbpo5R=v_B6MUTEYW4ya+cg@mail.gmail.com
* Fix ENABLE/DISABLE TRIGGER to handle recursion correctlyAlvaro Herrera2022-08-05
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Using ATSimpleRecursion() in ATPrepCmd() to do so as bbb927b4db9b did is not correct, because ATPrepCmd() can't distinguish between triggers that may be cloned and those that may not, so would wrongly try to recurse for the latter category of triggers. So this commit restores the code in EnableDisableTrigger() that 86f575948c77 had added to do the recursion, which would do it only for triggers that may be cloned, that is, row-level triggers. This also changes tablecmds.c such that ATExecCmd() is able to pass the value of ONLY flag down to EnableDisableTrigger() using its new 'recurse' parameter. This also fixes what seems like an oversight of 86f575948c77 that the recursion to partition triggers would only occur if EnableDisableTrigger() had actually changed the trigger. It is more apt to recurse to inspect partition triggers even if the parent's trigger didn't need to be changed: only then can we be certain that all descendants share the same state afterwards. Backpatch all the way back to 11, like bbb927b4db9b. Care is taken not to break ABI compatibility (and that no catversion bump is needed.) Co-authored-by: Amit Langote <amitlangote09@gmail.com> Reviewed-by: Dmitry Koval <d.koval@postgrespro.ru> Discussion: https://postgr.es/m/CA+HiwqG-cZT3XzGAnEgZQLoQbyfJApVwOTQaCaas1mhpf+4V5A@mail.gmail.com
* Remove configure probe for fdatasync.Thomas Munro2022-08-05
| | | | | | | | | | | | | | | | | fdatasync() is in SUSv2, and all targeted Unix systems have it. We have a replacement function for Windows. We retain the probe for the function declaration, which allows us to supply the mysteriously missing declaration for macOS, and also for Windows. No need to keep a HAVE_FDATASYNC macro around. Also rename src/port/fdatasync.c to win32fdatasync.c since it's only for Windows. 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 Discussion: https://postgr.es/m/CA%2BhUKGJZJVO%3DiX%2Beb-PXi2_XS9ZRqnn_4URh0NUQOwt6-_51xQ%40mail.gmail.com
* Simplify replacement code for preadv and pwritev.Thomas Munro2022-08-05
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | preadv() and pwritev() are not standardized by POSIX, but appeared in NetBSD in 1999 and were adopted by at least OpenBSD, FreeBSD, DragonFlyBSD, Linux, AIX, illumos and macOS. We don't use them much yet, but an active proposal uses them heavily. In 15, we had two replacement implementations for other OSes: one based on lseek() + -v function if available for true vector I/O, and the other based on a loop over p- function. The former would be an obstacle to hypothetical future multi-threaded code sharing file descriptors, while the latter would not, since commit cf112c12. Furthermore, the number of targeted systems that could benefit from the former's potential upside has dwindled to just one niche OS, since macOS added the functions and we de-supported HP-UX. That doesn't seem like a good trade-off. Therefore, drop the lseek()-based variant, and also the pg_ prefix now that the file position portability hazard is gone. At the time of writing, the only systems in our build farm that lack native preadv/pwritev and thus use fallback code are: * Solaris (but not illumos) * macOS before release 11.0 * Windows With this commit, the above systems will now use the *same* fallback code, the version that loops over pread()/pwrite(). Windows already used that (though a later proposal may include true vector I/O for Windows), so this decision really only affects Solaris, until it gets around to adding these system calls. Also remove some useless includes while here. Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/CA+hUKGJ3LHeP9w5Fgzdr4G8AnEtJ=z=p6hGDEm4qYGEUX5B6fQ@mail.gmail.com
* Make consistent a couple of log messages when parsing HBA filesMichael Paquier2022-08-05
| | | | | | | | | | | | | This commit adjusts two log messages: - When a field in pg_ident.conf is not populated, report the line of the configuration file in an error context message instead of the main entry. - When parsing pg_ident.conf and finding an invalid regexp, add some information about the line of the configuration file involved within an error context message. Author: Julien Rouhaud Discussion: https://postgr.es/m/20220223045959.35ipdsvbxcstrhya@jrouhaud
* 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
* Fix failure to set correct operator in window run conditionDavid Rowley2022-08-05
| | | | | | | | | | | | This was a simple omission in 9d9c02ccd where the code didn't correctly set the operator to use in the run condition OpExpr when the window function was both monotonically increasing and decreasing. Bug discovered by Julien Roze, although he did not report it. Reported-by: Phil Florent Discussion: https://postgr.es/m/PA4P191MB160009A09B9D0624359278CFBA9F9@PA4P191MB1600.EURP191.PROD.OUTLOOK.COM Backpatch-through: 15, where 9d9c02ccd was added
* 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 and related tests for getrlimit.Thomas Munro2022-08-05
| | | | | | | | | | | | | | | | | | getrlimit() is in SUSv2 and all targeted systems have it. Windows doesn't have it. We could just use #ifndef WIN32, but for a little more explanation about why we're making things conditional, let's retain the HAVE_GETRLIMIT macro. It's defined in port.h for Unix systems. On systems that have it, it's not necessary to test for RLIMIT_CORE, RLIMIT_STACK or RLIMIT_NOFILE macros, since SUSv2 requires those and all targeted systems have them. Also remove references to a pre-historic alternative spelling of RLIMIT_NOFILE, and coding that seemed to believe that Cygwin didn't have it. 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
* Fix check_exclusion_or_unique_constraint for UNIQUE NULLS NOT DISTINCT.Tom Lane2022-08-04
| | | | | | | | | | Adjusting this function was overlooked in commit 94aa7cc5f. The only visible symptom (so far) is that INSERT ... ON CONFLICT could go into an endless loop when inserting a null that has a conflict. Richard Guo and Tom Lane, per bug #17558 from Andrew Kesper Discussion: https://postgr.es/m/17558-3f6599ffcf52fd4a@postgresql.org
* Add CHECK_FOR_INTERRUPTS in ExecInsert's speculative insertion loop.Tom Lane2022-08-04
| | | | | | | | | | | | | Ordinarily the functions called in this loop ought to have plenty of CFIs themselves; but we've now seen a case where no such CFI is reached, making the loop uninterruptible. Even though that's from a recently-introduced bug, it seems prudent to install a CFI at the loop level in all branches. Per discussion of bug #17558 from Andrew Kesper (an actual fix for that bug will follow). Discussion: https://postgr.es/m/17558-3f6599ffcf52fd4a@postgresql.org
* Rephrase comments to make them clearerDaniel Gustafsson2022-08-04
| | | | | | | | | | | | The use of "we" when referring to the active backend might be misunderstood, so rephrase to make it clearer who is performing the actions discussed in the comment. Author: Junwang Zhao <zhjwpku@gmail.com> Reviewed-by: Erikjan Rijkers <er@xs4all.nl> Reviewed-by: Robert Treat <rob@xzilla.net> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CAEG8a3LRSMqkvjiURiJoSi4aGWORpiXUmUfQQK5PaD6WfPzu3w@mail.gmail.com
* Fix formatting and comment typosJohn Naylor2022-08-04
| | | | | | Justin Pryzby Discussion: https://www.postgresql.org/message-id/20220801181136.GJ15006%40telsasoft.com
* Fix incorrect tests for SRFs in relation_can_be_sorted_early().Tom Lane2022-08-03
| | | | | | | | | | | | | | | | | | | | | | | | | Commit fac1b470a thought we could check for set-returning functions by testing only the top-level node in an expression tree. This is wrong in itself, and to make matters worse it encouraged others to make the same mistake, by exporting tlist.c's special-purpose IS_SRF_CALL() as a widely-visible macro. I can't find any evidence that anyone's taken the bait, but it was only a matter of time. Use expression_returns_set() instead, and stuff the IS_SRF_CALL() genie back in its bottle, this time with a warning label. I also added a couple of cross-reference comments. After a fair amount of fooling around, I've despaired of making a robust test case that exposes the bug reliably, so no test case here. (Note that the test case added by fac1b470a is itself broken, in that it doesn't notice if you remove the code change. The repro given by the bug submitter currently doesn't fail either in v15 or HEAD, though I suspect that may indicate an unrelated bug.) Per bug #17564 from Martijn van Oosterhout. Back-patch to v13, as the faulty patch was. Discussion: https://postgr.es/m/17564-c7472c2f90ef2da3@postgresql.org
* 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
* Improve performance of ORDER BY / DISTINCT aggregatesDavid Rowley2022-08-02
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | ORDER BY / DISTINCT aggreagtes have, since implemented in Postgres, been executed by always performing a sort in nodeAgg.c to sort the tuples in the current group into the correct order before calling the transition function on the sorted tuples. This was not great as often there might be an index that could have provided pre-sorted input and allowed the transition functions to be called as the rows come in, rather than having to store them in a tuplestore in order to sort them once all the tuples for the group have arrived. Here we change the planner so it requests a path with a sort order which supports the most amount of ORDER BY / DISTINCT aggregate functions and add new code to the executor to allow it to support the processing of ORDER BY / DISTINCT aggregates where the tuples are already sorted in the correct order. Since there can be many ORDER BY / DISTINCT aggregates in any given query level, it's very possible that we can't find an order that suits all of these aggregates. The sort order that the planner chooses is simply the one that suits the most aggregate functions. We take the most strictly sorted variation of each order and see how many aggregate functions can use that, then we try again with the order of the remaining aggregates to see if another order would suit more aggregate functions. For example: SELECT agg(a ORDER BY a),agg2(a ORDER BY a,b) ... would request the sort order to be {a, b} because {a} is a subset of the sort order of {a,b}, but; SELECT agg(a ORDER BY a),agg2(a ORDER BY c) ... would just pick a plan ordered by {a} (we give precedence to aggregates which are earlier in the targetlist). SELECT agg(a ORDER BY a),agg2(a ORDER BY b),agg3(a ORDER BY b) ... would choose to order by {b} since two aggregates suit that vs just one that requires input ordered by {a}. Author: David Rowley Reviewed-by: Ronan Dunklau, James Coleman, Ranier Vilela, Richard Guo, Tom Lane Discussion: https://postgr.es/m/CAApHDvpHzfo92%3DR4W0%2BxVua3BUYCKMckWAmo-2t_KiXN-wYH%3Dw%40mail.gmail.com
* 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
* Relax overly strict rules in select_outer_pathkeys_for_merge()David Rowley2022-08-02
| | | | | | | | | | | | | | | | | | The select_outer_pathkeys_for_merge function made an attempt to build the merge join pathkeys in the same order as query_pathkeys. This was done as it may have led to no sort being required for an ORDER BY or GROUP BY clause in the upper planner. However, this restriction seems overly strict as it required that we match the query_pathkeys entirely or we don't bother putting the merge join pathkeys in that order. Here we relax this rule so that we use a prefix of the query_pathkeys providing that prefix matches all of the join quals. This may provide the upper planner with partially sorted input which will allow the use of incremental sorts instead of full sorts. Author: David Rowley Reviewed-by: Richard Guo Discussion: https://postgr.es/m/CAApHDvrtZu0PHVfDPFM4Yx3jNR2Wuwosv+T2zqa7LrhhBr2rRg@mail.gmail.com