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

Alternative for "ResultSetHeader.changedRows" #2456

Closed
CComparon opened this issue Feb 26, 2024 · 2 comments
Closed

Alternative for "ResultSetHeader.changedRows" #2456

CComparon opened this issue Feb 26, 2024 · 2 comments

Comments

@CComparon
Copy link

CComparon commented Feb 26, 2024

Hello gents,

In recent versions of mysql2, the changedRows result set header is not deprecated (see below).
The documentation says to use affectedRows instead, which really isn't the same information though.
Curious about the rationale for this deprecation and possible alternative for this property?

Many thanks

declare interface ResultSetHeader {
  constructor: {
    name: 'ResultSetHeader';
  };
  affectedRows: number;
  fieldCount: number;
  info: string;
  insertId: number;
  serverStatus: number;
  warningStatus: number;
  /**
   * @deprecated
   * `changedRows` is deprecated and might be removed in the future major release. Please use `affectedRows` property instead.
   */
  changedRows: number;
}
@wellwelwel
Copy link
Sponsor Collaborator

Hey @CComparon 🙋🏻‍♂️

Copying it from #2123 (comment):

@wellwelwel is there a way to deprecate a field in TS? Quick googling gives me jsdock comment option "@deprecated reason description" - is this the best option?

Reasons I want to remove completely changedRows:

  • it's confusing given affectedRows exist
  • not part of the protocol and is extracted from human readable message. Will break if wording is changed or server is localised or non-mysql server ( MariaDB etc ) choses to use different message.

Re 0 vs undefined when message can't be parsed - probably not that important right now

You can also follow this discussion in #2122, from #2122 (comment) onwards.

@CComparon
Copy link
Author

Thanks a ton @wellwelwel for your quick reply.
"Not part of the protocol" comes as a surprise as this information is interesting for multiple use cases, but I get it now.
Cheers

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

No branches or pull requests

2 participants