search

jOOQ / jOOQ

jOOQ is the best way to write SQL in Java
https://www.jooq.org
Other
6.17k stars 1.21k forks source link

DefaultRecordMapper shouldn't call generated from() or into() methods, nor java.lang.Object methods for conflicting columns #16292

Open sargue opened 9 months ago

sargue commented 9 months ago

Expected behavior

Any valid column name should work with code generation and DAO. I've found that a column named "from" raises an exception. I've initially detected it with version 3.15.12 but reproduced it also with version 3.19.3.

Actual behavior

It fails with a message about a converter:

[main] INFO org.jooq.Constants - 

@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
@@@@@@@@@@@@@@@@  @@        @@@@@@@@@@
@@@@@@@@@@@@@@@@@@@@        @@@@@@@@@@
@@@@@@@@@@@@@@@@  @@  @@    @@@@@@@@@@
@@@@@@@@@@  @@@@  @@  @@    @@@@@@@@@@
@@@@@@@@@@        @@        @@@@@@@@@@
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
@@@@@@@@@@        @@        @@@@@@@@@@
@@@@@@@@@@    @@  @@  @@@@  @@@@@@@@@@
@@@@@@@@@@    @@  @@  @@@@  @@@@@@@@@@
@@@@@@@@@@        @@  @  @  @@@@@@@@@@
@@@@@@@@@@        @@        @@@@@@@@@@
@@@@@@@@@@@@@@@@@@@@@@@  @@@@@@@@@@@@@
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@  Thank you for using jOOQ 3.19.3 (Build date: 2024-01-19T12:43:13Z)

[main] INFO org.jooq.Constants - 

jOOQ tip of the day: The jOOQ code generator can generate code from an XML representation of your schema: https://www.jooq.org/doc/latest/manual/code-generation/codegen-xml/

[main] INFO org.jooq.impl.DefaultExecuteContext.logVersionSupport - Version                  : Database version is supported by dialect MARIADB: 11.2.3-MariaDB-1:11.2.3+maria~ubu2204
Exception in thread "main" org.jooq.exception.MappingException: An error occurred when mapping record to class net.sargue.jooq.generated.tables.pojos.Event
    at org.jooq.impl.DefaultRecordMapper$MutablePOJOMapper.map(DefaultRecordMapper.java:925)
    at org.jooq.impl.DefaultRecordMapper.map(DefaultRecordMapper.java:649)
    at org.jooq.RecordMapper.apply(RecordMapper.java:87)
    at org.jooq.RecordMapper.apply(RecordMapper.java:72)
    at java.base/java.util.stream.Collectors.lambda$mapping$13(Collectors.java:469)
    at java.base/java.util.stream.ReduceOps$3ReducingSink.accept(ReduceOps.java:169)
    at java.base/java.util.Iterator.forEachRemaining(Iterator.java:133)
    at java.base/java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1845)
    at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
    at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
    at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:921)
    at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
    at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:682)
    at org.jooq.impl.AbstractCursor.collect(AbstractCursor.java:78)
    at org.jooq.impl.ResultQueryTrait.collect(ResultQueryTrait.java:361)
    at org.jooq.impl.ResultQueryTrait.fetch(ResultQueryTrait.java:1465)
    at org.jooq.impl.DAOImpl.findAll(DAOImpl.java:321)
    at test.Main.main(Main.java:21)
Caused by: org.jooq.exception.DataTypeException: No Converter found for types java.lang.String and net.sargue.jooq.generated.tables.interfaces.IEvent
    at org.jooq.impl.Tools.converterOrFail(Tools.java:1602)
    at org.jooq.impl.Tools.converterOrFail(Tools.java:1611)
    at org.jooq.impl.AbstractRecord.get(AbstractRecord.java:355)
    at org.jooq.impl.DefaultRecordMapper$MutablePOJOMapper.map(DefaultRecordMapper.java:886)
    ... 17 more

Steps to reproduce the problem

I'm managed to reproduce the problem with a small set-up.

I've started an empty MariaDB instance using Docker.

docker run -p 3306:3306 --env MARIADB_ROOT_PASSWORD=my-secret-pw mariadb:latest

Then I connected to MariaDB to create a schema called testdb as well as a simple table:

create table event (
    token varchar(50) primary key,
    `from` varchar(200) not null default ''
);

insert into event values('token1','Alice');

Please note the "from" column name, as it needs to be quoted due to "from" being a reserved keyword. Also note the insert, as you need at least a row to reproduce the problem.

Then I create a simple gradle-based project:

buildscript {
    configurations['classpath'].resolutionStrategy.eachDependency {
        if (requested.group.startsWith('org.jooq') && requested.name.startsWith('jooq')) {
            useVersion '3.19.3'
        }
    }
}

plugins {
    id 'java'
    id 'nu.studer.jooq' version '9.0'
}

group = 'org.example'
version = '1.0-SNAPSHOT'

repositories {
    mavenCentral()
}

dependencies {
    jooqGenerator 'org.mariadb.jdbc:mariadb-java-client:3.3.2'
    implementation 'org.mariadb.jdbc:mariadb-java-client:3.3.2'
    implementation 'org.jooq:jooq'
    implementation 'org.slf4j:slf4j-simple:2.0.12'
}

jooq {
    // https://www.jooq.org/download/support-matrix
    version = '3.19.3'

    configurations {
        main {
            generationTool {
                jdbc {
                    driver = 'org.mariadb.jdbc.Driver'
                    url = 'jdbc:mariadb://localhost/'
                    user = 'root'
                    password = 'my-secret-pw'
                }
                generator {
                    database {
                        name = 'org.jooq.meta.mariadb.MariaDBDatabase'
                        inputSchema = 'testdb'
                        outputSchemaToDefault = true
                    }
                    generate {
                        deprecated = false
                        fluentSetters = true
                        daos = true
                        interfaces = true
                        javaTimeTypes = true
                    }
                    target {
                        packageName = 'net.sargue.jooq.generated'
                    }
                }
            }
        }
    }
}

With a single Main class:

package test;

import net.sargue.jooq.generated.tables.daos.EventDao;
import org.jooq.SQLDialect;
import org.jooq.impl.DSL;

import java.sql.DriverManager;
import java.sql.SQLException;

public class Main {
    public static final String DB_URL = "jdbc:mariadb://localhost/testdb";
    public static final String DB_USERNAME = "root";
    public static final String DB_PASSWORD = "my-secret-pw";

    public static void main(String[] args) throws SQLException {
        try ( var c = DriverManager.getConnection(DB_URL, DB_USERNAME, DB_PASSWORD)) {
            var db = DSL.using(c, SQLDialect.MARIADB);
            var events = new EventDao(db.configuration()).findAll();
            System.out.println("events = " + events);
        }
    }
}

Then it's all about generating the jOOQ classes, including DAOs as per the configuration, and running the main class. The result is the exception posted above.

Just renaming the column to something different (e.g. "fromm") works fine.

jOOQ Version

jOOQ OSS 3.19.3

Database product and version

MARIADB: 11.2.3-MariaDB-1:11.2.3+maria~ubu2204

Java Version

Liberica JDK 17.0.10

OS Version

Windows 10 Pro 19045.4046

JDBC driver name and version (include name if unofficial driver)

org.mariadb.jdbc:mariadb-java-client:3.3.2

lukaseder commented 9 months ago

Thanks for your report. I can reproduce the issue. Will look into this right away.

lukaseder commented 9 months ago

I see, this is because the jOOQ DefaultRecordMapper treats both from(...) and setFrom(...) methods as setters for the FROM attribute, see the specification here: https://www.jooq.org/javadoc/latest/org.jooq/org/jooq/impl/DefaultRecordMapper.html

This would also happen with other attribute names conflicting with methods, including into or wait (from java.lang.Object):


