Code review comment for lp:~gl-az/percona-server/BT-23310-bug1218664-5.5

On 9/26/2013 11:18 PM, Laurynas Biveinis wrote:
> Review: Needs Fixing
>
> - It is suspicious that we want to include PERCONA_SCHEMA. Why?
> Shouldn't it be treated as user table for our purposes?
> Also wouldn't it provide a hole around schema enforcement if
> the server is missing PERCONA_SCHEMA schema, thus user is free
> to create it? Such scenario is exactly how the MTR testcase
> currently works, isn't it?
>
> - Assuming PERCONA_SCHEMA is not desired, then there is no need
> to maintain our own list of special schemas (also what about
> case sensitivity?) There is get_table_category(), which will
> return TABLE_CATEGORY_INFORMATION for I_S,
> TABLE_CATEGORY_PERFROMANCE for P_S, TABLE_CATEGORY_SYSTEM for
> mysql, TABLE_CATEGORY_LOG for slow/general logs if tables, and
> TABLE_CATEGORY_USER. Thus it seems that we can simply call
> get_table_category() and enforce storage engine only if it
> returned TABLE_CATEGORY_USER. Note that it would cover log
> tables as well, which we haven't discussed previously, but for
> which SE enforcement is probably a bug too.
>
> - Assuming PERCONA_SCHEMA is not desired, an MTR testcase becomes
> more complicated. I'd look if it is possible to test through
> the log tables, if possible.
>
I was entirely unaware of this get_table_category(). This is IMHO the
correct way to do it and I even almost implemented such a call like
'get_schema_type' or something along those lines. I do think though that
PERCONA_SCHEMA does need some protection but it is hard to say right now
because it is so new and nobody yet seems to have any idea or opinion on
it. I suppose we could always add it in later but by then it may be too
late (closing the door after the horse is out of the barn). This is a
sticky integration point between XB and PS. By not protecting it then
yes, it does make creating a proper test case more difficult but as you
say, we could try with the log tables...