postgresql.git
3 years agoPrevent ALTER TYPE/DOMAIN/OPERATOR from changing extension membership.
Tom Lane [Tue, 17 Aug 2021 18:29:22 +0000 (14:29 -0400)]
Prevent ALTER TYPE/DOMAIN/OPERATOR from changing extension membership.

If recordDependencyOnCurrentExtension is invoked on a pre-existing,
free-standing object during an extension update script, that object
will become owned by the extension.  In our current code this is
possible in three cases:

* Replacing a "shell" type or operator.
* CREATE OR REPLACE overwriting an existing object.
* ALTER TYPE SET, ALTER DOMAIN SET, and ALTER OPERATOR SET.

The first of these cases is intentional behavior, as noted by the
existing comments for GenerateTypeDependencies.  It seems like
appropriate behavior for CREATE OR REPLACE too; at least, the obvious
alternatives are not better.  However, the fact that it happens during
ALTER is an artifact of trying to share code (GenerateTypeDependencies
and makeOperatorDependencies) between the CREATE and ALTER cases.
Since an extension script would be unlikely to ALTER an object that
didn't already belong to the extension, this behavior is not very
troubling for the direct target object ... but ALTER TYPE SET will
recurse to dependent domains, and it is very uncool for those to
become owned by the extension if they were not already.

Let's fix this by redefining the ALTER cases to never change extension
membership, full stop.  We could minimize the behavioral change by
only changing the behavior when ALTER TYPE SET is recursing to a
domain, but that would complicate the code and it does not seem like
a better definition.

Per bug #17144 from Alex Kozhemyakin.  Back-patch to v13 where ALTER
TYPE SET was added.  (The other cases are older, but since they only
affect the directly-named object, there's not enough of a problem to
justify changing the behavior further back.)

Discussion: https://postgr.es/m/17144-e67d7a8f049de9af@postgresql.org

3 years agoReduce assumptions about locale's behavior in new regex tests.
Tom Lane [Tue, 17 Aug 2021 17:00:36 +0000 (13:00 -0400)]
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.

3 years agoImprove regex compiler's arc moving/copying logic.
Tom Lane [Tue, 17 Aug 2021 16:00:02 +0000 (12:00 -0400)]
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

3 years agoImproved ECPG warning as suggested by Michael Paquier and removed test case
Michael Meskes [Tue, 17 Aug 2021 12:58:33 +0000 (14:58 +0200)]
Improved ECPG warning as suggested by Michael Paquier and removed test case
that triggers the warning during regression tests.

3 years agoSet type identifier on BIO
Daniel Gustafsson [Tue, 17 Aug 2021 12:27:37 +0000 (14:27 +0200)]
Set type identifier on BIO

In OpenSSL there are two types of BIO's (I/O abstractions):
source/sink and filters. A source/sink BIO is a source and/or
sink of data, ie one acting on a socket or a file. A filter
BIO takes a stream of input from another BIO and transforms it.
In order for BIO_find_type() to be able to traverse the chain
of BIO's and correctly find all BIO's of a certain type they
shall have the type bit set accordingly, source/sink BIO's
(what PostgreSQL implements) use BIO_TYPE_SOURCE_SINK and
filter BIO's use BIO_TYPE_FILTER. In addition to these, file
descriptor based BIO's should have the descriptor bit set,
BIO_TYPE_DESCRIPTOR.

The PostgreSQL implementation didn't set the type bits, which
went unnoticed for a long time as it's only really relevant
for code auditing the OpenSSL installation, or doing similar
tasks. It is required by the API though, so this fixes it.

Backpatch through 9.6 as this has been wrong for a long time.

Author: Itamar Gafni
Discussion: https://postgr.es/m/SN6PR06MB39665EC10C34BB20956AE4578AF39@SN6PR06MB3966.namprd06.prod.outlook.com
Backpatch-through: 9.6

3 years agodoc: \123 and \x12 escapes in COPY are in database encoding.
Heikki Linnakangas [Tue, 17 Aug 2021 07:00:06 +0000 (10:00 +0300)]
doc: \123 and \x12 escapes in COPY are in database encoding.

The backslash sequences, including \123 and \x12 escapes, are interpreted
after encoding conversion. The docs failed to mention that.

Backpatch to all supported versions.

Reported-by: Andreas Grob
Discussion: https://www.postgresql.org/message-id/17142-9181542ca1df75ab%40postgresql.org

3 years agoRevert analyze support for partitioned tables
Alvaro Herrera [Mon, 16 Aug 2021 21:27:52 +0000 (17:27 -0400)]
Revert analyze support for partitioned tables

This reverts the following commits:
1b5617eb844cd2470a334c1d2eec66cf9b39c41a Describe (auto-)analyze behavior for partitioned tables
0e69f705cc1a3df273b38c9883fb5765991e04fe Set pg_class.reltuples for partitioned tables
41badeaba8beee7648ebe7923a41c04f1f3cb302 Document ANALYZE storage parameters for partitioned tables
0827e8af70f4653ba17ed773f123a60eadd9f9c9 autovacuum: handle analyze for partitioned tables

There are efficiency issues in this code when handling databases with
large numbers of partitions, and it doesn't look like there isn't any
trivial way to handle those.  There are some other issues as well.  It's
now too late in the cycle for nontrivial fixes, so we'll have to let
Postgres 14 users continue to manually deal with ANALYZE their
partitioned tables, and hopefully we can fix the issues for Postgres 15.

I kept [most of] be280cdad298 ("Don't reset relhasindex for partitioned
tables on ANALYZE") because while we added it due to 0827e8af70f4, it is
a good bugfix in its own right, since it affects manual analyze as well
as autovacuum-induced analyze, and there's no reason to revert it.

I retained the addition of relkind 'p' to tables included by
pg_stat_user_tables, because reverting that would require a catversion
bump.
Also, in pg14 only, I keep a struct member that was added to
PgStat_TabStatEntry to avoid breaking compatibility with existing stat
files.

Backpatch to 14.

Discussion: https://postgr.es/m/20210722205458.f2bug3z6qzxzpx2s@alap3.anarazel.de

3 years agoReduce memory consumption for pending invalidation messages.
Tom Lane [Mon, 16 Aug 2021 20:48:25 +0000 (16:48 -0400)]
Reduce memory consumption for pending invalidation messages.

The existing data structures in inval.c are fairly inefficient for
the common case of a command or subtransaction that registers a small
number of cache invalidation events.  While this doesn't matter if we
commit right away, it can build up to a lot of bloat in a transaction
that contains many DDL operations.  By making a few more assumptions
about the expected use-case, we can switch to a representation using
densely-packed arrays.  Although this eliminates some data-copying,
it doesn't seem to make much difference time-wise.  But the space
consumption decreases substantially.

Patch by me; thanks to Nathan Bossart for review.

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

3 years agoEmit namespace in the post-copy errmsg
Daniel Gustafsson [Mon, 16 Aug 2021 18:06:54 +0000 (20:06 +0200)]
Emit namespace in the post-copy errmsg

During a VACUUM or CLUSTER command, the initial output emits a
fully qualified relation path with namespace.  The post-action
errmsg only emitted the relation name however, which may lead
to hard to parse output when using multiple jobs with vacuumdb
as the output from different jobs may be interleaved.  Include
the full path in the post-action errmsg to be consistent with
the initial errmsg.

Author: Mike Fiedler <miketheman@gmail.com>
Reviewed-by: Corey Huinker <corey.huinker@gmail.com>
Discussion: https://postgr.es/m/CAMerE0oz+8G-aORZL_BJcPxnBqewZAvND4bSUysjz+r-oT1BxQ@mail.gmail.com

3 years agoUse direct function calls for pg_popcount{32,64} on non-x86 platforms
John Naylor [Mon, 16 Aug 2021 15:45:21 +0000 (11:45 -0400)]
Use direct function calls for pg_popcount{32,64} on non-x86 platforms

Previously, all pg_popcount{32,64} calls were indirected through
a function pointer, even though we had no fast implementation for
non-x86 platforms. Instead, for those platforms use wrappers around
the pg_popcount{32,64}_slow functions.

Review and additional hacking by David Rowley
Reviewed by Álvaro Herrera

Discussion: https://www.postgresql.org/message-id/flat/CAFBsxsE7otwnfA36Ly44zZO%2Bb7AEWHRFANxR1h1kxveEV%3DghLQ%40mail.gmail.com

3 years agoClarify initdb --sync-only help message and docs
Daniel Gustafsson [Mon, 16 Aug 2021 11:38:01 +0000 (13:38 +0200)]
Clarify initdb --sync-only help message and docs

The initdb help message for --sync-only was a bit terse, and not
really self-explanatory. Make it clearer that initdb --sync-only
will exit after syncing, and expand the docs with a note on when
the option can be useful. Also align the help output with others
that exit immediately.

Author: Nathan Bossart, Gurjeet Singh
Discussion: https://postgr.es/m/CABwTF4U6hbNNE1bv=LxQdJybmUdZ5NJQ9rKY9tN82NXM8QH+iQ@mail.gmail.com

3 years agoRefresh apply delay on reload of recovery_min_apply_delay at recovery
Michael Paquier [Mon, 16 Aug 2021 03:10:22 +0000 (12:10 +0900)]
Refresh apply delay on reload of recovery_min_apply_delay at recovery

This commit ensures that the wait interval in the replay delay loop
waiting for an amount of time defined by recovery_min_apply_delay is
correctly handled on reload, recalculating the delay if this GUC value
is updated, based on the timestamp of the commit record being replayed.

The previous behavior would be problematic for example with replay
still waiting even if the delay got reduced or just cancelled.  If the
apply delay was increased to a larger value, the wait would have just
respected the old value set, finishing earlier.

Author: Soumyadeep Chakraborty, Ashwin Agrawal
Reviewed-by: Kyotaro Horiguchi, Michael Paquier
Discussion: https://postgr.es/m/CAE-ML+93zfr-HLN8OuxF0BjpWJ17O5dv1eMvSE5jsj9jpnAXZA@mail.gmail.com
Backpatch-through: 9.6

3 years agoUn-break s_lock_test.
Tom Lane [Fri, 13 Aug 2021 18:42:27 +0000 (14:42 -0400)]
Un-break s_lock_test.

Commit 80abbeba2 evidently didn't bother checking this code.
Also, list the generated executable in .gitignore (so it's
been a REALLY long time since anyone tried this).

Noted while trying out RISC-V spinlock patch.  Given that
this has been broken for 5 years and nobody noticed, it's
likely not worth back-patching.

3 years agoAdd RISC-V spinlock support in s_lock.h.
Tom Lane [Fri, 13 Aug 2021 17:58:47 +0000 (13:58 -0400)]
Add RISC-V spinlock support in s_lock.h.

Like the ARM case, just use gcc's __sync_lock_test_and_set();
that will compile into AMOSWAP.W.AQ which does what we need.

At some point it might be worth doing some work on atomic ops
for RISC-V, but this should be enough for a creditable port.

Back-patch to all supported branches, just in case somebody
wants to try them on RISC-V.

Marek Szuba

Discussion: https://postgr.es/m/dea97b6d-f55f-1f6d-9109-504aa7dfa421@gentoo.org

3 years agopg_amcheck: Message style and structuring improvements
Peter Eisentraut [Fri, 13 Aug 2021 15:15:03 +0000 (17:15 +0200)]
pg_amcheck: Message style and structuring improvements

3 years agoRemove support for background workers without BGWORKER_SHMEM_ACCESS.
Andres Freund [Fri, 13 Aug 2021 12:49:26 +0000 (05:49 -0700)]
Remove support for background workers without BGWORKER_SHMEM_ACCESS.

Background workers without shared memory access have been broken on
EXEC_BACKEND / windows builds since shortly after background workers have been
introduced, without that being reported. Clearly they are not commonly used.

The problem is that bgworker startup requires to be attached to shared memory
in EXEC_BACKEND child processes. StartBackgroundWorker() detaches from shared
memory for unconnected workers, but at that point we already have initialized
subsystems referencing shared memory.

Fixing this problem is not entirely trivial, so removing the option to not be
connected to shared memory seems the best way forward. In most use cases the
advantages of being connected to shared memory far outweigh the disadvantages.

As there have been no reports about this issue so far, we have decided that it
is not worth trying to address the problem in the back branches.

Per discussion with Alvaro Herrera, Robert Haas and Tom Lane.

Author: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/20210802065116.j763tz3vz4egqy3w@alap3.anarazel.de

3 years agoFix typo.
Andres Freund [Fri, 13 Aug 2021 12:44:03 +0000 (05:44 -0700)]
Fix typo.

Reported-By: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/YRIlNQhLNfx555Nx@paquier.xyz

3 years agoFix connection handling for DEALLOCATE and DESCRIBE statements
Michael Meskes [Fri, 13 Aug 2021 08:34:04 +0000 (10:34 +0200)]
Fix connection handling for DEALLOCATE and DESCRIBE statements

After binding a statement to a connection with DECLARE STATEMENT the connection
was still not used for DEALLOCATE and DESCRIBE statements. This patch fixes
that, adds a missing warning and cleans up the code.

Author: Hayato Kuroda
Reviewed-by: Kyotaro Horiguchi, Michael Paquier
Discussion: https://postgr.es/m/TYAPR01MB5866BA57688DF2770E2F95C6F5069%40TYAPR01MB5866.jpnprd01.prod.outlook.com

3 years agoFix sslsni connparam boolean check
Daniel Gustafsson [Fri, 13 Aug 2021 08:32:17 +0000 (10:32 +0200)]
Fix sslsni connparam boolean check

The check for sslsni only checked for existence of the parameter
but not for the actual value of the param.  This meant that the
SNI extension was always turned on.  Fix by inspecting the value
of sslsni and only activate the SNI extension iff sslsni has been
enabled.  Also update the docs to be more in line with how other
boolean params are documented.

Backpatch to 14 where sslsni was first implemented.

Reviewed-by: Tom Lane
Backpatch-through: 14, where sslni was added

3 years agoFix incorrect hash table resizing code in simplehash.h
David Rowley [Fri, 13 Aug 2021 04:41:26 +0000 (16:41 +1200)]
Fix incorrect hash table resizing code in simplehash.h

This fixes a bug in simplehash.h which caused an incorrect size mask to be
used when the hash table grew to SH_MAX_SIZE (2^32).  The code was
incorrectly setting the size mask to 0 when the hash tables reached the
maximum possible number of buckets.  This would result always trying to
use the 0th bucket causing an  infinite loop of trying to grow the hash
table due to there being too many collisions.

Seemingly it's not that common for simplehash tables to ever grow this big
as this bug dates back to v10 and nobody seems to have noticed it before.
However, probably the most likely place that people would notice it would
be doing a large in-memory Hash Aggregate with something close to at least
2^31 groups.

After this fix, the code now works correctly with up to within 98% of 2^32
groups and will fail with the following error when trying to insert any
more items into the hash table:

ERROR:  hash table size exceeded

However, the work_mem (or hash_mem_multiplier in newer versions) settings
will generally cause Hash Aggregates to spill to disk long before reaching
that many groups.  The minimal test case I did took a work_mem setting of
over 192GB to hit the bug.

simplehash hash tables are used in a few other places such as Bitmap Index
Scans, however, again the size that the hash table can become there is
also limited to work_mem and it would take a relation of around 16TB
(2^31) pages and a very large work_mem setting to hit this.  With smaller
work_mem values the table would become lossy and never grow large enough
to hit the problem.

Author: Yura Sokolov
Reviewed-by: David Rowley, Ranier Vilela
Discussion: https://postgr.es/m/b1f7f32737c3438136f64b26f4852b96@postgrespro.ru
Backpatch-through: 10, where simplehash.h was added

3 years agoMake EXEC_BACKEND more convenient on macOS.
Thomas Munro [Thu, 12 Aug 2021 22:38:22 +0000 (10:38 +1200)]
Make EXEC_BACKEND more convenient on macOS.

It's hard to disable ASLR on current macOS releases, for testing with
-DEXEC_BACKEND.  You could already set the environment variable
PG_SHMEM_ADDR to something not likely to collide with mappings created
earlier in process startup.  Let's also provide a default value that
works on current releases and architectures, for developer convenience.

As noted in the pre-existing comment, this is a horrible hack, but
-DEXEC_BACKEND is only used by Unix-based PostgreSQL developers for
testing some otherwise Windows-only code paths, so it seems excusable.

Back-patch to all supported branches.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/20210806032944.m4tz7j2w47mant26%40alap3.anarazel.de

3 years agoUse appropriate tuple descriptor in FDW batching
Tomas Vondra [Thu, 12 Aug 2021 19:32:53 +0000 (21:32 +0200)]
Use appropriate tuple descriptor in FDW batching

The FDW batching code was using the same tuple descriptor both for all
slots (regular and plan slots), but that's incorrect - the subplan may
use a different descriptor. Currently this is benign, because batching
is used only for INSERTs, and in that case the descriptors always match.
But that would change if we allow batching UPDATEs.

Fix by copying the appropriate tuple descriptor. Backpatch to 14, where
the FDW batching was implemented.

Author: Amit Langote
Backpatch-through: 14, where FDW batching was added
Discussion: https://postgr.es/m/CA%2BHiwqEWd5B0-e-RvixGGUrNvGkjH2s4m95%3DJcwUnyV%3Df0rAKQ%40mail.gmail.com

3 years agoSpeed up generation of Unicode hash functions.
John Naylor [Thu, 12 Aug 2021 13:08:56 +0000 (09:08 -0400)]
Speed up generation of Unicode hash functions.

Sets of Unicode keys are picky about the primes used when generating
a perfect hash function for them. Callers can spend many seconds
iterating through all the possible combinations of candidate
multipliers and seeds to find one that works.

Unicode updates typically happen only once a year, but it still makes
development and testing of Unicode scripts unnecessarily slow. To fix,
iterate over the primes in the innermost loop. This does not change
any existing functions checked into the tree.

3 years agoFix grammar mistake in hash index README
John Naylor [Thu, 12 Aug 2021 12:53:41 +0000 (08:53 -0400)]
Fix grammar mistake in hash index README

Dilip Kumar

Discussion: https://www.postgresql.org/message-id/CAFiTN-tjZbuY6vy7kZZ6xO%2BD4mVcO5wOPB5KiwJ3AHhpytd8fg%40mail.gmail.com

3 years agoAvoid unnecessary shared invalidations in ROLLBACK PREPARED
Michael Paquier [Thu, 12 Aug 2021 11:12:47 +0000 (20:12 +0900)]
Avoid unnecessary shared invalidations in ROLLBACK PREPARED

The performance gain is minimal, but this makes the logic more
consistent with AtEOXact_Inval().  No other invalidation is needed in
this case as PREPARE takes already care of sending any local ones.

Author: Liu Huailing
Reviewed-by: Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/OSZPR01MB6215AA84D71EF2B3D354CF86BE139@OSZPR01MB6215.jpnprd01.prod.outlook.com

3 years agoFix segfault during EvalPlanQual with mix of local and foreign partitions.
Heikki Linnakangas [Thu, 12 Aug 2021 08:02:29 +0000 (11:02 +0300)]
Fix segfault during EvalPlanQual with mix of local and foreign partitions.

It's not sensible to re-evaluate a direct-modify Foreign Update or Delete
during EvalPlanQual. However, ExecInitForeignScan() can still get called
if a table mixes local and foreign partitions. EvalPlanQualStart() left
the es_result_relations array uninitialized in the child EPQ EState, but
ExecInitForeignScan() still expected to find it. That caused a segfault.

Fix by skipping the es_result_relations lookup during EvalPlanQual
processing. To make things a bit more robust, also skip the
BeginDirectModify calls, and add a runtime check that ExecForeignScan()
is not called on direct-modify foreign scans during EvalPlanQual
processing.

This is new in v14, commit 1375422c782. Before that, EvalPlanQualStart()
copied the whole ResultRelInfo array to the EPQ EState. Backpatch to v14.

Report and diagnosis by Andrey Lepikhov.

Discussion: https://www.postgresql.org/message-id/cb2b808d-cbaa-4772-76ee-c8809bafcf3d%40postgrespro.ru

3 years agoFix failure of btree_gin indexscans with "char" type and </<= operators.
Tom Lane [Tue, 10 Aug 2021 22:10:29 +0000 (18:10 -0400)]
Fix failure of btree_gin indexscans with "char" type and </<= operators.

As a result of confusion about whether the "char" type is signed or
unsigned, scans for index searches like "col < 'x'" or "col <= 'x'"
would start at the middle of the index not the left end, thus missing
many or all of the entries they should find.  Fortunately, this
is not a symptom of index corruption.  It's only the search logic
that is broken, and we can fix it without unpleasant side-effects.

Per report from Jason Kim.  This has been wrong since btree_gin's
beginning, so back-patch to all supported branches.

Discussion: https://postgr.es/m/20210810001649.htnltbh7c63re42p@jasonk.me

3 years agoAdd alternative output for OpenSSL 3 without legacy loaded
Daniel Gustafsson [Tue, 10 Aug 2021 13:08:46 +0000 (15:08 +0200)]
Add alternative output for OpenSSL 3 without legacy loaded

OpenSSL 3 introduced the concept of providers to support modularization,
and moved the outdated ciphers to the new legacy provider. In case it's
not loaded in the users openssl.cnf file there will be a lot of regress
test failures, so add alternative outputs covering those.

Also document the need to load the legacy provider in order to use older
ciphers with OpenSSL-enabled pgcrypto.

This will be backpatched to all supported version once there is sufficient
testing in the buildfarm of OpenSSL 3.

Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/FEF81714-D479-4512-839B-C769D2605F8A@yesql.se

3 years agoDisable OpenSSL EVP digest padding in pgcrypto
Daniel Gustafsson [Tue, 10 Aug 2021 13:01:52 +0000 (15:01 +0200)]
Disable OpenSSL EVP digest padding in pgcrypto

The PX layer in pgcrypto is handling digest padding on its own uniformly
for all backend implementations. Starting with OpenSSL 3.0.0, DecryptUpdate
doesn't flush the last block in case padding is enabled so explicitly
disable it as we don't use it.

This will be backpatched to all supported version once there is sufficient
testing in the buildfarm of OpenSSL 3.

Reviewed-by: Peter Eisentraut, Michael Paquier
Discussion: https://postgr.es/m/FEF81714-D479-4512-839B-C769D2605F8A@yesql.se

3 years agoRemove unused regression test certificate server-ss
Daniel Gustafsson [Tue, 10 Aug 2021 09:15:02 +0000 (11:15 +0200)]
Remove unused regression test certificate server-ss

The server-ss certificate was included in e39250c64 but was never
used in the TLS regression tests so remove.

Author: Jacob Champion
Discussion: https://postgr.es/m/d15a9838344ba090e09fd866abf913584ea19fb7.camel@vmware.com

3 years agoAdd tab completion for DECLARE .. ASENSITIVE in psql
Michael Paquier [Tue, 10 Aug 2021 06:54:42 +0000 (15:54 +0900)]
Add tab completion for DECLARE .. ASENSITIVE in psql

This option has been introduced in dd13ad9.

Author: Shinya Kato
Discussion: https://postgr.es/m/TYAPR01MB289665526B76DA29DC70A031C4F09@TYAPR01MB2896.jpnprd01.prod.outlook.com

3 years agoFix regression test output of sepgsql
Michael Paquier [Tue, 10 Aug 2021 04:14:37 +0000 (13:14 +0900)]
Fix regression test output of sepgsql

The difference is caused by 7b56584, for the tests involving a table
rewrite.

Per buildfarm member rhinoceros.

Discussion: https://postgr.es/m/YRHxXcyFjPuPTZui@paquier.xyz

3 years agoAdd call to object access hook at the end of table rewrite in ALTER TABLE
Michael Paquier [Tue, 10 Aug 2021 03:21:05 +0000 (12:21 +0900)]
Add call to object access hook at the end of table rewrite in ALTER TABLE

ALTER TABLE .. SET {LOGGED,UNLOGGED,ACCESS METHOD} would never do a
table-level object access hook, which was inconsistent with SET
TABLESPACE.  Note that contrary to SET TABLESPACE, the no-op case is
left off for those commands as this requires tracking if commands have
been called, but they may not execute a physical rewrite.  Another thing
worth noting is that the physical file swap at the end of a rewrite
does a couple of access calls for internal objects created for the swap
operation (internal objects are for example skipped by the tests of
sepgsql), but this does not trigger the hook for the table on which the
operation is done.

f41872d, that added support for SET LOGGED/UNLOGGED in ALTER TABLE,
visibly forgot to consider that.

Based on what I checked, two regression tests of sepgsql in ddl.sql are
going to log more information with this test, something that buildfarm
member rhinoceros will tell soon enough.  I am not completely sure of
their format though, so these are not refreshed yet.

This is arguably a bug, but no backpatch is done as this could cause a
behavior change for anybody using object access hooks.

Reported-by: Jeff Davis
Discussion: https://postgr.es/m/YQJKV29/1a60uG68@paquier.xyz

3 years agoLet regexp_replace() make use of REG_NOSUB when feasible.
Tom Lane [Tue, 10 Aug 2021 00:53:25 +0000 (20:53 -0400)]
Let regexp_replace() make use of REG_NOSUB when feasible.

If the replacement string doesn't contain \1...\9, then we don't
need sub-match locations, so we can use the REG_NOSUB optimization
here too.  There's already a pre-scan of the replacement string
to look for backslashes, so extend that to check for digits, and
refactor to allow that to happen before we compile the regexp.

While at it, try to speed up the pre-scan by using memchr() instead
of a handwritten loop.  It's likely that this is lost in the noise
compared to the regexp processing proper, but maybe not.  In any
case, this coding is shorter.

Also, add some test cases to improve the poor coverage of
appendStringInfoRegexpSubstr().

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

3 years agoFix bogus assertion in BootstrapModeMain().
Andres Freund [Mon, 9 Aug 2021 15:26:59 +0000 (08:26 -0700)]
Fix bogus assertion in BootstrapModeMain().

The assertion was always true, as written, thanks to me "simplifying" it
before commit.

Per coverity and Tom Lane.

3 years agoAvoid determining regexp subexpression matches, when possible.
Tom Lane [Mon, 9 Aug 2021 15:26:34 +0000 (11:26 -0400)]
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

3 years agoRemove some special cases from MSVC build scripts
David Rowley [Mon, 9 Aug 2021 07:45:26 +0000 (19:45 +1200)]
Remove some special cases from MSVC build scripts

Here we add additional parsing of Makefiles to determine when to add
references to libpgport and libpgcommon.  We also remove the need for
adding the current contrib_extrasource by adding sine very basic logic to
implement the Makefile rules which add .l and .y files when they exist for
a given .o file in the Makefile.

This is just some very basic additional parsing of Makefiles to try to
keep things more consistent between builds using make and MSVC builds.
This happens to work with how our current Makefiles are laid out, but it
could easily be broken in the future if someone chooses do something in
the Makefile that we don't have parsing support for.  We will cross that
bridge when we come to it.

Author: David Rowley
Discussion: https://postgr.es/m/CAApHDvoPULi5JW3933NxgwxOmu9Ncvpcyt87UhEHAUX16QqmpA@mail.gmail.com

3 years agoDoc: Fix misleading statement about VACUUM memory limits
David Rowley [Mon, 9 Aug 2021 04:45:35 +0000 (16:45 +1200)]
Doc: Fix misleading statement about VACUUM memory limits

In ec34040af I added a mention that there was no point in setting
maintenance_work_limit to anything higher than 1GB for vacuum, but that
was incorrect as ginInsertCleanup() also looks at what
maintenance_work_mem is set to during VACUUM and that's not limited to
1GB.

Here I attempt to make it more clear that the limitation is only around
the number of dead tuple identifiers that we can collect during VACUUM.

I've also added a note to autovacuum_work_mem to mention this limitation.
I didn't do that in ec34040af as I'd had some wrong-headed ideas about
just limiting the maximum value for that GUC to 1GB.

Author: David Rowley
Discussion: https://postgr.es/m/CAApHDvpGwOAvunp-E-bN_rbAs3hmxMoasm5pzkYDbf36h73s7w@mail.gmail.com
Backpatch-through: 9.6, same as ec34040af

3 years agoUse ExplainPropertyInteger for queryid in EXPLAIN
David Rowley [Mon, 9 Aug 2021 03:47:00 +0000 (15:47 +1200)]
Use ExplainPropertyInteger for queryid in EXPLAIN

This saves a few lines of code.  Also add a comment to mention why we use
ExplainPropertyInteger instead of ExplainPropertyUInteger given that
queryid is a uint64 type.

Author: David Rowley
Reviewed-by: Julien Rouhaud
Discussion: https://postgr.es/m/CAApHDvqhSLYpSU_EqUdN39w9Uvb8ogmHV7_3YhJ0S3aScGBjsg@mail.gmail.com
Backpatch-through: 14, where this code was originally added

3 years agoFix typo in 022_twophase_cascade.pl.
Amit Kapila [Mon, 9 Aug 2021 03:28:38 +0000 (08:58 +0530)]
Fix typo in 022_twophase_cascade.pl.

Author: Peter Smith
Discussion: https://postgr.es/m/CAHut+Pta=zo8G1DWVVg-LU6b_JvHHCueC=AKVpKJOrwLzj9EZA@mail.gmail.com

3 years agoAdd POPCNT support for MSVC x86_64 builds
David Rowley [Mon, 9 Aug 2021 03:23:48 +0000 (15:23 +1200)]
Add POPCNT support for MSVC x86_64 builds

02a6a54ec added code to make use of the POPCNT instruction when available
for many of our common platforms.  Here we do the same for MSVC for x86_64
machines.

MSVC's intrinsic functions for popcnt seem to differ from GCCs in that
they always appear to emit the popcnt instructions.  In GCC the behavior
will depend on if the source file was compiled with -mpopcnt or not.  For
this reason, the MSVC intrinsic function has been lumped into the
pg_popcount*_asm function, however doing that sort of invalidates the name
of that function, so let's rename it to pg_popcount*_fast().

Author: David Rowley
Reviewed-by: John Naylor
Discussion: https://postgr.es/m/CAApHDvqL3cbbK%3DGzNcwzsNR9Gi%2BaUvTudKkC4XgnQfXirJ_oRQ%40mail.gmail.com

3 years agodoc: mention pg_upgrade extension script
Bruce Momjian [Mon, 9 Aug 2021 01:05:46 +0000 (21:05 -0400)]
doc:  mention  pg_upgrade extension script

Since commit e462856a7a, pg_upgrade automatically creates a script to
update extensions, so mention that instead of ALTER EXTENSION.

Backpatch-through: 9.6

3 years agoRemove some unnecessary casts in format arguments
Peter Eisentraut [Sun, 8 Aug 2021 20:05:42 +0000 (22:05 +0200)]
Remove some unnecessary casts in format arguments

We can use %zd or %zu directly, no need to cast to int.  Conversely,
some code was casting away from int when it could be using %d
directly.

3 years agoDoc: remove bogus <indexterm> items.
Tom Lane [Sun, 8 Aug 2021 19:35:30 +0000 (15:35 -0400)]
Doc: remove bogus <indexterm> items.

Copy-and-pasteo in 665c5855e, evidently.  The 9.6 docs toolchain
whined about duplicate index entries, though our modern toolchain
doesn't.  In any case, these GUCs surely are not about the
default settings of these values.

3 years agoCheck the size in COPY_POINTER_FIELD
Peter Eisentraut [Sun, 8 Aug 2021 14:55:51 +0000 (16:55 +0200)]
Check the size in COPY_POINTER_FIELD

instead of making each caller do it.

Discussion: https://www.postgresql.org/message-id/flat/c1097590-a6a4-486a-64b1-e1f9cc0533ce@enterprisedb.com

3 years agoChange NestPath node to contain JoinPath node
Peter Eisentraut [Sun, 8 Aug 2021 14:55:51 +0000 (16:55 +0200)]
Change NestPath node to contain JoinPath node

This makes the structure of all JoinPath-derived nodes the same,
independent of whether they have additional fields.

Discussion: https://www.postgresql.org/message-id/flat/c1097590-a6a4-486a-64b1-e1f9cc0533ce@enterprisedb.com

3 years agoChange SeqScan node to contain Scan node
Peter Eisentraut [Sun, 8 Aug 2021 14:55:51 +0000 (16:55 +0200)]
Change SeqScan node to contain Scan node

This makes the structure of all Scan-derived nodes the same,
independent of whether they have additional fields.

Discussion: https://www.postgresql.org/message-id/flat/c1097590-a6a4-486a-64b1-e1f9cc0533ce@enterprisedb.com

3 years agoRethink regexp engine's backref-related compilation state.
Tom Lane [Sun, 8 Aug 2021 15:56:29 +0000 (11:56 -0400)]
Rethink regexp engine's backref-related compilation state.

I had committer's remorse almost immediately after pushing cb76fbd7e,
upon finding that removing capturing subexpressions' subREs from the
data structure broke my proposed patch for REG_NOSUB optimization.
Revert that data structure change.  Instead, address the concern
about not changing capturing subREs' endpoints by not changing the
endpoints.  We don't need to, because the point of that bit was just
to ensure that the atom has endpoints distinct from the outer state
pair that we're stringing the branch between.  We already made
suitable states in the parenthesized-subexpression case, so the
additional ones were just useless overhead.  This seems more
understandable than Spencer's original coding, and it ought to be
a shade faster too by saving a few state creations and arc changes.
(I actually see a couple percent improvement on Jacobson's web
corpus, though that's barely above the noise floor so I wouldn't
put much stock in that result.)

Also, fix the logic added by ea1268f63 to ensure that the subRE
recorded in v->subs[subno] is exactly the one with capno == subno.
Spencer's original coding recorded the child subRE of the capture
node, which is okay so far as having the right endpoint states is
concerned, but as of cb76fbd7e the capturing subRE itself always
has those endpoints too.  I think the inconsistency is confusing
for the REG_NOSUB optimization.

As before, backpatch to v14.

Discussion: https://postgr.es/m/0203588E-E609-43AF-9F4F-902854231EE7@enterprisedb.com

3 years agoRemove unused function declaration
David Rowley [Sun, 8 Aug 2021 11:27:57 +0000 (23:27 +1200)]
Remove unused function declaration

It appears that check_track_commit_timestamp was declared but has never
been defined in our code base.  Likely this is just leftover cruft from
a development version of the original patch to add commit timestamps.

Let's just remove the useless declaration.  The inclusion of guc.h also
seems surplus to requirements.

Author: Andrey Lepikhov
Discussion: https://postgr.es/m/f49aefb5-edbb-633a-af07-3e777023a94d@postgrespro.ru

3 years agoMake regexp engine's backref-related compilation state more bulletproof.
Tom Lane [Sun, 8 Aug 2021 02:27:02 +0000 (22:27 -0400)]
Make regexp engine's backref-related compilation state more bulletproof.

Up to now, we remembered the definition of a capturing parenthesis
subexpression by storing a pointer to the associated subRE node.
That was okay before, because that subRE didn't get modified anymore
while parsing the rest of the regexp.  However, in the wake of
commit ea1268f63, that's no longer true: the outer invocation of
parseqatom() feels free to scribble on that subRE.  This seems to
work anyway, because the states we jam into the child atom in the
"prepare a general-purpose state skeleton" stanza aren't really
semantically different from the original endpoints of the child atom.
But that would be mighty easy to break, and it's definitely not how
things worked before.

Between this and the issue fixed in the prior commit, it seems best
to get rid of this dependence on subRE nodes entirely.  We don't need
the whole child subRE for future backrefs, only its starting and ending
NFA states; so let's just store pointers to those.

Also, in the corner case where we make an extra subRE to handle
immediately-nested capturing parentheses, it seems like it'd be smart
to have the extra subRE have the same begin/end states as the original
child subRE does (s/s2 not lp/rp).  I think that linking it from lp to
rp might actually be semantically wrong, though since Spencer's original
code did it that way, I'm not totally certain.  Using s/s2 is certainly
not wrong, in any case.

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

3 years agoFix use-after-free issue in regexp engine.
Tom Lane [Sun, 8 Aug 2021 02:05:27 +0000 (22:05 -0400)]
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

3 years agoMove temporary file cleanup to before_shmem_exit().
Andres Freund [Sun, 8 Aug 2021 02:14:20 +0000 (19:14 -0700)]
Move temporary file cleanup to before_shmem_exit().

As reported by a few OSX buildfarm animals there exist at least one path where
temporary files exist during AtProcExit_Files() processing. As temporary file
cleanup causes pgstat reporting, the assertions added in ee3f8d3d3ae caused
failures.

This is not an OSX specific issue, we were just lucky that timing on OSX
reliably triggered the problem.  The known way to cause this is a FATAL error
during perform_base_backup() with a MANIFEST used - adding an elog(FATAL)
after InitializeBackupManifest() reliably reproduces the problem in isolation.

The problem is that the temporary file created in InitializeBackupManifest()
is not cleaned up via resource owner cleanup as WalSndResourceCleanup()
currently is only used for non-FATAL errors. That then allows to reach
AtProcExit_Files() with existing temporary files, causing the assertion
failure.

To fix this problem, move temporary file cleanup to a before_shmem_exit() hook
and add assertions ensuring that no temporary files are created before / after
temporary file management has been initialized / shut down. The cleanest way
to do so seems to be to split fd.c initialization into two, one for plain file
access and one for temporary file access.

Right now there's no need to perform further fd.c cleanup during process exit,
so I just renamed AtProcExit_Files() to BeforeShmemExit_Files(). Alternatively
we could perform another pass through the files to check that no temporary
files exist, but the added assertions seem to provide enough protection
against that.

It might turn out that the assertions added in ee3f8d3d3ae will cause too much
noise - in that case we'll have to downgrade them to a WARNING, at least
temporarily.

This commit is not necessarily the best approach to address this issue, but it
should resolve the buildfarm failures. We can revise later.

Author: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/20210807190131.2bm24acbebl4wl6i@alap3.anarazel.de

3 years agoRemove T_MemoryContext
Peter Eisentraut [Sat, 7 Aug 2021 21:21:24 +0000 (23:21 +0200)]
Remove T_MemoryContext

This is an abstract node that shouldn't have a node tag defined.

Discussion: https://www.postgresql.org/message-id/flat/c1097590-a6a4-486a-64b1-e1f9cc0533ce@enterprisedb.com

3 years agopg_amcheck: Message style improvements
Peter Eisentraut [Sat, 7 Aug 2021 18:34:49 +0000 (20:34 +0200)]
pg_amcheck: Message style improvements

3 years agoReally fix the ambiguity in REFRESH MATERIALIZED VIEW CONCURRENTLY.
Tom Lane [Sat, 7 Aug 2021 17:29:32 +0000 (13:29 -0400)]
Really fix the ambiguity in REFRESH MATERIALIZED VIEW CONCURRENTLY.

Rather than trying to pick table aliases that won't conflict with
any possible user-defined matview column name, adjust the queries'
syntax so that the aliases are only used in places where they can't be
mistaken for column names.  Mostly this consists of writing "alias.*"
not just "alias", which adds clarity for humans as well as machines.
We do have the issue that "SELECT alias.*" acts differently from
"SELECT alias", but we can use the same hack ruleutils.c uses for
whole-row variables in SELECT lists: write "alias.*::compositetype".

We might as well revert to the original aliases after doing this;
they're a bit easier to read.

Like 75d66d10e, back-patch to all supported branches.

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

3 years agopg_amcheck: Add missing translation markers
Peter Eisentraut [Sat, 7 Aug 2021 11:36:59 +0000 (13:36 +0200)]
pg_amcheck: Add missing translation markers

3 years agoMessage style improvements
Peter Eisentraut [Sat, 7 Aug 2021 10:09:22 +0000 (12:09 +0200)]
Message style improvements

3 years agopgstat: Schedule per-backend pgstat shutdown via before_shmem_exit().
Andres Freund [Fri, 6 Aug 2021 17:05:57 +0000 (10:05 -0700)]
pgstat: Schedule per-backend pgstat shutdown via before_shmem_exit().

Previously on_shmem_exit() was used. The upcoming shared memory stats patch
uses DSM segments to store stats, which can not be used after the
dsm_backend_shutdown() call in shmem_exit().

The preceding commits were required to permit this change. This commit is
split off the shared memory stats patch to make it easier to isolate problems
caused by the ordering changes rather than the much larger changes in where
stats are stored.

Author: Andres Freund <andres@anarazel.de>
Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://postgr.es/m/20210405092914.mmxqe7j56lsjfsej@alap3.anarazel.de
Discussion: https://postgr.es/m/20210802164124.ufo5buo4apl6yuvs@alap3.anarazel.de
Discussion: https://postgr.es/m/20210803023612.iziacxk5syn2r4ut@alap3.anarazel.de

3 years agoSchedule ShutdownXLOG() in single user mode using before_shmem_exit().
Andres Freund [Fri, 26 Mar 2021 17:52:14 +0000 (10:52 -0700)]
Schedule ShutdownXLOG() in single user mode using before_shmem_exit().

Previously on_shmem_exit() was used. The upcoming shared memory stats patch
uses DSM segments to store stats, which can not be used after the
dsm_backend_shutdown() call in shmem_exit().  There does not seem to be any
reason to do ShutdownXLOG() via on_shmem_exit(), so change it.

Author: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/20210405092914.mmxqe7j56lsjfsej@alap3.anarazel.de
Discussion: https://postgr.es/m/20210803023612.iziacxk5syn2r4ut@alap3.anarazel.de

3 years agoMake parallel worker shutdown complete entirely via before_shmem_exit().
Andres Freund [Wed, 31 Mar 2021 07:50:28 +0000 (00:50 -0700)]
Make parallel worker shutdown complete entirely via before_shmem_exit().

This is a step toward storing stats in dynamic shared memory. As dynamic
shared memory segments are detached from just after before_shmem_exit()
callbacks are processed, but before on_shmem_exit() callbacks are, no stats
can be collected after before_shmem_exit() callbacks have been processed.

Parallel worker shutdown can cause stats to be emitted during DSM detach
callbacks, e.g. for SharedFileSet (which closes its files, which can causes
fd.c to emit stats about temporary files). Therefore parallel worker shutdown
needs to complete during the processing of before_shmem_exit callbacks.

One might think this problem could instead be solved by carefully ordering the
attaching to DSM segments, so that the pgstats segments get detached from
later than the parallel query ones. That turns out to not work because the
stats hash might need to grow which can cause new segments to be
allocated, which then will be detached from earlier.

There are two code changes:

First, call ParallelWorkerShutdown() via before_shmem_exit. That's a good idea
on its own, because other shutdown callbacks like ShutdownPostgres and
ShutdownAuxiliaryProcess are called via before_*.

Second, explicitly detach from the parallel query DSM segment, thereby
ensuring all stats are emitted during ParallelWorkerShutdown().

There are nicer solutions to these problems, but it's not obvious which of
those solutions is the correct one. As the shared memory stats work already is
a huge amount of work...

Author: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/20210405092914.mmxqe7j56lsjfsej@alap3.anarazel.de
Discussion: https://postgr.es/m/20210803023612.iziacxk5syn2r4ut@alap3.anarazel.de

3 years agopgstat: Bring up pgstat in BaseInit() to fix uninitialized use of pgstat by AV.
Andres Freund [Sat, 7 Aug 2021 01:54:39 +0000 (18:54 -0700)]
pgstat: Bring up pgstat in BaseInit() to fix uninitialized use of pgstat by AV.

Previously pgstat_initialize() was called in InitPostgres() and
AuxiliaryProcessMain(). As it turns out there was at least one case where we
reported stats before pgstat_initialize() was called, see
AutoVacWorkerMain()'s intentionally early call to pgstat_report_autovac().

This turns out to not be a problem with the current pgstat implementation as
pgstat_initialize() only registers a shutdown callback. But in the shared
memory based stats implementation we are working towards pgstat_initialize()
has to do more work.

After b406478b87e BaseInit() is a central place where initialization shared by
normal backends and auxiliary backends can be put. Obviously BaseInit() is
called before InitPostgres() registers ShutdownPostgres. Previously
ShutdownPostgres was the first before_shmem_exit callback, now that's commonly
pgstats. That should be fine.

Previously pgstat_initialize() was not called in bootstrap mode, but there
does not appear to be a need for that. It's now done unconditionally.

To detect future issues like this, assertions are added to a few places
verifying that the pgstat subsystem is initialized and not yet shut down.

Author: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/20210405092914.mmxqe7j56lsjfsej@alap3.anarazel.de
Discussion: https://postgr.es/m/20210802164124.ufo5buo4apl6yuvs@alap3.anarazel.de

3 years agoDon't elide casting to typmod -1.
Tom Lane [Fri, 6 Aug 2021 21:32:54 +0000 (17:32 -0400)]
Don't elide casting to typmod -1.

Casting a value that's already of a type with a specific typmod
to an unspecified typmod doesn't do anything so far as run-time
behavior is concerned.  However, it really ought to change the
exposed type of the expression to match.  Up to now,
coerce_type_typmod hasn't bothered with that, which creates gotchas
in contexts such as recursive unions.  If for example one side of
the union is numeric(18,3), but it needs to be plain numeric to
match the other side, there's no direct way to express that.

This is easy enough to fix, by inserting a RelabelType to update the
exposed type of the expression.  However, it's a bit nervous-making
to change this behavior, because it's stood for a really long time.
(I strongly suspect that it's like this in part because the logic
pre-dates the introduction of RelabelType in 7.0.  The commit log
message for 57b30e8e2 is interesting reading here.)  As a compromise,
we'll sneak the change into 14beta3, and consider back-patching to
stable branches if no complaints emerge in the next three months.

Discussion: https://postgr.es/m/CABNQVagu3bZGqiTjb31a8D5Od3fUMs7Oh3gmZMQZVHZ=uWWWfQ@mail.gmail.com

3 years agoAdjust the integer overflow tests in the numeric code.
Dean Rasheed [Fri, 6 Aug 2021 20:29:15 +0000 (21:29 +0100)]
Adjust the integer overflow tests in the numeric code.

Formerly, the numeric code tested whether an integer value of a larger
type would fit in a smaller type by casting it to the smaller type and
then testing if the reverse conversion produced the original value.
That's perfectly fine, except that it caused a test failure on
buildfarm animal castoroides, most likely due to a compiler bug.

Instead, do these tests by comparing against PG_INT16/32_MIN/MAX. That
matches existing code in other places, such as int84(), which is more
widely tested, and so is less likely to go wrong.

While at it, add regression tests covering the numeric-to-int8/4/2
conversions, and adjust the recently added tests to the style of
434ddfb79a (on the v11 branch) to make failures easier to diagnose.

Per buildfarm via Tom Lane, reviewed by Tom Lane.

Discussion: https://postgr.es/m/2394813.1628179479%40sss.pgh.pa.us

3 years agoAdd missing message punctuation
Peter Eisentraut [Fri, 6 Aug 2021 20:11:02 +0000 (22:11 +0200)]
Add missing message punctuation

3 years agoFix wording
Peter Eisentraut [Fri, 6 Aug 2021 18:55:59 +0000 (20:55 +0200)]
Fix wording

3 years agoprocess startup: Always call Init[Auxiliary]Process() before BaseInit().
Andres Freund [Thu, 5 Aug 2021 21:37:09 +0000 (14:37 -0700)]
process startup: Always call Init[Auxiliary]Process() before BaseInit().

For EXEC_BACKEND InitProcess()/InitAuxiliaryProcess() needs to have been
called well before we call BaseInit(), as SubPostmasterMain() needs LWLocks to
work. Having the order of initialization differ between platforms makes it
unnecessarily hard to understand the system and to add initialization points
for new subsystems without a lot of duplication.

To be able to change the order, BaseInit() cannot trigger
CreateSharedMemoryAndSemaphores() anymore - obviously that needs to have
happened before we can call InitProcess(). It seems cleaner to create shared
memory explicitly in single user/bootstrap mode anyway.

After this change the separation of bufmgr initialization into
InitBufferPoolAccess() / InitBufferPoolBackend() is not meaningful anymore so
the latter is removed.

Author: Andres Freund <andres@anarazel.de>
Reviewed-By: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://postgr.es/m/20210802164124.ufo5buo4apl6yuvs@alap3.anarazel.de

3 years agoCall pgwin32_signal_initialize() in postmaster as well.
Andres Freund [Thu, 5 Aug 2021 22:31:29 +0000 (15:31 -0700)]
Call pgwin32_signal_initialize() in postmaster as well.

This was an oversight in 07bf3785099. While it does reduce the benefit of the
simplification due to that commit, it still seems like a win to me.

It seems like it might be a good idea to have a function mirroring
InitPostmasterChild() / InitStandaloneProcess() for postmaster in miscinit.c
to make it easier to keep initialization between the three possible
environment in sync.

Author: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/20210805214109.lzfk3r3ae37bahmv@alap3.anarazel.de

3 years agoprocess startup: Centralize pgwin32_signal_initialize() calls.
Andres Freund [Thu, 5 Aug 2021 19:36:06 +0000 (12:36 -0700)]
process startup: Centralize pgwin32_signal_initialize() calls.

For one, the existing location lead to somewhat awkward code in main(). For
another, the new location is easier to understand anyway.

Author: Andres Freund <andres@anarazel.de>
Reviewed-By: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-By: Robert Haas <robertmhaas@gmail.com>
Discussion: https://postgr.es/m/20210802164124.ufo5buo4apl6yuvs@alap3.anarazel.de

3 years agoprocess startup: Remove bootstrap / checker modes from AuxProcType.
Andres Freund [Thu, 5 Aug 2021 19:17:31 +0000 (12:17 -0700)]
process startup: Remove bootstrap / checker modes from AuxProcType.

Neither is actually initialized as an auxiliary process, so it does not really
make sense to reserve a PGPROC etc for them.

This keeps checker mode implemented by exiting partway through bootstrap
mode. That might be worth changing at some point, perhaps if we ever extend
checker mode to be a more general tool.

Author: Andres Freund <andres@anarazel.de>
Reviewed-By: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-By: Robert Haas <robertmhaas@gmail.com>
Discussion: https://postgr.es/m/20210802164124.ufo5buo4apl6yuvs@alap3.anarazel.de

3 years agoprocess startup: Move AuxiliaryProcessMain into its own file.
Andres Freund [Thu, 5 Aug 2021 19:09:19 +0000 (12:09 -0700)]
process startup: Move AuxiliaryProcessMain into its own file.

After the preceding commits the auxprocess code is independent from
bootstrap.c - so a dedicated file seems less confusing.

Author: Andres Freund <andres@anarazel.de>
Reviewed-By: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-By: Robert Haas <robertmhaas@gmail.com>
Discussion: https://postgr.es/m/20210802164124.ufo5buo4apl6yuvs@alap3.anarazel.de

3 years agoprocess startup: auxprocess: reindent block
Andres Freund [Thu, 5 Aug 2021 02:30:12 +0000 (19:30 -0700)]
process startup: auxprocess: reindent block

Kept separate for ease of review, particularly because pgindent insists on
reflowing a few comments.

Author: Andres Freund <andres@anarazel.de>
Reviewed-By: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-By: Robert Haas <robertmhaas@gmail.com>
Discussion: https://postgr.es/m/20210802164124.ufo5buo4apl6yuvs@alap3.anarazel.de

3 years agoprocess startup: Separate out BootstrapModeMain from AuxiliaryProcessMain.
Andres Freund [Thu, 5 Aug 2021 02:29:45 +0000 (19:29 -0700)]
process startup: Separate out BootstrapModeMain from AuxiliaryProcessMain.

There practically was no shared code between the two, once all the ifs are
removed. And it was quite confusing that aux processes weren't actually
started by the call to AuxiliaryProcessMain() in main().

There's more to do, AuxiliaryProcessMain() should move out of bootstrap.c, and
BootstrapModeMain() shouldn't use/be part of AuxProcType.

Author: Andres Freund <andres@anarazel.de>
Reviewed-By: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-By: Robert Haas <robertmhaas@gmail.com>
Discussion: https://postgr.es/m/20210802164124.ufo5buo4apl6yuvs@alap3.anarazel.de

3 years agoprocess startup: Rename postmaster's --forkboot to --forkaux.
Andres Freund [Thu, 5 Aug 2021 02:28:46 +0000 (19:28 -0700)]
process startup: Rename postmaster's --forkboot to --forkaux.

It is confusing that aux processes are started with --forkboot, given that
bootstrap mode cannot run below postmaster.

Author: Andres Freund <andres@anarazel.de>
Reviewed-By: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-By: Robert Haas <robertmhaas@gmail.com>
Discussion: https://postgr.es/m/20210802164124.ufo5buo4apl6yuvs@alap3.anarazel.de

3 years agopostgres_fdw: Fix issues with generated columns in foreign tables.
Etsuro Fujita [Thu, 5 Aug 2021 11:00:00 +0000 (20:00 +0900)]
postgres_fdw: Fix issues with generated columns in foreign tables.

postgres_fdw imported generated columns from the remote tables as plain
columns, and caused failures like "ERROR: cannot insert a non-DEFAULT
value into column "foo"" when inserting into the foreign tables, as it
tried to insert values into the generated columns.  To fix, we do the
following under the assumption that generated columns in a postgres_fdw
foreign table are defined so that they represent generated columns in
the underlying remote table:

* Send DEFAULT for the generated columns to the foreign server on insert
  or update, not generated column values computed on the local server.
* Add to postgresImportForeignSchema() an option "import_generated" to
  include column generated expressions in the definitions of foreign
  tables imported from a foreign server.  The option is true by default.

The assumption seems reasonable, because that would make a query of the
postgres_fdw foreign table return values for the generated columns that
are consistent with the generated expression.

While here, fix another issue in postgresImportForeignSchema(): it tried
to include column generated expressions as column default expressions in
the foreign table definitions when the import_default option was enabled.

Per bug #16631 from Daniel Cherniy.  Back-patch to v12 where generated
columns were added.

Discussion: https://postgr.es/m/16631-e929fe9db0ffc7cf%40postgresql.org

3 years agoRemove unused argument "txn" in maybe_send_schema().
Fujii Masao [Thu, 5 Aug 2021 08:49:51 +0000 (17:49 +0900)]
Remove unused argument "txn" in maybe_send_schema().

Commit 464824323e added the argument "txn" into maybe_send_schema()
though it was not used.

Author: Hou Zhijie
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/OS0PR01MB5716146E43928FB92D45D29794EC9@OS0PR01MB5716.jpnprd01.prod.outlook.com

3 years agoFix division-by-zero error in to_char() with 'EEEE' format.
Dean Rasheed [Thu, 5 Aug 2021 08:24:11 +0000 (09:24 +0100)]
Fix division-by-zero error in to_char() with 'EEEE' format.

This fixes a long-standing bug when using to_char() to format a
numeric value in scientific notation -- if the value's exponent is
less than -NUMERIC_MAX_DISPLAY_SCALE-1 (-1001), it produced a
division-by-zero error.

The reason for this error was that get_str_from_var_sci() divides its
input by 10^exp, which it produced using power_var_int(). However, the
underflow test in power_var_int() causes it to return zero if the
result scale is too small. That's not a problem for power_var_int()'s
only other caller, power_var(), since that limits the rscale to 1000,
but in get_str_from_var_sci() the exponent can be much smaller,
requiring a much larger rscale. Fix by introducing a new function to
compute 10^exp directly, with no rscale limit. This also allows 10^exp
to be computed more efficiently, without any numeric multiplication,
division or rounding.

Discussion: https://postgr.es/m/CAEZATCWhojfH4whaqgUKBe8D5jNHB8ytzemL-PnRx+KCTyMXmg@mail.gmail.com

3 years agopgbench: When using pipelining only do PQconsumeInput() when necessary.
Andres Freund [Thu, 5 Aug 2021 02:19:44 +0000 (19:19 -0700)]
pgbench: When using pipelining only do PQconsumeInput() when necessary.

Up to now we did a PQconsumeInput() for each pipelined query, asking the OS
for more input - which it often won't have, as all results might already have
been sent. That turns out to have a noticeable performance impact.

Alvaro Herrera reviewed the idea to add the PQisBusy() check, but not this
concrete patch.

Author: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/20210720180039.23rivhdft3l4mayn@alap3.anarazel.de
Backpatch: 14, where libpq/pgbench pipelining was introduced.

3 years agopgstat: split reporting/fetching of bgwriter and checkpointer stats.
Andres Freund [Thu, 5 Aug 2021 02:16:04 +0000 (19:16 -0700)]
pgstat: split reporting/fetching of bgwriter and checkpointer stats.

These have been unrelated since bgwriter and checkpointer were split into two
processes in 806a2aee379. As there several pending patches (shared memory
stats, extending the set of tracked IO / buffer statistics) that are made a
bit more awkward by the grouping, split them. Done separately to make
reviewing easier.

This does *not* change the contents of pg_stat_bgwriter or move fields out of
bgwriter/checkpointer stats that arguably do not belong in either. However
pgstat_fetch_global() was renamed and split into
pgstat_fetch_stat_checkpointer() and pgstat_fetch_stat_bgwriter().

Author: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/20210405092914.mmxqe7j56lsjfsej@alap3.anarazel.de

3 years agoMake vacuum_index_cleanup reloption RELOPT_TYPE_ENUM.
Peter Geoghegan [Wed, 4 Aug 2021 04:53:41 +0000 (21:53 -0700)]
Make vacuum_index_cleanup reloption RELOPT_TYPE_ENUM.

Oversight in commit 3499df0d, which generalized the reloption as a way
of giving users a way to consistently avoid VACUUM's index bypass
optimization.

Per off-list report from Nikolay Shaplov.

Backpatch: 14-, where index cleanup reloption was extended.

3 years agoAdd prepare API support for streaming transactions in logical replication.
Amit Kapila [Wed, 4 Aug 2021 02:17:06 +0000 (07:47 +0530)]
Add prepare API support for streaming transactions in logical replication.

Commit a8fd13cab0 added support for prepared transactions to built-in
logical replication via a new option "two_phase" for a subscription. The
"two_phase" option was not allowed with the existing streaming option.

This commit permits the combination of "streaming" and "two_phase"
subscription options. It extends the pgoutput plugin and the subscriber
side code to add the prepare API for streaming transactions which will
apply the changes accumulated in the spool-file at prepare time.

Author: Peter Smith and Ajin Cherian
Reviewed-by: Vignesh C, Amit Kapila, Greg Nancarrow
Tested-By: Haiying Tang
Discussion: https://postgr.es/m/02DA5F5E-CECE-4D9C-8B4B-418077E2C010@postgrespro.ru
Discussion: https://postgr.es/m/CAMGcDxeqEpWj3fTXwqhSwBdXd2RS9jzwWscO-XbeCfso6ts3+Q@mail.gmail.com

3 years agoAdd assorted new regexp_xxx SQL functions.
Tom Lane [Tue, 3 Aug 2021 17:08:49 +0000 (13:08 -0400)]
Add assorted new regexp_xxx SQL functions.

This patch adds new functions regexp_count(), regexp_instr(),
regexp_like(), and regexp_substr(), and extends regexp_replace()
with some new optional arguments.  All these functions follow
the definitions used in Oracle, although there are small differences
in the regexp language due to using our own regexp engine -- most
notably, that the default newline-matching behavior is different.
Similar functions appear in DB2 and elsewhere, too.  Aside from
easing portability, these functions are easier to use for certain
tasks than our existing regexp_match[es] functions.

Gilles Darold, heavily revised by me

Discussion: https://postgr.es/m/fc160ee0-c843-b024-29bb-97b5da61971f@darold.net

3 years agoC comment: correct heading of extension query
Bruce Momjian [Tue, 3 Aug 2021 16:26:08 +0000 (12:26 -0400)]
C comment:  correct heading of extension query

Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/20210803161345.GZ12533@telsasoft.com

Backpatch-through: 9.6

3 years agointerval: round values when spilling to months
Bruce Momjian [Tue, 3 Aug 2021 16:10:29 +0000 (12:10 -0400)]
interval:  round values when spilling to months

Previously spilled units greater than months were truncated to months.
Also document the spill behavior.

Reported-by: Bryn Llewelly
Discussion: https://postgr.es/m/BDAE4B56-3337-45A2-AC8A-30593849D6C0@yugabyte.com

Backpatch-through: master

3 years agopg_upgrade: warn about extensions that need updating
Bruce Momjian [Tue, 3 Aug 2021 15:58:15 +0000 (11:58 -0400)]
pg_upgrade:  warn about extensions that need updating

Also create a script that can be run to update them.

Reported-by: Dave Cramer
Discussion: https://postgr.es/m/CADK3HHKawwbOcGwMGnDuAf3-U8YfvTcS8jqDv3UM=niijs3MMA@mail.gmail.com

Backpatch-through: 9.6

3 years agopg_upgrade: improve docs about extension upgrades
Bruce Momjian [Tue, 3 Aug 2021 15:27:33 +0000 (11:27 -0400)]
pg_upgrade:  improve docs about extension upgrades

The previous wording was unclear about the steps needed to upgrade
extensions, and how to update them after pg_upgrade.

Reported-by: Dave Cramer
Discussion: https://postgr.es/m/CADK3HHKawwbOcGwMGnDuAf3-U8YfvTcS8jqDv3UM=niijs3MMA@mail.gmail.com

Backpatch-through: 9.6

3 years agodoc: mention inheritance's tableoid can be used in partitioning
Bruce Momjian [Tue, 3 Aug 2021 15:11:51 +0000 (11:11 -0400)]
doc:  mention inheritance's tableoid can be used in partitioning

Previously tableoid was not mentioned in the partition doc section.  We
only had a link to the "all the normal rules" of inheritance section.

Reported-by: michal.palenik@freemap.sk
Discussion: https://postgr.es/m/162627031219.693.11508199541771263335@wrigleys.postgresql.org

Backpatch-through: 10

3 years agodoc: add example of using pg_dump with GNU split and gzip
Bruce Momjian [Tue, 3 Aug 2021 14:57:32 +0000 (10:57 -0400)]
doc:  add example of using pg_dump with GNU split and gzip

This is only possible with GNU split, not other versions like BSD split.

Reported-by: jim@jdoherty.net
Discussion: https://postgr.es/m/162653459215.701.6323855956817776386@wrigleys.postgresql.org

Backpatch-through: 9.6

3 years agoFurther simplify a bit of logic in StartupXLOG().
Thomas Munro [Tue, 3 Aug 2021 02:11:55 +0000 (14:11 +1200)]
Further simplify a bit of logic in StartupXLOG().

Commit 7ff23c6d277d1d90478a51f0dd81414d343f3850 left us with two
identical cases. Collapse them.

Author: Robert Haas <robertmhaas@gmail.com>
Discussion: https://postgr.es/m/CA%2BhUKGJ8NRsqgkZEnsnRc2MFROBV-jCnacbYvtpptK2A9YYp9Q%40mail.gmail.com

3 years agoAllow ordered partition scans in more cases
David Rowley [Tue, 3 Aug 2021 00:25:52 +0000 (12:25 +1200)]
Allow ordered partition scans in more cases

959d00e9d added the ability to make use of an Append node instead of a
MergeAppend when we wanted to perform a scan of a partitioned table and
the required sort order was the same as the partitioned keys and the
partitioned table was defined in such a way that earlier partitions were
guaranteed to only contain lower-order values than later partitions.
However, previously we didn't allow these ordered partition scans for
LIST partitioned table when there were any partitions that allowed
multiple Datums.  This was a very cheap check to make and we could likely
have done a little better by checking if there were interleaved
partitions, but at the time we didn't have visibility about which
partitions were pruned, so we still may have disallowed cases where all
interleaved partitions were pruned.

Since 475dbd0b7, we now have knowledge of pruned partitions, we can do a
much better job inside partitions_are_ordered().

Here we pass which partitions survived partition pruning into
partitions_are_ordered() and, for LIST partitioning, have it check to see
if any live partitions exist that are also in the new "interleaved_parts"
field defined in PartitionBoundInfo.

For RANGE partitioning we can relax the code which caused the partitions
to be unordered if a DEFAULT partition existed.  Since we now know which
partitions were pruned, partitions_are_ordered() now returns true when the
DEFAULT partition was pruned.

Reviewed-by: Amit Langote, Zhihong Yu
Discussion: https://postgr.es/m/CAApHDvrdoN_sXU52i=QDXe2k3WAo=EVry29r2+Tq2WYcn2xhEA@mail.gmail.com

3 years agoTrack a Bitmapset of non-pruned partitions in RelOptInfo
David Rowley [Mon, 2 Aug 2021 23:47:24 +0000 (11:47 +1200)]
Track a Bitmapset of non-pruned partitions in RelOptInfo

For partitioned tables with large numbers of partitions where queries are
able to prune all but a very small number of partitions, the time spent in
the planner looping over RelOptInfo.part_rels checking for non-NULL
RelOptInfos could become a large portion of the overall planning time.

Here we add a Bitmapset that records the non-pruned partitions.  This
allows us to more efficiently skip the pruned partitions by looping over
the Bitmapset.

This will cause a very slight slow down in cases where no or not many
partitions could be pruned, however, those cases are already slow to plan
anyway and the overhead of looping over the Bitmapset would be
unmeasurable when compared with the other tasks such as path creation for
a large number of partitions.

Reviewed-by: Amit Langote, Zhihong Yu
Discussion: https://postgr.es/m/CAApHDvqnPx6JnUuPwaf5ao38zczrAb9mxt9gj4U1EKFfd4AqLA@mail.gmail.com

3 years agoDoc: minor improvements for logical replication protocol documentation.
Tom Lane [Mon, 2 Aug 2021 15:32:17 +0000 (11:32 -0400)]
Doc: minor improvements for logical replication protocol documentation.

Where appropriate, annotate message field data types with the backend
code's name for the data type, eg XLogRecPtr or TimestampTz.  Previously
we just said "Int64" which didn't provide as much info.

Also clarify references to object OIDs, and make use of the existing
convention to denote the value of a field that must have a fixed value.

Vignesh C, reviewed by Peter Smith and Euler Taveira.

Discussion: https://postgr.es/m/CALDaNm0+fatx57KFcBopUZWQpH_tz3WKKfm-_eiTwcXui5BnhQ@mail.gmail.com

3 years agoRun checkpointer and bgwriter in crash recovery.
Thomas Munro [Mon, 2 Aug 2021 05:32:20 +0000 (17:32 +1200)]
Run checkpointer and bgwriter in crash recovery.

Start up the checkpointer and bgwriter during crash recovery (except in
--single mode), as we do for replication.  This wasn't done back in
commit cdd46c76 out of caution.  Now it seems like a better idea to make
the environment as similar as possible in both cases.  There may also be
some performance advantages.

Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Reviewed-by: Aleksander Alekseev <aleksander@timescale.com>
Tested-by: Jakub Wartak <Jakub.Wartak@tomtom.com>
Discussion: https://postgr.es/m/CA%2BhUKGJ8NRsqgkZEnsnRc2MFROBV-jCnacbYvtpptK2A9YYp9Q%40mail.gmail.com

3 years agoRemove misplaced comment from AuxiliaryProcessMain().
Andres Freund [Mon, 2 Aug 2021 04:10:11 +0000 (21:10 -0700)]
Remove misplaced comment from AuxiliaryProcessMain().

The comment didn't make sense anymore since at least 626eb021988. As it didn't
actually explain anything anyway, just remove it.

Author: Andres Freund <andres@anarazel.de>

3 years agoFix oversight in commit 1ec7fca8592178281cd5cdada0f27a340fb813fc.
Etsuro Fujita [Mon, 2 Aug 2021 03:45:00 +0000 (12:45 +0900)]
Fix oversight in commit 1ec7fca8592178281cd5cdada0f27a340fb813fc.

I failed to account for the possibility that when
ExecAppendAsyncEventWait() notifies multiple async-capable nodes using
postgres_fdw, a preceding node might invoke process_pending_request() to
process a pending asynchronous request made by a succeeding node.  In
that case the succeeding node should produce a tuple to return to the
parent Append node from tuples fetched by process_pending_request() when
notified.  Repair.

Per buildfarm via Michael Paquier.  Back-patch to v14, like the previous
commit.

Thanks to Tom Lane for testing.

Discussion: https://postgr.es/m/YQP0UPT8KmPiHTMs%40paquier.xyz

3 years agoFix test failure in 021_twophase.pl.
Amit Kapila [Mon, 2 Aug 2021 02:59:26 +0000 (08:29 +0530)]
Fix test failure in 021_twophase.pl.

The test is expecting two prepared transactions corresponding to two
subscriptions but it waits to catch up for just one subscription. Fix it
by allowing to wait for both subscriptions.

Reported-by: Michael Paquier, as per buildfarm
Author: Ajin Cherian
Reviewed-By: Amit Kapila, Vignesh C, Peter Smith
Discussion: https://postgr.es/m/CAA4eK1+_0iNQ8Z=KVTjmmAqNX-hyv+1+fnZ-Yx8CVP=uAcekqw@mail.gmail.com

3 years agoSilence perl warning about uninitialized value
Andrew Dunstan [Sun, 1 Aug 2021 17:03:15 +0000 (13:03 -0400)]
Silence perl warning about uninitialized value

3 years agoDoc: alphabetize the regexp_foo() function descriptions in 9.7.3.
Tom Lane [Sat, 31 Jul 2021 19:35:27 +0000 (15:35 -0400)]
Doc: alphabetize the regexp_foo() function descriptions in 9.7.3.

For no visible reason (other than historical accident no doubt),
regexp_replace() was out of order.  Re-order to match the way
that these functions are listed in 9.4.  (That means substring()
remains first, because it's SQL-standard and the rest aren't.)
I've not touched the text other than to move it.  This is just
to reduce confusion in the diffs for upcoming additions.

3 years agoUse elog, not Assert, to report failure to provide an outer snapshot.
Tom Lane [Sat, 31 Jul 2021 15:50:14 +0000 (11:50 -0400)]
Use elog, not Assert, to report failure to provide an outer snapshot.

As of commit 84f5c2908, executing SQL commands (via SPI or otherwise)
requires having either an active Portal, or a caller-established
active snapshot.  We were simply Assert'ing that that's the case.
But we've now had a couple different reports of people testing
extensions that didn't meet this requirement, and were confused by
the resulting crash.  Let's convert the Assert to a test-and-elog,
in hopes of making the issue clearer for extension authors.

Per gripes from Liu Huailing and RekGRpth.  Back-patch to v11,
like the prior commit.

Discussion: https://postgr.es/m/OSZPR01MB6215671E3C5956A034A080DFBEEC9@OSZPR01MB6215.jpnprd01.prod.outlook.com
Discussion: https://postgr.es/m/17035-14607d308ac8643c@postgresql.org

3 years agoRemove redundant setting of pg_attribute.attcompression
John Naylor [Sat, 31 Jul 2021 11:25:27 +0000 (07:25 -0400)]
Remove redundant setting of pg_attribute.attcompression

Since e6241d8e0, no attribute needs a non-default value of this during
initdb, so let the usual machinery for defaults take care of it.

3 years agoFix corner-case errors and loss of precision in numeric_power().
Dean Rasheed [Sat, 31 Jul 2021 10:21:44 +0000 (11:21 +0100)]
Fix corner-case errors and loss of precision in numeric_power().

This fixes a couple of related problems that arise when raising
numbers to very large powers.

Firstly, when raising a negative number to a very large integer power,
the result should be well-defined, but the previous code would only
cope if the exponent was small enough to go through power_var_int().
Otherwise it would throw an internal error, attempting to take the
logarithm of a negative number. Fix this by adding suitable handling
to the general case in power_var() to cope with negative bases,
checking for integer powers there.

Next, when raising a (positive or negative) number whose absolute
value is slightly less than 1 to a very large power, the result should
approach zero as the power is increased. However, in some cases, for
sufficiently large powers, this would lose all precision and return 1
instead of 0. This was due to the way that the local_rscale was being
calculated for the final full-precision calculation:

  local_rscale = rscale + (int) val - ln_dweight + 8

The first two terms on the right hand side are meant to give the
number of significant digits required in the result ("val" being the
estimated result weight). However, this failed to account for the fact
that rscale is clipped to a maximum of NUMERIC_MAX_DISPLAY_SCALE
(1000), and the result weight might be less then -1000, causing their
sum to be negative, leading to a loss of precision. Fix this by
forcing the number of significant digits calculated to be nonnegative.
It's OK for it to be zero (when the result weight is less than -1000),
since the local_rscale value then includes a few extra digits to
ensure an accurate result.

Finally, add additional underflow checks to exp_var() and power_var(),
so that they consistently return zero for cases like this where the
result is indistinguishable from zero. Some paths through this code
already returned zero in such cases, but others were throwing overflow
errors.

Dean Rasheed, reviewed by Yugo Nagata.

Discussion: http://postgr.es/m/CAEZATCW6Dvq7+3wN3tt5jLj-FyOcUgT5xNoOqce5=6Su0bCR0w@mail.gmail.com