aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* Support tcp_keepalives_idle option on Solaris.Tom Lane2017-06-27
| | | | | | | | | | | | | | | | Turns out that the socket option for this is named TCP_KEEPALIVE_THRESHOLD, at least according to the tcp(7P) man page for Solaris 11. (But since that text refers to "SunOS", it's likely pretty ancient.) It appears that the symbol TCP_KEEPALIVE does get defined on that platform, but it doesn't seem to represent a valid protocol-level socket option. This leads to bleats in the postmaster log, and no tcp_keepalives_idle functionality. Per bug #14720 from Andrey Lizenko, as well as an earlier report from Dhiraj Chawla that nobody had followed up on. The issue's been there since we added the TCP_KEEPALIVE code path in commit 5acd417c8, so back-patch to all supported branches. Discussion: https://postgr.es/m/20170627163757.25161.528@wrigleys.postgresql.org
* Re-allow SRFs and window functions within sub-selects within aggregates.Tom Lane2017-06-27
| | | | | | | | | | | check_agg_arguments_walker threw an error upon seeing a SRF or window function, but that is too aggressive: if the function is within a sub-select then it's perfectly fine. I broke the SRF case in commit 0436f6bde by copying the logic for window functions ... but that was broken too, and had been since commit eaccfded9. Repair both cases in HEAD, and the window function case back to 9.3. 9.2 gets this right.
* Reduce wal_retrieve_retry_interval in applicable TAP tests.Tom Lane2017-06-26
| | | | | | | | | | | | | | | | By default, wal_retrieve_retry_interval is five seconds, which is far more than is needed in any of our TAP tests, leaving the test cases just twiddling their thumbs for significant stretches. Moreover, because it's so large, we get basically no testing of the retry-before- master-is-ready code path. Hence, make PostgresNode::init set up wal_retrieve_retry_interval = '500ms' as part of its customization of test clusters' postgresql.conf. This shaves quite a few seconds off the runtime of the recovery TAP tests. Back-patch into 9.6. We have wal_retrieve_retry_interval in 9.5, but the test infrastructure isn't there. Discussion: https://postgr.es/m/31624.1498500416@sss.pgh.pa.us
* Don't lose walreceiver start requests due to race condition in postmaster.Tom Lane2017-06-26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | When a walreceiver dies, the startup process will notice that and send a PMSIGNAL_START_WALRECEIVER signal to the postmaster, asking for a new walreceiver to be launched. There's a race condition, which at least in HEAD is very easy to hit, whereby the postmaster might see that signal before it processes the SIGCHLD from the walreceiver process. In that situation, sigusr1_handler() just dropped the start request on the floor, reasoning that it must be redundant. Eventually, after 10 seconds (WALRCV_STARTUP_TIMEOUT), the startup process would make a fresh request --- but that's a long time if the connection could have been re-established almost immediately. Fix it by setting a state flag inside the postmaster that we won't clear until we do launch a walreceiver. In cases where that results in an extra walreceiver launch, it's up to the walreceiver to realize it's unwanted and go away --- but we have, and need, that logic anyway for the opposite race case. I came across this through investigating unexpected delays in the src/test/recovery TAP tests: it manifests there in test cases where a master server is stopped and restarted while leaving streaming slaves active. This logic has been broken all along, so back-patch to all supported branches. Discussion: https://postgr.es/m/21344.1498494720@sss.pgh.pa.us
* Ignore old stats file timestamps when starting the stats collector.Tom Lane2017-06-26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The stats collector disregards inquiry messages that bear a cutoff_time before when it last wrote the relevant stats file. That's fine, but at startup when it reads the "permanent" stats files, it absorbed their timestamps as if they were the times at which the corresponding temporary stats files had been written. In reality, of course, there's no data out there at all. This led to disregarding inquiry messages soon after startup if the postmaster had been shut down and restarted within less than PGSTAT_STAT_INTERVAL; which is a pretty common scenario, both for testing and in the field. Requesting backends would hang for 10 seconds and then report failure to read statistics, unless they got bailed out by some other backend coming along and making a newer request within that interval. I came across this through investigating unexpected delays in the src/test/recovery TAP tests: it manifests there because the autovacuum launcher hangs for 10 seconds when it can't get statistics at startup, thus preventing a second shutdown from occurring promptly. We might want to do some things in the autovac code to make it less prone to getting stuck that way, but this change is a good bug fix regardless. In passing, also fix pgstat_read_statsfiles() to ensure that it re-zeroes its global stats variables if they are corrupted by a short read from the stats file. (Other reads in that function go into temp variables, so that the issue doesn't arise.) This has been broken since we created the separation between permanent and temporary stats files in 8.4, so back-patch to all supported branches. Discussion: https://postgr.es/m/16860.1498442626@sss.pgh.pa.us
* Reduce pg_ctl's reaction time when waiting for postmaster start/stop.Tom Lane2017-06-26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | pg_ctl has traditionally waited one second between probes for whether the start or stop request has completed. That behavior was embodied in the original shell script written in 1999 (commit 5b912b089) and I doubt anyone's questioned it since. Nowadays, machines are a lot faster, and the shell script is long since replaced by C code, so it's fair to reconsider how long we ought to wait. This patch adjusts the coding so that the wait time can be any even divisor of 1 second, and sets the actual probe rate to 10 per second. That's based on experimentation with the src/test/recovery TAP tests, which include a lot of postmaster starts and stops. This patch alone reduces the (non-parallelized) runtime of those tests from ~4m30s to ~3m5s on my machine. Increasing the probe rate further doesn't help much, so this seems like a good number. In the real world this probably won't have much impact, since people don't start/stop production postmasters often, and the shutdown checkpoint usually takes nontrivial time too. But it makes development work and testing noticeably snappier, and that's good enough reason for me. Also, by reducing the dead time in postmaster restart sequences, this change has made it easier to reproduce some bugs that have been lurking for awhile. Patches for those will follow. Discussion: https://postgr.es/m/18444.1498428798@sss.pgh.pa.us
* Improve wait logic in TAP tests for streaming replication.Tom Lane2017-06-26
| | | | | | | | | | | | | | | | Remove hard-wired sleep(2) delays in 001_stream_rep.pl in favor of using poll_query_until to check for the desired state to appear. In addition, add such a wait before the last test in the script, as it's possible to demonstrate failures there after upcoming improvements in pg_ctl. (We might end up adding polling before each of the get_slot_xmins calls in this script, but I feel no great need to do that until shown necessary.) In passing, clarify the description strings for some of the test cases. Michael Paquier and Craig Ringer, pursuant to a complaint from me Discussion: https://postgr.es/m/8962.1498425057@sss.pgh.pa.us
* Avoid useless "x = ANY(ARRAY[])" test for empty partition list.Tom Lane2017-06-26
| | | | | | | | This arises in practice if the partition only admits NULL values. Jeevan Ladhe Discussion: https://postgr.es/m/CAOgcT0OChrN--uuqH6wG6Z8+nxnCWJ+2Q-uhnK4KOANdRRxuAw@mail.gmail.com
* Minor code review for parse_phrase_operator().Tom Lane2017-06-26
| | | | | | | | | | | | | | | Fix its header comment, which described the old behavior of the <N> phrase distance operator; we missed updating that in commit 028350f61. Also, reset errno before strtol() call, to defend against the possibility that it was already ERANGE at entry. (The lack of complaints says that it generally isn't, but this is at least a latent bug.) Very minor stylistic improvements as well. Victor Drobny noted the obsolete comment, I noted the errno issue. Back-patch to 9.6 where this code was added, just in case the errno issue is a live bug in some cases. Discussion: https://postgr.es/m/2b5382fdff9b1f79d5eb2c99c4d2cbe2@postgrespro.ru
* Further hacking on ICU collation creation and usage.Tom Lane2017-06-24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | pg_import_system_collations() refused to create any ICU collations if the current database's encoding didn't support ICU. This is wrongheaded: initdb must initialize pg_collation in an encoding-independent way since it might be used in other databases with different encodings. The reason for the restriction seems to be that get_icu_locale_comment() used icu_from_uchar() to convert the UChar-format display name, and that unsurprisingly doesn't know what to do in unsupported encodings. But by the same token that the initial catalog contents must be encoding-independent, we can't allow non-ASCII characters in the comment strings. So we don't really need icu_from_uchar() here: just check for Unicode codes outside the ASCII range, and if there are none, the format conversion is trivial. If there are some, we can simply not install the comment. (In my testing, this affects only Norwegian Bokmål, which has given us trouble before.) For paranoia's sake, also check for non-ASCII characters in ICU locale names, and skip such locales, as we do for libc locales. I don't currently have a reason to believe that this will ever reject anything, but then again the libc maintainers should have known better too. With just the import changes, ICU collations can be found in pg_collation in databases with unsupported encodings. This resulted in more or less clean failures at runtime, but that's not how things act for unsupported encodings with libc collations. Make it work the same as our traditional behavior for libc collations by having collation lookup take into account whether is_encoding_supported_by_icu(). Adjust documentation to match. Also, expand Table 23.1 to show which encodings are supported by ICU. catversion bump because of likely change in pg_collation/pg_description initial contents in ICU-enabled builds. Discussion: https://postgr.es/m/20c74bc3-d6ca-243d-1bbc-12f17fa4fe9a@gmail.com
* Fix typo in comment in SerializeSnapshotSimon Riggs2017-06-24
| | | | Author: Masahiko Sawada
* Revert 1f30295eab65eddaa88528876ab66e7095f4bb65Simon Riggs2017-06-24
| | | | Reported-by: Tom Lane
* Fix incorrect buffer-length argument to uloc_getDisplayName().Tom Lane2017-06-23
| | | | | | | | | | | | | | The maxResultSize argument of uloc_getDisplayName is the number of UChars in the output buffer, not the number of bytes. In principle this could result in a stack smash, although at least in my Fedora 25 install there are no ICU locales with display names long enough to overrun the buffer. But it's easily proven to be wrong by reducing the length of displayname to around 20, whereupon a stack smash does happen. (This is a rather scary bug, because the same mistake could easily have been made in other places; but in a quick code search looking at uses of UChar I could not find any other instances.)
* Fix replication with replica identity fullPeter Eisentraut2017-06-23
| | | | | | | | | | | The comparison with the target rows on the subscriber side was done with datumIsEqual(), which can have false negatives. For instance, it didn't work reliably for text columns. So use the equality operator provided by the type cache instead. Also add more user documentation about replica identity requirements. Reported-by: Tatsuo Ishii <ishii@sraoss.co.jp>
* Rethink behavior of pg_import_system_collations().Tom Lane2017-06-23
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Marco Atzeri reported that initdb would fail if "locale -a" reported the same locale name more than once. All previous versions of Postgres implicitly de-duplicated the results of "locale -a", but the rewrite to move the collation import logic into C had lost that property. It had also lost the property that locale names matching built-in collation names were silently ignored. The simplest way to fix this is to make initdb run the function in if-not-exists mode, which means that there's no real use-case for non if-not-exists mode; we might as well just drop the boolean argument and simplify the function's definition to be "add any collations not already known". This change also gets rid of some odd corner cases caused by the fact that aliases were added in if-not-exists mode even if the function argument said otherwise. While at it, adjust the behavior so that pg_import_system_collations() doesn't spew "collation foo already exists, skipping" messages during a re-run; that's completely unhelpful, especially since there are often hundreds of them. And make it return a count of the number of collations it did add, which seems like it might be helpful. Also, re-integrate the previous coding's property that it would make a deterministic selection of which alias to use if there were conflicting possibilities. This would only come into play if "locale -a" reports multiple equivalent locale names, say "de_DE.utf8" and "de_DE.UTF-8", but that hardly seems out of the question. In passing, fix incorrect behavior in pg_import_system_collations()'s ICU code path: it neglected CommandCounterIncrement, which would result in failures if ICU returns duplicate names, and it would try to create comments even if a new collation hadn't been created. Also, reorder operations in initdb so that the 'ucs_basic' collation is created before calling pg_import_system_collations() not after. This prevents a failure if "locale -a" were to report a locale named that. There's no reason to think that that ever happens in the wild, but the old coding would have survived it, so let's be equally robust. Discussion: https://postgr.es/m/20c74bc3-d6ca-243d-1bbc-12f17fa4fe9a@gmail.com
* Improve replication lag interpolation after idle periodSimon Riggs2017-06-23
| | | | | | | | | | | | After sitting idle and fully replayed for a while and then encountering a new burst of WAL activity, we interpolate between an ancient sample and the not-yet-reached one for the new traffic. That produced a corner case report of lag after receiving first new reply from standby, which might sometimes be a large spike. Correct this by resetting last_read time and handle that new case. Author: Thomas Munro
* Fix memory leakage in ICU encoding conversion, and other code review.Tom Lane2017-06-23
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Callers of icu_to_uchar() neglected to pfree the result string when done with it. This results in catastrophic memory leaks in varstr_cmp(), because of our prevailing assumption that btree comparison functions don't leak memory. For safety, make all the call sites clean up leaks, though I suspect that we could get away without it in formatting.c. I audited callers of icu_from_uchar() as well, but found no places that seemed to have a comparable issue. Add function API specifications for icu_to_uchar() and icu_from_uchar(); the lack of any thought-through specification is perhaps not unrelated to the existence of this bug in the first place. Fix icu_to_uchar() to guarantee a nul-terminated result; although no existing caller appears to care, the fact that it would have been nul-terminated except in extreme corner cases seems ideally designed to bite someone on the rear someday. Fix ucnv_fromUChars() destCapacity argument --- in the worst case, that could perhaps have led to a non-nul-terminated result, too. Fix icu_from_uchar() to have a more reasonable definition of the function result --- no callers are actually paying attention, so this isn't a live bug, but it's certainly sloppily designed. Const-ify icu_from_uchar()'s input string for consistency. That is not the end of what needs to be done to these functions, but it's as much as I have the patience for right now. Discussion: https://postgr.es/m/1955.1498181798@sss.pgh.pa.us
* Add testing to detect errors of omission in "pin" dependency creation.Tom Lane2017-06-23
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It's essential that initdb.c's setup_depend() scan each system catalog that could contain objects that need to have "p" (pin) entries in pg_depend or pg_shdepend. Forgetting to add that, either when a catalog is first invented or when it first acquires DATA() entries, is an obvious bug hazard. We can detect such omissions at reasonable cost by probing every OID-containing system catalog to see whether the lowest-numbered OID in it is pinned. If so, the catalog must have been properly accounted for in setup_depend(). If the lowest OID is above FirstNormalObjectId then the catalog must have been empty at the end of initdb, so it doesn't matter. There are a small number of catalogs whose first entry is made later in initdb than setup_depend(), resulting in nonempty expected output of the test, but these can be manually inspected to see that they are OK. Any future mistake of this ilk will manifest as a new entry in the test's output. Since pg_conversion is already in the test's output, add it to the set of catalogs scanned by setup_depend(). That has no effect today (hence, no catversion bump here) but it will protect us if we ever do add pin-worthy conversions. This test is very much like the catalog sanity checks embodied in opr_sanity.sql and type_sanity.sql, but testing pg_depend doesn't seem to fit naturally into either of those scripts' charters. Hence, invent a new test script misc_sanity.sql, which can be a home for this as well as tests on any other catalogs we might want in future. Discussion: https://postgr.es/m/8068.1498155068@sss.pgh.pa.us
* Fix typos in README.dependenciesAlvaro Herrera2017-06-22
| | | | | | | | | | | | | | | There was a logic error in a formula, reported by Atsushi Torokoshi. Ashutosh Bapat furthermore recommended to change notation for a variable that was re-using a letter from a previous formula, though his proposed patch contained a small error in attributing what the new letter is for. Also, instead of his proposed d' I ended up using e, to avoid confusing the reader with quotes which are used differently in the explaining prose. Bugs appeared in commit 2686ee1b7ccfb9214064d4d2a98ea77382880306. Reported-by: Atsushi Torikoshi, Ashutosh Bapat Discussion: https://postgr.es/m/CAFjFpRd03YojT4wyuDcjhCfYuygfWfnt68XGn2CKv=rcjRCtTA@mail.gmail.com
* Fix typo in commentAlvaro Herrera2017-06-22
| | | | | | | | Once upon a time, WAL pointers could be NULL, but no longer. We talk about "valid" now. Reported-by: Amit Langote Discussion: https://postgr.es/m/33e9617d-27f1-eee8-3311-e27af98eaf2b@lab.ntt.co.jp
* Document partitioned_rels in create_modifytable_path header comment.Robert Haas2017-06-22
| | | | | | Etsuro Fujita, slightly adjusted by me. Discussion: http://postgr.es/m/e87c4a6d-23d7-5e7c-e8db-44ed418eb5d1@lab.ntt.co.jp
* Fix autovacuum launcher attachment to its DSAAlvaro Herrera2017-06-22
| | | | | | | | | | | | | | | | | The autovacuum launcher doesn't actually do anything with its DSA other than creating it and attaching to it, but it's been observed that after longjmp'ing to the standard error handling block (for example after getting SIGINT) the autovacuum enters an infinite loop reporting that it cannot attach to its DSA anymore (which is correct, because it's already attached to it.) Fix by only attempting to attach if not already attached. I introduced this bug together with BRIN autosummarization in 7526e10224f0. Reported-by: Yugo Nagata. Author: Thomas Munro. I added the comment to go with it. Discussion: https://postgr.es/m/20170621211538.0c9eae73.nagata@sraoss.co.jp
* Update out-of-date comment in vacuumlazy.cRobert Haas2017-06-22
| | | | | | | | | Commit 15c121b3ed7eb2f290e19533e41ccca734d23574 seems to have overlooked the need to trim this part of the comment. Pavan Deolasee Discussion: http://postgr.es/m/CABOikdPq_9+cWRNZ0RLKTwuZyj=uL85X=Usifa-CbPee1ZCM5A@mail.gmail.com
* Fix IF NOT EXISTS in CREATE STATISTICSAlvaro Herrera2017-06-22
| | | | | | | | | | | I misplaced the IF NOT EXISTS clause in commit 7b504eb282, before the word STATISTICS. Put it where it belongs. Patch written independently by Amit Langote and myself. I adopted his submitted test case with a slight edit also. Reported-by: Bruno Wolff III Discussion: https://postgr.es/m/20170621004237.GB8337@wolff.to
* psql: Restore alphabetical order in words_after_create.Robert Haas2017-06-22
| | | | | | Rushabh Lathia Discussion: http://postgr.es/m/CAGPqQf3yKG0Eo04ePfLPG_-KTo=7ZkxbGDVUWfSGN35Y3SG+PA@mail.gmail.com
* Update comment to account for table partitioning.Robert Haas2017-06-22
| | | | | | Ashutosh Bapat and Amit Langote Discussion: http://postgr.es/m/CAFjFpRcG_NaAv6cDHD-9VfGdvB8maAtSfB=fTQr5+kxP2_sXzg@mail.gmail.com
* Fix typo in commentMagnus Hagander2017-06-22
| | | | Author: Masahiko Sawada
* Fix possibility of creating a "phantom" segment after promotion.Andres Freund2017-06-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When promoting a standby just after a XLOG_SWITCH record was replayed, and next segment(s) are already are locally available (via walsender, restore_command + trigger/recovery target), that segment could accidentally be recycled onto the past of the new timeline. Later checkpointer would create a .ready file for it, assuming there was an error during creation, and it would get archived. That causes trouble if another standby is later brought up from a basebackup from before the timeline creation, because it would try to read the segment, because XLogFileReadAnyTLI just tries all possible timelines, which doesn't have valid contents. Thus replay would fail. The problem, if already occurred, can be fixed by removing the segment and/or having restore_command filter it out. The reason for the creation of such "phantom" segments was, that after an XLOG_SWITCH record the EndOfLog variable points to the beginning of the next segment, and RemoveXlogFile() used XLByteToPrevSeg(). Normally RemoveXlogFile() doing so is harmless, because the last segment will still exist preventing InstallXLogFileSegment() from causing harm, but just after promotion there's no previous segment on the new timeline. Fix that by using XLByteToSeg() instead of XLByteToPrevSeg(). Author: Andres Freund Reported-By: Greg Burek Discussion: https://postgr.es/m/20170619073026.zcwpe6mydsaz5ygd@alap3.anarazel.de Backpatch: 9.2-, bug older than all supported versions
* Manually un-break a few URLs that pgindent used to insist on splitting.Tom Lane2017-06-21
| | | | | | | | These will no longer get re-split by pgindent runs, so it's worth cleaning them up now. Discussion: https://postgr.es/m/E1dAmxK-0006EE-1r@gemulon.postgresql.org Discussion: https://postgr.es/m/30527.1495162840@sss.pgh.pa.us
* Remove entab and associated detritus.Tom Lane2017-06-21
| | | | | | | | We don't need this anymore, because pg_bsd_indent has been taught to follow the same tab-vs-space rules that entab used to enforce. Discussion: https://postgr.es/m/E1dAmxK-0006EE-1r@gemulon.postgresql.org Discussion: https://postgr.es/m/30527.1495162840@sss.pgh.pa.us
* Reformat comments about ResultRelInfoPeter Eisentraut2017-06-21
| | | | | | Also add a comment on its new member PartitionRoot. Reported-by: Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>
* Phase 3 of pgindent updates.Tom Lane2017-06-21
| | | | | | | | | | | | | | | | | | | | | | | | | Don't move parenthesized lines to the left, even if that means they flow past the right margin. By default, BSD indent lines up statement continuation lines that are within parentheses so that they start just to the right of the preceding left parenthesis. However, traditionally, if that resulted in the continuation line extending to the right of the desired right margin, then indent would push it left just far enough to not overrun the margin, if it could do so without making the continuation line start to the left of the current statement indent. That makes for a weird mix of indentations unless one has been completely rigid about never violating the 80-column limit. This behavior has been pretty universally panned by Postgres developers. Hence, disable it with indent's new -lpl switch, so that parenthesized lines are always lined up with the preceding left paren. This patch is much less interesting than the first round of indent changes, but also bulkier, so I thought it best to separate the effects. Discussion: https://postgr.es/m/E1dAmxK-0006EE-1r@gemulon.postgresql.org Discussion: https://postgr.es/m/30527.1495162840@sss.pgh.pa.us
* Phase 2 of pgindent updates.Tom Lane2017-06-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Change pg_bsd_indent to follow upstream rules for placement of comments to the right of code, and remove pgindent hack that caused comments following #endif to not obey the general rule. Commit e3860ffa4dd0dad0dd9eea4be9cc1412373a8c89 wasn't actually using the published version of pg_bsd_indent, but a hacked-up version that tried to minimize the amount of movement of comments to the right of code. The situation of interest is where such a comment has to be moved to the right of its default placement at column 33 because there's code there. BSD indent has always moved right in units of tab stops in such cases --- but in the previous incarnation, indent was working in 8-space tab stops, while now it knows we use 4-space tabs. So the net result is that in about half the cases, such comments are placed one tab stop left of before. This is better all around: it leaves more room on the line for comment text, and it means that in such cases the comment uniformly starts at the next 4-space tab stop after the code, rather than sometimes one and sometimes two tabs after. Also, ensure that comments following #endif are indented the same as comments following other preprocessor commands such as #else. That inconsistency turns out to have been self-inflicted damage from a poorly-thought-through post-indent "fixup" in pgindent. This patch is much less interesting than the first round of indent changes, but also bulkier, so I thought it best to separate the effects. Discussion: https://postgr.es/m/E1dAmxK-0006EE-1r@gemulon.postgresql.org Discussion: https://postgr.es/m/30527.1495162840@sss.pgh.pa.us
* Restart logical replication launcher when killedPeter Eisentraut2017-06-21
| | | | Author: Yugo Nagata <nagata@sraoss.co.jp>
* Initial pgindent run with pg_bsd_indent version 2.0.Tom Lane2017-06-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The new indent version includes numerous fixes thanks to Piotr Stefaniak. The main changes visible in this commit are: * Nicer formatting of function-pointer declarations. * No longer unexpectedly removes spaces in expressions using casts, sizeof, or offsetof. * No longer wants to add a space in "struct structname *varname", as well as some similar cases for const- or volatile-qualified pointers. * Declarations using PG_USED_FOR_ASSERTS_ONLY are formatted more nicely. * Fixes bug where comments following declarations were sometimes placed with no space separating them from the code. * Fixes some odd decisions for comments following case labels. * Fixes some cases where comments following code were indented to less than the expected column 33. On the less good side, it now tends to put more whitespace around typedef names that are not listed in typedefs.list. This might encourage us to put more effort into typedef name collection; it's not really a bug in indent itself. There are more changes coming after this round, having to do with comment indentation and alignment of lines appearing within parentheses. I wanted to limit the size of the diffs to something that could be reviewed without one's eyes completely glazing over, so it seemed better to split up the changes as much as practical. Discussion: https://postgr.es/m/E1dAmxK-0006EE-1r@gemulon.postgresql.org Discussion: https://postgr.es/m/30527.1495162840@sss.pgh.pa.us
* Adjust pgindent script to use pg_bsd_indent 2.0.Tom Lane2017-06-21
| | | | | | | | | | | | | | | | | | | | | | | Update version-checking code and list of switches. Delete obsolete quasi-support for using GNU indent. Remove a lot of no-longer-needed workarounds for bugs of the old version, and improve comments for the hacks that remain. Update run_build() subroutine to fetch the pg_bsd_indent code from the newly established git repo for it. In passing, fix pgindent to not overwrite files that require no changes; this makes it a bit more friendly to run on a built tree. Adjust relevant documentation. Remove indent.bsd.patch; it's not relevant anymore (and was obsolete long ago anyway). Likewise remove pgcppindent, since we're no longer in the business of shipping C++ code. Piotr Stefaniak is responsible for most of the algorithmic changes to the pgindent script; I did the rest. Discussion: https://postgr.es/m/E1dAmxK-0006EE-1r@gemulon.postgresql.org Discussion: https://postgr.es/m/30527.1495162840@sss.pgh.pa.us
* Final pgindent run with old pg_bsd_indent (version 1.3).Tom Lane2017-06-21
| | | | | | | | | This is just to have a clean basis for comparison with the results of the new version (which will indeed end up reverting some of these changes...) Discussion: https://postgr.es/m/E1dAmxK-0006EE-1r@gemulon.postgresql.org Discussion: https://postgr.es/m/30527.1495162840@sss.pgh.pa.us
* Prevent table partitions from being turned into views.Dean Rasheed2017-06-21
| | | | | | | | | | A table partition must be a table, not a view, so don't allow a "_RETURN" rule to be added that would convert an existing table partition into a view. Amit Langote Discussion: https://postgr.es/m/CAEZATCVzFcAjZwC1bTFvJ09skB_sgkF4SwPKMywev-XTnimp9Q%40mail.gmail.com
* Fix typo in comment.Heikki Linnakangas2017-06-21
| | | | Etsuro Fujita
* Make opr_sanity test complain about built-in functions marked prosecdef.Tom Lane2017-06-20
| | | | | | | | | | | | | Currently, there are no built-in functions that are SECURITY DEFINER. But we just found an instance where one was mistakenly marked that way, so it seems prudent to add a test about it. If we ever grow some functions that are intentionally SECURITY DEFINER, we can alter the expected output of this test, or adjust the query to filter out functions for which it's okay. Per suggestion from Robert Haas. Discussion: https://postgr.es/m/CA+TgmoYXg7McY33+jbWmG=rS-HNUur0S6W8Q8kVNFf7epFimVA@mail.gmail.com
* Fix typo in code commentPeter Eisentraut2017-06-20
| | | | Author: Masahiko Sawada <sawada.mshk@gmail.com>
* pg_upgrade: start/stop new server after pg_resetwalBruce Momjian2017-06-20
| | | | | | | | | | | | | | | | | | When commit 0f33a719fdbb5d8c43839ea0d2c90cd03e2af2d2 removed the instructions to start/stop the new cluster before running rsync, it was now possible for pg_resetwal/pg_resetxlog to leave the final WAL record at wal_level=minimum, preventing upgraded standby servers from reconnecting. This patch fixes that by having pg_upgrade unconditionally start/stop the new cluster after pg_resetwal/pg_resetxlog has run. Backpatch through 9.2 since, though the instructions were added in PG 9.5, they worked all the way back to 9.2. Discussion: https://postgr.es/m/20170620171844.GC24975@momjian.us Backpatch-through: 9.2
* Don't downcase entries within shared_preload_libraries et al.Tom Lane2017-06-20
| | | | | | | | | | | | | | | | | | load_libraries(), which processes the various xxx_preload_libraries GUCs, was parsing them using SplitIdentifierString() which isn't really appropriate for values that could be path names: it downcases unquoted text, and it doesn't allow embedded whitespace unless quoted. Use SplitDirectoriesString() instead. That also allows us to simplify load_libraries() a bit, since canonicalize_path() is now done for it. While this definitely seems like a bug fix, it has the potential to break configuration settings that accidentally worked before because of the downcasing behavior. Also, there's an easy workaround for the bug, namely to double-quote troublesome text. Hence, no back-patch. QL Zhuo, tweaked a bit by me Discussion: https://postgr.es/m/CAB-oJtxHVDc3H+Km3CjB9mY1VDzuyaVH_ZYSz7iXcRqCtb93Ew@mail.gmail.com
* Tweak publication fetching in psqlPeter Eisentraut2017-06-20
| | | | | | | | | | | | | | | | | Viewing a table with \d in psql also shows the publications at table is in. If a publication is concurrently dropped, this shows an error, because the view pg_publication_tables internally uses pg_get_publication_tables(), which uses a catalog snapshot. This can be particularly annoying if a for-all-tables publication is concurrently dropped. To avoid that, write the query in psql differently. Expose the function pg_relation_is_publishable() to SQL and write the query using that. That still has a risk of being affected by concurrent catalog changes, but in this case it would be a table drop that causes problems, and then the psql \d command wouldn't be interesting anymore anyway. Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
* Change pg_get_publication_tables to prosecdef falsePeter Eisentraut2017-06-20
| | | | | | This was apparently a mistake in the original commit. Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
* Fix materialized-view documentation oversights.Tom Lane2017-06-19
| | | | | | | | | | When materialized views were added, psql's \d commands were made to treat them as a separate object category ... but not everyplace in the documentation or comments got the memo. Noted by David Johnston. Back-patch to 9.3 where matviews came in. Discussion: https://postgr.es/m/CAKFQuwb27M3VXRhHErjCpkWwN9eKThbqWb1=trtoXi9_ejqPXQ@mail.gmail.com
* Avoid regressions in foreign-key-based selectivity estimates.Tom Lane2017-06-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | David Rowley found that the "use the smallest per-column selectivity" heuristic applied in some cases by get_foreign_key_join_selectivity() was badly off if the FK columns are independent, producing estimates much worse than we got before that code was added in 9.6. One case where that heuristic was used was for LEFT and FULL outer joins with the referenced rel on the outside of the join. But we should not really need to special-case those here. eqjoinsel() never has had such a special case; the correction is applied by calc_joinrel_size_estimate() instead. Let's just estimate such cases like inner joins and rely on that later adjustment. (I think there was something of a thinko here, in that the comments seem to be thinking about the selectivity as defined for semi/anti joins; but that shouldn't apply to left/full joins.) Add a regression test exercising such a case to show that this is sane in at least some cases. The other case where we used that heuristic was for SEMI/ANTI outer joins, either if the referenced rel was on the outside, or if it was on the inside but was part of a join within the RHS. In either case, the FK doesn't give us a lot of traction towards estimating the selectivity. To ensure that we don't have regressions from what happened before 9.6, let's punt by ignoring the FK in such cases and applying the traditional selectivity calculation. (We might be able to improve on that later, but for now I just want to be sure it's not worse than 9.5.) Report and patch by David Rowley, simplified a bit by me. Back-patch to 9.6 where this code was added. Discussion: https://postgr.es/m/CAKJS1f8NO8oCDcxrteohG6O72uU1saEVT9qX=R8pENr5QWerXw@mail.gmail.com
* On Windows, make pg_dump use binary mode for compressed plain text output.Tom Lane2017-06-19
| | | | | | | | | | | | | The combination of -Z -Fp and output to stdout resulted in corrupted output data, because we left stdout in text mode, resulting in newline conversion being done on the compressed stream. Switch stdout to binary mode for this case, at the same place where we do it for non-text output formats. Report and patch by Kuntal Ghosh, tested by Ashutosh Sharma and Neha Sharma. Back-patch to all supported branches. Discussion: https://postgr.es/m/CAGz5QCJPvbBjXAmJuGx1B_41yVCetAJhp7rtaDf7XQGWuB1GSw@mail.gmail.com
* Fix leaking of small spilled subtransactions during logical decoding.Andres Freund2017-06-18
| | | | | | | | | | | | | | | | | | When, during logical decoding, a transaction gets too big, it's contents get spilled to disk. Not just the top-transaction gets spilled, but *also* all of its subtransactions, even if they're not that large themselves. Unfortunately we didn't clean up such small spilled subtransactions from disk. Fix that, by keeping better track of whether a transaction has been spilled to disk. Author: Andres Freund Reported-By: Dmitriy Sarafannikov, Fabrízio de Royes Mello Discussion: https://postgr.es/m/1457621358.355011041@f382.i.mail.ru https://postgr.es/m/CAFcNs+qNMhNYii4nxpO6gqsndiyxNDYV0S=JNq0v_sEE+9PHXg@mail.gmail.com Backpatch: 9.4-, where logical decoding was introduced
* Set statement timestamp in apply workerPeter Eisentraut2017-06-17
| | | | | | This ensures that triggers can see an up-to-date timestamp. Reported-by: Konstantin Evteev <konst583@gmail.com>