Skip to content

Commit

Permalink
fix(#471): Extra results sorter does not carry the correct query context
Browse files Browse the repository at this point in the history 
Query that used prefetch evaluation logic revealed that sorters in extra result producers - namely facet summary and hierarchy summary stick to the original query context. This resulted in incorrect sorting implementation which tried to locate entities of invalid type in original queried entity query context, which provided unexpected NULL value.
  • Loading branch information
@novoj
novoj committed Feb 13, 2024
1 parent 3300122 commit 41e081c
Show file tree
Hide file tree
Showing 14 changed files with 261 additions and 153 deletions.
8 changes: 3 additions & 5 deletions evita_engine/src/main/java/io/evitadb/core/SessionRegistry.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,23 +178,21 @@ public Object invoke(Object proxy, Method method, Object[] args) {
throw evitaInvalidUsageException;
} else if (targetException instanceof EvitaInternalError evitaInternalError) {
log.error(
"Internal Evita error occurred in {}: {}",
evitaInternalError.getErrorCode(),
evitaInternalError.getPrivateMessage(),
"Internal Evita error occurred in " + evitaInternalError.getErrorCode() + ": " + evitaInternalError.getPrivateMessage(),
targetException
);
// unwrap and rethrow
throw evitaInternalError;
} else {
log.error("Unexpected internal Evita error occurred: {}", ex.getCause().getMessage(), targetException);
log.error("Unexpected internal Evita error occurred: " + ex.getCause().getMessage(), targetException);
throw new EvitaInternalError(
"Unexpected internal Evita error occurred: " + ex.getCause().getMessage(),
"Unexpected internal Evita error occurred.",
targetException
);
}
} catch (Throwable ex) {
log.error("Unexpected system error occurred: {}", ex.getMessage(), ex);
log.error("Unexpected system error occurred: " + ex.getMessage(), ex);
throw new EvitaInternalError(
"Unexpected system error occurred: " + ex.getMessage(),
"Unexpected system error occurred.",
Expand Down
16 changes: 16 additions & 0 deletions evita_engine/src/main/java/io/evitadb/core/query/QueryContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -908,6 +908,22 @@ public SealedEntity enrichOrLimitReferencedEntity(
PRIVATE METHODS
*/

/**
* Method returns appropriate {@link EntityCollection} for the {@link #evitaRequest} or empty value.
*/
@Nonnull
public Optional<EntityCollection> getEntityCollection(@Nullable String entityType) {
if (entityType == null) {
return Optional.empty();
} else if (Objects.equals(entityType, this.entityType) && entityCollection != null) {
return Optional.of(entityCollection);
} else {
return Optional.ofNullable(
(EntityCollection) catalog.getCollectionForEntity(entityType).orElse(null)
);
}
}

/**
* 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
Expand Down
2 changes: 1 addition & 1 deletion evita_engine/src/main/java/io/evitadb/core/query/QueryPlanner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ private static PrefetchFormulaVisitor createPrefetchFormulaVisitor(
@Nonnull QueryContext queryContext
) {
if (targetIndex.isGlobalIndex() || targetIndex.isCatalogIndex()) {
return new PrefetchFormulaVisitor(queryContext);
return new PrefetchFormulaVisitor();
} else {
return null;
}
Expand Down
7 changes: 1 addition & 6 deletions ...a_engine/src/main/java/io/evitadb/core/query/algebra/prefetch/PrefetchFormulaVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,6 @@ public class PrefetchFormulaVisitor implements FormulaVisitor, FormulaPostProces
* that needs to be prefetched.
*/
@Nonnull private final Bitmap entityReferences = new BaseBitmap();
/**
* The query context that will be used to prefetch entities.
*/
@Nonnull private final QueryContext queryContext;
/**
* Flag that signalizes {@link #visit(Formula)} happens in conjunctive scope.
*/
Expand Down Expand Up @@ -134,8 +130,7 @@ private static long estimatePrefetchCost(int prefetchedEntityCount, @Nonnull Ent
return PREFETCH_COST_ESTIMATOR.apply(prefetchedEntityCount, requirements.getRequirements().length);
}

public PrefetchFormulaVisitor(@Nonnull QueryContext queryContext) {
this.queryContext = queryContext;
public PrefetchFormulaVisitor() {
}

/**
Expand Down
97 changes: 63 additions & 34 deletions evita_engine/src/main/java/io/evitadb/core/query/extraResult/ExtraResultPlanningVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,15 @@
import io.evitadb.api.query.filter.HierarchyWithinRoot;
import io.evitadb.api.query.filter.ReferenceHaving;
import io.evitadb.api.query.filter.UserFilter;
import io.evitadb.api.query.order.OrderBy;
import io.evitadb.api.query.require.*;
import io.evitadb.api.query.visitor.ConstraintCloneVisitor;
import io.evitadb.api.requestResponse.EvitaRequest;
import io.evitadb.api.requestResponse.extraResult.Hierarchy.LevelInfo;
import io.evitadb.api.requestResponse.extraResult.QueryTelemetry.QueryPhase;
import io.evitadb.api.requestResponse.schema.EntitySchemaContract;
import io.evitadb.api.requestResponse.schema.ReferenceSchemaContract;
import io.evitadb.core.EntityCollection;
import io.evitadb.core.query.AttributeSchemaAccessor;
import io.evitadb.core.query.PrefetchRequirementCollector;
import io.evitadb.core.query.QueryContext;
Expand Down Expand Up @@ -74,11 +76,14 @@
import io.evitadb.core.query.extraResult.translator.reference.ReferenceContentTranslator;
import io.evitadb.core.query.indexSelection.TargetIndexes;
import io.evitadb.core.query.sort.DeferredSorter;
import io.evitadb.core.query.sort.NestedContextSorter;
import io.evitadb.core.query.sort.NoSorter;
import io.evitadb.core.query.sort.OrderByVisitor;
import io.evitadb.core.query.sort.Sorter;
import io.evitadb.core.query.sort.attribute.translator.EntityAttributeExtractor;
import io.evitadb.exception.EvitaInternalError;
import io.evitadb.index.EntityIndex;
import io.evitadb.index.GlobalEntityIndex;
import io.evitadb.utils.ArrayUtils;
import lombok.Getter;
import lombok.experimental.Delegate;
Expand Down Expand Up @@ -164,6 +169,10 @@ public class ExtraResultPlanningVisitor implements ConstraintVisitor {
* Contains an accessor providing access to the attribute schemas.
*/
@Getter private final AttributeSchemaAccessor attributeSchemaAccessor;
/**
* Contemporary stack for auxiliary data resolved for each level of the query.
*/
private final Deque<ProcessingScope> scope = new ArrayDeque<>(32);
/**
* Performance optimization when multiple translators ask for the same (last) producer.
*/
Expand All @@ -189,10 +198,6 @@ public class ExtraResultPlanningVisitor implements ConstraintVisitor {
* times.
*/
private Set<Formula> userFilterFormula;
/**
* Contemporary stack for auxiliary data resolved for each level of the query.
*/
private final Deque<ProcessingScope> scope = new ArrayDeque<>(32);

public ExtraResultPlanningVisitor(
@Nonnull QueryContext queryContext,
Expand Down Expand Up @@ -357,10 +362,10 @@ public FilterBy getFilterByWithoutHierarchyAndUserFilter(@Nullable ReferenceSche
* the {@link io.evitadb.api.requestResponse.extraResult.Hierarchy} result object.
*/
@Nonnull
public Sorter createSorter(
public NestedContextSorter createSorter(
@Nonnull ConstraintContainer<OrderConstraint> orderBy,
@Nullable Locale locale,
@Nonnull EntityIndex entityIndex,
@Nonnull EntityCollection entityCollection,
@Nonnull String entityType,
@Nonnull Supplier<String> stepDescriptionSupplier
) {
Expand All @@ -369,38 +374,62 @@ public Sorter createSorter(
QueryPhase.PLANNING_SORT,
stepDescriptionSupplier
);
// crete a visitor
final OrderByVisitor orderByVisitor = new OrderByVisitor(
queryContext,
Collections.emptyList(),
prefetchRequirementCollector,
filteringFormula
);
// now analyze the filter by in a nested context with exchanged primary entity index
return orderByVisitor.executeInContext(
new EntityIndex[] {entityIndex},
entityType,
locale,
new AttributeSchemaAccessor(queryContext.getCatalogSchema(), queryContext.getSchema(entityType)),
EntityAttributeExtractor.INSTANCE,
() -> {
for (OrderConstraint innerConstraint : orderBy.getChildren()) {
innerConstraint.accept(orderByVisitor);
}
// create a deferred sorter that will log the execution time to query telemetry
return new DeferredSorter(
orderByVisitor.getSorter(),
sorter -> {
try {
queryContext.pushStep(QueryPhase.EXECUTION_SORT_AND_SLICE, stepDescriptionSupplier);
return sorter.getAsInt();
} finally {
queryContext.popStep();
// we have to create and trap the nested query context here to carry it along with the sorter
// otherwise the sorter will target and use the incorrectly originally queried (prefetched) entities
try (
final QueryContext nestedQueryContext = entityCollection.createQueryContext(
queryContext,
queryContext.getEvitaRequest().deriveCopyWith(
entityType,
null,
new OrderBy(orderBy.getChildren()),
queryContext.getLocale()
),
queryContext.getEvitaSession()
)
) {
final GlobalEntityIndex entityIndex = entityCollection.getGlobalIndexIfExists().orElse(null);
final Sorter sorter;
if (entityIndex == null) {
sorter = NoSorter.INSTANCE;
} else {
// create a visitor
final OrderByVisitor orderByVisitor = new OrderByVisitor(
nestedQueryContext,
Collections.emptyList(),
prefetchRequirementCollector,
filteringFormula
);
// now analyze the filter by in a nested context with exchanged primary entity index
sorter = orderByVisitor.executeInContext(
new EntityIndex[]{entityIndex},
entityType,
locale,
new AttributeSchemaAccessor(nestedQueryContext.getCatalogSchema(), entityCollection.getSchema()),
EntityAttributeExtractor.INSTANCE,
() -> {
for (OrderConstraint innerConstraint : orderBy.getChildren()) {
innerConstraint.accept(orderByVisitor);
}
// create a deferred sorter that will log the execution time to query telemetry
return new DeferredSorter(
orderByVisitor.getSorter(),
theSorter -> {
try {
nestedQueryContext.pushStep(QueryPhase.EXECUTION_SORT_AND_SLICE, stepDescriptionSupplier);
return theSorter.getAsInt();
} finally {
nestedQueryContext.popStep();
}
}
);
}
);
}
);
return new NestedContextSorter(
nestedQueryContext, sorter
);
}
} finally {
queryContext.popStep();
}
Expand Down
22 changes: 11 additions & 11 deletions ...io/evitadb/core/query/extraResult/translator/facet/FacetSummaryOfReferenceTranslator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@
import io.evitadb.core.query.extraResult.translator.facet.producer.FilteringFormulaPredicate;
import io.evitadb.core.query.extraResult.translator.reference.EntityFetchTranslator;
import io.evitadb.core.query.indexSelection.TargetIndexes;
import io.evitadb.core.query.sort.NestedContextSorter;
import io.evitadb.core.query.sort.NoSorter;
import io.evitadb.core.query.sort.Sorter;
import io.evitadb.index.EntityIndex;
import io.evitadb.index.bitmap.Bitmap;
import io.evitadb.index.bitmap.collection.BitmapIntoBitmapCollector;
Expand Down Expand Up @@ -159,7 +159,7 @@ static IntPredicate createFacetPredicate(
* @return the created facet sorter, or null if the reference schema is not managed and sorting is not required
*/
@Nullable
static Sorter createFacetSorter(
static NestedContextSorter createFacetSorter(
@Nonnull OrderBy orderBy,
@Nullable Locale locale,
@Nonnull ExtraResultPlanningVisitor extraResultPlanner,
Expand All @@ -175,16 +175,16 @@ static Sorter createFacetSorter(
} else if (!referenceSchema.isReferencedEntityTypeManaged()) {
return null;
}
return extraResultPlanner.getGlobalEntityIndexIfExists(referenceSchema.getReferencedEntityType())
.map(ix -> extraResultPlanner.createSorter(
return extraResultPlanner.getEntityCollection(referenceSchema.getReferencedEntityType())
.map(collection -> extraResultPlanner.createSorter(
orderBy,
locale,
ix,
collection,
referenceSchema.getReferencedEntityType(),
() -> "Facet summary `" + referenceSchema.getName() + "` facet ordering: " + orderBy
)
)
.orElse(NoSorter.INSTANCE);
.orElseGet(() -> new NestedContextSorter(extraResultPlanner.getQueryContext(), NoSorter.INSTANCE));
}

/**
Expand All @@ -198,7 +198,7 @@ static Sorter createFacetSorter(
* @return The created sorter for facet group ordering, or null if not required.
*/
@Nullable
static Sorter createFacetGroupSorter(
static NestedContextSorter createFacetGroupSorter(
@Nullable OrderGroupBy orderBy,
@Nullable Locale locale,
@Nonnull ExtraResultPlanningVisitor extraResultPlanner,
Expand All @@ -215,16 +215,16 @@ static Sorter createFacetGroupSorter(
return null;
}

return extraResultPlanner.getGlobalEntityIndexIfExists(referenceSchema.getReferencedGroupType())
.map(ix -> extraResultPlanner.createSorter(
return extraResultPlanner.getEntityCollection(referenceSchema.getReferencedGroupType())
.map(collection -> extraResultPlanner.createSorter(
orderBy,
locale,
ix,
collection,
referenceSchema.getReferencedGroupType(),
() -> "Facet summary `" + referenceSchema.getName() + "` group ordering: " + orderBy
)
)
.orElse(NoSorter.INSTANCE);
.orElseGet(() -> new NestedContextSorter(extraResultPlanner.getQueryContext(), NoSorter.INSTANCE));
}

/**
Expand Down
Loading

0 comments on commit 41e081c

Please sign in to comment.