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

feat: Order alias target #3217

Merged
merged 4 commits into from
Nov 11, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
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 @@
}
}

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 @@
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 @@
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
}

Check warning on line 420 in internal/planner/mapper/mapper.go

View check run for this annotation

Codecov / codecov/patch

internal/planner/mapper/mapper.go#L419-L420

Added lines #L419 - L420 were not covered by tests
host, hasHost = tryGetTarget(
target.hostExternalName,
convertedFilter,
target.limit,
toOrderBy(target.order, childMapping),
orderBy,
fields,
)
}
Expand Down Expand Up @@ -479,7 +492,10 @@
// 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
}

Check warning on line 498 in internal/planner/mapper/mapper.go

View check run for this annotation

Codecov / codecov/patch

internal/planner/mapper/mapper.go#L497-L498

Added lines #L497 - L498 were not covered by tests
dummyJoin := &Select{
Targetable: Targetable{
Field: Field{
Expand All @@ -488,7 +504,7 @@
},
Filter: convertedFilter,
Limit: target.limit,
OrderBy: toOrderBy(target.order, childMapping),
OrderBy: orderBy,
},
CollectionName: childCollectionName,
DocumentMapping: childMapping,
Expand Down Expand Up @@ -992,9 +1008,9 @@
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 @@
}, 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 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 @@

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 @@
fieldName = name
}
switch t := arg[fieldName].(type) {
case string:
if fieldName == request.AliasFieldName {
return nil, ErrInvalidOrderInput
}

Check warning on line 50 in internal/request/graphql/parser/order.go

View check run for this annotation

Codecov / codecov/patch

internal/request/graphql/parser/order.go#L49-L50

Added lines #L49 - L50 were not covered by tests
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
}

Check warning on line 63 in internal/request/graphql/parser/order.go

View check run for this annotation

Codecov / codecov/patch

internal/request/graphql/parser/order.go#L62-L63

Added lines #L62 - L63 were not covered by tests
dir, err := parseOrderDirection(t)
if err != nil {
return nil, err
Expand All @@ -70,13 +86,28 @@
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

Check warning on line 103 in internal/request/graphql/parser/order.go

View check run for this annotation

Codecov / codecov/patch

internal/request/graphql/parser/order.go#L102-L103

Added lines #L102 - L103 were not covered by tests

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
Loading