public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v56 4/6] Doc part of shared-memory based stats collector.
8+ messages / 5 participants
[nested] [flat]
* [PATCH v56 4/6] Doc part of shared-memory based stats collector.
@ 2020-03-19 06:11 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 8+ messages in thread
From: Kyotaro Horiguchi @ 2020-03-19 06:11 UTC (permalink / raw)
---
doc/src/sgml/catalogs.sgml | 6 +-
doc/src/sgml/config.sgml | 25 +++---
doc/src/sgml/high-availability.sgml | 13 +--
doc/src/sgml/monitoring.sgml | 127 +++++++++++++---------------
doc/src/sgml/ref/pg_dump.sgml | 9 +-
src/backend/postmaster/postmaster.c | 2 -
6 files changed, 85 insertions(+), 97 deletions(-)
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index b1de6d0674..0ef684d4d0 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -9261,9 +9261,9 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l
<para>
<xref linkend="view-table"/> lists the system views described here.
More detailed documentation of each view follows below.
- There are some additional views that provide access to the results of
- the statistics collector; they are described in <xref
- linkend="monitoring-stats-views-table"/>.
+ There are some additional views that provide access to the activity
+ statistics; they are described in
+ <xref linkend="monitoring-stats-views-table"/>.
</para>
<para>
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index a218d78bef..53d7dfda93 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -7375,11 +7375,11 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
<title>Run-time Statistics</title>
<sect2 id="runtime-config-statistics-collector">
- <title>Query and Index Statistics Collector</title>
+ <title>Query and Index Activity Statistics</title>
<para>
- These parameters control server-wide statistics collection features.
- When statistics collection is enabled, the data that is produced can be
+ These parameters control server-wide activity statistics features.
+ When activity statistics is enabled, the data that is produced can be
accessed via the <structname>pg_stat</structname> and
<structname>pg_statio</structname> family of system views.
Refer to <xref linkend="monitoring"/> for more information.
@@ -7395,14 +7395,13 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
</term>
<listitem>
<para>
- Enables the collection of information on the currently
- executing command of each session, along with the time when
- that command began execution. This parameter is on by
- default. Note that even when enabled, this information is not
- visible to all users, only to superusers and the user owning
- the session being reported on, so it should not represent a
- security risk.
- Only superusers can change this setting.
+ Enables activity tracking on the currently executing command of
+ each session, along with the time when that command began
+ execution. This parameter is on by default. Note that even when
+ enabled, this information is not visible to all users, only to
+ superusers and the user owning the session being reported on, so it
+ should not represent a security risk. Only superusers can change this
+ setting.
</para>
</listitem>
</varlistentry>
@@ -7433,9 +7432,9 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
</term>
<listitem>
<para>
- Enables collection of statistics on database activity.
+ Enables tracking of database activity.
This parameter is on by default, because the autovacuum
- daemon needs the collected information.
+ daemon needs the activity information.
Only superusers can change this setting.
</para>
</listitem>
diff --git a/doc/src/sgml/high-availability.sgml b/doc/src/sgml/high-availability.sgml
index f49f5c0108..45095857eb 100644
--- a/doc/src/sgml/high-availability.sgml
+++ b/doc/src/sgml/high-availability.sgml
@@ -2217,12 +2217,13 @@ HINT: You can then restart the server after making the necessary configuration
</para>
<para>
- The statistics collector is active during recovery. All scans, reads, blocks,
- index usage, etc., will be recorded normally on the standby. Replayed
- actions will not duplicate their effects on primary, so replaying an
- insert will not increment the Inserts column of pg_stat_user_tables.
- The stats file is deleted at the start of recovery, so stats from primary
- and standby will differ; this is considered a feature, not a bug.
+ The activity statistics is collected during recovery. All scans, reads,
+ blocks, index usage, etc., will be recorded normally on the
+ standby. Replayed actions will not duplicate their effects on primary, so
+ replaying an insert will not increment the Inserts column of
+ pg_stat_user_tables. The activity statistics is reset at the start of
+ recovery, so stats from primary and standby will differ; this is
+ considered a feature, not a bug.
</para>
<para>
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index db4b4e460c..90cf3072ac 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -22,7 +22,7 @@
<para>
Several tools are available for monitoring database activity and
analyzing performance. Most of this chapter is devoted to describing
- <productname>PostgreSQL</productname>'s statistics collector,
+ <productname>PostgreSQL</productname>'s activity statistics,
but one should not neglect regular Unix monitoring programs such as
<command>ps</command>, <command>top</command>, <command>iostat</command>, and <command>vmstat</command>.
Also, once one has identified a
@@ -53,7 +53,6 @@ postgres 15554 0.0 0.0 57536 1184 ? Ss 18:02 0:00 postgres: back
postgres 15555 0.0 0.0 57536 916 ? Ss 18:02 0:00 postgres: checkpointer
postgres 15556 0.0 0.0 57536 916 ? Ss 18:02 0:00 postgres: walwriter
postgres 15557 0.0 0.0 58504 2244 ? Ss 18:02 0:00 postgres: autovacuum launcher
-postgres 15558 0.0 0.0 17512 1068 ? Ss 18:02 0:00 postgres: stats collector
postgres 15582 0.0 0.0 58772 3080 ? Ss 18:04 0:00 postgres: joe runbug 127.0.0.1 idle
postgres 15606 0.0 0.0 58772 3052 ? Ss 18:07 0:00 postgres: tgl regression [local] SELECT waiting
postgres 15610 0.0 0.0 58772 3056 ? Ss 18:07 0:00 postgres: tgl regression [local] idle in transaction
@@ -65,9 +64,8 @@ postgres 15610 0.0 0.0 58772 3056 ? Ss 18:07 0:00 postgres: tgl
primary server process. The command arguments
shown for it are the same ones used when it was launched. The next five
processes are background worker processes automatically launched by the
- primary process. (The <quote>stats collector</quote> process will not be present
- if you have set the system not to start the statistics collector; likewise
- the <quote>autovacuum launcher</quote> process can be disabled.)
+ primary process. (The <quote>autovacuum launcher</quote> process will not
+ be present if you have set the system not to start it.)
Each of the remaining
processes is a server process handling one client connection. Each such
process sets its command line display in the form
@@ -130,20 +128,21 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser
</sect1>
<sect1 id="monitoring-stats">
- <title>The Statistics Collector</title>
+ <title>The Activity Statistics</title>
<indexterm zone="monitoring-stats">
<primary>statistics</primary>
</indexterm>
<para>
- <productname>PostgreSQL</productname>'s <firstterm>statistics collector</firstterm>
- is a subsystem that supports collection and reporting of information about
- server activity. Presently, the collector can count accesses to tables
- and indexes in both disk-block and individual-row terms. It also tracks
- the total number of rows in each table, and information about vacuum and
- analyze actions for each table. It can also count calls to user-defined
- functions and the total time spent in each one.
+ <productname>PostgreSQL</productname>'s <firstterm>activity
+ statistics</firstterm> is a subsystem that supports tracking and reporting
+ of information about server activity. Presently, the activity statistics
+ tracks the count of accesses to tables and indexes in both disk-block and
+ individual-row terms. It also tracks the total number of rows in each
+ table, and information about vacuum and analyze actions for each table. It
+ can also track calls to user-defined functions and the total time spent in
+ each one.
</para>
<para>
@@ -151,15 +150,15 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser
information about exactly what is going on in the system right now, such as
the exact command currently being executed by other server processes, and
which other connections exist in the system. This facility is independent
- of the collector process.
+ of the activity statistics.
</para>
<sect2 id="monitoring-stats-setup">
- <title>Statistics Collection Configuration</title>
+ <title>Activity Statistics Configuration</title>
<para>
- Since collection of statistics adds some overhead to query execution,
- the system can be configured to collect or not collect information.
+ Since tracking for the activity statistics adds some overhead to query
+ execution, the system can be configured to track or not track activity.
This is controlled by configuration parameters that are normally set in
<filename>postgresql.conf</filename>. (See <xref linkend="runtime-config"/> for
details about setting configuration parameters.)
@@ -172,7 +171,7 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser
<para>
The parameter <xref linkend="guc-track-counts"/> controls whether
- statistics are collected about table and index accesses.
+ to track activity about table and index accesses.
</para>
<para>
@@ -201,18 +200,11 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser
</para>
<para>
- The statistics collector transmits the collected information to other
- <productname>PostgreSQL</productname> processes through temporary files.
- These files are stored in the directory named by the
- <xref linkend="guc-stats-temp-directory"/> parameter,
- <filename>pg_stat_tmp</filename> by default.
- For better performance, <varname>stats_temp_directory</varname> can be
- pointed at a RAM-based file system, decreasing physical I/O requirements.
- When the server shuts down cleanly, a permanent copy of the statistics
- data is stored in the <filename>pg_stat</filename> subdirectory, so that
- statistics can be retained across server restarts. When recovery is
- performed at server start (e.g., after immediate shutdown, server crash,
- and point-in-time recovery), all statistics counters are reset.
+ down cleanly, a permanent copy of the statistics data is stored in
+ the <filename>pg_stat</filename> subdirectory, so that statistics can be
+ retained across server restarts. When recovery is performed at server
+ start (e.g. after immediate shutdown, server crash, and point-in-time
+ recovery), all statistics counters are reset.
</para>
</sect2>
@@ -225,48 +217,46 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser
linkend="monitoring-stats-dynamic-views-table"/>, are available to show
the current state of the system. There are also several other
views, listed in <xref
- linkend="monitoring-stats-views-table"/>, available to show the results
- of statistics collection. Alternatively, one can
- build custom views using the underlying statistics functions, as discussed
- in <xref linkend="monitoring-stats-functions"/>.
+ linkend="monitoring-stats-views-table"/>, available to show the activity
+ statistics. Alternatively, one can build custom views using the underlying
+ statistics functions, as discussed in
+ <xref linkend="monitoring-stats-functions"/>.
</para>
<para>
- When using the statistics to monitor collected data, it is important
- to realize that the information does not update instantaneously.
- Each individual server process transmits new statistical counts to
- the collector just before going idle; so a query or transaction still in
- progress does not affect the displayed totals. Also, the collector itself
- emits a new report at most once per <varname>PGSTAT_STAT_INTERVAL</varname>
- milliseconds (500 ms unless altered while building the server). So the
- displayed information lags behind actual activity. However, current-query
- information collected by <varname>track_activities</varname> is
- always up-to-date.
+ When using the activity statistics, it is important to realize that the
+ information does not update instantaneously. Each individual server writes
+ out new statistical counts just before going idle, not frequent than once
+ per <varname>PGSTAT_STAT_INTERVAL</varname> milliseconds (1 second unless
+ altered while building the server); so a query or transaction still in
+ progress does not affect the displayed totals. However, current-query
+ information tracked by <varname>track_activities</varname> is always
+ up-to-date.
</para>
<para>
Another important point is that when a server process is asked to display
- any of these statistics, it first fetches the most recent report emitted by
- the collector process and then continues to use this snapshot for all
- statistical views and functions until the end of its current transaction.
- So the statistics will show static information as long as you continue the
- current transaction. Similarly, information about the current queries of
- all sessions is collected when any such information is first requested
- within a transaction, and the same information will be displayed throughout
- the transaction.
- This is a feature, not a bug, because it allows you to perform several
- queries on the statistics and correlate the results without worrying that
- the numbers are changing underneath you. But if you want to see new
- results with each query, be sure to do the queries outside any transaction
- block. Alternatively, you can invoke
+ any of these statistics, it first reads the current statistics and then
+ continues to use this snapshot for all statistical views and functions
+ until the end of its current transaction. So the statistics will show
+ static information as long as you continue the current transaction.
+ Similarly, information about the current queries of all sessions is tracked
+ when any such information is first requested within a transaction, and the
+ same information will be displayed throughout the transaction. This is a
+ feature, not a bug, because it allows you to perform several queries on the
+ statistics and correlate the results without worrying that the numbers are
+ changing underneath you. But if you want to see new results with each
+ query, be sure to do the queries outside any transaction block.
+ Alternatively, you can invoke
<function>pg_stat_clear_snapshot</function>(), which will discard the
current transaction's statistics snapshot (if any). The next use of
statistical information will cause a new snapshot to be fetched.
</para>
-
+
<para>
- A transaction can also see its own statistics (as yet untransmitted to the
- collector) in the views <structname>pg_stat_xact_all_tables</structname>,
+ A transaction can also see its own statistics (as yet unwritten to the
+ server-wide activity statistics) in the
+ views <structname>pg_stat_xact_all_tables</structname>,
<structname>pg_stat_xact_sys_tables</structname>,
<structname>pg_stat_xact_user_tables</structname>, and
<structname>pg_stat_xact_user_functions</structname>. These numbers do not act as
@@ -648,7 +638,7 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser
kernel's I/O cache, and might therefore still be fetched without
requiring a physical read. Users interested in obtaining more
detailed information on <productname>PostgreSQL</productname> I/O behavior are
- advised to use the <productname>PostgreSQL</productname> statistics collector
+ advised to use the <productname>PostgreSQL</productname> activity statistics
in combination with operating system utilities that allow insight
into the kernel's handling of I/O.
</para>
@@ -1086,10 +1076,6 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser
<entry><literal>LogicalLauncherMain</literal></entry>
<entry>Waiting in main loop of logical replication launcher process.</entry>
</row>
- <row>
- <entry><literal>PgStatMain</literal></entry>
- <entry>Waiting in main loop of statistics collector process.</entry>
- </row>
<row>
<entry><literal>RecoveryWalStream</literal></entry>
<entry>Waiting in main loop of startup process for WAL to arrive, during
@@ -1852,6 +1838,10 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser
</thead>
<tbody>
+ <row>
+ <entry><literal>ActivityStatistics</literal></entry>
+ <entry>Waiting to write out activity statistics to shared memory.</entry>
+ </row>
<row>
<entry><literal>AddinShmemInit</literal></entry>
<entry>Waiting to manage an extension's space allocation in shared
@@ -6128,9 +6118,10 @@ SELECT pg_stat_get_backend_pid(s.backendid) AS pid,
<entry><literal>performing final cleanup</literal></entry>
<entry>
<command>VACUUM</command> is performing final cleanup. During this phase,
- <command>VACUUM</command> will vacuum the free space map, update statistics
- in <literal>pg_class</literal>, and report statistics to the statistics
- collector. When this phase is completed, <command>VACUUM</command> will end.
+ <command>VACUUM</command> will vacuum the free space map, update
+ statistics in <literal>pg_class</literal>, and system-wide activity
+ statistics. When this phase is completed, <command>VACUUM</command>
+ will end.
</entry>
</row>
</tbody>
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index bcbb7a25fb..1fa59a2fdf 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -1280,11 +1280,10 @@ PostgreSQL documentation
</para>
<para>
- The database activity of <application>pg_dump</application> is
- normally collected by the statistics collector. If this is
- undesirable, you can set parameter <varname>track_counts</varname>
- to false via <envar>PGOPTIONS</envar> or the <literal>ALTER
- USER</literal> command.
+ The database activity of <application>pg_dump</application> is normally
+ collected. If this is undesirable, you can set
+ parameter <varname>track_counts</varname> to false
+ via <envar>PGOPTIONS</envar> or the <literal>ALTER USER</literal> command.
</para>
</refsect1>
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 5d5802f61d..b5a37b5e0c 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -6328,8 +6328,6 @@ restore_backend_variables(BackendParameters *param, Port *port)
if (postmaster_alive_fds[1] >= 0)
ReserveExternalFD();
#endif
- if (pgStatSock != PGINVALID_SOCKET)
- ReserveExternalFD();
}
--
2.27.0
----Next_Part(Tue_Mar_16_10_27_55_2021_500)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v56-0005-Remove-the-GUC-stats_temp_directory.patch"
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: BUG #18097: Immutable expression not allowed in generated at
@ 2023-09-08 15:11 David G. Johnston <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: David G. Johnston @ 2023-09-08 15:11 UTC (permalink / raw)
To: [email protected] <[email protected]>; [email protected] <[email protected]>
On Thursday, September 7, 2023, PG Bug reporting form <
[email protected]> wrote:
> The following bug has been logged on the website:
>
> Bug reference: 18097
> Logged by: Jim Keener
> Email address: [email protected]
> PostgreSQL version: 15.0
> Operating system: Linux
> Description:
>
> However, the following DOES NOT work with an error of (ERROR: generation
> expression is not immutable):
>
> * alter table test_table add created_local text GENERATED ALWAYS AS
> (EXTRACT(isoyear FROM created_at AT TIME ZONE 'America/New_York') || '|' ||
> EXTRACT(week FROM created_at AT TIME ZONE 'America/New_York')) STORED;
>
> Given that casting shouldn't "increase" the immutability of an expression,
> and expression indexes need also be immutable afaik, I think that there is
> a
> bug somewhere here?
>
Casting very much can be a non-immutable activity, dates being the prime
example, and I presume going from numeric to text is indeed defined to be
stable hence the error. This is probably due to needing to consult locale
for deciding how to represent the decimal places divider. This is one of
the few places, assuming you write the function to set an environment
fixing locale to some know value like you did with the time zones, where
creating an immutable function around a stable expression makes sense.
David J.
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: BUG #18097: Immutable expression not allowed in generated at
@ 2023-09-08 15:22 James Keener <[email protected]>
parent: David G. Johnston <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: James Keener @ 2023-09-08 15:22 UTC (permalink / raw)
To: David G. Johnston <[email protected]>; [email protected] <[email protected]>
The issue here, though, is that it works as an expression for an index, but doesn't work as a generated column unless I explicitly cast it to text (which should have happened implicitly anyways). (The cast is turning a non-immutable expression to be immutable.)
I'm also able to make generated fields for the individual function calls, but concatenation doesn't work without the explicit cast.
Jim
On September 8, 2023 11:11:42 AM EDT, "David G. Johnston" <[email protected]> wrote:
>On Thursday, September 7, 2023, PG Bug reporting form <
>[email protected]> wrote:
>
>> The following bug has been logged on the website:
>>
>> Bug reference: 18097
>> Logged by: Jim Keener
>> Email address: [email protected]
>> PostgreSQL version: 15.0
>> Operating system: Linux
>> Description:
>>
>> However, the following DOES NOT work with an error of (ERROR: generation
>> expression is not immutable):
>>
>> * alter table test_table add created_local text GENERATED ALWAYS AS
>> (EXTRACT(isoyear FROM created_at AT TIME ZONE 'America/New_York') || '|' ||
>> EXTRACT(week FROM created_at AT TIME ZONE 'America/New_York')) STORED;
>>
>> Given that casting shouldn't "increase" the immutability of an expression,
>> and expression indexes need also be immutable afaik, I think that there is
>> a
>> bug somewhere here?
>>
>
>Casting very much can be a non-immutable activity, dates being the prime
>example, and I presume going from numeric to text is indeed defined to be
>stable hence the error. This is probably due to needing to consult locale
>for deciding how to represent the decimal places divider. This is one of
>the few places, assuming you write the function to set an environment
>fixing locale to some know value like you did with the time zones, where
>creating an immutable function around a stable expression makes sense.
>
>David J.
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: BUG #18097: Immutable expression not allowed in generated at
@ 2023-09-08 16:08 Tom Lane <[email protected]>
parent: James Keener <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: Tom Lane @ 2023-09-08 16:08 UTC (permalink / raw)
To: James Keener <[email protected]>; +Cc: David G. Johnston <[email protected]>; [email protected] <[email protected]>
James Keener <[email protected]> writes:
> The issue here, though, is that it works as an expression for an index, but doesn't work as a generated column unless I explicitly cast it to text (which should have happened implicitly anyways). (The cast is turning a non-immutable expression to be immutable.)
The reason that the generated expression fails is that (if you don't
explicitly cast to text) then it relies on anytextcat(anynonarray,text),
which is only stable, and can't be marked any more restrictively because
depending on the type of the non-text argument the corresponding output
function might not be immutable.
But then why doesn't the equivalent index definition spit up?
I found the answer in indexcmds.c's CheckMutability():
/*
* First run the expression through the planner. This has a couple of
* important consequences. First, function default arguments will get
* inserted, which may affect volatility (consider "default now()").
* Second, inline-able functions will get inlined, which may allow us to
* conclude that the function is really less volatile than it's marked. As
* an example, polymorphic functions must be marked with the most volatile
* behavior that they have for any input type, but once we inline the
* function we may be able to conclude that it's not so volatile for the
* particular input type we're dealing with.
*
* We assume here that expression_planner() won't scribble on its input.
*/
expr = expression_planner(expr);
/* Now we can search for non-immutable functions */
return contain_mutable_functions((Node *) expr);
Applying expression_planner() solves the problem because it inlines
anytextcat(anynonarray,text), resolving that the required cast is
numeric->text which is immutable. The code for generated expressions
omits that step and arrives at the less desirable answer. I wonder
where else we have the same issue.
regards, tom lane
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: BUG #18097: Immutable expression not allowed in generated at
@ 2023-09-09 19:18 Tom Lane <[email protected]>
parent: Tom Lane <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: Tom Lane @ 2023-09-09 19:18 UTC (permalink / raw)
To: [email protected]; +Cc: James Keener <[email protected]>
[ moving to pgsql-hackers ]
I wrote:
> Applying expression_planner() solves the problem because it inlines
> anytextcat(anynonarray,text), resolving that the required cast is
> numeric->text which is immutable. The code for generated expressions
> omits that step and arrives at the less desirable answer. I wonder
> where else we have the same issue.
After digging around, I could only find one other place where
outside-the-planner code was doing this wrong: AddRelationNewConstraints
can come to the wrong conclusion about whether it's safe to use
missingMode. So here's a patch series to resolve this. I split it
into three parts mostly because 0002 will only go back to v12 where
we added GENERATED, but the missingMode bug exists in v11.
There are a couple of points worth bikeshedding perhaps. I didn't
spend much thought on the wrapper functions' names, but it's surely
true that the semantic difference between contain_mutable_functions
and ContainMutableFunctions is quite un-apparent from those names.
Anybody got a better idea? It also seemed about fifty-fifty whether
to make the wrappers' argument types be Node * or Expr *. I stuck
with Expr * because that's what the predecessor code CheckMutability()
used, but that's not a very strong argument.
BTW, the test function in 0003 might look funny:
CREATE FUNCTION foolme(timestamptz DEFAULT clock_timestamp())
RETURNS timestamptz
IMMUTABLE AS 'select $1' LANGUAGE sql;
but AFAICS it's perfectly legit. The function itself is indeed immutable,
since it's only "select $1"; it's the default argument that's volatile.
I'll add this to the open CF 2023-11, but we really ought to
get it committed before that so we can ship these bug fixes in
November's releases.
regards, tom lane
Attachments:
[text/x-diff] v1-0001-Refactor-to-add-wrappers-for-contain_mutable-vola.patch (12.5K, ../../[email protected]/2-v1-0001-Refactor-to-add-wrappers-for-contain_mutable-vola.patch)
download | inline diff:
From b8387f19c846de52083121e36c0a40cad61d594b Mon Sep 17 00:00:00 2001
From: Tom Lane <[email protected]>
Date: Sat, 9 Sep 2023 13:54:16 -0400
Subject: [PATCH v1 1/3] Refactor to add wrappers for
contain_mutable/volatile_functions().
contain_mutable_functions and contain_volatile_functions give
reliable answers only after expression preprocessing (specifically
eval_const_expressions). Some places understand this, but some did
not get the memo --- which is not entirely their fault, because the
problem is documented only in places far away from those functions.
Introduce wrapper functions that allow doing the right thing easily,
and add commentary in hopes of preventing future mistakes from
copy-and-paste of code that's only conditionally safe.
This patch doesn't fix any actual bugs, just provide the infrastructure
for doing so. The only behavioral change is that I re-ordered the
steps in ComputePartitionAttrs so that its checks for invalid column
references are done before applying expression_planner, rather than
after. The previous coding would not complain if a partition expression
contains a disallowed column reference that gets optimized away by
constant folding, which seems to me to be a behavior we do not want.
Discussion: https://postgr.es/m/[email protected]
---
src/backend/commands/copyfrom.c | 6 ++-
src/backend/commands/indexcmds.c | 31 +-------------
src/backend/commands/tablecmds.c | 50 +++++++++++-----------
src/backend/optimizer/util/clauses.c | 64 ++++++++++++++++++++++++++++
src/include/optimizer/optimizer.h | 2 +
5 files changed, 99 insertions(+), 54 deletions(-)
diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c
index 70871ed819..a7ec68c25d 100644
--- a/src/backend/commands/copyfrom.c
+++ b/src/backend/commands/copyfrom.c
@@ -887,6 +887,9 @@ CopyFrom(CopyFromState cstate)
* Can't support multi-inserts if there are any volatile function
* expressions in WHERE clause. Similarly to the trigger case above,
* such expressions may query the table we're inserting into.
+ *
+ * Note: the whereClause was already preprocessed in DoCopy(), so it's
+ * okay to use contain_volatile_functions() directly.
*/
insertMethod = CIM_SINGLE;
}
@@ -1599,7 +1602,8 @@ BeginCopyFrom(ParseState *pstate,
* known to be safe for use with the multi-insert
* optimization. Hence we use this special case function
* checker rather than the standard check for
- * contain_volatile_functions().
+ * contain_volatile_functions(). Note also that we already
+ * ran the expression through expression_planner().
*/
if (!volatile_defexprs)
volatile_defexprs = contain_volatile_functions_not_nextval((Node *) defexpr);
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index ab8b81b302..9f9e2612cf 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -1785,33 +1785,6 @@ DefineIndex(Oid tableId,
}
-/*
- * CheckMutability
- * Test whether given expression is mutable
- */
-static bool
-CheckMutability(Expr *expr)
-{
- /*
- * First run the expression through the planner. This has a couple of
- * important consequences. First, function default arguments will get
- * inserted, which may affect volatility (consider "default now()").
- * Second, inline-able functions will get inlined, which may allow us to
- * conclude that the function is really less volatile than it's marked. As
- * an example, polymorphic functions must be marked with the most volatile
- * behavior that they have for any input type, but once we inline the
- * function we may be able to conclude that it's not so volatile for the
- * particular input type we're dealing with.
- *
- * We assume here that expression_planner() won't scribble on its input.
- */
- expr = expression_planner(expr);
-
- /* Now we can search for non-immutable functions */
- return contain_mutable_functions((Node *) expr);
-}
-
-
/*
* CheckPredicate
* Checks that the given partial-index predicate is valid.
@@ -1835,7 +1808,7 @@ CheckPredicate(Expr *predicate)
* A predicate using mutable functions is probably wrong, for the same
* reasons that we don't allow an index expression to use one.
*/
- if (CheckMutability(predicate))
+ if (ContainMutableFunctions(predicate))
ereport(ERROR,
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
errmsg("functions in index predicate must be marked IMMUTABLE")));
@@ -1978,7 +1951,7 @@ ComputeIndexAttrs(IndexInfo *indexInfo,
* same data every time, it's not clear what the index entries
* mean at all.
*/
- if (CheckMutability((Expr *) expr))
+ if (ContainMutableFunctions((Expr *) expr))
ereport(ERROR,
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
errmsg("functions in index expression must be marked IMMUTABLE")));
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 8a2c671b66..dfba7abbe0 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -18079,30 +18079,6 @@ ComputePartitionAttrs(ParseState *pstate, Relation rel, List *partParams, AttrNu
partattrs[attn] = 0; /* marks the column as expression */
*partexprs = lappend(*partexprs, expr);
- /*
- * Try to simplify the expression before checking for
- * mutability. The main practical value of doing it in this
- * order is that an inline-able SQL-language function will be
- * accepted if its expansion is immutable, whether or not the
- * function itself is marked immutable.
- *
- * Note that expression_planner does not change the passed in
- * expression destructively and we have already saved the
- * expression to be stored into the catalog above.
- */
- expr = (Node *) expression_planner((Expr *) expr);
-
- /*
- * Partition expression cannot contain mutable functions,
- * because a given row must always map to the same partition
- * as long as there is no change in the partition boundary
- * structure.
- */
- if (contain_mutable_functions(expr))
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
- errmsg("functions in partition key expression must be marked IMMUTABLE")));
-
/*
* transformPartitionSpec() should have already rejected
* subqueries, aggregates, window functions, and SRFs, based
@@ -18144,6 +18120,32 @@ ComputePartitionAttrs(ParseState *pstate, Relation rel, List *partParams, AttrNu
parser_errposition(pstate, pelem->location)));
}
+ /*
+ * Preprocess the expression before checking for mutability.
+ * This is essential for the reasons described in
+ * ContainMutableFunctions. However, we call
+ * expression_planner for ourselves rather than using that
+ * function, because if constant-folding reduces the
+ * expression to a constant, we'd like to know that so we can
+ * complain below.
+ *
+ * Like ContainMutableFunctions, assume that
+ * expression_planner won't scribble on its input, so this
+ * doesn't affect the partexprs entry we saved above.
+ */
+ expr = (Node *) expression_planner((Expr *) expr);
+
+ /*
+ * Partition expressions cannot contain mutable functions,
+ * because a given row must always map to the same partition
+ * as long as there is no change in the partition boundary
+ * structure.
+ */
+ if (contain_mutable_functions(expr))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("functions in partition key expression must be marked IMMUTABLE")));
+
/*
* While it is not exactly *wrong* for a partition expression
* to be a constant, it seems better to reject such keys.
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index da258968b8..484d8e6b3f 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -360,6 +360,10 @@ contain_subplans_walker(Node *node, void *context)
* mistakenly think that something like "WHERE random() < 0.5" can be treated
* as a constant qualification.
*
+ * This will give the right answer only for clauses that have been put
+ * through expression preprocessing. Callers outside the planner typically
+ * should use ContainMutableFunctions() instead, for the reasons given there.
+ *
* We will recursively look into Query nodes (i.e., SubLink sub-selects)
* but not into SubPlans. See comments for contain_volatile_functions().
*/
@@ -446,6 +450,34 @@ contain_mutable_functions_walker(Node *node, void *context)
context);
}
+/*
+ * ContainMutableFunctions
+ * Test whether given expression contains mutable functions.
+ *
+ * This is a wrapper for contain_mutable_functions() that is safe to use from
+ * outside the planner. The difference is that it first runs the expression
+ * through expression_planner(). There are two key reasons why we need that:
+ *
+ * First, function default arguments will get inserted, which may affect
+ * volatility (consider "default now()").
+ *
+ * Second, inline-able functions will get inlined, which may allow us to
+ * conclude that the function is really less volatile than it's marked.
+ * As an example, polymorphic functions must be marked with the most volatile
+ * behavior that they have for any input type, but once we inline the
+ * function we may be able to conclude that it's not so volatile for the
+ * particular input type we're dealing with.
+ */
+bool
+ContainMutableFunctions(Expr *expr)
+{
+ /* We assume here that expression_planner() won't scribble on its input */
+ expr = expression_planner(expr);
+
+ /* Now we can search for non-immutable functions */
+ return contain_mutable_functions((Node *) expr);
+}
+
/*****************************************************************************
* Check clauses for volatile functions
@@ -459,6 +491,10 @@ contain_mutable_functions_walker(Node *node, void *context)
* volatile function) is found. This test prevents, for example,
* invalid conversions of volatile expressions into indexscan quals.
*
+ * This will give the right answer only for clauses that have been put
+ * through expression preprocessing. Callers outside the planner typically
+ * should use ContainVolatileFunctions() instead, for the reasons given there.
+ *
* We will recursively look into Query nodes (i.e., SubLink sub-selects)
* but not into SubPlans. This is a bit odd, but intentional. If we are
* looking at a SubLink, we are probably deciding whether a query tree
@@ -582,6 +618,34 @@ contain_volatile_functions_walker(Node *node, void *context)
context);
}
+/*
+ * ContainVolatileFunctions
+ * Test whether given expression contains volatile functions.
+ *
+ * This is a wrapper for contain_volatile_functions() that is safe to use from
+ * outside the planner. The difference is that it first runs the expression
+ * through expression_planner(). There are two key reasons why we need that:
+ *
+ * First, function default arguments will get inserted, which may affect
+ * volatility (consider "default random()").
+ *
+ * Second, inline-able functions will get inlined, which may allow us to
+ * conclude that the function is really less volatile than it's marked.
+ * As an example, polymorphic functions must be marked with the most volatile
+ * behavior that they have for any input type, but once we inline the
+ * function we may be able to conclude that it's not so volatile for the
+ * particular input type we're dealing with.
+ */
+bool
+ContainVolatileFunctions(Expr *expr)
+{
+ /* We assume here that expression_planner() won't scribble on its input */
+ expr = expression_planner(expr);
+
+ /* Now we can search for volatile functions */
+ return contain_volatile_functions((Node *) expr);
+}
+
/*
* Special purpose version of contain_volatile_functions() for use in COPY:
* ignore nextval(), but treat all other functions normally.
diff --git a/src/include/optimizer/optimizer.h b/src/include/optimizer/optimizer.h
index 514746c585..8254da373b 100644
--- a/src/include/optimizer/optimizer.h
+++ b/src/include/optimizer/optimizer.h
@@ -138,7 +138,9 @@ extern Expr *canonicalize_qual(Expr *qual, bool is_check);
/* in util/clauses.c: */
extern bool contain_mutable_functions(Node *clause);
+extern bool ContainMutableFunctions(Expr *expr);
extern bool contain_volatile_functions(Node *clause);
+extern bool ContainVolatileFunctions(Expr *expr);
extern bool contain_volatile_functions_not_nextval(Node *clause);
extern Node *eval_const_expressions(PlannerInfo *root, Node *node);
--
2.39.3
[text/x-diff] v1-0002-Preprocess-column-GENERATED-expressions-before-ch.patch (3.1K, ../../[email protected]/3-v1-0002-Preprocess-column-GENERATED-expressions-before-ch.patch)
download | inline diff:
From 1e836fa15ff30c22de9b555196da3e8fbb5a2ad7 Mon Sep 17 00:00:00 2001
From: Tom Lane <[email protected]>
Date: Sat, 9 Sep 2023 14:40:15 -0400
Subject: [PATCH v1 2/3] Preprocess column GENERATED expressions before
checking mutability.
The previous coding skipped the preprocessing step and thus had
(at least) two failure modes: it could fail to notice the use of a
volatile function default-argument expression, or it could reject a
polymorphic function that is actually immutable on the datatype of
interest.
Per bug #18097 from Jim Keener. Back-patch to v12 where we added
generated columns.
Discussion: https://postgr.es/m/[email protected]
---
src/backend/catalog/heap.c | 4 +++-
src/test/regress/expected/generated.out | 3 +++
src/test/regress/sql/generated.sql | 3 +++
3 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index b42711f574..9b7544e9b7 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -3087,9 +3087,11 @@ cookDefault(ParseState *pstate,
if (attgenerated)
{
+ /* Disallow refs to other generated columns */
check_nested_generated(pstate, expr);
- if (contain_mutable_functions(expr))
+ /* Disallow mutable functions */
+ if (ContainMutableFunctions((Expr *) expr))
ereport(ERROR,
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
errmsg("generation expression is not immutable")));
diff --git a/src/test/regress/expected/generated.out b/src/test/regress/expected/generated.out
index dc97ed3fe0..a2f38d0f50 100644
--- a/src/test/regress/expected/generated.out
+++ b/src/test/regress/expected/generated.out
@@ -61,6 +61,9 @@ LINE 1: ..._3 (a int PRIMARY KEY, b int GENERATED ALWAYS AS (c * 2) STO...
-- generation expression must be immutable
CREATE TABLE gtest_err_4 (a int PRIMARY KEY, b double precision GENERATED ALWAYS AS (random()) STORED);
ERROR: generation expression is not immutable
+-- ... but be sure that the immutability test is accurate
+CREATE TABLE gtest2 (a int, b text GENERATED ALWAYS AS (a || ' sec') STORED);
+DROP TABLE gtest2;
-- cannot have default/identity and generated
CREATE TABLE gtest_err_5a (a int PRIMARY KEY, b int DEFAULT 5 GENERATED ALWAYS AS (a * 2) STORED);
ERROR: both default and generation expression specified for column "b" of table "gtest_err_5a"
diff --git a/src/test/regress/sql/generated.sql b/src/test/regress/sql/generated.sql
index 8ddecf0cc3..298f6b3aa8 100644
--- a/src/test/regress/sql/generated.sql
+++ b/src/test/regress/sql/generated.sql
@@ -26,6 +26,9 @@ CREATE TABLE gtest_err_3 (a int PRIMARY KEY, b int GENERATED ALWAYS AS (c * 2) S
-- generation expression must be immutable
CREATE TABLE gtest_err_4 (a int PRIMARY KEY, b double precision GENERATED ALWAYS AS (random()) STORED);
+-- ... but be sure that the immutability test is accurate
+CREATE TABLE gtest2 (a int, b text GENERATED ALWAYS AS (a || ' sec') STORED);
+DROP TABLE gtest2;
-- cannot have default/identity and generated
CREATE TABLE gtest_err_5a (a int PRIMARY KEY, b int DEFAULT 5 GENERATED ALWAYS AS (a * 2) STORED);
--
2.39.3
[text/x-diff] v1-0003-Preprocess-column-DEFAULT-expressions-before-chec.patch (3.5K, ../../[email protected]/4-v1-0003-Preprocess-column-DEFAULT-expressions-before-chec.patch)
download | inline diff:
From 26cfe11c9487d53982c903d929d9686fecf04911 Mon Sep 17 00:00:00 2001
From: Tom Lane <[email protected]>
Date: Sat, 9 Sep 2023 14:53:24 -0400
Subject: [PATCH v1 3/3] Preprocess column DEFAULT expressions before checking
volatility.
The previous coding skipped the preprocessing step and thus had
(at least) two failure modes: it could fail to notice the use of a
volatile function default-argument expression, or it could reject a
polymorphic function that is actually immutable on the datatype of
interest. The latter would just result in an unnecessary table
rewrite, but the former could allow the attmissingval functionality
to be used in a case where it should not be.
Noted while investigating bug #18097 from Jim Keener. Back-patch
to all supported versions.
Discussion: https://postgr.es/m/[email protected]
---
src/backend/catalog/heap.c | 2 +-
src/test/regress/expected/fast_default.out | 18 ++++++++++++++++++
src/test/regress/sql/fast_default.sql | 11 +++++++++++
3 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 9b7544e9b7..b44a4c96bb 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -2365,7 +2365,7 @@ AddRelationNewConstraints(Relation rel,
continue;
/* If the DEFAULT is volatile we cannot use a missing value */
- if (colDef->missingMode && contain_volatile_functions((Node *) expr))
+ if (colDef->missingMode && ContainVolatileFunctions((Expr *) expr))
colDef->missingMode = false;
defOid = StoreAttrDefault(rel, colDef->attnum, expr, is_internal,
diff --git a/src/test/regress/expected/fast_default.out b/src/test/regress/expected/fast_default.out
index 91f25717b5..59365dad96 100644
--- a/src/test/regress/expected/fast_default.out
+++ b/src/test/regress/expected/fast_default.out
@@ -272,7 +272,25 @@ SELECT comp();
Rewritten
(1 row)
+-- check that we notice insertion of a volatile default argument
+CREATE FUNCTION foolme(timestamptz DEFAULT clock_timestamp())
+ RETURNS timestamptz
+ IMMUTABLE AS 'select $1' LANGUAGE sql;
+ALTER TABLE T ADD COLUMN c3 timestamptz DEFAULT foolme();
+NOTICE: rewriting table t for reason 2
+SELECT attname, atthasmissing, attmissingval FROM pg_attribute
+ WHERE attrelid = 't'::regclass AND attnum > 0
+ ORDER BY attnum;
+ attname | atthasmissing | attmissingval
+---------+---------------+---------------
+ pk | f |
+ c1 | f |
+ c2 | f |
+ c3 | f |
+(4 rows)
+
DROP TABLE T;
+DROP FUNCTION foolme(timestamptz);
-- Simple querie
CREATE TABLE T (pk INT NOT NULL PRIMARY KEY);
SELECT set('t');
diff --git a/src/test/regress/sql/fast_default.sql b/src/test/regress/sql/fast_default.sql
index 16a3b7ca51..dc9df78a35 100644
--- a/src/test/regress/sql/fast_default.sql
+++ b/src/test/regress/sql/fast_default.sql
@@ -256,7 +256,18 @@ ALTER TABLE T ADD COLUMN c2 TIMESTAMP DEFAULT clock_timestamp();
SELECT comp();
+-- check that we notice insertion of a volatile default argument
+CREATE FUNCTION foolme(timestamptz DEFAULT clock_timestamp())
+ RETURNS timestamptz
+ IMMUTABLE AS 'select $1' LANGUAGE sql;
+ALTER TABLE T ADD COLUMN c3 timestamptz DEFAULT foolme();
+
+SELECT attname, atthasmissing, attmissingval FROM pg_attribute
+ WHERE attrelid = 't'::regclass AND attnum > 0
+ ORDER BY attnum;
+
DROP TABLE T;
+DROP FUNCTION foolme(timestamptz);
-- Simple querie
CREATE TABLE T (pk INT NOT NULL PRIMARY KEY);
--
2.39.3
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: BUG #18097: Immutable expression not allowed in generated at
@ 2023-09-09 22:56 Tom Lane <[email protected]>
parent: Tom Lane <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: Tom Lane @ 2023-09-09 22:56 UTC (permalink / raw)
To: [email protected]; +Cc: James Keener <[email protected]>
I wrote:
> After digging around, I could only find one other place where
> outside-the-planner code was doing this wrong: AddRelationNewConstraints
> can come to the wrong conclusion about whether it's safe to use
> missingMode. So here's a patch series to resolve this.
Argh ... I forgot to mention that there's one other place that
this patch series doesn't address, which is that publicationcmds.c's
check_simple_rowfilter_expr() also checks for volatile functions
without having preprocessed the expression. I'm not entirely sure
that there's a reachable problem in the direction of underestimating
the expression's volatility, given that that logic rejects non-builtin
functions entirely: it seems unlikely that any immutable builtin
function would have a volatile default expression. But it definitely
seems possible that there would be a problem in the other direction,
leading to rejecting row filter expressions that we'd like to allow,
much as in bug #18097.
I'm not sure about a good way to resolve this. Simply applying
expression simplification ahead of the check would break the code's
intent of rejecting non-builtin operators, in the case where such
an operator resolves to an inline-able builtin function. I find
the entire design of check_simple_rowfilter_expr pretty questionable
anyway, and there are a bunch of dubious (and undocumented) individual
decisions like allowing whole-row Vars, using FirstNormalObjectId
rather than FirstUnpinnedObjectId as the cutoff, etc. So I'm not
planning to touch that code, but somebody who was paying attention
when it was written might want to take a second look.
regards, tom lane
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: BUG #18097: Immutable expression not allowed in generated at
@ 2023-11-14 12:10 Aleksander Alekseev <[email protected]>
parent: Tom Lane <[email protected]>
0 siblings, 1 reply; 8+ messages in thread
From: Aleksander Alekseev @ 2023-11-14 12:10 UTC (permalink / raw)
To: [email protected]; +Cc: Tom Lane <[email protected]>; James Keener <[email protected]>
Hi,
I noticed that the patchset needs a review and decided to take a look.
> There are a couple of points worth bikeshedding perhaps. I didn't
> spend much thought on the wrapper functions' names, but it's surely
> true that the semantic difference between contain_mutable_functions
> and ContainMutableFunctions is quite un-apparent from those names.
> Anybody got a better idea?
Oh no! We encountered one of the most difficult problems in computer
science [1].
ContainMutableFunctionsAfterPerformingPlannersTransformations() would
be somewhat long but semantically correct. It can be shortened to
ContainMutableFunctionsAfterTransformations() or perhaps
TransformedExprContainMutableFunctions(). Personally I don't mind long
names. This being said, ContainMutableFunctions() doesn't disgusts my
sense of beauty too much either. All in all any name will do IMO.
Naturally ContainVolatileFunctions() should be renamed consistently
with ContainMutableFunctions().
I couldn't find anything wrong with 0001..0003. The parches were
tested in several environments and passed `make check-world`. I
suggest merging them.
[1]: https://martinfowler.com/bliki/TwoHardThings.html
--
Best regards,
Aleksander Alekseev
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: BUG #18097: Immutable expression not allowed in generated at
@ 2023-11-14 17:48 Tom Lane <[email protected]>
parent: Aleksander Alekseev <[email protected]>
0 siblings, 0 replies; 8+ messages in thread
From: Tom Lane @ 2023-11-14 17:48 UTC (permalink / raw)
To: Aleksander Alekseev <[email protected]>; +Cc: [email protected]; James Keener <[email protected]>
Aleksander Alekseev <[email protected]> writes:
>> There are a couple of points worth bikeshedding perhaps. I didn't
>> spend much thought on the wrapper functions' names, but it's surely
>> true that the semantic difference between contain_mutable_functions
>> and ContainMutableFunctions is quite un-apparent from those names.
>> Anybody got a better idea?
> Oh no! We encountered one of the most difficult problems in computer
> science [1].
Indeed :-(. Looking at it again this morning, I'm thinking of
using "contain_mutable_functions_after_planning" --- what do you
think of that?
regards, tom lane
^ permalink raw reply [nested|flat] 8+ messages in thread
end of thread, other threads:[~2023-11-14 17:48 UTC | newest]
Thread overview: 8+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-03-19 06:11 [PATCH v56 4/6] Doc part of shared-memory based stats collector. Kyotaro Horiguchi <[email protected]>
2023-09-08 15:11 Re: BUG #18097: Immutable expression not allowed in generated at David G. Johnston <[email protected]>
2023-09-08 15:22 ` Re: BUG #18097: Immutable expression not allowed in generated at James Keener <[email protected]>
2023-09-08 16:08 ` Re: BUG #18097: Immutable expression not allowed in generated at Tom Lane <[email protected]>
2023-09-09 19:18 ` Re: BUG #18097: Immutable expression not allowed in generated at Tom Lane <[email protected]>
2023-09-09 22:56 ` Re: BUG #18097: Immutable expression not allowed in generated at Tom Lane <[email protected]>
2023-11-14 12:10 ` Re: BUG #18097: Immutable expression not allowed in generated at Aleksander Alekseev <[email protected]>
2023-11-14 17:48 ` Re: BUG #18097: Immutable expression not allowed in generated at Tom Lane <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox