#2773 Fix wrong query in combination with fetch & where

[rPraml]

Hello Rob,

we may have a fix for #2773

we debugged and noticed, that combinations of joins between 'fetch' & 'where' will not always work.

By trial&error we found out, that deleting of

          if (selectIncludes.contains(parentPropertyName)) {
            // parent already handled by select
            return childJoin;
          }

will solve the problem.... and will not break any other test.

I admit that I don't quite understand the fix. Maybe you can take another look to see if this is correct.

cheers
Roland

[rbygrave]

Yup cool, I'll try to have a look tomorrow night.

[rbygrave]

Note: In DefaultDbSqlContext.addJoin() there is the logic below that ensures that we only add a particular join ONCE (based on joinKey below). The effect of removing the // parent already handled by select return; is that we get "duplicate joins" defined in the SqlTreeNodeExtraJoin ... but that doesn't matter for the existing tests etc because the "duplicate join" is de-duplicated by this logic in DefaultDbSqlContext.addJoin().

    String joinKey = table + "-" + a1 + "-" + a2;
    if (tableJoins.contains(joinKey)) {
      return;
    }

Hence removing that logic doesn't seem to break any existing tests (we are getting extra joins defined in the SqlTree but those are being de-duplicated in DefaultDbSqlContext.addJoin().

So with that, it's now a question of looking at the failing test(s).

[rbygrave]

So I'm looking at the test case and what is being attempted here with @Formula and the joins part and ...

... to take a step back and look at the commented out:
@Coalesce({ "familyName", "parent.familyName", "parent.parent.familyName" })

Well Ebean does not support that today but it almost does, in that it can almost do:

.select("name, coalesce(someBean, parent.someBean) as effectiveBean")

For example, today we can do:

.select("name, coalesce(someBean, 42) as effectiveBean")

The reason why I am saying this is because those formula joins look fragile wrt the extent that they are being used with this test. This "fix" to me is potentially fragile because there looks like an ordering / precedence issue wrt the formula joins and the "extra joins" and the fix proposed is more "lucky" than "good".

Stepping back, to me the approach taken here wrt using the formula joins like this doesn't look ideal due to the dynamic nature of the joins that are desired and coalesce(someBean, parent.someBean) looks more robust longer term to me.

[rbygrave]

My thoughts are:

• @Formula should really have been called @SqlFormula darn it
• We should be able to use logical property names and do @LogicalFormula("coalese(familyName, parent.familyName, parent.parent.familyName") ... and Ebean automatically extracts that the "extra join paths" needed being "parent" and "parent.parent" in this case ... and that the logical names get translated with the appropriate table alias' being used (and it's nice and dynamic and imo more robust handling different fetch/join scenarios - dynamically using left join etc).

... and then ideally we could have called @LogicalFormula @Formula