diff --git a/client/request/consts.go b/client/request/consts.go index 18fed52946..ce31c95133 100644 --- a/client/request/consts.go +++ b/client/request/consts.go @@ -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. diff --git a/client/request/filter.go b/client/request/filter.go index feacb02f2b..aabfafb9b9 100644 --- a/client/request/filter.go +++ b/client/request/filter.go @@ -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 diff --git a/internal/planner/mapper/mapper.go b/internal/planner/mapper/mapper.go index 15014fb9f4..826c29ffbc 100644 --- a/internal/planner/mapper/mapper.go +++ b/internal/planner/mapper/mapper.go @@ -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, @@ -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}} @@ -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, ) } @@ -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{ @@ -488,7 +504,7 @@ func resolveAggregates( }, Filter: convertedFilter, Limit: target.limit, - OrderBy: toOrderBy(target.order, childMapping), + OrderBy: orderBy, }, CollectionName: childCollectionName, DocumentMapping: childMapping, @@ -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 } @@ -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 { @@ -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] } @@ -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 diff --git a/internal/planner/mapper/targetable.go b/internal/planner/mapper/targetable.go index 55bc256327..e25c8b03f5 100644 --- a/internal/planner/mapper/targetable.go +++ b/internal/planner/mapper/targetable.go @@ -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) diff --git a/internal/request/graphql/parser/errors.go b/internal/request/graphql/parser/errors.go index 658f50219c..b4692d10e8 100644 --- a/internal/request/graphql/parser/errors.go +++ b/internal/request/graphql/parser/errors.go @@ -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") diff --git a/internal/request/graphql/parser/filter.go b/internal/request/graphql/parser/filter.go index 1995eeb58b..3fa376a0b1 100644 --- a/internal/request/graphql/parser/filter.go +++ b/internal/request/graphql/parser/filter.go @@ -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 { diff --git a/internal/request/graphql/parser/order.go b/internal/request/graphql/parser/order.go index 983988f5f9..ea6da4a08d 100644 --- a/internal/request/graphql/parser/order.go +++ b/internal/request/graphql/parser/order.go @@ -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 @@ -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 diff --git a/internal/request/graphql/schema/generate.go b/internal/request/graphql/schema/generate.go index 608c83e381..1c1bb70e2f 100644 --- a/internal/request/graphql/schema/generate.go +++ b/internal/request/graphql/schema/generate.go @@ -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(), } @@ -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 { diff --git a/tests/integration/query/one_to_one/with_order_test.go b/tests/integration/query/one_to_one/with_order_test.go index 3475ebfc63..e91e803473 100644 --- a/tests/integration/query/one_to_one/with_order_test.go +++ b/tests/integration/query/one_to_one/with_order_test.go @@ -287,3 +287,145 @@ func TestQueryOneToOneWithChildIntOrderAscendingWithNoSubTypeFieldsSelected(t *t executeTestCase(t, test) } + +func TestQueryOneToOne_WithAliasedChildIntOrderAscending_ShouldOrder(t *testing.T) { + test := testUtils.TestCase{ + Description: "Relation query with ascending order by aliased child's int field.", + Actions: []any{ + testUtils.CreateDoc{ + CollectionID: 0, + Doc: `{ + "name": "Painted House", + "rating": 4.9 + }`, + }, + testUtils.CreateDoc{ + CollectionID: 0, + Doc: `{ + "name": "Theif Lord", + "rating": 4.8 + }`, + }, + testUtils.CreateDoc{ + CollectionID: 1, + DocMap: map[string]any{ + "name": "John Grisham", + "age": 65, + "verified": true, + "published_id": testUtils.NewDocIndex(0, 0), + }, + }, + testUtils.CreateDoc{ + CollectionID: 1, + DocMap: map[string]any{ + "name": "Cornelia Funke", + "age": 62, + "verified": false, + "published_id": testUtils.NewDocIndex(0, 1), + }, + }, + testUtils.Request{ + Request: `query { + Book(order: {_alias: {writer: {age: ASC}}}) { + name + rating + writer: author { + age + } + } + }`, + Results: map[string]any{ + "Book": []map[string]any{ + { + "name": "Theif Lord", + "rating": 4.8, + "writer": map[string]any{ + "age": int64(62), + }, + }, + { + "name": "Painted House", + "rating": 4.9, + "writer": map[string]any{ + "age": int64(65), + }, + }, + }, + }, + }, + }, + } + + executeTestCase(t, test) +} + +func TestQueryOneToOne_WithChildAliasedIntOrderAscending_ShouldOrder(t *testing.T) { + test := testUtils.TestCase{ + Description: "Relation query with ascending order by child's aliased int field.", + Actions: []any{ + testUtils.CreateDoc{ + CollectionID: 0, + Doc: `{ + "name": "Painted House", + "rating": 4.9 + }`, + }, + testUtils.CreateDoc{ + CollectionID: 0, + Doc: `{ + "name": "Theif Lord", + "rating": 4.8 + }`, + }, + testUtils.CreateDoc{ + CollectionID: 1, + DocMap: map[string]any{ + "name": "John Grisham", + "age": 65, + "verified": true, + "published_id": testUtils.NewDocIndex(0, 0), + }, + }, + testUtils.CreateDoc{ + CollectionID: 1, + DocMap: map[string]any{ + "name": "Cornelia Funke", + "age": 62, + "verified": false, + "published_id": testUtils.NewDocIndex(0, 1), + }, + }, + testUtils.Request{ + Request: `query { + Book(order: {author: {_alias: {authorAge: ASC}}}) { + name + rating + author { + authorAge: age + } + } + }`, + Results: map[string]any{ + "Book": []map[string]any{ + { + "name": "Theif Lord", + "rating": 4.8, + "author": map[string]any{ + "authorAge": int64(62), + }, + }, + { + "name": "Painted House", + "rating": 4.9, + "author": map[string]any{ + "authorAge": int64(65), + }, + }, + }, + }, + }, + }, + } + + executeTestCase(t, test) +} diff --git a/tests/integration/query/simple/with_order_test.go b/tests/integration/query/simple/with_order_test.go index 82245de369..57f08e4b92 100644 --- a/tests/integration/query/simple/with_order_test.go +++ b/tests/integration/query/simple/with_order_test.go @@ -451,3 +451,451 @@ func TestQuerySimple_WithMultipleOrderFields_ReturnsError(t *testing.T) { executeTestCase(t, test) } } + +func TestQuerySimple_WithAliasOrder_ShouldOrderResults(t *testing.T) { + test := testUtils.TestCase{ + Description: "Simple query with basic alias order ASC", + Actions: []any{ + testUtils.CreateDoc{ + Doc: `{ + "Name": "John", + "Age": 21 + }`, + }, + testUtils.CreateDoc{ + Doc: `{ + "Name": "Bob", + "Age": 32 + }`, + }, + testUtils.CreateDoc{ + Doc: `{ + "Name": "Carlo", + "Age": 55 + }`, + }, + testUtils.CreateDoc{ + Doc: `{ + "Name": "Alice", + "Age": 19 + }`, + }, + testUtils.Request{ + Request: `query { + Users(order: {_alias: {UserAge: ASC}}) { + Name + UserAge: Age + } + }`, + Results: map[string]any{ + "Users": []map[string]any{ + { + "Name": "Alice", + "UserAge": int64(19), + }, + { + "Name": "John", + "UserAge": int64(21), + }, + { + "Name": "Bob", + "UserAge": int64(32), + }, + { + "Name": "Carlo", + "UserAge": int64(55), + }, + }, + }, + }, + }, + } + + executeTestCase(t, test) +} + +func TestQuerySimple_WithAliasOrderOnNonAliasedField_ShouldOrderResults(t *testing.T) { + test := testUtils.TestCase{ + Description: "Simple query with basic alias order on non aliased field ASC", + Actions: []any{ + testUtils.CreateDoc{ + Doc: `{ + "Name": "John", + "Age": 21 + }`, + }, + testUtils.CreateDoc{ + Doc: `{ + "Name": "Bob", + "Age": 32 + }`, + }, + testUtils.CreateDoc{ + Doc: `{ + "Name": "Carlo", + "Age": 55 + }`, + }, + testUtils.CreateDoc{ + Doc: `{ + "Name": "Alice", + "Age": 19 + }`, + }, + testUtils.Request{ + Request: `query { + Users(order: {_alias: {Age: ASC}}) { + Name + Age + } + }`, + Results: map[string]any{ + "Users": []map[string]any{ + { + "Name": "Alice", + "Age": int64(19), + }, + { + "Name": "John", + "Age": int64(21), + }, + { + "Name": "Bob", + "Age": int64(32), + }, + { + "Name": "Carlo", + "Age": int64(55), + }, + }, + }, + }, + }, + } + + executeTestCase(t, test) +} + +func TestQuerySimple_WithAliasOrderOnNonExistantField_ShouldError(t *testing.T) { + test := testUtils.TestCase{ + Description: "Simple query with basic alias order on non existant field ASC", + Actions: []any{ + testUtils.CreateDoc{ + Doc: `{ + "Name": "John", + "Age": 21 + }`, + }, + testUtils.CreateDoc{ + Doc: `{ + "Name": "Bob", + "Age": 32 + }`, + }, + testUtils.CreateDoc{ + Doc: `{ + "Name": "Carlo", + "Age": 55 + }`, + }, + testUtils.CreateDoc{ + Doc: `{ + "Name": "Alice", + "Age": 19 + }`, + }, + testUtils.Request{ + Request: `query { + Users(order: {_alias: {UserAge: ASC}}) { + Name + Age + } + }`, + ExpectedError: `field or alias not found. Name: UserAge`, + }, + }, + } + + executeTestCase(t, test) +} + +func TestQuerySimple_WithInvalidAliasOrder_ShouldError(t *testing.T) { + test := testUtils.TestCase{ + Description: "Simple query with basic alias order invalid", + Actions: []any{ + testUtils.CreateDoc{ + Doc: `{ + "Name": "John", + "Age": 21 + }`, + }, + testUtils.CreateDoc{ + Doc: `{ + "Name": "Bob", + "Age": 32 + }`, + }, + testUtils.CreateDoc{ + Doc: `{ + "Name": "Carlo", + "Age": 55 + }`, + }, + testUtils.CreateDoc{ + Doc: `{ + "Name": "Alice", + "Age": 19 + }`, + }, + testUtils.Request{ + Request: `query { + Users(order: {_alias: {UserAge: invalid}}) { + Name + UserAge: Age + } + }`, + ExpectedError: `invalid order direction`, + }, + }, + } + + executeTestCase(t, test) +} + +func TestQuerySimple_WithEmptyAliasOrder_ShouldDoNothing(t *testing.T) { + test := testUtils.TestCase{ + Description: "Simple query with basic alias order empty", + Actions: []any{ + testUtils.CreateDoc{ + Doc: `{ + "Name": "John", + "Age": 21 + }`, + }, + testUtils.CreateDoc{ + Doc: `{ + "Name": "Bob", + "Age": 32 + }`, + }, + testUtils.CreateDoc{ + Doc: `{ + "Name": "Carlo", + "Age": 55 + }`, + }, + testUtils.CreateDoc{ + Doc: `{ + "Name": "Alice", + "Age": 19 + }`, + }, + testUtils.Request{ + Request: `query { + Users(order: {_alias: {}}) { + Name + Age + } + }`, + Results: map[string]any{ + "Users": []map[string]any{ + { + "Name": "Carlo", + "Age": int64(55), + }, + { + "Name": "Bob", + "Age": int64(32), + }, + { + "Name": "John", + "Age": int64(21), + }, + { + "Name": "Alice", + "Age": int64(19), + }, + }, + }, + }, + }, + } + + executeTestCase(t, test) +} + +func TestQuerySimple_WithNullAliasOrder_ShouldDoNothing(t *testing.T) { + test := testUtils.TestCase{ + Description: "Simple query with basic alias order null", + Actions: []any{ + testUtils.CreateDoc{ + Doc: `{ + "Name": "John", + "Age": 21 + }`, + }, + testUtils.CreateDoc{ + Doc: `{ + "Name": "Bob", + "Age": 32 + }`, + }, + testUtils.CreateDoc{ + Doc: `{ + "Name": "Carlo", + "Age": 55 + }`, + }, + testUtils.CreateDoc{ + Doc: `{ + "Name": "Alice", + "Age": 19 + }`, + }, + testUtils.Request{ + Request: `query { + Users(order: {_alias: null}) { + Name + Age + } + }`, + Results: map[string]any{ + "Users": []map[string]any{ + { + "Name": "Carlo", + "Age": int64(55), + }, + { + "Name": "Bob", + "Age": int64(32), + }, + { + "Name": "John", + "Age": int64(21), + }, + { + "Name": "Alice", + "Age": int64(19), + }, + }, + }, + }, + }, + } + + executeTestCase(t, test) +} + +func TestQuerySimple_WithIntAliasOrder_ShouldError(t *testing.T) { + test := testUtils.TestCase{ + Description: "Simple query with basic alias order empty", + Actions: []any{ + testUtils.CreateDoc{ + Doc: `{ + "Name": "John", + "Age": 21 + }`, + }, + testUtils.CreateDoc{ + Doc: `{ + "Name": "Bob", + "Age": 32 + }`, + }, + testUtils.CreateDoc{ + Doc: `{ + "Name": "Carlo", + "Age": 55 + }`, + }, + testUtils.CreateDoc{ + Doc: `{ + "Name": "Alice", + "Age": 19 + }`, + }, + testUtils.Request{ + Request: `query { + Users(order: {_alias: 1}) { + Name + Age + } + }`, + ExpectedError: `invalid order input`, + }, + }, + } + + executeTestCase(t, test) +} + +func TestQuerySimple_WithCompoundAliasOrder_ShouldOrderResults(t *testing.T) { + test := testUtils.TestCase{ + Description: "Simple query with compound alias order", + Actions: []any{ + testUtils.CreateDoc{ + Doc: `{ + "Name": "John", + "Age": 21, + "Verified": true + }`, + }, + testUtils.CreateDoc{ + Doc: `{ + "Name": "Bob", + "Age": 21, + "Verified": false + }`, + }, + testUtils.CreateDoc{ + Doc: `{ + "Name": "Carlo", + "Age": 55, + "Verified": true + }`, + }, + testUtils.CreateDoc{ + Doc: `{ + "Name": "Alice", + "Age": 19, + "Verified": false + }`, + }, + testUtils.Request{ + Request: `query { + Users(order: [{_alias: {userAge: DESC}}, {_alias: {isVerified: ASC}}]) { + Name + userAge: Age + isVerified: Verified + } + }`, + Results: map[string]any{ + "Users": []map[string]any{ + { + "Name": "Carlo", + "userAge": int64(55), + "isVerified": true, + }, + { + "Name": "Bob", + "userAge": int64(21), + "isVerified": false, + }, + { + "Name": "John", + "userAge": int64(21), + "isVerified": true, + }, + { + "Name": "Alice", + "userAge": int64(19), + "isVerified": false, + }, + }, + }, + }, + }, + } + + executeTestCase(t, test) +}