Changeset View
Standalone View
lib/datastore/query-step.js
- This file was added.
const object_hash = require("object-hash"); | |||||
class QueryStep { | |||||
kuba-orlik: (could) Aby uniknąć luźno wiszącej funkcji, przypiąłbym ją jako statyczną metodę `QueryStep. | |||||
constructor(body) { | |||||
this.body = body; | |||||
} | |||||
Done Inline Actions(should) przydałoby się dodać unorderedObjects: true, aby hash nie był wrażliwy na zmianę kolejności kluczy kuba-orlik: (should) przydałoby się dodać `unorderedObjects: true`, aby hash nie był wrażliwy na zmianę… | |||||
Not Done Inline ActionsNie jest wrażliwy - to domyślne (i zdroworozsądkowe) ustawienie ;) piotr-ptaszynski: Nie jest wrażliwy - to domyślne (i zdroworozsądkowe) ustawienie ;) | |||||
hash() { | |||||
return QueryStep.hashBody(this.body); | |||||
} | |||||
static fromStage(stage, unwind = true) { | |||||
if (stage.$lookup) { | |||||
const clonedStageBody = Object.assign({}, stage.$lookup); | |||||
clonedStageBody.unwind = unwind; | |||||
return [new QueryStep.Lookup(clonedStageBody)]; | |||||
} else if (stage.$match) { | |||||
return Object.keys(stage.$match).map( | |||||
field => new QueryStep.Match({ [field]: stage.$match[field] }) | |||||
); | |||||
} | |||||
throw new Error("Unsupported stage: " + JSON.stringify(stage)); | |||||
} | |||||
Done Inline Actions(should) lepiej ustawić jedno API do tworzenia QuerySteps. Proponuję wybrać new QueryStep.Lookup i zrezygnować z QueryStep.makeMatch kuba-orlik: (should) lepiej ustawić jedno API do tworzenia `QuerySteps`. Proponuję wybrać `new QueryStep. | |||||
Done Inline ActionsWybacz, ale nie rozumiem tego komentarza, bo odczytuję go w ten sposób, że matche też byśmy mieli tworzyć za pomocą QueryStep.Lookup, a wiem że na pewno nie to miałeś na myśli piotr-ptaszynski: Wybacz, ale nie rozumiem tego komentarza, bo odczytuję go w ten sposób, że matche też byśmy… | |||||
Done Inline ActionsChodzi o to, że mamy dwa sposoby na tworzenie QuerySteps - jedno z operatorem new QueryStep.Cośtam, a drugie za pomocą metody QueryStep.makeCośtam. Myślę, że warto to ujednolicić np do używania tylko notacji z new kuba-orlik: Chodzi o to, że mamy dwa sposoby na tworzenie `QuerySteps` - jedno z operatorem `new QueryStep. | |||||
pushDump(dumps) { | |||||
dumps.push(this.body); | |||||
return dumps; | |||||
} | |||||
Done Inline ActionsDlaczego zwracamy tutaj tablicę? :o Nazwa QueryStep.fromStage sugeruje, że to jest helper tworzący jeden krok kuba-orlik: Dlaczego zwracamy tutaj tablicę? :o Nazwa `QueryStep.fromStage` sugeruje, że to jest helper… | |||||
Done Inline ActionsTo dla zapewnienia spójności z matchem. Chodzi o to, że match może mieć postać np. {$match: {a: {$eq: 1}, {b: {$eq: 2}}} Czyli zawierać dwa kroki agregacji, a QueryStep z założenia ma zwracać jeden. Alternatywą jest zabronienie przekazywania wielu kroków w obrębie jednego matcha piotr-ptaszynski: To dla zapewnienia spójności z matchem. Chodzi o to, że match może mieć postać np.
```
{$match… | |||||
Done Inline ActionsHm, co masz na myśli? {$match: {a: {$eq: 1}, {b: {$eq: 2}}} ^ to jest jeden krok agregacji, prawda? kuba-orlik: Hm, co masz na myśli?
```
{$match: {a: {$eq: 1}, {b: {$eq: 2}}}
```
^ to jest jeden krok… | |||||
Done Inline ActionsNo nie, a przynajmniej nie chcemy żeby był, bo ogranicza to możliwości optymalizacji. Zauważ, że w wypadku czegoś takiego: [ {lookup}, { $match: { field: {$eq: 1}, field_dependent_on_lookup: {$eq: 2}, } }, ] Chcielibyśmy to zoptymalizować do postaci: [ {$match: {field: {$eq: 1}}}, {lookup}, {$match: {field_dependent_on_lookup: {$eq: 2}}}, ] Tak czy siak gdzieś tego rozbicia będziemy musieli dokonać. piotr-ptaszynski: No nie, a przynajmniej nie chcemy żeby był, bo ogranicza to możliwości optymalizacji. Zauważ… | |||||
Done Inline ActionsI see. Nie wiedziałem, że używamy rozdrabniania do optymalizacji :D Najs. To kładzie fajny fundament pod optymalizację OR ;) kuba-orlik: I see. Nie wiedziałem, że używamy rozdrabniania do optymalizacji :D Najs. To kładzie fajny… | |||||
static hashBody(body) { | |||||
return object_hash(body, { | |||||
algorithm: "md5", | |||||
excludeKeys: key => key === "as", | |||||
}); | |||||
} | |||||
getUsedFields() { | |||||
throw new Error("Cannot be used on base QueryStep class"); | |||||
} | |||||
} | |||||
QueryStep.Lookup = class extends QueryStep { | |||||
constructor(body) { | |||||
const cleared_body = { | |||||
from: body.from, | |||||
localField: body.localField, | |||||
foreignField: body.foreignField, | |||||
}; | |||||
cleared_body.as = QueryStep.hashBody(cleared_body); | |||||
super(cleared_body); | |||||
this.unwind = body.unwind; | |||||
} | |||||
hash() { | |||||
return this.body.as; | |||||
Done Inline Actions(should) Ta funkcja ma za dużo odpowiedzialności:
Myślę, że zyskałaby na czytelności i ogólności, gdyby jej użycia zastąpić prostym steps.filter(step=>step instanceof QueryStep.Lookup).map(lookup_handler); steps.filter(step=>step instanceof QueryStep.Match).map(match_handler); kuba-orlik: (should) Ta funkcja ma za dużo odpowiedzialności:
* filtrowanie;
* mapowanie;
* sklejanie… | |||||
} | |||||
pushStage(pipeline) { | |||||
pipeline.push({ $lookup: this.body }); | |||||
if (this.unwind) { | |||||
pipeline.push({ $unwind: "$" + this.body.as }); | |||||
} | |||||
return pipeline; | |||||
} | |||||
getUsedFields() { | |||||
return this.body.localField.split("."); | |||||
} | |||||
Done Inline Actions(could) Skoro robimy już inną reprezentację wewnętrzną, to rozważyłbym uczynić unwind jednym z kluczy w body, zamiast osobnym argumentem do konstruktora kuba-orlik: (could) Skoro robimy już inną reprezentację wewnętrzną, to rozważyłbym uczynić `unwind` jednym… | |||||
getCost() { | |||||
return 8; | |||||
} | |||||
}; | |||||
QueryStep.Match = class extends QueryStep { | |||||
pushStage(pipeline) { | |||||
pipeline.push({ $match: this.body }); | |||||
return pipeline; | |||||
} | |||||
getUsedFields() { | |||||
return getAllKeys(this.body) | |||||
.map(path => path.split(".")) | |||||
.reduce((acc, fields) => | |||||
acc.concat(fields.filter(field => !field.startsWith("$"))) | |||||
); | |||||
} | |||||
getCost() { | |||||
return this.body.$or ? 2 : 0; | |||||
} | |||||
}; | |||||
function getAllKeys(obj) { | |||||
return Object.keys(obj).reduce((acc, key) => { | |||||
if (obj[key] instanceof Object) { | |||||
acc.push(...getAllKeys(obj[key])); | |||||
} | |||||
if (!Array.isArray(obj)) { | |||||
acc.push(key); | |||||
} | |||||
return acc; | |||||
}, []); | |||||
} | |||||
module.exports = QueryStep; |
(could) Aby uniknąć luźno wiszącej funkcji, przypiąłbym ją jako statyczną metodę QueryStep.hashBody