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

mysqldump backups broken with #3408 #3577

Open
rPraml opened this issue Feb 26, 2025 · 9 comments · May be fixed by #3578
Open

mysqldump backups broken with #3408 #3577

rPraml opened this issue Feb 26, 2025 · 9 comments · May be fixed by #3578

Comments

@rPraml
Copy link
Contributor

rPraml commented Feb 26, 2025

@rbygrave @mkurz The change in #3408 breaks backups, that are made with "mysqldump" / "mariadbdump"
(We use mysqldump --hex-blob --routines --single-transaction --skip-extended-insert --complete-insert --no-autocommit)

When backing up a DB, "mysqldump" creates the following output:

DELIMITER ;;
CREATE DEFINER=`user`@`%` PROCEDURE `usp_ebean_drop_column`(IN p_table_name VARCHAR(255), IN p_column_name VARCHAR(255))
BEGIN
CALL usp_ebean_drop_foreign_keys(p_table_name, p_column_name);
SET @sql = CONCAT('ALTER TABLE `', p_table_name, '` DROP COLUMN `', p_column_name, '`');
PREPARE stmt FROM @sql;
EXECUTE stmt;
END
-- play-ebean-end ;;
DELIMITER ;

Before, the penultimate line was END;;. Now it is -- play-ebean-end ;; and the backup cannot be restored, because the delimiter is behind the comment. You may have to replace -- play-ebean-end ;; by ;; in these backups.

I think this is a bug in mysqldump, but I still wanted to mention that we have a problem here now.

an easy workaround could be, that after -- play-ebean-end a newline is added or that it is ignored by the ebean migration-runner - but we have not verified that, yet.

@mkurz
Copy link
Contributor

mkurz commented Feb 26, 2025

So we are talking about these lines here:

delimiter $$
--
-- PROCEDURE: usp_ebean_drop_column TABLE, COLUMN
-- deletes the column and ensures that all indices and constraints are dropped first
--
CREATE PROCEDURE usp_ebean_drop_column(IN p_table_name VARCHAR(255), IN p_column_name VARCHAR(255))
-- play-ebean-start
BEGIN
CALL usp_ebean_drop_foreign_keys(p_table_name, p_column_name);
SET @sql = CONCAT('ALTER TABLE `', p_table_name, '` DROP COLUMN `', p_column_name, '`');
PREPARE stmt FROM @sql;
EXECUTE stmt;
END
-- play-ebean-end
$$

So all my PR did was inserting the comment:

END
++-- play-ebean-end
$$

This really looks like a bug in mysqldump. Probably it just goes ahead an adds ;; at the end of the last line, but does not check if that line is a comment.

I agree as workaround might be to add an empty line after the comment and to hope that works 🤞
@rPraml Before we fix that in ebean, can you test if adding that empty line makes mysqldump behave correctly again? You could just copy that procedure, at the new line and submit it into your mysql instance.

@mkurz
Copy link
Contributor

mkurz commented Feb 26, 2025

Also, you probably want to report that to the mysql maintainers.

@rPraml
Copy link
Contributor Author

rPraml commented Feb 26, 2025

Short update:

I agree as workaround might be to add an empty line after the comment and to hope that works 🤞

we did only a short investigation today. Creating a stored procedure in "Dbeaver" when the last line is a comment did also not work. even if we try it to execute with multiple blank lines after the comment.

This really looks like a bug in mysqldump

I agree. But the problem is there now. And our customers may have to wait a long time until they get a fixed version.

Also, you probably want to report that to the mysql maintainers.

NO, I will not report that to mysql maintainers. But I'll report it to mariadb maintainers in the next few days ;)

@focdanisch
Copy link
Contributor

Unfortunately, a new line does not seem to affect the outcome. I also tested it with a whitespace in the new line, same result (all my testing takes place in the official MariaDB-Docker-Container mariadb:10.6, on a database created using CREATE DATABASE testdb CHARACTER SET utf8mb4 COLLATE utf8mb4_bin;)

Starting the application with this single (modified) script (I__create_procs.sql) (with a single whitespace in the new lines before '$$')

-- Inital script to create stored procedures etc for mysql platform
DROP PROCEDURE IF EXISTS usp_ebean_drop_foreign_keys;

delimiter $$
--
-- PROCEDURE: usp_ebean_drop_foreign_keys TABLE, COLUMN
-- deletes all constraints and foreign keys referring to TABLE.COLUMN
--
CREATE PROCEDURE usp_ebean_drop_foreign_keys(IN p_table_name VARCHAR(255), IN p_column_name VARCHAR(255))
-- play-ebean-start
BEGIN
DECLARE done INT DEFAULT FALSE;
DECLARE c_fk_name CHAR(255);
DECLARE curs CURSOR FOR SELECT CONSTRAINT_NAME from information_schema.KEY_COLUMN_USAGE
WHERE TABLE_SCHEMA = DATABASE() and TABLE_NAME = p_table_name and COLUMN_NAME = p_column_name
AND REFERENCED_TABLE_NAME IS NOT NULL;
DECLARE CONTINUE HANDLER FOR NOT FOUND SET done = TRUE;

OPEN curs;

read_loop: LOOP
FETCH curs INTO c_fk_name;
IF done THEN
LEAVE read_loop;
END IF;
SET @sql = CONCAT('ALTER TABLE `', p_table_name, '` DROP FOREIGN KEY ', c_fk_name);
PREPARE stmt FROM @sql;
EXECUTE stmt;
END LOOP;

CLOSE curs;
END
-- play-ebean-end

$$

DROP PROCEDURE IF EXISTS usp_ebean_drop_column;

delimiter $$
--
-- PROCEDURE: usp_ebean_drop_column TABLE, COLUMN
-- deletes the column and ensures that all indices and constraints are dropped first
--
CREATE PROCEDURE usp_ebean_drop_column(IN p_table_name VARCHAR(255), IN p_column_name VARCHAR(255))
-- play-ebean-start
BEGIN
CALL usp_ebean_drop_foreign_keys(p_table_name, p_column_name);
SET @sql = CONCAT('ALTER TABLE `', p_table_name, '` DROP COLUMN `', p_column_name, '`');
PREPARE stmt FROM @sql;
EXECUTE stmt;
END
-- play-ebean-end

$$

lead to this dump (docker compose exec -T db mysqldump --hex-blob --routines --single-transaction --skip-extended-insert --complete-insert --no-autocommit -p testdb)

/*M!999999\- enable the sandbox mode */
-- MariaDB dump 10.19  Distrib 10.6.21-MariaDB, for debian-linux-gnu (x86_64)
--
-- Host: localhost    Database: testdb
-- ------------------------------------------------------
-- Server version       10.6.21-MariaDB-ubu2004

/*!40101 SET @OLD_CHARACTER_SET_CLIENT=@@CHARACTER_SET_CLIENT */;
/*!40101 SET @OLD_CHARACTER_SET_RESULTS=@@CHARACTER_SET_RESULTS */;
/*!40101 SET @OLD_COLLATION_CONNECTION=@@COLLATION_CONNECTION */;
/*!40101 SET NAMES utf8mb4 */;
/*!40103 SET @OLD_TIME_ZONE=@@TIME_ZONE */;
/*!40103 SET TIME_ZONE='+00:00' */;
/*!40014 SET @OLD_UNIQUE_CHECKS=@@UNIQUE_CHECKS, UNIQUE_CHECKS=0 */;
/*!40014 SET @OLD_FOREIGN_KEY_CHECKS=@@FOREIGN_KEY_CHECKS, FOREIGN_KEY_CHECKS=0 */;
/*!40101 SET @OLD_SQL_MODE=@@SQL_MODE, SQL_MODE='NO_AUTO_VALUE_ON_ZERO' */;
/*!40111 SET @OLD_SQL_NOTES=@@SQL_NOTES, SQL_NOTES=0 */;

--
-- Table structure for table `db_migration`
--

DROP TABLE IF EXISTS `db_migration`;
/*!40101 SET @saved_cs_client     = @@character_set_client */;
/*!40101 SET character_set_client = utf8mb4 */;
CREATE TABLE `db_migration` (
  `id` int(11) NOT NULL,
  `mchecksum` int(11) NOT NULL,
  `mtype` varchar(1) NOT NULL,
  `mversion` varchar(150) NOT NULL,
  `mcomment` varchar(150) NOT NULL,
  `mstatus` varchar(10) NOT NULL,
  `run_on` timestamp NOT NULL DEFAULT current_timestamp() ON UPDATE current_timestamp(),
  `run_by` varchar(30) NOT NULL,
  `run_time` int(11) NOT NULL,
  PRIMARY KEY (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin;
/*!40101 SET character_set_client = @saved_cs_client */;

--
-- Dumping data for table `db_migration`
--

LOCK TABLES `db_migration` WRITE;
/*!40000 ALTER TABLE `db_migration` DISABLE KEYS */;
set autocommit=0;
INSERT INTO `db_migration` (`id`, `mchecksum`, `mtype`, `mversion`, `mcomment`, `mstatus`, `run_on`, `run_by`, `run_time`) VALUES (0,1,'I','0','<init>','SUCCESS','2025-02-26 13:59:24','www-data',0);
/*!40000 ALTER TABLE `db_migration` ENABLE KEYS */;
UNLOCK TABLES;
commit;

--
-- Dumping routines for database 'testdb'
--
/*!50003 SET @saved_sql_mode       = @@sql_mode */ ;
/*!50003 SET sql_mode              = 'IGNORE_SPACE,STRICT_TRANS_TABLES,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION' */ ;
/*!50003 DROP PROCEDURE IF EXISTS `usp_ebean_drop_column` */;
/*!50003 SET @saved_cs_client      = @@character_set_client */ ;
/*!50003 SET @saved_cs_results     = @@character_set_results */ ;
/*!50003 SET @saved_col_connection = @@collation_connection */ ;
/*!50003 SET character_set_client  = utf8mb4 */ ;
/*!50003 SET character_set_results = utf8mb4 */ ;
/*!50003 SET collation_connection  = utf8mb4_bin */ ;
DELIMITER ;;
CREATE DEFINER=`root`@`%` PROCEDURE `usp_ebean_drop_column`(IN p_table_name VARCHAR(255), IN p_column_name VARCHAR(255))
BEGIN
CALL usp_ebean_drop_foreign_keys(p_table_name, p_column_name);
SET @sql = CONCAT('ALTER TABLE `', p_table_name, '` DROP COLUMN `', p_column_name, '`');
PREPARE stmt FROM @sql;
EXECUTE stmt;
END
-- play-ebean-end ;;
DELIMITER ;
/*!50003 SET sql_mode              = @saved_sql_mode */ ;
/*!50003 SET character_set_client  = @saved_cs_client */ ;
/*!50003 SET character_set_results = @saved_cs_results */ ;
/*!50003 SET collation_connection  = @saved_col_connection */ ;
/*!50003 SET @saved_sql_mode       = @@sql_mode */ ;
/*!50003 SET sql_mode              = 'IGNORE_SPACE,STRICT_TRANS_TABLES,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION' */ ;
/*!50003 DROP PROCEDURE IF EXISTS `usp_ebean_drop_foreign_keys` */;
/*!50003 SET @saved_cs_client      = @@character_set_client */ ;
/*!50003 SET @saved_cs_results     = @@character_set_results */ ;
/*!50003 SET @saved_col_connection = @@collation_connection */ ;
/*!50003 SET character_set_client  = utf8mb4 */ ;
/*!50003 SET character_set_results = utf8mb4 */ ;
/*!50003 SET collation_connection  = utf8mb4_bin */ ;
DELIMITER ;;
CREATE DEFINER=`root`@`%` PROCEDURE `usp_ebean_drop_foreign_keys`(IN p_table_name VARCHAR(255), IN p_column_name VARCHAR(255))
BEGIN
DECLARE done INT DEFAULT FALSE;
DECLARE c_fk_name CHAR(255);
DECLARE curs CURSOR FOR SELECT CONSTRAINT_NAME from information_schema.KEY_COLUMN_USAGE
WHERE TABLE_SCHEMA = DATABASE() and TABLE_NAME = p_table_name and COLUMN_NAME = p_column_name
AND REFERENCED_TABLE_NAME IS NOT NULL;
DECLARE CONTINUE HANDLER FOR NOT FOUND SET done = TRUE;

OPEN curs;

read_loop: LOOP
FETCH curs INTO c_fk_name;
IF done THEN
LEAVE read_loop;
END IF;
SET @sql = CONCAT('ALTER TABLE `', p_table_name, '` DROP FOREIGN KEY ', c_fk_name);
PREPARE stmt FROM @sql;
EXECUTE stmt;
END LOOP;

CLOSE curs;
END
-- play-ebean-end ;;
DELIMITER ;
/*!50003 SET sql_mode              = @saved_sql_mode */ ;
/*!50003 SET character_set_client  = @saved_cs_client */ ;
/*!50003 SET character_set_results = @saved_cs_results */ ;
/*!50003 SET collation_connection  = @saved_col_connection */ ;
/*!40103 SET TIME_ZONE=@OLD_TIME_ZONE */;

/*!40101 SET SQL_MODE=@OLD_SQL_MODE */;
/*!40014 SET FOREIGN_KEY_CHECKS=@OLD_FOREIGN_KEY_CHECKS */;
/*!40014 SET UNIQUE_CHECKS=@OLD_UNIQUE_CHECKS */;
/*!40101 SET CHARACTER_SET_CLIENT=@OLD_CHARACTER_SET_CLIENT */;
/*!40101 SET CHARACTER_SET_RESULTS=@OLD_CHARACTER_SET_RESULTS */;
/*!40101 SET COLLATION_CONNECTION=@OLD_COLLATION_CONNECTION */;
/*!40111 SET SQL_NOTES=@OLD_SQL_NOTES */;

-- Dump completed on 2025-02-26 14:00:02

So, the delimiters are still in the same lines as the comments, preventing the dump from being successfully restored.

@mkurz
Copy link
Contributor

mkurz commented Feb 26, 2025

I think to work around this mariadb bug it's good enough if we move the comment one line up, before the END

@mkurz
Copy link
Contributor

mkurz commented Feb 26, 2025

Fix is here:

@rPraml @focdanisch Can you take a look, but based on what we know this should fix it.

@focdanisch
Copy link
Contributor

focdanisch commented Feb 26, 2025

This looks better:

DROP PROCEDURE IF EXISTS usp_ebean_drop_column;

delimiter $$
--sql
-- PROCEDURE: usp_ebean_drop_column TABLE, COLUMN
-- deletes the column and ensures that all indices and constraints are dropped first
--
CREATE PROCEDURE usp_ebean_drop_column(IN p_table_name VARCHAR(255), IN p_column_name VARCHAR(255))
-- play-ebean-start
BEGIN
CALL usp_ebean_drop_foreign_keys(p_table_name, p_column_name);
SET @sql = CONCAT('ALTER TABLE `', p_table_name, '` DROP COLUMN `', p_column_name, '`');
PREPARE stmt FROM @sql;
EXECUTE stmt;
-- play-ebean-end
END
$$

leads to

/*!50003 SET @saved_sql_mode       = @@sql_mode */ ;
/*!50003 SET sql_mode              = 'IGNORE_SPACE,STRICT_TRANS_TABLES,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION' */ ;
/*!50003 DROP PROCEDURE IF EXISTS `usp_ebean_drop_column` */;
/*!50003 SET @saved_cs_client      = @@character_set_client */ ;
/*!50003 SET @saved_cs_results     = @@character_set_results */ ;
/*!50003 SET @saved_col_connection = @@collation_connection */ ;
/*!50003 SET character_set_client  = utf8mb4 */ ;
/*!50003 SET character_set_results = utf8mb4 */ ;
/*!50003 SET collation_connection  = utf8mb4_bin */ ;
DELIMITER ;;
CREATE DEFINER=`root`@`%` PROCEDURE `usp_ebean_drop_column`(IN p_table_name VARCHAR(255), IN p_column_name VARCHAR(255))
BEGIN
CALL usp_ebean_drop_foreign_keys(p_table_name, p_column_name);
SET @sql = CONCAT('ALTER TABLE `', p_table_name, '` DROP COLUMN `', p_column_name, '`');
PREPARE stmt FROM @sql;
EXECUTE stmt;
-- play-ebean-end
END ;;
DELIMITER ;

This is still not entirely correct, as the comment should not be in the dump in the first place (but that seems to be the mariadb-bug we are talking about). But I was able to restore the generated dump again with this change.

@mkurz
Copy link
Contributor

mkurz commented Feb 26, 2025

Perfect! Thanks for confirming!

@rPraml
Copy link
Contributor Author

rPraml commented Feb 26, 2025

Issue reported to https://jira.mariadb.org/browse/MDEV-36183

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.

3 participants