aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
* Fix unitialized variablesAlvaro Herrera2015-08-13
| | | | | | | | | As complained by clang, reported by Andres Freund. Brown paper bag bug in ccc4c074994d. Add some comments, too. Backpatch to 9.5, like that one.
* Undo mistaken tightening in join_is_legal().Tom Lane2015-08-12
| | | | | | | | | | | | | | | | | | | One of the changes I made in commit 8703059c6b55c427 turns out not to have been such a good idea: we still need the exception in join_is_legal() that allows a join if both inputs already overlap the RHS of the special join we're checking. Otherwise we can miss valid plans, and might indeed fail to find a plan at all, as in recent report from Andreas Seltenreich. That code was added way back in commit c17117649b9ae23d, but I failed to include a regression test case then; my bad. Put it back with a better explanation, and a test this time. The logic does end up a bit different than before though: I now believe it's appropriate to make this check first, thereby allowing such a case whether or not we'd consider the previous SJ(s) to commute with this one. (Presumably, we already decided they did; but it was confusing to have this consideration in the middle of the code that was handling the other case.) Back-patch to all active branches, like the previous patch.
* Close some holes in BRIN page assignmentAlvaro Herrera2015-08-12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In some corner cases, it is possible for the BRIN index relation to be extended by brin_getinsertbuffer but the new page not be used immediately for anything by its callers; when this happens, the page is initialized and the FSM is updated (by brin_getinsertbuffer) with the info about that page, but these actions are not WAL-logged. A later index insert/update can use the page, but since the page is already initialized, the initialization itself is not WAL-logged then either. Replay of this sequence of events causes recovery to fail altogether. There is a related corner case within brin_getinsertbuffer itself, in which we extend the relation to put a new index tuple there, but later find out that we cannot do so, and do not return the buffer; the page obtained from extension is not even initialized. The resulting page is lost forever. To fix, shuffle the code so that initialization is not the responsibility of brin_getinsertbuffer anymore, in normal cases; instead, the initialization is done by its callers (brin_doinsert and brin_doupdate) once they're certain that the page is going to be used. When either those functions determine that the new page cannot be used, before bailing out they initialize the page as an empty regular page, enter it in FSM and WAL-log all this. This way, the page is usable for future index insertions, and WAL replay doesn't find trying to insert tuples in pages whose initialization didn't make it to the WAL. The same strategy is used in brin_getinsertbuffer when it cannot return the new page. Additionally, add a new step to vacuuming so that all pages of the index are scanned; whenever an uninitialized page is found, it is initialized as empty and WAL-logged. This closes the hole that the relation is extended but the system crashes before anything is WAL-logged about it. We also take this opportunity to update the FSM, in case it has gotten out of date. Thanks to Heikki Linnakangas for finding the problem that kicked some additional analysis of BRIN page assignment code. Backpatch to 9.5, where BRIN was introduced. Discussion: https://www.postgresql.org/message-id/20150723204810.GY5596@postgresql.org
* Remove duplicated assignment in pg_create_physical_replication_slot.Andres Freund2015-08-12
| | | | Reported-By: Gurjeet Singh
* Handle PQresultErrorField(PG_DIAG_SQLSTATE) returning NULL in streamutil.c.Andres Freund2015-08-12
| | | | | | | | | | In ff27db5d I missed that PQresultErrorField() may return NULL if there's no sqlstate associated with an error. Spotted-By: Coverity Reported-By: Michael Paquier Discussion: CAB7nPqQ3o10SY6NVdU4pjq85GQTN5tbbkq2gnNUh2fBNU3rKyQ@mail.gmail.com Backpatch: 9.5, like ff27db5d
* Fix two off-by-one errors in bufmgr.c.Andres Freund2015-08-12
| | | | | | | | | | | | | | | | | | | In 4b4b680c I passed a buffer index number (starting from 0) instead of a proper Buffer id (which start from 1 for shared buffers) in two places. This wasn't noticed so far as one of those locations isn't compiled at all (PrintPinnedBufs) and the other one (InvalidBuffer) requires a unlikely, but possible, set of circumstances to trigger a symptom. To reduce the likelihood of such incidents a bit also convert existing open coded mappings from buffer descriptors to buffer ids with BufferDescriptorGetBuffer(). Author: Qingqing Zhou Reported-By: Qingqing Zhou Discussion: CAJjS0u2ai9ooUisKtkV8cuVUtEkMTsbK8c7juNAjv8K11zeCQg@mail.gmail.com Backpatch: 9.5 where the private ref count infrastructure was introduced
* Fix some possible low-memory failures in regexp compilation.Tom Lane2015-08-12
| | | | | | | | | newnfa() failed to set the regex error state when malloc() fails. Several places in regcomp.c failed to check for an error after calling subre(). Each of these mistakes could lead to null-pointer-dereference crashes in memory-starved backends. Report and patch by Andreas Seltenreich. Back-patch to all branches.
* Postpone extParam/allParam calculations until the very end of planning.Tom Lane2015-08-11
| | | | | | | | | | | | | | | | | | | | | | | | Until now we computed these Param ID sets at the end of subquery_planner, but that approach depends on subquery_planner returning a concrete Plan tree. We would like to switch over to returning one or more Paths for a subquery, and in that representation the necessary details aren't fully fleshed out (not to mention that we don't really want to do this work for Paths that end up getting discarded). Hence, refactor so that we can compute the param ID sets at the end of planning, just before set_plan_references is run. The main change necessary to make this work is that we need to capture the set of outer-level Param IDs available to the current query level before exiting subquery_planner, since the outer levels' plan_params lists are transient. (That's not going to pose a problem for returning Paths, since all the work involved in producing that data is part of expression preprocessing, which will continue to happen before Paths are produced.) On the plus side, this change gets rid of several existing kluges. Eventually I'd like to get rid of SS_finalize_plan altogether in favor of doing this work during set_plan_references, but that will require some complex rejiggering because SS_finalize_plan needs to visit subplans and initplans before the main plan. So leave that idea for another day.
* Don't include rel.h when relcache.h is sufficientAlvaro Herrera2015-08-11
| | | | Trivial change to reduce exposure of rel.h.
* More fixes to allow pg_rewind tests to run on Msys.Andrew Dunstan2015-08-11
|
* Allow pg_create_physical_replication_slot() to reserve WAL.Andres Freund2015-08-11
| | | | | | | | | | | | | | | | When creating a physical slot it's often useful to immediately reserve the current WAL position instead of only doing after the first feedback message arrives. That e.g. allows slots to guarantee that all the WAL for a base backup will be available afterwards. Logical slots already have to reserve WAL during creation, so generalize that logic into being usable for both physical and logical slots. Catversion bump because of the new parameter. Author: Gurjeet Singh Reviewed-By: Andres Freund Discussion: CABwTF4Wh_dBCzTU=49pFXR6coR4NW1ynb+vBqT+Po=7fuq5iCw@mail.gmail.com
* Introduce macros determining if a replication slot is physical or logical.Andres Freund2015-08-11
| | | | | | | | These make the code a bit easier to read, and make it easier to add a more explicit notion of a slot's type at some point in the future. Author: Gurjeet Singh Discussion: CABwTF4Wh_dBCzTU=49pFXR6coR4NW1ynb+vBqT+Po=7fuq5iCw@mail.gmail.com
* Minor cleanups in slot related code.Andres Freund2015-08-11
| | | | | | | | Fix a bunch of typos, and remove two superflous includes. Author: Gurjeet Singh Discussion: CABwTF4Wh_dBCzTU=49pFXR6coR4NW1ynb+vBqT+Po=7fuq5iCw@mail.gmail.com Backpatch: 9.4
* Fix privilege dumping from servers too old to have that type of privilege.Tom Lane2015-08-10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | pg_dump produced fairly silly GRANT/REVOKE commands when dumping types from pre-9.2 servers, and when dumping functions or procedural languages from pre-7.3 servers. Those server versions lack the typacl, proacl, and/or lanacl columns respectively, and pg_dump substituted default values that were in fact incorrect. We ended up revoking all the owner's own privileges for the object while granting all privileges to PUBLIC. Of course the owner would then have those privileges again via PUBLIC, so long as she did not try to revoke PUBLIC's privileges; which may explain the lack of field reports. Nonetheless this is pretty silly behavior. The stakes were raised by my recent patch to make pg_dump dump shell types, because 9.2 and up pg_dump would proceed to emit bogus GRANT/REVOKE commands for a shell type if dumping from a pre-9.2 server; and the server will not accept GRANT/REVOKE commands for a shell type. (Perhaps it should, but that's a topic for another day.) So the resulting dump script wouldn't load without errors. The right thing to do is to act as though these objects have default privileges (null ACL entries), which causes pg_dump to print no GRANT/REVOKE commands at all for them. That fixes the silly results and also dodges the problem with shell types. In passing, modify getProcLangs() to be less creatively different about how to handle missing columns when dumping from older server versions. Every other data-acquisition function in pg_dump does that by substituting appropriate default values in the version-specific SQL commands, and I see no reason why this one should march to its own drummer. Its use of "SELECT *" was likewise not conformant with anyplace else, not to mention it's not considered good SQL style for production queries. Back-patch to all supported versions. Although 9.0 and 9.1 pg_dump don't have the issue with typacl, they are more likely than newer versions to be used to dump from ancient servers, so we ought to fix the proacl/lanacl issues all the way back.
* Accept alternate spellings of __sparcv7 and __sparcv8.Tom Lane2015-08-10
| | | | | Apparently some versions of gcc prefer __sparc_v7__ and __sparc_v8__. Per report from Waldemar Brodkorb.
* Further mucking with PlaceHolderVar-related restrictions on join order.Tom Lane2015-08-10
| | | | | | | | | | | | | | | | | | | Commit 85e5e222b1dd02f135a8c3bf387d0d6d88e669bd turns out not to have taken care of all cases of the partially-evaluatable-PlaceHolderVar problem found by Andreas Seltenreich's fuzz testing. I had set it up to check for risky PHVs only in the event that we were making a star-schema-based exception to the param_source_rels join ordering heuristic. However, it turns out that the problem can occur even in joins that satisfy the param_source_rels heuristic, in which case allow_star_schema_join() isn't consulted. Refactor so that we check for risky PHVs whenever the proposed join has any remaining parameterization. Back-patch to 9.2, like the previous patch (except for the regression test case, which only works back to 9.3 because it uses LATERAL). Note that this discovery implies that problems of this sort could've occurred in 9.2 and up even before the star-schema patch; though I've not tried to prove that experimentally.
* Work around an apparent bug in the Msys DTK perl's regex engine.Andrew Dunstan2015-08-10
| | | | | | | | Several versions of the perl that comes with the Msys DTK have been found to have a bug that fails to recognize a ' before a multiline $ in some circumstances. To work around the problem, use a character class for the '. Another solution would have been to use \n instead of $, but that would have changed the test semantics very slightly.
* Temporarily(?) remove BRIN isolation test.Tom Lane2015-08-10
| | | | | | | | | | | | Commit 2834855cb added a not-very-carefully-thought-out isolation test to check a BRIN index bug fix. The test depended on the availability of the pageinspect contrib module, which meant it did not work in several common testing scenarios such as "make check-world". It's not clear whether we want a core test depending on a contrib module like that, but in any case, failing to deal with the possibility that the module isn't present in the installation-under-test is not acceptable. Remove that test pending some better solution.
* Add confirmed_flush column to pg_replication_slots.Andres Freund2015-08-10
| | | | | | | | | | | | | There's no reason not to expose both restart_lsn and confirmed_flush since they have rather distinct meanings. The former is the oldest WAL still required and valid for both physical and logical slots, whereas the latter is the location up to which a logical slot's consumer has confirmed receiving data. Most of the time a slot will require older WAL (i.e. restart_lsn) than the confirmed position (i.e. confirmed_flush_lsn). Author: Marko Tiikkaja, editorialized by me Discussion: 559D110B.1020109@joh.to
* Fix copy & paste mistake in pg_get_replication_slots().Andres Freund2015-08-10
| | | | | | | | XLogRecPtr was compared with InvalidTransactionId instead of InvalidXLogRecPtr. As both are defined to the same value this doesn't cause any actual problems, but it's still wrong. Backpatch: 9.4-master, bug was introduced in 9.4
* Don't start to stream after pg_receivexlog --create-slot.Andres Freund2015-08-10
| | | | | | | | | | | Immediately starting to stream after --create-slot is inconvenient in a number of situations (e.g. when configuring a slot for use in recovery.conf) and it's easy to just call pg_receivexlog twice in the rest of the cases. Author: Michael Paquier Discussion: CAB7nPqQ9qEtuDiKY3OpNzHcz5iUA+DUX9FcN9K8GUkCZvG7+Ew@mail.gmail.com Backpatch: 9.5, where the option was introduced
* Remove gram.y's precedence declaration for OVERLAPS.Tom Lane2015-08-09
| | | | | | | | | | | | | | | | | The allowed syntax for OVERLAPS, viz "row OVERLAPS row", is sufficiently constrained that we don't actually need a precedence declaration for OVERLAPS; indeed removing this declaration does not change the generated gram.c file at all. Let's remove it to avoid confusion about whether OVERLAPS has precedence or not. If we ever generalize what we allow for OVERLAPS, we might need to put back a precedence declaration for it, but we might want some other level than what it has today --- and leaving the declaration there would just risk confusion about whether that would be an incompatible change. Likewise, remove OVERLAPS from the documentation's precedence table. Per discussion with Noah Misch. Back-patch to 9.5 where we hacked up some nearby precedence decisions.
* docs: update major release notes item checklistBruce Momjian2015-08-08
|
* 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.