Naive Datetime values comparison using < and > operators

Is it not a right method to compare naive date time values using basic operators like > and < ?

I get wrong answers when I compare two date time values using these operators.

iex(1)> datetime1 = ~N[2019-09-26 01:00:00.000000]

iex(2)> datetime2 = ~N[2020-01-24 00:00:00.000000]

iex(3)> datetime1 > datetime2 
true

What is going wrong here?

</2 and >/2 work as expected and documented here.

They do a structural comparison. If you want to do a value comparison of those, please use NaiveDateTime.compare/2

3 Likes

Can you please tell me how does NaiveDateTime.compare/2 and structural comparison work?

From the erlang documentation:

Maps are ordered by size, two maps with the same size are compared by keys in ascending term order and then by values in key order. In maps key order integers types are considered less than floats types.

1 Like

This means in structural comparison the :day key has precedence over the :month key. So for any value in :month the higher :day value will sort last.

1 Like

ok, that helps :slight_smile:

So, is the Naive DateTime basically a map?

Yes, as any other struct.

You can take a peek at the internal structure by using IO.inspect/2 and it’s option structs:

iex(7)> NaiveDateTime.utc_now() |> IO.inspect(structs: false)
%{
  __struct__: NaiveDateTime,
  calendar: Calendar.ISO,
  day: 17,
  hour: 12,
  microsecond: {614456, 6},
  minute: 37,
  month: 9,
  second: 47,
  year: 2019
}

That’s how all structs work in Elixir.

2 Likes

Thank you :blush:

1 Like

I know this is an old topic and I understand that using < does structural comparison.

But I just discovered a bad bug in my application that was super nasty to track down.
I had first_entry = entries |> Enum.min_by(& &1.inserted_at) somewhere in critical business logic.
Obviously, this code is wrong, it should be:

first_entry = entries |> Enum.min_by(& &1.inserted_at, NaiveDateTime)

Having found this bug, I’m now paranoid that this incorrect use of < is hiding in potentially many other places.

My tests didn’t catch this, since they used accidentally correctly sorting dates.

Is there a way to find these sort of issues?
Would the new type system be able to catch this?
Could credo catch this?

1 Like

Structural comparison of dates was actually used as an example of the new type system when it was introduced! At the moment I don’t believe it can catch this within a function like Enum.min_by() (obviously since it didn’t for you), but there are more type system changes coming in 1.19 and 1.20 which have to do with functions, so perhaps it will eventually?

2 Likes

Honestly? I think this is just one of those things you need to be paranoid about.

Comparing structs falls into an unfortunate local minima of Elixir’s design. There is a total ordering on its terms, so a < b is always valid. So the compiler is a bit limited in what it can do. Though as @garrison noted, you do get warnings in certain circumstances.

However, unless the compiler knows for some reason that the result of & &1.inserted_at will always be a %NaiveDateTime{}, there’s nothing for it to do. You could maybe do:

Enum.min_by(entries, fn %{inserted_at: %NaiveDateTime{} = ndt} -> ndt end)

But at that point, you’re bending over backwards to get the compiler to tell you something you clearly already know. Though maybe if you use TypedEctoSchema, the compiler would be able to infer.

One other thought: always consider adding an order_by clause to your queries. Similar to this particular gotcha, it’s very easy to unknowingly rely on the order a query even though you didn’t specify it. E.g. we had a series of heisenbugs in our tests where I was blithely asserting on the result of Repo.all(). The results rows usually ordered the same, but not always.

1 Like

Oh that’s great, I must have missed that.
I’ll need to do some experiments to see under which circumstances it can catch these things.
I’m already on 1.19.0-rc.2, so maybe if add some pattern matches on the Repo.all() results it might catch this.

It looks like TypedEctoStruct helps to create dializer types, but I think those are ignored in the new type system, so this won’t help.
Also, I think the source of the problem is that it can’t know the type of Repo.all.
But I guess in theory it would be possible to infer the type from the query, so maybe the compiler can do it in the future.

Did anyone craft some macro-helper that lets use “>” / “<“ etc in a more direct way? “compare” is not intuitive to read :frowning:

Yes: CompareChain — compare_chain v0.6.0

The OP would be:

import CompareChain

datetime1 = ~N[2019-09-26 01:00:00.000000]
datetime2 = ~N[2020-01-24 00:00:00.000000]
compare?(datetime1 > datetime2, NaiveDateTime) #=> false
2 Likes