Skip to content

Commit

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

Resolves sourcenetwork#3196

## Description

This PR adds alias targeting to query orderings.

**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 integration tests.

Specify the platform(s) on which this was tested:
- MacOS
  • Loading branch information
nasdf authored Nov 11, 2024
1 parent a89524a commit dae2820
Show file tree
Hide file tree
Showing 10 changed files with 692 additions and 32 deletions.
1 change: 1 addition & 0 deletions client/request/consts.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ const (
VersionFieldName = "_version"
MaxFieldName = "_max"
MinFieldName = "_min"
AliasFieldName = "_alias"

// New generated document id from a backed up document,
// which might have a different _docID originally.
Expand Down
7 changes: 3 additions & 4 deletions client/request/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,9 @@ package request
import "github.com/sourcenetwork/immutable"

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

// Filter contains the parsed condition map to be
Expand Down
76 changes: 55 additions & 21 deletions internal/planner/mapper/mapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,8 +208,12 @@ func toSelect(
}
}

targetable, err := toTargetable(thisIndex, selectRequest, mapping)
if err != nil {
return nil, err
}
return &Select{
Targetable: toTargetable(thisIndex, selectRequest, mapping),
Targetable: targetable,
DocumentMapping: mapping,
Cid: selectRequest.CID,
CollectionName: collectionName,
Expand Down Expand Up @@ -239,6 +243,11 @@ outer:
for _, condition := range source.Value().Conditions {
fields := condition.Fields[:] // copy slice
for {
// alias fields are guaranteed to be resolved
// because they refer to existing fields
if fields[0] == request.AliasFieldName {
continue outer
}
numFields := len(fields)
// <2 fields: Direct field on the root type: {age: DESC}
// 2 fields: Single depth related type: {author: {age: DESC}}
Expand Down Expand Up @@ -405,11 +414,15 @@ func resolveAggregates(
childObjectIndex := mapping.FirstIndexOfName(target.hostExternalName)
childMapping := mapping.ChildMappings[childObjectIndex]
convertedFilter = ToFilter(target.filter.Value(), childMapping)
orderBy, err := toOrderBy(target.order, childMapping)
if err != nil {
return nil, err
}
host, hasHost = tryGetTarget(
target.hostExternalName,
convertedFilter,
target.limit,
toOrderBy(target.order, childMapping),
orderBy,
fields,
)
}
Expand Down Expand Up @@ -479,7 +492,10 @@ func resolveAggregates(
// If the child was not mapped, the filter will not have been converted yet
// so we must do that now.
convertedFilter = ToFilter(target.filter.Value(), mapping.ChildMappings[index])

orderBy, err := toOrderBy(target.order, childMapping)
if err != nil {
return nil, err
}
dummyJoin := &Select{
Targetable: Targetable{
Field: Field{
Expand All @@ -488,7 +504,7 @@ func resolveAggregates(
},
Filter: convertedFilter,
Limit: target.limit,
OrderBy: toOrderBy(target.order, childMapping),
OrderBy: orderBy,
},
CollectionName: childCollectionName,
DocumentMapping: childMapping,
Expand Down Expand Up @@ -992,9 +1008,9 @@ func resolveInnerFilterDependencies(
newFields := []Requestable{}

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

Expand Down Expand Up @@ -1290,16 +1306,20 @@ func toMutation(
}, nil
}

func toTargetable(index int, selectRequest *request.Select, docMap *core.DocumentMapping) Targetable {
func toTargetable(index int, selectRequest *request.Select, docMap *core.DocumentMapping) (Targetable, error) {
orderBy, err := toOrderBy(selectRequest.OrderBy, docMap)
if err != nil {
return Targetable{}, err
}
return Targetable{
Field: toField(index, selectRequest),
DocIDs: selectRequest.DocIDs,
Filter: ToFilter(selectRequest.Filter.Value(), docMap),
Limit: toLimit(selectRequest.Limit, selectRequest.Offset),
GroupBy: toGroupBy(selectRequest.GroupBy, docMap),
OrderBy: toOrderBy(selectRequest.OrderBy, docMap),
OrderBy: orderBy,
ShowDeleted: selectRequest.ShowDeleted,
}
}, nil
}

func toField(index int, selectRequest *request.Select) Field {
Expand Down Expand Up @@ -1483,23 +1503,37 @@ func toGroupBy(source immutable.Option[request.GroupBy], mapping *core.DocumentM
}
}

func toOrderBy(source immutable.Option[request.OrderBy], mapping *core.DocumentMapping) *OrderBy {
func toOrderBy(source immutable.Option[request.OrderBy], mapping *core.DocumentMapping) (*OrderBy, error) {
if !source.HasValue() {
return nil
return nil, nil
}

conditions := make([]OrderCondition, len(source.Value().Conditions))
for conditionIndex, condition := range source.Value().Conditions {
fieldIndexes := make([]int, len(condition.Fields))
fieldIndexes := make([]int, 0)
currentMapping := mapping
for fieldIndex, field := range condition.Fields {
// 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.
firstFieldIndex := currentMapping.FirstIndexOfName(field)
fieldIndexes[fieldIndex] = firstFieldIndex
if fieldIndex != len(condition.Fields)-1 {
for _, field := range condition.Fields {
// flatten alias fields
if field == request.AliasFieldName {
continue
}
var firstFieldIndex int
// if we have a mapping available check if the
// source key is a field or alias (render key)
if indexes, ok := currentMapping.IndexesByName[field]; 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.
firstFieldIndex = indexes[0]
} else if index, ok := currentMapping.TryToFindIndexFromRenderKey(field); ok {
firstFieldIndex = index
} else {
return nil, NewErrFieldOrAliasNotFound(field)
}

fieldIndexes = append(fieldIndexes, firstFieldIndex)
if firstFieldIndex < len(currentMapping.ChildMappings) {
// no need to do this for the last (and will panic)
currentMapping = currentMapping.ChildMappings[firstFieldIndex]
}
Expand All @@ -1513,7 +1547,7 @@ func toOrderBy(source immutable.Option[request.OrderBy], mapping *core.DocumentM

return &OrderBy{
Conditions: conditions,
}
}, nil
}

// RunFilter runs the given filter expression
Expand Down
2 changes: 1 addition & 1 deletion internal/planner/mapper/targetable.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,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, request.FilterOpAlias:
case request.FilterOpNot, request.AliasFieldName:
itemMap, ok := v.(map[connor.FilterKey]any)
if ok {
outmap[keyType.Operation] = filterObjectToMap(mapping, itemMap)
Expand Down
3 changes: 2 additions & 1 deletion internal/request/graphql/parser/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ import "github.com/sourcenetwork/defradb/errors"

var (
ErrFilterMissingArgumentType = errors.New("couldn't find filter argument type")
ErrInvalidOrderDirection = errors.New("invalid order direction string")
ErrInvalidOrderDirection = errors.New("invalid order direction")
ErrInvalidOrderInput = errors.New("invalid order input")
ErrFailedToParseConditionsFromAST = errors.New("couldn't parse conditions value from AST")
ErrFailedToParseConditionValue = errors.New("failed to parse condition value from query filter statement")
ErrEmptyDataPayload = errors.New("given data payload is empty")
Expand Down
2 changes: 1 addition & 1 deletion internal/request/graphql/parser/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func parseFilterFieldsForDescriptionMap(
return nil, err
}
fields = append(fields, parsedFields...)
case request.FilterOpNot, request.FilterOpAlias:
case request.FilterOpNot, request.AliasFieldName:
conds := v.(map[string]any)
parsedFields, err := parseFilterFieldsForDescriptionMap(conds, col)
if err != nil {
Expand Down
37 changes: 34 additions & 3 deletions internal/request/graphql/parser/order.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,23 @@ func parseOrderCondition(arg map[string]any) (*request.OrderCondition, error) {
fieldName = name
}
switch t := arg[fieldName].(type) {
case string:
if fieldName == request.AliasFieldName {
return nil, ErrInvalidOrderInput
}
dir, err := parseOrderDirectionString(t)
if err != nil {
return nil, err
}
return &request.OrderCondition{
Fields: []string{fieldName},
Direction: dir,
}, nil

case int:
if fieldName == request.AliasFieldName {
return nil, ErrInvalidOrderInput
}
dir, err := parseOrderDirection(t)
if err != nil {
return nil, err
Expand All @@ -70,13 +86,28 @@ func parseOrderCondition(arg map[string]any) (*request.OrderCondition, error) {
cond.Fields = append([]string{fieldName}, cond.Fields...)
return cond, nil

default:
// field value is null so don't include the condition
case nil:
return nil, nil

default:
return nil, ErrInvalidOrderInput
}
}

func parseOrderDirectionString(v string) (request.OrderDirection, error) {
switch v {
case string(request.ASC):
return request.ASC, nil

case string(request.DESC):
return request.DESC, nil

default:
return request.ASC, ErrInvalidOrderDirection
}
}

func parseOrderDirection(v int) (request.OrderDirection, error) {
func parseOrderDirection(v any) (request.OrderDirection, error) {
switch v {
case 0:
return request.ASC, nil
Expand Down
6 changes: 5 additions & 1 deletion internal/request/graphql/schema/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -1200,7 +1200,7 @@ func (g *Generator) genTypeFilterArgInput(obj *gql.Object) *gql.InputObject {
Description: schemaTypes.NotOperatorDescription,
Type: selfRefType,
}
fields[request.FilterOpAlias] = &gql.InputObjectFieldConfig{
fields[request.AliasFieldName] = &gql.InputObjectFieldConfig{
Description: "The alias operator allows filters to target aliased fields.",
Type: schemaTypes.JSONScalarType(),
}
Expand Down Expand Up @@ -1291,6 +1291,10 @@ func (g *Generator) genTypeOrderArgInput(obj *gql.Object) *gql.InputObject {
fieldThunk := (gql.InputObjectConfigFieldMapThunk)(
func() (gql.InputObjectConfigFieldMap, error) {
fields := gql.InputObjectConfigFieldMap{}
fields[request.AliasFieldName] = &gql.InputObjectFieldConfig{
Description: "The alias field allows ordering by aliased fields.",
Type: schemaTypes.JSONScalarType(),
}

for f, field := range obj.Fields() {
if _, ok := request.ReservedFields[f]; ok && f != request.DocIDFieldName {
Expand Down
Loading

0 comments on commit dae2820

Please sign in to comment.