Skip to content

Commit

Permalink
Empty datetime will fail when returning results (hypermodeinc#3169)
Browse files Browse the repository at this point in the history
* types/conversion.go: dont convert datetime to datetime when empty

If the stored value of datetime predicate is empty, types.Marshal will
return empty when converting to binary. When we return the result the empty
binary is converted to datetime, this causes time.UnmarshalBinary() to
fail.

* query/outputnode.go: return empty string when result is zero-time value

The zero-time value is an internal values that should be exposed to the client.

* types/conversion_test.go: add tests for empty values

Added tests to preserve the behavior with empty values.

* types/conversion.go: expand the comment so it is clear

* types/conversion.go: add specific conversion checks for datetime conversions

Conversion from binary to datetime should happen for value length greater than zero.
Convertion from empty string to int or float results their zero value.
Conversion from datetime binary value (stored) will unmarshal only if the length is correct.
Fixed a panic when bool is invalid, which obscures the real reason.

* types/conversion_test.go: add test for conversion edge cases

The edge test cases are for unexpected values that must be handled nicely.
Updated float and int tests when converting from empty string value.

* types/conversion.go: add sanity check for data type assertion

* types/conversion.go: remove any default value conversions and return errors instead

Remove all handling of default values for types that can return error when syntax fails.
Update the tests to check for behavior.

* types/conversion_test.go: remove extra value in test, fix 100 char length.
  • Loading branch information
srfrog authored and dna2github committed Jul 19, 2019
1 parent 6184249 commit 6516b1d
Show file tree
Hide file tree
Showing 3 changed files with 170 additions and 99 deletions.
7 changes: 6 additions & 1 deletion query/outputnode.go
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
121 changes: 45 additions & 76 deletions types/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,17 @@ import (

// Convert converts the value to given scalar type.
func Convert(from Val, toID TypeID) (Val, error) {
to := ValueForType(toID)
var to Val

// sanity: we expect a value
data, ok := from.Value.([]byte)
if !ok {
return to, x.Errorf("Invalid data to convert to %s", toID.Name())
}
to = ValueForType(toID)
fromID := from.Tid
data := from.Value.([]byte)
res := &to.Value

// Convert from-type to to-type and store in the result interface.
switch fromID {
case BinaryID:
Expand Down Expand Up @@ -72,10 +79,8 @@ func Convert(from Val, toID TypeID) (Val, error) {
return to, x.Errorf("Invalid value for bool %v", data[0])
case DateTimeID:
var t time.Time
if !bytes.Equal(data, []byte("")) {
if err := t.UnmarshalBinary(data); err != nil {
return to, err
}
if err := t.UnmarshalBinary(data); err != nil {
return to, err
}
*res = t
case GeoID:
Expand Down Expand Up @@ -114,23 +119,17 @@ func Convert(from Val, toID TypeID) (Val, error) {
case StringID, DefaultID:
*res = vc
case BoolID:
*res = false
if vc != "" {
val, err := strconv.ParseBool(vc)
if err != nil {
return to, err
}
*res = val
val, err := strconv.ParseBool(vc)
if err != nil {
return to, err
}
*res = val
case DateTimeID:
*res = time.Time{}
if vc != "" {
t, err := ParseTime(vc)
if err != nil {
return to, err
}
*res = t
t, err := ParseTime(vc)
if err != nil {
return to, err
}
*res = t
case GeoID:
var g geom.T
text := bytes.Replace([]byte(vc), []byte("'"), []byte("\""), -1)
Expand Down Expand Up @@ -169,10 +168,7 @@ func Convert(from Val, toID TypeID) (Val, error) {
case StringID, DefaultID:
*res = strconv.FormatInt(vc, 10)
case DateTimeID:
*res = time.Time{}
if vc != 0 {
*res = time.Unix(vc, 0).UTC()
}
*res = time.Unix(vc, 0).UTC()
default:
return to, cantConvert(fromID, toID)
}
Expand Down Expand Up @@ -202,28 +198,21 @@ func Convert(from Val, toID TypeID) (Val, error) {
case StringID, DefaultID:
*res = strconv.FormatFloat(vc, 'G', -1, 64)
case DateTimeID:
*res = time.Time{}
if vc != 0 {
secs := int64(vc)
fracSecs := vc - float64(secs)
nsecs := int64(fracSecs * nanoSecondsInSec)
*res = time.Unix(secs, nsecs).UTC()
}
secs := int64(vc)
fracSecs := vc - float64(secs)
nsecs := int64(fracSecs * nanoSecondsInSec)
*res = time.Unix(secs, nsecs).UTC()
default:
return to, cantConvert(fromID, toID)
}
}
case BoolID:
{
var vc bool
switch {
case data[0] == 0:
vc = false
case data[0] == 1:
vc = true
default:
return to, x.Errorf("Invalid value for bool %v", data[0])
if len(data) == 0 || data[0] > 1 {
return to, x.Errorf("Invalid value for bool %v", data)
}
vc = data[0] == 1

switch toID {
case BoolID:
Expand Down Expand Up @@ -255,39 +244,25 @@ func Convert(from Val, toID TypeID) (Val, error) {
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.
switch toID {
case DateTimeID:
*res = t
case BinaryID:
*res = []byte("")
if !t.IsZero() {
r, err := t.MarshalBinary()
if err != nil {
return to, err
}
*res = r
r, err := t.MarshalBinary()
if err != nil {
return to, err
}
*res = r
case StringID, DefaultID:
*res = ""
if !t.IsZero() {
val, err := t.MarshalText()
if err != nil {
return to, err
}
*res = string(val)
val, err := t.MarshalText()
if err != nil {
return to, err
}
*res = string(val)
case IntID:
*res = int64(0)
if !t.IsZero() {
*res = t.Unix()
}
*res = t.Unix()
case FloatID:
*res = float64(0)
if !t.IsZero() {
*res = float64(t.UnixNano()) / float64(nanoSecondsInSec)
}
*res = float64(t.UnixNano()) / float64(nanoSecondsInSec)
default:
return to, cantConvert(fromID, toID)
}
Expand Down Expand Up @@ -415,23 +390,17 @@ func Marshal(from Val, to *Val) error {
vc := val.(time.Time)
switch toID {
case StringID, DefaultID:
*res = ""
if !vc.IsZero() {
val, err := vc.MarshalText()
if err != nil {
return err
}
*res = string(val)
val, err := vc.MarshalText()
if err != nil {
return err
}
*res = string(val)
case BinaryID:
*res = []byte("")
if !vc.IsZero() {
r, err := vc.MarshalBinary()
if err != nil {
return err
}
*res = r
r, err := vc.MarshalBinary()
if err != nil {
return err
}
*res = r
default:
return cantConvert(fromID, toID)
}
Expand Down
Loading

0 comments on commit 6516b1d

Please sign in to comment.