Alvaro Herrera [Mon, 2 Mar 2020 21:19:51 +0000 (18:19 -0300)]
Represent command completion tags as structs
The backend was using strings to represent command tags and doing string
comparisons in multiple places, but that's slow and unhelpful. Create a
new command list with a supporting structure to use instead; this is
stored in a tag-list-file that can be tailored to specific purposes with
a caller-definable C macro, similar to what we do for WAL resource
managers. The first first such uses are a new CommandTag enum and a
CommandTagBehavior struct.
Replace numerous occurrences of char *completionTag with a
QueryCompletion struct so that the code no longer stores information
about completed queries in a cstring. Only at the last moment, in
EndCommand(), does this get converted to a string.
EventTriggerCacheItem no longer holds an array of palloc’d tag strings
in sorted order, but rather just a Bitmapset over the CommandTags.
Author: Mark Dilger, with unsolicited help from Álvaro Herrera
Reviewed-by: John Naylor, Tom Lane
Discussion: https://postgr.es/m/
981A9DB4-3F0C-4DA5-88AD-
CB9CFF4D6CAD@enterprisedb.com
Tom Lane [Mon, 2 Mar 2020 19:35:22 +0000 (14:35 -0500)]
Blacklist port/win32_msvc/utime.h in cpluspluscheck and headerscheck.
Since commit
481c8e923 it tends to produce "error: sys/utime.h: No such
file or directory" on non-Windows platforms.
Peter Geoghegan [Mon, 2 Mar 2020 18:29:30 +0000 (10:29 -0800)]
Silence nbtree.h cpluspluscheck warning.
Add a cast to size_t to silence "comparison between signed and unsigned
integer expressions" cpluspluscheck warning.
Reported-By: Tom Lane
Discussion: https://postgr.es/m/7971.
1583171266@sss.pgh.pa.us
Peter Geoghegan [Mon, 2 Mar 2020 16:07:16 +0000 (08:07 -0800)]
Add assertions to _bt_update_posting().
Copy some assertions from _bt_form_posting() to its sibling function,
_bt_update_posting().
Discussion: https://postgr.es/m/CAH2-WzkPR8KMwkL0ap976kmXwBCeukTeHz6fB-U__wvuP1S9Zg@mail.gmail.com
Peter Eisentraut [Mon, 2 Mar 2020 14:21:42 +0000 (15:21 +0100)]
Update Microsoft documentation link
Reported-by: Juan José Santamaría Flecha <juanjo.santamaria@gmail.com>
Discussion: https://www.postgresql.org/message-id/CAC%2BAXB1EDXiRPmiVfh%2BWX79x5vXJDU17k0GkDjfyPgOWO4Y5og%40mail.gmail.com
Peter Eisentraut [Mon, 2 Mar 2020 07:55:31 +0000 (08:55 +0100)]
Remove long unused code behind a #if 0
Author: Vignesh C <vignesh21@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CALDaNm3sn4yOq-4rogb-CfE0EYw6b3mVzz8+DnS9BNRwPnhngw@mail.gmail.com
Michael Paquier [Mon, 2 Mar 2020 06:45:34 +0000 (15:45 +0900)]
Fix command-line colorization on Windows with VT100-compatible environments
When setting PG_COLOR to "always" or "auto" in a Windows terminal
VT100-compatible, the colorization output was not showing up correctly
because it is necessary to update the console's output handling mode.
This fix allows to detect automatically if the environment is compatible
with VT100. Hence, PG_COLOR=auto is able to detect and handle both
compatible and non-compatible environments. The behavior of
PG_COLOR=always remains unchanged, as it enforces the use of colorized
output even if the environment does not allow it.
This fix is based on an initial suggestion from Thomas Munro.
Reported-by: Haiying Tang
Author: Juan José Santamaría Flecha
Reviewed-by: Michail Nikolaev, Michael Paquier, Haiying Tang
Discussion: https://postgr.es/m/16108-
134692e97146b7bc@postgresql.org
Backpatch-through: 12
Michael Paquier [Mon, 2 Mar 2020 01:00:37 +0000 (10:00 +0900)]
Handle logical decoding in multi-insert for catalog tuples
The code path for multi-insert decoding is not stressed yet for
catalogs (a future patch may introduce this capability), so no
back-patch is needed.
Author: Daniel Gustafsson
Discussion: https://postgr.es/m/
9690D72F-5C4F-4016-9572-
6D16684E1D87@yesql.se
Peter Geoghegan [Sun, 1 Mar 2020 20:11:26 +0000 (12:11 -0800)]
Remove dead code from _bt_update_posting().
Discussion: https://postgr.es/m/CAH2-WzmAufHiOku6AGiFD=81VQs5nYJ1L2YkhW1t+BH4CMsgRw@mail.gmail.com
Dean Rasheed [Sun, 1 Mar 2020 14:49:25 +0000 (14:49 +0000)]
Fix corner-case loss of precision in numeric ln().
When deciding on the local rscale to use for the Taylor series
expansion, ln_var() neglected to account for the fact that the result
is subsequently multiplied by a factor of 2^(nsqrt+1), where nsqrt is
the number of square root operations performed in the range reduction
step, which can be as high as 22 for very large inputs. This could
result in a loss of precision, particularly when combined with large
rscale values, for which a large number of Taylor series terms is
required (up to around 400).
Fix by computing a few extra digits in the Taylor series, based on the
weight of the multiplicative factor log10(2^(nsqrt+1)). It remains to
be proven whether or not the other 8 extra digits used for the Taylor
series is appropriate, but this at least deals with the obvious
oversight of failing to account for the effects of the final
multiplication.
Per report from Justin AnyhowStep. Reviewed by Tom Lane.
Discussion: https://postgr.es/m/16280-
279f299d9c06e56f@postgresql.org
Peter Geoghegan [Sat, 29 Feb 2020 23:10:13 +0000 (15:10 -0800)]
Doc: Fix pageinspect bt_page_items() example.
Oversight in commit
93ee38ea.
Peter Geoghegan [Sat, 29 Feb 2020 20:10:17 +0000 (12:10 -0800)]
Teach pageinspect about nbtree deduplication.
Add a new bt_metap() column to display the metapage's allequalimage
field. Also add three new columns to contrib/pageinspect's
bt_page_items() function:
* Add a boolean column ("dead") that displays the LP_DEAD bit value for
each non-pivot tuple.
* Add a TID column ("htid") that displays a single heap TID value for
each tuple. This is the TID that is returned by BTreeTupleGetHeapTID(),
so comparable values are shown for pivot tuples, plain non-pivot tuples,
and posting list tuples.
* Add a TID array column ("tids") that displays TIDs from each tuple's
posting list, if any. This works just like the "tids" column from
pageinspect's gin_leafpage_items() function.
No version bump for the pageinspect extension, since there hasn't been a
stable Postgres release since the last version bump (the last bump was
part of commit
58b4cb30).
Author: Peter Geoghegan
Discussion: https://postgr.es/m/CAH2-WzmSMmU2eNvY9+a4MNP+z02h6sa-uxZvN3un6jY02ZVBSw@mail.gmail.com
Tom Lane [Sat, 29 Feb 2020 18:48:09 +0000 (13:48 -0500)]
Correctly re-use hash tables in buildSubPlanHash().
Commit
356687bd8 omitted to remove leftover code for destroying
a hashed subplan's hash tables, with the result that the tables
were always rebuilt not reused; this leads to severe memory
leakage if a hashed subplan is re-executed enough times.
Moreover, the code for reusing the hashnulls table had a typo
that would have made it do the wrong thing if it were reached.
Looking at the code coverage report shows severe under-coverage
of the potential callers of ResetTupleHashTable, so add some test
cases that exercise them.
Andreas Karlsson and Tom Lane, per reports from Ranier Vilela
and Justin Pryzby.
Backpatch to v11, as the faulty commit was.
Discussion: https://postgr.es/m/
edb62547-c453-c35b-3ed6-
a069e4d6b937@proxel.se
Discussion: https://postgr.es/m/CAEudQAo=DCebm1RXtig9OH+QivpS97sMkikt0A9qHmMUs+g6ZA@mail.gmail.com
Discussion: https://postgr.es/m/
20200210032547.GA1412@telsasoft.com
Tom Lane [Sat, 29 Feb 2020 18:23:12 +0000 (13:23 -0500)]
Remove obsolete comment.
Noted while studying subplan hash issue.
Tom Lane [Sat, 29 Feb 2020 01:28:34 +0000 (20:28 -0500)]
Avoid failure if autovacuum tries to access a just-dropped temp namespace.
Such an access became possible when commit
246a6c8f7 added more
aggressive cleanup of orphaned temp relations by autovacuum.
Since autovacuum's snapshot might be slightly stale, it could
attempt to access an already-dropped temp namespace, resulting in
an assertion failure or null-pointer dereference. (In practice,
since we don't drop temp namespaces automatically but merely
recycle them, this situation could only arise if a superuser does
a manual drop of a temp namespace. Still, that should be allowed.)
The core of the bug, IMO, is that isTempNamespaceInUse and its callers
failed to think hard about whether to treat "temp namespace isn't there"
differently from "temp namespace isn't in use". In hopes of forestalling
future mistakes of the same ilk, replace that function with a new one
checkTempNamespaceStatus, which makes the same tests but returns a
three-way enum rather than just a bool. isTempNamespaceInUse is gone
entirely in HEAD; but just in case some external code is relying on it,
keep it in the back branches, as a bug-compatible wrapper around the
new function.
Per report originally from Prabhat Kumar Sahu, investigated by Mahendra
Singh and Michael Paquier; the final form of the patch is my fault.
This replaces the failed fix attempt in
a052f6cbb.
Backpatch as far as v11, as
246a6c8f7 was.
Discussion: https://postgr.es/m/CAKYtNAr9Zq=1-ww4etHo-VCC-k120YxZy5OS01VkaLPaDbv2tg@mail.gmail.com
Jeff Davis [Fri, 28 Feb 2020 17:32:35 +0000 (09:32 -0800)]
Fix commit
c11cb17d.
I neglected to update copyfuncs/outfuncs/readfuncs.
Discussion: https://postgr.es/m/12491.
1582833409%40sss.pgh.pa.us
Tom Lane [Fri, 28 Feb 2020 16:29:58 +0000 (11:29 -0500)]
Doc: correct thinko in pg_buffercache documentation.
Access to this module is granted to the pg_monitor role, not
pg_read_all_stats. (Given the view's performance impact,
it seems wise to be restrictive, so I think this was the
correct decision --- and anyway it was clearly intentional.)
Per bug #16279 from Philip Semanchuk.
Discussion: https://postgr.es/m/16279-
fcaac33c68aab0ab@postgresql.org
Alvaro Herrera [Fri, 28 Feb 2020 16:13:54 +0000 (13:13 -0300)]
Add comments on avoid reuse of parse-time snapshot
Apparently, reusing the parse-time query snapshot for later steps
(execution) is a frequently considered optimization ... but it doesn't
work, for reasons discovered in thread [1]. Adding some comments about
why it doesn't really work can relieve some future hackers from wasting
time reimplementing it again.
[1] https://postgr.es/m/flat/
5075D8DF.
6050500@fuzzy.cz
Author: Michail Nikolaev
Discussion: https://postgr.es/m/CANtu0ogp6cTvMJObXP8n=k+JtqxY1iT9UV5MbGCpjjPa5crCiw@mail.gmail.com
Peter Eisentraut [Fri, 28 Feb 2020 07:54:49 +0000 (08:54 +0100)]
Add PostgreSQL home page to --help output
Per emerging standard in GNU programs and elsewhere. Autoconf already
has support for specifying a home page, so we can just that.
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://www.postgresql.org/message-id/flat/
8d389c5f-7fb5-8e48-9a4a-
68cec44786fa%402ndquadrant.com
Peter Eisentraut [Fri, 28 Feb 2020 07:54:49 +0000 (08:54 +0100)]
Refer to bug report address by symbol rather than hardcoding
Use the PACKAGE_BUGREPORT macro that is created by Autoconf for
referring to the bug reporting address rather than hardcoding it
everywhere. This makes it easier to change the address and it reduces
translation work.
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://www.postgresql.org/message-id/flat/
8d389c5f-7fb5-8e48-9a4a-
68cec44786fa%402ndquadrant.com
Alvaro Herrera [Thu, 27 Feb 2020 20:25:47 +0000 (17:25 -0300)]
Catversion bump for
b9b408c48724
Per Tom Lane.
Jeff Davis [Thu, 27 Feb 2020 18:46:58 +0000 (10:46 -0800)]
Save calculated transitionSpace in Agg node.
This will be useful in the upcoming Hash Aggregation work to improve
estimates for hash table sizing.
Discussion: https://postgr.es/m/
37091115219dd522fd9ed67333ee8ed1b7e09443.camel%40j-davis.com
Peter Geoghegan [Thu, 27 Feb 2020 17:32:34 +0000 (09:32 -0800)]
Doc: Fix deduplicate_items index term.
Reported-By: Fujii Masao
Discussion: https://postgr.es/m/
18f07ae8-7d89-537c-b0a9-
54100a1b46da@oss.nttdata.com
Alvaro Herrera [Thu, 27 Feb 2020 16:23:33 +0000 (13:23 -0300)]
Record parents of triggers
This let us get rid of a recently introduced ugly hack (commit
1fa846f1c9af).
Author: Álvaro Herrera
Reviewed-by: Amit Langote, Tom Lane
Discussion: https://postgr.es/m/
20200217215641.GA29784@alvherre.pgsql
Michael Paquier [Thu, 27 Feb 2020 12:58:37 +0000 (21:58 +0900)]
Remove TAP test for createdb --lc-ctype
OpenBSD falls back to "C" when using an incorrect input with setlocale()
and LC_CTYPE, causing this test, introduced by
008cf04, to fail. This
removes the culprit test to avoid the portability issue.
Per report from Robert Haas, via buildfarm member curculio.
Discussion: https://postgr.es/m/CA+TgmoZ6ddh3mHD9gU8DvNYoFmuJaYYn1+4AvZNp25vTdRwCAQ@mail.gmail.com
Backpatch-through: 11
Michael Paquier [Thu, 27 Feb 2020 06:31:27 +0000 (15:31 +0900)]
Skip foreign tablespaces when running pg_checksums/pg_verify_checksums
Attempting to use pg_checksums (pg_verify_checksums in 11) on a data
folder which includes tablespace paths used across multiple major
versions would cause pg_checksums to scan all directories present in
pg_tblspc, and not only marked with TABLESPACE_VERSION_DIRECTORY. This
could lead to failures when for example running sanity checks on an
upgraded instance with --check. Even worse, it was possible to rewrite
on-disk pages with --enable for a cluster potentially online.
This commit makes pg_checksums skip any directories not named
TABLESPACE_VERSION_DIRECTORY, similarly to what is done for base
backups.
Reported-by: Michael Banck
Author: Michael Banck, Bernd Helmle
Discussion: https://postgr.es/m/
62031974fd8e941dd8351fbc8c7eff60d59c5338.camel@credativ.de
backpatch-through: 11
Robert Haas [Thu, 27 Feb 2020 03:55:41 +0000 (09:25 +0530)]
Move src/backend/utils/hash/hashfn.c to src/common
This also involves renaming src/include/utils/hashutils.h, which
becomes src/include/common/hashfn.h. Perhaps an argument can be
made for keeping the hashutils.h name, but it seemed more
consistent to make it match the name of the file, and also more
descriptive of what is actually going on here.
Patch by me, reviewed by Suraj Kharage and Mark Dilger. Off-list
advice on how not to break the Windows build from Davinder Singh
and Amit Kapila.
Discussion: http://postgr.es/m/CA+TgmoaRiG4TXND8QuM6JXFRkM_1wL2ZNhzaUKsuec9-4yrkgw@mail.gmail.com
Michael Paquier [Thu, 27 Feb 2020 02:20:46 +0000 (11:20 +0900)]
createdb: Fix quoting of --encoding, --lc-ctype and --lc-collate
The original coding failed to properly quote those arguments, leading to
failures when using quotes in the values used. As the quoting can be
encoding-sensitive, the connection to the backend needs to be taken
before applying the correct quoting.
Author: Michael Paquier
Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/
20200214041004.GB1998@paquier.xyz
Backpatch-through: 9.5
Peter Geoghegan [Wed, 26 Feb 2020 23:15:45 +0000 (15:15 -0800)]
Silence another compiler warning in nbtinsert.c.
Per complaint from Álvaro Herrera.
Tom Lane [Wed, 26 Feb 2020 23:13:58 +0000 (18:13 -0500)]
Suppress unnecessary RelabelType nodes in more cases.
eval_const_expressions sometimes produced RelabelType nodes that
were useless because they just relabeled an expression to the same
exposed type it already had. This is worth avoiding because it can
cause two equivalent expressions to not be equal(), preventing
recognition of useful optimizations. In the test case added here,
an unpatched planner fails to notice that the "sqli = constant" clause
renders a sort step unnecessary, because one code path produces an
extra RelabelType and another doesn't.
Fix by ensuring that eval_const_expressions_mutator's T_RelabelType
case will not add in an unnecessary RelabelType. Also save some
code by sharing a subroutine with the effectively-equivalent cases
for CollateExpr and CoerceToDomain. (CollateExpr had no bug, and
I think that the case couldn't arise with CoerceToDomain, but
it seems prudent to do the same check for all three cases.)
Back-patch to v12. In principle this has been wrong all along,
but I haven't seen a case where it causes visible misbehavior
before v12, so refrain from changing stable branches unnecessarily.
Per investigation of a report from Eric Gillum.
Discussion: https://postgr.es/m/CAMmjdmvAZsUEskHYj=KT9sTukVVCiCSoe_PBKOXsncFeAUDPCQ@mail.gmail.com
Alvaro Herrera [Wed, 26 Feb 2020 22:57:14 +0000 (19:57 -0300)]
Fix docs regarding AFTER triggers on partitioned tables
In commit
86f575948c77 I forgot to update the trigger.sgml paragraph
that needs to explain that AFTER triggers are allowed in partitioned
tables. Do so now.
Discussion: https://postgr.es/m/
20200224185850.GA30899@alvherre.pgsql
Peter Geoghegan [Wed, 26 Feb 2020 21:17:36 +0000 (13:17 -0800)]
Silence compiler warning in nbtinsert.c.
Per buildfarm member longfin.
Peter Geoghegan [Wed, 26 Feb 2020 21:05:30 +0000 (13:05 -0800)]
Add deduplication to nbtree.
Deduplication reduces the storage overhead of duplicates in indexes that
use the standard nbtree index access method. The deduplication process
is applied lazily, after the point where opportunistic deletion of
LP_DEAD-marked index tuples occurs. Deduplication is only applied at
the point where a leaf page split would otherwise be required. New
posting list tuples are formed by merging together existing duplicate
tuples. The physical representation of the items on an nbtree leaf page
is made more space efficient by deduplication, but the logical contents
of the page are not changed. Even unique indexes make use of
deduplication as a way of controlling bloat from duplicates whose TIDs
point to different versions of the same logical table row.
The lazy approach taken by nbtree has significant advantages over a GIN
style eager approach. Most individual inserts of index tuples have
exactly the same overhead as before. The extra overhead of
deduplication is amortized across insertions, just like the overhead of
page splits. The key space of indexes works in the same way as it has
since commit
dd299df8 (the commit that made heap TID a tiebreaker
column).
Testing has shown that nbtree deduplication can generally make indexes
with about 10 or 15 tuples for each distinct key value about 2.5X - 4X
smaller, even with single column integer indexes (e.g., an index on a
referencing column that accompanies a foreign key). The final size of
single column nbtree indexes comes close to the final size of a similar
contrib/btree_gin index, at least in cases where GIN's posting list
compression isn't very effective. This can significantly improve
transaction throughput, and significantly reduce the cost of vacuuming
indexes.
A new index storage parameter (deduplicate_items) controls the use of
deduplication. The default setting is 'on', so all new B-Tree indexes
automatically use deduplication where possible. This decision will be
reviewed at the end of the Postgres 13 beta period.
There is a regression of approximately 2% of transaction throughput with
synthetic workloads that consist of append-only inserts into a table
with several non-unique indexes, where all indexes have few or no
repeated values. The underlying issue is that cycles are wasted on
unsuccessful attempts at deduplicating items in non-unique indexes.
There doesn't seem to be a way around it short of disabling
deduplication entirely. Note that deduplication of items in unique
indexes is fairly well targeted in general, which avoids the problem
there (we can use a special heuristic to trigger deduplication passes in
unique indexes, since we're specifically targeting "version bloat").
Bump XLOG_PAGE_MAGIC because xl_btree_vacuum changed.
No bump in BTREE_VERSION, since the representation of posting list
tuples works in a way that's backwards compatible with version 4 indexes
(i.e. indexes built on PostgreSQL 12). However, users must still
REINDEX a pg_upgrade'd index to use deduplication, regardless of the
Postgres version they've upgraded from. This is the only way to set the
new nbtree metapage flag indicating that deduplication is generally
safe.
Author: Anastasia Lubennikova, Peter Geoghegan
Reviewed-By: Peter Geoghegan, Heikki Linnakangas
Discussion:
https://postgr.es/m/
55E4051B.
7020209@postgrespro.ru
https://postgr.es/m/
4ab6e2db-bcee-f4cf-0916-
3a06e6ccbb55@postgrespro.ru
Peter Geoghegan [Wed, 26 Feb 2020 19:28:25 +0000 (11:28 -0800)]
Add equalimage B-Tree support functions.
Invent the concept of a B-Tree equalimage ("equality implies image
equality") support function, registered as support function 4. This
indicates whether it is safe (or not safe) to apply optimizations that
assume that any two datums considered equal by an operator class's order
method must be interchangeable without any loss of semantic information.
This is static information about an operator class and a collation.
Register an equalimage routine for almost all of the existing B-Tree
opclasses. We only need two trivial routines for all of the opclasses
that are included with the core distribution. There is one routine for
opclasses that index non-collatable types (which returns 'true'
unconditionally), plus another routine for collatable types (which
returns 'true' when the collation is a deterministic collation).
This patch is infrastructure for an upcoming patch that adds B-Tree
deduplication.
Author: Peter Geoghegan, Anastasia Lubennikova
Discussion: https://postgr.es/m/CAH2-Wzn3Ee49Gmxb7V1VJ3-AC8fWn-Fr8pfWQebHe8rYRxt5OQ@mail.gmail.com
Magnus Hagander [Wed, 26 Feb 2020 09:03:11 +0000 (10:03 +0100)]
Include error code in message from pg_upgrade
In passing, also quote the filename in one message where it wasn't.
Author: Dagfinn Ilmari Mannsåker
Discussion: https://postgr.es/m/87pne2w98h.fsf@wibble.ilmari.org
Michael Paquier [Tue, 25 Feb 2020 04:57:40 +0000 (13:57 +0900)]
Fix build failure on header generation with repetitive builds of MSVC
GenerateConfigHeader() in Solution.pm was complaining about unused
define symbols even if a newer config header was not generated, causing
successive build attempts with MSVC to fail.
Oversight in commit
8f4fb4c.
Author: Kyotaro Horiguchi
Reviewed-by: Juan José Santamaría Flecha
Discussion: https://postgr.es/m/
20200218.160500.
44393633318853097.horikyota.ntt@gmail.com
Tom Lane [Mon, 24 Feb 2020 23:43:23 +0000 (18:43 -0500)]
Fix compile failure.
I forgot that some compilers won't handle #if constructs within
ereport() calls. Duplicating most of the call is annoying but simple.
Per buildfarm.
Andres Freund [Mon, 24 Feb 2020 22:39:22 +0000 (14:39 -0800)]
expression eval: Reduce number of steps for agg transition invocations.
Do so by combining the various steps that are part of aggregate
transition function invocation into one larger step. As some of the
current steps are only necessary for some aggregates, have one variant
of the aggregate transition step for each possible combination.
To avoid further manual copies of code in the different transition
step implementations, move most of the code into helper functions
marked as "always inline".
The benefit of this change is an increase in performance when
aggregating lots of rows. This comes in part due to the reduced number
of indirect jumps due to the reduced number of steps, and in part by
reducing redundant setup code across steps. This mainly benefits
interpreted execution, but the code generated by JIT is also improved
a bit.
As a nice side-effect it also ends up making the code a bit simpler.
A small additional optimization is removing the need to set
aggstate->curaggcontext before calling ExecAggInitGroup, choosing to
instead passign curaggcontext as an argument. It was, in contrast to
other aggregate related functions, only needed to fetch a memory
context to copy the transition value into.
Author: Andres Freund
Discussion:
https://postgr.es/m/
20191023163849.sosqbfs5yenocez3@alap3.anarazel.de
https://postgr.es/m/
5c371df7cee903e8cd4c685f90c6c72086d3a2dc.camel@j-davis.com
Michael Paquier [Mon, 24 Feb 2020 22:55:22 +0000 (07:55 +0900)]
Issue properly WAL record for CID of first catalog tuple in multi-insert
Multi-insert for heap is not yet used actively for catalogs, but the
code to support this case is in place for logical decoding. The
existing code forgot to issue a XLOG_HEAP2_NEW_CID record for the first
tuple inserted, leading to failures when attempting to use multiple
inserts for catalogs at decoding time. This commit fixes the problem by
WAL-logging the needed CID.
This is not an active bug, so no back-patch is done.
Author: Daniel Gustafsson
Discussion: https://postgr.es/m/
E0D4CC67-A1CF-4DF4-991D-
B3AC2EB5FAE9@yesql.se
Tom Lane [Mon, 24 Feb 2020 22:28:33 +0000 (17:28 -0500)]
Account explicitly for long-lived FDs that are allocated outside fd.c.
The comments in fd.c have long claimed that all file allocations should
go through that module, but in reality that's not always practical.
fd.c doesn't supply APIs for invoking some FD-producing syscalls like
pipe() or epoll_create(); and the APIs it does supply for non-virtual
FDs are mostly insistent on releasing those FDs at transaction end;
and in some cases the actual open() call is in code that can't be made
to use fd.c, such as libpq.
This has led to a situation where, in a modern server, there are likely
to be seven or so long-lived FDs per backend process that are not known
to fd.c. Since NUM_RESERVED_FDS is only 10, that meant we had *very*
few spare FDs if max_files_per_process is >= the system ulimit and
fd.c had opened all the files it thought it safely could. The
contrib/postgres_fdw regression test, in particular, could easily be
made to fall over by running it under a restrictive ulimit.
To improve matters, invent functions Acquire/Reserve/ReleaseExternalFD
that allow outside callers to tell fd.c that they have or want to allocate
a FD that's not directly managed by fd.c. Add calls to track all the
fixed FDs in a standard backend session, so that we are honestly
guaranteeing that NUM_RESERVED_FDS FDs remain unused below the EMFILE
limit in a backend's idle state. The coding rules for these functions say
that there's no need to call them in code that just allocates one FD over
a fairly short interval; we can dip into NUM_RESERVED_FDS for such cases.
That means that there aren't all that many places where we need to worry.
But postgres_fdw and dblink must use this facility to account for
long-lived FDs consumed by libpq connections. There may be other places
where it's worth doing such accounting, too, but this seems like enough
to solve the immediate problem.
Internally to fd.c, "external" FDs are limited to max_safe_fds/3 FDs.
(Callers can choose to ignore this limit, but of course it's unwise
to do so except for fixed file allocations.) I also reduced the limit
on "allocated" files to max_safe_fds/3 FDs (it had been max_safe_fds/2).
Conceivably a smarter rule could be used here --- but in practice,
on reasonable systems, max_safe_fds should be large enough that this
isn't much of an issue, so KISS for now. To avoid possible regression
in the number of external or allocated files that can be opened,
increase FD_MINFREE and the lower limit on max_files_per_process a
little bit; we now insist that the effective "ulimit -n" be at least 64.
This seems like pretty clearly a bug fix, but in view of the lack of
field complaints, I'll refrain from risking a back-patch.
Discussion: https://postgr.es/m/E1izCmM-0005pV-Co@gemulon.postgresql.org
Peter Eisentraut [Mon, 24 Feb 2020 15:32:34 +0000 (16:32 +0100)]
Change client-side fsync_fname() to report errors fatally
Given all we have learned about fsync() error handling in the last few
years, reporting an fsync() error non-fatally is not useful,
unless you don't care much about the file, in which case you probably
don't need to use fsync() in the first place.
Change fsync_fname() and durable_rename() to exit(1) on fsync() errors
other than those that we specifically chose to ignore.
This affects initdb, pg_basebackup, pg_checksums, pg_dump, pg_dumpall,
and pg_rewind.
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/flat/
d239d1bd-aef0-ca7c-dc0a-
da14bdcf0392%402ndquadrant.com
Robert Haas [Mon, 24 Feb 2020 11:57:15 +0000 (17:27 +0530)]
Adapt hashfn.c and hashutils.h for frontend use.
hash_any() and its various variants are defined to return Datum,
which is a backend-only concept, but the underlying functions
actually want to return uint32 and uint64, and only return Datum
because it's convenient for callers who are using them to
implement a hash function for some SQL datatype.
However, changing these functions to return uint32 and uint64
seems like it might lead to programming errors or back-patching
difficulties, both because they are widely used and because
failure to use UInt{32,64}GetDatum() might not provoke a
compilation error. Instead, rename the existing functions as
well as changing the return type, and add static inline wrappers
for those callers that need the previous behavior.
Although this commit adapts hashutils.h and hashfn.c so that they
can be compiled as frontend code, it does not actually do
anything that would cause them to be so compiled. That is left
for another commit.
Patch by me, reviewed by Suraj Kharage and Mark Dilger.
Discussion: http://postgr.es/m/CA+TgmoaRiG4TXND8QuM6JXFRkM_1wL2ZNhzaUKsuec9-4yrkgw@mail.gmail.com
Robert Haas [Mon, 24 Feb 2020 11:52:45 +0000 (17:22 +0530)]
Put all the prototypes for hashfn.c into the same header file.
Previously, some of the prototypes for functions in hashfn.c were
in utils/hashutils.h and others were in utils/hsearch.h, but that
is confusing and has no particular benefit.
Patch by me, reviewed by Suraj Kharage and Mark Dilger.
Discussion: http://postgr.es/m/CA+TgmoaRiG4TXND8QuM6JXFRkM_1wL2ZNhzaUKsuec9-4yrkgw@mail.gmail.com
Robert Haas [Mon, 24 Feb 2020 11:47:08 +0000 (17:17 +0530)]
Move bitmap_hash and bitmap_match to bitmapset.c.
The closely-related function bms_hash_value is already defined in that
file, and this change means that hashfn.c no longer needs to depend on
nodes/bitmapset.h. That gets us closer to allowing use of the hash
functions in hashfn.c in frontend code.
Patch by me, reviewed by Suraj Kharage and Mark Dilger.
Discussion: http://postgr.es/m/CA+TgmoaRiG4TXND8QuM6JXFRkM_1wL2ZNhzaUKsuec9-4yrkgw@mail.gmail.com
Michael Paquier [Mon, 24 Feb 2020 09:13:25 +0000 (18:13 +0900)]
Add prefix checks in exclude lists for pg_rewind, pg_checksums and base backups
An instance of PostgreSQL crashing with a bad timing could leave behind
temporary pg_internal.init files, potentially causing failures when
verifying checksums. As the same exclusion lists are used between
pg_rewind, pg_checksums and basebackup.c, all those tools are extended
with prefix checks to keep everything in sync, with dedicated checks
added for pg_internal.init.
Backpatch down to 11, where pg_checksums (pg_verify_checksums in 11) and
checksum verification for base backups have been introduced.
Reported-by: Michael Banck
Author: Michael Paquier
Reviewed-by: Kyotaro Horiguchi, David Steele
Discussion: https://postgr.es/m/
62031974fd8e941dd8351fbc8c7eff60d59c5338.camel@credativ.de
Backpatch-through: 11
Peter Eisentraut [Mon, 17 Feb 2020 16:58:02 +0000 (17:58 +0100)]
Factor out InitControlFile() from BootStrapXLOG()
Right now this only makes BootStrapXLOG() a bit more manageable, but
in the future there may be external callers.
Discussion: https://www.postgresql.org/message-id/
e8f86ba5-48f1-a80a-7f1d-
b76bcb9c5c47@2ndquadrant.com
Peter Eisentraut [Mon, 17 Feb 2020 16:46:37 +0000 (17:46 +0100)]
Reformat code comment
Discussion: https://www.postgresql.org/message-id/
e8f86ba5-48f1-a80a-7f1d-
b76bcb9c5c47@2ndquadrant.com
Peter Eisentraut [Mon, 17 Feb 2020 16:35:48 +0000 (17:35 +0100)]
pg_resetwal: Rename function to avoid potential conflict
ReadControlFile() here conflicts with a function of the same name in
xlog.c. There is no actual conflict right now, but since
pg_resetwal.c reaches deep inside backend headers, it's possible in
the future.
Discussion: https://www.postgresql.org/message-id/
e8f86ba5-48f1-a80a-7f1d-
b76bcb9c5c47@2ndquadrant.com
Tom Lane [Fri, 21 Feb 2020 21:14:09 +0000 (16:14 -0500)]
Adjust Solution.pm to set HAVE_STDINT_H.
We're not testing that anywhere anymore, but for consistency,
it should get defined.
Peter Eisentraut [Fri, 21 Feb 2020 21:03:05 +0000 (22:03 +0100)]
Fix perlcritic warnings
Peter Eisentraut [Fri, 21 Feb 2020 19:50:56 +0000 (20:50 +0100)]
Allow running src/tools/msvc/mkvcbuild.pl under not Windows
This to allow verifying the MSVC build file generation without having
to have Windows.
To do this, we avoid Windows-specific Perl modules and don't run the
"cl" compiler or "nmake". The resulting build files won't actually be
completely correct, but it's useful enough.
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Julien Rouhaud <rjuju123@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/
d73b2c7b-f081-8357-8422-
7564d55f1aac%402ndquadrant.com
Tom Lane [Fri, 21 Feb 2020 19:30:21 +0000 (14:30 -0500)]
Assume that we have signed integral types and flexible array members.
These compiler features are required by C99, so remove the configure
probes for them.
This is part of a series of commits to get rid of no-longer-relevant
configure checks and dead src/port/ code. I'm committing them separately
to make it easier to back out individual changes if they prove less
portable than I expect.
Discussion: https://postgr.es/m/15379.
1582221614@sss.pgh.pa.us
Tom Lane [Fri, 21 Feb 2020 19:11:39 +0000 (14:11 -0500)]
Assume that we have <wchar.h>.
Windows has this, and so do all other live platforms according to the
buildfarm; it's been required by POSIX since SUSv2. So remove the
configure probe and tests of HAVE_WCHAR_H.
This is part of a series of commits to get rid of no-longer-relevant
configure checks and dead src/port/ code. I'm committing them separately
to make it easier to back out individual changes if they prove less
portable than I expect.
Discussion: https://postgr.es/m/15379.
1582221614@sss.pgh.pa.us
Tom Lane [Fri, 21 Feb 2020 19:04:19 +0000 (14:04 -0500)]
Assume that we have utime() and <utime.h>.
These are required by POSIX since SUSv2, and no live platforms fail
to provide them. On Windows, utime() exists and we bring our own
<utime.h>, so we're good there too. So remove the configure probes
and ad-hoc substitute code. We don't need to check for utimes()
anymore either, since that was only used as a substitute.
In passing, make the Windows build include <sys/utime.h> only where
we need it, not everywhere.
This is part of a series of commits to get rid of no-longer-relevant
configure checks and dead src/port/ code. I'm committing them separately
to make it easier to back out individual changes if they prove less
portable than I expect.
Discussion: https://postgr.es/m/15379.
1582221614@sss.pgh.pa.us
Tom Lane [Fri, 21 Feb 2020 18:47:40 +0000 (13:47 -0500)]
Assume that we have rint().
Windows has this since _MSC_VER >= 1200, and so do all other live
platforms according to the buildfarm, so remove the configure probe
and src/port/ substitution.
This is part of a series of commits to get rid of no-longer-relevant
configure checks and dead src/port/ code. I'm committing them separately
to make it easier to back out individual changes if they prove less
portable than I expect.
Discussion: https://postgr.es/m/15379.
1582221614@sss.pgh.pa.us
Tom Lane [Fri, 21 Feb 2020 18:42:28 +0000 (13:42 -0500)]
Assume that we have memmove().
Windows has this, and so do all other live platforms according to the
buildfarm, so remove the configure probe and c.h's substitute code.
This is part of a series of commits to get rid of no-longer-relevant
configure checks and dead src/port/ code. I'm committing them separately
to make it easier to back out individual changes if they prove less
portable than I expect.
Discussion: https://postgr.es/m/15379.
1582221614@sss.pgh.pa.us
Tom Lane [Fri, 21 Feb 2020 18:24:21 +0000 (13:24 -0500)]
Assume that we have cbrt().
Windows has this, and so do all other live platforms according to the
buildfarm, so remove the configure probe and float.c's substitute code.
This is part of a series of commits to get rid of no-longer-relevant
configure checks and dead src/port/ code. I'm committing them separately
to make it easier to back out individual changes if they prove less
portable than I expect.
Discussion: https://postgr.es/m/15379.
1582221614@sss.pgh.pa.us
Tom Lane [Fri, 21 Feb 2020 18:18:27 +0000 (13:18 -0500)]
Assume that we have isinf().
Windows has this, and so do all other live platforms according to the
buildfarm, so remove the configure probe and src/port/ substitution.
This also lets us get rid of some configure probes that existed only
to support src/port/isinf.c. I kept the port.h hack to force using
__builtin_isinf() on clang, though.
This is part of a series of commits to get rid of no-longer-relevant
configure checks and dead src/port/ code. I'm committing them separately
to make it easier to back out individual changes if they prove less
portable than I expect.
Discussion: https://postgr.es/m/15379.
1582221614@sss.pgh.pa.us
Tom Lane [Fri, 21 Feb 2020 17:49:42 +0000 (12:49 -0500)]
Assume that we have functional, 64-bit fseeko()/ftello().
Windows has this, and so do all other live platforms according to the
buildfarm, so remove the configure probe and src/port/ substitution.
Keep the probe that detects whether _LARGEFILE_SOURCE has to be
defined to get that, though ... that seems to be still relevant in
some places.
This is part of a series of commits to get rid of no-longer-relevant
configure checks and dead src/port/ code. I'm committing them separately
to make it easier to back out individual changes if they prove less
portable than I expect.
Discussion: https://postgr.es/m/15379.
1582221614@sss.pgh.pa.us
Peter Eisentraut [Fri, 21 Feb 2020 18:49:44 +0000 (19:49 +0100)]
Fix compiler warnings on 64-bit Windows
GCC reports various instances of
warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
and MSVC equivalently
warning C4312: 'type cast': conversion from 'int' to 'void *' of greater size
warning C4311: 'type cast': pointer truncation from 'void *' to 'long'
in ECPG test files. This is because void* and long are cast back and
forth, but on 64-bit Windows, these have different sizes. Fix by
using intptr_t instead.
The code actually worked fine because the integer values in use are
all small. So this is just to get the test code to compile warning-free.
This change is simplified by having made stdint.h required (commit
957338418b69e9774ccc1bab59f765a62f0aa6f9). Before this it would have
been more complicated because the ecpg test source files don't use the
full pg_config.h.
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/
5d398bbb-262a-5fed-d839-
d0e5cff3c0d7%402ndquadrant.com
Jeff Davis [Fri, 21 Feb 2020 01:04:19 +0000 (17:04 -0800)]
Fixup for nodeAgg.c refactor.
Commit
5b618e1f made an unintended behavior change.
Etsuro Fujita [Fri, 21 Feb 2020 11:00:45 +0000 (20:00 +0900)]
Avoid redundant checks in partition_bounds_copy().
Previously, partition_bounds_copy() checked whether the strategy for the
given partition bounds was hash or not, and then determined the number of
elements in the datums in the datums array for the partition bounds, on
each iteration of the loop for copying the datums array, but there is no
need to do that. Perform the checks only once before the loop iteration.
Author: Etsuro Fujita
Reported-by: Amit Langote and Julien Rouhaud
Discussion: https://postgr.es/m/CAPmGK14Rvxrm8DHWvCjdoks6nwZuHBPvMnWZ6rkEx2KhFeEoPQ@mail.gmail.com
Peter Eisentraut [Fri, 21 Feb 2020 08:14:03 +0000 (09:14 +0100)]
Require stdint.h
stdint.h belongs to the compiler (as opposed to inttypes.h), so by
requiring a C99 compiler we can also require stdint.h
unconditionally. Remove configure checks and other workarounds for
it.
This also removes a few steps in the required portability adjustments
to the imported time zone code, which can be applied on the next
import.
When using GCC on a platform that is otherwise pre-C99, this will now
require at least GCC 4.5, which is the first release that supplied a
standard-conforming stdint.h if the native platform didn't have it.
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/
5d398bbb-262a-5fed-d839-
d0e5cff3c0d7%402ndquadrant.com
Michael Paquier [Fri, 21 Feb 2020 03:05:29 +0000 (12:05 +0900)]
Doc: Fix instructions to control build environment with MSVC
The documentation included some outdated instructions to change the
architecture, build type or target OS of a build done with MSVC. This
commit updates the documentation to include the modern options
available, down to Visual Studio 2013.
Reported-by: Kyotaro Horiguchi
Author: Juan José Santamaría Flecha
Discussion: https://postgr.es/m/CAC+AXB0J7tAqW_2F1fCE4Dh2=Ccz96TcLpsGXOCvka7VvWG9Qw@mail.gmail.com
Backpatch-through: 12
Alvaro Herrera [Thu, 20 Feb 2020 17:14:20 +0000 (14:14 -0300)]
Simplify FK-to-partitioned regression test query
Avoid a join between relations having the FK to detect FK violation.
The planner might optimize this considering the PK must exist on the
referenced side at some point, effectively masking a bug this test
tries to detect.
Tom Lane and Jehan-Guillaume de Rorthais
Discussion: https://postgr.es/m/467.
1581270529@sss.pgh.pa.us
Etsuro Fujita [Thu, 20 Feb 2020 10:15:00 +0000 (19:15 +0900)]
Remove extra word from comment.
Michael Paquier [Thu, 20 Feb 2020 02:57:41 +0000 (11:57 +0900)]
Cleanup more code related to ws2_32.dll loading in src/port/getaddrinfo.c
e2e0219 has removed a code path for Windows 2000 that attempts to load
wship6.dll as fallback if ws2_32.dll is found but not getaddrinfo(),
leaving behind a dangling pointer as the library is freed. However,
there is no point in this check as ws2_32.dll exists since Windows XP,
so just remove the duplicated check.
Reported-by: Tom Lane
Discussion: https://postgr.es/m/9781.
1582146114@sss.pgh.pa.us
Tom Lane [Wed, 19 Feb 2020 23:52:18 +0000 (18:52 -0500)]
Doc: discourage use of partial indexes for poor-man's-partitioning.
Creating a bunch of non-overlapping partial indexes is generally
a bad idea, so add an example saying not to do that.
Back-patch to v10. Before that, the alternative of using (real)
partitioning wasn't available, so that the tradeoff isn't quite
so clear cut.
Discussion: https://postgr.es/m/CAKVFrvFY-f7kgwMRMiPLbPYMmgjc8Y2jjUGK_Y0HVcYAmU6ymg@mail.gmail.com
Tom Lane [Wed, 19 Feb 2020 21:59:14 +0000 (16:59 -0500)]
Remove support for upgrading extensions from "unpackaged" state.
Andres Freund pointed out that allowing non-superusers to run
"CREATE EXTENSION ... FROM unpackaged" has security risks, since
the unpackaged-to-1.0 scripts don't try to verify that the existing
objects they're modifying are what they expect. Just attaching such
objects to an extension doesn't seem too dangerous, but some of them
do more than that.
We could have resolved this, perhaps, by still requiring superuser
privilege to use the FROM option. However, it's fair to ask just what
we're accomplishing by continuing to lug the unpackaged-to-1.0 scripts
forward. None of them have received any real testing since 9.1 days,
so they may not even work anymore (even assuming that one could still
load the previous "loose" object definitions into a v13 database).
And an installation that's trying to go from pre-9.1 to v13 or later
in one jump is going to have worse compatibility problems than whether
there's a trivial way to convert their contrib modules into extension
style.
Hence, let's just drop both those scripts and the core-code support
for "CREATE EXTENSION ... FROM".
Discussion: https://postgr.es/m/
20200213233015.r6rnubcvl4egdh5r@alap3.anarazel.de
Peter Eisentraut [Wed, 19 Feb 2020 19:52:42 +0000 (20:52 +0100)]
Fix typo
Reported-by: Daniel Verite <daniel@manitou-mail.org>
Tom Lane [Wed, 19 Feb 2020 19:44:58 +0000 (14:44 -0500)]
Fix confusion about event trigger vs. plain function in plpgsql.
The function hash table keys made by compute_function_hashkey() failed
to distinguish event-trigger call context from regular call context.
This meant that once we'd successfully made a hash entry for an event
trigger (either by validation, or by normal use as an event trigger),
an attempt to call the trigger function as a plain function would
find this hash entry and thereby bypass the you-can't-do-that check in
do_compile(). Thus we'd attempt to execute the function, leading to
strange errors or even crashes, depending on function contents and
server version.
To fix, add an isEventTrigger field to PLpgSQL_func_hashkey,
paralleling the longstanding infrastructure for regular triggers.
This fits into what had been pad space, so there's no risk of an ABI
break, even assuming that any third-party code is looking at these
hash keys. (I considered replacing isTrigger with a PLpgSQL_trigtype
enum field, but felt that that carried some API/ABI risk. Maybe we
should change it in HEAD though.)
Per bug #16266 from Alexander Lakhin. This has been broken since
event triggers were invented, so back-patch to all supported branches.
Discussion: https://postgr.es/m/16266-
fcd7f838e97ba5d4@postgresql.org
Peter Eisentraut [Wed, 19 Feb 2020 19:09:32 +0000 (20:09 +0100)]
Set gen_random_uuid() to volatile
It was set to immutable. This was a mistake in the initial
commit (
5925e5549890416bcf588334d9d0bc99f8ad6c7f).
Reported-by: hubert depesz lubaczewski <depesz@depesz.com>
Discussion: https://www.postgresql.org/message-id/flat/
20200218185452.GA8710%40depesz.com
Jeff Davis [Wed, 19 Feb 2020 18:15:16 +0000 (10:15 -0800)]
Minor refactor of nodeAgg.c.
* Separate calculation of hash value from the lookup.
* Split build_hash_table() into two functions.
* Change lookup_hash_entry() to return AggStatePerGroup. That's all
the caller needed, anyway.
These changes are to support the upcoming Disk-based Hash Aggregation
work.
Discussion: https://postgr.es/m/
31f5ab871a3ad5a1a91a7a797651f20e77ac7ce3.camel%40j-davis.com
Jeff Davis [Tue, 18 Feb 2020 20:31:24 +0000 (12:31 -0800)]
logtape.c: allocate read buffer even for an empty tape.
Prior to this commit, the read buffer was allocated at the time the tape
was rewound; but as an optimization, would not be allocated at all if
the tape was empty.
That optimization meant that it was valid to have a rewound tape with
the buffer set to NULL, but only if a number of conditions were met
and only if the API was used properly. After
7fdd919a refactored the
code to support lazily-allocating the buffer, Coverity started
complaining.
The optimization for empty tapes doesn't seem important, so just
allocate the buffer whether the tape has any data or not.
Discussion: https://postgr.es/m/20351.
1581868306%40sss.pgh.pa.us
Fujii Masao [Wed, 19 Feb 2020 11:37:26 +0000 (20:37 +0900)]
Fix mesurement of elapsed time during truncating heap in VACUUM.
VACUUM may truncate heap in several batches. The activity report
is logged for each batch, and contains the number of pages in the table
before and after the truncation, and also the elapsed time during
the truncation. Previously the elapsed time reported in each batch was
the total elapsed time since starting the truncation until finishing
each batch. For example, if the truncation was processed dividing into
three batches, the second batch reported the accumulated time elapsed
during both first and second batches. This is strange and confusing
because the number of pages in the table reported together is not
total. Instead, each batch should report the time elapsed during
only that batch.
The cause of this issue was that the resource usage snapshot was
initialized only at the beginning of the truncation and was never
reset later. This commit fixes the issue by changing VACUUM so that
the resource usage snapshot is reset at each batch.
Back-patch to all supported branches.
Reported-by: Tatsuhito Kasahara
Author: Tatsuhito Kasahara
Reviewed-by: Masahiko Sawada, Fujii Masao
Discussion: https://postgr.es/m/CAP0=ZVJsf=NvQuy+QXQZ7B=ZVLoDV_JzsVC1FRsF1G18i3zMGg@mail.gmail.com
Michael Paquier [Wed, 19 Feb 2020 04:20:33 +0000 (13:20 +0900)]
Clean up some code, comments and docs referring to Windows 2000 and older
This fixes and updates a couple of comments related to outdated Windows
versions. Particularly, src/common/exec.c had a fallback implementation
to read a file's line from a pipe because stdin/stdout/stderr does not
exist in Windows 2000 that is removed to simplify src/common/ as there
are unlikely versions of Postgres running on such platforms.
Author: Michael Paquier
Reviewed-by: Kyotaro Horiguchi, Juan José Santamaría Flecha
Discussion: https://postgr.es/m/
20191219021526.GC4202@paquier.xyz
Amit Kapila [Wed, 12 Feb 2020 05:53:24 +0000 (11:23 +0530)]
Stop demanding that top xact must be seen before subxact in decoding.
Manifested as
ERROR: subtransaction logged without previous top-level txn record
this check forbids legit behaviours like
- First xl_xact_assignment record is beyond reading, i.e. earlier
restart_lsn.
- After restart_lsn there is some change of a subxact.
- After that, there is second xl_xact_assignment (for another subxact)
revealing the relationship between top and first subxact.
Such a transaction won't be streamed anyway because we hadn't seen it in
full. Saying for sure whether xact of some record encountered after
the snapshot was deserialized can be streamed or not requires to know
whether it wrote something before deserialization point --if yes, it
hasn't been seen in full and can't be decoded. Snapshot doesn't have such
info, so there is no easy way to relax the check.
Reported-by: Hsu, John
Diagnosed-by: Arseny Sher
Author: Arseny Sher, Amit Kapila
Reviewed-by: Amit Kapila, Dilip Kumar
Backpatch-through: 9.5
Discussion: https://postgr.es/m/
AB5978B2-1772-4FEE-A245-
74C91704ECB0@amazon.com
Peter Geoghegan [Wed, 19 Feb 2020 00:07:16 +0000 (16:07 -0800)]
Remove obsolete _bt_compare() comment.
btbuild() has nothing to say about how NULL values compare in nbtree.
Besides, there are _bt_compare() header comments that describe how NULL
values are handled.
Fujii Masao [Tue, 18 Feb 2020 04:13:15 +0000 (13:13 +0900)]
Make inherited LOCK TABLE perform access permission checks on parent table only.
Previously, LOCK TABLE command through a parent table checked
the permissions on not only the parent table but also the children
tables inherited from it. This was a bug and inherited queries should
perform access permission checks on the parent table only. This
commit fixes LOCK TABLE so that it does not check the permission
on children tables.
This patch is applied only in the master branch. We decided not to
back-patch because it's not hard to imagine that there are some
applications expecting the old behavior and the change breaks their
security.
Author: Amit Langote
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/CAHGQGwE+GauyG7POtRfRwwthAGwTjPQYdFHR97+LzA4RHGnJxA@mail.gmail.com
Michael Paquier [Tue, 18 Feb 2020 03:20:55 +0000 (12:20 +0900)]
Remove duplicated words in comments
Author: Daniel Gustafsson
Reviewed-by: Vik Fearing
Discussion: https://postgr.es/m/
EBC3BFEB-664C-4063-81ED-
29F1227DB012@yesql.se
Michael Paquier [Tue, 18 Feb 2020 01:49:44 +0000 (10:49 +0900)]
Fix grammar in monitoring.sgml
This is related to progress reporting for ANALYZE and partitioned
tables.
Author: Amit Langote
Reviewed-by: Daniel Gustafsson, Julien Rouhaud
Discussion: https://postgr.es/m/CA+HiwqGx6C=-bFTX=ryMThyvM7CcSC3b1x8_6zh4Uo41Kvu-zw@mail.gmail.com
Tom Lane [Mon, 17 Feb 2020 23:40:02 +0000 (18:40 -0500)]
Teach pg_dump to dump comments on RLS policy objects.
This was unaccountably omitted in the original RLS patch.
The SQL syntax is basically the same as for comments on triggers,
so crib code from dumpTrigger().
Per report from Marc Munro. Back-patch to all supported branches.
Discussion: https://postgr.es/m/
1581889298.18009.15.camel@bloodnok.com
Peter Eisentraut [Mon, 17 Feb 2020 14:19:58 +0000 (15:19 +0100)]
Optimize update of tables with generated columns
When updating a table row with generated columns, only recompute those
generated columns whose base columns have changed in this update and
keep the rest unchanged. This can result in a significant performance
benefit. The required information was already kept in
RangeTblEntry.extraUpdatedCols; we just have to make use of it.
Reviewed-by: Pavel Stehule <pavel.stehule@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/
b05e781a-fa16-6b52-6738-
761181204567@2ndquadrant.com
Peter Eisentraut [Mon, 17 Feb 2020 14:19:58 +0000 (15:19 +0100)]
Fill in extraUpdatedCols in logical replication
The extraUpdatedCols field of the target RTE records which generated
columns are affected by an update. This is used in a variety of
places, including per-column triggers and foreign data wrappers. When
an update was initiated by a logical replication subscription, this
field was not filled in, so such an update would not affect generated
columns in a way that is consistent with normal updates. To fix,
factor out some code from analyze.c to fill in extraUpdatedCols in the
logical replication worker as well.
Reviewed-by: Pavel Stehule <pavel.stehule@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/
b05e781a-fa16-6b52-6738-
761181204567@2ndquadrant.com
Fujii Masao [Mon, 17 Feb 2020 07:16:08 +0000 (16:16 +0900)]
Add description about GSSOpenServer wait event into document.
This commit also updates wait event enum into alphabetical order.
Previously the enum entry for GSSOpenServer was added out-of-order.
Back-patch to v12 where commit
b0b39f72b9 introduced
GSSOpenServer wait event. In v12, the commit doesn't include
the update of wait event enum, not to break ABI.
Author: Fujii Masao
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/
949931aa-4ed4-d867-a7b5-
de9c02b2292b@oss.nttdata.com
Fujii Masao [Mon, 17 Feb 2020 06:33:32 +0000 (15:33 +0900)]
Add description about LogicalRewriteTruncate wait event into document.
Back-patch to v10 where commit
249cf070e3 introduced
LogicalRewriteTruncate wait event.
Author: Fujii Masao
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/
949931aa-4ed4-d867-a7b5-
de9c02b2292b@oss.nttdata.com
Tom Lane [Sun, 16 Feb 2020 17:20:18 +0000 (12:20 -0500)]
Try again to work around Windows' ERROR_SHARING_VIOLATION in pg_ctl.
Commit
0da33c762 introduced an unfortunate regression in pg_ctl on
Windows: if the log file specified with -l doesn't exist yet, and
pg_ctl is running with Administrator privileges, then the log file
might get created with permissions that prevent the postmaster from
writing on it. (It seems that whether this happens depends on whether
the log file is inside the user's home directory or not, and perhaps
on other phase-of-the-moon conditions, which may explain why we failed
to notice it sooner.)
To fix, just don't create the log file if it doesn't exist yet. The
case where we need to wait obviously only occurs with a pre-existing
log file.
In passing, switch from using fopen() to plain open(), saving a few
cycles.
Per bug #16259 from Jonathan Katz and Heath Lord. Back-patch to v12,
as the faulty commit was.
Alexander Lakhin
Discussion: https://postgr.es/m/16259-
c5ebed32a262a8b1@postgresql.org
Tom Lane [Sat, 15 Feb 2020 20:22:40 +0000 (15:22 -0500)]
Update obsolete comment.
Noted by Justin Pryzby, though I chose to just rip out the stale text,
as it's in no way relevant to this particular function.
Discussion: https://postgr.es/m/
20200212182337.GZ1412@telsasoft.com
Tom Lane [Sat, 15 Feb 2020 20:13:44 +0000 (15:13 -0500)]
Clarify coding in Catalog::AddDefaultValues.
Make it a bit shorter and better-commented; no functional change.
Alvaro Herrera and Tom Lane
Discussion: https://postgr.es/m/
20200212182337.GZ1412@telsasoft.com
Tom Lane [Sat, 15 Feb 2020 19:58:30 +0000 (14:58 -0500)]
Run "make reformat-dat-files".
Mostly to make sure the previous commit didn't break this.
Discussion: https://postgr.es/m/
20200212182337.GZ1412@telsasoft.com
Tom Lane [Sat, 15 Feb 2020 19:57:27 +0000 (14:57 -0500)]
Don't require pg_class.dat to contain correct relnatts values.
Practically everybody who's ever added a column to one of the bootstrap
catalogs has been burnt by the need to update the relnatts field in the
initial pg_class data to match. Now that we use Perl scripts to
generate postgres.bki, we can have the machines take care of that,
by filling the field during genbki.pl.
While at it, use the BKI_DEFAULTS mechanism to eliminate repetitive
specifications of other column values in pg_class.dat, too. They
weren't particularly a maintenance problem, but this way is prettier
(certainly the spotty previous usage of BKI_DEFAULTS wasn't pretty).
No catversion bump needed, since this doesn't actually change the
contents of postgres.bki.
Per gripe from Justin Pryzby, though this is quite different from
his originally proposed solution.
Amit Langote, John Naylor, Tom Lane
Discussion: https://postgr.es/m/
20200212182337.GZ1412@telsasoft.com
Peter Geoghegan [Sat, 15 Feb 2020 02:38:35 +0000 (18:38 -0800)]
Recreate website's formatting for "website" doc builds.
The stylesheets used for the HTML documentation rendered on
postgresql.org have shifted, and no longer matched what was expected by
"make STYLE=website html" builds performed locally. Local doc builds
did not reflect other aspects of the website, including font and
margins.
This patch updates the references to use the current set of stylesheets
that are used by the documentation on postgresql.org. This also wraps
the documentation preview in a HTML container so it can keep the content
within similar margins to those found on the website.
The documentation on building the docs is updated to reflect this
change, and to let the documentation builder know that an external
network connection is required to properly preview documentation built
with "make STYLE=website html" (which was true prior to this patch too,
but not mentioned).
Author: Jonathan Katz
Reported-By: Tom Lane
Discussion: https://postgr.es/m/1375.
1581446233@sss.pgh.pa.us
Tom Lane [Fri, 14 Feb 2020 16:20:07 +0000 (11:20 -0500)]
Remove pg_regress' --load-language option.
We haven't used this option since inventing extensions. As of commit
50fc694e4 it's actually formally equivalent to --load-extension, so
let's just drop it.
Discussion: https://postgr.es/m/6853.
1581627393@sss.pgh.pa.us
Michael Paquier [Fri, 14 Feb 2020 03:38:44 +0000 (12:38 +0900)]
Remove some dead code in contrib/adminpack/
Since its introduction in
fe59e56, the code in charge of validating and
converting a file path includes some extra handling for absolute paths
pointing to an external log_directory, but this has never been used.
Author: Antonin Houska
Reviewed-by: Julien Rouhaud, Michael Paquier
Discussion: https://postgr.es/m/32663.
1581592539@antos
Tom Lane [Thu, 13 Feb 2020 20:02:35 +0000 (15:02 -0500)]
Mark some contrib modules as "trusted".
This allows these modules to be installed into a database without
superuser privileges (assuming that the DBA or sysadmin has installed
the module's files in the expected place). You only need CREATE
privilege on the current database, which by default would be
available to the database owner.
The following modules are marked trusted:
btree_gin
btree_gist
citext
cube
dict_int
earthdistance
fuzzystrmatch
hstore
hstore_plperl
intarray
isn
jsonb_plperl
lo
ltree
pg_trgm
pgcrypto
seg
tablefunc
tcn
tsm_system_rows
tsm_system_time
unaccent
uuid-ossp
In the future we might mark some more modules trusted, but there
seems to be no debate about these, and on the whole it seems wise
to be conservative with use of this feature to start out with.
Discussion: https://postgr.es/m/32315.
1580326876@sss.pgh.pa.us
Jeff Davis [Thu, 13 Feb 2020 17:43:51 +0000 (09:43 -0800)]
Logical Tape Set: lazily allocate read buffer.
The write buffer was already lazily-allocated, so this is more
symmetric. It also means that a freshly-rewound tape (whether for
reading or writing) is not consuming memory for the buffer.
Discussion: https://postgr.es/m/
97c46a59c27f3c38e486ca170fcbc618d97ab049.camel%40j-davis.com
Tom Lane [Thu, 13 Feb 2020 18:37:43 +0000 (13:37 -0500)]
Avoid a performance regression in float overflow/underflow detection.
Commit
6bf0bc842 replaced float.c's CHECKFLOATVAL() macro with static
inline subroutines, but that wasn't too well thought out. In the original
coding, the unlikely condition (isinf(result) or result == 0) was checked
first, and the inf_is_valid or zero_is_valid condition only afterwards.
The inline-subroutine coding caused that to be swapped around, which is
pretty horrid for performance because (a) in common cases the is_valid
condition is twice as expensive to evaluate (e.g., requiring two isinf()
calls not one) and (b) in common cases the is_valid condition is false,
requiring us to perform the unlikely-condition check anyway. Net result
is that one isinf() call becomes two or three, resulting in visible
performance loss as reported by Keisuke Kuroda.
The original fix proposal was to revert the replacement of the macro,
but on second thought, that macro was just a bad idea from the beginning:
if anything it's a net negative for readability of the code. So instead,
let's just open-code all the overflow/underflow tests, being careful to
test the unlikely condition first (and mark it unlikely() to help the
compiler get the point).
Also, rather than having N copies of the actual ereport() calls, collapse
those into out-of-line error subroutines to save some code space. This
does mean that the error file/line numbers won't be very helpful for
figuring out where the issue really is --- but we'd already burned that
bridge by putting the ereports into static inlines.
In HEAD, check_float[48]_val() are gone altogether. In v12, leave them
present in float.h but unused in the core code, just in case some
extension is depending on them.
Emre Hasegeli, with some kibitzing from me and Andres Freund
Discussion: https://postgr.es/m/CANDwggLe1Gc1OrRqvPfGE=kM9K0FSfia0hbeFCEmwabhLz95AA@mail.gmail.com
Peter Geoghegan [Wed, 12 Feb 2020 22:08:34 +0000 (14:08 -0800)]
Doc: Restructure B-Tree support routine docs.
Use a top-level "variablelist", with one item per B-Tree support
function. This structure matches the structure used by various
"Extensibility" sections in other documentation chapters for other index
access methods.
An explicit list makes it much clearer where each item begins and ends.
This wasn't really a problem before now, but an upcoming patch that adds
deduplication to nbtree will need to have its own new B-Tree support
function. Ease the burden of translators by tidying up btree.sgml ahead
of committing the deduplication patch.
Tom Lane [Wed, 12 Feb 2020 19:33:49 +0000 (14:33 -0500)]
Remove long-dead comments.
These should've been dropped by
a8bb8eb58, but evidently were
missed. Not important enough to back-patch.
Tom Lane [Wed, 12 Feb 2020 19:13:13 +0000 (14:13 -0500)]
Doc: fix old oversights in GRANT/REVOKE documentation.
The GRANTED BY clause in GRANT/REVOKE ROLE has been there since 2005
but was never documented. I'm not sure now whether that was just an
oversight or was intentional (given the limited capability of the
option). But seeing that pg_dumpall does emit code that uses this
option, it seems like not documenting it at all is a bad idea.
Also, when we upgraded the syntax to allow CURRENT_USER/SESSION_USER
as the privilege recipient, the role form of GRANT was incorrectly
not modified to show that, and REVOKE's docs weren't touched at all.
Although I'm not that excited about GRANTED BY, the other oversight
seems serious enough to justify a back-patch.
Discussion: https://postgr.es/m/3070.
1581526786@sss.pgh.pa.us