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

knex.increment/decrement bug #100

Closed
mclueppers opened this issue Nov 6, 2013 · 3 comments
Closed

knex.increment/decrement bug #100

mclueppers opened this issue Nov 6, 2013 · 3 comments
Labels

Comments

@mclueppers
Copy link

@mclueppers mclueppers commented Nov 6, 2013

Hi,

I'm using Knex 0.4.13 from npm install. There is a bug in lib/builder.js:504, function _counter. knex.update() accepts an object, BUT Javascript is not replacing variable values when declaring them as a key inside an object.

For example:

var column = 'some_key';
var test = {column: column}
console.log(test);

will output {column: some_key} and not {some_key: some_key}.

Following patch fixes it:

-      return this.update({column: this.knex.raw('' + this.grammar.wrap(column) + ' ' + (symbol || '+') + ' ' + amount)}); 
+      var toUpdate = {};
+      toUpdate[column] = this.knex.raw('' + this.grammar.wrap(column) + ' ' + (symbol || '+') + ' ' + amount);
+      return this.update(toUpdate);

I hope this helps others that rely on increment, decrement operations too.

Kind regards,
Martin

@tgriesser
Copy link
Member

@tgriesser tgriesser commented Nov 6, 2013

Thanks @mclueppers - I'm traveling right now but I'll try to get to this one soon (simple fix but I need to sort out some WIP stuff that I merged onto master but isn't quite ready yet).

@mclueppers
Copy link
Author

@mclueppers mclueppers commented Nov 7, 2013

@tgriesser
Bug#100 is the most easiest one ;) I hope the amount of bugs stays low

@hccampos
Copy link

@hccampos hccampos commented Nov 19, 2013

+1

@tgriesser tgriesser closed this in 64f9deb Nov 20, 2013
elliotf pushed a commit to elliotf/knex that referenced this issue Nov 24, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants