Rework MyRocks transaction DDL flag

[laurynas-biveinis]

class Rdb_transaction had a field, m_ddl_transaction, which was effectively a cached flag for the current SQL statement belonging to a certain subset of DDL commands. This was used primarily to check whether key uniqueness on write can be skipped (and secondarily to skip the "Applier is committing transaction without binlog position" warning).

As a cached value, it was not always set when it should have been, which is possible to see by adding asserts at the flag read sites that check the flag value against the THD SQL statement type: rocksdb.commit_without_prepare, rocksdb.add_index_inplace_sstfilewriter. While it is possible to add the missing updates, it is much simpler not to cache it in the first place and just check the THD SQL statement type directly.

At the same time, as the DD tables are being converted to MyRocks, and system tables are planned too, many more commands can skip the uniqueness checks.

Thus remove the flag, remove its setters and getters, and introduce a helper is_safe_for_wb with a compilation-time lookup array.

[laurynas-biveinis]

I am not sure whether the condition for "Applier is committing transaction without binlog position info" is now correct, as the DDL guard has been replaced with a much wider one.

@luqun, this PR is also on my critical path for the DDSE

[luqun]

I am not sure whether the condition for "Applier is committing transaction without binlog position info" is now correct, as the DDL guard has been replaced with a much wider one.

yeah, the first scenario for is_ddl_transaction is for skip uniq check, although try to figure out why DDL trx can skip uniq check..

the 2nd scenario is to exclude trx which doesn't update binlog pos..

maybe the common one between these two scenario is DDL?

[facebook-github-bot]

@luqun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[laurynas-biveinis]

yeah, the first scenario for is_ddl_transaction is for skip uniq check, although try to figure out why DDL trx can skip uniq check..

I believe the idea there is that any uniqueness conflicts must have been detected earlier in DDL and other administrative statements, where the write is checked and initiated by the server rather than by the user.

[luqun]

For 2nd case, I think use thd_sqlcom_can_generate_row_events() may be better. but one catch is about create table..

[laurynas-biveinis]

The 2nd case is split to https://github.com/facebook/mysql-5.6/pull/1368, I will rebase/update this PR once that one is merged