Re: [Maria-developers] 32c73138ff4: Remove some usage of Check_level_instant_set and Sql_mode_save
![](https://secure.gravatar.com/avatar/39b623a1559cf9c69ac3d9d4fb44e7fe.jpg?s=120&d=mm&r=g)
Hi, Monty! On Apr 19, Michael Widenius wrote:
revision-id: 32c73138ff4 (mariadb-10.5.2-559-g32c73138ff4) parent(s): 07708eb9b28 author: Michael Widenius
committer: Michael Widenius timestamp: 2021-03-25 12:06:34 +0200 message: Remove some usage of Check_level_instant_set and Sql_mode_save
The reason for the removal are: - Generates more code - Storing and retreving THD - Causes extra code and daata to be generated to handle possible throw exceptions (which never happens in MariaDB code) - Uses more stack space
No, Monty. We've discussed it already, this makes code more complex, fragile and bug prone. And it does *not* generates extra code or data or stack - before writing this email I've compiled mariadbd both ways and compared the generated code for the function in question, it was *identical*.
Other things:
- Changed convert_const_to_int() to use item->save_in_field_no_warnings(), which made the code shorter and simpler. - Added thd as argument to store_key.copy() to make function simpler - Added thd as argument to some subselect* constructor that inherites from Item_subselect.
these changes are good, thanks Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
![](https://secure.gravatar.com/avatar/68cce0b6e372c2c66f4d66c6356102af.jpg?s=120&d=mm&r=g)
Hi!
On Mon, Apr 19, 2021 at 9:55 PM Sergei Golubchik
Hi, Monty!
On Apr 19, Michael Widenius wrote:
revision-id: 32c73138ff4 (mariadb-10.5.2-559-g32c73138ff4) parent(s): 07708eb9b28 author: Michael Widenius
committer: Michael Widenius timestamp: 2021-03-25 12:06:34 +0200 message: Remove some usage of Check_level_instant_set and Sql_mode_save
The reason for the removal are: - Generates more code - Storing and retreving THD - Causes extra code and daata to be generated to handle possible throw exceptions (which never happens in MariaDB code) - Uses more stack space
No, Monty. We've discussed it already, this makes code more complex, fragile and bug prone.
Yes, we discussed this but we did not reach an agreement. I say that the other version generates slopper, harder to understand code that makes it easier to do mistakes. I also HATE with passion code that does things behind the scenes. I want to understand what the code does without having to loo at every function that is used by the code.
And it does *not* generates extra code or data or stack - before writing this email I've compiled mariadbd both ways and compared the generated code for the function in question, it was *identical*.
I did the same and they are NOT identical in cases where there was more than 'one value saved'. Depends probably on compiler options. Also, if we ever would need to allow 'throw', then any usage of constructs like this gets much worse. Try and check yourself. Note that I did only change this in very simple functions and kept usage of Check_level_instant_set in other places. Regards, Monty
participants (2)
-
Michael Widenius
-
Sergei Golubchik