Michael Paquier

Spotted by David Johnston

MapArrayTypeName would copy up to NAMEDATALEN-1 bytes of the base type
name, which of course is wrong: after prepending '_' there is only room for
NAMEDATALEN-2 bytes.  Aside from being the wrong result, this case would
lead to overrunning the statically allocated work buffer.  This would be a
security bug if the function were ever used outside bootstrap mode, but it
isn't, at least not in any currently supported branches.

Aside from fixing the off-by-one loop logic, this patch gets rid of the
static work buffer by having MapArrayTypeName pstrdup its result; the sole
caller was already doing that, so this just requires moving the pstrdup
call.  This saves a few bytes but mainly it makes the API a lot cleaner.

Back-patch on the off chance that there is some third-party code using
MapArrayTypeName with less-secure input.  Pushing pstrdup into the function
should not cause any serious problems for such hypothetical code; at worst
there might be a short term memory leak.

Per Coverity scanning.

Code added in 9.4 would attempt to divide by zero in such cases.
Noted while testing fix for missing-pclose problem.

If the called command fails to return data, runShellCommand forgot to
pclose() the pipe before returning.  This is fairly harmless in the current
code, because pgbench would then abandon further processing of that client
thread; so no more than nclients descriptors could be leaked this way.  But
it's not hard to imagine future improvements whereby that wouldn't be true.
In any case, it's sloppy coding, so patch all branches.  Found by Coverity.

Mostly these issues concern the non-use of function results. These
have been changed to use (void) pushJsonbValue(...) instead of assigning
the result to a variable that gets overwritten before it is used.

There is a larger issue that we should possibly examine the API for
pushJsonbValue(), so that instead of returning a value it modifies a
state argument. The current idiom is rather clumsy. However, changing
that requires quite a bit more work, so this change should do for the
moment.

Backpatch the applicable parts, just to make backpatching future patches
easier.

It does not include the possible full-page image. While at it, reformat the
comment slightly to make it more readable.

Reported by Rahila Syed

Noticed on a couple of Windows configurations.

Petr Jelinek, reviewed by Michael Paquier.

"PG_RETURN_FLOAT8(x)" is not "return x", except perhaps by accident
on some platforms.

Alexander Korotkov, reviewed by Emre Hasegeli.

The ALTER SYSTEM ref page hadn't been held to a very high standard, nor
was the feature well integrated into section 18.1 (parameter setting).
Also, though commit 4c4654af had improved the structure of 18.1, it also
introduced a lot of poor wording, imprecision, and outright falsehoods.
Try to clean that up.

Set release date, do a final pass of wordsmithing, improve some other
new-in-9.4 documentation.

Fabrízio de Royes Mello reviewed by Rushabh Lathia.

Commit 815d71de hadn't bothered to update the documentation to match the
behavioral change, and a lot of other text in this section was badly in
need of copy-editing.

The code for advancing through the input rows overlooked the case that we
might already be past the first row of the row pair now being considered,
in case the previous percentile also fell between the same two input rows.

Report and patch by Andrew Gierth; logic rewritten a bit for clarity by me.

Mark Dilger

The planner seems to like to do this join query as a hash join, making
the output ordering machine-dependent; worse, it's a hash on OIDs, so
that it's a bit astonishing that the result doesn't change from run to
run even on one machine.  Add an ORDER BY to get consistent results.
Per buildfarm.

I also suppressed output from the final DROP SCHEMA CASCADE, to avoid
occasional failures similar to those fixed in commit 81d815dc.
That hasn't been observed in the buildfarm yet, but it seems likely
to happen in future if we leave it as-is.

The functions are:
    to_jsonb()
    jsonb_object()
    jsonb_build_object()
    jsonb_build_array()
    jsonb_agg()
    jsonb_object_agg()

Also along the way some better logic is implemented in
json_categorize_type() to match that in the newly implemented
jsonb_categorize_type().

Andrew Dunstan, reviewed by Pavel Stehule and Alvaro Herrera.

In commit 462bd957, I changed postgres_fdw
to rely on get_plan_rowmark() instead of get_parse_rowmark().  I still
think that's a good idea in the long run, but as Etsuro Fujita pointed out,
it doesn't work today because planner.c forces PlanRowMarks to have
markType = ROW_MARK_COPY for all foreign tables.  There's no urgent reason
to change this in the back branches, so let's just revert that part of
yesterday's commit rather than trying to design a better solution under
time pressure.

Also, add a regression test case showing what postgres_fdw does with FOR
UPDATE/SHARE.  I'd blithely assumed there was one already, else I'd have
realized yesterday that this code didn't work.

The functions remove object fields, including in nested objects, that
have null as a value. In certain cases this can lead to considerably
smaller datums, with no loss of semantic information.

Andrew Dunstan, reviewed by Pavel Stehule.

This avoids duplicating the code.

Michael Paquier, reviewed by Simon Riggs and me

Otherwise the pg_ctl start and stop messages get mixed up with the TAP
output, which isn't technically valid.

Ordinarily we can omit checking of a WHERE condition that matches a partial
index's condition, when we are using an indexscan on that partial index.
However, in SELECT FOR UPDATE we must include the "redundant" filter
condition in the plan so that it gets checked properly in an EvalPlanQual
recheck.  The planner got this mostly right, but improperly omitted the
filter condition if the index in question was on an inheritance child
table.  In READ COMMITTED mode, this could result in incorrectly returning
just-updated rows that no longer satisfy the filter condition.

The cause of the error is using get_parse_rowmark() when get_plan_rowmark()
is what should be used during planning.  In 9.3 and up, also fix the same
mistake in contrib/postgres_fdw.  It's currently harmless there (for lack
of inheritance support) but wrong is wrong, and the incorrect code might
get copied to someplace where it's more significant.

Report and fix by Kyotaro Horiguchi.  Back-patch to all supported branches.

In READ COMMITTED mode, if a SELECT FOR UPDATE discovers it has to redo
WHERE-clause checking on rows that have been updated since the SELECT's
snapshot, it invokes EvalPlanQual processing to do that.  If this first
occurs within a non-first child table of an inheritance tree, the previous
coding could accidentally re-return a matching row from an earlier,
already-scanned child table.  (And, to add insult to injury, I think this
could make it miss returning a row that should have been returned, if the
updated row that this happens on should still have passed the WHERE qual.)
Per report from Kyotaro Horiguchi; the added isolation test is based on his
test case.

This has been broken for quite awhile, so back-patch to all supported
branches.

Ensure we reindex indexes built on Mat Views.
Based on patch from Micheal Paquier

Add thorough tests to check that indexes on
tables, toast tables and mat views are reindexed.

Simon Riggs

Leaving global objects like roles hanging around is bad practice.

Aside from not testing the case it claimed to test (namely a permissions
failure), it left a login-capable role lying around, which quite aside
from possibly being a security hole would cause subsequent regression runs
to fail since the role would already exist.

In passing, also make some debugging elog's in pgstat.c a bit more
consistently worded.

Back-patch as far as applicable (9.3 or 9.4; none of these mistakes are
really old).

Mark Dilger identified and patched the type violations; the message
rewordings are mine.

It's an OID. WRITE_UINT_FIELD is identical to WRITE_OID_FIELD, but let's
be tidy.

Mark Dilger

Author: Fabrízio de Royes Mello <fabriziomello@gmail.com>

The amount of space to reserve for the value's varlena header is
VARHDRSZ, not sizeof(VARHDRSZ).  The latter coding accidentally
failed to fail because of the way the VARHDRSZ macro is currently
defined; but if we ever change it to return size_t (as one might
reasonably expect it to do), convertToJsonb() would have failed.

Spotted by Mark Dilger.

It's not run by the global "check" or "installcheck" targets, because the
temporary installation it creates accepts TCP connections from any user
the same host, which is insecure.