diff --git a/planner/mapper/mapper.go b/planner/mapper/mapper.go index 9cf83c977c..b293eb2a76 100644 --- a/planner/mapper/mapper.go +++ b/planner/mapper/mapper.go @@ -93,6 +93,11 @@ func toSelect( return nil, err } + fields, err = resolveSecondaryRelationIDs(descriptionsRepo, desc, mapping, fields) + if err != nil { + return nil, err + } + // Resolve groupBy mappings i.e. alias remapping and handle missed inner group. if selectRequest.GroupBy.HasValue() { groupByFields := selectRequest.GroupBy.Value().Fields @@ -941,6 +946,91 @@ func constructDummyJoin( }, nil } +// resolveSecondaryRelationIDs contructs the required stuff needed to resolve secondary relation ids. +// +// They are handled by joining (if not already done so) the related object and copying its key into the +// secondary relation id field. +// +// They copying itself is handled within [typeJoinOne]. +func resolveSecondaryRelationIDs( + descriptionsRepo *DescriptionsRepo, + desc *client.CollectionDescription, + mapping *core.DocumentMapping, + requestables []Requestable, +) ([]Requestable, error) { + fields := requestables + + for _, requestable := range requestables { + existingField, isField := requestable.(*Field) + if !isField { + continue + } + + fieldDesc, descFound := desc.Schema.GetField(existingField.Name) + if !descFound { + continue + } + + if !fieldDesc.RelationType.IsSet(client.Relation_Type_INTERNAL_ID) { + continue + } + + objectFieldDesc, descFound := desc.Schema.GetField( + strings.TrimSuffix(existingField.Name, request.RelatedObjectID), + ) + if !descFound { + continue + } + + if objectFieldDesc.RelationName == "" { + continue + } + + var siblingFound bool + for _, siblingRequestable := range requestables { + siblingSelect, isSelect := siblingRequestable.(*Select) + if !isSelect { + continue + } + + siblingFieldDesc, descFound := desc.Schema.GetField(siblingSelect.Field.Name) + if !descFound { + continue + } + + if siblingFieldDesc.RelationName != objectFieldDesc.RelationName { + continue + } + + if siblingFieldDesc.Kind != client.FieldKind_FOREIGN_OBJECT { + continue + } + + siblingFound = true + break + } + + if !siblingFound { + objectFieldName := strings.TrimSuffix(existingField.Name, request.RelatedObjectID) + + // We only require the dockey of the related object, so an empty join is all we need. + dummyJoin, err := constructDummyJoin( + descriptionsRepo, + desc.Name, + mapping, + objectFieldName, + ) + if err != nil { + return nil, err + } + + fields = append(fields, dummyJoin) + } + } + + return fields, nil +} + // ToCommitSelect converts the given [request.CommitSelect] into a [CommitSelect]. // // In the process of doing so it will construct the document map required to access the data diff --git a/planner/type_join.go b/planner/type_join.go index d79cf64af3..f0dd2c6d19 100644 --- a/planner/type_join.go +++ b/planner/type_join.go @@ -11,6 +11,8 @@ package planner import ( + "github.com/sourcenetwork/immutable" + "github.com/sourcenetwork/defradb/client" "github.com/sourcenetwork/defradb/client/request" "github.com/sourcenetwork/defradb/connor" @@ -255,7 +257,8 @@ type typeJoinOne struct { subTypeName string subTypeFieldName string - primary bool + primary bool + secondaryFieldIndex immutable.Option[int] spans core.Spans subSelect *mapper.Select @@ -305,15 +308,24 @@ func (p *Planner) makeTypeJoinOne( return nil, client.NewErrFieldNotExist(subTypeFieldDesc.RelationName) } + var secondaryFieldIndex immutable.Option[int] + if !isPrimary { + idFieldName := subTypeFieldDesc.Name + request.RelatedObjectID + secondaryFieldIndex = immutable.Some( + parent.documentMapping.FirstIndexOfName(idFieldName), + ) + } + return &typeJoinOne{ - p: p, - root: source, - subSelect: subType, - subTypeName: subType.Name, - subTypeFieldName: subTypeField.Name, - subType: selectPlan, - primary: isPrimary, - docMapper: docMapper{parent.documentMapping}, + p: p, + root: source, + subSelect: subType, + subTypeName: subType.Name, + subTypeFieldName: subTypeField.Name, + subType: selectPlan, + primary: isPrimary, + secondaryFieldIndex: secondaryFieldIndex, + docMapper: docMapper{parent.documentMapping}, }, nil } @@ -387,6 +399,11 @@ func (n *typeJoinOne) valuesSecondary(doc core.Doc) (core.Doc, error) { subdoc := n.subType.Value() doc.Fields[n.subSelect.Index] = subdoc + + if n.secondaryFieldIndex.HasValue() { + doc.Fields[n.secondaryFieldIndex.Value()] = subdoc.GetKey() + } + return doc, nil } diff --git a/tests/integration/query/one_to_one/simple_test.go b/tests/integration/query/one_to_one/simple_test.go index 80de564469..1fcefa0606 100644 --- a/tests/integration/query/one_to_one/simple_test.go +++ b/tests/integration/query/one_to_one/simple_test.go @@ -374,9 +374,8 @@ func TestQueryOneToOne_WithRelationIDFromSecondarySide(t *testing.T) { }`, Results: []map[string]any{ { - "name": "Painted House", - // This is undesirable and needs to change within this PR - "author_id": nil, + "name": "Painted House", + "author_id": "bae-6b624301-3d0a-5336-bd2c-ca00bca3de85", }, }, }, diff --git a/tests/integration/query/one_to_one/with_clashing_id_field_test.go b/tests/integration/query/one_to_one/with_clashing_id_field_test.go index 04f2964dde..11d1c04faf 100644 --- a/tests/integration/query/one_to_one/with_clashing_id_field_test.go +++ b/tests/integration/query/one_to_one/with_clashing_id_field_test.go @@ -16,6 +16,7 @@ import ( testUtils "github.com/sourcenetwork/defradb/tests/integration" ) +// This documents unwanted behaviour, see https://github.com/sourcenetwork/defradb/issues/1520 func TestQueryOneToOneWithClashingIdFieldOnSecondary(t *testing.T) { test := testUtils.TestCase{ Description: "One-to-one relation secondary direction, id field with name clash on secondary side", @@ -62,7 +63,7 @@ func TestQueryOneToOneWithClashingIdFieldOnSecondary(t *testing.T) { Results: []map[string]any{ { "name": "Painted House", - "author_id": uint64(123456), + "author_id": "bae-9d67a886-64e3-520b-8cd5-1ca7b098fabe", "author": map[string]any{ "name": "John Grisham", }, diff --git a/tests/integration/query/one_to_one/with_group_related_id_alias_test.go b/tests/integration/query/one_to_one/with_group_related_id_alias_test.go index cbe75c9318..eb53ab5d47 100644 --- a/tests/integration/query/one_to_one/with_group_related_id_alias_test.go +++ b/tests/integration/query/one_to_one/with_group_related_id_alias_test.go @@ -26,7 +26,7 @@ func TestQueryOneToOneWithGroupRelatedIDAlias(t *testing.T) { name: String author: Author @primary } - + type Author { name: String published: Book @@ -106,9 +106,233 @@ func TestQueryOneToOneWithGroupRelatedIDAlias(t *testing.T) { testUtils.ExecuteTestCase(t, test) } -// This test documents unwanted behaviour, see: -// https://github.com/sourcenetwork/defradb/issues/1654 -func TestQueryOneToOneWithGroupRelatedIDAliasFromSecondary(t *testing.T) { +func TestQueryOneToOneWithGroupRelatedIDAliasFromSecondaryWithoutInnerGroup(t *testing.T) { + test := testUtils.TestCase{ + Description: "One-to-one relation query with group by related id alias (secondary side)", + Actions: []any{ + testUtils.SchemaUpdate{ + Schema: ` + type Book { + name: String + author: Author + } + + type Author { + name: String + published: Book @primary + } + `, + }, + testUtils.CreateDoc{ + CollectionID: 0, + // bae-3d236f89-6a31-5add-a36a-27971a2eac76 + Doc: `{ + "name": "Painted House" + }`, + }, + testUtils.CreateDoc{ + CollectionID: 0, + // bae-d6627fea-8bf7-511c-bcf9-bac4212bddd6 + Doc: `{ + "name": "Go Guide for Rust developers" + }`, + }, + testUtils.CreateDoc{ + CollectionID: 1, + // bae-6b624301-3d0a-5336-bd2c-ca00bca3de85 + Doc: `{ + "name": "John Grisham", + "published_id": "bae-3d236f89-6a31-5add-a36a-27971a2eac76" + }`, + }, + testUtils.CreateDoc{ + CollectionID: 1, + // bae-92fa9dcb-c1ee-5b84-b2f6-e9437c7f261c + Doc: `{ + "name": "Andrew Lone", + "published_id": "bae-d6627fea-8bf7-511c-bcf9-bac4212bddd6" + }`, + }, + testUtils.Request{ + Request: `query { + Book(groupBy: [author]) { + author_id + } + }`, + Results: []map[string]any{ + { + "author_id": "bae-6b624301-3d0a-5336-bd2c-ca00bca3de85", + }, + { + "author_id": "bae-92fa9dcb-c1ee-5b84-b2f6-e9437c7f261c", + }, + }, + }, + }, + } + + testUtils.ExecuteTestCase(t, test) +} + +func TestQueryOneToOneWithGroupRelatedIDAliasFromSecondaryWithoutInnerGroupWithJoin(t *testing.T) { + test := testUtils.TestCase{ + Description: "One-to-one relation query with group by related id alias (secondary side)", + Actions: []any{ + testUtils.SchemaUpdate{ + Schema: ` + type Book { + name: String + author: Author + } + + type Author { + name: String + published: Book @primary + } + `, + }, + testUtils.CreateDoc{ + CollectionID: 0, + // bae-3d236f89-6a31-5add-a36a-27971a2eac76 + Doc: `{ + "name": "Painted House" + }`, + }, + testUtils.CreateDoc{ + CollectionID: 0, + // bae-d6627fea-8bf7-511c-bcf9-bac4212bddd6 + Doc: `{ + "name": "Go Guide for Rust developers" + }`, + }, + testUtils.CreateDoc{ + CollectionID: 1, + // bae-6b624301-3d0a-5336-bd2c-ca00bca3de85 + Doc: `{ + "name": "John Grisham", + "published_id": "bae-3d236f89-6a31-5add-a36a-27971a2eac76" + }`, + }, + testUtils.CreateDoc{ + CollectionID: 1, + // bae-92fa9dcb-c1ee-5b84-b2f6-e9437c7f261c + Doc: `{ + "name": "Andrew Lone", + "published_id": "bae-d6627fea-8bf7-511c-bcf9-bac4212bddd6" + }`, + }, + testUtils.Request{ + Request: `query { + Book(groupBy: [author]) { + author_id + author { + name + } + } + }`, + Results: []map[string]any{ + { + "author_id": "bae-6b624301-3d0a-5336-bd2c-ca00bca3de85", + "author": map[string]any{ + "name": "John Grisham", + }, + }, + { + "author_id": "bae-92fa9dcb-c1ee-5b84-b2f6-e9437c7f261c", + "author": map[string]any{ + "name": "Andrew Lone", + }, + }, + }, + }, + }, + } + + testUtils.ExecuteTestCase(t, test) +} + +func TestQueryOneToOneWithGroupRelatedIDAliasFromSecondaryWithInnerGroup(t *testing.T) { + test := testUtils.TestCase{ + Description: "One-to-one relation query with group by related id alias (secondary side)", + Actions: []any{ + testUtils.SchemaUpdate{ + Schema: ` + type Book { + name: String + author: Author + } + + type Author { + name: String + published: Book @primary + } + `, + }, + testUtils.CreateDoc{ + CollectionID: 0, + // bae-3d236f89-6a31-5add-a36a-27971a2eac76 + Doc: `{ + "name": "Painted House" + }`, + }, + testUtils.CreateDoc{ + CollectionID: 0, + // bae-d6627fea-8bf7-511c-bcf9-bac4212bddd6 + Doc: `{ + "name": "Go Guide for Rust developers" + }`, + }, + testUtils.CreateDoc{ + CollectionID: 1, + // bae-6b624301-3d0a-5336-bd2c-ca00bca3de85 + Doc: `{ + "name": "John Grisham", + "published_id": "bae-3d236f89-6a31-5add-a36a-27971a2eac76" + }`, + }, + testUtils.CreateDoc{ + CollectionID: 1, + // bae-92fa9dcb-c1ee-5b84-b2f6-e9437c7f261c + Doc: `{ + "name": "Andrew Lone", + "published_id": "bae-d6627fea-8bf7-511c-bcf9-bac4212bddd6" + }`, + }, + testUtils.Request{ + Request: `query { + Book(groupBy: [author]) { + author_id + _group { + name + } + } + }`, + Results: []map[string]any{ + { + "author_id": "bae-6b624301-3d0a-5336-bd2c-ca00bca3de85", + "_group": []map[string]any{ + { + "name": "Painted House", + }, + }, + }, + { + "author_id": "bae-92fa9dcb-c1ee-5b84-b2f6-e9437c7f261c", + "_group": []map[string]any{ + { + "name": "Go Guide for Rust developers", + }, + }, + }, + }, + }, + }, + } + + testUtils.ExecuteTestCase(t, test) +} + +func TestQueryOneToOneWithGroupRelatedIDAliasFromSecondaryWithInnerGroupWithJoin(t *testing.T) { test := testUtils.TestCase{ Description: "One-to-one relation query with group by related id alias (secondary side)", Actions: []any{ @@ -169,14 +393,22 @@ func TestQueryOneToOneWithGroupRelatedIDAliasFromSecondary(t *testing.T) { }`, Results: []map[string]any{ { - "author_id": nil, + "author_id": "bae-6b624301-3d0a-5336-bd2c-ca00bca3de85", "author": map[string]any{ - "name": "Andrew Lone", + "name": "John Grisham", }, "_group": []map[string]any{ { "name": "Painted House", }, + }, + }, + { + "author_id": "bae-92fa9dcb-c1ee-5b84-b2f6-e9437c7f261c", + "author": map[string]any{ + "name": "Andrew Lone", + }, + "_group": []map[string]any{ { "name": "Go Guide for Rust developers", }, diff --git a/tests/integration/query/one_to_one/with_group_related_id_test.go b/tests/integration/query/one_to_one/with_group_related_id_test.go index 50a1111475..45b432192d 100644 --- a/tests/integration/query/one_to_one/with_group_related_id_test.go +++ b/tests/integration/query/one_to_one/with_group_related_id_test.go @@ -97,9 +97,7 @@ func TestQueryOneToOneWithGroupRelatedID(t *testing.T) { testUtils.ExecuteTestCase(t, test) } -// This test documents unwanted behaviour, see: -// https://github.com/sourcenetwork/defradb/issues/1654 -func TestQueryOneToOneWithGroupRelatedIDFromSecondary(t *testing.T) { +func TestQueryOneToOneWithGroupRelatedIDFromSecondaryWithoutGroup(t *testing.T) { test := testUtils.TestCase{ Description: "One-to-one relation query with group by related id (secondary side)", Actions: []any{ @@ -150,6 +148,235 @@ func TestQueryOneToOneWithGroupRelatedIDFromSecondary(t *testing.T) { Request: `query { Book(groupBy: [author_id]) { author_id + } + }`, + Results: []map[string]any{ + { + "author_id": "bae-6b624301-3d0a-5336-bd2c-ca00bca3de85", + }, + { + "author_id": "bae-92fa9dcb-c1ee-5b84-b2f6-e9437c7f261c", + }, + }, + }, + }, + } + + testUtils.ExecuteTestCase(t, test) +} + +func TestQueryOneToOneWithGroupRelatedIDFromSecondaryWithoutGroupWithJoin(t *testing.T) { + test := testUtils.TestCase{ + Description: "One-to-one relation query with group by related id (secondary side)", + Actions: []any{ + testUtils.SchemaUpdate{ + Schema: ` + type Book { + name: String + author: Author + } + + type Author { + name: String + published: Book @primary + } + `, + }, + testUtils.CreateDoc{ + CollectionID: 0, + // bae-3d236f89-6a31-5add-a36a-27971a2eac76 + Doc: `{ + "name": "Painted House" + }`, + }, + testUtils.CreateDoc{ + CollectionID: 0, + // bae-d6627fea-8bf7-511c-bcf9-bac4212bddd6 + Doc: `{ + "name": "Go Guide for Rust developers" + }`, + }, + testUtils.CreateDoc{ + CollectionID: 1, + // bae-6b624301-3d0a-5336-bd2c-ca00bca3de85 + Doc: `{ + "name": "John Grisham", + "published_id": "bae-3d236f89-6a31-5add-a36a-27971a2eac76" + }`, + }, + testUtils.CreateDoc{ + CollectionID: 1, + // bae-92fa9dcb-c1ee-5b84-b2f6-e9437c7f261c + Doc: `{ + "name": "Andrew Lone", + "published_id": "bae-d6627fea-8bf7-511c-bcf9-bac4212bddd6" + }`, + }, + testUtils.Request{ + Request: `query { + Book(groupBy: [author_id]) { + author_id + author { + name + } + } + }`, + Results: []map[string]any{ + { + "author_id": "bae-6b624301-3d0a-5336-bd2c-ca00bca3de85", + "author": map[string]any{ + "name": "John Grisham", + }, + }, + { + "author_id": "bae-92fa9dcb-c1ee-5b84-b2f6-e9437c7f261c", + "author": map[string]any{ + "name": "Andrew Lone", + }, + }, + }, + }, + }, + } + + testUtils.ExecuteTestCase(t, test) +} + +func TestQueryOneToOneWithGroupRelatedIDFromSecondaryWithGroup(t *testing.T) { + test := testUtils.TestCase{ + Description: "One-to-one relation query with group by related id (secondary side)", + Actions: []any{ + testUtils.SchemaUpdate{ + Schema: ` + type Book { + name: String + author: Author + } + + type Author { + name: String + published: Book @primary + } + `, + }, + testUtils.CreateDoc{ + CollectionID: 0, + // bae-3d236f89-6a31-5add-a36a-27971a2eac76 + Doc: `{ + "name": "Painted House" + }`, + }, + testUtils.CreateDoc{ + CollectionID: 0, + // bae-d6627fea-8bf7-511c-bcf9-bac4212bddd6 + Doc: `{ + "name": "Go Guide for Rust developers" + }`, + }, + testUtils.CreateDoc{ + CollectionID: 1, + // bae-6b624301-3d0a-5336-bd2c-ca00bca3de85 + Doc: `{ + "name": "John Grisham", + "published_id": "bae-3d236f89-6a31-5add-a36a-27971a2eac76" + }`, + }, + testUtils.CreateDoc{ + CollectionID: 1, + // bae-92fa9dcb-c1ee-5b84-b2f6-e9437c7f261c + Doc: `{ + "name": "Andrew Lone", + "published_id": "bae-d6627fea-8bf7-511c-bcf9-bac4212bddd6" + }`, + }, + testUtils.Request{ + Request: `query { + Book(groupBy: [author_id]) { + author_id + _group { + name + } + } + }`, + Results: []map[string]any{ + { + "author_id": "bae-6b624301-3d0a-5336-bd2c-ca00bca3de85", + "_group": []map[string]any{ + { + "name": "Painted House", + }, + }, + }, + { + "author_id": "bae-92fa9dcb-c1ee-5b84-b2f6-e9437c7f261c", + "_group": []map[string]any{ + { + "name": "Go Guide for Rust developers", + }, + }, + }, + }, + }, + }, + } + + testUtils.ExecuteTestCase(t, test) +} + +func TestQueryOneToOneWithGroupRelatedIDFromSecondaryWithGroupWithJoin(t *testing.T) { + test := testUtils.TestCase{ + Description: "One-to-one relation query with group by related id (secondary side)", + Actions: []any{ + testUtils.SchemaUpdate{ + Schema: ` + type Book { + name: String + author: Author + } + + type Author { + name: String + published: Book @primary + } + `, + }, + testUtils.CreateDoc{ + CollectionID: 0, + // bae-3d236f89-6a31-5add-a36a-27971a2eac76 + Doc: `{ + "name": "Painted House" + }`, + }, + testUtils.CreateDoc{ + CollectionID: 0, + // bae-d6627fea-8bf7-511c-bcf9-bac4212bddd6 + Doc: `{ + "name": "Go Guide for Rust developers" + }`, + }, + testUtils.CreateDoc{ + CollectionID: 1, + // bae-6b624301-3d0a-5336-bd2c-ca00bca3de85 + Doc: `{ + "name": "John Grisham", + "published_id": "bae-3d236f89-6a31-5add-a36a-27971a2eac76" + }`, + }, + testUtils.CreateDoc{ + CollectionID: 1, + // bae-92fa9dcb-c1ee-5b84-b2f6-e9437c7f261c + Doc: `{ + "name": "Andrew Lone", + "published_id": "bae-d6627fea-8bf7-511c-bcf9-bac4212bddd6" + }`, + }, + testUtils.Request{ + Request: `query { + Book(groupBy: [author_id]) { + author_id + author { + name + } _group { name } @@ -157,11 +384,22 @@ func TestQueryOneToOneWithGroupRelatedIDFromSecondary(t *testing.T) { }`, Results: []map[string]any{ { - "author_id": nil, + "author_id": "bae-6b624301-3d0a-5336-bd2c-ca00bca3de85", + "author": map[string]any{ + "name": "John Grisham", + }, "_group": []map[string]any{ { "name": "Painted House", }, + }, + }, + { + "author_id": "bae-92fa9dcb-c1ee-5b84-b2f6-e9437c7f261c", + "author": map[string]any{ + "name": "Andrew Lone", + }, + "_group": []map[string]any{ { "name": "Go Guide for Rust developers", },