Tom Lane [Sun, 29 Nov 2020 18:58:30 +0000 (13:58 -0500)]
Doc: clarify behavior of PQconnectdbParams().
The documentation omitted the critical tidbit that a keyword-array entry
is simply ignored if its corresponding value-array entry is NULL or an
empty string; it will *not* override any previously-obtained value for
the parameter. (See conninfo_array_parse().) I'd supposed that would
force the setting back to default, which is what led me into bug #16746;
but it doesn't.
While here, I couldn't resist the temptation to do some copy-editing,
both in the description of PQconnectdbParams() and in the section
about connection URI syntax.
Discussion: https://postgr.es/m/931505.
1606618746@sss.pgh.pa.us
Noah Misch [Sun, 29 Nov 2020 05:52:27 +0000 (21:52 -0800)]
Retry initial slurp_file("current_logfiles"), in test 004_logrotate.pl.
Buildfarm member topminnow failed when the test script attempted this
before the syslogger would have created the file. Back-patch to v12,
which introduced the test.
Tom Lane [Sat, 28 Nov 2020 21:08:36 +0000 (16:08 -0500)]
Clean up after tests in src/test/locale/.
Oversight in
257836a75, which added these tests.
Tom Lane [Sat, 28 Nov 2020 19:03:40 +0000 (14:03 -0500)]
Fix a recently-introduced race condition in LISTEN/NOTIFY handling.
Commit
566372b3d fixed some race conditions involving concurrent
SimpleLruTruncate calls, but it introduced new ones in async.c.
A newly-listening backend could attempt to read Notify SLRU pages that
were in process of being truncated, possibly causing an error. Also,
the QUEUE_TAIL pointer could become set to a value that's not equal to
the queue position of any backend. While that's fairly harmless in
v13 and up (thanks to commit
51004c717), in older branches it resulted
in near-permanent disabling of the queue truncation logic, so that
continued use of NOTIFY led to queue-fill warnings and eventual
inability to send any more notifies. (A server restart is enough to
make that go away, but it's still pretty unpleasant.)
The core of the problem is confusion about whether QUEUE_TAIL
represents the "logical" tail of the queue (i.e., the oldest
still-interesting data) or the "physical" tail (the oldest data we've
not yet truncated away). To fix, split that into two variables.
QUEUE_TAIL regains its definition as the logical tail, and we
introduce a new variable to track the oldest un-truncated page.
Per report from Mikael Gustavsson. Like the previous patch,
back-patch to all supported branches.
Discussion: https://postgr.es/m/
1b8561412e8a4f038d7a491c8b922788@smhi.se
Fujii Masao [Fri, 27 Nov 2020 11:16:44 +0000 (20:16 +0900)]
Fix CLUSTER progress reporting of number of blocks scanned.
Previously pg_stat_progress_cluster view reported the current block
number in heap scan as the number of heap blocks scanned (i.e.,
heap_blks_scanned). This reported number could be incorrect when
synchronize_seqscans is enabled, because it allowed the heap scan to
start at block in middle. This could result in wraparounds in the
heap_blks_scanned column when the heap scan wrapped around.
This commit fixes the bug by calculating the number of blocks from
the block that the heap scan starts at to the current block in scan,
and reporting that number in the heap_blks_scanned column.
Also, in pg_stat_progress_cluster view, previously heap_blks_scanned
could not reach heap_blks_total at the end of heap scan phase
if the last pages scanned were empty. This commit fixes the bug by
manually updating heap_blks_scanned to the same value as
heap_blks_total when the heap scan phase finishes.
Back-patch to v12 where pg_stat_progress_cluster view was introduced.
Reported-by: Matthias van de Meent
Author: Matthias van de Meent
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/CAEze2WjCBWSGkVfYag001Rc4+-nNLDpWM7QbyD6yPvuhKs-gYQ@mail.gmail.com
Fujii Masao [Fri, 27 Nov 2020 06:45:01 +0000 (15:45 +0900)]
Use standard SIGTERM signal handler die() in test_shm_mq worker.
Previously test_shm_mq worker used the stripped-down version of die()
as the SIGTERM signal handler. This commit makes it use die(), instead,
to simplify the code.
In terms of the code, the difference between die() and the stripped-down
version previously used is whether the signal handler directly may call
ProcessInterrupts() or not. But this difference doesn't exist in
a background worker because, in bgworker, DoingCommandRead flag will
never be true and die() will never call ProcessInterrupts() directly.
Therefore test_shm_mq worker can safely use die(), like other bgworker
proceses (e.g., logical replication apply launcher or autoprewarm worker)
currently do.
Thanks to Craig Ringer for the report and investigation of the issue.
Author: Bharath Rupireddy
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/CAGRY4nxsAe_1k_9g5b47orA0S011iBoHsXHFMH7cg7HV0O1bwQ@mail.gmail.com
Fujii Masao [Fri, 27 Nov 2020 06:11:19 +0000 (15:11 +0900)]
Use standard SIGHUP and SIGTERM signal handlers in worker_spi.
Previously worker_spi used its custom signal handlers for SIGHUP and
SIGTERM. This commit makes worker_spi use the standard signal handlers,
to simplify the code.
Note that die() is used as the standard SIGTERM signal handler in
worker_spi instead of SignalHandlerForShutdownRequest() or bgworker_die().
Previously the exit handling was only able to exit from within the main loop,
and not from within the backend code it calls. This is why die() needs to
be used here, so worker_spi can respond to SIGTERM signal while it's
executing a query.
Maybe we can say that it's a bug that worker_spi could not respond to
SIGTERM during query execution. But since worker_spi is a just example
of the background worker code, we don't do the back-patch.
Thanks to Craig Ringer for the report and investigation of the issue.
Author: Bharath Rupireddy
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/CALj2ACXDEZhAFOTDcqO9cFSRvrFLyYOnPKrcA1UG4uZn9hUAVg@mail.gmail.com
Discussion: https://postgr.es/m/CAGRY4nxsAe_1k_9g5b47orA0S011iBoHsXHFMH7cg7HV0O1bwQ@mail.gmail.com
Amit Kapila [Fri, 27 Nov 2020 02:13:34 +0000 (07:43 +0530)]
Fix replication of in-progress transactions in tablesync worker.
Tablesync worker runs under a single transaction but in streaming mode, we
were committing the transaction on stream_stop, stream_abort, and
stream_commit. We need to avoid committing the transaction in a streaming
mode in tablesync worker.
In passing move the call to process_syncing_tables in
apply_handle_stream_commit after clean up of stream files. This will
allow clean up of files to happen before the exit of tablesync worker
which would otherwise be handled by one of the proc exit routines.
Author: Dilip Kumar
Reviewed-by: Amit Kapila and Peter Smith
Tested-by: Peter Smith
Discussion: https://postgr.es/m/CAHut+Pt4PyKQCwqzQ=EFF=bpKKJD7XKt_S23F6L20ayQNxg77A@mail.gmail.com
Alvaro Herrera [Thu, 26 Nov 2020 15:30:48 +0000 (12:30 -0300)]
Restore lock level to update statusFlags
Reverts
27838981be9d (some comments are kept). Per discussion, it does
not seem safe to relax the lock level used for this; in order for it to
be safe, there would have to be memory barriers between the point we set
the flag and the point we set the trasaction Xid, which perhaps would
not be so bad; but there would also have to be barriers at the readers'
side, which from a performance perspective might be bad.
Now maybe this analysis is wrong and it *is* safe for some reason, but
proof of that is not trivial.
Discussion: https://postgr.es/m/
20201118190928.vnztes7c2sldu43a@alap3.anarazel.de
Fujii Masao [Thu, 26 Nov 2020 12:18:05 +0000 (21:18 +0900)]
pg_stat_statements: Track number of times pgss entries were deallocated.
If more distinct statements than pg_stat_statements.max are observed,
pg_stat_statements entries about the least-executed statements are
deallocated. This commit enables us to track the total number of times
those entries were deallocated. That number can be viewed in the
pg_stat_statements_info view that this commit adds. It's useful when
tuning pg_stat_statements.max parameter. If it's high, i.e., the entries
are deallocated very frequently, which might cause the performance
regression and we can increase pg_stat_statements.max to avoid those
frequent deallocations.
The pg_stat_statements_info view is intended to display the statistics
of pg_stat_statements module itself. Currently it has only one column
"dealloc" indicating the number of times entries were deallocated.
But an upcoming patch will add other columns (for example, the time
at which pg_stat_statements statistics were last reset) into the view.
Author: Katsuragi Yuta, Yuki Seino
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/
0d9f1107772cf5c3f954e985464c7298@oss.nttdata.com
Fujii Masao [Thu, 26 Nov 2020 07:17:10 +0000 (16:17 +0900)]
doc: Add description about re-analysis and re-planning of a prepared statement.
A prepared statement is re-analyzed and re-planned whenever database
objects used in the statement have undergone definitional changes or
the planner statistics of them have been updated. The former has been
documented from before, but the latter was not previously. This commit
adds the description about the latter case into the docs.
Author: Atsushi Torikoshi
Reviewed-by: Andy Fan, Fujii Masao
Discussion: https://postgr.es/m/
3ac82f4817c9fe274a905c8a38d87bd9@oss.nttdata.com
Amit Kapila [Thu, 26 Nov 2020 03:51:14 +0000 (09:21 +0530)]
Use Enums for logical replication message types at more places.
Commit
644f0d7cc9 added logical replication message type enums to use
instead of character literals but some char substitutions were overlooked.
Author: Peter Smith
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/CAHut+PsTG=Vrv8hgrvOnAvCNR21jhqMdPk2n0a1uJPoW0p+UfQ@mail.gmail.com
Alvaro Herrera [Wed, 25 Nov 2020 21:21:08 +0000 (18:21 -0300)]
Avoid spurious waits in concurrent indexing
In the various waiting phases of CREATE INDEX CONCURRENTLY (CIC) and
REINDEX CONCURRENTLY (RC), we wait for other processes to release their
snapshots; this is necessary in general for correctness. However,
processes doing CIC in other tables cannot possibly affect CIC or RC
done in "this" table, so we don't need to wait for those. This commit
adds a flag in MyProc->statusFlags to indicate that the current process
is doing CIC, so that other processes doing CIC or RC can ignore it when
waiting.
Note that this logic is only valid if the index does not access other
tables. For simplicity we avoid setting the flag if the index has a
column that's an expression, or has a WHERE predicate. (It is possible
to have expressional or partial indexes that do not access other tables,
but figuring that out would require more work.)
This flag can potentially also be used by processes doing REINDEX
CONCURRENTLY to be skipped; and by VACUUM to ignore processes in CIC or
RC for the purposes of computing an Xmin. That's left for future
commits.
Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Author: Dimitry Dolgov <9erthalion6@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/
20200810233815.GA18970@alvherre.pgsql
Tom Lane [Wed, 25 Nov 2020 21:19:25 +0000 (16:19 -0500)]
In psql's \d commands, don't truncate attribute default values.
Historically, psql has truncated the text of a column's default
expression at 128 characters. This is unlike any other behavior
in describe.c, and it's become particularly confusing now that
the limit is only applied to the expression proper and not to
the "generated always as (...) stored" text that may get wrapped
around it.
Excavation in our git history suggests that the original motivation
for this limit was not really to limit the display width (as I'd long
supposed), but to make it safe to use a fixed-width output buffer to
store the result. That implementation restriction is long gone of
course, but the limit remained. Let's just get rid of it.
While here, rearrange the logic about when to free the output string
so that it's not so dependent on unstated assumptions about the
possible values of attidentity and attgenerated.
Per bug #16743 from David Turon. Back-patch to v12 where GENERATED
came in. (Arguably we could take it back further, but I'm hesitant
to change the behavior of long-stable branches for this.)
Discussion: https://postgr.es/m/16743-
7b1bacc4af76e7ad@postgresql.org
Tom Lane [Wed, 25 Nov 2020 19:04:28 +0000 (14:04 -0500)]
Doc: minor improvements for section 11.2 "Index Types".
Break the per-index-type discussions into <sect2>'s so as to make
them more visually separate and easier to find. Improve the markup,
and make a couple of small wording adjustments.
This also fixes one stray reference to the now-deprecated point
operators <^ and >^.
Dagfinn Ilmari Mannsåker, reviewed by David Johnston and Jürgen Purtz
Discussion: https://postgr.es/m/877dukhvzg.fsf@wibble.ilmari.org
Tom Lane [Wed, 25 Nov 2020 16:40:44 +0000 (11:40 -0500)]
Avoid spamming the client with multiple ParameterStatus messages.
Up to now, we sent a ParameterStatus message to the client immediately
upon any change in the active value of any GUC_REPORT variable. This
was only barely okay when the feature was designed; now that we have
things like function SET clauses, there are very plausible use-cases
where a GUC_REPORT variable might change many times within a query
--- and even end up back at its original value, perhaps. Fortunately
most of our GUC_REPORT variables are unlikely to be changed often;
but there are proposals in play to enlarge that set, or even make it
user-configurable.
Hence, let's fix things to not generate more than one ParameterStatus
message per variable per query, and to not send any message at all
unless the end-of-query value is different from what we last reported.
Discussion: https://postgr.es/m/5708.
1601145259@sss.pgh.pa.us
Peter Eisentraut [Wed, 25 Nov 2020 14:30:18 +0000 (15:30 +0100)]
tablefunc: Reject negative number of tuples passed to normal_rand()
The function converted the first argument i.e. the number of tuples to
return into an unsigned integer which turns out to be huge number when
a negative value is passed. This causes the function to take much
longer time to execute. Instead, reject a negative value.
(If someone really wants to generate many more result rows, they
should consider adding a bigint or numeric variant.)
While at it, improve SQL test to test the number of tuples returned by
this function.
Author: Ashutosh Bapat <ashutosh.bapat@2ndquadrant.com>
Discussion: https://www.postgresql.org/message-id/CAG-ACPW3PUUmSnM6cLa9Rw4BEC5cEMKjX8Gogc8gvQcT3cYA1A@mail.gmail.com
Peter Eisentraut [Wed, 25 Nov 2020 08:49:00 +0000 (09:49 +0100)]
doc: Fix typos
Author: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://www.postgresql.org/message-id/
20201121194105.GO24784@telsasoft.com
Peter Eisentraut [Wed, 25 Nov 2020 07:14:23 +0000 (08:14 +0100)]
Make error hint from bind() failure more accurate
The hint "Is another postmaster already running ..." should only be
printed for errors that are really about something else already using
the address. In other cases it is misleading. So only show that hint
if errno == EADDRINUSE.
Also, since Unix-domain sockets in the file-system namespace never
report EADDRINUSE for an existing file (they would just overwrite it),
the part of the hint saying "If not, remove socket file \"%s\" and
retry." can never happen, so remove it. Unix-domain sockets in the
abstract namespace can report EADDRINUSE, but in that case there is no
file to remove, so the hint doesn't work there either.
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/flat/
6dee8574-b0ad-fc49-9c8c-
2edc796f0033@2ndquadrant.com
Peter Eisentraut [Wed, 25 Nov 2020 07:14:23 +0000 (08:14 +0100)]
Add support for abstract Unix-domain sockets
This is a variant of the normal Unix-domain sockets that don't use the
file system but a separate "abstract" namespace. At the user
interface, such sockets are represented by names starting with "@".
Supported on Linux and Windows right now.
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/flat/
6dee8574-b0ad-fc49-9c8c-
2edc796f0033@2ndquadrant.com
Thomas Munro [Wed, 25 Nov 2020 04:44:15 +0000 (17:44 +1300)]
Fix WaitLatch(NULL) on Windows.
Further to commit
733fa9aa, on Windows when a latch is triggered but we
aren't currently waiting for it, we need to locate the latch's HANDLE
rather than calling ResetEvent(NULL).
Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reported-by: Ranier Vilela <ranier.vf@gmail.com>
Discussion: https://postgr.es/m/CAEudQArTPi1YBc%2Bn1fo0Asy3QBFhVjp_QgyKG-8yksVn%2ByRTiw%40mail.gmail.com
Amit Kapila [Wed, 25 Nov 2020 03:44:45 +0000 (09:14 +0530)]
Remove obsolete comment atop ri_PlanCheck.
Commit
5b7ba75f7f removed the unused parameter but forgot to update the
nearby comments.
Author: Li Japin
Backpatch-through: 13, where it was introduced
Discussion: https://postgr.es/m/
0E2F62A2-B2F1-4052-83AE-
F0BEC8A75789@hotmail.com
David Rowley [Wed, 25 Nov 2020 03:33:43 +0000 (16:33 +1300)]
Stop gap fix for __attribute__((cold)) compiler bug in MinGW 8.1
The buildfarm animal walleye, running MinGW 8.1 has been having problems
ever since
697e1d02f and
913ec71d6 went in. This appears to be a bug in
assembler which was fixed in a later version.
For now, in order to get that animal running green again, let's just
define pg_attribute_cold and pg_attribute_hot to be empty macros on that
compiler. Hopefully, we can get the support of the owner of the animal to
upgrade to a less buggy compiler and revert this at a later date.
Discussion: https://postgr.es/m/286560.
1606233316@sss.pgh.pa.us
Michael Paquier [Wed, 25 Nov 2020 03:18:26 +0000 (12:18 +0900)]
Remove catalog function currtid()
currtid() and currtid2() are an undocumented set of functions whose sole
known user is the Postgres ODBC driver, able to retrieve the latest TID
version for a tuple given by the caller of those functions.
As used by Postgres ODBC, currtid() is a shortcut able to retrieve the
last TID loaded into a backend by passing an OID of 0 (magic value)
after a tuple insertion. This is removed in this commit, as it became
obsolete after the driver began using "RETURNING ctid" with inserts, a
clause supported since Postgres 8.2 (using RETURNING is better for
performance anyway as it reduces the number of round-trips to the
backend).
currtid2() is still used by the driver, so this remains around for now.
Note that this function is kept in its original shape for backward
compatibility reasons.
Per discussion with many people, including Andres Freund, Peter
Eisentraut, Álvaro Herrera, Hiroshi Inoue, Tom Lane and myself.
Bump catalog version.
Discussion: https://postgr.es/m/
20200603021448.GB89559@paquier.xyz
Andrew Gierth [Tue, 24 Nov 2020 20:58:32 +0000 (20:58 +0000)]
Properly check index mark/restore in ExecSupportsMarkRestore.
Previously this code assumed that all IndexScan nodes supported
mark/restore, which is not true since it depends on optional index AM
support functions. This could lead to errors about missing support
functions in rare edge cases of mergejoins with no sort keys, where an
unordered non-btree index scan was placed on the inner path without a
protecting Materialize node. (Normally, the fact that merge join
requires ordered input would avoid this error.)
Backpatch all the way since this bug is ancient.
Per report from Eugen Konkov on irc.
Discussion: https://postgr.es/m/87o8jn50be.fsf@news-spur.riddles.org.uk
David Rowley [Tue, 24 Nov 2020 21:52:50 +0000 (10:52 +1300)]
Tidy up definitions of pg_attribute_hot and pg_attribute_cold
1fa22a43a was a quick fix for portability problem I introduced in
697e1d02f.
1fa22a43a adds a few more cases to the preprocessor logic than
I'd have liked. Andres Freund and Dagfinn Ilmari Mannsåker suggested a
better way to do this.
In passing, also adjust the only current usage of these macros so that the
macro comes before the function's return type in the declaration of the
function. This now matches what the definition of the function does.
Discussion: https://postgr.es/m/
20200625163553.lt6wocbjhklp5pl4@alap3.anarazel.de
Discussion: https://postgr.es/m/87pn43bmok.fsf@wibble.ilmari.org
Tom Lane [Tue, 24 Nov 2020 20:43:01 +0000 (15:43 -0500)]
Put "inline" marker on declarations of inline functions.
I'm having a hard time telling whether the letter of the C standard
requires this, but we do have a couple of buildfarm members that
throw warnings when this is not done. Oversight in
c532d15dd.
Heikki Linnakangas [Tue, 24 Nov 2020 10:50:16 +0000 (12:50 +0200)]
Fix expected output: the order of agg permission checks changed.
Commit
0a2bc5d61e changed the order that permissions on the final and
transition functions of an aggregate are checked in. That shows up as a
difference in the order the LOG messages in this sepgsql regression test
are printed. Adjust the expected output.
Per buildfarm failure in rhinoceros.
Heikki Linnakangas [Tue, 24 Nov 2020 08:45:00 +0000 (10:45 +0200)]
Move per-agg and per-trans duplicate finding to the planner.
This has the advantage that the cost estimates for aggregates can count
the number of calls to transition and final functions correctly.
Bump catalog version, because views can contain Aggrefs.
Reviewed-by: Andres Freund
Discussion: https://www.postgresql.org/message-id/
b2e3536b-1dbc-8303-c97e-
89cb0b4a9a48%40iki.fi
Fujii Masao [Tue, 24 Nov 2020 08:00:16 +0000 (17:00 +0900)]
doc: Get rid of unnecessary space character from some index items.
Previously some index items have " ," (i.e., space + comma) in the docs
as follows. Since the space character before the comma is unnecessary,
this commit gets rid of that for the sake of consistency with other
index items.
parallel_leader_participation configuration parameter , Other Planner Options
Author: Fujii Masao
Reviewed-by: Euler Taveira
Discussion: https://postgr.es/m/
e87b4ddf-1498-2850-bf55-
519df3928fd4@oss.nttdata.com
Michael Paquier [Tue, 24 Nov 2020 03:39:58 +0000 (12:39 +0900)]
Use macros instead of hardcoded offsets for LWLock initialization
This makes the code slightly easier to follow, as the initialization
relies on an offset that overlapped with an equivalent set of macros
defined, which are used in other places already.
Author: Japin Li
Discussion: https://postgr.es/m/MEYP282MB1669FB410006758402F2C3A2B6E00@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM
Tom Lane [Tue, 24 Nov 2020 00:04:07 +0000 (19:04 -0500)]
Centralize logic for skipping useless ereport/elog calls.
While ereport() and elog() themselves are quite cheap when the
error message level is too low to be printed, some places need to do
substantial work before they can call those macros at all. To allow
optimizing away such setup work when nothing is to be printed, make
elog.c export a new function message_level_is_interesting(elevel)
that reports whether ereport/elog will do anything. Make use of that
in various places that had ad-hoc direct tests of log_min_messages etc.
Also teach ProcSleep to use it to avoid some work. (There may well
be other places that could usefully use this; I didn't search hard.)
Within elog.c, refactor a little bit to avoid having duplicate copies
of the policy-setting logic. When that code was written, we weren't
relying on the availability of inline functions; so it had some
duplications in the name of efficiency, which I got rid of.
Alvaro Herrera and Tom Lane
Discussion: https://postgr.es/m/129515.
1606166429@sss.pgh.pa.us
David Rowley [Tue, 24 Nov 2020 00:09:35 +0000 (13:09 +1300)]
Fix unportable usage of __has_attribute
This should fix the breakages caused by
697e1d02f, which seems to break
the build for GCC version < 5.
It seems, according to the GCC manual that __has_attribute is a "special
operator" and must be tested without any other conditions in the
preprocessor test.
Per recommendation from the GCC manual via Greg Nancarrow
Reported-by: Greg Nancarrow
Discussion: https://postgr.es/m/CAJcOf-euSu8fhC10v476o9dqnjqKysVs1_vRms-_fvajpZ3kFw@mail.gmail.com
David Rowley [Mon, 23 Nov 2020 23:04:42 +0000 (12:04 +1300)]
Improve compiler code layout in elog/ereport ERROR calls
Here we use a bit of preprocessor trickery to coax supporting compilers
into laying out their generated code so that the code that's in the same
branch as elog(ERROR)/ereport(ERROR) calls is moved away from the hot
path. Effectively, this reduces the size of the hot code meaning that it
can sit on fewer cache lines.
Performance improvements of between 10-15% have been seen on highly CPU
bound workloads using pgbench's TPC-b benchmark.
What's achieved here is very similar to putting the error condition inside
an unlikely() macro. For example;
if (unlikely(x < 0))
elog(ERROR, "invalid x value");
now there's no need to make use of unlikely() here as the common macro
used by elog and ereport will now see that elevel is >= ERROR and make use
of a pg_attribute_cold marked version of errstart().
When elevel < ERROR or if it cannot be determined to be constant, the
original behavior is maintained.
Author: David Rowley
Reviewed-by: Andres Freund, Peter Eisentraut
Discussion: https://postgr.es/m/CAApHDvrVpasrEzLL2er7p9iwZFZ%3DJj6WisePcFeunwfrV0js_A%40mail.gmail.com
David Rowley [Mon, 23 Nov 2020 22:29:28 +0000 (11:29 +1300)]
Define pg_attribute_cold and pg_attribute_hot macros
For compilers supporting __has_attribute and __has_attribute (hot/cold).
__has_attribute is supported on gcc >= 5, clang >= 2.9 and icc >= 17.
A followup commit will implement some usages of these macros.
Author: David Rowley
Reviewed-by: Andres Freund, Peter Eisentraut
Discussion: https://postgr.es/m/CAApHDvrVpasrEzLL2er7p9iwZFZ%3DJj6WisePcFeunwfrV0js_A%40mail.gmail.com
Tom Lane [Mon, 23 Nov 2020 22:00:01 +0000 (17:00 -0500)]
Remove unnecessary #include.
Justin Pryzby
Discussion: https://postgr.es/m/
20201123205505.GJ24052@telsasoft.com
Alvaro Herrera [Mon, 23 Nov 2020 21:55:23 +0000 (18:55 -0300)]
Don't hold ProcArrayLock longer than needed in rare cases
While cancelling an autovacuum worker, we hold ProcArrayLock while
formatting a debugging log string. We can make this shorter by saving
the data we need to produce the message and doing the formatting outside
the locked region.
This isn't terribly critical, as it only occurs pretty rarely: when a
backend runs deadlock detection and it happens to be blocked by a
autovacuum running autovacuum. Still, there's no need to cause a hiccup
in ProcArrayLock processing, which can be very high-traffic in some
cases.
While at it, rework code so that we only print the string when it is
really going to be used, as suggested by Michael Paquier.
Discussion: https://postgr.es/m/
20201118214127.GA3179@alvherre.pgsql
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Tom Lane [Mon, 23 Nov 2020 16:38:37 +0000 (11:38 -0500)]
Rename the "point is strictly above/below point" comparison operators.
Historically these were called >^ and <^, but that is inconsistent
with the similar box, polygon, and circle operators, which are named
|>> and <<| respectively. Worse, the >^ and <^ names are used for
*not* strict above/below tests for the box type.
Hence, invent new operators following the more common naming. The
old operators remain available for now, and are still accepted by
the relevant index opclasses too. But there's a deprecation notice,
so maybe we can get rid of them someday.
Emre Hasegeli, reviewed by Pavel Borisov
Discussion: https://postgr.es/m/24348.
1587444160@sss.pgh.pa.us
Tom Lane [Mon, 23 Nov 2020 16:15:03 +0000 (11:15 -0500)]
Improve wording of two error messages related to generated columns.
Clarify that you can "insert" into a generated column as long as what
you're inserting is a DEFAULT placeholder.
Also, use ERRCODE_GENERATED_ALWAYS in place of ERRCODE_SYNTAX_ERROR;
there doesn't seem to be any reason to use the less specific errcode.
Discussion: https://postgr.es/m/9q0sgcr416t.fsf@gmx.us
Alvaro Herrera [Mon, 23 Nov 2020 16:10:03 +0000 (13:10 -0300)]
Make some sanity-check elogs more verbose
A few sanity checks in funcapi.c were not mentioning all the possible
clauses for failure, confusing developers who fat-fingered catalog data
additions. Make the errors more detailed to avoid wasting time in
pinpointing mistakes.
Per complaint from Craig Ringer.
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAMsr+YH7Kd87A3cU5m_wKo46HPQ46zFv5wesFNL0YWxkGhGv3g@mail.gmail.com
Heikki Linnakangas [Mon, 23 Nov 2020 09:36:13 +0000 (11:36 +0200)]
Fix a few comments that referred to copy.c.
Missed these in the previous commit.
Heikki Linnakangas [Mon, 23 Nov 2020 08:50:50 +0000 (10:50 +0200)]
Split copy.c into four files.
Copy.c has grown really large. Split it into more manageable parts:
- copy.c now contains only a few functions that are common to COPY FROM
and COPY TO.
- copyto.c contains code for COPY TO.
- copyfrom.c contains code for initializing COPY FROM, and inserting the
tuples to the correct table.
- copyfromparse.c contains code for reading from the client/file/program,
and parsing the input text/CSV/binary format into tuples.
All of these parts are fairly complicated, and fairly independent of each
other. There is a patch being discussed to implement parallel COPY FROM,
which will add a lot of new code to the COPY FROM path, and another patch
which would allow INSERTs to use the same multi-insert machinery as COPY
FROM, both of which will require refactoring that code. With those two
patches, there's going to be a lot of code churn in copy.c anyway, so now
seems like a good time to do this refactoring.
The CopyStateData struct is also split. All the formatting options, like
FORMAT, QUOTE, ESCAPE, are put in a new CopyFormatOption struct, which
is used by both COPY FROM and TO. Other state data are kept in separate
CopyFromStateData and CopyToStateData structs.
Reviewed-by: Soumyadeep Chakraborty, Erik Rijkers, Vignesh C, Andres Freund
Discussion: https://www.postgresql.org/message-id/
8e15b560-f387-7acc-ac90-
763986617bfb%40iki.fi
Tom Lane [Sun, 22 Nov 2020 20:48:32 +0000 (15:48 -0500)]
Allow a multi-row INSERT to specify DEFAULTs for a generated column.
One can say "INSERT INTO tab(generated_col) VALUES (DEFAULT)" and not
draw an error. But the equivalent case for a multi-row VALUES list
always threw an error, even if one properly said DEFAULT in each row.
Fix that. While here, improve the test cases for nearby logic about
OVERRIDING SYSTEM/USER values.
Dean Rasheed
Discussion: https://postgr.es/m/9q0sgcr416t.fsf@gmx.us
Tom Lane [Sat, 21 Nov 2020 22:24:07 +0000 (17:24 -0500)]
In geo_ops.c, represent infinite slope as Infinity, not DBL_MAX.
Since we're assuming IEEE floats these days, there seems little
reason not to do this. It has the advantage that when the slope is
computed as infinite due to the presence of Inf coordinates, we get
saner behavior than before from line_construct(), and thence also
in some dependent operations such as finding the closest point.
Also fix line_construct() to special-case slope zero. The previous
coding got the right answer in most cases, but it could compute
C as NaN when the point has Inf coordinates.
Discussion: https://postgr.es/m/CAGf+fX70rWFOk5cd00uMfa__0yP+vtQg5ck7c2Onb-Yczp0URA@mail.gmail.com
Tom Lane [Sat, 21 Nov 2020 21:46:43 +0000 (16:46 -0500)]
Fix FPeq() and friends to get the right answers for infinities.
"FPeq(infinity, infinity)" returned false, on account of getting NaN
when it subtracts the two inputs. Fix that by adding a separate
check for exact equality.
FPle() and FPge() similarly got the wrong answer for two like-signed
infinities. In those cases, we can just rearrange the comparisons
to avoid potentially subtracting infinities.
While the sibling functions FPne() etc accidentally gave the right
answers even with the internal NaN results, it seems best to make
similar adjustments to them to avoid depending on this.
FPeq() has to be converted to an inline function to avoid double
evaluations of its arguments, and I did the same for the others
just for consistency.
In passing, make the handling of NaN cases in line_eq() and
point_eq_point() simpler and easier to reason about, and perhaps
faster.
This results in just one visible regression test change: slope()
now gives DBL_MAX for two inputs of (inf,1e300), which is consistent
with what it does for (1e300,inf), so that seems like a bug fix.
Discussion: https://postgr.es/m/CAGf+fX70rWFOk5cd00uMfa__0yP+vtQg5ck7c2Onb-Yczp0URA@mail.gmail.com
Tom Lane [Sat, 21 Nov 2020 21:34:22 +0000 (16:34 -0500)]
Extend the geometric regression test cases a little.
Add another edge-case value to "point_tbl", and add a test for
the line(point, point) function.
Some of the behaviors exposed here are wrong, but the idea of
committing this separately is to memorialize what we were getting,
and to allow easier inspection of the behavior changes caused by
upcoming patches.
Kyotaro Horiguchi (line() test added by me)
Discussion: https://postgr.es/m/CAGf+fX70rWFOk5cd00uMfa__0yP+vtQg5ck7c2Onb-Yczp0URA@mail.gmail.com
Michael Paquier [Sat, 21 Nov 2020 10:45:30 +0000 (19:45 +0900)]
Remove INSERT privilege check at table creation of CTAS and matview
As per discussion with Peter Eisentraunt, the SQL standard specifies
that any tuple insertion done as part of CREATE TABLE AS happens without
any extra ACL check, so it makes little sense to keep a check for INSERT
privileges when using WITH DATA. Materialized views are not part of the
standard, but similarly, this check can be confusing as this refers to
an access check on a table created within the same command as the one
that would insert data into this table.
This commit removes the INSERT privilege check for WITH DATA, the
default, that
846005e removed partially, but only for WITH NO DATA.
Author: Bharath Rupireddy
Discussion: https://postgr.es/m/
d049c272-9a47-d783-46b0-
46665b011598@enterprisedb.com
Peter Eisentraut [Sat, 21 Nov 2020 07:26:20 +0000 (08:26 +0100)]
doc: Improve tableoid description
Mention that it's useful for determining table names for partitioned
tables as well as for those in inheritance hierarchies.
Author: Ian Lawrence Barwick <barwick@gmail.com>
Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAB8KJ=gFmBXP=P9htziOj+WM5PDAK4qc7iGQta+8kUh306kQnw@mail.gmail.com
Heikki Linnakangas [Fri, 20 Nov 2020 14:08:58 +0000 (16:08 +0200)]
Make pg_rewind test case more stable.
If replication is exceptionally slow for some reason, pg_rewind might run
before the test row has been replicated. Add an explicit wait for it.
Reported-by: Andres Freund
Discussion: https://www.postgresql.org/message-id/
20201120003811.iknhqwatitw2vvxf%40alap3.anarazel.de
Magnus Hagander [Fri, 20 Nov 2020 11:26:57 +0000 (12:26 +0100)]
Remove ability to independently select random number generator
Remove the ability to select random number generator independently from
SSL library. Instead, use the random number generator from the SSL
library (today only OpenSSL supported) if one is configured. If no SSL
library is configured, use the platform default (which means use
CryptoAPI on Win32 and /dev/urandom on Linux).
This also restructures pg_strong_random.c to have three clearly separate
sections, one for each implementation, with two functions in each,
instead of a scattered set of ifdefs throughout the whole file.
Author: Daniel Gustafsson, Magnus Hagander, Michael Paquier
Discussion: https://postgr.es/m/632623.
1605460616@sss.pgh.pa.us
Peter Eisentraut [Fri, 20 Nov 2020 10:25:25 +0000 (11:25 +0100)]
Replace a macro by a function
Using a macro is ugly and not justified here.
Discussion: https://www.postgresql.org/message-id/flat/
4ad69a4c-cc9b-0dfe-0352-
8b1b0cd36c7b@2ndquadrant.com
Thomas Munro [Fri, 20 Nov 2020 08:49:57 +0000 (21:49 +1300)]
Add collation versions for FreeBSD.
On FreeBSD 13, use querylocale() to read the current version of libc
collations. Similar to commits
352f6f2d for Windows and
d5ac14f9 for
GNU/Linux.
Discussion: https://postgr.es/m/CAEepm%3D0uEQCpfq_%2BLYFBdArCe4Ot98t1aR4eYiYTe%3DyavQygiQ%40mail.gmail.com
Fujii Masao [Fri, 20 Nov 2020 06:42:47 +0000 (15:42 +0900)]
Emit log when restore_command succeeds but archived file faills to be restored.
Previously, when restore_command claimed to succeed but failed to restore
the file with the right name, for example, due to mis-configuration of
restore_command, no log message was reported. Then the recovery failed
later with an error message not directly related to the issue.
This commit changes the recovery so that a log message is emitted in
this error case. This would enable us to investigate what happened in
this case more easily.
Author: Jeff Janes, Fujii Masao
Reviewed-by: Pavel Borisov, Kyotaro Horiguchi
Discussion: https://postgr.es/m/CAMkU=1xkFs3Omp4JR4wMYWdam_KLuj6LXnTYfU8u3T0h=PLLMQ@mail.gmail.com
Tom Lane [Fri, 20 Nov 2020 05:07:09 +0000 (00:07 -0500)]
On macOS, use -isysroot in link steps as well as compile steps.
We previously put the -isysroot switch only into CPPFLAGS, theorizing
that it was only needed to find the right copies of include files.
However, it seems that we also need to use it while linking programs,
to find the right stub ".tbd" files for libraries. We got away
without that up to now, but apparently that was mostly luck. It may
also be that failures are only observed when the Xcode version is
noticeably out of sync with the host macOS version; the case that's
prompting action right now is that builds fail when using latest Xcode
(12.2) on macOS Catalina, even though it's fine on Big Sur.
Hence, add -isysroot to LDFLAGS as well. (It seems that the more
common practice is to put it in CFLAGS, whence it'd be included at
both compile and link steps. However, we can't mess with CFLAGS in
the platform template file without confusing configure's logic for
choosing default CFLAGS.)
This should be back-patched, but first let's see if the buildfarm
likes it on HEAD.
Report and patch by James Hilliard (some cosmetic mods by me)
Discussion: https://postgr.es/m/
20201120003314.20560-1-james.hilliard1@gmail.com
Tom Lane [Thu, 19 Nov 2020 22:39:39 +0000 (17:39 -0500)]
Remove undocumented IS [NOT] OF syntax.
This feature was added a long time ago, in
7c1e67bd5 and
eb121ba2c,
but never documented in any user-facing way. (Documentation added
in
6126d3e70 was commented out almost immediately, in
8272fc3f7.)
That's because, while this syntax is defined by SQL:99, our
implementation is only vaguely related to the standard's semantics.
The standard appears to intend a run-time not parse-time test, and
it definitely intends that the test should understand subtype
relationships.
No one has stepped up to fix that in the intervening years, but
people keep coming across the code and asking why it's not documented.
Let's just get rid of it: if anyone ever wants to make it work per
spec, they can easily recover whatever parts of this code are still
of value from our git history.
If there's anyone out there who's actually using this despite its
undocumented status, they can switch to using pg_typeof() instead,
eg. "pg_typeof(something) = 'mytype'::regtype". That gives
essentially the same semantics as what our IS OF code did.
(We didn't have that function last time this was discussed, or
we would have ripped out IS OF then.)
Discussion: https://postgr.es/m/CAKFQuwZ2pTc-DSkOiTfjauqLYkNREeNZvWmeg12Q-_69D+sYZA@mail.gmail.com
Discussion: https://postgr.es/m/BAY20-
F23E9F2B4DAB3E4E88D3623F99B0@phx.gbl
Discussion: https://postgr.es/m/
3E7CF81D.
1000203@joeconway.com
Tom Lane [Thu, 19 Nov 2020 20:03:17 +0000 (15:03 -0500)]
Further fixes for CREATE TABLE LIKE: cope with self-referential FKs.
Commit
502898192 was too careless about the order of execution of the
additional ALTER TABLE operations generated by expandTableLikeClause.
It just stuck them all at the end, which seems okay for most purposes.
But it falls down in the case where LIKE is importing a primary key
or unique index and the outer CREATE TABLE includes a FOREIGN KEY
constraint that needs to depend on that index. Weird as that is,
it used to work, so we ought to keep it working.
To fix, make parse_utilcmd.c insert LIKE clauses between index-creation
and FK-creation commands in the transformed list of commands, and change
utility.c so that the commands generated by expandTableLikeClause are
executed immediately not at the end. One could imagine scenarios where
this wouldn't work either; but currently expandTableLikeClause only
makes column default expressions, CHECK constraints, and indexes, and
this ordering seems fine for those.
Per bug #16730 from Sofoklis Papasofokli. Like the previous patch,
back-patch to all supported branches.
Discussion: https://postgr.es/m/16730-
b902f7e6e0276b30@postgresql.org
Peter Eisentraut [Thu, 19 Nov 2020 10:34:54 +0000 (11:34 +0100)]
Rename object in test to avoid conflict
In
01e658fa74cb7e3292448f6663b549135958003b, the hash_func test
creates a type t1, but apparently a test running in parallel might
also use that name, depending on timing. Rename the type to avoid the
issue.
Peter Eisentraut [Thu, 19 Nov 2020 08:24:37 +0000 (09:24 +0100)]
Hash support for row types
Add hash functions for the record type as well as a hash operator
family and operator class for the record type. This enables all the
hash functionality for the record type such as hash-based plans for
UNION/INTERSECT/EXCEPT DISTINCT, recursive queries using UNION
DISTINCT, hash joins, and hash partitioning.
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/
38eccd35-4e2d-6767-1b3c-
dada1eac3124%402ndquadrant.com
Thomas Munro [Thu, 19 Nov 2020 05:09:50 +0000 (18:09 +1300)]
Add BarrierArriveAndDetachExceptLast().
Provide a way for one process to continue the remaining phases of a
(previously) parallel computation alone. Later patches will use this to
extend Parallel Hash Join.
Author: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://postgr.es/m/CA%2BhUKG%2BA6ftXPz4oe92%2Bx8Er%2BxpGZqto70-Q_ERwRaSyA%3DafNg%40mail.gmail.com
Michael Paquier [Thu, 19 Nov 2020 01:36:08 +0000 (10:36 +0900)]
Improve failure detection with array parsing in pg_dump
Similarly to
3636efa, the checks done in pg_dump when parsing array
values from catalogs have been too lax. Under memory pressure, it could
be possible, though very unlikely, to finish with dumps that miss some
data like:
- Statistics for indexes
- Run-time configuration of functions
- Configuration of extensions
- Publication list for a subscription
No backpatch is done as this is not going to be a problem in practice.
For example, if an OOM causes an array parsing to fail, a follow-up code
path of pg_dump would most likely complain with an allocation failure
due to the memory pressure.
Author: Michael Paquier
Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/
20201111061319.GE2276@paquier.xyz
Alvaro Herrera [Wed, 18 Nov 2020 16:24:22 +0000 (13:24 -0300)]
Relax lock level for setting PGPROC->statusFlags
We don't actually need a lock to set PGPROC->statusFlags itself; what we
do need is a shared lock on either XidGenLock or ProcArrayLock in order to
ensure MyProc->pgxactoff keeps still while we modify the mirror array in
ProcGlobal->statusFlags. Some places were using an exclusive lock for
that, which is excessive. Relax those to use shared lock only.
procarray.c has a couple of places with somewhat brittle assumptions
about PGPROC changes: ProcArrayEndTransaction uses only shared lock, so
it's permissible to change MyProc only. On the other hand,
ProcArrayEndTransactionInternal also changes other procs, so it must
hold exclusive lock. Add asserts to ensure those assumptions continue
to hold.
Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/
20201117155501.GA13805@alvherre.pgsql
Heikki Linnakangas [Wed, 18 Nov 2020 10:39:15 +0000 (12:39 +0200)]
Skip allocating hash table in EXPLAIN-only mode.
Author: Alexey Bashtanov
Discussion: https://www.postgresql.org/message-id/
36823f65-050d-ae24-aa4d-
a37726998240%40imap.cc
Heikki Linnakangas [Wed, 18 Nov 2020 08:28:50 +0000 (10:28 +0200)]
docs: Clarify that signal functions have no feedback.
Bug #16652 complains that pg_reload_conf() returned true, even though
the configuration file contained errors. That's the way pg_reload_conf()
works, by design, but the documentation wasn't very clear on it. Clarify
that a 'true' return value only means that the signal was sent
successfully. Also add links to the system views that can be used to
check the configuration files for errors.
David G. Johnston, with some rewording by me.
Discussion: https://www.postgresql.org/message-id/CAKFQuwax6GxhUQEes0D045UtXG-fBraM39_6UMd5JyR5K1HWCQ%40mail.gmail.com
Peter Eisentraut [Wed, 18 Nov 2020 06:58:37 +0000 (07:58 +0100)]
Add more tests for hashing and hash-based plans
- Test hashing of an array of a non-hashable element type.
- Test UNION [DISTINCT] with hash- and sort-based plans. (Previously,
only INTERSECT and EXCEPT where tested there.)
- Test UNION [DISTINCT] with a non-hashable column type. This
currently reverts to a sort-based plan even if enable_hashagg is on.
- Test UNION/INTERSECT/EXCEPT hash- and sort-based plans with arrays
as column types. Also test an array with a non-hashable element
type.
- Test UNION/INTERSECT/EXCEPT similarly with row types as column
types. Currently, this uses only sort-based plans because there is
no hashing support for row types.
- Add a test case that shows that recursive queries using UNION
[DISTINCT] require hashable column types.
- Add a currently failing test that uses UNION DISTINCT in a
cycle-detection use case using row types as column types.
Discussion: https://www.postgresql.org/message-id/flat/
38eccd35-4e2d-6767-1b3c-
dada1eac3124%402ndquadrant.com
Michael Paquier [Wed, 18 Nov 2020 05:01:53 +0000 (14:01 +0900)]
Add tab completion for CREATE [OR REPLACE] TRIGGER in psql
92bf7e2 has added support for this grammar.
Author: Noriyoshi Shinoda
Discussion: https://postgr.es/m/TU4PR8401MB115244623CF4724DCA0D507FEEE30@TU4PR8401MB1152.NAMPRD84.PROD.OUTLOOK.COM
Peter Geoghegan [Tue, 17 Nov 2020 17:45:56 +0000 (09:45 -0800)]
Deprecate nbtree's BTP_HAS_GARBAGE flag.
Streamline handling of the various strategies that we have to avoid a
page split in nbtinsert.c. When it looks like a leaf page is about to
overflow, we now perform deleting LP_DEAD items and deduplication in one
central place. This greatly simplifies _bt_findinsertloc().
This has an independently useful consequence: nbtree no longer relies on
the BTP_HAS_GARBAGE page level flag/hint for anything important. We
still set and unset the flag in the same way as before, but it's no
longer treated as a gating condition when considering if we should check
for already-set LP_DEAD bits. This happens at the point where the page
looks like it might have to be split anyway, so simply checking the
LP_DEAD bits in passing is practically free. This avoids missing
LP_DEAD bits just because the page-level hint is unset, which is
probably reasonably common (e.g. it happens when VACUUM unsets the
page-level flag without actually removing index tuples whose LP_DEAD-bit
was set recently, after the VACUUM operation began but before it reached
the leaf page in question).
Note that this isn't a big behavioral change compared to PostgreSQL 13.
We were already checking for set LP_DEAD bits regardless of whether the
BTP_HAS_GARBAGE page level flag was set before we considered doing a
deduplication pass. This commit only goes slightly further by doing the
same check for all indexes, even indexes where deduplication won't be
performed.
We don't completely remove the BTP_HAS_GARBAGE flag. We still rely on
it as a gating condition with pg_upgrade'd indexes from before B-tree
version 4/PostgreSQL 12. That makes sense because we sometimes have to
make a choice among pages full of duplicates when inserting a tuple with
pre version 4 indexes. It probably still pays to avoid accessing the
line pointer array of a page there, since it won't yet be clear whether
we'll insert on to the page in question at all, let alone split it as a
result.
Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: Victor Yegorov <vyegorov@gmail.com>
Discussion: https://postgr.es/m/CAH2-Wz%3DYpc1PDdk8OVJDChGJBjT06%3DA0Mbv9HyTLCsOknGcUFg%40mail.gmail.com
Alvaro Herrera [Tue, 17 Nov 2020 14:37:36 +0000 (11:37 -0300)]
indexcmds.c: reorder function prototypes
... out of an overabundance of neatnikism, perhaps.
Peter Geoghegan [Tue, 17 Nov 2020 17:01:14 +0000 (09:01 -0800)]
nbtree: Rename nbtinsert.c variables for consistency.
Stop naming special area/opaque pointer variables 'lpageop' in contexts
where it doesn't make sense. This is a holdover from a time when logic
that performs tasks that are now spread across _bt_insertonpg(),
_bt_findinsertloc(), and _bt_split() was more centralized. 'lpageop'
denotes "left page", which doesn't make sense outside of contexts in
which there isn't also a right page.
Also acquire page flag variables up front within _bt_insertonpg(). This
makes it closer to _bt_split() following refactoring commit
bc3087b626d.
This allows the page split and retail insert paths to both make use of
the same variables.
Amit Kapila [Tue, 17 Nov 2020 06:44:53 +0000 (12:14 +0530)]
Fix 'skip-empty-xacts' option in test_decoding for streaming mode.
In streaming mode, the transaction can be decoded in multiple streams and
those streams can be interleaved with streams of other transactions. So,
we can't remember the transaction's write status in the logical decoding
context because that might get changed due to some other transactions and
lead to wrong answers for 'skip-empty-xacts' option. We decided to keep
each transaction's write status in the ReorderBufferTxn to avoid
interleaved streams changing the status of some unrelated transactions.
Diagnosed-by: Amit Kapila
Author: Dilip Kumar
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/CAA4eK1LR7=XNM_TLmpZMFuV8ZQpoxkem--NZJYf8YXmesbvwLA@mail.gmail.com
Tom Lane [Tue, 17 Nov 2020 01:32:35 +0000 (20:32 -0500)]
Don't Insert() a VFD entry until it's fully built.
Otherwise, if FDDEBUG is enabled, the debugging output fails because
it tries to read the fileName, which isn't set up yet (and should in
fact always be NULL).
AFAICT, this has been wrong since Berkeley. Before
96bf88d52,
it would accidentally fail to crash on platforms where snprintf()
is forgiving about being passed a NULL pointer for %s; but the
file name intended to be included in the debug output wouldn't
ever have shown up.
Report and fix by Greg Nancarrow. Although this is only visibly
broken in custom-made builds, it still seems worth back-patching
to all supported branches, as the FDDEBUG code is pretty useless
as it stands.
Discussion: https://postgr.es/m/CAJcOf-cUDgm9qYtC_B6XrC6MktMPNRby2p61EtSGZKnfotMArw@mail.gmail.com
Alvaro Herrera [Mon, 16 Nov 2020 22:42:55 +0000 (19:42 -0300)]
Rename PGPROC->vacuumFlags to statusFlags
With more flags associated to a PGPROC entry that are not related to
vacuum (currently existing or planned), the name "statusFlags" describes
its purpose better.
(The same is done to the mirroring PROC_HDR->vacuumFlags.)
No functional changes in this commit.
This was suggested first by Hari Babu Kommi in [1] and then by Michael
Paquier at [2].
[1] https://postgr.es/m/CAJrrPGcsDC-oy1AhqH0JkXYa0Z2AgbuXzHPpByLoBGMxfOZMEQ@mail.gmail.com
[2] https://postgr.es/m/
20200820060929.GB3730@paquier.xyz
Author: Dmitry Dolgov <9erthalion6@gmail.com>
Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/
20201116182446.qcg3o6szo2zookyr@localhost
Tom Lane [Mon, 16 Nov 2020 21:39:59 +0000 (16:39 -0500)]
Do not return NULL for error cases in satisfies_hash_partition().
Since this function is used as a CHECK constraint condition,
returning NULL is tantamount to returning TRUE, which would have the
effect of letting in a row that doesn't satisfy the hash condition.
Admittedly, the cases for which this is done should be unreachable
in practice, but that doesn't make it any less a bad idea. It also
seems like a dartboard was used to decide which error cases should
throw errors as opposed to returning NULL.
For the checks for NULL input values, I just switched it to returning
false. There's some argument that an error would be better; but the
case really should be can't-happen in a generated hash constraint,
so it's likely not worth more code for.
For the parent-relation-open-failure case, it seems like we might
as well let relation_open throw an error, instead of having an
impossible-to-diagnose constraint failure.
Back-patch to v11 where this code came in.
Discussion: https://postgr.es/m/24067.
1605134819@sss.pgh.pa.us
Tom Lane [Mon, 16 Nov 2020 20:16:39 +0000 (15:16 -0500)]
Use "true" not "TRUE" in one ICU function call.
This was evidently missed in commit
6337865f3, which generally did
s/TRUE/true/ everywhere. It escaped notice up to now because ICU
versions before ICU 68 provided definitions of "TRUE" and "FALSE"
regardless. With ICU 68, it fails to compile.
Per report from Condor. Back-patch to v11 where
6337865f3 came in.
(I've not tested v10, where this call originated, but I imagine
it's fine since we defined TRUE in c.h back then.)
Discussion: https://postgr.es/m/
7a6f3336165bfe3ca66abcda7966f9d0@stz-bg.com
Bruce Momjian [Mon, 16 Nov 2020 18:13:43 +0000 (13:13 -0500)]
doc: update bgwriter description
This clarifies exactly what the bgwriter does, which should help with
tuning.
Reported-by: Chris Wilson
Discussion: https://postgr.es/m/
160399562040.7809.
7335281028960123489@wrigleys.postgresql.org
Backpatch-through: 9.5
Bruce Momjian [Mon, 16 Nov 2020 17:36:17 +0000 (12:36 -0500)]
doc: clarify how to find pg_type_d.h in the install tree
Followup to patch
152ed04799.
Reported-by: Alvaro Herrera
Discussion: https://postgr.es/m/
20201112202900.GA28098@alvherre.pgsql
Backpatch-through: 9.5
Peter Eisentraut [Mon, 16 Nov 2020 16:25:41 +0000 (17:25 +0100)]
Remove unused and deprecated strategy numbers from BRIN code
These were dead code.
Discussion: https://www.postgresql.org/message-id/flat/
20201027032511.GF9241@telsasoft.com
Bruce Momjian [Mon, 16 Nov 2020 15:26:17 +0000 (10:26 -0500)]
doc: improve wording of the need for analyze of exp. indexes
This is a followup commit on
3370207986.
Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/
20201112211143.GL30691@telsasoft.com
Backpatch-through: 9.5
Alvaro Herrera [Mon, 16 Nov 2020 13:54:11 +0000 (10:54 -0300)]
Fix typo
Introduced in
90fdc259866e; backpatch to 12.
Author: Erik Rijkers <er@xs4all.nl>
Discussion: https://postgr.es/m/
e92b3fba98a0c0f7afc0a2a37e765954@xs4all.nl
Peter Eisentraut [Mon, 16 Nov 2020 10:54:52 +0000 (11:54 +0100)]
Normalize comment in empty grammar rules
Change lower case /* empty */ to /* EMPTY */ for consistency with the
majority.
Discussion: https://www.postgresql.org/message-id/flat/
e9eed669-e32d-6919-fed4-
acc0daea857b%40enterprisedb.com
Peter Eisentraut [Mon, 16 Nov 2020 10:45:00 +0000 (11:45 +0100)]
Remove code handling removed deprecated containment operators
This removes the code that was there for handling the operators
removed by
2f70fdb0644c32c4154236c2b5c241bec92eac5e.
Discussion: https://www.postgresql.org/message-id/flat/
20201027032511.GF9241@telsasoft.com
Fujii Masao [Mon, 16 Nov 2020 09:27:51 +0000 (18:27 +0900)]
Make the standby server promptly handle interrupt signals.
This commit changes the startup process in the standby server so that
it handles the interrupt signals after waiting for wal_retrieve_retry_interval
on the latch and resetting it, before entering another wait on the latch.
This change causes the standby server to promptly handle interrupt signals.
Otherwise, previously, there was the case where the standby needs to
wait extra five seconds to shutdown when the shutdown request arrived
while the startup process was waiting for wal_retrieve_retry_interval
on the latch.
Author: Fujii Masao, but implementation idea is from Soumyadeep Chakraborty
Reviewed-by: Soumyadeep Chakraborty
Discussion: https://postgr.es/m/
9d7e6ab0-8a53-ddb9-63cd-
289bcb25fe0e@oss.nttdata.com
Alexander Korotkov [Mon, 16 Nov 2020 06:09:07 +0000 (09:09 +0300)]
Reword 'simple comparison' => 'inequality' in pgtrgm.sgml
Discussion: https://postgr.es/m/CAMkU%3D1zxfA8_MGBW6sJMj54p8nPoe4bMb5LoG-rMYZVPq4j08Q%40mail.gmail.com
Author: Jeff Janes
Michael Paquier [Mon, 16 Nov 2020 02:52:40 +0000 (11:52 +0900)]
Relax INSERT privilege requirement for CTAS and matviews WITH NO DATA
When specified, WITH NO DATA does not insert any data into the relation
created, so skip checking for the insert permissions. With WITH DATA or
WITH NO DATA, it is always required for the user to have CREATE
privileges on the schema targeted for the relation.
Note that plain CREATE TABLE AS or CREATE MATERIALIZED VIEW queries have
begun to work accidentally without INSERT privilege checks as of
874fe3ae, while using EXECUTE or EXPLAIN ANALYZE would fail with the ACL
check, so this makes the behavior for all the command flavors consistent
with each other. This is arguably a bug fix, but there have been no
complaints about the current behavior either so stable branches are not
changed.
While on it, document properly the privileges requirements for each
commands with more tests for all the scenarios possible, and avoid a
useless bulk-insert allocation when using WITH NO DATA.
Author: Bharath Rupireddy
Reviewed-by: Anastasia Lubennikova, Michael Paquier
Discussion: https://postgr.es/m/CALj2ACWc3N8j0_9nMPz9wcAUnVcdKHzFdDZJ3hVFNEbqtcyG9w@mail.gmail.com
Tom Lane [Sun, 15 Nov 2020 21:10:48 +0000 (16:10 -0500)]
Fix fuzzy thinking about amcanmulticol versus amcaninclude.
These flags should be independent: in particular an index AM should
be able to say that it supports include columns without necessarily
supporting multiple key columns. The included-columns patch got
this wrong, possibly aided by the fact that it didn't bother to
update the documentation.
While here, clarify some text about amcanreturn, which was a little
vague about what should happen when amcanreturn reports that only
some of the index columns are returnable.
Noted while reviewing the SP-GiST included-columns patch, which
quite incorrectly (and unsafely) changed SP-GiST to claim
amcanmulticol = true as a workaround for this bug.
Backpatch to v11 where included columns were introduced.
Peter Geoghegan [Sun, 15 Nov 2020 19:53:37 +0000 (11:53 -0800)]
nbtree: Demote incomplete split "can't happen" error.
Only a basic logic bug in a _bt_insertonpg() caller could lead to a
violation of this invariant (index corruption won't do it). A "can't
happen" error seems inappropriate (it is arbitrary at best).
Demote the error to a simple assertion. This matches similar nearby
sanity checks.
Tom Lane [Sun, 15 Nov 2020 17:39:49 +0000 (12:39 -0500)]
Suppress "warning: variable 'collcollate' set but not used".
Buildfarm members that lack both HAVE_LOCALE_T and USE_ICU have been
complaining about pg_newlocale_from_collation's collcollate variable.
This is evidently fallout from commit
7d1297df0, which removed the
only usage outside those two #ifdef'd code paths. Mark the variable
pg_attribute_unused(), like its sibling collctype, which has been that
way for a long time.
Heikki Linnakangas [Sun, 15 Nov 2020 15:09:31 +0000 (17:09 +0200)]
Fix timing issue in pg_rewind test.
The test inserts a row in primary server, waits until the insertion has
been replicated to a cascaded standby, and checks that it's visible there
by querying the cascaded standby. In order for that to work reliably, the
test needs to wait until the insertion WAL record has been fully replayed.
This should fix the occasional buildfarm failures. Diagnosis by Tom Lane.
Discussion: https://www.postgresql.org/message-id/606796.
1605424022@sss.pgh.pa.us
Alexander Korotkov [Sun, 15 Nov 2020 11:01:22 +0000 (14:01 +0300)]
Add missing 'the' to pgtrgm.sgml
Author: Erik Rijkers
Discussion: https://postgr.es/m/
159fba265fe7c37334640fdc0444cc4b%40xs4all.nl
Alexander Korotkov [Sun, 15 Nov 2020 05:56:31 +0000 (08:56 +0300)]
Fix alphabetic ordering in typedefs.list
Alexander Korotkov [Sun, 15 Nov 2020 05:52:12 +0000 (08:52 +0300)]
Handle equality operator in contrib/pg_trgm
Obviously, in order to equality operator be satisfiable, target string must
contain all the trigrams of the search string. On this base, we implement
equality operator in GiST/GIN indexes with recheck.
Discussion: https://postgr.es/m/CAOBaU_YWwtT7tdggtROacjdOdeYHCz-tmSwuC-j-TOG-g97J0w%40mail.gmail.com
Author: Julien Rouhaud
Reviewed-by: Tom Lane, Alexander Korotkov, Georgios Kokolatos, Erik Rijkers
Tom Lane [Sat, 14 Nov 2020 22:05:34 +0000 (17:05 -0500)]
Provide the OR REPLACE option for CREATE TRIGGER.
This is mostly straightforward. However, we disallow replacing
constraint triggers or changing the is-constraint property; perhaps
that can be added later, but the complexity versus benefit tradeoff
doesn't look very good.
Also, no special thought is taken here for whether replacing an
existing trigger should result in changes to queued-but-not-fired
trigger actions. We just document that if you're surprised by the
results, too bad, don't do that. (Note that any such pending trigger
activity would have to be within the current session.)
Takamichi Osumi, reviewed at various times by Surafel Temesgen,
Peter Smith, and myself
Discussion: https://postgr.es/m/
0DDF369B45A1B44B8A687ED43F06557C010BC362@G01JPEXMBYT03
Tom Lane [Sat, 14 Nov 2020 18:09:53 +0000 (13:09 -0500)]
Doc: improve partitioning discussion in ddl.sgml.
This started with the intent to explain that range upper bounds
are exclusive, which previously you could only find out by reading
the CREATE TABLE man page. But I soon found that section 5.11
really could stand a fair amount of editorial attention. It's
apparently been revised several times without much concern for
overall flow, nor careful copy-editing.
Back-patch to v11, which is as far as the patch goes easily.
Per gripe from Edson Richter. Thanks to David Johnston for review.
Discussion: https://postgr.es/m/DM6PR13MB3988736CF8F5DC5720440231CFE60@DM6PR13MB3988.namprd13.prod.outlook.com
Michael Paquier [Sat, 14 Nov 2020 02:43:10 +0000 (11:43 +0900)]
Fix some typos
Author: Daniel Gustafsson
Discussion: https://postgr.es/m/
C36ADFDF-D09A-4EE5-B186-
CB46C3653F4C@yesql.se
Tom Lane [Fri, 13 Nov 2020 18:49:48 +0000 (13:49 -0500)]
Support negative indexes in split_part().
This provides a handy way to get, say, the last field of the string.
Use of a negative index in this way has precedent in the nearby
left() and right() functions.
The implementation scans the string twice when N < -1, but it seems
likely that N = -1 will be the huge majority of actual use cases,
so I'm not really excited about adding complexity to avoid that.
Nikhil Benesch, reviewed by Jacob Champion; cosmetic tweakage by me
Discussion: https://postgr.es/m/
cbb7f861-6162-3a51-9823-
97bc3aa0b638@gmail.com
Heikki Linnakangas [Fri, 13 Nov 2020 11:41:58 +0000 (13:41 +0200)]
Remove another test that doesn't work on Windows.
Apparently double-quotes are not allowed in filenames on Windows, either.
Per buildfarm.
Bruce Momjian [Thu, 12 Nov 2020 20:13:02 +0000 (15:13 -0500)]
doc: clarify where to find pg_type_d.h (PG 11+) and pg_type.h
These files are in compiled directories and install directories.
Reported-by: e.indrupskaya@postgrespro.ru
Discussion: https://postgr.es/m/
160379609706.24746.
7506163279454026608@wrigleys.postgresql.org
Backpatch-through: 9.5
Bruce Momjian [Thu, 12 Nov 2020 20:00:44 +0000 (15:00 -0500)]
docs: mention that expression indexes need analyze
Expression indexes can't benefit from pre-computed statistics on
columns.
Reported-by: Nikolay Samokhvalov
Discussion: https://postgr.es/m/CANNMO++5rw9RDA=p40iMVbMNPaW6O=S0AFzTU=KpYHRpCd1voA@mail.gmail.com
Author: Nikolay Samokhvalov, modified
Backpatch-through: 9.5
Bruce Momjian [Thu, 12 Nov 2020 19:08:59 +0000 (14:08 -0500)]
change wire protocol data type for history file content
This was marked as BYTEA, but is more like TEXT, which is how we already
pass the history timeline file name. Internally, we don't do any
encoding or bytea escape handling, but TEXT seems closest. This should
cause no behavioral change.
Reported-by: Brar Piening
Discussion: https://postgr.es/m/
6a1b9cd9-17e3-df67-be55-
86102af6bdf5@gmx.de
Backpatch-through: master
Heikki Linnakangas [Thu, 12 Nov 2020 17:18:34 +0000 (19:18 +0200)]
Remove tests that don't work on Windows.
On Windows, a filename cannot contain backslashes, because a backslash
is used directory separator. Remove tests I added in commit
9c4f5192f
that tried to do that. We could perhaps use a SKIP block to only skip
them on Windows, but I'm not sure how exactly to formulate that, so just
remove the tests to make the buildfarm green again.
Per buildfarm.
Andrew Gierth [Thu, 12 Nov 2020 14:34:37 +0000 (14:34 +0000)]
pg_trgm: fix crash in 2-item picksplit
Whether from size overflow in gistSplit or from secondary splits,
picksplit is (rarely) called with exactly two items to split.
Formerly, due to special-case handling of the last item, this would
lead to access to an uninitialized cache entry; prior to PG 13 this
might have been harmless or at worst led to an incorrect union datum,
but in 13 onwards it can cause a backend crash from using an
uninitialized pointer.
Repair by removing the special case, which was deemed not to have been
appropriate anyway. Backpatch all the way, because this bug has
existed since pg_trgm was added.
Per report on IRC from user "ftzdomino". Analysis and testing by me,
patch from Alexander Korotkov.
Discussion: https://postgr.es/m/87k0usfdxg.fsf@news-spur.riddles.org.uk