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

backticks don't work in postgres for database health check #15

Closed
tacman opened this issue Apr 1, 2024 · 6 comments · Fixed by #19
Closed

backticks don't work in postgres for database health check #15

tacman opened this issue Apr 1, 2024 · 6 comments · Fixed by #19

Comments

@tacman
Copy link
Contributor

tacman commented Apr 1, 2024

I think this works in MySQL, but not postgres:

                    // Perform a light-weight query on the chosen table
                    $query = 'SELECT * FROM `%s` LIMIT 1';
                    $this->entityManager->getConnection()->executeQuery(sprintf($query, $table->getName()));

when I remove the backticks, it works as expected for me, running postgres 16.

I thought doctrine had some sort of escape method that was database-specific, but I can't find it now.

FWIW, the table is doctrine_migrations_table

@tacman
Copy link
Contributor Author

tacman commented Apr 3, 2024

I'm new to github actions, but if you're able to, can you add postgres, sqlite and mysql/mariadb to the action?

I don't think it's working for postgres, and now that postgres is the default database when running

symfony new --webapp my-application

it's even more important than this bundle work with it. Thanks.

@thijskh
Copy link
Member

thijskh commented Apr 3, 2024

I agree that it is good to fix it. The question is what a proper fix would be, it's not completely straightforward as you say. Suggestions are welcome.

@tacman
Copy link
Contributor Author

tacman commented Apr 3, 2024

If all we're doing is testing that the database is connected and running, I think

 SELECT 1

is the fastest. It also saves the time of looking for the tables, etc.

@tacman
Copy link
Contributor Author

tacman commented Apr 3, 2024

Alternatively, we could allow the user to define their own query, defaulting to "Select 1". That way, they could check a particular table, for example, or even a certain number of rows, or that all migrations have been executed, etc.

But I'm looking at using this for something that's going to be checking uptime status every few seconds, so would like it to be as fast as possible and with as little overhead as possible.

tacman added a commit to tacman/Monitor-bundle that referenced this issue Apr 3, 2024
@thijskh
Copy link
Member

thijskh commented Apr 4, 2024

With SELECT 1 you know nothing about the database, only about the database server being up. Although that is a large part of the test, we think it's essential to know that the actual database is there and can be read (i.e. no account permission issue - which would not be triggered by a SELECT 1) or database present but completely empty.

If we can configure the query needed then the bundle can satisfy both our needs I think.

@tacman
Copy link
Contributor Author

tacman commented Apr 10, 2024

Can you tweak the bundle so that postgres users can use it?

I like the idea of a configurable query, but all my bundle experience is Symfony 6.1+, which extends AbstractBundle. Much easier to configure.

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 a pull request may close this issue.

2 participants