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

Feature: update multiple rows. #55

Merged
merged 10 commits into from
Sep 30, 2018
Merged

Conversation

nodejh
Copy link
Contributor

@nodejh nodejh commented Aug 3, 2018

Support update multiple rows

@codecov
Copy link

codecov bot commented Aug 3, 2018

Codecov Report

Merging #55 into master will decrease coverage by 12.93%.
The diff coverage is 2.32%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #55       +/-   ##
===========================================
- Coverage   98.92%   85.98%   -12.94%     
===========================================
  Files           6        6               
  Lines         278      321       +43     
  Branches       41       49        +8     
===========================================
+ Hits          275      276        +1     
- Misses          3       45       +42
Impacted Files Coverage Δ
lib/operator.js 77.65% <2.32%> (-22.35%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update db6d596...f8c0998. Read the comment docs.

@fengmk2
Copy link
Member

fengmk2 commented Aug 6, 2018

README 也更新一下 https://github.com/ali-sdk/ali-rds#io-queries

@fengmk2 fengmk2 self-assigned this Aug 6, 2018
README.md Outdated
changedRows: 2 }
```

- Update multiple rows with primary key: `id`
Copy link
Member

Choose a reason for hiding this comment

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

跟上面的例子重复了?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

没有重复。上面一个例子中,rows 的两个元素,都有 name 属性。下面一个例子中,一个有 name 属性,一个没有。

lib/operator.js Outdated
* @param {Array<Object>} rows Object Arrays, each Object needs a primary key `id`
* @return {object} update result
*/
proto.updateRows = function* (table, rows) {
Copy link
Member

Choose a reason for hiding this comment

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

还是需要提供一下 options.where,默认 id 是为主键,但是也有可能其他更新条件的。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的,我再修改一下

lib/operator.js Outdated
let columns = [];
let ids = [];
rows.forEach(item => {
if (!item.hasOwnProperty('id')) {
Copy link
Member

Choose a reason for hiding this comment

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

primary key 万一不是叫 id,叫 userid 怎么办?

Copy link
Contributor Author

@nodejh nodejh Aug 6, 2018

Choose a reason for hiding this comment

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

这个没有实现,目前只能根据 id 来更新

lib/operator.js Outdated
VALUES.push(ids);

const sql = this.format(SQL, VALUES);
debug('updateRows(%j, %j) \n=> %j', table, rows, sql);
Copy link
Member

Choose a reason for hiding this comment

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

这个 sql 语句最终是怎样的?可以加个注释。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的👌

@nodejh nodejh changed the title Feature: update multiple rows [TODO, please don't merge] Feature: update multiple rows. Aug 7, 2018
@nodejh
Copy link
Contributor Author

nodejh commented Aug 7, 2018

已更新。

支持通过 id 作为主键进行更新,也支持传入自定义的 where

// updateRows(table, options)

// update rows with primary key `id`
let options = [{
  id: 123,
  name: 'fengmk2',
  email: 'm@fengmk2.com',
  otherField: 'other field value',
  modifiedAt: db.literals.now, // `now()` on db server
}, {
   id: 124,
  name: 'fengmk2_2',
  email: 'm@fengmk2_2.com',
  otherField: 'other field value 2',
  modifiedAt: db.literals.now, // `now()` on db server
}]

// update rows with `row` and `where` properties
options = [{
  row: {
    email: 'm@fengmk2.com',
    otherField: 'other field value',
    modifiedAt: db.literals.now, // `now()` on db server
  },
  where: {
    id: 123,
    name: 'fengmk2',
  }
}, {
  row: {
    email: 'm@fengmk2_2.com',
    otherField: 'other field value2',
    modifiedAt: db.literals.now, // `now()` on db server
  }, 
  where: {
    id: 124,
    name: 'fengmk2_2',
  }
}]

@nodejh nodejh changed the title [TODO, please don't merge] Feature: update multiple rows. Feature: update multiple rows. Aug 7, 2018
* each Object needs a primary key `id`, or each Object has `row` and `where` properties
* e.g.
* [{ id: 1, name: 'fengmk21' }]
* or [{ row: { name: 'fengmk21' }, where: { id: 1 } }]
Copy link
Member

Choose a reason for hiding this comment

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

@nodejh 还有最后一个问题,如何 id 和 where 都不存在,会怎样?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fengmk2 会抛错。

代码实现:

if (!option.hasOwnProperty('id') && !(option.row && option.where)) {

测试用例:

it('should throw error when rows has neither primary key `id` nor `row` and `where` properties', async function() {

@fengmk2
Copy link
Member

fengmk2 commented Sep 26, 2018

@nodejh update case when 这种语法在 mysql 的帮助手册里面搜索不到?请将文档链接加到 updateRows 的注释里面吧。

@fengmk2
Copy link
Member

fengmk2 commented Sep 30, 2018

@nodejh #55 (comment) 加上这个注释,就合并发布了。

@nodejh
Copy link
Contributor Author

nodejh commented Sep 30, 2018

@fengmk2 已添加

* See MySQL Case Syntax: https://dev.mysql.com/doc/refman/5.7/en/case.html

@fengmk2 fengmk2 merged commit 859d818 into ali-sdk:master Sep 30, 2018
@fengmk2
Copy link
Member

fengmk2 commented Sep 30, 2018

晚点开电脑的时候我会发个版本

@fengmk2
Copy link
Member

fengmk2 commented Sep 30, 2018

3.1.0

fengmk2 pushed a commit to node-modules/rds that referenced this pull request Jan 30, 2024
[skip ci]

## 1.0.0 (2024-01-30)

### ⚠ BREAKING CHANGES

* In `Promise.all` case, Parallel beginTransactionScope will create isolated transactions.
* drop Node.js < 16 support

### Features

* add *beginTransactionScope(scope) ([0013a63](0013a63))
* add count(table, where) ([6286c46](6286c46))
* add get(), list(), insert(), update() ([9cae1cb](9cae1cb))
* add options.needFields, default is true ([18e0dea](18e0dea))
* add queryOne api ([ali-sdk#9](https://github.com/node-modules/myrds/issues/9)) ([19fc1bb](19fc1bb))
* add Transaction ([cfdcf26](cfdcf26))
* add unlock/lock tables ([ali-sdk#97](https://github.com/node-modules/myrds/issues/97)) ([4dc3452](4dc3452))
* add unlock/lock tables ([ali-sdk#97](https://github.com/node-modules/myrds/issues/97)) ([0a61be6](0a61be6))
* dynamic retrieval of database connection configuration ([ali-sdk#110](https://github.com/node-modules/myrds/issues/110)) ([f437efb](f437efb))
* export connection and query diagnostics_channel ([ali-sdk#111](https://github.com/node-modules/myrds/issues/111)) ([64aa75d](64aa75d))
* export sqlstring method ([ali-sdk#79](https://github.com/node-modules/myrds/issues/79)) ([2e99ab8](2e99ab8))
* impl with typescript ([ali-sdk#103](https://github.com/node-modules/myrds/issues/103)) ([1cf7814](1cf7814))
* promiseify ([ali-sdk#20](https://github.com/node-modules/myrds/issues/20)) ([e4aed30](e4aed30))
* stats 增加使用中的连接数 ([ali-sdk#115](https://github.com/node-modules/myrds/issues/115)) ([2b152a1](2b152a1))
* support custom query lifecricle ([ali-sdk#104](https://github.com/node-modules/myrds/issues/104)) ([5941c69](5941c69))
* support doomed transaction scope on test cases ([ali-sdk#58](https://github.com/node-modules/myrds/issues/58)) ([b227bc1](b227bc1))
* support end() ([b3eab93](b3eab93))
* support insert multi rows ([abb4804](abb4804))
* support mysql2 ([#1](#1)) ([eb9f391](eb9f391))
* support query(sql, object) ([ali-sdk#12](https://github.com/node-modules/myrds/issues/12)) ([a55e82f](a55e82f))
* support transaction on one request ctx ([ali-sdk#7](https://github.com/node-modules/myrds/issues/7)) ([3bd4e44](3bd4e44))
* update multiple rows ([ali-sdk#55](https://github.com/node-modules/myrds/issues/55)) ([859d818](859d818))
* use AsyncLocalStorage to refactor transaction, to make it more safe ([ali-sdk#108](https://github.com/node-modules/myrds/issues/108)) ([ae327fa](ae327fa))
* where condition support NULL value ([ali-sdk#60](https://github.com/node-modules/myrds/issues/60)) ([0d4d4ab](0d4d4ab))
* wrap generator function to promise ([ali-sdk#19](https://github.com/node-modules/myrds/issues/19)) ([fe1b4a3](fe1b4a3))

### Bug Fixes

* `where` with empty object ([ali-sdk#15](https://github.com/node-modules/myrds/issues/15)) ([db0b90e](db0b90e))
* add default value now() of `gmt_modified` and `gmt_create` ([ali-sdk#56](https://github.com/node-modules/myrds/issues/56)) ([db6d596](db6d596))
* don't export protected methods ([ali-sdk#106](https://github.com/node-modules/myrds/issues/106)) ([b2757df](b2757df))
* don't redefined sqlstring.escape ([ali-sdk#39](https://github.com/node-modules/myrds/issues/39)) ([5ca4489](5ca4489))
* export pool getter from rds client ([ali-sdk#102](https://github.com/node-modules/myrds/issues/102)) ([4048807](4048807))
* handle concurrent transaction ([ali-sdk#85](https://github.com/node-modules/myrds/issues/85)) ([d983478](d983478))
* move sql to error stack ([ali-sdk#8](https://github.com/node-modules/myrds/issues/8)) ([54349cd](54349cd))
* mysql type not found ([ali-sdk#109](https://github.com/node-modules/myrds/issues/109)) ([6a9bc45](6a9bc45))
* query parameters are not allowed to be included in where ([ali-sdk#67](https://github.com/node-modules/myrds/issues/67)) ([52147de](52147de))
* should export conn property ([ali-sdk#101](https://github.com/node-modules/myrds/issues/101)) ([37afa42](37afa42))
* support multi lifecricle hooks ([ali-sdk#105](https://github.com/node-modules/myrds/issues/105)) ([53b0a70](53b0a70))
* use master branch ([758877d](758877d))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants