diff --git a/internal/planner/max.go b/internal/planner/max.go index da68cbcd1a..dbcc991268 100644 --- a/internal/planner/max.go +++ b/internal/planner/max.go @@ -24,10 +24,10 @@ type maxNode struct { documentIterator docMapper - p *Planner - plan planNode + p *Planner + plan planNode + parent *mapper.Select - isFloat bool // virtualFieldIndex is the index of the field // that contains the result of the aggregate. virtualFieldIndex int @@ -45,22 +45,9 @@ func (p *Planner) Max( field *mapper.Aggregate, parent *mapper.Select, ) (*maxNode, error) { - isFloat := false - for _, target := range field.AggregateTargets { - isTargetFloat, err := p.isValueFloat(parent, &target) - if err != nil { - return nil, err - } - // If one source property is a float, the result will be a float - no need to check the rest - if isTargetFloat { - isFloat = true - break - } - } - return &maxNode{ p: p, - isFloat: isFloat, + parent: parent, aggregateMapping: field.AggregateTargets, virtualFieldIndex: field.Index, docMapper: docMapper{field.DocumentMapping}, @@ -142,6 +129,8 @@ func (n *maxNode) Next() (bool, error) { n.currentValue = n.plan.Value() var max *big.Float + isFloat := false + for _, source := range n.aggregateMapping { child := n.currentValue.Fields[source.Index] var collectionMax *big.Float @@ -242,14 +231,20 @@ func (n *maxNode) Next() (bool, error) { if err != nil { return false, err } - if max == nil || collectionMax == nil || collectionMax.Cmp(max) > 0 { - max = collectionMax + if collectionMax == nil || (max != nil && collectionMax.Cmp(max) <= 0) { + continue + } + isTargetFloat, err := n.p.isValueFloat(n.parent, &source) + if err != nil { + return false, err } + isFloat = isTargetFloat + max = collectionMax } if max == nil { n.currentValue.Fields[n.virtualFieldIndex] = nil - } else if n.isFloat { + } else if isFloat { res, _ := max.Float64() n.currentValue.Fields[n.virtualFieldIndex] = res } else { diff --git a/internal/planner/min.go b/internal/planner/min.go index e43d2b4564..9be8ecd30a 100644 --- a/internal/planner/min.go +++ b/internal/planner/min.go @@ -24,10 +24,10 @@ type minNode struct { documentIterator docMapper - p *Planner - plan planNode + p *Planner + plan planNode + parent *mapper.Select - isFloat bool // virtualFieldIndex is the index of the field // that contains the result of the aggregate. virtualFieldIndex int @@ -45,22 +45,9 @@ func (p *Planner) Min( field *mapper.Aggregate, parent *mapper.Select, ) (*minNode, error) { - isFloat := false - for _, target := range field.AggregateTargets { - isTargetFloat, err := p.isValueFloat(parent, &target) - if err != nil { - return nil, err - } - // If one source property is a float, the result will be a float - no need to check the rest - if isTargetFloat { - isFloat = true - break - } - } - return &minNode{ p: p, - isFloat: isFloat, + parent: parent, aggregateMapping: field.AggregateTargets, virtualFieldIndex: field.Index, docMapper: docMapper{field.DocumentMapping}, @@ -142,6 +129,8 @@ func (n *minNode) Next() (bool, error) { n.currentValue = n.plan.Value() var min *big.Float + isFloat := false + for _, source := range n.aggregateMapping { child := n.currentValue.Fields[source.Index] var collectionMin *big.Float @@ -242,14 +231,20 @@ func (n *minNode) Next() (bool, error) { if err != nil { return false, err } - if min == nil || collectionMin == nil || collectionMin.Cmp(min) < 0 { - min = collectionMin + if collectionMin == nil || (min != nil && collectionMin.Cmp(min) >= 0) { + continue + } + isTargetFloat, err := n.p.isValueFloat(n.parent, &source) + if err != nil { + return false, err } + isFloat = isTargetFloat + min = collectionMin } if min == nil { n.currentValue.Fields[n.virtualFieldIndex] = nil - } else if n.isFloat { + } else if isFloat { res, _ := min.Float64() n.currentValue.Fields[n.virtualFieldIndex] = res } else { diff --git a/tests/integration/query/simple/with_max_test.go b/tests/integration/query/simple/with_max_test.go index 7b2a14cb0b..bdb47b6f8c 100644 --- a/tests/integration/query/simple/with_max_test.go +++ b/tests/integration/query/simple/with_max_test.go @@ -11,9 +11,12 @@ package simple import ( + "math" "testing" testUtils "github.com/sourcenetwork/defradb/tests/integration" + + "github.com/sourcenetwork/immutable" ) func TestQuerySimple_WithMaxOnUndefinedObject_ReturnsError(t *testing.T) { @@ -95,3 +98,32 @@ func TestQuerySimple_WithMax_Succeeds(t *testing.T) { executeTestCase(t, test) } + +func TestQuerySimple_WithMaxAndMaxValueInt_Succeeds(t *testing.T) { + test := testUtils.TestCase{ + SupportedMutationTypes: immutable.Some([]testUtils.MutationType{ + // GraphQL does not support 64 bit int + testUtils.CollectionSaveMutationType, + testUtils.CollectionNamedMutationType, + }), + Description: "Simple query max and max value int", + Actions: []any{ + testUtils.CreateDoc{ + DocMap: map[string]any{ + "Name": "John", + "Age": int64(math.MaxInt64), + }, + }, + testUtils.Request{ + Request: `query { + _max(Users: {field: Age}) + }`, + Results: map[string]any{ + "_max": int64(math.MaxInt64), + }, + }, + }, + } + + executeTestCase(t, test) +} diff --git a/tests/integration/query/simple/with_min_test.go b/tests/integration/query/simple/with_min_test.go index 33311aebd2..feb8e54e2f 100644 --- a/tests/integration/query/simple/with_min_test.go +++ b/tests/integration/query/simple/with_min_test.go @@ -11,9 +11,12 @@ package simple import ( + "math" "testing" testUtils "github.com/sourcenetwork/defradb/tests/integration" + + "github.com/sourcenetwork/immutable" ) func TestQuerySimple_WithMinOnUndefinedObject_ReturnsError(t *testing.T) { @@ -95,3 +98,32 @@ func TestQuerySimple_WithMin_Succeeds(t *testing.T) { executeTestCase(t, test) } + +func TestQuerySimple_WithMinAndMaxValueInt_Succeeds(t *testing.T) { + test := testUtils.TestCase{ + SupportedMutationTypes: immutable.Some([]testUtils.MutationType{ + // GraphQL does not support 64 bit int + testUtils.CollectionSaveMutationType, + testUtils.CollectionNamedMutationType, + }), + Description: "Simple query min and max value int", + Actions: []any{ + testUtils.CreateDoc{ + DocMap: map[string]any{ + "Name": "John", + "Age": int64(math.MaxInt64), + }, + }, + testUtils.Request{ + Request: `query { + _max(Users: {field: Age}) + }`, + Results: map[string]any{ + "_max": int64(math.MaxInt64), + }, + }, + }, + } + + executeTestCase(t, test) +}