Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Empty datetime will fail when returning results #3169

Merged
merged 9 commits into from
Apr 11, 2019
Merged

Empty datetime will fail when returning results #3169

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
798d598
types/conversion.go: dont convert datetime to datetime when empty
Mar 18, 2019
ecf699a
query/outputnode.go: return empty string when result is zero-time value
Mar 18, 2019
060a622
types/conversion_test.go: add tests for empty values
Mar 18, 2019
32b0eaa
types/conversion.go: expand the comment so it is clear
Mar 20, 2019
7193c84
types/conversion.go: add specific conversion checks for datetime conv…
Mar 26, 2019
b5bd724
types/conversion_test.go: add test for conversion edge cases
Mar 26, 2019
ca6a5e0
types/conversion.go: add sanity check for data type assertion
Mar 26, 2019
ff91e4e
types/conversion.go: remove any default value conversions and return …
Mar 28, 2019
d6fa2c7
types/conversion_test.go: remove extra value in test, fix 100 char le…
Apr 11, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion query/outputnode.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,12 @@ func valToBytes(v types.Val) ([]byte, error) {
}
return []byte("false"), nil
case types.DateTimeID:
return v.Value.(time.Time).MarshalJSON()
// Return empty string instead of zero-time value string - issue#3166
t := v.Value.(time.Time)
if t.IsZero() {
return []byte(`""`), nil
}
return t.MarshalJSON()
case types.GeoID:
return geojson.Marshal(v.Value.(geom.T))
case types.UidID:
Expand Down
7 changes: 5 additions & 2 deletions types/conversion.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,11 @@ func Convert(from Val, toID TypeID) (Val, error) {
case DateTimeID:
{
var t time.Time
if err := t.UnmarshalBinary(data); err != nil {
return to, err
// We must inverse the binary->datetime for zero-time values.
if !bytes.Equal(data, []byte("")) {
if err := t.UnmarshalBinary(data); err != nil {
return to, err
}
}
// NOTE: when converting datetime values to anything else, we must
// check for zero-time value and return the zero value of the new type.
Expand Down
30 changes: 30 additions & 0 deletions types/conversion_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,36 @@ func TestSameConversionDateTime(t *testing.T) {
}
}

func TestConversionEmpty(t *testing.T) {
tests := []struct {
in, out Val
}{
{in: Val{Tid: BinaryID, Value: []byte{}},
out: Val{Tid: DateTimeID, Value: time.Time{}}},
{in: Val{Tid: BinaryID, Value: bs("")},
out: Val{Tid: DateTimeID, Value: time.Time{}}},
{in: Val{Tid: DateTimeID, Value: bs(time.Time{})},
out: Val{Tid: BinaryID, Value: []byte{}}},
{in: Val{Tid: StringID, Value: bs("")},
out: Val{Tid: DateTimeID, Value: time.Time{}}},
{in: Val{Tid: DateTimeID, Value: bs(time.Time{})},
out: Val{Tid: StringID, Value: ""}},
{in: Val{Tid: DefaultID, Value: bs("")},
out: Val{Tid: DateTimeID, Value: time.Time{}}},
{in: Val{Tid: DateTimeID, Value: bs(time.Time{})},
out: Val{Tid: DefaultID, Value: ""}},
{in: Val{Tid: DateTimeID, Value: bs("")},
out: Val{Tid: DateTimeID, Value: time.Time{}}},
{in: Val{Tid: DateTimeID, Value: []byte{}},
out: Val{Tid: DateTimeID, Value: time.Time{}}},
}
for _, tc := range tests {
out, err := Convert(tc.in, tc.out.Tid)
require.NoError(t, err)
require.EqualValues(t, tc.out, out)
}
}

func TestConvertToDefault(t *testing.T) {
tests := []struct {
in Val
Expand Down