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
Extensions that define any custom GUCs should call
EmitWarningsOnPlaceholders after doing so, to help catch misspellings.
Many of our contrib modules hadn't gotten the memo on that, though.
Also add such calls to src/test/modules extensions that have GUCs.
While these aren't really user-facing, they should illustrate good
practice not faulty practice.
Shinya Kato
Discussion: https://postgr.es/m/524fa2c0a34f34b68fbfa90d0760d515@oss.nttdata.com
There's no longer any reason to fire up a separate psql run
to create these functions. (Some refactoring in the main
regression tests is also called for, but that will take
more thought.)
Discussion: https://postgr.es/m/1655733.1639871614@sss.pgh.pa.us
"git mv" all the input/*.source and output/*.source files into
the corresponding sql/ and expected/ directories. Then remove
the pg_regress and Makefile infrastructure associated with
dynamic translation.
Discussion: https://postgr.es/m/1655733.1639871614@sss.pgh.pa.us
pg_regress has long had provisions for dynamically substituting path
names into regression test scripts and result files, but use of that
feature has always been a serious pain in the neck, mainly because
updating the result files requires tedious manual editing. Let's
get rid of that in favor of passing down the paths in environment
variables.
In addition to being easier to maintain, this way is capable of
dealing with path names that require escaping at runtime, for example
paths containing single-quote marks. (There are other stumbling
blocks in the way of actually building in a path that looks like
that, but removing this one seems like a good thing to do.) The key
coding rule that makes that possible is to concatenate pieces of a
dynamically-variable string using psql's \set command, and then use
the :'variable' notation to quote and escape the string for the next
level of interpretation.
In hopes of making this change more transparent to "git blame",
I've split it into two steps. This commit adds the necessary
pg_regress.c support and changes all the *.source files in-place
so that they no longer require any dynamic translation. The next
commit will just "git mv" them into the regular sql/ and expected/
directories.
Discussion: https://postgr.es/m/1655733.1639871614@sss.pgh.pa.us
\getenv fetches the value of an environment variable into a psql
variable. This is the inverse of the \setenv command that was added
over ten years ago. We'd not seen a compelling use-case for \getenv
at the time, but upcoming regression test refactoring provides a
sufficient reason to add it now.
Discussion: https://postgr.es/m/1655733.1639871614@sss.pgh.pa.us
Our previous validator used a traditional algorithm that performed
comparison and branching one byte at a time. It's useful in that
we always know exactly how many bytes we have validated, but that
precision comes at a cost. Input validation can show up prominently
in profiles of COPY FROM, and future improvements to COPY FROM such
as parallelism or faster line parsing will put more pressure on input
validation. Hence, add fast paths for both ASCII and multibyte UTF-8:
Use bitwise operations to check 16 bytes at a time for ASCII. If
that fails, use a "shift-based" DFA on those bytes to handle the
general case, including multibyte. These paths are relatively free
of branches and thus robust against all kinds of byte patterns. With
these algorithms, UTF-8 validation is several times faster, depending
on platform and the input byte distribution.
The previous coding in pg_utf8_verifystr() is retained for short
strings and for when the fast path returns an error.
Review, performance testing, and additional hacking by: Heikki
Linakangas, Vladimir Sitnikov, Amit Khandekar, Thomas Munro, and
Greg Stark
Discussion:
https://www.postgresql.org/message-id/CAFBsxsEV_SzH%2BOLyCiyon%3DiwggSyMh_eF6A3LU2tiWf3Cy2ZQg%40mail.gmail.com
Coverity complained that parts of dumpFunc() and buildACLCommands()
were now unreachable, as indeed they are. Remove 'em.
In passing, make dumpFunc's handling of protrftypes less gratuitously
different from other fields.
This is an option consistent with what the other tools of src/bin/
(pg_checksums, pg_dump, pg_rewind and pg_basebackup) provide which is
useful for leveraging the I/O effort when testing things. This is not
to be used in a production environment.
All the regression tests of pg_upgrade are updated to use this new
option. This happens to cut at most a couple of seconds in environments
constrained on I/O, by avoiding a flush of data folder for the new
cluster upgraded.
Author: Michael Paquier
Reviewed-by: Peter Eisentraut
Discussion: https://postgr.es/m/YbrhzuBmBxS/DkfX@paquier.xyz
In the wake of commit b073c3ccd, it's necessary to grant create
permissions on the public schema to PUBLIC to get many of the
core regression test scripts to pass. That commit did so via the
quick-n-dirty expedient of adding the GRANT to the tablespace test,
which runs first. This is problematic for single-machine
replication testing, though. The least painful way to run the
regression tests on such a setup is to skip the tablespace test,
and that no longer works.
To fix, let's invent a separate "test_setup" script to run first,
and put the GRANT there. Revert b073c3ccd's changes to
the tablespace.source files.
In the future it might be good to try to reduce coupling between
the various test scripts by having test_setup create widely-used
objects, with the goal that most of the scripts could run after
having run only test_setup. That's going to take some effort,
so this commit just addresses my immediate pain point.
Discussion: https://postgr.es/m/1363170.1639763559@sss.pgh.pa.us
pg_strtouint64() is a wrapper around strtoull/strtoul/_strtoui64, but
it seems no longer necessary to have this indirection.
msvc/Solution.pm claims HAVE_STRTOULL, so the "MSVC only" part seems
unnecessary. Also, we have code in c.h to substitute alternatives for
strtoull() if not found, and that would appear to cover all currently
supported platforms, so having a further fallback in pg_strtouint64()
seems unnecessary.
Therefore, we could remove pg_strtouint64(), and use strtoull()
directly in all call sites. However, it seems useful to keep a
separate notation for parsing exactly 64-bit integers, matching the
type definition int64/uint64. For that, add new macros strtoi64() and
strtou64() in c.h as thin wrappers around strtol()/strtoul() or
strtoll()/stroull(). This makes these functions available everywhere
instead of just in the server code, and it makes the function naming
notably different from the pg_strtointNN() functions in numutils.c,
which have a different API.
Discussion: https://www.postgresql.org/message-id/flat/a3df47c9-b1b4-29f2-7e91-427baf8b75a3%40enterprisedb.com
Fix the code changed by commit 5c056b0c2 so that we always generate
RelabelType, not something else, for a cast to unspecified typmod.
Otherwise planner optimizations might not happen.
It appears we missed this point because the previous experiments were
done on type numeric: the parser undesirably generates a call on the
numeric() length-coercion function, but then numeric_support()
optimizes that down to a RelabelType, so that everything seems fine.
It misbehaves for types that have a non-optimized length coercion
function, such as bpchar.
Per report from John Naylor. Back-patch to all supported branches,
as the previous patch eventually was. Unfortunately, that no longer
includes 9.6 ... we really shouldn't put this type of change into a
nearly-EOL branch.
Discussion: https://postgr.es/m/CAFBsxsEfbFHEkouc+FSj+3K1sHipLPbEC67L0SAe-9-da8QtYg@mail.gmail.com
Per discussion, we'll limit support for old servers to those branches
that can still be built easily on modern platforms, which as of now
is 9.2 and up.
Aside from removing code that is dead per the assumption of
server >= 9.2, I tweaked the startup warning for unsupported versions
to complain about too-old servers as well as too-new ones. The
warning that "Some psql features might not work" applies precisely
to both cases.
Discussion: https://postgr.es/m/2923349.1634942313@sss.pgh.pa.us
postgres_fdw.application_name can be any string of any length
and contain even non-ASCII characters. However when it's passed
to and used as application_name in a foreign server, it's truncated
to less than NAMEDATALEN characters and any characters
other than printable ASCII ones in it will be replaced with question
marks. This commit adds these notes into the docs.
Author: Hayato Kuroda
Reviewed-by: Kyotaro Horiguchi, Fujii Masao
Discussion: https://postgr.es/m/TYCPR01MB5870D1E8B949DAF6D3B84E02F5F29@TYCPR01MB5870.jpnprd01.prod.outlook.com
Instead of referring to target backends by pid, use pgprocno. This
means that we don't have to scan the ProcArray and we can drop some
special case code for dealing with the startup process.
Discussion: https://postgr.es/m/CA%2BhUKGLYRyDaneEwz5Uya_OgFLMx5BgJfkQSD%3Dq9HmwsfRRb-w%40mail.gmail.com
Reviewed-by: Soumyadeep Chakraborty <soumyadeep2007@gmail.com>
Reviewed-by: Ashwin Agrawal <ashwinstar@gmail.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
The API spec for lookup_rowtype_tupdesc previously said you could use
either ReleaseTupleDesc or DecrTupleDescRefCount. However, the latter
choice means the caller must be certain that the returned tupdesc is
refcounted. I don't recall right now whether that was always true
when this spec was written, but it's certainly not always true since
we introduced shared record typcaches for parallel workers. That means
that callers using DecrTupleDescRefCount are dependent on typcache
behavior details that they probably shouldn't be. Hence, change the API
spec to say that you must call ReleaseTupleDesc, and fix the half-dozen
callers that weren't.
AFAICT this is just future-proofing, there's no live bug here.
So no back-patch.
Per gripe from Chapman Flack.
Discussion: https://postgr.es/m/61B901A4.1050808@anastigmatix.net
Server versions for which there was a plausible reason to
use this switch are all out of support now. Leaving it
around would accomplish little except to let careless DBAs
shoot themselves in the foot.
Discussion: https://postgr.es/m/556122.1639520324@sss.pgh.pa.us
Previously, in parallel vacuum, we allocated shmem area of
IndexBulkDeleteResult only for indexes where parallel index vacuuming is
safe and had null-bitmap in shmem area to access them. This logic was too
complicated with a small benefit of saving only a few bits per indexes.
In this commit, we allocate a dedicated shmem area for the array of
LVParallelIndStats that includes a parallel-safety flag, the index vacuum
status, and IndexBulkdeleteResult. There is one array element for every
index, even those indexes where parallel index vacuuming is unsafe or not
worthwhile. This commit makes the code clear by removing all
bitmap-related code.
Also, add the check each index vacuum status after parallel index vacuum
to make sure that all indexes have been processed.
Finally, rename parallel vacuum functions to parallel_vacuum_* for
consistency.
Author: Masahiko Sawada, based on suggestions by Andres Freund
Reviewed-by: Hou Zhijie, Amit Kapila
Discussion: https://www.postgresql.org/message-id/20211030212101.ae3qcouatwmy7tbr%40alap3.anarazel.de
edc2332 has introduced in vcregress.pl some control on the environment
variables LZ4, TAR and GZIP_PROGRAM to allow any TAP tests to be able
use those commands. This makes the settings more consistent with
src/Makefile.global.in, as the same default gets used for Make and MSVC
builds.
Each parameter can be changed in buildenv.pl, but as a default gets
assigned after loading buldenv.pl, it is not possible to unset any of
these, and using an empty value would not work with "||=" either. As
some environments may not have a compatible command in their PATH (tar
coming from MinGW is an issue, for one), this could break tests without
an exit path to bypass any failing test. This commit changes things so
as the default values for LZ4, TAR and GZIP_PROGRAM are assigned before
loading buildenv.pl, not after. This way, we keep the same amount of
compatibility as a GNU build with the same defaults, and it becomes
possible to unset any of those values.
While on it, this adds some documentation about those three variables in
the section dedicated to the TAP tests for MSVC.
Per discussion with Andrew Dunstan.
Discussion: https://postgr.es/m/YbGYe483803il3X7@paquier.xyz
Backpatch-through: 10
Per discussion, we'll limit support for old servers to those branches
that can still be built easily on modern platforms, which as of now
is 9.2 and up.
Discussion: https://postgr.es/m/2923349.1634942313@sss.pgh.pa.us
Per discussion, we'll limit support for old servers to those branches
that can still be built easily on modern platforms, which as of now
is 9.2 and up. Remove over a thousand lines of code dedicated to
dumping from older server versions. (As in previous changes of
this sort, we aren't removing pg_restore's ability to read older
archive files ... though it's fair to wonder how that might be
tested nowadays.) This cleans up some dead code left behind by
commit 989596152.
Discussion: https://postgr.es/m/2923349.1634942313@sss.pgh.pa.us
Borrow the logic that's long been used in tuplesort.c: instead
of physically swapping the data in two heap entries, keep the
value that's being sifted up or down in a local variable, and
just move the other values as necessary. This makes the code
shorter as well as faster. It's not clear that any current
callers are really time-critical enough to notice, but we
might as well code heap maintenance the same way everywhere.
Ma Liangzhu and Tom Lane
Discussion: https://postgr.es/m/17336-fc4e522d26a750fd@postgresql.org
This could only matter if (a) long is wider than int, and (b) the heap
of free blocks exceeds UINT_MAX entries, which seems pretty unlikely.
Still, it's a theoretical bug, so backpatch to v13 where the typo came
in (in commit c02fdc922).
In passing, also make swap_nodes() use consistent datatypes.
Ma Liangzhu
Discussion: https://postgr.es/m/17336-fc4e522d26a750fd@postgresql.org
When using replication origins, pg_replication_origin_xact_setup() is an
optional choice to be able to set a LSN and a timestamp to mark the
origin, which would be additionally added to WAL for transaction commits
or aborts (including 2PC transactions). An assertion in the code path
of PREPARE TRANSACTION assumed that this data should always be set, so
it would trigger when using replication origins without setting up an
origin LSN. Some tests are added to cover more this kind of scenario.
Oversight in commit 1eb6d65.
Per discussion with Amit Kapila and Masahiko Sawada.
Discussion: https://postgr.es/m/YbbBfNSvMm5nIINV@paquier.xyz
Backpatch-through: 11
Nobody has filled in these stubs for upwards of twenty years,
so it's time to drop the idea that they might get implemented
any day now. The associated pg_operator and pg_proc entries
are just confusing wastes of space.
Per complaint from Anton Voloshin.
Discussion: https://postgr.es/m/3426566.1638832718@sss.pgh.pa.us
In commit 791090bd7, I made an effort to fill in documentation
for all geometric operators listed in pg_operator. However,
it now appears that at least some of the omissions may have been
intentional, because some of those operator entries point at
unimplemented stub functions. Remove those from the docs again.
(In HEAD, poly_distance stays, because c5c192d7b just added an
implementation for it.)
Per complaint from Anton Voloshin.
Discussion: https://postgr.es/m/3426566.1638832718@sss.pgh.pa.us
geo_ops.c contains half a dozen functions that are just stubs throwing
ERRCODE_FEATURE_NOT_SUPPORTED. Since it's been like that for more
than twenty years, there's clearly not a lot of interest in filling in
the stubs. However, I'm uncomfortable with deleting poly_distance(),
since every other geometric type supports a distance-to-another-object-
of-the-same-type function. We can easily add this capability by
cribbing from poly_overlap() and path_distance().
It's possible that the (existing) test case for this will show some
numeric instability, but hopefully the buildfarm will expose it if so.
In passing, improve the documentation to try to explain why polygons
are distinct from closed paths in the first place.
Discussion: https://postgr.es/m/3426566.1638832718@sss.pgh.pa.us
When writing / debugging an isolation test it sometimes is useful to see which
session holds what lock etc. To make it easier, both as part of spec files and
interactively, append the session name to application_name. Since b1907d688
application_name already contains the test name, this appends the session's
name to that.
insert-conflict-specconflict did something like this manually, which can now
be removed.
As we have done lately with other test infrastructure improvements, backpatch
this change, to make it easier to backpatch tests.
Author: Andres Freund <andres@anarazel.de>
Reviewed-By: Michael Paquier <michael@paquier.xyz>
Reviewed-By: Andrew Dunstan <andrew@dunslane.net>
Discussion: https://postgr.es/m/20211211012052.2blmzcmxnxqawd2z@alap3.anarazel.de
Backpatch: 10-, to make backpatching of tests easier.
We need to replace windows-style \ path separators with / when putting socket
directories either in postgresql.conf or libpq connection strings, otherwise
they are interpreted as escapes.
Author: Andres Freund <andres@anarazel.de>
Reviewed-By: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Discussion: https://postgr.es/m/4da250a5-4222-1522-f14d-8a72bcf7e38e@enterprisedb.com
It's not great that RecoveryInProgress() calls InitXLOGAccess(),
because a status inquiry function typically shouldn't have the side
effect of performing initializations. We could fix that by calling
InitXLOGAccess() from some other place, but instead, let's remove it
altogether.
One thing InitXLogAccess() did is initialize wal_segment_size, but it
doesn't need to do that. In the postmaster, PostmasterMain() calls
LocalProcessControlFile(), and all child processes will inherit that
value -- except in EXEC_BACKEND bulds, but then each backend runs
SubPostmasterMain() which also calls LocalProcessControlFile().
The other thing InitXLOGAccess() did is update RedoRecPtr and
doPageWrites, but that's not critical, because all code that uses
them will just retry if it turns out that they've changed. The
only difference is that most code will now see an initial value that
is definitely invalid instead of one that might have just been way
out of date, but that will only happen once per backend lifetime,
so it shouldn't be a big deal.
Patch by me, reviewed by Nathan Bossart, Michael Paquier, Andres
Freund, Heikki Linnakangas, and Álvaro Herrera.
Discussion: http://postgr.es/m/CA+TgmoY7b65qRjzHN_tWUk8B4sJqk1vj1d31uepVzmgPnZKeLg@mail.gmail.com
The idea here is that when a performance problem is known to have
occurred at a certain point in time, it's a good thing if there is
some information available from the logs to help figure out what
might have happened around that time.
This change attracted an above-average amount of dissent, because
it means that a server with default settings will produce some amount
of log output even if nothing has gone wrong. However, by my count,
the mailing list discussion had about twice as many people in favor
of the change as opposed. The reasons for believing that the extra
log output is not an issue in practice are: (1) the rate at which
messages can be generated by this setting is bounded to one every
few minutes on a properly-configured system and (2) production
systems tend to have a lot more junk in the log from that due to
failed connection attempts, ERROR messages generated by application
activity, and the like.
Bharath Rupireddy, reviewed by Fujii Masao and by me. Many other
people commented on the thread, but as far as I can see that was
discussion of the merits of the change rather than review of the
patch.
Discussion: https://postgr.es/m/CALj2ACX-rW_OeDcp4gqrFUAkf1f50Fnh138dmkd0JkvCNQRKGA@mail.gmail.com
The multirange_get_range() function fails when two boundaries of the same
range have different alignments. Fix that by adding proper pointer alignment.
Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/17300-dced2d01ddeb1f2f%40postgresql.org
Backpatch-through: 14
This commit improves the description of some WAL records for the
Transaction RMGR:
- Track remote_apply for a transaction commit. This GUC is
user-settable, so this information can be useful for debugging.
- Add replication origin information for PREPARE TRANSACTION, with the
origin ID, LSN and timestamp
- Same as above, for ROLLBACK PREPARED.
This impacts the format of pg_waldump or anything using these
description routines, so no backpatch is done.
Author: Masahiko Sawada, Michael Paquier
Discussion: https://postgr.es/m/CAD21AoD2dJfgsdxk4_KciAZMZQoUiCvmV9sDpp8ZuKLtKCNXaA@mail.gmail.com
Historically we've put type "char" into the S (String) typcategory,
although calling it a string is a stretch considering it can only
store one byte. (In our actual usage, it's more like an enum.)
This choice now seems wrong in view of the special heuristics
that parse_func.c and parse_coerce.c have for TYPCATEGORY_STRING:
it's not a great idea for "char" to have those preferential casting
behaviors.
Worse than that, recent patches inventing special-purpose types
like pg_node_tree have assigned typcategory S to those types,
meaning they also get preferential casting treatment that's designed
on the assumption that they can hold arbitrary text.
To fix, invent a new category TYPCATEGORY_INTERNAL for internal-use
types, and assign that to all these types. I used code 'Z' for
lack of a better idea ('I' was already taken).
This change breaks one query in psql/describe.c, which now needs to
explicitly cast a catalog "char" column to text before concatenating
it with an undecorated literal. Also, a test case in contrib/citext
now needs an explicit cast to convert citext to "char". Since the
point of this change is to not have "char" be a surprisingly-available
cast target, these breakages seem OK.
Per report from Ian Campbell.
Discussion: https://postgr.es/m/2216388.1638480141@sss.pgh.pa.us
Fujii Masao