HHH-12866 : apply "for update of" with aliases in Oracle dialect

[jedichenbin]

Similar to the issue HHH-5654, which had been fixed in PostgreSql81Dialect, when lock mode was specified with an alias, the generated SQL for Oracle database is not using the alias at all.

According to Oracle's documentation of for-update clause, the syntax allows user to specify "table.column" in order to lock the select rows only for a particular table or view in a join.

Tests have been added for Oracle dialects including 8i, 9i, 10g, and 12c. The fix is only in Oracle8iDialect as all the other dialects extends from this one.

Please refer to the description of JIRA ticket HHH-12866.

[gsmet]

@jedichenbin thanks for the PR! Do you know if there's a test really executing this type of query?

Because we test how it is generated but we don't test if the query can be executed by Oracle AFAICS.

[jedichenbin]

@gsmet Good point. I've added a test (in the latest commit) to verify the query can be executed by Oracle. Please have a look and let me know your thoughts.

Besides, there are a few existing tests which is related with locking, including org.hibernate.test.locking.PessimisticWriteLockTimeoutTest, org.hibernate.test.locking.PessimisticWriteSkipLockedTest and etc. The queries generated in these tests now contains the for update of [table.column] because of the use of alias. They would fail if the generated queries cannot be executed by Oracle.

Thank you.

[gsmet]

@jedichenbin I made a couple of minor adjustments in your commits (mostly formatting but also some minor tuning), squashed some of them and force pushed to your branch.

Could you get a fresh copy of your branch and rerun the test suite with Oracle?

[jedichenbin]

@gsmet Thank you so much for reviewing and updating the commits.

I thought the Travis build would run those tests against various databases, obviously I was wrong. Bad news - there are a few tests failed, including:

1. Gradle [documentation:test]
   1. ExplicitLockingTest
2. Gradle [hibernate-core:test]
   1. UpgradeSkipLockedTest (extends AbstractSkipLockedTest)
   2. PessimisticWriteSkipLockedTest (extends AbstractSkipLockedTest)
   3. PessimisticReadSkipLockedTest (extends AbstractSkipLockedTest)

The root cause is that, these test cases were trying to create a query, apply a pagination, and also specify lock options. The generated SQL (see below) did not expose alias properly hence the for update of [alias.column] cannot pass in Oracle.

-- Caused by: java.sql.SQLSyntaxErrorException: ORA-00904: "ABSTRACTSK0_"."ID": invalid identifier

select *
  from (
        select abstractsk0_.id as id1_0_,
               abstractsk0_.processed as processed2_0_
          from BatchJob abstractsk0_
       )
 where rownum <= :1
   for update of abstractsk0_.id skip locked

I am working on a fix, which will pass down the aliases only if the aliases intended to be locked are not equal to all the tables used in the query. Hopefully I'll be able to push something tomorrow.

[gsmet]

The Travis build only runs the tests with H2.

About locking and pagination, I recently removed the SQL 2008 limit handler as Oracle did not support for update with it: d85831f .

I'm not sure there's a perfect solution for these limitations, especially when we don't have enough information to take the right decision in the dialects.

[jedichenbin]

@gsmet - thanks for the clarification about the Travis builds. Apologise for my naive assumptions. Just curious - is there any other CI which is configured to run the tests, which are annotated with @RequiresDialect, e.g. for MySQL/PostgreSQL/DB2?

About that commit related with HHH-12848 - yes, I noticed that commit yesterday. It looks good because according to Oracle 12c document one of the restrictions of using row_limiting_clause is that,

you cannot specify this clause with for_update_clause.

So without that commit the for update of won't work with pagination at all.

I've pushed a commit that fixes all the failed tests mentioned in my previous
comment. Please refer to javadoc of the new method
Oracle8iDialect.needToSpecifyAliasesInForUpdate() for details.

[vladmihalcea]

@gsmet I have both Oracle 11g and 12c installed on my machine. Would you like me to test and validate this? If it works, I could integrate it to master and 5.3 if you want.

[hibernate-github-bot]

Thanks for your pull request!

This pull request does not follow the contribution rules. Could you have a look?

❌ All commit messages should start with a JIRA issue key matching pattern HHH-\d+
    ↳ Offending commits: [ea35641]

› This message was automatically generated.

[gavinking]

superseded by #9330