agora inbox for pgsql-hackers@postgresql.org  
help / color / mirror / Atom feed
[PATCH 3/3] Fix failure of standby startup caused by tablespace removal
6+ messages / 3 participants
[nested] [flat]

* [PATCH 4/5] Fix failure of standby startup caused by tablespace removal
@ 2019-04-22 11:59 Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
  0 siblings, 0 replies; 6+ messages in thread

From: Kyotaro Horiguchi @ 2019-04-22 11:59 UTC (permalink / raw)

When standby restarts after a crash after drop of a tablespace,
there's a possibility that recovery fails trying an object-creation in
already removed tablespace directory. Allow recovery to continue by
ignoring the error if not reaching consistency point.
---
 src/backend/access/transam/xlogutils.c | 34 ++++++++++++++++++++++++++++++++++
 src/backend/commands/tablespace.c      | 12 ++++++------
 src/backend/storage/file/copydir.c     | 12 +++++++-----
 src/include/access/xlogutils.h         |  1 +
 4 files changed, 48 insertions(+), 11 deletions(-)

diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index 10a663bae6..75cdb882cd 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -522,6 +522,40 @@ XLogReadBufferExtended(RelFileNode rnode, ForkNumber forknum,
 	return buffer;
 }
 
+/*
+ * XLogMakePGDirectory
+ *
+ * There is a possibility that WAL replay causes an error by creation of a
+ * directory under a directory removed before the previous crash. Issuing
+ * ERROR prevents the caller from continuing recovery.
+ *
+ * To prevent that case, this function issues WARNING instead of ERROR on
+ * error if consistency is not reached yet.
+ */
+int
+XLogMakePGDirectory(const char *directoryName)
+{
+	int ret;
+
+	ret = MakePGDirectory(directoryName);
+
+	if (ret != 0)
+	{
+		int elevel = ERROR;
+
+		/* Don't issue ERROR for this failure before reaching consistency. */
+		if (InRecovery && !reachedConsistency)
+			elevel = WARNING;
+
+		ereport(elevel,
+				(errcode_for_file_access(),
+				 errmsg("could not create directory \"%s\": %m", directoryName)));
+		return ret;
+	}
+
+	return 0;
+}
+
 /*
  * Struct actually returned by XLogFakeRelcacheEntry, though the declared
  * return type is Relation.
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index 66a70871e6..c9fb2af015 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -303,12 +303,6 @@ CreateTableSpace(CreateTableSpaceStmt *stmt)
 				(errcode(ERRCODE_INVALID_NAME),
 				 errmsg("tablespace location cannot contain single quotes")));
 
-	/* Reject tablespaces in the data directory. */
-	if (is_in_data_directory(location))
-		ereport(ERROR,
-				(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-				 errmsg("tablespace location must not be inside the data directory")));
-
 	/*
 	 * Check that location isn't too long. Remember that we're going to append
 	 * 'PG_XXX/<dboid>/<relid>_<fork>.<nnn>'.  In the relative path case, we
@@ -323,6 +317,12 @@ CreateTableSpace(CreateTableSpaceStmt *stmt)
 				 errmsg("tablespace location \"%s\" is too long",
 						location)));
 
+	/* Reject tablespaces in the data directory. */
+	if (is_in_data_directory(location))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+				 errmsg("tablespace location must not be inside the data directory")));
+
 	/*
 	 * Disallow creation of tablespaces named "pg_xxx"; we reserve this
 	 * namespace for system purposes.
diff --git a/src/backend/storage/file/copydir.c b/src/backend/storage/file/copydir.c
index 30f6200a86..0216270dd3 100644
--- a/src/backend/storage/file/copydir.c
+++ b/src/backend/storage/file/copydir.c
@@ -22,11 +22,11 @@
 #include <unistd.h>
 #include <sys/stat.h>
 
+#include "access/xlogutils.h"
 #include "storage/copydir.h"
 #include "storage/fd.h"
 #include "miscadmin.h"
 #include "pgstat.h"
-
 /*
  * copydir: copy a directory
  *
@@ -41,10 +41,12 @@ copydir(char *fromdir, char *todir, bool recurse)
 	char		fromfile[MAXPGPATH * 2];
 	char		tofile[MAXPGPATH * 2];
 
-	if (MakePGDirectory(todir) != 0)
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("could not create directory \"%s\": %m", todir)));
+	/*
+	 * We might have to skip copydir to continue recovery. See the function
+	 * for details.
+	 */
+	if (XLogMakePGDirectory(todir) != 0)
+		return;
 
 	xldir = AllocateDir(fromdir);
 
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 0ab5ba62f5..46a7596315 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -43,6 +43,7 @@ extern XLogRedoAction XLogReadBufferForRedoExtended(XLogReaderState *record,
 
 extern Buffer XLogReadBufferExtended(RelFileNode rnode, ForkNumber forknum,
 					   BlockNumber blkno, ReadBufferMode mode);
+extern int XLogMakePGDirectory(const char *directoryName);
 
 extern Relation CreateFakeRelcacheEntry(RelFileNode rnode);
 extern void FreeFakeRelcacheEntry(Relation fakerel);
-- 
2.16.3


----Next_Part(Wed_Apr_24_17_02_28_2019_644)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline; filename="v2-0005-Documentation-update.patch"



^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* [PATCH 4/5] Fix failure of standby startup caused by tablespace removal
@ 2019-04-22 11:59 Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
  0 siblings, 0 replies; 6+ messages in thread

From: Kyotaro Horiguchi @ 2019-04-22 11:59 UTC (permalink / raw)

When standby restarts after a crash after drop of a tablespace,
there's a possibility that recovery fails trying an object-creation in
already removed tablespace directory. Allow recovery to continue by
ignoring the error if not reaching consistency point.
---
 src/backend/access/transam/xlogutils.c | 34 ++++++++++++++++++++++++++++++++++
 src/backend/commands/tablespace.c      | 12 ++++++------
 src/backend/storage/file/copydir.c     | 12 +++++++-----
 src/include/access/xlogutils.h         |  1 +
 4 files changed, 48 insertions(+), 11 deletions(-)

diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index 10a663bae6..75cdb882cd 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -522,6 +522,40 @@ XLogReadBufferExtended(RelFileNode rnode, ForkNumber forknum,
 	return buffer;
 }
 
+/*
+ * XLogMakePGDirectory
+ *
+ * There is a possibility that WAL replay causes an error by creation of a
+ * directory under a directory removed before the previous crash. Issuing
+ * ERROR prevents the caller from continuing recovery.
+ *
+ * To prevent that case, this function issues WARNING instead of ERROR on
+ * error if consistency is not reached yet.
+ */
+int
+XLogMakePGDirectory(const char *directoryName)
+{
+	int ret;
+
+	ret = MakePGDirectory(directoryName);
+
+	if (ret != 0)
+	{
+		int elevel = ERROR;
+
+		/* Don't issue ERROR for this failure before reaching consistency. */
+		if (InRecovery && !reachedConsistency)
+			elevel = WARNING;
+
+		ereport(elevel,
+				(errcode_for_file_access(),
+				 errmsg("could not create directory \"%s\": %m", directoryName)));
+		return ret;
+	}
+
+	return 0;
+}
+
 /*
  * Struct actually returned by XLogFakeRelcacheEntry, though the declared
  * return type is Relation.
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index 66a70871e6..c9fb2af015 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -303,12 +303,6 @@ CreateTableSpace(CreateTableSpaceStmt *stmt)
 				(errcode(ERRCODE_INVALID_NAME),
 				 errmsg("tablespace location cannot contain single quotes")));
 
-	/* Reject tablespaces in the data directory. */
-	if (is_in_data_directory(location))
-		ereport(ERROR,
-				(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-				 errmsg("tablespace location must not be inside the data directory")));
-
 	/*
 	 * Check that location isn't too long. Remember that we're going to append
 	 * 'PG_XXX/<dboid>/<relid>_<fork>.<nnn>'.  In the relative path case, we
@@ -323,6 +317,12 @@ CreateTableSpace(CreateTableSpaceStmt *stmt)
 				 errmsg("tablespace location \"%s\" is too long",
 						location)));
 
+	/* Reject tablespaces in the data directory. */
+	if (is_in_data_directory(location))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+				 errmsg("tablespace location must not be inside the data directory")));
+
 	/*
 	 * Disallow creation of tablespaces named "pg_xxx"; we reserve this
 	 * namespace for system purposes.
diff --git a/src/backend/storage/file/copydir.c b/src/backend/storage/file/copydir.c
index 30f6200a86..0216270dd3 100644
--- a/src/backend/storage/file/copydir.c
+++ b/src/backend/storage/file/copydir.c
@@ -22,11 +22,11 @@
 #include <unistd.h>
 #include <sys/stat.h>
 
+#include "access/xlogutils.h"
 #include "storage/copydir.h"
 #include "storage/fd.h"
 #include "miscadmin.h"
 #include "pgstat.h"
-
 /*
  * copydir: copy a directory
  *
@@ -41,10 +41,12 @@ copydir(char *fromdir, char *todir, bool recurse)
 	char		fromfile[MAXPGPATH * 2];
 	char		tofile[MAXPGPATH * 2];
 
-	if (MakePGDirectory(todir) != 0)
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("could not create directory \"%s\": %m", todir)));
+	/*
+	 * We might have to skip copydir to continue recovery. See the function
+	 * for details.
+	 */
+	if (XLogMakePGDirectory(todir) != 0)
+		return;
 
 	xldir = AllocateDir(fromdir);
 
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 0ab5ba62f5..46a7596315 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -43,6 +43,7 @@ extern XLogRedoAction XLogReadBufferForRedoExtended(XLogReaderState *record,
 
 extern Buffer XLogReadBufferExtended(RelFileNode rnode, ForkNumber forknum,
 					   BlockNumber blkno, ReadBufferMode mode);
+extern int XLogMakePGDirectory(const char *directoryName);
 
 extern Relation CreateFakeRelcacheEntry(RelFileNode rnode);
 extern void FreeFakeRelcacheEntry(Relation fakerel);
-- 
2.16.3


----Next_Part(Wed_Apr_24_17_13_54_2019_374)----






^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* [PATCH 3/3] Fix failure of standby startup caused by tablespace removal
@ 2019-04-22 11:59 Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
  0 siblings, 0 replies; 6+ messages in thread

From: Kyotaro Horiguchi @ 2019-04-22 11:59 UTC (permalink / raw)

When standby restarts after a crash after drop of a tablespace,
there's a possibility that recovery fails trying an object-creation in
already removed tablespace directory. Allow recovery to continue by
ignoring the error if not reaching consistency point.
---
 src/backend/access/transam/xlogutils.c | 34 ++++++++++++++++++++++++++++++++++
 src/backend/storage/file/copydir.c     | 12 +++++++-----
 src/include/access/xlogutils.h         |  1 +
 3 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index 10a663bae6..75cdb882cd 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -522,6 +522,40 @@ XLogReadBufferExtended(RelFileNode rnode, ForkNumber forknum,
 	return buffer;
 }
 
+/*
+ * XLogMakePGDirectory
+ *
+ * There is a possibility that WAL replay causes an error by creation of a
+ * directory under a directory removed before the previous crash. Issuing
+ * ERROR prevents the caller from continuing recovery.
+ *
+ * To prevent that case, this function issues WARNING instead of ERROR on
+ * error if consistency is not reached yet.
+ */
+int
+XLogMakePGDirectory(const char *directoryName)
+{
+	int ret;
+
+	ret = MakePGDirectory(directoryName);
+
+	if (ret != 0)
+	{
+		int elevel = ERROR;
+
+		/* Don't issue ERROR for this failure before reaching consistency. */
+		if (InRecovery && !reachedConsistency)
+			elevel = WARNING;
+
+		ereport(elevel,
+				(errcode_for_file_access(),
+				 errmsg("could not create directory \"%s\": %m", directoryName)));
+		return ret;
+	}
+
+	return 0;
+}
+
 /*
  * Struct actually returned by XLogFakeRelcacheEntry, though the declared
  * return type is Relation.
diff --git a/src/backend/storage/file/copydir.c b/src/backend/storage/file/copydir.c
index 30f6200a86..0216270dd3 100644
--- a/src/backend/storage/file/copydir.c
+++ b/src/backend/storage/file/copydir.c
@@ -22,11 +22,11 @@
 #include <unistd.h>
 #include <sys/stat.h>
 
+#include "access/xlogutils.h"
 #include "storage/copydir.h"
 #include "storage/fd.h"
 #include "miscadmin.h"
 #include "pgstat.h"
-
 /*
  * copydir: copy a directory
  *
@@ -41,10 +41,12 @@ copydir(char *fromdir, char *todir, bool recurse)
 	char		fromfile[MAXPGPATH * 2];
 	char		tofile[MAXPGPATH * 2];
 
-	if (MakePGDirectory(todir) != 0)
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("could not create directory \"%s\": %m", todir)));
+	/*
+	 * We might have to skip copydir to continue recovery. See the function
+	 * for details.
+	 */
+	if (XLogMakePGDirectory(todir) != 0)
+		return;
 
 	xldir = AllocateDir(fromdir);
 
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 0ab5ba62f5..46a7596315 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -43,6 +43,7 @@ extern XLogRedoAction XLogReadBufferForRedoExtended(XLogReaderState *record,
 
 extern Buffer XLogReadBufferExtended(RelFileNode rnode, ForkNumber forknum,
 					   BlockNumber blkno, ReadBufferMode mode);
+extern int XLogMakePGDirectory(const char *directoryName);
 
 extern Relation CreateFakeRelcacheEntry(RelFileNode rnode);
 extern void FreeFakeRelcacheEntry(Relation fakerel);
-- 
2.16.3


----Next_Part(Mon_Apr_22_21_19_33_2019_510)----






^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* [PATCH 4/4] Fix failure of standby startup caused by tablespace removal
@ 2019-04-22 11:59 Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
  0 siblings, 0 replies; 6+ messages in thread

From: Kyotaro Horiguchi @ 2019-04-22 11:59 UTC (permalink / raw)

When standby restarts after a crash after drop of a tablespace,
there's a possibility that recovery fails trying an object-creation in
already removed tablespace directory. Allow recovery to continue by
ignoring the error if not reaching consistency point.
---
 src/backend/access/transam/xlogutils.c | 34 ++++++++++++++++++++++++++++++++++
 src/backend/commands/tablespace.c      | 12 ++++++------
 src/backend/storage/file/copydir.c     | 12 +++++++-----
 src/include/access/xlogutils.h         |  1 +
 4 files changed, 48 insertions(+), 11 deletions(-)

diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index 10a663bae6..75cdb882cd 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -522,6 +522,40 @@ XLogReadBufferExtended(RelFileNode rnode, ForkNumber forknum,
 	return buffer;
 }
 
+/*
+ * XLogMakePGDirectory
+ *
+ * There is a possibility that WAL replay causes an error by creation of a
+ * directory under a directory removed before the previous crash. Issuing
+ * ERROR prevents the caller from continuing recovery.
+ *
+ * To prevent that case, this function issues WARNING instead of ERROR on
+ * error if consistency is not reached yet.
+ */
+int
+XLogMakePGDirectory(const char *directoryName)
+{
+	int ret;
+
+	ret = MakePGDirectory(directoryName);
+
+	if (ret != 0)
+	{
+		int elevel = ERROR;
+
+		/* Don't issue ERROR for this failure before reaching consistency. */
+		if (InRecovery && !reachedConsistency)
+			elevel = WARNING;
+
+		ereport(elevel,
+				(errcode_for_file_access(),
+				 errmsg("could not create directory \"%s\": %m", directoryName)));
+		return ret;
+	}
+
+	return 0;
+}
+
 /*
  * Struct actually returned by XLogFakeRelcacheEntry, though the declared
  * return type is Relation.
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index 66a70871e6..c9fb2af015 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -303,12 +303,6 @@ CreateTableSpace(CreateTableSpaceStmt *stmt)
 				(errcode(ERRCODE_INVALID_NAME),
 				 errmsg("tablespace location cannot contain single quotes")));
 
-	/* Reject tablespaces in the data directory. */
-	if (is_in_data_directory(location))
-		ereport(ERROR,
-				(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-				 errmsg("tablespace location must not be inside the data directory")));
-
 	/*
 	 * Check that location isn't too long. Remember that we're going to append
 	 * 'PG_XXX/<dboid>/<relid>_<fork>.<nnn>'.  In the relative path case, we
@@ -323,6 +317,12 @@ CreateTableSpace(CreateTableSpaceStmt *stmt)
 				 errmsg("tablespace location \"%s\" is too long",
 						location)));
 
+	/* Reject tablespaces in the data directory. */
+	if (is_in_data_directory(location))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+				 errmsg("tablespace location must not be inside the data directory")));
+
 	/*
 	 * Disallow creation of tablespaces named "pg_xxx"; we reserve this
 	 * namespace for system purposes.
diff --git a/src/backend/storage/file/copydir.c b/src/backend/storage/file/copydir.c
index 30f6200a86..0216270dd3 100644
--- a/src/backend/storage/file/copydir.c
+++ b/src/backend/storage/file/copydir.c
@@ -22,11 +22,11 @@
 #include <unistd.h>
 #include <sys/stat.h>
 
+#include "access/xlogutils.h"
 #include "storage/copydir.h"
 #include "storage/fd.h"
 #include "miscadmin.h"
 #include "pgstat.h"
-
 /*
  * copydir: copy a directory
  *
@@ -41,10 +41,12 @@ copydir(char *fromdir, char *todir, bool recurse)
 	char		fromfile[MAXPGPATH * 2];
 	char		tofile[MAXPGPATH * 2];
 
-	if (MakePGDirectory(todir) != 0)
-		ereport(ERROR,
-				(errcode_for_file_access(),
-				 errmsg("could not create directory \"%s\": %m", todir)));
+	/*
+	 * We might have to skip copydir to continue recovery. See the function
+	 * for details.
+	 */
+	if (XLogMakePGDirectory(todir) != 0)
+		return;
 
 	xldir = AllocateDir(fromdir);
 
diff --git a/src/include/access/xlogutils.h b/src/include/access/xlogutils.h
index 0ab5ba62f5..46a7596315 100644
--- a/src/include/access/xlogutils.h
+++ b/src/include/access/xlogutils.h
@@ -43,6 +43,7 @@ extern XLogRedoAction XLogReadBufferForRedoExtended(XLogReaderState *record,
 
 extern Buffer XLogReadBufferExtended(RelFileNode rnode, ForkNumber forknum,
 					   BlockNumber blkno, ReadBufferMode mode);
+extern int XLogMakePGDirectory(const char *directoryName);
 
 extern Relation CreateFakeRelcacheEntry(RelFileNode rnode);
 extern void FreeFakeRelcacheEntry(Relation fakerel);
-- 
2.16.3


----Next_Part(Wed_Apr_24_13_18_45_2019_938)----






^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* Re: Allow to collect statistics on virtual generated columns
@ 2026-04-01 16:12 Dean Rasheed <dean.a.rasheed@gmail.com>
  2026-04-03 01:10 ` Re: Allow to collect statistics on virtual generated columns Yugo Nagata <nagata@sraoss.co.jp>
  0 siblings, 1 reply; 6+ messages in thread

From: Dean Rasheed @ 2026-04-01 16:12 UTC (permalink / raw)
  To: Yugo Nagata <nagata@sraoss.co.jp>; +Cc: Andres Freund <andres@anarazel.de>; pgsql-hackers

On Tue, 31 Mar 2026 at 15:35, Yugo Nagata <nagata@sraoss.co.jp> wrote:
>
> Thank you for updating the patch. I am fine with that.
>
> One concern is that users might interpret "stored" as referring to
> "stored generated columns", rather than including regular columns,

Yeah, possibly. I changed "stored" to "non-virtual", which should
reduce the chances of that particular confusion.

> Also, the meaning of "automatically" might be a bit unclear, so we
> could clarify it by adding "without defining extended statistics."
>
>       Defining extended statistics on a single <emphasis>stored</emphasis>
>       column is not supported or necessary, because statistics are built
>       automatically on such columns without defining extended statistics.

OK, pushed that way.

I also noticed that a few places in CreateStatistics() could use the
variable "numcols" instead of "list_length(stmt->exprs)", so I changed
that.

I decided to include the change to the error message discussed in [1],
since there seemed to be a consensus there, except that I think it
also needs to make it clear that it refers only to non-virtual
columns.

In addition, there was another nearby error message which was no
longer quite right for statistics on virtual generated columns, and I
changed the order of checks, since that allowed the "if" statements to
be simplified a bit.

Regards,
Dean

[1] https://www.postgresql.org/message-id/20260324203430.411331c59ca462457ec0aa8b%40sraoss.co.jp





^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* Re: Allow to collect statistics on virtual generated columns
  2026-04-01 16:12 Re: Allow to collect statistics on virtual generated columns Dean Rasheed <dean.a.rasheed@gmail.com>
@ 2026-04-03 01:10 ` Yugo Nagata <nagata@sraoss.co.jp>
  0 siblings, 0 replies; 6+ messages in thread

From: Yugo Nagata @ 2026-04-03 01:10 UTC (permalink / raw)
  To: Dean Rasheed <dean.a.rasheed@gmail.com>; +Cc: Andres Freund <andres@anarazel.de>; pgsql-hackers

On Wed, 1 Apr 2026 17:12:38 +0100
Dean Rasheed <dean.a.rasheed@gmail.com> wrote:

> On Tue, 31 Mar 2026 at 15:35, Yugo Nagata <nagata@sraoss.co.jp> wrote:
> >
> > Thank you for updating the patch. I am fine with that.
> >
> > One concern is that users might interpret "stored" as referring to
> > "stored generated columns", rather than including regular columns,
> 
> Yeah, possibly. I changed "stored" to "non-virtual", which should
> reduce the chances of that particular confusion.
> 
> > Also, the meaning of "automatically" might be a bit unclear, so we
> > could clarify it by adding "without defining extended statistics."
> >
> >       Defining extended statistics on a single <emphasis>stored</emphasis>
> >       column is not supported or necessary, because statistics are built
> >       automatically on such columns without defining extended statistics.
> 
> OK, pushed that way.

Thank you!

> 
> I also noticed that a few places in CreateStatistics() could use the
> variable "numcols" instead of "list_length(stmt->exprs)", so I changed
> that.
> 
> I decided to include the change to the error message discussed in [1],
> since there seemed to be a consensus there, except that I think it
> also needs to make it clear that it refers only to non-virtual
> columns.
> 
> In addition, there was another nearby error message which was no
> longer quite right for statistics on virtual generated columns, and I
> changed the order of checks, since that allowed the "if" statements to
> be simplified a bit.

I reviewed the commit, and it looks good to me.
Thanks again.

Regards,
Yugo Nagata


-- 
Yugo Nagata <nagata@sraoss.co.jp>





^ permalink  raw  reply  [nested|flat] 6+ messages in thread


end of thread, other threads:[~2026-04-03 01:10 UTC | newest]

Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-04-22 11:59 [PATCH 4/5] Fix failure of standby startup caused by tablespace removal Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
2019-04-22 11:59 [PATCH 4/5] Fix failure of standby startup caused by tablespace removal Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
2019-04-22 11:59 [PATCH 3/3] Fix failure of standby startup caused by tablespace removal Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
2019-04-22 11:59 [PATCH 4/4] Fix failure of standby startup caused by tablespace removal Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
2026-04-01 16:12 Re: Allow to collect statistics on virtual generated columns Dean Rasheed <dean.a.rasheed@gmail.com>
2026-04-03 01:10 ` Re: Allow to collect statistics on virtual generated columns 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