Skip to content

Commit

Permalink
perf(relation): improve has many relation by using not in when possib…
Browse files Browse the repository at this point in the history
…le and available (#1148)
  • Loading branch information
Thenkei authored Sep 16, 2024
1 parent b114f74 commit a15eefe
Show file tree
Hide file tree
Showing 2 changed files with 133 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
Collection,
CollectionDecorator,
CollectionSchema,
CollectionUtils,
ColumnSchema,
ConditionTree,
ConditionTreeLeaf,
Expand Down Expand Up @@ -213,32 +214,66 @@ export default class RelationCollectionDecorator extends CollectionDecorator {
if (schema.type === 'Column') return leaf;

const relation = this.dataSource.getCollection(schema.foreignCollection);
let result = leaf as ConditionTree;
const result = leaf as ConditionTree;

if (!this.relations[prefix]) {
result = (await relation.rewriteLeaf(caller, leaf.unnest())).nest(prefix);
} else if (schema.type === 'ManyToOne') {
return (await relation.rewriteLeaf(caller, leaf.unnest())).nest(prefix);
}

if (schema.type === 'ManyToOne') {
const leafSchema = CollectionUtils.getFieldSchema(
this.childCollection,
schema.foreignKey,
) as ColumnSchema;

if (leafSchema.filterOperators.has('NotIn') && leaf.operator === 'NotEqual') {
// Possible optimization NotEqual
// We compute the inverse list and use NotIn to build the relation with the target

const records = await relation.list(
caller,
new Filter({ conditionTree: leaf.unnest().inverse() }),
new Projection(schema.foreignKeyTarget),
);

return new ConditionTreeLeaf(schema.foreignKey, 'NotIn', [
...new Set(
records
.map(record => RecordUtils.getFieldValue(record, schema.foreignKeyTarget))
.filter(v => v !== null),
),
]);
}

if (!leafSchema.filterOperators.has('NotIn') && leaf.operator === 'NotEqual') {
console.warn(
`Performances could be improved by implementing the NotIn operator on ${schema.foreignKey} (use replaceFieldOperator)`,
);
}

const records = await relation.list(
caller,
new Filter({ conditionTree: leaf.unnest() }),
new Projection(schema.foreignKeyTarget),
);

result = new ConditionTreeLeaf(schema.foreignKey, 'In', [
return new ConditionTreeLeaf(schema.foreignKey, 'In', [
...new Set(
records
.map(record => RecordUtils.getFieldValue(record, schema.foreignKeyTarget))
.filter(v => v !== null),
),
]);
} else if (schema.type === 'OneToOne') {
}

if (schema.type === 'OneToOne') {
const records = await relation.list(
caller,
new Filter({ conditionTree: leaf.unnest() }),
new Projection(schema.originKey),
);

result = new ConditionTreeLeaf(schema.originKeyTarget, 'In', [
return new ConditionTreeLeaf(schema.originKeyTarget, 'In', [
...new Set(
records
.map(record => RecordUtils.getFieldValue(record, schema.originKey))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -572,6 +572,98 @@ describe('RelationCollectionDecorator', () => {
{ personId: 203, name: 'Joseph P. Rodriguez', passport: null },
]);
});

describe('performances enhancement (operator: NotEqual)', () => {
describe('when NotIn filterOperator is available', () => {
test('should fetch fields from a many to one relation using NotIn', async () => {
const schema = passports.schema.fields.ownerId as ColumnSchema;
schema.filterOperators = new Set(['In', 'NotIn']);

newPassports.addRelation('owner', {
type: 'ManyToOne',
foreignCollection: 'persons',
foreignKey: 'ownerId',
});

const records = await newPassports.list(
factories.caller.build(),
new Filter({
conditionTree: new ConditionTreeLeaf('owner:name', 'NotEqual', 'Mae S. Waldron'),
}),
new Projection('passportId', 'owner:name'),
);

expect(records).toStrictEqual([
{ passportId: 102, owner: { name: 'Sharon J. Whalen' } },
{ passportId: 103, owner: null },
]);

// Inverted condition NotEqual becomes Equal
expect(persons.list).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({
conditionTree: { field: 'name', operator: 'Equal', value: 'Mae S. Waldron' },
}),
expect.anything(),
);

// Using NotIn match the previous inverted condition result
expect(passports.list).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({
conditionTree: { field: 'ownerId', operator: 'NotIn', value: [202] },
}),
expect.anything(),
);
});
});
});

describe('when NotIn filterOperator is not available', () => {
test('should fetch fields from a many to one relation using In', async () => {
newPassports.addRelation('owner', {
type: 'ManyToOne',
foreignCollection: 'persons',
foreignKey: 'ownerId',
});

const logger = jest.spyOn(console, 'warn').mockImplementation();

const records = await newPassports.list(
factories.caller.build(),
new Filter({
conditionTree: new ConditionTreeLeaf('owner:name', 'NotEqual', 'Mae S. Waldron'),
}),
new Projection('passportId', 'owner:name'),
);

// WARNING: Null results are not returned
expect(records).toStrictEqual([{ passportId: 102, owner: { name: 'Sharon J. Whalen' } }]);

expect(persons.list).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({
conditionTree: { field: 'name', operator: 'NotEqual', value: 'Mae S. Waldron' },
}),
expect.anything(),
);

// Using IN match the previous condition result
expect(passports.list).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({
conditionTree: { field: 'ownerId', operator: 'In', value: [201, 203] },
}),
expect.anything(),
);

expect(logger).toHaveBeenCalledWith(
'Performances could be improved by implementing the NotIn operator on ownerId (use replaceFieldOperator)',
);

logger.mockRestore();
});
});
});

describe('with two emulated relations', () => {
Expand Down

0 comments on commit a15eefe

Please sign in to comment.