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

Fix 'Failed to index' warnings for arrays from entities #17890

Merged
merged 1 commit into from
Jun 14, 2021
Merged

Fix 'Failed to index' warnings for arrays from entities #17890

merged 1 commit into from
Jun 14, 2021

Conversation

@quarkus-bot quarkus-bot bot added area/hibernate-orm Hibernate ORM area/persistence OBSOLETE, DO NOT USE labels Jun 14, 2021
Copy link
Member

@yrodiere yrodiere left a comment

Choose a reason for hiding this comment

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

LGTM. The check skips arrays of enums, but then the code after the check isn't able to handle those anyway, so it's just a pre-existing limitation (which I'm perfectly fine with, BTW).

@yrodiere yrodiere added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Jun 14, 2021
@famod famod merged commit 36877ca into quarkusio:main Jun 14, 2021
@quarkus-bot quarkus-bot bot added this to the 2.1 - main milestone Jun 14, 2021
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Jun 14, 2021
@famod famod deleted the fix-jpa-index-warnings branch June 14, 2021 13:59
@gsmet gsmet modified the milestones: 2.1 - main, 2.0.0.Final Jun 21, 2021
@@ -350,7 +350,8 @@ private void addClassHierarchyToReflectiveList(Collector collector, DotName clas
// we need to check for enums
for (FieldInfo fieldInfo : classInfo.fields()) {
Type fieldType = fieldInfo.type();
if (Type.Kind.PRIMITIVE == fieldType.kind()) {
if (Type.Kind.CLASS != fieldType.kind()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, this would also skip parameterized types. I don't know if it could be a problem in the context of hibernate extension... just saying.

Copy link
Member

@yrodiere yrodiere Jul 15, 2021

Choose a reason for hiding this comment

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

This loop is about registering enum types for reflection. I don't think an enum can even be parameterized? Even if it can, I don't think the code below would work.

Regardless... I'm sure there are problems with this code (I mean the whole class), in particular with complex models involving generics. IMO our best way forward is to just avoid the need for registering types for reflection. Which is something we've been working on :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok np, I didn't know it's for enums only ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/hibernate-orm Hibernate ORM area/persistence OBSOLETE, DO NOT USE
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants