8426 Commits (2f39106a209e647d7b1895331fca115f9bb6ec8d)

Author	SHA1	Message	Date
Tom Lane	4f3b38fe2b	Get rid of explicit argument-count markings in tab-complete.c. ... This replaces the "TailMatchesN" macros with just "TailMatches", and likewise "HeadMatchesN" becomes "HeadMatches" and "MatchesN" becomes "Matches". The various COMPLETE_WITH_LISTn macros are reduced to COMPLETE_WITH, and the single-item COMPLETE_WITH_CONST also gets folded into that. This eliminates a lot of minor annoyance in writing tab-completion rules. Usefully, the compiled code also gets a bit smaller (10% or so, on my machine). The implementation depends on variadic macros, so we couldn't have done this before we required C99. Andres Freund and Thomas Munro; some cosmetic cleanup by me. Discussion: https://postgr.es/m/d8jo9djvm7h.fsf@dalvik.ping.uio.no	7 years ago
Thomas Munro	f025bd2ddd	Use size_t consistently in dsa.{ch}. ... Takeshi Ideriha complained that there is a mixture of Size and size_t in dsa.c and corresponding header. Let's use size_t. Back-patch to 10 where dsa.c landed, to make future back-patching easy. Discussion: https://postgr.es/m/4E72940DA2BF16479384A86D54D0988A6F19ABD9%40G01JPEXMBKW04	7 years ago
Tom Lane	b09a64d602	Add missing pg_description strings for pg_type entries. ... I noticed that all non-composite, non-array entries in pg_type.dat had descr strings, except for "json" and the pseudo-types. The lack for json seems certainly an oversight, and there's surely little reason to not have entries for the pseudo-types either. So add some. "make reformat-dat-files" turned up some formatting issues in pg_amop.dat, too, so fix those in passing. No catversion bump since the backend doesn't care too much what is in pg_description.	7 years ago
Tom Lane	3dc820c43e	Teach genbki.pl to auto-generate pg_type entries for array types. ... This eliminates some more tedium in adding new catalog entries, specifically the need to set up an array type when adding a new built-in data type. Now it's sufficient to assign an OID for the array type and write it in an "array_type_oid" metadata field. You don't have to fill the base type's typarray link explicitly, either. No catversion bump since the contents of pg_type aren't changed. (Well, their order might be different, but that doesn't matter.) John Naylor, reviewed and whacked around a bit by Dagfinn Ilmari Mannsåker, and some more by me. Discussion: https://postgr.es/m/CAJVSVGVTb6m9pJF49b3SuA8J+T-THO9c0hxOmoyv-yGKh-FbNg@mail.gmail.com	7 years ago
Alexander Korotkov	2a6368343f	Add support for nearest-neighbor (KNN) searches to SP-GiST ... Currently, KNN searches were supported only by GiST. SP-GiST also capable to support them. This commit implements that support. SP-GiST scan stack is replaced with queue, which serves as stack if no ordering is specified. KNN support is provided for three SP-GIST opclasses: quad_point_ops, kd_point_ops and poly_ops (catversion is bumped). Some common parts between GiST and SP-GiST KNNs are extracted into separate functions. Discussion: https://postgr.es/m/570825e8-47d0-4732-2bf6-88d67d2d51c8%40postgrespro.ru Author: Nikita Glukhov, Alexander Korotkov based on GSoC work by Vlad Sterzhanov Review: Andrey Borodin, Alexander Korotkov	7 years ago
Tom Lane	d0cfc3d6a4	Add a debugging option to stress-test outfuncs.c and readfuncs.c. ... In the normal course of operation, query trees will be serialized only if they are stored as views or rules; and plan trees will be serialized only if they get passed to parallel-query workers. This leaves an awful lot of opportunity for bugs/oversights to not get detected, as indeed we've just been reminded of the hard way. To improve matters, this patch adds a new compile option WRITE_READ_PARSE_PLAN_TREES, which is modeled on the longstanding option COPY_PARSE_PLAN_TREES; but instead of passing all parse and plan trees through copyObject, it passes them through nodeToString + stringToNode. Enabling this option in a buildfarm animal or two will catch problems at least for cases that are exercised by the regression tests. A small problem with this idea is that readfuncs.c historically has discarded location fields, on the reasonable grounds that parse locations in a retrieved view are not relevant to the current query. But doing that in WRITE_READ_PARSE_PLAN_TREES breaks pg_stat_statements, and it could cause problems for future improvements that might try to report error locations at runtime. To fix that, provide a variant behavior in readfuncs.c that makes it restore location fields when told to. In passing, const-ify the string arguments of stringToNode and its subsidiary functions, just because it annoyed me that they weren't const already. Discussion: https://postgr.es/m/17114.1537138992@sss.pgh.pa.us	7 years ago
Tom Lane	db1071d4ee	Fix some minor issues exposed by outfuncs/readfuncs testing. ... A test patch to pass parse and plan trees through outfuncs + readfuncs exposed several issues that need to be fixed to get clean matches: Query.withCheckOptions failed to get copied; it's intentionally ignored by outfuncs/readfuncs on the grounds that it'd always be NIL anyway in stored rules. This seems less than future-proof, and it's not even saving very much, so just undo the decision and treat the field like all others. Several places that convert a view RTE into a subquery RTE, or similar manipulations, failed to clear out fields that were specific to the original RTE type and should be zero in a subquery RTE. Since readfuncs.c will leave such fields as zero, equalfuncs.c thinks the nodes are different leading to a reported mismatch. It seems like a good idea to clear out the no-longer-needed fields, even though in principle nothing should look at them; the node ought to be indistinguishable from how it would look if we'd built a new node instead of scribbling on the old one. BuildOnConflictExcludedTargetlist randomly set the resname of some TargetEntries to "" not NULL. outfuncs/readfuncs don't distinguish those cases, and so the string will read back in as NULL ... but equalfuncs.c does distinguish. Perhaps we ought to try to make things more consistent in this area --- but it's just useless extra code space for BuildOnConflictExcludedTargetlist to not use NULL here, so I fixed it for now by making it do that. catversion bumped because the change in handling of Query.withCheckOptions affects stored rules. Discussion: https://postgr.es/m/17114.1537138992@sss.pgh.pa.us	7 years ago
Tom Lane	09991e5a47	Fix some probably-minor oversights in readfuncs.c. ... The system expects TABLEFUNC RTEs to have coltypes, coltypmods, and colcollations lists, but outfuncs doesn't dump them and readfuncs doesn't restore them. This doesn't cause obvious failures, because the only things that look at those fields are expandRTE() and get_rte_attribute_type(), which are mostly used during parse analysis, before anything would've passed the parsetree through outfuncs/readfuncs. But expandRTE() is used in build_physical_tlist(), which means that that function will return a wrong answer for a TABLEFUNC RTE that came from a view. Very accidentally, this doesn't cause serious problems, because what it will return is NIL which callers will interpret as "couldn't build a physical tlist because of dropped columns". So you still get a plan that works, though it's marginally less efficient than it could be. There are also some other expandRTE() calls associated with transformation of whole-row Vars in the planner. I have been unable to exhibit misbehavior from that, and it may be unreachable in any case that anyone would care about ... but I'm not entirely convinced, so this seems like something we should back- patch a fix for. Fortunately, we can fix it without forcing a change of stored rules and a catversion bump, because we can just copy these lists from the subsidiary TableFunc object. readfuncs.c was also missing support for NamedTuplestoreScan plan nodes. This accidentally fails to break parallel query because a query using a named tuplestore would never be considered parallel-safe anyway. However, project policy since parallel query came in is that all plan node types should have outfuncs/readfuncs support, so this is clearly an oversight that should be repaired. Noted while fooling around with a patch to test outfuncs/readfuncs more thoroughly. That exposed some other issues too, but these are the only ones that seem worth back-patching. Back-patch to v10 where both of these features came in. Discussion: https://postgr.es/m/17114.1537138992@sss.pgh.pa.us	7 years ago
Michael Paquier	1d6fbc38d9	Refactor routines for subscription and publication lookups ... Those routines gain a missing_ok argument, allowing a caller to get a NULL result instead of an error if set to true. This is part of a larger refactoring effort for objectaddress.c where trying to check for non-existing objects does not result in cache lookup failures. Author: Michael Paquier Reviewed-by: Aleksander Alekseev, Álvaro Herrera Discussion: https://postgr.es/m/CAB7nPqSZxrSmdHK-rny7z8mi=EAFXJ5J-0RbzDw6aus=wB5azQ@mail.gmail.com	7 years ago
Tom Lane	07a3af0ff8	Fix parsetree representation of XMLTABLE(XMLNAMESPACES(DEFAULT ...)). ... The original coding for XMLTABLE thought it could represent a default namespace by a T_String Value node with a null string pointer. That's not okay, though; in particular outfuncs.c/readfuncs.c are not on board with such a representation, meaning you'll get a null pointer crash if you try to store a view or rule containing this construct. To fix, change the parsetree representation so that we have a NULL list element, instead of a bogus Value node. This isn't really a functional limitation since default XML namespaces aren't yet implemented in the executor; you'd just get "DEFAULT namespace is not supported" anyway. But crashes are not nice, so back-patch to v10 where this syntax was added. Ordinarily we'd consider a parsetree representation change to be un-backpatchable; but since existing releases would crash on the way to storing such constructs, there can't be any existing views/rules to be incompatible with. Per report from Andrey Lepikhov. Discussion: https://postgr.es/m/3690074f-abd2-56a9-144a-aa5545d7a291@postgrespro.ru	7 years ago
Tom Lane	1f4a920b73	Fix failure with initplans used conditionally during EvalPlanQual rechecks. ... The EvalPlanQual machinery assumes that any initplans (that is, uncorrelated sub-selects) used during an EPQ recheck would have already been evaluated during the main query; this is implicit in the fact that execPlan pointers are not copied into the EPQ estate's es_param_exec_vals. But it's possible for that assumption to fail, if the initplan is only reached conditionally. For example, a sub-select inside a CASE expression could be reached during a recheck when it had not been previously, if the CASE test depends on a column that was just updated. This bug is old, appearing to date back to my rewrite of EvalPlanQual in commit 9f2ee8f28, but was not detected until Kyle Samson reported a case. To fix, force all not-yet-evaluated initplans used within the EPQ plan subtree to be evaluated at the start of the recheck, before entering the EPQ environment. This could be inefficient, if such an initplan is expensive and goes unused again during the recheck --- but that's piling one layer of improbability atop another. It doesn't seem worth adding more complexity to prevent that, at least not in the back branches. It was convenient to use the new-in-v11 ExecEvalParamExecParams function to implement this, but I didn't like either its name or the specifics of its API, so revise that. Back-patch all the way. Rather than rewrite the patch to avoid depending on bms_next_member() in the oldest branches, I chose to back-patch that function into 9.4 and 9.3. (This isn't the first time back-patches have needed that, and it exhausted my patience.) I also chose to back-patch some test cases added by commits 71404af2a and 342a1ffa2 into 9.4 and 9.3, so that the 9.x versions of eval-plan-qual.spec are all the same. Andrew Gierth diagnosed the problem and contributed the added test cases, though the actual code changes are by me. Discussion: https://postgr.es/m/A033A40A-B234-4324-BE37-272279F7B627@tripadvisor.com	7 years ago
Alvaro Herrera	6b78231d91	Move PartitionDispatchData struct definition to execPartition.c ... There's no reason to expose the struct definition, so don't. Author: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> Discussion: https://postgr.es/m/d3fa24c1-bc65-7133-81df-6474387ccc4f@lab.ntt.co.jp	7 years ago
Amit Kapila	75f9c4ca5a	Don't allow LIMIT/OFFSET clause within sub-selects to be pushed to workers. ... Allowing sub-select containing LIMIT/OFFSET in workers can lead to inconsistent results at the top-level as there is no guarantee that the row order will be fully deterministic. The fix is to prohibit pushing LIMIT/OFFSET within sub-selects to workers. Reported-by: Andrew Fletcher Bug: 15324 Author: Amit Kapila Reviewed-by: Dilip Kumar Backpatch-through: 9.6 Discussion: https://postgr.es/m/153417684333.10284.11356259990921828616@wrigleys.postgresql.org	7 years ago
Michael Paquier	0ba06e0bfb	Allow concurrent-safe open() and fopen() in frontend code for Windows ... PostgreSQL uses a custom wrapper for open() and fopen() which is concurrent-safe, allowing multiple processes to open and work on the same file. This has a couple of advantages: - pg_test_fsync does not handle O_DSYNC correctly otherwise, leading to false claims that disks are unsafe. - TAP tests can run into race conditions when a postmaster and pg_ctl open postmaster.pid, fixing some random failures in the buildfam. pg_upgrade is one frontend tool using workarounds to bypass file locking issues with the log files it generates, however the interactions with pg_ctl are proving to be tedious to get rid of, so this is left for later. Author: Laurenz Albe Reviewed-by: Michael Paquier, Kuntal Ghosh Discussion: https://postgr.es/m/1527846213.2475.31.camel@cybertec.at Discussion: https://postgr.es/m/16922.1520722108@sss.pgh.pa.us	7 years ago
Tom Lane	14ea365203	Hide a static inline from FRONTEND code. ... For some reason pg_waldump is including tuptable.h, and the recent addition of a static inline function to it is causing problems on older buildfarm members that fail to optimize such functions away completely. I wonder if this situation doesn't mean that some header refactoring is called for ... but as a band-aid, wrap the static function in "#ifndef FRONTEND". Discussion: https://postgr.es/m/20180824154237.mabsv6fsz5q37bma@alap3.anarazel.de	7 years ago
Tom Lane	ed0cdf0e05	Install a check for mis-linking of src/port and src/common functions. ... On ELF-based platforms (and maybe others?) it's possible for a shared library, when dynamically loaded into the backend, to call the backend versions of src/port and src/common functions rather than the frontend versions that are actually linked into the shlib. This is definitely not what we want, because the frontend versions often behave slightly differently. Up to now it's been "slight" enough that nobody noticed; but with the addition of SCRAM support functions in src/common, we're observing crashes due to the difference between palloc and malloc memory allocation rules, as reported in bug #15367 from Jeremy Evans. The purpose of this patch is to create a direct test for this type of mis-linking, so that we know whether any given platform requires extra measures to prevent using the wrong functions. If the test fails, it will lead to connection failures in the contrib/postgres_fdw regression test. At the moment, *BSD platforms using ELF format are known to have the problem and can be expected to fail; but we need to know whether anything else does, and we need a reliable ongoing check for future platforms. Actually fixing the problem will be the subject of later commit(s). Discussion: https://postgr.es/m/153626613985.23143.4743626885618266803@wrigleys.postgresql.org	7 years ago
Tom Lane	361844fe56	Save/restore SPI's global variables in SPI_connect() and SPI_finish(). ... This patch removes two sources of interference between nominally independent functions when one SPI-using function calls another, perhaps without knowing that it does so. Chapman Flack pointed out that xml.c's query_to_xml_internal() expects SPI_tuptable and SPI_processed to stay valid across datatype output function calls; but it's possible that such a call could involve re-entrant use of SPI. It seems likely that there are similar hazards elsewhere, if not in the core code then in third-party SPI users. Previously SPI_finish() reset SPI's API globals to zeroes/nulls, which would typically make for a crash in such a situation. Restoring them to the values they had at SPI_connect() seems like a considerably more useful behavior, and it still meets the design goal of not leaving any dangling pointers to tuple tables of the function being exited. Also, cause SPI_connect() to reset these variables to zeroes/nulls after saving them. This prevents interference in the opposite direction: it's possible that a SPI-using function that's only ever been tested standalone contains assumptions that these variables start out as zeroes. That was the case as long as you were the outermost SPI user, but not so much for an inner user. Now it's consistent. Report and fix suggestion by Chapman Flack, actual patch by me. Back-patch to all supported branches. Discussion: https://postgr.es/m/9fa25bef-2e4f-1c32-22a4-3ad0723c4a17@anastigmatix.net	7 years ago
Tom Lane	f868a8143a	Fix longstanding recursion hazard in sinval message processing. ... LockRelationOid and sibling routines supposed that, if our session already holds the lock they were asked to acquire, they could skip calling AcceptInvalidationMessages on the grounds that we must have already read any remote sinval messages issued against the relation being locked. This is normally true, but there's a critical special case where it's not: processing inside AcceptInvalidationMessages might attempt to access system relations, resulting in a recursive call to acquire a relation lock. Hence, if the outer call had acquired that same system catalog lock, we'd fall through, despite the possibility that there's an as-yet-unread sinval message for that system catalog. This could, for example, result in failure to access a system catalog or index that had just been processed by VACUUM FULL. This is the explanation for buildfarm failures we've been seeing intermittently for the past three months. The bug is far older than that, but commits a54e1f158 et al added a new recursion case within AcceptInvalidationMessages that is apparently easier to hit than any previous case. To fix this, we must not skip calling AcceptInvalidationMessages until we have *finished* a call to it since acquiring a relation lock, not merely acquired the lock. (There's already adequate logic inside AcceptInvalidationMessages to deal with being called recursively.) Fortunately, we can implement that at trivial cost, by adding a flag to LOCALLOCK hashtable entries that tracks whether we know we have completed such a call. There is an API hazard added by this patch for external callers of LockAcquire: if anything is testing for LOCKACQUIRE_ALREADY_HELD, it might be fooled by the new return code LOCKACQUIRE_ALREADY_CLEAR into thinking the lock wasn't already held. This should be a fail-soft condition, though, unless something very bizarre is being done in response to the test. Also, I added an additional output argument to LockAcquireExtended, assuming that that probably isn't called by any outside code given the very limited usefulness of its additional functionality. Back-patch to all supported branches. Discussion: https://postgr.es/m/12259.1532117714@sss.pgh.pa.us	7 years ago
Peter Eisentraut	842cb9fa62	Refactor dlopen() support ... Nowadays, all platforms except Windows and older HP-UX have standard dlopen() support. So having a separate implementation per platform under src/backend/port/dynloader/ is a bit excessive. Instead, treat dlopen() like other library functions that happen to be missing sometimes and put a replacement implementation under src/port/. Discussion: https://www.postgresql.org/message-id/flat/e11a49cb-570a-60b7-707d-7084c8de0e61%402ndquadrant.com#54e735ae37476a121abb4e33c2549b03	7 years ago
Amit Kapila	ac27c74def	Fix the overrun in hash index metapage for smaller block sizes. ... The commit 620b49a1 changed the value of HASH_MAX_BITMAPS with the intent to allow many non-unique values in hash indexes without worrying to reach the limit of the number of overflow pages. At that time, this didn't occur to us that it can overrun the block for smaller block sizes. Choose the value of HASH_MAX_BITMAPS based on BLCKSZ such that it gives the same answer as now for the cases where the overrun doesn't occur, and some other sufficiently-value for the cases where an overrun currently does occur. This allows us not to change the behavior in any case that currently works, so there's really no reason for a HASH_VERSION bump. Author: Dilip Kumar Reviewed-by: Amit Kapila Backpatch-through: 10 Discussion: https://postgr.es/m/CAA4eK1LtF4VmU4mx_+i72ff1MdNZ8XaJMGkt2HV8+uSWcn8t4A@mail.gmail.com	7 years ago
Tom Lane	ae5205c8a8	Make argument names of pg_get_object_address consistent, and fix docs. ... pg_get_object_address and pg_identify_object_as_address are supposed to be inverses, but they disagreed as to the names of the arguments representing the textual form of an object address. Moreover, the documented argument names didn't agree with reality at all, either for these functions or pg_identify_object. In HEAD and v11, I think we can get away with renaming the input arguments of pg_get_object_address to match the outputs of pg_identify_object_as_address. In theory that might break queries using named-argument notation to call pg_get_object_address, but it seems really unlikely that anybody is doing that, or that they'd have much trouble adjusting if they were. In older branches, we'll just live with the lack of consistency. Aside from fixing the documentation of these functions to match reality, I couldn't resist the temptation to do some copy-editing. Per complaint from Jean-Pierre Pelletier. Back-patch to 9.5 where these functions were introduced. (Before v11, this is a documentation change only.) Discussion: https://postgr.es/m/CANGqjDnWH8wsTY_GzDUxbt4i=y-85SJreZin4Hm8uOqv1vzRQA@mail.gmail.com	7 years ago
Tom Lane	17b7c302b5	Fully enforce uniqueness of constraint names. ... It's been true for a long time that we expect names of table and domain constraints to be unique among the constraints of that table or domain. However, the enforcement of that has been pretty haphazard, and it missed some corner cases such as creating a CHECK constraint and then an index constraint of the same name (as per recent report from André Hänsel). Also, due to the lack of an actual unique index enforcing this, duplicates could be created through race conditions. Moreover, the code that searches pg_constraint has been quite inconsistent about how to handle duplicate names if one did occur: some places checked and threw errors if there was more than one match, while others just processed the first match they came to. To fix, create a unique index on (conrelid, contypid, conname). Since either conrelid or contypid is zero, this will separately enforce uniqueness of constraint names among constraints of any one table and any one domain. (If we ever implement SQL assertions, and put them into this catalog, more thought might be needed. But it'd be at least as reasonable to put them into a new catalog; having overloaded this one catalog with two kinds of constraints was a mistake already IMO.) This index can replace the existing non-unique index on conrelid, though we need to keep the one on contypid for query performance reasons. Having done that, we can simplify the logic in various places that either coped with duplicates or neglected to, as well as potentially improve lookup performance when searching for a constraint by name. Also, as per our usual practice, install a preliminary check so that you get something more friendly than a unique-index violation report in the case complained of by André. And teach ChooseIndexName to avoid choosing autogenerated names that would draw such a failure. While it's not possible to make such a change in the back branches, it doesn't seem quite too late to put this into v11, so do so. Discussion: https://postgr.es/m/0c1001d4428f$0942b430$1bc81c90$@webkr.de	7 years ago
Alvaro Herrera	c076f3d74a	Remove pg_constraint.conincluding ... This column was added in commit 8224de4f42 ("Indexes with INCLUDE columns and their support in B-tree") to ease writing the ruleutils.c supporting code for that feature, but it turns out to be unnecessary -- we can do the same thing with just one more syscache lookup. Even the documentation for the new column being removed in this commit is awkward. Discussion: https://postgr.es/m/20180902165018.33otxftp3olgtu4t@alvherre.pgsql	7 years ago
Tomas Vondra	4ddd8f5f55	Fix memory leak in TRUNCATE decoding ... When decoding a TRUNCATE record, the relids array was being allocated in the main ReorderBuffer memory context, but not released with the change resulting in a memory leak. The array was also ignored when serializing/deserializing the change, assuming all the information is stored in the change itself. So when spilling the change to disk, we've only we have serialized only the pointer to the relids array. Thanks to never releasing the array, the pointer however remained valid even after loading the change back to memory, preventing an actual crash. This fixes both the memory leak and (de)serialization. The relids array is still allocated in the main ReorderBuffer memory context (none of the existing ones seems like a good match, and adding an extra context seems like an overkill). The allocation is wrapped in a new ReorderBuffer API functions, to keep the details within reorderbuffer.c, just like the other ReorderBufferGet methods do. Author: Tomas Vondra Discussion: https://www.postgresql.org/message-id/flat/66175a41-9342-2845-652f-1bd4c3ee50aa%402ndquadrant.com Backpatch: 11, where decoding of TRUNCATE was introduced	7 years ago
Tom Lane	44cac93464	Avoid using potentially-under-aligned page buffers. ... There's a project policy against using plain "char buf[BLCKSZ]" local or static variables as page buffers; preferred style is to palloc or malloc each buffer to ensure it is MAXALIGN'd. However, that policy's been ignored in an increasing number of places. We've apparently got away with it so far, probably because (a) relatively few people use platforms on which misalignment causes core dumps and/or (b) the variables chance to be sufficiently aligned anyway. But this is not something to rely on. Moreover, even if we don't get a core dump, we might be paying a lot of cycles for misaligned accesses. To fix, invent new union types PGAlignedBlock and PGAlignedXLogBlock that the compiler must allocate with sufficient alignment, and use those in place of plain char arrays. I used these types even for variables where there's no risk of a misaligned access, since ensuring proper alignment should make kernel data transfers faster. I also changed some places where we had been palloc'ing short-lived buffers, for coding style uniformity and to save palloc/pfree overhead. Since this seems to be a live portability hazard (despite the lack of field reports), back-patch to all supported versions. Patch by me; thanks to Michael Paquier for review. Discussion: https://postgr.es/m/1535618100.1286.3.camel@credativ.de	7 years ago
Alexander Korotkov	ec74369931	Implement "pg_ctl logrotate" command ... Currently there are two ways to trigger log rotation in logging collector process: call pg_rotate_logfile() SQL-function or send SIGUSR1 signal directly to logging collector process. However, it's nice to have more suitable way for external tools to do that, which wouldn't require SQL connection or knowledge of logging collector pid. This commit implements triggering log rotation by "pg_ctl logrotate" command. Discussion: https://postgr.es/m/20180416.115435.28153375.horiguchi.kyotaro%40lab.ntt.co.jp Author: Kyotaro Horiguchi, Alexander Kuzmenkov, Alexander Korotkov	7 years ago
Andres Freund	5758685c9f	Fix 8a934d677 for libc++ and make more include order resistant. ... The previous definition was used in C++ mode, which causes problems when using clang with libc++ (rather than libstdc++), due to bugs therein. So just avoid in C++ mode. A second problem is that depending on include order and implicit includes the previous definition did not guarantee that the current hack was effective by the time isinf was used, fix that by forcing math.h to be included. This can cause clang using builds, or gcc using ones with JIT enabled, to slow down noticably. It's likely that we at some point want a better solution for the performance problem, but while it's there it should better work. Reported-By: Steven Winfield Bug: #15270 Discussion: https://postgr.es/m/153116283147.1401.360416241833049560@wrigleys.postgresql.org Author: Andres Freund Backpatch: 11, like the previous commit.	7 years ago
Tom Lane	8c62d9d16f	Make checksum_impl.h safe to compile with -fstrict-aliasing. ... In general, Postgres requires -fno-strict-aliasing with compilers that implement C99 strict aliasing rules. There's little hope of getting rid of that overall. But it seems like it would be a good idea if storage/checksum_impl.h in particular didn't depend on it, because that header is explicitly intended to be included by external programs. We don't have a lot of control over the compiler switches that an external program might use, as shown by Michael Banck's report of failure in a privately-modified version of pg_verify_checksums. Hence, switch to using a union in place of willy-nilly pointer casting inside this file. I think this makes the code a bit more readable anyway. checksum_impl.h hasn't changed since it was introduced in 9.3, so back-patch all the way. Discussion: https://postgr.es/m/1535618100.1286.3.camel@credativ.de	7 years ago
Etsuro Fujita	7cfdc77023	Disable support for partitionwise joins in problematic cases. ... Commit f49842d, which added support for partitionwise joins, built the child's tlist by applying adjust_appendrel_attrs() to the parent's. So in the case where the parent's included a whole-row Var for the parent, the child's contained a ConvertRowtypeExpr. To cope with that, that commit added code to the planner, such as setrefs.c, but some code paths still assumed that the tlist for a scan (or join) rel would only include Vars and PlaceHolderVars, which was true before that commit, causing errors: * When creating an explicit sort node for an input path for a mergejoin path for a child join, prepare_sort_from_pathkeys() threw the 'could not find pathkey item to sort' error. * When deparsing a relation participating in a pushed down child join as a subquery in contrib/postgres_fdw, get_relation_column_alias_ids() threw the 'unexpected expression in subquery output' error. * When performing set_plan_references() on a local join plan generated by contrib/postgres_fdw for EvalPlanQual support for a pushed down child join, fix_join_expr() threw the 'variable not found in subplan target lists' error. To fix these, two approaches have been proposed: one by Ashutosh Bapat and one by me. While the former keeps building the child's tlist with a ConvertRowtypeExpr, the latter builds it with a whole-row Var for the child not to violate the planner assumption, and tries to fix it up later, But both approaches need more work, so refuse to generate partitionwise join paths when whole-row Vars are involved, instead. We don't need to handle ConvertRowtypeExprs in the child's tlists for now, so this commit also removes the changes to the planner. Previously, partitionwise join computed attr_needed data for each child separately, and built the child join's tlist using that data, which also required an extra step for adding PlaceHolderVars to that tlist, but it would be more efficient to build it from the parent join's tlist through the adjust_appendrel_attrs() transformation. So this commit builds that list that way, and simplifies build_joinrel_tlist() and placeholder.c as well as part of set_append_rel_size() to basically what they were before partitionwise join went in. Back-patch to PG11 where partitionwise join was introduced. Report by Rajkumar Raghuwanshi. Analysis by Ashutosh Bapat, who also provided some of regression tests. Patch by me, reviewed by Robert Haas. Discussion: https://postgr.es/m/CAKcux6ktu-8tefLWtQuuZBYFaZA83vUzuRd7c1YHC-yEWyYFpg@mail.gmail.com	7 years ago
Peter Eisentraut	7061e03319	Add semicolons to end of internally run queries ... This ensures that the --echo output of various tools (under scripts) is valid multiline SQL. Author: Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp>	7 years ago
Peter Eisentraut	a4a232b1e7	Error position support for defaults and check constraints ... Add support for error position reporting for the expressions contained in defaults and check constraint definitions. This currently works only for CREATE TABLE, not ALTER TABLE, because the latter is not set up to pass around the original query string. Reviewed-by: Fabien COELHO <coelho@cri.ensmp.fr>	7 years ago
Heikki Linnakangas	4b035841a1	Fix IndexInfo comments. ... Recently, ii_KeyAttrNumbers was renamed to ii_IndexAttrNumbers, and ii_Am field was added, but the comments were not updated. Author: Yugo Nagata Discussion: https://www.postgresql.org/message-id/20180830134831.e35a91b8b978b248c16c8f7b@sraoss.co.jp	7 years ago
Thomas Munro	ee0e2745c2	Code review for simplehash.h. ... Fix reference to non-existent file in comment. Add SH_ prefix to the EMPTY and IN_USE tokens, to reduce likelihood of collisions with unrelated macros. Add include guards around the function definitions that are not "parameterized", so the header can be used again in the same translation unit. Undefine SH_EQUAL macro where other "parameter" macros are undefined, for the same reason. Author: Thomas Munro Reviewed-by: Tom Lane Discussion: https://postgr.es/m/CAEepm%3D1LdXZ3mMTM8tHt_b%3DK1kREit%3Dp8sikesak%3DkzHHM07Nw%40mail.gmail.com	7 years ago
Peter Eisentraut	9b39b799db	Add some not null constraints to catalogs ... Use BKI_FORCE_NOT_NULL on some catalog field declarations that are never null (according to the source code that accesses them).	7 years ago
Michael Paquier	a556549d7e	Improve VACUUM and ANALYZE by avoiding early lock queue ... A caller of VACUUM can perform early lookup obtention which can cause other sessions to block on the request done, causing potentially DOS attacks as even a non-privileged user can attempt a vacuum fill of a critical catalog table to block even all incoming connection attempts. Contrary to TRUNCATE, a client could attempt a system-wide VACUUM after building the list of relations to VACUUM, which can cause vacuum_rel() or analyze_rel() to try to lock the relation but the operation would just block. When the client specifies a list of relations and the relation needs to be skipped, ownership checks are done when building the list of relations to work on, preventing a later lock attempt. vacuum_rel() already had the sanity checks needed, except that those were applied too late. This commit refactors the code so as relation skips are checked beforehand, making it safer to avoid too early locks, for both manual VACUUM with and without a list of relations specified. An isolation test is added emulating the fact that early locks do not happen anymore, issuing a WARNING message earlier if the user calling VACUUM is not a relation owner. When a partitioned table is listed in a manual VACUUM or ANALYZE command, its full list of partitions is fetched, all partitions get added to the list to work on, and then each one of them is processed one by one, with ownership checks happening at the later phase of vacuum_rel() or analyze_rel(). Trying to do early ownership checks for each partition is proving to be tedious as this would result in deadlock risks with lock upgrades, and skipping all partitions if the listed partitioned table is not owned would result in a behavior change compared to how Postgres 10 has implemented vacuum for partitioned tables. The original problem reported related to early lock queue for critical relations is fixed anyway, so priority is given to avoiding a backward-incompatible behavior. Reported-by: Lloyd Albin, Jeremy Schneider Author: Michael Paquier Reviewed by: Nathan Bossart, Kyotaro Horiguchi Discussion: https://postgr.es/m/152512087100.19803.12733865831237526317@wrigleys.postgresql.org Discussion: https://postgr.es/m/20180812222142.GA6097@paquier.xyz	7 years ago
Jeff Davis	e75733d46c	Comment fix for rewriteheap.h. ... The description of the filename for mapping files did not match the code.	7 years ago
Andres Freund	8ecdefc261	Remove test for VA_ARGS, implied by C99. ... This simplifies logic / reduces duplication in a few headers. Author: Andres Freund Discussion: https://postgr.es/m/97d4b165-192d-3605-749c-f614a0c4e783@2ndquadrant.com	7 years ago
Andres Freund	cb92520563	LLVMJIT: LLVMGetHostCPUFeatures now is upstream, use LLMV version if available. ... Noticed thanks to buildfarm animal seawasp. Author: Andres Freund Backpatch: v11-, where LLVM based JIT compliation was introduced.	7 years ago
Andres Freund	88ebd62fcc	Deduplicate code between slot_getallattrs() and slot_getsomeattrs(). ... Code in slot_getallattrs() is the same as if slot_getsomeattrs() is called with number of attributes specified in the tuple descriptor. Implement it that way instead of duplicating the code between those two functions. This is part of a patchseries abstracting TupleTableSlots so they can store arbitrary forms of tuples, but is a nice enough cleanup on its own. Author: Ashutosh Bapat Reviewed-By: Andres Freund Discussion: https://postgr.es/m/20180220224318.gw4oe5jadhpmcdnm@alap3.anarazel.de	7 years ago
Peter Eisentraut	b19495772e	doc: Update uses of the word "procedure" ... Historically, the term procedure was used as a synonym for function in Postgres/PostgreSQL. Now we have procedures as separate objects from functions, so we need to clean up the documentation to not mix those terms. In particular, mentions of "trigger procedures" are changed to "trigger functions", and access method "support procedures" are changed to "support functions". (The latter already used FUNCTION in the SQL syntax anyway.) Also, the terminology in the SPI chapter has been cleaned up. A few tests, examples, and code comments are also adjusted to be consistent with documentation changes, but not everything. Reported-by: Peter Geoghegan <pg@bowt.ie> Reviewed-by: Jonathan S. Katz <jonathan.katz@excoventures.com>	7 years ago
Tom Lane	6771c932cf	Ensure schema qualification in pg_restore DISABLE/ENABLE TRIGGER commands. ... Previously, this code blindly followed the common coding pattern of passing PQserverVersion(AH->connection) as the server-version parameter of fmtQualifiedId. That works as long as we have a connection; but in pg_restore with text output, we don't. Instead we got a zero from PQserverVersion, which fmtQualifiedId interpreted as "server is too old to have schemas", and so the name went unqualified. That still accidentally managed to work in many cases, which is probably why this ancient bug went undetected for so long. It only became obvious in the wake of the changes to force dump/restore to execute with restricted search_path. In HEAD/v11, let's deal with this by ripping out fmtQualifiedId's server- version behavioral dependency, and just making it schema-qualify all the time. We no longer support pg_dump from servers old enough to need the ability to omit schema name, let alone restoring to them. (Also, the few callers outside pg_dump already didn't work with pre-schema servers.) In older branches, that's not an acceptable solution, so instead just tweak the DISABLE/ENABLE TRIGGER logic to ensure it will schema-qualify its output regardless of server version. Per bug #15338 from Oleg somebody. Back-patch to all supported branches. Discussion: https://postgr.es/m/153452458706.1316.5328079417086507743@wrigleys.postgresql.org	7 years ago
Tomas Vondra	c4c3400885	Use the built-in float datatypes to implement geometric types ... This patch makes the geometric operators and functions use the exported function of the float4/float8 datatypes. The main reason of doing so is to check for underflow and overflow, and to handle NaNs consciously. The float datatypes consider NaNs values to be equal and greater than all non-NaN values. This change considers NaNs equal only for equality operators. The placement operators, contains, overlaps, left/right of etc. continue to return false when NaNs are involved. We don't need to worry about them being considered greater than any-NaN because there aren't any basic comparison operators like less/greater than for the geometric datatypes. The changes may be summarised as: * Check for underflow, overflow and division by zero * Consider NaN values to be equal * Return NULL when the distance is NaN for all closest point operators * Favour not-NaN over NaN where it makes sense The patch also replaces all occurrences of "double" as "float8". They are the same, but were used inconsistently in the same file. Author: Emre Hasegeli Reviewed-by: Kyotaro Horiguchi, Tomas Vondra Discussion: https://www.postgresql.org/message-id/CAE2gYzxF7-5djV6-cEvqQu-fNsnt%3DEqbOURx7ZDg%2BVv6ZMTWbg%40mail.gmail.com	7 years ago
Tom Lane	02dc7466ba	Remove duplicate function declarations. ... Christoph Berg Discussion: https://postgr.es/m/20180814165536.GB21152@msg.df7cb.de	7 years ago
Peter Eisentraut	351855fc4e	Remove obsolete linux dynloader code ... This has been obsolete probably since the late 1990s.	7 years ago
Michael Paquier	246a6c8f7b	Make autovacuum more aggressive to remove orphaned temp tables ... Commit dafa084, added in 10, made the removal of temporary orphaned tables more aggressive. This commit makes an extra step into the aggressiveness by adding a flag in each backend's MyProc which tracks down any temporary namespace currently in use. The flag is set when the namespace gets created and can be reset if the temporary namespace has been created in a transaction or sub-transaction which is aborted. The flag value assignment is assumed to be atomic, so this can be done in a lock-less fashion like other flags already present in PGPROC like databaseId or backendId, still the fact that the temporary namespace and table created are still locked until the transaction creating those commits acts as a barrier for other backends. This new flag gets used by autovacuum to discard more aggressively orphaned tables by additionally checking for the database a backend is connected to as well as its temporary namespace in-use, removing orphaned temporary relations even if a backend reuses the same slot as one which created temporary relations in a past session. The base idea of this patch comes from Robert Haas, has been written in its first version by Tsunakawa Takayuki, then heavily reviewed by me. Author: Tsunakawa Takayuki Reviewed-by: Michael Paquier, Kyotaro Horiguchi, Andres Freund Discussion: https://postgr.es/m/0A3221C70F24FB45833433255569204D1F8A4DC6@G01JPEXMBYT05 Backpatch: 11-, as PGPROC gains a new flag and we don't want silent ABI breakages on already released versions.	7 years ago
Andrew Gierth	07172d5aff	Avoid query-lifetime memory leaks in XMLTABLE (bug #15321) ... Multiple calls to XMLTABLE in a query (e.g. laterally applying it to a table with an xml column, an important use-case) were leaking large amounts of memory into the per-query context, blowing up memory usage. Repair by reorganizing memory context usage in nodeTableFuncscan; use the usual per-tuple context for row-by-row evaluations instead of perValueCxt, and use the explicitly created context -- renamed from perValueCxt to perTableCxt -- for arguments and state for each individual table-generation operation. Backpatch to PG10 where this code was introduced. Original report by IRC user begriffs; analysis and patch by me. Reviewed by Tom Lane and Pavel Stehule. Discussion: https://postgr.es/m/153394403528.10284.7530399040974170549@wrigleys.postgresql.org	7 years ago
Tom Lane	46b5e7c4b5	Revert "Distinguish printf-like functions that support %m from those that don't." ... This reverts commit 3a60c8ff89. Buildfarm results show that that caused a whole bunch of new warnings on platforms where gcc believes the local printf to be non-POSIX-compliant. This problem outweighs the hypothetical-anyway possibility of getting warnings for misuse of %m. We could use gnu_printf archetype when we've substituted src/port/snprintf.c, but that brings us right back to the problem of not getting warnings for %m. A possible answer is to attack it in the other direction by insisting that %m support be included in printf's feature set, but that will take more investigation. In the meantime, revert the previous change, and update the comment for PGAC_C_PRINTF_ARCHETYPE to more fully explain what's going on. Discussion: https://postgr.es/m/2975.1526862605@sss.pgh.pa.us	7 years ago
Tom Lane	a2a8acd152	Produce compiler errors if errno is referenced inside elog/ereport calls. ... It's often unsafe to reference errno within an elog/ereport call, because there are a lot of sub-functions involved and they might not all preserve errno. (This is why we support the %m format spec: it works off a value of errno captured before we execute any potentially-unsafe functions in the arguments.) Therefore, we have a project policy not to use errno there. This patch adds a hack to cause an (admittedly obscure) compiler error for such unsafe usages. With the current code, the error will only be seen on Linux, macOS, and FreeBSD, but that should certainly be enough to catch mistakes in the buildfarm if they somehow get missed earlier. In addition, fix some places in src/common/exec.c that trip the error. I think these places are actually all safe, but it's simple enough to avoid the error by capturing errno manually, and doing so is good future-proofing in case these call sites get any more complicated. Thomas Munro (exec.c fixes by me) Discussion: https://postgr.es/m/2975.1526862605@sss.pgh.pa.us	7 years ago
Tom Lane	3a60c8ff89	Distinguish printf-like functions that support %m from those that don't. ... The elog/ereport family of functions certainly support the %m format spec, because they implement it "by hand". But elsewhere we have printf wrappers that might or might not allow it depending on whether the platform's printf does. (Most non-glibc versions don't, and notably, src/port/snprintf.c doesn't.) Hence, rather than using the gnu_printf format archetype interchangeably for all these functions, use it only for elog/ereport. This will allow us to get compiler warnings for mistakes like the ones fixed in commit a13b47a59, at least on platforms where printf doesn't take %m and gcc is correctly configured to know it. (Unfortunately, that won't happen on Linux, nor on macOS according to my testing. It remains to be seen what the buildfarm's gcc-on-Windows animals will think of this, but we may well have to rely on less-popular platforms to warn us about unportable code of this kind.) Discussion: https://postgr.es/m/2975.1526862605@sss.pgh.pa.us	7 years ago
Peter Geoghegan	4974d7f87e	Handle parallel index builds on mapped relations. ... Commit 9da0cc3528, which introduced parallel CREATE INDEX, failed to propagate relmapper.c backend local cache state to parallel worker processes. This could result in parallel index builds against mapped catalog relations where the leader process (participating as a worker) scans the new, pristine relfilenode, while worker processes scan the obsolescent relfilenode. When this happened, the final index structure was typically not consistent with the owning table's structure. The final index structure could contain entries formed from both heap relfilenodes. Only rebuilds on mapped catalog relations that occur as part of a VACUUM FULL or CLUSTER could become corrupt in practice, since their mapped relation relfilenode swap is what allows the inconsistency to arise. On master, fix the problem by propagating the required relmapper.c backend state as part of standard parallel initialization (Cf. commit 29d58fd3). On v11, simply disallow builds against mapped catalog relations by deeming them parallel unsafe. Author: Peter Geoghegan Reported-By: "death lock" Reviewed-By: Tom Lane, Amit Kapila Bug: #15309 Discussion: https://postgr.es/m/153329671686.1405.18298309097348420351@wrigleys.postgresql.org Backpatch: 11-, where parallel CREATE INDEX was introduced.	7 years ago