aboutsummaryrefslogtreecommitdiff
path: root/src/test/modules
Commit message (Collapse)AuthorAge
...
* Fix various typos, grammar and code style in comments and docsMichael Paquier2022-01-25
| | | | | | | | | This fixes a set of issues that have accumulated over the past months (or years) in various code areas. Most fixes are related to some recent additions, as of the development of v15. Author: Justin Pryzby Discussion: https://postgr.es/m/20220124030001.GQ23027@telsasoft.com
* Add new simple TAP test for tablespaces, attempt II.Thomas Munro2022-01-21
| | | | | | | See commit message for d1511fe1b040853f6e10d353e56b42bb96ae239d. This new version attempts to fix path translation problem on MSYS/Windows. Discussion: https://postgr.es/m/20220117055326.GD756210%40rfd.leadboat.com
* pg_upgrade: Preserve relfilenodes and tablespace OIDs.Robert Haas2022-01-17
| | | | | | | | | | | | | | | | | | | | | | | | Currently, database OIDs, relfilenodes, and tablespace OIDs can all change when a cluster is upgraded using pg_upgrade. It seems better to preserve them, because (1) it makes troubleshooting pg_upgrade easier, since you don't have to do a lot of work to match up files in the old and new clusters, (2) it allows 'rsync' to save bandwidth when used to re-sync a cluster after an upgrade, and (3) if we ever encrypt or sign blocks, we would likely want to use a nonce that depends on these values. This patch only arranges to preserve relfilenodes and tablespace OIDs. The task of preserving database OIDs is left for another patch, since it involves some complexities that don't exist in these cases. Database OIDs have a similar issue, but there are some tricky points in that case that do not apply to these cases, so that problem is left for another patch. Shruthi KC, based on an earlier patch from Antonin Houska, reviewed and with some adjustments by me. Discussion: http://postgr.es/m/CA+TgmoYgTwYcUmB=e8+hRHOFA0kkS6Kde85+UNdon6q7bt1niQ@mail.gmail.com
* Revert "Add new simple TAP test for tablespaces."Thomas Munro2022-01-15
| | | | | | This reverts commit d1511fe1b040853f6e10d353e56b42bb96ae239d. Discussion: https://postgr.es/m/CA%2BhUKG%2BGBC-6QhOKt6Y7ccrXSjbRHB7Di295%3D0rAGhE7a7hSrQ%40mail.gmail.com
* Add new simple TAP test for tablespaces.Thomas Munro2022-01-15
| | | | | | | | | | | The tablespace tests in the main regression tests have been changed to use "in-place" tablespaces, so that they work when streamed to a replica on the same host. Add a new TAP test that exercises tablespaces with absolute paths, for coverage. Reviewed-by: Andres Freund <andres@anarazel.de> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/CA%2BhUKGKpRWQ9SxdxxDmTBCJoR0YnFpMBe7kyzY8SUQk%2BHeskxg%40mail.gmail.com
* Update copyright for 2022Bruce Momjian2022-01-07
| | | | Backpatch-through: 10
* Revert changes about warnings/errors for placeholders.Tom Lane2021-12-27
| | | | | | | | Revert commits 5609cc01c, 2ed8a8cc5, and 75d22069e until we have a less broken idea of how this should work in parallel workers. Per buildfarm. Discussion: https://postgr.es/m/1640909.1640638123@sss.pgh.pa.us
* Rename EmitWarningsOnPlaceholders() to MarkGUCPrefixReserved().Tom Lane2021-12-27
| | | | | | | | | This seems like a clearer name for what it does now. Provide a compatibility macro so that extensions don't have to convert to the new name right away. Discussion: https://postgr.es/m/116024.1640111629@sss.pgh.pa.us
* Add missing EmitWarningsOnPlaceholders() calls.Tom Lane2021-12-21
| | | | | | | | | | | | | | Extensions that define any custom GUCs should call EmitWarningsOnPlaceholders after doing so, to help catch misspellings. Many of our contrib modules hadn't gotten the memo on that, though. Also add such calls to src/test/modules extensions that have GUCs. While these aren't really user-facing, they should illustrate good practice not faulty practice. Shinya Kato Discussion: https://postgr.es/m/524fa2c0a34f34b68fbfa90d0760d515@oss.nttdata.com
* Ignore BRIN indexes when checking for HOT udpatesTomas Vondra2021-11-30
| | | | | | | | | | | | | | | | When determining whether an index update may be skipped by using HOT, we can ignore attributes indexed only by BRIN indexes. There are no index pointers to individual tuples in BRIN, and the page range summary will be updated anyway as it relies on visibility info. This also removes rd_indexattr list, and replaces it with rd_attrsvalid flag. The list was not used anywhere, and a simple flag is sufficient. Patch by Josef Simanek, various fixes and improvements by me. Author: Josef Simanek Reviewed-by: Tomas Vondra, Alvaro Herrera Discussion: https://postgr.es/m/CAFp7QwpMRGcDAQumN7onN9HjrJ3u4X3ZRXdGFT0K5G2JWvnbWg%40mail.gmail.com
* Replace random(), pg_erand48(), etc with a better PRNG API and algorithm.Tom Lane2021-11-28
| | | | | | | | | | | | | | | | | | | Standardize on xoroshiro128** as our basic PRNG algorithm, eliminating a bunch of platform dependencies as well as fundamentally-obsolete PRNG code. In addition, this API replacement will ease replacing the algorithm again in future, should that become necessary. xoroshiro128** is a few percent slower than the drand48 family, but it can produce full-width 64-bit random values not only 48-bit, and it should be much more trustworthy. It's likely to be noticeably faster than the platform's random(), depending on which platform you are thinking about; and we can have non-global state vectors easily, unlike with random(). It is not cryptographically strong, but neither are the functions it replaces. Fabien Coelho, reviewed by Dean Rasheed, Aleksander Alekseev, and myself Discussion: https://postgr.es/m/alpine.DEB.2.22.394.2105241211230.165418@pseudo
* Move Perl test modules to a better namespaceAndrew Dunstan2021-10-24
| | | | | | | | | | | | | | | The five modules in our TAP test framework all had names in the top level namespace. This is unwise because, even though we're not exporting them to CPAN, the names can leak, for example if they are exported by the RPM build process. We therefore move the modules to the PostgreSQL::Test namespace. In the process PostgresNode is renamed to Cluster, and TestLib is renamed to Utils. PostgresVersion becomes simply PostgreSQL::Version, to avoid possible confusion about what it's the version of. Discussion: https://postgr.es/m/aede93a4-7d92-ef26-398f-5094944c2504@dunslane.net Reviewed by Erik Rijkers and Michael Paquier
* Add module build directory to the PATH for TAP testsAndrew Dunstan2021-10-22
| | | | | | | | | | | | | | | | | | For non-MSVC builds this is make's $(CURDIR), while for MSVC builds it is $topdir/$Config/$module. The directory is added as the second element in the PATH, so that the install location takes precedence, but the added PATH element takes precedence over the rest of the PATH. The reason for this is to allow tests to find built products that are not installed, such as the libpq_pipeline test driver. The libpq_pipeline test is adjusted to take advantage of this. Based on a suggestion from Andres Freund. Backpatch to release 14. Discussion: https://postgr.es/m/4941f5a5-2d50-1a0e-6701-14c5fefe92d6@dunslane.net
* shm_mq: Update mq_bytes_written less often.Robert Haas2021-10-14
| | | | | | | | | | | | | | Do not update shm_mq's mq_bytes_written until we have written an amount of data greater than 1/4th of the ring size, unless the caller of shm_mq_send(v) requests a flush at the end of the message. This reduces the number of calls to SetLatch(), and also the number of CPU cache misses, considerably, and thus makes shm_mq significantly faster. Dilip Kumar, reviewed by Zhihong Yu and Tomas Vondra. Some minor cosmetic changes by me. Discussion: http://postgr.es/m/CAFiTN-tVXqn_OG7tHNeSkBbN+iiCZTiQ83uakax43y1sQb2OBA@mail.gmail.com
* Fix loop variable signednessPeter Eisentraut2021-10-06
|
* Reference test binary using TESTDIR in 001_libpq_pipeline.pl.Andres Freund2021-10-01
| | | | | | | | | | | The previous approach didn't really work on windows, due to the PATH separator being ';' not ':'. Instead of making the PATH change more complicated, reference the binary using the TESTDIR environment. Reported-By: Andres Freund <andres@anarazel.de> Suggested-By: Andrew Dunstan <andrew@dunslane.net> Discussion: https://postgr.es/m/20210930214040.odkdd42vknvzifm6@alap3.anarazel.de Backpatch: 14-, where the test was introduced.
* Remove some unused variables in TAP testsMichael Paquier2021-09-06
| | | | | Author: Amul Sul Discussion: https://postgr.es/m/CAAJ_b96xuFh4JZE6p-zhLyDu7q=NbxJfb1z_yeAu6t-MqaBC+Q@mail.gmail.com
* Fix regexp misbehavior with capturing parens inside "{0}".Tom Lane2021-08-24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Regexps like "(.){0}...\1" drew an "invalid backreference number". That's not unreasonable on its face, since the capture group will never be matched if it's iterated zero times. However, other engines such as Perl's don't complain about this, nor do we throw an error for related cases such as "(.)|\1", even though that backref can never succeed either. Also, if the zero-iterations case happens at runtime rather than compile time --- say, "(x)*...\1" when there's no "x" to be found --- that's not an error, we just deem the backref to not match. Making this even less defensible, no error was thrown for nested cases such as "((.)){0}...\2"; and to add insult to injury, those cases could result in assertion failures instead. (It seems that nothing especially bad happened in non-assert builds, though.) Let's just fix it so that no error is thrown and instead the backref is deemed to never match, so that compile-time detection of no iterations behaves the same as run-time detection. Per report from Mark Dilger. This appears to be an aboriginal error in Spencer's library, so back-patch to all supported versions. Pre-v14, it turns out to also be necessary to back-patch one aspect of commits cb76fbd7e/00116dee5, namely to create capture-node subREs with the begin/end states of their subexpressions, not the current lp/rp of the outer parseqatom invocation. Otherwise delsub complains that we're trying to disconnect a state from itself. This is a bit scary but code examination shows that it's safe: in the pre-v14 code, if we want to wrap iteration around the subexpression, the first thing we do is overwrite the atom's begin/end fields with new states. So the bogus values didn't survive long enough to be used for anything, except if no iteration is required, in which case it doesn't matter. Discussion: https://postgr.es/m/A099E4A8-4377-4C64-A98C-3DEDDC075502@enterprisedb.com
* Prevent regexp back-refs from sometimes matching when they shouldn't.Tom Lane2021-08-23
| | | | | | | | | | | | | | | | | | | | | | | | The recursion in cdissect() was careless about clearing match data for capturing parentheses after rejecting a partial match. This could allow a later back-reference to succeed when by rights it should fail for lack of a defined referent. To fix, think a little more rigorously about what the contract between different levels of cdissect's recursion needs to be. With the right spec, we can fix this using fewer rather than more resets of the match data; the key decision being that a failed sub-match is now explicitly responsible for clearing any matches it may have set. There are enough other cross-checks and optimizations in the code that it's not especially easy to exhibit this problem; usually, the match will fail as-expected. Plus, regexps that are even potentially vulnerable are most likely user errors, since there's just not much point in writing a back-ref that doesn't always have a referent. These facts perhaps explain why the issue hasn't been detected, even though it's almost certainly a couple of decades old. Discussion: https://postgr.es/m/151435.1629733387@sss.pgh.pa.us
* 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
* 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
* Fix use-after-free issue in regexp engine.Tom Lane2021-08-07
| | | | | | | | | | | | | | | | | | | | | | | | | Commit cebc1d34e taught parseqatom() to optimize cases where a branch contains only one, "messy", atom by getting rid of excess subRE nodes. The way we really should do that is to keep the subRE built for the "messy" child atom; but to avoid changing parseqatom's nominal API, I made it delete that node after copying its fields to the outer subRE made by parsebranch(). It seems that that actually worked at the time; but it became dangerous after ea1268f63, because that later commit allowed the lower invocation of parse() to return a subRE that was also pointed to by some v->subs[] entry. This meant we could wind up with a dangling pointer in v->subs[], allowing a later backref to misbehave, but only if that subRE struct had been reused in between. So the damage seems confined to cases like '((...))...(...\2'. To fix, do what I should have done before and modify parseqatom's API to make it possible for it to remove the caller's subRE instead of the callee's. That's safer because we know that subRE isn't complete yet, so noplace else will have a pointer to it. Per report from Mark Dilger. Back-patch to v14 where the problematic patches came in. Discussion: https://postgr.es/m/0203588E-E609-43AF-9F4F-902854231EE7@enterprisedb.com
* Silence perl warning about uninitialized valueAndrew Dunstan2021-08-01
|
* Unify PostgresNode's new() and get_new_node() methodsAndrew Dunstan2021-07-29
| | | | | | | | | There is only one constructor now for PostgresNode, with the idiomatic name 'new'. The method is not exported by the class, and must be called as "PostgresNode->new('name',[args])". All the TAP tests that use PostgresNode are modified accordingly. Third party scripts will need adjusting, which is a fairly mechanical process (I just used a sed script).
* Avoid using ambiguous word "non-negative" in error messages.Fujii Masao2021-07-28
| | | | | | | | | | | | | | | | | | | | | | The error messages using the word "non-negative" are confusing because it's ambiguous about whether it accepts zero or not. This commit improves those error messages by replacing it with less ambiguous word like "greater than zero" or "greater than or equal to zero". Also this commit added the note about the word "non-negative" to the error message style guide, to help writing the new error messages. When postgres_fdw option fetch_size was set to zero, previously the error message "fetch_size requires a non-negative integer value" was reported. This error message was outright buggy. Therefore back-patch to all supported versions where such buggy error message could be thrown. Reported-by: Hou Zhijie Author: Bharath Rupireddy Reviewed-by: Kyotaro Horiguchi, Fujii Masao Discussion: https://postgr.es/m/OS0PR01MB5716415335A06B489F1B3A8194569@OS0PR01MB5716.jpnprd01.prod.outlook.com
* Use l*_node() family of functions where appropriatePeter Eisentraut2021-07-19
| | | | | | | Instead of castNode(…, lfoo(…)) Author: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> Discussion: https://www.postgresql.org/message-id/flat/87eecahraj.fsf@wibble.ilmari.org
* libpq: Fix sending queries in pipeline aborted stateAlvaro Herrera2021-07-09
| | | | | | | | | | | | | | | | | | | | | | | | | | | | When sending queries in pipeline mode, we were careless about leaving the connection in the right state so that PQgetResult would behave correctly; trying to read further results after sending a query after having read a result with an error would sometimes hang. Fix by ensuring internal libpq state is changed properly. All the state changes were being done by the callers of pqAppendCmdQueueEntry(); it would have become too repetitious to have this logic in each of them, so instead put it all in that function and relieve callers of the responsibility. Add a test to verify this case. Without the code fix, this new test hangs sometimes. Also, document that PQisBusy() would return false when no queries are pending result. This is not intuitively obvious, and NULL would be obtained by calling PQgetResult() at that point, which is confusing. Wording by Boris Kolpackov. In passing, fix bogus use of "false" to mean "0", per Ranier Vilela. Backpatch to 14. Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Reported-by: Boris Kolpackov <boris@codesynthesis.com> Discussion: https://postgr.es/m/boris.20210624103805@codesynthesis.com
* Fix libpq state machine in pipeline modeAlvaro Herrera2021-06-29
| | | | | | | | | | | | | | | | | The original coding required that PQpipelineSync had been called before the first call to PQgetResult, and failure to do that would result in an unexpected NULL result being returned. Fix by setting the right state when a query is sent, rather than leaving it unchanged and having PQpipelineSync apply the necessary state change. A new test case to verify the behavior is added, which relies on the new PQsendFlushRequest() function added by commit a7192326c74d. Backpatch to 14, where pipeline mode was added. Reported-by: Boris Kolpackov <boris@codesynthesis.com> Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/boris.20210616110321@codesynthesis.com
* Dump public schema ownership and security labels.Noah Misch2021-06-28
| | | | | | | | | As a side effect, this corrects dumps of public schema ACLs in databases where the DBA dropped and recreated that schema. Reviewed by Asif Rehman. Discussion: https://postgr.es/m/20201229134924.GA1431748@rfd.leadboat.com
* Improve display of query results in isolation tests.Tom Lane2021-06-23
| | | | | | | | | | | | | | | | | | | Previously, isolationtester displayed SQL query results using some ad-hoc code that clearly hadn't had much effort expended on it. Field values longer than 14 characters weren't separated from the next field, and usually caused misalignment of the columns too. Also there was no visual separation of a query's result from subsequent isolationtester output. This made test result files confusing and hard to read. To improve matters, let's use libpq's PQprint() function. Although that's long since unused by psql, it's still plenty good enough for the purpose here. Like 741d7f104, back-patch to all supported branches, so that this isn't a stumbling block for back-patching isolation test changes. Discussion: https://postgr.es/m/582362.1623798221@sss.pgh.pa.us
* Add 'Portal Close' message to pipelined PQsendQuery()Alvaro Herrera2021-06-11
| | | | | | | | | | | | Commit acb7e4eb6b1c added a new implementation for PQsendQuery so that it works in pipeline mode (by using extended query protocol), but it behaves differently from the 'Q' message (in simple query protocol) used by regular implementation: the new one doesn't close the unnamed portal. Change the new code to have identical behavior to the old. Reported-by: Yura Sokolov <y.sokolov@postgrespro.ru> Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/202106072107.d4i55hdscxqj@alvherre.pgsql
* Rename PQtraceSetFlags() to PQsetTraceFlags().Noah Misch2021-06-10
| | | | | | | | | | | We have a dozen PQset*() functions. PQresultSetInstanceData() and this were the libpq setter functions having a different word order. Adopt the majority word order. Reviewed by Alvaro Herrera and Robert Haas, though this choice of name was not unanimous. Discussion: https://postgr.es/m/20210605060555.GA216695@rfd.leadboat.com
* Add Windows file version information to libpq_pipeline.exe.Noah Misch2021-06-01
|
* Fix vpath build in libpq_pipeline testPeter Eisentraut2021-05-27
| | | | | | | | The path needs to be set to refer to the build directory, not the current directory, because that's actually the source directory at that point. fix for 6abc8c2596dbfcb24f9b4d954a1465b8015118c3
* Add NO_INSTALL option to pgxsPeter Eisentraut2021-05-27
| | | | | | | | | Apply in libpq_pipeline test makefile, so that the test file is not installed into tmp_install. Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/cb9d16a6-760f-cd44-28d6-b091d5fb6ca7%40enterprisedb.com
* Prevent infinite insertion loops in spgdoinsert().Tom Lane2021-05-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Formerly we just relied on operator classes that assert longValuesOK to eventually shorten the leaf value enough to fit on an index page. That fails since the introduction of INCLUDE-column support (commit 09c1c6ab4), because the INCLUDE columns might alone take up more than a page, meaning no amount of leaf-datum compaction will get the job done. At least with spgtextproc.c, that leads to an infinite loop, since spgtextproc.c won't throw an error for not being able to shorten the leaf datum anymore. To fix without breaking cases that would otherwise work, add logic to spgdoinsert() to verify that the leaf tuple size is decreasing after each "choose" step. Some opclasses might not decrease the size on every single cycle, and in any case, alignment roundoff of the tuple size could obscure small gains. Therefore, allow up to 10 cycles without additional savings before throwing an error. (Perhaps this number will need adjustment, but it seems quite generous right now.) As long as we've developed this logic, let's back-patch it. The back branches don't have INCLUDE columns to worry about, but this seems like a good defense against possible bugs in operator classes. We already know that an infinite loop here is pretty unpleasant, so having a defense seems to outweigh the risk of breaking things. (Note that spgtextproc.c is actually the only known opclass with longValuesOK support, so that this is all moot for known non-core opclasses anyway.) Per report from Dilip Kumar. Discussion: https://postgr.es/m/CAFiTN-uxP_soPhVG840tRMQTBmtA_f_Y8N51G7DKYYqDh7XN-A@mail.gmail.com
* Initial pgindent and pgperltidy run for v14.Tom Lane2021-05-12
| | | | | | | | Also "make reformat-dat-files". The only change worthy of note is that pgindent messed up the formatting of launcher.c's struct LogicalRepWorkerId, which led me to notice that that struct wasn't used at all anymore, so I just took it out.
* Add a copyright notice to perl files lacking one.Andrew Dunstan2021-05-07
|
* Tweak behavior of pg_dump --extension with configuration tablesMichael Paquier2021-04-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | 6568cef, that introduced the option, had an inconsistent behavior when it comes to configuration tables set up by pg_extension_config_dump, as the data of all configuration tables would included in a dump even for extensions not listed by a set of --extension switches. The contents dumped changed depending on the schema where an extension was installed when an extension was not listed. For example, an extension installed under the public schema would have its configuration data not dumped even when not listed with --extension, which was inconsistent with the case of an extension installed on a non-public schema, where the configuration would be dumped. Per discussion with Noah, we have settled down to the simple rule of dumping configuration data of an extension if it is listed in --extension (default is unchanged and backward-compatible, to dump everything on sight if there are no extensions directly listed). This avoids some weird cases where the dumps depended on a --schema for one. More tests are added to cover the gap, where we cross-check more behaviors depending on --schema when an extension is not listed. Reported-by: Noah Misch Reviewed-by: Noah Misch Discussion: https://postgr.es/m/20210404220802.GA728316@rfd.leadboat.com
* Remove duplicated --no-sync switches in new tests of test_pg_dumpMichael Paquier2021-04-13
| | | | | | | These got introduced in 6568cef. Reported-by: Noah Misch Discussion: https://postgr.es/m/20210404220802.GA728316@rfd.leadboat.com
* Suppress length of Notice/Error msgs in PQtrace regress modeAlvaro Herrera2021-04-09
| | | | | | | | | | | | | | A (relatively minor) annoyance of ErrorResponse/NoticeResponse messages as printed by PQtrace() is that their length might vary when we move error messages from one source file to another, one function to another, or even when their location line numbers change number of digits. To avoid having to adjust expected files for some tests, make the regress mode of PQtrace() suppress the length word of NoticeResponse and ErrorResponse messages. Discussion: https://postgr.es/m/20210402023010.GA13563@alvherre.pgsql Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
* Fix compiler warning for MSVC in libpq_pipeline.cDavid Rowley2021-04-07
| | | | | | | | | | | | DEBUG was already defined by the MSVC toolchain for "Debug" builds. On these systems the unconditional #define DEBUG was causing a 'DEBUG': macro redefinition warning. Here we rename DEBUG to DEBUG_OUPUT and also get rid of the #define which defined this constant. This appears to have been left in the code by mistake. Discussion: https://postgr.es/m/CAApHDvqTTgDm38s4HRj03nhzhzQ1oMOj-RXFUB1pE6Bj07jyuQ@mail.gmail.com
* Support INCLUDE'd columns in SP-GiST.Tom Lane2021-04-05
| | | | | | | | | | | | | Not much to say here: does what it says on the tin. We steal a previously-always-zero bit from the nextOffset field of leaf index tuples in order to track whether there is a nulls bitmap. Otherwise it works about like included columns in other index types. Pavel Borisov, reviewed by Andrey Borodin and Anastasia Lubennikova, and rather heavily editorialized on by me Discussion: https://postgr.es/m/CALT9ZEFi-vMp4faht9f9Junb1nO3NOSjhpxTmbm1UGLMsLqiEQ@mail.gmail.com
* Fix confusion in SP-GiST between attribute type and leaf storage type.Tom Lane2021-04-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | According to the documentation, the attType passed to the opclass config function (and also relied on by the core code) is the type of the heap column or expression being indexed. But what was actually being passed was the type stored for the index column. This made no difference for user-defined SP-GiST opclasses, because we weren't allowing the STORAGE clause of CREATE OPCLASS to be used, so the two types would be the same. But it's silly not to allow that, seeing that the built-in poly_ops opclass has a different value for opckeytype than opcintype, and that if you want to do lossy storage then the types must really be different. (Thus, user-defined opclasses doing lossy storage had to lie about what type is in the index.) Hence, remove the restriction, and make sure that we use the input column type not opckeytype where relevant. For reasons of backwards compatibility with existing user-defined opclasses, we can't quite insist that the specified leafType match the STORAGE clause; instead just add an amvalidate() warning if they don't match. Also fix some bugs that would only manifest when trying to return index entries when attType is different from attLeafType. It's not too surprising that these have not been reported, because the only usual reason for such a difference is to store the leaf value lossily, rendering index-only scans impossible. Add a src/test/modules module to exercise cases where attType is different from attLeafType and yet index-only scan is supported. Discussion: https://postgr.es/m/3728741.1617381471@sss.pgh.pa.us
* Fix setvbuf()-induced crash in libpq_pipelineAlvaro Herrera2021-04-01
| | | | | | | | | | | | | | | Windows doesn't like setvbuf(..., _IOLBF) and crashes if you use it, which has been causing the libpq_pipeline failures all along ... and our own port.h has known about it for a long time: it offers PG_IOLBF that's defined to _IONBF on that platform. Follow its advice. While at it, get rid of a bogus bitshift that used a constant of the wrong size. Decorate the constant as LL to fix. While at it, remove a pointless addition that only confused matters. All as diagnosed by Tom Lane. Discussion: https://postgr.es/m/3458958.1617302154@sss.pgh.pa.us
* libpq_pipeline: Must strdup(optarg) to avoid crashAlvaro Herrera2021-04-01
| | | | | | | | | | | | | | | | I forgot to strdup() when processing argv[]. Apparently many platforms hide this mistake from users, but in those that don't you may get a program crash. Repair. Per buildfarm member drongo, which is the only one in all the buildfarm manifesting a problem here. While at it, move "numrows" processing out of the line of special cases, and make it getopt's -r instead. (A similar thing could be done to 'conninfo', but current use of the program doesn't warrant spending time on that -- nowhere else we use conninfo in so simplistic a manner.) Discussion: https://postgr.es/m/20210401124850.GA19247@alvherre.pgsql
* Remove setvbuf() call from PQtrace()Alvaro Herrera2021-03-31
| | | | | | | | | | | | | | | | | It's misplaced there -- it's not libpq's output stream to tweak in that way. In particular, POSIX says that it has to be called before any other operation on the file, so if a stream previously used by the calling application, bad things may happen. Put setvbuf() in libpq_pipeline for good measure. Also, reduce fopen(..., "w+") to just fopen(..., "w") in libpq_pipeline.c. It's not clear that this fixes anything, but we don't use w+ anywhere. Per complaints from Tom Lane. Discussion: https://postgr.es/m/3337422.1617229905@sss.pgh.pa.us
* Disable force_parallel_mode in libpq_pipelineAlvaro Herrera2021-03-31
| | | | | | | | | | | | Some buildfarm animals with force_parallel_mode=regress were failing this test because the error is reported in a parallel worker quicker than the rows that succeed. Take the opportunity to move the SET of lc_messages out of the traced section, because it's not very interesting. Diagnosed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/3304521.1617221724@sss.pgh.pa.us
* Suppress compiler warning in libpq_pipeline.c.Tom Lane2021-03-31
| | | | | | | Some compilers seem to be concerned about the possibility that recv_step is not any of the defined enum values. Silence warnings about uninitialized cmdtag in a different way than I did in 9fb9691a8.