Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Filter splitting and rendering for related types #1541

Merged
merged 8 commits into from
May 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions planner/count.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,22 @@ func (n *countNode) simpleExplain() (map[string]any, error) {
simpleExplainMap := map[string]any{}

// Add the filter attribute if it exists.
if source.Filter == nil || source.Filter.ExternalConditions == nil {
if source.Filter == nil {
simpleExplainMap[filterLabel] = nil
} else {
simpleExplainMap[filterLabel] = source.Filter.ExternalConditions
// get the target aggregate document mapping. Since the filters
// are relative to the target aggregate collection (and doc mapper).
//
// We can determine if there is a child map if the index from the
// aggregate target is set (non nil) on the childMapping
var targetMap *core.DocumentMapping
if source.Index < len(n.documentMapping.ChildMappings) &&
n.documentMapping.ChildMappings[source.Index] != nil {
targetMap = n.documentMapping.ChildMappings[source.Index]
} else {
targetMap = n.documentMapping
}
simpleExplainMap[filterLabel] = source.Filter.ToMap(targetMap)
}

// Add the main field name.
Expand Down
4 changes: 2 additions & 2 deletions planner/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,10 @@ func (n *deleteNode) simpleExplain() (map[string]any, error) {
simpleExplainMap[idsLabel] = n.ids

// Add the filter attribute if it exists, otherwise have it nil.
if n.filter == nil || n.filter.ExternalConditions == nil {
if n.filter == nil {
simpleExplainMap[filterLabel] = nil
} else {
simpleExplainMap[filterLabel] = n.filter.ExternalConditions
simpleExplainMap[filterLabel] = n.filter.ToMap(n.documentMapping)
}

return simpleExplainMap, nil
Expand Down
4 changes: 2 additions & 2 deletions planner/group.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,10 +236,10 @@ func (n *groupNode) simpleExplain() (map[string]any, error) {
childExplainGraph["docKeys"] = nil
}

if c.Filter == nil || c.Filter.ExternalConditions == nil {
if c.Filter == nil {
childExplainGraph[filterLabel] = nil
} else {
childExplainGraph[filterLabel] = c.Filter.ExternalConditions
childExplainGraph[filterLabel] = c.Filter.ToMap(n.documentMapping)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: Does this need a new test as well?

}

if c.Limit != nil {
Expand Down
46 changes: 46 additions & 0 deletions planner/mapper/targetable.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,52 @@ func NewFilter() *Filter {
}
}

func (f *Filter) ToMap(mapping *core.DocumentMapping) map[string]any {
return filterObjectToMap(mapping, f.Conditions)
}

func filterObjectToMap(mapping *core.DocumentMapping, obj map[connor.FilterKey]any) map[string]any {
outmap := make(map[string]any)
if obj == nil {
return nil
}
for k, v := range obj {
switch keyType := k.(type) {
case *PropertyIndex:

subObj := v.(map[connor.FilterKey]any)
outkey, _ := mapping.TryToFindNameFromIndex(keyType.Index)
if childMapping, ok := tryGetChildMapping(mapping, keyType.Index); !ok {
outmap[outkey] = filterObjectToMap(mapping, subObj)
} else {
outmap[outkey] = filterObjectToMap(childMapping, subObj)
}

case *Operator:
switch keyType.Operation {
case "_and", "_or":
v := v.([]any)
logicMapEntries := make([]any, len(v))
for i, item := range v {
itemMap := item.(map[connor.FilterKey]any)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: Can this ever not be a map[connor.FilterKey]any type?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did have a whole type checks for all the type casting in this function. But there really isn't a way for them, to not be the respective types, unless there is a notable bug, in which case it should be caught much higher in the call stack.

logicMapEntries[i] = filterObjectToMap(mapping, itemMap)
}
outmap[keyType.Operation] = logicMapEntries
default:
outmap[keyType.Operation] = v
}
}
}
return outmap
}

func tryGetChildMapping(mapping *core.DocumentMapping, index int) (*core.DocumentMapping, bool) {
if index <= len(mapping.ChildMappings)-1 {
return mapping.ChildMappings[index], true
}
return nil, false
}

// Limit represents a limit-offset pairing that controls how many
// and which records will be returned from a request.
type Limit struct {
Expand Down
4 changes: 2 additions & 2 deletions planner/scan.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,10 +160,10 @@ func (n *scanNode) simpleExplain() (map[string]any, error) {
simpleExplainMap := map[string]any{}

// Add the filter attribute if it exists.
if n.filter == nil || n.filter.ExternalConditions == nil {
if n.filter == nil {
simpleExplainMap[filterLabel] = nil
} else {
simpleExplainMap[filterLabel] = n.filter.ExternalConditions
simpleExplainMap[filterLabel] = n.filter.ToMap(n.documentMapping)
}

// Add the collection attributes.
Expand Down
5 changes: 2 additions & 3 deletions planner/select.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,10 +194,10 @@ func (n *selectNode) simpleExplain() (map[string]any, error) {
simpleExplainMap := map[string]any{}

// Add the filter attribute if it exists.
if n.filter == nil || n.filter.ExternalConditions == nil {
if n.filter == nil {
simpleExplainMap[filterLabel] = nil
} else {
simpleExplainMap[filterLabel] = n.filter.ExternalConditions
simpleExplainMap[filterLabel] = n.filter.ToMap(n.documentMapping)
}

return simpleExplainMap, nil
Expand Down Expand Up @@ -359,7 +359,6 @@ func (n *selectNode) addTypeIndexJoin(subSelect *mapper.Select) error {
if err != nil {
return err
}

if err := n.addSubPlan(subSelect.Index, typeIndexJoin); err != nil {
return err
}
Expand Down
13 changes: 11 additions & 2 deletions planner/sum.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,10 +163,19 @@ func (n *sumNode) simpleExplain() (map[string]any, error) {
simpleExplainMap := map[string]any{}

// Add the filter attribute if it exists.
if source.Filter == nil || source.Filter.ExternalConditions == nil {
if source.Filter == nil {
simpleExplainMap[filterLabel] = nil
} else {
simpleExplainMap[filterLabel] = source.Filter.ExternalConditions
// get the target aggregate document mapping. Since the filters
// are relative to the target aggregate collection (and doc mapper).
var targetMap *core.DocumentMapping
if source.Index < len(n.documentMapping.ChildMappings) &&
n.documentMapping.ChildMappings[source.Index] != nil {
targetMap = n.documentMapping.ChildMappings[source.Index]
} else {
targetMap = n.documentMapping
}
simpleExplainMap[filterLabel] = source.Filter.ToMap(targetMap)
}

// Add the main field name.
Expand Down
56 changes: 43 additions & 13 deletions planner/type_join.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,12 +228,17 @@ func splitFilterByType(filter *mapper.Filter, subType int) (*mapper.Filter, *map

keyFound, sub := removeConditionIndex(conditionKey, filter.Conditions)
if !keyFound {
return filter, &mapper.Filter{}
return filter, nil
}

// create new splitup filter
// our schema ensures that if sub exists, its of type map[string]any
splitF := &mapper.Filter{Conditions: map[connor.FilterKey]any{conditionKey: sub}}

// check if we have any remaining filters
if len(filter.Conditions) == 0 {
return nil, splitF
}
return filter, splitF
}

Expand Down Expand Up @@ -264,7 +269,15 @@ func (p *Planner) makeTypeJoinOne(
) (*typeJoinOne, error) {
// split filter
if scan, ok := source.(*scanNode); ok {
scan.filter, parent.filter = splitFilterByType(scan.filter, subType.Index)
var parentfilter *mapper.Filter
scan.filter, parentfilter = splitFilterByType(scan.filter, subType.Index)
if parentfilter != nil {
if parent.filter == nil {
parent.filter = new(mapper.Filter)
}
parent.filter.Conditions = mergeFilterConditions(
parent.filter.Conditions, parentfilter.Conditions)
}
subType.ShowDeleted = parent.selectReq.ShowDeleted
}

Expand Down Expand Up @@ -453,7 +466,15 @@ func (p *Planner) makeTypeJoinMany(
) (*typeJoinMany, error) {
// split filter
if scan, ok := source.(*scanNode); ok {
scan.filter, parent.filter = splitFilterByType(scan.filter, subType.Index)
var parentfilter *mapper.Filter
scan.filter, parentfilter = splitFilterByType(scan.filter, subType.Index)
if parentfilter != nil {
if parent.filter == nil {
parent.filter = new(mapper.Filter)
}
parent.filter.Conditions = mergeFilterConditions(
parent.filter.Conditions, parentfilter.Conditions)
}
subType.ShowDeleted = parent.selectReq.ShowDeleted
}

Expand Down Expand Up @@ -574,19 +595,11 @@ func appendFilterToScanNode(plan planNode, filterCondition map[connor.FilterKey]
switch node := plan.(type) {
case *scanNode:
filter := node.filter
if filter == nil {
if filter == nil && len(filterCondition) > 0 {
filter = mapper.NewFilter()
}

// merge filter conditions
for k, v := range filterCondition {
indexKey, isIndexKey := k.(*mapper.PropertyIndex)
if !isIndexKey {
continue
}
removeConditionIndex(indexKey, filter.Conditions)
filter.Conditions[k] = v
}
filter.Conditions = mergeFilterConditions(filter.Conditions, filterCondition)

node.filter = filter
case nil:
Expand All @@ -597,6 +610,23 @@ func appendFilterToScanNode(plan planNode, filterCondition map[connor.FilterKey]
return nil
}

// merge into dest with src, return dest
func mergeFilterConditions(dest map[connor.FilterKey]any, src map[connor.FilterKey]any) map[connor.FilterKey]any {
if dest == nil {
dest = make(map[connor.FilterKey]any)
}
// merge filter conditions
for k, v := range src {
indexKey, isIndexKey := k.(*mapper.PropertyIndex)
if !isIndexKey {
continue
}
removeConditionIndex(indexKey, dest)
dest[k] = v
}
return dest
}

func removeConditionIndex(
key *mapper.PropertyIndex,
filterConditions map[connor.FilterKey]any,
Expand Down
4 changes: 2 additions & 2 deletions planner/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,10 +118,10 @@ func (n *updateNode) simpleExplain() (map[string]any, error) {
simpleExplainMap[idsLabel] = n.ids

// Add the filter attribute if it exists, otherwise have it nil.
if n.filter == nil || n.filter.ExternalConditions == nil {
if n.filter == nil {
simpleExplainMap[filterLabel] = nil
} else {
simpleExplainMap[filterLabel] = n.filter.ExternalConditions
simpleExplainMap[filterLabel] = n.filter.ToMap(n.documentMapping)
}

// Add the attribute that represents the patch to update with.
Expand Down
Loading