Commit 449ab63505 added postgres_fdw.application_name GUC that specifies
a value for application_name configuration parameter used when postgres_fdw
establishes a connection to a foreign server. Also commit 6e0cb3dec1
allowed it to include escape sequences. Both commits added the regression
tests for the GUC, but those tests were reverted by commits 98dbef90eb and
5e64ad3697 because they were unstable and caused some buildfarm members
to report the failure.
This is the third try to add the regression test for
postgres_fdw.application_name GUC.
One of issues to make the test unstable was to have used
postgres_fdw_disconnect_all() to close the existing remote connections.
The test expected that the remote connection and its corresponding backend
at the remote server disappeared just after postgres_fdw_disconnect_all()
was executed, but it could take a bit time for them to disappear.
To make sure that they exit, this commit makes the test use
pg_terminate_backend() with the timeout at the remote server, instead.
If the timeout is set to greater than zero, this function waits until
they are actually terminated (or until the given time has passed).
Another issue was that the test didn't take into consideration the case
where postgres_fdw.application_name containing some escape sequences was
converted to the string larger than NAMEDATALEN. In this case it was
truncated to less than NAMEDATALEN when it's passed to the remote server,
but the test expected wrongly that full string of application_name was
always viewable. This commit changes the test so that it can handle that case.
Author: Fujii Masao
Reviewed-by: Masahiko Sawada, Hayato Kuroda, Kyotaro Horiguchi
Discussion: https://postgr.es/m/3220909.1631054766@sss.pgh.pa.us
Discussion: https://postgr.es/m/20211224.180006.2247635208768233073.horikyota.ntt@gmail.com
Discussion: https://postgr.es/m/e7b61420-a97b-8246-77c4-a0d48fba5a45@oss.nttdata.com
pg_basebackup is able to use gzip to compress the contents of backups
with the tar format, but there were no tests for that. This adds a
minimalistic set of tests to check the contents of such base backups,
including sanity checks on the contents generated with gzip commands.
The tests are skipped if Postgres is compiled --without-zlib, and the
checks based on the gzip command are skipped if nothing can be found,
following the same model as the existing tests for pg_receivewal.
Reviewed-by: Georgios Kokolatos
Discussion: https://postgr.es/m/Yb3GEgWwcu4wZDuA@paquier.xyz
Since d62bcc8, the directory method of walmethods.c uses the compression
method to determine which code path to take. The tar method, used by
pg_basebackup --format=t, was inconsistent regarding that, as it relied
on the compression level to check if no compression or gzip should be
used. This commit makes the code more consistent as a whole in this
file, making the tar logic use a compression method rather than
assigning COMPRESSION_NONE that would be ignored.
The options of pg_basebackup are planned to be reworked but we are not
sure yet of the shape they should have as this has some dependency with
the integration of the server-side compression for base backups, so this
is left out for the moment. This change has as benefit to make easier
the future integration of new compression methods for the tar method of
walmethods.c, for the client-side compression.
Reviewed-by: Georgios Kokolatos
Discussion: https://postgr.es/m/Yb3GEgWwcu4wZDuA@paquier.xyz
We disallow altering a column datatype within a regular table,
if the table's rowtype is used as a column type elsewhere,
because we lack code to go around and rewrite the other tables.
This restriction should apply to partitioned tables as well, but it
was not checked because ATRewriteTables and ATPrepAlterColumnType
were not on the same page about who should do it for which relkinds.
Per bug #17351 from Alexander Lakhin. Back-patch to all supported
branches.
Discussion: https://postgr.es/m/17351-6db1870f3f4f612a@postgresql.org
The ACL is printed when you add + to the command, similarly to
various other psql backslash commands.
Along the way, move the code for this into describe.c,
where it is a better fit (and can share some code).
Pavel Luzanov, reviewed by Georgios Kokolatos
Discussion: https://postgr.es/m/6d722115-6297-bc53-bb7f-5f150e765299@postgrespro.ru
The link used in the documentation is dead, and the only options to have
an access to this part of the SQL specification are not free. Like any
other books referred, just remove the link to keep some neutrality but
keep its reference.
Reported-by: Erik Rijkers
Discussion: https://postgr.es/m/989abd7d-af30-ab52-1201-bf0b4f33b872@xs4all.nl
Backpatch-through: 12
The code-coverage report says that this test doesn't increase
coverage by one single line, which I now realize is because
I made src/test/modules/test_regex/sql/test_regex_utf8.sql
to cover all the code that this would. So really it's pointless
and we should just drop it.
Up to now this has just sat there as a test you could invoke via
EXTRA_TESTS, which of course nobody does. I'm feeling encouraged
because c2e8bd275 hasn't yet broke anything, so let's try making this
run with a suitable guard condition (similar to collate.linux.utf8).
While user-defined triggers defined on a partitioned table have
a catalog definition for both it and its partitions, internal
triggers used by foreign keys defined on partitioned tables only
have a catalog definition for its partitions. This commit fixes
that so that partitioned tables get the foreign key triggers too,
just like user-defined triggers. Moreover, like user-defined
triggers, partitions' internal triggers will now also have their
tgparentid set appropriately. This is to allow subsequent commit(s)
to make the foreign key related events to be fired in some cases
using the parent table triggers instead of those of partitions'.
This also changes what tgisinternal means in some cases. Currently,
it means either that the trigger is an internal implementation object
of a foreign key constraint, or a "child" trigger on a partition
cloned from the trigger on the parent. This commit changes it to
only mean the former to avoid confusion. As for the latter, it can
be told by tgparentid being nonzero, which is now true both for user-
defined and foreign key's internal triggers.
Author: Amit Langote <amitlangote09@gmail.com>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Reviewed-by: Arne Roland <A.Roland@index.de>
Discussion: https://postgr.es/m/CA+HiwqG7LQSK+n8Bki8tWv7piHD=PnZro2y6ysU2-28JS6cfgQ@mail.gmail.com
These test cases have been commented out since citext was invented,
because at the time we had no nice way to deal with tests that
have restrictions such as requiring UTF8 encoding. But now we do
have a convention for that, ie put them into a separate test file
with an early-exit path. So let's enable these tests to run when
their prerequisites are satisfied.
(We may have to tighten the prerequisites beyond the "encoding = UTF8
and locale != C" checks made here. But let's put it on the buildfarm
and see what blows up.)
Dag Lem
Discussion: https://postgr.es/m/ygezgoacs4e.fsf_-_@sid.nimrod.no
get_rel_sync_entry(), which is called each time a change needs to be
logically replicated, is a rather hot code path in the WAL sender
sending logical changes. This code path was doing a relcache access on
relkind and relpartition for each logical change, but we only need to
know this information when building or re-building the cached
information for a relation.
Some measurements prove that this is noticeable in perf profiles,
particularly when attempting to replicate changes from relations that
are not published as these cause less overhead in the WAL sender,
delaying further the replication of changes for relations that are
published.
Issue introduced in 83fd453.
Author: Hou Zhijie
Reviewed-by: Kyotaro Horiguchi, Euler Taveira
Discussion: https://postgr.es/m/OS0PR01MB5716E863AA9E591C1F010F7A947D9@OS0PR01MB5716.jpnprd01.prod.outlook.com
Backpatch-through: 13
Under concurrency, it is possible for two sessions to be merrily locking
and releasing a tuple and marking it again as HEAP_XMAX_INVALID all the
while a third session attempts to lock it, miserably fails at it, and
then contemplates life, the universe and everything only to eventually
fail an assertion that said bit is not set. Before SKIP LOCKED that was
indeed a reasonable expectation, but alas! commit df630b0dd5 falsified
it.
This bug is as old as time itself, and even older, if you think time
begins with the oldest supported branch. Therefore, backpatch to all
supported branches.
Author: Simon Riggs <simon.riggs@enterprisedb.com>
Discussion: https://postgr.es/m/CANbhV-FeEwMnN8yuMyss7if1ZKjOKfjcgqB26n8pqu1e=q0ebg@mail.gmail.com
Improve psql so that "ALTER TABLE foo VALIDATE CONSTRAINT <TAB>"
only offers not-convalidated entries. While it's not formally
wrong to offer validated ones, there's not much point either,
and it can save some typing if we incorporate this knowledge.
David Fetter, reviewed by Aleksander Alekseev
Discussion: https://postgr.es/m/20210427002433.GB17834@fetter.org
We can revert the code changes of commit b5febc1d1 now, because
commit 9a3ddeb51 installed a real solution for the difficulty
that b5febc1d1 just dodged, namely that the planner might pick
the wrong one of several index columns nominally containing the
same value. It only matters which one we pick if we pick one
that's not returnable, and that mistake is now foreclosed.
Although both of the aforementioned commits were back-patched,
I don't feel a need to take any risk by back-patching this one.
The cases that it improves are very corner-ish.
Discussion: https://postgr.es/m/3179992.1641150853@sss.pgh.pa.us
Commit 4ace45677 failed to fix the problem fully, because the
same issue of attempting to fetch a non-returnable index column
can occur when rechecking the indexqual after using a lossy index
operator. Moreover, it broke EXPLAIN for such indexquals (which
indicates a gap in our test cases :-().
Revert the code changes of 4ace45677 in favor of adding a new field
to struct IndexOnlyScan, containing a version of the indexqual that
can be executed against the index-returned tuple without using any
non-returnable columns. (The restrictions imposed by check_index_only
guarantee this is possible, although we may have to recompute indexed
expressions.) Support construction of that during setrefs.c
processing by marking IndexOnlyScan.indextlist entries as resjunk
if they can't be returned, rather than removing them entirely.
(We could alternatively require setrefs.c to look up the IndexOptInfo
again, but abusing resjunk this way seems like a reasonably safe way
to avoid needing to do that.)
This solution isn't great from an API-stability standpoint: if there
are any extensions out there that build IndexOnlyScan structs directly,
they'll be broken in the next minor releases. However, only a very
invasive extension would be likely to do such a thing. There's no
change in the Path representation, so typical planner extensions
shouldn't have a problem.
As before, back-patch to all supported branches.
Discussion: https://postgr.es/m/3179992.1641150853@sss.pgh.pa.us
Discussion: https://postgr.es/m/17350-b5bdcf476e5badbb@postgresql.org
Adjust the error texts used for unrecognized/unsupported datetime
units so that there are just two strings to translate, not two
per datatype. Along the way, follow our usual error message style
of not double-quoting type names, and instead making sure that we
say the name is a type. Fix a couple of places in date.c that
were using the wrong one of "unrecognized" and "unsupported".
Nikhil Benesch, with a bit more editing by me
Discussion: https://postgr.es/m/CAPWqQZTURGixmbMH2_Z3ZtWGA0ANjUb9bwtkkxSxSfDeFHuM6Q@mail.gmail.com
Since 4f0b096, pgss_store() does nothing if compute_query_id is disabled
or if no other module computed a query identifier, but the top comment
of this function did not reflect that. This behavior is already
documented in its own code path, and this just removes the inconsistent
comment.
Author: Kyotaro Horiguchi
Reviewed-by: Julien Rouhaud
Discussion: https://postgr.es/m/20211122.153823.1325120762360533122.horikyota.ntt@gmail.com
Backpatch-through: 14
If an index has both returnable and non-returnable columns, and one of
the non-returnable columns is an expression using a Var that is in a
returnable column, then a query returning that expression could result
in an index-only scan plan that attempts to read the non-returnable
column, instead of recomputing the expression from the returnable
column as intended.
To fix, redefine the "indextlist" list of an IndexOnlyScan plan node
as containing null Consts in place of any non-returnable columns.
This solves the problem by preventing setrefs.c from falsely matching
to such entries. The executor is happy since it only cares about the
exposed types of the entries, and ruleutils.c doesn't care because a
correct plan won't reference those entries. I considered some other
ways to prevent setrefs.c from doing the wrong thing, but this way
seems good since (a) it allows a very localized fix, (b) it makes
the indextlist structure more compact in many cases, and (c) the
indextlist is now a more faithful representation of what the index AM
will actually produce, viz. nulls for any non-returnable columns.
This is easier to hit since we introduced included columns, but it's
possible to construct failing examples without that, as per the
added regression test. Hence, back-patch to all supported branches.
Per bug #17350 from Louis Jachiet.
Discussion: https://postgr.es/m/17350-b5bdcf476e5badbb@postgresql.org
Get rid of the "username_subquery" mechanism in favor of doing
local lookups of role names from role OIDs. The PG backend isn't
terribly smart about scalar SubLinks in SELECT output lists,
so this offers a small performance improvement, at least in
installations with more than a couple of users. In any case
the old method didn't make for particularly readable SQL code.
While at it, I removed the various custom warning messages about
failing to find an object's owner, in favor of just fatal'ing
in the local lookup function. AFAIK there is no reason any
longer to treat that as anything but a catalog-corruption case,
and certainly no reason to make translators deal with a dozen
different messages where one would do. (If it turns out that
fatal() is indeed a bad idea, we can back off to issuing
pg_log_warning() and returning an empty string, resulting in
the same behavior as before, except more consistent.)
Also drop an entirely unnecessary sub-SELECT to check on the
pg_depend status of a sequence relation: we already have a
LEFT JOIN to fetch the row of interest in the FROM clause.
Discussion: https://postgr.es/m/2460369.1640903318@sss.pgh.pa.us
Currently FreeBSD, Linux, macOS and Windows (Visual Studio) are tested.
The main goal of this integration is to make it easier to test in-development
patches across multiple platforms. This includes improving the testing done
automatically by cfbot [1] for commitfest entries. It is *not* the goal to
supersede the buildfarm.
cirrus-ci [2] was chosen because it was already in use for cfbot, allows using
full VMs, has good OS coverage and allows accessing the full test results
without authentication (like a github account). It might be worth adding
support for further CI providers, particularly ones supporting other git
forges, in the future.
To keep CI times tolerable, most platforms use pre-generated images. Some
platforms use containers, others use full VMs.
For instructions on how to enable the CI integration in a repository and
further details, see src/tools/ci/README
[1] http://cfbot.cputube.org/
[2] https://cirrus-ci.org/
Author: Andres Freund <andres@anarazel.de>
Author: Thomas Munro <tmunro@postgresql.org>
Author: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-By: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-By: Justin Pryzby <pryzby@telsasoft.com>
Reviewed-By: Thomas Munro <tmunro@postgresql.org>
Reviewed-By: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Discussion: https://postgr.es/m/20211001222752.wrz7erzh4cajvgp6@alap3.anarazel.de
dumpPublication, dumpPublicationNamespace, dumpPublicationTable, and
dumpSubscription failed to check dataOnly. This is just a latent bug,
because pg_backup_archiver.c would filter out the ArchiveEntry later;
but they're wasting cycles in data-only dumps, and the omission might
become a live bug someday. In any case, it's not good to have some
dumpFoo functions do this and some not.
On the same reasoning, make dumpPublicationNamespace follow the
same pattern as every other dumpFoo function for checking the
DUMP_COMPONENT_DEFINITION flag. (Since 5209c0ba0, we wouldn't
even get here if that flag isn't set, so checking it is just
pro forma right now. But it might not be so forever.)
Since this is just cosmetic and/or future-proofing, no need for
back-patch.
In the wake of commits 05649b88c and 5209c0ba0, findComments() and
findSecLabels() no longer use their "Archive *fout" arguments,
so get rid of those.
While doing that, I noticed that there's no very good reason why
dumpCompositeTypeColComments() should be doing its own query to fetch
the column names of the composite type, when the calling function has
just fetched the same data. Tweak it to use that query result. This
probably doesn't save a lot for most people, because since 5209c0ba0
we won't get into this code at all unless the composite type has at
least one comment. Nonetheless, it's a wasted query.
The reverted commit attempted to fix SQL specification compliance for
the cases which 6aaaa76bb left. This however broke existing behavior
which takes precedence over spec compliance so revert. The introduced
tests are left after the revert since the codepath isn't well covered.
Per bug report 17346. Backpatch down to 14 where it was introduced.
Reported-by: Andrew Bille <andrewbille@gmail.com>
Discussion: https://postgr.es/m/17346-f72b28bd1a341060@postgresql.org
The arch_filenames[] array elements were one byte too small, so that
a maximum-length filename would get corrupted if another entry
were made after it. (Noted by Thomas Munro, fix by Nathan Bossart.)
Move these arrays into a palloc'd struct, so that we aren't wasting
a few kilobytes of static data in each non-archiver process.
Add a binaryheap_reset() call to make it plain that we start the
directory scan with an empty heap. I don't think there's any live
bug of that sort, but it seems fragile, and this is very cheap
insurance.
Cleanup for commit beb4e9ba1, so no back-patch needed.
Discussion: https://postgr.es/m/CA+hUKGLHAjHuKuwtzsW7uMJF4BVPcQRL-UMZG_HM-g0y7yLkUg@mail.gmail.com
This seems like a clearer name for what it does now.
Provide a compatibility macro so that extensions don't have to convert
to the new name right away.
Discussion: https://postgr.es/m/116024.1640111629@sss.pgh.pa.us
Commit 75d22069e made SET print a warning if you tried to set an
unrecognized parameter within namespace previously reserved by an
extension. It seems better for that to be an outright error though,
for the same reason that we don't let you set unrecognized unqualified
parameter names. In any case, the preceding implementation was
inefficient and erroneous. Perform the check in a more appropriate
spot, and be more careful about prefix-match cases.
Discussion: https://postgr.es/m/116024.1640111629@sss.pgh.pa.us
Commit 6e0cb3dec1 added the tests that check that escape sequences in
postgres_fdw.application_name setting are replaced with status information
expectedly. But they were unstable and caused some buildfarm
members to report the failure. This commit reverts those unstable tests.
application_name that used when postgres_fdw establishes a connection to
a foreign server can be specified in either or both a connection parameter
of a server object and GUC postgres_fdw.application_name. This commit
allows those parameters to include escape sequences that begins with
% character. Then postgres_fdw replaces those escape sequences with
status information. For example, %d and %u are replaced with user name
and database name in local server, respectively. This feature enables us
to add information more easily to track remote transactions or queries,
into application_name of a remote connection.
Author: Hayato Kuroda
Reviewed-by: Kyotaro Horiguchi, Masahiro Ikeda, Hou Zhijie, Fujii Masao
Discussion: https://postgr.es/m/TYAPR01MB5866FAE71C66547C64616584F5EB9@TYAPR01MB5866.jpnprd01.prod.outlook.com
Discussion: https://postgr.es/m/TYCPR01MB5870D1E8B949DAF6D3B84E02F5F29@TYCPR01MB5870.jpnprd01.prod.outlook.com
This commit moves parallel vacuum related code to a new file
commands/vacuumparallel.c so that any table AM supporting indexes can
utilize parallel vacuum in order to call index AM callbacks (ambulkdelete
and amvacuumcleanup) with parallel workers.
Another reason for this refactoring is that the parallel vacuum isn't
specific to heap so it doesn't make sense to keep this code in
heap/vacuumlazy.c.
Author: Masahiko Sawada, based on suggestion from Andres Freund
Reviewed-by: Hou Zhijie, Amit Kapila, Haiying Tang
Discussion: https://www.postgresql.org/message-id/20211030212101.ae3qcouatwmy7tbr%40alap3.anarazel.de
catalog/pg_class.h was stating that REPLICA_IDENTITY_INDEX with a
dropped index is equivalent to REPLICA_IDENTITY_DEFAULT. The code tells
a different story, as it is equivalent to REPLICA_IDENTITY_NOTHING.
The behavior exists since the introduction of replica identities, and
fe7fd4e even added tests for this case but I somewhat forgot to fix this
comment.
While on it, this commit reorganizes the documentation about replica
identities on the ALTER TABLE page, and a note is added about the case
of dropped indexes with REPLICA_IDENTITY_INDEX.
Author: Michael Paquier, Wei Wang
Reviewed-by: Euler Taveira
Discussion: https://postgr.es/m/OS3PR01MB6275464AD0A681A0793F56879E759@OS3PR01MB6275.jpnprd01.prod.outlook.com
Backpatch-through: 10
One code path related to this flavor of ALTER TABLE was checking that
the relation to detach has to be a normal table or a partitioned table,
which would fail if using the command with a different relation kind.
Views, sequences and materialized views cannot be part of a partition
tree, so these would cause the command to fail anyway, but the assertion
was triggered. Foreign tables can be part of a partition tree, and
again the assertion would have failed. The simplest solution is just to
remove this assertion, so as we get the same failure as the
non-concurrent code path.
While on it, add a regression test in postgres_fdw for the concurrent
partition detach of a foreign table, as per a suggestion from Alexander
Lakhin.
Issue introduced in 71f4c8c.
Reported-by: Alexander Lakhin
Author: Michael Paquier, Alexander Lakhin
Reviewed-by: Peter Eisentraut, Kyotaro Horiguchi
Discussion: https://postgr.es/m/17339-a9e09aaf38a3457a@postgresql.org
Backpatch-through: 14
Michael Paquier