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.

Sign up for GitHub

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

fix: don't overwrite primaryKey with LAST_INSERT_ID() if exists #398

Merged
merged 2 commits into from
Aug 31, 2023
Merged

fix: don't overwrite primaryKey with LAST_INSERT_ID() if exists #398

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5b74a22
fix: don't overwrite primaryKey with LAST_INSERT_ID() if exists
cyjake Aug 31, 2023
ca573a6
Merge branch 'master' into fix-oceanbase-insert-id
cyjake Aug 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/bone.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -666,7 +666,7 @@ class Bone {
// - http://dev.mysql.com/doc/refman/5.7/en/information-functions.html#function_last-insert-id
const spell = new Spell(Model, opts).$upsert(data);
return spell.later(result => {
// LAST_INSERT_ID() breaks on TDDL
// LAST_INSERT_ID() breaks on TDDL, and on OceanBase if primary key is not integer
if (this[primaryKey] == null) this[primaryKey] = result.insertId;
this.syncRaw();
return result.affectedRows;
Expand Down Expand Up @@ -806,7 +806,8 @@ class Bone {

const spell = new Spell(Model, opts).$insert(data);
return spell.later(result => {
this[primaryKey] = result.insertId;
// LAST_INSERT_ID() breaks on TDDL, and on OceanBase if primary key is not integer
if (this[primaryKey] == null) this[primaryKey] = result.insertId;
Copy link
Contributor

Choose a reason for hiding this comment

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

没测试用例么。。

Copy link
Owner Author

Choose a reason for hiding this comment

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

要 oceanbase 用非整型字段当主键才可以复现，比较难复刻……

Copy link
Collaborator

Choose a reason for hiding this comment

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

刚好带来一个单测报错
image

Copy link
Owner Author

Choose a reason for hiding this comment

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

厉害👍

这个是数据库自动做了取整然后通过 L809 订正了 user.id？

Copy link
Collaborator

Choose a reason for hiding this comment

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

MySQL 本地测试，数据库存的肯定不是这个浮点数 ID，否则不会报错（后面查用户权限的时候失败）

Copy link
Owner Author

Choose a reason for hiding this comment

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

理论上 user_id 如果声明了类型，也可以在应用层取整缓解这个问题

主要这个 edge case 和之前 ob 那个是冲突的，ob 那个是主键自定义之后返回了一个其他的唯一 id（可能是内部 id），只能两者里面选一个……

// this.#rawSaved[primaryKey] = null;
this.syncRaw();
return this;
Expand Down