diff --git a/executor.go b/executor.go index 5733c4b87..2c127cc67 100644 --- a/executor.go +++ b/executor.go @@ -861,7 +861,6 @@ func (e *executor) executeGenericField(ctx context.Context, index string, c *pql func (e *executor) executeMin(ctx context.Context, index string, c *pql.Call, shards []uint64, opt *execOptions) (ValCount, error) { span, ctx := tracing.StartSpanFromContext(ctx, "Executor.executeMin") defer span.Finish() - if field := c.Args["field"]; field == "" { return ValCount{}, errors.New("Min(): field required") } @@ -4124,7 +4123,7 @@ func (vc *ValCount) add(other ValCount) ValCount { // smaller returns the smaller of the two ValCounts. func (vc *ValCount) smaller(other ValCount) ValCount { - if vc.DecimalVal != nil { + if vc.DecimalVal != nil || other.DecimalVal != nil { return vc.decimalSmaller(other) } else if vc.FloatVal != 0 || other.FloatVal != 0 { return vc.floatSmaller(other) @@ -4143,7 +4142,10 @@ func (vc *ValCount) smaller(other ValCount) ValCount { } func (vc *ValCount) decimalSmaller(other ValCount) ValCount { - if vc.Count == 0 || (other.DecimalVal.LessThan(*vc.DecimalVal) && other.Count > 0) { + if other.DecimalVal == nil { + return *vc + } + if vc.Count == 0 || vc.DecimalVal == nil || (other.DecimalVal.LessThan(*vc.DecimalVal) && other.Count > 0) { return other } extra := int64(0) @@ -4172,7 +4174,7 @@ func (vc *ValCount) floatSmaller(other ValCount) ValCount { // larger returns the larger of the two ValCounts. func (vc *ValCount) larger(other ValCount) ValCount { - if vc.DecimalVal != nil { + if vc.DecimalVal != nil || other.DecimalVal != nil { return vc.decimalLarger(other) } else if vc.FloatVal != 0 || other.FloatVal != 0 { return vc.floatLarger(other) @@ -4191,7 +4193,10 @@ func (vc *ValCount) larger(other ValCount) ValCount { } func (vc *ValCount) decimalLarger(other ValCount) ValCount { - if vc.Count == 0 || (other.DecimalVal.GreaterThan(*vc.DecimalVal) && other.Count > 0) { + if other.DecimalVal == nil { + return *vc + } + if vc.Count == 0 || vc.DecimalVal == nil || (other.DecimalVal.GreaterThan(*vc.DecimalVal) && other.Count > 0) { return other } extra := int64(0) diff --git a/executor_test.go b/executor_test.go index dd0f7b811..8ca45b777 100644 --- a/executor_test.go +++ b/executor_test.go @@ -1512,6 +1512,38 @@ func TestExecutor_Execute_MinMax(t *testing.T) { pql.Decimal{Value: -1150, Scale: 2}, }, } + // This extra field exists to make there be shards which are present, + // but have no decimal values set, to make sure they don't break + // the results. + if _, err := idx.CreateFieldIfNotExists("z", pilosa.OptFieldTypeDefault()); err != nil { + t.Fatal(err) + } + if res, err := c[0].API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Set(1, z=0)`}); err != nil { + t.Fatal(err) + } else if !res.Results[0].(bool) { + t.Fatalf("expected column changed") + } + // set things in other shards, that won't have decimal values + if res, err := c[0].API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Set(1234567, z=0)`}); err != nil { + t.Fatal(err) + } else if !res.Results[0].(bool) { + t.Fatalf("expected column changed") + } + if res, err := c[0].API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Set(2345678, z=0)`}); err != nil { + t.Fatal(err) + } else if !res.Results[0].(bool) { + t.Fatalf("expected column changed") + } + if res, err := c[0].API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Set(3456789, z=0)`}); err != nil { + t.Fatal(err) + } else if !res.Results[0].(bool) { + t.Fatalf("expected column changed") + } + if res, err := c[0].API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Set(4567890, z=0)`}); err != nil { + t.Fatal(err) + } else if !res.Results[0].(bool) { + t.Fatalf("expected column changed") + } for i, test := range tests { fld := fmt.Sprintf("f%d", i) t.Run("MinMaxField_"+fld, func(t *testing.T) { @@ -1520,7 +1552,7 @@ func TestExecutor_Execute_MinMax(t *testing.T) { } if _, err := c[0].API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: fmt.Sprintf(` - Set(10, %s=%s) + Set(6700000, %s=%s) `, fld, test.set)}); err != nil { t.Fatal(err) }