|
|
|
Just an observation: JDBCQueryExpression does NOT implememt QueryExpession,
though it should (I assume) Here's an idea, why not only support only Oracle 8.1.6 and above? We can ask on
the castor-dev at list to see if anyone is using a version lower than this. The more I think about this, the more I agree. Unless, Martin, you can think of
a way of introducing this new required functionality in a meaningful way. I had a thorough look at the sourdes, but cannot think of such a solution. > I just had a look at the code, and to me the main problem is that to perform a
> DatabaseMetaData.getDatabaseProductVersion() one needs a valid > java.sql.Connection instance. Unfortunately, at the moment of this conversation, > (JDBC)QueryExpression does not have access to a java.sql.Connection instance. Well, this problem can be overcome if you accept a interface change to Currently the database connection is typically opened at the first execute() I need just some time clean up my patch. It will include Werner's patch for bug Created an attachment (id=664)
proposed patch The patch uses a new helper class DbMetaInfo to retrieve the database version
from the JDBC connection and allows comparison with the required databse versions. For efficiency the DbMetaInfo is cached in TransactionContextImpl. It is passed from OQLQueryImpl.create() through ParseTreeWalker and JDBCQueryExpression into isLimitClauseSupported/isOffsetClauseSupported() in OracleQueryExpression. This is where the database version is queried and decided whether LIMIT/OFFSET is supported or not. Martin, I had a brief look at your patch, and I think it all makes sense. Can I
ask you two things: a) Try to run the Castor JDO CTF suite using test.bat|sh castor.oracle, and see Created an attachment (id=668)
updated patch with DBMetaInfo member in JDBCQueryExpression Because QueryExpression is only a interface, no regular class, I can't put
DBMetaInfo as constructor parameter, so I added a setDbMetaInfo() function instead. Trying to extend SQLEngine.getQueryExpression() and initializing the dbInfo object in PersistenceFactory would result in much more code changes. So I just call setDbMetaInfo() in ParseTreeWalker.createQueryExpression(). A small disadvantage of this altered patch is, you can't be sure _dbInfo is really initialized any time in calls to isLimit/OffsetClauseSupported(), so you first have to look if it might still be null. (see OracleQueryExpression) Running the test cases gives me the following output: There was 1 error: FAILURES!!! But this is exactly the same when running them against the unchanged CVS Martin, thans for the patch. I have a look at some of the code, but as it
duplicates most of bug 1732, I'd rather wait until bug 1732 has been committed, which shouldn't be too far in the future. Can I please ask you to bear with me for the time being, and possibly m aybe I like the DbMetaInfo object, but I wonder if we could formalize it a bit more?
The reason I'm asking is because it would be nice to be able to house relational database metadata or LDAP metadata in the *MetaInfoImpl object. Sure, DbMetaInfo can be extended to expose more than just the (JDBC) database
version. For example there are functions like nullPlusNonNullIsNull() or supportsOrderByUnrelated() in DatabaseMetaData. I don't know anything about using LDAP. Bruce, please feel free to add a NEW bug report, as this bug is in support of
bug 1732. And as we have (internally) agreed upon new release for Nov 7th, 2004, I am sure you'll understand. Unless of course you happen to have some more time to provide a patch yourself .. ![]() Martin, can you please re-post your patch after you've updated your code from
the CVS server, as I've just committed the changes for 1732. Pretty please ? Created an attachment (id=699)
updated patch The new patch now applies to the current CVS.
It also contains some minor import and comment cleanups. Committed patch as provided. Please don't forget to close this bug once you've
asserted its correctness. There is a small typo in the comment to JDBCQueryExpression.getStatement():
Index: src/main/org/exolab/castor/jdo/drivers/JDBCQueryExpression.java /**
Anything other is OK. So Closed. |
I just had a look at the code, and to me the main problem is that to perform a
DatabaseMetaData.getDatabaseProductVersion() one needs a valid
java.sql.Connection instance. Unfortunately, at the moment of this conversation,
(JDBC)QueryExpression does not have access to a java.sql.Connection instance.
Iow, we first needed to investigate how to go about this problem ...