postgresql-pgindent.git
2 years agoRemove uses of MemoryContextContains in nodeAgg.c and nodeWindowAgg.c.
Tom Lane [Thu, 6 Oct 2022 17:27:34 +0000 (13:27 -0400)]
Remove uses of MemoryContextContains in nodeAgg.c and nodeWindowAgg.c.

MemoryContextContains is no longer reliable in the wake of c6e0fe1f2,
so we need to get rid of these uses.

It appears that there's no really good reason to force the result of
an aggregate's finalfn or serialfn to be allocated in the per-tuple
context.  The only other plausible case is that the result points to
or into the aggregate's transition value, and that's fine because it
will last as long as we need it to.  (This conclusion depends on the
assumption that finalfns are not allowed to scribble on the transition
value, but we've long required that.)  So we can just drop the
MemoryContextContains plus datumCopy business, although we do need
to take care to not return a read-write pointer when the transition
value is an expanded datum.

Likewise, we don't really need to force the result of a window
function to be in the output context.  In this case, the plausible
alternative is that it's pointing into the temporary tuple slot used
by WinGetFuncArgInPartition or WinGetFuncArgInFrame (since those
functions could return such a pointer, which might become the window
function's result).  That will hold still for long enough, unless
there is another window function using the same WindowObject.
I'm content to always perform a datumCopy when there's more than one
such function.

On net, these changes should provide small speed improvements as well
as removing problematic code.

Tom Lane and David Rowley

Discussion: https://postgr.es/m/1913788.1664898906@sss.pgh.pa.us

2 years agoTake care to de-duplicate entries in standby.c's table of locks.
Tom Lane [Thu, 6 Oct 2022 16:27:36 +0000 (12:27 -0400)]
Take care to de-duplicate entries in standby.c's table of locks.

The RecoveryLockLists data structure, which tracks all exclusive
locks that the startup process is holding on behalf of transactions
being replayed, did not have any provision for avoiding duplicate
entries for the same lock.  Maybe that was okay when the code was
first written.  However, modern practice is for checkpoints to
write fresh lists of all active exclusive locks into the WAL.
Thus, an exclusive lock that survives across multiple checkpoints
causes bloat in standbys' startup processes.  If there are a lot
of such locks this can look like a memory leak, and it's even
possible to drive the startup process into a palloc failure from
an over-length List.

To fix, use a hash table instead of simple lists to track the
locks being held.  Allowing for dynahash overhead, this requires
a little more space per lock than the old way (although it's the
same size as what we were allocating prior to c6e0fe1f2).  It's
probably a shade slower too.  However, testing indicates that the
penalty is negligible on ordinary workloads, so let's make this
change to improve robustness in extreme cases.

Patch by me, per report from Dmitriy Kuzmin.  No back-patch
(for now anyway), since it seems that a significant improvement
would only occur in corner cases.

Discussion: https://postgr.es/m/CAHLDt=_ts0A7Agn=hCpUh+RCFkxd+G6uuT=kcTfqFtGur0dp=A@mail.gmail.com

2 years agoRemove useless character-length checks in contrib/ltree.
Tom Lane [Thu, 6 Oct 2022 15:18:32 +0000 (11:18 -0400)]
Remove useless character-length checks in contrib/ltree.

The t_iseq() macro does not need to be guarded by a character
length check (at least when the comparison value is an ASCII
character, as its documentation requires).  Some portions of
contrib/ltree hadn't read that memo, so simplify them.

The last change in gettoken_query,

-                else if (charlen == 1 && !t_iseq(state->buf, ' '))
+                else if (!t_iseq(state->buf, ' '))

looks like it's actually a bug fix: I doubt that the intention
was to silently ignore multibyte characters as if they were
whitespace.  I'm not tempted to back-patch though, because this
will have the effect of tightening what is allowed in ltxtquery
strings.

Discussion: https://postgr.es/m/2548310.1664999615@sss.pgh.pa.us

2 years agoIntroduce t_isalnum() to replace t_isalpha() || t_isdigit() tests.
Tom Lane [Thu, 6 Oct 2022 15:08:56 +0000 (11:08 -0400)]
Introduce t_isalnum() to replace t_isalpha() || t_isdigit() tests.

ts_locale.c omitted support for "isalnum" tests, perhaps on the
grounds that there were initially no use-cases for that.  However,
both ltree and pg_trgm need such tests, and we do also have one
use-case now in the core backend.  The workaround of testing
isalpha and isdigit separately seems quite inefficient, especially
when dealing with multibyte characters; so let's fill in the
missing support.

Discussion: https://postgr.es/m/2548310.1664999615@sss.pgh.pa.us

2 years agoFix comment in xlogprefetcher.c
Michael Paquier [Thu, 6 Oct 2022 11:25:02 +0000 (20:25 +0900)]
Fix comment in xlogprefetcher.c

Author: Sho Kato
Discussion: https://postgr.es/m/TYCPR01MB684954052EC534A3261B29249F5C9@TYCPR01MB6849.jpnprd01.prod.outlook.com

2 years agoRefactor TAP test authentication/001_password.pl
Michael Paquier [Thu, 6 Oct 2022 00:40:16 +0000 (09:40 +0900)]
Refactor TAP test authentication/001_password.pl

The test is changed to test for connection strings rather than specific
roles, and the reset logic of pg_hba.conf is extended so as the database
and user name entries can be directly specified.  This is aimed at being
used as a base for more test scenarios of pg_hba.conf and authentication
paths.

Author: Bertrand Drouvot, Michael Paquier
Discussion: https://postgr.es/m/[email protected]

2 years agoFix final compiler warning produced by -Wshadow=compatible-local
David Rowley [Wed, 5 Oct 2022 21:19:36 +0000 (10:19 +1300)]
Fix final compiler warning produced by -Wshadow=compatible-local

We're now able to compile the entire tree with -Wshadow=compatible-local
without any compiler warnings.

Author: David Rowley
Discussion: https://postgr.es/m/CAApHDvqWGMdB_pATeUqE=JCtNqNxObPOJ00jFEa2_sZ20j_Wvg@mail.gmail.com

2 years agoAdd optional parameter to PG_TRY() macros
David Rowley [Wed, 5 Oct 2022 21:08:31 +0000 (10:08 +1300)]
Add optional parameter to PG_TRY() macros

This optional parameter can be specified in cases where there are nested
PG_TRY() statements within a function in order to stop the compiler from
issuing warnings about shadowed local variables when compiling with
-Wshadow.  The optional parameter is used as a suffix on the variable
names declared within the PG_TRY(), PG_CATCH(), PG_FINALLY() and
PG_END_TRY() macros.  The parameter, if specified, must be the same in
each component macro of the given PG_TRY() block.

This also adjusts the single case where we have nested PG_TRY() statements
to add a parameter to the inner-most PG_TRY().

This reduces the number of compiler warnings when compiling with
-Wshadow=compatible-local from 5 down to 1.

Author: David Rowley
Discussion: https://postgr.es/m/CAApHDvqWGMdB_pATeUqE=JCtNqNxObPOJ00jFEa2_sZ20j_Wvg@mail.gmail.com

2 years agodoc: clarify description for log_startup_progress_interval
Bruce Momjian [Wed, 5 Oct 2022 19:53:40 +0000 (15:53 -0400)]
doc:  clarify description for log_startup_progress_interval

Reported-by: Elena Indrupskaya
Discussion: https://postgr.es/m/0af43b49-1646-93d0-ccf1-bb3c635c8c6f@postgrespro.ru

Author: Elena Indrupskaya

Backpatch-through: 15

2 years agotests: Restrict pg_locks queries in advisory_locks.sql to current database
Andres Freund [Wed, 5 Oct 2022 17:44:38 +0000 (10:44 -0700)]
tests: Restrict pg_locks queries in advisory_locks.sql to current database

Otherwise testing an existing installation can fail, if there are other locks,
e.g. from one of the isolation tests.

Reviewed-by: Michael Paquier <[email protected]>
Discussion: https://postgr.es/m/20221003234111[email protected]

2 years agotests: Rename conflicting role names
Andres Freund [Wed, 5 Oct 2022 17:43:13 +0000 (10:43 -0700)]
tests: Rename conflicting role names

These cause problems when running installcheck-world USE_MODULE_DB=1 with -j.

Reviewed-by: Michael Paquier <[email protected]>
Discussion: https://postgr.es/m/20221003234111[email protected]

2 years agomeson: Add windows resource files
Andres Freund [Wed, 5 Oct 2022 16:56:05 +0000 (09:56 -0700)]
meson: Add windows resource files

The generated resource files aren't exactly the same ones as the old
buildsystems generate. Previously "InternalName" and "OriginalFileName" were
mostly wrong / not set (despite being required), but that was hard to fix in
at least the make build. Additionally, the meson build falls back to a
"auto-generated" description when not set, and doesn't set it in a few cases -
unlikely that anybody looks at these descriptions in detail.

Author: Andres Freund <[email protected]>
Author: Nazir Bilal Yavuz <[email protected]>
Reviewed-by: Peter Eisentraut <[email protected]>
2 years agomeson: ecpg: Split definition of static and shared libraries
Andres Freund [Wed, 5 Oct 2022 16:56:05 +0000 (09:56 -0700)]
meson: ecpg: Split definition of static and shared libraries

Required for correct resource file generation, as the resource files should
only be added to the shared library.

This also fixes a bunch of issues in the .pc files.

Previously I tried to avoid building sources twice, once for the static and
once for the shared libraries. We could still do so, but it's not clear that
it's worth the complication.

Reviewed-by: Peter Eisentraut <[email protected]>
Discussion: https://postgr.es/m/20220927011951[email protected]

2 years agomeson: libpq: Revise static / shared library setup
Andres Freund [Wed, 5 Oct 2022 16:56:05 +0000 (09:56 -0700)]
meson: libpq: Revise static / shared library setup

Improvements:
- we don't need -DFRONTEND for libpq anymore since 1d77afefbd1
- the .pc file contents for a static libpq were wrong (referencing
  {pgport, common}_shlib)
- incidentally fixes meson not supporting link_whole on AIX yet
- added explanatory comments

Previously I tried to avoid building libpq's sources twice, once for the
static and once for the shared library. We could still do so, but it's not
clear that it's worth the complication.

Reviewed-by: Peter Eisentraut <[email protected]>
2 years agomeson: docs: Add xml{lint,proc} wrapper to collect dependencies
Andres Freund [Wed, 5 Oct 2022 16:56:05 +0000 (09:56 -0700)]
meson: docs: Add xml{lint,proc} wrapper to collect dependencies

meson/ninja do not support specifying dependencies via globs (as those make it
significantly more expensive to check the current build state). Instead
targets should emit dependency information when running that then can be
cheaply re-checked during future builds.

To handle xmllint and xsltproc invocations in the docs, add and use a wrapper
that uses --load-trace to collect dependency information.

Reviewed-by: Peter Eisentraut <[email protected]>
Author: Nazir Bilal Yavuz <[email protected]>
Author: Andres Freund <[email protected]>
Discussion: https://postgr.es/m/c5736f70-bb6d-8d25-e35c-e3d886e4e905@enterprisedb.com

2 years agoFix whitespace
Peter Eisentraut [Wed, 5 Oct 2022 13:06:43 +0000 (15:06 +0200)]
Fix whitespace

2 years agoRename shadowed local variables
David Rowley [Wed, 5 Oct 2022 08:01:41 +0000 (21:01 +1300)]
Rename shadowed local variables

In a similar effort to f01592f91, here we mostly rename shadowed local
variables to remove the warnings produced when compiling with
-Wshadow=compatible-local.

This fixes 63 warnings and leaves just 5.

Author: Justin Pryzby, David Rowley
Reviewed-by: Justin Pryzby
Discussion https://postgr.es/m/20220817145434.GC26426%40telsasoft.com

2 years agoRemove definition of JUMBLE_SIZE from queryjumble.h
Michael Paquier [Wed, 5 Oct 2022 05:27:50 +0000 (14:27 +0900)]
Remove definition of JUMBLE_SIZE from queryjumble.h

The same exists in queryjumble.c, and it is used only locally in this
file so let's remove the definition in the header.

Author: Tatsu Nakamori
Reviewed-by: Tom Lane, Julien Rouhaud
Discussion: https://postgr.es/m/bb4ebd0412da9b1ac87a5eb2a3646bf1@oss.nttdata.com

2 years agoUse macros from xlog_internal.h for WAL segment logic in pg_resetwal
Michael Paquier [Wed, 5 Oct 2022 05:10:13 +0000 (14:10 +0900)]
Use macros from xlog_internal.h for WAL segment logic in pg_resetwal

When scanning for the end of WAL, pg_resetwal has been maintaining its
own internal logic to calculate segment numbers and to parse a WAL
segment name for its timeline and segment number.  The code claimed for
example that XLogFromFileName() cannot be used because it lacks the
possibility of specifying a WAL segment size, which is not the case
since fc49e24, that has made the WAL segment size configurable at
initialization time, extending this routine to do so.

Similarly, this switches one segment number calculation to use
XLByteToSeg() rather than the same logic present in xlog_internal.h.
While on it, switch to TimeLineID in pg_resetwal.c for TLI numbers
parsed from segment names, to be more consistent with
XLogFromFileName().  The maths are exactly the same, but the code gets
simplified.

Author: Bharath Rupireddy
Reviewed-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/CALj2ACX+E_jnwqH_jmjhNG8BczJTNRTOLpw8K1CB1OcB48MJ8w@mail.gmail.com

2 years agoAdd a few new patterns to the tab completion of psql
Michael Paquier [Wed, 5 Oct 2022 02:46:10 +0000 (11:46 +0900)]
Add a few new patterns to the tab completion of psql

This improves the tab completion of psql on a few points:
- Provide a list of subscriptions on \dRs.
- Provide a list of publications on \dRp.
- Add CURRENT_ROLE, CURRENT_USER, SESSION_USER when OWNER TO is provided
at the end of a query (as defined by RoleSpec in gram.y).

Author: Vignesh C
Reviewed-by: Dagfinn Ilmari Mannsåker
Discussion: https://postgr.es/m/CALDaNm3toRBt6c6saY3174f7CsGztXRvVvfWbikkJEXY7x5WAA@mail.gmail.com

2 years agoFix comment in guc_tables.c
Michael Paquier [Tue, 4 Oct 2022 06:39:41 +0000 (15:39 +0900)]
Fix comment in guc_tables.c

s/ERROR_HANDLING/ERROR_HANDLING_OPTIONS/.

Author: Peter Smith
Discussion: https://postgr.es/m/CAHut+PtDj3CV+f0pVisc0XYMi2LHGBpQxQWtF0FjiSVN_nV17Q@mail.gmail.com

2 years agoCleanup useless assignments and checks
Michael Paquier [Tue, 4 Oct 2022 04:16:23 +0000 (13:16 +0900)]
Cleanup useless assignments and checks

This cleans up a couple of areas:
- Remove XLogSegNo calculation for the last WAL segment in backup in
xlog.c (7d70809 has moved this logic entirely to xlogbackup.c when
building the contents of the backup history file).
- Remove check on log_min_duration in analyze.c, as it is already true
where this code path is reached.
- Simplify call to find_option() in guc.c.

Author: Ranier Vilela
Reviewed-by: Masahiko Sawada
Discussion: https://postgr.es/m/CAEudQArCDQQiPiFR16=yu9k5s2tp4tgEe1U1ZbkW4ofx81AWWQ@mail.gmail.com

2 years agoAdd filtering capability for cross-version pg_upgrade tests
Michael Paquier [Tue, 4 Oct 2022 01:11:08 +0000 (10:11 +0900)]
Add filtering capability for cross-version pg_upgrade tests

This commit expands the TAP tests of pg_upgrade when running these with
different major versions for the "old" cluster (to-be-upgraded) and the
"new" cluster (upgraded-to), by backporting some of the buildfarm
facilities directory into the script:
- Remove comments from the dump files, avoiding version-dependent
information.
- Remove empty lines from the dump files.
- Use --extra-float-digits=0 in the pg_dump command, when using an "old"
cluster with version equal to or lower than v11.
- Use --wal-segsize and --allow-group-access in initdb only when the
"old" cluster is equal to or higher than v11.

This allows the tests to pass down to v14 with the main regression test
suite, while v9.5~13 still generate some diffs, but these are minimal
compared to what happened before this commit.  Much more could be done,
especially around dump differences with function and procedures (these
can also be avoided with direct manipulation of the dumps loaded, for
example, in a way similar to the buildfarm), but at least the basics are
in place now.

Reviewed-by: Justin Pryzby, Anton A. Melnikov
Discussion: https://postgr.es/m/[email protected]

2 years agomeson: llvm: Use llvm-config's --cxxflags when building llvmjit
Andres Freund [Mon, 3 Oct 2022 21:52:51 +0000 (14:52 -0700)]
meson: llvm: Use llvm-config's --cxxflags when building llvmjit

Otherwise we don't use LLVM's flags when building llvmjit_wrap.cpp and
llvmjit_inline.cpp. That can cause compile time failures if the C++ compiler
doesn't default to a new enough C++ standards version and link time failures
due to ABI influencing flags like -fno-rtti.

2 years agoFix psql's behavior with \g for a multiple-command string.
Tom Lane [Mon, 3 Oct 2022 19:07:10 +0000 (15:07 -0400)]
Fix psql's behavior with \g for a multiple-command string.

The pre-v15 behavior was to discard all but the last result,
but with the new behavior of printing all results by default,
we will send each such result to the \g file.  However,
we're still opening and closing the \g file for each result,
so you lose all but the last result anyway.  Move the output-file
state up to ExecQueryAndProcessResults so that we open/close the
\g file only once per command string.

To support this without changing other behavior, we must
adjust PrintQueryResult to have separate FILE * arguments
for query and status output (since status output has never
gone to the \g file).  That in turn makes it a good idea
to push the responsibility for fflush'ing output down to
PrintQueryTuples and PrintQueryStatus.

Also fix an infinite loop if COPY IN/OUT is attempted in \watch.
We used to reject that, but that error exit path got broken
somewhere along the line in v15.  There seems no real reason
to reject it anyway as the code now stands, so just remove
the error exit and make sure that COPY OUT data goes to the
right place.

Also remove PrintQueryResult's unused is_watch parameter,
and make some other cosmetic cleanups (adjust obsolete
comments, break some overly-long lines).

Daniel Vérité and Tom Lane

Discussion: https://postgr.es/m/4333844c-2244-4d6e-a49a-1d483fbe304f@manitou-mail.org

2 years agomeson: respect -Dldap=disabled
Andres Freund [Mon, 3 Oct 2022 17:13:12 +0000 (10:13 -0700)]
meson: respect -Dldap=disabled

I noticed during some manual testing that -Dldap=disabled (or
--auto-features=disabled) doesn't disable ldap if available - that's obviously
wrong.

2 years agoRevert "Optimize order of GROUP BY keys".
Tom Lane [Mon, 3 Oct 2022 14:56:16 +0000 (10:56 -0400)]
Revert "Optimize order of GROUP BY keys".

This reverts commit db0d67db2401eb6238ccc04c6407a4fd4f985832 and
several follow-on fixes.  The idea of making a cost-based choice
of the order of the sorting columns is not fundamentally unsound,
but it requires cost information and data statistics that we don't
really have.  For example, relying on procost to distinguish the
relative costs of different sort comparators is pretty pointless
so long as most such comparator functions are labeled with cost 1.0.
Moreover, estimating the number of comparisons done by Quicksort
requires more than just an estimate of the number of distinct values
in the input: you also need some idea of the sizes of the larger
groups, if you want an estimate that's good to better than a factor of
three or so.  That's data that's often unknown or not very reliable.
Worse, to arrive at estimates of the number of calls made to the
lower-order-column comparison functions, the code needs to make
estimates of the numbers of distinct values of multiple columns,
which are necessarily even less trustworthy than per-column stats.
Even if all the inputs are perfectly reliable, the cost algorithm
as-implemented cannot offer useful information about how to order
sorting columns beyond the point at which the average group size
is estimated to drop to 1.

Close inspection of the code added by db0d67db2 shows that there
are also multiple small bugs.  These could have been fixed, but
there's not much point if we don't trust the estimates to be
accurate in-principle.

Finally, the changes in cost_sort's behavior made for very large
changes (often a factor of 2 or so) in the cost estimates for all
sorting operations, not only those for multi-column GROUP BY.
That naturally changes plan choices in many situations, and there's
precious little evidence to show that the changes are for the better.
Given the above doubts about whether the new estimates are really
trustworthy, it's hard to summon much confidence that these changes
are better on the average.

Since we're hard up against the release deadline for v15, let's
revert these changes for now.  We can always try again later.

Note: in v15, I left T_PathKeyInfo in place in nodes.h even though
it's unreferenced.  Removing it would be an ABI break, and it seems
a bit late in the release cycle for that.

Discussion: https://postgr.es/m/TYAPR01MB586665EB5FB2C3807E893941F5579@TYAPR01MB5866.jpnprd01.prod.outlook.com

2 years agoAdd authentication TAP test for peer authentication
Michael Paquier [Mon, 3 Oct 2022 07:42:25 +0000 (16:42 +0900)]
Add authentication TAP test for peer authentication

This commit introduces an authentication test for the peer method, as of
a set of scenarios with and without a user name map.  The script is
automatically skipped if peer is not supported in the environment where
this test is run, checking this behavior by attempting a connection
first on a cluster up and running.

Author: Bertrand Drouvot
Discussion: https://postgr.es/m/aa60994b-1c66-ca7a-dab9-9a200dbac3d2@amazon.com

2 years agoci: enable various runtime checks on FreeBSD and macOS
Andres Freund [Sun, 2 Oct 2022 00:04:13 +0000 (17:04 -0700)]
ci: enable various runtime checks on FreeBSD and macOS

Increase likelihood of discovering problems during CI rather the buildfarm by
defining -DRELCACHE_FORCE_RELEASE -DCOPY_PARSE_PLAN_TREES
-DWRITE_READ_PARSE_PLAN_TREES -DRAW_EXPRESSION_COVERAGE_TEST on FreeBSD, and
-DRANDOMIZE_ALLOCATED_MEMORY on macOS.

FreeBSD and macoS are currently not CI bottlenecks, so we can afford to
increase their test times a bit.

Author: Justin Pryzby <[email protected]>
Discussion: https://postgr.es/m/20220910200542[email protected]

2 years agoci: macos: Reduce test concurrency
Andres Freund [Sat, 1 Oct 2022 23:55:16 +0000 (16:55 -0700)]
ci: macos: Reduce test concurrency

Test performance regresses noticably when using all cores. This is more
pronounced with meson than with autoconf, presumably because meson will
schedule the "full number" of tests more consistently.  8 seems to work
OK.

Discussion: https://postgr.es/m/20220927040208[email protected]
Backpatch: 15-, where CI was introduced

2 years agomeson: Add prefix=/usr/local/pgsql to default_options
Andres Freund [Sat, 1 Oct 2022 19:39:12 +0000 (12:39 -0700)]
meson: Add prefix=/usr/local/pgsql to default_options

autoconf set PREFIX to /usr/local/pgsql, so using the same default for meson
makes sense. The effect on windows is that installation defaults to installing
to C:/usr/local/pgsql rather than meson's default of C:/, which doesn't seem
perfect, but OK enough.

Signed-off-by: Junwang Zhao <[email protected]>
Author: Junwang Zhao <[email protected]>
Discussion: https://postgr.es/CAEG8a3LGWE-gG6vuddmH91RORhi8gWs0mMB-hcTmP3_NVgM7dg@mail.gmail.com

2 years agomeson: windows: Determine path to tmp_install + prefix using meson
Andres Freund [Sat, 1 Oct 2022 19:39:12 +0000 (12:39 -0700)]
meson: windows: Determine path to tmp_install + prefix using meson

Previously some paths (like c:\ or d:/) worked, but plenty others (like
/path/to or //computer/share/path) didn't. As we'd like to change the default
prefix to /usr/local/pgsql, that's a problem.

Instead of trying to do this in meson.build, call out to the implementation
meson install uses. This isn't pretty, but it's more reliable than what we had
before.

Discussion: https://postgr.es/CAEG8a3LGWE-gG6vuddmH91RORhi8gWs0mMB-hcTmP3_NVgM7dg@mail.gmail.com

2 years agoFix tiny memory leaks
Peter Eisentraut [Sat, 1 Oct 2022 10:48:24 +0000 (12:48 +0200)]
Fix tiny memory leaks

Both check_application_name() and check_cluster_name() use
pg_clean_ascii() but didn't release the memory.  Depending on when the
GUC is set, this might be cleaned up at some later time or it would
leak postmaster memory once.  In any case, it seems better not to have
to rely on such analysis and make the code locally robust.  Also, this
makes Valgrind happier.

Author: Masahiko Sawada <[email protected]>
Reviewed-by: Jacob Champion <[email protected]>
Discussion: https://www.postgresql.org/message-id/CAD21AoBmFNy9MPfA0UUbMubQqH3AaK5U3mrv6pSeWrwCk3LJ8g@mail.gmail.com

2 years agodoc: Fix some grammar and typos
Michael Paquier [Sat, 1 Oct 2022 06:28:02 +0000 (15:28 +0900)]
doc: Fix some grammar and typos

This fixes some areas related to logical replication and custom RMGRs.

Author: Ekaterina Kiryanova
Discussion: https://postgr.es/m/fa4773f1-1396-384a-bcd7-85b5e013f399@postgrespro.ru
Backpatch-through: 15

2 years agomeson: mingw: Add -Wl,--disable-auto-import, enable when linking with readline
Andres Freund [Wed, 28 Sep 2022 17:19:00 +0000 (10:19 -0700)]
meson: mingw: Add -Wl,--disable-auto-import, enable when linking with readline

I hadn't ported -Wl,--disable-auto-import over from the win32 template as I
had focused on msvc for windows. The flag is desirable as it makes it easier
to find problems one would have with msvc, particularly useful during cross
compilation.

This turned out to be a somewhat happy accident, as it allowed me to realize
that readline actually works on windows these days, as long as auto imports to
enable. Therefore enable auto-import again as part of linking to readline.

We perhaps can come up with a better solution for the readline issue, but this
seems good enough for now.

Discussion: http://postgr.es/m/20220928022724[email protected]

2 years agoAvoid improbable PANIC during heap_update, redux.
Tom Lane [Fri, 30 Sep 2022 23:36:46 +0000 (19:36 -0400)]
Avoid improbable PANIC during heap_update, redux.

Commit 34f581c39 intended to ensure that RelationGetBufferForTuple
would acquire a visibility-map page pin in case the otherBuffer's
all-visible bit had become set since we last had lock on that page.
But I missed a case: when we're extending the relation, VM concerns
were dealt with only in the relatively-less-likely case that we
fail to conditionally lock the otherBuffer.  I think I'd believed
that we couldn't need to worry about it if the conditional lock
succeeds, which is true for the target buffer; but the otherBuffer
was unlocked for awhile so its bit might be set anyway.  So we need
to do the GetVisibilityMapPins dance, and then also recheck the
page's free space, in both cases.

Per report from Jaime Casanova.  Back-patch to v12 as the previous
patch was (although there's still no evidence that the bug is
reachable pre-v14).

Discussion: https://postgr.es/m/[email protected]

2 years agomingw: Define PGDLLEXPORT as __declspec (dllexport) as done for msvc
Andres Freund [Wed, 28 Sep 2022 17:16:49 +0000 (10:16 -0700)]
mingw: Define PGDLLEXPORT as __declspec (dllexport) as done for msvc

While mingw would otherwise fall back to
__attribute__((visibility("default"))), that appears to only work as long as
no symbols are declared with __declspec(dllexport). But we can end up with
some, e.g. plpython's Py_Init.

It's quite possible we should do the same for cygwin, but I don't have a test
environment for that...

Discussion: http://postgr.es/m/20220928022724[email protected]
Discussion: http://postgr.es/m/20220928025242[email protected]

2 years agoAdjust PQsslAttributeNames() to match PQsslAttribute().
Tom Lane [Fri, 30 Sep 2022 14:26:47 +0000 (10:26 -0400)]
Adjust PQsslAttributeNames() to match PQsslAttribute().

Currently, PQsslAttributeNames() returns the same list of attribute
names regardless of its conn parameter.  This patch changes it to
have behavior parallel to what 80a05679d installed for PQsslAttribute:
you get OpenSSL's attributes if conn is NULL or is an SSL-encrypted
connection, or an empty list if conn is a non-encrypted connection.
The point of this is to have sensible connection-dependent behavior
in case we ever support multiple SSL libraries.  The behavior for
NULL can be defined as "the attributes for the default SSL library",
parallel to what PQsslAttribute(NULL, "library") does.

Since this is mostly just future-proofing, no back-patch.

Discussion: https://postgr.es/m/17625-fc47c78b7d71b534@postgresql.org

2 years agoFix tab-completion after commit 790bf615ddba
Alvaro Herrera [Fri, 30 Sep 2022 10:53:31 +0000 (12:53 +0200)]
Fix tab-completion after commit 790bf615ddba

I (Álvaro) broke tab-completion for GRANT .. ALL TABLES IN SCHEMA while
removing ALL from the publication syntax for schemas in the
aforementioned commit.  I also missed to update a bunch of
tab-completion rules for ALTER/CREATE PUBLICATION that match each
individual piece of ALL TABLES IN SCHEMA.  Repair those bugs.

While fixing up that commit, update a couple of outdated comments
related to the same change.

Backpatch to 15.

Author: Shi yu <[email protected]>
Reviewed-by: Peter Smith <[email protected]>
Discussion: https://postgr.es/m/OSZPR01MB6310FCE8609185A56344EED2FD559@OSZPR01MB6310.jpnprd01.prod.outlook.com

2 years agodoc: Fix PQsslAttribute docs for compression
Daniel Gustafsson [Fri, 30 Sep 2022 10:03:48 +0000 (12:03 +0200)]
doc: Fix PQsslAttribute docs for compression

The compression parameter to PQsslAttribute has never returned the
compression method used, it has always returned "on" or "off since
it was added in commit 91fa7b4719ac. Backpatch through v10.

Reviewed-by: Tom Lane <[email protected]>
Discussion: https://postgr.es/m/B9EC60EC-F665-47E8-A221-398C76E382C9@yesql.se
Backpatch-through: v10

2 years agoRemove useless argument from UnpinBuffer()
Michael Paquier [Fri, 30 Sep 2022 06:57:47 +0000 (15:57 +0900)]
Remove useless argument from UnpinBuffer()

The last caller of UnpinBuffer() that did not want to adjust
CurrentResourceOwner was removed in 2d115e4, and nothing has been
introduced in bufmgr.c to do the same thing since.  This simplifies 10
code paths.

Author: Aleksander Alekseev
Reviewed-by: Nathan Bossart, Zhang Mingli, Bharath Rupireddy
Discussion: https://postgr.es/m/CAJ7c6TOmmFpb6ohurLhTC7hKNJWGzdwf8s4EAtAZxD48g-e6Jw@mail.gmail.com

2 years agoci: Add 32bit build and test
Andres Freund [Thu, 29 Sep 2022 23:09:09 +0000 (16:09 -0700)]
ci: Add 32bit build and test

It's easy enough to make changes that break on 32bit platforms and few people
test that locally. Add a test for that to CI. LLVM is disabled on 32bit
because installing it would bloat the image size further.

The debian w/ meson task is fast enough that we can afford to test both.

Use the occasion of a separate run of the tests to run them under LANG=C,
we've recently discovered there's not a lot of testing in the buildfarm for
the case.

Discussion: https://postgr.es/m/4033181.1664395633@sss.pgh.pa.us

2 years agoFix bogus behavior of PQsslAttribute(conn, "library").
Tom Lane [Thu, 29 Sep 2022 21:28:09 +0000 (17:28 -0400)]
Fix bogus behavior of PQsslAttribute(conn, "library").

Commit ebc8b7d44 intended to change the behavior of
PQsslAttribute(NULL, "library"), but accidentally also changed
what happens with a non-NULL conn pointer.  Undo that so that
only the intended behavior change happens.  Clarify some
associated documentation.

Per bug #17625 from Heath Lord.  Back-patch to v15.

Discussion: https://postgr.es/m/17625-fc47c78b7d71b534@postgresql.org

2 years agoImprove wording of log messages triggered by max_slot_wal_keep_size.
Tom Lane [Thu, 29 Sep 2022 17:27:48 +0000 (13:27 -0400)]
Improve wording of log messages triggered by max_slot_wal_keep_size.

The one about "terminating process to release replication slot" told
you nothing about why that was happening.  The one about "invalidating
slot because its restart_lsn exceeds max_slot_wal_keep_size" told you
what was happening, but violated our message style guideline about
keeping the primary message short.  Add DETAIL/HINT lines to carry
the appropriate detail and make the two cases more uniform.

While here, fix bogus test logic in 019_replslot_limit.pl: if it timed
out without seeing the expected log message, no test failure would be
reported.  This is flat broken since commit 549ec201d removed the test
counts; even before that it was horribly bad style, since you'd only
get told that not all tests had been run.

Kyotaro Horiguchi, reviewed by Bertrand Drouvot; test fixes by me

Discussion: https://postgr.es/m/20211214.130456.2233153190058148084[email protected]

2 years agoUse actual backend IDs in pg_stat_get_backend_idset() and friends.
Tom Lane [Thu, 29 Sep 2022 16:14:39 +0000 (12:14 -0400)]
Use actual backend IDs in pg_stat_get_backend_idset() and friends.

Up to now, the ID values returned by pg_stat_get_backend_idset() and
used by pg_stat_get_backend_activity() and allied functions were just
indexes into a local array of sessions seen by the last stats refresh.
This is problematic for a few reasons.  The "ID" of a session can vary
over its existence, which is surprising.  Also, while these numbers
often match the "backend ID" used for purposes like temp schema
assignment, that isn't reliably true.  We can fairly cheaply switch
things around to make these numbers actually be the sessions' backend
IDs.  The added test case illustrates that with this definition, the
temp schema used by a given session can be obtained given its PID.

While here, delete some dead code that guarded against getting
a NULL return from pgstat_fetch_stat_local_beentry().  That can't
happen as long as the caller is careful to pass an in-range array
index, as all the callers are.  (This code may not have been dead
when written, but it surely is now.)

Nathan Bossart

Discussion: https://postgr.es/m/20220815205811.GA250990@nathanxps13

2 years agoUpdate comment in ExecInsert() regarding batch insertion.
Etsuro Fujita [Thu, 29 Sep 2022 07:55:00 +0000 (16:55 +0900)]
Update comment in ExecInsert() regarding batch insertion.

Remove the stale text that is a leftover from an earlier version of the
patch to add support for batch insertion, and adjust the wording in the
remaining text.

Back-patch to v14 where batch insertion came in.

Review and wording adjustment by Tom Lane.

Discussion: https://postgr.es/m/CAPmGK14goatHPHQv2Aeu_UTKqZ%2BBO%2BP%2Bzd3HKv5D%2BdyyfWKDSw%40mail.gmail.com

2 years agoIntroduce SYSTEM_USER
Michael Paquier [Thu, 29 Sep 2022 06:05:40 +0000 (15:05 +0900)]
Introduce SYSTEM_USER

SYSTEM_USER is a reserved keyword of the SQL specification that,
roughly described, is aimed at reporting some information about the
system user who has connected to the database server.  It may include
implementation-specific information about the means by the user
connected, like an authentication method.

This commit implements SYSTEM_USER as of auth_method:identity, where
"auth_method" is a keyword about the authentication method used to log
into the server (like peer, md5, scram-sha-256, gss, etc.) and
"identity" is the authentication identity as introduced by 9afffcb (peer
sets authn to the OS user name, gss to the user principal, etc.).  This
format has been suggested by Tom Lane.

Note that thanks to d951052, SYSTEM_USER is available to parallel
workers.

Bump catalog version.

Author: Bertrand Drouvot
Reviewed-by: Jacob Champion, Joe Conway, Álvaro Herrera, Michael Paquier
Discussion: https://postgr.es/m/7e692b8c-0b11-45db-1cad-3afc5b57409f@amazon.com

2 years agoMark sigint_interrupt_enabled as sig_atomic_t
Michael Paquier [Thu, 29 Sep 2022 05:28:13 +0000 (14:28 +0900)]
Mark sigint_interrupt_enabled as sig_atomic_t

This is a continuation of 78fdb1e, where this flag is set in the psql
callback handler used for SIGINT.  This was previously a boolean but the
C standard recommends the use of sig_atomic_t.  Note that this
influences PromptInterruptContext in string.h, where the same flag is
tracked.

Author: Hayato Kuroda
Discussion: https://postgr.es/m/TYAPR01MB58669A9EC96AA3078C2CD938F5549@TYAPR01MB5866.jpnprd01.prod.outlook.com

2 years agowindows: Set UMDF_USING_NTSTATUS globally, include ntstatus.h
Andres Freund [Thu, 29 Sep 2022 04:59:15 +0000 (21:59 -0700)]
windows: Set UMDF_USING_NTSTATUS globally, include ntstatus.h

We'd like to use precompiled headers on windows to reduce compile times. Right
now we rely on defining UMDF_USING_NTSTATUS before including postgres.h in a few
select places - which doesn't work with precompiled headers.  Instead define
it globally.

When UMDF_USING_NTSTATUS is defined we need to explicitly include ntstatus.h,
winternl.h to get a comparable set of symbols. Right now these includes would
be required in a number of non-platform-specific .c files - to avoid that,
include them in win32_port.h. Based on my measurements that doesn't increase
compile times measurably.

Reviewed-by: Thomas Munro <[email protected]>
Discussion: https://postgr.es/m/20220927011951[email protected]

2 years agomeson: Implement getopt logic from autoconf
Andres Freund [Wed, 28 Sep 2022 21:21:43 +0000 (14:21 -0700)]
meson: Implement getopt logic from autoconf

Not replacing getopt/getopt_long definitely causes issues on mingw. It's not
as clear whether the solaris & openbsd aspect is still needed, but if not, we
should remove it from both autoconf and meson.

Discussion: http://postgr.es/m/20220928022724[email protected]

2 years agomeson: mingw: Allow multiple definitions
Andres Freund [Wed, 28 Sep 2022 16:48:09 +0000 (09:48 -0700)]
meson: mingw: Allow multiple definitions

I didn't carry this forward from the win32 template. It's not needed anymore
for the reason stated therein, but it turns out to be required to
e.g. override getopt. Possibly a better solution exists, but that's for later.

Discussion: http://postgr.es/m/20220928022724[email protected]

2 years agomeson: pg_regress: Define a HOST_TUPLE sufficient to make resultmap work
Andres Freund [Tue, 27 Sep 2022 19:01:35 +0000 (12:01 -0700)]
meson: pg_regress: Define a HOST_TUPLE sufficient to make resultmap work

This doesn't end up with a triple that's exactly the same as config.guess -
it'd be hard to achieve that and it doesn't seem required. We can't rely on
config.guess as we don't necessarily have a /bin/sh on windows, e.g., when
building on windows with msvc.

This isn't perfect, e.g., clang works on windows as well.  But I suspect we'd
need a bunch of other changes to make clang on windows work, and we haven't
supported it historically.

Discussion: http://postgr.es/m/20220928022724[email protected]

2 years agomeson: windows: Normalize slashes in prefix
Andres Freund [Tue, 27 Sep 2022 18:55:00 +0000 (11:55 -0700)]
meson: windows: Normalize slashes in prefix

This fixes a build issue on windows, when the prefix is set to a path with
forward slashes. Windows python defaults to a path with a backslash, but mingw
ucrt python defaults to a forward slash. This in turn lead to a wrong PATH set
during tests, causing tests to fail.

Reported-by: Melih Mutlu <[email protected]>
Discussion: http://postgr.es/m/20220928022724[email protected]

2 years agoMap ERROR_INVALID_NAME to ENOENT in mapping table of win32error.c
Michael Paquier [Thu, 29 Sep 2022 01:14:47 +0000 (10:14 +0900)]
Map ERROR_INVALID_NAME to ENOENT in mapping table of win32error.c

This error can be reached when sending an incorrect file name to open()
on Windows, resulting in a confusing errno reported.  This has been seen
in the development of a different patch by the same author.

Author: Bharath Rupireddy
Discussion: https://postgr.es/m/CALj2ACWet-b8Juba0DiXwfGCyyOcohzwksahE5ebB9rcbLZKCQ@mail.gmail.com

2 years agoRestore pg_pread and friends.
Thomas Munro [Thu, 29 Sep 2022 00:12:11 +0000 (13:12 +1300)]
Restore pg_pread and friends.

Commits cf112c12 and a0dc8271 were a little too hasty in getting rid of
the pg_ prefixes where we use pread(), pwrite() and vectored variants.

We dropped support for ancient Unixes where we needed to use lseek() to
implement replacements for those, but it turns out that Windows also
changes the current position even when you pass in an offset to
ReadFile() and WriteFile() if the file handle is synchronous, despite
its documentation saying otherwise.

Switching to asynchronous file handles would fix that, but have other
complications.  For now let's just put back the pg_ prefix and add some
comments to highlight the non-standard side-effect, which we can now
describe as Windows-only.

Reported-by: Bharath Rupireddy <[email protected]>
Reviewed-by: Bharath Rupireddy <[email protected]>
Discussion: https://postgr.es/m/20220923202439.GA1156054%40nathanxps13

2 years agoRestrict Datum sort optimization to byval types only
David Rowley [Wed, 28 Sep 2022 22:43:00 +0000 (11:43 +1300)]
Restrict Datum sort optimization to byval types only

91e9e89dc modified nodeSort.c so that it used datum sorts when the
targetlist of the outer node contained only a single column.  That commit
failed to recognise that the Datum returned by tuplesort_getdatum() must
be pfree'd when the type is a byref type.  Ronan Dunklau did originally
propose the patch with that restriction, but that, probably through my own
fault, got lost during further development work.

Due to the timing of this report (PG15 RC1 is almost out the door), let's
just restrict the datum sort optimization to apply for byval types only.
We might want to look harder into making this work for byref types in
PG16.

Reported-by: Önder Kalacı
Diagnosis-by: Tom Lane
Discussion: https://postgr.es/m/CACawEhVxe0ufR26UcqtU7GYGRuubq3p6ZWPGXL4cxy_uexpAAQ@mail.gmail.com
Backpatch-through: 15, where 91e9e89dc was introduced.

2 years agodoc: clarify internal behavior of RECURSIVE CTE queries
Bruce Momjian [Wed, 28 Sep 2022 17:14:38 +0000 (13:14 -0400)]
doc: clarify internal behavior of RECURSIVE CTE queries

Reported-by: Tom Lane
Discussion: https://postgr.es/m/3976627.1662651004@sss.pgh.pa.us

Backpatch-through: 10

2 years agorevert "warn of SECURITY DEFINER schemas for non-sql_body funcs"
Bruce Momjian [Wed, 28 Sep 2022 17:05:21 +0000 (13:05 -0400)]
revert "warn of SECURITY DEFINER schemas for non-sql_body funcs"

doc revert of commit 1703726488.  Change was applied to irrelevant
branches, and was not detailed enough to be helpful in relevant
branches.

Reported-by: Peter Eisentraut, Noah Misch
Discussion: https://postgr.es/m/a2dc9de4-24fc-3222-87d3-0def8057d7d8@enterprisedb.com

Backpatch-through: 10

2 years agoDoc: document bpchar, clarify relationship of text and varchar.
Tom Lane [Wed, 28 Sep 2022 16:31:36 +0000 (12:31 -0400)]
Doc: document bpchar, clarify relationship of text and varchar.

For some reason the "bpchar" type name was defined nowhere in
our SGML docs, although several places refer to it in passing.
Give it a proper mention under Character Types.

While here, also provide an explanation of how the text and varchar
types relate.  The previous wording seemed to be doing its best
to sweep text under the rug, which doesn't seem very appropriate
given its prominence in other parts of the docs.

Minor rearrangements and word-smithing for clarity, too.

Laurenz Albe and Tom Lane, per gripe from Yanliang Lei

Discussion: https://postgr.es/m/120b3084.56b6.1833b5ffe4b[email protected]

2 years agoAllow callback functions to deregister themselves during a call.
Tom Lane [Wed, 28 Sep 2022 15:23:14 +0000 (11:23 -0400)]
Allow callback functions to deregister themselves during a call.

Fetch the next-item pointer before the call not after, so that
we aren't dereferencing a dangling pointer if the callback
deregistered itself during the call.  The risky coding pattern
appears in CallXactCallbacks, CallSubXactCallbacks, and
ResourceOwnerReleaseInternal.  (There are some other places that
might be at hazard if they offered deregistration functionality,
but they don't.)

I (tgl) considered back-patching this, but desisted because it
wouldn't be very safe for extensions to rely on this working in
pre-v16 branches.

Hao Wu

Discussion: https://postgr.es/m/CAH+9SWXTiERkmhRke+QCcc+jRH8d5fFHTxh8ZK0-Yn4BSpyaAg@mail.gmail.com

2 years agoChange some errdetail() to errdetail_internal()
Alvaro Herrera [Wed, 28 Sep 2022 15:14:53 +0000 (17:14 +0200)]
Change some errdetail() to errdetail_internal()

This prevents marking the argument string for translation for gettext,
and it also prevents the given string (which is already translated) from
being translated at runtime.

Also, mark the strings used as arguments to check_rolespec_name for
translation.

Backpatch all the way back as appropriate.  None of this is caught by
any tests (necessarily so), so I verified it manually.

2 years agoFix bug in DROP OWNED BY.
Robert Haas [Wed, 28 Sep 2022 14:42:07 +0000 (10:42 -0400)]
Fix bug in DROP OWNED BY.

Commit 6566133c5f52771198aca07ed18f84519fac1be7 broke the case where
the role passed to DROP OWNED BY owns a database.

Report by Rushabh Lathia, who also provided a patch, but this patch
takes a slightly different approach to fixing the problem.

Discussion: http://postgr.es/m/CAGPqQf2vO+nbo=3yAdZ8v26Rbug7bY4YjPaPLZx=L1NZ9-CC3w@mail.gmail.com

2 years agoRevert 56-bit relfilenode change and follow-up commits.
Robert Haas [Wed, 28 Sep 2022 13:45:27 +0000 (09:45 -0400)]
Revert 56-bit relfilenode change and follow-up commits.

There are still some alignment-related failures in the buildfarm,
which might or might not be able to be fixed quickly, but I've also
just realized that it increased the size of many WAL records by 4 bytes
because a block reference contains a RelFileLocator. The effect of that
hasn't been studied or discussed, so revert for now.

2 years agoFix InitializeRelfilenumberMap for 05d4cbf9b6ba708858984b01ca0fc56d59d4ec7c
Robert Haas [Wed, 28 Sep 2022 12:02:30 +0000 (08:02 -0400)]
Fix InitializeRelfilenumberMap for 05d4cbf9b6ba708858984b01ca0fc56d59d4ec7c

Since relfilenodes are now 56-bits, we use bigint as the SQL type
to represent them, which means F_INT8EQ must be used here rather
than F_OIDEQ. On 64-bit machines this doesn't matter, but 32-bit
machines are unhappy.

Dilip Kumar

Discussion: http://postgr.es/m/CAFiTN-t71ciSckMzixAhrF9py7oRO6xszKi4mTRwjuucXr5tpw@mail.gmail.com

2 years agoFix alignment problems with SharedInvalSmgrMsg.
Robert Haas [Wed, 28 Sep 2022 11:51:48 +0000 (07:51 -0400)]
Fix alignment problems with SharedInvalSmgrMsg.

SharedInvalSmgrMsg can't require 8-byte alignment, because then
SharedInvalidationMessage will require 8-byte alignment, which will
then cause ParseCommitRecord to fail on machines that are picky
about alignment, because it assumes that everything that gets
packed into a commit record requires only 4-byte alignment.

Another problem with 05d4cbf9b6ba708858984b01ca0fc56d59d4ec7c.

Discussion: http://postgr.es/m/3825454.1664310917@sss.pgh.pa.us

2 years agoRemove publicationcmds.c's expr_allowed_in_node as a function
Alvaro Herrera [Wed, 28 Sep 2022 11:47:25 +0000 (13:47 +0200)]
Remove publicationcmds.c's expr_allowed_in_node as a function

Its API is quite strange, and since there's only one caller, there's no
reason for it to be a separate function in the first place.  Inline it
instead.

Discussion: https://postgr.es/m/20220927124249[email protected]

2 years agoFix some comments of do_pg_backup_start() and do_pg_backup_stop()
Michael Paquier [Wed, 28 Sep 2022 00:58:44 +0000 (09:58 +0900)]
Fix some comments of do_pg_backup_start() and do_pg_backup_stop()

Both functions referred to an incorrect variable name, so make the whole
more consistent.

Oversight in 7d70809.

Author: Kyotaro Horiguchi, Bharath Rupireddy
Discussion: https://postgr.es/m/20220927.172427.467118514018439476[email protected]

2 years agoIn BufTagGetForkNum, cast to the correct type.
Robert Haas [Tue, 27 Sep 2022 20:12:43 +0000 (16:12 -0400)]
In BufTagGetForkNum, cast to the correct type.

Another defect in 05d4cbf9b6ba708858984b01ca0fc56d59d4ec7c.

Per CI, via Justin Pryzby.

Discussion: http://postgr.es/m/20220927200712[email protected]

2 years agoUpdate pg_buffercache's meson.build.
Robert Haas [Tue, 27 Sep 2022 19:31:36 +0000 (15:31 -0400)]
Update pg_buffercache's meson.build.

Commit 05d4cbf9b6ba708858984b01ca0fc56d59d4ec7c needed to do this,
but didn't.

Per Justin Pryzby.

Discussion: 20220927191710[email protected]

2 years agoFix typos in commit 05d4cbf9b6ba708858984b01ca0fc56d59d4ec7c.
Robert Haas [Tue, 27 Sep 2022 19:13:34 +0000 (15:13 -0400)]
Fix typos in commit 05d4cbf9b6ba708858984b01ca0fc56d59d4ec7c.

Reported by Justin Pryzby.

Discussion: http://postgr.es/m/20220927185121[email protected]

2 years agoConvert *GetDatum() and DatumGet*() macros to inline functions
Peter Eisentraut [Tue, 27 Sep 2022 18:47:07 +0000 (20:47 +0200)]
Convert *GetDatum() and DatumGet*() macros to inline functions

The previous macro implementations just cast the argument to a target
type but did not check whether the input type was appropriate.  The
function implementation can do better type checking of the input type.

For the *GetDatumFast() macros, converting to an inline function
doesn't work in the !USE_FLOAT8_BYVAL case, but we can use
AssertVariableIsOfTypeMacro() to get a similar level of type checking.

Reviewed-by: Aleksander Alekseev <[email protected]>
Reviewed-by: Tom Lane <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/8528fb7e-0aa2-6b54-85fb-0c0886dbd6ed%40enterprisedb.com

2 years agoInclude common/relpath.h in utils/relfilenumbermap.h
Robert Haas [Tue, 27 Sep 2022 17:34:23 +0000 (13:34 -0400)]
Include common/relpath.h in utils/relfilenumbermap.h

Buildfarm member crake ran headerscheck, which complained about
a missing include here.

Defect introduced by commit 2f47715cc8649f854b1df28dfc338af9801db217.

2 years agoIncrease width of RelFileNumbers from 32 bits to 56 bits.
Robert Haas [Tue, 27 Sep 2022 17:25:21 +0000 (13:25 -0400)]
Increase width of RelFileNumbers from 32 bits to 56 bits.

RelFileNumbers are now assigned using a separate counter, instead of
being assigned from the OID counter. This counter never wraps around:
if all 2^56 possible RelFileNumbers are used, an internal error
occurs. As the cluster is limited to 2^64 total bytes of WAL, this
limitation should not cause a problem in practice.

If the counter were 64 bits wide rather than 56 bits wide, we would
need to increase the width of the BufferTag, which might adversely
impact buffer lookup performance. Also, this lets us use bigint for
pg_class.relfilenode and other places where these values are exposed
at the SQL level without worrying about overflow.

This should remove the need to keep "tombstone" files around until
the next checkpoint when relations are removed. We do that to keep
RelFileNumbers from being recycled, but now that won't happen
anyway. However, this patch doesn't actually change anything in
this area; it just makes it possible for a future patch to do so.

Dilip Kumar, based on an idea from Andres Freund, who also reviewed
some earlier versions of the patch. Further review and some
wordsmithing by me. Also reviewed at various points by Ashutosh
Sharma, Vignesh C, Amul Sul, Álvaro Herrera, and Tom Lane.

Discussion: http://postgr.es/m/CA+Tgmobp7+7kmi4gkq7Y+4AM9fTvL+O1oQ4-5gFTT+6Ng-dQ=g@mail.gmail.com

2 years agoMove RelFileNumber declarations to common/relpath.h.
Robert Haas [Tue, 27 Sep 2022 16:01:57 +0000 (12:01 -0400)]
Move RelFileNumber declarations to common/relpath.h.

Previously, these were declared in postgres_ext.h, but they are not
needed nearly so widely as the OID declarations, so that doesn't
necessarily make sense. Also, because postgres_ext.h is included
before most of c.h has been processed, the previous location creates
some problems for a pending patch.

Patch by me, reviewed by Dilip Kumar.

Discussion: http://postgr.es/m/CA+TgmoYc8oevMqRokZQ4y_6aRn-7XQny1JBr5DyWR_jiFtONHw@mail.gmail.com

2 years agoRenumber GUC flags for a bit more sanity.
Tom Lane [Tue, 27 Sep 2022 15:51:06 +0000 (11:51 -0400)]
Renumber GUC flags for a bit more sanity.

Push the units fields over to the left so that all the single-bit
flags can be together.  I considered rearranging the single-bit
flags to try to group flags with similar purposes, but eventually
decided that that involved too many judgment calls.

Discussion: https://postgr.es/m/17385-9ee529fb091f0ce5@postgresql.org

2 years agoIntroduce GUC_NO_RESET flag.
Tom Lane [Tue, 27 Sep 2022 15:47:12 +0000 (11:47 -0400)]
Introduce GUC_NO_RESET flag.

Previously, the transaction-property GUCs such as transaction_isolation
could be reset after starting a transaction, because we marked them
as GUC_NO_RESET_ALL but still allowed a targeted RESET.  That leads to
assertion failures or worse, because those properties aren't supposed
to change after we've acquired a transaction snapshot.

There are some NO_RESET_ALL variables for which RESET is okay, so
we can't just redefine the semantics of that flag.  Instead introduce
a separate GUC_NO_RESET flag.  Mark "seed", as well as the transaction
property GUCs, as GUC_NO_RESET.

We have to disallow GUC_ACTION_SAVE as well as straight RESET, because
otherwise a function having a "SET transaction_isolation" clause can
still break things: the end-of-function restore action is equivalent
to a RESET.

No back-patch, as it's conceivable that someone is doing something
this patch will forbid (like resetting one of these GUCs at transaction
start, or "CREATE FUNCTION ... SET transaction_read_only = 1") and not
running into problems with it today.  Given how long we've had this
issue and not noticed, the side effects in non-assert builds can't be
too serious.

Per bug #17385 from Andrew Bille.

Masahiko Sawada

Discussion: https://postgr.es/m/17385-9ee529fb091f0ce5@postgresql.org

2 years agoImprove some publication-related error messages
Alvaro Herrera [Tue, 27 Sep 2022 12:11:31 +0000 (14:11 +0200)]
Improve some publication-related error messages

While at it, remove an unused queryString parameter from
CheckPubRelationColumnList() and make other minor stylistic changes.

Backpatch to 15.

Reported by Kyotaro Horiguchi <[email protected]>
Co-authored-by: Hou zj <[email protected]>
Discussion: https://postgr.es/m/20220926.160426.454497059203258582[email protected]

2 years agoFix pg_stat_statements for MERGE
Alvaro Herrera [Tue, 27 Sep 2022 08:44:42 +0000 (10:44 +0200)]
Fix pg_stat_statements for MERGE

We weren't jumbling the merge action list, so wildly different commands
would be considered to use the same query ID.  Add that, mention it in
the docs, and some test lines.

Backpatch to 15.

Author: Tatsu <[email protected]>
Reviewed-by: Julien Rouhaud <[email protected]>
Discussion: https://postgr.es/m/d87e391694db75a038abc3b2597828e8@oss.nttdata.com

2 years agoci: Add hint about downloadable logs to README
Andres Freund [Tue, 27 Sep 2022 03:02:26 +0000 (20:02 -0700)]
ci: Add hint about downloadable logs to README

I (Andres) chose to backpatch this to 15, as it seems better to keep the
README the same.

Author: James Coleman <[email protected]>
Discussion: https://postgr.es/m/CAAaqYe_7BXDjpk0Ks_eqf1r6LZpC_rfB7kjhb_T3+eC4t6yiGQ@mail.gmail.com
Backpatch: 15-, where CI came in

2 years agomeson: Set up absolute rpaths to libdir
Andres Freund [Tue, 27 Sep 2022 02:44:05 +0000 (19:44 -0700)]
meson: Set up absolute rpaths to libdir

While I (Andres) had planned to use relative rpaths, it looks like agreeing on
the precise path might take a bit. So set up absolute rpaths for now.

I'm pushing this fairly quickly after posting the patch as several hackers
fought with this issue.

Discussion: https://postgr.es/m/20220927011951[email protected]

2 years agomeson: Include CFLAGS/c_args in summary and pg_config output
Andres Freund [Tue, 27 Sep 2022 02:36:24 +0000 (19:36 -0700)]
meson: Include CFLAGS/c_args in summary and pg_config output

Previously arguments passed in via CFLAGS/-Dc_args were neither displayed in
meson's summary, nor in pg_config's output.

Reported-by: "[email protected]" <[email protected]>
Discussion: https://postgr.es/m/OS3PR01MB62751847BC9CD2DB7B29AC129E529@OS3PR01MB6275.jpnprd01.prod.outlook.com

2 years agoMark ParallelMessagePending as sig_atomic_t
Michael Paquier [Tue, 27 Sep 2022 00:29:56 +0000 (09:29 +0900)]
Mark ParallelMessagePending as sig_atomic_t

ParallelMessagePending was previously marked as a boolean which should
be fine on modern platforms, but the C standard recommends the use of
sig_atomic_t for variables manipulated in signal handlers.

Author: Hayato Kuroda
Discussion: https://postgr.es/m/TYAPR01MB58667C15A95A234720F4F876F5529@TYAPR01MB5866.jpnprd01.prod.outlook.com

2 years agoRemove dependency to StringInfo in xlogbackup.{c.h}
Michael Paquier [Tue, 27 Sep 2022 00:15:07 +0000 (09:15 +0900)]
Remove dependency to StringInfo in xlogbackup.{c.h}

This was used as the returned result type of the generated contents for
the backup_label and backup history files.  This is replaced by a simple
string, reducing the cleanup burden of all the callers of
build_backup_content().

Reviewed-by: Bharath Rupireddy
Discussion: https://postgr.es/m/[email protected]

2 years agowindows: remove date from version number in win32ver.rc
Andres Freund [Mon, 26 Sep 2022 18:38:02 +0000 (11:38 -0700)]
windows: remove date from version number in win32ver.rc

This may have served a purpose at some point, but these days it just
contributes to a non-reproducible build.

Reviewed-by: Peter Eisentraut <[email protected]>
Discussion: https://postgr.es/m/c5736f70-bb6d-8d25-e35c-e3d886e4e905@enterprisedb.com
Discussion: https://postgr.es/m/1cef5b48-32bd-5cbf-fb62-fb648860f5ef@enterprisedb.com

2 years agoDoc: further adjust notes about pg_upgrade_output.d.
Tom Lane [Mon, 26 Sep 2022 18:19:21 +0000 (14:19 -0400)]
Doc: further adjust notes about pg_upgrade_output.d.

I'd misunderstood how it worked in 5f1048881.

Discussion: https://postgr.es/m/20220925215009[email protected]

2 years agoEnable WRITE_READ_PARSE_PLAN_TREES of rewritten utility statements
Tom Lane [Mon, 26 Sep 2022 14:32:16 +0000 (16:32 +0200)]
Enable WRITE_READ_PARSE_PLAN_TREES of rewritten utility statements

This was previously disabled because we lacked outfuncs/readfuncs
support for most utility statement types.

Reviewed-by: Tom Lane <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/4159834.1657405226@sss.pgh.pa.us

2 years agoImplement WRITE_READ_PARSE_PLAN_TREES for raw parse trees
Tom Lane [Mon, 26 Sep 2022 14:32:16 +0000 (16:32 +0200)]
Implement WRITE_READ_PARSE_PLAN_TREES for raw parse trees

Reviewed-by: Tom Lane <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/4159834.1657405226@sss.pgh.pa.us

2 years agoDon't lose precision for float fields of Nodes.
Peter Eisentraut [Mon, 26 Sep 2022 14:02:09 +0000 (16:02 +0200)]
Don't lose precision for float fields of Nodes.

Historically we've been more worried about making the output of
float fields look pretty than whether they'd be read back exactly.
That won't work if we're to compare the read-back nodes for
equality, so switch to using the Ryu code for float output.

Author: Tom Lane <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/4159834.1657405226@sss.pgh.pa.us

2 years agocatversion bump
Peter Eisentraut [Mon, 26 Sep 2022 13:56:47 +0000 (15:56 +0200)]
catversion bump

for 8999f5ed3cd7d26be1121d912086d04d134d398b

2 years agoFix write/read of empty string fields in Nodes.
Peter Eisentraut [Mon, 26 Sep 2022 13:25:59 +0000 (15:25 +0200)]
Fix write/read of empty string fields in Nodes.

Historically, outToken has represented both NULL and empty-string
strings as "<>", which readfuncs.c then read as NULL, thus failing
to preserve empty-string fields accurately.  Remarkably, this has
not caused any serious problems yet, but let's fix it.

We'll keep the "<>" notation for NULL, and use """" for empty string,
because that matches other notational choices already in use.
An actual input string of """" is converted to "\""" (this was true
already, apparently as a hangover from an ancient time when string
quoting was handled directly by pg_strtok).

CHAR fields also use "<>", but for '\0'.

Author: Tom Lane <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/4159834.1657405226@sss.pgh.pa.us

2 years agoRemove unused xid parameter.
Amit Kapila [Mon, 26 Sep 2022 03:17:00 +0000 (08:47 +0530)]
Remove unused xid parameter.

Commit 6c2003f8a1 removes the use of transaction id's for exporting
snapshots. This commit removes one unused xid parameter left behind in
SnapBuildGetOrBuildSnapshot.

Author: Melih Mutlu
Reviewed-By: Zhang Mingli
Discussion: https://postgr.es/m/CAGPVpCTqZRoDKgCycw+eYi+Gq41rN9pU-gntgTd7wfsNDpPL3Q@mail.gmail.com

2 years agoRefactor creation of backup_label and backup history files
Michael Paquier [Mon, 26 Sep 2022 02:15:47 +0000 (11:15 +0900)]
Refactor creation of backup_label and backup history files

This change simplifies some of the logic related to the generation and
creation of the backup_label and backup history files, which has become
unnecessarily complicated since the removal of the exclusive backup mode
in commit 39969e2.  The code was previously generating the contents of
these files as a string (start phase for the backup_label and stop phase
for the backup history file), one problem being that the contents of the
backup_label string were scanned to grab some of its internal contents
at the stop phase.

This commit changes the logic so as we store the data required to build
these files in an intermediate structure named BackupState.  The
backup_label file and backup history file strings are generated when
they are ready to be sent back to the client.  Both files are now
generated with the same code path.  While on it, this commit renames
some variables for clarity.

Two new files named xlogbackup.{c,h} are introduced in this commit, to
remove from xlog.c some of the logic around base backups.  Note that
more could be moved to this new set of files.

Author: Bharath Rupireddy, Michael Paquier
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/CALj2ACXWwTDgJqCjdaPyfR7djwm6SrybGcrZyrvojzcsmt4FFw@mail.gmail.com

2 years agoFix tupdesc lifespan bug with AfterTriggersTableData.storeslot.
Tom Lane [Sun, 25 Sep 2022 21:10:58 +0000 (17:10 -0400)]
Fix tupdesc lifespan bug with AfterTriggersTableData.storeslot.

Commit 25936fd46 adjusted things so that the "storeslot" we use
for remapping trigger tuples would have adequate lifespan, but it
neglected to consider the lifespan of the tuple descriptor that
the slot depends on.  It turns out that in at least some cases, the
tupdesc we are passing is a refcounted tupdesc, and the refcount for
the slot's reference can get assigned to a resource owner having
different lifespan than the slot does.  That leads to an error like
"tupdesc reference 0x7fdef236a1b8 is not owned by resource owner
SubTransaction".  Worse, because of a second oversight in the same
commit, we'd try to free the same tupdesc refcount again while
cleaning up after that error, leading to recursive errors and an
"ERRORDATA_STACK_SIZE exceeded" PANIC.

To fix the initial problem, let's just make a non-refcounted copy
of the tupdesc we're supposed to use.  That seems likely to guard
against additional problems, since there's no strong reason for
this code to assume that what it's given is a refcounted tupdesc;
in which case there's an independent hazard of the tupdesc having
shorter lifespan than the slot does.  (I didn't bother trying to
free said copy, since it should go away anyway when the (sub)
transaction context is cleaned up.)

The other issue can be fixed by making the code added to
AfterTriggerFreeQuery work like the rest of that function, ie be
sure that it doesn't try to free the same slot twice in the event
of recursive error cleanup.

While here, also clean up minor stylistic issues in the test case
added by 25936fd46: don't use "create or replace function", as any
name collision within the tests is likely to have ill effects
that that won't mask; and don't use function names as generic as
trigger_function1, especially if you're not going to drop them
at the end of the test stanza.

Per bug #17607 from Thomas Mc Kay.  Back-patch to v12, as the
previous fix was.

Discussion: https://postgr.es/m/17607-bd8ccc81226f7f80@postgresql.org

2 years agoAvoid loss of code coverage with unlogged-index test cases.
Tom Lane [Sun, 25 Sep 2022 17:10:10 +0000 (13:10 -0400)]
Avoid loss of code coverage with unlogged-index test cases.

Commit 4fb5c794e intended to add coverage of some ambuildempty
methods that were not getting reached, without removing any
test coverage.  However, by changing a temp table to unlogged
it managed to negate the intent of 4c51a2d1e, which means that
we didn't have reliable test coverage of ginvacuum.c anymore.
As things stand, much of that file might or might not get reached
depending on timing, which seems pretty undesirable.

Although this is only clearly broken for the GIN test, it seems
best to revert 4fb5c794e altogether and instead add bespoke test
cases covering unlogged indexes for these four AMs.  We don't
need to do very much with them, so the extra tests are cheap.
(Note that btree, hash, and bloom already have similar test cases,
so they need no additional work.)

We can also undo dec8ad367.  Since the testing deficiency that that
hacked around was later fixed by 2f2e24d90, let's intentionally leave
an unlogged table behind to improve test coverage in the modules that
use the regression database for other test purposes.  (The case I used
also leaves an unlogged sequence behind.)

Per report from Alex Kozhemyakin.  Back-patch to v15 where the
faulty test came in.

Discussion: https://postgr.es/m/b00c8ee096ee46cd25c183125562a1a7@postgrespro.ru

2 years agoAdd missing source files to pg_waldump/nls.mk
Alvaro Herrera [Sun, 25 Sep 2022 15:48:03 +0000 (17:48 +0200)]
Add missing source files to pg_waldump/nls.mk

2 years agoMessage style improvements
Peter Eisentraut [Sat, 24 Sep 2022 22:38:35 +0000 (18:38 -0400)]
Message style improvements

2 years agoAdd read support for some missing raw parse nodes
Peter Eisentraut [Sat, 24 Sep 2022 22:18:33 +0000 (18:18 -0400)]
Add read support for some missing raw parse nodes

The node types A_Const, Constraint, and A_Expr had custom output
functions, but no read functions were implemented so far.

The A_Expr output format had to be tweaked a bit to make it easier to
parse.

Be a bit more cautious about applying strncmp to unterminated strings.

Also error out if an unrecognized enum value is found in each case,
instead of just printing a placeholder value.  That was maybe ok for
debugging but won't work if we want to have robust round-tripping.

Reviewed-by: Tom Lane <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/4159834.1657405226@sss.pgh.pa.us

2 years agoFix reading of BitString nodes
Peter Eisentraut [Sat, 24 Sep 2022 22:10:52 +0000 (18:10 -0400)]
Fix reading of BitString nodes

The node tokenizer went out of its way to store BitString node values
without the leading 'b'.  But everything else in the system stores the
leading 'b'.  This would break if a BitString node is
read-printed-read.

Also, the node tokenizer didn't know that BitString node tokens could
also start with 'x'.

Reviewed-by: Tom Lane <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/4159834.1657405226@sss.pgh.pa.us

2 years agoFix reading of most-negative integer value nodes
Peter Eisentraut [Sat, 24 Sep 2022 22:10:11 +0000 (18:10 -0400)]
Fix reading of most-negative integer value nodes

The main parser checks whether a literal fits into an int when
deciding whether it should be put into an Integer or Float node.  The
parser processes integer literals without signs.  So a most-negative
integer literal will not fit into Integer and will end up as a Float
node.

The node tokenizer did this differently.  It included the sign when
checking whether the literal fit into int.  So a most-negative integer
would indeed fit that way and end up as an Integer node.

In order to preserve the node structure correctly, we need the node
tokenizer to also analyze integer literals without sign.

There are a number of test cases in the regression tests that have a
most-negative integer argument of some utility statement, so this
issue is easily reproduced under WRITE_READ_PARSE_PLAN_TREES.

Reviewed-by: Tom Lane <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/4159834.1657405226@sss.pgh.pa.us

2 years agoRemove uses of register due to incompatibility with C++17 and up
Andres Freund [Sat, 24 Sep 2022 19:01:06 +0000 (12:01 -0700)]
Remove uses of register due to incompatibility with C++17 and up

The use in regexec.c could remain, since we only try to keep headers C++
clean. But there really doesn't seem to be a good reason to use register in
that spot.

Discussion: https://postgr.es/m/20220308185902[email protected]