sql: interval division with float rounds differently from Python & PostgreSQL

[timgraham]

see #66118 (comment) for solution

---

Describe the problem

Interval division with a float rounds differently from Python & PostgreSQL.

To Reproduce

CREATE TABLE "test1" (
    "id" serial NOT NULL PRIMARY KEY,
    "duration" interval NOT NULL
);

INSERT INTO "test1" ("duration") VALUES ('0 days 0.253000 seconds'::interval);

SELECT ("duration" / 3.2) FROM "test1";
     ?column?
-------------------
  00:00:00.079063

Expected behavior

Expected result of SELECT query: 00:00:00.079062.

Environment:

• CockroachDB version: 21.1.1 (and older versions)

Additional context

A Django test fails:

======================================================================
FAIL: test_durationfield_multiply_divide (expressions.tests.FTimeDeltaTests) (expr=3.2)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/tim/code/django/tests/expressions/tests.py", line 1565, in test_durationfield_multiply_divide
    self.assertEqual(
AssertionError: datetime.timedelta(microseconds=79063) != datetime.timedelta(microseconds=79062)

[blathers-crl]

Hello, I am Blathers. I am here to help you get the issue triaged.

Hoot - a bug! Though bugs are the bane of my existence, rest assured the wretched thing will get the best of care here.

I have CC'd a few people who may be able to assist you:

• @rafiss (found keywords: Django)

If we have not gotten back to your issue within a few business days, you can try the following:

• Join our community slack channel and ask on #cockroachdb.
• Try find someone from here if you know they worked closely on the area and CC them.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.}

[kernfeld-cockroach]

I made a minimal reproduction of this discrepancy. Looks like Cockroach is somehow keeping track of more precision than Postgres, because Cockroach decides to round (0.000001 / 2) up to 0.000001, whereas Postgres treats it more like integer division, rounding (0.000001 / 2) down to 0.

Postgres:

postgres=# SELECT (INTERVAL '0.000001' / 2);
 ?column? 
----------
 00:00:00
(1 row)

Cockroach:

root@:26257/defaultdb> SELECT (INTERVAL '0.000001' / 2);
     ?column?
-------------------
  00:00:00.000001
(1 row)

Time: 0ms total (execution 0ms / network 0ms)

[rafiss]

Thanks @kernfeld-cockroach! cc @otan does anything come to mind immediately from when you last looked at INTERVAL?

[otan]

ah yes, it's because we use Round here:

cockroach/pkg/util/duration/duration.go

Line 251 in 3f04bd5

v := dur.Round(time.Microsecond).Nanoseconds()

[otan]

on closer observation, the round down behaviour only exists for division.

• as such, we need to change

  cockroach/pkg/util/duration/duration.go

  Line 619 in 16e72f3

  func (d Duration) DivFloat(x float64) Duration {

  to round down nanos before rounded is called.
• then we should change the test cases here:
  

  cockroach/pkg/util/duration/duration_test.go

  Lines 461 to 501 in 16e72f3

  	func TestFloatMath(t *testing.T) {
  	const nanosInMinute = nanosInSecond * 60
  	const nanosInHour = nanosInMinute * 60
  	tests := []struct {
  	d Duration
  	f float64
  	mul Duration
  	div Duration
  	}{
  	{
  	Duration{Months: 1, Days: 2, nanos: nanosInHour * 2},
  	0.15,
  	Duration{Days: 4, nanos: nanosInHour*19 + nanosInMinute*30},
  	Duration{Months: 6, Days: 33, nanos: nanosInHour*21 + nanosInMinute*20},
  	},
  	{
  	Duration{Months: 1, Days: 2, nanos: nanosInHour * 2},
  	0.3,
  	Duration{Days: 9, nanos: nanosInHour * 15},
  	Duration{Months: 3, Days: 16, nanos: nanosInHour*22 + nanosInMinute*40},
  	},
  	{
  	Duration{Months: 1, Days: 2, nanos: nanosInHour * 2},
  	0.5,
  	Duration{Days: 16, nanos: nanosInHour * 1},
  	Duration{Months: 2, Days: 4, nanos: nanosInHour * 4},
  	},
  	{
  	Duration{Months: 1, Days: 2, nanos: nanosInHour * 2},
  	0.8,
  	Duration{Days: 25, nanos: nanosInHour * 16},
  	Duration{Months: 1, Days: 10, nanos: nanosInHour*2 + nanosInMinute*30},
  	},
  	{
  	Duration{Months: 1, Days: 17, nanos: nanosInHour * 2},
  	2.0,
  	Duration{Months: 2, Days: 34, nanos: nanosInHour * 4},
  	Duration{Days: 23, nanos: nanosInHour * 13},
  	},
  	}

  to include these test values.
• Then we should add a test in

  cockroach/pkg/sql/logictest/testdata/logic_test/interval

  Line 1 in e73c15c

  # test we store various types with precision correctly.

  and run it with make testbaselogic FILES='interval' TESTFLAGS='-rewrite' to ensure output is appropriate

[TszKitLo40]

@otan I am new here. Can I try this issue? I think maybe I can try this issue by following the instructions listed by you.

[otan]

Go for it!

[TszKitLo40]

@otan I have ran the command make testbaselogic FILES='interval' TESTFLAGS='-rewrite' you mentioned, but the following error is returned. How can I fix it?
E210610 12:24:31.728365 1841 sql/logictest/logic.go:3579 [-] 1 E210610 12:24:31.728365 1841 sql/logictest/logic.go:3579 [-] 1 +testdata/logic_test/interval:378: E210610 12:24:31.728365 1841 sql/logictest/logic.go:3579 [-] 1 +expected success, but found E210610 12:24:31.728365 1841 sql/logictest/logic.go:3579 [-] 1 +(42P01) relation "test1" does not exist E210610 12:24:31.728365 1841 sql/logictest/logic.go:3579 [-] 1 +errors.go:114: in NewUndefinedRelationError() E210610 12:24:31.728434 1841 sql/logictest/logic.go:3566 [-] 2 E210610 12:24:31.728434 1841 sql/logictest/logic.go:3566 [-] 2 +pq: relation "test1" does not exist

[otan]

this means the table "test1" does not exist. did you create something that references test1? what code did you add to the file?

[TszKitLo40]

I have fixed the problem I mentioned. I have also filed a PR for this issue. @otan PTAL

[kernfeld-cockroach]

@TszKitLo40 thanks for the quick fix!!