Comment 2 for bug 127176

Willi, thanks for your work on the module. Here are some
intial comments about the current implementation:

[1]

  if select.offset is not Undef or select.limit is not Undef:
      pagination = True
      limit = select.limit or sys.maxint
      select.limit = Undef
      offset = select.offset or 0
      select.offset = Undef
      maxrn = offset + limit

It'd be good to create another select statement rather than modifying
the original one. Otherwise executing the same statement twice will
bring different results.

[2]

  # XXX
  # temporary hack to remove the table part of the
  # column specifiction: 'tab.col1, tab.col2' => 'col1, col2'
  # i will fix it when i have a better understanding
  # of the expression compilation process

Can you describe in which cases Oracle doesn't like the table
prefix? E.g. how does it handle the case where there are two
tables being selected from, and there are columns from both
tables in the columns specification?

I can try to explain to you how the compilation system might
help if I understand the problem better.

[3]

  # XXX: this is needed for alias tables as in "test_class_alias"
  # and is copied from info.py (with the "AS" removed)
  # wouldnt it be better to reference to compile_oracle_alias from there?

Yeah, it'd be nice to make it more flexible rather than requiring
copying the whole logic. Don't worry about it for now..

[4]

   def _from_database(self, params):
       for param in params:
       (...)
       elif isinstance(param, Variable):
           yield param.get(to_db=True)

The param may never be a Variable in this case, since the value
is coming from the database.

[5]

  _param_mark = ":1"

Is that the only mark kind accepted by the DBAPI module?

[6]

  raise DatabaseModuleError("DCORacle2' module not found")

Minor typo in the message.

[7]

  dsn = "%s/%s@%s" % (uri.username, uri.password, uri.host)

Doesn't Oracle require a database name?

[8]

- yield result
+ yield tuple(self._from_database(result))

Good catch! Thanks!