From a183191a3ade9d31019eb9571e592a26e4653d8b Mon Sep 17 00:00:00 2001 From: Lukas Eder Date: Tue, 7 Dec 2021 15:59:25 +0100 Subject: [PATCH] [jOOQ/jOOQ#12706] Cannot nest MULTISET in top level ROW in PostgreSQL --- .../java/org/jooq/impl/DefaultBinding.java | 90 +++++++++++++------ .../org/jooq/impl/EmbeddableTableField.java | 1 + .../java/org/jooq/impl/MultisetDataType.java | 2 +- .../java/org/jooq/impl/RecordDataType.java | 4 +- .../src/main/java/org/jooq/impl/RowField.java | 17 +--- jOOQ/src/main/java/org/jooq/impl/Tools.java | 5 ++ jOOQ/src/main/java/org/jooq/impl/Val.java | 1 + 7 files changed, 75 insertions(+), 45 deletions(-) diff --git a/jOOQ/src/main/java/org/jooq/impl/DefaultBinding.java b/jOOQ/src/main/java/org/jooq/impl/DefaultBinding.java index c0439acb44..147403dec6 100644 --- a/jOOQ/src/main/java/org/jooq/impl/DefaultBinding.java +++ b/jOOQ/src/main/java/org/jooq/impl/DefaultBinding.java @@ -46,6 +46,7 @@ import static java.time.temporal.ChronoField.MONTH_OF_YEAR; import static java.time.temporal.ChronoField.NANO_OF_SECOND; import static java.time.temporal.ChronoField.SECOND_OF_MINUTE; import static java.time.temporal.ChronoField.YEAR; +import static java.util.function.Function.identity; // ... // ... // ... @@ -91,6 +92,8 @@ import static org.jooq.impl.DefaultBinding.DefaultDoubleBinding.REQUIRES_LITERAL import static org.jooq.impl.DefaultBinding.DefaultDoubleBinding.infinity; import static org.jooq.impl.DefaultBinding.DefaultDoubleBinding.nan; import static org.jooq.impl.DefaultBinding.DefaultJSONBBinding.EMULATE_AS_BLOB; +import static org.jooq.impl.DefaultBinding.DefaultResultBinding.readMultisetJSON; +import static org.jooq.impl.DefaultBinding.DefaultResultBinding.readMultisetXML; import static org.jooq.impl.DefaultExecuteContext.localExecuteContext; import static org.jooq.impl.DefaultExecuteContext.localTargetConnection; import static org.jooq.impl.Internal.arrayType; @@ -130,6 +133,7 @@ import static org.jooq.impl.SQLDataType.SMALLINT; import static org.jooq.impl.SQLDataType.TIME; import static org.jooq.impl.SQLDataType.TIMESTAMP; import static org.jooq.impl.SQLDataType.VARCHAR; +import static org.jooq.impl.Tools.apply; import static org.jooq.impl.Tools.asInt; import static org.jooq.impl.Tools.attachRecords; import static org.jooq.impl.Tools.convertBytesToHex; @@ -3596,7 +3600,7 @@ public class DefaultBinding implements Binding { case POSTGRES: case YUGABYTE: - return pgNewRecord(dataType.getType(), null, ctx.resultSet().getObject(ctx.index())); + return pgNewRecord(ctx, dataType.getType(), (AbstractRow) dataType.getRow(), ctx.resultSet().getObject(ctx.index())); default: return localExecuteContext(ctx.executeContext(), () -> (Record) ctx.resultSet().getObject(ctx.index(), typeMap(dataType.getType(), ctx))); @@ -3610,7 +3614,7 @@ public class DefaultBinding implements Binding { case POSTGRES: case YUGABYTE: - return pgNewRecord(dataType.getType(), null, ctx.statement().getObject(ctx.index())); + return pgNewRecord(ctx, dataType.getType(), (AbstractRow) dataType.getRow(), ctx.statement().getObject(ctx.index())); default: return localExecuteContext(ctx.executeContext(), () -> (Record) ctx.statement().getObject(ctx.index(), typeMap(dataType.getType(), ctx))); @@ -3622,6 +3626,14 @@ public class DefaultBinding implements Binding { return (Record) ctx.input().readObject(); } + static final R readMultiset(BindingGetResultSetContext ctx, DataType type) throws SQLException { + return DefaultResultBinding.readMultiset(ctx, (AbstractRow) type.getRow(), type.getType(), + b -> b, + s -> "[" + s + "]", + s -> "" + s + "" + ).get(0); + } + @Override final int sqltype(Statement statement, Configuration configuration) { return Types.STRUCT; @@ -3641,7 +3653,7 @@ public class DefaultBinding implements Binding { } @SuppressWarnings("unchecked") - private static final T pgFromString(Field field, String string) { + private static final T pgFromString(Scope ctx, Field field, String string) { Converter converter = field.getConverter(); Class type = Reflect.wrapper(converter.toType()); @@ -3702,18 +3714,23 @@ public class DefaultBinding implements Binding { else if (type == UUID.class) return (T) UUID.fromString(string); else if (type.isArray()) - return (T) pgNewArray(field, type, string); + return (T) pgNewArray(ctx, field, type, string); else if (EnumType.class.isAssignableFrom(type)) return (T) DefaultEnumTypeBinding.getEnumType((Class) type, string); + else if (Result.class.isAssignableFrom(type)) + if (string.startsWith("<")) + return (T) readMultisetXML(ctx, (AbstractRow) field.getDataType().getRow(), (Class) field.getDataType().getRecordType(), string); + else + return (T) readMultisetJSON(ctx, (AbstractRow) field.getDataType().getRow(), (Class) field.getDataType().getRecordType(), string); else if (Record.class.isAssignableFrom(type) // [#11812] UDTRecords/TableRecords or InternalRecords that don't have an explicit converter && (!InternalRecord.class.isAssignableFrom(type) || type == converter.fromType())) - return (T) pgNewRecord(type, (AbstractRow) field.getDataType().getRow(), string); + return (T) pgNewRecord(ctx, type, (AbstractRow) field.getDataType().getRow(), string); else if (type == Object.class) return (T) string; @@ -3721,7 +3738,7 @@ public class DefaultBinding implements Binding { // which would cause a StackOverflowError, here! else if (type != converter.fromType()) { Converter c = (Converter) converter; - return c.from(pgFromString(field("converted_field", ((ConvertedDataType) field.getDataType()).delegate), string)); + return c.from(pgFromString(ctx, field("converted_field", ((ConvertedDataType) field.getDataType()).delegate), string)); } throw new UnsupportedOperationException("Class " + type + " is not supported"); @@ -3739,11 +3756,12 @@ public class DefaultBinding implements Binding { * @return The converted {@link UDTRecord} */ @SuppressWarnings("unchecked") - static final Record pgNewRecord(Class type, AbstractRow fields, final Object object) { + static final Record pgNewRecord(Scope ctx, Class type, AbstractRow fields, Object object) { if (object == null) return null; - final List values = PostgresUtils.toPGObject(object.toString()); + String s = object.toString(); + List values = PostgresUtils.toPGObject(s); // [#6404] [#7691] // In the event of an unknown record type, derive the record length from actual values. @@ -3762,14 +3780,14 @@ public class DefaultBinding implements Binding { Row row = record.fieldsRow(); for (int i = 0; i < row.size(); i++) - pgSetValue(record, row.field(i), values.get(i)); + pgSetValue(ctx, record, row.field(i), values.get(i)); return record; }); } - private static final void pgSetValue(Record record, Field field, String value) { - record.set(field, pgFromString(field, value)); + private static final void pgSetValue(Scope ctx, Record record, Field field, String value) { + record.set(field, pgFromString(ctx, field, value)); } /** @@ -3781,14 +3799,14 @@ public class DefaultBinding implements Binding { * @param string A String representation of an array * @return The converted array */ - private static final Object[] pgNewArray(Field field, Class type, String string) { + private static final Object[] pgNewArray(Scope ctx, Field field, Class type, String string) { if (string == null) return null; try { return Tools.map( toPGArray(string), - v -> pgFromString(field("array_element", field.getDataType().getArrayComponentDataType()), v), + v -> pgFromString(ctx, field("array_element", field.getDataType().getArrayComponentDataType()), v), size -> (Object[]) java.lang.reflect.Array.newInstance(type.getComponentType(), size) ); } @@ -3831,7 +3849,19 @@ public class DefaultBinding implements Binding { } @SuppressWarnings("unchecked") - private final Result readMultiset(BindingGetResultSetContext ctx, DataType> type) throws SQLException { + static final Result readMultiset(BindingGetResultSetContext ctx, DataType> type) throws SQLException { + return readMultiset(ctx, (AbstractRow) type.getRow(), (Class) type.getRecordType(), identity(), identity(), identity()); + } + + static final Result readMultiset( + BindingGetResultSetContext ctx, + AbstractRow row, + Class recordType, + Function jsonBytesPatch, + Function jsonStringPatch, + Function xmlStringPatch + ) + throws SQLException { NestedCollectionEmulation emulation = emulateMultiset(ctx.configuration()); switch (emulation) { @@ -3839,26 +3869,30 @@ public class DefaultBinding implements Binding { // return copy(ctx, (Multiset) field, ctx.configuration().dsl().fetch(ctx.resultSet().getArray(ctx.index()).getResultSet())); case JSON: - case JSONB: { - if (emulation == NestedCollectionEmulation.JSONB && EMULATE_AS_BLOB.contains(ctx.dialect())) { - byte[] s = ctx.resultSet().getBytes(ctx.index()); - return s == null ? null : new JSONReader<>(ctx.dsl(), (AbstractRow) type.getRow(), (Class) type.getRecordType()).read(new InputStreamReader(new ByteArrayInputStream(s), ctx.configuration().charsetProvider().provide()), true); - } - else { - String s = ctx.resultSet().getString(ctx.index()); - return s == null ? null : new JSONReader<>(ctx.dsl(), (AbstractRow) type.getRow(), (Class) type.getRecordType()).read(new StringReader(s), true); - } - } + case JSONB: + if (emulation == NestedCollectionEmulation.JSONB && EMULATE_AS_BLOB.contains(ctx.dialect())) + return apply(jsonBytesPatch.apply(ctx.resultSet().getBytes(ctx.index())), + s -> new JSONReader<>(ctx.dsl(), row, recordType).read(new InputStreamReader(new ByteArrayInputStream(s), ctx.configuration().charsetProvider().provide()), true)); + else + return apply(jsonStringPatch.apply(ctx.resultSet().getString(ctx.index())), + s -> readMultisetJSON(ctx, row, recordType, s)); - case XML: { - String s = ctx.resultSet().getString(ctx.index()); - return s == null ? null : new XMLHandler<>(ctx.dsl(), (AbstractRow) type.getRow(), (Class) type.getRecordType()).read(s); - } + case XML: + return apply(xmlStringPatch.apply(ctx.resultSet().getString(ctx.index())), + s -> readMultisetXML(ctx, row, recordType, s)); } throw new UnsupportedOperationException("Multiset emulation not yet supported: " + emulation); } + static Result readMultisetXML(Scope ctx, AbstractRow row, Class recordType, String s) { + return new XMLHandler<>(ctx.dsl(), row, recordType).read(s); + } + + static Result readMultisetJSON(Scope ctx, AbstractRow row, Class recordType, String s) { + return new JSONReader<>(ctx.dsl(), row, recordType).read(new StringReader(s), true); + } + @Override final Result get0(BindingGetStatementContext ctx) throws SQLException { return ctx.configuration().dsl().fetch(convert(ctx.statement().getObject(ctx.index()), ResultSet.class)); diff --git a/jOOQ/src/main/java/org/jooq/impl/EmbeddableTableField.java b/jOOQ/src/main/java/org/jooq/impl/EmbeddableTableField.java index 3ce4b2ffcb..eb66986cca 100644 --- a/jOOQ/src/main/java/org/jooq/impl/EmbeddableTableField.java +++ b/jOOQ/src/main/java/org/jooq/impl/EmbeddableTableField.java @@ -94,6 +94,7 @@ implements @Override public final void accept(Context ctx) { + // TODO [#12021] [#12706] ROW must consistently follow MULTISET emulation // [#12237] If a RowField is nested somewhere in MULTISET, we must apply // the MULTISET emulation as well, here if (TRUE.equals(ctx.data(DATA_MULTISET_CONTENT))) diff --git a/jOOQ/src/main/java/org/jooq/impl/MultisetDataType.java b/jOOQ/src/main/java/org/jooq/impl/MultisetDataType.java index cb8aaa1ebb..db5379323f 100644 --- a/jOOQ/src/main/java/org/jooq/impl/MultisetDataType.java +++ b/jOOQ/src/main/java/org/jooq/impl/MultisetDataType.java @@ -66,7 +66,7 @@ final class MultisetDataType extends DefaultDataType final Class recordType; @SuppressWarnings("unchecked") - public MultisetDataType(AbstractRow row, Class recordType) { + MultisetDataType(AbstractRow row, Class recordType) { // [#11829] TODO: Implement this correctly for ArrayRecord super(null, (Class) Result.class, "multiset", "multiset"); diff --git a/jOOQ/src/main/java/org/jooq/impl/RecordDataType.java b/jOOQ/src/main/java/org/jooq/impl/RecordDataType.java index 45c26723f0..b5ee2ba4e0 100644 --- a/jOOQ/src/main/java/org/jooq/impl/RecordDataType.java +++ b/jOOQ/src/main/java/org/jooq/impl/RecordDataType.java @@ -65,12 +65,12 @@ final class RecordDataType extends DefaultDataType { final AbstractRow row; @SuppressWarnings("unchecked") - public RecordDataType(Row row) { + RecordDataType(Row row) { this(row, (Class) recordType(row.size()), "record"); } @SuppressWarnings("unchecked") - public RecordDataType(Row row, Class recordType, String name) { + RecordDataType(Row row, Class recordType, String name) { super(null, recordType, name, name); this.row = (AbstractRow) row; diff --git a/jOOQ/src/main/java/org/jooq/impl/RowField.java b/jOOQ/src/main/java/org/jooq/impl/RowField.java index 4cbb276620..7e3e0bbeca 100644 --- a/jOOQ/src/main/java/org/jooq/impl/RowField.java +++ b/jOOQ/src/main/java/org/jooq/impl/RowField.java @@ -115,21 +115,8 @@ final class RowField extends AbstractField< this(row, N_ROW); } - @SuppressWarnings({ "unchecked", "rawtypes" }) RowField(final ROW row, Name as) { - super(as, new RecordDataType<>(row), CommentImpl.NO_COMMENT, binding(fromNullable( - Object.class, - (Class) Tools.recordType(row.size()), - - // [#7100] In non-PostgreSQL style dialects, RowField is emulated, - // and at conversion time, we already have a synthetic Record[N], - // so no further conversion is required. - t -> (REC) ( - t instanceof InternalRecord - ? t - : pgNewRecord(Record.class, (AbstractRow) row, t) - ) - ))); + super(as, new RecordDataType<>(row)); this.row = row; } @@ -239,7 +226,9 @@ final class RowField extends AbstractField< break; + // case ARRAY: case NATIVE: + default: acceptDefault.accept(ctx); break; } diff --git a/jOOQ/src/main/java/org/jooq/impl/Tools.java b/jOOQ/src/main/java/org/jooq/impl/Tools.java index 9da664f43b..669de623fc 100644 --- a/jOOQ/src/main/java/org/jooq/impl/Tools.java +++ b/jOOQ/src/main/java/org/jooq/impl/Tools.java @@ -1916,6 +1916,11 @@ final class Tools { return t == null ? null : f.apply(t); } + static final T let(T t, Consumer consumer) { + consumer.accept(t); + return t; + } + static final boolean anyMatch(T[] array, ThrowingPredicate test) throws E { return findAny(array, test, t -> TRUE) != null; } diff --git a/jOOQ/src/main/java/org/jooq/impl/Val.java b/jOOQ/src/main/java/org/jooq/impl/Val.java index 0c444510c3..b3d6007508 100644 --- a/jOOQ/src/main/java/org/jooq/impl/Val.java +++ b/jOOQ/src/main/java/org/jooq/impl/Val.java @@ -161,6 +161,7 @@ final class Val extends AbstractParam implements QOM.Val, UEmpty { public void accept(Context ctx) { if (getDataType().isEmbeddable()) { + // TODO [#12021] [#12706] ROW must consistently follow MULTISET emulation // [#12237] If a RowField is nested somewhere in MULTISET, we must apply // the MULTISET emulation as well, here if (TRUE.equals(ctx.data(DATA_MULTISET_CONTENT)))