Column names in lowercase using lb4 discover method - SQL connector

[sureshkodur]

Describe

Hi Team. I'm generating models from SQL db using lb4 discover method. But, the column names generating in lowercase. While in my SQL db, they are in camelCase. See, below code:

@property({
    type: Number,
    required: true,
    precision: 10,
    scale: 0,
    id: 1,
    mssql: 
         {"columnName":"ErrorID",
          "dataType":"int",
          "dataLength":null,
          "dataPrecision":10,
          "dataScale":0,"
          nullable":"NO"
         },
  })
  errorid: Number;

Current behavior

The column name is generated in lowercase as errorid instead of ErrorID.

Expected Behavior

• Would like to get the column name ErrorID as it is.

Acceptance Criteria

• column names should default to camelCase instead of lowercase
• add a prompt of 2 options when doing lb4 discover
  • column names to property names:
    • camelCase ( LB4 default, recommended)
    • keep it the same as the column names ( warning: might cause unexpected behavior. please read the ....file)
• enhance the documentation. List out options and their limitations.

Out of scope:

• an extension point allowing users to provide custom function to transform database column names to LB4 property names

[sureshkodur]

@bajtos , @emonddr , @raymondfeng, @ritch , Can anyone, any update on this. Help me

[dhmlau]

@agnes512, could you please take a look? Thanks.

[agnes512]

I reproduced the above issue by using lb4 discover --schema testdb with datasource config:

{
 "name": "db",
 "connector": "mysql",
  ...
 "database": "testdb"
}

I can access database with the generated models. I figured that this issue is more about developer-experience instead of a bug. And the lowerCamelCase name might be generated here, not sure about it.

As for the naming convention feature, it needs further discussion.
Will link this issue to other similar issues.

[agnes512]

@sureshkodur Hi! I was testing it out yesterday. And it does generate errorid instead of ErrorID. If it has errors because the naming convention, we would label it as a bug. But it works fine for me.
Are you getting any errors causing by the names?

[sureshkodur]

@agnes512 , In my MSSQL db, it has the column name as 'ErrorID', which is in Uppper CamelCase. But when I'm trying to generate schema using lb4 discover it is errorid instead of ErrorID. But I need it as ErrorID in model. Are you understand the issue?

[agnes512]

@agnes512 , In my MSSQL db, it has the column name as 'ErrorID', which is in Uppper CamelCase. But when I'm trying to generate schema using lb4 discover it is errorid instead of ErrorID. But I need it as ErrorID in model. Are you understand the issue?

Yes. After getting models from MySQL, do you still use MySQL as your database? In my case, I did, and the app works fine, by that I mean, my application can store/get data via MySQL even it is using errorid.

That's why I figured the naming convention does not cause functional errors. It's about how we convert column names to property names. This issue is more about dev experience. That's why I changed the label, which helps us to plan things out. 😄

If you're getting any functional errors (such as API doesn't work, controller generating error) because of the naming, please let me know, and how I can reproduce it.

[sureshkodur]

@agnes512 , Are you read that message properly.. Its not MySQL. Its MSSQL... Can you check it once with MSSQL? @rmg

[agnes512]

@sureshkodur Hi, I also tried it out on MSSQL. I got the same result: reproduced that issue, and the App still works. i.e I can use POST/GET APIs to interact with MSSQL. Here is my datasource config:

{
  "name": "db",
  "connector": "mssql",
  "host": "localhost",
  "port": 1433,
  "user": ...,
  "password": ... ,
  "database": "master"
}

Like I mentioned above, I believe this (ErrorID -> errorid) is because of the naming convention that we are using.
It is not a functional error of any specific connectors.

Sorry that we don't have naming convention options available for now. You can still change errorid to ErrorID manually as a workaround. I believe that the App would still work.

Thanks

[sureshkodur]

@agnes512 , Yes. App working fine for me also. There is no issue. Only issue with the naming convention. For now, I'm done it manulally. Thank you.

[dhmlau]

@agnes512 , could you please add the acceptance criteria so that the team can estimate? Thanks.
I believe we're looking for a way to allow users to specify their naming convention?

[agnes512]

@strongloop/loopback-maintainers We've been having this issue for a while. To improve the developer experience, what kind of acceptance criteria do we want to achieve?
To have different naming convention options for CLI lb4 discover, or default it as what the column name is?

I mentioned the naming convention here is because from issue Provide a way to specify table and field naming convention with automigrate/autoupdate, it seems like different conventions are needed for different DBs.

We know that db column name and the corresponding model property name can be different. Maybe this will allow us to have different naming conventions.

Any thoughts?

[dhmlau]

I'm thinking whether it's possible to allow users to plug-in their own naming convention (similar to what's mentioned in loopbackio/loopback-connector-mysql#57 (comment)). This is because users have different naming conventions, it's not likely that we pick one naming convention and satisfy everyone.

[jannyHou]

For this particular use case:

@property({
    type: Number,
    required: true,
    precision: 10,
    scale: 0,
    id: 1,
    mssql: 
         {"columnName":"ErrorID",
          "dataType":"int",
          "dataLength":null,
          "dataPrecision":10,
          "dataScale":0,"
          nullable":"NO"
         },
  })
  errorid: Number;

I think we should at least allow people to either honor the column name from db (ErrorID) or have the property name in our default convention(errorid).

Some thought about supporting custom naming convension:
Do you know where does the name coercion happen? I did a very quick search in loopback-connector and loopback-connector-mysql but didn't find the corresponding code.

• If there is a particular function doing it, we can think about adding mixin to connector to allow people override the function and provide their own implementation
• If it's hard coded somewhere we can extract it to a function then do ^

[agnes512]

Do you know where does the name coercion happen?

I am not sure about MySQL, but for Postgres, auto-migrate calls function columnEscaped, which modifies property names to lowerCase.

[bajtos]

My two cents:

By default, I would like our model discovery to recognize all common naming styles (PascalCase, camelCase, snake_case, UPPER_SNAKE_CASE) and convert them to camelCase style that's the norm in our TypeScript codebase. To me, that's the acceptance criteria to resolve this issue.

style	database name (input)	LB4 property name (expected output)
pascal case	CustomerId	customerId
camel case	customerId	customerId
snake case	customer_id	customerId
	Customer_Id	customerId
	CUSTOMER_ID	customerId

While we can add an extension point allowing users to provide custom function to transform database column names to LB4 property names, I consider that as out of scope.

I also think we should focus on model discovery only, I am proposing to leave naming conventions used during database migrations out of scope of this discussion.

[agnes512]

@bajtos I agree that the default property name should be camelCase instead of lowercase and focus on discover first.

I also agree with @jannyHou that we should also allow people to have their property name the same as db column names as part of scope in the story. e.g:
column name CustomerId -> 2 options: customerId(default) or CustomerId (keep it the same).
But I am not sure how much effort it might need. Connectors validate the names before converting them to properties.

How about:

Criteria:

• column names should default to camelCase instead of lowercase
• add a prompt of 2 options when doing lb4 discover
  • column names to property names:
    • camelCase ( LB4 default, recommended)
    • keep it the same as the column names ( might cause unexpected behavior. please read the ....file)
• enhance the documentation. List out the limitations.

Out of scope:

• an extension point allowing users to provide custom function to transform database column names to LB4 property names

[bajtos]

Criteria:

• column names should default to camelCase instead of lowercase

• add a prompt of 2 options when doing lb4 discover

  • column names to property names:

    • camelCase ( LB4 default, recommended)
    • keep it the same as the column names ( might cause unexpected behavior. please read the ....file)

• enhance the documentation. List out the limitations.

Out of scope:

• an extension point allowing users to provide custom function to transform database column names to LB4 property names

LGTM 👏

Personally, I am also ok to leave the task "keep it the same as the column names ( might cause unexpected behavior. please read the ....file)" out of scope, depending on what other team members prefer.

[agnes512]

Make sure the name passes the validation function and also converts the expected case.

[agnes512]

closing as done!