]> granicus.if.org Git - postgresql/log
postgresql
6 years agoInitialize TransactionState and user ID consistently at transaction start
Michael Paquier [Wed, 14 Nov 2018 07:46:53 +0000 (16:46 +0900)]
Initialize TransactionState and user ID consistently at transaction start

If a failure happens when a transaction is starting between the moment
the transaction status is changed from TRANS_DEFAULT to TRANS_START and
the moment the current user ID and security context flags are fetched
via GetUserIdAndSecContext(), or before initializing its basic fields,
then those may get reset to incorrect values when the transaction
aborts, leaving the session in an inconsistent state.

One problem reported is that failing a starting transaction at the first
query of a session could cause several kinds of system crashes on the
follow-up queries.

In order to solve that, move the initialization of the transaction state
fields and the call of GetUserIdAndSecContext() in charge of fetching
the current user ID close to the point where the transaction status is
switched to TRANS_START, where there cannot be any error triggered
in-between, per an idea of Tom Lane.  This properly ensures that the
current user ID, the security context flags and that the basic fields of
TransactionState remain consistent even if the transaction fails while
starting.

Reported-by: Richard Guo
Diagnosed-By: Richard Guo
Author: Michael Paquier
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/CAN_9JTxECSb=pEPcb0a8d+6J+bDcOZ4=DgRo_B7Y5gRHJUM=Rw@mail.gmail.com
Backpatch-through: 9.4

6 years agoAdd flag values in WAL description to all heap records
Michael Paquier [Wed, 14 Nov 2018 01:33:10 +0000 (10:33 +0900)]
Add flag values in WAL description to all heap records

Hexadecimal is consistently used as format to not bloat too much the
output but keep it readable.  This information is useful mainly for
debugging purposes with for example pg_waldump.

Author: Michael Paquier
Reviewed-by: Nathan Bossart, Dmitry Dolgov, Andres Freund, Álvaro
Herrera
Discussion: https://postgr.es/m/20180413034734.GE1552@paquier.xyz

6 years agoRefactor code creating PartitionBoundInfo
Michael Paquier [Wed, 14 Nov 2018 01:01:49 +0000 (10:01 +0900)]
Refactor code creating PartitionBoundInfo

The code building PartitionBoundInfo based on the constituent partition
data read from catalogs has been located in partcache.c, with a specific
set of routines dedicated to bound types, like sorting or bound data
creation.  All this logic is moved to partbounds.c and relocates all the
bound-specific logistic into it, with partition_bounds_create() as
principal entry point.

Author: Amit Langote
Reviewed-by: Michael Paquier, Álvaro Herrera
Discussion: https://postgr.es/m/3f289da8-6d10-75fe-814a-635e8b191d43@lab.ntt.co.jp

6 years agoAdd INSERT ON CONFLICT test on partitioned tables with transition table
Alvaro Herrera [Tue, 13 Nov 2018 21:12:39 +0000 (18:12 -0300)]
Add INSERT ON CONFLICT test on partitioned tables with transition table

This case was uncovered by existing tests, so breakage went undetected.
Make sure it remains stable.

Extracted from a larger patch by
Author: David Rowley
Reviewed-by: Amit Langote
Discussion: https://postgr.es/m/CAKJS1f-aGCJ5H7_hiSs5PhWs6Obmj+vGARjGymqH1=o5PcrNnQ@mail.gmail.com

6 years agoFix incorrect results for numeric data passed through an ECPG SQLDA.
Tom Lane [Tue, 13 Nov 2018 20:46:08 +0000 (15:46 -0500)]
Fix incorrect results for numeric data passed through an ECPG SQLDA.

Numeric values with leading zeroes were incorrectly copied into a
SQLDA (SQL Descriptor Area), leading to wrong results in ECPG programs.

Report and patch by Daisuke Higuchi.  Back-patch to all supported
versions.

Discussion: https://postgr.es/m/1803D792815FC24D871C00D17AE95905C71161@g01jpexmbkw24

6 years agoFix realfailN lexer rules to not make assumptions about input format.
Tom Lane [Tue, 13 Nov 2018 19:54:41 +0000 (14:54 -0500)]
Fix realfailN lexer rules to not make assumptions about input format.

The realfail1 and realfail2 backup-prevention rules always returned
token type FCONST, ignoring the possibility that what we've scanned
is more appropriately described as ICONST.  I think that at the
time that code was added, it might actually have been safe to not
distinguish; but since we started allowing AS-less aliases in SELECT
target lists, it's definitely legal to have a number immediately
followed by an identifier.

In the SELECT case, it seems there's no visible consequence because
make_const() will change the type back to integer anyway.  But I'm
worried that there are other contexts, or will be in future, where
it's more important to get the constant's type right.

Hence, use process_integer_literal to correctly determine which
token type to return.

Arguably this is a bug fix, but given the lack of evidence of
user-visible problems, I'll refrain from back-patching.

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

6 years agoRemove unused code in ECPG.
Tom Lane [Tue, 13 Nov 2018 18:04:15 +0000 (13:04 -0500)]
Remove unused code in ECPG.

scanner_init/scanner_finish weren't actually called from anywhere,
and the scanbuf variables they set up weren't used either.

Remove unused declaration for mm_realloc, too.

John Naylor

Discussion: https://postgr.es/m/CAJVSVGWGqY9YBs2EwtRUkbNv=hXkN8yRPOoD1wxE6COgvvrz5g@mail.gmail.com

6 years agoAlign ECPG lexer more closely with the core and psql lexers.
Tom Lane [Tue, 13 Nov 2018 17:57:52 +0000 (12:57 -0500)]
Align ECPG lexer more closely with the core and psql lexers.

Make a bunch of basically-cosmetic changes to reduce the diffs between
the flex rules in scan.l, psqlscan.l, and pgc.l.  Reorder some code,
adjust a lot of whitespace, sync some comments, make use of flex start
condition scopes to do that.

There are a few non-cosmetic changes in the ECPG lexer:

* Bring over the decimalfail rule (and support function
process_integer_literal) so that ECPG will lex "1..10" into
the same tokens as the backend would.  I'm not sure this makes any
visible difference to users, but I'm not sure it doesn't, either.

* <xdc><<EOF>> gets its own rule so as to produce a more on-point
error message.

* Remove duplicate <SQL>{xdstart} rule.

John Naylor, with a few additional changes by me

Discussion: https://postgr.es/m/CAJVSVGWGqY9YBs2EwtRUkbNv=hXkN8yRPOoD1wxE6COgvvrz5g@mail.gmail.com

6 years agodoc: Fix minor whitespace issue
Peter Eisentraut [Tue, 13 Nov 2018 12:51:38 +0000 (13:51 +0100)]
doc: Fix minor whitespace issue

Reported-by: David G. Johnston <david.g.johnston@gmail.com>
6 years agopg_dump: Fix dumping of WITH OIDS tables
Peter Eisentraut [Fri, 9 Nov 2018 08:17:25 +0000 (09:17 +0100)]
pg_dump: Fix dumping of WITH OIDS tables

A table with OIDs that was the first in the dump output would not get
dumped with OIDs enabled.  Fix that.

The reason was that the currWithOids flag was declared to be bool but
actually also takes a -1 value for "don't know yet".  But under
stdbool.h semantics, that is coerced to true, so the required SET
default_with_oids command is not output again.  Change the variable
type to char to fix that.

Reported-by: Derek Nelson <derek@pipelinedb.com>
6 years agoFix const correctness warning.
Thomas Munro [Tue, 13 Nov 2018 05:32:05 +0000 (18:32 +1300)]
Fix const correctness warning.

Per buildfarm.

6 years agoFix the initialization of atomic variables introduced by the
Amit Kapila [Tue, 13 Nov 2018 05:22:40 +0000 (10:52 +0530)]
Fix the initialization of atomic variables introduced by the
group clearing mechanism.

Commits 0e141c0fbb and baaf272ac9 introduced initialization of atomic
variables in InitProcess which means that it's not safe to look at those
for backends that aren't currently in use.  Fix that by initializing them
during postmaster startup.

Reported-by: Andres Freund
Author: Amit Kapila
Backpatch-through: 9.6
Discussion: https://postgr.es/m/20181027104138.qmbbelopvy7cw2qv@alap3.anarazel.de

6 years agoFix handling of HBA ldapserver with multiple hostnames.
Thomas Munro [Tue, 13 Nov 2018 04:39:36 +0000 (17:39 +1300)]
Fix handling of HBA ldapserver with multiple hostnames.

Commit 35c0754f failed to handle space-separated lists of alternative
hostnames in ldapserver, when building a URI for ldap_initialize()
(OpenLDAP).  Such lists need to be expanded to space-separated URIs.

Repair.  Back-patch to 11, to fix bug report #15495.

