Tom Lane [Fri, 23 Oct 2020 23:08:01 +0000 (19:08 -0400)]
Fix more portability issues in new amcheck code.
verify_heapam() wasn't being careful to sanity-check tuple line
pointers before using them, resulting in SIGBUS on alignment-picky
architectures. Fix that, add some more test coverage.
Mark Dilger, some tweaking by me
Discussion: https://postgr.es/m/
30B8E99A-2D9C-48D4-A55C-
741C9D5F1563@enterprisedb.com
Tom Lane [Fri, 23 Oct 2020 21:07:15 +0000 (17:07 -0400)]
Allow psql to re-use connection parameters after a connection loss.
Instead of immediately PQfinish'ing a dead connection, save it aside
so that we can still extract its parameters for \connect attempts.
(This works because PQconninfo doesn't care whether the PGconn is in
CONNECTION_BAD state.) This allows developers to reconnect with
just \c after a database crash and restart.
It's tempting to use the same approach instead of closing the old
connection after a failed non-interactive \connect command. However,
that would not be very safe: consider a script containing
\c db1 user1 live_server
\c db2 user2 dead_server
\c db3
The script would be expecting to connect to db3 at dead_server, but
if we re-use parameters from the first connection then it might
successfully connect to db3 at live_server. This'd defeat the goal
of not letting a script accidentally execute commands against the
wrong database.
Discussion: https://postgr.es/m/38464.
1603394584@sss.pgh.pa.us
Tom Lane [Fri, 23 Oct 2020 18:02:00 +0000 (14:02 -0400)]
Fix portability issues in new amcheck test.
The tests added by commit
866e24d47 failed on big-endian machines
due to lack of attention to endianness considerations. Fix that.
While here, improve a few small cosmetic things, such as running
it through perltidy.
Mark Dilger
Discussion: https://postgr.es/m/
30B8E99A-2D9C-48D4-A55C-
741C9D5F1563@enterprisedb.com
Tom Lane [Fri, 23 Oct 2020 15:32:33 +0000 (11:32 -0400)]
Fix broken XML formatting in EXPLAIN output for incremental sorts.
The ExplainCloseGroup arguments for incremental sort usage data
didn't match the corresponding ExplainOpenGroup. This only matters
for XML-format output, which is probably why we'd not noticed.
Daniel Gustafsson, per bug #16683 from Frits Jalvingh
Discussion: https://postgr.es/m/16683-
8005033324ad34e9@postgresql.org
Peter Eisentraut [Fri, 23 Oct 2020 11:01:39 +0000 (13:01 +0200)]
doc: Fix order of protocol messages in listing
The order of AuthenticationGSSContinue and AuthenticationSSPI was
swapped, based on the other Authentication* protocol messages being
listed in subcode order.
Heikki Linnakangas [Fri, 23 Oct 2020 08:49:59 +0000 (11:49 +0300)]
doc: Remove reference to pre-8.2 pg_dump behaviour
The behavioural change in the -t/--table option happened around 15 years
ago and there seems little point in keeping it around.
Author: Ian Barwick
Discussion: https://www.postgresql.org/message-id/CAB8KJ%3Dh-XALik4M7gv-pX48%3D%2BSPWexfaYwa%2ByTnPwD3DxceXrg%40mail.gmail.com
Heikki Linnakangas [Fri, 23 Oct 2020 06:30:08 +0000 (09:30 +0300)]
Fix initialization of es_result_relations in EvalPlanQualStart().
Thinko in commit
1375422c782. EvalPlanQualStart() was mistakenly
resetting the parent EState's es_result_relations, when it should
initialize the field in the child EPQ EState it just created.
That was clearly wrong, but it didn't cause any ill effects, because
es_result_relations is currently not used after the ExecInit* phase.
Author: Amit Langote
Discussion: https://www.postgresql.org/message-id/CA%2BHiwqFEuq8AAAmxXsTDVZ1r38cHbfYuiPQx_%3DYyKe2DC-6q4A%40mail.gmail.com
Michael Paquier [Fri, 23 Oct 2020 02:05:46 +0000 (11:05 +0900)]
Improve performance of Unicode {de,re}composition in the backend
This replaces the existing binary search with two perfect hash functions
for the composition and the decomposition in the backend code, at the
cost of slightly-larger binaries there (35kB in libpgcommon_srv.a). Per
the measurements done, this improves the speed of the recomposition and
decomposition by up to 30~40 times for the NFC and NFKC conversions,
while all other operations get at least 40% faster. This is not as
"good" as what libicu has, but it closes the gap a lot as per the
feedback from Daniel Verite.
The decomposition table remains the same, getting used for the binary
search in the frontend code, where we care more about the size of the
libraries like libpq over performance as this gets involved only in code
paths related to the SCRAM authentication. In consequence, note that
the perfect hash function for the recomposition needs to use a new
inverse lookup array back to to the existing decomposition table.
The size of all frontend deliverables remains unchanged, even with
--enable-debug, including libpq.
Author: John Naylor
Reviewed-by: Michael Paquier, Tom Lane
Discussion: https://postgr.es/m/CAFBsxsHUuMFCt6-pU+oG-F1==CmEp8wR+O+bRouXWu6i8kXuqA@mail.gmail.com
Tom Lane [Fri, 23 Oct 2020 01:23:47 +0000 (21:23 -0400)]
Update time zone data files to tzdata release 2020d.
DST law changes in Palestine, with a whopping 120 hours' notice.
Also some historical corrections for Palestine.
Tom Lane [Fri, 23 Oct 2020 01:15:22 +0000 (21:15 -0400)]
Sync our copy of the timezone library with IANA release tzcode2020d.
There's no functional change at all here, but I'm curious to see
whether this change successfully shuts up Coverity's warning about
a useless strcmp(), which appeared with the previous update.
Discussion: http://mm.icann.org/pipermail/tz/2020-October/029370.html
Tom Lane [Thu, 22 Oct 2020 22:29:40 +0000 (18:29 -0400)]
Add documentation and tests for quote marks in ECPG literal queries.
ECPG's PREPARE ... FROM and EXECUTE IMMEDIATE can optionally take
the target query as a simple literal, rather than the more usual
string-variable reference. This was previously documented as
being a C string literal, but that's a lie in one critical respect:
you can't write a data double quote as \" in such literals. That's
because the lexer is in SQL mode at this point, so it'll parse
double-quoted strings as SQL identifiers, within which backslash
is not special, so \" ends the literal.
I looked into making this work as documented, but getting the lexer
to switch behaviors at just the right point is somewhere between
very difficult and impossible. It's not really worth the trouble,
because these cases are next to useless: if you have a fixed SQL
statement to execute or prepare, you might as well write it as
a direct EXEC SQL, saving the messiness of converting it into
a string literal and gaining the opportunity for compile-time
SQL syntax checking.
Instead, let's just document (and test) the workaround of writing
a double quote as an octal escape (\042) in such cases.
There's no code behavioral change here, so in principle this could
be back-patched, but it's such a niche case I doubt it's worth
the trouble.
Per report from 1250kv.
Discussion: https://postgr.es/m/673825.
1603223178@sss.pgh.pa.us
Tom Lane [Thu, 22 Oct 2020 21:34:32 +0000 (17:34 -0400)]
Avoid premature de-doubling of quote marks in ECPG strings.
If you write the literal 'abc''def' in an EXEC SQL command, that will
come out the other end as 'abc'def', triggering a syntax error in the
backend. Likewise, "abc""def" is reduced to "abc"def" which is wrong
syntax for a quoted identifier.
The cause is that the lexer thinks it should emit just one quote
mark, whereas what it really should do is keep the string as-is.
Add some docs and test cases, too.
Although this seems clearly a bug, I fear users wouldn't appreciate
changing it in minor releases. Some may well be working around it
by applying an extra doubling of affected quotes, as for example
sql/dyntest.pgc has been doing.
Per investigation of a report from 1250kv, although this isn't
exactly what he/she was on about.
Discussion: https://postgr.es/m/673825.
1603223178@sss.pgh.pa.us
Robert Haas [Thu, 22 Oct 2020 20:13:58 +0000 (16:13 -0400)]
Try to avoid a compiler warning about using fxid uninitialized.
Mark Dilger, with a couple of stray semicolons removed by me.
Discussion: http://postgr.es/m/
2A7DA1A8-C4AA-43DF-A985-
3CA52F4DC775@enterprisedb.com
Tom Lane [Thu, 22 Oct 2020 18:04:21 +0000 (14:04 -0400)]
Clean up some unpleasant behaviors in psql's \connect command.
The check for whether to complain about not having an old connection
to get parameters from was seriously out of date: it had not been
rethought when we invented connstrings, nor when we invented the
-reuse-previous option. Replace it with a check that throws an
error if reuse-previous is active and we lack an old connection to
reuse. While that doesn't move the goalposts very far in terms of
easing reconnection after a server crash, at least it's consistent.
If the user specifies a connstring plus additional parameters
(which is invalid per the documentation), the extra parameters were
silently ignored. That seems like it could be really confusing,
so let's throw a syntax error instead.
Teach the connstring code path to re-use the old connection's password
in the same cases as the old-style-syntax code path would, ie if we
are reusing parameters and the values of username, host/hostaddr, and
port are not being changed. Document this behavior, too, since it was
unmentioned before. Also simplify the implementation a bit, giving
rise to two new and useful properties: if there's a "password=xxx" in
the connstring, we'll use it not ignore it, and by default (i.e.,
except with --no-password) we will prompt for a password if the
re-used password or connstring password doesn't work. The previous
code just failed if the re-used password didn't work.
Given the paucity of field complaints about these issues, I don't
think that they rise to the level of back-patchable bug fixes,
and in any case they might represent undesirable behavior changes
in minor releases. So no back-patch.
Discussion: https://postgr.es/m/235210.
1603321144@sss.pgh.pa.us
Robert Haas [Thu, 22 Oct 2020 12:44:18 +0000 (08:44 -0400)]
Extend amcheck to check heap pages.
Mark Dilger, reviewed by Peter Geoghegan, Andres Freund, Álvaro Herrera,
Michael Paquier, Amul Sul, and by me. Some last-minute cosmetic
revisions by me.
Discussion: http://postgr.es/m/
12ED3DA8-25F0-4B68-937D-
D907CFBF08E7@enterprisedb.com
Peter Eisentraut [Thu, 22 Oct 2020 11:29:39 +0000 (13:29 +0200)]
Use croak instead of die in Perl code when appropriate
David Rowley [Thu, 22 Oct 2020 01:36:32 +0000 (14:36 +1300)]
Optimize a few list_delete_ptr calls
There is a handful of places where we called list_delete_ptr() to remove
some element from a List. In many of these places we know, or with very
little additional effort know the index of the ListCell that we need to
remove.
Here we change all of those places to instead either use one of;
list_delete_nth_cell(), foreach_delete_current() or list_delete_last().
Each of these saves from having to iterate over the list to search for the
element to remove by its pointer value.
There are some small performance gains to be had by doing this, but in the
general case, none of these lists are likely to be very large, so the
lookup was probably never that expensive anyway. However, some of the
calls are in fairly hot code paths, e.g process_equivalence(). So any
small gains there are useful.
Author: Zhijie Hou and David Rowley
Discussion: https://postgr.es/m/
b3517353ec7c4f87aa560678fbb1034b@G08CNEXMBPEKD05.g08.fujitsu.local
Tom Lane [Wed, 21 Oct 2020 20:18:40 +0000 (16:18 -0400)]
Fix connection string handling in psql's \connect command.
psql's \connect claims to be able to re-use previous connection
parameters, but in fact it only re-uses the database name, user name,
host name (and possibly hostaddr, depending on version), and port.
This is problematic for assorted use cases. Notably, pg_dump[all]
emits "\connect databasename" commands which we would like to have
re-use all other parameters. If such a script is loaded in a psql run
that initially had "-d connstring" with some non-default parameters,
those other parameters would be lost, potentially causing connection
failure. (Thus, this is the same kind of bug addressed in commits
a45bc8a4f and
8e5793ab6, although the details are much different.)
To fix, redesign do_connect() so that it pulls out all properties
of the old PGconn using PQconninfo(), and then replaces individual
properties in that array. In the case where we don't wish to re-use
anything, get libpq's default settings using PQconndefaults() and
replace entries in that, so that we don't need different code paths
for the two cases.
This does result in an additional behavioral change for cases where
the original connection parameters allowed multiple hosts, say
"psql -h host1,host2", and the \connect request allows re-use of the
host setting. Because the previous coding relied on PQhost(), it
would only permit reconnection to the same host originally selected.
Although one can think of scenarios where that's a good thing, there
are others where it is not. Moreover, that behavior doesn't seem to
meet the principle of least surprise, nor was it documented; nor is
it even clear it was intended, since that coding long pre-dates the
addition of multi-host support to libpq. Hence, this patch is content
to drop it and re-use the host list as given.
Per Peter Eisentraut's comments on bug #16604. Back-patch to all
supported branches.
Discussion: https://postgr.es/m/16604-
933f4b8791227b15@postgresql.org
Alvaro Herrera [Wed, 21 Oct 2020 17:37:26 +0000 (14:37 -0300)]
Use fast checkpoint in PostgresNode::backup()
Should cause tests to be a bit faster
Tom Lane [Wed, 21 Oct 2020 16:08:48 +0000 (12:08 -0400)]
Remove the option to build thread_test.c outside configure.
Theoretically one could go into src/test/thread and build/run this
program there. In practice, that hasn't worked since
96bf88d52,
and probably much longer on some platforms (likely including just
the sort of hoary leftovers where this test might be of interest).
While it wouldn't be too hard to repair the breakage, the fact that
nobody has noticed for two years shows that there is zero usefulness
in maintaining this build pathway. Let's get rid of it and decree
that thread_test.c is *only* meant to be built/used in configure.
Given that decision, it makes sense to put thread_test.c under config/
and get rid of src/test/thread altogether, so that's what I did.
In passing, update src/test/README, which had been ignored by some
not-so-recent additions of subdirectories.
Discussion: https://postgr.es/m/227659.
1603041612@sss.pgh.pa.us
Peter Eisentraut [Wed, 21 Oct 2020 12:20:50 +0000 (14:20 +0200)]
Remove obsolete ifdefs
Commit
8dace66e0735ca39b779922d02c24ea2686e6521 added #ifdefs for a
number of errno symbols because they were not present on Windows.
Later, commit
125ad539a275db5ab8f4647828b80a16d02eabd2 added
replacement #defines for some of those symbols. So some of the
changes from the first commit are made dead code by the second commit
and can now be removed.
Discussion: https://www.postgresql.org/message-id/flat/
6dee8574-b0ad-fc49-9c8c-
2edc796f0033@2ndquadrant.com
Peter Eisentraut [Wed, 21 Oct 2020 06:17:51 +0000 (08:17 +0200)]
Fix -Wcast-function-type warnings on Windows/MinGW
After
de8feb1f3a23465b5737e8a8c160e8ca62f61339, some warnings remained
that were only visible when using GCC on Windows. Fix those as well.
Note that the ecpg test source files don't use the full pg_config.h,
so we can't use pg_funcptr_t there but have to do it the long way.
Michael Paquier [Wed, 21 Oct 2020 00:22:27 +0000 (09:22 +0900)]
Review format of code generated by PerfectHash.pm
80f8eb7 has added to the normalization quick check headers some code
generated by PerfectHash.pm that is incompatible with the settings of
gitattributes for this repository, as whitespaces followed a set of tabs
for the first element of a line in the table. Instead of adding a new
exception to gitattributes, rework the format generated so as a right
padding with spaces is used instead of a left padding. This keeps the
table generated in a readable shape with its set of columns, making
unnecessary an update of gitattributes.
Reported-by: Peter Eisentraut
Author: John Naylor
Discussion: https://postgr.es/m/
d601b3b5-a3c7-5457-2f84-
3d6513d690fc@2ndquadrant.com
Alvaro Herrera [Tue, 20 Oct 2020 22:22:09 +0000 (19:22 -0300)]
Fix ALTER TABLE .. ENABLE/DISABLE TRIGGER recursion
More precisely, correctly handle the ONLY flag indicating not to
recurse. This was implemented in
86f575948c77 by recursing in
trigger.c, but that's the wrong place; use ATSimpleRecursion instead,
which behaves properly. However, because legacy inheritance has never
recursed in that situation, make sure to do that only for new-style
partitioning.
I noticed this problem while testing a fix for another bug in the
vicinity.
This has been wrong all along, so backpatch to 11.
Discussion: https://postgr.es/m/
20201016235925[email protected]
Amit Kapila [Tue, 20 Oct 2020 04:54:36 +0000 (10:24 +0530)]
Change the attribute name in pg_stat_replication_slots view.
Change the attribute 'name' to 'slot_name' in pg_stat_replication_slots
view to make it clear and that way we will be consistent with the other
places like pg_stat_wal_receiver view where we display the same attribute.
In the passing, fix the typo in one of the macros in the related code.
Bump the catversion as we have modified the name in the catalog as well.
Reported-by: Noriyoshi Shinoda
Author: Noriyoshi Shinoda
Reviewed-by: Sawada Masahiko and Amit Kapila
Discussion: https://postgr.es/m/CA+fd4k5_pPAYRTDrO2PbtTOe0eHQpBvuqmCr8ic39uTNmR49Eg@mail.gmail.com
Tom Lane [Mon, 19 Oct 2020 23:03:46 +0000 (19:03 -0400)]
Fix connection string handling in src/bin/scripts/ programs.
When told to process all databases, clusterdb, reindexdb, and vacuumdb
would reconnect by replacing their --maintenance-db parameter with the
name of the target database. If that parameter is a connstring (which
has been allowed for a long time, though we failed to document that
before this patch), we'd lose any other options it might specify, for
example SSL or GSS parameters, possibly resulting in failure to connect.
Thus, this is the same bug as commit
a45bc8a4f fixed in pg_dump and
pg_restore. We can fix it in the same way, by using libpq's rules for
handling multiple "dbname" parameters to add the target database name
separately. I chose to apply the same refactoring approach as in that
patch, with a struct to handle the command line parameters that need to
be passed through to connectDatabase. (Maybe someday we can unify the
very similar functions here and in pg_dump/pg_restore.)
Per Peter Eisentraut's comments on bug #16604. Back-patch to all
supported branches.
Discussion: https://postgr.es/m/16604-
933f4b8791227b15@postgresql.org
Tom Lane [Mon, 19 Oct 2020 18:33:01 +0000 (14:33 -0400)]
Fix list-munging bug that broke SQL function result coercions.
Since commit
913bbd88d, check_sql_fn_retval() can either insert type
coercion steps in-line in the Query that produces the SQL function's
results, or generate a new top-level Query to perform the coercions,
if modifying the Query's output in-place wouldn't be safe. However,
it appears that the latter case has never actually worked, because
the code tried to inject the new Query back into the query list it was
passed ... which is not the list that will be used for later processing
when we execute the SQL function "normally" (without inlining it).
So we ended up with no coercion happening at run-time, leading to
wrong results or crashes depending on the datatypes involved.
While the regression tests look like they cover this area well enough,
through a huge bit of bad luck all the test cases that exercise the
separate-Query path were checking either inline-able cases (which
accidentally didn't have the bug) or cases that are no-ops at runtime
(e.g., varchar to text), so that the failure to perform the coercion
wasn't obvious. The fact that the cases that don't work weren't
allowed at all before v13 probably contributed to not noticing the
problem sooner, too.
To fix, get rid of the separate "flat" list of Query nodes and instead
pass the real two-level list that is going to be used later. I chose
to make the same change in check_sql_fn_statements(), although that has
no actual bug, just so that we don't need that data structure at all.
This is an API change, as evidenced by the adjustments needed to
callers outside functions.c. That's a bit scary to be doing in a
released branch, but so far as I can tell from a quick search,
there are no outside callers of these functions (and they are
sufficiently specific to our semantics for SQL-language functions that
it's not apparent why any extension would need to call them). In any
case, v13 already changed the API of check_sql_fn_retval() compared to
prior branches.
Per report from pinker. Back-patch to v13 where this code came in.
Discussion: https://postgr.es/m/
1603050466566[email protected]
Heikki Linnakangas [Mon, 19 Oct 2020 16:28:54 +0000 (19:28 +0300)]
Misc documentation fixes.
- Misc grammar and punctuation fixes.
- Stylistic cleanup: use spaces between function arguments and JSON fields
in examples. For example "foo(a,b)" -> "foo(a, b)". Add semicolon after
last END in a few PL/pgSQL examples that were missing them.
- Make sentence that talked about "..." and ".." operators more clear,
by avoiding to end the sentence with "..". That makes it look the same
as "..."
- Fix syntax description for HAVING: HAVING conditions cannot be repeated
Patch by Justin Pryzby, per Yaroslav Schekin's report. Backpatch to all
supported versions, to the extent that the patch applies easily.
Discussion: https://www.postgresql.org/message-id/
20201005191922.GE17626%40telsasoft.com
Heikki Linnakangas [Mon, 19 Oct 2020 16:02:25 +0000 (19:02 +0300)]
Fix TRUNCATE doc: ALTER SEQUENCE RESTART is now transactional.
ALTER SEQUENCE RESTART was made transactional in commit
3d79013b97.
Backpatch to v10, where that was introduced.
Patch by Justin Pryzby, per Yaroslav Schekin's report.
Discussion: https://www.postgresql.org/message-id/
20201005191922.GE17626%40telsasoft.com
Heikki Linnakangas [Mon, 19 Oct 2020 15:50:33 +0000 (18:50 +0300)]
Fix output of tsquery example in docs.
The output for this query changed in commit
4e2477b7b8. Backport to 9.6
like that commit.
Patch by Justin Pryzby, per Yaroslav Schekin's report.
Discussion: https://www.postgresql.org/message-id/
20201005191922.GE17626%40telsasoft.com
Heikki Linnakangas [Mon, 19 Oct 2020 14:58:38 +0000 (17:58 +0300)]
Fix doc for full text search distance operator.
Commit
028350f619 changed its behavior from "at most" to "exactly", but
forgot to update the documentation. Backpatch to 9.6.
Patch by Justin Pryzby, per Yaroslav Schekin's report.
Discussion: https://www.postgresql.org/message-id/
20201005191922.GE17626%40telsasoft.com
Magnus Hagander [Mon, 19 Oct 2020 11:47:09 +0000 (13:47 +0200)]
Update link for pllua
Author: Daniel Gustafsson <
[email protected]>
Discussion: https://postgr.es/m/
A05874AE-8771-4C61-A24E-
0B6249B8F3C2@yesql.se
Heikki Linnakangas [Mon, 19 Oct 2020 11:11:57 +0000 (14:11 +0300)]
Remove PartitionRoutingInfo struct.
The extra indirection neeeded to access its members via its enclosing
ResultRelInfo seems pointless. Move all the fields from
PartitionRoutingInfo to ResultRelInfo.
Author: Amit Langote
Reviewed-by: Alvaro Herrera
Discussion: https://www.postgresql.org/message-id/CA%2BHiwqFViT47Zbr_ASBejiK7iDG8%3DQ1swQ-tjM6caRPQ67pT%3Dw%40mail.gmail.com
Heikki Linnakangas [Mon, 19 Oct 2020 11:11:54 +0000 (14:11 +0300)]
Revise child-to-root tuple conversion map management.
Store the tuple conversion map to convert a tuple from a child table's
format to the root format in a new ri_ChildToRootMap field in
ResultRelInfo. It is initialized if transition tuple capture for FOR
STATEMENT triggers or INSERT tuple routing on a partitioned table is
needed. Previously, ModifyTable kept the maps in the per-subplan
ModifyTableState->mt_per_subplan_tupconv_maps array, or when tuple
routing was used, in
ResultRelInfo->ri_Partitioninfo->pi_PartitionToRootMap. The new field
replaces both of those.
Now that the child-to-root tuple conversion map is always available in
ResultRelInfo (when needed), remove the TransitionCaptureState.tcs_map
field. The callers of Exec*Trigger() functions no longer need to set or
save it, which is much less confusing and bug-prone. Also, as a future
optimization, this will allow us to delay creating the map for a given
result relation until the relation is actually processed during
execution.
Author: Amit Langote
Discussion: https://www.postgresql.org/message-id/CA%2BHiwqHtCWLdK-LO%3DNEsvOdHx%2B7yv4mE_zYK0i3BH7dXb-wxog%40mail.gmail.com
Heikki Linnakangas [Mon, 19 Oct 2020 11:11:44 +0000 (14:11 +0300)]
Clean up code to resolve the "root target relation" in nodeModifyTable.c
When executing DDL on a partitioned table or on a table with inheritance
children, statement-level triggers must be fired against the table given
in the original statement. The code to look that up was a bit messy and
duplicative. Commit
501ed02cf6 added a helper function,
getASTriggerResultRelInfo() (later renamed to getTargetResultRelInfo())
for it, but for some reason it was only used when firing AFTER STATEMENT
triggers and the code to fire BEFORE STATEMENT triggers duplicated the
logic.
Determine the target relation in ExecInitModifyTable(), and set it always
in ModifyTableState. Code that used to call getTargetResultRelInfo() can
now use ModifyTableState->rootResultRelInfo directly.
Discussion: https://www.postgresql.org/message-id/CA%2BHiwqFViT47Zbr_ASBejiK7iDG8%3DQ1swQ-tjM6caRPQ67pT%3Dw%40mail.gmail.com
Peter Eisentraut [Mon, 19 Oct 2020 06:52:25 +0000 (08:52 +0200)]
Avoid invalid alloc size error in shm_mq
In shm_mq_receive(), a huge payload could trigger an unjustified
"invalid memory alloc request size" error due to the way the buffer
size is increased.
Add error checks (documenting the upper limit) and avoid the error by
limiting the allocation size to MaxAllocSize.
Author: Markus Wanner <
[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/
3bb363e7-ac04-0ac4-9fe8-
db1148755bfa%402ndquadrant.com
Peter Eisentraut [Mon, 19 Oct 2020 06:32:53 +0000 (08:32 +0200)]
Improve whitespace
The SQL file was using a mix of tabs and spaces inconsistently.
Convert all to spaces and adjust indentation accordingly.
Amit Kapila [Mon, 19 Oct 2020 03:43:17 +0000 (09:13 +0530)]
Change the docs for PARALLEL option of Vacuum.
The rules to choose the number of parallel workers to perform parallel
vacuum operation were not clearly specified.
Reported-by: Peter Eisentraut
Author: Amit Kapila
Backpatch-through: 13, where it was introduced
Discussion: https://postgr.es/m/
36aa8aea-61b7-eb3c-263b-
648e0cb117b7@2ndquadrant.com
Michael Paquier [Mon, 19 Oct 2020 00:36:56 +0000 (09:36 +0900)]
Fix potential memory leak in pgcrypto
When allocating a EVP context, it would have been possible to leak some
memory allocated directly by OpenSSL, that PostgreSQL lost track of if
the initialization of the context allocated failed. The cleanup can be
done with EVP_MD_CTX_destroy().
Note that EVP APIs exist since OpenSSL 0.9.7 and we have in the tree
equivalent implementations for older versions since
ce9b75d (code
removed with
9b7cd59a as of 10~). However, in 9.5 and 9.6, the existing
code makes use of EVP_MD_CTX_destroy() and EVP_MD_CTX_create() without
an equivalent implementation when building the tree with OpenSSL 0.9.6
or older, meaning that this code is in reality broken with such versions
since it got introduced in
e2838c5. As we have heard no complains about
that, it does not seem worth bothering with in 9.5 and 9.6, so I have
left that out for simplicity.
Author: Michael Paquier
Discussion: https://postgr.es/m/
20201015072212[email protected]
Backpatch-through: 9.5
David Rowley [Sun, 18 Oct 2020 21:53:52 +0000 (10:53 +1300)]
Prevent overly large and NaN row estimates in relations
Given a query with enough joins, it was possible that the query planner,
after multiplying the row estimates with the join selectivity that the
estimated number of rows would exceed the limits of the double data type
and become infinite.
To give an indication on how extreme a case is required to hit this, the
particular example case reported required 379 joins to a table without any
statistics, which resulted in the 1.0/DEFAULT_NUM_DISTINCT being used for
the join selectivity. This eventually caused the row estimates to go
infinite and resulted in an assert failure in initial_cost_mergejoin()
where the infinite row estimated was multiplied by an outerstartsel of 0.0
resulting in NaN. The failing assert verified that NaN <= Inf, which is
false.
To get around this we use clamp_row_est() to cap row estimates at a
maximum of 1e100. This value is thought to be low enough that costs
derived from it would remain within the bounds of what the double type can
represent.
Aside from fixing the failing Assert, this also has the added benefit of
making it so add_path() will still receive proper numerical values as
costs which will allow it to make more sane choices when determining the
cheaper path in extreme cases such as the one described above.
Additionally, we also get rid of the isnan() checks in the join costing
functions. The actual case which originally triggered those checks to be
added in the first place never made it to the mailing lists. It seems
likely that the new code being added to clamp_row_est() will result in
those becoming checks redundant, so just remove them.
The fairly harmless assert failure problem does also exist in the
backbranches, however, a more minimalistic fix will be applied there.
Reported-by: Onder Kalaci
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/DM6PR21MB1211FF360183BCA901B27F04D80B0@DM6PR21MB1211.namprd21.prod.outlook.com
Tom Lane [Sun, 18 Oct 2020 16:56:43 +0000 (12:56 -0400)]
Update the Winsock API version requested by libpq.
According to Microsoft's documentation, 2.2 has been the current
version since Windows 98 or so. Moreover, that's what the Postgres
backend has been requesting since 2004 (cf commit
4cdf51e64).
So there seems no reason for libpq to keep asking for 1.1.
Bring thread_test along, too, so that we're uniformly asking for 2.2
in all our WSAStartup calls.
It's not clear whether there's any point in back-patching this,
so for now I didn't.
Discussion: https://postgr.es/m/132799.
1602960277@sss.pgh.pa.us
Tom Lane [Sun, 18 Oct 2020 16:26:02 +0000 (12:26 -0400)]
In pg_restore's dump_lo_buf(), work a little harder on error handling.
Failure to write data to a large object during restore led to an ugly
and uninformative error message. To add insult to injury, it then
fatal'd out, where other SQL-level errors usually result in pressing on.
Report the underlying error condition, rather than just giving not-very-
useful byte counts, and use warn_or_exit_horribly() so as to adhere to
pg_restore's general policy about whether to continue or not.
Also recognize that lo_write() returns int not size_t.
Per report from Justin Pryzby, though I didn't use his patch.
Given the lack of comparable complaints, I'm not sure this is
worth back-patching.
Discussion: https://postgr.es/m/
20201018010232[email protected]
Tom Lane [Sat, 17 Oct 2020 20:53:48 +0000 (16:53 -0400)]
In libpq for Windows, call WSAStartup once and WSACleanup not at all.
The Windows documentation insists that every WSAStartup call should
have a matching WSACleanup call. However, if that ever had actual
relevance, it wasn't in this century. Every remotely-modern Windows
kernel is capable of cleaning up when a process exits without doing
that, and must be so to avoid resource leaks in case of a process
crash. Moreover, Postgres backends have done WSAStartup without
WSACleanup since commit
4cdf51e64 in 2004, and we've never seen any
indication of a problem with that.
libpq's habit of doing WSAStartup during connection start and
WSACleanup during shutdown is also rather inefficient, since a
series of non-overlapping connection requests leads to repeated,
quite expensive DLL unload/reload cycles. We document a workaround
for that (having the application call WSAStartup for itself), but
that's just a kluge. It's also worth noting that it's far from
uncommon for applications to exit without doing PQfinish, and
we've not heard reports of trouble from that either.
However, the real reason for acting on this is that recent
experiments by Alexander Lakhin suggest that calling WSACleanup
during PQfinish might be triggering the symptom we occasionally see
that a process using libpq fails to emit expected stdio output.
Therefore, let's change libpq so that it calls WSAStartup only
once per process, during the first connection attempt, and never
calls WSACleanup at all.
While at it, get rid of the only other WSACleanup call in our code
tree, in pg_dump/parallel.c; that presumably is equally useless.
If this proves to suppress the fairly-common ecpg test failures
we see on Windows, I'll back-patch, but for now let's just do it
in HEAD and see what happens.
Discussion: https://postgr.es/m/
ac976d8c-03df-d6b8-025c-
15a2de8d9af1@postgrespro.ru
Tom Lane [Sat, 17 Oct 2020 20:02:47 +0000 (16:02 -0400)]
Doc: caution against misuse of 'now' and related datetime literals.
Section 8.5.1.4, which defines these literals, made only a vague
reference to the fact that they might be evaluated too soon to be
safe in non-interactive contexts. Provide a more explicit caution
against misuse. Also, generalize the wording in the related tip in
section 9.9.4: while it clearly described this problem, it implied
(or really, stated outright) that the problem only applies to table
DEFAULT clauses.
Per gripe from Tijs van Dam. Back-patch to all supported branches.
Discussion: https://postgr.es/m/c2LuRv9BiRT3bqIo5mMQiVraEXey_25B4vUn0kDqVqilwOEu_iVF1tbtvLnyQK7yDG3PFaz_GxLLPil2SDkj1MCObNRVaac-7j1dVdFERk8=@thalex.com
Tom Lane [Sat, 17 Oct 2020 01:53:33 +0000 (21:53 -0400)]
Update time zone data files to tzdata release 2020c.
DST law changes in Morocco, Canadian Yukon, Fiji, Macquarie Island,
Casey Station (Antarctica). Historical corrections for France,
Hungary, Monaco.
Tom Lane [Sat, 17 Oct 2020 01:40:16 +0000 (21:40 -0400)]
Sync our copy of the timezone library with IANA release tzcode2020c.
This changes zic's default output format from "-b fat" to "-b slim".
We were already using "slim" in v13/HEAD, so those branches drop
the explicit -b switch in the Makefiles. Instead, add an explicit
"-b fat" in v12 and before, so that we don't change the output file
format in those branches. (This is perhaps excessively conservative,
but we decided not to do so in
a12079109, and I'll stick with that.)
Other non-cosmetic changes are to drop support for zic's long-obsolete
"-y" switch, and to ensure that strftime() does not change errno
unless it fails.
As usual with tzcode changes, back-patch to all supported branches.
Tom Lane [Fri, 16 Oct 2020 15:59:13 +0000 (11:59 -0400)]
Add missing error check in pgcrypto/crypt-md5.c.
In theory, the second px_find_digest call in px_crypt_md5 could fail
even though the first one succeeded, since resource allocation is
required. Don't skip testing for a failure. (If one did happen,
the likely result would be a crash rather than clean recovery from
an OOM failure.)
The code's been like this all along, so back-patch to all supported
branches.
Daniel Gustafsson
Discussion: https://postgr.es/m/
AA8D6FE9-4AB2-41B4-98CB-
AE64BA668C03@yesql.se
Tom Lane [Fri, 16 Oct 2020 15:36:34 +0000 (11:36 -0400)]
Doc: tweak column widths in synchronous-commit-matrix table.
Commit
a97e85f2b caused "exceed the available area" warnings in PDF
builds. Fine-tune colwidth values to avoid that.
Back-patch to 9.6, like the prior patch. (This is of dubious value
before v13, since we were far from free of such warnings in older
branches. But we might as well keep the SGML looking the same in all
branches.)
Per buildfarm.
Fujii Masao [Fri, 16 Oct 2020 04:58:45 +0000 (13:58 +0900)]
postgres_fdw: Restructure connection retry logic.
Commit
32a9c0bdf introduced connection retry logic into postgres_fdw.
Previously it used goto statement for retry. This commit gets rid of that
goto from the logic to make the code simpler and easier-to-read.
When getting out of PG_CATCH() for the retry, the error state should be
cleaned up and the memory context should be reset. But commit
32a9c0bdf
forgot to do that. This commit also fixes this bug.
Previously only PQstatus()==CONNECTION_BAD was verified to detect
connection failure. But this could cause false detection in the case where
any error other than connection failure (e.g., out-of-memory) was thrown
after a broken connection was detected in libpq and CONNECTION_BAD is set.
To fix this issue, this commit changes the logic so that it also checks
the error's sqlstate is ERRCODE_CONNECTION_FAILURE.
Author: Fujii Masao
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/
2943611.
1602375376@sss.pgh.pa.us
Andres Freund [Fri, 16 Oct 2020 00:38:00 +0000 (17:38 -0700)]
llvmjit: Work around bug in LLVM 3.9 causing crashes after
72559438f92.
Unfortunately in LLVM 3.9 LLVMGetAttributeCountAtIndex(func, index)
crashes when called with an index that has 0 attributes. Since there's
no way to work around this in the C API, add a small C++ wrapper doing
so.
The only reason this didn't fail before
72559438f92 is that there
always are function attributes...
Author: Andres Freund <
[email protected]>
Discussion: https://postgr.es/m/
20201016001254[email protected]
Backpatch: 11-, like
72559438f92
Bruce Momjian [Fri, 16 Oct 2020 00:37:20 +0000 (20:37 -0400)]
pg_upgrade: remove C99 compiler req. from commit
3c0471b5fd
This commit required support for inline variable definition, which is
not a requirement.
RELEASE NOTE AUTHOR: the author of commit
3c0471b5fd
(pg_upgrade/tablespaces) was Justin Pryzby, not me.
Reported-by: Andres Freund
Discussion: https://postgr.es/m/
20201016001959[email protected]
Backpatch-through: 9.5
Bruce Momjian [Thu, 15 Oct 2020 23:33:36 +0000 (19:33 -0400)]
pg_upgrade: generate check error for left-over new tablespace
Previously, if pg_upgrade failed, and the user recreated the cluster but
did not remove the new cluster tablespace directory, a later pg_upgrade
would fail since the new tablespace directory would already exists.
This adds error reporting for this during check.
Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/
20200925005531[email protected]
Backpatch-through: 9.5
Andres Freund [Thu, 15 Oct 2020 20:39:41 +0000 (13:39 -0700)]
llvmjit: Also copy parameter / return value attributes from template functions.
Previously we only copied the function attributes. That caused problems at
least on s390x: Because we didn't copy the 'zeroext' attribute for
ExecAggTransReparent()'s *IsNull parameters, expressions invoking it didn't
ensure that the upper bytes of the registers were zeroed. In the - relatively
rare - cases where not, ExecAggTransReparent() wrongly ended up in the
newValueIsNull branch due to the register not being zero. Subsequently causing
a crash.
It's quite possible that this would cause problems on other platforms, and in
other places than just ExecAggTransReparent() on s390x.
Thanks to Christoph (and the Debian project) for providing me with access to a
s390x machine, allowing me to debug this.
Reported-By: Christoph Berg
Author: Andres Freund
Discussion: https://postgr.es/m/
20201015083246[email protected]
Backpatch: 11-, where JIT was added
Bruce Momjian [Thu, 15 Oct 2020 19:15:29 +0000 (15:15 -0400)]
doc: improve description of synchronous_commit modes
Previously it wasn't clear exactly what each of the synchronous_commit
modes accomplished. This clarifies that, and adds a table describing it.
Only backpatched through 9.6 since 9.5 doesn't have all the options.
Reported-by: [email protected]
Discussion: https://postgr.es/m/
159741195522.14321.
13812604195366728976@wrigleys.postgresql.org
Backpatch-through: 9.6
Alvaro Herrera [Thu, 15 Oct 2020 18:16:11 +0000 (15:16 -0300)]
Revert "Remove pointless HeapTupleHeaderIndicatesMovedPartitions calls"
This reverts commit
85adb5e91ec2. It was not intended for commit just
yet.
Alvaro Herrera [Thu, 15 Oct 2020 17:32:34 +0000 (14:32 -0300)]
Remove pointless HeapTupleHeaderIndicatesMovedPartitions calls
Pavan Deolasee recently noted that a few of the
HeapTupleHeaderIndicatesMovedPartitions calls added by commit
5db6df0c0117 are useless, since they are done after comparing t_self
with t_ctid. But because t_self can never be set to the magical values
that indicate that the tuple moved partition, this can never succeed: if
the first test fails (so we know t_self equals t_ctid), necessarily the
second test will also fail.
So these checks can be removed and no harm is done.
Discussion: https://postgr.es/m/
20200929164411[email protected]
Alvaro Herrera [Thu, 15 Oct 2020 16:09:29 +0000 (13:09 -0300)]
Install pg_isolation_regress and isolationtester
We already install assorted tools for testing extensions, but these two
were missing. Having them installed, and after ISOLATION support was
added to PGXS's makefiles by
d3c09b9b1307, helps third-party modules
usefully include isolation tests. Compare
c3a0818460a8.
Author: Craig Ringer <
[email protected]>
Reviewed-by: Álvaro Herrera <[email protected]>
Discussion: https://postgr.es/m/CAMsr+YFsCMH3B4uOPFE+2qWM6k=o=hf9LGiPNCfwqKdUPz_BsQ@mail.gmail.com
Alvaro Herrera [Thu, 15 Oct 2020 14:33:48 +0000 (11:33 -0300)]
Review logical replication tablesync code
Most importantly, remove optimization in LogicalRepSyncTableStart that
skips the normal walrcv_startstreaming/endstreaming dance. The
optimization is not critically important for production uses anyway,
since it only fires in cases with no activity, and saves an
uninteresting amount of work even then. Critically, it obscures bugs by
hiding the interesting code path from test cases.
Also: in GetSubscriptionRelState, remove pointless relation open; access
pg_subscription_rel->srsubstate with GETSTRUCT as is typical rather than
SysCacheGetAttr; remove unused 'missing_ok' argument.
In wait_for_relation_state_change, use explicit catalog snapshot
invalidation rather than obscurely (and expensively) through
GetLatestSnapshot.
In various places: sprinkle comments more liberally and rewrite a number
of them. Other cosmetic code improvements.
No backpatch, since no bug is being fixed here.
Author: Álvaro Herrera <
[email protected]>
Reviewed-by: Petr Jelínek <[email protected]>
Discussion: https://postgr.es/m/
20201010190637[email protected]
Heikki Linnakangas [Thu, 15 Oct 2020 14:08:10 +0000 (17:08 +0300)]
Refactor code for cross-partition updates to a separate function.
ExecUpdate() is very long, so extract the part of it that deals with
cross-partition updates to a separate function to make it more readable.
Per Andres Freund's suggestion.
Author: Amit Langote
Discussion: https://www.postgresql.org/message-id/CA%2BHiwqEUgb5RdUgxR7Sqco4S09jzJstHiaT2vnCRPGR4JCAPqA%40mail.gmail.com
Alvaro Herrera [Thu, 15 Oct 2020 12:48:36 +0000 (09:48 -0300)]
Fix query in new test to check tables are synced
Rather than looking for tablesync workers, it is more reliable to see
the sync state of the tables.
Per note from Amit Kapila.
Discussion: https://postgr.es/m/CAA4eK1JSSD7FVwq+_rOme86jUZTQFzjsNU06hQ4-LiRt1xFmSg@mail.gmail.com
Michael Paquier [Thu, 15 Oct 2020 08:03:56 +0000 (17:03 +0900)]
Replace calls of htonl()/ntohl() with pg_bswap.h for GSSAPI encryption
The in-core equivalents can make use of built-in functions if the
compiler supports this option, making optimizations possible.
0ba99c8
replaced all existing calls in the code base at this time, but
b0b39f7
(GSSAPI encryption) has forgotten to do the switch.
Discussion: https://postgr.es/m/
20201014055303[email protected]
Fujii Masao [Thu, 15 Oct 2020 07:50:57 +0000 (16:50 +0900)]
Improve tab-completion for FETCH/MOVE.
Author: Naoki Nakamichi
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/
d05a46b599634ca0d94144387507f4b4@oss.nttdata.com
David Rowley [Thu, 15 Oct 2020 07:35:17 +0000 (20:35 +1300)]
Fixup some appendStringInfo and appendPQExpBuffer calls
A number of places were using appendStringInfo() when they could have been
using appendStringInfoString() instead. While there's no functionality
change there, it's just more efficient to use appendStringInfoString()
when no formatting is required. Likewise for some
appendStringInfoString() calls which were just appending a single char.
We can just use appendStringInfoChar() for that.
Additionally, many places were using appendPQExpBuffer() when they could
have used appendPQExpBufferStr(). Change those too.
Patch by Zhijie Hou, but further searching by me found significantly more
places that deserved the same treatment.
Author: Zhijie Hou, David Rowley
Discussion: https://postgr.es/m/
cb172cf4361e4c7ba7167429070979d4@G08CNEXMBPEKD05.g08.fujitsu.local
Peter Eisentraut [Thu, 15 Oct 2020 06:54:16 +0000 (08:54 +0200)]
Add documentation link to attributes supported by Clang
Thomas Munro [Thu, 15 Oct 2020 05:23:30 +0000 (18:23 +1300)]
Handle EACCES errors from kevent() better.
While registering for postmaster exit events, we have to handle a couple
of edge cases where the postmaster is already gone. Commit
815c2f09
missed one: EACCES must surely imply that PostmasterPid no longer
belongs to our postmaster process (or alternatively an unexpected
permissions model has been imposed on us). Like ESRCH, this should be
treated as a WL_POSTMASTER_DEATH event, rather than being raised with
ereport().
No known problems reported in the wild. Per code review from Tom Lane.
Back-patch to 13.
Reported-by: Tom Lane <[email protected]>
Discussion: https://postgr.es/m/
3624029.
1602701929%40sss.pgh.pa.us
Amit Kapila [Thu, 15 Oct 2020 02:47:51 +0000 (08:17 +0530)]
Execute invalidation messages for each XLOG_XACT_INVALIDATIONS message
during logical decoding.
Prior to commit
c55040ccd0 we have no way of knowing the invalidations
before commit. So, while decoding we use to execute all the invalidations
at each command end as we had no way of knowing which invalidations
happened before that command. Due to this, transactions involving large
amounts of DDLs use to take more time and also lead to high CPU usage. But
now we know specific invalidations at each command end so we execute only
required invalidations.
It has been observed that decoding of a transaction containing truncation
of a table with 1000 partitions would be finished in 1s whereas before
this patch it used to take 4-5 minutes.
Author: Dilip Kumar
Reviewed-by: Amit Kapila and Keisuke Kuroda
Discussion: https://postgr.es/m/CANDwggKYveEtXjXjqHA6RL3AKSHMsQyfRY6bK+NqhAWJyw8psQ@mail.gmail.com
Fujii Masao [Thu, 15 Oct 2020 02:04:07 +0000 (11:04 +0900)]
doc: Mention that toast_tuple_target affects also column marked as Main.
Previously it was documented that toast_tuple_target affected column
marked as only External or Extended. But this description is not correct
and toast_tuple_target affects also column marked as Main.
Back-patch to v11 where toast_tuple_target reloption was introduced.
Author: Shinya Okano
Reviewed-by: Tatsuhito Kasahara, Fujii Masao
Discussion: https://postgr.es/m/
93f46e311a67422e89e770d236059817@oss.nttdata.com
Alvaro Herrera [Wed, 14 Oct 2020 23:12:26 +0000 (20:12 -0300)]
Restore replication protocol's duplicate command tags
I removed the duplicate command tags for START_REPLICATION inadvertently
in commit
07082b08cc5d, but the replication protocol requires them. The
fact that the replication protocol was broken was not noticed because
all our test cases use an optimized code path that exits early, failing
to verify that the behavior is correct for non-optimized cases. Put
them back.
Also document this protocol quirk.
Add a test case that shows the failure. It might still succeed even
without the patch when run on a fast enough server, but it suffices to
show the bug in enough cases that it would be noticed in buildfarm.
Author: Álvaro Herrera <
[email protected]>
Reported-by: Henry Hinze <[email protected]>
Reviewed-by: Petr Jelínek <[email protected]>
Discussion: https://postgr.es/m/16643-
eaadeb2a1a58d28c@postgresql.org
Thomas Munro [Wed, 14 Oct 2020 22:31:20 +0000 (11:31 +1300)]
Make WL_POSTMASTER_DEATH level-triggered on kqueue builds.
If WaitEventSetWait() reports that the postmaster has gone away, later
calls to WaitEventSetWait() should continue to report that. Otherwise
further waits that occur in the proc_exit() path after we already
noticed the postmaster's demise could block forever.
Back-patch to 13, where the kqueue support landed.
Reported-by: Tom Lane <[email protected]>
Discussion: https://postgr.es/m/
3624029.
1602701929%40sss.pgh.pa.us
Heikki Linnakangas [Wed, 14 Oct 2020 08:41:40 +0000 (11:41 +0300)]
Remove es_result_relation_info from EState.
Maintaining 'es_result_relation_info' correctly at all times has become
cumbersome, especially with partitioning where each partition gets its
own result relation info. Having to set and reset it across arbitrary
operations has caused bugs in the past.
This changes all the places that used 'es_result_relation_info', to
receive the currently active ResultRelInfo via function parameters
instead.
Author: Amit Langote
Discussion: https://www.postgresql.org/message-id/CA%2BHiwqGEmiib8FLiHMhKB%2BCH5dRgHSLc5N5wnvc4kym%2BZYpQEQ%40mail.gmail.com
Heikki Linnakangas [Wed, 14 Oct 2020 07:58:38 +0000 (10:58 +0300)]
Include result relation info in direct modify ForeignScan nodes.
FDWs that can perform an UPDATE/DELETE remotely using the "direct modify"
set of APIs need to access the ResultRelInfo of the target table. That's
currently available in EState.es_result_relation_info, but the next
commit will remove that field.
This commit adds a new resultRelation field in ForeignScan, to store the
target relation's RT index, and the corresponding ResultRelInfo in
ForeignScanState. The FDW's PlanDirectModify callback is expected to set
'resultRelation' along with 'operation'. The core code doesn't need them
for anything, they are for the convenience of FDW's Begin- and
IterateDirectModify callbacks.
Authors: Amit Langote, Etsuro Fujita
Discussion: https://www.postgresql.org/message-id/CA%2BHiwqGEmiib8FLiHMhKB%2BCH5dRgHSLc5N5wnvc4kym%2BZYpQEQ%40mail.gmail.com
Peter Eisentraut [Wed, 14 Oct 2020 06:24:54 +0000 (08:24 +0200)]
Use https for gnu.org links
Mostly already done, but there were some stragglers.
Peter Eisentraut [Wed, 14 Oct 2020 05:54:14 +0000 (07:54 +0200)]
Correct error message
Apparently copy-and-pasted from nearby.
Tom Lane [Tue, 13 Oct 2020 21:44:56 +0000 (17:44 -0400)]
Paper over regression failures in infinite_recurse() on PPC64 Linux.
Our infinite_recurse() test to verify sane stack-overrun behavior
is affected by a bug of the Linux kernel on PPC64: it will get SIGSEGV
if it receives a signal when the stack depth is (a) over 1MB and
(b) within a few kB of filling the current physical stack allocation.
See https://bugzilla.kernel.org/show_bug.cgi?id=205183.
Since this test is a bit time-consuming and we run it in parallel with
test scripts that do a lot of DDL, it can be expected to get an sinval
catchup interrupt at some point, leading to failure if the timing is
wrong. This has caused more than 100 buildfarm failures over the
past year or so.
While a fix exists for the kernel bug, it might be years before that
propagates into all production kernels, particularly in some of the
older distros we have in the buildfarm. For now, let's just back off
and not run this test on Linux PPC64; that loses nothing in test
coverage so far as our own code is concerned.
To do that, split this test into a new script infinite_recurse.sql
and skip the test when the platform name is powerpc64...-linux-gnu.
Back-patch to v12. Branches before that have not been seen to get
this failure. No doubt that's because the "errors" test was not
run in parallel with other tests before commit
798070ec0, greatly
reducing the odds of an sinval catchup being necessary.
I also back-patched
3c8553547 into v12, just so the new regression
script would look the same in all branches having it.
Discussion: https://postgr.es/m/
3479046.
1602607848@sss.pgh.pa.us
Discussion: https://postgr.es/m/
20190723162703.GM22387%40telsasoft.com
Heikki Linnakangas [Tue, 13 Oct 2020 09:57:02 +0000 (12:57 +0300)]
Create ResultRelInfos later in InitPlan, index them by RT index.
Instead of allocating all the ResultRelInfos upfront in one big array,
allocate them in ExecInitModifyTable(). es_result_relations is now an
array of ResultRelInfo pointers, rather than an array of structs, and it
is indexed by the RT index.
This simplifies things: we get rid of the separate concept of a "result
rel index", and don't need to set it in setrefs.c anymore. This also
allows follow-up optimizations (not included in this commit yet) to skip
initializing ResultRelInfos for target relations that were not needed at
runtime, and removal of the es_result_relation_info pointer.
The EState arrays of regular result rels and root result rels are merged
into one array. Similarly, the resultRelations and rootResultRelations
lists in PlannedStmt are merged into one. It's not actually clear to me
why they were kept separate in the first place, but now that the
es_result_relations array is indexed by RT index, it certainly seems
pointless.
The PlannedStmt->resultRelations list is now only needed for
ExecRelationIsTargetRelation(). One visible effect of this change is that
ExecRelationIsTargetRelation() will now return 'true' also for the
partition root, if a partitioned table is updated. That seems like a good
thing, although the function isn't used in core code, and I don't see any
reason for an FDW to call it on a partition root.
Author: Amit Langote
Discussion: https://www.postgresql.org/message-id/CA%2BHiwqGEmiib8FLiHMhKB%2BCH5dRgHSLc5N5wnvc4kym%2BZYpQEQ%40mail.gmail.com
Amit Kapila [Tue, 13 Oct 2020 07:16:38 +0000 (12:46 +0530)]
Fix the unstable output of tests added by commit
8fccf75834.
The test cases added by that commit were trying to test the exact number of
times a particular transaction has spilled. However, that number can vary if
any background transaction (say by autovacuum) happens in parallel to the main
transaction. So let's not try to verify the exact count.
Author: Amit Kapila
Reviewed-by: Sawada Masahiko
Discussion: https://postgr.es/m/CA+fd4k5_pPAYRTDrO2PbtTOe0eHQpBvuqmCr8ic39uTNmR49Eg@mail.gmail.com
Peter Eisentraut [Tue, 13 Oct 2020 04:29:06 +0000 (06:29 +0200)]
doc: Expand recursive query documentation
Break the section up with subsection headings. Add examples for depth-
and breadth-first search ordering. For consistency with the SQL
search clause, start the depth counting at 0 instead of 1 in the
examples.
Discussion: https://www.postgresql.org/message-id/
c5603982-0088-7f14-0caa-
fdcd0c837b57@2ndquadrant.com
Amit Kapila [Tue, 13 Oct 2020 03:00:35 +0000 (08:30 +0530)]
Add tests for logical replication spilled stats.
Commit
9868167500 added a mechanism to track statistics corresponding to
the spilling of changes from ReorderBuffer but didn't add any tests.
Author: Amit Kapila and Sawada Masahiko
Discussion: https://postgr.es/m/CA+fd4k5_pPAYRTDrO2PbtTOe0eHQpBvuqmCr8ic39uTNmR49Eg@mail.gmail.com
Tom Lane [Mon, 12 Oct 2020 22:01:34 +0000 (18:01 -0400)]
Fix GiST buffering build to work when there are included columns.
gistRelocateBuildBuffersOnSplit did not get the memo about which
attribute count to use. This could lead to a crash if there were
included columns and buffering build was chosen. (Because there
are random page-split decisions elsewhere in GiST index build,
the crashes are not entirely deterministic.)
Back-patch to v12 where GiST gained support for included columns.
Pavel Borisov
Discussion: https://postgr.es/m/CALT9ZEECCV5m7wvxg46PC-7x-EybUmnpupBGhSFMoAAay+r6HQ@mail.gmail.com
Tom Lane [Mon, 12 Oct 2020 21:09:50 +0000 (17:09 -0400)]
Re-allow testing of GiST buffered builds.
Commit
16fa9b2b3 broke the ability to reliably test GiST buffered builds,
because it caused sorted builds to be done instead if sortsupport is
available, regardless of any attempt to override that. While a would-be
test case could try to work around that by choosing an opclass that has
no sortsupport function, coverage would be silently lost the moment
someone decides it'd be a good idea to add a sortsupport function.
Hence, rearrange the logic in gistbuild() so that if "buffering = on"
is specified in CREATE INDEX, we will use that method, sortsupport or no.
Also document the interaction between sorting and the buffering
parameter, as
16fa9b2b3 failed to do.
(Note that in fact we still lack any test coverage of buffered builds,
but this is a prerequisite to adding a non-fragile test.)
Discussion: https://postgr.es/m/
3249980.
1602532990@sss.pgh.pa.us
Tom Lane [Mon, 12 Oct 2020 17:31:24 +0000 (13:31 -0400)]
Fix memory leak when guc.c decides a setting can't be applied now.
The prohibitValueChange code paths in set_config_option(), which
are executed whenever we re-read a PGC_POSTMASTER variable from
postgresql.conf, neglected to free anything before exiting. Thus
we'd leak the proposed new value of a PGC_STRING variable, as noted
by BoChen in bug #16666. For all variable types, if the check hook
creates an "extra" chunk, we'd also leak that.
These are malloc not palloc chunks, so there is no mechanism for
recovering the leaks before process exit. Fortunately, the values
are typically not very large, meaning you'd have to go through an
awful lot of SIGHUP configuration-reload cycles to make the leakage
amount to anything. Still, for a long-lived postmaster process it
could potentially be a problem.
Oversight in commit
2594cf0e8. Back-patch to all supported branches.
Discussion: https://postgr.es/m/16666-
2c41a4eec61b03e1@postgresql.org
Tom Lane [Mon, 12 Oct 2020 15:13:02 +0000 (11:13 -0400)]
Minor cleanup for win32stat.c.
Use GetLastError(), rather than assuming that CreateFile() failure
must map to ENOENT. Noted by Michael Paquier.
Discussion: https://postgr.es/m/CAC+AXB0g44SbvSpC86o_1HWh8TAU2pZrMRW6tJT-dkijotx5Qg@mail.gmail.com
Michael Paquier [Mon, 12 Oct 2020 11:34:55 +0000 (20:34 +0900)]
Fix compilation warning in unicode_norm.c
80f8eb7 has introduced in unicode_norm.c some new code that uses
htonl(). On at least some FreeBSD environments, it is possible to find
that this function is undeclared, causing a compilation warning. It is
worth noting that no buildfarm members have reported this issue.
Instead of adding a new inclusion to arpa/inet.h, switch to use
the equivalent defined in pg_bswap.h, to benefit from any built-in
function if the compiler has one.
Reported-by: Masahiko Sawada
Discussion: https://postgr.es/m/CA+fd4k7D4b12ShywWj=AbcHZzV1-OqMjNe7RZAu+tgz5rd_11A@mail.gmail.com
Thomas Munro [Mon, 12 Oct 2020 07:41:16 +0000 (20:41 +1300)]
Fix estimates for ModifyTable paths without RETURNING.
In the past, we always estimated that a ModifyTable node would emit the
same number of rows as its subpaths. Without a RETURNING clause, the
correct estimate is zero. Fix, in preparation for a proposed parallel
write patch that is sensitive to that number.
A remaining problem is that for RETURNING queries, the estimated width
is based on subpath output rather than the RETURNING tlist.
Reviewed-by: Greg Nancarrow <[email protected]>
Discussion: https://postgr.es/m/CAJcOf-cXnB5cnMKqWEp2E2z7Mvcd04iLVmV%3DqpFJrR3AcrTS3g%40mail.gmail.com
Peter Eisentraut [Mon, 12 Oct 2020 05:46:20 +0000 (07:46 +0200)]
Adjust cycle detection examples and tests
Adjust the existing cycle detection example and test queries to put
the cycle column before the path column. This is mainly because the
SQL-standard CYCLE clause puts them in that order, and so if we added
that feature that would make the sequence of examples more consistent
and easier to follow.
Discussion: https://www.postgresql.org/message-id/
c5603982-0088-7f14-0caa-
fdcd0c837b57@2ndquadrant.com
Noah Misch [Mon, 12 Oct 2020 04:31:37 +0000 (21:31 -0700)]
Choose ppc compare_exchange constant path for more operand values.
The implementation uses smaller code when the "expected" operand is a
small constant, but the implementation needlessly defined the set of
acceptable constants more narrowly than the ABI does. Core PostgreSQL
and PGXN don't use the constant path at all, so this is future-proofing.
Back-patch to v13, where commit
30ee5d17c20dbb282a9952b3048d6ad52d56c371
introduced this code.
Reviewed by Tom Lane. Reported by Christoph Berg.
Discussion: https://postgr.es/m/
20201009092825[email protected]
Noah Misch [Mon, 12 Oct 2020 04:31:37 +0000 (21:31 -0700)]
For ppc gcc, implement 64-bit compare_exchange and fetch_add with asm.
While xlc defines __64BIT__, gcc does not. Due to this oversight in
commit
30ee5d17c20dbb282a9952b3048d6ad52d56c371, gcc builds continued
implementing 64-bit atomics by way of intrinsics. Back-patch to v13,
where that commit first appeared.
Reviewed by Tom Lane.
Discussion: https://postgr.es/m/
20201011051043[email protected]
Michael Paquier [Sun, 11 Oct 2020 10:09:01 +0000 (19:09 +0900)]
Use perfect hash for NFC and NFKC Unicode Normalization quick check
This makes the normalization quick check about 30% faster for NFC and
50% faster for NFKC than the binary search used previously. The hash
lookup reuses the existing array of bit fields used for the binary
search to get the quick check property and is generated as part of "make
update-unicode" in src/common/unicode/.
Author: John Naylor
Reviewed-by: Mark Dilger, Michael Paquier
Discussion: https://postgr.es/m/CACPNZCt4fbJ0_bGrN5QPt34N4whv=mszM0LMVQdoa2rC9UMRXA@mail.gmail.com
Tom Lane [Sat, 10 Oct 2020 23:57:25 +0000 (19:57 -0400)]
Band-aid new postgres_fdw test case to remove error text dependency.
Buildfarm member lorikeet is still failing the test from commit
32a9c0bdf, but now it's down to the should-have-foreseen-it problem
that the error message isn't what the expected-output file expects.
Let's see if we can get stable results by printing just the SQLSTATE.
I believe we'll reliably see ERRCODE_CONNECTION_FAILURE, since
pgfdw_report_error() will report that for any libpq-originated error.
There may be a better way to do this, but I'd like to get the
buildfarm back to green before we discuss further improvements.
Discussion: https://postgr.es/m/
[email protected]
Discussion: https://postgr.es/m/
2621622.
1602184554@sss.pgh.pa.us
Tom Lane [Sat, 10 Oct 2020 19:33:54 +0000 (15:33 -0400)]
Remove pointless error-code checking in pg_dump/parallel.c.
Commit
fe27009cb tried to make parallel.c's Windows implementation of
piperead() translate Windows socket errors to Unix, but that didn't
actually work because TranslateSocketError() is backend-internal code
(and not even public there). But on closer inspection, the sole
caller of this function doesn't actually care whether the result is
zero or negative, much less inspect the errno. So the whole exercise
is totally useless, and has been since this code was introduced.
Rip it out and just call recv() directly.
Per buildfarm.
Discussion: https://postgr.es/m/
2621622.
1602184554@sss.pgh.pa.us
Tom Lane [Sat, 10 Oct 2020 18:53:23 +0000 (14:53 -0400)]
Minor cleanup for win32stat.c.
Fix silly typo in previous commit.
Discussion: https://postgr.es/m/CAC+AXB0g44SbvSpC86o_1HWh8TAU2pZrMRW6tJT-dkijotx5Qg@mail.gmail.com
Tom Lane [Sat, 10 Oct 2020 17:39:21 +0000 (13:39 -0400)]
Minor cleanup for win32stat.c.
Ensure that CloseHandle() can't clobber the errno we set for
failure exits, and make a couple of tweaks for pgindent.
Juan José Santamaría Flecha
Discussion: https://postgr.es/m/CAC+AXB0g44SbvSpC86o_1HWh8TAU2pZrMRW6tJT-dkijotx5Qg@mail.gmail.com
Tom Lane [Sat, 10 Oct 2020 17:28:12 +0000 (13:28 -0400)]
Recognize network-failure errnos as indicating hard connection loss.
Up to now, only ECONNRESET (and EPIPE, in most but not quite all places)
received special treatment in our error handling logic. This patch
changes things so that related error codes such as ECONNABORTED are
also recognized as indicating that the connection's dead and unlikely
to come back.
We continue to think, however, that only ECONNRESET and EPIPE should be
reported as probable server crashes; the other cases indicate network
connectivity problems but prove little about the server's state. Thus,
there's no change in the error message texts that are output for such
cases. The key practical effect is that errcode_for_socket_access()
will report ERRCODE_CONNECTION_FAILURE rather than
ERRCODE_INTERNAL_ERROR for a network failure. It's expected that this
will fix buildfarm member lorikeet's failures since commit
32a9c0bdf,
as that seems to be due to not treating ECONNABORTED equivalently to
ECONNRESET.
The set of errnos treated this way now includes ECONNABORTED, EHOSTDOWN,
EHOSTUNREACH, ENETDOWN, ENETRESET, and ENETUNREACH. Several of these
were second-class citizens in terms of their handling in places like
get_errno_symbol(), so upgrade the infrastructure where necessary.
As committed, this patch assumes that all these symbols are defined
everywhere. POSIX specifies all of them except EHOSTDOWN, but that
seems to exist on all platforms of interest; we'll see what the
buildfarm says about that.
Probably this should be back-patched, but let's see what the buildfarm
thinks of it first.
Fujii Masao and Tom Lane
Discussion: https://postgr.es/m/
2621622.
1602184554@sss.pgh.pa.us
Tom Lane [Fri, 9 Oct 2020 21:54:34 +0000 (17:54 -0400)]
plperl.h should #undef fstat along with stat and lstat.
Needed now that commit
bed90759f caused win32_port.h to provide
a #define for that too. Per buildfarm.
Tom Lane [Fri, 9 Oct 2020 20:20:12 +0000 (16:20 -0400)]
Fix our Windows stat() emulation to handle file sizes > 4GB.
Hack things so that our idea of "struct stat" is equivalent to Windows'
struct __stat64, allowing it to have a wide enough st_size field.
Instead of relying on native stat(), use GetFileInformationByHandle().
This avoids a number of issues with Microsoft's multiple and rather
slipshod emulations of stat(). We still need to jump through hoops
to deal with ERROR_DELETE_PENDING, though :-(
Pull the relevant support code out of dirmod.c and put it into
its own file, win32stat.c.
Still TODO: do we need to do something different with lstat(),
rather than treating it identically to stat()?
Juan José Santamaría Flecha, reviewed by Emil Iggland;
based on prior work by Michael Paquier, Sergey Zubkovsky, and others
Discussion: https://postgr.es/m/
1803D792815FC24D871C00D17AE95905CF5099@g01jpexmbkw24
Discussion: https://postgr.es/m/15858-
9572469fd3b73263@postgresql.org
Amit Kapila [Fri, 9 Oct 2020 02:45:53 +0000 (08:15 +0530)]
Fix typos in logical.c and reorderbuffer.c.
Reviewed-by: Sawada Masahiko
Discussion: https://postgr.es/m/CAA4eK1K6zTpuqf_d7wXCBjo_EF0_B6Fz3Ecp71Vq18t=wG-nzg@mail.gmail.com
Tom Lane [Thu, 8 Oct 2020 17:06:27 +0000 (13:06 -0400)]
Avoid gratuitous inaccuracy in numeric width_bucket().
Multiply before dividing, not the reverse, so that cases that should
produce exact results do produce exact results. (width_bucket_float8
got this right already.) Even when the result is inexact, this avoids
making it more inexact, since only the division step introduces any
imprecision.
While at it, fix compute_bucket() to not uselessly repeat the sign
check already done by its caller, and avoid duplicating the
multiply/divide steps by adjusting variable usage.
Per complaint from Martin Visser. Although this seems like a bug fix,
I'm hesitant to risk changing width_bucket()'s results in stable
branches, so no back-patch.
Discussion: https://postgr.es/m/
6FA5117D-6AED-4656-8FEF-
B74AC18FAD85@brytlyt.com
Tom Lane [Thu, 8 Oct 2020 16:37:59 +0000 (12:37 -0400)]
Fix numeric width_bucket() to allow its first argument to be infinite.
While the calculation is not well-defined if the bounds arguments are
infinite, there is a perfectly sane outcome if the test operand is
infinite: it's just like any other value that's before the first bucket
or after the last one. width_bucket_float8() got this right, but
I was too hasty about the case when adding infinities to numerics
(commit
a57d312a7), so that width_bucket_numeric() just rejected it.
Fix that, and sync the relevant error message strings.
No back-patch needed, since infinities-in-numeric haven't shipped yet.
Discussion: https://postgr.es/m/
2465409.
1602170063@sss.pgh.pa.us
Michael Paquier [Thu, 8 Oct 2020 05:06:12 +0000 (14:06 +0900)]
Fix typo in multixact.c
AtEOXact_MultiXact() was referenced in two places with an incorrect
routine name.
Author: Hou Zhijie
Discussion: https://postgr.es/m/
1b41e9311e8f474cb5a360292f0b3cb1@G08CNEXMBPEKD05.g08.fujitsu.local
Michael Paquier [Thu, 8 Oct 2020 04:16:43 +0000 (13:16 +0900)]
Improve set of candidate multipliers for perfect hash function generation
The previous set of multipliers was not adapted for large sets of short
keys, and this new set of multipliers allows to generate perfect hash
functions for larger sets without having an impact for existing callers
of those functions, as experimentation has showed. A future commit will
make use of that to improve the performance of unicode normalization.
All multipliers compile to shift-and-add instructions on most platforms.
This has been tested as far back as gcc 4.1 and clang 3.8.
Author: John Naylor
Reviewed-by: Mark Dilger, Michael Paquier
Discussion: https://postgr.es/m/CACPNZCt4fbJ0_bGrN5QPt34N4whv=mszM0LMVQdoa2rC9UMRXA@mail.gmail.com