Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run integration tests with Oracle [DATAJDBC-256] #478

Closed
spring-projects-issues opened this issue Sep 7, 2018 · 26 comments
Closed

Run integration tests with Oracle [DATAJDBC-256] #478

spring-projects-issues opened this issue Sep 7, 2018 · 26 comments

Comments

@spring-projects-issues
Copy link

@spring-projects-issues spring-projects-issues commented Sep 7, 2018

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

  • ignore the failing tests only for Oracle
  • create an issue for the failing tests
  • add a comment to the failing tests referencing the created issue

Note that there is an existing PR which might be used as a starting point


Issue Links:

  • DATAJDBC-350 Make handling of ID generation work with Oracle
    ("is depended on by")
  • DATAJDBC-550 Support Oracle Dialect
    ("is duplicated by")

Referenced from: pull request #232, and commits 1b8e0a9, 3a889ed, 4208511, 325c502

4 votes, 8 watchers

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Oct 10, 2018

Michael Bahr commented

I'll start working on this

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Oct 11, 2018

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 @IfProfileValue

https://docs.spring.io/spring/docs/current/spring-framework-reference/testing.html#integration-testing-annotations-junit4-ifprofilevalue

Also, create an issue for the problem to solve and add a comment on the annotation referring to the new issue

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Oct 12, 2018

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 DROP TABLE table fails if the table isn't present. I tried to work around this using PL/SQL, but this brings an execution error has shown [here|https://stackoverflow.com/questions/30753901/executesqlscript-fails-with-spring-for-pl-sql-block.]

As far as I'm aware, versions 12+ of Oracle don't support a syntax like DROP TABLE IF EXISTS table either.

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

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Oct 13, 2018

Jens Schauder commented

If you call setIgnoreFailedDrops(true) on the database populator this should work without any PL/SQL trickery.

This method: https://docs.spring.io/spring/docs/current/javadoc-api/org/springframework/jdbc/datasource/init/ResourceDatabasePopulator.html#setIgnoreFailedDrops-boolean-

Invoke it here:

ResourceDatabasePopulator populator = new ResourceDatabasePopulator(script);

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Oct 13, 2018

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 END; is not a valid SQL statement.

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 @GeneratedValue for the @Id columns in this project

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Oct 13, 2018

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

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Oct 15, 2018

Jens Schauder commented

The script execution infrastructure should have a way to set the statement separator.
We should be able to use that to define a custom separator for the profiles that need it (like oracle) and leave the default of ";" for everything else.

If you can't find it let me know, then I'll take another look

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Oct 19, 2018

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.

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Nov 1, 2018

Jens Schauder commented

Michael Bahr are you making progress on this? 
Or do you need assistance?

Anyway, DATAJDBC-258 introduced some infrastructure for ignoring tests based on the current database

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Nov 1, 2018

Michael Bahr commented

I was occupied over the last weeks, but am still on it. Sorry for the delay

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Nov 1, 2018

Jens Schauder commented

Nothing to be sorry for. 

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Nov 14, 2018

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?

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Nov 14, 2018

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?

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Nov 15, 2018

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

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Nov 16, 2018

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

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Nov 17, 2018

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)

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Nov 17, 2018

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

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Nov 17, 2018

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.

https://github.com/bahrmichael/spring-data-jdbc/blob/master/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java#L113

It seems to me that I won't get around db-context information at that line, to either

  1. only set the keyColumnNames when we're in an Oracle context or
  2. switch the keyColumnNames to uppercase (for Hsql) or lowercase (for Postgres)

Is there a way to get the context or do you recommend a different approach?

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Nov 17, 2018

Jens Schauder commented

We don't have this kind of context information yet. I guess that would be part of a to be created DatabaseDialect.
It might be available via JDBC Metadata, although I doubt it.

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

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Nov 17, 2018

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

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Nov 18, 2018

Jens Schauder commented

Great, looking forward to it

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Nov 18, 2018

Michael Bahr commented

PR added: #103
Let me know what I can improve!

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Mar 12, 2020

Jens Schauder commented

This is still open since at the time we didn't have a legal way to integrate Oracle in the build

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 22, 2020

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?
In the meantime there has some testcontainer`s image surfaced: https://www.testcontainers.org/modules/databases/oraclexe/
I will give that a try

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 25, 2020

Jens Schauder commented

Thomas Lang go ahead. Let me know if you have any problems

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 26, 2020

Thomas Lang commented

Jens Schauder here is my current state as PR: #223

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants