[#1522] Unable to make a paginated query (GraphQl) for an entity view with inheritance configured

[kurbaniec]

Description

Adapted ViewTypeObjectBuilder to fetch discriminator column instead of selecting NULL when inheritance and public <X> SubGraph<X> fetch(String path) are in use.

Related Issue

#1522

Motivation and Context

I encountered a similar issue at my company, though it wasn’t directly related to GraphQL. The problem occurred with certain public <X> SubGraph<X> fetch(String path) calls of EntityViewSetting<T, Q extends FullQueryBuilder<T, Q>> for inheritance queries. From what I understand of the codebase, this mechanism is also involved in the GraphQL query flow.

Because I approached the issue from a slightly different angle, I’m not entirely certain this fix resolves every aspect of the GraphQL case, but based on my tests, I believe it should help.

This is my first time contributing to Blaze-Persistence, so I’m not entirely sure if I’ve done everything correctly. I’d really appreciate any guidance or feedback to help me improve this PR. Specifically:

• Does the approach I’ve taken make sense, or are there better ways to solve this?
• Are there any additional tests or scenarios I should include?
• Would it help if I added more comments or documentation to explain the changes?

I’m eager to learn and happy to make any changes needed. Thank you so much for taking the time to review this! 😊

[beikov]

Thanks for this work. The direction of the approach looks good. I would like to understand what effect this change is going to have on generated queries though.
My main concern is that a join could be created that is unexpected/unnecessary. If an entity view has a polymorphic subview, it might create a join for that subview even though none of its properties are fetched/requested. You seem to have tests that cover this scenario logically, but the check in the isInheritance() method looks like it might be too lenient. Maybe you can reassure me a bit or you see a problem in your code? :)

[kurbaniec]

Thanks for the feedback! I completely understand the concerns—fixing one issue while unintentionally introducing new ones would be the last thing we want. 😊

You’re absolutely right that the isInheritance check might have been too lenient. After reflecting on this and digging into the code more, I’ve made some improvements:

I updated isInheritance to include an attributePath check:

static boolean isInheritance(TupleElementMapper mapper, String attributePath) {  
    // Should fetch the discriminator column instead of selecting “NULL”  
    if (!attributePath.isEmpty()) {  
        return false; // Not a discriminator column  
    }  
    return mapper instanceof AliasExpressionTupleElementMapper && ((AliasExpressionTupleElementMapper) mapper).getAlias().endsWith("_class");  
}

During debugging, I noticed this behaviour, and now the method is stricter and avoids taking every attributePath as-is.

Regarding the overfetching concern, I thought about checking the expression property of AliasExpressionTupleElementMapper, which contains a value like:

CASE WHEN TYPE(inheritanceFetchesTest$Base) = InheritanceFetchesTest$Bar THEN 1 ...  

But since expression is private, it would require deeper changes, and I’m not sure if it would add much value in this context.

As for overfetching joins: with my (admittedly naive!) understanding of the codebase, I believe that if fetches is empty, the block

if (fetches == null || fetches.isEmpty()) { ... }  

in void applySelects(X queryBuilder) would handle this or am I wrong? But I’m happy to explore further if you think this isn’t sufficient!

Lastly, I realised I made a silly mistake in the tests — calling a column value, which is an identifier. 😅 I’ve updated it to a different name and hope this was the only issue of the failing tests.