On Thu, 2009-12-31 at 10:11 +0300, Alexander Bukharov wrote:
> It would be great if you consider to include this driver to mainstream so 
> Dovecot will be the first IMAP server supporting Oracle as auth backend.

The transaction handling doesn't look correct to me. The sql_update()s
just add the change to a linked list and commit() is then supposed to
run them in one transaction and either everything should succeed or
fail. Your commit appears to ignore errors and just commit everything
that goes through? Also since nothing is actually sent before commit(),
your rollback() shouldn't need to send ROLLBACK.

The "VARCHAR sqltext[2048]" seems like an unnecessary restriction on the
query length. Wasn't there a way to do this without the limit? Or in
general the fixed size VARCHARs are kind of annoying.. Would it be
possible to do something like:

VARCHAR *v;

v = t_malloc(sizeof(*v));
v->arr = query;
v->len = strlen(query);

test_connection() seems unnecessary. If the connection is up (and it
usually is), it adds extra latency. If the connection goes down, nothing
prevents that from happening after test_connection() but before running
the actual query.

Couldn't driver_oracle_generate_name() be simply:
static unsigned int counter = 0;
return i_strdup_printf("ORACONN_%x", ++counter);

driver_oracle_escape_string() really should be escaping the string.

Kind of annoying that each query needs a cursor, even though nearly all
queries are expected to return only a single row, but I guess that can't
be helped with the current API..

Attachment: signature.asc
Description: This is a digitally signed message part

Reply via email to