Commit Graph

345 Commits

Author SHA1 Message Date
Tom Lane
a0ffa885e4 Allow granting SET and ALTER SYSTEM privileges on GUC parameters.
This patch allows "PGC_SUSET" parameters to be set by non-superusers
if they have been explicitly granted the privilege to do so.
The privilege to perform ALTER SYSTEM SET/RESET on a specific parameter
can also be granted.
Such privileges are cluster-wide, not per database.  They are tracked
in a new shared catalog, pg_parameter_acl.

Granting and revoking these new privileges works as one would expect.
One caveat is that PGC_USERSET GUCs are unaffected by the SET privilege
--- one could wish that those were handled by a revocable grant to
PUBLIC, but they are not, because we couldn't make it robust enough
for GUCs defined by extensions.

Mark Dilger, reviewed at various times by Andrew Dunstan, Robert Haas,
Joshua Brindle, and myself

Discussion: https://postgr.es/m/3D691E20-C1D5-4B80-8BA5-6BEB63AF3029@enterprisedb.com
2022-04-06 13:24:33 -04:00
Joe Conway
6198420ad8 Use has_privs_for_roles for predefined role checks
Generally if a role is granted membership to another role with NOINHERIT
they must use SET ROLE to access the privileges of that role, however
with predefined roles the membership and privilege is conflated. Fix that
by replacing is_member_of_role with has_privs_for_role for predefined
roles. Patch does not remove is_member_of_role from acl.h, but it does
add a warning not to use that function for privilege checking. Not
backpatched based on hackers list discussion.

Author: Joshua Brindle
Reviewed-by: Stephen Frost, Nathan Bossart, Joe Conway
Discussion: https://postgr.es/m/flat/CAGB+Vh4Zv_TvKt2tv3QNS6tUM_F_9icmuj0zjywwcgVi4PAhFA@mail.gmail.com
2022-03-28 15:10:04 -04:00
Daniel Gustafsson
7dac61402e Remove unused module imports from TAP tests
The Config and Cwd modules were no longer used, but remained imported,
in a number of tests.  Remove to keep the imports to the actually used
modules.

Reviewed-by: Andrew Dunstan <andrew@dunslane.net>
Discussion: https://postgr.es/m/A5A074CD-3198-492B-BE5E-7961EFC3733F@yesql.se
2022-03-24 20:51:40 +01:00
Andrew Dunstan
c97e4bdcb1 Clean test_rls_hooks module
This module isn't an extension and doesn't need to be preloaded.
Adjust the Makefile and remove the extraneous .control and .conf
files accordingly.

Discussion: https://postgr.es/m/43bcaaab-077e-cebe-35be-3cd7f2633449@dunslane.net
2022-03-23 15:11:45 -04:00
Andrew Dunstan
7c51b7f7cc
Force NO_LOCALE / UTF8 for test_oat_hooks tests
This will fix cases like fairywren that have been having issues.

Discussion: https://postgr.es/m/2630561.1647994022@sss.pgh.pa.us
2022-03-23 11:13:33 -04:00
Andrew Dunstan
fc15396dc6
Use approved style for listing OBJS in test_oat_hooks Makefile 2022-03-23 10:42:25 -04:00
Andrew Dunstan
5b29a9f772 Temporarily disable installcheck for test_oat_hooks module
Buildfarm members are encountering errors when the test is run under
various locales/encodings. As the buildfarm only does this for
installchecks, disable them for now.

Discussion: https://postgr.es/m/6067945b-960a-ab04-d40f-06b006a1dcd0@dunslane.net
2022-03-22 20:18:30 -04:00
Andrew Dunstan
a3b071bbe0 Tidy up Object Access hooks tests
per gripelet from Tom Lane.
2022-03-22 16:18:06 -04:00
Andrew Dunstan
f0206d9952 Fix new Object Access hooks test
Commit 90efa2f556 caused some issues with EXEC_BACKEND builds and with
force_parallel_mode = regress setups. For the first issue we no longer
test if the module has been preloaded, and in fact we don't preload it,
but simply LOAD it in the test script. For the second issue we suppress
error messages emanating from parallel workers.

Mark Dilger

Discussion: https://postgr.es/m/7f6d54a1-4024-3b6e-e3ec-26cd394aac9e@dunslane.net
2022-03-22 13:27:26 -04:00
Andrew Dunstan
90efa2f556 Add a test module for Object Access hooks
This includes tests of both the newly added name type object access
hooks and the older Oid type hooks, and provides a useful example
of how to use the hooks.

Mark Dilger, based on some code from Joshua Brindle.

Discussion: https://postgr.es/m/47F87A0E-C0E5-43A6-89F6-D403F2B45175@enterprisedb.com
2022-03-22 10:28:31 -04:00
Tom Lane
88103567cb Disallow setting bogus GUCs within an extension's reserved namespace.
Commit 75d22069e tried to throw a warning for setting a custom GUC whose
prefix belongs to a previously-loaded extension, if there is no such GUC
defined by the extension.  But that caused unstable behavior with
parallel workers, because workers don't necessarily load extensions and
GUCs in the same order their leader did.  To make that work safely, we
have to completely disallow the case.  We now actually remove any such
GUCs at the time of initial extension load, and then throw an error not
just a warning if you try to add one later.  While this might create a
compatibility issue for a few people, the improvement in error-detection
capability seems worth it; it's hard to believe that there's any good
use-case for choosing such GUC names.

This also un-reverts 5609cc01c (Rename EmitWarningsOnPlaceholders() to
MarkGUCPrefixReserved()), since that function's old name is now even
more of a misnomer.

Florin Irion and Tom Lane

Discussion: https://postgr.es/m/1902182.1640711215@sss.pgh.pa.us
2022-02-21 14:10:43 -05:00
Andrew Dunstan
95d981338b
Remove PostgreSQL::Test::Utils::perl2host completely
Commit f1ac4a74de disabled this processing, and as nothing has broken (as
expected) here we proceed to remove the routine and adjust all the call
sites.

Backpatch to release 10

Discussion: https://postgr.es/m/0ba775a2-8aa0-0d56-d780-69427cf6f33d@dunslane.net
Discussion: https://postgr.es/m/20220125023609.5ohu3nslxgoygihl@alap3.anarazel.de
2022-02-20 11:51:45 -05:00
Michael Paquier
7265dbffad Add TAP test to automate the equivalent of check_guc, take two
src/backend/utils/misc/check_guc is a script that cross-checks the
consistency of the GUCs with postgresql.conf.sample, making sure that
its format is in line with what guc.c has.  It has never been run
automatically, and has rotten over the years, creating a lot of false
positives as per a report from Justin Pryzby.

d10e41d has introduced a SQL function to publish the most relevant flags
associated to a GUC, with tests added in the main regression test suite
to make sure that we avoid most of the inconsistencies in the GUC
settings, based on recent reports, but there was nothing able to
cross-check postgresql.conf.sample with the contents of guc.c.

This commit adds a TAP test that covers the remaining gap.  It emulates
the most relevant checks that check_guc did, so as any format mistakes
are detected in postgresql.conf.sample at development stage, with the
following checks:
- Check that parameters marked as NOT_IN_SAMPLE are not in the sample
file.
- Check that there are no dead entries in postgresql.conf.sample for
parameters not marked as NOT_IN_SAMPLE.
- Check that no parameters are missing from the sample file if listed in
guc.c without NOT_IN_SAMPLE.

The idea of building a list of the GUCs by parsing the sample file comes
from Justin, and he wrote the regex used in the patch to find all the
GUCs (this same formatting rule basically applies for the last 20~ years
or so).  In order to test this patch, I have played with manual
modifications of postgresql.conf.sample and guc.c, making sure that we
detect problems with the GUC rules and the sample file format.

The test is located in src/test/modules/test_misc, which is the best
location I could think about for such sanity checks, rather than the
main regression test suite (src/test/regress) to avoid a new type of
dependency with the source tree.

The first attempt of this patch was b0a55f4, where the location of
postgresql.conf.sample was retrieved using pg_config --sharedir.  This
has proven to be an issue for distributions that patch pg_config to
enforce the installation paths at some wanted location (like Debian),
that may not exist when the test is run, hence causing a failure.
Instead of that, as per a suggestion from Andres Freund, rely on the
fact that the test is always executed from its directory in the source
tree and use a relative path to find the sample file.  This works for
the CI, VPATH builds and on Windows, and tests like the recovery one
added in f47ed79 rely on that already.

Reviewed-by: Justin Pryzby
Discussion: https://postgr.es/m/Yf9YGSwPiMu0c7fP@paquier.xyz
2022-02-16 10:25:12 +09:00
Michael Paquier
52377bb81a Revert "Add TAP test to automate the equivalent of check_guc"
This reverts commit b0a55f4, to remove for now the TAP test that did the
equivalent of check_guc.  The test has been using pg_config --sharedir
to find the location of postgresql.conf.sample.  While the buildfarm and
normal build environments rather liked that, this proves to be an issue
for Debian where pg_config is patched to not be relocatable, causing the
test to fail.

Rather than relying on pg_config, we'd better find the sample file based
on its location from the source directory.  However, this is also an
issue as a TAP test only offers the build directory as of TESTDIR in the
environment context, so this would fail with VPATH builds.  Perhaps the
source path could be provided additionally when running the TAP tests.
Or perhaps we may be able to get away by just switching to a SQL
approach, by using PG_ABS_SRCDIR but this is going to require some extra
loops to get the sample file from the correct path in src/backend/.  In
any case, this needs more thoughts, so just revert the test case until
something better is done about this relocation problem.

Reported-by: Christopher Berg
Discussion: https://postgr.es/m/YgYw25OXV5men8Fj@msg.df7cb.de
2022-02-12 12:53:59 +09:00
Daniel Gustafsson
549ec201d6 Replace Test::More plans with done_testing
Rather than doing manual book keeping to plan the number of tests to run
in each TAP suite, conclude each run with done_testing() summing up the
the number of tests that ran. This removes the need for maintaning and
updating the plan count at the expense of an accurate count of remaining
during the test suite runtime.

This patch has been discussed a number of times, often in the context of
other patches which updates tests, so a larger number of discussions can
be found in the archives.

Reviewed-by: Julien Rouhaud <rjuju123@gmail.com>
Reviewed-by: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://postgr.es/m/DD399313-3D56-4666-8079-88949DAC870F@yesql.se
2022-02-11 20:54:44 +01:00
Michael Paquier
b0a55f4d4a Add TAP test to automate the equivalent of check_guc
src/backend/utils/misc/check_guc is a script that cross-checks the
consistency of the GUCs with postgresql.conf.sample, making sure that
its format is in line with what guc.c has.  It has never been run
automatically, and has rotten over the years, creating a lot of false
positives as per a report from Justin Pryzby.

d10e41d has introduced a SQL function to publish the most relevant flags
associated to a GUC, with tests added in the main regression test suite
to make sure that we avoid most of the inconsistencies in the GUC
settings, based on recent reports, but there was nothing able to
cross-check postgresql.conf.sample with the contents of guc.c.

This commit adds a TAP test that covers the remaining gap.  It emulates
the most relevant checks that check_guc does, so as any format mistakes
are detected in postgresql.conf.sample at development stage, with the
following checks:
- Check that parameters marked as NOT_IN_SAMPLE are not in the sample
file.
- Check that there are no dead entries in postgresql.conf.sample for
parameters not marked as NOT_IN_SAMPLE.
- Check that no parameters are missing from the sample file if listed in
guc.c without NOT_IN_SAMPLE.

The idea of building a list of the GUCs by parsing the sample file comes
from Justin, and he wrote the regex used in the patch to find all the
GUCs (this same formatting rule basically applies for the last 20~ years
or so).  In order to test this patch, I have played with manual
modifications of postgresql.conf.sample and guc.c, making sure that we
detect problems with the GUC rules and the sample file format.

The test is located in src/test/modules/test_misc, which is the best
location I could think about for such sanity checks.

Reviewed-by: Justin Pryzby
Discussion: https://postgr.es/m/Yf9YGSwPiMu0c7fP@paquier.xyz
2022-02-09 10:15:26 +09:00
Michael Paquier
410aa248e5 Fix various typos, grammar and code style in comments and docs
This fixes a set of issues that have accumulated over the past months
(or years) in various code areas.  Most fixes are related to some recent
additions, as of the development of v15.

Author: Justin Pryzby
Discussion: https://postgr.es/m/20220124030001.GQ23027@telsasoft.com
2022-01-25 09:40:04 +09:00
Thomas Munro
cfe7bd17e4 Add new simple TAP test for tablespaces, attempt II.
See commit message for d1511fe1b0.  This
new version attempts to fix path translation problem on MSYS/Windows.

Discussion: https://postgr.es/m/20220117055326.GD756210%40rfd.leadboat.com
2022-01-21 15:38:05 +13:00
Robert Haas
9a974cbcba pg_upgrade: Preserve relfilenodes and tablespace OIDs.
Currently, database OIDs, relfilenodes, and tablespace OIDs can all
change when a cluster is upgraded using pg_upgrade. It seems better
to preserve them, because (1) it makes troubleshooting pg_upgrade
easier, since you don't have to do a lot of work to match up files
in the old and new clusters, (2) it allows 'rsync' to save bandwidth
when used to re-sync a cluster after an upgrade, and (3) if we ever
encrypt or sign blocks, we would likely want to use a nonce that
depends on these values.

This patch only arranges to preserve relfilenodes and tablespace
OIDs. The task of preserving database OIDs is left for another patch,
since it involves some complexities that don't exist in these cases.

Database OIDs have a similar issue, but there are some tricky points
in that case that do not apply to these cases, so that problem is left
for another patch.

Shruthi KC, based on an earlier patch from Antonin Houska, reviewed
and with some adjustments by me.

Discussion: http://postgr.es/m/CA+TgmoYgTwYcUmB=e8+hRHOFA0kkS6Kde85+UNdon6q7bt1niQ@mail.gmail.com
2022-01-17 13:40:27 -05:00
Thomas Munro
0c53a6658e Revert "Add new simple TAP test for tablespaces."
This reverts commit d1511fe1b0.

Discussion: https://postgr.es/m/CA%2BhUKG%2BGBC-6QhOKt6Y7ccrXSjbRHB7Di295%3D0rAGhE7a7hSrQ%40mail.gmail.com
2022-01-15 02:16:07 +13:00
Thomas Munro
d1511fe1b0 Add new simple TAP test for tablespaces.
The tablespace tests in the main regression tests have been changed to
use "in-place" tablespaces, so that they work when streamed to a replica
on the same host.  Add a new TAP test that exercises tablespaces with
absolute paths, for coverage.

Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/CA%2BhUKGKpRWQ9SxdxxDmTBCJoR0YnFpMBe7kyzY8SUQk%2BHeskxg%40mail.gmail.com
2022-01-15 00:09:24 +13:00
Bruce Momjian
27b77ecf9f Update copyright for 2022
Backpatch-through: 10
2022-01-07 19:04:57 -05:00
Tom Lane
cab5b9ab2c Revert changes about warnings/errors for placeholders.
Revert commits 5609cc01c, 2ed8a8cc5, and 75d22069e until we have
a less broken idea of how this should work in parallel workers.
Per buildfarm.

Discussion: https://postgr.es/m/1640909.1640638123@sss.pgh.pa.us
2021-12-27 16:01:10 -05:00
Tom Lane
5609cc01c6 Rename EmitWarningsOnPlaceholders() to MarkGUCPrefixReserved().
This seems like a clearer name for what it does now.

Provide a compatibility macro so that extensions don't have to convert
to the new name right away.

Discussion: https://postgr.es/m/116024.1640111629@sss.pgh.pa.us
2021-12-27 14:39:08 -05:00
Tom Lane
1fada5d81e Add missing EmitWarningsOnPlaceholders() calls.
Extensions that define any custom GUCs should call
EmitWarningsOnPlaceholders after doing so, to help catch misspellings.
Many of our contrib modules hadn't gotten the memo on that, though.

Also add such calls to src/test/modules extensions that have GUCs.
While these aren't really user-facing, they should illustrate good
practice not faulty practice.

Shinya Kato

Discussion: https://postgr.es/m/524fa2c0a34f34b68fbfa90d0760d515@oss.nttdata.com
2021-12-21 12:12:24 -05:00
Tomas Vondra
5753d4ee32 Ignore BRIN indexes when checking for HOT udpates
When determining whether an index update may be skipped by using HOT, we
can ignore attributes indexed only by BRIN indexes. There are no index
pointers to individual tuples in BRIN, and the page range summary will
be updated anyway as it relies on visibility info.

This also removes rd_indexattr list, and replaces it with rd_attrsvalid
flag. The list was not used anywhere, and a simple flag is sufficient.

Patch by Josef Simanek, various fixes and improvements by me.

Author: Josef Simanek
Reviewed-by: Tomas Vondra, Alvaro Herrera
Discussion: https://postgr.es/m/CAFp7QwpMRGcDAQumN7onN9HjrJ3u4X3ZRXdGFT0K5G2JWvnbWg%40mail.gmail.com
2021-11-30 20:04:38 +01:00
Tom Lane
3804539e48 Replace random(), pg_erand48(), etc with a better PRNG API and algorithm.
Standardize on xoroshiro128** as our basic PRNG algorithm, eliminating
a bunch of platform dependencies as well as fundamentally-obsolete PRNG
code.  In addition, this API replacement will ease replacing the
algorithm again in future, should that become necessary.

xoroshiro128** is a few percent slower than the drand48 family,
but it can produce full-width 64-bit random values not only 48-bit,
and it should be much more trustworthy.  It's likely to be noticeably
faster than the platform's random(), depending on which platform you
are thinking about; and we can have non-global state vectors easily,
unlike with random().  It is not cryptographically strong, but neither
are the functions it replaces.

Fabien Coelho, reviewed by Dean Rasheed, Aleksander Alekseev, and myself

Discussion: https://postgr.es/m/alpine.DEB.2.22.394.2105241211230.165418@pseudo
2021-11-28 21:33:07 -05:00
Andrew Dunstan
b3b4d8e68a
Move Perl test modules to a better namespace
The five modules in our TAP test framework all had names in the top
level namespace. This is unwise because, even though we're not
exporting them to CPAN, the names can leak, for example if they are
exported by the RPM build process. We therefore move the modules to the
PostgreSQL::Test namespace. In the process PostgresNode is renamed to
Cluster, and TestLib is renamed to Utils. PostgresVersion becomes simply
PostgreSQL::Version, to avoid possible confusion about what it's the
version of.

Discussion: https://postgr.es/m/aede93a4-7d92-ef26-398f-5094944c2504@dunslane.net

Reviewed by Erik Rijkers and Michael Paquier
2021-10-24 10:28:19 -04:00
Andrew Dunstan
f4ce6c4d3a
Add module build directory to the PATH for TAP tests
For non-MSVC builds this is make's $(CURDIR), while for MSVC builds it
is $topdir/$Config/$module. The directory is added as the second element
in the PATH, so that the install location takes precedence, but the
added PATH element takes precedence over the rest of the PATH.

The reason for this is to allow tests to find built products that are
not installed, such as the libpq_pipeline test driver.

The libpq_pipeline test is adjusted to take advantage of this.

Based on a suggestion from Andres Freund.

Backpatch to release 14.

Discussion: https://postgr.es/m/4941f5a5-2d50-1a0e-6701-14c5fefe92d6@dunslane.net
2021-10-22 09:49:07 -04:00
Robert Haas
46846433a0 shm_mq: Update mq_bytes_written less often.
Do not update shm_mq's mq_bytes_written until we have written
an amount of data greater than 1/4th of the ring size, unless
the caller of shm_mq_send(v) requests a flush at the end of
the message. This reduces the number of calls to SetLatch(),
and also the number of CPU cache misses, considerably, and thus
makes shm_mq significantly faster.

Dilip Kumar, reviewed by Zhihong Yu and Tomas Vondra. Some
minor cosmetic changes by me.

Discussion: http://postgr.es/m/CAFiTN-tVXqn_OG7tHNeSkBbN+iiCZTiQ83uakax43y1sQb2OBA@mail.gmail.com
2021-10-14 16:13:36 -04:00
Peter Eisentraut
ba216d3b54 Fix loop variable signedness 2021-10-06 07:22:47 +02:00
Andres Freund
795862c280 Reference test binary using TESTDIR in 001_libpq_pipeline.pl.
The previous approach didn't really work on windows, due to the PATH separator
being ';' not ':'. Instead of making the PATH change more complicated,
reference the binary using the TESTDIR environment.

Reported-By: Andres Freund <andres@anarazel.de>
Suggested-By: Andrew Dunstan <andrew@dunslane.net>
Discussion: https://postgr.es/m/20210930214040.odkdd42vknvzifm6@alap3.anarazel.de
Backpatch: 14-, where the test was introduced.
2021-10-01 15:30:16 -07:00
Michael Paquier
5fcb23c18f Remove some unused variables in TAP tests
Author: Amul Sul
Discussion: https://postgr.es/m/CAAJ_b96xuFh4JZE6p-zhLyDu7q=NbxJfb1z_yeAu6t-MqaBC+Q@mail.gmail.com
2021-09-06 09:25:45 +09:00
Tom Lane
65dc30ced6 Fix regexp misbehavior with capturing parens inside "{0}".
Regexps like "(.){0}...\1" drew an "invalid backreference number".
That's not unreasonable on its face, since the capture group will
never be matched if it's iterated zero times.  However, other engines
such as Perl's don't complain about this, nor do we throw an error for
related cases such as "(.)|\1", even though that backref can never
succeed either.  Also, if the zero-iterations case happens at runtime
rather than compile time --- say, "(x)*...\1" when there's no "x" to
be found --- that's not an error, we just deem the backref to not
match.  Making this even less defensible, no error was thrown for
nested cases such as "((.)){0}...\2"; and to add insult to injury,
those cases could result in assertion failures instead.  (It seems
that nothing especially bad happened in non-assert builds, though.)

Let's just fix it so that no error is thrown and instead the backref
is deemed to never match, so that compile-time detection of no
iterations behaves the same as run-time detection.

Per report from Mark Dilger.  This appears to be an aboriginal error
in Spencer's library, so back-patch to all supported versions.

Pre-v14, it turns out to also be necessary to back-patch one aspect of
commits cb76fbd7e/00116dee5, namely to create capture-node subREs with
the begin/end states of their subexpressions, not the current lp/rp
of the outer parseqatom invocation.  Otherwise delsub complains that
we're trying to disconnect a state from itself.  This is a bit scary
but code examination shows that it's safe: in the pre-v14 code, if we
want to wrap iteration around the subexpression, the first thing we do
is overwrite the atom's begin/end fields with new states.  So the
bogus values didn't survive long enough to be used for anything, except
if no iteration is required, in which case it doesn't matter.

Discussion: https://postgr.es/m/A099E4A8-4377-4C64-A98C-3DEDDC075502@enterprisedb.com
2021-08-24 16:37:26 -04:00
Tom Lane
9bbf6f7341 Prevent regexp back-refs from sometimes matching when they shouldn't.
The recursion in cdissect() was careless about clearing match data
for capturing parentheses after rejecting a partial match.  This
could allow a later back-reference to succeed when by rights it
should fail for lack of a defined referent.

To fix, think a little more rigorously about what the contract
between different levels of cdissect's recursion needs to be.
With the right spec, we can fix this using fewer rather than more
resets of the match data; the key decision being that a failed
sub-match is now explicitly responsible for clearing any matches
it may have set.

There are enough other cross-checks and optimizations in the code
that it's not especially easy to exhibit this problem; usually, the
match will fail as-expected.  Plus, regexps that are even potentially
vulnerable are most likely user errors, since there's just not much
point in writing a back-ref that doesn't always have a referent.
These facts perhaps explain why the issue hasn't been detected,
even though it's almost certainly a couple of decades old.

Discussion: https://postgr.es/m/151435.1629733387@sss.pgh.pa.us
2021-08-23 17:41:07 -04:00
Tom Lane
b66336c4e1 Reduce assumptions about locale's behavior in new regex tests.
I was overoptimistic to assume that UTF8-based locales would all
consider U+1500 to be a member of the [[:alpha:]] char class.
Tweak the test cases added by commit 78a843f11 to avoid that
assumption.  We might need to lobotomize them further, but this
should be enough to fix the early buildfarm reports.
2021-08-17 13:00:36 -04:00
Tom Lane
78a843f119 Improve regex compiler's arc moving/copying logic.
The functions moveins(), copyins(), moveouts(), copyouts() are
required to preserve the invariant that there are no duplicate arcs in
the regex's NFA.  Spencer's original implementation of them was O(N^2)
since it checked separately for a match to each source arc.  In commit
579840ca0 I improved that by adding sort/merge logic to be used if
more than a few arcs are to be moved/copied.  However, I now realize
that that missed a bet.  At many call sites, the target state is newly
made and cannot have any existing in-arcs (respectively out-arcs)
that could be duplicates.  So spending any cycles at all on checking
for duplicates is wasted effort; in these cases we can just blindly
move/copy all the source arcs.  Add code paths to do that.

It turns out that for copyins()/copyouts(), *all* the call sites have
this property, making all the "improved" logic in them flat out
unreachable.  Perhaps we'll need the full capability again someday,
so I just #ifdef'd those paths out rather than removing them entirely.

In passing, add a few test cases to improve code coverage in this
area as well as in regc_locale.c/regc_pg_locale.c.

Discussion: https://postgr.es/m/810272.1629064063@sss.pgh.pa.us
2021-08-17 12:00:02 -04:00
Tom Lane
0e6aa8747d Avoid determining regexp subexpression matches, when possible.
Identifying the precise match locations for parenthesized subexpressions
is a fairly expensive task given the way our regexp engine works, both
at regexp compile time (where we must create an optimized NFA for each
parenthesized subexpression) and at runtime (where determining exact
match locations requires laborious search).

Up to now we've made little attempt to optimize this situation.  This
patch identifies cases where we know at compile time that we won't
need to know subexpression match locations, and teaches the regexp
compiler to not bother creating per-subexpression regexps for
parenthesis pairs that are not referenced by backrefs elsewhere in
the regexp.  (To preserve semantics, we obviously still have to
pin down the match locations of backref references.)  Users could
have obtained the same results before this by being careful to
write "non capturing" parentheses wherever possible, but few people
bother with that.

Discussion: https://postgr.es/m/2219936.1628115334@sss.pgh.pa.us
2021-08-09 11:26:34 -04:00
Tom Lane
cc1868799c Fix use-after-free issue in regexp engine.
Commit cebc1d34e taught parseqatom() to optimize cases where a branch
contains only one, "messy", atom by getting rid of excess subRE nodes.
The way we really should do that is to keep the subRE built for the
"messy" child atom; but to avoid changing parseqatom's nominal API,
I made it delete that node after copying its fields to the outer subRE
made by parsebranch().  It seems that that actually worked at the time;
but it became dangerous after ea1268f63, because that later commit
allowed the lower invocation of parse() to return a subRE that was also
pointed to by some v->subs[] entry.  This meant we could wind up with a
dangling pointer in v->subs[], allowing a later backref to misbehave,
but only if that subRE struct had been reused in between.  So the damage
seems confined to cases like '((...))...(...\2'.

To fix, do what I should have done before and modify parseqatom's API
to make it possible for it to remove the caller's subRE instead of the
callee's.  That's safer because we know that subRE isn't complete yet,
so noplace else will have a pointer to it.

Per report from Mark Dilger.  Back-patch to v14 where the problematic
patches came in.

Discussion: https://postgr.es/m/0203588E-E609-43AF-9F4F-902854231EE7@enterprisedb.com
2021-08-07 22:27:13 -04:00
Andrew Dunstan
0d14019318
Silence perl warning about uninitialized value 2021-08-01 13:03:15 -04:00
Andrew Dunstan
201a76183e
Unify PostgresNode's new() and get_new_node() methods
There is only one constructor now for PostgresNode, with the idiomatic
name 'new'. The method is not exported by the class, and must be called
as "PostgresNode->new('name',[args])". All the TAP tests that use
PostgresNode are modified accordingly. Third party scripts will need
adjusting, which is a fairly mechanical process (I just used a sed
script).
2021-07-29 05:58:08 -04:00
Fujii Masao
0e1275fb07 Avoid using ambiguous word "non-negative" in error messages.
The error messages using the word "non-negative" are confusing
because it's ambiguous about whether it accepts zero or not.
This commit improves those error messages by replacing it with
less ambiguous word like "greater than zero" or
"greater than or equal to zero".

Also this commit added the note about the word "non-negative" to
the error message style guide, to help writing the new error messages.

When postgres_fdw option fetch_size was set to zero, previously
the error message "fetch_size requires a non-negative integer value"
was reported. This error message was outright buggy. Therefore
back-patch to all supported versions where such buggy error message
could be thrown.

Reported-by: Hou Zhijie
Author: Bharath Rupireddy
Reviewed-by: Kyotaro Horiguchi, Fujii Masao
Discussion: https://postgr.es/m/OS0PR01MB5716415335A06B489F1B3A8194569@OS0PR01MB5716.jpnprd01.prod.outlook.com
2021-07-28 01:20:16 +09:00
Peter Eisentraut
2b00db4fb0 Use l*_node() family of functions where appropriate
Instead of castNode(…, lfoo(…))

