lando / mysql

The Official MySQL Lando Plugin
https://docs.lando.dev/mysql/
GNU General Public License v3.0
1 stars 2 forks source link

Cannot import database to additional database service since upgrade to v3.21.0 #53

Closed drupov closed 4 months ago

drupov commented 4 months ago

Importing into an additional database service seems to not be possible anymore, as emptying the database in question fails.

I have this Drupal 10 recipe, but it is not related to Drupal IMO:

name: drupal10
recipe: drupal10
config:
  webroot: web
services:
  db2:
    type: mysql
    portforward: true

lando db-import initial.sql.gz works ok. lando db-import initial.sql.gz --host db2 fails though, seemingly when the command tries to empty the database:

{main} ~/apps/drupal10$ lando db-import initial.sql.gz --host db2
Preparing to import /app/initial.sql.gz into database 'database' on service 'db2' as user root...

Emptying database... 
NOTE: See the --no-wipe flag to avoid this step!
ERROR 1064 (42000) at line 1: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'database' at line 1

I can confirm, that lando db-import initial.sql.gz --host db2 --no-wipe works.

drupov commented 4 months ago

Just saw https://github.com/lando/mysql/issues/52

So it seems like the issue is fixed for the database in the main container, but not when you have additional database containers:

Issue

I still have the issue, when I define multiple database containers (should not be Drupal-related as in the example):

name: myproject
recipe: drupal10
config:
  webroot: web
services:
  additionaldb1:
    type: mysql
    portforward: true
  additionaldb2:
    type: mysql
    portforward: true

Now I have this:

~/apps/drupal10$ lando db-import initial.sql.gz --host additionaldb1
Preparing to import /app/initial.sql.gz into database 'database' on service 'additionaldb1' as user root...

Emptying database... 
NOTE: See the --no-wipe flag to avoid this step!
ERROR 1064 (42000) at line 1: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'database' at line 1

Importing into the main database works though:

~/apps/drupal10$ lando db-import initial.sql.gz
Preparing to import /app/initial.sql.gz into database 'drupal10' on service 'database' as user root...

Emptying drupal10... 
NOTE: See the --no-wipe flag to avoid this step!
Gzipped file detected!
Importing /app/initial.sql.gz...
Import complete!

Workaround

The lando drush sql-drop --yes - see https://github.com/lando/mysql/issues/52#issuecomment-2100844694 - will clear only the main database, from the recipe. In this case I use a similar approach to https://github.com/lando/mysql/issues/52#issuecomment-2100844686 as a workaround:

lando ssh --service=aditionaldb1
mysql -u root
drop database `database`;
create database `database`;

and then lando db-import database.sql.gz --no-wipe --host aditionaldb1.

Version

I am on

~/apps/drupal10$ lando version
v3.21.0
reynoldsalec commented 4 months ago

I'll try to replicate this, would you mind sharing your complete Landofile?

Would be good to add a test for this in our recipe tests of the lando db-import command: https://github.com/lando/lamp/tree/main/examples/lamp-import

drupov commented 4 months ago

Hey, @reynoldsalec thanks for jumping in.

The lando file from my comment is complete, you could use that.

The database I am trying to import is one I take after an initial Drupal installation, but any dummy one would suffice, e.g. the one from here.

reynoldsalec commented 4 months ago

Confirmed thaTI'm experiencing the same issue; it looks like recent changes to the sql-import.sh script caused a regression that wasn't covered in the relevant test coverage. Basically ${DATABASE} needs to be quoted in backticks ($SQLSTART -e "DROP DATABASE IF EXISTS \${DATABASE}`"`), see https://github.com/lando/core/blob/main/scripts/sql-import.sh#L111

I'm making a PR @drupov to address this and add test coverage, thanks for putting in the issue!

reynoldsalec commented 4 months ago

...adding to this, I think the reason the script works otherwise is because database is a reserved word in SQL that needs to be enclosed in quotes. If the env var MYSQL_DATABASE isn't set by the service, it defaults to database and creates this issue with the new version of the script. The recipes set different default names for the database and our tests explicitly set the env vars, hence why it didn't pop up.

The earlier version of the sql-import.sh script dropped all the database's tables instead of dropping the DB, hence why it probably wasn't an issue before.

reynoldsalec commented 4 months ago

...and just a note that this won't ACTUALLY be deployed until Lando v3.21.1 is released, which will probably be this Friday...just in case someone see this.

drupov commented 3 months ago

This works now with v3.21.2

Thanks a lot