Michael Paquier [Tue, 5 Nov 2019 01:32:38 +0000 (10:32 +0900)]
Doc: Clarify locks taken when using ALTER TABLE ATTACH PARTITION
Since
898e5e32, this command uses partially ShareUpdateExclusiveLock,
but the docs did not get the call.
Author: Justin Pryzby
Reviewed-by: Amit Langote, Álvaro Herrera, Michael Paquier
Discussion: https://postgr.es/m/
20191028001207.GB23808@telsasoft.com
Backpatch-through: 12
Michael Paquier [Tue, 5 Nov 2019 01:17:33 +0000 (10:17 +0900)]
Doc: Improve description around ALTER TABLE ATTACH PARTITION
This clarifies more how to use and how to take advantage of constraints
when attaching a new partition.
Author: Justin Pryzby
Reviewed-by: Amit Langote, Álvaro Herrera, Michael Paquier
Discussion: https://postgr.es/m/
20191028001207.GB23808@telsasoft.com
Backpatch-through: 10
Michael Paquier [Tue, 5 Nov 2019 00:17:05 +0000 (09:17 +0900)]
Refactor code building relation options
Historically, the code to build relation options has been shaped the
same way in multiple code paths by using a set of datums in input with
the options parsed with a static table which is then filled with the
option values. This introduces a new common routine in reloptions.c to
do most of the legwork for the in-core code paths.
Author: Amit Langote
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/CA+HiwqGsoSn_uTPPYT19WrtR7oYpYtv4CdS0xuedTKiHHWuk_g@mail.gmail.com
Tom Lane [Mon, 4 Nov 2019 21:25:05 +0000 (16:25 -0500)]
Stabilize pg_dump output order for similarly-named triggers and policies.
The code only compared two triggers' names and namespaces (the latter
being the owning table's schema). This could result in falling back
to an OID-based sort of similarly-named triggers on different tables.
We prefer to avoid that, so add a comparison of the table names too.
(The sort order is thus table namespace, trigger name, table name,
which is a bit odd, but it doesn't seem worth contorting the code
to work around that.)
Likewise for policy objects, in 9.5 and up.
Complaint and fix by Benjie Gillam. Back-patch to all supported
branches.
Discussion: https://postgr.es/m/CAMThMzEEt2mvBbPgCaZ1Ap1N-moGn=Edxmadddjq89WG4NpPtQ@mail.gmail.com
Tom Lane [Mon, 4 Nov 2019 19:16:42 +0000 (14:16 -0500)]
Fix ginEntryInsert's counting of GIN leaf tuples.
As the code stands, nEntries counts the number of ginEntryInsert()
calls, so that's what you end up with at the end of a GIN index build.
However, ginvacuumcleanup() recomputes nEntries as the number of
surviving leaf tuples, and that's generally consistent with the way that
gincostestimate() uses the value. So let's clearly define nEntries
as the number of leaf tuples, and therefore adjust ginEntryInsert() to
increment it only when we make a new one, not when we add TIDs into an
existing tuple or posting tree.
In practice this inconsistency probably has little impact, so I don't
feel a need to back-patch.
Insung Moon and Keisuke Kuroda
Discussion: https://postgr.es/m/CAEMmqBuH_O-oXL+3_ArQ6F5cJ7kXVow2SGQB3HRacku_T+xkmA@mail.gmail.com
Peter Eisentraut [Mon, 4 Nov 2019 10:07:32 +0000 (11:07 +0100)]
Fix some compiler warnings on older compilers
Some older compilers appear to not understand the recently introduced
PG_FINALLY code structure that well in some circumstances and complain
about possibly uninitialized variables. So to fix, initialize the
variables explicitly in the cases complained about.
Discussion: https://www.postgresql.org/message-id/flat/
95a822c3-728b-af0e-d7e5-
71890507ae0c%402ndquadrant.com
Peter Eisentraut [Mon, 4 Nov 2019 07:30:00 +0000 (08:30 +0100)]
Catch invalid typlens in a couple of places
Rearrange the logic in record_image_cmp() and datum_image_eq() to
error out on unexpected typlens (either not supported there or
completely invalid due to corruption). Barring corruption, this is
not possible today but it seems more future-proof and robust to fix
this.
Reported-by: Peter Geoghegan <pg@bowt.ie>
Tom Lane [Sun, 3 Nov 2019 21:10:23 +0000 (16:10 -0500)]
Suppress warning from older compilers.
Commit
8af1624e3 introduced a warning about possibly returning
without a value, on compilers that don't realize that ereport(ERROR)
doesn't return. Tweak the code to avoid that.
Per buildfarm. Back-patch to 9.6, like the aforesaid commit.
Tom Lane [Sun, 3 Nov 2019 15:57:49 +0000 (10:57 -0500)]
Fix PG_GETARG_SEG_P() definition.
DatumGetPointer() takes a Datum argument, not a pointer.
This is cosmetic given the current definitions of the
underlying macros, but it's still formally a type violation.
Bug was introduced in commit
389bb2818, but there seems
no need to back-patch.
Dagfinn Ilmari Mannsåker
Discussion: https://postgr.es/m/d8jlfsxq3a0.fsf@dalvik.ping.uio.no
Tom Lane [Sat, 2 Nov 2019 20:45:32 +0000 (16:45 -0400)]
Validate ispell dictionaries more carefully.
Using incorrect, or just mismatched, dictionary and affix files
could result in a crash, due to failure to cross-check offsets
obtained from the file. Add necessary validation, as well as
some Asserts for future-proofing.
Per bug #16050 from Alexander Lakhin. Back-patch to 9.6 where the
problem was introduced.
Arthur Zakirov, per initial investigation by Tomas Vondra
Discussion: https://postgr.es/m/16050-
024ae722464ab604@postgresql.org
Discussion: https://postgr.es/m/
20191013012610.2p2fp3zzpoav7jzf@development
Michael Paquier [Sat, 2 Nov 2019 05:16:04 +0000 (14:16 +0900)]
Fix failure when creating cloned indexes for a partition
When using CREATE TABLE for a new partition, the partitioned indexes of
the parent are created automatically in a fashion similar to LIKE
INDEXES. The new partition and its parent use a mapping for attribute
numbers for this operation, and while the mapping was correctly built,
its length was defined as the number of attributes of the newly-created
child, and not the parent. If the parent includes dropped columns, this
could cause failures.
This is wrong since
8b08f7d which has introduced the concept of
partitioned indexes, so backpatch down to 11.
Reported-by: Wyatt Alt
Author: Michael Paquier
Reviewed-by: Amit Langote
Discussion: https://postgr.es/m/CAGem3qCcRmhbs4jYMkenYNfP2kEusDXvTfw-q+eOhM0zTceG-g@mail.gmail.com
Backpatch-through: 11
Michael Paquier [Fri, 1 Nov 2019 13:51:05 +0000 (22:51 +0900)]
Add some assertions in syncrep.c
A couple of routines assume that the LWLock SyncRepLock needs to be
taken, so add a couple of assertions to be sure of that. Also, when
waiting for a given LSN at transaction commit, the code implied that the
syncrep queue cleanup happens while holding interrupts, but the code
never checked after that.
Author: Michael Paquier
Reviewed-by: Fujii Masao, Kyotaro Horiguchi, Dongming Liu
Discussion: https://postgr.es/m/
a0806273-8bbb-43b3-bbe1-
c45a58f6ae21.lingce.ldm@alibaba-inc.com
Michael Paquier [Fri, 1 Nov 2019 13:38:32 +0000 (22:38 +0900)]
Fix race condition at backend exit when deleting element in syncrep queue
When a backend exits, it gets deleted from the syncrep queue if present.
The queue was checked without SyncRepLock taken in exclusive mode, so it
would have been possible for a backend to remove itself after a WAL
sender already did the job. Fix this issue based on a suggestion from
Fujii Masao, by first checking the queue without the lock. Then, if the
backend is present in the queue, take the lock and perform an additional
lookup check before doing the element deletion.
Author: Dongming Liu
Reviewed-by: Kyotaro Horiguchi, Fujii Masao, Michael Paquier
Discussion: https://postgr.es/m/
a0806273-8bbb-43b3-bbe1-
c45a58f6ae21.lingce.ldm@alibaba-inc.com
Backpatch-through: 9.4
Peter Eisentraut [Fri, 1 Nov 2019 12:16:21 +0000 (13:16 +0100)]
Add some assertions to view reloption macros
In these macros, the rd_options pointer is cast to ViewOption *. Add
some assertions that the passed-in relation is actually a view before
doing that.
Author: Nikolay Shaplov <dhyan@nataraj.su>
Discussion: https://www.postgresql.org/message-id/flat/
3634983.eHpMQ1mJnI@x200m
Peter Eisentraut [Fri, 1 Nov 2019 10:09:52 +0000 (11:09 +0100)]
PG_FINALLY
This gives an alternative way of catching exceptions, for the common
case where the cleanup code is the same in the error and non-error
cases. So instead of
PG_TRY();
{
... code that might throw ereport(ERROR) ...
}
PG_CATCH();
{
cleanup();
PG_RE_THROW();
}
PG_END_TRY();
cleanup();
one can write
PG_TRY();
{
... code that might throw ereport(ERROR) ...
}
PG_FINALLY();
{
cleanup();
}
PG_END_TRY();
Discussion: https://www.postgresql.org/message-id/flat/
95a822c3-728b-af0e-d7e5-
71890507ae0c%402ndquadrant.com
Peter Eisentraut [Thu, 31 Oct 2019 06:41:41 +0000 (07:41 +0100)]
Add const qualifiers to internal range type APIs
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/flat/
dc9b45fa-b950-fadc-4751-
85d6f729df55%402ndquadrant.com
Michael Paquier [Thu, 31 Oct 2019 01:22:24 +0000 (10:22 +0900)]
Fix typo in comment of syncrep.c
Author: Kyotaro Horiguchi
Discussion: https://postgr.es/m/
20191030.123428.
18823202335157111.horikyota.ntt@gmail.com
Peter Eisentraut [Wed, 30 Oct 2019 10:01:44 +0000 (11:01 +0100)]
Remove one use of IDENT_USERNAME_MAX
IDENT_USERNAME_MAX is the maximum length of the information returned
by an ident server, per RFC 1413. Using it as the buffer size in peer
authentication is inappropriate. It was done here because of the
historical relationship between peer and ident authentication. To
reduce confusion between the two authenticaton methods and disentangle
their code, use a dynamically allocated buffer instead.
Discussion: https://www.postgresql.org/message-id/flat/
c798fba5-8b71-4f27-c78e-
37714037ea31%402ndquadrant.com
Peter Eisentraut [Wed, 30 Oct 2019 08:13:39 +0000 (09:13 +0100)]
Update code comments about peer authenticaton
For historical reasons, the functions for peer authentication were
grouped under ident authentication. But they are really completely
separate, so give them their own section headings.
Andres Freund [Wed, 30 Oct 2019 05:46:40 +0000 (22:46 -0700)]
pg_waldump: Fix --bkp-details to not issue spurious newlines for FPWs.
The additional newline seems to have accidentally been introduced in
2c03216d831, in 9.5. The newline is only issued when an FPW is
present for the block reference.
While there could be an argument that removing the newlines in the
back branches could cause a problem for somebody parsing the
pg_waldump output, the likelihood of that seems small enough. It seems
at least equally likely that the randomness of when newlines are
issued causes problems.
Author: Andres Freund
Discussion: https://postgr.es/m/
20191029233341.4gnyau7e5v2lh5sc@alap3.anarazel.de
Backpatch: 9.5, like
2c03216d831.
Andres Freund [Wed, 30 Oct 2019 02:18:07 +0000 (19:18 -0700)]
pg_waldump: Fix small memory leak when rmgr->rm_identify returns NULL.
This got broken in
604f7956b94, shortly after rm_identify's
introduction.
Author: Andres Freund
Discussion: https://postgr.es/m/
20191029233341.4gnyau7e5v2lh5sc@alap3.anarazel.de
Backpatch: 9.5, where rm_identify was introduced
Michael Paquier [Wed, 30 Oct 2019 01:03:00 +0000 (10:03 +0900)]
Fix typos in the code
Author: Vignesh C
Reviewed-by: Dilip Kumar, Michael Paquier
Discussion: https://postgr.es/m/CALDaNm0ni+GAOe4+fbXiOxNrVudajMYmhJFtXGX-zBPoN8ixhw@mail.gmail.com
Peter Eisentraut [Tue, 29 Oct 2019 08:14:43 +0000 (09:14 +0100)]
Fix compiler warnings in ecpg tests
Under MinGW, when compiling the ecpg test files, you get compiler
warnings about the use of %lld in printf().
These files don't use our printf replacement or the c.h porting layer,
so determine the appropriate format conversion the hard way.
Reviewed-by: Michael Meskes <meskes@postgresql.org>
Discussion: https://www.postgresql.org/message-id/flat/
760c9dd1-2d80-c223-3f90-
609b615f7918%402ndquadrant.com
Michael Paquier [Tue, 29 Oct 2019 02:08:09 +0000 (11:08 +0900)]
Fix handling of pg_class.relispartition at swap phase in REINDEX CONCURRENTLY
When cancelling REINDEX CONCURRENTLY after swapping the old and new
indexes (for example interruption at step 5), the old index remains
around and is marked as invalid. The old index should also be manually
droppable to clean up the parent relation from any invalid indexes still
remaining. For a partition index reindexed, pg_class.relispartition was
not getting updated, causing the index to not be droppable as DROP INDEX
would look for dependencies in a partition tree, which do not exist
anymore after the swap phase is done.
The fix here is simple: when swapping the old and new indexes, make sure
that pg_class.relispartition is correctly switched, similarly to what is
done for the index name.
Reported-by: Justin Pryzby
Author: Michael Paquier
Discussion: https://postgr.es/m/
20191015164047.GA22729@telsasoft.com
Backpatch-through: 12
Tom Lane [Mon, 28 Oct 2019 19:08:24 +0000 (15:08 -0400)]
Allow extracting fields from a ROW() expression in more cases.
Teach get_expr_result_type() to manufacture a tuple descriptor directly
from a RowExpr node. If the RowExpr has type RECORD, this is the only
way to get a tupdesc for its result, since even if the rowtype has been
blessed, we don't have its typmod available at this point. (If the
RowExpr has some named composite type, we continue to let the existing
code handle it, since the RowExpr might well not have the correct column
names embedded in it.)
This fixes assorted corner cases illustrated by the added regression
tests.
Discussion: https://postgr.es/m/10872.
1572202006@sss.pgh.pa.us
Tom Lane [Mon, 28 Oct 2019 18:15:03 +0000 (14:15 -0400)]
On Windows, use COMSPEC to find the location of cmd.exe.
Historically, psql consulted COMSPEC to spawn a shell in its \! command,
but we just invoked "cmd" when spawning shells in pg_ctl and pg_regress.
It seems better to rely on the environment variable, if it's set,
in all cases.
It's debatable whether this is a bug fix or just a behavioral change,
so no back-patch.
Juan José Santamaría Flecha
Discussion: https://postgr.es/m/16080-
5d7f03222469f717@postgresql.org
Tom Lane [Mon, 28 Oct 2019 16:21:13 +0000 (12:21 -0400)]
Handle empty-string edge cases correctly in strpos().
Commit
9556aa01c rearranged the innards of text_position() in a way
that would make it not work for empty search strings. Which is fine,
because all callers of that code special-case an empty pattern in
some way. However, the primary use-case (text_position itself) got
special-cased incorrectly: historically it's returned 1 not 0 for
an empty search string. Restore the historical behavior.
Per complaint from Austin Drenski (via Shay Rojansky).
Back-patch to v12 where it got broken.
Discussion: https://postgr.es/m/CADT4RqAz7oN4vkPir86Kg1_mQBmBxCp-L_=9vRpgSNPJf0KRkw@mail.gmail.com
Michael Paquier [Mon, 28 Oct 2019 05:23:42 +0000 (14:23 +0900)]
Doc: Add missing step for pg_stat_progress_cluster
There is a step to track when the new heap is written, but this was
missing in the documentation.
Author: Noriyoshi Shinoda
Discussion: https://postgr.es/m/AT5PR8401MB06447FAE88E1592754E958B8EE640@AT5PR8401MB0644.NAMPRD84.PROD.OUTLOOK.COM
Backpatch-through: 12
Michael Paquier [Mon, 28 Oct 2019 02:57:31 +0000 (11:57 +0900)]
Fix dependency handling at swap phase of REINDEX CONCURRENTLY
When swapping the dependencies of the old and new indexes, the code has
been correctly switching all links in pg_depend from the old to the new
index for both referencing and referenced entries. However it forgot
the fact that the new index may itself have existing entries in
pg_depend, like references to the parent table attributes. This
resulted in duplicated entries in pg_depend after running REINDEX
CONCURRENTLY.
Fix this problem by removing any existing entries in pg_depend on the
new index before switching the dependencies of the old index to the new
one. More regression tests are added to check the consistency of
entries in pg_depend for indexes, including partition indexes.
Author: Michael Paquier
Discussion: https://postgr.es/m/
20191025064318.GF8671@paquier.xyz
Backpatch-through: 12
Michael Paquier [Sun, 27 Oct 2019 04:54:12 +0000 (13:54 +0900)]
Fix initialization of fake LSN for unlogged relations
9155580 has changed the value of the first fake LSN for unlogged
relations from 1 to FirstNormalUnloggedLSN (aka 1000), GiST requiring a
non-zero LSN on some pages to allow an interlocking logic to work, but
its value was still initialized to 1 at the beginning of recovery or
after running pg_resetwal. This fixes the initialization for both code
paths.
Author: Takayuki Tsunakawa
Reviewed-by: Dilip Kumar, Kyotaro Horiguchi, Michael Paquier
Discussion: https://postgr.es/m/OSBPR01MB2503CE851940C17DE44AE3D9FE6F0@OSBPR01MB2503.jpnprd01.prod.outlook.com
Backpatch-through: 12
Noah Misch [Sat, 26 Oct 2019 19:55:16 +0000 (12:55 -0700)]
Fix copy-paste defect in comment.
Commit
a7471bd85c05f849e88d6cfe9da3c795008e8f2e introduced it.
Noah Misch [Sat, 26 Oct 2019 19:55:06 +0000 (12:55 -0700)]
Update comment about __sync_lock_test_and_set() bug.
State the earliest known fixed version, so we can someday judge the
workaround to be obsolete.
Tom Lane [Sat, 26 Oct 2019 16:30:41 +0000 (12:30 -0400)]
Doc: improve documentation of configuration settings that have units.
When we added the GUC units feature, we didn't make any great effort
to adjust the documentation of individual GUCs; they tended to still
say things like "this is the number of milliseconds that ...", even
though users might prefer to write some other units, and SHOW might
even show the value in other units. Commit
6c9fb69f2 made an effort
to improve this situation, but I thought it made things less readable
by injecting units information in mid-sentence. It also wasn't very
consistent, and did not touch all the GUCs that have units.
To improve matters, standardize on the phrasing "If this value is
specified without units, it is taken as <units>". Also, try to
standardize where this is mentioned, right before the specification
of the default. (In a couple of places, doing that would've required
more rewriting than seemed justified, so I wasn't 100% consistent
about that.) I also tried to use the phrases "amount of time",
"amount of memory", etc rather than describing the contents of GUCs
in other ways, as those were the majority usage in places that weren't
overcommitting to a particular unit. (I left "length of time" alone
in a couple of places, though.)
I failed to resist the temptation to copy-edit some awkward text, too.
Backpatch to v12, like
6c9fb69f2, mainly because v12 hasn't diverged
much from HEAD yet.
Discussion: https://postgr.es/m/15882.
1571942223@sss.pgh.pa.us
Peter Eisentraut [Fri, 25 Oct 2019 19:11:48 +0000 (21:11 +0200)]
Remove obsolete information schema tables
Remove SQL_LANGUAGES, which was eliminated in SQL:2008, and
SQL_PACKAGES and SQL_SIZING_PROFILES, which were eliminated in
SQL:2011. Since they were dropped by the SQL standard, the
information in them was no longer updated and therefore no longer
useful.
This also removes the feature-package association information in
sql_feature_packages.txt, but for the time begin we are keeping the
information which features are in the Core package (that is, mandatory
SQL features). Maybe at some point someone wants to invent a way to
store that that does not involve using the "package" mechanism
anymore.
Discussion https://www.postgresql.org/message-id/flat/
91334220-7900-071b-9327-
0c6ecd012017%402ndquadrant.com
Tom Lane [Fri, 25 Oct 2019 19:22:40 +0000 (15:22 -0400)]
Avoid failure when selecting a namespace node in XMLTABLE.
It appears that libxml2 doesn't bother to set the "children" field of
an XML_NAMESPACE_DECL node to null; that field just contains garbage.
In v10 and v11, this can result in a crash in XMLTABLE(). The rewrite
done in commit
251cf2e27 fixed this, somewhat accidentally, in v12.
We're not going to back-patch
251cf2e27, however. The case apparently
doesn't have wide use, so rather than risk introducing other problems,
just add a safety check to throw an error.
Even though no bug manifests in v12/HEAD, add the relevant test case
there too, to prevent future regressions.
Chapman Flack (per private report)
Peter Eisentraut [Fri, 25 Oct 2019 18:23:44 +0000 (20:23 +0200)]
doc: Use proper em and en dashes
Tom Lane [Fri, 25 Oct 2019 16:18:11 +0000 (12:18 -0400)]
Revert "Revert part of commit
dddf4cdc3."
This reverts commit
c114229ca2519620703a4be4e38181290cad8c0a.
Commit
1408d5d869925c8ea7ca01c2644e8903fbab23de should make it
safe to include these headers in the natural order.
Tom Lane [Fri, 25 Oct 2019 16:17:41 +0000 (12:17 -0400)]
Get rid of useless/dangerous redefinition of bool in ECPG.
pgtypeslib_extern.h contained fallback definitions of "bool", "FALSE",
and "TRUE". The latter two are just plain unused, and have been for
awhile. The former came into play only if there wasn't a macro
definition of "bool", which is true only if we aren't using <stdbool.h>.
However, it then defined bool as "char"; since commit
d26a810eb that
conflicts with c.h's desire to use "unsigned char". We'd missed seeing
any bad effects of that due to accidental header inclusion order choices,
but
dddf4cdc3 exposed that it was problematic.
To fix, let's just get rid of these definitions. They should not be
needed because everyplace in Postgres should be relying on c.h to
provide a definition for type bool. (Note that despite its name,
pgtypeslib_extern.h isn't exposed to any outside code; we don't
install it.)
This doesn't fully resolve the issue, because ecpglib.h is doing
similar things, but that seems to require more thought to fix.
Back-patch to v12 where
d26a810eb came in, to forestall any unpleasant
surprises from future back-patched bug fixes.
Discussion: https://postgr.es/m/CAA4eK1LmaKO7Du9M9Lo=kxGU8sB6aL8fa3sF6z6d5yYYVe3BuQ@mail.gmail.com
Tom Lane [Fri, 25 Oct 2019 15:41:16 +0000 (11:41 -0400)]
Improve management of statement timeouts.
Commit
f8e5f156b added private state in postgres.c to track whether
a statement timeout is running. This seems like bad design to me;
timeout.c's private state should be the single source of truth about
that. We already fixed one bug associated with failure to keep those
states in sync (cf.
be42015fc), and I've got little faith that we
won't find more in future. So get rid of postgres.c's local variable
by exposing a way to ask timeout.c whether a timeout is running.
(Obviously, such an inquiry is subject to race conditions, but it
seems fine for the purpose at hand.)
To make get_timeout_active() as cheap as possible, add a flag in
the per-timeout struct showing whether that timeout is active.
This allows some small savings elsewhere in timeout.c, mainly
elimination of unnecessary searches of the active_timeouts array.
While at it, fix enable_statement_timeout to not call disable_timeout
when statement_timeout is 0 and the timeout is not running. This
avoids a useless deschedule-and-reschedule-timeouts cycle, which
represents a significant savings (at least one kernel call) when
there is any other active timeout. Right now, there usually isn't,
but there are proposals around to change that.
Discussion: https://postgr.es/m/16035-
456e6e69ebfd4374@postgresql.org
Tom Lane [Fri, 25 Oct 2019 15:15:50 +0000 (11:15 -0400)]
Reset statement_timeout between queries of a multi-query string.
Historically, we started the timer (if StatementTimeout > 0) at the
beginning of a simple-Query message and usually let it run until the
end, so that the timeout limit applied to the entire query string,
and intra-string changes of the statement_timeout GUC had no effect.
But, confusingly, a COMMIT within the string would reset the state
and allow a fresh timeout cycle to start with the current setting.
Commit
f8e5f156b changed the behavior of statement_timeout for extended
query protocol, and as an apparently-unintended side effect, a change in
the statement_timeout GUC during a multi-statement simple-Query message
might have an effect immediately --- but only if it was going from
"disabled" to "enabled".
This is all pretty confusing, not to mention completely undocumented.
Let's change things so that the timeout is always reset between queries
of a multi-query string, whether they're transaction control commands
or not. Thus the active timeout setting is applied to each query in
the string, separately. This costs a few more cycles if statement_timeout
is active, but it provides much more intuitive behavior, especially if one
changes statement_timeout in one of the queries of the string.
Also, add something to the documentation to explain all this.
Per bug #16035 from Raj Mohite. Although this is a bug fix, I'm hesitant
to back-patch it; conceivably somebody has worked out the old behavior
and is depending on it. (But note that this change should make the
behavior less restrictive in most cases, since the timeout will now
be applied to shorter segments of code.)
Discussion: https://postgr.es/m/16035-
456e6e69ebfd4374@postgresql.org
Amit Kapila [Fri, 25 Oct 2019 04:34:20 +0000 (10:04 +0530)]
Revert part of commit
dddf4cdc3.
The commit
dddf4cdc3 tries to ensure that the Postgres header file
inclusions are in order based on their ASCII value. However, in one of
the case there is a header file dependency due to which we can't maintain
such order.
Author: Amit Kapila
Discussion: https://postgr.es/m/E1iNpHW-000855-1u@gemulon.postgresql.org
Amit Kapila [Wed, 23 Oct 2019 04:08:53 +0000 (09:38 +0530)]
Make the order of the header file includes consistent in non-backend modules.
Similar to commit
7e735035f2, this commit makes the order of header file
inclusion consistent for non-backend modules.
In passing, fix the case where we were using angle brackets (<>) for the
local module includes instead of quotes ("").
Author: Vignesh C
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/CALDaNm2Sznv8RR6Ex-iJO6xAdsxgWhCoETkaYX=+9DW3q0QCfA@mail.gmail.com
Michael Paquier [Fri, 25 Oct 2019 01:20:08 +0000 (10:20 +0900)]
Handle interrupts within a transaction context in REINDEX CONCURRENTLY
Phases 2 (building the new index) and 3 (validating the new index)
checked for interrupts outside a transaction context, having as
consequence to not release session-level locks taken on the parent
relation and the old and new indexes processed. This could for example
be triggered with statement_timeout and a bad timing, and would issue
confusing error messages when shutting down the session still holding
the locks (note that an assertion failure would be triggered first), on
top of more issues with concurrent sessions trying to take a lock that
would interfere with the SHARE UPDATE EXCLUSIVE locks hold here.
This moves all the interruption checks inside a transaction context.
Note that I have manually tested all interruptions to make sure that
invalid indexes can be cleaned up properly. Partition indexes still
have issues on their own with some missing dependency handling, which
will be dealt with in a follow-up patch.
Reported-by: Justin Pryzby
Author: Michael Paquier
Discussion: https://postgr.es/m/
20191013025145.GC4475@telsasoft.com
Backpatch-through: 12
Fujii Masao [Thu, 24 Oct 2019 05:13:36 +0000 (14:13 +0900)]
Fix typo in xlog.c.
Author: Fujii Masao
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/CAHGQGwH7dtYvOZZ8c0AG5AJwH5pfiRdKaCptY1_RdHy0HYeRfQ@mail.gmail.com
Amit Kapila [Wed, 23 Oct 2019 03:56:22 +0000 (09:26 +0530)]
Make the order of the header file includes consistent in contrib modules.
The basic rule we follow here is to always first include 'postgres.h' or
'postgres_fe.h' whichever is applicable, then system header includes and
then Postgres header includes. In this, we also follow that all the
Postgres header includes are in order based on their ASCII value. We
generally follow these rules, but the code has deviated in many places.
This commit makes it consistent just for contrib modules. The later
commits will enforce similar rules in other parts of code.
Author: Vignesh C
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/CALDaNm2Sznv8RR6Ex-iJO6xAdsxgWhCoETkaYX=+9DW3q0QCfA@mail.gmail.com
Bruce Momjian [Thu, 24 Oct 2019 00:29:02 +0000 (20:29 -0400)]
docs: fix wording of REFRESH CONCURRENTLY manual page
Reported-by: Asim / apraveen@pivotal.io
Discussion: https://postgr.es/m/
157076828181.26165.
15231292023740913543@wrigleys.postgresql.org
Backpatch-through: 9.4
Bruce Momjian [Wed, 23 Oct 2019 22:06:38 +0000 (18:06 -0400)]
pg_upgrade: adjust error output to use new database list format
Commit
a524f50d0f added
old_11_check_for_sql_identifier_data_type_usage(), but it did not use
the clearer database error list format added to the master branch in
commit
1634d36157. This commit fixes that.
Backpatch-through: master
Michael Paquier [Wed, 23 Oct 2019 06:04:48 +0000 (15:04 +0900)]
Acquire properly session-level lock on new index in REINDEX CONCURRENTLY
In the first transaction run for REINDEX CONCURRENTLY, a thinko in the
existing logic caused two session locks to be taken on the old index,
causing the session lock on the newly-created index to be missed. This
made possible concurrent DDL commands (like ALTER INDEX) on the new
index while REINDEX CONCURRENTLY was processing from the point where the
first internal transaction committed.
This issue has been discovered while digging into another bug.
Author: Michael Paquier
Discussion: https://postgr.es/m/
20191021074323.GB1869@paquier.xyz
Backpatch-through: 12
Peter Eisentraut [Wed, 23 Oct 2019 05:10:09 +0000 (07:10 +0200)]
Remove libpq-dist.rc
The use of this was removed by
6da56f3f84d430671d5edd8f9336bd744c089e31.
Discussion: https://www.postgresql.org/message-id/
87d95052-3780-b833-9953-
27eab80186cf%402ndquadrant.com
Michael Paquier [Wed, 23 Oct 2019 03:27:03 +0000 (12:27 +0900)]
Remove last traces of --adduser/--no-adduser in createuser
8ae0d47 marked those options as obsolete back in 2005, with the options
removed from the documentation. This removes the last references to
both options in the code which were kept around for compatibility
purposes with past commands.
Author: Alexander Lakhin
Discussion: https://postgr.es/m/
5da284a2-62d9-e338-88d1-
26ee5009d93e@gmail.com
Michael Paquier [Wed, 23 Oct 2019 02:34:18 +0000 (11:34 +0900)]
Fix thinkos from
4f4061b for libpq integer parsing
A check was redundant. While on it, add an assertion to make sure that
the parsing routine is never called with a NULL input. All the code
paths currently calling the parsing routine are careful with NULL inputs
already, but future callers may forget that.
Reported-by: Peter Eisentraut, Lars Kanis
Discussion: https://postgr.es/m/
ec64956b-4597-56b6-c3db-
457d15250fe4@2ndquadrant.com
Backpatch-through: 12
Michael Paquier [Wed, 23 Oct 2019 01:25:06 +0000 (10:25 +0900)]
Clean up properly error_context_stack in autovacuum worker on exception
Any callback set would have no meaning in the context of an exception.
As an autovacuum worker exits quickly in this context, this could be
only an issue within EmitErrorReport(), where the elog hook is for
example called. That's unlikely to going to be a problem, but let's be
clean and consistent with other code paths handling exceptions. This is
present since
2909419, which introduced autovacuum.
Author: Ashwin Agrawal
Reviewed-by: Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/CALfoeisM+_+dgmAdAOHAu0k-ZpEHHqSSG=GRf3pKJGm8OqWX0w@mail.gmail.com
Backpatch-through: 9.4
Peter Eisentraut [Tue, 22 Oct 2019 08:35:54 +0000 (10:35 +0200)]
Make command order in test more sensible
Through several updates, the CREATE USER command has been separated
from where the user is actually used in the test.
Peter Eisentraut [Tue, 22 Oct 2019 07:58:20 +0000 (09:58 +0200)]
Fix comment
The last argument of smgrextend() was renamed from isTemp to skipFsync
in
debcec7dc31a992703911a9953e299c8d730c778, but the comments at two
call sites were not updated.
Alexander Korotkov [Mon, 21 Oct 2019 20:04:14 +0000 (23:04 +0300)]
Refactor jsonpath's compareDatetime()
This commit refactors come ridiculous coding in compareDatetime(). Also, it
provides correct cross-datatype comparison even when one of values overflows
during cast. That eliminates dilemma on whether we should suppress overflow
errors during cast.
Reported-by: Tom Lane
Discussion: https://postgr.es/m/32308.
1569455803%40sss.pgh.pa.us
Discussion: https://postgr.es/m/
a5629d0c-8162-7559-16aa-
0c8390d6ba5f%40postgrespro.ru
Author: Nikita Glukhov, Alexander Korotkov
Alexander Korotkov [Mon, 21 Oct 2019 20:03:55 +0000 (23:03 +0300)]
Refactor timestamp2timestamptz_opt_error()
While casting from timestamp to timestamptz we do timestamp2tm() then
tm2timestamp(). This commit eliminates call to tm2timestamp(). Instead, it
directly applies timezone offset to the original timestamp value. That makes
upcoming datetime overflow handling in jsonpath easier. That should also save
us some CPU cycles.
Discussion: https://postgr.es/m/CAPpHfdvRPRh_mTGar5WmDeRZ%3DU5dOXHdxspYYD%3D76m3knNGjXA%40mail.gmail.com
Author: Alexander Korotkov
Reviewed-by: Tom Lane
Tom Lane [Mon, 21 Oct 2019 18:18:01 +0000 (14:18 -0400)]
Deal with yet another issue related to "Norwegian (Bokmål)" locale.
It emerges that recent versions of Windows (at least 2016 Standard)
spell this locale name as "Norwegian Bokmål_Norway.1252", defeating
our mapping code that translates "Norwegian (Bokmål)_Norway" to
something that's all-ASCII (cf commits
db29620d4 and
aa1d2fc5e).
Add another mapping entry to handle this spelling.
Per bug #16068 from Robert Ford. Like the previous patches,
back-patch to all supported branches.
Discussion: https://postgr.es/m/16068-
4cb6eeaa7eb46d93@postgresql.org
Tom Lane [Mon, 21 Oct 2019 17:52:25 +0000 (13:52 -0400)]
Use CFLAGS_SL while probing linkability of libperl.
On recent Red Hat platforms (at least RHEL 8 and Fedora 30, maybe older),
configure's probe for libperl failed if the user forces CFLAGS to be -O0.
This is because some code in perl's inline.h fails to be optimized away
at -O0, and said code doesn't work if compiled without -fPIC.
To fix, add CFLAGS_SL to the compile flags used during the libperl probe.
This is a better simulation of the way that plperl is built, anyway,
so it might forestall other issues in future.
Per gripe from Kyotaro Horiguchi. Back-patch to all supported branches,
since people might want to build older branches on these platforms.
Discussion: https://postgr.es/m/
20191010.144533.
263180400.horikyota.ntt@gmail.com
Tom Lane [Mon, 21 Oct 2019 16:32:35 +0000 (12:32 -0400)]
Select CFLAGS_SL at configure time, not in platform-specific Makefiles.
Move the platform-dependent logic that sets CFLAGS_SL from
src/makefiles/Makefile.foo to src/template/foo, so that the value
is determined at configure time and thus is available while running
configure's tests.
On a couple of platforms this might save a few microseconds of build
time by eliminating a test that make otherwise has to do over and over.
Otherwise it's pretty much a wash for build purposes; in particular,
this makes no difference to anyone who might be overriding CFLAGS_SL
via a make option.
This patch in itself does nothing with the value and thus should not
change any behavior, though you'll probably have to re-run configure
to get a correctly updated Makefile.global. We'll use the new
configure variable in a follow-on patch.
Per gripe from Kyotaro Horiguchi. Back-patch to all supported branches,
because the follow-on patch is a portability bug fix.
Discussion: https://postgr.es/m/
20191010.144533.
263180400.horikyota.ntt@gmail.com
Etsuro Fujita [Mon, 21 Oct 2019 08:30:00 +0000 (17:30 +0900)]
Update obsolete comment.
Commit
b52b7dc25, which moved code creating PartitionBoundInfo in
RelationBuildPartitionDesc() in partcache.c (relocated to partdesc.c
afterwards) to partbounds.c, should have updated this, but didn't.
Author: Etsuro Fujita
Reviewed-by: Alvaro Herrera
Backpatch-through: 12
Discussion: https://postgr.es/m/CAPmGK16Uxr%3DPatiGyaRwiQVLB7Y-GqbkK3AxRLVYzU0Czv%3DsEw%40mail.gmail.com
Amit Kapila [Thu, 17 Oct 2019 03:15:43 +0000 (08:45 +0530)]
Fix memory leak introduced in commit
7df159a620.
We memorize all internal and empty leaf pages in the 1st vacuum stage for
gist indexes. They are used in the 2nd stage, to delete all the empty
pages. There was a memory context page_set_context for this purpose, but
we never used it.
Reported-by: Amit Kapila
Author: Dilip Kumar
Reviewed-by: Amit Kapila
Backpatch-through: 12, where it got introduced
Discussion: https://postgr.es/m/CAA4eK1LGr+MN0xHZpJ2dfS8QNQ1a_aROKowZB+MPNep8FVtwAA@mail.gmail.com
Michael Paquier [Mon, 21 Oct 2019 02:39:15 +0000 (11:39 +0900)]
Fix error reporting of connect_timeout in libpq for value parsing
The logic was correctly detecting a parsing failure, but the parsing
error did not get reported back to the client properly.
Reported-by: Ed Morley
Author: Lars Kanis
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/
a9b4cbd7-4ecb-06b2-ebd7-
1739bbff3217@greiz-reinsdorf.de
Backpatch-through: 12
Michael Paquier [Mon, 21 Oct 2019 02:17:13 +0000 (11:17 +0900)]
Fix parsing of integer values for connection parameters in libpq
Commit
e7a2217 has introduced stricter checks for integer values in
connection parameters for libpq. However this failed to correctly check
after trailing whitespaces, while leading whitespaces were discarded per
the use of strtol(3). This fixes and refactors the parsing logic to
handle both cases consistently. Note that trying to restrict the use of
trailing whitespaces can easily break connection strings like in ECPG
regression tests (these have allowed me to catch the parsing bug with
connect_timeout).
Author: Michael Paquier
Reviewed-by: Lars Kanis
Discussion: https://postgr.es/m/
a9b4cbd7-4ecb-06b2-ebd7-
1739bbff3217@greiz-reinsdorf.de
Backpatch-through: 12
Peter Eisentraut [Sun, 20 Oct 2019 08:19:13 +0000 (10:19 +0200)]
Clean up MinGW def file generation
There were some leftovers from ancient ad-hoc ways to build on
Windows, prior to the standardization on MSVC and MinGW. We don't
need to build a lib$(NAME)ddll.def (debug build, as opposed to
lib$(NAME)dll.def) for MinGW, since nothing uses that. We also don't
need to build the regular .def file during distprep, since the MinGW
build environment is perfectly capable of creating that normally at
build time.
Discussion: https://www.postgresql.org/message-id/flat/
0f9db9f8-47b8-a48b-6ccc-
15b22b412316%402ndquadrant.com
Peter Eisentraut [Sat, 19 Oct 2019 16:21:58 +0000 (18:21 +0200)]
Fix most -Wundef warnings
In some cases #if was used instead of #ifdef in an inconsistent style.
Cleaning this up also helps when analyzing cases like
38d8dce61fff09daae0edb6bcdd42b0c7f10ebcd where this makes a
difference.
There are no behavior changes here, but the change in pg_bswap.h would
prevent possible accidental misuse by third-party code.
Discussion: https://www.postgresql.org/message-id/flat/
3b615ca5-c595-3f1d-fdf7-
a429e564f614%402ndquadrant.com
Noah Misch [Sat, 19 Oct 2019 03:21:10 +0000 (20:21 -0700)]
Use standard compare_exchange loop style in ProcArrayGroupClearXid().
Besides style, this might improve performance in the contended case.
Reviewed by Amit Kapila.
Discussion: https://postgr.es/m/
20191015035348.GA4166224@rfd.leadboat.com
Noah Misch [Sat, 19 Oct 2019 03:20:52 +0000 (20:20 -0700)]
For all ppc compilers, implement compare_exchange and fetch_add with asm.
This is more like how we handle s_lock.h and arch-x86.h.
Reviewed by Tom Lane.
Discussion: https://postgr.es/m/
20191005173400.GA3979129@rfd.leadboat.com
Noah Misch [Sat, 19 Oct 2019 03:20:28 +0000 (20:20 -0700)]
For PowerPC instruction "addi", use constraint "b".
Without "b", a variant of the tas() code miscompiles on macOS 10.4.
This may also fix a compilation failure involving macOS 10.1. Today's
compilers have been allocating acceptable registers with or without this
change, but this future-proofs the code by precisely conveying the
acceptable registers. Back-patch to 9.4 (all supported versions).
Reviewed by Tom Lane.
Discussion: https://postgr.es/m/
20191009063900.GA4066266@rfd.leadboat.com
Michael Paquier [Sat, 19 Oct 2019 02:18:15 +0000 (11:18 +0900)]
Remove last traces of heap_open/close in the tree
Since pluggable storage has been introduced, those two routines have
been replaced by table_open/close, with some compatibility macros still
present to allow extensions to compile correctly with v12.
Some code paths using the old routines still remained, so replace them.
Based on the discussion done, the consensus reached is that it is better
to remove those compatibility macros so as nothing new uses the old
routines, so remove also the compatibility macros.
Discussion: https://postgr.es/m/
20191017014706.GF5605@paquier.xyz
Fujii Masao [Fri, 18 Oct 2019 13:32:18 +0000 (22:32 +0900)]
Fix failure of archive recovery with recovery_min_apply_delay enabled.
recovery_min_apply_delay parameter is intended for use with streaming
replication deployments. However, the document clearly explains that
the parameter will be honored in all cases if it's specified. So it should
take effect even if in archive recovery. But, previously, archive recovery
with recovery_min_apply_delay enabled always failed, and caused assertion
failure if --enable-caasert is enabled.
The cause of this problem is that; the ownership of recoveryWakeupLatch
that recovery_min_apply_delay uses was taken only when standby mode
is requested. So unowned latch could be used in archive recovery, and
which caused the failure.
This commit changes recovery code so that the ownership of
recoveryWakeupLatch is taken even in archive recovery. Which prevents
archive recovery with recovery_min_apply_delay from failing.
Back-patch to v9.4 where recovery_min_apply_delay was added.
Author: Fujii Masao
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/CAHGQGwEyD6HdZLfdWc+95g=VQFPR4zQL4n+yHxQgGEGjaSVheQ@mail.gmail.com
Fujii Masao [Fri, 18 Oct 2019 13:24:18 +0000 (22:24 +0900)]
Make crash recovery ignore recovery_min_apply_delay setting.
In v11 or before, this setting could not take effect in crash recovery
because it's specified in recovery.conf and crash recovery always
starts without recovery.conf. But commit
2dedf4d9a8 integrated
recovery.conf into postgresql.conf and which unexpectedly allowed
this setting to take effect even in crash recovery. This is definitely
not good behavior.
To fix the issue, this commit makes crash recovery always ignore
recovery_min_apply_delay setting.
Back-patch to v12 where the issue was added.
Author: Fujii Masao
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/CAHGQGwEyD6HdZLfdWc+95g=VQFPR4zQL4n+yHxQgGEGjaSVheQ@mail.gmail.com
Discussion: https://postgr.es/m/
e445616d-023e-a268-8aa1-
67b8b335340c@pgmasters.net
Alvaro Herrera [Fri, 18 Oct 2019 12:49:39 +0000 (14:49 +0200)]
Fix typo
Apparently while this code was being developed,
ReindexRelationConcurrently operated on multiple relations. The version
that was ultimately pushed doesn't, so this comment's use of plural is
inaccurate.
Alvaro Herrera [Fri, 18 Oct 2019 10:18:50 +0000 (07:18 -0300)]
Update comments about progress reporting by index_drop
Michaël Paquier complained that index_drop is requesting progress
reporting for non-obvious reasons, so let's add a comment to explain
why.
Discussion: https://postgr.es/m/
20191017010412.GH2602@paquier.xyz
Michael Paquier [Fri, 18 Oct 2019 05:26:29 +0000 (14:26 +0900)]
Fix timeout handling in logical replication worker
The timestamp tracking the last moment a message is received in a
logical replication worker was initialized in each loop checking if a
message was received or not, causing wal_receiver_timeout to be ignored
in basically any logical replication deployments. This also broke the
ping sent to the server when reaching half of wal_receiver_timeout.
This simply moves the initialization of the timestamp out of the apply
loop to the beginning of LogicalRepApplyLoop().
Reported-by: Jehan-Guillaume De Rorthais
Author: Julien Rouhaud
Discussion: https://postgr.es/m/CAOBaU_ZHESFcWva8jLjtZdCLspMj7vqaB2k++rjHLY897ZxbYw@mail.gmail.com
Backpatch-through: 10
Alvaro Herrera [Thu, 17 Oct 2019 13:06:06 +0000 (15:06 +0200)]
Fix minor bug in logical-replication walsender shutdown
Logical walsender should exit when it catches up with sending WAL during
shutdown; but there was a rare corner case when it failed to because of
a race condition that puts it back to wait for more WAL instead -- but
since there wasn't any, it'd not shut down immediately. It would only
continue the shutdown when wal_sender_timeout terminates the sleep,
which causes annoying waits during shutdown procedure. Restructure the
code so that we no longer forget to set WalSndCaughtUp in that case.
This was an oversight in commit
c6c333436.
Backpatch all the way down to 9.4.
Author: Craig Ringer, Álvaro Herrera
Discussion: https://postgr.es/m/CAMsr+YEuz4XwZX_QmnX_-2530XhyAmnK=zCmicEnq1vLr0aZ-g@mail.gmail.com
Alvaro Herrera [Thu, 17 Oct 2019 07:58:01 +0000 (09:58 +0200)]
Fix parallel restore of FKs to partitioned tables
When an FK constraint is created, it needs the index on the referenced
table to exist and be valid. When doing parallel pg_restore and the
referenced table was partitioned, this condition can sometimes not be
met, because pg_dump didn't emit sufficient object dependencies to
ensure so; this means that parallel pg_restore would fail in certain
conditions. Fix by having pg_dump make the FK constraint object
dependent on the partition attachment objects for the constraint's
referenced index.
This has been broken since
f56f8f8da6af, so backpatch to Postgres 12.
Discussion: https://postgr.es/m/
20191005224333.GA9738@alvherre.pgsql
Thomas Munro [Thu, 17 Oct 2019 00:24:50 +0000 (13:24 +1300)]
When restoring GUCs in parallel workers, show an error context.
Otherwise it can be hard to see where an error is coming from, when
the parallel worker sets all the GUCs that it received from the
leader. Bug #15726. Back-patch to 9.5, where RestoreGUCState()
appeared.
Reported-by: Tiago Anastacio
Reviewed-by: Daniel Gustafsson, Tom Lane
Discussion: https://postgr.es/m/15726-
6d67e4fa14f027b3%40postgresql.org
Thomas Munro [Wed, 16 Oct 2019 20:59:21 +0000 (09:59 +1300)]
Fix bug that could try to freeze running multixacts.
Commits
801c2dc7 and
801c2dc7 made it possible for vacuum to
try to freeze a multixact that is still running. That was
prevented by a check, but raised an error. Repair.
Back-patch all the way.
Author: Nathan Bossart, Jeremy Schneider
Reported-by: Jeremy Schneider
Reviewed-by: Jim Nasby, Thomas Munro
Discussion: https://postgr.es/m/
DAFB8AFF-2F05-4E33-AD7F-
FF8B0F760C17%40amazon.com
Alvaro Herrera [Wed, 16 Oct 2019 12:51:34 +0000 (14:51 +0200)]
Fix crash when reporting CREATE INDEX progress
A race condition can make us try to dereference a NULL pointer to the
PGPROC struct of a process that's already finished. That results in
crashes during REINDEX CONCURRENTLY and CREATE INDEX CONCURRENTLY.
This was introduced in
ab0dfc961b6a, so backpatch to pg12.
Reported by: Justin Pryzby
Reviewed-by: Michaël Paquier
Discussion: https://postgr.es/m/
20191012004446.GT10470@telsasoft.com
Tomas Vondra [Wed, 16 Oct 2019 11:23:18 +0000 (13:23 +0200)]
Improve the check for pg_catalog.unknown data type in pg_upgrade
The pg_upgrade check for pg_catalog.unknown type when upgrading from 9.6
had a couple of issues with domains and composite types - it detected
even composite types unused in objects with storage. So for example this
was enough to trigger an unnecessary pg_upgrade failure:
CREATE TYPE unknown_composite AS (u pg_catalog.unknown)
On the other hand, this only happened with composite types directly on
the pg_catalog.unknown data type, but not with a domain. So this was not
detected
CREATE DOMAIN unknown_domain AS pg_catalog.unknown;
CREATE TYPE unknown_composite_2 AS (u unknown_domain);
unlike the first example. These false positives and inconsistencies are
unfortunate, but what's worse we've failed to detected objects using the
pg_catalog.unknown type through a domain. So we missed cases like this
CREATE TABLE t (u unknown_composite_2);
The consequence is clusters broken after a pg_upgrade.
This fixes these false positives and false negatives by using the same
recursive CTE introduced by
eaf900e842 for sql_identifier. Backpatch all
the way to 10, where the of pg_catalog.unknown data type was restricted.
Author: Tomas Vondra
Backpatch-to: 10-
Discussion: https://postgr.es/m/16045-
673e8fa6b5ace196%40postgresql.org
Tomas Vondra [Wed, 16 Oct 2019 11:23:14 +0000 (13:23 +0200)]
Improve the check for pg_catalog.line data type in pg_upgrade
The pg_upgrade check for pg_catalog.line data type when upgrading from
9.3 had a couple of issues with domains and composite types. Firstly, it
triggered false positives for composite types unused in objects with
storage. This was enough to trigger an unnecessary pg_upgrade failure:
CREATE TYPE line_composite AS (l pg_catalog.line)
On the other hand, this only happened with composite types directly on
the pg_catalog.line data type, but not with a domain. So this was not
detected
CREATE DOMAIN line_domain AS pg_catalog.line;
CREATE TYPE line_composite_2 AS (l line_domain);
unlike the first example. These false positives and inconsistencies are
unfortunate, but what's worse we've failed to detected objects using the
pg_catalog.line data type through a domain. So we missed cases like this
CREATE TABLE t (l line_composite_2);
The consequence is clusters broken after a pg_upgrade.
This fixes these false positives and false negatives by using the same
recursive CTE introduced by
eaf900e842 for sql_identifier. 9.3 did not
support domains on composite types, but we can still have multi-level
composite types.
Backpatch all the way to 9.4, where the format for pg_catalog.line data
type changed.
Author: Tomas Vondra
Backpatch-to: 9.4-
Discussion: https://postgr.es/m/16045-
673e8fa6b5ace196%40postgresql.org
Andres Freund [Wed, 16 Oct 2019 09:37:34 +0000 (02:37 -0700)]
Replace alter_table.sql test usage of event triggers.
The test in
93765bd956b added an event trigger to ensure that the
tested table rewrites do not get optimized away (as happened in the
past). But doing so would require running the tests in isolation, as
otherwise the trigger might also fire in concurrent sessions, causing
test failures there.
Reported-By: Tom Lane
Discussion: https://postgr.es/m/3328.
1570740683@sss.pgh.pa.us
Backpatch: 12, just as
93765bd956b
Michael Paquier [Wed, 16 Oct 2019 06:10:14 +0000 (15:10 +0900)]
Refresh some incorrect links in pg_crc.c/h
Author: Vignesh C
Discussion: https://postgr.es/m/CALDaNm0LPk9vTGTBPBRv0=fX=94o4r6-DuBbHNeCN2AH5bufLw@mail.gmail.com
Thomas Munro [Wed, 16 Oct 2019 04:55:51 +0000 (17:55 +1300)]
Remove obsolete collation test.
The previous commit forgot to remove this test, which no longer
holds on all systems.
Thomas Munro [Wed, 16 Oct 2019 03:51:40 +0000 (16:51 +1300)]
Use libc version as a collation version on glibc systems.
Using glibc's version string to detect potential collation definition
changes is not 100% reliable, but it's better than nothing. Currently
this affects only collations explicitly provided by "libc". More work
will be needed to handle the default collation.
Author: Thomas Munro, based on a suggestion from Christoph Berg
Reviewed-by: Peter Eisentraut
Discussion: https://postgr.es/m/
4b76c6d4-ae5e-0dc6-7d0d-
b5c796a07e34%402ndquadrant.com
Michael Paquier [Wed, 16 Oct 2019 04:09:52 +0000 (13:09 +0900)]
Doc: Fix various inconsistencies
This fixes multiple areas of the documentation:
- COPY for its past compatibility section.
- SET ROLE mentioning INHERITS instead of INHERIT
- PREPARE referring to stmt_name, that is not present.
- Extension documentation about format name with upgrade scripts.
Backpatch down to 9.4 for the relevant parts.
Author: Alexander Lakhin
Discussion: https://postgr.es/m/
bf95233a-9943-b341-e2ff-
a860c28af481@gmail.com
Backpatch-through: 9.4
Andres Freund [Tue, 15 Oct 2019 17:40:13 +0000 (10:40 -0700)]
Fix CLUSTER on expression indexes.
Since the introduction of different slot types, in
1a0586de3657, we
create a virtual slot in tuplesort_begin_cluster(). While that looks
right, it unfortunately doesn't actually work, as ExecStoreHeapTuple()
is used to store tuples in the slot. Unfortunately no regression tests
for CLUSTER on expression indexes existed so far.
Fix the slot type, and add bare bones tests for CLUSTER on expression
indexes.
Reported-By: Justin Pryzby
Author: Andres Freund
Discussion: https://postgr.es/m/
20191011210320.GS10470@telsasoft.com
Backpatch: 12, like
1a0586de3657
Tomas Vondra [Mon, 14 Oct 2019 22:25:04 +0000 (00:25 +0200)]
Correct reference to pg_catalog.regtype in pg_upgrade query
The recursive CTE added in
0ccfc28223 referenced pg_catalog.regtype,
without the schema part, unlike all other queries in pg_upgrade.
Backpatch-to: 12
Tomas Vondra [Mon, 14 Oct 2019 20:31:56 +0000 (22:31 +0200)]
Check for tables with sql_identifier during pg_upgrade
Commit
7c15cef86d changed sql_identifier data type to be based on name
instead of varchar. Unfortunately, this breaks on-disk format for this
data type. Luckily, that should be a very rare problem, as this data
type is used only in information_schema views, so this only affects user
objects (tables, materialized views and indexes). One way to end in
such situation is to do CTAS with a query on those system views.
There are two options to deal with this - we can either abort pg_upgrade
if there are user objects with sql_identifier columns in pg_upgrade, or
we could replace the sql_identifier type with varchar. Considering how
rare the issue is expected to be, and the complexity of replacing the
data type (e.g. in matviews), we've decided to go with the simple check.
The query is somewhat complex - the sql_identifier data type may be used
indirectly - through a domain, a composite type or both, possibly in
multiple levels. Detecting this requires a recursive CTE.
Backpatch to 12, where the sql_identifier definition changed.
Reported-by: Hans Buschmann
Author: Tomas Vondra
Reviewed-by: Tom Lane
Backpatch-to: 12
Discussion: https://postgr.es/m/16045-
673e8fa6b5ace196%40postgresql.org
Michael Paquier [Sun, 13 Oct 2019 23:58:38 +0000 (08:58 +0900)]
Update test output of sepgsql for ALTER TABLE COLUMN DROP
1df5875 has changed the way dependencies are dropped for this command
with inheritance trees, which impacts sepgsql. This just updates the
regression test output to take care of the failures and adapt to the new
code.
Reported by buildfarm member rhinoceros.
Author: Michael Paquier
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/
20191013101331.GC1434@paquier.xyz
Backpatch-through: 12
Peter Eisentraut [Sun, 13 Oct 2019 20:10:38 +0000 (22:10 +0200)]
Update unicode.org URLs
Use https, consistent host name, remove references to ftp. Also
update the URLs for CLDR, which has moved from Trac to GitHub.
Tom Lane [Sun, 13 Oct 2019 19:48:26 +0000 (15:48 -0400)]
In the postmaster, rely on the signal infrastructure to block signals.
POSIX sigaction(2) can be told to block a set of signals while a
signal handler executes. Make use of that instead of manually
blocking and unblocking signals in the postmaster's signal handlers.
This should save a few cycles, and it also prevents recursive
invocation of signal handlers when many signals arrive in close
succession. We have seen buildfarm failures that seem to be due to
postmaster stack overflow caused by such recursion (exacerbated by
a Linux PPC64 kernel bug).
This doesn't change anything about the way that it works on Windows.
Somebody might consider adjusting port/win32/signal.c to let it work
similarly, but I'm not in a position to do that.
For the moment, just apply to HEAD. Possibly we should consider
back-patching this, but it'd be good to let it age awhile first.
Discussion: https://postgr.es/m/14878.
1570820201@sss.pgh.pa.us
Tom Lane [Sun, 13 Oct 2019 16:56:16 +0000 (12:56 -0400)]
Revert "Hack pg_ctl to report postmaster's exit status."
This reverts commit
6a5084eed49552bfc8859c438c8d74ad09fc5d3f.
We learned what we needed to know from that.
Michael Paquier [Sun, 13 Oct 2019 08:51:55 +0000 (17:51 +0900)]
Fix dependency handling of column drop with partitioned tables
When dropping a column on a partitioned table which has one or more
partitioned indexes, the operation was failing as dependencies with
partitioned indexes using the column dropped were not getting removed in
a way consistent with the columns involved across all the relations part
of an inheritance tree.
This commit refactors the code executing column drop so as all the
columns from an inheritance tree to remove are gathered first, and
dropped all at the end. This way, we let the dependency machinery sort
out by itself the deletion of all the columns with the partitioned
indexes across a partition tree.
This issue has been introduced by
1d92a0c, so backpatch down to
REL_12_STABLE.
Author: Amit Langote, Michael Paquier
Reviewed-by: Álvaro Herrera, Ashutosh Sharma
Discussion: https://postgr.es/m/CA+HiwqE9kuBsZ3b5pob2-cvE8ofzPWs-og+g8bKKGnu6b4-yTQ@mail.gmail.com
Backpatch-through: 12
Peter Eisentraut [Sat, 12 Oct 2019 19:17:34 +0000 (21:17 +0200)]
Fix use of term "verifier"
Within the context of SCRAM, "verifier" has a specific meaning in the
protocol, per RFCs. The existing code used "verifier" differently, to
mean whatever is or would be stored in pg_auth.rolpassword.
Fix this by using the term "secret" for this, following RFC 5803.
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/flat/
be397b06-6e4b-ba71-c7fb-
54cae84a7e18%402ndquadrant.com
Noah Misch [Sat, 12 Oct 2019 07:21:47 +0000 (00:21 -0700)]
AIX: Stop adding option -qsrcmsg.
With xlc v16.1.0, it causes internal compiler errors. With xlc versions
not exhibiting that bug, removing -qsrcmsg merely changes the compiler
error reporting format. Back-patch to 9.4 (all supported versions).
Discussion: https://postgr.es/m/
20191003064105.GA3955242@rfd.leadboat.com
Fujii Masao [Fri, 11 Oct 2019 06:47:59 +0000 (15:47 +0900)]
Make crash recovery ignore restore_command and recovery_end_command settings.
In v11 or before, those settings could not take effect in crash recovery
because they are specified in recovery.conf and crash recovery always
starts without recovery.conf. But commit
2dedf4d9a8 integrated
recovery.conf into postgresql.conf and which unexpectedly allowed
those settings to take effect even in crash recovery. This is definitely
not good behavior.
To fix the issue, this commit makes crash recovery always ignore
restore_command and recovery_end_command settings.
Back-patch to v12 where the issue was added.
Author: Fujii Masao
Reviewed-by: Peter Eisentraut
Discussion: https://postgr.es/m/
e445616d-023e-a268-8aa1-
67b8b335340c@pgmasters.net
Tom Lane [Thu, 10 Oct 2019 18:24:56 +0000 (14:24 -0400)]
Put back pqsignal() as an exported libpq symbol.
This reverts commit
f7ab80285. Per discussion, we can't remove an
exported symbol without a SONAME bump, which we don't want to do.
In particular that breaks usage of current libpq.so with pre-9.3
versions of psql etc, which need libpq to export pqsignal().
As noted in that commit message, exporting the symbol from libpgport.a
won't work reliably; but actually we don't want to export src/port's
implementation anyway. Any pre-9.3 client is going to be expecting the
definition that pqsignal() had before 9.3, which was that it didn't
set SA_RESTART for SIGALRM. Hence, put back pqsignal() in a separate
source file in src/interfaces/libpq, and give it the old semantics.
Back-patch to v12.
Discussion: https://postgr.es/m/E1g5vmT-0003K1-6S@gemulon.postgresql.org
Peter Eisentraut [Thu, 10 Oct 2019 08:51:11 +0000 (10:51 +0200)]
pg_upgrade: Clean up some redundant code
No need to call exit() after pg_fatal(). Clean up a few stragglers
for consistency.
Andres Freund [Thu, 10 Oct 2019 05:00:50 +0000 (22:00 -0700)]
Fix table rewrites that include a column without a default.
In
c2fe139c201c I made ATRewriteTable() use tuple slots. Unfortunately
I did not notice that columns can be added in a rewrite that do not
have a default, when another column is added/altered requiring one.
Initialize columns to NULL again, and add tests.
Bug: #16038
Reported-By: anonymous
Author: Andres Freund
Discussion: https://postgr.es/m/16038-
5c974541f2bf6749@postgresql.org
Backpatch: 12, where the bug was introduced in
c2fe139c201c