dovecot - Dec 2009 - Oracle SQL driver

Hi all.

Recently I was looking for an IMAP-server with Oracle support and 
unfortunatelly I couldn't find one. It was a surprise for me but there is 
no such functionality at all. So I've chosen an IMAP-server with most 
flexible and convinient architecture that was Dovecot-1.2.9 and written a 
driver for Oracle RDBMS support. I used Oracle Pro*C so a Pro*C 
preprocessor is needed to compile the driver.
There is a lot of logging there that should be removed.

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.

All related files are attached.



Cheers.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dovecot-ora.tar.gz
Type: application/octet-stream
Size: 29285 bytes
Desc: not available
URL:
<http://dovecot.org/pipermail/dovecot/attachments/20091231/70ff36e1/attachment-0002.obj>

And here is an example of dovecot-sql.conf

-------------- next part --------------
A non-text attachment was scrubbed...
Name: dovecot-sql.conf
Type: application/octet-stream
Size: 539 bytes
Desc: not available
URL:
<http://dovecot.org/pipermail/dovecot/attachments/20091231/1339d0bd/attachment-0002.obj>

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..
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
URL:
<http://dovecot.org/pipermail/dovecot/attachments/20091231/8db744c3/attachment-0002.bin>

> 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.
You right. I've corrected the transaction handling mechanism. The only
thing I couldn't understand: why rollback() shouldn't need to make real
ROLLBACK? In case when whole transaction was failed succeded statements
will remain uncommited. Then they will be commited later on next successful
transaction. May be ROLLBACK is still needed?
> 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
AFAIK there is no possibility to use C types in PREPARE statement. Also
Oracle cannot handle unsized VARCHARs. I've increased VARCHAR size to
65535.
> test_connection() seems unnecessary.
Right you are.
> Couldn't driver_oracle_generate_name() be simply:
> static unsigned int counter = 0;
> return i_strdup_printf("ORACONN_%x", ++counter);
=) Right.
> driver_oracle_escape_string() really should be escaping the string.
I made this function to replace single quote in string with two single
quotes. IMHO this should be enough. How do you think?
> 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..
Yes. There are no other ways to work with untyped statements in OCI. But
Oracle claims that all these statements shouldn't produce significant
performance impact.

The new version attached.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: driver-oracle.pc
Type: application/octet-stream
Size: 25740 bytes
Desc: not available
URL:
<http://dovecot.org/pipermail/dovecot/attachments/20100106/e058341b/attachment-0002.obj>

On 6.1.2010, at 14.09, Alexander Bukharov wrote:
>> 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.
> 
> You right. I've corrected the transaction handling mechanism. The only
> thing I couldn't understand: why rollback() shouldn't need to make
real
> ROLLBACK? In case when whole transaction was failed succeded statements
> will remain uncommited. Then they will be commited later on next successful
> transaction. May be ROLLBACK is still needed?
There wouldn't be any uncommitted statements. The only such statements are
sent inside commit(), the others are SELECTs and such that don't modify
anything.
>> 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
> 
> AFAIK there is no possibility to use C types in PREPARE statement. Also
> Oracle cannot handle unsized VARCHARs. I've increased VARCHAR size to
> 65535.
Well, that also wastes memory :) I thought I remembered using VARCHAR pointers
about 10 years ago. I think I still have that code around, but I currently
don't have a computer where I could put that hard drive. Maybe in a few
days.
>> driver_oracle_escape_string() really should be escaping the string.
> 
> I made this function to replace single quote in string with two single
> quotes. IMHO this should be enough. How do you think?
Does Oracle handle \ specially? I don't have Oracle installed currently or
access to any installations.

I'll check the updated patch later.

> There wouldn't be any uncommitted statements. The only such statements
are
> sent inside commit(), the others are SELECTs and such that don't modify
> anything.
Ok. I made things this way.
> Well, that also wastes memory :) I thought I remembered using VARCHAR
> pointers about 10 years ago. I think I still have that code around, but I
> currently don't have a computer where I could put that hard drive.
Maybe
in
> a few days.
No need for this. I found a way to handle VARCHAR pointers. Now all ok.
There is always a time to learn =)
> Does Oracle handle \ specially? I don't have Oracle installed currently
or
> access to any installations.
No unless an escape clause is specified in SQL-statement.
> I'll check the updated patch later.
Here is a fixed one.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: driver-oracle.pc
Type: application/octet-stream
Size: 25566 bytes
Desc: not available
URL:
<http://dovecot.org/pipermail/dovecot/attachments/20100106/682e4c2c/attachment-0002.obj>

Added free() for the used VARCHAR pointer.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: driver-oracle.pc.gz
Type: application/octet-stream
Size: 6249 bytes
Desc: not available
URL:
<http://dovecot.org/pipermail/dovecot/attachments/20100109/e84d5ba2/attachment-0002.obj>