]> granicus.if.org Git - postgresql/log
postgresql
5 years agoFix ecpglib regression that made it impossible to close a cursor that was
Michael Meskes [Mon, 11 Mar 2019 15:00:13 +0000 (16:00 +0100)]
Fix ecpglib regression that made it impossible to close a cursor that was
opened in a prepared statement.

Patch by: "Kuroda, Hayato" <kuroda.hayato@jp.fujitsu.com>

5 years agoRemove unused macro
Peter Eisentraut [Mon, 11 Mar 2019 08:29:50 +0000 (09:29 +0100)]
Remove unused macro

Use was removed in 25ca5a9a54923a5d6746f771c4c23e85a195bde5.

5 years agopsql: Add documentation URL to \help output
Peter Eisentraut [Mon, 11 Mar 2019 07:50:02 +0000 (08:50 +0100)]
psql: Add documentation URL to \help output

Add a link to the specific command's reference web page to the bottom
of its \help output.

Discussion: https://www.postgresql.org/message-id/flat/40179bd0-fa7d-4108-1991-a20ae9ad5667%402ndquadrant.com

5 years agoAdjust error message for partial writes in WAL segments
Michael Paquier [Mon, 11 Mar 2019 00:31:25 +0000 (09:31 +0900)]
Adjust error message for partial writes in WAL segments

93473c6 has removed openLogOff, changing on the way the error message
which is used to report partial writes to WAL segments.  The
newly-introduced error message used the offset up to which the write has
happened, keeping always the same total length to write.  This changes
the error message so as the number of bytes left to write are reported.

Reported-by: Michael Paquier
Author: Robert Haas
Discussion: https://postgr.es/m/20190306235251.GA17293@paquier.xyz

5 years agoFix documentation on partitioning vs. foreign tables
Alvaro Herrera [Sun, 10 Mar 2019 22:45:29 +0000 (19:45 -0300)]
Fix documentation on partitioning vs. foreign tables

1. The PARTITION OF clause of CREATE FOREIGN TABLE was not explained in
   the CREATE FOREIGN TABLE reference page.  Add it.
   (Postgres 10 onwards)

2. The limitation that tuple routing cannot target partitions that are
   foreign tables was not documented clearly enough.  Improve wording.
   (Postgres 10 onwards)

3. The UPDATE tuple re-routing concurrency behavior was explained in
   the DDL chapter, which doesn't seem the right place.  Move it to the
   UPDATE reference page instead.  (Postgres 11 onwards).

Authors: Amit Langote, David Rowley.
Reviewed-by: Etsuro Fujita.
Reported-by: Derek Hans
Discussion: https://postgr.es/m/CAGrP7a3Xc1Qy_B2WJcgAD8uQTS_NDcJn06O5mtS_Ne1nYhBsyw@mail.gmail.com

5 years agoReduce the default value of autovacuum_vacuum_cost_delay to 2ms.
Tom Lane [Sun, 10 Mar 2019 19:16:21 +0000 (15:16 -0400)]
Reduce the default value of autovacuum_vacuum_cost_delay to 2ms.

This is a better way to implement the desired change of increasing
autovacuum's default resource consumption.

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

5 years agoRevert "Increase the default vacuum_cost_limit from 200 to 2000"
Tom Lane [Sun, 10 Mar 2019 19:05:25 +0000 (15:05 -0400)]
Revert "Increase the default vacuum_cost_limit from 200 to 2000"

This reverts commit bd09503e633b8077822bb4daf91625b71ac16253.

Per discussion, it seems like what we should do instead is to
reduce the default value of autovacuum_vacuum_cost_delay by the
same factor.  That's functionally equivalent as long as the
platform can accurately service the smaller delay request, which
should be true on anything released in the last 10 years or more.
And smaller, more-closely-spaced delays are better in terms of
providing a steady I/O load.

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

5 years agoConvert [autovacuum_]vacuum_cost_delay into floating-point GUCs.
Tom Lane [Sun, 10 Mar 2019 19:01:39 +0000 (15:01 -0400)]
Convert [autovacuum_]vacuum_cost_delay into floating-point GUCs.

This change makes it possible to specify sub-millisecond delays,
which work well on most modern platforms, though that was not true
when the cost-delay feature was designed.

To support this without breaking existing configuration entries,
improve guc.c to allow floating-point GUCs to have units.  Also,
allow "us" (microseconds) as an input/output unit for time-unit GUCs.
(It's not allowed as a base unit, at least not yet.)

Likewise change the autovacuum_vacuum_cost_delay reloption to be
floating-point; this forces a catversion bump because the layout of
StdRdOptions changes.

This patch doesn't in itself change the default values or allowed
ranges for these parameters, and it should not affect the behavior
for any already-allowed setting for them.

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

5 years agoInclude GUC's unit, if it has one, in out-of-range error messages.
Tom Lane [Sun, 10 Mar 2019 17:18:17 +0000 (13:18 -0400)]
Include GUC's unit, if it has one, in out-of-range error messages.

This should reduce confusion in cases where we've applied a units
conversion, so that the number being reported (and the quoted range
limits) are in some other units than what the user gave in the
setting we're rejecting.

Some of the changes here assume that float GUCs can have units,
which isn't true just yet, but will be shortly.

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

5 years agoDisallow NaN as a value for floating-point GUCs.
Tom Lane [Sun, 10 Mar 2019 16:58:51 +0000 (12:58 -0400)]
Disallow NaN as a value for floating-point GUCs.

None of the code that uses GUC values is really prepared for them to
hold NaN, but parse_real() didn't have any defense against accepting
such a value.  Treat it the same as a syntax error.

I haven't attempted to analyze the exact consequences of setting any
of the float GUCs to NaN, but since they're quite unlikely to be good,
this seems like a back-patchable bug fix.

Note: we don't need an explicit test for +-Infinity because those will
be rejected by existing range checks.  I added a regression test for
that in HEAD, but not older branches because the spelling of the value
in the error message will be platform-dependent in branches where we
don't always use port/snprintf.c.

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

5 years agopg_upgrade: Ignore TOAST for partitioned tables
Alvaro Herrera [Sun, 10 Mar 2019 16:20:58 +0000 (13:20 -0300)]
pg_upgrade: Ignore TOAST for partitioned tables

Since partitioned tables in pg12 do not have toast tables, trying to set
the toast OID confuses pg_upgrade.  Have pg_dump omit those values to
avoid the problem.

Per Andres Freund and buildfarm members crake and snapper
Discussion: https://postgr.es/m/20190306204104.yle5jfbnqkcwykni@alap3.anarazel.de

5 years agoSupport for INCLUDE attributes in GiST indexes
Alexander Korotkov [Sun, 10 Mar 2019 08:36:47 +0000 (11:36 +0300)]
Support for INCLUDE attributes in GiST indexes

Similarly to B-tree, GiST index access method gets support of INCLUDE
attributes.  These attributes aren't used for tree navigation and aren't
present in non-leaf pages.  But they are present in leaf pages and can be
fetched during index-only scan.

The point of having INCLUDE attributes in GiST indexes is slightly different
from the point of having them in B-tree.  The main point of INCLUDE attributes
in B-tree is to define UNIQUE constraint over part of attributes enabled for
index-only scan.  In GiST the main point of INCLUDE attributes is to use
index-only scan for attributes, whose data types don't have GiST opclasses.

Discussion: https://postgr.es/m/73A1A452-AD5F-40D4-BD61-978622FF75C1%40yandex-team.ru
Author: Andrey Borodin, with small changes by me
Reviewed-by: Andreas Karlsson
5 years agoSimplify release-note links to back branches.
Tom Lane [Sat, 9 Mar 2019 23:42:19 +0000 (18:42 -0500)]
Simplify release-note links to back branches.

Now that https://www.postgresql.org/docs/release/ is populated,
replace the stopgap text we had under "Prior Releases" with
a pointer to that archive.

Discussion: https://postgr.es/m/e0f09c9a-bd2b-862a-d379-601dfabc8969@postgresql.org

5 years agoAdd new clientcert hba option verify-full
Magnus Hagander [Sat, 9 Mar 2019 20:09:10 +0000 (12:09 -0800)]
Add new clientcert hba option verify-full

This allows a login to require both that the cn of the certificate
matches (like authentication type cert) *and* that another
authentication method (such as password or kerberos) succeeds as well.

The old value of clientcert=1 maps to the new clientcert=verify-ca,
clientcert=0 maps to the new clientcert=no-verify, and the new option
erify-full will add the validation of the CN.

Author: Julian Markwort, Marius Timmer
Reviewed by: Magnus Hagander, Thomas Munro

5 years agoTrack block level checksum failures in pg_stat_database
Magnus Hagander [Sat, 9 Mar 2019 18:45:17 +0000 (10:45 -0800)]
Track block level checksum failures in pg_stat_database

This adds a column that counts how many checksum failures have occurred
on files belonging to a specific database. Both checksum failures
during normal backend processing and those created when a base backup
detects a checksum failure are counted.

Author: Magnus Hagander
Reviewed by: Julien Rouhaud

5 years agoAvoid some table rewrites for ALTER TABLE .. SET DATA TYPE timestamp.
Noah Misch [Sat, 9 Mar 2019 04:16:27 +0000 (20:16 -0800)]
Avoid some table rewrites for ALTER TABLE .. SET DATA TYPE timestamp.

When the timezone is UTC, timestamptz and timestamp are binary coercible
in both directions.  See b8a18ad4850ea5ad7884aa6ab731fd392e73b4ad and
c22ecc6562aac895f0f0529707d7bdb460fd2a49 for the previous attempt in
this problem space.  Skip the table rewrite; for now, continue to
needlessly rewrite any index on an affected column.

Reviewed by Simon Riggs and Tom Lane.

Discussion: https://postgr.es/m/20190226061450.GA1665944@rfd.leadboat.com

5 years agoTighten use of OpenTransientFile and CloseTransientFile
Michael Paquier [Fri, 8 Mar 2019 23:50:55 +0000 (08:50 +0900)]
Tighten use of OpenTransientFile and CloseTransientFile

This fixes two sets of issues related to the use of transient files in
the backend:
1) OpenTransientFile() has been used in some code paths with read-write
flags while read-only is sufficient, so switch those calls to be
read-only where necessary.  These have been reported by Joe Conway.
2) When opening transient files, it is up to the caller to close the
file descriptors opened.  In error code paths, CloseTransientFile() gets
called to clean up things before issuing an error.  However in normal
exit paths, a lot of callers of CloseTransientFile() never actually
reported errors, which could leave a file descriptor open without
knowing about it.  This is an issue I complained about a couple of
times, but never had the courage to write and submit a patch, so here we
go.

