These "community health files" provide important information about
the project and will be displayed prominently on the PostgreSQL
GitHub mirror. For now, they just point to the website, but we may
want to expand on the content in the future.
Reviewed-by: Peter Eisentraut, Alvaro Herrera, Tom Lane
Discussion: https://postgr.es/m/20240417023609.GA3228660%40nathanxps13
After several refactoring iterations, auxiliary processes are no
longer initialized from the bootstrapper. Using the InitProcessing
mode for initializing auxiliary processes is more appropriate. Since
the global variable Mode is initialized to InitProcessing, we can just
remove the redundant calls of SetProcessingMode(InitProcessing).
Author: Xing Guo <higuoxing@gmail.com>
Discussion: https://www.postgresql.org/message-id/CACpMh%2BDBHVT4xPGimzvex%3DwMdMLQEu9PYhT%2BkwwD2x2nu9dU_Q%40mail.gmail.com
getSchemaData() does not use the return values of many of its get*
helper functions because they store the data elsewhere. For
example, commit 92316a4582 introduced a separate hash table for
dumpable objects that said helper functions populate. This commit
changes these functions to return void and removes their "int *"
parameters that returned the number of objects found.
Reviewed-by: Neil Conway, Tom Lane, Daniel Gustafsson
Discussion: https://postgr.es/m/ZmCAtVaOrHpf31PJ%40nathan
Using memcpy with strlen as the size parameter will not take the
NULL terminator into account, relying instead on the destination
buffer being properly initialized. Replace with strlcpy which is
a safer alternative, and more in line with how we handle copying
strings elsewhere.
Author: Ranier Vilela <ranier.vf@gmail.com>
Discussion: https://postgr.es/m/CAEudQApAsbLsQ+gGiw-hT+JwGhgogFa_=5NUkgFO6kOPxyNidQ@mail.gmail.com
Remove the test from commit 9c2e660b07. This test ends up allocating
quite a bit of memory, which can make the test fail with out of memory
errors on some build farm machines.
MaxAllocSize puts an upper bound on the largest possible parameter
number ($268435455). Use that limit instead of INT_MAX to report that
no parameters exist beyond that point instead of reporting an error
about the maximum allocation size being exceeded.
Author: Erik Wienhold <ewie@ewie.name>
Discussion: https://www.postgresql.org/message-id/flat/5d216d1c-91f6-4cbe-95e2-b4cbd930520c@ewie.name
Replace atol with pg_strtoint32_safe in the backend parser and with
strtoint in ECPG to reject overflows when parsing the number of a
positional parameter. With atol from glibc, parameters $2147483648 and
$4294967297 turn into $-2147483648 and $1, respectively.
Author: Erik Wienhold <ewie@ewie.name>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Reviewed-by: Alexander Lakhin <exclusion@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/5d216d1c-91f6-4cbe-95e2-b4cbd930520c@ewie.name
We don't need the pre-existing subscriptions on the newly formed
subscriber by using pg_createsubscriber. The apply workers corresponding
to these subscriptions can connect to other publisher nodes and either get
some unwarranted data or can lead to ERRORs in connecting to such nodes.
Author: Kuroda Hayato
Reviewed-by: Amit Kapila, Shlok Kyal, Vignesh C
Backpatch-through: 17
Discussion: https://postgr.es/m/OSBPR01MB25526A30A1FBF863ACCDDA3AF5C92@OSBPR01MB2552.jpnprd01.prod.outlook.com
We have in launch_backend.c:
/*
* The following need to be available to the save/restore_backend_variables
* functions. They are marked NON_EXEC_STATIC in their home modules.
*/
extern slock_t *ShmemLock;
extern slock_t *ProcStructLock;
extern PGPROC *AuxiliaryProcs;
extern PMSignalData *PMSignalState;
extern pg_time_t first_syslogger_file_time;
extern struct bkend *ShmemBackendArray;
extern bool redirection_done;
That comment is not completely true: ShmemLock, ShmemBackendArray, and
redirection_done are not in fact NON_EXEC_STATIC. ShmemLock once was,
but was then needed elsewhere. ShmemBackendArray was static inside
postmaster.c before launch_backend.c was created. redirection_done
was never static.
This patch moves the declaration of ShmemLock and redirection_done to
a header file.
ShmemBackendArray gets a NON_EXEC_STATIC. This doesn't make a
difference, since it only exists if EXEC_BACKEND anyway, but it makes
it consistent.
After that, the comment is now correct.
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/flat/e0a62134-83da-4ba4-8cdb-ceb0111c95ce@eisentraut.org
src/backend/libpq/pqcomm.c: "postmaster/postmaster.h" for Unix_socket_group, Unix_socket_permissions
src/backend/utils/init/globals.c: "postmaster/postmaster.h" for MyClientSocket
src/backend/utils/misc/guc_tables.c: "utils/rls.h" for row_security
src/backend/utils/sort/tuplesort.c: "utils/guc.h" for trace_sort
Nothing currently diagnoses missing includes for global variables, but
this is being cleaned up, and these ones had an obvious header file
available.
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/flat/e0a62134-83da-4ba4-8cdb-ceb0111c95ce@eisentraut.org
This commit removes unused variables and routines from some perl code
that have accumulated across the years. This touches the following
areas:
- Wait event generation script.
- AdjustUpgrade.pm.
- TAP perl code
Author: Alexander Lakhin
Reviewed-by: Dagfinn Ilmari Mannsåker
Discussion: https://postgr.es/m/70b340bc-244a-589d-ef8b-d8aebb707a84@gmail.com
Since 374c7a2290, it is possible to set a table AM on a partitioned
table. This information was showing up already in psql with \d+, while
\dP+ provided no information.
This commit extends \dP+ to show the access method used by a partitioned
table or index, if set.
Author: Justin Pryzby
Discussion: https://postgr.es/m/ZkyivySXnbvOogZz@pryzbyj2023
This old CPU architecture hasn't been produced in decades, and
whatever instances might still survive are surely too underpowered
for anyone to consider running Postgres on in production. We'd
nonetheless continued to carry code support for it (largely at my
insistence), because its unique implementation of spinlocks seemed
like a good edge case for our spinlock infrastructure. However,
our last buildfarm animal of this type was retired last year, and
it seems quite unlikely that another will emerge. Without the ability
to run tests, the argument that this is useful test code fails to
hold water. Furthermore, carrying code support for an untestable
architecture has costs not to be ignored. So, remove HPPA-specific
code, in the same vein as commits 718aa43a4 and 92d70b77e.
Discussion: https://postgr.es/m/3351991.1697728588@sss.pgh.pa.us
This commit adjusts pg_sequence_last_value() to return NULL instead
of ERROR-ing for sequences for which the current user lacks
privileges. This allows us to remove the call to
has_sequence_privilege() in the definition of the pg_sequences
system view.
Bumps catversion.
Suggested-by: Michael Paquier
Reviewed-by: Michael Paquier, Tom Lane
Discussion: https://postgr.es/m/20240501005730.GA594666%40nathanxps13
Up to now, committing a transaction has caused CurrentMemoryContext to
get set to TopMemoryContext. Most callers did not pay any particular
heed to this, which is problematic because TopMemoryContext is a
long-lived context that never gets reset. If the caller assumes it
can leak memory because it's running in a limited-lifespan context,
that behavior translates into a session-lifespan memory leak.
The first-reported instance of this involved ProcessIncomingNotify,
which is called from the main processing loop that normally runs in
MessageContext. That outer-loop code assumes that whatever it
allocates will be cleaned up when we're done processing the current
client message --- but if we service a notify interrupt, then whatever
gets allocated before the next switch to MessageContext will be
permanently leaked in TopMemoryContext. sinval catchup interrupts
have a similar problem, and I strongly suspect that some places in
logical replication do too.
To fix this in a generic way, let's redefine the behavior as
"CommitTransactionCommand restores the memory context that was current
at entry to StartTransactionCommand". This clearly fixes the issue
for the notify and sinval cases, and it seems to match the mental
model that's in use in the logical replication code, to the extent
that anybody thought about it there at all.
For consistency, likewise make subtransaction exit restore the context
that was current at subtransaction start (rather than always selecting
the CurTransactionContext of the parent transaction level). This case
has less risk of resulting in a permanent leak than the outer-level
behavior has, but it would not meet the principle of least surprise
for some CommitTransactionCommand calls to restore the previous
context while others don't.
While we're here, also change xact.c so that we reset
TopTransactionContext at transaction exit and then re-use it in later
transactions, rather than dropping and recreating it in each cycle.
This probably doesn't save a lot given the context recycling mechanism
in aset.c, but it should save a little bit. Per suggestion from David
Rowley. (Parenthetically, the text in src/backend/utils/mmgr/README
implies that this is how I'd planned to implement it as far back as
commit 1aebc3618 --- but the code actually added in that commit just
drops and recreates it each time.)
This commit also cleans up a few places outside xact.c that were
needlessly making TopMemoryContext current, and thus risking more
leaks of the same kind. I don't think any of them represent
significant leak risks today, but let's deal with them while the
issue is top-of-mind.
Per bug #18512 from wizardbrony. Commit to HEAD only, as this change
seems to have some risk of breaking things for some callers. We'll
apply a narrower fix for the known-broken cases in the back branches.
Discussion: https://postgr.es/m/3478884.1718656625@sss.pgh.pa.us
There's no reason to ensure that the files pg_upgrade generates
with pg_dump and pg_dumpall have been written safely to disk. If
there is a crash during pg_upgrade, the upgrade must be restarted
from the beginning; dump files left behind by previous pg_upgrade
attempts cannot be reused.
Reviewed-by: Peter Eisentraut, Tom Lane, Michael Paquier, Daniel Gustafsson
Discussion: https://postgr.es/m/20240503171348.GA1731524%40nathanxps13
When an OOM occurred, this function was incorrectly setting a status of
CONNECTION_BAD on the passed in PGconn instead of on the newly created
PGcancelConn.
Mistake introduced with 61461a300c. Backpatch to 17.
Author: Jelte Fennema-Nio <postgres@jeltef.nl>
Reported-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/20240630190040.26.nmisch@google.com
BuildDescForRelation() goes out of its way to fill in
->constr->has_not_null, but that value is not used for anything later,
so this code can all be removed. Note that BuildDescForRelation()
doesn't make any effort to fill in the rest of ->constr, so there is
no claim that that structure is completely filled in.
Reviewed-by: Tomasz Rybak <tomasz.rybak@post.pl>
Discussion: https://www.postgresql.org/message-id/flat/a368248e-69e4-40be-9c07-6c3b5880b0a6@eisentraut.org
The standby_slot_names GUC allows the specification of physical standby
slots that must be synchronized before the logical walsenders associated
with logical failover slots. However, for this purpose, the GUC name is
too generic.
Author: Hou Zhijie
Reviewed-by: Bertrand Drouvot, Masahiko Sawada
Backpatch-through: 17
Discussion: https://postgr.es/m/ZnWeUgdHong93fQN@momjian.us
Shared statistics with a fixed number of objects are read from the stats
file in pgstat_read_statsfile() using members of PgStat_ShmemControl and
following an order based on their PgStat_Kind value.
Instead of being explicit, this commit changes the stats read to iterate
over the pgstat_kind_infos array to find the memory locations to read
into, based on a new shared_ctl_off in PgStat_KindInfo that can be used
to define the position of this stats kind in shared memory. This makes
the read logic simpler, and eases the introduction of future
improvements aimed at making this area more pluggable for external
modules.
Original idea suggested by Andres Freund.
Author: Tristan Partin
Reviewed-by: Andres Freund, Michael Paquier
Discussion: https://postgr.es/m/D12SQ7OYCD85.20BUVF3DWU5K7@neon.tech
Also omit backslashes (\) in the generated database names on Windows.
As before, perhaps we can revert this after updating affected
buildfarm animals.
Discussion: https://postgr.es/m/2509767.1719773880@sss.pgh.pa.us
This field is used to track if a stats kind can use a custom format
representation on disk when reading or writing its stats case. On HEAD,
this exists for replication slots stats, that need a mapping between an
internal index ID and the slot names.
named_on_disk is currently used nowhere and the callbacks
to_serialized_name and from_serialized_name are in charge of checking if
the serialization of the stats data should apply, so let's remove it.
Reviewed-by: Andres Freund
Discussion: https://postgr.es/m/ZmKVlSX_T5YvIOsd@paquier.xyz
Until now, when an enlargeStringInfo() call would cause the StringInfo to
exceed its maximum size, we reported an "out of memory" error. This is
misleading as it's no such thing.
Here we remove the "out of memory" text and replace it with something
more relevant to better indicate that it's a program limitation that's
been reached.
Reported-by: Michael Banck
Reviewed-by: Daniel Gustafsson, Tom Lane
Discussion: https://postgr.es/m/18484-3e357ade5fe50e61@postgresql.org
This is required before the creation of a new branch. pgindent is
clean, as well as is reformat-dat-files.
perltidy version is v20230309, as documented in pgindent's README.
Don't include double-quotes (") in the generated database names
on Windows. Doing so tickles a bug in older versions of IPC::Run,
which fail to quote command line arguments correctly for that
platform. Possibly we can revert this after updating affected
buildfarm animals.
Discussion: https://postgr.es/m/2509767.1719773880@sss.pgh.pa.us
Introduces an environment variable PG_TEST_PG_COMBINEBACKUP_MODE, that
determines copy mode used by pg_combinebackup in TAP tests. Defaults to
"--copy" but may be set to "--clone" or "--copy-file-range" to use the
alternative stategies.
Reported-by: Peter Eisentraut
Discussion: https://postgr.es/m/48da4a1f-ccd9-4988-9622-24f37b1de2b4%40eisentraut.org
Introduces --copy as an alternative to --clone and --copy-file-range.
This option simply picks the default mode to copy files, as if none of
the options was specified. This makes pg_combinebackup options more
consistent with pg_upgrade, and it makes testing simpler.
Reported-by: Peter Eisentraut
Discussion: https://postgr.es/m/48da4a1f-ccd9-4988-9622-24f37b1de2b4%40eisentraut.org
The code for file cloning existed, but was not reachable as it relied on
constants from missing headers. Due to that, on Linux --clone always
failed with
error: file cloning not supported on this platform
Fixed by including the missing headers to relevant places. Adding the
headers revealed a couple compile errors in copy_file_clone(), so fix
those too.
Reported-by: Peter Eisentraut
Discussion: https://postgr.es/m/48da4a1f-ccd9-4988-9622-24f37b1de2b4%40eisentraut.org
pg_createsubscriber currently always sets up logical replication
with two-phase commit disabled. Improving that is not going to
happen for v17. In the meantime, document the deficiency, and
adjust pg_createsubscriber so that it will emit a warning if
the source installation has max_prepared_transactions > 0.
Hayato Kuroda (some mods by Amit Kapila and me), per complaint from
Noah Misch
Discussion: https://postgr.es/m/20240623062157.97.nmisch@google.com
The original coding here could fail with database names, user names,
etc that contain spaces or other special characters.
As partial test coverage, extend the 040_pg_createsubscriber.pl
test script so that it uses a generated database name containing
funny characters.
Hayato Kuroda (some mods by me), per complaint from Noah Misch
Discussion: https://postgr.es/m/20240623062157.97.nmisch@google.com
Instead of looking up casts at parse time for converting the result
of JsonPath* query functions to the specified or the default
RETURNING type, always perform the conversion at runtime using either
the target type's input function or the function
json_populate_type().
There are two motivations for this change:
1. json_populate_type() coerces to types with typmod such that any
string values that exceed length limit cause an error instead of
silent truncation, which is necessary to be standard-conforming.
2. It was possible to end up with a cast expression that doesn't
support soft handling of errors causing bugs in the of handling
ON ERROR clause.
JsonExpr.coercion_expr which would store the cast expression is no
longer necessary, so remove.
Bump catversion because stored rules change because of the above
removal.
Reported-by: Alvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: Jian He <jian.universality@gmail.com>
Discussion: Discussion: https://postgr.es/m/202405271326.5a5rprki64aw%40alvherre.pgsql
Ensure SQL/JSON constructor functions that allow specifying the
target type using the RETURNING clause perform implicit cast to
that type. This ensures that output values that exceed the specified
length produce an error rather than being silently truncated. This
behavior conforms to the SQL standard.
Reported-by: Alvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: Jian He <jian.universality@gmail.com>
Discussion: https://postgr.es/m/202405271326.5a5rprki64aw%40alvherre.pgsql
Before this commit, when the WAL summarizer started up or recovered
from an error, it would resume summarization from wherever it left
off. That was OK normally, but wrong if summarize_wal=off had been
turned off temporary, allowing some WAL to be removed, and then turned
back on again. In such cases, the WAL summarizer would simply hang
forever. This commit changes the reinitialization sequence for WAL
summarizer to rederive the starting position in the way we were
already doing at initial startup, fixing the problem.
Per report from Israel Barth Rubio. Reviewed by Tom Lane.
Discussion: http://postgr.es/m/CA+TgmoYN6x=YS+FoFOS6=nr6=qkXZFWhdiL7k0oatGwug2hcuA@mail.gmail.com
Currently, the grammar allows any supported values in the ON ERROR
and ON EMPTY clauses for SQL/JSON functions, regardless of whether
the values are appropriate for the function. This commit ensures
that during parse analysis, the provided value is checked for
validity for the given function and throws a syntax error if it is
not.
While at it, this fixes some omissions in the documentation of the
ON ERROR/EMPTY clauses for JSON_TABLE().
Reported-by: Jian He <jian.universality@gmail.com>
Reviewed-by: Jian He <jian.universality@gmail.com>
Discussion: https://postgr.es/m/CACJufxFgWGqpESSYzyJ6tSurr3vFYBSNEmCfkGyB_dMdptFnZQ%40mail.gmail.com
Nathan Bossart