]> granicus.if.org Git - postgresql/log
postgresql
7 years agoAdd PQencryptPasswordConn function to libpq, use it in psql and createuser.
Heikki Linnakangas [Wed, 3 May 2017 08:19:07 +0000 (11:19 +0300)]
Add PQencryptPasswordConn function to libpq, use it in psql and createuser.

The new function supports creating SCRAM verifiers, in addition to md5
hashes. The algorithm is chosen based on password_encryption, by default.

This fixes the issue reported by Jeff Janes, that there was previously
no way to create a SCRAM verifier with "\password".

Michael Paquier and me

Discussion: https://www.postgresql.org/message-id/CAMkU%3D1wfBgFPbfAMYZQE78p%3DVhZX7nN86aWkp0QcCp%3D%2BKxZ%3Dbg%40mail.gmail.com

7 years agoImprove performance of timezone loading, especially pg_timezone_names view.
Tom Lane [Wed, 3 May 2017 01:50:35 +0000 (21:50 -0400)]
Improve performance of timezone loading, especially pg_timezone_names view.

tzparse() would attempt to load the "posixrules" timezone database file on
each call.  That might seem like it would only be an issue when selecting a
POSIX-style zone name rather than a zone defined in the timezone database,
but it turns out that each zone definition file contains a POSIX-style zone
string and tzload() will call tzparse() to parse that.  Thus, when scanning
the whole timezone file tree as we do in the pg_timezone_names view,
"posixrules" was read repetitively for each zone definition file.  Fix
that by caching the file on first use within any given process.  (We cache
other zone definitions for the life of the process, so there seems little
reason not to cache this one as well.)  This probably won't help much in
processes that never run pg_timezone_names, but even one additional SET
of the timezone GUC would come out ahead.

An even worse problem for pg_timezone_names is that pg_open_tzfile()
has an inefficient way of identifying the canonical case of a zone name:
it basically re-descends the directory tree to the zone file.  That's not
awful for an individual "SET timezone" operation, but it's pretty horrid
when we're inspecting every zone in the database.  And it's pointless too
because we already know the canonical spelling, having just read it from
the filesystem.  Fix by teaching pg_open_tzfile() to avoid the directory
search if it's not asked for the canonical name, and backfilling the
proper result in pg_tzenumerate_next().

In combination these changes seem to make the pg_timezone_names view
about 3x faster to read, for me.  Since a scan of pg_timezone_names
has up to now been one of the slowest queries in the regression tests,
this should help some little bit for buildfarm cycle times.

Back-patch to all supported branches, not so much because it's likely
that users will care much about the view's performance as because
tracking changes in the upstream IANA timezone code is really painful
if we don't keep all the branches in sync.

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

7 years agoRemove create_singleton_array(), hard-coding the case in its sole caller.
Tom Lane [Wed, 3 May 2017 00:41:37 +0000 (20:41 -0400)]
Remove create_singleton_array(), hard-coding the case in its sole caller.

create_singleton_array() was not really as useful as we perhaps thought
when we added it.  It had never accreted more than one call site, and is
only saving a dozen lines of code at that one, which is considerably less
bulk than the function itself.  Moreover, because of its insistence on
using the caller's fn_extra cache space, it's arguably a coding hazard.
text_to_array_internal() does not currently use fn_extra in any other way,
but if it did it would be subtly broken, since the conflicting fn_extra
uses could be needed within a single query, in the seldom-tested case that
the field separator varies during the query.  The same objection seems
likely to apply to any other potential caller.

The replacement code is a bit uglier, because it hardwires knowledge of
the storage parameters of type TEXT, but it's not like we haven't got
dozens or hundreds of other places that do the same.  Uglier seems like
a good tradeoff for smaller, faster, and safer.

Per discussion with Neha Khatri.

Discussion: https://postgr.es/m/CAFO0U+_fS5SRhzq6uPG+4fbERhoA9N2+nPrtvaC9mmeWivxbsA@mail.gmail.com

7 years agoEnsure commands in extension scripts see the results of preceding DDL.
Tom Lane [Tue, 2 May 2017 22:05:53 +0000 (18:05 -0400)]
Ensure commands in extension scripts see the results of preceding DDL.

Due to a missing CommandCounterIncrement() call, parsing of a non-utility
command in an extension script would not see the effects of the immediately
preceding DDL command, unless that command's execution ends with
CommandCounterIncrement() internally ... which some do but many don't.
Report by Philippe Beaudoin, diagnosis by Julien Rouhaud.

Rather remarkably, this bug has evaded detection since extensions were
invented, so back-patch to all supported branches.

Discussion: https://postgr.es/m/2cf7941e-4e41-7714-3de8-37b1a8f74dff@free.fr

7 years agoextstats: change output functions to emit valid JSON
Alvaro Herrera [Tue, 2 May 2017 21:49:32 +0000 (18:49 -0300)]
extstats: change output functions to emit valid JSON

Manipulating extended statistics is more convenient as JSON than the
current ad-hoc format, so let's change before it's too late.

Discussion: https://postgr.es/m/20170420193828.k3fliiock5hdnehn@alvherre.pgsql

7 years agodoc: Improve order in ALTER PUBLICATION/SUBSCRIPTION ref pages
Peter Eisentraut [Tue, 2 May 2017 19:29:30 +0000 (15:29 -0400)]
doc: Improve order in ALTER PUBLICATION/SUBSCRIPTION ref pages

Move the OWNER and RENAME clauses to the end, so the interesting
functionality is listed first.  This is more typical on nearby reference
pages, whereas the previous order was the order in which the clauses
were added.

7 years agoFix typos in comments.
Robert Haas [Tue, 2 May 2017 18:47:46 +0000 (14:47 -0400)]
Fix typos in comments.

Etsuro Fujita

Discussion: http://postgr.es/m/00e88999-684d-d79a-70e4-908c937a0126@lab.ntt.co.jp

7 years agodoc: Add missing markup
Peter Eisentraut [Tue, 2 May 2017 18:33:19 +0000 (14:33 -0400)]
doc: Add missing markup

7 years agoAvoid unnecessary catalog updates in ALTER SEQUENCE
Peter Eisentraut [Tue, 2 May 2017 14:41:48 +0000 (10:41 -0400)]
Avoid unnecessary catalog updates in ALTER SEQUENCE

ALTER SEQUENCE can do nontransactional changes to the sequence (RESTART
clause) and transactional updates to the pg_sequence catalog (most other
clauses).  When just calling RESTART, the code would still needlessly do
a catalog update without any changes.  This would entangle that
operation in the concurrency issues of a catalog update (causing either
locking or concurrency errors, depending on how that issue is to be
resolved).

Fix by keeping track during options parsing whether a catalog update is
needed, and skip it if not.

Reported-by: Jason Petersen <jason@citusdata.com>
7 years agodoc: Update ALTER SEQUENCE claims about changes being nontransactional
Peter Eisentraut [Tue, 2 May 2017 14:34:49 +0000 (10:34 -0400)]
doc: Update ALTER SEQUENCE claims about changes being nontransactional

Clarify that all changes except RESTART are transactional (since
1753b1b027035029c2a2a1649065762fafbf63f3).

Reported-by: Michael Paquier <michael.paquier@gmail.com>
7 years agoFix perl thinko in commit fed6df486dca
Andrew Dunstan [Tue, 2 May 2017 12:20:11 +0000 (08:20 -0400)]
Fix perl thinko in commit fed6df486dca

Report and fix from Vaishnavi Prabakaran

Backpatch to 9.4 like original.

7 years agoChange hot_standby default value to 'on'
Magnus Hagander [Tue, 2 May 2017 09:12:30 +0000 (11:12 +0200)]
Change hot_standby default value to 'on'

This goes together with the changes made to enable replication on the
sending side by default (wal_level, max_wal_senders etc) by making the
receiving stadby node also enable it by default.

Huong Dangminh

7 years agoDon't wake up logical replication launcher unnecessarily
Peter Eisentraut [Tue, 2 May 2017 02:50:32 +0000 (22:50 -0400)]
Don't wake up logical replication launcher unnecessarily

In CREATE SUBSCRIPTION, only wake up the launcher when the subscription
is enabled.

Author: Fujii Masao <masao.fujii@gmail.com>

7 years agoImprove function header comment for create_singleton_array().
Tom Lane [Mon, 1 May 2017 19:31:41 +0000 (15:31 -0400)]
Improve function header comment for create_singleton_array().

Mentioning the caller is neither future-proof nor an adequate substitute
for giving an API specification.  Per gripe from Neha Khatri, though
I changed the patch around some.

Discussion: https://postgr.es/m/CAFO0U+_fS5SRhzq6uPG+4fbERhoA9N2+nPrtvaC9mmeWivxbsA@mail.gmail.com

7 years agoReduce semijoins with unique inner relations to plain inner joins.
Tom Lane [Mon, 1 May 2017 18:53:42 +0000 (14:53 -0400)]
Reduce semijoins with unique inner relations to plain inner joins.

If the inner relation can be proven unique, that is it can have no more
than one matching row for any row of the outer query, then we might as
well implement the semijoin as a plain inner join, allowing substantially
more freedom to the planner.  This is a form of outer join strength
reduction, but it can't be implemented in reduce_outer_joins() because
we don't have enough info about the individual relations at that stage.
Instead do it much like remove_useless_joins(): once we've built base
relations, we can make another pass over the SpecialJoinInfo list and
get rid of any entries representing reducible semijoins.

This is essentially a followon to the inner-unique patch (commit 9c7f5229a)
and makes use of the proof machinery that that patch created.  We need only
minor refactoring of innerrel_is_unique's API to support this usage.

Per performance complaint from Teodor Sigaev.

Discussion: https://postgr.es/m/f994fc98-389f-4a46-d1bc-c42e05cb43ed@sigaev.ru

7 years agoFix mis-optimization of semijoins with more than one LHS relation.
Tom Lane [Mon, 1 May 2017 18:39:11 +0000 (14:39 -0400)]
Fix mis-optimization of semijoins with more than one LHS relation.

The inner-unique patch (commit 9c7f5229a) supposed that if we're
considering a JOIN_UNIQUE_INNER join path, we can always set inner_unique
for the join, because the inner path produced by create_unique_path should
be unique relative to the outer relation.  However, that's true only if
we're considering joining to the whole outer relation --- otherwise we may
be applying only some of the join quals, and so the inner path might be
non-unique from the perspective of this join.  Adjust the test to only
believe that we can set inner_unique if we have the whole semijoin LHS on
the outer side.

There is more that can be done in this area, but this commit is only
intended to provide the minimal fix needed to get correct plans.

Per report from Teodor Sigaev.  Thanks to David Rowley for preliminary
investigation.

Discussion: https://postgr.es/m/f994fc98-389f-4a46-d1bc-c42e05cb43ed@sigaev.ru

7 years agoUpdate time zone data files to tzdata release 2017b.
Tom Lane [Mon, 1 May 2017 15:52:59 +0000 (11:52 -0400)]
Update time zone data files to tzdata release 2017b.

DST law changes in Chile, Haiti, and Mongolia.  Historical corrections for
Ecuador, Kazakhstan, Liberia, and Spain.

The IANA crew continue their campaign to replace invented time zone
abbrevations with numeric GMT offsets.  This update changes numerous zones
in South America, the Pacific and Indian oceans, and some Asian and Middle
Eastern zones.  I kept these abbreviations in the tznames/ data files,
however, so that we will still accept them for input.  (We may want to
start trimming those files someday, but I think we should wait for the
upstream dust to settle before deciding what to do.)

In passing, add MESZ (Mitteleuropaeische Sommerzeit) to the tznames lists;
since we accept MEZ (Mitteleuropaeische Zeit) it seems rather strange not
to take the other one.  And fix some incorrect, or at least obsolete,
comments that certain abbreviations are not traceable to the IANA data.

7 years agolibpq: Fix inadvertent change in .pgpass lookup behavior.
Robert Haas [Mon, 1 May 2017 15:27:09 +0000 (11:27 -0400)]
libpq: Fix inadvertent change in .pgpass lookup behavior.

Commit 274bb2b3857cc987cfa21d14775cae9b0dababa5 caused password file
lookups to use the hostaddr in preference to the host, but that was
not intended and the documented behavior is the opposite.

Report and patch by Kyotaro Horiguchi.

Discussion: http://postgr.es/m/20170428.165432.60857995.horiguchi.kyotaro@lab.ntt.co.jp

7 years agoAllow vcregress.pl to run an arbitrary TAP test set
Andrew Dunstan [Mon, 1 May 2017 14:12:02 +0000 (10:12 -0400)]
Allow vcregress.pl to run an arbitrary TAP test set

Currently only provision for running the bin checks in a single step is
provided for. Now these tests can be run individually, as well as tests
in other locations (e.g. src.test/recover).

Also provide for suppressing unnecessary temp installs by setting the
NO_TEMP_INSTALL environment variable just as the Makefiles do.

Backpatch to 9.4.

7 years agoFix logical replication launcher wake up and reset
Peter Eisentraut [Mon, 1 May 2017 14:18:09 +0000 (10:18 -0400)]
Fix logical replication launcher wake up and reset

After the logical replication launcher was told to wake up at
commit (for example, by a CREATE SUBSCRIPTION command), the flag to wake
up was not reset, so it would be woken up at every following commit as
well.  So fix that by resetting the flag.

Also, we don't need to wake up anything if the transaction was rolled
back.  Just reset the flag in that case.

Author: Masahiko Sawada <sawada.mshk@gmail.com>
Reported-by: Fujii Masao <masao.fujii@gmail.com>
7 years agoFire per-statement triggers on partitioned tables.
Robert Haas [Mon, 1 May 2017 12:23:01 +0000 (08:23 -0400)]
Fire per-statement triggers on partitioned tables.

Even though no actual tuples are ever inserted into a partitioned
table (the actual tuples are in the partitions, not the partitioned
table itself), we still need to have a ResultRelInfo for the
partitioned table, or per-statement triggers won't get fired.

Amit Langote, per a report from Rajkumar Raghuwanshi.  Reviewed by me.

Discussion: http://postgr.es/m/CAKcux6%3DwYospCRY2J4XEFuVy0L41S%3Dfic7rmkbsU-GXhhSbmBg%40mail.gmail.com

7 years agoSync our copy of the timezone library with IANA release tzcode2017b.
Tom Lane [Sun, 30 Apr 2017 19:13:51 +0000 (15:13 -0400)]
Sync our copy of the timezone library with IANA release tzcode2017b.

zic no longer mishandles some transitions in January 2038 when it
attempts to work around Qt bug 53071.  This fixes a bug affecting
Pacific/Tongatapu that was introduced in zic 2016e.  localtime.c
now contains a workaround, useful when loading a file generated by
a buggy zic.

There are assorted cosmetic changes as well, notably relocation
of a bunch of #defines.

7 years agoFix possible null pointer dereference or invalid warning message.
Tom Lane [Sun, 30 Apr 2017 16:21:02 +0000 (12:21 -0400)]
Fix possible null pointer dereference or invalid warning message.

Thinko in commit de4389712: this warning message references the wrong
"LogicalRepWorker *" variable.  This would often result in a core dump,
but if it didn't, the message would show the wrong subscription OID.

In passing, adjust the message text to format a subscription OID
similarly to how that's done elsewhere in the function; and fix
grammatical issues in some nearby messages.

Per Coverity testing.

7 years agoMicro-optimize some slower queries in the opr_sanity regression test.
Tom Lane [Sun, 30 Apr 2017 00:14:52 +0000 (20:14 -0400)]
Micro-optimize some slower queries in the opr_sanity regression test.

Convert the binary_coercible() and physically_coercible() functions from
SQL to plpgsql.  It's not that plpgsql is inherently better at doing
queries; if you simply convert the previous single SQL query into one
RETURN expression, it's no faster.  The problem with the existing code
is that it fools the plancache into deciding that it's worth re-planning
the query every time, since constant-folding with a concrete value for $2
allows elimination of at least one sub-SELECT.  In reality that's using the
planner to do the equivalent of a few runtime boolean tests, causing the
function to run much slower than it should.  Splitting the AND/OR logic
into separate plpgsql statements allows each if-expression to acquire a
static plan.

Also, get rid of some uses of obj_description() in favor of explicitly
joining to pg_description, allowing the joins to be optimized better.
(Someday we might improve the SQL-function-inlining logic enough that
this happens automatically, but today is not that day.)

Together, these changes reduce the runtime of the opr_sanity regression
test by about a factor of two on one of my slower machines.  They don't
seem to help as much on a fast machine, but this should at least benefit
the buildfarm.

7 years agodoc: Fix typo in 9.6 release notes
Peter Eisentraut [Fri, 28 Apr 2017 19:30:54 +0000 (15:30 -0400)]
doc: Fix typo in 9.6 release notes

Author: Huong Dangminh <huo-dangminh@ys.jp.nec.com>

7 years agoFix VALIDATE CONSTRAINT to consider NO INHERIT attribute.
Robert Haas [Fri, 28 Apr 2017 18:48:38 +0000 (14:48 -0400)]
Fix VALIDATE CONSTRAINT to consider NO INHERIT attribute.

Currently, trying to validate a NO INHERIT constraint on the parent will
search for the constraint in child tables (where it is not supposed to
exist), wrongly causing a "constraint does not exist" error.

Amit Langote, per a report from Hans Buschmann.

Discussion: http://postgr.es/m/20170421184012.24362.19@wrigleys.postgresql.org

7 years agopsql: Support identity columns in sequence display
Peter Eisentraut [Fri, 28 Apr 2017 18:43:36 +0000 (14:43 -0400)]
psql: Support identity columns in sequence display

Where the footer for an owned serial sequence would say "Owned by", put
something analogous for a sequence belonging to an identity column.

Reported-by: Vitaly Burovoy <vitaly.burovoy@gmail.com>
7 years agoIn load_relcache_init_file, initialize rd_pdcxt.
Robert Haas [Fri, 28 Apr 2017 18:05:13 +0000 (14:05 -0400)]
In load_relcache_init_file, initialize rd_pdcxt.

Oversight noted by Gao Zeng Qi.

Discussion: http://postgr.es/m/CAFmBtr1N3-SbepJbnGpaYp=jw-FvWMnYY7-bTtRgvjvbyB8YJA@mail.gmail.com

7 years agoSpeed up dropping tables with many partitions.
Robert Haas [Fri, 28 Apr 2017 18:00:58 +0000 (14:00 -0400)]
Speed up dropping tables with many partitions.

We need to lock the parent, but we don't need a relcache entry
for it.

Gao Zeng Qi, reviewed by Amit Langote

Discussion: http://postgr.es/m/CAFmBtr0ukqJjRJEhPWL5wt4rNMrJUUxggVAGXPR3SyYh3E+HDQ@mail.gmail.com

7 years agoFix crash when partitioned column specified twice.
Robert Haas [Fri, 28 Apr 2017 17:52:17 +0000 (13:52 -0400)]
Fix crash when partitioned column specified twice.

Amit Langote, reviewed by Beena Emerson

Discussion: http://postgr.es/m/6ed23d3d-c09d-4cbc-3628-0a8a32f750f4@lab.ntt.co.jp

7 years agoWait between tablesync worker restarts
Peter Eisentraut [Thu, 27 Apr 2017 18:57:26 +0000 (14:57 -0400)]
Wait between tablesync worker restarts

Before restarting a tablesync worker for the same relation, wait
wal_retrieve_retry_interval (currently 5s by default).  This avoids
restarting failing workers in a tight loop.

We keep the last start times in a hash table last_start_times that is
separate from the table_states list, because that list is cleared out on
syscache invalidation, which happens whenever a table finishes syncing.
The hash table is kept until all tables have finished syncing.

A future project might be to unify these two and keep everything in one
data structure, but for now this is a less invasive change to accomplish
the original purpose.

For the test suite, set wal_retrieve_retry_interval to its minimum
value, to not increase the test suite run time.

Reviewed-by: Petr Jelinek <petr.jelinek@2ndquadrant.com>
Reported-by: Masahiko Sawada <sawada.mshk@gmail.com>
7 years agoMisc SCRAM code cleanups.
Heikki Linnakangas [Fri, 28 Apr 2017 12:04:02 +0000 (15:04 +0300)]
Misc SCRAM code cleanups.

* Move computation of SaltedPassword to a separate function from
  scram_ClientOrServerKey(). This saves a lot of cycles in libpq, by
  computing SaltedPassword only once per authentication. (Computing
  SaltedPassword is expensive by design.)

* Split scram_ClientOrServerKey() into two functions. Improves
  readability, by making the calling code less verbose.

* Rename "server proof" to "server signature", to better match the
  nomenclature used in RFC 5802.

* Rename SCRAM_SALT_LEN to SCRAM_DEFAULT_SALT_LEN, to make it more clear
  that the salt can be of any length, and the constant only specifies how
  long a salt we use when we generate a new verifier. Also rename
  SCRAM_ITERATIONS_DEFAULT to SCRAM_DEFAULT_ITERATIONS, for consistency.

These things caught my eye while working on other upcoming changes.

7 years agoRemove unnecessairly duplicated gram.y productions
Stephen Frost [Fri, 28 Apr 2017 00:14:39 +0000 (20:14 -0400)]
Remove unnecessairly duplicated gram.y productions

Declarative partitioning duplicated the TypedTableElement productions,
evidently to remove the need to specify WITH OPTIONS when creating
partitions.  Instead, simply make WITH OPTIONS optional in the
TypedTableElement production and remove all of the duplicate
PartitionElement-related productions.  This change simplifies the
syntax and makes WITH OPTIONS optional when adding defaults, constraints
or storage parameters to columns when creating either typed tables or
partitions.

Also update pg_dump to no longer include WITH OPTIONS, since it's not
necessary, and update the documentation to reflect that WITH OPTIONS is
now optional.

7 years agoDon't build full initial logical decoding snapshot if NOEXPORT_SNAPSHOT.
Andres Freund [Thu, 27 Apr 2017 22:49:22 +0000 (15:49 -0700)]
Don't build full initial logical decoding snapshot if NOEXPORT_SNAPSHOT.

