Fixes #1445 - Date +/- null resolves null

[garrett-is-a-swann]

Fixes #1445 Date and null arithmetic operations ending in error.

This can occur when using the dur() function on a possibly-undefined key to offset a date, and therefore should not cause errors in parsing.

Example test:

- [ ] %% ➕2022-09-22 #todo [[todos]] %%(context:: [[reminder]]) [in:: 1 days] Check this off
- [ ] %% ➕2022-09-23 #todo [[todos]] %%(context:: [[reminder]]) 📆2022-09-23 Check this off

\```dataview
TASK FROM #todo 
WHERE context = [[reminder]]
    and (!completed or completion = date(today))
    and (
        (due & date(today) >= due)
     or (in & date(today) >= date(created) + dur(in))
    )
\```

After change:

[AB1908]

I wonder if it's possible to add a test for this.

[garrett-is-a-swann]

Are there similar tests you'd like me to add to the PR that you can point me to? I'm not immediately sure of how you would test a logical statement/declaration like this, but I'm happy to add one if we feel it makes sense.

[blacksmithgu]

You can add tests for evaluations in the src/tests/expression folder (I believe), just keying off of other tests that test behavior like addition and so on.

My main concern here is that I believe null + <thing> has not been something that I've supported for any data type, and adding support for just dates is an unusual special case (and also, having it choose null instead of choosing a valid date is curious - why not have date + null = date?)

[blacksmithgu]

I'm okay with broadly adding defined behavior for null + thing (either null + thing = null or null + thing = thing) but it may be worth thinking out the use cases a bit to see which one makes more intuitive sense or leads to fewer bugs.

[blacksmithgu]

I think intuitively I would expect null + date = date; would that work in the case that your original issue reported?

[garrett-is-a-swann]

I think generally, we think an operation of a data type vs null would generally result in null. I can't think of an example where this isn't the case, so I think it'd make sense to allow for this. I think this statement is logical: null being in an operation with a date is an undefined operation -- I think we can all agree. This PR is just defining that, for the parser, instead of erroring we can handle this gracefully and pass null through. That way, the actually expressions can be allowed to resolve themselves.

This proposal is the cleanest and imo the most correct that I can immediately think of. This also tends to mimick SQL where any operation on a data type with null ends in a null value.

[garrett-is-a-swann]

I think null + date resulting in null is unintuitive -- I can't think of any language where I'd expect this to be the case. Generally, I think we expect null to consume anything in an operation and to receive null, since those operations are undefined.

That being said, I actually personally don't have a hard line here. I primarily want to get this kind of query past the parser stage, and either decision -- passing null or date through, allows for that.

[garrett-is-a-swann]

I wanted to substantiate my claim of "other SQL engines resolve null-date arithmetic to null". Here are some queries you can try -- if you need a playground, the best I found after a quick search was http://sqlfiddle.com/.

Sqlite:

select date(), date() + 1, date() + null, time('now'), time('now', '+2 hours'), time('now', null), time('now', '+null hours');

date() | date() + 1 | date() + null | time('now') | time('now', '+2 hours') | time('now', null) | time('now', '+null hours')
-- | -- | -- | -- | -- | -- | --
2022-09-24 | 2023 | (null) | 05:36:18 | 07:36:18 | (null) | (null)

MySQL 5.6

select  now(), now() + 1, now() + null, 1 days, null days, now() + null days;

now() | now() + 1 | now() + null | days | days | days
-- | -- | -- | -- | -- | --
2022-09-24T05:56:14Z | 20220924055615 | (null) | 1 | (null) | (null)

postgresql 9.6

select now(), /*now() + 1,*/ now() + null, 1 days, '1 days', '1 days'::interval, null days, 'null days', /*'null days'::interval,*/ now() + '1 day', now() + null days/*, now() + 'null days'*//*, now() + 'null days'::interval*/;

now | ?column? | days | ?column? | interval | days | ?column? | ?column? | days
-- | -- | -- | -- | -- | -- | -- | -- | --
2022-09-24T05:46:46.003845Z | (null) | 1 | 1 days | 0 years 0 mons 1 days 0 hours 0 mins 0.00 secs | (null) | null days | 2022-09-25T05:46:46.003845Z | (null)

The commented out portions in the psql examples are due to syntax errors, but I kept them in, since they're related to operations that did work.

[garrett-is-a-swann]

I added a few tests for this behavior.

Something I noticed while looking at the code -- the dur() function seems like it's going to allow for a lot more to be passed to it than maybe one would want. Not necessarily a huge issue, since this is the user's fault if they decide to pass silly things like lists and junk to it and end with a runtime error. Just thought it could be worth noting in case no one's been in there recently.

By the way, I really enjoy how everything here is laid out.

[blacksmithgu]

I think I may holistically revisit the whole concept of null + <thing> in the future to see if there are better ergonomics, but I take your points on this mimic-ing existing SQL behavior and that null is also not unreasonable as an output.

Merged; I will release this shortly.

[blacksmithgu]

Released in 0.5.47.