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

Check database exist before create #329

Merged
merged 7 commits into from
Nov 9, 2023
Merged

Check database exist before create #329

merged 7 commits into from
Nov 9, 2023

Conversation

fogelito
Copy link
Contributor

@fogelito fogelito commented Oct 3, 2023

No description provided.

@@ -49,11 +49,6 @@ public static function getDatabase(): Database
$database = new Database(new MariaDB($pdo), $cache);
$database->setDefaultDatabase('utopiaTests');
$database->setNamespace('myapp_'.uniqid());

if ($database->exists('utopiaTests')) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I remove this, since it is Done in Base.php Tests

Copy link
Member

Choose a reason for hiding this comment

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

Can you link it? I don't see a call to delete in Base.php

Copy link
Contributor Author

Choose a reason for hiding this comment

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

$this->assertEquals(true, static::getDatabase()->delete($this->testDatabase));

I guess I can revert this to be more consistent for a fresh start, not to depend on this exist method 👍

@abnegate
Copy link
Member

abnegate commented Oct 6, 2023

Do we need this change? Shouldn't it be okay as is with CREATE DATABASE IF NOT EXISTS?

@fogelito
Copy link
Contributor Author

fogelito commented Oct 8, 2023

We had some lock incidents with the create schema, it is triggering a DDL operation,
https://stackoverflow.com/questions/6169263/mysqls-create-database-if-not-exists-misbehaves-when-database-has-live-connec
@stnguyen90 What do you think about it? you saw this live

@fogelito fogelito requested review from abnegate and removed request for abnegate October 11, 2023 09:24
@abnegate
Copy link
Member

We had some lock incidents with the create schema, it is triggering a DDL operation, https://stackoverflow.com/questions/6169263/mysqls-create-database-if-not-exists-misbehaves-when-database-has-live-connec @stnguyen90 What do you think about it? you saw this live

@fogelito I don't think this query was the one that held the lock, but the one that was blocked when trying to acquire it

@stnguyen90
Copy link
Contributor

@fogelito I don't think this query was the one that held the lock, but the one that was blocked when trying to acquire it

@abnegate I think this can make the problem worse because once this query starts, a full database lock is created and can block other queries after.

…abase-check

� Conflicts:
�	src/Database/Adapter/MariaDB.php
�	src/Database/Adapter/SQL.php
if ($this->exists($name)) {
return true;
}

$sql = "CREATE DATABASE IF NOT EXISTS `{$name}` /*!40100 DEFAULT CHARACTER SET utf8mb4 */;";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
$sql = "CREATE DATABASE IF NOT EXISTS `{$name}` /*!40100 DEFAULT CHARACTER SET utf8mb4 */;";
$sql = "CREATE DATABASE `{$name}` /*!40100 DEFAULT CHARACTER SET utf8mb4 */;";

With the new check we don't really need this.

Copy link
Member

Choose a reason for hiding this comment

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

Also, do we have the same in other adapters?

@abnegate abnegate merged commit 0e76f99 into main Nov 9, 2023
3 checks passed
@abnegate abnegate deleted the create-database-check branch November 23, 2023 01:47
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.

4 participants