search

techouse / mysql-to-sqlite3

Transfer data from MySQL to SQLite
https://techouse.github.io/mysql-to-sqlite3/
MIT License
217 stars 31 forks source link

Default column value not getting converted from mysql #16

Closed jmordica closed 3 years ago

jmordica commented 3 years ago

Describe the bug The default column values don't seem to get converted to sqlite. Here's the command that was run to generate the output:

mysql2sqlite -f mydb.db -d mydb -u myuser --mysql-password mypwd -t dispatcher

Here's the actual table schema in mysql 5.7:

CREATE TABLE `dispatcher` (
  `id` int(10) unsigned NOT NULL AUTO_INCREMENT,
  `setid` int(11) NOT NULL DEFAULT '0',
  `destination` varchar(192) NOT NULL DEFAULT '',
  `flags` int(11) NOT NULL DEFAULT '0',
  `priority` int(11) NOT NULL DEFAULT '0',
  `attrs` varchar(128) NOT NULL DEFAULT '',
  `description` varchar(64) NOT NULL DEFAULT '',
  PRIMARY KEY (`id`),
  UNIQUE KEY `description` (`description`) USING BTREE
) ENGINE=InnoDB AUTO_INCREMENT=595 DEFAULT CHARSET=latin1;

Expected behaviour

sqlite> .schema dispatcher
CREATE TABLE "dispatcher" (
    "id" INTEGER NOT NULL,
    "setid" INTEGER NOT NULL DEFAULT '0',
    "destination" VARCHAR(192) NOT NULL DEFAULT '',
    "flags" INTEGER NOT NULL DEFAULT '0',
    "priority" INTEGER NOT NULL DEFAULT '0',
    "attrs" VARCHAR(128) NOT NULL DEFAULT '',
    "description" VARCHAR(64) NOT NULL DEFAULT '',
    PRIMARY KEY ("id")
);
CREATE UNIQUE INDEX "dispatcher_description" ON "dispatcher" ("description");

Actual result

sqlite> .schema dispatcher
CREATE TABLE "dispatcher" (
    "id" INTEGER NOT NULL,
    "setid" INTEGER NOT NULL,
    "destination" VARCHAR(192) NOT NULL,
    "flags" INTEGER NOT NULL,
    "priority" INTEGER NOT NULL,
    "attrs" VARCHAR(128) NOT NULL,
    "description" VARCHAR(64) NOT NULL,
    PRIMARY KEY ("id")
);
CREATE UNIQUE INDEX "dispatcher_description" ON "dispatcher" ("description");

System Information

$ mysql2sqlite --version
software version
mysql-to-sqlite3 1.4.0
Operating System Linux 3.10.0-1160.2.2.el7.x86_64
Python CPython 2.7.5
MySQL mysql Ver 14.14 Distrib 5.7.32, for Linux (x86_64) using EditLine wrapper
SQLite 3.7.17
click 7.1.2
mysql-connector-python 8.0.23
python-slugify 5.0.0
pytimeparse 1.1.8
simplejson 3.17.2
six 1.9.0
tabulate 0.8.9
tqdm 4.61.1

BTW, thanks for this tool. It seems to work very well. I've tried others and they all failed to produce a production ready sqlite export but this one seems great :)

techouse commented 3 years ago

Thanx for pointing this out. It wasn't really a bug it was more a case of me completely forgetting about implementing this 🤣

jmordica commented 3 years ago

Wow quick fix! Really appreciate this!

jmordica commented 3 years ago

Ok did more testing here and it turns out that it kind of worked but there are a couple of things that still aren't just right:

  1. When a column is INT/INTEGER and has a default value, the default value for the column needs to also be an int. I can see how this didn't get picked up because running SHOW CREATE TABLE dispatcher; as you can see above shows us that the setid column (and others that are type int) have a default value of '0'. So I can see how this would translate to sqlite as '0' instead of 0 during the conversion. From my understanding this would need to be converted to an int vs a string based on my testing.
  2. When a column is varchar and has a default value of empty string '' like the destination column above, this converts as "destination" VARCHAR(192) NOT NULL instead of "destination" VARCHAR(192) NOT NULL DEFAULT '',.

So after your latest update, here's the new output of dispatcher:

sqlite> .schema dispatcher
CREATE TABLE "dispatcher" (
    "id" INTEGER NOT NULL ,
    "setid" INTEGER NOT NULL DEFAULT '0',
    "destination" VARCHAR(192) NOT NULL ,
    "flags" INTEGER NOT NULL DEFAULT '0',
    "priority" INTEGER NOT NULL DEFAULT '0',
    "attrs" VARCHAR(128) NOT NULL ,
    "description" VARCHAR(64) NOT NULL ,
    PRIMARY KEY ("id")
);
CREATE UNIQUE INDEX "dispatcher_description" ON "dispatcher" ("description");

When the expected output I believe should be:

sqlite> .schema dispatcher
CREATE TABLE "dispatcher" (
    "id" INTEGER NOT NULL ,
    "setid" INTEGER NOT NULL DEFAULT 0,
    "destination" VARCHAR(192) NOT NULL DEFAULT '',
    "flags" INTEGER NOT NULL DEFAULT 0,
    "priority" INTEGER NOT NULL DEFAULT 0,
    "attrs" VARCHAR(128) NOT NULL DEFAULT '',
    "description" VARCHAR(64) NOT NULL DEFAULT '',
    PRIMARY KEY ("id")
);
CREATE UNIQUE INDEX "dispatcher_description" ON "dispatcher" ("description");

This is partly my fault for not catching this on the original issue 🤦

techouse commented 3 years ago

I see. I thought that numbers have to be wrapped into quotes reading the documentation on the DEFAULT clause. After all, that's what MySQL uses.

Seems I was wrong.

techouse commented 3 years ago

This commit should fix it. Also, check the test test_translate_default_from_mysql_to_sqlite

jmordica commented 3 years ago

Ah ok so I just tested number 1 again with the int wrapped in single quotes and it does seem to work. I’m not sure what I was doing before that made it not work. Ans you are right about the docs saying it is acceptable so I would leave that as is and not deviate from the docs.

I see in your latest commit that number 2 should also be resolved. Sorry for the confusion here. I should have done more testing on number 1 before posting.

techouse commented 3 years ago

I made even more explicit tests for this and handled some edge cases, so this should work now as expected. Do check the tests:

jmordica commented 3 years ago

Ok this latest change seems to work great. Here's the output:

sqlite> .schema dispatcher
CREATE TABLE "dispatcher" (
    "id" INTEGER NOT NULL ,
    "setid" INTEGER NOT NULL DEFAULT 0,
    "destination" VARCHAR(192) NOT NULL DEFAULT '',
    "flags" INTEGER NOT NULL DEFAULT 0,
    "priority" INTEGER NOT NULL DEFAULT 0,
    "attrs" VARCHAR(128) NOT NULL DEFAULT '',
    "description" VARCHAR(64) NOT NULL DEFAULT '',
    PRIMARY KEY ("id")
);
CREATE UNIQUE INDEX "dispatcher_description" ON "dispatcher" ("description");

I'm a little afraid that if a default int isn't wrapped in '' people may complain about it not converting back to mysql from sqlite, and since the docs recommend wrapping them should you just leave them wrapped in '' like they were in your first commit?

I tested it both ways and it works as expected both ways. Sorry to keep re-opening this 😆

techouse commented 3 years ago

Right, so with the latest changes this MySQL DDL

CREATE TABLE `bingo` (
  `id` int unsigned NOT NULL AUTO_INCREMENT,
  `integer_col` int DEFAULT '1',
  `integer2_col` int DEFAULT '0',
  `varchar_col` varchar(100) COLLATE utf8mb4_general_ci DEFAULT 'lalala',
  `varchar2_col` varchar(100) COLLATE utf8mb4_general_ci DEFAULT '1',
  `varchar3_col` varchar(100) COLLATE utf8mb4_general_ci DEFAULT '0',
  `float_col` float DEFAULT '1234.57',
  `bool_col` tinyint(1) DEFAULT '1',
  `bool2_col` tinyint(1) DEFAULT '0',
  `text_col` text COLLATE utf8mb4_general_ci,
  `empty_varchar_col` varchar(100) COLLATE utf8mb4_general_ci DEFAULT '',
  PRIMARY KEY (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci

gets converted to this SQLite DDL

CREATE TABLE "bingo" (
    "id" INTEGER NOT NULL ,
    "integer_col" INTEGER NULL DEFAULT 1,
    "integer2_col" INTEGER NULL DEFAULT 0,
    "varchar_col" VARCHAR(100) NULL DEFAULT 'lalala',
    "varchar2_col" VARCHAR(100) NULL DEFAULT '1',
    "varchar3_col" VARCHAR(100) NULL DEFAULT '0',
    "float_col" FLOAT NULL DEFAULT '1234.57',
    "bool_col" TINYINT NULL DEFAULT 1,
    "bool2_col" TINYINT NULL DEFAULT 0,
    "text_col" TEXT NULL ,
    "empty_varchar_col" VARCHAR(100) NULL DEFAULT '',
    PRIMARY KEY ("id")
);

And yes, I'm also concerned about the integers being unwrapped. It's kinda unsafe. I'll put the quotes back.

techouse commented 3 years ago

OK, I've put the quotation wrapping back, so now this MySQL DDL

CREATE TABLE `bingo` (
  `id` int unsigned NOT NULL AUTO_INCREMENT,
  `integer_col` int DEFAULT '1',
  `integer2_col` int DEFAULT '0',
  `varchar_col` varchar(100) COLLATE utf8mb4_general_ci DEFAULT 'lalala',
  `varchar2_col` varchar(100) COLLATE utf8mb4_general_ci DEFAULT '1',
  `varchar3_col` varchar(100) COLLATE utf8mb4_general_ci DEFAULT '0',
  `float_col` float DEFAULT '1234.57',
  `bool_col` tinyint(1) DEFAULT '1',
  `bool2_col` tinyint(1) DEFAULT '0',
  `text_col` text COLLATE utf8mb4_general_ci,
  `empty_varchar_col` varchar(100) COLLATE utf8mb4_general_ci DEFAULT '',
  `timestamp_col` timestamp NULL DEFAULT CURRENT_TIMESTAMP,
  PRIMARY KEY (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci

gets converted to this SQLite DDL

CREATE TABLE "bingo" (
    "id" INTEGER NOT NULL ,
    "integer_col" INTEGER NULL DEFAULT '1',
    "integer2_col" INTEGER NULL DEFAULT '0',
    "varchar_col" VARCHAR(100) NULL DEFAULT 'lalala',
    "varchar2_col" VARCHAR(100) NULL DEFAULT '1',
    "varchar3_col" VARCHAR(100) NULL DEFAULT '0',
    "float_col" FLOAT NULL DEFAULT '1234.57',
    "bool_col" TINYINT NULL DEFAULT '1',
    "bool2_col" TINYINT NULL DEFAULT '0',
    "text_col" TEXT NULL ,
    "empty_varchar_col" VARCHAR(100) NULL DEFAULT '',
    "timestamp_col" DATETIME NULL DEFAULT CURRENT_TIMESTAMP,
    PRIMARY KEY ("id")
);
techouse commented 3 years ago

Fix released in v1.4.2

jmordica commented 3 years ago

Perfection! Sorry for the back and forth here and again, thanks for the promptness.

jmordica commented 3 years ago

You're going to hate me. I have tested all column default types and found one more small issue. NULL defaults.

Here's a mysql table:

CREATE TABLE `address` (
  `id` int(10) unsigned NOT NULL AUTO_INCREMENT,
  `grp` int(11) unsigned NOT NULL DEFAULT '1',
  `ip_addr` varchar(50) NOT NULL,
  `mask` int(11) NOT NULL DEFAULT '32',
  `port` smallint(5) unsigned NOT NULL DEFAULT '0',
  `tag` varchar(64) DEFAULT NULL,
  PRIMARY KEY (`id`)
) ENGINE=InnoDB AUTO_INCREMENT=39 DEFAULT CHARSET=latin1

Here's what we get from the conversion tool:

sqlite> .schema address
CREATE TABLE "address" (
    "id" INTEGER NOT NULL ,
    "grp" INTEGER NOT NULL DEFAULT '1',
    "ip_addr" VARCHAR(50) NOT NULL ,
    "mask" INTEGER NOT NULL DEFAULT '32',
    "port" SMALLINT NOT NULL DEFAULT '0',
    "tag" VARCHAR(64) NULL ,
    PRIMARY KEY ("id")
);

Notice the tag column isn't getting the DEFAULT NULL

techouse commented 3 years ago

To be honest I've never even heard of something like DEFAULT NULL. A column is either NOT NULL or NULL. And according to the SQLite docs:

The DEFAULT clause specifies a default value to use for the column if no value is explicitly provided by the user when doing an INSERT. If there is no explicit DEFAULT clause attached to a column definition, then the default value of the column is NULL. An explicit DEFAULT clause may specify that the default value is NULL, a string constant, a blob constant, a signed-number, or any constant expression enclosed in parentheses. A default value may also be one of the special case-independent keywords CURRENT_TIME, CURRENT_DATE or CURRENT_TIMESTAMP. For the purposes of the DEFAULT clause, an expression is considered constant if it contains no sub-queries, column or table references, bound parameters, or string literals enclosed in double-quotes instead of single-quotes.

So the code works as expected and your example up there is perfectly fine.

jmordica commented 3 years ago

Ok good deal. I’m new to SQLite. I appreciate it!

techouse commented 3 years ago

FYI MySQL behaves just the same with default null. As I said, I've never seen it used in either MySQL or SQLite.