Author: Thomas Munro
Reported-by: Renaud Navarro
Discussion: https://postgr.es/m/15495-2c39fc196c95cd72%40postgresql.org

6 years agoFix possible buffer overrun in hba.c.
Thomas Munro [Tue, 13 Nov 2018 03:27:13 +0000 (16:27 +1300)]
Fix possible buffer overrun in hba.c.

Coverty reports a possible buffer overrun in the code that populates the
pg_hba_file_rules view.  It may not be a live bug due to restrictions
on options that can be used together, but let's increase MAX_HBA_OPTIONS
and correct a nearby misleading comment.

Back-patch to 10 where this code arrived.

Reported-by: Julian Hsiao
Discussion: https://postgr.es/m/CADnGQpzbkWdKS2YHNifwAvX5VEsJ5gW49U4o-7UL5pzyTv4vTg%40mail.gmail.com

6 years agoRemove CommandCounterIncrement() after processing ON COMMIT DELETE
Michael Paquier [Mon, 12 Nov 2018 23:59:41 +0000 (08:59 +0900)]
Remove CommandCounterIncrement() after processing ON COMMIT DELETE

This comes from f9b5b41, which is part of one the original commits that
implemented ON COMMIT actions.  By looking at the truncation code, any
CCI needed happens locally when rebuilding indexes, so it looks safe to
just remove this final incrementation.

Author: Michael Paquier
Reviewed-by: Álvaro Herrera
Discussion: https://postgr.es/m/20181109024731.GF2652@paquier.xyz

6 years agoSimplify null-element handling in extension_config_remove().
Tom Lane [Mon, 12 Nov 2018 16:50:28 +0000 (11:50 -0500)]
Simplify null-element handling in extension_config_remove().

There's no point in asking deconstruct_array() for a null-flags
array when we already checked the array has no nulls, and aren't
going to examine the output anyhow.  Not asking for this output
should make the code marginally faster, and it's also more
robust since if there somehow were nulls, deconstruct_array()
would throw an error.

Daniel Gustafsson

Discussion: https://postgr.es/m/289FFB8B-7AAB-48B5-A497-6E0D41D7BA47@yesql.se

6 years agoLimit the number of index clauses considered in choose_bitmap_and().
Tom Lane [Mon, 12 Nov 2018 16:19:04 +0000 (11:19 -0500)]
Limit the number of index clauses considered in choose_bitmap_and().

classify_index_clause_usage() is O(N^2) in the number of distinct index
qual clauses it considers, because of its use of a simple search list to
store them.  For nearly all queries, that's fine because only a few clauses
will be considered.  But Alexander Kuzmenkov reported a machine-generated
query with 80000 (!) index qual clauses, which caused this code to take
forever.  Somewhat remarkably, this is the only O(N^2) behavior we now
have for such a query, so let's fix it.

We can get rid of the O(N^2) runtime for cases like this without much
damage to the functionality of choose_bitmap_and() by separating out
paths with "too many" qual or pred clauses, and deeming them to always
be nonredundant with other paths.  Then their clauses needn't go into
the search list, so it doesn't get too long, but we don't lose the
ability to consider bitmap AND plans altogether.  I set the threshold
for "too many" to be 100 clauses per path, which should be plenty to
ensure no change in planning behavior for normal queries.

There are other things we could do to make this go faster, but it's not
clear that it's worth any additional effort.  80000 qual clauses require
a whole lot of work in many other places, too.

The code's been like this for a long time, so back-patch to all supported
branches.  The troublesome query only works back to 9.5 (in 9.4 it fails
with stack overflow in the parser); so I'm not sure that fixing this in
9.4 has any real-world benefit, but perhaps it does.

Discussion: https://postgr.es/m/90c5bdfa-d633-dabe-9889-3cf3e1acd443@postgrespro.ru

6 years agoFix incorrect author name in release notes
Michael Paquier [Mon, 12 Nov 2018 14:00:38 +0000 (23:00 +0900)]
Fix incorrect author name in release notes

Author: Alexander Lakhin
Discussion: https://postgr.es/m/2f55f6d2-3fb0-d4f6-5c47-18da3a1117e0@gmail.com

6 years agodoc: Small punctuation improvement
Peter Eisentraut [Mon, 12 Nov 2018 13:47:58 +0000 (14:47 +0100)]
doc: Small punctuation improvement

6 years agodoc: Small run-time pruning doc fix
Peter Eisentraut [Mon, 12 Nov 2018 13:34:28 +0000 (14:34 +0100)]
doc: Small run-time pruning doc fix

A note in ddl.sgml used to mention that run-time pruning was only
implemented for Append.  When we got MergeAppend support, this was
updated to mention that MergeAppend is supported too.  This is
slightly weird as it's not all that obvious what exactly isn't
supported when we mention:

    <para>
     Both of these behaviors are likely to be changed in a future release
     of <productname>PostgreSQL</productname>.
    </para>

This patch updates this to mention that ModifyTable is unsupported,
which makes the above fragment make sense again.

Author: David Rowley <david.rowley@2ndquadrant.com>

6 years agoDisable MSVC warning caused by recent snprintf.c changes
Andrew Dunstan [Sun, 11 Nov 2018 01:20:54 +0000 (20:20 -0500)]
Disable MSVC warning caused by recent snprintf.c changes

Discussion: https://postgr.es/m/05f348de-0c79-d88d-69b7-434ef828bd4d@2ndQuadrant.com

6 years agoRemove volatiles from {procarray,volatile}.c and fix memory ordering issue.
Andres Freund [Sat, 10 Nov 2018 04:54:40 +0000 (20:54 -0800)]
Remove volatiles from {procarray,volatile}.c and fix memory ordering issue.

The use of volatiles in procarray.c largely originated from the time
when postgres did not have reliable compiler and memory
barriers. That's not the case anymore, so we can do better.

Several of the functions in procarray.c can be bottlenecks, and
removal of volatile yields mildly better code.

The new state, with explicit memory barriers, is also more
correct. The previous use of volatile did not actually deliver
sufficient guarantees on weakly ordered machines, in particular the
logic in GetNewTransactionId() does not look safe.  It seems unlikely
to be a problem in practice, but worth fixing.

Thomas and I independently wrote a patch for this.

Reported-By: Andres Freund and Thomas Munro
Author: Andres Freund, with cherrypicked changes from a patch by Thomas Munro
Discussion:
    https://postgr.es/m/20181005172955.wyjb4fzcdzqtaxjq@alap3.anarazel.de
    https://postgr.es/m/CAEepm=1nff0x=7i3YQO16jLA2qw-F9O39YmUew4oq-xcBQBs0g@mail.gmail.com

6 years agoApply RI trigger skipping tests also for DELETE
Peter Eisentraut [Thu, 19 Jul 2018 06:37:32 +0000 (08:37 +0200)]
Apply RI trigger skipping tests also for DELETE

The tests added in cfa0f4255bb0f5550d37a01c4d8fe2966d20040c to skip
firing an RI trigger if any old key value is NULL can also be applied
for DELETE.  This should give a performance gain in those cases, and it
also saves a lot of duplicate code in the actual RI triggers.  (That
code was already dead code for the UPDATE cases.)

Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
6 years agoRemove dead foreign key optimization code
Peter Eisentraut [Thu, 19 Jul 2018 06:20:59 +0000 (08:20 +0200)]
Remove dead foreign key optimization code

The ri_KeysEqual() calls in the foreign-key trigger functions to
optimize away some updates are useless because since
adfeef55cbcc5dc72a772777f88c1be05a70dfee those triggers are not enqueued
at all.  (It's also not useful to keep these checks as some kind of
backstop, since it's also semantically correct to just run the full
check even with equal keys.)

Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
6 years agoCombine two flag tests in GetSnapshotData().
Andres Freund [Sat, 10 Nov 2018 04:43:56 +0000 (20:43 -0800)]
Combine two flag tests in GetSnapshotData().

Previously the code checked PROC_IN_LOGICAL_DECODING and
PROC_IN_VACUUM separately. As the relevant variable is marked as
volatile, the compiler cannot combine the two tests.  As
GetSnapshotData() is pretty hot in a number of workloads, it's
worthwhile to fix that.

It'd also be a good idea to get rid of the volatiles altogether. But
for one that's a larger patch, and for another, the code after this
change still seems at least as easy to read as before.

Author: Andres Freund
Discussion: https://postgr.es/m/20181005172955.wyjb4fzcdzqtaxjq@alap3.anarazel.de

6 years agodocs: Adapt wal_segment_size docs to fc49e24fa69.
Andres Freund [Sat, 10 Nov 2018 03:24:05 +0000 (19:24 -0800)]
docs: Adapt wal_segment_size docs to fc49e24fa69.

