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

Comments

Projects
None yet
3 participants
@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

This comment has been minimized.

Owner

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

This comment has been minimized.

mclueppers commented Nov 7, 2013

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

@hccampos

This comment has been minimized.

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