storm icon indicating copy to clipboard operation
storm copied to clipboard

Adding nullable time comparison

Open benber86 opened this issue 6 years ago • 3 comments

Storm currently offers the possibility to compare time.Time value, which works great. Unfortunately this does not work for nullable Time types (types that can represent a time.Time that may be Null), such as go-sql-driver's or pq's NullTime, null's null.Time or similar, since the time.Time value is nested.

Since all of these follow the same format:

    type NullTime struct {
        time.Time
        Valid bool
    }

it would be possible to add support for it in compare:

if (reflect.TypeOf(a).String() == "null.Time" || strings.HasSuffix(reflect.TypeOf(a).String(), "NullTime")) &&
  (reflect.TypeOf(b).String() == reflect.TypeOf(a).String()) {
  var x, y int64
  x = 1
  validfielda := vala.FieldByName("Valid")
  validfieldb := valb.FieldByName("Valid")
  timea := vala.FieldByName("Time")
  timeb := valb.FieldByName("Time")

  if !validfielda.IsValid() || !validfieldb.IsValid() {
    return false
  }

  if validfielda.Interface() == false && validfieldb.Interface() == false && tok == token.EQL {
    return true
  }
  if (validfielda.Interface() != true) || (validfieldb.Interface() != true) {
    return false
  }

  equala := timea.MethodByName("Equal")
  beforea := timea.MethodByName("Before")

  if !equala.IsValid() || !beforea.IsValid() {
    return false
  }

  if equala.Call([]reflect.Value{timeb})[0].Bool() {
    y = 1
  } else if beforea.Call([]reflect.Value{timeb})[0].Bool() {
    y = 2
  }
  return constant.Compare(constant.MakeInt64(x), tok, constant.MakeInt64(y))
}

provided that you deem the addition useful enough and don't mind adding support for custom types.

benber86 avatar Jun 11 '18 20:06 benber86

Another option would be to add support for nested struct in queries as mentioned in #166 I tried with a basic patch on MatchValue to add parsing of period-separated nested query strings:

    func GetNestedValue(v *reflect.Value, fields []string) (interface{}, error) {
        nestedV := *v
        for _, element := range fields {
          nestedField := nestedV.FieldByName(element)
          if !nestedField.IsValid() {
            return nil, ErrUnknownField
          }      
          nestedV = nestedField
        }
        return nestedV.Interface(), nil
    }
    
    func (r fieldMatcherDelegate) MatchValue(v *reflect.Value) (bool, error) {
      if !strings.Contains(r.Field, ".") {
        field := v.FieldByName(r.Field)
        if !field.IsValid() {
          return false, ErrUnknownField
        }
        return r.MatchField(field.Interface())
      } else {
        nestedValue, err := GetNestedValue(v, strings.Split(r.Field, "."))
        if err != nil {
          return false, err
        }
        return r.MatchField(nestedValue)
      }
    }

which works but is a bit less practical for queries on NullTime as one would have to be careful to have both a comparison on the Time field and a check on the Valid field when creating the query.

benber86 avatar Jun 11 '18 20:06 benber86

What about adding an interface that custom types would have to implement? Ex:

type Comparable interface {
  // Returns a number negative, null or positive if lower, equal or greater than the other.
  Compare(other interface{}) (int, error)
}

And it could be used that way

type NullTime pq.NullTime

func (n *NullTime) Compare(other interface{}) (int, error) {
  o, ok := other.(*pq.NullTime)
  if !ok {
    return 0, errors.New(...)
  }

  // compare and return
  ...
}

asdine avatar Jun 24 '18 10:06 asdine

Ok, so the only thing needed in compare would be to check that the interface is implemented with a type assertion and then run the comparison ?:

o, ok := b.(Comparable)
if ok && ak == bk {
	result, err := o.Compare(a)
	if err != nil {
		return false
	}
	return constant.Compare(constant.MakeInt64(0), tok, constant.MakeInt64(int64(result)))
}

benber86 avatar Jul 12 '18 07:07 benber86