sqlite dialect: getCollectionProperties error

[smilingthax]

/[...]/migrate-orm2/node_modules/sql-ddl-sync/lib/Dialects/sqlite.js:58
                    column.defaultValue = m[0];
                                           ^
TypeError: Cannot read property '0' of null
    at Statement.<anonymous> (/[...]/migrate-orm2/node_modules/sql-ddl-sync/lib/Dialects/sqlite.js:58:29)

replacing the line with column.defaultValue = dCol.dflt_value; seems to work.

[dxg]

Can you please submit a pull request with a test or two?

[smilingthax]

Well, I'm just using migrate-orm2, which happens to depend on sql-ddl-sync. I have no clue why migrate-orm2 even calls exports.getCollectionProperties in sqlite.js

[smilingthax]

Ok, I've tried to create a test, but your test-suite is a mess:

1. It does not even run.

   
   node-sql-ddl-sync/test$ nodejs run-db.js -u sqlite:test1.sqlite3`
   /[...]/node-sql-ddl-sync/test/integration/db.js:4
   var sync    = new Sync({
                 ^

ReferenceError: Sync is not defined at Object. (node-sql-ddl-sync/test/integration/db.js:4:19) at Module._compile (module.js:570:32) at Object.Module._extensions..js (module.js:579:10) at Module.load (module.js:487:32) at tryModuleLoad (module.js:446:12) at Function.Module._load (module.js:438:3) at Module.require (module.js:497:17) at require (internal/module.js:20:19) at node-sql-ddl-sync/node_modules/mocha/lib/mocha.js:230:27 at Array.forEach (native)

=>  Solution:

--- a/test/integration/db.js +++ b/test/integration/db.js @@ -1,7 +1,7 @@ -var index = require("../../lib/index").Sync; +var index = require("../../lib/index"); var common = require("../common"); var should = require("should"); -var sync = new Sync({ +var sync = new index.Sync({ driver : common.driver, debug : function (text) { //console.log("> %s", text);

2. "Dropping a column" test is executed and fails, despite sqlite should conditionally skip that test.
=> Solution:

--- a/test/integration/db.js +++ b/test/integration/db.js @@ -50,7 +50,7 @@ describe("db", function () { }); });

• if (common.dialect != "sqlite") {
• if (common.driver.dialect != "sqlite") { describe("Dropping a column", function () { before(common.dropColumn('born'));

3. The tests are commented out. Maybe that's related to the comment // These will fail because autosync has been disabled pending data integrity concerns. Anyways, trying to enable them (e.g.

   
   @@ -124,7 +124,7 @@ describe("db", function () {
               });
       }

• xdescribe("Changing a column", function () {
• describe("Changing a column", function () { before(common.changeColumn('int4'));

  ) will result in:

  1) db Changing a column "before all" hook: Error: SQLITE_ERROR: near "MODIFY": syntax error at Error (native)

4. Trying to investigate 3. by adding ?debug=1 to the uri passed to run-db.js:

   TypeError: Cannot use 'in' operator to search for 'debug' in debug=1
   at Object.exports.connect (node-sql-ddl-sync/node_modules/orm/lib/ORM.js:121:30)
   at Object.<anonymous> (node-sql-ddl-sync/test/run-db.js:24:5)
   at Module._compile (module.js:570:32)
   [...]

   => Solution (orm expects .query to be an object, not a string):

   
   --- a/test/run-db.js
   +++ b/test/run-db.js
   @@ -13,7 +13,7 @@ if (!program.uri) {
       process.exit(1);
   }

-var uri = url.parse(program.uri); +var uri = url.parse(program.uri, { parseQueryString: true });

if (!uri.hasOwnProperty("protocol") || !uri.protocol) { program.outputHelp();

3.b As sqlite3 does not support "ALTER COLUMN ... MODIFY", it does not make sense to have a case for sqlite in common.js's `export.changeColumn`. The according describe(...) in integration/db.js should probably be moved into the non-sqlite conditional section.

5. The the database-specific tests (`test/run-db.js`) will not run automatically, there are not even hints that these tests have to be run manually - and thus breakage will not be noticed ...

(Sidenote regarding the other disabled tests: The sync process does not seem to sync the Indices after creation despite the reasoning for not syncing DDL changes to collections after initial creation (possible data loss) does not apply to indices.)

---
Ok, back to what I actually wanted to do (note: diff contains change from 2.):

--- a/test/integration/db.js +++ b/test/integration/db.js @@ -15,7 +15,7 @@ sync.defineCollection(common.table, { male : { type: "boolean" }, born : { type: "date", time: true }, born2 : { type: "date" },

• int2 : { type: "integer", size: 2 },

• int2 : { type: "integer", size: 2, defaultValue: 1 }, int4 : { type: "integer", size: 4 }, int8 : { type: "integer", size: 8 }, float4 : { type: "number", size: 4 }, @@ -50,7 +50,18 @@ describe("db", function () { }); });

• if (common.dialect != "sqlite") {

• describe("Getting current table layout", function() {

• it("should execute without error", function (done) {

• index.dialect(common.driver.dialect).getCollectionProperties(common.driver, common.table, function(err, cols) {

• should.not.exist(err);

• should.exist(cols);

• return done();

• });

• });

• });

• if (common.driver.dialect != "sqlite") { describe("Dropping a column", function () { before(common.dropColumn('born'));

  
  But, all in all, you probably want to test "current table layout matches original specification". 
  I'll leave that as an exercise to the reader...

PS:

--- a/test/common.js
+++ b/test/common.js
@@ -90,7 +90,7 @@ exports.dropTable = function (name) {
        return function (done) {
                exports.driver.execQuery("DROP TABLE IF EXISTS ??", [exports.table], done);
        };
-}
+};

 function unknownProtocol() {
        return new Error("Unknown protocol - " + exports.driver.dialect);