Added a function to allow creation of database with owner #37

aashikgowda · 2025-11-05T21:58:47Z

Checklist

I have read the Contributing Guide
I have checked to ensure this does not introduce an unintended breaking changes
I have considered appropriate testing for my change

Description

Added a new function in PostgresqlExtensions which takes in an owner name. The owner username defaults to the username in the connection string if there is no owner name specified.
Added a check and throw an exception for owner username if it does not exist
CREATE DATABASE sql is now updated to execute WITH OWNER
Resolves Allow creating PostgreSQL database "with owner" #3
Added missing comments for existing functions to create a database

src/dbup-postgresql/PostgresqlExtensions.cs

Copilot

Pull Request Overview

This PR adds support for specifying a database owner when creating PostgreSQL databases and includes code formatting improvements. The key changes include:

Adding an optional owner parameter to the PostgresqlDatabase method for database creation with ownership assignment
Implementing role validation before database creation
Removing obsolete method overloads from the public API
Applying consistent code formatting (spacing, indentation)

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
src/dbup-postgresql/PostgresqlExtensions.cs	Added new method overload with owner parameter, implemented role existence validation, updated CREATE DATABASE SQL to include owner clause, and applied formatting fixes
src/Tests/ApprovalFiles/NoPublicApiChanges.Run.approved.cs	Updated approved public API surface to reflect removal of deprecated methods and addition of new owner parameter overload

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/dbup-postgresql/PostgresqlExtensions.cs

Copilot · 2025-11-06T04:39:23Z

src/dbup-postgresql/PostgresqlExtensions.cs

+        sqlCommandText = $"select 1 from pg_roles where rolname = \'{owner}\' limit 1;";
+        // check to see if the owner exists..
+        using (var command = new NpgsqlCommand(sqlCommandText, connection)
+        {
+            CommandType = CommandType.Text
+        })
+        {


SQL injection vulnerability: the owner parameter is interpolated directly into the SQL query using single quotes. Use parameterized queries with NpgsqlParameter to prevent SQL injection attacks.

Suggested change

sqlCommandText = $"select 1 from pg_roles where rolname = \'{owner}\' limit 1;";

// check to see if the owner exists..

using (var command = new NpgsqlCommand(sqlCommandText, connection)

{

CommandType = CommandType.Text

})

{

sqlCommandText = "select 1 from pg_roles where rolname = @owner limit 1;";

// check to see if the owner exists..

using (var command = new NpgsqlCommand(sqlCommandText, connection)

{

CommandType = CommandType.Text

})

{

command.Parameters.AddWithValue("@owner", owner);

Well... it's unlikely SQLi would happen this way, but I don't think it hurts to do it this way.

Updated the SQL command to take in parameters instead.

Copilot · 2025-11-06T04:39:23Z

src/dbup-postgresql/PostgresqlExtensions.cs

+            var results = Convert.ToInt32(command.ExecuteScalar());
+
+            // if the owner role does not exist, we throw an exception.
+            if (results == 0)


Potential null reference exception: ExecuteScalar() can return null when no rows are found. The code should handle the null case explicitly before converting to Int32, e.g., var results = command.ExecuteScalar() as int? ?? 0;

This sounds right based on my SQL Server knowledge. If owner does not exist, no rows are returned (since it doesn't use Count(*).

Convert.ToInt32 can take in a null object and return a zero for null objects.

However, to keep it cleaner, I updated the SQL to return a boolean instead which makes it more readable.

Copilot · 2025-11-06T04:39:24Z

src/dbup-postgresql/PostgresqlExtensions.cs

+            }
+        }
+
+        sqlCommandText = $"create database \"{databaseName}\" with owner = \"{owner}\";";


SQL injection vulnerability: both databaseName and owner are directly interpolated into the CREATE DATABASE statement. While PostgreSQL doesn't support parameterized DDL, consider validating these identifiers (e.g., checking they only contain alphanumeric characters and underscores) or using identifier quoting functions to prevent injection.

SQLi via connection string...

Addressed it by formatting using helper functions.

Co-authored-by: Copilot <[email protected]>

droyad

On change that is breaking that I'd like to avoid. Also some Co-pilot suggestions. The SQLi ones are a stretch though.

droyad · 2025-11-06T04:53:58Z

src/dbup-postgresql/PostgresqlExtensions.cs

+        owner = string.IsNullOrWhiteSpace(owner) ? masterConnectionStringBuilder.Username : owner;
+


According to CoPilot

The default owner of the new database is the user executing the CREATE DATABASE command

✅

However

The [user[:password]@] part is optional. If omitted, most PostgreSQL clients will default to using the operating system's current username (the user running the client) for authentication

Confirmed via the docs.

So I think the best approach would be to not do this line and keep the existing statement if owner is null. If it's not null the new check for existance and with owner should be used.

The [user[:password]@] part is optional. If omitted, most PostgreSQL clients will default to using the operating system's current username (the user running the client) for authentication

I missed out on this part. Updated as per your suggestion.

…tabase with or without owner

Aashik Nagadikeri Harish added 3 commits November 5, 2025 13:40

Added function to create postgres database with owner

c824d96

Remonved using statement in NoPublicApiChanges.Run.approved.cs

a686ef3

Fixed test

3375b7e

aashikgowda mentioned this pull request Nov 5, 2025

Allow creating PostgreSQL database "with owner" #3

Open

Merge branch 'main' into main

31d7eac

droyad reviewed Nov 6, 2025

View reviewed changes

src/dbup-postgresql/PostgresqlExtensions.cs Outdated Show resolved Hide resolved

Apply suggestion from @droyad

8a002b2

droyad requested a review from Copilot November 6, 2025 04:37

Copilot AI reviewed Nov 6, 2025

View reviewed changes

Update src/dbup-postgresql/PostgresqlExtensions.cs

daea170

Co-authored-by: Copilot <[email protected]>

droyad requested changes Nov 6, 2025

View reviewed changes

droyad self-assigned this Nov 6, 2025

Aashik Nagadikeri Harish added 3 commits November 6, 2025 11:55

Addressed PR comments around SQLi and using if/else for creating a da…

a9f9ae3

…tabase with or without owner

Flipped boolean logic around

7e17862

formatting

0a58605

		owner = string.IsNullOrWhiteSpace(owner) ? masterConnectionStringBuilder.Username : owner;

Added a function to allow creation of database with owner #37

Are you sure you want to change the base?

Added a function to allow creation of database with owner #37

Conversation

aashikgowda commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Description

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

droyad left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

aashikgowda commented Nov 5, 2025 •

edited

Loading