Author: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Discussion: https://www.postgresql.org/message-id/flat/87eecahraj.fsf@wibble.ilmari.org
2021-07-19 08:20:24 +02:00
Alvaro Herrera
ab09679429
libpq: Fix sending queries in pipeline aborted state
When sending queries in pipeline mode, we were careless about leaving
the connection in the right state so that PQgetResult would behave
correctly; trying to read further results after sending a query after
having read a result with an error would sometimes hang.  Fix by
ensuring internal libpq state is changed properly.  All the state
changes were being done by the callers of pqAppendCmdQueueEntry(); it
would have become too repetitious to have this logic in each of them, so
instead put it all in that function and relieve callers of the
responsibility.

Add a test to verify this case.  Without the code fix, this new test
hangs sometimes.

Also, document that PQisBusy() would return false when no queries are
pending result.  This is not intuitively obvious, and NULL would be
obtained by calling PQgetResult() at that point, which is confusing.
Wording by Boris Kolpackov.

In passing, fix bogus use of "false" to mean "0", per Ranier Vilela.

Backpatch to 14.

Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reported-by: Boris Kolpackov <boris@codesynthesis.com>
Discussion: https://postgr.es/m/boris.20210624103805@codesynthesis.com
2021-07-09 15:57:59 -04:00
Alvaro Herrera
b71a9cb31e
Fix libpq state machine in pipeline mode
The original coding required that PQpipelineSync had been called before
the first call to PQgetResult, and failure to do that would result in an
unexpected NULL result being returned.  Fix by setting the right state
when a query is sent, rather than leaving it unchanged and having
PQpipelineSync apply the necessary state change.

A new test case to verify the behavior is added, which relies on the new
PQsendFlushRequest() function added by commit a7192326c7.

Backpatch to 14, where pipeline mode was added.

Reported-by: Boris Kolpackov <boris@codesynthesis.com>
Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/boris.20210616110321@codesynthesis.com
2021-06-29 15:01:29 -04:00
Noah Misch
a7a7be1f2f Dump public schema ownership and security labels.
As a side effect, this corrects dumps of public schema ACLs in databases
where the DBA dropped and recreated that schema.

Reviewed by Asif Rehman.

Discussion: https://postgr.es/m/20201229134924.GA1431748@rfd.leadboat.com
2021-06-28 18:34:55 -07:00
Tom Lane
4a054069a3 Improve display of query results in isolation tests.
Previously, isolationtester displayed SQL query results using some
ad-hoc code that clearly hadn't had much effort expended on it.
Field values longer than 14 characters weren't separated from
the next field, and usually caused misalignment of the columns
too.  Also there was no visual separation of a query's result
from subsequent isolationtester output.  This made test result
files confusing and hard to read.

To improve matters, let's use libpq's PQprint() function.  Although
that's long since unused by psql, it's still plenty good enough
for the purpose here.

Like 741d7f104, back-patch to all supported branches, so that this
isn't a stumbling block for back-patching isolation test changes.

Discussion: https://postgr.es/m/582362.1623798221@sss.pgh.pa.us
2021-06-23 11:13:00 -04:00
Alvaro Herrera
4efcf47053
Add 'Portal Close' message to pipelined PQsendQuery()
Commit acb7e4eb6b added a new implementation for PQsendQuery so that
it works in pipeline mode (by using extended query protocol), but it
behaves differently from the 'Q' message (in simple query protocol) used
by regular implementation: the new one doesn't close the unnamed portal.
Change the new code to have identical behavior to the old.

Reported-by: Yura Sokolov <y.sokolov@postgrespro.ru>
Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/202106072107.d4i55hdscxqj@alvherre.pgsql
2021-06-11 16:05:50 -04:00
Noah Misch
d0e750c0ac Rename PQtraceSetFlags() to PQsetTraceFlags().
We have a dozen PQset*() functions.  PQresultSetInstanceData() and this
were the libpq setter functions having a different word order.  Adopt
the majority word order.

Reviewed by Alvaro Herrera and Robert Haas, though this choice of name
was not unanimous.

Discussion: https://postgr.es/m/20210605060555.GA216695@rfd.leadboat.com
2021-06-10 21:56:13 -07:00
Noah Misch
42344ad0ed Add Windows file version information to libpq_pipeline.exe. 2021-06-01 18:04:15 -07:00