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 DriverError::StmtParamsNumberExceedsLimit #327

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

stephen-hlx
Copy link

When comparing the number of params in a statement against the number of params supplied, if they do not match, the error is supposed to provide what these two numbers are for users to understand what might have gone wrong. This is achieved via following code path in ExecRoutine:

    if self.stmt.num_params() as usize != params.len() {
        Err(DriverError::StmtParamsMismatch {
            required: self.stmt.num_params(),
            supplied: params.len() as u16,
        })?
    }

The maximum number of bind variables in a statement supported by MySQL is 65,535 and this is guarded at the server side which means the aforementioned comparison in ExecRoutine is hit before that.

But at this point, both of these number could go over 65,535, which is the u16::MAX. In that case, when the error is constructed, the conversion from params.len()'s usize to u16 would result in a loss of precision, producing a confusing error. Because self.stmt.num_params() returns a u16 while params.len() returns a usize.

For example, if both the number of the required params and the supplied params are 65,539, which is greater than u16::MAX, self.stmt.num_params() would have returned a truncated number 3 and even with as usize would not have recovered the loss precision. Meanwhile, params.len() still returns 65,539 because it is of usize. The comparison then produces an incorrect result, saying that they do not match, even though they do. What makes it even more confusing is that in the error message there are two identical numbers because params.len() as u16 converts the usize to u16, causing the same loss of precision.

Because the self.stmt.num_params() returns a u16, which intrinsically prevents the detection of mismatched param numbers if they go over u16::MAX, and changing that type would be rather intrusive. Therefore, this commit fixes merely the symptom by providing a new error variant StmtParamsNumberExceedsLimit that represents the case when the number of provided params exceeds the limit.

Ref: https://stackoverflow.com/questions/4922345/how-many-bind-variables-can-i-use-in-a-sql-query-in-mysql-5#comment136409462_11131824

When comparing the number of params in a statement against the number of
params supplied, if they do not match, the error is supposed to provide
what these two numbers are for users to understand what might have gone
wrong. This is achieved via following code path in `ExecRoutine`:
```rust
    if self.stmt.num_params() as usize != params.len() {
        Err(DriverError::StmtParamsMismatch {
            required: self.stmt.num_params(),
            supplied: params.len() as u16,
        })?
    }
```

The maximum number of bind variables in a statement supported by MySQL
is `65,535` and this is guarded at the server side which means the
aforementioned comparison in `ExecRoutine` is hit before that.

But at this point, both of these number could go over `65,535`, which is
the `u16::MAX`. In that case, when the error is constructed, the
conversion from `params.len()`'s `usize` to `u16` would result in a loss
of precision, producing a confusing error. Because
`self.stmt.num_params()` returns a `u16` while `params.len()` returns a
`usize`.

For example, if both the number of the required params and the supplied
params are `65,539`, which is greater than `u16::MAX`,
`self.stmt.num_params()` would have returned a truncated number `3` and
even with `as usize` would not have recovered the loss precision.
Meanwhile, `params.len()` still returns `65,539` because it is of
`usize`. The comparison then produces an incorrect result, saying that
they do not match, even though they do. What makes it even more
confusing is that in the error message there are two identical numbers
because `params.len() as u16` converts the `usize` to `u16`, causing the
same loss of precision.

Because the `self.stmt.num_params()` returns a `u16`, which
intrinsically prevents the detection of mismatched param numbers if they
go over `u16::MAX`, and changing that type would be rather intrusive.
Therefore, this commit fixes merely the symptom by providing a new error
variant `StmtParamsNumberExceedsLimit` that represents the case when the
number of provided params exceeds the limit.

Ref: https://stackoverflow.com/questions/4922345/how-many-bind-variables-can-i-use-in-a-sql-query-in-mysql-5#comment136409462_11131824
@stephen-hlx
Copy link
Author

The failed test seems to be a flaky one?
Do you know how to retry it?

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.

1 participant