aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* Accept XML documents when xmloption = content, as required by SQL:2006+.Tom Lane2019-03-23
| | | | | | | | | | | | | | | | | | | | | | | | | | | Previously we were using the SQL:2003 definition, which doesn't allow this, but that creates a serious dump/restore gotcha: there is no setting of xmloption that will allow all valid XML data. Hence, switch to the 2006 definition. Since libxml doesn't accept <!DOCTYPE> directives in the mode we use for CONTENT parsing, the implementation is to detect <!DOCTYPE> in the input and switch to DOCUMENT parsing mode. This should not cost much, because <!DOCTYPE> should be close to the front of the input if it's there at all. It's possible that this causes the error messages for malformed input to be slightly different than they were before, if said input includes <!DOCTYPE>; but that does not seem like a big problem. In passing, buy back a few cycles in parsing of large XML documents by not doing strlen() of the whole input in parse_xml_decl(). Back-patch because dump/restore failures are not nice. This change shouldn't break any cases that worked before, so it seems safe to back-patch. Chapman Flack (revised a bit by me) Discussion: https://postgr.es/m/CAN-V+g-6JqUQEQZ55Q3toXEN6d5Ez5uvzL4VR+8KtvJKj31taw@mail.gmail.com
* Make checkpoint requests more robust.Tom Lane2019-03-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 6f6a6d8b1 introduced a delay of up to 2 seconds if we're trying to request a checkpoint but the checkpointer hasn't started yet (or, much less likely, our kill() call fails). However buildfarm experience shows that that's not quite enough for slow or heavily-loaded machines. There's no good reason to assume that the checkpointer won't start eventually, so we may as well make the timeout much longer, say 60 sec. However, if the caller didn't say CHECKPOINT_WAIT, it seems like a bad idea to be waiting at all, much less for as long as 60 sec. We can remove the need for that, and make this whole thing more robust, by adjusting the code so that the existence of a pending checkpoint request is clear from the contents of shared memory, and making sure that the checkpointer process will notice it at startup even if it did not get a signal. In this way there's no need for a non-CHECKPOINT_WAIT call to wait at all; if it can't send the signal, it can nonetheless assume that the checkpointer will eventually service the request. A potential downside of this change is that "kill -INT" on the checkpointer process is no longer enough to trigger a checkpoint, should anyone be relying on something so hacky. But there's no obvious reason to do it like that rather than issuing a plain old CHECKPOINT command, so we'll assume that nobody is. There doesn't seem to be a way to preserve this undocumented quasi-feature without introducing race conditions. Since a principal reason for messing with this is to prevent intermittent buildfarm failures, back-patch to all supported branches. Discussion: https://postgr.es/m/27830.1552752475@sss.pgh.pa.us
* Ensure dummy paths have correct required_outer if rel is parameterized.Tom Lane2019-03-14
| | | | | | | | | | | | | | | | The assertions added by commits 34ea1ab7f et al found another problem: set_dummy_rel_pathlist and mark_dummy_rel were failing to label the dummy paths they create with the correct outer_relids, in case the relation is necessarily parameterized due to having lateral references in its tlist. It's likely that this has no user-visible consequences in production builds, at the moment; but still an assertion failure is a bad thing, so back-patch the fix. Per bug #15694 from Roman Zharkov (via Alexander Lakhin) and an independent report by Tushar Ahuja. Discussion: https://postgr.es/m/15694-74f2ca97e7044f7f@postgresql.org Discussion: https://postgr.es/m/7d72ab20-c725-3ce2-f99d-4e64dd8a0de6@enterprisedb.com
* Fix potential memory access violation in ecpg if filename of include file isMichael Meskes2019-03-11
| | | | | | shorter than 2 characters. Patch by: "Wu, Fei" <wufei.fnst@cn.fujitsu.com>
* Disallow NaN as a value for floating-point GUCs.Tom Lane2019-03-10
| | | | | | | | | | | | | | | | | | None of the code that uses GUC values is really prepared for them to hold NaN, but parse_real() didn't have any defense against accepting such a value. Treat it the same as a syntax error. I haven't attempted to analyze the exact consequences of setting any of the float GUCs to NaN, but since they're quite unlikely to be good, this seems like a back-patchable bug fix. Note: we don't need an explicit test for +-Infinity because those will be rejected by existing range checks. I added a regression test for that in HEAD, but not older branches because the spelling of the value in the error message will be platform-dependent in branches where we don't always use port/snprintf.c. Discussion: https://postgr.es/m/1798.1552165479@sss.pgh.pa.us
* Fix handling of targetlist SRFs when scan/join relation is known empty.Tom Lane2019-03-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When we introduced separate ProjectSetPath nodes for application of set-returning functions in v10, we inadvertently broke some cases where we're supposed to recognize that the result of a subquery is known to be empty (contain zero rows). That's because IS_DUMMY_REL was just looking for a childless AppendPath without allowing for a ProjectSetPath being possibly stuck on top. In itself, this didn't do anything much worse than produce slightly worse plans for some corner cases. Then in v11, commit 11cf92f6e rearranged things to allow the scan/join targetlist to be applied directly to partial paths before they get gathered. But it inserted a short-circuit path for dummy relations that was a little too short: it failed to insert a ProjectSetPath node at all for a targetlist containing set-returning functions, resulting in bogus "set-valued function called in context that cannot accept a set" errors, as reported in bug #15669 from Madelaine Thibaut. The best way to fix this mess seems to be to reimplement IS_DUMMY_REL so that it drills down through any ProjectSetPath nodes that might be there (and it seems like we'd better allow for ProjectionPath as well). While we're at it, make it look at rel->pathlist not cheapest_total_path, so that it gives the right answer independently of whether set_cheapest has been done lately. That dependency looks pretty shaky in the context of code like apply_scanjoin_target_to_paths, and even if it's not broken today it'd certainly bite us at some point. (Nastily, unsafe use of the old coding would almost always work; the hazard comes down to possibly looking through a dangling pointer, and only once in a blue moon would you find something there that resulted in the wrong answer.) It now looks like it was a mistake for IS_DUMMY_REL to be a macro: if there are any extensions using it, they'll continue to use the old inadequate logic until they're recompiled, after which they'll fail to load into server versions predating this fix. Hopefully there are few such extensions. Having fixed IS_DUMMY_REL, the special path for dummy rels in apply_scanjoin_target_to_paths is unnecessary as well as being wrong, so we can just drop it. Also change a few places that were testing for partitioned-ness of a planner relation but not using IS_PARTITIONED_REL for the purpose; that seems unsafe as well as inconsistent, plus it required an ugly hack in apply_scanjoin_target_to_paths. In passing, save a few cycles in apply_scanjoin_target_to_paths by skipping processing of pre-existing paths for partitioned rels, and do some cosmetic cleanup and comment adjustment in that function. I renamed IS_DUMMY_PATH to IS_DUMMY_APPEND with the intention of breaking any code that might be using it, since in almost every case that would be wrong; IS_DUMMY_REL is what to be using instead. In HEAD, also make set_dummy_rel_pathlist static (since it's no longer used from outside allpaths.c), and delete is_dummy_plan, since it's no longer used anywhere. Back-patch as appropriate into v11 and v10. Tom Lane and Julien Rouhaud Discussion: https://postgr.es/m/15669-02fb3296cca26203@postgresql.org
* Disable dump_connstr test on Msys2Andrew Dunstan2019-03-05
| | | | | | For some reason the dump test with names with high bits set fails on Msys2 (although not Msys1). Disable the tests for now, so that other tests can run.
* Fix error handling of readdir() port implementation on first file lookupMichael Paquier2019-03-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | The implementation of readdir() in src/port/ which gets used by MSVC has been added in 399a36a, and since the beginning it considers all errors on the first file lookup as ENOENT, setting errno accordingly and letting the routine caller think that the directory is empty. While this is normally enough for the case of the backend, this can confuse callers of this routine on Windows as all errors would map to the same behavior. So, for example, even permission errors would be thought as having an empty directory, while there could be contents in it. This commit changes the error handling so as readdir() gets a behavior similar to native implementations: force errno=0 when seeing ERROR_FILE_NOT_FOUND as error and consider other errors as plain failures. While looking at the patch, I noticed that MinGW does not enforce errno=0 when looking at the first file, but it gets enforced on the next file lookups. A comment related to that was incorrect in the code. Reported-by: Yuri Kurenkov Diagnosed-by: Yuri Kurenkov, Grigory Smolkin Author: Konstantin Knizhnik Reviewed-by: Andrew Dunstan, Michael Paquier Discussion: https://postgr.es/m/2cad7829-8d66-e39c-b937-ac825db5203d@postgrespro.ru Backpatch-through: 9.4
* Further fixing for multi-row VALUES lists for updatable views.Dean Rasheed2019-03-03
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, rewriteTargetListIU() generated a list of attribute numbers from the targetlist, which were passed to rewriteValuesRTE(), which expected them to contain the same number of entries as there are columns in the VALUES RTE, and to be in the same order. That was fine when the target relation was a table, but for an updatable view it could be broken in at least three different ways --- rewriteTargetListIU() could insert additional targetlist entries for view columns with defaults, the view columns could be in a different order from the columns of the underlying base relation, and targetlist entries could be merged together when assigning to elements of an array or composite type. As a result, when recursing to the base relation, the list of attribute numbers generated from the rewritten targetlist could no longer be relied upon to match the columns of the VALUES RTE. We got away with that prior to 41531e42d3 because it used to always be the case that rewriteValuesRTE() did nothing for the underlying base relation, since all DEFAULTS had already been replaced when it was initially invoked for the view, but that was incorrect because it failed to apply defaults from the base relation. Fix this by examining the targetlist entries more carefully and picking out just those that are simple Vars referencing the VALUES RTE. That's sufficient for the purposes of rewriteValuesRTE(), which is only responsible for dealing with DEFAULT items in the VALUES RTE. Any DEFAULT item in the VALUES RTE that doesn't have a matching simple-Var-assignment in the targetlist is an error which we complain about, but in theory that ought to be impossible. Additionally, move this code into rewriteValuesRTE() to give a clearer separation of concerns between the 2 functions. There is no need for rewriteTargetListIU() to know about the details of the VALUES RTE. While at it, fix the comment for rewriteValuesRTE() which claimed that it doesn't support array element and field assignments --- that hasn't been true since a3c7a993d5 (9.6 and later). Back-patch to all supported versions, with minor differences for the pre-9.6 branches, which don't support array element and field assignments to the same column in multi-row VALUES lists. Reviewed by Amit Langote. Discussion: https://postgr.es/m/15623-5d67a46788ec8b7f@postgresql.org
* Improve documentation of data_sync_retryMichael Paquier2019-02-28
| | | | | | | | Reflecting an updated parameter value requires a server restart, which was not mentioned in the documentation and in postgresql.conf.sample. Reported-by: Thomas Poty Discussion: https://postgr.es/m/15659-0cd812f13027a2d8@postgresql.org
* Fix inconsistent out-of-memory error reporting in dsa.c.Thomas Munro2019-02-25
| | | | | | | | | | | | | | | | | | | | | | | | | | Commit 16be2fd1 introduced the flag DSA_ALLOC_NO_OOM to control whether the DSA allocator would raise an error or return InvalidDsaPointer on failure to allocate. One edge case was not handled correctly: if we fail to allocate an internal "span" object for a large allocation, we would always return InvalidDsaPointer regardless of the flag; a caller not expecting that could then dereference a null pointer. This is a plausible explanation for a one-off report of a segfault. Remove a redundant pair of braces so that all three stanzas that handle DSA_ALLOC_NO_OOM match in style, for visual consistency. While fixing inconsistencies, if FreePageManagerGet() can't supply the pages that our book-keeping says it should be able to supply, then we should always report a FATAL error. Previously we treated that as a regular allocation failure in one code path, but as a FATAL condition in another. Back-patch to 10, where dsa.c landed. Author: Thomas Munro Reported-by: Jakub Glapa Discussion: https://postgr.es/m/CAEepm=2oPqXxyWQ-1o60tpOLrwkw=VpgNXqqF1VN2EyO9zKGQw@mail.gmail.com
* Fix ecpg bugs caused by missing semicolons in the backend grammar.Tom Lane2019-02-24
| | | | | | | | | | | | | | | | | | | | | | | | | The Bison documentation clearly states that a semicolon is required after every grammar rule, and our scripts that generate ecpg's grammar from the backend's implicitly assumed this is true. But it turns out that only ancient versions of Bison actually enforce that. There have been a couple of rules without trailing semicolons in gram.y for some time, and as a consequence, ecpg's grammar was faulty and produced wrong output for the affected statements. To fix, add the missing semis, and add some cross-checks to ecpg's scripts so that they'll bleat if we mess this up again. The cases that were broken were: * "SET variable = DEFAULT" (but not "SET variable TO DEFAULT"), as well as allied syntaxes such as ALTER SYSTEM SET ... DEFAULT. These produced syntactically invalid output that the server would reject. * Multiple type names in DROP TYPE/DOMAIN commands. Only the first type name would be listed in the emitted command. Per report from Daisuke Higuchi. Back-patch to all supported versions. Discussion: https://postgr.es/m/1803D792815FC24D871C00D17AE95905DB51CE@g01jpexmbkw24
* Tolerate EINVAL when calling fsync() on a directory.Thomas Munro2019-02-24
| | | | | | | | | | | Previously, we tolerated EBADF as a way for the operating system to indicate that it doesn't support fsync() on a directory. Tolerate EINVAL too, for older versions of Linux CIFS. Bug #15636. Back-patch all the way. Reported-by: John Klann Discussion: https://postgr.es/m/15636-d380890dafd78fc6@postgresql.org
* Tolerate ENOSYS failure from sync_file_range().Thomas Munro2019-02-24
| | | | | | | | | | | | | | | | | | | | | One unintended consequence of commit 9ccdd7f6 was that Windows WSL users started getting a panic whenever we tried to initiate data flushing with sync_file_range(), because WSL does not implement that system call. Previously, they got a stream of periodic warnings, which was also undesirable but at least ignorable. Prevent the panic by handling ENOSYS specially and skipping the panic promotion with data_sync_elevel(). Also suppress future attempts after the first such failure so that the pre-existing problem of noisy warnings is improved. Back-patch to 9.6 (older branches were not affected in this way by 9ccdd7f6). Author: Thomas Munro and James Sewell Tested-by: James Sewell Reported-by: Bruce Klein Discussion: https://postgr.es/m/CA+mCpegfOUph2U4ZADtQT16dfbkjjYNJL1bSTWErsazaFjQW9A@mail.gmail.com
* Fix plan created for inherited UPDATE/DELETE with all tables excluded.Tom Lane2019-02-22
| | | | | | | | | | | | | | | | | | | | | | In the case where inheritance_planner() finds that every table has been excluded by constraints, it thought it could get away with making a plan consisting of just a dummy Result node. While certainly there's no updating or deleting to be done, this had two user-visible problems: the plan did not report the correct set of output columns when a RETURNING clause was present, and if there were any statement-level triggers that should be fired, it didn't fire them. Hence, rather than only generating the dummy Result, we need to stick a valid ModifyTable node on top, which requires a tad more effort here. It's been broken this way for as long as inheritance_planner() has known about deleting excluded subplans at all (cf commit 635d42e9c), so back-patch to all supported branches. Amit Langote and Tom Lane, per a report from Petr Fedorov. Discussion: https://postgr.es/m/5da6f0f0-1364-1876-6978-907678f89a3e@phystech.edu
* Report correct name in autovacuum "work items" activityAlvaro Herrera2019-02-22
| | | | | | | | We were reporting the database name instead of the relation name to pg_stat_activity. Repair. Reported-by: Justin Pryzby Discussion: https://postgr.es/m/20190220185552.GR28750@telsasoft.com
* Speed up match_eclasses_to_foreign_key_col() when there are many ECs.Tom Lane2019-02-20
| | | | | | | | | | | | | Check ec_relids before bothering to iterate through the EC members. On a perhaps extreme, but still real-world, query in which match_eclasses_to_foreign_key_col() accounts for the bulk of the planner's runtime, this saves nearly 40% of the runtime. It's a bit of a stopgap fix, but it's simple enough to be back-patched to 9.6 where this code came in; so let's do that. David Rowley Discussion: https://postgr.es/m/6970.1545327857@sss.pgh.pa.us
* Make object address handling more robustAlvaro Herrera2019-02-20
| | | | | | | | | pg_identify_object_as_address crashes when passed certain tuples from inconsistent system catalogs. Make it more defensive. Author: Álvaro Herrera Reviewed-by: Michaël Paquier Discussion: https://postgr.es/m/20190218202743.GA12392@alvherre.pgsql
* Fix DEFAULT-handling in multi-row VALUES lists for updatable views.Dean Rasheed2019-02-20
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | INSERT ... VALUES for a single VALUES row is implemented differently from a multi-row VALUES list, which causes inconsistent behaviour in the way that DEFAULT items are handled. In particular, when inserting into an auto-updatable view on top of a table with a column default, a DEFAULT item in a single VALUES row gets correctly replaced with the table column's default, but for a multi-row VALUES list it is replaced with NULL. Fix this by allowing rewriteValuesRTE() to leave DEFAULT items in the VALUES list untouched if the target relation is an auto-updatable view and has no column default, deferring DEFAULT-expansion until the query against the base relation is rewritten. For all other types of target relation, including tables and trigger- and rule-updatable views, we must continue to replace DEFAULT items with NULL in the absence of a column default. This is somewhat complicated by the fact that if an auto-updatable view has DO ALSO rules attached, the VALUES lists for the product queries need to be handled differently from the original query, since the product queries need to act like rule-updatable views whereas the original query has auto-updatable view semantics. Back-patch to all supported versions. Reported by Roger Curley (bug #15623). Patch by Amit Langote and me. Discussion: https://postgr.es/m/15623-5d67a46788ec8b7f@postgresql.org
* Mark correctly initial slot snapshots with MVCC type when builtMichael Paquier2019-02-20
| | | | | | | | | | | | | When building an initial slot snapshot, snapshots are marked with historic MVCC snapshots as type with the marker field being set in SnapBuildBuildSnapshot() but not overriden in SnapBuildInitialSnapshot(). Existing callers of SnapBuildBuildSnapshot() do not care about the type of snapshot used, but extensions calling it actually may, as reported. Author: Antonin Houska Reviewed-by: Álvaro Herrera, Michael Paquier Discussion: https://postgr.es/m/23215.1527665193@localhost Backpatch-through: 9.4
* Fix omissions in ecpg/test/sql/.gitignore.Tom Lane2019-02-18
| | | | Oversights in commits 050710b36 and e81f0e311.
* Sync ECPG's CREATE TABLE AS statement with backend's.Michael Meskes2019-02-18
| | | | Author: Higuchi-san ("Higuchi, Daisuke" <higuchi.daisuke@jp.fujitsu.com>)
* Fix race in dsm_unpin_segment() when handles are reused.Thomas Munro2019-02-18
| | | | | | | | | | | | | | | | | | | Teach dsm_unpin_segment() to skip segments that are in the process of being destroyed by another backend, when searching for a handle. Such a segment cannot possibly be the one we are looking for, even if its handle matches. Another slot might hold a recently created segment that has the same handle value by coincidence, and we need to keep searching for that one. The bug caused rare "cannot unpin a segment that is not pinned" errors on 10 and 11. Similar to commit 6c0fb941 for dsm_attach(). Back-patch to 10, where dsm_unpin_segment() landed. Author: Thomas Munro Reported-by: Justin Pryzby Tested-by: Justin Pryzby (along with other recent DSA/DSM fixes) Discussion: https://postgr.es/m/20190216023854.GF30291@telsasoft.com
* Fix CREATE VIEW to allow zero-column views.Tom Lane2019-02-17
| | | | | | | | | | | | | | | | | | | | | We should logically have allowed this case when we allowed zero-column tables, but it was overlooked. Although this might be thought a feature addition, it's really a bug fix, because it was possible to create a zero-column view via the convert-table-to-view code path, and then you'd have a situation where dump/reload would fail. Hence, back-patch to all supported branches. Arrange the added test cases to provide coverage of the related pg_dump code paths (since these views will be dumped and reloaded during the pg_upgrade regression test). I also made them test the case where pg_dump has to postpone the view rule into post-data, which disturbingly had no regression coverage before. Report and patch by Ashutosh Sharma (test case by me) Discussion: https://postgr.es/m/CAE9k0PkmHdeSaeZt2ujnb_cKucmK3sDDceDzw7+d5UZoNJPYOg@mail.gmail.com
* Fix support for CREATE TABLE IF NOT EXISTS AS EXECUTEMichael Paquier2019-02-15
| | | | | | | | | | | The grammar IF NOT EXISTS for CTAS is supported since 9.5 and documented as such, however the case of using EXECUTE as query has never been covered as EXECUTE CTAS statements and normal CTAS statements are parsed separately. Author: Andreas Karlsson Discussion: https://postgr.es/m/2ddcc188-e37c-a0be-32bf-a56b07c3559e@proxel.se Backpatch-through: 9.5
* Fix race in dsm_attach() when handles are reused.Thomas Munro2019-02-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | DSM handle values can be reused as soon as the underlying shared memory object has been destroyed. That means that for a brief moment we might have two DSM slots with the same handle. While trying to attach, if we encounter a slot with refcnt == 1, meaning that it is currently being destroyed, we should continue our search in case the same handle exists in another slot. The race manifested as a rare "dsa_area could not attach to segment" error, and was more likely in 10 and 11 due to the lack of distinct seed for random() in parallel workers. It was made very unlikely in in master by commit 197e4af9, and older releases don't usually create new DSM segments in background workers so it was also unlikely there. This fixes the root cause of bug report #15585, in which the error could also sometimes result in a self-deadlock in the error path. It's not yet clear if further changes are needed to avoid that failure mode. Back-patch to 9.4, where dsm.c arrived. Author: Thomas Munro Reported-by: Justin Pryzby, Sergei Kornilov Discussion: https://postgr.es/m/20190207014719.GJ29720@telsasoft.com Discussion: https://postgr.es/m/15585-324ff6a93a18da46@postgresql.org
* Fix rare dsa_allocate() failures due to freepage.c corruption.Thomas Munro2019-02-13
| | | | | | | | | | | | | | | | | | In a corner case, a btree page was allocated during a clean-up operation that could cause the tracking of the largest contiguous span of free space to get out of whack. That was supposed to be prevented by the use of the "soft" flag to avoid allocating internal pages during incidental clean-up work, but the flag was ignored in the case where the FPM was promoted from singleton format to btree format. Repair. Remove an obsolete comment in passing. Back-patch to 10, where freepage.c arrived (as support for dsa.c). Author: Robert Haas Diagnosed-by: Thomas Munro and Robert Haas Reported-by: Justin Pryzby, Rick Otten, Sand Stone, Arne Roland and others Discussion: https://postgr.es/m/CAMAYy4%2Bw3NTBM5JLWFi8twhWK4%3Dk_5L4nV5%2BbYDSPu8r4b97Zg%40mail.gmail.com
* Relax overly strict assertionAlvaro Herrera2019-02-12
| | | | | | | | | | | | | | | Ever since its birth, ReorderBufferBuildTupleCidHash() has contained an assertion that a catalog tuple cannot change Cmax after acquiring one. But that's wrong: if a subtransaction executes DDL that affects that catalog tuple, and later aborts and another DDL affects the same tuple, it will change Cmax. Relax the assertion to merely verify that the Cmax remains valid and monotonically increasing, instead. Add a test that tickles the relevant code. Diagnosed by, and initial patch submitted by: Arseny Sher Co-authored-by: Arseny Sher Discussion: https://postgr.es/m/874l9p8hyw.fsf@ars-thinkpad
* Fix erroneous error reports in snapbuild.c.Tom Lane2019-02-12
| | | | | | | | | | | | | | | | It's pretty unhelpful to report the wrong file name in a complaint about syscall failure, but SnapBuildSerialize managed to do that twice in a span of 50 lines. Also fix half a dozen missing or poorly-chosen errcode assignments; that's mostly cosmetic, but still wrong. Noted while studying recent failures on buildfarm member nightjar. I'm not sure whether those reports are actually giving the wrong filename, because there are two places here with identically spelled error messages. The other one is specifically coded not to report ENOENT, but if it's this one, how could we be getting ENOENT from open() with O_CREAT? Need to sit back and await results. However, these ereports are clearly broken from birth, so back-patch.
* Stamp 10.7.REL_10_7Tom Lane2019-02-11
|
* Translation updatesPeter Eisentraut2019-02-11
| | | | | Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git Source-Git-Hash: da5fe0060d012477d807c1b60f1ff2188947a72f
* Adjust error messagePeter Eisentraut2019-02-11
| | | | | | | | | We usually don't use "namespace" in user-facing error messages. Also, in master this was replaced by another error message referring to "temporary objects", so we might as well use that here to avoid introducing too many variants. Discussion: https://www.postgresql.org/message-id/bbd3f8d9-e3d5-e5aa-4305-7f0121c3fa94@2ndquadrant.com
* Solve cross-version-upgrade testing problem induced by 1fb57af92.Tom Lane2019-02-09
| | | | | | | | | | | | | | | | | | | | Renaming varchar_transform to varchar_support had a side effect I hadn't foreseen: the core regression tests leave around a transform object that relies on that function, so the name change breaks cross-version upgrade tests, because the name used in the older branches doesn't match. Since the dependency on varchar_transform was chosen with the aid of a dartboard anyway (it would surely not work as a language transform support function), fix by just choosing a different random builtin function with the right signature. Also add some comments explaining why this isn't horribly unsafe. I chose to make the same substitution in a couple of other copied-and-pasted test cases, for consistency, though those aren't directly contributing to the testing problem. Per buildfarm. Back-patch, else it doesn't fix the problem.
* Repair unsafe/unportable snprintf usage in pg_restore.Tom Lane2019-02-09
| | | | | | | | | | | | | | | | | warn_or_exit_horribly() was blithely passing a potentially-NULL string pointer to a %s format specifier. That works (at least to the extent of not crashing) on some platforms, but not all, and since we switched to our own snprintf.c it doesn't work for us anywhere. Of the three string fields being handled this way here, I think that only "owner" is supposed to be nullable ... but considering that this is error-reporting code, it has very little business assuming anything, so put in defenses for all three. Per a crash observed on buildfarm member crake and then reproduced here. Because of the portability aspect, back-patch to all supported versions.
* Call set_rel_pathlist_hook before generate_gather_paths, not after.Tom Lane2019-02-09
| | | | | | | | | | | | | | | | | | | | | | | | The previous ordering of these steps satisfied the nominal requirement that set_rel_pathlist_hook could editorialize on the whole set of Paths constructed for a base relation. In practice, though, trying to change the set of partial paths was impossible. Adding one didn't work because (a) it was too late to be included in Gather paths made by the core code, and (b) calling add_partial_path after generate_gather_paths is unsafe, because it might try to delete a path it thinks is dominated, but that is already embedded in some Gather path(s). Nor could the hook safely remove partial paths, for the same reason that they might already be embedded in Gathers. Better to call extensions first, let them add partial paths as desired, and then gather. In v11 and up, we already doubled down on that ordering by postponing gathering even further for single-relation queries; so even if the hook wished to editorialize on Gather path construction, it could not. Report and patch by KaiGai Kohei. Back-patch to 9.6 where Gather paths were added. Discussion: https://postgr.es/m/CAOP8fzahwpKJRTVVTqo2AE=mDTz_efVzV6Get_0=U3SO+-ha1A@mail.gmail.com
* Defend against null error message reported by libxml2.Tom Lane2019-02-08
| | | | | | | | | | | | | | While this isn't really supposed to happen, it can occur in OOM situations and perhaps others. Instead of crashing, substitute "(no message provided)". I didn't worry about localizing this text, since we aren't localizing anything else here; besides, if we're on the edge of OOM, it's unlikely gettext() would work. Report and fix by Sergio Conde Gómez in bug #15624. Discussion: https://postgr.es/m/15624-4dea54091a2864e6@postgresql.org
* Ensure that foreign scans with lateral refs are planned correctly.Tom Lane2019-02-07
| | | | | | | | | | | | | | | | | | | | | | | | | As reported in bug #15613 from Srinivasan S A, file_fdw and postgres_fdw neglected to mark plain baserel foreign paths as parameterized when the relation has lateral_relids. Other FDWs have surely copied this mistake, so rather than just patching those two modules, install a band-aid fix in create_foreignscan_path to rectify the mistake centrally. Although the band-aid is enough to fix the visible symptom, correct the calls in file_fdw and postgres_fdw anyway, so that they are valid examples for external FDWs. Also, since the band-aid isn't enough to make this work for parameterized foreign joins, throw an elog(ERROR) if such a case is passed to create_foreignscan_path. This shouldn't pose much of a problem for existing external FDWs, since it's likely they aren't trying to make such paths anyway (though some of them may need a defense against joins with lateral_relids, similar to the one this patch installs into postgres_fdw). Add some assertions in relnode.c to catch future occurrences of the same error --- in particular, as backstop against core-code mistakes like the one fixed by commit bdd9a99aa. Discussion: https://postgr.es/m/15613-092be1be9576c728@postgresql.org
* Fix searchpath and module location for pg_rewind and ssl TAP testsAndrew Dunstan2019-02-07
| | | | | | | | | | | | | | | The modules RewindTest.pm and ServerSetup.pm are really only useful for TAP tests, so they really belong in the TAP test directories. In addition, ServerSetup.pm is renamed to SSLServer.pm. The test scripts have their own directories added to the search path so that the relocated modules will be found, regardless of where the tests are run from, even on modern perl where "." is no longer in the searchpath. Discussion: https://postgr.es/m/e4b0f366-269c-73c3-9c90-d9cb0f4db1f9@2ndQuadrant.com Backpatch as appropriate to 9.5
* Propagate lateral-reference information to indirect descendant relations.Tom Lane2019-02-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | create_lateral_join_info() computes a bunch of information about lateral references between base relations, and then attempts to propagate those markings to appendrel children of the original base relations. But the original coding neglected the possibility of indirect descendants (grandchildren etc). During v11 development we noticed that this was wrong for partitioned-table cases, but failed to realize that it was just as wrong for any appendrel. While the case can't arise for appendrels derived from traditional table inheritance (because we make a flat appendrel for that), nested appendrels can arise from nested UNION ALL subqueries. Failure to mark the lower-level relations as having lateral references leads to confusion in add_paths_to_append_rel about whether unparameterized paths can be built. It's not very clear whether that leads to any user-visible misbehavior; the lack of field reports suggests that it may cause nothing worse than minor cost misestimation. Still, it's a bug, and it leads to failures of Asserts that I intend to add later. To fix, we need to propagate information from all appendrel parents, not just those that are RELOPT_BASERELs. We can still do it in one pass, if we rely on the append_rel_list to be ordered with ancestor relationships before descendant ones; add assertions checking that. While fixing this, we can make a small performance improvement by traversing the append_rel_list just once instead of separately for each appendrel parent relation. Noted while investigating bug #15613, though this patch does not fix that (which is why I'm not committing the related Asserts yet). Discussion: https://postgr.es/m/3951.1549403812@sss.pgh.pa.us
* Unify searchpath and do file logic in MSVC build scripts.Andrew Dunstan2019-02-06
| | | | | | Commit f83419b739 failed to notice that mkvcbuild.pl and build.pl use different searchpath and do-file logic, breaking the latter, so it is adjusted to use the same logic as mkvcbuild.pl.
* Fix included file path for modern perlAndrew Dunstan2019-02-05
| | | | | | | | | Contrary to the comment on 772d4b76, only paths starting with "./" or "../" are considered relative to the current working directory by perl's "do" function. So this patch converts all the relevant cases to use "./" paths. This only affects MSVC. Backpatch to all live branches.
* Keep perl style checker happyAndrew Dunstan2019-02-05
| | | | It doesn't like code before "use strict;".
* Update time zone data files to tzdata release 2018i.Tom Lane2019-02-05
| | | | | | | DST law changes in Kazakhstan, Metlakatla, and São Tomé and Príncipe. Kazakhstan's Qyzylorda zone is split in two, creating a new zone Asia/Qostanay, as some areas did not change UTC offset. Historical corrections for Hong Kong and numerous Pacific islands.
* Fix searchpath for modern Perl for genbki.plAndrew Dunstan2019-02-05
| | | | | | | This was fixed for MSVC tools by commit 1df92eeafefac4, but per buildfarm member bowerbird genbki.pl needs the same treatment. Backpatch to all live branches.
* Fix dumping of matviews with indirect dependencies on primary keys.Tom Lane2019-02-04
| | | | | | | | | | | | | | | | | | | | | | | | Commit 62215de29 turns out to have been not quite on-the-mark. When we are forced to postpone dumping of a materialized view into the dump's post-data section (because it depends on a unique index that isn't created till that section), we may also have to postpone dumping other matviews that depend on said matview. The previous fix didn't reliably work for such cases: it'd break the dependency loops properly, producing a workable object ordering, but it didn't necessarily mark all the matviews as "postponed_def". This led to harmless bleating about "archive items not in correct section order", as reported by Tom Cassidy in bug #15602. Less harmlessly, selective-restore options such as --section might misbehave due to the matview dump objects not being properly labeled. The right way to fix it is to consider that each pre-data dependency we break amounts to moving the no-longer-dependent object into post-data, and hence we should mark that object if it's a matview. Back-patch to all supported versions, since the issue's been there since matviews were introduced. Discussion: https://postgr.es/m/15602-e895445f73dc450b@postgresql.org
* Move port-specific parts of with_temp_install to port makefile.Andrew Gierth2019-02-04
| | | | | | | | | | | | | | Rather than define ld_library_path_ver with a big nested $(if), just put the overriding values in the makefiles for the relevant ports. Also add a variable for port makefiles to append their own stuff to with_temp_install, and use it to set LD_LIBRARY_PATH_RPATH=1 on FreeBSD which is needed to make LD_LIBRARY_PATH override DT_RPATH if DT_RUNPATH is not set (which seems to depend in unpredictable ways on the choice of compiler, at least on my system). Backpatch for the benefit of anyone doing regression tests on FreeBSD. (For other platforms there should be no functional change.)
* Add PG_CFLAGS, PG_CXXFLAGS, and PG_LDFLAGS variables to PGXSMichael Paquier2019-02-03
| | | | | | | | | | | | | | | | Add PG_CFLAGS, PG_CXXFLAGS, and PG_LDFLAGS variables to pgxs.mk which will be appended or prepended to the corresponding make variables. Notably, there was previously no way to pass custom CXXFLAGS to third party extension module builds, COPT and PROFILE supporting only CFLAGS and LDFLAGS. Backpatch all the way down to ease integration with existing extensions. Author: Christoph Berg Reviewed-by: Andres Freund, Tom Lane, Michael Paquier Discussion: https://postgr.es/m/20181113104005.GA32154@msg.credativ.de Backpatch-through: 9.4
* Avoid possible deadlock while locking multiple heap pages.Amit Kapila2019-02-02
| | | | | | | | | | | | | | | | | To avoid deadlock, backend acquires a lock on heap pages in block number order. In certain cases, lock on heap pages is dropped and reacquired. In this case, the locks are dropped for reading in corresponding VM page/s. The issue is we re-acquire locks in bufferId order whereas the intention was to acquire in blockid order. This commit ensures that we will always acquire locks on heap pages in blockid order. Reported-by: Nishant Fnu Author: Nishant Fnu Reviewed-by: Amit Kapila and Robert Haas Backpatch-through: 9.4 Discussion: https://postgr.es/m/5883C831-2ED1-47C8-BFAC-2D5BAE5A8CAE@amazon.com
* Fix use of dangling pointer in heap_delete() when logging replica identityMichael Paquier2019-02-01
| | | | | | | | | | | | | | When logging the replica identity of a deleted tuple, XLOG_HEAP_DELETE records include references of the old tuple. Its data is stored in an intermediate variable used to register this information for the WAL record, but this variable gets away from the stack when the record gets actually inserted. Spotted by clang's AddressSanitizer. Author: Stas Kelvish Discussion: https://postgr.es/m/085C8825-AD86-4E93-AF80-E26CDF03D1EA@postgrespro.ru Backpatch-through: 9.4
* Fix a crash in logical replicationPeter Eisentraut2019-01-30
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The bug was that determining which columns are part of the replica identity index using RelationGetIndexAttrBitmap() would run eval_const_expressions() on index expressions and predicates across all indexes of the table, which in turn might require a snapshot, but there wasn't one set, so it crashes. There were actually two separate bugs, one on the publisher and one on the subscriber. To trigger the bug, a table that is part of a publication or subscription needs to have an index with a predicate or expression that lends itself to constant expressions simplification. The fix is to avoid the constant expressions simplification in RelationGetIndexAttrBitmap(), so that it becomes safe to call in these contexts. The constant expressions simplification comes from the calls to RelationGetIndexExpressions()/RelationGetIndexPredicate() via BuildIndexInfo(). But RelationGetIndexAttrBitmap() calling BuildIndexInfo() is overkill. The latter just takes pg_index catalog information, packs it into the IndexInfo structure, which former then just unpacks again and throws away. We can just do this directly with less overhead and skip the troublesome calls to eval_const_expressions(). This also removes the awkward cross-dependency between relcache.c and index.c. Bug: #15114 Reported-by: Петър Славов <pet.slavov@gmail.com> Reviewed-by: Noah Misch <noah@leadboat.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://www.postgresql.org/message-id/flat/152110589574.1223.17983600132321618383@wrigleys.postgresql.org/