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

MonetaryAmountType: Problem with desc-sort on money attribute #505

Open
MichaelZett opened this issue Oct 27, 2022 · 9 comments
Open

MonetaryAmountType: Problem with desc-sort on money attribute #505

MichaelZett opened this issue Oct 27, 2022 · 9 comments
Assignees

Comments

@MichaelZett
Copy link

Hi Vlad,

we updated recently from jadiraUsertype to your hibernateTypes for the MonetaryAmount-Usertype. We think we found an issue with the type: It does not support descending sorting on MonetaryAmount fields in SpringData Repo like Jadira did.
Here is a repository where we built a test case with both user types: https://github.com/Bungi77/MonetaryAmountDeceasedSortingBug/
The hibernateTypes test fails while the Yadira test works as expected. Perhaps you find the time to look into that issue.
Thanks for your work and support.

@vladmihalcea
Copy link
Owner

@MichaelZett Thanks for reporting the issue. Since you already have a replicating test case, you could try to see how to add support for this feature.

I won't have the time to investigate it, so if this is important to you, feel free to contribute this enhancement.

@MichaelZett
Copy link
Author

Hi @vladmihalcea,

the problem seems to be that from jpql to sql an orderBy on a UserType field leads to an orderBy on all attributes but only the last one gets added the order-direction.
order by price DESC leads to order by amount, currency DESC. So with MonetaryAmountType it will always be setting the direction on the currency. The jadira usertype has currency and amount switched so here it works as expected.
To conclude: this seems to be a bug or missing feature on jpql - what do you think?
A work-around would be to also switch the order of amount and currency but that would probably break existing code because the column annotations to use to different attribute names would need to be changed as well.

@vladmihalcea
Copy link
Owner

JPQL is parsed by Hibernate according to its internal ANTLR grammar. You could try to investigate whether a custom Type could influence it.

@MichaelZett
Copy link
Author

Hi Vlad, I think it is not possible to influence that behavior with the custom type.
The order on the custom type is parsed to a single hsql-DotNode with a sinlge Node with desc next to it. Then the initText() Method of the DotNode is called which resolves the MonetaryAmountType to amount,currency text. When the sql is generated the desc is appended to this text (amount,currency desc). For it to work like I'd expected (amount desc,currency desc) it needed to get parsed to two hsql-nodes (so resolving the Type earlier).
Do you think that would be something, the hibernate team would consider?
For your custom type to "solve" this, the order of the attributes needed to change.
Our work-around is that we have an extra readonly attribute for the amount in our entities that we use for Jpql.

@vladmihalcea
Copy link
Owner

Do you think that would be something, the hibernate team would consider?

I have no idea since I quit Red Hat almost 4 years ago. You could ask on the Hibernate Discourse forum.

However, since Jadira has a solution, you could try to see how did they manage to get it working with their Type.

@MichaelZett
Copy link
Author

Basically "luck". The order of their currency and amount attribute is the other way around - so here the desc is added to the amount what is what we expect. If it would be changed on your type it is breaking for users that overwrite the names of the attributes with the Column Annotation. The order there would need to be swapped as well.

@vladmihalcea
Copy link
Owner

In that case, as you mentioned, adding a virtual column mapping using insertable and updatable false would provide you with that flexibility.

@MichaelZett
Copy link
Author

Ok, thanks.

@vladmihalcea
Copy link
Owner

@MichaelZett I think this could be solved via a configuration that reverses the field order. That would fix the problem you are facing, but it will require setting a given configuration property explicitly.

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

No branches or pull requests

2 participants