postgresql.git
4 years agoFix new GIST build code to work under CLOBBER_CACHE_ALWAYS.
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

4 years agoFix whitespace
Peter Eisentraut [Sun, 20 Sep 2020 12:42:54 +0000 (14:42 +0200)]
Fix whitespace

4 years agoRemove precedence hacks no longer needed without postfix operators.
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

4 years agoRemove unused parameters
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

4 years agoCode review for dynahash change.
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

4 years agoRemove large fill factor support from dynahash.c.
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

4 years agoAllow most keywords to be used as column labels without requiring AS.
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

4 years agopg_surgery: Try to stabilize regression tests.
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

4 years agoUpdate file header comments for logical/relation.c.
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

4 years agoFix comments in heapam.c.
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

4 years agoTry to stabilize output from rolenames regression test.
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

4 years agoRemove support for postfix (right-unary) operators.
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

4 years agoRemove factorial operators, leaving only the factorial() function.
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

4 years agoFurther improve pgindent's list of file exclusions.
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

4 years agoImprove common/logging.c's support for multiple verbosity levels.
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

4 years agoUpdate parallel BTree scan state when the scan keys can't be satisfied.
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

4 years agoAllow CURRENT_ROLE where CURRENT_USER is accepted
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

4 years agoAdd support for building GiST index by sorting.
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

4 years agodoc: Apply more consistently <productname> markup for OpenSSL
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

4 years agoImprove tab completion of IMPORT FOREIGN SCHEMA in psql
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

4 years agoTeach walsender to update its process title for replication commands.
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

4 years agoImprove formatting of create_help.pl and plperl_opmask.pl output.
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

4 years agoFix bogus completion tag usage in walsender
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

4 years agoCentralize setup of SIGQUIT handling for postmaster child processes.
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

4 years agoDon't fetch partition check expression during InitResultRelInfo.
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

4 years agoFix amcheck child check pg_upgrade bug.
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.

4 years agoAvoid unnecessary recursion to child tables in ALTER TABLE SET NOT NULL.
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

4 years agoFix bogus cache-invalidation logic in logical replication worker.
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

4 years agoAdd leader_pid field into the example of file_fdw for csvlog.
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

4 years agoAvoid retrieval of CHECK constraints and DEFAULT exprs in data-only dump
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

4 years agoChange LogicalTapeSetBlocks() to use nBlocksWritten.
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

4 years agoHashAgg: release write buffers sooner by rewinding tape.
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

4 years agoFix initialization of RelationSyncEntry for streaming transactions.
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

4 years agoOptimize compactify_tuples function
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

4 years agoFix use-after-free bug with event triggers in an extension script
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

4 years agoReport resource usage at the end of recovery
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

4 years agoAllow incremental sorts for windowing functions
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

4 years agoFix compiler warning
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

4 years agoMake walsenders show their replication commands in pg_stat_activity.
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

4 years agoFix interpolation in test name.
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

4 years agoFix typos.
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

4 years agoMake index_set_state_flags() transactional
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

4 years agoMessage fixes and style improvements
Peter Eisentraut [Mon, 14 Sep 2020 04:42:07 +0000 (06:42 +0200)]
Message fixes and style improvements

4 years agoAvoid useless allocations for information of dumpable objects in pg_dump/
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

4 years agoUse the properly transformed RangeVar for expandTableLikeClause().
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

4 years agoFix inconsistency in determining the timestamp of the db statfile.
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

4 years agoRemove unused function declaration in logicalproto.h.
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

4 years agologtape.c: do not preallocate for tapes when sorting
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

4 years agoAccept SIGQUIT during error recovery in auxiliary processes.
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

4 years agopsql: Display stats target of extended statistics
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>
4 years agoLog a message when resorting to SIGKILL during shutdown/crash recovery.
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

4 years agoDon't run atexit callbacks during signal exits from ProcessStartupPacket.
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

4 years agoUpdate copyright year
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

4 years agoSkip empty transaction stream in test_decoding.
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

4 years agoPrint WAL logical message contents in pg_waldump
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

4 years agoUse _exit(2) for SIGQUIT during ProcessStartupPacket, too.
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

4 years agoNew contrib module, pg_surgery, with heap surgery functions.
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

4 years agoRemove unused parameter
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

4 years agoAdd libpq's openssl dependencies to pkg-config file
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

4 years agodoc: Remove buggy ICU collation from documentation
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

4 years agoAdd more tests for EXTRACT of date type
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

4 years agoFix title in reference section
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

4 years agoClean up some code and comments in partbounds.c.
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

4 years agodoc: Fix some grammar and inconsistencies
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

4 years agoFix rd_firstRelfilenodeSubid for nailed relations, in parallel workers.
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

4 years agoMake archiver's SIGQUIT handler exit via _exit().
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

4 years agoExpose internal function for converting int64 to numeric
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

4 years agoDoc: adjust documentation related to index support functions.
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.

4 years agoMinor fixes in docs and error messages.
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

4 years agoAdd missing quote in docs
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
4 years agoAdd some more numeric test coverage
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.

4 years agoCheck default partitions constraints while descending
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

4 years agoInstall an error check into cancel_before_shmem_exit().
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

4 years agoFix autovacuum cancellation.
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

4 years agoUse plain memset() in numeric.c, not MemSet and friends.
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

4 years agoUse <unnamed> for name of unnamed portal's memory context
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

4 years agoRemove unused parameter
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

4 years agoRemove isolation test reindex-partitions
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

4 years agoAdd support for partitioned tables and indexes in REINDEX
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

4 years agoAdjust cost model for HashAgg that spills to disk.
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

4 years agoClarify comments in enforce_generic_type_consistency().
Tom Lane [Mon, 7 Sep 2020 18:52:33 +0000 (14:52 -0400)]
Clarify comments in enforce_generic_type_consistency().

Some of the pre-existing comments were vague about whether they
referred to all polymorphic types or only the old-style ones.

Also be more consistent about using the "family 1" vs "family 2"
terminology.

Himanshu Upadhyaya and Tom Lane

Discussion: https://postgr.es/m/CAPF61jBUg9XoMPNuLpoZ+h6UZ2VxKdNt3rQL1xw1GOBwjWzAXQ@mail.gmail.com

4 years agoFrob numeric.c loop so that clang will auto-vectorize it too.
Tom Lane [Mon, 7 Sep 2020 16:03:04 +0000 (12:03 -0400)]
Frob numeric.c loop so that clang will auto-vectorize it too.

Experimentation shows that clang will auto-vectorize the critical
multiplication loop if the termination condition is written "i2 < limit"
rather than "i2 <= limit".  This seems unbelievably stupid, but I've
reproduced it on both clang 9.0.1 (RHEL8) and 11.0.3 (macOS Catalina).
gcc doesn't care, so tweak the code to do it that way.

Discussion: https://postgr.es/m/CAJ3gD9evtA_vBo+WMYMyT-u=keHX7-r8p2w7OSRfXf42LTwCZQ@mail.gmail.com

4 years agoAdd d_type to our Windows dirent emulation.
Thomas Munro [Mon, 7 Sep 2020 11:18:22 +0000 (23:18 +1200)]
Add d_type to our Windows dirent emulation.

This allows us to skip some stat calls, by extending commit 861c6e7c to
cover Windows systems.

Author: Juan José Santamaría Flecha <juanjo.santamaria@gmail.com>
Reviewed-by: Alvaro Herrera <alvherre@2ndquadrant.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Magnus Hagander <magnus@hagander.net>
Reviewed-by: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://postgr.es/m/CA%2BhUKG%2BFzxupGGN4GpUdbzZN%2Btn6FQPHo8w0Q%2BAPH5Wz8RG%2Bww%40mail.gmail.com

4 years agodoc: Fix table cell overflow
Peter Eisentraut [Mon, 7 Sep 2020 07:59:50 +0000 (09:59 +0200)]
doc: Fix table cell overflow

