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

GenericExporter doesn't check for reserved keywords when choosing identifiers #93

Closed
micdah opened this Issue Feb 3, 2012 · 7 comments

Comments

Projects
None yet
2 participants
@micdah

micdah commented Feb 3, 2012

Recently we introduced AspectJ into our project, which also uses QueryDSL for an easier interface for JPA Queries. In this endeavor we had to start using the GenericExporter class instead of the APT plugin for Eclipse, as it seems to collide with the AJDT builder (solution found via: #56), specifically APT generated Q-files would get deleted once the AJDT Builder was applied, which it has do be after the Java Builder.

Anyway, enough backstory - the problem we are having, is that GenericExporter doesn't seem to check for reserved keywords when choosing identifiers in the generated Q-files, specifically we have a Entity class called "Group" (which is a reserved keyword in HQL - we are building on top of Hibernate).

Specifically the EntityPathBase reference being generated in the QGroup class, is using a reserved keyword as identifier, resulting in HQL errors when using queries involving said entity class:
public class QGroup extends EntityPathBase {
...
public static final QGroup group = new QGroup("group");
...
}

When generating the samme QGroup class using the JPAAnnotationProcessor within Eclipse (or using the Maven plugin), it seems to correctly recognise "group" as being a reserved keyword and chooses "group1" instead:

public class QGroup extends EntityPathBase {
...
public static final QGroup group = new QGroup("group1");
...
}

The codebase is identical in both cases, so the determining factor would be the use of GenericExporter instead of APT.

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest Feb 4, 2012

Member

Thanks for the bug report. I will make the keyword escaping also available in the GenericExporter with the same keywords sets available like in the APT processor.

Member

timowest commented Feb 4, 2012

Thanks for the bug report. I will make the keyword escaping also available in the GenericExporter with the same keywords sets available like in the APT processor.

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest Feb 4, 2012

Member

You can use the keywords support now by specifiying the set of keywords in capitalized form like this

exporter.setKeywords(Keywords.JPA);

The Keywords class is located in com.mysema.query.codegen.

Member

timowest commented Feb 4, 2012

You can use the keywords support now by specifiying the set of keywords in capitalized form like this

exporter.setKeywords(Keywords.JPA);

The Keywords class is located in com.mysema.query.codegen.

@micdah

This comment has been minimized.

Show comment
Hide comment
@micdah

micdah Feb 4, 2012

Excellent, thanks for the extremely quick response.
Come monday, I will test the additions on our codebase to verify the bug has been fixed (although looking at your commit it does look like you have already checked it ;-).

Are there any development repository for QueryDSL from which it is possible to pull an updated version from (using maven)?

micdah commented Feb 4, 2012

Excellent, thanks for the extremely quick response.
Come monday, I will test the additions on our codebase to verify the bug has been fixed (although looking at your commit it does look like you have already checked it ;-).

Are there any development repository for QueryDSL from which it is possible to pull an updated version from (using maven)?

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest Feb 5, 2012

Member

You can use the latest snapshot from here : http://source.mysema.com/maven2/snapshots/

The version is 2.3.0.BUILD-SNAPSHOT

Member

timowest commented Feb 5, 2012

You can use the latest snapshot from here : http://source.mysema.com/maven2/snapshots/

The version is 2.3.0.BUILD-SNAPSHOT

@micdah

This comment has been minimized.

Show comment
Hide comment
@micdah

micdah Feb 6, 2012

I can verify that the snapshot release with the new JPA keywords, indeed fixes the issue.

Many thanks for an extremely quick response, very much appreciated.
I will now close this issue.

micdah commented Feb 6, 2012

I can verify that the snapshot release with the new JPA keywords, indeed fixes the issue.

Many thanks for an extremely quick response, very much appreciated.
I will now close this issue.

@micdah micdah closed this Feb 6, 2012

@timowest timowest reopened this Feb 6, 2012

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest Feb 6, 2012

Member

I will close it when it's released

Member

timowest commented Feb 6, 2012

I will close it when it's released

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest Feb 7, 2012

Member

released in 2.3.1

Member

timowest commented Feb 7, 2012

released in 2.3.1

@timowest timowest closed this Feb 7, 2012

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