Change Details

The table names in the Query class are mostly used for two things: a) For tracking what session temp tables exists, which effects whether a query counts a permanent write or not. This is used for logging and safety checks. Accommodations are made for the TEMPORARY tables used to mock permanent tables during testing, so they are treated similarly by these checks. b) For supplying names to ChangedTablesTracker. This is used for tracking what tables changing during integration tests, so that we can reset them. These tables are usually though not always TEMPORARY. For the session temp table tracking, we use a field in Database called "sessionTempTables", keyed by table name. Such table names should to be fully qualified before any simple equality comparisons. Currently, registerTempTables() and hasPermanentTable() just use tableName(...,'raw'), which resolves table prefixes/$wgSharedTables. There are a few problems here: * This does not qualify the table with the db name. Thus, it does not account for the fact temp tables on different dbs can have the same name in mysql. * We use schemas in postgres, but temp tables always go in the pg_temp schema, which makes it hard to use fully qualified table names in "sessionTempTables". Determining when to use db.pg_temp.table vs db.schema.table would be messy. Technically, sqlite uses a similar TEMP schema for temp table too, but doesn't practically matter for us unless ATTACH is used (e.g. $wgSharedTables). * These methods compare "raw" tables names with those from Query::getTable(), yet the corresponding "new Query()" callers do not prefix/qualify the table name. This means that, normally, a magic "base" table name (no prefix/$wgSharedTables applied) will be compared to the tableName(...,'raw') keys in "sessionTempTables", which does not make sense. It isn't clear how much ChangedTablesTracker::getTables( $domainId ) is supposed to be for "changed tables belonging to this domain" or "table changed while the current domain was this domain", though it seems to mostly do the later. $wgSharedTables muddles the waters too much for the first case. In the second case, it's still odd that a table from $domainId could be changed while the current domain was $otherDomain, which would then not show in ChangedTablesTracker::getTables( $domainId ). This would only happen with something like $dbw->insert( "foriegn_db.table", ... ), which shouldn't really happen, but isn't proscribed by a comments nor safety checks. It isn't documented whether ChangedTablesTracker is supposed to work with "base" table names. That seems to be the expectation though. For example, MediaWikiIntegrationTestCase::resetDB() checking against the string "user". When MediaWikiIntegrationTestCase::truncateTables() runs on those entries, this works as long as all callers of the RDBMS query wrapper methods provided "base" table names. The duplicateTableStructure() methods are special case, using a dot prefix to treat the table name as full (bypass prefix/$wgSharedTables). Pretty much all of the $table arguments to RDBMs methods should be documented as "base" names and made to refer to some common documentation of the concept.

The table names in the Query class are mostly used for two things: a) For tracking what session temp tables exists, which effects whether a query counts a permanent write or not. This is used for logging and safety checks. Accommodations are made for the TEMPORARY tables used to mock permanent tables during testing, so they are treated similarly by these checks. b) For supplying names to ChangedTablesTracker. This is used for tracking what tables changing during integration tests, so that we can reset them. These tables are usually though not always TEMPORARY. For the session temp table tracking, we use a field in Database called "sessionTempTables", keyed by table name. Such table names should to be fully qualified before any simple equality comparisons. Currently, registerTempTables() and hasPermanentTable() just use tableName(...,'raw'), which resolves table prefixes/$wgSharedTables. There are a few problems here: * This does not qualify the table with the db name. Thus, it does not account for the fact temp tables on different dbs can have the same name in mysql. * We use schemas in postgres, but temp tables always go in the pg_temp schema, which makes it hard to use fully qualified table names in "sessionTempTables". Determining when to use db.pg_temp.table vs db.schema.table would be messy. Technically, sqlite uses a similar TEMP schema for temp table too, but doesn't practically matter for us unless ATTACH is used (e.g. $wgSharedTables). * These methods compare "raw" tables names with those from Query::getTable(), yet the corresponding "new Query()" callers do not prefix/qualify the table name. This means that, normally, a magic "base" table name (no prefix/$wgSharedTables applied) will be compared to the tableName(...,'raw') keys in "sessionTempTables", which does not make sense. It isn't clear how much ChangedTablesTracker::getTables( $domainId ) is supposed to be for "changed tables belonging to this domain" or "table changed while the current domain was this domain", though it seems to mostly do the later. $wgSharedTables muddles the waters too much for the first case. In the second case, it's still odd that a table from $domainId could be changed while the current domain was $otherDomain, which would then not show in ChangedTablesTracker::getTables( $domainId ). This would only happen with something like $dbw->insert( "foriegn_db.table", ... ), which shouldn't really happen, but isn't proscribed by a comments nor safety checks. It isn't documented whether ChangedTablesTracker is supposed to work with "base" table names (no prefix/$wgSharedTables applied yet). That seems to be the expectation though. For example, MediaWikiIntegrationTestCase::resetDB() checking against the string "user". When MediaWikiIntegrationTestCase::truncateTables() runs on those entries, this works as long as all callers of the RDBMS query wrapper methods provided "base" table names. The duplicateTableStructure() methods are special case, using a dot prefix to treat the table name as full (bypass prefix/$wgSharedTables). If we want ChangedTablesTracker to use "base" table names, then pretty much all of the $table arguments to RDBMs methods should be documented as requiring "base" names and made to refer to some common documentation of the concept. This would still run into the problem of queries doing cross-table joins, which involve using remote DB table names (e.g. "db2.table"). SQLPlatform::tableName() will assumed that such tables are already prefixed and $wgSharedTables was resolved. Getting "base" names from qualified names is not possible (similar to "is this string encoded?" questions). This case would really only matter for cross-DB JOINs, which are rarely a thing in MediaWiki. It would otherwise be silly to get a db1 handle to do "SELECT ... FROM d2.table" and even more so for writes. The write methods could actually be made to disallow table names with "db.table" in them, and these are the ones that matter with regard to temp/write table tracking.

The table names in the Query class are mostly used for two things: a) For tracking what session temp tables exists, which effects whether a query counts a permanent write or not. This is used for logging and safety checks. Accommodations are made for the TEMPORARY tables used to mock permanent tables during testing, so they are treated similarly by these checks. b) For supplying names to ChangedTablesTracker. This is used for tracking what tables changing during integration tests, so that we can reset them. These tables are usually though not always TEMPORARY. For the session temp table tracking, we use a field in Database called "sessionTempTables", keyed by table name. Such table names should to be fully qualified before any simple equality comparisons. Currently, registerTempTables() and hasPermanentTable() just use tableName(...,'raw'), which resolves table prefixes/$wgSharedTables. There are a few problems here: * This does not qualify the table with the db name. Thus, it does not account for the fact temp tables on different dbs can have the same name in mysql. * We use schemas in postgres, but temp tables always go in the pg_temp schema, which makes it hard to use fully qualified table names in "sessionTempTables". Determining when to use db.pg_temp.table vs db.schema.table would be messy. Technically, sqlite uses a similar TEMP schema for temp table too, but doesn't practically matter for us unless ATTACH is used (e.g. $wgSharedTables). * These methods compare "raw" tables names with those from Query::getTable(), yet the corresponding "new Query()" callers do not prefix/qualify the table name. This means that, normally, a magic "base" table name (no prefix/$wgSharedTables applied) will be compared to the tableName(...,'raw') keys in "sessionTempTables", which does not make sense. It isn't clear how much ChangedTablesTracker::getTables( $domainId ) is supposed to be for "changed tables belonging to this domain" or "table changed while the current domain was this domain", though it seems to mostly do the later. $wgSharedTables muddles the waters too much for the first case. In the second case, it's still odd that a table from $domainId could be changed while the current domain was $otherDomain, which would then not show in ChangedTablesTracker::getTables( $domainId ). This would only happen with something like $dbw->insert( "foriegn_db.table", ... ), which shouldn't really happen, but isn't proscribed by a comments nor safety checks. It isn't documented whether ChangedTablesTracker is supposed to work with "base" table names (no prefix/$wgSharedTables applied yet). That seems to be the expectation though. For example, MediaWikiIntegrationTestCase::resetDB() checking against the string "user". When MediaWikiIntegrationTestCase::truncateTables() runs on those entries, this works as long as all callers of the RDBMS query wrapper methods provided "base" table names. The duplicateTableStructure() methods are special case, using a dot prefix to treat the table name as full (bypass prefix/$wgSharedTables). If we want ChangedTablesTracker to use "base" table names, then pretty much all of the $table arguments to RDBMs methods should be documented as requiring "base" names and made to refer to some common documentation of the concept. This would still run into the problem of queries doing cross-table joins, which involve using remote DB table names (e.g. "db2.table"). SQLPlatform::tableName() will assumed that such tables are already prefixed and $wgSharedTables was resolved. Getting "base" names from qualified names is not possible (similar to "is this string encoded?" questions). This case would really only matter for cross-DB JOINs, which are rarely a thing in MediaWiki. It would otherwise be silly to get a db1 handle to do "SELECT ... FROM d2.table" and even more so for writes. The write methods could actually be made to disallow table names with "db.table" in them, Pretty much all of the $table arguments to RDBMs methods should be documented as "base" names and made to refer to some common documentation of the conceptand these are the ones that matter with regard to temp/write table tracking.