aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* 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.
* Fix typoMagnus Hagander2016-04-29
| | | | Author: Thomas Munro
* Fix typo in VS2015 patchAndrew Dunstan2016-04-29
| | | | reported by Christian Ullrich
* Support building with Visual Studio 2015Andrew Dunstan2016-04-29
| | | | | | | | | | | Adjust the way we detect the locale. As a result the minumum Windows version supported by VS2015 and later is Windows Vista. Add some tweaks to remove new compiler warnings. Remove documentation references to the now obsolete msysGit. Michael Paquier, somewhat edited by me, reviewed by Christian Ullrich. Backpatch to 9.5
* Remember asking for feedback during walsender shutdown.Andres Freund2016-04-28
| | | | | | | | | | | | | | | | | | | | | | | | | Since 5a991ef8 we're explicitly asking for feedback from the receiving side when shutting down walsender, if there's not yet replicated data. Unfortunately we didn't remember (i.e. set waiting_for_ping_response to true) having asked for feedback, leading to scenarios in which replies were requested at a high frequency. I can't reproduce this problem on my laptop, I think that's because the problem requires a significant TCP window to manifest due to the !pq_is_send_pending() condition. But since this clearly is a bug, let's fix it. There's quite possibly more wrong than just this though. While fiddling with WalSndDone(), I rewrote a hard to understand comment about looking at the flush vs. the write position. Reported-By: Nick Cleaton, Magnus Hagander Author: Nick Cleaton Discussion: CAFgz3kus=rC_avEgBV=+hRK5HYJ8vXskJRh8yEAbahJGTzF2VQ@mail.gmail.com CABUevExsjROqDcD0A2rnJ6HK6FuKGyewJr3PL12pw85BHFGS2Q@mail.gmail.com Backpatch: 9.4, were 5a991ef8 introduced the use of feedback messages during shutdown.
* Adjust DatumGetBool macro, this time for sure.Tom Lane2016-04-28
| | | | | | | | | | | | | | | Commit 23a41573c attempted to fix the DatumGetBool macro to ignore bits in a Datum that are to the left of the actual bool value. But it did that by casting the Datum to bool; and on compilers that use C99 semantics for bool, that ends up being a whole-word test, not a 1-byte test. This seems to be the true explanation for contrib/seg failing in VS2015. To fix, use GET_1_BYTE() explicitly. I think in the previous patch, I'd had some idea of not having to commit to bool being exactly 1 byte wide, but regardless of what the compiler's bool is, boolean columns and Datums are certainly 1 byte wide. The previous fix was (eventually) back-patched into all active versions, so do likewise with this one.
* Prevent to use magic constantsTeodor Sigaev2016-04-28
| | | | | | | Use macroses for definition amstrategies/amsupport fields instead of hardcoded values. Author: Nikolay Shaplov with addition for contrib/bloom
* Prevent multiple cleanup process for pending list in GIN.Teodor Sigaev2016-04-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, ginInsertCleanup could exit early if it detects that someone else is cleaning up the pending list, without waiting for that someone else to finish the job. But in this case vacuum could miss tuples to be deleted. Cleanup process now locks metapage with a help of heavyweight LockPage(ExclusiveLock), and it guarantees that there is no another cleanup process at the same time. Lock is taken differently depending on caller of cleanup process: any vacuums and gin_clean_pending_list() will be blocked until lock becomes available, ordinary insert uses conditional lock to prevent indefinite waiting on lock. Insert into pending list doesn't use this lock, so insertion isn't blocked. Also, patch adds stopping of cleanup process when at-start-cleanup-tail is reached in order to prevent infinite cleanup in case of massive insertion. But it will stop only for automatic maintenance tasks like autovacuum. Patch introduces choice of limit of memory to use: autovacuum_work_mem, maintenance_work_mem or work_mem depending on call path. Patch for previous releases should be reworked due to changes between 9.6 and previous ones in this area. Discover and diagnostics by Jeff Janes and Tomas Vondra Patch by me with some ideas of Jeff Janes
* Use memmove() not memcpy() to slide some pointers down.Tom Lane2016-04-27
| | | | | | | | | The previous coding here was formally undefined, though it seems to accidentally work on most platforms in the buildfarm. Caught by some OpenBSD platforms in which libc contains an assertion check for overlapping areas passed to memcpy(). Thomas Munro
* Clean up parsing of synchronous_standby_names GUC variable.Tom Lane2016-04-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 989be0810dffd08b added a flex/bison lexer/parser to interpret synchronous_standby_names. It was done in a pretty crufty way, though, making assorted end-use sites responsible for calling the parser at the right times. That was not only vulnerable to errors of omission, but made it possible that lexer/parser errors occur at very undesirable times, and created memory leakages even if there was no error. Instead, perform the parsing once during check_synchronous_standby_names and let guc.c manage the resulting data. To do that, we have to flatten the parsed representation into a single hunk of malloc'd memory, but that is not very hard. While at it, work a little harder on making useful error reports for parsing problems; the previous code felt that "synchronous_standby_names parser returned 1" was an appropriate user-facing error message. (To be fair, it did also log a syntax error message, but separately from the GUC problem report, which is at best confusing.) It had some outright bugs in the face of invalid input, too. I (tgl) also concluded that we need to restrict unquoted names in synchronous_standby_names to be just SQL identifiers. The previous coding would accept darn near anything, which (1) makes the quoting convention both nearly-unnecessary and formally ambiguous, (2) makes it very hard to understand what is a syntax error and what is a creative interpretation of the input as a standby name, and (3) makes it impossible to further extend the syntax in future without a compatibility break. I presume that we're intending future extensions of the syntax, else this parsing infrastructure is massive overkill, so (3) is an important objection. Since we've taken a compatibility hit for non-identifier names with this change anyway, we might as well lock things down now and insist that users use double quotes for standby names that aren't identifiers. Kyotaro Horiguchi and Tom Lane
* Fix wrong word.Robert Haas2016-04-27
| | | | | | Commit a31212b429cd3397fb3147b1a584ae33224454a6 was a little too hasty. Per report from Tom Lane.
* Change postgresql.conf.sample to say that fsync=off will corrupt data.Robert Haas2016-04-27
| | | | | | | Discussion: 24748.1461764666@sss.pgh.pa.us Per a suggestion from Craig Ringer. This wording from Tom Lane, following discussion.
* Tighten up sanity checks for parallel aggregate in execQual.c.Robert Haas2016-04-27
| | | | David Rowley
* Remove inadvertently commited vim swapfile.Robert Haas2016-04-27
| | | | If you were wondering what editor I use, now you know.
* Update typedefs.list file in preparation for pgindent runRobert Haas2016-04-27
| | | | | | In addition to adding new typedefs, I also re-sorted the file so that various entries add piecemeal, mostly or entirely by me, were alphabetized the same way as other entries in the file.
* Clean up a few parallelism-related things that pgindent wants to mangle.Robert Haas2016-04-27
| | | | | | | | | | | | | | In nodeFuncs.c, pgindent wants to introduce spurious indentation into the definitions of planstate_tree_walker and planstate_walk_subplans. Fix that by spreading the definition out across several lines, similar to what is already done for other walker functions in that file. In execParallel.c, in the definition of SharedExecutorInstrumentation, pgindent wants to insert more whitespace between the type name and the member name. That causes it to mangle comments later on the line. Fix by moving the comments out of line. Now that we have a bit more room, add some more details that may be useful to the next person reading this code.
* Remove mergeHyperLogLog.Robert Haas2016-04-27
| | | | | | | | | | It's buggy. If somebody needs this later, they'll need to put back a non-buggy vesion of it. Discussion: CAM3SWZT-i6R9JU5YXa8MJUou2_r3LfGJZpQ9tYa1BYxfkj0=cQ@mail.gmail.com Discussion: CAM3SWZRUOLsYoTT83QgdUy9D8ehYWm_nvbrrfcOOzikiRfFY7g@mail.gmail.com Peter Geoghegan
* Fix EXPLAIN VERBOSE output for parallel aggregate.Robert Haas2016-04-27
| | | | | | | | | | | | The way that PartialAggregate and FinalizeAggregate plan nodes were displaying output columns before was bogus. Now, FinalizeAggregate produces the same outputs as an Aggregate would have produced, while PartialAggregate produces each of those outputs prefixed by the word PARTIAL. Discussion: 12585.1460737650@sss.pgh.pa.us Patch by me, reviewed by David Rowley.
* Don't open formally non-existent segments in _mdfd_getseg().Andres Freund2016-04-26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Before this commit _mdfd_getseg(), in contrast to mdnblocks(), did not verify whether all segments leading up to the to-be-opened one, were RELSEG_SIZE sized. That is e.g. not the case after truncating a relation, because later segments just get truncated to zero length, not removed. Once a "non-existent" segment has been opened in a session, mdnblocks() will return wrong results, causing errors like "could not read block %u in file" when accessing blocks. Closing the session, or the later arrival of relevant invalidation messages, would "fix" the problem. That, so far, was mostly harmless, because most segment accesses are only done after an mdnblocks() call. But since 428b1d6b29ca we try to open segments that might have been deleted, to trigger kernel writeback from a backend's queue of recent writes. To fix check segment sizes in _mdfd_getseg() when opening previously unopened segments. In practice this shouldn't imply a lot of additional lseek() calls, because mdnblocks() will most of the time already have opened all relevant segments. This commit also fixes a second problem, namely that _mdfd_getseg( EXTENSION_RETURN_NULL) extends files during recovery, which is not desirable for the mdwriteback() case. Add EXTENSION_REALLY_RETURN_NULL, which does not behave that way, and use it. Reported-By: Thom Brown Author: Andres Freund, Abhijit Menon-Sen Reviewd-By: Robert Haas, Fabien Coehlo Discussion: CAA-aLv6Dp_ZsV-44QA-2zgkqWKQq=GedBX2dRSrWpxqovXK=Pg@mail.gmail.com Fixes: 428b1d6b29ca599c5700d4bc4f4ce4c5880369bf
* Emit invalidations to standby for transactions without xid.Andres Freund2016-04-26
| | | | | | | | | | | | | | | | | | | | | | | | | | | So far, when a transaction with pending invalidations, but without an assigned xid, committed, we simply ignored those invalidation messages. That's problematic, because those are actually sent for a reason. Known symptoms of this include that existing sessions on a hot-standby replica sometimes fail to notice new concurrently built indexes and visibility map updates. The solution is to WAL log such invalidations in transactions without an xid. We considered to alternatively force-assign an xid, but that'd be problematic for vacuum, which might be run in systems with few xids. Important: This adds a new WAL record, but as the patch has to be back-patched, we can't bump the WAL page magic. This means that standbys have to be updated before primaries; otherwise "PANIC: standby_redo: unknown op code 32" errors can be encountered. XXX: Reported-By: Васильев Дмитрий, Masahiko Sawada Discussion: CAB-SwXY6oH=9twBkXJtgR4UC1NqT-vpYAtxCseME62ADwyK5OA@mail.gmail.com CAD21AoDpZ6Xjg=gFrGPnSn4oTRRcwK1EBrWCq9OqOHuAcMMC=w@mail.gmail.com
* Fix pg_get_functiondef to dump parallel-safety markings.Robert Haas2016-04-26
| | | | Ashutosh Sharma
* Impose a full barrier in generic-xlc.h atomics functions.Noah Misch2016-04-26
| | | | | | | | | | | | pg_atomic_compare_exchange_*_impl() were providing only the semantics of an acquire barrier. Buildfarm members hornet and mandrill revealed this deficit beginning with commit 008608b9d51061b1f598c197477b3dc7be9c4a64. While we have no report of symptoms in 9.5, we can't rule out the possibility of certain compilers, hardware, or extension code relying on these functions' specified barrier semantics. Back-patch to 9.5, where commit b64d92f1a5602c55ee8b27a7ac474f03b7aee340 introduced atomics. Reviewed by Andres Freund.
* pg_dump: Message style improvementsPeter Eisentraut2016-04-26
| | | | forgotten in b6dacc173b6830c515d970698cead9a85663c553
* Add a --brief option to git_changelog.Tom Lane2016-04-26
| | | | | | | | | In commit c0b050192, Andres introduced the idea of including one-line commit references in our major release notes. Teach git_changelog to emit a (lightly adapted) version of that format, so that we don't have to laboriously add it to the notes after the fact. The default output isn't changed, since I anticipate still using that for minor release notes.
* Fix order of shutdown cleanup operations in PostgresNode.pm.Tom Lane2016-04-26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, database clusters created by a TAP test were shut down by DESTROY methods attached to the PostgresNode objects representing them. The trouble with that is that if the objects survive into the final global destruction phase (which they do), Perl executes the DESTROY methods in an unspecified order. Thus, the order of shutdown of multiple clusters was indeterminate, which might lead to not-very-reproducible errors getting logged (eg from a slave whose master might or might not get killed first). Worse, the File::Temp objects representing the temporary PGDATA directories might get destroyed before the PostgresNode objects, resulting in attempts to delete PGDATA directories that still have live servers in them. On Windows, this would lead to directory deletion failures; on Unix, it usually had no effects worse than erratic "could not open temporary statistics file "pg_stat/global.tmp": No such file or directory" log messages. While none of this would affect the reported result of the TAP test, which is already determined, it could be very confusing when one is trying to understand from the logs what went wrong with a failed test. To fix, do the postmaster shutdowns in an END block rather than at object destruction time. The END block will execute at a well-defined (and reasonable) time during script termination, and it will stop the postmasters in order of PostgresNode object creation. (Perhaps we should change that to be reverse order of creation, but the main point here is that we now have control which we did not before.) Use "pg_ctl stop", not an asynchronous kill(SIGQUIT), so that we wait for the postmasters to shut down before proceeding with directory deletion. Deletion of temporary directories still happens in an unspecified order during global destruction, but I can see no reason to care about that once the postmasters are stopped.
* Yet more portability hacking for degree-based trig functions.Tom Lane2016-04-26
| | | | | | | | | | | | | The true explanation for Peter Eisentraut's report of inexact asind results seems to be that (a) he's compiling into x87 instruction set, which uses wider-than-double float registers, plus (b) the library function asin() on his platform returns a result that is wider than double and is not rounded to double width. To fix, we have to force the function's result to be rounded comparably to what happened to the scaling constant asin_0_5. Experimentation suggests that storing it into a volatile local variable is the least ugly way of making that happen. Although only asin() is known to exhibit an observable inexact result, we'd better do this in all the places where we're hoping to get an exact result by scaling.
* Enable parallel query by default.Robert Haas2016-04-26
| | | | | | | | | | Change max_parallel_degree default from 0 to 2. It is possible that this is not a good idea, or that we should go with 1 worker rather than 2, but we won't find out without trying it. Along the way, reword the documentation for max_parallel_degree a little bit to hopefully make it more clear. Discussion: 20160420174631.3qjjhpwsvvx5bau5@alap3.anarazel.de
* Fix typo in commentMagnus Hagander2016-04-26
| | | | Author: Daniel Gustafsson
* pg_dump: Message style improvementsPeter Eisentraut2016-04-25
|
* Fix C comment typo and redundant testKevin Grittner2016-04-25
|
* New method for preventing compile-time calculation of degree constants.Tom Lane2016-04-25
| | | | | | | | | | | | | Commit 65abaab547a5758b tried to prevent the scaling constants used in the degree-based trig functions from being precomputed at compile time, because some compilers do that with functions that don't yield results identical-to-the-last-bit to what you get at runtime. A report from Peter Eisentraut suggests that some recent compilers are smart enough to see through that trick, though. Instead, let's put the inputs to these calculations into non-const global variables, which should be a more reliable way of convincing the compiler that it can't assume that they are compile-time constants. (If we really get desperate, we could mark these variables "volatile", but I do not believe we should have to.)
* Try harder to detect a port conflict in PostgresNode.pm.Tom Lane2016-04-25
| | | | | | | | | | | Commit fab84c7787f25756 tried to get away without doing an actual bind(), but buildfarm results show that that doesn't get the job done. So we must really bind to the target port --- and at least on my Linux box, we need a listen() as well, or conflicts won't be detected. We rely on SO_REUSEADDR to prevent problems from starting a postmaster on the socket immediately after we've bound to it in the test code. (There may be platforms where that doesn't work too well. But fortunately, we only really care whether this works on Windows, and there the default behavior should be OK.)
* Update GETTEXT_FILES after config and controldata refactoringPeter Eisentraut2016-04-24
|
* Improve PostgresNode.pm's logic for detecting already-in-use ports.Tom Lane2016-04-24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Buildfarm members bowerbird and jacana have shown intermittent "could not bind IPv4 socket" failures in the BinInstallCheck stage since mid-December, shortly after commits 1caef31d9e550408 and 9821492ee417a591 changed the logic for selecting which port to use in temporary installations. One plausible explanation is that we are randomly selecting ports that are already in use for some non-Postgres purpose. Although the code tried to defend against already-in-use ports, it used pg_isready to probe the port which is quite unhelpful: if some non-Postgres server responds at the given address, pg_isready will generally say "no response", leading to exactly the wrong conclusion about whether the port is free. Instead, let's use a simple TCP connect() call to see if anything answers without making assumptions about what it is. Note that this means there's no direct check for a conflicting Unix socket, but that should be okay because there should be no other Unix sockets in use in the temporary socket directory created for a test run. This is only a partial solution for the TCP case, since if the port number is in use for an outgoing connection rather than a listening socket, we'll fail to detect that. We could try to bind() to the proposed port as a means of detecting that case, but that would introduce its own failure modes, since the system might consider the address to remain reserved for some period of time after we drop the bound socket. Close study of the errors returned by bowerbird and jacana suggests that what we're seeing there may be conflicts with listening not outgoing sockets, so let's try this and see if it improves matters. It's certainly better than what's there now, in any case. Michael Paquier, adjusted by me to work on non-Windows as well as Windows
* Fix documentation & config inconsistencies around 428b1d6b2.Andres Freund2016-04-24
| | | | | | | | | | | | | Several issues: 1) checkpoint_flush_after doc and code disagreed about the default 2) new GUCs were missing from postgresql.conf.sample 3) Outdated source-code comment about bgwriter_flush_after's default 4) Sub-optimal categories assigned to new GUCs 5) Docs suggested backend_flush_after is PGC_SIGHUP, but it's PGC_USERSET. 6) Spell out int as integer in the docs, as done elsewhere Reported-By: Magnus Hagander, Fujii Masao Discussion: CAHGQGwETyTG5VYQQ5C_srwxWX7RXvFcD3dKROhvAWWhoSBdmZw@mail.gmail.com
* Rename strtoi() to strtoint().Tom Lane2016-04-23
| | | | | | | | | | | | NetBSD has seen fit to invent a libc function named strtoi(), which conflicts with the long-established static functions of the same name in datetime.c and ecpg's interval.c. While muttering darkly about intrusions on application namespace, we'll rename our functions to avoid the conflict. Back-patch to all supported branches, since this would affect attempts to build any of them on recent NetBSD. Thomas Munro
* Properly mark initRectBox() as taking 'void' argsBruce Momjian2016-04-23
| | | | | | Was part of box type in SP-GiST index patch. Reported-by: Emre Hasegeli
* Add putenv support for msvcrt from Visual Studio 2013Magnus Hagander2016-04-22
| | | | | | This was missed when VS 2013 support was added. Michael Paquier
* Fix unexpected side-effects of operator_precedence_warning.Tom Lane2016-04-21
| | | | | | | | | | | | | | | The implementation of that feature involves injecting nodes into the raw parsetree where explicit parentheses appear. Various places in parse_expr.c that test to see "is this child node of type Foo" need to look through such nodes, else we'll get different behavior when operator_precedence_warning is on than when it is off. Note that we only need to handle this when testing untransformed child nodes, since the AEXPR_PAREN nodes will be gone anyway after transformExprRecurse. Per report from Scott Ribe and additional code-reading. Back-patch to 9.5 where this feature was added. Report: <ED37E303-1B0A-4CD8-8E1E-B9C4C2DD9A17@elevated-dev.com>
* Fix planner failure with full join in RHS of left join.Tom Lane2016-04-21
| | | | | | | | | | | | | | | | | | | | | | | | | Given a left join containing a full join in its righthand side, with the left join's joinclause referencing only one side of the full join (in a non-strict fashion, so that the full join doesn't get simplified), the planner could fail with "failed to build any N-way joins" or related errors. This happened because the full join was seen as overlapping the left join's RHS, and then recent changes within join_is_legal() caused that function to conclude that the full join couldn't validly be formed. Rather than try to rejigger join_is_legal() yet more to allow this, I think it's better to fix initsplan.c so that the required join order is explicit in the SpecialJoinInfo data structure. The previous coding there essentially ignored full joins, relying on the fact that we don't flatten them in the joinlist data structure to preserve their ordering. That's sufficient to prevent a wrong plan from being formed, but as this example shows, it's not sufficient to ensure that the right plan will be formed. We need to work a bit harder to ensure that the right plan looks sane according to the SpecialJoinInfos. Per bug #14105 from Vojtech Rylko. This was apparently induced by commit 8703059c6 (though now that I've seen it, I wonder whether there are related cases that could have failed before that); so back-patch to all active branches. Unfortunately, that patch also went into 9.0, so this bug is a regression that won't be fixed in that branch.
* Improve TranslateSocketError() to handle more Windows error codes.Tom Lane2016-04-21
| | | | | | The coverage was rather lean for cases that bind() or listen() might return. Add entries for everything that there's a direct equivalent for in the set of Unix errnos that elog.c has heard of.
* Remove dead code in win32.h.Tom Lane2016-04-21
| | | | | | | | | | | There's no longer a need for the MSVC-version-specific code stanza that forcibly redefines errno code symbols, because since commit 73838b52 we're unconditionally redefining them in the stanza before this one anyway. Now it's merely confusing and ugly, so get rid of it; and improve the comment that explains what's going on here. Although this is just cosmetic, back-patch anyway since I'm intending to back-patch some less-cosmetic changes in this same hunk of code.
* PGDLLIMPORT-ify old_snapshot_threshold.Tom Lane2016-04-21
| | | | | | | Revert commit 7cb1db1d9599f0a09d6920d2149d956ef6d88b0e, which represented a misunderstanding of the problem (if snapmgr.h weren't already included in bufmgr.h, things wouldn't compile anywhere). Instead install what I think is the real fix.
* Fix ruleutils.c's dumping of ScalarArrayOpExpr containing an EXPR_SUBLINK.Tom Lane2016-04-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | When we shoehorned "x op ANY (array)" into the SQL syntax, we created a fundamental ambiguity as to the proper treatment of a sub-SELECT on the righthand side: perhaps what's meant is to compare x against each row of the sub-SELECT's result, or perhaps the sub-SELECT is meant as a scalar sub-SELECT that delivers a single array value whose members should be compared against x. The grammar resolves it as the former case whenever the RHS is a select_with_parens, making the latter case hard to reach --- but you can get at it, with tricks such as attaching a no-op cast to the sub-SELECT. Parse analysis would throw away the no-op cast, leaving a parsetree with an EXPR_SUBLINK SubLink directly under a ScalarArrayOpExpr. ruleutils.c was not clued in on this fine point, and would naively emit "x op ANY ((SELECT ...))", which would be parsed as the first alternative, typically leading to errors like "operator does not exist: text = text[]" during dump/reload of a view or rule containing such a construct. To fix, emit a no-op cast when dumping such a parsetree. This might well be exactly what the user wrote to get the construct accepted in the first place; and even if she got there with some other dodge, it is a valid representation of the parsetree. Per report from Karl Czajkowski. He mentioned only a case involving RLS policies, but actually the problem is very old, so back-patch to all supported branches. Report: <20160421001832.GB7976@moraine.isi.edu>
* Prevent possible crash reading pg_stat_activity.Robert Haas2016-04-21
| | | | | | | | Also, avoid reading PGPROC's wait_event field twice, once for the wait event and again for the wait_event_type, because the value might change in the middle. Petr Jelinek and Robert Haas
* Comment improvements for ForeignPath.Robert Haas2016-04-21
| | | | | | It's not necessarily just scanning a base relation any more. Amit Langote and Etsuro Fujita
* Fix assorted defects in 09adc9a8c09c9640de05c7023b27fb83c761e91c.Robert Haas2016-04-21
| | | | | | | | | | | | | | That commit increased all shared memory allocations to the next higher multiple of PG_CACHE_LINE_SIZE, but it didn't ensure that allocation started on a cache line boundary. It also failed to remove a couple other pieces of now-useless code. BUFFERALIGN() is perhaps obsolete at this point, and likely should be removed at some point, too, but that seems like it can be left to a future cleanup. Mistakes all pointed out by Andres Freund. The patch is mine, with a few extra assertions which I adopted from his version of this fix.
* Inline initial comparisons in TestForOldSnapshot()Kevin Grittner2016-04-21
| | | | | | | | | | | | Even with old_snapshot_threshold = -1 (which disables the "snapshot too old" feature), performance regressions were seen at moderate to high concurrency. For example, a one-socket, four-core system running 200 connections at saturation could see up to a 2.3% regression, with larger regressions possible on NUMA machines. By inlining the early (smaller, faster) tests in the TestForOldSnapshot() function, the i7 case dropped to a 0.2% regression, which could easily just be noise, and is clearly an improvement. Further testing will show whether more is needed.