Change Details

From the comments on [I9c42c87c55](https://gerrit.wikimedia.org/r/615709): Currently, when using a `JOIN` in `IDatabase::select()`, the joined tables must also be mentioned in the first argument (`$tables`). If a table is missing, the corresponding join is silently dropped. ```lang=shell-session $ phps Psy Shell v0.10.4 (PHP 7.4.3 — cli) by Justin Hileman >>> $dbr = wfGetDB( DB_REPLICA ); => Wikimedia\Rdbms\MaintainableDBConnRef {#2823} >>> $dbr->selectSQLText( [ 'a', 'b' ], [ 'x', 'y' ], [], __FUNCTION__, [], [ 'b' => [ 'JOIN', 'x = y' ] ] ) => "SELECT x,y FROM `a` JOIN `b` ON ((x = y)) " >>> $dbr->selectSQLText( [ 'a' ], [ 'x', 'y' ], [], __FUNCTION__, [], [ 'b' => [ 'JOIN', 'x = y' ] ] ) => "SELECT x,y FROM `a` " ``` This seems like a great source of bugs and exploding database servers. In my opinion, it should behave in one of two ways: 1. The joined table should only be mentioned in the `$join_conds`, removing the redundant mention in the `$tables`. As @daniel put it in [his comment](https://gerrit.wikimedia.org/r/c/mediawiki/core/+/615709/1#message-966f9f2f958f80181826c31f11ec8a60410ac3a0), “supplying multiple tables in the first argument should be deprecated.” 2. Any `$join_conds` table should be required in the `$tables` as well, and the `IDatabase` should throw an error and refuse to operate if it’s missing. (Maybe it should first be a soft or hard deprecation warning for a certain period.)

From the comments on [I9c42c87c55](https://gerrit.wikimedia.org/r/615709): Currently, when using a `JOIN` in `IDatabase::select()`, the joined tables must also be mentioned in the first argument (`$table`). If a table is missing, the corresponding join is silently dropped. ```lang=shell-session $ phps Psy Shell v0.10.4 (PHP 7.4.3 — cli) by Justin Hileman >>> $dbr = wfGetDB( DB_REPLICA ); => Wikimedia\Rdbms\MaintainableDBConnRef {#2823} >>> $dbr->selectSQLText( [ 'a', 'b' ], [ 'x', 'y' ], [], __FUNCTION__, [], [ 'b' => [ 'JOIN', 'x = y' ] ] ) => "SELECT x,y FROM `a` JOIN `b` ON ((x = y)) " >>> $dbr->selectSQLText( [ 'a' ], [ 'x', 'y' ], [], __FUNCTION__, [], [ 'b' => [ 'JOIN', 'x = y' ] ] ) => "SELECT x,y FROM `a` " ``` This seems like a great source of bugs and exploding database servers. In my opinion, it should behave in one of two ways: 1. The joined table should only be mentioned in the `$join_conds`, removing the redundant mention in the `$table`. As @daniel put it in [his comment](https://gerrit.wikimedia.org/r/c/mediawiki/core/+/615709/1#message-966f9f2f958f80181826c31f11ec8a60410ac3a0), “supplying multiple tables in the first argument should be deprecated.” 2. Any `$join_conds` table should be required in the `$table` as well, and the `IDatabase` should throw an error and refuse to operate if it’s missing. (Maybe it should first be a soft or hard deprecation warning for a certain period.)

From the comments on [I9c42c87c55](https://gerrit.wikimedia.org/r/615709): Currently, when using a `JOIN` in `IDatabase::select()`, the joined tables must also be mentioned in the first argument (`$tables`). If a table is missing, the corresponding join is silently dropped. ```lang=shell-session $ phps Psy Shell v0.10.4 (PHP 7.4.3 — cli) by Justin Hileman >>> $dbr = wfGetDB( DB_REPLICA ); => Wikimedia\Rdbms\MaintainableDBConnRef {#2823} >>> $dbr->selectSQLText( [ 'a', 'b' ], [ 'x', 'y' ], [], __FUNCTION__, [], [ 'b' => [ 'JOIN', 'x = y' ] ] ) => "SELECT x,y FROM `a` JOIN `b` ON ((x = y)) " >>> $dbr->selectSQLText( [ 'a' ], [ 'x', 'y' ], [], __FUNCTION__, [], [ 'b' => [ 'JOIN', 'x = y' ] ] ) => "SELECT x,y FROM `a` " ``` This seems like a great source of bugs and exploding database servers. In my opinion, it should behave in one of two ways: 1. The joined table should only be mentioned in the `$join_conds`, removing the redundant mention in the `$tables`. As @daniel put it in [his comment](https://gerrit.wikimedia.org/r/c/mediawiki/core/+/615709/1#message-966f9f2f958f80181826c31f11ec8a60410ac3a0), “supplying multiple tables in the first argument should be deprecated.” 2. Any `$join_conds` table should be required in the `$tables` as well, and the `IDatabase` should throw an error and refuse to operate if it’s missing. (Maybe it should first be a soft or hard deprecation warning for a certain period.)