Skip to content

Conversation

@leopoldchen
Copy link
Contributor

@leopoldchen leopoldchen commented Mar 22, 2020

Checklist
  • npm test passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)
Description of change

Current sequelize-ts example can't not pass all test cases, it will raise error while using 'include' operator.

Here is the error log:

SequelizeDatabaseError: Column 'id' in field list is ambiguous

The reason is the sql generated by sequelize was missing the mainTable name, here is the incorrect sql code:

SELECT ``.`id`, ``.`title`, ``.`content`, ``.`user_id`, ``.`created_at`, ``.`updated_at`, ``.`created_at` AS `createdAt`, ``.`updated_at` AS `updatedAt`, ``.`user_id` AS `userId`, `user`.`id` AS `user.id`, `user`.`name` AS `user.name`, `user`.`age` AS `user.age` FROM `posts` AS `` LEFT OUTER JOIN `users` AS `user` ON ``.`user_id` = `user`.`id` WHERE ``.`id` = 1;

Dig into the source code of sequelize we could find out these lines:

// resolve table name options
    if (options.tableAs) {
      mainTable.as = this.quoteIdentifier(options.tableAs);
    } else if (!Array.isArray(mainTable.name) && mainTable.model) {
      mainTable.as = this.quoteIdentifier(mainTable.model.name);
    }

In current example, app.model.Post.name will return empty string, because its model return an anonymous class.

return class extends Post { 
   //...
 }

Copy link

@lvchengli lvchengli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can help

@ZHAOzn
Copy link

ZHAOzn commented Jun 17, 2022

this can help

1 similar comment
@bobby169
Copy link

this can help

Copy link
Member

@fengmk2 fengmk2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@fengmk2 fengmk2 merged commit 5ea83ef into eggjs:master Dec 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants