webyog / sqlyog-community

Webyog provides monitoring and management tools for open source relational databases. We develop easy-to-use MySQL client tools for performance tuning and database management. Webyog's solutions include SQL Diagnostic Manager for MySQL performance optimization and SQLyog for MySQL administration. More than 35,000 companies (including Amazon, IBM, Salesforce, AT&T, eBay, and GE) and 2.5 million users rely on Webyog's solutions to provide valuable insights into their databases. Webyog is an Idera, Inc. company.
https://webyog.com/
GNU General Public License v2.0
2.21k stars 326 forks source link

SQL-mode for triggers should be frozen #2177

Open peterwy opened 6 years ago

peterwy commented 6 years ago

https://dev.mysql.com/doc/refman/5.7/en/create-trigger.html "MySQL stores the sql_mode system variable setting in effect when a trigger is created, and always executes the trigger body with this setting in force, regardless of the current server SQL mode when the trigger begins executing." ("storing" probably in I_S).

This MySQL bug report https://bugs.mysql.com/bug.php?id=90624 seems to indicate that WB/mysqldump executes a SET SQL_MODE statement before creating a trigger. In SQLyog we don't, what means that when copying between databases or when exporting importing the SQL_mode for the trigger may change from source to target.

It would be simple to test like this

CREATE DATABASE ttest; CREATE TABLE ttest (id int); -- no INSERTs are required CREATE TRIGGER .. ON ttest ... -- this can be extremely simple!

Dump with 'mysqldump' and SQLyog and compare the script generated.

peterwy commented 6 years ago

Confirmed! Here is a sample 'mysqldump'

-- MySQL dump 10.13 Distrib 5.7.22, for Win64 (x86_64)

-- Host: localhost Database: ttest


-- Server version 5.7.22-log

/!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 utf8 /; /!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 test

DROP TABLE IF EXISTS test; /!40101 SET @saved_cs_client = @@character_set_client /; /!40101 SET character_set_client = utf8 /; CREATE TABLE test ( id int(11) DEFAULT NULL ) ENGINE=InnoDB DEFAULT CHARSET=latin1; /!40101 SET character_set_client = @saved_cs_client /;

-- -- Dumping data for table test

LOCK TABLES test WRITE; /!40000 ALTER TABLE test DISABLE KEYS /; /!40000 ALTER TABLE test ENABLE KEYS /; UNLOCK TABLES; /!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 = utf8 / ; /!50003 SET character_set_results = utf8 / ; /!50003 SET collation_connection = utf8_general_ci / ; /!50003 SET @saved_sql_mode = @@sql_mode / ; /!50003 SET sql_mode = 'NO_AUTO_CREATE_USER' / ; DELIMITER ;; /!50003 CREATE/ /!50017 DEFINER=root@localhost/ /*!50003 TRIGGER ttest.trg BEFORE INSERT ON ttest.test

FOR EACH ROW BEGIN

SET @a = 'a';

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 2018-04-26 13:16:13

We always strived for the closest possible compability of SQLyog dumps and mysqldump. I blieve that with MySQL up to and including 5.5 there is little difference. But since then MySQL has 'moved'.

We should check MariaDB 10.3 as well!

peterwy commented 6 years ago

Check and MariaDB 10.3 also wraps CREATE TRIGGER in SET SQL_MODE statements,

And both MySQL and MariaDB also manipulates charset and collation in order to create the trigger with same charset and collation as it was created.

To do the same in SQLyog, exports, Schema Sync and 'copy database' all need to do this.

peterwy commented 6 years ago

Similar differences with VIEWs, BTW.

CREATE DATABASE vtest; CREATE TABLE aaa(id INT); CREATE VIEW v1 AS SELECT FROM aaa; CREATE VIEW v2 AS SELECT FROM v1;

.. now compare SQLyog and mysqldump dumps.

DillonSadofsky commented 3 years ago

This needs to be fixed. A SQL dump/sync should not be this opinionated. The 'current' connection SQL_MODE is stored on functions, procs, triggers, etc. And it CAN affect the execution of that stored code. We have been having issues where copying a customer database introduces bugs because the stored procedures now execute in a non-standard SQL_MODE because of that line /!40101 SET @OLD_SQL_MODE=@@SQL_MODE, SQL_MODE='NO_AUTO_VALUE_ON_ZERO' /; NO_AUTO_VALUE_ON_ZERO is a big opinion for all stored procedures to have. It mostly comes up insidiously for stored procs we have that attempt to 'clone' rows in tables. Since this is often done as copying the row to a temporary table, setting the PK to 0, then moving the row back into the main table. Normally, this causes the auto-increment to assign a value, but since yog's database copy/sync operation institutes all procs/functions/triggers with this non-standard opinion, you can have hard to detect and debug errors during runtime (in this case, 0 does not get reassigned, so the second time you attempt to clone, you get a key collision on 0).

SQLYog should not apply this strong of an opinion when dumping stored procedures. I'm guessing this is done so that data can be reimported without any complaints on 0 pk values (which we don't support or want). If so, turn the SQL mode back to the default after data import and before the part of the script where stored procs are created. A proc should inherit the server's configured default sql_mode (which is the connection's sql mode, which is stored in @OLD_SQL_MODE. So I think that should be restored after the data import, before the proc import (or if procs go first, wait to set SQL_MODE until after all procs are created).

This is an old ticket, so I assume its being ignored, but this would be really nice in sqlyog.