diesel-rs / diesel

A safe, extensible ORM and Query Builder for Rust
https://diesel.rs
Apache License 2.0
12.79k stars 1.08k forks source link

`diesel database reset` errors with `Failed to execute a database query` #4275

Closed musjj closed 1 week ago

musjj commented 2 months ago

Setup

Versions

Feature Flags

Problem Description

What are you trying to accomplish?

Run diesel database reset.

What is the expected output?

Database is successfully reset.

What is the actual output?

Dropping database: postgres
Failed to execute a database query: cannot drop the currently open database

Are you seeing any additional errors?

No.

Steps to reproduce

.env

DATABASE_URL="postgresql://postgres:postgres@localhost:5432/postgres"

diesel.toml

[print_schema]
file = "src/schema.rs"
custom_type_derives = ["diesel::query_builder::QueryId", "Clone"]

[migrations_directory]
dir = "migrations"

I only have one migration file:

up.sql

CREATE TABLE users (
    id UUID PRIMARY KEY DEFAULT gen_random_uuid(),
    name TEXT NOT NULL,
);

down.sql

DROP TABLE IF EXISTS users;

Checklist

weiznich commented 2 months ago

Given that we have tests for this functionality and that these tests pass in the CI this seems to be a problem with your environment. Are you sure that your user is allowed to drop the database? Did you double check that nothing else is currently connected to the database?

Closed as this seems to be an setup issue.

musjj commented 2 months ago

The issue was that I was using the default postgres database. It looks like that dropping a database requires you to connect to a different database, and postgres is usually chosen for this.

https://www.postgresql.org/docs/current/app-dropdb.html

If not specified, the postgres database will be used; if that does not exist (or is the database being dropped), template1 will be used

Not sure why template1 wasn't automatically used though.

weiznich commented 2 months ago

Thanks for clarifying. Yes it seems like using the Postgres database is the problematic part here. We explicitly connect to that database to drop database, so it seems to be sensible to get that error message there. It might be useful to check for this case and connect to template1 instead if the user tries to drop the Postgres database. The relevant code is here: https://github.com/diesel-rs/diesel/blob/master/diesel_cli/src/database.rs#L295-L296

zaira-bibi commented 1 month ago

I'd like to work on this please.

weiznich commented 1 month ago

That would be great :+1:

zaira-bibi commented 1 month ago

@weiznich I've started working on this issue, and I think it would be better to check for the database inside the change_database_of_url method. What do you think?