Commit Graph

23205 Commits

Author SHA1 Message Date
Tom Lane 0f47457f11 Remove our artificial PG_SOMAXCONN limit on listen queue length.
I added this in commit 153f40067, out of paranoia about kernels
possibly rejecting very large listen backlog requests.  However,
POSIX has said for decades that the kernel must silently reduce
any value it considers too large, and there's no evidence that
any current system doesn't obey that.  Let's just drop this limit
and save some complication.

While we're here, compute the request as twice MaxConnections not
twice MaxBackends; the latter no longer means what it did in 2001.

Per discussion of a report from Kevin McKibbin.

Discussion: https://postgr.es/m/CADc_NKg2d+oZY9mg4DdQdoUcGzN2kOYXBu-3--RW_hEe0tUV=g@mail.gmail.com
2022-08-23 10:15:06 -04:00
Tom Lane 4ee6740167 Doc: prefer sysctl to /proc/sys in docs and comments.
sysctl is more portable than Linux's /proc/sys file tree, and
often easier to use too.  That's why most of our docs refer to
sysctl when talking about how to adjust kernel parameters.
Bring the few stragglers into line.

Discussion: https://postgr.es/m/361175.1661187463@sss.pgh.pa.us
2022-08-23 10:15:06 -04:00
Amit Kapila f972ec5c28 Add CHECK_FOR_INTERRUPTS while decoding changes.
While decoding changes in a loop, if we skip all the changes there is no
CFI making the loop uninterruptible.

Reported-by: Whale Song and Andrey Borodin
Bug: 17580
Author: Masahiko Sawada
Reviwed-by: Amit Kapila
Backpatch-through: 10
Discussion: https://postgr.es/m/17580-849c1d5b6d7eb422@postgresql.org
Discussion: https://postgr.es/m/B319ECD6-9A28-4CDF-A8F4-3591E0BF2369@yandex-team.ru
2022-08-23 10:20:02 +05:30
Andres Freund 1bdd54e662 Remove redundant call to pgstat_report_wal()
pgstat_report_stat() will be called before shutdown so an explicit call to
pgstat_report_wal() just before shutdown is redundant.

This likely was not redundant before 5891c7a8ed, but now it clearly is.

Author: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/CAAKRu_aaq33UnG4TXq3S-OSXGWj1QGf0sU%2BECH4tNwGFNERkZA%40mail.gmail.com
2022-08-22 20:25:42 -07:00
Andres Freund 0c679464a8 Add BackendType for standalone backends
All backends should have a BackendType to enable statistics reporting
per BackendType.

Add a new BackendType for standalone backends, B_STANDALONE_BACKEND (and
alphabetize the BackendTypes). Both the bootstrap backend and single
user mode backends will have BackendType B_STANDALONE_BACKEND.

Author: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/CAAKRu_aaq33UnG4TXq3S-OSXGWj1QGf0sU%2BECH4tNwGFNERkZA%40mail.gmail.com
2022-08-22 20:22:50 -07:00
Andres Freund cd063344fb pgstat: Acquire lock when reading variable-numbered stats
Somewhere during the development of the patch acquiring a lock during read
access to variable-numbered stats got lost. The missing lock acquisition won't
cause corruption, but can lead to reading torn values when accessing
stats. Add the missing lock acquisitions.

Reported-by: Greg Stark <stark@mit.edu>
Reviewed-by: "Drouvot, Bertrand" <bdrouvot@amazon.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://postgr.es/m/CAM-w4HMYkM_DkYhWtUGV+qE_rrBxKOzOF0+5faozxO3vXrc9wA@mail.gmail.com
Backpatch: 15-
2022-08-22 20:16:50 -07:00
John Naylor ba8321349b Switch format specifier for replication origins to %d
Using %u with uint16 causes warnings with -Wformat-signedness. There are many
other warnings, but for now change only these since c920fe4818 already changed
the message string for most of them.

Per report from Peter Eisentraut
Discussion: https://www.postgresql.org/message-id/31e63649-0355-7088-831e-b07d5f908a8c%40enterprisedb.com
2022-08-23 09:55:05 +07:00
John Naylor 1b9050da66 Remove empty statement
Peter Smith

Discussion: https://www.postgresql.org/message-id/CAHut%2BPtRGVuj8Q_GpHHxZyk7fGwdYDG8_s4GSfKoc_4Yd9vR-w%40mail.gmail.com
2022-08-23 09:24:32 +07:00
Robert Haas ce6b672e44 Make role grant system more consistent with other privileges.
Previously, membership of role A in role B could be recorded in the
catalog tables only once. This meant that a new grant of role A to
role B would overwrite the previous grant. For other object types, a
new grant of permission on an object - in this case role A - exists
along side the existing grant provided that the grantor is different.
Either grant can be revoked independently of the other, and
permissions remain so long as at least one grant remains. Make role
grants work similarly.

