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

fix: remove the database name removal on the builder #50

Merged
merged 3 commits into from
Jun 13, 2024

Conversation

parmcoder
Copy link
Contributor

@parmcoder parmcoder commented May 28, 2024

Hi owner! @max-ieremenko

I somehow found your package interesting, so I installed this package to manage my cluster.

However, I found that you removed the database name from the builder and that made me unable to connect to the database I proposed.

I don't have a test, so please add the test for me if you need one.

Please review my code and release the new version.

Thanks.

@max-ieremenko
Copy link
Owner

Hi @parmcoder

I'd like to discuss your case first. The change breaks new database creation:

  • there is no MyDatabase
  • run $ SqlDatabase create "-database=Host=server;Username=postgres;Password=qwerty;Database=MyDatabase" -from=02.create.sql

... unable to connect to the database I proposed.

please share your use-case

@parmcoder
Copy link
Contributor Author

parmcoder commented Jun 6, 2024

Hi @parmcoder

I'd like to discuss your case first. The change breaks new database creation:

  • there is no MyDatabase
  • run $ SqlDatabase create "-database=Host=server;Username=postgres;Password=qwerty;Database=MyDatabase" -from=02.create.sql

... unable to connect to the database I proposed.

Please share your use case

Of course, it should break. You were logging into the Postgres maintainer default database (aka Postgres). That's why you can create a database. Now, you should not be able to do that.

Currently, I cannot use your command to do anything with custom users except with Postgres user.

Use-case

Somehow, I want to be able to run some scripts as a non-superuser to check if they can do something silly.
Not sure if I did that correctly, I wrote something like SqlDatabase -database='Host=localhost;Username=myuser;Password=qwerty; Database=MyDatabase' -fromSql='SQL' [command].

Problem

I technically wanna quickly create a database from a new cluster and run some scripts using myuser, but it will instead log me into the myuser database.
That database exists because I used your script to create it at first. Then, when I try to create new databases using another user, it breaks.

@max-ieremenko
Copy link
Owner

max-ieremenko commented Jun 8, 2024

Hi @parmcoder

I created issue #53 and the corresponding branch https://github.com/max-ieremenko/SqlDatabase/tree/feature/53.
May I ask you to redirect your PR into the branch feature/53?

To make work the existing and your use case:

// builder.Database = null;
builder.Database = "postgres"
_connectionStringMaster = builder.ToString();

The fix will be released in version 4.2.2

@parmcoder parmcoder closed this Jun 8, 2024
@parmcoder parmcoder reopened this Jun 8, 2024
@parmcoder parmcoder changed the base branch from master to feature/53 June 8, 2024 15:50
@parmcoder
Copy link
Contributor Author

Thanks, I changed the base to merge.

@parmcoder
Copy link
Contributor Author

@max-ieremenko please review my code again. I think it should be corrected now.

@max-ieremenko max-ieremenko merged commit 66b1172 into max-ieremenko:feature/53 Jun 13, 2024
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