← Back to team overview

maria-developers team mailing list archive

Re MDEV-21610, cleanup patch

 

Hello Igor,

> @spetrunia: your "cleanup" of commit e637355156cb28388a291b0e3a5e9ee863b2854d 
> actually changes the architecture: you forced the pushed filter to be the 
> same as the filter of SQL layer. 

I'm not sure I understand that.
I assume the layers are:

1. SQL layer
2. handler level (generic, with ha_xxx methods)
3. DS-MRR layer
4. The storage engine internals

handler::pushed_rowid_filter is at level 2.
Its value matches the rowid filter that has been pushed to level 4. 

What the code does after my fix:

   // Take the filter from level 2, put it to level 3 (DS-MRR)
   rowid_filter= h_arg->pushed_rowid_filter;

   // Inform level 4 that it doesn't need to do filtering
   // (This also affects level 2 as handler::pushed_rowid_filter will be
   //  cleared)
   h_arg->cancel_pushed_rowid_filter();


I see a slight problem here that handler->pushed_rowid_filter will be NULL
while actually storage engine + MRR (levels 2,3,4 together) will still be 
checking the rowid filter.
But this seems to be unavoidable in current design (and it was also the case 
before my cleanup patch).

> Although they are the same now for each engine where filters are allowed
> this might be not the case in future development. 

I'm not sure what scenario you have in mind, could you please elaborate?

> I agree that accessing filter from the handler layer is not a clean solution,
> but in this case the owner of SQL layer filter must be the TABLE structure. 

> Summing up: if we decide not to use a pushed filter in the engine we have to 
> take the filter from SQL layer.

So, you're suggesting that taking the filter from handler::pushed_rowid_filter
is worse than taking it from table->reginfo.join_tab ?

BR
 Sergei
-- 
Sergei Petrunia, Software Developer
MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog