Cannot insert into table with DEFAULT value #695

Closed
balazs-zsoldos opened this Issue Apr 1, 2014 · 11 comments

Comments

Projects
None yet
2 participants
@balazs-zsoldos
Contributor

balazs-zsoldos commented Apr 1, 2014

Imagine that there is a table called 'resource' with only one auto_increment column called 'resourceId'.

The following code snippet runs well on H2 but fails on Postgres:

QResource qResource = new QResource("qResource");
SQLInsertClause insertClause = new SQLInsertClause(connection, sqlTemplates, qResource);
id = insertClause.executeWithKey(qResource.resourceId);

The generated SQL is (works with H2, tested):

insert into "res_resource" values ()

In PostgreSQL this does not work. The following would work (tested):

insert into "res_resource" values (DEFAULT)

DEFAULT should work on SQLServer, MySQL, CUBRID, and Oracle as well (not tested).

In MySQL NULL also works (not tested):

insert into "res_resource" values (NULL)

In SQLite columns with default values should not appear in values (question is how it behaves with one column table).

I could imagine to have a DefaultExpressionImpl like ConstantExpressionImpl that could be translated with SQLTemplates to the necessary value. With many of the database engines this could do the trick. The problem could come when a DB engine needs the default columns to be left out from values.

As much as I see the "DEFAULT VALUES" keyword is supported with many database engines as well. This could be a function of SQLInsertClause.

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest Apr 1, 2014

Member

Do you think DEFAULT could be at first used just under the hood? It could be internally enabled if no bindings are given.

Member

timowest commented Apr 1, 2014

Do you think DEFAULT could be at first used just under the hood? It could be internally enabled if no bindings are given.

@balazs-zsoldos

This comment has been minimized.

Show comment
Hide comment
@balazs-zsoldos

balazs-zsoldos Apr 1, 2014

Contributor

Thinking loudly, I will test the followings tomorrow:

There could be an defaultValues() function in SQLTemplates that would be called under the hood if no values are provided in the insert clause. The function would provide the following:

  • CUBRID: DEFAULT VALUES
  • Postgresql DEFAULT VALUES // tested
  • MySQL: VALUES() // tested
  • H2: VALUES() // tested
  • HSQLDB: VALUES()
  • Oracle: VALUES()
  • SQLServer DEFAULT VALUES
  • SQLite: : DEFAULT VALUES
  • Derby: VALUES (DEFAULT) // only works if there is only one column in the table.
  • Teradata: DEFAULT VALUES

I will test them tomorrow. Not all of them but I can test Postgres, H2, HSQLDB, Oracle, SQLServer, SQLite and Derby. The other should be ok based on google-ing :)

As much as I see the SQLSerializer class should be changed a little bit in the method serializeForInsert(). In case the list of values is empty, the SQLTemplates instance should be called.

Contributor

balazs-zsoldos commented Apr 1, 2014

Thinking loudly, I will test the followings tomorrow:

There could be an defaultValues() function in SQLTemplates that would be called under the hood if no values are provided in the insert clause. The function would provide the following:

  • CUBRID: DEFAULT VALUES
  • Postgresql DEFAULT VALUES // tested
  • MySQL: VALUES() // tested
  • H2: VALUES() // tested
  • HSQLDB: VALUES()
  • Oracle: VALUES()
  • SQLServer DEFAULT VALUES
  • SQLite: : DEFAULT VALUES
  • Derby: VALUES (DEFAULT) // only works if there is only one column in the table.
  • Teradata: DEFAULT VALUES

I will test them tomorrow. Not all of them but I can test Postgres, H2, HSQLDB, Oracle, SQLServer, SQLite and Derby. The other should be ok based on google-ing :)

As much as I see the SQLSerializer class should be changed a little bit in the method serializeForInsert(). In case the list of values is empty, the SQLTemplates instance should be called.

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest Apr 2, 2014

Member

@balazs-zsoldos Thanks for looking into this.

Member

timowest commented Apr 2, 2014

@balazs-zsoldos Thanks for looking into this.

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest Apr 3, 2014

Member

@balazs-zsoldos Would you be interested to provide a pull request for this?

Member

timowest commented Apr 3, 2014

@balazs-zsoldos Would you be interested to provide a pull request for this?

@balazs-zsoldos

This comment has been minimized.

Show comment
Hide comment
@balazs-zsoldos

balazs-zsoldos Apr 3, 2014

Contributor

Yes of course. Unfortunately, yesterday I was working on another computer where there are no test DB systems installed. I hope I will have time to test the solution today. A pull request may come today or if not, on Tuesday.

Contributor

balazs-zsoldos commented Apr 3, 2014

