aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
* pgbench: Report errors during run betterPeter Eisentraut2018-10-15
| | | | | | | | | | | | When an error occurs during a benchmark run, exit with a nonzero exit code and write a message at the end. Previously, it would just print the error message when it happened but then proceed to print the run summary normally and exit with status 0. To still allow distinguishing setup from run-time errors, we use exit status 2 for the new state, whereas existing errors during pgbench initialization use exit status 1. Reviewed-by: Fabien COELHO <coelho@cri.ensmp.fr>
* Make spelling of "acknowledgment" consistentPeter Eisentraut2018-10-15
| | | | I used the preferred U.S. spelling, as we do in other cases.
* Make some subquery-using test cases a bit more robust.Tom Lane2018-10-14
| | | | | | | | These test cases could be adversely affected by an upcoming change to allow pullup of FROM-less subqueries. Tweak them to ensure that they'll continue to test what they did before. Discussion: https://postgr.es/m/5395.1539275668@sss.pgh.pa.us
* Use PlaceHolderVars within the quals of a FULL JOIN.Tom Lane2018-10-14
| | | | | | | | | | | | | | | | | | | | | | This prevents failures in cases where we pull up a constant or var-free expression from a subquery and put it into a full join's qual. That can result in not recognizing the qual as containing a mergejoin-able or hashjoin-able condition. A PHV prevents the problem because it is still recognized as belonging to the side of the join the subquery is in. I'm not very sure about the net effect of this change on plan quality. In "typical" cases where the join keys are Vars, nothing changes. In an affected case, the PHV-wrapped expression is less likely to be seen as equal to PHV-less instances below the join, but more likely to be seen as equal to similar expressions above the join, so it may end up being a wash. In the one existing case where there's any visible change in a regression-test plan, it amounts to referencing a lower computation of a COALESCE result instead of recomputing it, which seems like a win. Given my uncertainty about that and the lack of field complaints, no back-patch, even though this is a very ancient problem. Discussion: https://postgr.es/m/32090.1539378124@sss.pgh.pa.us
* Clean up/tighten up coercibility checks in opr_sanity regression test.Tom Lane2018-10-14
| | | | | | | | | | | | | | | | | | | | With the removal of the old abstime type, there are no longer any cases in this test where we need to use the weaker castcontext-ignoring form of binary coercibility check. (The other major source of such headaches, apparently-incompatible hash functions, is now hashvalidate()'s problem not this test script's problem.) Hence, just use binary_coercible() everywhere, and remove the comments explaining why we don't do so --- which were broken anyway by cda6a8d01. I left physically_coercible() in place but renamed it to better match what it's actually testing, and added some comments. Also, in test queries that have an assumption about the maximum number of function arguments they need to handle, add a clause to make them fail if someday there's a relevant function with more arguments. Otherwise we're likely not to notice that we need to extend the queries. Discussion: https://postgr.es/m/27637.1539388060@sss.pgh.pa.us
* Avoid duplicate XIDs at recovery when building initial snapshotMichael Paquier2018-10-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | On a primary, sets of XLOG_RUNNING_XACTS records are generated on a periodic basis to allow recovery to build the initial state of transactions for a hot standby. The set of transaction IDs is created by scanning all the entries in ProcArray. However it happens that its logic never counted on the fact that two-phase transactions finishing to prepare can put ProcArray in a state where there are two entries with the same transaction ID, one for the initial transaction which gets cleared when prepare finishes, and a second, dummy, entry to track that the transaction is still running after prepare finishes. This way ensures a continuous presence of the transaction so as callers of for example TransactionIdIsInProgress() are always able to see it as alive. So, if a XLOG_RUNNING_XACTS takes a standby snapshot while a two-phase transaction finishes to prepare, the record can finish with duplicated XIDs, which is a state expected by design. If this record gets applied on a standby to initial its recovery state, then it would simply fail, so the odds of facing this failure are very low in practice. It would be tempting to change the generation of XLOG_RUNNING_XACTS so as duplicates are removed on the source, but this requires to hold on ProcArrayLock for longer and this would impact all workloads, particularly those using heavily two-phase transactions. XLOG_RUNNING_XACTS is also actually used only to initialize the standby state at recovery, so instead the solution is taken to discard duplicates when applying the initial snapshot. Diagnosed-by: Konstantin Knizhnik Author: Michael Paquier Discussion: https://postgr.es/m/0c96b653-4696-d4b4-6b5d-78143175d113@postgrespro.ru Backpatch-through: 9.3
* Another round of portability hacking on ECPG regression tests.Tom Lane2018-10-12
| | | | | | | | | | | | | | Removing the separate Windows expected-files in commit f1885386f turns out to have been too optimistic: on most (but not all!) of our Windows buildfarm members, the tests still print floats with three exponent digits, because they're invoking the native printf() not snprintf.c. But rather than put back the extra expected-files, let's hack the three tests in question so that they adjust float formatting the same way snprintf.c does. Discussion: https://postgr.es/m/18890.1539374107@sss.pgh.pa.us
* Simplify use of AllocSetContextCreate() wrapper macro.Tom Lane2018-10-12
| | | | | | | | | | | | | | | We can allow this macro to accept either abbreviated or non-abbreviated allocation parameters by making use of __VA_ARGS__. As noted by Andres Freund, it's unlikely that any compiler would have __builtin_constant_p but not __VA_ARGS__, so this gives up little or no error checking, and it avoids a minor but annoying API break for extensions. With this change, there is no reason for anybody to call AllocSetContextCreateExtended directly, so in HEAD I renamed it to AllocSetContextCreateInternal. It's probably too late for an ABI break like that in 11, though. Discussion: https://postgr.es/m/20181012170355.bhxi273skjt6sag4@alap3.anarazel.de
* Remove dead reference to ecpg resultmap file.Tom Lane2018-10-12
| | | | | | | I missed this in my prior commit because it doesn't matter in non-VPATH builds. Per buildfarm.
* Correct attach/detach logic for FKs in partitionsAlvaro Herrera2018-10-12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | There was no code to handle foreign key constraints on partitioned tables in the case of ALTER TABLE DETACH; and if you happened to ATTACH a partition that already had an equivalent constraint, that one was ignored and a new constraint was created. Adding this to the fact that foreign key cloning reuses the constraint name on the partition instead of generating a new name (as it probably should, to cater to SQL standard rules about constraint naming within schemas), the result was a pretty poor user experience -- the most visible failure was that just detaching a partition and re-attaching it failed with an error such as ERROR: duplicate key value violates unique constraint "pg_constraint_conrelid_contypid_conname_index" DETAIL: Key (conrelid, contypid, conname)=(26702, 0, test_result_asset_id_fkey) already exists. because it would try to create an identically-named constraint in the partition. To make matters worse, if you tried to drop the constraint in the now-independent partition, that would fail because the constraint was still seen as dependent on the constraint in its former parent partitioned table: ERROR: cannot drop inherited constraint "test_result_asset_id_fkey" of relation "test_result_cbsystem_0001_0050_monthly_2018_09" This fix attacks the problem from two angles: first, when the partition is detached, the constraint is also marked as independent, so the drop now works. Second, when the partition is re-attached, we scan existing constraints searching for one matching the FK in the parent, and if one exists, we link that one to the parent constraint. So we don't end up with a duplicate -- and better yet, we don't need to scan the referenced table to verify that the constraint holds. To implement this I made a small change to previously planner-only struct ForeignKeyCacheInfo to contain the constraint OID; also relcache now maintains the list of FKs for partitioned tables too. Backpatch to 11. Reported-by: Michael Vitale (bug #15425) Discussion: https://postgr.es/m/15425-2dbc9d2aa999f816@postgresql.org
* Make float exponent output on Windows look the same as elsewhere.Tom Lane2018-10-12
| | | | | | | | | Windows, alone among our supported platforms, likes to emit three-digit exponent fields even when two digits would do. Adjust such results to look like the way everyone else does it. Eliminate a bunch of variant expected-output files that were needed only because of this quirk. Discussion: https://postgr.es/m/2934.1539122454@sss.pgh.pa.us
* Add TAP tests for pg_verify_checksumsMichael Paquier2018-10-12
| | | | | | | | | | | | | | | | All options available in the utility get coverage: - Tests with disabled page checksums. - Tests with enabled test checksums. - Emulation of corruption and broken checksums with a full scan and single relfilenode scan. This patch has been contributed mainly by Michael Banck and Magnus Hagander with things presented on various threads, and I have gathered all the contents into a single patch. Author: Michael Banck, Magnus Hagander, Michael Paquier Reviewed-by: Peter Eisentraut Discussion: https://postgr.es/m/20181005012645.GE1629@paquier.xyz
* Remove deprecated abstime, reltime, tinterval datatypes.Andres Freund2018-10-11
| | | | | | | | | | | | These types have been deprecated for a *long* time. Catversion bump, for obvious reasons. Author: Andres Freund Discussion: https://postgr.es/m/20181009192237.34wjp3nmw7oynmmr@alap3.anarazel.de https://postgr.es/m/20171213080506.cwjkpcz3bkk6yz2u@alap3.anarazel.de https://postgr.es/m/25615.1513115237@sss.pgh.pa.us
* Remove timetravel extension.Andres Freund2018-10-11
| | | | | | | | | | | | | The extension depended on old types which are about to be removed. As the code additionally was pretty crufty and didn't provide much in the way of functionality, removing the extension seems to be the best way forward. It's fairly trivial to write functionality in plpgsql that more than covers what timetravel did. Author: Andres Freund Discussion: https://postgr.es/m/20171213080506.cwjkpcz3bkk6yz2u@alap3.anarazel.de https://postgr.es/m/25615.1513115237@sss.pgh.pa.us
* Move timeofday() implementation out of nabstime.c.Andres Freund2018-10-11
| | | | | | | | | | nabstime.c is about to be removed, but timeofday() isn't related to the rest of the functionality therein, and some find it useful. Move to timestamp.c. Discussion: https://postgr.es/m/20181009192237.34wjp3nmw7oynmmr@alap3.anarazel.de https://postgr.es/m/20180928223240.kgwc4czzzekrpsid%40alap3.anarazel.de
* Fix logical decoding error when system table w/ toast is repeatedly rewritten.Andres Freund2018-10-10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Repeatedly rewriting a mapped catalog table with VACUUM FULL or CLUSTER could cause logical decoding to fail with: ERROR, "could not map filenode \"%s\" to relation OID" To trigger the problem the rewritten catalog had to have live tuples with toasted columns. The problem was triggered as during catalog table rewrites the heap_insert() check that prevents logical decoding information to be emitted for system catalogs, failed to treat the new heap's toast table as a system catalog (because the new heap is not recognized as a catalog table via RelationIsLogicallyLogged()). The relmapper, in contrast to the normal catalog contents, does not contain historical information. After a single rewrite of a mapped table the new relation is known to the relmapper, but if the table is rewritten twice before logical decoding occurs, the relfilenode cannot be mapped to a relation anymore. Which then leads us to error out. This only happens for toast tables, because the main table contents aren't re-inserted with heap_insert(). The fix is simple, add a new heap_insert() flag that prevents logical decoding information from being emitted, and accept during decoding that there might not be tuple data for toast tables. Unfortunately that does not fix pre-existing logical decoding errors. Doing so would require not throwing an error when a filenode cannot be mapped to a relation during decoding, and that seems too likely to hide bugs. If it's crucial to fix decoding for an existing slot, temporarily changing the ERROR in ReorderBufferCommit() to a WARNING appears to be the best fix. Author: Andres Freund Discussion: https://postgr.es/m/20180914021046.oi7dm4ra3ot2g2kt@alap3.anarazel.de Backpatch: 9.4-, where logical decoding was introduced
* Slightly correct context check for event triggersPeter Eisentraut2018-10-10
| | | | | | | | | The previous check for a "complete query" omitted the new PROCESS_UTILITY_QUERY_NONATOMIC value. This didn't actually make a difference in practice, because only CALL and SET from PL/pgSQL run in this state, but it's more correct to include it anyway. Discussion: https://www.postgresql.org/message-id/4566041d-2567-74d2-d135-19ff6a20fe51%402ndquadrant.com
* Test that event triggers work in functions and proceduresPeter Eisentraut2018-10-10
| | | | | This ensures that we have coverage of all the ProcessUtilityContext variants.
* Turn transaction_isolation into GUC enumPeter Eisentraut2018-10-09
| | | | | | | | | | | | | | | It was previously a string setting that was converted into an enum by custom code, but using the GUC enum facility seems much simpler and doesn't change any functionality, except that set transaction_isolation='default'; no longer works, but that was never documented and doesn't work with any other transaction characteristics. (Note that this is not the same as RESET or SET TO DEFAULT, which still work.) Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Discussion: https://www.postgresql.org/message-id/457db615-e84c-4838-310e-43841eb806e5@iki.fi
* Make src/common/exec.c's error logging less ugly.Tom Lane2018-10-09
| | | | | | | | | | | | | | | | | This code used elog where it really ought to use ereport, mainly so that it can report a SQLSTATE different from ERRCODE_INTERNAL_ERROR. There were some other random deviations from typical error report practice too. In addition, we can make some cleanups that were impractical six months ago: * Use one variadic macro, instead of several with different numbers of arguments, reducing the temptation to force-fit messages into particular numbers of arguments; * Use %m, even in the frontend case, simplifying the code. Discussion: https://postgr.es/m/6025.1527351693@sss.pgh.pa.us
* Remove no-longer-needed variant expected regression result files.Tom Lane2018-10-09
| | | | | | | | | | | | | numerology_1.out and float8-small-is-zero_1.out differ from their base files only in showing plain zero rather than minus zero for some results. I believe that in the wake of commit 6eb3eb577, we will print minus zero as such on all IEEE-float platforms (and non-IEEE floats are going to cause many more regression diffs than this, anyway). Hence we should be able to remove these and eliminate a bit of maintenance pain. Let's see if the buildfarm agrees. Discussion: https://postgr.es/m/29037.1539021687@sss.pgh.pa.us
* Select appropriate PG_PRINTF_ATTRIBUTE for recent NetBSD.Tom Lane2018-10-09
| | | | | | | | | | | | | | NetBSD-current generates a large number of warnings about "%m" not being appropriate to use with *printf functions. While that's true for their native printf, it's surely not true for snprintf.c, so I think they have misunderstood gcc's definition of the "gnu_printf" archetype. Nonetheless, choosing "__syslog__" instead silences the warnings; so teach configure about that. Since this is only a cosmetic warning issue (and anyway it depends on previous hacking to be self-consistent), no back-patch. Discussion: https://postgr.es/m/16785.1539046036@sss.pgh.pa.us
* Add pg_ls_archive_statusdir functionMichael Paquier2018-10-09
| | | | | | | | | | | | This function lists the contents of the WAL archive status directory, and is intended to be used by monitoring tools. Unlike pg_ls_dir(), access to it can be granted to non-superusers so that those monitoring tools can observe the principle of least privilege. Access is also given by default to members of pg_monitor. Author: Christoph Moench-Tegeder Reviewed-by: Aya Iwata Discussion: https://postgr.es/m/20180930205920.GA64534@elch.exwg.net
* Relax transactional restrictions on ALTER TYPE ... ADD VALUE (redux).Thomas Munro2018-10-09
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Originally committed as 15bc038f (plus some follow-ups), this was reverted in 28e07270 due to a problem discovered in parallel workers. This new version corrects that problem by sending the list of uncommitted enum values to parallel workers. Here follows the original commit message describing the change: To prevent possibly breaking indexes on enum columns, we must keep uncommitted enum values from getting stored in tables, unless we can be sure that any such column is new in the current transaction. Formerly, we enforced this by disallowing ALTER TYPE ... ADD VALUE from being executed at all in a transaction block, unless the target enum type had been created in the current transaction. This patch removes that restriction, and instead insists that an uncommitted enum value can't be referenced unless it belongs to an enum type created in the same transaction as the value. Per discussion, this should be a bit less onerous. It does require each function that could possibly return a new enum value to SQL operations to check this restriction, but there aren't so many of those that this seems unmaintainable. Author: Andrew Dunstan and Tom Lane, with parallel query fix by Thomas Munro Reviewed-by: Tom Lane Discussion: https://postgr.es/m/CAEepm%3D0Ei7g6PaNTbcmAh9tCRahQrk%3Dr5ZWLD-jr7hXweYX3yg%40mail.gmail.com Discussion: https://postgr.es/m/4075.1459088427%40sss.pgh.pa.us
* Fix omissions in snprintf.c's coverage of standard *printf functions.Tom Lane2018-10-08
| | | | | | | | | | | | | | | | | | A warning on a NetBSD box revealed to me that pg_waldump/compat.c is using vprintf(), which snprintf.c did not provide coverage for. This is not good if we want to have uniform *printf behavior, and it's pretty silly to omit when it's a one-line function. I also noted that snprintf.c has pg_vsprintf() but for some reason it was not exposed to the outside world, creating another way in which code might accidentally invoke the platform *printf family. Let's just make sure that we replace all eight of the POSIX-standard printf family. Also, upgrade plperl.h and plpython.h to make sure that they do their undefine/redefine rain dance for all eight, not some random maybe-sufficient subset thereof.
* Advance transaction timestamp for intra-procedure transactions.Tom Lane2018-10-08
| | | | | | | | Per discussion, this behavior seems less astonishing than not doing so. Peter Eisentraut and Tom Lane Discussion: https://postgr.es/m/20180920234040.GC29981@momjian.us
* Improve snprintf.c's handling of NaN, Infinity, and minus zero.Tom Lane2018-10-08
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Up to now, float4out/float8out handled NaN and Infinity cases explicitly, and invoked psprintf only for ordinary float values. This was done because platform implementations of snprintf produce varying representations of these special cases. But now that we use snprintf.c always, it's better to give it the responsibility to produce a uniform representation of these cases, so that we have uniformity across the board not only in float4out/float8out. Hence, move that work into fmtfloat(). Also, teach fmtfloat() to recognize IEEE minus zero and handle it correctly. The previous coding worked only accidentally, and would fail for e.g. "%+f" format (it'd print "+-0.00000"). Now that we're using snprintf.c everywhere, it's not acceptable for it to do weird things in corner cases. (This incidentally avoids a portability problem we've seen on some really ancient platforms, that native sprintf does the wrong thing with minus zero.) Also, introduce a new entry point in snprintf.c to allow float[48]out to bypass the work of interpreting a well-known format spec, as well as bypassing the overhead of the psprintf layer. I modeled this API loosely on strfromd(). In my testing, this brings float[48]out back to approximately the same speed they had when using native snprintf, fixing one of the main performance issues caused by using snprintf.c. (There is some talk of more aggressive work to improve the speed of floating-point output conversion, but these changes seem to provide a better starting point for such work anyway.) Getting rid of the previous ad-hoc hack for Infinity/NaN in fmtfloat() allows removing <ctype.h> from snprintf.c's #includes. I also removed a few other #includes that I think are historical, though the buildfarm may expose that as wrong. Discussion: https://postgr.es/m/13178.1538794717@sss.pgh.pa.us
* Avoid O(N^2) cost in ExecFindRowMark().Tom Lane2018-10-08
| | | | | | | | | | | | | | | | | | | | If there are many ExecRowMark structs, we spent O(N^2) time in ExecFindRowMark during executor startup. Once upon a time this was not of great concern, but the addition of native partitioning has squeezed out enough other costs that this can become the dominant overhead in some use-cases for tables with many partitions. To fix, simply replace that List data structure with an array. This adds a little bit of cost to execCurrentOf(), but not much, and anyway that code path is neither of large importance nor very efficient now. If we ever decide it is a bottleneck, constructing a hash table for lookup-by-tableoid would likely be the thing to do. Per complaint from Amit Langote, though this is different from his fix proposal. Discussion: https://postgr.es/m/468c85d9-540e-66a2-1dde-fec2b741e688@lab.ntt.co.jp
* Silence compiler warning in Assert()Alvaro Herrera2018-10-08
| | | | | | | | gcc 6.3 does not whine about this mistake I made in 39808e8868c8 but evidently lots of other compilers do, according to Michael Paquier, Peter Eisentraut, Arthur Zakirov, Tomas Vondra. Discussion: too many to list
* Track procedure calls in pg_stat_user_functionsPeter Eisentraut2018-10-08
| | | | | | This was forgotten when procedures were implemented. Reported-by: Lukas Fittl <lukas@fittl.com>
* Improve two error messages related to foreign keys on partitioned tablesMichael Paquier2018-10-08
| | | | | | | | | | | Error messages for creating a foreign key on a partitioned table using ONLY or NOT VALID were wrong in mentioning the objects they worked on. This commit adds on the way some regression tests missing for those cases. Author: Laurenz Albe Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/c11c05810a9ed65e9b2c817a9ef442275a32fe80.camel@cybertec.at
* Fix speling errorMagnus Hagander2018-10-08
| | | | Reported by Alexander Lakhin in bug #15423
* Remove some unnecessary fields from Plan trees.Tom Lane2018-10-07
| | | | | | | | | | | | | | | | | | | | In the wake of commit f2343653f, we no longer need some fields that were used before to control executor lock acquisitions: * PlannedStmt.nonleafResultRelations can go away entirely. * partitioned_rels can go away from Append, MergeAppend, and ModifyTable. However, ModifyTable still needs to know the RT index of the partition root table if any, which was formerly kept in the first entry of that list. Add a new field "rootRelation" to remember that. rootRelation is partly redundant with nominalRelation, in that if it's set it will have the same value as nominalRelation. However, the latter field has a different purpose so it seems best to keep them distinct. Amit Langote, reviewed by David Rowley and Jesper Pedersen, and whacked around a bit more by me Discussion: https://postgr.es/m/468c85d9-540e-66a2-1dde-fec2b741e688@lab.ntt.co.jp
* Fix catalog insertion order for ATTACH PARTITIONAlvaro Herrera2018-10-06
| | | | | | | | | | | | | | | Commit 2fbdf1b38bc changed the order in which we inserted catalog rows when creating partitions, so that we could remove an unsightly hack required for untimely relcache invalidations. However, that commit only changed the ordering for CREATE TABLE PARTITION OF, and left ALTER TABLE ATTACH PARTITION unchanged, so the latter can be affected when catalog invalidations occur, for instance when the partition key involves an SQL function. Reported-by: Rajkumar Raghuwanshi Author: Amit Langote Reviewed-by: Michaël Paquier Discussion: https://postgr.es/m/CAKcux6=nTz9KSfTr_6Z2mpzLJ_09JN-rK6=dWic6gGyTSWueyQ@mail.gmail.com
* Fix event triggers for partitioned tablesAlvaro Herrera2018-10-06
| | | | | | | | | | | | | | | | | | Index DDL cascading on partitioned tables introduced a way for ALTER TABLE to be called reentrantly. This caused an an important deficiency in event trigger support to be exposed: on exiting the reentrant call, the alter table state object was clobbered, causing a crash when the outer alter table tries to finalize its processing. Fix the crash by creating a stack of event trigger state objects. There are still ways to cause things to misbehave (and probably other crashers) with more elaborate tricks, but at least it now doesn't crash in the obvious scenario. Backpatch to 9.5, where DDL deparsing of event triggers was introduced. Reported-by: Marco Slot Authors: Michaël Paquier, Álvaro Herrera Discussion: https://postgr.es/m/CANNhMLCpi+HQ7M36uPfGbJZEQLyTy7XvX=5EFkpR-b1bo0uJew@mail.gmail.com
* Restore sane locking behavior during parallel query.Tom Lane2018-10-06
| | | | | | | | | | | | | | Commit 9a3cebeaa changed things so that parallel workers didn't obtain any lock of their own on tables they access. That was clearly a bad idea, but I'd mistakenly supposed that it was the intended end result of the series of patches for simplifying the executor's lock management. Undo that change in relation_open(), and adjust ExecOpenScanRelation() so that it gets the correct lock if inside a parallel worker. In passing, clean up some more obsolete comments about when locks are acquired. Discussion: https://postgr.es/m/468c85d9-540e-66a2-1dde-fec2b741e688@lab.ntt.co.jp
* Remove more redundant relation locking during executor startup.Tom Lane2018-10-06
| | | | | | | | | | | | | | | | | We already have appropriate locks on every relation listed in the query's rangetable before we reach the executor. Take the next step in exploiting that knowledge by removing code that worries about taking locks on non-leaf result relations in a partitioned table. In particular, get rid of ExecLockNonLeafAppendTables and a stanza in InitPlan that asserts we already have locks on certain such tables. In passing, clean up some now-obsolete comments in InitPlan. Amit Langote, reviewed by David Rowley and Jesper Pedersen, and whacked around a bit more by me Discussion: https://postgr.es/m/468c85d9-540e-66a2-1dde-fec2b741e688@lab.ntt.co.jp
* Don't use is_infinite() where isinf() will do.Tom Lane2018-10-06
| | | | | | | | | | Places that aren't testing for sign should not use the more expensive function; it's just wasteful, not to mention being a cognitive load for readers who may know what isinf() is but not is_infinite(). As things stand, we actually don't need is_infinite() anyplace except float4out/float8out, which means it could potentially go away altogether after the changes I proposed in <13178.1538794717@sss.pgh.pa.us>.
* Propagate xactStartTimestamp and stmtStartTimestamp to parallel workers.Tom Lane2018-10-06
| | | | | | | | | | | | | | | | | | | | | | Previously, a worker process would establish values for these based on its own start time. In v10 and up, this can trivially be shown to cause misbehavior of transaction_timestamp(), timestamp_in(), and related functions which are (perhaps unwisely?) marked parallel-safe. It seems likely that other behaviors might diverge from what happens in the parent as well. It's not as trivial to demonstrate problems in 9.6 or 9.5, but I'm sure it's still possible, so back-patch to all branches containing parallel worker infrastructure. In HEAD only, mark now() and statement_timestamp() as parallel-safe (other affected functions already were). While in theory we could still squeeze that change into v11, it doesn't seem important enough to force a last-minute catversion bump. Konstantin Knizhnik, whacked around a bit by me Discussion: https://postgr.es/m/6406dbd2-5d37-4cb6-6eb2-9c44172c7e7c@postgrespro.ru
* Improve the accuracy of floating point statistical aggregates.Dean Rasheed2018-10-06
| | | | | | | | | | | | | | | | | | | | | | | When computing statistical aggregates like variance, the common schoolbook algorithm which computes the sum of the squares of the values and subtracts the square of the mean can lead to a large loss of precision when using floating point arithmetic, because the difference between the two terms is often very small relative to the terms themselves. To avoid this, re-work these aggregates to use the Youngs-Cramer algorithm, which is a proven, numerically stable algorithm that directly aggregates the sum of the squares of the differences of the values from the mean in a single pass over the data. While at it, improve the test coverage to test the aggregate combine functions used during parallel aggregation. Per report and suggested algorithm from Erich Schubert. Patch by me, reviewed by Madeleine Thompson. Discussion: https://postgr.es/m/153313051300.1397.9594490737341194671@wrigleys.postgresql.org
* Assign constraint name when cloning FK definition for partitionsMichael Paquier2018-10-06
| | | | | | | | | | | | | | | | | | This is for example used when attaching a partition to a partitioned table which includes foreign keys, and in this case the constraint name has been missing in the data cloned. This could lead to hard crashes, as when validating the foreign key constraint, the constraint name is always expected. Particularly, when using log_min_messages >= DEBUG1, a log message would be generated with this unassigned constraint name, leading to an assertion failure on HEAD. While on it, rename a variable in ATExecAttachPartition which was declared twice with the same name. Author: Michael Paquier Reviewed-by: Álvaro Herrera Discussion: https://postgr.es/m/20181005042236.GG1629@paquier.xyz Backpatch-through: 11
* Allow btree comparison functions to return INT_MIN.Tom Lane2018-10-05
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Historically we forbade datatype-specific comparison functions from returning INT_MIN, so that it would be safe to invert the sort order just by negating the comparison result. However, this was never really safe for comparison functions that directly return the result of memcmp(), strcmp(), etc, as POSIX doesn't place any such restriction on those library functions. Buildfarm results show that at least on recent Linux on s390x, memcmp() actually does return INT_MIN sometimes, causing sort failures. The agreed-on answer is to remove this restriction and fix relevant call sites to not make such an assumption; code such as "res = -res" should be replaced by "INVERT_COMPARE_RESULT(res)". The same is needed in a few places that just directly negated the result of memcmp or strcmp. To help find places having this problem, I've also added a compile option to nbtcompare.c that causes some of the commonly used comparators to return INT_MIN/INT_MAX instead of their usual -1/+1. It'd likely be a good idea to have at least one buildfarm member running with "-DSTRESS_SORT_INT_MIN". That's far from a complete test of course, but it should help to prevent fresh introductions of such bugs. This is a longstanding portability hazard, so back-patch to all supported branches. Discussion: https://postgr.es/m/20180928185215.ffoq2xrq5d3pafna@alap3.anarazel.de
* Ensure that PLPGSQL_DTYPE_ROW variables have valid refname fields.Tom Lane2018-10-05
| | | | | | | | | | Without this, the syntax-tree-dumping functions in pl_funcs.c crash, and there are other places that might be at risk too. Per report from Pavel Stehule. Looks like I broke this in commit f9263006d, so back-patch to v11. Discussion: https://postgr.es/m/CAFj8pRA+3f5n4642q2g8BXCKjbTd7yU9JMYAgDyHgozk6cQ-VA@mail.gmail.com
* Add pg_ls_tmpdir functionMichael Paquier2018-10-05
| | | | | | | | | | | | | | | | This lists the contents of a temporary directory associated to a given tablespace, useful to get information about on-disk consumption caused by temporary files used by a session query. By default, pg_default is scanned, and a tablespace can be specified as argument. This function is intended to be used by monitoring tools, and, unlike pg_ls_dir(), access to them can be granted to non-superusers so that those monitoring tools can observe the principle of least privilege. Access is also given by default to members of pg_monitor. Author: Nathan Bossart Reviewed-by: Laurenz Albe Discussion: https://postgr.es/m/92F458A2-6459-44B8-A7F2-2ADD3225046A@amazon.com
* In the executor, use an array of pointers to access the rangetable.Tom Lane2018-10-04
| | | | | | | | | | | | | | Instead of doing a lot of list_nth() accesses to es_range_table, create a flattened pointer array during executor startup and index into that to get at individual RangeTblEntrys. This eliminates one source of O(N^2) behavior with lots of partitions. (I'm not exactly convinced that it's the most important source, but it's an easy one to fix.) Amit Langote and David Rowley Discussion: https://postgr.es/m/468c85d9-540e-66a2-1dde-fec2b741e688@lab.ntt.co.jp
* Centralize executor's opening/closing of Relations for rangetable entries.Tom Lane2018-10-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | Create an array estate->es_relations[] paralleling the es_range_table, and store references to Relations (relcache entries) there, so that any given RT entry is opened and closed just once per executor run. Scan nodes typically still call ExecOpenScanRelation, but ExecCloseScanRelation is no more; relation closing is now done centrally in ExecEndPlan. This is slightly more complex than one would expect because of the interactions with relcache references held in ResultRelInfo nodes. The general convention is now that ResultRelInfo->ri_RelationDesc does not represent a separate relcache reference and so does not need to be explicitly closed; but there is an exception for ResultRelInfos in the es_trig_target_relations list, which are manufactured by ExecGetTriggerResultRel and have to be cleaned up by ExecCleanUpTriggerState. (That much was true all along, but these ResultRelInfos are now more different from others than they used to be.) To allow the partition pruning logic to make use of es_relations[] rather than having its own relcache references, adjust PartitionedRelPruneInfo to store an RT index rather than a relation OID. Amit Langote, reviewed by David Rowley and Jesper Pedersen, some mods by me Discussion: https://postgr.es/m/468c85d9-540e-66a2-1dde-fec2b741e688@lab.ntt.co.jp
* Fix duplicate primary keys in partitionsAlvaro Herrera2018-10-04
| | | | | | | | | | When using the CREATE TABLE .. PARTITION OF syntax, it's possible to cause a partition to get two primary keys if the parent already has one. Tighten the check to disallow that. Reported-by: Rajkumar Raghuwanshi Author: Amul Sul Discussion: https://postgr.es/m/CAKcux6=OnSV3-qd8Gb6W=KPPwcCz6Fe_O_MQYjTa24__Xn8XxA@mail.gmail.com
* Refactor user-facing SQL functions signalling backendsMichael Paquier2018-10-04
| | | | | | | | | | This moves the system administration functions for signalling backends from backend/utils/adt/misc.c into a separate file dedicated to backend signalling. No new functionality is introduced in this commit. Author: Daniel Gustafsson Reviewed-by: Michael Paquier, Álvaro Herrera Discussion: https://postgr.es/m/C2C7C3EC-CC5F-44B6-9C78-637C88BD7D14@yesql.se
* Add option SKIP_LOCKED to VACUUM and ANALYZEMichael Paquier2018-10-04
| | | | | | | | | | | | | | | | | | When specified, this option allows VACUUM to skip the work on a relation if there is a conflicting lock on it when trying to open it at the beginning of its processing. Similarly to autovacuum, this comes with a couple of limitations while the relation is processed which can cause the process to still block: - when opening the relation indexes. - when acquiring row samples for table inheritance trees, partition trees or certain types of foreign tables, and that a lock is taken on some leaves of such trees. Author: Nathan Bossart Reviewed-by: Michael Paquier, Andres Freund, Masahiko Sawada Discussion: https://postgr.es/m/9EF7EBE4-720D-4CF1-9D0E-4403D7E92990@amazon.com Discussion: https://postgr.es/m/20171201160907.27110.74730@wrigleys.postgresql.org
* Replace uint64 use introduced in 4868e446859 in light of 595a0eab7f42.Andres Freund2018-10-03
| | | | | Reported-By: Tom Lane Discussion: https://postgr.es/m/527.1538598263@sss.pgh.pa.us