Peter Geoghegan [Thu, 31 Dec 2020 00:29:05 +0000 (16:29 -0800)]
Fix index deletion latestRemovedXid bug.
The logic for determining the latest removed XID for the purposes of
generating recovery conflicts in REDO routines was subtly broken. It
failed to follow links from HOT chains, and so failed to consider all
relevant heap tuple headers in some cases.
To fix, expand the loop that deals with LP_REDIRECT line pointers to
also deal with HOT chains. The new version of the loop is loosely based
on a similar loop from heap_prune_chain().
The impact of this bug is probably quite limited, since the horizon code
necessarily deals with heap tuples that are pointed to by LP_DEAD-set
index tuples. The process of setting LP_DEAD index tuples (e.g. within
the kill_prior_tuple mechanism) is highly correlated with opportunistic
pruning of pointed-to heap tuples. Plus the question of generating a
recovery conflict usually comes up some time after index tuple LP_DEAD
bits were initially set, unlike heap pruning, where a latestRemovedXid
is generated at the point of the pruning operation (heap pruning has no
deferred "would-be page split" style processing that produces conflicts
lazily).
Only backpatch to Postgres 12, the first version where this logic runs
during original execution (following commit 558a9165e08). The index
latestRemovedXid mechanism has had the same bug since it first appeared
over 10 years ago (in commit a760893d), but backpatching to all
supported versions now seems like a bad idea on balance. Running the
new improved code during recovery seems risky, especially given the lack
of complaints from the field.
Author: Peter Geoghegan <pg@bowt.ie>
Discussion: https://postgr.es/m/CAH2-Wz=Eib393+HHcERK_9MtgNS7Ew1HY=RDC_g6GL46zM5C6Q@mail.gmail.com
Backpatch: 12-
Tom Lane [Wed, 30 Dec 2020 22:48:43 +0000 (17:48 -0500)]
Doc: spell out comparison behaviors for the date/time types.
The behavior of cross-type comparisons among date/time data types was
not really explained anywhere. You could probably infer it if you
recognized the applicability of comments elsewhere about datatype
conversions, but it seems worthy of explicit documentation.
Tom Lane [Wed, 30 Dec 2020 19:15:41 +0000 (14:15 -0500)]
More fixups for pg_upgrade cross-version tests.
Commit 7ca37fb04 removed regress_putenv from the regress.so library,
so reloading a SQL function dependent on that would not work.
Fix similarly to 52202bb39.
This commit preserves the logic of multirange_in() but makes it more clear
what's going on. Also, this commit fixes the compiler warning spotted by the
buildfarm.
Reported-by: Tom Lane
Discussion: https://postgr.es/m/2246043.1609290699%40sss.pgh.pa.us
Tom Lane [Wed, 30 Dec 2020 17:55:59 +0000 (12:55 -0500)]
Use setenv() in preference to putenv().
Since at least 2001 we've used putenv() and avoided setenv(), on the
grounds that the latter was unportable and not in POSIX. However,
POSIX added it that same year, and by now the situation has reversed:
setenv() is probably more portable than putenv(), since POSIX now
treats the latter as not being a core function. And setenv() has
cleaner semantics too. So, let's reverse that old policy.
This commit adds a simple src/port/ implementation of setenv() for
any stragglers (we have one in the buildfarm, but I'd not be surprised
if that code is never used in the field). More importantly, extend
win32env.c to also support setenv(). Then, replace usages of putenv()
with setenv(), and get rid of some ad-hoc implementations of setenv()
wannabees.
Also, adjust our src/port/ implementation of unsetenv() to follow the
POSIX spec that it returns an error indicator, rather than returning
void as per the ancient BSD convention. I don't feel a need to make
all the call sites check for errors, but the portability stub ought
to match real-world practice.
Attempt to get selectivity estimation for @> (anymultirange, anyrange) operator
caused an error in buildfarm, because this operator was missed in switch()
of calc_hist_selectivity(). Fix that and also make regression tests reliably
check that selectivity estimation for (multi)ranges doesn't fall. Previously,
whether we test selectivity estimation for (multi)ranges depended on
whether autovacuum managed to gather concurrently to the test.
Reported-by: Michael Paquier
Discussion: https://postgr.es/m/X%2BwmgjRItuvHNBeV%40paquier.xyz
Tom Lane [Wed, 30 Dec 2020 16:38:42 +0000 (11:38 -0500)]
Fix up usage of krb_server_keyfile GUC parameter.
secure_open_gssapi() installed the krb_server_keyfile setting as
KRB5_KTNAME unconditionally, so long as it's not empty. However,
pg_GSS_recvauth() only installed it if KRB5_KTNAME wasn't set already,
leading to a troubling inconsistency: in theory, clients could see
different sets of server principal names depending on whether they
use GSSAPI encryption. Always using krb_server_keyfile seems like
the right thing, so make both places do that. Also fix up
secure_open_gssapi()'s lack of a check for setenv() failure ---
it's unlikely, surely, but security-critical actions are no place
to be sloppy.
Also improve the associated documentation.
This patch does nothing about secure_open_gssapi()'s use of setenv(),
and indeed causes pg_GSS_recvauth() to use it too. That's nominally
against project portability rules, but since this code is only built
with --with-gssapi, I do not feel a need to do something about this
in the back branches. A fix will be forthcoming for HEAD though.
Back-patch to v12 where GSSAPI encryption was introduced. The
dubious behavior in pg_GSS_recvauth() goes back further, but it
didn't have anything to be inconsistent with, so let it be.
Michael Paquier [Wed, 30 Dec 2020 12:23:24 +0000 (21:23 +0900)]
Sanitize IF NOT EXISTS in EXPLAIN for CTAS and matviews
IF NOT EXISTS was ignored when specified in an EXPLAIN query for CREATE
MATERIALIZED VIEW or CREATE TABLE AS. Hence, if this clause was
specified, the caller would get a failure if the relation already
exists instead of a success with a NOTICE message.
This commit makes the behavior of IF NOT EXISTS in EXPLAIN consistent
with the non-EXPLAIN'd DDL queries, preventing a failure with IF NOT
EXISTS if the relation to-be-created already exists. The skip is done
before the SELECT query used for the relation is planned or executed,
and a "dummy" plan is generated instead depending on the format used by
EXPLAIN.
Author: Bharath Rupireddy Reviewed-by: Zhijie Hou, Michael Paquier
Discussion: https://postgr.es/m/CALj2ACVa3oJ9O_wcGd+FtHWZds04dEKcakxphGz5POVgD4wC7Q@mail.gmail.com
Most of this is a simple extension of the existing methods, with the
semantic difference that the transaction is not yet committed and maybe
aborted later.
Until now two-phase transactions were translated into regular transactions
on the subscriber, and the GID was not forwarded to it. None of the
two-phase commands were communicated to the subscriber.
This patch provides the infrastructure for logical decoding plugins to be
informed of two-phase commands Like PREPARE TRANSACTION, COMMIT PREPARED
and ROLLBACK PREPARED commands with the corresponding GID.
This also extends the 'test_decoding' plugin, implementing these new
methods.
This commit simply adds these new APIs and the upcoming patch to "allow
the decoding at prepare time in ReorderBuffer" will use these APIs.
Author: Ajin Cherian and Amit Kapila based on previous work by Nikhil Sontakke and Stas Kelvich Reviewed-by: Amit Kapila, Peter Smith, Sawada Masahiko, and Dilip Kumar
Discussion:
https://postgr.es/m/02DA5F5E-CECE-4D9C-8B4B-418077E2C010@postgrespro.ru
https://postgr.es/m/CAMGcDxeqEpWj3fTXwqhSwBdXd2RS9jzwWscO-XbeCfso6ts3+Q@mail.gmail.com
Noah Misch [Wed, 30 Dec 2020 09:43:43 +0000 (01:43 -0800)]
In pg_upgrade cross-version test, handle postfix operators.
Commit 1ed6b895634ce0dc5fd4bd040e87252b32182cba eliminated support for
them, so drop them from regression databases before upgrading. This is
necessary but not sufficient for testing v13 -> v14 upgrades.
Michael Paquier [Wed, 30 Dec 2020 03:38:16 +0000 (12:38 +0900)]
Remove references to libpq_srcdir in adminpack and old_snapshot
Those two modules included references to libpq's source path, without
using anything from libpq. Some copy-pastos done when each module was
created are likely at the origin of those useless references (aecf5ee
for old_snapshot, fe59e56 for adminpack).
Reviewed-by: Tom Lane, David Rowley
Discussion: https://postgr.es/m/X+LQpfLyk7jgzUki@paquier.xyz
Tom Lane [Wed, 30 Dec 2020 01:44:03 +0000 (20:44 -0500)]
Doc: fix up PDF build warnings from over-width table columns.
Addition of multirange info to tables 8.27 and 65.1 made them start
throwing "exceed the available area" warnings in PDF docs builds.
For 8.27, twiddling the existing column width hints was enough to
fix this. For 65.1, I twiddled the widths a little, but to really
fix it I had to insert a space after each comma in the table, to
allow a line break to occur there. (This seemed easier to read
and maintain than the alternative of inserting &zwsp; entities.)
Tom Lane [Tue, 29 Dec 2020 23:02:38 +0000 (18:02 -0500)]
Suppress log spam from multiple reports of SIGQUIT shutdown.
When the postmaster sends SIGQUIT to its children, there's no real
need for all the children to log that fact; the postmaster already
made a log entry about it, so adding perhaps dozens or hundreds of
child-process log entries adds nothing of value. So, let's introduce
a new ereport level to specify "WARNING, but never send to log" and
use that for these messages.
Such a change wouldn't have been desirable before commit 7e784d1dc,
because if someone manually SIGQUIT's a backend, we *do* want to log
that. But now we can tell the difference between a signal that was
issued by the postmaster and one that was not with reasonable
certainty.
While we're here, also clear error_context_stack before ereport'ing,
to prevent error callbacks from being invoked in the signal-handler
context. This should reduce the odds of getting hung up while trying
to notify the client.
Add support of multirange matching to the existing range GiST indexes
6df7a9698b has introduced a set of operators between ranges and multiranges.
Existing GiST indexes for ranges could easily support majority of them.
This commit adds support for new operators to the existing range GiST indexes.
New operators resides the same strategy numbers as existing ones. Appropriate
check function is determined using the subtype.
Improve the signature of internal multirange functions
There is a set of *_internal() functions exposed in
include/utils/multirangetypes.h. This commit improves the signatures of these
functions in two ways.
* Add const qualifies where applicable.
* Replace multirange typecache argument with range typecache argument.
Multirange typecache was used solely to find the range typecache. At the
same time, range typecache is easier for the caller to find.
Implement operators for checking if the range contains a multirange
We have operators for checking if the multirange contains a range but don't
have the opposite. This commit improves completeness of the operator set by
adding two new operators: @> (anyrange,anymultirange) and
<@(anymultirange,anyrange).
Fix bugs in comparison functions for multirange_bsearch_match()
Two functions multirange_range_overlaps_bsearch_comparison() and
multirange_range_contains_bsearch_comparison() contain bugs of returning -1
instead of 1. This commit fixes these bugs and adds corresponding regression
tests.
Michael Paquier [Tue, 29 Dec 2020 07:49:14 +0000 (16:49 +0900)]
doc: Improve description of min_dynamic_shared_memory
While on it, fix one oversight in 90fbf7c, that introduced a reference
to an incorrect value for the compression level of pg_dump.
Author: Justin Pryzby Reviewed-by: Thomas Munro, Michael Paquier
Discussion: https://postgr.es/m/CA+hUKGJRTLWWPcQfjm_xaOk98M8aROK903X92O0x-4vLJPWrrA@mail.gmail.com
Tom Lane [Mon, 28 Dec 2020 22:44:17 +0000 (17:44 -0500)]
Fix assorted issues in backend's GSSAPI encryption support.
Unrecoverable errors detected by GSSAPI encryption can't just be
reported with elog(ERROR) or elog(FATAL), because attempting to
send the error report to the client is likely to lead to infinite
recursion or loss of protocol sync. Instead make this code do what
the SSL encryption code has long done, which is to just report any
such failure to the server log (with elevel COMMERROR), then pretend
we've lost the connection by returning errno = ECONNRESET.
Along the way, fix confusion about whether message translation is done
by pg_GSS_error() or its callers (the latter should do it), and make
the backend version of that function work more like the frontend
version.
Avoid allocating the port->gss struct until it's needed; we surely
don't need to allocate it in the postmaster.
Improve logging of "connection authorized" messages with GSS enabled.
(As part of this, I back-patched the code changes from dc11f31a1.)
Make BackendStatusShmemSize() account for the GSS-related space that
will be allocated by CreateSharedBackendStatus(). This omission
could possibly cause out-of-shared-memory problems with very high
max_connections settings.
Remove arbitrary, pointless restriction that only GSS authentication
can be used on a GSS-encrypted connection.
Improve documentation; notably, document the fact that libpq now
prefers GSS encryption over SSL encryption if both are possible.
Per report from Mikael Gustavsson. Back-patch to v12 where
this code was introduced.
Tom Lane [Mon, 28 Dec 2020 20:43:44 +0000 (15:43 -0500)]
Fix bugs in libpq's GSSAPI encryption support.
The critical issue fixed here is that if a GSSAPI-encrypted connection
is successfully made, pqsecure_open_gss() cleared conn->allow_ssl_try,
as an admittedly-hacky way of preventing us from then trying to tunnel
SSL encryption over the already-encrypted connection. The problem
with that is that if we abandon the GSSAPI connection because of a
failure during authentication, we would not attempt SSL encryption
in the next try with the same server. This can lead to unexpected
connection failure, or silently getting a non-encrypted connection
where an encrypted one is expected.
Fortunately, we'd only manage to make a GSSAPI-encrypted connection
if both client and server hold valid tickets in the same Kerberos
infrastructure, which is a relatively uncommon environment.
Nonetheless this is a very nasty bug with potential security
consequences. To fix, don't reset the flag, instead adding a
check for conn->gssenc being already true when deciding whether
to try to initiate SSL.
While here, fix some lesser issues in libpq's GSSAPI code:
* Use the need_new_connection stanza when dropping an attempted
GSSAPI connection, instead of partially duplicating that code.
The consequences of this are pretty minor: AFAICS it could only
lead to auth_req_received or password_needed remaining set when
they shouldn't, which is not too harmful.
* Fix pg_GSS_error() to not repeat the "mprefix" it's given multiple
times, and to notice any failure return from gss_display_status().
* Avoid gratuitous dependency on NI_MAXHOST in
pg_GSS_load_servicename().
Per report from Mikael Gustavsson. Back-patch to v12 where
this code was introduced.
Tom Lane [Mon, 28 Dec 2020 16:55:23 +0000 (11:55 -0500)]
Further fix thinko in plpgsql memory leak fix.
There's a second call of get_eval_mcontext() that should also be
get_stmt_mcontext(). This is actually dead code, since no
interesting allocations happen before switching back to the
original context, but we should keep it in sync with the other
call to forestall possible future bugs.
Tom Lane [Mon, 28 Dec 2020 16:41:25 +0000 (11:41 -0500)]
Fix thinko in plpgsql memory leak fix.
Commit a6b1f5365 intended to place the transient "target" list of
a CALL statement in the function's statement-lifespan context,
but I fat-fingered that and used get_eval_mcontext() instead of
get_stmt_mcontext(). The eval_mcontext belongs to the "simple
expression" infrastructure, which is destroyed at transaction end.
The net effect is that a CALL in a procedure to another procedure
that has OUT or INOUT parameters would fail if the called procedure
did a COMMIT.
Per report from Peter Eisentraut. Back-patch to v11, like the
prior patch.
Michael Paquier [Mon, 28 Dec 2020 13:16:49 +0000 (22:16 +0900)]
Fix inconsistent code with shared invalidations of snapshots
The code in charge of processing a single invalidation message has been
using since 568d413 the structure for relation mapping messages. This
had fortunately no consequence as both locate the database ID at the
same location, but it could become a problem in the future if this area
of the code changes.
Author: Konstantin Knizhnik
Discussion: https://postgr.es/m/8044c223-4d3a-2cdb-42bf-29940840ce94@postgrespro.ru
Backpatch-through: 9.5
Fujii Masao [Mon, 28 Dec 2020 10:56:13 +0000 (19:56 +0900)]
postgres_fdw: Fix connection leak.
In postgres_fdw, the cached connections to foreign servers will not be
closed until the local session exits if the user mappings or foreign servers
that those connections depend on are dropped. Those connections can be
leaked.
To fix that connection leak issue, after a change to a pg_foreign_server
or pg_user_mapping catalog entry, this commit makes postgres_fdw close
the connections depending on that entry immediately if current
transaction has not used those connections yet. Otherwise, mark those
connections as invalid and then close them at the end of current transaction,
since they cannot be closed in the midst of the transaction using them.
Closed connections will be remade at the next opportunity if necessary.
Bruce Momjian [Mon, 28 Dec 2020 02:37:42 +0000 (21:37 -0500)]
Revert "Add key management system" (978f869b99) & later commits
The patch needs test cases, reorganization, and cfbot testing.
Technically reverts commits 5c31afc49d..e35b2bad1a (exclusive/inclusive)
and 08db7c63f3..ccbe34139b.
Reported-by: Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/E1ktAAG-0002V2-VB@gemulon.postgresql.org
Removing the EXPLAIN test to stabilize the buildfarm. The execution
test should still be effective to catch the bug even if the plan is
slightly different on different platforms.
Jeff Davis [Sun, 27 Dec 2020 01:25:30 +0000 (17:25 -0800)]
Fix bug #16784 in Disk-based Hash Aggregation.
Before processing tuples, agg_refill_hash_table() was setting all
pergroup pointers to NULL to signal to advance_aggregates() that it
should not attempt to advance groups that had spilled.
The problem was that it also set the pergroups for sorted grouping
sets to NULL, which caused rescanning to fail.
Instead, change agg_refill_hash_table() to only set the pergroups for
hashed grouping sets to NULL; and when compiling the expression, pass
doSort=false.
Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/16784-7ff169bf2c3d1588%40postgresql.org
Backpatch-through: 13
Noah Misch [Fri, 25 Dec 2020 18:41:59 +0000 (10:41 -0800)]
Invalidate acl.c caches when pg_authid changes.
This makes existing sessions reflect "ALTER ROLE ... [NO]INHERIT" as
quickly as they have been reflecting "GRANT role_name". Back-patch to
9.5 (all supported versions).
Bruce Momjian [Fri, 25 Dec 2020 15:19:44 +0000 (10:19 -0500)]
Add key management system
This adds a key management system that stores (currently) two data
encryption keys of length 128, 192, or 256 bits. The data keys are
AES256 encrypted using a key encryption key, and validated via GCM
cipher mode. A command to obtain the key encryption key must be
specified at initdb time, and will be run at every database server
start. New parameters allow a file descriptor open to the terminal to
be passed. pg_upgrade support has also been added.
Tom Lane [Fri, 25 Dec 2020 02:37:46 +0000 (21:37 -0500)]
Avoid time-of-day-dependent failure in log rotation test.
Buildfarm members pogona and petalura have shown a failure when
pg_ctl/t/004_logrotate.pl starts just before local midnight.
The default rotate-at-midnight behavior occurs just before the
Perl script examines current_logfiles, so it figures that the
rotation it's already requested has occurred ... but in reality,
that rotation happens just after it looks, so the expected new
log data goes into a different file than the one it's examining.
In HEAD, src/test/kerberos/t/001_auth.pl has acquired similar code
that evidently has a related failure mode. Besides being quite new,
few buildfarm critters run that test, so it's unsurprising that
we've not yet seen a failure there.
Fix both cases by setting log_rotation_age = 0 so that no time-based
rotation can occur. Also absorb 004_logrotate.pl's decision to
set lc_messages = 'C' into the kerberos test, in hopes that it will
work in non-English prevailing locales.
Bruce Momjian [Thu, 24 Dec 2020 23:21:37 +0000 (18:21 -0500)]
revert removal of hex_decode() from ecpg from commit c3826f831e
ecpglib on certain platforms can't handle the pg_log_fatal calls from
libraries. This was reported by the buildfarm. It needs a refactoring
and return value change if it is later removed.
Tom Lane [Thu, 24 Dec 2020 22:00:43 +0000 (17:00 -0500)]
Fix race condition between shutdown and unstarted background workers.
If a database shutdown (smart or fast) is commanded between the time
some process decides to request a new background worker and the time
that the postmaster can launch that worker, then nothing happens
because the postmaster won't launch any bgworkers once it's exited
PM_RUN state. This is fine ... unless the requesting process is
waiting for that worker to finish (or even for it to start); in that
case the requestor is stuck, and only manual intervention will get us
to the point of being able to shut down.
To fix, cancel pending requests for workers when the postmaster sends
shutdown (SIGTERM) signals, and similarly cancel any new requests that
arrive after that point. (We can optimize things slightly by only
doing the cancellation for workers that have waiters.) To fit within
the existing bgworker APIs, the "cancel" is made to look like the
worker was started and immediately stopped, causing deregistration of
the bgworker entry. Waiting processes would have to deal with
premature worker exit anyway, so this should introduce no bugs that
weren't there before. We do have a side effect that registration
records for restartable bgworkers might disappear when theoretically
they should have remained in place; but since we're shutting down,
that shouldn't matter.
Back-patch to v10. There might be value in putting this into 9.6
as well, but the management of bgworkers is a bit different there
(notably see 8ff518699) and I'm not convinced it's worth the effort
to validate the patch for that branch.
Tom Lane [Thu, 24 Dec 2020 17:58:32 +0000 (12:58 -0500)]
Improve client error messages for immediate-stop situations.
Up to now, if the DBA issued "pg_ctl stop -m immediate", the message
sent to clients was the same as for a crash-and-restart situation.
This is confusing, not least because the message claims that the
database will soon be up again, something we have no business
predicting.
Improve things so that we can generate distinct messages for the two
cases (and also recognize an ad-hoc SIGQUIT, should somebody try that).
To do that, add a field to pmsignal.c's shared memory data structure
that the postmaster sets just before broadcasting SIGQUIT to its
children. No interlocking seems to be necessary; the intervening
signal-sending and signal-receipt should sufficiently serialize accesses
to the field. Hence, this isn't any riskier than the existing usages
of pmsignal.c.
We might in future extend this idea to improve other
postmaster-to-children signal scenarios, although none of them
currently seem to be as badly overloaded as SIGQUIT.
Michael Paquier [Wed, 23 Dec 2020 03:51:22 +0000 (12:51 +0900)]
Fix portability issues with parsing of recovery_target_xid
The parsing of this parameter has been using strtoul(), which is not
portable across platforms. On most Unix platforms, unsigned long has a
size of 64 bits, while on Windows it is 32 bits. It is common in
recovery scenarios to rely on the output of txid_current() or even the
newer pg_current_xact_id() to get a transaction ID for setting up
recovery_target_xid. The value returned by those functions includes the
epoch in the computed result, which would cause strtoul() to fail where
unsigned long has a size of 32 bits once the epoch is incremented.
WAL records and 2PC data include only information about 32-bit XIDs and
it is not possible to have XIDs across more than one epoch, so
discarding the high bits from the transaction ID set has no impact on
recovery. On the contrary, the use of strtoul() prevents a consistent
behavior across platforms depending on the size of unsigned long.
This commit changes the parsing of recovery_target_xid to use
pg_strtouint64() instead, available down to 9.6. There is one TAP test
stressing recovery with recovery_target_xid, where a tweak based on
pg_reset{xlog,wal} is added to bump the XID epoch so as this change gets
tested, as per an idea from Alexander Lakhin.
Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/16780-107fd0c0385b1035@postgresql.org
Backpatch-through: 9.6
Tom Lane [Tue, 22 Dec 2020 18:23:49 +0000 (13:23 -0500)]
Improve autoprewarm's handling of early-shutdown scenarios.
Bad things happen if the DBA issues "pg_ctl stop -m fast" before
autoprewarm finishes loading its list of blocks to prewarm.
The current worker process successfully terminates early, but
(if this wasn't the last database with blocks to prewarm) the
leader process will just try to launch another worker for the
next database. Since the postmaster is now in PM_WAIT_BACKENDS
state, it ignores the launch request, and the leader just sits
until it's killed manually.
This is mostly the fault of our half-baked design for launching
background workers, but a proper fix for that is likely to be
too invasive to be back-patchable. To ameliorate the situation,
fix apw_load_buffers() to check whether SIGTERM has arrived
just before trying to launch another worker. That leaves us with
only a very narrow window in each worker launch where SIGTERM
could occur between the launch request and successful worker start.
Another issue is that if the leader process does manage to exit,
it unconditionally rewrites autoprewarm.blocks with only the
blocks currently in shared buffers, thus forgetting any blocks
that we hadn't reached yet while prewarming. This seems quite
unhelpful, since the next database start will then not have the
expected prewarming benefit. Fix it to not modify the file if
we shut down before the initial load attempt is complete.
Per bug #16785 from John Thompson. Back-patch to v11 where
the autoprewarm code was introduced.
Clarify the relationship between find_em_expr_usable_for_sorting_rel and
prepare_sort_from_pathkeys, i.e. what restrictions need to be shared
between those two places.
Author: James Coleman Reviewed-by: Tomas Vondra
Backpatch-through: 13
Discussion: https://postgr.es/m/CAAaqYe8cK3g5CfLC4w7bs%3DhC0mSksZC%3DH5M8LSchj5e5OxpTAg%40mail.gmail.com
Tomas Vondra [Mon, 21 Dec 2020 18:37:14 +0000 (19:37 +0100)]
Don't search for volatile expr in find_em_expr_usable_for_sorting_rel
While prepare_sort_from_pathkeys has to be concerned about matching up
a volatile expression to the proper tlist entry, we don't need to do
that in find_em_expr_usable_for_sorting_rel becausee such a sort will
have to be postponed anyway.
Author: James Coleman Reviewed-by: Tomas Vondra
Backpatch-through: 13
Discussion: https://postgr.es/m/CAAaqYe8cK3g5CfLC4w7bs%3DhC0mSksZC%3DH5M8LSchj5e5OxpTAg%40mail.gmail.com
Tomas Vondra [Mon, 21 Dec 2020 17:58:32 +0000 (18:58 +0100)]
Disallow SRFs when considering sorts below Gather Merge
While we do allow SRFs in ORDER BY, scan/join processing should not
consider such cases - such sorts should only happen via final Sort atop
a ProjectSet. So make sure we don't try adding such sorts below Gather
Merge, just like we do for expressions that are volatile and/or not
parallel safe.
Backpatch to PostgreSQL 13, where this code was introduced as part of
the Incremental Sort patch.
Author: James Coleman Reviewed-by: Tomas Vondra
Backpatch-through: 13
Discussion: https://postgr.es/m/CAAaqYe8cK3g5CfLC4w7bs=hC0mSksZC=H5M8LSchj5e5OxpTAg@mail.gmail.com
Discussion: https://postgr.es/m/295524.1606246314%40sss.pgh.pa.us
Tom Lane [Mon, 21 Dec 2020 18:11:29 +0000 (13:11 -0500)]
Remove "invalid concatenation of jsonb objects" error case.
The jsonb || jsonb operator arbitrarily rejected certain combinations
of scalar and non-scalar inputs, while being willing to concatenate
other combinations. This was of course quite undocumented. Rather
than trying to document it, let's just remove the restriction,
creating a uniform rule that unless we are handling an object-to-object
concatenation, non-array inputs are converted to one-element arrays,
resulting in an array-to-array concatenation. (This does not change
the behavior for any case that didn't throw an error before.)
Per complaint from Joel Jacobson. Back-patch to all supported branches.
Tomas Vondra [Mon, 21 Dec 2020 17:29:46 +0000 (18:29 +0100)]
Check parallel safety in generate_useful_gather_paths
Commit ebb7ae839d ensured we ignore pathkeys with volatile expressions
when considering adding a sort below a Gather Merge. Turns out we need
to care about parallel safety of the pathkeys too, otherwise we might
try sorting e.g. on results of a correlated subquery (as demonstrated
by a report from Luis Roberto).
Initial investigation by Tom Lane, patch by James Coleman. Backpatch
to 13, where the code was instroduced (as part of Incremental Sort).
Reported-by: Luis Roberto
Author: James Coleman Reviewed-by: Tomas Vondra
Backpatch-through: 13
Discussion: https://postgr.es/m/622580997.37108180.1604080457319.JavaMail.zimbra%40siscobra.com.br
Discussion: https://postgr.es/m/CAAaqYe8cK3g5CfLC4w7bs=hC0mSksZC=H5M8LSchj5e5OxpTAg@mail.gmail.com
Tomas Vondra [Mon, 21 Dec 2020 17:09:57 +0000 (18:09 +0100)]
Consider unsorted paths in generate_useful_gather_paths
generate_useful_gather_paths used to skip unsorted paths (without any
pathkeys), but that is unnecessary - the later code actually can handle
such paths just fine by adding a Sort node. This is clearly a thinko,
preventing construction of useful plans.
Backpatch to 13, where Incremental Sort was introduced.
Author: James Coleman Reviewed-by: Tomas Vondra
Backpatch-through: 13
Discussion: https://postgr.es/m/CAAaqYe8cK3g5CfLC4w7bs=hC0mSksZC=H5M8LSchj5e5OxpTAg@mail.gmail.com
6df7a9698b introduces multirange types, whose typanalyze function shares
infrastructure with range types typanalyze function. Since 6df7a9698b,
information about type gathered by statistics is filled from typcache.
But typalign is mistakenly always set to double. This commit fixes this
oversight.
Tom Lane [Sun, 20 Dec 2020 20:28:22 +0000 (15:28 -0500)]
Doc: fix description of how to use src/tutorial files.
The separate "cd" command before invoking psql made sense (or at least
I thought so) when it was added in commit ed1939332. But 4e3a61635
removed the supporting text that explained when to use it, making it
just confusing. So drop it.
Also switch from four-dot to three-dot filler for the unsupplied
part of the path, since at least one person has read the four-dot
filler as a typo for "../..". And fix these/those inconsistency.
Tom Lane [Sun, 20 Dec 2020 17:38:32 +0000 (12:38 -0500)]
Avoid memcpy() with same source and destination in pgstat_recv_replslot.
Same type of issue as in commit 53d4f5fef and earlier fixes; also
found by apparently-more-picky-than-the-buildfarm valgrind testing.
This one is an oversight in commit 986816750. Since that's new in
HEAD, no need for a back-patch.
Multiranges are basically sorted arrays of non-overlapping ranges with
set-theoretic operations defined over them.
Since v14, each range type automatically gets a corresponding multirange
datatype. There are both manual and automatic mechanisms for naming multirange
types. Once can specify multirange type name using multirange_type_name
attribute in CREATE TYPE. Otherwise, a multirange type name is generated
automatically. If the range type name contains "range" then we change that to
"multirange". Otherwise, we add "_multirange" to the end.
Implementation of multiranges comes with a space-efficient internal
representation format, which evades extra paddings and duplicated storage of
oids. Altogether this format allows fetching a particular range by its index
in O(n).
Statistic gathering and selectivity estimation are implemented for multiranges.
For this purpose, stored multirange is approximated as union range without gaps.
This field will likely need improvements in the future.
Catversion is bumped.
Discussion: https://postgr.es/m/CALNJ-vSUpQ_Y%3DjXvTxt1VYFztaBSsWVXeF1y6gTYQ4bOiWDLgQ%40mail.gmail.com
Discussion: https://postgr.es/m/a0b8026459d1e6167933be2104a6174e7d40d0ab.camel%40j-davis.com#fe7218c83b08068bfffb0c5293eceda0
Author: Paul Jungwirth, revised by me Reviewed-by: David Fetter, Corey Huinker, Jeff Davis, Pavel Stehule Reviewed-by: Alvaro Herrera, Tom Lane, Isaac Morland, David G. Johnston Reviewed-by: Zhihong Yu, Alexander Korotkov
Tom Lane [Sat, 19 Dec 2020 22:58:30 +0000 (17:58 -0500)]
Remove now-useless ALWAYS_SUBDIRS entry in src/test/Makefile.
Commit 257836a75 added the "locale" subdirectory to SUBDIRS,
but neglected to remove it from ALWAYS_SUBDIRS. This oversight
had no functional effect because the filter-out function would
remove it anyway. Still, it's confusing to readers to list a
subdirectory in both places, especially because it makes the
associated comment into a partial lie.
Amit Kapila [Sat, 19 Dec 2020 04:38:46 +0000 (10:08 +0530)]
Update comment atop of ReorderBufferQueueMessage().
The comments atop of this function describes behaviour in case of a
transactional WAL message only, but it accepts both transactional and
non-transactional WAL messages. Update the comments to describe
behaviour in case of non-transactional WAL message as well.
Ashutosh Bapat, rephrased by Amit Kapila
Discussion: https://postgr.es/m/CAGEoWWTTzNzHOi8bj0wfAo1siGi-YEh6wqH1oaz4DrkTJ6HbTQ@mail.gmail.com
Tom Lane [Fri, 18 Dec 2020 21:24:49 +0000 (16:24 -0500)]
Add a couple of missed .gitignore entries.
Any subdirectory that's ignoring /output_iso/ should also
ignore /tmp_check_iso/, which could be left behind by a
failed pg_isolation_regress_check run.
I think these have been wrong for awhile, but it doesn't
seem important to fix in back branches.
Tom Lane [Fri, 18 Dec 2020 20:46:44 +0000 (15:46 -0500)]
Avoid memcpy() with same source and destination during relmapper init.
A narrow reading of the C standard says that memcpy(x,x,n) is undefined,
although it's hard to envision an implementation that would really
misbehave. However, analysis tools such as valgrind might whine about
this; accordingly, let's band-aid relmapper.c to not do it.
See also 5b630501e, d3f4e8a8a, ad7b48ea0, and other similar fixes.
Apparently, none of those folk tried valgrinding initdb? This has been
like this for long enough that I'm surprised it hasn't been reported
before.
Back-patch, just in case anybody wants to use a back branch on a platform
that complains about this; we back-patched those earlier fixes too.
Fujii Masao [Fri, 18 Dec 2020 01:49:58 +0000 (10:49 +0900)]
pg_stat_statements: Track time at which all statistics were last reset.
This commit adds "stats_reset" column into the pg_stat_statements_info
view. This column indicates the time at which all statistics in the
pg_stat_statements view were last reset.
Per discussion, this commit also changes pg_stat_statements_info code
so that "dealloc" column is reset at the same time as "stats_reset" is reset,
i.e., whenever all pg_stat_statements entries are removed, for the sake
of consistency. Previously "dealloc" was reset only when
pg_stat_statements_reset(0, 0, 0) is called and was not reset when
pg_stat_statements_reset() with non-zero value argument discards all
entries. This was confusing.
Fujii Masao [Thu, 17 Dec 2020 09:06:51 +0000 (18:06 +0900)]
Revert "Get rid of the dedicated latch for signaling the startup process".
Revert ac22929a26, as well as the followup fix 113d3591b8. Because it broke
the assumption that the startup process waiting for the recovery conflict
on buffer pin should be waken up only by buffer unpin or the timeout enabled
in ResolveRecoveryConflictWithBufferPin(). It caused, for example,
SIGHUP signal handler or walreceiver process to wake that startup process
up unnecessarily frequently.
Additionally, add the comments about why that dedicated latch that
the reverted patch tried to get rid of should not be removed.
Peter Geoghegan [Tue, 15 Dec 2020 23:55:07 +0000 (15:55 -0800)]
Remove obsolete btrescan() comment.
"Ordering stuff" refered to a _bt_first() call to _bt_orderkeys().
However, the _bt_orderkeys() function was renamed to
_bt_preprocess_keys() by commit fa5c8a055a0.
_bt_preprocess_keys() is directly referenced just after the removed
comment already, which seems sufficient.
Tomas Vondra [Tue, 15 Dec 2020 22:13:38 +0000 (23:13 +0100)]
Error out when Gather Merge input is not sorted
To build Gather Merge path, the input needs to be sufficiently sorted.
Ensuring this is the responsibility of the code constructing the paths,
but create_gather_merge_plan tried to handle unsorted paths by adding
an explicit Sort. In light of the recent issues related to Incremental
Sort, this is rather fragile. Some of the expressions may be volatile
or parallel unsafe, in which case we can't add the Sort here.
We could do more checks and add the Sort in at least some cases, but
it seems cleaner to just error out and make it clear this is a bug in
code constructing those paths.
Author: James Coleman Reviewed-by: Tomas Vondra
Discussion: https://postgr.es/m/CAAaqYe8cK3g5CfLC4w7bs%3DhC0mSksZC%3DH5M8LSchj5e5OxpTAg%40mail.gmail.com
Discussion: https://postgr.es/m/CAJGNTeNaxpXgBVcRhJX%2B2vSbq%2BF2kJqGBcvompmpvXb7pq%2BoFA%40mail.gmail.com
Peter Eisentraut [Tue, 15 Dec 2020 20:54:06 +0000 (21:54 +0100)]
Clean up ancient test style
Many older tests where written in a style like
SELECT '' AS two, i.* FROM INT2_TBL
where the first column indicated the number of expected result rows.
This has gotten increasingly out of date, as the test data fixtures
have expanded, so a lot of these were wrong and misleading. Moreover,
this style isn't really necessary, since the psql output already shows
the number of result rows.
Tom Lane [Tue, 15 Dec 2020 16:38:53 +0000 (11:38 -0500)]
Improve hash_create()'s API for some added robustness.
Invent a new flag bit HASH_STRINGS to specify C-string hashing, which
was formerly the default; and add assertions insisting that exactly
one of the bits HASH_STRINGS, HASH_BLOBS, and HASH_FUNCTION be set.
This is in hopes of preventing recurrences of the type of oversight
fixed in commit a1b8aa1e4 (i.e., mistakenly omitting HASH_BLOBS).
Also, when HASH_STRINGS is specified, insist that the keysize be
more than 8 bytes. This is a heuristic, but it should catch
accidental use of HASH_STRINGS for integer or pointer keys.
(Nearly all existing use-cases set the keysize to NAMEDATALEN or
more, so there's little reason to think this restriction should
be problematic.)
Tweak hash_create() to insist that the HASH_ELEM flag be set, and
remove the defaults it had for keysize and entrysize. Since those
defaults were undocumented and basically useless, no callers
omitted HASH_ELEM anyway.
Also, remove memset's zeroing the HASHCTL parameter struct from
those callers that had one. This has never been really necessary,
and while it wasn't a bad coding convention it was confusing that
some callers did it and some did not. We might as well save a few
cycles by standardizing on "not".
Also improve the documentation for hash_create().
In passing, improve reinit.c's usage of a hash table by storing
the key as a binary Oid rather than a string; and, since that's
a temporary hash table, allocate it in CurrentMemoryContext for
neatness.
Michael Paquier [Mon, 14 Dec 2020 03:38:13 +0000 (12:38 +0900)]
Improve some code around cryptohash functions
This adjusts some code related to recent changes for cryptohash
functions:
- Add a variable in md5.h to track down the size of a computed result,
moved from pgcrypto. Note that pg_md5_hash() assumed a result of this
size already.
- Call explicit_bzero() on the hashed data when freeing the context for
fallback implementations. For MD5, particularly, it would be annoying
to leave some non-zeroed data around.
- Clean up some code related to recent changes of uuid-ossp. .gitignore
still included md5.c and a comment was incorrect.
Michael Paquier [Mon, 14 Dec 2020 02:53:58 +0000 (11:53 +0900)]
Add some checkpoint/restartpoint status to ps display
This is done for end-of-recovery and shutdown checkpoints/restartpoints
(end-of-recovery restartpoints don't exist) rather than all types of
checkpoints, in cases where it may not be possible to rely on
pg_stat_activity to get a status from the startup or checkpointer
processes.
For example, at the end of a crash recovery, this is useful to know if a
checkpoint is running in the startup process, while previously the ps
display may only show some information about "recovering" something,
that can be confusing while a checkpoint runs.
Noah Misch [Sun, 13 Dec 2020 05:38:36 +0000 (21:38 -0800)]
Use HASH_BLOBS for xidhash.
This caused BufFile errors on buildfarm member sungazer, and SIGSEGV was
possible. Conditions for reaching those symptoms were more frequent on
big-endian systems.
Tom Lane [Fri, 11 Dec 2020 23:58:07 +0000 (18:58 -0500)]
Allow subscripting of hstore values.
This is basically a finger exercise to prove that it's possible for
an extension module to add subscripting ability. Subscripted fetch
from an hstore is not different from the existing "hstore -> text"
operator. Subscripted update does seem to be a little easier to
use than the traditional update method using hstore concatenation,
but it's not a fundamentally new ability.
However, there may be some value in the code as sample code, since
it shows what's basically the minimum-complexity way to implement
subscripting when one needn't consider nested container objects.
Tom Lane [Fri, 11 Dec 2020 23:07:02 +0000 (18:07 -0500)]
Allow ALTER TYPE to update an existing type's typsubscript value.
This is essential if we'd like to allow existing extension data types
to support subscripting in future, since dropping and recreating the
type isn't a practical thing for an extension upgrade script, and
direct manipulation of pg_type isn't a great answer either.
There was some discussion about also allowing alteration of typelem,
but it's less clear whether that's a good idea or not, so for now
I forebore.
Tom Lane [Fri, 11 Dec 2020 22:54:10 +0000 (17:54 -0500)]
Provide an error cursor for "can't subscript" error messages.
Commit c7aba7c14 didn't add this, but after more fooling with the
feature I feel that it'd be useful. To make this possible, refactor
getSubscriptingRoutines() so that the caller is responsible for
throwing any error. (In clauses.c, I just chose to make the
most conservative assumption rather than throwing an error. We don't
expect failures there anyway really, so the code space for an error
message would be a poor investment.)
Michael Paquier [Thu, 10 Dec 2020 03:49:43 +0000 (12:49 +0900)]
Fix compilation of uuid-ossp
This module had a dependency on pgcrypto's md5.c that got removed by b67b57a. Instead of the code from pgcrypto, this code can just use the
new cryptohash routines for MD5 as a drop-in replacement, so let's just
do this switch. This has also the merit to simplify a bit the
compilation of uuid-ossp.
This requires --with-uuid to be reproduced, and I have used e2fs as a
way to reproduce the failure, then test this commit.
Per reports from buildfarm members longfin, florican and sifaka.
Michael Paquier [Thu, 10 Dec 2020 02:59:10 +0000 (11:59 +0900)]
Refactor MD5 implementations according to new cryptohash infrastructure
This commit heavily reorganizes the MD5 implementations that exist in
the tree in various aspects.
First, MD5 is added to the list of options available in cryptohash.c and
cryptohash_openssl.c. This means that if building with OpenSSL, EVP is
used for MD5 instead of the fallback implementation that Postgres had
for ages. With the recent refactoring work for cryptohash functions,
this change is straight-forward. If not building with OpenSSL, a
fallback implementation internal to src/common/ is used.
Second, this reduces the number of MD5 implementations present in the
tree from two to one, by moving the KAME implementation from pgcrypto to
src/common/, and by removing the implementation that existed in
src/common/. KAME was already structured with an init/update/final set
of routines by pgcrypto (see original pgcrypto/md5.h) for compatibility
with OpenSSL, so moving it to src/common/ has proved to be a
straight-forward move, requiring no actual manipulation of the internals
of each routine. Some benchmarking has not shown any performance gap
between both implementations.
Similarly to the fallback implementation used for SHA2, the fallback
implementation of MD5 is moved to src/common/md5.c with an internal
header called md5_int.h for the init, update and final routines. This
gets then consumed by cryptohash.c.
The original routines used for MD5-hashed passwords are moved to a
separate file called md5_common.c, also in src/common/, aimed at being
shared between all MD5 implementations as utility routines to keep
compatibility with any code relying on them.
Like the SHA2 changes, this commit had its round of tests on both Linux
and Windows, across all versions of OpenSSL supported on HEAD, with and
even without OpenSSL.
Author: Michael Paquier Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/20201106073434.GA4961@paquier.xyz
projects / thirdparty / postgresql.git / log