Yes of course. Unfortunately, yesterday I was working on another computer where there are no test DB systems installed. I hope I will have time to test the solution today. A pull request may come today or if not, on Tuesday.

@balazs-zsoldos

This comment has been minimized.

Show comment
Hide comment
@balazs-zsoldos

balazs-zsoldos Apr 9, 2014

Contributor

Please check these two commits:

balazs-zsoldos@d999b52
balazs-zsoldos@ab33e55

I am sorry but I have no more time to test with databases. Based on google, every other databases should be ok (that are not tested).

Shall I write unit tests? If yes, how? I can imagine writing tests into the *TemplatesTest classes.

The only database where I could not find a perfect solution is Derby. It does not support any of the syntax. I made a solution that works if there is only one column in the table but it fails if there are more columns. In that case at least one column should be added into the insert into statement with the default keyword.

If everything is ok, I will send the pull request.

Contributor

balazs-zsoldos commented Apr 9, 2014

Please check these two commits:

balazs-zsoldos@d999b52
balazs-zsoldos@ab33e55

I am sorry but I have no more time to test with databases. Based on google, every other databases should be ok (that are not tested).

Shall I write unit tests? If yes, how? I can imagine writing tests into the *TemplatesTest classes.

The only database where I could not find a perfect solution is Derby. It does not support any of the syntax. I made a solution that works if there is only one column in the table but it fails if there are more columns. In that case at least one column should be added into the insert into statement with the default keyword.

If everything is ok, I will send the pull request.

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest Apr 9, 2014

Member

These commits look.

Shall I write unit tests? If yes, how? I can imagine writing tests into the *TemplatesTest classes.

You can add a test case here https://github.com/mysema/querydsl/blob/master/querydsl-sql/src/test/java/com/mysema/query/InsertBase.java

The test suites are here https://github.com/mysema/querydsl/tree/master/querydsl-sql/src/test/java/com/mysema/query/suites

Member

timowest commented Apr 9, 2014

These commits look.

Shall I write unit tests? If yes, how? I can imagine writing tests into the *TemplatesTest classes.

You can add a test case here https://github.com/mysema/querydsl/blob/master/querydsl-sql/src/test/java/com/mysema/query/InsertBase.java

The test suites are here https://github.com/mysema/querydsl/tree/master/querydsl-sql/src/test/java/com/mysema/query/suites

@balazs-zsoldos

This comment has been minimized.

Show comment
Hide comment
@balazs-zsoldos

balazs-zsoldos Apr 9, 2014

Contributor

I wrote a simple test that inserts into the table survey. The tests fail on two databases:

Derby: I expected that as there are more columns in the survey table.
HSQLDB: I did not expect this: DEFAULT keyword cannot be used as column has no DEFAULT

Based on HSQLDB doc it should work: http://hsqldb.org/web/features200.html See "New Data Manipulation Language Features" chapter. Is it possible that the primary key of the table Surve does not have a default value in HSQLDB? It has in Postgres or MySQL.

How shall we proceed? Shall we switch the test off in case of these two databases?

In case of Derby I can imagine only a solution where the Default type is implemented like ConstantImpl so the use can define the value "DEFAULT" by hand when he/she creates an InsertClause.

Contributor

balazs-zsoldos commented Apr 9, 2014

I wrote a simple test that inserts into the table survey. The tests fail on two databases:

Derby: I expected that as there are more columns in the survey table.
HSQLDB: I did not expect this: DEFAULT keyword cannot be used as column has no DEFAULT

Based on HSQLDB doc it should work: http://hsqldb.org/web/features200.html See "New Data Manipulation Language Features" chapter. Is it possible that the primary key of the table Surve does not have a default value in HSQLDB? It has in Postgres or MySQL.

How shall we proceed? Shall we switch the test off in case of these two databases?

In case of Derby I can imagine only a solution where the Default type is implemented like ConstantImpl so the use can define the value "DEFAULT" by hand when he/she creates an InsertClause.

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest Apr 9, 2014

Member

You can ignore the test for HSQLDB and DERBY for now.

Member

timowest commented Apr 9, 2014

You can ignore the test for HSQLDB and DERBY for now.

@balazs-zsoldos

This comment has been minimized.

Show comment
Hide comment
@balazs-zsoldos

balazs-zsoldos Apr 10, 2014

Contributor

Pull request sent: #708

Contributor

balazs-zsoldos commented Apr 10, 2014

Pull request sent: #708

@timowest timowest added the fixed label Apr 10, 2014

@timowest timowest modified the milestone: 3.3.3 Apr 13, 2014

@timowest timowest modified the milestone: 3.3.3 Apr 30, 2014

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest May 2, 2014

Member

Released in 3.3.3

Member

timowest commented May 2, 2014

Released in 3.3.3

@timowest timowest closed this May 2, 2014

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