Skip to content

Get formatted query from Pool.query() method #991

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.

Sign up for GitHub

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
+76 −2
Open

Get formatted query from Pool.query() method #991

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b44221d
Enable returning formatted sql in Pool.query()
surfbuds Feb 11, 2015
f341946
Enable returning formatted query in Pool.query()
surfbuds Feb 11, 2015
249b0a3
Logical statement
surfbuds Feb 12, 2015
e350fad
Merge remote-tracking branch 'upstream/master'
surfbuds Feb 12, 2015
6bf1987
Unit test for fix to issue #984
surfbuds Feb 16, 2015
119d6ec
Merge remote-tracking branch 'upstream/master'
surfbuds Feb 16, 2015
3cd3833
Moved isFormatted property
surfbuds Feb 16, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions lib/Connection.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,10 @@ Connection.prototype.query = function(sql, values, cb) {
if (!(typeof sql == 'object' && 'typeCast' in sql)) {
query.typeCast = this.config.typeCast;
}

query.sql = this.format(query.sql, query.values);
if (!query.isFormatted) {
query.sql = this.format(query.sql, query.values);
query.isFormatted = true;
}

return this._protocol._enqueue(query);
};
Expand Down
13 changes: 13 additions & 0 deletions lib/Pool.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,8 @@ Pool.prototype.end = function (cb) {
Pool.prototype.query = function (sql, values, cb) {
var query = Connection.createQuery(sql, values, cb);

query.isFormatted = false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this so it always exists, otherwise we are altering the hidden class and fracturing it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i.e. it should be set within createQuery


if (!(typeof sql === 'object' && 'typeCast' in sql)) {
query.typeCast = this.config.connectionConfig.typeCast;
}
Expand All @@ -195,6 +197,10 @@ Pool.prototype.query = function (sql, values, cb) {
conn.query(query);
});

if (!query.isFormatted) {
query.isFormatted = true;
query.sql = this.format(query.sql, query.values);
}
return query;
};

Expand Down Expand Up @@ -255,6 +261,13 @@ Pool.prototype.escapeId = function escapeId(value) {
return mysql.escapeId(value, false);
};

Pool.prototype.format = function(sql, values) {
if (typeof this.config.connectionConfig.queryFormat == "function") {
return this.config.connectionConfig.queryFormat.call(this, sql, values, this.config.connectionConfig.timezone);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to think about if it's acceptable that the context of the queryFormat call is not going to be a Connection object in these cases and if this is going to be an issue.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did find a solution to keep createQuery as a static method of Connection and provide the right context for instance config, which keeps things backward compatible. I'll commit the changes later from another machine.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it take care of this context issue, though?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise there is no need to add a new commit, really. I've already adjusted your commit to make it an instance method (still under your name). The problem right now is backwards compatibility with this function context.

}
return mysql.format(sql, values, this.config.connectionConfig.stringifyObjects, this.config.connectionConfig.timezone);
};

function spliceConnection(array, connection) {
var index;
if ((index = array.indexOf(connection)) !== -1) {
Expand Down