aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* Fix GetNewTransactionId()'s interaction with xidVacLimit.Thomas Munro2019-04-12
| | | | | | | | | | | | | | | Commit ad308058 switched to returning a FullTransactionId, but failed to load the potentially updated value in the case where xidVacLimit is reached and we release and reacquire the lock. Repair, closing bug #15727. While reviewing that commit, also fix the size computation used by EstimateTransactionStateSize() and switch to the mul_size() macro traditionally used in such expressions. Author: Thomas Munro Reported-by: Roman Zharkov Discussion: https://postgr.es/m/15727-0be246e7d852d229%40postgresql.org
* Fix typos in reloptions.cMichael Paquier2019-04-12
| | | | | Author: Kirk Jamison Discussion: https://postgr.es/m/D09B13F772D2274BB348A310EE3027C6493463@g01jpexmbkw24
* Switch TAP tests of pg_rewind to use a role with minimal permissionsMichael Paquier2019-04-12
| | | | | | | | | | | | | | | | | | | | Up to now the tests of pg_rewind have been using a superuser for all the tests (which is the default of many tests actually, and something that ought to be reviewed) when involving an online source server, still it is possible to use a non-superuser role to do that as long as this role is granted permissions to execute all the source-side functions used for the rewind. This is possible since v11, and was already documented as of bfc8068. This will allow to catch up easily any change in pg_rewind if the tool begins to use more backend-side functions, so as the properties introduced by v11 are kept. Per suggestion from Peter Eisentraut. Author: Michael Paquier Reviewed-by: Magnus Hagander Discussion: https://postgr.es/m/20190411041336.GM2728@paquier.xyz
* Fix more strcmp() calls using boolean-like comparisons for result checksMichael Paquier2019-04-12
| | | | | | | | | | Such calls can confuse the reader as strcmp() uses an integer as result. The places patched here have been spotted by Thomas Munro, David Rowley and myself. Author: Michael Paquier Reviewed-by: David Rowley Discussion: https://postgr.es/m/20190411021946.GG2728@paquier.xyz
* Re-order some regression test scripts for more parallelism.Tom Lane2019-04-11
| | | | | | | | | | | | | | | | | | | Move the strings, numerology, insert, insert_conflict, select and errors tests to be parts of nearby parallel groups, instead of executing by themselves. (Moving "select" required adjusting the constraints test, which uses a table named "tmp" as select also does. There don't seem to be any other conflicts.) Move psql and stats_ext to the next parallel group, where the rules test also has a long runtime. To make it safe to run stats_ext in parallel with rules, I adjusted the latter to only dump views/rules from the pg_catalog and public schemas, which was what it was doing anyway. stats_ext makes some views in a transient schema, which now will not affect rules. Reorder serial_schedule to match parallel_schedule. Discussion: https://postgr.es/m/735.1554935715@sss.pgh.pa.us
* Speed up sort-order-comparison tests in create_index_spgist.Tom Lane2019-04-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This test script verifies that KNN searches of an SP-GiST index produce the same sort order as a seqscan-and-sort. The FULL JOINs used for that are exceedingly slow, however. Investigation shows that the problem is that the initial join is on the rank() values, and we have a lot of duplicates due to the data set containing 1000 duplicate points. We're therefore going to produce 1000000 join rows that have to be thrown away again by the join filter. We can improve matters by using row_number() instead of rank(), so that the initial join keys are unique. The catch is that that makes the results sensitive to the sorting of rows with equal distances from the reference point. That doesn't matter for the actually-equal points, but as luck would have it, the data set also contains two distinct points that have identical distances to the origin. So those two rows could legitimately appear in either order, causing unwanted output from the check queries. However, it doesn't seem like it's the job of this test to check whether the <-> operator correctly computes distances; its charter is just to verify that SP-GiST emits the values in distance order. So we can dodge the indeterminacy problem by having the check only compare row numbers and distances not the actual point values. This change reduces the run time of create_index_spgist by a good three-quarters, on my machine, with ensuing beneficial effects on the runtime of create_index (thanks to interactions with CREATE INDEX CONCURRENTLY tests in the latter). I see a net improvement of more than 2X in the runtime of their parallel test group. Discussion: https://postgr.es/m/735.1554935715@sss.pgh.pa.us
* Split up a couple of long-running regression test scripts.Tom Lane2019-04-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The point of this change is to increase the potential for parallelism while running the core regression tests. Most people these days are using parallel testing modes on multi-core machines, so we might as well try a bit harder to keep multiple cores busy. Hence, a test that runs much longer than others in its parallel group is a candidate to be sub-divided. In this patch, create_index.sql and join.sql are split up. I haven't changed the content of the tests in any way, just moved them. I moved create_index.sql's SP-GiST-related tests into a new script create_index_spgist, and moved its btree multilevel page deletion test over to the existing script btree_index. (btree_index is a more natural home for that test, and it's shorter than others in its parallel group, so this doesn't hurt total runtime of that group.) There might be room for more aggressive splitting of create_index, but this is enough to improve matters considerably. Likewise, I moved join.sql's "exercises for the hash join code" into a new file join_hash. Those exercises contributed three-quarters of the script's runtime. Which might well be excessive ... but for the moment, I'm satisfied with shoving them into a different parallel group, where they can share runtime with the roughly-equally-lengthy gist test. (Note for anybody following along at home: there are interesting interactions between the runtimes of create_index and anything running in parallel with it, because the tests of CREATE INDEX CONCURRENTLY in that file will repeatedly block waiting for concurrent transactions to commit. As committed in this patch, create_index and create_index_spgist have roughly equal runtimes, but that's mostly an artifact of forced synchronization of the CONCURRENTLY tests; when run serially, create_index is much faster. A followup patch will reduce the runtime of create_index_spgist and thereby also create_index.) Discussion: https://postgr.es/m/735.1554935715@sss.pgh.pa.us
* Move plpgsql error-trapping tests to a new module-specific test file.Tom Lane2019-04-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | The test for statement timeout has a 2-second timeout, which was only moderately annoying when it was written, but nowadays it contributes a pretty significant chunk of the elapsed time needed to run the core regression tests on a fast machine. We can improve this situation by pushing the test into a plpgsql-specific test file instead of having it in a core regression test. That's a clean win when considering just the core tests. Even when considering check-world or a buildfarm test run, we should come out ahead because the core tests get run more times in those sequences. Furthermore, since the plpgsql tests aren't currently parallelized, it seems likely that the timing problems reflected in commit f1e671a0b (which increased that timeout from 1 sec to 2) will be much less severe in this context. Hence, let's try cutting the timeout back to 1 second in hopes of a further win for check-world. We can undo that if buildfarm experience proves it to be a bad idea. To give the new test file some modicum of intellectual coherency, I moved the surrounding tests related to error-trapping along with the statement timeout test proper. Those other tests don't run long enough to have any particular bearing on test-runtime considerations. The tests are the same as before, except with minor adjustments to not depend on an externally-created table. Discussion: https://postgr.es/m/735.1554935715@sss.pgh.pa.us
* Fix off-by-one check that can lead to a memory overflow in ecpg.Michael Meskes2019-04-11
| | | | Patch by Liu Huailing <liuhuailing@cn.fujitsu.com>
* Remove duplicative polygon SP-GiST sequencing test.Tom Lane2019-04-11
| | | | | | | | | | | | | | Code coverage comparisons confirm that the tests using quad_poly_tbl_ord_seq1/quad_poly_tbl_ord_idx1 hit no code paths not also covered by the similar tests using quad_poly_tbl_ord_seq2/quad_poly_tbl_ord_idx2. Since these test cases are pretty expensive, they need to contribute more than zero benefit. In passing, make quad_poly_tbl_ord_seq2 a temp table, since there seems little reason to keep it around after the test. Discussion: https://postgr.es/m/735.1554935715@sss.pgh.pa.us
* Remove redundant and ineffective test for btree insertion fast path.Tom Lane2019-04-11
| | | | | | | | | | | | | | | | indexing.sql's test for this feature was added along with the feature in commit 2b2727343. However, shortly later that test was rendered ineffective by commit 074251db6, which limited when the optimization would be applied, so that the test didn't test it. Since then, commit dd299df81 added new tests (in btree_index.sql) that actually do test the feature. Code coverage comparisons confirm that this test sequence adds no meaningful coverage, and it's rather expensive, accounting for nearly half of the runtime of indexing.sql according to my measurements. So let's remove it. Per advice from Peter Geoghegan. Discussion: https://postgr.es/m/735.1554935715@sss.pgh.pa.us
* Fix declaration after statementAlvaro Herrera2019-04-10
| | | | | | | | This style is frowned upon. I inadvertently introduced one in commit fe0e0b4fc7f0. (My compiler does not complain about it, even though -Wdeclaration-after-statement is specified. Weird.) Author: Masahiko Sawada
* Fix backwards test in operator_precedence_warning logic.Tom Lane2019-04-10
| | | | | | | | | | Warnings about unary minus might have been wrong. It's a bit surprising that nobody noticed yet ... probably the precedence-warning feature hasn't really been used much in the field. Rikard Falkeborn Discussion: https://postgr.es/m/CADRDgG6fzA8A2oeygUw4=o7ywo4kvz26NxCSgpq22nMD73Bx4Q@mail.gmail.com
* pg_restore: Make not verbose by defaultPeter Eisentraut2019-04-10
| | | | | | | This was accidentally changed in cc8d41511721d25d557fc02a46c053c0a602fed0. Reported-by: Christoph Berg <myon@debian.org>
* Avoid counting transaction stats for parallel worker cooperatingAmit Kapila2019-04-10
| | | | | | | | | | | | | | | | | | | | | | transaction. The transaction that is initiated by the parallel worker to cooperate with the actual transaction started by the main backend to complete the query execution should not be counted as a separate transaction. The other internal transactions started and committed by the parallel worker are still counted as separate transactions as we that is what we do in other places like autovacuum. This will partially fix the bloat in transaction stats due to additional transactions performed by parallel workers. For a complete fix, we need to decide how we want to show all the transactions that are started internally for various operations and that is a matter of separate patch. Reported-by: Haribabu Kommi Author: Haribabu Kommi Reviewed-by: Amit Kapila, Jamison Kirk and Rahila Syed Backpatch-through: 9.6 Discussion: https://postgr.es/m/CAJrrPGc9=jKXuScvNyQ+VNhO0FZk7LLAShAJRyZjnedd2D61EQ@mail.gmail.com
* Improve comment in sync.h.Thomas Munro2019-04-10
| | | | Per off-list complaint from Andres Freund.
* Fix typos.Thomas Munro2019-04-10
|
* Prevent inlining of multiply-referenced CTEs with outer recursive refs.Tom Lane2019-04-09
| | | | | | | | | | | This has to be prevented because inlining would result in multiple self-references, which we don't support (and in fact that's disallowed by the SQL spec, see statements about linearly vs. nonlinearly recursive queries). Bug fix for commit 608b167f9. Per report from Yaroslav Schekin (via Andrew Gierth) Discussion: https://postgr.es/m/87wolmg60q.fsf@news-spur.riddles.org.uk
* Fix typoAlvaro Herrera2019-04-09
|
* Fix memory leak in pgbenchAlvaro Herrera2019-04-09
| | | | | | | | | | | | | | | | Commit 25ee70511ec2 introduced a memory leak in pgbench: some PGresult structs were not being freed during error bailout, because we're now doing more PQgetResult() calls than previously. Since there's more cleanup code outside the discard_response() routine than in it, refactor the cleanup code, removing the routine. This has little effect currently, since we abandon processing after hitting errors, but if we ever get further pgbench features (such as testing for serializable transactions), it'll matter. Per Coverity. Reviewed-by: Michaël Paquier
* Test some more cases with partitioned tables in EvalPlanQual.Tom Lane2019-04-09
| | | | | | | | | | | | We weren't testing anything involving EPQ on UPDATEs that move tuples into different partitions. Depending on the implementation, it might be that these cases aren't actually very interesting ... but given our thin coverage of EPQ in general, I think it's a good idea to have a test case. Amit Langote, minor tweak by me Discussion: https://postgr.es/m/7889df35-ad1a-691a-00e3-4d4b18f364e3@lab.ntt.co.jp
* Define WIN32_STACK_RLIMIT throughout win32 and cygwin builds.Noah Misch2019-04-09
| | | | | | | | The MSVC build system already did this, and commit 617dc6d299c957e2784320382b3277ede01d9c63 used it in a second file. Back-patch to 9.4, like that commit. Discussion: https://postgr.es/m/CAA8=A7_1SWc3+3Z=-utQrQFOtrj_DeohRVt7diA2tZozxsyUOQ@mail.gmail.com
* Replace tabs with spaces in one .sql filePeter Eisentraut2019-04-09
| | | | Let's at least keep this consistent within the same file.
* Fix example in comment.Heikki Linnakangas2019-04-09
| | | | Author: Adrien Nayrat
* Avoid "could not reattach" by providing space for concurrent allocation.Noah Misch2019-04-08
| | | | | | | | | | | | | | | | | We've long had reports of intermittent "could not reattach to shared memory" errors on Windows. Buildfarm member dory fails that way when PGSharedMemoryReAttach() execution overlaps with creation of a thread for the process's "default thread pool". Fix that by providing a second region to receive asynchronous allocations that would otherwise intrude into UsedShmemSegAddr. In pgwin32_ReserveSharedMemoryRegion(), stop trying to free reservations landing at incorrect addresses; the caller's next step has been to terminate the affected process. Back-patch to 9.4 (all supported versions). Reviewed by Tom Lane. He also did much of the prerequisite research; see commit bcbf2346d69f6006f126044864dd9383d50d87b4. Discussion: https://postgr.es/m/20190402135442.GA1173872@rfd.leadboat.com
* tableam: comment and formatting fixes.Andres Freund2019-04-08
| | | | | Author: Heikki Linnakangas Discussion: https://postgr.es/m/9a7fb9cc-2419-5db7-8840-ddc10c93f122@iki.fi
* Fix improper interaction of FULL JOINs with lateral references.Tom Lane2019-04-08
| | | | | | | | | | | | | | | | | | | | | join_is_legal() needs to reject forming certain outer joins in cases where that would lead the planner down a blind alley. However, it mistakenly supposed that the way to handle full joins was to treat them as applying the same constraints as for left joins, only to both sides. That doesn't work, as shown in bug #15741 from Anthony Skorski: given a lateral reference out of a join that's fully enclosed by a full join, the code would fail to believe that any join ordering is legal, resulting in errors like "failed to build any N-way joins". However, we don't really need to consider full joins at all for this purpose, because we effectively force them to be evaluated in syntactic order, and that order is always legal for lateral references. Hence, get rid of this broken logic for full joins and just ignore them instead. This seems to have been an oversight in commit 7e19db0c0. Back-patch to all supported branches, as that was. Discussion: https://postgr.es/m/15741-276f1f464b3f40eb@postgresql.org
* Fix EvalPlanQualStart to handle partitioned result rels correctly.Tom Lane2019-04-08
| | | | | | | | | | | | The es_root_result_relations array needs to be shallow-copied in the same way as the main es_result_relations array, else EPQ rechecks on partitioned result relations fail, as seen in bug #15677 from Norbert Benkocs. Amit Langote, isolation test case added by me Discussion: https://postgr.es/m/15677-0bf089579b4cd02d@postgresql.org Discussion: https://postgr.es/m/19321.1554567786@sss.pgh.pa.us
* Add vacuum_truncate reloption.Fujii Masao2019-04-08
| | | | | | | | | | | | | | | vacuum_truncate controls whether vacuum tries to truncate off any empty pages at the end of the table. Previously vacuum always tried to do the truncation. However, the truncation could cause some problems; for example, ACCESS EXCLUSIVE lock needs to be taken on the table during the truncation and can cause the query cancellation on the standby even if hot_standby_feedback is true. Setting this reloption to false can be helpful to avoid such problems. Author: Tsunakawa Takayuki Reviewed-By: Julien Rouhaud, Masahiko Sawada, Michael Paquier, Kirk Jamison and Fujii Masao Discussion: https://postgr.es/m/CAHGQGwE5UqFqSq1=kV3QtTUtXphTdyHA-8rAj4A=Y+e4kyp3BQ@mail.gmail.com
* Reset memory context once per tuple in validateForeignKeyConstraint.Andres Freund2019-04-07
| | | | | | | | | | | | | | | | | | | | | When using tableam ExecFetchSlotHeapTuple() might return a separately allocated tuple. We could use the shouldFree argument to explicitly free it, but it seems more robust to to protect Also add a CHECK_FOR_INTERRUPTS() after each tuple. It's likely that each AM has (heap does) a CFI somewhere in the relevant path, but it seems more robust to have one in validateForeignKeyConstraint() itself. Note that this only affects the cases that couldn't be optimized to be verified with a query. Author: Andres Freund Reviewed-By: Tom Lane (in an earlier version) Discussion: https://postgr.es/m/19030.1554574075@sss.pgh.pa.us https://postgr.es/m/CAKJS1f_SHKcPYMsi39An5aUjhAcEMZb6Cx1Sj1QWEWSiKJkBVQ@mail.gmail.com https://postgr.es/m/20180711185628.mrvl46bjgk2uxoki@alap3.anarazel.de
* Fix a number of issues around modifying a previously updated row.Andres Freund2019-04-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This commit fixes three, unfortunately related, issues: 1) Since 5db6df0c01, the introduction of DML via tableam, it was possible to trigger "ERROR: unexpected table_lock_tuple status: 1" when updating a row that was previously updated in the same transaction - but only when the previously updated row was before updated in a concurrent transaction (and READ COMMITTED was used). The reason for that was that that case simply wasn't expected. Fixing that lead to: 2) Even before the above commit, there were error checks (introduced in 6868ed7491b7) preventing a row being updated by different commands within the same statement (say in a function called by an UPDATE) - but that check wasn't performed when the row was first updated in a concurrent transaction - instead the second update was silently skipped in that case. After this change we throw the same error as we'd without the concurrent transaction. 3) The error messages (introduced in 6868ed7491b7) preventing such updates emitted the same error message for both DELETE and UPDATE ("tuple to be updated was already modified by an operation triggered by the current command"). While that could be changed separately, it made it hard to write tests that verify the correct correct behavior of the code. This commit changes heap's implementation of table_lock_tuple() to return TM_SelfModified instead of TM_Invisible (previously loosely modeled after EvalPlanQualFetch), and teaches nodeModifyTable.c to handle that in response to table_lock_tuple() and not just in response to table_(delete|update). Additionally it fixes the wrong error message (see 3 above). The comment for table_lock_tuple() is also adjusted to state that TM_Deleted won't return information in TM_FailureData - it'll not always be available. This also adds tests to ensure that DELETE/UPDATE correctly error out when affecting a row that concurrently was modified by another transaction. Author: Andres Freund Reported-By: Tom Lane, when investigating a bug bug fix to another bug by Amit Langote Discussion: https://postgr.es/m/19321.1554567786@sss.pgh.pa.us
* Add more tests for partition tuple routing with dropped attributesMichael Paquier2019-04-08
| | | | | | | | | | | As bug #15733 has proved, we are lacking coverage for partition tuple routing with dropped attributes when involving three levels of partitioning or more. There was only an active bug in this area for v11, and HEAD is proving to handle those scenarios fine, still it lacked some coverage for the previous problem. Author: Amit Langote, Michael Paquier Discussion: https://postgr.es/m/15733-7692379e310b80ec@postgresql.org
* Avoid fetching past the end of the indoption array.Tom Lane2019-04-07
| | | | | | | | | | | pg_get_indexdef_worker carelessly fetched indoption entries even for non-key index columns that don't have one. 99.999% of the time this would be harmless, since the code wouldn't examine the value ... but some fine day this will be a fetch off the end of memory, resulting in SIGSEGV. Detected through valgrind testing. Odd that the buildfarm's valgrind critters haven't noticed.
* psql \dP: list partitioned tables and indexesAlvaro Herrera2019-04-07
| | | | | | | | | | | | | | | | The new command lists partitioned relations (tables and/or indexes), possibly with their sizes, possibly including partitioned partitions; their parents (if not top-level); if indexes show the tables they belong to; and their descriptions. While there are various possible improvements to this, having it in this form is already a great improvement over not having any way to obtain this report. Author: Pavel Stěhule, with help from Mathias Brossard, Amit Langote and Justin Pryzby. Reviewed-by: Amit Langote, Mathias Brossard, Melanie Plageman, Michaël Paquier, Álvaro Herrera
* Clean up side-effects of commits ab5fcf2b0 et al.Tom Lane2019-04-07
| | | | | | | | | | | | | | | | | | | | | | | Before those commits, partitioning-related code in the executor could assume that ModifyTableState.resultRelInfo[] contains only leaf partitions. However, now a fully-pruned update results in a dummy ModifyTable that references the root partitioned table, and that breaks some stuff. In v11, this led to an assertion or core dump in the tuple routing code. Fix by disabling tuple routing, since we don't need that anyway. (I chose to do that in HEAD as well for safety, even though the problem doesn't manifest in HEAD as it stands.) In v10, this confused ExecInitModifyTable's decision about whether it needed to close the root table. But we can get rid of that altogether by being smarter about where to find the root table. Note that since the referenced commits haven't shipped yet, this isn't fixing any bug the field has seen. Amit Langote, per a report from me Discussion: https://postgr.es/m/20710.1554582479@sss.pgh.pa.us
* Report progress of REINDEX operationsPeter Eisentraut2019-04-07
| | | | | | | | | | | | This uses the same infrastructure that the CREATE INDEX progress reporting uses. Add a column to pg_stat_progress_create_index to report the OID of the index being worked on. This was not necessary for CREATE INDEX, but it's useful for REINDEX. Also edit the phase descriptions a bit to be more consistent with the source code comments. Discussion: https://www.postgresql.org/message-id/ef6a6757-c36a-9e81-123f-13b19e36b7d7%402ndquadrant.com
* Cast pg_stat_progress_cluster.cluster_index_relid to oidPeter Eisentraut2019-04-07
| | | | | It's tracked internally as bigint, but when presented to the user it should be oid.
* Fix failures in validateForeignKeyConstraint's slow path.Tom Lane2019-04-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | The foreign-key-checking loop in ATRewriteTables failed to ignore relations without storage (e.g., partitioned tables), unlike the initial loop. This accidentally worked as long as RI_Initial_Check succeeded, which it does in most practical cases (including all the ones exercised in the existing regression tests :-(). However, if that failed, as for instance when there are permissions issues, then we entered the slow fire-the-trigger-on-each-tuple path. And that would try to read from the referencing relation, and fail if it lacks storage. A second problem, recently introduced in HEAD, was that this loop had been broken by sloppy refactoring for the tableam API changes. Repair both issues, and add a regression test case so we have some coverage on this code path. Back-patch as needed to v11. (It looks like this code could do with additional bulletproofing, but let's get a working test case in place first.) Hadi Moshayedi, Tom Lane, Andres Freund Discussion: https://postgr.es/m/CAK=1=WrnNmBbe5D9sm3t0a6dnAq3cdbF1vXY816j1wsMqzC8bw@mail.gmail.com Discussion: https://postgr.es/m/19030.1554574075@sss.pgh.pa.us Discussion: https://postgr.es/m/20190325180405.jytoehuzkeozggxx%40alap3.anarazel.de
* Add support TCP user timeout in libpq and the backend serverMichael Paquier2019-04-06
| | | | | | | | | | | | | | | | | Similarly to the set of parameters for keepalive, a connection parameter for libpq is added as well as a backend GUC, called tcp_user_timeout. Increasing the TCP user timeout is useful to allow a connection to survive extended periods without end-to-end connection, and decreasing it allows application to fail faster. By default, the parameter is 0, which makes the connection use the system default, and follows a logic close to the keepalive parameters in its handling. When connecting through a Unix-socket domain, the parameters have no effect. Author: Ryohei Nagaura Reviewed-by: Fabien Coelho, Robert Haas, Kyotaro Horiguchi, Kirk Jamison, Mikalai Keida, Takayuki Tsunakawa, Andrei Yahorau Discussion: https://postgr.es/m/EDA4195584F5064680D8130B1CA91C45367328@G01JPEXMBYT04
* Use Append rather than MergeAppend for scanning ordered partitions.Tom Lane2019-04-05
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If we need ordered output from a scan of a partitioned table, but the ordering matches the partition ordering, then we don't need to use a MergeAppend to combine the pre-ordered per-partition scan results: a plain Append will produce the same results. This both saves useless comparison work inside the MergeAppend proper, and allows us to start returning tuples after istarting up just the first child node not all of them. However, all is not peaches and cream, because if some of the child nodes have high startup costs then there will be big discontinuities in the tuples-returned-versus-elapsed-time curve. The planner's cost model cannot handle that (yet, anyway). If we model the Append's startup cost as being just the first child's startup cost, we may drastically underestimate the cost of fetching slightly more tuples than are available from the first child. Since we've had bad experiences with over-optimistic choices of "fast start" plans for ORDER BY LIMIT queries, that seems scary. As a klugy workaround, set the startup cost estimate for an ordered Append to be the sum of its children's startup costs (as MergeAppend would). This doesn't really describe reality, but it's less likely to cause a bad plan choice than an underestimated startup cost would. In practice, the cases where we really care about this optimization will have child plans that are IndexScans with zero startup cost, so that the overly conservative estimate is still just zero. David Rowley, reviewed by Julien Rouhaud and Antonin Houska Discussion: https://postgr.es/m/CAKJS1f-hAqhPLRk_RaSFTgYxd=Tz5hA7kQ2h4-DhJufQk8TGuw@mail.gmail.com
* Add facility to copy replication slotsAlvaro Herrera2019-04-05
| | | | | | | | | | | | | | This allows the user to create duplicates of existing replication slots, either logical or physical, and even changing properties such as whether they are temporary or the output plugin used. There are multiple uses for this, such as initializing multiple replicas using the slot for one base backup; when doing investigation of logical replication issues; and to select a different output plugins. Author: Masahiko Sawada Reviewed-by: Michael Paquier, Andres Freund, Petr Jelinek Discussion: https://postgr.es/m/CAD21AoAm7XX8y_tOPP6j4Nzzch12FvA1wPqiO690RCk+uYVstg@mail.gmail.com
* Wake up interested backends when a checkpoint fails.Thomas Munro2019-04-06
| | | | | | | | | Commit c6c9474a switched to condition variables instead of sleep loops to notify backends of checkpoint start and stop, but forgot to broadcast in case of checkpoint failure. Author: Thomas Munro Discussion: https://postgr.es/m/CA%2BhUKGJKbCd%2B_K%2BSEBsbHxVT60SG0ivWHHAdvL0bLTUt2xpA2w%40mail.gmail.com
* Ensure consistent name matching behavior in processSQLNamePattern().Tom Lane2019-04-05
| | | | | | | | | | | | | | | | | | | | Prior to v12, if you used a collation-sensitive regex feature in a pattern handled by processSQLNamePattern() (for instance, \d '\\w+' in psql), the behavior you got matched the database's default collation. Since commit 586b98fdf you'd usually get C-collation behavior, because the catalog "name"-type columns are now marked as COLLATE "C". Add explicit COLLATE specifications to restore the prior behavior. (Note for whoever writes the v12 release notes: the need for this shows that while 586b98fdf preserved pre-v12 behavior of "name" columns for simple comparison operators, it changed the behavior of regex operators on those columns. Although this patch fixes it for pattern matches generated by our own tools, user-written queries will still be affected. So we'd better mention this issue as a compatibility item.) Daniel Vérité Discussion: https://postgr.es/m/701e51f0-0ec0-4e70-a365-1958d66dd8d2@manitou-mail.org
* Fix compiler warningPeter Eisentraut2019-04-05
| | | | | | | Rewrite get_attgenerated() to avoid compiler warning if the compiler does not recognize that elog(ERROR) does not return. Reported-by: David Rowley <david.rowley@2ndquadrant.com>
* Revert "Consistently test for in-use shared memory."Noah Misch2019-04-05
| | | | | | | | | This reverts commits 2f932f71d9f2963bbd201129d7b971c8f5f077fd, 16ee6eaf80a40007a138b60bb5661660058d0422 and 6f0e190056fe441f7cf788ff19b62b13c94f68f3. The buildfarm has revealed several bugs. Back-patch like the original commits. Discussion: https://postgr.es/m/20190404145319.GA1720877@rfd.leadboat.com
* Fix bugs in mdsyncfiletag().Thomas Munro2019-04-05
| | | | | | | | | | | | | | | | Commit 3eb77eba moved a _mdfd_getseg() call from mdsync() into a new callback function mdsyncfiletag(), but didn't get the arguments quite right. Without the EXTENSION_DONT_CHECK_SIZE flag we fail to open a segment if lower-numbered segments have been truncated, and it wants a block number rather than a segment number. While comparing with the older coding, also remove an unnecessary clobbering of errno, and adjust the code in mdunlinkfiletag() to ressemble the original code from mdpostckpt() more closely instead of using an unnecessary call to smgropen(). Author: Thomas Munro Discussion: https://postgr.es/m/CA%2BhUKGL%2BYLUOA0eYiBXBfwW%2BbH5kFgh94%3DgQH0jHEJ-t5Y91wQ%40mail.gmail.com
* Handle errors during GSSAPI startup betterStephen Frost2019-04-04
| | | | | | | | | | | | | | | | | | | | | There was some confusion over the format of the error message returned from the server during GSSAPI startup; specifically, it was expected that a length would be returned when, in reality, at this early stage in the startup sequence, no length is returned from the server as part of an error message. Correct the client-side code for dealing with error messages sent by the server during startup by simply reading what's available into our buffer, after we've discovered it's an error message, and then reporting back what was returned. In passing, also add in documentation of the environment variable PGGSSENCMODE which was missed previously, and adjust the code to look for the PGGSSENCMODE variable (the environment variable change was missed in the prior GSSMODE -> GSSENCMODE commit). Error-handling issue discovered by Peter Eisentraut, the rest were items discovered during testing of the error handling.
* Remove unused struct member, enforce multi_insert callback presence.Andres Freund2019-04-04
| | | | | Author: David Rowley, Andres Freund Discussion: https://postgr.es/m/CAKJS1f9=9phmm66diAji4gvHnWSrK7BGFoNct+mEUT_c8pPOjw@mail.gmail.com
* Harden tableam against nonexistant / wrong kind of AMs.Andres Freund2019-04-04
| | | | | | | | | | | | | | | | | Previously it was allowed to set default_table_access_method to an empty string. That makes sense for default_tablespace, where that was copied from, as it signals falling back to the database's default tablespace. As there is no equivalent for table AMs, forbid that. Also make sure to throw a usable error when creating a table using an index AM, by using get_am_type_oid() to implement get_table_am_oid() instead of a separate copy. Previously we'd error out only later, in GetTableAmRoutine(). Thirdly remove GetTableAmRoutineByAmId() - it was only used in an earlier version of 8586bf7ed8. Add tests for the above (some for index AMs as well).
* Add test coverage for rootdescend verification.Peter Geoghegan2019-04-04
| | | | | | | | | | | Commit c1afd175, which added support for rootdescend verification to amcheck, added only minimal regression test coverage. Address this by making sure that rootdescend verification is run on a multi-level index. In passing, simplify some of the regression tests that exercise multi-level nbtree page deletion. Both issues spotted while rereviewing coverage of the nbtree patch series using gcov.