pgsql: pg_rewind: Move syncTargetDirectory() to file_ops.c
pg_rewind: Move syncTargetDirectory() to file_ops.c For consistency. All the other low-level functions that operate on the target directory are in file_ops.c. Reviewed-by: Michael Paquier Discussion: https://www.postgresql.org/message-id/0c5b3783-af52-3ee5-f8fa-6e794061f70d%40iki.fi Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/ffb4e27e9c5ea87f9fecb7036dfc7cc1f38169b6 Modified Files -- src/bin/pg_rewind/file_ops.c | 19 +++ src/bin/pg_rewind/file_ops.h | 1 + src/bin/pg_rewind/pg_rewind.c | 22 +- src/bin/pg_rewind/pg_rewind.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-)
pgsql: pg_rewind: Replace the hybrid list+array data structure with sim
pg_rewind: Replace the hybrid list+array data structure with simplehash. Now that simplehash can be used in frontend code, let's make use of it. Reviewed-by: Kyotaro Horiguchi, Soumyadeep Chakraborty Discussion: https://www.postgresql.org/message-id/0c5b3783-af52-3ee5-f8fa-6e794061f70d%40iki.fi Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/f81e97d0475cd4bc597adc23b665bd84fbf79a0d Modified Files -- src/bin/pg_rewind/copy_fetch.c | 4 +- src/bin/pg_rewind/fetch.c | 2 +- src/bin/pg_rewind/fetch.h | 2 +- src/bin/pg_rewind/filemap.c | 294 ++-- src/bin/pg_rewind/filemap.h | 67 - src/bin/pg_rewind/libpq_fetch.c | 4 +- src/bin/pg_rewind/pg_rewind.c | 14 +- 7 files changed, 176 insertions(+), 211 deletions(-)
pgsql: Refactor pg_rewind for more clear decision making.
Refactor pg_rewind for more clear decision making. Deciding what to do with each file is now a separate step after all the necessary information has been gathered. It is more clear that way. Previously, the decision-making was divided between process_source_file() and process_target_file(), and it was a bit hard to piece together what the overall rules were. Reviewed-by: Kyotaro Horiguchi, Soumyadeep Chakraborty Discussion: https://www.postgresql.org/message-id/0c5b3783-af52-3ee5-f8fa-6e794061f70d%40iki.fi Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/eb00f1d4bf96bdba236bcc089f3ae94db9b7c603 Modified Files -- src/bin/pg_rewind/copy_fetch.c | 14 +- src/bin/pg_rewind/file_ops.c| 16 +- src/bin/pg_rewind/filemap.c | 572 +--- src/bin/pg_rewind/filemap.h | 76 -- src/bin/pg_rewind/libpq_fetch.c | 12 +- src/bin/pg_rewind/parsexlog.c | 2 +- src/bin/pg_rewind/pg_rewind.c | 8 +- 7 files changed, 386 insertions(+), 314 deletions(-)
pgsql: pg_rewind: Refactor the abstraction to fetch from local/libpq so
pg_rewind: Refactor the abstraction to fetch from local/libpq source.
This makes the abstraction of a "source" server more clear, by introducing
a common abstract class, borrowing the object-oriented programming term,
that represents all the operations that can be done on the source server.
There are two implementations of it, one for fetching via libpq, and
another to fetch from a local directory. This adds some code, but makes it
easier to understand what's going on.
The copy_executeFileMap() and libpq_executeFileMap() functions contained
basically the same logic, just calling different functions to fetch the
source files. Refactor so that the common logic is in one place, in a new
function called perform_rewind().
Reviewed-by: Kyotaro Horiguchi, Soumyadeep Chakraborty
Discussion:
https://www.postgresql.org/message-id/0c5b3783-af52-3ee5-f8fa-6e794061f70d%40iki.fi
Branch
--
master
Details
---
https://git.postgresql.org/pg/commitdiff/37d2ff38031262a1778bc76a9c55fff7afbcf275
Modified Files
--
src/bin/pg_rewind/Makefile | 5 +-
src/bin/pg_rewind/copy_fetch.c | 266 --
src/bin/pg_rewind/fetch.c | 60
src/bin/pg_rewind/fetch.h | 44 ---
src/bin/pg_rewind/file_ops.c | 133 ++-
src/bin/pg_rewind/file_ops.h | 3 +
.../pg_rewind/{libpq_fetch.c => libpq_source.c}| 395 ++---
src/bin/pg_rewind/local_source.c | 131 +++
src/bin/pg_rewind/pg_rewind.c | 208 ---
src/bin/pg_rewind/pg_rewind.h | 5 -
src/bin/pg_rewind/rewind_source.h | 73
src/tools/pgindent/typedefs.list | 5 +
12 files changed, 697 insertions(+), 631 deletions(-)
Re: pgsql: Get rid of the dedicated latch for signaling the startup process
On 04/11/2020 09:44, Fujii Masao wrote: Get rid of the dedicated latch for signaling the startup process. This commit gets rid of the dedicated latch for signaling the startup process in favor of using its procLatch, since that comports better with possible generic signal handlers using that latch. Commit 1e53fe0e70 changed background processes so that they use standard SIGHUP handler. Like that, this commit also makes the startup process use standard SIGHUP handler to simplify the code. This seems to have made buildfarm member 'elver' to segfault. I've got a hunch that setting recoveryWakeupLatch to NULL, when WakeupRecovery() doesn't check for NULL, is not OK. It's surprising that we're only seeing this on 'elver', though. - Heikki
Re: pgsql: Get rid of the dedicated latch for signaling the startup process
On 2020/11/04 19:27, Heikki Linnakangas wrote: On 04/11/2020 09:44, Fujii Masao wrote: Get rid of the dedicated latch for signaling the startup process. This commit gets rid of the dedicated latch for signaling the startup process in favor of using its procLatch, since that comports better with possible generic signal handlers using that latch. Commit 1e53fe0e70 changed background processes so that they use standard SIGHUP handler. Like that, this commit also makes the startup process use standard SIGHUP handler to simplify the code. This seems to have made buildfarm member 'elver' to segfault. I've got a hunch that setting recoveryWakeupLatch to NULL, when WakeupRecovery() doesn't check for NULL, is not OK. It's surprising that we're only seeing this on 'elver', though. Thanks for the report! The latch is reset to NULL after ShutdownWalRcv(). So I thought that there are no processes setting the latch (i.e., walreceiver has already exited) after it's reset to NULL. But this is not true. There seem a window between ShutdownWalRcv() and the actual exit of walreceiver. If a signal is sent during that window, the segmentation fault would happen. This would be the reason why segv happened in some platforms, but not in others. I'm thinking to remove the following code to fix this issue. Thought? /* * We don't need the latch anymore. It's not strictly necessary to reset * it to NULL, but let's do it for the sake of tidiness. */ if (ArchiveRecoveryRequested) XLogCtl->recoveryWakeupLatch = NULL; Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
pgsql: Enable hash partitioning of text arrays
Enable hash partitioning of text arrays hash_array_extended() needs to pass PG_GET_COLLATION() to the hash function of the element type. Otherwise, the hash function of a collation-aware data type such as text will error out, since the introduction of nondeterministic collation made hash functions require a collation, too. The consequence of this is that before this change, hash partitioning using an array over text in the partition key would not work. Reviewed-by: Heikki Linnakangas Reviewed-by: Tom Lane Reviewed-by: Michael Paquier Discussion: https://www.postgresql.org/message-id/flat/32c1fdae-95c6-5dc6-058a-a90330a3b621%40enterprisedb.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/560564d3addcb6c54e13c1ca8bd11afafbb7ba11 Modified Files -- src/backend/utils/adt/arrayfuncs.c | 2 +- src/test/regress/expected/collate.icu.utf8.out | 50 ++ src/test/regress/sql/collate.icu.utf8.sql | 26 ++ 3 files changed, 77 insertions(+), 1 deletion(-)
pgsql: Enable hash partitioning of text arrays
Enable hash partitioning of text arrays hash_array_extended() needs to pass PG_GET_COLLATION() to the hash function of the element type. Otherwise, the hash function of a collation-aware data type such as text will error out, since the introduction of nondeterministic collation made hash functions require a collation, too. The consequence of this is that before this change, hash partitioning using an array over text in the partition key would not work. Reviewed-by: Heikki Linnakangas Reviewed-by: Tom Lane Reviewed-by: Michael Paquier Discussion: https://www.postgresql.org/message-id/flat/32c1fdae-95c6-5dc6-058a-a90330a3b621%40enterprisedb.com Branch -- REL_13_STABLE Details --- https://git.postgresql.org/pg/commitdiff/82d4a2a7d63e79f6a6724f366cfaa4beed6b8326 Modified Files -- src/backend/utils/adt/arrayfuncs.c | 2 +- src/test/regress/expected/collate.icu.utf8.out | 50 ++ src/test/regress/sql/collate.icu.utf8.sql | 26 ++ 3 files changed, 77 insertions(+), 1 deletion(-)
pgsql: Enable hash partitioning of text arrays
Enable hash partitioning of text arrays hash_array_extended() needs to pass PG_GET_COLLATION() to the hash function of the element type. Otherwise, the hash function of a collation-aware data type such as text will error out, since the introduction of nondeterministic collation made hash functions require a collation, too. The consequence of this is that before this change, hash partitioning using an array over text in the partition key would not work. Reviewed-by: Heikki Linnakangas Reviewed-by: Tom Lane Reviewed-by: Michael Paquier Discussion: https://www.postgresql.org/message-id/flat/32c1fdae-95c6-5dc6-058a-a90330a3b621%40enterprisedb.com Branch -- REL_12_STABLE Details --- https://git.postgresql.org/pg/commitdiff/ea9087938131ae0da822ccb96c2d8dc71c177f53 Modified Files -- src/backend/utils/adt/arrayfuncs.c | 2 +- src/test/regress/expected/collate.icu.utf8.out | 50 ++ src/test/regress/sql/collate.icu.utf8.sql | 26 ++ 3 files changed, 77 insertions(+), 1 deletion(-)
Re: pgsql: Get rid of the dedicated latch for signaling the startup process
On 2020/11/04 20:20, Fujii Masao wrote: On 2020/11/04 19:27, Heikki Linnakangas wrote: On 04/11/2020 09:44, Fujii Masao wrote: Get rid of the dedicated latch for signaling the startup process. This commit gets rid of the dedicated latch for signaling the startup process in favor of using its procLatch, since that comports better with possible generic signal handlers using that latch. Commit 1e53fe0e70 changed background processes so that they use standard SIGHUP handler. Like that, this commit also makes the startup process use standard SIGHUP handler to simplify the code. This seems to have made buildfarm member 'elver' to segfault. I've got a hunch that setting recoveryWakeupLatch to NULL, when WakeupRecovery() doesn't check for NULL, is not OK. It's surprising that we're only seeing this on 'elver', though. Thanks for the report! The latch is reset to NULL after ShutdownWalRcv(). So I thought that there are no processes setting the latch (i.e., walreceiver has already exited) after it's reset to NULL. But this is not true. There seem a window between ShutdownWalRcv() and the actual exit of walreceiver. If a signal is sent during that window, the segmentation fault would happen. This would be the reason why segv happened in some platforms, but not in others. I'm thinking to remove the following code to fix this issue. Thought? /* * We don't need the latch anymore. It's not strictly necessary to reset * it to NULL, but let's do it for the sake of tidiness. */ if (ArchiveRecoveryRequested) XLogCtl->recoveryWakeupLatch = NULL; Or ISTM that WakeupRecovery() should set the latch only when the latch has not been reset to NULL yet. Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
pgsql: Fix segmentation fault that commit ac22929a26 caused.
Fix segmentation fault that commit ac22929a26 caused. Commit ac22929a26 changed recoveryWakeupLatch so that it's reset to NULL at the end of recovery. This change could cause a segmentation fault in the buildfarm member 'elver'. Previously the latch was reset to NULL after calling ShutdownWalRcv(). But there could be a window between ShutdownWalRcv() and the actual exit of walreceiver. If walreceiver set the latch during that window, the segmentation fault could happen. To fix the issue, this commit changes walreceiver so that it sets the latch only when the latch has not been reset to NULL yet. Author: Fujii Masao Discussion: https://postgr.es/m/[email protected] Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/113d3591b859fb8dc191bc0599d1ad62d91f1aa4 Modified Files -- src/backend/access/transam/xlog.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
Re: pgsql: Get rid of the dedicated latch for signaling the startup process
On 04/11/2020 14:03, Fujii Masao wrote: I'm thinking to remove the following code to fix this issue. Thought? /* * We don't need the latch anymore. It's not strictly necessary to reset * it to NULL, but let's do it for the sake of tidiness. */ if (ArchiveRecoveryRequested) XLogCtl->recoveryWakeupLatch = NULL; That should work, but it seems a bit sloppy to leave it pointing to a latch that belongs to a random process though. Or ISTM that WakeupRecovery() should set the latch only when the latch has not been reset to NULL yet. Got to be careful with race conditions, if the latch is set to NULL at the same time that WakeupRecovery() is called. - Heikki
pgsql: Remove useless entries for aggregate functions from fmgrtab.c.
Remove useless entries for aggregate functions from fmgrtab.c. Gen_fmgrtab.pl treated aggregate functions the same as other built-in functions, which is wasteful because there is no real need to have entries for them in the fmgr_builtins[] table. Suppressing those entries saves about 3KB in the compiled table on my machine; which is not a lot but it's not nothing either, considering that that table is pretty "hot". The only outside code change needed is that ExecInitWindowAgg() can't be allowed to call fmgr_info_cxt() on a plain aggregate function. But that saves a few cycles anyway. Having done that, the aggregate_dummy() function is unreferenced and might as well be dropped. Using "aggregate_dummy" as the prosrc value for an aggregate is now just a documentation convention not something that matters. There was some discussion of using NULL instead to save a few bytes in pg_proc, but we'd have to remove prosrc's BKI_FORCE_NOT_NULL marking which doesn't seem a great idea. Anyway, it's possible there's client-side code that expects to see "aggregate_dummy" there, so I'm loath to change it without a strong reason. Discussion: https://postgr.es/m/[email protected] Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/f21636e5d5b8394ed076e18ddc5f4ba710c69c99 Modified Files -- src/backend/catalog/pg_aggregate.c | 2 +- src/backend/executor/nodeAgg.c | 18 -- src/backend/executor/nodeWindowAgg.c | 10 +- src/backend/utils/Gen_fmgrtab.pl | 9 +++-- 4 files changed, 13 insertions(+), 26 deletions(-)
pgsql: Improve our ability to regurgitate SQL-syntax function calls.
Improve our ability to regurgitate SQL-syntax function calls. The SQL spec calls out nonstandard syntax for certain function calls, for example substring() with numeric position info is supposed to be spelled "SUBSTRING(string FROM start FOR count)". We accept many of these things, but up to now would not print them in the same format, instead simplifying down to "substring"(string, start, count). That's long annoyed me because it creates an interoperability problem: we're gratuitously injecting Postgres-specific syntax into what might otherwise be a perfectly spec-compliant view definition. However, the real reason for addressing it right now is to support a planned change in the semantics of EXTRACT() a/k/a date_part(). When we switch that to returning numeric, we'll have the parser translate EXTRACT() to some new function name (might as well be "extract" if you ask me) and then teach ruleutils.c to reverse-list that per SQL spec. In this way existing calls to date_part() will continue to have the old semantics. To implement this, invent a new CoercionForm value COERCE_SQL_SYNTAX, and make the parser insert that rather than COERCE_EXPLICIT_CALL when the input has SQL-spec decoration. (But if the input has the form of a plain function call, continue to mark it COERCE_EXPLICIT_CALL, even if it's calling one of these functions.) Then ruleutils.c recognizes COERCE_SQL_SYNTAX as a cue to emit SQL call syntax. It can know which decoration to emit using hard-wired knowledge about the functions that could be called this way. (While this solution isn't extensible without manual additions, neither is the grammar, so this doesn't seem unmaintainable.) Notice that this solution will reverse-list a function call with SQL decoration only if it was entered that way; so dump-and-reload will not by itself produce any changes in the appearance of views. This requires adding a CoercionForm field to struct FuncCall. (I couldn't resist the temptation to rearrange that struct's field order a tad while I was at it.) FuncCall doesn't appear in stored rules, so that change isn't a reason for a catversion bump, but I did one anyway because the new enum value for CoercionForm fields could confuse old backend code. Possible future work: * Perhaps CoercionForm should now be renamed to DisplayForm, or something like that, to reflect its more general meaning. This'd require touching a couple hundred places, so it's not clear it's worth the code churn. * The SQLValueFunction node type, which was invented partly for the same goal of improving SQL-compatibility of view output, could perhaps be replaced with regular function calls marked with COERCE_SQL_SYNTAX. It's unclear if this would be a net code savings, however. Discussion: https://postgr.es/m/[email protected] Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/40c24bfef92530bd846e111c1742c2a54441c62c Modified Files -- src/backend/nodes/copyfuncs.c| 3 +- src/backend/nodes/equalfuncs.c | 3 +- src/backend/nodes/makefuncs.c| 5 +- src/backend/nodes/outfuncs.c | 3 +- src/backend/parser/gram.y| 194 ++- src/backend/parser/parse_clause.c| 4 +- src/backend/parser/parse_func.c | 6 +- src/backend/parser/parse_utilcmd.c | 1 + src/backend/utils/adt/ruleutils.c| 232 ++- src/include/catalog/catversion.h | 2 +- src/include/nodes/makefuncs.h| 3 +- src/include/nodes/parsenodes.h | 5 +- src/include/nodes/primnodes.h| 8 +- src/test/modules/test_rls_hooks/test_rls_hooks.c | 10 +- src/test/regress/expected/create_view.out| 44 - src/test/regress/expected/timestamptz.out| 6 +- src/test/regress/sql/create_view.sql | 22 +++ 17 files changed, 480 insertions(+), 71 deletions(-)
pgsql: Declare lead() and lag() using anycompatible not anyelement.
Declare lead() and lag() using anycompatible not anyelement. This allows use of a "default" expression that doesn't slavishly match the data column's type. Formerly you got something like "function lag(numeric, integer, integer) does not exist", which is not just unhelpful but actively misleading. The SQL spec suggests that the default should be coerced to the data column's type, but this implementation instead chooses the common supertype, which seems at least as reasonable. (Note: I took the opportunity to run "make reformat-dat-files" on pg_proc.dat, so this commit includes some cosmetic changes to recently-added entries that aren't related to lead/lag.) Vik Fearing Discussion: https://postgr.es/m/[email protected] Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/5c292e6b90433c760a3e15027646c7b94afd0cdd Modified Files -- doc/src/sgml/func.sgml | 16 src/include/catalog/catversion.h | 2 +- src/include/catalog/pg_proc.dat | 32 +--- src/test/regress/expected/window.out | 30 ++ src/test/regress/sql/window.sql | 2 ++ 5 files changed, 58 insertions(+), 24 deletions(-)
Re: pgsql: Get rid of the dedicated latch for signaling the startup process
On 04/11/2020 15:17, Heikki Linnakangas wrote:
On 04/11/2020 14:03, Fujii Masao wrote:
Or ISTM that WakeupRecovery() should set the latch only when the latch
has not been reset to NULL yet.
Got to be careful with race conditions, if the latch is set to NULL at
the same time that WakeupRecovery() is called.
I don't think commit 113d3591b8 got this quite right:
void
WakeupRecovery(void)
{
if (XLogCtl->recoveryWakeupLatch)
SetLatch(XLogCtl->recoveryWakeupLatch);
}
If XLogCtl->recoveryWakeupLatch is set to NULL between the if and the
SetLatch, you'll still get a segfault. That's highly unlikely to happen
in practice because the compiler will optimize that into a single load
instruction, but could happen with -O0. I think you'd need to do the
access only once, using a volatile pointer, to make that safe. Maybe
it's simpler to just not reset it to NULL, after all.
- Heikki
Re: pgsql: Get rid of the dedicated latch for signaling the startup process
On 2020/11/05 5:36, Heikki Linnakangas wrote:
On 04/11/2020 15:17, Heikki Linnakangas wrote:
On 04/11/2020 14:03, Fujii Masao wrote:
Or ISTM that WakeupRecovery() should set the latch only when the latch
has not been reset to NULL yet.
Got to be careful with race conditions, if the latch is set to NULL at
the same time that WakeupRecovery() is called.
I don't think commit 113d3591b8 got this quite right:
void
WakeupRecovery(void)
{
if (XLogCtl->recoveryWakeupLatch)
SetLatch(XLogCtl->recoveryWakeupLatch);
}
If XLogCtl->recoveryWakeupLatch is set to NULL between the if and the SetLatch,
you'll still get a segfault. That's highly unlikely to happen in practice because
the compiler will optimize that into a single load instruction, but could happen
with -O0. I think you'd need to do the access only once, using a volatile pointer,
to make that safe. Maybe it's simpler to just not reset it to NULL, after all.
Yes, you're right. I agree it's simpler to remove the code resetting
the latch to NULL. Also as the comment for that code explains,
basically it's not necessary to reset it to NULL.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
pgsql: Declare assorted array functions using anycompatible not anyelem
Declare assorted array functions using anycompatible not anyelement. Convert array_append, array_prepend, array_cat, array_position, array_positions, array_remove, array_replace, and width_bucket to use anycompatiblearray. This is a simple extension of commit 5c292e6b9 to hit some other places where there's a pretty obvious gain in usability from doing so. Ideally we'd also modify other functions taking multiple old-style polymorphic arguments. But most of the remainder are tied into one or more operator classes, making any such change a much larger can of worms than I desire to open right now. Discussion: https://postgr.es/m/[email protected] Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/9e38c2bb5093ceb0c04d6315ccd8975bd17add66 Modified Files -- doc/src/sgml/func.sgml | 50 -- doc/src/sgml/xaggr.sgml| 4 +-- src/include/catalog/catversion.h | 2 +- src/include/catalog/pg_operator.dat| 13 src/include/catalog/pg_proc.dat| 40 +++- src/test/regress/expected/arrays.out | 12 +++ src/test/regress/expected/polymorphism.out | 10 +++--- src/test/regress/sql/arrays.sql| 2 ++ src/test/regress/sql/polymorphism.sql | 10 +++--- 9 files changed, 86 insertions(+), 57 deletions(-)
pgsql: Remove underflow error in float division with infinite divisor.
Remove underflow error in float division with infinite divisor. float4_div and float8_div correctly produced zero for zero divided by infinity, but threw an underflow error for nonzero finite values divided by infinity. This seems wrong; at the very least it's inconsistent with the behavior recently implemented for numeric infinities. Remove the error and allow zero to be returned. This patch also removes a useless isinf() test from the overflow checks in these functions (non-Inf divided by Inf can't produce Inf). Extracted from a larger patch; this seems significant outside the context of geometric operators, so it deserves its own commit. Kyotaro Horiguchi Discussion: https://postgr.es/m/cagf+fx70rwfok5cd00umfa__0yp+vtqg5ck7c2onb-yczp0...@mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/fac83dbd6fe1ac3d4125bfa39f287f95bffe6cda Modified Files -- src/include/utils/float.h | 8 src/test/regress/expected/float4-misrounded-input.out | 6 ++ src/test/regress/expected/float4.out | 6 ++ src/test/regress/expected/float8.out | 6 ++ src/test/regress/sql/float4.sql | 1 + src/test/regress/sql/float8.sql | 1 + 6 files changed, 24 insertions(+), 4 deletions(-)
pgsql: Fix unlinking of SLRU segments.
Fix unlinking of SLRU segments. Commit dee663f7 intended to drop any queued up fsync requests before unlinking segment files, but missed a code path. Fix, by centralizing the forget-and-unlink code into a single function. Reported-by: Tomas Vondra Discussion: https://postgr.es/m/20201104013205.icogbi773przyny5%40development Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/c732c3f8c122009de373cff9b44b5cf0992ba1bf Modified Files -- src/backend/access/transam/slru.c | 44 --- 1 file changed, 18 insertions(+), 26 deletions(-)
pgsql: Fix nbtree cleanup-only VACUUM stats inaccuracies.
Fix nbtree cleanup-only VACUUM stats inaccuracies. Logic for counting heap TIDs from posting list tuples (added by commit 0d861bbb) was faulty. It didn't count any TIDs/index tuples in the event of no callback being set. This meant that we incorrectly counted no index tuples in clean-up only VACUUMs, which could lead to pg_class.reltuples being spuriously set to 0 in affected indexes. To fix, go back to counting items from the page in cases where there is no callback. This approach isn't very accurate, but it works well enough in practice while avoiding the expense of accessing every index tuple during cleanup-only VACUUMs. Author: Peter Geoghegan Reported-By: Jehan-Guillaume de Rorthais https://postgr.es/m/20201023174451.69e358f1@firost Backpatch: 13-, where nbtree deduplication was introduced Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/48e1291342dd7771cf8c67aa1d7ec1f394b95dd8 Modified Files -- src/backend/access/nbtree/nbtree.c | 15 ++- 1 file changed, 14 insertions(+), 1 deletion(-)
pgsql: Fix nbtree cleanup-only VACUUM stats inaccuracies.
Fix nbtree cleanup-only VACUUM stats inaccuracies. Logic for counting heap TIDs from posting list tuples (added by commit 0d861bbb) was faulty. It didn't count any TIDs/index tuples in the event of no callback being set. This meant that we incorrectly counted no index tuples in clean-up only VACUUMs, which could lead to pg_class.reltuples being spuriously set to 0 in affected indexes. To fix, go back to counting items from the page in cases where there is no callback. This approach isn't very accurate, but it works well enough in practice while avoiding the expense of accessing every index tuple during cleanup-only VACUUMs. Author: Peter Geoghegan Reported-By: Jehan-Guillaume de Rorthais https://postgr.es/m/20201023174451.69e358f1@firost Backpatch: 13-, where nbtree deduplication was introduced Branch -- REL_13_STABLE Details --- https://git.postgresql.org/pg/commitdiff/02c9386ca4f706364904be2720e2d09916e2b619 Modified Files -- src/backend/access/nbtree/nbtree.c | 15 ++- 1 file changed, 14 insertions(+), 1 deletion(-)
