Properties with mysql custom "columnName" don't get autoupdated

[stelescuraul]

Bug or feature request

If you have a model that has properties with mysql field and columnName that is different than the model property name, then it will throw error on autoupdate

• Bug
• Feature request

Description of feature (or steps to reproduce if bug)

Say we have a model "Users" with the following property:

"firstName": {
      "type": "String",
      "required": true,
      "length": 255,
      "precision": null,
      "scale": null,
      "mysql": {
        "columnName": "first_name",
        "dataType": "varchar",
        "dataLength": 255,
        "dataPrecision": null,
        "dataScale": null,
        "nullable": "N"
      },
      "_selectable": true
    }

Because it is save in the database as "first_name" and not "firstName" if you change the "nullable" property to "T" (true) the mysql connector throws error when doing autoupdate:

Error: ER_BAD_FIELD_ERROR: Unknown column 'firstName' in 'modelName'

Expected result

To be able to detect that the property name has a different name in the database, and use that when you call autoupdate function on the datasource.

Fix

I have followed down the flow and I managed to fix this error changing the line 310 in lib/migration.js to use the mysql property name if it is found as follows:

   var pName;
   if(newSettings.mysql && newSettings.mysql.columnName) pName = newSettings.mysql.columnName;
   else pName = self.client.escapeId(propName);

the above code replaces line 310. Using that, the mysql connector works fine when doing autoupdate on an existing database.

I have run the tests and they pass as expected.

Additional information (Node.js version, LoopBack version, etc)

node version : 7.7.1
strongloop v6.0.3 (node v7.7.1)
loopback version: 2.38.0```

[ygaller]

Great catch! Ran into the same problem myself with [email protected]

For a more succinct version one could just change it to this:
var pName = (newSettings.mysql && newSettings.mysql.columnName) || self.client.escapeId(propName);

[ssh24]

@stelescuraul Could you please submit a PR to fix this patch? Please tag me on it and I will take a look at it.

[ssh24]

@stelescuraul @ygaller Any updates on submitting a PR for this patch?

[stelescuraul]

@ssh24 will do it in an hour.

[stelescuraul]

@ssh24 Please check PR.

[ssh24]

@stelescuraul PR #255 is ready to be landed. Please submit the same patch for 3.x branch as soon as you can.

[darknos]

I met with this issue 5 minutes ago in my code. :(

and this fix is not full yet (especially for 3.x branch).

look here:

      var colName = expectedColNameForModel(propName, m);
      ///skip some code...
//NOTICE: we call actualize with column name like "first_name"
      actualize(colName, found);
      ///skip some code...
    function actualize(propName, oldSettings) {
//BUG: try to find property not by firstName but for first_name and then newSettings will be undefined
      var newSettings = m.properties[propName];
      if (newSettings && changed(newSettings, oldSettings)) {
//BUG: use self.columnEscaped below
        var pName = self.client.escapeId(propName);
        sql.push('CHANGE COLUMN ' + pName + ' ' + pName + ' ' +
          self.buildColumnDefinition(model, propName));
      }
    }

@ssh24 can I provide PR for it?

[stelescuraul]

@darknos this is not fixed in 3.x. i will fix it in a few hours when i get home

[darknos]

@ssh24 Lets rename #272 to "fix autoupdate issues" and I'll fix this.

[ssh24]

@stelescuraul @darknos Either one of you can do a PR for the fix on 3.x.

[darknos]

@ssh24 I'll fix this and also #259 in #272

[ssh24]

@darknos I suggest creating two different PR's for this (#250) and #259. PR #272 is good as it should be.

[darknos]

ok. I just already fixed it locally but is not yet committed. :) I'll make branches for the issues.

[darknos]

done. look at #273

[ssh24]

Fixed in both 2.x and 3.x. Closing this issue as a result.