]> granicus.if.org Git - postgresql/log
postgresql
6 years agoDoc: fix incorrectly stated argument list for pgcrypto's hmac() function.
Tom Lane [Sun, 5 Aug 2018 17:03:42 +0000 (13:03 -0400)]
Doc: fix incorrectly stated argument list for pgcrypto's hmac() function.

The bytea variant takes (bytea, bytea, text).
Per unsigned report.

Discussion: https://postgr.es/m/153344327294.1404.654155870612982042@wrigleys.postgresql.org

6 years agoRemove now unused check for HAVE_X509_GET_SIGNATURE_NID in test.
Heikki Linnakangas [Sun, 5 Aug 2018 14:16:12 +0000 (17:16 +0300)]
Remove now unused check for HAVE_X509_GET_SIGNATURE_NID in test.

I removed the code that used this in the previous commit.

Spotted by Michael Paquier.

6 years agoRemove support for tls-unique channel binding.
Heikki Linnakangas [Sun, 5 Aug 2018 10:44:21 +0000 (13:44 +0300)]
Remove support for tls-unique channel binding.

There are some problems with the tls-unique channel binding type. It's not
supported by all SSL libraries, and strictly speaking it's not defined for
TLS 1.3 at all, even though at least in OpenSSL, the functions used for it
still seem to work with TLS 1.3 connections. And since we had no
mechanism to negotiate what channel binding type to use, there would be
awkward interoperability issues if a server only supported some channel
binding types. tls-server-end-point seems feasible to support with any SSL
library, so let's just stick to that.

This removes the scram_channel_binding libpq option altogether, since there
is now only one supported channel binding type.

This also removes all the channel binding tests from the SSL test suite.
They were really just testing the scram_channel_binding option, which
is now gone. Channel binding is used if both client and server support it,
so it is used in the existing tests. It would be good to have some tests
specifically for channel binding, to make sure it really is used, and the
different combinations of a client and a server that support or doesn't
support it. The current set of settings we have make it hard to write such
tests, but I did test those things manually, by disabling
HAVE_BE_TLS_GET_CERTIFICATE_HASH and/or
HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH.

I also removed the SCRAM_CHANNEL_BINDING_TLS_END_POINT constant. This is a
matter of taste, but IMO it's more readable to just use the
"tls-server-end-point" string.

Refactor the checks on whether the SSL library supports the functions
needed for tls-server-end-point channel binding. Now the server won't
advertise, and the client won't choose, the SCRAM-SHA-256-PLUS variant, if
compiled with an OpenSSL version too old to support it.

In the passing, add some sanity checks to check that the chosen SASL
mechanism, SCRAM-SHA-256 or SCRAM-SHA-256-PLUS, matches whether the SCRAM
exchange used channel binding or not. For example, if the client selects
the non-channel-binding variant SCRAM-SHA-256, but in the SCRAM message
uses channel binding anyway. It's harmless from a security point of view,
I believe, and I'm not sure if there are some other conditions that would
cause the connection to fail, but it seems better to be strict about these
things and check explicitly.

Discussion: https://www.postgresql.org/message-id/ec787074-2305-c6f4-86aa-6902f98485a4%40iki.fi

6 years agoUpdate version 11 release notes.
Tom Lane [Sun, 5 Aug 2018 03:49:53 +0000 (23:49 -0400)]
Update version 11 release notes.

Remove description of commit 1944cdc98, which has now been back-patched
so it's not relevant to v11 any longer.  Add descriptions of other
recent commits that seemed worth mentioning.

I marked the update as stopping at 2018-07-30, because it's unclear
whether d06eebce5 will be allowed to stay in v11, and I didn't feel like
putting effort into writing a description of it yet.  If it does stay,
I think it will deserve mention in the Source Code section.

6 years agoFix INSERT ON CONFLICT UPDATE through a view that isn't just SELECT *.
Tom Lane [Sat, 4 Aug 2018 23:38:58 +0000 (19:38 -0400)]
Fix INSERT ON CONFLICT UPDATE through a view that isn't just SELECT *.

When expanding an updatable view that is an INSERT's target, the rewriter
failed to rewrite Vars in the ON CONFLICT UPDATE clause.  This accidentally
worked if the view was just "SELECT * FROM ...", as the transformation
would be a no-op in that case.  With more complicated view targetlists,
this omission would often lead to "attribute ... has the wrong type" errors
or even crashes, as reported by Mario De Frutos Dieguez.

Fix by adding code to rewriteTargetView to fix up the data structure
correctly.  The easiest way to update the exclRelTlist list is to rebuild
it from scratch looking at the new target relation, so factor the code
for that out of transformOnConflictClause to make it sharable.

In passing, avoid duplicate permissions checks against the EXCLUDED
pseudo-relation, and prevent useless view expansion of that relation's
dummy RTE.  The latter is only known to happen (after this patch) in cases
where the query would fail later due to not having any INSTEAD OF triggers
for the view.  But by exactly that token, it would create an unintended
and very poorly tested state of the query data structure, so it seems like
a good idea to prevent it from happening at all.

This has been broken since ON CONFLICT was introduced, so back-patch
to 9.5.

Dean Rasheed, based on an earlier patch by Amit Langote;
comment-kibitzing and back-patching by me

Discussion: https://postgr.es/m/CAFYwGJ0xfzy8jaK80hVN2eUWr6huce0RU8AgU04MGD00igqkTg@mail.gmail.com

6 years agoReset properly errno before calling write()
Michael Paquier [Sat, 4 Aug 2018 20:31:18 +0000 (05:31 +0900)]
Reset properly errno before calling write()

6cb3372 enforces errno to ENOSPC when less bytes than what is expected
have been written when it is unset, though it forgot to properly reset
errno before doing a system call to write(), causing errno to
potentially come from a previous system call.

Reported-by: Tom Lane
Author: Michael Paquier
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/31797.1533326676@sss.pgh.pa.us

6 years agoMake "kerberos" test suite independent of "localhost" name resolution.
Noah Misch [Sat, 4 Aug 2018 03:53:25 +0000 (20:53 -0700)]
Make "kerberos" test suite independent of "localhost" name resolution.

This suite malfunctioned if the canonical name of "localhost" was
something other than "localhost", such as "localhost.localdomain".  Use
hostaddr=127.0.0.1 and a fictitious host=, so the resolver's answers for
"localhost" don't affect the outcome.  Back-patch to v11, which
introduced this test suite.

Discussion: https://postgr.es/m/20180801050903.GA1392916@rfd.leadboat.com

6 years agoAdd table relcache invalidation to index builds.
Peter Geoghegan [Fri, 3 Aug 2018 21:45:11 +0000 (14:45 -0700)]
Add table relcache invalidation to index builds.

It's necessary to make sure that owning tables have a relcache
invalidation prior to advancing the command counter to make
newly-entered catalog tuples for the index visible.  inval.c must be
able to maintain the consistency of the local caches in the event of
transaction abort.  There is usually only a problem when CREATE INDEX
transactions abort, since there is a generic invalidation once we reach
index_update_stats().

This bug is of long standing.  Problems were made much more likely by
the addition of parallel CREATE INDEX (commit 9da0cc35284), but it is
strongly suspected that similar problems can be triggered without
involving plan_create_index_workers().  (plan_create_index_workers()
triggers a relcache build or rebuild, which previously only happened in
rare edge cases.)

Author: Peter Geoghegan
Reported-By: Luca Ferrari
Diagnosed-By: Andres Freund
Reviewed-By: Andres Freund
Discussion: https://postgr.es/m/CAKoxK+5fVodiCtMsXKV_1YAKXbzwSfp7DgDqUmcUAzeAhf=HEQ@mail.gmail.com
Backpatch: 9.3-

6 years agoFirst-draft release notes for 10.5.
Tom Lane [Fri, 3 Aug 2018 22:08:57 +0000 (18:08 -0400)]
First-draft release notes for 10.5.

As usual, the release notes for other branches will be made by cutting
these down, but put them up for community review first.

6 years agoAdd 'n' to list of possible values to pg_default_acl.defaclobjtype
Alvaro Herrera [Fri, 3 Aug 2018 20:45:08 +0000 (16:45 -0400)]
Add 'n' to list of possible values to pg_default_acl.defaclobjtype

This was missed in commit ab89e465cb20; backpatch to v10.

Author: Fabien Coelho <coelho@cri.ensmp.fr>
Discussion: https://postgr.es/m/alpine.DEB.2.21.1807302243001.13230@lancre

6 years agoFix pg_replication_slot example output
Alvaro Herrera [Fri, 3 Aug 2018 20:34:59 +0000 (16:34 -0400)]
Fix pg_replication_slot example output

The example output of pg_replication_slot is wrong.  Correct it and make
the output stable by explicitly listing columns to output.

Author: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/20180731.190909.42582169.horiguchi.kyotaro@lab.ntt.co.jp

6 years agoRemove no-longer-appropriate special case in psql's \conninfo code.
Tom Lane [Fri, 3 Aug 2018 16:20:47 +0000 (12:20 -0400)]
Remove no-longer-appropriate special case in psql's \conninfo code.

