refactor: driver.querySchemaInfo() should still return defaultValue

and the defaultValue can be and should be omitted at the model initialization process instead

index.js

@@ -55,10 +55,13 @@ function initAttributes(model, columns) {
   const attributes = {};
 
   for (const columnInfo of columns) {
-    const { columnName, dataType } = columnInfo;
+    const { columnName, dataType, defaultValue, ...restInfo } = columnInfo;
     const name = columnName == '_id' ? columnName : camelCase(columnName);
+    // leave out defaultValue to let database take over the default
     attributes[name] = {
-      ...columnInfo,
+      ...restInfo,
+      columnName,
+      dataType,
       type: model.driver.DataTypes.findType(dataType),
     };
   }

src/drivers/mysql/schema.js

@@ -38,8 +38,7 @@ module.exports = {
         columnName: row.column_name,
         columnType: row.column_type,
         columnComment: row.column_comment,
-        // It seems there is NO NEED to assign defaultValue from schema info, otherwise it may increase SQL execution cost
-        // defaultValue: row.column_default,
+        defaultValue: row.column_default,
         dataType: row.data_type,
         allowNull: row.is_nullable === 'YES',
         primaryKey: row.column_key == 'PRI',

src/drivers/postgres/schema.js

@@ -48,11 +48,11 @@ module.exports = {
 
     const { pool } = this;
     const { rows } = await pool.query(text, [database, tables]);
-    const schema = {};
+    const schemaInfo = {};
 
     for (const row of rows) {
       const tableName = row.table_name;
-      const columns = schema[tableName] || (schema[tableName] = []);
+      const columns = schemaInfo[tableName] || (schemaInfo[tableName] = []);
       let { data_type: dataType, character_octet_length: length } = row;
 
       if (dataType === 'character varying') dataType = 'varchar';
@@ -61,8 +61,7 @@ module.exports = {
       columns.push({
         columnName: row.column_name,
         columnType: length > 0 ? `${dataType}(${length})` : dataType,
-        // It seems there is NO NEED to assign defaultValue from table schema, otherwise it may increase SQL execution cost
-        // defaultValue: primaryKey ? null : row.column_default,
+        defaultValue: primaryKey ? null : row.column_default,
         dataType,
         allowNull: row.is_nullable !== 'NO',
         // https://www.postgresql.org/docs/9.5/infoschema-table-constraints.html
@@ -71,7 +70,7 @@ module.exports = {
       });
     }
 
-    return schema;
+    return schemaInfo;
   },
 
   async alterTable(table, changes) {

src/drivers/sqlite/schema.js

@@ -100,7 +100,7 @@ module.exports = {
       return this.query(`PRAGMA table_info(${this.escapeId(table)})`);
     });
     const results = await Promise.all(queries);
-    const schema = {};
+    const schemaInfo = {};
     for (let i = 0; i < tables.length; i++) {
       const table = tables[i];
       const { rows } = results[i];
@@ -110,18 +110,17 @@ module.exports = {
         const result = {
           columnName: name,
           columnType,
-          // It seems there is NO NEED to assign defaultValue from table schema, otherwise it may increase SQL execution cost
-          // defaultValue: parseDefaultValue(dflt_value),
+          defaultValue: parseDefaultValue(dflt_value),
           dataType: columnType.split('(')[0],
           allowNull: primaryKey ? false : notnull == 0,
           primaryKey,
         };
         return result;
       });
-      if (columns.length > 0) schema[table] = columns;
+      if (columns.length > 0) schemaInfo[table] = columns;
     }
 
-    return schema;
+    return schemaInfo;
   },
 
   async createTable(table, attributes, opts = {}) {
@@ -171,8 +170,8 @@ module.exports = {
 
   async changeColumn(table, name, params) {
     const attribute = new Attribute(name, params);
-    const schema = await this.querySchemaInfo(null, table);
-    const columns = schema[table];
+    const schemaInfo = await this.querySchemaInfo(null, table);
+    const columns = schemaInfo[table];
 
     for (const entry of columns) {
       if (entry.columnName === attribute.columnName) {

test/unit/drivers/sqlite/index.test.js

@@ -6,7 +6,7 @@ const strftime = require('strftime');
 const { heresql } = require('../../../../src/utils/string');
 const SqliteDriver = require('../../../../src/drivers/sqlite');
 
-const { INTEGER, BIGINT, STRING, DATE, BOOLEAN } = SqliteDriver.prototype.DataTypes;
+const { INTEGER, BIGINT, STRING, DATE, BOOLEAN, JSONB } = SqliteDriver.prototype.DataTypes;
 
 const options = {
   database: '/tmp/leoric.sqlite3',
@@ -68,7 +68,7 @@ describe('=> SQLite driver', () => {
     });
   });
 
-  it('driver.truncateTable(table', async () => {
+  it('driver.truncateTable(table)', async () => {
     await driver.dropTable('notes');
     await driver.createTable('notes', {
       id: { type: BIGINT, primaryKey: true, autoIncrement: true },
@@ -79,6 +79,52 @@ describe('=> SQLite driver', () => {
     await driver.truncateTable('notes');
     assert.equal((await driver.query('SELECT count(*) AS count FROM notes')).rows[0].count, 0);
   });
+
+  it('driver.alterTable(table, changes)', async function() {
+    await driver.dropTable('notes');
+    await driver.createTable('notes', {
+      id: { type: BIGINT, primaryKey: true, autoIncrement: true },
+      title: { type: STRING, allowNull: false },
+    });
+    await driver.alterTable('notes', {
+      params: { type: JSONB },
+    });
+    const { rows } = await driver.describeTable('notes');
+    assert.deepEqual(rows.pop(), {
+      cid: 2,
+      name: 'params',
+      type: 'JSON',
+      notnull: 0,
+      dflt_value: null,
+      pk: 0
+    });
+  });
+
+  it('driver.alterTable(table, changes) should not break table', async function() {
+    await driver.dropTable('notes');
+    await driver.createTable('notes', {
+      id: { type: BIGINT, primaryKey: true, autoIncrement: true },
+      title: { type: new STRING(255) },
+    });
+    await driver.query('INSERT INTO notes (title) VALUES (NULL)');
+    await assert.rejects(async function() {
+      await driver.alterTable('notes', {
+        title: { type: new STRING(127), allowNull: false, modify: true },
+      });
+    }, /NOT NULL/);
+    // should rollback if failed to alter table
+    const { rows } = await driver.describeTable('notes');
+    assert.deepEqual(rows.pop(),   {
+      cid: 1,
+      name: 'title',
+      type: 'VARCHAR(255)',
+      notnull: 0,
+      dflt_value: null,
+      pk: 0
+    });
+    const result = await driver.query('SELECT * FROM notes');
+    assert.equal(result.rows.length, 1);
+  });
 });
 
 describe('=> SQLite driver.query()', () => {

test/unit/utils/index.test.js

@@ -2,9 +2,28 @@
 
 const assert = require('assert').strict;
 const { Bone } = require('../../..');
-const { getPropertyNames } = require('../../../src/utils');
+const { compose, getPropertyNames, logger } = require('../../../src/utils');
+
+describe('=> compose', function() {
+  it('should return a default function if nothing to compose', function() {
+    const fn = compose();
+    for (const arg of  [ null, 1, function() {} ]) {
+      assert.equal(fn(arg), arg);
+    }
+  });
+
+  it('should return as is if only one function to compose', function() {
+    const fn = compose((a, b) => a + b);
+    assert.equal(fn(1, 2), 3);
+  });
+});
 
 describe('=> getPropertyNames', function() {
+  it('should return empty result if nothing to get', async function() {
+    assert.deepEqual(getPropertyNames(), []);
+    assert.deepEqual(getPropertyNames(null), []);
+  });
+
   it('should traverse up to Bone.prototype', async function() {
     class Foo extends Bone {
       get a() {
@@ -46,3 +65,9 @@ describe('=> getPropertyNames', function() {
     assert.deepEqual(getPropertyNames(new Foo()).sort(), [ 'a' ]);
   });
 });
+
+describe('=> logger', function() {
+  it('should prefix output with [leoric]', function() {
+    logger.log('foo');
+  });
+});