maria-developers team mailing list archive

[Michael Widenius <michael.widenius@xxxxxxxxx>]

Hi!

On Mon, Apr 19, 2021 at 9:55 PM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:
>
> Hi, Monty!
>
> On Apr 19, Michael Widenius wrote:
> > revision-id: 32c73138ff4 (mariadb-10.5.2-559-g32c73138ff4)
> > parent(s): 07708eb9b28
> > author: Michael Widenius <michael.widenius@xxxxxxxxx>
> > committer: Michael Widenius <michael.widenius@xxxxxxxxx>
> > 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