aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
* Improve error messages about misuse of SELECT INTO.Tom Lane2021-08-21
| | | | | | | | | | | | | | | | | | | | | | | | | Improve two places in plpgsql, and one in spi.c, where an error message would confusingly tell you that you couldn't use a SELECT query, when what you had written *was* a SELECT query. The actual problem is that you can't use SELECT ... INTO in these contexts, but the messages failed to make that apparent. Special-case SELECT INTO to make these errors more helpful. Also, fix the same spots in plpgsql, as well as several messages in exec_eval_expr(), to not quote the entire complained-of query or expression in the primary error message. That behavior very easily led to violating our message style guideline about keeping the primary error message short and single-line. Also, since the important part of the message was after the inserted text, it could make the real problem very hard to see. We can report the query or expression as the first line of errcontext instead. Per complaint from Roger Mason. Back-patch to v14, since (a) some of these messages are new in v14 and (b) v14's translatable strings are still somewhat in flux. The problem's older than that of course, but I'm hesitant to change the behavior further back. Discussion: https://postgr.es/m/1914708.1629474624@sss.pgh.pa.us
* Fix performance bug in regexp's citerdissect/creviterdissect.Tom Lane2021-08-20
| | | | | | | | | | | | | | | | | | | | | After detecting a sub-match "dissect" failure (i.e., a backref match failure) in the i'th sub-match of an iteration node, we should proceed by adjusting the attempted length of the i'th submatch. As coded, though, these functions changed the attempted length of the *last* sub-match, and only after exhausting all possibilities for that would they back up to adjust the next-to-last sub-match, and then the second-from-last, etc; all of which is wasted effort, since only changing the start or length of the i'th sub-match can possibly make it succeed. This oversight creates the possibility for exponentially bad performance. Fortunately the problem is masked in most cases by optimizations or constraints applied elsewhere; which explains why we'd not noticed it before. But it is possible to reach the problem with fairly simple, if contrived, regexps. Oversight in my commit 173e29aa5. That's pretty ancient now, so back-patch to all supported branches. Discussion: https://postgr.es/m/1808998.1629412269@sss.pgh.pa.us
* Remove --quiet option from pg_amcheckDaniel Gustafsson2021-08-20
| | | | | | | | | | | | | | | Using --quiet in combination with --no-strict-names didn't work as documented, a warning message was still emitted. Since the --quiet flag was working in an unconventional way to other utilities, fix by removing the functionality instead. Backpatch through 14 where pg_amcheck was introduced. Bug: 17148 Reported-by: Chen Jiaoqian <chenjq.jy@fujitsu.com> Reviewed-by: Julien Rouhaud <rjuju123@gmail.com> Discussion: https://postgr.es/m/17148-b5087318e2b04fc6@postgresql.org Backpatch-through: 14
* psql: Add test for query cancelingPeter Eisentraut2021-08-20
| | | | | | | | | Query canceling in psql was accidentally broken by 3a5130672296ed4e682403a77a9a3ad3d21cef75 (since reverted), so having some test coverage for that seems useful. Reviewed-by: Fabien COELHO <coelho@cri.ensmp.fr> Discussion: https://www.postgresql.org/message-id/18c78a01-4a34-9dd4-f78b-6860f1420c8e@enterprisedb.com
* pg_resetwal: Improve numeric command-line argument parsingPeter Eisentraut2021-08-20
| | | | | | | | | Check errno after strtoul()/strtol() to handle out of range errors better. For out of range, strtoul() returns ULONG_MAX, and the previous code would proceed with that result. Reported-by: Mark Dilger <mark.dilger@enterprisedb.com> Discussion: https://www.postgresql.org/message-id/flat/6a10a211-872b-3c4c-106b-909ae5fefa61%40enterprisedb.com
* pg_amcheck: Fix block number parsing on command linePeter Eisentraut2021-08-20
| | | | | | | | The previous code wouldn't handle higher block numbers on systems where sizeof(long) == 4. Reviewed-by: Mark Dilger <mark.dilger@enterprisedb.com> Discussion: https://www.postgresql.org/message-id/flat/6a10a211-872b-3c4c-106b-909ae5fefa61%40enterprisedb.com
* Avoid trying to lock OLD/NEW in a rule with FOR UPDATE.Tom Lane2021-08-19
| | | | | | | | | | | | | | transformLockingClause neglected to exclude the pseudo-RTEs for OLD/NEW when processing a rule's query. This led to odd errors or even crashes later on. This bug is very ancient, but it's not terribly surprising that nobody noticed, since the use-case for SELECT FOR UPDATE in a non-view rule is somewhere between thin and non-existent. Still, crashing is not OK. Per bug #17151 from Zhiyong Wu. Thanks to Masahiko Sawada for analysis of the problem. Discussion: https://postgr.es/m/17151-c03a3e6e4ec9aadb@postgresql.org
* Unset MyBEEntry, making elog.c's call to pgstat_get_my_query_id() safe.Andres Freund2021-08-19
| | | | | | | | | | | | | | | | | | Previously log messages late during shutdown could end up using either another backend's PgBackendStatus (multi user) or segfault (single user) because pgstat_get_my_query_id()'s check for !MyBEEntry didn't filter out use after pgstat_beshutdown_hook(). This became a bug in 4f0b0966c86, but was a bit fishy before. But given there's no known problematic cases before 14, it doesn't seem worth backpatching further. Also fixes a wrong filename in a comment, introduced in e1025044. Reported-By: Andres Freund <andres@anarazel.de> Reviewed-By: Julien Rouhaud <rjuju123@gmail.com> Discussion: https://postgr.es/m/Julien Rouhaud <rjuju123@gmail.com> Backpatch: 14-
* Rename LOGICAL_REP_MSG_STREAM_END to LOGICAL_REP_MSG_STREAM_STOP.Amit Kapila2021-08-19
| | | | | | | | | | In the code, most places used the term "Stream Stop" for the logical stream message. This commit improves consistency by renaming LogicalRepMsgType "LOGICAL_REP_MSG_STREAM_END" to "LOGICAL_REP_MSG_STREAM_STOP". Author: Masahiko Sawada Reviewed-by: Hou Zhijie, Amit Kapila Discussion: https://postgr.es/m/CAD21AoDeScrsHhLyEPYqN3sydg6PxAPVBboK=30xJfUVihNZDA@mail.gmail.com
* Revert refactoring of hex code to src/common/Michael Paquier2021-08-19
| | | | | | | | | | | | | | | | | | | | | | | This is a combined revert of the following commits: - c3826f8, a refactoring piece that moved the hex decoding code to src/common/. This code was cleaned up by aef8948, as it originally included no overflow checks in the same way as the base64 routines in src/common/ used by SCRAM, making it unsafe for its purpose. - aef8948, a more advanced refactoring of the hex encoding/decoding code to src/common/ that added sanity checks on the result buffer for hex decoding and encoding. As reported by Hans Buschmann, those overflow checks are expensive, and it is possible to see a performance drop in the decoding/encoding of bytea or LOs the longer they are. Simple SQLs working on large bytea values show a clear difference in perf profile. - ccf4e27, a cleanup made possible by aef8948. The reverts of all those commits bring back the performance of hex decoding and encoding back to what it was in ~13. Fow now and post-beta3, this is the simplest option. Reported-by: Hans Buschmann Discussion: https://postgr.es/m/1629039545467.80333@nidsa.net Backpatch-through: 14
* Fix check_agg_arguments' examination of aggregate FILTER clauses.Tom Lane2021-08-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Recursion into the FILTER clause was mis-implemented, such that a relevant Var or Aggref at the very top of the FILTER clause would be ignored. (Of course, that'd have to be a plain boolean Var or boolean-returning aggregate.) The consequence would be mis-identification of the correct semantic level of the aggregate, which could lead to not-per-spec query behavior. If the FILTER expression is an aggregate, this could also lead to failure to issue an expected "aggregate function calls cannot be nested" error, which would likely result in a core dump later on, since the planner and executor aren't expecting such cases to appear. The root cause is that commit b560ec1b0 blindly copied some code that assumed it's recursing into a List, and thus didn't examine the top-level node. To forestall questions about why this call doesn't look like the others, as well as possible future copy-and-paste mistakes, let's change all three check_agg_arguments_walker calls in check_agg_arguments, even though only the one for the filter clause is really broken. Per bug #17152 from Zhiyong Wu. This has been wrong since we implemented FILTER, so back-patch to all supported versions. (Testing suggests that pre-v11 branches manage to avoid crashing in the bad-Aggref case, thanks to "redundant" checks in ExecInitAgg. But I'm not sure how thorough that protection is, and anyway the wrong-behavior issue remains, so fix 9.6 and 10 too.) Discussion: https://postgr.es/m/17152-c7f906cc1a88e61b@postgresql.org
* Fix pg_amcheck --skip option parameter handlingDaniel Gustafsson2021-08-18
| | | | | | | | | | | | | | | The skip options set for all-visible and all-frozen were incorrect as they used space rather than hyphen, causing a syntax error when invoked. Also, the option for not skipping any pages at all, none, was documented but not implemented. Backpatch through 14 where pg_amcheck was introduced. Bug: #17149 Reported-by: Chen Jiaoqian <chenjq.jy@fujitsu.com> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Discussion: https://postgr.es/m/17149-5918ea748da36b15@postgresql.org Backpatch-through: 14
* Prevent ALTER TYPE/DOMAIN/OPERATOR from changing extension membership.Tom Lane2021-08-17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If recordDependencyOnCurrentExtension is invoked on a pre-existing, free-standing object during an extension update script, that object will become owned by the extension. In our current code this is possible in three cases: * Replacing a "shell" type or operator. * CREATE OR REPLACE overwriting an existing object. * ALTER TYPE SET, ALTER DOMAIN SET, and ALTER OPERATOR SET. The first of these cases is intentional behavior, as noted by the existing comments for GenerateTypeDependencies. It seems like appropriate behavior for CREATE OR REPLACE too; at least, the obvious alternatives are not better. However, the fact that it happens during ALTER is an artifact of trying to share code (GenerateTypeDependencies and makeOperatorDependencies) between the CREATE and ALTER cases. Since an extension script would be unlikely to ALTER an object that didn't already belong to the extension, this behavior is not very troubling for the direct target object ... but ALTER TYPE SET will recurse to dependent domains, and it is very uncool for those to become owned by the extension if they were not already. Let's fix this by redefining the ALTER cases to never change extension membership, full stop. We could minimize the behavioral change by only changing the behavior when ALTER TYPE SET is recursing to a domain, but that would complicate the code and it does not seem like a better definition. Per bug #17144 from Alex Kozhemyakin. Back-patch to v13 where ALTER TYPE SET was added. (The other cases are older, but since they only affect the directly-named object, there's not enough of a problem to justify changing the behavior further back.) Discussion: https://postgr.es/m/17144-e67d7a8f049de9af@postgresql.org
* Reduce assumptions about locale's behavior in new regex tests.Tom Lane2021-08-17
| | | | | | | | I was overoptimistic to assume that UTF8-based locales would all consider U+1500 to be a member of the [[:alpha:]] char class. Tweak the test cases added by commit 78a843f11 to avoid that assumption. We might need to lobotomize them further, but this should be enough to fix the early buildfarm reports.
* Improve regex compiler's arc moving/copying logic.Tom Lane2021-08-17
| | | | | | | | | | | | | | | | | | | | | | | | The functions moveins(), copyins(), moveouts(), copyouts() are required to preserve the invariant that there are no duplicate arcs in the regex's NFA. Spencer's original implementation of them was O(N^2) since it checked separately for a match to each source arc. In commit 579840ca0 I improved that by adding sort/merge logic to be used if more than a few arcs are to be moved/copied. However, I now realize that that missed a bet. At many call sites, the target state is newly made and cannot have any existing in-arcs (respectively out-arcs) that could be duplicates. So spending any cycles at all on checking for duplicates is wasted effort; in these cases we can just blindly move/copy all the source arcs. Add code paths to do that. It turns out that for copyins()/copyouts(), *all* the call sites have this property, making all the "improved" logic in them flat out unreachable. Perhaps we'll need the full capability again someday, so I just #ifdef'd those paths out rather than removing them entirely. In passing, add a few test cases to improve code coverage in this area as well as in regc_locale.c/regc_pg_locale.c. Discussion: https://postgr.es/m/810272.1629064063@sss.pgh.pa.us
* Improved ECPG warning as suggested by Michael Paquier and removed test caseMichael Meskes2021-08-17
| | | | that triggers the warning during regression tests.
* Set type identifier on BIODaniel Gustafsson2021-08-17
| | | | | | | | | | | | | | | | | | | | | | | | | In OpenSSL there are two types of BIO's (I/O abstractions): source/sink and filters. A source/sink BIO is a source and/or sink of data, ie one acting on a socket or a file. A filter BIO takes a stream of input from another BIO and transforms it. In order for BIO_find_type() to be able to traverse the chain of BIO's and correctly find all BIO's of a certain type they shall have the type bit set accordingly, source/sink BIO's (what PostgreSQL implements) use BIO_TYPE_SOURCE_SINK and filter BIO's use BIO_TYPE_FILTER. In addition to these, file descriptor based BIO's should have the descriptor bit set, BIO_TYPE_DESCRIPTOR. The PostgreSQL implementation didn't set the type bits, which went unnoticed for a long time as it's only really relevant for code auditing the OpenSSL installation, or doing similar tasks. It is required by the API though, so this fixes it. Backpatch through 9.6 as this has been wrong for a long time. Author: Itamar Gafni Discussion: https://postgr.es/m/SN6PR06MB39665EC10C34BB20956AE4578AF39@SN6PR06MB3966.namprd06.prod.outlook.com Backpatch-through: 9.6
* Revert analyze support for partitioned tablesAlvaro Herrera2021-08-16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This reverts the following commits: 1b5617eb844cd2470a334c1d2eec66cf9b39c41a Describe (auto-)analyze behavior for partitioned tables 0e69f705cc1a3df273b38c9883fb5765991e04fe Set pg_class.reltuples for partitioned tables 41badeaba8beee7648ebe7923a41c04f1f3cb302 Document ANALYZE storage parameters for partitioned tables 0827e8af70f4653ba17ed773f123a60eadd9f9c9 autovacuum: handle analyze for partitioned tables There are efficiency issues in this code when handling databases with large numbers of partitions, and it doesn't look like there isn't any trivial way to handle those. There are some other issues as well. It's now too late in the cycle for nontrivial fixes, so we'll have to let Postgres 14 users continue to manually deal with ANALYZE their partitioned tables, and hopefully we can fix the issues for Postgres 15. I kept [most of] be280cdad298 ("Don't reset relhasindex for partitioned tables on ANALYZE") because while we added it due to 0827e8af70f4, it is a good bugfix in its own right, since it affects manual analyze as well as autovacuum-induced analyze, and there's no reason to revert it. I retained the addition of relkind 'p' to tables included by pg_stat_user_tables, because reverting that would require a catversion bump. Also, in pg14 only, I keep a struct member that was added to PgStat_TabStatEntry to avoid breaking compatibility with existing stat files. Backpatch to 14. Discussion: https://postgr.es/m/20210722205458.f2bug3z6qzxzpx2s@alap3.anarazel.de
* Reduce memory consumption for pending invalidation messages.Tom Lane2021-08-16
| | | | | | | | | | | | | | | | The existing data structures in inval.c are fairly inefficient for the common case of a command or subtransaction that registers a small number of cache invalidation events. While this doesn't matter if we commit right away, it can build up to a lot of bloat in a transaction that contains many DDL operations. By making a few more assumptions about the expected use-case, we can switch to a representation using densely-packed arrays. Although this eliminates some data-copying, it doesn't seem to make much difference time-wise. But the space consumption decreases substantially. Patch by me; thanks to Nathan Bossart for review. Discussion: https://postgr.es/m/2380555.1622395376@sss.pgh.pa.us
* Emit namespace in the post-copy errmsgDaniel Gustafsson2021-08-16
| | | | | | | | | | | | | | During a VACUUM or CLUSTER command, the initial output emits a fully qualified relation path with namespace. The post-action errmsg only emitted the relation name however, which may lead to hard to parse output when using multiple jobs with vacuumdb as the output from different jobs may be interleaved. Include the full path in the post-action errmsg to be consistent with the initial errmsg. Author: Mike Fiedler <miketheman@gmail.com> Reviewed-by: Corey Huinker <corey.huinker@gmail.com> Discussion: https://postgr.es/m/CAMerE0oz+8G-aORZL_BJcPxnBqewZAvND4bSUysjz+r-oT1BxQ@mail.gmail.com
* Use direct function calls for pg_popcount{32,64} on non-x86 platformsJohn Naylor2021-08-16
| | | | | | | | | | | | Previously, all pg_popcount{32,64} calls were indirected through a function pointer, even though we had no fast implementation for non-x86 platforms. Instead, for those platforms use wrappers around the pg_popcount{32,64}_slow functions. Review and additional hacking by David Rowley Reviewed by Álvaro Herrera Discussion: https://www.postgresql.org/message-id/flat/CAFBsxsE7otwnfA36Ly44zZO%2Bb7AEWHRFANxR1h1kxveEV%3DghLQ%40mail.gmail.com
* Clarify initdb --sync-only help message and docsDaniel Gustafsson2021-08-16
| | | | | | | | | | | The initdb help message for --sync-only was a bit terse, and not really self-explanatory. Make it clearer that initdb --sync-only will exit after syncing, and expand the docs with a note on when the option can be useful. Also align the help output with others that exit immediately. Author: Nathan Bossart, Gurjeet Singh Discussion: https://postgr.es/m/CABwTF4U6hbNNE1bv=LxQdJybmUdZ5NJQ9rKY9tN82NXM8QH+iQ@mail.gmail.com
* Refresh apply delay on reload of recovery_min_apply_delay at recoveryMichael Paquier2021-08-16
| | | | | | | | | | | | | | | | | This commit ensures that the wait interval in the replay delay loop waiting for an amount of time defined by recovery_min_apply_delay is correctly handled on reload, recalculating the delay if this GUC value is updated, based on the timestamp of the commit record being replayed. The previous behavior would be problematic for example with replay still waiting even if the delay got reduced or just cancelled. If the apply delay was increased to a larger value, the wait would have just respected the old value set, finishing earlier. Author: Soumyadeep Chakraborty, Ashwin Agrawal Reviewed-by: Kyotaro Horiguchi, Michael Paquier Discussion: https://postgr.es/m/CAE-ML+93zfr-HLN8OuxF0BjpWJ17O5dv1eMvSE5jsj9jpnAXZA@mail.gmail.com Backpatch-through: 9.6
* Un-break s_lock_test.Tom Lane2021-08-13
| | | | | | | | | | Commit 80abbeba2 evidently didn't bother checking this code. Also, list the generated executable in .gitignore (so it's been a REALLY long time since anyone tried this). Noted while trying out RISC-V spinlock patch. Given that this has been broken for 5 years and nobody noticed, it's likely not worth back-patching.
* Add RISC-V spinlock support in s_lock.h.Tom Lane2021-08-13
| | | | | | | | | | | | | | | Like the ARM case, just use gcc's __sync_lock_test_and_set(); that will compile into AMOSWAP.W.AQ which does what we need. At some point it might be worth doing some work on atomic ops for RISC-V, but this should be enough for a creditable port. Back-patch to all supported branches, just in case somebody wants to try them on RISC-V. Marek Szuba Discussion: https://postgr.es/m/dea97b6d-f55f-1f6d-9109-504aa7dfa421@gentoo.org
* pg_amcheck: Message style and structuring improvementsPeter Eisentraut2021-08-13
|
* Remove support for background workers without BGWORKER_SHMEM_ACCESS.Andres Freund2021-08-13
| | | | | | | | | | | | | | | | | | | | | | | Background workers without shared memory access have been broken on EXEC_BACKEND / windows builds since shortly after background workers have been introduced, without that being reported. Clearly they are not commonly used. The problem is that bgworker startup requires to be attached to shared memory in EXEC_BACKEND child processes. StartBackgroundWorker() detaches from shared memory for unconnected workers, but at that point we already have initialized subsystems referencing shared memory. Fixing this problem is not entirely trivial, so removing the option to not be connected to shared memory seems the best way forward. In most use cases the advantages of being connected to shared memory far outweigh the disadvantages. As there have been no reports about this issue so far, we have decided that it is not worth trying to address the problem in the back branches. Per discussion with Alvaro Herrera, Robert Haas and Tom Lane. Author: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/20210802065116.j763tz3vz4egqy3w@alap3.anarazel.de
* Fix typo.Andres Freund2021-08-13
| | | | | Reported-By: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/YRIlNQhLNfx555Nx@paquier.xyz
* Fix connection handling for DEALLOCATE and DESCRIBE statementsMichael Meskes2021-08-13
| | | | | | | | | | After binding a statement to a connection with DECLARE STATEMENT the connection was still not used for DEALLOCATE and DESCRIBE statements. This patch fixes that, adds a missing warning and cleans up the code. Author: Hayato Kuroda Reviewed-by: Kyotaro Horiguchi, Michael Paquier Discussion: https://postgr.es/m/TYAPR01MB5866BA57688DF2770E2F95C6F5069%40TYAPR01MB5866.jpnprd01.prod.outlook.com
* Fix sslsni connparam boolean checkDaniel Gustafsson2021-08-13
| | | | | | | | | | | | | | The check for sslsni only checked for existence of the parameter but not for the actual value of the param. This meant that the SNI extension was always turned on. Fix by inspecting the value of sslsni and only activate the SNI extension iff sslsni has been enabled. Also update the docs to be more in line with how other boolean params are documented. Backpatch to 14 where sslsni was first implemented. Reviewed-by: Tom Lane Backpatch-through: 14, where sslni was added
* Fix incorrect hash table resizing code in simplehash.hDavid Rowley2021-08-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This fixes a bug in simplehash.h which caused an incorrect size mask to be used when the hash table grew to SH_MAX_SIZE (2^32). The code was incorrectly setting the size mask to 0 when the hash tables reached the maximum possible number of buckets. This would result always trying to use the 0th bucket causing an infinite loop of trying to grow the hash table due to there being too many collisions. Seemingly it's not that common for simplehash tables to ever grow this big as this bug dates back to v10 and nobody seems to have noticed it before. However, probably the most likely place that people would notice it would be doing a large in-memory Hash Aggregate with something close to at least 2^31 groups. After this fix, the code now works correctly with up to within 98% of 2^32 groups and will fail with the following error when trying to insert any more items into the hash table: ERROR: hash table size exceeded However, the work_mem (or hash_mem_multiplier in newer versions) settings will generally cause Hash Aggregates to spill to disk long before reaching that many groups. The minimal test case I did took a work_mem setting of over 192GB to hit the bug. simplehash hash tables are used in a few other places such as Bitmap Index Scans, however, again the size that the hash table can become there is also limited to work_mem and it would take a relation of around 16TB (2^31) pages and a very large work_mem setting to hit this. With smaller work_mem values the table would become lossy and never grow large enough to hit the problem. Author: Yura Sokolov Reviewed-by: David Rowley, Ranier Vilela Discussion: https://postgr.es/m/b1f7f32737c3438136f64b26f4852b96@postgrespro.ru Backpatch-through: 10, where simplehash.h was added
* Make EXEC_BACKEND more convenient on macOS.Thomas Munro2021-08-13
| | | | | | | | | | | | | | | | | It's hard to disable ASLR on current macOS releases, for testing with -DEXEC_BACKEND. You could already set the environment variable PG_SHMEM_ADDR to something not likely to collide with mappings created earlier in process startup. Let's also provide a default value that works on current releases and architectures, for developer convenience. As noted in the pre-existing comment, this is a horrible hack, but -DEXEC_BACKEND is only used by Unix-based PostgreSQL developers for testing some otherwise Windows-only code paths, so it seems excusable. Back-patch to all supported branches. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/20210806032944.m4tz7j2w47mant26%40alap3.anarazel.de
* Use appropriate tuple descriptor in FDW batchingTomas Vondra2021-08-12
| | | | | | | | | | | | | | | The FDW batching code was using the same tuple descriptor both for all slots (regular and plan slots), but that's incorrect - the subplan may use a different descriptor. Currently this is benign, because batching is used only for INSERTs, and in that case the descriptors always match. But that would change if we allow batching UPDATEs. Fix by copying the appropriate tuple descriptor. Backpatch to 14, where the FDW batching was implemented. Author: Amit Langote Backpatch-through: 14, where FDW batching was added Discussion: https://postgr.es/m/CA%2BHiwqEWd5B0-e-RvixGGUrNvGkjH2s4m95%3DJcwUnyV%3Df0rAKQ%40mail.gmail.com
* Speed up generation of Unicode hash functions.John Naylor2021-08-12
| | | | | | | | | | | | Sets of Unicode keys are picky about the primes used when generating a perfect hash function for them. Callers can spend many seconds iterating through all the possible combinations of candidate multipliers and seeds to find one that works. Unicode updates typically happen only once a year, but it still makes development and testing of Unicode scripts unnecessarily slow. To fix, iterate over the primes in the innermost loop. This does not change any existing functions checked into the tree.
* Fix grammar mistake in hash index READMEJohn Naylor2021-08-12
| | | | | | Dilip Kumar Discussion: https://www.postgresql.org/message-id/CAFiTN-tjZbuY6vy7kZZ6xO%2BD4mVcO5wOPB5KiwJ3AHhpytd8fg%40mail.gmail.com
* Avoid unnecessary shared invalidations in ROLLBACK PREPAREDMichael Paquier2021-08-12
| | | | | | | | | | The performance gain is minimal, but this makes the logic more consistent with AtEOXact_Inval(). No other invalidation is needed in this case as PREPARE takes already care of sending any local ones. Author: Liu Huailing Reviewed-by: Tom Lane, Michael Paquier Discussion: https://postgr.es/m/OSZPR01MB6215AA84D71EF2B3D354CF86BE139@OSZPR01MB6215.jpnprd01.prod.outlook.com
* Fix segfault during EvalPlanQual with mix of local and foreign partitions.Heikki Linnakangas2021-08-12
| | | | | | | | | | | | | | | | | | | | | It's not sensible to re-evaluate a direct-modify Foreign Update or Delete during EvalPlanQual. However, ExecInitForeignScan() can still get called if a table mixes local and foreign partitions. EvalPlanQualStart() left the es_result_relations array uninitialized in the child EPQ EState, but ExecInitForeignScan() still expected to find it. That caused a segfault. Fix by skipping the es_result_relations lookup during EvalPlanQual processing. To make things a bit more robust, also skip the BeginDirectModify calls, and add a runtime check that ExecForeignScan() is not called on direct-modify foreign scans during EvalPlanQual processing. This is new in v14, commit 1375422c782. Before that, EvalPlanQualStart() copied the whole ResultRelInfo array to the EPQ EState. Backpatch to v14. Report and diagnosis by Andrey Lepikhov. Discussion: https://www.postgresql.org/message-id/cb2b808d-cbaa-4772-76ee-c8809bafcf3d%40postgrespro.ru
* Remove unused regression test certificate server-ssDaniel Gustafsson2021-08-10
| | | | | | | | The server-ss certificate was included in e39250c64 but was never used in the TLS regression tests so remove. Author: Jacob Champion Discussion: https://postgr.es/m/d15a9838344ba090e09fd866abf913584ea19fb7.camel@vmware.com
* Add tab completion for DECLARE .. ASENSITIVE in psqlMichael Paquier2021-08-10
| | | | | | | This option has been introduced in dd13ad9. Author: Shinya Kato Discussion: https://postgr.es/m/TYAPR01MB289665526B76DA29DC70A031C4F09@TYAPR01MB2896.jpnprd01.prod.outlook.com
* Add call to object access hook at the end of table rewrite in ALTER TABLEMichael Paquier2021-08-10
| | | | | | | | | | | | | | | | | | | | | | | | | | | ALTER TABLE .. SET {LOGGED,UNLOGGED,ACCESS METHOD} would never do a table-level object access hook, which was inconsistent with SET TABLESPACE. Note that contrary to SET TABLESPACE, the no-op case is left off for those commands as this requires tracking if commands have been called, but they may not execute a physical rewrite. Another thing worth noting is that the physical file swap at the end of a rewrite does a couple of access calls for internal objects created for the swap operation (internal objects are for example skipped by the tests of sepgsql), but this does not trigger the hook for the table on which the operation is done. f41872d, that added support for SET LOGGED/UNLOGGED in ALTER TABLE, visibly forgot to consider that. Based on what I checked, two regression tests of sepgsql in ddl.sql are going to log more information with this test, something that buildfarm member rhinoceros will tell soon enough. I am not completely sure of their format though, so these are not refreshed yet. This is arguably a bug, but no backpatch is done as this could cause a behavior change for anybody using object access hooks. Reported-by: Jeff Davis Discussion: https://postgr.es/m/YQJKV29/1a60uG68@paquier.xyz
* Let regexp_replace() make use of REG_NOSUB when feasible.Tom Lane2021-08-09
| | | | | | | | | | | | | | | | | | If the replacement string doesn't contain \1...\9, then we don't need sub-match locations, so we can use the REG_NOSUB optimization here too. There's already a pre-scan of the replacement string to look for backslashes, so extend that to check for digits, and refactor to allow that to happen before we compile the regexp. While at it, try to speed up the pre-scan by using memchr() instead of a handwritten loop. It's likely that this is lost in the noise compared to the regexp processing proper, but maybe not. In any case, this coding is shorter. Also, add some test cases to improve the poor coverage of appendStringInfoRegexpSubstr(). Discussion: https://postgr.es/m/3534632.1628536485@sss.pgh.pa.us
* Fix bogus assertion in BootstrapModeMain().Andres Freund2021-08-09
| | | | | | | The assertion was always true, as written, thanks to me "simplifying" it before commit. Per coverity and Tom Lane.
* Avoid determining regexp subexpression matches, when possible.Tom Lane2021-08-09
| | | | | | | | | | | | | | | | | | | | | Identifying the precise match locations for parenthesized subexpressions is a fairly expensive task given the way our regexp engine works, both at regexp compile time (where we must create an optimized NFA for each parenthesized subexpression) and at runtime (where determining exact match locations requires laborious search). Up to now we've made little attempt to optimize this situation. This patch identifies cases where we know at compile time that we won't need to know subexpression match locations, and teaches the regexp compiler to not bother creating per-subexpression regexps for parenthesis pairs that are not referenced by backrefs elsewhere in the regexp. (To preserve semantics, we obviously still have to pin down the match locations of backref references.) Users could have obtained the same results before this by being careful to write "non capturing" parentheses wherever possible, but few people bother with that. Discussion: https://postgr.es/m/2219936.1628115334@sss.pgh.pa.us
* Remove some special cases from MSVC build scriptsDavid Rowley2021-08-09
| | | | | | | | | | | | | | | | | | Here we add additional parsing of Makefiles to determine when to add references to libpgport and libpgcommon. We also remove the need for adding the current contrib_extrasource by adding sine very basic logic to implement the Makefile rules which add .l and .y files when they exist for a given .o file in the Makefile. This is just some very basic additional parsing of Makefiles to try to keep things more consistent between builds using make and MSVC builds. This happens to work with how our current Makefiles are laid out, but it could easily be broken in the future if someone chooses do something in the Makefile that we don't have parsing support for. We will cross that bridge when we come to it. Author: David Rowley Discussion: https://postgr.es/m/CAApHDvoPULi5JW3933NxgwxOmu9Ncvpcyt87UhEHAUX16QqmpA@mail.gmail.com
* Use ExplainPropertyInteger for queryid in EXPLAINDavid Rowley2021-08-09
| | | | | | | | | | | This saves a few lines of code. Also add a comment to mention why we use ExplainPropertyInteger instead of ExplainPropertyUInteger given that queryid is a uint64 type. Author: David Rowley Reviewed-by: Julien Rouhaud Discussion: https://postgr.es/m/CAApHDvqhSLYpSU_EqUdN39w9Uvb8ogmHV7_3YhJ0S3aScGBjsg@mail.gmail.com Backpatch-through: 14, where this code was originally added
* Fix typo in 022_twophase_cascade.pl.Amit Kapila2021-08-09
| | | | | Author: Peter Smith Discussion: https://postgr.es/m/CAHut+Pta=zo8G1DWVVg-LU6b_JvHHCueC=AKVpKJOrwLzj9EZA@mail.gmail.com
* Add POPCNT support for MSVC x86_64 buildsDavid Rowley2021-08-09
| | | | | | | | | | | | | | | | | 02a6a54ec added code to make use of the POPCNT instruction when available for many of our common platforms. Here we do the same for MSVC for x86_64 machines. MSVC's intrinsic functions for popcnt seem to differ from GCCs in that they always appear to emit the popcnt instructions. In GCC the behavior will depend on if the source file was compiled with -mpopcnt or not. For this reason, the MSVC intrinsic function has been lumped into the pg_popcount*_asm function, however doing that sort of invalidates the name of that function, so let's rename it to pg_popcount*_fast(). Author: David Rowley Reviewed-by: John Naylor Discussion: https://postgr.es/m/CAApHDvqL3cbbK%3DGzNcwzsNR9Gi%2BaUvTudKkC4XgnQfXirJ_oRQ%40mail.gmail.com
* Remove some unnecessary casts in format argumentsPeter Eisentraut2021-08-08
| | | | | | We can use %zd or %zu directly, no need to cast to int. Conversely, some code was casting away from int when it could be using %d directly.
* Check the size in COPY_POINTER_FIELDPeter Eisentraut2021-08-08
| | | | | | instead of making each caller do it. Discussion: https://www.postgresql.org/message-id/flat/c1097590-a6a4-486a-64b1-e1f9cc0533ce@enterprisedb.com
* Change NestPath node to contain JoinPath nodePeter Eisentraut2021-08-08
| | | | | | | This makes the structure of all JoinPath-derived nodes the same, independent of whether they have additional fields. Discussion: https://www.postgresql.org/message-id/flat/c1097590-a6a4-486a-64b1-e1f9cc0533ce@enterprisedb.com