Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[@loopback/sequelize] Duplicate column names in relation queries #9617

Open
KalleV opened this issue Jun 5, 2023 · 2 comments
Open

[@loopback/sequelize] Duplicate column names in relation queries #9617

KalleV opened this issue Jun 5, 2023 · 2 comments

Comments

@KalleV
Copy link
Contributor

KalleV commented Jun 5, 2023

Describe the bug

Repository queries with relations that do not explicitly set all the keyTo / keyFrom / through properties will lead to broken SQL queries that contain duplicated title case column names mixed in with the expected camelcase columns.

Example:

SELECT bookid as bookId, reader.id as readerId, ReaderId …

Relates to:
sequelize/sequelize#9328
#9591
sourcefuse/loopback4-sequelize#35

Logs

No response

Additional information

Workaround seems to be to go through all the Entity relations and explicitly set all the relation key names but that can be time-consuming and error-prone with a larger project. It seems like it might be possible to mitigate this at the loopback model to Sequelize relation layer.

Reproduction

With these changes, running the tests for the Sequelize extension will replicate the error:
KalleV@3c29d85

Turns out it's necessary to define additional "belongsTo" relations in other entities before this happens. The extra relation is added to the "Patient" entity in this case. With this set up, I am seeing the following happen:

  • The default relation properties are set by Loopback (i.e. I can see the "keyFrom" is populated as todoListId) but the "keyTo" is undefined:
    loopback_relation_data
  • This leads to undefined being passed as the foreign key to sequelize:
    undefined_foreign_key
  • And then that causes Sequelize to assign a Title Case property through it's own default relation column logic leading to a duplicate column name in the database query:
    duplicate_column_sqlite_error
@KalleV KalleV added the bug label Jun 5, 2023
@KalleV KalleV changed the title [@loopback/sequelize] Duplicate column names in relation queries and foreign keys with Title Case [@loopback/sequelize] Duplicate column names in relation queries Jun 5, 2023
@shubhamp-sf
Copy link
Contributor

shubhamp-sf commented Jun 6, 2023

I personally feel that when it comes to anything that gets included in query be it table name, column name, relations keys etc. We should explicitly set it rather than relying on LoopBack's defaults for the obvious surety reasons.

Although it would be very beneficial for new adopters to face as little code changes as possible for them to implement this in their project.

I already have so much in my bucket list so I'll pick this up in the first week of July, unless any one else from the community wants to work on this.

@samarpanB
Copy link
Contributor

@KalleV I see some commits from you on your forked repo. Were you able to fix this issue ? If yes, would you mind to submit a PR in this repo too, so that community can benefit from it too ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants