Before, SQL-level boolean constants were represented by a string with
a cast, and internal Boolean values in DDL commands were usually
represented by Integer nodes. This takes the place of both of these
uses, making the intent clearer and having some amount of type safety.
Reviewed-by: Pavel Stehule <pavel.stehule@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/8c1a2e37-c68d-703c-5a83-7a6077f4f997@enterprisedb.com
The existing cryptohash facility was causing problems in some code paths
related to MD5 (frontend and backend) that relied on the fact that the
only type of error that could happen would be an OOM, as the MD5
implementation used in PostgreSQL ~13 (the in-core implementation is
used when compiling with or without OpenSSL in those older versions),
could fail only under this circumstance.
The new cryptohash facilities can fail for reasons other than OOMs, like
attempting MD5 when FIPS is enabled (upstream OpenSSL allows that up to
1.0.2, Fedora and Photon patch OpenSSL 1.1.1 to allow that), so this
would cause incorrect reports to show up.
This commit extends the cryptohash APIs so as callers of those routines
can fetch more context when an error happens, by using a new routine
called pg_cryptohash_error(). The error states are stored within each
implementation's internal context data, so as it is possible to extend
the logic depending on what's suited for an implementation. The default
implementation requires few error states, but OpenSSL could report
various issues depending on its internal state so more is needed in
cryptohash_openssl.c, and the code is shaped so as we are always able to
grab the necessary information.
The core code is changed to adapt to the new error routine, painting
more "const" across the call stack where the static errors are stored,
particularly in authentication code paths on variables that provide
log details. This way, any future changes would warn if attempting to
free these strings. The MD5 authentication code was also a bit blurry
about the handling of "logdetail" (LOG sent to the postmaster), so
improve the comments related that, while on it.
The origin of the problem is 87ae969, that introduced the centralized
cryptohash facility. Extra changes are done for pgcrypto in v14 for the
non-OpenSSL code path to cope with the improvements done by this
commit.
Reported-by: Michael Mühlbeyer
Author: Michael Paquier
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/89B7F072-5BBE-4C92-903E-D83E865D9367@trivadis.com
Backpatch-through: 14
If contrib/btree_gist is used to make a GIST index on a char(N)
(bpchar) column, and that column is retrieved via an index-only
scan, what came out had all trailing spaces removed. Since
that doesn't happen in any other kind of table scan, this is
clearly a bug. The cause is that gbt_bpchar_compress() strips
trailing spaces (using rtrim1) before a new index entry is made.
That was probably a good idea when this code was first written,
but since we invented index-only scans, it's not so good.
One answer could be to mark this opclass as incapable of index-only
scans. But to do so, we'd need an extension module version bump,
followed by manual action by DBAs to install the updated version
of btree_gist. And it's not really a desirable place to end up,
anyway.
Instead, let's fix the code by removing the unwanted space-stripping
action and adjusting the opclass's comparison logic to ignore
trailing spaces as bpchar normally does. This will not hinder
cases that work today, since index searches with this logic will
act the same whether trailing spaces are stored or not. It will
not by itself fix the problem of getting space-stripped results
from index-only scans, of course. Users who care about that can
REINDEX affected indexes after installing this update, to immediately
replace all improperly-truncated index entries. Otherwise, it can
be expected that the index's behavior will change incrementally as
old entries are replaced by new ones.
Per report from Alexander Lakhin. Back-patch to all supported branches.
Discussion: https://postgr.es/m/696c995b-b37f-5526-f45d-04abe713179f@gmail.com
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
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
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
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
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 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
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
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
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
When aborting remote transaction or sending cancel request to a remote server,
postgres_fdw calls pgfdw_get_cleanup_result() to wait for the result of
transaction abort query or cancel request to arrive. It fails to get the result
if the timeout expires or a connection trouble happens.
Previously postgres_fdw reported no warning message even when the timeout
expired or a connection trouble happened in pgfdw_get_cleanup_result().
This could make the troubleshooting harder when such an event occurred.
This commit makes pgfdw_get_cleanup_result() tell its caller what trouble
(timeout or connection error) occurred, on failure, and also makes its caller
report the proper warning message based on that information.
Author: Fujii Masao
Reviewed-by: Bharath Rupireddy
Discussion: https://postgr.es/m/15aa988c-722e-ad3e-c936-4420c5b2bfea@oss.nttdata.com
pgfdw_report_error() in postgres_fdw gets a message from PGresult or
PGconn to report an error received from a remote server. Previously
if it could get a message from neither of them, it reported empty
message unexpectedly. The cause of this issue was that pgfdw_report_error()
didn't handle properly the case where no message could be obtained
and its local variable message_primary was set to '\0'.
This commit improves pgfdw_report_error() so that it reports the message
"could not obtain ..." when it gets no message and message_primary
is set to '\0'. This is the same behavior as when message_primary is NULL.
dblink_res_error() in dblink has the same issue, so this commit also
improves it in the same way.
Back-patch to all supported branches.
Author: Fujii Masao
Reviewed-by: Bharath Rupireddy
Discussion: https://postgr.es/m/477c16c8-7ea4-20fc-38d5-ed3a77ed616c@oss.nttdata.com
psql's lexer has historically deleted dash-dash (single-line) comments
from what's collected and sent to the server. This is inconsistent
with what it does for slash-star comments, and people have complained
before that they wish such comments would be captured in the server log.
Undoing the decision completely seems like too big a behavioral change,
however. In particular, comments on lines preceding the start of a
query are generally not thought of as being part of that query.
What we can do to improve the situation is to capture comments that
are clearly *within* a query, that is after the first non-whitespace,
non-comment token but before the query's ending semicolon or backslash
command. This is a nearly trivial code change, and it affects only a
few regression test results.
(It is tempting to try to apply the same rule to slash-star comments.
But it's hard to see how to do that without getting strange history
behavior for comments that cross lines, especially if the user then
starts a new query on the same line as the star-slash. In view of
the lack of complaints, let's leave that case alone.)
Discussion: https://postgr.es/m/CAJcOf-cAdMVr7azeYR7nWKsNp7qhORzc84rV6d7m7knG5Hrtsw@mail.gmail.com
fsstate in process_pending_requests (in postgres_fdw.c) was added in
8998e3cafa as an assertion-only variable, 1ec7fca859 stated using
the variable outside of assertions.
rd_index in get_index_column_opclass (in lsyscache.c) was introduced
in 2a6368343f, and then promptly used in the fix commit 7e04160390
shortly thereafter.
This removes the PG_USED_FOR_ASSERTS_ONLY variable decoration from
the above mentioned variables.
Reviewed-by: Greg Nancarrow <gregn4422@gmail.com>
Discussion: https://postgr.es/m/F959106C-0F21-43A5-B2AE-D007D51ACBEE@yesql.se
Standardize on xoroshiro128** as our basic PRNG algorithm, eliminating
a bunch of platform dependencies as well as fundamentally-obsolete PRNG
code. In addition, this API replacement will ease replacing the
algorithm again in future, should that become necessary.
xoroshiro128** is a few percent slower than the drand48 family,
but it can produce full-width 64-bit random values not only 48-bit,
and it should be much more trustworthy. It's likely to be noticeably
faster than the platform's random(), depending on which platform you
are thinking about; and we can have non-global state vectors easily,
unlike with random(). It is not cryptographically strong, but neither
are the functions it replaces.
Fabien Coelho, reviewed by Dean Rasheed, Aleksander Alekseev, and myself
Discussion: https://postgr.es/m/alpine.DEB.2.22.394.2105241211230.165418@pseudo
Memoize would always use the hash equality operator for the cache key
types to determine if the current set of parameters were the same as some
previously cached set. Certain types such as floating points where -0.0
and +0.0 differ in their binary representation but are classed as equal by
the hash equality operator may cause problems as unless the join uses the
same operator it's possible that whichever join operator is being used
would be able to distinguish the two values. In which case we may
accidentally return in the incorrect rows out of the cache.
To fix this here we add a binary mode to Memoize to allow it to the
current set of parameters to previously cached values by comparing
bit-by-bit rather than logically using the hash equality operator. This
binary mode is always used for LATERAL joins and it's used for normal
joins when any of the join operators are not hashable.
Reported-by: Tom Lane
Author: David Rowley
Discussion: https://postgr.es/m/3004308.1632952496@sss.pgh.pa.us
Backpatch-through: 14, where Memoize was added
This commit adds a set of functions able to look at the contents of
various paths related to replication slots:
- pg_ls_logicalsnapdir, for pg_logical/snapshots/
- pg_ls_logicalmapdir, for pg_logical/mappings/
- pg_ls_replslotdir, for pg_replslot/<slot_name>/
These are intended to be used by monitoring tools. Unlike pg_ls_dir(),
execution permission can be granted to non-superusers. Roles members of
pg_monitor gain have access to those functions.
Bump catalog version.
Author: Bharath Rupireddy
Reviewed-by: Nathan Bossart, Justin Pryzby
Discussion: https://postgr.es/m/CALj2ACWsfizZjMN6bzzdxOk1ADQQeSw8HhEjhmVXn_Pu+7VzLw@mail.gmail.com
Commit 81d5995b4b introduced more fine-grained errormessages for
incorrect relkinds for publication, while unlogged and temporary
tables were reported with using the same message. This provides
separate error messages for these types of relpersistence.
Author: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Reviewed-by: Jeevan Ladhe <jeevan.ladhe@enterprisedb.com>
Reviewed-by: Euler Taveira <euler@eulerto.com>
Discussion: https://postgr.es/m/CALj2ACW9S=AswyQHjtO6WMcsergMkCBTtzXGrM8DX26DzfeTLQ@mail.gmail.com
When deparsing an expression of the form "remote_var OP constant",
we'd normally apply a cast to the constant to make sure that the
remote parser thinks it's of the same type we do. However, doing
so is often not necessary, and it causes problems if the user has
intentionally declared the local column as being of a different
type than the remote column. A plausible use-case for that is
using text to represent a type that's an enum on the remote side.
A comparison on such a column will get shipped as "var = 'foo'::text",
which blows up on the remote side because there's no enum = text
operator. But if we simply leave off the explicit cast, the
comparison will do exactly what the user wants.
It's possible to do this without major risk of semantic problems, by
relying on the longstanding parser heuristic that "if one operand of
an operator is of type unknown, while the other one has a known type,
assume that the unknown operand is also of that type". Hence, this
patch leaves off the cast only if (a) the operator inputs have the same
type locally; (b) the constant will print as a string literal or NULL,
both of which are initially taken as type unknown; and (c) the non-Const
input is a plain foreign Var. Rule (c) guarantees that the remote
parser will know the type of the non-Const input; moreover, it means
that if this cast-omission does cause any semantic surprises, that can
only happen in cases where the local column has a different type than
the remote column. That wasn't guaranteed to work anyway, and this
patch should represent a net usability gain for such cases.
One point that I (tgl) remain slightly uncomfortable with is that we
will ignore an implicit RelabelType when deciding if the non-Const input
is a plain Var. That makes it a little squishy to argue that the remote
should resolve the Const as being of the same type as its Var, because
then our Const is not the same type as our Var. However, if we don't do
that, then this hack won't work as desired if the user chooses to use
varchar rather than text to represent some remote column. That seems
useful, so do it like this for now. We might have to give up the
RelabelType-ignoring bit if any problems surface.
Dian Fay, with review and kibitzing by me
Discussion: https://postgr.es/m/C9LU294V7K4F.34LRRDU449O45@lamia
Commit 57e3c5160b added a new GiST bool opclass, but it used gbtreekey4
to store the data, which left two bytes undefined, as reported by skink,
our valgrind animal. There was a bit more confusion, because the opclass
also used gbtreekey8 in the definition.
Fix by defining a new gbtreekey2 struct, and using it in all the places.
Discussion: https://postgr.es/m/CAE2gYzyDKJBZngssR84VGZEN=Ux=V9FV23QfPgo+7-yYnKKg4g@mail.gmail.com
Re-ordering the #include's is a bit problematic here because
libpq/libpq-be.h needs to include <openssl/ssl.h>. Instead,
let's #undef the unwanted macro after all the #includes.
This is definitely uglier than the other way, but it should
work despite possible future header rearrangements.
(A look at the openssl headers indicates that X509_NAME is the
only conflicting symbol that we use.)
In passing, remove a related but long-incorrect comment in
pg_backup_archiver.h.
Discussion: https://postgr.es/m/1051867.1635720347@sss.pgh.pa.us
Adds bool opclass to btree_gist extension, to allow creating GiST
indexes on bool columns. GiST indexes on a single bool column don't seem
particularly useful, but this allows defining exclusion constraings
involving a bool column, for example.
Author: Emre Hasegeli
Reviewed-by: Andrey Borodin
Discussion: https://postgr.es/m/CAE2gYzyDKJBZngssR84VGZEN=Ux=V9FV23QfPgo+7-yYnKKg4g@mail.gmail.com
Expand the checks of toasted attributes to complain if the rawsize is
overlarge. For compressed attributes, also complain if compression
appears to have expanded the attribute or if the compression method is
invalid.
Mark Dilger, reviewed by Justin Pryzby, Alexander Alekseev, Heikki
Linnakangas, Greg Stark, and me.
Discussion: http://postgr.es/m/8E42250D-586A-4A27-B317-8B062C3816A8@enterprisedb.com
pgcrypto had internal implementations of some encryption algorithms,
as an alternative to calling out to OpenSSL. These were rarely used,
since most production installations are built with OpenSSL. Moreover,
maintaining parallel code paths makes the code more complex and
difficult to maintain.
This patch removes these internal implementations. Now, pgcrypto is
only built if OpenSSL support is configured.
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://www.postgresql.org/message-id/flat/0b42f1df-8cba-6a30-77d7-acc241cc88c1%40enterprisedb.com
In the same spirit as 6301c3ada, fix some more places where we were
using list_delete_first() in a loop and thereby risking O(N^2)
behavior. It's not clear that the lists manipulated in these spots
can get long enough to be really problematic ... but it's not clear
that they can't, either, and the fixes are simple enough.
As before, back-patch to v13.
Discussion: https://postgr.es/m/CD2F0E7F-9822-45EC-A411-AE56F14DEA9F@amazon.com
Windows fails on a request to read() more than INT_MAX bytes,
and perhaps other platforms could have similar issues. Let's
adjust this code to read at most 1GB per call.
(One would not have thought the file could get that big, but now
we have a field report of trouble, so it can. We likely ought to
add some mechanism to limit the size of the query-texts file
separately from the size of the hash table. That is not this
patch, though.)
Per bug #17254 from Yusuke Egashira. It's been like this for
awhile, so back-patch to all supported branches.
Discussion: https://postgr.es/m/17254-a926c89dc03375c2@postgresql.org
Commits fdd965d07 and 3cd9c3b92 tested CREATE INDEX CONCURRENTLY by
launching two separate pgbench runs concurrently. This was needed so
that only a single client thread would run CREATE INDEX CONCURRENTLY,
avoiding deadlock between two CICs. However, there's a better way,
which is to use an advisory lock to prevent concurrent CICs. That's
better in part because the test code is shorter and more readable, but
mostly because it automatically scales things to launch an appropriate
number of CICs relative to the number of INSERT transactions.
As committed, typically half to three-quarters of the CIC transactions
were pointless because the INSERT transactions had already stopped.
In passing, remove background_pgbench, which was added to support
these tests and isn't needed anymore. We can always put it back
if we find a use for it later.
Back-patch to v12; older pgbench versions lack the
conditional-execution features needed for this method.
Tom Lane and Andrey Borodin
Discussion: https://postgr.es/m/139687.1635277318@sss.pgh.pa.us
The foreign data wrapper's validator function provides a HINT message with
list of valid options for the object specified in CREATE or ALTER command,
when the option given in the command is invalid. Previously
postgresql_fdw_validator() and the validator functions for postgres_fdw and
dblink_fdw worked in that way even there were no valid options in the object,
which could lead to the HINT message with empty list (because there were
no valid options). For example, ALTER FOREIGN DATA WRAPPER postgres_fdw
OPTIONS (format 'csv') reported the following ERROR and HINT messages.
This behavior was confusing.
ERROR: invalid option "format"
HINT: Valid options in this context are:
There is no such issue in file_fdw. The validator function for file_fdw
reports the HINT message "There are no valid options in this context."
instead in that case.
This commit improves postgresql_fdw_validator() and the validator functions
for postgres_fdw and dblink_fdw so that they do likewise. For example,
this change causes the above ALTER FOREIGN DATA WRAPPER command to
report the following messages.
ERROR: invalid option "nonexistent"
HINT: There are no valid options in this context.
Author: Kosei Masumura
Reviewed-by: Bharath Rupireddy, Fujii Masao
Discussion: https://postgr.es/m/557d06cebe19081bfcc83ee2affc98d3@oss.nttdata.com
The five modules in our TAP test framework all had names in the top
level namespace. This is unwise because, even though we're not
exporting them to CPAN, the names can leak, for example if they are
exported by the RPM build process. We therefore move the modules to the
PostgreSQL::Test namespace. In the process PostgresNode is renamed to
Cluster, and TestLib is renamed to Utils. PostgresVersion becomes simply
PostgreSQL::Version, to avoid possible confusion about what it's the
version of.
Discussion: https://postgr.es/m/aede93a4-7d92-ef26-398f-5094944c2504@dunslane.net
Reviewed by Erik Rijkers and Michael Paquier
The purpose of commit 8a54e12a38 was to
fix this, and it sufficed when the PREPARE TRANSACTION completed before
the CIC looked for lock conflicts. Otherwise, things still broke. As
before, in a cluster having used CIC while having enabled prepared
transactions, queries that use the resulting index can silently fail to
find rows. It may be necessary to reindex to recover from past
occurrences; REINDEX CONCURRENTLY suffices. Fix this for future index
builds by making CIC wait for arbitrarily-recent prepared transactions
and for ordinary transactions that may yet PREPARE TRANSACTION. As part
of that, have PREPARE TRANSACTION transfer locks to its dummy PGPROC
before it calls ProcArrayClearTransaction(). Back-patch to 9.6 (all
supported versions).
Andrey Borodin, reviewed (in earlier versions) by Andres Freund.
Discussion: https://postgr.es/m/01824242-AA92-4FE9-9BA7-AEBAFFEA3D0C@yandex-team.ru
CIC and REINDEX CONCURRENTLY assume backends see their catalog changes
no later than each backend's next transaction start. That failed to
hold when a backend absorbed a relevant invalidation in the middle of
running RelationBuildDesc() on the CIC index. Queries that use the
resulting index can silently fail to find rows. Fix this for future
index builds by making RelationBuildDesc() loop until it finishes
without accepting a relevant invalidation. It may be necessary to
reindex to recover from past occurrences; REINDEX CONCURRENTLY suffices.
Back-patch to 9.6 (all supported versions).
Noah Misch and Andrey Borodin, reviewed (in earlier versions) by Andres
Freund.
Discussion: https://postgr.es/m/20210730022548.GA1940096@gust.leadboat.com
Incrementing the level of the call stack reported is useful for
debugging purposes as it allows to control which part of the test is
exactly failing, especially if a test is structured with subroutines
that call routines from Test::More.
This adds more incrementations of $Test::Builder::Level where debugging
gets improved (for example it does not make sense for some paths like
pg_rewind where long subroutines are used).
A note is added to src/test/perl/README about that, based on a
suggestion from Andrew Dunstan and a wording coming from both of us.
Usage of Test::Builder::Level has spread in 12, so a backpatch down to
this version is done.
Reviewed-by: Andrew Dunstan, Peter Eisentraut, Daniel Gustafsson
Discussion: https://postgr.es/m/YV1CCFwgM1RV1LeS@paquier.xyz
Backpatch-through: 12
Peter Eisentraut