- 10 Aug, 2015 7 commits
-
-
Tom Lane authored
Apparently some versions of gcc prefer __sparc_v7__ and __sparc_v8__. Per report from Waldemar Brodkorb.
-
Tom Lane authored
Commit 85e5e222 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.
-
Andrew Dunstan authored
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.
-
Tom Lane authored
Commit 2834855c 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.
-
Andres Freund authored
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
-
Andres Freund authored
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
-
Andres Freund authored
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
-
- 09 Aug, 2015 5 commits
-
-
Tom Lane authored
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.
-
Magnus Hagander authored
Reported by William Meitzen
-
Bruce Momjian authored
-
Tatsuo Ishii authored
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 07af5238. Back patched to 9.5 stable tree.
-
Bruce Momjian authored
Report by Dean Rasheed Patch by Dean Rasheed Backpatch through 9.5
-
- 08 Aug, 2015 2 commits
-
-
Bruce Momjian authored
-
Bruce Momjian authored
Report by Robert Haas, Andres Freund Backpatch through 9.5
-
- 07 Aug, 2015 10 commits
-
-
Andres Freund authored
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
-
Andres Freund authored
A removed check in ba3deeef 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
-
Tom Lane authored
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.)
-
Robert Haas authored
Spotted by Antonin Houska.
-
Andres Freund authored
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 de6fd1c8 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
-
Andres Freund authored
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.
-
Bruce Momjian authored
Report by Andres Freund and Jeff Janes Backpatch through 9.5
-
Tom Lane authored
Commit 9e7e29c7 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.
-
Bruce Momjian authored
Report by Peter Geoghegan Backpatch through 9.5
-
Tom Lane authored
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.
-
- 06 Aug, 2015 11 commits
-
-
Bruce Momjian authored
Report by Andres Freund Backpatch through 9.5
-
Bruce Momjian authored
Report by Andres Freund Backpatch through 9.5
-
Bruce Momjian authored
Report by Dean Rasheed Backpatch through 9.5
-
Tom Lane authored
Further testing revealed that commit f69b4b94 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.
-
Robert Haas authored
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
-
Robert Haas authored
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.
-
Kevin Grittner authored
Commit e5550d5f 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.
-
Andres Freund authored
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.
-
Bruce Momjian authored
-
Noah Misch authored
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.
-
Noah Misch authored
Commit 0ffc201a 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.
-
- 05 Aug, 2015 5 commits
-
-
Andrew Dunstan authored
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.
-
Andrew Dunstan authored
These were causing spurious test failures.
-
Alvaro Herrera authored
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.
-
Tom Lane authored
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.
-
Andres Freund authored
Per buildfarm members mandrill and hornet.
-