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

[RFC] Placeholder substitution. #297

Closed
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@methane
Contributor

methane commented Dec 31, 2014

mysql-driver doesn't support placeholder substitution currently.
DB.Query() and DB.Exec() with params make 3 round trips: prepare, exec and close.

@julienschmidt

This comment has been minimized.

Member

julienschmidt commented Jan 19, 2015

Prepared statements exist for good reasons. That includes performance (if the same query it executed multiple times, often with different values) and security (proper escaping is hard!).
In some cases placeholder substitution makes sense, but this should be an optional feature.

Maybe it should be just an exported helper function, which allows the user of the driver to build an parameter-less query, which then can be executed by db.Exec or db.Query. Because of the Execer / Queryer interface implementation, this also saves the extra round-trips.

@xaprb

This comment has been minimized.

xaprb commented Jan 19, 2015

This would be a good feature. We wrote about our adventures with it here:
https://vividcortex.com/blog/2014/11/19/analyzing-prepared-statement-performance-with-vividcortex/

Question: if you could enable such a setting, perhaps in the DSN string,
could the driver do it internally so you wouldn't need to alter the
interface or calling code in any way? Or does the driver not get a chance
to intercept and "emulate" such calls?

@methane

This comment has been minimized.

Contributor

methane commented Jan 20, 2015

@julienschmidt Placement holder substitution takes some config from mysqlConn: mc.cfg.loc, mc.maxPamaxPacketAllowed and mc.status I've added in this pull request.
Exposing helper function is possible, but integrating it is safer and usuful.

@xaprb I think adding an option to DSN is good idea, while I don't think PHP's "EMULATED PREPARE" is good word.

@julienschmidt

This comment has been minimized.

Member

julienschmidt commented Jan 20, 2015

Another use: SphinxQL, which does not support prepared statements at all (#221).
We should maybe investigate if we can somehow find out during connecting if the server supports prepared statements.

@xaprb

This comment has been minimized.

xaprb commented Jan 20, 2015

Try to prepare

SELECT 'go driver testing prepared statements'

@julienschmidt julienschmidt added this to the v1.3 milestone Jan 20, 2015

This was referenced Feb 1, 2015

@methane methane closed this Feb 5, 2015

@methane methane deleted the methane:placeholder branch Aug 2, 2016

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