https://bugzilla.wikimedia.org/show_bug.cgi?id=8130
--- Comment #4 from [email protected] 2009-04-24 19:31:34 UTC --- Yes, the code may be ugly. But the patch is not the source of the ugliness -- the existing SpecialPage classes all pass around raw SQL fragments and make minimal use of the Database class functions. In a nutshell, trying to refactor the code to make it pretty would be a major code revamp -- with no performance or functionality benefits -- and refactoring the code would by itself still do nothing to fix this bug. I'm not trying to provide the ultimate, perfect code; I'm just trying to provide some progress towards fixing a two-year old bug. Some of the fundamental obstacles complicating a conversion of all this code to use the Database class functions include: * Some 32 special pages are based on QueryPage. Any comprehensive fix requires revamping all of those pages, plus QueryPage.php and PageQueryPage.php. Even worse, a comprehensive fix is likely to break any extension special pages that create special pages derived from QueryPage -- possibly 235 extensions based on Mediawiki-registered extensions alone. * QueryPage::getSQL is supposed to return raw SQL. If getSQL is changed to instead return an array of ($table, $var, $cond, $fname, $options), then every use of getSQL is made uglier, because it's now passing around 5+ parameters instead of just one. The other option is that getSQL still returns raw SQL, but that the various implementations of getSQL use Database functions to generate that raw SQL -- however, that's not currently possible, because the Database class does not have functions that construct a raw SQL query and return it -- without actually executing the SQL query. * Many of the existing SQL queries cannot be handled by the Database functions. See SpecialDisambiguations, for example, where the page table is accessed twice within a single query, and therefore it is aliased ("FROM {$page} AS pb, {$pagelinks} AS la, {$page} AS pa"). The database functions are unable to handle table aliases. Without major revisions to Database, it is impossible to change all of the special pages to use the database functions -- and changing half of the special pages to use one syntax and leaving half using the old syntax is even uglier, in my opinion. After all of that is done, it's still necessary to figure out how to fix this bug. * The "prettiest" way to do it would seem to be adding it as a flag recognized by Database::makeSelectOptions. However, burying it that deep within the Database class then means it's unavailable to queries that are not entirely constructed by the Database functions -- such as SpecialDisambiguations (unless the database functions are made fully alias-aware). So then a separate function is needed outside of makeSelectOptions that spits out raw SQL fragments -- so why not just introduce that separate function right now instead of forcing a code revamp that won't even get rid of the separate function? * I added isContentQuery to the MWNamespace class because the necessary information is ''not'' part of the database: the list of content namespaces is defined solely through a global variable, $wgContentNamespaces. With my patch, all uses of $wgContentNamespaces are centralized in Namespace.php -- only MWNamespace needs to know the inner details of the content namespaces are defined. I'm willing to update my patch if provided with specific suggestions about what could be done differently within the scope of this approach. But I'm not prepared to take on a huge project to fix something that's basically unrelated to this bug. Especially since with my superficial knowledge of the MW codebase, my efforts to tackle such a huge revamp would inevitably add a whole new stack of bugs. -- Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the assignee for the bug. You are on the CC list for the bug. _______________________________________________ Wikibugs-l mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
