Skip to content

Commit

Permalink
feat: Filter alias target (sourcenetwork#3201)
Browse files Browse the repository at this point in the history
## Relevant issue(s)

Resolves sourcenetwork#3194

## Description

This PR adds alias targeting in filters. 

**Aggregate targets are not included in this PR as they require more
changes.**

## Tasks

- [x] I made sure the code is well commented, particularly
hard-to-understand areas.
- [x] I made sure the repository-held documentation is changed
accordingly.
- [x] I made sure the pull request title adheres to the conventional
commit style (the subset used in the project can be found in
[tools/configs/chglog/config.yml](tools/configs/chglog/config.yml)).
- [x] I made sure to discuss its limitations such as threats to
validity, vulnerability to mistake and misuse, robustness to
invalidation of assumptions, resource requirements, ...

## How has this been tested?

Added and updated integration tests.

Specify the platform(s) on which this was tested:
- MacOS
  • Loading branch information
nasdf authored Nov 6, 2024
1 parent 2b0b862 commit c8fd3b1
Show file tree
Hide file tree
Showing 19 changed files with 610 additions and 65 deletions.
7 changes: 4 additions & 3 deletions client/request/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@ package request
import "github.com/sourcenetwork/immutable"

const (
FilterOpOr = "_or"
FilterOpAnd = "_and"
FilterOpNot = "_not"
FilterOpOr = "_or"
FilterOpAnd = "_and"
FilterOpNot = "_not"
FilterOpAlias = "_alias"
)

// Filter contains the parsed condition map to be
Expand Down
9 changes: 5 additions & 4 deletions internal/connor/connor.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@
package connor

const (
AndOp = "_and"
OrOp = "_or"
NotOp = "_not"
AliasOp = "_alias"
AndOp = "_and"
OrOp = "_or"
NotOp = "_not"

AnyOp = "_any"
AllOp = "_all"
Expand Down Expand Up @@ -62,7 +63,7 @@ func matchWith(op string, conditions, data any) (bool, error) {
return anyOp(conditions, data)
case AllOp:
return all(conditions, data)
case EqualOp:
case EqualOp, AliasOp:
return eq(conditions, data)
case GreaterOrEqualOp:
return ge(conditions, data)
Expand Down
9 changes: 7 additions & 2 deletions internal/connor/eq.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,15 @@ func eq(condition, data any) (bool, error) {
switch cn := condition.(type) {
case map[FilterKey]any:
for prop, cond := range cn {
m, err := matchWith(prop.GetOperatorOrDefault(EqualOp), cond, prop.GetProp(data))
d, op, err := prop.PropertyAndOperator(data, EqualOp)
if err != nil {
return false, err
} else if !m {
}
m, err := matchWith(op, cond, d)
if err != nil {
return false, err
}
if !m {
return false, nil
}
}
Expand Down
11 changes: 5 additions & 6 deletions internal/connor/key.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,11 @@ package connor
// FilterKey represents a type that may be used as a map key
// in a filter.
type FilterKey interface {
// GetProp returns the data that should be used with this key
// from the given data.
GetProp(data any) any
// GetOperatorOrDefault returns either the operator that corresponds
// to this key, or the given default.
GetOperatorOrDefault(defaultOp string) string
// PropertyAndOperator returns the data and operator that should be used
// to filter the value matching this key.
//
// If the key does not have an operator the given defaultOp will be returned.
PropertyAndOperator(data any, defaultOp string) (any, string, error)
// Equal returns true if other is equal, otherwise returns false.
Equal(other FilterKey) bool
}
8 changes: 2 additions & 6 deletions internal/connor/not_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,8 @@ type operator struct {
Operation string
}

func (k *operator) GetProp(data any) any {
return data
}

func (k *operator) GetOperatorOrDefault(defaultOp string) string {
return k.Operation
func (k *operator) PropertyAndOperator(data any, defaultOp string) (any, string, error) {
return data, k.Operation, nil
}

func (k *operator) Equal(other FilterKey) bool {
Expand Down
12 changes: 12 additions & 0 deletions internal/core/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,3 +298,15 @@ func (mapping *DocumentMapping) TryToFindNameFromIndex(targetIndex int) (string,

return "", false
}

// TryToFindIndexFromRenderKey returns the corresponding index of the given render key.
//
// Additionally, will also return true if the render key was found, and false otherwise.
func (mapping *DocumentMapping) TryToFindIndexFromRenderKey(key string) (int, bool) {
for _, renderKey := range mapping.RenderKeys {
if renderKey.Key == key {
return renderKey.Index, true
}
}
return -1, false
}
14 changes: 11 additions & 3 deletions internal/planner/filter/copy_field.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,10 @@ func traverseFilterByProperty(
}
}
} else if opKey, isOpKey := targetKey.(*mapper.Operator); isOpKey {
clauseArr, isArr := clause.([]any)
if isArr {
switch t := clause.(type) {
case []any:
resultArr := make([]any, 0)
for _, elementClause := range clauseArr {
for _, elementClause := range t {
elementMap, ok := elementClause.(map[connor.FilterKey]any)
if !ok {
continue
Expand All @@ -80,6 +80,14 @@ func traverseFilterByProperty(
} else if shouldDelete {
delete(result, opKey)
}

case map[connor.FilterKey]any:
resultMap := traverseFilterByProperty(keys, t, shouldDelete)
if len(resultMap) > 0 {
result[opKey] = resultMap
} else if shouldDelete {
delete(result, opKey)
}
}
}
}
Expand Down
10 changes: 10 additions & 0 deletions internal/planner/filter/copy_field_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,16 @@ func TestCopyField(t *testing.T) {
m("age", m("_gt", 55)),
),
},
{
name: "within _not",
inputFilter: m("_not",
m("age", m("_gt", 55)),
),
inputField: []mapper.Field{{Index: authorAgeInd}},
expectedFilter: m("_not",
m("age", m("_gt", 55)),
),
},
{
name: "within _or and _and",
inputFilter: r("_and",
Expand Down
5 changes: 5 additions & 0 deletions internal/planner/mapper/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import "github.com/sourcenetwork/defradb/errors"
const (
errInvalidFieldToGroupBy string = "invalid field value to groupBy"
errTypeNotFound string = "type not found"
errFieldOrAliasNotFound string = "field or alias not found"
)

var (
Expand All @@ -33,3 +34,7 @@ func NewErrInvalidFieldToGroupBy(field string) error {
func NewErrTypeNotFound(name string) error {
return errors.New(errTypeNotFound, errors.NewKV("Type", name))
}

func NewErrFieldOrAliasNotFound(name string) error {
return errors.New(errFieldOrAliasNotFound, errors.NewKV("Name", name))
}
35 changes: 26 additions & 9 deletions internal/planner/mapper/mapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -992,6 +992,12 @@ func resolveInnerFilterDependencies(
newFields := []Requestable{}

for key, value := range source {
// alias fields are guarenteed to be resolved
// because they refer to existing fields
if key == request.FilterOpAlias {
continue
}

if key == request.FilterOpAnd || key == request.FilterOpOr {
if value == nil {
continue
Expand Down Expand Up @@ -1335,8 +1341,27 @@ func toFilterKeyValue(
sourceClause any,
mapping *core.DocumentMapping,
) (connor.FilterKey, any) {
var propIndex = -1
if mapping != nil {
// if we have a mapping available check if the
// source key is a field or alias (render key)
if indexes, ok := mapping.IndexesByName[sourceKey]; ok {
// If there are multiple properties of the same name we can just take the first as
// we have no other reasonable way of identifying which property they mean if multiple
// consumer specified requestables are available. Aggregate dependencies should not
// impact this as they are added after selects.
propIndex = indexes[0]
} else if index, ok := mapping.TryToFindIndexFromRenderKey(sourceKey); ok {
propIndex = index
}
}

var returnKey connor.FilterKey
if strings.HasPrefix(sourceKey, "_") && sourceKey != request.DocIDFieldName {
if propIndex >= 0 {
returnKey = &PropertyIndex{
Index: propIndex,
}
} else if strings.HasPrefix(sourceKey, "_") {
returnKey = &Operator{
Operation: sourceKey,
}
Expand All @@ -1345,14 +1370,6 @@ func toFilterKeyValue(
if connor.IsOpSimple(sourceKey) {
return returnKey, sourceClause
}
} else if mapping != nil && len(mapping.IndexesByName[sourceKey]) > 0 {
// If there are multiple properties of the same name we can just take the first as
// we have no other reasonable way of identifying which property they mean if multiple
// consumer specified requestables are available. Aggregate dependencies should not
// impact this as they are added after selects.
returnKey = &PropertyIndex{
Index: mapping.FirstIndexOfName(sourceKey),
}
} else {
returnKey = &ObjectProperty{
Name: sourceKey,
Expand Down
36 changes: 13 additions & 23 deletions internal/planner/mapper/targetable.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,11 @@ type PropertyIndex struct {
Index int
}

func (k *PropertyIndex) GetProp(data any) any {
func (k *PropertyIndex) PropertyAndOperator(data any, defaultOp string) (any, string, error) {
if data == nil {
return nil
return nil, defaultOp, nil
}

return data.(core.Doc).Fields[k.Index]
}

func (k *PropertyIndex) GetOperatorOrDefault(defaultOp string) string {
return defaultOp
return data.(core.Doc).Fields[k.Index], defaultOp, nil
}

func (k *PropertyIndex) Equal(other connor.FilterKey) bool {
Expand All @@ -57,12 +52,8 @@ type Operator struct {
Operation string
}

func (k *Operator) GetProp(data any) any {
return data
}

func (k *Operator) GetOperatorOrDefault(defaultOp string) string {
return k.Operation
func (k *Operator) PropertyAndOperator(data any, defaultOp string) (any, string, error) {
return data, k.Operation, nil
}

func (k *Operator) Equal(other connor.FilterKey) bool {
Expand All @@ -81,16 +72,15 @@ type ObjectProperty struct {
Name string
}

func (k *ObjectProperty) GetProp(data any) any {
func (k *ObjectProperty) PropertyAndOperator(data any, defaultOp string) (any, string, error) {
if data == nil {
return nil
return nil, defaultOp, nil
}
object := data.(map[string]any)
return object[k.Name]
}

func (k *ObjectProperty) GetOperatorOrDefault(defaultOp string) string {
return defaultOp
docMap, ok := data.(map[string]any)
if !ok {
return nil, defaultOp, NewErrFieldOrAliasNotFound(k.Name)
}
return docMap[k.Name], defaultOp, nil
}

func (k *ObjectProperty) Equal(other connor.FilterKey) bool {
Expand Down Expand Up @@ -165,7 +155,7 @@ func filterObjectToMap(mapping *core.DocumentMapping, obj map[connor.FilterKey]a
logicMapEntries[i] = filterObjectToMap(mapping, itemMap)
}
outmap[keyType.Operation] = logicMapEntries
case request.FilterOpNot:
case request.FilterOpNot, request.FilterOpAlias:
itemMap, ok := v.(map[connor.FilterKey]any)
if ok {
outmap[keyType.Operation] = filterObjectToMap(mapping, itemMap)
Expand Down
12 changes: 6 additions & 6 deletions internal/request/graphql/parser/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,20 +93,20 @@ func parseFilterFieldsForDescriptionMap(
fields := make([]client.FieldDefinition, 0)
for k, v := range conditions {
switch k {
case "_or", "_and":
case request.FilterOpOr, request.FilterOpAnd:
conds := v.([]any)
parsedFileds, err := parseFilterFieldsForDescriptionSlice(conds, col)
parsedFields, err := parseFilterFieldsForDescriptionSlice(conds, col)
if err != nil {
return nil, err
}
fields = append(fields, parsedFileds...)
case "_not":
fields = append(fields, parsedFields...)
case request.FilterOpNot, request.FilterOpAlias:
conds := v.(map[string]any)
parsedFileds, err := parseFilterFieldsForDescriptionMap(conds, col)
parsedFields, err := parseFilterFieldsForDescriptionMap(conds, col)
if err != nil {
return nil, err
}
fields = append(fields, parsedFileds...)
fields = append(fields, parsedFields...)
default:
f, found := col.GetFieldByName(k)
if !found || f.Kind.IsObject() {
Expand Down
10 changes: 7 additions & 3 deletions internal/request/graphql/schema/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -1188,18 +1188,22 @@ func (g *Generator) genTypeFilterArgInput(obj *gql.Object) *gql.InputObject {
func() (gql.InputObjectConfigFieldMap, error) {
fields := gql.InputObjectConfigFieldMap{}

fields["_and"] = &gql.InputObjectFieldConfig{
fields[request.FilterOpAnd] = &gql.InputObjectFieldConfig{
Description: schemaTypes.AndOperatorDescription,
Type: gql.NewList(gql.NewNonNull(selfRefType)),
}
fields["_or"] = &gql.InputObjectFieldConfig{
fields[request.FilterOpOr] = &gql.InputObjectFieldConfig{
Description: schemaTypes.OrOperatorDescription,
Type: gql.NewList(gql.NewNonNull(selfRefType)),
}
fields["_not"] = &gql.InputObjectFieldConfig{
fields[request.FilterOpNot] = &gql.InputObjectFieldConfig{
Description: schemaTypes.NotOperatorDescription,
Type: selfRefType,
}
fields[request.FilterOpAlias] = &gql.InputObjectFieldConfig{
Description: "The alias operator allows filters to target aliased fields.",
Type: schemaTypes.JSONScalarType(),
}

// generate basic filter operator blocks
for f, field := range obj.Fields() {
Expand Down
Loading

0 comments on commit c8fd3b1

Please sign in to comment.