If the field doesn't exist return ErrFieldNotFound (#2081)

This way we don't return seemingly valid data for calls on non-existent fields. See
also [FB-237](https://molecula.atlassian.net/browse/FB-237).
This commit is contained in:
reesporte 2022-05-26 16:45:10 -05:00 committed by GitHub
parent 8ba81643d2
commit 964f14a3a1
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 38 additions and 6 deletions

View file

@ -1861,7 +1861,7 @@ func (e *executor) executeSumCountShard(ctx context.Context, qcx *Qcx, index str
field := e.Holder.Field(index, fieldName)
if field == nil {
return ValCount{}, nil
return ValCount{}, ErrFieldNotFound
}
bsig := field.bsiGroup(fieldName)
@ -1919,7 +1919,7 @@ func (e *executor) executeMinShard(ctx context.Context, qcx *Qcx, index string,
field := e.Holder.Field(index, fieldName)
if field == nil {
return ValCount{}, nil
return ValCount{}, ErrFieldNotFound
}
tx, finisher, err := qcx.GetTx(Txo{Write: !writable, Index: idx, Shard: shard})
@ -1951,7 +1951,7 @@ func (e *executor) executeMaxShard(ctx context.Context, qcx *Qcx, index string,
field := e.Holder.Field(index, fieldName)
if field == nil {
return ValCount{}, nil
return ValCount{}, ErrFieldNotFound
}
tx, finisher, err := qcx.GetTx(Txo{Write: !writable, Index: idx, Shard: shard})
@ -1978,7 +1978,7 @@ func (e *executor) executeMinRowShard(ctx context.Context, qcx *Qcx, index strin
fieldName, _ := c.Args["field"].(string)
field := e.Holder.Field(index, fieldName)
if field == nil {
return PairField{}, nil
return PairField{}, ErrFieldNotFound
}
fragment := e.Holder.fragment(index, fieldName, viewStandard, shard)
@ -2023,7 +2023,7 @@ func (e *executor) executeMaxRowShard(ctx context.Context, qcx *Qcx, index strin
fieldName, _ := c.Args["field"].(string)
field := e.Holder.Field(index, fieldName)
if field == nil {
return PairField{}, nil
return PairField{}, ErrFieldNotFound
}
fragment := e.Holder.fragment(index, fieldName, viewStandard, shard)

View file

@ -2664,7 +2664,12 @@ func TestExecutor_Execute_MinMaxRow(t *testing.T) {
t.Fatalf("unexpected result %v != %v", target, result.Results[0])
}
})
t.Run("MinRowNonExistent", func(t *testing.T) {
_, err := c.GetNode(0).API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: "MinRow(field=fake)"})
if got, exp := err.Error(), "executing: executeMinRow: mapping on primary node: field not found"; got != exp {
t.Fatalf("expected %v, got %v", exp, got)
}
})
t.Run("MaxRow", func(t *testing.T) {
result, err := c.GetNode(0).API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: "MaxRow(field=f)"})
if err != nil {
@ -2678,6 +2683,12 @@ func TestExecutor_Execute_MinMaxRow(t *testing.T) {
t.Fatalf("unexpected result %v != %v", target, result.Results[0])
}
})
t.Run("MaxRowNonExistent", func(t *testing.T) {
_, err := c.GetNode(0).API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: "MaxRow(field=fake)"})
if got, exp := err.Error(), "executing: executeMaxRow: mapping on primary node: field not found"; got != exp {
t.Fatalf("expected %v, got %v", exp, got)
}
})
})
t.Run("RowKey", func(t *testing.T) {
@ -2827,6 +2838,13 @@ func TestExecutor_Execute_Sum(t *testing.T) {
})
})
t.Run("SumNonExistent", func(t *testing.T) {
_, err := c.GetNode(0).API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Sum(field=fake)`})
if err.Error() != "executing: executeSum: mapping on primary node: field not found" {
t.Fatal(err)
}
})
t.Run("Decimal", func(t *testing.T) {
t.Run("NoFilter", func(t *testing.T) {
if result, err := c.GetNode(0).API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Sum(field=dec)`}); err != nil {
@ -6627,6 +6645,20 @@ func TestExecutor_Execute_MinMaxCountEqual(t *testing.T) {
}
}
})
t.Run("MinNonExistent", func(t *testing.T) {
pql := `Min(field=fake)`
_, err := c.GetNode(0).API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: pql})
if err.Error() != "executing: executeMin: mapping on primary node: field not found" {
t.Fatal(err)
}
})
t.Run("MaxNonExistent", func(t *testing.T) {
pql := `Max(field=fake)`
_, err := c.GetNode(0).API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: pql})
if err.Error() != "executing: executeMax: mapping on primary node: field not found" {
t.Fatal(err)
}
})
t.Run("MinDec", func(t *testing.T) {
tests := []struct {
filter string