Skip to content
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

Add a new Setting to inline LIMIT values by default #7720

Open
lukaseder opened this issue Aug 6, 2018 · 2 comments
Open

Add a new Setting to inline LIMIT values by default #7720

lukaseder opened this issue Aug 6, 2018 · 2 comments

Comments

@lukaseder
Copy link
Member

lukaseder commented Aug 6, 2018

By default, jOOQ generates bind parameters for all client values that are passed to the DSL, which is a sane choice in most cases.

In some cases, however, defaulting to inlining the value might be more reasonable, specifically the LIMIT clause, which is unlikely to have tons of different possible values. Mostly, people are limiting their queries to 1 or some N, without modifying the value of N between executions.

A new setting could allow to govern this behaviour, inlining LIMIT values by default (it is still possible to create explicit bind parameters using DSL.val()).

Other clauses might be affected by this flag as well. This needs further design

@PhilippSalvisberg
Copy link

Do you really want to change the default behaviour for this particular case? It's a matter of consistency. Right now I know that bind parameters are created for all client values. That's fine. I use DSL.literal() if I do not like it.

I consider using literals instead of bind parameters when a) the values change per execution and b) the changes affects the cardinality strongly. If this is not the case then a bind variable is usually not harmful. The optimizer will find a good execution plan based on bind variable peeking.

So, what exactly is the benefit of changing the default behaviour in this particular case?

@lukaseder
Copy link
Member Author

I didn't say that the default behaviour will be changed by default :-) There will be a setting that allows to change the default.

Also, just because this idea is here on the roadmap doesn't mean it's going to be implemented. I will still need more traction for this idea to actually add it.

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

No branches or pull requests

2 participants