DataType.getArrayDataType() should wrap custom Converter<T, U> implementations into Converter<T[], U[]> automatically #10664

pschichtel · 2020-09-21T15:29:45Z

Expected behavior

jOOQ should be able to apply converters even on array types.

Actual behavior

jOOQ is unable to find the converter for the custom type when the conversion is happening while converting an array.

Steps to reproduce the problem

Kotlin code being used:

data class TestWrapper(val s: String)

val dt = SQLDataType.CLOB.asConvertedDataType(object : AbstractConverter<String, TestWrapper>(String::class.java, TestWrapper::class.java) {
    override fun from(databaseObject: String?): TestWrapper? {
        return databaseObject?.let(::TestWrapper)
    }

    override fun to(userObject: TestWrapper?): String? {
        return userObject?.s
    }
})

val keyField = DSL.field("i", SQLDataType.INTEGER)
val valueField = DSL.field("s", dt)
val data = DSL.select(DSL.inline(1).`as`(keyField), DSL.inline("test1").`as`(valueField))

val results = dslContext.select(keyField, valueField).from(data)
    .fetch()
    .toList()

println(results)

val aggregatedResults = dslContext.select(keyField, DSL.arrayAgg(valueField)).from(data)
    .groupBy(keyField)
    .fetch()
    .toList()

println(aggregatedResults)

The working example produces the following output on the console:

[+----+--------------------+
|   i|s                   |
+----+--------------------+
|   1|TestWrapper(s=test1)|
+----+--------------------+
]

The failing example produces the following stacktrace:

org.jooq.exception.DataAccessException: SQL [select i, array_agg(s) from (select 1 as "i", 'test1' as "s" union all select 1, 'test2') as "alias_16876193" group by i]; Error while reading field: array_agg(s), at JDBC index: 2

	at org.jooq_3.13.4.POSTGRES.debug(Unknown Source)
	at org.jooq_3.13.4.POSTGRES.debug(Unknown Source)
	at org.jooq.impl.Tools.translate(Tools.java:2753)
	at org.jooq.impl.DefaultExecuteContext.sqlException(DefaultExecuteContext.java:755)
	at org.jooq.impl.CursorImpl$CursorIterator.fetchNext(CursorImpl.java:1630)
	at org.jooq.impl.CursorImpl$CursorIterator.hasNext(CursorImpl.java:1581)
	at org.jooq.impl.CursorImpl.fetchNext(CursorImpl.java:353)
	at org.jooq.impl.CursorImpl.fetch(CursorImpl.java:339)
	at org.jooq.impl.CursorImpl.fetch(CursorImpl.java:246)
	at org.jooq.impl.AbstractResultQuery.execute(AbstractResultQuery.java:324)
	at org.jooq.impl.AbstractQuery.execute(AbstractQuery.java:371)
	at org.jooq.impl.AbstractResultQuery.fetch(AbstractResultQuery.java:354)
	at org.jooq.impl.SelectImpl.fetch(SelectImpl.java:2690)
	at ____.reproJooqBug(____.kt:134)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:64)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:564)
	at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:686)
	at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131)
	at org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:149)
	at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:140)
	at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:84)
	at org.junit.jupiter.engine.execution.ExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(ExecutableInvoker.java:115)
	at org.junit.jupiter.engine.execution.ExecutableInvoker.lambda$invoke$0(ExecutableInvoker.java:105)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:106)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.proceed(InvocationInterceptorChain.java:64)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.chainAndInvoke(InvocationInterceptorChain.java:45)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.invoke(InvocationInterceptorChain.java:37)
	at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:104)
	at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:98)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeTestMethod$6(TestMethodTestDescriptor.java:212)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(TestMethodTestDescriptor.java:208)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:137)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:71)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:135)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:38)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:38)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:32)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:51)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:248)
	at org.junit.platform.launcher.core.DefaultLauncher.lambda$execute$5(DefaultLauncher.java:211)
	at org.junit.platform.launcher.core.DefaultLauncher.withInterceptedStreams(DefaultLauncher.java:226)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:199)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:132)
	at com.intellij.junit5.JUnit5IdeaTestRunner.startRunnerWithArgs(JUnit5IdeaTestRunner.java:71)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:33)
	at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:220)
	at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:53)
Caused by: java.sql.SQLException: Error while reading field: array_agg(s), at JDBC index: 2
	at org.jooq.impl.CursorImpl$CursorIterator$CursorRecordInitialiser.setValue(CursorImpl.java:1735)
	at org.jooq.impl.CursorImpl$CursorIterator$CursorRecordInitialiser.operate(CursorImpl.java:1685)
	at org.jooq.impl.CursorImpl$CursorIterator$CursorRecordInitialiser.operate(CursorImpl.java:1650)
	at org.jooq.impl.RecordDelegate.operate(RecordDelegate.java:130)
	at org.jooq.impl.CursorImpl$CursorIterator.fetchNext(CursorImpl.java:1614)
	... 72 more
