agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feed[PATCH v5 01/12] review docs for pg12dev
5+ messages / 5 participants
[nested] [flat]
* [PATCH v5 01/12] review docs for pg12dev
@ 2019-03-28 23:50 Justin Pryzby <pryzbyj@telsasoft.com>
0 siblings, 0 replies; 5+ messages in thread
From: Justin Pryzby @ 2019-03-28 23:50 UTC (permalink / raw)
---
doc/src/sgml/catalogs.sgml | 4 ++--
doc/src/sgml/config.sgml | 14 +++++++------
doc/src/sgml/ddl.sgml | 16 +++++++-------
doc/src/sgml/ecpg.sgml | 16 +++++++-------
doc/src/sgml/monitoring.sgml | 43 +++++++++++++++++++-------------------
doc/src/sgml/perform.sgml | 8 +++----
doc/src/sgml/protocol.sgml | 6 +++---
doc/src/sgml/ref/create_index.sgml | 4 ++--
doc/src/sgml/ref/pg_rewind.sgml | 13 ++++++------
doc/src/sgml/ref/pgbench.sgml | 7 +++----
doc/src/sgml/ref/vacuum.sgml | 6 +++---
doc/src/sgml/runtime.sgml | 4 ++--
doc/src/sgml/sources.sgml | 18 ++++++++--------
doc/src/sgml/xfunc.sgml | 12 +++++------
src/bin/pg_upgrade/check.c | 6 +++---
15 files changed, 87 insertions(+), 90 deletions(-)
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 3428a7c..47482f2 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -3060,7 +3060,7 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l
simplifies <command>ATTACH/DETACH PARTITION</command> operations:
the partition dependencies need only be added or removed.
Example: a child partitioned index is made partition-dependent
- on both the partition table it is on and the parent partitioned
+ on both the table partition and the parent partitioned
index, so that it goes away if either of those is dropped, but
not otherwise. The dependency on the parent index is primary,
so that if the user tries to drop the child partitioned index,
@@ -3123,7 +3123,7 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l
Note that it's quite possible for two objects to be linked by more than
one <structname>pg_depend</structname> entry. For example, a child
partitioned index would have both a partition-type dependency on its
- associated partition table, and an auto dependency on each column of
+ associated table partition, and an auto dependency on each column of
that table that it indexes. This sort of situation expresses the union
of multiple dependency semantics. A dependent object can be dropped
without <literal>CASCADE</literal> if any of its dependencies satisfies
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index aaab1c5..fa8b40a 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -3433,7 +3433,7 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows
current when the base backup was taken. The
value <literal>latest</literal> recovers
to the latest timeline found in the archive, which is useful in
- a standby server. <literal>latest</literal> is the default.
+ a standby server. The default is <literal>latest</literal>.
</para>
<para>
@@ -4195,8 +4195,9 @@ ANY <replaceable class="parameter">num_sync</replaceable> ( <replaceable class="
This parameter is intended for use with streaming replication deployments;
however, if the parameter is specified it will be honored in all cases.
- <varname>hot_standby_feedback</varname> will be delayed by use of this feature
- which could lead to bloat on the master; use both together with care.
+ <varname>hot_standby_feedback</varname> will be delayed by use of this feature.
+ Combinining these settings could lead to bloat on the master, so should
+ be done only with care.
<warning>
<para>
@@ -5305,9 +5306,10 @@ SELECT * FROM parent WHERE key = 2400;
<listitem>
<para>
Prepared statements (either explicitly prepared or implicitly
- generated, for example in PL/pgSQL) can be executed using custom or
- generic plans. A custom plan is replanned for a new parameter value,
- a generic plan is reused for repeated executions of the prepared
+ generated, for example in PL/pgSQL) can be executed using a custom or
+ generic plan. If executed with a custom plan, the prepared statement
+ is replanned for its specific parameter values; if executed with a
+ generic plan, the same plan is reused for repeated executions of the prepared
statement. The choice between them is normally made automatically.
This setting overrides the default behavior and forces either a custom
or a generic plan. This can be used to work around performance
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index ed2d9c6..cf390a2 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -3868,12 +3868,12 @@ CREATE INDEX ON measurement (logdate);
<para>
Normally the set of partitions established when initially defining the
- table are not intended to remain static. It is common to want to
- remove old partitions of data and periodically add new partitions for
+ table are not intended to remain static. It is common to
+ remove partitions of old data and add partitions for
new data. One of the most important advantages of partitioning is
- precisely that it allows this otherwise painful task to be executed
+ allowing this otherwise painful task to be executed
nearly instantaneously by manipulating the partition structure, rather
- than physically moving large amounts of data around.
+ than physically moving around large amounts of data.
</para>
<para>
@@ -3951,10 +3951,10 @@ ALTER TABLE measurement ATTACH PARTITION measurement_y2008m02
</para>
<para>
- As explained above, it is possible to create indexes on partitioned tables
- and they are applied automatically to the entire hierarchy. This is very
- convenient, as not only the existing partitions will become indexed, but
- also any partitions that are created in the future will. One limitation is
+ As explained above, it is possible to create indexes on partitioned tables;
+ such indexes are propagated automatically to the entire hierarchy, and
+ any partitions created in the future will be indexed, in addition to the
+ existing partitions. One limitation is
that it's not possible to use the <literal>CONCURRENTLY</literal>
qualifier when creating such a partitioned index. To overcome long lock
times, it is possible to use <command>CREATE INDEX ON ONLY</command>
diff --git a/doc/src/sgml/ecpg.sgml b/doc/src/sgml/ecpg.sgml
index 6641eee..d2dd96c 100644
--- a/doc/src/sgml/ecpg.sgml
+++ b/doc/src/sgml/ecpg.sgml
@@ -979,7 +979,7 @@ EXEC SQL END DECLARE SECTION;
<para>
The other way is using the <type>VARCHAR</type> type, which is a
- special type provided by ECPG. The definition on an array of
+ special type provided by ECPG. The definition of an array of
type <type>VARCHAR</type> is converted into a
named <type>struct</type> for every variable. A declaration like:
<programlisting>
@@ -989,7 +989,7 @@ VARCHAR var[180];
<programlisting>
struct varchar_var { int len; char arr[180]; } var;
</programlisting>
- The member <structfield>arr</structfield> hosts the string
+ The member <structfield>arr</structfield> stores the string
including a terminating zero byte. Thus, to store a string in
a <type>VARCHAR</type> host variable, the host variable has to be
declared with the length including the zero byte terminator. The
@@ -1209,8 +1209,8 @@ EXEC SQL END DECLARE SECTION;
<title id="ecpg-type-bytea">bytea</title>
<para>
- The handling of the <type>bytea</type> type is also similar to
- the <type>VARCHAR</type>. The definition on an array of type
+ The handling of the <type>bytea</type> type is similar to
+ the <type>VARCHAR</type>. The definition of an array of type
<type>bytea</type> is converted into a named struct for every
variable. A declaration like:
<programlisting>
@@ -1220,9 +1220,8 @@ bytea var[180];
<programlisting>
struct bytea_var { int len; char arr[180]; } var;
</programlisting>
- The member <structfield>arr</structfield> hosts binary format
- data. It also can handle even <literal>'\0'</literal> as part of
- data unlike <type>VARCHAR</type>.
+ The member <structfield>arr</structfield> stores binary format
+ data, which can include zero bytes, unlike <type>VARCHAR</type>.
The data is converted from/to hex format and sent/received by
ecpglib.
</para>
@@ -7571,8 +7570,7 @@ PREPARE <replaceable class="parameter">name</replaceable> FROM <replaceable clas
<listitem>
<para>
A literal C string or a host variable containing a preparable
- statement, one of the SELECT, INSERT, UPDATE, or
- DELETE.
+ statement (SELECT, INSERT, UPDATE, or DELETE).
</para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index bf72d0c..7cec551 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -271,13 +271,13 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser
<para>
Some of the information in the dynamic statistics views shown in <xref
linkend="monitoring-stats-dynamic-views-table"/> is security restricted.
- Ordinary users can only see all the information about their own sessions
- (sessions belonging to a role that they are a member of). In rows about
+ Ordinary users can only see all information about their own sessions
+ (sessions belonging to a role of which they are a member). In rows for
other sessions, many columns will be null. Note, however, that the
existence of a session and its general properties such as its sessions user
and database are visible to all users. Superusers and members of the
built-in role <literal>pg_read_all_stats</literal> (see also <xref
- linkend="default-roles"/>) can see all the information about all sessions.
+ linkend="default-roles"/>) can see all information about all sessions.
</para>
<table id="monitoring-stats-dynamic-views-table">
@@ -667,7 +667,7 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser
<entry><type>timestamp with time zone</type></entry>
<entry>Time when this process' current transaction was started, or null
if no transaction is active. If the current
- query is the first of its transaction, this column is equal to the
+ query is the first of its transaction, this is equal to the
<structfield>query_start</structfield> column.
</entry>
</row>
@@ -3603,16 +3603,16 @@ SELECT pg_stat_get_backend_pid(s.backendid) AS pid,
<entry><structfield>partitions_total</structfield></entry>
<entry><type>bigint</type></entry>
<entry>
- When creating an index on a partitioned table, this column is set to
- the total number of partitions on which the index is to be created.
+ When creating an index on a partitioned table, this is
+ the total number of partitions to be processed.
</entry>
</row>
<row>
<entry><structfield>partitions_done</structfield></entry>
<entry><type>bigint</type></entry>
<entry>
- When creating an index on a partitioned table, this column is set to
- the number of partitions on which the index has been completed.
+ When creating an index on a partitioned table, this is
+ the number of partitions for which the process is complete.
</entry>
</row>
</tbody>
@@ -3650,11 +3650,10 @@ SELECT pg_stat_get_backend_pid(s.backendid) AS pid,
<row>
<entry><literal>building index</literal></entry>
<entry>
- The index is being built by the access method-specific code. In this phase,
- access methods that support progress reporting fill in their own progress data,
- and the subphase is indicated in this column. Typically,
+ The index is being built by code specific to the access method, and if
+ it supports progress reporting, this indicates the method's subphase.
<structname>blocks_total</structname> and <structname>blocks_done</structname>
- will contain progress data, as well as potentially
+ will contain progress data, as may
<structname>tuples_total</structname> and <structname>tuples_done</structname>.
</entry>
</row>
@@ -3739,15 +3738,15 @@ SELECT pg_stat_get_backend_pid(s.backendid) AS pid,
<title>VACUUM Progress Reporting</title>
<para>
- Whenever <command>VACUUM</command> is running, the
- <structname>pg_stat_progress_vacuum</structname> view will contain
- one row for each backend (including autovacuum worker processes) that is
- currently vacuuming. The tables below describe the information
+ The <structname>pg_stat_progress_vacuum</structname> view will contain one
+ row for each backend that is running <command>VACUUM</command>, including
+ autovacuum worker processes.
+ The tables below describe the information
that will be reported and provide information about how to interpret it.
Progress for <command>VACUUM FULL</command> commands is reported via
- <structname>pg_stat_progress_cluster</structname>
- because both <command>VACUUM FULL</command> and <command>CLUSTER</command>
- rewrite the table, while regular <command>VACUUM</command> only modifies it
+ <structname>pg_stat_progress_cluster</structname>,
+ because <command>VACUUM FULL</command> rewrites the table, like <command>CLUSTER</command>,
+ while regular <command>VACUUM</command> only modifies it
in place. See <xref linkend='cluster-progress-reporting'/>.
</para>
@@ -3931,9 +3930,9 @@ SELECT pg_stat_get_backend_pid(s.backendid) AS pid,
<title>CLUSTER Progress Reporting</title>
<para>
- Whenever <command>CLUSTER</command> or <command>VACUUM FULL</command> is
- running, the <structname>pg_stat_progress_cluster</structname> view will
- contain a row for each backend that is currently running either command.
+ The <structname>pg_stat_progress_cluster</structname> view will contain a
+ row for each backend that is running either
+ <command>CLUSTER</command> or <command>VACUUM FULL</command>.
The tables below describe the information that will be reported and
provide information about how to interpret it.
</para>
diff --git a/doc/src/sgml/perform.sgml b/doc/src/sgml/perform.sgml
index 8e16583..0e34652 100644
--- a/doc/src/sgml/perform.sgml
+++ b/doc/src/sgml/perform.sgml
@@ -899,10 +899,10 @@ EXPLAIN ANALYZE SELECT * FROM tenk1 WHERE unique1 < 100 AND unique2 > 9000
Generally, the <command>EXPLAIN</command> output will display details for
every plan node which was generated by the query planner. However, there
are cases where the executor is able to determine that certain nodes are
- not required; currently, the only node types to support this are the
- <literal>Append</literal> and <literal>MergeAppend</literal> nodes. These
- node types have the ability to discard subnodes which they are able to
- determine won't contain any records required by the query. It is possible
+ not required; currently, the only node types to support this are
+ <literal>Append</literal> and <literal>MergeAppend</literal>, which
+ are able to discard subnodes when it's deduced that
+ they will not contain any records required by the query. It is possible
to determine that nodes have been removed in this way by the presence of a
"Subplans Removed" property in the <command>EXPLAIN</command> output.
</para>
diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index b20f169..70f7286 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -1515,7 +1515,7 @@ SELECT 1/0;
the server will only accept encrypted packets from the client which are less
than 16kB; <function>gss_wrap_size_limit()</function> should be used by the
client to determine the size of the unencrypted message which will fit
- within this limit and larger messages should be broken up into multiple
+ within this limit; larger messages should be broken up into multiple
<function>gss_wrap()</function> calls. Typical segments are 8kB of
unencrypted data, resulting in encrypted packets of slightly larger than 8kB
but well within the 16kB maximum. The server can be expected to not send
@@ -1531,8 +1531,8 @@ SELECT 1/0;
support to <productname>PostgreSQL</productname>. In this case the
connection must be closed, but the frontend might choose to open a fresh
connection and proceed without requesting <acronym>GSSAPI</acronym>
- encryption. Given the length limits specified above, the ErrorMessage can
- not be confused with a proper response from the server with an appropriate
+ encryption. Given the length limits specified above, the ErrorMessage
+ cannot be confused with a proper response from the server with an appropriate
length.
</para>
diff --git a/doc/src/sgml/ref/create_index.sgml b/doc/src/sgml/ref/create_index.sgml
index 629a31e..713e2c3 100644
--- a/doc/src/sgml/ref/create_index.sgml
+++ b/doc/src/sgml/ref/create_index.sgml
@@ -181,8 +181,8 @@ CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ [ IF NOT EXISTS ] <replaceable class=
</para>
<para>
- Currently, the B-tree and the GiST index access methods support this
- feature. In B-tree and the GiST indexes, the values of columns listed
+ Currently, only the B-tree and GiST index access methods support this
+ feature. In B-tree and GiST indexes, the values of columns listed
in the <literal>INCLUDE</literal> clause are included in leaf tuples
which correspond to heap tuples, but are not included in upper-level
index entries used for tree navigation.
diff --git a/doc/src/sgml/ref/pg_rewind.sgml b/doc/src/sgml/ref/pg_rewind.sgml
index 4d91eeb..f39fe53 100644
--- a/doc/src/sgml/ref/pg_rewind.sgml
+++ b/doc/src/sgml/ref/pg_rewind.sgml
@@ -106,15 +106,14 @@ PostgreSQL documentation
</para>
<para>
- <application>pg_rewind</application> will fail immediately if it finds
- files it cannot write directly to. This can happen for example when
- the source and the target server use the same file mapping for read-only
- SSL keys and certificates. If such files are present on the target server
+ <application>pg_rewind</application> will fail immediately if it experiences
+ a write error. This can happen for example when
+ the source and target server use the same file mapping for read-only
+ SSL keys and certificates. If such files are present on the target server,
it is recommended to remove them before running
- <application>pg_rewind</application>. After doing the rewind, some of
+ <application>pg_rewind</application>. After performing the rewind, some of
those files may have been copied from the source, in which case it may
- be necessary to remove the data copied and restore back the set of links
- used before the rewind.
+ be necessary to remove them and restore the files removed beforehand.
</para>
</warning>
</refsect1>
diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index e3b73a4..cc2e533 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -474,10 +474,9 @@ pgbench <optional> <replaceable>options</replaceable> </optional> <replaceable>d
</listitem>
</itemizedlist>
- Because in "prepared" mode <application>pgbench</application> reuses
- the parse analysis result for the second and subsequent query
- iteration, <application>pgbench</application> runs faster in the
- prepared mode than in other modes.
+ <application>pgbench</application> runs faster in prepared mode because
+ parse analysis and planning are done only once rather than during each
+ query.
</para>
<para>
The default is simple query protocol. (See <xref linkend="protocol"/>
diff --git a/doc/src/sgml/ref/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml
index f9b0fb8..ec2503d 100644
--- a/doc/src/sgml/ref/vacuum.sgml
+++ b/doc/src/sgml/ref/vacuum.sgml
@@ -215,9 +215,9 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet
the operating system. This is normally the desired behavior
and is the default unless the <literal>vacuum_truncate</literal>
option has been set to false for the table to be vacuumed.
- Setting this option to false may be useful to avoid
- <literal>ACCESS EXCLUSIVE</literal> lock on the table that
- the truncation requires. This option is ignored if the
+ Setting this option to false may be useful to avoid an
+ <literal>ACCESS EXCLUSIVE</literal> lock on the table as needed for
+ truncation. This option is ignored if the
<literal>FULL</literal> option is used.
</para>
</listitem>
diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml
index 365ec75..cefff77 100644
--- a/doc/src/sgml/runtime.sgml
+++ b/doc/src/sgml/runtime.sgml
@@ -2632,8 +2632,8 @@ openssl x509 -req -in server.csr -text -days 365 \
using <acronym>GSSAPI</acronym> to encrypt client/server communications for
increased security. Support requires that a <acronym>GSSAPI</acronym>
implementation (such as MIT krb5) is installed on both client and server
- systems, and that support in <productname>PostgreSQL</productname> is
- enabled at build time (see <xref linkend="installation"/>).
+ systems, and that <productname>PostgreSQL</productname> was built with
+ <acronym>GSSAPI</acronym> support (see <xref linkend="installation"/>).
</para>
<sect2 id="gssapi-setup">
diff --git a/doc/src/sgml/sources.sgml b/doc/src/sgml/sources.sgml
index a339ebb..e869a31 100644
--- a/doc/src/sgml/sources.sgml
+++ b/doc/src/sgml/sources.sgml
@@ -103,7 +103,7 @@ less -x4
message text. In addition there are optional elements, the most
common of which is an error identifier code that follows the SQL spec's
SQLSTATE conventions.
- <function>ereport</function> itself is just a shell function, that exists
+ <function>ereport</function> itself is just a shell function that exists
mainly for the syntactic convenience of making message generation
look like a function call in the C source code. The only parameter
accepted directly by <function>ereport</function> is the severity level.
@@ -886,10 +886,10 @@ BETTER: unrecognized node type: 42
<para>
For example <literal>_StaticAssert()</literal> and
<literal>__builtin_constant_p</literal> are currently used, even though
- they are from newer revisions of the C standard and a
- <productname>GCC</productname> extension respectively. If not available
- we respectively fall back to using a C99 compatible replacement that
- performs the same checks, but emits rather cryptic messages and do not
+ they are from a newer revision of the C standard and a
+ <productname>GCC</productname> extension, respectively. If not available, in the first case,
+ we fall back to using a C99 compatible replacement that
+ performs the same checks, but emits rather cryptic messages; in the second case, we do not
use <literal>__builtin_constant_p</literal>.
</para>
</simplesect>
@@ -948,7 +948,7 @@ MemoryContextSwitchTo(MemoryContext context)
Barring special arrangements code in signal handlers may only
call async-signal safe functions (as defined in POSIX) and access
variables of type <literal>volatile sig_atomic_t</literal>. A few
- functions in <command>postgres</command> are also deemed signal safe, importantly
+ functions in <command>postgres</command> are also deemed signal safe; specifically,
<function>SetLatch()</function>.
</para>
<para>
@@ -969,9 +969,9 @@ handle_sighup(SIGNAL_ARGS)
}
</programlisting>
<varname>errno</varname> is saved and restored because
- <function>SetLatch()</function> might change it. If that were not done
- interrupted code that's currently inspecting <varname>errno</varname> might see the wrong
- value.
+ <function>SetLatch()</function> might change it. If that were not done,
+ code interrupted by a signal might see the wrong value of
+ <varname>errno</varname>.
</para>
</simplesect>
diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
index 34a1938..d14457b 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -3364,11 +3364,11 @@ if (!ptr)
</indexterm>
<para>
- By default, a function is just a <quote>black box</quote> that the
- database system knows very little about the behavior of. However,
- that means that queries using the function may be executed much less
+ By default, a function is a <quote>black box</quote> that the
+ database system knows very little about, which
+ means queries calling the function may be executed less
efficiently than they could be. It is possible to supply additional
- knowledge that helps the planner optimize function calls.
+ information that helps the planner optimize function calls.
</para>
<para>
@@ -3381,7 +3381,7 @@ if (!ptr)
The parallel safety property (<literal>PARALLEL
UNSAFE</literal>, <literal>PARALLEL RESTRICTED</literal>, or
<literal>PARALLEL SAFE</literal>) must also be specified if you hope
- to use the function in parallelized queries.
+ queries calling the function to be run using the parallel infrastructure.
It can also be useful to specify the function's estimated execution
cost, and/or the number of rows a set-returning function is estimated
to return. However, the declarative way of specifying those two
@@ -3393,7 +3393,7 @@ if (!ptr)
It is also possible to attach a <firstterm>planner support
function</firstterm> to a SQL-callable function (called
its <firstterm>target function</firstterm>), and thereby provide
- knowledge about the target function that is too complex to be
+ information about the target function that is too complex to be
represented declaratively. Planner support functions have to be
written in C (although their target functions might not be), so this is
an advanced feature that relatively few people will use.
diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index 617270f..370c7aa 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -222,13 +222,13 @@ output_completion_banner(char *analyze_script_file_name,
/* Did we copy the free space files? */
if (GET_MAJOR_VERSION(old_cluster.major_version) >= 804)
pg_log(PG_REPORT,
- "Optimizer statistics are not transferred by pg_upgrade so,\n"
- "once you start the new server, consider running:\n"
+ "Optimizer statistics are not transferred by pg_upgrade.\n"
+ "Once you start the new server, consider running:\n"
" %s\n\n", analyze_script_file_name);
else
pg_log(PG_REPORT,
"Optimizer statistics and free space information are not transferred\n"
- "by pg_upgrade so, once you start the new server, consider running:\n"
+ "by pg_upgrade. Once you start the new server, consider running:\n"
" %s\n\n", analyze_script_file_name);
--
2.7.4
--FkmkrVfFsRoUs1wW
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v5-0002-Say-it-more-naturally.patch"
^ permalink raw reply [nested|flat] 5+ messages in thread
* [PATCH v4 3/7] Row pattern recognition patch (planner).
@ 2023-08-09 07:56 Tatsuo Ishii <ishii@postgresql.org>
0 siblings, 0 replies; 5+ messages in thread
From: Tatsuo Ishii @ 2023-08-09 07:56 UTC (permalink / raw)
---
src/backend/optimizer/plan/createplan.c | 23 ++++++++++++++++++-----
src/backend/optimizer/plan/setrefs.c | 23 +++++++++++++++++++++++
src/include/nodes/plannodes.h | 15 +++++++++++++++
3 files changed, 56 insertions(+), 5 deletions(-)
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index af48109058..828d8d5b62 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -286,9 +286,10 @@ static WindowAgg *make_windowagg(List *tlist, Index winref,
int ordNumCols, AttrNumber *ordColIdx, Oid *ordOperators, Oid *ordCollations,
int frameOptions, Node *startOffset, Node *endOffset,
Oid startInRangeFunc, Oid endInRangeFunc,
- Oid inRangeColl, bool inRangeAsc, bool inRangeNullsFirst,
- List *runCondition, List *qual, bool topWindow,
- Plan *lefttree);
+ Oid inRangeColl, bool inRangeAsc, bool inRangeNullsFirst, List *runCondition,
+ RPSkipTo rpSkipTo, List *patternVariable, List *patternRegexp, List *defineClause,
+ List *defineInitial,
+ List *qual, bool topWindow, Plan *lefttree);
static Group *make_group(List *tlist, List *qual, int numGroupCols,
AttrNumber *grpColIdx, Oid *grpOperators, Oid *grpCollations,
Plan *lefttree);
@@ -2679,6 +2680,11 @@ create_windowagg_plan(PlannerInfo *root, WindowAggPath *best_path)
wc->inRangeAsc,
wc->inRangeNullsFirst,
wc->runCondition,
+ wc->rpSkipTo,
+ wc->patternVariable,
+ wc->patternRegexp,
+ wc->defineClause,
+ wc->defineInitial,
best_path->qual,
best_path->topwindow,
subplan);
@@ -6582,8 +6588,10 @@ make_windowagg(List *tlist, Index winref,
int ordNumCols, AttrNumber *ordColIdx, Oid *ordOperators, Oid *ordCollations,
int frameOptions, Node *startOffset, Node *endOffset,
Oid startInRangeFunc, Oid endInRangeFunc,
- Oid inRangeColl, bool inRangeAsc, bool inRangeNullsFirst,
- List *runCondition, List *qual, bool topWindow, Plan *lefttree)
+ Oid inRangeColl, bool inRangeAsc, bool inRangeNullsFirst, List *runCondition,
+ RPSkipTo rpSkipTo, List *patternVariable, List *patternRegexp, List *defineClause,
+ List *defineInitial,
+ List *qual, bool topWindow, Plan *lefttree)
{
WindowAgg *node = makeNode(WindowAgg);
Plan *plan = &node->plan;
@@ -6609,6 +6617,11 @@ make_windowagg(List *tlist, Index winref,
node->inRangeAsc = inRangeAsc;
node->inRangeNullsFirst = inRangeNullsFirst;
node->topWindow = topWindow;
+ node->rpSkipTo = rpSkipTo,
+ node->patternVariable = patternVariable;
+ node->patternRegexp = patternRegexp;
+ node->defineClause = defineClause;
+ node->defineInitial = defineInitial;
plan->targetlist = tlist;
plan->lefttree = lefttree;
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index 97fa561e4e..2ed00b5d41 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -2456,6 +2456,29 @@ set_upper_references(PlannerInfo *root, Plan *plan, int rtoffset)
NRM_EQUAL,
NUM_EXEC_QUAL(plan));
+ /*
+ * Modifies an expression tree in each DEFINE clause so that all Var
+ * nodes reference outputs of a subplan.
+ */
+ if (IsA(plan, WindowAgg))
+ {
+ WindowAgg *wplan = (WindowAgg *) plan;
+
+ foreach(l, wplan->defineClause)
+ {
+ TargetEntry *tle = (TargetEntry *) lfirst(l);
+
+ tle->expr = (Expr *)
+ fix_upper_expr(root,
+ (Node *) tle->expr,
+ subplan_itlist,
+ OUTER_VAR,
+ rtoffset,
+ NRM_EQUAL,
+ NUM_EXEC_QUAL(plan));
+ }
+ }
+
pfree(subplan_itlist);
}
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index 1b787fe031..19815a98bb 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -1096,6 +1096,21 @@ typedef struct WindowAgg
/* nulls sort first for in_range tests? */
bool inRangeNullsFirst;
+ /* Row Pattern Recognition AFTER MACH SKIP clause */
+ RPSkipTo rpSkipTo; /* Row Pattern Skip To type */
+
+ /* Row Pattern PATTERN variable name (list of String) */
+ List *patternVariable;
+
+ /* Row Pattern RPATTERN regular expression quantifier ('+' or ''. list of String) */
+ List *patternRegexp;
+
+ /* Row Pattern DEFINE clause (list of TargetEntry) */
+ List *defineClause;
+
+ /* Row Pattern DEFINE variable initial names (list of String) */
+ List *defineInitial;
+
/*
* false for all apart from the WindowAgg that's closest to the root of
* the plan
--
2.25.1
----Next_Part(Wed_Aug__9_17_41_12_2023_134)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v4-0004-Row-pattern-recognition-patch-executor.patch"
^ permalink raw reply [nested|flat] 5+ messages in thread
* [PATCH] use C99 named designators
@ 2023-08-23 10:19 Alvaro Herrera <alvherre@alvh.no-ip.org>
0 siblings, 0 replies; 5+ messages in thread
From: Alvaro Herrera @ 2023-08-23 10:19 UTC (permalink / raw)
---
src/backend/access/heap/heapam.c | 29 +++++++++++++++--------------
src/backend/postmaster/bgworker.c | 18 ++++++++++++------
2 files changed, 27 insertions(+), 20 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index f30a56ecf55..75a77052cd4 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -127,29 +127,30 @@ static const struct
LOCKMODE hwlock;
int lockstatus;
int updstatus;
-}
+} tupleLockExtraInfo[] =
- tupleLockExtraInfo[MaxLockTupleMode + 1] =
{
{ /* LockTupleKeyShare */
- AccessShareLock,
- MultiXactStatusForKeyShare,
- -1 /* KeyShare does not allow updating tuples */
+ .hwlock = AccessShareLock,
+ .lockstatus = MultiXactStatusForKeyShare,
+ /* KeyShare does not allow updating tuples */
+ .updstatus = -1
},
{ /* LockTupleShare */
- RowShareLock,
- MultiXactStatusForShare,
- -1 /* Share does not allow updating tuples */
+ .hwlock = RowShareLock,
+ .lockstatus = MultiXactStatusForShare,
+ /* Share does not allow updating tuples */
+ .updstatus = -1
},
{ /* LockTupleNoKeyExclusive */
- ExclusiveLock,
- MultiXactStatusForNoKeyUpdate,
- MultiXactStatusNoKeyUpdate
+ .hwlock = ExclusiveLock,
+ .lockstatus = MultiXactStatusForNoKeyUpdate,
+ .updstatus = MultiXactStatusNoKeyUpdate
},
{ /* LockTupleExclusive */
- AccessExclusiveLock,
- MultiXactStatusForUpdate,
- MultiXactStatusUpdate
+ .hwlock = AccessExclusiveLock,
+ .lockstatus = MultiXactStatusForUpdate,
+ .updstatus = MultiXactStatusUpdate
}
};
diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c
index 65deabe91a7..51874481751 100644
--- a/src/backend/postmaster/bgworker.c
+++ b/src/backend/postmaster/bgworker.c
@@ -120,22 +120,28 @@ static const struct
{
{
- "ParallelWorkerMain", ParallelWorkerMain
+ .fn_name = "ParallelWorkerMain",
+ .fn_addr = ParallelWorkerMain
},
{
- "ApplyLauncherMain", ApplyLauncherMain
+ .fn_name = "ApplyLauncherMain",
+ .fn_addr = ApplyLauncherMain
},
{
- "ApplyWorkerMain", ApplyWorkerMain
+ .fn_name = "ApplyWorkerMain",
+ .fn_addr = ApplyWorkerMain
},
{
- "ParallelApplyWorkerMain", ParallelApplyWorkerMain
+ .fn_name = "ParallelApplyWorkerMain",
+ .fn_addr = ParallelApplyWorkerMain
},
{
- "TableSyncWorkerMain", TableSyncWorkerMain
+ .fn_name = "TableSyncWorkerMain",
+ .fn_addr = TableSyncWorkerMain
},
{
- "SequenceSyncWorkerMain", SequenceSyncWorkerMain
+ .fn_name = "SequenceSyncWorkerMain",
+ .fn_addr = SequenceSyncWorkerMain
}
};
--
2.47.3
--hedw4mraldmc2ejv--
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Infinite Autovacuum loop caused by failing virtual generated column expression
@ 2026-05-03 18:04 SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com>
2026-05-12 10:30 ` Re: Infinite Autovacuum loop caused by failing virtual generated column expression Yugo Nagata <nagata@sraoss.co.jp>
0 siblings, 1 reply; 5+ messages in thread
From: SATYANARAYANA NARLAPURAM @ 2026-05-03 18:04 UTC (permalink / raw)
To: Yugo Nagata <nagata@sraoss.co.jp>; +Cc: Dean Rasheed <dean.a.rasheed@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Hi,
On Tue, Apr 28, 2026 at 2:14 AM Yugo Nagata <nagata@sraoss.co.jp> wrote:
> On Tue, 14 Apr 2026 00:16:42 -0700
> SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com> wrote:
>
> > Hi
> >
> > On Mon, Apr 13, 2026 at 11:24 PM Yugo Nagata <nagata@sraoss.co.jp>
> wrote:
> >
> > > On Sat, 11 Apr 2026 17:33:13 +0100
> > > Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
> > >
> > > > On Fri, 10 Apr 2026 at 21:19, SATYANARAYANA NARLAPURAM
> > > > <satyanarlapuram@gmail.com> wrote:
> > > > >
> > > > > PG19 added support for stats on virtual generated columns [1].
> > > Creating extended statistics on a virtual generated column whose
> expression
> > > can raise an error leads to ANALYZE failing repeatedly, and autovacuum
> > > retrying indefinitely. This floods the server logs and also wastes
> > > resources. Vacuum analyze on that column (without extended stats)
> succeeds.
> > > > >
> > > >
> > > > True, though this is nothing new. The same thing can happen with
> > > > expression statistics on an expression that raises an error, which
> has
> > > > been possible since PG14.
> > >
> > > Yes, this issue is not new, and I’m not aware of a way to prevent it a
> > > priori.
> > >
> > > >
> > > > > In order to avoid retry storms, I think we have two options. (1)
> > > skipping the offending row from the sample, (2) skipping the extended
> stats
> > > computation for that table with a warning message. At least this avoid
> > > autovacuum infinite retry. Attached a draft patch for the option (2).
> > > Thoughts?
> > > > >
> > > >
> > > > I'm not sure. The default retry interval is 1 minute, so it won't
> > > > exactly be a flood of messages. Also, if the error only occurs for a
> > > > small subset of rows, it's possible that retrying might succeed.
> > >
> > > I think it would be good to skip ANALYZE for the extended statistics
> that
> > > cause
> > > errors and just emit a warning, rather than aborting ANALYZE for the
> > > entire table.
> > > It seems reasonable to treat this as the user’s responsibility to
> notice
> > > the warning
> > > and address the underlying issue.
> > >
> >
> > Yugo, thanks for the comments. Could you please review the v1 patch when
> you
> > get a chance. It is in the direction you suggested.
>
> I've looked into the patch and have some comments.
>
> The child ResourceOwner is created and released in
> BuildRelationExtStatistics(),
> but I don't think it is necessary if we add other PG_TRY block in
> make_build_data()
> and compute_expr_stats(). For example in make_build_data():
>
> + PG_TRY();
> + {
> + datum = ExecEvalExpr(exprstate,
> +
> GetPerTupleExprContext(estate),
> +
> &isnull);
> + }
> + PG_CATCH();
> + {
> + ExecDropSingleTupleTableSlot(slot);
> + FreeExecutorState(estate);
> + PG_RE_THROW();
> + }
> + PG_END_TRY();
>
Thanks, for reviewing the patch. Agreed, please find the updated patch.
> Also, we could add tests for extended statistics that do not involve
> virtual generated
> columns, since those are not the cause root of the issue. In addition, it
> might be useful
> to verify that non-skipped extended statistics are still computed
> successfully.
> For example:
>
> +CREATE TABLE expr_err (a int);
> +INSERT INTO expr_err VALUES (1), (2), (3);
> +CREATE STATISTICS expr_err_s1 ON ((a/0)) FROM expr_err;
> +CREATE STATISTICS expr_err_s2 ON (a/0),(a+1) FROM expr_err;
> +CREATE STATISTICS expr_err_s3 ON ((a+1)) FROM expr_err;
> +ANALYZE expr_err; -- should warn, not fail
> +SELECT statistics_name from pg_stats_ext x
> + WHERE tablename = 'expr_err' ORDER BY ROW(x.*);
>
Added these tests as well in the v2 patch.
Thanks,
Satya
Attachments:
[application/octet-stream] v2-0001-Fix-ANALYZE-crash-on-extended-stats-with-virtual-gen.patch (13.5K, ../../CAHg+QDdFeuYKuX3=_-5-VvyaCkS9zgzOEjeC77dw7Wtrx_TqAA@mail.gmail.com/3-v2-0001-Fix-ANALYZE-crash-on-extended-stats-with-virtual-gen.patch)
download | inline diff:
From 6a2705d2084abd9b6b304c460cf17a4c96c1fdad Mon Sep 17 00:00:00 2001
From: Satya Narlapuram <satyanarlapuram@gmail.com>
Date: Sun, 3 May 2026 17:37:24 +0000
Subject: [PATCH] Fix ANALYZE crash on extended stats with virtual generated
columns
When a table has extended statistics involving expressions on virtual
generated columns, ANALYZE can fail if evaluating those expressions
raises an error (e.g. division by zero). This happens because the
generated column expression is evaluated directly during statistics
computation.
Fix by wrapping expression evaluation in PG_TRY/PG_CATCH blocks:
- In make_build_data() and compute_expr_stats(), add PG_TRY blocks
that clean up executor resources (slot, estate) before re-throwing,
preventing resource leaks on error.
- In BuildRelationExtStatistics(), catch errors from expression
evaluation, issue a WARNING with the statistics object name and
error details, then continue processing the remaining statistics
objects.
This ensures ANALYZE completes successfully even when some extended
statistics objects cannot be computed due to expression evaluation
errors.
---
src/backend/statistics/extended_stats.c | 238 +++++++++++++++---------
src/test/regress/expected/stats_ext.out | 29 +++
src/test/regress/sql/stats_ext.sql | 20 ++
3 files changed, 195 insertions(+), 92 deletions(-)
diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c
index 2b83355d..88cea81b 100644
--- a/src/backend/statistics/extended_stats.c
+++ b/src/backend/statistics/extended_stats.c
@@ -194,41 +194,73 @@ BuildRelationExtStatistics(Relation onerel, bool inh, double totalrows,
if (stattarget == 0)
continue;
- /* evaluate expressions (if the statistics object has any) */
- data = make_build_data(onerel, stat, numrows, rows, stats, stattarget);
-
- /* compute statistic of each requested type */
- foreach(lc2, stat->types)
+ /*
+ * Wrap expression evaluation and stats computation in PG_TRY so
+ * that errors from evaluating expressions (e.g. division by zero
+ * in virtual generated columns) don't cause ANALYZE to fail
+ * entirely. Skip the statistics object and issue a WARNING
+ * instead.
+ */
+ PG_TRY();
{
- char t = (char) lfirst_int(lc2);
-
- if (t == STATS_EXT_NDISTINCT)
- ndistinct = statext_ndistinct_build(totalrows, data);
- else if (t == STATS_EXT_DEPENDENCIES)
- dependencies = statext_dependencies_build(data);
- else if (t == STATS_EXT_MCV)
- mcv = statext_mcv_build(data, totalrows, stattarget);
- else if (t == STATS_EXT_EXPRESSIONS)
+ /* evaluate expressions (if the statistics has any) */
+ data = make_build_data(onerel, stat, numrows, rows,
+ stats, stattarget);
+
+ /* compute statistic of each requested type */
+ foreach(lc2, stat->types)
{
- AnlExprData *exprdata;
- int nexprs;
+ char t = (char) lfirst_int(lc2);
+
+ if (t == STATS_EXT_NDISTINCT)
+ ndistinct = statext_ndistinct_build(totalrows, data);
+ else if (t == STATS_EXT_DEPENDENCIES)
+ dependencies = statext_dependencies_build(data);
+ else if (t == STATS_EXT_MCV)
+ mcv = statext_mcv_build(data, totalrows, stattarget);
+ else if (t == STATS_EXT_EXPRESSIONS)
+ {
+ AnlExprData *exprdata;
+ int nexprs;
- /* should not happen, thanks to checks when defining stats */
- if (!stat->exprs)
- elog(ERROR, "requested expression stats, but there are no expressions");
+ /* should not happen, thanks to checks when defining stats */
+ if (!stat->exprs)
+ elog(ERROR, "requested expression stats, but there are no expressions");
- exprdata = build_expr_data(stat->exprs, stattarget);
- nexprs = list_length(stat->exprs);
+ exprdata = build_expr_data(stat->exprs, stattarget);
+ nexprs = list_length(stat->exprs);
- compute_expr_stats(onerel, exprdata, nexprs, rows, numrows);
+ compute_expr_stats(onerel, exprdata, nexprs,
+ rows, numrows);
- exprstats = serialize_expr_stats(exprdata, nexprs);
+ exprstats = serialize_expr_stats(exprdata, nexprs);
+ }
}
- }
- /* store the statistics in the catalog */
- statext_store(stat->statOid, inh,
- ndistinct, dependencies, mcv, exprstats, stats);
+ /* store the statistics in the catalog */
+ statext_store(stat->statOid, inh,
+ ndistinct, dependencies, mcv, exprstats, stats);
+ }
+ PG_CATCH();
+ {
+ ErrorData *edata;
+
+ /* Save the error, issue a WARNING and continue */
+ MemoryContextSwitchTo(cxt);
+ edata = CopyErrorData();
+ FlushErrorState();
+
+ ereport(WARNING,
+ (errcode(ERRCODE_WARNING),
+ errmsg("skipping statistics object \"%s.%s\" for relation \"%s.%s\"",
+ stat->schema, stat->name,
+ get_namespace_name(onerel->rd_rel->relnamespace),
+ RelationGetRelationName(onerel)),
+ errdetail("%s", edata->message)));
+
+ FreeErrorData(edata);
+ }
+ PG_END_TRY();
/* for reporting progress */
pgstat_progress_update_param(PROGRESS_ANALYZE_EXT_STATS_COMPUTED,
@@ -2185,46 +2217,58 @@ compute_expr_stats(Relation onerel, AnlExprData *exprdata, int nexprs,
exprnulls = (bool *) palloc(numrows * sizeof(bool));
tcnt = 0;
- for (i = 0; i < numrows; i++)
+ PG_TRY();
{
- Datum datum;
- bool isnull;
+ for (i = 0; i < numrows; i++)
+ {
+ Datum datum;
+ bool isnull;
- /*
- * Reset the per-tuple context each time, to reclaim any cruft
- * left behind by evaluating the statistics expressions.
- */
- ResetExprContext(econtext);
+ /*
+ * Reset the per-tuple context each time, to reclaim any
+ * cruft left behind by evaluating the statistics
+ * expressions.
+ */
+ ResetExprContext(econtext);
- /* Set up for expression evaluation */
- ExecStoreHeapTuple(rows[i], slot, false);
+ /* Set up for expression evaluation */
+ ExecStoreHeapTuple(rows[i], slot, false);
- /*
- * Evaluate the expression. We do this in the per-tuple context so
- * as not to leak memory, and then copy the result into the
- * context created at the beginning of this function.
- */
- datum = ExecEvalExprSwitchContext(exprstate,
- GetPerTupleExprContext(estate),
- &isnull);
- if (isnull)
- {
- exprvals[tcnt] = (Datum) 0;
- exprnulls[tcnt] = true;
- }
- else
- {
- /* Make sure we copy the data into the context. */
- Assert(CurrentMemoryContext == expr_context);
+ /*
+ * Evaluate the expression. We do this in the per-tuple
+ * context so as not to leak memory, and then copy the
+ * result into the context created at the beginning of
+ * this function.
+ */
+ datum = ExecEvalExprSwitchContext(exprstate,
+ GetPerTupleExprContext(estate),
+ &isnull);
+ if (isnull)
+ {
+ exprvals[tcnt] = (Datum) 0;
+ exprnulls[tcnt] = true;
+ }
+ else
+ {
+ /* Make sure we copy the data into the context. */
+ Assert(CurrentMemoryContext == expr_context);
- exprvals[tcnt] = datumCopy(datum,
- stats->attrtype->typbyval,
- stats->attrtype->typlen);
- exprnulls[tcnt] = false;
- }
+ exprvals[tcnt] = datumCopy(datum,
+ stats->attrtype->typbyval,
+ stats->attrtype->typlen);
+ exprnulls[tcnt] = false;
+ }
- tcnt++;
+ tcnt++;
+ }
+ }
+ PG_CATCH();
+ {
+ ExecDropSingleTupleTableSlot(slot);
+ FreeExecutorState(estate);
+ PG_RE_THROW();
}
+ PG_END_TRY();
/*
* Now we can compute the statistics for the expression columns.
@@ -2624,46 +2668,56 @@ make_build_data(Relation rel, StatExtEntry *stat, int numrows, HeapTuple *rows,
/* Set up expression evaluation state */
exprstates = ExecPrepareExprList(stat->exprs, estate);
- for (i = 0; i < numrows; i++)
+ PG_TRY();
{
- /*
- * Reset the per-tuple context each time, to reclaim any cruft left
- * behind by evaluating the statistics object expressions.
- */
- ResetExprContext(econtext);
-
- /* Set up for expression evaluation */
- ExecStoreHeapTuple(rows[i], slot, false);
-
- idx = bms_num_members(stat->columns);
- foreach(lc, exprstates)
+ for (i = 0; i < numrows; i++)
{
- Datum datum;
- bool isnull;
- ExprState *exprstate = (ExprState *) lfirst(lc);
-
/*
- * XXX This probably leaks memory. Maybe we should use
- * ExecEvalExprSwitchContext but then we need to copy the result
- * somewhere else.
+ * Reset the per-tuple context each time, to reclaim any cruft
+ * left behind by evaluating the statistics object expressions.
*/
- datum = ExecEvalExpr(exprstate,
- GetPerTupleExprContext(estate),
- &isnull);
- if (isnull)
- {
- result->values[idx][i] = (Datum) 0;
- result->nulls[idx][i] = true;
- }
- else
+ ResetExprContext(econtext);
+
+ /* Set up for expression evaluation */
+ ExecStoreHeapTuple(rows[i], slot, false);
+
+ idx = bms_num_members(stat->columns);
+ foreach(lc, exprstates)
{
- result->values[idx][i] = datum;
- result->nulls[idx][i] = false;
- }
+ Datum datum;
+ bool isnull;
+ ExprState *exprstate = (ExprState *) lfirst(lc);
- idx++;
+ /*
+ * XXX This probably leaks memory. Maybe we should use
+ * ExecEvalExprSwitchContext but then we need to copy the
+ * result somewhere else.
+ */
+ datum = ExecEvalExpr(exprstate,
+ GetPerTupleExprContext(estate),
+ &isnull);
+ if (isnull)
+ {
+ result->values[idx][i] = (Datum) 0;
+ result->nulls[idx][i] = true;
+ }
+ else
+ {
+ result->values[idx][i] = datum;
+ result->nulls[idx][i] = false;
+ }
+
+ idx++;
+ }
}
}
+ PG_CATCH();
+ {
+ ExecDropSingleTupleTableSlot(slot);
+ FreeExecutorState(estate);
+ PG_RE_THROW();
+ }
+ PG_END_TRY();
ExecDropSingleTupleTableSlot(slot);
FreeExecutorState(estate);
diff --git a/src/test/regress/expected/stats_ext.out b/src/test/regress/expected/stats_ext.out
index 37070c1a..43892f4f 100644
--- a/src/test/regress/expected/stats_ext.out
+++ b/src/test/regress/expected/stats_ext.out
@@ -3207,6 +3207,14 @@ SELECT * FROM check_estimated_rows('SELECT * FROM virtual_gen_stats WHERE w = 0'
(1 row)
DROP TABLE virtual_gen_stats;
+-- extended statistics on virtual generated columns whose expressions can error
+CREATE TABLE virtual_gen_err (a int, b int GENERATED ALWAYS AS (a / 0) VIRTUAL);
+INSERT INTO virtual_gen_err VALUES (1), (2), (3);
+CREATE STATISTICS virtual_gen_err_s ON a, b FROM virtual_gen_err;
+ANALYZE virtual_gen_err; -- should warn, not fail
+WARNING: skipping statistics object "public.virtual_gen_err_s" for relation "public.virtual_gen_err"
+DETAIL: division by zero
+DROP TABLE virtual_gen_err;
-- Permission tests. Users should not be able to see specific data values in
-- the extended statistics, if they lack permission to see those values in
-- the underlying table.
@@ -3709,3 +3717,24 @@ SELECT range_length_histogram, range_empty_frac, range_bounds_histogram
(1 row)
DROP TABLE stats_ext_tbl_range;
+-- Expression statistics with errors (not involving virtual generated columns).
+-- Errors during expression evaluation should produce a WARNING, not a failure,
+-- and non-erroring statistics objects should still be computed.
+CREATE TABLE expr_err (a int);
+INSERT INTO expr_err VALUES (1), (2), (3);
+CREATE STATISTICS expr_err_s1 ON ((a/0)) FROM expr_err;
+CREATE STATISTICS expr_err_s2 ON (a/0),(a+1) FROM expr_err;
+CREATE STATISTICS expr_err_s3 ON ((a+1)) FROM expr_err;
+ANALYZE expr_err; -- should warn, not fail
+WARNING: skipping statistics object "public.expr_err_s1" for relation "public.expr_err"
+DETAIL: division by zero
+WARNING: skipping statistics object "public.expr_err_s2" for relation "public.expr_err"
+DETAIL: division by zero
+SELECT statistics_name from pg_stats_ext x
+ WHERE tablename = 'expr_err' ORDER BY ROW(x.*);
+ statistics_name
+-----------------
+ expr_err_s3
+(1 row)
+
+DROP TABLE expr_err;
diff --git a/src/test/regress/sql/stats_ext.sql b/src/test/regress/sql/stats_ext.sql
index 3cc6012b..f78fd902 100644
--- a/src/test/regress/sql/stats_ext.sql
+++ b/src/test/regress/sql/stats_ext.sql
@@ -1608,6 +1608,13 @@ SELECT * FROM check_estimated_rows('SELECT * FROM virtual_gen_stats WHERE w = 0'
DROP TABLE virtual_gen_stats;
+-- extended statistics on virtual generated columns whose expressions can error
+CREATE TABLE virtual_gen_err (a int, b int GENERATED ALWAYS AS (a / 0) VIRTUAL);
+INSERT INTO virtual_gen_err VALUES (1), (2), (3);
+CREATE STATISTICS virtual_gen_err_s ON a, b FROM virtual_gen_err;
+ANALYZE virtual_gen_err; -- should warn, not fail
+DROP TABLE virtual_gen_err;
+
-- Permission tests. Users should not be able to see specific data values in
-- the extended statistics, if they lack permission to see those values in
-- the underlying table.
@@ -1908,3 +1915,16 @@ SELECT range_length_histogram, range_empty_frac, range_bounds_histogram
FROM pg_stats_ext_exprs
WHERE statistics_name = 'stats_ext_range';
DROP TABLE stats_ext_tbl_range;
+
+-- Expression statistics with errors (not involving virtual generated columns).
+-- Errors during expression evaluation should produce a WARNING, not a failure,
+-- and non-erroring statistics objects should still be computed.
+CREATE TABLE expr_err (a int);
+INSERT INTO expr_err VALUES (1), (2), (3);
+CREATE STATISTICS expr_err_s1 ON ((a/0)) FROM expr_err;
+CREATE STATISTICS expr_err_s2 ON (a/0),(a+1) FROM expr_err;
+CREATE STATISTICS expr_err_s3 ON ((a+1)) FROM expr_err;
+ANALYZE expr_err; -- should warn, not fail
+SELECT statistics_name from pg_stats_ext x
+ WHERE tablename = 'expr_err' ORDER BY ROW(x.*);
+DROP TABLE expr_err;
--
2.43.0
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Infinite Autovacuum loop caused by failing virtual generated column expression
2026-05-03 18:04 Re: Infinite Autovacuum loop caused by failing virtual generated column expression SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com>
@ 2026-05-12 10:30 ` Yugo Nagata <nagata@sraoss.co.jp>
0 siblings, 0 replies; 5+ messages in thread
From: Yugo Nagata @ 2026-05-12 10:30 UTC (permalink / raw)
To: SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com>; +Cc: Dean Rasheed <dean.a.rasheed@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Sun, 3 May 2026 11:04:59 -0700
SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com> wrote:
Thank you for updating the patch!
There are a few comments on v2 patch.
In compute_expr_stats(),
+ PG_CATCH();
+ {
+ ExecDropSingleTupleTableSlot(slot);
+ FreeExecutorState(estate);
+ PG_RE_THROW();
}
Should we switch the context to expr_context before releasing
slot and estate? Also, should we call MemoryContextDelete(expr_context)
to release palloc'ed memory in the loop?
(In fact, the error would not be cached in compute_expr_stats(), though,
since it would be cached in make_build_data() if any.)
Should we switch to expr_context before releasing slot and estate?
Also, should we call MemoryContextDelete(expr_context) to release
memory allocated by palloc() in the loop?
(In practice, errors would not catched in compute_expr_stats() itself,
since they would already be caught in make_build_data() if any.)
+ /*
+ * Wrap expression evaluation and stats computation in PG_TRY so
+ * that errors from evaluating expressions (e.g. division by zero
+ * in virtual generated columns) don't cause ANALYZE to fail
+ * entirely. Skip the statistics object and issue a WARNING
+ * instead.
How about rewriting the comments to reflect the more general case?
Extended statistics involving virtual generated columns are a somewhat
special case, while errors in expression statistics seem more common
in practice. The same applies to the commit message as well.
Regards,
Yugo Nagata
--
Yugo Nagata <nagata@sraoss.co.jp>
^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2026-05-12 10:30 UTC | newest]
Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-03-28 23:50 [PATCH v5 01/12] review docs for pg12dev Justin Pryzby <pryzbyj@telsasoft.com>
2023-08-09 07:56 [PATCH v4 3/7] Row pattern recognition patch (planner). Tatsuo Ishii <ishii@postgresql.org>
2023-08-23 10:19 [PATCH] use C99 named designators Alvaro Herrera <alvherre@alvh.no-ip.org>
2026-05-03 18:04 Re: Infinite Autovacuum loop caused by failing virtual generated column expression SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com>
2026-05-12 10:30 ` Re: Infinite Autovacuum loop caused by failing virtual generated column expression Yugo Nagata <nagata@sraoss.co.jp>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox