]> granicus.if.org Git - postgresql/log
postgresql
7 years agoFix possible crash with Parallel Bitmap Heap Scan.
Robert Haas [Fri, 13 Oct 2017 18:53:28 +0000 (14:53 -0400)]
Fix possible crash with Parallel Bitmap Heap Scan.

If a Parallel Bitmap Heap scan's chain of leftmost descendents
includes a BitmapOr whose first child is a BitmapAnd, the prior coding
would mistakenly create a non-shared TIDBitmap and then try to perform
shared iteration.

Report by Tomas Vondra.  Patch by Dilip Kumar.

Discussion: http://postgr.es/m/50e89684-8ad9-dead-8767-c9545bafd3b6@2ndquadrant.com

7 years agoImprove implementation of CRE-stack-flattening in map_variable_attnos().
Tom Lane [Fri, 13 Oct 2017 17:43:55 +0000 (13:43 -0400)]
Improve implementation of CRE-stack-flattening in map_variable_attnos().

I (tgl) objected to the obscure implementation introduced in commit
1c497fa72.  This one seems a bit less action-at-a-distance-y, at the
price of repeating a few lines of code.

Improve the comments about what the function is doing, too.

Amit Khandekar, whacked around a bit more by me

Discussion: https://postgr.es/m/CAJ3gD9egYTyHUH0nTMxm8-1m3RvdqEbaTyGC-CUNtYf7tKNDaQ@mail.gmail.com

7 years agoRely on sizeof(typename) rather than sizeof(variable) in pqformat.h.
Tom Lane [Fri, 13 Oct 2017 15:46:05 +0000 (11:46 -0400)]
Rely on sizeof(typename) rather than sizeof(variable) in pqformat.h.

In each of the pq_writeintN functions, the three uses of sizeof() should
surely all be consistent.  I started out to make them all sizeof(ni),
but on reflection let's make them sizeof(typename) instead.  That's more
like our usual style elsewhere, and it's just barely possible that the
failures buildfarm member hornet has shown since 4c119fbcd went in are
caused by the compiler getting confused about sizeof() a parameter that
it's optimizing away.

In passing, improve a couple of comments.

Discussion: https://postgr.es/m/E1e2RML-0002do-Lc@gemulon.postgresql.org

7 years agoAttempt to fix LDAP build
Peter Eisentraut [Fri, 13 Oct 2017 03:47:48 +0000 (23:47 -0400)]
Attempt to fix LDAP build

Apparently, an older spelling of LDAP_OPT_DIAGNOSTIC_MESSAGE is
LDAP_OPT_ERROR_STRING, so fall back to that one.

7 years agoLog diagnostic messages if errors occur during LDAP auth.
Peter Eisentraut [Fri, 13 Oct 2017 02:33:34 +0000 (22:33 -0400)]
Log diagnostic messages if errors occur during LDAP auth.

Diagnostic messages seem likely to help users diagnose root
causes more easily, so let's report them as errdetail.

Author: Thomas Munro
Reviewed-By: Ashutosh Bapat, Christoph Berg, Alvaro Herrera, Peter Eisentraut
Discussion: https://postgr.es/m/CAEepm=2_dA-SYpFdmNVwvKsEBXOUj=K4ooKovHmvj6jnMdt8dw@mail.gmail.com

7 years agoImprove LDAP cleanup code in error paths.
Peter Eisentraut [Fri, 13 Oct 2017 02:33:15 +0000 (22:33 -0400)]
Improve LDAP cleanup code in error paths.

After calling ldap_unbind_s() we probably shouldn't try to use the LDAP
connection again to call ldap_get_option(), even if it failed.  The OpenLDAP
man page for ldap_unbind[_s] says "Once it is called, the connection to the
LDAP server is closed, and the ld structure is invalid."  Otherwise, as a
general rule we should probably call ldap_unbind() before returning in all
paths to avoid leaking resources.  It is unlikely there is any practical
leak problem since failure to authenticate currently results in the backend
exiting soon afterwards.

Author: Thomas Munro
Reviewed-By: Alvaro Herrera, Peter Eisentraut
Discussion: https://postgr.es/m/20170914141205.eup4kxzlkagtmfac%40alvherre.pgsql

7 years agoUse C99 restrict via pg_restrict, rather than restrict directly.
Andres Freund [Thu, 12 Oct 2017 22:25:38 +0000 (15:25 -0700)]
Use C99 restrict via pg_restrict, rather than restrict directly.

Unfortunately using 'restrict' plainly causes problems with MSVC,
which supports restrict only as '__restrict'. Defining 'restrict' to
'__restrict' unfortunately causes a conflict with MSVC's usage of
__declspec(restrict) in headers.

Therefore define pg_restrict to the appropriate keyword instead, and
replace existing usages.

This replaces the temporary workaround introduced in 36b4b91ba078.

Author: Andres Freund
Discussion: https://postgr.es/m/2656.1507830907@sss.pgh.pa.us

7 years agoAvoid coercing a whole-row variable that is already coerced.
Robert Haas [Thu, 12 Oct 2017 21:10:48 +0000 (17:10 -0400)]
Avoid coercing a whole-row variable that is already coerced.

Marginal efficiency and beautification hack.  I'm not sure whether
this case ever arises currently, but the pending patch for update
tuple routing will cause it to arise.

Amit Khandekar

Discussion: http://postgr.es/m/CAJ3gD9cazfppe7-wwUbabPcQ4_0SubkiPFD1+0r5_DkVNWo5yg@mail.gmail.com

7 years agoUse ResultRelInfo ** rather than ResultRelInfo * for tuple routing.
Robert Haas [Thu, 12 Oct 2017 20:50:53 +0000 (16:50 -0400)]
Use ResultRelInfo ** rather than ResultRelInfo * for tuple routing.

The previous convention doesn't lend itself to creating ResultRelInfos
lazily, as we already do in ExecGetTriggerResultRel.  This patch
doesn't make anything lazier than before, but the pending patch for
UPDATE tuple routing proposes to do so (and there might be other
opportunities as well).

Amit Khandekar with some adjustments by me.

Discussion: http://postgr.es/m/CA+TgmoYPVP9Lyf6vUFA5DwxS4c--x6LOj2y36BsJaYtp62eXPQ@mail.gmail.com

7 years agoFix AggGetAggref() so it won't lie to aggregate final functions.
Tom Lane [Thu, 12 Oct 2017 19:20:04 +0000 (15:20 -0400)]
Fix AggGetAggref() so it won't lie to aggregate final functions.

If we merge the transition calculations for two different aggregates,
it's reasonable to assume that the transition function should not care
which of those Aggref structs it gets from AggGetAggref().  It is not
reasonable to make the same assumption about an aggregate final function,
however.  Commit 804163bc2 broke this, as it will pass whichever Aggref
was first associated with the transition state in both cases.

This doesn't create an observable bug so far as the core system is
concerned, because the only existing uses of AggGetAggref() are in
ordered-set aggregates that happen to not pay attention to anything
but the input properties of the Aggref; and besides that, we disabled
sharing of transition calculations for OSAs yesterday.  Nonetheless,
if some third-party code were using AggGetAggref() in a normal aggregate,
they would be entitled to call this a bug.  Hence, back-patch the fix
to 9.6 where the problem was introduced.

In passing, improve some of the comments about transition state sharing.

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

7 years agoSynchronize error messages.
Robert Haas [Thu, 12 Oct 2017 19:14:22 +0000 (15:14 -0400)]
Synchronize error messages.

Commits 6476b26115f3ef25a9cd87880e0ac5ec5f7a05f6
and 14f67a8ee282ebc0de78e773fbd597f460ab4a54 didn't use quite the
same error message for what is basically the same situation.

Amit Langote, pared back a bit by me.

Discussion: http://postgr.es/m/54dc76d0-3b5b-ba5a-27dc-fb31a3975b61@lab.ntt.co.jp

7 years agoDoc: fix typo in release notes.
Tom Lane [Thu, 12 Oct 2017 15:35:54 +0000 (11:35 -0400)]
Doc: fix typo in release notes.

Ioseph Kim

Discussion: https://postgr.es/m/e7a79f91-8244-5bcb-afcc-96c817e86f4e@postgresql.kr

7 years agoInfer functional dependency past RelabelType
Alvaro Herrera [Thu, 12 Oct 2017 15:23:47 +0000 (17:23 +0200)]
Infer functional dependency past RelabelType

Vars hidden within a RelabelType would not be detected as compatible
with some functional dependency.  Repair by properly ignoring the
RelabelType.