Fix one instance of a table cell overflow by adding a zero-width
space.  The visual impact of this is minimal, but since this is
currently the only such case reported by FOP ("contents of ... exceed
the available area"), it seems worth getting rid of.

4 years agoSkip unnecessary stat() calls in walkdir().
Thomas Munro [Mon, 7 Sep 2020 06:11:46 +0000 (18:11 +1200)]
Skip unnecessary stat() calls in walkdir().

Some kernels can tell us the type of a "dirent", so we can avoid a call
to stat() or lstat() in many cases.  Define a new function
get_dirent_type() to contain that logic, for use by the backend and
frontend versions of walkdir(), and perhaps other callers in future.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Juan José Santamaría Flecha <juanjo.santamaria@gmail.com>
Discussion: https://postgr.es/m/CA%2BhUKG%2BFzxupGGN4GpUdbzZN%2Btn6FQPHo8w0Q%2BAPH5Wz8RG%2Bww%40mail.gmail.com

4 years agodoc: Tweak sentence for pg_checksums when enabling checksums
Michael Paquier [Mon, 7 Sep 2020 05:34:59 +0000 (14:34 +0900)]
doc: Tweak sentence for pg_checksums when enabling checksums

The previous version of the docs mentioned that files are rewritten,
implying that a second copy of each file gets created, but each file is
updated in-place.

Author: Michael Banck
Reviewed-by: Daniel Gustafsson, Michael Paquier
Discussion: https://postgr.es/m/858086b6a42fb7d17995b6175856f7e7ec44d0a2.camel@credativ.de
Backpatch-through: 12

4 years agoAdd additional tests to test streaming of in-progress transactions.
Amit Kapila [Mon, 7 Sep 2020 02:38:58 +0000 (08:08 +0530)]
Add additional tests to test streaming of in-progress transactions.

This covers the functionality tests for streaming in-progress
subtransactions, streaming transactions containing rollback to savepoints,
and streaming transactions having DDLs.

Author: Tomas Vondra, Amit Kapila and Dilip Kumar
Reviewed-by: Dilip Kumar
Discussion: https://postgr.es/m/688b0b7f-2f6c-d827-c27b-216a8e3ea700@2ndquadrant.com

4 years agoApply auto-vectorization to the inner loop of numeric multiplication.
Tom Lane [Mon, 7 Sep 2020 01:40:39 +0000 (21:40 -0400)]
Apply auto-vectorization to the inner loop of numeric multiplication.

Compile numeric.c with -ftree-vectorize where available, and adjust
the innermost loop of mul_var() so that it is amenable to being
auto-vectorized.  (Mainly, that involves making it process the arrays
left-to-right not right-to-left.)

Applying -ftree-vectorize actually makes numeric.o smaller, at least
with my compiler (gcc 8.3.1 on x86_64), and it's a little faster too.
Independently of that, fixing the inner loop to be vectorizable also
makes things a bit faster.  But doing both is a huge win for
multiplications with lots of digits.  For me, the numeric regression
test is the same speed to within measurement noise, but numeric_big
is a full 45% faster.

We also looked into applying -funroll-loops, but that makes numeric.o
bloat quite a bit, and the additional speed improvement is very
marginal.

Amit Khandekar, reviewed and edited a little by me

Discussion: https://postgr.es/m/CAJ3gD9evtA_vBo+WMYMyT-u=keHX7-r8p2w7OSRfXf42LTwCZQ@mail.gmail.com

4 years agoSplit Makefile symbol CFLAGS_VECTOR into two symbols.
Tom Lane [Mon, 7 Sep 2020 01:28:16 +0000 (21:28 -0400)]
Split Makefile symbol CFLAGS_VECTOR into two symbols.

Replace CFLAGS_VECTOR with CFLAGS_UNROLL_LOOPS and CFLAGS_VECTORIZE,
allowing us to distinguish whether we want to apply -funroll-loops,
-ftree-vectorize, or both to a particular source file.  Up to now
the only consumer of the symbol has been checksum.c which wants
both, so that there was no need to distinguish; but that's about
to change.

Amit Khandekar, reviewed and edited a little by me

Discussion: https://postgr.es/m/CAJ3gD9evtA_vBo+WMYMyT-u=keHX7-r8p2w7OSRfXf42LTwCZQ@mail.gmail.com

4 years agoRemove arbitrary line length limits in pg_regress (plain and ECPG).
Tom Lane [Sun, 6 Sep 2020 18:13:02 +0000 (14:13 -0400)]
Remove arbitrary line length limits in pg_regress (plain and ECPG).

Refactor replace_string() to use a StringInfo for the modifiable
string argument.  This allows the string to be of indefinite size
initially and/or grow substantially during replacement.  The previous
logic in convert_sourcefiles_in() had a hard-wired limit of 1024
bytes on any line in input/*.sql or output/*.out files.  While we've
not had reports of trouble yet, it'd surely have bit us someday.

This also fixes replace_string() so it won't get into an infinite
loop if the string-to-be-replaced is a substring of the replacement.
That's unlikely to happen in current usage, but the function surely
shouldn't depend on it.

Also fix ecpg_filter() to use a StringInfo and thereby remove its
hard limit of 300 bytes on the length of an ecpg source line.

Asim Rama Praveen and Georgios Kokolatos,
reviewed by Alvaro Herrera and myself

Discussion: https://postgr.es/m/y9Dlk2QhiZ39DhaB1QE9mgZ95HcOQKZCNtGwN7XCRKMdBRBnX_0woaRUtTjloEp4PKA6ERmcUcfq3lPGfKPOJ5xX2TV-5WoRYyySeNHRzdw=@protonmail.com

4 years agoRefactor pg_get_line() to expose an alternative StringInfo-based API.
Tom Lane [Sun, 6 Sep 2020 17:57:10 +0000 (13:57 -0400)]
Refactor pg_get_line() to expose an alternative StringInfo-based API.

Letting the caller provide a StringInfo to read into is helpful when
the caller needs to merge lines or otherwise modify the data after
it's been read.  Notably, now the code added by commit 8f8154a50
can use pg_get_line_append() instead of having its own copy of that
logic.  A follow-on commit will also make use of this.

Also, since StringInfo buffers are a minimum of 1KB long, blindly
using pg_get_line() in a loop can eat a lot more memory than one would
expect.  I discovered for instance that commit e0f05cd5b caused initdb
to consume circa 10MB to read postgres.bki, even though that's under
1MB worth of data.  A less memory-hungry alternative is to re-use the
same StringInfo for all lines and pg_strdup the results.

Discussion: https://postgr.es/m/1315832.1599345736@sss.pgh.pa.us

4 years agoChange path in example of file_fdw for logs
Magnus Hagander [Sun, 6 Sep 2020 17:28:32 +0000 (19:28 +0200)]
Change path in example of file_fdw for logs

It's better to use a relative path into the data directory, than to a
hardcoded home directory of user 'josh'.

Discussion: https://postgr.es/m/CABUevEyuf67Yu_r9gpDMs5MKifK7+-+pe=ZjKzya4JEn9kUk1w@mail.gmail.com

4 years agoFix typo in comment
Magnus Hagander [Sun, 6 Sep 2020 17:26:55 +0000 (19:26 +0200)]
Fix typo in comment

Author: Hou, Zhijie

4 years agoFix misleading error message about inconsistent moving-aggregate types.
Tom Lane [Sun, 6 Sep 2020 16:55:13 +0000 (12:55 -0400)]
Fix misleading error message about inconsistent moving-aggregate types.

We reported the wrong types when complaining that an aggregate's
moving-aggregate implementation is inconsistent with its regular
implementation.

This was wrong since the feature was introduced, so back-patch
to all supported branches.

Jeff Janes

Discussion: https://postgr.es/m/CAMkU=1x808LH=LPhZp9mNSP0Xd1xDqEd+XeGcvEe48dfE6xV=A@mail.gmail.com

4 years agoRemove useless lstat() call in pg_rewind.
Tom Lane [Sun, 6 Sep 2020 15:50:40 +0000 (11:50 -0400)]
Remove useless lstat() call in pg_rewind.

This is duplicative of an lstat that was just done by the calling
function (traverse_datadir), besides which we weren't really doing
anything with the results.  There's not much point in checking to
see if someone removed the file since the previous lstat, since the
FILE_ACTION_REMOVE code would have to deal with missing-file cases
anyway.  Moreover, the "exists = false" assignment was a dead store;
nothing was done with that value later.

A syscall saved is a syscall earned, so back-patch to 9.5
where this code was introduced.

Discussion: https://postgr.es/m/1221796.1599329320@sss.pgh.pa.us

4 years agodoc: Don't hide the "Up" link when it is the same as "Home"
Peter Eisentraut [Sun, 6 Sep 2020 14:46:13 +0000 (16:46 +0200)]
doc: Don't hide the "Up" link when it is the same as "Home"

The original stylesheets seemed to think this was a good idea, but our
users find it confusing and unhelpful, so undo that logic.

Reported-by: Fabien COELHO <coelho@cri.ensmp.fr>
Discussion: https://www.postgresql.org/message-id/flat/alpine.DEB.2.22.394.2006210914370.859381%40pseudo

4 years agoRemove unused parameter
Peter Eisentraut [Sun, 6 Sep 2020 07:32:16 +0000 (09:32 +0200)]
Remove unused parameter

unused since 84d723b6cefcf25b8c800f8aa6cf3c9538a546b4

Discussion: https://www.postgresql.org/message-id/flat/511bb100-f829-ba21-2f10-9f952ec06ead%402ndquadrant.com

4 years agoImprove some ancient, crufty code in bootstrap + initdb.
Tom Lane [Sat, 5 Sep 2020 20:20:04 +0000 (16:20 -0400)]
Improve some ancient, crufty code in bootstrap + initdb.

At some point back in the last century, somebody felt that reading
all of pg_type twice was cheaper, or at least easier, than using
repalloc() to resize the Typ[] array dynamically.  That seems like an
entirely wacko proposition, so rewrite the code to do it the other
way.  (To add insult to injury, there were two not-quite-identical
copies of said code.)

initdb.c's readfile() function had the same disease of preferring
to do double the I/O to avoid resizing its output array.  Here,
we can make things easier by using the just-invented pg_get_line()
function to handle reading individual lines without a predetermined
notion of how long they are.

On my machine, it's difficult to detect any net change in the
overall runtime of initdb from these changes; but they should
help on slower buildfarm machines (especially since a buildfarm
cycle involves a lot of initdb's these days).

My attention was drawn to these places by scan-build complaints,
but on inspection they needed a lot more work than just suppressing
dead stores :-(

4 years agoYet more elimination of dead stores and useless initializations.
Tom Lane [Sat, 5 Sep 2020 17:17:32 +0000 (13:17 -0400)]
Yet more elimination of dead stores and useless initializations.

I'm not sure what tool Ranier was using, but the ones I contributed
were found by using a newer version of scan-build than I tried before.

Ranier Vilela and Tom Lane

Discussion: https://postgr.es/m/CAEudQAo1+AcGppxDSg8k+zF4+Kv+eJyqzEDdbpDg58-=MQcerQ@mail.gmail.com

4 years agoSwitch to multi-inserts when registering dependencies for many code paths
Michael Paquier [Sat, 5 Sep 2020 12:33:53 +0000 (21:33 +0900)]
Switch to multi-inserts when registering dependencies for many code paths

This commit improves the dependency registrations by taking advantage of
the preliminary work done in 63110c62, to group together the insertion
of dependencies of the same type to pg_depend.  With the current layer
of routines available, and as only dependencies of the same type can be
grouped, there are code paths still doing more than one multi-insert
when it is necessary to register dependencies of multiple types
(constraint and index creation are two cases doing that).

While on it, this refactors some of the code to use ObjectAddressSet()
when manipulating object addresses.

Author: Daniel Gustafsson, Michael Paquier
Reviewed-by: Andres Freund, Álvaro Herrera
Discussion: https://postgr.es/m/20200807061619.GA23955@paquier.xyz