Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bindArray doesn't work for bytea[] in Postgres #2109

Closed
pingw33n opened this issue Sep 22, 2022 · 5 comments · Fixed by #2113
Closed

bindArray doesn't work for bytea[] in Postgres #2109

pingw33n opened this issue Sep 22, 2022 · 5 comments · Fixed by #2113

Comments

@pingw33n
Copy link

JDBI version: 3.32.0
Postgres driver: org.postgresql:postgresql:42.5.0

Reproducer:

@Testcontainers
public class JdbiBugTest {
    @Container
    PostgreSQLContainer<?> postgres = new PostgreSQLContainer<>(DockerImageName.parse("postgres:12-alpine"));

    @Test
    void bug() {
        Jdbi.create(postgres.getJdbcUrl(), postgres.getUsername(), postgres.getPassword())
            .registerArrayType(byte[].class, "bytea")
            .withHandle(h -> {
                h.execute("CREATE TABLE tbl (arr bytea[])");
                h.createUpdate("INSERT INTO tbl (arr) VALUES (:arr)")
                    .bindArray("arr", byte[].class, new byte[]{1, 2})
                    .execute();
               return null;
            });
    }
}

Throws:

java.lang.UnsupportedOperationException: byte[] nested inside Object[]

	at org.postgresql.jdbc.ArrayEncoding$15.appendArray(ArrayEncoding.java:1083)
	at org.postgresql.jdbc.ArrayEncoding$15.appendArray(ArrayEncoding.java:1045)
	at org.postgresql.jdbc.ArrayEncoding$AbstractArrayEncoder.toArrayString(ArrayEncoding.java:204)
	at org.postgresql.jdbc.PgConnection.createArrayOf(PgConnection.java:1422)
	at org.postgresql.jdbc.PgConnection.createArrayOf(PgConnection.java:1429)
	at org.jdbi.v3.core.array.SqlArrayArgument.apply(SqlArrayArgument.java:42)
	at org.jdbi.v3.core.statement.ArgumentBinder.bindNamed(ArgumentBinder.java:120)
	at org.jdbi.v3.core.statement.ArgumentBinder.bind(ArgumentBinder.java:68)
	at org.jdbi.v3.core.statement.SqlStatement.internalExecute(SqlStatement.java:1783)
	at org.jdbi.v3.core.result.ResultProducers.lambda$returningUpdateCount$0(ResultProducers.java:46)
	at org.jdbi.v3.core.statement.Update.execute(Update.java:60)
	at org.jdbi.v3.core.statement.Update.execute(Update.java:48)
	at JdbiBugTest.lambda$bug$0(JdbiBugTest.java:24)
	at org.jdbi.v3.core.Jdbi.withHandle(Jdbi.java:357)
	at JdbiBugTest.bug(JdbiBugTest.java:20)

Not sure whether this is a Postgres driver issue, but it wants bytea[] to be of Java type byte[][] but JDBI erases it to Object[]. For other primitive arrays it appears to work fine when converted to Object[] but also precludes the use of more efficient binary encoding.

@hgschmie
Copy link
Contributor

hgschmie commented Sep 23, 2022

This actually seems to be a postgres issue. The driver code reads (In ArrayEncoding.java)

        } else if (array[i].getClass().isArray()) {
          if (array[i] instanceof byte[]) {
            throw new UnsupportedOperationException("byte[] nested inside Object[]");
          }
          try {
            getArrayEncoder(array[i]).appendArray(sb, delim, array[i]);

so they explicitly forbid an array of byte[] (which would translate into bytea[]. Ironically, the getArrayEncoder looks at a list of supported conversions which reads

    ARRAY_CLASS_TO_ENCODER.put(byte[].class, BYTEA_ARRAY);

so I am literally clueless why the driver does not support it.

hgschmie added a commit to hgschmie/jdbi that referenced this issue Sep 28, 2022
Add an explicit array type that creates the postgres internal BYTEA
format so that the driver is willing to write arrays. Fixes jdbi#2109,
workaround for the problem in
pgjdbc/pgjdbc#2630
@hgschmie
Copy link
Contributor

can you try

Jdbi.create(postgres.getJdbcUrl(), postgres.getUsername(), postgres.getPassword())
            .withHandle(h -> {
                h.registerArrayType(new ByteaArrayType());
                h.execute("CREATE TABLE tbl (arr bytea[])");
                h.createUpdate("INSERT INTO tbl (arr) VALUES (:arr)")
                    .bindArray("arr", new byte[]{1, 2})
                    .execute();
               return null;
            });

public final class ByteaArrayType implements SqlArrayType<byte[]> {

    @Override
    public String getTypeName() {
        return "bytea";
    }

    @Override
    public Object convertArrayElement(byte[] element) {
        return PGbytea.toPGString(element);
    }
}

and tell me whether that changes / improves your situation?

@pingw33n
Copy link
Author

The code seems to work, thanks. Though it is sub-optimal because text encoding is used (this is crucial for me to have the most efficient format). For my task this workaround was good enough (Kotlin):

            registerArgument(object : ArgumentFactory.Preparable {
                override fun prepare(type: Type, config: ConfigRegistry): Optional<Function<Any?, Argument>> {
                    val elementType = IterableLike.elementTypeOf(type)
                        .orElse(null) ?: return Optional.empty()
                    val sqlArrType = config.get(SqlArrayTypes::class.java).findFor(elementType)
                        .orElse(null) ?: return Optional.empty()

                    @Suppress("UNCHECKED_CAST")
                    sqlArrType as SqlArrayType<Any>

                    return Optional.of(
                        Function<Any?, Argument> { array ->
                            if (array == null) {
                                NullArgument(Types.ARRAY)
                            } else {
                                var homogeneous = true
                                var effectiveElementType: Class<*>? = null
                                val elements = IterableLike.stream(array)
                                    .map {
                                        sqlArrType.convertArrayElement(it).also { v ->
                                            if (effectiveElementType == null) {
                                                effectiveElementType = v.javaClass
                                            } else if (homogeneous) {
                                                homogeneous = v.javaClass == effectiveElementType
                                            }
                                        }
                                    }
                                    .collect(Collectors.toList())
                                if (!homogeneous) {
                                    effectiveElementType = null
                                }
                                val arr = if (effectiveElementType == null) {
                                    elements.stream().toArray()
                                } else {
                                    elements.stream().toArray { len ->
                                        @Suppress("UNCHECKED_CAST")
                                        java.lang.reflect.Array.newInstance(effectiveElementType, len) as Array<Any>
                                    }
                                }
                                Argument { position, statement, ctx ->
                                    when (ctx.sqlArrayArgumentStrategy) {
                                        SqlArrayArgumentStrategy.SQL_ARRAY -> {
                                            val sqlArray = statement.connection.createArrayOf(sqlArrType.typeName, arr)
                                            ctx.addCleanable { sqlArray.free() }
                                            statement.setArray(position, sqlArray)
                                        }
                                        SqlArrayArgumentStrategy.OBJECT_ARRAY -> statement.setObject(position, array)
                                        null -> throw NullPointerException()
                                    }
                                }
                            }
                        }
                    )
                }
            })

It works for all "special" array types and the driver happily uses binary encoding for non-empty arrays. I think this is what the driver should be doing.

hgschmie added a commit to hgschmie/jdbi that referenced this issue Sep 28, 2022
Add an explicit array type that creates the postgres internal BYTEA
format so that the driver is willing to write arrays. Fixes jdbi#2109,
workaround for the problem in
pgjdbc/pgjdbc#2630
hgschmie added a commit to hgschmie/jdbi that referenced this issue Sep 28, 2022
Add an explicit array type that creates the postgres internal BYTEA
format so that the driver is willing to write arrays. Fixes jdbi#2109,
workaround for the problem in
pgjdbc/pgjdbc#2630
@hgschmie
Copy link
Contributor

My understanding is that this is just the wire format between the driver and the database. The data itself ends up in the same shape on disk whether you send it in the "text" format or as binary (in that case, there is a some recoding but I did not really follow the driver code too closely). Does this cause a problem with the wire format or does the data end up differently in the database?

In any case, if this fixes your problem, I will add this fix to the code base so starting with the next release, this should work out of the box.

hgschmie added a commit to hgschmie/jdbi that referenced this issue Sep 28, 2022
Add an explicit array type that creates the postgres internal BYTEA
format so that the driver is willing to write arrays. Fixes jdbi#2109,
workaround for the problem in
pgjdbc/pgjdbc#2630
@pingw33n
Copy link
Author

Does this cause a problem with the wire format or does the data end up differently in the database?

Yes, it's about the wire format. Text form is larger and harder to parser compared to binary form where it can just copy bytes directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants