> Zend_Db_Adapter_Sqlsrv
> 1) I see some references to Oracle and things like tnsnames.ora still in the
> comments.
I will remove these pronto. Sounds like I need to really read the
comments that are in there instead of speed-reading them :)
> 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.
Well, the issue here is that we cant know what constants are valid as
driver options (I guess we could make a list, but then the adapter would
have to maintain that list).
The issue is that if we wanted to throw an exception, we'd have to
supress any errors coming out of sqlsrv_connect() function. Considering
we dont fully know ALL of the different errors that could be thrown out
of that function, its probably best to not supress them.
Ideally, if you were using Zend_Config, you would be able to have a
section such that only the specific driver options for SQL Server
extension are passed to sqlsrv_connect().
Does that make sense?
> 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.
Yeah, I see no real need to have a return value in this method since its
a protected method.
> 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.
> ...
> with CTEs, etc. On large tables, it can be the difference between queries
> that finish in seconds and queries that finish in hours.)
I guess this is two part. One is that if you think the behavior of
limit() is wrong, lets try and get a test script going that will produce
the bad behavior, that way i can unit test it, and also see if we have
bad logic inside other adapters as well.
Secondly though, I dont know nearly enough about 2005 and 2008
differences to be able to say that one method is better than another
with regards to who limit() works. Perhaps you have a proposal here?
> 5) lastInsertId can be tricky because of scope, but it looks like what
> you've done here is better than what I used. :-)
Cool!
>
> 6) getServerVersion is returning the driver version, not the database server
> version.
hmm, you might need to submit a bug for this. How would one get the
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. :-\
Hmm, could you explain a little more here?
> 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?
I'd have to pass this onto the original contributor and ask why.
> 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?
I have yet to build an application with Sqlsrv, so I wouldnt know the
pain points a developer might encounter with regards to errors and
exceptions. Perhaps this would be a good opportunity to speak with the
contributor who contributed this adapter/driver? Would you be
interested in that?
> Thoughts?
>
> Andrew
没有评论:
发表评论