Skip to content

Conversation

gavinking
Copy link
Member

[Please describe here what your change is about]


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.


1. use typesafe logging methods
2. use 'var'
3. don't use Array.newInstance() when new Object[] works
@hibernate-github-bot
Copy link

hibernate-github-bot bot commented Oct 6, 2025

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: [e5bd817, 3d56536, 29ade08, caadfdf]

› This message was automatically generated.

@gavinking gavinking changed the title Batch fetch stuff Batch load stuff Oct 6, 2025
@gavinking gavinking marked this pull request as ready for review October 6, 2025 07:37
the existence of this assertion was forcing array-based
batch loaders to use Array.newInstance() when it looks
like the JDBC drivers don't require that
@gavinking gavinking merged commit f79de81 into hibernate:main Oct 6, 2025
37 of 39 checks passed
assert bindValue == null || jdbcMapping == null || jdbcMapping.getJdbcJavaType().isInstance( bindValue )
: String.format( Locale.ROOT, "Unexpected value type (expected : %s) : %s (%s)",
jdbcMapping.getJdbcJavaType().getJavaTypeClass().getName(), bindValue, bindValue.getClass().getName() );
// assert bindValue == null || jdbcMapping == null || jdbcMapping.getJdbcJavaType().isInstance( bindValue )
Copy link
Member

Choose a reason for hiding this comment

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

The purpose of this assertion was to protect us from using the wrong values when creating binding values, ensuring that we use the proper jdbc value representation instead of the domain value. I remember that this assertion helped us identify wrong translations in the past, so I'd rather not see it going. It would be nice if we could simply special case arrays i.e. add || jdbcMapping.getJdbcJavaType() instanceof BasicPluralJavaType<?> && bindValue instanceof Object[]. It's a bit sad that we loose the type-checkability by using Object[], but I understand the need to get rid of typed arrays to reduce the amount of metadata for Quarkus.

Copy link
Member Author

Choose a reason for hiding this comment

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

It would be nice if we could simply special case arrays i.e. add || jdbcMapping.getJdbcJavaType() instanceof BasicPluralJavaType<?> && bindValue instanceof Object[].

Oh, sure, that's fine by me.

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.

2 participants