Author: David Rowley
Reviewed-by: Tomas Vondra
Discussion: https://postgr.es/m/CAKJS1f-y-UEy=rsBXynBOgiW1fKMr_LVoYSGL9QOc36mLEC-ww@mail.gmail.com

7 years agoFix logical replication to fire BEFORE ROW DELETE triggers.
Robert Haas [Thu, 12 Oct 2017 14:09:26 +0000 (10:09 -0400)]
Fix logical replication to fire BEFORE ROW DELETE triggers.

Before, that would fail to happen unless a BEFORE ROW UPDATE trigger
was also present.

Noted by me while reviewing a patch from Masahiko Sawada, who also
wrote this patch.  Reviewed by Petr Jelinek.

Discussion: http://postgr.es/m/CA+TgmobAZvCxduG8y_mQKBK7nz-vhbdLvjM354KEFozpuzMN5A@mail.gmail.com

7 years agoReplace remaining uses of pq_sendint with pq_sendint{8,16,32}.
Andres Freund [Thu, 12 Oct 2017 04:00:46 +0000 (21:00 -0700)]
Replace remaining uses of pq_sendint with pq_sendint{8,16,32}.

pq_sendint() remains, so extension code doesn't unnecessarily break.

Author: Andres Freund
Discussion: https://postgr.es/m/20170914063418.sckdzgjfrsbekae4@alap3.anarazel.de

7 years agoPrevent sharing transition states between ordered-set aggregates.
Tom Lane [Thu, 12 Oct 2017 02:18:01 +0000 (22:18 -0400)]
Prevent sharing transition states between ordered-set aggregates.

This ought to work, but the built-in OSAs are not capable of coping,
because their final-functions destructively modify their transition
state (specifically, the contained tuplesort object).  That was fine
when those functions were written, but commit 804163bc2 moved the
goalposts without telling orderedsetaggs.c.

We should fix the built-in OSAs to support this, but it will take
a little work, especially if we don't want to sacrifice performance
in the normal non-shared-state case.  Given that it took a year after
9.6 release for anyone to notice this bug, we should not prioritize
sharable-state over nonsharable-state performance.  And a proper fix
is likely to be more complicated than we'd want to back-patch, too.

Therefore, let's just put in this stop-gap patch to prevent nodeAgg.c
from choosing to use shared state for OSAs.  We can revert it in HEAD
when we get a better fix.

Report from Lukas Eder, diagnosis by me, patch by David Rowley.
Back-patch to 9.6 where the problem was introduced.

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

7 years agoTemporary attempt at a workaround for further MSVC restrict build failures.
Andres Freund [Thu, 12 Oct 2017 02:06:29 +0000 (19:06 -0700)]
Temporary attempt at a workaround for further MSVC restrict build failures.

It appears some versions of msvc use __declspec(restrict) in stdlib.h
and subsidiary headers. Including those after defining 'restrict' to
'__restrict' doesn't work.  Try to get the buildfarm green to see
whether there's further problems, by including stdlib.h just before
said define.

7 years agoWork around overly strict restrict checks by MSVC.
Andres Freund [Thu, 12 Oct 2017 00:16:16 +0000 (17:16 -0700)]
Work around overly strict restrict checks by MSVC.

Apparently MSVC requires a * before a restrict in a variable
declaration, even if the adorned type already is a pointer, just via
typedef.

As reported by buildfarm animal woodlouse.

Author: Andres Freund
Discussion: https://postgr.es/m/20171012001320.4putagiruuehtvb6@alap3.anarazel.de

7 years agoImprove performance of SendRowDescriptionMessage.
Andres Freund [Wed, 11 Oct 2017 23:49:31 +0000 (16:49 -0700)]
Improve performance of SendRowDescriptionMessage.

There's three categories of changes leading to better performance:
- Splitting the per-attribute part of SendRowDescriptionMessage into a
  v2 and a v3 version allows avoiding branches for every attribute.
- Preallocating the size of the buffer to be big enough for all
  attributes and then using pq_write* avoids unnecessary buffer
  size checks & resizing.
- Reusing a persistently allocated StringInfo for all
  SendRowDescriptionMessage() invocations avoids repeated allocations
  & reallocations.

Author: Andres Freund
Discussion: https://postgr.es/m/20170914063418.sckdzgjfrsbekae4@alap3.anarazel.de

7 years agopg_stat_statements: Widen query IDs from 32 bits to 64 bits.
Robert Haas [Wed, 11 Oct 2017 23:52:46 +0000 (19:52 -0400)]
pg_stat_statements: Widen query IDs from 32 bits to 64 bits.

This takes advantage of the infrastructure introduced by commit
81c5e46c490e2426db243eada186995da5bb0ba7 to greatly reduce the
likelihood that two different queries will end up with the same query
ID.  It's still possible, of course, but whereas before it the chances
of a collision reached 25% around 50,000 queries, it will now take
more than 3 billion queries.

Backward incompatibility: Because the type exposed at the SQL level is
int8, users may now see negative query IDs in the pg_stat_statements
view (and also, query IDs more than 4 billion, which was the old
limit).

Patch by me, reviewed by Michael Paquier and Peter Geoghegan.

Discussion: http://postgr.es/m/CA+TgmobG_Kp4cBKFmsznUAaM1GWW6hhRNiZC0KjRMOOeYnz5Yw@mail.gmail.com

7 years agoUse one stringbuffer for all rows printed in printtup.c.
Andres Freund [Wed, 11 Oct 2017 23:26:35 +0000 (16:26 -0700)]
Use one stringbuffer for all rows printed in printtup.c.

This avoids newly allocating, and then possibly growing, the
stringbuffer for every row. For wide rows this can substantially
reduce memory allocator overhead, at the price of not immediately
reducing memory usage after outputting an especially wide row.

Author: Andres Freund
Discussion: https://postgr.es/m/20170914063418.sckdzgjfrsbekae4@alap3.anarazel.de

7 years agoAdd more efficient functions to pqformat API.
Andres Freund [Wed, 11 Oct 2017 23:01:52 +0000 (16:01 -0700)]
Add more efficient functions to pqformat API.

There's three prongs to achieve greater efficiency here:

1) Allow reusing a stringbuffer across pq_beginmessage/endmessage,
   with the new pq_beginmessage_reuse/endmessage_reuse. This can be
   beneficial both because it avoids allocating the initial buffer,
   and because it's more likely to already have an correctly sized
   buffer.

2) Replacing pq_sendint() with pq_sendint$width() inline
   functions. Previously unnecessary and unpredictable branches in
   pq_sendint() were needed. Additionally the replacement functions
   are implemented more efficiently.  pq_sendint is now deprecated, a
   separate commit will convert all in-tree callers.

3) Add pq_writeint$width(), pq_writestring(). These rely on sufficient
   space in the StringInfo's buffer, avoiding individual space checks
   & potential individual resizing.  To allow this to be used for
   strings, expose mbutil.c's MAX_CONVERSION_GROWTH.

Followup commits will make use of these facilities.

Author: Andres Freund
Discussion: https://postgr.es/m/20170914063418.sckdzgjfrsbekae4@alap3.anarazel.de

7 years agoAllow to avoid NUL-byte management for stringinfos and use in format.c.
Andres Freund [Wed, 11 Oct 2017 23:01:52 +0000 (16:01 -0700)]
Allow to avoid NUL-byte management for stringinfos and use in format.c.

In a lot of the places having appendBinaryStringInfo() maintain a
trailing NUL byte wasn't actually meaningful, e.g. when appending an
integer which can contain 0 in one of its bytes.

Removing this yields some small speedup, but more importantly will be
more consistent when providing faster variants of pq_sendint etc.

Author: Andres Freund
Discussion: https://postgr.es/m/20170914063418.sckdzgjfrsbekae4@alap3.anarazel.de

7 years agoAdd configure infrastructure to detect support for C99's restrict.
Andres Freund [Wed, 11 Oct 2017 23:01:52 +0000 (16:01 -0700)]
Add configure infrastructure to detect support for C99's restrict.

Will be used in later commits improving performance for a few key
routines where information about aliasing allows for significantly
better code generation.

This allows to use the C99 'restrict' keyword without breaking C89, or
for that matter C++, compilers. If not supported it's defined to be
empty.

Author: Andres Freund
Discussion: https://postgr.es/m/20170914063418.sckdzgjfrsbekae4@alap3.anarazel.de

7 years agoRemove unnecessary PG_TRY overhead for CurrentResourceOwner changes.
Tom Lane [Wed, 11 Oct 2017 21:43:50 +0000 (17:43 -0400)]
Remove unnecessary PG_TRY overhead for CurrentResourceOwner changes.

