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

Update typedefs.list from current buildfarm results.  Adjust pgindent's
typedef blacklist to block some more unfortunate typedef names that have
snuck in since last time.  Manually tweak a few places where I didn't
like the initial results of pgindent'ing.

In the wake of commit 5602265f, we were doing duplicate-OID detection
quite inefficiently, by invoking duplicate_oids which does all the same
parsing of catalog headers and .dat files as genbki.pl does.  That adds
under half a second on modern machines, but quite a bit more on slow
buildfarm critters, so it seems worth avoiding.  Let's just extend
genbki.pl a little so it can also detect duplicate OIDs, and remove
the duplicate_oids call from the build process.

(This also means that duplicate OID detection will happen during
Windows builds, which AFAICS it didn't before.)

This makes the use-case for duplicate_oids a bit dubious, but it's
possible that people will still want to run that check without doing
a whole build run, so let's keep that script.

In passing, move down genbki.pl's creation of its temp output files
so that it doesn't happen until after we've done parsing and validation
of the input.  This avoids leaving a lot of clutter around after a
failure.

John Naylor and Tom Lane

Discussion: https://postgr.es/m/37D774E4-FE1F-437E-B3D2-593F314B7505@postgrespro.ru

Previously, you had to cd into src/include/catalog before running either
of these scripts.  That's a bit tedious, so let's make the scripts do it
for you.

In passing, improve the initial comments in both scripts.  Also remove
unused_oids' code to complain about duplicate oids.  That was added in
yesterday's commit 5602265f, but on second thought we shouldn't be
randomly redefining the script's behavior that way.

John Naylor and Tom Lane

Discussion: https://postgr.es/m/37D774E4-FE1F-437E-B3D2-593F314B7505@postgrespro.ru

Reported-by: vodevsh@gmail.com

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

Backpatch-through: 9.3

The predecessor test boiled down to "PQserverVersion(NULL) >= 100000",
which is always false.  No release includes that, so it could not have
reintroduced CVE-2018-1058.  Back-patch to 9.4, like the addition of the
predecessor in commit 8d2814f2.

Discussion: https://postgr.es/m/20180422215551.GB2676194@rfd.leadboat.com

unused_oids was previously a shell script, which of course didn't work at
all on Windows.  Also, commit 372728b0 introduced some other portability
problems, as complained of by Stas Kelvich.  We can improve matters by
converting it to Perl.

While we're at it, let's future-proof both this script and duplicate_oids
to use Catalog.pm rather than having a bunch of ad-hoc logic for parsing
catalog headers and .dat files.  These scripts are thereby a bit slower,
which doesn't seem like a problem for typical manual use.  It is a little
annoying for buildfarm purposes, but we should be able to fix that case
by having genbki.pl make the check instead of parsing the headers twice.
(That's not done in this commit, though.)

Stas Kelvich, adjusted a bit by me

Discussion: https://postgr.es/m/37D774E4-FE1F-437E-B3D2-593F314B7505@postgrespro.ru

Instead of immediately constructing the string we need to emit into the
.BKI file, preserve the items we extracted from the header file in a hash.
This eases using the info for other purposes.

John Naylor (with cosmetic adjustments by me)

Discussion: https://postgr.es/m/37D774E4-FE1F-437E-B3D2-593F314B7505@postgrespro.ru

Commit 0927d2f4 didn't check that
consider_parallel was set for the target relation or account for
the possibility that required_outer might be non-empty.

To prevent future bugs of this ilk, add some assertions to
add_partial_path and do a bit of future-proofing of the code
recently added to recurse_set_operations.

Report by Andreas Seltenreich.  Patch by Jeevan Chalke.  Review
by Amit Kapila and by me.

Discussion: http://postgr.es/m/CAM2+6=U+9otsyF2fYB8x_2TBeHTR90itarqW=qAEjN-kHaC7kw@mail.gmail.com

Also use palloc0() for main amcheck state, and adjust a few comments.

Somehow I pushed old version of patch in commit
4eaf7eac, so commit the difference.

Peter Geoghegan

Discussion: https://postgr.es/m/CABUevEzK3cNiHZQ18f5tK0guoT+cN_jWeVzhYYxY=r+1Q3SmoA@mail.gmail.com

Per discussion, this is now the project's standard version.
Update the obsolete URL, and explain how to install a specific version.

Discussion: https://postgr.es/m/CABUevEzK3cNiHZQ18f5tK0guoT+cN_jWeVzhYYxY=r+1Q3SmoA@mail.gmail.com

When bt_index_parent_check() is called with the heapallindexed option,
allocate a second Bloom filter to fingerprint block numbers that appear
in the downlinks of internal pages.  Use Bloom filter probes when
walking the B-Tree to detect missing downlinks.  This can detect subtle
problems with page deletion/VACUUM, such as corruption caused by the bug
just fixed in commit 6db4b499.

The downlink Bloom filter is bound in size by work_mem.  Its optimal
size is typically far smaller than that of the regular heapallindexed
Bloom filter, especially when the index has high fan-out.

Author: Peter Geoghegan
Reviewer: Teodor Sigaev
Discussion: https://postgr.es/m/CAH2-WznUzY4fWTjm1tBB3JpVz8cCfz7k_qVp5BhuPyhivmWJFg@mail.gmail.com

Author: Michael Paquier

Remove the words "if not already done."  This obsolete wording
corresponds to an early development version of what became edd44738.

Author: Etsuro Fujita
Reviewed-by: Amit Langote
Discussion: https://postgr.es/m/5ADF117B.5030606@lab.ntt.co.jp

Instead of doing ExecInitExpr every time a Param needs to be evaluated
in run-time partition pruning, do it once during run-time pruning
set-up and cache the exprstate in PartitionPruneContext, saving a lot of
work.

Author: David Rowley
Reviewed-by: Amit Langote, Álvaro Herrera
Discussion: https://postgr.es/m/CAKJS1f8-x+q-90QAPDu_okhQBV4DPEtPz8CJ=m0940GyT4DA4w@mail.gmail.com

This controls both plan-time and execution-time new-style partition
pruning.  While finer-grain control is possible (maybe using an enum GUC
instead of boolean), there doesn't seem to be much need for that.

This new parameter controls partition pruning for all queries:
trivially, SELECT queries that affect partitioned tables are naturally
under its control since they are using the new technology.  However,
while UPDATE/DELETE queries do not use the new code, we make the new GUC
control their behavior also (stealing control from
constraint_exclusion), because it is more natural, and it leads to a
more natural transition to the future in which those queries will also
use the new pruning code.

Constraint exclusion still controls pruning for regular inheritance
situations (those not involving partitioned tables).

Author: David Rowley
Review: Amit Langote, Ashutosh Bapat, Justin Pryzby, David G. Johnston
Discussion: https://postgr.es/m/CAKJS1f_0HwsxJG9m+nzU+CizxSdGtfe6iF_ykPYBiYft302DCw@mail.gmail.com

Previously, you could partition by a boolean column as long as you
spelled the bound values as string literals, for instance FOR VALUES
IN ('t').  The trouble with this is that ruleutils.c printed that as
FOR VALUES IN (TRUE), which is reasonable syntax but wasn't accepted by
the grammar.  That results in dump-and-reload failures for such cases.

Apply a minimal fix that just causes TRUE and FALSE to be converted to
strings 'true' and 'false'.  This is pretty grotty, but it's too late for
a more principled fix in v11 (to say nothing of v10).  We should revisit
the whole issue of how partition bound values are parsed for v12.

Amit Langote

Discussion: https://postgr.es/m/e05c5162-1103-7e37-d1ab-6de3e0afaf70@lab.ntt.co.jp

This could result in some misbehavior in a cascading replication setup.

Set Emacs's perl-continued-statement-offset to match perltidy's
--continuation-indentation, which is 2 (not overridden in PostgreSQL's
profile) rather than the 4 that Emacs uses by default.

Lifetime of the input string is not right, so create a separate copy.

Author: Amit Langote
Discussion: https://postgr.es/m/a2773420-50d1-0a42-3396-fe42b0921134@lab.ntt.co.jp

The switch covers all values of the enum driver variable, so having a
default: clause is useless, even if it's only to do Assert(false).

5c067521 erroneously had coded bms_prev_member assuming that a bitmapword
would always hold 32 bits and started it's search on what it thought was the
highest 8-bits of the word.  This was not the case if bitmapwords were 64
bits.

In passing add a test to exercise this function a little. Previously there was
no coverage at all.

David Rowly

Author: Liudmila Mantrova

After introducing usage of t_tid of inner or page high key for storing
number of attributes of tuple, validation of tuple's ItemPointer with
ItemPointerIsValid becomes incorrect, it's need to validate only blocknumber of
ItemPointer. Missing this causes a incorrect page deletion, fix that. Test is
added.

BTW, current contrib/amcheck doesn't fail on index corrupted by this way.

Also introduce BTreeTupleGetTopParent/BTreeTupleSetTopParent macroses to improve
code readability and to avoid possible confusion with page high key: high key
is used to store top-parent link for branch to remove.

Bug found by Michael Paquier, but bug doesn't exist in previous versions because
t_tid was set to P_HIKEY.

Author: Teodor Sigaev
Reviewer: Peter Geoghegan
Discussion: https://www.postgresql.org/message-id/flat/20180419052436.GA16000%40paquier.xyz

Author: David Rowley
Discussion: https://www.postgresql.org/message-id/CAKJS1f8Ge2y0sDs6RQEJFH-vjb-bWhs86rCX4Fp4FZ+TmxtRkw@mail.gmail.com

Suggested by Michael Paquier

This was missed in eed1ce72.

Reported by Michael Paquier

Per pademelon.

Move the authentication methods sections up to sect1, so they are easier
to navigate in HTML.

Results from buildfarm member opossum suggest that this doesn't work
quite right on that platform.  We've seen issues with NaN support on
MIPS/NetBSD before ... allegedly they fixed this stuff back in 2010,
but maybe only for small values of "fixed".

If, in fact, opossum fails this test then I plan to revert it;
it's mainly for diagnostic purposes rather than something we'd
necessarily keep long-term.  I think that the failures in window.sql
could be worked around with some code duplication, but I want to
verify my theory about the cause first.

In commit f0e44751, GetIndexOpClass was renamed to ResolveOpClass, but
the comment in typecmds.c didn't get the memo.

In objectaddress.c, missing 'of' in a comment.

Both noticed by Vik Fearing, patch is mine though.

Since it sets up an event trigger that would fire on DDL done by any
concurrent test script, the original scheduling is just an invitation
to irreproducible test failures.  (The fact that we found a bug through
exactly such irreproducible test failures doesn't really change the
calculus here: this script is a hazard to anything that runs in parallel
with it today or might be added to that parallel group in future.  No,
I don't believe that the trigger is protecting itself sufficiently to
avoid all possible trouble.)

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

EventTriggerTableRewrite crashed if there were table_rewrite triggers
present, but there had not been when the calling command started.

EventTriggerDDLCommandEnd called ddl_command_end triggers if present,
even if there had been no such triggers when the calling command started,
which would lead to a failure in pg_event_trigger_ddl_commands.

In both cases, fix by doing nothing; it's better to wait till the next
command when things will be properly initialized.

In passing, remove an elog(DEBUG1) call that might have seemed interesting
four years ago but surely isn't today.

We found this because of intermittent failures in the buildfarm.  Thanks
to Alvaro Herrera and Andrew Gierth for analysis.

Back-patch to 9.5; some of this code exists before that, but the specific
hazards we need to guard against don't.

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

Discussion: https://postgr.es/m/f8128b11-c5bf-3539-48cd-234178b2314d@proxel.se

On further reflection, commit e5d83995 didn't go far enough: pretty much
everywhere in the planner that examines a clause's is_pushed_down flag
ought to be changed to use the more complicated behavior where we also
check the clause's required_relids.  Otherwise we could make incorrect
decisions about whether, say, a clause is safe to use as a hash clause.

Some (many?) of these places are safe as-is, either because they are
never reached while considering a parameterized path, or because there
are additional checks that would reject a pushed-down clause anyway.
However, it seems smarter to just code them all the same way rather
than rely on easily-broken reasoning of that sort.

In support of that, invent a new macro RINFO_IS_PUSHED_DOWN that should
be used in place of direct tests on the is_pushed_down flag.

Like the previous patch, back-patch to all supported branches.

Discussion: https://postgr.es/m/f8128b11-c5bf-3539-48cd-234178b2314d@proxel.se

Use the term "system catalog" rather than "system relation" in assorted
places where it's clearly referring to a table rather than, say, an
index.  Use more natural word order in the header boilerplate, improve
some of the one-liner catalog descriptions, and fix assorted random
deviations from the normal boilerplate.  All purely neatnik-ism, but
why not.

John Naylor, some additional cleanup by me

Discussion: https://postgr.es/m/CAJVSVGUeJmFB3h-NJ18P32NPa+kzC165nm7GSoGHfPaN80Wxcw@mail.gmail.com

In some cases a clause attached to an outer join can be pushed down into
the outer join's RHS even though the clause is not degenerate --- this
can happen if we choose to make a parameterized path for the RHS.  If
the clause ends up attached to a lower outer join, we'd misclassify it
as being a "join filter" not a plain "filter" condition at that node,
leading to wrong query results.

To fix, teach extract_actual_join_clauses to examine each join clause's
required_relids, not just its is_pushed_down flag.  (The latter now
seems vestigial, or at least in need of rethinking, but we won't do
anything so invasive as redefining it in a bug-fix patch.)

This has been wrong since we introduced parameterized paths in 9.2,
though it's evidently hard to hit given the lack of previous reports.
The test case used here involves a lateral function call, and I think
that a lateral reference may be required to get the planner to select
a broken plan; though I wouldn't swear to that.  In any case, even if
LATERAL is needed to trigger the bug, it still affects all supported
branches, so back-patch to all.

Per report from Andreas Karlsson.  Thanks to Andrew Gierth for
preliminary investigation.

Discussion: https://postgr.es/m/f8128b11-c5bf-3539-48cd-234178b2314d@proxel.se

I added this "optimization" on top of Amit Langote's 158b7bc6, but
the quick path is never taken because the partition uses a different
pg_type oid than its parent table (causing equalTupleDescs to return
false).  Changing that requires more analysis and is too considered
dangerous at this point in the cycle, so revert it.

We might make it work someday, but not for pg11.

Discussion: https://postgr.es/m/825031be-942c-8c24-6163-13c27f217a3d@lab.ntt.co.jp