Page MenuHomeSealhub
Differential D213 Diff 762 lib/datastore/query-step.js

Changeset View

Standalone View

View Options

lib/datastore/query-step.js

  • This file was added.
const object_hash = require("object-hash");
const hash_item = value =>
kuba-orlikUnsubmitted
Done
Inline Actions

(could) Aby uniknąć luźno wiszącej funkcji, przypiąłbym ją jako statyczną metodę QueryStep.hashBody

kuba-orlik: (could) Aby uniknąć luźno wiszącej funkcji, przypiąłbym ją jako statyczną metodę `QueryStep.
object_hash(value, {
algorithm: "md5",
excludeKeys: key => key === "as",
kuba-orlikUnsubmitted
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ę…
piotr-ptaszynskiAuthorUnsubmitted
Not Done
Inline Actions

Nie jest wrażliwy - to domyślne (i zdroworozsądkowe) ustawienie ;)

piotr-ptaszynski: Nie jest wrażliwy - to domyślne (i zdroworozsądkowe) ustawienie ;)
});
class QueryStep {
constructor(body) {
this.body = body;
}
hash() {
return hash_item(this.body);
}
static makeLookup(body, unwind) {
return new QueryStep.Lookup(body, unwind);
}
static makeMatch(body) {
return new QueryStep.Match(body);
}
kuba-orlikUnsubmitted
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.
piotr-ptaszynskiAuthorUnsubmitted
Done
Inline Actions

Wybacz, 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…
kuba-orlikUnsubmitted
Done
Inline Actions

Chodzi 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.
static fromStage(stage, unwind = true) {
if (stage.$lookup) {
const clonedStageBody = Object.assign({}, stage.$lookup);
return [QueryStep.makeLookup(clonedStageBody, unwind)];
kuba-orlikUnsubmitted
Done
Inline Actions

Dlaczego 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…
piotr-ptaszynskiAuthorUnsubmitted
Done
Inline Actions

To 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…
kuba-orlikUnsubmitted
Done
Inline Actions

Hm, 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…
piotr-ptaszynskiAuthorUnsubmitted
Done
Inline Actions

No 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ż…
kuba-orlikUnsubmitted
Done
Inline Actions

I 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…
} else if (stage.$match) {
return Object.keys(stage.$match).map(field =>
QueryStep.makeMatch({ [field]: stage.$match[field] })
);
}
throw new Error("Unsupported stage: " + stage);
}
pushDump(dumps) {
dumps.push(this.body);
return dumps;
}
static group(steps, putters) {
return steps.reduce(
(acc, step) => {
if (step instanceof QueryStep.Lookup) {
putters.lookup(acc[0], step);
} else if (step instanceof QueryStep.Match) {
putters.match(acc[1], step);
}
return acc;
},
[[], []]
);
}
kuba-orlikUnsubmitted
Done
Inline Actions

(should) Ta funkcja ma za dużo odpowiedzialności:

  • filtrowanie;
  • mapowanie;
  • sklejanie wartości dwóch różnych wartości filtra.

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…
getUsedFields() {
throw new Error("Cannot be used on base QueryStep class");
}
}
QueryStep.Lookup = class extends QueryStep {
constructor(body, unwind) {
delete body.as;
body.as = hash_item(body);
super(body);
this.unwind = unwind;
kuba-orlikUnsubmitted
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…
}
hash() {
return this.body.as;
}
pushStage(pipeline) {
pipeline.push({ $lookup: this.body });
if (this.unwind) {
pipeline.push({ $unwind: "$" + this.body.as });
}
return pipeline;
}
getUsedFields() {
return this.body.localField.split(".");
}
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;