Heikki Linnakangas [Mon, 19 Oct 2020 15:50:33 +0000 (18:50 +0300)]
Fix output of tsquery example in docs.
The output for this query changed in commit
4e2477b7b8. Backport to 9.6
like that commit.
Patch by Justin Pryzby, per Yaroslav Schekin's report.
Discussion: https://www.postgresql.org/message-id/
20201005191922.GE17626%40telsasoft.com
Tom Lane [Mon, 19 Oct 2020 15:23:51 +0000 (11:23 -0400)]
In libpq for Windows, call WSAStartup once and WSACleanup not at all.
The Windows documentation insists that every WSAStartup call should
have a matching WSACleanup call. However, if that ever had actual
relevance, it wasn't in this century. Every remotely-modern Windows
kernel is capable of cleaning up when a process exits without doing
that, and must be so to avoid resource leaks in case of a process
crash. Moreover, Postgres backends have done WSAStartup without
WSACleanup since commit
4cdf51e64 in 2004, and we've never seen any
indication of a problem with that.
libpq's habit of doing WSAStartup during connection start and
WSACleanup during shutdown is also rather inefficient, since a
series of non-overlapping connection requests leads to repeated,
quite expensive DLL unload/reload cycles. We document a workaround
for that (having the application call WSAStartup for itself), but
that's just a kluge. It's also worth noting that it's far from
uncommon for applications to exit without doing PQfinish, and
we've not heard reports of trouble from that either.
However, the real reason for acting on this is that recent
experiments by Alexander Lakhin show that calling WSACleanup
during PQfinish is triggering the symptom we occasionally see
that a process using libpq fails to emit expected stdio output.
Therefore, let's change libpq so that it calls WSAStartup only
once per process, during the first connection attempt, and never
calls WSACleanup at all.
While at it, get rid of the only other WSACleanup call in our code
tree, in pg_dump/parallel.c; that presumably is equally useless.
Back-patch of HEAD commit
7d00a6b2d.
Discussion: https://postgr.es/m/
ac976d8c-03df-d6b8-025c-
15a2de8d9af1@postgrespro.ru
Heikki Linnakangas [Mon, 19 Oct 2020 14:58:38 +0000 (17:58 +0300)]
Fix doc for full text search distance operator.
Commit
028350f619 changed its behavior from "at most" to "exactly", but
forgot to update the documentation. Backpatch to 9.6.
Patch by Justin Pryzby, per Yaroslav Schekin's report.
Discussion: https://www.postgresql.org/message-id/
20201005191922.GE17626%40telsasoft.com
Magnus Hagander [Mon, 19 Oct 2020 11:47:09 +0000 (13:47 +0200)]
Update link for pllua
Author: Daniel Gustafsson
Discussion: https://postgr.es/m/A05874AE-8771-4C61-A24E-0B6249B8F3C2@yesql.se
David Rowley [Mon, 19 Oct 2020 11:05:41 +0000 (00:05 +1300)]
Relax some asserts in merge join costing code
In the planner, it was possible, given an extreme enough case containing a
large number of joins for the number of estimated rows to become infinite.
This could cause problems in initial_cost_mergejoin() where we perform
some calculations based on those row estimates.
A problem case, presented by Onder Kalaci showed an Assert failure from
an Assert checking outerstartsel <= outerendsel. In his test case this
was effectively NaN <= Inf, which is false. The NaN outerstartsel came
from multiplying the infinite outer_path_rows by 0.0.
In master, this problem was fixed by
a90c950fc, however, that fix was too
invasive for the backbranches. Here we just relax the Asserts to allow
them to pass. The worst that appears to happen from this is that we show
NaN cost values and infinite row estimates in EXPLAIN. add_path() would
have had a hard time doing anything useful with such costs, but that does
not really matter as if the row estimates were even close to accurate,
such plan would not complete this side of the heat death of the universe.
Reported-by: Onder Kalaci
Backpatch: 9.5 to 13
Discussion: https://postgr.es/m/DM6PR21MB1211FF360183BCA901B27F04D80B0@DM6PR21MB1211.namprd21.prod.outlook.com
Michael Paquier [Mon, 19 Oct 2020 00:37:55 +0000 (09:37 +0900)]
Fix potential memory leak in pgcrypto
When allocating a EVP context, it would have been possible to leak some
memory allocated directly by OpenSSL, that PostgreSQL lost track of if
the initialization of the context allocated failed. The cleanup can be
done with EVP_MD_CTX_destroy().
Note that EVP APIs exist since OpenSSL 0.9.7 and we have in the tree
equivalent implementations for older versions since
ce9b75d (code
removed with
9b7cd59a as of 10~). However, in 9.5 and 9.6, the existing
code makes use of EVP_MD_CTX_destroy() and EVP_MD_CTX_create() without
an equivalent implementation when building the tree with OpenSSL 0.9.6
or older, meaning that this code is in reality broken with such versions
since it got introduced in
e2838c5. As we have heard no complains about
that, it does not seem worth bothering with in 9.5 and 9.6, so I have
left that out for simplicity.
Author: Michael Paquier
Discussion: https://postgr.es/m/
20201015072212[email protected]
Backpatch-through: 9.5
Tom Lane [Sat, 17 Oct 2020 20:02:47 +0000 (16:02 -0400)]
Doc: caution against misuse of 'now' and related datetime literals.
Section 8.5.1.4, which defines these literals, made only a vague
reference to the fact that they might be evaluated too soon to be
safe in non-interactive contexts. Provide a more explicit caution
against misuse. Also, generalize the wording in the related tip in
section 9.9.4: while it clearly described this problem, it implied
(or really, stated outright) that the problem only applies to table
DEFAULT clauses.
Per gripe from Tijs van Dam. Back-patch to all supported branches.
Discussion: https://postgr.es/m/c2LuRv9BiRT3bqIo5mMQiVraEXey_25B4vUn0kDqVqilwOEu_iVF1tbtvLnyQK7yDG3PFaz_GxLLPil2SDkj1MCObNRVaac-7j1dVdFERk8=@thalex.com
Tom Lane [Sat, 17 Oct 2020 01:53:33 +0000 (21:53 -0400)]
Update time zone data files to tzdata release 2020c.
DST law changes in Morocco, Canadian Yukon, Fiji, Macquarie Island,
Casey Station (Antarctica). Historical corrections for France,
Hungary, Monaco.
Tom Lane [Sat, 17 Oct 2020 01:40:16 +0000 (21:40 -0400)]
Sync our copy of the timezone library with IANA release tzcode2020c.
This changes zic's default output format from "-b fat" to "-b slim".
We were already using "slim" in v13/HEAD, so those branches drop
the explicit -b switch in the Makefiles. Instead, add an explicit
"-b fat" in v12 and before, so that we don't change the output file
format in those branches. (This is perhaps excessively conservative,
but we decided not to do so in
a12079109, and I'll stick with that.)
Other non-cosmetic changes are to drop support for zic's long-obsolete
"-y" switch, and to ensure that strftime() does not change errno
unless it fails.
As usual with tzcode changes, back-patch to all supported branches.
Tom Lane [Fri, 16 Oct 2020 15:59:13 +0000 (11:59 -0400)]
Add missing error check in pgcrypto/crypt-md5.c.
In theory, the second px_find_digest call in px_crypt_md5 could fail
even though the first one succeeded, since resource allocation is
required. Don't skip testing for a failure. (If one did happen,
the likely result would be a crash rather than clean recovery from
an OOM failure.)
The code's been like this all along, so back-patch to all supported
branches.
Daniel Gustafsson
Discussion: https://postgr.es/m/
AA8D6FE9-4AB2-41B4-98CB-
AE64BA668C03@yesql.se
Tom Lane [Fri, 16 Oct 2020 15:36:34 +0000 (11:36 -0400)]
Doc: tweak column widths in synchronous-commit-matrix table.
Commit
a97e85f2b caused "exceed the available area" warnings in PDF
builds. Fine-tune colwidth values to avoid that.
Back-patch to 9.6, like the prior patch. (This is of dubious value
before v13, since we were far from free of such warnings in older
branches. But we might as well keep the SGML looking the same in all
branches.)
Per buildfarm.
Andres Freund [Fri, 16 Oct 2020 00:38:00 +0000 (17:38 -0700)]
llvmjit: Work around bug in LLVM 3.9 causing crashes after
72559438f92.
Unfortunately in LLVM 3.9 LLVMGetAttributeCountAtIndex(func, index)
crashes when called with an index that has 0 attributes. Since there's
no way to work around this in the C API, add a small C++ wrapper doing
so.
The only reason this didn't fail before
72559438f92 is that there
always are function attributes...
Author: Andres Freund
Discussion: https://postgr.es/m/20201016001254[email protected]
Backpatch: 11-, like 72559438f92
Bruce Momjian [Fri, 16 Oct 2020 00:37:20 +0000 (20:37 -0400)]
pg_upgrade: remove C99 compiler req. from commit
3c0471b5fd
This commit required support for inline variable definition, which is
not a requirement.
RELEASE NOTE AUTHOR: the author of commit
3c0471b5fd
(pg_upgrade/tablespaces) was Justin Pryzby, not me.
Reported-by: Andres Freund
Discussion: https://postgr.es/m/
20201016001959[email protected]
Backpatch-through: 9.5
Bruce Momjian [Thu, 15 Oct 2020 23:33:36 +0000 (19:33 -0400)]
pg_upgrade: generate check error for left-over new tablespace
Previously, if pg_upgrade failed, and the user recreated the cluster but
did not remove the new cluster tablespace directory, a later pg_upgrade
would fail since the new tablespace directory would already exists.
This adds error reporting for this during check.
Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/
20200925005531[email protected]
Backpatch-through: 9.5
Andres Freund [Thu, 15 Oct 2020 20:39:41 +0000 (13:39 -0700)]
llvmjit: Also copy parameter / return value attributes from template functions.
Previously we only copied the function attributes. That caused problems at
least on s390x: Because we didn't copy the 'zeroext' attribute for
ExecAggTransReparent()'s *IsNull parameters, expressions invoking it didn't
ensure that the upper bytes of the registers were zeroed. In the - relatively
rare - cases where not, ExecAggTransReparent() wrongly ended up in the
newValueIsNull branch due to the register not being zero. Subsequently causing
a crash.
It's quite possible that this would cause problems on other platforms, and in
other places than just ExecAggTransReparent() on s390x.
Thanks to Christoph (and the Debian project) for providing me with access to a
s390x machine, allowing me to debug this.
Reported-By: Christoph Berg
Author: Andres Freund
Discussion: https://postgr.es/m/
20201015083246[email protected]
Backpatch: 11-, where JIT was added
Bruce Momjian [Thu, 15 Oct 2020 19:15:29 +0000 (15:15 -0400)]
doc: improve description of synchronous_commit modes
Previously it wasn't clear exactly what each of the synchronous_commit
modes accomplished. This clarifies that, and adds a table describing it.
Only backpatched through 9.6 since 9.5 doesn't have all the options.
Reported-by: [email protected]
Discussion: https://postgr.es/m/
159741195522.14321.
13812604195366728976@wrigleys.postgresql.org
Backpatch-through: 9.6
Tom Lane [Thu, 15 Oct 2020 16:50:57 +0000 (12:50 -0400)]
In the postmaster, rely on the signal infrastructure to block signals.
POSIX sigaction(2) can be told to block a set of signals while a
signal handler executes. Make use of that instead of manually
blocking and unblocking signals in the postmaster's signal handlers.
This should save a few cycles, but more importantly it prevents
recursive invocation of signal handlers when many signals arrive in
close succession. (Assuming that the platform's signal infrastructure
is designed to avoid consuming stack space in that case, but this is
demonstrably true at least on Linux.) The existing code has been seen
to recurse to the point of stack overflow, either in the postmaster
or in a forked-off child.
Back-patch of commit
9abb2bfc0. At the time, we'd only seen excess
postmaster stack consumption in the buildfarm; but we now have a
user report of it, and that commit has aged enough to have a fair
amount of confidence that it doesn't break anything.
This still doesn't change anything about the way that it works on
Windows. Perhaps someone else would like to fix that?
Per bug #16673 from David Geier. Back-patch to 9.6. Although
the problem exists in principle before that, we've only seen it
actually materialize in connection with heavy use of parallel
workers, so it doesn't seem necessary to do anything in 9.5;
and the relevant code is different there, too.
Discussion: https://postgr.es/m/16673-
d278c604f8e34ec0@postgresql.org
Discussion: https://postgr.es/m/14878.
1570820201@sss.pgh.pa.us
Fujii Masao [Thu, 15 Oct 2020 02:04:07 +0000 (11:04 +0900)]
doc: Mention that toast_tuple_target affects also column marked as Main.
Previously it was documented that toast_tuple_target affected column
marked as only External or Extended. But this description is not correct
and toast_tuple_target affects also column marked as Main.
Back-patch to v11 where toast_tuple_target reloption was introduced.
Author: Shinya Okano
Reviewed-by: Tatsuhito Kasahara, Fujii Masao
Discussion: https://postgr.es/m/
93f46e311a67422e89e770d236059817@oss.nttdata.com
Tom Lane [Tue, 13 Oct 2020 21:44:56 +0000 (17:44 -0400)]
Paper over regression failures in infinite_recurse() on PPC64 Linux.
Our infinite_recurse() test to verify sane stack-overrun behavior
is affected by a bug of the Linux kernel on PPC64: it will get SIGSEGV
if it receives a signal when the stack depth is (a) over 1MB and
(b) within a few kB of filling the current physical stack allocation.
See https://bugzilla.kernel.org/show_bug.cgi?id=205183.
Since this test is a bit time-consuming and we run it in parallel with
test scripts that do a lot of DDL, it can be expected to get an sinval
catchup interrupt at some point, leading to failure if the timing is
wrong. This has caused more than 100 buildfarm failures over the
past year or so.
While a fix exists for the kernel bug, it might be years before that
propagates into all production kernels, particularly in some of the
older distros we have in the buildfarm. For now, let's just back off
and not run this test on Linux PPC64; that loses nothing in test
coverage so far as our own code is concerned.
To do that, split this test into a new script infinite_recurse.sql
and skip the test when the platform name is powerpc64...-linux-gnu.
Back-patch to v12. Branches before that have not been seen to get
this failure. No doubt that's because the "errors" test was not
run in parallel with other tests before commit
798070ec0, greatly
reducing the odds of an sinval catchup being necessary.
I also back-patched
3c8553547 into v12, just so the new regression
script would look the same in all branches having it.
Discussion: https://postgr.es/m/
3479046.
1602607848@sss.pgh.pa.us
Discussion: https://postgr.es/m/
20190723162703.GM22387%40telsasoft.com
Tom Lane [Mon, 12 Oct 2020 22:01:34 +0000 (18:01 -0400)]
Fix GiST buffering build to work when there are included columns.
gistRelocateBuildBuffersOnSplit did not get the memo about which
attribute count to use. This could lead to a crash if there were
included columns and buffering build was chosen. (Because there
are random page-split decisions elsewhere in GiST index build,
the crashes are not entirely deterministic.)
Back-patch to v12 where GiST gained support for included columns.
Pavel Borisov
Discussion: https://postgr.es/m/CALT9ZEECCV5m7wvxg46PC-7x-EybUmnpupBGhSFMoAAay+r6HQ@mail.gmail.com
Tom Lane [Mon, 12 Oct 2020 17:31:24 +0000 (13:31 -0400)]
Fix memory leak when guc.c decides a setting can't be applied now.
The prohibitValueChange code paths in set_config_option(), which
are executed whenever we re-read a PGC_POSTMASTER variable from
postgresql.conf, neglected to free anything before exiting. Thus
we'd leak the proposed new value of a PGC_STRING variable, as noted
by BoChen in bug #16666. For all variable types, if the check hook
creates an "extra" chunk, we'd also leak that.
These are malloc not palloc chunks, so there is no mechanism for
recovering the leaks before process exit. Fortunately, the values
are typically not very large, meaning you'd have to go through an
awful lot of SIGHUP configuration-reload cycles to make the leakage
amount to anything. Still, for a long-lived postmaster process it
could potentially be a problem.
Oversight in commit
2594cf0e8. Back-patch to all supported branches.
Discussion: https://postgr.es/m/16666-
2c41a4eec61b03e1@postgresql.org
Tom Lane [Wed, 7 Oct 2020 22:41:39 +0000 (18:41 -0400)]
Fix optimization hazard in gram.y's makeOrderedSetArgs(), redux.
It appears that commit
cf63c641c, which intended to prevent
misoptimization of the result-building step in makeOrderedSetArgs,
didn't go far enough: buildfarm member hornet's version of xlc
is now optimizing back to the old, broken behavior in which
list_length(directargs) is fetched only after list_concat() has
changed that value. I'm not entirely convinced whether that's
an undeniable compiler bug or whether it can be justified by a
sufficiently aggressive interpretation of C sequence points.
So let's just change the code to make it harder to misinterpret.
Back-patch to all supported versions, just in case.
Discussion: https://postgr.es/m/
1830491.
1601944935@sss.pgh.pa.us
Tom Lane [Wed, 7 Oct 2020 16:50:54 +0000 (12:50 -0400)]
Rethink recent fix for pg_dump's handling of extension config tables.
Commit
3eb3d3e78 was a few bricks shy of a load: while it correctly
set the table's "interesting" flag when deciding to dump the data of
an extension config table, it was not correct to clear that flag
if we concluded we shouldn't dump the data. This led to the crash
reported in bug #16655, because in fact we'll traverse dumpTableSchema
anyway for all extension tables (to see if they have user-added
seclabels or RLS policies).
The right thing to do is to force "interesting" true in makeTableDataInfo,
and otherwise leave the flag alone. (Doing it there is more future-proof
in case additional calls are added, and it also avoids setting the flag
unnecessarily if that function decides the table is non-dumpable.)
This investigation also showed that while only the --inserts code path
had an obvious failure in the case considered by
3eb3d3e78, the COPY
code path also has a problem with not having loaded table subsidiary
data. That causes fmtCopyColumnList to silently return an empty string
instead of the correct column list. That accidentally mostly works,
which perhaps is why we didn't notice this before. It would only fail
if the restore column order is different from the dump column order,
which only happens in weird inheritance cases, so it's not surprising
nobody had hit the case with an extension config table. Nonetheless,
it's a bug, and it goes a long way back, not just to v12 where the
--inserts code path started to have a problem with this.
In hopes of catching such cases a bit sooner in future, add some
Asserts that "interesting" has been set in both dumpTableData and
dumpTableSchema. Adjust the test case added by
3eb3d3e78 so that it
checks the COPY rather than INSERT form of that bug, allowing it to
detect the longer-standing symptom.
Per bug #16655 from Cameron Daniel. Back-patch to all supported
branches.
Discussion: https://postgr.es/m/16655-
5c92d6b3a9438137@postgresql.org
Discussion: https://postgr.es/m/
18048b44-3414-b983-8c7c-
9165b177900d@2ndQuadrant.com
Bruce Momjian [Tue, 6 Oct 2020 18:31:21 +0000 (14:31 -0400)]
pg_upgrade: remove pre-8.4 code and >= 8.4 check
We only support upgrading from >= 8.4 so no need for this code or tests.
Reported-by: Magnus Hagander
Discussion: https://postgr.es/m/CABUevEx-D0PNVe00tkeQRGennZQwDtBJn=493MJt-x6sppbUxA@mail.gmail.com
Backpatch-through: 9.5
Bruce Momjian [Tue, 6 Oct 2020 16:12:09 +0000 (12:12 -0400)]
pg_upgrade; change major version comparisons to use <=, not <
This makes checking for older major versions more consistent.
Backpatch-through: 9.5
Tom Lane [Tue, 6 Oct 2020 15:43:54 +0000 (11:43 -0400)]
Build EC members for child join rels in the right memory context.
This patch prevents crashes or wrong plans when partition-wise joins
are considered during GEQO planning, as a consequence of the
EquivalenceClass data structures becoming corrupt after a GEQO
context reset.
A remaining problem is that successive GEQO cycles will make multiple
copies of the required EC members, since add_child_join_rel_equivalences
has no idea that such members might exist already. For now we'll just
live with that. The lack of field complaints of crashes suggests that
this is a mighty little-used situation.
Back-patch to v12 where this code was introduced.
Discussion: https://postgr.es/m/
1683100.
1601860653@sss.pgh.pa.us
Bruce Momjian [Mon, 5 Oct 2020 20:27:33 +0000 (16:27 -0400)]
doc: show functions returning record types and use of ROWS FROM
Previously it was unclear exactly how ROWS FROM behaved and how to cast
the data types of columns returned by FROM functions. Also document
that only non-OUT record functions can have their columns cast to data
types.
Reported-by: [email protected]
Discussion: https://postgr.es/m/
158638264419.662.
2482095087061084020@wrigleys.postgresql.org
Backpatch-through: 9.5
Bruce Momjian [Mon, 5 Oct 2020 20:07:15 +0000 (16:07 -0400)]
docs: clarify the interaction of clientcert and cert auth.
This is the first paragraph change of master-only commit
253f1025da.
Backpatch-through: PG 12-13 only
Tom Lane [Mon, 5 Oct 2020 17:15:39 +0000 (13:15 -0400)]
Fix two latent(?) bugs in equivclass.c.
get_eclass_for_sort_expr() computes expr_relids and nullable_relids
early on, even though they won't be needed unless we make a new
EquivalenceClass, which we often don't. Aside from the probably-minor
inefficiency, there's a memory management problem: these bitmapsets will
be built in the caller's context, leading to dangling pointers if that
is shorter-lived than root->planner_cxt. This would be a live bug if
get_eclass_for_sort_expr() could be called with create_it = true during
GEQO join planning. So far as I can find, the core code never does
that, but it's hard to be sure that no extensions do, especially since
the comments make it clear that that's supposed to be a supported case.
Fix by not computing these values until we've switched into planner_cxt
to build the new EquivalenceClass.
generate_join_implied_equalities() uses inner_rel->relids to look up
relevant eclasses, but it ought to be using nominal_inner_relids.
This is presently harmless because a child RelOptInfo will always have
exactly the same eclass_indexes as its topmost parent; but that might
not be true forever, and anyway it makes the code confusing.
The first of these is old (introduced by me in
f3b3b8d5b), so back-patch
to all supported branches. The second only dates to v13, but we might
as well back-patch it to keep the code looking similar across branches.
Discussion: https://postgr.es/m/
1508010.
1601832581@sss.pgh.pa.us
Tom Lane [Mon, 5 Oct 2020 00:45:06 +0000 (20:45 -0400)]
Improve stability of identity.sql regression test.
I noticed while trying to run the regression tests under a low
geqo_threshold that one query on information_schema.columns had
unstable (as in, variable from one run to the next) output order.
This is pretty unsurprising given the complexity of the underlying
plan. Interestingly, of this test's three nigh-identical queries on
information_schema.columns, the other two already had ORDER BY clauses
guaranteeing stable output. Let's make this one look the same.
Back-patch to v10 where this test was added. We've not heard field
reports of the test failing, but this experience shows that it can
happen when testing under even slightly unusual conditions.
Bruce Momjian [Sat, 3 Oct 2020 02:19:31 +0000 (22:19 -0400)]
doc: libpq connection options can override command-line flags
Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/16486-
b9c93d71c02c4907@postgresql.org
Backpatch-through: 9.5
Bruce Momjian [Sat, 3 Oct 2020 01:39:33 +0000 (21:39 -0400)]
doc: clarify the use of ssh port forwarding
Reported-by: [email protected]
Discussion: https://postgr.es/m/
159854511172.24991.
4373145230066586863@wrigleys.postgresql.org
Backpatch-through: 9.5
Tom Lane [Thu, 1 Oct 2020 14:59:20 +0000 (10:59 -0400)]
Put back explicit setting of replication values within TAP tests.
Commit
151c0c5f7 neglected the possibility that a TEMP_CONFIG file
would explicitly set max_wal_senders=0; as indeed buildfarm member
thorntail does, so that it can test wal_level=minimal in other test
suites. Hence, rather than assuming that max_wal_senders=10 will
prevail if we say nothing, set it explicitly.
Set max_replication_slots=10 explicitly too, just to be safe.
Back-patch to v10, like the previous patch.
Discussion: https://postgr.es/m/723911.
1601417626@sss.pgh.pa.us
Heikki Linnakangas [Thu, 1 Oct 2020 08:48:48 +0000 (11:48 +0300)]
Fix incorrect assertion on number of array dimensions.
This has been wrong ever since the support for multi-dimensional
arrays as PL/python function arguments and return values was
introduced in commit
94aceed317.
Backpatch-through: 10
Discussion: https://www.postgresql.org/message-id/
61647b8e-961c-0362-d5d3-
c8a18f4a7ec6%40iki.fi
Alvaro Herrera [Wed, 30 Sep 2020 21:25:23 +0000 (18:25 -0300)]
Reword partitioning error message
The error message about columns in the primary key not including all of
the partition key was unclear; reword it.
Backpatch all the way to pg11, where it appeared.
Reported-by: Nagaraj Raj
Discussion: https://postgr.es/m/
64062533.78364.
1601415362244@mail.yahoo.com
Tom Lane [Wed, 30 Sep 2020 19:40:23 +0000 (15:40 -0400)]
Fix handling of BC years in to_date/to_timestamp.
Previously, a conversion such as
to_date('-44-02-01','YYYY-MM-DD')
would result in '0045-02-01 BC', as the code attempted to interpret
the negative year as BC, but failed to apply the correction needed
for our internal handling of BC years. Fix the off-by-one problem.
Also, arrange for the combination of a negative year and an
explicit "BC" marker to cancel out and produce AD. This is how
the negative-century case works, so it seems sane to do likewise.
Continue to read "year 0000" as 1 BC. Oracle would throw an error,
but we've accepted that case for a long time so I'm hesitant to
change it in a back-patch.
Per bug #16419 from Saeed Hubaishan. Back-patch to all supported
branches.
Dar Alathar-Yemen and Tom Lane
Discussion: https://postgr.es/m/16419-
d8d9db0a7553f01b@postgresql.org
David Rowley [Wed, 30 Sep 2020 00:03:34 +0000 (13:03 +1300)]
Doc: Improve clarity on partitioned table limitations
Explicitly mention that primary key constraints are also included in the
limitation that the constraint columns must be a superset of the partition key
columns.
Wording suggestion from Tom Lane.
Discussion: https://postgr.es/m/
64062533.78364.
1601415362244@mail.yahoo.com
Backpatch-through: 11, where unique constraints on partitioned tables were added
Tom Lane [Wed, 30 Sep 2020 00:02:58 +0000 (20:02 -0400)]
Remove obsolete replication settings within TAP tests.
PostgresNode.pm set "max_wal_senders = 5" for replication testing,
but this seems to be slightly too low for our current test suite.
Slower buildfarm members frequently report "number of requested standby
connections exceeds max_wal_senders" failures, due to old walsenders
not exiting instantaneously. Usually, the test does not fail overall
because of automatic walreceiver restart, but sometimes the failure
becomes visible; and in any case such retries slow down the test.
That value came in with commit
89ac7004d, but was soon obsoleted by
f6d6d2920, which raised the built-in default from zero to 10; so that
PostgresNode.pm is actually setting it to less than the conservative
built-in default. That seems pretty pointless, so let's remove the
special setting and let the default prevail, in hopes of making
the TAP tests more robust.
Likewise, the setting "max_replication_slots = 5" is obsolete and
can be removed.
While here, reverse-engineer a comment about why we're choosing
less-than-default values for some other settings.
(Note: before v12, max_wal_senders counted against max_connections
so that the latter setting also needs some fiddling with.)
Back-patch to v10 where the subscription tests were added.
It's likely that the older branches aren't pushing the boundaries
of max_wal_senders, but I'm disinclined to spend time trying to
figure out exactly when it started to be a problem.
Discussion: https://postgr.es/m/723911.
1601417626@sss.pgh.pa.us
Tom Lane [Tue, 29 Sep 2020 15:18:31 +0000 (11:18 -0400)]
Fix memory leak in plpgsql's CALL processing.
When executing a CALL or DO in a non-atomic context (i.e., not inside
a function or query), plpgsql creates a new plan each time through,
as a rather hacky solution to some resource management issues. But
it failed to free this plan until exit of the current procedure or DO
block, resulting in serious memory bloat in procedures that called
other procedures many times. Fix by remembering to free the plan,
and by being more honest about restoring the previous state (otherwise,
recursive procedure calls have a problem).
There was also a smaller leak associated with recalculation of the
"target" list of output variables. Fix that by using the statement-
lifespan context to hold non-permanent values.
Back-patch to v11 where procedures were introduced.
Pavel Stehule and Tom Lane
Discussion: https://postgr.es/m/CAFj8pRDiiU1dqym+_P4_GuTWm76knJu7z9opWayBJTC0nQGUUA@mail.gmail.com
Fujii Masao [Tue, 29 Sep 2020 07:21:46 +0000 (16:21 +0900)]
Archive timeline history files in standby if archive_mode is set to "always".
Previously the standby server didn't archive timeline history files
streamed from the primary even when archive_mode is set to "always",
while it archives the streamed WAL files. This could cause the PITR to
fail because there was no required timeline history file in the archive.
The cause of this issue was that walreceiver didn't mark those files as
ready for archiving.
This commit makes walreceiver mark those streamed timeline history
files as ready for archiving if archive_mode=always. Then the archiver
process archives the marked timeline history files.
Back-patch to all supported versions.
Reported-by: Grigory Smolkin
Author: Grigory Smolkin, Fujii Masao
Reviewed-by: David Zhang, Anastasia Lubennikova
Discussion: https://postgr.es/m/
54b059d4-2b48-13a4-6f43-
95a087c92367@postgrespro.ru
Michael Paquier [Tue, 29 Sep 2020 05:16:18 +0000 (14:16 +0900)]
Fix progress reporting of REINDEX CONCURRENTLY
This addresses a couple of issues with the so-said subject:
- Report the correct parent relation with the index actually being
rebuilt or validated. Previously, the command status remained set to
the last index created for the progress of the index build and
validation, which would be incorrect when working on a table that has
more than one index.
- Use the correct phase when waiting before the drop of the old
indexes. Previously, this was reported with the same status as when
waiting before the old indexes are marked as dead.
Author: Matthias van de Meent, Michael Paquier
Discussion: https://postgr.es/m/CAEze2WhqFgcwe1_tv=sFYhLWV2AdpfukumotJ6JNcAOQs3jufg@mail.gmail.com
Backpatch-through: 12
Tom Lane [Mon, 28 Sep 2020 18:12:38 +0000 (14:12 -0400)]
Assign collations in partition bound expressions.
Failure to do this can result in errors during evaluation of
the bound expression, as illustrated by the new regression test.
Back-patch to v12 where the ability for partition bounds to be
expressions was added.
Discussion: https://postgr.es/m/CAJV4CdrZ5mKuaEsRSbLf2URQ3h6iMtKD=hik8MaF5WwdmC9uZw@mail.gmail.com
Tom Lane [Sat, 26 Sep 2020 20:04:06 +0000 (16:04 -0400)]
Revise RelationBuildRowSecurity() to avoid memory leaks.
This function leaked some memory while loading qual clauses for
an RLS policy. While ordinarily negligible, that could build up
in some repeated-reload cases, as reported by Konstantin Knizhnik.
We can improve matters by borrowing the coding long used in
RelationBuildRuleLock: build stringToNode's result directly in
the target context, and remember to explicitly pfree the
input string.
This patch by no means completely guarantees zero leaks within
this function, since we have no real guarantee that the catalog-
reading subroutines it calls don't leak anything. However,
practical tests suggest that this is enough to resolve the issue.
In any case, any remaining leaks are similar to those risked by
RelationBuildRuleLock and other relcache-loading subroutines.
If we need to fix them, we should adopt a more global approach
such as that used by the RECOVER_RELATION_BUILD_MEMORY hack.
While here, let's remove the need for an expensive PG_TRY block by
using MemoryContextSetParent to reparent an initially-short-lived
context for the RLS data.
Back-patch to all supported branches.
Discussion: https://postgr.es/m/
21356c12-8917-8249-b35f-
1c447231922b@postgrespro.ru
Tom Lane [Thu, 24 Sep 2020 22:19:38 +0000 (18:19 -0400)]
Fix handling of -d "connection string" in pg_dump/pg_restore.
Parallel pg_dump failed if its -d parameter was a connection string
containing any essential information other than host, port, or username.
The same was true for pg_restore with --create.
The reason is that these scenarios failed to preserve the connection
string from the command line; the code felt free to replace that with
just the database name when reconnecting from a pg_dump parallel worker
or after creating the target database. By chance, parallel pg_restore
did not suffer this defect, as long as you didn't say --create.
In practice it seems that the error would be obvious only if the
connstring included essential, non-default SSL or GSS parameters.
This may explain why it took us so long to notice. (It also makes
it very difficult to craft a regression test case illustrating the
problem, since the test would fail in builds without those options.)
Fix by refactoring so that ConnectDatabase always receives all the
relevant options directly from the command line, rather than
reconstructed values. Inject a different database name, when necessary,
by relying on libpq's rules for handling multiple "dbname" parameters.
While here, let's get rid of the essentially duplicate _connectDB
function, as well as some obsolete nearby cruft.
Per bug #16604 from Zsolt Ero. Back-patch to all supported branches.
Discussion: https://postgr.es/m/16604-
933f4b8791227b15@postgresql.org
Thomas Munro [Wed, 23 Sep 2020 21:26:09 +0000 (09:26 +1200)]
Fix missing fsync of SLRU directories.
Harmonize behavior by moving reponsibility for fsyncing directories down
into slru.c. In 10 and later, only the multixact directories were
missed (see commit
1b02be21), and in older branches all SLRUs were
missed.
Back-patch to all supported releases.
Reviewed-by: Andres Freund
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/CA%2BhUKGLtsTUOScnNoSMZ-2ZLv%2BwGh01J6kAo_DM8mTRq1sKdSQ%40mail.gmail.com
Tom Lane [Wed, 23 Sep 2020 15:36:13 +0000 (11:36 -0400)]
Avoid possible dangling-pointer access in tsearch_readline_callback.
tsearch_readline() saves the string pointer it returns to the caller
for possible use in the associated error context callback. However,
the caller will usually pfree that string sometime before it next
calls tsearch_readline(), so that there is a window where an ereport
will try to print an already-freed string.
The built-in users of tsearch_readline() happen to all do that pfree
at the bottoms of their loops, so that the window is effectively
empty for them. However, this is not documented as a requirement,
and contrib/dict_xsyn doesn't do it like that, so it seems likely
that third-party dictionaries might have live bugs here.
The practical consequences of this seem pretty limited in any case,
since production builds wouldn't clobber the freed string immediately,
besides which you'd not expect syntax errors in dictionary files
being used in production. Still, it's clearly a bug waiting to bite
somebody.
Fix by pstrdup'ing the string to be saved for the error callback,
and then pfree'ing it next time through. It's been like this for
a long time, so back-patch to all supported branches.
Discussion: https://postgr.es/m/
48A4FA71-524E-41B9-953A-
FD04EF36E2E7@yesql.se
Peter Eisentraut [Sun, 20 Sep 2020 12:41:28 +0000 (14:41 +0200)]
Fix whitespace
Tom Lane [Fri, 18 Sep 2020 22:03:44 +0000 (18:03 -0400)]
Use factorial rather than numeric_fac in create_operator.sql.
These two SQL functions are aliases for the same C function, so this
change has no semantic effect. However, because we dropped the
numeric_fac alias in HEAD (commit
76f412ab3), operator definitions
based on that one don't port forward, causing problems for cross-version
upgrade tests based on the regression database.
Patch all active back branches to dodge the problem.
Discussion: https://postgr.es/m/449144.
1600439950@sss.pgh.pa.us
Amit Kapila [Thu, 17 Sep 2020 09:46:46 +0000 (15:16 +0530)]
Update parallel BTree scan state when the scan keys can't be satisfied.
For parallel btree scan to work for array of scan keys, it should reach
BTPARALLEL_DONE state once for every distinct combination of array keys.
This is required to ensure that the parallel workers don't try to seize
blocks at the same time for different scan keys. We missed to update this
state when we discovered that the scan keys can't be satisfied.
Author: James Hunter
Reviewed-by: Amit Kapila
Tested-by: Justin Pryzby
Backpatch-through: 10, where it was introduced
Discussion: https://postgr.es/m/
4248CABC-25E3-4809-B4D0-
128E1BAABC3C@amazon.com
Tom Lane [Wed, 16 Sep 2020 17:38:26 +0000 (13:38 -0400)]
Avoid unnecessary recursion to child tables in ALTER TABLE SET NOT NULL.
If a partitioned table's column is already marked NOT NULL, there is
no need to examine its partitions, because we can rely on previous
DDL to have enforced that the child columns are NOT NULL as well.
(Unfortunately, the same cannot be said for traditional inheritance,
so for now we have to restrict the optimization to partitioned tables.)
Hence, we may skip recursing to child tables in this situation.
The reason this case is worth worrying about is that when pg_dump dumps
a partitioned table having a primary key, it will include the requisite
NOT NULL markings in the CREATE TABLE commands, and then add the
primary key as a separate step. The primary key addition generates a
SET NOT NULL as a subcommand, just to be sure. So the situation where
a SET NOT NULL is redundant does arise in the real world.
Skipping the recursion does more than just save a few cycles: it means
that a command such as "ALTER TABLE ONLY partition_parent ADD PRIMARY
KEY" will take locks only on the partition parent table, not on the
partitions. It turns out that parallel pg_restore is effectively
assuming that that's true, and has little choice but to do so because
the dependencies listed for such a TOC entry don't include the
partitions. pg_restore could thus issue this ALTER while data restores
on the partitions are still in progress. Taking unnecessary locks on
the partitions not only hurts concurrency, but can lead to actual
deadlock failures, as reported by Domagoj Smoljanovic.
(A contributing factor in the deadlock is that TRUNCATE on a child
partition wants a non-exclusive lock on the parent. This seems
likewise unnecessary, but the fix for it is more invasive so we
won't consider back-patching it. Fortunately, getting rid of one
of these two poor behaviors is enough to remove the deadlock.)
Although support for partitioned primary keys came in with v11,
this patch is dependent on the SET NOT NULL refactoring done by
commit
f4a3fdfbd, so we can only patch back to v12.
Patch by me; thanks to Alvaro Herrera and Amit Langote for review.
Discussion: https://postgr.es/m/VI1PR03MB31670CA1BD9625C3A8C5DD05EB230@VI1PR03MB3167.eurprd03.prod.outlook.com
Tom Lane [Wed, 16 Sep 2020 16:07:31 +0000 (12:07 -0400)]
Fix bogus cache-invalidation logic in logical replication worker.
The code recorded cache invalidation events by zeroing the "localreloid"
field of affected cache entries. However, it's possible for an inval
event to occur even while we have the entry open and locked. So an
ill-timed inval could result in "cache lookup failed for relation 0"
errors, if the worker's code tried to use the cleared field. We can
fix that by creating a separate bool field to record whether the entry
needs to be revalidated. (In the back branches, cram the bool into
what had been padding space, to avoid an ABI break in the somewhat
unlikely event that any extension is looking at this struct.)
Also, rearrange the logic in logicalrep_rel_open so that it
does the right thing in cases where table_open would fail.
We should retry the lookup by name in that case, but we didn't.
The real-world impact of this is probably small. In the first place,
the error conditions are very low probability, and in the second place,
the worker would just exit and get restarted. We only noticed because
in a CLOBBER_CACHE_ALWAYS build, the failure can occur repeatedly,
preventing the worker from making progress. Nonetheless, it's clearly
a bug, and it impedes a useful type of testing; so back-patch to v10
where this code was introduced.
Discussion: https://postgr.es/m/
1032727.
1600096803@sss.pgh.pa.us
Noah Misch [Mon, 14 Sep 2020 06:29:51 +0000 (23:29 -0700)]
Fix interpolation in test name.
A pre-commit review had reported the problem, but the fix reached only
v10 and earlier. Back-patch to v11.
Discussion: https://postgr.es/m/
20200423.140546.
1055476118690602079[email protected]
Tom Lane [Sun, 13 Sep 2020 16:51:21 +0000 (12:51 -0400)]
Use the properly transformed RangeVar for expandTableLikeClause().
transformCreateStmt() adjusts the transformed statement's RangeVar
to specify the target schema explicitly, for the express reason
of making sure that auxiliary statements derived by parse
transformation operate on the right table. But the refactoring
I did in commit
502898192 got this wrong and passed the untransformed
RangeVar to expandTableLikeClause(). This could lead to assertion
failures or weird misbehavior if the wrong table was accessed.
Per report from Alexander Lakhin. Like the previous patch, back-patch
to all supported branches.
Discussion: https://postgr.es/m/
05051f9d-b32b-cb35-6735-
0e9f2ab86b5f@gmail.com
Peter Eisentraut [Sun, 6 Sep 2020 14:46:13 +0000 (16:46 +0200)]
doc: Don't hide the "Up" link when it is the same as "Home"
The original stylesheets seemed to think this was a good idea, but our
users find it confusing and unhelpful, so undo that logic.
Reported-by: Fabien COELHO
Discussion: https://www.postgresql.org/message-id/flat/alpine.DEB.2.22.394.
2006210914370.859381%40pseudo
Tom Lane [Thu, 10 Sep 2020 16:06:26 +0000 (12:06 -0400)]
Use _exit(2) for SIGQUIT during ProcessStartupPacket, too.
Bring the signal handling for startup-packet collection into line
with the policy established in commits
bedadc732 and
8e19a8264,
namely don't risk running atexit callbacks when handling SIGQUIT.
Ideally, we'd not do so for SIGTERM or timeout interrupts either,
but that change seems a bit too risky for the back branches.
For now, just improve the comments in this area to describe the risk.
Also relocate where BackendInitialize re-disables these interrupts,
to minimize the code span where they're active. This doesn't buy
a whole lot of safety, but it can't hurt.
In passing, rename startup_die() to remove confusion about whether
it is for the startup process.
Like the previous commits, back-patch to all supported branches.
Discussion: https://postgr.es/m/
1850884.
1599601164@sss.pgh.pa.us
Peter Eisentraut [Thu, 10 Sep 2020 13:31:09 +0000 (15:31 +0200)]
doc: Remove buggy ICU collation from documentation
We have had multiple reports that point to the
'@colReorder=latn-digit' collation customization being buggy. We have
reported this to ICU and are waiting for a fix. In the meantime,
remove references to this from the documentation and replace it by
another reordering example. Apparently, many users have been picking
up this example specifically from the documentation.
Author: Jehan-Guillaume de Rorthais
Discussion: https://www.postgresql.org/message-id/flat/153201618542.1404.3611626898935613264%40wrigleys.postgresql.org
Magnus Hagander [Thu, 10 Sep 2020 12:15:26 +0000 (14:15 +0200)]
Fix title in reference section
Reported-by: Robert Kahlert
Author: Daniel Gustafsson
Michael Paquier [Thu, 10 Sep 2020 06:50:46 +0000 (15:50 +0900)]
doc: Fix some grammar and inconsistencies
Some comments are fixed while on it.
Author: Justin Pryzby
Discussion: https://postgr.es/m/
20200818171702[email protected]
Backpatch-through: 9.6
Tom Lane [Wed, 9 Sep 2020 19:32:34 +0000 (15:32 -0400)]
Make archiver's SIGQUIT handler exit via _exit().
Commit
8e19a8264 changed the SIGQUIT handlers of almost all server
processes not to run atexit callbacks. The archiver process was
skipped, perhaps because it's not connected to shared memory; but
it's just as true here that running atexit callbacks in a signal
handler is unsafe. So let's make it work like the rest.
In HEAD and v13, we can use the common SignalHandlerForCrashExit
handler. Before that, just tweak pgarch_exit to use _exit(2)
explicitly.
Like the previous commit, back-patch to all supported branches.
Kyotaro Horiguchi, back-patching by me
Discussion: https://postgr.es/m/
1850884.
1599601164@sss.pgh.pa.us
Alvaro Herrera [Tue, 8 Sep 2020 22:35:15 +0000 (19:35 -0300)]
Check default partitions constraints while descending
Partitioning tuple route code assumes that the partition chosen while
descending the partition hierarchy is always the correct one. This is
true except when the partition is the default partition and another
partition has been added concurrently: the partition constraint changes
and we don't recheck it. This can lead to tuples mistakenly being added
to the default partition that should have been rejected.
Fix by rechecking the default partition constraint while descending the
hierarchy.
An isolation test based on the reproduction steps described by Hao Wu
(with tweaks for extra coverage) is included.
Backpatch to 12, where this bug came in with
898e5e3290a7.
Reported by: Hao Wu
Author: Amit Langote
Author: Álvaro Herrera
Discussion: https://postgr.es/m/CA+HiwqFqBmcSSap4sFnCBUEL_VfOMmEKaQ3gwUhyfa4c7J_-nA@mail.gmail.com
Discussion: https://postgr.es/m/DM5PR0501MB3910E97A9EDFB4C775CF3D75A42F0@DM5PR0501MB3910.namprd05.prod.outlook.com
Michael Paquier [Mon, 7 Sep 2020 05:35:17 +0000 (14:35 +0900)]
doc: Tweak sentence for pg_checksums when enabling checksums
The previous version of the docs mentioned that files are rewritten,
implying that a second copy of each file gets created, but each file is
updated in-place.
Author: Michael Banck
Reviewed-by: Daniel Gustafsson, Michael Paquier
Discussion: https://postgr.es/m/
858086b6a42fb7d17995b6175856f7e7ec44d0a2[email protected]
Backpatch-through: 12
Tom Lane [Sun, 6 Sep 2020 16:55:13 +0000 (12:55 -0400)]
Fix misleading error message about inconsistent moving-aggregate types.
We reported the wrong types when complaining that an aggregate's
moving-aggregate implementation is inconsistent with its regular
implementation.
This was wrong since the feature was introduced, so back-patch
to all supported branches.
Jeff Janes
Discussion: https://postgr.es/m/CAMkU=1x808LH=LPhZp9mNSP0Xd1xDqEd+XeGcvEe48dfE6xV=A@mail.gmail.com
Tom Lane [Sun, 6 Sep 2020 15:50:41 +0000 (11:50 -0400)]
Remove useless lstat() call in pg_rewind.
This is duplicative of an lstat that was just done by the calling
function (traverse_datadir), besides which we weren't really doing
anything with the results. There's not much point in checking to
see if someone removed the file since the previous lstat, since the
FILE_ACTION_REMOVE code would have to deal with missing-file cases
anyway. Moreover, the "exists = false" assignment was a dead store;
nothing was done with that value later.
A syscall saved is a syscall earned, so back-patch to 9.5
where this code was introduced.
Discussion: https://postgr.es/m/
1221796.
1599329320@sss.pgh.pa.us
Tom Lane [Sat, 5 Sep 2020 01:01:59 +0000 (21:01 -0400)]
Make new authentication test case more robust.
I happened to notice that the new test case I added in
b55b4dad9
falls over if one runs "make check" repeatedly; though not in branches
after v10. That's because it was assuming that tmp_check/pgpass
wouldn't exist already. However, it's only been since v11 that the
Makefiles forcibly remove all of tmp_check/ before starting a TAP run.
This fix to unlink the file is therefore strictly necessary only in
v10 ... but it seems wisest to do it across the board, rather than
let the test rely on external logic to get the conditions right.
Tom Lane [Sat, 5 Sep 2020 00:20:05 +0000 (20:20 -0400)]
Fix over-eager ping'ing in logical replication receiver.
Commit
3f60f690f only partially fixed the broken-status-tracking
issue in LogicalRepApplyLoop: we need ping_sent to have the same
lifetime as last_recv_timestamp. The effects are much less serious
than what that commit fixed, though. AFAICS this would just lead to
extra ping requests being sent, once per second until the sender
responds. Still, it's a bug, so backpatch to v10 as before.
Discussion: https://postgr.es/m/959627.
1599248476@sss.pgh.pa.us
Andrew Dunstan [Fri, 4 Sep 2020 17:53:09 +0000 (13:53 -0400)]
Collect attribute data on extension owned tables being dumped
If this data is not collected, pg_dump segfaults if asked for column
inserts.
Fix by Fabrízio de Royes Mello
Backpatch to release 12 where the bug was introduced.
Bruce Momjian [Fri, 4 Sep 2020 17:27:52 +0000 (13:27 -0400)]
C comment: correct use of 64-"byte" cache line size
Reported-by: Kelly Min
Discussion: https://postgr.es/m/CAPSbxatOiQO90LYpSC3+svAU9-sHgDfEP4oFhcEUt_X=DqFA9g@mail.gmail.com
Backpatch-through: 9.5
Tom Lane [Fri, 4 Sep 2020 16:40:28 +0000 (12:40 -0400)]
Fix rare deadlock failure in create_am regression test.
The "DROP ACCESS METHOD gist2" test will require locking the index
to be dropped and then its table; while most ordinary operations
lock a table first then its index. While no concurrent test scripts
should be touching fast_emp4000, autovacuum might chance to be
processing that table when the DROP runs, resulting in a deadlock
failure. This is pretty rare but we see it in the buildfarm from
time to time.
To fix, acquire a lock on fast_emp4000 before issuing the DROP.
Since the point of the exercise is mostly to prevent buildfarm
failures, back-patch to 9.6 where this test was introduced.
Discussion: https://postgr.es/m/839004.
1599185607@sss.pgh.pa.us
Tom Lane [Thu, 3 Sep 2020 20:52:09 +0000 (16:52 -0400)]
Avoid lockup of a parallel worker when reporting a long error message.
Because sigsetjmp() will restore the initial state with signals blocked,
the code path in bgworker.c for reporting an error and exiting would
execute that way. Usually this is fairly harmless; but if a parallel
worker had an error message exceeding the shared-memory communication
buffer size (16K) it would lock up, because it would wait for a
resume-sending signal from its parallel leader which it would never
detect.
To fix, just unblock signals at the appropriate point.
This can be shown to fail back to 9.6. The lack of parallel query
infrastructure makes it difficult to provide a simple test case for
9.5; but I'm pretty sure the issue exists in some form there as well,
so apply the code change there too.
Vignesh C, reviewed by Bharath Rupireddy, Robert Haas, and myself
Discussion: https://postgr.es/m/CALDaNm1d1hHPZUg3xU4XjtWBOLCrA+-2cJcLpw-cePZ=GgDVfA@mail.gmail.com
Alvaro Herrera [Wed, 2 Sep 2020 00:43:23 +0000 (20:43 -0400)]
Fix typo in comment
Introduced by
8b08f7d4820f; backpatch to 11.
Discussion: https://postgr.es/m/
20200812214918[email protected]
Bruce Momjian [Tue, 1 Sep 2020 21:00:10 +0000 (17:00 -0400)]
doc: clarify that max_wal_size is "during" checkpoints
Previous wording was "between".
Reported-by: Pavel Luzanov
Discussion: https://postgr.es/m/
26906a54-d7cb-2f8e-eed7-
e31660024694@postgrespro.ru
Backpatch-through: 9.5
Alvaro Herrera [Tue, 1 Sep 2020 17:40:43 +0000 (13:40 -0400)]
Raise error on concurrent drop of partitioned index
We were already raising an error for DROP INDEX CONCURRENTLY on a
partitioned table, albeit a different and confusing one:
ERROR: DROP INDEX CONCURRENTLY must be first action in transaction
Change that to throw a more comprehensible error:
ERROR: cannot drop partitioned index \"%s\" concurrently
Michael Paquier authored the test case for indexes on temporary
partitioned tables.
Backpatch to 11, where indexes on partitioned tables were added.
Reported-by: Jan Mussler
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/16594-
d2956ca909585067@postgresql.org
Tom Lane [Tue, 1 Sep 2020 17:14:44 +0000 (13:14 -0400)]
Teach libpq to handle arbitrary-length lines in .pgpass files.
Historically there's been a hard-wired assumption here that no line of
a .pgpass file could be as long as NAMEDATALEN*5 bytes. That's a bit
shaky to start off with, because (a) there's no reason to suppose that
host names fit in NAMEDATALEN, and (b) this figure fails to allow for
backslash escape characters. However, it fails completely if someone
wants to use a very long password, and we're now hearing reports of
people wanting to use "security tokens" that can run up to several
hundred bytes. Another angle is that the file is specified to allow
comment lines, but there's no reason to assume that long comment lines
aren't possible.
Rather than guessing at what might be a more suitable limit, let's
replace the fixed-size buffer with an expansible PQExpBuffer. That
adds one malloc/free cycle to the typical use-case, but that's surely
pretty cheap relative to the I/O this code has to do.
Also, add TAP test cases to exercise this code, because there was no
test coverage before.
This reverts most of commit
2eb3bc588, as there's no longer a need for
a warning message about overlength .pgpass lines. (I kept the explicit
check for comment lines, though.)
In HEAD and v13, this also fixes an oversight in
74a308cf5: there's not
much point in explicit_bzero'ing the line buffer if we only do so in two
of the three exit paths.
Back-patch to all supported branches, except that the test case only
goes back to v10 where src/test/authentication/ was added.
Discussion: https://postgr.es/m/
4187382.
1598909041@sss.pgh.pa.us
Bruce Momjian [Mon, 31 Aug 2020 22:33:37 +0000 (18:33 -0400)]
doc: add commas after 'i.e.' and 'e.g.'
This follows the American format,
https://jakubmarian.com/comma-after-i-e-and-e-g/. There is no intention
of requiring this format for future text, but making existing text
consistent every few years makes sense.
Discussion: https://postgr.es/m/
20200825183619[email protected]
Backpatch-through: 9.5
Bruce Momjian [Mon, 31 Aug 2020 21:51:31 +0000 (17:51 -0400)]
C comment: remove mention of use of t_hoff WAL structure member
Reported-by: Antonin Houska
Discussion: https://postgr.es/m/21643.
1595353537@antos
Backpatch-through: 9.5
Bruce Momjian [Mon, 31 Aug 2020 21:36:22 +0000 (17:36 -0400)]
pg_upgrade doc: mention saving postgresql.conf.auto files
Also mention files included by postgresql.conf.
Reported-by: Álvaro Herrera
Discussion: https://postgr.es/m/
08AD4526-75AB-457B-B2DD-
099663F28040@yesql.se
Backpatch-through: 9.5
Bruce Momjian [Mon, 31 Aug 2020 21:05:53 +0000 (17:05 -0400)]
docs: in mapping SQL to C data types, timestamp isn't a pointer
It is an int64.
Reported-by: [email protected]
Discussion: https://postgr.es/m/
159845038271.24995.
15682121015698255155@wrigleys.postgresql.org
Backpatch-through: 9.5
Bruce Momjian [Mon, 31 Aug 2020 20:59:58 +0000 (16:59 -0400)]
doc: cross-link file-fdw and CSV config log sections
There is an file-fdw example that reads the server config file, so cross
link them.
Reported-by: Oleg Samoilov
Discussion: https://postgr.es/m/
159800192078.2886.
10431506404995508950@wrigleys.postgresql.org
Backpatch-through: 9.5
Bruce Momjian [Mon, 31 Aug 2020 20:21:03 +0000 (16:21 -0400)]
docs: clarify intermediate certificate creation instructions
Specifically, explain the v3_ca openssl specification.
Discussion: https://postgr.es/m/
20200824175653[email protected]
Backpatch-through: 9.5
Bruce Momjian [Mon, 31 Aug 2020 19:23:19 +0000 (15:23 -0400)]
docs: replace "stable storage" with "durable" in descriptions
For PG, "durable storage" has a clear meaning, while "stable storage"
does not, so use the former.
Discussion: https://postgr.es/m/
20200817165222[email protected]
Backpatch-through: 9.5
Bruce Momjian [Mon, 31 Aug 2020 17:49:17 +0000 (13:49 -0400)]
doc: improve description of subscripting of arrays
It wasn't clear the non-integers are cast to integers for subscripting,
rather than throwing an error.
Reported-by: [email protected]
Discussion: https://postgr.es/m/
159538675800.624.
7728794628229799531@wrigleys.postgresql.org
Backpatch-through: 9.5
Bruce Momjian [Mon, 31 Aug 2020 17:43:04 +0000 (13:43 -0400)]
docs: improve 'capitals' inheritance example
Adds constraints and improves wording.
Reported-by: [email protected]
Discussion: https://postgr.es/m/
159586122762.680.
1361378513036616007@wrigleys.postgresql.org
Backpatch-through: 9.5
Bruce Momjian [Mon, 31 Aug 2020 17:20:04 +0000 (13:20 -0400)]
doc: clarify the useful features of procedures
This was not clearly documented when procedures were added in PG 11.
Reported-by: Robin Abbi
Discussion: https://postgr.es/m/CAGmg_NX327KKVuJmbWZD=pGutYFxzZjX1rU+3ji8UuX=8ONn9Q@mail.gmail.com
Backpatch-through: 11
Magnus Hagander [Mon, 31 Aug 2020 11:03:54 +0000 (13:03 +0200)]
Fix docs bug stating file_fdw requires absolute paths
It has always (since the first commit) worked with relative paths, so
use the same wording as other parts of the documentation.
Author: Bruce Momjian
Discussion: https://postgr.es/m/CABUevExx-hm=cit+A9LeKBH39srvk8Y2tEZeEAj5mP8YfzNKUg@mail.gmail.com
Tom Lane [Sun, 30 Aug 2020 20:03:19 +0000 (16:03 -0400)]
Mark factorial operator, and postfix operators in general, as deprecated.
Back-patch key parts of
4c5cf5431 and
6ca547cf7 into stable branches.
I didn't touch pg_description entries here, so it's purely a docs
change; and I didn't fool with any examples either. The main point
is so that anyone who's wondering if factorial() exists in the stable
branches will be reassured.
Mark Dilger and John Naylor, with some adjustments by me
Discussion: https://postgr.es/m/
BE2DF53D-251A-4E26-972F-
930E523580E9@enterprisedb.com
Tom Lane [Thu, 27 Aug 2020 21:36:13 +0000 (17:36 -0400)]
Fix code for re-finding scan position in a multicolumn GIN index.
collectMatchBitmap() needs to re-find the index tuple it was previously
looking at, after transiently dropping lock on the index page it's on.
The tuple should still exist and be at its prior position or somewhere
to the right of that, since ginvacuum never removes tuples but
concurrent insertions could add one. However, there was a thinko in
that logic, to the effect of expecting any inserted tuples to have the
same index "attnum" as what we'd been scanning. Since there's no
physical separation of tuples with different attnums, it's not terribly
hard to devise scenarios where this fails, leading to transient "lost
saved point in index" errors. (While I've duplicated this with manual
testing, it seems impossible to make a reproducible test case with our
available testing technology.)
Fix by just continuing the scan when the attnum doesn't match.
While here, improve the error message used if we do fail, so that it
matches the wording used in btree for a similar case.
collectMatchBitmap()'s posting-tree code path was previously not
exercised at all by our regression tests. While I can't make
a regression test that exhibits the bug, I can at least improve
the code coverage here, so do that. The test case I made for this
is an extension of one added by
4b754d6c1, so it only works in
HEAD and v13; didn't seem worth trying hard to back-patch it.
Per bug #16595 from Jesse Kinkead. This has been broken since
multicolumn capability was added to GIN (commit
27cb66fdf),
so back-patch to all supported branches.
Discussion: https://postgr.es/m/16595-
633118be8eef9ce2@postgresql.org
Bruce Momjian [Tue, 25 Aug 2020 13:53:12 +0000 (09:53 -0400)]
docs: client certificates are always sent to the server
They are not "requested" by the server.
Reported-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/
20200825.155320.
986648039251743210[email protected]
Backpatch-through: 9.5
Peter Eisentraut [Tue, 25 Aug 2020 05:29:05 +0000 (07:29 +0200)]
doc: Fix up title case
This fixes some instances that were missed in earlier processings and
that now look a bit strange because they are inconsistent with nearby
titles.
Tom Lane [Sat, 22 Aug 2020 18:46:40 +0000 (14:46 -0400)]
Avoid pushing quals down into sub-queries that have grouping sets.
The trouble with doing this is that an apparently-constant subquery
output column isn't really constant if it is a grouping column that
appears in only some of the grouping sets. A qual using such a
column would be subject to incorrect const-folding after push-down,
as seen in bug #16585 from Paul Sivash.
To fix, just disable qual pushdown altogether if the sub-query has
nonempty groupingSets. While we could imagine far less restrictive
solutions, there is not much point in working harder right now,
because subquery_planner() won't move HAVING clauses to WHERE within
such a subquery. If the qual stays in HAVING it's not going to be
a lot more useful than if we'd kept it at the outer level.
Having said that, this restriction could be removed if we used a
parsetree representation that distinguished such outputs from actual
constants, which is something I hope to do in future. Hence, make
the patch a minimal addition rather than integrating it more tightly
(e.g. by renumbering the existing items in subquery_is_pushdown_safe's
comment).
Back-patch to 9.5 where grouping sets were introduced.
Discussion: https://postgr.es/m/16585-
9d8c340d23ade8c1@postgresql.org
Bruce Momjian [Sat, 22 Aug 2020 00:23:09 +0000 (20:23 -0400)]
docs: improve description of how to handle multiple databases
This is a redesign of the intro to the managing databases chapter.
Discussion: https://postgr.es/m/
159586122762.680.
1361378513036616007@wrigleys.postgresql.org
Author: David G. Johnston
Backpatch-through: 9.5
Bruce Momjian [Fri, 21 Aug 2020 22:29:37 +0000 (18:29 -0400)]
docs: add COMMENT examples for new features, rename rtree
Reported-by: Jürgen Purtz
Discussion: https://postgr.es/m/
15ec5428-d46a-1725-f38d-
44986a977abb@purtz.de
Author: Jürgen Purtz
Backpatch-through: 11
Tom Lane [Fri, 21 Aug 2020 19:00:43 +0000 (15:00 -0400)]
Fix handling of CREATE TABLE LIKE with inheritance.
If a CREATE TABLE command uses both LIKE and traditional inheritance,
Vars in CHECK constraints and expression indexes that are absorbed
from a LIKE parent table tended to get mis-numbered, resulting in
wrong answers and/or bizarre error messages (though probably not any
actual crashes, thanks to validation occurring in the executor).
In v12 and up, the same could happen to Vars in GENERATED expressions,
even in cases with no LIKE clause but multiple traditional-inheritance
parents.
The cause of the problem for LIKE is that parse_utilcmd.c supposed
it could renumber such Vars correctly during transformCreateStmt(),
which it cannot since we have not yet accounted for columns added via
inheritance. Fix that by postponing processing of LIKE INCLUDING
CONSTRAINTS, DEFAULTS, GENERATED, INDEXES till after we've performed
DefineRelation().
The error with GENERATED and multiple inheritance is a simple oversight
in MergeAttributes(); it knows it has to renumber Vars in inherited
CHECK constraints, but forgot to apply the same processing to inherited
GENERATED expressions (a/k/a defaults).
Per bug #16272 from Tom Gottfried. The non-GENERATED variants of the
issue are ancient, presumably dating right back to the addition of
CREATE TABLE LIKE; hence back-patch to all supported branches.
Discussion: https://postgr.es/m/16272-
6e32da020e9a9381@postgresql.org
David Rowley [Thu, 20 Aug 2020 21:35:24 +0000 (09:35 +1200)]
Fix a few typos in JIT comments and README
Reviewed-by: Abhijit Menon-Sen
Reviewed-by: Andres Freund
Discussion: https://postgr.es/m/CAApHDvobgmCs6CohqhKTUf7D8vffoZXQTCBTERo9gbOeZmvLTw%40mail.gmail.com
Backpatch-through: 11, where JIT was added
Alvaro Herrera [Thu, 20 Aug 2020 17:49:04 +0000 (13:49 -0400)]
Revise REINDEX CONCURRENTLY recovery instructions
When the leftover invalid index is "ccold", there's no need to re-run
the command. Reword the instructions to make that explicit.
Backpatch to 12, where REINDEX CONCURRENTLY appeared.
Author: Álvaro Herrera
Reviewed-by: Michael Paquier
Reviewed-by: Julien Rouhaud
Discussion: https://postgr.es/m/20200819211312[email protected]
Tom Lane [Wed, 19 Aug 2020 18:07:49 +0000 (14:07 -0400)]
Suppress unnecessary RelabelType nodes in yet more cases.
Commit
a477bfc1d fixed eval_const_expressions() to ensure that it
didn't generate unnecessary RelabelType nodes, but I failed to notice
that some other places in the planner had the same issue. Really
noplace in the planner should be using plain makeRelabelType(), for
fear of generating expressions that should be equal() to semantically
equivalent trees, but aren't.
An example is that because canonicalize_ec_expression() failed
to be careful about this, we could end up with an equivalence class
containing both a plain Const, and a Const-with-RelabelType
representing exactly the same value. So far as I can tell this led to
no visible misbehavior, but we did waste a bunch of cycles generating
and evaluating "Const = Const-with-RelabelType" to prove such entries
are redundant.
Hence, move the support function added by
a477bfc1d to where it can
be more generally useful, and use it in the places where planner code
previously used makeRelabelType.
Back-patch to v12, like the previous patch. While I have no concrete
evidence of any real misbehavior here, it's certainly possible that
I overlooked a case where equivalent expressions that aren't equal()
could cause a user-visible problem. In any case carrying extra
RelabelType nodes through planning to execution isn't very desirable.
Discussion: https://postgr.es/m/
1311836.
1597781384@sss.pgh.pa.us
Heikki Linnakangas [Tue, 18 Aug 2020 10:13:09 +0000 (13:13 +0300)]
Avoid non-constant format string argument to fprintf().
As Tom Lane pointed out, it could defeat the compiler's printf() format
string verification.
Backpatch to v12, like that patch that introduced it.
Discussion: https://www.postgresql.org/message-id/
1069283.
1597672779%40sss.pgh.pa.us
Alvaro Herrera [Mon, 17 Aug 2020 20:20:06 +0000 (16:20 -0400)]
Disable autovacuum for BRIN test table
This should improve stability in the tests.
Per buildfarm member hyrax (CLOBBER_CACHE_ALWAYS) via Tom Lane.
Discussion: https://postgr.es/m/871534.
1597503261@sss.pgh.pa.us
Tom Lane [Mon, 17 Aug 2020 19:40:07 +0000 (15:40 -0400)]
Doc: fix description of UNION/CASE/etc type unification.
The description of what select_common_type() does was not terribly
accurate. Improve it.
David Johnston and Tom Lane
Discussion: https://postgr.es/m/
1019930.
1597613200@sss.pgh.pa.us
Heikki Linnakangas [Mon, 17 Aug 2020 06:27:29 +0000 (09:27 +0300)]
Fix printing last progress report line in client programs.
A number of client programs have a "--progress" option that when printing
to a TTY, updates the current line by printing a '\r' and overwriting it.
After the last line, '\n' needs to be printed to move the cursor to the
next line. pg_basebackup and pgbench got this right, but pg_rewind and
pg_checksums were slightly wrong. pg_rewind printed the newline to stdout
instead of stderr, and pg_checksums printed the newline even when not
printing to a TTY. Fix them, and also add a 'finished' argument to
pg_basebackup's progress_report() function, to keep it consistent with
the other programs.
Backpatch to v12. pg_rewind's newline was broken with the logging changes
in commit
cc8d415117 in v12, and pg_checksums was introduced in v12.
Discussion: https://www.postgresql.org/message-id/
82b539e5-ae33-34b0-1aee-
22b3379fd3eb@iki.fi
Michael Paquier [Mon, 17 Aug 2020 01:24:31 +0000 (10:24 +0900)]
doc: Fix description about bgwriter and checkpoint in HA section
Since
806a2ae, the work of the bgwriter is split the checkpointer, but a
portion of the documentation did not get the message.
Author: Masahiko Sawada
Discussion: https://postgr.es/m/CA+fd4k6jXxjAtjMVC=wG3=QGpauZBtcgN3Jhw+oV7zXGKVLKzQ@mail.gmail.com
Backpatch-through: 9.5