2009年7月29日星期三

[fw-db] Comments on Zend_Db_Adapter_Sqlsrv

I'm glad to see an adapter for the SQL Server Driver for PHP (Sqlsrv) being
added to Zend_Framework with the upcoming 1.9 release. Overall they look
similar to what I have been using, so it will be good to see them become
part of the core. After looking at RC1, I have a few observations:


Zend_Db_Adapter_Sqlsrv
1) I see some references to Oracle and things like tnsnames.ora still in the
comments.

2) In _connect(), I think it would be more efficient for the adapter to
throw an exception on any driver options that are not supported by the DLL
rather than passing them off to sqlsrv_connect() and letting it return an
error.

3) I see that the transaction handlers (_beginTransaction, _commit and
_rollback) now return void and raise exceptions on failure. I simply had
them pass back the return value of the corresponding sqlsrv_ function, but
it appears that Zend_Db_Adapter_Abstract now returns a self reference to
provide a fluent interface. I guess this implementation is consistent with
the change in the abstract base class.

4) The implementation of limit() looks like it uses a pretty common
technique I have seen in other algorithms, but I think it contains a slight
flaw regarding the last "page" in the result set.

For example if the total rows matching the WHERE clause is 55, the value of
rows-per-page is 10, and selected page number is 6, the inner query would
request the top 60 (offset + rows = 50 + 10) rows. Since there are only 55
total rows, the inner query will return all 55 rows. The outer query then
takes the bottom 10 of the set returned by the inner query. In this case,
that would be rows 46-55 rather than just 51-55.

In my class, I started with this approach, but I modified it to add an
additional WHERE condition so that it would only grab the rows in the bottom
10 that were not also part of the top 50. Due to its complexity I left room
for the adapter to select alternate algorithms [yet to be added] for queries
against newer versions since there are now better ways to implement this in
2005 and 2008. (The trouble is, depending on the tables you may get better
or worse performance by using either row_number() or by various approaches
with CTEs, etc. On large tables, it can be the difference between queries
that finish in seconds and queries that finish in hours.)

5) lastInsertId can be tricky because of scope, but it looks like what
you've done here is better than what I used. :-)


6) getServerVersion is returning the driver version, not the database server
version.

Zend_Db_Statement_Sqlsrv
1) I like the addition of the class constants for the parameter directions.
Perhaps the other constants should be added as well? A couple of them
probably have to be implemented as static functions, rather than members of
that enumeration array though. :-\

2) I'm curious why internally fetch() always fetches a row as two separate
arrays (keys and values) and then uses the fetch-mode to determine whether
to discard one and return the other, or merge them together (and optionally
cast the combined array to an object). Is that faster than calling either
sqlsrv_fetch_array with the appropriate 2nd parameter or
sqlsrv_fetch_object?


Error handling:
I am glad to see the full array returned by sqlsrv_errors() being passed to
the exception constructors. A single query could return multiple error
messages, especially from stored procedures, and often the last error
message returned is simply "Statement failed." I wish there was a good way
to be able to access either the full array of messages and codes or specific
items in the errors array rather than just getting the first/last code
returned.

I'm not sure either of these are good ideas, but what about either having
getMessage() return each() on the message array or else providing
getMessages() and getCodes() as additional methods on those classes?

Thoughts?

Andrew
--
View this message in context: http://www.nabble.com/Comments-on-Zend_Db_Adapter_Sqlsrv-tp24725644p24725644.html
Sent from the Zend DB mailing list archive at Nabble.com.

没有评论: