Commit Graph

42113 Commits

Author SHA1 Message Date
Michael Paquier 1301c80b21 Remove MSVC scripts
This commit removes all the scripts located in src/tools/msvc/ to build
PostgreSQL with Visual Studio on Windows, meson becoming the recommended
way to achieve that.  The scripts held some information that is still
relevant with meson, information kept and moved to better locations.
Comments that referred directly to the scripts are removed.

All the documentation still relevant that was in install-windows.sgml
has been moved to installation.sgml under a new subsection for Visual.
All the content specific to the scripts is removed.  Some adjustments
for the documentation are planned in a follow-up set of changes.

Author: Michael Paquier
Reviewed-by: Peter Eisentraut, Andres Freund
Discussion: https://postgr.es/m/ZQzp_VMJcerM1Cs_@paquier.xyz
2023-12-20 09:44:37 +09:00
Robert Haas aafc07c7a1 Move src/bin/pg_verifybackup/parse_manifest.c into src/common.
This makes it possible for the code to be easily reused by other
client-side tools, and/or by the server.

Patch by me. Review of this patch in particular by at least Peter
Eisentraut; reviewers for the patch series in general include Dilip
Kumar, Andres Fruend, David Steele, Álvaro Herrera, and Jakub Wartak.

Discussion: http://postgr.es/m/CA+TgmoZ6UGZVnSy5iak6s6+AXu_DewXovDjhLs3-su6nmU_x_g@mail.gmail.com
2023-12-19 15:24:31 -05:00
Robert Haas 47f01d727e Fix brown paper bag bug in 5c47c6546c.
The previous logic failed to work for anything other than the first
segment of a relation.

Report by Jakub Wartak. Patch by me.

Discussion: http://postgr.es/m/CAKZiRmwd3KTNMQhm9Bv4oR_1uMehXroO6kGyJQkiw9DfM8cMwQ@mail.gmail.com
2023-12-19 15:00:23 -05:00
Tom Lane 7e1ce2b3de Prevent integer overflow when forming tuple width estimates.
It's at least theoretically possible to overflow int32 when adding up
column width estimates to make a row width estimate.  (The bug example
isn't terribly convincing as a real use-case, but perhaps wide joins
would provide a more plausible route to trouble.)  This'd lead to
assertion failures or silly planner behavior.  To forestall it, make
the relevant functions compute their running sums in int64 arithmetic
and then clamp to int32 range at the end.  We can reasonably assume
that MaxAllocSize is a hard limit on actual tuple width, so clamping
to that is simply a correction for dubious input values, and there's
no need to go as far as widening width variables to int64 everywhere.

Per bug #18247 from RekGRpth.  There've been no reports of this issue
arising in practical cases, so I feel no need to back-patch.

Richard Guo and Tom Lane

Discussion: https://postgr.es/m/18247-11ac477f02954422@postgresql.org
2023-12-19 11:12:16 -05:00
Peter Eisentraut 2a607fb822 Update comment for Cardinality typedef
Author: Richard Guo <guofenglinux@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAMbWs4-Zd7Yy80RL1NdskLLo-wz6QoqsbC5TKs%3D3yZxG3BT_aA%40mail.gmail.com
2023-12-19 14:58:47 +01:00
Heikki Linnakangas 3c080fb4fa Simplify newNode() by removing special cases
- Remove MemoryContextAllocZeroAligned(). It was supposed to be a
  faster version of MemoryContextAllocZero(), but modern compilers turn
  the MemSetLoop() into a call to memset() anyway, making it more or
  less identical to MemoryContextAllocZero(). That was the only user of
  MemSetTest, MemSetLoop, so remove those too, as well as palloc0fast().

- Convert newNode() to a static inline function. When this was
  originally originally written, it was written as a macro because
  testing showed that gcc didn't inline the size check as we
  intended. Modern compiler versions do, and now that it just calls
  palloc0() there is no size-check to inline anyway.

One nice effect is that the palloc0() takes one less argument than
MemoryContextAllocZeroAligned(), which saves a few instructions in the
callers of newNode().

Reviewed-by: Peter Eisentraut, Tom Lane, John Naylor, Thomas Munro
Discussion: https://www.postgresql.org/message-id/b51f1fa7-7e6a-4ecc-936d-90a8a1659e7c@iki.fi
2023-12-19 12:11:47 +02:00
Amit Kapila c8bc807cf8 pgoutput: Raise an error for missing protocol version parameter.
Currently, we give a misleading error if the user omits to pass the
required parameter 'proto_version' in SQL API
pg_logical_slot_get_changes() or during START_REPLICATION protocol
message. The error raised is as follows which indicates that the wrong
version is passed.
ERROR:  client sent proto_version=0 but server only supports protocol 1 or higher

Author: Emre Hasegeli
Reviewed-by: Peter Smith, Amit Kapila
Discussion: https://postgr.es/m/CAE2gYzwdwtUbs-tPSV-QBwgTubiyGD2ZGsSnAVsDfAGGLDrGOA@mail.gmail.com
2023-12-19 09:53:33 +05:30
Tom Lane 8b965c549d compute_bitmap_pages' loop_count parameter should be double not int.
The value was double in the original implementation of this logic.
Commit da08a6598 pulled it out into a subroutine, but carelessly
declared the parameter as int when it should have been double.
On most platforms, the only ill effect would be to clamp the value
to be not more than INT_MAX, which would seldom be exceeded and
probably wouldn't change the estimates too much anyway.  Nonetheless,
it's wrong and can cause complaints from ubsan.

While here, improve the comments and parameter names.

This is an ABI change in a globally exposed subroutine, so
back-patching would create some risk of breaking extensions.
The value of the fix doesn't seem high enough to warrant taking
that risk, so fix in HEAD only.

Per report from Alexander Lakhin.

Discussion: https://postgr.es/m/f5e15fe1-202d-1936-f47c-f0c69a936b72@gmail.com
2023-12-18 12:46:10 -05:00
Nathan Bossart 64b1fb5f03 Optimize pg_atomic_exchange_u32 and pg_atomic_exchange_u64.
Presently, all platforms implement atomic exchanges by performing
an atomic compare-and-swap in a loop until it succeeds.  This can
be especially expensive when there is contention on the atomic
variable.  This commit optimizes atomic exchanges on many platforms
by using compiler intrinsics, which should compile into something
much less expensive than a compare-and-swap loop.  Since these
intrinsics have been available for some time, the inline assembly
implementations are omitted.

Suggested-by: Andres Freund
Reviewed-by: Andres Freund
Discussion: https://postgr.es/m/20231129212905.GA1258737%40nathanxps13
2023-12-18 10:53:32 -06:00
Nathan Bossart 0d1adae6f7 Micro-optimize datum_to_json_internal() some more.
Commit dc3f9bc549 mainly targeted the JSONTYPE_NUMERIC code path.
This commit applies similar optimizations (e.g., removing
unnecessary runtime calls to strlen() and palloc()) to nearby code.

Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/20231208203708.GA4126315%40nathanxps13
2023-12-18 10:34:33 -06:00
Thomas Munro 4908c58720 Provide vectored variants of smgrread() and smgrwrite().
smgrreadv() and smgrwritev() and their md.c implementations call
FileReadV() and FileWriteV().  A range of disk blocks beginning at
'blocknum' and extending for 'nblocks' can be scattered to or gathered
from multiple buffers with a single system call.  The traditional
smgrread() and smgrwrite() functions are implemented in terms of the new
functions.

Later commits will introduce calls with nblocks > 1, but the following
behavioral changes can be seen already:

* After a short transfer we'll now retry until we eventually read 0
  bytes (= EOF) or get ENOSPC, EDQUOT, EFBIG etc, where previously we
  would infer the reason.  Retrying is consistent with xlog.c's
  treatment of large WAL writes, and arguably also xlog.c and fd.c's
  treatment of EINTR.  Arbitrary short returns for larger transfers have
  been observed on several OSes, and might in theory also happen for
  transient reasons with our own pg_p*v() fallback code.

* After unexpected EOF or -1, the error thrown now talks about
  a range even for the single block case, eg "blocks 42..42".

Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://postgr.es/m/CA+hUKGJkOiOCa+mag4BF+zHo7qo=o9CFheB8=g6uT5TUm2gkvA@mail.gmail.com
2023-12-18 15:01:50 +13:00
Michael Paquier 3c9d9acae0 Refactor pgstat_prepare_io_time() with an input argument instead of a GUC
Originally, this routine relied on track_io_timing to check if a time
interval for an I/O operation stored in pg_stat_io should be initialized
or not.  However, the addition of WAL statistics to pg_stat_io requires
that the initialization happens when track_wal_io_timing is enabled,
which is dependent on the code path where the I/O operation happens.

Author: Nazir Bilal Yavuz
Discussion: https://postgr.es/m/CAN55FZ3AiQ+ZMxUuXnBpd0Rrh1YhwJ5FudkHg=JU0P+-W8T4Vg@mail.gmail.com
2023-12-16 20:16:20 +01:00
Alvaro Herrera a6be0600ac
Remove useless LIMIT_OPTION_DEFAULT value from LimitOption
During the development that led to commit 357889eb17, for a time we
had the value LIMIT_OPTION_DEFAULT, which was mostly but not completely
removed later on, before commit.  Complete the removal now.

Author: Zhang Mingli <avamingli@gmail.com>
Discussion: https://postgr.es/m/59d61a1a-3858-475a-964f-24468c97cc67@Spark
2023-12-16 18:20:03 +01:00
Thomas Munro b485ad7f07 Provide multi-block smgrprefetch().
Previously smgrprefetch() could issue POSIX_FADV_WILLNEED advice for a
single block at a time.  Add an nblocks argument so that we can do the
same for a range of blocks.  This usually produces a single system call,
but might need to loop if it crosses a segment boundary.  Initially it
is only called with nblocks == 1, but proposed patches will make wider
calls.

Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> (earlier version)
Discussion: https://postgr.es/m/CA+hUKGJkOiOCa+mag4BF+zHo7qo=o9CFheB8=g6uT5TUm2gkvA@mail.gmail.com
2023-12-16 17:51:21 +13:00
Tom Lane 59bd34c2fa Fix bugs in manipulation of large objects.
In v16 and up (since commit afbfc0298), large object ownership
checking has been broken because object_ownercheck() didn't take care
of the discrepancy between our object-address representation of large
objects (classId == LargeObjectRelationId) and the catalog where their
ownership info is actually stored (LargeObjectMetadataRelationId).
This resulted in failures such as "unrecognized class ID: 2613"
when trying to update blob properties as a non-superuser.

Poking around for related bugs, I found that AlterObjectOwner_internal
would pass the wrong classId to the PostAlterHook in the no-op code
path where the large object already has the desired owner.  Also,
recordExtObjInitPriv checked for the wrong classId; that bug is only
latent because the stanza is dead code anyway, but as long as we're
carrying it around it should be less wrong.  These bugs are quite old.

In HEAD, we can reduce the scope for future bugs of this ilk by
changing AlterObjectOwner_internal's API to let the translation happen
inside that function, rather than requiring callers to know about it.

A more bulletproof fix, perhaps, would be to start using
LargeObjectMetadataRelationId as the dependency and object-address
classId for blobs.  However that has substantial risk of breaking
third-party code; even within our own code, it'd create hassles
for pg_dump which would have to cope with a version-dependent
representation.  For now, keep the status quo.

Discussion: https://postgr.es/m/2650449.1702497209@sss.pgh.pa.us
2023-12-15 13:55:05 -05:00
Michael Paquier 8a7cbfce13 Prevent tuples to be marked as dead in subtransactions on standbys
Dead tuples are ignored and are not marked as dead during recovery, as
it can lead to MVCC issues on a standby because its xmin may not match
with the primary.  This information is tracked by a field called
"xactStartedInRecovery" in the transaction state data, switched on when
starting a transaction in recovery.

Unfortunately, this information was not correctly tracked when starting
a subtransaction, because the transaction state used for the
subtransaction did not update "xactStartedInRecovery" based on the state
of its parent.  This would cause index scans done in subtransactions to
return inconsistent data, depending on how the xmin of the primary
and/or the standby evolved.

This is broken since the introduction of hot standby in efc16ea520, so
backpatch all the way down.

Author: Fei Changhong
Reviewed-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/tencent_C4D907A5093C071A029712E73B43C6512706@qq.com
Backpatch-through: 12
2023-12-12 17:05:18 +01:00
Daniel Gustafsson c5962ad21a Fix typo in comment
Commit 98e675ed7a accidentally mistyped IDENTIFY_SYSTEM as
IDENTIFY_SERVER. Backpatch to all supported branches.

Reported-by: Alexander Lakhin <exclusion@gmail.com>
Discussion: https://postgr.es/m/68138521-5345-8780-4390-1474afdcba1f@gmail.com
2023-12-12 12:16:38 +01:00
Thomas Munro 871fe4917e Provide vectored variants of FileRead() and FileWrite().
FileReadV() and FileWriteV() adapt pg_preadv() and pg_pwritev() for
fd.c's virtual file descriptors.  The simple FileRead() and FileWrite()
functions are now implemented in terms of the vectored functions, to
avoid code duplication, and they are converted back to the corresponding
simple system calls further down (commit 15c9ac36).  Later work will
make more interesting multi-iovec calls.

The traditional behavior of reporting a "fake" ENOSPC error is
simplified.  It's now always set for non-failing writes, for the benefit
of callers that expect to log a meaningful "%m" if they determine that
the write was short.  (Perhaps we should consider getting rid of that
expectation one day.)

Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://postgr.es/m/CA+hUKGJkOiOCa+mag4BF+zHo7qo=o9CFheB8=g6uT5TUm2gkvA@mail.gmail.com
2023-12-12 13:12:43 +13:00
Thomas Munro 0c6be59f5e Provide helper for retrying partial vectored I/O.
compute_remaining_iovec() is a re-usable routine for retrying after
pg_readv() or pg_writev() reports a short transfer.  This will gain new
users in a later commit, but can already replace the open-coded
equivalent code in the existing pg_pwritev_with_retry() function.

Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://postgr.es/m/CA+hUKGJkOiOCa+mag4BF+zHo7qo=o9CFheB8=g6uT5TUm2gkvA@mail.gmail.com
2023-12-12 10:57:18 +13:00
Thomas Munro baf7c93ed5 Define unconstify() and unvolatize() for C++.
These two macros wouldn't work if used in an inline function definition
in a header seen by g++, because __builtin_types_compatible_p is only
available in C.  Redirect to standard C++ const_cast (which also
adds/removes volatile despite its name).

Per cpluspluscheck failure in a development branch.

Suggested-by: Peter Eisentraut <peter@eisentraut.org>
Discussion: https://postgr.es/m/CA%2BhUKGK3OXFjkOyZiw-DgL2bUqk9by1uGuCnViJX786W%2BfyDSw%40mail.gmail.com
2023-12-12 09:46:46 +13:00
Tom Lane 0a5c46a7a4 Be more wary about OpenSSL not setting errno on error.
OpenSSL will sometimes return SSL_ERROR_SYSCALL without having set
errno; this is apparently a reflection of recv(2)'s habit of not
setting errno when reporting EOF.  Ensure that we treat such cases
the same as read EOF.  Previously, we'd frequently report them like
"could not accept SSL connection: Success" which is confusing, or
worse report them with an unrelated errno left over from some
previous syscall.

To fix, ensure that errno is zeroed immediately before the call,
and report its value only when it's not zero afterwards; otherwise
report EOF.

For consistency, I've applied the same coding pattern in libpq's
pqsecure_raw_read().  Bare recv(2) shouldn't really return -1 without
setting errno, but in case it does we might as well cope.

Per report from Andres Freund.  Back-patch to all supported versions.

Discussion: https://postgr.es/m/20231208181451.deqnflwxqoehhxpe@awork3.anarazel.de
2023-12-11 11:51:56 -05:00
Alvaro Herrera d3fe6e90ba
Simplify productions for FORMAT JSON [ ENCODING name ]
This removes the production json_encoding_clause_opt, instead merging
it into json_format_clause.  Also remove the auxiliary
makeJsonEncoding() function.

Reviewed-by: Amit Langote <amitlangote09@gmail.com>
Discussion: https://postgr.es/m/202312071841.u2gueb5dsrbk%40alvherre.pgsql
2023-12-11 11:55:34 +01:00
Michael Paquier c7a3e6b46d Remove trace_recovery_messages
This GUC was intended as a debugging help in the 9.0 area when hot
standby and streaming replication were being developped, able to offer
more information at LOG level rather than DEBUGn.  There are more tools
available these days that are able to offer rather equivalent
information, like pg_waldump introduced in 9.3.  It is not obvious how
this facility is useful these days, so let's remove it.

Author: Bharath Rupireddy
Discussion: https://postgr.es/m/ZXEXEAUVFrvpquSd@paquier.xyz
2023-12-11 11:49:02 +01:00
Amit Kapila 8d7d2197f3 Fix an undetected deadlock due to apply worker.
The apply worker needs to update the state of the subscription tables to
'READY' during the synchronization phase which requires locking the
corresponding subscription. The apply worker also waits for the
subscription tables to reach the 'SYNCDONE' state after holding the locks
on the subscription and the wait is done using WaitLatch. The 'SYNCDONE'
state is changed by tablesync workers again by locking the corresponding
subscription. Both the state updates use AccessShareLock mode to lock the
subscription, so they can't block each other. However, a backend can
simultaneously try to acquire a lock on the same subscription using
AccessExclusiveLock mode to alter the subscription. Now, the backend's
wait on a lock can sneak in between the apply worker and table sync worker
causing deadlock.

In other words, apply_worker waits for tablesync worker which waits for
backend, and backend waits for apply worker. This is not detected by the
deadlock detector because apply worker uses WaitLatch.

The fix is to release existing locks in apply worker before it starts to
wait for tablesync worker to change the state.

Reported-by: Tomas Vondra
Author: Shlok Kyal
Reviewed-by: Amit Kapila, Peter Smith
Backpatch-through: 12
Discussion: https://postgr.es/m/d291bb50-12c4-e8af-2af2-7bb9bb4d8e3e@enterprisedb.com
2023-12-11 08:50:43 +05:30
Peter Eisentraut 90834ceccd Remove some unnecessary includes of "access/xlog_internal.h"
There were a few places where access/xlog_internal.h was apparently
included unnecessarily.  In some of those places, a more specific
header file (that somehow came in via access/xlog_internal.h) can be
used instead.

Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://www.postgresql.org/message-id/flat/a56a6eec-eb14-471b-9570-3cac23603964%40eisentraut.org
2023-12-10 07:46:06 +01:00
Peter Geoghegan aa210e0c12 Fix nbtree backward scan race condition comments.
Remove comments that supposed that holding a pin was a useful interlock
for _bt_walk_left().  There are times when _bt_walk_left() doesn't hold
either a lock or a pin on any page, so clearly this can't be true.
_bt_walk_left() is even prepared to deal with concurrent deletion of
both the original page and any pages to its left.

Oversight in commit 2ed5b87f96.
2023-12-08 15:37:53 -08:00
Nathan Bossart dc3f9bc549 Micro-optimize JSONTYPE_NUMERIC code path in json.c.
This commit does the following:

* In datum_to_json_internal(), the call to IsValidJsonNumber() is
  replaced with simplified validation code.  This avoids an extra
  call to strlen() in this path, and it avoids validating the
  entire string (which is okay since we know we're dealing with a
  numeric data type's output).

* In datum_to_json_internal(), the call to escape_json() in the
  JSONTYPE_NUMERIC path is replaced with code that just surrounds
  the string with quotes.  In passing, some other nearby calls to
  appendStringInfo() have been replaced with similar code to avoid
  unnecessary calls to vsnprintf().

* In composite_to_json(), the length of the separator is now
  determined at compile time to avoid unnecessary calls to
  strlen().

On my machine, this speeds up a benchmark for the proposed COPY TO
(FORMAT json) command with many integers by upwards of 20%.  There
are likely other code paths that could be given a similar
treatment, but that is left as a future exercise.

Reviewed-by: Jeff Davis, Tom Lane, David Rowley, John Naylor
Discussion: https://postgr.es/m/20231207231251.GB3359478%40nathanxps13
2023-12-08 13:39:08 -06:00
Jeff Davis 867dd2dc87 Cache opaque handle for GUC option to avoid repeasted lookups.
When setting GUCs from proconfig, performance is important, and hash
lookups in the GUC table are significant.

Per suggestion from Robert Haas.

Discussion: https://postgr.es/m/CA+TgmoYpKxhR3HOD9syK2XwcAUVPa0+ba0XPnwWBcYxtKLkyxA@mail.gmail.com
Reviewed-by: John Naylor
2023-12-08 11:16:01 -08:00
Peter Geoghegan c9c0589fda Optimize nbtree backward scan boundary cases.
Teach _bt_binsrch (and related helper routines like _bt_search and
_bt_compare) about the initial positioning requirements of backward
scans.  Routines like _bt_binsrch already know all about "nextkey"
searches, so it seems natural to teach them about "goback"/backward
searches, too.  These concepts are closely related, and are much easier
to understand when discussed together.

Now that certain implementation details are hidden from _bt_first, it's
straightforward to add a new optimization: backward scans using the <
strategy now avoid extra leaf page accesses in certain "boundary cases".
Consider the following example, which uses the tenk1 table (and its
tenk1_hundred index) from the standard regression tests:

SELECT * FROM tenk1 WHERE hundred < 12 ORDER BY hundred DESC LIMIT 1;

Before this commit, nbtree would scan two leaf pages, even though it was
only really necessary to scan one leaf page.  We'll now descend straight
to the leaf page containing a (12, -inf) high key instead.  The scan
will locate matching non-pivot tuples with "hundred" values starting
from the value 11.  The scan won't waste a page access on the right
sibling leaf page, which cannot possibly contain any matching tuples.

You can think of the optimization added by this commit as disabling an
optimization (the _bt_compare "!pivotsearch" behavior that was added to
Postgres 12 in commit dd299df8) for a small subset of cases where it was
always counterproductive.

Equivalently, you can think of the new optimization as extending the
"pivotsearch" behavior that page deletion by VACUUM has long required
(since the aforementioned Postgres 12 commit went in) to other, similar
cases.  Obviously, this isn't strictly necessary for these new cases
(unlike VACUUM, _bt_first is prepared to move the scan to the left once
on the leaf level), but the underlying principle is the same.

Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: Matthias van de Meent <boekewurm+postgres@gmail.com>
Discussion: https://postgr.es/m/CAH2-Wz=XPzM8HzaLPq278Vms420mVSHfgs9wi5tjFKHcapZCEw@mail.gmail.com
2023-12-08 11:05:17 -08:00
Tomas Vondra b437571714 Allow parallel CREATE INDEX for BRIN indexes
Allow using multiple worker processes to build BRIN index, which until
now was supported only for BTREE indexes. For large tables this often
results in significant speedup when the build is CPU-bound.

The work is split in a simple way - each worker builds BRIN summaries on
a subset of the table, determined by the regular parallel scan used to
read the data, and feeds them into a shared tuplesort which sorts them
by blkno (start of the range). The leader then reads this sorted stream
of ranges, merges duplicates (which may happen if the parallel scan does
not align with BRIN pages_per_range), and adds the resulting ranges into
the index.

The number of duplicate results produced by workers (requiring merging
in the leader process) should be fairly small, thanks to how parallel
scans assign chunks to workers. The likelihood of duplicate results may
increase for higher pages_per_range values, but then there are fewer
page ranges in total. In any case, we expect the merging to be much
cheaper than summarization, so this should be a win.

Most of the parallelism infrastructure is a simplified copy of the code
used by BTREE indexes, omitting the parts irrelevant for BRIN indexes
(e.g. uniqueness checks).

This also introduces a new index AM flag amcanbuildparallel, determining
whether to attempt to start parallel workers for the index build.

Original patch by me, with reviews and substantial reworks by Matthias
van de Meent, certainly enough to make him a co-author.

Author: Tomas Vondra, Matthias van de Meent
Reviewed-by: Matthias van de Meent
Discussion: https://postgr.es/m/c2ee7d69-ce17-43f2-d1a0-9811edbda6e6%40enterprisedb.com
2023-12-08 18:15:26 +01:00
Tomas Vondra dae761a87e Add empty BRIN ranges during CREATE INDEX
When building BRIN indexes, the brinbuildCallback only advances to the
next page range when seeing a tuple that doesn't belong to the current
one. This means that the index may end up missing ranges at the end of
the table, if those pages do not contain any indexable tuples.

We tend not to have completely empty pages at the end of a relation, but
this also applies to partial indexes, where the tuples may simply not
match the index predicate. This results in inefficient scans using the
affected BRIN index - without the summaries, the page ranges have to be
read and processed, which consumes I/O and possibly also CPU time.

The existing code already added empty ranges for earlier parts of the
table, this commit makes sure we add them for the ranges at the end of
the table too.

Patch by Matthias van de Meent, with review/improvements by me.

Author: Matthias van de Meent
Reviewed-by: Tomas Vondra
Discussion: https://postgr.es/m/CAEze2WiMsPZg%3DxkvSF_jt4%3D69k6K7gz5B8V2wY3gCGZ%2B1BzCbQ%40mail.gmail.com
2023-12-08 17:14:32 +01:00
Daniel Gustafsson 00edb2061f Don't clean initdb files on template creation failure
Commit 252dcb3239 introduced initdb template caching to speed up
tests by re-using initdb output.  The initdb command didn't however
use the --no-clean option to preserve generated data in case initdb
crashes unlike pg_regress which does do this.  This adds the option
to initdb to aid debugging.

While changing the commandline, switch to using long options for
initdb to make the code more self-documenting.

Author: Matthias van de Meent <boekewurm+postgres@gmail.com>
Discussion: https://postgr.es/m/CAEze2WhSTjfK_M+Ea4GSQp8odrEOaQS8HyORd1TJUEiyXaB+rw@mail.gmail.com
2023-12-08 13:42:54 +01:00
Heikki Linnakangas 44913add91 Remove some unnecessary #includes of postmaster/interrupt.h
Commit 44fc6e259b removed a bunch of references to
SignalHandlerForCrashExit, leaving these #includes unneeded.
2023-12-08 13:19:37 +02:00
Alvaro Herrera 7db01fbcef
Test that it works to RESET an invalid reloption
This works today, and it's valuable to ensure it doesn't get broken
if/when we get around to refactoring the implementation.

Author: Nikolay Shaplov <dhyan@nataraj.su>
Discussion: https://postgr.es/m/4563991.km65PDbjlG@thinkpad-pgpro
2023-12-08 11:58:58 +01:00
Heikki Linnakangas b31ba5310b Rename ShmemVariableCache to TransamVariables
The old name was misleading: It's not a cache, the values kept in the
struct are the authoritative source.

Reviewed-by: Tristan Partin, Richard Guo
Discussion: https://www.postgresql.org/message-id/6537d63d-4bb5-46f8-9b5d-73a8ba4720ab@iki.fi
2023-12-08 09:47:15 +02:00
Heikki Linnakangas 15916ffb04 Initialize ShmemVariableCache like other shmem areas
For sake of consistency.

Reviewed-by: Tristan Partin, Richard Guo
Discussion: https://www.postgresql.org/message-id/6537d63d-4bb5-46f8-9b5d-73a8ba4720ab@iki.fi
2023-12-08 09:46:59 +02:00
Heikki Linnakangas 049ef3398d Don't try to open visibilitymap when analyzing a foreign table
It's harmless, visibilitymap_count() returns 0 if the file doesn't
exist. But it's also very pointless. I noticed this when I added an
assertion in smgropen() that the relnumber is valid.

Discussion: https://www.postgresql.org/message-id/621a52fd-3cd8-4f5d-a561-d510b853bbaf@iki.fi
2023-12-08 09:16:21 +02:00
Thomas Munro cd7f19da34 Fix potential pointer overflow in xlogreader.c.
While checking if a record could fit in the circular WAL decoding
buffer, the coding from commit 3f1ce973 used arithmetic that could
overflow.  64 bit systems were unaffected for various technical reasons,
which probably explains the lack of problem reports.  Likewise for 32
bit systems running known 32 bit kernels.  The systems at risk of
problems appear to be 32 bit processes running on 64 bit kernels, with
unlucky placement in memory.

Per complaint from GCC -fsanitize=undefined -m32, while testing
variations of 039_end_of_wal.pl.

Back-patch to 15.

Reviewed-by: Nathan Bossart <nathandbossart@gmail.com>
Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Discussion: https://postgr.es/m/CA%2BhUKGKH0oRPOX7DhiQ_b51sM8HqcPp2J3WA-Oen%3DdXog%2BAGGQ%40mail.gmail.com
2023-12-08 16:09:03 +13:00
Michael Paquier e5b8c4f68f Fix path of regress shared library in pg_upgrade test
During a pg_upgrade test using an old dump, all references to the old
regress shared library path (so, dylib or dll) are updated to point to
the library path used by the new build, to ensure a consistent
comparison between the old and new dumps.

The test previously relied on a hardcoded value of "src/test/regress/"
to build the new path value, which would point to an incorrect location
for the meson and vpath builds.  This is replaced by REGRESS_SHLIB, able
to point to the correct location of the regress shared library.

Author: Alexander Lakhin
Discussion: https://postgr.es/m/a628d8ad-a08a-2eab-4ca9-641bc82d3193@gmail.com
Backpatch-through: 15
2023-12-08 10:36:23 +09:00
Jeff Davis 719b342d36 Shrink Unicode category table.
Missing entries can implicitly be considered "unassigned".

Discussion: https://postgr.es/m/ff4c2f2f9c8fc7ca27c1c24ae37ecaeaeaff6b53.camel@j-davis.com
2023-12-07 15:44:03 -08:00
David Rowley d16a0c1e2e Verify that attribute counts match in ExecCopySlot
tts_virtual_copyslot() contained an Assert that checked that the srcslot
contained <= attributes than the dstslot.  This seems to be backwards as
if the srcslot contained fewer attributes then the dstslot could be left
with stale Datum values from the previously stored tuple.  It might make
more sense to allow the source to contain additional attributes and only
copy the leading ones that also exist in the destination, however, that's
not what we're doing here.

Here we just remove the Assert from tts_virtual_copyslot() and add an
Assert to ExecCopySlot() to verify the attribute counts match.  There
does not seem to be any places where the destination contains fewer
attributes, so instead of going to the trouble of making the code
properly handle this, let's just ensure the attribute counts match.  If
this Assert fails then that will indicate that we do have cases that
require us to handle the srcslot with more attributes than the dstslot.
It seems better to only write this code if there's a genuine requirement
for it rather than write it now only to leave it untested.

Thanks to Andres Freund for helping with the analysis of this.

Discussion: https://postgr.es/m/CAApHDvpMAvBL0T+TRORquyx1iqFQKMVTXtqNocOw0Pa2uh1heg@mail.gmail.com
2023-12-07 21:28:24 +13:00
Michael Paquier d43bd090a8 Improve some error messages with invalid indexes for REINDEX CONCURRENTLY
An invalid index is skipped when doing REINDEX CONCURRENTLY at table
level, with INDEX_CORRUPTED used as errcode.  This is confusing,
because an invalid index could exist after an interruption.  The errcode
is switched to OBJECT_NOT_IN_PREREQUISITE_STATE instead, as per a
suggestion from Andres Freund.

While on it, the error messages are reworded, and a hint is added,
telling how to rebuild an invalid index in this case.  This has been
suggested by Noah Misch.

Discussion: https://postgr.es/m/20231118230958.4fm3fhk4ypshxopa@awork3.anarazel.de
2023-12-07 14:27:54 +09:00
Amit Kapila 0bf62460bb Fix issues in binary_upgrade_logical_slot_has_caught_up().
The commit 29d0a77fa6 labelled binary_upgrade_logical_slot_has_caught_up()
as a non-strict function to allow providing a better error message to callers
in case the passed slot_name is NULL. On further discussion, it seems that
it is not helpful to have a different error message for NULL input in this
function, so this patch marks the function as strict.

This patch also removes the explicit permission check to use replication
slots as this function is invoked only by superusers and instead adds an
Assert.

Reported-by: Masahiko Sawada
Author: Hayato Kuroda
Reviewed-by: Vignesh C
Discussion: https://postgr.es/m/CAD21AoDSyiBKkMXBxN_gUayZZUCOgyHnG8Ge8rcPXNP3Tf6B4g@mail.gmail.com
2023-12-07 08:42:48 +05:30
Michael Paquier c426f7c2b3 Fix assertion failure with REINDEX and event triggers
A REINDEX CONCURRENTLY run on a table with no indexes would always pop
the topmost snapshot from the active snapshot stack, making the snapshot
handling inconsistent between the multiple-relation and single-relation
cases.  This commit slightly changes the snapshot stack handling so as a
snapshot is popped only ReindexMultipleInternal() in this case after a
relation has been reindexed, fixing a problem where an event trigger
function may need a snapshot but does not have one.  This also keeps the
places where PopActiveSnapshot() is called closer to each other.

While on it, this expands the existing tests to cover all the cases that
could be faced with REINDEX commands and such event triggers, for one or
more relations, with or without indexes.

This behavior is inconsistent since 5dc92b844e, but we've never had a
need for an active snapshot at the end of a REINDEX until now.

Thanks also to Jian He for the input.

Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/cb538743-484c-eb6a-a8c5-359980cd3a17@gmail.com
2023-12-07 08:31:02 +09:00
Nathan Bossart c2a465b2c9 Suppress -Wunused-result warning about write().
pg_test_fsync's signal_cleanup() intentionally ignores the write()
result since there's not much we could do about it, but certain
compilers make that harder than it ought to be.

This was missed in commit 52e98d4502.

Reviewed-by: Tristan Partin, Peter Eisentraut
Discussion: https://postgr.es/m/20231206161839.GA2828158%40nathanxps13
2023-12-06 17:16:57 -06:00
Peter Eisentraut 52e98d4502 Use signal-safe functions in signal handler
According to signal-safety(7), exit(3) and puts(3) are not safe to call
in a signal handler.

Author: Tristan Partin <tristan@neon.tech>
Discussion: https://www.postgresql.org/message-id/flat/CTVDKVZCCVSY.1XQ87UL50KQRD%40gonk
2023-12-06 10:11:36 +01:00
Michael Paquier 7636725b92 Fix compilation on Windows with WAL_DEBUG
This has been broken since b060dbe000 that has reworked the callback
mechanism of XLogReader, most likely unnoticed because any form of
development involving WAL happens on platforms where this compiles fine.

Author: Bharath Rupireddy
Discussion: https://postgr.es/m/CALj2ACVF14WKQMFwcJ=3okVDhiXpuK5f7YdT+BdYXbbypMHqWA@mail.gmail.com
Backpatch-through: 13
2023-12-06 14:10:39 +09:00
Michael Paquier bfc677c3bc Apply filters to dump files all the time in 002_pg_upgrade.pl
This commit removes the restriction that would not apply filters to the
dumps used for comparison in the TAP test of pg_upgrade when using the
same base version for the old and new nodes.

The previous logic would fail on Windows if loading a dump while using
the same set of binaries for the old and new nodes, as the library
dependencies updated in the old dump would append CRLFs to the dump
file as it is treated as a text file.  The dump filtering logic replaces
all CRLFs (\r\n) by LFs (\n), which is able to prevent this issue.

When the old and new versions of the binaries are the same,
AdjustUpgrade removes all blank lines, removes version-based comments
generated by pg_dump and replaces CRLFs by LFs.

Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/60d434b9-53d9-9ea1-819b-efebdcf44e41@gmail.com
Backpatch-through: 15
2023-12-06 09:54:47 +09:00
Robert Haas 278eb13c48 Rename pg_verifybackup's JsonManifestParseContext callback functions.
The old names were too generic, and would have applied to any binary
that made use of JsonManifestParseContext. Rename to make the names
specific to pg_verifybackup, since there are plans afoot to reuse
this infrastructure.

Per suggestion from Álvaro Herrra.

Discussion: http://postgr.es/m/202311131625.o7hzq3oukuyd@alvherre.pgsql
2023-12-05 12:51:39 -05:00
Robert Haas d463aa06a9 Rename JsonManifestParseContext callbacks.
There is no real reason to just run multiple words together when
we can instead punctuate them with marks intended for that purpose.

Per suggestion from Álvaro Herrera.

Discussion: http://postgr.es/m/202311161021.nisg7imt7kyf@alvherre.pgsql
2023-12-05 12:17:49 -05:00