master: Alpha stops running when querying for invalid datetime value.

[danielmai]

What version of Dgraph are you using?

master.

$ dgraph version

Dgraph version   : v1.0.12-rc3-225-g207e1349b
Commit SHA-1     : 207e1349b
Commit timestamp : 2019-03-15 16:22:58 -0700
Branch           : master
Go version       : go1.11.5

Have you tried reproducing the issue with latest release?

This does not happen in v1.0.

Steps to reproduce the issue (command/config used to run Dgraph).

• Start a Dgraph cluster
• Run schema update to set predicate datetime type:

curl localhost:8080/alter -d 'pred.date: datetime .'

• Commit a mutation with an invalid datetime value (empty string). This mutation is successful.

curl -H "X-Dgraph-CommitNow: true" localhost:8080/mutate -d '{
  set {
    _:n <pred.date> "" .
  }
}'

• Run a query for that predicate.

curl localhost:8080/query -d '{
  q(func: has(pred.date)) {
    uid
    pred.date
  }
}'

Alpha shows this error log and stops running:

2019/03/18 18:07:30 Error while interpreting appropriate type from binary error: Time.UnmarshalBinary: no data

The query never succeeds since the Alpha stopped after receiving the query.

Expected behaviour and actual result.

Dgraph shouldn't accept invalid data. In v1.0.13 (latest), the mutation does not succeed since "" is an invalid datetime format.

curl -H "X-Dgraph-CommitNow: true" http://localhost:8180/mutate -d '
{
  set {
    _:n <pred.date> "" .
  }
}'
# {
#   "errors": [
#     {
#       "code": "ErrorInvalidRequest",
#       "message": "parsing time \"\" as \"2006\": cannot parse \"\" as \"2006\""
#     }
#   ],
#   "data": null
# }

[srfrog]

We accept empty string value, which is converted to zero time value datetime. For some reason this empty string was not converted, that's the bug.

[srfrog]

We are saving the zero-time value, but we then marshal to binary which converts it to "". That causes the query to convert "" which is what triggers the error. In reality a nil datetime should be a zero-time value.

[danielmai]

In v1.0.13 empty values weren't accepted as a mutation. It was an error. Are we changing the behavior so that the mutation is accepted and the data is converted internally?

[srfrog]

Yes, empty strings will be converted internally to zero-time value. So it looks like we were missing a case for this.

[manishrjain]

Empty string should be rejected for a datetime, shouldn't it? It is not a valid datetime.

[srfrog]

That was the old behavior that we changed to fix an issue with exports. We allow empty string as datetime and internally convert them to zero-times. So empty strings are now handled transparently with all types.

[manishrjain]

I still think that empty string as a datetime should have been rejected during mutation. Why does an export (read path) cause an issue with a mutation (write path)?

[srfrog]

@manishrjain As you and I discussed just now, I'm going to remove the code that handles default conversions. That should trigger an error if a mutation is tried with unsupported values. So the empty string mutation on datetime will trigger an error instead of Dgraph try to handle it.