Skip to content

Drop support for max_length in mysqli_fetch_fields() #6512

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

Closed
wants to merge 5 commits into from

Conversation

nikic
Copy link
Member

@nikic nikic commented Dec 14, 2020

Retain the field, but always populate it with zero. This was
already the case for PS without length updating.

max_length has nothing lost in the field metadata -- it is a
property of the specific result set, and requires scanning the
whole result set to compute. PHP itself never uses max_length
with mysqlnd, it is only exposed in the raw mysqli API.

Keeping it for just that purpose is not worthwhile given the costs
involved. People who actually need this for some unfathomable
reason can easily calculate it themselves, while making it obvious
that the calculation requires a full result set scan.

@nikic nikic force-pushed the mysqli-max-length branch from 193cf2c to 73a4e83 Compare December 14, 2020 15:09
@nikic
Copy link
Member Author

nikic commented Dec 14, 2020

cc @kamil-tekiela Do you know whether max_length is important for some PHP-level application?

Dropping this will allow simplifying our implementation of buffered result sets, and making them use less memory. Currently this is not possible because max_length requires the whole result set to be decoded up front.

@nikic nikic force-pushed the mysqli-max-length branch from 73a4e83 to 64df3e2 Compare December 15, 2020 14:57
@nikic
Copy link
Member Author

nikic commented Dec 15, 2020

Second commit is the cleanup of the result set implementation. Based on this I think we'll be able to a) support PS cursors + get_result and b) typed values in PDO MySQL with emulated PS.

Retain the field, but always populate it with zero. This was
already the case for PS without length updating.

max_length has nothing lost in the field metadata -- it is a
property of the specific result set, and requires scanning the
whole result set to compute. PHP itself never uses max_length
with mysqlnd, it is only exposed in the raw mysqli API.

Keeping it for just that purpose is not worthwhile given the costs
involved. People who actually need this for some unfathomable
reason can easily calculate it themselves, while making it obvious
that the calculation requires a full result set scan.
@nikic nikic force-pushed the mysqli-max-length branch from 64df3e2 to 48202a7 Compare December 16, 2020 11:42
@kamil-tekiela
Copy link
Member

Ok, I have done some research and I think we can drop this field, but this should not be done in a patch version. We can include this change in PHP 8.1.

From my initial analysis, I came to the conclusion that this field probably has no use in PHP at all. I was not able to find any sample usage online. The goal of mysqli was to retain as much backwards compatibility with the C API as possible. MySQL C API exposes this value, and it might have certain applications in C/C++ code. However, PHP is higher level and knowing the length of the columns and/or data in bytes is not very useful. This also applies to the length property although we should leave that one alone.

Taking a look at other values exposed by MySQL C API we can see that not all of them are useful for PHP developers. We can probably also hide catalog since the value is always constant. The most used one seems to be the table/column name field.

In terms of all other changes, you have included in the PR I need to test them out first. On the surface, they look ok, but I don't see how it helps with cursor fetches. Let me dig deeper into it now.

@nikic
Copy link
Member Author

nikic commented Dec 16, 2020

Ok, I have done some research and I think we can drop this field, but this should not be done in a patch version. We can include this change in PHP 8.1.

Yeah, this is targeted at PHP 8.1.

From my initial analysis, I came to the conclusion that this field probably has no use in PHP at all. I was not able to find any sample usage online. The goal of mysqli was to retain as much backwards compatibility with the C API as possible. MySQL C API exposes this value, and it might have certain applications in C/C++ code. However, PHP is higher level and knowing the length of the columns and/or data in bytes is not very useful. This also applies to the length property although we should leave that one alone.

On the C API level, max_length exists because users of the C API need to preallocate a buffer of the correct size to fetch a row. max_length determines the smallest buffer size that can be used for all rows. Of course, PHP developers don't need to allocate any buffers, PHP itself takes care of that, so the value is meaningless to them.

Taking a look at other values exposed by MySQL C API we can see that not all of them are useful for PHP developers. We can probably also hide catalog since the value is always constant. The most used one seems to be the table/column name field.

In terms of all other changes, you have included in the PR I need to test them out first. On the surface, they look ok, but I don't see how it helps with cursor fetches. Let me dig deeper into it now.

I originally thought this will help cursors by removing all the code duplication for reading of result sets, so we could handle cursors in a central location. However, I then realized that we can solve the cursor problem more easily (and more correctly) by instructing the cursor to fetch all rows, and then forgetting about it. This is what I implemented in #6518.

@nikic nikic closed this Dec 17, 2020
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.

2 participants