Hi Sergei, Thanks for the comment, see the new comment in the ticket for the new patch url. On Wed 2023-06-21 15:11:01 +0200, Sergei Golubchik wrote:
-#ifdef ITEM_FUNC_TIMESTAMPDIFF_ARE_PUBLIC
Why did you removed this and not the #else part?
This is part of MDEV-29269. It was a done task with a fixversion 11.0+, but there's no reason the same change shouldn't be in earlier versions. This is one of the redundant #ifdefs, where the flag is deterministically 0 or 1 (see MDEV-26178 for more context). In this case ITEM_FUNC_TIMESTAMPDIFF_ARE_PUBLIC is always undefined, so we remove the #if part and keep the #else part.
But it was *incorrectly* undefined. It was waiting for spider to have access to item_func_timestamp_diff->int_type, which was private at some point. It is private still, but there's an accessor and spider can perfectly use
switch (item_func_timestamp_diff->get_int_type())
there's no reason to remove all that code neither in 10.4 nor in 11.0
I see, I have reinstated the ifdef and updated the check_item_func() accordingly. I will update the patch for other server versions after the review is done. Since this patch should focus on the conservative refactoring while fixing MDEV-29447 and MDEV-31338, I will do the removal of the flag and keeping the ifdef branch etc. in a separate patch, probably for MDEV-30504. Best, Yuchen