fix increment/decrement method float bug

[olalonde]

The increment/decrement method would convert float numbers
to integers instead of leaving them as is.

Fixes #868

[elhigu]

Implementation looks good, but it seems to be missing test case for it. test/integration/builder/updates.js could be good place to put the test.

[olalonde]

Sorry can't manage to run integration tests. Maybe merge this and open another issue?

[avimar]

I was pulling my hair out for a bit trying to figure out why decrement didn't work, so I dug into the source code and realized it did parseInt...
So I had to do this silly workaround:
.update('mycol', knex.raw('mycol-'+ variable))

[rhys-vdw]

@olalonde Just write the test and push to the PR, Travis will run it for you. If it fails you can just fix it and push again. (Or set up the test environment - a bit more involved.)

Also I agree with @chrisbroome that parseFloat would be preferable to new Number.

[rhys-vdw]

Thinking on this more - I'm actually not in favor of this change. These methods are meant to be for use with 'counter' style columns. The default of 1 for the increment/decrement size makes sense only in the context of integers.

When dealing with floating point numbers it would make more sense to have methods add and subtract that will be no-ops if passed null. But then if you're going to add add and subtract why not add multiply and divide? And what if you want to use a different column (eg. colA = colB + 5)? The permutations grow near endlessly. Currently this is exactly what knex.raw is for:

knex('table').update({ column: knex.raw('?? + ?', column, 1.5) }).then(result =>

If you want to support writing arbitrary statements like this without using raw I think we'd need something like this:

const { column, value } = knex.statement;
knex('table').update({
  thing: column('thing').plus(1.5), // "thing" = "thing" + 1.5
  other: column('other').times(column('thing')), // "other" = "other" * "thing"
  thingy: value(3).plus(1.5) // "thingy" = 3 + 1.5
).then(result =>

[olalonde]

That's a lot of bike-shedding over a removed parseInt call. People who want to use increment/decrement with integers still can. People who want to increment/decrement floats will now be able to. Simple as that. Can we just merge this and open another issue for the unrelated stuff?

[rhys-vdw]

People who want to increment/decrement floats will now be able to.

Fair enough @olalonde.

I was waiting for the change to parseFloat, which you've just added. There are some conflicts so it'll have to rebased and merge manually. I've gotta run now but I'll set a reminder to do it (if nobody beats me to it).

[olalonde]

Thanks.

[rhys-vdw]

Hey @olalonde, did you end up writing a test for this?

[elhigu]

Closing abandoned pull request, I'll update the issue mentioning that here is almost ready pull request if someone feels like finishing this.