Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

List databases #50

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

List databases #50

wants to merge 2 commits into from

Conversation

ylavoie
Copy link

@ylavoie ylavoie commented Mar 17, 2022

This PR refines list_dbs to allow optional exclusions from the list of databases.
It also provides list_dbs_this_user to give a list of databases owned by a specific user.

@ylavoie ylavoie requested a review from ehuelsmann March 17, 2022 18:55
lib/PGObject/Util/DBAdmin.pm Outdated Show resolved Hide resolved
my $_query = 'SELECT datname from pg_database ' .
"WHERE datdba=(SELECT usesysid FROM pg_user WHERE usename='$user') " .
'AND datallowconn ' .
( $excludes ? "AND datname NOT IN ('" . $excludes.join("','") . "') " : '' ) .
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same remark about SQL injection.


Returns a list of db names.

When a database name is specified, uses that database to connect to,
using the credentials specified in the instance.

If no database name is specified, 'template1' is used.
If an array of excluded databases is provided, it is used to limit the list
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. But what is the reason we want this? I mean Perl has a nice construct with grep { not $excludes{$_} } @all_databases where you can really simply limit the values in a list. Is there a reason this must happen in the SQL phase?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The latest commit does address the SQL injection, thanks. That leaves the above question (why is it necessary to do this at the query level) though.

@@ -559,26 +559,62 @@ sub server_version {
}


=head2 list_dbs([$dbname])
=head2 list_dbs([$dbname],[$excludes])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implies my $excludes = shift with a list of excludes passed as an arrayref. The implementation below takes all remaining arguments as excludes. I like this definition better than the implementation, because the latter does not allow further extension of the API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants