Hello Sergei, On 10/25/21 11:03 PM, Sergei Golubchik wrote:
Hi, Alexander!
On Oct 25, Alexander Barkov wrote:
MDEV-26732 Assertion `0' failed in Item::val_native
Also fixes MDEV-24619 Wrong result or Assertion `0' in Item::val_native / Type_handler_inet6::Item_val_native_with_conversion
would be good to have some explanation here, what was wrong, what was the fix.
Right, I suggest this comment:
MDEV-26732 Assertion `0' failed in Item::val_native
Also fixes MDEV-24619 Wrong result or Assertion `0' in Item::val_native / Type_handler_inet6::Item_val_native_with_conversion
Type_handler::create_item_copy() created a generic Item_copy_string, which does not implement val_native() - it has a dummy implementation with DBUG_ASSERT(0), which made the server crash.
Why does it call val_native() if Item_copy_string doesn't implement it?
Because I forgot to implement Item_copy_inet6 - I missed the reported scenario in my original test coverage for INET6. The default implementation in Item::val_native(), which is called for Item_copy_string in this scenario, intentionally consists only of DBUG_ASSERT. If it had some working implementation, returning the expected value, it would hide a performance problem: Conversion from the string INET6 representation to the binary INET6 representation would happen per row. This is not desirable. So the DBUG_ASSERT is a trap to find such places not implementing fast code paths for the native representation.
If the item was a string in the first place, that is, if Item_copy_string would've been used correctly where it should've been used, then val_native() wouldn't be called? What would it use, val_str()?
Yes, val_str().
So why does it use val_native() here, because that Item_copy_string uses Type_handler_inet6?
Correct.
Fix:
- Adding a new class Item_copy_inet6, which implements val_native(). - Fixing Type_handler::create_item_copy() to make Item_copy_inet6 instead of Item_copy_string.
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org