- 04 Jun, 2015 7 commits
-
-
Tom Lane authored
Verify that the number of matches is exactly what it should be, not just that it not be zero. This should help us detect any environment-dependent issues. Also, verify that we're getting the expected type of scan plan (either bitmap or seqscan as appropriate). Right now, this is failing on the cidrcol test cases, as shown in the output file. I'll look into that in a bit, but it seems good to commit this as-is temporarily to verify that it behaves as expected on the buildfarm.
-
Tom Lane authored
Casting to char, without quotes, does not give the same results as casting to "char". That meant we were not testing the brin "char" paths at all, since we ended up with a text operator not a "char" operator.
-
Tom Lane authored
This test used seqscans on tenk1, with LIMIT, to build test data. That works most of the time, but if the synchronized-seqscan logic kicks in, we get varying test data. This seems likely to explain the erratic test failures on buildfarm member chipmunk, which uses smaller-than-default shared_buffers. To fix, add ORDER BY clauses to force the ordering to be what it was implicitly being assumed to be. Peter Geoghegan had noticed this with respect to one of the trouble spots, though not the ones actually causing the chipmunk issue.
-
Tom Lane authored
Some recent buildfarm failures can be explained by supposing that autovacuum or autoanalyze fired on the tables created by this test, resulting in plan changes. Do a proactive VACUUM ANALYZE on the test's principal tables to try to forestall such changes.
-
Fujii Masao authored
The commit c22ed3d5 turned the -i/--ignore-version options into no-ops and marked as deprecated. Considering we shipped that in 8.4, it's time to remove all trace of those switches, per discussion. We'd still have to wait a couple releases before it'd be safe to use -i for something else, but it'd be a start.
-
Fujii Masao authored
- Correct the name of directory which those catalog columns allow to be shrunk. - Correct the name of symbol which is used as the value of pg_class.relminmxid when the relation is not a table. - Fix "ID ID" typo. Backpatch to 9.3 where those cataog columns were introduced.
-
Peter Eisentraut authored
Because of a bug in the DocBook XSL FO style sheet, an xref to a varlistentry whose term includes an indexterm fails to build. One such instance was introduced in commit 5086dfce. Fix by adding the upstream bug fix to our customization layer.
-
- 03 Jun, 2015 3 commits
-
-
Tom Lane authored
add_path_precheck was doing exact comparisons of path costs, but it really needs to do them fuzzily to be sure it won't reject paths that could survive add_path's comparisons. (This can only matter if the initial cost estimate is very close to the final one, but that turns out to often be true.) Also, it should ignore startup cost for this purpose if and only if compare_path_costs_fuzzily would do so. The previous coding always ignored startup cost for parameterized paths, which is wrong as of commit 3f59be83; it could result in improper early rejection of paths that we care about for SEMI/ANTI joins. It also always considered startup cost for unparameterized paths, which is just as wrong though the only effect is to waste planner cycles on paths that can't survive. Instead, it should consider startup cost only when directed to by the consider_startup/ consider_param_startup relation flags. Likewise, compare_path_costs_fuzzily should have symmetrical behavior for parameterized and unparameterized paths. In this case, the best answer seems to be that after establishing that total costs are fuzzily equal, we should compare startup costs whether or not the consider_xxx flags are on. That is what it's always done for unparameterized paths, so let's make the behavior for parameterized paths match. These issues were noted while developing the SEMI/ANTI join costing fix of commit 3f59be83, but we chose not to back-patch these fixes, because they can cause changes in the planner's choices among nearly-same-cost plans. (There is in fact one minor change in plan choice within the core regression tests.) Destabilizing plan choices in back branches without very clear improvements is frowned on, so we'll just fix this in HEAD.
-
Tom Lane authored
When the inner side of a nestloop SEMI or ANTI join is an indexscan that uses all the join clauses as indexquals, it can be presumed that both matched and unmatched outer rows will be processed very quickly: for matched rows, we'll stop after fetching one row from the indexscan, while for unmatched rows we'll have an indexscan that finds no matching index entries, which should also be quick. The planner already knew about this, but it was nonetheless charging for at least one full run of the inner indexscan, as a consequence of concerns about the behavior of materialized inner scans --- but those concerns don't apply in the fast case. If the inner side has low cardinality (many matching rows) this could make an indexscan plan look far more expensive than it actually is. To fix, rearrange the work in initial_cost_nestloop/final_cost_nestloop so that we don't add the inner scan cost until we've inspected the indexquals, and then we can add either the full-run cost or just the first tuple's cost as appropriate. Experimentation with this fix uncovered another problem: add_path and friends were coded to disregard cheap startup cost when considering parameterized paths. That's usually okay (and desirable, because it thins the path herd faster); but in this fast case for SEMI/ANTI joins, it could result in throwing away the desired plain indexscan path in favor of a bitmap scan path before we ever get to the join costing logic. In the many-matching-rows cases of interest here, a bitmap scan will do a lot more work than required, so this is a problem. To fix, add a per-relation flag consider_param_startup that works like the existing consider_startup flag, but applies to parameterized paths, and set it for relations that are the inside of a SEMI or ANTI join. To make this patch reasonably safe to back-patch, care has been taken to avoid changing the planner's behavior except in the very narrow case of SEMI/ANTI joins with inner indexscans. There are places in compare_path_costs_fuzzily and add_path_precheck that are not terribly consistent with the new approach, but changing them will affect planner decisions at the margins in other cases, so we'll leave that for a HEAD-only fix. Back-patch to 9.3; before that, the consider_startup flag didn't exist, meaning that the second aspect of the patch would be too invasive. Per a complaint from Peter Holzer and analysis by Tomas Vondra.
-
Fujii Masao authored
Michael Paquier, reviewed by Christoph Berg and Naoya Anzai
-
- 01 Jun, 2015 5 commits
-
-
Tom Lane authored
Also sneak entries for commits 97ff2a564 et al into the sections for the previous releases in the relevant branches. Those fixes did go out in the previous releases, but missed getting documented.
-
Bruce Momjian authored
-
Andrew Dunstan authored
Per gripe from Tom Lane.
-
Andrew Dunstan authored
-
Andrew Dunstan authored
The function is given a fourth parameter, which defaults to true. When this parameter is true, if the last element of the path is missing in the original json, jsonb_set creates it in the result and assigns it the new value. If it is false then the function does nothing unless all elements of the path are present, including the last. Based on some original code from Dmitry Dolgov, heavily modified by me. Catalog version bumped.
-
- 31 May, 2015 1 commit
-
-
Peter Eisentraut authored
Newer Python versions randomize the hash seed for dictionaries, resulting in a random output order, which messes up the regression test diffs. Instead, use Python assert to compare the dictionaries with their expected value.
-
- 29 May, 2015 6 commits
-
-
Bruce Momjian authored
-
Tom Lane authored
The fsync code from the backend essentially assumes that somebody's already validated PGDATA, at least to the extent of it being a readable directory. That's safe enough for initdb's normal code path too, but "initdb -S" doesn't have any other processing at all that touches the target directory. To have reasonable error-case behavior, add a pg_check_dir call. Per gripe from Peter E.
-
Tom Lane authored
The argument that this is a sufficiently-expected case to be silently ignored seems pretty thin. Andres had brought it up back when we were still considering that most fsync failures should be hard errors, and it probably would be legit not to fail hard for ETXTBSY --- but the same is true for EROFS and other cases, which is why we gave up on hard failures. ETXTBSY is surely not a normal case, so logging the failure seems fine from here.
-
Tom Lane authored
opr_sanity.sql has a test checking that relevant properties of built-in functions match when the same C function is referenced by multiple pg_proc entries. The test neglected to check proleakproof, though, and when I added that condition it exposed that xideqint4 hadn't been updated to match xideq. So fix that as well, and in consequence bump catversion. This isn't very critical, so no need to worry about fixing back branches.
-
Tom Lane authored
Make initdb's version of this logic look as much like the backend's as possible. This is much less critical than in the backend since not so many people use "initdb -S", but we want the same corner-case error handling in both cases. Back-patch to 9.3 where initdb -S option was introduced. Before that, initdb only had to deal with freshly-created data directories, wherein no failures should be expected. Abhijit Menon-Sen
-
Tom Lane authored
This undoes a poorly-thought-out choice in commit 970a1868, namely to export guc.c's internal variable data_directory. The authoritative variable so far as C code is concerned is DataDir; there is no reason for anything except specific bits of GUC code to look at the GUC variable. After yesterday's commits fixing the fsync-on-restart patch, the only remaining misuse of data_directory was in AlterSystemSetConfigFile(), which would be much better off just using a relative path anyhow: it's less code and it doesn't break if the DBA moves the data directory of a running system, which is a case we've taken some pains over in the past. This is mostly cosmetic, so no need for a back-patch (and I'd be hesitant to remove a global variable in stable branches anyway).
-
- 28 May, 2015 7 commits
-
-
Tom Lane authored
Commit 2ce439f3 introduced a rather serious regression, namely that if its scan of the data directory came across any un-fsync-able files, it would fail and thereby prevent database startup. Worse yet, symlinks to such files also caused the problem, which meant that crash restart was guaranteed to fail on certain common installations such as older Debian. After discussion, we agreed that (1) failure to start is worse than any consequence of not fsync'ing is likely to be, therefore treat all errors in this code as nonfatal; (2) we should not chase symlinks other than those that are expected to exist, namely pg_xlog/ and tablespace links under pg_tblspc/. The latter restriction avoids possibly fsync'ing a much larger part of the filesystem than intended, if the user has left random symlinks hanging about in the data directory. This commit takes care of that and also does some code beautification, mainly moving the relevant code into fd.c, which seems a much better place for it than xlog.c, and making sure that the conditional compilation for the pre_sync_fname pass has something to do with whether pg_flush_data works. I also relocated the call site in xlog.c down a few lines; it seems a bit silly to be doing this before ValidateXLOGDirectoryStructure(). The similar logic in initdb.c ought to be made to match this, but that change is noncritical and will be dealt with separately. Back-patch to all active branches, like the prior commit. Abhijit Menon-Sen and Tom Lane
-
Stephen Frost authored
Fixes the docs build.
-
Stephen Frost authored
-
Tom Lane authored
The previous coding suffered a null-pointer dereference if it found any symlink at the top level of $PGDATA. Fix that, and teach it to recurse into a symlink for pg_xlog, but not anything else. Per note from Abhijit Menon-Sen.
-
Stephen Frost authored
This removes pg_audit, per discussion: 20150528082038.GU26667@tamriel.snowman.net
-
Tom Lane authored
Ensure that we null-terminate the result string (one place in pg_rewind). Be paranoid about out-of-range results from readlink() (should not happen, but there is no good reason for some call sites to be careful about it and others not). Consistently use the whole buffer, not sometimes one byte less. Ensure we emit an appropriate errcode() in all cases. Spell the error messages the same way. The only serious bug here is the missing null-termination in pg_rewind, which is new code, so no need for a back-patch. Abhijit Menon-Sen and Tom Lane
-
Tom Lane authored
Seems to have been an oversight in the original leakproofness patch. Per report and patch from Jeevan Chalke. In passing, prettify some awkward leakproof-related code in AlterFunction.
-
- 27 May, 2015 4 commits
-
-
Tom Lane authored
specparse.y and specscanner.l used "string" as a token name. Now, bison likes to define each token name as a macro for the token code it assigns, which means those names are basically off-limits for any other use within the grammar file or included headers. So names as generic as "string" are dangerous. This is what was causing the recent failures on protosciurus: some versions of Solaris' sys/kstat.h use "string" as a field name. With late-model bison we don't see this problem because the token macros aren't defined till later (that is why castoroides didn't show the problem even though it's on the same machine). But protosciurus uses bison 1.875 which defines the token macros up front. This land mine has been there from day one; we'd have found it sooner except that protosciurus wasn't trying to run the isolation tests till recently. To fix, rename the token to "string_literal" which is hopefully less likely to collide with names used by system headers. Back-patch to all branches containing the isolation tests.
-
Andrew Dunstan authored
This reverts commit 54547bd8. This appears to have been a thinko on my part. I will try to come up wioth a better solution.
-
Andrew Dunstan authored
This reverts commit fba12c8c. This relied on a commit that is also being reverted.
-
Tom Lane authored
According to recent tests, this case now works fine, so there's no reason to reject it anymore. (Even if there are still some OpenBSD platforms in the wild where it doesn't work, removing the check won't break any case that worked before.) We can actually remove the entire test that discovers whether libpython is threaded, since without the OpenBSD case there's no need to know that at all. Per report from Davin Potts. Back-patch to all active branches.
-
- 26 May, 2015 4 commits
-
-
Tom Lane authored
brin.sql included a call of brin_summarize_new_values(), and expected it to always report exactly 5 summarization events. This failed sometimes during parallel regression tests, as a consequence of the database-wide VACUUM in gist.sql getting there first. The most future-proof way to avoid variation in the test results is to forget about using brin_summarize_new_values() and just do a plain "VACUUM brintest", which will exercise the same code anyway. Having done that, there's no need for preventing autovacuum on brintest; doing so just reduces the scope of test coverage, so let's not.
-
Andrew Dunstan authored
This function no longer needs to walk non-scalar structures passed to it, following commit 54547bd8.
-
Andrew Dunstan authored
Commit 9b74f32cdbff8b9be47fc69164eae552050509ff did this for objects of type jbvBinary, but in trying further to simplify some of the new jsonb code I discovered that objects of type jbvObject or jbvArray passed as WJB_ELEM or WJB_VALUE also caused problems. These too are now added component by component. Backpatch to 9.4.
-
Tom Lane authored
brin_form_tuple calculated an exact tuple size, then palloc'd and filled just that much. Later, brin_doinsert or brin_doupdate would MAXALIGN the tuple size and tell PageAddItem that that was the size of the tuple to insert. If the original tuple size wasn't a multiple of MAXALIGN, the net result would be that PageAddItem would memcpy a few more bytes than the palloc request had been for. AFAICS, this is totally harmless in the real world: the error is a read overrun not a write overrun, and palloc would certainly have rounded the request up to a MAXALIGN multiple internally, so there's no chance of the memcpy fetching off the end of memory. Valgrind, however, is picky to the byte level not the MAXALIGN level. Fix it by pushing the MAXALIGN step back to brin_form_tuple. (The other possible source of tuples in this code, brin_form_placeholder_tuple, was already producing a MAXALIGN'd result.) In passing, be a bit more paranoid about internal allocations in brin_form_tuple.
-
- 25 May, 2015 3 commits
-
-
Bruce Momjian authored
-
Tom Lane authored
The existing documentation could easily be misinterpreted, and it failed to explain the inconsistent-evaluation hazard that deterred us from supporting automatic importing of check constraints. Revise it. Etsuro Fujita, further expanded by me
-
Alvaro Herrera authored
Multixact truncation is now handled differently, and this file hadn't gotten the memo. Per note from Amit Langote. I didn't use his patch, though. Also update the description of infomask bits, which weren't completely up to date either. This commit also propagates b01a4f68 back to 9.3 and 9.4, which apparently I failed to do back then.
-