3.15.3 behavior change

[Thomas Matthijs]

Hello,

I updated to 3.15.3 from 3.15.2 and one of my queries started failing,
reproducible example here:
https://github.com/selckin/jooq-testcontainers-example
You can see both SQL generated and error for the 3.15.3 and the link
to the jooq query in java

Anyone who knows if i'm doing something wrong in this query or what
the cause might be? Or what the correct method would be?

Thanks

[Lukas Eder]

Hi Thomas,

Thanks a lot for your report. I've created an issue for it here:

https://github.com/jOOQ/jOOQ/issues/12455

The regression was probably caused by this fix here:

https://github.com/jOOQ/jOOQ/issues/12210

I'm not sure if it is a bug or if your query worked by accident. The usage of implicit join paths within the FROM clause is not yet supported. At least, we don't have any integration tests for it yet. This isn't well documented, it's mentioned in a few issues "implicitly", e.g. here: https://github.com/jOOQ/jOOQ/issues/12037

I'm trying to understand the intent of a query where you use both implicit joins and the onKey() clause. I mean, it happened to work this way for you, but what was the rationale to combine the two? If we ever support implicit join paths in FROM (and hopefully, we'll do), then the ON clause seems mostly redundant and should be optional. A typical approach would be to just list multiple tables in FROM, or use CROSS JOIN, or perhaps a new IMPLICIT JOIN. There's no need to re-specify "ON KEY" because that's already being done by the implicit join, implicitly.

So, what I'm wondering here is if you're adding your paths redundantly, should we perhaps avoid re-adding them implicitly, which seems to have worked by accident before and stopped working because of the aliasing fix #12210.

In any case, you should settle for only one type of join, then your query would better be written as such:

 ctx.select(targetAts.UID, targetAts.ID)
        .from(push_run_match)
        .where(
                sourceAts.TYPE.eq(atsType),
                targetAts.TYPE.eq(atsType),
                push_run_match.STATUS.eq(AmPushRunMatchStatus.NEW)
        )
        .groupBy(targetAts.UID)
        .orderBy(targetAts.UID)
        .limit(10)
        .fetch(record -> new AtsRef(atsType, record.get(targetAts.UID), record.get(targetAts.ID)));

Note there seems to be another issue with resolving paths this way, related to aliasing. If you remove all the aliases, then this warning will go away:

09:24:12,929  INFO [org.jooq.impl.FieldsImpl                          ] - Ambiguous match found for uid. Both "am"."uid" and "source_ats"."uid" match.
java.sql.SQLWarning: null
        at org.jooq.impl.FieldsImpl.field0(FieldsImpl.java:274) [jooq-3.15.3.jar:?]
        at org.jooq.impl.FieldsImpl.field(FieldsImpl.java:213) [jooq-3.15.3.jar:?]
        at org.jooq.impl.AbstractRow.field(AbstractRow.java:238) [jooq-3.15.3.jar:?]
        at org.jooq.impl.FieldsTrait.field(FieldsTrait.java:67) [jooq-3.15.3.jar:?]
        at org.jooq.impl.JoinTable.onKey(JoinTable.java:748) [jooq-3.15.3.jar:?]
        at org.jooq.impl.JoinTable.onKey(JoinTable.java:732) [jooq-3.15.3.jar:?]
        at org.jooq.impl.JoinTable.onKey(JoinTable.java:148) [jooq-3.15.3.jar:?]
        at org.jooq.impl.AbstractContext$JoinNode.joinTree(AbstractContext.java:1049) [jooq-3.15.3.jar:?]
        at org.jooq.impl.AbstractContext$JoinNode.joinTree(AbstractContext.java:1049) [jooq-3.15.3.jar:?]
        at org.jooq.impl.AbstractContext$JoinNode.joinTree(AbstractContext.java:1049) [jooq-3.15.3.jar:?]
        at org.jooq.impl.DefaultRenderContext.scopeEnd0(DefaultRenderContext.java:306) [jooq-3.15.3.jar:?]
        at org.jooq.impl.AbstractContext.scopeEnd(AbstractContext.java:739) [jooq-3.15.3.jar:?]
        at org.jooq.impl.SelectQueryImpl.accept0(SelectQueryImpl.java:1850) [jooq-3.15.3.jar:?]
        at org.jooq.impl.SelectQueryImpl.accept(SelectQueryImpl.java:1435) [jooq-3.15.3.jar:?]
        at org.jooq.impl.DefaultRenderContext.visit0(DefaultRenderContext.java:720) [jooq-3.15.3.jar:?]
        at org.jooq.impl.AbstractContext.visit(AbstractContext.java:296) [jooq-3.15.3.jar:?]
        at org.jooq.impl.AbstractQuery.getSQL0(AbstractQuery.java:469) [jooq-3.15.3.jar:?]
        at org.jooq.impl.AbstractQuery.execute(AbstractQuery.java:287) [jooq-3.15.3.jar:?]
        at org.jooq.impl.AbstractResultQuery.fetchLazy(AbstractResultQuery.java:295) [jooq-3.15.3.jar:?]
        at org.jooq.impl.AbstractResultQuery.fetchLazyNonAutoClosing(AbstractResultQuery.java:316) [jooq-3.15.3.jar:?]
        at org.jooq.impl.SelectImpl.fetchLazyNonAutoClosing(SelectImpl.java:2866) [jooq-3.15.3.jar:?]
        at org.jooq.impl.ResultQueryTrait.collect(ResultQueryTrait.java:357) [jooq-3.15.3.jar:?]
        at org.jooq.impl.ResultQueryTrait.fetch(ResultQueryTrait.java:1454) [jooq-3.15.3.jar:?]
        at org.jooq.example.test.containers.TestContainersTest.testMultisetMappingIntoJavaRecords(TestContainersTest.java:75) [test-classes/:?]

That's definitely a bug, which I'll investigate separately:

https://github.com/jOOQ/jOOQ/issues/12456

--
You received this message because you are subscribed to the Google Groups "jOOQ User Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jooq-user+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jooq-user/CABY_-Z48MwB2UV9_8V1%2B3AO1GcAcNwKisxya9_MqXQEy6gbQBg%40mail.gmail.com.

[Thomas Matthijs]

Thanks, for the detailed & quick response

Guess the reason for using both is mostly that I didn't know i was :)

For example you start off with something like

ctx.select(*)
.from(Tables.FOO)
.leftOuterJoin(Tables.BAR).onKey(Tables.FOO.bar_id.eq(Tables.BAR.id)

Then you learn the code generation knows about your foreign keys

ctx.select(*)
.from(Tables.FOO)
.leftOuterJoin(Tables.BAR).onKey()

Then as queries get more complex, you start having trouble with how
are all the tables with absolute references are connected/related to
each other and if the query is correct or not,
and you notice the Foo.bar() methods

ctx.select(*)
.from(Tables.FOO)
.leftOuterJoin(Tables.FOO.bar()).onKey()

has to be correct compiler verified!

Then you look at some sql logging from the database and can't read
anything, so you include some .as() aliases and everything is pretty
(both generated sql and explicit in code what it does)

Foo foo = Tables.FOO.as("foo");
Foo bar = foo.bar().as("bar");
ctx.select(*)
.from(foo)
.leftOuterJoin(bar).onKey()

(And As i understand now possibly completely wrong usage of jooq?)
(You need the onKey for things to compile)

So more correct usage would be like
Foo foo = Tables.FOO.as("foo");
Foo bar = foo.bar().as("bar");
ctx.select(*)
.from(foo, bar)
.helloworld()

or just not reference any joins and let jooq figure it out? I'll have
to do some more playing around and reread some documentation
Can't do for example rightJoins then ?



Regards

> To view this discussion on the web visit https://groups.google.com/d/msgid/jooq-user/CAB4ELO4HhFBfg_%3DU4ySdZ8k3FYPLzZmSqfP8Ktzq6XX2gwm8cA%40mail.gmail.com.

[Lukas Eder]

Thanks for sharing your line of thought. That makes sense.

It's not completely wrong, it just worked by accident (and it should work in the future). But the ON KEY clause is still redundant. Once this works, that particular syntax will probably have to repeat the ON predicate 2x, once because of the implicit join usage, and once because of ON KEY, at least that's what this API usage is telling jOOQ to do.

 

So more correct usage would be like
Foo foo = Tables.FOO.as("foo");
Foo bar = foo.bar().as("bar");
ctx.select(*)
       .from(foo, bar)
       .helloworld()

Yes that's what would be a desirable usage. HQL / JPQL supports precisely this syntax, and I don't see why we shouldn't. Though, as I said, there aren't any integration tests for this yet, so I can't tell you what the current behaviour is, exactly.

As an aside: The reason why this hasn't been tested yet is because the most common use case for putting implicit joins in FROM is to create to-many joins (which jOOQ doesn't support via implicit joins yet), not to-one joins. But obviously, as I'm seeing now, there's no reason not to use this approach for to-one joins as well.

or just not reference any joins and let jooq figure it out?

The point of implicit joins is that you don't write explicit joins. The join tree is created "implicitly"

 

I'll have to do some more playing around and reread some documentation Can't do for example rightJoins then ?

jOOQ's implicit joins produce inner joins for "mandatory" foreign keys (with a NOT NULL constraint) and left joins for "optional" foreign keys (nullable). I don't think you would ever really need a right join?

[Thomas Matthijs]

Guess, the problem is that in my metal modal i was never using
implicit joins and saw it more like the Foo.bar() is a reference to
bar table that knows to use that primary key in joins

> --
> You received this message because you are subscribed to the Google Groups "jOOQ User Group" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to jooq-user+...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/jooq-user/CAB4ELO6z5D7Y__RLsXc%2BOUWuw8fbezTk92CqEZP%2BJdnQ3TGjQA%40mail.gmail.com.

[Lukas Eder]

Aah, I get it now. I can see how that can be confusing. There's no documentation attached to the method, nor does its generated name (or implementation) clearly indicate the intent. As such, the navigation method looks like it can be used simply to access a parent table. Interestingly, you've used the feature correctly nonetheless, by intuition :)

I think that generating Javadoc on those navigation methods could help resolve some confusion:

https://github.com/jOOQ/jOOQ/issues/12457

Any other ideas that could have prevented confusion here would be very helpful.

Thanks,

Lukas

To view this discussion on the web visit https://groups.google.com/d/msgid/jooq-user/CABY_-Z7NQWTP%2BvHyVtarx5xLBxsruQ7NKS56Q8cY418oUJ6Oaw%40mail.gmail.com.