Identifier gets discarded after running migration

[agnes512]

Current Behavior

If we run migration on model:

@model()
export class Order extends Entity {
  @property({
    type: 'number',
    id: true,
    generated: true,
  })
  orderId?: number;

  @property({
    type: 'number',
  })
  quantity: number;

  // other props..

  constructor(data?: Partial<Order>) {
    super(data);
  }
}

and check the DB:
Order, it doesn't have the identifier:

We need to specify the column setting (column name esp, should be in lowercase) for the id property to have it on the table:

@model()
export class Order extends Entity {
  @property({
    type: 'number',
    id: true,
    generated: true,
    mysql: {  // this setting allows you to have different names for model and db column
      columnName: 'orderid',
      dataType: 'integer',
      dataLength: null,
      dataPrecision: null,
      dataScale: 0,
      nullable: 'NO',
    },
})
  orderId?: number;
...
}

so that we can have:

Expected Behavior

There is no documentation on that ( in LB4 at least).
Should:

• have it documented
• or fix the migration function

Additional information

So far the issue occurs when using MySQL or PostgreSQL as datasource.
Might have something to do with the function

/**
 * Get the escaped column name for a given model property
 * @param {String} model The model name
 * @param {String} property The property name
 * @returns {String} The escaped column name
 */
SQLConnector.prototype.columnEscaped = function(model, property) {
  return this.escapeName(this.column(model, property));
};

and also some escapeIdName functions in connector-postgres,mysql

Acceptance Criteria

• should have it documented on LB4 site ( both migration and migration-cli pages)
• fix how we check the identifier. If it's not an easy fix, should at least warn users that the identifier gets discarded and the column settings needs to be added.
• add test cases ( for MySQL and Postgres at least)

[agnes512]

Looks like the bug has been fixed in PR loopbackio/loopback-connector#171
i.e

If the column name is not specified:

export class Customer extends Entity {
  @property({
    id: true,
    generated: true,
    type: number,
  })
  testID: number;

• for DBs that don't have a preferred naming convention e.g MySQL, the column name is the same as the property name:

mysql> show columns from Customer;
+----------+--------------+------+-----+---------+----------------+
| Field    | Type         | Null | Key | Default | Extra          |
+----------+--------------+------+-----+---------+----------------+
| testID   | int(11)      | NO   | PRI | NULL    | auto_increment |
| name     | varchar(512) | YES  |     | NULL    |                |
| parentId | text         | YES  |     | NULL    |                |
+----------+--------------+------+-----+---------+----------------+

• for DBs that have a preferred naming convention e.g Postgres in lowercase and Oracle in UPPERCASE, the column name is converted to the preferred case:

 column_name |              column_default              |     data_type     
-------------+------------------------------------------+-------------------
 testid      | nextval('customer_testid_seq'::regclass) | integer
 name        |                                          | character varying
 parentid    |                                          | text

If the column name is specified in the property definition:

  @property({
    id: true,
    generated: true,
    type: number,
    mysql: {
      columnName: 'testID',
      dataType: 'integer',
    },
    postgresql: {
      columnName: 'testID',
      dataType: 'integer',
    }
  })
  ID: number; // not the same as the column name

DBs have the correct column name generated. And confirmed with tests that the column name maps to the property name as expected.

MySQL:

mysql> show columns from Customer;
+----------+--------------+------+-----+---------+----------------+
| Field    | Type         | Null | Key | Default | Extra          |
+----------+--------------+------+-----+---------+----------------+
| testID   | int(11)      | NO   | PRI | NULL    | auto_increment |
...
+----------+--------------+------+-----+---------+----------------+

PostgreSQL:

  
-------------+--------------------------------------------+-------------------
 testID      | nextval('"customer_testID_seq"'::regclass) | integer
...
-------------+--------------------------------------------+-------------------

[agnes512]

Thes tests are being added to Postgres and Oracle in loopbackio/loopback-connector-postgresql#422 and loopbackio/loopback-connector-oracle#200 ( they migrate first then discover tables). And MySQL already has tests.

[agnes512]

Closing as done