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

createArrayOf fails if byte[][] erased to Object[] #2630

Open
pingw33n opened this issue Sep 23, 2022 · 11 comments
Open

createArrayOf fails if byte[][] erased to Object[] #2630

pingw33n opened this issue Sep 23, 2022 · 11 comments

Comments

@pingw33n
Copy link

Postgres driver: org.postgresql:postgresql:42.5.0

Reproducer:

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

    @Test
    void test() throws Exception {
        try (var c = DriverManager.getConnection(postgres.getJdbcUrl(), postgres.getUsername(), postgres.getPassword())) {
            c.createArrayOf("bytea", new Object[]{new byte[]{1, 2, 3}});
        }
    }
}

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 ttt.PgTest.test(PgTest.java:19)

Original issue in JDBI: jdbi/jdbi#2109

The problem is that the code uses getComponentType() to select ArrayEncoder in ArrayEncoding.getArrayEncoder() which obviously fails for erased Object[] and falls back to using OBJECT_ARRAY which for some reason doesn't support encoding byte[] elements. I think it should be possible to addiitionally use the first element to detect element type falling back to OBJECT_ARRAY for empty arrays only.

@davecramer
Copy link
Member

Interesting @bokken I believe you have some insight into this ?

@bokken
Copy link
Member

bokken commented Sep 23, 2022

There is discussion on #2226.
The specific carve out for byte[] is mentioned there as a use case which has never been supported, and the primary focus there was the regression from previous support.

I think it would work to use PGbytea.toPGString((byte[])array[i]) (presumably in quotes). The BYTEA_ARRAY does the string representation using HEX, perhaps that could be refactored out to be reusable

@davecramer
Copy link
Member

So I guess the issues here is we don't know whether this is an array of bytes or a string?

@bokken
Copy link
Member

bokken commented Sep 23, 2022

we don't know whether this is an array of bytes or a string

There is some ambiguity there, but /assuming/ an array of bytea seems reasonable enough in this scenario.
Part of the question is why would one represent a byte[][] as an Object[], where the values are byte[]. A byte[][] is an Object[], so any api (such as Connection.createArrayOf(String, Object[])) can be given a byte[][] directly.

@davecramer
Copy link
Member

There is some ambiguity there, but /assuming/ an array of bytea seems reasonable enough in this scenario.

if this is the case should we implement it this way ?

@pingw33n
Copy link
Author

Part of the question is why would one represent a byte[][] as an Object[], where the values are byte[].

As an example, one can have createArray<T, U>(String typeName, Collection<T> items, Function<T, U> mapper) and U is erased so it's not possible to construct typed array.

Looking at JDBC docs, it doesn't restrict elements to be in either of the forms, so implementors must support both.

@davecramer
Copy link
Member

Looking at JDBC docs, it doesn't restrict elements to be in either of the forms, so implementors must support both.

Not sure I agree with this opinion; but I'm not ruling it out. There is a considerably amount of ambiguity in the spec.

@bokken
Copy link
Member

bokken commented Sep 23, 2022

As an example, one can have createArray<T, U>(String typeName, Collection<T> items, Function<T, U> mapper) and U is erased so it's not possible to construct typed array.

That is not part of jdbc spec we are implementing, so not sure how relevant it is.
Even then it is still /possible/ to construct a typed array, it is just not convenient. The same instanceof check/flexing could be done there.

Looking at JDBC docs, it doesn't restrict elements to be in either of the forms, so implementors must support both.

I agree with @davecramer that there is much leeway/ambiguity in the spec. For example oracle does not support this method at all.

This is a specific use case which has never been supported by pgjdbc. It might be reasonable to add support.

@hgschmie
Copy link

Hi (JDBI code author/maintainer here),

What our code does is that we receive a request to bind a varargs list of byte[] arrays onto a specific column (which maps onto a bytea []. So our code at some point calls

                java.sql.Array sqlArray = statement.getConnection().createArrayOf(typeName, array);
                statement.setArray(position, sqlArray);

with typeName being "bytea" and the array being an Object[] which contains byte[] arrays. While I understand that it would be preferable for this to be a byte[][], our calling API does not work this way (because users call our code with a varargs list of regular byte arrays). And our arrays are not uniform but "ragged" (the Object[] may contain a byte[10], then a byte[30], then a byte[5]) and this should be adequately reflected in the resulting data in the DB (which would not work with a byte[][] where all elements in the array would end up with the same length.

I feel that this is an actual driver limitation, because the DB itself seems to support this just fine.

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
@davecramer
Copy link
Member

I think we are open to the implementation. Care to provide a PR ?

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 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
@davecramer
Copy link
Member

@hgschmie any reason not to provide a PR here ?

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

No branches or pull requests

4 participants