The common recipe when TAP tests are disabled doesn't work, because the
libpq-specific recipe wants to define the PATH environment variable, so
the starting '@' is misinterpreted as part of the command instead of
silencing said command.
Fix by setting the environment variable in a way that doesn't interfere
with the recipe.
Reported-by: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/20220720172321.GL12702@telsasoft.com
The dependency logic failed to register a column-level dependency
when a view or rule contains a reference to a specific column of
the result of a function-returning-composite. That meant you could
drop the column from the composite type, causing trouble for future
executions of the view. We've known about this for years, but never
summoned the energy to actually fix it, instead installing various
low-level defenses to prevent crashing on references to dropped columns.
We had to do that to plug the hole in stable branches, where there might
be pre-existing broken references; but let's fix the root cause today.
To do that, add some logic (borrowed from get_rte_attribute_is_dropped)
to find_expr_references_walker, to check whether a Var referencing an
RTE_FUNCTION RTE is referencing a column of a composite type, and if
so add the proper dependency.
However ... it seems mighty unwise to remove said low-level defenses,
since there could be other bugs now or in the future that allow
reaching them. By the same token, letting those defenses go untested
seems unwise. Hence, rather than just dropping the associated test
cases, hack them to continue working by the expedient of manually
dropping the pg_depend entries that this fix installs.
Back-patch into v15. I don't want to risk changing this behavior
in stable branches, but it seems not too late for v15. (Since
we have already forced initdb for beta3, we can be sure that all
production v15 installations will have these added dependencies.)
Discussion: https://postgr.es/m/182492.1658431155@sss.pgh.pa.us
Commit c6f2f016 added an explicit check for a Windows "junction point".
That turned out to be needed only because get_dirent_type() was busted
on Windows. It's been fixed by commit 9d3444dc, so remove it.
Add a TAP-test to demonstrate that in-place tablespaces are copied by
pg_basebackup. This exercises the codepath that would fail before
c6f2f016 on Windows, and shows that it still doesn't fail now that we're
using get_dirent_type() on both Windows and Unix.
Back-patch to 15, where in-place tablespaces arrived and caused this
problem (ie directories where previously only symlinks were expected).
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/CA%2BhUKGLzLK4PUPx0_AwXEWXOYAejU%3D7XpxnYE55Y%2Be7hB2N3FA%40mail.gmail.com
Commit 87e6ed7c8 added code that intended to report Windows "junction
points" as DT_LNK (the same way we report symlinks on Unix). Windows
junction points are *also* directories according to the Windows
attributes API, and we were reporting them as as DT_DIR. Change the
order we check the attribute flags, to prioritize DT_LNK.
If at some point we start using Windows' recently added real symlinks
and need to distinguish them from junction points, we may need to
rethink this, but for now this continues the tradition of wrapper
functions that treat junction points as symlinks.
Back-patch to 14, where get_dirent_type() landed.
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/CA%2BhUKGLzLK4PUPx0_AwXEWXOYAejU%3D7XpxnYE55Y%2Be7hB2N3FA%40mail.gmail.com
Discussion: https://postgr.es/m/20220721111751.x7hod2xgrd76xr5c%40alvherre.pgsql
When postgres_fdw begins an asynchronous data fetch, it submits FETCH query
by using PQsendQuery(). If PQsendQuery() fails and returns 0, postgres_fdw
should report an error. But, previously, postgres_fdw reported an error
only when the return value is less than 0, though PQsendQuery() never return
the values other than 0 and 1. Therefore postgres_fdw could not handle
the failure to send FETCH query in an asynchronous data fetch.
This commit fixes postgres_fdw so that it reports an error
when PQsendQuery() returns 0.
Back-patch to v14 where asynchronous execution was supported in postgres_fdw.
Author: Fujii Masao
Reviewed-by: Japin Li, Tom Lane
Discussion: https://postgr.es/m/b187a7cf-d4e3-5a32-4d01-8383677797f3@oss.nttdata.com
Due to lack of concern for the case in the dependency code, it's
possible to drop a column of a composite type even though stored
queries have references to the dropped column via functions-in-FROM
that return the composite type. There are "soft" references,
namely FROM-clause aliases for such columns, and "hard" references,
that is actual Vars referring to them. The right fix for hard
references is to add dependencies preventing the drop; something
we've known for many years and not done (and this commit still doesn't
address it). A "soft" reference shouldn't prevent a drop though.
We've been around on this before (cf. 9b35ddce9, 2c4debbd0), but
nobody had noticed that the current behavior can result in dump/reload
failures, because ruleutils.c can print more column aliases than the
underlying composite type now has. So we need to rejigger the
column-alias-handling code to treat such columns as dropped and not
print aliases for them.
Rather than writing new code for this, I used expandRTE() which already
knows how to figure out which function result columns are dropped.
I'd initially thought maybe we could use expandRTE() in all cases, but
that fails for EXPLAIN's purposes, because the planner strips a lot of
RTE infrastructure that expandRTE() needs. So this patch just uses it
for unplanned function RTEs and otherwise does things the old way.
If there is a hard reference (Var), then removing the column alias
causes us to fail to print the Var, since there's no longer a name
to print. Failing seems less desirable than printing a made-up
name, so I made it print "?dropped?column?" instead.
Per report from Timo Stolz. Back-patch to all supported branches.
Discussion: https://postgr.es/m/5c91267e-3b6d-5795-189c-d15a55d61dbb@nullachtvierzehn.de
Most of these have been introduced in d2d3547 with the new pattern
validation logic, and would leak memory worth an amount of one
PQExpBuffer each time (as of 256 bytes at minimum, possibly more).
Most of the patch has been written by Tang Haiying, with a few tweaks
coming from Álvaro Herrera.
Reported-by: Tang Haiying
Author: Tang Haiying, Álvaro Herrera
Reviewed-by: Mark Dilger, Andres Freund, Álvaro Herrera, Tom Lane, Japin
Li, Michael Paquier, Junwang Zhao
Backpatch-through: 15
Without processing shared_preload_libraries, it's impossible to
recover if custom WAL resource managers are needed. It may also pose a
problem running VACUUM on a table with a custom AM, if the module
implementing the AM is expecting to be loaded by
shared_preload_libraries.
The reason this wasn't done before was just the general principle to
do fewer things in single-user mode. But it's easy enough to just set
shared_preload_libraries to empty, for the same effect.
Discussion: https://postgr.es/m/9decc18a42634f8a2f15c97a385a0f51a752f396.camel%40j-davis.com
Reviewed-by: Tom Lane, Andres Freund
Backpatch-through: 15
When a non-exclusive backup is canceled, do_pg_abort_backup() is called
and resets some variables set by pg_backup_start (pg_start_backup in v14
or before). But previously it forgot to reset the session state indicating
whether a non-exclusive backup is in progress or not in this session.
This issue could cause an assertion failure when the session running
BASE_BACKUP is terminated after it executed pg_backup_start and
pg_backup_stop (pg_stop_backup in v14 or before). Also it could cause
a segmentation fault when pg_backup_stop is called after BASE_BACKUP
in the same session is canceled.
This commit fixes the issue by making do_pg_abort_backup reset
that session state.
Back-patch to all supported branches.
Author: Fujii Masao
Reviewed-by: Kyotaro Horiguchi, Masahiko Sawada, Michael Paquier, Robert Haas
Discussion: https://postgr.es/m/3374718f-9fbf-a950-6d66-d973e027f44c@oss.nttdata.com
Multiple non-exclusive backups are able to be run conrrently in different
sessions. But, in the same session, only one non-exclusive backup can be
run at the same moment. If pg_backup_start (pg_start_backup in v14 or before)
is called in the middle of another non-exclusive backup in the same session,
an error is thrown.
However, previously, in logical replication walsender mode, even if that
walsender session had already called pg_backup_start and started
a non-exclusive backup, it could execute BASE_BACKUP command and
start another non-exclusive backup. Which caused subsequent pg_backup_stop
to throw an error because BASE_BACKUP unexpectedly reset the session state
marked by pg_backup_start.
This commit prevents BASE_BACKUP command in the middle of another
non-exclusive backup in the same session.
Back-patch to all supported branches.
Author: Fujii Masao
Reviewed-by: Kyotaro Horiguchi, Masahiko Sawada, Michael Paquier, Robert Haas
Discussion: https://postgr.es/m/3374718f-9fbf-a950-6d66-d973e027f44c@oss.nttdata.com
Detail and hint messages should be full sentences and should end with a
period, but some of the messages newly-introduced in v15 did not follow
that.
Author: Justin Pryzby
Reviewed-by: Álvaro Herrera
Discussion: https://postgr.es/m/20220719120948.GF12702@telsasoft.com
Backpatch-through: 15
We allow users to set the values of not-yet-loaded extension GUCs,
remembering those values in "placeholder" GUC entries. When/if
the extension is loaded later in the session, we need to verify that
the user had permissions to set the GUC. That was done correctly
before commit a0ffa885e, but as of that commit, we'd check the
permissions of the active role when the LOAD happens, not the role
that had set the value. (This'd be a security bug if it had made it
into a released version.)
In principle this is simple enough to fix: we just need to remember
the exact role OID that set each GUC value, and use that not
GetUserID() when verifying permissions. Maintaining that data in
the guc.c data structures is slightly tedious, but fortunately it's
all basically just copy-n-paste of the logic for tracking the
GucSource of each setting, as we were already doing.
Another oversight is that validate_option_array_item() hadn't
been taught to check for granted GUC privileges. This appears
to manifest only in that ALTER ROLE/DATABASE RESET ALL will
fail to reset settings that the user should be allowed to reset.
Patch by myself and Nathan Bossart, per report from Nathan Bossart.
Back-patch to v15 where the faulty code came in.
Discussion: https://postgr.es/m/20220706224727.GA2158260@nathanxps13
This fixes problems on windows when logging collector is used in a service,
failing with:
FATAL: could not redirect stderr: Bad file descriptor
This is triggered by 76e38b37a5. The problem is that STDOUT/STDERR_FILENO
aren't defined on windows, which lead us to use _fileno(stdout) etc, but that
doesn't work if stdout/stderr are closed.
Author: Andres Freund <andres@anarazel.de>
Reported-By: Sandeep Thakkar <sandeep.thakkar@enterprisedb.com>
Message-Id: 20220520164558.ozb7lm6unakqzezi@alap3.anarazel.de (on pgsql-packagers)
Backpatch: 15-, where 76e38b37a5 came in
Because they are not available we've used _fileno(stdin) in some places, but
that doesn't reliably work, because stdin might be closed. This is the
prerequisite of the subsequent commit, fixing a failure introduced in
76e38b37a5.
Author: Andres Freund <andres@anarazel.de>
Reported-By: Sandeep Thakkar <sandeep.thakkar@enterprisedb.com>
Message-Id: 20220520164558.ozb7lm6unakqzezi@alap3.anarazel.de (on pgsql-packagers)
Backpatch: 15-, where 76e38b37a5 came in
As $gzip is embedded in postgresql.conf \ needs to be escaped, otherwise guc.c
will take it as a string escape. Similarly, if "$gzip" contains spaces, the
prior incantation will fail. Both of these are common on windows.
Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Discussion: https://postgr.es/m/ce1b6eb3-5736-6f38-9775-b7020128b8d8@enterprisedb.com
Backpatch: 15-, where the test was added in 027fa0fd72
A code comment said that the standard does not define a number for
ERRCODE_SQL_JSON_ITEM_CANNOT_BE_CAST_TO_TARGET_TYPE, but this was
fixed in a later draft version of the standard, so use that number
now.
The patch that added regcollation doesn't seem to have been too
thorough about supporting it everywhere that other reg* types
are supported. Fix that. (The find_expr_references omission
is moderately serious, since it could result in missing expression
dependencies. The others are less exciting.)
Noted while fixing bug #17483. Back-patch to v13 where
regcollation was added.
Discussion: https://postgr.es/m/1423433.1652722406@sss.pgh.pa.us
The motivation for this is to ensure successful transmission of the
values of constants of regconfig and other reg* types. The remote
will be reading them with search_path = 'pg_catalog', so schema
qualification is necessary when referencing objects in other schemas.
Per bug #17483 from Emmanuel Quincerot. Back-patch to all supported
versions. (There's some other stuff to do here, but it's less
back-patchable.)
Discussion: https://postgr.es/m/1423433.1652722406@sss.pgh.pa.us
Commit 4518c798 blocks signals for a short region of code, but it
assumed that whatever called it had the signal mask set to UnBlockSig on
entry. That may be true today (or may even not be, in extensions in the
wild), but it would be better not to make that assumption. We should
save-and-restore the caller's signal mask.
The PG_SETMASK() portability macro couldn't be used for that, which is
why it wasn't done before. But... considering that commit a65e0864
established back in 9.6 that supported POSIX systems have sigprocmask(),
and that this is POSIX-only code, there is no reason not to use standard
sigprocmask() directly to achieve that.
Back-patch to all supported releases, like 4518c798 and 80845b7c.
Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CA%2BhUKGKx6Biq7_UuV0kn9DW%2B8QWcpJC1qwhizdtD9tN-fn0H0g%40mail.gmail.com
Noticed while working in this area. This code was introduced in PG15,
which is still in beta, so backpatch to there for consistency.
Backpatch-through: 15
On Linux, we call posix_fallocate() on shm_open()'d memory to avoid
later potential SIGBUS (see commit 899bd785).
Based on field reports of systems stuck in an EINTR retry loop there,
there, we made it possible to break out of that loop via slightly odd
coding where the CHECK_FOR_INTERRUPTS() call was somewhat removed from
the loop (see commit 422952ee).
On further reflection, that was not a great choice for at least two
reasons:
1. If interrupts were held, the CHECK_FOR_INTERRUPTS() would do nothing
and the EINTR error would be surfaced to the user.
2. If EINTR was reported but neither QueryCancelPending nor
ProcDiePending was set, then we'd dutifully retry, but with a bit more
understanding of how posix_fallocate() works, it's now clear that you
can get into a loop that never terminates. posix_fallocate() is not a
function that can do some of the job and tell you about progress if it's
interrupted, it has to undo what it's done so far and report EINTR, and
if signals keep arriving faster than it can complete (cf recovery
conflict signals), you're stuck.
Therefore, for now, we'll simply block most signals to guarantee
progress. SIGQUIT is not blocked (see InitPostmasterChild()), because
its expected handler doesn't return, and unblockable signals like
SIGCONT are not expected to arrive at a high rate. For good measure,
we'll include the ftruncate() call in the blocked region, and add a
retry loop.
Back-patch to all supported releases.
Reported-by: Alvaro Herrera <alvherre@alvh.no-ip.org>
Reported-by: Nicola Contu <nicola.contu@gmail.com>
Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/20220701154105.jjfutmngoedgiad3%40alvherre.pgsql
Commit 054325c5ee created a memory leak in PQsendQueryInternal in case
an error occurs while sending the message. Repair.
Backpatch to 14, like that commit. Reported by Coverity.
When checking for interleaved partitions, we mark the partition as
interleaved when;
1. we find an earlier partition index when looping over the
sorted-by-Datum indexes[] array, or;
2. we find that the NULL partition allows some non-NULL Datum value.
In the code, as it was written in db632fbca we'll continue to check for
case 2 when we've already marked the partition as interleaved for case 1.
Here we make it so we don't bother marking the partition as interleaved
for case 2 when it's already been marked due to case 1.
Really all this saves is a useless call to bms_add_member(), but since
this code is new to PG15, it seems worth fixing it now to save anyone the
trouble of complaining at some time in the future. We have the
opportunity to improve this now before PG15 is out. This might ease some
future back-patching pain.
Per report and patch by Zhihong Yu. However, I slightly revised the
comments and altered the bms_add_member() code to match in both locations.
We already know that index is equal to boundinfo->null_index from the if
condition.
Author: Zhihong Yu
Discussion: https://postgr.es/m/CALNJ-vQbZR0pYxz9zQ5bqXVcwtGgNgVupeEpNT65HZ+yWZnc4g@mail.gmail.com
Backpatch-through: 15, same as db632fbca.
There are a few things that we could do a little better within
get_cheapest_group_keys_order():
1. We should be using list_free() rather than pfree() on a List.
2. We should use for_each_from() instead of manually coding a for loop to
skip the first n elements of a List
3. list_truncate(list_copy(...), n) is not a great way to copy the first n
elements of a list. Let's invent list_copy_head() for that. That way we
don't need to copy the entire list just to truncate it directly
afterwards.
4. We can simplify finding the cheapest cost by setting the cheapest cost
variable to DBL_MAX. That allows us to skip special-casing the initial
iteration of the loop.
Author: David Rowley
Discussion: https://postgr.es/m/CAApHDvrGyL3ft8waEkncG9y5HDMu5TFFJB1paoTC8zi9YK97Nw@mail.gmail.com
Backpatch-through: 15, where get_cheapest_group_keys_order was added.
Justin Pryzby reported that some scenarios could cause gathering
of extended statistics to spend many seconds in an un-cancelable
qsort() operation. To fix, invent qsort_interruptible(), which is
just like qsort_arg() except that it will also do CHECK_FOR_INTERRUPTS
every so often. This bloats the backend by a couple of kB, which
seems like a good investment. (We considered just enabling
CHECK_FOR_INTERRUPTS in the existing qsort and qsort_arg functions,
but there are some callers for which that'd demonstrably be unsafe.
Opt-in seems like a better way.)
For now, just apply qsort_interruptible() in statistics collection.
There's probably more places where it could be useful, but we can
always change other call sites as we find problems.
Back-patch to v14. Before that we didn't have extended stats on
expressions, so that the problem was less severe. Also, this patch
depends on the sort_template infrastructure introduced in v14.
Tom Lane and Justin Pryzby
Discussion: https://postgr.es/m/20220509000108.GQ28830@telsasoft.com
dshash.c previously maintained flags to be able to assert that you
didn't hold any partition lock. These flags could get out of sync with
reality in error scenarios.
Get rid of all that, and make assertions about the locks themselves
instead. Since LWLockHeldByMe() loops internally, we don't want to put
that inside another loop over all partition locks. Introduce a new
debugging-only interface LWLockAnyHeldByMe() to avoid that.
This problem was noted by Tom and Andres while reviewing changes to
support the new shared memory stats system, and later showed up in
reality while working on commit 389869af.
Back-patch to 11, where dshash.c arrived.
Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Reported-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>
Reviewed-by: Zhihong Yu <zyu@yugabyte.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/20220311012712.botrpsikaufzteyt@alap3.anarazel.de
Discussion: https://postgr.es/m/CA%2BhUKGJ31Wce6HJ7xnVTKWjFUWQZPBngxfJVx4q0E98pDr3kAw%40mail.gmail.com
This addresses two issues in the tests of test_oat_hooks:
- The role regress_test_user was being left behind, preventing the test
to succeed on repeated runs. It makes sense to leave some objects
behind to have more coverage for pg_upgrade (as does test_pg_dump), but
the role dropped here does not own any objects so there is no reason to
keep it.
- GRANT SET ON PARAMETER is issued, creating an entry in
pg_parameter_acl without cleaning up the entry created. This causes
an overlap with unsafe_tests as both use work_mem, making the latter
fail. This commit adds an extra REVOKE SET ON PARAMETER to clean the
contents of pg_parameter_acl, switching to maintenance_work_mem rather
than work_mem to avoid an overlap between both tests.
The tests of test_oat_hooks cannot use installcheck yet as these are
proving to be unstable with caching and the namespace search hooks, so
the issues fixed here cannot be reached yet, but they would be once the
hook issue is addressed and installcheck is allowed again in
test_oat_hooks.
Discussion: https://postgr.es/m/YrpVkADAY0knF6vM@paquier.xyz
Backpatch-through: 15
When you hit ^C, the terminal driver in Unix-like systems echoes "^C" as
well as sending an interrupt signal (depending on stty settings). At
least libedit (but maybe also libreadline) is then confused about the
current cursor location, and corrupts the display if you try to scroll
back. Fix, by moving to a new line before the next prompt is displayed.
Back-patch to all supported released.
Author: Pavel Stehule <pavel.stehule@gmail.com>
Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/3278793.1626198638%40sss.pgh.pa.us
Alvaro Herrera
Tom Lane
Thomas Munro
Fujii Masao
Bruce Momjian
John Naylor
Michael Paquier
Jeff Davis
Andres Freund
Peter Eisentraut
David Rowley