Repository: genjidb/genji (go)

Issue:
Inserting empty array and document into boolean field
If I insert an empty array or an empty document into a boolean field, I would expect that the value of that field will be `false`. Exactly the same way as with empty strings.

What do you think?

```sql
genji> CREATE TABLE test(a bool, b bool, c bool);
genji> INSERT INTO test VALUES {a: "", b: [], c: {}};
genji> SELECT * FROM test;
{
  "a": false,
  "b": true,
  "c": true
}
```

Pull request description:
Consider empty documents and arrays as falsy
This fixes #89.

There is some significative changes in this PR:
- I added a `Length` function that returns the length of a document same as `ArrayLength` for arrays
- `Value.IsZeroValue` now returns an error next to the initial returned parameter because the underlying calls might return errors
- `Value.IsTruthy` now returns an error because of the previous point

Patch:
diff --git a/document/compare.go b/document/compare.go
index 0f7dd337..e74a73ff 100644
--- a/document/compare.go
+++ b/document/compare.go
@@ -131,16 +131,13 @@ func compareWithBool(op operator, l, r Value) (bool, error) {
 
 	var a, b bool
 
-	if l.Type == BoolValue {
-		a = l.V.(bool)
-	} else {
-		a = l.IsTruthy()
+	a, err := l.IsTruthy()
+	if err != nil {
+		return false, err
 	}
-
-	if r.Type == BoolValue {
-		b = r.V.(bool)
-	} else {
-		b = r.IsTruthy()
+	b, err = r.IsTruthy()
+	if err != nil {
+		return false, err
 	}
 
 	switch op {
diff --git a/document/document.go b/document/document.go
index 83c63ad6..58fcdac3 100644
--- a/document/document.go
+++ b/document/document.go
@@ -27,6 +27,20 @@ type Keyer interface {
 	Key() []byte
 }
 
+// Length returns the length of a document.
+func Length(d Document) (int, error) {
+	if fb, ok := d.(*FieldBuffer); ok {
+		return fb.Len(), nil
+	}
+
+	var len int
+	err := d.Iterate(func(_ string, _ Value) error {
+		len++
+		return nil
+	})
+	return len, err
+}
+
 // FieldBuffer stores a group of fields in memory. It implements the Document interface.
 type FieldBuffer struct {
 	fields []fieldValue
diff --git a/document/value.go b/document/value.go
index c2d66ce2..74b7287d 100644
--- a/document/value.go
+++ b/document/value.go
@@ -253,12 +253,13 @@ func NewZeroValue(t ValueType) Value {
 }
 
 // IsTruthy returns whether v is not equal to the zero value of its type.
-func (v Value) IsTruthy() bool {
+func (v Value) IsTruthy() (bool, error) {
 	if v.Type == NullValue {
-		return false
+		return false, nil
 	}
 
-	return !v.IsZeroValue()
+	b, err := v.IsZeroValue()
+	return !b, err
 }
 
 // ConvertTo decodes v to the selected type when possible.
@@ -382,7 +383,8 @@ func (v Value) ConvertToBool() (bool, error) {
 		return false, nil
 	}
 
-	return !v.IsZeroValue(), nil
+	b, err := v.IsZeroValue()
+	return !b, err
 }
 
 // ConvertToInt64 turns any number into an int64.
@@ -494,27 +496,53 @@ func (v Value) ConvertToDuration() (time.Duration, error) {
 
 // IsZeroValue indicates if the value data is the zero value for the value type.
 // This function doesn't perform any allocation.
-func (v Value) IsZeroValue() bool {
+func (v Value) IsZeroValue() (bool, error) {
 	switch v.Type {
 	case BlobValue, TextValue:
-		return bytesutil.CompareBytes(v.V.([]byte), blobZeroValue.V.([]byte)) == 0
+		return bytesutil.CompareBytes(v.V.([]byte), blobZeroValue.V.([]byte)) == 0, nil
 	case BoolValue:
-		return v.V == boolZeroValue.V
+		return v.V == boolZeroValue.V, nil
 	case Int8Value:
-		return v.V == int8ZeroValue.V
+		return v.V == int8ZeroValue.V, nil
 	case Int16Value:
-		return v.V == int16ZeroValue.V
+		return v.V == int16ZeroValue.V, nil
 	case Int32Value:
-		return v.V == int32ZeroValue.V
+		return v.V == int32ZeroValue.V, nil
 	case Int64Value:
-		return v.V == int64ZeroValue.V
+		return v.V == int64ZeroValue.V, nil
 	case Float64Value:
-		return v.V == float64ZeroValue.V
+		return v.V == float64ZeroValue.V, nil
 	case DurationValue:
-		return v.V == durationZeroValue.V
+		return v.V == durationZeroValue.V, nil
+	case ArrayValue:
+		// The zero value of an array is an empty array.
+		// Thus, if GetByIndex(0) returns the ErrValueNotFound
+		// it means that the array is empty.
+		_, err := v.V.(Array).GetByIndex(0)
+		if err == ErrValueNotFound {
+			return true, nil
+		}
+		return false, err
+	case DocumentValue:
+		err := v.V.(Document).Iterate(func(_ string, _ Value) error {
+			// We return an error in the first iteration to stop it.
+			return errStop
+		})
+		if err == nil {
+			// If err is nil, it means that we didn't iterate,
+			// thus the document is empty.
+			return true, nil
+		}
+		if err == errStop {
+			// If err is errStop, it means that we iterate
+			// at least once, thus the document is not empty.
+			return false, nil
+		}
+		// An unexpecting error occurs, let's return it!
+		return false, err
 	}
 
-	return false
+	return false, nil
 }
 
 // Add u to v and return the result.
diff --git a/sql/query/expr/logical.go b/sql/query/expr/logical.go
index aba4e078..f431eff4 100644
--- a/sql/query/expr/logical.go
+++ b/sql/query/expr/logical.go
@@ -15,16 +15,24 @@ func And(a, b Expr) Expr {
 	return &AndOp{&simpleOperator{a, b, scanner.AND}}
 }
 
-// Eval implements the Expr interface. It evaluates a and b and returns true if both evalutate
+// Eval implements the Expr interface. It evaluates a and b and returns true if both evaluate
 // to true.
 func (op *AndOp) Eval(ctx EvalStack) (document.Value, error) {
 	s, err := op.a.Eval(ctx)
-	if err != nil || !s.IsTruthy() {
+	if err != nil {
+		return falseLitteral, err
+	}
+	isTruthy, err := s.IsTruthy()
+	if !isTruthy || err != nil {
 		return falseLitteral, err
 	}
 
 	s, err = op.b.Eval(ctx)
-	if err != nil || !s.IsTruthy() {
+	if err != nil {
+		return falseLitteral, err
+	}
+	isTruthy, err = s.IsTruthy()
+	if !isTruthy || err != nil {
 		return falseLitteral, err
 	}
 
@@ -48,7 +56,11 @@ func (op *OrOp) Eval(ctx EvalStack) (document.Value, error) {
 	if err != nil {
 		return falseLitteral, err
 	}
-	if s.IsTruthy() {
+	isTruthy, err := s.IsTruthy()
+	if err != nil {
+		return falseLitteral, err
+	}
+	if isTruthy {
 		return trueLitteral, nil
 	}
 
@@ -56,7 +68,11 @@ func (op *OrOp) Eval(ctx EvalStack) (document.Value, error) {
 	if err != nil {
 		return falseLitteral, err
 	}
-	if s.IsTruthy() {
+	isTruthy, err = s.IsTruthy()
+	if err != nil {
+		return falseLitteral, err
+	}
+	if isTruthy {
 		return trueLitteral, nil
 	}
 
diff --git a/sql/query/query.go b/sql/query/query.go
index 602e0c1d..6aa45a32 100644
--- a/sql/query/query.go
+++ b/sql/query/query.go
@@ -146,6 +146,6 @@ func whereClause(e expr.Expr, stack expr.EvalStack) func(d document.Document) (b
 			return false, err
 		}
 
-		return v.IsTruthy(), nil
+		return v.IsTruthy()
 	}
 }


Test patch:
diff --git a/document/value_test.go b/document/value_test.go
index 1dc05d8c..9d1b15b0 100644
--- a/document/value_test.go
+++ b/document/value_test.go
@@ -198,9 +198,9 @@ func TestConvertToBool(t *testing.T) {
 		{"float64", document.NewFloat64Value(10.1), false, true},
 		{"zero float64", document.NewFloat64Value(0), false, false},
 		{"document", document.NewDocumentValue(document.NewFieldBuffer().Add("a", document.NewBoolValue(true))), false, true},
-		{"zero document", document.NewDocumentValue(document.NewFieldBuffer()), false, true},
+		{"zero document", document.NewDocumentValue(document.NewFieldBuffer()), false, false},
 		{"array", document.NewArrayValue(document.NewValueBuffer(document.NewInt16Value(1))), false, true},
-		{"zero array", document.NewArrayValue(document.NewValueBuffer()), false, true},
+		{"zero array", document.NewArrayValue(document.NewValueBuffer()), false, false},
 		{"duration", document.NewDurationValue(10 * time.Nanosecond), false, true},
 		{"zero duration", document.NewDurationValue(0), false, false},
 	}