Since the initial goal of pg_read_all_data was to be able to run
pg_dump as a non-superuser without explicitly granting access to
every object, it follows that it should allow reading all large
objects. For consistency, pg_write_all_data should allow writing
all large objects, too.
Author: Nitin Motiani <nitinmotiani@google.com>
Co-authored-by: Nathan Bossart <nathandbossart@gmail.com>
Reviewed-by: Dilip Kumar <dilipbalaut@gmail.com>
Discussion: https://postgr.es/m/CAH5HC96dxAEvP78s1-JK_nDABH5c4w2MDfyx4vEWxBEfofGWsw%40mail.gmail.com
lgamma(NaN) should produce NaN, but on older versions of AIX
it reports an ERANGE error. While that's been fixed in the latest
version of libm, it'll take awhile for the fix to propagate. This
workaround is harmless even when the underlying bug does get fixed.
Discussion: https://postgr.es/m/3603369.1771877682@sss.pgh.pa.us
There were a couple of comments in parse_relation.c
> Note: properly, lockmode should be declared LOCKMODE not int, but that
> would require importing storage/lock.h into parse_relation.h. Since
> LOCKMODE is typedef'd as int anyway, that seems like overkill.
but actually LOCKMODE has been in storage/lockdefs.h for a while,
which is intentionally a more narrow header. So we can include that
one in parse_relation.h and just use LOCKMODE normally.
An alternative would be to add a duplicate typedef into
parse_relation.h, but that doesn't seem necessary here.
Reviewed-by: Andreas Karlsson <andreas@proxel.se>
Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/4bcd65fb-2497-484c-bb41-83cb435eb64d%40eisentraut.org
Send PG_PROTOCOL_GREASE and _pq_.test_protocol_negotiation, which were
introduced in commit d8d7c5dc8, by default, and fail the connection if
the server attempts to claim support for them. The hope is to provide
feedback to noncompliant implementations and gain confidence in our
ability to advance the protocol. (See the other commit for details.)
To help end users navigate the situation, a link to our documentation
that explains the behavior is displayed. We append this to the error
message when the NegotiateProtocolVersion response is incorrect, or when
the peer sends an error during startup that appears to be grease-
related.
It's still possible for users to connect to servers that don't support
protocol negotiation, by adding max_protocol_version=3.0 to their
connection strings. Only the default connection behavior is impacted.
This commit is tracked as a PG19 open item and will be reverted before
RC1. (The implementation here doesn't handle negotiation with later
server versions, so it can't be released into the wild as a
five-year-supported feature. But an improved implementation might be
able to do so, in the future...)
Author: Jelte Fennema-Nio <postgres@jeltef.nl>
Co-authored-by: Jacob Champion <jacob.champion@enterprisedb.com>
Discussion: https://postgr.es/m/DDPR5BPWH1RJ.1LWAK6QAURVAY%40jeltef.nl
The concerns that led us to remove AIX support in commit 0b16bb877
have now been alleviated:
1. IBM has stepped forward to provide support, including buildfarm
animal(s).
2. AIX 7.2 and later seem to be fine with large pg_attribute_aligned
requirements. Since 7.1 is now EOL anyway, we can just cease to
support it.
3. Tossing xlc support overboard seems okay as well. It's a bit
sad to drop one of the few remaining non-gcc-alike compilers, but
working around xlc's bugs and idiosyncrasies doesn't seem justified
by the theoretical portability benefits.
4. Likewise, we can stop supporting 32-bit AIX builds. This is
not so much about whether we could build such executables as that
they're too much of a pain to manage in the field, due to limited
address space available for dynamic library loading.
5. We hit on a way to manage catalog column alignment that doesn't
require continuing developer effort (see commit ecae09725).
Hence, this commit reverts 0b16bb877 and some follow-on commits
such as e6bb491bf, except for not putting back XLC support nor
the changes related to catalog column alignment.
Some other notable changes from the way things were in v16:
Prefer unnamed POSIX semaphores on AIX, rather than the default
choice of SysV semaphores.
Include /opt/freeware/lib in -Wl,-blibpath, even when it is not
mentioned anywhere in LDFLAGS.
Remove platform-specific adjustment of MEMSET_LOOP_LIMIT; maybe
that's still the right thing, but it really ought to be re-tested.
Silence compiler warnings related to getpeereid(), wcstombs_l(),
and PAM conversation procs.
Accept "libpythonXXX.a" as an okay name for the Python shared
library (but only on AIX!).
Author: Aditya Kamath <Aditya.Kamath1@ibm.com>
Author: Srirama Kucherlapati <sriram.rk@in.ibm.com>
Co-authored-by: Peter Eisentraut <peter@eisentraut.org>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CY5PR11MB63928CC05906F27FB10D74D0FD322@CY5PR11MB6392.namprd11.prod.outlook.com
Because we assume that int64 and double have the same alignment
requirement, AIX's default behavior that alignof(double) = 4 while
alignof(int64) = 8 is a headache. There are two issues:
1. We align both int8 and float8 tuple columns per ALIGNOF_DOUBLE,
which is an ancient choice that can't be undone without breaking
pg_upgrade and creating some subtle SQL-level compatibility issues
too. However, the cost of that is just some marginal inefficiency
in fetching int8 values, which can't be too awful if the platform
architects were willing to pay the same costs for fetching float8s.
So our decision is to leave that alone. This patch makes our
alignment choices the same as they were pre-v17, namely that
ALIGNOF_DOUBLE and ALIGNOF_INT64_T are whatever the compiler prefers
and then MAXIMUM_ALIGNOF is the larger of the two. (On all supported
platforms other than AIX, all three values will be the same.)
2. We need to overlay C structs onto catalog tuples, and int8 fields
in those struct declarations may not be aligned to match this rule.
In the old branches we had some annoying rules about ordering catalog
columns to avoid alignment problems, but nobody wants to resurrect
those. However, there's a better answer: make the compiler construe
those struct declarations the way we need it to by using the pack(N)
pragma. This requires no manual effort to maintain going forward;
we only have to insert the pragma into all the catalog *.h files.
(As the catalogs stand at this writing, nothing actually changes
because we've not moved any affected columns since v16; hence no
catversion bump is required. The point of this is to not have
to worry about the issue going forward.)
We did not have this option when the AIX port was first made. This
patch depends on the C99 feature _Pragma(), as well as the pack(N)
pragma which dates to somewhere around gcc 4.0, and probably doesn't
exist in xlc at all. But now that we've agreed to toss xlc support
out the window, there doesn't seem to be a reason not to go this way.
In passing, I got rid of LONGALIGN[_DOWN] along with the configure
probes for ALIGNOF_LONG. We were not using those anywhere and it
seems highly unlikely that we'd do so in future. Instead supply
INT64ALIGN[_DOWN], which isn't used either but at least could
have a good reason to be used.
Discussion: https://postgr.es/m/1127261.1769649624@sss.pgh.pa.us
This uses the "connection warning" infrastructure introduced by
commit 1d92e0c2cc to emit a WARNING when an MD5 password is used to
authenticate. MD5 password support was marked as deprecated in
v18 and will be removed in a future release of Postgres. These
warnings are on by default but can be turned off via the existing
md5_password_warnings parameter.
Reviewed-by: Andreas Karlsson <andreas@proxel.se>
Reviewed-by: Xiangyu Liang <liangxiangyu_2013@163.com>
Discussion: https://postgr.es/m/aYzeAYEbodkkg5e-%40nathan
There are three static definitions of validate_relation_kind() in the
codebase, one each in table.c, indexam.c and sequence.c, validating that
the given relation is a table, an index or a sequence respectively.
The compiler knows which definition to use where because they are static.
But this could be confusing to a reader. Rename these functions so that
their names reflect the kind of relation they are validating. While at
it, also update the comments in table.c to clarify the definition of
table-like relkinds so that we don't have to maintain the exclusion list
as the set of relkinds undergoes changes.
Author: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Reviewed-by: Junwang Zhao <zhjwpku@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/6d3fef19-a420-4e11-8235-8ea534bf2080%40eisentraut.org
It instead of checking which relkinds it shouldn't be, explicitly list
the ones we accept. This is used to check which relkinds are accepted
in table_open() and related functions. Before this change, figuring
that out was always a few steps too complicated. This also makes
changes for new relkinds more explicit instead of accidental.
Finally, this makes this more aligned with the functions of the same
name in src/backend/access/index/indexam.c and
src/backend/access/sequence/sequence.c.
Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Reviewed-by: Junwang Zhao <zhjwpku@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/6d3fef19-a420-4e11-8235-8ea534bf2080%40eisentraut.org
Previously, these characters could cause problems when passed through
shell commands, and were flagged with a comment in string_utils.c
suggesting they be rejected in a future major release.
The affected commands are CREATE DATABASE, CREATE ROLE, CREATE TABLESPACE,
ALTER DATABASE RENAME, ALTER ROLE RENAME, and ALTER TABLESPACE RENAME.
Also add a pg_upgrade check to detect these invalid names in clusters
being upgraded from pre-v19 versions, producing a report file listing
any offending objects that must be renamed before upgrading.
Tests have been modified accordingly.
Author: Mahendra Singh Thalor <mahi6run@gmail.com>
Reviewed-By: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-By: Andrew Dunstan <andrew@dunslane.net>
Reviewed-By: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-By: Nathan Bossart <nathandbossart@gmail.com>
Reviewed-By: Srinath Reddy <srinath2133@gmail.com>
Discussion: https://postgr.es/m/CAKYtNApkOi4FY0S7+3jpTqnHVyyZ6Tbzhtbah-NBbY-mGsiKAQ@mail.gmail.com
We now support the common meson option -Ddefault_library, with values
'both' (the default), 'shared' (install only shared libraries), and
'static' (install only static libraries). The 'static' choice doesn't
actually work, since psql and other programs insist on linking to the
shared version of libpq, but it's there pro-forma. It could be built
out if we really wanted, but since we have never supported the
equivalent in the autoconf build system, there doesn't appear to be an
urgent need.
With an eye to re-supporting AIX, the internal implementation
distinguishes whether to install libpgport.a and other static-only
libraries from whether to build/install the static variant of
libraries that we can build both ways. This detail isn't exposed as a
meson option, though it could be if there's demand.
The Cirrus CI task SanityCheck now uses -Ddefault_library=shared to
save a little bit of build time (and to test this option).
Author: Peter Eisentraut <peter@eisentraut.org>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/e8aa97db-872b-4087-b073-f296baae948d@eisentraut.org
This replaces some loops over word-length popcount functions with
calls to pg_popcount(). Since pg_popcount() may use a function
pointer for inputs with sizes >= a Bitmapset word, this produces a
small regression for the common one-word case in bms_num_members().
To deal with that, this commit adds an inlined fast-path for that
case. This fast-path could arguably go in pg_popcount() itself
(with an appropriate alignment check), but that is left for future
study.
Suggested-by: John Naylor <johncnaylorls@gmail.com>
Reviewed-by: John Naylor <johncnaylorls@gmail.com>
Discussion: https://postgr.es/m/CANWCAZY7R%2Biy%2Br9YM_sySNydHzNqUirx1xk0tB3ej5HO62GdgQ%40mail.gmail.com
This commit replaces the implementations of pg_popcount{32,64} with
branchless ones in plain C. While these new implementations do not
make use of more sophisticated population count instructions
available on some CPUs, testing indicates they perform well,
especially now that they are inlined. Newer versions of popular
compilers will automatically replace these with special
instructions if possible, anyway. A follow-up commit will replace
various loops over these functions with calls to pg_popcount(),
leaving us little reason to worry about micro-optimizing them
further.
Since this commit removes the only uses of the popcount builtins,
we can also remove the corresponding configuration checks.
Suggested-by: John Naylor <johncnaylorls@gmail.com>
Reviewed-by: John Naylor <johncnaylorls@gmail.com>
Discussion: https://postgr.es/m/CANWCAZY7R%2Biy%2Br9YM_sySNydHzNqUirx1xk0tB3ej5HO62GdgQ%40mail.gmail.com
Future commits will consolidate the CPU feature detection functionality
now scattered around in various files, and the CRC "*_choose.c"
files seem to be the natural place for it. For now, just rename in
a separate commit to make it easier to follow the git log. Do the
minimum necessary to keep the build systems functional, and build the
new file pg_cpu_x86.c unconditionally using guards to control the
visibility of its contents, following the model of some more recent
files in src/port.
Limit scope to x86 to reduce the number of moving parts, since the
motivation for doing this now is to clear out some technical debt
before adding AVX2 detection. Arm is left for future work.
Reviewed-by: Zsolt Parragi <zsolt.parragi@percona.com>
Discussion: https://postgr.es/m/CANWCAZbgEUFw7LuYSVeJ=Tj98R5HoOB1Ffeqk3aLvbw5rU5NTw@mail.gmail.com
conflict.h currently includes utils/timestamp.h despite only requiring
basic timestamp type definitions. This creates unnecessary overhead.
Replace the include with datatype/timestamp.h to provide the necessary
types. This change requires explicitly including utils/timestamp.h in
test_custom_fixed_stats.c, which previously relied on the indirect
inclusion.
Extracted from the larger patch by Andres Freund.
Discussion: https://postgr.es/m/aY-UE-4t7FiYgH3t@alap3.anarazel.de
On common architectures, the PGPROC struct happened to be a multiple
of 64 bytes on PG 18, but it's changed on 'master' since. There was
worry that changing the alignment might hurt performance, due to false
cacheline sharing across elements in the proc array. However, there
was no explicit alignment, so any alignment to cache lines was
accidental. Add explicit alignment to remove worry about false
sharing.
Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion: https://www.postgresql.org/message-id/3dd6f70c-b94d-4428-8e75-74a7136396be@iki.fi
The ordering was pretty random, making it hard to get an overview of
what's in it. Group related fields together, and add comments to act
as separators between the groups.
Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion: https://www.postgresql.org/message-id/3dd6f70c-b94d-4428-8e75-74a7136396be@iki.fi
If a CREATE TABLE statement defined a constraint whose name is identical
to the name generated for a NOT NULL constraint, we'd throw an
(unnecessary) unique key violation error on
pg_constraint_conrelid_contypid_conname_index: this can easily be
avoided by choosing a different name for the NOT NULL constraint.
Fix by passing the constraint names already created by
AddRelationNewConstraints() to AddRelationNotNullConstraints(), so that
the latter can avoid name collisions with them.
Bug: #19393
Author: Laurenz Albe <laurenz.albe@cybertec.at>
Reported-by: Hüseyin Demir <huseyin.d3r@gmail.com>
Backpatch-through: 18
Discussion: https://postgr.es/m/19393-6a82427485a744cf@postgresql.org
The field was mainly used for the position in a LOCK's wait queue, but
also as the position in a the freelist when the PGPROC entry was not
in use. The reuse saves some memory at the expense of readability,
which seems like a bad tradeoff. If we wanted to make the struct
smaller there's other things we could do, but we're actually just
discussing adding padding to the struct for performance reasons.
Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion: https://www.postgresql.org/message-id/3dd6f70c-b94d-4428-8e75-74a7136396be@iki.fi
Following the example set by commit 58a359e585, we can squeeze out
a little more performance from COPY FROM (FORMAT {text,csv}) by
inlining CopyReadLineText() and passing the is_csv parameter as a
constant. This allows the compiler to emit specialized code with
fewer branches.
This is preparatory work for a proposed follow-up commit that would
further optimize this code with SIMD instructions.
Author: Nazir Bilal Yavuz <byavuz81@gmail.com>
Reviewed-by: Ayoub Kazar <ma_kazar@esi.dz>
Tested-by: Manni Wood <manni.wood@enterprisedb.com>
Discussion: https://postgr.es/m/CAOzEurSW8cNr6TPKsjrstnPfhf4QyQqB4tnPXGGe8N4e_v7Jig%40mail.gmail.com
When adjust_appendrel_attrs translates a Var referencing a parent
relation into a Var referencing a child relation, it propagates
varnullingrels from the parent Var to the translated Var. Previously,
the code simply overwrote the translated Var's varnullingrels with
those of the parent.
This was incorrect because the translated Var might already possess
nonempty varnullingrels. This happens, for example, when a LATERAL
subquery within a UNION ALL references a Var from the nullable side of
an outer join. In such cases, the translated Var correctly carries
the outer join's relid in its varnullingrels. Overwriting these bits
with the parent Var's set caused the planner to lose track of the fact
that the Var could be nulled by that outer join.
In the reported case, because the underlying column had a NOT NULL
constraint, the planner incorrectly deduced that the Var could never
be NULL and discarded essential IS NOT NULL filters. This led to
incorrect query results where NULL rows were returned instead of being
filtered out.
To fix, use bms_add_members to merge the parent Var's varnullingrels
into the translated Var's existing set, preserving both sources of
nullability.
Back-patch to v16. Although the reported case does not seem to cause
problems in v16, leaving incorrect varnullingrels in the tree seems
like a trap for the unwary.
Bug: #19412
Reported-by: Sergey Shinderuk <s.shinderuk@postgrespro.ru>
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/19412-1d0318089b86859e@postgresql.org
Backpatch-through: 16
pgstat.h is a widely included header. Including worker_internal.h there is
unnecessary and creates tight coupling. By refactoring
pgstat_report_subscription_error() to fetch the required
LogicalRepWorkerType internally rather than receiving it as an argument,
we can eliminate the need for the internal header.
Reported-by: Andres Freund <andres@anarazel.de>
Author: Nisha Moond <nisha.moond412@gmail.com>
Reviewed-by: vignesh C <vignesh21@gmail.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Discussion: https://postgr.es/m/aY-UE-4t7FiYgH3t@alap3.anarazel.de
S_LOCK_FREE() is used by the test program in s_lock.c, but nobody
has voiced concerns about losing some coverage there.
SpinLockFree() appears to have been unused since it was introduced
by commit 499abb0c0f. There was agreement to remove these in 2020,
but it never happened. Since we still have agreement for removal
in 2026, let's do that now.
Reviewed-by: Fabrízio de Royes Mello <fabriziomello@gmail.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/aZX2oUcKf7IzHnnK%40nathan
Discussion: https://postgr.es/m/20200608225338.m5zho424w6lpwb2d%40alap3.anarazel.de
This has been a keyword since C99, and we now require C11, so we no
longer need to use __inline__ or to check for it at configure time.
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/aZdGbDaV4_yKCMc-%40nathan
Commit e222534679 adjusted the logic in
add_path() to keep the path list sorted by disabled_nodes and then by
total_cost, but failed to make the corresponding adjustment to
add_partial_path. As a result, add_partial_path might sort the path list
just by total cost, which could lead to later planner misbehavior.
In principle, this should be back-patched to v18, but we are typically
reluctant to back-patch planner fixes for fear of destabilizing working
installations, and it is unclear to me that this has sufficiently
serious consequences to justify an exception, so for now, no back-patch.
Reviewed-by: Richard Guo <guofenglinux@gmail.com>
Discussion: http://postgr.es/m/CAMbWs4-mO3jMK4t_LgcJ+7Eo=NmGgkxettgRaVbJzZvVZ1koMA@mail.gmail.com
The source version of pg_hba.conf.sample contains
@remove-line-for-nolocal@ markers that indicate which lines should
be deleted for an installation that doesn't HAVE_UNIX_SOCKETS.
We no longer support that case, and since commit f55808828 all
that initdb is doing is unconditionally removing the markers.
We might as well remove the markers from the source version and
drop the removal code, which is unintelligible now anyway.
This will not of course save any noticeable number of cycles
in initdb, but it might save some confusion for future
developers looking at pg_hba.conf.sample. It also reduces the
number of distinct cases that replace_token() has to support,
possibly allowing some tightening of that function.
Discussion: https://postgr.es/m/2287786.1771458157@sss.pgh.pa.us
This commit allows setting wal_receiver_timeout per subscription
using the CREATE SUBSCRIPTION and ALTER SUBSCRIPTION commands.
The value is stored in the subwalrcvtimeout column of the pg_subscription
catalog.
When set, this value overrides the global wal_receiver_timeout for
the subscription's apply worker. The default is -1, which means the
global setting (from the server configuration, command line, role,
or database) remains in effect.
This feature is useful for configuring different timeout values for
each subscription, especially when connecting to multiple publisher
servers, to improve failure detection.
Bump catalog version.
Author: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Japin Li <japinli@hotmail.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Discussion: https://postgr.es/m/a1414b64-bf58-43a6-8494-9704975a41e9@oss.nttdata.com
When multiple subscribers connect to different publisher servers,
it can be useful to set different wal_receiver_timeout values for
each connection to better detect failures. However, previously
this wasn't possible, which limited flexibility in managing subscriptions.
This commit changes wal_receiver_timeout to be user-settable,
allowing different values to be assigned using ALTER ROLE SET for
each subscription owner. This effectively enables per-subscription
configuration.
Author: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Japin Li <japinli@hotmail.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Discussion: https://postgr.es/m/a1414b64-bf58-43a6-8494-9704975a41e9@oss.nttdata.com
Checkpoint completion log messages include more detail than checkpoint
start messages, but previously omitted the checkpoint request flags,
which were only logged at checkpoint start. As a result, users had to
correlate completion messages with earlier start messages to see
the full context.
This commit includes the checkpoint request flags in the checkpoint
completion log message as well. This duplicates some information,
but makes the completion message self-contained and easier to interpret.
Author: Soumya S Murali <soumyamurali.work@gmail.com>
Reviewed-by: Michael Banck <mbanck@gmx.net>
Reviewed-by: Yuan Li <carol.li2025@outlook.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/CAMtXxw9tPwV=NBv5S9GZXMSKPeKv5f9hRhSjZ8__oLsoS5jcuA@mail.gmail.com
Instead of using comments to mark fallthrough switch cases, use the
fallthrough attribute. This will (in the future, not here) allow
supporting other compilers besides gcc. The commenting convention is
only supported by gcc, the attribute is supported by clang, and in the
fullness of time the C23 standard attribute would allow supporting
other compilers as well.
Right now, we package the attribute into a macro called
pg_fallthrough. This commit defines that macro and replaces the
existing comments with that macro invocation.
We also raise the level of the gcc -Wimplicit-fallthrough= option from
3 to 5 to enforce the use of the attribute.
Reviewed-by: Jelte Fennema-Nio <postgres@jeltef.nl>
Discussion: https://www.postgresql.org/message-id/flat/76a8efcd-925a-4eaf-bdd1-d972cd1a32ff%40eisentraut.org
A fallthrough attribute after the last case is a constraint violation
in C23, and clang warns about it (not about this comment, but if we
changed it to an attribute). Remove it. (There was apparently never
anything after this to fall through to, even in the first commit
da07a1e8565.)
Reviewed-by: Jelte Fennema-Nio <postgres@jeltef.nl>
Discussion: https://www.postgresql.org/message-id/flat/76a8efcd-925a-4eaf-bdd1-d972cd1a32ff%40eisentraut.org
As transam's README documents, the general order of actions recommended
when WAL-logging a buffer is to unlock and unpin buffers after leaving a
critical section. This pattern was not being followed by some code
paths of GIN and GiST, adjusted in this commit, where buffers were
either unlocked or unpinned inside a critical section. Based on my
analysis of each code path updated here, there is no reason to not
follow the recommended unlocking/unpin pattern done outside of a
critical section.
These inconsistencies are rather old, coming mainly from ecaa4708e5
and ff301d6e69. The guidelines in the README predate these commits,
being introduced in 6d61cdec07.
Author: Kirill Reshke <reshkekirill@gmail.com>
Discussion: https://postgr.es/m/CALdSSPgBPnpNNzxv0Y+_GNFzW6PmzRZYh+_hpf06Y1N2zLhZaQ@mail.gmail.com
Up to now, to create such a function, one had to make a pg_proc.dat
entry and then overwrite it with a CREATE OR REPLACE command in
system_functions.sql. That's error-prone (cf. bug #19409) and
results in leaving dead rows in the initial contents of pg_proc.
Manual maintenance of pg_node_tree strings seems entirely impractical,
and parsing expressions during bootstrap would be extremely difficult
as well. But Andres Freund observed that all the current use-cases
are simple constants, and building a Const node is well within the
capabilities of bootstrap mode. So this patch invents a special case:
if bootstrap mode is asked to ingest a non-null value for
pg_proc.proargdefaults (which would otherwise fail in
pg_node_tree_in), it parses the value as an array literal and then
feeds the element strings to the input functions for the corresponding
parameter types. Then we can build a suitable pg_node_tree string
with just a few more lines of code.
This allows removing all the system_functions.sql entries that are
just there to set up default arguments, replacing them with
proargdefaults fields in pg_proc.dat entries. The old technique
remains available in case someone needs a non-constant default.
The initial contents of pg_proc are demonstrably the same after
this patch, except that (1) json_strip_nulls and jsonb_strip_nulls
now have the correct provolatile setting, as per bug #19409;
(2) pg_terminate_backend, make_interval, and drandom_normal
now have defaults that don't include a type coercion, which is
how they should have been all along.
In passing, remove some unused entries from bootstrap.c's TypInfo[]
array. I had to add some new ones because we'll now need an entry for
each default-possessing system function parameter, but we shouldn't
carry more than we need there; it's just a maintenance gotcha.
Bug: #19409
Reported-by: Lucio Chiessi <lucio.chiessi@trustly.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Author: Andrew Dunstan <andrew@dunslane.net>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/183292bb-4891-4c96-a3ca-e78b5e0e1358@dunslane.net
Discussion: https://postgr.es/m/19409-e16cd2605e59a4af@postgresql.org
The previous default bgworker_die() signal would exit with elog(FATAL)
directly from the signal handler. That could cause deadlocks or
crashes if the signal handler runs while we're e.g holding a spinlock
or in the middle of a memory allocation.
All the built-in background workers overrode that to use the normal
die() handler and CHECK_FOR_INTERRUPTS(). Let's make that the default
for all background workers. Some extensions relying on the old
behavior might need to adapt, but the new default is much safer and is
the right thing to do for most background workers.
Reviewed-by: Nathan Bossart <nathandbossart@gmail.com>
Reviewed-by: Kirill Reshke <reshkekirill@gmail.com>
Discussion: https://www.postgresql.org/message-id/5238fe45-e486-4c62-a7f3-c7d8d416e812@iki.fi
Previously, if --stamp_file was specified, libpq_check.pl would create a
new stamp file only if none could be found. If there was already a
stamp file, the script would do nothing, leaving the previous stamp file
in place. This logic could cause unnecessary rebuilds because meson
relies on the timestamp of the output files to determine if a rebuild
should happen. In this case, a stamp file generated during an older
check would be kept, but we need a stamp file from the latest moment
where the libpq check has been run, so as correct rebuild decisions can
be taken.
This commit changes libpq_check.pl so as a fresh stamp file is created
each time libpq_check.pl is run, when --stamp_file is specified.
Oversight in commit 4a8e6f43a6.
Reported-by: Andres Freund <andres@anarazel.de>
Author: Nazir Bilal Yavuz <byavuz81@gmail.com>
Reviewed-by: VASUKI M <vasukim1992002@gmail.com>
Discussion: https://postgr.es/m/CAN55FZ22rrN6gCn7urtmTR=_5z7ArZLUJu-TsMChdXwmRTaquA@mail.gmail.com
The main purpose of this change is to allow an ABI checker to understand
when the list of SysCacheIdentifier changes, by switching all the
routine declarations that relied on a signed integer for a syscache ID
to this new type. This is going to be useful in the long-term for
versions newer than v19 so as we will be able to check when the list of
values in SysCacheIdentifier is updated in a non-ABI compliant fashion.
Most of the changes of this commit are due to the new definition of
SyscacheCallbackFunction, where a SysCacheIdentifier is now required for
the syscache ID. It is a mechanical change, still slightly invasive.
There are more areas in the tree that could be improved with an ABI
checker in mind; this takes care of only one area.
Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Author: Andreas Karlsson <andreas@proxel.se>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/289125.1770913057@sss.pgh.pa.us
This commit tweaks the generation of the syscache IDs for the enum
SysCacheIdentifier to now include an invalid value, with -1 assigned as
value. The concept of an invalid syscache ID exists when handling
lookups of a ObjectAddress, based on their set of properties in
ObjectPropertyType. -1 is used for the case where an object type has no
option for a syscache lookup.
This has been found as independently useful while discussing a switch of
SysCacheIdentifier to a typedef, as we already have places that want to
know about the concept of an invalid value when dealing with
ObjectAddresses.
Reviewed-by: Andreas Karlsson <andreas@proxel.se>
Discussion: https://postgr.es/m/aZQRnmp9nVjtxAHS@paquier.xyz
get_catalog_object_by_oid_extended() has been doing a syscache lookup
when given a cache ID strictly higher than 0, which is wrong because the
first valid value of SysCacheIdentifier is 0.
This issue had no consequences, as the first value assigned in the
enum SysCacheIdentifier is AGGFNOID, which is not used in the object
type properties listed in objectaddress.c. Even if an ID of 0 was
hypotherically given, the code would still work with a less efficient
heap-or-index scan.
Discussion: https://postgr.es/m/aZTr_R6JGmqokUBb@paquier.xyz
... instead of passing a bunch of separate booleans.
Also, rearrange the argument list in a hopefully more sensible order.
Discussion: https://postgr.es/m/202602111846.xpvuccb3inbx@alvherre.pgsql
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Fabrízio de Royes Mello <fabriziomello@gmail.com> (older version)
Commit 38e0190ced forgot to pfree() an allocation (freed in other
places of the same function) in only one of several spots in
check_log_min_messages(). Per Coverity. Add that.
While at it, avoid open-coding guc_strdup(). The new coding does a
strlen() that wasn't there before, but I doubt it's measurable.
Previously, SIGINT was treated the same as SIGTERM in walwriter and
walsummarizer. That decision goes back to when the walwriter process
was introduced (commit ad4295728e), and was later copied to
walsummarizer. It was a pretty arbitrary decision back then, and we
haven't adopted that convention in all the other processes that have
been introduced later.
Summary of how other processes respond to SIGINT:
- Autovacuum launcher: Cancel the current iteration of launching
- bgworker: Ignore (unless connected to a database)
- checkpointer: Request shutdown checkpoint
- bgwriter: Ignore
- pgarch: Ignore
- startup process: Ignore
- walreceiver: Ignore
- IO worker: die()
IO workers are a notable exception in that they exit on SIGINT, and
there's a documented reason for that: IO workers ignore SIGTERM, so
SIGINT provides a way to manually kill them. (They do respond to
SIGUSR2, though, like all the other processes that we don't want to
exit immediately on SIGTERM on operating system shutdown.)
To make this a little more consistent, ignore SIGINT in walwriter and
walsummarizer. They have no "query" to cancel, and they react to
SIGTERM just fine.
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/818bafaf-1e77-4c78-8037-d7120878d87c@iki.fi
Most of the StaticAssert macros already worked in C++ with Clang and
GCC:(the only compilers we're currently testing C++ extension support
for). This adds a regression test for them in our test C++ extension,
so we can safely change their implementation without accidentally
breaking C++.
The only macros that StaticAssert macros that don't work yet are the
StaticAssertVariableIsOfType and StaticAssertVariableIsOfTypeMacro.
These will be added in a follow-on commit.
Author: Jelte Fennema-Nio <postgres@jeltef.nl>
Discussion: https://www.postgresql.org/message-id/flat/CAGECzQR21OnnKiZO_1rLWO0-16kg1JBxnVq-wymYW0-_1cUNtg@mail.gmail.com
