Lucas_Werkmeister_WMDE added a comment.

  > - This issue would have been caught immediately by DBAs or other developers 
on review and asked to be changed immediately. This leads to the more important 
issue that **review process may be broken** because not even the most obvious 
formatting issue has been corrected on review. I am not looking to assign any 
blame here, I am asking to review the reviewing process for database layer 
changes so obvious mistakes like these are caught on time.
  
  The earliest version of this code seems to date back to me (Ide4f3d1644 
<https://gerrit.wikimedia.org/r/512705>), and if I recall correctly, I was 
unsure about how to do a JOIN properly and found the documentation to be 
extremely confusing. This part of IDatabase::select 
<https://gerrit.wikimedia.org/g/mediawiki/core/+/c2aa38f46b769839d043d037165a22a91f232fdd/includes/libs/rdbms/database/IDatabase.php#592>
 is probably what led to the current code: (emphasis added):
  
  > @param string|array $join_conds Join conditions: Optional associative array 
of table-specific join conditions. **In the most common case, this is 
unnecessary, since the join condition can be in $conds.** However, it is useful 
for doing a LEFT JOIN.
  
  Based on that sentence, I would even today vote //against// the change Amir 
just uploaded if it wasn’t for this ticket, because it seems to fly directly in 
the face of this documentation: why use `$join_conds` if the documentation says 
it’s unnecessary except for a `LEFT JOIN` that we don’t want?

TASK DETAIL
  https://phabricator.wikimedia.org/T246232

EMAIL PREFERENCES
  https://phabricator.wikimedia.org/settings/panel/emailpreferences/

To: Ladsgroup, Lucas_Werkmeister_WMDE
Cc: Lucas_Werkmeister_WMDE, Addshore, Aklapper, Marostegui, Ladsgroup, jcrespo, 
Alter-paule, Hazizibinmahdi, Beast1978, Un1tY, Akuckartz, Hook696, Iflorez, 
darthmon_wmde, Kent7301, alaa_wmde, joker88john, CucyNoiD, Nandana, jijiki, 
Klaas_Z4us_V, Gaboe420, Giuliamocci, Cpaulf30, Lahi, Gq86, Af420, Bsandipan, 
GoranSMilovanovic, QZanden, LawExplorer, Lewizho99, Maathavan, elukey, _jensen, 
rosalieper, Scott_WUaS, Jonas, Wikidata-bugs, aude, Lydia_Pintscher, Mbch331, 
Jay8g
_______________________________________________
Wikidata-bugs mailing list
Wikidata-bugs@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs

Reply via email to