Andrew Gierth [Wed, 13 Feb 2019 15:20:33 +0000 (15:20 +0000)]
Change floating-point output format for improved performance.
Previously, floating-point output was done by rounding to a specific
decimal precision; by default, to 6 or 15 decimal digits (losing
information) or as requested using extra_float_digits. Drivers that
wanted exact float values, and applications like pg_dump that must
preserve values exactly, set extra_float_digits=3 (or sometimes 2 for
historical reasons, though this isn't enough for float4).
Unfortunately, decimal rounded output is slow enough to become a
noticable bottleneck when dealing with large result sets or COPY of
large tables when many floating-point values are involved.
Floating-point output can be done much faster when the output is not
rounded to a specific decimal length, but rather is chosen as the
shortest decimal representation that is closer to the original float
value than to any other value representable in the same precision. The
recently published Ryu algorithm by Ulf Adams is both relatively
simple and remarkably fast.
Accordingly, change float4out/float8out to output shortest decimal
representations if extra_float_digits is greater than 0, and make that
the new default. Applications that need rounded output can set
extra_float_digits back to 0 or below, and take the resulting
performance hit.
We make one concession to portability for systems with buggy
floating-point input: we do not output decimal values that fall
exactly halfway between adjacent representable binary values (which
would rely on the reader doing round-to-nearest-even correctly). This
is known to be a problem at least for VS2013 on Windows.
Our version of the Ryu code originates from
https://github.com/ulfjack/ryu/ at commit c9c3fb1979, but with the
following (significant) modifications:
- Output format is changed to use fixed-point notation for small
exponents, as printf would, and also to use lowercase 'e', a
minimum of 2 exponent digits, and a mandatory sign on the exponent,
to keep the formatting as close as possible to previous output.
- The output of exact midpoint values is disabled as noted above.
- The integer fast-path code is changed somewhat (since we have
fixed-point output and the upstream did not).
- Our project style has been largely applied to the code with the
exception of C99 declaration-after-statement, which has been
retained as an exception to our present policy.
- Most of upstream's debugging and conditionals are removed, and we
use our own configure tests to determine things like uint128
availability.
Changing the float output format obviously affects a number of
regression tests. This patch uses an explicit setting of
extra_float_digits=0 for test output that is not expected to be
exactly reproducible (e.g. due to numerical instability or differing
algorithms for transcendental functions).
Conversions from floats to numeric are unchanged by this patch. These
may appear in index expressions and it is not yet clear whether any
change should be made, so that can be left for another day.
This patch assumes that the only supported floating point format is
now IEEE format, and the documentation is updated to reflect that.
Code by me, adapting the work of Ulf Adams and other contributors.
Andrew Gierth [Wed, 13 Feb 2019 15:19:44 +0000 (15:19 +0000)]
Use strtof() and not strtod() for float4 input.
Using strtod() creates a double-rounding problem; the input decimal
value is first rounded to the nearest double; rounding that to the
nearest float may then give an incorrect result.
An example is that 7.038531e-26 when input via strtod and then rounded
to float4 gives 0xAE43FEp-107 instead of the correct 0xAE43FDp-107.
Values output by earlier PG versions with extra_float_digits=3 should
all be read in with the same values as previously. However, values
supplied by other software using shortest representations could be
mis-read.
On platforms that lack a strtof() entirely, we fall back to the old
incorrect rounding behavior. (As strtof() is required by C99, such
platforms are considered of primarily historical interest.) On VS2013,
some workarounds are used to get correct error handling.
The regression tests now test for the correct input values, so
platforms that lack strtof() will need resultmap entries. An entry for
HP-UX 10 is included (more may be needed).
Reviewed-By: Tom Lane
Discussion: https://postgr.es/m/871s5emitx.fsf@news-spur.riddles.org.uk
Discussion: https://postgr.es/m/87d0owlqpv.fsf@news-spur.riddles.org.uk
Thomas Munro [Wed, 13 Feb 2019 00:14:10 +0000 (13:14 +1300)]
Fix rare dsa_allocate() failures due to freepage.c corruption.
In a corner case, a btree page was allocated during a clean-up operation
that could cause the tracking of the largest contiguous span of free
space to get out of whack. That was supposed to be prevented by the use
of the "soft" flag to avoid allocating internal pages during incidental
clean-up work, but the flag was ignored in the case where the FPM was
promoted from singleton format to btree format. Repair.
Remove an obsolete comment in passing.
Back-patch to 10, where freepage.c arrived (as support for dsa.c).
Author: Robert Haas Diagnosed-by: Thomas Munro and Robert Haas Reported-by: Justin Pryzby, Rick Otten, Sand Stone, Arne Roland and others
Discussion: https://postgr.es/m/CAMAYy4%2Bw3NTBM5JLWFi8twhWK4%3Dk_5L4nV5%2BbYDSPu8r4b97Zg%40mail.gmail.com
Tom Lane [Tue, 12 Feb 2019 23:38:32 +0000 (18:38 -0500)]
Clean up planner confusion between ncolumns and nkeycolumns.
We're only going to consider key columns when creating indexquals,
so there is no point in having the outer loops in indxpath.c iterate
further than nkeycolumns.
Doing so in match_pathkeys_to_index() is actually wrong, and would have
caused crashes by now, except that we have no index AMs supporting both
amcanorderbyop and amcaninclude.
It's also wrong in relation_has_unique_index_for(). The effect there is
to fail to prove uniqueness even when the index does prove it, if there
are extra columns.
Also future-proof examine_variable() for the day when extra columns can
be expressions, and fix what's either a thinko or just an oversight in
btcostestimate(): we should consider the number of key columns, not the
total, when deciding whether to derate correlation.
None of these things seemed important enough to risk changing in a
just-before-wrap patch, but since we're past the release wrap window,
time to fix 'em.
Alvaro Herrera [Tue, 12 Feb 2019 21:42:37 +0000 (18:42 -0300)]
Relax overly strict assertion
Ever since its birth, ReorderBufferBuildTupleCidHash() has contained an
assertion that a catalog tuple cannot change Cmax after acquiring one. But
that's wrong: if a subtransaction executes DDL that affects that catalog
tuple, and later aborts and another DDL affects the same tuple, it will
change Cmax. Relax the assertion to merely verify that the Cmax remains
valid and monotonically increasing, instead.
Alvaro Herrera [Tue, 12 Feb 2019 14:53:36 +0000 (11:53 -0300)]
Use Getopt::Long for catalog scripts
Replace hand-rolled option parsing with the Getopt module. This is
shorter and easier to read. In passing, make some cosmetic adjustments
for consistency.
Author: John Naylor Reviewed-by: David Fetter
Discussion: https://postgr.es/m/CACPNZCvRjepXh5b2N50njN+rO_2Nzcf=jhMkKX7=79XWUKJyKA@mail.gmail.com
Tom Lane [Tue, 12 Feb 2019 06:12:52 +0000 (01:12 -0500)]
Fix erroneous error reports in snapbuild.c.
It's pretty unhelpful to report the wrong file name in a complaint
about syscall failure, but SnapBuildSerialize managed to do that twice
in a span of 50 lines. Also fix half a dozen missing or poorly-chosen
errcode assignments; that's mostly cosmetic, but still wrong.
Noted while studying recent failures on buildfarm member nightjar.
I'm not sure whether those reports are actually giving the wrong
filename, because there are two places here with identically
spelled error messages. The other one is specifically coded not
to report ENOENT, but if it's this one, how could we be getting
ENOENT from open() with O_CREAT? Need to sit back and await results.
However, these ereports are clearly broken from birth, so back-patch.
Tom Lane [Tue, 12 Feb 2019 03:37:16 +0000 (22:37 -0500)]
Fix header inclusion issue.
partprune.h failed to compile by itself; needs to include partdefs.h.
I think I must've broken this in fa2cf164a, though I'd swear I ran
the appropriate tests when removing #includes. Anyway, it's very
sensible for this file to include partdefs.h, so let's just do that.
Michael Paquier [Tue, 12 Feb 2019 03:01:56 +0000 (12:01 +0900)]
Clarify docs about limitations of constraint exclusion with partitions
The current wording can confuse the reader about constraint exclusion
being available at query execution, but this only applies to partition
pruning.
Reported-by: Shouyu Luo
Author: David Rowley Reviewed-by: Chapman Flack, Amit Langote
Discussion: https://postgr.es/m/15629-2ef8b22e61f8333f@postgresql.org
Tom Lane [Tue, 12 Feb 2019 02:26:08 +0000 (21:26 -0500)]
Allow extensions to generate lossy index conditions.
For a long time, indxpath.c has had the ability to extract derived (lossy)
index conditions from certain operators such as LIKE. For just as long,
it's been obvious that we really ought to make that capability available
to extensions. This commit finally accomplishes that, by adding another
API for planner support functions that lets them create derived index
conditions for their functions. As proof of concept, the hardwired
"special index operator" code formerly present in indxpath.c is pushed
out to planner support functions attached to LIKE and other relevant
operators.
A weak spot in this design is that an extension needs to know OIDs for
the operators, datatypes, and opfamilies involved in the transformation
it wants to make. The core-code prototypes use hard-wired OID references
but extensions don't have that option for their own operators etc. It's
usually possible to look up the required info, but that may be slow and
inconvenient. However, improving that situation is a separate task.
I want to do some additional refactorization around selfuncs.c, but
that also seems like a separate task.
Michael Paquier [Tue, 12 Feb 2019 01:07:56 +0000 (10:07 +0900)]
Move max_wal_senders out of max_connections for connection slot handling
Since its introduction, max_wal_senders is counted as part of
max_connections when it comes to define how many connection slots can be
used for replication connections with a WAL sender context. This can
lead to confusion for some users, as it could be possible to block a
base backup or replication from happening because other backend sessions
are already taken for other purposes by an application, and
superuser-only connection slots are not a correct solution to handle
that case.
This commit makes max_wal_senders independent of max_connections for its
handling of PGPROC entries in ProcGlobal, meaning that connection slots
for WAL senders are handled using their own free queue, like autovacuum
workers and bgworkers.
One compatibility issue that this change creates is that a standby now
requires to have a value of max_wal_senders at least equal to its
primary. So, if a standby created enforces the value of
max_wal_senders to be lower than that, then this could break failovers.
Normally this should not be an issue though, as any settings of a
standby are inherited from its primary as postgresql.conf gets normally
copied as part of a base backup, so parameters would be consistent.
Author: Alexander Kukushkin Reviewed-by: Kyotaro Horiguchi, Petr Jelínek, Masahiko Sawada, Oleksii
Kliukin
Discussion: https://postgr.es/m/CAFh8B=nBzHQeYAu0b8fjK-AF1X4+_p6GRtwG+cCgs6Vci2uRuQ@mail.gmail.com
Tom Lane [Mon, 11 Feb 2019 19:41:13 +0000 (14:41 -0500)]
Redesign the partition dependency mechanism.
The original setup for dependencies of partitioned objects had
serious problems:
1. It did not verify that a drop cascading to a partition-child object
also cascaded to at least one of the object's partition parents. Now,
normally a child object would share all its dependencies with one or
another parent (e.g. a child index's opclass dependencies would be shared
with the parent index), so that this oversight is usually harmless.
But if some dependency failed to fit this pattern, the child could be
dropped while all its parents remain, creating a logically broken
situation. (It's easy to construct artificial cases that break it,
such as attaching an unrelated extension dependency to the child object
and then dropping the extension. I'm not sure if any less-artificial
cases exist.)
2. Management of partition dependencies during ATTACH/DETACH PARTITION
was complicated and buggy; for example, after detaching a partition
table it was possible to create cases where a formerly-child index
should be dropped and was not, because the correct set of dependencies
had not been reconstructed.
Less seriously, because multiple partition relationships were
represented identically in pg_depend, there was an order-of-traversal
dependency on which partition parent was cited in error messages.
We also had some pre-existing order-of-traversal hazards for error
messages related to internal and extension dependencies. This is
cosmetic to users but causes testing problems.
To fix #1, add a check at the end of the partition tree traversal
to ensure that at least one partition parent got deleted. To fix #2,
establish a new policy that partition dependencies are in addition to,
not instead of, a child object's usual dependencies; in this way
ATTACH/DETACH PARTITION need not cope with adding or removing the
usual dependencies.
To fix the cosmetic problem, distinguish between primary and secondary
partition dependency entries in pg_depend, by giving them different
deptypes. (They behave identically except for having different
priorities for being cited in error messages.) This means that the
former 'I' dependency type is replaced with new 'P' and 'S' types.
This also fixes a longstanding bug that after handling an internal
dependency by recursing to the owning object, findDependentObjects
did not verify that the current target was now scheduled for deletion,
and did not apply the current recursion level's objflags to it.
Perhaps that should be back-patched; but in the back branches it
would only matter if some concurrent transaction had removed the
internal-linkage pg_depend entry before the recursive call found it,
or the recursive call somehow failed to find it, both of which seem
unlikely.
Catversion bump because the contents of pg_depend change for
partitioning relationships.
Patch HEAD only. It's annoying that we're not fixing #2 in v11,
but there seems no practical way to do so given that the problem
is exactly a poor choice of what entries to put in pg_depend.
We can't really fix that while staying compatible with what's
in pg_depend in existing v11 installations.
Tom Lane [Mon, 11 Feb 2019 03:51:32 +0000 (22:51 -0500)]
Fix indexable-row-comparison logic to account for covering indexes.
indxpath.c needs a good deal more attention for covering indexes than
it's gotten. But so far as I can tell, the only really awful breakage
is in expand_indexqual_rowcompare (nee adjust_rowcompare_for_index),
which was only half fixed in c266ed31a. The other problems aren't
bad enough to take the risk of a just-before-wrap fix.
The problem here is that if the leading column of a row comparison
matches an index (allowing this code to be reached), and some later
column doesn't match the index, it'll nonetheless believe that that
column matches the first included index column. Typically that'll
lead to an error like "operator M is not a member of opfamily N" as
a result of fetching a garbage opfamily OID. But with enough bad
luck, maybe a broken plan would be generated.
Alvaro Herrera [Sun, 10 Feb 2019 13:00:11 +0000 (10:00 -0300)]
Fix trigger drop procedure
After commit 123cc697a8eb, we remove redundant FK action triggers during
partition ATTACH by merely deleting the catalog tuple, but that's wrong:
it should use performDeletion() instead. Repair, and make the comments
more explicit.
Tom Lane [Sun, 10 Feb 2019 02:02:06 +0000 (21:02 -0500)]
Solve cross-version-upgrade testing problem induced by 1fb57af92.
Renaming varchar_transform to varchar_support had a side effect
I hadn't foreseen: the core regression tests leave around a
transform object that relies on that function, so the name
change breaks cross-version upgrade tests, because the name
used in the older branches doesn't match.
Since the dependency on varchar_transform was chosen with the
aid of a dartboard anyway (it would surely not work as a
language transform support function), fix by just choosing
a different random builtin function with the right signature.
Also add some comments explaining why this isn't horribly unsafe.
I chose to make the same substitution in a couple of other
copied-and-pasted test cases, for consistency, though those
aren't directly contributing to the testing problem.
Per buildfarm. Back-patch, else it doesn't fix the problem.
Tom Lane [Sun, 10 Feb 2019 00:45:38 +0000 (19:45 -0500)]
Repair unsafe/unportable snprintf usage in pg_restore.
warn_or_exit_horribly() was blithely passing a potentially-NULL
string pointer to a %s format specifier. That works (at least
to the extent of not crashing) on some platforms, but not all,
and since we switched to our own snprintf.c it doesn't work
for us anywhere.
Of the three string fields being handled this way here, I think
that only "owner" is supposed to be nullable ... but considering
that this is error-reporting code, it has very little business
assuming anything, so put in defenses for all three.
Per a crash observed on buildfarm member crake and then
reproduced here. Because of the portability aspect,
back-patch to all supported versions.
Tom Lane [Sat, 9 Feb 2019 23:32:23 +0000 (18:32 -0500)]
Build out the planner support function infrastructure.
Add support function requests for estimating the selectivity, cost,
and number of result rows (if a SRF) of the target function.
The lack of a way to estimate selectivity of a boolean-returning
function in WHERE has been a recognized deficiency of the planner
since Berkeley days. This commit finally fixes it.
In addition, non-constant estimates of cost and number of output
rows are now possible. We still fall back to looking at procost
and prorows if the support function doesn't service the request,
of course.
To make concrete use of the possibility of estimating output rowcount
for SRFs, this commit adds support functions for array_unnest(anyarray)
and the integer variants of generate_series; the lack of plausible
rowcount estimates for those, even when it's obvious to a human,
has been a repeated subject of complaints. Obviously, much more
could now be done in this line, but I'm mostly just trying to get
the infrastructure in place.
Tom Lane [Sat, 9 Feb 2019 23:08:48 +0000 (18:08 -0500)]
Create the infrastructure for planner support functions.
Rename/repurpose pg_proc.protransform as "prosupport". The idea is
still that it names an internal function that provides knowledge to
the planner about the behavior of the function it's attached to;
but redesign the API specification so that it's not limited to doing
just one thing, but can support an extensible set of requests.
The original purpose of simplifying a function call is handled by
the first request type to be invented, SupportRequestSimplify.
Adjust all the existing transform functions to handle this API,
and rename them fron "xxx_transform" to "xxx_support" to reflect
the potential generalization of what they do. (Since we never
previously provided any way for extensions to add transform functions,
this change doesn't create an API break for them.)
Also add DDL and pg_dump support for attaching a support function to a
user-defined function. Unfortunately, DDL access has to be restricted
to superusers, at least for now; but seeing that support functions
will pretty much have to be written in C, that limitation is just
theoretical. (This support is untested in this patch, but a follow-on
patch will add cases that exercise it.)
Tom Lane [Sat, 9 Feb 2019 22:30:43 +0000 (17:30 -0500)]
Refactor the representation of indexable clauses in IndexPaths.
In place of three separate but interrelated lists (indexclauses,
indexquals, and indexqualcols), an IndexPath now has one list
"indexclauses" of IndexClause nodes. This holds basically the same
information as before, but in a more useful format: in particular, there
is now a clear connection between an indexclause (an original restriction
clause from WHERE or JOIN/ON) and the indexquals (directly usable index
conditions) derived from it.
We also change the ground rules a bit by mandating that clause commutation,
if needed, be done up-front so that what is stored in the indexquals list
is always directly usable as an index condition. This gets rid of repeated
re-determination of which side of the clause is the indexkey during costing
and plan generation, as well as repeated lookups of the commutator
operator. To minimize the added up-front cost, the typical case of
commuting a plain OpExpr is handled by a new special-purpose function
commute_restrictinfo(). For RowCompareExprs, generating the new clause
properly commuted to begin with is not really any more complex than before,
it's just different --- and we can save doing that work twice, as the
pretty-klugy original implementation did.
Tracking the connection between original and derived clauses lets us
also track explicitly whether the derived clauses are an exact or lossy
translation of the original. This provides a cheap solution to getting
rid of unnecessary rechecks of boolean index clauses, which previously
seemed like it'd be more expensive than it was worth.
Another pleasant (IMO) side-effect is that EXPLAIN now always shows
index clauses with the indexkey on the left; this seems less confusing.
This commit leaves expand_indexqual_conditions() and some related
functions in a slightly messy state. I didn't bother to change them
any more than minimally necessary to work with the new data structure,
because all that code is going to be refactored out of existence in
a follow-on patch.
Tom Lane [Sat, 9 Feb 2019 16:41:09 +0000 (11:41 -0500)]
Call set_rel_pathlist_hook before generate_gather_paths, not after.
The previous ordering of these steps satisfied the nominal requirement
that set_rel_pathlist_hook could editorialize on the whole set of Paths
constructed for a base relation. In practice, though, trying to change
the set of partial paths was impossible. Adding one didn't work because
(a) it was too late to be included in Gather paths made by the core code,
and (b) calling add_partial_path after generate_gather_paths is unsafe,
because it might try to delete a path it thinks is dominated, but that
is already embedded in some Gather path(s). Nor could the hook safely
remove partial paths, for the same reason that they might already be
embedded in Gathers.
Better to call extensions first, let them add partial paths as desired,
and then gather. In v11 and up, we already doubled down on that ordering
by postponing gathering even further for single-relation queries; so even
if the hook wished to editorialize on Gather path construction, it could
not.
Report and patch by KaiGai Kohei. Back-patch to 9.6 where Gather paths
were added.
The comment marker "#" is copied to the output, so it's only
appropriate for comments that make sense in the shell output. For
comments about the Autoconf language, "dnl" should be used.
Andres Freund [Sat, 9 Feb 2019 08:35:57 +0000 (00:35 -0800)]
Reset, not recreate, execGrouping.c style hashtables.
This uses the facility added in the preceding commit to fix
performance issues caused by rebuilding the hashtable (with its
comparator expression being the most expensive bit), after every
reset. That's especially important when the comparator is JIT
compiled.
Bug: #15592 #15486 Reported-By: Jakub Janeček, Dmitry Marakasov
Author: Andres Freund
Discussion:
https://postgr.es/m/15486-05850f065da42931@postgresql.org
https://postgr.es/m/20190114180423.ywhdg2iagzvh43we@alap3.anarazel.de
Backpatch: 11, where I broke this in bf6c614a2f2c5
Andres Freund [Sat, 9 Feb 2019 08:35:57 +0000 (00:35 -0800)]
Allow to reset execGrouping.c style tuple hashtables.
This has the advantage that the comparator expression, the table's
slot, etc do not have to be rebuilt. Additionally the simplehash.h
hashtable within the tuple hashtable now keeps its previous size and
doesn't need to be reallocated. That both reduces allocator overhead,
and improves performance in cases where the input estimation was off
by a significant factor.
To avoid an API/ABI break, the new parameter is exposed via the new
BuildTupleHashTableExt(), and BuildTupleHashTable() now is a wrapper
around the former, that continues to allocate the table itself in the
tablecxt.
Using this fixes performance issues discovered in the two bugs
referenced. This commit however has not converted the callers, that's
done in a separate commit.
Bug: #15592 #15486 Reported-By: Jakub Janeček, Dmitry Marakasov
Author: Andres Freund
Discussion:
https://postgr.es/m/15486-05850f065da42931@postgresql.org
https://postgr.es/m/20190114180423.ywhdg2iagzvh43we@alap3.anarazel.de
Backpatch: 11, this is a prerequisite for other fixes
Andres Freund [Sat, 9 Feb 2019 08:35:57 +0000 (00:35 -0800)]
simplehash: Add support for resetting a hashtable's contents.
A hashtable reset just reset the hashtable entries, but does not free
memory.
Author: Andres Freund
Discussion: https://postgr.es/m/20190114180423.ywhdg2iagzvh43we@alap3.anarazel.de
Bug: #15592 #15486
Backpatch: 11, this is a prerequisite for other fixes
Andres Freund [Sat, 9 Feb 2019 08:35:57 +0000 (00:35 -0800)]
Plug leak in BuildTupleHashTable by creating ExprContext in correct context.
In bf6c614a2f2c5 I added a expr context to evaluate the grouping
expression. Unfortunately the code I added initialized them while in
the calling context, rather the table context. Additionally, I used
CreateExprContext() rather than CreateStandaloneExprContext(), which
creates the econtext in the estate's query context.
Fix that by using CreateStandaloneExprContext when in the table's
tablecxt. As we rely on the memory being freed by a memory context
reset that means that the econtext's shutdown callbacks aren't being
called, but that seems ok as the expressions are tightly controlled
due to ExecBuildGroupingEqual().
Bug: #15592 Reported-By: Dmitry Marakasov
Author: Andres Freund
Discussion: https://postgr.es/m/20190114222838.h6r3fuyxjxkykf6t@alap3.anarazel.de
Backpatch: 11, where I broke this in bf6c614a2f2c5
Tom Lane [Fri, 8 Feb 2019 18:30:42 +0000 (13:30 -0500)]
Defend against null error message reported by libxml2.
While this isn't really supposed to happen, it can occur in OOM
situations and perhaps others. Instead of crashing, substitute
"(no message provided)".
I didn't worry about localizing this text, since we aren't
localizing anything else here; besides, if we're on the edge of
OOM, it's unlikely gettext() would work.
Report and fix by Sergio Conde Gómez in bug #15624.
Tom Lane [Fri, 8 Feb 2019 17:49:36 +0000 (12:49 -0500)]
Doc: fix thinko in description of how to escape a backslash in bytea.
Also clean up some discussion that had been left in a very confused
state thanks to half-hearted adjustments for the change to
standard_conforming_strings being the default.
Fix error handling around ssl_*_protocol_version settings
In case of a reload, we just want to LOG errors instead of FATAL when
processing SSL configuration, but the more recent code for the
ssl_*_protocol_version settings didn't behave like that.
Author: Daniel Gustafsson <daniel@yesql.se> Reviewed-by: Michael Paquier <michael@paquier.xyz>
Michael Paquier [Thu, 7 Feb 2019 23:56:14 +0000 (08:56 +0900)]
Add pg_partition_root to display top-most parent of a partition tree
This is useful when looking at partition trees with multiple layers, and
combined with pg_partition_tree, it provides the possibility to show up
an entire tree by just knowing one member at any level.
Author: Michael Paquier Reviewed-by: Álvaro Herrera, Amit Langote
Discussion: https://postgr.es/m/20181207014015.GP2407@paquier.xyz
Tom Lane [Thu, 7 Feb 2019 17:59:47 +0000 (12:59 -0500)]
Split create_foreignscan_path() into three functions.
Up to now postgres_fdw has been using create_foreignscan_path() to
generate not only base-relation paths, but also paths for foreign joins
and foreign upperrels. This is wrong, because create_foreignscan_path()
calls get_baserel_parampathinfo() which will only do the right thing for
baserels. It accidentally fails to fail for unparameterized paths, which
are the only ones postgres_fdw (thought it) was handling, but we really
need different APIs for the baserel and join cases.
In HEAD, the best thing to do seems to be to split up the baserel,
joinrel, and upperrel cases into three functions so that they can
have different APIs. I haven't actually given create_foreign_join_path
a different API in this commit: we should spend a bit of time thinking
about just what we want to do there, since perhaps FDWs would want to
do something different from the build-up-a-join-pairwise approach that
get_joinrel_parampathinfo expects. In the meantime, since postgres_fdw
isn't prepared to generate parameterized joins anyway, just give it a
defense against trying to plan joins with lateral refs.
In addition (and this is what triggered this whole mess) fix bug #15613
from Srinivasan S A, by teaching file_fdw and postgres_fdw that plain
baserel foreign paths still have outer refs if the relation has
lateral_relids. Add some assertions in relnode.c to catch future
occurrences of the same error --- in particular, to catch other FDWs
doing that, but also as backstop against core-code mistakes like the
one fixed by commit bdd9a99aa.
Bug #15613 also needs to be fixed in the back branches, but the
appropriate fix will look quite a bit different there, since we don't
want to assume that existing FDWs get the word right away.
Andrew Dunstan [Thu, 7 Feb 2019 15:22:49 +0000 (10:22 -0500)]
Fix searchpath and module location for pg_rewind and ssl TAP tests
The modules RewindTest.pm and ServerSetup.pm are really only useful for
TAP tests, so they really belong in the TAP test directories. In
addition, ServerSetup.pm is renamed to SSLServer.pm.
The test scripts have their own directories added to the search path so
that the relocated modules will be found, regardless of where the tests
are run from, even on modern perl where "." is no longer in the
searchpath.
Change pg_dump and ruleutils.c to use the FUNCTION keyword instead of
PROCEDURE in trigger and event trigger definitions.
This completes the pieces of the transition started in 0a63f996e018ac508c858e87fa39cc254a5db49f that were kept out of
PostgreSQL 11 because of the required catversion change.
Otherwise functions that require collation information will not have
it if they are called in arguments to a CALL statement.
Reported-by: Jean-Marc Voillequin <Jean-Marc.Voillequin@moodys.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/1EC8157EB499BF459A516ADCF135ADCE39FFAC54%40LON-WGMSX712.ad.moodys.net
Amit Kapila [Thu, 7 Feb 2019 03:28:29 +0000 (08:58 +0530)]
Doc: Update the documentation for row movement behavior across partitions.
In commit f16241bef7c, we have changed the behavior for concurrent updates
that move row to a different partition, but forgot to update the docs.
Previously when an UPDATE command causes a row to move from one partition
to another, there is a chance that another concurrent UPDATE or DELETE
misses this row. However, now we raise a serialization failure error in
such a case.
Reported-by: David Rowley
Author: David Rowley and Amit Kapila
Backpatch-through: 11 where it was introduced
Discussion: https://postgr.es/m/CAKJS1f-iVhGD4-givQWpSROaYvO3c730W8yoRMTF9Gc3craY3w@mail.gmail.com
Michael Paquier [Thu, 7 Feb 2019 01:04:55 +0000 (10:04 +0900)]
Align better test output regex with grammar in pg_dump TAP tests
This enforces one-or-more character matches in the regular expressions
for pg_dump testing on SQL syntax output where zero-or-more matches
implies a syntax error.
Author: Daniel Gustafsson Reviewed-by: David G. Johnston, Michael Paquier
Discussion: https://postgr.es/m/B313C32C-0E24-4AFB-95FF-6DA0C4E18A89@yesql.se
Michael Paquier [Thu, 7 Feb 2019 00:21:57 +0000 (09:21 +0900)]
Add more tests for CREATE TABLE AS with WITH NO DATA
The relation creation is done at executor startup, however the main
regression test suite is lacking scenarios where no data is inserted
which is something that can happen when using EXECUTE or EXPLAIN with
CREATE TABLE AS and WITH NO DATA.
Some patches are worked on to reshape the way CTAS relations are
created, so this makes sure that we do not miss some query patterns
already supported.
Reported-by: Andreas Karlsson
Author: Michael Paquier Reviewed-by: Andreas Karlsson
Discussion: https://postgr.es/m/20190206091817.GB14980@paquier.xyz
Peter Geoghegan [Wed, 6 Feb 2019 23:54:19 +0000 (15:54 -0800)]
Avoid amcheck inline compression false positives.
The previous tacit assumption that index_form_tuple() hides differences
in the TOAST state of its input datums was wrong. Normalize input
varlena datums by decompressing compressed values, and forming a new
index tuple for fingerprinting using uncompressed inputs. The final
normalized representation may actually be compressed once again within
index_form_tuple(), though that shouldn't matter. When the original
tuple is found to have no datums that are compressed inline, fingerprint
the original tuple directly.
Normalization avoids false positive reports of corruption in certain
cases. For example, the executor can apply toasting with some inline
compression to an entire heap tuple because its input has a single
external TOAST pointer. Varlena datums for other attributes that are
not particularly good candidates for inline compression can be
compressed in the heap tuple in passing, without the representation of
the same values in index tuples ever receiving concomitant inline
compression.
Add a test case to recreate the issue in a simpler though less realistic
way: by exploiting differences in pg_attribute.attstorage between heap
and index relations.
This bug was discovered by me during testing of an upcoming set of nbtree
enhancements. It was also independently reported by Andreas Kunert, as
bug #15597. His test case was rather more realistic than the one I
ended up using.
Bug: #15597
Discussion: https://postgr.es/m/CAH2-WznrVd9ie+TTJ45nDT+v2nUt6YJwQrT9SebCdQKtAvfPZw@mail.gmail.com
Discussion: https://postgr.es/m/15597-294e5d3e7f01c407@postgresql.org
Backpatch: 11-, where heapallindexed verification was introduced.
Tom Lane [Wed, 6 Feb 2019 17:44:58 +0000 (12:44 -0500)]
Propagate lateral-reference information to indirect descendant relations.
create_lateral_join_info() computes a bunch of information about lateral
references between base relations, and then attempts to propagate those
markings to appendrel children of the original base relations. But the
original coding neglected the possibility of indirect descendants
(grandchildren etc). During v11 development we noticed that this was
wrong for partitioned-table cases, but failed to realize that it was just
as wrong for any appendrel. While the case can't arise for appendrels
derived from traditional table inheritance (because we make a flat
appendrel for that), nested appendrels can arise from nested UNION ALL
subqueries. Failure to mark the lower-level relations as having lateral
references leads to confusion in add_paths_to_append_rel about whether
unparameterized paths can be built. It's not very clear whether that
leads to any user-visible misbehavior; the lack of field reports suggests
that it may cause nothing worse than minor cost misestimation. Still,
it's a bug, and it leads to failures of Asserts that I intend to add
later.
To fix, we need to propagate information from all appendrel parents,
not just those that are RELOPT_BASERELs. We can still do it in one
pass, if we rely on the append_rel_list to be ordered with ancestor
relationships before descendant ones; add assertions checking that.
While fixing this, we can make a small performance improvement by
traversing the append_rel_list just once instead of separately for
each appendrel parent relation.
Noted while investigating bug #15613, though this patch does not fix
that (which is why I'm not committing the related Asserts yet).
Andrew Dunstan [Wed, 6 Feb 2019 12:32:35 +0000 (07:32 -0500)]
Unify searchpath and do file logic in MSVC build scripts.
Commit f83419b739 failed to notice that mkvcbuild.pl and build.pl use
different searchpath and do-file logic, breaking the latter, so it is
adjusted to use the same logic as mkvcbuild.pl.
Andres Freund [Wed, 6 Feb 2019 09:09:32 +0000 (01:09 -0800)]
Fix heap_getattr() handling of fast defaults.
Previously heap_getattr() returned NULL for attributes with a fast
default value (c.f. 16828d5c0273), as it had no handling whatsoever
for that case.
A previous fix, 7636e5c60f, attempted to fix issues caused by this
oversight, but just expanding OLD tuples for triggers doesn't actually
solve the underlying issue.
One known consequence of this bug is that the check for HOT updates
can return the wrong result, when a previously fast-default'ed column
is set to NULL. Which in turn means that an index over a column with
fast default'ed columns might be corrupt if the underlying column(s)
allow NULLs.
Fix by handling fast default columns in heap_getattr(), remove now
superfluous expansion in GetTupleForTrigger().
Author: Andres Freund
Discussion: https://postgr.es/m/20190201162404.onngi77f26baem4g@alap3.anarazel.de
Backpatch: 11, where fast defaults were introduced
Michael Paquier [Wed, 6 Feb 2019 08:33:55 +0000 (17:33 +0900)]
Tighten some regexes with proper character escaping in pg_dump TAP tests
Some tests have been using regular expressions which have been lax in
escaping dots, which may cause tests to pass when they should not. This
make the whole set of tests more robust where needed.
Author: David Rowley Reviewed-by: Daniel Gustafsson, Michael Paquier
Discussion: https://postgr.es/m/CAKJS1f9jD8aVo1BTH+Vgwd=f-ynbuRVrS90XbWMT6UigaOQJTA@mail.gmail.com
Andrew Dunstan [Tue, 5 Feb 2019 23:57:12 +0000 (18:57 -0500)]
Fix included file path for modern perl
Contrary to the comment on 772d4b76, only paths starting with "./" or
"../" are considered relative to the current working directory by perl's
"do" function. So this patch converts all the relevant cases to use "./"
paths. This only affects MSVC.
Tom Lane [Tue, 5 Feb 2019 15:58:53 +0000 (10:58 -0500)]
Update time zone data files to tzdata release 2018i.
DST law changes in Kazakhstan, Metlakatla, and São Tomé and Príncipe.
Kazakhstan's Qyzylorda zone is split in two, creating a new zone
Asia/Qostanay, as some areas did not change UTC offset.
Historical corrections for Hong Kong and numerous Pacific islands.
Tom Lane [Tue, 5 Feb 2019 02:45:39 +0000 (21:45 -0500)]
Remove unnecessary "inline" marker introduced in commit 4be058fe9.
Some of our older buildfarm members bleat about this coding,
along the lines of
prepjointree.c:112: warning: 'get_result_relid' declared inline after being called
prepjointree.c:112: warning: previous declaration of 'get_result_relid' was here
Modern compilers will probably inline this function without being
prompted, so rather than move the function, let's just drop the
marking.
Tom Lane [Tue, 5 Feb 2019 00:18:49 +0000 (19:18 -0500)]
Doc: in each release branch, keep only that branch's own release notes.
Historically we've had each release branch include all prior branches'
notes, including minor-release changes, back to the beginning of the
project. That's basically an O(N^2) proposition, and it was starting to
catch up with us: as of HEAD the back-branch release notes alone accounted
for nearly 30% of the documentation. While there's certainly some value
in easy access to back-branch notes, this is getting out of hand.
Hence, switch over to the rule that each branch contains only its own
release notes. So as to not make older notes too hard to find, each
branch will provide URLs for the immediately preceding branches'
release notes on the project website.
There might be value in providing aggregated notes across all branches
somewhere on the website, but that's a task for another day.
Tom Lane [Mon, 4 Feb 2019 22:20:02 +0000 (17:20 -0500)]
Fix dumping of matviews with indirect dependencies on primary keys.
Commit 62215de29 turns out to have been not quite on-the-mark.
When we are forced to postpone dumping of a materialized view into
the dump's post-data section (because it depends on a unique index
that isn't created till that section), we may also have to postpone
dumping other matviews that depend on said matview. The previous fix
didn't reliably work for such cases: it'd break the dependency loops
properly, producing a workable object ordering, but it didn't
necessarily mark all the matviews as "postponed_def". This led to
harmless bleating about "archive items not in correct section order",
as reported by Tom Cassidy in bug #15602. Less harmlessly,
selective-restore options such as --section might misbehave due to
the matview dump objects not being properly labeled.
The right way to fix it is to consider that each pre-data dependency
we break amounts to moving the no-longer-dependent object into
post-data, and hence we should mark that object if it's a matview.
Back-patch to all supported versions, since the issue's been there
since matviews were introduced.
Andrew Gierth [Mon, 4 Feb 2019 18:47:33 +0000 (18:47 +0000)]
Move port-specific parts of with_temp_install to port makefile.
Rather than define ld_library_path_ver with a big nested $(if), just
put the overriding values in the makefiles for the relevant ports.
Also add a variable for port makefiles to append their own stuff to
with_temp_install, and use it to set LD_LIBRARY_PATH_RPATH=1 on
FreeBSD which is needed to make LD_LIBRARY_PATH override DT_RPATH
if DT_RUNPATH is not set (which seems to depend in unpredictable ways
on the choice of compiler, at least on my system).
Backpatch for the benefit of anyone doing regression tests on FreeBSD.
(For other platforms there should be no functional change.)
Amit Kapila [Mon, 4 Feb 2019 04:38:29 +0000 (10:08 +0530)]
Make FSM test portable.
In b0eaa4c51b, we allow FSM to be created only after 4 pages. One of the
tests check the FSM contents and to do that it populates many tuples in
the relation. The FSM contents depend on the availability of freespace in
the page and it could vary because of the alignment of tuples.
This commit removes the dependency on FSM contents.
Author: Amit Kapila
Discussion: https://postgr.es/m/CAA4eK1KADF6K1bagr0--mGv3dMcZ%3DH_Z-Qtvdfbp5PjaC6PJJA%40mail.gmail.com
Amit Kapila [Mon, 4 Feb 2019 02:19:15 +0000 (07:49 +0530)]
Avoid creation of the free space map for small heap relations, take 2.
Previously, all heaps had FSMs. For very small tables, this means that the
FSM took up more space than the heap did. This is wasteful, so now we
refrain from creating the FSM for heaps with 4 pages or fewer. If the last
known target block has insufficient space, we still try to insert into some
other page before giving up and extending the relation, since doing
otherwise leads to table bloat. Testing showed that trying every page
penalized performance slightly, so we compromise and try every other page.
This way, we visit at most two pages. Any pages with wasted free space
become visible at next relation extension, so we still control table bloat.
As a bonus, directly attempting one or two pages can even be faster than
consulting the FSM would have been.
Once the FSM is created for a heap we don't remove it even if somebody
deletes all the rows from the corresponding relation. We don't think it is
a useful optimization as it is quite likely that relation will again grow
to the same size.
Author: John Naylor, Amit Kapila Reviewed-by: Amit Kapila Tested-by: Mithun C Y
Discussion: https://www.postgresql.org/message-id/CAJVSVGWvB13PzpbLEecFuGFc5V2fsO736BsdTakPiPAcdMM5tQ@mail.gmail.com
Thomas Munro [Sun, 3 Feb 2019 08:55:39 +0000 (09:55 +0100)]
Add shared_memory_type GUC.
Since 9.3 we have used anonymous shared mmap for our main shared memory
region, except in EXEC_BACKEND builds. Provide a GUC so that users
can opt for System V shared memory once again, like in 9.2 and earlier.
A later patch proposes to add huge/large page support for AIX, which
requires System V shared memory and provided the motivation to revive
this possibility. It may also be useful on some BSDs.
Author: Andres Freund (revived and documented by Thomas Munro)
Discussion: https://postgr.es/m/HE1PR0202MB28126DB4E0B6621CC6A1A91286D90%40HE1PR0202MB2812.eurprd02.prod.outlook.com
Discussion: https://postgr.es/m/2AE143D2-87D3-4AD1-AC78-CE2258230C05%40FreeBSD.org
Andres Freund [Sun, 3 Feb 2019 09:27:19 +0000 (01:27 -0800)]
Move page initialization from RelationAddExtraBlocks() to use, take 2.
Previously we initialized pages when bulk extending in
RelationAddExtraBlocks(). That has a major disadvantage: It ties
RelationAddExtraBlocks() to heap, as other types of storage are likely
to need different amounts of special space, have different amount of
free space (previously determined by PageGetHeapFreeSpace()).
That we're relying on initializing pages, but not WAL logging the
initialization, also means the risk for getting
"WARNING: relation \"%s\" page %u is uninitialized --- fixing"
style warnings in vacuums after crashes/immediate shutdowns, is
considerably higher. The warning sounds much more serious than what
they are.
Fix those two issues together by not initializing pages in
RelationAddExtraPages() (but continue to do so in
RelationGetBufferForTuple(), which is linked much more closely to
heap), and accepting uninitialized pages as normal in
vacuumlazy.c. When vacuumlazy encounters an empty page it now adds it
to the FSM, but does nothing else. We chose to not issue a debug
message, much less a warning in that case - it seems rarely useful,
and quite likely to scare people unnecessarily.
For now empty pages aren't added to the VM, because standbys would not
re-discover such pages after a promotion. In contrast to other sources
for empty pages, there's no corresponding WAL records triggering FSM
updates during replay.
Previously when extending the relation, there was a moment between
extending the relation, and acquiring an exclusive lock on the new
page, in which another backend could lock the page. To avoid new
content being put on that new page, vacuumlazy needed to acquire the
extension lock for a brief moment when encountering a new page. A
second corner case, only working somewhat by accident, was that
RelationGetBufferForTuple() sometimes checks the last page in a
relation for free space, without consulting the FSM; that only worked
because PageGetHeapFreeSpace() interprets the zero page header in a
new page as no free space. The lack of handling this properly
required reverting the previous attempt in 684200543b.
This issue can be solved by using RBM_ZERO_AND_LOCK when extending the
relation, thereby avoiding this window. There's some added complexity
when RelationGetBufferForTuple() is called with another buffer (for
updates), to avoid deadlocks, but that's rarely hit at runtime.
Author: Andres Freund Reviewed-By: Tom Lane
Discussion: https://postgr.es/m/20181219083945.6khtgm36mivonhva@alap3.anarazel.de
Michael Paquier [Sun, 3 Feb 2019 08:48:09 +0000 (17:48 +0900)]
Add PG_CFLAGS, PG_CXXFLAGS, and PG_LDFLAGS variables to PGXS
Add PG_CFLAGS, PG_CXXFLAGS, and PG_LDFLAGS variables to pgxs.mk which
will be appended or prepended to the corresponding make variables.
Notably, there was previously no way to pass custom CXXFLAGS to third
party extension module builds, COPT and PROFILE supporting only CFLAGS
and LDFLAGS.
Backpatch all the way down to ease integration with existing
extensions.
Author: Christoph Berg Reviewed-by: Andres Freund, Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/20181113104005.GA32154@msg.credativ.de
Backpatch-through: 9.4
Amit Kapila [Sat, 2 Feb 2019 10:17:00 +0000 (15:47 +0530)]
Avoid possible deadlock while locking multiple heap pages.
To avoid deadlock, backend acquires a lock on heap pages in block
number order. In certain cases, lock on heap pages is dropped and
reacquired. In this case, the locks are dropped for reading in
corresponding VM page/s. The issue is we re-acquire locks in bufferId
order whereas the intention was to acquire in blockid order.
This commit ensures that we will always acquire locks on heap pages in
blockid order.
Reported-by: Nishant Fnu
Author: Nishant Fnu Reviewed-by: Amit Kapila and Robert Haas
Backpatch-through: 9.4
Discussion: https://postgr.es/m/5883C831-2ED1-47C8-BFAC-2D5BAE5A8CAE@amazon.com
Michael Paquier [Sat, 2 Feb 2019 04:23:26 +0000 (13:23 +0900)]
Improve installation instructions with pg_ctl in documentation
The documentation includes sections to be able to initialize and start
Postgres via a couple of commands. Some of its recommendations involve
using directly "postgres", which is inconsistent with the recommendation
given by initdb. At the same time make some other command calls more
consistent with the rest, by using an absolute path when creating a
database.
Author: Andreas Scherbaum Reviewed-by: Michael Banck, Ryan Lambert
Alvaro Herrera [Fri, 1 Feb 2019 15:50:32 +0000 (12:50 -0300)]
Renaming for new subscripting mechanism
Over at patch https://commitfest.postgresql.org/21/1062/ Dmitry wants to
introduce a more generic subscription mechanism, which allows
subscripting not only arrays but also other object types such as JSONB.
That functionality is introduced in a largish invasive patch, out of
which this internal renaming patch was extracted.
Author: Dmitry Dolgov Reviewed-by: Tom Lane, Arthur Zakirov
Discussion: https://postgr.es/m/CA+q6zcUK4EqPAu7XRRO5CCjMwhz5zvg+rfWuLzVoxp_5sKS6=w@mail.gmail.com
Alvaro Herrera [Fri, 1 Feb 2019 14:29:42 +0000 (11:29 -0300)]
Add ArchiveOpts to pass options to ArchiveEntry
The ArchiveEntry function has a number of arguments that can be
considered optional. Split them out into a separate struct, to make the
API more flexible for changes.
Alvaro Herrera [Fri, 1 Feb 2019 09:47:49 +0000 (06:47 -0300)]
Move building of child base quals out into a new function
An upcoming patch which changes how inheritance planning works requires
adding a new function that does a similar job to set_append_rel_size() but
for child target relations. To save it from having to duplicate the qual
building code, move that to a separate function first.
Here we also change things so that we never attempt to build security quals
after detecting some const false child quals. We needlessly used to do this
just before we marked the child relation as a dummy rel.
In passing, this also moves the partition pruned check to before the qual
building code. We don't need to build the child quals before we check if
the partition has been pruned.
Author: David Rowley
Discussion: https://postgr.es/m/CAKJS1f_i+jrrD+if8qC7KPuTAAWsd=dtepgY_7u=P86GDEwm7A@mail.gmail.com
Michael Paquier [Fri, 1 Feb 2019 01:46:45 +0000 (10:46 +0900)]
Adjust comment about timeout when waiting for WAL at recovery
A timeout of 5s is used when waiting for WAL to become available at
recovery so as the startup process is able to react promptly if a
trigger file shows up. However this missed the fact that the startup
process also relies on the timeout to check periodically the status of
any active WAL receiver.
Michael Paquier [Fri, 1 Feb 2019 01:35:16 +0000 (10:35 +0900)]
Fix use of dangling pointer in heap_delete() when logging replica identity
When logging the replica identity of a deleted tuple, XLOG_HEAP_DELETE
records include references of the old tuple. Its data is stored in an
intermediate variable used to register this information for the WAL
record, but this variable gets away from the stack when the record gets
actually inserted.
Michael Paquier [Thu, 31 Jan 2019 04:06:51 +0000 (13:06 +0900)]
Add --min-xid-age and --min-mxid-age options to vacuumdb
These two new options can be used to improve the selectivity of
relations to vacuum or analyze even further depending on the age of
respectively their transaction ID or multixact ID, so as it is possible
to prioritize tables to prevent wraparound of one or the other.
Combined with --table, it is possible to target a subset of tables to
choose as potential processing targets.
Author: Nathan Bossart Reviewed-by: Michael Paquier, Masahiko Sawada
Discussion: https://postgr.es/m/FFE5373C-E26A-495B-B5C8-911EC4A41C5E@amazon.com
Tom Lane [Thu, 31 Jan 2019 00:25:33 +0000 (19:25 -0500)]
Allow RECORD and RECORD[] to be specified in function coldeflists.
We can't allow these pseudo-types to be used as table column types,
because storing an anonymous record value in a table would result
in data that couldn't be understood by other sessions. However,
it seems like there's no harm in allowing the case in a column
definition list that's specifying what a function-returning-record
returns. The data involved is all local to the current session,
so we should be just as able to resolve its actual tuple type as
we are for the function-returning-record's top-level tuple output.
Peter Eisentraut [Wed, 30 Jan 2019 22:26:10 +0000 (23:26 +0100)]
Log PostgreSQL version number on startup
Logging the PostgreSQL version on startup is useful for two reasons:
There is a clear marker in the log file that a new postmaster is
beginning, and it's useful for tracking the server version across
startup while upgrading.
Author: Christoph Berg <christoph.berg@credativ.de>
Discussion: https://www.postgresql.org/message-id/flat/20181121144611.GJ15795@msg.credativ.de/
Peter Eisentraut [Wed, 16 Jan 2019 16:32:01 +0000 (17:32 +0100)]
postmaster: Start syslogger earlier
When the syslogger was originally
added (bdf8ef6925de6ea1a9330fa1ce32e1a315d07eb2), nothing was normally
logged before the point where it was started. But since f9dfa5c9776649f769d537dd0923003b35f128de, the creation of sockets
causes messages of level LOG to be written routinely, so those don't
go to the syslogger now.
To improve that, arrange the sequence in PostmasterMain() slightly so
that the syslogger is started early enough to capture those messages.
Discussion: https://www.postgresql.org/message-id/d5d50936-20b9-85f1-06bc-94a01c5040c1%402ndquadrant.com Reviewed-by: Christoph Berg <christoph.berg@credativ.de>
Michael Meskes [Wed, 30 Jan 2019 12:58:25 +0000 (13:58 +0100)]
Change error handling of out of scope variables in ecpg.
The function called can result in an out of memory error that subsequently was
disregarded. Instead it should set the appropriate SQL error variables and be
checked by whatever whenever statement is defined.
Peter Eisentraut [Mon, 28 Jan 2019 21:09:33 +0000 (22:09 +0100)]
Fix a crash in logical replication
The bug was that determining which columns are part of the replica
identity index using RelationGetIndexAttrBitmap() would run
eval_const_expressions() on index expressions and predicates across
all indexes of the table, which in turn might require a snapshot, but
there wasn't one set, so it crashes. There were actually two separate
bugs, one on the publisher and one on the subscriber.
To trigger the bug, a table that is part of a publication or
subscription needs to have an index with a predicate or expression
that lends itself to constant expressions simplification.
The fix is to avoid the constant expressions simplification in
RelationGetIndexAttrBitmap(), so that it becomes safe to call in these
contexts. The constant expressions simplification comes from the
calls to RelationGetIndexExpressions()/RelationGetIndexPredicate() via
BuildIndexInfo(). But RelationGetIndexAttrBitmap() calling
BuildIndexInfo() is overkill. The latter just takes pg_index catalog
information, packs it into the IndexInfo structure, which former then
just unpacks again and throws away. We can just do this directly with
less overhead and skip the troublesome calls to
eval_const_expressions(). This also removes the awkward
cross-dependency between relcache.c and index.c.
Bug: #15114 Reported-by: Петър Славов <pet.slavov@gmail.com> Reviewed-by: Noah Misch <noah@leadboat.com> Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/flat/152110589574.1223.17983600132321618383@wrigleys.postgresql.org/
Michael Paquier [Wed, 30 Jan 2019 00:44:08 +0000 (09:44 +0900)]
Do not filter by relkind in vacuumdb's catalog query if --table is used
If a user specifies a relation name which cannot be processed, then the
backend can warn directly about what is wrong with it. This fixes an
oversight from e0c2933.
Tom Lane [Tue, 29 Jan 2019 21:49:25 +0000 (16:49 -0500)]
Rename nodes/relation.h to nodes/pathnodes.h.
The old name of this file was never a very good indication of what it
was for. Now that there's also access/relation.h, we have a potential
confusion hazard as well, so let's rename it to something more apropos.
Per discussion, "pathnodes.h" is reasonable, since a good fraction of
the file is Path node definitions.
While at it, tweak a couple of other headers that were gratuitously
importing relation.h into modules that don't need it.
Tom Lane [Tue, 29 Jan 2019 20:48:51 +0000 (15:48 -0500)]
Refactor planner's header files.
Create a new header optimizer/optimizer.h, which exposes just the
planner functions that can be used "at arm's length", without need
to access Paths or the other planner-internal data structures defined
in nodes/relation.h. This is intended to provide the whole planner
API seen by most of the rest of the system; although FDWs still need
to use additional stuff, and more thought is also needed about just
what selfuncs.c should rely on.
The main point of doing this now is to limit the amount of new
#include baggage that will be needed by "planner support functions",
which I expect to introduce later, and which will be in relevant
datatype modules rather than anywhere near the planner.
This commit just moves relevant declarations into optimizer.h from
other header files (a couple of which go away because everything
got moved), and adjusts #include lists to match. There's further
cleanup that could be done if we want to decide that some stuff
being exposed by optimizer.h doesn't belong in the planner at all,
but I'll leave that for another day.
Tom Lane [Tue, 29 Jan 2019 20:26:44 +0000 (15:26 -0500)]
Make some small planner API cleanups.
Move a few very simple node-creation and node-type-testing functions
from the planner's clauses.c to nodes/makefuncs and nodes/nodeFuncs.
There's nothing planner-specific about them, as evidenced by the
number of other places that were using them.
While at it, rename and_clause() etc to is_andclause() etc, to clarify
that they are node-type-testing functions not node-creation functions.
And use "static inline" implementations for the shortest ones.
Also, modify flatten_join_alias_vars() and some subsidiary functions
to take a Query not a PlannerInfo to define the join structure that
Vars should be translated according to. They were only using the
"parse" field of the PlannerInfo anyway, so this just requires removing
one level of indirection. The advantage is that now parse_agg.c can
use flatten_join_alias_vars() without the horrid kluge of creating an
incomplete PlannerInfo, which will allow that file to be decoupled from
relation.h in a subsequent patch.
Peter Eisentraut [Tue, 29 Jan 2019 09:29:07 +0000 (10:29 +0100)]
Make SSL tests more robust
Someone running these test could have key or certificate files in
their ~/.postgresql/, which would interfere with the tests. The way
to override that is to specify sslcert=invalid and/or
sslrootcert=invalid if no actual certificate is used for a particular
test. Document that and fix up one test that had a risk of failing in
these circumstances.
Michael Paquier [Tue, 29 Jan 2019 02:22:03 +0000 (11:22 +0900)]
Use catalog query to discover tables to process in vacuumdb
vacuumdb would use a catalog query only when the command caller does not
define a list of tables. Switching to a catalog table represents two
advantages:
- Relation existence check can happen before running any VACUUM or
ANALYZE query. Before this change, if multiple relations are defined
using --table, the utility would fail only after processing the
firstly-defined ones, which may be a long some depending on the size of
the relation. This adds checks for the relation names, and does
nothing, at least yet, for the attribute names.
- More filtering options can become available for the utility user.
These options, which may be introduced later on, are based on the
relation size or the relation age, and need to be made available even if
the user does not list any specific table with --table.
Author: Nathan Bossart Reviewed-by: Michael Paquier, Masahiko Sawada
Discussion: https://postgr.es/m/FFE5373C-E26A-495B-B5C8-911EC4A41C5E@amazon.com
Andres Freund [Tue, 29 Jan 2019 02:05:52 +0000 (18:05 -0800)]
Fix LLVM related headers to compile standalone (to fix cpluspluscheck).
Previously llvmjit.h #error'ed when USE_LLVM was not defined, to
prevent it from being included from code not having #ifdef USE_LLVM
guards - but that's not actually that useful after, during the
development of JIT support, LLVM related code was moved into a
separately compiled .so. Having that #error means cpluspluscheck
doesn't work when llvm support isn't enabled, which isn't great.
Similarly add USE_LLVM guards to llvmjit_emit.h, and additionally make
sure it compiles standalone.
Per complaint from Tom Lane.
Author: Andres Freund
Discussion: https://postgr.es/m/19808.1548692361@sss.pgh.pa.us
Backpatch: 11, where JIT support was added
Parts of the buildfarm error out with
ERROR: page %u of relation "%s" should be empty but is not
errors, and so far I/we do not know why. fc02e672 didn't fix the
issue. As I cannot reproduce the issue locally, it seems best to get
the buildfarm green again, and reproduce the issue without time
pressure.
Andres Freund [Mon, 28 Jan 2019 23:41:23 +0000 (15:41 -0800)]
Fix race condition between relation extension and vacuum.
In e6799d5a5301 I removed vacuumlazy.c trickery around re-checking
whether a page is actually empty after acquiring an extension lock on
the relation, because the page is not PageInit()ed anymore, and
entries in the FSM ought not to lead to user-visible errors.
As reported by various buildfarm animals that is not correct, given
the way to code currently stands: If vacuum processes a page that's
just been newly added by either RelationGetBufferForTuple() or
RelationAddExtraBlocks(), it could add that page to the FSM and it
could be reused by other backends, before those two functions check
whether the newly added page is actually new. That's a relatively
narrow race, but several buildfarm machines appear to be able to hit
it.
While it seems wrong that the FSM, given it's lack of durability and
approximative nature, can trigger errors like this, that seems better
fixed in a separate commit. Especially given that a good portion of
the buildfarm is red, and this is just re-introducing logic that
existed a few hours ago.
Author: Andres Freund
Discussion: https://postgr.es/m/20190128222259.zhi7ovzgtkft6em6@alap3.anarazel.de
projects / postgresql / log