- 11 Feb, 2019 3 commits
-
-
Peter Eisentraut authored
-
Peter Eisentraut authored
Last use was removed in 2c66f992.
-
Tom Lane authored
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 c266ed31. 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
-
- 10 Feb, 2019 4 commits
-
-
Tom Lane authored
It seems useful to have this information available, so that it's easier to tell when a test script is taking a disproportionate amount of time. Discussion: https://postgr.es/m/16646.1549770618@sss.pgh.pa.us
-
Alvaro Herrera authored
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
-
Tom Lane authored
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.
-
Tom Lane authored
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.
-
- 09 Feb, 2019 9 commits
-
-
Tom Lane authored
Add support function requests for estimating the selectivity, cost, and number of result rows (if a SRF) of the target function. The lack of a way to estimate selectivity of a boolean-returning function in WHERE has been a recognized deficiency of the planner since Berkeley days. This commit finally fixes it. In addition, non-constant estimates of cost and number of output rows are now possible. We still fall back to looking at procost and prorows if the support function doesn't service the request, of course. To make concrete use of the possibility of estimating output rowcount for SRFs, this commit adds support functions for array_unnest(anyarray) and the integer variants of generate_series; the lack of plausible rowcount estimates for those, even when it's obvious to a human, has been a repeated subject of complaints. Obviously, much more could now be done in this line, but I'm mostly just trying to get the infrastructure in place. Discussion: https://postgr.es/m/15193.1548028093@sss.pgh.pa.us
-
Tom Lane authored
Rename/repurpose pg_proc.protransform as "prosupport". The idea is still that it names an internal function that provides knowledge to the planner about the behavior of the function it's attached to; but redesign the API specification so that it's not limited to doing just one thing, but can support an extensible set of requests. The original purpose of simplifying a function call is handled by the first request type to be invented, SupportRequestSimplify. Adjust all the existing transform functions to handle this API, and rename them fron "xxx_transform" to "xxx_support" to reflect the potential generalization of what they do. (Since we never previously provided any way for extensions to add transform functions, this change doesn't create an API break for them.) Also add DDL and pg_dump support for attaching a support function to a user-defined function. Unfortunately, DDL access has to be restricted to superusers, at least for now; but seeing that support functions will pretty much have to be written in C, that limitation is just theoretical. (This support is untested in this patch, but a follow-on patch will add cases that exercise it.) Discussion: https://postgr.es/m/15193.1548028093@sss.pgh.pa.us
-
Tom Lane authored
In place of three separate but interrelated lists (indexclauses, indexquals, and indexqualcols), an IndexPath now has one list "indexclauses" of IndexClause nodes. This holds basically the same information as before, but in a more useful format: in particular, there is now a clear connection between an indexclause (an original restriction clause from WHERE or JOIN/ON) and the indexquals (directly usable index conditions) derived from it. We also change the ground rules a bit by mandating that clause commutation, if needed, be done up-front so that what is stored in the indexquals list is always directly usable as an index condition. This gets rid of repeated re-determination of which side of the clause is the indexkey during costing and plan generation, as well as repeated lookups of the commutator operator. To minimize the added up-front cost, the typical case of commuting a plain OpExpr is handled by a new special-purpose function commute_restrictinfo(). For RowCompareExprs, generating the new clause properly commuted to begin with is not really any more complex than before, it's just different --- and we can save doing that work twice, as the pretty-klugy original implementation did. Tracking the connection between original and derived clauses lets us also track explicitly whether the derived clauses are an exact or lossy translation of the original. This provides a cheap solution to getting rid of unnecessary rechecks of boolean index clauses, which previously seemed like it'd be more expensive than it was worth. Another pleasant (IMO) side-effect is that EXPLAIN now always shows index clauses with the indexkey on the left; this seems less confusing. This commit leaves expand_indexqual_conditions() and some related functions in a slightly messy state. I didn't bother to change them any more than minimally necessary to work with the new data structure, because all that code is going to be refactored out of existence in a follow-on patch. Discussion: https://postgr.es/m/22182.1549124950@sss.pgh.pa.us
-
Tom Lane authored
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
-
Peter Eisentraut authored
The comment marker "#" is copied to the output, so it's only appropriate for comments that make sense in the shell output. For comments about the Autoconf language, "dnl" should be used.
-
Andres Freund authored
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 bf6c614a
-
Andres Freund authored
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
-
Andres Freund authored
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
-
Andres Freund authored
In bf6c614a 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 bf6c614a
-
- 08 Feb, 2019 4 commits
-
-
Tom Lane authored
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
-
Tom Lane authored
Also clean up some discussion that had been left in a very confused state thanks to half-hearted adjustments for the change to standard_conforming_strings being the default. Discussion: https://postgr.es/m/154954987367.1297.4358910045409218@wrigleys.postgresql.org
-
Peter Eisentraut authored
In case of a reload, we just want to LOG errors instead of FATAL when processing SSL configuration, but the more recent code for the ssl_*_protocol_version settings didn't behave like that. Author: Daniel Gustafsson <daniel@yesql.se> Reviewed-by: Michael Paquier <michael@paquier.xyz>
-
Peter Eisentraut authored
These mainly help understanding the function signatures better.
-
- 07 Feb, 2019 10 commits
-
-
Michael Paquier authored
This is useful when looking at partition trees with multiple layers, and combined with pg_partition_tree, it provides the possibility to show up an entire tree by just knowing one member at any level. Author: Michael Paquier Reviewed-by: Álvaro Herrera, Amit Langote Discussion: https://postgr.es/m/20181207014015.GP2407@paquier.xyz
-
Tom Lane authored
Up to now postgres_fdw has been using create_foreignscan_path() to generate not only base-relation paths, but also paths for foreign joins and foreign upperrels. This is wrong, because create_foreignscan_path() calls get_baserel_parampathinfo() which will only do the right thing for baserels. It accidentally fails to fail for unparameterized paths, which are the only ones postgres_fdw (thought it) was handling, but we really need different APIs for the baserel and join cases. In HEAD, the best thing to do seems to be to split up the baserel, joinrel, and upperrel cases into three functions so that they can have different APIs. I haven't actually given create_foreign_join_path a different API in this commit: we should spend a bit of time thinking about just what we want to do there, since perhaps FDWs would want to do something different from the build-up-a-join-pairwise approach that get_joinrel_parampathinfo expects. In the meantime, since postgres_fdw isn't prepared to generate parameterized joins anyway, just give it a defense against trying to plan joins with lateral refs. In addition (and this is what triggered this whole mess) fix bug #15613 from Srinivasan S A, by teaching file_fdw and postgres_fdw that plain baserel foreign paths still have outer refs if the relation has lateral_relids. Add some assertions in relnode.c to catch future occurrences of the same error --- in particular, to catch other FDWs doing that, but also as backstop against core-code mistakes like the one fixed by commit bdd9a99a. Bug #15613 also needs to be fixed in the back branches, but the appropriate fix will look quite a bit different there, since we don't want to assume that existing FDWs get the word right away. Discussion: https://postgr.es/m/15613-092be1be9576c728@postgresql.org
-
Andrew Dunstan authored
as found by running src/tools/perlcheck/pgperlsyncheck
-
Andrew Dunstan authored
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
-
Peter Eisentraut authored
Change pg_dump and ruleutils.c to use the FUNCTION keyword instead of PROCEDURE in trigger and event trigger definitions. This completes the pieces of the transition started in 0a63f996 that were kept out of PostgreSQL 11 because of the required catversion change. Discussion: https://www.postgresql.org/message-id/381bef53-f7be-29c8-d977-948e389161d6@2ndquadrant.com
-
Peter Eisentraut authored
Change archive_cleanup_command promote_trigger_file recovery_end_command recovery_min_apply_delay from PGC_POSTMASTER to PGC_SIGHUP. This did not require any further changes. Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://www.postgresql.org/message-id/flat/ca28011a-cfaa-565c-d622-c1907c33ecf7%402ndquadrant.com
-
Peter Eisentraut authored
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
-
Amit Kapila authored
In commit f16241be, we have changed the behavior for concurrent updates that move row to a different partition, but forgot to update the docs. Previously when an UPDATE command causes a row to move from one partition to another, there is a chance that another concurrent UPDATE or DELETE misses this row. However, now we raise a serialization failure error in such a case. Reported-by: David Rowley Author: David Rowley and Amit Kapila Backpatch-through: 11 where it was introduced Discussion: https://postgr.es/m/CAKJS1f-iVhGD4-givQWpSROaYvO3c730W8yoRMTF9Gc3craY3w@mail.gmail.com
-
Michael Paquier authored
This enforces one-or-more character matches in the regular expressions for pg_dump testing on SQL syntax output where zero-or-more matches implies a syntax error. Author: Daniel Gustafsson Reviewed-by: David G. Johnston, Michael Paquier Discussion: https://postgr.es/m/B313C32C-0E24-4AFB-95FF-6DA0C4E18A89@yesql.se
-
Michael Paquier authored
The relation creation is done at executor startup, however the main regression test suite is lacking scenarios where no data is inserted which is something that can happen when using EXECUTE or EXPLAIN with CREATE TABLE AS and WITH NO DATA. Some patches are worked on to reshape the way CTAS relations are created, so this makes sure that we do not miss some query patterns already supported. Reported-by: Andreas Karlsson Author: Michael Paquier Reviewed-by: Andreas Karlsson Discussion: https://postgr.es/m/20190206091817.GB14980@paquier.xyz
-
- 06 Feb, 2019 7 commits
-
-
Peter Geoghegan authored
The previous tacit assumption that index_form_tuple() hides differences in the TOAST state of its input datums was wrong. Normalize input varlena datums by decompressing compressed values, and forming a new index tuple for fingerprinting using uncompressed inputs. The final normalized representation may actually be compressed once again within index_form_tuple(), though that shouldn't matter. When the original tuple is found to have no datums that are compressed inline, fingerprint the original tuple directly. Normalization avoids false positive reports of corruption in certain cases. For example, the executor can apply toasting with some inline compression to an entire heap tuple because its input has a single external TOAST pointer. Varlena datums for other attributes that are not particularly good candidates for inline compression can be compressed in the heap tuple in passing, without the representation of the same values in index tuples ever receiving concomitant inline compression. Add a test case to recreate the issue in a simpler though less realistic way: by exploiting differences in pg_attribute.attstorage between heap and index relations. This bug was discovered by me during testing of an upcoming set of nbtree enhancements. It was also independently reported by Andreas Kunert, as bug #15597. His test case was rather more realistic than the one I ended up using. Bug: #15597 Discussion: https://postgr.es/m/CAH2-WznrVd9ie+TTJ45nDT+v2nUt6YJwQrT9SebCdQKtAvfPZw@mail.gmail.com Discussion: https://postgr.es/m/15597-294e5d3e7f01c407@postgresql.org Backpatch: 11-, where heapallindexed verification was introduced.
-
Peter Eisentraut authored
These are not relevant to the tests and would just uselessly bloat patches.
-
Tom Lane authored
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
-
Andrew Dunstan authored
Commit f83419b7 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.
-
Andres Freund authored
Previously heap_getattr() returned NULL for attributes with a fast default value (c.f. 16828d5c), as it had no handling whatsoever for that case. A previous fix, 7636e5c6, 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
-
Michael Paquier authored
Some tests have been using regular expressions which have been lax in escaping dots, which may cause tests to pass when they should not. This make the whole set of tests more robust where needed. Author: David Rowley Reviewed-by: Daniel Gustafsson, Michael Paquier Discussion: https://postgr.es/m/CAKJS1f9jD8aVo1BTH+Vgwd=f-ynbuRVrS90XbWMT6UigaOQJTA@mail.gmail.com
-
Andrew Dunstan authored
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.
-
- 05 Feb, 2019 3 commits
-
-
Andrew Dunstan authored
It doesn't like code before "use strict;".
-
Tom Lane authored
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.
-
Andrew Dunstan authored
This was fixed for MSVC tools by commit 1df92eea, but per buildfarm member bowerbird genbki.pl needs the same treatment. Backpatch to all live branches.
-