]> granicus.if.org Git - postgresql/log
postgresql
8 years agoEmit invalidations to standby for transactions without xid.
Andres Freund [Sun, 24 Apr 2016 02:18:00 +0000 (19:18 -0700)]
Emit invalidations to standby for transactions without xid.

So far, when a transaction with pending invalidations, but without an
assigned xid, committed, we simply ignored those invalidation
messages. That's problematic, because those are actually sent for a
reason.

Known symptoms of this include that existing sessions on a hot-standby
replica sometimes fail to notice new concurrently built indexes and
visibility map updates.

The solution is to WAL log such invalidations in transactions without an
xid. We considered to alternatively force-assign an xid, but that'd be
problematic for vacuum, which might be run in systems with few xids.

Important: This adds a new WAL record, but as the patch has to be
back-patched, we can't bump the WAL page magic. This means that standbys
have to be updated before primaries; otherwise
"PANIC: standby_redo: unknown op code 32" errors can be encountered.

XXX:

Reported-By: Васильев Дмитрий, Masahiko Sawada
Discussion:
    CAB-SwXY6oH=9twBkXJtgR4UC1NqT-vpYAtxCseME62ADwyK5OA@mail.gmail.com
    CAD21AoDpZ6Xjg=gFrGPnSn4oTRRcwK1EBrWCq9OqOHuAcMMC=w@mail.gmail.com

8 years agoFix pg_get_functiondef to dump parallel-safety markings.
Robert Haas [Wed, 27 Apr 2016 02:56:04 +0000 (22:56 -0400)]
Fix pg_get_functiondef to dump parallel-safety markings.

Ashutosh Sharma

8 years agoImpose a full barrier in generic-xlc.h atomics functions.
Noah Misch [Wed, 27 Apr 2016 01:53:58 +0000 (21:53 -0400)]
Impose a full barrier in generic-xlc.h atomics functions.

pg_atomic_compare_exchange_*_impl() were providing only the semantics of
an acquire barrier.  Buildfarm members hornet and mandrill revealed this
deficit beginning with commit 008608b9d51061b1f598c197477b3dc7be9c4a64.
While we have no report of symptoms in 9.5, we can't rule out the
possibility of certain compilers, hardware, or extension code relying on
these functions' specified barrier semantics.  Back-patch to 9.5, where
commit b64d92f1a5602c55ee8b27a7ac474f03b7aee340 introduced atomics.

Reviewed by Andres Freund.

8 years agopg_dump: Message style improvements
Peter Eisentraut [Tue, 26 Apr 2016 16:04:43 +0000 (12:04 -0400)]
pg_dump: Message style improvements

forgotten in b6dacc173b6830c515d970698cead9a85663c553

8 years agoAdd a --brief option to git_changelog.
Tom Lane [Tue, 26 Apr 2016 22:52:17 +0000 (18:52 -0400)]
Add a --brief option to git_changelog.

In commit c0b050192, Andres introduced the idea of including one-line
commit references in our major release notes.  Teach git_changelog to
emit a (lightly adapted) version of that format, so that we don't
have to laboriously add it to the notes after the fact.  The default
output isn't changed, since I anticipate still using that for minor
release notes.

8 years agoFix tsearch docs
Teodor Sigaev [Tue, 26 Apr 2016 17:26:26 +0000 (20:26 +0300)]
Fix tsearch docs

Remove mention of setweight(tsquery) which wasn't included in 9.6. Also
replace old forgotten phrase operator to new one.

Dmitry Ivanov

8 years agoFix order of shutdown cleanup operations in PostgresNode.pm.
Tom Lane [Tue, 26 Apr 2016 16:43:03 +0000 (12:43 -0400)]
Fix order of shutdown cleanup operations in PostgresNode.pm.

Previously, database clusters created by a TAP test were shut down by
DESTROY methods attached to the PostgresNode objects representing them.
The trouble with that is that if the objects survive into the final global
destruction phase (which they do), Perl executes the DESTROY methods in an
unspecified order.  Thus, the order of shutdown of multiple clusters was
indeterminate, which might lead to not-very-reproducible errors getting
logged (eg from a slave whose master might or might not get killed first).
Worse, the File::Temp objects representing the temporary PGDATA directories
might get destroyed before the PostgresNode objects, resulting in attempts
to delete PGDATA directories that still have live servers in them.  On
Windows, this would lead to directory deletion failures; on Unix, it
usually had no effects worse than erratic "could not open temporary
statistics file "pg_stat/global.tmp": No such file or directory" log
messages.

While none of this would affect the reported result of the TAP test, which
is already determined, it could be very confusing when one is trying to
understand from the logs what went wrong with a failed test.

To fix, do the postmaster shutdowns in an END block rather than at object
destruction time.  The END block will execute at a well-defined (and
reasonable) time during script termination, and it will stop the
postmasters in order of PostgresNode object creation.  (Perhaps we should
change that to be reverse order of creation, but the main point here is
that we now have control which we did not before.)  Use "pg_ctl stop", not
an asynchronous kill(SIGQUIT), so that we wait for the postmasters to shut
down before proceeding with directory deletion.

Deletion of temporary directories still happens in an unspecified order
during global destruction, but I can see no reason to care about that
once the postmasters are stopped.

8 years agoYet more portability hacking for degree-based trig functions.
Tom Lane [Tue, 26 Apr 2016 15:24:15 +0000 (11:24 -0400)]
Yet more portability hacking for degree-based trig functions.

The true explanation for Peter Eisentraut's report of inexact asind results
seems to be that (a) he's compiling into x87 instruction set, which uses
wider-than-double float registers, plus (b) the library function asin() on
his platform returns a result that is wider than double and is not rounded
to double width.  To fix, we have to force the function's result to be
rounded comparably to what happened to the scaling constant asin_0_5.
Experimentation suggests that storing it into a volatile local variable is
the least ugly way of making that happen.  Although only asin() is known to
exhibit an observable inexact result, we'd better do this in all the places
where we're hoping to get an exact result by scaling.

8 years agoEnable parallel query by default.
Robert Haas [Tue, 26 Apr 2016 12:31:38 +0000 (08:31 -0400)]
Enable parallel query by default.

Change max_parallel_degree default from 0 to 2.  It is possible that
this is not a good idea, or that we should go with 1 worker rather
than 2, but we won't find out without trying it.  Along the way,
reword the documentation for max_parallel_degree a little bit to
hopefully make it more clear.

Discussion: 20160420174631.3qjjhpwsvvx5bau5@alap3.anarazel.de

8 years agoFix typo in comment
Magnus Hagander [Tue, 26 Apr 2016 08:38:32 +0000 (10:38 +0200)]
Fix typo in comment

Author: Daniel Gustafsson

8 years agopg_dump: Message style improvements
Peter Eisentraut [Mon, 25 Apr 2016 21:16:59 +0000 (17:16 -0400)]
pg_dump: Message style improvements

8 years agoFix C comment typo and redundant test
Kevin Grittner [Mon, 25 Apr 2016 20:42:29 +0000 (15:42 -0500)]
Fix C comment typo and redundant test

8 years agoNew method for preventing compile-time calculation of degree constants.
Tom Lane [Mon, 25 Apr 2016 19:21:04 +0000 (15:21 -0400)]
New method for preventing compile-time calculation of degree constants.

Commit 65abaab547a5758b tried to prevent the scaling constants used in
the degree-based trig functions from being precomputed at compile time,
because some compilers do that with functions that don't yield results
identical-to-the-last-bit to what you get at runtime.  A report from
Peter Eisentraut suggests that some recent compilers are smart enough
to see through that trick, though.  Instead, let's put the inputs to
these calculations into non-const global variables, which should be a
more reliable way of convincing the compiler that it can't assume that
they are compile-time constants.  (If we really get desperate, we could
mark these variables "volatile", but I do not believe we should have to.)

8 years agoTry harder to detect a port conflict in PostgresNode.pm.
Tom Lane [Mon, 25 Apr 2016 16:28:49 +0000 (12:28 -0400)]
Try harder to detect a port conflict in PostgresNode.pm.

Commit fab84c7787f25756 tried to get away without doing an actual bind(),
but buildfarm results show that that doesn't get the job done.  So we must
really bind to the target port --- and at least on my Linux box, we need a
listen() as well, or conflicts won't be detected.  We rely on SO_REUSEADDR
to prevent problems from starting a postmaster on the socket immediately
after we've bound to it in the test code.  (There may be platforms where
that doesn't work too well.  But fortunately, we only really care whether
this works on Windows, and there the default behavior should be OK.)

8 years agoUpdate GETTEXT_FILES after config and controldata refactoring
Peter Eisentraut [Mon, 25 Apr 2016 00:58:11 +0000 (20:58 -0400)]
Update GETTEXT_FILES after config and controldata refactoring

8 years agodoc: Fix typo
Peter Eisentraut [Mon, 25 Apr 2016 00:44:22 +0000 (20:44 -0400)]
doc: Fix typo

From: Andreas Seltenreich <andreas.seltenreich@credativ.de>

8 years agoImprove PostgresNode.pm's logic for detecting already-in-use ports.
Tom Lane [Sun, 24 Apr 2016 19:31:36 +0000 (15:31 -0400)]
Improve PostgresNode.pm's logic for detecting already-in-use ports.

Buildfarm members bowerbird and jacana have shown intermittent "could not
bind IPv4 socket" failures in the BinInstallCheck stage since mid-December,
shortly after commits 1caef31d9e550408 and 9821492ee417a591 changed the
logic for selecting which port to use in temporary installations.  One
plausible explanation is that we are randomly selecting ports that are
already in use for some non-Postgres purpose.  Although the code tried
to defend against already-in-use ports, it used pg_isready to probe
the port which is quite unhelpful: if some non-Postgres server responds
at the given address, pg_isready will generally say "no response",
leading to exactly the wrong conclusion about whether the port is free.

Instead, let's use a simple TCP connect() call to see if anything answers
without making assumptions about what it is.  Note that this means there's
no direct check for a conflicting Unix socket, but that should be okay
because there should be no other Unix sockets in use in the temporary
socket directory created for a test run.

This is only a partial solution for the TCP case, since if the port number
is in use for an outgoing connection rather than a listening socket, we'll
fail to detect that.  We could try to bind() to the proposed port as a
means of detecting that case, but that would introduce its own failure
modes, since the system might consider the address to remain reserved for
some period of time after we drop the bound socket.  Close study of the
errors returned by bowerbird and jacana suggests that what we're seeing
there may be conflicts with listening not outgoing sockets, so let's try
this and see if it improves matters.  It's certainly better than what's
there now, in any case.

Michael Paquier, adjusted by me to work on non-Windows as well as Windows

8 years agoFix documentation & config inconsistencies around 428b1d6b2.
Andres Freund [Sun, 24 Apr 2016 19:26:55 +0000 (12:26 -0700)]
Fix documentation & config inconsistencies around 428b1d6b2.

Several issues:
1) checkpoint_flush_after doc and code disagreed about the default
2) new GUCs were missing from postgresql.conf.sample
3) Outdated source-code comment about bgwriter_flush_after's default
4) Sub-optimal categories assigned to new GUCs
5) Docs suggested backend_flush_after is PGC_SIGHUP, but it's PGC_USERSET.
6) Spell out int as integer in the docs, as done elsewhere

Reported-By: Magnus Hagander, Fujii Masao
Discussion: CAHGQGwETyTG5VYQQ5C_srwxWX7RXvFcD3dKROhvAWWhoSBdmZw@mail.gmail.com

8 years agoRename strtoi() to strtoint().
Tom Lane [Sat, 23 Apr 2016 20:53:15 +0000 (16:53 -0400)]
Rename strtoi() to strtoint().

NetBSD has seen fit to invent a libc function named strtoi(), which
conflicts with the long-established static functions of the same name in
datetime.c and ecpg's interval.c.  While muttering darkly about intrusions
on application namespace, we'll rename our functions to avoid the conflict.

Back-patch to all supported branches, since this would affect attempts
to build any of them on recent NetBSD.

Thomas Munro

8 years agodoc: Fix typos
Peter Eisentraut [Sat, 23 Apr 2016 18:48:02 +0000 (14:48 -0400)]
doc: Fix typos

From: Erik Rijkers <er@xs4all.nl>

8 years agoProperly mark initRectBox() as taking 'void' args
Bruce Momjian [Sat, 23 Apr 2016 14:41:11 +0000 (10:41 -0400)]
Properly mark initRectBox() as taking 'void' args

Was part of box type in SP-GiST index patch.

Reported-by: Emre Hasegeli
8 years agoConvert contrib/seg's bool-returning SQL functions to V1 call convention.
Tom Lane [Fri, 22 Apr 2016 15:54:23 +0000 (11:54 -0400)]
Convert contrib/seg's bool-returning SQL functions to V1 call convention.

It appears that we can no longer get away with using V0 call convention
for bool-returning functions in newer versions of MSVC.  The compiler
seems to generate code that doesn't clear the higher-order bits of the
result register, causing the bool result Datum to often read as "true"
when "false" was intended.  This is not very surprising, since the
function thinks it's returning a bool-width result but fmgr_oldstyle
assumes that V0 functions return "char *"; what's surprising is that
that hack worked for so long on so many platforms.

The only functions of this description in core+contrib are in contrib/seg,
which we'd intentionally left mostly in V0 style to serve as a warning
canary if V0 call convention breaks.  We could imagine hacking things
so that they're still V0 (we'd have to redeclare the bool-returning
functions as returning some suitably wide integer type, like size_t,
at the C level).  But on the whole it seems better to convert 'em to V1.
We can still leave the pointer- and int-returning functions in V0 style,
so that the test coverage isn't gone entirely.

Back-patch to 9.5, since our intention is to support VS2015 in 9.5
and later.  There's no SQL-level change in the functions' behavior
so back-patching should be safe enough.

Discussion: <22094.1461273324@sss.pgh.pa.us>

Michael Paquier, adjusted some by me

8 years agoAdd putenv support for msvcrt from Visual Studio 2013
Magnus Hagander [Fri, 22 Apr 2016 09:18:59 +0000 (05:18 -0400)]
Add putenv support for msvcrt from Visual Studio 2013

This was missed when VS 2013 support was added.

Michael Paquier

8 years agoFix unexpected side-effects of operator_precedence_warning.
Tom Lane [Fri, 22 Apr 2016 03:17:36 +0000 (23:17 -0400)]
Fix unexpected side-effects of operator_precedence_warning.

The implementation of that feature involves injecting nodes into the
raw parsetree where explicit parentheses appear.  Various places in
parse_expr.c that test to see "is this child node of type Foo" need to
look through such nodes, else we'll get different behavior when
operator_precedence_warning is on than when it is off.  Note that we only
need to handle this when testing untransformed child nodes, since the
AEXPR_PAREN nodes will be gone anyway after transformExprRecurse.

Per report from Scott Ribe and additional code-reading.  Back-patch
to 9.5 where this feature was added.

Report: <ED37E303-1B0A-4CD8-8E1E-B9C4C2DD9A17@elevated-dev.com>

8 years agoFix planner failure with full join in RHS of left join.
Tom Lane [Fri, 22 Apr 2016 00:05:58 +0000 (20:05 -0400)]
Fix planner failure with full join in RHS of left join.

Given a left join containing a full join in its righthand side, with
the left join's joinclause referencing only one side of the full join
(in a non-strict fashion, so that the full join doesn't get simplified),
the planner could fail with "failed to build any N-way joins" or related
errors.  This happened because the full join was seen as overlapping the
left join's RHS, and then recent changes within join_is_legal() caused
that function to conclude that the full join couldn't validly be formed.
Rather than try to rejigger join_is_legal() yet more to allow this,
I think it's better to fix initsplan.c so that the required join order
is explicit in the SpecialJoinInfo data structure.  The previous coding
there essentially ignored full joins, relying on the fact that we don't
flatten them in the joinlist data structure to preserve their ordering.
That's sufficient to prevent a wrong plan from being formed, but as this
example shows, it's not sufficient to ensure that the right plan will
be formed.  We need to work a bit harder to ensure that the right plan
looks sane according to the SpecialJoinInfos.

Per bug #14105 from Vojtech Rylko.  This was apparently induced by
commit 8703059c6 (though now that I've seen it, I wonder whether there
are related cases that could have failed before that); so back-patch
to all active branches.  Unfortunately, that patch also went into 9.0,
so this bug is a regression that won't be fixed in that branch.

8 years agoImprove TranslateSocketError() to handle more Windows error codes.
Tom Lane [Thu, 21 Apr 2016 20:58:47 +0000 (16:58 -0400)]
Improve TranslateSocketError() to handle more Windows error codes.

The coverage was rather lean for cases that bind() or listen() might
return.  Add entries for everything that there's a direct equivalent
for in the set of Unix errnos that elog.c has heard of.

8 years agoRemove dead code in win32.h.
Tom Lane [Thu, 21 Apr 2016 20:16:19 +0000 (16:16 -0400)]
Remove dead code in win32.h.

There's no longer a need for the MSVC-version-specific code stanza that
forcibly redefines errno code symbols, because since commit 73838b52 we're
unconditionally redefining them in the stanza before this one anyway.
Now it's merely confusing and ugly, so get rid of it; and improve the
comment that explains what's going on here.

Although this is just cosmetic, back-patch anyway since I'm intending
to back-patch some less-cosmetic changes in this same hunk of code.

8 years agoPGDLLIMPORT-ify old_snapshot_threshold.
Tom Lane [Thu, 21 Apr 2016 18:33:34 +0000 (14:33 -0400)]
PGDLLIMPORT-ify old_snapshot_threshold.

Revert commit 7cb1db1d9599f0a09d6920d2149d956ef6d88b0e, which represented
a misunderstanding of the problem (if snapmgr.h weren't already included
in bufmgr.h, things wouldn't compile anywhere).  Instead install what
I think is the real fix.

8 years agoFix ruleutils.c's dumping of ScalarArrayOpExpr containing an EXPR_SUBLINK.
Tom Lane [Thu, 21 Apr 2016 18:20:18 +0000 (14:20 -0400)]
Fix ruleutils.c's dumping of ScalarArrayOpExpr containing an EXPR_SUBLINK.

When we shoehorned "x op ANY (array)" into the SQL syntax, we created a
fundamental ambiguity as to the proper treatment of a sub-SELECT on the
righthand side: perhaps what's meant is to compare x against each row of
the sub-SELECT's result, or perhaps the sub-SELECT is meant as a scalar
sub-SELECT that delivers a single array value whose members should be
compared against x.  The grammar resolves it as the former case whenever
the RHS is a select_with_parens, making the latter case hard to reach ---
but you can get at it, with tricks such as attaching a no-op cast to the
sub-SELECT.  Parse analysis would throw away the no-op cast, leaving a
parsetree with an EXPR_SUBLINK SubLink directly under a ScalarArrayOpExpr.
ruleutils.c was not clued in on this fine point, and would naively emit
"x op ANY ((SELECT ...))", which would be parsed as the first alternative,
typically leading to errors like "operator does not exist: text = text[]"
during dump/reload of a view or rule containing such a construct.  To fix,
emit a no-op cast when dumping such a parsetree.  This might well be
exactly what the user wrote to get the construct accepted in the first
place; and even if she got there with some other dodge, it is a valid
representation of the parsetree.

Per report from Karl Czajkowski.  He mentioned only a case involving
RLS policies, but actually the problem is very old, so back-patch to
all supported branches.

Report: <20160421001832.GB7976@moraine.isi.edu>

8 years agoPrevent possible crash reading pg_stat_activity.
Robert Haas [Thu, 21 Apr 2016 18:02:15 +0000 (14:02 -0400)]
Prevent possible crash reading pg_stat_activity.

Also, avoid reading PGPROC's wait_event field twice, once for the wait
event and again for the wait_event_type, because the value might change
in the middle.

Petr Jelinek and Robert Haas

8 years agoComment improvements for ForeignPath.
Robert Haas [Thu, 21 Apr 2016 17:30:48 +0000 (13:30 -0400)]
Comment improvements for ForeignPath.

It's not necessarily just scanning a base relation any more.

Amit Langote and Etsuro Fujita

8 years agoFix assorted defects in 09adc9a8c09c9640de05c7023b27fb83c761e91c.
Robert Haas [Thu, 21 Apr 2016 17:24:09 +0000 (13:24 -0400)]
Fix assorted defects in 09adc9a8c09c9640de05c7023b27fb83c761e91c.

That commit increased all shared memory allocations to the next higher
multiple of PG_CACHE_LINE_SIZE, but it didn't ensure that allocation
started on a cache line boundary.  It also failed to remove a couple
other pieces of now-useless code.

BUFFERALIGN() is perhaps obsolete at this point, and likely should be
removed at some point, too, but that seems like it can be left to a
future cleanup.

Mistakes all pointed out by Andres Freund.  The patch is mine, with
a few extra assertions which I adopted from his version of this fix.

8 years agoInclude snapmgr.h in blscan.c
Kevin Grittner [Thu, 21 Apr 2016 16:51:20 +0000 (11:51 -0500)]
Include snapmgr.h in blscan.c

Windows builds on buildfarm are failing because
old_snapshot_threshold is not found in the bloom filter contrib
module.

8 years agoAllow queries submitted by postgres_fdw to be canceled.
Robert Haas [Thu, 21 Apr 2016 14:46:09 +0000 (10:46 -0400)]
Allow queries submitted by postgres_fdw to be canceled.

This fixes a problem which is not new, but with the advent of direct
foreign table modification in 0bf3ae88af330496517722e391e7c975e6bad219,
it's somewhat more likely to be annoying than previously.  So,
arrange for a local query cancelation to propagate to the remote side.

Michael Paquier, reviewed by Etsuro Fujita.  Original report by
Thom Brown.

8 years agoInline initial comparisons in TestForOldSnapshot()
Kevin Grittner [Thu, 21 Apr 2016 13:40:08 +0000 (08:40 -0500)]
Inline initial comparisons in TestForOldSnapshot()

Even with old_snapshot_threshold = -1 (which disables the "snapshot
too old" feature), performance regressions were seen at moderate to
high concurrency.  For example, a one-socket, four-core system
running 200 connections at saturation could see up to a 2.3%
regression, with larger regressions possible on NUMA machines.
By inlining the early (smaller, faster) tests in the
TestForOldSnapshot() function, the i7 case dropped to a 0.2%
regression, which could easily just be noise, and is clearly an
improvement.  Further testing will show whether more is needed.

8 years agopostgres_fdw: Don't push down certain full joins.
Robert Haas [Thu, 21 Apr 2016 03:34:07 +0000 (23:34 -0400)]
postgres_fdw: Don't push down certain full joins.

If there's a filter condition on either side of a full outer join,
it is neither correct to attach it to the join's ON clause nor to
throw it into the toplevel WHERE clause.  Just don't push down the
join in that case.

To maximize the number of cases where we can still push down full
joins, push inner join conditions into the ON clause at the first
opportunity rather than postponing them to the top-level WHERE
clause.  This produces nicer SQL, anyway.

This bug was introduced in e4106b2528727c4b48639c0e12bf2f70a766b910.

Ashutosh Bapat, per report from Rajkumar Raghuwanshi.

8 years agoHonor PGCTLTIMEOUT environment variable for pg_regress' startup wait.
Tom Lane [Thu, 21 Apr 2016 03:48:13 +0000 (23:48 -0400)]
Honor PGCTLTIMEOUT environment variable for pg_regress' startup wait.

In commit 2ffa86962077c588 we made pg_ctl recognize an environment variable
PGCTLTIMEOUT to set the default timeout for starting and stopping the
postmaster.  However, pg_regress uses pg_ctl only for the "stop" end of
that; it has bespoke code for starting the postmaster, and that code has
historically had a hard-wired 60-second timeout.  Further buildfarm
experience says it'd be a good idea if that timeout were also controlled
by PGCTLTIMEOUT, so let's make it so.  Like the previous patch, back-patch
to all active branches.

Discussion: <13969.1461191936@sss.pgh.pa.us>

8 years agoAdd pg_dump support for the new PARALLEL option for aggregates.
Robert Haas [Thu, 21 Apr 2016 02:47:20 +0000 (22:47 -0400)]
Add pg_dump support for the new PARALLEL option for aggregates.

This was an oversight in commit 41ea0c23761ca108e2f08f6e3151e3cb1f9652a1.

Fabrízio de Royes Mello, per a report from Tushar Ahuja

8 years agoForbid parallel Hash Right Join or Hash Full Join.
Robert Haas [Wed, 20 Apr 2016 21:48:55 +0000 (17:48 -0400)]
Forbid parallel Hash Right Join or Hash Full Join.

That won't work.  You'll get bogus null-extended rows.

Mithun Cy

8 years agoUpdate backup documentation for new APIs
Magnus Hagander [Wed, 20 Apr 2016 18:40:04 +0000 (14:40 -0400)]
Update backup documentation for new APIs

This includes the rest of the documentation that was not included
in 7117685. A larger restructure would still be wanted, but with
this commit the documentation of the new features is complete.

8 years agoFix memory leak and other bugs in ginPlaceToPage() & subroutines.
Tom Lane [Wed, 20 Apr 2016 18:25:15 +0000 (14:25 -0400)]
Fix memory leak and other bugs in ginPlaceToPage() & subroutines.

Commit 36a35c550ac114ca turned the interface between ginPlaceToPage and
its subroutines in gindatapage.c and ginentrypage.c into a royal mess:
page-update critical sections were started in one place and finished in
another place not even in the same file, and the very same subroutine
might return having started a critical section or not.  Subsequent patches
band-aided over some of the problems with this design by making things
even messier.

One user-visible resulting problem is memory leaks caused by the need for
the subroutines to allocate storage that would survive until ginPlaceToPage
calls XLogInsert (as reported by Julien Rouhaud).  This would not typically
be noticeable during retail index updates.  It could be visible in a GIN
index build, in the form of memory consumption swelling to several times
the commanded maintenance_work_mem.

Another rather nasty problem is that in the internal-page-splitting code
path, we would clear the child page's GIN_INCOMPLETE_SPLIT flag well before
entering the critical section that it's supposed to be cleared in; a
failure in between would leave the index in a corrupt state.  There were
also assorted coding-rule violations with little immediate consequence but
possible long-term hazards, such as beginning an XLogInsert sequence before
entering a critical section, or calling elog(DEBUG) inside a critical
section.

To fix, redefine the API between ginPlaceToPage() and its subroutines
by splitting the subroutines into two parts.  The "beginPlaceToPage"
subroutine does what can be done outside a critical section, including
full computation of the result pages into temporary storage when we're
going to split the target page.  The "execPlaceToPage" subroutine is called
within a critical section established by ginPlaceToPage(), and it handles
the actual page update in the non-split code path.  The critical section,
as well as the XLOG insertion call sequence, are both now always started
and finished in ginPlaceToPage().  Also, make ginPlaceToPage() create and
work in a short-lived memory context to eliminate the leakage problem.
(Since a short-lived memory context had been getting created in the most
common code path in the subroutines, this shouldn't cause any noticeable
performance penalty; we're just moving the overhead up one call level.)

In passing, fix a bunch of comments that had gone unmaintained throughout
all this klugery.

Report: <571276DD.5050303@dalibo.com>

8 years agoRevert no-op changes to BufferGetPage()
Kevin Grittner [Wed, 20 Apr 2016 13:31:19 +0000 (08:31 -0500)]
Revert no-op changes to BufferGetPage()

The reverted changes were intended to force a choice of whether any
newly-added BufferGetPage() calls needed to be accompanied by a
test of the snapshot age, to support the "snapshot too old"
feature.  Such an accompanying test is needed in about 7% of the
cases, where the page is being used as part of a scan rather than
positioning for other purposes (such as DML or vacuuming).  The
additional effort required for back-patching, and the doubt whether
the intended benefit would really be there, have indicated it is
best just to rely on developers to do the right thing based on
comments and existing usage, as we do with many other conventions.

This change should have little or no effect on generated executable
code.

Motivated by the back-patching pain of Tom Lane and Robert Haas

8 years agoImprove regression tests for degree-based trigonometric functions.
Tom Lane [Tue, 19 Apr 2016 20:47:21 +0000 (16:47 -0400)]
Improve regression tests for degree-based trigonometric functions.

Print the actual value of each function result that's expected to be exact,
rather than merely emitting a NULL if it's not right.  Although we print
these with extra_float_digits = 3, we should not trust that the platform
will produce a result visibly different from the expected value if it's off
only in the last place; hence, also include comparisons against the exact
values as before.  This is a bit bulkier and uglier than the previous
printout, but it will provide more information and be easier to interpret
if there's a test failure.

Discussion: <18241.1461073100@sss.pgh.pa.us>

8 years agoMake partition-lock-release coding more transparent in BufferAlloc().
Tom Lane [Mon, 18 Apr 2016 22:05:56 +0000 (18:05 -0400)]
Make partition-lock-release coding more transparent in BufferAlloc().

Coverity complained that oldPartitionLock was possibly dereferenced after
having been set to NULL.  That actually can't happen, because we'd only use
it if (oldFlags & BM_TAG_VALID) is true.  But nonetheless Coverity is
justified in complaining, because at line 1275 we actually overwrite
oldFlags, and then still expect its BM_TAG_VALID bit to be a safe guide to
whether to release the oldPartitionLock.  Thus, the code would be incorrect
if someone else had changed the buffer's BM_TAG_VALID flag meanwhile.
That should not happen, since we hold pin on the buffer throughout this
sequence, but it's starting to look like a rather shaky chain of logic.
And there's no need for such assumptions, because we can simply replace
the (oldFlags & BM_TAG_VALID) tests with (oldPartitionLock != NULL),
which has identical results and makes it plain to all comers that we don't
dereference a null pointer.  A small side benefit is that the range of
liveness of oldFlags is greatly reduced, possibly allowing the compiler
to save a register.

This is just cleanup, not an actual bug fix, so there seems no need
for a back-patch.

8 years agoFurther reduce the number of semaphores used under --disable-spinlocks.
Tom Lane [Mon, 18 Apr 2016 17:33:06 +0000 (13:33 -0400)]
Further reduce the number of semaphores used under --disable-spinlocks.

Per discussion, there doesn't seem to be much value in having
NUM_SPINLOCK_SEMAPHORES set to 1024: under any scenario where you are
running more than a few backends concurrently, you really had better have a
real spinlock implementation if you want tolerable performance.  And 1024
semaphores is a sizable fraction of the system-wide SysV semaphore limit
on many platforms.  Therefore, reduce this setting's default value to 128
to make it less likely to cause out-of-semaphores problems.

8 years agoFix typo in docs.
Fujii Masao [Mon, 18 Apr 2016 04:35:21 +0000 (13:35 +0900)]
Fix typo in docs.

Artur Zakirov

8 years agodoc: Document that sequences can also be extension configuration tables
Peter Eisentraut [Mon, 18 Apr 2016 03:13:45 +0000 (23:13 -0400)]
doc: Document that sequences can also be extension configuration tables

From: Michael Paquier <michael.paquier@gmail.com>

8 years agoAvoid code duplication in \crosstabview.
Tom Lane [Sun, 17 Apr 2016 15:37:58 +0000 (11:37 -0400)]
Avoid code duplication in \crosstabview.

In commit 6f0d6a507 I added a duplicate copy of psqlscanslash's identifier
downcasing code, but actually it's not hard to split that out as a callable
subroutine and avoid the duplication.

8 years agoAdjust spin.c's spinlock emulation so that 0 is not a valid spinlock value.
Tom Lane [Sat, 16 Apr 2016 23:53:38 +0000 (19:53 -0400)]
Adjust spin.c's spinlock emulation so that 0 is not a valid spinlock value.

We've had repeated troubles over the years with failures to initialize
spinlocks correctly; see 6b93fcd14 for a recent example.  Most of the time,
on most platforms, such oversights can escape notice because all-zeroes is
the expected initial content of an slock_t variable.  The only platform
we have where the initialized state of an slock_t isn't zeroes is HPPA,
and that's practically gone in the wild.  To make it easier to catch such
errors without needing one of those, adjust the --disable-spinlocks code
so that zero is not a valid value for an slock_t for it.

In passing, remove a bunch of unnecessary #include's from spin.c;
commit daa7527afc227443 removed all the intermodule coupling that
made them necessary.

8 years agodoc: Change some "user" to "role" for consistency in the section
Peter Eisentraut [Sat, 16 Apr 2016 16:54:56 +0000 (12:54 -0400)]
doc: Change some "user" to "role" for consistency in the section

suggested by Johannes Choo

8 years agodoc: Markup improvement
Peter Eisentraut [Sat, 16 Apr 2016 16:49:36 +0000 (12:49 -0400)]
doc: Markup improvement

8 years agoDisallow creation of indexes on system columns (except for OID).
Tom Lane [Sat, 16 Apr 2016 16:11:41 +0000 (12:11 -0400)]
Disallow creation of indexes on system columns (except for OID).

Although OID acts pretty much like user data, the other system columns do
not, so an index on one would likely misbehave.  And it's pretty hard to
see a use-case for one, anyway.  Let's just forbid the case rather than
worry about whether it should be supported.

David Rowley

8 years agoIn recordExtensionInitPriv(), keep the scan til we're done with it
Stephen Frost [Sat, 16 Apr 2016 01:57:15 +0000 (21:57 -0400)]
In recordExtensionInitPriv(), keep the scan til we're done with it

For reasons of sheer brain fade, we (I) was calling systable_endscan()
immediately after systable_getnext() and expecting the tuple returned
by systable_getnext() to still be valid.

That's clearly wrong.  Move the systable_endscan() down below the tuple
usage.

Discovered initially by Pavel Stehule and then also by Alvaro.

Add a regression test based on Alvaro's testing.

8 years agodoc: Add missing parentheses
Peter Eisentraut [Sat, 16 Apr 2016 00:44:10 +0000 (20:44 -0400)]
doc: Add missing parentheses

From: Alexander Law <exclusion@gmail.com>

8 years agopsql: Add new gettext trigger
Peter Eisentraut [Sat, 16 Apr 2016 00:23:41 +0000 (20:23 -0400)]
psql: Add new gettext trigger

8 years agoUse less-generic names in matview.sql.
Tom Lane [Fri, 15 Apr 2016 17:04:17 +0000 (13:04 -0400)]
Use less-generic names in matview.sql.

The original coding of this test used table and view names like "t",
"tv", "foo", etc.  This tended to interfere with doing simple manual
tests in the regression database; not to mention that it posed a
considerable risk of conflict with other regression test scripts.
Prefix these names with "mvtest_" to avoid such conflicts.

Also, change transiently-created role name to be "regress_xxx" per
discussions about being careful with regression-test role creation.

8 years agoFix possible crash in ALTER TABLE ... REPLICA IDENTITY USING INDEX.
Tom Lane [Fri, 15 Apr 2016 16:11:27 +0000 (12:11 -0400)]
Fix possible crash in ALTER TABLE ... REPLICA IDENTITY USING INDEX.

Careless coding added by commit 07cacba983ef79be could result in a crash
or a bizarre error message if someone tried to select an index on the
OID column as the replica identity index for a table.  Back-patch to 9.4
where the feature was introduced.

Discussion: CAKJS1f8TQYgTRDyF1_u9PVCKWRWz+DkieH=U7954HeHVPJKaKg@mail.gmail.com

David Rowley

8 years agopostgres_fdw: Clean up handling of system columns.
Robert Haas [Fri, 15 Apr 2016 15:58:56 +0000 (11:58 -0400)]
postgres_fdw: Clean up handling of system columns.

Previously, querying the xmin column of a single postgres_fdw foreign
table fetched the tuple length, xmax the typmod, and cmin or cmax the
composite type OID of the tuple.  However, when you queried several
such tables and the join got shipped to the remote side, these columns
ended up containing the remote values of the corresponding columns.
Both behaviors are rather unprincipled, the former for obvious reasons
and the latter because the remote values of these columns don't have
any local significance; our transaction IDs are in a different space
than those of the remote machine.  Clean this up by setting all of
these fields to 0 in both cases.  Also fix the handling of tableoid
to be sane.

Robert Haas and Ashutosh Bapat, reviewed by Etsuro Fujita.

8 years agoTweak EXPLAIN for parallel query to show workers launched.
Robert Haas [Fri, 15 Apr 2016 15:49:41 +0000 (11:49 -0400)]
Tweak EXPLAIN for parallel query to show workers launched.

The previous display was sort of confusing, because it didn't
distinguish between the number of workers that we planned to launch
and the number that actually got launched.  This has already confused
several people, so display both numbers and label them clearly.

Julien Rouhaud, reviewed by me.

8 years agoFix portability problem induced by commit a6f6b7819.
Tom Lane [Fri, 15 Apr 2016 14:44:28 +0000 (10:44 -0400)]
Fix portability problem induced by commit a6f6b7819.

pg_xlogdump includes bufmgr.h.  With a compiler that emits code for
static inline functions even when they're unreferenced, that leads
to unresolved external references in the new static-inline version
of BufferGetPage().  So hide it with #ifndef FRONTEND, as we've done
for similar issues elsewhere.  Per buildfarm member pademelon.

8 years agoFix typo in comment
Magnus Hagander [Fri, 15 Apr 2016 11:32:54 +0000 (13:32 +0200)]
Fix typo in comment

8 years agoUpdate helptext for vcregress.pl
Magnus Hagander [Fri, 15 Apr 2016 08:04:10 +0000 (10:04 +0200)]
Update helptext for vcregress.pl

This has clearly not been tracking the code changse for quite some time.

Michael Paquier, problem spotted by Kyotaro HORIGUCHI

8 years agoMake regression test for multiple synchronous standbys more stable.
Fujii Masao [Fri, 15 Apr 2016 04:58:14 +0000 (13:58 +0900)]
Make regression test for multiple synchronous standbys more stable.

The regression test checks whether the output of pg_stat_replication is
expected or not after changing synchronous_standby_names and reloading
the configuration file. Regarding this test logic, previously there was
a timing issue which made the test result unstable. That is,
pg_stat_replication could return unexpected result during small window
after the configuration file was reloaded before new setting value
took effect, and which made the test fail.

This commit changes the test logic so that it uses a loop with a timeout
to give some room for the test to pass. Now the test fails only when
pg_stat_replication keeps returning unexpected result for 30 seconds.

Michael Paquier

8 years agoFix memory leak in GIN index scans.
Tom Lane [Fri, 15 Apr 2016 04:02:26 +0000 (00:02 -0400)]
Fix memory leak in GIN index scans.

The code had a query-lifespan memory leak when encountering GIN entries
that have posting lists (rather than posting trees, ie, there are a
relatively small number of heap tuples containing this index key value).
With a suitable data distribution this could add up to a lot of leakage.
Problem seems to have been introduced by commit 36a35c550, so back-patch
to 9.4.

Julien Rouhaud

8 years agoRethink \crosstabview's argument parsing logic.
Tom Lane [Fri, 15 Apr 2016 02:54:26 +0000 (22:54 -0400)]
Rethink \crosstabview's argument parsing logic.

\crosstabview interpreted its arguments in an unusual way, including
doing case-insensitive matching of unquoted column names, which is
surely not the right thing.  Rip that out in favor of doing something
equivalent to the dequoting/case-folding rules used by other psql
commands.  To keep it simple, change the syntax so that the optional
sort column is specified as a separate argument, instead of the
also-quite-unusual syntax that attached it to the colH argument with
a colon.

Also, rework the error messages to be closer to project style.

8 years agoMake init_spin_delay() C89 compliant #2.
Andres Freund [Fri, 15 Apr 2016 02:26:13 +0000 (19:26 -0700)]
Make init_spin_delay() C89 compliant #2.

My previous attempt at doing so, in 80abbeba23, was not sufficient. While that
fixed the problem for bufmgr.c and lwlock.c , s_lock.c still has non-constant
expressions in the struct initializer, because the file/line/function
information comes from the caller of s_lock().

Give up on using a macro, and use a static inline instead.

Discussion: 4369.1460435533@sss.pgh.pa.us

8 years agoRemove trailing commas in enums.
Andres Freund [Fri, 15 Apr 2016 01:54:06 +0000 (18:54 -0700)]
Remove trailing commas in enums.

These aren't valid C89. Found thanks to gcc's -Wc90-c99-compat. These
exist in differing places in most supported branches.

8 years agoFix trivial typo.
Andres Freund [Thu, 14 Apr 2016 00:47:41 +0000 (17:47 -0700)]
Fix trivial typo.

8 years agoFix core dump in ReorderBufferRestoreChange on alignment-picky platforms.
Tom Lane [Thu, 14 Apr 2016 23:42:21 +0000 (19:42 -0400)]
Fix core dump in ReorderBufferRestoreChange on alignment-picky platforms.

When re-reading an update involving both an old tuple and a new tuple from
disk, reorderbuffer.c was careless about whether the new tuple is suitably
aligned for direct access --- in general, it isn't.  We'd missed seeing
this in the buildfarm because the contrib/test_decoding tests exercise this
code path only a few times, and by chance all of those cases have old
tuples with length a multiple of 4, which is usually enough to make the
access to the new tuple's t_len safe.  For some still-not-entirely-clear
reason, however, Debian's sparc build gets a bus error, as reported by
Christoph Berg; perhaps it's assuming 8-byte alignment of the pointer?

The lack of previous field reports is probably because you need all of
these conditions to trigger a crash: an alignment-picky platform (not
Intel), a transaction large enough to spill to disk, an update within
that xact that changes a primary-key field and has an odd-length old tuple,
and of course logical decoding tracing the transaction.

Avoid the alignment assumption by using memcpy instead of fetching t_len
directly, and add a test case that exposes the crash on picky platforms.
Back-patch to 9.4 where the bug was introduced.

Discussion: <20160413094117.GC21485@msg.credativ.de>

8 years agoAdjust signature of walrcv_receive hook.
Tom Lane [Thu, 14 Apr 2016 17:49:37 +0000 (13:49 -0400)]
Adjust signature of walrcv_receive hook.

Commit 314cbfc5da988eff redefined the signature of this hook as
typedef int (*walrcv_receive_type) (char **buffer, int *wait_fd);

But in fact the type of the "wait_fd" variable ought to be pgsocket,
which is what WaitLatchOrSocket expects, and which is necessary if
we want to be able to assign PGINVALID_SOCKET to it on Windows.
So fix that.

8 years agoAdjust datatype of ReplicationState.acquired_by.
Tom Lane [Thu, 14 Apr 2016 16:18:09 +0000 (12:18 -0400)]
Adjust datatype of ReplicationState.acquired_by.

It was declared as "pid_t", which would be fine except that none of
the places that printed it in error messages took any thought for the
possibility that it's not equivalent to "int".  This leads to warnings
on some buildfarm members, and could possibly lead to actually wrong
error messages on those platforms.  There doesn't seem to be any very
good reason not to just make it "int"; it's only ever assigned from
MyProcPid, which is int.  If we want to cope with PIDs that are wider
than int, this is not the place to start.

Also, fix the comment, which seems to perhaps be a leftover from a time
when the field was only a bool?

Per buildfarm.  Back-patch to 9.5 which has same issue.

8 years agoDocs: clarify description of LIMIT/OFFSET behavior.
Tom Lane [Thu, 14 Apr 2016 14:57:29 +0000 (10:57 -0400)]
Docs: clarify description of LIMIT/OFFSET behavior.

Section 7.6 was a tad confusing because it specified what LIMIT NULL
does, but neglected to do the same for OFFSET NULL, making this look
like perhaps a special case or a wrong restatement of the bit about
LIMIT ALL.  Wordsmith a bit while at it.  Per bug #14084.

8 years agoFix prototype of pgwin32_bind().
Tom Lane [Thu, 14 Apr 2016 13:44:21 +0000 (09:44 -0400)]
Fix prototype of pgwin32_bind().

I (tgl) had copied-and-pasted this from pgwin32_accept(), failing to
notice that the third parameter should be "int" not "int *".

David Rowley

8 years agoFix broken dependency-mongering for index operator classes/families.
Tom Lane [Thu, 14 Apr 2016 03:33:31 +0000 (23:33 -0400)]
Fix broken dependency-mongering for index operator classes/families.

For a long time, opclasscmds.c explained that "we do not create a
dependency link to the AM [for an opclass or opfamily], because we don't
currently support DROP ACCESS METHOD".  Commit 473b93287040b200 invented
DROP ACCESS METHOD, but it batted only 1 for 2 on adding the dependency
links, and 0 for 2 on updating the comments about the topic.

In passing, undo the same commit's entirely inappropriate decision to
blow away an existing index as a side-effect of create_am.sql.

8 years agoFix duplicated index entry in doc.
Fujii Masao [Thu, 14 Apr 2016 02:17:41 +0000 (11:17 +0900)]
Fix duplicated index entry in doc.

Commit cfe96ae corrected the name of pg_logical_emit_message()
in its index entry. But this typo fix caused duplicated index
entry because there was another index entry for the function.

Spotted by Tom Lane.

8 years agoDisallow SET SESSION AUTHORIZATION pg_*
Stephen Frost [Thu, 14 Apr 2016 01:31:24 +0000 (21:31 -0400)]
Disallow SET SESSION AUTHORIZATION pg_*

As part of reserving the pg_* namespace for default roles and in line
with SET ROLE and other previous efforts, disallow settings the role
to a default/reserved role using SET SESSION AUTHORIZATION.

These checks and restrictions on what is allowed regarding default /
reserved roles are under debate, but it seems prudent to ensure that
the existing checks at least cover the intended cases while the
debate rages on.  On me to clean it up if the consensus decision is
to remove these checks.

8 years agoAdd required database and origin filtering for logical messages.
Andres Freund [Thu, 14 Apr 2016 00:38:54 +0000 (17:38 -0700)]
Add required database and origin filtering for logical messages.

Logical messages, added in 3fe3511d05, during decoding failed to filter
messages emitted in other databases and messages emitted "under" a
replication origin the output plugin isn't interested in.

Add tests to verify that both types of filtering actually work. While
touching message.sql remove hunk obsoleted by d25379e.

Bump XLOG_PAGE_MAGIC because xl_logical_message changed and because
3fe3511d05 had omitted doing so. 3fe3511d05 additionally didn't bump
catversion, but 7a542700d has done so since.

Author: Petr Jelinek
Reported-By: Andres Freund
Discussion: 20160406142513.wotqy3ba3kanr423@alap3.anarazel.de

8 years agoMake init_spin_delay() C89 compliant and change stuck spinlock reporting.
Andres Freund [Wed, 13 Apr 2016 23:42:01 +0000 (16:42 -0700)]
Make init_spin_delay() C89 compliant and change stuck spinlock reporting.

The current definition of init_spin_delay (introduced recently in
48354581a) wasn't C89 compliant. It's not legal to refer to refer to
non-constant expressions, and the ptr argument was one.  This, as
reported by Tom, lead to a failure on buildfarm animal pademelon.

The pointer, especially on system systems with ASLR, isn't super helpful
anyway, though. So instead of making init_spin_delay into an inline
function, make s_lock_stuck() report the function name in addition to
file:line and change init_spin_delay() accordingly. While not a direct
replacement, the function name is likely more useful anyway (line
numbers are often hard to interpret in third party reports).

This also fixes what file/line number is reported for waits via
s_lock().

As PG_FUNCNAME_MACRO is now used outside of elog.h, move it to c.h.

Reported-By: Tom Lane
Discussion: 4369.1460435533@sss.pgh.pa.us

8 years agoFix pg_dump so pg_upgrade'ing an extension with simple opfamilies works.
Tom Lane [Wed, 13 Apr 2016 22:57:52 +0000 (18:57 -0400)]
Fix pg_dump so pg_upgrade'ing an extension with simple opfamilies works.

As reported by Michael Feld, pg_upgrade'ing an installation having
extensions with operator families that contain just a single operator class
failed to reproduce the extension membership of those operator families.
This caused no immediate ill effects, but would create problems when later
trying to do a plain dump and restore, because the seemingly-not-part-of-
the-extension operator families would appear separately in the pg_dump
output, and then would conflict with the families created by loading the
extension.  This has been broken ever since extensions were introduced,
and many of the standard contrib extensions are affected, so it's a bit
astonishing nobody complained before.

The cause of the problem is a perhaps-ill-considered decision to omit
such operator families from pg_dump's output on the grounds that the
CREATE OPERATOR CLASS commands could recreate them, and having explicit
CREATE OPERATOR FAMILY commands would impede loading the dump script into
pre-8.3 servers.  Whatever the merits of that decision when 8.3 was being
written, it looks like a poor tradeoff now.  We can fix the pg_upgrade
problem simply by removing that code, so that the operator families are
dumped explicitly (and then will be properly made to be part of their
extensions).

Although this fixes the behavior of future pg_upgrade runs, it does nothing
to clean up existing installations that may have improperly-linked operator
families.  Given the small number of complaints to date, maybe we don't
need to worry about providing an automated solution for that; anyone who
needs to clean it up can do so with manual "ALTER EXTENSION ADD OPERATOR
FAMILY" commands, or even just ignore the duplicate-opfamily errors they
get during a pg_restore.  In any case we need this fix.

Back-patch to all supported branches.

Discussion: <20228.1460575691@sss.pgh.pa.us>

8 years agoAvoid atomic operation in MarkLocalBufferDirty().
Andres Freund [Wed, 13 Apr 2016 22:28:29 +0000 (15:28 -0700)]
Avoid atomic operation in MarkLocalBufferDirty().

The recent patch to make Pin/UnpinBuffer lockfree in the hot
path (48354581a), accidentally used pg_atomic_fetch_or_u32() in
MarkLocalBufferDirty(). Other code operating on local buffers was
careful to only use pg_atomic_read/write_u32 which just read/write from
memory; to avoid unnecessary overhead.

On its own that'd just make MarkLocalBufferDirty() slightly less
efficient, but in addition InitLocalBuffers() doesn't call
pg_atomic_init_u32() - thus the spinlock fallback for the atomic
operations isn't initialized. That in turn caused, as reported by Tom,
buildfarm animal gaur to fail.  As those errors are actually useful
against this type of error, continue to omit - intentionally this time -
initialization of the atomic variable.

In addition, add an explicit note about only using pg_atomic_read/write
on local buffers's state to BufferDesc's description.

Reported-By: Tom Lane
Discussion: 1881.1460431476@sss.pgh.pa.us

8 years agoWiden amount-to-flush arguments of FileWriteback and callers.
Tom Lane [Wed, 13 Apr 2016 22:12:06 +0000 (18:12 -0400)]
Widen amount-to-flush arguments of FileWriteback and callers.

It's silly to define these counts as narrower than they might someday
need to be.  Also, I believe that the BLCKSZ * nflush calculation in
mdwriteback was capable of overflowing an int.

8 years agoFix assorted portability issues with using msync() for data flushing.
Tom Lane [Wed, 13 Apr 2016 21:17:51 +0000 (17:17 -0400)]
Fix assorted portability issues with using msync() for data flushing.

Commit 428b1d6b29ca599c5700d4bc4f4ce4c5880369bf introduced the use of
msync() for flushing dirty data from the kernel's file buffers.  Several
portability issues were overlooked, though:

* Not all implementations of mmap() think that nbytes == 0 means "map
the whole file".  To fix, use lseek() to find out the true length.
Fix callers of pg_flush_data to be aware that nbytes == 0 may result
in trashing the file's seek position.

* Not all implementations of mmap() will accept partial-page mmap
requests.  To fix, round down the length request to whatever sysconf()
says the page size is.  (I think this is OK from a portability standpoint,
because sysconf() is required by SUS v2, and we aren't trying to compile
this part on Windows anyway.  Buildfarm should let us know if not.)

* On 32-bit machines, the file size might exceed the available free
address space, or even exceed what will fit in size_t.  Check for
the latter explicitly to avoid passing a false request size to mmap().
If mmap fails, silently fall through to the next implementation method,
rather than bleating to the postmaster log and giving up.

* mmap'ing directories fails on some platforms, and even if it works,
msync'ing the directory is quite unlikely to help, as for that matter are
the other flush implementations.  In pre_sync_fname(), just skip flush
attempts on directories.

In passing, copy-edit the comments a bit.

Stas Kelvich and myself

8 years agoImprove documentation for \crosstabview.
Tom Lane [Wed, 13 Apr 2016 15:49:47 +0000 (11:49 -0400)]
Improve documentation for \crosstabview.

Fix misleading syntax summary (there cannot be a space between colH and
scolH).  Provide a link from the existing crosstab() function's
documentation to \crosstabview.  Copy-edit the command's description.

Christoph Berg and Tom Lane

8 years agoUse PG_INT32_MIN instead of reiterating the constant.
Robert Haas [Wed, 13 Apr 2016 11:53:54 +0000 (07:53 -0400)]
Use PG_INT32_MIN instead of reiterating the constant.

Makes no difference, but it's cleaner this way.

Michael Paquier

8 years agoProvide errno-translation wrappers around bind() and listen() on Windows.
Tom Lane [Tue, 12 Apr 2016 23:52:21 +0000 (19:52 -0400)]
Provide errno-translation wrappers around bind() and listen() on Windows.

I've seen one too many "could not bind IPv4 socket: No error" log entries
from the Windows buildfarm members.  Per previous discussion, this is
likely caused by the fact that we're doing nothing to translate
WSAGetLastError() to errno.  Put in a wrapper layer to do that.

If this works as expected, it should get back-patched, but let's see what
happens in the buildfarm first.

Discussion: <4065.1452450340@sss.pgh.pa.us>

8 years agoFix costing for parallel aggregation.
Robert Haas [Tue, 12 Apr 2016 20:24:55 +0000 (16:24 -0400)]
Fix costing for parallel aggregation.

The original patch kind of ignored the fact that we were doing something
different from a costing point of view, but nobody noticed.  This patch
fixes that oversight.

David Rowley

8 years agoRemove unused function GetOldestWALSendPointer from walsender code.
Fujii Masao [Tue, 12 Apr 2016 19:36:29 +0000 (04:36 +0900)]
Remove unused function GetOldestWALSendPointer from walsender code.

That unused function was introduced as a sample because synchronous
replication or replication monitoring tools might need it in the future.
Recently commit 989be08 added the function SyncRepGetOldestSyncRecPtr
which provides almost the same functionality for multiple synchronous
standbys feature. So it's time to remove that unused sample function.
This commit does that.

8 years agoRedefine create_upper_paths_hook as being invoked once per upper relation.
Tom Lane [Tue, 12 Apr 2016 19:23:14 +0000 (15:23 -0400)]
Redefine create_upper_paths_hook as being invoked once per upper relation.

Per discussion, this gives potential users of the hook more flexibility,
because they can build custom Paths that implement only one stage of
upper processing atop core-provided Paths for earlier stages.

8 years agoImprove coding of column-name parsing in psql's new crosstabview.c.
Tom Lane [Tue, 12 Apr 2016 16:52:35 +0000 (12:52 -0400)]
Improve coding of column-name parsing in psql's new crosstabview.c.

Coverity complained about this code, not without reason because it was
rather messy.  Adjust it to not scribble on the passed string; that adds
one malloc/free cycle per column name, which is going to be insignificant
in context.  We can actually const-ify both the string argument and the
PGresult.

Daniel Verité, with some further cleanup by me

8 years agoAvoid extra locks in GetSnapshotData if old_snapshot_threshold < 0
Kevin Grittner [Tue, 12 Apr 2016 16:48:02 +0000 (11:48 -0500)]
Avoid extra locks in GetSnapshotData if old_snapshot_threshold < 0

On a big NUMA machine with 1000 connections in saturation load
there was a performance regression due to spinlock contention, for
acquiring values which were never used.  Just fill with dummy
values if we're not going to use them.

This patch has not been benchmarked yet on a big NUMA machine, but
it seems like a good idea on general principle, and it seemed to
prevent an apparent 2.2% regression on a single-socket i7 box
running 200 connections at saturation load.

8 years agoImprove API of GenericXLogRegister().
Tom Lane [Tue, 12 Apr 2016 15:42:06 +0000 (11:42 -0400)]
Improve API of GenericXLogRegister().

Rename this function to GenericXLogRegisterBuffer() to make it clearer
what it does, and leave room for other sorts of "register" actions in
future.  Also, replace its "bool isNew" argument with an integer flags
argument, so as to allow adding more flags in future without an API
break.

Alexander Korotkov, adjusted slightly by me

8 years agoIn generic WAL application and replay, ensure page "hole" is always zero.
Tom Lane [Tue, 12 Apr 2016 15:13:52 +0000 (11:13 -0400)]
In generic WAL application and replay, ensure page "hole" is always zero.

The previous coding could allow the contents of the "hole" between pd_lower
and pd_upper to diverge during replay from what it had been when the update
was originally applied.  This would pose a problem if checksums were in
use, and in any case would complicate forensic comparisons between master
and slave servers.  So force the "hole" to contain zeroes, both at initial
application of a generically-logged action, and at replay.

Alexander Korotkov, adjusted slightly by me

8 years agoAdd page id to bloom index
Teodor Sigaev [Tue, 12 Apr 2016 15:03:01 +0000 (18:03 +0300)]
Add page id to bloom index

Added to ensure that bloom index pages can be distinguished from other pages
by pg_filedump. Because there wasn't any public/production versions before,
it doesn't pay attention to any compatibility issues.

Per notice from Tom Lane

8 years agoRemove unnecessary definition of _WIN64 in libpq/win32.mak.
Tom Lane [Tue, 12 Apr 2016 14:52:58 +0000 (10:52 -0400)]
Remove unnecessary definition of _WIN64 in libpq/win32.mak.

In commit b0e40d189325dc7a54d2546245e766f8c47a7c8d, I should have just
removed the /D switch defining WIN64.  The reason the code worked before
is that all Windows64 compilers automatically predefine _WIN64.  Perhaps
at one time we had code that depended on WIN64 being defined, but it's
long gone, and we should not encourage any reappearance.  Per discussion
with Christian Ullrich.

8 years agoCorrect copyright for newly added genericdesc.c
Stephen Frost [Tue, 12 Apr 2016 12:45:09 +0000 (08:45 -0400)]
Correct copyright for newly added genericdesc.c

It's 2016 these days (no, not entirely sure how we got here either).

Pointed out by Amit Langote

8 years agoFix whitespace
Peter Eisentraut [Tue, 12 Apr 2016 00:59:04 +0000 (20:59 -0400)]
Fix whitespace

8 years agoFix _SPI_execute_plan() for CREATE TABLE IF NOT EXISTS foo AS ...
Tom Lane [Tue, 12 Apr 2016 00:07:17 +0000 (20:07 -0400)]
Fix _SPI_execute_plan() for CREATE TABLE IF NOT EXISTS foo AS ...

When IF NOT EXISTS was added to CREATE TABLE AS, this logic didn't get
the memo, possibly resulting in an Assert failure.  It looks like there
would have been no ill effects in a non-Assert build, though.  Back-patch
to 9.5 where the IF NOT EXISTS option was added.

Stas Kelvich

8 years agoFix two places that thought Windows64 is indicated by WIN64 macro.
Tom Lane [Mon, 11 Apr 2016 23:37:04 +0000 (19:37 -0400)]
Fix two places that thought Windows64 is indicated by WIN64 macro.

Everyplace else thinks it's _WIN64, so make these places fall in line.

The pg_regress.c usage is not going to result in any change in behavior,
only suppressing (or not) a compiler warning about downcasting HANDLEs.
So there seems no need for back-patching there.

The libpq/win32.mak usage might represent an actual bug, if anyone were
using this script to build for Windows64, which perhaps nobody is.
Given the lack of field complaints, no back-patch here either.

pg_regress.c problem found by Christian Ullrich, the other by me.

8 years agoFix freshly-introduced PL/Python portability bug.
Tom Lane [Mon, 11 Apr 2016 22:17:02 +0000 (18:17 -0400)]
Fix freshly-introduced PL/Python portability bug.

It turns out that those PyErr_Clear() calls I removed from plpy_elog.c
in 7e3bb080387f4143 et al were not quite as random as they appeared: they
mask a Python 2.3.x bug.  (Specifically, it turns out that PyType_Ready()
can fail if the error indicator is set on entry, and PLy_traceback's fetch
of frame.f_code may be the first operation in a session that requires the
"frame" type to be readied.  Ick.)  Put back the clear call, but in a more
centralized place closer to what it's protecting, and this time with a
comment warning what it's really for.

Per buildfarm member prairiedog.  Although prairiedog was only failing
on HEAD, it seems clearly possible for this to occur in older branches
as well, so back-patch to 9.2 the same as the previous patch.

8 years agoUse static inline function for BufferGetPage()
Kevin Grittner [Mon, 11 Apr 2016 21:47:50 +0000 (16:47 -0500)]
Use static inline function for BufferGetPage()

I was initially concerned that the some of the hundreds of
references to BufferGetPage() where the literal
BGP_NO_SNAPSHOT_TEST were passed might not optimize as well as a
macro, leading to some hard-to-find performance regressions in
corner cases.  Inspection of disassembled code has shown identical
code at all inspected locations, and the size difference doesn't
amount to even one byte per such call.  So make it readable.

Per gripes from Álvaro Herrera and Tom Lane