Alvaro Herrera [Thu, 7 Mar 2019 14:17:09 +0000 (11:17 -0300)]
Fix the BY {REF,VALUE} clause of XMLEXISTS/XMLTABLE
This clause is used to indicate the passing mode of a XML document, but
we were doing it wrong: we accepted BY REF and ignored it, and rejected
BY VALUE as a syntax error. The reality, however, is that documents are
always passed BY VALUE, so rejecting that clause was silly. Change
things so that we accept BY VALUE.
BY REF continues to be accepted, and continues to be ignored.
Author: Chapman Flack Reviewed-by: Pavel Stehule
Discussion: https://postgr.es/m/5C297BB7.9070509@anastigmatix.net
Alvaro Herrera [Thu, 7 Mar 2019 12:26:14 +0000 (09:26 -0300)]
pg_dump: allow multiple rows per insert
This is useful to speed up loading data in a different database engine.
Authors: Surafel Temesgen and David Rowley. Lightly edited by Álvaro. Reviewed-by: Fabien Coelho
Discussion: https://postgr.es/m/CALAY4q9kumSdnRBzvRJvSRf2+BH20YmSvzqOkvwpEmodD-xv6g@mail.gmail.com
Thomas Munro [Thu, 7 Mar 2019 02:43:37 +0000 (15:43 +1300)]
Drop the vestigial "smgr" type.
Before commit 3fa2bb31 this type appeared in the catalogs to
select which of several block storage mechanisms each relation
used.
New features under development propose to revive the concept of
different block storage managers for new kinds of data accessed
via bufmgr.c, but don't need to put references to them in the
catalogs. So, avoid useless maintenance work on this type by
dropping it. Update some regression tests that were referencing
it where any type would do.
Andres Freund [Wed, 6 Mar 2019 23:43:33 +0000 (15:43 -0800)]
Don't reuse slots between root and partition in ON CONFLICT ... UPDATE.
Until now the the slot to store the conflicting tuple, and the result
of the ON CONFLICT SET, where reused between partitions. That
necessitated changing slots descriptor when switching partitions.
Besides the overhead of switching descriptors on a slot (which
requires memory allocations and prevents JITing), that's importantly
also problematic for tableam. There individual partitions might belong
to different tableams, needing different kinds of slots.
In passing also fix ExecOnConflictUpdate to clear the existing slot at
exit. Otherwise that slot could continue to hold a pin till the query
ends, which could be far too long if the input data set is large, and
there's no further conflicts. While previously also problematic, it's
now more important as there will be more such slots when partitioned.
Author: Andres Freund Reviewed-By: Robert Haas, David Rowley
Discussion: https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de
Andrew Dunstan [Wed, 6 Mar 2019 20:27:19 +0000 (15:27 -0500)]
Don't log incomplete startup packet if it's empty
This will stop logging cases where, for example, a monitor opens a
connection and immediately closes it. If the packet contains any data an
incomplete packet will still be logged.
Andres Freund [Wed, 6 Mar 2019 19:55:28 +0000 (11:55 -0800)]
Fix copy/out/readfuncs for accessMethod addition in 8586bf7ed8.
This includes a catversion bump, as IntoClause is theoretically
speaking part of storable rules. In practice I don't think that can
happen, but there's no reason to be stingy here.
Andres Freund [Wed, 6 Mar 2019 17:54:38 +0000 (09:54 -0800)]
tableam: Add pg_dump support.
This adds pg_dump support for table AMs in a similar manner to how
tablespaces are handled. That is, instead of specifying the AM for
every CREATE TABLE etc, emit SET default_table_access_method
statements. That makes it easier to change the AM for all/most tables
in a dump, and allows restore to succeed even if some AM is not
available.
This increases the dump archive version, as a tables/matview's AM
needs to be tracked therein.
Author: Dimitri Dolgov, Andres Freund
Discussion:
https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de
https://postgr.es/m/20190304234700.w5tmhducs5wxgzls@alap3.anarazel.de
Andres Freund [Wed, 6 Mar 2019 17:54:38 +0000 (09:54 -0800)]
tableam: introduce table AM infrastructure.
This introduces the concept of table access methods, i.e. CREATE
ACCESS METHOD ... TYPE TABLE and
CREATE TABLE ... USING (storage-engine).
No table access functionality is delegated to table AMs as of this
commit, that'll be done in following commits.
Subsequent commits will incrementally abstract table access
functionality to be routed through table access methods. That change
is too large to be reviewed & committed at once, so it'll be done
incrementally.
Docs will be updated at the end, as adding them incrementally would
likely make them less coherent, and definitely is a lot more work,
without a lot of benefit.
Table access methods are specified similar to index access methods,
i.e. pg_am.amhandler returns, as INTERNAL, a pointer to a struct with
callbacks. In contrast to index AMs that struct needs to live as long
as a backend, typically that's achieved by just returning a pointer to
a constant struct.
Psql's \d+ now displays a table's access method. That can be disabled
with HIDE_TABLEAM=true, which is mainly useful so regression tests can
be run against different AMs. It's quite possible that this behaviour
still needs to be fine tuned.
For now it's not allowed to set a table AM for a partitioned table, as
we've not resolved how partitions would inherit that. Disallowing
allows us to introduce, if we decide that's the way forward, such a
behaviour without a compatibility break.
Catversion bumped, to add the heap table AM and references to it.
Author: Haribabu Kommi, Andres Freund, Alvaro Herrera, Dimitri Golgov and others
Discussion:
https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de
https://postgr.es/m/20160812231527.GA690404@alvherre.pgsql
https://postgr.es/m/20190107235616.6lur25ph22u5u5av@alap3.anarazel.de
https://postgr.es/m/20190304234700.w5tmhducs5wxgzls@alap3.anarazel.de
Andres Freund [Wed, 6 Mar 2019 17:54:38 +0000 (09:54 -0800)]
Fix bug in clearing of virtual tuple slot.
I broke/typoed this in 4da597edf1bae0c. Astonishingly this mostly
doesn't cause breakage, except when trying to change the tuple
descriptor of a slot (because TTS_FLAG_FIXED is assumed to be set).
Andrew Dunstan [Wed, 6 Mar 2019 14:10:12 +0000 (09:10 -0500)]
Increase the default vacuum_cost_limit from 200 to 2000
The original 200 default value was set back in f425b605f4e when the cost
delay settings were first added. Hardware has improved quite a bit since
then and we've also made improvements such as sorting buffers during
checkpoints (9cd00c457e6) which should result in less random writes.
This low default value was reportedly causing problems with badly
configured servers and in the absence of a native method to remove
excessive bloat from tables without incurring an AccessExclusiveLock, this
often made cleaning up the damage caused by badly configured auto-vacuums
difficult.
It seems more likely that someone will notice that auto-vacuum is running
too quickly than too slowly, so let's go all out and multiple the default
value for the setting by 10. With the default vacuum_cost_page_dirty and
autovacuum_vacuum_cost_delay (assuming a page size of 8192 bytes), this
allows autovacuum a theoretical maximum dirty write rate of around 39MB/s
instead of just 3.9MB/s.
Andrew Dunstan [Tue, 5 Mar 2019 15:46:21 +0000 (10:46 -0500)]
Fix pgbench TAP test failure with funky file names (redux)
This test fails if the containing directory contains a funny character
such as a space or some perl metacharacter. To avoid that, we check for
files names using readdir and a regex, rather than using a glob pattern.
Scan GiST indexes in physical order during VACUUM.
Scanning an index in physical order is faster than walking it in logical
order, because sequential I/O is faster than random I/O. The idea and code
structure is borrowed from B-tree vacuum code.
Patch by Andrey Borodin, with changes by me. Based on early work by
Konstantin Kuznetsov, although the patch has been rewritten multiple times
since his original version.
Peter Geoghegan [Tue, 5 Mar 2019 01:57:36 +0000 (17:57 -0800)]
Note case where nbtree VACUUM finishes splits.
The nbtree README claims that VACUUM can never finish interrupted page
splits by design. That isn't entirely accurate, though. Note an
exception to the general rule.
Andrew Dunstan [Mon, 4 Mar 2019 22:11:18 +0000 (17:11 -0500)]
Disable dump_connstr test on Msys2
For some reason the dump test with names with high bits set fails on
Msys2 (although not Msys1). Disable the tests for now, so that other
tests can run.
Andrew Dunstan [Mon, 4 Mar 2019 20:50:23 +0000 (15:50 -0500)]
Allow recovery tests to run on Windows as an admin user
This is the only test that fails when run as an admin user. The reason
is that when Postgres is started via pg_ctl its admin privileges are
lowered. However, this test called 'postgres -D datadir' directly,
resulting in a failure. Replace that by calling pg_ctl and then checking
the result for the expected failure, and the logfile for the expected
error message.
Andrew Dunstan [Mon, 4 Mar 2019 18:53:06 +0000 (13:53 -0500)]
Reorder configure tests for accept() in Windows
Currently only frogmouth in the buildfarm uses the 32bit params, and
it's not able to build past release 10, so put those last, saving
substantial configure time on more modern systems. Even if we get a
modern 32 bit Windows system at some stage we should probably prefer the
64 bit interface here these days.
Peter Eisentraut [Mon, 25 Feb 2019 08:24:15 +0000 (09:24 +0100)]
Remove volatile from latch API
This was no longer useful since the latch functions use memory
barriers already, which are also compiler barriers, and volatile does
not help with cross-process access.
Michael Paquier [Mon, 4 Mar 2019 00:49:06 +0000 (09:49 +0900)]
Fix error handling of readdir() port implementation on first file lookup
The implementation of readdir() in src/port/ which gets used by MSVC has
been added in 399a36a, and since the beginning it considers all errors
on the first file lookup as ENOENT, setting errno accordingly and
letting the routine caller think that the directory is empty. While
this is normally enough for the case of the backend, this can confuse
callers of this routine on Windows as all errors would map to the same
behavior. So, for example, even permission errors would be thought as
having an empty directory, while there could be contents in it.
This commit changes the error handling so as readdir() gets a behavior
similar to native implementations: force errno=0 when seeing
ERROR_FILE_NOT_FOUND as error and consider other errors as plain
failures.
While looking at the patch, I noticed that MinGW does not enforce
errno=0 when looking at the first file, but it gets enforced on the next
file lookups. A comment related to that was incorrect in the code.
Reported-by: Yuri Kurenkov Diagnosed-by: Yuri Kurenkov, Grigory Smolkin
Author: Konstantin Knizhnik Reviewed-by: Andrew Dunstan, Michael Paquier
Discussion: https://postgr.es/m/2cad7829-8d66-e39c-b937-ac825db5203d@postgrespro.ru
Backpatch-through: 9.4
Andrew Dunstan [Sun, 3 Mar 2019 23:19:44 +0000 (18:19 -0500)]
Don't do pg_ctl logrotate test on Windows
The test crashes and burns quite badly, for some reason, but even if it
didn't it wouldn't work, since Windows doesn't let you rename a file
held by a running process.
Tom Lane [Sun, 3 Mar 2019 21:57:14 +0000 (16:57 -0500)]
Improve performance of index-only scans with many index columns.
StoreIndexTuple was a loop over index_getattr, which is O(N^2)
if the index columns are variable-width, and the performance
impact is already quite visible at ten columns. The obvious
move is to replace that with a call to index_deform_tuple ...
but that's *also* a loop over index_getattr. Improve it to
be essentially a clone of heap_deform_tuple.
(There are a few other places that loop over all index columns
with index_getattr, and perhaps should be changed likewise,
but most of them don't seem performance-critical. Anyway, the
rest would mostly only be interested in the index key columns,
which there aren't likely to be so many of. Wide index tuples
are a new thing with INCLUDE.)
Andrew Dunstan [Sun, 3 Mar 2019 16:48:12 +0000 (11:48 -0500)]
Avoid accidental wildcard expansion in msys shell
Commit f092de05 added a test for pg_dumpall --exclude-database including
the wildcard pattern '*dump*' which matches some files in the source
directory. The test library on msys uses the shell which expands this
and thus the program gets incorrect arguments. This doesn't happen if
the pattern doesn't match any files, so here the pattern is set to
'*dump_test*' which is such a pattern.
Dean Rasheed [Sun, 3 Mar 2019 10:51:13 +0000 (10:51 +0000)]
Further fixing for multi-row VALUES lists for updatable views.
Previously, rewriteTargetListIU() generated a list of attribute
numbers from the targetlist, which were passed to rewriteValuesRTE(),
which expected them to contain the same number of entries as there are
columns in the VALUES RTE, and to be in the same order. That was fine
when the target relation was a table, but for an updatable view it
could be broken in at least three different ways ---
rewriteTargetListIU() could insert additional targetlist entries for
view columns with defaults, the view columns could be in a different
order from the columns of the underlying base relation, and targetlist
entries could be merged together when assigning to elements of an
array or composite type. As a result, when recursing to the base
relation, the list of attribute numbers generated from the rewritten
targetlist could no longer be relied upon to match the columns of the
VALUES RTE. We got away with that prior to 41531e42d3 because it used
to always be the case that rewriteValuesRTE() did nothing for the
underlying base relation, since all DEFAULTS had already been replaced
when it was initially invoked for the view, but that was incorrect
because it failed to apply defaults from the base relation.
Fix this by examining the targetlist entries more carefully and
picking out just those that are simple Vars referencing the VALUES
RTE. That's sufficient for the purposes of rewriteValuesRTE(), which
is only responsible for dealing with DEFAULT items in the VALUES
RTE. Any DEFAULT item in the VALUES RTE that doesn't have a matching
simple-Var-assignment in the targetlist is an error which we complain
about, but in theory that ought to be impossible.
Additionally, move this code into rewriteValuesRTE() to give a clearer
separation of concerns between the 2 functions. There is no need for
rewriteTargetListIU() to know about the details of the VALUES RTE.
While at it, fix the comment for rewriteValuesRTE() which claimed that
it doesn't support array element and field assignments --- that hasn't
been true since a3c7a993d5 (9.6 and later).
Back-patch to all supported versions, with minor differences for the
pre-9.6 branches, which don't support array element and field
assignments to the same column in multi-row VALUES lists.
Michael Paquier [Sat, 2 Mar 2019 09:18:59 +0000 (18:18 +0900)]
Consider only relations part of partition trees in partition functions
This changes the partition functions so as tables and indexes which are
not part of partition trees are handled the same way as what is done for
undefined objects and unsupported relkinds: pg_partition_tree() returns
no rows and pg_partition_root() returns a NULL result. Hence,
partitioned tables, partitioned indexes and relations whose flag
pg_class.relispartition is set are considered as valid objects to
process.
Previously, tables and indexes not included in a partition tree were
processed the same way as a partition or a partitioned table, which
caused the functions to return inconsistent results for inherited
tables, especially when inheriting from multiple tables.
Reported-by: Álvaro Herrera
Author: Amit Langote, Michael Paquier Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/20190228193203.GA26151@alvherre.pgsql
Tom Lane [Fri, 1 Mar 2019 22:57:20 +0000 (17:57 -0500)]
Check we don't misoptimize a NOT IN where the subquery returns no rows.
Future-proofing against a common mistake in attempts to optimize NOT IN.
We don't have such an optimization right now, but attempts to do so
are in the works, and some of 'em are buggy. Add a regression test case
covering the point.
Tom Lane [Fri, 1 Mar 2019 22:14:07 +0000 (17:14 -0500)]
Teach optimizer's predtest.c more things about ScalarArrayOpExpr.
In particular, make it possible to prove/refute "x IS NULL" and
"x IS NOT NULL" predicates from a clause involving a ScalarArrayOpExpr
even when we are unable or unwilling to deconstruct the expression
into an AND/OR tree. This avoids a former unexpected degradation of
plan quality when the size of an ARRAY[] expression or array constant
exceeded the arbitrary MAX_SAOP_ARRAY_SIZE limit. For IS-NULL proofs,
we don't really care about the values of the individual array elements;
at most, we care whether there are any, and for some common cases we
needn't even know that.
The main user-visible effect of this is to let the optimizer recognize
applicability of partial indexes with "x IS NOT NULL" predicates to
queries with "x IN (array)" clauses in some cases where it previously
failed to recognize that. The structure of predtest.c is such that a
bunch of related proofs will now also succeed, but they're probably
much less useful in the wild.
Andrew Dunstan [Fri, 1 Mar 2019 19:11:37 +0000 (14:11 -0500)]
Remove tests for pg_dumpall --exclude-database missing argument
It turns out that different getopt implementations spell the error for
missing arguments different ways. This test is of fairly marginal
value, so instead of trying to keep up with the different error
messages just remove the test.
Andres Freund [Fri, 1 Mar 2019 18:37:57 +0000 (10:37 -0800)]
Store tuples for EvalPlanQual in slots, rather than as HeapTuples.
For the upcoming pluggable table access methods it's quite
inconvenient to store tuples as HeapTuples, as that'd require
converting tuples from a their native format into HeapTuples. Instead
use slots to manage epq tuples.
To fit into that scheme, change the foreign data wrapper callback
RefetchForeignRow, to store the tuple in a slot. Insist on using the
caller provided slot, so it conveniently can be stored in the
corresponding EPQ slot. As there is no in core user of
RefetchForeignRow, that change was done blindly, but we plan to test
that soon.
To avoid duplicating that work for row locks, move row locks to just
directly use the EPQ slots - it previously temporarily stored tuples
in LockRowsState.lr_curtuples, but that doesn't seem beneficial, given
we'd possibly end up with a significant number of additional slots.
The behaviour of es_epqTupleSet[rti -1] is now checked by
es_epqTupleSlot[rti -1] != NULL, as that is distinguishable from a
slot containing an empty tuple.
Author: Andres Freund, Haribabu Kommi, Ashutosh Bapat
Discussion: https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de
Andrew Dunstan [Fri, 1 Mar 2019 15:47:44 +0000 (10:47 -0500)]
Add --exclude-database option to pg_dumpall
This option functions similarly to pg_dump's --exclude-table option, but
for database names. The option can be given once, and the argument can
be a pattern including wildcard characters.
Author: Andrew Dunstan. Reviewd-by: Fabien Coelho and Michael Paquier
Discussion: https://postgr.es/m/43a54a47-4aa7-c70e-9ca6-648f436dd6e6@2ndQuadrant.com
Amit Kapila [Fri, 1 Mar 2019 02:08:47 +0000 (07:38 +0530)]
Clear the local map when not used.
After commit b0eaa4c51b, we use a local map of pages to find the required
space for small relations. We do clear this map when we have found a block
with enough free space, when we extend the relation, or on transaction
abort so that it can be used next time. However, we miss to clear it when
we didn't find any pages to try from the map which leads to an assertion
failure when we later tried to use it after relation extension.
In the passing, I have improved some comments in this area.
Reported-by: Tom Lane based on buildfarm results
Author: Amit Kapila Reviewed-by: John Naylor Tested-by: Kuntal Ghosh
Discussion: https://postgr.es/m/32368.1551114120@sss.pgh.pa.us
Michael Paquier [Fri, 1 Mar 2019 00:07:07 +0000 (09:07 +0900)]
Make pg_partition_tree return no rows on unsupported and undefined objects
The function was tweaked so as it returned one row full of NULLs when
working on an unsupported relkind or an undefined object as of cc53123,
and after discussion with Amit and Álvaro it looks more natural to make
it return no rows.
Author: Michael Paquier Reviewed-by: Álvaro Herrera, Amit Langote
Discussion: https://postgr.es/m/20190227184808.GA17357@alvherre.pgsql
Andres Freund [Thu, 28 Feb 2019 21:55:27 +0000 (13:55 -0800)]
Don't force materializing when copying a buffer tuple table slot.
After 5408e233f0667478 it's not necessary to force materializing the
target slot when copying from one buffer slot to another. Previously
that was required because the HeapTupleData portion of the source slot
wasn't guaranteed to stay valid long enough, but now we can simply
copy that part into the destination slot's tupdata.
Joe Conway [Thu, 28 Feb 2019 20:57:40 +0000 (15:57 -0500)]
Make get_controlfile not leak file descriptors
When backend functions were added to expose controldata via SQL,
reading of pg_control was consolidated under src/common so that
both frontend and backend could share the same code. That move
from frontend-only to shared frontend-backend failed to recognize
the risk (and coding standards violation) of using a bare open().
In particular, it risked leaking file descriptors if transient
errors occurred while reading the file. Fix that by using
OpenTransientFile() instead in the backend case, which is
purpose-built for this type of usage.
Since there have been no complaints from the field, and an intermittent
failure low risk, no backpatch. Hard failure would of course be bad, but
in that case these functions are probably the least of your worries.
Author: Joe Conway Reviewed-By: Michael Paquier
Reported by: Michael Paquier
Discussion: https://postgr.es/m/20190227074728.GA15710@paquier.xyz
Andres Freund [Thu, 28 Feb 2019 20:27:58 +0000 (12:27 -0800)]
Allow buffer tuple table slots to materialize after ExecStoreVirtualTuple().
While not common, it can be useful to store a virtual tuple into a
buffer tuple table slot, and then materialize that slot. So far we've
asserted out, which surprisingly wasn't a problem for anything in
core. But that seems fragile, and it also breaks redis_fdw after ff11e7f4b9.
Thus, allow materializing a virtual tuple stored in a buffer tuple
table slot.
Author: Andres Freund
Discussion:
https://postgr.es/m/20190227181621.xholonj7ff7ohxsg@alap3.anarazel.de
https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de
Alvaro Herrera [Thu, 28 Feb 2019 20:16:08 +0000 (17:16 -0300)]
pg_dump: Fix ArchiveEntry handling of some empty values
Commit f831d4acc changed what pg_dump emits for some empty fields: they
were output as empty strings before, NULL pointer afterwards. That
makes old pg_restore unable to work (crash) with such files, which is
unacceptable. Return to the original representation by explicitly
setting those struct members to "" where needed; remove some no longer
needed checks for NULL input.
We can declutter the code a little by returning to NULLs when we next
update the archive version, so add a note to remind us later.
Tom Lane [Thu, 28 Feb 2019 19:25:01 +0000 (14:25 -0500)]
Standardize some more loops that chase down parallel lists.
We have forboth() and forthree() macros that simplify iterating
through several parallel lists, but not everyplace that could
reasonably use those was doing so. Also invent forfour() and
forfive() macros to do the same for four or five parallel lists,
and use those where applicable.
The immediate motivation for doing this is to reduce the number
of ad-hoc lnext() calls, to reduce the footprint of a WIP patch.
However, it seems like good cleanup and error-proofing anyway;
the places that were combining forthree() with a manually iterated
loop seem particularly illegible and bug-prone.
There was some speculation about restructuring related parsetree
representations to reduce the need for parallel list chasing of
this sort. Perhaps that's a win, or perhaps not, but in any case
it would be considerably more invasive than this patch; and it's
not particularly related to my immediate goal of improving the
List infrastructure. So I'll leave that question for another day.
Peter Eisentraut [Thu, 28 Feb 2019 14:29:00 +0000 (15:29 +0100)]
Clean up some variable names in ri_triggers.c
There was a mix of old_slot/oldslot, new_slot/newslot. Since we've
changed everything from row to slot, we might as well take this
opportunity to clean this up.
Also update some more comments for the slot change.
Peter Eisentraut [Thu, 28 Feb 2019 09:54:44 +0000 (10:54 +0100)]
Reduce comments
Reduce the vertical space used by comments in ri_triggers.c, making
the file longer and more tedious to read than it needs to be. Update
some comments to use a more common style.
Peter Eisentraut [Thu, 28 Feb 2019 09:54:44 +0000 (10:54 +0100)]
Remove unnecessary unused MATCH PARTIAL code
ri_triggers.c spends a lot of space catering to a not-yet-implemented
MATCH PARTIAL option. An actual implementation would probably not use
the existing code structure anyway, so let's just simplify this for
now.
First, have ri_FetchConstraintInfo() check that riinfo->confmatchtype
is valid. Then we don't have to repeat that everywhere.
In the various referential action functions, we don't need to pay
attention to the match type at all right now, so remove all that code.
A future MATCH PARTIAL implementation would probably have some
conditions added to the present code, but it won't need an entirely
separate switch branch in each case.
In RI_FKey_fk_upd_check_required(), reorganize the code to make it
much simpler.
Michael Paquier [Thu, 28 Feb 2019 00:40:28 +0000 (09:40 +0900)]
Fix SCRAM authentication via SSL when mixing versions of OpenSSL
When using a libpq client linked with OpenSSL 1.0.1 or older to connect
to a backend linked with OpenSSL 1.0.2 or newer, the server would send
SCRAM-SHA-256-PLUS and SCRAM-SHA-256 as valid mechanisms for the SASL
exchange, and the client would choose SCRAM-SHA-256-PLUS even if it does
not support channel binding, leading to a confusing error. In this
case, what the client ought to do is switch to SCRAM-SHA-256 so as the
authentication can move on and succeed.
So for a SCRAM authentication over SSL, here are all the cases present
and how we deal with them using libpq:
1) Server supports channel binding, it sends SCRAM-SHA-256-PLUS and
SCRAM-SHA-256 as allowed mechanisms.
1-1) Client supports channel binding, chooses SCRAM-SHA-256-PLUS.
1-2) Client does not support channel binding, chooses SCRAM-SHA-256.
2) Server does not support channel binding, sends SCRAM-SHA-256 as
allowed mechanism.
2-1) Client supports channel binding, still it has no choice but to
choose SCRAM-SHA-256.
2-2) Client does not support channel binding, it chooses SCRAM-SHA-256.
In all these scenarios the connection should succeed, and the one which
was handled incorrectly prior this commit is 1-2), causing the
connection attempt to fail because client chose SCRAM-SHA-256-PLUS over
SCRAM-SHA-256.
Reported-by: Hugh Ranalli Diagnosed-by: Peter Eisentraut
Author: Michael Paquier Reviewed-by: Peter Eisentraut
Discussion: https://postgr.es/m/CAAhbUMO89SqUk-5mMY+OapgWf-twF2NA5sCucbHEzMfGbvcepA@mail.gmail.com
Backpatch-through: 11
Andres Freund [Wed, 27 Feb 2019 17:14:34 +0000 (09:14 -0800)]
Initialize variable to silence compiler warning.
After ff11e7f4b9ae Tom's compiler warns about accessing a potentially
uninitialized rInfo. That's not actually possible, but it's
understandable the compiler would get this wrong. NULL initialize too.
Reported-By: Tom Lane
Discussion: https://postgr.es/m/11199.1551285318@sss.pgh.pa.us
Set fallback_application_name for a walreceiver to cluster_name
By default, the fallback_application_name for a physical walreceiver
is "walreceiver". This means that multiple standbys cannot be
distinguished easily on a primary, for example in pg_stat_activity or
synchronous_standby_names.
If cluster_name is set, use that for fallback_application_name in the
walreceiver. (If it's not set, it remains "walreceiver".) If someone
set cluster_name to identify their instance, we might as well use that
by default to identify the node remotely as well. It's still possible
to specify another application_name in primary_conninfo explicitly.
Michael Paquier [Wed, 27 Feb 2019 05:14:06 +0000 (14:14 +0900)]
Fix memory leak when inserting tuple at relation creation for CTAS
The leak has been introduced by 763f2ed which has addressed the problem
for transient tables, and forgot CREATE TABLE AS which shares a similar
logic when receiving a new tuple to store into the newly-created
relation.
Author: Jeff Janes
Discussion: https://postgr.es/m/CAMkU=1xZXtz3mziPEPD2Fubbas4G2RWkZm5HHABtfKVcbu1=Sg@mail.gmail.com
Andres Freund [Wed, 27 Feb 2019 04:30:28 +0000 (20:30 -0800)]
Use slots in trigger infrastructure, except for the actual invocation.
In preparation for abstracting table storage, convert trigger.c to
track tuples in slots. Which also happens to make code calling
triggers simpler.
As the calling interface for triggers themselves is not changed in
this patch, HeapTuples still are extracted from the slot at that
time. But that's handled solely inside trigger.c, not visible to
callers. It's quite likely that we'll want to revise the external
trigger interface, but that's a separate large project.
As part of this work the slots used for old/new/return tuples are
moved from EState into ResultRelInfo, as different updated tables
might need different slots. The slots are now also now created
on-demand, which is good both from an efficiency POV, but also makes
the modifying code simpler.
Author: Andres Freund, Amit Khandekar and Ashutosh Bapat
Discussion: https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de
Andres Freund [Wed, 27 Feb 2019 02:21:44 +0000 (18:21 -0800)]
Store table oid and tuple's tid in tuple slots directly.
After the introduction of tuple table slots all table AMs need to
support returning the table oid of the tuple stored in a slot created
by said AM. It does not make sense to re-implement that in every AM,
therefore move handling of table OIDs into the TupleTableSlot
structure itself. It's possible that we, at a later date, might want
to get rid of HeapTupleData.t_tableOid entirely, but doing so before
the abstractions for table AMs are integrated turns out to be too
hard, so delay that for now.
Similarly, every AM needs to support the concept of a tuple
identifier (tid / item pointer) for its tuples. It's quite possible
that we'll generalize the exact form of a tid at a future point (to
allow for things like index organized tables), but for now many parts
of the code know about tids, so there's not much point in abstracting
tids away. Therefore also move into slot (rather than providing API to
set/get the tid associated with the tuple in a slot).
Once table AM includes insert/updating/deleting tuples, the
responsibility to set the correct tid after such an action will move
into that. After that change, code doing such modifications, should
not have to deal with HeapTuples directly anymore.
Author: Andres Freund, Haribabu Kommi and Ashutosh Bapat
Discussion: https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de
Andres Freund [Wed, 27 Feb 2019 02:15:59 +0000 (18:15 -0800)]
Allow to use HeapTupleData embedded in [Buffer]HeapTupleTableSlot.
That avoids having to care about the lifetime of the
HeapTupleHeaderData passed to ExecStore[Buffer]HeapTuple(). That
doesn't make a huge difference for a plain HeapTupleTableSlot, but for
BufferHeapTupleTableSlot it can be a significant advantage, avoiding
the need to materialize slots where it's inconvenient to provide a
HeapTupleData with appropriate lifetime to point to the on-disk tuple.
It's quite possible that we'll want to add support functions for
constructing HeapTuples using that embedded HeapTupleData, but for now
callers do so themselves.
Author: Andres Freund
Discussion: https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de
Andres Freund [Wed, 27 Feb 2019 01:59:01 +0000 (17:59 -0800)]
Add ExecStorePinnedBufferHeapTuple.
This allows to avoid an unnecessary pin/unpin cycle when storing a
tuple in an already pinned buffer into a slot, when the pin isn't
further needed at the call site.
Only a single caller for now (to ensure coverage), but upcoming
patches will increase use of the new function.
Author: Andres Freund
Discussion: https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de
Robert Haas [Tue, 26 Feb 2019 17:22:57 +0000 (12:22 -0500)]
Change lock acquisition order in expand_inherited_rtentry.
Previously, this function acquired locks in the order using
find_all_inheritors(), which locks the children of each table that it
processes in ascending OID order, and which processes the inheritance
hierarchy as a whole in a breadth-first fashion. Now, it processes
the inheritance hierarchy in a depth-first fashion, and at each level
it proceeds in the order in which tables appear in the PartitionDesc.
If table inheritance rather than table partitioning is used, the old
order is preserved.
This change moves the locking of any given partition much closer to
the code that actually expands that partition. This seems essential
if we ever want to allow concurrent DDL to add or remove partitions,
because if the set of partitions can change, we must use the same data
to decide which partitions to lock as we do to decide which partitions
to expand; otherwise, we might expand a partition that we haven't
locked. It should hopefully also facilitate efforts to postpone
inheritance expansion or locking for performance reasons, because
there's really no way to postpone locking some partitions if
we're blindly locking them all using find_all_inheritors().
The only downside of this change which is known to me is that it
further deviates from the principle that we should always lock the
inheritance hierarchy in find_all_inheritors() order to avoid deadlock
risk. However, we've already crossed that bridge in commit 9eefba181f7782d27d85d7e94e6028371e7ab2d7 and there are futher patches
pending that make similar changes, so this isn't really giving up
anything that we haven't surrendered already -- and it seems entirely
worth it, given the performance benefits some of those changes seem
likely to bring.
Patch by me; thanks to David Rowley for discussion of these issues.
Peter Geoghegan [Tue, 26 Feb 2019 01:47:43 +0000 (17:47 -0800)]
Remove unneeded argument from _bt_getstackbuf().
_bt_getstackbuf() is called at exactly two points following commit efada2b8e92 (one call site is concerned with page splits, while the
other is concerned with page deletion). The parent buffer returned by
_bt_getstackbuf() is write-locked in both cases. Remove the 'access'
argument and make _bt_getstackbuf() assume that callers require a
write-lock.
Peter Geoghegan [Tue, 26 Feb 2019 00:54:18 +0000 (16:54 -0800)]
Correct obsolete nbtree page deletion comment.
Commit efada2b8e92, which made the nbtree page deletion algorithm more
robust, removed _bt_getstackbuf() calls from _bt_pagedel(). It failed
to update a comment that referenced the earlier approach. Update the
comment to explain that the _bt_getstackbuf() page deletion call site
mirrors the only other remaining _bt_getstackbuf() call site, which is
reached during page splits.
Peter Eisentraut [Mon, 25 Feb 2019 07:38:59 +0000 (08:38 +0100)]
Remove unnecessary use of PROCEDURAL
Remove some unnecessary, legacy-looking use of the PROCEDURAL keyword
before LANGUAGE. We mostly don't use this anymore, so some of these
look a bit old.
There is still some use in pg_dump, which is harder to remove because
it's baked into the archive format, so I'm not touching that.
Michael Paquier [Mon, 25 Feb 2019 05:19:34 +0000 (14:19 +0900)]
Make release of 2PC identifier and locks consistent in COMMIT PREPARED
When preparing a transaction in two-phase commit, a dummy PGPROC entry
holding the GID used for the transaction is registered, which gets
released once COMMIT PREPARED is run. Prior releasing its shared memory
state, all the locks taken in the prepared transaction are released
using a dedicated set of callbacks (pgstat and multixact having similar
callbacks), which may cause the locks to be released before the GID is
set free.
Hence, there is a small window where lock conflicts could happen, for
example:
- Transaction A releases its locks, still holding its GID in shared
memory.
- Transaction B held a lock which conflicted with locks of transaction
A.
- Transaction B continues its processing, reusing the same GID as
transaction A.
- Transaction B fails because of a conflicting GID, already in use by
transaction A.
This commit changes the shared memory state release so as post-commit
callbacks and predicate lock cleanup happen consistently with the shared
memory state cleanup for the dummy PGPROC entry. The race window is
small and 2PC had this issue from the start, so no backpatch is done.
On top if that fixes discussed involved ABI breakages, which are not
welcome in stable branches.
Thomas Munro [Sun, 24 Feb 2019 21:54:12 +0000 (10:54 +1300)]
Fix inconsistent out-of-memory error reporting in dsa.c.
Commit 16be2fd1 introduced the flag DSA_ALLOC_NO_OOM to control whether
the DSA allocator would raise an error or return InvalidDsaPointer on
failure to allocate. One edge case was not handled correctly: if we
fail to allocate an internal "span" object for a large allocation, we
would always return InvalidDsaPointer regardless of the flag; a caller
not expecting that could then dereference a null pointer.
This is a plausible explanation for a one-off report of a segfault.
Remove a redundant pair of braces so that all three stanzas that handle
DSA_ALLOC_NO_OOM match in style, for visual consistency.
While fixing inconsistencies, if FreePageManagerGet() can't supply the
pages that our book-keeping says it should be able to supply, then we
should always report a FATAL error. Previously we treated that as a
regular allocation failure in one code path, but as a FATAL condition
in another.
Back-patch to 10, where dsa.c landed.
Author: Thomas Munro Reported-by: Jakub Glapa
Discussion: https://postgr.es/m/CAEepm=2oPqXxyWQ-1o60tpOLrwkw=VpgNXqqF1VN2EyO9zKGQw@mail.gmail.com
Tom Lane [Sun, 24 Feb 2019 17:51:50 +0000 (12:51 -0500)]
Fix ecpg bugs caused by missing semicolons in the backend grammar.
The Bison documentation clearly states that a semicolon is required
after every grammar rule, and our scripts that generate ecpg's
grammar from the backend's implicitly assumed this is true. But it
turns out that only ancient versions of Bison actually enforce that.
There have been a couple of rules without trailing semicolons in
gram.y for some time, and as a consequence, ecpg's grammar was faulty
and produced wrong output for the affected statements.
To fix, add the missing semis, and add some cross-checks to ecpg's
scripts so that they'll bleat if we mess this up again.
The cases that were broken were:
* "SET variable = DEFAULT" (but not "SET variable TO DEFAULT"),
as well as allied syntaxes such as ALTER SYSTEM SET ... DEFAULT.
These produced syntactically invalid output that the server
would reject.
* Multiple type names in DROP TYPE/DOMAIN commands. Only the
first type name would be listed in the emitted command.
Per report from Daisuke Higuchi. Back-patch to all supported versions.
Thomas Munro [Sun, 24 Feb 2019 10:48:52 +0000 (23:48 +1300)]
Tolerate EINVAL when calling fsync() on a directory.
Previously, we tolerated EBADF as a way for the operating system to
indicate that it doesn't support fsync() on a directory. Tolerate
EINVAL too, for older versions of Linux CIFS.
Bug #15636. Back-patch all the way.
Reported-by: John Klann
Discussion: https://postgr.es/m/15636-d380890dafd78fc6@postgresql.org
Thomas Munro [Sun, 24 Feb 2019 00:38:15 +0000 (13:38 +1300)]
Tolerate ENOSYS failure from sync_file_range().
One unintended consequence of commit 9ccdd7f6 was that Windows WSL
users started getting a panic whenever we tried to initiate data
flushing with sync_file_range(), because WSL does not implement that
system call. Previously, they got a stream of periodic warnings,
which was also undesirable but at least ignorable.
Prevent the panic by handling ENOSYS specially and skipping the panic
promotion with data_sync_elevel(). Also suppress future attempts
after the first such failure so that the pre-existing problem of
noisy warnings is improved.
Back-patch to 9.6 (older branches were not affected in this way by 9ccdd7f6).
Author: Thomas Munro and James Sewell Tested-by: James Sewell Reported-by: Bruce Klein
Discussion: https://postgr.es/m/CA+mCpegfOUph2U4ZADtQT16dfbkjjYNJL1bSTWErsazaFjQW9A@mail.gmail.com
Michael Paquier [Fri, 22 Feb 2019 23:19:31 +0000 (08:19 +0900)]
Add TAP tests for 2PC post-commit callbacks of multixacts at recovery
The current set of TAP tests for two-phase transactions include some
coverage for post-commit callbacks of multixact, but it lacked tests for
testing the recovery of those callbacks. This commit adds some tests
with soft and hard restarts in this case, using transactions which
include DDLs.
Author: Michael Paquier Reviewed-by: Oleksii Kliukin
Discussion: https://postgr.es/m/20190221055431.GO15532@paquier.xyz
Tom Lane [Fri, 22 Feb 2019 17:23:00 +0000 (12:23 -0500)]
Fix plan created for inherited UPDATE/DELETE with all tables excluded.
In the case where inheritance_planner() finds that every table has
been excluded by constraints, it thought it could get away with
making a plan consisting of just a dummy Result node. While certainly
there's no updating or deleting to be done, this had two user-visible
problems: the plan did not report the correct set of output columns
when a RETURNING clause was present, and if there were any
statement-level triggers that should be fired, it didn't fire them.
Hence, rather than only generating the dummy Result, we need to
stick a valid ModifyTable node on top, which requires a tad more
effort here.
It's been broken this way for as long as inheritance_planner() has
known about deleting excluded subplans at all (cf commit 635d42e9c),
so back-patch to all supported branches.
Amit Langote and Tom Lane, per a report from Petr Fedorov.
Peter Eisentraut [Fri, 22 Feb 2019 08:01:19 +0000 (09:01 +0100)]
Add const qualifier
New code introduced in 050710b36964dee7e1b2bf6b5ef00041fd5d2787. The
lack of const is not currently a compiler warning, but it's nice to
have for consistency with surrounding code.
Tom Lane [Thu, 21 Feb 2019 23:55:29 +0000 (18:55 -0500)]
Fix mark-and-restore-skipping test case to not be a self-join.
There isn't any good reason for this test to be a self-join rather
than a join between separate tables, except that it saved a couple
of SQL commands for setup. A proposed patch to optimize away
self-joins breaks the test, so adjust it to avoid that happening.
Tom Lane [Thu, 21 Feb 2019 19:59:02 +0000 (14:59 -0500)]
Move estimate_hashagg_tablesize to selfuncs.c, and widen result to double.
It seems to make more sense for this to be in selfuncs.c, since it's
largely a statistical-estimation thing, and it's related to other
functions like estimate_hash_bucket_stats that are there.
While at it, change the result type from Size to double. Perhaps at one
point it was impossible for the result to overflow an integer, but
I've got no confidence in that proposition anymore. Nothing's actually
done with the result except to compare it to a work_mem-based limit,
so as long as we don't get an overflow on the way to that comparison,
things should be fine even with very large dNumGroups.
Code movement proposed by Antonin Houska, type change by me
Peter Eisentraut [Wed, 20 Feb 2019 10:38:44 +0000 (11:38 +0100)]
Hide other user's pg_stat_ssl rows
Change pg_stat_ssl so that an unprivileged user can only see their own
rows; other rows will be all null. This makes the behavior consistent
with pg_stat_activity, where information about where the connection
came from is also restricted.
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/flat/63117976-d02c-c8e2-3aef-caa31a5ab8d3%402ndquadrant.com
Peter Eisentraut [Thu, 21 Feb 2019 17:34:19 +0000 (18:34 +0100)]
pg_regress: Don't use absolute paths for the diff
Don't expand inputfile and outputfile to absolute paths globally, just
where needed. In particular, pass them as is to the file name
arguments of the diff command, so that we don't see the full absolute
path in the diff header, which makes the diff unnecessarily verbose
and harder to read.
Robert Haas [Thu, 21 Feb 2019 16:38:54 +0000 (11:38 -0500)]
Move code for managing PartitionDescs into a new file, partdesc.c
This is similar in spirit to the existing partbounds.c file in the
same directory, except that there's a lot less code in the new file
created by this commit. Pending work in this area proposes to add a
bunch more code related to PartitionDescs, though, and this will give
us a good place to put it.
Robert Haas [Thu, 21 Feb 2019 16:24:40 +0000 (11:24 -0500)]
Delay lock acquisition for partitions until we route a tuple to them.
Instead of locking all partitions to which we might route a tuple at
executor startup, just lock them as we use them. In some cases such a
partition might get locked at executor startup anyway because it
appears in the query's range table for some other reason, but in other
cases this is a bit savings.
This changes the order in which partitions are locked in some cases,
which might conceivably create deadlock hazards that don't exist
today, but per discussion, it seems like such cases should be rare
enough that we can neglect them in favor of improving performance.
David Rowley, reviewed and tested by Tomas Vondra, Sho Kato, John
Naylor, Tom Lane, and me.
Peter Eisentraut [Thu, 21 Feb 2019 14:39:37 +0000 (15:39 +0100)]
Fix dbtoepub output file name
In previous releases, the input file of dbtoepub was postgres.xml, and
dbtoepub knows to derive the output file name postgres.epub from that
automatically. But now the intput file is postgres.sgml (since
postgres.sgml is itself an XML file and we no longer need the
intermediate postgres.xml file), but dbtoepub doesn't know how to deal
with the .sgml suffix, so the automatically derived output file name
becomes postgres.sgml.epub. Fix by adding an explicit -o option.
Tom Lane [Thu, 21 Feb 2019 01:53:08 +0000 (20:53 -0500)]
Speed up match_eclasses_to_foreign_key_col() when there are many ECs.
Check ec_relids before bothering to iterate through the EC members.
On a perhaps extreme, but still real-world, query in which
match_eclasses_to_foreign_key_col() accounts for the bulk of the
planner's runtime, this saves nearly 40% of the runtime. It's a bit
of a stopgap fix, but it's simple enough to be back-patched to 9.6
where this code came in; so let's do that.
Andrew Gierth [Wed, 20 Feb 2019 21:31:02 +0000 (21:31 +0000)]
Use an unsigned char for bool if we don't use the native bool.
On (rare) platforms where sizeof(bool) > 1, we need to use our own
bool, but imported c99 code (such as Ryu) may want to use bool values
as array subscripts, which elicits warnings if bool is defined as
char. Using unsigned char instead should work just as well for our
purposes, and avoid such warnings.
Tom Lane [Wed, 20 Feb 2019 19:39:11 +0000 (14:39 -0500)]
Improve planner's understanding of strictness of type coercions.
PG type coercions are generally strict, ie a NULL input must produce
a NULL output (or, in domain cases, possibly an error). The planner's
understanding of that was a bit incomplete though, so improve it:
* Teach contain_nonstrict_functions() that CoerceViaIO can always be
considered strict. Previously it believed that only if the underlying
I/O functions were marked strict, which is often but not always true.
* Teach clause_is_strict_for() that CoerceViaIO, ArrayCoerceExpr,
ConvertRowtypeExpr, CoerceToDomain can all be considered strict.
Previously it knew nothing about any of them.
The main user-visible impact of this is that IS NOT NULL predicates
can be proven to hold from expressions involving casts in more cases
than before, allowing partial indexes with such predicates to be used
without extra pushups. This reduces the surprise factor for users,
who may well be used to ordinary (function-call-based) casts being
known to be strict.
Per a gripe from Samuel Williams. This doesn't rise to the level of
a bug, IMO, so no back-patch.
projects / postgresql / log