Protect against malformed timestamps. Also protect against negative WalSegSz
as it triggers division by zero:
((0x100000000UL) / (WalSegSz)) can turn into zero in
XLogFileName(xlogfilename, ControlFile->checkPointCopy.ThisTimeLineID,
segno, WalSegSz);
because if WalSegSz is -1 then by arithmetic rules in C we get
0x100000000UL / 0xFFFFFFFFFFFFFFFFUL == 0.
Author: Ilyasov Ian <ianilyasov@outlook.com>
Author: Anton Voloshin <a.voloshin@postgrespro.ru>
Backpatch-through: 13
A few of the version checks in vacuum_one_database() do not call
PQfinish() before exiting. This precedent was unintentionally
established in commit 00d1e88d36, and while it's probably not too
problematic, it seems better to properly close the connection.
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/Z6JAwqN1I8ljTuXp%40nathan
Backpatch-through: 13
When running on Windows, canonicalize_path() converts '\' to '/'
to prevent confusing the Windows command processor. It was
doing that in a non-encoding-aware fashion; but in SJIS there
are valid two-byte characters whose second byte matches '\'.
So encoding corruption ensues if such a character is used in
the path.
We can fairly easily fix this if we know which encoding is
in use, but a lot of our utilities don't have much of a clue
about that. After some discussion we decided we'd settle for
fixing this only in psql, and assuming that its value of
client_encoding matches what the user is typing.
It seems hopeless to get the server to deal with the problematic
characters in database path names, so we'll just declare that
case to be unsupported. That means nothing need be done in
the server, nor in utility programs whose only contact with
file path names is for database paths. But psql frequently
deals with client-side file paths, so it'd be good if it
didn't mess those up.
Bug: #18735
Reported-by: Koichi Suzuki <koichi.suzuki@enterprisedb.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Koichi Suzuki <koichi.suzuki@enterprisedb.com>
Discussion: https://postgr.es/m/18735-4acdb3998bb9f2b1@postgresql.org
Backpatch-through: 13
The column added in commit e65dbc9927, pubgencols_type, was inconsistent
with the naming conventions of other columns in the pg_publication
catalog.
Author: Vignesh C <vignesh21@gmail.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Reviewed-by: Peter Smith <smithpb2250@gmail.com>
Discussion: https://postgr.es/m/CALDaNm1u-ufVOW-RUsXSooqzkpohxfZYy=z78fbcr_9Pq5hbCg@mail.gmail.com
For SSPI auth extra users need to be explicitly allowed, or we get
"SSPI authentication failed" instead of the expected "role does not
exist" error.
This report also means that the test has never worked on Windows since
its introduction in 9706092839, because it has always bumped on an
authentication failure rather than an error about the role not existing.
Oversight in eef4a33f62, that has added a pattern check on the error
generated by the command.
Per report from Tom Lane, via buildfarm member drongo.
Author: Dagfinn Ilmari Mannsåker
Reviewed-by: Andrew Dunstan
Discussion: https://postgr.es/m/379085.1737734611@sss.pgh.pa.us
Instead of checking just the first letter, match the whole string
using pg_strcasecmp. Per the documentation, we allow either just
the first letter (e.g. "c") or the whole name ("custom"); but we
will no longer accept random variations such as "chump". This
matches pg_dump's longstanding parsing code for the same option.
Also for consistency with pg_dump, recognize "p"/"plain". We don't
support it, but we can give a more helpful error message than
"unrecognized archive format".
Author: Srinath Reddy <srinath2133@gmail.com>
Discussion: https://postgr.es/m/CAFC+b6pfK-BGcWW1kQmtxVrCh-JGjB2X02rLPQs_ZFaDGjZDsQ@mail.gmail.com
The current boolean publish_generated_columns option only supports a
binary choice, which is insufficient for future enhancements where
generated columns can be of different types (e.g., stored or virtual). The
supported values for the publish_generated_columns option are 'none' and
'stored'.
Author: Vignesh C <vignesh21@gmail.com>
Reviewed-by: Peter Smith <smithpb2250@gmail.com>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Discussion: https://postgr.es/m/d718d219-dd47-4a33-bb97-56e8fc4da994@eisentraut.org
Discussion: https://postgr.es/m/B80D17B2-2C8E-4C7D-87F2-E5B4BE3C069E@gmail.com
Some tests are updated to use command_fails_like(), gaining a check for
the error output generated. The test changed in pg_amcheck has come up
after noticing that an incorrect option name still made the test to
pass, while the command failed. The three other tests changed in
src/bin/scripts/ have been noticed by me, in passing.
Author: Dagfinn Ilmari Mannsåker, Michael Paquier
Discussion: https://postgr.es/m/87bjvy50cs.fsf@wibble.ilmari.org
This addresses some minor issues with the TAP tests of pg_basebackup:
- Remove three duplicated tests used for incorrect option combinations.
- Add more pattern checks for commands doomed to fail, to make sure that
the error generated is the expected one. These are for tests related to
the tablespace mapping and incorrect option combinations.
- Fix the description of one test for the case of backup target versus
format.
Issues noticed while reviewing this area of the tests.
Discussion: https://postgr.es/m/87bjvy50cs.fsf@wibble.ilmari.org
This commit rewrites a good chunk of the command arrays in TAP tests
with a grammar based on the following rules:
- Fat commas are used between option names and their values, making it
clear to both humans and perltidy that values and names are bound
together. This is particularly useful for the readability of multi-line
command arrays, and there are plenty of them in the TAP tests. Most of
the test code is updated to use this style. Some commands used
parenthesis to show the link, or attached values and options in a single
string. These are updated to use fat commas instead.
- Option names are switched to use their long names, making them more
self-documented. Based on a suggestion by Andrew Dunstan.
- Add some trailing commas after the last item in multi-line arrays,
which is a common perl style.
Not all the places are taken care of, but this covers a very good chunk
of them.
Author: Dagfinn Ilmari Mannsåker
Reviewed-by: Michael Paquier, Peter Smith, Euler Taveira
Discussion: https://postgr.es/m/87jzc46d8u.fsf@wibble.ilmari.org
A follow-up patch will adjust the TAP tests to follow a more-structured
format for option lists in commands, that perltidy is able to cope
better with. Putting the tree first in a clean state makes the next
change a bit easier. v20230309 has been used.
Author: Dagfinn Ilmari Mannsåker
Discussion: https://postgr.es/m/87jzc46d8u.fsf@wibble.ilmari.org
The PG_UNICODE_FAST locale uses code point sort order (fast,
memcmp-based) combined with Unicode character semantics. The character
semantics are based on Unicode full case mapping.
Full case mapping can map a single codepoint to multiple codepoints,
such as "ß" uppercasing to "SS". Additionally, it handles
context-sensitive mappings like the "final sigma", and it uses
titlecase mappings such as "Dž" when titlecasing (rather than plain
uppercase mappings).
Importantly, the uppercasing of "ß" as "SS" is specifically mentioned
by the SQL standard. In Postgres, UCS_BASIC uses plain ASCII semantics
for case mapping and pattern matching, so if we changed it to use the
PG_UNICODE_FAST locale, it would offer better compliance with the
standard. For now, though, do not change the behavior of UCS_BASIC.
Discussion: https://postgr.es/m/ddfd67928818f138f51635712529bc5e1d25e4e7.camel@j-davis.com
Discussion: https://postgr.es/m/27bb0e52-801d-4f73-a0a4-02cfdd4a9ada@eisentraut.org
Reviewed-by: Peter Eisentraut, Daniel Verite
Since commit e0c2933a76, vacuum_one_database() always uses a
catalog query to discover the tables to process, but this comment
still notes the special case for which we used a catalog query
before that commit. Let's just remove that note.
Also, commit 7781f4e3e7 renamed the "tables" parameter to "objects"
but missed updating this comment. This commit fixes that as well.
As noted by the comment at the top of port/pqsignal.c, Windows
frontend programs can only use pqsignal() with the 6 signals
required by C. Most places avoid using invalid signals via #ifndef
WIN32, but initdb and pg_test_fsync check whether the signal itself
is defined, which doesn't work because win32_port.h defines many
extra signals for the signal emulation code. pg_regress seems to
have missed the memo completely. These issues aren't causing any
real problems today because nobody checks the return value of
pqsignal(), but a follow-up commit will add some error checking.
To fix, surround all frontend calls to pqsignal() that use signals
that are invalid on Windows with #ifndef WIN32. We cannot simply
skip defining the extra signals in win32_port.h for frontends
because they are needed in places such as pgkill().
Reviewed-by: Thomas Munro
Discussion: https://postgr.es/m/Z4chOKfnthRH71mw%40nathan
This removes all the various workarounds for avoiding compiler
warnings with Flex 2.5.35. Several recent patches have added
additional warnings that would either need to be fixed along the lines
of the existing workarounds, or we decide to no longer care about
this, which we do here.
Flex 2.5.35 is extremely outdated, and you can't even download it
anymore from any of the Flex project sites, so it's nearly impossible
to support.
After this, using Flex 2.5.35 will still work, but the generated code
will produce numerous compiler warnings.
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/1a204ccd-7ae6-478c-a431-407b5c48ccc6@eisentraut.org
This allows "x" to be appended to any psql list-like meta-command,
forcing its output to be displayed in expanded mode. This improves
readability in cases where the output is very wide. For example,
"\dfx+" (or equivalently "\df+x") will produce a list of functions,
with additional details, in expanded mode.
This works with all \d* meta-commands, plus \l, \z, and \lo_list, with
the one exception that the expanded mode option "x" cannot be appended
to "\d" by itself, since "\dx" already means something else.
Dean Rasheed, reviewed by Greg Sabino Mullane.
Discussion: https://postgr.es/m/CAEZATCVXJk3KsmCncf7PAVbxdDAUDm3QzDgGT7mBYySWikuOYw@mail.gmail.com
This allows users to determine whether particular functions are
leakproof, and whether the underlying functions used by operators and
casts are leakproof. This is useful to determine whether indexes can
be used in queries on security barrier views or tables with row-level
security policies.
Yugo Nagata, reviewed by Erik Wienhold and Dean Rasheed.
Discussion: https://postgr.es/m/20240701220817.483f9b645b95611f8b1f65da%40sranhm.sraoss.co.jp
walmethods.c used off_t to navigate around a pg_wal.tar file that could
exceed 2GB, which doesn't work on Windows and would fail with misleading
errors. Use pgoff_t instead.
Back-patch to all supported branches.
Author: Davinder Singh <davinder.singh@enterprisedb.com>
Reported-by: Jakub Wartak <jakub.wartak@enterprisedb.com>
Discussion: https://postgr.es/m/CAKZiRmyM4YnokK6Oenw5JKwAQ3rhP0YTz2T-tiw5dAQjGRXE3Q%40mail.gmail.com
Commit c758119e5b increased the default number of semaphores
required for autovacuum workers from 3 to 16. Unfortunately, some
systems have very low default settings for SEMMNS, and this change
moved the minimum required for Postgres well beyond that limit (see
commit 38da053463 for more details).
With this commit, initdb will lower the default value for
autovacuum_worker_slots as needed, just like it already does for
parameters such as max_connections and shared_buffers. We test
for (max_connections / 6) slots, which conveniently has the
following properties:
* For the initial max_connections default of 100, the default of
autovacuum_worker_slots will be 16, which is its initial default
value specified in the documentation and in guc_tables.c.
* For the lowest possible max_connections default of 25, the
default of autovacuum_worker_slots will be 4, which means we only
need one additional semaphore for autovacuum workers (as compared
to before commit c758119e5b). This leaves some wiggle room for
new auxiliary workers, etc. on systems with low SEMMNS, and it
ensures that the default number of slots will be greater than or
equal to the default value of autovacuum_max_workers (3).
Reported-by: Tom Lane
Suggested-by: Andres Freund
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/1346002.1736198977%40sss.pgh.pa.us
Trying to clean up the code a bit while we're working on these files
for the reentrant scanner/pure parser patches. This cleanup only
touches the code sections after the second '%%' in each file, via a
manually-supervised and locally hacked up pgindent.
Per git log, the last time someone tried to do something with
pgrminclude was around 2011. Many (not all) of the "pgrminclude
ignore" annotations are of a newer date but seem to have just been
copied around during refactorings and file moves and don't seem to
reflect an actual need anymore.
There have been some parallel experiments with include-what-you-use
(IWYU) annotations, but these don't seem to correspond very strongly
to pgrminclude annotations, so there is no value in keeping the
existing ones even for that kind of thing.
So, wipe them all away. We can always add new ones in the future
based on actual needs.
Discussion: https://www.postgresql.org/message-id/flat/2d4dc7b2-cb2e-49b1-b8ca-ba5f7024f05b%40eisentraut.org
These two platforms have a remarkably tight default limit on the
number of SysV semaphores in the system: SEMMNS is only 60
out-of-the-box. Unless manual action is taken to raise that,
we'll only be able to allocate 3 sets of 16 usable semaphores
each, leading to initdb setting max_connections to just 20.
That's problematic because the core regression tests expect
to be able to launch 20 concurrent sessions, leaving us with
no headroom. This seems to be the cause of intermittent
buildfarm failures on some machines.
While there's no getting around the fact that you'd better raise
SEMMNS for production use on these platforms, it does seem desirable
for "make check" to pass reliably without that. We can make that
happen, at least for awhile longer, with two small changes:
* Change sysv_sema.c's SEMAS_PER_SET to 19, so that we can eat up
all of the available semas not just most of them.
* Change initdb to make the smallest max_connections value it will
consider be 25 not 20.
As of HEAD this will leave us with four free semaphores (using the
default values for other relevant parameters such as max_wal_senders).
So we won't need to consider this again until we've invented five
more background processes. Maybe by then we can switch both these
platforms to some other semaphore API.
For the moment, do this only in master; there've not been field
complaints that might justify a back-patch.
Discussion: https://postgr.es/m/db2773a2-aca0-43d0-99c1-060efcd9954e@gmail.com
This commit adds support for the following items in psql, able to show a
service name, when available:
- Variable SERVICE.
- Substitution %s in PROMPT{1,2,3}.
This relies on 4b99fed754, that has made the service name available in
PGconn for libpq.
Author: Michael Banck
Reviewed-by: Greg Sabino Mullane
Discussion: https://postgr.es/m/6723c612.050a0220.1567f4.b94a@mx.google.com
The test was creating both the dumps to compare from the same database
on the same node, so it would never detect any mismatches when comparing
the logical dumps of the two servers.
Fixing this issue has revealed that there is a difference in the dumps:
the tablespaces paths are different. This commit uses compare_text()
with a custom comparison function to erase the difference (slightly
tweaked to be able to work with WIN32 and non-WIN32 paths). This way,
the non-relevant parts of the tablespace path are ignored from the check
with the basic structure of the query string still compared.
Author: Dagfinn Ilmari Mannsåker
Discussion: https://postgr.es/m/87h67653ns.fsf@wibble.ilmari.org
Backpatch-through: 17
Updates table completion for ALTER TYPE to offer CASCADE/RESTRICT for a
number of actions on attributes:
ALTER TYPE ... ADD/DROP/RENAME ATTRIBUTE ... [CASCADE|RESTRICT]
ALTER TYPE ... TYPE ... [CASCADE|RESTRICT]
Author: Kirill Reshke
Reviewed-By: Karina Litskevich
Discussion: https://postgr.es/m/CALdSSPhVELkvutquqrDB=Ujfq_Pjz=6jn-kzh+291KPNViLTfw@mail.gmail.com
An \if command appearing within a false (not-to-be-executed) \if
branch was incorrectly treated the same as \elif. This could allow
statements within the inner \if to be executed when they should
not be. Also the missing inner \if stack entry would result in an
assertion failure (in assert-enabled builds) when the final \endif
is reached.
Report and patch by Michail Nikolaev. Back-patch to all
supported branches.
Discussion: https://postgr.es/m/CANtu0oiA1ke=SP6tauhNqkUdv5QFsJtS1p=aOOf_iU+EhyKkjQ@mail.gmail.com
If an owned sequence is considered interesting, force its owning
table to be marked interesting too. This ensures, in particular,
that we'll fetch the owning table's column names so we have the
data needed for ALTER TABLE ... ADD GENERATED. Previously there were
edge cases where pg_dump could get SIGSEGV due to not having filled in
the column names. (The known case is where the owning table has been
made part of an extension while its identity sequence is not a member;
but there may be others.)
Also, if it's an identity sequence, force its dumped-components mask
to exactly match the owning table: dump definition only if we're
dumping the table's definition, dump data only if we're dumping the
table's data, etc. This generalizes the code introduced in commit
b965f2617 that set the sequence's dump mask to NONE if the owning
table's mask is NONE. That's insufficient to prevent failures,
because for example the table's mask might only request dumping ACLs,
which would lead us to still emit ALTER TABLE ADD GENERATED even
though we didn't create the table. It seems better to treat an
identity sequence as though it were an inseparable aspect of the
table, matching the treatment used in the backend's dependency logic.
Perhaps this policy needs additional refinement, but let's wait to
see some field use-cases before changing it further.
While here, add a comment in pg_dump.h warning against writing tests
like "if (dobj->dump == DUMP_COMPONENT_NONE)", which was a bug in this
case. There is one other example in getPublicationNamespaces, which
if it's not a bug is at least remarkably unclear and under-documented.
Changing that requires a separate discussion, however.
Per report from Artur Zakirov. Back-patch to all supported branches.
Discussion: https://postgr.es/m/CAKNkYnwXFBf136=u9UqUxFUVagevLQJ=zGd5BsLhCsatDvQsKQ@mail.gmail.com
With not-null constraints defined in child tables for columns that are
coming from their parent tables, we were printing ALTER TABLE SET NOT
NULL commands that were missing the constraint name, so the original
constraint name was being lost, which is bogus. Fix by instead adding
a table-constraint constraint declaration with the correct constraint
name in the CREATE TABLE instead.
Oversight in commit 14e87ffa5c.
We could have fixed it by changing the ALTER TABLE SET NOT NULL to ALTER
TABLE ADD CONSTRAINT, but I'm not sure that's any better. A potential
problem here might be that if sent to a non-Postgres server, the new
pg_dump output would fail because the "CONSTRAINT foo NOT NULL colname"
syntax isn't SQL-conforming. However, Postgres' implementation of
inheritance is already non-SQL-conforming, so that'd likely fail anyway.
This problem was only noticed by Ashutosh's proposed test framework for
pg_dump, https://postgr.es/m/CAExHW5uF5V=Cjecx3_Z=7xfh4rg2Wf61PT+hfquzjBqouRzQJQ@mail.gmail.com
Author: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Reported-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/CAExHW5tbdgAKDfqjDJ-7Fk6PJtHg8D4zUF6FQ4H2Pq8zK38Nyw@mail.gmail.com
The gneration of the dump clause for functions with TRANSFORM
calls would leak the memory for holding the result of the Oid
array parsing. Fix by freeing.
While in there, switch to using pg_malloc instead of palloc in
order to be consistent with the rest of the file.
Author: Oleg Tselebrovskiy <o.tselebrovskiy@postgrespro.ru>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/baf1ae4511288e5b421f41e79a3df1a0@postgrespro.ru
On failure, the pg_upgrade log files are automatically appended to the
test log file, but the information reported was inconsistent.
A header, with the log file name, was reported with note(), while the
log contents and a footer used print(), making it harder to diagnose
failures when these are split into console output and test log file
because the pg_upgrade log file path in the header may not be included
in the test log file.
The output is now consolidated so as the header uses print() rather than
note(). An extra note() is added to inform that the contents of a
pg_upgrade log file are appended to the test log file.
The diffs from the regression test suite and dump files all use print()
to show their contents on failure.
Author: Joel Jacobson
Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/49f7e64a-b9be-4a90-a9fe-210a7740405e@app.fastmail.com
Backpatch-through: 15
Redefine our exact width types with standard C99 types and macros,
including int64_t, INT64_MAX, INT64_C(), PRId64 etc. We were already
using <stdint.h> types in a few places.
One complication is that Windows' <inttypes.h> uses format strings like
"%I64d", "%I32", "%I" for PRI*64, PRI*32, PTR*PTR, instead of mapping to
other standardized format strings like "%lld" etc as seen on other known
systems. Teach our snprintf.c to understand them.
This removes a lot of configure clutter, and should also allow 64-bit
numbers and other standard types to be used in localized messages
without casting.
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Discussion: https://postgr.es/m/ME3P282MB3166F9D1F71F787929C0C7E7B6312%40ME3P282MB3166.AUSP282.PROD.OUTLOOK.COM
This commit fixes some inconsistencies found in the frontend code when
dealing with subscription catalog data.
The following changes are done:
- pg_subscription.h gains a EXPOSE_TO_CLIENT_CODE, so as more content
defined in pg_subscription.h becomes available in pg_subscription_d.h
for the frontend.
- In psql's describe.c, substream can be switched to use CppAsString2()
with its three LOGICALREP_STREAM_* values, with pg_subscription_d.h
included.
- pg_dump.c included pg_subscription.h, which is a header that should
only be used in the backend code. The code is updated to use
pg_subscription_d.h instead.
- pg_dump stored all the data from pg_subscription in SubscriptionInfo
with only strings, and a good chunk of them are boolean and char values.
Using strings is not necessary, complicates the code (see for example
two_phase_disabled[] removed here), and is inconsistent with the way
other catalogs' data is handled. The fields of SubscriptionInfo are
reordered to match with the order in its catalog, while on it.
Reviewed-by: Hayato Kuroda
Discussion: https://postgr.es/m/Z0lB2kp0ksHgmVuk@paquier.xyz
Like 91f5a4a000 for pg_amcheck, this makes the code more
self-documented as there is less need to look in the headers what a
hardcoded value means. This touches queries related to procedures, AMs,
functions, databases, relations, constraints, collations, types and
extended stats, pulling into psql their *_d.h headers. The queries are
written the same way as originally.
There are still a couple of hardcoded values. These cannot be included
yet as they are not exposed in headers that are safe to use in frontend
code.
Note that describe.c was including pg_am.h that should be used only in
backend code. This is updated to use pg_am_d.h.
Reviewed-by: Daniel Gustafsson, Corey Huinker
Discussion: https://postgr.es/m/Zxb2hpca-pZc6zKe@paquier.xyz
