search

EnterpriseDB / system_stats

A Postgres extension for exposing system metrics such as CPU, memory and disk information
Other
111 stars 24 forks source link

SQL Error [42710]: ERROR: role "monitor_system_stats" already exists #2

Closed JamesInform closed 4 years ago

JamesInform commented 4 years ago

Hi,

first of all I want to thank you for this great extension. This is something really useful for people using the community edition of PostgreSQL.

Back to the issue: It seems that the monitor_system_stats role is not dropped if you drop the extension itself.

Steps to reproduce:

create extension system_stats;
drop extension system_stats;
create extension system_stats;

You get the error: SQL Error [42710]: ERROR: role "monitor_system_stats" already exists

It's not a big deal. One can simply execute a "drop role monitor_system_stats", but I think the idea is that drop extension should do this kind of "garbage collection".

But If you want to use the extension in more than one database in the same PostgreSQL cluster, then this is not possible, because you get this error as soon as you want to create the extension a second time.

So the best solutions from my point of view would be, if "Create extension" checked for an already existing role "monitor_system_stats" and skipped the creation of the role if so.

Cheers James

neel5481 commented 4 years ago

@JamesInform Thanks for reporting an issue. yes, we can check for role during "CREATE EXTENSION" if user want to create the extension on another database of same server so that "CREATE EXTENSION" should not fail. But I have a question. What if user want to drop the extension on one database and should we drop the role as well ? As on another database the extension is still valid.

@dpage What is your thoughts on the same ?

JamesInform commented 4 years ago

While dropping the extension you can try to drop the role monitor_system_stats as well WITHOUT cascading and put that code in an exception handler.

If the extension is also used in a different database you will get an error like

SQL Error [2BP01]: ERROR: role "monitor_system_stats" cannot be dropped because some objects depend on it Detail: 10 objects in database ...

that will just be ignored by the exception handler.

So, if the dropped extension is the last one in the whole PostgreSQL cluster, the role will also be dropped.

neel5481 commented 4 years ago

@JamesInform yes, that can be done but here there is no call for "uninstall.sql" or unpackaged*.sql file during dropping extension. I think, postgres internally dropped those functions but not the role while dropping the extension.

We can add the check not to create the role if already exists in "system_stats--1.0.sql" file, that make sense but i don't think we can remove the role and at the same time we can add in README for removal of role by the user.

Let us know in case of anything we are missing here.

JamesInform commented 4 years ago

Yes, I think checking the role's existance during CREATE EXTENSION and just leaving the role untouched during DROP EXTENSION is currently the best possible solution.

Having this information in the README file would be fine.

Thanks for your effort!