resowner/README contained advice to use a PG_TRY block to restore the
old CurrentResourceOwner value anywhere that that variable is transiently
changed.  That advice was only inconsistently followed, however, and
on reflection it seems like unnecessary overhead.  We don't bother
with such a convention for transient CurrentMemoryContext changes,
on the grounds that any (sub)transaction abort will start out by
resetting CurrentMemoryContext to what it wants.  But the same is
true of CurrentResourceOwner, so there seems no need to treat it
differently.

Hence, remove PG_TRY blocks that exist only to restore CurrentResourceOwner
before re-throwing the error.  There are a couple of places that restore
it along with some other actions, and I left those alone; the restore is
probably unnecessary but no noticeable gain will result from removing it.

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

7 years agoPrevent idle in transaction session timeout from sometimes being ignored.
Andres Freund [Wed, 11 Oct 2017 19:03:26 +0000 (12:03 -0700)]
Prevent idle in transaction session timeout from sometimes being ignored.

The previous coding in ProcessInterrupts() could lead to
idle_in_transaction_session_timeout being ignored, when
statement_timeout occurred earlier.

The problem was that ProcessInterrupts() would return before
processing the transaction timeout if QueryCancelPending was set while
QueryCancelHoldoffCount != 0 - which is the case when reading new
commands from the client. Ergo when the idle transaction timeout would
hit.

Fix that by removing the early return. Alternatively the transaction
timeout code could have been moved up, but that early return seems
like an issue that could hit other cases too.

Author: Lukas Fittl
Bug: #14821
Discussion:
    https://www.postgresql.org/message-id/20170921010956.17345.61461%40wrigleys.postgresql.org
    https://www.postgresql.org/message-id/CAP53PkxQnv3OWJpyNPGJYT62uY=n1=2CF_Lpc6gVOFnc0-gazw@mail.gmail.com
Backpatch: 9.6-, where idle_in_transaction_session_timeout was introduced.

7 years agoDoc: fix missing explanation of default object privileges.
Tom Lane [Wed, 11 Oct 2017 20:56:23 +0000 (16:56 -0400)]
Doc: fix missing explanation of default object privileges.

The GRANT reference page, which lists the default privileges for new
objects, failed to mention that USAGE is granted by default for data
types and domains.  As a lesser sin, it also did not specify anything
about the initial privileges for sequences, FDWs, foreign servers,
or large objects.  Fix that, and add a comment to acldefault() in the
probably vain hope of getting people to maintain this list in future.

Noted by Laurenz Albe, though I editorialized on the wording a bit.
Back-patch to all supported branches, since they all have this behavior.

Discussion: https://postgr.es/m/1507620895.4152.1.camel@cybertec.at

7 years agoFix mistakes in comments.
Robert Haas [Wed, 11 Oct 2017 19:55:10 +0000 (15:55 -0400)]
Fix mistakes in comments.

Masahiko Sawada

Discussion: http://postgr.es/m/CAD21AoBsfYsMHD6_SL9iN3n_Foaa+oPbL5jG55DxU1ChaujqwQ@mail.gmail.com

7 years agoFix low-probability loss of NOTIFY messages due to XID wraparound.
Tom Lane [Wed, 11 Oct 2017 18:28:33 +0000 (14:28 -0400)]
Fix low-probability loss of NOTIFY messages due to XID wraparound.

Up to now async.c has used TransactionIdIsInProgress() to detect whether
a notify message's source transaction is still running.  However, that
function has a quick-exit path that reports that XIDs before RecentXmin
are no longer running.  If a listening backend is doing nothing but
listening, and not running any queries, there is nothing that will advance
its value of RecentXmin.  Once 2 billion transactions elapse, the
RecentXmin check causes active transactions to be reported as not running.
If they aren't committed yet according to CLOG, async.c decides they
aborted and discards their messages.  The timing for that is a bit tight
but it can happen when multiple backends are sending notifies concurrently.
The net symptom therefore is that a sufficiently-long-surviving
listen-only backend starts to miss some fraction of NOTIFY traffic,
but only under heavy load.

The only function that updates RecentXmin is GetSnapshotData().
A brute-force fix would therefore be to take a snapshot before
processing incoming notify messages.  But that would add cycles,
as well as contention for the ProcArrayLock.  We can be smarter:
having taken the snapshot, let's use that to check for running
XIDs, and not call TransactionIdIsInProgress() at all.  In this
way we reduce the number of ProcArrayLock acquisitions from one
per message to one per notify interrupt; that's the same under
light load but should be a benefit under heavy load.  Light testing
says that this change is a wash performance-wise for normal loads.

I looked around for other callers of TransactionIdIsInProgress()
that might be at similar risk, and didn't find any; all of them
are inside transactions that presumably have already taken a
snapshot.

Problem report and diagnosis by Marko Tiikkaja, patch by me.
Back-patch to all supported branches, since it's been like this
since 9.0.

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

7 years agoAdd port/strnlen support to libpq and ecpg Makefiles.
Tom Lane [Wed, 11 Oct 2017 15:27:57 +0000 (11:27 -0400)]
Add port/strnlen support to libpq and ecpg Makefiles.

In the wake of fffd651e8, any makefile that pulls in snprintf.c
from src/port/ needs to be prepared to pull in strnlen.c as well.
Per buildfarm.

7 years agoFix whitespace
Peter Eisentraut [Wed, 11 Oct 2017 13:15:20 +0000 (09:15 -0400)]
Fix whitespace

7 years agoRegenerate configure script.
Tom Lane [Tue, 10 Oct 2017 23:14:06 +0000 (19:14 -0400)]
Regenerate configure script.

Not sure how fffd651e83ccbd6191a76be6ec7c6b1b27888fde ended up
probing for "strnlenfrak" rather than "strnlen".
My autoconf doesn't do that ...

7 years agoRewrite strnlen replacement implementation from 8a241792f96.
Andres Freund [Tue, 10 Oct 2017 21:42:16 +0000 (14:42 -0700)]
Rewrite strnlen replacement implementation from 8a241792f96.

The previous placement of the fallback implementation in libpgcommon
was problematic, because libpqport functions need strnlen
functionality.

Move replacement into libpgport. Provide strnlen() under its posix
name, instead of pg_strnlen(). Fix stupid configure bug, executing the
test only when compiled with threading support.

Author: Andres Freund
Discussion: https://postgr.es/m/E1e1gR2-0005fB-SI@gemulon.postgresql.org

7 years agoAdd missing clean step to src/test/modules/brin/Makefile.
Tom Lane [Tue, 10 Oct 2017 16:51:09 +0000 (12:51 -0400)]
Add missing clean step to src/test/modules/brin/Makefile.

I noticed the tmp_check subdirectory wasn't getting cleaned up
after a check-world run.  Apparently pgxs.mk will only do this
for you if you've defined REGRESS.  The only other src/test/modules
Makefile that does not set that is snapshot_too_old, and it
does it like this.

7 years agoUse lower-case SGML attribute values
Peter Eisentraut [Mon, 9 Oct 2017 02:00:57 +0000 (22:00 -0400)]
Use lower-case SGML attribute values

for DocBook XML compatibility

7 years agoFix pnstrdup() to not memcpy() the maximum allowed length.
Andres Freund [Mon, 9 Oct 2017 22:20:42 +0000 (15:20 -0700)]
Fix pnstrdup() to not memcpy() the maximum allowed length.

The previous behaviour was dangerous if the length passed wasn't the
size of the underlying buffer, but the maximum size of the underlying
buffer.

Author: Andres Freund
Discussion: https://postgr.es/m/20161003215524.mwz5p45pcverrkyk@alap3.anarazel.de

7 years agoAdd pg_strnlen() a portable implementation of strlen.
Andres Freund [Mon, 9 Oct 2017 22:20:42 +0000 (15:20 -0700)]
Add pg_strnlen() a portable implementation of strlen.

As the OS version is likely going to be more optimized, fall back to
it if available, as detected by configure.

7 years agoRemove unused documentation file
Peter Eisentraut [Mon, 9 Oct 2017 01:51:58 +0000 (21:51 -0400)]
Remove unused documentation file

7 years agoReduce memory usage of targetlist SRFs.
Andres Freund [Sun, 8 Oct 2017 22:08:25 +0000 (15:08 -0700)]
Reduce memory usage of targetlist SRFs.

Previously nodeProjectSet only released memory once per input tuple,
rather than once per returned tuple. If the computation of an
individual returned tuple requires a lot of memory, that can lead to
problems.

Instead change things so that the expression context can be reset once
per output tuple, which requires a new memory context to store SRF
arguments in.

This is a longstanding issue, but was hard to fix before 9.6, due to
the way tSRFs where evaluated. But it's fairly easy to fix now. We
could backpatch this into 10, but given there've been fewc omplaints
that doesn't seem worth the risk so far.

Reported-By: Lucas Fairchild
Author: Andres Freund, per discussion with Tom Lane
Discussion: https://postgr.es/m/4514.1507318623@sss.pgh.pa.us

7 years agoIncrease distance between flush requests during bulk file copies.
Tom Lane [Sun, 8 Oct 2017 19:25:26 +0000 (15:25 -0400)]
Increase distance between flush requests during bulk file copies.

copy_file() reads and writes data 64KB at a time (with default BLCKSZ),
and historically has issued a pg_flush_data request after each write.
This turns out to interact really badly with macOS's new APFS file
system: a large file copy takes over 100X longer than it ought to on
APFS, as reported by Brent Dearth.  While that's arguably a macOS bug,
it's not clear whether Apple will do anything about it in the near
future, and in any case experimentation suggests that issuing flushes
a bit less often can be helpful on other platforms too.

Hence, rearrange the logic in copy_file() so that flush requests are
issued once per N writes rather than every time through the loop.
I set the FLUSH_DISTANCE to 32MB on macOS (any less than that still
results in a noticeable speed degradation on APFS), but 1MB elsewhere.
In limited testing on Linux and FreeBSD, this seems slightly faster
than the previous code, and certainly no worse.  It helps noticeably
on macOS even with the older HFS filesystem.

A simpler change would have been to just increase the size of the
copy buffer without changing the loop logic, but that seems likely
to trash the processor cache without really helping much.

Back-patch to 9.6 where we introduced msync() as an implementation
option for pg_flush_data().  The problem seems specific to APFS's
mmap/msync support, so I don't think we need to go further back.

Discussion: https://postgr.es/m/CADkxhTNv-j2jw2g8H57deMeAbfRgYBoLmVuXkC=YCFBXRuCOww@mail.gmail.com

7 years agoReduce "X = X" to "X IS NOT NULL", if it's easy to do so.
Tom Lane [Sun, 8 Oct 2017 16:23:32 +0000 (12:23 -0400)]
Reduce "X = X" to "X IS NOT NULL", if it's easy to do so.

If the operator is a strict btree equality operator, and X isn't volatile,
then the clause must yield true for any non-null value of X, or null if X
is null.  At top level of a WHERE clause, we can ignore the distinction
between false and null results, so it's valid to simplify the clause to
"X IS NOT NULL".  This is a useful improvement mainly because we'll get
a far better selectivity estimate in most cases.

Because such cases seldom arise in well-written queries, it is unappetizing
to expend a lot of planner cycles looking for them ... but it turns out
that there's a place we can shoehorn this in practically for free, because
equivclass.c already has to detect and reject candidate equivalences of the
form X = X.  That doesn't catch every place that it would be valid to
simplify to X IS NOT NULL, but it catches the typical case.  Working harder
doesn't seem justified.

Patch by me, reviewed by Petr Jelinek

Discussion: https://postgr.es/m/CAMjNa7cC4X9YR-vAJS-jSYCajhRDvJQnN7m2sLH1wLh-_Z2bsw@mail.gmail.com

7 years agoImprove pg_regress's error reporting for schedule-file problems.
Tom Lane [Sat, 7 Oct 2017 22:04:25 +0000 (18:04 -0400)]
Improve pg_regress's error reporting for schedule-file problems.

The previous coding here trashed the line buffer as it scanned it,
making it impossible to print the source line in subsequent error
messages.  With a few save/restore/strdup pushups we can improve
that situation.

In passing, move the free'ing of the various strings that are collected
while processing one set of tests down to the bottom of the loop.
That's simpler, less surprising, and should make valgrind less unhappy
about the strings that were previously leaked by the last iteration.

7 years agoEnforce our convention about max number of parallel regression tests.
Tom Lane [Sat, 7 Oct 2017 21:20:09 +0000 (17:20 -0400)]
Enforce our convention about max number of parallel regression tests.

We have a very old rule that parallel_schedule should have no more
than twenty tests in any one parallel group, so as to provide a
bound on the number of concurrently running processes needed to
pass the tests.  But people keep forgetting the rule, so let's add
a few lines of code to check it.

Discussion: https://postgr.es/m/a37e9c57-22d4-1b82-1270-4501cd2e984e@2ndquadrant.com

7 years agoClean up sloppy maintenance of regression test schedule files.
Tom Lane [Sat, 7 Oct 2017 17:19:13 +0000 (13:19 -0400)]
Clean up sloppy maintenance of regression test schedule files.

The partition_join test was added to a parallel group that was already
at the maximum of 20 concurrent tests.  The hash_func test wasn't
added to serial_schedule at all.  The identity and partition_join tests
were added to serial_schedule with the aid of a dartboard, rather than
maintaining consistency with parallel_schedule.

There are proposals afoot to make these sorts of errors harder to make,
but in the meantime let's fix the ones already in place.

Discussion: https://postgr.es/m/a37e9c57-22d4-1b82-1270-4501cd2e984e@2ndquadrant.com

7 years agoFix crash when logical decoding is invoked from a PL function.
Tom Lane [Fri, 6 Oct 2017 23:18:58 +0000 (19:18 -0400)]
Fix crash when logical decoding is invoked from a PL function.

The logical decoding functions do BeginInternalSubTransaction and
RollbackAndReleaseCurrentSubTransaction to clean up after themselves.
It turns out that AtEOSubXact_SPI has an unrecognized assumption that
we always need to cancel the active SPI operation in the SPI context
that surrounds the subtransaction (if there is one).  That's true
when the RollbackAndReleaseCurrentSubTransaction call is coming from
the SPI-using function itself, but not when it's happening inside
some unrelated function invoked by a SPI query.  In practice the
affected callers are the various PLs.

To fix, record the current subtransaction ID when we begin a SPI
operation, and clean up only if that ID is the subtransaction being
canceled.

Also, remove AtEOSubXact_SPI's assertion that it must have cleaned
up the surrounding SPI context's active tuptable.  That's proven
wrong by the same test case.

Also clarify (or, if you prefer, reinterpret) the calling conventions
for _SPI_begin_call and _SPI_end_call.  The memory context cleanup
in the latter means that these have always had the flavor of a matched
resource-management pair, but they weren't documented that way before.

Per report from Ben Chobot.

Back-patch to 9.4 where logical decoding came in.  In principle,
the SPI changes should go all the way back, since the problem dates
back to commit 7ec1c5a86.  But given the lack of field complaints
it seems few people are using internal subtransactions in this way.
So I don't feel a need to take any risks in 9.2/9.3.

Discussion: https://postgr.es/m/73FBA179-C68C-4540-9473-71E865408B15@silentmedia.com

7 years agoCopy information from the relcache instead of pointing to it.
Robert Haas [Fri, 6 Oct 2017 19:27:11 +0000 (15:27 -0400)]
Copy information from the relcache instead of pointing to it.

We have the relations continuously locked, but not open, so relcache
pointers are not guaranteed to be stable.  Per buildfarm member
prion.

Ashutosh Bapat.  I fixed a typo.

Discussion: http://postgr.es/m/CAFjFpRcRBqoKLZSNmRsjKr81uEP=ennvqSQaXVCCBTXvJ2rW+Q@mail.gmail.com

7 years agoFix intra-query memory leakage in nodeProjectSet.c.
Tom Lane [Fri, 6 Oct 2017 18:28:42 +0000 (14:28 -0400)]
Fix intra-query memory leakage in nodeProjectSet.c.

Both ExecMakeFunctionResultSet() and evaluation of simple expressions
need to be done in the per-tuple memory context, not per-query, else
we leak data until end of query.  This is a consideration that was
missed while refactoring code in the ProjectSet patch (note that in
pre-v10, ExecMakeFunctionResult is called in the per-tuple context).

Per bug #14843 from Ben M.  Diagnosed independently by Andres and myself.

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

7 years agoFix access-off-end-of-array in clog.c.
Tom Lane [Fri, 6 Oct 2017 16:20:12 +0000 (12:20 -0400)]
Fix access-off-end-of-array in clog.c.

Sloppy loop coding in set_status_by_pages() resulted in fetching one array
element more than it should from the subxids[] array.  The odds of this
resulting in SIGSEGV are pretty small, but we've certainly seen that happen
with similar mistakes elsewhere.  While at it, we can get rid of an extra
TransactionIdToPage() calculation per loop.

Per report from David Binderman.  Back-patch to all supported branches,
since this code is quite old.

Discussion: https://postgr.es/m/HE1PR0802MB2331CBA919CBFFF0C465EB429C710@HE1PR0802MB2331.eurprd08.prod.outlook.com

7 years agoSupport coverage on vpath builds
Peter Eisentraut [Fri, 11 Aug 2017 03:33:47 +0000 (23:33 -0400)]
Support coverage on vpath builds

A few paths needed to be tweaked so everything looks into the
appropriate directories.

Reviewed-by: Michael Paquier <michael.paquier@gmail.com>
7 years agoRun coverage commands quietly
Peter Eisentraut [Fri, 11 Aug 2017 03:33:47 +0000 (23:33 -0400)]
Run coverage commands quietly

They are very chatty by default, but the output doesn't seem all that
useful for normal operation.

Reviewed-by: Michael Paquier <michael.paquier@gmail.com>
7 years agoRemove coverage details view
Peter Eisentraut [Fri, 11 Aug 2017 03:33:47 +0000 (23:33 -0400)]
Remove coverage details view

This is only useful if we name the different tests, which we don't do at
the moment.

Reviewed-by: Michael Paquier <michael.paquier@gmail.com>
7 years ago#ifdef out some dead code in psql/mainloop.c.
Tom Lane [Fri, 6 Oct 2017 15:35:49 +0000 (11:35 -0400)]
#ifdef out some dead code in psql/mainloop.c.

This pg_send_history() call is unreachable, since the block it's in
is currently only entered in !cur_cmd_interactive mode.  But rather
than just delete it, make it #ifdef NOT_USED, in hopes that we'll
remember to enable it if we ever change that decision.

Per report from David Binderman.  Since this is basically cosmetic,
I see no great need to back-patch.

Discussion: https://postgr.es/m/HE1PR0802MB233122B61F00A15E035C83BE9C710@HE1PR0802MB2331.eurprd08.prod.outlook.com

7 years agoFix traversal of half-frozen update chains
Alvaro Herrera [Fri, 6 Oct 2017 15:14:42 +0000 (17:14 +0200)]
Fix traversal of half-frozen update chains

When some tuple versions in an update chain are frozen due to them being
older than freeze_min_age, the xmax/xmin trail can become broken.  This
breaks HOT (and probably other things).  A subsequent VACUUM can break
things in more serious ways, such as leaving orphan heap-only tuples
whose root HOT redirect items were removed.  This can be seen because
index creation (or REINDEX) complain like
  ERROR:  XX000: failed to find parent tuple for heap-only tuple at (0,7) in table "t"

Because of relfrozenxid contraints, we cannot avoid the freezing of the
early tuples, so we must cope with the results: whenever we see an Xmin
of FrozenTransactionId, consider it a match for whatever the previous
Xmax value was.

This problem seems to have appeared in 9.3 with multixact changes,
though strictly speaking it seems unrelated.

Since 9.4 we have commit 37484ad2a "Change the way we mark tuples as
frozen", so the fix is simple: just compare the raw Xmin (still stored
in the tuple header, since freezing merely set an infomask bit) to the
Xmax.  But in 9.3 we rewrite the Xmin value to FrozenTransactionId, so
the original value is lost and we have nothing to compare the Xmax with.
To cope with that case we need to compare the Xmin with FrozenXid,
assume it's a match, and hope for the best.  Sadly, since you can
pg_upgrade a 9.3 instance containing half-frozen pages to newer
releases, we need to keep the old check in newer versions too, which
seems a bit brittle; I hope we can somehow get rid of that.

I didn't optimize the new function for performance.  The new coding is
probably a bit slower than before, since there is a function call rather
than a straight comparison, but I'd rather have it work correctly than
be fast but wrong.

This is a followup after 20b655224249 fixed a few related problems.
Apparently, in 9.6 and up there are more ways to get into trouble, but
in 9.3 - 9.5 I cannot reproduce a problem anymore with this patch, so
there must be a separate bug.

Reported-by: Peter Geoghegan
Diagnosed-by: Peter Geoghegan, Michael Paquier, Daniel Wood,
Yi Wen Wong, Álvaro
Discussion: https://postgr.es/m/CAH2-Wznm4rCrhFAiwKPWTpEw2bXDtgROZK7jWWGucXeH3D1fmA@mail.gmail.com

7 years agoBasic partition-wise join functionality.
Robert Haas [Fri, 6 Oct 2017 15:11:10 +0000 (11:11 -0400)]
Basic partition-wise join functionality.

Instead of joining two partitioned tables in their entirety we can, if
it is an equi-join on the partition keys, join the matching partitions
individually.  This involves teaching the planner about "other join"
rels, which are related to regular join rels in the same way that
other member rels are related to baserels.  This can use significantly
more CPU time and memory than regular join planning, because there may
now be a set of "other" rels not only for every base relation but also
for every join relation.  In most practical cases, this probably
shouldn't be a problem, because (1) it's probably unusual to join many
tables each with many partitions using the partition keys for all
joins and (2) if you do that scenario then you probably have a big
enough machine to handle the increased memory cost of planning and (3)
the resulting plan is highly likely to be better, so what you spend in
planning you'll make up on the execution side.  All the same, for now,
turn this feature off by default.

Currently, we can only perform joins between two tables whose
partitioning schemes are absolutely identical.  It would be nice to
cope with other scenarios, such as extra partitions on one side or the
other with no match on the other side, but that will have to wait for
a future patch.

Ashutosh Bapat, reviewed and tested by Rajkumar Raghuwanshi, Amit
Langote, Rafia Sabih, Thomas Munro, Dilip Kumar, Antonin Houska, Amit
Khandekar, and by me.  A few final adjustments by me.

Discussion: http://postgr.es/m/CAFjFpRfQ8GrQvzp3jA2wnLqrHmaXna-urjm_UY9BqXj=EaDTSA@mail.gmail.com
Discussion: http://postgr.es/m/CAFjFpRcitjfrULr5jfuKWRPsGUX0LQ0k8-yG0Qw2+1LBGNpMdw@mail.gmail.com

7 years agoFix typo in README.
Tom Lane [Thu, 5 Oct 2017 19:05:49 +0000 (15:05 -0400)]
Fix typo in README.

s/BeginInternalSubtransaction/BeginInternalSubTransaction/

7 years agoOn CREATE TABLE, consider skipping validation of subpartitions.
Robert Haas [Thu, 5 Oct 2017 17:21:50 +0000 (13:21 -0400)]
On CREATE TABLE, consider skipping validation of subpartitions.

This is just like commit 14f67a8ee282ebc0de78e773fbd597f460ab4a54, but
for CREATE PARTITION rather than ATTACH PARTITION.

Jeevan Ladhe, with test case changes by me.

Discussion: http://postgr.es/m/CAOgcT0MWwG8WBw8frFMtRYHAgDD=tpt6U7WcsO_L2k0KYpm4Jg@mail.gmail.com

7 years agoOn attach, consider skipping validation of subpartitions individually.
Robert Haas [Thu, 5 Oct 2017 17:06:46 +0000 (13:06 -0400)]
On attach, consider skipping validation of subpartitions individually.

If the table attached as a partition is itself partitioned, individual
partitions might have constraints strong enough to skip scanning the
table even if the table actually attached does not.  This is pretty
cheap to check, and possibly a big win if it works out.

Amit Langote, with test case changes by me.

Discussion: http://postgr.es/m/1f08b844-0078-aa8d-452e-7af3bf77d05f@lab.ntt.co.jp

7 years agoImprove error message when skipping scan of default partition.
Robert Haas [Thu, 5 Oct 2017 16:19:40 +0000 (12:19 -0400)]
Improve error message when skipping scan of default partition.

It seems like a good idea to clearly distinguish between skipping the
scan of the new partition itself and skipping the scan of the default
partition.

Amit Langote

Discussion: http://postgr.es/m/1f08b844-0078-aa8d-452e-7af3bf77d05f@lab.ntt.co.jp

7 years agoAllow DML commands that create tables to use parallel query.
Robert Haas [Thu, 5 Oct 2017 15:34:38 +0000 (11:34 -0400)]
Allow DML commands that create tables to use parallel query.

Haribabu Kommi, reviewed by Dilip Kumar and Rafia Sabih.  Various
cosmetic changes by me to explain why this appears to be safe but
allowing inserts in parallel mode in general wouldn't be.  Also, I
removed the REFRESH MATERIALIZED VIEW case from Haribabu's patch,
since I'm not convinced that case is OK, and hacked on the
documentation somewhat.

Discussion: http://postgr.es/m/CAJrrPGdo5bak6qnPWe8Kpi8g_jfQEs-G4SYmG9y+OFaw2-dPvA@mail.gmail.com

7 years agoImprove comments in vacuum_rel() and analyze_rel().
Tom Lane [Thu, 5 Oct 2017 14:47:47 +0000 (10:47 -0400)]
Improve comments in vacuum_rel() and analyze_rel().

Remove obsolete references to get_rel_oids().  Avoid listing specific
relkinds in the comments, since we seem unable to keep such things
in sync with the code, and it's not all that helpful anyhow.

Noted by Michael Paquier, though I rewrote the comments a bit more.

Discussion: https://postgr.es/m/CAB7nPqTWiN9zwKTaOrsnKiGDChqRt7C1+CiiDk4N4OMn92rs6A@mail.gmail.com

7 years agoFix typo.
Robert Haas [Thu, 5 Oct 2017 12:45:24 +0000 (08:45 -0400)]
Fix typo.

Etsuro Fujita

Discussion: http://postgr.es/m/1b2e9ac7-b99a-2769-5e42-afdf62bfa7fa@lab.ntt.co.jp

7 years agoFix more user-visible elog() calls.
Robert Haas [Thu, 5 Oct 2017 11:58:02 +0000 (07:58 -0400)]
Fix more user-visible elog() calls.

Michael Paquier discovered that this could be triggered via SQL;
give a nicer message instead.

Patch by Michael Paquier, reviewed by Masahiko Sawada.

Discussion: http://postgr.es/m/CAB7nPqQtPg+LKKtzdKN26judHcvPZ0s1gNigzOT4j8CYuuuBYg@mail.gmail.com

7 years agoDocument and use SPI_result_code_string()
Peter Eisentraut [Thu, 31 Aug 2017 02:16:50 +0000 (22:16 -0400)]
Document and use SPI_result_code_string()

A lot of semi-internal code just prints out numeric SPI error codes,
which is not very helpful.  We already have an API function to convert
the codes to a string, so let's make more use of that.

Reviewed-by: Michael Paquier <michael.paquier@gmail.com>
7 years agoMove SPI error reporting out of ri_ReportViolation()
Peter Eisentraut [Thu, 31 Aug 2017 02:16:50 +0000 (22:16 -0400)]
Move SPI error reporting out of ri_ReportViolation()

These are two completely unrelated code paths, so it doesn't make sense
to pack them into one function.

Add attribute noreturn to ri_ReportViolation().

Reviewed-by: Michael Paquier <michael.paquier@gmail.com>
7 years agoMsvc doesn't know UINT16_MAX, replace with PG_UINT16_MAX.
Andres Freund [Wed, 4 Oct 2017 17:01:02 +0000 (10:01 -0700)]
Msvc doesn't know UINT16_MAX, replace with PG_UINT16_MAX.

UINT16_MAX usage is originating from commit 212e6f34d55c.

Per buildfarm animal currawong.

7 years agoAttempt to adapt windows build for 212e6f34d55c.
Andres Freund [Wed, 4 Oct 2017 16:32:02 +0000 (09:32 -0700)]
Attempt to adapt windows build for 212e6f34d55c.

Per buildfarm animal baiji.

7 years agoReplace binary search in fmgr_isbuiltin with a lookup array.
Andres Freund [Wed, 4 Oct 2017 07:22:38 +0000 (00:22 -0700)]
Replace binary search in fmgr_isbuiltin with a lookup array.

Turns out we have enough functions that the binary search is quite
noticeable in profiles.

Thus have Gen_fmgrtab.pl build a new mapping from a builtin function's
oid to an index in the existing fmgr_builtins array. That keeps the
additional memory usage at a reasonable amount.

Author: Andres Freund, with input from Tom Lane
Discussion: https://postgr.es/m/20170914065128.a5sk7z4xde5uy3ei@alap3.anarazel.de

7 years agoMove genbki.pl's find_defined_symbol to Catalog.pm.
Andres Freund [Wed, 4 Oct 2017 07:11:36 +0000 (00:11 -0700)]
Move genbki.pl's find_defined_symbol to Catalog.pm.

Will be used in Gen_fmgrtab.pl in a followup commit.

7 years agoAdjust git_changelog for new-style release tags.
Tom Lane [Wed, 4 Oct 2017 04:45:15 +0000 (00:45 -0400)]
Adjust git_changelog for new-style release tags.

It wasn't on board with REL_n_n format.

7 years agoAllow multiple tables to be specified in one VACUUM or ANALYZE command.
Tom Lane [Tue, 3 Oct 2017 22:53:44 +0000 (18:53 -0400)]
Allow multiple tables to be specified in one VACUUM or ANALYZE command.

Not much to say about this; does what it says on the tin.

However, formerly, if there was a column list then the ANALYZE action was
implied; now it must be specified, or you get an error.  This is because
it would otherwise be a bit unclear what the user meant if some tables
have column lists and some don't.

Nathan Bossart, reviewed by Michael Paquier and Masahiko Sawada, with some
editorialization by me

Discussion: https://postgr.es/m/E061A8E3-5E3D-494D-94F0-E8A9B312BBFC@amazon.com

7 years agoFix race condition with unprotected use of a latch pointer variable.
Tom Lane [Tue, 3 Oct 2017 18:00:56 +0000 (14:00 -0400)]
Fix race condition with unprotected use of a latch pointer variable.

Commit 597a87ccc introduced a latch pointer variable to replace use
of a long-lived shared latch in the shared WalRcvData structure.
This was not well thought out, because there are now hazards of the
pointer variable changing while it's being inspected by another
process.  This could obviously lead to a core dump in code like

if (WalRcv->latch)
SetLatch(WalRcv->latch);

and there's a more remote risk of a torn read, if we have any
platforms where reading/writing a pointer is not atomic.

An actual problem would occur only if the walreceiver process
exits (gracefully) while the startup process is trying to
signal it, but that seems well within the realm of possibility.

To fix, treat the pointer variable (not the referenced latch)
as being protected by the WalRcv->mutex spinlock.  There
remains a race condition that we could apply SetLatch to a
process latch that no longer belongs to the walreceiver, but
I believe that's harmless: at worst it'd cause an extra wakeup
of the next process to use that PGPROC structure.

Back-patch to v10 where the faulty code was added.

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

7 years agoFix coding rules violations in walreceiver.c
Alvaro Herrera [Tue, 3 Oct 2017 12:58:25 +0000 (14:58 +0200)]
Fix coding rules violations in walreceiver.c

1. Since commit b1a9bad9e744 we had pstrdup() inside a
spinlock-protected critical section; reported by Andreas Seltenreich.
Turn those into strlcpy() to stack-allocated variables instead.
Backpatch to 9.6.

2. Since commit 9ed551e0a4fd we had a pfree() uselessly inside a
spinlock-protected critical section.  Tom Lane noticed in code review.
Move down.  Backpatch to 9.6.

3. Since commit 64233902d22b we had GetCurrentTimestamp() (a kernel
call) inside a spinlock-protected critical section.  Tom Lane noticed in
code review.  Move it up.  Backpatch to 9.2.

4. Since commit 1bb2558046cc we did elog(PANIC) while holding spinlock.
Tom Lane noticed in code review.  Release spinlock before dying.
Backpatch to 9.2.

Discussion: https://postgr.es/m/87h8vhtgj2.fsf@ansel.ydns.eu

7 years agoExpand collation documentation
Peter Eisentraut [Fri, 22 Sep 2017 17:51:01 +0000 (13:51 -0400)]
Expand collation documentation

Document better how to create custom collations and what locale strings
ICU accepts.  Explain the ICU examples in more detail.  Also update the
text on the CREATE COLLATION reference page a bit to take ICU more into
account.

7 years agoGrammar typo in security warning about md5
Simon Riggs [Mon, 2 Oct 2017 09:27:46 +0000 (10:27 +0100)]
Grammar typo in security warning about md5

7 years agoYet another pg_bswap typo in a windows only file.
Andres Freund [Mon, 2 Oct 2017 03:05:27 +0000 (20:05 -0700)]
Yet another pg_bswap typo in a windows only file.

Per buildfarm animal frogmouth.

Brown-Paper-Bagged-By: Andres Freund
7 years agoCorrect include file name in inet_aton fallback.
Andres Freund [Mon, 2 Oct 2017 00:41:00 +0000 (17:41 -0700)]
Correct include file name in inet_aton fallback.

Per buildfarm animal frogmouth.

Author: Andres Freund

7 years agoReplace most usages of ntoh[ls] and hton[sl] with pg_bswap.h.
Andres Freund [Sun, 1 Oct 2017 22:36:14 +0000 (15:36 -0700)]
Replace most usages of ntoh[ls] and hton[sl] with pg_bswap.h.

All postgres internal usages are replaced, it's just libpq example
usages that haven't been converted. External users of libpq can't
generally rely on including postgres internal headers.

Note that this includes replacing open-coded byte swapping of 64bit
integers (using two 32 bit swaps) with a single 64bit swap.

Where it looked applicable, I have removed netinet/in.h and
arpa/inet.h usage, which previously provided the relevant
functionality. It's perfectly possible that I missed other reasons for
including those, the buildfarm will tell.

Author: Andres Freund
Discussion: https://postgr.es/m/20170927172019.gheidqy6xvlxb325@alap3.anarazel.de

7 years agoRemove redundant stdint.h include.
Andres Freund [Sun, 1 Oct 2017 22:24:14 +0000 (15:24 -0700)]
Remove redundant stdint.h include.

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

7 years agoTry to make crash restart test work on windows.
Andres Freund [Sun, 1 Oct 2017 22:21:45 +0000 (15:21 -0700)]
Try to make crash restart test work on windows.

Author: Andres Freund
Tested-By: Andrew Dunstan
Discussion: https://postgr.es/m/20170930224424.ud5ilchmclbl5y5n@alap3.anarazel.de

7 years agoAllow pg_ctl kill to send SIGKILL.
Andres Freund [Sun, 1 Oct 2017 22:17:10 +0000 (15:17 -0700)]
Allow pg_ctl kill to send SIGKILL.

Previously that was disallowed out of an abundance of
caution. Providing KILL support however is helpful to make the
013_crash_restart.pl test portable, and there's no actual issue with
allowing it.  SIGABRT, which has similar consequences except it also
dumps core, was already allowed.

Author: Andres Freund
Discussion: https://postgr.es/m/45d42d41-6145-9be1-7261-84acf6d9e344@2ndQuadrant.com

7 years agoUpdate v10 release notes, and set the official release date.
Tom Lane [Sun, 1 Oct 2017 17:32:26 +0000 (13:32 -0400)]
Update v10 release notes, and set the official release date.

Last(?) round of changes for 10.0.

7 years agoUse a longer connection timeout in pg_isready test.
Tom Lane [Sun, 1 Oct 2017 16:43:46 +0000 (12:43 -0400)]
Use a longer connection timeout in pg_isready test.

Buildfarm members skink and sungazer have both recently failed this
test, with symptoms indicating that the default 3-second timeout
isn't quite enough for those very slow systems.  There's no reason
to be miserly with this timeout, so boost it to 60 seconds.

Back-patch to all versions containing this test.  That may be overkill,
because the failure has only been observed in the v10 branch, but
I don't feel like having to revisit this later.

7 years agoAdd list of acknowledgments to release notes
Peter Eisentraut [Sun, 1 Oct 2017 12:51:20 +0000 (08:51 -0400)]
Add list of acknowledgments to release notes

This contains all individuals mentioned in the commit messages during
PostgreSQL 10 development.

current through babf18579455e85269ad75e1ddb03f34138f77b6

Discussion: https://www.postgresql.org/message-id/flat/54ad0e42-770e-dfe1-123e-bce9361ad452%402ndquadrant.com

7 years agoFix busy-wait in pgbench, with --rate.
Heikki Linnakangas [Sun, 1 Oct 2017 06:29:27 +0000 (09:29 +0300)]
Fix busy-wait in pgbench, with --rate.

If --rate was used to throttle pgbench, it failed to sleep when it had
nothing to do, leading to a busy-wait with 100% CPU usage. This bug was
introduced in the refactoring in v10. Before that, sleep() was called with
a timeout, even when there were no file descriptors to wait for.

Reported by Jeff Janes, patch by Fabien COELHO. Backpatch to v10.

Discussion: https://www.postgresql.org/message-id/CAMkU%3D1x5hoX0pLLKPRnXCy0T8uHoDvXdq%2B7kAM9eoC9_z72ucw%40mail.gmail.com

7 years agoFix pg_dump to assign domain array type OIDs during pg_upgrade.
Tom Lane [Sat, 30 Sep 2017 21:05:07 +0000 (17:05 -0400)]
Fix pg_dump to assign domain array type OIDs during pg_upgrade.

During a binary upgrade, all type OIDs are supposed to be assigned by
pg_dump based on their values in the old cluster.  But now that domains
have arrays, there's nothing to base the arrays' type OIDs on, if we're
upgrading from a pre-v11 cluster.  Make pg_dump search for an unused type
OID to use for this purpose.  Per buildfarm.

Discussion: https://postgr.es/m/E1dyLlE-0002gT-H5@gemulon.postgresql.org

7 years agoSupport arrays over domains.
Tom Lane [Sat, 30 Sep 2017 17:40:56 +0000 (13:40 -0400)]
Support arrays over domains.

Allowing arrays with a domain type as their element type was left un-done
in the original domain patch, but not for any very good reason.  This
omission leads to such surprising results as array_agg() not working on
a domain column, because the parser can't identify a suitable output type
for the polymorphic aggregate.

In order to fix this, first clean up the APIs of coerce_to_domain() and
some internal functions in parse_coerce.c so that we consistently pass
around a CoercionContext along with CoercionForm.  Previously, we sometimes
passed an "isExplicit" boolean flag instead, which is strictly less
information; and coerce_to_domain() didn't even get that, but instead had
to reverse-engineer isExplicit from CoercionForm.  That's contrary to the
documentation in primnodes.h that says that CoercionForm only affects
display and not semantics.  I don't think this change fixes any live bugs,
but it makes things more consistent.  The main reason for doing it though
is that now build_coercion_expression() receives ccontext, which it needs
in order to be able to recursively invoke coerce_to_target_type().

Next, reimplement ArrayCoerceExpr so that the node does not directly know
any details of what has to be done to the individual array elements while
performing the array coercion.  Instead, the per-element processing is
represented by a sub-expression whose input is a source array element and
whose output is a target array element.  This simplifies life in
parse_coerce.c, because it can build that sub-expression by a recursive
invocation of coerce_to_target_type().  The executor now handles the
per-element processing as a compiled expression instead of hard-wired code.
The main advantage of this is that we can use a single ArrayCoerceExpr to
handle as many as three successive steps per element: base type conversion,
typmod coercion, and domain constraint checking.  The old code used two
stacked ArrayCoerceExprs to handle type + typmod coercion, which was pretty
inefficient, and adding yet another array deconstruction to do domain
constraint checking seemed very unappetizing.

In the case where we just need a single, very simple coercion function,
doing this straightforwardly leads to a noticeable increase in the
per-array-element runtime cost.  Hence, add an additional shortcut evalfunc
in execExprInterp.c that skips unnecessary overhead for that specific form
of expression.  The runtime speed of simple cases is within 1% or so of
where it was before, while cases that previously required two levels of
array processing are significantly faster.

Finally, create an implicit array type for every domain type, as we do for
base types, enums, etc.  Everything except the array-coercion case seems
to just work without further effort.

Tom Lane, reviewed by Andrew Dunstan

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

7 years agoFix copy & pasto in 510b8cbff15f.
Andres Freund [Sat, 30 Sep 2017 00:41:20 +0000 (17:41 -0700)]
Fix copy & pasto in 510b8cbff15f.

Reported-By: Peter Geoghegan
7 years agoFix typo.
Andres Freund [Fri, 29 Sep 2017 22:52:55 +0000 (15:52 -0700)]
Fix typo.

Reported-By: Thomas Munro and Jesper Pedersen
7 years agoExtend & revamp pg_bswap.h infrastructure.
Andres Freund [Fri, 29 Sep 2017 22:52:55 +0000 (15:52 -0700)]
Extend & revamp pg_bswap.h infrastructure.

Upcoming patches are going to address performance issues that involve
slow system provided ntohs/htons etc. To address that expand
pg_bswap.h to provide pg_ntoh{16,32,64}, pg_hton{16,32,64} and
optimize their respective implementations by using compiler intrinsics
for gcc compatible compilers and msvc. Fall back to manual
implementations using shifts etc otherwise.

Additionally remove multiple evaluation hazards from the existing
BSWAP32/64 macros, by replacing them with inline functions when
necessary. In the course of that the naming scheme is changed to
pg_bswap16/32/64.

Author: Andres Freund
Discussion: https://postgr.es/m/20170927172019.gheidqy6xvlxb325@alap3.anarazel.de

7 years agoUse Py_RETURN_NONE where suitable
Peter Eisentraut [Fri, 29 Sep 2017 20:50:01 +0000 (16:50 -0400)]
Use Py_RETURN_NONE where suitable

This is more idiomatic style and available as of Python 2.4, which is
our minimum.

7 years agoFix inadequate locking during get_rel_oids().
Tom Lane [Fri, 29 Sep 2017 20:26:21 +0000 (16:26 -0400)]
Fix inadequate locking during get_rel_oids().

get_rel_oids used to not take any relation locks at all, but that stopped
being a good idea with commit 3c3bb9933, which inserted a syscache lookup
into the function.  A concurrent DROP TABLE could now produce "cache lookup
failed", which we don't want to have happen in normal operation.  The best
solution seems to be to transiently take a lock on the relation named by
the RangeVar (which also makes the result of RangeVarGetRelid a lot less
spongy).  But we shouldn't hold the lock beyond this function, because we
don't want VACUUM to lock more than one table at a time.  (That would not
be a big problem right now, but it will become one after the pending
feature patch to allow multiple tables to be named in VACUUM.)

In passing, adjust vacuum_rel and analyze_rel to document that we don't
trust the passed RangeVar to be accurate, and allow the RangeVar to
possibly be NULL --- which it is anyway for a whole-database VACUUM,
though we accidentally didn't crash for that case.

The passed RangeVar is in fact inaccurate when dealing with a child
partition, as of v10, and it has been wrong for a whole long time in the
case of vacuum_rel() recursing to a TOAST table.  None of these things
present visible bugs up to now, because the passed RangeVar is in fact
only consulted for autovacuum logging, and in that particular context it's
always accurate because autovacuum doesn't let vacuum.c expand partitions
nor recurse to toast tables.  Still, this seems like trouble waiting to
happen, so let's nail the door at least partly shut.  (Further cleanup
is planned, in HEAD only, as part of the pending feature patch.)

Fix some sadly inaccurate/obsolete comments too.  Back-patch to v10.

Michael Paquier and Tom Lane

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

7 years agopsql: Don't try to print a partition constraint we didn't fetch.
Robert Haas [Fri, 29 Sep 2017 19:59:11 +0000 (15:59 -0400)]
psql: Don't try to print a partition constraint we didn't fetch.

If \d rather than \d+ is used, then verbose is false and we don't ask
the server for the partition constraint; so we shouldn't print it in
that case either.

Maksim Milyutin, per a report from Jesper Pedersen.  Reviewed by
Jesper Pedersen and Amit Langote.

Discussion: http://postgr.es/m/2af5fc4d-7bcc-daa8-4fe6-86274bea363c@redhat.com

7 years agopgbench: If we fail to send a command to the server, fail.
Robert Haas [Fri, 29 Sep 2017 17:51:14 +0000 (13:51 -0400)]
pgbench: If we fail to send a command to the server, fail.

This beats the old behavior of busy-waiting hands down.

Oversight in commit 12788ae49e1933f463bc59a6efe46c4a01701b76.

Report by Pavan Deolasee. Patch by Fabien Coelho.  Reviewed by
Pavan Deolasee.

Discussion: http://postgr.es/m/CABOikdPhfXTypckMC1Ux6Ko+hKBWwUBA=EXsvamXYSg8M9J94w@mail.gmail.com

7 years agopsql: Update \d sequence display
Peter Eisentraut [Mon, 25 Sep 2017 15:59:46 +0000 (11:59 -0400)]
psql: Update \d sequence display

For \d sequencename, the psql code just did SELECT * FROM sequencename
to get the information to display, but this does not contain much
interesting information anymore in PostgreSQL 10, because the metadata
has been moved to a separate system catalog.

This patch creates a newly designed sequence display that is not merely
an extension of the general relation/table display as it was previously.

Example:

PostgreSQL 9.6:

=> \d foobar
           Sequence "public.foobar"
    Column     |  Type   |        Value
---------------+---------+---------------------
 sequence_name | name    | foobar
 last_value    | bigint  | 1
 start_value   | bigint  | 1
 increment_by  | bigint  | 1
 max_value     | bigint  | 9223372036854775807
 min_value     | bigint  | 1
 cache_value   | bigint  | 1
 log_cnt       | bigint  | 0
 is_cycled     | boolean | f
 is_called     | boolean | f

PostgreSQL 10 before this change:

=> \d foobar
   Sequence "public.foobar"
   Column   |  Type   | Value
------------+---------+-------
 last_value | bigint  | 1
 log_cnt    | bigint  | 0
 is_called  | boolean | f

New:

=> \d foobar
                           Sequence "public.foobar"
  Type  | Start | Minimum |       Maximum       | Increment | Cycles? | Cache
--------+-------+---------+---------------------+-----------+---------+-------
 bigint |     1 |       1 | 9223372036854775807 |         1 | no      |     1

Reviewed-by: Fabien COELHO <coelho@cri.ensmp.fr>
7 years agoMarginal improvement for generated code in execExprInterp.c.
Tom Lane [Fri, 29 Sep 2017 15:32:05 +0000 (11:32 -0400)]
Marginal improvement for generated code in execExprInterp.c.

Avoid the coding pattern "*op->resvalue = f();", as some compilers think
that requires them to evaluate "op->resvalue" before the function call.
Unless there are lots of free registers, this can lead to a useless
register spill and reload across the call.

I changed all the cases like this in ExecInterpExpr(), but didn't bother
in the out-of-line opcode eval subroutines, since those are presumably
not as performance-critical.

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

7 years agoAdd background worker type
Peter Eisentraut [Thu, 31 Aug 2017 16:24:47 +0000 (12:24 -0400)]
Add background worker type

Add bgw_type field to background worker structure.  It is intended to be
set to the same value for all workers of the same type, so they can be
grouped in pg_stat_activity, for example.

The backend_type column in pg_stat_activity now shows bgw_type for a
background worker.  The ps listing also no longer calls out that a
process is a background worker but just show the bgw_type.  That way,
being a background worker is more of an implementation detail now that
is not shown to the user.  However, most log messages still refer to
'background worker "%s"'; otherwise constructing sensible and
translatable log messages would become tricky.

Reviewed-by: Michael Paquier <michael.paquier@gmail.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
7 years agoRemove replacement selection sort.
Robert Haas [Fri, 29 Sep 2017 14:20:44 +0000 (10:20 -0400)]
Remove replacement selection sort.

At the time replacement_sort_tuples was introduced, there were still
cases where replacement selection sort noticeably outperformed using
quicksort even for the first run.  However, those cases seem to have
evaporated as a result of further improvements made since that time
(and perhaps also advances in CPU technology).  So remove replacement
selection and the controlling GUC entirely.  This makes tuplesort.c
noticeably simpler and probably paves the way for further
optimizations someone might want to do later.

Peter Geoghegan, with review and testing by Tomas Vondra and me.

Discussion: https://postgr.es/m/CAH2-WzmmNjG_K0R9nqYwMq3zjyJJK+hCbiZYNGhAy-Zyjs64GQ@mail.gmail.com

7 years agoAdd PostgreSQL version to coverage output
Peter Eisentraut [Fri, 11 Aug 2017 03:33:47 +0000 (23:33 -0400)]
Add PostgreSQL version to coverage output

Also make overriding the title easier.  That helps telling where the
report came from and labeling different variants of a report.

Reviewed-by: Michael Paquier <michael.paquier@gmail.com>
7 years agoAdd lcov --initial
Peter Eisentraut [Fri, 11 Aug 2017 03:33:47 +0000 (23:33 -0400)]
Add lcov --initial

By just running lcov on the produced .gcda data files, we don't account
for source files that are not touched by tests at all.  To fix that, run
lcov --initial to create a base line info file with all zero counters,
and merge that with the actual counters when creating the final report.

Reviewed-by: Michael Paquier <michael.paquier@gmail.com>
7 years agoRemove SGML marked sections
Peter Eisentraut [Thu, 28 Sep 2017 20:17:28 +0000 (16:17 -0400)]
Remove SGML marked sections

For XML compatibility, replace marked sections <![IGNORE[ ]]> with
comments <!-- -->.  In some cases it seemed better to remove the ignored
text altogether, and in one case the text should not have been ignored.