diff --git a/internal/planner/mapper/mapper.go b/internal/planner/mapper/mapper.go index 54decbd7cc..80247a7bf2 100644 --- a/internal/planner/mapper/mapper.go +++ b/internal/planner/mapper/mapper.go @@ -210,31 +210,12 @@ func toSelect( } } - // 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 { - fieldDesc, ok := definition.GetFieldByName(groupByField) - if ok && fieldDesc.Kind.IsObject() { - if fieldDesc.Kind.IsArray() { - return nil, NewErrInvalidFieldToGroupBy(groupByField) - } - groupByFields[index] = request.ToFieldID(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) - } + // Resolve groupBy dependencies: alias/relation remapping, map the missed inner + // group field, and ensure groupBy fields are fetched even when not part of the + // selection set. + fields, err = resolveGroupByDependencies(definition, selectRequest, mapping, fields) + if err != nil { + return nil, err } targetable, err := toTargetable(thisIndex, selectRequest, mapping) @@ -391,6 +372,78 @@ func resolveChildOrder( return nil, ErrMissingSelect } +// resolveGroupByDependencies remaps the groupBy fields to their internal field names, +// maps the synthetic GROUP field if no inner group was requested, and ensures that +// every groupBy field is fetched even when it is not part of the selection set. +// +// This mirrors how [resolveOrderDependencies] adds order fields that were missed due +// to not being rendered. +func resolveGroupByDependencies( + definition client.CollectionVersion, + selectRequest *request.Select, + mapping *core.DocumentMapping, + fields []Requestable, +) ([]Requestable, error) { + if !selectRequest.GroupBy.HasValue() { + return fields, nil + } + + groupByFields := selectRequest.GroupBy.Value().Fields + // Remap all object (relation) field names to use their internal foreign-key field + // id, as that is the scalar value actually stored on - and fetched from - the document. + for index, groupByField := range groupByFields { + fieldDesc, ok := definition.GetFieldByName(groupByField) + if ok && fieldDesc.Kind.IsObject() { + if fieldDesc.Kind.IsArray() { + return nil, NewErrInvalidFieldToGroupBy(groupByField) + } + groupByFields[index] = request.ToFieldID(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) + } + + // Ensure every groupBy field is fetched, even when it is not part of the selection + // set. The field is added as a hidden dependency (it is given no render key) so that + // its value is available for group-key computation without being returned in the response. + for _, groupByField := range groupByFields { + alreadyRequested := false + for _, existingField := range fields { + if existingField.GetName() == groupByField { + alreadyRequested = true + break + } + } + if alreadyRequested { + continue + } + + fieldIndexes := mapping.IndexesByName[groupByField] + if len(fieldIndexes) == 0 { + // Should be unreachable for a valid groupBy field as all base fields are + // mapped by getTopLevelInfo, but guard against an out-of-range panic. + continue + } + + fields = append(fields, &Field{ + Index: fieldIndexes[0], + Name: groupByField, + }) + } + + return fields, nil +} + // resolveAggregates figures out which fields the given aggregates are targeting // and converts the aggregateRequest into an Aggregate, appending it onto the given // fields slice. diff --git a/tests/integration/query/one_to_many/with_group_unselected_field_test.go b/tests/integration/query/one_to_many/with_group_unselected_field_test.go new file mode 100644 index 0000000000..e74c2487ae --- /dev/null +++ b/tests/integration/query/one_to_many/with_group_unselected_field_test.go @@ -0,0 +1,181 @@ +// Copyright 2026 Democratized Data Foundation +// +// This file is part of the DefraDB test suite. +// +// The DefraDB test suite is licensed under either: +// +// (1) GNU Affero General Public License v3 +// (2) Business Source License 1.1 +// +// See tests/LICENSE for details. + +package one_to_many + +import ( + "testing" + + "github.com/sourcenetwork/defradb/tests/action" + testUtils "github.com/sourcenetwork/defradb/tests/integration" +) + +var booksByThreeAuthors = []any{ + &action.AddDoc{ + CollectionID: 0, + Doc: `{ + "name": "Painted House", + "rating": 4.9, + "_authorID": "bae-9d52c335-c8e3-5782-8daa-e359c106e0ab" + }`, + }, + &action.AddDoc{ + CollectionID: 0, + Doc: `{ + "name": "A Time for Mercy", + "rating": 4.5, + "_authorID": "bae-9d52c335-c8e3-5782-8daa-e359c106e0ab" + }`, + }, + &action.AddDoc{ + CollectionID: 0, + Doc: `{ + "name": "Candide", + "rating": 4.95, + "_authorID": "bae-b9c6cd5a-a931-5984-994d-7c435baa9f32" + }`, + }, + &action.AddDoc{ + CollectionID: 0, + Doc: `{ + "name": "Zadig", + "rating": 4.91, + "_authorID": "bae-b9c6cd5a-a931-5984-994d-7c435baa9f32" + }`, + }, + &action.AddDoc{ + CollectionID: 1, + Doc: `{ + "name": "John Grisham", + "age": 65, + "verified": true + }`, + }, + &action.AddDoc{ + CollectionID: 1, + Doc: `{ + "name": "Voltaire", + "age": 327, + "verified": true + }`, + }, +} + +func TestQueryOneToMany_WithGroupByRelatedIDWithoutRenderedGroupField(t *testing.T) { + test := testUtils.TestCase{ + Actions: append( + append([]any{}, booksByThreeAuthors...), + &action.Request{ + Request: `query { + Book(groupBy: [_authorID]) { + GROUP { + name + } + } + }`, + Results: map[string]any{ + "Book": []map[string]any{ + { + "GROUP": []map[string]any{ + {"name": "Painted House"}, + {"name": "A Time for Mercy"}, + }, + }, + { + "GROUP": []map[string]any{ + {"name": "Candide"}, + {"name": "Zadig"}, + }, + }, + }, + }, + NonOrderedResults: true, + }, + ), + } + + executeTestCase(t, test) +} + +func TestQueryOneToMany_WithGroupByRelationObjectRenderingRelatedID(t *testing.T) { + test := testUtils.TestCase{ + Actions: append( + append([]any{}, booksByThreeAuthors...), + &action.Request{ + Request: `query { + Book(groupBy: [author]) { + _authorID + GROUP { + name + } + } + }`, + Results: map[string]any{ + "Book": []map[string]any{ + { + "_authorID": "bae-9d52c335-c8e3-5782-8daa-e359c106e0ab", + "GROUP": []map[string]any{ + {"name": "Painted House"}, + {"name": "A Time for Mercy"}, + }, + }, + { + "_authorID": "bae-b9c6cd5a-a931-5984-994d-7c435baa9f32", + "GROUP": []map[string]any{ + {"name": "Candide"}, + {"name": "Zadig"}, + }, + }, + }, + }, + NonOrderedResults: true, + }, + ), + } + + executeTestCase(t, test) +} + +func TestQueryOneToMany_WithGroupByRelationObjectWithoutRenderedGroupField(t *testing.T) { + test := testUtils.TestCase{ + Actions: append( + append([]any{}, booksByThreeAuthors...), + &action.Request{ + Request: `query { + Book(groupBy: [author]) { + GROUP { + name + } + } + }`, + Results: map[string]any{ + "Book": []map[string]any{ + { + "GROUP": []map[string]any{ + {"name": "Painted House"}, + {"name": "A Time for Mercy"}, + }, + }, + { + "GROUP": []map[string]any{ + {"name": "Candide"}, + {"name": "Zadig"}, + }, + }, + }, + }, + NonOrderedResults: true, + }, + ), + } + + executeTestCase(t, test) +} diff --git a/tests/integration/query/simple/with_group_unselected_field_test.go b/tests/integration/query/simple/with_group_unselected_field_test.go new file mode 100644 index 0000000000..ce423d8c3e --- /dev/null +++ b/tests/integration/query/simple/with_group_unselected_field_test.go @@ -0,0 +1,134 @@ +// Copyright 2026 Democratized Data Foundation +// +// This file is part of the DefraDB test suite. +// +// The DefraDB test suite is licensed under either: +// +// (1) GNU Affero General Public License v3 +// (2) Business Source License 1.1 +// +// See tests/LICENSE for details. + +package simple + +import ( + "testing" + + "github.com/sourcenetwork/defradb/tests/action" + testUtils "github.com/sourcenetwork/defradb/tests/integration" +) + +func TestQuerySimple_WithGroupByNumberWithoutRenderedGroupField(t *testing.T) { + test := testUtils.TestCase{ + Actions: []any{ + &action.AddDoc{ + Doc: `{ + "Name": "John", + "Age": 32 + }`, + }, + &action.AddDoc{ + Doc: `{ + "Name": "Bob", + "Age": 32 + }`, + }, + &action.AddDoc{ + Doc: `{ + "Name": "Carlo", + "Age": 55 + }`, + }, + &action.AddDoc{ + Doc: `{ + "Name": "Alice", + "Age": 19 + }`, + }, + &action.Request{ + Request: `query { + Users(groupBy: [Age]) { + GROUP { + Name + } + } + }`, + Results: map[string]any{ + "Users": []map[string]any{ + { + "GROUP": []map[string]any{ + {"Name": "John"}, + {"Name": "Bob"}, + }, + }, + { + "GROUP": []map[string]any{ + {"Name": "Carlo"}, + }, + }, + { + "GROUP": []map[string]any{ + {"Name": "Alice"}, + }, + }, + }, + }, + NonOrderedResults: true, + }, + }, + } + + executeTestCase(t, test) +} + +func TestQuerySimple_WithGroupByStringWithoutRenderedGroupField(t *testing.T) { + test := testUtils.TestCase{ + Actions: []any{ + &action.AddDoc{ + Doc: `{ + "Name": "John", + "Email": "fiction@example.com" + }`, + }, + &action.AddDoc{ + Doc: `{ + "Name": "Bob", + "Email": "fiction@example.com" + }`, + }, + &action.AddDoc{ + Doc: `{ + "Name": "Carlo", + "Email": "nonfiction@example.com" + }`, + }, + &action.Request{ + Request: `query { + Users(groupBy: [Email]) { + GROUP { + Name + } + } + }`, + Results: map[string]any{ + "Users": []map[string]any{ + { + "GROUP": []map[string]any{ + {"Name": "John"}, + {"Name": "Bob"}, + }, + }, + { + "GROUP": []map[string]any{ + {"Name": "Carlo"}, + }, + }, + }, + }, + NonOrderedResults: true, + }, + }, + } + + executeTestCase(t, test) +}