Note that one frontend code path is impacted by this commit so as an
error is issued when fetching control file data, making backend and
frontend to be treated consistently.

Reported-by: Joe Conway, Michael Paquier
Author: Michael Paquier
Reviewed-by: Álvaro Herrera, Georgios Kokolatos, Joe Conway
Discussion: https://postgr.es/m/20190301023338.GD1348@paquier.xyz
Discussion: https://postgr.es/m/c49b69ec-e2f7-ff33-4f17-0eaa4f2cef27@joeconway.com

5 years agoFix crash with old libxml2
Alvaro Herrera [Fri, 8 Mar 2019 22:13:25 +0000 (19:13 -0300)]
Fix crash with old libxml2

Certain libxml2 versions (such as the 2.7.6 commonly seen in older
distributions, but apparently only on x86_64) contain a bug that causes
xmlCopyNode, when called on a XML_DOCUMENT_NODE, to return a node that
xmlFreeNode crashes on.  Arrange to call xmlFreeDoc instead of
xmlFreeNode for those nodes.

Per buildfarm members lapwing and grison.

Author: Pavel Stehule, light editing by Álvaro.
Discussion: https://postgr.es/m/20190308024436.GA2374@alvherre.pgsql

5 years agoReformat catalog .dat files.
Tom Lane [Fri, 8 Mar 2019 17:01:27 +0000 (12:01 -0500)]
Reformat catalog .dat files.

Test run for my previous commit; cleans up formatting issues in some
other recent commits.

5 years agoMinor improvements for reformat_dat_file.pl.
Tom Lane [Fri, 8 Mar 2019 16:48:49 +0000 (11:48 -0500)]
Minor improvements for reformat_dat_file.pl.

Use Getopt::Long in preference to hand-rolled option parsing code.

Also, remove "-I .../backend/catalog" switch from the Makefile
invocations.  That's been unnecessary for some time, and leaving it
there gives the false impression it's needed in manual invocations.

John Naylor (extracted from a larger but more controversial patch)

Discussion: https://postgr.es/m/CACPNZCsHdcQN2jQ1=ptbi1Co2Nj3aHgRCUMk62=ThgWNabPY+Q@mail.gmail.com

5 years agoFix function signatures of pageinspect in documentation
Michael Paquier [Fri, 8 Mar 2019 06:10:14 +0000 (15:10 +0900)]
Fix function signatures of pageinspect in documentation

tuple_data_split() lacked the type of the first argument, and
heap_page_item_attrs() has reversed the first and second argument,
with the bytea argument using an incorrect name.

Author: Laurenz Albe
Discussion: https://postgr.es/m/8f9ab7b16daf623e87eeef5203a4ffc0dece8dfd.camel@cybertec.at
Backpatch-through: 9.6

5 years agoFix compatibility of pg_basebackup -R with 11 and older versions
Michael Paquier [Fri, 8 Mar 2019 01:14:03 +0000 (10:14 +0900)]
Fix compatibility of pg_basebackup -R with 11 and older versions

When 2dedf4d9 has integrated recovery.conf into postgresql.conf, it also
changed pg_basebackup -R in the way recovery configuration is
generated.  However this implementation forgot the fact that
pg_basebackup needs to keep compatibility with older server versions as
well.

Reported-by: Devrim Gündüz
Author: Sergei Kornilov, Michael Paquier
Discussion: https://postgr.es/m/3458f7cd12d74acd90180a671c8d5a081d60e162.camel@gunduz.org

5 years agoFix minor deficiencies in XMLTABLE, xpath(), xmlexists()
Alvaro Herrera [Thu, 7 Mar 2019 18:21:56 +0000 (15:21 -0300)]
Fix minor deficiencies in XMLTABLE, xpath(), xmlexists()

Correctly process nodes of more types than previously.  In some cases,
nodes were being ignored (nothing was output); in other cases, trying to
return them resulted in errors about unrecognized nodes.  In yet other
cases, necessary escaping (of XML special characters) was not being
done.  Fix all those (as far as the authors could find) and add
regression tests cases verifying the new behavior.

I (Álvaro) was of two minds about backpatching these changes.  They do
seem bugfixes that would benefit most users of the affected functions;
but on the other hand it would change established behavior in minor
releases, so it seems prudent not to.

Authors: Pavel Stehule, Markus Winand, Chapman Flack
Discussion:
   https://postgr.es/m/CAFj8pRA6J25CtAZ2TuRvxK3gat7-bBUYh0rfE2yM7Hj9GD14Dg@mail.gmail.com
   https://postgr.es/m/8BDB0627-2105-4564-AA76-7849F028B96E@winand.at

The elephant in the room as pointed out by Chapman Flack, not fixed in
this commit, is that we still have XMLTABLE operating on XPath 1.0
instead of the standard-mandated XQuery (or even its subset XPath 2.0).
Fixing that is a major undertaking, however.

5 years agoFix handling of targetlist SRFs when scan/join relation is known empty.
Tom Lane [Thu, 7 Mar 2019 19:21:52 +0000 (14:21 -0500)]
Fix handling of targetlist SRFs when scan/join relation is known empty.

When we introduced separate ProjectSetPath nodes for application of
set-returning functions in v10, we inadvertently broke some cases where
we're supposed to recognize that the result of a subquery is known to be
empty (contain zero rows).  That's because IS_DUMMY_REL was just looking
for a childless AppendPath without allowing for a ProjectSetPath being
possibly stuck on top.  In itself, this didn't do anything much worse
than produce slightly worse plans for some corner cases.

Then in v11, commit 11cf92f6e rearranged things to allow the scan/join
targetlist to be applied directly to partial paths before they get
gathered.  But it inserted a short-circuit path for dummy relations
that was a little too short: it failed to insert a ProjectSetPath node
at all for a targetlist containing set-returning functions, resulting in
bogus "set-valued function called in context that cannot accept a set"
errors, as reported in bug #15669 from Madelaine Thibaut.

The best way to fix this mess seems to be to reimplement IS_DUMMY_REL
so that it drills down through any ProjectSetPath nodes that might be
there (and it seems like we'd better allow for ProjectionPath as well).

While we're at it, make it look at rel->pathlist not cheapest_total_path,
so that it gives the right answer independently of whether set_cheapest
has been done lately.  That dependency looks pretty shaky in the context
of code like apply_scanjoin_target_to_paths, and even if it's not broken
today it'd certainly bite us at some point.  (Nastily, unsafe use of the
old coding would almost always work; the hazard comes down to possibly
looking through a dangling pointer, and only once in a blue moon would
you find something there that resulted in the wrong answer.)

It now looks like it was a mistake for IS_DUMMY_REL to be a macro: if
there are any extensions using it, they'll continue to use the old
inadequate logic until they're recompiled, after which they'll fail
to load into server versions predating this fix.  Hopefully there are
few such extensions.

Having fixed IS_DUMMY_REL, the special path for dummy rels in
apply_scanjoin_target_to_paths is unnecessary as well as being wrong,
so we can just drop it.

Also change a few places that were testing for partitioned-ness of a
planner relation but not using IS_PARTITIONED_REL for the purpose; that
seems unsafe as well as inconsistent, plus it required an ugly hack in
apply_scanjoin_target_to_paths.

In passing, save a few cycles in apply_scanjoin_target_to_paths by
skipping processing of pre-existing paths for partitioned rels,
and do some cosmetic cleanup and comment adjustment in that function.

