aboutsummaryrefslogtreecommitdiff
path: root/contrib
Commit message (Collapse)AuthorAge
...
* hash: Increase the number of possible overflow bitmaps by 8x.Robert Haas2017-08-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Per a report from AP, it's not that hard to exhaust the supply of bitmap pages if you create a table with a hash index and then insert a few billion rows - and then you start getting errors when you try to insert additional rows. In the particular case reported by AP, there's another fix that we can make to improve recycling of overflow pages, which is another way to avoid the error, but there may be other cases where this problem happens and that fix won't help. So let's buy ourselves as much headroom as we can without rearchitecting anything. The comments claim that the old limit was 64GB, but it was really only 32GB, because we didn't use all the bits in the page for bitmap bits - only the largest power of 2 that could fit after deducting space for the page header and so forth. Thus, we have 4kB per page for bitmap bits, not 8kB. The new limit is thus actually 8 times the old *real* limit but only 4 times the old *purported* limit. Since this breaks on-disk compatibility, bump HASH_VERSION. We've already done this earlier in this release cycle, so this doesn't cause any incremental inconvenience for people using pg_upgrade from releases prior to v10. However, users who use pg_upgrade to reach 10beta3 or later from 10beta2 or earlier will need to REINDEX any hash indexes again. Amit Kapila and Robert Haas Discussion: http://postgr.es/m/20170704105728.mwb72jebfmok2nm2@zip.com.au
* PL/Perl portability fix: absorb relevant -D switches from Perl.Tom Lane2017-07-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The Perl documentation is very clear that stuff calling libperl should be built with the compiler switches shown by Perl's $Config{ccflags}. We'd been ignoring that up to now, and mostly getting away with it, but recent Perl versions contain ABI compatibility cross-checks that fail on some builds because of this omission. In particular the sizeof(PerlInterpreter) can come out different due to some fields being added or removed; which means we have a live ABI hazard that we'd better fix rather than continuing to sweep it under the rug. However, it still seems like a bad idea to just absorb $Config{ccflags} verbatim. In some environments Perl was built with a different compiler that doesn't even use the same switch syntax. -D switch syntax is pretty universal though, and absorbing Perl's -D switches really ought to be enough to fix the problem. Furthermore, Perl likes to inject stuff like -D_LARGEFILE_SOURCE and -D_FILE_OFFSET_BITS=64 into $Config{ccflags}, which affect libc ABIs on platforms where they're relevant. Adopting those seems dangerous too. It's unclear whether a build wherein Perl and Postgres have different ideas of sizeof(off_t) etc would work, or whether anyone would care about making it work. But it's dead certain that having different stdio ABIs in core Postgres and PL/Perl will not work; we've seen that movie before. Therefore, let's also ignore -D switches for symbols beginning with underscore. The symbols that we actually need to import should be the ones mentioned in perl.h's PL_bincompat_options stanza, and none of those start with underscore, so this seems likely to work. (If it turns out not to work everywhere, we could consider intersecting the symbols mentioned in PL_bincompat_options with the -D switches. But that will be much more complicated, so let's try this way first.) This will need to be back-patched, but first let's see what the buildfarm makes of it. Ashutosh Sharma, some adjustments by me Discussion: https://postgr.es/m/CANFyU97OVQ3+Mzfmt3MhuUm5NwPU=-FtbNH5Eb7nZL9ua8=rcA@mail.gmail.com
* PL/Perl portability fix: avoid including XSUB.h in plperl.c.Tom Lane2017-07-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In Perl builds that define PERL_IMPLICIT_SYS, XSUB.h defines macros that replace a whole lot of basic libc functions with Perl functions. We can't tolerate that in plperl.c; it breaks at least PG_TRY and probably other stuff. The core idea of this patch is to include XSUB.h only in the .xs files where it's really needed, and to move any code broken by PERL_IMPLICIT_SYS out of the .xs files and into plperl.c. The reason this hasn't been a problem before is that our build techniques did not result in PERL_IMPLICIT_SYS appearing as a #define in PL/Perl, even on some platforms where Perl thinks it is defined. That's about to change in order to fix a nasty portability issue, so we need this work to make the code safe for that. Rather unaccountably, the Perl people chose XSUB.h as the place to provide the versions of the aTHX/aTHX_ macros that are needed by code that's not explicitly aware of the MULTIPLICITY API conventions. Hence, just removing XSUB.h from plperl.c fails miserably. But we can work around that by defining PERL_NO_GET_CONTEXT (which would make the relevant stanza of XSUB.h a no-op anyway). As explained in perlguts.pod, that means we need to add a "dTHX" macro call in every C function that calls a Perl API function. In most of them we just add this at the top; but since the macro fetches the current Perl interpreter pointer, more care is needed in functions that switch the active interpreter. Lack of the macro is easily recognized since it results in bleats about "my_perl" not being defined. (A nice side benefit of this is that it significantly reduces the number of fetches of the current interpreter pointer. On my machine, plperl.so gets more than 10% smaller, and there's probably some performance win too. We could reduce the number of fetches still more by decorating the code with pTHX_/aTHX_ macros to pass the interpreter pointer around, as explained by perlguts.pod; but that's a task for another day.) Formatting note: pgindent seems happy to treat "dTHX;" as a declaration so long as it's the first thing after the left brace, as we'd already observed with respect to the similar macro "dSP;". If you try to put it later in a set of declarations, pgindent puts ugly extra space around it. Having removed XSUB.h from plperl.c, we need only move the support functions for spi_return_next and util_elog (both of which use PG_TRY) out of the .xs files and into plperl.c. This seems sufficient to avoid the known problems caused by PERL_IMPLICIT_SYS, although we could move more code if additional issues emerge. This will need to be back-patched, but first let's see what the buildfarm makes of it. Patch by me, with some help from Ashutosh Sharma Discussion: https://postgr.es/m/CANFyU97OVQ3+Mzfmt3MhuUm5NwPU=-FtbNH5Eb7nZL9ua8=rcA@mail.gmail.com
* When WCOs are present, disable direct foreign table modification.Robert Haas2017-07-24
| | | | | | | | | | | | | | | If the user modifies a view that has CHECK OPTIONs and this gets translated into a modification to an underlying relation which happens to be a foreign table, the check options should be enforced. In the normal code path, that was happening properly, but it was not working properly for "direct" modification because the whole operation gets pushed to the remote side in that case and we never have an option to enforce the constraint against individual tuples. Fix by disabling direct modification when there is a need to enforce CHECK OPTIONs. Etsuro Fujita, reviewed by Kyotaro Horiguchi and by me. Discussion: http://postgr.es/m/f8a48f54-6f02-9c8a-5250-9791603171ee@lab.ntt.co.jp
* Stabilize postgres_fdw regression tests.Tom Lane2017-07-21
| | | | | | | | | | | | | The new test cases added in commit 8bf58c0d9 turn out to have output that can vary depending on the lc_messages setting prevailing on the test server. Hide the remote end's error messages to ensure stable output. This isn't a terribly desirable solution; we'd rather know that the connection failed for the expected reason and not some other one. But there seems little choice for the moment. Per buildfarm. Discussion: https://postgr.es/m/18419.1500658570@sss.pgh.pa.us
* Re-establish postgres_fdw connections after server or user mapping changes.Tom Lane2017-07-21
| | | | | | | | | | | | | | | | | Previously, postgres_fdw would keep on using an existing connection even if the user did ALTER SERVER or ALTER USER MAPPING commands that should affect connection parameters. Teach it to watch for catcache invals on these catalogs and re-establish connections when the relevant catalog entries change. Per bug #14738 from Michal Lis. In passing, clean up some rather crufty decisions in commit ae9bfc5d6 about where fields of ConnCacheEntry should be reset. We now reset all the fields whenever we open a new connection. Kyotaro Horiguchi, reviewed by Ashutosh Bapat and myself. Back-patch to 9.3 where postgres_fdw appeared. Discussion: https://postgr.es/m/20170710113917.7727.10247@wrigleys.postgresql.org
* Code review for NextValueExpr expression node type.Tom Lane2017-07-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add missing infrastructure for this node type, notably in ruleutils.c where its lack could demonstrably cause EXPLAIN to fail. Add outfuncs/readfuncs support. (outfuncs support is useful today for debugging purposes. The readfuncs support may never be needed, since at present it would only matter for parallel query and NextValueExpr should never appear in a parallelizable query; but it seems like a bad idea to have a primnode type that isn't fully supported here.) Teach planner infrastructure that NextValueExpr is a volatile, parallel-unsafe, non-leaky expression node with cost cpu_operator_cost. Given its limited scope of usage, there *might* be no live bug today from the lack of that knowledge, but it's certainly going to bite us on the rear someday. Teach pg_stat_statements about the new node type, too. While at it, also teach cost_qual_eval() that MinMaxExpr, SQLValueFunction, XmlExpr, and CoerceToDomain should be charged as cpu_operator_cost. Failing to do this for SQLValueFunction was an oversight in my commit 0bb51aa96. The others are longer-standing oversights, but no time like the present to fix them. (In principle, CoerceToDomain could have cost much higher than this, but it doesn't presently seem worth trying to examine the domain's constraints here.) Modify execExprInterp.c to execute NextValueExpr as an out-of-line function; it seems quite unlikely to me that it's worth insisting that it be inlined in all expression eval methods. Besides, providing the out-of-line function doesn't stop anyone from inlining if they want to. Adjust some places where NextValueExpr support had been inserted with the aid of a dartboard rather than keeping it in the same order as elsewhere. Discussion: https://postgr.es/m/23862.1499981661@sss.pgh.pa.us
* Forbid gen_random_uuid() with --disable-strong-randomHeikki Linnakangas2017-07-03
| | | | | | | | | | | | | | | Previously, gen_random_uuid() would fall back to a weak random number generator, unlike gen_random_bytes() which would just fail. And this was not made very clear in the docs. For consistency, also make gen_random_uuid() fail outright, if compiled with --disable-strong-random. Re-word the error message you get with --disable-strong-random. It is also used by pgp functions that require random salts, and now also gen_random_uuid(). Reported by Radek Slupik. Discussion: https://www.postgresql.org/message-id/20170101232054.10135.50528@wrigleys.postgresql.org
* Fix typo in commentPeter Eisentraut2017-06-30
| | | | Author: Albe Laurenz <laurenz.albe@wien.gv.at>
* postgres_fdw: Move function prototype to correct section.Robert Haas2017-06-22
| | | | | | Etsuro Fujita, reviewed by Ashutosh Bapat. Discussion: http://postgr.es/m/93a9c487-9920-a38f-da96-503422c50f59@lab.ntt.co.jp
* Teach pgrowlocks to check relkind before scanningPeter Eisentraut2017-06-21
| | | | Author: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
* Phase 3 of pgindent updates.Tom Lane2017-06-21
| | | | | | | | | | | | | | | | | | | | | | | | | Don't move parenthesized lines to the left, even if that means they flow past the right margin. By default, BSD indent lines up statement continuation lines that are within parentheses so that they start just to the right of the preceding left parenthesis. However, traditionally, if that resulted in the continuation line extending to the right of the desired right margin, then indent would push it left just far enough to not overrun the margin, if it could do so without making the continuation line start to the left of the current statement indent. That makes for a weird mix of indentations unless one has been completely rigid about never violating the 80-column limit. This behavior has been pretty universally panned by Postgres developers. Hence, disable it with indent's new -lpl switch, so that parenthesized lines are always lined up with the preceding left paren. This patch is much less interesting than the first round of indent changes, but also bulkier, so I thought it best to separate the effects. Discussion: https://postgr.es/m/E1dAmxK-0006EE-1r@gemulon.postgresql.org Discussion: https://postgr.es/m/30527.1495162840@sss.pgh.pa.us
* Phase 2 of pgindent updates.Tom Lane2017-06-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Change pg_bsd_indent to follow upstream rules for placement of comments to the right of code, and remove pgindent hack that caused comments following #endif to not obey the general rule. Commit e3860ffa4dd0dad0dd9eea4be9cc1412373a8c89 wasn't actually using the published version of pg_bsd_indent, but a hacked-up version that tried to minimize the amount of movement of comments to the right of code. The situation of interest is where such a comment has to be moved to the right of its default placement at column 33 because there's code there. BSD indent has always moved right in units of tab stops in such cases --- but in the previous incarnation, indent was working in 8-space tab stops, while now it knows we use 4-space tabs. So the net result is that in about half the cases, such comments are placed one tab stop left of before. This is better all around: it leaves more room on the line for comment text, and it means that in such cases the comment uniformly starts at the next 4-space tab stop after the code, rather than sometimes one and sometimes two tabs after. Also, ensure that comments following #endif are indented the same as comments following other preprocessor commands such as #else. That inconsistency turns out to have been self-inflicted damage from a poorly-thought-through post-indent "fixup" in pgindent. This patch is much less interesting than the first round of indent changes, but also bulkier, so I thought it best to separate the effects. Discussion: https://postgr.es/m/E1dAmxK-0006EE-1r@gemulon.postgresql.org Discussion: https://postgr.es/m/30527.1495162840@sss.pgh.pa.us
* Initial pgindent run with pg_bsd_indent version 2.0.Tom Lane2017-06-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The new indent version includes numerous fixes thanks to Piotr Stefaniak. The main changes visible in this commit are: * Nicer formatting of function-pointer declarations. * No longer unexpectedly removes spaces in expressions using casts, sizeof, or offsetof. * No longer wants to add a space in "struct structname *varname", as well as some similar cases for const- or volatile-qualified pointers. * Declarations using PG_USED_FOR_ASSERTS_ONLY are formatted more nicely. * Fixes bug where comments following declarations were sometimes placed with no space separating them from the code. * Fixes some odd decisions for comments following case labels. * Fixes some cases where comments following code were indented to less than the expected column 33. On the less good side, it now tends to put more whitespace around typedef names that are not listed in typedefs.list. This might encourage us to put more effort into typedef name collection; it's not really a bug in indent itself. There are more changes coming after this round, having to do with comment indentation and alignment of lines appearing within parentheses. I wanted to limit the size of the diffs to something that could be reviewed without one's eyes completely glazing over, so it seemed better to split up the changes as much as practical. Discussion: https://postgr.es/m/E1dAmxK-0006EE-1r@gemulon.postgresql.org Discussion: https://postgr.es/m/30527.1495162840@sss.pgh.pa.us
* Fix low-probability leaks of PGresult objects in the backend.Tom Lane2017-06-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We had three occurrences of essentially the same coding pattern wherein we tried to retrieve a query result from a libpq connection without blocking. In the case where PQconsumeInput failed (typically indicating a lost connection), all three loops simply gave up and returned, forgetting to clear any previously-collected PGresult object. Since those are malloc'd not palloc'd, the oversight results in a process-lifespan memory leak. One instance, in libpqwalreceiver, is of little significance because the walreceiver process would just quit anyway if its connection fails. But we might as well fix it. The other two instances, in postgres_fdw, are somewhat more worrisome because at least in principle the scenario could be repeated, allowing the amount of memory leaked to build up to something worth worrying about. Moreover, in these cases the loops contain CHECK_FOR_INTERRUPTS calls, as well as other calls that could potentially elog(ERROR), providing another way to exit without having cleared the PGresult. Here we need to add PG_TRY logic similar to what exists in quite a few other places in postgres_fdw. Coverity noted the libpqwalreceiver bug; I found the other two cases by checking all calls of PQconsumeInput. Back-patch to all supported versions as appropriate (9.2 lacks postgres_fdw, so this is really quite unexciting for that branch). Discussion: https://postgr.es/m/22620.1497486981@sss.pgh.pa.us
* psql: Use more consistent capitalization of some output headingsPeter Eisentraut2017-06-13
|
* Fix contrib/sepgsql regr tests for tup-routing constraint check change.Joe Conway2017-06-07
| | | | | | | Commit 15ce775 changed tuple-routing constraint checking logic. This affects the expected output for contrib/sepgsql, because there's no longer LOG entries reporting allowance of int4eq() execution. Per buildfarm.
* postgres_fdw: Allow cancellation of transaction control commands.Robert Haas2017-06-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit f039eaac7131ef2a4cf63a10cf98486f8bcd09d2, later back-patched with commit 1b812afb0eafe125b820cc3b95e7ca03821aa675, allowed many of the queries issued by postgres_fdw to fetch remote data to respond to cancel interrupts in a timely fashion. However, it didn't do anything about the transaction control commands, which remained noninterruptible. Improve the situation by changing do_sql_command() to retrieve query results using pgfdw_get_result(), which uses the asynchronous interface to libpq so that it can check for interrupts every time libpq returns control. Since this might result in a situation where we can no longer be sure that the remote transaction state matches the local transaction state, add a facility to force all levels of the local transaction to abort if we've lost track of the remote state; without this, an apparently-successful commit of the local transaction might fail to commit changes made on the remote side. Also, add a 60-second timeout for queries issue during transaction abort; if that expires, give up and mark the state of the connection as unknown. Drop all such connections when we exit the local transaction. Together, these changes mean that if we're aborting the local toplevel transaction anyway, we can just drop the remote connection in lieu of waiting (possibly for a very long time) for it to complete an abort. This still leaves quite a bit of room for improvement. PQcancel() has no asynchronous interface, so if we get stuck sending the cancel request we'll still hang. Also, PQsetnonblocking() is not used, which means we could block uninterruptibly when sending a query. There might be some other optimizations possible as well. Nonetheless, this allows us to escape a wait for an unresponsive remote server quickly in many more cases than previously. Report by Suraj Kharage. Patch by me and Rafia Sabih. Review and testing by Amit Kapila and Tushar Ahuja. Discussion: http://postgr.es/m/CAF1DzPU8Kx+fMXEbFoP289xtm3bz3t+ZfxhmKavr98Bh-C0TqQ@mail.gmail.com
* Replace over-optimistic Assert in partitioning code with a runtime test.Tom Lane2017-06-04
| | | | | | | | | | | | | | | | get_partition_parent felt that it could simply Assert that systable_getnext found a tuple. This is unlike any other caller of that function, and it's unsafe IMO --- in fact, the reason I noticed it was that the Assert failed. (OK, I was working with known-inconsistent catalog contents, but I wasn't expecting the DB to fall over quite that violently. The behavior in a non-assert-enabled build wouldn't be very nice, either.) Fix it to do what other callers do, namely an actual runtime-test-and-elog. Also, standardize the wording of elog messages that are complaining about unexpected failure of systable_getnext. 90% of them say "could not find tuple for <object>", so make the remainder do likewise. Many of the holdouts were using the phrasing "cache lookup failed", which is outright misleading since no catcache search is involved.
* Fix wording in amvalidate error messagesAlvaro Herrera2017-05-30
| | | | | | | | Remove some gratuituous message differences by making the AM name previously embedded in each message be a %s instead. While at it, get rid of terminology that's unclear and unnecessary in one message. Discussion: https://postgr.es/m/20170523001557.bq2hbq7hxyvyw62q@alvherre.pgsql
* Fix contrib/sepgsql regression tests for partition NOT NULL change.Tom Lane2017-05-21
| | | | | | | Commit 3ec76ff1f changed the partitioning logic to not install a forced NOT NULL constraint on range partitioning columns. This affects the expected output for contrib/sepgsql, because there's no longer LOG entries reporting allowance of such a constraint. Per buildfarm.
* Don't explicitly mark range partitioning columns NOT NULL.Robert Haas2017-05-18
| | | | | | | | | | | | | | | | This seemed like a good idea originally because there's no way to mark a range partition as accepting NULL, but that now seems more like a current limitation than something we want to lock down for all time. For example, there's a proposal to add the notion of a default partition which accepts all rows not otherwise routed, which directly conflicts with the idea that a range-partitioned table should never allow nulls anywhere. So let's change this while we still can, by putting the NOT NULL test into the partition constraint instead of changing the column properties. Amit Langote and Robert Haas, reviewed by Amit Kapila Discussion: http://postgr.es/m/8e2dd63d-c6fb-bb74-3c2b-ed6d63629c9d@lab.ntt.co.jp
* Post-PG 10 beta1 pgperltidy runBruce Momjian2017-05-17
|
* Post-PG 10 beta1 pgindent runBruce Momjian2017-05-17
| | | | perltidy run not included.
* Preventive maintenance in advance of pgindent run.Tom Lane2017-05-16
| | | | | | | | | | Reformat various places in which pgindent will make a mess, and fix a few small violations of coding style that I happened to notice while perusing the diffs from a pgindent dry run. There is one actual bug fix here: the need-to-enlarge-the-buffer code path in icu_convert_case was obviously broken. Perhaps it's unreachable in our usage? Or maybe this is just sadly undertested.
* Suppress indentation from Data::Dumper in regression testsAndrew Dunstan2017-05-14
| | | | | | | | | Ultra-modern versions of the perl Data::Dumper module have apparently changed how they indent output. Instead of trying to keep up we choose to tell it to supporess all indentation in the hstore_plperl regression tests. Backpatch to 9.5 where this feature was introduced.
* Fix race condition leading to hanging logical slot creation.Andres Freund2017-05-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The snapshot assembly during the creation of logical slots relied waiting for transactions in xl_running_xacts to end, by checking for their commit/abort records. Unfortunately, despite locking, it is possible to see an xl_running_xact record listing transactions as ready, that have already WAL-logged an commit/abort record, as the locking just prevents the ProcArray to be adjusted, and the commit record has to be logged first. That lead to either delayed or hanging snapshot creation, because snapbuild.c would wait "forever" to see commit/abort records for some transactions. That hang resolved only if a xl_running_xacts record without any running transactions happened to be logged, far from certain on a busy server. It's impractical to prevent that via more heavyweight locking, the likelihood of deadlocks and significantly increased contention would be too big. Instead change the initial snapshot creation to be solely based on tracking the oldest running transaction via xl_running_xacts->oldestRunningXid - that actually ends up significantly simplifying the code. That has two disadvantages: 1) Because we cannot fully "trust" the contents of xl_running_xacts, we cannot use it to build the initial snapshot. Instead we have to wait twice for all running transactions to finish. 2) Previously a slot, unless the race occurred, could be created when the all transaction perceived as running based on commit/abort records, now we have to wait for the next xl_running_xacts record. To address that, trigger logging new xl_running_xacts record from within snapbuild.c exactly when necessary. Unfortunately snabuild.c's SnapBuild is stored on disk, one of the stupider ideas of a certain Mr Freund, so we can't change it in a minor release. As this is going to be backpatched, we have to hack around a bit to keep on-disk compatibility. A later commit will rejigger that on master. Author: Andres Freund, based on a quite different patch from Petr Jelinek Analyzed-By: Petr Jelinek Reviewed-By: Petr Jelinek Discussion: https://postgr.es/m/f37e975c-908f-858e-707f-058d3b1eb214@2ndquadrant.com Backpatch: 9.4-, where logical decoding has been introduced
* Redesign get_attstatsslot()/free_attstatsslot() for more safety and speed.Tom Lane2017-05-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The mess cleaned up in commit da0759600 is clear evidence that it's a bug hazard to expect the caller of get_attstatsslot()/free_attstatsslot() to provide the correct type OID for the array elements in the slot. Moreover, we weren't even getting any performance benefit from that, since get_attstatsslot() was extracting the real type OID from the array anyway. So we ought to get rid of that requirement; indeed, it would make more sense for get_attstatsslot() to pass back the type OID it found, in case the caller isn't sure what to expect, which is likely in binary- compatible-operator cases. Another problem with the current implementation is that if the stats array element type is pass-by-reference, we incur a palloc/memcpy/pfree cycle for each element. That seemed acceptable when the code was written because we were targeting O(10) array sizes --- but these days, stats arrays are almost always bigger than that, sometimes much bigger. We can save a significant number of cycles by doing one palloc/memcpy/pfree of the whole array. Indeed, in the now-probably-common case where the array is toasted, that happens anyway so this method is basically free. (Note: although the catcache code will inline any out-of-line toasted values, it doesn't decompress them. At the other end of the size range, it doesn't expand short-header datums either. In either case, DatumGetArrayTypeP would have to make a copy. We do end up using an extra array copy step if the element type is pass-by-value and the array length is neither small enough for a short header nor large enough to have suffered compression. But that seems like a very acceptable price for winning in pass-by-ref cases.) Hence, redesign to take these insights into account. While at it, convert to an API in which we fill a struct rather than passing a bunch of pointers to individual output arguments. That will make it less painful if we ever want further expansion of what get_attstatsslot can pass back. It's certainly arguable that this is new development and not something to push post-feature-freeze. However, I view it as primarily bug-proofing and therefore something that's better to have sooner not later. Since we aren't quite at beta phase yet, let's put it in. Discussion: https://postgr.es/m/16364.1494520862@sss.pgh.pa.us
* Rename WAL-related functions and views to use "lsn" not "location".Tom Lane2017-05-11
| | | | | | | | | | | | | | | Per discussion, "location" is a rather vague term that could refer to multiple concepts. "LSN" is an unambiguous term for WAL locations and should be preferred. Some function names, view column names, and function output argument names used "lsn" already, but others used "location", as well as yet other terms such as "wal_position". Since we've already renamed a lot of things in this area from "xlog" to "wal" for v10, we may as well incur a bit more compatibility pain and make these names all consistent. David Rowley, minor additional docs hacking by me Discussion: https://postgr.es/m/CAKJS1f8O0njDKe8ePFQ-LK5-EjwThsDws6ohJ-+c6nWK+oUxtg@mail.gmail.com
* Remove support for password_encryption='off' / 'plain'.Heikki Linnakangas2017-05-08
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Storing passwords in plaintext hasn't been a good idea for a very long time, if ever. Now seems like a good time to finally forbid it, since we're messing with this in PostgreSQL 10 anyway. Remove the CREATE/ALTER USER UNENCRYPTED PASSSWORD 'foo' syntax, since storing passwords unencrypted is no longer supported. ENCRYPTED PASSWORD 'foo' is still accepted, but ENCRYPTED is now just a noise-word, it does the same as just PASSWORD 'foo'. Likewise, remove the --unencrypted option from createuser, but accept --encrypted as a no-op for backward compatibility. AFAICS, --encrypted was a no-op even before this patch, because createuser encrypted the password before sending it to the server even if --encrypted was not specified. It added the ENCRYPTED keyword to the SQL command, but since the password was already in encrypted form, it didn't make any difference. The documentation was not clear on whether that was intended or not, but it's moot now. Also, while password_encryption='on' is still accepted as an alias for 'md5', it is now marked as hidden, so that it is not listed as an accepted value in error hints, for example. That's not directly related to removing 'plain', but it seems better this way. Reviewed by Michael Paquier Discussion: https://www.postgresql.org/message-id/16e9b768-fd78-0b12-cfc1-7b6b7f238fde@iki.fi
* postgres_fdw: Fix join push down with extensionsPeter Eisentraut2017-04-24
| | | | | | | | | | | | | | | | | | | | Objects in an extension are shippable to a foreign server if the extension is part of the foreign server definition's shippable extensions list. But this was not properly considered in some cases when checking whether a join condition can be pushed to a foreign server and the join condition uses an object from a shippable extension. So the join would never be pushed down in those cases. So, the list of extensions needs to be made available in fpinfo of the relation being considered to be pushed down before any expressions are assessed for being shippable. Fix foreign_join_ok() to do that for a join relation. The code to save FDW options in fpinfo is scattered at multiple places. Bring all of that together into functions apply_server_options(), apply_table_options(), and merge_fdw_options(). David Rowley and Ashutosh Bapat, per report from David Rowley
* Fix new warnings from GCC 7Peter Eisentraut2017-04-17
| | | | | This addresses the new warning types -Wformat-truncation -Wformat-overflow that are part of -Wall, via -Wformat, in GCC 7.
* Clean up manipulations of hash indexes' hasho_flag field.Tom Lane2017-04-14
| | | | | | | | | | | | | | | | | | | | | | | | Standardize on testing a hash index page's type by doing (opaque->hasho_flag & LH_PAGE_TYPE) == LH_xxx_PAGE Various places were taking shortcuts like opaque->hasho_flag & LH_BUCKET_PAGE which while not actually wrong, is still bad practice because it encourages use of opaque->hasho_flag & LH_UNUSED_PAGE which *is* wrong (LH_UNUSED_PAGE == 0, so the above is constant false). hash_xlog.c's hash_mask() contained such an incorrect test. This also ensures that we mask out the additional flag bits that hasho_flag has accreted since 9.6. pgstattuple's pgstat_hash_page(), for one, was failing to do that and was thus actively broken. Also fix assorted comments that hadn't been updated to reflect the extended usage of hasho_flag, and fix some macros that were testing just "(hasho_flag & bit)" to use the less dangerous, project-approved form "((hasho_flag & bit) != 0)". Coverity found the bug in hash_mask(); I noted the one in pgstat_hash_page() through code reading.
* Further fix pg_trgm's extraction of trigrams from regular expressions.Tom Lane2017-04-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 9e43e8714 turns out to have been insufficient: not only is it necessary to track tentative parent links while considering a set of arc removals, but it's necessary to track tentative flag additions as well. This is because we always merge arc target states into arc source states; therefore, when considering a merge of the final state with some other, it is the other state that will acquire a new TSTATE_FIN bit. If there's another arc for the same color trigram that would cause merging of that state with the initial state, we failed to recognize the problem. The test cases for the prior commit evidently only exercised situations where a tentative merge with the initial state occurs before one with the final state. If it goes the other way around, we'll happily merge the initial and final states, either producing a broken final graph that would never match anything, or triggering the Assert added by the prior commit. It's tempting to consider switching the merge direction when the merge involves the final state, but I lack the time to analyze that idea in detail. Instead just keep track of the flag changes that would result from proposed merges, in the same way that the prior commit tracked proposed parent links. Along the way, add some more debugging support, because I'm not entirely confident that this is the last bug here. And tweak matters so that the transformed.dot file uses small integers rather than pointer values to identify states; that makes it more readable if you're just eyeballing it rather than fooling with Graphviz. And rename a couple of identically named struct fields to reduce confusion. Per report from Corey Csuhta. Add a test case based on his example. (Note: this case does not trigger the bug under 9.3, apparently because its different measurement of costs causes it to stop merging states before it hits the failure. I spent some time trying to find a variant that would fail in 9.3, without success; but I'm sure such cases exist.) Like the previous patch, back-patch to 9.3 where this code was added. Report: https://postgr.es/m/E2B01A4B-4530-406B-8D17-2F67CF9A16BA@csuhta.com
* Remove useless trailing spaces in queries in C stringsPeter Eisentraut2017-04-13
| | | | Author: Alexander Law <exclusion@gmail.com>
* Fix regexport.c to behave sanely with lookaround constraints.Tom Lane2017-04-13
| | | | | | | | | | | | | | | | | | | | regexport.c thought it could just ignore LACON arcs, but the correct behavior is to treat them as satisfiable while consuming zero input (rather reminiscently of commit 9f1e642d5). Otherwise, the emitted simplified-NFA representation may contain no paths leading from initial to final state, which unsurprisingly confuses pg_trgm, as seen in bug #14623 from Jeff Janes. Since regexport's output representation has no concept of an arc that consumes zero input, recurse internally to find the next normal arc(s) after any LACON transitions. We'd be forced into changing that representation if a LACON could be the last arc reaching the final state, but fortunately the regex library never builds NFAs with such a configuration, so there always is a next normal arc. Back-patch to 9.3 where this logic was introduced. Discussion: https://postgr.es/m/20170413180503.25948.94871@wrigleys.postgresql.org
* Fix pgstattuple's handling of unused hash pages.Robert Haas2017-04-12
| | | | | | | | | | | | | | | | | | Hash indexes can contain both pages which are all-zeroes (i.e. PageIsNew()) and pages which have been initialized but currently aren't used. The latter category can happen either when a page has been reserved but not yet used or when it is used for a time and then freed. pgstattuple was only prepared to deal with the pages that are actually-zeroes, which it called zero_pages. Rename the column to unused_pages (extension version 1.5 is as-yet-unreleased) and make it count both kinds of unused pages. Along the way, slightly tidy up the way we test for pages of various types. Robert Haas and Ashutosh Sharma, reviewed by Amit Kapila Discussion: http://postgr.es/m/CAE9k0PkTtKFB3YndOyQMjwuHx+-FtUP1ynK8E-nHtetoow3NtQ@mail.gmail.com
* Simplify handling of remote-qual pass-forward in postgres_fdw.Tom Lane2017-04-11
| | | | | | | | | | | | | | | | | | | | | | Commit 0bf3ae88a encountered a need to pass the finally chosen remote qual conditions forward from postgresGetForeignPlan to postgresPlanDirectModify. It solved that by sticking them into the plan node's fdw_private list, which in hindsight was a pretty bad idea. In the first place, there's no use for those qual trees either in EXPLAIN or execution; indeed they could never safely be used for any post-planning purposes, because they would not get processed by setrefs.c. So they're just dead weight to carry around in the finished plan tree, plus being an attractive nuisance for somebody who might get the idea that they could be used that way. Secondly, because those qual trees (sometimes) contained RestrictInfos, they created a plan-transmission hazard for parallel query, which is how come we noticed a problem. We dealt with that symptom in commit 28b047875, but really a more straightforward and more efficient fix is to pass the data through in a new field of struct PgFdwRelationInfo. So do it that way. (There's no need to revert 28b047875, as it has sufficient reason to live anyway.) Per fuzz testing by Andreas Seltenreich. Discussion: https://postgr.es/m/87tw5x4vcu.fsf@credativ.de
* Handle restriction clause lists more uniformly in postgres_fdw.Tom Lane2017-04-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Clauses in the lists retained by postgres_fdw during planning were sometimes bare boolean clauses, sometimes RestrictInfos, and sometimes a mixture of the two in the same list. The comment about that situation didn't come close to telling the full truth, either. Aside from being confusing, this had a couple of bad practical consequences: * waste of planning cycles due to inability to cache per-clause selectivity and cost estimates; * sometimes, RestrictInfos would sneak into the fdw_private list of a finished Plan node, causing failures if, for example, we tried to ship the Plan tree to a parallel worker. (It may well be that it's a bug in the parallel-query logic that we would ever try to ship such a plan to a parallel worker, but in any case this deserves to be cleaned up.) To fix, rearrange so that clause lists in PgFdwRelationInfo are always lists of RestrictInfos, and then strip the RestrictInfos at the last minute when making a Plan node. In passing do a bit of refactoring and comment cleanup in postgresGetForeignPlan and foreign_join_ok. Although the messiness here dates back at least to 9.6, there's no evidence that it causes anything worse than wasted planning cycles in 9.6, so no back-patch for now. Per fuzz testing by Andreas Seltenreich. Tom Lane and Ashutosh Bapat Discussion: https://postgr.es/m/87tw5x4vcu.fsf@credativ.de
* Improve castNode notation by introducing list-extraction-specific variants.Tom Lane2017-04-10
| | | | | | | | | | | | | | | | | This extends the castNode() notation introduced by commit 5bcab1114 to provide, in one step, extraction of a list cell's pointer and coercion to a concrete node type. For example, "lfirst_node(Foo, lc)" is the same as "castNode(Foo, lfirst(lc))". Almost half of the uses of castNode that have appeared so far include a list extraction call, so this is pretty widely useful, and it saves a few more keystrokes compared to the old way. As with the previous patch, back-patch the addition of these macros to pg_list.h, so that the notation will be available when back-patching. Patch by me, after an idea of Andrew Gierth's. Discussion: https://postgr.es/m/14197.1491841216@sss.pgh.pa.us
* Fix whitespacePeter Eisentraut2017-04-10
|
* Make sepgsql regression tests robust vs. collation differencesJoe Conway2017-04-09
| | | | | | | | | In commit 25542d77, regression test coverage was added to sepgsql for partitioned tables. Unfortunately it was not robust in the face of collation differences, per the buildfarm. Force "C" collation in order to fix that. Discussion: https://postgr.es/m/flat/623bcaae-112e-ced0-8c22-a84f75ae0c53%40joeconway.com
* Add partitioned table support to sepgsqlJoe Conway2017-04-09
| | | | | | | | | | | | | The new partitioned table capability added a new relkind, namely RELKIND_PARTITIONED_TABLE. Update sepgsql to treat this new relkind exactly the same way it does RELKIND_RELATION. In addition, add regression test coverage for partitioned tables. Issue raised by Stephen Frost and initial patch by Mike Palmiotto. Review by Tom Lane and Robert Haas, and editorializing by me. Discussion: https://postgr.es/m/flat/623bcaae-112e-ced0-8c22-a84f75ae0c53%40joeconway.com
* Optimize joins when the inner relation can be proven unique.Tom Lane2017-04-07
| | | | | | | | | | | | | | | | | | | | | | | If there can certainly be no more than one matching inner row for a given outer row, then the executor can move on to the next outer row as soon as it's found one match; there's no need to continue scanning the inner relation for this outer row. This saves useless scanning in nestloop and hash joins. In merge joins, it offers the opportunity to skip mark/restore processing, because we know we have not advanced past the first possible match for the next outer row. Of course, the devil is in the details: the proof of uniqueness must depend only on joinquals (not otherquals), and if we want to skip mergejoin mark/restore then it must depend only on merge clauses. To avoid adding more planning overhead than absolutely necessary, the present patch errs in the conservative direction: there are cases where inner_unique or skip_mark_restore processing could be used, but it will not do so because it's not sure that the uniqueness proof depended only on "safe" clauses. This could be improved later. David Rowley, reviewed and rather heavily editorialized on by me Discussion: https://postgr.es/m/CAApHDvqF6Sw-TK98bW48TdtFJ+3a7D2mFyZ7++=D-RyPsL76gw@mail.gmail.com
* Reduce the number of pallocs() in BRINAlvaro Herrera2017-04-07
| | | | | | | | | | Instead of allocating memory in brin_deform_tuple and brin_copy_tuple over and over during a scan, allow reuse of previously allocated memory. This is said to make for a measurable performance improvement. Author: Jinyu Zhang, Álvaro Herrera Reviewed by: Tomas Vondra Discussion: https://postgr.es/m/495deb78.4186.1500dacaa63.Coremail.beijing_pg@163.com
* Reset API of clause_selectivity()Simon Riggs2017-04-06
| | | | Discussion: https://postgr.es/m/CAKJS1f9yurJQW9pdnzL+rmOtsp2vOytkpXKGnMFJEO-qz5O5eA@mail.gmail.com
* Silence uninitialized variable compiler warning in sepgsqlJoe Conway2017-04-06
| | | | | | | | | | | | | | | At -Og optimization gcc warns that variable tclass may be used uninitialized when relkind == RELKIND_INDEX. Actually that can't happen due to an early return, but quiet the compiler by initializing tclass to 0. In passing, use uint16_t consistently for the declaration of tclass. Complaint and initial patch by Mike Palmiotto. Editorializing by me. Probably not worth backpatching given that it is cosmetic, so apply to development head only. Discussion: https://postgr.es/m/flat/623bcaae-112e-ced0-8c22-a84f75ae0c53%40joeconway.com
* Silence compiler warning in sepgsqlJoe Conway2017-04-06
| | | | | | | | | | | | <selinux/label.h> includes <stdbool.h>, which creates an incompatible We don't care if <stdbool.h> redefines "true"/"false"; those are close enough. Complaint and initial patch by Mike Palmiotto. Final approach per Tom Lane's suggestion, as discussed on hackers. Backpatching to all supported branches. Discussion: https://postgr.es/m/flat/623bcaae-112e-ced0-8c22-a84f75ae0c53%40joeconway.com
* Fix AclResult vs bool type mix-upPeter Eisentraut2017-04-06
| | | | | Using AclResult as a bool or vice versa works by accident, but it's unusual and possibly confusing style, so write it out more explicitly.
* Collect and use multi-column dependency statsSimon Riggs2017-04-05
| | | | | | | | | | | | | | | | | | Follow on patch in the multi-variate statistics patch series. CREATE STATISTICS s1 WITH (dependencies) ON (a, b) FROM t; ANALYZE; will collect dependency stats on (a, b) and then use the measured dependency in subsequent query planning. Commit 7b504eb282ca2f5104b5c00b4f05a3ef6bb1385b added CREATE STATISTICS with n-distinct coefficients. These are now specified using the mutually exclusive option WITH (ndistinct). Author: Tomas Vondra, David Rowley Reviewed-by: Kyotaro HORIGUCHI, Álvaro Herrera, Dean Rasheed, Robert Haas and many other comments and contributions Discussion: https://postgr.es/m/56f40b20-c464-fad2-ff39-06b668fac47c@2ndquadrant.com