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

add numInsertedOrUpdatedRows to InsertResult. #188

Merged

Conversation

igalklebanov
Copy link
Member

@igalklebanov igalklebanov commented Oct 9, 2022

Also:

  • Deprecate QueryResult.numUpdatedOrDeletedRows and add QueryResult.numAffectedRows.

  • add to InsertResult.
  • unit tests.
    - [ ] typings tests. tests for InsertResult already in place.

On MySQL, this field lets consumers know whether an insert...on duplicate key update query inserted OR updated some records. Real world scenario, publishing an event following the query, "entity {created | updated}".

With ON DUPLICATE KEY UPDATE, the affected-rows value per row is 1 if the row is inserted as a new row, 2 if an existing row is updated, and 0 if an existing row is set to its current values. If you specify the CLIENT_FOUND_ROWS flag to the mysql_real_connect() C API function when connecting to mysqld, the affected-rows value is 1 (not 0) if an existing row is set to its current values.
https://dev.mysql.com/doc/refman/8.0/en/insert-on-duplicate.html

The field is also relevant to inserts in Postgres..

The rowCount is populated from the command tag supplied by the PostgreSQL server. It's generally of the form: COMMAND [OID] [ROWS]. For DML commands (INSERT, UPDATE, etc), it reflects how many rows the server modified to process the command.
https://node-postgres.com/api/result


Things to consider:

Feels like QueryResult's numUpdatedOrDeletedRows should be renamed to numAffectedRows, which is more readable, neutral and aligned with MySQL terminology.

Stopped myself from doing this change as it'll break 3rd party drivers.

@igalklebanov igalklebanov added enhancement New feature or request api Related to library's API labels Oct 9, 2022
@igalklebanov igalklebanov marked this pull request as ready for review October 18, 2022 21:02
@igalklebanov igalklebanov changed the title add affected rows to InsertResult. add numUpdatedOrDeletedRows to InsertResult. Oct 18, 2022
@igalklebanov igalklebanov changed the title add numUpdatedOrDeletedRows to InsertResult. add numInsertedOrUpdatedRows to InsertResult. Oct 19, 2022
@igalklebanov igalklebanov added the bug Something isn't working label Nov 4, 2022
@igalklebanov
Copy link
Member Author

igalklebanov commented Nov 4, 2022

  • Added numAffectedRows to QueryResult. Added @deprecated to numUpdatedOrDeletedRows jsdoc.
  • Added handling of both fields in built-in drivers & query builders.
  • Added one-time outdated driver / plugin detection and "deprecation warning" @ query executor base.

@igalklebanov igalklebanov added the deprecation Something is/should be deprecated label Nov 12, 2022
@koskimas koskimas force-pushed the add-affected-rows-to-insert-result branch from c360c94 to 7935c7e Compare December 10, 2022 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Related to library's API bug Something isn't working deprecation Something is/should be deprecated enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants