validate_exec() didn't guarantee to set errno to something appropriate
after a failure, leading to callers not being able to print an on-point
message. Improve that.
Noted by Kyotaro Horiguchi, though this isn't exactly his proposal.
Discussion: https://postgr.es/m/20220615.131403.1791191615590453058.horikyota.ntt@gmail.com
pg_upgrade does not use common/logging.c, which is unfortunate
but changing it to do so seems like more work than is justified.
However, we really need to make it work more like common/logging.c
in one respect: the latter expects supplied message strings to not
end with a newline, instead adding one internally. As it stands,
pg_upgrade's logging facilities expect a caller-supplied newline
in some cases and not others, which is already an invitation to bugs,
but the inconsistency with our other frontend code makes it worse.
There are already several places with missing or extra newlines,
and it's inevitable that there won't be more if we let this stand.
Hence, run around and get rid of all trailing newlines in message
strings, and add an Assert that there's not one, similar to the
existing Assert in common/logging.c. Adjust the logging functions
to supply a newline at the right places.
(Some of these strings also have a *leading* newline, which would
be a good thing to get rid of too; but this patch doesn't attempt
that.)
There are some consequent minor changes in output. The ones that
aren't outright bug fixes are generally removal of extra blank
lines that the original coding intentionally inserted. It didn't
seem worth being bug-compatible with that.
Patch by me, reviewed by Kyotaro Horiguchi and Peter Eisentraut
Discussion: https://postgr.es/m/113191.1655233060@sss.pgh.pa.us
Having different build systems producing different contents of the
NodeTag enum would be catastrophic for extension ABI stability.
But that ordering depends on the order in which gen_node_support.pl
processes its input files. It seems too fragile to let the Makefiles,
MSVC build scripts, and soon meson build scripts all set this order
independently. As a klugy but serviceable solution, put a canonical
copy of the file list into gen_node_support.pl itself, and check that
against the files given on the command line.
Also, while it's fine to add and delete node tags during development,
we must not let the assigned NodeTag values change unexpectedly in
stable branches. Add a cross-check that can be enabled when a branch
is forked off (or later, but that is a time when we're unlikely to
miss doing it). It just checks that the last auto-assigned number
doesn't change, which is simplistic but will catch the most likely
sorts of mistakes.
From time to time we do need to add a node tag in a stable branch.
To support doing that without changing the branch's auto-assigned
tag numbers, invent pg_node_attr(nodetag_number(VALUE)) which can
be used to give such a node a hand-assigned tag above the last
auto-assigned one.
Discussion: https://postgr.es/m/1249010.1657574337@sss.pgh.pa.us
This allows explaining gen_node_support.pl's handling of execnodes.h
and some other input files as being a shortcut for explicit marking
of all their node declarations as pg_node_attr(nodetag_only).
I foresee that someday we might need to be more fine-grained about
that, and this change provides the infrastructure needed to do so.
For now, it just allows removal of the script's klugy special case
for CallContext and InlineCodeBlock.
Discussion: https://postgr.es/m/75063.1657410615@sss.pgh.pa.us
Commit f10a025cfe97 added support for List to store Xids, but didn't
handle the new type in all cases. Add some obviously necessary pieces.
As far as I am aware, this is all dead code as far as core code is
concerned, but it seems unacceptable not to have it in case third-party
code wants to rely on this type of list. (Some parts of the List API
remain unimplemented, but that can be fixed as and when needed -- see
lack of list_intersection_oid, list_deduplicate_int as precedents.)
Discussion: https://postgr.es/m/20220708164534.nbejhgt4ajz35p65@alvherre.pgsql
Commit 3838fa269 added a lookahead loop to allow building strings multiple
bytes at a time. This loop could exit because it reached the end of input,
yet did not check for that before checking if we reached the end of a
valid string. To fix, put the end of string check back in the outer loop.
Per Valgrind animal skink
Now some foreign data wrappers support TRUNCATE command.
So it's useful to support TRUNCATE triggers on foreign tables for
audit logging or for preventing undesired truncation.
Author: Yugo Nagata
Reviewed-by: Fujii Masao, Ian Lawrence Barwick
Discussion: https://postgr.es/m/20220630193848.5b02e0d6076b86617a915682@sraoss.co.jp
PostgreSQL/POSTGRES has run on a huge range of CPUs and OSes. As we're
dropping some of the earliest systems the project was founded on, let's
provide a place to remember them.
Discussion: https://postgr.es/m/959917.1657522169%40sss.pgh.pa.us
Further to commit 92d70b77, let's drop the code we carry for the
following untested architectures: M68K, M88K, M32R, SuperH. We have no
idea if anything actually works there, and surely as vintage hardware
and microcontrollers they would be underpowered for modern purposes.
We could always consider re-adding SuperH based on evidence of usage and
build farm support, if someone shows up to provide it.
While here, SPARC is usually written in all caps.
Suggested-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Robert Haas <robertmhaas@gmail.com> (the idea, not the patch)
Discussion: https://postgr.es/m/959917.1657522169%40sss.pgh.pa.us
Refactor so that log_line_prefix() is a thin wrapper over a new
function log_status_format(), and move the implementation to the
latter. Export log_status_format() so that it can be used by an
emit_log_hook.
Discussion: https://postgr.es/m/39c8197652f4d3050aedafae79fa5af31096505f.camel%40j-davis.com
Reviewed-by: Michael Paquier, Alvaro Herrera
Remove PageIsValid() and PageSizeIsValid(), which weren't used and
seem unnecessary.
Some code using these formerly-macros needs some adjustments because
it was previously playing loose with the Page vs. PageHeader types,
which is no longer possible with the functions instead of macros.
Reviewed-by: Amul Sul <sulamul@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/5b558da8-99fb-0a99-83dd-f72f05388517%40enterprisedb.com
dshash.c previously maintained flags to be able to assert that you
didn't hold any partition lock. These flags could get out of sync with
reality in error scenarios.
Get rid of all that, and make assertions about the locks themselves
instead. Since LWLockHeldByMe() loops internally, we don't want to put
that inside another loop over all partition locks. Introduce a new
debugging-only interface LWLockAnyHeldByMe() to avoid that.
This problem was noted by Tom and Andres while reviewing changes to
support the new shared memory stats system, and later showed up in
reality while working on commit 389869af.
Back-patch to 11, where dshash.c arrived.
Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Reported-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>
Reviewed-by: Zhihong Yu <zyu@yugabyte.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/20220311012712.botrpsikaufzteyt@alap3.anarazel.de
Discussion: https://postgr.es/m/CA%2BhUKGJ31Wce6HJ7xnVTKWjFUWQZPBngxfJVx4q0E98pDr3kAw%40mail.gmail.com
This addresses two issues in the tests of test_oat_hooks:
- The role regress_test_user was being left behind, preventing the test
to succeed on repeated runs. It makes sense to leave some objects
behind to have more coverage for pg_upgrade (as does test_pg_dump), but
the role dropped here does not own any objects so there is no reason to
keep it.
- GRANT SET ON PARAMETER is issued, creating an entry in
pg_parameter_acl without cleaning up the entry created. This causes
an overlap with unsafe_tests as both use work_mem, making the latter
fail. This commit adds an extra REVOKE SET ON PARAMETER to clean the
contents of pg_parameter_acl, switching to maintenance_work_mem rather
than work_mem to avoid an overlap between both tests.
The tests of test_oat_hooks cannot use installcheck yet as these are
proving to be unstable with caching and the namespace search hooks, so
the issues fixed here cannot be reached yet, but they would be once the
hook issue is addressed and installcheck is allowed again in
test_oat_hooks.
Discussion: https://postgr.es/m/YrpVkADAY0knF6vM@paquier.xyz
Backpatch-through: 15
The original comments mentioned a "parameter" as something not defined
in a fast-exit path to assume a true status. This is rather confusing
as the parameter DefElem is defined, and the intention is to check if
its value is defined. This improves both comments to mention the value
assigned to the DefElem's value instead, so as future patches are able
to catch the tweak if this code pattern gets copied around more.
Author: Peter Smith
Discussion: https://postgr.es/m/CAHut+Pv0yWynWTmp4o34s0d98xVubys9fy=p0YXsZ5_sUcNnMw@mail.gmail.com
* Remove arbitrary mention of certain endianness and bitness variants;
it's enough to say that applicable variants are expected to work.
* List RISC-V (known to work, being tested).
* List SuperH and M88K (code exists, unknown status, like M68K).
* De-list VAX and remove code (known not to work).
* Remove stray trace of Alpha (support was removed years ago).
* List illumos, DragonFlyBSD (known to work, being tested).
* No need to single Windows out by listing a specific version, when we
don't do that for other OSes; it's enough to say that we support
current versions of the listed OSes (when 16 ships, that'll be
Windows 10+).
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Greg Stark <stark@mit.edu>
Discussion: https://postgr.es/m/CA%2BhUKGKk7NZO1UnJM0PyixcZPpCGqjBXW_0bzFZpJBGAf84XKg%40mail.gmail.com
When you hit ^C, the terminal driver in Unix-like systems echoes "^C" as
well as sending an interrupt signal (depending on stty settings). At
least libedit (but maybe also libreadline) is then confused about the
current cursor location, and corrupts the display if you try to scroll
back. Fix, by moving to a new line before the next prompt is displayed.
Back-patch to all supported released.
Author: Pavel Stehule <pavel.stehule@gmail.com>
Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/3278793.1626198638%40sss.pgh.pa.us
Fix incorrect reporting of the location of errors (such as bogus
node attributes). Add header comments to the generated files,
containing copyright notices and reminders that they are generated
files, as we do in other file-generating scripts. Arrange to not
leave a clutter of temporary files when the script detects an error.
Discussion: https://postgr.es/m/3843645.1657385930@sss.pgh.pa.us
copyfuncs.c and friends no longer seem like great places to put
high-level remarks about what's covered and what isn't. Move that
material to backend/nodes/README and other more-prominent places.
Add back (versions of) some remarks that disappeared in 2be87f092.
Discussion: https://postgr.es/m/3843645.1657385930@sss.pgh.pa.us
In the same vein as commit 251154beb, make it clear that we never
instantiate PlanState.
Also mark MemoryContextData as abstract. This has no effect right now,
since memnodes.h isn't one of the files fed to gen_node_support.pl.
But it seems like good documentation and future-proofing.
Add a script to automatically generate the node support functions
(copy, equal, out, and read, as well as the node tags enum) from the
struct definitions.
For each of the four node support files, it creates two include files,
e.g., copyfuncs.funcs.c and copyfuncs.switch.c, to include in the main
file. All the scaffolding of the main file stays in place.
I have tried to mostly make the coverage of the output match what is
currently there. For example, one could now do out/read coverage of
utility statement nodes, but I have manually excluded those for now.
The reason is mainly that it's easier to diff the before and after,
and adding a bunch of stuff like this might require a separate
analysis and review.
Subtyping (TidScan -> Scan) is supported.
For the hard cases, you can just write a manual function and exclude
generating one. For the not so hard cases, there is a way of
annotating struct fields to get special behaviors. For example,
pg_node_attr(equal_ignore) has the field ignored in equal functions.
(In this patch, I have only ifdef'ed out the code to could be removed,
mainly so that it won't constantly have merge conflicts. It will be
deleted in a separate patch. All the code comments that are worth
keeping from those sections have already been moved to the header
files where the structs are defined.)
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/c1097590-a6a4-486a-64b1-e1f9cc0533ce%40enterprisedb.com
PostgreSQL contains the implementation of the red-black tree. The red-black
tree is the ordered data structure, and one of its advantages is the ability
to do inequality searches. This commit adds rbt_find_less() and
rbt_find_great() functions implementing these searches. While these searches
aren't yet used in the core code, they might be useful for extensions.
Discussion: https://postgr.es/m/CAGRrpzYE8-7GCoaPjOiL9T_HY605MRax-2jgTtLq236uksZ1Sw%40mail.gmail.com
Author: Steve Chavez, Alexander Korotkov
Reviewed-by: Alexander Korotkov
Commit 9a974cbcba005256a19991203583a94b4f9a21a9 did this for user
tables, but pg_upgrade treats pg_largeobject as a user table, and so
needs the same treatment. Without this fix, if you rewrite the
pg_largeobject table and then perform an upgrade with pg_upgrade, the
table will apparently be empty on the new cluster, while all of your
objects will end up with an orphaned file.
With this fix, instead of the old cluster's pg_largeobject files ending
up orphaned, the original files fro the new cluster do. That's mostly
harmless because we expect the table to be empty, but we might want
to arrange to remove the as part of the upgrade. Since we're still
debating the best way of doing that, I (rhaas) have decided to postpone
dealing with that problem and get the basic fix committed.
Justin Pryzby, reviewed by Shruthi Gowda and by me.
Discussion: http://postgr.es/m/20220701231413.GI13040@telsasoft.com
This CPU architecture has been discontinued. We already removed HP-UX
support, we never supported Windows/Itanium, and the open source
operating systems that a vintage hardware owner might hope to run have
all either ended Itanium support or never fully released support (NetBSD
may eventually). The extra code we carry for this rare ISA is now
untested. It seems like a good time to remove it.
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/1415825.1656893299%40sss.pgh.pa.us
HP-UX hardware is no longer produced, build farm coverage recently
ended, and there are no known active maintainers targeting this OS.
Since there is a major rewrite of the build system in the pipeline for
PostgreSQL 16, and that requires development, testing and maintainance
for each OS and tool chain, it seems like a good time to drop support
for:
* HP-UX, the operating system.
* HP aCC, the HP-UX native compiler.
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/1415825.1656893299%40sss.pgh.pa.us
These are documented to be the allowed types for the RETURNING clause,
but the restriction was not being enforced, which caused a segfault if
another type was specified. Add some testing for this.
Per report from a.kozhemyakin
Backpatch to release 15.
The general convention in the executor is to refer to child plans
and planstates via the outerPlan[State] and innerPlan[State]
macros, but a few places didn't do it like that. For consistency
and readability, convert all the stragglers to use the macros.
(See also commit 40f42d2a3, which did some similar cleanup a few
years ago, but missed these cases.)
Richard Guo
Discussion: https://postgr.es/m/CAMbWs4-vYhh1xsa_veah4PUed2Xq=Ed_YH3=Mqt5A3Y=EgfCEg@mail.gmail.com
It is useful for debugging purposes to report the checkpoint LSN and
REDO LSN in log_checkpoints message. It can give more context while
analyzing checkpoint-related issues. pg_controldata reports the last
checkpoint LSN and REDO LSN, but having this information alongside
the log message helps analyze issues that happened previously,
connect the dots and identify the root cause.
Author: Bharath Rupireddy, Kyotaro Horiguchi
Reviewed-by: Michael Paquier, Julien Rouhaud, Nathan Bossart, Fujii Masao, Greg Stark
Discussion: https://postgr.es/m/CALj2ACWt6kqriAHrO+AJj+OmP=suwbktHT5JoYAn-nqZe2gd2g@mail.gmail.com
When locking a specific named relation for a FOR [KEY] UPDATE/SHARE
clause, transformLockingClause() finds the relation to lock by
scanning the rangetable for an RTE with a matching eref->aliasname.
However, it failed to account for the visibility rules of a join RTE.
If a join RTE doesn't have a user-supplied alias, it will have a
generated eref->aliasname of "unnamed_join" that is not visible as a
relation name in the parse namespace. Such an RTE needs to be skipped,
otherwise it might be found in preference to a regular base relation
with a user-supplied alias of "unnamed_join", preventing it from being
locked.
In addition, if a join RTE doesn't have a user-supplied alias, but
does have a join_using_alias, then the RTE needs to be matched using
that alias rather than the generated eref->aliasname, otherwise a
misleading "relation not found" error will be reported rather than a
"join cannot be locked" error.
Backpatch all the way, except for the second part which only goes back
to 14, where JOIN USING aliases were added.
Dean Rasheed, reviewed by Tom Lane.
Discussion: https://postgr.es/m/CAEZATCUY_KOBnqxbTSPf=7fz9HWPnZ5Xgb9SwYzZ8rFXe7nb=w@mail.gmail.com
