Skip to content

Commit ce146c3

Browse files
committed
refactor: remove root entity capture logic
Will replace this with a different approach where we just keep root entities in a variable instead
1 parent 1c74caa commit ce146c3

File tree

15 files changed

+25
-262
lines changed

15 files changed

+25
-262
lines changed

spec/dev/model/simple.graphqls

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ type Skill @valueObject {
5050
}
5151

5252
"A superhero movie"
53-
type Movie @rootEntity @roles(read: ["logistics-reader"]) {
53+
type Movie @rootEntity {
5454
name: String
5555
"All the heroes starring in this movie"
5656
heroes: [Hero] @relation

spec/regression/traversal-performance/tests/traversal-performance/aql/RelationAndFieldTraversalWithParent.aql

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,17 +13,11 @@ RETURN {
1313
IN (
1414
FOR v_node1, v_edge1, v_path1 IN @var3..@var4 OUTBOUND v_super1 @@supers_roots
1515
FILTER v_node1 != null
16-
RETURN (
17-
FOR v_entity1 IN v_node1.`children`[*]
18-
RETURN { obj: v_entity1, root: v_node1 }
19-
)
16+
RETURN v_node1.`children`[*]
2017
)[**]
21-
LET v_root1 = v_child1.`root`
2218
RETURN {
23-
"key": v_child1.`obj`.`key`,
24-
"parent": (IS_NULL(v_root1) ? null : {
25-
"key": v_root1.`key`
26-
})
19+
"key": v_child1.`key`,
20+
"parent": { __cruddl_runtime_error_code: @var5, __cruddl_runtime_error: @var6 }
2721
}
2822
)
2923
})

