Author: Doug Rady
Discussion: https://postgr.es/m/6323D83C-9FDA-4EE1-B0ED-6971E585066A@amazon.com

Use IndexTupleSize everywhere, instead.  Also, remove IndexTupleSize's
internal typecast, as that's not really needed and might mask coding
errors.  Change some pointer variable datatypes in the call sites
to compensate for that and make it clearer what we're assuming.

Ildar Musin, Robert Haas, Stephen Frost

Discussion: https://postgr.es/m/0274288e-9e88-13b6-c61c-7b36928bf221@postgrespro.ru

Commit ff963b39 added two identical copies of this row.

Dagfinn Ilmari Mannsåker

Discussion: https://postgr.es/m/d8j8tdevb7x.fsf@dalvik.ping.uio.no

Solaris 11.4 has built-in functions named b64_encode and b64_decode.
Rename ours to something else to avoid the conflict (fortunately,
ours are static so the impact is limited).

One could wish for less duplication of code in this area, but that
would be a larger patch and not very suitable for back-patching.
Since this is a portability fix, we want to put it into all supported
branches.

Report and initial patch by Rainer Orth, reviewed and adjusted a bit
by Michael Paquier

Discussion: https://postgr.es/m/ydd372wk28h.fsf@CeBiTec.Uni-Bielefeld.DE

specscanner.l had a fixed limit of 1024 bytes on the length of
individual SQL stanzas in an isolation test script.  People are
starting to run into that, so fix it by making the buffer resizable.

Once we allow this in HEAD, it seems inevitable that somebody will
try to back-patch a test that exceeds the old limit, so back-patch
this change as a preventive measure.

Daniel Gustafsson

Discussion: https://postgr.es/m/8D628BE4-6606-4FF6-A3FF-8B2B0E9B43D0@yesql.se

The previous code considered two tables to have the partition scheme
if the underlying columns had the same collation, but what we
actually need to compare is not the collations associated with the
column but the collation used for partitioning.  Fix that.

Robert Haas and Amit Langote

Discussion: http://postgr.es/m/0f95f924-0efa-4cf5-eb5f-9a3d1bc3c33d@lab.ntt.co.jp

Thomas Munro

Discussion: http://postgr.es/m/CAEepm=3g1hhbFzYkR_QT9RmBvsGX4UaeCtX-4Js8OOEMmFeaSQ@mail.gmail.com

Parallel-aware plan nodes must be prepared to run without parallelism
if it's not possible at execution time for whatever reason.  Commit
ab727167, which introduced Parallel
Append, overlooked this.

Rajkumar Raghuwanshi reported this problem, and I included his test
case in this patch.  The code changes are by me.

Discussion: http://postgr.es/m/CAKcux6=WqkUudLg1GLZZ7fc5ScWC1+Y9qD=pAHeqy32WoeJQvw@mail.gmail.com

Commit 1bc0100d added this test,
and commit 882ea509 tried to
stabilize it.  There were still failures, so commit
958e20e4 tried again to stabilize
it.  That approach is still failing on jaguarundi, though, so
back it out and try something else.  Specifically, instead of
disabling remote estimates for the table in question, let's tell
autovacuum to leave it alone.

Etsuro Fujita

Discussion: http://postgr.es/m/5A82DCCE.3060107@lab.ntt.co.jp

Commits 6f6b99d1 and
f3b0897a didn't properly update
these comments.

Etsuro Fujita, reviewed by Amit Langote

Discussion: http://postgr.es/m/5A671FE1.6020305@lab.ntt.co.jp

Turn off man.endnotes.are.numbered parameter, which we don't need, but
which increases performance vastly if off.  Also turn on
man.output.quietly, which also makes things a bit faster, but which is
also less useful now as a progress indicator because the build is so
fast now.

The changes in the CREATE POLICY man page from commit
87c2a17f triggered a stylesheet bug that
created some warning messages and incorrect output.  This installs a
workaround.

Also improve the whitespace a bit so it looks better.

Although configure-based builds correctly define HAVE_LONG_LONG_INT when
appropriate (in both pg_config.h and ecpg_config.h), builds using the MSVC
scripts failed to do so.  This currently has no impact on the backend,
since it uses that symbol nowhere; but it does prevent ecpg from
supporting "long long int".  Fix that.

Also, adjust Solution.pm so that in the constructed ecpg_config.h file,
the "#if (_MSC_VER > 1200)" covers only the LONG_LONG_INT-related
#defines, not the whole file.  AFAICS this was a thinko on somebody's
part: ENABLE_THREAD_SAFETY should always be defined in Windows builds,
and in branches using USE_INTEGER_DATETIMES, the setting of that shouldn't
depend on the compiler version either.  If I'm wrong, I imagine the
buildfarm will say so.

Per bug #15080 from Jonathan Allen; issue diagnosed by Michael Meskes
and Andrew Gierth.  Back-patch to all supported branches.

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

Tom Kazimiers reported that transition tables don't work correctly when
they are scanned by more than one executor node.  That's because commit
18ce3a4a allocated separate read pointers for each executor node, as it
must, but failed to make them active at the appropriate times.  Repair.

Thomas Munro

Discussion: https://postgr.es/m/20180224034748.bixarv6632vbxgeb%40dewberry.localdomain

This seemed like a good idea in commit be42eb9d, but it causes more
trouble than it's worth for cross-branch upgrade testing.

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

Michael Paquier

Discussion: http://postgr.es/m/20180209135327.GC29003@paquier.xyz

A before-update row trigger may choose to return the "new" or "old" tuple
unmodified.  ExecBRUpdateTriggers failed to consider the second
possibility, and would proceed to free the "old" tuple even if it was the
one returned, leading to subsequent access to already-deallocated memory.
In debug builds this reliably leads to an "invalid memory alloc request
size" failure; in production builds it might accidentally work, but data
corruption is also possible.

This is a very old bug.  There are probably a couple of reasons it hasn't
been noticed up to now.  It would be more usual to return NULL if one
wanted to suppress the update action; returning "old" is significantly less
efficient since the update will occur anyway.  Also, none of the standard
PLs would ever cause this because they all returned freshly-manufactured
tuples even if they were just copying "old".  But commit 4b93f579 changed
that for plpgsql, making it possible to see the bug with a plpgsql trigger.
Still, this is certainly legal behavior for a trigger function, so it's
ExecBRUpdateTriggers's fault not plpgsql's.

It seems worth creating a test case that exercises returning "old" directly
with a C-language trigger; testing this through plpgsql seems unreliable
because its behavior might change again.

Report and fix by Rushabh Lathia; regression test case by me.
Back-patch to all supported branches.

Discussion: https://postgr.es/m/CAGPqQf1P4pjiNPrMof=P_16E-DFjt457j+nH2ex3=nBTew7tXw@mail.gmail.com

Jeevan Chalke

Discussion: http://postgr.es/m/CAM2+6=X9kxQoL2ZqZ00E6asBt9z+rfyWbOmhXJ0+8fPAyMZ9Jg@mail.gmail.com

Commit 3bf05e09 sometimes uses the
cheapest_partial_path variable in this function to mean the cheapest
one from the input rel and at other times the cheapest one from the
partially grouped rel, but it never resets it, so we can end up with
bad plans, leading to "ERROR: Aggref found in non-Agg plan node".

Jeevan Chalke, per a report from Andreas Joseph Krogh and a separate
off-list report from Rajkumar Raghuwanshi

Discussion: http://postgr.es/m/CAM2+6=X9kxQoL2ZqZ00E6asBt9z+rfyWbOmhXJ0+8fPAyMZ9Jg@mail.gmail.com

It's a bit silly to have test functions that aren't tested, so test
them.

In passing, rename int44in/int44out to city_budget_in/_out so that they
match how the regression tests use them.  Also, fix city_budget_out
so that it emits the format city_budget_in expects to read; otherwise
we'd have dump/reload failures when testing pg_dump against the
regression database.  (We avoided that in the past only because no
data of type city_budget was actually stored anywhere.)

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

This patch removes five functions that presumably were once used in the
regression tests, but haven't been so used in many years.  Nonetheless
we've been wasting maintenance effort on them (e.g., by converting them
to V1 function protocol).  I see no reason to think that reviving them
would add any useful test coverage, so drop 'em.

In passing, mark regress_lseg_construct static, since it's not called
from outside this file.

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

Small review on edd44738.

Discussion: https://postgr.es/m/20180222165315.k27qfn4goskhoswj@alvherre.pgsql
Reviewed-by: Robert Haas, Amit Langote

This omission seems to be what is causing buildfarm failures on crake.

Security: CVE-2018-1058

Security: CVE-2018-1058

The ability to create like-named objects in different schemas opens up
the potential for users to change the behavior of other users' queries,
maliciously or accidentally.  When you connect to a PostgreSQL server,
you should remove from your search_path any schema for which a user
other than yourself or superusers holds the CREATE privilege.  If you do
not, other users holding CREATE privilege can redefine the behavior of
your commands, causing them to perform arbitrary SQL statements under
your identity.  "SET search_path = ..." and "SELECT
pg_catalog.set_config(...)" are not vulnerable to such hijacking, so one
can use either as the first command of a session.  As special
exceptions, the following client applications behave as documented
regardless of search_path settings and schema privileges: clusterdb
createdb createlang createuser dropdb droplang dropuser ecpg (not
programs it generates) initdb oid2name pg_archivecleanup pg_basebackup
pg_config pg_controldata pg_ctl pg_dump pg_dumpall pg_isready
pg_receivewal pg_recvlogical pg_resetwal pg_restore pg_rewind pg_standby
pg_test_fsync pg_test_timing pg_upgrade pg_waldump reindexdb vacuumdb
vacuumlo.  Not included are core client programs that run user-specified
SQL commands, namely psql and pgbench.  PostgreSQL encourages non-core
client applications to do likewise.

Document this in the context of libpq connections, psql connections,
dblink connections, ECPG connections, extension packaging, and schema
usage patterns.  The principal defense for applications is "SELECT
pg_catalog.set_config('search_path', '', false)", and the principal
defense for databases is "REVOKE CREATE ON SCHEMA public FROM PUBLIC".
Either one is sufficient to prevent attack.  After a REVOKE, consider
auditing the public schema for objects named like pg_catalog objects.

Authors of SECURITY DEFINER functions use some of the same defenses, and
the CREATE FUNCTION reference page already covered them thoroughly.
This is a good opportunity to audit SECURITY DEFINER functions for
robust security practice.

Back-patch to 9.3 (all supported versions).

Reviewed by Michael Paquier and Jonathan S. Katz.  Reported by Arseniy
Sharoglazov.

Security: CVE-2018-1058

This makes the client programs behave as documented regardless of the
connect-time search_path and regardless of user-created objects.  Today,
a malicious user with CREATE permission on a search_path schema can take
control of certain of these clients' queries and invoke arbitrary SQL
functions under the client identity, often a superuser.  This is
exploitable in the default configuration, where all users have CREATE
privilege on schema "public".

This changes behavior of user-defined code stored in the database, like
pg_index.indexprs and pg_extension_config_dump().  If they reach code
bearing unqualified names, "does not exist" or "no schema has been
selected to create in" errors might appear.  Users may fix such errors
by schema-qualifying affected names.  After upgrading, consider watching
server logs for these errors.

The --table arguments of src/bin/scripts clients have been lax; for
example, "vacuumdb -Zt pg_am\;CHECKPOINT" performed a checkpoint.  That
now fails, but for now, "vacuumdb -Zt 'pg_am(amname);CHECKPOINT'" still
performs a checkpoint.

Back-patch to 9.3 (all supported versions).

Reviewed by Tom Lane, though this fix strategy was not his first choice.
Reported by Arseniy Sharoglazov.

Security: CVE-2018-1058

Historically, pg_dump has "set search_path = foo, pg_catalog" when
dumping an object in schema "foo", and has also caused that setting
to be used while restoring the object.  This is problematic because
functions and operators in schema "foo" could capture references meant
to refer to pg_catalog entries, both in the queries issued by pg_dump
and those issued during the subsequent restore run.  That could
result in dump/restore misbehavior, or in privilege escalation if a
nefarious user installs trojan-horse functions or operators.

This patch changes pg_dump so that it does not change the search_path
dynamically.  The emitted restore script sets the search_path to what
was used at dump time, and then leaves it alone thereafter.  Created
objects are placed in the correct schema, regardless of the active
search_path, by dint of schema-qualifying their names in the CREATE
commands, as well as in subsequent ALTER and ALTER-like commands.

Since this change requires a change in the behavior of pg_restore
when processing an archive file made according to this new convention,
bump the archive file version number; old versions of pg_restore will
therefore refuse to process files made with new versions of pg_dump.

Security: CVE-2018-1058

Up until now, we've abused grouped_rel->partial_pathlist as a place to
store partial paths that have been partially aggregate, but that's
really not correct, because a partial path for a relation is supposed
to be one which produces the correct results with the addition of only
a Gather or Gather Merge node, and these paths also require a Finalize
Aggregate step.  Instead, add a new partially_group_rel which can hold
either partial paths (which need to be gathered and then have
aggregation finalized) or non-partial paths (which only need to have
aggregation finalized).  This allows us to reuse generate_gather_paths
for partially_grouped_rel instead of writing new code, so that this
patch actually basically no net new code while making things cleaner,
simplifying things for pending patches for partition-wise aggregate.

Robert Haas and Jeevan Chalke.  The larger patch series of which this
patch is a part was also reviewed and tested by Antonin Houska,
Rajkumar Raghuwanshi, David Rowley, Dilip Kumar, Konstantin Knizhnik,
Pascal Legrand, Rafia Sabih, and me.

Discussion: http://postgr.es/m/CA+TgmobrzFYS3+U8a_BCy3-hOvh5UyJbC18rEcYehxhpw5=ETA@mail.gmail.com
Discussion: http://postgr.es/m/CA+TgmoZyQEjdBNuoG9-wC5GQ5GrO4544Myo13dVptvx+uLg9uQ@mail.gmail.com

Commit b3f84012 changed pg_upgrade so that it'd actually drop and
re-create the template1 and postgres databases in the new cluster.
That works fine, serially.  With the -j option it's not so fine, because
other per-database jobs might be launched while the template1 database is
dropped.  Since they attempt to connect there to start up, kaboom.

This is the cause of the intermittent failures buildfarm member jacana
has been showing for the last month; evidently it is the only BF member
configured to run the pg_upgrade test with parallelism enabled.

Fix by processing template1 separately before we get into the parallel
sub-job launch loop.  (We could alternatively have made the postgres DB
be the special case, but it seems likely that template1 will contain
less stuff and so we lose less parallelism with this choice.)

The scripts were changed in c98c35cd,
but the output files were not updated to reflect the script changes.

Recent Perl versions don't have the current directory in the module
include path anymore, so we need to add it here explicitly to make these
scripts continue to work.

I forgot the coding rule for correct use of Float8GetDatumFast.
Per buildfarm.

Commit 0a459cec left this for later, but since time's running out,
I went ahead and took care of it.  There are more data types that
somebody might someday want RANGE support for, but this is enough
to satisfy all expectations of the SQL standard, which just says that
"numeric, datetime, and interval" types should have RANGE support.

In tests that check whether a connection fails, also check the error
message.  That makes sure that the connection was rejected for the right
reason.

This discovered that two tests had their connection failing for the
wrong reason.  One test failed because pg_hba.conf was not set up to
allow that user, one test failed because the client key file did not
have the right permissions.  Fix those tests and add a new one that is
really supposed to check the file permission issue.
Reviewed-by: Michael Paquier <michael@paquier.xyz>

In the pgoutput plugin, skip changes for relations that are not
publishable, per is_publishable_class().  This concerns in particular
materialized views and information_schema tables.  While those relations
cannot be part of a publication, per existing checks, they will be
considered by a FOR ALL TABLES publication.  A subscription would not
actually apply changes for those relations, again per existing checks,
but trying to match incoming changes to local tables on the subscriber
would lead to errors if no matching local table exists.  Skipping those
changes on the publisher avoids sending useless changes and eliminates
the error.

Bug: #15044
Reported-by: Chad Trabant <chad@iris.washington.edu>
Reviewed-by: Petr Jelinek <petr.jelinek@2ndquadrant.com>

RANGE_OFFSET comparisons need to examine the first ORDER BY column,
which isn't necessarily the first column in the incoming tuples.
No idea how this slipped through initial testing.

Per bug #15082 from Zhou Digoal.

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