datanucleus / datanucleus-rdbms

DataNucleus support for persistence to RDBMS Datastores
30 stars 66 forks source link

datanucleus.query.jdoql.{varName}.join extension does not work when "varName" is not lower case #459

Closed Leyart closed 1 year ago

Leyart commented 1 year ago

If we try to add an extension to tell Datanucleus to join aVariableName as a left outer join via addExtension, this is not happening. The culprit seems to be that on Datanucleus core has been decided to always add extensions in lowercase, see datanucleus/store/query/Query.java. datanucleus-rdbms on the other end, it is still checking for extensions with casing datanucleus/store/rdbms/query/QueryToSQLMapper.java

At the moment, this means that you can only have full lowercase variables if you intend to add extensions to them, which is not the expected or documented behavior.

Likely introduced in https://github.com/datanucleus/datanucleus-core/commit/3e5b661000d11b1c1920db8ad6b05ec2f31ee217#diff-8352fdfe6dea55a47bbf1804c58cc31c7f5678314777a96ad0c3d6b6a936195bL572

to fix

https://github.com/datanucleus/datanucleus-core/issues/457

Leyart commented 1 year ago

Handled in https://github.com/datanucleus/datanucleus-rdbms/pull/461

andyjefferson commented 1 year ago

Yes, all query extensions are stored internally in lowercase (and should automatically be available in QueryToSQLMapper in lowercase, so no need to convert) ... and this is because they have to be stored in one case and are intended to be case insensitive, hence store in lower and check against lower.

If that specific extension is the problem then you should look at https://github.com/datanucleus/datanucleus-rdbms/blob/master/src/main/java/org/datanucleus/store/rdbms/query/QueryToSQLMapper.java#L5157 and simply look at putting the "alias" in lowercase before the lookup.

Leyart commented 1 year ago

Thanks for the feedback. I am reporting this because prior to DN 6, the casing for variables used in JDOQL query expression did not matter. The problem is not that the extension should be defined lowercase. The problem is that when defining variables in query expressions, we are forced to have them in lowercase or join extensions will not work correctly anymore.

The join extension is NOT picked up in the following cases:

Here test-jdo.zip is a small example showing the unexpected behavior.

It seems that in QueryToSQLMapper the alias is actually picked up from the query variable name in the query expression.

In the provided test the path seems to be: https://github.com/datanucleus/datanucleus-rdbms/blob/master/src/main/java/org/datanucleus/store/rdbms/query/QueryToSQLMapper.java#L2846 <-- This is where the Alias is picked up from the query expression https://github.com/datanucleus/datanucleus-rdbms/blob/e3b9db780defb19e2e3b833fc6c9f8480a904176/src/main/java/org/datanucleus/store/rdbms/query/QueryToSQLMapper.java#L5146 https://github.com/datanucleus/datanucleus-rdbms/blob/e3b9db780defb19e2e3b833fc6c9f8480a904176/src/main/java/org/datanucleus/store/rdbms/query/QueryToSQLMapper.java#L5130

And at this point the extension is generated using the fully cased alias, which cannot be found in the extensionsByName map.

Thanks a lot for checking this out and sorry if my initial issue description was not clear enough.

Leyart commented 1 year ago

I updated the pull request with the correct fix, that makes the tests in my POC pass correctly and is less invasive than the previous one