spec/regression/traversal-performance/tests/traversal-performance/result.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
"RelationAndFieldTraversalWithParent": {
2222
"errors": [
2323
{
24-
"message": "AQL: query would use more memory than allowed [node #17: TraversalNode] [node #18: CalculationNode] [node #19: FilterNode] [node #43: SubqueryStartNode] [node #22: FilterNode] [node #23: CalculationNode] [node #24: EnumerateListNode] [node #25: CalculationNode] [node #44: SubqueryEndNode] [node #42: SubqueryEndNode] [node #39: SubqueryStartNode] [node #13: FilterNode] [node #30: CalculationNode] [node #31: EnumerateListNode] [node #33: CalculationNode] [node #40: SubqueryEndNode] [node #36: CalculationNode] [node #37: ReturnNode] (while executing)",
24+
"message": "AQL: query would use more memory than allowed [node #17: TraversalNode] [node #18: CalculationNode] [node #19: FilterNode] [node #20: CalculationNode] [node #34: SubqueryEndNode] [node #31: SubqueryStartNode] [node #13: FilterNode] [node #23: CalculationNode] [node #24: EnumerateListNode] [node #25: CalculationNode] [node #32: SubqueryEndNode] [node #28: CalculationNode] [node #29: ReturnNode] (while executing)",
2525
"locations": [
2626
{
2727
"line": 10,

src/authorization/transformers/traversal.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,6 @@ export function transformTraversalQueryNode(
8282
sourceEntityNode: node.sourceEntityNode,
8383
relationSegments: filteredRelationSegments,
8484
fieldSegments: fieldSegments,
85-
captureRootEntities: node.captureRootEntity,
8685
});
8786
}
8887

src/database/arangodb/aql-generator.ts

Lines changed: 2 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1406,45 +1406,8 @@ register(TraversalQueryNode, (node, context) => {
14061406
// if we have both, it might be beneficial to do the field traversal within the mapping node
14071407
// because it may allow ArangoDB to figure out that only one particular field is of interest, and e.g.
14081408
// discard the root entities earlier
1409-
1410-
if (node.captureRootEntity) {
1411-
if (fieldDepth === 0) {
1412-
// fieldSegments.length && fieldDepth === 0 means we only traverse through entity extensions
1413-
// actually, shouldn't really occur because a collect path can't end with an entity extension and
1414-
// value objects don't capture root entities
1415-
// however, we can easily implement this so let's do it
1416-
mapFrag = (nodeFrag) =>
1417-
aql`{ obj: ${getFieldTraversalFragmentWithoutFlattening(
1418-
node.fieldSegments,
1419-
nodeFrag,
1420-
)}, root: ${nodeFrag}) }`;
1421-
} else {
1422-
// the result of getFieldTraversalFragmentWithoutFlattening() now is a list, so we need to iterate
1423-
// over it. if the depth is > 1, we need to flatten the deeper ones so we can do one FOR loop over them
1424-
// we still return a list, so we just reduce the depth to 1 and not to 0
1425-
const entityVar = aql.variable('entity');
1426-
mapFrag = (rootEntityFrag) =>
1427-
aqlExt.parenthesizeList(
1428-
aql`FOR ${entityVar} IN ${getFlattenFrag(
1429-
getFieldTraversalFragmentWithoutFlattening(
1430-
node.fieldSegments,
1431-
rootEntityFrag,
1432-
),
1433-
fieldDepth - 1,
1434-
)}`,
1435-
aql`RETURN { obj: ${entityVar}, root: ${rootEntityFrag} }`,
1436-
);
1437-
remainingDepth = 1;
1438-
}
1439-
} else {
1440-
mapFrag = (nodeFrag) =>
1441-
getFieldTraversalFragmentWithoutFlattening(node.fieldSegments, nodeFrag);
1442-
}
1443-
} else {
1444-
if (node.captureRootEntity) {
1445-
// doesn't make sense to capture the root entity if we're returning the root entities
1446-
throw new Error(`captureRootEntity without fieldSegments detected`);
1447-
}
1409+
mapFrag = (nodeFrag) =>
1410+
getFieldTraversalFragmentWithoutFlattening(node.fieldSegments, nodeFrag);
14481411
}
14491412

14501413
// traversal requires real ids
@@ -1480,11 +1443,6 @@ register(TraversalQueryNode, (node, context) => {
14801443
return getFlattenFrag(frag, remainingDepth - 1);
14811444
}
14821445

1483-
if (node.captureRootEntity) {
1484-
// doesn't make sense (and isn't possible) to capture the root entity if we're not even crossing root entities
1485-
throw new Error(`captureRootEntity without relationSegments detected`);
1486-
}
1487-
14881446
if (node.sourceIsList) {
14891447
// don't need, don't bother
14901448
throw new Error(`sourceIsList without relationSegments detected`);

src/database/inmemory/js-generator.ts

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -857,12 +857,6 @@ register(TraversalQueryNode, (node, context) => {
857857
const relationTraversalReturnsList = isList;
858858
let rootVar: JSVariable | undefined;
859859
let relationFrag: JSFragment | undefined;
860-
if (node.captureRootEntity) {
861-
relationFrag = currentFrag;
862-
rootVar = js.variable('root');
863-
currentFrag = rootVar;
864-
isList = false; // we're going to be within the mapper, so not in a list
865-
}
866860

867861
for (const segment of node.fieldSegments) {
868862
if (isList) {
@@ -897,37 +891,6 @@ register(TraversalQueryNode, (node, context) => {
897891
}
898892
}
899893

900-
if (relationFrag && rootVar && node.captureRootEntity) {
901-
if (relationTraversalReturnsList) {
902-
if (node.fieldSegments.some((f) => f.isListSegment)) {
903-
const accVar = js.variable('acc');
904-
const objVar = js.variable('obj');
905-
const mapper = js`${objVar} => ({ obj: ${objVar}, root: ${rootVar} })`;
906-
const reducer = js`(${accVar}, ${rootVar}) => ${accVar}.concat((${currentFrag}).map(${mapper}))`;
907-
currentFrag = js`${relationFrag}.reduce(${reducer}, [])`;
908-
} else {
909-
const mapper = js`${rootVar} => ({ obj: ${currentFrag}, root: ${rootVar} })`;
910-
currentFrag = js`${relationFrag}.map(${mapper})`;
911-
}
912-
} else {
913-
if (node.fieldSegments.some((f) => f.isListSegment)) {
914-
const objVar = js.variable('obj');
915-
const mapper = js`${objVar} => ({ obj: ${objVar}, root: ${rootVar} })`;
916-
currentFrag = jsExt.evaluatingLambda(
917-
rootVar,
918-
js`(${currentFrag}).map(${mapper})`,
919-
relationFrag,
920-
);
921-
} else {
922-
currentFrag = jsExt.evaluatingLambda(
923-
rootVar,
924-
js`({ obj: ${currentFrag}, root: ${rootVar} })`,
925-
relationFrag,
926-
);
927-
}
928-
}
929-
}
930-
931894
if (node.alwaysProduceList && !isList) {
932895
const resultVar = js.variable('result');
933896
currentFrag = jsExt.evaluatingLambda(

src/query-tree/queries.ts

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,6 @@ interface TraversalQueryNodeParams {
163163
readonly alwaysProduceList?: boolean;
164164
readonly relationSegments: ReadonlyArray<RelationSegment>;
165165
readonly fieldSegments: ReadonlyArray<FieldSegment>;
166-
readonly captureRootEntities: boolean;
167166
}
168167

169168
/**
@@ -174,7 +173,6 @@ export class TraversalQueryNode extends QueryNode {
174173
readonly sourceEntityNode: QueryNode;
175174
readonly relationSegments: ReadonlyArray<RelationSegment>;
176175
readonly fieldSegments: ReadonlyArray<FieldSegment>;
177-
readonly captureRootEntity: boolean;
178176

179177
/**
180178
* Specifies if sourceEntityNode resolves to a list of entities instead of a single entity
@@ -189,12 +187,6 @@ export class TraversalQueryNode extends QueryNode {
189187
constructor(params: TraversalQueryNodeParams) {
190188
super();
191189

192-
if (params.captureRootEntities && (!params.relationSegments || !params.fieldSegments)) {
193-
throw new Error(
194-
`A TraversalQueryNode with captureRootEntity=true requires both relationSegments and fieldSegments`,
195-
);
196-
}
197-
198190
if (params.sourceIsList && !params.relationSegments) {
199191
// only need this, so keep it simpler
200192
throw new Error(
@@ -205,7 +197,6 @@ export class TraversalQueryNode extends QueryNode {
205197
this.sourceEntityNode = params.sourceEntityNode;
206198
this.relationSegments = params.relationSegments;
207199
this.fieldSegments = params.fieldSegments;
208-
this.captureRootEntity = params.captureRootEntities;
209200
this.sourceIsList = params.sourceIsList ?? false;
210201
this.alwaysProduceList = params.alwaysProduceList ?? false;
211202
this.entitiesIdentifierKind =
@@ -216,9 +207,7 @@ export class TraversalQueryNode extends QueryNode {
216207
const segments = [...this.relationSegments, ...this.fieldSegments];
217208
return (
218209
`traverse ${segments.map((s) => this.describeSegment(s)).join('.')}` +
219-
`from ${this.sourceEntityNode.describe()}${this.sourceIsList ? ' (as list)' : ''}${
220-
this.captureRootEntity ? ` into { obj, root }` : ''
221-
}${this.alwaysProduceList ? ` as list` : ''}`
210+
`from ${this.sourceEntityNode.describe()}${this.sourceIsList ? ' (as list)' : ''}${this.alwaysProduceList ? ` as list` : ''}`
222211
);
223212
}
224213

src/schema-generation/field-nodes.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import {
1414
FollowEdgeQueryNode,
1515
NullQueryNode,
1616
ObjectQueryNode,
17-
PropertyAccessQueryNode,
1817
QueryNode,
1918
RootEntityIDQueryNode,
2019
SafeListQueryNode,
@@ -24,7 +23,7 @@ import {
2423
VariableQueryNode,
2524
} from '../query-tree';
2625
import { ID_FIELD } from '../schema/constants';
27-
import { GraphQLOffsetDateTime, TIMESTAMP_PROPERTY } from '../schema/scalars/offset-date-time';
26+
import { GraphQLOffsetDateTime } from '../schema/scalars/offset-date-time';
2827
import { getScalarFilterValueNode } from './filter-input-types/filter-fields';
2928
import { and } from './utils/input-types';
3029

@@ -33,7 +32,7 @@ export function createFieldNode(
3332
sourceNode: QueryNode,
3433
options: {
3534
skipNullFallbackForEntityExtensions?: boolean;
36-
captureRootEntitiesOnCollectFields?: boolean;
35+
rootEntityVar?: VariableQueryNode;
3736
} = {},
3837
): QueryNode {
3938
// make use of the fact that field access on non-objects is NULL, so that type checks for OBJECT are redundant
@@ -61,7 +60,6 @@ export function createFieldNode(
6160
sourceEntityNode: sourceNode,
6261
relationSegments,
6362
fieldSegments,
64-
captureRootEntities: !!options.captureRootEntitiesOnCollectFields,
6563
});
6664

6765
if (!field.aggregationOperator) {

src/schema-generation/filter-augmentation.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,6 @@ export class FilterAugmentation {
3636
filterValue: args[FILTER_ARG],
3737
filterType,
3838
itemType,
39-
objectNodeCallback: (itemNode) =>
40-
this.rootFieldHelper.getRealItemNode(itemNode, info),
4139
});
4240
},
4341
};

src/schema-generation/flex-search-post-filter-augmentation.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,6 @@ export class FlexSearchPostFilterAugmentation {
6868
filterValue: filterValue || legacyFilterValue,
6969
filterType,
7070
itemType,
71-
objectNodeCallback: (itemNode) =>
72-
this.rootFieldHelper.getRealItemNode(itemNode, info),
7371
});
7472
},
7573
};

0 commit comments

Comments
 (0)