I renamed IS_DUMMY_PATH to IS_DUMMY_APPEND with the intention of breaking
any code that might be using it, since in almost every case that would
be wrong; IS_DUMMY_REL is what to be using instead.

In HEAD, also make set_dummy_rel_pathlist static (since it's no longer
used from outside allpaths.c), and delete is_dummy_plan, since it's no
longer used anywhere.

Back-patch as appropriate into v11 and v10.

Tom Lane and Julien Rouhaud

Discussion: https://postgr.es/m/15669-02fb3296cca26203@postgresql.org

5 years agoAllow ATTACH PARTITION with only ShareUpdateExclusiveLock.
Robert Haas [Thu, 7 Mar 2019 16:13:12 +0000 (11:13 -0500)]
Allow ATTACH PARTITION with only ShareUpdateExclusiveLock.

We still require AccessExclusiveLock on the partition itself, because
otherwise an insert that violates the newly-imposed partition
constraint could be in progress at the same time that we're changing
that constraint; only the lock level on the parent relation is
weakened.

To make this safe, we have to cope with (at least) three separate
problems. First, relevant DDL might commit while we're in the process
of building a PartitionDesc.  If so, find_inheritance_children() might
see a new partition while the RELOID system cache still has the old
partition bound cached, and even before invalidation messages have
been queued.  To fix that, if we see that the pg_class tuple seems to
be missing or to have a null relpartbound, refetch the value directly
from the table. We can't get the wrong value, because DETACH PARTITION
still requires AccessExclusiveLock throughout; if we ever want to
change that, this will need more thought. In testing, I found it quite
difficult to hit even the null-relpartbound case; the race condition
is extremely tight, but the theoretical risk is there.

Second, successive calls to RelationGetPartitionDesc might not return
the same answer.  The query planner will get confused if lookup up the
PartitionDesc for a particular relation does not return a consistent
answer for the entire duration of query planning.  Likewise, query
execution will get confused if the same relation seems to have a
different PartitionDesc at different times.  Invent a new
PartitionDirectory concept and use it to ensure consistency.  This
ensures that a single invocation of either the planner or the executor
sees the same view of the PartitionDesc from beginning to end, but it
does not guarantee that the planner and the executor see the same
view.  Since this allows pointers to old PartitionDesc entries to
survive even after a relcache rebuild, also postpone removing the old
PartitionDesc entry until we're certain no one is using it.

For the most part, it seems to be OK for the planner and executor to
have different views of the PartitionDesc, because the executor will
just ignore any concurrently added partitions which were unknown at
plan time; those partitions won't be part of the inheritance
expansion, but invalidation messages will trigger replanning at some
point.  Normally, this happens by the time the very next command is
executed, but if the next command acquires no locks and executes a
prepared query, it can manage not to notice until a new transaction is
started.  We might want to tighten that up, but it's material for a
separate patch.  There would still be a small window where a query
that started just after an ATTACH PARTITION command committed might
fail to notice its results -- but only if the command starts before
the commit has been acknowledged to the user. All in all, the warts
here around serializability seem small enough to be worth accepting
for the considerable advantage of being able to add partitions without
a full table lock.

Although in general the consequences of new partitions showing up
between planning and execution are limited to the query not noticing
the new partitions, run-time partition pruning will get confused in
that case, so that's the third problem that this patch fixes.
Run-time partition pruning assumes that indexes into the PartitionDesc
are stable between planning and execution.  So, add code so that if
new partitions are added between plan time and execution time, the
indexes stored in the subplan_map[] and subpart_map[] arrays within
the plan's PartitionedRelPruneInfo get adjusted accordingly.  There
does not seem to be a simple way to generalize this scheme to cope
with partitions that are removed, mostly because they could then get
added back again with different bounds, but it works OK for added
partitions.

This code does not try to ensure that every backend participating in
a parallel query sees the same view of the PartitionDesc.  That
currently doesn't matter, because we never pass PartitionDesc
indexes between backends.  Each backend will ignore the concurrently
added partitions which it notices, and it doesn't matter if different
backends are ignoring different sets of concurrently added partitions.
If in the future that matters, for example because we allow writes in
parallel query and want all participants to do tuple routing to the same
set of partitions, the PartitionDirectory concept could be improved to
share PartitionDescs across backends.  There is a draft patch to
serialize and restore PartitionDescs on the thread where this patch
was discussed, which may be a useful place to start.

Patch by me.  Thanks to Alvaro Herrera, David Rowley, Simon Riggs,
Amit Langote, and Michael Paquier for discussion, and to Alvaro
Herrera for some review.

Discussion: http://postgr.es/m/CA+Tgmobt2upbSocvvDej3yzokd7AkiT+PvgFH+a9-5VV1oJNSQ@mail.gmail.com
Discussion: http://postgr.es/m/CA+TgmoZE0r9-cyA-aY6f8WFEROaDLLL7Vf81kZ8MtFCkxpeQSw@mail.gmail.com
Discussion: http://postgr.es/m/CA+TgmoY13KQZF-=HNTrt9UYWYx3_oYOQpu9ioNT49jGgiDpUEA@mail.gmail.com

5 years agoFix broken markup
Alvaro Herrera [Thu, 7 Mar 2019 14:35:39 +0000 (11:35 -0300)]
Fix broken markup

5 years agoFix the BY {REF,VALUE} clause of XMLEXISTS/XMLTABLE
Alvaro Herrera [Thu, 7 Mar 2019 14:17:09 +0000 (11:17 -0300)]
Fix the BY {REF,VALUE} clause of XMLEXISTS/XMLTABLE

This clause is used to indicate the passing mode of a XML document, but
we were doing it wrong: we accepted BY REF and ignored it, and rejected
BY VALUE as a syntax error.  The reality, however, is that documents are
always passed BY VALUE, so rejecting that clause was silly.  Change
things so that we accept BY VALUE.

BY REF continues to be accepted, and continues to be ignored.

Author: Chapman Flack
Reviewed-by: Pavel Stehule
Discussion: https://postgr.es/m/5C297BB7.9070509@anastigmatix.net

5 years agoAdd missing <limits.h>
Alvaro Herrera [Thu, 7 Mar 2019 13:08:29 +0000 (10:08 -0300)]
Add missing <limits.h>

Per buildfarm

5 years agopg_dump: allow multiple rows per insert
Alvaro Herrera [Thu, 7 Mar 2019 12:26:14 +0000 (09:26 -0300)]
pg_dump: allow multiple rows per insert

This is useful to speed up loading data in a different database engine.

Authors: Surafel Temesgen and David Rowley.  Lightly edited by Álvaro.
Reviewed-by: Fabien Coelho
Discussion: https://postgr.es/m/CALAY4q9kumSdnRBzvRJvSRf2+BH20YmSvzqOkvwpEmodD-xv6g@mail.gmail.com

5 years agoRemove useless header inclusion.
Thomas Munro [Thu, 7 Mar 2019 06:38:47 +0000 (19:38 +1300)]
Remove useless header inclusion.

5 years agoDrop the vestigial "smgr" type.
Thomas Munro [Thu, 7 Mar 2019 02:43:37 +0000 (15:43 +1300)]
Drop the vestigial "smgr" type.

Before commit 3fa2bb31 this type appeared in the catalogs to
select which of several block storage mechanisms each relation
used.

New features under development propose to revive the concept of
different block storage managers for new kinds of data accessed
via bufmgr.c, but don't need to put references to them in the
catalogs.  So, avoid useless maintenance work on this type by
dropping it.  Update some regression tests that were referencing
it where any type would do.

Discussion: https://postgr.es/m/CA%2BhUKG%2BDE0mmiBZMtZyvwWtgv1sZCniSVhXYsXkvJ_Wo%2B83vvw%40mail.gmail.com

5 years agoDon't reuse slots between root and partition in ON CONFLICT ... UPDATE.
Andres Freund [Wed, 6 Mar 2019 23:43:33 +0000 (15:43 -0800)]
Don't reuse slots between root and partition in ON CONFLICT ... UPDATE.

Until now the the slot to store the conflicting tuple, and the result
of the ON CONFLICT SET, where reused between partitions. That
necessitated changing slots descriptor when switching partitions.

Besides the overhead of switching descriptors on a slot (which
requires memory allocations and prevents JITing), that's importantly
also problematic for tableam. There individual partitions might belong
to different tableams, needing different kinds of slots.

In passing also fix ExecOnConflictUpdate to clear the existing slot at
exit. Otherwise that slot could continue to hold a pin till the query
ends, which could be far too long if the input data set is large, and
there's no further conflicts. While previously also problematic, it's
now more important as there will be more such slots when partitioned.

Author: Andres Freund
Reviewed-By: Robert Haas, David Rowley
Discussion: https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de

5 years agoFix equalfuncs for accessMethod addition in 8586bf7ed8.
Andres Freund [Wed, 6 Mar 2019 21:04:09 +0000 (13:04 -0800)]
Fix equalfuncs for accessMethod addition in 8586bf7ed8.

In a complete brown paper bag moment, I forgot to include equalfuncs
in my previous fix of copy/out/readfuncs.  Thanks Tom for noticing.

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

5 years agoDon't log incomplete startup packet if it's empty
Andrew Dunstan [Wed, 6 Mar 2019 20:27:19 +0000 (15:27 -0500)]
Don't log incomplete startup packet if it's empty

This will stop logging cases where, for example, a monitor opens a
connection and immediately closes it. If the packet contains any data an
incomplete packet will still be logged.

Author: Tom Lane

Discussion: https://postgr.es/m/a1379a72-2958-1ed0-ef51-09a21219b155@2ndQuadrant.com

5 years agoFix copy/out/readfuncs for accessMethod addition in 8586bf7ed8.
Andres Freund [Wed, 6 Mar 2019 19:55:28 +0000 (11:55 -0800)]
Fix copy/out/readfuncs for accessMethod addition in 8586bf7ed8.

This includes a catversion bump, as IntoClause is theoretically
speaking part of storable rules. In practice I don't think that can
happen, but there's no reason to be stingy here.

Per buildfarm member calliphoridae.

5 years agoFix collation dependency in test introduced in 8586bf7ed8, take 2.
Andres Freund [Wed, 6 Mar 2019 19:06:01 +0000 (11:06 -0800)]
Fix collation dependency in test introduced in 8586bf7ed8, take 2.

Per buildfarm.  This time I hopefully actually made sure to get all
the cases...

5 years agoFix collation dependency in test introduced in 8586bf7ed8.
Andres Freund [Wed, 6 Mar 2019 19:06:01 +0000 (11:06 -0800)]
Fix collation dependency in test introduced in 8586bf7ed8.

Per buildfarm.

5 years agotableam: Add pg_dump support.
Andres Freund [Wed, 6 Mar 2019 17:54:38 +0000 (09:54 -0800)]
tableam: Add pg_dump support.

This adds pg_dump support for table AMs in a similar manner to how
tablespaces are handled. That is, instead of specifying the AM for
every CREATE TABLE etc, emit SET default_table_access_method
statements. That makes it easier to change the AM for all/most tables
in a dump, and allows restore to succeed even if some AM is not
available.

This increases the dump archive version, as a tables/matview's AM
needs to be tracked therein.

Author: Dimitri Dolgov, Andres Freund
Discussion:
    https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de
    https://postgr.es/m/20190304234700.w5tmhducs5wxgzls@alap3.anarazel.de

5 years agotableam: introduce table AM infrastructure.
Andres Freund [Wed, 6 Mar 2019 17:54:38 +0000 (09:54 -0800)]
tableam: introduce table AM infrastructure.

This introduces the concept of table access methods, i.e. CREATE
  ACCESS METHOD ... TYPE TABLE and
  CREATE TABLE ... USING (storage-engine).
No table access functionality is delegated to table AMs as of this
commit, that'll be done in following commits.

Subsequent commits will incrementally abstract table access
functionality to be routed through table access methods. That change
is too large to be reviewed & committed at once, so it'll be done
incrementally.

Docs will be updated at the end, as adding them incrementally would
likely make them less coherent, and definitely is a lot more work,
without a lot of benefit.

Table access methods are specified similar to index access methods,
i.e. pg_am.amhandler returns, as INTERNAL, a pointer to a struct with
callbacks. In contrast to index AMs that struct needs to live as long
as a backend, typically that's achieved by just returning a pointer to
a constant struct.

Psql's \d+ now displays a table's access method. That can be disabled
with HIDE_TABLEAM=true, which is mainly useful so regression tests can
be run against different AMs.  It's quite possible that this behaviour
still needs to be fine tuned.

For now it's not allowed to set a table AM for a partitioned table, as
we've not resolved how partitions would inherit that. Disallowing
allows us to introduce, if we decide that's the way forward, such a
behaviour without a compatibility break.

Catversion bumped, to add the heap table AM and references to it.

Author: Haribabu Kommi, Andres Freund, Alvaro Herrera, Dimitri Golgov and others
Discussion:
    https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de
    https://postgr.es/m/20160812231527.GA690404@alvherre.pgsql
    https://postgr.es/m/20190107235616.6lur25ph22u5u5av@alap3.anarazel.de
    https://postgr.es/m/20190304234700.w5tmhducs5wxgzls@alap3.anarazel.de

5 years agoFix bug in clearing of virtual tuple slot.
Andres Freund [Wed, 6 Mar 2019 17:54:38 +0000 (09:54 -0800)]
Fix bug in clearing of virtual tuple slot.

I broke/typoed this in 4da597edf1bae0c. Astonishingly this mostly
doesn't cause breakage, except when trying to change the tuple
descriptor of a slot (because TTS_FLAG_FIXED is assumed to be set).

Author: Andres Freund

5 years agoRemoved unused variable, openLogOff.
Robert Haas [Wed, 6 Mar 2019 14:30:50 +0000 (09:30 -0500)]
Removed unused variable, openLogOff.

Antonin Houska

Discussion: http://postgr.es/m/30413.1551870730@localhost

5 years agoIncrease the default vacuum_cost_limit from 200 to 2000
Andrew Dunstan [Wed, 6 Mar 2019 14:10:12 +0000 (09:10 -0500)]
Increase the default vacuum_cost_limit from 200 to 2000

The original 200 default value was set back in f425b605f4e when the cost
delay settings were first added.  Hardware has improved quite a bit since
then and we've also made improvements such as sorting buffers during
checkpoints (9cd00c457e6) which should result in less random writes.

This low default value was reportedly causing problems with badly
configured servers and in the absence of a native method to remove
excessive bloat from tables without incurring an AccessExclusiveLock, this
often made cleaning up the damage caused by badly configured auto-vacuums
difficult.

It seems more likely that someone will notice that auto-vacuum is running
too quickly than too slowly, so let's go all out and multiple the default
value for the setting by 10.  With the default vacuum_cost_page_dirty and
autovacuum_vacuum_cost_delay (assuming a page size of 8192 bytes), this
allows autovacuum a theoretical maximum dirty write rate of around 39MB/s
instead of just 3.9MB/s.

Author: David Rowley

Discussion: https://postgr.es/m/CAKJS1f_YbXC2qTMPyCbmsPiKvZYwpuQNQMohiRXLj1r=8_rYvw@mail.gmail.com

5 years agoTeach SKIP_LOCKED to psql tab completion of VACUUM and ANALYZE
Michael Paquier [Wed, 6 Mar 2019 05:42:52 +0000 (14:42 +0900)]
Teach SKIP_LOCKED to psql tab completion of VACUUM and ANALYZE

This was missing since 803b130, which has introduced the option for the
user-facing VACUUM and ANALYZE.

Author: Masahiko Sawada
Discussion: https://postgr.es/m/CAD21AoD2TMdTxRhZ7WSp940V82_OAyPmgHnbi25UbbArLgA92Q@mail.gmail.com

5 years agoFix pgbench TAP test failure with funky file names (redux)
Andrew Dunstan [Tue, 5 Mar 2019 15:46:21 +0000 (10:46 -0500)]
Fix pgbench TAP test failure with funky file names (redux)

This test fails if the containing directory contains a funny character
such as a space or some perl metacharacter. To avoid that, we check for
files names using readdir and a regex, rather than using a glob pattern.

Discussion: https://postgr.es/m/CAM6_UM6dGdU39PKAC24T+HD9ouy0jLN9vH6163K8QEEzr__iZw@mail.gmail.com

Author: Fabien COELHO
Reviewed-by: Raúl Marín Rodríguez
5 years agoRemove duplicate macro
Peter Eisentraut [Tue, 5 Mar 2019 14:02:56 +0000 (15:02 +0100)]
Remove duplicate macro

The original commit appears to have accidentally introduced a
duplicate definition.  Keep only one of them.

5 years agoScan GiST indexes in physical order during VACUUM.
Heikki Linnakangas [Tue, 5 Mar 2019 13:19:48 +0000 (15:19 +0200)]
Scan GiST indexes in physical order during VACUUM.

Scanning an index in physical order is faster than walking it in logical
order, because sequential I/O is faster than random I/O. The idea and code
structure is borrowed from B-tree vacuum code.

Patch by Andrey Borodin, with changes by me. Based on early work by
Konstantin Kuznetsov, although the patch has been rewritten multiple times
since his original version.

Discussion: https://www.postgresql.org/message-id/1B9FAC6F-FA19-4A24-8C1B-F4F574844892%40yandex-team.ru

5 years agoNote case where nbtree VACUUM finishes splits.
Peter Geoghegan [Tue, 5 Mar 2019 01:57:36 +0000 (17:57 -0800)]
Note case where nbtree VACUUM finishes splits.

The nbtree README claims that VACUUM can never finish interrupted page
splits by design.  That isn't entirely accurate, though.  Note an
exception to the general rule.

Discussion: https://postgr.es/m/CAH2-Wz=_Xvv8byzK_LvY4ci76OgsHCQzoKF7We8yG9waO7j6rA@mail.gmail.com

5 years agoDisable dump_connstr test on Msys2
Andrew Dunstan [Mon, 4 Mar 2019 22:11:18 +0000 (17:11 -0500)]
Disable dump_connstr test on Msys2

For some reason the dump test with names with high bits set fails on
Msys2 (although not Msys1). Disable the tests for now, so that other
tests can run.

5 years agoAllow recovery tests to run on Windows as an admin user
Andrew Dunstan [Mon, 4 Mar 2019 20:50:23 +0000 (15:50 -0500)]
Allow recovery tests to run on Windows as an admin user

This is the only test that fails when run as an admin user. The reason
is that when Postgres is started via pg_ctl its admin privileges are
lowered. However, this test called 'postgres -D datadir' directly,
resulting in a failure. Replace that by calling pg_ctl and then checking
the result for the expected failure, and the logfile for the expected
error message.

5 years agoCorrect obsolete nbtree page split WAL comment.
Peter Geoghegan [Mon, 4 Mar 2019 20:32:40 +0000 (12:32 -0800)]
Correct obsolete nbtree page split WAL comment.

Commit 2c03216d831, which revamped the WAL record format, failed to
update a comment referencing the old API.  Update the comment.

5 years agoReorder configure tests for accept() in Windows
Andrew Dunstan [Mon, 4 Mar 2019 18:53:06 +0000 (13:53 -0500)]
Reorder configure tests for accept() in Windows

Currently only frogmouth in the buildfarm uses the 32bit params, and
it's not able to build past release 10, so put those last, saving
substantial configure time on more modern systems. Even if we get a
modern 32 bit Windows system at some stage we should probably prefer the
64 bit interface here these days.

5 years agopg_partition_ancestors
Alvaro Herrera [Mon, 4 Mar 2019 19:14:29 +0000 (16:14 -0300)]
pg_partition_ancestors

Adds another introspection feature for partitioning, necessary for
further psql patches.

Reviewed-by: Michaël Paquier
Discussion: https://postgr.es/m/20190226222757.GA31622@alvherre.pgsql

5 years agoTest partition functions with legacy inheritance children, too
Alvaro Herrera [Mon, 4 Mar 2019 18:52:41 +0000 (15:52 -0300)]
Test partition functions with legacy inheritance children, too

It's worth immortalizing this behavior, per discussion.

Discussion: https://postgr.es/m/20190228193203.GA26151@alvherre.pgsql

5 years agoRemove volatile from latch API
Peter Eisentraut [Mon, 25 Feb 2019 08:24:15 +0000 (09:24 +0100)]
Remove volatile from latch API

This was no longer useful since the latch functions use memory
barriers already, which are also compiler barriers, and volatile does
not help with cross-process access.

Discussion: https://www.postgresql.org/message-id/flat/20190218202511.qsfpuj5sy4dbezcw%40alap3.anarazel.de#18783c27d73e9e40009c82f6e0df0974

5 years agoFix error handling of readdir() port implementation on first file lookup
Michael Paquier [Mon, 4 Mar 2019 00:49:06 +0000 (09:49 +0900)]
Fix error handling of readdir() port implementation on first file lookup

The implementation of readdir() in src/port/ which gets used by MSVC has
been added in 399a36a, and since the beginning it considers all errors
on the first file lookup as ENOENT, setting errno accordingly and
letting the routine caller think that the directory is empty.  While
this is normally enough for the case of the backend, this can confuse
callers of this routine on Windows as all errors would map to the same
behavior.  So, for example, even permission errors would be thought as
having an empty directory, while there could be contents in it.

This commit changes the error handling so as readdir() gets a behavior
similar to native implementations: force errno=0 when seeing
ERROR_FILE_NOT_FOUND as error and consider other errors as plain
failures.

While looking at the patch, I noticed that MinGW does not enforce
errno=0 when looking at the first file, but it gets enforced on the next
file lookups.  A comment related to that was incorrect in the code.

Reported-by: Yuri Kurenkov
Diagnosed-by: Yuri Kurenkov, Grigory Smolkin
Author: Konstantin Knizhnik
Reviewed-by: Andrew Dunstan, Michael Paquier
Discussion: https://postgr.es/m/2cad7829-8d66-e39c-b937-ac825db5203d@postgrespro.ru
Backpatch-through: 9.4

5 years agofix thinko in logrotate test
Andrew Dunstan [Mon, 4 Mar 2019 00:15:47 +0000 (19:15 -0500)]
fix thinko in logrotate test

5 years agoDon't do pg_ctl logrotate test on Windows
Andrew Dunstan [Sun, 3 Mar 2019 23:19:44 +0000 (18:19 -0500)]
Don't do pg_ctl logrotate test on Windows

The test crashes and burns quite badly, for some reason, but even if it
didn't it wouldn't work, since Windows doesn't let you rename a file
held by a running process.

5 years agoImprove performance of index-only scans with many index columns.
Tom Lane [Sun, 3 Mar 2019 21:57:14 +0000 (16:57 -0500)]
Improve performance of index-only scans with many index columns.

StoreIndexTuple was a loop over index_getattr, which is O(N^2)
if the index columns are variable-width, and the performance
impact is already quite visible at ten columns.  The obvious
move is to replace that with a call to index_deform_tuple ...
but that's *also* a loop over index_getattr.  Improve it to
be essentially a clone of heap_deform_tuple.

(There are a few other places that loop over all index columns
with index_getattr, and perhaps should be changed likewise,
but most of them don't seem performance-critical.  Anyway, the
rest would mostly only be interested in the index key columns,
which there aren't likely to be so many of.  Wide index tuples
are a new thing with INCLUDE.)

Konstantin Knizhnik

Discussion: https://postgr.es/m/e06b2d27-04fc-5c0e-bb8c-ecd72aa24959@postgrespro.ru

5 years agoAvoid accidental wildcard expansion in msys shell
Andrew Dunstan [Sun, 3 Mar 2019 16:48:12 +0000 (11:48 -0500)]
Avoid accidental wildcard expansion in msys shell

Commit f092de05 added a test for pg_dumpall --exclude-database including
the wildcard pattern '*dump*' which matches some files in the source
directory. The test library on msys uses the shell which expands this
and thus the program gets incorrect arguments. This doesn't happen if
the pattern doesn't match any files, so here the pattern is set to
'*dump_test*' which is such a pattern.

Per buildfarm animal jacana.

5 years agoFurther fixing for multi-row VALUES lists for updatable views.
Dean Rasheed [Sun, 3 Mar 2019 10:51:13 +0000 (10:51 +0000)]
Further fixing for multi-row VALUES lists for updatable views.

Previously, rewriteTargetListIU() generated a list of attribute
numbers from the targetlist, which were passed to rewriteValuesRTE(),
which expected them to contain the same number of entries as there are
columns in the VALUES RTE, and to be in the same order. That was fine
when the target relation was a table, but for an updatable view it
could be broken in at least three different ways ---
rewriteTargetListIU() could insert additional targetlist entries for
view columns with defaults, the view columns could be in a different
order from the columns of the underlying base relation, and targetlist
entries could be merged together when assigning to elements of an
array or composite type. As a result, when recursing to the base
relation, the list of attribute numbers generated from the rewritten
targetlist could no longer be relied upon to match the columns of the
VALUES RTE. We got away with that prior to 41531e42d3 because it used
to always be the case that rewriteValuesRTE() did nothing for the
underlying base relation, since all DEFAULTS had already been replaced
when it was initially invoked for the view, but that was incorrect
because it failed to apply defaults from the base relation.

Fix this by examining the targetlist entries more carefully and
picking out just those that are simple Vars referencing the VALUES
RTE. That's sufficient for the purposes of rewriteValuesRTE(), which
is only responsible for dealing with DEFAULT items in the VALUES
RTE. Any DEFAULT item in the VALUES RTE that doesn't have a matching
simple-Var-assignment in the targetlist is an error which we complain
about, but in theory that ought to be impossible.

Additionally, move this code into rewriteValuesRTE() to give a clearer
separation of concerns between the 2 functions. There is no need for
rewriteTargetListIU() to know about the details of the VALUES RTE.

While at it, fix the comment for rewriteValuesRTE() which claimed that
it doesn't support array element and field assignments --- that hasn't
been true since a3c7a993d5 (9.6 and later).

Back-patch to all supported versions, with minor differences for the
pre-9.6 branches, which don't support array element and field
assignments to the same column in multi-row VALUES lists.

Reviewed by Amit Langote.

Discussion: https://postgr.es/m/15623-5d67a46788ec8b7f@postgresql.org

5 years agoConsider only relations part of partition trees in partition functions
Michael Paquier [Sat, 2 Mar 2019 09:18:59 +0000 (18:18 +0900)]
Consider only relations part of partition trees in partition functions

This changes the partition functions so as tables and indexes which are
not part of partition trees are handled the same way as what is done for
undefined objects and unsupported relkinds: pg_partition_tree() returns
no rows and pg_partition_root() returns a NULL result.  Hence,
partitioned tables, partitioned indexes and relations whose flag
pg_class.relispartition is set are considered as valid objects to
process.

Previously, tables and indexes not included in a partition tree were
processed the same way as a partition or a partitioned table, which
caused the functions to return inconsistent results for inherited
tables, especially when inheriting from multiple tables.

Reported-by: Álvaro Herrera
Author: Amit Langote, Michael Paquier
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/20190228193203.GA26151@alvherre.pgsql

5 years agoUse a virtual rather than a heap slot in two places where that suffices.
Andres Freund [Sat, 2 Mar 2019 01:24:57 +0000 (17:24 -0800)]
Use a virtual rather than a heap slot in two places where that suffices.

Author: Andres Freund
Discussion: https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de

5 years agoCheck we don't misoptimize a NOT IN where the subquery returns no rows.
Tom Lane [Fri, 1 Mar 2019 22:57:20 +0000 (17:57 -0500)]
Check we don't misoptimize a NOT IN where the subquery returns no rows.

Future-proofing against a common mistake in attempts to optimize NOT IN.
We don't have such an optimization right now, but attempts to do so
are in the works, and some of 'em are buggy.  Add a regression test case
covering the point.

David Rowley

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

5 years agoTeach optimizer's predtest.c more things about ScalarArrayOpExpr.
Tom Lane [Fri, 1 Mar 2019 22:14:07 +0000 (17:14 -0500)]
Teach optimizer's predtest.c more things about ScalarArrayOpExpr.

In particular, make it possible to prove/refute "x IS NULL" and
"x IS NOT NULL" predicates from a clause involving a ScalarArrayOpExpr
even when we are unable or unwilling to deconstruct the expression
into an AND/OR tree.  This avoids a former unexpected degradation of
plan quality when the size of an ARRAY[] expression or array constant
exceeded the arbitrary MAX_SAOP_ARRAY_SIZE limit.  For IS-NULL proofs,
we don't really care about the values of the individual array elements;
at most, we care whether there are any, and for some common cases we
needn't even know that.

The main user-visible effect of this is to let the optimizer recognize
applicability of partial indexes with "x IS NOT NULL" predicates to
queries with "x IN (array)" clauses in some cases where it previously
failed to recognize that.  The structure of predtest.c is such that a
bunch of related proofs will now also succeed, but they're probably
much less useful in the wild.

James Coleman, reviewed by David Rowley

Discussion: https://postgr.es/m/CAAaqYe8yKSvzbyu8w-dThRs9aTFMwrFxn_BkTYeXgjqe3CbNjg@mail.gmail.com

5 years agoFix whitespace
Peter Eisentraut [Fri, 1 Mar 2019 19:56:53 +0000 (20:56 +0100)]
Fix whitespace

5 years agoRemove tests for pg_dumpall --exclude-database missing argument
Andrew Dunstan [Fri, 1 Mar 2019 19:11:37 +0000 (14:11 -0500)]
Remove tests for pg_dumpall --exclude-database missing argument

It turns out that different getopt implementations spell the error for
missing arguments different ways. This test is of fairly marginal
value, so instead of trying to keep up  with the different error
messages just remove the test.

5 years agoStore tuples for EvalPlanQual in slots, rather than as HeapTuples.
Andres Freund [Fri, 1 Mar 2019 18:37:57 +0000 (10:37 -0800)]
Store tuples for EvalPlanQual in slots, rather than as HeapTuples.

For the upcoming pluggable table access methods it's quite
inconvenient to store tuples as HeapTuples, as that'd require
converting tuples from a their native format into HeapTuples. Instead
use slots to manage epq tuples.

To fit into that scheme, change the foreign data wrapper callback
RefetchForeignRow, to store the tuple in a slot. Insist on using the
caller provided slot, so it conveniently can be stored in the
corresponding EPQ slot.  As there is no in core user of
RefetchForeignRow, that change was done blindly, but we plan to test
that soon.

To avoid duplicating that work for row locks, move row locks to just
directly use the EPQ slots - it previously temporarily stored tuples
in LockRowsState.lr_curtuples, but that doesn't seem beneficial, given
we'd possibly end up with a significant number of additional slots.

The behaviour of es_epqTupleSet[rti -1] is now checked by
es_epqTupleSlot[rti -1] != NULL, as that is distinguishable from a
slot containing an empty tuple.

Author: Andres Freund, Haribabu Kommi, Ashutosh Bapat
Discussion: https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de

5 years agoAdd extra descriptive headings in pg_dumpall
Andrew Dunstan [Fri, 1 Mar 2019 16:38:54 +0000 (11:38 -0500)]
Add extra descriptive headings in pg_dumpall

Headings are added for the User Configurations and Databases sections,
and for each user configuration and database in the output.

Author: Fabien Coelho
Discussion: https://postgr.es/m/alpine.DEB.2.21.1812272222130.32444@lancre

5 years agoAdd --exclude-database option to pg_dumpall
Andrew Dunstan [Fri, 1 Mar 2019 15:47:44 +0000 (10:47 -0500)]
Add --exclude-database option to pg_dumpall

This option functions similarly to pg_dump's --exclude-table option, but
for database names. The option can be given once, and the argument can
be a pattern including wildcard characters.

Author: Andrew Dunstan.
Reviewd-by: Fabien Coelho and Michael Paquier
Discussion: https://postgr.es/m/43a54a47-4aa7-c70e-9ca6-648f436dd6e6@2ndQuadrant.com

5 years agoClear the local map when not used.
Amit Kapila [Fri, 1 Mar 2019 02:08:47 +0000 (07:38 +0530)]
Clear the local map when not used.

After commit b0eaa4c51b, we use a local map of pages to find the required
space for small relations.  We do clear this map when we have found a block
with enough free space, when we extend the relation, or on transaction
abort so that it can be used next time.  However, we miss to clear it when
we didn't find any pages to try from the map which leads to an assertion
failure when we later tried to use it after relation extension.

In the passing, I have improved some comments in this area.

Reported-by: Tom Lane based on buildfarm results
Author: Amit Kapila
Reviewed-by: John Naylor
Tested-by: Kuntal Ghosh
Discussion: https://postgr.es/m/32368.1551114120@sss.pgh.pa.us

5 years agoMake pg_partition_tree return no rows on unsupported and undefined objects
Michael Paquier [Fri, 1 Mar 2019 00:07:07 +0000 (09:07 +0900)]
Make pg_partition_tree return no rows on unsupported and undefined objects

The function was tweaked so as it returned one row full of NULLs when
working on an unsupported relkind or an undefined object as of cc53123,
and after discussion with Amit and Álvaro it looks more natural to make
it return no rows.

Author: Michael Paquier
Reviewed-by: Álvaro Herrera, Amit Langote
Discussion: https://postgr.es/m/20190227184808.GA17357@alvherre.pgsql

5 years agoDon't superfluously materialize slot after DELETE from an FDW.
Andres Freund [Thu, 28 Feb 2019 21:57:49 +0000 (13:57 -0800)]
Don't superfluously materialize slot after DELETE from an FDW.

Previously that was needed to safely store the table oid, but after
b8d71745eac0a127 that's not necessary anymore.

Author: Andres Freund

5 years agoDon't force materializing when copying a buffer tuple table slot.
Andres Freund [Thu, 28 Feb 2019 21:55:27 +0000 (13:55 -0800)]
Don't force materializing when copying a buffer tuple table slot.

After 5408e233f0667478 it's not necessary to force materializing the
target slot when copying from one buffer slot to another. Previously
that was required because the HeapTupleData portion of the source slot
wasn't guaranteed to stay valid long enough, but now we can simply
copy that part into the destination slot's tupdata.

Author: Andres Freund

5 years agoImprove docs for ALTER TABLE .. SET TABLESPACE
Alvaro Herrera [Thu, 28 Feb 2019 22:24:21 +0000 (19:24 -0300)]
Improve docs for ALTER TABLE .. SET TABLESPACE

Discussion: https://postgr.es/m/20190220173815.GA7959@alvherre.pgsql
Reviewed-by: Robert Haas
5 years agoMake get_controlfile not leak file descriptors
Joe Conway [Thu, 28 Feb 2019 20:57:40 +0000 (15:57 -0500)]
Make get_controlfile not leak file descriptors

When backend functions were added to expose controldata via SQL,
reading of pg_control was consolidated under src/common so that
both frontend and backend could share the same code. That move
from frontend-only to shared frontend-backend failed to recognize
the risk (and coding standards violation) of using a bare open().
In particular, it risked leaking file descriptors if transient
errors occurred while reading the file. Fix that by using
OpenTransientFile() instead in the backend case, which is
purpose-built for this type of usage.

Since there have been no complaints from the field, and an intermittent
failure low risk, no backpatch. Hard failure would of course be bad, but
in that case these functions are probably the least of your worries.

Author: Joe Conway
Reviewed-By: Michael Paquier
Reported by: Michael Paquier
Discussion: https://postgr.es/m/20190227074728.GA15710@paquier.xyz

5 years agoAllow buffer tuple table slots to materialize after ExecStoreVirtualTuple().
Andres Freund [Thu, 28 Feb 2019 20:27:58 +0000 (12:27 -0800)]
Allow buffer tuple table slots to materialize after ExecStoreVirtualTuple().

While not common, it can be useful to store a virtual tuple into a
buffer tuple table slot, and then materialize that slot. So far we've
asserted out, which surprisingly wasn't a problem for anything in
core. But that seems fragile, and it also breaks redis_fdw after
ff11e7f4b9.

Thus, allow materializing a virtual tuple stored in a buffer tuple
table slot.

Author: Andres Freund
Discussion:
    https://postgr.es/m/20190227181621.xholonj7ff7ohxsg@alap3.anarazel.de
    https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de

5 years agopg_dump: Fix ArchiveEntry handling of some empty values
Alvaro Herrera [Thu, 28 Feb 2019 20:16:08 +0000 (17:16 -0300)]
pg_dump: Fix ArchiveEntry handling of some empty values

Commit f831d4acc changed what pg_dump emits for some empty fields: they
were output as empty strings before, NULL pointer afterwards.  That
makes old pg_restore unable to work (crash) with such files, which is
unacceptable.  Return to the original representation by explicitly
setting those struct members to "" where needed; remove some no longer
needed checks for NULL input.

We can declutter the code a little by returning to NULLs when we next
update the archive version, so add a note to remind us later.

Discussion: https://postgr.es/m/20190225074539.az6j3u464cvsoxh6@depesz.com
Reported-by: hubert depesz lubaczewski
Author: Dmitry Dolgov

5 years agoMerge near-duplicate code in RI triggers
Peter Eisentraut [Thu, 28 Feb 2019 18:08:55 +0000 (19:08 +0100)]
Merge near-duplicate code in RI triggers

Merge ri_setnull and ri_setdefault into one function ri_set.  These
functions were to a large part identical.

This is a continuation in spirit of
4797f9b519995ceca5d6b8550b5caa2ff6d19347.

Author: Corey Huinker <corey.huinker@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/0ccdd3e1-10b0-dd05-d8a7-183507c11eb1%402ndquadrant.com

5 years agoStandardize some more loops that chase down parallel lists.
Tom Lane [Thu, 28 Feb 2019 19:25:01 +0000 (14:25 -0500)]
Standardize some more loops that chase down parallel lists.

We have forboth() and forthree() macros that simplify iterating
through several parallel lists, but not everyplace that could
reasonably use those was doing so.  Also invent forfour() and
forfive() macros to do the same for four or five parallel lists,
and use those where applicable.

The immediate motivation for doing this is to reduce the number
of ad-hoc lnext() calls, to reduce the footprint of a WIP patch.
However, it seems like good cleanup and error-proofing anyway;
the places that were combining forthree() with a manually iterated
loop seem particularly illegible and bug-prone.

There was some speculation about restructuring related parsetree
representations to reduce the need for parallel list chasing of
this sort.  Perhaps that's a win, or perhaps not, but in any case
it would be considerably more invasive than this patch; and it's
not particularly related to my immediate goal of improving the
List infrastructure.  So I'll leave that question for another day.

Patch by me; thanks to David Rowley for review.

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

5 years agoClean up some variable names in ri_triggers.c
Peter Eisentraut [Thu, 28 Feb 2019 14:29:00 +0000 (15:29 +0100)]
Clean up some variable names in ri_triggers.c

There was a mix of old_slot/oldslot, new_slot/newslot.  Since we've
changed everything from row to slot, we might as well take this
opportunity to clean this up.

Also update some more comments for the slot change.

5 years agoCompact for loops
Peter Eisentraut [Thu, 28 Feb 2019 09:54:44 +0000 (10:54 +0100)]
Compact for loops

Declare loop variable in for loop, for readability and to save space.

Reviewed-by: Corey Huinker <corey.huinker@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/0ccdd3e1-10b0-dd05-d8a7-183507c11eb1%402ndquadrant.com

5 years agoReduce comments
Peter Eisentraut [Thu, 28 Feb 2019 09:54:44 +0000 (10:54 +0100)]
Reduce comments

Reduce the vertical space used by comments in ri_triggers.c, making
the file longer and more tedious to read than it needs to be.  Update
some comments to use a more common style.

Reviewed-by: Corey Huinker <corey.huinker@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/0ccdd3e1-10b0-dd05-d8a7-183507c11eb1%402ndquadrant.com

5 years agoRemove unnecessary unused MATCH PARTIAL code
Peter Eisentraut [Thu, 28 Feb 2019 09:54:44 +0000 (10:54 +0100)]
Remove unnecessary unused MATCH PARTIAL code

ri_triggers.c spends a lot of space catering to a not-yet-implemented
MATCH PARTIAL option.  An actual implementation would probably not use
the existing code structure anyway, so let's just simplify this for
now.

First, have ri_FetchConstraintInfo() check that riinfo->confmatchtype
is valid.  Then we don't have to repeat that everywhere.

In the various referential action functions, we don't need to pay
attention to the match type at all right now, so remove all that code.
A future MATCH PARTIAL implementation would probably have some
conditions added to the present code, but it won't need an entirely
separate switch branch in each case.

In RI_FKey_fk_upd_check_required(), reorganize the code to make it
much simpler.

Reviewed-by: Corey Huinker <corey.huinker@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/0ccdd3e1-10b0-dd05-d8a7-183507c11eb1%402ndquadrant.com

5 years agoUpdate comment
Peter Eisentraut [Thu, 28 Feb 2019 09:43:00 +0000 (10:43 +0100)]
Update comment

for ff11e7f4b9ae017585c3ba146db7ba39c31f209a

5 years agoImprove documentation of data_sync_retry
Michael Paquier [Thu, 28 Feb 2019 02:02:11 +0000 (11:02 +0900)]
Improve documentation of data_sync_retry

Reflecting an updated parameter value requires a server restart, which
was not mentioned in the documentation and in postgresql.conf.sample.

Reported-by: Thomas Poty
Discussion: https://postgr.es/m/15659-0cd812f13027a2d8@postgresql.org

5 years agoFix SCRAM authentication via SSL when mixing versions of OpenSSL
Michael Paquier [Thu, 28 Feb 2019 00:40:28 +0000 (09:40 +0900)]
Fix SCRAM authentication via SSL when mixing versions of OpenSSL

When using a libpq client linked with OpenSSL 1.0.1 or older to connect
to a backend linked with OpenSSL 1.0.2 or newer, the server would send
SCRAM-SHA-256-PLUS and SCRAM-SHA-256 as valid mechanisms for the SASL
exchange, and the client would choose SCRAM-SHA-256-PLUS even if it does
not support channel binding, leading to a confusing error.  In this
case, what the client ought to do is switch to SCRAM-SHA-256 so as the
authentication can move on and succeed.

So for a SCRAM authentication over SSL, here are all the cases present
and how we deal with them using libpq:
1) Server supports channel binding, it sends SCRAM-SHA-256-PLUS and
SCRAM-SHA-256 as allowed mechanisms.
1-1) Client supports channel binding, chooses SCRAM-SHA-256-PLUS.
1-2) Client does not support channel binding, chooses SCRAM-SHA-256.
2) Server does not support channel binding, sends SCRAM-SHA-256 as
allowed mechanism.
2-1) Client supports channel binding, still it has no choice but to
choose SCRAM-SHA-256.
2-2) Client does not support channel binding, it chooses SCRAM-SHA-256.
In all these scenarios the connection should succeed, and the one which
was handled incorrectly prior this commit is 1-2), causing the
connection attempt to fail because client chose SCRAM-SHA-256-PLUS over
SCRAM-SHA-256.

Reported-by: Hugh Ranalli
Diagnosed-by: Peter Eisentraut
Author: Michael Paquier
Reviewed-by: Peter Eisentraut
Discussion: https://postgr.es/m/CAAhbUMO89SqUk-5mMY+OapgWf-twF2NA5sCucbHEzMfGbvcepA@mail.gmail.com
Backpatch-through: 11

5 years agoRemove unused macro
Peter Eisentraut [Wed, 27 Feb 2019 23:28:50 +0000 (00:28 +0100)]
Remove unused macro

It has never been used as long as hstore has been in the tree.

5 years agoInitialize variable to silence compiler warning.
Andres Freund [Wed, 27 Feb 2019 17:14:34 +0000 (09:14 -0800)]
Initialize variable to silence compiler warning.

After ff11e7f4b9ae Tom's compiler warns about accessing a potentially
uninitialized rInfo. That's not actually possible, but it's
understandable the compiler would get this wrong. NULL initialize too.

Reported-By: Tom Lane
Discussion: https://postgr.es/m/11199.1551285318@sss.pgh.pa.us

5 years agoSet cluster_name for PostgresNode.pm instances
Peter Eisentraut [Fri, 8 Feb 2019 07:38:54 +0000 (08:38 +0100)]
Set cluster_name for PostgresNode.pm instances

This can help identifying test instances more easily at run time, and
it also provides some minimal test coverage for the cluster_name
feature.

Reviewed-by: Euler Taveira <euler@timbira.com.br>
Discussion: https://www.postgresql.org/message-id/flat/1257eaee-4874-e791-e83a-46720c72cac7@2ndquadrant.com

5 years agoSet fallback_application_name for a walreceiver to cluster_name
Peter Eisentraut [Fri, 8 Feb 2019 07:17:21 +0000 (08:17 +0100)]
Set fallback_application_name for a walreceiver to cluster_name

By default, the fallback_application_name for a physical walreceiver
is "walreceiver".  This means that multiple standbys cannot be
distinguished easily on a primary, for example in pg_stat_activity or
synchronous_standby_names.

If cluster_name is set, use that for fallback_application_name in the
walreceiver.  (If it's not set, it remains "walreceiver".)  If someone
set cluster_name to identify their instance, we might as well use that
by default to identify the node remotely as well.  It's still possible
to specify another application_name in primary_conninfo explicitly.

Reviewed-by: Euler Taveira <euler@timbira.com.br>
Discussion: https://www.postgresql.org/message-id/flat/1257eaee-4874-e791-e83a-46720c72cac7@2ndquadrant.com

5 years agoFix memory leak when inserting tuple at relation creation for CTAS
Michael Paquier [Wed, 27 Feb 2019 05:14:06 +0000 (14:14 +0900)]
Fix memory leak when inserting tuple at relation creation for CTAS

The leak has been introduced by 763f2ed which has addressed the problem
for transient tables, and forgot CREATE TABLE AS which shares a similar
logic when receiving a new tuple to store into the newly-created
relation.

Author: Jeff Janes
Discussion: https://postgr.es/m/CAMkU=1xZXtz3mziPEPD2Fubbas4G2RWkZm5HHABtfKVcbu1=Sg@mail.gmail.com

5 years agoUse slots in trigger infrastructure, except for the actual invocation.
Andres Freund [Wed, 27 Feb 2019 04:30:28 +0000 (20:30 -0800)]
Use slots in trigger infrastructure, except for the actual invocation.

In preparation for abstracting table storage, convert trigger.c to
track tuples in slots. Which also happens to make code calling
triggers simpler.

As the calling interface for triggers themselves is not changed in
this patch, HeapTuples still are extracted from the slot at that
time. But that's handled solely inside trigger.c, not visible to
callers. It's quite likely that we'll want to revise the external
trigger interface, but that's a separate large project.

As part of this work the slots used for old/new/return tuples are
moved from EState into ResultRelInfo, as different updated tables
might need different slots. The slots are now also now created
on-demand, which is good both from an efficiency POV, but also makes
the modifying code simpler.

Author: Andres Freund, Amit Khandekar and Ashutosh Bapat
Discussion: https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de

5 years agoStore table oid and tuple's tid in tuple slots directly.
Andres Freund [Wed, 27 Feb 2019 02:21:44 +0000 (18:21 -0800)]
Store table oid and tuple's tid in tuple slots directly.

After the introduction of tuple table slots all table AMs need to
support returning the table oid of the tuple stored in a slot created
by said AM. It does not make sense to re-implement that in every AM,
therefore move handling of table OIDs into the TupleTableSlot
structure itself.  It's possible that we, at a later date, might want
to get rid of HeapTupleData.t_tableOid entirely, but doing so before
the abstractions for table AMs are integrated turns out to be too
hard, so delay that for now.

Similarly, every AM needs to support the concept of a tuple
identifier (tid / item pointer) for its tuples. It's quite possible
that we'll generalize the exact form of a tid at a future point (to
allow for things like index organized tables), but for now many parts
of the code know about tids, so there's not much point in abstracting
tids away. Therefore also move into slot (rather than providing API to
set/get the tid associated with the tuple in a slot).

Once table AM includes insert/updating/deleting tuples, the
responsibility to set the correct tid after such an action will move
into that. After that change, code doing such modifications, should
not have to deal with HeapTuples directly anymore.

Author: Andres Freund, Haribabu Kommi and Ashutosh Bapat
Discussion: https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de

5 years agoAllow to use HeapTupleData embedded in [Buffer]HeapTupleTableSlot.
Andres Freund [Wed, 27 Feb 2019 02:15:59 +0000 (18:15 -0800)]
Allow to use HeapTupleData embedded in [Buffer]HeapTupleTableSlot.

That avoids having to care about the lifetime of the
HeapTupleHeaderData passed to ExecStore[Buffer]HeapTuple(). That
doesn't make a huge difference for a plain HeapTupleTableSlot, but for
BufferHeapTupleTableSlot it can be a significant advantage, avoiding
the need to materialize slots where it's inconvenient to provide a
HeapTupleData with appropriate lifetime to point to the on-disk tuple.

It's quite possible that we'll want to add support functions for
constructing HeapTuples using that embedded HeapTupleData, but for now
callers do so themselves.

Author: Andres Freund
Discussion: https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de

5 years agoAdd ExecStorePinnedBufferHeapTuple.
Andres Freund [Wed, 27 Feb 2019 01:59:01 +0000 (17:59 -0800)]
Add ExecStorePinnedBufferHeapTuple.

This allows to avoid an unnecessary pin/unpin cycle when storing a
tuple in an already pinned buffer into a slot, when the pin isn't
further needed at the call site.

Only a single caller for now (to ensure coverage), but upcoming
patches will increase use of the new function.

Author: Andres Freund
Discussion: https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de

5 years agoChange lock acquisition order in expand_inherited_rtentry.
Robert Haas [Tue, 26 Feb 2019 17:22:57 +0000 (12:22 -0500)]
Change lock acquisition order in expand_inherited_rtentry.

Previously, this function acquired locks in the order using
find_all_inheritors(), which locks the children of each table that it
processes in ascending OID order, and which processes the inheritance
hierarchy as a whole in a breadth-first fashion.  Now, it processes
the inheritance hierarchy in a depth-first fashion, and at each level
it proceeds in the order in which tables appear in the PartitionDesc.
If table inheritance rather than table partitioning is used, the old
order is preserved.

This change moves the locking of any given partition much closer to
the code that actually expands that partition.  This seems essential
if we ever want to allow concurrent DDL to add or remove partitions,
because if the set of partitions can change, we must use the same data
to decide which partitions to lock as we do to decide which partitions
to expand; otherwise, we might expand a partition that we haven't
locked.  It should hopefully also facilitate efforts to postpone
inheritance expansion or locking for performance reasons, because
there's really no way to postpone locking some partitions if
we're blindly locking them all using find_all_inheritors().

The only downside of this change which is known to me is that it
further deviates from the principle that we should always lock the
inheritance hierarchy in find_all_inheritors() order to avoid deadlock
risk.  However, we've already crossed that bridge in commit
9eefba181f7782d27d85d7e94e6028371e7ab2d7 and there are futher patches
pending that make similar changes, so this isn't really giving up
anything that we haven't surrendered already -- and it seems entirely
worth it, given the performance benefits some of those changes seem
likely to bring.

Patch by me; thanks to David Rowley for discussion of these issues.

Discussion: http://postgr.es/m/CAKJS1f_eEYVEq5tM8sm1k-HOwG0AyCPwX54XG9x4w0zy_N4Q_Q@mail.gmail.com
Discussion: http://postgr.es/m/CA+TgmoZUwPf_uanjF==gTGBMJrn8uCq52XYvAEorNkLrUdoawg@mail.gmail.com

5 years agoFree memory in ecpg bytea regression test.
Michael Meskes [Tue, 26 Feb 2019 10:59:35 +0000 (11:59 +0100)]
Free memory in ecpg bytea regression test.

While not really a problem it's easier to run tools like valgrind against it
when fixed.

5 years agoHopefully fixing memory handling issues in ecpglib that Coverity found.
Michael Meskes [Tue, 26 Feb 2019 09:56:54 +0000 (10:56 +0100)]
Hopefully fixing memory handling issues in ecpglib that Coverity found.

5 years agoSimplify some code in pg_rewind when syncing target directory
Michael Paquier [Tue, 26 Feb 2019 07:08:24 +0000 (16:08 +0900)]
Simplify some code in pg_rewind when syncing target directory

9a4059d simplified the flush of target data folder when finishing
processing, and could have done a bit more.

Discussion: https://postgr.es/m/20190131064759.GA13429@paquier.xyz

5 years agoRemove unneeded argument from _bt_getstackbuf().
Peter Geoghegan [Tue, 26 Feb 2019 01:47:43 +0000 (17:47 -0800)]
Remove unneeded argument from _bt_getstackbuf().

_bt_getstackbuf() is called at exactly two points following commit
efada2b8e92 (one call site is concerned with page splits, while the
other is concerned with page deletion).  The parent buffer returned by
_bt_getstackbuf() is write-locked in both cases.  Remove the 'access'
argument and make _bt_getstackbuf() assume that callers require a
write-lock.