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

Conversion failure when handling ARRAY types with HSQLDB #6452

Closed
rtomsick opened this issue Jul 25, 2017 · 21 comments
Closed

Conversion failure when handling ARRAY types with HSQLDB #6452

rtomsick opened this issue Jul 25, 2017 · 21 comments

Comments

@rtomsick
Copy link

rtomsick commented Jul 25, 2017

Description

I am unable to get a table with an ARRAY column to work properly with jOOQ's DSL API (no code-gen) when using a non-Postgres DB (in this case HSQLDB)

Environment

Java 8, HSQLDB 2.3.4, jOOQ 3.9.3 (repro'd back to 3.9.0)

Steps to reproduce

  1. Set up a database with a table such as the following:
CREATE TABLE "users" (
	"id"				serial NOT NULL,
	"username"			text NOT NULL,
	"name"				text NOT NULL,
	"active"			boolean NOT NULL DEFAULT true,
	"modified"			timestamp with time zone NOT NULL,
	"roles"				text ARRAY NOT NULL,
	
	PRIMARY KEY ("id"),
	UNIQUE ("username")
);
  1. Attempt to execute a query such as the following:
return this.dsl
	.selectFrom(TABLE)
	.where(field(name("id")).in(ids))
	.fetchMap(field(name("id"), Integer.class),
		r -> {
			final Collection<GrantedAuthority> roles = 
					Stream.of(r.get(field(name("roles"), String[].class), 
							String[].class))
						.map(SimpleGrantedAuthority :: new)
						.collect(Collectors.toList());
			
			return new User(r.get(name("id"), Integer.class), 
					r.get(name("username"), String.class), 
					r.get(name("name"), String.class), 
					r.get(name("modified"), Timestamp.class).toInstant(),
					r.get(name("active"), Boolean.class), 
					roles);
		});

Expected outcome

Query completes successfully (as it does under PostgreSQL 9.4).

Actual outcome:

An exception is thrown, sample trace as follows:


org.jooq.exception.DataTypeException: Cannot convert from ARRAY['superuser'] (class org.hsqldb.jdbc.JDBCArray) to class [Ljava.lang.String;
	at org.jooq.tools.Convert$ConvertAll.fail(Convert.java:1118) ~[jooq-3.9.3.jar:na]
	at org.jooq.tools.Convert$ConvertAll.from(Convert.java:1007) ~[jooq-3.9.3.jar:na]
	at org.jooq.tools.Convert.convert0(Convert.java:316) ~[jooq-3.9.3.jar:na]
	at org.jooq.tools.Convert.convert(Convert.java:308) ~[jooq-3.9.3.jar:na]
	at org.jooq.tools.Convert.convert(Convert.java:380) ~[jooq-3.9.3.jar:na]
	at org.jooq.impl.AbstractRecord.get(AbstractRecord.java:230) ~[jooq-3.9.3.jar:na]
	at com.example.appservice.storage.UserStorageImpl.lambda$0(UserStorageImpl.java:141) ~[classes/:na]
	at org.jooq.impl.ResultImpl.intoMap0(ResultImpl.java:1544) ~[jooq-3.9.3.jar:na]
	at org.jooq.impl.ResultImpl.intoMap(ResultImpl.java:1537) ~[jooq-3.9.3.jar:na]
	at org.jooq.impl.AbstractResultQuery.fetchMap(AbstractResultQuery.java:813) ~[jooq-3.9.3.jar:na]
	at org.jooq.impl.SelectImpl.fetchMap(SelectImpl.java:3143) ~[jooq-3.9.3.jar:na]
	at com.example.appservice.storage.UserStorageImpl.load(UserStorageImpl.java:138) ~[classes/:na]
	at com.example.appservice.storage.UserStorageImpl.load(UserStorageImpl.java:128) ~[classes/:na]
	at com.example.appservice.storage.UserStorageImpl.find(UserStorageImpl.java:273) ~[classes/:na]

Workaround

Use raw java.sql.Array as the field type, go from there.

@rtomsick rtomsick changed the title Apparent bug when handling ARRAY types with HSQLDB Conversion failure when handling ARRAY types with HSQLDB Jul 25, 2017
@rtomsick
Copy link
Author

Also of note: the following also fails.

r.get(field(name("roles"), String[].class))

Produces the following trace:

java.lang.ClassCastException: org.hsqldb.jdbc.JDBCArray cannot be cast to [Ljava.lang.String;
	at com.example.appservice.storage.UserStorageImpl.lambda$0(UserStorageImpl.java:141) ~[classes/:na]
	at org.jooq.impl.ResultImpl.intoMap0(ResultImpl.java:1544) ~[jooq-3.9.3.jar:na]
	at org.jooq.impl.ResultImpl.intoMap(ResultImpl.java:1537) ~[jooq-3.9.3.jar:na]
	at org.jooq.impl.AbstractResultQuery.fetchMap(AbstractResultQuery.java:813) ~[jooq-3.9.3.jar:na]
	at org.jooq.impl.SelectImpl.fetchMap(SelectImpl.java:3143) ~[jooq-3.9.3.jar:na]
	at com.example.appservice.storage.UserStorageImpl.load(UserStorageImpl.java:138) ~[classes/:na]
	at com.example.appservice.storage.UserStorageImpl.load(UserStorageImpl.java:128) ~[classes/:na]
	at com.example.appservice.storage.UserStorageImpl.find(UserStorageImpl.java:273) ~[classes/:na]

@rtomsick
Copy link
Author

Next idea: get it back as an Object array and cast.

Nope:

r.get(field(name("roles"), Object[].class), Object[].class) and r.get(field(name("roles")), Object[].class) both blow up with a stack trace similar to the one in the initial report.

OK, finally the nuclear option: get it back as java.sql.Array and go from there.

Sure, that works... but at that point we've lost any vague semblance of type safety, let alone a pleasant API.

@lukaseder
Copy link
Member

Indeed, this conversion is currently not supported. Interesting. We do support the inverse conversion (from Object[] types to java.sql.Array). I've created a feature request for this: #6454

The workaround is simple: Use an explicit SELECT list:

Field<Integer> id = r.get(name("id"), Integer.class);
Field<String[]> roles = field(name("roles"), String[].class);
return this.dsl
	.select(id, roles, ...)
        .from(TABLE)
	.where(id).in(ids))
	.fetchMap(id,
		r -> {
			final Collection<GrantedAuthority> rolesCollection = 
					Stream.of(r.get(roles))
						.map(SimpleGrantedAuthority :: new)
						.collect(Collectors.toList());
			
			return new User(r.get(roles), ...,
					rolesCollection );
		});

@lukaseder
Copy link
Member

I'm closing this as a duplicate of #6454

@lukaseder lukaseder marked this as a duplicate of #6454 Jul 26, 2017
@rtomsick
Copy link
Author

Great, thanks! I'll give that a shot.

One aside though: why does the code in the initial report work under PostgreSQL but not other DBs?

@lukaseder
Copy link
Member

If I remember correctly, the PostgreSQL JDBC driver deserialises arrays as Java Object[], not as java.sql.Array. This is why this works out of the box for PostgreSQL, but not yet for HSQLDB.

@rtomsick
Copy link
Author

rtomsick commented Jul 26, 2017

Ah. That makes sense. I'll use the above workaround (which works fine, BTW) until #6454 lands. Thanks!

@lukaseder
Copy link
Member

I wouldn't see it as a workaround. If you know the exact type you want to project, it's always better to pass that to the jOOQ SELECT clause explicitly. Also, if you can, you should avoid SELECT *...

@rtomsick
Copy link
Author

Fair enough. I think we're getting into software engineering philosophy here, so we'll have to agree to disagree on that particular point. 😉

Bottom line is, the above approach works fine and solves the issue I originally encountered.

Thanks as always!

@lukaseder
Copy link
Member

Well, we can disagree on SELECT * (although, I have strong arguments 😉), but the jOOQ part, I'm certain of. If you tell jOOQ in the select() clause about how to fetch your data types from JDBC through actual fields (sometimes with converters and/or bindings), then jOOQ will currently do a much better job at deserialising your results than if you try to map the auto-fetched results to concrete types afterwards. That's probably a flaw in jOOQ, but that's how it works today. Especially if you're using more than one database / JDBC driver, because, well, you saw the difference with arrays...

@rtomsick
Copy link
Author

I was referring to the SELECT * bit.

The jOOQ bit I'm all ears for. You are kinda the world expert. ;)

Is there any performance advantage to specifying the fields from jOOQ's perspective (leaving the DB's performance characteristics out of it), or is it just typing?

@lukaseder
Copy link
Member

There's the slight advantage of not having to instanciate any intermediate types (like said JDBCArray) prior to creating the more interesting type (like String[]). That's probably marginal, though.

Of course, if you're selecting all columns, say 500 of them, and then reject most of them in the client, that also incurs significant overhead in the client's CPU and memory consumption. This is not really just part of the DB's performance characteristics...

@rtomsick
Copy link
Author

Oh yeah, I should have clarified I was talking about the case where you're actually using all of the stuff that you bring back with a SELECT *.

I'll keep the GC pressure in mind.

@lukaseder
Copy link
Member

Oh yeah, I should have clarified I was talking about the case where you're actually using all of the stuff that you bring back with a SELECT *.

I see. No, then there's no significant advantage to specify fields in the SELECT clause from a performance perspective

@lukaseder
Copy link
Member

In fact, while #6454 is a nice to have addition, the real problem here is simply that jOOQ doesn't recognise the INTEGER ARRAY data type as reported by ResultSetMetaData. We're already doing this for PostgreSQL:
https://github.com/jOOQ/jOOQ/blob/version-3.9.4/jOOQ/src/main/java/org/jooq/impl/DefaultDataType.java#L771

So, it would make perfect sense to implement this behaviour also for HSQLDB:
#6466

@rtomsick
Copy link
Author

Huh? How would INTEGER ARRAY help in this case? (Not that it's not good to have, just I'm unsure how that ties in to the above)

@lukaseder
Copy link
Member

Sorry for the confusion. I wrote a test case with integer arrays, in case of which INTEGER ARRAY is the type reported by the HSQLDB JDBC API. In your case, it would obviously be VARCHAR ARRAY

@rtomsick
Copy link
Author

Gotcha.

Thanks for tackling this, BTW. jOOQ continues to impress me in how comprehensive it is re: database support.

@lukaseder
Copy link
Member

Thanks for your nice words. Yeah, jOOQ has seen 1-2 things in the last 9 years :)

@rtomsick
Copy link
Author

rtomsick commented Jul 28, 2017

Well you'll continue to have a steady stream of users with cross-DB use cases like mine. And some of us users will be stubborn/dumb enough to use the API without codegen. So plenty of good test coverage there... 😉

@lukaseder
Copy link
Member

Sure, that's where the interesting edge cases show up :)

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

No branches or pull requests

2 participants