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
Tomas Vondra [Mon, 28 Jan 2019 23:00:47 +0000 (00:00 +0100)]
Separate per-batch and per-tuple memory contexts in COPY
In batching mode, COPY was using the same (per-tuple) memory context for
allocations with longer lifetime. This was confusing but harmless, until
commit 31f3817402 added COPY FROM ... WHERE feature, introducing a risk
of memory leak.
The "per-tuple" memory context was reset only when starting new batch,
but as the rows may be filtered out by the WHERE clauses, that may not
happen at all. The WHERE clause however has to be evaluated for all
rows, before filtering them out.
This commit separates the per-tuple and per-batch contexts, removing the
ambiguity. Expressions (both defaults and WHERE clause) are evaluated
in the per-tuple context, while tuples are formed in the batch context.
This allows resetting the contexts at appropriate times.
The main complexity is related to partitioning, in which case we need to
reset the batch context after forming the tuple (which happens before
routing to leaf partition). Instead of switching between two contexts
as before, we simply copy the last tuple aside, reset the context and
then copy the tuple back. The performance impact is negligible, and
juggling with two contexts is not free either.
Tom Lane [Mon, 28 Jan 2019 22:54:10 +0000 (17:54 -0500)]
In the planner, replace an empty FROM clause with a dummy RTE.
The fact that "SELECT expression" has no base relations has long been a
thorn in the side of the planner. It makes it hard to flatten a sub-query
that looks like that, or is a trivial VALUES() item, because the planner
generally uses relid sets to identify sub-relations, and such a sub-query
would have an empty relid set if we flattened it. prepjointree.c contains
some baroque logic that works around this in certain special cases --- but
there is a much better answer. We can replace an empty FROM clause with a
dummy RTE that acts like a table of one row and no columns, and then there
are no such corner cases to worry about. Instead we need some logic to
get rid of useless dummy RTEs, but that's simpler and covers more cases
than what was there before.
For really trivial cases, where the query is just "SELECT expression" and
nothing else, there's a hazard that adding the extra RTE makes for a
noticeable slowdown; even though it's not much processing, there's not
that much for the planner to do overall. However testing says that the
penalty is very small, close to the noise level. In more complex queries,
this is able to find optimizations that we could not find before.
The new RTE type is called RTE_RESULT, since the "scan" plan type it
gives rise to is a Result node (the same plan we produced for a "SELECT
expression" query before). To avoid confusion, rename the old ResultPath
path type to GroupResultPath, reflecting that it's only used in degenerate
grouping cases where we know the query produces just one grouped row.
(It wouldn't work to unify the two cases, because there are different
rules about where the associated quals live during query_planner.)
Note: although this touches readfuncs.c, I don't think a catversion
bump is required, because the added case can't occur in stored rules,
only plans.
Patch by me, reviewed by David Rowley and Mark Dilger
Andres Freund [Mon, 28 Jan 2019 21:51:12 +0000 (13:51 -0800)]
Install JIT related headers.
There's no reason not to install these, and jit.h can be useful for
users of e.g. planner hooks.
Author: Donald Dong Reviewed-By: Andres Freund
Discussion: https://postgr.es/m/296D405F-7F95-49F1-B565-389D6AA78505@csumb.edu
Backpatch: 11-, where JIT compilation was introduced
Andres Freund [Mon, 28 Jan 2019 21:15:11 +0000 (13:15 -0800)]
Move page initialization from RelationAddExtraBlocks() to use.
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.
Author: Andres Freund Reviewed-By: Tom Lane
Discussion: https://postgr.es/m/20181219083945.6khtgm36mivonhva@alap3.anarazel.de
Amit Kapila [Mon, 28 Jan 2019 02:44:06 +0000 (08:14 +0530)]
Avoid creation of the free space map for small heap relations.
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 with design inputs and some code contribution by Amit Kapila Reviewed-by: Amit Kapila Tested-by: Mithun C Y
Discussion: https://www.postgresql.org/message-id/CAJVSVGWvB13PzpbLEecFuGFc5V2fsO736BsdTakPiPAcdMM5tQ@mail.gmail.com
Amit Kapila [Mon, 28 Jan 2019 02:21:02 +0000 (07:51 +0530)]
In bootstrap mode, don't allow the creation of files if they don't already
exist.
In commit's b9d01fe288 and 3908473c80, we have added some code where we
allowed the creation of files during mdopen even if they didn't exist
during the bootstrap mode. The later commit obviates the need for same.
This was harmless code till now but with an upcoming feature where we don't
allow to create FSM for small tables, this will needlessly create FSM
files.
Author: John Naylor Reviewed-by: Amit Kapila
Discussion: https://www.postgresql.org/message-id/CAJVSVGWvB13PzpbLEecFuGFc5V2fsO736BsdTakPiPAcdMM5tQ@mail.gmail.com
https://www.postgresql.org/message-id/CAA4eK1KsET6sotf+rzOTQfb83pzVEzVhbQi1nxGFYVstVWXUGw@mail.gmail.com
Michael Paquier [Sun, 27 Jan 2019 13:25:48 +0000 (22:25 +0900)]
Add TAP tests for vacuumdb with column lists
vacuumdb generates by itself SQL queries to run ANALYZE or VACUUM on the
backend, but we never actually checked for query patterns with column
lists defined.
Author: Michael Paquier Reviewed-by: Nathan Bossart
Discussion: https://postgr.es/m/FFE5373C-E26A-495B-B5C8-911EC4A41C5E@amazon.com
Tom Lane [Sun, 27 Jan 2019 03:12:48 +0000 (22:12 -0500)]
Allow for yet another crash symptom in 013_crash_restart.pl.
Given the right timing, psql could emit "connection to server was lost"
rather than one of the other messages that this test script checked for.
It looks like commit 4247db625 may have made this more likely, but
I don't really believe it was impossible before then. Rather than
stress about it, just add that spelling as one of the crash-successfully-
detected cases.
Andres Freund [Sat, 26 Jan 2019 22:17:52 +0000 (14:17 -0800)]
Change function call information to be variable length.
Before this change FunctionCallInfoData, the struct arguments etc for
V1 function calls are stored in, always had space for
FUNC_MAX_ARGS/100 arguments, storing datums and their nullness in two
arrays. For nearly every function call 100 arguments is far more than
needed, therefore wasting memory. Arg and argnull being two separate
arrays also guarantees that to access a single argument, two
cachelines have to be touched.
Change the layout so there's a single variable-length array with pairs
of value / isnull. That drastically reduces memory consumption for
most function calls (on x86-64 a two argument function now uses
64bytes, previously 936 bytes), and makes it very likely that argument
value and its nullness are on the same cacheline.
Arguments are stored in a new NullableDatum struct, which, due to
padding, needs more memory per argument than before. But as usually
far fewer arguments are stored, and individual arguments are cheaper
to access, that's still a clear win. It's likely that there's other
places where conversion to NullableDatum arrays would make sense,
e.g. TupleTableSlots, but that's for another commit.
Because the function call information is now variable-length
allocations have to take the number of arguments into account. For
heap allocations that can be done with SizeForFunctionCallInfoData(),
for on-stack allocations there's a new LOCAL_FCINFO(name, nargs) macro
that helps to allocate an appropriately sized and aligned variable.
Some places with stack allocation function call information don't know
the number of arguments at compile time, and currently variably sized
stack allocations aren't allowed in postgres. Therefore allow for
FUNC_MAX_ARGS space in these cases. They're not that common, so for
now that seems acceptable.
Because of the need to allocate FunctionCallInfo of the appropriate
size, older extensions may need to update their code. To avoid subtle
breakages, the FunctionCallInfoData struct has been renamed to
FunctionCallInfoBaseData. Most code only references FunctionCallInfo,
so that shouldn't cause much collateral damage.
This change is also a prerequisite for more efficient expression JIT
compilation (by allocating the function call information on the stack,
allowing LLVM to optimize it away); previously the size of the call
information caused problems inside LLVM's optimizer.
Author: Andres Freund Reviewed-By: Tom Lane
Discussion: https://postgr.es/m/20180605172952.x34m5uz6ju6enaem@alap3.anarazel.de
Tom Lane [Sat, 26 Jan 2019 19:15:42 +0000 (14:15 -0500)]
Fix psql's "\g target" meta-command to work with COPY TO STDOUT.
Previously, \g would successfully execute the COPY command, but
the target specification if any was ignored, so that the data was
always dumped to the regular query output target. This seems like
a clear bug, so let's not just fix it but back-patch it.
While at it, adjust the documentation for \copy to recommend
"COPY ... TO STDOUT \g foo" as a plausible alternative.
Back-patch to 9.5. The problem exists much further back, but the
code associated with \g was refactored enough in 9.5 that we'd
need a significantly different patch for 9.4, and it doesn't
seem worth the trouble.
Tom Lane [Sat, 26 Jan 2019 02:14:31 +0000 (21:14 -0500)]
Allow UNLISTEN in hot-standby mode.
Since LISTEN is (still) disallowed, UNLISTEN must be a no-op in a
hot-standby session, and so there's no harm in allowing it. This
change allows client code to not worry about whether it's connected
to a primary or standby server when performing session-state-reset
type activities. (Note that DISCARD ALL, which includes UNLISTEN,
was already allowed, making it inconsistent to reject UNLISTEN.)
Per discussion, back-patch to all supported versions.
Michael Paquier [Sat, 26 Jan 2019 01:45:23 +0000 (10:45 +0900)]
Simplify restriction handling of two-phase commit for temporary objects
There were two flags used to track the access to temporary tables and
to the temporary namespace of a session which are used to restrict
PREPARE TRANSACTION, however the first control flag is a concept
included in the second. This removes the flag for temporary table
tracking, keeping around only the one at namespace level.
Author: Michael Paquier Reviewed-by: Álvaro Herrera
Discussion: https://postgr.es/m/20190118053126.GH1883@paquier.xyz
Tom Lane [Fri, 25 Jan 2019 22:09:45 +0000 (17:09 -0500)]
Split QTW_EXAMINE_RTES flag into QTW_EXAMINE_RTES_BEFORE/_AFTER.
This change allows callers of query_tree_walker() to choose whether
to visit an RTE before or after visiting the contents of the RTE
(i.e., prefix or postfix tree order). All existing users of
QTW_EXAMINE_RTES want the QTW_EXAMINE_RTES_BEFORE behavior, but
an upcoming patch will want QTW_EXAMINE_RTES_AFTER, and it seems
like a potentially useful change on its own.
Andreas Karlsson (extracted from CTE inlining patch)
Tom Lane [Fri, 25 Jan 2019 16:44:26 +0000 (11:44 -0500)]
Teach nulltestsel() that system columns are never NULL.
While it's perhaps unlikely that users would write an explicit test
like "ctid IS NULL", this function is also used in range estimation,
and an incorrect answer can throw off the results for tight ranges.
Anyway it's not much code so we might as well do it.
Use single-byte Boyer-Moore-Horspool search even with multibyte encodings.
The old implementation first converted the input strings to arrays of
wchars, and performed the conversion on those. However, the conversion is
expensive, and for a large input string, consumes a lot of memory.
Allocating the large arrays also meant that these functions could not be
used on strings larger 1 GB / pg_encoding_max_length() (256 MB for UTF-8).
Avoid the conversion, and instead use the single-byte algorithm even with
multibyte encodings. That can get fooled, if there is a matching byte
sequence in the middle of a multi-byte character, so to eliminate false
positives like that, we verify any matches by walking the string character
by character with pg_mblen(). Also, if the caller needs the position of
the match, as a character-offset, we also need to walk the string to count
the characters.
Performance testing shows that walking the whole string with pg_mblen() is
somewhat slower than converting the whole string to wchars. It's still
often a win, though, because we don't need to do it if there is no match,
and even when there is, we only need to walk up to the point where the
match is, not the whole string. Even in the worst case, there would be
room for optimization: Much of the CPU time in the current loop with
pg_mblen() is function call overhead, and could be improved by inlining
pg_mblen() and/or the encoding-specific mblen() functions. But I didn't
attempt to do that as part of this patch.
Most of the callers of text_position_setup/next functions were actually
not interested in the position of the match, counted in characters. To
cater for them, refactor the text_position_next() interface into two
parts: searching for the next match (text_position_next()), and returning
the current match's position as a pointer (text_position_get_match_ptr())
or as a character offset (text_position_get_match_pos()). Getting the
pointer to the match is a more convenient API for many callers, and with
UTF-8, it allows skipping the character-walking step altogether, because
UTF-8 can't have false matches even when treated like raw byte strings.
Reviewed-by: John Naylor
Discussion: https://www.postgresql.org/message-id/3173d989-bc1c-fc8a-3b69-f24246f73876%40iki.fi
Fix comments that claimed that mblen() only looks at first byte.
GB18030's mblen() function looks at the first and the second byte of the
multibyte character, to determine its length. copy.c had made the
assumption that mblen() only looks at the first byte, but it turns out to
work out fine, because of the way the GB18030 encoding works. COPY will
see a 4-byte encoded character as two 2-byte encoded characters, which is
enough for COPY's purposes. It cannot mix those up with delimiter or
escaping characters, because only single-byte ASCII characters are
supported as delimiters or escape characters.
Peter Eisentraut [Fri, 25 Jan 2019 10:27:59 +0000 (11:27 +0100)]
Allow generalized expression syntax for partition bounds
Previously, only literals were allowed. This change allows general
expressions, including functions calls, which are evaluated at the
time the DDL command is executed.
Besides offering some more functionality, it simplifies the parser
structures and removes some inconsistencies in how the literals were
handled.
Author: Kyotaro Horiguchi, Tom Lane, Amit Langote Reviewed-by: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Discussion: https://www.postgresql.org/message-id/flat/9f88b5e0-6da2-5227-20d0-0d7012beaa1c@lab.ntt.co.jp/
Tom Lane [Thu, 24 Jan 2019 22:02:09 +0000 (17:02 -0500)]
Remove _configthreadlocale() calls in ecpg test suite.
This essentially reverts commits a772624b1 and 04fbe0e45, which
added "_configthreadlocale(_ENABLE_PER_THREAD_LOCALE)" calls to the
thread-related ecpg test programs. That was nothing but a hack,
because we shouldn't expect that ecpg-using applications have
done that for us; and now that we've inserted such calls into
ecpglib, the tests should still pass without it.
(If they don't, it would be good to know that.)
HEAD only; there seems no big need to change this in the
back branches.
Tom Lane [Thu, 24 Jan 2019 21:46:55 +0000 (16:46 -0500)]
Remove infinite-loop hazards in ecpg test suite.
A report from Andrew Dunstan showed that an ecpglib breakage that
causes repeated query failures could lead to infinite loops in some
ecpg test scripts, because they contain "while(1)" loops with no
exit condition other than successful test completion. That might
be all right for manual testing, but it seems entirely unacceptable
for automated test environments such as our buildfarm. We don't
want buildfarm owners to have to intervene manually when a test
goes wrong.
To fix, just change all those while(1) loops to exit after at most
100 iterations (which is more than any of them expect to iterate).
This seems sufficient since we'd see discrepancies in the test output
if any loop executed the wrong number of times.
I tested this by dint of intentionally breaking ecpg_do_prologue
to always fail, and verifying that the tests still got to completion.
Back-patch to all supported branches, since the whole point of this
exercise is to protect the buildfarm against future mistakes.
Peter Eisentraut [Thu, 24 Jan 2019 21:23:12 +0000 (22:23 +0100)]
PL/pgSQL: Add statement ID to statement structures
This can be used by a profiler as the index for an array of
per-statement metrics.
Author: Pavel Stehule <pavel.stehule@gmail.com> Reviewed-by: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Discussion: https://www.postgresql.org/message-id/flat/CAFj8pRDRCjN6rpM9ZccU7Ta_afsNX7mg9=n34F+r445Nt9v2tA@mail.gmail.com/
Alvaro Herrera [Thu, 24 Jan 2019 17:09:56 +0000 (14:09 -0300)]
Fix droppability of constraints upon partition detach
We were failing to set conislocal correctly for constraints in
partitions after partition detach, leading to those constraints becoming
undroppable. Fix by setting the flag correctly. Existing databases
might contain constraints with the conislocal wrongly set to false, for
partitions that were detached; this situation should be fixable by
applying an UPDATE on pg_constraint to set conislocal true. This
problem should otherwise be innocuous and should disappear across a
dump/restore or pg_upgrade.
Secondarily, when constraint drop was attempted in a partitioned table,
ATExecDropConstraint would try to recurse to partitions after doing
performDeletion() of the constraint in the partitioned table itself; but
since the constraint in the partitions are dropped by the initial call
of performDeletion() (because of following dependencies), the recursion
step would fail since it would not find the constraint, causing the
whole operation to fail. Fix by preventing recursion.
Reported-by: Amit Langote Diagnosed-by: Amit Langote
Author: Amit Langote, Álvaro Herrera
Discussion: https://postgr.es/m/f2b8ead5-4131-d5a8-8016-2ea0a31250af@lab.ntt.co.jp
projects / postgresql / log