Previously, when granting membership in a role, the superuser could
specify any role whatsoever as the grantor, but for other object types,
the grantor of record must be either the owner of the object, or a
role that currently has privileges to perform a similar GRANT.
Implement the same scheme for role grants, treating the bootstrap
superuser as the role owner since roles do not have owners. This means
that attempting to revoke a grant, or admin option on a grant, can now
fail if there are dependent privileges, and that CASCADE can be used
to revoke these. It also means that you can't grant ADMIN OPTION on
a role back to a user who granted it directly or indirectly to you,
similar to how you can't give WITH GRANT OPTION on a privilege back
to a role which granted it directly or indirectly to you.

Previously, only the superuser could specify GRANTED BY with a user
other than the current user. Relax that rule to allow the grantor
to be any role whose privileges the current user posseses. This
doesn't improve compatibility with what we do for other object types,
where support for GRANTED BY is entirely vestigial, but it makes this
feature more usable and seems to make sense to change at the same time
we're changing related behaviors.

Along the way, fix "ALTER GROUP group_name ADD USER user_name" to
require the same privileges as "GRANT group_name TO user_name".
Previously, CREATEROLE privileges were sufficient for either, but
only the former form was permissible with ADMIN OPTION on the role.
Now, either CREATEROLE or ADMIN OPTION on the role suffices for
either spelling.

Patch by me, reviewed by Stephen Frost.

Discussion: http://postgr.es/m/CA+TgmoaFr-RZeQ+WoQ5nKPv97oT9+aDgK_a5+qWHSgbDsMp1Vg@mail.gmail.com
2022-08-22 11:35:17 -04:00
Peter Eisentraut 36f729e2bc Fix assertion failure in CREATE DATABASE
An assertion would fail when creating a database with libc locale
provider from a template database with icu locale provider.

Reported-by: Marina Polyakova <m.polyakova@postgrespro.ru>
Reviewed-by: Julien Rouhaud <rjuju123@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/f385ba25e7f8be427b8c582e5cca7d79%40postgrespro.ru#515a31c5429d6d37ad1d5c9d66962a1e
2022-08-22 15:38:41 +02:00
Amit Kapila 838f798f17 Use logical operator && instead of & in vacuumparallel.c.
As such the current usage of & won't produce incorrect results but it
would be better to use && to short-circuit the evaluation of second
condition when the same is not required.

Author: Ranier Vilela
Reviewed-by: Tom Lane, Bharath Rupireddy
Backpatch-through: 15, where it was introduced
Discussion: https://postgr.es/m/CAEudQApL8QcoYwQuutkWKY_h7gBY8F0Xs34YKfc7-G0i83K_pw@mail.gmail.com
2022-08-22 08:53:58 +05:30
David Rowley f01592f915 Remove shadowed local variables that are new in v15
Compiling with -Wshadow=compatible-local yields quite a few warnings about
local variables being shadowed by compatible local variables in an inner
scope.  Of course, this is perfectly valid in C, but we have had bugs in
the past as a result of developers failing to notice this.  af7d270dd is a
recent example.

Here we do a cleanup of warnings we receive from -Wshadow=compatible-local
for code which is new to PostgreSQL 15.  We've yet to have the discussion
about if we actually ever want to run that as a standard compilation flag.
We'll need to at least get the number of warnings down to something easier
to manage before we can realistically consider if we want this or not.
This commit is the first step towards reducing the warnings.

The changes being made here are all fairly trivial.  Because of that, and
the fact that v15 is still in beta, this is being back-patched into 15.
It seems more risky not to do this as the risk of future bugs is increased
by the additional conflicts that this commit could cause for any future
bug fixes touching the same areas as this commit.

Author: Justin Pryzby
Discussion: https://postgr.es/m/20220817145434.GC26426%40telsasoft.com
Backpatch-through: 15
2022-08-20 11:40:44 +12:00
Peter Geoghegan 3097bde7dd Avoid reltuples distortion in very small tables.
Consistently avoid trusting a sample of only one page at the point that
VACUUM determines a new reltuples for the target table (though only when
the table is larger than a single page).  This is follow-up work to
commit 74388a1a, which added a heuristic to prevent reltuples from
becoming distorted by successive VACUUM operations that each scan only a
single heap page (which was itself more or less a bugfix for an issue in
commit 44fa8488, which simplified VACUUM's handling of scanned pages).

The original bugfix commit did not account for certain remaining cases
that where not affected by its "2% of total relpages" heuristic.  This
happened with relations that are small enough that just one of its pages
exceeded the 2% threshold, yet still big enough for VACUUM to deem
skipping most of its pages via the visibility map worthwhile.  reltuples
could still become distorted over time with such a table, at least in
scenarios where the VACUUM command is run repeatedly and without the
table itself ever changing.

Author: Peter Geoghegan <pg@bowt.ie>
Discussion: https://postgr.es/m/CAH2-Wzk7d4m3oEbEWkWQKd+gz-eD_peBvdXVk1a_KBygXadFeg@mail.gmail.com
Backpatch: 15-, where the rules for scanned pages changed.
2022-08-19 09:26:08 -07:00
Peter Geoghegan 662ba729a6 Initialize index stats during parallel VACUUM.
Initialize shared memory allocated for index stats to avoid a hard
crash.  This was possible when parallel VACUUM became confused about the
current phase of index processing.

Oversight in commit 8e1fae1938, which refactored parallel VACUUM.

Author: Masahiko Sawada <sawada.mshk@gmail.com>
Reported-By: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/20220818133406.GL26426@telsasoft.com
Backpatch: 15-, the first version with the refactoring commit.
2022-08-18 17:34:14 -07:00
Robert Haas 6566133c5f Ensure that pg_auth_members.grantor is always valid.
Previously, "GRANT foo TO bar" or "GRANT foo TO bar GRANTED BY baz"
would record the OID of the grantor in pg_auth_members.grantor, but
that role could later be dropped without modifying or removing the
pg_auth_members record. That's not great, because we typically try
to avoid dangling references in catalog data.

Now, a role grant depends on the grantor, and the grantor can't be
dropped without removing the grant or changing the grantor.  "DROP
OWNED BY" will remove the grant, just as it does for other kinds of
privileges. "REASSIGN OWNED BY" will not, again just like what we do
in other cases involving privileges.

pg_auth_members now has an OID column, because that is needed in order
for dependencies to work. It also now has an index on the grantor
column, because otherwise dropping a role would require a sequential
scan of the entire table to see whether the role's OID is in use as
a grantor. That probably wouldn't be too large a problem in practice,
but it seems better to have an index just in case.

A follow-on patch is planned with the goal of more thoroughly
rationalizing the behavior of role grants. This patch is just trying
to do enough to make sure that the data we store in the catalogs is at
some basic level valid.

Patch by me, reviewed by Stephen Frost

Discussion: http://postgr.es/m/CA+TgmoaFr-RZeQ+WoQ5nKPv97oT9+aDgK_a5+qWHSgbDsMp1Vg@mail.gmail.com
2022-08-18 13:13:02 -04:00
Tom Lane 2f17b57017 Improve performance of adjust_appendrel_attrs_multilevel.
The present implementations of adjust_appendrel_attrs_multilevel and
its sibling adjust_child_relids_multilevel are very messy, because
they work by reconstructing the relids of the child's immediate
parent and then seeing if that's bms_equal to the relids of the
target parent.  Aside from being quite inefficient, this will not
work with planned future changes to make joinrels' relid sets
contain outer-join relids in addition to baserels.

The whole thing can be solved at a stroke by adding explicit parent
and top_parent links to child RelOptInfos, and making these functions
work with RelOptInfo pointers instead of relids.  Doing that is
simpler for most callers, too.

In my original version of this patch, I got rid of
RelOptInfo.top_parent_relids on the grounds that it was now redundant.
However, that adds a lot of code churn in places that otherwise would
not need changing, and arguably the extra indirection needed to fetch
top_parent->relids in those places costs something.  So this version
leaves that field in place.

Discussion: https://postgr.es/m/553080.1657481916@sss.pgh.pa.us
2022-08-18 12:36:16 -04:00
Robert Haas ec97db399f Adjust assertion in XLogDecodeNextRecord.
As written, if you use XLogBeginRead() to position an xlogreader at
the beginning of a WAL page and then try to read WAL, this assertion
will fail. However, the header comment for XLogBeginRead() claims
that positioning an xlogreader at the beginning of a page is valid,
and the code here is perfectly able to cope with it. It's only the
assertion that causes trouble. So relax it.

This is formally a bug in all supported branches, but as it doesn't
seem to have any consequences for current uses of the xlogreader
facility, no back-patch, at least for now.

Dilip Kumar and Robert Haas

Discussion: http://postgr.es/m/CA+TgmoaJSs2_7WHW2GzFYe9+zfPtxBKvT3GW47+x=ptUE=cULw@mail.gmail.com
2022-08-18 12:22:20 -04:00
Tom Lane e6dbb48487 Fix subtly-incorrect matching of parent and child partitioned indexes.
When creating a partitioned index, DefineIndex tries to identify
any existing indexes on the partitions that match the partitioned
index, so that it can absorb those as child indexes instead of
building new ones.  Part of the matching is to compare IndexInfo
structs --- but that wasn't done quite right.  We're comparing
the IndexInfo built within DefineIndex itself to one made from
existing catalog contents by BuildIndexInfo.  Notably, while
BuildIndexInfo will run index expressions and predicates through
expression preprocessing, that has not happened to DefineIndex's
struct.  The result is failure to match and subsequent creation
of duplicate indexes.

The easiest and most bulletproof fix is to build a new IndexInfo
using BuildIndexInfo, thereby guaranteeing that the processing done
is identical.

While here, let's also extract the opfamily and collation data
from the new partitioned index, removing ad-hoc logic that
duplicated knowledge about how those are constructed.

Per report from Christophe Pettus.  Back-patch to v11 where
we invented partitioned indexes.

Richard Guo and Tom Lane

Discussion: https://postgr.es/m/8864BFAA-81FD-4BF9-8E06-7DEB8D4164ED@thebuild.com
2022-08-18 12:12:03 -04:00
Robert Haas 3e63e8462f When using the WAL-logged CREATE DATABASE strategy, bulk extend.
This should improve performance, and was suggested by Andres Freund.
Back-patch to v15 to keep the code consistent across branches.

Dilip Kumar

Discussion: http://postgr.es/m/C3458199-FEDD-4356-865A-08DFAA5D4065@anarazel.de
Discussion: http://postgr.es/m/CAFiTN-sJ0vVpJrZ=R5M+g7Tr8=NN4wKOtrqOcDEsfFfnZgivVA@mail.gmail.com
2022-08-18 11:26:34 -04:00
Peter Eisentraut 08909e3aee Simplify and clarify an error message 2022-08-18 11:36:55 +02:00
Thomas Munro f340f97a13 mstcpip.h is not missing on MinGW.
Remove a small difference between MinGW and MSVC builds which isn't
needed for modern MinGW, noticed in passing.

Discussion: https://postgr.es/m/CA+hUKGKErNfhmvb_H0UprEmp4LPzGN06yR2_0tYikjzB-2ECMw@mail.gmail.com
2022-08-18 16:31:11 +12:00
Thomas Munro 2492fe49dc Remove configure probe for netinet/tcp.h.
<netinet/tcp.h> is in SUSv3 and all targeted Unix systems have it.
For Windows, we can provide a stub include file, to avoid some #ifdef
noise.

Discussion: https://postgr.es/m/CA+hUKGKErNfhmvb_H0UprEmp4LPzGN06yR2_0tYikjzB-2ECMw@mail.gmail.com
2022-08-18 16:31:11 +12:00
Thomas Munro 2cea02fb85 Remove configure probe for sys/sockio.h.
On BSD-family systems, header <sys/sockio.h> defines socket ioctl
numbers like SIOCGIFCONF.  Only AIX is using those now, but it defines
them in <net/if.h> anyway.

Supposing some PostgreSQL hacker wants to test that AIX-only code path
on a more common development system by pretending not to have
getifaddrs().  It's enough to include <sys/ioctl.h>, at least on macOS,
FreeBSD and Linux, and we're already doing that.
2022-08-18 16:31:11 +12:00
Thomas Munro 2f8d918359 Remove configure probe for net/if.h.
<net/if.h> is in SUSv3 and all targeted Unixes have it.  It's used in a
region that is already ifdef'd out for Windows.  We're not using it for
any standard definitions, but it's where AIX defines conventional socket
ioctl numbers.

Discussion: https://postgr.es/m/CA+hUKGKErNfhmvb_H0UprEmp4LPzGN06yR2_0tYikjzB-2ECMw@mail.gmail.com
2022-08-18 16:31:11 +12:00
Thomas Munro a717cddcac Remove dead ifaddr.c fallback code.
We carried a special implementation of pg_foreach_ifaddr() using
Solaris's ioctl(SIOCGLIFCONF), but Solaris 11 and illumos adopted
getifaddrs() more than a decade ago, and we prefer to use that.  Solaris
10 is EOL'd.  Remove the dead code.

Adjust comment about which OSes have getifaddrs(), which also
incorrectly listed AIX.  AIX is in fact the only Unix in the build farm
that *doesn't* have it today, so the implementation based on
ioctl(SIOCGIFCONF) (note, no 'L') is still live.  All the others have
had it for at least one but mostly two decades.

The last-stop fallback at the bottom of the file is dead code in
practice, but it's hard to justify removing it because the better
options are all non-standard.

Discussion: https://postgr.es/m/CA+hUKGKErNfhmvb_H0UprEmp4LPzGN06yR2_0tYikjzB-2ECMw@mail.gmail.com
2022-08-18 16:28:52 +12:00
John Naylor c920fe4818 Refer to replication origin roident as "ID" in user facing messages and docs
The table column that stores this is of type oid, but is actually limited
to uint16 and has a different path for creating new values. Some of
the documentation already referred to it as an ID, so let's standardize
on that.

While at it, most format strings already use %u, so for consintency
change the remaining stragglers using %d.

Per suggestions from Tom Lane and Justin Pryzby
Discussion: https://www.postgresql.org/message-id/3437166.1659620465%40sss.pgh.pa.us
Backpatch to v15
2022-08-18 08:57:13 +07:00
David Rowley af7d270dd3 Fix hypothetical problem passing the wrong GROUP BY pathkeys
1349d2790 changed things to make the planner request that the
query_pathkeys contain pathkeys for any ORDER BY / DISTINCT aggregates.
Some code added prior to that commit in db0d67db2 made it so the order
that the pathkeys appear in the group_pathkeys could be changed so that
the GROUP BY could be executed in a more optimal order which minimized
sort comparisons.  1349d2790 had to make sure that the pathkeys for any
ORDER BY / DISTINCT aggregates remained at the end of the groupby_pathkeys
and wasn't reordered, so some code was added to
add_paths_to_grouping_rel() to first strip off any pathkeys belonging to
ORDER BY / DISTINCT aggregates before passing to the function to optimize
the order of the group_pathkeys.

It seems I dropped the ball in 1349d2790 and mistakenly used the untouched
PlannerInfo.group_pathkeys to pass to get_useful_group_keys_orderings()
instead of the version that had the aggregate pathkeys removed.  It was
only the code path that was handling creating paths for
partially_grouped_rel which made this mistake.  In practice, we'll never
have any extra pathkeys to strip off when processing
partially_grouped_rel as that's only used when considering partial
paths, which we never do when there are ORDER BY / DISTINCT aggregates.
So this is just a hypothetical bug, not a live bug.  We already have the
correct pathkeys determined, so it's of no extra cost to pass the
correct variable.

Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/20220817015755.GB26426@telsasoft.com
2022-08-18 11:32:55 +12:00
Tom Lane afa0ec30bf Refactor addition of PlaceHolderVars to joinrel targetlists.
Make build_joinrel_tlist() responsible for adding PHVs that were
already computed in one or the other input relation, and therefore
change add_placeholders_to_joinrel() to only add PHVs that will be
newly computed in this joinrel's output.  This makes the handling
of PHVs in build_joinrel_tlist() more like its handling of plain
Vars, which seems like a good thing on intelligibility grounds
and will simplify planned future changes.  There is a purely
cosmetic side-effect that the order of entries in the joinrel's
tlist may change; but since it becomes more like the order of
entries in the input tlists, that's not bad.

The reason it wasn't done like this originally was the potential
cost of looking up PlaceHolderInfo entries to consult ph_needed.
Now that that's O(1) it shouldn't hurt.

Discussion: https://postgr.es/m/1405792.1660677844@sss.pgh.pa.us
2022-08-17 16:12:23 -04:00
Tom Lane b3ff6c742f Use an explicit state flag to control PlaceHolderInfo creation.
Up to now, callers of find_placeholder_info() were required to pass
a flag indicating if it's OK to make a new PlaceHolderInfo.  That'd
be fine if the callers had free choice, but they do not.  Once we
begin deconstruct_jointree() it's no longer OK to make more PHIs;
while callers before that always want to create a PHI if it's not
there already.  So there's no freedom of action, only the opportunity
to cause bugs by creating PHIs too late.  Let's get rid of that in
favor of adding a state flag PlannerInfo.placeholdersFrozen, which
we can set at the point where it's no longer OK to make more PHIs.

This patch also simplifies a couple of call sites that were using
complicated logic to avoid calling find_placeholder_info() as much
as possible.  Now that that lookup is O(1) thanks to the previous
commit, the extra bitmap manipulations are probably a net negative.

Discussion: https://postgr.es/m/1405792.1660677844@sss.pgh.pa.us
2022-08-17 15:52:53 -04:00
Tom Lane 6569ca4397 Make PlaceHolderInfo lookup O(1).
Up to now we've just searched the placeholder_list when we want to
find the PlaceHolderInfo with a given ID.  While there's no evidence
of that being a problem in the field, an upcoming patch will add
find_placeholder_info() calls in build_joinrel_tlist(), which seems
likely to make it more of an issue: a joinrel emitting lots of
PlaceHolderVars would incur O(N^2) cost, and we might be building
a lot of joinrels in complex queries.  Hence, add an array that
can be indexed directly by phid to make the lookups constant-time.

Discussion: https://postgr.es/m/1405792.1660677844@sss.pgh.pa.us
2022-08-17 15:35:51 -04:00
Tom Lane efd0c16bec Avoid using list_length() to test for empty list.
The standard way to check for list emptiness is to compare the
List pointer to NIL; our list code goes out of its way to ensure
that that is the only representation of an empty list.  (An
acceptable alternative is a plain boolean test for non-null
pointer, but explicit mention of NIL is usually preferable.)

Various places didn't get that memo and expressed the condition
with list_length(), which might not be so bad except that there
were such a variety of ways to check it exactly: equal to zero,
less than or equal to zero, less than one, yadda yadda.  In the
name of code readability, let's standardize all those spellings
as "list == NIL" or "list != NIL".  (There's probably some
microscopic efficiency gain too, though few of these look to be
at all performance-critical.)

A very small number of cases were left as-is because they seemed
more consistent with other adjacent list_length tests that way.

Peter Smith, with bikeshedding from a number of us

Discussion: https://postgr.es/m/CAHut+PtQYe+ENX5KrONMfugf0q6NHg4hR5dAhqEXEc2eefFeig@mail.gmail.com
2022-08-17 11:12:35 -04:00
Michael Paquier d265cd2029 Use SetInstallXLogFileSegmentActive() in more places in xlog.c
This reduces the code paths where XLogCtl->InstallXLogFileSegmentActive
is directly touched, and this wrapper function does the same thing as
the original code replaced by the function call.

Author: Bharath Rupireddy
Reviewed-by: Nathan Bossart
Discussion: https://postgr.es/m/CALj2ACVhkf-bC5CX-=6iBUfkO5GqmBntQH+m=HpY0iQ=-g1pRg@mail.gmail.com
2022-08-17 15:28:45 +09:00
Tomas Vondra c52ad9c4ef Fix assert in logicalmsg_desc
The assert, introduced by 9f1cf97bb5, is intended to check if the prefix
is terminated by a \0 byte, but it has two flaws. Firstly, prefix_size
includes the \0 byte, so prefix[prefix_size] points to the byte after
the null byte. Secondly, the check ensures the byte is not equal \0,
while it should be checking the opposite.

Backpatch-through: 14
Discussion: https://postgr.es/m/b99b6101-2f14-3796-3dfa-4a6cd7d4326d@enterprisedb.com
2022-08-16 23:52:10 +02:00
Amit Kapila 0d5bd3a6cc Fix replica identity check for a partitioned table.
The current publisher code checks if UPDATE or DELETE can be executed with
the replica identity of the table even if it's a partitioned table. We can
skip checking the replica identity for partitioned tables because the
operations are actually performed on the leaf partitions (not the
partitioned table).

Reported-by: Brad Nicholson
Author: Hou Zhijie
Reviewed-by: Peter Smith, Amit Kapila
Backpatch-through: 13
Discussion: https://postgr.es/m/CAMMnM%3D8i5DohH%3DYKzV0_wYuYSYvuOJoL9F5nzXTc%2ByzsG1f6rg%40mail.gmail.com
2022-08-16 15:25:41 +05:30
Peter Eisentraut 1c5818b9c6 Remove redundant spaces in _outA_Expr() output
Since WRITE_NODE_FIELD() output always starts with a space, we don't
need to go out of our way to print another space right before it.

This change is only for visual appearance; the tokenizer on the
reading side would read it the same way (but there is no read support
for A_Expr at this time anyway).
2022-08-15 12:43:52 +02:00
Michael Paquier f2108d3bd0 Fix outdated --help message for postgres -f
This option switch supports a total of 8 values, as told by
set_plan_disabling_options() and the documentation, but this was not
reflected in the output generated by --help.

Author: Junwang Zhao
Discussion: https://postgr.es/m/CAEG8a3+pT3cWzyjzKs184L1XMNm8NDnoJLiSjAYSO7XqpRh_vA@mail.gmail.com
Backpatch-through: 10
2022-08-15 13:36:36 +09:00
Tom Lane a466219428 Preserve memory context of VarStringSortSupport buffers.
When enlarging the work buffers of a VarStringSortSupport object,
varstrfastcmp_locale was careful to keep them in the ssup_cxt
memory context; but varstr_abbrev_convert just used palloc().
The latter creates a hazard that the buffers could be freed out
from under the VarStringSortSupport object, resulting in stomping
on whatever gets allocated in that memory later.

In practice, because we only use this code for ICU collations
(cf. 3df9c374e), the problem is confined to use of ICU collations.
I believe it may have been unreachable before the introduction
of incremental sort, too, as traditional sorting usually just
uses one context for the duration of the sort.

We could fix this by making the broken stanzas in varstr_abbrev_convert
match the non-broken ones in varstrfastcmp_locale.  However, it seems
like a better idea to dodge the issue altogether by replacing the
pfree-and-allocate-anew coding with repalloc, which automatically
preserves the chunk's memory context.  This fix does add a few cycles
because repalloc will copy the chunk's content, which the existing
coding assumes is useless.  However, we don't expect that these buffer
enlargement operations are performance-critical.  Besides that, it's
far from obvious that copying the buffer contents isn't required, since
these stanzas make no effort to mark the buffers invalid by resetting
last_returned, cache_blob, etc.  That seems to be safe upon examination,
but it's fragile and could easily get broken in future, which wouldn't
get revealed in testing with short-to-moderate-size strings.

Per bug #17584 from James Inform.  Whether or not the issue is
reachable in the older branches, this code has been broken on its
own terms from its introduction, so patch all the way back.

Discussion: https://postgr.es/m/17584-95c79b4a7d771f44@postgresql.org
2022-08-14 12:05:27 -04:00
Thomas Munro 5579388d2d Remove replacement code for getaddrinfo.
SUSv3, all targeted Unixes and modern Windows have getaddrinfo() and
related interfaces.  Drop the replacement implementation, and adjust
some headers slightly to make sure that the APIs are visible everywhere
using standard POSIX headers and names.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CA%2BhUKG%2BL_3brvh%3D8e0BW_VfX9h7MtwgN%3DnFHP5o7X2oZucY9dg%40mail.gmail.com
2022-08-14 09:53:28 +12:00
Tom Lane 55d9cd46f6 Avoid misbehavior when hash_table_bytes < bucket_size.
It's possible to reach this case when work_mem is very small and tupsize
is (relatively) very large.  In that case ExecChooseHashTableSize would
get an assertion failure, or with asserts off it'd compute nbuckets = 0,
which'd likely cause misbehavior later (I've not checked).  To fix,
clamp the number of buckets to be at least 1.

This is due to faulty conversion of old my_log2() coding in 28d936031.
Back-patch to v13, as that was.

Zhang Mingli

Discussion: https://postgr.es/m/beb64ca0-91e2-44ac-bf4a-7ea36275ec02@Spark
2022-08-13 17:00:32 -04:00
Thomas Munro f558088285 Remove HAVE_UNIX_SOCKETS.
Since HAVE_UNIX_SOCKETS is now defined unconditionally, remove the macro
and drop a small amount of dead code.

The last known systems not to have them (as far as I know at least) were
QNX, which we de-supported years ago, and Windows, which now has them.

If a new OS ever shows up with the POSIX sockets API but without working
AF_UNIX, it'll presumably still be able to compile the code, and fail at
runtime with an unsupported address family error.  We might want to
consider adding a HINT that you should turn off the option to use it if
your network stack doesn't support it at that point, but it doesn't seem
worth making the relevant code conditional at compile time.

Also adjust a couple of places in the docs and comments that referred to
builds without Unix-domain sockets, since there aren't any.  Windows
still gets a special mention in those places, though, because we don't
try to use them by default there yet.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Discussion: https://postgr.es/m/CA%2BhUKG%2BL_3brvh%3D8e0BW_VfX9h7MtwgN%3DnFHP5o7X2oZucY9dg%40mail.gmail.com
2022-08-14 08:46:53 +12:00
Tom Lane e07ebd4b6e Catch stack overflow when recursing in transformFromClauseItem().
Most parts of the parser can expect that the stack overflow check
in transformExprRecurse() will trigger before things get desperate.
However, transformFromClauseItem() can recurse directly to self
without having analyzed any expressions, so it's possible to drive
it to a stack-overrun crash.  Add a check to prevent that.

Per bug #17583 from Egor Chindyaskin.  Back-patch to all supported
branches.

Richard Guo

Discussion: https://postgr.es/m/17583-33be55b9f981f75c@postgresql.org
2022-08-13 15:21:28 -04:00
Thomas Munro 36b3d52459 Remove configure probe for sys/resource.h and refactor.
<sys/resource.h> is in SUSv2 and is on all targeted Unix systems.  We
have a replacement for getrusage() on Windows, so let's just move its
declarations into src/include/port/win32/sys/resource.h so that we can
use a standard-looking #include.  Also remove an obsolete reference to
CLK_TCK.  Also rename src/port/getrusage.c to win32getrusage.c,
following the convention for Windows-only fallback code.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CA%2BhUKG%2BL_3brvh%3D8e0BW_VfX9h7MtwgN%3DnFHP5o7X2oZucY9dg%40mail.gmail.com
2022-08-14 00:09:47 +12:00
Thomas Munro 37a65d1db1 Remove configure probes for sys/ipc.h, sys/sem.h, sys/shm.h.
These are in SUSv2 and every targeted Unix system has them.  It's not
hard to avoid including them on Windows system because they're mostly
used in platform-specific translation units.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CA%2BhUKG%2BL_3brvh%3D8e0BW_VfX9h7MtwgN%3DnFHP5o7X2oZucY9dg%40mail.gmail.com
2022-08-14 00:09:47 +12:00
Thomas Munro 7e50b4e3c5 Remove configure probe for sys/select.h.
<sys/select.h> is in SUSv3 and every targeted Unix system has it.
Provide an empty header in src/include/port/win32 so that we can
include it unguarded even on Windows.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CA%2BhUKG%2BL_3brvh%3D8e0BW_VfX9h7MtwgN%3DnFHP5o7X2oZucY9dg%40mail.gmail.com
2022-08-14 00:09:47 +12:00
Peter Eisentraut abf46ad9c7 Add missing fields to _outConstraint()
As of 897795240c, check constraints can
be declared invalid.  But that patch didn't update _outConstraint() to
also show the relevant struct fields (which were only applicable to
foreign keys before that).  This currently only affects debugging
output, so no impact in practice.
2022-08-13 10:32:38 +02:00
Robert Haas 76733b399c Avoid using a fake relcache entry to own an SmgrRelation.
If an error occurs before we close the fake relcache entry, the the
fake relcache entry will be destroyed by the SmgrRelation will
survive until end of transaction. Its smgr_owner pointer ends up
pointing to already-freed memory.

The original reason for using a fake relcache entry here was to try
to avoid reusing an SMgrRelation across a relevant invalidation. To
avoid that problem, just call smgropen() again each time we need a
reference to it. Hopefully someday we will come up with a more
elegant approach, but accessing uninitialized memory is bad so let's
do this for now.

Dilip Kumar, reviewed by Andres Freund and Tom Lane. Report by
Justin Pryzby.

Discussion: http://postgr.es/m/20220802175043.GA13682@telsasoft.com
Discussion: http://postgr.es/m/CAFiTN-vSFeE6_W9z698XNtFROOA_nSqUXWqLcG0emob_kJ+dEQ@mail.gmail.com
2022-08-12 08:25:41 -04:00
Alvaro Herrera 92af9143f1
Reject MERGE in CTEs and COPY
The grammar added for MERGE inadvertently made it accepted syntax in
places that were not prepared to deal with it -- namely COPY and inside
CTEs, but invoking these things with MERGE currently causes assertion
failures or weird misbehavior in non-assertion builds.  Protect those
places by checking for it explicitly until somebody decides to implement
it.

Reported-by: Alexey Borzov <borz_off@cs.msu.su>
Discussion: https://postgr.es/m/17579-82482cd7b267b862@postgresql.org
2022-08-12 12:05:50 +02:00
Peter Eisentraut e7a552f303 Fix _outConstraint() for "identity" constraints
The set of fields printed by _outConstraint() in the CONSTR_IDENTITY
case didn't match the set of fields actually used in that case.  (The
code was probably uncarefully copied from the CONSTR_DEFAULT case.)
Fix that by using the right set of fields.  Since there is no read
support for this node type, this is really just for debugging output
right now, so it doesn't affect anything important.
2022-08-12 08:17:30 +02:00
Robert Haas 34dffa0224 Fix non-specific error message.
"something has gone wrong" is not helpful. Make this elog()
consistent with the other one in the same function.

Discussion: http://postgr.es/m/CA+Tgmoa_AZ2jUWSA_noiqOqnxBaWDR+t3bHjSygZi6+wqDBCXQ@mail.gmail.com
2022-08-11 14:12:11 -04:00
Alvaro Herrera 2c86077765
struct PQWalReceiverFunctions: use designated initializers
We now require that compilers support this, and it makes the code easier
to trace, so change it.  I'm fixated on this particular struct because
I've had to navigate around it a number of times, but there are others
elsewhere that could use the same treatment.

Discussion: https://postgr.es/m/20220810140300.ixhbmm4svo5yypv6@alvherre.pgsql
2022-08-11 12:07:05 +02:00