Commit Graph

21796 Commits

Author SHA1 Message Date
Alvaro Herrera a288d94c91
Remove redundant variable
Author: Amul Sul <sulamul@gmail.com>
Reviewed-by: Jeevan Ladhe <jeevan.ladhe@enterprisedb.com>
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/CAAJ_b94HaNcrPVREUuB9-qUn2uB+gfcoX3FG_Vx0S6aFse+yhw@mail.gmail.com
2021-05-06 17:28:36 -04:00
Peter Geoghegan c9787385db Remove overzealous VACUUM visibility map assertion.
The all_visible_according_to_vm variable's value is inherently prone to
becoming invalidated concurrently, since it is set before we even
acquire a lock on a related heap page buffer.

Oversight in commit 7136bf34, which added the assertion in passing.

Author: Masahiko Sawada <sawada.mshk@gmail.com>
Reported-By: Tang <tanghy.fnst@fujitsu.com>
Diagnosed-By:: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/CAD21AoDzgc8_MYrA5m1fyydomw_eVKtQiYh7sfDK4KEhdMsf_g@mail.gmail.com
2021-05-06 13:17:39 -07:00
Alvaro Herrera 3fe773b149
Track detached partitions more accurately in partdescs
In d6b8d29419 I (Álvaro) was sloppy about recording whether a
partition descripor does or does not include detached partitions, when
the snapshot checking does not see the pg_inherits row marked detached.
In that case no partition was omitted, yet in the relcache entry we were
saving the partdesc as omitting partitions.  Flip that (so we save it as
a partdesc not omitting partitions, which indeed it doesn't), which
hopefully makes the code easier to reason about.

Author: Amit Langote <amitlangote09@gmail.com>
Discussion: https://postgr.es/m/CA+HiwqE7GxGU4VdzwZzfiz+Ont5SsopoFkgtrZGEdPqWRL+biA@mail.gmail.com
2021-05-06 12:47:30 -04:00
Amit Kapila 592f00f8de Update replication statistics after every stream/spill.
Currently, replication slot statistics are updated at prepare, commit, and
rollback. Now, if the transaction is interrupted the stats might not get
updated. Fixed this by updating replication statistics after every
stream/spill.

In passing update the docs to change the description of some of the slot
stats.

Author: Vignesh C, Sawada Masahiko
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/20210319185247.ldebgpdaxsowiflw@alap3.anarazel.de
2021-05-06 11:21:26 +05:30
Andres Freund 7f2e10baa2 jit: Fix warning reported by gcc-11 caused by dubious function signature.
Reported-By: Erik Rijkers <er@xs4all.nl>
Discussion: https://postgr.es/m/833107370.1313189.1619647621213@webmailclassic.xs4all.nl
Backpatch: 13, where b059d2f456 introduced the issue.
2021-05-05 22:13:55 -07:00
Amit Kapila 2ce353fc19 Tighten the concurrent abort check during decoding.
During decoding of an in-progress or prepared transaction, we detect
concurrent abort with an error code ERRCODE_TRANSACTION_ROLLBACK. That is
not sufficient because a callback can decide to throw that error code
at other times as well.

Reported-by: Tom Lane
Author: Amit Kapila
Reviewed-by: Dilip Kumar
Discussion: https://postgr.es/m/CAA4eK1KCjPRS4aZHB48QMM4J8XOC1+TD8jo-4Yu84E+MjwqVhA@mail.gmail.com
2021-05-06 08:26:42 +05:30
Alvaro Herrera c250062df4
Remove unused argument of ATAddForeignConstraint
Commit 0325d7a595 made this unused but forgot to remove it. Do so now.

Author: Amit Langote <amitlangote09@gmail.com>
Discussion: https://postgr.es/m/209c99fe-b9a2-94f4-cd68-a8304186a09e@lab.ntt.co.jp
2021-05-05 12:27:39 -04:00
Alvaro Herrera 6f70d7ca1d
Have ALTER CONSTRAINT recurse on partitioned tables
When ALTER TABLE .. ALTER CONSTRAINT changes deferrability properties
changed in a partitioned table, we failed to propagate those changes
correctly to partitions and to triggers.  Repair by adding a recursion
mechanism to affect all derived constraints and all derived triggers.
(In particular, recurse to partitions even if their respective parents
are already in the desired state: it is possible for the partitions to
have been altered individually.)  Because foreign keys involve tables in
two sides, we cannot use the standard ALTER TABLE recursion mechanism,
so we invent our own by following pg_constraint.conparentid down.

When ALTER TABLE .. ALTER CONSTRAINT is invoked on the derived
pg_constraint object that's automaticaly created in a partition as a
result of a constraint added to its parent, raise an error instead of
pretending to work and then failing to modify all the affected triggers.
Before this commit such a command would be allowed but failed to affect
all triggers, so it would silently misbehave.  (Restoring dumps of
existing databases is not affected, because pg_dump does not produce
anything for such a derived constraint anyway.)

Add some tests for the case.

Backpatch to 11, where foreign key support was added to partitioned
tables by commit 3de241dba8.  (A related change is commit f56f8f8da6
in pg12 which added support for FKs *referencing* partitioned tables;
this is what forces us to use an ad-hoc recursion mechanism for this.)

Diagnosed by Tom Lane from bug report from Ron L Johnson.  As of this
writing, no reviews were offered.

Discussion: https://postgr.es/m/75fe0761-a291-86a9-c8d8-4906da077469@gmail.com
Discussion: https://postgr.es/m/3144850.1607369633@sss.pgh.pa.us
2021-05-05 12:21:50 -04:00
Peter Eisentraut 38f36aad8c GUC description improvements for clarity 2021-05-05 08:18:22 +02:00
Alvaro Herrera e798d095da
Fix OID passed to object-alter hook during ALTER CONSTRAINT
The OID of the constraint is used instead of the OID of the trigger --
an easy mistake to make.  Apparently the object-alter hooks are not very
well tested :-(

Backpatch to 12, where this typo was introduced by 578b229718

Discussion: https://postgr.es/m/20210503231633.GA6994@alvherre.pgsql
2021-05-04 10:09:12 -04:00
Peter Eisentraut a970edbed3 Fix ALTER TABLE / INHERIT with generated columns
When running ALTER TABLE t2 INHERIT t1, we must check that columns in
t2 that correspond to a generated column in t1 are also generated and
have the same generation expression.  Otherwise, this would allow
creating setups that a normal CREATE TABLE sequence would not allow.

Discussion: https://www.postgresql.org/message-id/22de27f6-7096-8d96-4619-7b882932ca25@2ndquadrant.com
2021-05-04 12:09:08 +02:00
Bruce Momjian f7a97b6ec3 Update query_id computation
Properly fix:

- the "ONLY" in FROM [ONLY] isn't hashed
- the agglevelsup field in GROUPING isn't hashed
- WITH TIES not being hashed (new in PG 13)
- "DISTINCT" in "GROUP BY [DISTINCT]" isn't hashed (new in PG 14)

Reported-by: Julien Rouhaud

Discussion: https://postgr.es/m/20210425081119.ulyzxqz23ueh3wuj@nol
2021-05-03 14:59:39 -04:00
Tom Lane f68970e33f Fix performance issue in new regex match-all detection code.
Commit 824bf7190 introduced a new search of the NFAs generated by
regex compilation.  I failed to think hard about the performance
characteristics of that search, with the predictable outcome
that it's bad: weird regexes can trigger exponential search time.
Worse, there's no check-for-interrupt in that code, so you can't
even cancel the query if this happens.

Fix by introducing memo-ization of the search results, so that any one
NFA state need be examined in detail just once.  This potentially uses
a lot of memory, but we can bound the memory usage by putting a limit
on the number of states for which we'll try to prove match-all-ness.
That is sane because we already have a limit (DUPINF) on the maximum
finite string length that a matchall regex can match; and patterns
that involve much more than DUPINF states would probably exceed that
limit anyway.

Also, rearrange the logic so that we check the basic is-the-graph-
all-RAINBOW-arcs property before we start the recursive search to
determine path lengths.  This will ensure that we fall out quickly
whenever the NFA couldn't possibly be matchall.

Also stick in a check-for-interrupt, just in case these measures
don't completely eliminate the risk of slowness.

Discussion: https://postgr.es/m/3483895.1619898362@sss.pgh.pa.us
2021-05-03 11:42:31 -04:00
Peter Eisentraut b94409a02f Prevent lwlock dtrace probes from unnecessary work
If dtrace is compiled in but disabled, the lwlock dtrace probes still
evaluate their arguments.  Since PostgreSQL 13, T_NAME(lock) does
nontrivial work, so it should be avoided if not needed.  To fix, make
these calls conditional on the *_ENABLED() macro corresponding to each
probe.

Reviewed-by: Craig Ringer <craig.ringer@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/CAGRY4nwxKUS_RvXFW-ugrZBYxPFFM5kjwKT5O+0+Stuga5b4+Q@mail.gmail.com
2021-05-03 12:18:27 +02:00
Peter Eisentraut c285babf8f Remove unused function argument
became unused by 04942bffd0
2021-05-03 09:05:58 +02:00
Amit Kapila 205f466282 Fix the computation of slot stats for 'total_bytes'.
Previously, we were using the size of all the changes present in
ReorderBuffer to compute total_bytes after decoding a transaction and that
can lead to counting some of the transactions' changes more than once. Fix
it by using the size of the changes decoded for a transaction to compute
'total_bytes'.

Author: Sawada Masahiko
Reviewed-by: Vignesh C, Amit Kapila
Discussion: https://postgr.es/m/20210319185247.ldebgpdaxsowiflw@alap3.anarazel.de
2021-05-03 07:22:08 +05:30
Alexander Korotkov eb086056fe Make websearch_to_tsquery() parse text in quotes as a single token
websearch_to_tsquery() splits text in quotes into tokens and connects them with
phrase operator on its own.  However, that leads to surprising results when the
token contains no words.

For instance, websearch_to_tsquery('"aaa: bbb"') is 'aaa <2> bbb', because
it is equivalent of to_tsquery(E'aaa <-> \':\' <-> bbb').  But
websearch_to_tsquery('"aaa: bbb"') has to be 'aaa <-> bbb' in order to match
to_tsvector('aaa: bbb').

Since 0c4f355c6a, we anyway connect lexemes of complex tokens with phrase
operators.  Thus, let's just websearch_to_tsquery() parse text in quotes as
a single token.  Therefore, websearch_to_tsquery() should process the quoted
text in the same way phraseto_tsquery() does.  This solution is what we exactly
need and also simplifies the code.

This commit is an incompatible change, so we don't backpatch it.

Reported-by: Valentin Gatien-Baron
Discussion: https://postgr.es/m/CA%2B0DEqiZs7gdOd4ikmg%3D0UWG%2BSwWOLxPsk_JW-sx9WNOyrb0KQ%40mail.gmail.com
Author: Alexander Korotkov
Reviewed-by: Tom Lane, Zhihong Yu
2021-05-03 04:18:19 +03:00
Bruce Momjian 651d005e76 Revert use singular for -1 (commits 9ee7d533da and 5da9868ed9
Turns out you can specify negative values using plurals:

	https://english.stackexchange.com/questions/9735/is-1-followed-by-a-singular-or-plural-noun

so the previous code was correct enough, and consistent with other usage
in our code.  Also add comment in the two places where this could be
confused.

Reported-by: Noah Misch

Diagnosed-by: 20210425115726.GA2353095@rfd.leadboat.com
2021-05-01 10:42:44 -04:00
Tom Lane 2efcd502e5 Disallow calling anything but plain functions via the fastpath API.
Reject aggregates, window functions, and procedures.  Aggregates
failed anyway, though with a somewhat obscure error message.
Window functions would hit an Assert or null-pointer dereference.
Procedures seemed to work as long as you didn't try to do
transaction control, but (a) transaction control is sort of the
point of a procedure, and (b) it's not entirely clear that no
bugs lurk in that path.  Given the lack of testing of this area,
it seems safest to be conservative in what we support.

Also reject proretset functions, as the fastpath protocol can't
support returning a set.

Also remove an easily-triggered assertion that the given OID
isn't 0; the subsequent lookups can handle that case themselves.

Per report from Theodor-Arsenij Larionov-Trichkin.
Back-patch to all supported branches.  (The procedure angle
only applies in v11+, of course.)

Discussion: https://postgr.es/m/2039442.1615317309@sss.pgh.pa.us
2021-04-30 14:10:26 -04:00
Amit Kapila ee4ba01dbb Fix the bugs in selecting the transaction for streaming.
There were two problems:
a. We were always selecting the next available txn instead of selecting it
when it is larger than the previous transaction.
b. We were selecting the transactions which haven't made any changes to
the database (base snapshot is not set). Later it was hitting an Assert
because we don't decode such transactions and the changes in txn remain as
it is. It is better not to choose such transactions for streaming in the
first place.

Reported-by: Haiying Tang
Author: Dilip Kumar
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/OS0PR01MB61133B94E63177040F7ECDA1FB429@OS0PR01MB6113.jpnprd01.prod.outlook.com
2021-04-30 10:49:52 +05:30
David Rowley 3c80e96dff Adjust EXPLAIN output for parallel Result Cache plans
Here we adjust the EXPLAIN ANALYZE output for Result Cache so that we
don't show any Result Cache stats for parallel workers who don't
contribute anything to Result Cache plan nodes.

I originally had ideas that workers who don't help could still have their
Result Cache stats displayed.  The idea with that was so that I could
write some parallel Result Cache regression tests that show the EXPLAIN
ANALYZE output.  However, I realized a little too late that such tests
would just not be possible to have run in a stable way on the buildfarm.

With that knowledge, before 9eacee2e6 went in, I had removed all of the
tests that were showing the EXPLAIN ANALYZE output of a parallel Result
Cache plan, however, I forgot to put back the code that adjusts the
EXPLAIN output to hide the Result Cache stats for parallel workers who
were not fast enough to help out before query execution was over. All
other nodes behave this way and so should Result Cache.

Additionally, with this change, it now seems safe enough to remove the SET
force_parallel_mode = off that I had added to the regression tests.

Also, perform some cleanup in the partition_prune tests. I had adjusted
the explain_parallel_append() function to sanitize the Result Cache
EXPLAIN ANALYZE output.  However, since I didn't actually include any
parallel Result Cache tests that show their EXPLAIN ANALYZE output, that
code does nothing and can be removed.

In passing, move the setting of memPeakKb into the scope where it's used.

Reported-by: Amit Khandekar
Author: David Rowley, Amit Khandekar
Discussion: https://postgr.es/m/CAJ3gD9d8SkfY95GpM1zmsOtX2-Ogx5q-WLsf8f0ykEb0hCRK3w@mail.gmail.com
2021-04-30 14:46:42 +12:00
Peter Eisentraut 3a948ea0a2 pg_hba.conf.sample: Reword connection type section
Improve the wording in the connection type section of
pg_hba.conf.sample a bit.  After the hostgssenc part was added on, the
whole thing became a bit wordy, and it's also a bit inaccurate for
example in that the current wording for "host" appears to say that it
does not apply to GSS-encrypted connections.

Discussion: https://www.postgresql.org/message-id/flat/fc06dcc5-513f-e944-cd07-ba51dd7c6916%40enterprisedb.com
2021-04-29 07:00:20 +02:00
Tom Lane 9626325da5 Add heuristic incoming-message-size limits in the server.
We had a report of confusing server behavior caused by a client bug
that sent junk to the server: the server thought the junk was a
very long message length and waited patiently for data that would
never come.  We can reduce the risk of that by being less trusting
about message lengths.

For a long time, libpq has had a heuristic rule that it wouldn't
believe large message size words, except for a small number of
message types that are expected to be (potentially) long.  This
provides some defense against loss of message-boundary sync and
other corrupted-data cases.  The server does something similar,
except that up to now it only limited the lengths of messages
received during the connection authentication phase.  Let's
do the same as in libpq and put restrictions on the allowed
length of all messages, while distinguishing between message
types that are expected to be long and those that aren't.

I used a limit of 10000 bytes for non-long messages.  (libpq's
corresponding limit is 30000 bytes, but given the asymmetry of
the FE/BE protocol, there's no good reason why the numbers should
be the same.)  Experimentation suggests that this is at least a
factor of 10, maybe a factor of 100, more than we really need;
but plenty of daylight seems desirable to avoid false positives.
In any case we can adjust the limit based on beta-test results.

For long messages, set a limit of MaxAllocSize - 1, which is the
most that we can absorb into the StringInfo buffer that the message
is collected in.  This just serves to make sure that a bogus message
size is reported as such, rather than as a confusing gripe about
not being able to enlarge a string buffer.

While at it, make sure that non-mainline code paths (such as
COPY FROM STDIN) are as paranoid as SocketBackend is, and validate
the message type code before believing the message length.
This provides an additional guard against getting stuck on corrupted
input.

Discussion: https://postgr.es/m/2003757.1619373089@sss.pgh.pa.us
2021-04-28 15:50:46 -04:00
Alvaro Herrera d6b8d29419
Allow a partdesc-omitting-partitions to be cached
Makes partition descriptor acquisition faster during the transient
period in which a partition is in the process of being detached.

This also adds the restriction that only one partition can be in
pending-detach state for a partitioned table.

While at it, return find_inheritance_children() API to what it was
before 71f4c8c6f7, and create a separate
find_inheritance_children_extended() that returns detailed info about
detached partitions.

(This incidentally fixes a bug in 8aba932251 whereby a memory context
holding a transient partdesc is reparented to a NULL PortalContext,
leading to permanent leak of that memory.  The fix is to no longer rely
on reparenting contexts to PortalContext.   Reported by Amit Langote.)

Per gripe from Amit Langote
Discussion: https://postgr.es/m/CA+HiwqFgpP1LxJZOBYGt9rpvTjXXkg5qG2+Xch2Z1Q7KrqZR1A@mail.gmail.com
2021-04-28 15:44:35 -04:00
Michael Paquier f93f0b5b25 Fix use-after-release issue with pg_identify_object_as_address()
Spotted by buildfarm member prion, with -DRELCACHE_FORCE_RELEASE.

Introduced in f7aab36.

Discussion: https://postgr.es/m/2759018.1619577848@sss.pgh.pa.us
Backpatch-through: 9.6
2021-04-28 11:58:08 +09:00
Michael Paquier f7aab36d61 Fix pg_identify_object_as_address() with event triggers
Attempting to use this function with event triggers failed, as, since
its introduction in a676201, this code has never associated an object
name with event triggers.  This addresses the failure by adding the
event trigger name to the set defining its object address.

Note that regression tests are added within event_trigger and not
object_address to avoid issues with concurrent connections in parallel
schedules.

Author: Joel Jacobson
Discussion: https://postgr.es/m/3c905e77-a026-46ae-8835-c3f6cd1d24c8@www.fastmail.com
Backpatch-through: 9.6
2021-04-28 11:17:58 +09:00
Fujii Masao 8e9ea08bae Don't pass "ONLY" options specified in TRUNCATE to foreign data wrapper.
Commit 8ff1c94649 allowed TRUNCATE command to truncate foreign tables.
Previously the information about "ONLY" options specified in TRUNCATE
command were passed to the foreign data wrapper. Then postgres_fdw
constructed the TRUNCATE command to issue the remote server and
included "ONLY" options in it based on the passed information.

On the other hand, "ONLY" options specified in SELECT, UPDATE or DELETE
have no effect when accessing or modifying the remote table, i.e.,
are not passed to the foreign data wrapper. So it's inconsistent to
make only TRUNCATE command pass the "ONLY" options to the foreign data
wrapper. Therefore this commit changes the TRUNCATE command so that
it doesn't pass the "ONLY" options to the foreign data wrapper,
for the consistency with other statements. Also this commit changes
postgres_fdw so that it always doesn't include "ONLY" options in
the TRUNCATE command that it constructs.

Author: Fujii Masao
Reviewed-by: Bharath Rupireddy, Kyotaro Horiguchi, Justin Pryzby, Zhihong Yu
Discussion: https://postgr.es/m/551ed8c1-f531-818b-664a-2cecdab99cd8@oss.nttdata.com
2021-04-27 14:41:27 +09:00
Amit Kapila 3fa17d3771 Use HTAB for replication slot statistics.
Previously, we used to use the array of size max_replication_slots to
store stats for replication slots. But that had two problems in the cases
where a message for dropping a slot gets lost: 1) the stats for the new
slot are not recorded if the array is full and 2) writing beyond the end
of the array if the user reduces the max_replication_slots.

This commit uses HTAB for replication slot statistics, resolving both
problems. Now, pgstat_vacuum_stat() search for all the dead replication
slots in stats hashtable and tell the collector to remove them. To avoid
showing the stats for the already-dropped slots, pg_stat_replication_slots
view searches slot stats by the slot name taken from pg_replication_slots.

Also, we send a message for creating a slot at slot creation, initializing
the stats. This reduces the possibility that the stats are accumulated
into the old slot stats when a message for dropping a slot gets lost.

Reported-by: Andres Freund
Author: Sawada Masahiko, test case by Vignesh C
Reviewed-by: Amit Kapila, Vignesh C, Dilip Kumar
Discussion: https://postgr.es/m/20210319185247.ldebgpdaxsowiflw@alap3.anarazel.de
2021-04-27 09:09:11 +05:30
Amit Kapila e7eea52b2d Fix Logical Replication of Truncate in synchronous commit mode.
The Truncate operation acquires an exclusive lock on the target relation
and indexes. It then waits for logical replication of the operation to
finish at commit. Now because we are acquiring the shared lock on the
target index to get index attributes in pgoutput while sending the
changes for the Truncate operation, it leads to a deadlock.

Actually, we don't need to acquire a lock on the target index as we build
the cache entry using a historic snapshot and all the later changes are
absorbed while decoding WAL. So, we wrote a special purpose function for
logical replication to get a bitmap of replica identity attribute numbers
where we get that information without locking the target index.

We decided not to backpatch this as there doesn't seem to be any field
complaint about this issue since it was introduced in commit 5dfd1e5a in
v11.

Reported-by: Haiying Tang
Author: Takamichi Osumi, test case by Li Japin
Reviewed-by: Amit Kapila, Ajin Cherian
Discussion: https://postgr.es/m/OS0PR01MB6113C2499C7DC70EE55ADB82FB759@OS0PR01MB6113.jpnprd01.prod.outlook.com
2021-04-27 08:28:26 +05:30
Tom Lane 04942bffd0 Remove rewriteTargetListIU's expansion of view targetlists in UPDATE.
Commit 2ec993a7c, which added triggers on views, modified the rewriter
to add dummy entries like "SET x = x" for all columns that weren't
actually being updated by the user in any UPDATE directed at a view.
That was needed at the time to produce a complete "NEW" row to pass
to the trigger.  Later it was found to cause problems for ordinary
updatable views, so commit cab5dc5da restricted it to happen only for
trigger-updatable views.  But in the wake of commit 86dc90056, we
really don't need it at all.  nodeModifyTable.c populates the trigger
"OLD" row from the whole-row variable that is generated for the view,
and then it computes the "NEW" row using that old row and the UPDATE
targetlist.  So there is no need for the UPDATE tlist to have dummy
entries, any more than it needs them for regular tables or other
types of views.

(The comments for rewriteTargetListIU suggest that we must do this
for correct expansion of NEW references in rules, but I now think
that that was just lazy comment editing in 2ec993a7c.  If we didn't
need it for rules on views before there were triggers, we don't need
it after that.)

This essentially propagates 86dc90056's decision that we don't need
dummy column updates into the view case.  Aside from making the
different cases more uniform and hence possibly forestalling future
bugs, it ought to save a little bit of rewriter/planner effort.

Discussion: https://postgr.es/m/2181213.1619397634@sss.pgh.pa.us
2021-04-26 13:58:00 -04:00
Amit Kapila f25a4584c6 Avoid sending prepare multiple times while decoding.
We send the prepare for the concurrently aborted xacts so that later when
rollback prepared is decoded and sent, the downstream should be able to
rollback such a xact. For 'streaming' case (when we send changes for
in-progress transactions), we were sending prepare twice when concurrent
abort was detected.

Author: Peter Smith
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/f82133c6-6055-b400-7922-97dae9f2b50b@enterprisedb.com
2021-04-26 11:27:44 +05:30
Amit Kapila 6d2e87a077 Fix typo in reorderbuffer.c.
Author: Peter Smith
Discussion: https://postgr.es/m/CAHut+PtvzuYY0zu=dVRK_WVz5WGos1+otZWgEWqjha1ncoSRag@mail.gmail.com
2021-04-26 08:42:46 +05:30
Tom Lane 08a9869665 Update comments for rewriteTargetListIU().
This function's behavior for UPDATE on a trigger-updatable view was
justified by analogy to what preptlist.c used to do for UPDATE on
regular tables.  Since preptlist.c hasn't done that since 86dc90056,
that argument is no longer sensible, let alone convincing.  I think
we do still need it to act that way, so update the comment to explain
why.
2021-04-25 18:02:03 -04:00
Michael Paquier 9b5558e7ad Fix come comments in execMain.c
1375422 has refactored this area of the executor code, and some comments
went out-of-sync.

Author: Yukun Wang
Reviewed-by: Amul Sul
Discussion: https://postgr.es/m/OS0PR01MB60033394FCAEF79B98F078F5B4459@OS0PR01MB6003.jpnprd01.prod.outlook.com
2021-04-24 15:07:04 +09:00
Michael Paquier 4aba61b870 Add some forgotten LSN_FORMAT_ARGS() in xlogreader.c
6f6f284 has introduced a specific macro to make printf()-ing of LSNs
easier.  This takes care of what looks like the remaining code paths
that did not get the call.

Author: Michael Paquier
Reviewed-by: Kyotaro Horiguchi, Tom Lane
Discussion: https://postgr.es/m/YIJS9x6K8ruizN7j@paquier.xyz
2021-04-24 09:09:02 +09:00
Peter Eisentraut 82c3cd9741 Factor out system call names from error messages
Instead, put them in via a format placeholder.  This reduces the
number of distinct translatable messages and also reduces the chances
of typos during translation.  We already did this for the system call
arguments in a number of cases, so this is just the same thing taken a
bit further.

Discussion: https://www.postgresql.org/message-id/flat/92d6f545-5102-65d8-3c87-489f71ea0a37%40enterprisedb.com
2021-04-23 14:21:37 +02:00
Peter Eisentraut 9486844f30 Use correct format placeholder for WSAGetLastError()
Some code thought this was unsigned, but it's signed int.
2021-04-23 14:21:37 +02:00
Alexander Korotkov 6bbcff096f Mark multirange_constructor0() and multirange_constructor2() strict
These functions shouldn't receive null arguments: multirange_constructor0()
doesn't have any arguments while multirange_constructor2() has a single array
argument, which is never null.

But mark them strict anyway for the sake of uniformity.

Also, make checks for null arguments use elog() instead of ereport() as these
errors should normally be never thrown.  And adjust corresponding comments.

Catversion is bumped.

Reported-by: Peter Eisentraut
Discussion: https://postgr.es/m/0f783a96-8d67-9e71-996b-f34a7352eeef%40enterprisedb.com
2021-04-23 13:25:45 +03:00
Fujii Masao 3f20d5f370 Reorder COMPRESSION option in gram.y and parsenodes.h into alphabetical order.
Commit bbe0a81db6 introduced "INCLUDING COMPRESSION" option
in CREATE TABLE command, but previously TableLikeOption in gram.y and
parsenodes.h didn't classify this new option in alphabetical order
with the rest.

Author: Fujii Masao
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/YHerAixOhfR1ryXa@paquier.xyz
2021-04-23 19:10:24 +09:00
Peter Eisentraut 7776a23a4b Fix incorrect format placeholder 2021-04-23 07:21:13 +02:00
Michael Paquier 45c0c5f70e Fix some comments in fmgr.c
Oversight in 2a0faed.

Author: Hou Zhijie
Discussion: https://postgr.es/m/OS0PR01MB5716405E2464D85E6DB6DC0794469@OS0PR01MB5716.jpnprd01.prod.outlook.com
2021-04-23 13:34:02 +09:00
Michael Paquier 62aa2bb293 Remove use of [U]INT64_FORMAT in some translatable strings
%lld with (long long), or %llu with (unsigned long long) are more
adapted.  This is similar to 3286065.

Author: Kyotaro Horiguchi
Discussion: https://postgr.es/m/20210421.200000.1462448394029407895.horikyota.ntt@gmail.com
2021-04-23 13:25:49 +09:00
Etsuro Fujita bb684c82f7 Minor code cleanup in asynchronous execution support.
This is cleanup for commit 27e1f1456:

* ExecAppendAsyncEventWait(), which was modified a bit further by commit
  a8af856d3, duplicated the same nevents calculation.  Simplify the code
  a little bit to avoid the duplication.  Update comments there.
* Add an assertion to ExecAppendAsyncRequest().
* Update a comment about merging the async_capable options from input
  relations in merge_fdw_options(), per complaint from Kyotaro Horiguchi.
* Add a comment for fetch_more_data_begin().

Author: Etsuro Fujita
Discussion: https://postgr.es/m/CAPmGK1637W30Wx3MnrReewhafn6F_0J76mrJGoFXFnpPq4QfvA%40mail.gmail.com
2021-04-23 12:00:00 +09:00
Tom Lane d479d00285 Don't crash on reference to an un-available system column.
Adopt a more consistent policy about what slot-type-specific
getsysattr functions should do when system attributes are not
available.  To wit, they should all throw the same user-oriented
error, rather than variously crashing or emitting developer-oriented
messages.

This closes a identifiable problem in commits a71cfc56b and
3fb93103a (in v13 and v12), so back-patch into those branches,
along with a test case to try to ensure we don't break it again.
It is not known that any of the former crash cases are reachable
in HEAD, but this seems like a good safety improvement in any case.

Discussion: https://postgr.es/m/141051591267657@mail.yandex.ru
2021-04-22 17:30:55 -04:00
Alvaro Herrera 43b55ec4bc
Fix uninitialized memory bug
Have interested callers of find_inheritance_children set the
detached_exist value to false prior to calling it, so that that routine
only has to set it true in the rare cases where it is necessary.  Don't
touch it otherwise.

Per buildfarm member thorntail (which reported a UBSan failure here).
2021-04-22 16:04:48 -04:00
Alvaro Herrera 8aba932251
Fix relcache inconsistency hazard in partition detach
During queries coming from ri_triggers.c, we need to omit partitions
that are marked pending detach -- otherwise, the RI query is tricked
into allowing a row into the referencing table whose corresponding row
is in the detached partition.  Which is bogus: once the detach operation
completes, the row becomes an orphan.

However, the code was not doing that in repeatable-read transactions,
because relcache kept a copy of the partition descriptor that included
the partition, and used it in the RI query.  This commit changes the
partdesc cache code to only keep descriptors that aren't dependent on
a snapshot (namely: those where no detached partition exist, and those
where detached partitions are included).  When a partdesc-without-
detached-partitions is requested, we create one afresh each time; also,
those partdescs are stored in PortalContext instead of
CacheMemoryContext.

find_inheritance_children gets a new output *detached_exist boolean,
which indicates whether any partition marked pending-detach is found.
Its "include_detached" input flag is changed to "omit_detached", because
that name captures desired the semantics more naturally.
CreatePartitionDirectory() and RelationGetPartitionDesc() arguments are
identically renamed.

This was noticed because a buildfarm member that runs with relcache
clobbering, which would not keep the improperly cached partdesc, broke
one test, which led us to realize that the expected output of that test
was bogus.  This commit also corrects that expected output.

Author: Amit Langote <amitlangote09@gmail.com>
Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/3269784.1617215412@sss.pgh.pa.us
2021-04-22 15:13:25 -04:00
Michael Paquier f3b141c482 Fix relation leak for subscribers firing triggers in logical replication
Creating a trigger on a relation to which an apply operation is
triggered would cause a relation leak once the change gets committed,
as the executor would miss that the relation needs to be closed
beforehand.  This issue got introduced with the refactoring done in
1375422c, where it becomes necessary to track relations within
es_opened_result_relations to make sure that they are closed.

We have discussed using ExecInitResultRelation() coupled with
ExecCloseResultRelations() for the relations in need of tracking by the
apply operations in the subscribers, which would simplify greatly the
opening and closing of indexes, but this requires a larger rework and
reorganization of the worker code, particularly for the tuple routing
part.  And that's not really welcome post feature freeze.  So, for now,
settle down to the same solution as TRUNCATE which is to fill in
es_opened_result_relations with the relation opened, to make sure that
ExecGetTriggerResultRel() finds them and that they get closed.

The code is lightly refactored so as a relation is not registered three
times for each DML code path, making the whole a bit easier to follow.

Reported-by: Tang Haiying, Shi Yu, Hou Zhijie
Author: Amit Langote, Masahiko Sawada, Hou Zhijie
Reviewed-by: Amit Kapila, Michael Paquier
Discussion: https://postgr.es/m/OS0PR01MB611383FA0FE92EB9DE21946AFB769@OS0PR01MB6113.jpnprd01.prod.outlook.com
2021-04-22 12:48:54 +09:00
Alvaro Herrera 7c298c6573
Add comment about extract_autovac_opts not holding lock
Per observation from Tom Lane.

Discussion: https://postgr.es/m/1901125.1617904665@sss.pgh.pa.us
2021-04-21 18:36:12 -04:00
Alvaro Herrera 7b357cc6ae
Don't add a redundant constraint when detaching a partition
On ALTER TABLE .. DETACH CONCURRENTLY, we add a new table constraint
that duplicates the partition constraint.  But if the partition already
has another constraint that implies that one, then that's unnecessary.
We were already avoiding the addition of a duplicate constraint if there
was an exact 'equal' match -- this just improves the quality of the check.

Author: Justin Pryzby <pryzby@telsasoft.com>
Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/20210410184226.GY6592@telsasoft.com
2021-04-21 18:12:05 -04:00
Peter Eisentraut d84ffffe58 Add DISTINCT to information schema usage views
Since pg_depend can contain duplicate entries, we need to eliminate
those in information schema views that build on pg_depend, using
DISTINCT.  Some of the older views already did that correctly, but
some of the more recently added ones didn't.  (In some of these views,
it might not be possible to reproduce the issue because of how the
implementation happens to deduplicate dependencies while recording
them, but it seems better to keep this consistent in all cases.)
2021-04-21 11:54:47 +02:00