Make sure the launcher cancels the currently running worker by calling
TerminateBackgroundWorker when it receives SIGINT. In order to handle
cases where SIGINT arrives while tje launcher is waiting for a worker
to start or exit, implement a version of WaitForBackgroundWorkerStartup
and WaitForBackgroundWorkerShutdown which checks the abort_requested
signalling. A new test which kills processing with SIGINT is added.
Backpatch to v19 where online checksums were introduced.
Author: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/CAHGQGwEQ1-+iPQnUpTXYiHmzSz9ufFVkOK4kL_uyTdYt7jgg0Q@mail.gmail.com
Backpatch-through: 19
This commit adds a new value to StdRdOptIndexCleanup to distinguish
whether it is explicitly set, similar to ViewOptCheckOption's
VIEW_OPTION_CHECK_OPTION_NOT_SET. This changes only the internal
representation; an unset value still defaults to AUTO, and the
option accepts the same input as before.
This is preparatory work for a follow-up commit that will make use
of the new "unset" state.
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Sami Imseih <samimseih@gmail.com>
Tested-by: Greg Burd <greg@burd.me>
Tested-by: solai v <solai.cdac@gmail.com>
Discussion: https://postgr.es/m/aFRxC1W_kZU9OjJ9%40nathan
plperl_func_handler()'s stanza for handling an arrayref result in
a SETOF function could loop forever (or at least till OOM) when
given a tied array, since av_fetch won't necessarily ever return
a null pointer in that case. Be consistent with the other places
where we traverse a perl array: call av_len() once and use len+1
as the loop limit, silently ignoring any null pointers we get back
from that range of subscripts.
But actually, Perl's preferred locution for this seems to be to
use av_count() not av_len()+1. av_count() seems better since
there's less risk of forgetting to add 1. Also, both of those
functions return Size_t (or SSize_t) not int, creating at least
a theoretical overflow hazard. While we're modernizing this,
let's use the correct variable type where we can, and include an
overflow check where we can't.
Reported-by: Claude Code (via Noah Misch)
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Discussion: https://postgr.es/m/569769.1786901901@sss.pgh.pa.us
Backpatch-through: 14
This commit reimplements autovacuum_enabled as a ternary, using the
support added in commit 4d6a66f675 and following the example of
vacuum_truncate. This changes only the internal representation: an
unset value still behaves as enabled, and the option accepts the
same input as before.
This is preparatory work for a follow-up commit that will make use
of the new "unset" state.
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Greg Burd <greg@burd.me>
Reviewed-by: Sami Imseih <samimseih@gmail.com>
Tested-by: solai v <solai.cdac@gmail.com>
Discussion: https://postgr.es/m/aFRxC1W_kZU9OjJ9%40nathan
extract_autovac_opts() returns a palloc'd copy of just the
AutoVacOpts portion of a relation's reloptions, but upcoming work
needs the other StdRdOptions fields, too. Remove it in favor of
calling extractRelOptions() directly. av_relation now caches the
main table's whole StdRdOptions.
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Sami Imseih <samimseih@gmail.com>
Tested-by: Greg Burd <greg@burd.me>
Tested-by: solai v <solai.cdac@gmail.com>
Discussion: https://postgr.es/m/aFRxC1W_kZU9OjJ9%40nathan
This is part of a more global effort to modernize the code tree with
safer allocation functions, which offer more protection in terms of:
- stronger type safety guarantees
- size overflow checks, particulary for arrays.
This set of updates includes allocation changes based on
palloc_object(), palloc_array() and repalloc_array(). This work is
similar to 1b105f9472, expanding more the new APIs in the tree in an
incremental fashion.
Author: Tristan Partin <tristan@partin.io>
Discussion: https://postgr.es/m/DKJB5887LGX9.359DV1UMZ1URK@partin.io
As detected by ASAN, the scanner value used when parsing GUC files can
be indeterminate when the flex error handler sigjumps to the old cleanup
path, before yylex_init() is called.
The flex scanner state is now made volatile in ParseConfigFp(), since
its value is assigned after sigsetjmp() and cna be accessed after
siglongjmp(). yylex_init() cannot use a volatile pointer; a temporary
variable is used before assigning the result of yylex_init() to it.
While on it, yy_create_buffer() is changed to detect the case where it
returns a NULL value. Based on my read of the flex code, this cannot be
reached currently. Future upstream changes or changes in the error
logic of the GUC file parsing could make that reachable, and it is four
extra lines of code.
Oversight in d663f150b5.
Reported-by: Ilia Kashintsev <ilia.kashintsev@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Discussion: https://postgr.es/m/19612-24ccb4fc6da7786f@postgresql.org
Backpatch-through: 18
Simple EXISTS subplans can be converted to hashed ANY subplans as
an alternative implementation. Previously, both the original EXISTS
subplan and the converted ANY subplan used names with the
exists_ prefix, making EXPLAIN output harder to read and
pg_plan_advice targets less clear.
Use the exists_to_any_ prefix for converted ANY subplans so that
their names distinguish them from the original EXISTS alternative.
This only changes the names shown by EXPLAIN and used by plan advice;
it does not affect planner behavior.
Author: Yugo Nagata <nagata@sraoss.co.jp>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: solai v <solai.cdac@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/20260605165641.3950f99ace0aad8f807abe96@sraoss.co.jp
Backpatch-through: 19
Presently, the documentation for logging_collector says that the
collector "is designed to never lose messages," which reads as a
stronger promise than we actually make. The collector does not
fsync the log file or retry failed writes, so log messages can go
missing after an operating system crash, power loss, or a write
error. Reword that sentence and add a note about what is not
guaranteed.
Author: Daniel Bauman <danielbaniel@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Zhenwei Shang <a934172442@gmail.com>
Reviewed-by: Robert Treat <rob@xzilla.net>
Discussion: https://postgr.es/m/CAMtj0_a86DdDKkW-ReVpQpqjndVS6GMrwXVpQY4G3-SGY7saMQ%40mail.gmail.com
Backpatch-through: 14
Commit 13b935cd changed hash_get_num_entries to return int64 instead
of long. This fixes a few more users of hash_get_num_entries which
were missed in the original commit.
PGSS_FILE_HEADER is left unchanged since this only affects as of yet
unreleased versions of PostgreSQL. Backpatch to v19 where the int64
change was performed.
Author: Karina Litskevich <litskevichkarina@gmail.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/CACiT8iYTkc33YWaA2D3t51Y5s=GqBO7T1zX7bkpSmet2njcLLw@mail.gmail.com
Backpatch-through: 19
PQfn() has been documented as somewhat obsolete since commit
efc3a25bb0 (2003), and commit bd48114937 recently marked it unsafe
because it may write past the end of result_buf when result_is_int
is 0. Furthermore, searches of publicly available code turned up
no callers, only language bindings that expose the function
without using it, so removing it seems unlikely to cause too much
trouble.
Since the symbol is exported, deleting it outright isn't an option,
so instead this commit teaches PQfn() to fail unconditionally with
an error that suggests alternatives. The documentation for the
fast-path interface is replaced with a stub in the "Obsolete or
Renamed Features" appendix. The server's fast-path support is
retained for the benefit of older clients and the frontend large
object interface.
The frontend large object interface, the only in-tree caller, now
uses PQnfn(), the private version of PQfn() added by commit
bd48114937. I considered converting this code away from the
fast-path protocol entirely, but prepared statements can be
deallocated out from under libpq (e.g., by DISCARD ALL), and
PQexecParams() proved noticeably slower, so that is left as a
future exercise.
Reviewed-by: Christoph Berg <myon@debian.org>
Reviewed-by: Jacob Champion <jacob.champion@enterprisedb.com>
Reviewed-by: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Discussion: https://postgr.es/m/ahXE28klgxIJXBLq%40nathan
When SendQuery() is not told how many COPY FROM STDIN commands the
query string contains (as for -c, \gexec, and \watch), it scans the
string to count them itself. But it called psql_scan() only once,
which stops at the first semicolon, so any COPY FROM STDIN past the
first sub-command was not counted, causing failure of cases that used
to work. Oversight in commit 3045a25ba.
Author: Zsolt Parragi <zsolt.parragi@percona.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAN4CZFPqa6c+u4uX5jJ8LANHTQ4dxM3m4_8G9WmX_A4-2wuv2A@mail.gmail.com
Backpatch-through: 14
The option added by commit 28972b6fc was named "restore_stats", because
it was calling pg_restore_relation_stats()/pg_restore_attribute_stats()
in place of fetching a remote rowsample. However, those code paths have
been replaced with calls to
import_relation_statistics()/import_attribute_statistics() (cf. commit
54cd6fc83), so the option is now slightly misnamed.
Switch the option to "import_stats", which more closely reflects both
the functions being called internally, and the user's perception of what
the operation is doing.
Back-patch to v19 where commit 28972b6fc went in.
Suggested-by: Etsuro Fujita <etsuro.fujita@gmail.com>
Author: Corey Huinker <corey.huinker@gmail.com>
Discussion: https://postgr.es/m/CADkLM%3DezuspTk3VD-2EM6XtFP-Dfz7cJKtnbn0cB6gocsAgu1g%40mail.gmail.com
Backpatch-through: 19
injection_points is extended so as a caller can attach a string value to
a point, checked with a value provided at runtime. There are a couple
of use cases where this can be useful, to be able to target points for
specific object names.
PID and string conditions can be combined together, so as a point can be
allowed for a specific process and string value at the same time.
The condition string is capped at 255 characters. As the attach SQL
function now takes an optional condition string, it is no longer STRICT,
and rejects NULL inputs. Empty condition strings are also rejected, as
they don't make sense.
Author: Sami Imseih <samimseih@gmail.com>
Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/CAA5RZ0tsGHu2h6YLnVu4HiK05q+gTE_9WVUAqihW2LSscAYS-g@mail.gmail.com
This new structure is split depending on the stats kind it deals with:
- PGSTAT_KIND_RELATION, for tables.
- PGSTAT_KIND_INDEX, for indexes.
This change designs a cleaner barrier for the handling of stats data
related to tables and indexes, by being able to track precisely what are
the counters used by one or the other for pending data. Using a common
ground for both eases the tracking of Relations in the relcache, with a
structure based on a union separated by stats kind.
The code originally pointed to some counters that may not be used at
all. For example, indexes have no need for the tracking of
sub-transaction data, still the code implied that these could be touched
for an index. A consequence is that PgStat_TableXactStatus is not
renamed, as it is not used by indexes.
Some asserts are added as an extra layer of protection to prevent the
update of fields that should not be touched. find_tabstat_entry() and
find_tabstat_entry_kind() are removed, replaced by a single
find_relstat_entry_kind() able to work for indexes and the rest.
72a6dad1c9 took care of the shared memory and on-disk part of the
split. This focuses on the pending data part at backend level.
Author: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion: https://postgr.es/m/f572abe7-a1bb-e13b-48c7-2ca150546822@gmail.com
This commit touches both the libpq and backend-side code of channel
binding where respectively pgtls_get_peer_certificate_hash() and
be_tls_get_certificate_hash() are upgraded to retrieve digests using the
method recommended by OpenSSL 3.0: no more direct EVP_sha256() or
similar, just a EVP_MD_fetch() through EVP to get an algorythm type,
based on a name.
The pre-3.0 code is still required for LibreSSL and as long as we
support OpenSSL 1.1.1.
Similar work has been done in b91f79cd08 and 1f3b9bb109.
Author: Mark Atwood <mark@reviewcommit.com>
Co-authored-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/178596055149.1584250.13974609482797470185@reviewcommit.com
Presently, check_role_membership_authorization() decides whether
the current user may grant or revoke membership in a role by
calling is_admin_of_role(), which recurses through all grants,
while it chooses the grantor to record for the resulting entry by
calling select_best_admin(), which recurses only through inherited
grants. When the two disagree, the permission check passes and the
grantor lookup then comes up empty, so the user sees an internal
"no possible grantors" error. ALTER GROUP ... ADD USER reaches the
same error through the separate check in AlterRole().
To fix, teach both checks to search the same way
select_best_admin() does via a new has_admin_privs_of_role(). The
new check passes exactly when the grantor lookup was going to
succeed, so nothing that works today starts failing; the internal
error simply becomes a proper permission error. Note that this
leaves the other callers of is_admin_of_role() alone, so a role
reachable only through a non-inherited grant can still be dropped,
renamed, or altered. Whether that ought to change as well is left
as a future exercise.
Oversight in commit ce6b672e44.
Reported-by: ChangAo Chen <cca5507@qq.com>
Author: ChangAo Chen <cca5507@qq.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Pretham <prezza672@gmail.com>
Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Reviewed-by: Jacob Champion <jacob.champion@enterprisedb.com>
Discussion: https://postgr.es/m/tencent_ADCE2B34B230A9B631854806104FEF40C105%40qq.com
Discussion: https://postgr.es/m/CAJUn_kN%2BMhbb8fYP5xxQCq1KEziOinM6HgYx4ts_pPDnQ2y1nQ%40mail.gmail.com
Backpatch-through: 16
The introduction of the REPACK command also added support for
CLUSTER (ANALYZE). Presumably this was unintentional, as the
CLUSTER docs make no mention of it. CLUSTER (ANALYZE) on an
ordinary table succeeds and does indeed analyze the table, but a
database-wide CLUSTER (ANALYZE) or one on a partitioned table
fails with an error like:
ERROR: cannot execute REPACK (ANALYZE) on multiple tables
Rather than improving support for CLUSTER (ANALYZE) and thereby
encouraging folks to use CLUSTER instead of REPACK, let's just
reject it. This commit does so by teaching ExecRepack() to ERROR
for the ANALYZE option in anything except REPACK commands. Note
that VACUUM (FULL, ANALYZE) does not go through ExecRepack() and
therefore is unaffected by this change.
Oversight in commit ac58465e06.
Reported-by: Zsolt Parragi <zsolt.parragi@percona.com>
Author: Zsolt Parragi <zsolt.parragi@percona.com>
Discussion: https://postgr.es/m/CAN4CZFMVcgv1b2G-i%2Bkhs2MDnzWSr7O_53j_x%2BuhAxUV5rwWqw%40mail.gmail.com
Backpatch-through: 19
Some places rejected individual tokens longer than MAXSTRLEN, while
others rejected ones longer than MAXSTRLEN-1. The data structure is
perfectly capable of handling MAXSTRLEN, so there's nothing wrong
with using the looser bound. Moreover, as things stand there is a
dump/reload hazard: some code paths permit construction of a tsvector
or tsquery that would later be rejected by tsvectorin or tsqueryin.
So standardize on using MAXSTRLEN.
Identical remarks apply to MAXSTRPOS (the total data length),
so fix that too.
Back-patch, in hopes of avoiding cases where a value acceptable to
one supported release is not acceptable to another.
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Zsolt Parragi <zsolt.parragi@percona.com>
Discussion: https://postgr.es/m/CAN4CZFNYQo4zfbRR435uD0vSfuy5y7dnFOXDfKr9zYoL1JnAxA@mail.gmail.com
Backpatch-through: 14
This was harmless because the function (ThereAreNoReadyPortals()) has
only one caller, and that caller throws an error if the function
returns false and transaction abort cleans up the in-progress hash
searches. But if someone called the function in some other way,
they'd get a warning about a leaked hash_seq_search scan.
mdc_finish used to set this field once it had successfully verified
the MDC packet, with its only reader in the same function. This was
removed by a59ee88197 when mdc_finish stopped being called through
the pullf_read API, the field has been unused since.
Note that MDC verification does not rely on it: process_data_packets
tracks whether an MDC packet was seen in its local got_mdc variable
and rejects the message if a required MDC is missing, while the
use_mdcbuf_filter path checks the hash in mdcbuf_finish().
Author: Aleksander Alekseev <aleksander@tigerdata.com>
Discussion: https://postgr.es/m/CAJ7c6TNmHnnCkn3uZwRUE7WWYftW--48JFoDVDqoZqMrLYxUZw@mail.gmail.com
When vacuumdb --analyze-only or --analyze-in-stages enumerates
tables itself, it can select both a partitioned table and its
partitions as separate work items. Previously, it generated plain
ANALYZE for the partitioned table, which recursively analyzed its
partitions as well.
This caused duplicate work when the partitions were already selected by
vacuumdb. With --analyze-in-stages, the redundant work could be
repeated at every stage.
Fix this by generating ANALYZE ONLY for automatically selected
partitioned tables when connected to servers that support it. This
updates inherited statistics for the partitioned table while leaving
per-partition statistics to the separately selected partition entries.
For older servers that do not support ANALYZE ONLY, vacuumdb falls back
to the previous plain ANALYZE command.
This also applies when partitioned tables are selected by schema
filters. In that case, partitions matching the filters are analyzed as
separate targets, while per-partition statistics for partitions outside
the filters are not updated via the partitioned parent.
This commit does not change the behavior for partitioned tables
specified explicitly with --table, i.e., in that case, vacuumdb
continues to generate plain ANALYZE, recursively analyzing the specified
partitioned table and its partitions.
Suggested-by: Justin Pryzby <pryzby@telsasoft.com>
Author: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/aKZ7lMYGnnIo35c0@pryzbyj2023
Backpatch-through: 19
Clarify that transactions failed under --continue-on-error are reported
separately and are not counted as transactions actually processed, while
time spent on them can lower reported TPS. Also adjust the latency
description to avoid implying that failed transactions are generally included
in latency measurements.
Backpatch to v19, where pgbench's --continue-on-error option was introduced.
Author: Chao Li <lic@highgo.com>
Reviewed-by: Yugo Nagata <nagata@sraoss.co.jp>
Reviewed-by: Xuneng Zhou <xunengzhou@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/597C9755-4386-488D-A289-16D50AD81FBF@gmail.com
Backpatch-through: 19
ALTER TABLE ... REPLICA IDENTITY USING INDEX verified key columns by
reading pg_attribute.attnotnull, but commit a379061a22 made
attnotnull true also for unvalidated (NOT VALID) not-null constraints,
which do not prove the column null-free. An index over such a column
could thus be marked as replica identity even though the column might
contain NULLs, causing apply-side divergence for UPDATE/DELETE on the
nullable rows.
Fix by additionally requiring convalidated for the underlying
constraint, mirroring the fix d9ffc27291 applied to ATExecAddIdentity
for the analogous identity-column case.
Author: Ante Krešić <ante@tigerdata.com>
Reviewed-by: Aleksander Alekseev <aleksander@tigerdata.com>
Reviewed-by: solai v <solai.cdac@gmail.com>
Backpatch-through: 18
Discussion: https://postgr.es/m/CABXQ4dJUibZzN91qvWmsfA7MUDn9YRCNyu3CcukdyokbH1=41Q@mail.gmail.com
Commit 649828769 improved this code to keep Sxx, Syy, and Sxy
exactly zero so long as we see only a single value of the input(s).
However, if any values of the other input are Inf or NaN, we'd
better set Sxy to NaN instead. Otherwise we risk reporting zero
variance when the result is really undefined. The old coding handled
this implicitly, but in the short-circuit path we have to take care
of it explicitly.
Bug: #19615
Reported-by: Junwen An <feasiblechart@gmail.com>
Author: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/19615-c7e390593416f6b6@postgresql.org
Backpatch-through: 19
Commit db89a47115 added an 'options' argument to table_tuple_update()
and table_tuple_delete(), correctly documenting it as recognizing no
values. It also (correctly) added pg_attribute_unused() to the
corresponding heap_update() argument. Commit 28d534e2ae then added
TABLE_UPDATE_NO_LOGICAL and TABLE_DELETE_NO_LOGICAL, but failed to
(correctly) update the comment and heap_update()'s signature.
In table_tuple_update(), move the correct explanation of 'options' to
the right place, remove the bogus one.
In table_tuple_delete(), add the missing TABLE_DELETE_NO_LOGICAL doc.
In heap_update(), remove the pg_attribute_unused() marker.
Author: Nikhil Sontakke <nikhil@planetscale.com>
Backpatch-through: 19
Discussion: https://postgr.es/m/CA+UBoq21SzkjThYMSwSpVU-jwzpuEsf22Hq1FPbcKcDc43C29g@mail.gmail.com
pg_waldump (--start/--end), pg_recvlogical (--startpos/--endpos), and
pg_receivewal (--endpos) parsed user-supplied WAL locations with
sscanf("%X/%08X"). That accepts several forms rejected by pg_lsn input
and can proceed with a different location than the user specified:
overlong components, trailing characters, leading whitespace, signs, and
0x prefixes.
Add pg_parse_lsn() to src/common and use it for those command-line
options. The helper accepts the pg_lsn text syntax only: one to eight
hexadecimal digits, a slash, one to eight hexadecimal digits, and no
trailing characters. Each frontend tool keeps its existing error
message.
Use the same helper from pg_lsn_in_safe(), leaving only backend-specific
soft error reporting there. This keeps frontend command-line validation
and backend pg_lsn input tied to one parser.
Bug: #19598
Reported-by: Michael Malis <malis@pgrust.com>
Author: Zexin Li <lizi.openmind@gmail.com>
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/19598-aa67c8f4331611b4@postgresql.org
A buildfarm failure in subscription/038_walsnd_shutdown_timeout was
diagnosed to the background psql session timing out due to a backup
taking a long time. The session was using a single timeout for all
queries, so switch to resetting the timer for each query to make it
survive slow backups due to constrained buildfarm machines.
Backpatch to v19 where the test was introduced.
Author: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Discussion: https://postgr.es/m/OS9PR01MB12149AA18EEA475D2AA22FD3BF5D12@OS9PR01MB12149.jpnprd01.prod.outlook.com
Backpatch-through: 19
On timeout, poll_query_until() would fail to print its report when
"$query" was undefined. Instead of the expected report, this prevented
the contents of stdout and stderr from being shown.
Note that some of the in-core tests use an undefined query for
connection-only polls.
Author: Bryan Green <dbryan.green@gmail.com>
Reviewed-by: Jonathan Gonzalez V. <jonathan.abdiel@gmail.com>
Discussion: https://postgr.es/m/0f515c6d-6032-4c5f-80ac-5c78faae9522@gmail.com
Backpatch-through: 14
REFRESH MATERIALIZED VIEW currently calls SetMatViewPopulatedState()
even when the materialized view's relispopulated value already matches
the state requested by the command. That performs a catalog update of
pg_class without changing the visible state of the materialized view.
The extra update creates a dead pg_class tuple and causes the usual
catalog-update side effects, including relcache invalidation and WAL,
for no semantic benefit. The effect is especially visible for
REFRESH MATERIALIZED VIEW CONCURRENTLY, where the refresh updates the
materialized view contents with DML and otherwise need not change
pg_class just to keep relispopulated set to true.
Check the current populated state before calling SetMatViewPopulatedState(),
and update pg_class only when the value would actually change. This
preserves the required state transitions for WITH DATA and WITH NO DATA
while avoiding catalog churn for repeated refreshes that leave the
populated state unchanged.
Author: ChangAo Chen <cca5507@qq.com>
Reviewed-by: Zizhuan Liu <44973863@qq.com>
Reviewed-by: Adam Brusselback <adambrusselback@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/tencent_DF2D8533B5412908326F7806075B7637A108@qq.com
This commit splits the relation and index stats data, where the index
data is now moved to a new kind called PGSTAT_KIND_INDEX. A benefit of
this refactoring is a reduction of the shared memory required for each
index, the new PgStat_StatIdxEntry including only the fields required
for indexes. This leads to less data flushed to disk on shutdown.
Another benefit is that the addition of new fields for relation-level
stats is easier to think about. Note that indexes have no need for most
of the xact-level fields that relations care about, clarifying the role
of each counter for each relkind.
System views are updated to use the index-level functions. Note this
split means a possible break for tools that had the idea to call the
SQL functions for relation stats (not documented), as now these would
return 0 when queried for an index. These tools can use the new index
functions to achieve the same result.
As of this change, the pending statistics are still controlled by
PgStat_TableStatus for both relation and index kinds. This is left as a
follow-up piece, this one being good enough on its own. This commit is
focused on the changes required for the SQL interface, shared memory
management and the on-disk format.
Bump catalog version.
Bump PGSTAT_FILE_FORMAT_ID.
Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Author: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/f572abe7-a1bb-e13b-48c7-2ca150546822@gmail.com
On the first LockAcquire for a lock tag, LockAcquireExtended() creates
a LOCALLOCK entry in the hash table LockMethodLocalHash, with lockOwners
set to NULL. lockOwners is then allocated in TopMemoryContext.
If the allocation fails, the hash entry would persist with lockOwners
still NULL and maxLockOwners already set to 8. A follow-up
LockAcquire() on the same entry would crash on a NULL pointer
dereference.
This commit adds an extra layer of safety for such a case, by
re-attempting MemoryContextAlloc() if a LOCALLOCK entry exists but its
lockOwners is still NULL due to a previous in-flight allocation failure.
This problem is unlikely going to show up in practice, so no backpatch
is done.
Author: Bryan Green <dbryan.green@gmail.com>
Author: Mark Dilger <mark.dilger@enterprisedb.com>
Discussion: https://postgr.es/m/c9a5eeb1-4d31-46e1-bb9e-33448a23a5d3@gmail.com
