- 11 Dec, 2018 2 commits
-
-
Tom Lane authored
Unlike other ALTER ref pages, this one neglected to mention that ALTER OWNER requires being a member of the new owning role. Per bug #15546 from Stefan Kadow. Discussion: https://postgr.es/m/15546-0558c75fd2025e7c@postgresql.org
-
Noah Misch authored
Slow runs of buildfarm members chipmunk, hornet and mandrill saw the shorter timeouts expire. The 180s timeout in poll_query_until has been trouble-free since 2a0f89cd introduced it two years ago, so use 180s more widely. Back-patch to 9.6, where the first of these timeouts was introduced. Reviewed by Michael Paquier. Discussion: https://postgr.es/m/20181209001601.GC2973271@rfd.leadboat.com
-
- 10 Dec, 2018 7 commits
-
-
Tom Lane authored
Although copyfuncs.c has a check_stack_depth call in its recursion, equalfuncs.c, outfuncs.c, and readfuncs.c lacked one. This seems unwise. Likewise fix planstate_tree_walker(), in branches where that exists. Discussion: https://postgr.es/m/30253.1544286631@sss.pgh.pa.us
-
Tom Lane authored
John Naylor Discussion: https://postgr.es/m/CAJVSVGUPH-0q5feP4v9b+Y8K3HGTn3bEd5KV7VbyUj-oFdSLzA@mail.gmail.com
-
Tom Lane authored
Previously, it would just pass back a partially-uninitialized tupdesc, which doesn't seem like a safe or useful behavior. Backpatch to v10 where this code came in. Discussion: https://postgr.es/m/30830.1544384975@sss.pgh.pa.us
-
Stephen Frost authored
This adds a pg_dump test for an empty operator class. Author: Michael Paquier Discussion: https://postgr.es/m/20181208011142.GU3415@tamriel.snowman.net
-
Stephen Frost authored
This adds a few tests for non-inherited constraints. Author: Amit Langote Discussion: https://postgr.es/m/20181208001735.GT3415%40tamriel.snowman.net
-
Stephen Frost authored
In toast_fetch_datum_slice(), we Assert() that what is passed in isn't compressed, but we then later had a check to see what the length of if what was passed in is compressed. That later check is rather confusing since toast_fetch_datum_slice() is only ever called with non-compressed datums and the Assert() earlier makes it clear that one shouldn't be passing in compressed datums. Add a comment to make it clear that toast_fetch_datum_slice() is just for non-compressed datums, and remove the dead code.
-
Michael Paquier authored
When a WAL segment is recycled, its ".ready" and ".done" status files get also automatically removed, however this is not done in a durable manner. Hence, in a subsequent crash, it could be possible that a ".ready" status file is still around with its corresponding segment already gone. If the backend reaches such a state, the archive command would most likely complain about a segment non-existing and would keep retrying, causing WAL segments to bloat pg_wal/, potentially making Postgres crash hard when running out of space. As status files are removed after each individual segment, using durable_unlink() does not completely close the window either, as a crash could happen between the moment the WAL segment is recycled and the moment its status files are removed. This has also some performance impact with the additional fsync() calls needed to make the removal in a durable manner. Doing the cleanup at recovery is not cost-free either as this makes crash recovery potentially take longer than necessary. So, instead, as per an idea of Stephen Frost, make the archiver aware of orphan status files and remove them on-the-fly if the corresponding segment goes missing. Removal failures follow a model close to what happens for WAL segments, where multiple attempts are done before giving up temporarily, and where a successful orphan removal makes the archiver move immediately to the next WAL segment thought as ready to be archived. Author: Michael Paquier Reviewed-by: Nathan Bossart, Andres Freund, Stephen Frost, Kyotaro Horiguchi Discussion: https://postgr.es/m/20180928032827.GF1500@paquier.xyz
-
- 09 Dec, 2018 1 commit
-
-
Michael Paquier authored
The timestamp generated by the standby at message transmission has been included in the protocol since its introduction for both the status update message and hot standby feedback message, but it has never appeared in pg_stat_replication. Seeing this timestamp does not matter much with a cluster which has a lot of activity, but on a mostly-idle cluster, this makes monitoring able to react faster than the configured timeouts. Author: MyungKyu LIM Reviewed-by: Michael Paquier, Masahiko Sawada Discussion: https://postgr.es/m/1657809367.407321.1533027417725.JavaMail.jboss@ep2ml404
-
- 07 Dec, 2018 6 commits
-
-
Tom Lane authored
This restriction is implicit in the check-only-once implementation we use for table and domain constraints, but it wasn't spelled out anywhere, nor was there any advice about how to alter a constraint's behavior safely. Improve that. I was also dissatisfied with the documentation of ALTER DOMAIN VALIDATE CONSTRAINT, which entirely failed to explain the use of that feature; and thence decided that ALTER TABLE VALIDATE CONSTRAINT could be documented better as well. Perhaps we should back-patch this, along with the related commit 36d442a2, but for now I refrained. Discussion: https://postgr.es/m/12539.1544107316@sss.pgh.pa.us
-
Tom Lane authored
This is an astonishingly ancient bit of silliness, dating AFAICS to commit edb519b1 of 27-Jul-1996 which added the pipe close stanza in the wrong place. It happens to be harmless given that the code above this won't enable the pager if html3 output mode is selected. Still, somebody might try to relax that restriction someday, and in any case it could confuse readers and static analysis tools, so let's fix it in HEAD. Per bug #15541 from Pan Bian. Discussion: https://postgr.es/m/15541-c835d8b9a903f7ad@postgresql.org
-
Tom Lane authored
The stanza of ExecuteTruncate[Guts] that truncates a target table's toast relation re-used the loop local variable "rel" to reference the toast rel. This was safe enough when written, but commit d42358ef added code below that that supposed "rel" still pointed to the parent table. Therefore, the stats counter update was applied to the wrong relcache entry (the toast rel not the user rel); and if we were unlucky and that relcache entry had been flushed during reindex_relation, very bad things could ensue. (I'm surprised that CLOBBER_CACHE_ALWAYS testing hasn't found this. I'm even more surprised that the problem wasn't detected during the development of d42358ef; it must not have been tested in any case with a toast table, as the incorrect stats counts are very obvious.) To fix, replace use of "rel" in that code branch with a more local variable. Adjust test cases added by d42358ef so that some of them use tables with toast tables. Per bug #15540 from Pan Bian. Back-patch to 9.5 where d42358ef came in. Discussion: https://postgr.es/m/15540-01078812338195c0@postgresql.org
-
Tom Lane authored
Remove dead code (which would be incorrect if it weren't dead), per report from Pan Bian. Add a CHECK_FOR_INTERRUPTS in the inner loop over child relations, because there's little point in having one in the outer loop if there's not one here too. Minor stylistic adjustments and comment improvements. Seems to be aboriginal to this code (cf commit 665d1fad). Back-patch to v10 where that came in, not because any of this is significant, but just to keep the branches looking similar. Discussion: https://postgr.es/m/15539-06d00ef6b1e2e1bb@postgresql.org
-
Tom Lane authored
Jeff Janes Discussion: https://postgr.es/m/CAMkU=1w7Tn2M9BhK+rt8Shtz1AkU+ty7By8gj5C==z65=U4vyQ@mail.gmail.com
-
- 06 Dec, 2018 6 commits
-
-
Michael Paquier authored
As per the error message style guide of the documentation, those should be full sentences. Author: Daniel Gustafsson Reviewed-by: Michael Paquier, Álvaro Herrera Discussion: https://1E8D49B4-16BC-4420-B4ED-58501D9E076B@yesql.se
-
Tom Lane authored
Places that are testing for *printf failure ought to include the format string in their error reports, since bad-format-string is one of the more likely causes of such failure. This both makes it easier to find and repair the mistake, and provides at least some useful info to the user who stumbles across such a problem. Also, tighten snprintf.c to report EINVAL for an invalid flag or final character in a format %-spec (including the case where the %-spec is missing a final character altogether). This seems like better project policy, and it also allows removing an instruction or two from the hot code path. Back-patch the error reporting change in pvsnprintf, since it should be harmless and may be helpful; but not the snprintf.c change. Per discussion of bug #15511 from Ertuğrul Kahveci, which reported an invalid translated format string. These changes don't fix that error, but they should improve matters next time we make such a mistake. Discussion: https://postgr.es/m/15511-1d8b6a0bc874112f@postgresql.org
-
Stephen Frost authored
It was pointed out that in the planner stats documentation under Extended Statistics, one of the sentences was a bit awkward. Improve that by rewording it slightly. Discussion: https://postgr.es/m/154409976780.14137.2785644488950047100@wrigleys.postgresql.org
-
Stephen Frost authored
In dumputils, we may have successfully parsed the acls when we discover that we can't parse the reverse ACLs and then return- check and free aclitems if that happens. In dumpTableSchema, move ftoptions and srvname under the relkind != RELKIND_VIEW branch (since they're only used there) and then check if they've been allocated and, if so, free them at the end of that block. Pointed out by Pavel Raiskup, though I didn't use those patches. Discussion: https://postgr.es/m/2183976.vkCJMhdhmF@nb.usersys.redhat.com
-
Stephen Frost authored
Skipping over the "hole" in full page images in the XLOG code was described as being a form of compression, but this got a bit confusing since we now have PGLZ-based compression happening, so adjust the wording to discuss "removing" the "hole" and keeping the talk about compression to where we're talking about using PGLZ-based compression of the full page images. Reviewed-By: Kyotaro Horiguchi Discussion: https://postgr.es/m/20181127234341.GM3415@tamriel.snowman.net
-
Tatsuo Ishii authored
We prefer to use on/off than true/false for boolean configuration parameters in the documentation, but there were a few places where true/false were still used. Dicussion: https://postgr.es/m/20181202.072508.618341295047874293.t-ishii%40sraoss.co.jp
-
- 05 Dec, 2018 2 commits
-
-
Alvaro Herrera authored
When an indexes is created on a partitioned table using ONLY (don't recurse to partitions), it gets marked invalid until index partitions are attached for each table partition. But there's no reason to do this if there are no partitions ... and moreover, there's no way to get the index to become valid afterwards, because all partitions that get created/attached get their own index partition already attached to the parent index, so there's no chance to do ALTER INDEX ... ATTACH PARTITION that would make the parent index valid. Fix by not marking the index as invalid to begin with. This is very similar to 9139aa19, but the pg_dump aspect does not appear to be relevant until we add FKs that can point to PKs on partitioned tables. (I tried to cause the pg_upgrade test to break by leaving some of these bogus tables around, but wasn't able to.) Making this change means that an index that was supposed to be invalid in the insert_conflict regression test is no longer invalid; reorder the DDL so that the test continues to verify the behavior we want it to. Author: Álvaro Herrera Reviewed-by: Amit Langote Discussion: https://postgr.es/m/20181203225019.2vvdef2ybnkxt364@alvherre.pgsql
-
Michael Paquier authored
"remote_flush" has never been a valid user-facing value, but "on" is. Author: Maksim Milyutin Discussion: https://postgr.es/m/27b3b80c-3615-2d76-02c5-44566b53136c@gmail.com
-
- 04 Dec, 2018 2 commits
-
-
Stephen Frost authored
Backends don't typically exist uncleanly, but they can certainly exit uncleanly, and it's exiting uncleanly that's being discussed here.
-
Etsuro Fujita authored
In commit 7012b132, which added aggregate pushdown to postgres_fdw, we didn't account for the evaluation cost and the selectivity of HAVING quals attached to ForeignPaths performing aggregate pushdown, as core had never accounted for that for AggPaths and GroupPaths. And we didn't set these values of the locally-checked quals (ie, fpinfo's local_conds_cost and local_conds_sel), which were initialized to zeros, but since estimate_path_cost_size factors in these to estimate the result size and the evaluation cost of such a ForeignPath when the use_remote_estimate option is enabled, this caused it to produce underestimated results in that case. By commit 7b6c0754 core was changed so that it accounts for the evaluation cost and the selectivity of HAVING quals in aggregation paths, so change the postgres_fdw's aggregate pushdown code as well as such. This not only fixes the underestimation issue mentioned above, but improves the estimation using local statistics in that function when that option is disabled. This would be a bug fix rather than an improvement, but apply it to HEAD only to avoid destabilizing existing plan choices. Author: Etsuro Fujita Discussion: https://postgr.es/m/5BFD3EAD.2060301%40lab.ntt.co.jp
-
- 03 Dec, 2018 3 commits
-
-
Tom Lane authored
Expand section 5.6 "Privileges" to include the full definition of each privilege type, and an explanation of aclitem privilege displays, along with some helpful summary tables. Most of this material came out of the GRANT reference page, although some of it is new. Adjust a bunch of links that were pointing to GRANT to point to 5.6. Fabien Coelho and Tom Lane, reviewed by Bradley DeJong Discussion: https://postgr.es/m/alpine.DEB.2.21.1807311735200.20743@lancre
-
Michael Paquier authored
This does not improve the security and reliability of the touched areas, but it makes the style more consistent. Author: Michael Paquier Reviewed-by- Noah Misch Discussion: https://postgr.es/m/20180309075538.GD9376@paquier.xyz
-
Michael Paquier authored
The following options are added for extensions: - TAP_TESTS, to allow an extention to run TAP tests which are the ones present in t/*.pl. A subset of tests can always be run with the existing PROVE_TESTS for developers. - ISOLATION, to define a list of isolation tests. - ISOLATION_OPTS, to pass custom options to isolation_tester. A couple of custom Makefile rules have been accumulated across the tree to cover the lack of facility in PGXS for a couple of releases when using those test suites, which are all now replaced with the new flags, without reducing the test coverage. Note that tests of contrib/bloom/ are not enabled yet, as those are proving unstable in the buildfarm. Author: Michael Paquier Reviewed-by: Adam Berlin, Álvaro Herrera, Tom Lane, Nikolay Shaplov, Arthur Zakirov Discussion: https://postgr.es/m/20180906014849.GG2726@paquier.xyz
-
- 01 Dec, 2018 3 commits
-
-
Tom Lane authored
Re-making ecpglib's typename.o is dangerous because another make thread could be doing that at the same time. While we've not heard field complaints traceable to this, it seems inevitable that it'd bite someone eventually. Instead, symlink typename.c into the preproc directory and recompile it there. That file is small enough that compiling it twice isn't much of a penalty. Furthermore, this way we get a .o file that's made without shlib CFLAGS, which seems cleaner. This requires adding more stuff to the module's -I list. The MSVC aspect of that is untested, but I'm sure the buildfarm will tell me if I got it wrong. Per a suggestion from Peter Eisentraut. Although this is theoretically a bug fix, the lack of field reports makes me feel we needn't back-patch. Discussion: https://postgr.es/m/31364.1543511708@sss.pgh.pa.us
-
Tom Lane authored
This should reduce confusion, and in particular make it safe to copy typename.c into preproc/ and compile it there. This doesn't affect anything outside ecpg, and particularly not end users, because these files don't get installed; they just exist to share declarations among the .c files of each subdirectory. Discussion: https://postgr.es/m/31364.1543511708@sss.pgh.pa.us
-
Tom Lane authored
This allows control of the directory in which the postmaster sockets are created for the temporary postmasters started by pg_upgrade. The default location remains the current working directory, which is typically fine, but if it is deeply nested then its pathname might be too long to be a socket name. In passing, clean up some messiness in pg_upgrade's option handling, particularly the confusing and undocumented way that configuration-only datadirs were handled. And fix check_required_directory's substantially under-baked cleanup of directory pathnames. Daniel Gustafsson, reviewed by Hironobu Suzuki, some code cleanup by me Discussion: https://postgr.es/m/E72DD5C3-2268-48A5-A907-ED4B34BEC223@yesql.se
-
- 30 Nov, 2018 5 commits
-
-
Michael Paquier authored
TAP tests on msys need to run with the DTK perl, which understands msys virtualized paths. Postgres, however, does not understand such paths, so before a path can be used safely with CREATE TABLESPACE, it needs to be translated into a path on the underlying file system. Per report from buildfarm member jacana. Suggested fix is from Andrew Dunstan. Discussion: https://postgr.es/m/20181130053555.GF2267@paquier.xyz
-
Alvaro Herrera authored
My original coding was questionable anyway. Reported-by: Sergei Kornilov Discussion: https://postgr.es/m/9645101543575886@myt6-27270b78ac4f.qloud-c.yandex.net
-
Amit Kapila authored
-
Michael Paquier authored
Three issues are fixed in this patch: - Base backups forgot to ignore files specific to EXEC_BACKEND, leading to spurious warnings when checksums are enabled, per analysis from me. - pg_verify_checksums forgot about files specific to EXEC_BACKEND, leading to failures of the tool on any such build, particularly Windows. This error was originally found by newly-introduced TAP tests in various buildfarm members using EXEC_BACKEND. - pg_verify_checksums forgot to count for temporary files and temporary paths, which could be valid relation files, without checksums, per report from Andres Freund. More tests are added to cover this case. A new test case which emulates corruption for a file in a different tablespace is added, coming from from Michael Banck, while I have coded the main code and refactored the test code. Author: Michael Banck, Michael Paquier Reviewed-by: Stephen Frost, David Steele Discussion: https://postgr.es/m/20181021134206.GA14282@paquier.xyz
-
Michael Paquier authored
This basically reverts commit d55241af, leaving around a portion of the regression tests still adapted with empty relation files, and corrupted cases. This is also proving to be failing to check properly relation files located in a non-default tablespace path. Per discussion with various folks, including Stephen Frost, David Steele, Andres Freund, Michael Banck and myself. Reported-by: Michael Banck Discussion: https://postgr.es/m/20181021134206.GA14282@paquier.xyz Backpatch-through: 11
-
- 29 Nov, 2018 3 commits
-
-
Tom Lane authored
The source code comments documented this, but the user-facing docs, not so much. Add a section to Appendix B that discusses it. In passing, improve a couple other things in Appendix B --- notably, a long-obsolete claim that time zone abbreviations are looked up in a fixed table. Per bug #15527 from Michael Davidson. Discussion: https://postgr.es/m/15527-f1be0b4dc99ebbe7@postgresql.org
-
Alvaro Herrera authored
This allows to set a lower log_min_duration_statement value without incurring excessive log traffic (which reduces performance). This can be useful to analyze workloads with lots of short queries. Author: Adrien Nayrat Reviewed-by: David Rowley, Vik Fearing Discussion: https://postgr.es/m/c30ee535-ee1e-db9f-fa97-146b9f62caed@anayrat.info
-
Tom Lane authored
In at least Apple's version of ranlib, the output file is updated to have a mod time equal to the max of the timestamps of its components, and that data only has seconds precision. On a filesystem with sub-second file timestamp precision --- say, APFS --- this can result in the finished static library appearing older than its input files, which causes useless rebuilds and possible outright failures in parallel makes. We've only seen this reported in the field from people using Apple's ranlib with a non-Apple make, because Apple's make doesn't know about sub-second timestamps either so it doesn't decide rebuilds are needed. But Apple's ranlib presumably shares code with at least some BSDen, so it's not that unlikely that the same problem could arise elsewhere. To fix, just "touch" the output file after ranlib finishes. We seem to need this in only one place. There are other calls of ranlib in our makefiles, but they are working on intermediate files whose timestamps are not actually important, or else on an installed static library for which sub-second timestamp precision is unlikely to matter either. (Also, so far as I can tell, Apple's ranlib doesn't mess up the file timestamp in the latter usage anyhow.) In passing, change "ranlib" to "$(RANLIB)" in one place that was bypassing the make macro for no good reason. Per bug #15525 from Jack Kelly (via Alyssa Ross). Back-patch to all supported branches. Discussion: https://postgr.es/m/15525-a30da084f17a1faa@postgresql.org
-