Configurable PropertyAccessInvocationHandler

Bug #839349 reported by John Cook
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Querydsl
Fix Released
Undecided
Unassigned

Bug Description

It would be great to have configurable behaviour for proxy used for QueryDSL alias feature. For example, to make it possible to pass a custom implementation of MethodInterceptor for Alias.alias(...) methods to allow customization of the alias feature.

Revision history for this message
Timo Westkämper (timo-westkamper) wrote :

What kind of customization do you have in mind? Alternatively I could make PropertyAccessInvocationHandler public and then the customization could be done that way.

Changed in querydsl:
status: New → In Progress
Revision history for this message
John Cook (smoulat) wrote :

Thank you for a quick answer.

Here's some reasoning behind this request:
Well, the reason why we ask for this is because our domain model uses hard convention over configuration naming, so there's no problem for us to translate property name on the domain object to column name in database table. Also, in our project, we have quite many domain objects and most of the queries are performed through JPA/Hibernate, with some native SQL queries. We would like to use QueryDSL for native SQL queries too (because it's such a great tool :)) but we would like to avoid generating so much SQL query times seldom used. Therefore, we want to use QueryDSL alias feuture for those couple native SQL queries in that way, that instead of Path(s) for property names, we want to generate Path(s) for table column names just like in generated SQL query types. Therefore, we would appreciate to have alias feature configurable.

As for implementation, here are some possibilities, my first proposition was not good enough as our team leader advised to me - it exposes alias feature implementation using CGLIB (which may be changed later) to public API. So:
1) Your proposal is quite good for us with two small additions - we would also need to make AliasFactory.createProxy() protected, and also make PropertyAccessInvocationHandler more subclassing-friendly, allowing parts of creating PathMetadata for individual cases (GETTER, SCALA_GETTER, etc.) to be overwritten in a subclass (and keep the rest like metadata caching etc. private in superclass).
2) Something similar with slight difference - edit the PropertyAccessInvocationHandler class in the way explained above and to introduce some kind of pluggable strategy interface for handling PathMetadata creation itself.

What do you think about it? Or is there any better way to use QueryDSL for native SQL queries without generating SQL query types?

Revision history for this message
Timo Westkämper (timo-westkamper) wrote :

The approach of generating SQL query types this way is not very expressive. There are lots of query expressions you wouldn't be able to generate this way easily such as

* joins
* properties from embeddables
* columns from mixed table inheritance

I don't see good alternatives to using the generated SQL types. You can restrict the tables to be mirrored via the schemaPattern and tableNamePattern properties.

Even if you would manage to generate proper expressions, your queries would become really verbose.

Compare this

from($(user)).innerJoin($(employee)).on($(user.getEmployee().getId()).eq($(employee.getId()));

with

SUser user = SUser.user;
SEmployee employee = SEmployee.employee;
from(user).innerJoin(user.employeeFk, employee);

Try to sketch some queries involving joins with your imagined approach. You will see that it doesn't scale.

Revision history for this message
John Cook (smoulat) wrote :

Yes, we're aware of this. However, as I said, it's quite exceptional case to use native SQL queries, so the additional verbosity is not a big deal. Also, our proposed variant 1) is no change for QueryDSL API, it just leaves a door a little for open for alternative implementation.

I attached my proposal of slight edition of the PropertyAccessInvocationHandler, please, let me know, if you're OK with it (and with making AliasFactory.createProxy() method protected)

Revision history for this message
John Cook (smoulat) wrote :
Revision history for this message
Timo Westkämper (timo-westkamper) wrote :

I made the changes you requested. You need also to remember that for Querydsl SQL you need to create RelationalPath instances as root paths.

Revision history for this message
John Cook (smoulat) wrote :

Thank you, Timo, it'll help us a lot!

Revision history for this message
Timo Westkämper (timo-westkamper) wrote :

I just deployed the version 2.2.1.BUILD-SNAPSHOT.

Changed in querydsl:
status: In Progress → Fix Committed
Revision history for this message
Timo Westkämper (timo-westkamper) wrote :

Released in 2.2.2

Changed in querydsl:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.