alsoFetch: - newer Glorp version joins more (unnecessary) tables, leading to wrong results

[jtuchel]

Hi there,

I am in the process of upgrading from VA Smalltalk 9.2.2 to 10.0.2. The old one contains a port of Glorp based on Glorp 8.1-7, the new one has a port of Glorp 9.0-11, nross

We just found out that the new version produces more complex joins for the same SimpleQuery (which is not actually simple).

Here is the code that creates the Query and is unchanged between VAST 9.2.2 and VAST 10.0.2 (the same method edition is present in the old and new image):

q :=
        SimpleQuery
            read: Umsatz
            where: [:ums |
                ums konto = self & (ums buchungssatz buchungsDatum between: strt and: ende)].
    q alsoFetch: [:u | u buchungssatz].
    q alsoFetch: [:u| u buchungssatz buchungsbeleg asOuterJoin].
    q alsoFetch: [:u| u buchungssatz buchungsbeleg externesDokument asOuterJoin].
    q alsoFetch: [:u | u buchungssatz umsaetze asOuterJoin].
    q alsoFetch: [:u | u buchungssatz umsaetze konto asOuterJoin].
    q orderBy: [:u | u buchungssatz buchungsDatum ].

As you can see, there are outer joins to related tables and even going one table further.

The old Glorp version creates this Statement:

SELECT *
FROM (
(
(
(
(DB2INST1.UMSATZ t1 INNER JOIN DB2INST1.BUCHUNGSSATZ t2 ON (t1.buchsatz_id = t2.id)
)
LEFT OUTER JOIN DB2INST1.BUCHUNGSBELEG t3 ON (t2.beleg_id = t3.id)
)
LEFT OUTER JOIN DB2INST1.EXTERNESDOKUMENT t4 ON (t3.ext_beleg_id = t4.id)
)
LEFT OUTER JOIN DB2INST1.UMSATZ t5 ON (t2.id = t5.buchsatz_id)
)
LEFT OUTER JOIN DB2INST1.KPELEMENT t6 ON (t5.konto_id = t6.id)
)
WHERE ((t1.konto_id = 2555494) AND t2.datum BETWEEN '2021-01-01' AND '2021-05-31')
ORDER BY t2.datum, t2.id

As you can see, it joins 6 tables and finds the expected result. The newer version, however, joins 8 tables and finds nothing:

SELECT *
FROM (
(
(
(
(
(
(DB2INST1.UMSATZ t1 INNER JOIN DB2INST1.BUCHUNGSSATZ t2 ON (t1.buchsatz_id = t2.id)
)
INNER JOIN DB2INST1.BUCHUNGSBELEG t3 ON (t2.beleg_id = t3.id)
)
LEFT OUTER JOIN DB2INST1.EXTERNESDOKUMENT t4 ON (t3.ext_beleg_id = t4.id)
)
INNER JOIN DB2INST1.UMSATZ t5 ON (t2.id = t5.buchsatz_id)
)
LEFT OUTER JOIN DB2INST1.KPELEMENT t6 ON (t5.konto_id = t6.id)
)
LEFT OUTER JOIN DB2INST1.BUCHUNGSBELEG t7 ON (t2.beleg_id = t7.id)
)
LEFT OUTER JOIN DB2INST1.UMSATZ t8 ON (t2.id = t8.buchsatz_id)
)
WHERE ((t1.konto_id = 2555494) AND t2.datum BETWEEN '2021-01-01' AND '2021-05-31')
ORDER BY t2.datum, t2.id;

I guess this has to do with the way #asOuterJoin worked then vs. how it works in this later version.

Does anybody have some insight into what is wrong here? Is this a bug or a feature and my query creation code in the old version was "wrong" from a Glorp standpoint?

Joachim

[jtuchel]

So I played a little with the query and I found that if I comment out one of the joins, the results are correct again:

    q :=
        SimpleQuery
            read: Umsatz
            where: [:ums |
                ums konto = self & (ums buchungssatz buchungsDatum between: strt and: ende)].
    q alsoFetch: [:u | u buchungssatz].
    q alsoFetch: [:u| u buchungssatz buchungsbeleg asOuterJoin].

"    q alsoFetch: [:u| u buchungssatz buchungsbeleg externesDokument asOuterJoin]."  "<--- works correctly in Glorp 8.1, but fails in 9.0"

    q alsoFetch: [:u | u buchungssatz umsaetze asOuterJoin].
    q alsoFetch: [:u | u buchungssatz umsaetze konto asOuterJoin].
    q orderBy: [:u | u buchungssatz buchungsDatum ].

I also tried leaving out the #asOuterJoin in that Block, because the relationship between buchungsbeleg and externesDokument is 1:1. But that doesn't make a difference.

(What is funny is that this is not necessarily a general problem with outer joins over more than one table, because the the one in the second to last line does work correctly... here it is a 1:1 relationship as well)

So can anybody explain what is going on?

Is the new behavior correct and I have to change my queries? What change is needed?

Or has this problem been reported and maybe fixed in versions of Glorp newer than 9.0-11 ?

Joachim

[jtuchel]

In the meantime, I found that there are multiple queries are affected in our Application. And it seems doing "too many" asOuterJoins in Queries leads to different results in Glorp 9.1 as compared to 8.1. And it seems this has to do with outerJoins on top of outerJoins...

I try to make a simple example here. Let's look at a query like this:

q := SimpleQuery read: Car where: [:c| c owner = steve].

q alsoFetch: [:car| car wheels asOuterJoin].

q alsoFetch: [:car| car wheels tyre asOuterJoin].

q alsoFetch: [:car| car wheels screws asOuterJoin].

q alsoFetch: [:car| car wheels screws manufacturer asOuterJoin].

While this complete statement works fine in Glorp 8.0, I have to remove the joins on tyres, screws and manufacturers in 9.1 to get the same number of cars. If I leave them in place, I get an empty list.
While not joining these tables means I get the correct list, it  of course means that the whole Application is a lot slower if it navigates to the screws or their manufacturers after the query, because it will result in lots of individual selects for the screws of a wheal and each of their maufacturers.

And, what's even more, if I want to use this query and extend it to get all cars whose wheel tyres were manufactured by Jones Inc., I get completely wrong results. Like so:

q AND: [:c| c wheels anySatisfy: [:w| w tyre  manufacturer name = 'Jones Inc.']].

So to me it seems like Glorp 9.1's joining logic is broken. Or I am using #asOuterJoin completely wrong and Glorp 8.0 returned the correct results by accident.

Can anybody shine some light on this?

[Esteban Maringolo]

Hi Joachim,

Looking at the code differences between 8.1 and 9.1 (latest) there are several changes in the way outer joins are handled to be ANSI compatible, that change was not documented (or at least not openly), introducing breaking issues as yours.

If you look at GlorpExpression>>#beOuterJoin it explicitly sets the expression as outer in GlorpExpression>>privateBeOuterJoin: anOuterJoin

But then the code mentions about this difference:

"NEVER use this method when defining joins in your descriptor systems;  always use parameterless #beOuterJoin and similar API methods.  

         Before version 8.3.1, Glorp only supported inner joins and the setting was a boolean.  

         It then changed to a boolean or nil, and what false meant also changed.  It may evolve further."

It's hard to tell whether the new behavior is right or wrong (compared with the previous one), since all tests are passing, but it certainly requires changes on your side.

In any case, Instantiations is willing to help you looking further into this.

Best regards,

Esteban A. Maringolo

[jtuchel]

Esteban,

you are a hero.

I just tried replacing asOuterJoin with beOuterJoin in one of the failing methods and guess what happened...?

Apart from being embarassed: what could I have done to inform myself about Glorp Changes?  How did you know or find out? Is there a source of info I didn't know yet, or do you follow Glorp so closely on VisualWorks so that you knew...?

Joachim

[Esteban Maringolo]

Hi Joachim,

I'm happy it was solved. I don't have hero powers nor any special
access to changelogs or migration guides from VW, I simply compared
the changes between the two applications versions and that was the
first thing that caught my attention.

As for being informed of the changes (or the rationale behind), many
of us want that to change, in order to have a semantically versioned,
canonical and open Glorp repository, more news to come...

Best regards,

Esteban A. Maringolo

> --
> You received this message because you are subscribed to the Google Groups "glorp-group" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to glorp-group...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/glorp-group/78b16081-5355-4f7f-8244-5bbe6c5d4f08n%40googlegroups.com.

[jtuchel]

Hi everybody,

as I already wrote, the problem could be solved be replacing all sends to asOuterJoin with beOuterJoin. Thanks to Esteban and Niall I could get my queries to work nicely again, since all my sends to asOuterJoin were inside of alsoFetch: clauses.

Here is a very interesting and insightful explanation from Niall I got about the why and what of this change (Niall can't post to this forum for whatever reason so he gave me his okay for forwarding the information):

[T]he outer join changes were indirectly induced by the fact that SQL will not accept, e.g.

        27 = t1.primaryKey

Thus Join>>asGeneralGlorpExpression constructs source and target expressions for the source and target fields and then creates an expression for the join - in _reverse_ order.

This reversal means that any outer join was unwittingly flipped from being a left-join from the source to being a left-join from the target, i.e. a right-join from the source.  This is why outer joins never worked in mappings before 8.3.1 - only in multipleTableJoins and alsoFetch blocks (i.e. MappingExpressions) where this reversal does not occur.

To flip any outerJoin state when flipping the order, we had to support right outer joins, so this was done in
    Glorp(8.3.1 - 9)
    GlorpTest(8.3.1 - 7)
The outerJoin value is now passed in Glorp's internal processing without any assumption it must be a boolean or must be set.  This increase the possible values:
    outerJoin nil = inner join
    outerJoin true = left outer join
    outerJoin false = right outer join
A minor additional issue was that AdHocMapping only set the outer join state on its impliedClauses but did not call beOuterJoin to push down that state to any children that needed it;  this was rectified in the same update.

BTW I also addressed support for Oracle notation for left and right outer joins, and arranged that if the never-used Microsoft outer join notation is switched on again and attempts to print a right outer join, it will raise an error.