Skip to content

Deprecate unused mysqli constants #6850

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

Merged

Conversation

kamil-tekiela
Copy link
Member

@kamil-tekiela kamil-tekiela commented Apr 9, 2021

Technically this is BC, but if anyone uses these constants in their code I would sincerely want to know why. From what I can tell all of them have been dead for years, if they ever were in use.
Removed:

  • MYSQLI_SERVER_QUERY_NO_GOOD_INDEX_USED
  • MYSQLI_SERVER_QUERY_NO_INDEX_USED
  • MYSQLI_SERVER_QUERY_WAS_SLOW
  • MYSQLI_SERVER_PS_OUT_PARAMS
  • MYSQLI_DATA_TRUNCATED
  • MYSQLI_NO_DATA

It's possible they were used in the past or that I am wrong. If anyone knows if they are used for anything please let me know.

#endif
#ifdef SERVER_PS_OUT_PARAMS
REGISTER_LONG_CONSTANT("MYSQLI_SERVER_PS_OUT_PARAMS", SERVER_PS_OUT_PARAMS, CONST_CS | CONST_PERSISTENT);
#endif
Copy link
Member

Choose a reason for hiding this comment

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

Just to confirm, there is no way to access server_status from mysqli?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know of any way to use these constants.

I was not aware that we can make constants deprecated. I will try to rewrite this PR now to make them deprecated instead of removing them.

@nikic
Copy link
Member

nikic commented Apr 12, 2021

I think that apart from ON_UPDATE_NOW_FLAG this is okay.

Rather then dropping the constants entirely, we could also mark them as CONST_DEPRECATED, now that we have support for it. If there's no particular urgency here, doing that would be more in line with usual process.

@kamil-tekiela kamil-tekiela force-pushed the Remove-unused-mysqli-constants branch from 055fa41 to a943012 Compare April 12, 2021 12:51
@kamil-tekiela kamil-tekiela changed the title Remove unused mysqli constants Deprecate unused mysqli constants Apr 12, 2021
2048

Deprecated: Constant MYSQLI_SERVER_PS_OUT_PARAMS is deprecated in %s
4096
Copy link
Member

Choose a reason for hiding this comment

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

I'd avoid testing specific values here -- possibly these vary by client version.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added %i instead. Is this better?

Copy link
Member

@nikic nikic left a comment

Choose a reason for hiding this comment

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

LGTM modulo some more test nits.

@kamil-tekiela kamil-tekiela force-pushed the Remove-unused-mysqli-constants branch from 73127f2 to a8d75b8 Compare April 12, 2021 18:56
@kamil-tekiela kamil-tekiela merged commit b7a298b into php:master Apr 12, 2021
jrfnl added a commit to PHPCompatibility/PHPCompatibility that referenced this pull request Mar 9, 2022
Not mentioned anywhere in the changelogs or migration guide. Still happened.

Includes unit tests.

Refs:
* php/php-src#6850
* php/php-src@b7a298b
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.

3 participants