https://bugzilla.wikimedia.org/show_bug.cgi?id=22093

Ryan Biesemeyer <[email protected]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #6957|0                           |1
        is obsolete|                            |

--- Comment #8 from Ryan Biesemeyer <[email protected]> 2010-04-26 23:44:22 UTC 
---
Created an attachment (id=7335)
 --> (https://bugzilla.wikimedia.org/attachment.cgi?id=7335)
DatabaseMssqlNative and supporting files

I've taken the recommendations and implemented them as best I could against the
current code base (r.65560). Some of the issues still exist, but the inline
comments reflect why some portions are still not quite in line with the
suggestions. 

A few notes:

DatabaseMssqlNative::select() and DatabaseMssqlNative::selectSQLText()
AryehGregor says that I shouldn't need to override either of these, and I agree
that the old patch was horridly verbose and duplicated the logic several times.
Instead of following the suggestion, which would have caused my patch to affect
all of the Database* classes that are currently working, I took a cue from
MaxSem's suggestion in b#23330 and re-wrote the methods from the original patch
to inherit the logic from the parent and either adjust the setup beforehand or
the output afterhand where appropriate. I think this helps isolate the
change-set and uses inheritance much better than the patch did previously

DatabaseMssqlNative::LimitToTopN()
I agree that this should not be needed, and that its implementation was less
than ideal, duplicating logic and doing a lot of extra work, but because we
still can't depend on extensions to use the SQL-generation functions, I think
this functionality is still needed as a last resort. I rewrote the function to
parse a LIMIT clause, remove it, and then pass the resulting parameters to the
existing DatabaseMssqlNative::limitResult() method, which I feel is a much
cleaner and lighter-weight way of implementing a necessary evil. 

DatabaseMssqlNative::doQuery() - Epoch magic.
I don't think this is the optimal solution, but since we can't rely on
extensions to not use this, I'm leaving it in place. The two known caller
functions have been updated to make the right calls.


This patch is against TRUNK rev 65560 and for feedback only. I'll be spending
the next few days going through and doing extensive bug-testing and will do my
best to implement your suggestions before committing to trunk.

I appreciate your feedback.

-yaauie

-- 
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

Reply via email to