Support non prepared statements #25
Conversation
| # :nodoc: | ||
| def self.fetch_bool(params : HTTP::Params, name, default : Bool) | ||
| if value = params[name]? | ||
| value.underscore == "true" |
There was a problem hiding this comment.
oops. changed to downcase and also I validate the value now.
* rename QueryMethods#prepare to QueryMethods#build * rename Connection#build_statement to Connection#build_prepared_statement * add Connection#build_unprepared_statement * add Connection #prepared and #unprepared dsl methods
* use ?prepared_statements=true|false on connection string (default: true) * make inmutable state in database * make mutable state in connection * change Connection#build to use the current prepared_statements flag to build prepared or unprepared statements.
unprepared statements are executed in any free connection of the pool at the moment of executing them
bcardiff
force-pushed
the
feature/unprepared
branch
from
13e7b78
to
da2831c
Compare
|
|
||
| # builds a statement over a real connection | ||
| private def build_statement | ||
| @db.pool.checkout.unprepared.build(@query) |
There was a problem hiding this comment.
I'd rather call build_unprepared_statement directly here, to avoid creating the extra intermediate struct
There was a problem hiding this comment.
since it is a struct I would bet llvm will inline that. We could even add the @[AlwaysInline] maybe. I prefer use public api if possible. build_(un)prepared_statements are to be defined by drivers but I wouldn't suggest to use them as public api: they are too verbose.
There was a problem hiding this comment.
Good point on the inlining!
|
LGTM. We seriously need to discuss documentation, though :-P |
|
Out of curiosity, whats the use case for this? The problem with an uprepared statement is that people start using them, aka PHP in the 90s and early 2000s and start generating SQL injection hell. Is there a solid reason to support using them, or is there something I'm missing here? |
|
@shayneoneill prepared statement consume resources in the database. With long living connections this could be a problem. There is also no profit in using prepared statements if the statements changes often like in dynamic queries. The author of crystal-pg also vouched for using non prepared statements as per his experience in heroku. SQL injection is independent of prepared/unprepared statements. You can always concatenate ugly strings and submitted. The difference is if you want to keep a handle to that statements for later re-execution without the need to build an execution plan again. |
fixes #3
Breaking changes:
Connection#buildis renamed toConnection#build_prepared_statement.Connection#build_unprepared_statementis added.Added:
prepared_statements=true|false(default:true) to the connection uri. This is forwarded toDatabaseandConnectionas#prepared_statements? : BoolDatabaseandConnectionhas also#preparedand#unpreparedmethods that returnQueryMethodinstances to query/exec prepared and unprepared statements despite the connection uri.Connection#prepared(sql : String)andDatabase#prepared(sql: String)should be used to create prepared statements explicitly that will be executed later. Note: there is a cache by sql statement so same query is not prepared twice on connection. This is not new, but proper interface was pending.A
SessionMethodsmodule is added to share code betweenDatabaseandConnectionregarding how to build statements of the different kinds. When transactions are implemented this will be helpful also. This module is generic due to implementation details:Databaseworks withPoolStatementandConnectionworks withStatement.cc: @spalladino @asterite