aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* Fix autovacuum for shared relationsAlvaro Herrera2016-05-10
| | | | | | | | | | | | | | | | | | | | The table-skipping logic in autovacuum would fail to consider that multiple workers could be processing the same shared catalog in different databases. This normally wouldn't be a problem: firstly because autovacuum workers not for wraparound would simply ignore tables in which they cannot acquire lock, and secondly because most of the time these tables are small enough that even if multiple for-wraparound workers are stuck in the same catalog, they would be over pretty quickly. But in cases where the catalogs are severely bloated it could become a problem. Backpatch all the way back, because the problem has been there since the beginning. Reported by Ondřej Světlík Discussion: https://www.postgresql.org/message-id/572B63B1.3030603%40flexibee.eu https://www.postgresql.org/message-id/572A1072.5080308%40flexibee.eu
* Stamp 9.6beta1.REL9_6_BETA1Tom Lane2016-05-09
|
* Translation updatesPeter Eisentraut2016-05-09
| | | | | Source-Git-URL: git://git.postgresql.org/git/pgtranslation/messages.git Source-Git-Hash: 17bf3e8564abf600274789fcc90e72532d5e7c05
* Wording quibbles regarding initdb usernameStephen Frost2016-05-08
| | | | | | | | | Use disallowed instead of reserved, cannot instead of can not, and double quotes instead of single quotes. Also add a test to cover the bug which started this discussion. Per discussion with Tom.
* Disallow superuser names starting with 'pg_' in initdbStephen Frost2016-05-08
| | | | | | | As with CREATE ROLE, disallow users from specifying initial superuser names which begin with 'pg_' in initdb. Per discussion with Tom.
* Fix poorly-worded log message.Tom Lane2016-05-08
| | | | Euler Taveira
* In new pg_dump TAP tests, remove trailing "$" from regexps using /m.Tom Lane2016-05-07
| | | | | | | | | | | | | | | | | | | It emerges that some Perl versions before 5.8.9 have a bug with regexps that use the /m flag and contain "$". This is the reason why jacana is still failing on HEAD, and I was able to duplicate the failure on prairiedog's host. There's no real need for "$" in these patterns, since they are already matching through the statement-terminating semicolons (or matching an explicit \n in some cases). So just remove it. Note: the reason jacana hasn't actually reported any failures in the last little while is that the way the pg_dump TAP tests are set up, any failure of this sort results in echoing the entire pg_dump dump output to stderr. Since there were about a hundred such failures, that resulted in a 30MB log file which choked the buildfarm upload script. There is room for improvement here :-(. Per off-list discussion with Andrew and Stephen.
* Clean up after pg_dump test runs.Tom Lane2016-05-06
| | | | | The tmp_check directory needs to be removed by "make clean", and also ignored by .gitignore.
* Fix pg_upgrade to not fail when new-cluster TOAST rules differ from old.Tom Lane2016-05-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch essentially reverts commit 4c6780fd17aa43ed, in favor of a much simpler solution for the case where the new cluster would choose to create a TOAST table but the old cluster doesn't have one: just don't create a TOAST table. The existing code failed in at least two different ways if the situation arose: (1) ALTER TABLE RESET didn't grab an exclusive lock, so that the lock sanity check in create_toast_table failed; (2) pg_upgrade did not provide a pg_type OID for the new toast table, so that the crosscheck in TypeCreate failed. While both these problems were introduced by later patches, they show that the hack being used to cause TOAST table creation is overwhelmingly fragile (and untested). I also note that before the TypeCreate crosscheck was added, the code would have resulted in assigning an indeterminate pg_type OID to the toast table, possibly causing a later OID conflict in that catalog; so that it didn't really work even when committed. If we simply don't create a TOAST table, there will only be a problem if the code tries to store a tuple that's wider than a page, and field compression isn't sufficient to get it under a page. Given that the TOAST creation threshold is intended to be about a quarter of a page, it's very hard to believe that cross-version differences in the do-we-need-a-toast- table heuristic could result in an observable problem. So let's just follow the old version's conclusion about whether a TOAST table is needed. (If we ever do change needs_toast_table() so much that this conclusion doesn't apply, we can devise a solution at that time, and hopefully do it in a less klugy way than 4c6780fd17aa43ed did.) Back-patch to 9.3, like the previous patch. Discussion: <8110.1462291671@sss.pgh.pa.us>
* Disable BLOB test in pg_dump TAP testsStephen Frost2016-05-06
| | | | | | | | | | Buildfarm member jacana appears to have an issue with running this test. It's not entirely clear to me why, but rather than try to fight with it, just disable it for now. None of the other tests try to write out from psql directly as this test does, so it seems likely that the rest of the tests will be fine (as they have been on numerous other systems).
* Mitigate "snapshot too old" performance regression on NUMAKevin Grittner2016-05-06
| | | | | | | | | | | | Limit maintenance of time to xid mapping to once per minute. At least in the tested case this brings performance within 5% of when the feature is off, compared to several times slower without this patch. While there, fix comments and whitespace. Ants Aasma, with cosmetic adjustments suggested by Andres Freund Reviewed by Kevin Grittner and Andres Freund
* Add test_pg_dump to @contrib_excludesStephen Frost2016-05-06
| | | | | | | | | | | | | The test_pg_dump extension doesn't have a C component, so we need to exclude it from the MSVC build system trying to figure out how to build it. Also add a "MODULES" line to the Makefile, as test_extensions has. Might not be necessary, but seems good to keep things consistent. Lastly, remove the 'installcheck' line from test_pg_dump, as that was causing redefinition errors, at least on my box. This also makes test_pg_dump consistent with how commit_ts is set up.
* Correct query in pg_dumpall:dumpRolesStephen Frost2016-05-06
| | | | | | | | We need to use a new branch due to the 9.5 addition of bypassrls when adding in the clause to exclude pg_* roles from being dumped by pg_dumpall. Pointed out by Noah, patch by me.
* Remove MODULES_big from test_pg_dumpStephen Frost2016-05-06
| | | | | | | | | | The Makefile for test_pg_dump shouldn't have a MODULES_big line because there's no actual compiled bit for that extension. Hopefully this will fix the Windows buildfarm members which were complaining. In passing, also add the 'prove_installcheck' bit to the pg_dump and test_pg_dump Makefiles, to get the buildfarm members to actually run those tests.
* Minimal fix for crash bug in quals_match_foreign_key.Robert Haas2016-05-06
| | | | | | | | Discussion is still underway as to whether to revert the entire patch that added this function, but that discussion may not conclude before beta1. So, in the meantime, let's do at least this much. David Rowley
* Limit maximum parallel degree to 1024.Robert Haas2016-05-06
| | | | | | | | | | | | | | This new limit affects both the max_parallel_degree GUC and the parallel_degree reloption. There may some day be a use case for using more than 1024 CPUs for a single query, but that's surely not the case right now. Not only do not very many people have that many CPUs, but the code hasn't been tested at that kind of scale and is very unlikely to perform well, or even work at all, without a lot more work. The issue addressed by commit 06bd458cb812623c3f1fdd55216c4c08b06a8447 is probably just one problem of many. The idea of a more reasonable limit here was suggested by Tom Lane; the value of 1024 was suggested by Amit Kapila.
* Improve pg_upgrade's report about failure to match up old and new tables.Tom Lane2016-05-06
| | | | | | | | | | | | | | | | | | | | Ordinarily, pg_upgrade shouldn't have any difficulty in matching up all the relations it sees in the old and new databases. If it does, however, it just goes belly-up with a pretty unhelpful error message. That seemed fine as long as we expected the case never to occur in the wild, but Alvaro reported that it had been seen in a database whose pg_largeobject table had somehow acquired a TOAST table. That doesn't quite seem like a case that pg_upgrade actually needs to handle, but it would be good if the report were more diagnosable. Hence, extend the logic to print out as much information as we can about the mismatch(es) before we quit. In passing, improve the readability of get_rel_infos()'s data collection query, which had suffered seriously from lets-not-bother-to-update-comments syndrome, and generally was unnecessarily disrespectful to readers. It could be argued that this is a bug fix, but given that we have so few reports, I don't feel a need to back-patch; at least not before this has baked awhile in HEAD.
* Use mul_size when multiplying by the number of parallel workers.Robert Haas2016-05-06
| | | | | | | | | | That way, if the result overflows size_t, you'll get an error instead of undefined behavior, which seems like a plus. This also has the effect of casting the number of workers from int to Size, which is better because it's harder to overflow int than size_t. Dilip Kumar reported this issue and provided a patch upon which this patch is based, but his version did use mul_size.
* Remove various special checks around default rolesStephen Frost2016-05-06
| | | | | | | | | | | | | Default roles really should be like regular roles, for the most part. This removes a number of checks that were trying to make default roles extra special by not allowing them to be used as regular roles. We still prevent users from creating roles in the "pg_" namespace or from altering roles which exist in that namespace via ALTER ROLE, as we can't preserve such changes, but otherwise the roles are very much like regular roles. Based on discussion with Robert and Tom.
* Add TAP tests for pg_dumpStephen Frost2016-05-06
| | | | | | | | | | | | | | | | | | This TAP test suite will create a new cluster, populate it based on the 'create_sql' values in the '%tests' hash, run all of the runs defined in the '%pgdump_runs' hash, and then for each test in the '%tests' hash, compare each run's output the the regular expression defined for the test under the 'like' and 'unlike' functions, as appropriate. While this test suite covers a fair bit of ground (67% of pg_dump.c and quite a bit of the other files in src/bin/pg_dump), there is still quite a bit which remains to be added to provide better code coverage. Still, this is quite a bit better than we had, and has found a few bugs already (note that the CREATE TRANSFORM test is commented out, as it is currently failing). Idea for using the TAP system from Tom, though all of the code is mine.
* Only issue LOCK TABLE commands when necessaryStephen Frost2016-05-06
| | | | | | | | | | | | | | Reviewing the cases where we need to LOCK a given table during a dump, it was pointed out by Tom that we really don't need to LOCK a table if we are only looking to dump the ACL for it, or certain other components. After reviewing the queries run for all of the component pieces, a list of components were determined to not require LOCK'ing of the table. This implements a check to avoid LOCK'ing those tables. Initial complaint from Rushabh Lathia, discussed with Robert and Tom, the patch is mine.
* pg_dump performance and other fixesStephen Frost2016-05-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Do not try to dump objects which do not have ACLs when only ACLs are being requested. This results in a significant performance improvement as we can avoid querying for further information on these objects when we don't need to. When limiting the components to dump for an extension, consider what components have been requested. Initially, we incorrectly hard-coded the components of the extension objects to dump, which would mean that we wouldn't dump some components even with they were asked for and in other cases we would dump components which weren't requested. Correct defaultACLs to use 'dump_contains' instead of 'dump'. The defaultACL is considered a member of the namespace and should be dumped based on the same set of components that the other objects in the schema are, not based on what we're dumping for the namespace itself (which might not include ACLs, if the namespace has just the default or initial ACL). Use DUMP_COMPONENT_ACL for from-initdb objects, to allow users to change their ACLs, should they wish to. This just extends what we are doing for the pg_catalog namespace to objects which are not members of namespaces. Due to column ACLs being treated a bit differently from other ACLs (they are actually reset to NULL when all privileges are revoked), adjust the query which gathers column-level ACLs to consider all of the ACL-relevant columns.
* Correct pg_dump WHERE clause for functions/aggregatesStephen Frost2016-05-06
| | | | | | | | | The query to grab the function/aggregate information is now joining to pg_init_privs, so we can simplify (and correct) the WHERE clause used to determine if a given function's ACL has changed from the initial ACL on the function. Bug found by Noah, patch by me.
* Fix possible read past end of string in to_timestamp().Tom Lane2016-05-06
| | | | | | | | | | | | | | | | | | | | | | | to_timestamp() handles the TH/th format codes by advancing over two input characters, whatever those are. It failed to notice whether there were two characters available to be skipped, making it possible to advance the pointer past the end of the input string and keep on parsing. A similar risk existed in the handling of "Y,YYY" format: it would advance over three characters after the "," whether or not three characters were available. In principle this might be exploitable to disclose contents of server memory. But the security team concluded that it would be very hard to use that way, because the parsing loop would stop upon hitting any zero byte, and TH/th format codes can't be consecutive --- they have to follow some other format code, which would have to match whatever data is there. So it seems impractical to examine memory very much beyond the end of the input string via this bug; and the input string will always be in local memory not in disk buffers, making it unlikely that anything very interesting is close to it in a predictable way. So this doesn't quite rise to the level of needing a CVE. Thanks to Wolf Roediger for reporting this bug.
* Fix pgbench's parsing of double values to notice trailing garbage.Tom Lane2016-05-06
| | | | | Noted by Fabien Coelho, though this isn't exactly his proposed patch. (The technique used here is borrowed from the zic sources.)
* Improve handling of numeric-valued variables in pgbench.Tom Lane2016-05-06
| | | | | | | | | | | | | | | | | | | | | The previous coding always stored variable values as strings, doing conversion on-the-fly when a numeric value was needed or a number was to be assigned. This was a bit inefficient and risked loss of precision for floating-point values. The precision aspect had been hacked around by printing doubles in "%.18e" format, which is ugly and has machine-dependent results. Instead, arrange to preserve an assigned numeric value in the original binary numeric format, converting to string only when and if needed. When we do need to convert a double to string, convert in "%g" format with DBL_DIG precision, which is the standard way to do it and produces the least surprising results in most cases. The implementation supports storing both a string value and a numeric value for any one variable, with lazy conversion between them. I also arranged for lazy re-sorting of the variable array when new variables are added. That was mainly to allow a clean refactoring of putVariable() into two levels of subroutine, but it may allow us to save a few sorts. Discussion: <9188.1462475559@sss.pgh.pa.us>
* Fix hash index vs "snapshot too old" problemmsKevin Grittner2016-05-06
| | | | | | | | | | | | | | | | Hash indexes are not WAL-logged, and so do not maintain the LSN of index pages. Since the "snapshot too old" feature counts on detecting error conditions using the LSN of a table and all indexes on it, this makes it impossible to safely do early vacuuming on any table with a hash index, so add this to the tests for whether the xid used to vacuum a table can be adjusted based on old_snapshot_threshold. While at it, add a paragraph to the docs for old_snapshot_threshold which specifically mentions this and other aspects of the feature which may otherwise surprise users. Problem reported and patch reviewed by Amit Kapila
* Fix psql's \ev and \sv commands so that they handle view reloptions.Dean Rasheed2016-05-06
| | | | | | | | | | | Commit 8eb6407aaeb6cbd972839e356b436bb698f51cff added support for editing and showing view definitions, but neglected to account for view options such as security_barrier and WITH CHECK OPTION which are not returned by pg_get_viewdef() and so need special handling. Author: Dean Rasheed Reviewed-by: Peter Eisentraut Discussion: http://www.postgresql.org/message-id/CAEZATCWZjCgKRyM-agE0p8ax15j9uyQoF=qew7D2xB6cF76T8A@mail.gmail.com
* Move and rename fmtReloptionsArray().Dean Rasheed2016-05-06
| | | | | | | | | | | | Move fmtReloptionsArray() from pg_dump.c to string_utils.c so that it is available to other frontend code. In particular psql's \ev and \sv commands need it to handle view reloptions. Also rename the function to appendReloptionsArray(), which is a more accurate description of what it does. Author: Dean Rasheed Reviewed-by: Peter Eisentraut Discussion: http://www.postgresql.org/message-id/CAEZATCWZjCgKRyM-agE0p8ax15j9uyQoF=qew7D2xB6cF76T8A@mail.gmail.com
* Update time zone data files to tzdata release 2016d.Tom Lane2016-05-05
| | | | | | | DST law changes in Russia (Magadan, Tomsk regions) and Venezuela. Historical corrections for Russia. There are new zone names Europe/Kirov and Asia/Tomsk reflecting the fact that these regions now have different time zone histories from adjacent regions.
* Rename tsvector delete() to ts_delete(), and filter() to ts_filter().Tom Lane2016-05-05
| | | | | | | | | The similarity of the original names to SQL keywords seems like a bad idea. Rename them before we're stuck with 'em forever. In passing, minor code and docs cleanup. Discussion: <4875.1462210058@sss.pgh.pa.us>
* Rename pgbench min/max to least/greatest, and fix handling of double args.Tom Lane2016-05-05
| | | | | | | | | | | | These functions behave like the backend's least/greatest functions, not like min/max, so the originally-chosen names invite confusion. Per discussion, rename to least/greatest. I also took it upon myself to make them return double if any input is double. The previous behavior of silently coercing all inputs to int surely does not meet the principle of least astonishment. Copy-edit some of the other new functions' documentation, too.
* Fix corner-case loss of precision in numeric pow() calculationDean Rasheed2016-05-05
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 7d9a4737c268f61fb8800957631f12d3f13be218 greatly improved the accuracy of the numeric transcendental functions, however it failed to consider the case where the result from pow() is close to the overflow threshold, for example 0.12 ^ -2345.6. For such inputs, where the result has more than 2000 digits before the decimal point, the decimal result weight estimate was being clamped to 2000, leading to a loss of precision in the final calculation. Fix this by replacing the clamping code with an overflow test that aborts the calculation early if the final result is sure to overflow, based on the overflow limit in exp_var(). This provides the same protection against integer overflow in the subsequent result scale computation as the original clamping code, but it also ensures that precision is never lost and saves compute cycles in cases that are sure to overflow. The new early overflow test works with the initial low-precision result (expected to be accurate to around 8 significant digits) and includes a small fuzz factor to ensure that it doesn't kick in for values that would not overflow exp_var(), so the overall overflow threshold of pow() is unchanged and consistent for all inputs with non-integer exponents. Author: Dean Rasheed Reviewed-by: Tom Lane Discussion: http://www.postgresql.org/message-id/CAEZATCUj3U-cQj0jjoia=qgs0SjE3auroxh8swvNKvZWUqegrg@mail.gmail.com See-also: http://www.postgresql.org/message-id/CAEZATCV7w+8iB=07dJ8Q0zihXQT1semcQuTeK+4_rogC_zq5Hw@mail.gmail.com
* Revert timeline following in replication slotsAlvaro Herrera2016-05-04
| | | | | | | | | This reverts commits f07d18b6e94d, 82c83b337202, 3a3b309041b0, and 24c5f1a103ce. This feature has shown enough immaturity that it was deemed better to rip it out before rushing some more fixes at the last minute. There are discussions on larger changes in this area for the next release.
* Fix crash of filter(tsvector)Teodor Sigaev2016-05-04
| | | | | | | Variable storing a position of lexeme, had a wrong type: char, it's obviously not enough to store 2^14 possible positions. Stas Kelvich
* Fix transient mdsync() errors of truncated relations due to 72a98a6395.Andres Freund2016-05-04
| | | | | | | | | | | | | | | | | | | | | Unfortunately the segment size checks from 72a98a6395 had the negative side-effect of breaking a corner case in mdsync(): When processing a fsync request for a truncated away segment mdsync() could fail with "could not fsync file" (if previous segment < RELSEG_SIZE) because _mdfd_getseg() now wouldn't return the relevant segment anymore. The cleanest fix seems to be to allow the caller of _mdfd_getseg() to specify whether checks for RELSEG_SIZE are performed. To allow doing so, change the ExtensionBehavior enum into a bitmask. Besides allowing for the addition of EXTENSION_DONT_CHECK_SIZE, this makes for a nicer implementation of EXTENSION_REALLY_RETURN_NULL. Besides mdsync() the only callsite that should change behaviour due to this is mdprefetch() which now doesn't create segments anymore, even in recovery. Given the uses of mdprefetch() that seems better. Reported-By: Thom Brown Discussion: CAA-aLv72QazLvPdKZYpVn4a_Eh+i4_cxuB03k+iCuZM_xjc+6Q@mail.gmail.com
* Fix more things to be parallel-safe.Robert Haas2016-05-03
| | | | | | | | | | | Conversion functions were previously marked as parallel-unsafe, since that is the default, but in fact they are safe. Parallel-safe functions defined in pg_proc.h and redefined in system_views.sql were ending up as parallel-unsafe because the redeclarations were not marked PARALLEL SAFE. While editing system_views.sql, mark ts_debug() parallel safe also. Andreas Karlsson
* Tweak a few more things in preparation for upcoming pgindent run.Robert Haas2016-05-03
| | | | | | | | These adjustments adjust code and comments in minor ways to prevent pgindent from mangling them. Among other things, I tried to avoid situations where pgindent would emit "a +b" instead of "a + b", and I tried to avoid having it break up inline comments across multiple lines.
* Note that max_worker_processes requires restart.Robert Haas2016-05-03
| | | | | | Since this is a minor issue, no back-patch. Julien Rouhaud
* Fix thinko in commentAlvaro Herrera2016-05-02
| | | | Pointed out by Andres Freund
* Fix code comments regarding logical decodingAlvaro Herrera2016-05-02
| | | | | | | | | | | | | | Back in 3b02ea4f0780 I added some comments in various places to explain how logical decoding and other things worked. Not all of the changes were welcome, because they were misleading or wrong. This changes them a little bit to make them more accurate. Some other comments are also changed to be more accurate. Also, fix a bunch of typos. Author: Álvaro Herrera, Craig Ringer Andres Freund reviewed some parts of this.
* Fix parallel safety markings for pg_start_backup.Robert Haas2016-05-02
| | | | | | | | | | | Commit 7117685461af50f50c03f43e6a622284c8d54694 made pg_start_backup parallel-restricted rather than parallel-safe, because it now relies on backend-private state that won't be synchronized with the parallel worker. However, it didn't update pg_proc.h. Separately, Andreas Karlsson observed that system_views.sql neglected to reiterate the parallel-safety markings whe redefining various functions, including this one; so add a PARALLEL RESTRICTED declaration there to match the new value in pg_proc.h.
* Again update typedefs.list file in preparation for pgindent runRobert Haas2016-05-02
| | | | | | | This time, use the buildfarm-supplied contents for this file, instead of trying to update it by eyeballing the pgindent output. Per discussion with Tom and Bruce.
* Add a --non-master-only option to git_changelog.Tom Lane2016-05-01
| | | | | | | This has the inverse effect of --master-only. It's needed to help find cases where a commit should not be described in major release notes because it was back-patched into older branches, though not at the same time as the HEAD commit.
* Small improvements to OPTIMIZER_DEBUG code.Tom Lane2016-04-30
| | | | | | | | | | | | | | | Now that Paths have their own rows field, print that rather than the parent relation's rowcount. Show the relid sets associated with Paths using table names rather than numbers; since this code is able to print simple Var references using table names, it seems a bit silly that print_relids can't. Print the cheapest_parameterized_paths list for a RelOptInfo, and include information about a parameterized path's required_outer rels. Noted while trying to use this feature to debug Alexander Kirkouski's recent bug report.
* Fix planner crash from pfree'ing a partial path that a GatherPath uses.Tom Lane2016-04-30
| | | | | | | | | | | | | | | | | | | | We mustn't run generate_gather_paths() during add_paths_to_joinrel(), because that function can be invoked multiple times for the same target joinrel. Not only is it wasteful to build GatherPaths repeatedly, but a later add_partial_path() could delete the partial path that a previously created GatherPath depends on. Instead establish the convention that we do generate_gather_paths() for a rel only just before set_cheapest(). The code was accidentally not broken for baserels, because as of today there never is more than one partial path for a baserel. But that assumption obviously has a pretty short half-life, so move the generate_gather_paths() calls for those cases as well. Also add some generic comments explaining how and why this all works. Per fuzz testing by Andreas Seltenreich. Report: <871t5pgwdt.fsf@credativ.de>
* Remove warning about num_sync being too large in synchronous_standby_names.Tom Lane2016-04-30
| | | | | | | | | | If we're not going to reject such setups entirely, throwing a WARNING in check_synchronous_standby_names() is unhelpful, because it will cause the warning to be logged again every time the postmaster receives SIGHUP. Per discussion, just remove the warning. In passing, improve the documentation for synchronous_commit, which had not gotten the word that now there can be more than one synchronous standby.
* Fix mishandling of equivalence-class tests in parameterized plans.Tom Lane2016-04-29
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Given a three-or-more-way equivalence class, such as X.Y = Y.Y = Z.Z, it was possible for the planner to omit one of the quals needed to enforce that all members of the equivalence class are actually equal. This only happened in the case of a parameterized join node for two of the relations, that is a plan tree like Nested Loop -> Scan X -> Nested Loop -> Scan Y -> Scan Z Filter: Z.Z = X.X The eclass machinery normally expects to apply X.X = Y.Y when those two relations are joined, but in this shape of plan tree they aren't joined until the top node --- and, if the lower nested loop is marked as parameterized by X, the top node will assume that the relevant eclass condition(s) got pushed down into the lower node. On the other hand, the scan of Z assumes that it's only responsible for constraining Z.Z to match any one of the other eclass members. So one or another of the required quals sometimes fell between the cracks, depending on whether consideration of the eclass in get_joinrel_parampathinfo() for the lower nested loop chanced to generate X.X = Y.Y or X.X = Z.Z as the appropriate constraint there. If it generated the latter, it'd erroneously suppose that the Z scan would take care of matters. To fix, force X.X = Y.Y to be generated and applied at that join node when this case occurs. This is *extremely* hard to hit in practice, because various planner behaviors conspire to mask the problem; starting with the fact that the planner doesn't really like to generate a parameterized plan of the above shape. (It might have been impossible to hit it before we tweaked things to allow this plan shape for star-schema cases.) Many thanks to Alexander Kirkouski for submitting a reproducible test case. The bug can be demonstrated in all branches back to 9.2 where parameterized paths were introduced, so back-patch that far.
* Add a few entries to the tail of time mapping, to see old values.Kevin Grittner2016-04-29
| | | | | | | | | | | | | | | | | | Without a few entries beyond old_snapshot_threshold, the lookup would often fail, resulting in the more aggressive pruning or vacuum being skipped often enough to matter. This was very clearly shown by a python test script posted by Ants Aasma, and was likely a factor in an earlier but somewhat less clear-cut test case posted by Jeff Janes. This patch makes no change to the logic, per se -- it just makes the array of mapping entries big enough to make lookup misses based on timing much less likely. An occasional miss is still possible if a thread stalls for more than 10 minutes, but that does not create any problem with correctness of behavior. Besides, if things are so busy that a thread is stalling for more than 10 minutes, it is probably OK to skip the more aggressive cleanup at that particular point in time.
* Fix comment whitespace in VS2105 patchAndrew Dunstan2016-04-29
| | | | per gripe from Michael Paquier.