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
Robert Haas [Thu, 24 Sep 2020 19:27:19 +0000 (15:27 -0400)]
Fix two bugs in MaintainOldSnapshotTimeMapping.
The previous coding was confused about whether head_timestamp was
intended to represent the timestamp for the newest bucket in the
mapping or the oldest timestamp for the oldest bucket in the mapping.
Decide that it's intended to be the oldest one, and repair
accordingly.
To do that, we need to do two things. First, when advancing to a
new bucket, don't categorically set head_timestamp to the new
timestamp. Do this only if we're blowing out the map completely
because a lot of time has passed since we last maintained it. If
we're replacing entries one by one, advance head_timestamp by
1 minute for each; if we're filling in unused entries, don't
advance head_timestamp at all.
Second, fix the computation of how many buckets we need to advance.
The previous formula would be correct if head_timestamp were the
timestamp for the new bucket, but we're now making all the code
agree that it's the timestamp for the oldest bucket, so adjust the
formula accordingly.
This is certainly a bug fix, but I don't feel good about
back-patching it without the introspection tools added by commit
aecf5ee2bb36c597d3c6142e367e38d67816c777, and perhaps also some
actual tests. Since back-patching the introspection tools might
not attract sufficient support and since there are no automated
tests of these fixes yet, I'm just committing this to master for
now.
Patch by me, reviewed by Thomas Munro, Dilip Kumar, Hamid Akhtar.
Discussion: http://postgr.es/m/CA+TgmoY=aqf0zjTD+3dUWYkgMiNDegDLFjo+6ze=Wtpik+3XqA@mail.gmail.com
Peter Eisentraut [Thu, 24 Sep 2020 18:45:57 +0000 (20:45 +0200)]
Standardize the printf format for st_size
Existing code used various inconsistent ways to printf struct stat's
st_size member. The type of that is off_t, which is in most cases a
signed 64-bit integer, so use the long long int format for it.
Robert Haas [Thu, 24 Sep 2020 17:55:47 +0000 (13:55 -0400)]
Add new 'old_snapshot' contrib module.
You can use this to view the contents of the time to XID mapping
which the server maintains when old_snapshot_threshold != -1.
Being able to view that information may be interesting for users,
and it's definitely useful for figuring out whether the mapping
is being maintained correctly. It isn't, so that will need to be
fixed in a subsequent commit.
Patch by me, reviewed by Thomas Munro, Dilip Kumar, Hamid Akhtar.
Discussion: http://postgr.es/m/CA+TgmoY=aqf0zjTD+3dUWYkgMiNDegDLFjo+6ze=Wtpik+3XqA@mail.gmail.com
Robert Haas [Thu, 24 Sep 2020 17:32:39 +0000 (13:32 -0400)]
Expose oldSnapshotControl definition via new header.
This makes it possible for code outside snapmgr.c to examine the
contents of this data structure. This commit does not add any code
which actually does so; a subsequent commit will make that change.
Patch by me, reviewed by Thomas Munro, Dilip Kumar, Hamid Akhtar.
Discussion: http://postgr.es/m/CA+TgmoY=aqf0zjTD+3dUWYkgMiNDegDLFjo+6ze=Wtpik+3XqA@mail.gmail.com
Tom Lane [Thu, 24 Sep 2020 14:39:11 +0000 (10:39 -0400)]
Doc: sync lobj.sgml's copy of testlo.c with the latter file.
Zhijie Hou
Discussion: https://postgr.es/m/
ce2cd951fe9b448a9cda99dc1a871fb9@G08CNEXMBPEKD05.g08.fujitsu.local
Tom Lane [Thu, 24 Sep 2020 00:26:58 +0000 (20:26 -0400)]
Improve behavior of tsearch_readline(), and remove t_readline().
Commit
fbeb9da22, which added the tsearch_readline APIs, left
t_readline() in place as a compatibility measure. But that function
has been unused and deprecated for twelve years now, so that seems
like enough time to remove it. Doing so, and merging t_readline's
code into tsearch_readline, aids in making several useful
improvements:
* The hard-wired 4K limit on line length in tsearch data files is
removed, by using a StringInfo buffer instead of a fixed-size buffer.
* We can buy back the per-line palloc/pfree added by
3ea7e9550
in the common case where encoding conversion is not required.
* We no longer need a separate pg_verify_mbstr call, as that
functionality was folded into encoding conversion some time ago.
(We could have done some of this stuff while keeping t_readline as a
separate API, but there seems little point, since there's no reason
for anyone to still be using t_readline directly.)
Discussion: https://postgr.es/m/
48A4FA71-524E-41B9-953A-
FD04EF36E2E7@yesql.se
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 <andres@anarazel.de>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/CA%2BhUKGLtsTUOScnNoSMZ-2ZLv%2BwGh01J6kAo_DM8mTRq1sKdSQ%40mail.gmail.com
Tom Lane [Wed, 23 Sep 2020 22:04:53 +0000 (18:04 -0400)]
Improve error cursor positions for problems with partition bounds.
We failed to pass down the query string to check_new_partition_bound,
so that its attempts to provide error cursor positions were for naught;
one must have the query string to get parser_errposition to do anything.
Adjust its API to require a ParseState to be passed down.
Also, improve the logic inside check_new_partition_bound so that the
cursor points at the partition bound for the specific column causing
the issue, when one can be identified.
That part is also for naught if we can't determine the query position of
the column with the problem. Improve transformPartitionBoundValue so
that it makes sure that const-simplified partition expressions will be
properly labeled with positions. In passing, skip calling evaluate_expr
if the value is already a Const, which is surely the most common case.
Alexandra Wang, Ashwin Agrawal, Amit Langote; reviewed by Ashutosh Bapat
Discussion: https://postgr.es/m/CACiyaSopZoqssfMzgHk6fAkp01cL6vnqBdmTw2C5_KJaFR_aMg@mail.gmail.com
Discussion: https://postgr.es/m/CAJV4CdrZ5mKuaEsRSbLf2URQ3h6iMtKD=hik8MaF5WwdmC9uZw@mail.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
Thomas Munro [Wed, 23 Sep 2020 03:17:30 +0000 (15:17 +1200)]
Allow WaitLatch() to be used without a latch.
Due to flaws in commit
3347c982bab, using WaitLatch() without
WL_LATCH_SET could cause an assertion failure or crash. Repair.
While here, also add a check that the latch we're switching to belongs
to this backend, when changing from one latch to another.
Discussion: https://postgr.es/m/CA%2BhUKGK1607VmtrDUHQXrsooU%3Dap4g4R2yaoByWOOA3m8xevUQ%40mail.gmail.com
Tom Lane [Tue, 22 Sep 2020 20:03:32 +0000 (16:03 -0400)]
Simplify SortTocFromFile() by removing fixed buffer-size limit.
pg_restore previously coped with overlength TOC-file lines using some
complicated logic to ignore additional bufferloads. While this isn't
wrong, since we don't expect that the interesting part of a line would
run to more than a dozen or so bytes, it's more complex than it needs
to be. Use a StringInfo instead of a fixed-size buffer so that we can
process long lines as single entities and thus not need the extra
logic.
Daniel Gustafsson
Discussion: https://postgr.es/m/
48A4FA71-524E-41B9-953A-
FD04EF36E2E7@yesql.se
Tom Lane [Tue, 22 Sep 2020 19:59:23 +0000 (15:59 -0400)]
Remove arbitrary line length limit for libpq service files.
Use a StringInfo instead of a fixed-size buffer in parseServiceInfo().
While we've not heard complaints about the existing 255-byte limit,
it certainly seems possible that complex cases could run afoul of it.
Daniel Gustafsson
Discussion: https://postgr.es/m/
48A4FA71-524E-41B9-953A-
FD04EF36E2E7@yesql.se
Tom Lane [Tue, 22 Sep 2020 19:55:13 +0000 (15:55 -0400)]
Rethink API for pg_get_line.c, one more time.
Further experience says that the appending behavior offered by
pg_get_line_append is useful to only a very small minority of callers.
For most, the requirement to reset the buffer after each line is just
an error-prone nuisance. Hence, invent another alternative call
pg_get_line_buf, which takes care of that detail.
Noted while reviewing a patch from Daniel Gustafsson.
Discussion: https://postgr.es/m/
48A4FA71-524E-41B9-953A-
FD04EF36E2E7@yesql.se
Tom Lane [Tue, 22 Sep 2020 15:32:10 +0000 (11:32 -0400)]
Exclude fmgrprotos.h from pgindent processing.
pgindent messes up entries in this file if their names match
typedef names. While there's reason to avoid choosing conflicting
names, we have some historical exceptions, and there's no guarantee
that more duplicates won't appear in future. Since this is a derived
file anyway, there's little harm in just excluding it.
I said yesterday that all our derived files are pgindent-clean, or else
explicitly excluded from indentation, but I'd forgotten about this one.
Now that project is really done, as confirmed by a test run.
Discussion: https://postgr.es/m/
79ed5348-be7a-b647-dd40-
742207186a22@2ndquadrant.com
Tom Lane [Tue, 22 Sep 2020 14:49:11 +0000 (10:49 -0400)]
Improve the error message for an inappropriate column definition list.
The existing message about "a column definition list is only allowed for
functions returning "record"" could be given in some cases where it was
fairly confusing; in particular, a function with multiple OUT parameters
*does* return record according to pg_proc. Break it down into a couple
more cases to deliver a more on-point complaint. Per complaint from
Bruce Momjian.
Discussion: https://postgr.es/m/798909.
1600562993@sss.pgh.pa.us
Tom Lane [Mon, 21 Sep 2020 17:58:26 +0000 (13:58 -0400)]
Fix a few more generator scripts to produce pgindent-clean output.
This completes the project of making all our derived files be
pgindent-clean (or else explicitly excluded from indentation),
so that no surprises result when running pgindent in a built-out
development tree.
Discussion: https://postgr.es/m/
79ed5348-be7a-b647-dd40-
742207186a22@2ndquadrant.com
Tom Lane [Mon, 21 Sep 2020 16:43:42 +0000 (12:43 -0400)]
Copy editing: fix a bunch of misspellings and poor wording.
99% of this is docs, but also a couple of comments. No code changes.
Justin Pryzby
Discussion: https://postgr.es/m/
20200919175804.GE30557@telsasoft.com
Peter Eisentraut [Mon, 21 Sep 2020 13:22:41 +0000 (15:22 +0200)]
Standardize order of use strict and use warnings in Perl code
The standard order in PostgreSQL and other code is use strict first,
but some code was uselessly inconsistent about this.
Heikki Linnakangas [Mon, 21 Sep 2020 11:50:07 +0000 (14:50 +0300)]
Fix checksum calculation in the new sorting GiST build.
Since we're bypassing the buffer manager, we need to call
PageSetChecksumInplace() directly. As reported by Justin Pryzby.
In the passing, add RelationOpenSmgr() calls before all smgrwrite() and
smgrextend() calls. Tom added one before the first smgrextend() call in
commit
c2bb287025, which seems to be enough, but let's play it safe and
do it before each one. That's how it's done in the similar code in
nbtsort.c, too.
Discussion: https://www.postgresql.org/message-id/
20200920224446.GF30557@telsasoft.com
Tom Lane [Sun, 20 Sep 2020 21:08:49 +0000 (17:08 -0400)]
Fix new GIST build code to work under CLOBBER_CACHE_ALWAYS.
Can't say if this fixes *all* cases, but at least we get through
the "point" regression test now, which hyrax's last run did not.
Report: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=hyrax&dt=2020-09-19%2021%3A27%3A23
Peter Eisentraut [Sun, 20 Sep 2020 12:42:54 +0000 (14:42 +0200)]
Fix whitespace
Tom Lane [Sat, 19 Sep 2020 19:11:26 +0000 (15:11 -0400)]
Remove precedence hacks no longer needed without postfix operators.
It's no longer necessary to assign explicit precedences to GENERATED,
NULL_P, PRESERVE, or STRIP_P.
Actually, we don't need to assign precedence to IDENT either; that was
really just there to govern the behavior of target_el's "a_expr IDENT"
production, which no longer ends with that terminal. However, it seems
like a good idea to continue to do so, because it provides a reference
point for a precedence level that we can assign to other unreserved
keywords that lack a natural precedence level.
Research by Peter Eisentraut and John Naylor; comment rewrite by me.
Discussion: https://postgr.es/m/
38ca86db-42ab-9b48-2902-
337a0d6b8311@2ndquadrant.com
Peter Eisentraut [Tue, 25 Aug 2020 05:24:15 +0000 (07:24 +0200)]
Remove unused parameters
Remove various unused parameters in pg_dump code. These have all
become unused over time or were never used.
Discussion: https://www.postgresql.org/message-id/flat/
511bb100-f829-ba21-2f10-
9f952ec06ead%402ndquadrant.com
Thomas Munro [Sat, 19 Sep 2020 03:39:48 +0000 (15:39 +1200)]
Code review for dynahash change.
Commit
be0a6666 left behind a comment about the order of some tests that
didn't make sense without the expensive division, and in fact we might
as well change the order to one that fails more cheaply most of the time
as a micro-optimization. Also, remove the "+ 1" applied to max_bucket,
to drop an instruction and match the original behavior. Per review
from Tom Lane.
Discussion: https://postgr.es/m/VI1PR0701MB696044FC35013A96FECC7AC8F62D0%40VI1PR0701MB6960.eurprd07.prod.outlook.com
Thomas Munro [Fri, 18 Sep 2020 23:28:34 +0000 (11:28 +1200)]
Remove large fill factor support from dynahash.c.
Since ancient times we have had support for a fill factor (maximum load
factor) to be set for a dynahash hash table, but:
1. It was an integer, whereas for in-memory hash tables interesting
load factor targets are probably somewhere near the 0.75-1.0 range.
2. It was implemented in a way that performed an expensive division
operation that regularly showed up in profiles.
3. We are not aware of anyone ever having used a non-default value.
Therefore, remove support, effectively fixing it at 1.
Author: Jakub Wartak <Jakub.Wartak@tomtom.com>
Reviewed-by: Alvaro Herrera <alvherre@2ndquadrant.com>
Reviewed-by: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Reviewed-by: Thomas Munro <thomas.munro@gmail.com>
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/VI1PR0701MB696044FC35013A96FECC7AC8F62D0%40VI1PR0701MB6960.eurprd07.prod.outlook.com
Tom Lane [Fri, 18 Sep 2020 20:46:26 +0000 (16:46 -0400)]
Allow most keywords to be used as column labels without requiring AS.
Up to now, if you tried to omit "AS" before a column label in a SELECT
list, it would only work if the column label was an IDENT, that is not
any known keyword. This is rather unfriendly considering that we have
so many keywords and are constantly growing more. In the wake of commit
1ed6b8956 it's possible to improve matters quite a bit.
We'd originally tried to make this work by having some of the existing
keyword categories be allowed without AS, but that didn't work too well,
because each category contains a few special cases that don't work
without AS. Instead, invent an entirely orthogonal keyword property
"can be bare column label", and mark all keywords that way for which
we don't get shift/reduce errors by doing so.
It turns out that of our 450 current keywords, all but 39 can be made
bare column labels, improving the situation by over 90%. This number
might move around a little depending on future grammar work, but it's
a pretty nice improvement.
Mark Dilger, based on work by myself and Robert Haas;
review by John Naylor
Discussion: https://postgr.es/m/
38ca86db-42ab-9b48-2902-
337a0d6b8311@2ndquadrant.com
Robert Haas [Fri, 18 Sep 2020 17:26:48 +0000 (13:26 -0400)]
pg_surgery: Try to stabilize regression tests.
According to buildfarm member sungazer, the behavior of VACUUM can be
unstable in these tests even if we prevent autovacuum from running on
the tables in question, apparently because even a manual vacuum can
behave differently depending on whether anything else is running that
holds back the global xmin. So use a temporary table instead, which
as of commit
a7212be8b9e0885ee769e8c55f99ef742cda487b enables
vacuuming using a more aggressive cutoff.
This approach can't be used for the regression test that involves a
materialized view, but that test doesn't run vacuum, so it shouldn't
be prone to this particular failure mode.
Analysis by Tom Lane. Patch by Ashutosh Sharma and me.
Discussion: http://postgr.es/m/665524.
1599948007@sss.pgh.pa.us
Amit Kapila [Fri, 18 Sep 2020 04:44:30 +0000 (10:14 +0530)]
Update file header comments for logical/relation.c.
Author: Amit Langote
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/CA+HiwqE20oZoix13JyCeALpTf_SmjarZWtBFe5sND6zz+iupAw@mail.gmail.com
Amit Kapila [Fri, 18 Sep 2020 04:10:04 +0000 (09:40 +0530)]
Fix comments in heapam.c.
After commits
85f6b49c2c and
3ba59ccc89, we can allow parallel inserts
which was earlier not possible as parallel group members won't conflict
for relation extension and page lock. In those commits, we forgot to
update comments at few places.
Author: Amit Kapila
Reviewed-by: Robert Haas and Dilip Kumar
Backpatch-through: 13
Discussion: https://postgr.es/m/CAFiTN-tMrQh5FFMPx5aWJ+1gi1H6JxktEhq5mDwCHgnEO5oBkA@mail.gmail.com
Tom Lane [Fri, 18 Sep 2020 01:02:55 +0000 (21:02 -0400)]
Try to stabilize output from rolenames regression test.
It's not quite clear why commit
45b980570 has resulted in
some instability here, though interference from concurrent
autovacuum runs seems like a reasonable guess. What is
clear is that the output ordering of the test queries is
underdetermined for no very good reason. Extend the
ORDER BY keys in hopes of fixing the buildfarm.
Discussion: https://postgr.es/m/147499.
1600351924@sss.pgh.pa.us
Tom Lane [Thu, 17 Sep 2020 23:38:05 +0000 (19:38 -0400)]
Remove support for postfix (right-unary) operators.
This feature has been a thorn in our sides for a long time, causing
many grammatical ambiguity problems. It doesn't seem worth the
pain to continue to support it, so remove it.
There are some follow-on improvements we can make in the grammar,
but this commit only removes the bare minimum number of productions,
plus assorted backend support code.
Note that pg_dump and psql continue to have full support, since
they may be used against older servers. However, pg_dump warns
about postfix operators. There is also a check in pg_upgrade.
Documentation-wise, I (tgl) largely removed the "left unary"
terminology in favor of saying "prefix operator", which is
a more standard and IMO less confusing term.
I included a catversion bump, although no initial catalog data
changes here, to mark the boundary at which oprkind = 'r'
stopped being valid in pg_operator.
Mark Dilger, based on work by myself and Robert Haas;
review by John Naylor
Discussion: https://postgr.es/m/
38ca86db-42ab-9b48-2902-
337a0d6b8311@2ndquadrant.com
Tom Lane [Thu, 17 Sep 2020 20:17:27 +0000 (16:17 -0400)]
Remove factorial operators, leaving only the factorial() function.
The "!" operator is our only built-in postfix operator. Remove it,
on the way to removal of grammar support for postfix operators.
There is also a "!!" prefix operator, but since it's been marked
deprecated for most of its existence, we might as well remove it too.
Also zap the SQL alias function numeric_fac(), which seems to have
equally little reason to live.
Mark Dilger, based on work by myself and Robert Haas;
review by John Naylor
Discussion: https://postgr.es/m/
38ca86db-42ab-9b48-2902-
337a0d6b8311@2ndquadrant.com
Tom Lane [Thu, 17 Sep 2020 18:16:18 +0000 (14:16 -0400)]
Further improve pgindent's list of file exclusions.
I despair of people keeping the README file's notes in sync with
the actual exclusion list, so move the notes into the exclusion file.
Adjust the pgindent script to explicitly ignore comments in the file,
just in case (though it's hard to believe any would match filenames).
Extend the list so that it doesn't process any files we'd not wish it to
even in a fully-built-out development directory. (There are still a
couple of derived files that it wants to reformat, but fixing the
generator scripts for those seems like fit material for a separate patch.)
Discussion: https://postgr.es/m/
79ed5348-be7a-b647-dd40-
742207186a22@2ndquadrant.com
Tom Lane [Thu, 17 Sep 2020 16:52:18 +0000 (12:52 -0400)]
Improve common/logging.c's support for multiple verbosity levels.
Instead of hard-wiring specific verbosity levels into the option
processing of client applications, invent pg_logging_increase_verbosity()
and encourage clients to implement --verbose by calling that. Then,
the common convention that more -v's gets you more verbosity just works.
In particular, this allows resurrection of the debug-grade messages that
have long existed in pg_dump and its siblings. They were unreachable
before this commit due to lack of a way to select PG_LOG_DEBUG logging
level. (It appears that they may have been unreachable for some time
before common/logging.c was introduced, too, so I'm not specifically
blaming
cc8d41511 for the oversight. One reason for thinking that is
that it's now apparent that _allocAH()'s message needs a null-pointer
guard. Testing might have failed to reveal that before
96bf88d52.)
Discussion: https://postgr.es/m/
1173106.
1600116625@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
Peter Eisentraut [Thu, 17 Sep 2020 09:39:28 +0000 (11:39 +0200)]
Allow CURRENT_ROLE where CURRENT_USER is accepted
In the particular case of GRANTED BY, this is specified in the SQL
standard. Since in PostgreSQL, CURRENT_ROLE is equivalent to
CURRENT_USER, and CURRENT_USER is already supported here, adding
CURRENT_ROLE is trivial. The other cases are PostgreSQL extensions,
but for the same reason it also makes sense there.
Reviewed-by: Vik Fearing <vik@postgresfriends.org>
Reviewed-by: Asif Rehman <asifr.rehman@gmail.com>
Reviewed-by: Alvaro Herrera <alvherre@2ndquadrant.com>
Discussion: https://www.postgresql.org/message-id/flat/
f2feac44-b4c5-f38f-3699-
2851d6a76dc9%402ndquadrant.com
Heikki Linnakangas [Thu, 17 Sep 2020 08:33:40 +0000 (11:33 +0300)]
Add support for building GiST index by sorting.
This adds a new optional support function to the GiST access method:
sortsupport. If it is defined, the GiST index is built by sorting all data
to the order defined by the sortsupport's comparator function, and packing
the tuples in that order to GiST pages. This is similar to how B-tree
index build works, and is much faster than inserting the tuples one by
one. The resulting index is smaller too, because the pages are packed more
tightly, upto 'fillfactor'. The normal build method works by splitting
pages, which tends to lead to more wasted space.
The quality of the resulting index depends on how good the opclass-defined
sort order is. A good order preserves locality of the input data.
As the first user of this facility, add 'sortsupport' function to the
point_ops opclass. It sorts the points in Z-order (aka Morton Code), by
interleaving the bits of the X and Y coordinates.
Author: Andrey Borodin
Reviewed-by: Pavel Borisov, Thomas Munro
Discussion: https://www.postgresql.org/message-id/
1A36620E-CAD8-4267-9067-
FB31385E7C0D%40yandex-team.ru
Michael Paquier [Thu, 17 Sep 2020 07:33:22 +0000 (16:33 +0900)]
doc: Apply more consistently <productname> markup for OpenSSL
OpenSSL was quoted in inconsistent ways in many places of the docs,
sometimes with <application>, <productname> or just nothing.
Author: Daniel Gustafsson
Discussion: https://postgr.es/m/
DA91E5F0-5F9D-41A7-A7A6-
B91CDE0F1D63@yesql.se
Michael Paquier [Thu, 17 Sep 2020 02:49:29 +0000 (11:49 +0900)]
Improve tab completion of IMPORT FOREIGN SCHEMA in psql
It is not possible to get a list of foreign schemas as the server is not
known, so this provides instead a list of local schemas, which is more
useful than nothing if using a loopback server or having schema names
matching in the local and remote servers.
Author: Jeff Janes
Reviewed-by: Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/CAMkU=1wr7Roj41q-XiJs=Uyc2xCmHhcGGy7J-peJQK-e+w=ghw@mail.gmail.com
Tom Lane [Thu, 17 Sep 2020 01:06:50 +0000 (21:06 -0400)]
Teach walsender to update its process title for replication commands.
Because the code path taken for SQL commands executed in a walsender
will update the process title, we pretty much have to update the
title for replication commands as well. Otherwise, the title shows
"idle" for the rest of a logical walsender's lifetime once it's
executed any SQL command.
Playing with this, I confirm that a walsender now typically spends
most of its life reporting
walsender postgres [local] START_REPLICATION
Considering this in isolation, it might be better to have it say
walsender postgres [local] sending replication data
However, consistency with the other cases seems to be a stronger
argument.
In passing, remove duplicative pgstat_report_activity call.
Discussion: https://postgr.es/m/880181.
1600026471@sss.pgh.pa.us
Tom Lane [Thu, 17 Sep 2020 00:31:19 +0000 (20:31 -0400)]
Improve formatting of create_help.pl and plperl_opmask.pl output.
Adjust the whitespace in the emitted files so that it matches
what pgindent would do. This makes the generated files look
like they match project style, and avoids confusion if someone
does run pgindent on the generated files.
Also, add probes.h to pgindent's exclusion list, because it can
confuse pgindent, plus there's not much point in processing it.
Daniel Gustafsson, additional fixes by me
Discussion: https://postgr.es/m/
79ed5348-be7a-b647-dd40-
742207186a22@2ndquadrant.com
Alvaro Herrera [Wed, 16 Sep 2020 16:04:38 +0000 (13:04 -0300)]
Fix bogus completion tag usage in walsender
Since commit
fd5942c18f97 (2012, 9.3-era), walsender has been sending
completion tags for certain replication commands twice -- and they're
not even consistent. Apparently neither libpq nor JDBC have a problem
with it, but it's not kosher. Fix by remove the EndCommand() call in
the common code path for them all, and inserting specific calls to
EndReplicationCommand() specifically in those places where it's needed.
EndReplicationCommand() is a new simple function to send the completion
tag for replication commands. Do this instead of sending a generic
SELECT completion tag for them all, which was also pretty bogus (if
innocuous). While at it, change StartReplication() to use
EndReplicationCommand() instead of pg_puttextmessage().
In commit
2f9661311b83, I failed to realize that replication commands
are not close-enough kin of regular SQL commands, so the
DROP_REPLICATION_SLOT tag I added is undeserved and a type pun. Take it
out.
Backpatch to 13, where the latter commit appeared. The duplicate tag
has been sent since 9.3, but since nothing is broken, it doesn't seem
worth fixing.
Per complaints from Tom Lane.
Discussion: https://postgr.es/m/
1347966.
1600195735@sss.pgh.pa.us
Tom Lane [Wed, 16 Sep 2020 20:04:36 +0000 (16:04 -0400)]
Centralize setup of SIGQUIT handling for postmaster child processes.
We decided that the policy established in commit
7634bd4f6 for
the bgwriter, checkpointer, walwriter, and walreceiver processes,
namely that they should accept SIGQUIT at all times, really ought
to apply uniformly to all postmaster children. Therefore, get
rid of the duplicative and inconsistent per-process code for
establishing that signal handler and removing SIGQUIT from BlockSig.
Instead, make InitPostmasterChild do it.
The handler set up by InitPostmasterChild is SignalHandlerForCrashExit,
which just summarily does _exit(2). In interactive backends, we
almost immediately replace that with quickdie, since we would prefer
to try to tell the client that we're dying. However, this patch is
changing the behavior of autovacuum (both launcher and workers), as
well as walsenders. Those processes formerly also used quickdie,
but AFAICS that was just mindless copy-and-paste: they don't have
any interactive client that's likely to benefit from being told this.
The stats collector continues to be an outlier, in that it thinks
SIGQUIT means normal exit. That should probably be changed for
consistency, but there's another patch set where that's being
dealt with, so I didn't do so here.
Discussion: https://postgr.es/m/644875.
1599933441@sss.pgh.pa.us
Tom Lane [Wed, 16 Sep 2020 18:28:11 +0000 (14:28 -0400)]
Don't fetch partition check expression during InitResultRelInfo.
Since there is only one place that actually needs the partition check
expression, namely ExecPartitionCheck, it's better to fetch it from
the relcache there. In this way we will never fetch it at all if
the query never has use for it, and we still fetch it just once when
we do need it.
The reason for taking an interest in this is that if the relcache
doesn't already have the check expression cached, fetching it
requires obtaining AccessShareLock on the partition root. That
means that operations that look like they should only touch the
partition itself will also take a lock on the root. In particular
we observed that TRUNCATE on a partition may take a lock on the
partition's root, contributing to a deadlock situation in parallel
pg_restore.
As written, this patch does have a small cost, which is that we
are microscopically reducing efficiency for the case where a partition
has an empty check expression. ExecPartitionCheck will be called,
and will go through the motions of setting up and checking an empty
qual, where before it would not have been called at all. We could
avoid that by adding a separate boolean flag to track whether there
is a partition expression to test. However, this case only arises
for a default partition with no siblings, which surely is not an
interesting case in practice. Hence adding complexity for it
does not seem like a good trade-off.
Amit Langote, per a suggestion by me
Discussion: https://postgr.es/m/VI1PR03MB31670CA1BD9625C3A8C5DD05EB230@VI1PR03MB3167.eurprd03.prod.outlook.com
Peter Geoghegan [Wed, 16 Sep 2020 17:42:30 +0000 (10:42 -0700)]
Fix amcheck child check pg_upgrade bug.
Commit
d114cc53 overlooked the fact that pg_upgrade'd B-Tree indexes
have leaf page high keys whose offset numbers do not match the one from
the copy of the tuple one level up (the copy stored with a downlink for
leaf page's right sibling page). This led to false positive reports of
corruption from bt_index_parent_check() when it was called to verify a
pg_upgrade'd index.
To fix, skip comparing the offset number on pg_upgrade'd B-Tree indexes.
Author: Anastasia Lubennikova <a.lubennikova@postgrespro.ru>
Author: Peter Geoghegan <pg@bowt.ie>
Reported-By: Andrew Bille <andrewbille@gmail.com>
Diagnosed-By: Anastasia Lubennikova <a.lubennikova@postgrespro.ru>
Bug: #16619
Discussion: https://postgr.es/m/16619-
aaba10f83fdc1c3c@postgresql.org
Backpatch: 13-, where child check was enhanced.
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
Fujii Masao [Wed, 16 Sep 2020 09:47:39 +0000 (18:47 +0900)]
Add leader_pid field into the example of file_fdw for csvlog.
Commit
b8fdee7d0c added leader_pid field into csvlog,
but forgot to update the example of file_fdw for csvlog.
Author: Yuta Katsuragi
Michael Paquier [Wed, 16 Sep 2020 07:26:50 +0000 (16:26 +0900)]
Avoid retrieval of CHECK constraints and DEFAULT exprs in data-only dump
Those extra queries are not necessary when doing a data-only dump. With
this change, this means that the dependencies between CHECK/DEFAULT and
the parent table are not tracked anymore for a data-only dump. However,
these dependencies are only used for the schema generation and we have
never guaranteed that a dump can be reloaded if a CHECK constraint uses
a custom function whose behavior changes when loading the data, like
when using cross-table references in the CHECK function.
Author: Julien Rouhaud
Reviewed-by: Daniel Gustafsson, Michael Paquier
Discussion: https://postgr.es/m/
20200712054850.GA92357@nol
Jeff Davis [Wed, 16 Sep 2020 04:34:05 +0000 (21:34 -0700)]
Change LogicalTapeSetBlocks() to use nBlocksWritten.
Previously, it was based on nBlocksAllocated to account for tapes with
open write buffers that may not have made it to the BufFile yet.
That was unnecessary, because callers do not need to get the number of
blocks while a tape has an open write buffer; and it also conflicted
with the preallocation logic added for HashAgg.
Reviewed-by: Peter Geoghegan
Discussion: https://postgr.es/m/
ce5af05900fdbd0e9185747825a7423c48501964.camel@j-davis.com
Backpatch-through: 13
Jeff Davis [Wed, 16 Sep 2020 04:16:31 +0000 (21:16 -0700)]
HashAgg: release write buffers sooner by rewinding tape.
This was an oversight. The purpose of
7fdd919ae7 was to avoid keeping
tape buffers around unnecessisarily, but HashAgg didn't rewind early
enough.
Reviewed-by: Peter Geoghegan
Discussion: https://postgr.es/m/
1fb1151c2cddf8747d14e0532da283c3f97e2685.camel@j-davis.com
Backpatch-through: 13
Amit Kapila [Wed, 16 Sep 2020 02:15:44 +0000 (07:45 +0530)]
Fix initialization of RelationSyncEntry for streaming transactions.
In commit
464824323e, for each RelationSyncEntry we maintained the list
of xids (streamed_txns) for which we have already sent the schema. This
helps us to track when to send the schema to the downstream node for
replication of streaming transactions. Before this list got initialized,
we were processing invalidation messages which access this list and led
to an assertion failure.
In passing, clean up the nearby code:
* Initialize the list of xids with NIL instead of NULL which is our usual
coding practice.
* Remove the MemoryContext switch for creating a RelationSyncEntry in dynahash.
Diagnosed-by: Amit Kapila and Tom Lane
Author: Amit Kapila
Reviewed-by: Tom Lane and Dilip Kumar
Discussion: https://postgr.es/m/904373.
1600033123@sss.pgh.pa.us
David Rowley [Wed, 16 Sep 2020 01:22:20 +0000 (13:22 +1200)]
Optimize compactify_tuples function
This function could often be seen in profiles of vacuum and could often
be a significant bottleneck during recovery. The problem was that a qsort
was performed in order to sort an array of item pointers in reverse offset
order so that we could use that to safely move tuples up to the end of the
page without overwriting the memory of yet-to-be-moved tuples. i.e. we
used to compact the page starting at the back of the page and move towards
the front. The qsort that this required could be expensive for pages with
a large number of tuples.
In this commit, we take another approach to tuple compactification.
Now, instead of sorting the remaining item pointers array we first check
if the array is presorted and only memmove() the tuples that need to be
moved. This presorted check can be done very cheaply in the calling
functions when the array is being populated. This presorted case is very
fast.
When the item pointer array is not presorted we must copy tuples that need
to be moved into a temp buffer before copying them back into the page
again. This differs from what we used to do here as we're now copying the
tuples back into the page in reverse line pointer order. Previously we
left the existing order alone. Reordering the tuples results in an
increased likelihood of hitting the pre-sorted case the next time around.
Any newly added tuple which consumes a new line pointer will also maintain
the correct sort order of tuples in the page which will also result in the
presorted case being hit the next time. Only consuming an unused line
pointer can cause the order of tuples to go out again, but that will be
corrected next time the function is called for the page.
Benchmarks have shown that the non-presorted case is at least equally as
fast as the original qsort method even when the page just has a few
tuples. As the number of tuples becomes larger the new method maintains
its performance whereas the original qsort method became much slower when
the number of tuples on the page became large.
Author: David Rowley
Reviewed-by: Thomas Munro
Tested-by: Jakub Wartak
Discussion: https://postgr.es/m/CA+hUKGKMQFVpjr106gRhwk6R-nXv0qOcTreZuQzxgpHESAL6dw@mail.gmail.com
Alvaro Herrera [Wed, 16 Sep 2020 00:03:14 +0000 (21:03 -0300)]
Fix use-after-free bug with event triggers in an extension script
ALTER TABLE commands in an extension script are added to an event
trigger command list; but starting with commit
b5810de3f4 they do so in
a memory context that's too short-lived, so when execution ends and time
comes to use the entries, they've already been freed.
(This would also be a problem with ALTER TABLE commands in a
multi-command query string, but these serendipitously end in
PortalContext -- which probably explains why it took so long for this to
be reported.)
Fix by using the memory context specifically set for that, instead.
Backpatch to 13, where the aforementioned commit appeared.
Reported-by: Philippe Beaudoin
Author: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
Discussion: https://postgr.es/m/
20200902193715.
6e0269d4@firost
David Rowley [Tue, 15 Sep 2020 23:25:46 +0000 (11:25 +1200)]
Report resource usage at the end of recovery
Reporting this has been rather useful in some recent recovery speedup
work. It also seems like something that will be useful to the average DBA
too.
Author: David Rowley
Reviewed-by: Thomas Munro
Discussion: https://postgr.es/m/CAApHDvqYVORiZxq2xPvP6_ndmmsTkvr6jSYv4UTNaFa5i1kd%3DQ%40mail.gmail.com
David Rowley [Tue, 15 Sep 2020 11:44:45 +0000 (23:44 +1200)]
Allow incremental sorts for windowing functions
This expands on the work done in
d2d8a229b and allows incremental sort
to be considered during create_window_paths().
Author: David Rowley
Reviewed-by: Daniel Gustafsson, Tomas Vondra
Discussion: https://postgr.es/m/CAApHDvoOHobiA2x13NtWnWLcTXYj9ddpCkv9PnAJQBMegYf_xw%40mail.gmail.com
David Rowley [Tue, 15 Sep 2020 03:07:57 +0000 (15:07 +1200)]
Fix compiler warning
Introduced in
0aa8f7640.
MSVC warned about performing 32-bit bit shifting when it appeared like we
might like a 64-bit result. We did, but it just so happened that none of
the calls to this function could have caused the 32-bit shift to overflow.
Here we just cast the constant to int64 to make the compiler happy.
Discussion: https://postgr.es/m/CAApHDvofA_vsrpC13mq_hZyuye5B-ssKEaer04OouXYCO5-uXQ@mail.gmail.com
Tom Lane [Mon, 14 Sep 2020 16:35:00 +0000 (12:35 -0400)]
Make walsenders show their replication commands in pg_stat_activity.
A walsender process that has executed a SQL command left the text of
that command in pg_stat_activity.query indefinitely, which is quite
confusing if it's in RUNNING state but not doing that query. An easy
and useful fix is to treat replication commands as if they were SQL
queries, and show them in pg_stat_activity according to the same rules
as for regular queries. While we're at it, it seems also sensible to
set debug_query_string, allowing error logging and debugging to see
the replication command.
While here, clean up assorted silliness in exec_replication_command:
* The SQLCmd path failed to restore CurrentMemoryContext to the caller's
value, and failed to delete the temp context created in this routine.
It's only through great good fortune that these oversights did not
result in long-term memory leaks or other problems. It seems cleaner
to code SQLCmd as a separate early-exit path, so do it like that.
* Remove useless duplicate call of SnapBuildClearExportedSnapshot().
* replication_scanner_finish() was never called.
None of those things are significant enough to merit a backpatch,
so this is for HEAD only.
Discussion: https://postgr.es/m/880181.
1600026471@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.horikyota.ntt@gmail.com
Fujii Masao [Mon, 14 Sep 2020 05:16:07 +0000 (14:16 +0900)]
Fix typos.
Author: Naoki Nakamichi
Discussion: https://postgr.es/m/
b6919d145af00295a8e86ce4d034b7cd@oss.nttdata.com
Michael Paquier [Mon, 14 Sep 2020 04:56:41 +0000 (13:56 +0900)]
Make index_set_state_flags() transactional
3c84046 is the original commit that introduced index_set_state_flags(),
where the presence of SnapshotNow made necessary the use of an in-place
update. SnapshotNow has been removed in
813fb03, so there is no actual
reasons to not make this operation transactional.
Note that while making the operation more robust, using a transactional
operation in this routine was not strictly necessary as there was no use
case for it yet. However, some future features are going to need a
transactional behavior, like support for CREATE/DROP INDEX CONCURRENTLY
with partitioned tables, where indexes in a partition tree need to have
all their pg_index.indis* flags updated in the same transaction to make
the operation stable to the end-user by keeping partition trees
consistent, even with a failure mid-flight.
REINDEX CONCURRENTLY uses already transactional updates when swapping
the old and new indexes, making this change more consistent with the
index-swapping logic.
Author: Michael Paquier
Reviewed-by: Anastasia Lubennikova
Discussion: https://postgr.es/m/
20200903080440.GA8559@paquier.xyz
Peter Eisentraut [Mon, 14 Sep 2020 04:42:07 +0000 (06:42 +0200)]
Message fixes and style improvements
Michael Paquier [Mon, 14 Sep 2020 01:44:23 +0000 (10:44 +0900)]
Avoid useless allocations for information of dumpable objects in pg_dump/
If there are no objects of a certain type, there is no need to do an
allocation for a set of DumpableObject items. The previous coding did
an allocation of 1 byte instead as per the fallback of pg_malloc() in
the event of an allocation size of zero. This assigns NULL instead for
a set of dumpable objects.
A similar rule already applied to findObjectByOid(), so this makes the
code more defensive as we would just fail with a pointer dereference
instead of attempting to use some incorrect data if a non-existing,
positive, OID is given by a caller of this function.
Author: Daniel Gustafsson
Reviewed-by: Julien Rouhaud, Ranier Vilela
Discussion: https://postgr.es/m/
26C43E58-BDD0-4F1A-97CC-
4A07B52E32C5@yesql.se
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
Amit Kapila [Sat, 12 Sep 2020 02:32:54 +0000 (08:02 +0530)]
Fix inconsistency in determining the timestamp of the db statfile.
We use the timestamp of the global statfile if we are not able to
determine it for a particular database in case the entry for that database
doesn't exist. However, we were using it even when the statfile is
corrupt.
As there is no user reported issue and it is not clear if there is any
impact of this on actual application so decided not to backpatch.
Reported-by: Amit Kapila
Author: Amit Kapila
Reviewed-by: Sawada Masahiko, Magnus Hagander and Alvaro Herrera
Discussion: https://postgr.es/m/CAA4eK1J3oTJKyVq6v7K4d3jD+vtnruG9fHRib6UuWWsrwAR6Aw@mail.gmail.com
Amit Kapila [Sat, 12 Sep 2020 02:17:53 +0000 (07:47 +0530)]
Remove unused function declaration in logicalproto.h.
In the passing, fix a typo in pgoutput.c.
Reported-by: Tomas Vondra
Author: Tomas Vondra
Reviewed-by: Dilip Kumar
Discussion: https://postgr.es/m/
20200909084353.pncuclpbwlr7vylh@development
Jeff Davis [Sat, 12 Sep 2020 00:10:02 +0000 (17:10 -0700)]
logtape.c: do not preallocate for tapes when sorting
The preallocation logic is only useful for HashAgg, so disable it when
sorting.
Also, adjust an out-of-date comment.
Reviewed-by: Peter Geoghegan
Discussion: https://postgr.es/m/CAH2-Wzn_o7tE2+hRVvwSFghRb75AJ5g-nqGzDUqLYMexjOAe=g@mail.gmail.com
Backpatch-through: 13
Tom Lane [Fri, 11 Sep 2020 20:01:28 +0000 (16:01 -0400)]
Accept SIGQUIT during error recovery in auxiliary processes.
The bgwriter, checkpointer, walwriter, and walreceiver processes
claimed to allow SIGQUIT "at all times". In reality SIGQUIT
would get re-blocked during error recovery, because we didn't
update the actual signal mask immediately, so sigsetjmp() would
save and reinstate a mask that includes SIGQUIT.
This appears to be simply a coding oversight. There's never a
good reason to hold off SIGQUIT in these processes, because it's
going to just call _exit(2) which should be safe enough, especially
since the postmaster is going to tear down shared memory afterwards.
Hence, stick in PG_SETMASK() calls to install the modified BlockSig
mask immediately.
Also try to improve the comments around sigsetjmp blocks. Most of
them were just referencing postgres.c, which is misleading because
actually postgres.c manages the signals differently.
No back-patch, since there's no evidence that this is causing any
problems in the field.
Discussion: https://postgr.es/m/CALDaNm1d1hHPZUg3xU4XjtWBOLCrA+-2cJcLpw-cePZ=GgDVfA@mail.gmail.com
Alvaro Herrera [Fri, 11 Sep 2020 19:15:47 +0000 (16:15 -0300)]
psql: Display stats target of extended statistics
The stats target can be set since commit
d06215d03, but wasn't shown by
psql.
Author: Justin Pryzby <justin@telsasoft.com>
Discussion: https://postgr.es/m/
20200831050047.GG5450@telsasoft.com
Reviewed-by: Georgios Kokolatos <gkokolatos@protonmail.com>
Reviewed-by: Tatsuro Yamada <tatsuro.yamada.tf@nttcom.co.jp>
Tom Lane [Fri, 11 Sep 2020 16:24:46 +0000 (12:24 -0400)]
Log a message when resorting to SIGKILL during shutdown/crash recovery.
Currently, no useful trace is left in the logs when the postmaster
is forced to use SIGKILL to shut down children that failed to respond
to SIGQUIT. Some questions were raised about how often that scenario
happens in the buildfarm, so let's add a LOG-level message showing
that it happened.
Discussion: https://postgr.es/m/
1850884.
1599601164@sss.pgh.pa.us
Tom Lane [Fri, 11 Sep 2020 16:20:16 +0000 (12:20 -0400)]
Don't run atexit callbacks during signal exits from ProcessStartupPacket.
Although
58c6feccf fixed the case for SIGQUIT, we were still calling
proc_exit() from signal handlers for SIGTERM and timeout failures in
ProcessStartupPacket. Fortunately, at the point where that code runs,
we haven't yet connected to shared memory in any meaningful way, so
there is nothing we need to undo in shared memory. This means it
should be safe to use _exit(1) here, ie, not run any atexit handlers
but also inform the postmaster that it's not a crash exit.
To make sure nobody breaks the "nothing to undo" expectation, add
a cross-check that no on-shmem-exit or before-shmem-exit handlers
have been registered yet when we finish using these signal handlers.
This change is simple enough that maybe it could be back-patched,
but I won't risk that right now.
Discussion: https://postgr.es/m/
1850884.
1599601164@sss.pgh.pa.us
Alvaro Herrera [Fri, 11 Sep 2020 15:53:25 +0000 (12:53 -0300)]
Update copyright year
Thinko in
40b3e2c201af.
Reported-by: "Wang, Shenhao" <wangsh.fnst@cn.fujitsu.com>
Discussion: https://postgr.es/m/
ed98706b82694b57a8c0d339a10732aa@G08CNEXMBPEKD06.g08.fujitsu.local
Amit Kapila [Fri, 11 Sep 2020 04:30:01 +0000 (10:00 +0530)]
Skip empty transaction stream in test_decoding.
We were decoding empty transactions via streaming APIs added in commit
45fdc9738b even when the user used the option 'skip-empty-xacts'. The APIs
makes no effort to skip empty xacts under the assumption that we will
never try to stream such transactions. However, that is not true because
we can pick to stream a transaction that has change messages for
REORDER_BUFFER_CHANGE_INTERNAL_SNAPSHOT and we don't send such messages to
downstream rather they are just to update the internal state. So, we need
to skip such xacts when plugin uses the option 'skip-empty-xacts'.
Diagnosed-By: Amit Kapila
Author: Dilip Kumar
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/CAA4eK1+OqgFNZkf7=ETe_y5ntjgDk3T0wcdkd4Sot_u1hySGfw@mail.gmail.com
Alvaro Herrera [Thu, 10 Sep 2020 22:37:02 +0000 (19:37 -0300)]
Print WAL logical message contents in pg_waldump
This helps debuggability when looking at WAL streams containing logical
messages.
Author: Ashutosh Bapat <ashutosh.bapat@2ndquadrant.com>
Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/CAExHW5sWx49rKmXbg5H1Xc1t+nRv9PaYKQmgw82HPt6vWDVmDg@mail.gmail.com
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
Robert Haas [Thu, 10 Sep 2020 15:10:55 +0000 (11:10 -0400)]
New contrib module, pg_surgery, with heap surgery functions.
Sometimes it happens that the visibility information for a tuple
becomes corrupted, either due to bugs in the database software or
external factors. Provide a function heap_force_kill() that can
be used to truncate such dead tuples to dead line pointers, and
a function heap_force_freeze() that can be used to overwrite the
visibility information in such a way that the tuple becomes
all-visible.
These functions are unsafe, in that you can easily use them to
corrupt a database that was not previously corrupted, and you can
use them to further corrupt an already-corrupted database or to
destroy data. The documentation accordingly cautions against
casual use. However, in some cases they permit recovery of data
that would otherwise be very difficult to recover, or to allow a
system to continue to function when it would otherwise be difficult
to do so.
Because we may want to add other functions for performing other
kinds of surgery in the future, the new contrib module is called
pg_surgery rather than something specific to these functions. I
proposed back-patching this so that it could be more easily used
by people running existing releases who are facing these kinds of
problems, but that proposal did not attract enough support, so
no back-patch for now.
Ashutosh Sharma, reviewed and tested by Andrey M. Borodin,
M. Beena Emerson, Masahiko Sawada, Rajkumar Raghuwanshi,
Asim Praveen, and Mark Dilger, and somewhat revised by me.
Discussion: http://postgr.es/m/CA+TgmoZW1fsU-QUNCRUQMGUygBDPVeOTLCqRdQZch=EYZnctSA@mail.gmail.com
Peter Eisentraut [Thu, 10 Sep 2020 14:13:19 +0000 (16:13 +0200)]
Remove unused parameter
Apparently, this was never used when
introduced (
3dad73e71f08abd86564d5090a58ca71740e07e0).
Discussion: https://www.postgresql.org/message-id/flat/
511bb100-f829-ba21-2f10-
9f952ec06ead%402ndquadrant.com
Peter Eisentraut [Thu, 10 Sep 2020 13:55:31 +0000 (15:55 +0200)]
Add libpq's openssl dependencies to pkg-config file
Add libssl and libcrypto to libpq.pc's Requires.private. This allows
static linking to work if those libssl or libcrypto themselves have
dependencies in their *.private fields, such as -lz in some cases.
Reported-by: Sandro Mani <manisandro@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/
837d1dcf-2fca-ee6e-0d7e-
6bce1a1bac75@gmail.com
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 <jgdr@dalibo.com>
Discussion: https://www.postgresql.org/message-id/flat/
153201618542.1404.
3611626898935613264%40wrigleys.postgresql.org
Peter Eisentraut [Thu, 10 Sep 2020 12:52:36 +0000 (14:52 +0200)]
Add more tests for EXTRACT of date type
EXTRACT of date type is implemented as a wrapper around EXTRACT of
timestamp, so the code is already tested there. But the externally
visible behavior of EXTRACT on date is not recorded anywhere. Since
there is some discussion about reimplementing or refactoring some of
this, add some more explicit tests of EXTRACT on date, similar in
structure to existing EXTRACT tests on other data types.
Discussion: https://www.postgresql.org/message-id/flat/
42b73d2d-da12-ba9f-570a-
420e0cce19d9@phystech.edu
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
Etsuro Fujita [Thu, 10 Sep 2020 09:00:00 +0000 (18:00 +0900)]
Clean up some code and comments in partbounds.c.
Do some minor cleanup for commit
c8434d64c: 1) remove a useless
assignment (in normal builds) and 2) improve comments a little.
Back-patch to v13 where the aforementioned commit went in.
Author: Etsuro Fujita
Reviewed-by: Alvaro Herrera
Discussion: https://postgr.es/m/CAPmGK16yCd2R4=bQ4g8N2dT9TtA5ZU+qNmJ3LPc_nypbNy4_2A@mail.gmail.com
Michael Paquier [Thu, 10 Sep 2020 06:50:19 +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.GK17022@telsasoft.com
Backpatch-through: 9.6
Noah Misch [Thu, 10 Sep 2020 01:50:24 +0000 (18:50 -0700)]
Fix rd_firstRelfilenodeSubid for nailed relations, in parallel workers.
Move applicable code out of RelationBuildDesc(), which nailed relations
bypass. Non-assert builds experienced no known problems. Back-patch to
v13, where commit
c6b92041d38512a4176ed76ad06f713d2e6c01a8 introduced
rd_firstRelfilenodeSubid.
Kyotaro Horiguchi. Reported by Justin Pryzby.
Discussion: https://postgr.es/m/
20200907023737.GA7158@telsasoft.com
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
Peter Eisentraut [Wed, 9 Sep 2020 18:16:28 +0000 (20:16 +0200)]
Expose internal function for converting int64 to numeric
Existing callers had to take complicated detours via
DirectFunctionCall1(). This simplifies a lot of code.
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/
42b73d2d-da12-ba9f-570a-
420e0cce19d9@phystech.edu
Tom Lane [Wed, 9 Sep 2020 16:00:49 +0000 (12:00 -0400)]
Doc: adjust documentation related to index support functions.
Commit
15cb2bd27 neglected to make the running text match the
tables, leaving the reader with the strong impression that
we cannot count. Also, don't drop an unrelated para between
a table and the para describing it.
Tom Lane [Wed, 9 Sep 2020 15:53:39 +0000 (11:53 -0400)]
Minor fixes in docs and error messages.
Alexander Lakhin
Discussion: https://postgr.es/m/
ce7debdd-c943-d7a7-9b41-
687107b27831@gmail.com
Magnus Hagander [Wed, 9 Sep 2020 10:20:53 +0000 (12:20 +0200)]
Add missing quote in docs
Mistake in commit
68b603e1a9.
Reported-by: Ian Barwick
Peter Eisentraut [Wed, 9 Sep 2020 07:58:12 +0000 (09:58 +0200)]
Add some more numeric test coverage
max(numeric) wasn't tested at all, min(numeric) was only used by some
unrelated tests. Add explicit tests with the other numeric aggregate
functions.
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 <hawu@vmware.com>
Author: Amit Langote <amitlangote09@gmail.com>
Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/CA+HiwqFqBmcSSap4sFnCBUEL_VfOMmEKaQ3gwUhyfa4c7J_-nA@mail.gmail.com
Discussion: https://postgr.es/m/DM5PR0501MB3910E97A9EDFB4C775CF3D75A42F0@DM5PR0501MB3910.namprd05.prod.outlook.com
Tom Lane [Tue, 8 Sep 2020 19:54:25 +0000 (15:54 -0400)]
Install an error check into cancel_before_shmem_exit().
Historically, cancel_before_shmem_exit() just silently did nothing
if the specified callback wasn't the top-of-stack. The folly of
ignoring this case was exposed by the bugs fixed in
303640199 and
bab150045, so let's make it throw elog(ERROR) instead.
There is a decent argument to be made that PG_ENSURE_ERROR_CLEANUP
should use some separate infrastructure, so it wouldn't break if
something inside the guarded code decides to register a new
before_shmem_exit callback. However, a survey of the surviving
uses of before_shmem_exit() and PG_ENSURE_ERROR_CLEANUP doesn't
show any plausible conflicts of that sort today, so for now we'll
forgo the extra complexity. (It will almost certainly become
necessary if anyone ever wants to wrap PG_ENSURE_ERROR_CLEANUP
around arbitrary user-defined actions, though.)
No backpatch, since this is developer support not a production issue.
Bharath Rupireddy, per advice from Andres Freund, Robert Haas, and myself
Discussion: https://postgr.es/m/CALj2ACWk7j4F2v2fxxYfrroOF=AdFNPr1WsV+AGtHAFQOqm_pw@mail.gmail.com
Andres Freund [Tue, 8 Sep 2020 18:25:34 +0000 (11:25 -0700)]
Fix autovacuum cancellation.
The problem is caused by me (Andres) having ProcSleep() look at the
wrong PGPROC entry in
5788e258bb2.
Unfortunately it seems hard to write a reliable test for autovacuum
cancellations. Perhaps somebody will come up with a good approach, but
it seems worth fixing the issue even without a test.
Reported-By: Jeff Janes <jeff.janes@gmail.com>
Author: Jeff Janes <jeff.janes@gmail.com>
Discussion: https://postgr.es/m/CAMkU=1wH2aUy+wDRDz+5RZALdcUnEofV1t9PzXS_gBJO9vZZ0Q@mail.gmail.com
Tom Lane [Tue, 8 Sep 2020 15:47:37 +0000 (11:47 -0400)]
Use plain memset() in numeric.c, not MemSet and friends.
This essentially reverts a micro-optimization I made years ago,
as part of the much larger commit
d72f6c750. It's doubtful
that there was any hard evidence for it being helpful even then,
and the case is even more dubious now that modern compilers
are so much smarter about inlining memset().
The proximate reason for undoing it is to get rid of the type punning
inherent in MemSet, for fear that that may cause problems now that
we're applying additional optimization switches to numeric.c.
At the very least this'll silence some warnings from a few old
buildfarm animals.
(It's probably past time for another look at whether MemSet is still
worth anything at all, but I do not propose to tackle that question
right now.)
Discussion: https://postgr.es/m/CAJ3gD9evtA_vBo+WMYMyT-u=keHX7-r8p2w7OSRfXf42LTwCZQ@mail.gmail.com
Peter Eisentraut [Tue, 8 Sep 2020 15:11:16 +0000 (17:11 +0200)]
Use <unnamed> for name of unnamed portal's memory context
Otherwise just printing an empty string makes the memory context debug
output slightly confusing.
Discussion: https://www.postgresql.org/message-id/flat/
ccb353ef-89ff-09b3-8046-
1d2514624b9c%402ndquadrant.com
Peter Eisentraut [Tue, 8 Sep 2020 08:08:46 +0000 (10:08 +0200)]
Remove unused parameter
unused since
f0d6f20278b7c5c412ce40a9b86c6b31dc2fbfdd
Discussion: https://www.postgresql.org/message-id/flat/
511bb100-f829-ba21-2f10-
9f952ec06ead%402ndquadrant.com
Michael Paquier [Tue, 8 Sep 2020 02:15:21 +0000 (11:15 +0900)]
Remove isolation test reindex-partitions
The isolation test added by
a6642b3 is proving to be unstable, as once
the first transaction holding a lock on the top-most partitioned table
or on a partition commits, the commit order of the follow-up DROP TABLE
and REINDEX could become reversed depending on the timing.
The only part of the test that could be entirely reliable is the one
using a SHARE lock, allowing REINDEX to commit first, but it is the
least interesting of the set.
Per buildfarm members rorqual and mylodon.
Discussion: https://postgr.es/m/E1kFSBj-00062c-Mu@gemulon.postgresql.org
Michael Paquier [Tue, 8 Sep 2020 01:09:22 +0000 (10:09 +0900)]
Add support for partitioned tables and indexes in REINDEX
Until now, REINDEX was not able to work with partitioned tables and
indexes, forcing users to reindex partitions one by one. This extends
REINDEX INDEX and REINDEX TABLE so as they can accept a partitioned
index and table in input, respectively, to reindex all the partitions
assigned to them with physical storage (foreign tables, partitioned
tables and indexes are then discarded).
This shares some logic with schema and database REINDEX as each
partition gets processed in its own transaction after building a list of
relations to work on. This choice has the advantage to minimize the
number of invalid indexes to one partition with REINDEX CONCURRENTLY in
the event a cancellation or failure in-flight, as the only indexes
handled at once in a single REINDEX CONCURRENTLY loop are the ones from
the partition being working on.
Isolation tests are added to emulate some cases I bumped into while
developing this feature, particularly with the concurrent drop of a
leaf partition reindexed. However, this is rather limited as LOCK would
cause REINDEX to block in the first transaction building the list of
partitions.
Per its multi-transaction nature, this new flavor cannot run in a
transaction block, similarly to REINDEX SCHEMA, SYSTEM and DATABASE.
Author: Justin Pryzby, Michael Paquier
Reviewed-by: Anastasia Lubennikova
Discussion: https://postgr.es/m/
db12e897-73ff-467e-94cb-
4af03705435f.adger.lj@alibaba-inc.com
Jeff Davis [Mon, 7 Sep 2020 20:31:59 +0000 (13:31 -0700)]
Adjust cost model for HashAgg that spills to disk.
Tomas Vondra observed that the IO behavior for HashAgg tends to be
worse than for Sort. Penalize HashAgg IO costs accordingly.
Also, account for the CPU effort of spilling the tuples and reading
them back.
Discussion: https://postgr.es/m/
20200906212112.nzoy5ytrzjjodpfh@development
Reviewed-by: Tomas Vondra
Backpatch-through: 13