On Windows with MSVC, get_dirent_type() was recently made to return
DT_LNK for junction points by commit 9d3444dc, which fixed some
defective dirent.c code.
On Windows with Cygwin, get_dirent_type() already worked for symlinks,
as it does on POSIX systems, because Cygwin has its own fake symlinks
that behave like POSIX (on closer inspection, Cygwin's dirent has the
BSD d_type extension but it's probably always DT_UNKNOWN, so we fall
back to lstat(), which understands Cygwin symlinks with S_ISLNK()).
On Windows with MinGW/MSYS, we need extra code, because the MinGW
runtime has its own readdir() without d_type, and the lstat()-based
fallback has no knowledge of our convention for treating junctions as
symlinks.
Back-patch to 14, where get_dirent_type() landed.
Reported-by: Andrew Dunstan <andrew@dunslane.net>
Discussion: https://postgr.es/m/b9ddf605-6b36-f90d-7c30-7b3e95c46276%40dunslane.net
In commit 7c34555f8, I overlooked the need to configure SSPI
on Windows to allow login as the non-superuser role.
Fix that by adding auth_extra/--create-role incantation
(which, oddly enough, doesn't actually create the role).
Per buildfarm.
While here, upgrade the mechanism for temporarily setting
$ENV{PGUSER}, as per recommendation from ilmari.
Discussion: https://postgr.es/m/87edy7j1zz.fsf@wibble.ilmari.org
The catalog contents haven't changed, but it's good to make clear
that initdb is required. Changing RELMAPPER_FILEMAGIC would be more
appropriate, but that doesn't actually produce a useful diagnostic,
so cheat by doing this instead.
Discussion: http://postgr.es/m/20220727171939.6ixixqcjt5riil2o@alvherre.pgsql
After commit 089480c07, it's necessary for background worker entry
points to be marked PGDLLEXPORT, else they aren't findable by
LookupBackgroundWorkerFunction(). Since pg_prewarm lacks any
regression tests, it's not surprising its worker entry points were
overlooked. (A quick search turned up no other such oversights.)
I added some documentation pointing out the need for this, too.
Robins Tharakan and Tom Lane
CAEP4nAzndnQv3-1QKb=D-hLoK3Rko12HHMFHHtdj2GQAUXO3gw@mail.gmail.com
Commit d8cd0c6c95 introduced a file
rename that could fail on Windows, probably due to other backends
having an open file handle to the old file of the same name.
Re-arrange the locking slightly to prevent that, by making sure the
open() and close() run while we hold the lock.
Thomas Munro. I added an explanatory comment.
Discussion: https://postgr.es/m/CA%2BhUKGLZtCTgp4NTWV-wGbR2Nyag71%3DEfYTKjDKnk%2BfkhuFMHw%40mail.gmail.com
GetSubscriptionRelations() and GetSubscriptionNotReadyRelations() share
mostly the same code, which scans pg_subscription_rel and fetches all
the relations of a given subscription. The only difference is that the
second routine looks for all the relations not in a ready state. This
commit refactors the code to use a single routine, shaving a bit of
code.
Author: Vignesh C
Reviewed-By: Kyotaro Horiguchi, Amit Kapila, Michael Paquier, Peter
Smith
Discussion: https://postgr.es/m/CALDaNm0eW-9g4G_EzHebnFT5zZoasWCS_EzZQ5BgnLZny9S=pg@mail.gmail.com
This commit puts the implementation of Tuple sort variants into the separate
file tuplesortvariants.c. That gives better separation of the code and
serves well as the demonstration that Tuple sort variant can be defined outside
of tuplesort.c.
Discussion: https://postgr.es/m/CAPpHfdvjix0Ahx-H3Jp1M2R%2B_74P-zKnGGygx4OWr%3DbUQ8BNdw%40mail.gmail.com
Author: Alexander Korotkov
Reviewed-by: Pavel Borisov, Maxim Orlov, Matthias van de Meent
Reviewed-by: Andres Freund, John Naylor
The new TuplesortPublic data structure contains the definition of
sort-variant-specific interface methods and the part of Tuple sort operation
state required by their implementations. This will let define Tuple sort
variants without knowledge of Tuplesortstate, that is without knowledge
of generic sort implementation guts.
Discussion: https://postgr.es/m/CAPpHfdvjix0Ahx-H3Jp1M2R%2B_74P-zKnGGygx4OWr%3DbUQ8BNdw%40mail.gmail.com
Author: Alexander Korotkov
Reviewed-by: Pavel Borisov, Maxim Orlov, Matthias van de Meent
Reviewed-by: Andres Freund, John Naylor
This commit puts some generic work away from sort-variant-specific function.
In particular, tuplesort_put*() now doesn't need to decrease available memory
and switch to sort context before calling puttuple_common(). writetup()
doesn't need to free SortTuple.tuple and increase available memory.
Discussion: https://postgr.es/m/CAPpHfdvjix0Ahx-H3Jp1M2R%2B_74P-zKnGGygx4OWr%3DbUQ8BNdw%40mail.gmail.com
Author: Alexander Korotkov
Reviewed-by: Pavel Borisov, Maxim Orlov, Matthias van de Meent
Reviewed-by: Andres Freund, John Naylor
Abbreviation code is very similar along tuplesort_put*() functions. This
commit unifies that code and puts it into puttuple_common(). tuplesort_put*()
functions differs in the abbreviation condition, so it has been added as an
argument to the puttuple_common() function.
Discussion: https://postgr.es/m/CAPpHfdvjix0Ahx-H3Jp1M2R%2B_74P-zKnGGygx4OWr%3DbUQ8BNdw%40mail.gmail.com
Author: Alexander Korotkov
Reviewed-by: Pavel Borisov, Maxim Orlov, Matthias van de Meent
Reviewed-by: Andres Freund, John Naylor
This commit is the preparation to move abbreviation logic into
puttuple_common(). The new removeabbrev function turns datum1 representation
of SortTuple's from the abbreviated key to the first column value. Therefore,
it encapsulates the differential part of abbreviation handling code in
tuplesort_put*() functions, making these functions similar.
Discussion: https://postgr.es/m/CAPpHfdvjix0Ahx-H3Jp1M2R%2B_74P-zKnGGygx4OWr%3DbUQ8BNdw%40mail.gmail.com
Author: Alexander Korotkov
Reviewed-by: Pavel Borisov, Maxim Orlov, Matthias van de Meent
Reviewed-by: Andres Freund, John Naylor
It's currently unclear how do we split functionality between
Tuplesortstate.copytup() function and tuplesort_put*() functions.
For instance, copytup_index() and copytup_datum() return error while
tuplesort_putindextuplevalues() and tuplesort_putdatum() do their work.
This commit removes Tuplesortstate.copytup() altogether, putting the
corresponding code into tuplesort_put*().
Discussion: https://postgr.es/m/CAPpHfdvjix0Ahx-H3Jp1M2R%2B_74P-zKnGGygx4OWr%3DbUQ8BNdw%40mail.gmail.com
Author: Alexander Korotkov
Reviewed-by: Pavel Borisov, Maxim Orlov, Matthias van de Meent
Reviewed-by: Andres Freund, John Naylor
XLogRecordBlockHeader, the header holding the information for the data
related to a block, tracks the length of the data appended to the WAL
record with data_length (uint16). This limitation in size was not
enforced by the public routine in charge of registering the data
assembled later to form the WAL record inserted, XLogRegisterBufData().
Incorrectly used, it could lead to the generation of records with some
of its data overflowed. This commit adds some safeguards to prevent
that for the block data, complaining immediately if attempting to add to
a record block information with a size larger than UINT16_MAX, which is
the limit implied by the internal logic.
Note that this also adjusts XLogRegisterData() and XLogRegisterBufData()
so as the length of the WAL record data given by the caller is unsigned,
matching with what gets stored in XLogRecData->len.
Extracted from a larger patch by the same author. The original patch
includes more protections when assembling a record in full that will be
looked at separately later.
Author: Matthias van de Meent
Reviewed-by: Andres Freund, Heikki Linnakangas, Michael Paquier, David
Zhang
Discussion: https://postgr.es/m/CAEze2WgGiw+LZt+vHf8tWqB_6VxeLsMeoAuod0N=ij1q17n5pw@mail.gmail.com
As before, we start by prepending one underscore (truncating the
base name if necessary). But if there is a conflict, then instead of
prepending more and more underscores, append an underscore and some
digits, in much the same way that ChooseRelationName does. While
the previous logic could be driven to fail by creating a lot of
types with long names differing only near the end, this version seems
certain enough to eventually succeed that we can remove the failure
code path that was there before.
While at it, undo 6df7a9698's decision to split this code out of
makeArrayTypeName. That wasn't actually accomplishing anything,
because no other function was using it --- and it would have been
wrong to do so. The convention that a prefix "_" means an array,
not something else, is too ancient to mess with.
Andrey Lepikhov and Dmitry Koval, reviewed by Masahiko Sawada and myself
Discussion: https://postgr.es/m/b84cd82c-cc67-198a-8b1c-60f44e1259ad@postgrespro.ru
The BoolGetDatum() call ended up in the wrong place. It should be
applied when we, err, want to convert a bool to a datum.
Thanks to Tom Lane for noticing this.
Discussion: http://postgr.es/m/2511599.1658861964@sss.pgh.pa.us
A bootstrap user who is not a superuser will still own many
important system objects, such as the pg_catalog schema, that
will likely allow that user to regain superuser status. Therefore,
allowing the superuser property to be removed from the superuser
creates a false perception of security where none exists.
Although removing superuser from the bootstrap user is also a bad idea
and should be considered unsupported in all released versions, no
back-patch, as this is a behavior change.
Discussion: http://postgr.es/m/CA+TgmoZirCwArJms_fgvLBFrC6b=HdxmG7iAhv+kt_=NBA7tEw@mail.gmail.com
We have a few commands that "can't run in a transaction block",
meaning that if they complete their processing but then we fail
to COMMIT, we'll be left with inconsistent on-disk state.
However, the existing defenses for this are only watertight for
simple query protocol. In extended protocol, we didn't commit
until receiving a Sync message. Since the client is allowed to
issue another command instead of Sync, we're in trouble if that
command fails or is an explicit ROLLBACK. In any case, sitting
in an inconsistent state while waiting for a client message
that might not come seems pretty risky.
This case wasn't reachable via libpq before we introduced pipeline
mode, but it's always been an intended aspect of extended query
protocol, and likely there are other clients that could reach it
before.
To fix, set a flag in PreventInTransactionBlock that tells
exec_execute_message to force an immediate commit. This seems
to be the approach that does least damage to existing working
cases while still preventing the undesirable outcomes.
While here, add some documentation to protocol.sgml that explicitly
says how to use pipelining. That's latent in the existing docs if
you know what to look for, but it's better to spell it out; and it
provides a place to document this new behavior.
Per bug #17434 from Yugo Nagata. It's been wrong for ages,
so back-patch to all supported branches.
Discussion: https://postgr.es/m/17434-d9f7a064ce2a88a3@postgresql.org
Presently, archive status files are durably renamed from .ready to
.done to indicate that a file has been archived. Persisting this
rename to disk accounts for a significant amount of the overhead
associated with archiving. While durably renaming the file
prevents re-archiving in most cases, archive commands and libraries
must already gracefully handle attempts to re-archive the last
archived file after a crash (e.g., a crash immediately after
archive_command exits but before the server renames the status
file).
This change reduces the amount of overhead associated with
archiving by using rename() instead of durable_rename() to rename
the archive status files. As a consequence, the server is more
likely to attempt to re-archive files after a crash, but as noted
above, archive commands and modules are already expected to handle
this. It is also possible that the server will attempt to re-
archive files that have been removed or recycled, but the archiver
already handles this, too.
Author: Nathan Bossart
Reviewed-by: Kyotaro Horiguchi, Fujii Masao
Discussion: https://postgr.es/m/20220222011948.GA3850532@nathanxps13
Since a2c8499, HbaFileName (default pg_hba.conf) was getting used
instead of IdentFileName (default pg_ident.conf) as the parent file to
use as reference when parsing the contents of pg_ident.conf, with
pg_ident.conf correctly opened, when feeding this information to
pg_ident_file_mappings. This had two consequences:
- On an I/O error when reading pg_ident.conf, the user would get an
ERROR message referring to pg_hba.conf and not pg_ident.conf.
- When reading an external file with a relative path using '@' in
pg_ident.conf, the directory used to look at the file to load would be
the base directory of pg_hba.conf rather than the one of pg_ident.conf,
leading to errors in pg_ident_file_mappings inconsistent with what gets
loaded at startup when pg_ident.conf and pg_hba.conf are located in
different directories.
This error only impacted the SQL view pg_ident_file_mappings that uses a
logic new to v15 to fill the view with the parsed information, not the
code paths loading these authentication files at startup.
Author: Julien Rouhaud
Discussion: https://postgr.es/m/20220726050402.vsr6fmz7rsgpmdz3@jrouhaud
Backpatch-through: 15
This addresses a couple of bugs in the REINDEX grammar, introduced by
83011ce:
- A name was never specified for DATABASE/SYSTEM, even if the query
included one. This caused such REINDEX queries to always work with any
object name, but we should complain if the object name specified does
not match the name of the database we are connected to. A test is added
for this case in the main regression test suite, provided by Álvaro.
- REINDEX SYSTEM CONCURRENTLY [name] was getting rejected in the
parser. Concurrent rebuilds are not supported for catalogs but the
error provided at execution time is more helpful for the user, and
allowing this flavor results in a simplification of the parsing logic.
- REINDEX DATABASE CONCURRENTLY was rebuilding the index in a
non-concurrent way, as the option was not being appended correctly in
the list of DefElems in ReindexStmt (REINDEX (CONCURRENTLY) DATABASE was
working fine. A test is added in the TAP tests of reindexdb for this
case, where we already have a REINDEX DATABASE CONCURRENTLY query
running on a small-ish instance. This relies on the work done in
2cbc3c1 for SYSTEM, but here we check if the OIDs of the index relations
match or not after the concurrent rebuild. Note that in order to get
this part to work, I had to tweak the tests so as the index OID and
names are saved separately. This change not affect the reliability or
of the coverage of the existing tests.
While on it, I have implemented a tweak in the grammar to reduce the
parsing by one branch, simplifying things even more.
Author: Michael Paquier, Álvaro Herrera
Discussion: https://postgr.es/m/YttqI6O64wDxGn0K@paquier.xyz
We weren't exercising the session_preload_libraries option in any
meaningful way. auto_explain is a good testbed for doing so, since
it's one of the primary use-cases for session_preload_libraries.
Hence, adjust its TAP test to load the library via
session_preload_libraries not shared_preload_libraries. While at it,
feed test-specific settings to the backend via PGOPTIONS rather than
tediously rewriting postgresql.conf.
Also, since auto_explain has some PGC_SUSET parameters, we can use it
to provide a test case for the permissions-checking bug just fixed
by commit b35617de3.
Back-patch to v15 so that we have coverage for the permissions issue
in that branch too. To do that, I back-patched the refactoring
recently done by commit 550bc0a6c.
Dagfinn Ilmari Mannsåker and Tom Lane
Discussion: https://postgr.es/m/CABwTF4VEpwTHhRQ+q5MiC5ucngN-whN-PdcKeufX7eLSoAfbZA@mail.gmail.com
The setting controls tha maximum length of the header line in expanded
format output. Possible settings are full, column, page, or an integer.
the default is full, the current behaviour, and in this case the header
line is the length of the widest line of output. column causes the
header to be truncated to the width of the first column, page causes it
to be truncated to the width of the terminal page, and an integer causes
it to be truncated to that value. If the full value is less than the
page or integer value no truncation occurs. If given without an argument
this option prints its current setting.
Platon Pronko, somewhat modified by me.
Discussion: https://postgr.es/m/f03d38a3-db96-a56e-d1bc-dbbc80bbde4d@gmail.com
Previously we did this after InitPostgres, at a somewhat randomly chosen
place within PostgresMain. However, since commit a0ffa885e doing this
outside a transaction can cause a crash, if we need to check permissions
while replacing a placeholder GUC. (Besides which, a preloaded library
could itself want to do database access within _PG_init.)
To avoid needing an additional transaction start/end in every session,
move the process_session_preload_libraries call to within InitPostgres's
transaction. That requires teaching the code not to call it when
InitPostgres is called from somewhere other than PostgresMain, since
we don't want session_preload_libraries to affect background workers.
The most future-proof solution here seems to be to add an additional
flag parameter to InitPostgres; fortunately, we're not yet very worried
about API stability for v15.
Doing this also exposed the fact that we're currently honoring
session_preload_libraries in walsenders, even those not connected to
any database. This seems, at minimum, a POLA violation: walsenders
are not interactive sessions. Let's stop doing that.
(All these comments also apply to local_preload_libraries, of course.)
Per report from Gurjeet Singh (thanks also to Nathan Bossart and Kyotaro
Horiguchi for review). Backpatch to v15 where a0ffa885e came in.
Discussion: https://postgr.es/m/CABwTF4VEpwTHhRQ+q5MiC5ucngN-whN-PdcKeufX7eLSoAfbZA@mail.gmail.com
It incorrectly used GetBufferDescriptor instead of
GetLocalBufferDescriptor, causing it to not find the correct buffer in
most cases, and performing an out-of-bounds memory read in the corner
case that temp_buffers > shared_buffers.
It also bumped the usage-count on the buffer, even if it was
previously pinned. That won't lead to crashes or incorrect results,
but it's different from what the shared-buffer case does, and
different from the usual code in LocalBufferAlloc. Fix that too, and
make the code ordering match LocalBufferAlloc() more closely, so that
it's easier to verify that it's doing the same thing.
Currently, ReadRecentBuffer() is only used with non-temp relations, in
WAL redo, so the broken code is currently dead code. However, it could
be used by extensions.
Backpatch-through: 14
Discussion: https://www.postgresql.org/message-id/2d74b46f-27c9-fb31-7f99-327a87184cc0%40iki.fi
Reviewed-by: Thomas Munro, Zhang Mingli, Richard Guo
This commit removes two arguments "report" and "whichChkpt"
in ReadCheckpointRecord().
"report" is obviously useless because it's always true, i.e., there are
two callers of the function and they always specify true as "report".
Commit 1d919de5eb removed the only call with "report" = false.
"whichChkpt" indicated where the specified checkpoint location
came from, pg_control or backup_label. This information was used
to report different error messages depending on where the invalid
checkpoint record came from, when it was found.
But ReadCheckpointRecord() doesn't need to do that because
its callers already do that and users can still identify where
the invalid checkpoint record came from, by reading such log messages.
Also when "whichChkpt" was 0, the word "primary checkpoint" was used
in the log message and could confuse users because the concept of
primary and secondary checkpoints was already removed before.
These are why this commit removes "whichChkpt" argument.
Author: Fujii Masao
Reviewed-by: Bharath Rupireddy, Kyotaro Horiguchi
Discussion: https://postgr.es/m/fa2e12eb-81c3-0717-0272-755f8a81c8f2@oss.nttdata.com
We didn't explicitly say that random() uses a randomly-chosen seed
if you haven't called setseed(). Do so.
Also, remove ref/set.sgml's no-longer-accurate (and never very
relevant) statement that the seed value is multiplied by 2^31-1.
Back-patch to v12 where set.sgml's claim stopped being true.
The claim that we use a source of random bits as seed was debatable
before 4203842a1, too, so v12 seems like a good place to stop.
Per question from Carl Sopchak.
Discussion: https://postgr.es/m/f37bb937-9d99-08f0-4de7-80c91a3cfc2e@sopchak.me
sigwait() is in SUSv2 and all targeted Unix systems have it. An earlier
pre-standard function prototype existed on some older systems, but we
no longer need a workaround for that.
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Greg Stark <stark@mit.edu>
Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Discussion: https://postgr.es/m/CA+hUKGJ3LHeP9w5Fgzdr4G8AnEtJ=z=p6hGDEm4qYGEUX5B6fQ@mail.gmail.com
getrusage() is in SUSv2 and all targeted Unix systems have it.
Note that POSIX only covers ru_utime and ru_stime and we rely on many
more fields without any kind of configure probe, but that predates this
commit.
The only supported system we need replacement code for now is Windows,
and that can be done without a configure probe.
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Greg Stark <stark@mit.edu>
Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Discussion: https://postgr.es/m/CA+hUKGJ3LHeP9w5Fgzdr4G8AnEtJ=z=p6hGDEm4qYGEUX5B6fQ@mail.gmail.com
Commit e2f65f425 added contrib/pg_prewarm to the prerequisites for
running the src/test/recovery suite, but did not bother to update
the documentation about that.
We've long held the minimum at 3.80, but that's required more than
one workaround. Commit 0f39b70a6 broke it again, because it turns
out that exporting a target-specific variable didn't work in 3.80.
Considering that 3.81 is now old enough to get a driver's license,
and that the only remaining buildfarm member testing 3.80 (prairiedog)
is likely to be retired soon, let's just stop supporting 3.80.
Adjust docs and Makefile.global's minimum-version check to match.
There are a couple of comments in the Makefiles suggesting that
random things could be done differently after we desupport 3.80,
but I couldn't get excited about changing any of them right now.
Back-patch to v15, as 0f39b70a6 was.
Discussion: https://postgr.es/m/20220720172321.GL12702@telsasoft.com
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
Things like "opt_name" can well be shared by various commands rather
than there being multiple definitions of the same thing. Rename these
productions and move them to appear together in gram.y, which may
improve chances of reuse in the future.
Discussion: https://postgr.es/m/20220721174212.cmitjpuimx6ssyyj@alvherre.pgsql
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
O_FSYNC was a pre-POSIX way of spelling O_SYNC, supported since commit
9d645fd84c for non-conforming operating systems of the time. It's not
needed on any modern system. We can just use standard O_SYNC directly
if it exists (= all targeted systems except Windows), and get rid of our
OPEN_SYNC_FLAG macro.
Similarly for standard O_DSYNC, we can just use that directly if it
exists (= all targeted systems except DragonFlyBSD), and get rid of our
OPEN_DATASYNC_FLAG macro.
We still avoid choosing open_datasync as a default value for
wal_sync_method if O_DSYNC has the same value as O_SYNC (= only
OpenBSD), so there is no change in default behavior.
Discussion: https://postgr.es/m/CA%2BhUKGJE7y92NY7FG2ftUbZUaqohBU65_Ys_7xF5mUHo4wirTQ%40mail.gmail.com
Commit 4f658dc8 provided the traditional BSD fls() function in
src/port/fls.c so it could be used in several places. Later we added a
bunch of similar facilities in pg_bitutils.h, based on compiler
builtins that map to hardware instructions. It's a bit confusing to
have both 1-based and 0-based variants of this operation in use in
different parts of the tree, and neither is blessed by a standard.
Let's drop fls.c and the configure probe, and reuse the newer code.
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CA%2BhUKG%2B7dSX1XF8yFGmYk-%3D48dbjH2kmzZj16XvhbrWP-9BzRg%40mail.gmail.com
Thomas Munro