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

SQLQuery option for Statement.setFetchSize(int) #1419

Closed
ssaarela opened this Issue Jun 25, 2015 · 6 comments

Comments

Projects
None yet
4 participants
@ssaarela
Contributor

ssaarela commented Jun 25, 2015

Currently setting fetchSize hint for Statement requires custom implementation using SQLDetailedListener. As Statement.setFetchSize(int) is part of standard JDBC API, it would make sense to implement this in core API.

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest Jun 26, 2015

Member

I suggest that we support setting the following Statement properties on configuration and query level:

  • maxFieldSize
  • maxRows
  • queryTimeout
  • fetchSize

Maybe like

query.setStatementOption(StatementOption.MAX_FIELD_SIZE, maxFieldSize)
Member

timowest commented Jun 26, 2015

I suggest that we support setting the following Statement properties on configuration and query level:

  • maxFieldSize
  • maxRows
  • queryTimeout
  • fetchSize

Maybe like

query.setStatementOption(StatementOption.MAX_FIELD_SIZE, maxFieldSize)
@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest Jun 26, 2015

Member

@ssaarela @Shredder121 @johnktims What do you think about the enum style, it would pollute the SQLQuery namespace less than Bean setters.

Member

timowest commented Jun 26, 2015

@ssaarela @Shredder121 @johnktims What do you think about the enum style, it would pollute the SQLQuery namespace less than Bean setters.

@Shredder121

This comment has been minimized.

Show comment
Hide comment
@Shredder121

Shredder121 Jun 27, 2015

Member

I think that should scale well.
Then we can give an enum constant a default value as field.

Member

Shredder121 commented Jun 27, 2015

I think that should scale well.
Then we can give an enum constant a default value as field.

@ssaarela

This comment has been minimized.

Show comment
Hide comment
@ssaarela

ssaarela Jun 29, 2015

Contributor

On the other hand setters would be more intuitive than enum style - distinct methods work well with auto-complete whereas finding that kind of enum setters would require googling/documentation.

I would leave defaults as unset, null or empty Optional, for all of these. At least fetchSize might slow down other queries and there are other ways of defining timeouts. Instead of setting maxRows, most developers would/should use SQL based limit, etc. If, however, someone wants to use Querydsl for setting these at global level, then using a listener is not that hard.

Contributor

ssaarela commented Jun 29, 2015

On the other hand setters would be more intuitive than enum style - distinct methods work well with auto-complete whereas finding that kind of enum setters would require googling/documentation.

I would leave defaults as unset, null or empty Optional, for all of these. At least fetchSize might slow down other queries and there are other ways of defining timeouts. Instead of setting maxRows, most developers would/should use SQL based limit, etc. If, however, someone wants to use Querydsl for setting these at global level, then using a listener is not that hard.

@gmokki

This comment has been minimized.

Show comment
Hide comment
@gmokki

gmokki Jul 1, 2015

Enum vs setters:

  • the enum cannot have any type-safety (string vs int vs boolean) on the parameter method (of course runtime checks catch problems)
  • adding many setters pollute the main class

How about a StatementOptions immutable object. Default is created with default constructor, builder might be overkill. That way code can apply the same combination of options easily to many statements.

And I agree with ssaarela that defaults should be null or empty Optional. There are no values for the any of the settings that are good for all jdbc drivers.

gmokki commented Jul 1, 2015

Enum vs setters:

  • the enum cannot have any type-safety (string vs int vs boolean) on the parameter method (of course runtime checks catch problems)
  • adding many setters pollute the main class

How about a StatementOptions immutable object. Default is created with default constructor, builder might be overkill. That way code can apply the same combination of options easily to many statements.

And I agree with ssaarela that defaults should be null or empty Optional. There are no values for the any of the settings that are good for all jdbc drivers.

@timowest

This comment has been minimized.

Show comment
Hide comment
@timowest

timowest Jul 1, 2015

Member

An immutable StatementOptions class sounds good, it provides typesafety and adds only a single method in the query signature.

Member

timowest commented Jul 1, 2015

An immutable StatementOptions class sounds good, it provides typesafety and adds only a single method in the query signature.

@timowest timowest added this to the 4.0.3 milestone Jul 31, 2015

@Shredder121 Shredder121 closed this in #1461 Aug 1, 2015

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