Re: pgsql: Use better comment marker in Autoconf input
Andres Freund writes: > On 2019-02-09 14:57:27 +, Peter Eisentraut wrote: >> Use better comment marker in Autoconf input > Weirdly enough this might have had some sideeffects. According to > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=elver=2019-02-09%2014%3A58%3A09 > this is where elver started to fail. Yeah, I noticed that coincidence too. > I'd for a minute just put this down > to an independent upgrade on the animal, but the other branches still > build fine. The error seems to be a library version mismatch, which sure looks like a busted software upgrade. But you have a darn good point: if that's the problem, why didn't v11 break too? > I'm somewhat confused. Me too, now. I stared closely at the configure shell script changes, and they certainly seem to be innocuous. regards, tom lane
Re: pgsql: Use better comment marker in Autoconf input
Hi, On 2019-02-09 14:57:27 +, Peter Eisentraut wrote: > Use better comment marker in Autoconf input > > 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. Weirdly enough this might have had some sideeffects. According to https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=elver=2019-02-09%2014%3A58%3A09 this is where elver started to fail. I'd for a minute just put this down to an independent upgrade on the animal, but the other branches still build fine. I'm somewhat confused. Greetings, Andres Freund
pgsql: Solve cross-version-upgrade testing problem induced by 1fb57af92
Solve cross-version-upgrade testing problem induced by 1fb57af92. 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. Branch -- REL_10_STABLE Details --- https://git.postgresql.org/pg/commitdiff/7cbfd8eb166dd7ab06201f29b72a467233fd7c43 Modified Files -- src/bin/pg_dump/t/002_pg_dump.pl| 4 ++-- src/test/modules/test_ddl_deparse/expected/create_transform.out | 6 -- src/test/modules/test_ddl_deparse/sql/create_transform.sql | 6 -- src/test/regress/expected/object_address.out| 4 +++- src/test/regress/sql/object_address.sql | 4 +++- 5 files changed, 16 insertions(+), 8 deletions(-)
pgsql: Solve cross-version-upgrade testing problem induced by 1fb57af92
Solve cross-version-upgrade testing problem induced by 1fb57af92. 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. Branch -- REL9_5_STABLE Details --- https://git.postgresql.org/pg/commitdiff/1afcf6aed73997c8beb340195a37cf5939c9eb2e Modified Files -- src/test/modules/test_ddl_deparse/expected/create_transform.out | 6 -- src/test/modules/test_ddl_deparse/sql/create_transform.sql | 6 -- src/test/regress/expected/object_address.out| 4 +++- src/test/regress/sql/object_address.sql | 4 +++- 4 files changed, 14 insertions(+), 6 deletions(-)
pgsql: Solve cross-version-upgrade testing problem induced by 1fb57af92
Solve cross-version-upgrade testing problem induced by 1fb57af92. 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. Branch -- REL9_6_STABLE Details --- https://git.postgresql.org/pg/commitdiff/4b235947c7c63a084a90c2e049d79a09ff03b9b3 Modified Files -- src/bin/pg_dump/t/002_pg_dump.pl| 4 ++-- src/test/modules/test_ddl_deparse/expected/create_transform.out | 6 -- src/test/modules/test_ddl_deparse/sql/create_transform.sql | 6 -- src/test/regress/expected/object_address.out| 4 +++- src/test/regress/sql/object_address.sql | 4 +++- 5 files changed, 16 insertions(+), 8 deletions(-)
pgsql: Solve cross-version-upgrade testing problem induced by 1fb57af92
Solve cross-version-upgrade testing problem induced by 1fb57af92. 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. Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/068503c76511cdb0080bab689662a20e86b9c845 Modified Files -- src/bin/pg_dump/t/002_pg_dump.pl| 4 ++-- src/test/modules/test_ddl_deparse/expected/create_transform.out | 6 -- src/test/modules/test_ddl_deparse/sql/create_transform.sql | 6 -- src/test/regress/expected/object_address.out| 4 +++- src/test/regress/sql/object_address.sql | 4 +++- 5 files changed, 16 insertions(+), 8 deletions(-)
pgsql: Repair unsafe/unportable snprintf usage in pg_restore.
Repair unsafe/unportable snprintf usage in pg_restore. 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. Branch -- REL_11_STABLE Details --- https://git.postgresql.org/pg/commitdiff/ef9bf359369b4dfaceb859941c3898b5ce1decf6 Modified Files -- src/bin/pg_dump/pg_backup_archiver.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-)
pgsql: Repair unsafe/unportable snprintf usage in pg_restore.
Repair unsafe/unportable snprintf usage in pg_restore. 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. Branch -- REL9_5_STABLE Details --- https://git.postgresql.org/pg/commitdiff/2c833217713776d8606fb94e9ab3877d102b86a6 Modified Files -- src/bin/pg_dump/pg_backup_archiver.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-)
pgsql: Repair unsafe/unportable snprintf usage in pg_restore.
Repair unsafe/unportable snprintf usage in pg_restore. 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. Branch -- REL_10_STABLE Details --- https://git.postgresql.org/pg/commitdiff/73668c590cf5c5719ac0d0481201fd4fd79454bb Modified Files -- src/bin/pg_dump/pg_backup_archiver.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-)
pgsql: Repair unsafe/unportable snprintf usage in pg_restore.
Repair unsafe/unportable snprintf usage in pg_restore. 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. Branch -- REL9_6_STABLE Details --- https://git.postgresql.org/pg/commitdiff/2b6009e2a2794df286939e5ce1196a23cca07d68 Modified Files -- src/bin/pg_dump/pg_backup_archiver.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-)
pgsql: Repair unsafe/unportable snprintf usage in pg_restore.
Repair unsafe/unportable snprintf usage in pg_restore. 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. Branch -- REL9_4_STABLE Details --- https://git.postgresql.org/pg/commitdiff/ed46d0d322b54b618557b88948c72bc2711d7c1b Modified Files -- src/bin/pg_dump/pg_backup_archiver.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-)
pgsql: Repair unsafe/unportable snprintf usage in pg_restore.
Repair unsafe/unportable snprintf usage in pg_restore. 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. Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/4dbe1969079672064777e71c3fc981abbf55e207 Modified Files -- src/bin/pg_dump/pg_backup_archiver.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-)
pgsql: Build out the planner support function infrastructure.
Build out the planner support function infrastructure. 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.1548028...@sss.pgh.pa.us Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/a391ff3c3d418e404a2c6e4ff0865a107752827b Modified Files -- contrib/postgres_fdw/postgres_fdw.c | 3 +- doc/src/sgml/xfunc.sgml | 21 +++ src/backend/optimizer/path/clausesel.c | 15 ++ src/backend/optimizer/path/costsize.c| 50 --- src/backend/optimizer/util/clauses.c | 27 ++-- src/backend/optimizer/util/pathnode.c| 2 +- src/backend/optimizer/util/plancat.c | 182 +++ src/backend/utils/adt/arrayfuncs.c | 34 + src/backend/utils/adt/int.c | 74 + src/backend/utils/adt/int8.c | 73 + src/backend/utils/adt/selfuncs.c | 13 +- src/backend/utils/cache/lsyscache.c | 45 ++ src/include/catalog/catversion.h | 2 +- src/include/catalog/pg_proc.dat | 25 +++- src/include/nodes/nodes.h| 5 +- src/include/nodes/pathnodes.h| 2 +- src/include/nodes/supportnodes.h | 100 + src/include/optimizer/clauses.h | 2 +- src/include/optimizer/plancat.h | 14 ++ src/include/utils/lsyscache.h| 3 +- src/test/regress/expected/misc_functions.out | 60 src/test/regress/expected/subselect.out | 8 +- src/test/regress/input/create_function_1.source | 5 + src/test/regress/output/create_function_1.source | 4 + src/test/regress/regress.c | 77 ++ src/test/regress/sql/misc_functions.sql | 32 src/test/regress/sql/subselect.sql | 4 +- 27 files changed, 792 insertions(+), 90 deletions(-)
pgsql: Create the infrastructure for planner support functions.
Create the infrastructure for planner support functions. 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.1548028...@sss.pgh.pa.us Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/1fb57af92069ee104c09e2016af9e0e620681be3 Modified Files -- doc/src/sgml/catalogs.sgml | 6 +- doc/src/sgml/keywords.sgml | 7 ++ doc/src/sgml/ref/alter_function.sgml | 19 +++ doc/src/sgml/ref/create_function.sgml | 14 +++ doc/src/sgml/xfunc.sgml| 120 +- doc/src/sgml/xoper.sgml| 12 ++ src/backend/catalog/pg_aggregate.c | 1 + src/backend/catalog/pg_depend.c| 52 +--- src/backend/catalog/pg_proc.c | 12 +- src/backend/commands/functioncmds.c| 88 - src/backend/commands/proclang.c| 3 + src/backend/commands/typecmds.c| 1 + src/backend/optimizer/util/clauses.c | 23 +++- src/backend/parser/gram.y | 7 +- src/backend/utils/adt/date.c | 23 +++- src/backend/utils/adt/datetime.c | 17 ++- src/backend/utils/adt/numeric.c| 69 ++- src/backend/utils/adt/ruleutils.c | 15 +++ src/backend/utils/adt/timestamp.c | 138 ++--- src/backend/utils/adt/varbit.c | 48 --- src/backend/utils/adt/varchar.c| 48 --- src/bin/pg_dump/pg_dump.c | 39 +- src/bin/pg_dump/t/002_pg_dump.pl | 22 +++- src/include/catalog/catversion.h | 2 +- src/include/catalog/pg_proc.dat| 81 ++-- src/include/catalog/pg_proc.h | 5 +- src/include/nodes/nodes.h | 3 +- src/include/nodes/supportnodes.h | 70 +++ src/include/parser/kwlist.h| 1 + src/include/utils/datetime.h | 2 +- .../test_ddl_deparse/expected/create_transform.out | 2 +- .../test_ddl_deparse/sql/create_transform.sql | 2 +- src/test/regress/expected/alter_table.out | 5 +- src/test/regress/expected/object_address.out | 2 +- src/test/regress/expected/oidjoins.out | 10 +- src/test/regress/expected/opr_sanity.out | 4 +- src/test/regress/sql/alter_table.sql | 5 +- src/test/regress/sql/object_address.sql| 2 +- src/test/regress/sql/oidjoins.sql | 6 +- src/test/regress/sql/opr_sanity.sql| 4 +- src/tools/findoidjoins/README | 2 +- 41 files changed, 698 insertions(+), 294 deletions(-)
pgsql: Refactor the representation of indexable clauses in IndexPaths.
Refactor the representation of indexable clauses in IndexPaths. 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.1549124...@sss.pgh.pa.us Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/1a8d5afb0dfc5d0dcc6eda0656a34cb1f0cf0bdf Modified Files -- src/backend/nodes/nodeFuncs.c | 22 ++ src/backend/nodes/outfuncs.c | 17 +- src/backend/optimizer/path/costsize.c | 43 ++- src/backend/optimizer/path/equivclass.c | 37 +++ src/backend/optimizer/path/indxpath.c | 404 ++ src/backend/optimizer/plan/createplan.c | 332 ++--- src/backend/optimizer/plan/planner.c | 2 +- src/backend/optimizer/util/clauses.c | 65 - src/backend/optimizer/util/pathnode.c | 17 +- src/backend/optimizer/util/restrictinfo.c | 64 src/backend/utils/adt/selfuncs.c | 149 +- src/include/nodes/nodes.h | 1 + src/include/nodes/pathnodes.h | 76 +++-- src/include/optimizer/clauses.h | 1 - src/include/optimizer/pathnode.h | 1 - src/include/optimizer/paths.h | 10 +- src/include/optimizer/restrictinfo.h | 1 + src/include/utils/selfuncs.h | 1 - src/test/regress/expected/create_index.out| 13 +- src/test/regress/expected/join.out| 16 +- src/test/regress/expected/partition_join.out | 12 +- src/test/regress/expected/partition_prune.out | 36 +-- 22 files changed, 698 insertions(+), 622 deletions(-)
Re: pgsql: Allow some recovery parameters to be changed with reload
Greetings, * Tom Lane (t...@sss.pgh.pa.us) wrote: > Alvaro Herrera writes: > > On 2019-Feb-08, Michael Paquier wrote: > >> The timestamp of this commit is a bit messed up: > >> commit: 13b89f96d07ad3da67b57f66c134c3609bd3e98f > >> author: Peter Eisentraut > >> date: Mon, 4 Feb 2019 09:28:17 +0100 > >> committer: Peter Eisentraut > >> date: Thu, 7 Feb 2019 08:34:48 +0100 > >> > >> Perhaps you overlooked a --reset-author switch? > > > I don't think we actually have a rule about these timestamps, and I > > don't think we really care, do we? > > Yeah, if you want to see a sequence of dates that makes sense, > you need to look at the commit-date. Whether the author-date > closely matches that depends on the particular committer's > workflow. While I agree that we don't really have a formal policy, there are certainly some who do (or, at least did) seem to care quite a bit about this and that's why I've been using '--ignore-date' for quite some time in my workflow: https://www.postgresql.org/message-id/CA%2BTgmobEgs1%3DAT0_SRvf6K9XrG7QAUyRNeuv5D9oaXrmpST9fw%40mail.gmail.com Thanks! Stephen signature.asc Description: PGP signature
pgsql: Call set_rel_pathlist_hook before generate_gather_paths, not aft
Call set_rel_pathlist_hook before generate_gather_paths, not after. 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+-h...@mail.gmail.com Branch -- REL_11_STABLE Details --- https://git.postgresql.org/pg/commitdiff/027b5a300a9e9b407f465f0264cb88305eb0539d Modified Files -- doc/src/sgml/custom-scan.sgml | 5 +++-- src/backend/optimizer/path/allpaths.c | 25 ++--- 2 files changed, 17 insertions(+), 13 deletions(-)
pgsql: Call set_rel_pathlist_hook before generate_gather_paths, not aft
Call set_rel_pathlist_hook before generate_gather_paths, not after. 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+-h...@mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/6401583863eaf83624994908911350b03f9978ae Modified Files -- doc/src/sgml/custom-scan.sgml | 5 +++-- src/backend/optimizer/path/allpaths.c | 25 ++--- 2 files changed, 17 insertions(+), 13 deletions(-)
pgsql: Call set_rel_pathlist_hook before generate_gather_paths, not aft
Call set_rel_pathlist_hook before generate_gather_paths, not after. 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+-h...@mail.gmail.com Branch -- REL9_6_STABLE Details --- https://git.postgresql.org/pg/commitdiff/6257f525aff4e711d7eb5405626e7eeac37e2ff5 Modified Files -- doc/src/sgml/custom-scan.sgml | 5 +++-- src/backend/optimizer/path/allpaths.c | 27 --- 2 files changed, 19 insertions(+), 13 deletions(-)
pgsql: Call set_rel_pathlist_hook before generate_gather_paths, not aft
Call set_rel_pathlist_hook before generate_gather_paths, not after. 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+-h...@mail.gmail.com Branch -- REL_10_STABLE Details --- https://git.postgresql.org/pg/commitdiff/dc0eb137fec20c4f2d168dfcb6bda001a27ad548 Modified Files -- doc/src/sgml/custom-scan.sgml | 5 +++-- src/backend/optimizer/path/allpaths.c | 27 --- 2 files changed, 19 insertions(+), 13 deletions(-)
Re: pgsql: Restrict the use of temporary namespace in two-phase transaction
On 08/02/2019 11:00, Michael Paquier wrote: > On Fri, Feb 08, 2019 at 10:41:59AM +0100, Peter Eisentraut wrote: >> We usually don't use "namespace" in user-facing error messages. Can you >> change it to say "temporary schema"? > > Or just switch to "temporary objects" like it's done on HEAD for the > second message? Yeah, even better. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
pgsql: Use better comment marker in Autoconf input
Use better comment marker in Autoconf input 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. Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/4446565d36ac4482282146a9ab35068f18dff4fd Modified Files -- config/llvm.m4 | 2 +- config/programs.m4 | 2 +- configure | 9 + configure.in | 12 ++-- 4 files changed, 9 insertions(+), 16 deletions(-)
pgsql: For 11 only, put back heap_expand_tuple to GetTupleForTrigger().
For 11 only, put back heap_expand_tuple to GetTupleForTrigger(). 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@news-spur.riddles.org.uk Branch -- REL_11_STABLE Details --- https://git.postgresql.org/pg/commitdiff/920311ab18aac799aee6ad2303b2ed2b6b44c1b8 Modified Files -- src/backend/commands/trigger.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-)
pgsql: Allow to reset execGrouping.c style tuple hashtables.
Allow to reset execGrouping.c style tuple hashtables. 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-05850f065da42...@postgresql.org https://postgr.es/m/20190114180423.ywhdg2iagzvh4...@alap3.anarazel.de Backpatch: 11, this is a prerequisite for other fixes Branch -- REL_11_STABLE Details --- https://git.postgresql.org/pg/commitdiff/6455c65882474a48b6bde298bd04c18aa4e4b27f Modified Files -- src/backend/executor/execGrouping.c | 70 + src/include/executor/executor.h | 10 ++ 2 files changed, 66 insertions(+), 14 deletions(-)
pgsql: Allow to reset execGrouping.c style tuple hashtables.
Allow to reset execGrouping.c style tuple hashtables. 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-05850f065da42...@postgresql.org https://postgr.es/m/20190114180423.ywhdg2iagzvh4...@alap3.anarazel.de Backpatch: 11, this is a prerequisite for other fixes Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/317ffdfeaac12e434b2befa24993dc1b52a140fd Modified Files -- src/backend/executor/execGrouping.c | 70 + src/include/executor/executor.h | 10 ++ 2 files changed, 66 insertions(+), 14 deletions(-)
pgsql: Reset, not recreate, execGrouping.c style hashtables.
Reset, not recreate, execGrouping.c style hashtables. 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-05850f065da42...@postgresql.org https://postgr.es/m/20190114180423.ywhdg2iagzvh4...@alap3.anarazel.de Backpatch: 11, where I broke this in bf6c614a2f2c5 Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/356687bd825e5ca7230d43c1bffe7a59ad2e77bd Modified Files -- src/backend/executor/nodeAgg.c| 34 ++ src/backend/executor/nodeRecursiveunion.c | 27 --- src/backend/executor/nodeSetOp.c | 25 +++--- src/backend/executor/nodeSubplan.c| 57 ++- 4 files changed, 79 insertions(+), 64 deletions(-)
pgsql: Plug leak in BuildTupleHashTable by creating ExprContext in corr
Plug leak in BuildTupleHashTable by creating ExprContext in correct context. 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.h6r3fuyxjxkyk...@alap3.anarazel.de Backpatch: 11, where I broke this in bf6c614a2f2c5 Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/5567d12ce030781a4f749c9cd2034b95a3b64424 Modified Files -- src/backend/executor/execGrouping.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-)
pgsql: Reset, not recreate, execGrouping.c style hashtables.
Reset, not recreate, execGrouping.c style hashtables. 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-05850f065da42...@postgresql.org https://postgr.es/m/20190114180423.ywhdg2iagzvh4...@alap3.anarazel.de Backpatch: 11, where I broke this in bf6c614a2f2c5 Branch -- REL_11_STABLE Details --- https://git.postgresql.org/pg/commitdiff/35afccaba6d0e0aa14e3d1f859e6d84e69aee2cc Modified Files -- src/backend/executor/nodeAgg.c| 34 ++ src/backend/executor/nodeRecursiveunion.c | 27 --- src/backend/executor/nodeSetOp.c | 25 +++--- src/backend/executor/nodeSubplan.c| 57 ++- 4 files changed, 79 insertions(+), 64 deletions(-)
pgsql: simplehash: Add support for resetting a hashtable's contents.
simplehash: Add support for resetting a hashtable's contents. A hashtable reset just reset the hashtable entries, but does not free memory. Author: Andres Freund Discussion: https://postgr.es/m/20190114180423.ywhdg2iagzvh4...@alap3.anarazel.de Bug: #15592 #15486 Backpatch: 11, this is a prerequisite for other fixes Branch -- REL_11_STABLE Details --- https://git.postgresql.org/pg/commitdiff/350b0a40375e5fa171da15bd3062de83c54cd099 Modified Files -- src/include/lib/simplehash.h | 11 +++ 1 file changed, 11 insertions(+)
pgsql: Plug leak in BuildTupleHashTable by creating ExprContext in corr
Plug leak in BuildTupleHashTable by creating ExprContext in correct context. 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.h6r3fuyxjxkyk...@alap3.anarazel.de Backpatch: 11, where I broke this in bf6c614a2f2c5 Branch -- REL_11_STABLE Details --- https://git.postgresql.org/pg/commitdiff/9cf37a527cf83e94f8f166d380baf53287a0337b Modified Files -- src/backend/executor/execGrouping.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-)
pgsql: simplehash: Add support for resetting a hashtable's contents.
simplehash: Add support for resetting a hashtable's contents. A hashtable reset just reset the hashtable entries, but does not free memory. Author: Andres Freund Discussion: https://postgr.es/m/20190114180423.ywhdg2iagzvh4...@alap3.anarazel.de Bug: #15592 #15486 Backpatch: 11, this is a prerequisite for other fixes Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/3b632a58e7985c6589dbd0e6c0f32ba007783cfa Modified Files -- src/include/lib/simplehash.h | 11 +++ 1 file changed, 11 insertions(+)