\conninfo prints the results of PQhost() and some other libpq functions.
It used to override the PQhost() result with the hostaddr parameter if
that'd been given, but that's unhelpful when multiple hosts were listed
in the connection string.  Furthermore, it seems unnecessary in the wake
of commit 1944cdc98, since PQhost does any useful substitution itself.
So let's just remove the extra code and print PQhost()'s result without
any editorialization.

Back-patch to v10, as 1944cdc98 (just) was.

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

6 years agoChange libpq's internal uses of PQhost() to inspect host field directly.
Tom Lane [Fri, 3 Aug 2018 16:12:10 +0000 (12:12 -0400)]
Change libpq's internal uses of PQhost() to inspect host field directly.

Commit 1944cdc98 changed PQhost() to return the hostaddr value when that
is specified and host isn't.  This is a good idea in general, but
fe-auth.c and related files contain PQhost() calls for which it isn't.
Specifically, when we compare SSL certificates or other server identity
information to the host field, we do not want to use hostaddr instead;
that's not what's documented, that's not what happened pre-v10, and
it doesn't seem like a good idea.

Instead, we can just look at connhost[].host directly.  This does what
we want in v10 and up; in particular, if neither host nor hostaddr
were given, the host field will be replaced with the default host name.
That seems useful, and it's likely the reason that these places were
coded to call PQhost() originally (since pre-v10, the stored field was
not replaced with the default).

Back-patch to v10, as 1944cdc98 (just) was.

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

6 years agoFix buffer usage stats for parallel nodes.
Amit Kapila [Fri, 3 Aug 2018 05:32:02 +0000 (11:02 +0530)]
Fix buffer usage stats for parallel nodes.

The buffer usage stats is accounted only for the execution phase of the
node.  For Gather and Gather Merge nodes, such stats are accumulated at
the time of shutdown of workers which is done after execution of node due
to which we missed to account them for such nodes.  Fix it by treating
nodes as running while we shut down them.

We can also miss accounting for a Limit node when Gather or Gather Merge
is beneath it, because it can finish the execution before shutting down
such nodes.  So we allow a Limit node to shut down the resources before it
completes the execution.

In the passing fix the gather node code to allow workers to shut down as
soon as we find that all the tuples from the workers have been retrieved.
The original code use to do that, but is accidently removed by commit
01edb5c7fc.

Reported-by: Adrien Nayrat
Author: Amit Kapila and Robert Haas
Reviewed-by: Robert Haas and Andres Freund
Backpatch-through: 9.6 where this code was introduced
Discussion: https://postgr.es/m/86137f17-1dfb-42f9-7421-82fd786b04a1@anayrat.info

6 years agoMatch the buffer usage tracking for leader and worker backends.
Amit Kapila [Fri, 3 Aug 2018 03:41:37 +0000 (09:11 +0530)]
Match the buffer usage tracking for leader and worker backends.

In the leader backend, we don't track the buffer usage for ExecutorStart
phase whereas in worker backend we track it for ExecutorStart phase as
well.  This leads to different value for buffer usage stats for the
parallel and non-parallel query.  Change the code so that worker backend
also starts tracking buffer usage after ExecutorStart.

Author: Amit Kapila and Robert Haas
Reviewed-by: Robert Haas and Andres Freund
Backpatch-through: 9.6 where this code was introduced
Discussion: https://postgr.es/m/86137f17-1dfb-42f9-7421-82fd786b04a1@anayrat.info

6 years agoAdd missing header include to pmsignal.h.
Thomas Munro [Wed, 1 Aug 2018 23:25:31 +0000 (11:25 +1200)]
Add missing header include to pmsignal.h.

pmsignal.h uses sig_atomic_t in some builds, but relied on signal.h
having been included already.  We could include it conditionally
but evidently that wouldn't save anything in practice and would
add more ugly macros, so let's just include signal.h always.

Reported-by: Tom Lane
Discussion: https://postgr.es/m/4166.1533154074%40sss.pgh.pa.us

6 years agoFix run-time partition pruning for appends with multiple source rels.
Tom Lane [Wed, 1 Aug 2018 23:42:46 +0000 (19:42 -0400)]
Fix run-time partition pruning for appends with multiple source rels.

The previous coding here supposed that if run-time partitioning applied to
a particular Append/MergeAppend plan, then all child plans of that node
must be members of a single partitioning hierarchy.  This is totally wrong,
since an Append could be formed from a UNION ALL: we could have multiple
hierarchies sharing the same Append, or child plans that aren't part of any
hierarchy.

To fix, restructure the related plan-time and execution-time data
structures so that we can have a separate list or array for each
partitioning hierarchy.  Also track subplans that are not part of any
hierarchy, and make sure they don't get pruned.

Per reports from Phil Florent and others.  Back-patch to v11, since
the bug originated there.

David Rowley, with a lot of cosmetic adjustments by me; thanks also
to Amit Langote for review.

Discussion: https://postgr.es/m/HE1PR03MB17068BB27404C90B5B788BCABA7B0@HE1PR03MB1706.eurprd03.prod.outlook.com

6 years agoFix logical replication slot initialization
Alvaro Herrera [Wed, 1 Aug 2018 21:39:07 +0000 (17:39 -0400)]
Fix logical replication slot initialization

This was broken in commit 9c7d06d60680, which inadvertently gave the
wrong value to fast_forward in one StartupDecodingContext call.  Fix by
flipping the value.  Add a test for the obvious error, namely trying to
initialize a replication slot with an nonexistent output plugin.

While at it, move the CreateDecodingContext call earlier, so that any
errors are reported before sending the CopyBoth message.

Author: Dave Cramer <davecramer@gmail.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CADK3HHLVkeRe1v4P02-5hj55H3_yJg3AEtpXyEY5T3wuzO2jSg@mail.gmail.com

6 years agoFix unnoticed variable shadowing in previous commit
Alvaro Herrera [Wed, 1 Aug 2018 21:04:57 +0000 (17:04 -0400)]
Fix unnoticed variable shadowing in previous commit

Per buildfarm.

6 years agoFix per-tuple memory leak in partition tuple routing
Alvaro Herrera [Wed, 1 Aug 2018 19:06:47 +0000 (15:06 -0400)]
Fix per-tuple memory leak in partition tuple routing

Some operations were being done in a longer-lived memory context,
causing intra-query leaks.  It's not noticeable unless you're doing a
large COPY, but if you are, it eats enough memory to cause a problem.

Co-authored-by: Kohei KaiGai <kaigai@heterodb.com>
Co-authored-by: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Co-authored-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/CAOP8fzYtVFWZADq4c=KoTAqgDrHWfng+AnEPEZccyxqxPVbbWQ@mail.gmail.com

6 years agoFix libpq's code for searching .pgpass; rationalize empty-list-item cases.
Tom Lane [Wed, 1 Aug 2018 16:30:36 +0000 (12:30 -0400)]
Fix libpq's code for searching .pgpass; rationalize empty-list-item cases.

Before v10, we always searched ~/.pgpass using the host parameter,
and nothing else, to match to the "hostname" field of ~/.pgpass.
(However, null host or host matching DEFAULT_PGSOCKET_DIR was replaced by
"localhost".)  In v10, this got broken by commit 274bb2b38, repaired by
commit bdac9836d, and broken again by commit 7b02ba62e; in the code
actually shipped, we'd search with hostaddr if both that and host were
specified --- though oddly, *not* if only hostaddr were specified.
Since this is directly contrary to the documentation, and not
backwards-compatible, it's clearly a bug.

However, the change wasn't totally without justification, even though it
wasn't done quite right, because the pre-v10 behavior has arguably been
buggy since we added hostaddr.  If hostaddr is specified and host isn't,
the pre-v10 code will search ~/.pgpass for "localhost", and ship that
password off to a server that most likely isn't local at all.  That's
unhelpful at best, and could be a security breach at worst.

Therefore, rather than just revert to that old behavior, let's define
the behavior as "search with host if provided, else with hostaddr if
provided, else search for localhost".  (As before, a host name matching
DEFAULT_PGSOCKET_DIR is replaced by localhost.)  This matches the
behavior of the actual connection code, so that we don't pick up an
inappropriate password; and it allows useful searches to happen when
only hostaddr is given.

While we're messing around here, ensure that empty elements within a
host or hostaddr list select the same behavior as a totally-empty
field would; for instance "host=a,,b" is equivalent to "host=a,/tmp,b"
if DEFAULT_PGSOCKET_DIR is /tmp.  Things worked that way in some cases
already, but not consistently so, which contributed to the confusion
about what key ~/.pgpass would get searched with.

Update documentation accordingly, and also clarify some nearby text.

Back-patch to v10 where the host/hostaddr list functionality was
introduced.

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

6 years agoUpdate parallel.sgml for Parallel Append
Robert Haas [Wed, 1 Aug 2018 12:14:05 +0000 (08:14 -0400)]
Update parallel.sgml for Parallel Append

