Run integration tests with Oracle [DATAJDBC-256] #478
Comments
|
Michael Bahr commented I'll start working on this |
|
Jens Schauder commented When certain test don't work and aren't readily fixable, please make JUnit ignore the tests but only for the relevant database by using Also, create an issue for the problem to solve and add a comment on the annotation referring to the new issue |
|
Michael Bahr commented Gotcha, though currently I have a different issue: I based the testcontainer on an Oracle XE image (version 11g). The containers appears to start up for the tests, but fails when running the according SQL file (applies to all tests that use a drop table). The As far as I'm aware, versions 12+ of Oracle don't support a syntax like It looks to me like the code executing the sql file should allow PL/SQL as well, if there isn't a better workaround that I'm not aware of yet |
|
Jens Schauder commented If you call |
|
Michael Bahr commented Thank you! That resolved my issue. However I found another one: While Oracle 12+ have Identity columns, 11xe only provides triggers to auto generate the id values if we don't want to do id generation in the java code. CREATE OR REPLACE TRIGGER MANUAL_ON_INSERT
BEFORE INSERT ON MANUAL
FOR EACH ROW
BEGIN
SELECT MANUAL_SEQ.nextval
INTO :new.id
FROM dual;
END;The problem with a trigger is that it contains multiple ; which the ResourceDatabasePopulator and the ScriptUtils from spring-jdbc interpret as multiple statements. This leads to an error saying that Is there a way to pass a SQL script through so that it will be executed as a whole without being split by ScriptUtils? I assume you don't want javax.persistence and it's |
|
Michael Bahr commented Another approach would be to build a 12+ image locally, which however requires downloading the database binaries (~2.5GB), build it based on this license and modifying it a bit more to run with java-testcontainers |
|
Jens Schauder commented The script execution infrastructure should have a way to set the statement separator. If you can't find it let me know, then I'll take another look |
|
Jens Schauder commented There seems to be an issue with obtaining IDs from Oracle (or not existing IDs). I made a proposal on Stackoverflow how this probably can be handled. https://stackoverflow.com/a/52886290/66686 It would be nice, if this could be included in this issue. |
|
Jens Schauder commented Michael Bahr are you making progress on this? Anyway, DATAJDBC-258 introduced some infrastructure for ignoring tests based on the current database |
|
Michael Bahr commented I was occupied over the last weeks, but am still on it. Sorry for the delay |
|
Jens Schauder commented Nothing to be sorry for. |
|
Michael Bahr commented I’m currently stuck with ojdbc6-11.2.0.3 returning a row identifier (class: ROWID, internally a byte array) which is not an instance of Number (GeneratedKeyHolder:35, spring-jdbc 5.1.2.RELEASE). My approach would be to extend the DefaultDataAccessStrategy#getIdFromHolder with a catch-alternative for Oracle (as already done for Postgre) if that's ok? |
|
Michael Bahr commented It looks like the ROWID won't do the trick either since it can't be converted to a Long which is required as an id (and which is also the id datatype that I specified in the create table script). CREATE TABLE LEGO_SET ( id NUMBER(13,0), "NAME" VARCHAR2(30)) This led me a bit deeper down the chain to the ColumnRowRowMapper (org.springframework.jdbc.core) where I realised that through this mechanism I'm not able to access the id column instead of the ROWID. I assume that those two are not the same. I believe this issue arises from the Oracle version (11) not having id generation like postgresql and mysql in the latest versions do. My workaround to get some id generation in oracle was to use triggers, which however seems to be ignored by the ResultSetMetaData mapping. Do you have some experience or recommendation on how to connect trigger-based id generation and the and the mechanism around the GeneratedKeyHolder? |
|
Jens Schauder commented If I have any experience with this it is so old that I can't remember it. But https://stackoverflow.com/a/17463443/66686 seems to suggest that you can query the id if you specify the name. An additional catch clause would be fine. Also if you have your current code somewhere I could do some experimenting |
|
Michael Bahr commented The Stackoverflow link helped. The catch clause has been added with DATAJDBC-258 already. My current problem is an "ORA-00905: missing keyword", maybe caused by the parameter not being set properly (though I couldn't find anything where the parameter was missing on the prepared statement yet). bad SQL grammar [SELECT lego_set.id1 AS id1, lego_set.name AS name, manual.id2 AS manual_id2, manual.content AS manual_content, alt.id2 AS alt_id2, alt.content AS alt_content FROM lego_set LEFT OUTER JOIN manual AS manual ON manual.lego_set = lego_set.id1 LEFT OUTER JOIN manual AS alt ON alt.alternative = lego_set.id1 WHERE lego_set.id1 = ?]; nested exception is java.sql.SQLSyntaxErrorException: ORA-00905: missing keyword I forked the repo and pushed my code to https://github.com/bahrmichael/spring-data-jdbc. So far I focused on the AggregateTemplateIntegrationTests and have only added SQL scripts for that test |
|
Michael Bahr commented I found a solution for the missing keyword error. It was caused by Oracle not allowing AS in the LEFT OUTER JOIN clause. By removing the AS from the string template in SelectBuilder#joinTable. This seems to be working with oracle, mysql, mariadb and postgres. For hsql I think I've broken something else: PreparedStatementCallback; bad SQL grammar [INSERT INTO lego_set (name) VALUES (?)]; nested exception is java.sql.SQLSyntaxErrorException: user lacks privilege or object not found: id1 If you're ok with removing the AS from SelectBuilder#joinTable, then I'll start porting the other tests (apart from AggregateTemplateIntegrationTests) |
|
Jens Schauder commented Go ahead with whatever makes getting the build green, but don't break the build for existing databases. Instead just ignore the tests for now using the mechanics introduced by DATAJDBC-258 and create a separate issue to fix this |
|
Michael Bahr commented Used the ignore mechanics on all other tests that require database setup. Apart from the error below everything seems to work. The Hsql error from above was due to Hsql requiring the column name to be uppercase. Oracle and most other dbs are ok with uppercase names, but Postgres seems to require lowercase. It seems to me that I won't get around db-context information at that line, to either
Is there a way to get the context or do you recommend a different approach? |
|
Jens Schauder commented We don't have this kind of context information yet. I guess that would be part of a to be created But I'm wondering if it is an upper/lower case thing or if we can solve that issue for now by tweaking the upper/lower case we are using in the Database creation scripts |
|
Michael Bahr commented I went for the option to get the database product name from the JdbcOperations. The tests work now with all-dbs, I'll squash the commits and then create a PR |
|
Jens Schauder commented Great, looking forward to it |
|
Michael Bahr commented PR added: #103 |
|
Jens Schauder commented This is still open since at the time we didn't have a legal way to integrate Oracle in the build |
|
Thomas Lang commented Are there still people actively working on it? I would like to work on it, if there is no one else around already doing so ok? |
|
Jens Schauder commented Thomas Lang go ahead. Let me know if you have any problems |
|
Thomas Lang commented Jens Schauder here is my current state as PR: #223 |
Jens Schauder opened DATAJDBC-256 and commented
All the integration tests should also run with an Oracle DB.
As a first step a single version of Oracle is sufficient.
If problems occur that can't readily be fixed it would be fine to
Note that there is an existing PR which might be used as a starting point
Issue Links:
("is depended on by")
("is duplicated by")
Referenced from: pull request #232, and commits 1b8e0a9, 3a889ed, 4208511, 325c502
4 votes, 8 watchers
The text was updated successfully, but these errors were encountered: