aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* 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 SCRAM authentication via SSL when mixing versions of OpenSSLMichael Paquier2019-02-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When using a libpq client linked with OpenSSL 1.0.1 or older to connect to a backend linked with OpenSSL 1.0.2 or newer, the server would send SCRAM-SHA-256-PLUS and SCRAM-SHA-256 as valid mechanisms for the SASL exchange, and the client would choose SCRAM-SHA-256-PLUS even if it does not support channel binding, leading to a confusing error. In this case, what the client ought to do is switch to SCRAM-SHA-256 so as the authentication can move on and succeed. So for a SCRAM authentication over SSL, here are all the cases present and how we deal with them using libpq: 1) Server supports channel binding, it sends SCRAM-SHA-256-PLUS and SCRAM-SHA-256 as allowed mechanisms. 1-1) Client supports channel binding, chooses SCRAM-SHA-256-PLUS. 1-2) Client does not support channel binding, chooses SCRAM-SHA-256. 2) Server does not support channel binding, sends SCRAM-SHA-256 as allowed mechanism. 2-1) Client supports channel binding, still it has no choice but to choose SCRAM-SHA-256. 2-2) Client does not support channel binding, it chooses SCRAM-SHA-256. In all these scenarios the connection should succeed, and the one which was handled incorrectly prior this commit is 1-2), causing the connection attempt to fail because client chose SCRAM-SHA-256-PLUS over SCRAM-SHA-256. Reported-by: Hugh Ranalli Diagnosed-by: Peter Eisentraut Author: Michael Paquier Reviewed-by: Peter Eisentraut Discussion: https://postgr.es/m/CAAhbUMO89SqUk-5mMY+OapgWf-twF2NA5sCucbHEzMfGbvcepA@mail.gmail.com Backpatch-through: 11
* 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
* Fix incorrect strictness test for ArrayCoerceExpr expressions.Tom Lane2019-02-20
| | | | | | | | | | | | | | | | | The recursion in contain_nonstrict_functions_walker() was done wrong, causing the strictness check to be bypassed for a parse node that is the immediate input of an ArrayCoerceExpr node. This could allow, for example, incorrect decisions about whether a strict SQL function can be inlined. I didn't add a regression test, because (a) the bug is so narrow and (b) I couldn't think of a test case that wasn't dependent on a large number of other behaviors, to the point where it would likely soon rot to the point of not testing what it was intended to. I broke this in commit c12d570fa, so back-patch to v11. Discussion: https://postgr.es/m/27571.1550617881@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 some issues with TAP tests of pg_basebackupMichael Paquier2019-02-18
| | | | | | | | | | | ee9e145 has fixed the tests of pg_basebackup for checksums a first time, still one seek() call missed the shot. Also, the data written in files to emulate corruptions was not actually writing zeros as the quoting style was incorrect. Author: Michael Banck Discussion: https://postgr.es/m/1550153276.796.35.camel@credativ.de Backpatch-through: 11
* 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
* Clean up planner confusion between ncolumns and nkeycolumns.Tom Lane2019-02-12
| | | | | | | | | | | | | | | | | | | | | | | | | We're only going to consider key columns when creating indexquals, so there is no point in having the outer loops in indxpath.c iterate further than nkeycolumns. Doing so in match_pathkeys_to_index() is actually wrong, and would have caused crashes by now, except that we have no index AMs supporting both amcanorderbyop and amcaninclude. It's also wrong in relation_has_unique_index_for(). The effect there is to fail to prove uniqueness even when the index does prove it, if there are extra columns. Also future-proof examine_variable() for the day when extra columns can be expressions, and fix what's either a thinko or just an oversight in btcostestimate(): we should consider the number of key columns, not the total, when deciding whether to derate correlation. None of these things seemed important enough to risk changing in a just-before-wrap patch, but since we're past the release wrap window, time to fix 'em. Discussion: https://postgr.es/m/25526.1549847928@sss.pgh.pa.us
* 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 11.2.REL_11_2Tom Lane2019-02-11
|
* Translation updatesPeter Eisentraut2019-02-11
| | | | | Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git Source-Git-Hash: 2cd47eeb832ed1bb1cbfff285cfc921ca4d07a9d
* 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
* Fix indexable-row-comparison logic to account for covering indexes.Tom Lane2019-02-10
| | | | | | | | | | | | | | | | | | indxpath.c needs a good deal more attention for covering indexes than it's gotten. But so far as I can tell, the only really awful breakage is in expand_indexqual_rowcompare (nee adjust_rowcompare_for_index), which was only half fixed in c266ed31a. The other problems aren't bad enough to take the risk of a just-before-wrap fix. The problem here is that if the leading column of a row comparison matches an index (allowing this code to be reached), and some later column doesn't match the index, it'll nonetheless believe that that column matches the first included index column. Typically that'll lead to an error like "operator M is not a member of opfamily N" as a result of fetching a garbage opfamily OID. But with enough bad luck, maybe a broken plan would be generated. Discussion: https://postgr.es/m/25526.1549847928@sss.pgh.pa.us
* Fix trigger drop procedureAlvaro Herrera2019-02-10
| | | | | | | | | | | After commit 123cc697a8eb, we remove redundant FK action triggers during partition ATTACH by merely deleting the catalog tuple, but that's wrong: it should use performDeletion() instead. Repair, and make the comments more explicit. Per code review from Tom Lane. Discussion: https://postgr.es/m/18885.1549642539@sss.pgh.pa.us
* 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
* For 11 only, put back heap_expand_tuple to GetTupleForTrigger().Andres Freund2019-02-09
| | | | | | | | | | | | This is not necessary anymore after 297d627e, but extensions that have not been recompiled after the fix will not use the new definition of heap_getattr(). While recompiling those extensions is obviously the suggested course, it's cheap enough to retain the expansion in GetTupleForTrigger(). Per suggestion from Andrew Gierth. Discussion: 87va1x43ot.fsf@news-spur.riddles.org.uk
* Reset, not recreate, execGrouping.c style hashtables.Andres Freund2019-02-09
| | | | | | | | | | | | | | | | This uses the facility added in the preceding commit to fix performance issues caused by rebuilding the hashtable (with its comparator expression being the most expensive bit), after every reset. That's especially important when the comparator is JIT compiled. Bug: #15592 #15486 Reported-By: Jakub Janeček, Dmitry Marakasov Author: Andres Freund Discussion: https://postgr.es/m/15486-05850f065da42931@postgresql.org https://postgr.es/m/20190114180423.ywhdg2iagzvh43we@alap3.anarazel.de Backpatch: 11, where I broke this in bf6c614a2f2c5
* Allow to reset execGrouping.c style tuple hashtables.Andres Freund2019-02-09
| | | | | | | | | | | | | | | | | | | | | | | | | | This has the advantage that the comparator expression, the table's slot, etc do not have to be rebuilt. Additionally the simplehash.h hashtable within the tuple hashtable now keeps its previous size and doesn't need to be reallocated. That both reduces allocator overhead, and improves performance in cases where the input estimation was off by a significant factor. To avoid an API/ABI break, the new parameter is exposed via the new BuildTupleHashTableExt(), and BuildTupleHashTable() now is a wrapper around the former, that continues to allocate the table itself in the tablecxt. Using this fixes performance issues discovered in the two bugs referenced. This commit however has not converted the callers, that's done in a separate commit. Bug: #15592 #15486 Reported-By: Jakub Janeček, Dmitry Marakasov Author: Andres Freund Discussion: https://postgr.es/m/15486-05850f065da42931@postgresql.org https://postgr.es/m/20190114180423.ywhdg2iagzvh43we@alap3.anarazel.de Backpatch: 11, this is a prerequisite for other fixes
* simplehash: Add support for resetting a hashtable's contents.Andres Freund2019-02-09
| | | | | | | | | | A hashtable reset just reset the hashtable entries, but does not free memory. Author: Andres Freund Discussion: https://postgr.es/m/20190114180423.ywhdg2iagzvh43we@alap3.anarazel.de Bug: #15592 #15486 Backpatch: 11, this is a prerequisite for other fixes
* Plug leak in BuildTupleHashTable by creating ExprContext in correct context.Andres Freund2019-02-09
| | | | | | | | | | | | | | | | | | | | In bf6c614a2f2c5 I added a expr context to evaluate the grouping expression. Unfortunately the code I added initialized them while in the calling context, rather the table context. Additionally, I used CreateExprContext() rather than CreateStandaloneExprContext(), which creates the econtext in the estate's query context. Fix that by using CreateStandaloneExprContext when in the table's tablecxt. As we rely on the memory being freed by a memory context reset that means that the econtext's shutdown callbacks aren't being called, but that seems ok as the expressions are tightly controlled due to ExecBuildGroupingEqual(). Bug: #15592 Reported-By: Dmitry Marakasov Author: Andres Freund Discussion: https://postgr.es/m/20190114222838.h6r3fuyxjxkykf6t@alap3.anarazel.de Backpatch: 11, where I broke this in bf6c614a2f2c5
* 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
* Add collation assignment to CALL statementPeter Eisentraut2019-02-07
| | | | | | | | | Otherwise functions that require collation information will not have it if they are called in arguments to a CALL statement. Reported-by: Jean-Marc Voillequin <Jean-Marc.Voillequin@moodys.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/1EC8157EB499BF459A516ADCF135ADCE39FFAC54%40LON-WGMSX712.ad.moodys.net
* 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 heap_getattr() handling of fast defaults.Andres Freund2019-02-06
| | | | | | | | | | | | | | | | | | | | | | | Previously heap_getattr() returned NULL for attributes with a fast default value (c.f. 16828d5c0273), as it had no handling whatsoever for that case. A previous fix, 7636e5c60f, attempted to fix issues caused by this oversight, but just expanding OLD tuples for triggers doesn't actually solve the underlying issue. One known consequence of this bug is that the check for HOT updates can return the wrong result, when a previously fast-default'ed column is set to NULL. Which in turn means that an index over a column with fast default'ed columns might be corrupt if the underlying column(s) allow NULLs. Fix by handling fast default columns in heap_getattr(), remove now superfluous expansion in GetTupleForTrigger(). Author: Andres Freund Discussion: https://postgr.es/m/20190201162404.onngi77f26baem4g@alap3.anarazel.de Backpatch: 11, where fast defaults were introduced
* 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.
generated by cgit v1.2.3 (git 2.25.1) at 2025-08-03 16:46:51 +0000