fix(mysql): restore bind buffer length for newer mariadb versions

[simonzkl]

No description provided.

[weiznich]

I think the better solution would be to replace the self.lenght value here:

diesel/diesel/src/mysql/connection/bind.rs

Line 419 in d51ee0d

self.length.try_into().expect("Usize is at least 32 bit"),

By conditionally checking if the type has a statically known size via known_buffer_size_for_ffi_type:

diesel/diesel/src/mysql/connection/bind.rs

Line 739 in d51ee0d

fn known_buffer_size_for_ffi_type(tpe: ffi::enum_field_types) -> Option<usize> {

The tricky bit is to reason about whether the buffer is sized that many bytes (which likely can be checked looking at the capacity field of the BindData struct) and if all possible byte patterns are valid for the target type.

[simonzkl]

Yeah I was just hacking around. That sounds good though, I'll give it a shot.

[simonzkl]

The tricky bit is to reason about whether the buffer is sized that many bytes (which likely can be checked looking at the capacity field of the BindData struct) and if all possible byte patterns are valid for the target type.

I don't really understand where this would be an issue. The way I see it we already set the correct capacity for fixed-sized types when we first initialise BindData. MariaDB then resets the length, but the underlying buffer/capacity should stay untouched, no? And dynamically-sized types should already be handled as the breaking mariadb change only affected fixed-sized types.

Though I think it would be safer to set the correct self.length right after calling the mysql ffi functions. It's hard to tell which other functions in the module rely on self.length being correct.

[weiznich]

At least I would expect some checks that the length is smaller or at least equal to the capacity. It should be always the case, but given what the mariadb team did here I would rather not trust that the library behaves in any particular way. Better have an assert for this.

The other question is still: Is any type that we eventually construct from this byte buffer able to represent any possible byte pattern? From a short look it seems like this is the case, but that needs to be verified.

---

Though I think it would be safer to set the correct self.length right after calling the mysql ffi functions. It's hard to tell which other functions in the module rely on self.length being correct.

Diesel always consumes these data via MysqlValue and that's the only location where we actually construct this type. Trying to fix the length directly after calling into libmsqlclient would be rather messy as we have quite a few call sides and we always deal with a list of binds. This place seems like a better solution.