Jesper Pedersen

Discussion: http://postgr.es/m/000f92d6-f623-95a5-b341-46e2c0495cea@redhat.com

Sometimes, for testing, it's useful to have the leader do nothing but
read tuples from workers; and it's possible that could work out better
even in production.

Thomas Munro, reviewed by Amit Kapila and by me.  A few final tweaks
by me.

Discussion: http://postgr.es/m/CAEepm=2U++Lp3bNTv2Bv_kkr5NE2pOyHhxU=G0YTa4ZhSYhHiw@mail.gmail.com

Our initial work with int128 neglected alignment considerations, an
oversight that came back to bite us in bug #14897 from Vincent Lachenal.
It is unsurprising that int128 might have a 16-byte alignment requirement;
what's slightly more surprising is that even notoriously lax Intel chips
sometimes enforce that.

Raising MAXALIGN seems out of the question: the costs in wasted disk and
memory space would be significant, and there would also be an on-disk
compatibility break.  Nor does it seem very practical to try to allow some
data structures to have more-than-MAXALIGN alignment requirement, as we'd
have to push knowledge of that throughout various code that copies data
structures around.

The only way out of the box is to make type int128 conform to the system's
alignment assumptions.  Fortunately, gcc supports that via its
__attribute__(aligned()) pragma; and since we don't currently support
int128 on non-gcc-workalike compilers, we shouldn't be losing any platform
support this way.

Although we could have just done pg_attribute_aligned(MAXIMUM_ALIGNOF) and
called it a day, I did a little bit of extra work to make the code more
portable than that: it will also support int128 on compilers without
__attribute__(aligned()), if the native alignment of their 128-bit-int
type is no more than that of int64.

Add a regression test case that exercises the one known instance of the
problem, in parallel aggregation over a bigint column.

This will need to be back-patched, along with the preparatory commit
91aec93e.  But let's see what the buildfarm makes of it first.

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

Generalize section 1 to handle stuff that is principally about the
compiler (not libraries), such as attributes, and collect stuff there
that had been dropped into various other parts of c.h.  Also, push
all the gettext macros into section 8, so that section 0 is really
just inclusions rather than inclusions and random other stuff.

The primary goal here is to get pg_attribute_aligned() defined before
section 3, so that we can use it with int128.  But this seems like good
cleanup anyway.

This patch just moves macro definitions around, and shouldn't result
in any changes in generated code.  But I'll push it out separately
to see if the buildfarm agrees.

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

Commit 5ecc0d73 removed the hard-wired superuser checks in lo_import
and lo_export in favor of protecting them with SQL permissions, but
failed to adjust the documentation to match.  Fix that, and add a
<caution> paragraph pointing out the nontrivial security hazards
involved with actually granting such permissions.  (It's still better
than ALLOW_DANGEROUS_LO_FUNCTIONS, though.)

Also, commit ae20b23a caused large object read/write privilege to
be checked during lo_open() rather than in the actual read or write
calls.  Document that.

Discussion: https://postgr.es/m/CAB7nPqRHmNOYbETnc_2EjsuzSM00Z+BWKv9sy6tnvSd5gWT_JA@mail.gmail.com

Instead of passing large swaths of boolean arguments, define some flags
that can be used in a bitmask.  This makes it easier not only to figure
out what each call site is doing, but also to add some new flags.

The flags are split in two -- one set for index_create directly and
another for constraints.  index_create() itself receives both, and then
passes down the latter to index_constraint_create(), which can also be
called standalone.

Discussion: https://postgr.es/m/20171023151251.j75uoe27gajdjmlm@alvherre.pgsql
Reviewed-by: Simon Riggs

This feature caters to specialized use-cases such as running the normal
pgbench scenario with nonstandard indexes, or inserting other actions
between steps of the initialization sequence.  The normal sequence of
initialization actions is broken down into half a dozen steps which can
be executed in a user-specified order, to the extent to which that's
sensible.  The actions themselves aren't changed, except to make them
more robust against nonstandard uses:

* all four tables are now dropped in one DROP command, to reduce
assumptions about what foreign key relationships exist;

* all four tables are now truncated at the start of the data load
step, for consistency;

* the foreign key creation commands now specify constraint names, to
prevent accidentally creating duplicate constraints by executing the
'f' step twice.

Make some cosmetic adjustments in the messages emitted by pgbench
so that it's clear which steps are getting run, and so that the
messages agree with the documented names of the steps.

In passing, fix failure to enforce that the -v option is used only
in benchmarking mode.

Masahiko Sawada, reviewed by Fabien Coelho, editorialized a bit by me

Discussion: https://postgr.es/m/CAD21AoCsz0ZzfCFcxYZ+PUdpkDd5VsCSG0Pre_-K1EgokCDFYA@mail.gmail.com

We already do this for Gather, but it got overlooked for Gather Merge.

Amit Kapila, with review and minor revisions by Rushabh Lathia
and by me.

Discussion: http://postgr.es/m/CAA4eK1KUC5Uyu7qaifxrjpHxbSeoQh3yzwN3bThnJsmJcZ-qtA@mail.gmail.com

Up until now, we only tracked the number of parameters, which was
sufficient to allocate an array of Datums of the appropriate size,
but not sufficient to, for example, know how to serialize a Datum
stored in one of those slots.  An upcoming patch wants to do that,
so add this tracking to make it possible.

Patch by me, reviewed by Tom Lane and Amit Kapila.

Discussion: http://postgr.es/m/CA+TgmoYqpxDKn8koHdW8BEKk8FMUL0=e8m2Qe=M+r0UBjr3tuQ@mail.gmail.com

The new commands are reported by event triggers, but they weren't
documented as such.  Repair.

Author: David Rowley
Discussion: https://postgr.es/m/CAKJS1f-t-NE=AThB3zu1mKhdrm8PCb=++3e7x=Lf343xcrFHxQ@mail.gmail.com

Determinisitcally -> Deterministically

Author: Michael Paquier <michael.paquier@gmail.com>
Discussion: https://postgr.es/m/CAB7nPqSauJ9gUMzj1aiXQVxqEkyko+WZ+wUac8_hB_M_bO6U_A@mail.gmail.com

Also, add a warning to catch future instances of naming a nonexistent
file as a prerequisite.  Back-patch to 9.3 (all supported versions).

Apart from calling write_stderr() on failure, the handler depends on no
PostgreSQL facilities.  We have experienced crashes before reaching the
former call site.  Given such an early crash, this change cannot hurt
and may produce a helpful dump.  Absent an early crash, this change has
no effect.  Back-patch to 9.3 (all supported versions).

Takayuki Tsunakawa

Discussion: https://postgr.es/m/0A3221C70F24FB45833433255569204D1F80CD13@G01JPEXMBYT05

PostgreSQL running as a Windows service crashed upon calling
write_stderr() before MemoryContextInit().  This fix completes work
started in 5735efee.  Messages this
early contain only ASCII bytes; if we removed the CurrentMemoryContext
requirement, the ensuing conversions would have no effect.  Back-patch
to 9.3 (all supported versions).

Takayuki Tsunakawa, reviewed by Michael Paquier.

Discussion: https://postgr.es/m/0A3221C70F24FB45833433255569204D1F80CC73@G01JPEXMBYT05

This suite had been a proper superset of the regular ecpg test suite,
but the three newest tests didn't reach it.  To make this less likely to
recur, delete the extra schedule file and pass the TCP-specific test on
the command line.  Back-patch to 9.3 (all supported versions).

Since commit 86889873, it has assumed
"localhost" resolves to both ::1 and 127.0.0.1.  We gain nothing from
that assumption, and it does not hold in a default installation of Red
Hat Enterprise Linux 5.  Back-patch to 9.3 (all supported versions).

To ensure stable output, catch one more configuration-specific error.
Back-patch to 9.3, like the commit that added the test.

When a value contained an XML declaration naming some other encoding,
this function interpreted UTF8 bytes as the named encoding, yielding
mojibake.  xml_parse() already has similar logic.  This would be
necessary but not sufficient for non-UTF8 databases, so preserve
behavior there until the xpath facility can support such databases
comprehensively.  Back-patch to 9.3 (all supported versions).

Pavel Stehule and Noah Misch

Discussion: https://postgr.es/m/CAFj8pRC-dM=tT=QkGi+Achkm+gwPmjyOayGuUfXVumCxkDgYWg@mail.gmail.com

Dilip Kumar, reviewed by Alexander Kumenkov, Amul Sul, and me.
Some final adjustments by me.

Discussion: http://postgr.es/m/CAFiTN-sYtqUOXQ4SpuhTv0Z9gD0si3YxZGv_PQAAMX8qbOotcg@mail.gmail.com

An LDAP URL without a host name such as "ldap://" or without a base DN
such as "ldap://localhost" would cause a crash when reading pg_hba.conf.

If no binddn is configured, an error message might end up trying to print a
null pointer, which could crash on some platforms.

Author: Thomas Munro <thomas.munro@enterprisedb.com>
Reviewed-by: Michael Paquier <michael.paquier@gmail.com>

Reviewed-by: Fabien COELHO <coelho@cri.ensmp.fr>

Make bloom WAL test compare psql output text, not just result codes;
this was evidently the intent all along, but it was mis-coded.
In passing, make sure we will notice any failure in setup steps.

Alexander Korotkov, reviewed by Michael Paquier and Masahiko Sawada

Discussion: https://postgr.es/m/CAPpHfdtohPdQ9rc5mdWjxq+3VsBNw534KV_5O65dTQrSdVJNgw@mail.gmail.com

Etsuro Fujita

Discussion: http://postgr.es/m/5A05728E.4050009@lab.ntt.co.jp

This causes a warning when accidentally backpatching an XML-style
empty-element tag like <xref linkend="abc"/>.

Hash partitioning is useful when you want to partition a growing data
set evenly.  This can be useful to keep table sizes reasonable, which
makes maintenance operations such as VACUUM faster, or to enable
partition-wise join.

At present, we still depend on constraint exclusion for partitioning
pruning, and the shape of the partition constraints for hash
partitioning is such that that doesn't work.  Work is underway to fix
that, which should both improve performance and make partitioning
pruning work with hash partitioning.

Amul Sul, reviewed and tested by Dilip Kumar, Ashutosh Bapat, Yugo
Nagata, Rajkumar Raghuwanshi, Jesper Pedersen, and by me.  A few
final tweaks also by me.

Discussion: http://postgr.es/m/CAAJ_b96fhpJAP=ALbETmeLk1Uni_GFZD938zgenhF49qgDTjaQ@mail.gmail.com

Up to now, ACL checks for large objects happened at the level of
the SQL-callable functions, which led to CVE-2017-7548 because of a
missing check.  Push them down to be enforced in inv_api.c as much
as possible, in hopes of preventing future bugs.  This does have the
effect of moving read and write permission errors to happen at lo_open
time not loread or lowrite time, but that seems acceptable.

Michael Paquier and Tom Lane

Discussion: https://postgr.es/m/CAB7nPqRHmNOYbETnc_2EjsuzSM00Z+BWKv9sy6tnvSd5gWT_JA@mail.gmail.com

While it's generally unwise to give permissions on these functions to
anyone but a superuser, we've been moving away from hard-wired permission
checks inside functions in favor of using the SQL permission system to
control access.  Bring lo_import() and lo_export() into compliance with
that approach.

In particular, this removes the manual configuration option
ALLOW_DANGEROUS_LO_FUNCTIONS.  That dates back to 1999 (commit 4cd4a54c);
it's unlikely anyone has used it in many years.  Moreover, if you really
want such behavior, now you can get it with GRANT ... TO PUBLIC instead.

Michael Paquier

Discussion: https://postgr.es/m/CAB7nPqRHmNOYbETnc_2EjsuzSM00Z+BWKv9sy6tnvSd5gWT_JA@mail.gmail.com

The header comment written into postgresql.auto.conf by ALTER SYSTEM
should match what initdb put there originally.

Feike Steenbergen

Discussion: https://postgr.es/m/CAK_s-G0KcKdO=0hqZkwb3s+tqZuuHwWqmF5BDsmoO9FtX75r0g@mail.gmail.com

Somebody messed up a refactoring here.  As it stood, we'd check pg_ctl's
--version output twice for each cluster.  Worse, the first check for the
new cluster's version happened before we'd done any validate_exec checks
there, breaking the check ordering the code intended.

A. Akenteva

Discussion: https://postgr.es/m/f9266a85d918a3cf3a386b5148aee666@postgrespro.ru

Upon further review, our Bonjour code doesn't actually work with the
Avahi not-too-compatible compatibility library.  While you can get it
to work on non-macOS platforms if you link to Apple's own mDNSResponder
code, there don't seem to be many people who care about that.  Leaving in
the AC_SEARCH_LIBS call seems more likely to encourage people to build
broken configurations than to do anything very useful.

Hence, remove the AC_SEARCH_LIBS call and put in a warning comment instead.

Discussion: https://postgr.es/m/2D8331C5-D64F-44C1-8717-63EDC6EAF7EB@brightforge.com

On macOS the relevant functions require no special library, but elsewhere
we need to pull in libdns_sd.

Back-patch to supported branches.  No docs change since the docs do not
suggest that this is a Mac-only feature.

Luke Lonergan

Discussion: https://postgr.es/m/2D8331C5-D64F-44C1-8717-63EDC6EAF7EB@brightforge.com

The grammar requires these options to appear the other way 'round.

jotpe@posteo.de

Discussion: https://postgr.es/m/78933bd0-45ce-690e-b832-a328dd1a5567@posteo.de

The point of having separate ResourceOwnerEnlargeFoo and
ResourceOwnerRememberFoo functions is so that resource allocation
can happen in between.  Doing it in some other order is just wrong.

OpenTemporaryFile() did open(), enlarge, remember, which would leak the
open file if the enlarge step ran out of memory.  Because fd.c has its own
layer of resource-remembering, the consequences look like they'd be limited
to an intratransaction FD leak, but it's still not good.

IncrBufferRefCount() did enlarge, remember, incr-refcount, which would blow
up if the incr-refcount step ever failed.  It was safe enough when written,
but since the introduction of PrivateRefCountHash, I think the assumption
that no error could happen there is pretty shaky.

The odds of real problems from either bug are probably small, but still,
back-patch to supported branches.

Thomas Munro and Tom Lane, per a comment from Andres Freund

The lower case spellings are C and C++ standard and are used in most
parts of the PostgreSQL sources.  The upper case spellings are only used
in some files/modules.  So standardize on the standard spellings.

The APIs for ICU, Perl, and Windows define their own TRUE and FALSE, so
those are left as is when using those APIs.

In code comments, we use the lower-case spelling for the C concepts and
keep the upper-case spelling for the SQL concepts.
Reviewed-by: Michael Paquier <michael.paquier@gmail.com>

Masahiko Sawada

Discussion: http://postgr.es/m/CAD21AoDrf5AOpZ-mX-j6O=zFNFfKaTdHkv3o1X2eSs2nBXALug@mail.gmail.com

Per buildfarm member pademelon.

isdigit(), isspace(), etc are likely to give surprising results if passed a
signed char.  We should always cast the argument to unsigned char to avoid
that.  Error in commit 63d6b97f, found by buildfarm member gaur.
Back-patch to 9.3, like that commit.