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

Result Class cannot subclass objects with same field names #384

Closed
benze opened this issue Jun 23, 2021 · 4 comments
Closed

Result Class cannot subclass objects with same field names #384

benze opened this issue Jun 23, 2021 · 4 comments
Labels
Milestone

Comments

@benze
Copy link
Contributor

benze commented Jun 23, 2021

Bug:
When specifying a ResultClass in a native SQL query without any CandidateClass specified, the result class cannot subclass another class with the same fieldname.

DN throws an exception:

Result Class query field names are not case sensitive. It does not allow fields with same name, but in a different case. For instance, the field "name" is conflicting.
org.datanucleus.exceptions.NucleusUserException: Result Class query field names are not case sensitive. It does not allow fields with same name, but in a different case. For instance, the field "name" is conflicting.
        at org.datanucleus.store.rdbms.query.ResultClassROF.populateDeclaredFieldsForUserType(ResultClassROF.java:499)
        at org.datanucleus.store.rdbms.query.ResultClassROF.populateDeclaredFieldsForUserType(ResultClassROF.java:505)
        at org.datanucleus.store.rdbms.query.ResultClassROF.access$100(ResultClassROF.java:61)
        at org.datanucleus.store.rdbms.query.ResultClassROF$1.run(ResultClassROF.java:228)
        at java.security.AccessController.doPrivileged(Native Method)
        at org.datanucleus.store.rdbms.query.ResultClassROF.<init>(ResultClassROF.java:224)
        at org.datanucleus.store.rdbms.query.RDBMSQueryUtils.getResultObjectFactoryForNoCandidateClass(RDBMSQueryUtils.java:565)
        at org.datanucleus.store.rdbms.query.SQLQuery.performExecute(SQLQuery.java:677)
        at org.datanucleus.store.query.Query.executeQuery(Query.java:1975)
        at org.datanucleus.store.rdbms.query.SQLQuery.executeWithArray(SQLQuery.java:818)
        at org.datanucleus.store.query.Query.execute(Query.java:1846)
        at org.datanucleus.api.jdo.JDOQuery.executeInternal(JDOQuery.java:439)
        at org.datanucleus.api.jdo.JDOQuery.executeResultList(JDOQuery.java:376)

however, the statement is not accurate. In fact, if a parent class has a field of the same name, the exception is thrown. This can be seen since ResultClassROF.java:499 normalizes the fieldnames (uppercase) in a map to determine if there are duplicate fieldnames:

            if (!field.isSynthetic() && resultClassFieldsByName.put(field.getName().toUpperCase(), field) != null && 
                !field.getName().startsWith(ec.getMetaDataManager().getEnhancedMethodNamePrefix()))

Steps to reproduce:

            Query q = pm.newQuery("SQL", "SELECT name from PERSON");
            List<PersonInfo> results = q.executeResultList(PersonInfo.class);

where PersonInfo is a subclass of another class with a "name" field.

Example:
See SimpleTest.java at https://github.com/benze/test-jdo/tree/inherited_field_result_class_error

Version:
DataNucleus 5.2.8

@andyjefferson
Copy link
Member

User result class has a code smell (field name that is inaccessible, and hence undefined what should happen to it in terms of population from the query result processing).
Exception message is wrong for your case, true. It is however correct for the other case that it was set to trap ... having field names "name" and "NAME" in a result class.

Test doesn't show that, even with the PR (to avoid the exception), the result object superclass field value is NOT populated when extracting the value from the JDBC result set. Maybe it should be, maybe not.

@benze
Copy link
Contributor Author

benze commented Jun 24, 2021

While I agree that this is code smell in the ResultClass, is it DN's responsibility to prevent that?

I would say that since the subclass is the class being presented as the ResultClass, its setter should be the one called and not that of the parent class. That would mean that the parenet class's field would remain undefined.

Agreed that the test (with the PR) does not show this explicitly, however implicitly I believe it does.

Are you inclined to accept this PR if I update the test to be more explicit? (Would also update to show that mismatched names will continue to be rejected). Or are shadowed field names considered to be non-DN compliant?

@andyjefferson
Copy link
Member

My comments are to CLARIFY what the "problem" is, not to say that a PR is not to be applied. The PR is simply to avoid the exception and ignore the setting of shadowed fields ... no issue with the PR for me there.

The fact that your situation is for multiple fields "name" (nothing to do with casing of the name) could imply that ALL fields in the result object should be set. This is a wider question than the problem you are wanting to solve presumably; the JDO spec doesn't define AFAIK what happens with shadowed fields. Either way I won't be including any support for setting shadowed fields in 5.x, just catering for the exception as your PR does

@andyjefferson
Copy link
Member

Fixed with commit for #385

@andyjefferson andyjefferson added this to the 5.2.9 milestone Jun 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants