aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* Fix broken multibyte regression tests.Tatsuo Ishii2015-08-09
| | | | | | | | | commit 9043Fe390f4f0b4586cfe59cbd22314b9c3e2957 broke multibyte regression tests because the commit removes the warning message when temporary hash indexes is created, which has been added by commit 07af523870bcfe930134054febd3a6a114942e5b. Back patched to 9.5 stable tree.
* Document items that should appear in the major release notesBruce Momjian2015-08-08
|
* Attempt to work around a 32bit xlc compiler bug from a different place.Andres Freund2015-08-08
| | | | | | | | | | | | | | | | | | | | | In de6fd1c8 I moved the the work around from 53f73879 into the aix template. The previous location was removed in the former commit, and I thought that it would be nice to emit a warning when running configure. That didn't turn out to work because at the point the template is included we don't know whether we're compiling a 32/64 bit binary and it's possible to install compilers for both on a 64 bit kernel/OS. So go back to a less ambitious approach and define PG_FORCE_DISABLE_INLINE in port/aix.h, without emitting a warning. We could try a more fancy approach, but it doesn't seem worth it. This requires moving the check for PG_FORCE_DISABLE_INLINE in c.h to after including the system headers included from therein which isn't perfect, as it seems slightly more robust to include all system headers in a similar environment. Oh well. Discussion: 20150807132000.GC13310@awork2.anarazel.de
* Fix bug slowing down pgbench when -P is used.Andres Freund2015-08-08
| | | | | | | | | | A removed check in ba3deeefb made all threads but the main one busy-loop when -P was used. All threads computed the time to the next time the progress report should be printed, but only the main thread did so and re-scheduled it only for the future. Reported-By: Jesper Pedersen Discussion: 55C4E190.3050104@redhat.com
* Further adjustments to PlaceHolderVar removal.Tom Lane2015-08-07
| | | | | | | | | | | | | | | | | | | | | | | | A new test case from Andreas Seltenreich showed that we were still a bit confused about removing PlaceHolderVars during join removal. Specifically, remove_rel_from_query would remove a PHV that was used only underneath the removable join, even if the place where it's used was the join partner relation and not the join clause being deleted. This would lead to a "too late to create a new PlaceHolderInfo" error later on. We can defend against that by checking ph_eval_at to see if the PHV could possibly be getting used at some partner rel. Also improve some nearby LATERAL-related logic. I decided that the check on ph_lateral needed to take precedence over the check on ph_needed, in case there's a lateral reference underneath the join being considered. (That may be impossible, but I'm not convinced of it, and it's easy enough to defend against the case.) Also, I realized that remove_rel_from_query's logic for updating LateralJoinInfos is dead code, because we don't build those at all until after join removal. Back-patch to 9.3. Previous versions didn't have the LATERAL issues, of course, and they also didn't attempt to remove PlaceHolderInfos during join removal. (I'm starting to wonder if changing that was really such a great idea.)
* Fix attach-related race condition in shm_mq_send_bytes.Robert Haas2015-08-07
| | | | Spotted by Antonin Houska.
* Don't include low level locking code from frontend code.Andres Freund2015-08-07
| | | | | | | | | | | | | | | | | | | | | | Some frontend code like e.g. pg_xlogdump or pg_resetxlog, has to use backend headers. Unfortunately until now that code includes most of the locking code. It's generally not nice to expose such low level details, but de6fd1c898 made that a hard problem. We fall back to defining 'inline' away if the compiler doesn't support it - that can cause linker errors like on buildfarm animal pademelon if a inline function references backend only code. To fix that problem separate definitions from lock.h that are required from frontend code into lockdefs.h and use it in the relevant places. I've only removed the minimal amount of necessary definitions for now - it might turn out that we want more for other reasons. To avoid such details being exposed again put some checks against being included from frontend code into atomics.h, lock.h, lwlock.h and s_lock.h. It's otherwise fairly easy to indirectly include these headers. Discussion: 20150806070902.GE12214@awork2.anarazel.de
* Address points made in post-commit review of replication origins.Andres Freund2015-08-07
| | | | | | | | | | Amit reviewed the replication origins patch and made some good points. Address them. This fixes typos in error messages, docs and comments and adds a missing error check (although in a should-never-happen scenario). Discussion: CAA4eK1JqUBVeWWKwUmBPryFaje4190ug0y-OAUHWQ6tD83V4xg@mail.gmail.com Backpatch: 9.5, where replication origins were introduced.
* Fix old oversight in join removal logic.Tom Lane2015-08-06
| | | | | | | | | | | | | Commit 9e7e29c75ad441450f9b8287bd51c13521641e3b introduced an Assert that join removal didn't reduce the eval_at set of any PlaceHolderVar to empty. At first glance it looks like join_is_removable ensures that's true --- but actually, the loop in join_is_removable skips PlaceHolderVars that are not referenced above the join due to be removed. So, if we don't want any empty eval_at sets, the right thing to do is to delete any now-unreferenced PlaceHolderVars from the data structure entirely. Per fuzz testing by Andreas Seltenreich. Back-patch to 9.3 where the aforesaid Assert was added.
* Fix eclass_useful_for_merging to give valid results for appendrel children.Tom Lane2015-08-06
| | | | | | | | | | | | | | | | | | | | Formerly, this function would always return "true" for an appendrel child relation, because it would think that the appendrel parent was a potential join target for the child. In principle that should only lead to some inefficiency in planning, but fuzz testing by Andreas Seltenreich disclosed that it could lead to "could not find pathkey item to sort" planner errors in odd corner cases. Specifically, we would think that all columns of a child table's multicolumn index were interesting pathkeys, causing us to generate a MergeAppend path that sorts by all the columns. However, if any of those columns weren't actually used above the level of the appendrel, they would not get added to that rel's targetlist, which would result in being unable to resolve the MergeAppend's sort keys against its targetlist during createplan.c. Backpatch to 9.3. In older versions, columns of an appendrel get added to its targetlist even if they're not mentioned above the scan level, so that the failure doesn't occur. It might be worth back-patching this fix to older versions anyway, but I'll refrain for the moment.
* Further fixes for degenerate outer join clauses.Tom Lane2015-08-06
| | | | | | | | | | | | | | | | | | | | | | | | | | Further testing revealed that commit f69b4b9495269cc4 was still a few bricks shy of a load: minor tweaking of the previous test cases resulted in the same wrong-outer-join-order problem coming back. After study I concluded that my previous changes in make_outerjoininfo() were just accidentally masking the problem, and should be reverted in favor of forcing syntactic join order whenever an upper outer join's predicate doesn't mention a lower outer join's LHS. This still allows the chained-outer-joins style that is the normally optimizable case. I also tightened things up some more in join_is_legal(). It seems to me on review that what's really happening in the exception case where we ignore a mismatched special join is that we're allowing the proposed join to associate into the RHS of the outer join we're comparing it to. As such, we should *always* insist that the proposed join be a left join, which eliminates a bunch of rather dubious argumentation. The case where we weren't enforcing that was the one that was already known buggy anyway (it had a violatable Assert before the aforesaid commit) so it hardly deserves a lot of deference. Back-patch to all active branches, like the previous patch. The added regression test case failed in all branches back to 9.1, and I think it's only an unrelated change in costing calculations that kept 9.0 from choosing a broken plan.
* Fix incorrect calculation in shm_mq_receive.Robert Haas2015-08-06
| | | | | | | | | If some, but not all, of the length word has already been read, and the next attempt to read sees exactly the number of bytes needed to complete the length word, or fewer, then we'll incorrectly read less than all of the available data. Antonin Houska
* Reduce ProcArrayLock contention by removing backends in batches.Robert Haas2015-08-06
| | | | | | | | | | | | | | | | | When a write transaction commits, it must clear its XID advertised via the ProcArray, which requires that we hold ProcArrayLock in exclusive mode in order to prevent concurrent processes running GetSnapshotData from seeing inconsistent results. When many processes try to commit at once, ProcArrayLock must change hands repeatedly, with each concurrent process trying to commit waking up to acquire the lock in turn. To make things more efficient, when more than one backend is trying to commit a write transaction at the same time, have just one of them acquire ProcArrayLock in exclusive mode and clear the XIDs of all processes in the group. Benchmarking reveals that this is much more efficient at very high client counts. Amit Kapila, heavily revised by me, with some review also from Pavan Deolasee.
* Fix `make installcheck` for serializable transactions.Kevin Grittner2015-08-06
| | | | | | | | | | | | | | | | | | | | | Commit e5550d5fec66aa74caad1f79b79826ec64898688 added some new tests for ALTER TABLE which involved table scans. When default_transaction_isolation = 'serializable' these acquire relation-level SIReadLocks. The test results didn't cope with that. Add SIReadLock as the minimum lock level for purposes of these tests. This could also be fixed by excluding this type of lock from the my_locks view, but it would be a bug for SIReadLock to show up for a relation which was not otherwise locked, so do it this way to allow that sort of condition to cause a regression test failure. There is some question whether we could avoid taking SIReadLocks during these operations, but confirming the safety of that and figuring out how to avoid the locks is not trivial, and would be a separate patch. Backpatch to 9.4 where the new tests were added.
* Improve includes introduced in the replication origins patch.Andres Freund2015-08-06
| | | | | | | | pg_resetxlog.h contained two superfluous includes, origin.h superfluously depended on logical.h, and pg_xlogdump's rmgrdesc.h only indirectly included origin.h. Backpatch: 9.5, where replication origins were introduced.
* Reconcile nodes/*funcs.c with recent work.Noah Misch2015-08-05
| | | | | | | | | A few of the discrepancies had semantic significance, but I did not track down the resulting user-visible bugs, if any. Back-patch to 9.5, where all but one discrepancy appeared. The _equalCreateEventTrigStmt() situation dates to 9.3 but does not affect semantics. catversion bump due to readfuncs.c field order changes.
* Link $(WIN32RES) into single-file modules only when PGFILEDESC is set.Noah Misch2015-08-05
| | | | | | | | | | Commit 0ffc201a51395ca71fe429ef86c872850a5850ee included this object unconditionally. Being unprepared for that, most external, single-file modules failed to build. This better aligns the GNU make build system with the heuristic in the MSVC build's Project::AddDirResourceFile(). In-tree, installed modules set PGFILEDESC, so they will see no change. Also, under PGXS, omit the nonfunctioning rule to build win32ver.rc. Back-patch to 9.5, where the aforementioned commit first appeared.
* Allow pg_rewind tap tests to run with older File::Path versionsAndrew Dunstan2015-08-05
| | | | | | Older versions have rmtree but not remove_tree. The one-argument forms of these are equivalent, so replace remove_tree with rmtree. This allows the tests to be run on oldish Msys systems.
* Remove carriage returns from certain tap test output under MsysAndrew Dunstan2015-08-05
| | | | These were causing spurious test failures.
* Fix BRIN to use SnapshotAny during summarizationAlvaro Herrera2015-08-05
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | For correctness of summarization results, it is critical that the snapshot used during the summarization scan is able to see all tuples that are live to all transactions -- including tuples inserted or deleted by in-progress transactions. Otherwise, it would be possible for a transaction to insert a tuple, then idle for a long time while a concurrent transaction executes summarization of the range: this would result in the inserted value not being considered in the summary. Previously we were trying to use a MVCC snapshot in conjunction with adding a "placeholder" tuple in the index: the snapshot would see all committed tuples, and the placeholder tuple would catch insertions by any new inserters. The hole is that prior insertions by transactions that are still in progress by the time the MVCC snapshot was taken were ignored. Kevin Grittner reported this as a bogus error message during vacuum with default transaction isolation mode set to repeatable read (because the error report mentioned a function name not being invoked during), but the problem is larger than that. To fix, tweak IndexBuildHeapRangeScan to have a new mode that behaves the way we need using SnapshotAny visibility rules. This change simplifies the BRIN code a bit, mainly by removing large comments that were mistaken. Instead, rely on the SnapshotAny semantics to provide what it needs. (The business about a placeholder tuple needs to remain: that covers the case that a transaction inserts a a tuple in a page that summarization already scanned.) Discussion: https://www.postgresql.org/message-id/20150731175700.GX2441@postgresql.org In passing, remove a couple of unused declarations from brin.h and reword a comment to be proper English. This part submitted by Kevin Grittner. Backpatch to 9.5, where BRIN was introduced.
* Make real sure we don't reassociate joins into or out of SEMI/ANTI joins.Tom Lane2015-08-05
| | | | | | | | | | | | | | | | Per the discussion in optimizer/README, it's unsafe to reassociate anything into or out of the RHS of a SEMI or ANTI join. An example from Piotr Stefaniak showed that join_is_legal() wasn't sufficiently enforcing this rule, so lock it down a little harder. I couldn't find a reasonably simple example of the optimizer trying to do this, so no new regression test. (Piotr's example involved the random search in GEQO accidentally trying an invalid case and triggering a sanity check way downstream in clause selectivity estimation, which did not seem like a sequence of events that would be useful to memorialize in a regression test as-is.) Back-patch to all active branches.
* Fix typo in commit de6fd1c.Andres Freund2015-08-05
| | | | Per buildfarm members mandrill and hornet.
* Rely on inline functions even if that causes warnings in older compilers.Andres Freund2015-08-05
| | | | | | | | | | | | | | | | | | | | | | | | | So far we have worked around the fact that some very old compilers do not support 'inline' functions by only using inline functions conditionally (or not at all). Since such compilers are very rare by now, we have decided to rely on inline functions from 9.6 onwards. To avoid breaking these old compilers inline is defined away when not supported. That'll cause "function x defined but not used" type of warnings, but since nobody develops on such compilers anymore that's ok. This change in policy will allow us to more easily employ inline functions. I chose to remove code previously conditional on PG_USE_INLINE as it seemed confusing to have code dependent on a define that's always defined. Blacklisting of compilers, like in c53f73879f, now has to be done differently. A platform template can define PG_FORCE_DISABLE_INLINE to force inline to be defined empty. Discussion: 20150701161447.GB30708@awork2.anarazel.de
* Fix debug message output when connecting to a logical slot.Andres Freund2015-08-05
| | | | | | | | | | Previously the message erroneously printed the same LSN twice as the assignment to the start_lsn variable was before the message. Correct that. Reported-By: Marko Tiikkaja Author: Marko Tiikkaja Backpatch: 9.5, where logical decoding was introduced
* Fix comment atomics.h.Andres Freund2015-08-05
| | | | | | | | | I appear to accidentally have switched the comments for pg_atomic_write_u32 and pg_atomic_read_u32 around. Also fix some minor typos I found while fixing. Noticed-By: Amit Kapila Backpatch: 9.5
* Fix pg_dump to dump shell types.Tom Lane2015-08-04
| | | | | | | | | | | Per discussion, it really ought to do this. The original choice to exclude shell types was probably made in the dark ages before we made it harder to accidentally create shell types; but that was in 7.3. Also, cause the standard regression tests to leave a shell type behind, for convenience in testing the case in pg_dump and pg_upgrade. Back-patch to all supported branches.
* Fix bogus "out of memory" reports in tuplestore.c.Tom Lane2015-08-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | The tuplesort/tuplestore memory management logic assumed that the chunk allocation overhead for its memtuples array could not increase when increasing the array size. This is and always was true for tuplesort, but we (I, I think) blindly copied that logic into tuplestore.c without noticing that the assumption failed to hold for the much smaller array elements used by tuplestore. Given rather small work_mem, this could result in an improper complaint about "unexpected out-of-memory situation", as reported by Brent DeSpain in bug #13530. The easiest way to fix this is just to increase tuplestore's initial array size so that the assumption holds. Rather than relying on magic constants, though, let's export a #define from aset.c that represents the safe allocation threshold, and make tuplestore's calculation depend on that. Do the same in tuplesort.c to keep the logic looking parallel, even though tuplesort.c isn't actually at risk at present. This will keep us from breaking it if we ever muck with the allocation parameters in aset.c. Back-patch to all supported versions. The error message doesn't occur pre-9.3, not so much because the problem can't happen as because the pre-9.3 tuplestore code neglected to check for it. (The chance of trouble is a great deal larger as of 9.3, though, due to changes in the array-size-increasing strategy.) However, allowing LACKMEM() to become true unexpectedly could still result in less-than-desirable behavior, so let's patch it all the way back.
* Fix a PlaceHolderVar-related oversight in star-schema planning patch.Tom Lane2015-08-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In commit b514a7460d9127ddda6598307272c701cbb133b7, I changed the planner so that it would allow nestloop paths to remain partially parameterized, ie the inner relation might need parameters from both the current outer relation and some upper-level outer relation. That's fine so long as we're talking about distinct parameters; but the patch also allowed creation of nestloop paths for cases where the inner relation's parameter was a PlaceHolderVar whose eval_at set included the current outer relation and some upper-level one. That does *not* work. In principle we could allow such a PlaceHolderVar to be evaluated at the lower join node using values passed down from the upper relation along with values from the join's own outer relation. However, nodeNestloop.c only supports simple Vars not arbitrary expressions as nestloop parameters. createplan.c is also a few bricks shy of being able to handle such cases; it misplaces the PlaceHolderVar parameters in the plan tree, which is why the visible symptoms of this bug are "plan should not reference subplan's variable" and "failed to assign all NestLoopParams to plan nodes" planner errors. Adding the necessary complexity to make this work doesn't seem like it would be repaid in significantly better plans, because in cases where such a PHV exists, there is probably a corresponding join order constraint that would allow a good plan to be found without using the star-schema exception. Furthermore, adding complexity to nodeNestloop.c would create a run-time penalty even for plans where this whole consideration is irrelevant. So let's just reject such paths instead. Per fuzz testing by Andreas Seltenreich; the added regression test is based on his example query. Back-patch to 9.2, like the previous patch.
* Cap wal_buffers to avoid a server crash when it's set very large.Robert Haas2015-08-04
| | | | | | | | | | It must be possible to multiply wal_buffers by XLOG_BLCKSZ without overflowing int, or calculations in StartupXLOG will go badly wrong and crash the server. Avoid that by imposing a maximum value on wal_buffers. This will be just under 2GB, assuming the usual value for XLOG_BLCKSZ. Josh Berkus, per an analysis by Andrew Gierth.
* Tab completion for CREATE SEQUENCE.Robert Haas2015-08-04
| | | | Vik Fearing, reviewed by Brendan Jurd, Michael Paquier, and myself
* Update comment to match behavior of latest code.Robert Haas2015-08-04
| | | | Peter Geoghegan
* Share transition state between different aggregates when possible.Heikki Linnakangas2015-08-04
| | | | | | | | | | | | If there are two different aggregates in the query with same inputs, and the aggregates have the same initial condition and transition function, only calculate the state value once, and only call the final functions separately. For example, AVG(x) and SUM(x) aggregates have the same transition function, which accumulates the sum and number of input tuples. For a query like "SELECT AVG(x), SUM(x) FROM x", we can therefore accumulate the state function only once, which gives a nice speedup. David Rowley, reviewed and edited by me.
* RLS: Keep deny policy when only restrictive existStephen Frost2015-08-03
| | | | | | | | | | | | | | | Only remove the default deny policy when a permissive policy exists (either from the hook or defined by the user). If only restrictive policies exist then no rows will be visible, as restrictive policies shouldn't make rows visible. To address this requirement, a single "USING (true)" permissive policy can be created. Update the test_rls_hooks regression tests to create the necessary "USING (true)" permissive policy. Back-patch to 9.5 where RLS was added. Per discussion with Dean.
* Fix psql \d output of policies.Joe Conway2015-08-03
| | | | | | psql neglected to wrap parenthesis around USING and WITH CHECK expressions -- fixed. Back-patched to 9.5 where RLS policies were introduced.
* Make recovery rename tablespace_map to *.old if backup_label is not present.Fujii Masao2015-08-03
| | | | | | | | | | | | | | | If tablespace_map file is present without backup_label file, there is no use of such file. There is no harm in retaining it, but it is better to get rid of the map file so that we don't have any redundant file in data directory and it will avoid any sort of confusion. It seems prudent though to just rename the file out of the way rather than delete it completely, also we ignore any error that occurs in rename operation as even if map file is present without backup_label file, it is harmless. Back-patch to 9.5 where tablespace_map file was introduced. Amit Kapila, reviewed by Robert Haas, Alvaro Herrera and me.
* Fix pg_rewind when pg_xlog is a symlink.Heikki Linnakangas2015-08-03
| | | | | | | | | | pg_xlog is often a symlink, typically to a different filesystem. Don't get confused and comlain about by that, and just always pretend that it's a normal directory, even if it's really a symlink. Also add a test case for this. Backpatch to 9.5.
* Clean up pg_rewind regression test script.Heikki Linnakangas2015-08-03
| | | | | | | | | | Since commit 01f6bb4b2, TestLib.pm has exported path to tmp_check directory, so let's use that also for the pg_rewind test clusters etc. Also, in master, the $tempdir_short variable has not been used since commit 13d856e17, which moved the initdb-running code to TestLib.pm. Backpatch to 9.5.
* Make modules/test_ddl_deparse/.gitignore match its siblings.Tom Lane2015-08-03
| | | | | Not sure why /tmp_check/ was omitted from this one, but even if it isn't really needed right now, it's inconsistent not to include it.
* Fix a number of places that produced XX000 errors in the regression tests.Tom Lane2015-08-02
| | | | | | | | | | | | | | | | | | | | It's against project policy to use elog() for user-facing errors, or to omit an errcode() selection for errors that aren't supposed to be "can't happen" cases. Fix all the violations of this policy that result in ERRCODE_INTERNAL_ERROR log entries during the standard regression tests, as errors that can reliably be triggered from SQL surely should be considered user-facing. I also looked through all the files touched by this commit and fixed other nearby problems of the same ilk. I do not claim to have fixed all violations of the policy, just the ones in these files. In a few places I also changed existing ERRCODE choices that didn't seem particularly appropriate; mainly replacing ERRCODE_SYNTAX_ERROR by something more specific. Back-patch to 9.5, but no further; changing ERRCODE assignments in stable branches doesn't seem like a good idea.
* Allow TAP tests to run under MsysAndrew Dunstan2015-08-02
| | | | | | | | | The Msys DTK perl, which is required to run TAP tests under Msys as a native perl won't recognize the correct virtual paths, has its osname recorded in the Config module as 'msys' instead of 'MSWin32'. To avoid having to repeat the test a variable is created that is true iff the osname is either of these values, and is then used everywhere that matters.
* Avoid calling memcpy() with a NULL source pointer and count == 0.Tom Lane2015-08-02
| | | | | | | | | | | | | | | | As in commit 0a52d378b03b7d5a, avoid doing something that has undefined results according to the C standard, even though in practice there does not seem to be any problem with it. This fixes two places in numeric.c that demonstrably could call memcpy() with such arguments. I looked through that file and didn't see any other places with similar hazards; this is not to claim that there are not such places in other files. Per report from Piotr Stefaniak. Back-patch to 9.5 which is where the previous commit was added. We're more or less setting a precedent that we will not worry about this type of issue in pre-9.5 branches unless someone demonstrates a problem in the field.
* Fix incorrect order of lock file removal and failure to close() sockets.Tom Lane2015-08-02
| | | | | | | | | | | | | | | | | | | | | | | | | Commit c9b0cbe98bd783e24a8c4d8d8ac472a494b81292 accidentally broke the order of operations during postmaster shutdown: it resulted in removing the per-socket lockfiles after, not before, postmaster.pid. This creates a race-condition hazard for a new postmaster that's started immediately after observing that postmaster.pid has disappeared; if it sees the socket lockfile still present, it will quite properly refuse to start. This error appears to be the explanation for at least some of the intermittent buildfarm failures we've seen in the pg_upgrade test. Another problem, which has been there all along, is that the postmaster has never bothered to close() its listen sockets, but has just allowed them to close at process death. This creates a different race condition for an incoming postmaster: it might be unable to bind to the desired listen address because the old postmaster is still incumbent. This might explain some odd failures we've seen in the past, too. (Note: this is not related to the fact that individual backends don't close their client communication sockets. That behavior is intentional and is not changed by this patch.) Fix by adding an on_proc_exit function that closes the postmaster's ports explicitly, and (in 9.3 and up) reshuffling the responsibility for where to unlink the Unix socket files. Lock file unlinking can stay where it is, but teach it to unlink the lock files in reverse order of creation.
* Fix race condition that lead to WALInsertLock deadlock with commit_delay.Heikki Linnakangas2015-08-02
| | | | | | | | | | | | | | | | | If a call to WaitForXLogInsertionsToFinish() returned a value in the middle of a page, and another backend then started to insert a record to the same page, and then you called WaitXLogInsertionsToFinish() again, the second call might return a smaller value than the first call. The problem was in GetXLogBuffer(), which always updated the insertingAt value to the beginning of the requested page, not the actual requested location. Because of that, the second call might return a xlog pointer to the beginning of the page, while the first one returned a later position on the same page. XLogFlush() performs two calls to WaitXLogInsertionsToFinish() in succession, and holds WALWriteLock on the second call, which can deadlock if the second call to WaitXLogInsertionsToFinish() blocks. Reported by Spiros Ioannou. Backpatch to 9.4, where the more scalable WALInsertLock mechanism, and this bug, was introduced.
* Micro optimize LWLockAttemptLock() a bit.Andres Freund2015-08-02
| | | | | | | | | | | | LWLockAttemptLock pointlessly read the lock's state in every loop iteration, even though pg_atomic_compare_exchange_u32() returns the old value. Instead do that only once before the loop iteration. Additionally there's no need to have the expected_state variable, old_state mostly had the same value anyway. Noticed-By: Heikki Linnakangas Backpatch: 9.5, no reason to let the branches diverge at this point
* Fix issues around the "variable" support in the lwlock infrastructure.Andres Freund2015-08-02
| | | | | | | | | | | | | | | | | | | | | | | | | The lwlock scalability work introduced two race conditions into the lwlock variable support provided for xlog.c. First, and harmlessly on most platforms, it set/read the variable without the spinlock in some places. Secondly, due to the removal of the spinlock, it was possible that a backend missed changes to the variable's state if it changed in the wrong moment because checking the lock's state, the variable's state and the queuing are not protected by a single spinlock acquisition anymore. To fix first move resetting the variable's from LWLockAcquireWithVar to WALInsertLockRelease, via a new function LWLockReleaseClearVar. That prevents issues around waiting for a variable's value to change when a new locker has acquired the lock, but not yet set the value. Secondly re-check that the variable hasn't changed after enqueing, that prevents the issue that the lock has been released and already re-acquired by the time the woken up backend checks for the lock's state. Reported-By: Jeff Janes Analyzed-By: Heikki Linnakangas Reviewed-By: Heikki Linnakangas Discussion: 5592DB35.2060401@iki.fi Backpatch: 9.5, where the lwlock scalability went in
* Fix some planner issues with degenerate outer join clauses.Tom Lane2015-08-01
| | | | | | | | | | | | | An outer join clause that didn't actually reference the RHS (perhaps only after constant-folding) could confuse the join order enforcement logic, leading to wrong query results. Also, nested occurrences of such things could trigger an Assertion that on reflection seems incorrect. Per fuzz testing by Andreas Seltenreich. The practical use of such cases seems thin enough that it's not too surprising we've not heard field reports about it. This has been broken for a long time, so back-patch to all active branches.
* Teach predtest.c that "foo" implies "foo IS NOT NULL".Tom Lane2015-08-01
| | | | | | | | | | | Per complaint from Peter Holzer. It's useful to cover this special case, since for a boolean variable "foo", earlier parts of the planner will have reduced variants like "foo = true" to just "foo", and thus we may fail to recognize the applicability of a partial index with predicate "foo IS NOT NULL". Back-patch to 9.5, but not further; given the lack of previous complaints this doesn't seem like behavior to change in stable branches.
* Fix an oversight in checking whether a join with LATERAL refs is legal.Tom Lane2015-07-31
| | | | | | | | | | | | | | | In many cases, we can implement a semijoin as a plain innerjoin by first passing the righthand-side relation through a unique-ification step. However, one of the cases where this does NOT work is where the RHS has a LATERAL reference to the LHS; that makes the RHS dependent on the LHS so that unique-ification is meaningless. joinpath.c understood this, and so would not generate any join paths of this kind ... but join_is_legal neglected to check for the case, so it would think that we could do it. The upshot would be a "could not devise a query plan for the given query" failure once we had failed to generate any join paths at all for the bogus join pair. Back-patch to 9.3 where LATERAL was added.
* Clean up Makefile.win32 "-I" flag additions.Noah Misch2015-07-30
| | | | | | The PGXS-case directory does not exist in the non-PGXS case, and vice versa. Add one or the other, not both. This is essentially cosmetic. It makes Makefile.win32 more like the similar Makefile.global code.
* Consolidate makefile code for setting top_srcdir, srcdir and VPATH.Noah Misch2015-07-30
| | | | | | | | | Responsibility was formerly split between Makefile.global and pgxs.mk. As a result of commit b58233c71b93a32fcab7219585cafc25a27eb769, in the PGXS case, these variables were unset while parsing Makefile.global and callees. Inclusion of Makefile.custom did not work from PGXS, and the subtle difference seemed like a recipe for future bugs. Back-patch to 9.4, where that commit first appeared.