Patch by me, reviewed by Thomas Munro, in response to a complaint
from Adrien Nayrat.

Discussion: http://postgr.es/m/baa0d036-7349-f722-ef88-2d8bb3413045@anayrat.info

6 years agoAllow multi-inserts during COPY into a partitioned table
Peter Eisentraut [Wed, 1 Aug 2018 08:23:09 +0000 (10:23 +0200)]
Allow multi-inserts during COPY into a partitioned table

CopyFrom allows multi-inserts to be used for non-partitioned tables, but
this was disabled for partitioned tables.  The reason for this appeared
to be that the tuple may not belong to the same partition as the
previous tuple did.  Not allowing multi-inserts here greatly slowed down
imports into partitioned tables.  These could take twice as long as a
copy to an equivalent non-partitioned table.  It seems wise to do
something about this, so this change allows the multi-inserts by
flushing the so-far inserted tuples to the partition when the next tuple
does not belong to the same partition, or when the buffer fills.  This
improves performance when the next tuple in the stream commonly belongs
to the same partition as the previous tuple.

In cases where the target partition changes on every tuple, using
multi-inserts slightly slows the performance.  To get around this we
track the average size of the batches that have been inserted and
adaptively enable or disable multi-inserts based on the size of the
batch.  Some testing was done and the regression only seems to exist
when the average size of the insert batch is close to 1, so let's just
enable multi-inserts when the average size is at least 1.3.  More
performance testing might reveal a better number for, this, but since
the slowdown was only 1-2% it does not seem critical enough to spend too
much time calculating it.  In any case it may depend on other factors
rather than just the size of the batch.

Allowing multi-inserts for partitions required a bit of work around the
per-tuple memory contexts as we must flush the tuples when the next
tuple does not belong the same partition.  In which case there is no
good time to reset the per-tuple context, as we've already built the new
tuple by this time.  In order to work around this we maintain two
per-tuple contexts and just switch between them every time the partition
changes and reset the old one.  This does mean that the first of each
batch of tuples is not allocated in the same memory context as the
others, but that does not matter since we only reset the context once
the previous batch has been inserted.

Author: David Rowley <david.rowley@2ndquadrant.com>
Reviewed-by: Melanie Plageman <melanieplageman@gmail.com>
6 years agopg_upgrade: fix --check for live source server checks
Bruce Momjian [Tue, 31 Jul 2018 22:10:06 +0000 (18:10 -0400)]
pg_upgrade:  fix --check for live source server checks

Fix for commit 244142d32afd02e7408a2ef1f249b00393983822.

Backpatch-through: 9.3

6 years agoProvide for contrib and pgxs modules to install include files.
Andrew Gierth [Tue, 31 Jul 2018 18:58:39 +0000 (19:58 +0100)]
Provide for contrib and pgxs modules to install include files.

This allows out-of-tree PLs and similar code to get access to
definitions needed to work with extension data types.

The following existing modules now install headers: contrib/cube,
contrib/hstore, contrib/isn, contrib/ltree, contrib/seg.

Discussion: https://postgr.es/m/87y3euomjh.fsf%40news-spur.riddles.org.uk

6 years agoFurther fixes for quoted-list GUC values in pg_dump and ruleutils.c.
Tom Lane [Tue, 31 Jul 2018 17:00:07 +0000 (13:00 -0400)]
Further fixes for quoted-list GUC values in pg_dump and ruleutils.c.

