Open kelvintaywl opened 1 year ago
It is the result of a modulo calculation on the floating-point numbers, since the result of the CEIL function is a float. However, it may be a bad idea to apply the % operator to float.
As a workaround, you can avoid that result by explicitly converting the ceil result to integer.
$ csvq "SELECT INTEGER(CEIL(13041/60))%60"
@mithrandie I don't get it, even for floats a
and b
the result a % b
should be in the range [0,|b|)
, right?
If you use math.Mod behind the scenes, the result could be negative if a
is negative. But:
CEIL(13041 / 60)
is not negative;math.Mod
is not "standard". If you decide to stick with it, you should probably document it. Otherwise, you can correct the result of a/b
by simply adding |b|
if it is negative.After verification, I'm wrong about "standard". There is no such thing about the sign of modulo. In some languages (like go and javascript) the sign of a/b
is the same as a
, and in other languages (like python) it is the same as b
. And in math it is often fixed as positive.
But in any case, if a
and b
are positive (like in CEIL(13041 / 60) % 60
), the modulo must be positive.
And in any case, it will be good to document the behavior of %
with respect to the sign.
@mithrandie You use math.Remainder instead of math.Mod
. This is why you get a negative remainder from two positive numbers. Is this intentional? Isn't it better to use math.Mod
instead?
Ah, I didn't know there was the math.Mod function... That would be more appropriate for the % operator. Thank you.
hi @mithrandie and @kpym
Thanks so much for the insights! I am nowhere familiar with the implementation nor Go, so your details here helped me understand better.
For myself, i am not blocked since we are using it for analysis internally; no huge repercussions with the negative sign.
I will give the suggestion here a try too.
I'll leave this open since it'd be nice to close this if a patch is on the way 🤓 thanks so much, again! 🙇
hi @mithrandie I wanted to share that I was able to fix/work-around this with casting the ceil result with INTEGER. Thank you so much!
I've leave it to you, if you'd like to keep this open, given https://github.com/mithrandie/csvq/issues/109#issuecomment-1577760706
Hi!
firstly, thank you so much for this tool. It's been amazingly helpful for me 🙇 .
I wanted to file an issue about seeing negative values in the modulo (
%
) result.To reproduce:
I was expecting 37 there since
CEIL(13041/60) = 217
and217 % 60 = 37
, not-23
.I am thinking this is a possible bug 🤔