We have, for a very long time, allowed the same subplan (same member of the
PlannedStmt.subplans list) to be referenced by more than one SubPlan node;
this avoids problems for cases such as subplans within an IndexScan's
indxqual and indxqualorig fields.  However, EXPLAIN had not gotten the memo
and would print each reference as though it were an independent identical
subplan.  To fix, track plan_ids of subplans we've printed and don't print
the same plan_id twice.  Per report from Pavel Stehule.

BTW: the particular case of IndexScan didn't cause visible duplication
in a plain EXPLAIN, only EXPLAIN ANALYZE, because in the former case we
short-circuit executor startup before the indxqual field is processed by
ExecInitExpr.  That seems like it could easily lead to other EXPLAIN
problems in future, but it's not clear how to avoid it without breaking
the "EXPLAIN a plan using hypothetical indexes" use-case.  For now I've
left that issue alone.

Although this is a longstanding bug, it's purely cosmetic (no great harm
is done by the repeat printout) and we haven't had field complaints before.
So I'm hesitant to back-patch it, especially since there is some small risk
of ABI problems due to the need to add a new field to ExplainState.

In passing, rearrange order of fields in ExplainState to be less random,
and update some obsolete comments about when/where to initialize them.

Report: <CAFj8pRAimq+NK-menjt+3J4-LFoodDD8Or6=Lc_stcFD+eD4DA@mail.gmail.com>

Add display of proparallel (parallel-safety) when the server is >= 9.6,
and display of proacl (access privileges) for all server versions.
Minor tweak of column ordering to keep related columns together.

Michael Paquier

Discussion: <CAB7nPqTR3Vu3xKOZOYqSm-+bSZV0kqgeGAXD6w5GLbkbfd5Q6w@mail.gmail.com>

On a standby, ThisTimelineID is always 0, so we would generate a
filename in timeline 0 even for other timelines. Instead, use starttli
which we have retreived from the controlfile.

Report by: Francesco Canovai in bug #14230
Author: Marco Nenciarini
Reviewed by: Michael Paquier and Amit Kapila

This reverts commit 88cf37d2 as well
as follow-on commits ea9c4a16 and
c5756272.  We've learned about as much
as we can from the buildfarm.

Change assorted places in our Perl code that did things like
	system("prog $path/file");
to do it more like
	system('prog', "$path/file");
which is safe against spaces and other special characters in the path
variable.  The latter was already the prevailing style, but a few bits
of code hadn't gotten this memo.  Back-patch to 9.4 as relevant.

Michael Paquier, Kyotaro Horiguchi

Discussion: <20160704.160213.111134711.horiguchi.kyotaro@lab.ntt.co.jp>

Examination of the results from anole and gharial suggests that we're
only managing to track the size of one of the two stacks of IA64 machines.
Some googling gave the answer: on HPUX11, the register stack is reported
as a page type I don't see in pstat.h on my HPUX10 box.  Let's try
testing for that.

After a look at preliminary results from commit 88cf37d2,
I realized it'd be a good idea to spew out the maximum depth measurement
seen by check_stack_depth.  So add some quick-n-dirty code to do that.
Like the previous commit, this will be reverted once we've gathered
a set of buildfarm runs with it.

Fabien Coelho, some further wordsmithing by me

Alexander Law

Etsuro Fujita

This patch is meant to gather information from the buildfarm members, and
will be reverted in a day or so.  The idea is to try to find out the
high-water stack consumption while running the regression tests,
particularly on IA64 which is suspected to use much more stack than other
architectures.  On machines with pmap, we can use that; but the IA64 farm
members are running HPUX, so also include some bespoke code for HPUX.
(I've tested the latter on HPUX 10/HPPA; not entirely sure it will work
on HPUX 11/IA64, but we'll soon find out.)

Discussion: <CAM-w4HMwwcwaVvYcAH0_FGtG5GeXdYVRfvG81pXnSJWHnCfosQ@mail.gmail.com>

Pointed out by Alexander Law.

Author: Alexander Law

Amit Langote

Otherwise, when we abandon incremental memory accounting and use
batch allocation for the final merge pass, we might crash.  This
has been broken since 0011c009.

Peter Geoghegan, tested by Noah Misch

Peter Geoghegan

temp_file_limit is a per-process limit, not a per-session limit across
all cooperating parallel processes; change wording accordingly, per a
suggestion from Tom Lane.

Also, document under max_parallel_workers_per_gather the fact that each
process involved in a parallel query may use as many resources as a
separate session.  Caveat emptor.

Per a complaint from Peter Geoghegan.

While syncing our timezone code with IANA's updates in commit 1c1a7cbd,
I'd chosen not to adopt the code they conditionally compile under #ifdef
ALL_STATE.  The main thing that that drives is that the space for gmtime
and localtime timezone definitions isn't statically allocated, but is
malloc'd on first use.  I reasoned we didn't need that logic: we don't have
localtime() at all, and we always initialize TimeZone to GMT so we always
need that one.  But there is one other thing ALL_STATE does, which is to
make tzload() malloc its transient workspace instead of just declaring it
as a local variable.  It turns out that that local variable occupies 78K.
Even worse is that, at least for common US timezone settings, there's a
recursive call to parse the "posixrules" zone name, making peak stack
consumption to select a time zone upwards of 150K.  That's an uncomfortably
large fraction of our STACK_DEPTH_SLOP safety margin, and could result in
outright crashes if we try to reduce STACK_DEPTH_SLOP as has been discussed
recently.  Furthermore, this means that the postmaster's peak stack
consumption is several times that of a backend running typical queries
(since, except on Windows, backends inherit the timezone GUC values and
don't ever run this code themselves unless you do SET TIMEZONE).  That's
completely backwards from a safety perspective.

Hence, adopt the ALL_STATE rather than non-ALL_STATE variant of tzload(),
while not changing the other code aspects that symbol controls.  The
risk of an ENOMEM error from malloc() seems less than that of a SIGSEGV
from stack overrun.

This should probably get back-patched along with 1c1a7cbd and followon
fixes, whenever we decide we have enough confidence in the updates to do
that.

Per discussion on pgsql-hackers, conninfo is better as the column name
because it's more commonly used in PostgreSQL.

Catalog version bumped due to the change of pg_proc.

Author: Michael Paquier

Author: Masahiko Sawada

ExecInsertIndexTuples treated an exclusion constraint as subject to
noDupErr processing even when it was not listed in arbiterIndexes, and
would therefore not error out for a conflict in such a constraint, instead
returning it as an arbiter-index failure.  That led to an infinite loop in
ExecInsert, since ExecCheckIndexConstraints ignored the index as-intended
and therefore didn't throw the expected error.  To fix, make the exclusion
constraint code path use the same condition as the index_insert call does
to decide whether no-error-for-duplicates behavior is appropriate.  While
at it, refactor a little bit to avoid unnecessary list_member_oid calls.
(That surely wouldn't save anything worth noticing, but I find the code
a bit clearer this way.)

Per bug report from Heikki Rauhala.  Back-patch to 9.5 where ON CONFLICT
was introduced.

Report: <4C976D6B-76B4-434C-8052-D009F7B7AEDA@reaktor.fi>

There isn't really any reason not to; the original comments here were
partly confused about subplans versus subquery-in-FROM, and partly
dependent on restrictions that no longer apply now that subqueries return
Paths not Plans.  Depending on what's inside the subquery, it might fail
to produce any parallel_safe Paths, but that's fine.

Tom Lane and Robert Haas

The previous coding assumed that the value derived by
set_rel_consider_parallel() for an appendrel parent would be accurate for
all the appendrel's children; but this is not so, for example because one
child might scan a temp table.  Instead, apply set_rel_consider_parallel()
to each child rel as well as the parent, and then take the AND of the
results as controlling parallel safety for the appendrel as a whole.

(We might someday be able to deal more intelligently than this with cases
in which some of the childrels are parallel-safe and others not, but that's
for later.)

Robert Haas and Tom Lane

This was the intention all along, but an extraneous "return;" in
set_rel_consider_parallel() caused sampled rels to never be marked
consider_parallel.

Since we don't have any partial tablesample path/plan type yet, there's
no possibility of parallelizing the sample scan itself; but this fix
allows such a scan to appear below a parallel join, for example.

We were just leaving the cost fields zeroes, which produces obviously bogus
output with force_parallel_mode = on.  With force_parallel_mode = regress,
the zeroes are hidden, but I wonder if they wouldn't still confuse add-on
code such as auto_explain.

I'd been wondering why I was sometimes seeing fractional rowcount
estimates in parallel-query situations, and this seems to be the
reason.  (You won't see the fractional parts in EXPLAIN, because it
prints rowcounts with %.0f, but they are apparent in the debugger.)
A fractional rowcount is not any saner for a partial path than any
other kind of path, and it's equally likely to break cost estimation
for higher paths, so apply clamp_row_est() like we do in other places.

Instead of calling PLy_elog() for reporting Python argument parsing
errors, generate appropriate exceptions.  This matches the existing plpy
functions and is more consistent with the behavior of the Python
argument parsing routines.

In commit 915b703e I gave get_agg_clause_costs() the responsibility of
marking Aggref nodes with the appropriate aggtranstype.  I failed to notice
that where it was being called from, it might see only a subset of the
Aggref nodes that were in the original targetlist.  Specifically, if there
are duplicate aggregate calls in the tlist, either make_sort_input_target
or make_window_input_target might put just a single instance into the
grouping_target, and then only that instance would get marked.  Fix by
moving the call back into grouping_planner(), before we start building
assorted PathTargets from the query tlist.  Per report from Stefan Huehner.

Report: <20160702131056.GD3165@huehner.biz>

Document that index storage is dependent on the operating system's
collation library ordering, and any change in that ordering can create
invalid indexes.

Discussion: 20160617154311.GB19359@momjian.us

Backpatch-through: 9.1

In commit 68fa28f7 I tried to teach SS_finalize_plan() to cope with
initPlans attached anywhere in the plan tree, by dint of moving its
handling of those into the recursion in finalize_plan().  It turns out that
that doesn't really work: if a lower-level plan node emits an initPlan
output parameter in its targetlist, it's legitimate for upper levels to
reference those Params --- and at the point where this code runs, those
references look just like the Param itself, so finalize_plan() quite
properly rejects them as being in the wrong place.  We could lobotomize
the checks enough to allow that, probably, but then it's not clear that
we'd have any meaningful check for misplaced Params at all.  What seems
better, at least in the near term, is to tweak standard_planner() a bit
so that initPlans are never placed anywhere but the topmost plan node
for a query level, restoring the behavior that occurred pre-9.6.  Possibly
we can do better if this code is ever merged into setrefs.c: then it would
be possible to check a Param's placement only when we'd failed to replace
it with a Var referencing a child plan node's targetlist.

BTW, I'm now suspicious that finalize_plan is doing the wrong thing by
returning the node's allParam rather than extParam to be incorporated
in the parent node's set of used parameters.  However, it makes no
difference given that initPlans only appear at top level, so I'll leave
that alone for now.

Another thing that emerged from this is that standard_planner() needs
to check for initPlans before deciding that it's safe to stick a Gather
node on top in force_parallel_mode mode.  We previously guarded against
that by deciding the plan wasn't wholePlanParallelSafe if any subplans
had been found, but after commit 5ce5e4a1 it's necessary to have this
substitute test, because path parallel_safe markings don't account for
initPlans.  (Normally, we'd have decided the paths weren't safe anyway
due to appearances of SubPlan nodes, Params, or CTE scans somewhere in
the tree --- but it's possible for those all to be optimized away while
initPlans still remain.)

Per fuzz testing by Andreas Seltenreich.

Report: <874m89rw7x.fsf@credativ.de>

Author: Masahiko Sawada
Discussion: CAD21AoBD=Of1OzL90Xx4Q-3j=-2q7=S87cs75HfutE=eCday2w@mail.gmail.com

As of 9.6, pg_regress doesn't build unless storage/lwlocknames.h has been
created; but there was nothing forcing that to happen if you just went into
src/test/regress/ and built there.  We previously had a similar complaint
about plpython.

To fix in a way that won't break next time we invent a generated header,
make src/backend/Makefile expose a phony target for updating all the
include files it builds, and invoke that before building pg_regress or
plpython.  In principle, maybe we ought to invoke that everywhere; but
it would add a lot of usually-useless make cycles, so let's just do it
in the places where people have complained.

I made a couple of cosmetic adjustments in src/backend/Makefile as well,
to deal with the generated headers in consistent orders.

Michael Paquier and Tom Lane

Report: <31398.1467036827@sss.pgh.pa.us>
Report: <20150916200959.GB32090@msg.df7cb.de>

There are two problems in the original coding: one is that if one
walreceiver process exits, the ready_to_display flag remains set in
shared memory, exposing the conninfo of the next walreceiver before
obfuscating.  Fix by having WalRcvDie reset the flag.

Second, the sleep-and-retry behavior that waited until walreceiver had
set ready_to_display wasn't liked; the preference is to have it return
no data instead, so let's do that.

Bugs in 9ed551e0 reported by Fujii Masao and Michël Paquier.

Author: Michaël Paquier

In the previous design, the GetForeignUpperPaths FDW callback hook was
called before we got around to labeling upper relations with the proper
consider_parallel flag; this meant that any upper paths created by an FDW
would be marked not-parallel-safe.  While that's probably just as well
right now, we aren't going to want it to be true forever.  Hence, abandon
the idea that FDWs should be allowed to inject upper paths before the core
code has gotten around to creating the relevant upper relation.  (Well,
actually they still can, but it's on their own heads how well it works.)
Instead, adopt the same API already designed for create_upper_paths_hook:
we call GetForeignUpperPaths after each upperrel has been created and
populated with the paths the core planner knows how to make.

Commit 3fc6e2d7 introduced new "upper"
RelOptInfo structures but didn't set consider_parallel for them
correctly, a point I completely missed when reviewing it.  Later,
commit e06a3896 made the situation
worse by doing it incorrectly for the grouping relation.  Try to
straighten all of that out.  Along the way, get rid of the annoying
wholePlanParallelSafe flag, which was only necessarily because of
the fact that upper planning stages didn't use paths at the time
that code was written.

The most important immediate impact of these changes is that
force_parallel_mode will provide useful test coverage in quite a few
more scenarios than it did previously, but it's also necessary
preparation for fixing some problems related to subqueries.

Patch by me, reviewed by Tom Lane.