Commits 742869946 et al turn out to be a couple bricks shy of a load.
We were dumping the stored values of GUC_LIST_QUOTE variables as they
appear in proconfig or setconfig catalog columns.  However, although that
quoting rule looks a lot like SQL-identifier double quotes, there are two
critical differences: empty strings ("") are legal, and depending on which
variable you're considering, values longer than NAMEDATALEN might be valid
too.  So the current technique fails altogether on empty-string list
entries (as reported by Steven Winfield in bug #15248) and it also risks
truncating file pathnames during dump/reload of GUC values that are lists
of pathnames.

To fix, split the stored value without any downcasing or truncation,
and then emit each element as a SQL string literal.

This is a tad annoying, because we now have three copies of the
comma-separated-string splitting logic in varlena.c as well as a fourth
one in dumputils.c.  (Not to mention the randomly-different-from-those
splitting logic in libpq...)  I looked at unifying these, but it would
be rather a mess unless we're willing to tweak the API definitions of
SplitIdentifierString, SplitDirectoriesString, or both.  That might be
worth doing in future; but it seems pretty unsafe for a back-patched
bug fix, so for now accept the duplication.

Back-patch to all supported branches, as the previous fix was.

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

6 years agoFix typo in file identification and copyright year
Alvaro Herrera [Tue, 31 Jul 2018 15:50:28 +0000 (11:50 -0400)]
Fix typo in file identification and copyright year

6 years agoRemove SGMLism from commit 2d36a5e9dab6
Andrew Dunstan [Tue, 31 Jul 2018 12:16:30 +0000 (08:16 -0400)]
Remove SGMLism from commit 2d36a5e9dab6

6 years agoProvide a log_level setting for auto_explain
Andrew Dunstan [Tue, 31 Jul 2018 12:03:57 +0000 (08:03 -0400)]
Provide a log_level setting for auto_explain

Up to now the log level has been hardcoded at LOG. A new
auto_explain.log_level setting allows that to be modified.

Discussion: https://postgr.es/m/CAPPfruyZh+snR2AdmutrA0B_caj=yWZkLqxUTZYNjJCaQ_wKQg@mail.gmail.com

Tom Dunstan and Andrew Dunstan
Reviewed by Daniel Gustafsson

6 years agoRemove dead code left behind by 1b6801051.
Tom Lane [Mon, 30 Jul 2018 23:11:02 +0000 (19:11 -0400)]
Remove dead code left behind by 1b6801051.

6 years agoVerify range bounds to bms_add_range when necessary
Alvaro Herrera [Mon, 30 Jul 2018 21:03:19 +0000 (17:03 -0400)]
Verify range bounds to bms_add_range when necessary

Now that the bms_add_range boundary protections are gone, some
alternative ones are needed in a few places.

Author: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Discussion: https://postgr.es/m/3437ccf8-a144-55ff-1e2f-fc16b437823b@lab.ntt.co.jp

6 years agoChange bms_add_range to be a no-op for empty ranges
Alvaro Herrera [Mon, 30 Jul 2018 21:18:42 +0000 (17:18 -0400)]
Change bms_add_range to be a no-op for empty ranges

In commit 84940644de93, bms_add_range was added with an API to fail with
an error if an empty range was specified.  This seems arbitrary and
unhelpful, so turn that case into a no-op instead.  Callers that require
further verification on the arguments or result can apply them by
themselves.

This fixes the bug that partition pruning throws an API error for a case
involving the default partition of a default partition, as in the
included test case.

Reported-by: Rajkumar Raghuwanshi <rajkumar.raghuwanshi@enterprisedb.com>
Diagnosed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/16590.1532622503@sss.pgh.pa.us

6 years agoEnsure we build generated headers at the start of some more cases.
Tom Lane [Mon, 30 Jul 2018 22:04:39 +0000 (18:04 -0400)]
Ensure we build generated headers at the start of some more cases.

"make installcheck" and some related cases, when invoked from the toplevel
directory, start out by doing "make all" in src/test/regress.  Since that's
one make recursion level down, the submake-generated-headers target will
do nothing, causing us to fail to create/update generated headers before
building pg_regress.  This is, I believe, a new failure mode induced by
commit 3b8f6e75f, so let's fix it.  To do so, we have to invoke
submake-generated-headers at the top level.

Discussion: https://postgr.es/m/0401efec-68f1-679d-3ea3-21d4e8dd11af@gmail.com

6 years agoSet ActiveSnapshot when logically replaying inserts
Alvaro Herrera [Mon, 30 Jul 2018 20:30:07 +0000 (16:30 -0400)]
Set ActiveSnapshot when logically replaying inserts

Input functions for the inserted tuples may require a snapshot, when
they are replayed by native logical replication.  An example is a domain
with a constraint using a SQL-language function, which prior to this
commit failed to apply on the subscriber side.

Reported-by: Mai Peng <maily.peng@webedia-group.com>
Co-authored-by: Minh-Quan TRAN <qtran@itscaro.me>
Co-authored-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/4EB4BD78-BFC3-4D04-B8DA-D53DF7160354@webedia-group.com
Discussion: https://postgr.es/m/153211336163.1404.11721804383024050689@wrigleys.postgresql.org

6 years agodoc: Improve CREATE COLLATION locking documentation
Peter Eisentraut [Mon, 30 Jul 2018 20:01:04 +0000 (22:01 +0200)]
doc: Improve CREATE COLLATION locking documentation

Move out of the concurrency control chapter, where mostly only user
table locks are discussed, and move to CREATE COLLATION reference page.

Author: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Author: Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>

6 years agoFix pg_dump's failure to dump REPLICA IDENTITY for constraint indexes.
Tom Lane [Mon, 30 Jul 2018 16:35:49 +0000 (12:35 -0400)]
Fix pg_dump's failure to dump REPLICA IDENTITY for constraint indexes.

pg_dump knew about printing ALTER TABLE ... REPLICA IDENTITY USING INDEX
for indexes declared as indexes, but it failed to print that for indexes
declared as unique or primary-key constraints.  Per report from Achilleas
Mantzios.

This has been broken since the feature was introduced, AFAICS.
Back-patch to 9.4.

Discussion: https://postgr.es/m/1e6cc5ad-b84a-7c07-8c08-a4d0c3cdc938@matrix.gatewaynet.com

6 years agoDoc: fix oversimplified example for CREATE POLICY.
Tom Lane [Mon, 30 Jul 2018 15:54:41 +0000 (11:54 -0400)]
Doc: fix oversimplified example for CREATE POLICY.

As written, this policy constrained only the post-image not the pre-image
of rows, meaning that users could delete other users' rows or take
ownership of such rows, contrary to what the docs claimed would happen.
We need two separate policies to achieve the documented effect.

While at it, try to explain what's happening a bit more fully.

Per report from Олег Самойлов.  Back-patch to 9.5 where this was added.
Thanks to Stephen Frost for off-list discussion.

Discussion: https://postgr.es/m/3298321532002010@sas1-2b3c3045b736.qloud-c.yandex.net

6 years agoAdd ssl_library preset parameter
Peter Eisentraut [Tue, 26 Jun 2018 08:19:35 +0000 (10:19 +0200)]
Add ssl_library preset parameter

This allows querying the SSL implementation used on the server side.
It's analogous to using PQsslAttribute(conn, "library") in libpq.

Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
6 years agoMark variable used only in assertion with PG_USED_FOR_ASSERTS_ONLY
Tomas Vondra [Sun, 29 Jul 2018 21:08:00 +0000 (23:08 +0200)]
Mark variable used only in assertion with PG_USED_FOR_ASSERTS_ONLY

Perpendicular lines always intersect, so the line_interpt_line() return
value in line_closept_point() was used only in an assertion, triggering
compiler warnings in non-assert builds.

6 years agoRestore handling of -0 in the C field of lines in line_construct().
Tomas Vondra [Sun, 29 Jul 2018 18:35:18 +0000 (20:35 +0200)]
Restore handling of -0 in the C field of lines in line_construct().

Commit a7dc63d904 inadvertedly removed this bit originally introduced
by 43fe90f66a, causing regression test failures on some platforms,
due to producing {1,-1,-0} instead of {1,-1,0}.

6 years agoFix earthdistance test suite function name typo.
Noah Misch [Sun, 29 Jul 2018 19:02:07 +0000 (12:02 -0700)]
Fix earthdistance test suite function name typo.

Affected test queries have been testing the wrong thing since their
introduction in commit 4c1383efd132e4f532213c8a8cc63a455f55e344.
Back-patch to 9.3 (all supported versions).

6 years agoMake error message of pageinspect more consistent for raw page inputs
Michael Paquier [Sun, 29 Jul 2018 14:50:35 +0000 (23:50 +0900)]
Make error message of pageinspect more consistent for raw page inputs

There is a copy-paste error from bt_page_items() which got into
bt_page_items_bytea().  A second message in get_raw_page_internal() was
inconsistent with all the other sub-modules.

Author: Ashutosh Sharma
Discussion: https://postgr.es/m/CAE9k0PnZuZ3PVXSyQY91-53E8JKFcaSyknFqqU43r9MabKSYZA@mail.gmail.com

6 years agoFix two oversights from 9ebe0572 which refactored cluster_rel
Michael Paquier [Sun, 29 Jul 2018 13:00:42 +0000 (22:00 +0900)]
Fix two oversights from 9ebe0572 which refactored cluster_rel

The recheck option became a no-op as ClusterOption failed to set proper
values for each element.  There was a second code path where local
options got overwritten.

Both issues have been spotted by Coverity.

6 years agoDocument security implications of qualified names.
Noah Misch [Sun, 29 Jul 2018 03:08:01 +0000 (20:08 -0700)]
Document security implications of qualified names.

Commit 5770172cb0c9df9e6ce27c507b449557e5b45124 documented secure schema
usage, and that advice suffices for using unqualified names securely.
Document, in typeconv-func primarily, the additional issues that arise
with qualified names.  Back-patch to 9.3 (all supported versions).

Reviewed by Jonathan S. Katz.

Discussion: https://postgr.es/m/20180721012446.GA1840594@rfd.leadboat.com

6 years agoProvide separate header file for built-in float types
Tomas Vondra [Sun, 29 Jul 2018 01:30:48 +0000 (03:30 +0200)]
Provide separate header file for built-in float types

Some data types under adt/ have separate header files, but most simple
ones do not, and their public functions are defined in builtins.h.  As
the patches improving geometric types will require making additional
functions public, this seems like a good opportunity to create a header
for floats types.

Commit 1acf757255 made _cmp functions public to solve NaN issues locally
for GiST indexes.  This patch reworks it in favour of a more widely
applicable API.  The API uses inline functions, as they are easier to
use compared to macros, and avoid double-evaluation hazards.

Author: Emre Hasegeli
Reviewed-by: Kyotaro Horiguchi
Discussion: https://www.postgresql.org/message-id/CAE2gYzxF7-5djV6-cEvqQu-fNsnt%3DEqbOURx7ZDg%2BVv6ZMTWbg%40mail.gmail.com

6 years agoRefactor geometric functions and operators
Tomas Vondra [Sun, 29 Jul 2018 00:02:48 +0000 (02:02 +0200)]
Refactor geometric functions and operators

The primary goal of this patch is to eliminate duplicate code and share
code between different geometric data types more often, to prepare the
ground for additional patches.  Until now the code reuse was limited,
probably because the simpler types (line and point) were implemented
after the more complex ones.

The changes are quite extensive and can be summarised as:

* Eliminate SQL-level function calls.
* Re-use more functions to implement others.
* Unify internal function names and signatures.
* Remove private functions from geo_decls.h.
* Replace should-not-happen checks with assertions.
* Add comments describe for various functions.
* Remove some unreachable code.
* Define delimiter symbols of line datatype like the other ones.
* Remove the GEODEBUG macro and printf() calls.
* Unify code style of a few oddly formatted lines.

While the goal was to cause minimal user-visible changes, it was not
possible to keep the original behavior in all cases - for example when
handling NaN values, or when reusing code makes the functions return
consistent results.

Author: Emre Hasegeli
Reviewed-by: Kyotaro Horiguchi, me
Discussion: https://www.postgresql.org/message-id/CAE2gYzxF7-5djV6-cEvqQu-fNsnt%3DEqbOURx7ZDg%2BVv6ZMTWbg%40mail.gmail.com

6 years agoAdd verbosity to pg_basebackup for sync
Michael Paquier [Sat, 28 Jul 2018 22:53:11 +0000 (07:53 +0900)]
Add verbosity to pg_basebackup for sync

This is useful to know when the data copy has been finished.  The
current situation can be confusing for users as the last message is
"waiting for background process to finish streaming", so it looks like
this is taking time but the final sync is instead.

Author: Jeff Janes
Discussion: https://postgr.es/m/CAMkU=1ypeoMJ=tFBG8vP13sxEtXd4Pm_x1SqsJdW_RvzpcvN=A@mail.gmail.com

6 years agopgtest: run clean, build, and check stages separately
Bruce Momjian [Sat, 28 Jul 2018 19:34:06 +0000 (15:34 -0400)]
pgtest:  run clean, build, and check stages separately

This allows for cleaner error reporting.

Backpatch-through: 9.5

6 years agopg_upgrade: check for clean server shutdowns
Bruce Momjian [Sat, 28 Jul 2018 19:01:55 +0000 (15:01 -0400)]
pg_upgrade:  check for clean server shutdowns

Previously pg_upgrade checked for the pid file and started/stopped the
server to force a clean shutdown.  However, "pg_ctl -m immediate"
removes the pid file but doesn't do a clean shutdown, so check
pg_controldata for a clean shutdown too.

Diagnosed-by: Vimalraj A
Discussion: https://postgr.es/m/CAFKBAK5e4Q-oTUuPPJ56EU_d2Rzodq6GWKS3ncAk3xo7hAsOZg@mail.gmail.com

Backpatch-through: 9.3

6 years agopg_upgrade: report database names with missing extension libs
Bruce Momjian [Sat, 28 Jul 2018 16:33:54 +0000 (12:33 -0400)]
pg_upgrade:  report database names with missing extension libs

Previously only the missing library name was reported, forcing users to
look in all databases to find the library entries.

Discussion: https://postgr.es/m/20180713162815.GA3835@momjian.us

Author: Daniel Gustafsson, me

6 years agopgtest: grab possible warnings from install.log
Bruce Momjian [Sat, 28 Jul 2018 15:35:53 +0000 (11:35 -0400)]
pgtest:  grab possible warnings from install.log

Since PG 9.5, 'make check' records the build output in install.log, so
look in there for warnings too.

Backpatch-through: 9.5

6 years agoReduce path length for locking leaf B-tree pages during insertion
Alexander Korotkov [Fri, 27 Jul 2018 21:31:40 +0000 (00:31 +0300)]
Reduce path length for locking leaf B-tree pages during insertion

In our B-tree implementation appropriate leaf page for new tuple
insertion is acquired using _bt_search() function.  This function always
returns leaf page locked in shared mode.  In order to obtain exclusive
lock, caller have to relock the page.

This commit makes _bt_search() function lock leaf page immediately in
exclusive mode when needed.  That removes unnecessary relock and, in
turn reduces lock contention for B-tree leaf pages.  Our experiments
on multi-core systems showed acceleration up to 4.5 times in corner
case.

Discussion: https://postgr.es/m/CAPpHfduAMDFMNYTCN7VMBsFg_hsf0GqiqXnt%2BbSeaJworwFoig%40mail.gmail.com
Author: Alexander Korotkov
Reviewed-by: Yoshikazu Imai, Simon Riggs, Peter Geoghegan
6 years agoFix grammar in README.tuplock
Alvaro Herrera [Fri, 27 Jul 2018 14:56:30 +0000 (10:56 -0400)]
Fix grammar in README.tuplock

Author: Brad DeJong
Discussion: https://postgr.es/m/CAJnrtnxrA4FqZi0Z6kGPQKMiZkWv2xxgSDQ+hv1jDrf8WCKjjw@mail.gmail.com

6 years agoUse key and partdesc from PartitionDispatch where possible.
Robert Haas [Fri, 27 Jul 2018 13:34:57 +0000 (09:34 -0400)]
Use key and partdesc from PartitionDispatch where possible.

Instead of repeatedly fishing the data out of the relcache entry,
let's use the version that we cached in the PartitionDispatch.  We
could alternatively rip out the PartitionDispatch fields altogether,
but it doesn't make much sense to have them and not use them; before
this patch, partdesc was set but altogether unused.  Amit Langote and
I both thought using them was a litle better than removing them, so
this patch takes that approach.

Discussion: http://postgr.es/m/CA+TgmobFnxcaW-Co-XO8=yhJ5pJXoNkCj6Z7jm9Mwj9FGv-D7w@mail.gmail.com

6 years agoFix the buffer release order for parallel index scans.
Amit Kapila [Fri, 27 Jul 2018 05:23:00 +0000 (10:53 +0530)]
Fix the buffer release order for parallel index scans.

During parallel index scans, if the current page to be read is deleted, we
skip it and try to get the next page for a scan without releasing the buffer
lock on the current page.  To get the next page, sometimes it needs to wait
for another process to complete its scan and advance it to the next page.
Now, it is quite possible that the master backend has errored out before
advancing the scan and issued a termination signal for all workers.  The
workers failed to notice the termination request during wait because the
interrupts are held due to buffer lock on the previous page.  This lead to
all workers being stuck.

The fix is to release the buffer lock on current page before trying to get
the next page.  We are already doing same in backward scans, but missed
it for forward scans.

Reported-by: Victor Yegorov
Bug: 15290
Diagnosed-by: Thomas Munro and Amit Kapila
Author: Amit Kapila
Reviewed-by: Thomas Munro
Tested-By: Thomas Munro and Victor Yegorov
Backpatch-through: 10 where parallel index scans were introduced
Discussion: https://postgr.es/m/153228422922.1395.1746424054206154747@wrigleys.postgresql.org

6 years agoFix handling of pgbench's hash when no argument is provided
Michael Paquier [Fri, 27 Jul 2018 04:40:24 +0000 (13:40 +0900)]
Fix handling of pgbench's hash when no argument is provided

Depending on the platform used, this can cause a crash in the worst
case, or an unhelpful error message, so fail gracefully.

Author: Fabien Coelho
Discussion: https://postgr.es/m/alpine.DEB.2.21.1807262302550.29874@lancre
Backpatch: 11-, where hash() has been added in pgbench.

6 years agoProvide plpgsql tests for cases involving record field changes.
Tom Lane [Thu, 26 Jul 2018 22:18:37 +0000 (18:18 -0400)]
Provide plpgsql tests for cases involving record field changes.

We suppressed one of these test cases in commit feb1cc559 because
it was failing to produce the expected results on CLOBBER_CACHE_ALWAYS
buildfarm members.  But now we need another test with similar behavior,
so let's set up a test file that is expected to vary between regular and
CLOBBER_CACHE_ALWAYS cases, and provide variant expected files.

Someday we should fix plpgsql's failure for change-of-field-type, and
then the discrepancy will go away and we can fold these tests back
into plpgsql_record.sql.  But today is not that day.

Discussion: https://postgr.es/m/87wotkfju1.fsf@news-spur.riddles.org.uk

6 years agoAvoid crash in eval_const_expressions if a Param's type changes.
Tom Lane [Thu, 26 Jul 2018 20:08:45 +0000 (16:08 -0400)]
Avoid crash in eval_const_expressions if a Param's type changes.

Since commit 6719b238e it's been possible for the values of plpgsql
record field variables to be exposed to the planner as Params.
(Before that, plpgsql never supplied values for such variables during
planning, so that the problematic code wasn't reached.)  Other places
that touch potentially-type-mutable Params either cope gracefully or
do runtime-test-and-ereport checks that the type is what they expect.
But eval_const_expressions() just had an Assert, meaning that it either
failed the assertion or risked crashes due to using an incompatible
value.

In this case, rather than throwing an ereport immediately, we can just
not perform a const-substitution in case of a mismatch.  This seems
important for the same reason that the Param fetch was speculative:
we might not actually reach this part of the expression at runtime.

Test case will follow in a separate commit.

Patch by me, pursuant to bug report from Andrew Gierth.
Back-patch to v11 where the previous commit appeared.

Discussion: https://postgr.es/m/87wotkfju1.fsf@news-spur.riddles.org.uk

6 years agoLLVMJIT: Release JIT context after running ExprContext shutdown callbacks.
Andres Freund [Wed, 25 Jul 2018 23:31:49 +0000 (16:31 -0700)]
LLVMJIT: Release JIT context after running ExprContext shutdown callbacks.

Due to inlining it previously was possible that an ExprContext's
shutdown callback pointed to a JITed function. As the JIT context
previously was shut down before the shutdown callbacks were called,
that could lead to segfaults.  Fix the ordering.

Reported-By: Dmitry Dolgov
Author: Andres Freund
Discussion: https://postgr.es/m/CA+q6zcWO7CeAJtHBxgcHn_hj+PenM=tvG0RJ93X1uEJ86+76Ug@mail.gmail.com
Backpatch: 11-, where JIT compilation was added

6 years agoLLVMJIT: Check for 'noinline' attribute in recursively inlined functions.
Andres Freund [Wed, 25 Jul 2018 23:23:59 +0000 (16:23 -0700)]
LLVMJIT: Check for 'noinline' attribute in recursively inlined functions.

Previously the attribute was only checked for external functions
inlined, not "static" functions that had to be inlined as
dependencies.

This isn't really a bug, but makes debugging a bit harder. The new
behaviour also makes more sense. Therefore backpatch.

Author: Andres Freund
Backpatch: 11-, where JIT compilation was added

6 years agoAdd strict_multi_assignment and too_many_rows plpgsql checks
Tomas Vondra [Tue, 24 Jul 2018 23:09:03 +0000 (01:09 +0200)]
Add strict_multi_assignment and too_many_rows plpgsql checks

Until now shadowed_variables was the only plpgsql check supported by
plpgsql.extra_warnings and plpgsql.extra_errors.  This patch introduces
two new checks - strict_multi_assignment and too_many_rows.  Unlike
shadowed_variables, these new checks are enforced at run-time.

strict_multi_assignment checks that commands allowing multi-assignment
(for example SELECT INTO) have the same number of sources and targets.
too_many_rows checks that queries with an INTO clause return one row
exactly.

These checks are aimed at cases that are technically valid and allowed,
but are often a sign of a bug.  Therefore those checks are expected to
be enabled primarily in development and testing environments.

Author: Pavel Stehule
Reviewed-by: Stephen Frost, Tomas Vondra
Discussion: https://www.postgresql.org/message-id/flat/CAFj8pRA2kKRDKpUNwLY0GeG1OqOp+tLS2yQA1V41gzuSz-hCng@mail.gmail.com

6 years agoPad semaphores to avoid false sharing.
Thomas Munro [Tue, 24 Jul 2018 22:57:52 +0000 (10:57 +1200)]
Pad semaphores to avoid false sharing.

In a USE_UNNAMED_SEMAPHORES build, the default on Linux and FreeBSD
since commit ecb0d20a, we have an array of sem_t objects.  This
turned out to reduce performance compared to the previous default
USE_SYSV_SEMAPHORES on an 8 socket system.  Testing showed that the
lost performance could be regained by padding the array elements so
that they have their own cache lines.  This matches what we do for
similar hot arrays (see LWLockPadded, WALInsertLockPadded).

Back-patch to 10, where unnamed semaphores were adopted as the default
semaphore interface on those operating systems.

Author: Thomas Munro
Reviewed-by: Andres Freund
Reported-by: Mithun Cy
Tested-by: Mithun Cy, Tom Lane, Thomas Munro
Discussion: https://postgr.es/m/CAD__OugYDM3O%2BdyZnnZSbJprSfsGFJcQ1R%3De59T3hcLmDug4_w%40mail.gmail.com

6 years agodoc: Fix reference to "decoder" to instead be the correct "output plugin".
Andres Freund [Tue, 24 Jul 2018 17:42:59 +0000 (10:42 -0700)]
doc: Fix reference to "decoder" to instead be the correct "output plugin".

Author: Jonathan Katz
Discussion: https://postgr.es/m/DD02DD86-5989-4BFD-8712-468541F68383@postgresql.org
Backpatch: 9.4-, where logical decoding was added

6 years agoDefend against some potential spurious compiler warnings in 86eaf208e.
Andres Freund [Tue, 24 Jul 2018 15:59:36 +0000 (08:59 -0700)]
Defend against some potential spurious compiler warnings in 86eaf208e.

Author: David Rowley
Discussion: https://postgr.es/m/CAKJS1f-AbCFeFU92GZZYqNOVRnPtUwczSYmR2NHCyf9uHUnNiw@mail.gmail.com

6 years agopsql: Add option for procedures to \df
Peter Eisentraut [Sat, 14 Jul 2018 10:17:49 +0000 (12:17 +0200)]
psql: Add option for procedures to \df

6 years agoRefactor cluster_rel() to handle more options
Michael Paquier [Tue, 24 Jul 2018 02:37:32 +0000 (11:37 +0900)]
Refactor cluster_rel() to handle more options

This extends cluster_rel() in such a way that more options can be added
in the future, which will reduce the amount of chunk code for an
upcoming SKIP_LOCKED aimed for VACUUM.  As VACUUM FULL is a different
flavor of CLUSTER, we want to make that extensible to ease integration.

This only reworks the API and its callers, without providing anything
user-facing.  Two options are present now: verbose mode and relation
recheck when doing the cluster command work across multiple
transactions.  This could be used as well as a base to extend the
grammar of CLUSTER later on.

Author: Michael Paquier
Reviewed-by: Nathan Bossart
Discussion: https://postgr.es/m/20180723031058.GE2854@paquier.xyz

6 years agoFix calculation for WAL segment recycling and removal
Michael Paquier [Tue, 24 Jul 2018 01:32:56 +0000 (10:32 +0900)]
Fix calculation for WAL segment recycling and removal

Commit 4b0d28de06 has removed the prior checkpoint and related
facilities but has left WAL recycling based on the LSN of the prior
checkpoint, which causes incorrect calculations for WAL removal and
recycling for max_wal_size and min_wal_size.  This commit changes things
so as the base calculation point is the last checkpoint generated.

Reported-by: Kyotaro Horiguchi
Author: Kyotaro Horiguchi
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/20180723.135748.42558387.horiguchi.kyotaro@lab.ntt.co.jp
Backpatch: 11-, where the prior checkpoint has been removed.

6 years agoUse setproctitle_fast() to update the ps status, if available.
Thomas Munro [Tue, 24 Jul 2018 01:09:22 +0000 (13:09 +1200)]
Use setproctitle_fast() to update the ps status, if available.

FreeBSD has introduced a faster variant of setproctitle().  Use it,
where available.

Author: Thomas Munro
Discussion: https://postgr.es/m/CAEepm=1wKMTi81uodJ=1KbJAz5WedOg=cr8ewEXrUFeaxWEgww@mail.gmail.com

6 years agopgbench: Remove duplicate entries from table of builtin functions.
Robert Haas [Mon, 23 Jul 2018 15:48:38 +0000 (11:48 -0400)]
pgbench: Remove duplicate entries from table of builtin functions.

Fabien Coelho

Discussion: http://postgr.es/m/alpine.DEB.2.21.1807221822320.19939@lancre

6 years agoLLVMJIT: Adapt to API changes in gdb and perf support.
Andres Freund [Mon, 23 Jul 2018 04:13:20 +0000 (21:13 -0700)]
LLVMJIT: Adapt to API changes in gdb and perf support.

During the work of upstreaming my previous patches for gdb and perf
support the API changed. Adapt.  Normally this wouldn't necessarily be
something to backpatch, but the previous API wasn't upstream, and at
least the gdb support is quite useful for debugging.

Author: Andres Freund
Backpatch: 11, where LLVM based JIT support was added.

6 years agoLLVMJIT: Fix LLVM build for LLVM > 7.
Andres Freund [Mon, 23 Jul 2018 04:05:06 +0000 (21:05 -0700)]
LLVMJIT: Fix LLVM build for LLVM > 7.

The location of LLVMAddPromoteMemoryToRegisterPass moved.

Author: Andres Freund
Backpatch: 11, where LLVM based JIT support was added.

6 years agoReset context at the tail end of JITed EEOP_AGG_PLAIN_TRANS.
Andres Freund [Mon, 23 Jul 2018 03:31:22 +0000 (20:31 -0700)]
Reset context at the tail end of JITed EEOP_AGG_PLAIN_TRANS.

While no negative consequences are currently known, it's clearly wrong
to not reset the context in one of the branches.

Reported-By: Dmitry Dolgov
Author: Dmitry Dolgov
Discussion: https://postgr.es/m/CAGPqQf165-=+Drw3Voim7M5EjHT1zwPF9BQRjLFQzCzYnNZEiQ@mail.gmail.com
Backpatch: 11-, where JIT compilation support was added

6 years agoMop-up for 3522d0eaba5, which missed some alternative output files.
Andres Freund [Mon, 23 Jul 2018 00:27:05 +0000 (17:27 -0700)]
Mop-up for 3522d0eaba5, which missed some alternative output files.

6 years agoAdd proper errcodes to new error messages for read() failures
Michael Paquier [Mon, 23 Jul 2018 00:37:36 +0000 (09:37 +0900)]
Add proper errcodes to new error messages for read() failures

Those would use the default ERRCODE_INTERNAL_ERROR, but for foreseeable
failures an errcode ought to be set, ERRCODE_DATA_CORRUPTED making the
most sense here.

While on the way, fix one errcode_for_file_access missing in origin.c
since the code has been created, and remove one assignment of errno to 0
before calling read(), as this was around to fit with what was present
before 811b6e36 where errno would not be set when not enough bytes are
read.  I have noticed the first one, and Tom has pinged me about the
second one.

Author: Michael Paquier
Reported-by: Tom Lane
Discussion: https://postgr.es/m/27265.1531925836@sss.pgh.pa.us

6 years agoMake more consistent some error messages for file-related operations
Michael Paquier [Mon, 23 Jul 2018 00:19:12 +0000 (09:19 +0900)]
Make more consistent some error messages for file-related operations

Some error messages which report something about a file operation use
as well context which is already provided within the path being worked
on, making things rather duplicated.  This creates more work for
translators, and does not actually bring clarity.

More could be done, however in a lot of cases the context used is
actually useful, still that patch gets down things with a good cut.

Author: Michael Paquier
Reviewed-by: Kyotaro Horiguchi, Tom Lane
Discussion: https://postgr.es/m/20180718044711.GA8565@paquier.xyz

6 years agoFix JITed EEOP_AGG_INIT_TRANS, which missed some state.
Andres Freund [Sun, 22 Jul 2018 23:47:00 +0000 (16:47 -0700)]
Fix JITed EEOP_AGG_INIT_TRANS, which missed some state.

The JIT compiled implementation missed maintaining
AggState->{current_set,curaggcontext}. That could lead to trouble
because the transition value could be allocated in the wrong context.

Reported-By: Rushabh Lathia
Diagnosed-By: Dmitry Dolgov
Author: Dmitry Dolgov, with minor changes by me
Discussion: https://postgr.es/m/CAGPqQf165-=+Drw3Voim7M5EjHT1zwPF9BQRjLFQzCzYnNZEiQ@mail.gmail.com
Backpatch: 11-, where JIT compilation support was added

6 years agoHand code string to integer conversion for performance.
Andres Freund [Sun, 22 Jul 2018 21:58:01 +0000 (14:58 -0700)]
Hand code string to integer conversion for performance.

As benchmarks show, using libc's string-to-integer conversion is
pretty slow. At least part of the reason for that is that strtol[l]
have to be more generic than what largely is required inside pg.

This patch considerably speeds up int2/int4 input (int8 already was
already using hand-rolled code).

Most of the existing pg_atoi callers have been converted. But as one
requires pg_atoi's custom delimiter functionality, and as it seems
likely that there's external pg_atoi users, it seems sensible to just
keep pg_atoi around.

Author: Andres Freund
Reviewed-By: Robert Haas
Discussion: https://postgr.es/m/20171208214437.qgn6zdltyq5hmjpk@alap3.anarazel.de

6 years agoDeduplicate "invalid input syntax" messages for various types.
Andres Freund [Sun, 22 Jul 2018 21:58:01 +0000 (14:58 -0700)]
Deduplicate "invalid input syntax" messages for various types.

Previously a lot of the error messages referenced the type in the
error message itself. That requires that the message is translated
separately for each type.

Note that currently a few smallint cases continue to reference the
integer, rather than smallint, type. A later patch will create a
separate routine for 16bit input.

Author: Andres Freund
Discussion: https://postgr.es/m/20180707200158.wpqkd7rjr4jxq5g7@alap3.anarazel.de

6 years agoFurther portability hacking in pg_upgrade's test script.
Tom Lane [Sat, 21 Jul 2018 19:40:51 +0000 (15:40 -0400)]
Further portability hacking in pg_upgrade's test script.

I blew the dust off a Bourne shell (file date 1996, yea verily) and
tried to run test.sh with it.  It mostly worked, but I found that the
temp-directory creation code introduced by commit be76a6d39 was not
compatible, for a couple of reasons: this shell thinks "set -e" should
force an exit if a command within backticks fails, and it also thinks code
within braces should be executed by a sub-shell, meaning that variable
settings don't propagate back up to the parent shell.  In view of Victor
Wagner's report that Solaris is still using pre-POSIX shells, seems like
we oughta make this case work.  It's not like the code is any less
idiomatic this way; the prior coding technique appeared nowhere else.

(There is a remaining bash-ism here, which is that $RANDOM doesn't do
what the code hopes in non-bash shells.  But the use of $$ elsewhere in
that path should be enough to ensure uniqueness and some amount of
randomness, so I think it's okay as-is.)

Back-patch to all supported branches, as the previous commit was.

Discussion: https://postgr.es/m/20180720153820.69e9ae6c@fafnir.local.vm

6 years agoBe more paranoid about quoting in pg_upgrade's test script.
Tom Lane [Sat, 21 Jul 2018 16:05:25 +0000 (12:05 -0400)]
Be more paranoid about quoting in pg_upgrade's test script.

Double-quote $PGDATA in "find" commands introduced by commit da9b580d8,
in case that path contains spaces or other special characters.

Adjust a few other places so that quoting is done more consistently.
None of the others are actual bugs AFAICS, but it's confusing to readers
if the same thing is done differently in different places.

Noted by Tels.

Discussion: https://postgr.es/m/c96303c04c360bbedaa04f90f515745b.squirrel@sm.webmail.pair.com

6 years agoAvoid unportable shell syntax in pg_upgrade's test script.
Tom Lane [Fri, 20 Jul 2018 17:59:27 +0000 (13:59 -0400)]
Avoid unportable shell syntax in pg_upgrade's test script.

Most of test.sh uses traditional backtick syntax for command substitution,
but commit da9b580d8 introduced two uses of $(...) syntax, which is not
recognized by very old shells.  Bring those into line with the rest.

Victor Wagner

Discussion: https://postgr.es/m/20180720153820.69e9ae6c@fafnir.local.vm

6 years agoGuard against rare RAND_bytes() failures in pg_strong_random().
Dean Rasheed [Fri, 20 Jul 2018 07:55:44 +0000 (08:55 +0100)]
Guard against rare RAND_bytes() failures in pg_strong_random().

When built using OpenSSL, pg_strong_random() uses RAND_bytes() to
generate the random number. On very rare occasions that can fail, if
its PRNG has not been seeded with enough data. Additionally, once it
does fail, all subsequent calls will also fail until more seed data is
added. Since this is required during backend startup, this can result
in all new backends failing to start until a postmaster restart.

Guard against that by checking the state of OpenSSL's PRNG using
RAND_status(), and if necessary (very rarely), seeding it using
RAND_poll().

Back-patch to v10, where pg_strong_random() was introduced.

Dean Rasheed and Michael Paquier.

Discussion: https://postgr.es/m/CAEZATCXMtxbzSAvyKKk5uCRf9pNt4UV%2BF_5v%3DgLfJUuPxU4Ytg%40mail.gmail.com

6 years agoBump catalog version for recent toast table additions
Michael Paquier [Fri, 20 Jul 2018 00:28:19 +0000 (09:28 +0900)]
Bump catalog version for recent toast table additions

This has been forgotten in 96cdeae.

6 years agoAdd toast tables to most system catalogs
Michael Paquier [Thu, 19 Jul 2018 22:43:41 +0000 (07:43 +0900)]
Add toast tables to most system catalogs

It has been project policy to create toast tables only for those catalogs
that might reasonably need one.  Since this judgment call can change over
time, just create one for every catalog, as this can be useful when
creating rather-long entries in catalogs, with recent examples being in
the shape of policy expressions or customly-formatted SCRAM verifiers.

To prevent circular dependencies and to avoid adding complexity to VACUUM
FULL logic, exclude pg_class, pg_attribute, and pg_index.  Also, to
prevent pg_upgrade from seeing a non-empty new cluster, exclude
pg_largeobject and pg_largeobject_metadata from the set as large object
data is handled as user data.  Those relations have no reason to use a
toast table anyway.

Author: Joe Conway, John Naylor
Reviewed-by: Michael Paquier, Tom Lane
Discussion: https://postgr.es/m/84ddff04-f122-784b-b6c5-3536804495f8@joeconway.com

6 years agoRemove undocumented restriction against duplicate partition key columns.
Tom Lane [Thu, 19 Jul 2018 19:41:46 +0000 (15:41 -0400)]
Remove undocumented restriction against duplicate partition key columns.

transformPartitionSpec rejected duplicate simple partition columns
(e.g., "PARTITION BY RANGE (x,x)") but paid no attention to expression
columns, resulting in inconsistent behavior.  Worse, cases like
"PARTITION BY RANGE (x,(x))") were accepted but would then result in
dump/reload failures, since the expression (x) would get simplified
to a plain column later.

There seems no better reason for this restriction than there was for
the one against duplicate included index columns (cf commit 701fd0bbc),
so let's just remove it.

Back-patch to v10 where this code was added.

Report and patch by Yugo Nagata.

Discussion: https://postgr.es/m/20180712165939.36b12aff.nagata@sraoss.co.jp

6 years agoImprove psql's \d command to show whether index columns are key columns.
Tom Lane [Thu, 19 Jul 2018 18:53:41 +0000 (14:53 -0400)]
Improve psql's \d command to show whether index columns are key columns.

This is essential information when looking at an index that has
"included" columns.  Per discussion, follow the style used in \dC
and some other places: column header is "Key?" and values are "yes"
or "no" (all translatable).

While at it, revise describeOneTableDetails to be a bit more maintainable:
avoid hard-wired column numbers and multiple repetitions of what needs
to be identical test logic.  This also results in the emitted catalog
query corresponding more closely to what we print, which should be a
benefit to users of ECHO_HIDDEN mode, and perhaps a bit faster too
(the old logic sometimes asked for values it would not print, even
ones that are fairly expensive to get).

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

6 years agoFix pg_get_indexdef()'s behavior for included index columns.
Tom Lane [Thu, 19 Jul 2018 17:48:05 +0000 (13:48 -0400)]
Fix pg_get_indexdef()'s behavior for included index columns.

The multi-argument form of pg_get_indexdef() failed to print anything when
asked to print a single index column that is an included column rather than
a key column.  This seems an unintentional result of someone having tried
to take a short-cut and use the attrsOnly flag for two different purposes.
To fix, split said flag into two flags, attrsOnly which suppresses
non-attribute info, and keysOnly which suppresses included columns.
Add a test case using psql's \d command, which relies on that function.

(It's mighty tempting at this point to replace pg_get_indexdef_worker's
mess of boolean flag arguments with a single bitmask-of-flags argument,
which would allow making the call sites much more self-documenting.
But I refrained for the moment.)

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

6 years agoRewrite comments in replication slot advance implementation
Alvaro Herrera [Thu, 19 Jul 2018 18:15:44 +0000 (14:15 -0400)]
Rewrite comments in replication slot advance implementation

The code added by 9c7d06d60680 was a bit obscure; clarify that by
rewriting the comments.  Lack of clarity has already caused bugs, so
it's a worthy goal.

Co-authored-by: Arseny Sher <a.sher@postgrespro.ru>
Co-authored-by: Michaël Paquier <michael@paquier.xyz>
Co-authored-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: Petr Jelínek <petr.jelinek@2ndquadrant.com>
Discussion: https://postgr.es/m/87y3fgoyrn.fsf@ars-thinkpad

6 years agoFix handling of empty uncompressed posting list pages in GIN
Alexander Korotkov [Thu, 19 Jul 2018 18:04:17 +0000 (21:04 +0300)]
Fix handling of empty uncompressed posting list pages in GIN

PostgreSQL 9.4 introduces posting list compression in GIN.  This feature
supports online upgrade, so that after pg_upgrade uncompressed posting
lists are compressed on-the-fly.  Underlying code appears to always
expect at least one item on uncompressed posting list page.  But there
could be completely empty pages, because VACUUM never deletes leftmost
and rightmost pages from posting trees.  This commit fixes that.

Reported-by: Sivasubramanian Ramasubramanian
Discussion: https://postgr.es/m/1531867212836.63354%40amazon.com
Author: Sivasubramanian Ramasubramanian, Alexander Korotkov
Backpatch-through: 9.4

6 years agoFix error message when a hostaddr cannot be parsed.
Heikki Linnakangas [Thu, 19 Jul 2018 17:24:29 +0000 (20:24 +0300)]
Fix error message when a hostaddr cannot be parsed.

We were incorrectly passing hostname, not hostaddr, in the error message,
and because of that, you got:

$ psql 'hostaddr=foo'
psql: could not parse network address "(null)": Name or service not known

Backpatch to v10, where this was broken (by commit 7b02ba62e9).

Report and fix by Robert Haas.

Discussion: https://www.postgresql.org/message-id/CA+TgmoapFQA30NomGKEaZCu3iN7mF7fux8fbbk9SouVOT2JP7w@mail.gmail.com

6 years agoRephrase a few comments for clarity.
Heikki Linnakangas [Thu, 19 Jul 2018 13:08:09 +0000 (16:08 +0300)]
Rephrase a few comments for clarity.

I was confused by what "intended to be parallel serially" meant, until
Robert Haas and David G. Johnston explained it. Rephrase the comment to
make it more clear, using David's suggested wording.

Discussion: https://www.postgresql.org/message-id/1fec9022-41e8-e484-70ce-2179b08c2092%40iki.fi

6 years agoFix comment.
Heikki Linnakangas [Thu, 19 Jul 2018 12:39:06 +0000 (15:39 +0300)]
Fix comment.

This comment was copy-pasted from nodeAppend.c to nodeMergeAppend.c, but
while committing 5220bb7533, I modified wrong copy of it.

Spotted by David Rowley

6 years agoExpand run-time partition pruning to work with MergeAppend
Heikki Linnakangas [Thu, 19 Jul 2018 10:49:43 +0000 (13:49 +0300)]
Expand run-time partition pruning to work with MergeAppend

This expands the support for the run-time partition pruning which was added
for Append in 499be013de to also allow unneeded subnodes of a MergeAppend
to be removed.

Author: David Rowley
Discussion: https://www.postgresql.org/message-id/CAKJS1f_F_V8D7Wu-HVdnH7zCUxhoGK8XhLLtd%3DCu85qDZzXrgg%40mail.gmail.com

6 years agoFix print of Path nodes when using OPTIMIZER_DEBUG
Michael Paquier [Thu, 19 Jul 2018 00:54:39 +0000 (09:54 +0900)]
Fix print of Path nodes when using OPTIMIZER_DEBUG

GatherMergePath (introduced in 10) and CustomPath (introduced in 9.5)
have gone missing.  The order of the Path nodes was inconsistent with
what is listed in nodes.h, so make the order consistent at the same time
to ease future checks and additions.

Author: Sawada Masahiko
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/CAD21AoBQMLoc=ohH-oocuAPsELrmk8_EsRJjOyR8FQLZkbE0wA@mail.gmail.com

6 years agoFix re-parameterize of MergeAppendPath
Michael Paquier [Thu, 19 Jul 2018 00:01:57 +0000 (09:01 +0900)]
Fix re-parameterize of MergeAppendPath

Instead of MergeAppendPath, MergeAppend nodes were considered.  This
code is not covered by any tests now, which should be addressed at some
point.

This is an oversight from f49842d, which introduced partition-wise joins
in v11, so back-patch down to that.

Author: Michael Paquier
Reviewed-by: Ashutosh Bapat
Discussion: https://postgr.es/m/20180718062202.GC8565@paquier.xyz

6 years agoRemove race-prone hot_standby_feedback test cases in 001_stream_rep.pl.
Tom Lane [Wed, 18 Jul 2018 21:39:27 +0000 (17:39 -0400)]
Remove race-prone hot_standby_feedback test cases in 001_stream_rep.pl.

This script supposed that if it turned hot_standby_feedback on and then
shut down the standby server, at least one feedback message would be
guaranteed to be sent before the standby stops.  But there is no such
guarantee, if the standby's walreceiver process is slow enough --- and
we've seen multiple failures in the buildfarm showing that that does
happen in practice.  While we could rearrange the walreceiver logic to
make it less likely, it seems probably impossible to create a really
bulletproof guarantee of that sort; and if we tried, we might create
situations where the walreceiver wouldn't react in a timely manner to
shutdown commands.  It seems better instead to remove the script's
assumption that feedback will occur before shutdown.

But once we do that, these last few tests seem quite redundant with
the earlier tests in the script.  So let's just drop them altogether
and save some buildfarm cycles.

Backpatch to v10 where these tests were added.

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

6 years agoDrop the rule against included index columns duplicating key columns.
Tom Lane [Wed, 18 Jul 2018 18:43:03 +0000 (14:43 -0400)]
Drop the rule against included index columns duplicating key columns.

The initial version of the included-index-column feature stated that
included columns couldn't be the same as any key column of the index.
While it'd be pretty silly to do that, since the included column would be
entirely redundant, we've never prohibited redundant index columns before
so it's not very consistent to do so here.  Moreover, the prohibition
was itself badly implemented, so that it failed to reject columns that
were effectively identical but not spelled quite alike, as reported by
Aditya Toshniwal.

(Moreover, it's not hard to imagine that for some non-btree index types,
such cases would be non-silly anyhow: the index might use a lossy
representation for key columns but be able to support retrieval of the
original form of included columns.)

Hence, let's just drop the prohibition.

In passing, do some copy-editing on the documentation for the
included-column feature.

Yugo Nagata; documentation and test corrections by me

Discussion: https://postgr.es/m/CAM9w-_mhBCys4fQNfaiQKTRrVWtoFrZ-wXmDuE9Nj5y-Y7aDKQ@mail.gmail.com

6 years agoUse a ResourceOwner to track buffer pins in all cases.
Tom Lane [Wed, 18 Jul 2018 16:15:16 +0000 (12:15 -0400)]
Use a ResourceOwner to track buffer pins in all cases.

Historically, we've allowed auxiliary processes to take buffer pins without
tracking them in a ResourceOwner.  However, that creates problems for error
recovery.  In particular, we've seen multiple reports of assertion crashes
in the startup process when it gets an error while holding a buffer pin,
as for example if it gets ENOSPC during a write.  In a non-assert build,
the process would simply exit without releasing the pin at all.  We've
gotten away with that so far just because a failure exit of the startup
process translates to a database crash anyhow; but any similar behavior
in other aux processes could result in stuck pins and subsequent problems
in vacuum.

To improve this, institute a policy that we must *always* have a resowner
backing any attempt to pin a buffer, which we can enforce just by removing
the previous special-case code in resowner.c.  Add infrastructure to make
it easy to create a process-lifespan AuxProcessResourceOwner and clear
out its contents at appropriate times.  Replace existing ad-hoc resowner
management in bgwriter.c and other aux processes with that.  (Thus, while
the startup process gains a resowner where it had none at all before, some
other aux process types are replacing an ad-hoc resowner with this code.)
Also use the AuxProcessResourceOwner to manage buffer pins taken during
StartupXLOG and ShutdownXLOG, even when those are being run in a bootstrap
process or a standalone backend rather than a true auxiliary process.

In passing, remove some other ad-hoc resource owner creations that had
gotten cargo-culted into various other places.  As far as I can tell
that was all unnecessary, and if it had been necessary it was incomplete,
due to lacking any provision for clearing those resowners later.
(Also worth noting in this connection is that a process that hasn't called
InitBufferPoolBackend has no business accessing buffers; so there's more
to do than just add the resowner if we want to touch buffers in processes
not covered by this patch.)

Although this fixes a very old bug, no back-patch, because there's no
evidence of any significant problem in non-assert builds.

Patch by me, pursuant to a report from Justin Pryzby.  Thanks to
Robert Haas and Kyotaro Horiguchi for reviews.

Discussion: https://postgr.es/m/20180627233939.GA10276@telsasoft.com

6 years agoFix misc typos, mostly in comments.
Heikki Linnakangas [Mon, 9 Jul 2018 13:10:44 +0000 (16:10 +0300)]
Fix misc typos, mostly in comments.

A collection of typos I happened to spot while reading code, as well as
grepping for common mistakes.

Backpatch to all supported versions, as applicable, to avoid conflicts
when backporting other commits in the future.

6 years agoFix more portability issues with casts to Size when using off_t
Michael Paquier [Wed, 18 Jul 2018 00:51:53 +0000 (09:51 +0900)]
Fix more portability issues with casts to Size when using off_t

This should tame the beast, as there are no other places where off_t is
used in the new error messages.

Reported again by longfin, which complained about walsender.c while I
spotted the other two ones while double-checking.