It would be useful for Query to track slightly better query verbs/flags to distinguish vastly different types of queries like DROP DATABASE, DROP TABLE and DROP INDEX. Currently, one can check getTables() to distinguish some of these (e.g. "DROP DATABASE" vs "DROP TABLE"), but not others (e.g. "DROP INDEX x ON a" vs "DROP TABLE a"). It should be easy to know if a query is a table read (DQL), table write (DML), table create, table drop, or table alter (or indexes thereof).
Also note that there can be different ways of altering things for different db engines and equivalent ways for a single engine. For example, index creation/dropping in mysql can use "ALTER TABLE a ADD index x"/"ALTER TABLE a DROP INDEX x" or "CREATE INDEX x ON TABLE a"/"DROP INDEX x ON A". It would be useful if these cases could be checked in a normalized way.
$flags is currently confusing since there are flags describing what the query does and flags that alter retry/DBO_TRX/error handling. The former could be replaced by $queryType and the later would stay in Database. QueryBuilderFromRawSql checks !$flags but there are flags other than QUERY_CHANGE_* flags that might be set. Giving $flags=0 to query() with a string $sql can cause $flags and $sql->getFlags() to diverge oddly.
I think the easiest thing is to:
- Provide Query:: constants for the $queryVerb argument to the Query constructor. These would distinguish table/index/db DDL statements. Use these in getVerb().
- Use the new constants in the callers (all RDBMS internal) when possible (things like "PRAGMA"/"ATTACH" would not have constants).
- Deprecate some QUERY_ flags that aren't needed anymore
Based on comments in https://gerrit.wikimedia.org/r/c/mediawiki/core/+/946538 and things I noticed looking at Query::getVerb() callers. Also, since 9e6fa0d2253c291e583cbe4366a6f2e41c94f958 ,getTempTableWrites() no longer distinguishes "CREATE" from "CREATE TEMPORARY" anymore (which happens to work since unit tests use TEMP_PSEUDO_PERMANENT and the only other thing sending CREATE is schema change scripts which still end up working give what sessionTempTables is used for).
I also notice that isWriteQuery() was made to return false for GET_LOCK() to avoid TransactionProfiler logging, which OK, except we lack enforcement of read-only mode on DB_REPLICA connections (which would blow up in production). executeQuery() should check QUERY_CHANGE_LOCKS or queryType().