Before this change the docs weren't adapted to the fact that
wal_segment_size is now measured in bytes, rather than multiples of
wal_block_size.

Author: David Steele
Discussion: https://postgr.es/m/68ea97d6-2ed9-f339-e57d-ab3a33caf3b1@pgmasters.net
Backpatch: 11-, like fc49e24fa69 itself.

6 years agoFix error-cleanup mistakes in exec_stmt_call().
Tom Lane [Sat, 10 Nov 2018 03:04:14 +0000 (22:04 -0500)]
Fix error-cleanup mistakes in exec_stmt_call().

Commit 15c729347 was a couple bricks shy of a load: we need to
ensure that expr->plan gets reset to NULL on any error exit,
if it's not supposed to be saved.  Also ensure that the
stmt->target calculation gets redone if needed.

The easy way to exhibit a problem is to set up code that
violates the writable-argument restriction and then execute
it twice.  But error exits out of, eg, setup_param_list()
could also break it.  Make the existing PG_TRY block cover
all of that code to be sure.

Per report from Pavel Stehule.

Discussion: https://postgr.es/m/CAFj8pRAeXNTO43W2Y0Cn0YOVFPv1WpYyOqQrrzUiN6s=dn7gCg@mail.gmail.com

6 years agoFix missing role dependencies for some schema and type ACLs.
Tom Lane [Sat, 10 Nov 2018 01:42:03 +0000 (20:42 -0500)]
Fix missing role dependencies for some schema and type ACLs.

This patch fixes several related cases in which pg_shdepend entries were
never made, or were lost, for references to roles appearing in the ACLs of
schemas and/or types.  While that did no immediate harm, if a referenced
role were later dropped, the drop would be allowed and would leave a
dangling reference in the object's ACL.  That still wasn't a big problem
for normal database usage, but it would cause obscure failures in
subsequent dump/reload or pg_upgrade attempts, taking the form of
attempts to grant privileges to all-numeric role names.  (I think I've
seen field reports matching that symptom, but can't find any right now.)

Several cases are fixed here:

1. ALTER DOMAIN SET/DROP DEFAULT would lose the dependencies for any
existing ACL entries for the domain.  This case is ancient, dating
back as far as we've had pg_shdepend tracking at all.

2. If a default type privilege applies, CREATE TYPE recorded the
ACL properly but forgot to install dependency entries for it.
This dates to the addition of default privileges for types in 9.2.

3. If a default schema privilege applies, CREATE SCHEMA recorded the
ACL properly but forgot to install dependency entries for it.
This dates to the addition of default privileges for schemas in v10
(commit ab89e465c).

Another somewhat-related problem is that when creating a relation
rowtype or implicit array type, TypeCreate would apply any available
default type privileges to that type, which we don't really want
since such an object isn't supposed to have privileges of its own.
(You can't, for example, drop such privileges once they've been added
to an array type.)

ab89e465c is also to blame for a race condition in the regression tests:
privileges.sql transiently installed globally-applicable default
privileges on schemas, which sometimes got absorbed into the ACLs of
schemas created by concurrent test scripts.  This should have resulted
in failures when privileges.sql tried to drop the role holding such
privileges; but thanks to the bug fixed here, it instead led to dangling
ACLs in the final state of the regression database.  We'd managed not to
notice that, but it became obvious in the wake of commit da906766c, which
allowed the race condition to occur in pg_upgrade tests.

To fix, add a function recordDependencyOnNewAcl to encapsulate what
callers of get_user_default_acl need to do; while the original call
sites got that right via ad-hoc code, none of the later-added ones
have.  Also change GenerateTypeDependencies to generate these
dependencies, which requires adding the typacl to its parameter list.
(That might be annoying if there are any extensions calling that
function directly; but if there are, they're most likely buggy in the
same way as the core callers were, so they need work anyway.)  While
I was at it, I changed GenerateTypeDependencies to accept most of its
parameters in the form of a Form_pg_type pointer, making its parameter
list a bit less unwieldy and mistake-prone.

The test race condition is fixed just by wrapping the addition and
removal of default privileges into a single transaction, so that that
state is never visible externally.  We might eventually prefer to
separate out tests of default privileges into a script that runs by
itself, but that would be a bigger change and would make the tests
run slower overall.

Back-patch relevant parts to all supported branches.

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

6 years agoRemove ineffective check against dropped columns from slot_getattr().
Andres Freund [Sat, 10 Nov 2018 01:40:40 +0000 (17:40 -0800)]
Remove ineffective check against dropped columns from slot_getattr().

Before this commit slot_getattr() checked for dropped
columns (returning NULL in that case), but only after checking for
previously deformed columns. As slot_deform_tuple() does not contain
such a check, the check in slot_getattr() would often not have been
reached, depending on previous use of the slot.

These days locking and plan invalidation ought to ensure that dropped
columns are not accessed in query plans. Therefore this commit just
drops the insufficient check in slot_getattr().  It's possible that
we'll find some holes againt use of dropped columns, but if so, those
need to be addressed independent of slot_getattr(), as most accesses
don't go through that function anyway.

Author: Andres Freund
Discussion: https://postgr.es/m/20181107174403.zai7fedgcjoqx44p@alap3.anarazel.de

6 years agoDon't require return slots for nodes without projection.
Andres Freund [Sat, 10 Nov 2018 01:19:39 +0000 (17:19 -0800)]
Don't require return slots for nodes without projection.

In a lot of nodes the return slot is not required. That can either be
because the node doesn't do any projection (say an Append node), or
because the node does perform projections but the projection is
optimized away because the projection would yield an identical row.

Slots aren't that small, especially for wide rows, so it's worthwhile
to avoid creating them.  It's not possible to just skip creating the
slot - it's currently used to determine the tuple descriptor returned
by ExecGetResultType().  So separate the determination of the result
type from the slot creation.  The work previously done internally
ExecInitResultTupleSlotTL() can now also be done separately with
ExecInitResultTypeTL() and ExecInitResultSlot().  That way nodes that
aren't guaranteed to need a result slot, can use
ExecInitResultTypeTL() to determine the result type of the node, and
ExecAssignScanProjectionInfo() (via
ExecConditionalAssignProjectionInfo()) determines that a result slot
is needed, it is created with ExecInitResultSlot().

Besides the advantage of avoiding to create slots that then are
unused, this is necessary preparation for later patches around tuple
table slot abstraction. In particular separating the return descriptor
and slot is a prerequisite to allow JITing of tuple deforming with
knowledge of the underlying tuple format, and to avoid unnecessarily
creating JITed tuple deforming for virtual slots.

This commit removes a redundant argument from
ExecInitResultTupleSlotTL(). While this commit touches a lot of the
relevant lines anyway, it'd normally still not worthwhile to cause
breakage, except that aforementioned later commits will touch *all*
ExecInitResultTupleSlotTL() callers anyway (but fits worse
thematically).

Author: Andres Freund
Discussion: https://postgr.es/m/20181105210039.hh4vvi4vwoq5ba2q@alap3.anarazel.de

6 years agoFix incorrect routine name in xlog_heapam.h
Michael Paquier [Fri, 9 Nov 2018 23:58:55 +0000 (08:58 +0900)]
Fix incorrect routine name in xlog_heapam.h

s/xl_heap_delete/xl_heap_truncate/ in a comment block referring to flags
for truncation.

Discussion: https://postgr.es/m/20180413034734.GE1552@paquier.xyz

6 years agoIndicate session name in isolationtester notices
Alvaro Herrera [Fri, 9 Nov 2018 16:08:00 +0000 (13:08 -0300)]
Indicate session name in isolationtester notices

When a session under isolationtester produces printable notices (NOTICE,
WARNING) we were just printing them unadorned, which can be confusing
when debugging.  Prefix them with the session name, which makes things
clearer.

Author: Álvaro Herrera
Reviewed-by: Hari Babu Kommi
Discussion: https://postgr.es/m/20181024213451.75nh3f3dctmcdbfq@alvherre.pgsql

6 years agoFix dependency handling of partitions and inheritance for ON COMMIT
Michael Paquier [Fri, 9 Nov 2018 01:03:22 +0000 (10:03 +0900)]
Fix dependency handling of partitions and inheritance for ON COMMIT

This commit fixes a set of issues with ON COMMIT actions when used on
partitioned tables and tables with inheritance children:
- Applying ON COMMIT DROP on a partitioned table with partitions or on a
table with inheritance children caused a failure at commit time, with
complains about the children being already dropped as all relations are
dropped one at the same time.
- Applying ON COMMIT DELETE on a partition relying on a partitioned
table which uses ON COMMIT DROP would cause the partition truncation to
fail as the parent is removed first.

The solution to the first problem is to handle the removal of all the
dependencies in one go instead of dropping relations one-by-one, based
on a suggestion from Álvaro Herrera.  So instead all the relation OIDs
to remove are gathered and then processed in one round of multiple
deletions.

The solution to the second problem is to reorder the actions, with
truncation happening first and relation drop done after.  Even if it
means that a partition could be first truncated, then immediately
dropped if its partitioned table is dropped, this has the merit to keep
the code simple as there is no need to do existence checks on the
relations to drop.

Contrary to a manual TRUNCATE on a partitioned table, ON COMMIT DELETE
does not cascade to its partitions.  The ON COMMIT action defined on
each partition gets the priority.

Author: Michael Paquier
Reviewed-by: Amit Langote, Álvaro Herrera, Robert Haas
Discussion: https://postgr.es/m/68f17907-ec98-1192-f99f-8011400517f5@lab.ntt.co.jp
Backpatch-through: 10

6 years agoDisallow setting client_min_messages higher than ERROR.
Tom Lane [Thu, 8 Nov 2018 22:33:25 +0000 (17:33 -0500)]
Disallow setting client_min_messages higher than ERROR.

Previously it was possible to set client_min_messages to FATAL or PANIC,
which had the effect of suppressing transmission of regular ERROR messages
to the client.  Perhaps that seemed like a useful option in the past, but
the trouble with it is that it breaks guarantees that are explicitly made
in our FE/BE protocol spec about how a query cycle can end.  While libpq
and psql manage to cope with the omission, that's mostly because they
are not very bright; client libraries that have more semantic knowledge
are likely to get confused.  Notably, pgODBC doesn't behave very sanely.
Let's fix this by getting rid of the ability to set client_min_messages
above ERROR.

In HEAD, just remove the FATAL and PANIC options from the set of allowed
enum values for client_min_messages.  (This change also affects
trace_recovery_messages, but that's OK since these aren't useful values
for that variable either.)

In the back branches, there was concern that rejecting these values might
break applications that are explicitly setting things that way.  I'm
pretty skeptical of that argument, but accommodate it by accepting these
values and then internally setting the variable to ERROR anyway.

In all branches, this allows a couple of tiny simplifications in the
logic in elog.c, so do that.

Also respond to the point that was made that client_min_messages has
exactly nothing to do with the server's logging behavior, and therefore
does not belong in the "When To Log" subsection of the documentation.
The "Statement Behavior" subsection is a better match, so move it there.

Jonah Harris and Tom Lane

Discussion: https://postgr.es/m/7809.1541521180@sss.pgh.pa.us
Discussion: https://postgr.es/m/15479-ef0f4cc2fd995ca2@postgresql.org

6 years agoRevise attribute handling code on partition creation
Alvaro Herrera [Thu, 8 Nov 2018 19:22:09 +0000 (16:22 -0300)]
Revise attribute handling code on partition creation

The original code to propagate NOT NULL and default expressions
specified when creating a partition was mostly copy-pasted from
typed-tables creation, but not being a great match it contained some
duplicity, inefficiency and bugs.

This commit fixes the bug that NOT NULL constraints declared in the
parent table would not be honored in the partition.  One reported issue
that is not fixed is that a DEFAULT declared in the child is not used
when inserting through the parent.  That would amount to a behavioral
change that's better not back-patched.

This rewrite makes the code simpler:

1. instead of checking for duplicate column names in its own block,
reuse the original one that already did that;

2. instead of concatenating the list of columns from parent and the one
declared in the partition and scanning the result to (incorrectly)
propagate defaults and not-null constraints, just scan the latter
searching the former for a match, and merging sensibly.  This works
because we know the list in the parent is already correct and there can
only be one parent.

This rewrite makes ColumnDef->is_from_parent unused, so it's removed
on branch master; on released branches, it's kept as an unused field in
order not to cause ABI incompatibilities.

This commit also adds a test case for creating partitions with
collations mismatching that on the parent table, something that is
closely related to the code being patched.  No code change is introduced
though, since that'd be a behavior change that could break some (broken)
working applications.

Amit Langote wrote a less invasive fix for the original
NOT NULL/defaults bug, but while I kept the tests he added, I ended up
not using his original code.  Ashutosh Bapat reviewed Amit's fix.  Amit
reviewed mine.

Author: Álvaro Herrera, Amit Langote
Reviewed-by: Ashutosh Bapat, Amit Langote
Reported-by: Jürgen Strobel (bug #15212)
Discussion: https://postgr.es/m/152746742177.1291.9847032632907407358@wrigleys.postgresql.org

6 years agoAdjust valgrind fix in commit 517b0d0b5f
Andrew Dunstan [Thu, 8 Nov 2018 13:38:46 +0000 (08:38 -0500)]
Adjust valgrind fix in commit 517b0d0b5f

lousyjack still wasn't happy. I have tested this modification and it
worked.

6 years agoFix incorrect routine name reference in partprune.c
Michael Paquier [Thu, 8 Nov 2018 11:14:16 +0000 (20:14 +0900)]
Fix incorrect routine name reference in partprune.c

Author: Yuzuko Hosoya
Discussion: https://postgr.es/m/00ac01d4774c$7feac860$7fc05920$@lab.ntt.co.jp

6 years agoFixup for b84a6dafbf triggering assert failure in LLVM debug builds.
Andres Freund [Wed, 7 Nov 2018 21:58:52 +0000 (13:58 -0800)]
Fixup for b84a6dafbf triggering assert failure in LLVM debug builds.

Author: Andres Freund

6 years agoFix inadequate autoconfiscation of copyfile() usage.
Tom Lane [Wed, 7 Nov 2018 21:41:42 +0000 (16:41 -0500)]
Fix inadequate autoconfiscation of copyfile() usage.

Per buildfarm, HAVE_COPYFILE is not the same thing as HAVE_COPYFILE_H.
Add the extra configure test.

6 years agoUse parallel installcheck in vcregress.pl's upgrade test
Andrew Dunstan [Wed, 7 Nov 2018 19:15:19 +0000 (14:15 -0500)]
Use parallel installcheck in vcregress.pl's upgrade test

This is to keep the test in sync with what's done in test.sh, which
acquired this change in commit da906766c.

6 years agoMove EEOP_*_SYSVAR evaluation out of line.
Andres Freund [Wed, 7 Nov 2018 19:08:45 +0000 (11:08 -0800)]
Move EEOP_*_SYSVAR evaluation out of line.

This mainly de-duplicates code. As evaluating a system variable isn't
the hottest path and the current inline implementation ends up calling
out to an external function anyway, this is OK from a performance POV.

The main motivation for de-duplicating is the upcoming slot
abstraction work, after which there's not guaranteed to be a HeapTuple
backing the slot.

Author: Andres Freund, Amit Khandekar
Discussion: https://postgr.es/m/20181105210039.hh4vvi4vwoq5ba2q@alap3.anarazel.de

6 years agoQuiet valgrind complaints following pread/pwrite changes
Andrew Dunstan [Wed, 7 Nov 2018 17:54:24 +0000 (12:54 -0500)]
Quiet valgrind complaints following pread/pwrite changes

Per complaints from buildfarm and elsewhere
Patch from Jasper Pedersen
Discussion: https://postgr.es/m/0f419c91-49ab-2399-0143-13063bd97c46@redhat.com

6 years agopg_upgrade: Allow use of file cloning
Peter Eisentraut [Wed, 7 Nov 2018 17:05:54 +0000 (18:05 +0100)]
pg_upgrade: Allow use of file cloning

Add another transfer mode --clone to pg_upgrade (besides the existing
--link and the default copy), using special file cloning calls.  This
makes the file transfer faster and more space efficient, achieving
speed similar to --link mode without the associated drawbacks.

On Linux, file cloning is supported on Btrfs and XFS (if formatted with
reflink support).  On macOS, file cloning is supported on APFS.

Reviewed-by: Michael Paquier <michael@paquier.xyz>
6 years agoBuild HashState's hashkeys expression with the correct parent.
Andres Freund [Wed, 7 Nov 2018 17:25:54 +0000 (09:25 -0800)]
Build HashState's hashkeys expression with the correct parent.

Previously the expressions were built with the HashJoinState as a
parent. That's incorrect.

Currently this does not appear to be harmful, but for the upcoming
'slot abstraction' work this proves to be problematic, as the
underlying slot types can differ between Hash and HashJoin.  It's
possible that this already causes a problem, but I've not been able to
come up with a scenario.  Therefore don't backpatch at this point.

Author: Andres Freund
Discussion: https://postgr.es/m/20180220224318.gw4oe5jadhpmcdnm@alap3.anarazel.de

6 years agoUse installcheck-parallel in pg_upgrade's testsuite.
Andres Freund [Wed, 7 Nov 2018 17:22:48 +0000 (09:22 -0800)]
Use installcheck-parallel in pg_upgrade's testsuite.

The installcheck run takes a sizable fraction of test.sh to run. Using
a parallel schedule reduces that noticably.

It's possible that we want to backpatch this at some point, to reduce
buildfarm times, but for now lets just see if this upsets the
buildfarm.

Author: Andres Freund
Discussion: https://postgr.es/m/20181105210039.hh4vvi4vwoq5ba2q@alap3.anarazel.de

6 years agoPostpone calculating total_table_pages until after pruning/exclusion.
Tom Lane [Wed, 7 Nov 2018 17:12:56 +0000 (12:12 -0500)]
Postpone calculating total_table_pages until after pruning/exclusion.

The planner doesn't make any use of root->total_table_pages until it
estimates costs of indexscans, so we don't need to compute it as
early as that's currently done.  By doing the calculation between
set_base_rel_sizes and set_base_rel_pathlists, we can omit relations
that get removed from the query by partition pruning or constraint
exclusion, which seems like a more accurate basis for costing.

(Historical note: I think at the time this code was written, there
was not a separation between the "set sizes" and "set pathlists"
steps, so that this approach would have been impossible at the time.
But now that we have that separation, this is clearly the better way
to do things.)

David Rowley, reviewed by Edmund Horner

Discussion: https://postgr.es/m/CAKJS1f-NG1mRM0VOtkAG7=ZLQWihoqees9R4ki3CKBB0-fRfCA@mail.gmail.com

6 years agoLast-minute updates for release notes.
Tom Lane [Tue, 6 Nov 2018 23:56:26 +0000 (18:56 -0500)]
Last-minute updates for release notes.

Add entries for v11 changes that went in post-stamping, but before
the final wrap.

6 years agoDisable recheck_on_update optimization to avoid crashes.
Tom Lane [Tue, 6 Nov 2018 23:33:15 +0000 (18:33 -0500)]
Disable recheck_on_update optimization to avoid crashes.

The code added by commit c203d6cf8 causes a crash in at least one case,
where a potentially-optimizable expression index has a storage type
different from the input data type.  A cursory code review turned up
numerous other problems that seem impractical to fix on short notice.

Andres argued for revert of that patch some time ago, and if additional
senior committers had been paying attention, that's likely what would
have happened, but we were not :-(

At this point we can't just revert, at least not in v11, because that would
mean an ABI break for code touching relcache entries.  And we should not
remove the (also buggy) support for the recheck_on_update index reloption,
since it might already be used in some databases in the field.  So this
patch just does the as-little-invasive-as-possible measure of disabling
the feature as though recheck_on_update were forced off for all indexes.
I also removed the related regression tests (which would otherwise fail)
and the user-facing documentation of the reloption.

We should undertake a more thorough code cleanup if the patch can't be
fixed, but not under the extreme time pressure of being already overdue
for 11.1 release.

Per report from Ondřej Bouda and subsequent private discussion among
pgsql-release.

Discussion: https://postgr.es/m/20181106185255.776mstcyehnc63ty@alvherre.pgsql

6 years agoRemove set-but-unused variable.
Thomas Munro [Tue, 6 Nov 2018 22:45:49 +0000 (11:45 +1300)]
Remove set-but-unused variable.

Clean-up for commit c24dcd0c.

Reported-by: Andrew Dunstan
Discussion: https://postgr.es/m/2d52ff4a-5440-f6f1-7806-423b0e6370cb%402ndQuadrant.com

6 years agoOptimize nested ConvertRowtypeExpr nodes.
Andrew Gierth [Tue, 6 Nov 2018 14:19:40 +0000 (14:19 +0000)]
Optimize nested ConvertRowtypeExpr nodes.

A ConvertRowtypeExpr is used to translate a whole-row reference of a
child to that of a parent. The planner produces nested
ConvertRowtypeExpr while translating whole-row reference of a leaf
partition in a multi-level partition hierarchy. Executor then
translates the whole-row reference from the leaf partition into all
the intermediate parent's whole-row references before arriving at the
final whole-row reference. It could instead translate the whole-row
reference from the leaf partition directly to the top-most parent's
whole-row reference skipping any intermediate translations.

Ashutosh Bapat, with tests by Kyotaro Horiguchi and some
editorialization by me. Reviewed by Andres Freund, Pavel Stehule,
Kyotaro Horiguchi, Dmitry Dolgov, Tom Lane.

6 years agoUse pg_pread() and pg_pwrite() for data files and WAL.
Thomas Munro [Tue, 6 Nov 2018 20:51:50 +0000 (09:51 +1300)]
Use pg_pread() and pg_pwrite() for data files and WAL.

Cut down on system calls by doing random I/O using offset-based OS
routines where available.  Remove the code for tracking the 'virtual'
seek position.  The only reason left to call FileSeek() was to get
the file's size, so provide a new function FileSize() instead.

Author: Oskari Saarenmaa, Thomas Munro
Reviewed-by: Thomas Munro, Jesper Pedersen, Tom Lane, Alvaro Herrera
Discussion: https://postgr.es/m/CAEepm=02rapCpPR3ZGF2vW=SBHSdFYO_bz_f-wwWJonmA3APgw@mail.gmail.com
Discussion: https://postgr.es/m/b8748d39-0b19-0514-a1b9-4e5a28e6a208%40gmail.com
Discussion: https://postgr.es/m/a86bd200-ebbe-d829-e3ca-0c4474b2fcb7%40ohmu.fi

6 years agoProvide pg_pread() and pg_pwrite() for random I/O.
Thomas Munro [Tue, 6 Nov 2018 20:50:01 +0000 (09:50 +1300)]
Provide pg_pread() and pg_pwrite() for random I/O.

Forward to POSIX pread() and pwrite(), or emulate them if unavailable.
The emulation is not perfect as the file position is changed, so
we'll put pg_ prefixes on the names to minimize the risk of confusion
in future patches that might inadvertently try to mix pread() and read()
on the same file descriptor.

Author: Thomas Munro
Reviewed-by: Tom Lane, Jesper Pedersen
Discussion: https://postgr.es/m/CAEepm=02rapCpPR3ZGF2vW=SBHSdFYO_bz_f-wwWJonmA3APgw@mail.gmail.com

6 years agoGUC: adjust effective_cache_size SQL descriptions
Bruce Momjian [Tue, 6 Nov 2018 18:40:03 +0000 (13:40 -0500)]
GUC: adjust effective_cache_size SQL descriptions

Follow on patch for commit 3e0f1a4741f564c1a2fa6e944729d6967355d8c7.

Reported-by: Peter Eisentraut
Discussion: https://postgr.es/m/369ec766-b947-51bd-4dad-6fb9e026439f@2ndquadrant.com

Backpatch-through: 9.4

6 years agoRename rbtree.c functions to use "rbt" prefix not "rb" prefix.
Tom Lane [Tue, 6 Nov 2018 18:25:24 +0000 (13:25 -0500)]
Rename rbtree.c functions to use "rbt" prefix not "rb" prefix.

The "rb" prefix is used by Ruby, so that our existing code results
in name collisions that break plruby.  We discussed ways to prevent
that by adjusting dynamic linker options, but it seems that at best
we'd move the pain to other cases.  Renaming to avoid the collision
is the only portable fix anyway.  Fortunately, our rbtree code is
not (yet?) widely used --- in core, there's only a single usage
in GIN --- so it seems likely that we can get away with a rename.

I chose to do this basically as s/rb/rbt/g, except for places where
there already was a "t" after "rb".  The patch could have been made
smaller by only touching linker-visible symbols, but it would have
resulted in oddly inconsistent-looking code.  Better to make it look
like "rbt" was the plan all along.

Back-patch to v10.  The rbtree.c code exists back to 9.5, but
rb_iterate() which is the actual immediate source of pain was added
in v10, so it seems like changing the names before that would have
more risk than benefit.

Per report from Pavel Raiskup.

Discussion: https://postgr.es/m/4738198.8KVIIDhgEB@nb.usersys.redhat.com

6 years agoRemove useless symbol from Makefile.global.
Tom Lane [Tue, 6 Nov 2018 15:57:51 +0000 (10:57 -0500)]
Remove useless symbol from Makefile.global.

I added HAVE_IPV6 to Makefile.global way back in commit 7703e55c3
so that we could transmit its value to the shell-script version of
initdb.  Since initdb was rewritten in C, it's been finding that
out from pg_config.h instead, so this is useless.  Keeping it here
just wastes configure and make cycles, plus it's a potential
two-sources-of-truth problem.

6 years agoRemove some remaining traces of dsm_resize().
Thomas Munro [Tue, 6 Nov 2018 08:20:58 +0000 (21:20 +1300)]
Remove some remaining traces of dsm_resize().

A couple of obsolete comments and unreachable blocks remained after
commit 3c60d0fa.

Discussion: https://postgr.es/m/CAA4eK1%2B%3DyAFUvpFoHXFi_gm8YqmXN-TtkFH%2BVYjvDLS6-SFq-Q%40mail.gmail.com

6 years agoReorganize format options of psql in alphabetical order
Michael Paquier [Tue, 6 Nov 2018 06:04:40 +0000 (15:04 +0900)]
Reorganize format options of psql in alphabetical order

This makes the addition of new formats easier, and documentation lookups
easier.

Author: Daniel Vérité
Reviewed-by: Fabien Coelho
Discussion: https://postgr.es/m/alpine.DEB.2.20.1803081004241.2916@lancre

6 years agoSwitch pg_promote to be parallel-safe
Michael Paquier [Tue, 6 Nov 2018 05:11:21 +0000 (14:11 +0900)]
Switch pg_promote to be parallel-safe

pg_promote uses nothing relying on a global state, so it is fine to mark
it as parallel-safe, conclusion based on a detailed analysis from Robert
Haas.  This also fixes an inconsistency where pg_proc.dat missed to mark
the function with its previous value for proparallel, update which does
not matter now as the default is used.

Based on a discussion between multiple folks: Laurenz Albe, Robert Haas,
Amit Kapila, Tom Lane and myself.

Discussion: https://postgr.es/m/20181029082530.GL14242@paquier.xyz

6 years agoRemove dsm_resize() and dsm_remap().
Thomas Munro [Tue, 6 Nov 2018 03:11:12 +0000 (16:11 +1300)]
Remove dsm_resize() and dsm_remap().

These interfaces were never used in core, didn't handle failure of
posix_fallocate() correctly and weren't supported on all platforms.
We agreed to remove them in 12.

Author: Thomas Munro
Reported-by: Andres Freund
Discussion: https://postgr.es/m/CAA4eK1%2B%3DyAFUvpFoHXFi_gm8YqmXN-TtkFH%2BVYjvDLS6-SFq-Q%40mail.gmail.com

6 years agoLast-minute updates for release notes.
Tom Lane [Mon, 5 Nov 2018 21:07:06 +0000 (16:07 -0500)]
Last-minute updates for release notes.

I removed the item about the pg_stat_statements change from
release-11.sgml, as part of a sweep to delete items already committed
in 11.0; but actually we'd best keep it to ensure that people who've
pg_upgraded their databases will take the requisite action.  Also make
said action more visible by making it into its own para.  Noted by
Jonathan Katz.

6 years agoFix copy-paste error in errhint() introduced in 691d79a07933.
Andres Freund [Mon, 5 Nov 2018 20:02:25 +0000 (12:02 -0800)]
Fix copy-paste error in errhint() introduced in 691d79a07933.

Reported-By: Petr Jelinek
Discussion: https://postgr.es/m/c95a620b-34f0-7930-aeb5-f7ab804f26cb@2ndquadrant.com
Backpatch: 9.4-, like the previous commit

6 years agoRemove unreferenced pg_opfamily entry.
Tom Lane [Mon, 5 Nov 2018 17:02:27 +0000 (12:02 -0500)]
Remove unreferenced pg_opfamily entry.

The entry with OID 4035, for GIST jsonb_ops, is unused; apparently
it was added in preparation for index support that never materialized.
Remove it, and add a regression test case to detect future mistakes
of the same kind.

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

6 years agoLast-minute updates for release notes.
Tom Lane [Mon, 5 Nov 2018 15:48:23 +0000 (10:48 -0500)]
Last-minute updates for release notes.

Security: CVE-2018-16850

6 years agoBlock creation of partitions with open references to its parent
Michael Paquier [Mon, 5 Nov 2018 02:04:02 +0000 (11:04 +0900)]
Block creation of partitions with open references to its parent

When a partition is created as part of a trigger processing, it is
possible that the partition which just gets created changes the
properties of the table the executor of the ongoing command relies on,
causing a subsequent crash.  This has been found possible when for
example using a BEFORE INSERT which creates a new partition for a
partitioned table being inserted to.

Any attempt to do so is blocked when working on a partition, with
regression tests added for both CREATE TABLE PARTITION OF and ALTER
TABLE ATTACH PARTITION.

Reported-by: Dmitry Shalashov
Author: Amit Langote
Reviewed-by: Michael Paquier, Tom Lane
Discussion: https://postgr.es/m/15437-3fe01ee66bd1bae1@postgresql.org
Backpatch-through: 10

6 years agoIgnore partitioned tables when processing ON COMMIT DELETE ROWS
Michael Paquier [Mon, 5 Nov 2018 00:14:33 +0000 (09:14 +0900)]
Ignore partitioned tables when processing ON COMMIT DELETE ROWS

Those tables have no physical storage, making this option unusable with
partition trees as at commit time an actual truncation was attempted.
There are still issues with the way ON COMMIT actions are done when
mixing several action types, however this impacts as well inheritance
trees, so this issue will be dealt with later.

Reported-by: Rajkumar Raghuwanshi
Author: Amit Langote
Reviewed-by: Michael Paquier, Tom Lane
Discussion: https://postgr.es/m/CAKcux6mhgcjSiB_egqEAEFgX462QZtncU8QCAJ2HZwM-wWGVew@mail.gmail.com

6 years agoRelease notes for 11.1, 10.6, 9.6.11, 9.5.15, 9.4.20, 9.3.25.
Tom Lane [Sun, 4 Nov 2018 21:57:14 +0000 (16:57 -0500)]
Release notes for 11.1, 10.6, 9.6.11, 9.5.15, 9.4.20, 9.3.25.

6 years agoFix ExecuteCallStmt to not scribble on the passed-in parse tree.
Tom Lane [Sun, 4 Nov 2018 19:50:55 +0000 (14:50 -0500)]
Fix ExecuteCallStmt to not scribble on the passed-in parse tree.

Modifying the parse tree at execution time is, or at least ought to be,
verboten.  It seems quite difficult to actually cause a crash this way
in v11 (although you can exhibit it pretty easily in HEAD by messing
with plan_cache_mode).  Nonetheless, it's risky, so fix and back-patch.

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

6 years agoFix bugs in plpgsql's handling of CALL argument lists.
Tom Lane [Sun, 4 Nov 2018 18:25:39 +0000 (13:25 -0500)]
Fix bugs in plpgsql's handling of CALL argument lists.

exec_stmt_call() tried to extract information out of a CALL statement's
argument list without using expand_function_arguments(), apparently in
the hope of saving a few nanoseconds by not processing defaulted
arguments.  It got that quite wrong though, leading to crashes with
named arguments, as well as failure to enforce writability of the
argument for a defaulted INOUT parameter.  Fix and simplify the logic
by using expand_function_arguments() before examining the list.

Also, move the argument-examination to just after producing the CALL
command's plan, before invoking the called procedure.  This ensures
that we'll track possible changes in the procedure's argument list
correctly, and avoids a hazard of the plan cache being flushed while
the procedure executes.

Also fix assorted falsehoods and omissions in associated documentation.

Per bug #15477 from Alexey Stepanov.

Patch by me, with some help from Pavel Stehule.  Back-patch to v11.

Discussion: https://postgr.es/m/15477-86075b1d1d319e0a@postgresql.org
Discussion: https://postgr.es/m/CAFj8pRA6UsujpTs9Sdwmk-R6yQykPx46wgjj+YZ7zxm4onrDyw@mail.gmail.com

6 years agoFix unused-variable warning.
Tom Lane [Sun, 4 Nov 2018 16:20:59 +0000 (11:20 -0500)]
Fix unused-variable warning.

Discussion: https://postgr.es/m/CAMkU=1xTHkS6d0iptCWykHc1Xrh3LBic_gZDo3JzDYru815fLQ@mail.gmail.com

6 years agoPrevent generating EEOP_AGG_STRICT_INPUT_CHECK operations when nargs == 0.
Andres Freund [Sat, 3 Nov 2018 22:55:23 +0000 (15:55 -0700)]
Prevent generating EEOP_AGG_STRICT_INPUT_CHECK operations when nargs == 0.

This only became a problem with 4c640f4f38, which didn't synchronize
the value agg_strict_input_check.nargs is set to, with the guard
condition for emitting the operation.

Besides such instructions being unnecessary overhead, currently the
LLVM JIT provider doesn't support them. It seems more sensible to
avoid generating such instruction than supporting them. Add assertions
to make it easier to debug a potential further occurance.

Discussion: https://postgr.es/m/2a505161-2727-2473-7c46-591ed108ac52@email.cz
Backpatch: 11-, like 4c640f4f38.

6 years agoFix STRICT check for strict aggregates with NULL ORDER BY columns.
Andres Freund [Sat, 3 Nov 2018 21:35:23 +0000 (14:35 -0700)]
Fix STRICT check for strict aggregates with NULL ORDER BY columns.

I (Andres) broke this unintentionally in 69c3936a14, by checking
strictness for all input expressions computed for an aggregate, rather
than just the input for the aggregate transition function.

Reported-By: Ondřej Bouda
Bisected-By: Tom Lane
Diagnosed-By: Andrew Gierth
Discussion: https://postgr.es/m/2a505161-2727-2473-7c46-591ed108ac52@email.cz
Backpatch: 11-, like 69c3936a14

6 years agoMake ts_locale.c's character-type functions cope with UTF-16.
Tom Lane [Sat, 3 Nov 2018 17:56:10 +0000 (13:56 -0400)]
Make ts_locale.c's character-type functions cope with UTF-16.

On Windows, in UTF8 database encoding, what char2wchar() produces is
UTF16 not UTF32, ie, characters above U+FFFF will be represented by
surrogate pairs.  t_isdigit() and siblings did not account for this
and failed to provide a large enough result buffer.  That in turn
led to bogus "invalid multibyte character for locale" errors, because
contrary to what you might think from char2wchar()'s documentation,
its Windows code path doesn't cope sanely with buffer overflow.

The solution for t_isdigit() and siblings is pretty clear: provide
a 3-wchar_t result buffer not 2.

char2wchar() also needs some work to provide more consistent, and more
accurately documented, buffer overrun behavior.  But that's a bigger job
and it doesn't actually have any immediate payoff, so leave it for later.

Per bug #15476 from Kenji Uno, who deserves credit for identifying the
cause of the problem.  Back-patch to all active branches.

Discussion: https://postgr.es/m/15476-4314f480acf0f114@postgresql.org

6 years agoFix tablespace handling for partitioned indexes
Alvaro Herrera [Sat, 3 Nov 2018 16:23:40 +0000 (13:23 -0300)]
Fix tablespace handling for partitioned indexes

When creating partitioned indexes, the tablespace was not being saved
for the parent index. This meant that subsequently created partitions
would not use the right tablespace for their indexes.

ALTER INDEX SET TABLESPACE and ALTER INDEX ALL IN TABLESPACE raised
errors when tried; fix them too.  This requires bespoke code for
ATExecCmd() that applies to the special case when the tablespace move is
just a catalog change.

Discussion: https://postgr.es/m/20181102003138.uxpaca6qfxzskepi@alvherre.pgsql

6 years agoRemove extra word from create sub docs
Stephen Frost [Sat, 3 Nov 2018 16:21:54 +0000 (12:21 -0400)]
Remove extra word from create sub docs

Improve the documentation in the CREATE SUBSCRIPTION command a bit by
removing an extraneous word and spelling out 'information'.

6 years agoFirst-draft release notes for 11.1.
Tom Lane [Sat, 3 Nov 2018 00:11:11 +0000 (20:11 -0400)]
First-draft release notes for 11.1.

As usual, the release notes for other branches will be made by cutting
these down, but put them up for community review first.  Note that a
fair percentage of the entries apply only to prior branches because
their issue was already fixed in 11.0.

6 years agoYet further rethinking of build changes for macOS Mojave.
Tom Lane [Fri, 2 Nov 2018 22:54:00 +0000 (18:54 -0400)]
Yet further rethinking of build changes for macOS Mojave.

The solution arrived at in commit e74dd00f5 presumes that the compiler
has a suitable default -isysroot setting ... but further experience
shows that in many combinations of macOS version, XCode version, Xcode
command line tools version, and phase of the moon, Apple's compiler
will *not* supply a default -isysroot value.

We could potentially go back to the approach used in commit 68fc227dd,
but I don't have a lot of faith in the reliability or life expectancy of
that either.  Let's just revert to the approach already shipped in 11.0,
namely specifying an -isysroot switch globally.  As a partial response to
the concerns raised by Jakob Egger, adjust the contents of Makefile.global
to look like

CPPFLAGS = -isysroot $(PG_SYSROOT) ...
PG_SYSROOT = /path/to/sysroot

This allows overriding the sysroot path at build time in a relatively
painless way.

Add documentation to installation.sgml about how to use the PG_SYSROOT
option.  I also took the opportunity to document how to work around
macOS's "System Integrity Protection" feature.

As before, back-patch to all supported versions.

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

6 years agoFix NULL handling in multi-batch Parallel Hash Left Join.
Thomas Munro [Fri, 2 Nov 2018 22:05:35 +0000 (11:05 +1300)]
Fix NULL handling in multi-batch Parallel Hash Left Join.

NULL keys in left joins were skipped when building batch files.
Repair, by making the keep_nulls argument to ExecHashGetHashValue()
depend on whether this is a left outer join, as we do in other
paths.

Bug #15475.  Thinko in 1804284042e.  Back-patch to 11.

Reported-by: Paul Schaap
Diagnosed-by: Andrew Gierth
Dicussion: https://postgr.es/m/15475-11a7a783fed72a36%40postgresql.org

6 years agodocs: adjust simpler language for NULL return from ANY/ALL
Bruce Momjian [Fri, 2 Nov 2018 17:05:30 +0000 (13:05 -0400)]
docs:  adjust simpler language for NULL return from ANY/ALL

Adjustment to commit 8610c973ddf1cbf0befc1369d2cf0d56c0efcd0a.

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

Backpatch-through: 9.3

6 years agoGUC: adjust effective_cache_size docs and SQL description
Bruce Momjian [Fri, 2 Nov 2018 13:11:00 +0000 (09:11 -0400)]
GUC:  adjust effective_cache_size docs and SQL description

Clarify that effective_cache_size is both kernel buffers and shared
buffers.

Reported-by: nat@makarevitch.org
Discussion: https://postgr.es/m/153685164808.22334.15432535018443165207@wrigleys.postgresql.org

Backpatch-through: 9.3

6 years agoFix some spelling errors in the documentation
Magnus Hagander [Fri, 2 Nov 2018 12:55:57 +0000 (13:55 +0100)]
Fix some spelling errors in the documentation

Author: Daniel Gustafsson <daniel@yesql.se>

6 years agoFix spelling errors and typos in comments
Magnus Hagander [Fri, 2 Nov 2018 12:56:16 +0000 (13:56 +0100)]
Fix spelling errors and typos in comments

Author: Daniel Gustafsson <daniel@yesql.se>

6 years agodoc: use simpler language for NULL return from ANY/ALL
Bruce Momjian [Fri, 2 Nov 2018 12:54:34 +0000 (08:54 -0400)]
doc:  use simpler language for NULL return from ANY/ALL

Previously the combination of "does not return" and "any row" caused
ambiguity.

Reported-by: KES <kes-kes@yandex.ru>
Discussion: https://postgr.es/m/153701242703.22334.1476830122267077397@wrigleys.postgresql.org

Reviewed-by: David G. Johnston
Backpatch-through: 9.3

6 years agoLower error level from PANIC to FATAL when restoring slots at startup
Michael Paquier [Thu, 1 Nov 2018 22:59:24 +0000 (07:59 +0900)]
Lower error level from PANIC to FATAL when restoring slots at startup

When restoring slot information from disk at startup and filling in
shared memory information, the startup process would issue a PANIC
message if more slots are found than what max_replication_slots allows,
and then Postgres generates a core dump, recommending to increase
max_replication_slots.  This gives users a switch to crash Postgres at
will by creating slots, lower the configuration to not support it, and
then restart it.

Making Postgres crash hard in this case is overdoing it just to give a
recommendation to users.  So instead use a FATAL, which makes Postgres
fail to start without crashing, still giving the recommendation.  This
is more consistent with what happens for prepared transactions for
example.

Author: Michael Paquier
Reviewed-by: Andres Freund
Discussion: https://postgr.es/m/20181030025109.GD1644@paquier.xyz

6 years agoRemove obsolete pg_constraint.consrc column
Peter Eisentraut [Sat, 27 Oct 2018 10:45:50 +0000 (11:45 +0100)]
Remove obsolete pg_constraint.consrc column

This has been deprecated and effectively unused for a long time.

Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
6 years agoRemove obsolete pg_attrdef.adsrc column
Peter Eisentraut [Sat, 27 Oct 2018 10:25:20 +0000 (11:25 +0100)]
Remove obsolete pg_attrdef.adsrc column

This has been deprecated and effectively unused for a long time.

Reviewed-by: Alvaro Herrera <alvherre@2ndquadrant.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
6 years agoFix error message typo introduced 691d79a07933.
Andres Freund [Thu, 1 Nov 2018 17:44:29 +0000 (10:44 -0700)]
Fix error message typo introduced 691d79a07933.

Reported-By: Michael Paquier
Discussion: https://postgr.es/m/20181101003405.GB1727@paquier.xyz
Backpatch: 9.4-, like the previous commit

6 years agoAdjust trace_sort log messages.
Peter Geoghegan [Thu, 1 Nov 2018 16:18:57 +0000 (09:18 -0700)]
Adjust trace_sort log messages.

The project message style guide dictates: "When citing the name of an
object, state what kind of object it is".  The parallel CREATE INDEX
patch added a worker number to most of the trace_sort messages within
tuplesort.c without specifying the object type.  Bring these messages
into compliance with the style guide.

We're still treating a leader or serial Tuplesortstate as having worker
number -1.  trace_sort is a developer option, and these two cases are
highly comparable, so this seems appropriate.

Per complaint from Tom Lane.

Discussion: https://postgr.es/m/8330.1540831863@sss.pgh.pa.us
Backpatch: 11-, where parallel CREATE INDEX was introduced.

6 years agoDisallow starting server with insufficient wal_level for existing slot.
Andres Freund [Wed, 31 Oct 2018 21:47:41 +0000 (14:47 -0700)]
Disallow starting server with insufficient wal_level for existing slot.

Previously it was possible to create a slot, change wal_level, and
restart, even if the new wal_level was insufficient for the
slot. That's a problem for both logical and physical slots, because
the necessary WAL records are not generated.

This removes a few tests in newer versions that, somewhat
inexplicably, whether restarting with a too low wal_level worked (a
buggy behaviour!).

Reported-By: Joshua D. Drake
Author: Andres Freund
Discussion: https://postgr.es/m/20181029191304.lbsmhshkyymhw22w@alap3.anarazel.de
Backpatch: 9.4-, where replication slots where introduced

6 years agoFix memory leak in repeated SPGIST index scans.
Tom Lane [Wed, 31 Oct 2018 21:04:42 +0000 (17:04 -0400)]
Fix memory leak in repeated SPGIST index scans.

spgendscan neglected to pfree all the memory allocated by spgbeginscan.
It's possible to get away with that in most normal queries, since the
memory is allocated in the executor's per-query context which is about
to get deleted anyway; but it causes severe memory leakage during
creation or filling of large exclusion-constraint indexes.

Also, document that amendscan is supposed to free what ambeginscan
allocates.  The docs' lack of clarity on that point probably caused this
bug to begin with.  (There is discussion of changing that API spec going
forward, but I don't think it'd be appropriate for the back branches.)

Per report from Bruno Wolff.  It's been like this since the beginning,
so back-patch to all active branches.

In HEAD, also fix an independent leak caused by commit 2a6368343
(allocating memory during spgrescan instead of spgbeginscan, which
might be all right if it got cleaned up, but it didn't).  And do a bit
of code beautification on that commit, too.

Discussion: https://postgr.es/m/20181024012314.GA27428@wolff.to

6 years agoFix typo in xlog.c.
Andres Freund [Wed, 31 Oct 2018 14:50:32 +0000 (07:50 -0700)]
Fix typo in xlog.c.

Author: Daniel Gustafsson
Discussion: https://postgr.es/m/A6817958-949E-4A5B-895D-FA421B6640C2@yesql.se

6 years agoSync our copy of the timezone library with IANA release tzcode2018g.
Tom Lane [Wed, 31 Oct 2018 13:47:53 +0000 (09:47 -0400)]
Sync our copy of the timezone library with IANA release tzcode2018g.

This patch absorbs an upstream fix to "zic" for a recently-introduced
bug that made it output data that some 32-bit clients couldn't read.
Given the current source data, the bug only manifests in zones with
leap seconds, which we don't generate, so that there's no actual
change in our installed timezone data files from this.  Still, in
case somebody uses our copy of "zic" to do something else, it seems
best to apply the fix promptly.

Also, update the README's notes about converting upstream code to
our conventions.

6 years agoUpdate time zone data files to tzdata release 2018g.
Tom Lane [Wed, 31 Oct 2018 12:35:50 +0000 (08:35 -0400)]
Update time zone data files to tzdata release 2018g.

DST law changes in Morocco (with, effectively, zero notice).
Historical corrections for Hawaii.

6 years agoFix interaction of CASE and ArrayCoerceExpr.
Tom Lane [Tue, 30 Oct 2018 19:26:11 +0000 (15:26 -0400)]
Fix interaction of CASE and ArrayCoerceExpr.

An array-type coercion appearing within a CASE that has a constant
(after const-folding) test expression was mangled by the planner, causing
all the elements of the resulting array to be equal to the coerced value
of the CASE's test expression.  This is my oversight in commit c12d570fa:
that changed ArrayCoerceExpr to use a subexpression involving a
CaseTestExpr, and I didn't notice that eval_const_expressions needed an
adjustment to keep from folding such a CaseTestExpr to a constant when
it's inside a suitable CASE.

This is another in what's getting to be a depressingly long line of bugs
associated with misidentification of the referent of a CaseTestExpr.
We're overdue to redesign that mechanism; but any such fix is unlikely
to be back-patchable into v11.  As a stopgap, fix eval_const_expressions
to do what it must here.  Also add a bunch of comments pointing out the
restrictions and assumptions that are needed to make this work at all.

Also fix a related oversight: contain_context_dependent_node() was not
aware of the relationship of ArrayCoerceExpr to CaseTestExpr.  That was
somewhat fail-soft, in that the outcome of a wrong answer would be to
prevent optimizations that could have been made, but let's fix it while
we're at it.

Per bug #15471 from Matt Williams.  Back-patch to v11 where the faulty
logic came in.

Discussion: https://postgr.es/m/15471-1117f49271989bad@postgresql.org

6 years agopg_rewind: Remove unused macro
Peter Eisentraut [Tue, 30 Oct 2018 12:22:11 +0000 (13:22 +0100)]
pg_rewind: Remove unused macro

This has never been used while pg_rewind was in the tree (possibly
once copied from pg_upgrade).

6 years agoConsolidate cross-option checks in pg_restore
Michael Paquier [Tue, 30 Oct 2018 02:38:35 +0000 (11:38 +0900)]
Consolidate cross-option checks in pg_restore

This moves one check for conflicting options from the archive restore
code to the main function where other similar checks are performed.
Also reword the error message to be consistent with other messages.

The only option combination impacted is --create specified with
--single-transaction, and informing the caller at an early step saves
from opening the archive worked on.  A TAP test is added for this
combination.

Author: Daniel Gustafsson
Reviewed-by: Fabien Coelho
Discussion: https://postgr.es/m/616808BD-4B59-4E6C-97A9-7317F62D5570@yesql.se

6 years agoAdd pg_partition_tree to display information about partitions
Michael Paquier [Tue, 30 Oct 2018 01:25:06 +0000 (10:25 +0900)]
Add pg_partition_tree to display information about partitions

This new function is useful to display a full tree of partitions with a
partitioned table given in output, and avoids the need of any complex
WITH RECURSIVE query when looking at partition trees which are
deep multiple levels.

It returns a set of records, one for each partition, containing the
partition's name, its immediate parent's name, a boolean value telling
if the relation is a leaf in the tree and an integer telling its level
in the partition tree with given table considered as root, beginning at
zero for the root, and incrementing by one each time the scan goes one
level down.

Author: Amit Langote
Reviewed-by: Jesper Pedersen, Michael Paquier, Robert Haas
Discussion: https://postgr.es/m/8d00e51a-9a51-ad02-d53e-ba6bf50b2e52@lab.ntt.co.jp

6 years agoFix missing whitespace in pg_dump ref page
Magnus Hagander [Mon, 29 Oct 2018 11:34:49 +0000 (12:34 +0100)]
Fix missing whitespace in pg_dump ref page

Author: Daniel Gustafsson <daniel@yesql.se>

6 years agoExclude temporary directories from pgindent
Peter Eisentraut [Mon, 29 Oct 2018 10:39:44 +0000 (11:39 +0100)]
Exclude temporary directories from pgindent

Exclude tmp_check and tmp_install from pgindent.  In a fully-built
tree, pgindent would spend a lot of time digging through these
directories and ends up re-indenting installed header files.

6 years agopg_restore: Augment documentation for -N option
Peter Eisentraut [Mon, 29 Oct 2018 10:31:43 +0000 (11:31 +0100)]
pg_restore: Augment documentation for -N option

This was forgotten when the option was added.

Author: Michael Banck <michael.banck@credativ.de>

6 years agoImprove description of pg_attrdef in documentation
Michael Paquier [Mon, 29 Oct 2018 07:38:54 +0000 (16:38 +0900)]
Improve description of pg_attrdef in documentation

The reference to pg_attribute is switched to a link, which is more
useful for the html documentation.  The conditions under which a default
value is defined for a given column are made more general.

Author: Daniel Gustafsson
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/0E8748E3-8B7D-445E-9ABA-09DA5C7345CC@yesql.se