[storm] Oracle support review

[James Henstridge]

On Wed, Sep 10, 2008 at 3:19 AM, Gustavo Noronha Silva <kov at alfaiati.net> wrote:
>> [3]
>>
>> +    # this is a simple hack to make sure all auto-generated aliases
>> +    # get escaped; I added this if because oracle was complaining of
>> +    # stuff such as money."value", that was being generated;
>>
>> Do you know what exactly is the syntax that Oracle complains about?
>> We can probably try to fix it without any hacks.
>
> This is the error message I was trying to get fixed:
>
> [15:43:35.047627] EXECUTE: 'SELECT money.id, money."value" FROM money WHERE money.id = :param_1 AND ROWNUM <= :param_2', [{':param_1': <storm.variables.IntVariable object at 0xa38704c>}, {':param_2': <storm.variables.IntVariable object at 0xa38728c>}]
> [15:43:35.051330] ERROR: ORA-00904: "MONEY"."value": invalid identifier
>
> Now, I'm a bit confused myself because what seems to be generated for
> that specific case is this:
>
> [16:06:54.650243] EXECUTE: 'SELECT money.id, money.value FROM money WHERE money.id = :1 AND ROWNUM <= :2', [10, 1]
>

The Django ORM has the following in its Oracle backend:

    def quote_name(self, name):
        # SQL92 requires delimited (quoted) names to be case-sensitive.  When
        # not quoted, Oracle has case-insensitive behavior for identifiers, but
        # always defaults to uppercase.
        # We simplify things by making Oracle identifiers always uppercase.
        if not name.startswith('"') and not name.endswith('"'):
            name = '"%s"' % util.truncate_name(name.upper(),
self.max_name_length())
        return name.upper()

So, if you don't quote the name of the "value" column in the CREATE
TABLE statement for the money table, it will be called VALUE.  Since
quoted identifiers are compared case sensitively, "value" doesn't
match "VALUE".  I'd try quoting the column name "value" in the CREATE
TABLE statement and see if that clears things up.

>> [4]
>>
>> +        super(OracleResult, self).__init__(connection, raw_cursor)
>> +        self.lastrowid = rowid
>>
>> This should be private, since it's not part of the API offered by Storm itself.
>> At least not yet.  We've been pondering about adding a standard way to
>> access such entries.
>
> You mean it should have a _ before its name? The main problem here is
> that, differently from some other db2api modules, cx_Oracle doesn't
> allow adding new attributes to the cursor object.
>
>> It might also be good to look at the mysql-insert-id branch from James
>> which is up for review.  It has some optimizations to avoid one extra
>> query on inserts which perhaps could be used here too.
>
> Interesting. I'll take a look at that, but as far as I know, by
> integrating Willi's work (even though it has been done in this fairly
> hackish way) actually made getting the last row id not need the extra
> query; the insert query itself gives us the id.
>
>
>> [5]
>> +    def execute(self, statement, params=None, noresult=False):
>> +        """NOTE: this is being overriden completely because the
>> +        original from the base class expects to receive only a
>> +        raw_cursor from raw_execute, and we need to receive also the
>> +        rowid, as we cannot set it in the cursor object
>>
>> I don't understand all the details here, but this should be avoided if
>> possible, since it's duplicating a lot of logic, which means that any
>> changes in the top class will break it.
>>
>> Would you mind to bring the topic up online so that we can figure it
>> out together?
>
> The problem is basically that I need to pass the rowid in, and I cannot
> use the raw_cursor as a bearer, as is done by other backends.

The code you've got in OracleConnection.raw_execute() looks like
you're generating a similar to the "INSERT ... RETURNING" statements
generated by the PostgreSQL backend.  Can you get Oracle to return the
primary key variables directly, similar to what we do with PostgreSQL?

Here is a short example of what you can do with PostgreSQL (using a
direct DB-API call):

    c.execute("INSERT INTO money (value) VALUES (42) RETURNING money.id")
    new_id = c.fetchone()[0]

If that works, it should be pretty easily to implement efficient inserts.


James.