https://bugzilla.wikimedia.org/show_bug.cgi?id=8130
--- Comment #5 from Roan Kattouw <[email protected]> 2009-04-24 19:34:06 UTC --- (In reply to comment #4) > 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. > I'm working on doing all this in the querypage-work branch; and yes, it is possible to prettify stuff and move away from raw SQL. -- 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
