Tom Lane [Wed, 22 Apr 2020 19:27:43 +0000 (15:27 -0400)]
Sync up some inconsistent comments in config/c-compiler.m4.
Make header/trailer comments agree with the actual names of some macros.
These seem like legit names in earlier iterations of respective patches
(commit
b779168ff "Detect PG_PRINTF_ATTRIBUTE automatically." and
commit
6869b4f25 "Add C++ support to configure.") but the macro had
been renamed out of sync with the header / trailer comment in the final
committed patch.
Even more nitpickily, make the dashed underlines agree with the lengths
of the macro names everyplace. There doesn't seem to have been any
meeting of the minds previously on whether those should match or not,
but at least some people have been trying to make 'em match.
Jesse Zhang, Tom Lane
Discussion: https://postgr.es/m/CAGf+fX7DDyq6WfCy6X_KtD28MkbNBE6NkRi26fSf25dfUwX0zw@mail.gmail.com
Tom Lane [Wed, 22 Apr 2020 18:43:26 +0000 (14:43 -0400)]
Doc: update section 9.11 for new function table layout.
This also makes an attempt to flesh out the docs for some of the more
severely underdocumented geometric operators and functions.
This effort exposed that the point <^ point (point_below) and
point >^ point (point_above) operators are misnamed; they should be
<<| and |>>, because they act like the other operators named that
way and not like the other operators named <^ and >^. But I just
documented them that way; fixing it is matter for another patch.
The haphazard datatype coverage of many of the operators is also
now depressingly obvious.
Discussion: https://postgr.es/m/
158110996889.1089.
4224139874633222837@wrigleys.postgresql.org
David Rowley [Wed, 22 Apr 2020 10:12:19 +0000 (22:12 +1200)]
Remove bogus Assert in foreign key cloning code
This Assert was trying to ensure that the number of columns in the foreign
key being cloned was the same number of attributes in the parentRel. Of
course, it's perfectly valid to have columns in the table which are not
part of the foreign key constraint. It appears that this Assert was
misunderstanding that.
Reported-by: Rajkumar Raghuwanshi
Reviewed-by: amul sul
Discussion: https://postgr.es/m/CAKcux6=z1dtiWw5BOpqDx-U6KTiq+zD0Y2m810zUtWL+giVXWA@mail.gmail.com
Peter Eisentraut [Tue, 21 Apr 2020 17:57:33 +0000 (19:57 +0200)]
Remove HEAPDEBUGALL
This has been broken since PostgreSQL 12 and was probably never really
used. PostgreSQL 12 added an analogous HEAPAMSLOTDEBUGALL, which
still works right now, but it's also not very useful, so remove that
as well.
Discussion: https://www.postgresql.org/message-id/flat/
645c0646-4218-d4c3-409a-
a7003a0c108d%402ndquadrant.com
Michael Paquier [Tue, 21 Apr 2020 23:08:28 +0000 (08:08 +0900)]
Fix single-record reads to use restore_command if available in pg_rewind
readOneRecord() is used now when looking for a checkpoint record to
check if the target server is an ancestor of the source across multiple
timelines, and using a restore_command if available improves the
stability of the operation. This part was missed in
a7e8ece.
Reported-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/
20200421.150830.
1410714948345179794.horikyota.ntt@gmail.com
Alvaro Herrera [Tue, 21 Apr 2020 22:37:26 +0000 (18:37 -0400)]
psql \d: Display table where trigger is defined, if inherited
It's important to know that a trigger is cloned from a parent table,
because of the behavior that the trigger is dropped on detach. Make
psql's \d display it.
We'd like to backpatch, but lack of the pg_trigger.tgparentid column
makes it more difficult. Punt for now. If somebody wants to volunteer
an implementation that reads pg_depend on older versions, that can
probably be backpatched.
Authors: Justin Pryzby, Amit Langote, Álvaro Herrera
Discussion: https://postgr.es/m/
20200419002206.GM26953@telsasoft.com
Michael Paquier [Tue, 21 Apr 2020 22:27:03 +0000 (07:27 +0900)]
Fix memory leak in libpq when using sslmode=verify-full
Checking if Subject Alternative Names (SANs) from a certificate match
with the hostname connected to leaked memory after each lookup done.
This is broken since
acd08d7 that added support for SANs in SSL
certificates, so backpatch down to 9.5.
Author: Roman Peshkurov
Reviewed-by: Hamid Akhtar, Michael Paquier, David Steele
Discussion: https://postgr.es/m/CALLDf-pZ-E3mjxd5=bnHsDu9zHEOnpgPgdnO84E2RuwMCjjyPw@mail.gmail.com
Backpatch-through: 9.5
Alvaro Herrera [Tue, 21 Apr 2020 21:14:18 +0000 (17:14 -0400)]
Document partitiong tables ancillary object handling some more
Add a couple of lines to make it explicit that indexes, constraints,
triggers are added, removed, or left alone.
Backpatch to pg11.
Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/
20200421162038.GA18628@alvherre.pgsql
Tom Lane [Tue, 21 Apr 2020 19:58:42 +0000 (15:58 -0400)]
Fix possible crash during FATAL exit from reindexing.
index.c supposed that it could just use a PG_TRY block to clean up the
state associated with an active REINDEX operation. However, that code
doesn't run if we do a FATAL exit --- for example, due to a SIGTERM
shutdown signal --- while the REINDEX is happening. And that state does
get consulted during catalog accesses, which makes it problematic if we
do any catalog accesses during shutdown --- for example, to clean up any
temp tables created in the session.
If this combination of circumstances occurred, we could find ourselves
trying to access already-freed memory. In debug builds that'd fairly
reliably cause an assertion failure. In production we might often
get away with it, but with some bad luck it could cause a core dump.
Another possible bad outcome is an erroneous conclusion that an
index-to-be-accessed is being reindexed; but it looks like that would
be unlikely to have any consequences worse than failing to drop temp
tables right away. (They'd still get dropped by the next session that
uses that temp schema.)
To fix, get rid of the use of PG_TRY here, and instead hook into
the transaction abort mechanisms to clean up reindex state.
Per bug #16378 from Alexander Lakhin. This has been wrong for a
very long time, so back-patch to all supported branches.
Discussion: https://postgr.es/m/16378-
7a70ca41b3ec2009@postgresql.org
Tom Lane [Tue, 21 Apr 2020 18:23:42 +0000 (14:23 -0400)]
Fix minor violations of FunctionCallInvoke usage protocol.
Working on commit
1c455078b led me to check through FunctionCallInvoke
call sites to see if every one was being honest about (a) making sure
that fcinfo.isnull is initially false, and (b) checking its state after
the call. Sure enough, I found some violations.
The main one is that finalize_partialaggregate re-used serialfn_fcinfo
without resetting isnull, even though it clearly intends to cater for
serialfns that return NULL. There would only be an issue with a
non-strict serialfn, since it's unlikely that a serialfn would return
NULL for non-null input. We have no non-strict serialfns in core, and
there may be none in the wild either, which would account for the lack
of complaints. Still, it's clearly wrong, so back-patch that fix to
9.6 where finalize_partialaggregate was introduced.
Also, arrayfuncs.c and rowtypes.c contained various callers that were
not bothering to check for result nulls. While what's being called is
a comparison or hash function that probably *shouldn't* return null,
that's a lousy excuse for not having any check at all. There are
existing places that just Assert(!fcinfo->isnull) in comparable
situations, so I added that to the places that were calling btree
comparison or hash support functions. In the places calling
boolean-returning equality functions, it's quite cheap to have them
treat isnull as FALSE, so make those places do that. Also remove some
"locfcinfo->isnull = false" assignments that are unnecessary given the
assumption that no previous call returned null. These changes seem like
mostly neatnik-ism or debugging support, so I didn't back-patch.
Alvaro Herrera [Tue, 21 Apr 2020 17:57:00 +0000 (13:57 -0400)]
Fix detaching partitions with cloned row triggers
When a partition is detached, any triggers that had been cloned from its
parent were not properly disentangled from its parent triggers.
This resulted in triggers that could not be dropped because they
depended on the trigger in the trigger in the no-longer-parent table:
ALTER TABLE t DETACH PARTITION t1;
DROP TRIGGER trig ON t1;
ERROR: cannot drop trigger trig on table t1 because trigger trig on table t requires it
HINT: You can drop trigger trig on table t instead.
Moreover the table can no longer be re-attached to its parent, because
the trigger name is already taken:
ALTER TABLE t ATTACH PARTITION t1 FOR VALUES FROM (1)TO(2);
ERROR: trigger "trig" for relation "t1" already exists
The former is a bug introduced in commit
86f575948c77. (The latter is
not necessarily a bug, but it makes the bug more uncomfortable.)
To avoid the complexity that would be needed to tell whether the trigger
has a local definition that has to be merged with the one coming from
the parent table, establish the behavior that the trigger is removed
when the table is detached.
Backpatch to pg11.
Author: Justin Pryzby <pryzby@telsasoft.com>
Reviewed-by: Amit Langote <amitlangote09@gmail.com>
Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://www.postgresql.org/message-id/flat/
20200408152412.GZ2228@telsasoft.com
Peter Geoghegan [Tue, 21 Apr 2020 16:59:24 +0000 (09:59 -0700)]
Consider outliers in split interval calculation.
Commit
0d861bbb, which introduced deduplication to nbtree, added some
logic to take large posting list tuples into account when choosing a
split point. We subtract firstright posting list overhead from the
projected new high key size when calculating leftfree/rightfree values
for an affected candidate split point. Posting list tuples aren't
special to nbtsplitloc.c, but taking them into account like this makes a
huge difference in practice. Posting list tuples are frequently tuple
size outliers.
However, commit
0d861bbb missed a closely related issue: split interval
itself is calculated based on the assumption that tuples on the page
being split are roughly equisized. That assumption was acceptable back
when commit
fab25024 taught the logic for choosing a split point about
suffix truncation, but it's pretty questionable now that very large
tuple sizes are common. This oversight led to unbalanced page splits in
low cardinality multi-column indexes when deduplication was used: page
splits that don't give sufficient weight to how unbalanced the split is
when the interval happens to include some large posting list tuples (and
when most other tuples on the page are not so large).
Nail this down by calculating an initial split interval in a way that's
attuned to the actual cost that we want to keep under control (not a
fuzzy proxy for the cost): apply a leftfree + rightfree evenness test to
each candidate split point that actually gets included in the split
interval (for the default strategy). This replaces logic that used a
percentage of all legal split points for the page as the basis of the
initial split interval.
Discussion: https://postgr.es/m/CAH2-WznJt5aT2uUB2Bs+JBLdwe0XTX67+xeLFcaNvCKxO=QBVQ@mail.gmail.com
Tom Lane [Tue, 21 Apr 2020 16:56:55 +0000 (12:56 -0400)]
Allow matchingsel() to be used with operators that might return NULL.
Although selfuncs.c will never call a target operator with null inputs,
some functions might return null anyway. The existing coding will fail
if that happens (since FunctionCall2Coll will punt), which seems
undesirable given that matchingsel() has such a broad range of potential
applicability --- in fact, we already have a problem because we apply it
to jsonb_path_exists_opr, which can return null. Hence, rejigger the
underlying functions mcv_selectivity and histogram_selectivity to cope,
treating a null result as false.
While we are at it, we can move the InitFunctionCallInfoData overhead
out of the inner loops, which isn't a huge number of cycles but might
save something considering we are likely calling functions as cheap
as int4eq(). Plus, the number of loop cycles to be expected is much
more than it was when this code was written, since typical settings
of default_statistics_target are higher.
In view of that consideration, let's apply the same change to
var_eq_const, eqjoinsel_inner, and eqjoinsel_semi. We do not expect
equality functions to ever return null for non-null inputs (and
certainly that code has been that way a long time without complaints),
but the cycle savings seem attractive, especially in the eqjoinsel loops
where there's potentially an O(N^2) savings.
Similar code exists in ineq_histogram_selectivity and
get_variable_range, but I forebore from changing those for now.
The performance argument for changing ineq_histogram_selectivity
is really weak anyway, since that will only iterate log2(N) times.
Nikita Glukhov and Tom Lane
Discussion: https://postgr.es/m/
9d3b0959-95d6-c37e-2c0b-
287bcfe5c705@postgrespro.ru
Tom Lane [Tue, 21 Apr 2020 15:21:15 +0000 (11:21 -0400)]
Clean up cpluspluscheck violation.
"operator" is a reserved word in C++, so per project conventions,
don't use it as an identifier in header files.
My oversight in commit
a80818605.
Tom Lane [Tue, 21 Apr 2020 15:13:05 +0000 (11:13 -0400)]
Fix duplicate typedef from commit
0d8c9c121.
Older gcc versions don't like duplicate typedefs, so get rid of
that in favor of doing it like we do it elsewhere, ie just use
a "struct" declaration when trying to avoid importing a whole
header file.
Also, there seems no reason to include stringinfo.h here at all,
so get rid of that addition too.
Discussion: https://postgr.es/m/27239.
1587415696@sss.pgh.pa.us
Fujii Masao [Tue, 21 Apr 2020 05:05:43 +0000 (14:05 +0900)]
Mention pg_promote() as a method to trigger promotion in documentation.
Previously in the "Standby Server Operation" section, pg_ctl promote and
protmote_trigger_file were documented as a method to trigger standby
promotion, but pg_promote() function not.
This commit also adds parentheses into <function>pg_promote</function>
in some docs to make it clearer that a function is being referred to.
Author: Masahiro Ikeda
Reviewed-by: Michael Paquier, Laurenz Albe, Tom Lane, Fujii Masao
Discussion: https://postgr.es/m/
de0068417a9f4046bac693cbcc00bdc9@oss.nttdata.com
Bruce Momjian [Tue, 21 Apr 2020 01:41:13 +0000 (21:41 -0400)]
doc: change SGML markup "figure" to "example"
Reported-by: Jürgen Purtz
Discussion: https://postgr.es/m/
709d7809-d7f4-8175-47f3-
4d131341bba8@purtz.de
Author: Jürgen Purtz
Backpatch-through: 9.5
Tom Lane [Mon, 20 Apr 2020 22:44:12 +0000 (18:44 -0400)]
Doc: update sections 9.7 and 9.8 for new function table layout.
Also some mop-up in section 9.9.
Robert Haas [Mon, 20 Apr 2020 18:37:38 +0000 (14:37 -0400)]
Move the server's backup manifest code to a separate file.
basebackup.c is already a pretty big and complicated file, so it
makes more sense to keep the backup manifest support routines
in a separate file, for clarity and ease of maintenance.
Discussion: http://postgr.es/m/CA+TgmoavRak5OdP76P8eJExDYhPEKWjMb0sxW7dF01dWFgE=uA@mail.gmail.com
Alvaro Herrera [Mon, 20 Apr 2020 17:42:41 +0000 (13:42 -0400)]
Add tab-completion for ALTER INDEX .. [NO] DEPENDS ON
... as added in the prior commit.
(We'd like to have tab-completion for the other object types too, but
they don't have sub-command completion yet.)
Author: Ibrar Ahmed <ibrar.ahmad@gmail.com>
Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/CALtqXTcogrFEVP9uou5vFtnGsn+vHZUu9+9a0inarfYVOHScYQ@mail.gmail.com
Alvaro Herrera [Mon, 20 Apr 2020 17:42:12 +0000 (13:42 -0400)]
Add ALTER .. NO DEPENDS ON
Commit
f2fcad27d59c (9.6 era) added the ability to mark objects as
dependent an extension, but forgot to add a way for such dependencies to
be removed. This commit fixes that oversight.
Strictly speaking this should be backpatched to 9.6, but due to lack of
demand we're not doing so at this time.
Discussion: https://postgr.es/m/
20200217225333.GA30974@alvherre.pgsql
Reviewed-by: ahsan hadi <ahsan.hadi@gmail.com>
Reviewed-by: Ibrar Ahmed <ibrar.ahmad@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Tom Lane [Mon, 20 Apr 2020 16:29:28 +0000 (12:29 -0400)]
Doc: update sections 9.5 and 9.6 for new function table layout.
Along the way, update the older examples for bytea to use "hex"
output format. That lets us get rid of the lame disclaimer about
how the examples assume bytea_output = escape, which was only half
true anyway because none of the more-recently-added examples had
paid any attention to that.
Magnus Hagander [Mon, 20 Apr 2020 10:53:40 +0000 (12:53 +0200)]
Allow pg_read_all_stats to access all stats views again
The views pg_stat_progress_* had not gotten the memo that
pg_read_all_stats is supposed to be able to read all statistics. Also
make a pass over all text-returning pg_stat_xyz functions that could
return "insufficient privilege" and make sure they also respect
pg_read_all_status.
Reported-by: Andrey M. Borodin
Reviewed-by: Andrey M. Borodin, Kyotaro Horiguchi
Discussion: https://postgr.es/m/
13145F2F-8458-4977-9D2D-
7B2E862E5722@yandex-team.ru
Tom Lane [Sun, 19 Apr 2020 21:44:28 +0000 (17:44 -0400)]
Doc: update the rest of section 9.4 for new function table layout.
Notably, this replaces the previous handwaving about these functions'
behavior with "character"-type inputs with some actual facts.
Jeff Davis [Sun, 19 Apr 2020 17:32:26 +0000 (10:32 -0700)]
Fix missing pfree() in logtape.c, missed by
24d85952.
Tom Lane [Sun, 19 Apr 2020 16:17:02 +0000 (12:17 -0400)]
Doc: update sections 9.1-9.3 for new function table layout.
I took the opportunity to do some copy-editing in this area as well,
and to add some new material such as a note about BETWEEN's syntactical
peculiarities.
Of note is that quite a few of the examples of transcendental functions
needed to be updated, because the displayed output no longer matched
what you get on a modern server. I believe some of these cases are
side-effects of the new Ryu algorithm in float8out. Others appear to be
because the examples predate the addition of type numeric, and were
expecting that float8 calculations would be done although the given
syntax would actually lead to calling the numeric function nowadays.
Peter Eisentraut [Sun, 19 Apr 2020 12:59:29 +0000 (14:59 +0200)]
Fix update-unicode target
The normalization-check target needs to be run last, after moving the
newly generated files into place. Also, we need an additional
dependency so that unicode_norm.o is rebuilt first. Otherwise,
norm_test will still test the old files but against the new expected
results, which will probably fail.
Tom Lane [Sat, 18 Apr 2020 19:36:43 +0000 (15:36 -0400)]
Doc: sync functableentry markup choices with website style.
Jonathan Katz felt that slightly different indentation settings made
for a better-looking result, so sync stylesheet-fo.xsl (for PDF) and
stylesheet.css (for non-website-style HTML) with those choices.
Discussion: https://postgr.es/m/31464.
1587156281@sss.pgh.pa.us
Tom Lane [Sat, 18 Apr 2020 18:02:44 +0000 (14:02 -0400)]
Fix race conditions in synchronous standby management.
We have repeatedly seen the buildfarm reach the Assert(false) in
SyncRepGetSyncStandbysPriority. This apparently is due to failing to
consider the possibility that the sync_standby_priority values in
shared memory might be inconsistent; but they will be whenever only
some of the walsenders have updated their values after a change in
the synchronous_standby_names setting. That function is vastly too
complex for what it does, anyway, so rewriting it seems better than
trying to apply a band-aid fix.
Furthermore, the API of SyncRepGetSyncStandbys is broken by design:
it returns a list of WalSnd array indexes, but there is nothing
guaranteeing that the contents of the WalSnd array remain stable.
Thus, if some walsender exits and then a new walsender process
takes over that WalSnd array slot, a caller might make use of
WAL position data that it should not, potentially leading to
incorrect decisions about whether to release transactions that
are waiting for synchronous commit.
To fix, replace SyncRepGetSyncStandbys with a new function
SyncRepGetCandidateStandbys that copies all the required data
from shared memory while holding the relevant mutexes. If the
associated walsender process then exits, this data is still safe to
make release decisions with, since we know that that much WAL *was*
sent to a valid standby server. This incidentally means that we no
longer need to treat sync_standby_priority as protected by the
SyncRepLock rather than the per-walsender mutex.
SyncRepGetSyncStandbys is no longer used by the core code, so remove
it entirely in HEAD. However, it seems possible that external code is
relying on that function, so do not remove it from the back branches.
Instead, just remove the known-incorrect Assert. When the bug occurs,
the function will return a too-short list, which callers should treat
as meaning there are not enough sync standbys, which seems like a
reasonably safe fallback until the inconsistent state is resolved.
Moreover it's bug-compatible with what has been happening in non-assert
builds. We cannot do anything about the walsender-replacement race
condition without an API/ABI break.
The bogus assertion exists back to 9.6, but 9.6 is sufficiently
different from the later branches that the patch doesn't apply at all.
I chose to just remove the bogus assertion in 9.6, feeling that the
probability of a bad outcome from the walsender-replacement race
condition is too low to justify rewriting the whole patch for 9.6.
Discussion: https://postgr.es/m/21519.
1585272409@sss.pgh.pa.us
David Rowley [Sat, 18 Apr 2020 02:10:37 +0000 (14:10 +1200)]
Fix possible crash with GENERATED ALWAYS columns
In some corner cases, this could also lead to corrupted values being
included in the tuple.
Users who are concerned that they are affected by this should first
upgrade and then perform a base backup of their database and restore onto
an off-line server. They should then query each table with generated
columns to ensure there are no rows where the generated expression does
not match a newly calculated version of the GENERATED ALWAYS expression.
If no crashes occur and no rows are returned then you're not affected.
Fixes bug #16369.
Reported-by: Cameron Ezell
Discussion: https://postgr.es/m/16369-
5845a6f1bef59884@postgresql.org
Backpatch-through: 12 (where GENERATED ALWAYS columns were added.)
Tom Lane [Sat, 18 Apr 2020 00:50:26 +0000 (20:50 -0400)]
Doc: revise formatting of function/operator tables.
The table layout ideas proposed in commit
e894c6183 were not as widely
popular as I'd hoped. After discussion, we've settled on a layout
that's effectively a single-column table with cell contents much like a
<varlistentry> description of the function or operator; though we're not
actually using <varlistentry>, because it'd add way too much vertical
space. Instead the effect is accomplished using line-break processing
instructions to separate the description and example(s), plus CSS or FO
customizations to produce indentation of all but the first line in each
cell. While technically this is a bit grotty, it does have the
advantage that we won't need to write nearly as much boilerplate markup.
This patch updates tables 9.30, 9.31, and 9.33 (which were touched by
the previous patch) to the revised style, and additionally converts
table 9.10. A lot of work still remains to do, but hopefully it won't
be too controversial.
Thanks to Andrew Dunstan, Pierre Giraud, Robert Haas, Alvaro Herrera,
David Johnston, Jonathan Katz, Isaac Morland for valuable ideas.
Discussion: https://postgr.es/m/8691.
1586798003@sss.pgh.pa.us
Andrew Dunstan [Fri, 17 Apr 2020 20:53:01 +0000 (16:53 -0400)]
Revert "Only provide new libpq sslpasskey hook for openssl-enabled builds"
This reverts commit
9e24109f1a4e4d8d1d372b004d6a0dd06e673fe7.
This caused build errors when building without openssl, and it's
simplest just to revert it.
Andrew Dunstan [Fri, 17 Apr 2020 19:57:19 +0000 (15:57 -0400)]
Only provide openssl_tls_init_hook if building with openssl
This should have been protected by #ifdef USE_OPENSSL in commit
896fcdb230.
Per the real complaint this time from Daniel Gustafsson.
Andrew Dunstan [Fri, 17 Apr 2020 18:44:33 +0000 (14:44 -0400)]
Use a slightly more liberal regex to detect Visual Studio version
Apparently in some language versions of Visual Studio nmake outputs some
material after the version number and before the end of the line. This
has been seen in Chinese versions. Therefore, we no longer demand that
the version string comes at the end of a line.
Per complaint from Cuiping Lin.
Backpatch to all live branches.
Andrew Dunstan [Fri, 17 Apr 2020 18:11:18 +0000 (14:11 -0400)]
Only provide new libpq sslpasskey hook for openssl-enabled builds
In commit
4dc6355210 I neglected to put #ifdef USE_OPENSSL around the
declarations of the new items. This is remedied here.
Per complaint from Daniel Gustafsson.
Tom Lane [Fri, 17 Apr 2020 17:41:59 +0000 (13:41 -0400)]
Fix possible future cache reference leak in ALTER EXTENSION ADD/DROP.
recordExtObjInitPriv and removeExtObjInitPriv were sloppy about
calling ReleaseSysCache. The cases cannot occur given current usage
in ALTER EXTENSION ADD/DROP, since we wouldn't get here for these
relkinds; but it seems wise to clean up better.
In passing, extend test logic in test_pg_dump to exercise the
dropped-column code paths here.
Since the case is unreachable at present, there seems no great
need to back-patch; hence fix HEAD only.
Kyotaro Horiguchi, with test case and comment adjustments by me
Discussion: https://postgr.es/m/
20200417.151831.
1153577605111650154.horikyota.ntt@gmail.com
Fujii Masao [Fri, 17 Apr 2020 09:37:38 +0000 (18:37 +0900)]
Add index term for backup manifest in documentation.
Author: Fujii Masao
Reviewed-by: Robert Haas
Discussion: https://postgr.es/m/
951743d0-fd7e-8e2b-d489-
1368a58b7304@oss.nttdata.com
Michael Paquier [Fri, 17 Apr 2020 01:45:08 +0000 (10:45 +0900)]
Fix minor memory leak in pg_basebackup and pg_receivewal
The result of the query used to retrieve the WAL segment size from the
backend was not getting freed in two code paths. Both pg_basebackup and
pg_receivewal exit immediately if a failure happened on this query, so
this was not an actual problem, but it could be an issue if this code
gets used for other tools in different ways, be they future tools in
this code tree or external, existing, ones.
Oversight in commit
fc49e24, so backpatch down to 11.
Author: Jie Zhang
Discussion: https://postgr.es/m/
970ad9508461469b9450b64027842331@G08CNEXMBPEKD06.g08.fujitsu.local
Backpatch-through: 11
David Rowley [Thu, 16 Apr 2020 22:29:49 +0000 (10:29 +1200)]
Remove unneeded constraint dependency tracking
It was previously thought that remove_useless_groupby_columns() needed to
keep track of which constraints the generated plan depended upon, however,
this is unnecessary. The confusion likely arose regarding this because of
check_functional_grouping(), which does need to track the dependency to
ensure VIEWs with columns which are functionally dependant on the GROUP BY
remain so. For remove_useless_groupby_columns(), cached plans will just
become invalidated when the primary key's underlying index is removed
through the normal relcache invalidation code.
Here we just remove the unneeded code which records the dependency and
updates the comments. The previous comments claimed that we could not use
UNIQUE constraints for the same optimization due to lack of a
pg_constraint record for NOT NULL constraints (which are required because
NULLs can be duplicated in a unique index). Since we don't actually need a
pg_constraint record to handle the invalidation, it looks like we could
add code to do this in the future. But not today.
We're not really fixing any bug in the code here, this fix is just to set
the record straight on UNIQUE constraints. This code was added back in
9.6, but due to lack of any bug, we'll not be backpatching this.
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/CAApHDvrdYa=VhOoMe4ZZjZ-G4ALnD-xuAeUNCRTL+PYMVN8OnQ@mail.gmail.com
Tom Lane [Thu, 16 Apr 2020 18:45:54 +0000 (14:45 -0400)]
Fix cache reference leak in contrib/sepgsql.
fixup_whole_row_references() did the wrong thing with a dropped column,
resulting in a commit-time warning about a cache reference leak.
I (tgl) added a test case exercising this, but back-patched the test
only as far as v10; the patch didn't apply cleanly to 9.6 and it
didn't seem worth the trouble to adapt it. The bug is pretty old
though, so apply the code change all the way back.
Michael Luo, with cosmetic improvements by me
Discussion: https://postgr.es/m/BYAPR08MB5606D1453D7F50E2AF4D2FD29AD80@BYAPR08MB5606.namprd08.prod.outlook.com
Amit Kapila [Thu, 16 Apr 2020 05:14:03 +0000 (10:44 +0530)]
Fix the usage of parallel and full options of vacuum command.
Earlier we were inconsistent in allowing the usage of parallel and
full options. Change it such that we disallow them only when they are
combined in a way that we don't support.
In passing, improve the comments in some of the existing tests of parallel
vacuum.
Reported-by: Tushar Ahuja
Author: Justin Pryzby, Amit Kapila
Reviewed-by: Sawada Masahiko, Michael Paquier, Mahendra Singh Thalor and
Amit Kapila
Discussion: https://postgr.es/m/
58c8d171-e665-6fa3-a9d3-
d9423b694dae%40enterprisedb.com
Michael Paquier [Thu, 16 Apr 2020 04:57:07 +0000 (13:57 +0900)]
Disable silently generation of manifests with servers <= 12 in pg_basebackup
Since
0d8c9c1, pg_basebackup would generate an error if connected to a
backend version older than 12 where backup manifests are not supported.
Avoiding this error is possible by using the --no-manifest option.
This error handling could be confusing for some users, where patching a
backup script that interacts with multiple backend versions would cause
the addition of --no-manifest to potentially not generate a backup
manifest even for Postgres 13 and newer versions. As we want to
encourage the use of backup manifests as much as possible, this commit
silently disables manifests where not supported, instead of generating
an error.
While on it, rework a bit the code to make it more consistent with the
surroundings when generating the BASE_BACKUP command.
Per discussion with Andres Freund, Stephen Frost, Robert Haas, Álvaro
Herrera, Kyotaro Horiguchi, Tom Lane, David Steele, and me.
Author: Michael Paquier
Discussion: https://postgr.es/m/
20200410080910.GZ1606@paquier.xyz
Peter Geoghegan [Wed, 15 Apr 2020 22:47:26 +0000 (15:47 -0700)]
Slightly simplify nbtree split point choice loop.
Spotted during post-commit review of the nbtree deduplication commit
(commit
0d861bbb).
Michael Paquier [Wed, 15 Apr 2020 06:56:01 +0000 (15:56 +0900)]
Fix minor memory leak in pg_dump
A query used to read default ACL information from the catalogs did not
free a set of PQExpBuffer.
Oversight in commit
e2090d9, so backpatch down to 9.6.
Author: Jie Zhang
Reviewed-by: Sawada Masahiko
Discussion: https://postgr.es/m/
05bcbc5857f948efa0b451b85a48ae10@G08CNEXMBPEKD06.g08.fujitsu.local
Backpatch-through: 9.6
Fujii Masao [Wed, 15 Apr 2020 02:15:12 +0000 (11:15 +0900)]
Code review for backup manifest.
This commit prevents pg_basebackup from receiving backup_manifest file
when --no-manifest is specified. Previously, when pg_basebackup was
writing a tarfile to stdout, it tried to receive backup_manifest file even
when --no-manifest was specified, and reported an error.
Also remove unused -m option from pg_basebackup.
Also fix typo in BASE_BACKUP command documentation.
Author: Fujii Masao
Reviewed-by: Michael Paquier, Robert Haas
Discussion: https://postgr.es/m/
01e3ed3a-8729-5aaa-ca84-
e60e3ca59db8@oss.nttdata.com
Peter Geoghegan [Tue, 14 Apr 2020 21:38:28 +0000 (14:38 -0700)]
Remove obsolete "hole in center of page" comment.
A comment from the Berkeley days incorrectly claimed that the page
management code cares about the contents of the hole in the center of
the page (at least in the case of the left half of an nbtree page
split). Commit
8fa30f906be added an addendum that stated that the
original comment was "probably obsolete". It's definitely obsolete,
though, so remove the original comment plus the addendum.
Tom Lane [Tue, 14 Apr 2020 21:30:13 +0000 (17:30 -0400)]
Account for collation when coercing the output of a SQL function.
Commit
913bbd88d overlooked that the result of coerce_to_target_type
might need collation fixups. Per report from Andreas Joseph Krogh.
Discussion: https://postgr.es/m/VisenaEmail.72.
37d08ec2b8cb8fb5.
17179940cd3@tc7-visena
Andrew Dunstan [Tue, 14 Apr 2020 20:55:34 +0000 (16:55 -0400)]
Stop requiring an explicit return from perl subroutines
The consensus of the project appears to be that this provides little
benefit and is simply an annoyance.
Discussion: https://postgr.es/m/27481.
1586618092@sss.pgh.pa.us
Andrew Dunstan [Tue, 14 Apr 2020 20:47:07 +0000 (16:47 -0400)]
Set Perl search path more idiomatically
Back in commits
1df92eeafe,
f884a96819, and
592123efbb I used some
hackish code to set the script search path, unaware despite decades of
perl that there was a completely standard way to do this. This patch
changes those cases to use the standard perl FindBin package.
Robert Haas [Tue, 14 Apr 2020 17:41:32 +0000 (13:41 -0400)]
Document the backup manifest file format.
Patch by me, at the request of Andres Freund. Reviewed by
Justin Pryzby, Erik Rijkers, Álvaro Herrera, and Andrew
Dunstan.
Discussion: http://postgr.es/m/
20200327203225.hcm6ag4grwsiruea@alap3.anarazel.de
Peter Geoghegan [Tue, 14 Apr 2020 16:33:18 +0000 (09:33 -0700)]
Rearrange _bt_insertonpg() "update metapage" code.
Nest the "update metapage as part of insert into root-like page" branch
inside the broader "insert into internal page" branch. This improves
readability.
Michael Paquier [Tue, 14 Apr 2020 05:45:43 +0000 (14:45 +0900)]
Fix collection of typos and grammar mistakes in the tree, volume 2
This fixes some comments and documentation new as of Postgres 13, and is
a follow-up of the work done in
dd0f37e.
Author: Justin Pryzby
Discussion: https://postgr.es/m/
20200408165653.GF2228@telsasoft.com
Peter Geoghegan [Tue, 14 Apr 2020 04:11:03 +0000 (21:11 -0700)]
Add defensive "split_only_page" nbtree assertion.
Clearly it's not okay for nbtree to split a page that is the only page
on its level, and then find that it has to split the parent one level up
in turn. There is simply no code to handle the split_only_page case in
the _bt_insertonpg() "newitem won't fit" branch (only the "newitem fits"
branch handles split_only_page). Add a defensive assertion that will
fail if a split_only_page call to _bt_insertonpg() somehow ends up
splitting the target/parent page.
I (pgeoghegan) believe that we don't need split_only_page handling for
the "newitem won't fit" branch because anybody calling _bt_insertonpg()
like this would have to hold a lock on the same one and only child page.
Amit Kapila [Tue, 14 Apr 2020 02:40:27 +0000 (08:10 +0530)]
Comments and doc fixes for commit
40d964ec99.
Reported-by: Justin Pryzby
Author: Justin Pryzby, with few changes by me
Reviewed-by: Amit Kapila and Sawada Masahiko
Discussion: https://postgr.es/m/
20200322021801.GB2563@telsasoft.com
Peter Geoghegan [Tue, 14 Apr 2020 02:26:41 +0000 (19:26 -0700)]
Make _bt_insertonpg() more like _bt_split().
It seems like a good idea for nbtree's retail insert code to be
absolutely consistent with nbtree's page split code for anything that
naturally requires equivalent handling. Anything that concerns
inserting newitem (which is handled as part of the page split atomic
action when a page split is required) should work in exactly the same
way. With that in mind, make _bt_insertonpg() handle 'cbuf' in a way
that matches _bt_split().
Noah Misch [Tue, 14 Apr 2020 01:47:28 +0000 (18:47 -0700)]
Add a wait_for_catchup() before immediate stop of a test
Per buildfarm member hoverfly, a slow walsender could make the test
fail. Back-patch to v10, where the test was introduced.
Discussion: https://postgr.es/m/
20200414013849.GA886648@rfd.leadboat.com
Alvaro Herrera [Mon, 13 Apr 2020 23:54:09 +0000 (19:54 -0400)]
Silence Perl warning
Now that warnings are enabled across the board, this code that tries to
print an undef variable emits one. Silently printing the empty string
achieves the previous behavior.
Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: Andrew Dunstan <andrew.dunstan@2ndquadrant.com>
Discussion: https://postgr.es/m/E1jO1VT-0008Qk-TM@gemulon.postgresql.org
Peter Geoghegan [Mon, 13 Apr 2020 23:39:55 +0000 (16:39 -0700)]
Harmonize nbtree page split point code.
An nbtree split point can be thought of as a point between two adjoining
tuples from an imaginary version of the page being split that includes
the incoming/new item (in addition to the items that really are on the
page). These adjoining tuples are called the lastleft and firstright
tuples.
The variables that represent split points contained a field called
firstright, which is an offset number of the first data item from the
original page that goes on the new right page. The corresponding tuple
from origpage was usually the same thing as the actual firstright tuple,
but not always: the firstright tuple is sometimes the new/incoming item
instead. This situation seems unnecessarily confusing.
Make things clearer by renaming the origpage offset returned by
_bt_findsplitloc() to "firstrightoff". We now have a firstright tuple
and a firstrightoff offset number which are comparable to the
newitem/lastleft tuples and the newitemoff/lastleftoff offset numbers
respectively. Also make sure that we are consistent about how we
describe nbtree page split point state.
Push the responsibility for dealing with pg_upgrade'd !heapkeyspace
indexes down to lower level code, relieving _bt_split() from dealing
with it directly. This means that we always have a palloc'd left page
high key on the leaf level, no matter what. This enables simplifying
some of the code (and code comments) within _bt_split().
Finally, restructure the page split code to make it clearer why suffix
truncation (which only takes place during leaf page splits) is
completely different to the first data item truncation that takes place
during internal page splits. Tuples are marked as having fewer
attributes stored in both cases, and the firstright tuple is truncated
in both cases, so it's easy to imagine somebody missing the distinction.
Andrew Dunstan [Mon, 13 Apr 2020 16:06:11 +0000 (12:06 -0400)]
Use perl's $/ more idiomatically
This replaces a few occurrences of ugly code with a more clean and
idiomatic usage. The problem was highlighted by perlcritic, but we're
not enforcing the policy that led to the discovery.
Discussion: https://postgr.es/m/
20200412074245.GB623763@rfd.leadboat.com
Andrew Dunstan [Mon, 13 Apr 2020 15:55:45 +0000 (11:55 -0400)]
Use perl warnings pragma consistently
We've had a mixture of the warnings pragma, the -w switch on the shebang
line, and no warnings at all. This patch removes the -w swicth and add
the warnings pragma to all perl sources missing it. It raises the
severity of the TestingAndDebugging::RequireUseWarnings perlcritic
policy to level 5, so that we catch any future violations.
Discussion: https://postgr.es/m/
20200412074245.GB623763@rfd.leadboat.com
Andrew Dunstan [Mon, 13 Apr 2020 15:46:18 +0000 (11:46 -0400)]
Print policy name in perlcritic messages
This makes it easier to do a web search for details of the policy that's
been violated, as well as displaying the name that might be needed for a
policy override.
Various perlcritic settings changes are being discussed, but this one
should be uncontroversial.
Robert Haas [Mon, 13 Apr 2020 14:48:23 +0000 (10:48 -0400)]
Rename pg_validatebackup to pg_verifybackup some more.
The previous commit missed an instance.
Noriyoshi Shinoda
Discussion: http://postgr.es/m/TU4PR8401MB115291AE850BA7CF1AEB2F0BEEDD0@TU4PR8401MB1152.NAMPRD84.PROD.OUTLOOK.COM
Amit Kapila [Mon, 13 Apr 2020 10:01:16 +0000 (15:31 +0530)]
Cosmetic fixups for WAL usage work.
Reported-by: Justin Pryzby and Euler Taveira
Author: Justin Pryzby and Julien Rouhaud
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/CAB-hujrP8ZfUkvL5OYETipQwA=e3n7oqHFU=4ZLxWS_Cza3kQQ@mail.gmail.com
Peter Eisentraut [Mon, 13 Apr 2020 08:21:15 +0000 (10:21 +0200)]
Improve error messages after LoadLibrary()
Move the file name to a format parameter to ease translatability. Add
error code where missing. Make the wording consistent.
Tom Lane [Sun, 12 Apr 2020 22:03:20 +0000 (18:03 -0400)]
Doc: introduce new layout for tables of functions and operators.
We've long fought with the draconian space limitations of our
traditional table layout for describing SQL functions and operators.
This commit introduces a new approach, though so far I've only applied
it to a few of those tables. The new way makes use of DocBook's support
for different layouts in different rows of a table, and allows the
descriptions and examples for a function or operator to run to several
lines without as much ugliness and wasted space as before.
The core layout concept is now
Name Signature
Description
Example Example Result
so that a function or operator really has three table rows not one,
but we group them to look like one row by having the name column
have only one entry for all three rows. (Actually, there could be
four or more rows if you wanted to have more than one example, which
is another thing that was painful before but works easily now.)
This is handled by a "morerows" annotation on the name entry, which
isn't perfect (notably, the toolchain is not smart enough to avoid
breaking these row groups across PDF pages) but there seems no better
solution in DocBook. The name column is normally fairly narrow,
allowing plenty of space for the other column(s), and not wasting too
much space when one of the other components runs to multiple lines.
The varying row layout is managed by defining named "spans" and then
tagging entries with a "spanname" of "name", "sig", "desc", "example",
or "exresult". This provides a bit of semantic annotation to go with
the formatting improvement, which seems like a good thing. (It seems
that we have to re-define these spans afresh for each table, which is
annoying, but it's not any worse than the duplication involved in
the table headers. At least that gives us an opportunity to vary the
relative column widths per-table, which is handy since function tables
sometimes need much wider name columns than operator tables.)
Signature entries should be written in the style
<function>fname</function>(<type>typename</type> ...)
<returnvalue>typename</returnvalue>
The <returnvalue> tag produces a right arrow before the result type
name. (I'll document that convention in a user-visible place later.)
While this provides significantly more horizontal space than before
for examples, it's still true that PDF output is a lot narrower than
typical webpage viewing windows, so some examples need to be broken
in places where there is no whitespace. I've added &zwsp; markers in
suitable places to allow the tables to render warning-free in PDF.
I've so far converted only the date/time operator, date/time function,
and enum function tables in sections 9.9 and 9.10; these were chosen
to provide a reasonable sample of the formatting problems that need
to be solved. Assuming that this looks good on the website and doesn't
provoke howls of anguish, I'll work on the other similar tables in the
near future.
There's a moderate amount of new editorial content in this patch along
with the raw formatting changes; for instance I had to write text
descriptions for operators that lacked them. I failed to resist the
temptation to improve some other descriptions and examples, too.
Patch by me, with thanks to Alexander Lakhin for assistance with
figuring out some formatting issues.
Discussion: https://postgr.es/m/9326.
1581457869@sss.pgh.pa.us
Tom Lane [Sun, 12 Apr 2020 18:03:24 +0000 (14:03 -0400)]
Doc: introduce and document "&zwsp;" for allowing optional line breaks.
We already had a couple of places using zero-width spaces for formatting
hackery, and we're going to need more if we ever want the PDF manuals to
look decent. But please let's not write hard-coded Unicode escapes.
We can avoid that by using a custom entity, which also provides a place
to put a teeny bit of documentation about what it is and how to use it.
I'd previously posted a patch using "&break;" for this, but on reflection
that would be horrible to grep for. Instead let's use "&zwsp;", based
on the name of the Unicode symbol ("zero width space").
Discussion: https://postgr.es/m/9326.
1581457869@sss.pgh.pa.us
Robert Haas [Sun, 12 Apr 2020 15:26:05 +0000 (11:26 -0400)]
Rename pg_validatebackup to pg_verifybackup.
Also, use "verify" rather than "validate" to refer to the process
being undertaken here. Per discussion, that is a more appropriate
term.
Discussion: https://www.postgresql.org/message-id/
172c9d9b-1d0a-1b94-1456-
376b1e017322@2ndquadrant.com
Discussion: http://postgr.es/m/CA+TgmobLgMh6p8FmLbj_rv9Uhd7tPrLnAyLgGd2SoSj=qD-bVg@mail.gmail.com
Peter Geoghegan [Sun, 12 Apr 2020 04:07:20 +0000 (21:07 -0700)]
Doc: Fix contrib/amcheck tip.
Fixes an oversight in commit
20fbb711.
Tom Lane [Sat, 11 Apr 2020 19:02:38 +0000 (15:02 -0400)]
Suppress -Wimplicit-fallthrough warning in new LIMIT WITH TIES code.
The placement of the fall-through comment in this code appears not to
work to suppress the warning in recent gcc. Move it to the bottom of
the case group, and add an assertion that we didn't get there through
some other code path. Also improve wording of nearby comments.
Julien Rouhaud, comment hacking by me
Discussion: https://postgr.es/m/CAOBaU_aLdPGU5wCpaowNLF-Q8328iR7mj1yJAhMOVsdLwY+sdg@mail.gmail.com
Noah Misch [Sat, 11 Apr 2020 17:30:12 +0000 (10:30 -0700)]
Optimize RelationFindReplTupleSeq() for CLOBBER_CACHE_ALWAYS.
Specifically, remember lookup_type_cache() results instead of retrieving
them once per comparison. Under CLOBBER_CACHE_ALWAYS, this reduced
src/test/subscription/t/001_rep_changes.pl elapsed time by an order of
magnitude, which reduced check-world elapsed time by 9%.
Discussion: https://postgr.es/m/
20200406085420.GC162712@rfd.leadboat.com
Noah Misch [Sat, 11 Apr 2020 17:30:00 +0000 (10:30 -0700)]
When WalSndCaughtUp, sleep only in WalSndWaitForWal().
Before sleeping, WalSndWaitForWal() sends a keepalive if MyWalSnd->write
< sentPtr. That is important in logical replication. When the latest
physical LSN yields no logical replication messages (a common case),
that keepalive elicits a reply, and processing the reply updates
pg_stat_replication.replay_lsn. WalSndLoop() lacks that; when
WalSndLoop() slept, replay_lsn advancement could stall until
wal_receiver_status_interval elapsed. This sometimes stalled
src/test/subscription/t/001_rep_changes.pl for up to 10s.
Discussion: https://postgr.es/m/
20200406063649.GA3738151@rfd.leadboat.com
Tom Lane [Sat, 11 Apr 2020 16:39:19 +0000 (12:39 -0400)]
Make EXPLAIN report maximum hashtable usage across multiple rescans.
Before discarding the old hash table in ExecReScanHashJoin, capture
its statistics, ensuring that we report the maximum hashtable size
across repeated rescans of the hash input relation. We can repurpose
the existing code for reporting hashtable size in parallel workers
to help with this, making the patch pretty small. This also ensures
that if rescans happen within parallel workers, we get the correct
maximums across all instances.
Konstantin Knizhnik and Tom Lane, per diagnosis by Thomas Munro
of a trouble report from Alvaro Herrera.
Discussion: https://postgr.es/m/
20200323165059.GA24950@alvherre.pgsql
Tom Lane [Sat, 11 Apr 2020 16:29:06 +0000 (12:29 -0400)]
Clear dangling pointer to avoid bogus EXPLAIN printout in a corner case.
ExecReScanHashJoin will destroy the join's hash table if it expects
that the inner relation will produce different rows on rescan.
Up to now it's not bothered to clear the additional pointer to that
hash table that exists in the child HashState node. However, it's
possible for the query to terminate without building a fresh hash
table (this happens if the outer relation is found to be empty
during the final rescan). So we can end with a dangling pointer
to a deleted hash table. That was harmless originally, but since
9.0 EXPLAIN ANALYZE has used that pointer to print hash table
statistics. In debug builds this reproducibly results in garbage
statistics. In non-debug builds there's frequently no ill effects,
but in principle one could get wrong EXPLAIN ANALYZE output, or
perhaps even a crash if free() has released the hashtable memory
back to the OS.
To fix, just make sure we clear the additional pointer when destroying
the hash table. In problematic cases, EXPLAIN ANALYZE will then print
no hashtable statistics (reverting to its pre-9.0 behavior). This isn't
ideal, but since the problem manifests only in unusual corner cases,
it's hard to justify taking any risks to do better in the back
branches. A follow-on patch will improve matters in HEAD.
Konstantin Knizhnik and Tom Lane, per diagnosis by Thomas Munro
of a trouble report from Alvaro Herrera.
Discussion: https://postgr.es/m/
20200323165059.GA24950@alvherre.pgsql
Peter Eisentraut [Sat, 11 Apr 2020 13:07:25 +0000 (15:07 +0200)]
Fix RELCACHE_FORCE_RELEASE issue
Introduced by
83fd4532a72179c370e318075a10e0e2aa832024. To fix, the
tuple descriptors need to be copied into the current memory context.
Discussion: https://www.postgresql.org/message-id/
04d78603-edae-9243-9dde-
fe3037176a7d@2ndquadrant.com
Peter Eisentraut [Sat, 11 Apr 2020 07:44:14 +0000 (09:44 +0200)]
Fix relcache reference leak
Introduced by
83fd4532a72179c370e318075a10e0e2aa832024
Andrew Gierth [Sat, 11 Apr 2020 07:04:57 +0000 (08:04 +0100)]
doc: restore intentional typo
Commit
ac8623760 "fixed" a typo in an example of what would happen in
the event of a typo. Restore the original typo and add a comment about
its intentionality. Backpatch to 12 where the error was introduced.
Per report from irc user Nicolás Alvarez.
Peter Geoghegan [Sat, 11 Apr 2020 00:44:08 +0000 (17:44 -0700)]
Add contrib/amcheck debug message.
Add a DEBUG1 message indicating that verification of the index structure
is underway. Also reduce the severity level of the existing "tree
level" debug message to DEBUG1. It should never have been made DEBUG2.
Any B-Tree index with more than a couple of levels will generally also
have so many pages that the per-page DEBUG2 messages will become
completely unmanageable.
In passing, add a new "Tip" to the docs that advises users that run into
corruption that the debug messages might provide useful additional
context.
Tom Lane [Fri, 10 Apr 2020 17:12:58 +0000 (13:12 -0400)]
Doc: clarify locking requirements for ALTER TABLE ADD FOREIGN KEY.
The docs explained that a SHARE ROW EXCLUSIVE lock is needed on the
referenced table, but failed to say the same about the table being
altered. Since the page says that ACCESS EXCLUSIVE lock is taken
unless otherwise stated, this left readers with the wrong conclusion.
Discussion: https://postgr.es/m/
834603375.
3470346.
1586482852542@mail.yahoo.com
Tom Lane [Fri, 10 Apr 2020 16:00:28 +0000 (12:00 -0400)]
Suppress unused-variable warning.
Ashutosh Bapat
Discussion: https://postgr.es/m/CAG-ACPWPB8Lc_aFj25eiPFqi31YB5vmaZnb39mbHSf5Yej=miA@mail.gmail.com
Tom Lane [Fri, 10 Apr 2020 14:44:09 +0000 (10:44 -0400)]
Doc: sync CREATE GROUP syntax synopsis with CREATE ROLE.
CREATE GROUP is an exact alias for CREATE ROLE, and CREATE USER is
almost an exact alias, as can easily be confirmed by checking the
code. So the man page syntax descriptions ought to match up. The
last few additions of role options seem to have forgotten to update
create_group.sgml, though. Fix that, and add a naggy reminder to
create_role.sgml in hopes of not forgetting again.
Discussion: https://postgr.es/m/
158647836143.655.
9853963229391401576@wrigleys.postgresql.org
Michael Paquier [Fri, 10 Apr 2020 02:18:39 +0000 (11:18 +0900)]
Fix collection of typos and grammar mistakes in the tree
This fixes some comments and documentation new as of Postgres 13.
Author: Justin Pryzby
Discussion: https://postgr.es/m/
20200408165653.GF2228@telsasoft.com
Tom Lane [Thu, 9 Apr 2020 21:28:58 +0000 (17:28 -0400)]
Further stabilize results of 019_replslot_limit.pl.
Depending on specific values of restart_lsn or pg_current_wal_lsn()
is obviously unsafe. The previous coding tried to dodge this issue
by rounding off, but that's not good enough, as shown by multiple
buildfarm members. Nuke all the uses of these values except for
null-ness checks, pending some credible argument why we should think
something else could be usefully stable.
Kyotaro Horiguchi, further modified by me
Discussion: https://postgr.es/m/E1jM1Sa-0003mS-99@gemulon.postgresql.org
Tom Lane [Thu, 9 Apr 2020 19:38:43 +0000 (15:38 -0400)]
Further cleanup of ts_headline code.
Suppress a probably-meaningless uninitialized-variable warning
(induced by my previous patch, I'm sorry to say).
Improve mark_hl_fragments()'s test for overlapping cover strings:
it failed to consider the possibility that the current string is
strictly within another one. That's unlikely given the preceding
splitting into MaxWords fragments, but I don't think it's impossible.
Discussion: https://postgr.es/m/16345-
2e0cf5cddbdcd3b4@postgresql.org
Tom Lane [Thu, 9 Apr 2020 19:11:08 +0000 (15:11 -0400)]
Doc: improve documentation about ts_headline() function.
Now that I've had my nose in that code, I thought the docs about
it left something to be desired.
Tom Lane [Thu, 9 Apr 2020 17:19:23 +0000 (13:19 -0400)]
Fix default text search parser's ts_headline code for phrase queries.
This code could produce very poor results when asked to highlight a
string based on a query using phrase-match operators. The root cause
is that hlCover(), which is supposed to find a minimal substring that
matches the query, was written assuming that word position is not
significant. I'm only 95% convinced that its algorithm was correct even
for plain AND/OR queries; but it definitely fails completely for phrase
matches, causing it to possibly not identify a cover string at all.
Hence, rewrite hlCover() with a less-tense algorithm that just tries
all the possible substrings, earlier and shorter ones first. (This is
not as bad as it sounds performance-wise, because all of the string
matching has been done already: the repeated tsquery match checks
boil down to pointer comparisons.)
Unfortunately, since that approach produces more candidate cover
strings than before, it also exposes that there were bugs in the
heuristics in mark_hl_words() for selecting a best cover string.
Fixes there include:
* Do not apply the ShortWord filter to words that appear in the query.
* Remove a misguided optimization for quickly rejecting a cover.
* Fix order-of-operation bug that could cause computation of a
wrong figure of merit (poslen) when shortening a cover.
* Change the preference rule so that candidate headlines that do not
include their whole cover string (after MaxWords trimming) are lowest
priority, since they may not actually satisfy the user's query.
This results in some changes in existing regression test cases,
but they all seem reasonable. Note in particular that the tests
involving strings like "1 2 3" were previously being affected by
the ShortWord filter, masking the normal matching behavior.
Per bug #16345 from Augustinas Jokubauskas; the new test cases are
based on that example. Back-patch to 9.6 where phrase search was
added to tsquery.
Discussion: https://postgr.es/m/16345-
2e0cf5cddbdcd3b4@postgresql.org
Tom Lane [Thu, 9 Apr 2020 16:36:59 +0000 (12:36 -0400)]
Cosmetic improvements for default text search parser's ts_headline code.
This code was woefully unreadable and under-commented. Try to improve
matters by adding comments, using some macros to make complicated
if-tests more readable, using boolean type where appropriate, etc.
There are a couple of tiny coding improvements too, but this commit
includes (I hope) no behavioral change.
Nonetheless, back-patch as far as 9.6, because a followup bug-fixing
commit depends on this.
Discussion: https://postgr.es/m/16345-
2e0cf5cddbdcd3b4@postgresql.org
Peter Eisentraut [Thu, 9 Apr 2020 14:17:55 +0000 (16:17 +0200)]
Fix CREATE TABLE LIKE INCLUDING GENERATED column order issue
CREATE TABLE LIKE INCLUDING GENERATED would fail if a generated column
referred to a column with a higher attribute number. This is because
the column mapping mechanism created the mapping incrementally as
columns are added. This was sufficient for previous uses of that
mechanism (omitting dropped columns), and it also happened to work if
generated columns only referred to columns with lower attribute
numbers, but here it failed.
This fix is to build the attribute mapping in a separate loop before
processing the columns in detail.
Bug: #16342
Reported-by: Ethan Waldo <ewaldo@healthetechs.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Fujii Masao [Thu, 9 Apr 2020 13:38:24 +0000 (22:38 +0900)]
Fix typo in pg_validatebackup documentation.
Author: Fujii Masao
Reviewed-by: Robert Haas
Discussion: https://postgr.es/m/
78f76a3d-1a28-a97d-0394-
5c96985dd1c0@oss.nttdata.com
Fujii Masao [Thu, 9 Apr 2020 13:37:11 +0000 (22:37 +0900)]
Exclude backup_manifest file that existed in database, from BASE_BACKUP.
If there is already a backup_manifest file in the database cluster,
it belongs to the past backup that was used to start this server.
It is not correct for the backup being taken now. So this commit
changes pg_basebackup so that it always skips such backup_manifest
file. The backup_manifest file for the current backup will be injected
separately if users want it.
Author: Fujii Masao
Reviewed-by: Robert Haas
Discussion: https://postgr.es/m/
78f76a3d-1a28-a97d-0394-
5c96985dd1c0@oss.nttdata.com
Amit Kapila [Thu, 9 Apr 2020 04:19:30 +0000 (09:49 +0530)]
Allow parallel create index to accumulate buffer usage stats.
Currently, we don't account for buffer usage incurred by parallel workers
for parallel create index. This commit allows each worker to record the
buffer usage stats and leader backend to accumulate that stats at the
end of the operation. This will allow pg_stat_statements to display
correct buffer usage stats for (parallel) create index command.
Reported-by: Julien Rouhaud
Author: Sawada Masahiko
Reviewed-by: Dilip Kumar, Julien Rouhaud and Amit Kapila
Backpatch-through: 11, where this was introduced
Discussion: https://postgr.es/m/
20200328151721.GB12854@nol
Fujii Masao [Thu, 9 Apr 2020 03:56:36 +0000 (12:56 +0900)]
Add note in pg_stat_statements documentation about planning statistics.
The added note explains that the numbers of planning and execution in
the statement are not always expected to match because their statistics are
updated at their respective end phase, and only for successful operations.
Author: Pascal Legrand, Julien Rouhaud, tweaked a bit by Fujii Masao
Discussion: https://postgr.es/m/
1585857868967-0.post@n3.nabble.com
Andrew Dunstan [Wed, 8 Apr 2020 21:50:55 +0000 (17:50 -0400)]
Msys2 tweaks for pg_validatebackup corruption test
1. Tell Msys2 not to mangle the tablespace map parameter
2. If rmdir doesn't work, fall back to trying unlink on the entry in
pg_tblspc.
Discussion: https://postgr.es/m/
7330a7c7-ce5f-9769-39a1-
bdb0b32bb4a6@2ndQuadrant.com
Peter Eisentraut [Wed, 8 Apr 2020 17:19:45 +0000 (19:19 +0200)]
createuser: Change a fprintf to pg_log_error
Tomas Vondra [Wed, 8 Apr 2020 16:30:11 +0000 (18:30 +0200)]
Stabilize incremental_sort tests
The test never did ANALYZE on the test table, so the plans depended on
various defaults (e.g. number of groups being 200). This worked most of
the time, but with CLOBBER_CACHE_ALWAYS the autoanalyze often managed
to build accurate stats, changing the plan.
Fixed by increasing the size of test tables a bit, making the Sort a bit
more expensive than Incremental Sort. The tests were constructed to test
transitions in the Incremental Sort algorithm, and this change does not
break that.
Reviewed-by: James Coleman
Discussion: https://postgr.es/m/CAPpHfds1waRZ=NOmueYq0sx1ZSCnt+5QJvizT8ndT2=etZEeAQ@mail.gmail.com
Tom Lane [Wed, 8 Apr 2020 15:23:39 +0000 (11:23 -0400)]
Fix pg_dump/pg_restore to restore event trigger comments later.
Repair an oversight in commit
8728b2c70: if we're postponing restore
of event triggers to the end, we must also postpone restoring any
comments on them, since of course we cannot create the comments first.
(This opens yet another opportunity for an event trigger to bollix
the restore, but there's no help for that.)
Per bug #16346 from Alexander Lakhin.
Like the previous commit, back-patch to all supported branches.
Hamid Akhtar and Tom Lane
Discussion: https://postgr.es/m/16346-
6210ad7a0ea81be1@postgresql.org
Thomas Munro [Wed, 8 Apr 2020 11:45:09 +0000 (23:45 +1200)]
Rationalize GetWalRcv{Write,Flush}RecPtr().
GetWalRcvWriteRecPtr() previously reported the latest *flushed*
location. Adopt the conventional terminology used elsewhere in the tree
by renaming it to GetWalRcvFlushRecPtr(), and likewise for some related
variables that used the term "received".
Add a new definition of GetWalRcvWriteRecPtr(), which returns the latest
*written* value. This will allow later patches to use the value for
non-data-integrity purposes, without having to wait for the flush
pointer to advance.
Reviewed-by: Alvaro Herrera <alvherre@2ndquadrant.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CA%2BhUKGJ4VJN8ttxScUFM8dOKX0BrBiboo5uz1cq%3DAovOddfHpA%40mail.gmail.com
Peter Eisentraut [Wed, 8 Apr 2020 07:59:27 +0000 (09:59 +0200)]
Allow publishing partition changes via ancestors
To control whether partition changes are replicated using their own
identity and schema or an ancestor's, add a new parameter that can be
set per publication named 'publish_via_partition_root'.
This allows replicating a partitioned table into a different partition
structure on the subscriber.
Author: Amit Langote <amitlangote09@gmail.com>
Reviewed-by: Rafia Sabih <rafia.pghackers@gmail.com>
Reviewed-by: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Reviewed-by: Petr Jelinek <petr@2ndquadrant.com>
Discussion: https://www.postgresql.org/message-id/flat/CA+HiwqH=Y85vRK3mOdjEkqFK+E=ST=eQiHdpj43L=_eJMOOznQ@mail.gmail.com
Alexander Korotkov [Wed, 8 Apr 2020 08:37:27 +0000 (11:37 +0300)]
Revert
0f5ca02f53
0f5ca02f53 introduces 3 new keywords. It appears to be too much for relatively
small feature. Given now we past feature freeze, it's already late for
discussion of the new syntax. So, revert.
Discussion: https://postgr.es/m/28209.
1586294824%40sss.pgh.pa.us
David Rowley [Wed, 8 Apr 2020 06:29:51 +0000 (18:29 +1200)]
Modify additional power 2 calculations to use new helper functions
2nd pass of modifying various places which obtain the next power
of 2 of a number and make them use the new functions added in
f0705bb62.
In passing, also modify num_combinations(). This can be implemented
using simple bitshifting rather than looping.
Reviewed-by: John Naylor
Discussion: https://postgr.es/m/
20200114173553.GE32763%40fetter.org
Michael Paquier [Wed, 8 Apr 2020 06:04:51 +0000 (15:04 +0900)]
Fix crash when using COLLATE in partition bound expressions
Attempting to use a COLLATE clause with a type that it not collatable in
a partition bound expression could crash the server. This commit fixes
the code by adding more checks similar to what is done when computing
index or partition attributes by making sure that there is a collation
iff the type is collatable.
Backpatch down to 12, as
7c079d7 introduced this problem.
Reported-by: Alexander Lakhin
Author: Dmitry Dolgov
Discussion: https://postgr.es/m/16325-
809194cf742313ab@postgresql.org
Backpatch-through: 12