maria-developers team mailing list archive

[Nikita Malyavin <nikitamalyavin@xxxxxxxxx>]

Sorry, the assertion should be
DBUG_ASSERT(table->file == this || dynamic_cast<ha_innobase*>(table->file)
== NULL);

Well, the tests seem passing after adding this assertion (and a similar one
for myisam):
https://github.com/MariaDB/server/commit/f221bc46e6981c9e7c5d2806faefb1ed086e7bf8

Okay, I think I can agree on this approach, but then I want these
assertions to be committed as well.
dynamic_cast can be replaced with hton comparison, of your choice.

Maybe somebody later will need row changes from a separate handler, imagine
for example cascade
foreign key updates, which would have used a lookup handler for the cascade
changes. For that case,
I'll try to gather these table->file comparisons under a single method.

Alternatively, we could introduce some bool handler::root_handler just now,
without waiting for a demand,
but I'm afraid it could become another source for bugs. So better later on
demand.



On Mon, 8 May 2023 at 22:49, Sergei Golubchik <serg@xxxxxxxxxxx> wrote:

> Hi, Nikita,
>
> On May 08, Nikita Malyavin wrote:
> > On Fri, 5 May 2023 at 20:46, Sergei Golubchik <serg@xxxxxxxxxxx> wrote:
> >
> > > >  #ifdef HAVE_REPLICATION
> > > > -  if (unlikely(!error && table->s->online_alter_binlog))
> > > > +  if (unlikely(!error && table->s->online_alter_binlog &&
> > > > +               !table->skip_online_logging))
> > >
> > > this doesn't need any juggling with skip_online_logging.
> > > This problem is already solved for, say, long uniques. You can use
> > >
> > >    this == table->file
> > >
> > > condition to filter out individual partitions and have your
> > > binlog_log_row_online_alter called only for the main ha_partition
> > > handler.
> >
> > This doesn't look reliable -- won't there be any cases when we make a
> > change from another handler, not table->file?
>
> So far, that's how it works - updates come through table->file
>
> > If that never happens, we could add DBUG_ASSERT(this == table->file) in
> > ha_innodb's/ha_myisam's ha_*_row methods.
>
> Of course, not. That's the whole point. The server calls
> ha_partition::ha_write_row, which calls, in turn,
> ha_innodb::ha_write_row. For individual partitions this != table->file,
> that's how we make sure that certain code (e.g. long unique checks) are
> only run once and not repeated per partition.
>
> Regards,
> Sergei
> VP of MariaDB Server Engineering
> and security@xxxxxxxxxxx
>


-- 
Yours truly,
Nikita Malyavin