- 05 Aug, 2015 5 commits
-
-
Andres Freund authored
Per buildfarm members mandrill and hornet.
-
Andres Freund authored
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 c53f7387, 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
-
Andres Freund authored
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
-
Andres Freund authored
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
-
Tom Lane authored
Per discussion of bug #13538.
-
- 04 Aug, 2015 7 commits
-
-
Tom Lane authored
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.
-
Tom Lane authored
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.
-
Tom Lane authored
In commit b514a746, 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.
-
Robert Haas authored
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.
-
Robert Haas authored
Vik Fearing, reviewed by Brendan Jurd, Michael Paquier, and myself
-
Robert Haas authored
Peter Geoghegan
-
Heikki Linnakangas authored
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.
-
- 03 Aug, 2015 10 commits
-
-
Stephen Frost authored
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.
-
Tom Lane authored
-
Joe Conway authored
psql neglected to wrap parenthesis around USING and WITH CHECK expressions -- fixed. Back-patched to 9.5 where RLS policies were introduced.
-
Fujii Masao authored
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.
-
Heikki Linnakangas authored
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.
-
Heikki Linnakangas authored
Since commit 01f6bb4b, 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 13d856e1, which moved the initdb-running code to TestLib.pm. Backpatch to 9.5.
-
Tom Lane authored
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.
-
Tom Lane authored
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.
-
Andrew Dunstan authored
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.
-
- 02 Aug, 2015 7 commits
-
-
Tom Lane authored
As in commit 0a52d378, 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.
-
Heikki Linnakangas authored
An EAN beginning with 979 (but not 9790 - those are ISMN's) are accepted as ISBN numbers, but they cannot be represented in the old, 10-digit ISBN format. They must be output in the new 13-digit ISBN-13 format. We printed out an incorrect value for those. Also add a regression test, to test this and some other basic functionality of the module. Patch by Fabien Coelho. This fixes bug #13442, reported by B.Z. Backpatch to 9.1, where we started to recognize ISBN-13 numbers.
-
Tom Lane authored
Commit c9b0cbe9 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.
-
Heikki Linnakangas authored
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.
-
Andres Freund authored
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
-
Andres Freund authored
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
-
Tom Lane authored
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.
-
- 01 Aug, 2015 1 commit
-
-
Tom Lane authored
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.
-
- 31 Jul, 2015 3 commits
-
-
Tom Lane authored
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.
-
Noah Misch authored
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.
-
Noah Misch authored
Responsibility was formerly split between Makefile.global and pgxs.mk. As a result of commit b58233c7, 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.
-
- 30 Jul, 2015 7 commits
-
-
Alvaro Herrera authored
They are marked stable, but since they act on instantaneous state and it is possible to consult state of transactions as they commit, the results could change mid-query. They need to be marked volatile, and this commit does so. There would normally be a catversion bump here, but this is so much a niche feature and I don't believe there's real damage from the incorrect marking, that I refrained. Backpatch to 9.5, where commit timestamps where introduced. Per note from Fujii Masao.
-
Alvaro Herrera authored
The code was assuming that any NULL value in scan keys was due to IS NULL or IS NOT NULL, but it turns out to be possible to get them with other operators too, if they are used in contrived-enough ways. Easiest way out of the problem seems to check explicitely for the IS NOT NULL flag, instead of assuming it must be set if the IS NULL flag is not set, when a null scan key is found; if neither flag is set, follow the lead of other index AMs and assume that all indexable operators must be strict, and thus the query is never satisfiable. Also, add a comment to try and lure some future hacker into improving analysis of scan keys in brin. Per report from Andreas Seltenreich; diagnosis by Tom Lane. Backpatch to 9.5. Discussion: http://www.postgresql.org/message-id/20646.1437919632@sss.pgh.pa.us
-
Joe Conway authored
Patch by Dean Rasheed. Back-patched to 9.5 where RLS was introduced.
-
Joe Conway authored
When retrieving policies, if not working on the root target relation, we actually want the relation's SELECT policies, regardless of the top level query command type. For example in UPDATE t1...FROM t2 we need to apply t1's UPDATE policies and t2's SELECT policies. Previously top level query command type was applied to all relations, which was wrong. Add some regression coverage to ensure we don't violate this principle in the future. Report and patch by Dean Rasheed. Cherry picked from larger refactoring patch and tweaked by me. Back-patched to 9.5 where RLS was introduced.
-
Tom Lane authored
Although I think on all modern machines floating division by zero results in Infinity not SIGFPE, we still don't want infinities running around in the planner's costing estimates; too much risk of that leading to insane behavior. grouping_planner() failed to consider the possibility that final_rel might be known dummy and hence have zero rowcount. (I wonder if it would be better to set a rows estimate of 1 for dummy relations? But at least in the back branches, changing this convention seems like a bad idea, so I'll leave that for another day.) Make certain that get_variable_numdistinct() produces a nonzero result. The case that can be shown to be broken is with stadistinct < 0.0 and small ntuples; we did not prevent the result from rounding to zero. For good luck I applied clamp_row_est() to all the nonconstant return values. In ExecChooseHashTableSize(), Assert that we compute positive nbuckets and nbatch. I know of no reason to think this isn't the case, but it seems like a good safety check. Per reports from Piotr Stefaniak. Back-patch to all active branches.
-
Heikki Linnakangas authored
When we loop back to the top of doCustom after processing a backslash command, we must reset the "now" timestamp, because that's used to calculate the time spent executing the previous command. Report and fix by Fabien Coelho. Backpatch to 9.5, where this was broken.
-
Heikki Linnakangas authored
The current version is adding a spurious -pthread option on some Darwin systems that don't need it, which leads to a bunch of "unrecognized option '-pthread'" warnings. There is a proposed fix for that in the upstream autoconf archive's bug tracker, see https://savannah.gnu.org/patch/?8186. This commit updates our version of ax_pthread.m4 to the "draft2" version proposed there by Daniel Richard G. I'm using our buildfarm to help Daniel to test this, before he commits this to the upstream repository.
-