Skip to content

Commit

Permalink
Merge pull request #733 from FgForrest/2024-11-fix-patch
Browse files Browse the repository at this point in the history
fix: cumulative patch
  • Loading branch information
novoj authored Nov 4, 2024
2 parents c61d6de + aa7bff0 commit 8d90003
Show file tree
Hide file tree
Showing 8 changed files with 158 additions and 101 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -170,9 +170,10 @@ public static QueryPlan planQuery(@Nonnull QueryPlanningContext context) {
*/
@Nonnull
public static QueryPlan planNestedQuery(
@Nonnull QueryPlanningContext context
@Nonnull QueryPlanningContext context,
@Nonnull Supplier<String> nestedQueryDescription
) {
context.pushStep(QueryPhase.PLANNING_NESTED_QUERY);
context.pushStep(QueryPhase.PLANNING_NESTED_QUERY, nestedQueryDescription);
try {
// determine the indexes that should be used for filtering
final IndexSelectionResult<?> indexSelectionResult = selectIndexes(context);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -605,6 +605,17 @@ public EntityCollection getEntityCollectionOrThrowException(@Nullable String ent
.orElseThrow(() -> new EntityCollectionRequiredException(reason));
}

/**
* Method returns appropriate {@link EntityCollection} for the {@link #evitaRequest} or throws comprehensible
* exception. In order exception to be comprehensible you need to provide sensible `reason` for accessing
* the collection in the input parameter.
*/
@Nonnull
public EntityCollection getEntityCollectionOrThrowException(@Nullable String entityType, @Nonnull Supplier<String> reasonSupplier) {
return getEntityCollection(entityType)
.orElseThrow(() -> new EntityCollectionRequiredException(reasonSupplier.get()));
}

/**
* Method creates new {@link EvitaRequest} for particular `entityType` that takes all passed `requiredConstraints`
* into the account. Fabricated request is expected to be used only for passing the scope to
Expand Down Expand Up @@ -637,9 +648,9 @@ public Formula computeOnlyOnce(
@Nonnull Supplier<Formula> formulaSupplier,
long... additionalCacheKeys
) {
if (parentContext == null) {
if (internalCache == null) {
internalCache = new HashMap<>();
if (this.parentContext == null) {
if (this.internalCache == null) {
this.internalCache = new HashMap<>();
}
final InternalCacheKey cacheKey = new InternalCacheKey(
LongStream.concat(
Expand All @@ -648,17 +659,17 @@ public Formula computeOnlyOnce(
).toArray(),
constraint
);
final Formula cachedResult = internalCache.get(cacheKey);
final Formula cachedResult = this.internalCache.get(cacheKey);
if (cachedResult == null) {
final Formula computedResult = formulaSupplier.get();
computedResult.initialize(this.internalExecutionContext);
internalCache.put(cacheKey, computedResult);
this.internalCache.put(cacheKey, computedResult);
return computedResult;
} else {
return cachedResult;
}
} else {
return parentContext.computeOnlyOnce(
return this.parentContext.computeOnlyOnce(
entityIndexes, constraint, formulaSupplier, additionalCacheKeys
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -578,9 +578,9 @@ private static Formula computeResultWithPassedIndex(
// compute the result formula in the initialized context
final String referenceName = referenceSchema.getName();
final ProcessingScope<?> processingScope = filterByVisitor.getProcessingScope();
final Formula filterFormula = filterByVisitor.executeInContext(
final Formula filterFormula = filterByVisitor.executeInContextAndIsolatedFormulaStack(
ReducedEntityIndex.class,
Collections.singletonList(index),
() -> Collections.singletonList(index),
ReferenceContent.ALL_REFERENCES,
processingScope.getEntitySchema(),
referenceSchema,
Expand Down Expand Up @@ -635,7 +635,7 @@ private static void initNestedQueryComparator(
* Initializes the {@link Sorter} implementation in the passed `entityNestedQueryComparator` from the global index
* of the passed `targetEntityCollection`.
*
* @param referenceSchemaContract the schema of the reference
* @param referenceSchema the schema of the reference
* @param targetEntityCollection collection of the referenced entity type
* @param targetEntityGroupCollection collection of the referenced entity type group
* @param entityNestedQueryComparator comparator that holds information about requested ordering so that we can
Expand All @@ -645,7 +645,7 @@ private static void initNestedQueryComparator(
* @param evitaSession current session
*/
private static void initializeComparatorFromGlobalIndex(
@Nonnull ReferenceSchemaContract referenceSchemaContract,
@Nonnull ReferenceSchemaContract referenceSchema,
@Nonnull EntityCollection targetEntityCollection,
@Nullable EntityCollection targetEntityGroupCollection,
@Nonnull EntityNestedQueryComparator entityNestedQueryComparator,
Expand All @@ -662,24 +662,32 @@ private static void initializeComparatorFromGlobalIndex(
),
evitaSession
);
final QueryPlan queryPlan = QueryPlanner.planNestedQuery(nestedQueryContext);
final QueryPlan queryPlan = QueryPlanner.planNestedQuery(
nestedQueryContext,
() -> "ordering reference `" + referenceSchema.getName() +
"` by entity `" + targetEntityCollection.getEntityType() + "`: " + entityOrderBy
);
final Sorter sorter = queryPlan.getSorter();
entityNestedQueryComparator.setSorter(nestedQueryContext.createExecutionContext(), sorter);
}
final EntityGroupProperty entityGroupOrderBy = entityNestedQueryComparator.getGroupOrderBy();
if (entityGroupOrderBy != null) {
Assert.isTrue(
targetEntityGroupCollection != null,
"The `entityGroupProperty` ordering is specified in the query but the reference `" + referenceSchemaContract.getName() + "` does not have managed entity group collection!"
"The `entityGroupProperty` ordering is specified in the query but the reference `" + referenceSchema.getName() + "` does not have managed entity group collection!"
);

final OrderBy orderBy = new OrderBy(entityGroupOrderBy.getChildren());
final QueryPlanningContext nestedQueryContext = targetEntityGroupCollection.createQueryContext(
evitaRequest.deriveCopyWith(targetEntityCollection.getEntityType(), null, orderBy, entityNestedQueryComparator.getLocale()),
evitaRequest.deriveCopyWith(targetEntityGroupCollection.getEntityType(), null, orderBy, entityNestedQueryComparator.getLocale()),
evitaSession
);

final QueryPlan queryPlan = QueryPlanner.planNestedQuery(nestedQueryContext);
final QueryPlan queryPlan = QueryPlanner.planNestedQuery(
nestedQueryContext,
() -> "ordering reference groups `" + referenceSchema.getName() +
"` by entity group `" + targetEntityGroupCollection.getEntityType() + "`: " + entityGroupOrderBy
);
final Sorter sorter = queryPlan.getSorter();
entityNestedQueryComparator.setGroupSorter(nestedQueryContext.createExecutionContext(), sorter);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,9 @@ protected <T extends EntityIndex> Formula createFilterFormula(
);
// now analyze the filter by in a nested context with exchanged primary entity index
final Formula theFormula = queryContext.analyse(
theFilterByVisitor.executeInContext(
theFilterByVisitor.executeInContextAndIsolatedFormulaStack(
indexType,
Collections.singletonList(entityIndex),
() -> Collections.singletonList(entityIndex),
null,
targetEntitySchema,
null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,10 @@ protected List<Accumulator> createStatistics(
);

final List<Accumulator> children = childVisitor.getAccumulators();
if (children.isEmpty()) {
return Collections.emptyList();
}

Assert.isPremiseValid(children.size() == 1, "Expected exactly one node but found `" + children.size() + "`!");
final Accumulator startNode = children.get(0);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,15 +189,15 @@ public class FilterByVisitor implements ConstraintVisitor, PrefetchStrategyResol
* Contemporary stack for keeping results resolved for each level of the query.
*/
@Getter(AccessLevel.PROTECTED)
private final Deque<ProcessingScope<? extends Index<?>>> scope = new ArrayDeque<>(16);
private final Deque<ProcessingScope<? extends Index<?>>> scope = new ArrayDeque<>(8);
/**
* Contains list of registered post processors. Formula post processor is used to transform final {@link Formula}
* tree constructed in {@link FilterByVisitor} before computing the result. Post processors should analyze created
* tree and optimize it to achieve maximal impact of memoization process or limit the scope of processed records
* as soon as possible. We may take advantage of transitivity in boolean algebra to exchange formula placement
* the way it's most performant.
*/
private final LinkedHashMap<Class<? extends FormulaPostProcessor>, FormulaPostProcessor> postProcessors = new LinkedHashMap<>(16);
private final Deque<LinkedHashMap<Class<? extends FormulaPostProcessor>, FormulaPostProcessor>> postProcessors = new ArrayDeque<>(8);
/**
* Reference to the query context that allows to access entity bodies, indexes, original request and much more.
*/
Expand Down Expand Up @@ -262,9 +262,9 @@ public static Formula createFormulaForTheFilter(
theFormula = queryContext.getGlobalEntityIndexIfExists(entityType)
.map(
entityIndex -> queryContext.analyse(
theFilterByVisitor.executeInContext(
theFilterByVisitor.executeInContextAndIsolatedFormulaStack(
GlobalEntityIndex.class,
Collections.singletonList(entityIndex),
() -> Collections.singletonList(entityIndex),
null,
queryContext.getSchema(entityType),
null,
Expand Down Expand Up @@ -293,6 +293,7 @@ protected <T extends Index<?>> FilterByVisitor(
@Nonnull TargetIndexes<T> indexSetToUse
) {
this.stack.push(new LinkedList<>());
this.postProcessors.push(new LinkedHashMap<>(16));
this.scope.push(processingScope);
this.queryContext = queryContext;
//I just can't get generic to work here
Expand Down Expand Up @@ -342,7 +343,6 @@ public Formula getFormulaAndClear() {
.map(this::constructFinalFormula)
.orElseGet(this::getSuperSetFormula);
this.computedFormula = null;
this.postProcessors.clear();
return result;
}

Expand All @@ -368,7 +368,7 @@ public boolean isReferencingHierarchicalEntity(@Nonnull ReferenceSchemaContract
*/
@Nonnull
public Formula[] getCollectedFormulasOnCurrentLevel() {
return ofNullable(stack.peek())
return ofNullable(this.stack.peek())
.map(it -> it.toArray(Formula[]::new))
.orElse(EMPTY_INTEGER_FORMULA);
}
Expand Down Expand Up @@ -507,7 +507,7 @@ public <T extends FormulaPostProcessor> T registerFormulaPostProcessor(
@Nonnull Supplier<T> formulaPostProcessorSupplier
) {
final T value = formulaPostProcessorSupplier.get();
this.postProcessors.put(
this.postProcessors.peek().put(
postProcessorType,
value
);
Expand Down Expand Up @@ -580,9 +580,9 @@ public Formula getReferencedRecordIdFormula(
return EmptyFormula.INSTANCE;
}

final Formula resultFormula = executeInContext(
final Formula resultFormula = executeInContextAndIsolatedFormulaStack(
ReferencedTypeEntityIndex.class,
Collections.singletonList(entityIndex.get()),
() -> Collections.singletonList(entityIndex.get()),
ReferenceContent.ALL_REFERENCES,
entitySchema,
referenceSchema,
Expand Down Expand Up @@ -703,6 +703,7 @@ public final <T, S extends Index<?>> T executeInContextAndIsolatedFormulaStack(
) {
try {
this.stack.push(new LinkedList<>());
this.postProcessors.push(new LinkedHashMap<>(16));
return executeInContext(
indexType,
targetIndexSupplier,
Expand All @@ -718,6 +719,7 @@ public final <T, S extends Index<?>> T executeInContextAndIsolatedFormulaStack(
);
} finally {
this.stack.pop();
this.postProcessors.poll();
}
}

Expand Down Expand Up @@ -979,9 +981,10 @@ private void addFormula(@Nonnull Formula formula) {
@Nonnull
private Formula constructFinalFormula(@Nonnull Formula constraintFormula) {
Formula finalFormula = constraintFormula;
if (!this.postProcessors.isEmpty()) {
final Set<FormulaPostProcessor> executedProcessors = CollectionUtils.createHashSet(postProcessors.size());
for (FormulaPostProcessor postProcessor : this.postProcessors.values()) {
final LinkedHashMap<Class<? extends FormulaPostProcessor>, FormulaPostProcessor> thePostProcessors = this.postProcessors.peek();
if (!thePostProcessors.isEmpty()) {
final Set<FormulaPostProcessor> executedProcessors = CollectionUtils.createHashSet(thePostProcessors.size());
for (FormulaPostProcessor postProcessor : thePostProcessors.values()) {
if (!executedProcessors.contains(postProcessor)) {
postProcessor.visit(finalFormula);
finalFormula = postProcessor.getPostProcessedFormula();
Expand Down Expand Up @@ -1180,6 +1183,7 @@ public ProcessingScope(
/**
* Returns indexes that should be used for searching.
*/
@Nonnull
public List<T> getIndexes() {
if (indexes == null) {
this.indexes = this.indexSupplier.get();
Expand All @@ -1190,6 +1194,7 @@ public List<T> getIndexes() {
/**
* Returns stream of indexes that should be used for searching.
*/
@Nonnull
public Stream<T> getIndexStream() {
return getIndexes().stream();
}
Expand Down
Loading

0 comments on commit 8d90003

Please sign in to comment.