On Fri, Feb 1, 2019 at 3:16 AM Sergei Golubchik <serg@mariadb.org> wrote:

Hi, Nikita!

On Jan 30, Nikita Malyavin wrote:
> >
> > #define ER_NO_CONST_EXPR_IN_RANGE_OR_LIST_ERROR ER_NOT_CONSTANT_EXPRESSION
> >
> This error seems to be unused... Considering that, do You still want the
> macro added to mysql.h?

Okay, if it's never issued, then no need to add the define.

✅

> > > + if (need_update && !record_was_same && table_list->has_period())
> >
> > I suspect this should happen even if record_was_same. The standard never says
> > "if new values are the same as old values, don't update anything"
> >
> Yes, looks like it never says so. And it was implemented in that way.
> Those two checks -- need_update && !record_was_same -- could be safely
> omitted, they don't change the behavior.
> Because in that way lcond and rcond are false, anyway.
>
> Maybe it's better to remove them -- the optimization is quite arguable
> here. Up to You, ok?

yes, let's remove. it's not an optimization even, but a bug,
because it changes behavior from what the standard says.

It doesn't change the bahavior.

Proof. Let's consider record_was_same:

it is true iff the result from ha_update_row was HA_ERR_RECORD_IS_THE_SAME

=> record[0] equals record[1] => neither BSTARTVAL nor BENDVAL changed.

Consequently, 15.3 <Effect of replacing rows in base tables>, General Rules, 10) c)ii and d)ii do not apply, so no INSERTs should be done. And they are not done anyway, because BSTARTVAL check (along with BENDVAL) is done inside insert_portion_of_time.

Same applies to need_update with the following note: if it's false, then no ha_update_row is called, because record[0] will be equal record[1]. The thing here is that BSTARTVAL is updated to FROMVAL (BENDVAL same) before that check

That all was quite useless of course, because anyway i'm going to remove those checks, because the only thing they are doing is avoiding two datetime compaisons known to be false in that case:)

Yours truly,

Nikita Malyavin