Skip to content

HHH-19472: native queries can return Object[] #10178

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

victornoel
Copy link

@victornoel victornoel commented May 19, 2025

The fix is quite simple and obvious. I also refactored the test that was previously introduced for HHH-18450 to cover more cases and make it easier to maintain.

My understanding is that when executing queries (any kind), we either provide a row transformer, a tuple transformer or the expected result class. Sqm-based queries are providing the row transformer directly (based on the result class) while native queries are currently implemented with providing directly the tuple transformer.

So I went with fitting in the existing code by supporting Object[] at the tuple transformer level for native queries.

Alternatively I looked into producing a row transformer in NativeSelectQueryPlanImpl or moving all the default row transformer instantiation (that can be found in a few places of the codebase, e.g., ConcreteSqmSelectQueryPlan#determineRowTransformer) directly inside JdbcSelectExecutorStandardImpl to reduce code duplication and align behaviours but it was way too much work and I worried to introduce regressions.

See also #10177 for 6.6 branch.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.


https://hibernate.atlassian.net/browse/HHH-19472

Copy link
Member

@mbladel mbladel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, once @victornoel fixes CI we can merge this.

@victornoel
Copy link
Author

Ok so as recommend by @mbladel in #10177, I added @RequiresDialect(H2Dialect.class) as well as for PostgreSQL (since it worked and it's good to at least test this on 2 dbs) because some db does not even accept select 1 while some other returns Long instead of Integer in some cases… I hope this time it's mergeable :)

@gavinking
Copy link
Member

I think we can merge this now.

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

Successfully merging this pull request may close these issues.

3 participants