Thomas Munro [Tue, 6 Nov 2018 20:50:01 +0000 (09:50 +1300)]
Provide pg_pread() and pg_pwrite() for random I/O.
Forward to POSIX pread() and pwrite(), or emulate them if unavailable.
The emulation is not perfect as the file position is changed, so
we'll put pg_ prefixes on the names to minimize the risk of confusion
in future patches that might inadvertently try to mix pread() and read()
on the same file descriptor.
Author: Thomas Munro Reviewed-by: Tom Lane, Jesper Pedersen
Discussion: https://postgr.es/m/CAEepm=02rapCpPR3ZGF2vW=SBHSdFYO_bz_f-wwWJonmA3APgw@mail.gmail.com
Tom Lane [Tue, 6 Nov 2018 18:25:24 +0000 (13:25 -0500)]
Rename rbtree.c functions to use "rbt" prefix not "rb" prefix.
The "rb" prefix is used by Ruby, so that our existing code results
in name collisions that break plruby. We discussed ways to prevent
that by adjusting dynamic linker options, but it seems that at best
we'd move the pain to other cases. Renaming to avoid the collision
is the only portable fix anyway. Fortunately, our rbtree code is
not (yet?) widely used --- in core, there's only a single usage
in GIN --- so it seems likely that we can get away with a rename.
I chose to do this basically as s/rb/rbt/g, except for places where
there already was a "t" after "rb". The patch could have been made
smaller by only touching linker-visible symbols, but it would have
resulted in oddly inconsistent-looking code. Better to make it look
like "rbt" was the plan all along.
Back-patch to v10. The rbtree.c code exists back to 9.5, but
rb_iterate() which is the actual immediate source of pain was added
in v10, so it seems like changing the names before that would have
more risk than benefit.
Tom Lane [Tue, 6 Nov 2018 15:57:51 +0000 (10:57 -0500)]
Remove useless symbol from Makefile.global.
I added HAVE_IPV6 to Makefile.global way back in commit 7703e55c3
so that we could transmit its value to the shell-script version of
initdb. Since initdb was rewritten in C, it's been finding that
out from pg_config.h instead, so this is useless. Keeping it here
just wastes configure and make cycles, plus it's a potential
two-sources-of-truth problem.
Michael Paquier [Tue, 6 Nov 2018 05:11:21 +0000 (14:11 +0900)]
Switch pg_promote to be parallel-safe
pg_promote uses nothing relying on a global state, so it is fine to mark
it as parallel-safe, conclusion based on a detailed analysis from Robert
Haas. This also fixes an inconsistency where pg_proc.dat missed to mark
the function with its previous value for proparallel, update which does
not matter now as the default is used.
Based on a discussion between multiple folks: Laurenz Albe, Robert Haas,
Amit Kapila, Tom Lane and myself.
Thomas Munro [Tue, 6 Nov 2018 03:11:12 +0000 (16:11 +1300)]
Remove dsm_resize() and dsm_remap().
These interfaces were never used in core, didn't handle failure of
posix_fallocate() correctly and weren't supported on all platforms.
We agreed to remove them in 12.
Author: Thomas Munro Reported-by: Andres Freund
Discussion: https://postgr.es/m/CAA4eK1%2B%3DyAFUvpFoHXFi_gm8YqmXN-TtkFH%2BVYjvDLS6-SFq-Q%40mail.gmail.com
Tom Lane [Mon, 5 Nov 2018 21:07:06 +0000 (16:07 -0500)]
Last-minute updates for release notes.
I removed the item about the pg_stat_statements change from
release-11.sgml, as part of a sweep to delete items already committed
in 11.0; but actually we'd best keep it to ensure that people who've
pg_upgraded their databases will take the requisite action. Also make
said action more visible by making it into its own para. Noted by
Jonathan Katz.
Andres Freund [Mon, 5 Nov 2018 20:02:25 +0000 (12:02 -0800)]
Fix copy-paste error in errhint() introduced in 691d79a07933.
Reported-By: Petr Jelinek
Discussion: https://postgr.es/m/c95a620b-34f0-7930-aeb5-f7ab804f26cb@2ndquadrant.com
Backpatch: 9.4-, like the previous commit
Tom Lane [Mon, 5 Nov 2018 17:02:27 +0000 (12:02 -0500)]
Remove unreferenced pg_opfamily entry.
The entry with OID 4035, for GIST jsonb_ops, is unused; apparently
it was added in preparation for index support that never materialized.
Remove it, and add a regression test case to detect future mistakes
of the same kind.
Michael Paquier [Mon, 5 Nov 2018 02:04:02 +0000 (11:04 +0900)]
Block creation of partitions with open references to its parent
When a partition is created as part of a trigger processing, it is
possible that the partition which just gets created changes the
properties of the table the executor of the ongoing command relies on,
causing a subsequent crash. This has been found possible when for
example using a BEFORE INSERT which creates a new partition for a
partitioned table being inserted to.
Any attempt to do so is blocked when working on a partition, with
regression tests added for both CREATE TABLE PARTITION OF and ALTER
TABLE ATTACH PARTITION.
Reported-by: Dmitry Shalashov
Author: Amit Langote Reviewed-by: Michael Paquier, Tom Lane
Discussion: https://postgr.es/m/15437-3fe01ee66bd1bae1@postgresql.org
Backpatch-through: 10
Michael Paquier [Mon, 5 Nov 2018 00:14:33 +0000 (09:14 +0900)]
Ignore partitioned tables when processing ON COMMIT DELETE ROWS
Those tables have no physical storage, making this option unusable with
partition trees as at commit time an actual truncation was attempted.
There are still issues with the way ON COMMIT actions are done when
mixing several action types, however this impacts as well inheritance
trees, so this issue will be dealt with later.
Reported-by: Rajkumar Raghuwanshi
Author: Amit Langote Reviewed-by: Michael Paquier, Tom Lane
Discussion: https://postgr.es/m/CAKcux6mhgcjSiB_egqEAEFgX462QZtncU8QCAJ2HZwM-wWGVew@mail.gmail.com
Tom Lane [Sun, 4 Nov 2018 19:50:55 +0000 (14:50 -0500)]
Fix ExecuteCallStmt to not scribble on the passed-in parse tree.
Modifying the parse tree at execution time is, or at least ought to be,
verboten. It seems quite difficult to actually cause a crash this way
in v11 (although you can exhibit it pretty easily in HEAD by messing
with plan_cache_mode). Nonetheless, it's risky, so fix and back-patch.
Tom Lane [Sun, 4 Nov 2018 18:25:39 +0000 (13:25 -0500)]
Fix bugs in plpgsql's handling of CALL argument lists.
exec_stmt_call() tried to extract information out of a CALL statement's
argument list without using expand_function_arguments(), apparently in
the hope of saving a few nanoseconds by not processing defaulted
arguments. It got that quite wrong though, leading to crashes with
named arguments, as well as failure to enforce writability of the
argument for a defaulted INOUT parameter. Fix and simplify the logic
by using expand_function_arguments() before examining the list.
Also, move the argument-examination to just after producing the CALL
command's plan, before invoking the called procedure. This ensures
that we'll track possible changes in the procedure's argument list
correctly, and avoids a hazard of the plan cache being flushed while
the procedure executes.
Also fix assorted falsehoods and omissions in associated documentation.
Per bug #15477 from Alexey Stepanov.
Patch by me, with some help from Pavel Stehule. Back-patch to v11.
Andres Freund [Sat, 3 Nov 2018 22:55:23 +0000 (15:55 -0700)]
Prevent generating EEOP_AGG_STRICT_INPUT_CHECK operations when nargs == 0.
This only became a problem with 4c640f4f38, which didn't synchronize
the value agg_strict_input_check.nargs is set to, with the guard
condition for emitting the operation.
Besides such instructions being unnecessary overhead, currently the
LLVM JIT provider doesn't support them. It seems more sensible to
avoid generating such instruction than supporting them. Add assertions
to make it easier to debug a potential further occurance.
Andres Freund [Sat, 3 Nov 2018 21:35:23 +0000 (14:35 -0700)]
Fix STRICT check for strict aggregates with NULL ORDER BY columns.
I (Andres) broke this unintentionally in 69c3936a14, by checking
strictness for all input expressions computed for an aggregate, rather
than just the input for the aggregate transition function.
Reported-By: Ondřej Bouda Bisected-By: Tom Lane Diagnosed-By: Andrew Gierth
Discussion: https://postgr.es/m/2a505161-2727-2473-7c46-591ed108ac52@email.cz
Backpatch: 11-, like 69c3936a14
Tom Lane [Sat, 3 Nov 2018 17:56:10 +0000 (13:56 -0400)]
Make ts_locale.c's character-type functions cope with UTF-16.
On Windows, in UTF8 database encoding, what char2wchar() produces is
UTF16 not UTF32, ie, characters above U+FFFF will be represented by
surrogate pairs. t_isdigit() and siblings did not account for this
and failed to provide a large enough result buffer. That in turn
led to bogus "invalid multibyte character for locale" errors, because
contrary to what you might think from char2wchar()'s documentation,
its Windows code path doesn't cope sanely with buffer overflow.
The solution for t_isdigit() and siblings is pretty clear: provide
a 3-wchar_t result buffer not 2.
char2wchar() also needs some work to provide more consistent, and more
accurately documented, buffer overrun behavior. But that's a bigger job
and it doesn't actually have any immediate payoff, so leave it for later.
Per bug #15476 from Kenji Uno, who deserves credit for identifying the
cause of the problem. Back-patch to all active branches.
Alvaro Herrera [Sat, 3 Nov 2018 16:23:40 +0000 (13:23 -0300)]
Fix tablespace handling for partitioned indexes
When creating partitioned indexes, the tablespace was not being saved
for the parent index. This meant that subsequently created partitions
would not use the right tablespace for their indexes.
ALTER INDEX SET TABLESPACE and ALTER INDEX ALL IN TABLESPACE raised
errors when tried; fix them too. This requires bespoke code for
ATExecCmd() that applies to the special case when the tablespace move is
just a catalog change.
Tom Lane [Sat, 3 Nov 2018 00:11:11 +0000 (20:11 -0400)]
First-draft release notes for 11.1.
As usual, the release notes for other branches will be made by cutting
these down, but put them up for community review first. Note that a
fair percentage of the entries apply only to prior branches because
their issue was already fixed in 11.0.
Tom Lane [Fri, 2 Nov 2018 22:54:00 +0000 (18:54 -0400)]
Yet further rethinking of build changes for macOS Mojave.
The solution arrived at in commit e74dd00f5 presumes that the compiler
has a suitable default -isysroot setting ... but further experience
shows that in many combinations of macOS version, XCode version, Xcode
command line tools version, and phase of the moon, Apple's compiler
will *not* supply a default -isysroot value.
We could potentially go back to the approach used in commit 68fc227dd,
but I don't have a lot of faith in the reliability or life expectancy of
that either. Let's just revert to the approach already shipped in 11.0,
namely specifying an -isysroot switch globally. As a partial response to
the concerns raised by Jakob Egger, adjust the contents of Makefile.global
to look like
This allows overriding the sysroot path at build time in a relatively
painless way.
Add documentation to installation.sgml about how to use the PG_SYSROOT
option. I also took the opportunity to document how to work around
macOS's "System Integrity Protection" feature.
Thomas Munro [Fri, 2 Nov 2018 22:05:35 +0000 (11:05 +1300)]
Fix NULL handling in multi-batch Parallel Hash Left Join.
NULL keys in left joins were skipped when building batch files.
Repair, by making the keep_nulls argument to ExecHashGetHashValue()
depend on whether this is a left outer join, as we do in other
paths.
Bug #15475. Thinko in 1804284042e. Back-patch to 11.
Reported-by: Paul Schaap Diagnosed-by: Andrew Gierth
Dicussion: https://postgr.es/m/15475-11a7a783fed72a36%40postgresql.org
Michael Paquier [Thu, 1 Nov 2018 22:59:24 +0000 (07:59 +0900)]
Lower error level from PANIC to FATAL when restoring slots at startup
When restoring slot information from disk at startup and filling in
shared memory information, the startup process would issue a PANIC
message if more slots are found than what max_replication_slots allows,
and then Postgres generates a core dump, recommending to increase
max_replication_slots. This gives users a switch to crash Postgres at
will by creating slots, lower the configuration to not support it, and
then restart it.
Making Postgres crash hard in this case is overdoing it just to give a
recommendation to users. So instead use a FATAL, which makes Postgres
fail to start without crashing, still giving the recommendation. This
is more consistent with what happens for prepared transactions for
example.
Author: Michael Paquier Reviewed-by: Andres Freund
Discussion: https://postgr.es/m/20181030025109.GD1644@paquier.xyz
Peter Geoghegan [Thu, 1 Nov 2018 16:18:57 +0000 (09:18 -0700)]
Adjust trace_sort log messages.
The project message style guide dictates: "When citing the name of an
object, state what kind of object it is". The parallel CREATE INDEX
patch added a worker number to most of the trace_sort messages within
tuplesort.c without specifying the object type. Bring these messages
into compliance with the style guide.
We're still treating a leader or serial Tuplesortstate as having worker
number -1. trace_sort is a developer option, and these two cases are
highly comparable, so this seems appropriate.
Per complaint from Tom Lane.
Discussion: https://postgr.es/m/8330.1540831863@sss.pgh.pa.us
Backpatch: 11-, where parallel CREATE INDEX was introduced.
Andres Freund [Wed, 31 Oct 2018 21:47:41 +0000 (14:47 -0700)]
Disallow starting server with insufficient wal_level for existing slot.
Previously it was possible to create a slot, change wal_level, and
restart, even if the new wal_level was insufficient for the
slot. That's a problem for both logical and physical slots, because
the necessary WAL records are not generated.
This removes a few tests in newer versions that, somewhat
inexplicably, whether restarting with a too low wal_level worked (a
buggy behaviour!).
Reported-By: Joshua D. Drake
Author: Andres Freund
Discussion: https://postgr.es/m/20181029191304.lbsmhshkyymhw22w@alap3.anarazel.de
Backpatch: 9.4-, where replication slots where introduced
Tom Lane [Wed, 31 Oct 2018 21:04:42 +0000 (17:04 -0400)]
Fix memory leak in repeated SPGIST index scans.
spgendscan neglected to pfree all the memory allocated by spgbeginscan.
It's possible to get away with that in most normal queries, since the
memory is allocated in the executor's per-query context which is about
to get deleted anyway; but it causes severe memory leakage during
creation or filling of large exclusion-constraint indexes.
Also, document that amendscan is supposed to free what ambeginscan
allocates. The docs' lack of clarity on that point probably caused this
bug to begin with. (There is discussion of changing that API spec going
forward, but I don't think it'd be appropriate for the back branches.)
Per report from Bruno Wolff. It's been like this since the beginning,
so back-patch to all active branches.
In HEAD, also fix an independent leak caused by commit 2a6368343
(allocating memory during spgrescan instead of spgbeginscan, which
might be all right if it got cleaned up, but it didn't). And do a bit
of code beautification on that commit, too.
Tom Lane [Wed, 31 Oct 2018 13:47:53 +0000 (09:47 -0400)]
Sync our copy of the timezone library with IANA release tzcode2018g.
This patch absorbs an upstream fix to "zic" for a recently-introduced
bug that made it output data that some 32-bit clients couldn't read.
Given the current source data, the bug only manifests in zones with
leap seconds, which we don't generate, so that there's no actual
change in our installed timezone data files from this. Still, in
case somebody uses our copy of "zic" to do something else, it seems
best to apply the fix promptly.
Also, update the README's notes about converting upstream code to
our conventions.
Tom Lane [Tue, 30 Oct 2018 19:26:11 +0000 (15:26 -0400)]
Fix interaction of CASE and ArrayCoerceExpr.
An array-type coercion appearing within a CASE that has a constant
(after const-folding) test expression was mangled by the planner, causing
all the elements of the resulting array to be equal to the coerced value
of the CASE's test expression. This is my oversight in commit c12d570fa:
that changed ArrayCoerceExpr to use a subexpression involving a
CaseTestExpr, and I didn't notice that eval_const_expressions needed an
adjustment to keep from folding such a CaseTestExpr to a constant when
it's inside a suitable CASE.
This is another in what's getting to be a depressingly long line of bugs
associated with misidentification of the referent of a CaseTestExpr.
We're overdue to redesign that mechanism; but any such fix is unlikely
to be back-patchable into v11. As a stopgap, fix eval_const_expressions
to do what it must here. Also add a bunch of comments pointing out the
restrictions and assumptions that are needed to make this work at all.
Also fix a related oversight: contain_context_dependent_node() was not
aware of the relationship of ArrayCoerceExpr to CaseTestExpr. That was
somewhat fail-soft, in that the outcome of a wrong answer would be to
prevent optimizations that could have been made, but let's fix it while
we're at it.
Per bug #15471 from Matt Williams. Back-patch to v11 where the faulty
logic came in.
Michael Paquier [Tue, 30 Oct 2018 02:38:35 +0000 (11:38 +0900)]
Consolidate cross-option checks in pg_restore
This moves one check for conflicting options from the archive restore
code to the main function where other similar checks are performed.
Also reword the error message to be consistent with other messages.
The only option combination impacted is --create specified with
--single-transaction, and informing the caller at an early step saves
from opening the archive worked on. A TAP test is added for this
combination.
Author: Daniel Gustafsson Reviewed-by: Fabien Coelho
Discussion: https://postgr.es/m/616808BD-4B59-4E6C-97A9-7317F62D5570@yesql.se
Michael Paquier [Tue, 30 Oct 2018 01:25:06 +0000 (10:25 +0900)]
Add pg_partition_tree to display information about partitions
This new function is useful to display a full tree of partitions with a
partitioned table given in output, and avoids the need of any complex
WITH RECURSIVE query when looking at partition trees which are
deep multiple levels.
It returns a set of records, one for each partition, containing the
partition's name, its immediate parent's name, a boolean value telling
if the relation is a leaf in the tree and an integer telling its level
in the partition tree with given table considered as root, beginning at
zero for the root, and incrementing by one each time the scan goes one
level down.
Author: Amit Langote Reviewed-by: Jesper Pedersen, Michael Paquier, Robert Haas
Discussion: https://postgr.es/m/8d00e51a-9a51-ad02-d53e-ba6bf50b2e52@lab.ntt.co.jp
Peter Eisentraut [Mon, 29 Oct 2018 10:39:44 +0000 (11:39 +0100)]
Exclude temporary directories from pgindent
Exclude tmp_check and tmp_install from pgindent. In a fully-built
tree, pgindent would spend a lot of time digging through these
directories and ends up re-indenting installed header files.
Michael Paquier [Mon, 29 Oct 2018 07:38:54 +0000 (16:38 +0900)]
Improve description of pg_attrdef in documentation
The reference to pg_attribute is switched to a link, which is more
useful for the html documentation. The conditions under which a default
value is defined for a given column are made more general.
Author: Daniel Gustafsson Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/0E8748E3-8B7D-445E-9ABA-09DA5C7345CC@yesql.se
Andrew Dunstan [Sun, 28 Oct 2018 16:22:32 +0000 (12:22 -0400)]
Fix perl searchpath for modern perl for MSVC tools
Modern versions of perl no longer include the current directory in the
perl searchpath, as it's insecure. Instead of adding the current
directory, we get around the problem by adding the directory where the
script lives.
Michael Paquier [Fri, 26 Oct 2018 04:46:20 +0000 (13:46 +0900)]
Improve tab completion of CREATE EVENT TRIGGER in psql
This adds tab completion of the clauses WHEN and EXECUTE
FUNCTION|PROCEDURE clauses to CREATE EVENT TRIGGER, similar to CREATE
TRIGGER in the previous commit. This has version-dependent logic so as
FUNCTION is chosen over PROCEDURE for 11 and newer versions.
Author: Dagfinn Ilmari Mannsåker Reviewed-by: Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/d8jmur4q4yc.fsf@dalvik.ping.uio.no
Michael Paquier [Fri, 26 Oct 2018 00:30:43 +0000 (09:30 +0900)]
Add tab completion of EXECUTE FUNCTION for CREATE TRIGGER in psql
The change to accept EXECUTE FUNCTION as well as EXECUTE PROCEDURE in
CREATE TRIGGER (added by 0a63f99) forgot to tell psql's tab completion
system about this. In passing, add tab completion of EXECUTE
FUNCTION/PROCEDURE after a complete WHEN ( … ) clause.
This change is version-aware, with FUNCTION being selected automatically
instead of PROCEDURE depending on the backend version, PROCEDURE being
an historical grammar kept for compatibility and considered as
deprecated in v11.
Author: Dagfinn Ilmari Mannsåker Reviewed-by: Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/d8jmur4q4yc.fsf@dalvik.ping.uio.no
Michael Paquier [Thu, 25 Oct 2018 00:46:00 +0000 (09:46 +0900)]
Add pg_promote function
This function is able to promote a standby with this new SQL-callable
function. Execution access can be granted to non-superusers so that
failover tools can observe the principle of least privilege.
Andrew Dunstan [Wed, 24 Oct 2018 14:45:34 +0000 (10:45 -0400)]
Correctly set t_self for heap tuples in expand_tuple
Commit 16828d5c0 incorrectly set an invalid pointer for t_self for heap
tuples. This patch correctly copies it from the source tuple, and
includes a regression test that relies on it being set correctly.
Michael Paquier [Wed, 24 Oct 2018 08:02:37 +0000 (17:02 +0900)]
List wait events in alphabetical order
This changes the documentation, and the related structures so as
everything is consistent.
Some wait events were not listed alphabetically since their
introduction, others have been added rather randomly. Keeping all those
entries in order helps in maintenance, and helps the user looking at the
documentation.
Author: Michael Paquier, Kuntal Ghosh
Discussion: https://postgr.es/m/20181024002539.GI1658@paquier.xyz
Backpatch-through: 10, only for the documentation part to avoid an ABI
breakage.
Michael Paquier [Mon, 22 Oct 2018 02:04:48 +0000 (11:04 +0900)]
Set pg_class.relhassubclass for partitioned indexes
Like for relations, switching this parameter is optimistic by turning it
on each time a partitioned index gains a partition. So seeing this
parameter set to true means that the partitioned index has or has had
partitions. The flag cannot be reset yet for partitioned indexes, which
is something not obvious anyway as partitioned relations exist to have
partitions.
This allows to track more conveniently partition trees for indexes,
which will come in use with an upcoming patch helping in listing
partition trees with an SQL-callable function.
Author: Amit Langote Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/80306490-b5fc-ea34-4427-f29c52156052@lab.ntt.co.jp
Andrew Dunstan [Sun, 21 Oct 2018 13:00:13 +0000 (09:00 -0400)]
Don't try to test files named with a trailing dot on Windows
The pg_verify_checksums test tries to create files with corrupt data
named "123." and "123_." But on Windows a file name with a trailing dot
is the same as a file without the trailing dot. In the first case this
will create a file with a "valid" name, which causes the test to fail in
an unexpected way, and in the secongd case this will be redandant as the
test already creates a file named "123_".
Andrew Dunstan [Sat, 20 Oct 2018 13:02:36 +0000 (09:02 -0400)]
Lower privilege level of programs calling regression_main
On Windows this mean that the regression tests can now safely and
successfully run as Administrator, which is useful in situations like
Appveyor. Elsewhere it's a no-op.
Backpatch to 9.5 - this is harder in earlier branches and not worth the
trouble.
Tom Lane [Sat, 20 Oct 2018 02:22:57 +0000 (22:22 -0400)]
Client-side fixes for delayed NOTIFY receipt.
PQnotifies() is defined to just process already-read data, not try to read
any more from the socket. (This is a debatable decision, perhaps, but I'm
hesitant to change longstanding library behavior.) The documentation has
long recommended calling PQconsumeInput() before PQnotifies() to ensure
that any already-arrived message would get absorbed and processed.
However, psql did not get that memo, which explains why it's not very
reliable about reporting notifications promptly.
Also, most (not quite all) callers called PQconsumeInput() just once before
a PQnotifies() loop. Taking this recommendation seriously implies that we
should do PQconsumeInput() before each call. This is more important now
that we have "payload" strings in notification messages than it was before;
that increases the probability of having more than one packet's worth
of notify messages. Hence, adjust code as well as documentation examples
to do it like that.
Back-patch to 9.5 to match related server fixes. In principle we could
probably go back further with these changes, but given lack of field
complaints I doubt it's worthwhile.
Tom Lane [Sat, 20 Oct 2018 01:39:21 +0000 (21:39 -0400)]
Server-side fix for delayed NOTIFY and SIGTERM processing.
Commit 4f85fde8e introduced some code that was meant to ensure that we'd
process cancel, die, sinval catchup, and notify interrupts while waiting
for client input. But there was a flaw: it supposed that the process
latch would be set upon arrival at secure_read() if any such interrupt
was pending. In reality, we might well have cleared the process latch
at some earlier point while those flags remained set -- particularly
notifyInterruptPending, which can't be handled as long as we're within
a transaction.
To fix the NOTIFY case, also attempt to process signals (except
ProcDiePending) before trying to read.
Also, if we see that ProcDiePending is set before we read, forcibly set the
process latch to ensure that we will handle that signal promptly if no data
is available. I also made it set the process latch on the way out, in case
there is similar logic elsewhere. (It remains true that we won't service
ProcDiePending here unless we need to wait for input.)
The code for handling ProcDiePending during a write needs those changes,
too.
Also be a little more careful about when to reset whereToSendOutput,
and improve related comments.
Back-patch to 9.5 where this code was added. I'm not entirely convinced
that older branches don't have similar issues, but the complaint at hand
is just about the >= 9.5 code.
Tom Lane [Fri, 19 Oct 2018 23:36:34 +0000 (19:36 -0400)]
Sync our copy of the timezone library with IANA release tzcode2018f.
About half of this is purely cosmetic changes to reduce the diff between
our code and theirs, like inserting "const" markers where they have them.
The other half is tracking actual code changes in zic.c and localtime.c.
I don't think any of these represent near-term compatibility hazards, but
it seems best to stay up to date.
I also fixed longstanding bugs in our code for producing the
known_abbrevs.txt list, which by chance hadn't been exposed before,
but which resulted in some garbage output after applying the upstream
changes in zic.c. Notably, because upstream removed their old phony
transitions at the Big Bang, it's now necessary to cope with TZif files
containing no DST transition times at all.
Tom Lane [Fri, 19 Oct 2018 21:01:34 +0000 (17:01 -0400)]
Update time zone data files to tzdata release 2018f.
DST law changes in Chile, Fiji, and Russia (Volgograd).
Historical corrections for China, Japan, Macau, and North Korea.
Note: like the previous tzdata update, this involves a depressingly
large amount of semantically-meaningless churn in tzdata.zi. That
is a consequence of upstream's data compression method assigning
unstable abbreviations to DST rulesets. I complained about that
to them last time, and this version now uses an assignment method
that pays some heed to not changing abbreviations unnecessarily.
So hopefully, that'll be better going forward.
Michael Paquier [Fri, 19 Oct 2018 13:44:12 +0000 (22:44 +0900)]
Use whitelist to choose files scanned with pg_verify_checksums
The original implementation of pg_verify_checksums used a blacklist to
decide which files should be skipped for scanning as they do not include
data checksums, like pg_internal.init or pg_control. However, this
missed two things:
- Some files are created within builds of EXEC_BACKEND and these were
not listed, causing failures on Windows.
- Extensions may create custom files in data folders, causing the tool
to equally fail.
This commit switches to a whitelist-like method instead by checking if
the files to scan are authorized relation files. This is close to a
reverse-engineering of what is defined in relpath.c in charge of
building the relation paths, and we could consider refactoring what this
patch does so as all routines are in a single place. This is left for
later.
This is based on a suggestion from Andres Freund. TAP tests are updated
so as multiple file patterns are tested. The bug has been spotted by
various buildfarm members as a result of b34e84f which has introduced
the TAP tests of pg_verify_checksums.
Author: Michael Paquier Reviewed-by: Andrew Dunstan, Michael Banck
Discussion: https://postgr.es/m/20181012005614.GC26424@paquier.xyz
Backpatch-through: 11
Thomas Munro [Fri, 19 Oct 2018 00:59:14 +0000 (13:59 +1300)]
Refactor pid, random seed and start time initialization.
Background workers, including parallel workers, were generating
the same sequence of numbers in random(). This showed up as DSM
handle collisions when Parallel Hash created multiple segments,
but any code that calls random() in background workers could be
affected if it cares about different backends generating different
numbers.
Repair by making sure that all new processes initialize the seed
at the same time as they set MyProcPid and MyStartTime in a new
function InitProcessGlobals(), called by the postmaster, its
children and also standalone processes. Also add a new high
resolution MyStartTimestamp as a potentially useful by-product,
and remove SessionStartTime from struct Port as it is now
redundant.
No back-patch for now, as the known consequences so far are just
a bunch of harmless shm_open(O_EXCL) collisions.
Author: Thomas Munro Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/CAEepm%3D2eJj_6%3DB%2B2tEpGu2nf1BjthCf9nXXUouYvJJ4C5WSwhg%40mail.gmail.com
Tom Lane [Thu, 18 Oct 2018 18:55:23 +0000 (14:55 -0400)]
Still further rethinking of build changes for macOS Mojave.
To avoid the sorts of problems complained of by Jakob Egger, it'd be
best if configure didn't emit any references to the sysroot path at all.
In the case of PL/Tcl, we can do that just by keeping our hands off the
TCL_INCLUDE_SPEC string altogether. In the case of PL/Perl, we need to
substitute -iwithsysroot for -I in the compile commands, which is easily
handled if we change to using a configure output variable that includes
the switch not only the directory name. Since PL/Tcl and PL/Python
already do it like that, this seems like good consistency cleanup anyway.
Hence, this replaces the advice given to Perl-related extensions in commit 5e2217131; instead of writing "-I$(perl_archlibexp)/CORE", they should
just write "$(perl_includespec)". (The old way continues to work, but not
on recent macOS.)
It's still the case that configure needs to be aware of the sysroot
path internally, but that's cleaner than what we had before.
Tom Lane [Wed, 17 Oct 2018 20:41:00 +0000 (16:41 -0400)]
Improve some comments related to executor result relations.
es_leaf_result_relations doesn't exist; perhaps this was an old name
for es_tuple_routing_result_relations, or maybe this comment has gone
unmaintained through multiple rounds of whacking the code around.
Related comment in execnodes.h was both obsolete and ungrammatical.
Tom Lane [Wed, 17 Oct 2018 19:06:38 +0000 (15:06 -0400)]
Fix minor bug in isolationtester.
If the lock wait query failed, isolationtester would report the
PQerrorMessage from some other connection, meaning there would be
no message or an unrelated one. This seems like a pretty unlikely
occurrence, but if it did happen, this bug could make it really
difficult/confusing to figure out what happened. That seems to
justify patching all the way back.
In passing, clean up another place where the "wrong" conn was used
for an error report. That one's not actually buggy because it's
a different alias for the same connection, but it's still confusing
to the reader.
Peter Eisentraut [Wed, 17 Oct 2018 18:31:20 +0000 (20:31 +0200)]
Fix crash in multi-insert COPY
A bug introduced in 0d5f05cde011512e605bb2688d9b1fbb5b3ae152
considered the *previous* partition's triggers when deciding whether
multi-insert can be used. Rearrange the code so that the current
partition is considered.
Tom Lane [Wed, 17 Oct 2018 18:22:33 +0000 (14:22 -0400)]
Minor additional improvements for ecpglib/prepare.c.
Avoid allocating never-used entries in stmtCacheEntries[], other than the
intentionally-unused zero'th entry. Tie the array size directly to the
bucket count and size, rather than having undocumented dependencies between
three magic constants. Fix the hash calculation to be platform-independent
--- notably, it was sensitive to the signed'ness of "char" before, not to
mention having an unnecessary hard-wired dependency on the existence and
size of type "long long". (The lack of complaints says it's been a long
time since anybody tried to build PG on a compiler without "long long",
and certainly with the requirement for C99 this isn't a live bug anymore.
But it's still not per project coding style.) Fix ecpg_auto_prepare's
new-cache-entry path so that it increments the exec count for the new
cache entry not the dummy zero'th entry.
The last of those is an actual bug, though one of little consequence;
the rest is mostly future-proofing and neatnik-ism. Doesn't seem
necessary to back-patch.
Tom Lane [Wed, 17 Oct 2018 16:26:48 +0000 (12:26 -0400)]
Improve tzparse's handling of TZDEFRULES ("posixrules") zone data.
In the IANA timezone code, tzparse() always tries to load the zone
file named by TZDEFRULES ("posixrules"). Previously, we'd hacked
that logic to skip the load in the "lastditch" code path, which we use
only to initialize the default "GMT" zone during GUC initialization.
That's critical for a couple of reasons: since we do not support leap
seconds, we *must not* allow "GMT" to have leap seconds, and since this
case runs before the GUC subsystem is fully alive, we'd really rather
not take the risk of pg_open_tzfile throwing any errors.
However, that still left the code reading TZDEFRULES on every other
call, something we'd noticed to the extent of having added code to cache
the result so it was only done once per process not a lot of times.
Andres Freund complained about the static data space used up for the
cache; but as long as the logic was like this, there was no point in
trying to get rid of that space.
We can improve matters by looking a bit more closely at what the IANA
code actually needs the TZDEFRULES data for. One thing it does is
that if "posixrules" is a leap-second-aware zone, the leap-second
behavior will be absorbed into every POSIX-style zone specification.
However, that's a behavior we'd really prefer to do without, since
for our purposes the end effect is to render every POSIX-style zone
name unsupported. Otherwise, the TZDEFRULES data is used only if
the POSIX zone name specifies DST but doesn't include a transition
date rule (e.g., "EST5EDT" rather than "EST5EDT,M3.2.0,M11.1.0").
That is a minority case for our purposes --- in particular, it
never happens when tzload() invokes tzparse() to interpret a
transition date rule string found in a tzdata zone file.
Hence, if we legislate that we're going to ignore leap-second data
from "posixrules", we can postpone the TZDEFRULES load into the path
where we actually need to substitute for a missing date rule string.
That means it will never happen at all in common scenarios, making it
reasonable to dynamically allocate the cache space when it does happen.
Even when the data is already loaded, this saves some cycles in the
common code path since we avoid a memcpy of 23KB or so. And, IMO at
least, this is a less ugly hack on the IANA logic than what we had
before, since it's not messing with the lastditch-vs-regular code paths.
Back-patch to all supported branches, not so much because this is a
critical change as that I want to keep all our copies of the IANA
timezone code in sync.
Tom Lane [Wed, 17 Oct 2018 04:04:48 +0000 (00:04 -0400)]
Avoid statically allocating statement cache in ecpglib/prepare.c.
This removes a megabyte of storage that isn't used at all in ecpglib's
default operating mode --- you have to enable auto-prepare to get any
use out of it. Seems well worth the trouble to allocate on demand.
Tom Lane [Wed, 17 Oct 2018 03:43:15 +0000 (23:43 -0400)]
Formatting cleanup in ecpglib/prepare.c.
Looking at this code made my head hurt. Format the comments more
like the way it's done elsewhere, break a few overly long lines.
No actual code changes in this commit.
Andres Freund [Tue, 16 Oct 2018 21:51:18 +0000 (14:51 -0700)]
Reorder FmgrBuiltin members, saving 25% in size.
That's worth it, as fmgr_builtins is frequently accessed, and as
fmgr_builtins is one of the biggest constant variables in a backend.
On most 64bit systems this will change the size of the struct from
32byte to 24bytes. While that could make indexing into the array
marginally more expensive, the higher cache hit ratio is worth more,
especially because these days fmgr_builtins isn't searched with a
binary search anymore (c.f. 212e6f34d5).
Tom Lane [Tue, 16 Oct 2018 20:27:15 +0000 (16:27 -0400)]
Back off using -isysroot on Darwin.
Rethink the solution applied in commit 5e2217131 to get PL/Tcl to
build on macOS Mojave. I feared that adding -isysroot globally might
have undesirable consequences, and sure enough Jakob Egger reported
one: it complicates building extensions with a different Xcode version
than was used for the core server. (I find that a risky proposition
in general, but apparently it works most of the time, so we shouldn't
break it if we don't have to.)
We'd already adopted the solution for PL/Perl of inserting the sysroot
path directly into the -I switches used to find Perl's headers, and we
can do the same thing for PL/Tcl by changing the -iwithsysroot switch
that Apple's tclConfig.sh reports. This restricts the risks to PL/Perl
and PL/Tcl themselves and directly-dependent extensions, which is a lot
more pleasing in general than a global -isysroot switch.
Along the way, tighten the test to see if we need to inject the sysroot
path into $perl_includedir, as I'd speculated about upthread but not
gotten round to doing.
Andres Freund [Tue, 16 Oct 2018 19:05:50 +0000 (12:05 -0700)]
Mark constantly allocated dest receiver as const.
This allows the compiler / linker to mark affected pages as read-only.
Doing so requires casting constness away, as CreateDestReceiver()
returns both constant and non-constant dest receivers. That's fine
though, as any modification of the statically allocated receivers
would already have been a bug (and would now be caught on some
platforms).
Andres Freund [Tue, 16 Oct 2018 19:05:50 +0000 (12:05 -0700)]
Add macro to cast away const without allowing changes to underlying type.
The new unconsitify(underlying_type, var) macro allows to cast
constness away from a variable, but doesn't allow changing the
underlying type. Enforcement of the latter currently only works for
gcc like compilers.
Please note IT IS NOT SAFE to cast constness away if the variable will ever
be modified (it would be undefined behaviour). Doing so anyway can cause
compiler misoptimizations or runtime crashes (modifying readonly memory).
It is only safe to use when the the variable will not be modified, but API
design or language restrictions prevent you from declaring that
(e.g. because a function returns both const and non-const variables).
This'll be used in an upcoming change, but seems like it's independent
infrastructure.
Author: Andres Freund
Discussion: https://postgr.es/m/20181015200754.7y7zfuzsoux2c4ya@alap3.anarazel.de
Tom Lane [Tue, 16 Oct 2018 18:57:14 +0000 (14:57 -0400)]
Be smarter about age-counter overflow in formatting.c caches.
The previous code here simply threw away whatever it knew about cache
entry ages whenever a counter overflow occurred. Since the counter
is int width and will be bumped once per format function execution,
overflows are not really so rare as to not be worth thinking about.
Instead, let's deal with the situation by halving all the age values,
essentially rescaling the age metric. In that way, we retain a
pretty accurate (if not quite perfect) idea of which entries are oldest.
Tom Lane [Tue, 16 Oct 2018 17:56:58 +0000 (13:56 -0400)]
Avoid rare race condition in privileges.sql regression test.
We created a temp table, then switched to a new session, leaving
the old session to clean up its temp objects in background. If that
took long enough, the eventual attempt to drop the user that owns
the temp table could fail, as exhibited today by sidewinder.
Fix by dropping the temp table explicitly when we're done with it.
It's been like this for quite some time, so back-patch to all
supported branches.
Tom Lane [Tue, 16 Oct 2018 17:11:05 +0000 (13:11 -0400)]
Avoid statically allocating formatting.c's format string caches.
This eliminates circa 120KB of static data from Postgres' memory
footprint. In some usage patterns that space will get allocated
anyway, but in many processes it never will be allocated.
We can improve matters further by allocating only as many cache
entries as we actually use, rather than allocating the whole array
on first use. However, to avoid wasting lots of space due to
palloc's habit of rounding requests up to power-of-2 sizes, tweak
the maximum cacheable format string length to make the struct sizes
be powers of 2 or just less. The sizes I chose make the maximums
a little bit less than they were before, but I doubt it matters much.
While at it, rearrange struct FormatNode to avoid wasting quite so
much padding space. This change actually halves the size of that
struct on 64-bit machines.
Andres Freund [Tue, 16 Oct 2018 16:44:43 +0000 (09:44 -0700)]
Correct constness of system attributes in heap.c & prerequisites.
This allows the compiler / linker to mark affected pages as read-only.
There's a fair number of pre-requisite changes, to allow the const
properly be propagated. Most of consts were already required for
correctness anyway, just not represented on the type-level. Arguably
we could be more aggressive in using consts in related code, but..
This requires using a few of the types underlying typedefs that
removes pointers (e.g. const NameData *) as declaring the typedefed
type constant doesn't have the same meaning (it makes the variable
const, not what it points to).
Tom Lane [Tue, 16 Oct 2018 16:27:14 +0000 (12:27 -0400)]
Make PostgresNode.pm's poll_query_until() more chatty about failures.
Reporting only the stderr is unhelpful when the problem is that the
server output we're getting doesn't match what was expected. So we
should report the query output too; and just for good measure, let's
print the query we used and the output we expected.
Back-patch to 9.5 where poll_query_until was introduced.
Tom Lane [Tue, 16 Oct 2018 16:01:18 +0000 (12:01 -0400)]
Improve stability of recently-added regression test case.
Commit b5febc1d1 added a contrib/btree_gist test case that has been
observed to fail in the buildfarm as a result of background auto-analyze
updating stats and changing the selected plan. Forestall that by
forcibly analyzing in foreground, instead. The new plan choice is
just as good for our purposes, since we really only care that an
index-only plan does not get selected.
localtime.c's "struct state" is a rather large object, ~23KB. We were
statically allocating one for gmtsub() to use to represent the GMT
timezone, even though that function is not at all heavily used and is
never reached in most backends. Let's malloc it on-demand, instead.
This does pose the question of how to handle a malloc failure, but
there's already a well-defined error report convention here, ie
set errno and return NULL.
We have but one caller of pg_gmtime in HEAD, and two in back branches,
neither of which were troubling to check for error. Make them do so.
The possible errors are sufficiently unlikely (out-of-range timestamp,
and now malloc failure) that I think elog() is adequate.
Back-patch to all supported branches to keep our copies of the IANA
timezone code in sync. This particular change is in a stanza that
already differs from upstream, so it's a wash for maintenance purposes
--- but only as long as we keep the branches the same.
Andres Freund [Mon, 15 Oct 2018 22:24:33 +0000 (15:24 -0700)]
Move TupleTableSlots boolean member into one flag variable.
There's several reasons for this change:
1) It reduces the total size of TupleTableSlot / reduces alignment
padding, making the commonly accessed members fit into a single
cacheline (but we currently do not force proper alignment, so
that's not yet guaranteed to be helpful)
2) Combining the booleans into a flag allows to combine read/writes
from memory.
3) With the upcoming slot abstraction changes, it allows to have core
and extended flags, in a memory efficient way.
Author: Ashutosh Bapat and Andres Freund
Discussion: https://postgr.es/m/20180220224318.gw4oe5jadhpmcdnm@alap3.anarazel.de
Andres Freund [Sun, 14 Oct 2018 22:18:16 +0000 (15:18 -0700)]
Move generic slot support functions from heaptuple.c into execTuples.c.
heaptuple.c was never a particular good fit for slot_getattr(),
slot_getsomeattrs() and slot_getmissingattrs(), but in upcoming
changes slots will be made more abstract (allowing slots that contain
different types of tuples), making it clearly the wrong place.
Note that slot_deform_tuple() remains in it's current place, as it
clearly deals with a HeapTuple. getmissingattrs() also remains, but
it's less clear that that's correct - but execTuples.c wouldn't be the
right place.
Thomas Munro [Mon, 15 Oct 2018 22:04:41 +0000 (11:04 +1300)]
Move the replication lag tracker into heap memory.
Andres Freund complained about the 128KB of .bss occupied by LagTracker.
It's only needed in the walsender process, so allocate it in heap
memory there.
Author: Thomas Munro
Discussion: https://postgr.es/m/20181015200754.7y7zfuzsoux2c4ya%40alap3.anarazel.de
projects / postgresql / log