Caused by: org.jooq.exception.DataTypeException: Cannot convert from test1 (class java.lang.String) to class ____$TestWrapper
	at org.jooq.tools.Convert$ConvertAll.fail(Convert.java:1194)
	at org.jooq.tools.Convert$ConvertAll.from(Convert.java:1083)
	at org.jooq.tools.Convert.convert0(Convert.java:324)
	at org.jooq.tools.Convert.convert(Convert.java:316)
	at org.jooq.tools.Convert.convert(Convert.java:387)
	at org.jooq.tools.Convert.convertArray(Convert.java:295)
	at org.jooq.tools.Convert$ConvertAll.from(Convert.java:537)
	at org.jooq.tools.Convert.convert0(Convert.java:324)
	at org.jooq.tools.Convert.convert(Convert.java:316)
	at org.jooq.tools.Convert.convert(Convert.java:387)
	at org.jooq.impl.DefaultBinding$DefaultArrayBinding.convertArray(DefaultBinding.java:1212)
	at org.jooq.impl.DefaultBinding$DefaultArrayBinding.pgGetArray(DefaultBinding.java:1201)
	at org.jooq.impl.DefaultBinding$DefaultArrayBinding.get0(DefaultBinding.java:1118)
	at org.jooq.impl.DefaultBinding$DefaultArrayBinding.get0(DefaultBinding.java:974)
	at org.jooq.impl.DefaultBinding$AbstractBinding.get(DefaultBinding.java:837)
	at org.jooq.impl.CursorImpl$CursorIterator$CursorRecordInitialiser.setValue(CursorImpl.java:1725)
	... 76 more

Versions

jOOQ: 3.13.4
Java: 15
Database (include vendor): Postgresql 11 (fresh instance using testcontainers)
OS: Fedora 32
JDBC Driver (include name if inofficial driver): 42.2.5

The text was updated successfully, but these errors were encountered:

lukaseder · 2020-09-25T08:36:49Z

Thanks a lot for your report. This is specific to ARRAY_AGG, in which case I agree we should be a bit smarter about the data type.

Side note, you can create ad-hoc converters using Converter.of(), or Converter.ofNullable(), which is more convenient than the AbstractConverter approach.

lukaseder · 2020-09-25T08:42:41Z

In fact, the bug isn't specific to ARRAY_AGG, but to the DataType.getArrayDataType() call in the org.jooq.impl.ArrayAgg constructor, which does not take converters into consideration.

lukaseder · 2020-09-25T08:43:04Z

We should also offer the fix as public API, via Converter::forArrays: #10688

lukaseder · 2020-09-25T13:22:22Z

A separate issue is how to support custom bindings in ARRAY_AGG, which leaves more open questions, as the binding doesn't apply to the T[] type, only to the T type. I don't know the answers yet to this.

lukaseder · 2020-09-25T13:27:25Z

I cannot seem to reproduce this with jOOQ 3.14.0. Will try again next week with 3.13.5 to see if it was already fixed.

lukaseder · 2020-10-06T09:49:02Z

Indeed, the problem is reproducible in 3.13.6-SNAPSHOT. There have been a lot of changes in the area of data types in 3.14. Given the complexity and implied risks arising from a backport. I prefer not to backport any fixes in this area. 3.14 will ship soon.

pschichtel · 2020-10-10T01:30:30Z

3.14 would be fine for me. We did just notice a case where this actually worked with arrayAggDistinct on 3.13.5, haven't looked into what was different there.

lukaseder · 2020-10-13T11:30:46Z

Hmm, so perhaps I did something differently when I tried reproducing this? 3.14 will be released soon. If you can still reproduce this with 3.14, please reopen this issue and I'll have another look

lukaseder · 2020-10-13T15:12:28Z

See also: #7471

stravag · 2021-03-30T11:23:12Z

I can confirm this now works with the latest 3.14 version

pschichtel · 2021-09-29T09:46:17Z

I'm hitting this again on 3.15.3 after a change on my wrapper types. Only the implementation of the converter changed, but the interface is identical. I'm very confused.

pschichtel · 2021-09-29T10:37:59Z

While debugging I noticed that the converter isn't even being applied.

My change was:

data class Wrapper(val value: String)

to

data class SomeOtherWrapper(val value: String)
data class Wrapper(value: SomeOtherWrapper)

Previously it worked due to the logic at the end of tools.Convert.ConvertAll.from, which tries to find a single-arg constructor that matches the fromType. This was the case in the old Wrapper, but not in the new Wrapper.

Problem is still, that the String->Wrapper converter is not being applied in the String[]->Wrapper[] case.

One workaround for everyone else hitting this: Add a secondary constructor (can be private) that wraps the value and calls the primary constructor.

lukaseder · 2022-02-10T20:00:52Z

I just found your not-yet-answered comments, @pschichtel. Is this still a problem? Is this the same problem as your original one? In either case, I think it would be worth creating a new issue with a new description and reproducer. I'm not entirely sure if it's the same problem, and since this issue is already closed, it isn't on my radar anymore.

pschichtel · 2022-02-10T21:50:13Z

Yeah I think what I was describing in the last comment is the problem this issues was originally about, but back when reporting this issue I didn't fully understand how to trigger it. I can try to extract a reproducer from our code base tomorrow and report it as a new issue.

juriad · 2022-02-11T10:20:28Z

I am not sure if I am hitting he same issue; here is MCVE. There is a solution but it is not pretty (explained why at the end)

class Id {
            UUID uuid;

            public Id(UUID uuid) {
                this.uuid = uuid;
            }

            public UUID getUuid() {
                return uuid;
            }
        }

        Converter<UUID, Id> converter = Converter.ofNullable(UUID.class, Id.class, Id::new, Id::getUuid);
        DataType<Id[]> IDS = SQLDataType.UUID
                .asConvertedDataType(converter)
                .getArrayDataType();

        DataType<Id[]> IDS2 = SQLDataType.UUID
                .getArrayDataType()
                .asConvertedDataType(Converter.ofNullable(UUID[].class, Id[].class,
                        uuids -> Arrays.stream(uuids).map(Id::new).toArray(Id[]::new),
                        ids -> Arrays.stream(ids).map(Id::getUuid).toArray(UUID[]::new)
                ));

        DataType<Id[]> IDS3 = SQLDataType.UUID
                .getArrayDataType()
                .asConvertedDataType(converter.forArrays());

        ctx.createTableIfNotExists("a").column("b", IDS).execute();
        ctx.insertInto(DSL.table("a"))
                // .values(DSL.val(new Id[]{new Id(UUID.randomUUID())}, IDS)) // does not work
                .values(DSL.val(new Id[]{new Id(UUID.randomUUID())}, IDS2)) // works
                .values(DSL.val(new Id[]{new Id(UUID.randomUUID())}, IDS3)) // works
                .execute();

        ctx
                // .select(DSL.field("b", IDS)) // does not work
                .select(DSL.field("b", IDS2)) // works
                .select(DSL.field("b", IDS3)) // works
                .from("a")
                .fetch();

Similar issue here:

        class Id {
            UUID uuid;

            public Id(UUID uuid) {
                this.uuid = uuid;
            }

            public UUID getUuid() {
                return uuid;
            }
        }

        Converter<UUID, Id> converter = Converter.ofNullable(UUID.class, Id.class, Id::new, Id::getUuid);
        DataType<Id> ID = SQLDataType.UUID.asConvertedDataType(converter);

        ctx.createTableIfNotExists("a").column("b", ID).execute();
        ctx.insertInto(DSL.table("a"))
                .values(DSL.val(new Id(UUID.randomUUID()), ID))
                .values(DSL.val(new Id(UUID.randomUUID()), ID))
                .execute();

        ctx
                // .select(DSL.arrayAgg(DSL.field("b", ID))) // does not work
                // .select(DSL.arrayAgg(DSL.field("b")).coerce(ID.getArrayDataType())) // does not work
               .select(DSL.arrayAgg(DSL.field("b")).coerce(SQLDataType.UUID.getArrayDataType().asConvertedDataType(converter.forArrays())))
                .from("a")
                .fetch();

My main problem is not that I am unable to create the convertor (as I show there are workarounds) but that I need to construct it from scratch. When I am working with a table (generated or not) and have DataType<Id> at my hand, it is hard to turn it into DataType<Id[]> without knowing that the underlying datatype is UUID.

We actually offer the user to choose the Id datatype for the tables (either Long, or UUID). We then pass DataType throughout the application as a parameter. We are starting to use arrays in some scenarios and are hit with unexpected behavior.

Is it indeed intended that:

DataType<Id> ID = SQLDataType.UUID
                .asConvertedDataType(converter)
                .getArrayDataType();

does not produce usable DataType<Id[]>?

Tested on 3.16.4 and Postgres.

pschichtel · 2022-02-11T11:55:14Z

@juriad Yes I think that's exactly the issue, yes. Would you mind opening the new issue for that? I can add my reproducer to that as well when I get to creating it.

juriad · 2022-02-11T13:59:16Z

To make it even worse, if the create table in the first example uses IDS2 or IDS3, then the table is created incorrectly with type uuid instead of uuid[] causing errors:

org.jooq.exception.DataAccessException: SQL [insert into a values (?::uuid[]), (?::uuid[])]; ERROR: column "b" is of type uuid but expression is of type uuid[]
  Hint: You will need to rewrite or cast the expression.
  Position: 23

This means that there does not exist a single definition of DataType<Id[]> that works in all situations.

lukaseder · 2022-02-17T16:07:19Z

Thanks again for your comments, folks. I'm currently working on the fix in this issue here: #13073

The new fix will be backported to 3.16 and 3.15.

pschichtel added the T: Defect label Sep 21, 2020

lukaseder added this to the Version 3.14.0 milestone Sep 25, 2020

lukaseder added this to To do in 3.14 Other improvements via automation Sep 25, 2020

lukaseder added C: Functionality E: All Editions P: Medium labels Sep 25, 2020

lukaseder changed the title ~~Custom type cannot be converted in in array~~ ARRAY_AGG() should turn custom Converter<T, U> implementations into Converter<T[], U[]> automatically Sep 25, 2020

lukaseder changed the title ~~ARRAY_AGG() should turn custom Converter<T, U> implementations into Converter<T[], U[]> automatically~~ DataType.getArrayDataType() custom Converter<T, U> implementations into Converter<T[], U[]> automatically Sep 25, 2020

lukaseder changed the title ~~DataType.getArrayDataType() custom Converter<T, U> implementations into Converter<T[], U[]> automatically~~ DataType.getArrayDataType() should wrap custom Converter<T, U> implementations into Converter<T[], U[]> automatically Sep 25, 2020

lukaseder closed this as completed Oct 6, 2020

3.14 Other improvements automation moved this from To do to Done Oct 6, 2020

lukaseder added the R: Fixed label Oct 6, 2020

lukaseder added a commit that referenced this issue Oct 8, 2020

[#10664] Backported additional improvements

7d4c09d

lukaseder mentioned this issue Oct 13, 2020

arrayAgg() does not respect custom converters #7471

Closed

juriad mentioned this issue Feb 15, 2022

Order of calling DataType.asConvertedDataType(Converter) and DataType.getArrayDataType() should not matter #13073

Closed

lukaseder mentioned this issue Feb 17, 2022

Throw an exception when calling ConvertedDataType.getArrayDataType() when the type has a custom Binding #13094

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DataType.getArrayDataType() should wrap custom Converter<T, U> implementations into Converter<T[], U[]> automatically #10664

DataType.getArrayDataType() should wrap custom Converter<T, U> implementations into Converter<T[], U[]> automatically #10664

pschichtel commented Sep 21, 2020

lukaseder commented Sep 25, 2020

lukaseder commented Sep 25, 2020

lukaseder commented Sep 25, 2020

lukaseder commented Sep 25, 2020

lukaseder commented Sep 25, 2020

lukaseder commented Oct 6, 2020

pschichtel commented Oct 10, 2020

lukaseder commented Oct 13, 2020

lukaseder commented Oct 13, 2020

stravag commented Mar 30, 2021

pschichtel commented Sep 29, 2021

pschichtel commented Sep 29, 2021 •

edited

lukaseder commented Feb 10, 2022

pschichtel commented Feb 10, 2022

juriad commented Feb 11, 2022 •

edited

pschichtel commented Feb 11, 2022

juriad commented Feb 11, 2022 •

edited

lukaseder commented Feb 17, 2022

DataType.getArrayDataType() should wrap custom Converter<T, U> implementations into Converter<T[], U[]> automatically #10664

DataType.getArrayDataType() should wrap custom Converter<T, U> implementations into Converter<T[], U[]> automatically #10664

Comments

pschichtel commented Sep 21, 2020

Expected behavior

Actual behavior

Steps to reproduce the problem

Versions

lukaseder commented Sep 25, 2020

lukaseder commented Sep 25, 2020

lukaseder commented Sep 25, 2020

lukaseder commented Sep 25, 2020

lukaseder commented Sep 25, 2020

lukaseder commented Oct 6, 2020

pschichtel commented Oct 10, 2020

lukaseder commented Oct 13, 2020

lukaseder commented Oct 13, 2020

stravag commented Mar 30, 2021

pschichtel commented Sep 29, 2021

pschichtel commented Sep 29, 2021 • edited

lukaseder commented Feb 10, 2022

pschichtel commented Feb 10, 2022

juriad commented Feb 11, 2022 • edited

pschichtel commented Feb 11, 2022

juriad commented Feb 11, 2022 • edited

lukaseder commented Feb 17, 2022

pschichtel commented Sep 29, 2021 •

edited

juriad commented Feb 11, 2022 •

edited

juriad commented Feb 11, 2022 •

edited