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

Fix debug logger messing up queries with % #3566

Merged
merged 1 commit into from Dec 7, 2019
Merged

Fix debug logger messing up queries with % #3566

merged 1 commit into from Dec 7, 2019

Conversation

@lorefnon
Copy link
Collaborator

lorefnon commented Dec 7, 2019

Resolves #3564

const debugBindings = require('debug')('knex:bindings');

const debugQuery = (sql, txId) => _debugQuery(sql.replace(/%/, '%%'), txId)

This comment has been minimized.

Copy link
@kibertoad

kibertoad Dec 7, 2019

Collaborator

wouldn't that mess up queries that had %% in them already? or that is not a realistic case? maybe we can limit regexp to just single instances of %?

This comment has been minimized.

Copy link
@lorefnon

lorefnon Dec 7, 2019

Author Collaborator

I intended this to be /%/g.

Although I cant imagine a situation where we would have multiple consecutive % in an sql query, if we do, I think we would always want them to be reported as many as there are.

> debug('x')('%foo%')
  x NaNoo%

> debug('x')('%%foo%%')
  x %foo% +0ms

> debug('x')('%foo%'.replace(/%/g, '%%'))
  x %foo% +0ms

> debug('x')('%%foo%%'.replace(/%/g, '%%'))
  x %%foo%% +0ms

This comment has been minimized.

Copy link
@kibertoad

kibertoad Dec 7, 2019

Collaborator

fair :)

Resolves #3564
@lorefnon lorefnon force-pushed the #3564 branch from fe2f075 to d3eb373 Dec 7, 2019
@kibertoad kibertoad merged commit 65e1a3a into master Dec 7, 2019
0 of 2 checks passed
0 of 2 checks passed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/travis-ci/push The Travis CI build failed
Details
@kibertoad kibertoad deleted the #3564 branch Dec 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.