Clarify semantics of columns().select() versus set() for INSERT ... SELECT statements #510

Closed
cowwoc opened this Issue Sep 15, 2013 · 7 comments

Comments

Projects
None yet
2 participants
@cowwoc
Contributor

cowwoc commented Sep 15, 2013

I posted an example at http://stackoverflow.com/q/18816748/14731 which shows that QueryDSL allows users to form invalid queries.

It turns out that I needed to replace set(modules.name, setModuleName) with columns(modules.name).select(setModuleName) in order to correct the problem.

Expected behavior: QueryDSL should either prevent construction of invalid queries (compile or runtime failure) or translate set(column, subQuery) into columns(column).select(subQuery) on behalf of the user.

There is also the outstanding question of why QueryDSL is injecting from dual into the query and whether it is possible to correct that as well.

timowest added a commit that referenced this issue Sep 17, 2013

timowest added a commit that referenced this issue Sep 17, 2013

timowest added a commit that referenced this issue Sep 17, 2013

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest Sep 17, 2013

Member

Querydsl inserts dual because it is defined as the dummy table for H2. Does it affect the query semantics?

Both options are syntactically valid INSERT clauses, but with different semantics.

Using

columns(...).select(...)

you will get the conditional insertion you are aiming for and with

set(...)

you will get null values, if the sub query doesn't return any rows.

So I suggest to fix this issue on the documentation level.

Member

timowest commented Sep 17, 2013

Querydsl inserts dual because it is defined as the dummy table for H2. Does it affect the query semantics?

Both options are syntactically valid INSERT clauses, but with different semantics.

Using

columns(...).select(...)

you will get the conditional insertion you are aiming for and with

set(...)

you will get null values, if the sub query doesn't return any rows.

So I suggest to fix this issue on the documentation level.

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest Sep 17, 2013

Member

Also I suggest to update the issue title, since this is not a case of invalid SQL, but of unclear semantics/limited documentation.

Member

timowest commented Sep 17, 2013

Also I suggest to update the issue title, since this is not a case of invalid SQL, but of unclear semantics/limited documentation.

@cowwoc

This comment has been minimized.

Show comment
Hide comment
@cowwoc

cowwoc Sep 18, 2013

Contributor

I've updated the title but I don't think documentation alone will prevent others from making the same mistake. I wish there was something more we could do (but I can't think of anything else either).

Contributor

cowwoc commented Sep 18, 2013

I've updated the title but I don't think documentation alone will prevent others from making the same mistake. I wish there was something more we could do (but I can't think of anything else either).

timowest added a commit that referenced this issue Sep 18, 2013

@cowwoc

This comment has been minimized.

Show comment
Hide comment
@cowwoc

cowwoc Sep 19, 2013

Contributor

On a side-note, how do I preview what the committed documentation looks like (as HTML) before it gets pushed to the live website? I find it hard to digest some of the commits in XML format.

Contributor

cowwoc commented Sep 19, 2013

On a side-note, how do I preview what the committed documentation looks like (as HTML) before it gets pushed to the live website? I find it hard to digest some of the commits in XML format.

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest Sep 19, 2013

Member

You can for example run ./docs.sh in querydsl-root

Member

timowest commented Sep 19, 2013

You can for example run ./docs.sh in querydsl-root

@cowwoc

This comment has been minimized.

Show comment
Hide comment
@cowwoc

cowwoc Sep 20, 2013

Contributor

This runs for a while, then fails with:

[WARNING] Missing POM for net.socialchange.doctype:doctype-changer:jar:1.1
[WARNING] Missing POM for net.sf.docbook:docbook:jar:1.72.0
[WARNING] Missing POM for net.sf.docbook:docbook:jar:1.74.0

Failure to find net.sf.docbook:docbook:jar:1.74.0
Contributor

cowwoc commented Sep 20, 2013

This runs for a while, then fails with:

[WARNING] Missing POM for net.socialchange.doctype:doctype-changer:jar:1.1
[WARNING] Missing POM for net.sf.docbook:docbook:jar:1.72.0
[WARNING] Missing POM for net.sf.docbook:docbook:jar:1.74.0

Failure to find net.sf.docbook:docbook:jar:1.74.0

timowest added a commit that referenced this issue Sep 21, 2013

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest Oct 20, 2013

Member

Released in 3.2.4

Member

timowest commented Oct 20, 2013

Released in 3.2.4

@timowest timowest closed this Oct 20, 2013

@timowest timowest added this to the 3.2.4 milestone Apr 13, 2014

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