The regression tests of libpq have been converted to a TAP test as of
ac25173, but they were not run when running the global make targets
mentioned in $subject as the code only considered src/interfaces/ecpg/
as a path to run. This switches to src/interfaces/, to always include
the tests of libpq in each run.
Reported-by: Justin Pryzby
Reviewed-by: Andres Freund
Discussion: https://postgr.es/m/20220529151850.GN19626@telsasoft.com
Currently, we simply combine the column lists when publishing tables on
multiple publications and that can sometimes lead to unexpected behavior.
Say, if a column is published in any row-filtered publication, then the
values for that column are sent to the subscriber even for rows that don't
match the row filter, as long as the row matches the row filter for any
other publication, even if that other publication doesn't include the
column.
The main purpose of introducing a column list is to have statically
different shapes on publisher and subscriber or hide sensitive column
data. In both cases, it doesn't seem to make sense to combine column
lists.
So, we disallow the cases where the column list is different for the same
table when combining publications. It can be later extended to combine the
column lists for selective cases where required.
Reported-by: Alvaro Herrera
Author: Hou Zhijie
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/202204251548.mudq7jbqnh7r@alvherre.pgsql
Since a117cebd6, some older gcc versions issue "variable may be used
uninitialized in this function" complaints for brin_summarize_range.
Silence that using the same coding pattern as in bt_index_check_internal;
arguably, a117cebd6 had too narrow a view of which compilers might give
trouble.
Nathan Bossart and Tom Lane. Back-patch as the previous commit was.
Discussion: https://postgr.es/m/20220601163537.GA2331988@nathanxps13
Perl 5.36 has reclassified the warning condition that this test
case used, so that the expected error fails to appear. Tweak
the test so it instead exercises a case that's handled the same
way in all Perl versions of interest.
This appears to meet our standards for back-patching into
out-of-support branches: it changes no user-visible behavior
but enables testing of old branches with newer tools.
Hence, back-patch as far as 9.2.
Dagfinn Ilmari Mannsåker, per report from Jitka Plesníková.
Discussion: https://postgr.es/m/564579.1654093326@sss.pgh.pa.us
The example given for anyelement <@ anymultirange didn't return
true as claimed; adjust it so it does.
In passing, change a couple of sample results where the modern
numeric-based logic produces a different number of trailing zeroes
than before.
Erik Rijkers
Discussion: https://postgr.es/m/cc35735d-1ec1-5bb3-9e27-cddbab7afa23@xs4all.nl
The PostgreSQL limitations section of the documents mentioned the limit
on the number of columns that can exist in a table. Users might be
surprised to find that there's also a limit on the number of columns that
can exist in a targetlist. Users may experience restrictions which
surprise them if they happened to select a large number of columns from
several tables with many columns. Here we document that there is a
limitation on this and mention what that limit actually is.
Wording proposal by Alvaro Herrera
Reported-by: Vladimir Sitnikov
Author: Dave Crammer
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/CAB=Je-E18aTYpNqje4mT0iEADpeGLSzwUvo3H9kRRuDdsNo4aQ@mail.gmail.com
Backpatch-through: 12, where the limitations section was added
This reverts commit d9d076222f "VACUUM: ignore indexing operations
with CONCURRENTLY".
These changes caused indexes created with the CONCURRENTLY option to
miss heap tuples that were HOT-updated and HOT-pruned during the index
creation. Before these changes, HOT pruning would have been prevented
by the Xmin of the transaction creating the index, but because this
change was precisely to allow the Xmin to move forward ignoring that
backend, now other backends scanning the table can prune them. This is
not a problem for VACUUM (which requires a lock that conflicts with a
CREATE INDEX CONCURRENTLY operation), but HOT-prune can definitely
occur. In other words, Xmin advancement was sped up, but at the cost of
corrupting the resulting index.
Regrettably, this means that the new feature in PG14 that RIC/CIC on
very large tables no longer force VACUUM to retain very old tuples goes
away. We might try to implement it again in a later release, but for
now the risk of indexes missing tuples is too high and there's no easy
fix.
Backpatch to 14, where this change appeared.
Reported-by: Peter Slavov <pet.slavov@gmail.com>
Diagnosys-by: Andrey Borodin <x4mmm@yandex-team.ru>
Diagnosys-by: Michael Paquier <michael@paquier.xyz>
Diagnosys-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/17485-396609c6925b982d%40postgresql.org
We hadn't noticed this because (a) few people feed invalid
timezone abbreviation files to the server, and (b) in typical
scenarios guc.c would throw ereport(ERROR) and then transaction
abort handling would silently clean up the leaked file reference.
However, it was possible to observe file leakage warnings if one
breaks an already-active abbreviation file, because guc.c does
not throw ERROR when loading supposedly-validated settings during
session start or SIGHUP processing.
Report and fix by Kyotaro Horiguchi (cosmetic adjustments by me)
Discussion: https://postgr.es/m/20220530.173740.748502979257582392.horikyota.ntt@gmail.com
runtime.sgml contains a passing reference to the minimum server
version that pg_dump[all] can dump from. That was 7.0 for many
years, but when 64f3524e2 raised it to 8.0, we missed updating this
bit. Then when 30e7c175b raised it to 9.2, we missed it again.
Given that track record, I'm not too hopeful that we'll remember
to fix this in future changes ... but for now, make the docs match
reality in each branch.
Noted by Daniel Westermann.
Discussion: https://postgr.es/m/GV0P278MB041917EB3E2FE8704B5AE2C6D2DC9@GV0P278MB0419.CHEP278.PROD.OUTLOOK.COM
With the old logic, when the reciever had not yet attached, we would
never call shm_mq_inc_bytes_written(), even if force_flush = true
was specified. That could result in a situation where data that the
sender believes it has sent is never received.
Along the way, remove a useless function prototype for a nonexistent
function from shm_mq.h.
Commit 46846433a0 introduced these
problems.
Pavan Deolasee, with a few changes by me.
Discussion: https://postgr.es/m/CABOikdPkwtLLCTnzzmpSMXo3QZa2yXq0J7Q61ssdLFAJYrOVvQ@mail.gmail.com
foreign_data has kept around a set of tests for TRUNCATE to look after
the case of foreign tables, with[out] inheritance and with[out]
partitions, assuming that the command is not supported for this relkind.
However, TRUNCATE is supported on foreign tables if the FDW involved is
able to handle the command, like postgres_fdw.
Note that postgres_fdw includes tests to cover all the cases removed by
this commit (which had misleading comments), so these did not provide
any additional coverage anyway.
Author: Yugo Nagata
Discussion: https://postgr.es/m/20220527172543.0a2fdb469cf048b81c0967d3@sraoss.co.jp
Commit 1a36bc9db (SQL/JSON query functions) introduced STRING as a
type_func_name_keyword, thereby breaking applications that use
"string" as a table name, column name, function parameter name, etc.
That seems like a pretty bad thing, not least because the SQL spec
says that STRING is an unreserved keyword.
This is easy enough to fix so far as the core grammar is concerned.
However, doing so causes some ECPG test cases to fail, specifically
those that use "string" as a typedef name. It turns out this is
because portions of the ECPG grammar allow type_func_name_keywords
but not unreserved_keywords as typedef names. That's pretty horrid,
and it's mildly astonishing that we've not heard complaints about it
before. We can fix two of those uses trivially, but the ones in the
var_type production are less easy. As a stopgap, hard-code STRING as
an allowed alternative in var_type.
Per report from Alastair McKinley.
Discussion: https://postgr.es/m/3661437.1653855582@sss.pgh.pa.us
The information generated when track_activities is accessible to
superusers, roles with the privileges of pg_read_all_stats, as well as
roles one has the privileges of. The original text did not outline the
last point, while the change done in ac1ae47 was unclear about the
second point.
Per discussion with Nathan Bossart.
Discussion: https://postgr.es/m/20220521185743.GA886636@nathanxps13
Backpatch-through: 10
In the codepath when no encoding conversion is required, the check for
incomplete character at the end of input incorrectly used server
encoding's max character length, instead of the client's. Usually the
server and client encodings are the same when we're not performing
encoding conversion, but SQL_ASCII is an exception.
In the passing, also fix some outdated comments that still talked about
the old COPY protocol. It was removed in v14.
Per bug #17501 from Vitaly Voronov. Backpatch to v14 where this was
introduced.
Discussion: https://www.postgresql.org/message-id/17501-128b1dd039362ae6@postgresql.org
The form taking anymultirange had not been documented. This was
fixed in HEAD in b21c4cf95, but that should have been back-patched
to v14 since the function was added there. Do so now.
Also, the form taking anyrange was incorrectly documented as
returning anymultirange, when it returns anyrange.
Remove b21c4cf95 from the v15 release notes, since it no longer
qualifies as new-in-v15.
Noted by Shay Rojansky.
Discussion: https://postgr.es/m/CADT4RqAktzP7t6SFf0Xqm9YhahzvsmxFbzXe-gFOd=+_CHm0JA@mail.gmail.com
This reverts commit 06f5295 as per issues with this approach, both in
terms of efficiency impact and stability. First, contrary to the
single-item cache for transaction IDs in transam.c, the cache may finish
by not be hit for a long time, and without an invalidation mechanism to
clear it, it would cause inconsistent results on wraparound for
example. Second, the use of SubTransGetTopmostTransaction() for the
caching has a limited impact on performance. SubTransGetParent() could
have more impact, though the benchmarking of the single-item approach
still needs to be proved, particularly under the conditions where SLRU
lookups are stressed in parallel with overflowed snapshots (aka more
than 64 subxids generated, for example).
After discussion with Andres Freund.
Discussion: https://postgr.es/m/20220524235250.gtt3uu5zktfkr4hv@alap3.anarazel.de
If a short description is specified as NULL in one of the various
DefineCustomXXXVariable() functions available to external modules to
define a custom parameter, SHOW ALL would crash. This change teaches
SHOW ALL to properly handle NULL short descriptions, as well as any code
paths that manipulate it, to gain in flexibility. Note that
help_config.c was already able to do that, when describing a set of GUCs
for postgres --describe-config.
Author: Steve Chavez
Reviewed by: Nathan Bossart, Andres Freund, Michael Paquier, Tom Lane
Discussion: https://postgr.es/m/CAGRrpzY6hO-Kmykna_XvsTv8P2DshGiU6G3j8yGao4mk0CqjHA%40mail.gmail.com
Backpatch-through: 10
9d9c02ccd added code to allow the executor to take shortcuts when quals
on monotonic window functions guaranteed that once the qual became false
it could never become true again. When possible, baserestrictinfo quals
are converted to become these quals, which we call run conditions.
Unfortunately, in 9d9c02ccd, I forgot to update
remove_unused_subquery_outputs to teach it about these run conditions.
This could cause a WindowFunc column which was unused in the target list
but referenced by an upper-level WHERE clause to be removed from the
subquery when the qual in the WHERE clause was converted into a window run
condition. Because of this, the entire WindowClause would be removed from
the query resulting in additional rows making it into the resultset when
they should have been filtered out by the WHERE clause.
Here we fix this by recording which target list items in the subquery have
run conditions. That gets passed along to remove_unused_subquery_outputs
to tell it not to remove these items from the target list.
Bug: #17495
Reported-by: Jeremy Evans
Reviewed-by: Richard Guo
Discussion: https://postgr.es/m/17495-7ffe2fa0b261b9fa@postgresql.org
Commits a59c79564 et al. tried to sync libpq's SSL key file
permissions checks with what we've used for years in the backend.
We did not intend to create any new failure cases, but it turns out
we did: restricting the key file's ownership breaks cases where the
client is allowed to read a key file despite not having the identical
UID. In particular a client running as root used to be able to read
someone else's key file; and having seen that I suspect that there are
other, less-dubious use cases that this restriction breaks on some
platforms.
We don't really need an ownership check, since if we can read the key
file despite its having restricted permissions, it must have the right
ownership --- under normal conditions anyway, and the point of this
patch is that any additional corner cases where that works should be
deemed allowable, as they have been historically. Hence, just drop
the ownership check, and rearrange the permissions check to get rid
of its faulty assumption that geteuid() can't be zero. (Note that the
comparable backend-side code doesn't have to cater for geteuid() == 0,
since the server rejects that very early on.)
This does have the end result that the permissions safety check used
for a root user's private key file is weaker than that used for
anyone else's. While odd, root really ought to know what she's doing
with file permissions, so I think this is acceptable.
Per report from Yogendra Suralkar. Like the previous patch,
back-patch to all supported branches.
Discussion: https://postgr.es/m/MW3PR15MB3931DF96896DC36D21AFD47CA3D39@MW3PR15MB3931.namprd15.prod.outlook.com
Foreign tables can be partitioned, but previous documentation commits
left the syntax synopsis both incomplete and incorrect.
Justin Pryzby and Amit Langote
Discussion: http://postgr.es/m/20220521130922.GX19626@telsasoft.com
repeat() checked for integer overflow during its calculation of the
required output space, but it just passed the resulting integer to
palloc(). This meant that result sizes between 1GB and 2GB led to
ERRCODE_INTERNAL_ERROR, "invalid memory alloc request size" rather
than ERRCODE_PROGRAM_LIMIT_EXCEEDED, "requested length too large".
That seems like a bit of a wart, so add an explicit AllocSizeIsValid
check to make these error cases uniform.
Do likewise in the sibling functions lpad() etc. While we're here,
also modernize their overflow checks to use pg_mul_s32_overflow() etc
instead of expensive divisions.
Per complaint from Japin Li. This is basically cosmetic, so I don't
feel a need to back-patch.
Discussion: https://postgr.es/m/ME3P282MB16676ED32167189CB0462173B6D69@ME3P282MB1667.AUSP282.PROD.OUTLOOK.COM
Mistake in 5891c7a8ed, likely made when switching the default value from none
to fetch during development.
Reported-By: Nathan Bossart <nathandbossart@gmail.com>
Author: Nathan Bossart <nathandbossart@gmail.com>
Discussion: https://postgr.es/m/20220524220147.GA1298892@nathanxps13
"\r" (for progress output) must not be inside a translatable string
(gettext gets upset).
In passing, move the minimum supported version number to a separate
argument, so that we don't have to retranslate this string every year
now.
The changes to show all query results (7844c9918) broke \timing output
in case of an error; it didn't update the timing result and showed
0.000 ms.
Fix by updating the timing result also in the error case. Also, for
robustness, update the timing result any time a result is obtained,
not only for the last, so a sensible value is always available.
Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Author: Richard Guo <guofenglinux@gmail.com>
Author: Fabien COELHO <coelho@cri.ensmp.fr>
Discussion: https://www.postgresql.org/message-id/3813350.1652111765%40sss.pgh.pa.us
On slow machines the modified test could end up switching the order in which
transactional stats are reported in one session and non-transactional stats in
another session. As stats handling of truncate is implemented as setting
live/dead rows 0, the order in which a truncate's stats changes are applied,
relative to normal stats updates, matters. The handling of stats for truncate
hasn't changed due to shared memory stats, this is longstanding behavior.
We might want to improve truncate's stats handling in the future, but for now
just change the order of forced flushed to make the test stable.
Reported-By: Christoph Berg <myon@debian.org>
Discussion: https://postgr.es/m/YoZf7U/WmfmFYFEx@msg.df7cb.de
ruleutils.c was coded to suppress the AS label for a SELECT output
expression if the column name is "?column?", which is the parser's
fallback if it can't think of something better. This is fine, and
avoids ugly clutter, so long as (1) nothing further up in the parse
tree relies on that column name or (2) the same fallback would be
assigned when the rule or view definition is reloaded. Unfortunately
(2) is far from certain, both because ruleutils.c might print the
expression in a different form from how it was originally written
and because FigureColname's rules might change in future releases.
So we shouldn't rely on that.
Detecting exactly whether there is any outer-level use of a SELECT
column name would be rather expensive. This patch takes the simpler
approach of just passing down a flag indicating whether there *could*
be any outer use; for example, the output column names of a SubLink
are not referenceable, and we also do not care about the names exposed
by the right-hand side of a setop. This is sufficient to suppress
unwanted clutter in all but one case in the regression tests. That
seems like reasonable evidence that it won't be too much in users'
faces, while still fixing the cases we need to fix.
Per bug #17486 from Nicolas Lutic. This issue is ancient, so
back-patch to all supported branches.
Discussion: https://postgr.es/m/17486-1ad6fd786728b8af@postgresql.org
Several places in the planner tried to clamp a double value to fit
in a "long" by doing
(long) Min(x, (double) LONG_MAX);
This is subtly incorrect, because it casts LONG_MAX to double and
potentially back again. If long is 64 bits then the double value
is inexact, and the platform might round it up to LONG_MAX+1
resulting in an overflow and an undesirably negative output.
While it's not hard to rewrite the expression into a safe form,
let's put it into a common function to reduce the risk of someone
doing it wrong in future.
In principle this is a bug fix, but since the problem could only
manifest with group count estimates exceeding 2^63, it seems unlikely
that anyone has actually hit this or will do so anytime soon. We're
fixing it mainly to satisfy fuzzer-type tools. That being the case,
a HEAD-only fix seems sufficient.
Andrey Lepikhov
Discussion: https://postgr.es/m/ebbc2efb-7ef9-bf2f-1ada-d6ec48f70e58@postgrespro.ru
The description of track_activities mentioned that it is visible to
superusers and that the information related to the current session can
be seen, without telling about pg_read_all_stats. Roles that are
granted the privileges of pg_read_all_stats can also see this
information, so mention it in the docs.
Author: Ian Barwick
Reviewed-by: Nathan Bossart
Discussion: https://postgr.es/m/CAB8KJ=jhPyYFu-A5r-ZGP+Ax715mUKsMxAGcEQ9Cx_mBAmrPow@mail.gmail.com
Backpatch-through: 10
This is based on a set of suggestions from Noah, with the following
changes made:
- The set of databases created in the tests are now prefixed with
"regression" to not trigger any warnings with name restrictions when
compiling the code with -DENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS, and
now only the first name checks after the Windows case of double quotes
mixed with backslashes.
- Fix an issue with EXTRA_REGRESS_OPTS, which were not processed in a
way consistent with 027_stream_regress.pl (missing space between the
option string and pg_regress). This got introduced in 7dd3ee5.
- Add a check on the exit code of the pg_regress command, to catch
failures after running the regression tests.
Reviewed-by: Noah Misch
Discussion: https://postgr.es/m/YoHhWD5vQzb2mmiF@paquier.xyz
This new-in-v15 test case assumed it could set max_stack_depth as high
as 2MB. You might think that'd be true on any modern platform but
you'd be wrong, as I found out while experimenting with NetBSD/hppa.
This test is about privileges not platform capabilities, so there seems
no need to use any value greater than the 100kB setting already used
in a couple of places in the core regression tests. There's certainly
no call to expect people to raise their platform's default ulimit just
to run this test.
When an implicit operator family is created, it wasn't getting reported.
Make it do so.
This has always been missing. Backpatch to 10.
Author: Masahiko Sawada <sawada.mshk@gmail.com>
Reported-by: Leslie LEMAIRE <leslie.lemaire@developpement-durable.gouv.fr>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Reviewed-by: Michael Paquiër <michael@paquier.xyz>
Discussion: https://postgr.es/m/f74d69e151b22171e8829551b1159e77@developpement-durable.gouv.fr
_pg_version (version number based on PostgreSQL::Version) is a field
private to Cluster.pm but there was no helper routine to retrieve it
from a Cluster's node. The same is done for install_path, for example,
and the version object becomes handy when writing tests that need
version-specific handling.
Reviewed-by: Andrew Dunstan, Daniel Gustafsson
Discussion: https://postgr.es/m/YoWfoJTc987tsxpV@paquier.xyz
Michael Paquier
Etsuro Fujita
Amit Kapila
Tom Lane
David Rowley
Magnus Hagander
Alvaro Herrera
Robert Haas
Thomas Munro
Peter Eisentraut
Heikki Linnakangas
Andres Freund
Bruce Momjian
John Naylor