org.jooq.exception.MappingException: An error occurred when mapping record to class org.jooq.test.mariadb.generatedclasses.tables.pojos.Event
    at org.jooq.impl.DefaultRecordMapper$MutablePOJOMapper.map(DefaultRecordMapper.java:925)
    at org.jooq.impl.DefaultRecordMapper.map(DefaultRecordMapper.java:649)
    at org.jooq.RecordMapper.apply(RecordMapper.java:87)
    at org.jooq.impl.DefaultRecordMapper.apply(DefaultRecordMapper.java:1)
    at java.base/java.util.stream.Collectors.lambda$mapping$13(Collectors.java:432)
    at java.base/java.util.stream.ReduceOps$3ReducingSink.accept(ReduceOps.java:169)
    at java.base/java.util.Iterator.forEachRemaining(Iterator.java:133)
    at java.base/java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1939)
    at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
    at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
    at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:921)
    at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
    at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:682)
    at org.jooq.impl.AbstractCursor.collect(AbstractCursor.java:78)
    at org.jooq.impl.ResultQueryTrait.collect(ResultQueryTrait.java:361)
    at org.jooq.impl.ResultQueryTrait.fetch(ResultQueryTrait.java:1465)
    at org.jooq.impl.DAOImpl.findAll(DAOImpl.java:321)
    at org.jooq.testscripts.JDBC.main(JDBC.java:35)
Caused by: java.lang.reflect.InvocationTargetException
    at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:118)
    at java.base/java.lang.reflect.Method.invoke(Method.java:580)
    at org.jooq.impl.DefaultRecordMapper$MutablePOJOMapper.map(DefaultRecordMapper.java:893)
    ... 17 more
Caused by: java.lang.IllegalMonitorStateException: current thread is not owner
    at java.base/java.lang.Object.wait0(Native Method)
    at java.base/java.lang.Object.wait(Object.java:366)
    at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
    ... 19 more

It's not really related to code generation or even using DAOs. Generating interfaces is why the conflicting from() method is causing trouble, but again, conflicts can also arise with methods from java.lang.Object.

It should be simple to fix the conflicts caused by java.lang.Object: If both wait and setWait methods are found, and wait is from java.lang.Object, then don't call it.

Your case is a bit more tricky because it might not be obvious what is intended as a setter and what isn't. The from() and into() methods aren't recognisable as such by the jOOQ runtime. They're simply code generation artifacts, and could also be written manually by a "custom code section."

I guess the main hint we can take here is that the argument type doesn't match the type of the getter (getFrom() returns String), but that shouldn't matter according to DefaultRecordMapper.

I don't think there's a very simple fix here. The reflection based DefaultRecordMapper will always run into such conflict risks... Will think about it a bit more.

lukaseder commented 9 months ago

Well, there is always an option to slightly tweak the DAO.mapper() to handle such cases, because it works on generated code. It would be a more local fix to your specific setup, not a generic fix, so preferable only if there's no reasonable generic fix.

sargue commented 9 months ago

Interesting. In my case it has a very easy solution, I'm still designing the database structure so I will go with another name. It's not a big deal. And I will try to remember to avoid reserved words in the future (which sounds like a reasonable advice not just for jOOQ, now that I think about it).

Hey, is not that different from other preventive precautions like avoiding non-ASCII or spaces for file names, or things like that, which experience reminds you of now and then.

But I understand that, for your point of view as an API developer, this is a bit of a PITA.

lukaseder commented 9 months ago

(which sounds like a reasonable advice not just for jOOQ, now that I think about it).

Well, jOOQ tries hard to make all these edge cases work somehow, but being edge cases, they keep popping up here and there. Just today, I've updated this integration test table to make sure all 3 supported languages can compile jOOQ-generated code :)

CREATE TABLE reserved(
  id INT,

  "abstract" INT,
  "case" INT,
  "catch" INT,
  "class" INT,
  "def" INT,
  "do" INT,
  "else" INT,
  "extends" INT,
  "false" INT,
  "final" INT,
  "finally" INT,
  "for" INT,
  "forSome" INT,
  "if" INT,
  "implicit" INT,
  "import" INT,
  "lazy" INT,
  "match" INT,
  "new" INT,
  "null" INT,
  "object" INT,
  "override" INT,
  "package" INT,
  "private" INT,
  "protected" INT,
  "return" INT,
  "sealed" INT,
  "super" INT,
  "this" INT,
  "throw" INT,
  "trait" INT,
  "try" INT,
  "true" INT,
  "type" INT,
  "val" INT,
  "var" INT,
  "while" INT,
  "with" INT,
  "yield" INT,

  CONSTRAINT pk_reserved PRIMARY KEY (id)
);

But I understand that, for your point of view as an API developer, this is a bit of a PITA.

Well, it never gets dull! :)