Earlier commits (56e19d938dd14 and 2bef06d5164) make it cheaper to
create a logical slot if not exporting the initial snapshot.  If
NOEXPORT_SNAPSHOT is specified, we can skip the overhead, not just
when creating a slot via sql (which can't export snapshots).  As
NOEXPORT_SNAPSHOT has only recently been introduced, this shouldn't be
backpatched.

7 years agoDon't use on-disk snapshots for exported logical decoding snapshot.
Andres Freund [Thu, 27 Apr 2017 22:28:24 +0000 (15:28 -0700)]
Don't use on-disk snapshots for exported logical decoding snapshot.

Logical decoding stores historical snapshots on disk, so that logical
decoding can restart without having to reconstruct a snapshot from
scratch (for which the resources are not guaranteed to be present
anymore).  These serialized snapshots were also used when creating a
new slot via the walsender interface, which can export a "full"
snapshot (i.e. one that can read all tables, not just catalog ones).

The problem is that the serialized snapshots are only useful for
catalogs and not for normal user tables.  Thus the use of such a
serialized snapshot could result in an inconsistent snapshot being
exported, which could lead to queries returning wrong data.  This
would only happen if logical slots are created while another logical
slot already exists.

Author: Petr Jelinek
Reviewed-By: Andres Freund
Discussion: https://postgr.es/m/f37e975c-908f-858e-707f-058d3b1eb214@2ndquadrant.com
Backport: 9.4, where logical decoding was introduced.

7 years agoAvoid slow shutdown of pg_basebackup.
Tom Lane [Thu, 27 Apr 2017 22:27:02 +0000 (18:27 -0400)]
Avoid slow shutdown of pg_basebackup.

pg_basebackup's child process did not pay any attention to the pipe
from its parent while waiting for input from the source server.
If no server data was arriving, it would only wake up and check the
pipe every standby_message_timeout or so.  This creates a problem
since the parent process might determine and send the desired stop
position only after the server has reached end-of-WAL and stopped
sending data.  In the src/test/recovery regression tests, the timing
is repeatably such that it takes nearly 10 seconds for the child
process to realize that it should shut down.  It's not clear how
often that would happen in real-world cases, but it sure seems like
a bug --- and if the user turns off standby_message_timeout or sets
it very large, the delay could be a lot worse.

To fix, expand the StreamCtl API to allow the pipe input FD to be
passed down to the low-level wait routine, and watch both sockets
when sleeping.

(Note: AFAICS this issue doesn't affect the Windows port, since
it doesn't rely on a pipe to transfer the stop position to the
child thread.)

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

7 years agoFix bug so logical rep launcher saves correctly time of last startup of worker.
Fujii Masao [Thu, 27 Apr 2017 21:35:00 +0000 (06:35 +0900)]
Fix bug so logical rep launcher saves correctly time of last startup of worker.

Previously the logical replication launcher stored the last timestamp
when it started the worker, in the local variable "last_start_time",
in order to check whether wal_retrive_retry_interval elapsed since
the last startup of worker. If it has elapsed, the launcher sees
pg_subscription and starts new worker if necessary. This is for
limitting the startup of worker to once a wal_retrieve_retry_interval.

The bug was that the variable "last_start_time" was defined and
always initialized with 0 at the beginning of the launcher's main loop.
So even if it's set to the last timestamp in later phase of the loop,
it's always reset to 0. Therefore the launcher could not check
correctly whether wal_retrieve_retry_interval elapsed since
the last startup.

This patch moves the variable "last_start_time" outside the main loop
so that it will not be reset.

Reviewed-by: Petr Jelinek
Discussion: http://postgr.es/m/CAHGQGwGJrPO++XM4mFENAwpy1eGXKsGdguYv43GUgLgU-x8nTQ@mail.gmail.com

7 years agoCope with glibc too old to have epoll_create1().
Tom Lane [Thu, 27 Apr 2017 21:13:29 +0000 (17:13 -0400)]
Cope with glibc too old to have epoll_create1().

Commit fa31b6f4e supposed that we didn't have to worry about that
anymore, but it seems that RHEL5 is like that, and that's still
a supported platform.  Put back the prior coding under an #ifdef,
adding an explicit fcntl() to retain the desired CLOEXEC property.

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

7 years agoPreserve required !catalog tuples while computing initial decoding snapshot.
Andres Freund [Mon, 24 Apr 2017 03:41:29 +0000 (20:41 -0700)]
Preserve required !catalog tuples while computing initial decoding snapshot.

The logical decoding machinery already preserved all the required
catalog tuples, which is sufficient in the course of normal logical
decoding, but did not guarantee that non-catalog tuples were preserved
during computation of the initial snapshot when creating a slot over
the replication protocol.

This could cause a corrupted initial snapshot being exported.  The
time window for issues is usually not terribly large, but on a busy
server it's perfectly possible to it hit it.  Ongoing decoding is not
affected by this bug.

To avoid increased overhead for the SQL API, only retain additional
tuples when a logical slot is being created over the replication
protocol.  To do so this commit changes the signature of
CreateInitDecodingContext(), but it seems unlikely that it's being
used in an extension, so that's probably ok.

In a drive-by fix, fix handling of
ReplicationSlotsComputeRequiredXmin's already_locked argument, which
should only apply to ProcArrayLock, not ReplicationSlotControlLock.

Reported-By: Erik Rijkers
Analyzed-By: Petr Jelinek
Author: Petr Jelinek, heavily editorialized by Andres Freund
Reviewed-By: Andres Freund
Discussion: https://postgr.es/m/9a897b86-46e1-9915-ee4c-da02e4ff6a95@2ndquadrant.com
Backport: 9.4, where logical decoding was introduced.

7 years agoMake latch.c more paranoid about child-process cases.
Tom Lane [Thu, 27 Apr 2017 19:07:36 +0000 (15:07 -0400)]
Make latch.c more paranoid about child-process cases.

Although the postmaster doesn't currently create a self-pipe or any
latches, there's discussion of it doing so in future.  It's also
conceivable that a shared_preload_libraries extension would try to
create such a thing in the postmaster process today.  In that case
the self-pipe FDs would be inherited by forked child processes.
latch.c was entirely unprepared for such a case and could suffer an
assertion failure, or worse try to use the inherited pipe if somebody
called WaitLatch without having called InitializeLatchSupport in that
process.  Make it keep track of whether InitializeLatchSupport has been
called in the *current* process, and do the right thing if state has
been inherited from a parent.

Apply FD_CLOEXEC to file descriptors created in latch.c (the self-pipe,
as well as epoll event sets).  This ensures that child processes spawned
in backends, the archiver, etc cannot accidentally or intentionally mess
with these FDs.  It also ensures that we end up with the right state
for the self-pipe in EXEC_BACKEND processes, which otherwise wouldn't
know to close the postmaster's self-pipe FDs.

Back-patch to 9.6, mainly to keep latch.c looking similar in all branches
it exists in.

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

7 years agodoc: PG10 release note typo fix
Bruce Momjian [Thu, 27 Apr 2017 14:21:44 +0000 (10:21 -0400)]
doc:  PG10 release note typo fix

Reported-by: daniel.westermann
7 years agodoc PG10rel: adjust hash index commits and add parallel subquery
Bruce Momjian [Thu, 27 Apr 2017 14:17:08 +0000 (10:17 -0400)]
doc PG10rel: adjust hash index commits and add parallel subquery

Reported-by: Amit Kapila
7 years agoRework handling of subtransactions in 2PC recovery
Simon Riggs [Thu, 27 Apr 2017 12:41:22 +0000 (14:41 +0200)]
Rework handling of subtransactions in 2PC recovery

The bug fixed by 0874d4f3e183757ba15a4b3f3bf563e0393dd9c2
caused us to question and rework the handling of
subtransactions in 2PC during and at end of recovery.
Patch adds checks and tests to ensure no further bugs.

This effectively removes the temporary measure put in place
by 546c13e11b29a5408b9d6a6e3cca301380b47f7f.

Author: Simon Riggs
Reviewed-by: Tom Lane, Michael Paquier
Discussion: http://postgr.es/m/CANP8+j+vvXmruL_i2buvdhMeVv5TQu0Hm2+C5N+kdVwHJuor8w@mail.gmail.com

7 years agoAdditional tests for subtransactions in recovery
Simon Riggs [Thu, 27 Apr 2017 12:26:57 +0000 (14:26 +0200)]
Additional tests for subtransactions in recovery

Tests for normal and prepared transactions

Author: Nikhil Sontakke, placed in new test file by me

7 years agoFix typo in comment
Peter Eisentraut [Thu, 27 Apr 2017 01:12:35 +0000 (21:12 -0400)]
Fix typo in comment

Author: Masahiko Sawada <sawada.mshk@gmail.com>

7 years agoAllow multiple bgworkers to be launched per postmaster iteration.
Tom Lane [Wed, 26 Apr 2017 20:17:29 +0000 (16:17 -0400)]
Allow multiple bgworkers to be launched per postmaster iteration.

Previously, maybe_start_bgworker() would launch at most one bgworker
process per call, on the grounds that the postmaster might otherwise
neglect its other duties for too long.  However, that seems overly
conservative, especially since bad effects only become obvious when
many hundreds of bgworkers need to be launched at once.  On the other
side of the coin is that the existing logic could result in substantial
delay of bgworker launches, because ServerLoop isn't guaranteed to
iterate immediately after a signal arrives.  (My attempt to fix that
by using pselect(2) encountered too many portability question marks,
and in any case could not help on platforms without pselect().)
One could also question the wisdom of using an O(N^2) processing
method if the system is intended to support so many bgworkers.

As a compromise, allow that function to launch up to 100 bgworkers
per call (and in consequence, rename it to maybe_start_bgworkers).
This will allow any normal parallel-query request for workers
to be satisfied immediately during sigusr1_handler, avoiding the
question of whether ServerLoop will be able to launch more promptly.

There is talk of rewriting the postmaster to use a WaitEventSet to
avoid the signal-response-delay problem, but I'd argue that this change
should be kept even after that happens (if it ever does).

Backpatch to 9.6 where parallel query was added.  The issue exists
before that, but previous uses of bgworkers typically aren't as
sensitive to how quickly they get launched.

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

7 years agodoc PG10: add commit for transition table item
Bruce Momjian [Wed, 26 Apr 2017 19:50:51 +0000 (15:50 -0400)]
doc PG10:  add commit for transition table item

7 years agopg_get_partkeydef: return NULL for non-partitions
Stephen Frost [Wed, 26 Apr 2017 18:59:22 +0000 (14:59 -0400)]
pg_get_partkeydef: return NULL for non-partitions

Our general rule for pg_get_X(oid) functions is to simply return NULL
when passed an invalid or inappropriate OID.  Teach pg_get_partkeydef to
do this also, making it easier for users to use this function when
querying against tables with both partitions and non-partitions (such as
pg_class).

As a concrete example, this makes pg_dump's life a little easier.

Author: Amit Langote

7 years agoSilence compiler warning induced by commit de4389712.
Tom Lane [Wed, 26 Apr 2017 18:01:26 +0000 (14:01 -0400)]
Silence compiler warning induced by commit de4389712.

Smarter compilers can see that "slot" can't be used uninitialized,
but some popular ones cannot.  Noted by Jeff Janes.

7 years agodoc: ALTER SUBSCRIPTION documentation fixes
Peter Eisentraut [Wed, 26 Apr 2017 16:05:11 +0000 (12:05 -0400)]
doc: ALTER SUBSCRIPTION documentation fixes

WITH is optional for REFRESH PUBLICATION.  Also, remove a spurious
bracket and fix a punctuation.

Author: Euler Taveira <euler@timbira.com.br>

7 years agoFix query that gets remote relation info
Peter Eisentraut [Wed, 26 Apr 2017 16:05:04 +0000 (12:05 -0400)]
Fix query that gets remote relation info

Publisher relation can be incorrectly chosen, if there are more than
one relation in different schemas with the same name.

Author: Euler Taveira <euler@timbira.com.br>

7 years agoSpelling fixes in code comments
Peter Eisentraut [Wed, 26 Apr 2017 16:04:44 +0000 (12:04 -0400)]
Spelling fixes in code comments

Author: Euler Taveira <euler@timbira.com.br>

7 years agoFix typo in comment.
Fujii Masao [Wed, 26 Apr 2017 15:03:07 +0000 (00:03 +0900)]
Fix typo in comment.

Author: Masahiko Sawada

7 years agoFix various concurrency issues in logical replication worker launching
Peter Eisentraut [Wed, 26 Apr 2017 14:43:04 +0000 (10:43 -0400)]
Fix various concurrency issues in logical replication worker launching

The code was originally written with assumption that launcher is the
only process starting the worker.  However that hasn't been true since
commit 7c4f52409 which failed to modify the worker management code
adequately.

This patch adds an in_use field to the LogicalRepWorker struct to
indicate whether the worker slot is being used and uses proper locking
everywhere this flag is set or read.

However if the parent process dies while the new worker is starting and
the new worker fails to attach to shared memory, this flag would never
get cleared.  We solve this rare corner case by adding a sort of garbage
collector for in_use slots.  This uses another field in the
LogicalRepWorker struct named launch_time that contains the time when
the worker was started.  If any request to start a new worker does not
find free slot, we'll check for workers that were supposed to start but
took too long to actually do so, and reuse their slot.

In passing also fix possible race conditions when stopping a worker that
hasn't finished starting yet.

Author: Petr Jelinek <petr.jelinek@2ndquadrant.com>
Reported-by: Fujii Masao <masao.fujii@gmail.com>
7 years agodoc PG10: add Rafia Sabih to parallel index scan item
Bruce Momjian [Wed, 26 Apr 2017 10:33:25 +0000 (06:33 -0400)]
doc PG10:  add Rafia Sabih to parallel index scan item

Reported-by: Amit Kapila
7 years agoAllow ALTER TABLE ONLY on partitioned tables
Stephen Frost [Tue, 25 Apr 2017 20:57:43 +0000 (16:57 -0400)]
Allow ALTER TABLE ONLY on partitioned tables

There is no need to forbid ALTER TABLE ONLY on partitioned tables,
when no partitions exist yet.  This can be handy for users who are
building up their partitioned table independently and will create actual
partitions later.

In addition, this is how pg_dump likes to operate in certain instances.

Author: Amit Langote, with some error message word-smithing by me

7 years agodoc PG10: update EXPLAIN SUMMARY item
Bruce Momjian [Tue, 25 Apr 2017 19:30:45 +0000 (15:30 -0400)]
doc PG10:  update EXPLAIN SUMMARY item

Reported-by: Tels
7 years agoWake up launcher when enabling a subscription
Peter Eisentraut [Tue, 25 Apr 2017 18:40:33 +0000 (14:40 -0400)]
Wake up launcher when enabling a subscription

Otherwise one would have to wait up to DEFAULT_NAPTIME_PER_CYCLE until
the subscription worker is considered for starting.

There is a small race condition:  If one enables a subscription right
after disabling it, the launcher might not have registered the stopping
when receiving the wakeup signal for the re-enabling.  The start will
then not happen right away but after the full cycle time.

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

7 years agodoc: update PG 10 item about referencing many relations
Bruce Momjian [Tue, 25 Apr 2017 17:47:51 +0000 (13:47 -0400)]
doc:  update PG 10 item about referencing many relations

Reported-by: Tom Lane
7 years agodoc: add PG 10 doc item about VACUUM truncation, 7e26e02ee
Bruce Momjian [Tue, 25 Apr 2017 17:45:47 +0000 (13:45 -0400)]
doc:  add PG 10 doc item about VACUUM truncation, 7e26e02ee

Reported-by: Andres Freund
7 years agodoc PG10: add commit 090010f2e and adjust EXPLAIN SUMMARY item
Bruce Momjian [Tue, 25 Apr 2017 17:29:26 +0000 (13:29 -0400)]
doc PG10:  add commit 090010f2e and adjust EXPLAIN SUMMARY item

Reported-by: Tels, Andres Freund
7 years agodoc: properly indent SGML tags in PG 10 release notes
Bruce Momjian [Tue, 25 Apr 2017 16:54:37 +0000 (12:54 -0400)]
doc:  properly indent SGML tags in PG 10 release notes

7 years agoSet the priorities of all quorum synchronous standbys to 1.
Fujii Masao [Tue, 25 Apr 2017 16:07:13 +0000 (01:07 +0900)]
Set the priorities of all quorum synchronous standbys to 1.

In quorum-based synchronous replication, all the standbys listed in
synchronous_standby_names equally have chances to be chosen
as synchronous standbys. So they should have the same priority.
However, previously, quorum standbys whose names appear earlier
in the list were given higher priority values though the difference of
those priority values didn't affect the selection of synchronous standbys.
Users could see those "meaningless" priority values in pg_stat_replication
and this was confusing.

This commit gives all the quorum synchronous standbys the same
highest priority, i.e., 1, in order to remove such confusion.

Author: Fujii Masao
Reviewed-by: Masahiko Sawada, Kyotaro Horiguchi
Discussion: http://postgr.es/m/CAHGQGwEKOw=SmPLxJzkBsH6wwDBgOnVz46QjHbtsiZ-d-2RGUg@mail.gmail.com

7 years agodoc: PG 10 release notes updates
Bruce Momjian [Tue, 25 Apr 2017 15:17:14 +0000 (11:17 -0400)]
doc:  PG 10 release notes updates

Reported-by: Michael Paquier, Felix Gerzaguet
7 years agodoc: PG 10 release note updates
Bruce Momjian [Tue, 25 Apr 2017 15:04:35 +0000 (11:04 -0400)]
doc:  PG 10 release note updates

Reported-by: David Rowley, Amit Langote, Ashutosh Bapat
7 years agoAdjust outdated comment.
Robert Haas [Tue, 25 Apr 2017 14:57:13 +0000 (10:57 -0400)]
Adjust outdated comment.

Commit 5dfc198146b49ce7ecc8a1fc9d5e171fb75f6ba5 removed the only
existing caller of hash_freeze, but left behind a comment indicating
that hash_freeze was still used.  Adjust.

Kyotaro Horiguchi

Discussion: http://postgr.es/m/20170424.165541.230634914.horiguchi.kyotaro@lab.ntt.co.jp

7 years agoUpdate copyright in recently added files.
Fujii Masao [Tue, 25 Apr 2017 14:38:41 +0000 (23:38 +0900)]
Update copyright in recently added files.

This commit also fixes copyright line missed by the automated script.

Author: Masahiko Sawada

7 years agodoc: move hash info to new section and split out growth item
Bruce Momjian [Tue, 25 Apr 2017 13:44:50 +0000 (09:44 -0400)]
doc: move hash info to new section and split out growth item

Reported-by: Amit Kapila
7 years agodoc: move hash performance item into index section
Bruce Momjian [Tue, 25 Apr 2017 03:29:14 +0000 (23:29 -0400)]
doc:  move hash performance item into index section

The requirement to rebuild pg_upgrade-ed hash indexes was kept in the
incompatibilities section.

Reported-by: Amit Kapila
7 years agodoc: add Rafia Sabih to PG 10 release note item
Bruce Momjian [Tue, 25 Apr 2017 03:08:25 +0000 (23:08 -0400)]
doc:  add Rafia Sabih to PG 10 release note item

Reported-by: Amit Kapila
7 years agodoc: fix PG 10 release note doc markup
Bruce Momjian [Tue, 25 Apr 2017 02:53:16 +0000 (22:53 -0400)]
doc:  fix PG 10 release note doc markup

7 years agodoc: merge PG 10 release SysV item
Bruce Momjian [Tue, 25 Apr 2017 02:51:20 +0000 (22:51 -0400)]
doc:  merge PG 10 release SysV item

Reported-by: Takayuki Tsunakawa
7 years agopostgres_fdw: Fix join push down with extensions
Peter Eisentraut [Tue, 25 Apr 2017 02:50:07 +0000 (22:50 -0400)]
postgres_fdw: Fix join push down with extensions

Objects in an extension are shippable to a foreign server if the
extension is part of the foreign server definition's shippable
extensions list.  But this was not properly considered in some cases
when checking whether a join condition can be pushed to a foreign server
and the join condition uses an object from a shippable extension.  So
the join would never be pushed down in those cases.

So, the list of extensions needs to be made available in fpinfo of the
relation being considered to be pushed down before any expressions are
assessed for being shippable.  Fix foreign_join_ok() to do that for a
join relation.

The code to save FDW options in fpinfo is scattered at multiple places.
Bring all of that together into functions apply_server_options(),
apply_table_options(), and merge_fdw_options().

David Rowley and Ashutosh Bapat, per report from David Rowley

7 years agodoc: PG 10 fixes
Bruce Momjian [Tue, 25 Apr 2017 02:48:25 +0000 (22:48 -0400)]
doc:  PG 10 fixes

Reported-by: Takayuki Tsunakawa
7 years agodoc: several minor PG 10 doc adjustments
Bruce Momjian [Tue, 25 Apr 2017 02:45:06 +0000 (22:45 -0400)]
doc:  several minor PG 10 doc adjustments

7 years agodoc: fix attribution of sequence item, order incompatibilities
Bruce Momjian [Tue, 25 Apr 2017 01:53:37 +0000 (21:53 -0400)]
doc:  fix attribution of sequence item, order incompatibilities

Reported-by: Andreas Karlsson
7 years agodoc: first draft of Postgres 10 release notes
Bruce Momjian [Tue, 25 Apr 2017 01:26:33 +0000 (21:26 -0400)]
doc:  first draft of Postgres 10 release notes

7 years agodoc: update release doc markup instructions
Bruce Momjian [Mon, 24 Apr 2017 23:04:28 +0000 (19:04 -0400)]
doc:  update release doc markup instructions

7 years agoRevert "Use pselect(2) not select(2), if available, to wait in postmaster's loop."
Tom Lane [Mon, 24 Apr 2017 22:29:03 +0000 (18:29 -0400)]
Revert "Use pselect(2) not select(2), if available, to wait in postmaster's loop."

This reverts commit 81069a9efc5a374dd39874a161f456f0fb3afba4.

Buildfarm results suggest that some platforms have versions of pselect(2)
that are not merely non-atomic, but flat out non-functional.  Revert the
use-pselect patch to confirm this diagnosis (and exclude the no-SA_RESTART
patch as the source of trouble).  If it's so, we should probably look into
blacklisting specific platforms that have broken pselect.

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

7 years agoUse pselect(2) not select(2), if available, to wait in postmaster's loop.
Tom Lane [Mon, 24 Apr 2017 18:03:14 +0000 (14:03 -0400)]
Use pselect(2) not select(2), if available, to wait in postmaster's loop.

Traditionally we've unblocked signals, called select(2), and then blocked
signals again.  The code expects that the select() will be cancelled with
EINTR if an interrupt occurs; but there's a race condition, which is that
an already-pending signal will be delivered as soon as we unblock, and then
when we reach select() there will be nothing preventing it from waiting.
This can result in a long delay before we perform any action that
ServerLoop was supposed to have taken in response to the signal.  As with
the somewhat-similar symptoms fixed by commit 893902085, the main practical
problem is slow launching of parallel workers.  The window for trouble is
usually pretty short, corresponding to one iteration of ServerLoop; but
it's not negligible.

To fix, use pselect(2) in place of select(2) where available, as that's
designed to solve exactly this problem.  Where not available, we continue
to use the old way, and are no worse off than before.

pselect(2) has been required by POSIX since about 2001, so most modern
platforms should have it.  A bigger portability issue is that some
implementations are said to be non-atomic, ie pselect() isn't really
any different from unblock/select/reblock.  Still, we're no worse off
than before on such a platform.

There is talk of rewriting the postmaster to use a WaitEventSet and
not do signal response work in signal handlers, at which point this
could be reverted, since we'd be using a self-pipe to solve the race
condition.  But that's not happening before v11 at the earliest.

Back-patch to 9.6.  The problem exists much further back, but the
worst symptom arises only in connection with parallel query, so it
does not seem worth taking any portability risks in older branches.

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

7 years agoRun the postmaster's signal handlers without SA_RESTART.
Tom Lane [Mon, 24 Apr 2017 17:00:23 +0000 (13:00 -0400)]
Run the postmaster's signal handlers without SA_RESTART.

The postmaster keeps signals blocked everywhere except while waiting
for something to happen in ServerLoop().  The code expects that the
select(2) will be cancelled with EINTR if an interrupt occurs; without
that, followup actions that should be performed by ServerLoop() itself
will be delayed.  However, some platforms interpret the SA_RESTART
signal flag as meaning that they should restart rather than cancel
the select(2).  Worse yet, some of them restart it with the original
timeout delay, meaning that a steady stream of signal interrupts can
prevent ServerLoop() from iterating at all if there are no incoming
connection requests.

Observable symptoms of this, on an affected platform such as HPUX 10,
include extremely slow parallel query startup (possibly as much as
30 seconds) and failure to update timestamps on the postmaster's sockets
and lockfiles when no new connections arrive for a long time.

We can fix this by running the postmaster's signal handlers without
SA_RESTART.  That would be quite a scary change if the range of code
where signals are accepted weren't so tiny, but as it is, it seems
safe enough.  (Note that postmaster children do, and must, reset all
the handlers before unblocking signals; so this change should not
affect any child process.)

There is talk of rewriting the postmaster to use a WaitEventSet and
not do signal response work in signal handlers, at which point it might
be appropriate to revert this patch.  But that's not happening before
v11 at the earliest.

Back-patch to 9.6.  The problem exists much further back, but the
worst symptom arises only in connection with parallel query, so it
does not seem worth taking any portability risks in older branches.

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

7 years agoGet rid of extern declarations of non-existent functions.
Fujii Masao [Mon, 24 Apr 2017 16:31:42 +0000 (01:31 +0900)]
Get rid of extern declarations of non-existent functions.

Those extern declartions were mistakenly added by commit 7c4f52409.

Author: Petr Jelinek

7 years agoFix postmaster's handling of fork failure for a bgworker process.
Tom Lane [Mon, 24 Apr 2017 16:16:58 +0000 (12:16 -0400)]
Fix postmaster's handling of fork failure for a bgworker process.

This corner case didn't behave nicely at all: the postmaster would
(partially) update its state as though the process had started
successfully, and be quite confused thereafter.  Fix it to act
like the worker had crashed, instead.

In passing, refactor so that do_start_bgworker contains all the
state-change logic for bgworker launch, rather than just some of it.

Back-patch as far as 9.4.  9.3 contains similar logic, but it's just
enough different that I don't feel comfortable applying the patch
without more study; and the use of bgworkers in 9.3 was so small
that it doesn't seem worth the extra work.

transam/parallel.c is still entirely unprepared for the possibility
of bgworker startup failure, but that seems like material for a
separate patch.

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

7 years agoCode review for commands/statscmds.c.
Tom Lane [Mon, 24 Apr 2017 15:15:15 +0000 (11:15 -0400)]
Code review for commands/statscmds.c.

Fix machine-dependent sorting of column numbers.  (Odd behavior
would only materialize for column numbers above 255, but that's
certainly legal.)

Fix poor choice of SQLSTATE for some errors, and improve error message
wording.  (Notably, "is not a scalar type" is a totally misleading way
to explain "does not have a default btree opclass".)

Avoid taking AccessExclusiveLock on the associated relation during DROP
STATISTICS.  That's neither necessary nor desirable, and it could easily
have put us into situations where DROP fails (compare commit 68ea2b7f9).

Adjust/improve comments.

David Rowley and Tom Lane

Discussion: https://postgr.es/m/CAKJS1f-GmCfPvBbAEaM5xoVOaYdVgVN1gicALSoYQ77z-+vLbw@mail.gmail.com

7 years agoDon't include sys/poll.h anymore.
Andres Freund [Sun, 23 Apr 2017 23:04:46 +0000 (16:04 -0700)]
Don't include sys/poll.h anymore.

poll.h is mandated by Single Unix Spec v2, the usual baseline for
postgres on unix.  None of the unixoid buildfarms animals has
sys/poll.h but not poll.h.  Therefore there's not much point to test
for sys/poll.h's existence and include it optionally.

Author: Andres Freund, per suggestion from Tom Lane
Discussion: https://postgr.es/m/20505.1492723662@sss.pgh.pa.us

7 years agoZero padding in replication origin's checkpointed on disk-state.
Andres Freund [Sun, 23 Apr 2017 22:48:31 +0000 (15:48 -0700)]
Zero padding in replication origin's checkpointed on disk-state.

This seems to be largely cosmetic, avoiding valgrind bleats and the
like. The uninitialized padding influences the CRC of the on-disk
entry, but because it's also used when verifying the CRC, that doesn't
cause spurious failures.  Backpatch nonetheless.

It's a bit unfortunate that contrib/test_decoding/sql/replorigin.sql
doesn't exercise the checkpoint path, but checkpoints are fairly
expensive on weaker machines, and we'd have to stop/start for that to
be meaningful.

Author: Andres Freund
Discussion: https://postgr.es/m/20170422183123.w2jgiuxtts7qrqaq@alap3.anarazel.de
Backpatch: 9.5, where replication origins were introduced

7 years agoInitialize all memory for logical replication relation cache.
Andres Freund [Sun, 23 Apr 2017 22:36:47 +0000 (15:36 -0700)]
Initialize all memory for logical replication relation cache.

As reported by buildfarm animal skink / valgrind, some of the
variables weren't always initialized.  To avoid further mishaps use
memset to ensure the entire entry is initialized.

Author: Petr Jelinek
Reported-By: Andres Freund
Discussion: https://postgr.es/m/20170422183123.w2jgiuxtts7qrqaq@alap3.anarazel.de
Backpatch: none, code new in master

7 years agoRemove select(2) backed latch implementation.
Andres Freund [Sun, 23 Apr 2017 22:26:25 +0000 (15:26 -0700)]
Remove select(2) backed latch implementation.

poll(2) is required by Single Unix Spec v2, the usual baseline for
postgres (leaving windows aside).  There's not been any buildfarm
animals without poll(2) for a long while, leaving the select(2)
implementation to be largely untested.

On windows, including mingw, poll() is not available, but we have a
special case implementation for windows anyway.

Author: Andres Freund
Discussion: https://postgr.es/m/20170420003611.7r2sdvehesdyiz2i@alap3.anarazel.de

7 years agoWorkaround for RecoverPreparedTransactions()
Simon Riggs [Sun, 23 Apr 2017 21:12:01 +0000 (22:12 +0100)]
Workaround for RecoverPreparedTransactions()

Force overwriteOK = true while we investigate deeper fix

Proposed by Tom Lane as temporary measure, accepted by me

7 years agoFix LagTrackerRead() for timeline increments
Simon Riggs [Sun, 23 Apr 2017 20:35:41 +0000 (21:35 +0100)]
Fix LagTrackerRead() for timeline increments

Bug was masked by error in running 004_timeline_switch.pl that was
fixed recently in 7d68f2281a.

Detective work by Alvaro Herrera and Tom Lane

Author: Thomas Munro

7 years agoFix order of arguments to SubTransSetParent().
Tom Lane [Sun, 23 Apr 2017 17:10:57 +0000 (13:10 -0400)]
Fix order of arguments to SubTransSetParent().

ProcessTwoPhaseBuffer (formerly StandbyRecoverPreparedTransactions)
mixed up the parent and child XIDs when calling SubTransSetParent to
record the transactions' relationship in pg_subtrans.

Remarkably, analysis by Simon Riggs suggests that this doesn't lead to
visible problems (at least, not in non-Assert builds).  That might
explain why we'd not noticed it before.  Nonetheless, it's surely wrong.

This code was born broken, so back-patch to all supported branches.

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

7 years agoFix TAP infrastructure to support Mingw better
Andrew Dunstan [Sun, 23 Apr 2017 13:21:38 +0000 (09:21 -0400)]
Fix TAP infrastructure to support Mingw better

archive_command and restore_command need to refer to Windows paths, not
Msys virtual file system paths, as postgres is completely unaware of the
latter, so prefix them with the Windows path to the virtual file system
root. Clean psql and pg_recvlogical output of carriage returns.

7 years agoMake PostgresNode.pm check server status more carefully.
Tom Lane [Sat, 22 Apr 2017 22:18:25 +0000 (18:18 -0400)]
Make PostgresNode.pm check server status more carefully.

PostgresNode blithely ignored the exit status of pg_ctl, and in general
made no effort to be sure that the server was running when it should be.
This caused it to miss server crashes, which is a serious shortcoming
in a test scaffold.  Make it complain if pg_ctl fails, and modify the
start and stop logic to complain if the server doesn't start, or doesn't
stop, when expected.

Also, have it turn off the "restart_after_crash" configuration parameter
in created clusters, as bitter experience has shown that leaving that on
can mask crashes too.

We might at some point need variant functions that allow for, eg,
server start failure to be expected.  But no existing test case appears
to want that, and it surely shouldn't be the default behavior.

Note that this *will* break the buildfarm, as it will expose known
bugs that the previous testing failed to.  I'm committing it despite
that, to verify that we get the expected failures in the buildfarm
not just in manual testing.

Back-patch into 9.6 where PostgresNode was introduced.  (The 9.6
branch is not expected to show any failures.)

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

7 years agoMake PostgresNode::append_conf append a newline automatically.
Tom Lane [Sat, 22 Apr 2017 20:58:15 +0000 (16:58 -0400)]
Make PostgresNode::append_conf append a newline automatically.

Although the documentation for append_conf said clearly that it didn't
add a newline, many test authors seem to have forgotten that ... or maybe
they just consulted the example at the top of the POD documentation,
which clearly shows adding a config entry without bothering to add a
trailing newline.  The worst part of that is that it works, as long as
you don't do it more than once, since the backend isn't picky about
whether config files end with newlines.  So there's not a strong forcing
function reminding test authors not to do it like that.  Upshot is that
this is a terribly fragile way to go about things, and there's at least
one existing test case that is demonstrably broken and not testing what
it thinks it is.

Let's just make append_conf append a newline, instead; that is clearly
way safer than the old definition.

I also cleaned up a few call sites that were unnecessarily ugly.
(I left things alone in places where it's plausible that additional
config lines would need to be added someday.)

Back-patch the change in append_conf itself to 9.6 where it was added,
as having a definitional inconsistency between branches would obviously
be pretty hazardous for back-patching TAP tests.  The other changes are
just cosmetic and don't need to be back-patched.

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

7 years agoRequire sufficiently modern version of Test::More for TAP tests
Andrew Dunstan [Sat, 22 Apr 2017 14:04:01 +0000 (10:04 -0400)]
Require sufficiently modern version of Test::More for TAP tests

Ancient versions of Test::More don't support the note() function used in
some TAP tests, so we require the minimum version of the module that
does.

7 years agoPartially revert commit 536d47bd9d5fce8d91929bee3128fa1d08dbcc57.
Tom Lane [Sat, 22 Apr 2017 06:06:16 +0000 (02:06 -0400)]
Partially revert commit 536d47bd9d5fce8d91929bee3128fa1d08dbcc57.

Per buildfarm, the "#ifdef F_SETFD" removed in that commit actually
is needed on Windows, because fcntl() isn't available at all on that
platform, unless using Cygwin.  We could perhaps spell it more like
"#ifdef HAVE_FCNTL", or "#ifndef WIN32", but it's not clear that
those choices are better.

It does seem that we don't need the bogus manual definition of
FD_CLOEXEC, though, so keep that change.

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

7 years agodoc: Update link
Peter Eisentraut [Fri, 21 Apr 2017 23:42:01 +0000 (19:42 -0400)]
doc: Update link

The reference "That is the topic of the next section." has been
incorrect since the materialized views documentation got inserted
between the section "rules-views" and "rules-update".

Author: Zertrin <postgres_wiki@zertrin.org>

7 years agoAvoid depending on non-POSIX behavior of fcntl(2).
Tom Lane [Fri, 21 Apr 2017 19:55:56 +0000 (15:55 -0400)]
Avoid depending on non-POSIX behavior of fcntl(2).

The POSIX standard does not say that the success return value for
fcntl(F_SETFD) and fcntl(F_SETFL) is zero; it says only that it's not -1.
We had several calls that were making the stronger assumption.  Adjust
them to test specifically for -1 for strict spec compliance.

The standard further leaves open the possibility that the O_NONBLOCK
flag bit is not the only active one in F_SETFL's argument.  Formally,
therefore, one ought to get the current flags with F_GETFL and store
them back with only the O_NONBLOCK bit changed when trying to change
the nonblock state.  In port/noblock.c, we were doing the full pushup
in pg_set_block but not in pg_set_noblock, which is just weird.  Make
both of them do it properly, since they have little business making
any assumptions about the socket they're handed.  The other places
where we're issuing F_SETFL are working with FDs we just got from
pipe(2), so it's reasonable to assume the FDs' properties are all
default, so I didn't bother adding F_GETFL steps there.

Also, while pg_set_block deserves some points for trying to do things
right, somebody had decided that it'd be even better to cast fcntl's
third argument to "long".  Which is completely loony, because POSIX
clearly says the third argument for an F_SETFL call is "int".

Given the lack of field complaints, these missteps apparently are not
of significance on any common platforms.  But they're still wrong,
so back-patch to all supported branches.

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

7 years agoChange the on-disk format of SCRAM verifiers to conform to RFC 5803.
Heikki Linnakangas [Fri, 21 Apr 2017 19:51:57 +0000 (22:51 +0300)]
Change the on-disk format of SCRAM verifiers to conform to RFC 5803.

It doesn't make any immediate difference to PostgreSQL, but might as well
follow the standard, since one exists. (I looked at RFC 5803 earlier, but
didn't fully understand it back then.)

The new format uses Base64 instead of hex to encode StoredKey and
ServerKey, which makes the verifiers slightly smaller. Using the same
encoding for the salt and the keys also means that you only need one
encoder/decoder instead of two. Although we have code in the backend to
do both, we are talking about teaching libpq how to create SCRAM verifiers
for PQencodePassword(), and libpq doesn't currently have any code for hex
encoding.

Bump catversion, because this renders any existing SCRAM verifiers in
pg_authid invalid.

Discussion: https://www.postgresql.org/message-id/351ba574-85ea-d9b8-9689-8c928dd0955d@iki.fi

7 years agodoc: Fix typo
Peter Eisentraut [Fri, 21 Apr 2017 19:33:25 +0000 (15:33 -0400)]
doc: Fix typo