Skip to content

Commit

Permalink
feat: Add alias to groupBy related object (#1579)
Browse files Browse the repository at this point in the history
Resolves #1578
Resolves #1551
Part-of: #1279

## Description
- Add some tests of existing object_id functionality.
- Add implementation for groupBy alias.
- Add tests for groupBy alias ability.
- Fix the previous panic using alias, with more user friendly error.
- Add tests for panic fix.
- Make a const for `_id`.
- Schema Test to ensure the field we want to alias exists (even though
it actually existed from before)
  • Loading branch information
shahzadlone authored Jun 20, 2023
1 parent be619fd commit e082fe1
Show file tree
Hide file tree
Showing 11 changed files with 1,682 additions and 7 deletions.
4 changes: 4 additions & 0 deletions client/request/consts.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ const (
// https://spec.graphql.org/October2021/#sec-Type-Name-Introspection
TypeNameFieldName = "__typename"

// This is appended to the related object name to give us the field name
// that corresponds to the related object's join relation id, i.e. `Author_id`.
RelatedObjectID = "_id"

Cid = "cid"
Data = "data"
DocKey = "dockey"
Expand Down
6 changes: 5 additions & 1 deletion client/request/select.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,17 @@ func (s *Select) validateGroupBy() []error {
}

var fieldExistsInGroupBy bool
var isAliasFieldInGroupBy bool
for _, groupByField := range s.GroupBy.Value().Fields {
if typedChildSelection.Name == groupByField {
fieldExistsInGroupBy = true
break
} else if typedChildSelection.Name == groupByField+RelatedObjectID {
isAliasFieldInGroupBy = true
break
}
}
if !fieldExistsInGroupBy {
if !fieldExistsInGroupBy && !isAliasFieldInGroupBy {
result = append(result, client.NewErrSelectOfNonGroupField(typedChildSelection.Name))
}
default:
Expand Down
6 changes: 4 additions & 2 deletions db/collection_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,9 @@ func (c *collection) isSecondaryIDField(fieldDesc client.FieldDescription) (clie
return client.FieldDescription{}, false
}

relationFieldDescription, valid := c.Description().GetField(strings.TrimSuffix(fieldDesc.Name, "_id"))
relationFieldDescription, valid := c.Description().GetField(
strings.TrimSuffix(fieldDesc.Name, request.RelatedObjectID),
)
return relationFieldDescription, valid && !relationFieldDescription.IsPrimaryRelation()
}

Expand Down Expand Up @@ -431,7 +433,7 @@ func (c *collection) patchPrimaryDoc(
_, err = primaryCol.UpdateWithKey(
ctx,
primaryDockey,
fmt.Sprintf(`{"%s": "%s"}`, primaryField.Name+"_id", docKey),
fmt.Sprintf(`{"%s": "%s"}`, primaryField.Name+request.RelatedObjectID, docKey),
)
if err != nil {
return err
Expand Down
8 changes: 8 additions & 0 deletions planner/mapper/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,16 @@ package mapper

import "github.com/sourcenetwork/defradb/errors"

const (
errInvalidFieldToGroupBy string = "invalid field value to groupBy"
)

var (
ErrUnableToIdAggregateChild = errors.New("unable to identify aggregate child")
ErrAggregateTargetMissing = errors.New("aggregate must be provided with a property to aggregate")
ErrFailedToFindHostField = errors.New("failed to find host field")
)

func NewErrInvalidFieldToGroupBy(field string) error {
return errors.New(errInvalidFieldToGroupBy, errors.NewKV("Field", field))
}
25 changes: 24 additions & 1 deletion planner/mapper/mapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,31 @@ func toSelect(
return nil, err
}

// If there is a groupBy, and no inner group has been requested, we need to map the property here
// Resolve groupBy mappings i.e. alias remapping and handle missed inner group.
if selectRequest.GroupBy.HasValue() {
groupByFields := selectRequest.GroupBy.Value().Fields
// Remap all alias field names to use their internal field name mappings.
for index, groupByField := range groupByFields {
if _, fieldHasMapping := mapping.IndexesByName[groupByField]; fieldHasMapping {
// Not an alias as is already mapped.
continue
} else if _, isAlias := mapping.IndexesByName[groupByField+request.RelatedObjectID]; isAlias {
// Remap the alias to it's actual internal name.
groupByFields[index] = groupByField + request.RelatedObjectID
} else {
// Field is not mapped nor is an alias, then is invalid field to group on. This can be
// incase of when an alias might have been used on groupBy relation from the single side.
return nil, NewErrInvalidFieldToGroupBy(groupByField)
}
}

selectRequest.GroupBy = immutable.Some(
request.GroupBy{
Fields: groupByFields,
},
)

// If there is a groupBy, and no inner group has been requested, we need to map the property here
if _, isGroupFieldMapped := mapping.IndexesByName[request.GroupFieldName]; !isGroupFieldMapped {
index := mapping.GetNextIndex()
mapping.Add(index, request.GroupFieldName)
Expand Down
6 changes: 3 additions & 3 deletions planner/type_join.go
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ func (n *typeJoinOne) Next() (bool, error) {

func (n *typeJoinOne) valuesSecondary(doc core.Doc) core.Doc {
fkIndex := &mapper.PropertyIndex{
Index: n.subType.DocumentMap().FirstIndexOfName(n.subTypeFieldName + "_id"),
Index: n.subType.DocumentMap().FirstIndexOfName(n.subTypeFieldName + request.RelatedObjectID),
}
filter := map[connor.FilterKey]any{
fkIndex: doc.GetKey(),
Expand Down Expand Up @@ -386,7 +386,7 @@ func (n *typeJoinOne) valuesSecondary(doc core.Doc) core.Doc {

func (n *typeJoinOne) valuesPrimary(doc core.Doc) core.Doc {
// get the subtype doc key
subDocKey := n.docMapper.documentMapping.FirstOfName(doc, n.subTypeName+"_id")
subDocKey := n.docMapper.documentMapping.FirstOfName(doc, n.subTypeName+request.RelatedObjectID)

subDocKeyStr, ok := subDocKey.(string)
if !ok {
Expand Down Expand Up @@ -546,7 +546,7 @@ func (n *typeJoinMany) Next() (bool, error) {
// @todo: handle index for one-to-many setup
} else {
fkIndex := &mapper.PropertyIndex{
Index: n.subSelect.FirstIndexOfName(n.rootName + "_id"),
Index: n.subSelect.FirstIndexOfName(n.rootName + request.RelatedObjectID),
}
filter := map[connor.FilterKey]any{
fkIndex: n.currentValue.GetKey(), // user_id: "bae-ALICE" | user_id: "bae-CHARLIE"
Expand Down
Loading

0 comments on commit e082fe1

Please sign in to comment.