improve CREATE EXTENSION error message
Hi hackers,
Currently, if you attempt to use CREATE EXTENSION for an extension
that is not installed, you'll see something like the following:
postgres=# CREATE EXTENSION does_not_exist;
ERROR: could not open extension control file "/usr/local/pgsql/share/extension/does_not_exist.control": No such file or directory
I suspect this ERROR message is confusing for novice users, so perhaps
we should add a HINT. With the attached patch, you'd see the
following:
postgres=# CREATE EXTENSION does_not_exist;
ERROR: could not open extension control file "/usr/local/pgsql/share/extension/does_not_exist.control": No such file or directory
HINT: This typically indicates that the specified extension is not installed on the system.
Thoughts?
Nathan
Attachments:
v1-0001-Improve-CREATE-EXTENSION-error-message.patchapplication/octet-stream; name=v1-0001-Improve-CREATE-EXTENSION-error-message.patchDownload
From 5cc486ccef2f34fc89b2a7f522fb57a9258b36f1 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <bossartn@amazon.com>
Date: Mon, 29 Nov 2021 19:53:00 +0000
Subject: [PATCH v1 1/1] Improve CREATE EXTENSION error message.
---
src/backend/commands/extension.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index eaa76af47b..4604b01ac9 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -496,7 +496,9 @@ parse_extension_control_file(ExtensionControlFile *control,
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not open extension control file \"%s\": %m",
- filename)));
+ filename),
+ errhint("This typically indicates that the specified "
+ "extension is not installed on the system.")));
}
/*
--
2.16.6
On 29 Nov 2021, at 20:54, Bossart, Nathan <bossartn@amazon.com> wrote:
Hi hackers,
Currently, if you attempt to use CREATE EXTENSION for an extension
that is not installed, you'll see something like the following:postgres=# CREATE EXTENSION does_not_exist;
ERROR: could not open extension control file "/usr/local/pgsql/share/extension/does_not_exist.control": No such file or directoryI suspect this ERROR message is confusing for novice users, so perhaps
we should add a HINT. With the attached patch, you'd see the
following:postgres=# CREATE EXTENSION does_not_exist;
ERROR: could not open extension control file "/usr/local/pgsql/share/extension/does_not_exist.control": No such file or directory
HINT: This typically indicates that the specified extension is not installed on the system.Thoughts?
I haven't given the suggested wording too much thought, but in general that
sounds like a good idea.
--
Daniel Gustafsson https://vmware.com/
"Bossart, Nathan" <bossartn@amazon.com> writes:
Currently, if you attempt to use CREATE EXTENSION for an extension
that is not installed, you'll see something like the following:
postgres=# CREATE EXTENSION does_not_exist;
ERROR: could not open extension control file "/usr/local/pgsql/share/extension/does_not_exist.control": No such file or directory
I suspect this ERROR message is confusing for novice users, so perhaps
we should add a HINT.
If we issue the hint only for errno == ENOENT, I think we could be
less wishy-washy (and if it's not ENOENT, the hint is likely
inappropriate anyway). I'm thinking something more like
HINT: This means the extension is not installed on the system.
I'm not quite satisfied with the "on the system" wording, but I'm
not sure of what would be better. I agree that we can't just say
"is not installed", because people will confuse that with whether
it is installed within the database.
regards, tom lane
On 11/29/21, 12:33 PM, "Daniel Gustafsson" <daniel@yesql.se> wrote:
I haven't given the suggested wording too much thought, but in general that
sounds like a good idea.
Thanks. I'm flexible with the wording.
Nathan
On 29 Nov 2021, at 22:02, Tom Lane <tgl@sss.pgh.pa.us> wrote:
"Bossart, Nathan" <bossartn@amazon.com> writes:
Currently, if you attempt to use CREATE EXTENSION for an extension
that is not installed, you'll see something like the following:postgres=# CREATE EXTENSION does_not_exist;
ERROR: could not open extension control file "/usr/local/pgsql/share/extension/does_not_exist.control": No such file or directoryI suspect this ERROR message is confusing for novice users, so perhaps
we should add a HINT.If we issue the hint only for errno == ENOENT, I think we could be
less wishy-washy (and if it's not ENOENT, the hint is likely
inappropriate anyway). I'm thinking something more likeHINT: This means the extension is not installed on the system.
I'm not quite satisfied with the "on the system" wording, but I'm
not sure of what would be better. I agree that we can't just say
"is not installed", because people will confuse that with whether
it is installed within the database.
That's a good point, the hint is targeting users who might not even know that
an extension needs to be physically and separately installed on the machine
before it can be installed in their database; so maybe using "installed" here
isn't entirely helpful at all. That being said I'm at a loss for a more
suitable word, "available" perhaps?
--
Daniel Gustafsson https://vmware.com/
On 11/29/21, 1:03 PM, "Tom Lane" <tgl@sss.pgh.pa.us> wrote:
If we issue the hint only for errno == ENOENT, I think we could be
less wishy-washy (and if it's not ENOENT, the hint is likely
inappropriate anyway). I'm thinking something more likeHINT: This means the extension is not installed on the system.
Good idea.
I'm not quite satisfied with the "on the system" wording, but I'm
not sure of what would be better. I agree that we can't just say
"is not installed", because people will confuse that with whether
it is installed within the database.
Right. The only other idea I have at the moment is to say something
like
This means the extension is not available[ on the system].
I don't know whether that is actually any less confusing, though.
Nathan
On 11/29/21 16:31, Daniel Gustafsson wrote:
That's a good point, the hint is targeting users who might not even know that
an extension needs to be physically and separately installed on the machine
before it can be installed in their database; so maybe using "installed" here
isn't entirely helpful at all. That being said I'm at a loss for a more
suitable word, "available" perhaps?
Maybe a larger break with the "This means the extension something something"
formulation, and more on the lines of
HINT: an extension must first be present (for example, installed with a
package manager) on the system where PostgreSQL is running.
Regards,
-Chap
On 11/29/21, 1:32 PM, "Daniel Gustafsson" <daniel@yesql.se> wrote:
On 29 Nov 2021, at 22:02, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I'm not quite satisfied with the "on the system" wording, but I'm
not sure of what would be better. I agree that we can't just say
"is not installed", because people will confuse that with whether
it is installed within the database.That's a good point, the hint is targeting users who might not even know that
an extension needs to be physically and separately installed on the machine
before it can be installed in their database; so maybe using "installed" here
isn't entirely helpful at all. That being said I'm at a loss for a more
suitable word, "available" perhaps?
I was just thinking the same thing. I used "available" in v2, which
is attached.
Nathan
Attachments:
v2-0001-Improve-CREATE-EXTENSION-error-message.patchapplication/octet-stream; name=v2-0001-Improve-CREATE-EXTENSION-error-message.patchDownload
From 73fcba4a050e240d4e7c0a9e230e544ee6971a15 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <bossartn@amazon.com>
Date: Mon, 29 Nov 2021 21:35:59 +0000
Subject: [PATCH v2 1/1] Improve CREATE EXTENSION error message.
---
src/backend/commands/extension.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index eaa76af47b..e345f7fd52 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -487,11 +487,21 @@ parse_extension_control_file(ExtensionControlFile *control,
if ((file = AllocateFile(filename, "r")) == NULL)
{
- if (version && errno == ENOENT)
+ if (errno == ENOENT)
{
- /* no auxiliary file for this version */
- pfree(filename);
- return;
+ /* auxiliary files are optional */
+ if (version)
+ {
+ pfree(filename);
+ return;
+ }
+
+ /* missing control file indicates extension is not installed */
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not open extension control file \"%s\": %m",
+ filename),
+ errhint("This means the extension is not available on the system.")));
}
ereport(ERROR,
(errcode_for_file_access(),
--
2.16.6
On 11/29/21, 1:38 PM, "Chapman Flack" <chap@anastigmatix.net> wrote:
On 11/29/21 16:31, Daniel Gustafsson wrote:
That's a good point, the hint is targeting users who might not even know that
an extension needs to be physically and separately installed on the machine
before it can be installed in their database; so maybe using "installed" here
isn't entirely helpful at all. That being said I'm at a loss for a more
suitable word, "available" perhaps?Maybe a larger break with the "This means the extension something something"
formulation, and more on the lines ofHINT: an extension must first be present (for example, installed with a
package manager) on the system where PostgreSQL is running.
I like this idea. I can do it this way in the next revision if others
agree.
Nathan
On 29 Nov 2021, at 22:47, Bossart, Nathan <bossartn@amazon.com> wrote:
On 11/29/21, 1:38 PM, "Chapman Flack" <chap@anastigmatix.net> wrote:
On 11/29/21 16:31, Daniel Gustafsson wrote:
That's a good point, the hint is targeting users who might not even know that
an extension needs to be physically and separately installed on the machine
before it can be installed in their database; so maybe using "installed" here
isn't entirely helpful at all. That being said I'm at a loss for a more
suitable word, "available" perhaps?Maybe a larger break with the "This means the extension something something"
formulation, and more on the lines ofHINT: an extension must first be present (for example, installed with a
package manager) on the system where PostgreSQL is running.I like this idea. I can do it this way in the next revision if others
agree.
I think taking it in this direction has merits.
--
Daniel Gustafsson https://vmware.com/
Daniel Gustafsson <daniel@yesql.se> writes:
On 29 Nov 2021, at 22:47, Bossart, Nathan <bossartn@amazon.com> wrote:
On 11/29/21, 1:38 PM, "Chapman Flack" <chap@anastigmatix.net> wrote:Maybe a larger break with the "This means the extension something something"
formulation, and more on the lines of
HINT: an extension must first be present (for example, installed with a
package manager) on the system where PostgreSQL is running.
I like this idea. I can do it this way in the next revision if others
agree.
I think taking it in this direction has merits.
I think "The extension must ..." would read better, otherwise +1.
I don't especially like intertwining the hint choice with the existing
special case for per-version files. Our usual style for conditional
hints can be found in places like sysv_shmem.c, and following that
would lead to a patch roughly like
if ((file = AllocateFile(filename, "r")) == NULL)
{
+ int ext_errno = errno;
+
if (version && errno == ENOENT)
{
/* no auxiliary file for this version */
pfree(filename);
return;
}
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not open extension control file \"%s\": %m",
- filename)));
+ filename),
+ (ext_errno == ENOENT) ? errhint("...") : 0));
}regards, tom lane
On 11/29/21, 2:04 PM, "Tom Lane" <tgl@sss.pgh.pa.us> wrote:
I think "The extension must ..." would read better, otherwise +1.
I don't especially like intertwining the hint choice with the existing
special case for per-version files. Our usual style for conditional
hints can be found in places like sysv_shmem.c, and following that
would lead to a patch roughly like
Alright, here's v3. In this version, I actually removed the message
about the control file entirely, so now the error message looks like
this:
postgres=# CREATE EXTENSION does_not_exist;
ERROR: extension "does_not_exist" is not available
DETAIL: The extension must first be installed on the system where PostgreSQL is running.
HINT: The pg_available_extensions view lists the extensions that are available for installation.
I can add the control file part back if we think it's necessary.
Nathan
Attachments:
v3-0001-Improve-CREATE-EXTENSION-error-message.patchapplication/octet-stream; name=v3-0001-Improve-CREATE-EXTENSION-error-message.patchDownload
From f9c149b1031769516702826c6529e86159a2149a Mon Sep 17 00:00:00 2001
From: Nathan Bossart <bossartn@amazon.com>
Date: Mon, 29 Nov 2021 22:06:27 +0000
Subject: [PATCH v3 1/1] Improve CREATE EXTENSION error message.
---
src/backend/commands/extension.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index eaa76af47b..d7713a5b3f 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -487,11 +487,21 @@ parse_extension_control_file(ExtensionControlFile *control,
if ((file = AllocateFile(filename, "r")) == NULL)
{
- if (version && errno == ENOENT)
+ if (errno == ENOENT)
{
- /* no auxiliary file for this version */
- pfree(filename);
- return;
+ /* auxiliary files are optional */
+ if (version)
+ {
+ pfree(filename);
+ return;
+ }
+
+ /* missing control file indicates extension is not installed */
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("extension \"%s\" is not available", control->name),
+ errdetail("The extension must first be installed on the system where PostgreSQL is running."),
+ errhint("The pg_available_extensions view lists the extensions that are available for installation.")));
}
ereport(ERROR,
(errcode_for_file_access(),
--
2.16.6
On 11/29/21, 2:13 PM, "Bossart, Nathan" <bossartn@amazon.com> wrote:
Alright, here's v3. In this version, I actually removed the message
about the control file entirely, so now the error message looks like
this:postgres=# CREATE EXTENSION does_not_exist;
ERROR: extension "does_not_exist" is not available
DETAIL: The extension must first be installed on the system where PostgreSQL is running.
HINT: The pg_available_extensions view lists the extensions that are available for installation.I can add the control file part back if we think it's necessary.
Hm. I should probably adjust the hint to avoid confusion from
"installed on the system" and "available for installation." Maybe
something like
The pg_available_extensions view lists the available extensions.
Nathan
On 11/29/21 17:03, Tom Lane wrote:
I think "The extension must ..." would read better, otherwise +1.
I'm not strongly invested either way, but I'll see if I can get at why
I used 'an' ...
Hints are hinty. We can give them, and they're helpful, because there
are certain situations that we know are very likely to be what's behind
certain errors. ENOENT on the control file? Yeah, probably means the
extension needs to be installed. In somebody's specific case, though,
it could mean most of the extension is there but the other sysadmin
overnight fat-fingered an rm command and has been spending the morning
idly wondering why the file he /meant/ to remove is still there. Or a bit
flipped in an inode and a directory became a file. (That happened to me on
a production system once; the directory was /usr. That'll mess stuff up.)
So, in my view, a hint doesn't need to sound omniscient, or as if it
somehow knows precisely what happened in your case. It's enough (maybe
better, even?) if a hint reads like a hint, a general statement that
you may ponder for a moment and then think "yeah, that sounds like it's
probably what I needed to know."
Regards,
-Chap
On 11/29/21 17:13, Bossart, Nathan wrote:
postgres=# CREATE EXTENSION does_not_exist;
ERROR: extension "does_not_exist" is not available
DETAIL: The extension must first be installed on the system where PostgreSQL is running.
HINT: The pg_available_extensions view lists the extensions that are available for installation.
Messages crossed ...
If it were me, I would combine that DETAIL and HINT as one larger HINT,
and use DETAIL for specific details about what actually happened (such
as the exact filename sought and the %m).
The need for those details doesn't go away; they're still what you need
when what went wrong is some other freak occurrence the hint doesn't
explain.
Regards,
-Chap
"Bossart, Nathan" <bossartn@amazon.com> writes:
Alright, here's v3. In this version, I actually removed the message
about the control file entirely, so now the error message looks like
this:
postgres=# CREATE EXTENSION does_not_exist;
ERROR: extension "does_not_exist" is not available
DETAIL: The extension must first be installed on the system where PostgreSQL is running.
HINT: The pg_available_extensions view lists the extensions that are available for installation.
I don't think that HINT is useful at all, and I agree with Chapman
that we should still show the filename we tried to look up,
just in case there's a path problem or the like.
regards, tom lane
On 11/29/21, 2:32 PM, "Chapman Flack" <chap@anastigmatix.net> wrote:
If it were me, I would combine that DETAIL and HINT as one larger HINT,
and use DETAIL for specific details about what actually happened (such
as the exact filename sought and the %m).The need for those details doesn't go away; they're still what you need
when what went wrong is some other freak occurrence the hint doesn't
explain.
How's this?
postgres=# CREATE EXTENSION does_not_exist;
ERROR: extension "does_not_exist" is not available
DETAIL: Extension control file "/usr/local/pgsql/share/extension/does_not_exist.control" does not exist.
HINT: The extension must first be installed on the system where PostgreSQL is running. The pg_available_extensions view lists the available extensions.
Nathan
On 11/29/21, 2:43 PM, "Tom Lane" <tgl@sss.pgh.pa.us> wrote:
"Bossart, Nathan" <bossartn@amazon.com> writes:
postgres=# CREATE EXTENSION does_not_exist;
ERROR: extension "does_not_exist" is not available
DETAIL: The extension must first be installed on the system where PostgreSQL is running.
HINT: The pg_available_extensions view lists the extensions that are available for installation.I don't think that HINT is useful at all, and I agree with Chapman
that we should still show the filename we tried to look up,
just in case there's a path problem or the like.
Okay, I removed the part about pg_available_extensions and now the
message looks like this:
postgres=# CREATE EXTENSION does_not_exist;
ERROR: extension "does_not_exist" is not available
DETAIL: Extension control file "/usr/local/pgsql/share/extension/does_not_exist.control" does not exist.
HINT: The extension must first be installed on the system where PostgreSQL is running.
Nathan
Attachments:
v4-0001-Improve-CREATE-EXTENSION-error-message.patchapplication/octet-stream; name=v4-0001-Improve-CREATE-EXTENSION-error-message.patchDownload
From 1d39e0c304f4f1c83fc02cf6f333928a73559591 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <bossartn@amazon.com>
Date: Mon, 29 Nov 2021 22:46:59 +0000
Subject: [PATCH v4 1/1] Improve CREATE EXTENSION error message.
---
src/backend/commands/extension.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index eaa76af47b..e4b739143a 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -487,11 +487,23 @@ parse_extension_control_file(ExtensionControlFile *control,
if ((file = AllocateFile(filename, "r")) == NULL)
{
- if (version && errno == ENOENT)
+ if (errno == ENOENT)
{
- /* no auxiliary file for this version */
- pfree(filename);
- return;
+ /* auxiliary files are optional */
+ if (version)
+ {
+ pfree(filename);
+ return;
+ }
+
+ /* missing control file indicates extension is not installed */
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("extension \"%s\" is not available", control->name),
+ errdetail("Extension control file \"%s\" does not exist.",
+ filename),
+ errhint("The extension must first be installed on the "
+ "system where PostgreSQL is running.")));
}
ereport(ERROR,
(errcode_for_file_access(),
--
2.16.6
On 11/29/21 17:54, Bossart, Nathan wrote:
postgres=# CREATE EXTENSION does_not_exist;
ERROR: extension "does_not_exist" is not available
DETAIL: Extension control file "/usr/local/pgsql/share/extension/does_not_exist.control" does not exist.
HINT: The extension must first be installed on the system where PostgreSQL is running.
That looks like the direction I would have gone with it.
I wonder, though, is it better to write "does not exist." in the message,
or to use %m and get the exact message from the OS (which presumably would
be "No such file or directory" on Unix, and whatever Windows says for such
things on Windows).
My leaning is generally to use %m and therefore the exact OS message
in the detail, but I don't claim to speak for the project style on that.
Regards,
-Chap
On 11/29/21, 3:47 PM, "Chapman Flack" <chap@anastigmatix.net> wrote:
My leaning is generally to use %m and therefore the exact OS message
in the detail, but I don't claim to speak for the project style on that.
Okay, the message looks like this in v5:
postgres=# CREATE EXTENSION does_not_exist;
ERROR: extension "does_not_exist" is not available
DETAIL: Could not open extension control file "/usr/local/pgsql/share/extension/does_not_exist.control": No such file or directory.
HINT: The extension must first be installed on the system where PostgreSQL is running.
Nathan
Attachments:
v5-0001-Improve-CREATE-EXTENSION-error-message.patchapplication/octet-stream; name=v5-0001-Improve-CREATE-EXTENSION-error-message.patchDownload
From 18194c7be0e374eed72de254cd25e8ba99f53c8b Mon Sep 17 00:00:00 2001
From: Nathan Bossart <bossartn@amazon.com>
Date: Tue, 30 Nov 2021 00:10:39 +0000
Subject: [PATCH v5 1/1] Improve CREATE EXTENSION error message.
---
src/backend/commands/extension.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index eaa76af47b..08e5abb57d 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -487,11 +487,23 @@ parse_extension_control_file(ExtensionControlFile *control,
if ((file = AllocateFile(filename, "r")) == NULL)
{
- if (version && errno == ENOENT)
+ if (errno == ENOENT)
{
- /* no auxiliary file for this version */
- pfree(filename);
- return;
+ /* auxiliary files are optional */
+ if (version)
+ {
+ pfree(filename);
+ return;
+ }
+
+ /* missing control file indicates extension is not installed */
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("extension \"%s\" is not available", control->name),
+ errdetail("Could not open extension control file \"%s\": %m.",
+ filename),
+ errhint("The extension must first be installed on the "
+ "system where PostgreSQL is running.")));
}
ereport(ERROR,
(errcode_for_file_access(),
--
2.16.6
"Bossart, Nathan" <bossartn@amazon.com> writes:
Okay, the message looks like this in v5:
postgres=# CREATE EXTENSION does_not_exist;
ERROR: extension "does_not_exist" is not available
DETAIL: Could not open extension control file "/usr/local/pgsql/share/extension/does_not_exist.control": No such file or directory.
HINT: The extension must first be installed on the system where PostgreSQL is running.
Nobody complained about that wording, so pushed.
regards, tom lane
On 1/11/22, 11:23 AM, "Tom Lane" <tgl@sss.pgh.pa.us> wrote:
"Bossart, Nathan" <bossartn@amazon.com> writes:
Okay, the message looks like this in v5:
postgres=# CREATE EXTENSION does_not_exist;
ERROR: extension "does_not_exist" is not available
DETAIL: Could not open extension control file "/usr/local/pgsql/share/extension/does_not_exist.control": No such file or directory.
HINT: The extension must first be installed on the system where PostgreSQL is running.Nobody complained about that wording, so pushed.
Thanks!
Nathan