Jooq: Unsafe re-use of object in Expression math causes incorrect SQL generation

[cam72cam-CoxAuto]

Related to https://github.com/jOOQ/jOOQ/issues/9196

Expected behavior

Field<Long> a = DSL.field("a", Long.class);
Field<Long> b = DSL.field("b", Long.class);
Field<Long> c = DSL.field("c", Long.class);

Field<Long> computedOne = a.add(b);
Field<Long> computedTwo = computedOne.add(c);

System.out.println(DSL.select(computedOne).getSQL());
System.out.println(DSL.select(computedTwo).getSQL());

Prints:

SELECT a + b;
SELECT a + b + c;

Actual behavior

SELECT a + b + c;
SELECT a + b + c;

Steps to reproduce the problem

See: https://github.com/jOOQ/jOOQ/blame/d5bf671329c5784bb44e6d5d4ff67771af69c08a/jOOQ/src/main/java/org/jooq/impl/Expression.java#L107-L114

This modifies the existing object and returns it, which breaks the patter of saving and using this intermediary field result.

Versions

• jOOQ: oldest rev -> latest release
• Java: 1.8
• Database (include vendor): Vertica
• OS: Any
• JDBC Driver (include name if inofficial driver): Any

[cam72cam-CoxAuto]

Actually, this may be resolved by https://github.com/jOOQ/jOOQ/commit/c8d9a5b99145a45a3505c8ad3937844d57070b4c#diff-f6946733190b72192b88f0009371ed13

[cam72cam-CoxAuto]

Confirmed, the linked commit above fixed this issue by refactoring out the broken code @lukaseder

[lukaseder]

Thanks a lot for your report, @cam72cam-CoxAuto, and thanks for validating this on jOOQ 3.14. I'll try to confirm this, and possibly backport https://github.com/jOOQ/jOOQ/issues/10665 to 3.13.5, which I was going to release this week.

[cam72cam-CoxAuto]

Awesome thanks! I just discovered this bug a few days ago and was planning on putting in a PR this sprint, but you beat me to fixing the issue :)

Looking forward to the release!

[cam72cam-CoxAuto]

3ec7707b7e79c5edd1de49e11ff6dc7f96d2cd14 / main:

select (a + b)
select (a + b + c)

0d31f5b26db3861d8f2d19a666d5ca35fc7856e5 / version-3.13.4

select (a + b + c)
select (a + b + c)

[lukaseder]

I can confirm, this test runs on the current 3.14.0-SNAPSHOT version and fails on 3.13.5-SNAPSHOT:

@Test
public void testAssociativeArithmeticOperators() {
    Field<Integer> _1 = inline(1);

    // [#10665] [#10678] Historically, Expression was accidentally mutable
    Field<Integer> add1 = _1.add(_1);
    Field<Integer> add2 = add1.add(_1);

    assertEquals("(1 + 1)", create.render(add1));
    assertEquals("(1 + 1 + 1)", create.render(add2));
}

The failure:

org.junit.ComparisonFailure: expected:<(1 + 1[])> but was:<(1 + 1[ + 1])>
    at org.junit.Assert.assertEquals(Assert.java:117)
    at org.junit.Assert.assertEquals(Assert.java:146)
    at org.jooq.test.AbstractTest.assertEquals(AbstractTest.java:204)
    at org.jooq.test.MutableDSLTest.testAssociativeArithmeticOperators(MutableDSLTest.java:104)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:566)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)

[lukaseder]

With the backport of #10655 (via #10683), this works also on 3.13.5

[lukaseder]

Thanks again for the report and the heads up via your investigations, which were very helpful!