Skip to content

Commit

Permalink
Merge pull request #547 from FgForrest/dev
Browse files Browse the repository at this point in the history
Hotfix for insufficient #537 correction
  • Loading branch information
novoj authored Apr 29, 2024
2 parents f53594f + 4225b35 commit 5ac37a4
Show file tree
Hide file tree
Showing 18 changed files with 180 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,12 @@ public Bitmap compute() {

@Override
public String toString() {
return "FLATTENED: " + memoizedResult.size() + " primary keys";
}

@Nonnull
@Override
public String toStringVerbose() {
return "FLATTENED: " + memoizedResult.toString();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -277,9 +277,9 @@ private static <T extends Index<?>> List<QueryPlanBuilder> createFilterFormula(
queryContext, adeptFormula, targetIndex, prefetchFormulaVisitor
);
if (result.isEmpty() || adeptFormula.getEstimatedCost() < result.get(0).getEstimatedCost()) {
result.addLast(queryPlanBuilder);
} else {
result.addFirst(queryPlanBuilder);
} else {
result.addLast(queryPlanBuilder);
}
} finally {
if (adeptFormula == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,12 @@ public String prettyPrint() {
return PrettyPrintingFormulaVisitor.toString(this);
}

@Nonnull
@Override
public String toStringVerbose() {
return toString();
}

/**
* Method signalizes whether the {@link #innerFormulas} order is significant in this formula. Usually the order
* is not significant, and we want to order the hashes by their value to avoid different hashes for formula
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,4 +87,11 @@ public interface Formula extends TransactionalDataRelatedStructure, PrettyPrinta
* Clears the memoized results and hashes of the formula.
*/
void clearMemory();

/**
* Prints information about the formula in a user-friendly way in verbose mode.
*/
@Nonnull
String toStringVerbose();

}
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,12 @@ protected Bitmap computeInternal() {

@Override
public String toString() {
return delegate.toString();
return delegate.size() + " primary keys";
}

@Nonnull
@Override
public String toStringVerbose() {
return delegate.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,16 @@ public long getOperationCost() {

@Override
public String toString() {
if (mainBitmap != null && controlBitmap != null) {
return "DISENTANGLE: main" + mainBitmap.size() + ", control: " + controlBitmap.size() + " primary keys";
} else {
return "DISENTANGLE";
}
}

@Nonnull
@Override
public String toStringVerbose() {
if (mainBitmap != null && controlBitmap != null) {
return "DISENTANGLE: " + Stream.of(mainBitmap, controlBitmap).map(Bitmap::toString).collect(Collectors.joining(", "));
} else {
Expand Down Expand Up @@ -241,7 +251,7 @@ protected Bitmap computeInternal() {
PRIVATE METHODS
*/

private int computeNextInt(OfInt mainIt, OfInt controlIt, AtomicInteger controlNumberRef) {
private static int computeNextInt(OfInt mainIt, OfInt controlIt, AtomicInteger controlNumberRef) {
if (mainIt.hasNext()) {
do {
final int nextNumberAdept = mainIt.next();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,12 @@ public Formula getAsOrFormula() {

@Override
public String toString() {
return "JOIN: " + Arrays.stream(bitmaps).map(it -> String.valueOf(it.size())).collect(Collectors.joining(", ")) + " primary keys";
}

@Nonnull
@Override
public String toStringVerbose() {
return "JOIN: " + Arrays.stream(bitmaps).map(Bitmap::toString).collect(Collectors.joining(", "));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,20 @@ public FacetGroupFormula mergeWith(@Nonnull FacetGroupFormula anotherFormula) {

@Override
public String toString() {
final StringBuilder sb = new StringBuilder("FACET " + referenceName + " AND (" + ofNullable(facetGroupId).map(Object::toString).orElse("-") + " - " + facetIds.toString() + "): ");
for (int i = 0; i < bitmaps.length; i++) {
final Bitmap bitmap = bitmaps[i];
sb.append(" ↦ ").append(bitmap.size());
if (i + 1 < facetIds.size()) {
sb.append(", ");
}
}
return sb.append(" primary keys").toString();
}

@Nonnull
@Override
public String toStringVerbose() {
final StringBuilder sb = new StringBuilder("FACET " + referenceName + " AND (" + ofNullable(facetGroupId).map(Object::toString).orElse("-") + " - " + facetIds.toString() + "): ");
for (int i = 0; i < bitmaps.length; i++) {
final Bitmap bitmap = bitmaps[i];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,20 @@ public FacetGroupFormula mergeWith(@Nonnull FacetGroupFormula anotherFormula) {

@Override
public String toString() {
final StringBuilder sb = new StringBuilder("FACET " + referenceName + " OR (" + ofNullable(facetGroupId).map(Object::toString).orElse("-") + " - " + facetIds.toString() + "): ");
for (int i = 0; i < bitmaps.length; i++) {
final Bitmap bitmap = bitmaps[i];
sb.append(" ↦ ").append(bitmap.size());
if (i + 1 < facetIds.size()) {
sb.append(", ");
}
}
return sb.append(" primary keys").toString();
}

@Nonnull
@Override
public String toStringVerbose() {
final StringBuilder sb = new StringBuilder("FACET " + referenceName + " OR (" + ofNullable(facetGroupId).map(Object::toString).orElse("-") + " - " + facetIds.toString() + "): ");
for (int i = 0; i < bitmaps.length; i++) {
final Bitmap bitmap = bitmaps[i];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,14 @@ protected long getClassId() {
return CLASS_ID;
}

@Override
public String toString() {
return "LOCALE: " + super.toString();
}

@Nonnull
@Override
public String toStringVerbose() {
return "LOCALE: " + super.toStringVerbose();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,13 @@ public boolean isWithin(@Nonnull Predicate<Formula> formulaTester) {
return false;
}

/**
* Returns true if all parents match the passed predicate.
*/
public boolean allParentsMatch(@Nonnull Predicate<Formula> formulaTester) {
return parents.stream().allMatch(formulaTester);
}

/**
* Returns true if there is at least single parent formula of passed `formulaType` for currently visited formula.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ public class PrettyPrintingFormulaVisitor implements FormulaVisitor {
* Indentation used for distinguishing inner formulas in the tree.
*/
private final int indent;
/**
* Contains true if the calculated results should contain precise primary keys.
*/
private final PrettyPrintStyle style;
/**
* This map keeps track of already seen formulas so that we can mark duplicate instances in tree as references
* to them.
Expand All @@ -57,17 +61,33 @@ public class PrettyPrintingFormulaVisitor implements FormulaVisitor {

public PrettyPrintingFormulaVisitor() {
this.indent = 3;
this.style = PrettyPrintStyle.NORMAL;
}

public PrettyPrintingFormulaVisitor(int indent) {
this.indent = indent;
this.style = PrettyPrintStyle.NORMAL;
}

public PrettyPrintingFormulaVisitor(int indent, @Nonnull PrettyPrintStyle style) {
this.indent = indent;
this.style = style;
}

/**
* Preferred way of invoking this visitor. Accepts formula (tree) and produces description string.
*/
public static String toString(@Nonnull Formula formula) {
final PrettyPrintingFormulaVisitor visitor = new PrettyPrintingFormulaVisitor(3);
formula.accept(visitor);
return visitor.getResult();
}

/**
* Preferred way of invoking this visitor. Accepts formula (tree) and produces description string.
*/
public static String toString(Formula formula) {
final PrettyPrintingFormulaVisitor visitor = new PrettyPrintingFormulaVisitor();
public static String toStringVerbose(@Nonnull Formula formula) {
final PrettyPrintingFormulaVisitor visitor = new PrettyPrintingFormulaVisitor(3, PrettyPrintStyle.VERBOSE);
formula.accept(visitor);
return visitor.getResult();
}
Expand All @@ -83,9 +103,14 @@ public void visit(@Nonnull Formula formula) {
formulasSeen.put(formula, new FormulaInstance(id, formula));
result.append("[#").append(id).append("] ");
}
result.append(formula);
if (style == PrettyPrintStyle.VERBOSE) {
result.append(formula.toStringVerbose());
} else {
result.append(formula);
}
if (formula.getInnerFormulas().length > 0) {
result.append(" → ").append(formula.compute());
result.append(" → ")
.append(style == PrettyPrintStyle.VERBOSE ? formula.compute() : " result count " + formula.compute().size());
}
result.append("\n");
level++;
Expand All @@ -108,4 +133,20 @@ private static class FormulaInstance {
private final Formula formula;
}

/**
* Defines the style of pretty-printing.
*/
public enum PrettyPrintStyle {

/**
* Output is concise and contains only necessary information.
*/
NORMAL,
/**
* Output is verbose and contains all possible information.
*/
VERBOSE

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,6 @@ public static Formula optimize(@Nonnull Formula inputFormula) {
final FilterFormulaFacetOptimizeVisitor visitor = new FilterFormulaFacetOptimizeVisitor();
inputFormula.accept(visitor);
return visitor.userFormulaFound ?
visitor.getResult() : FormulaFactory.and(inputFormula, new UserFilterFormula());
visitor.getResult() : FormulaFactory.and(visitor.getResult(), new UserFilterFormula());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import io.evitadb.core.query.algebra.facet.UserFilterFormula;
import io.evitadb.core.query.algebra.utils.FormulaFactory;
import io.evitadb.core.query.algebra.utils.visitor.FormulaCloner;
import io.evitadb.core.query.filter.FilterByVisitor;
import io.evitadb.core.query.filter.translator.facet.FacetHavingTranslator;
import io.evitadb.dataType.array.CompositeObjectArray;
import io.evitadb.exception.EvitaInternalError;
Expand Down Expand Up @@ -329,31 +330,33 @@ private static Formula[] addNewFormulaAsConjunction(@Nonnull Formula newFormula,
// iterate over existing children
final AtomicBoolean childrenAltered = new AtomicBoolean();
for (int i = 0; i < children.length; i++) {
final Formula mutatedChild = FormulaCloner.clone(children[i], examinedFormula -> {
// and if existing AND formula is found
if (examinedFormula instanceof AndFormula) {
// simply add new facet group formula to the AND formula
return examinedFormula.getCloneWithInnerFormulas(
ArrayUtils.insertRecordIntoArray(
newFormula, examinedFormula.getInnerFormulas(), examinedFormula.getInnerFormulas().length
)
);
} else if (examinedFormula instanceof final CombinedFacetFormula combinedFacetFormula) {
// if combined facet formula is found - we know there is combination of AND and OR formulas inside
// take the AND part of the combined formula
final Formula andFormula = combinedFacetFormula.getAndFormula();
// and replace combined formula with OR part untouched and AND part enriched with new facet formula
return examinedFormula.getCloneWithInnerFormulas(
FormulaFactory.and(
andFormula,
newFormula
),
combinedFacetFormula.getOrFormula()
);
} else {
return examinedFormula;
}
});
final Formula mutatedChild = FormulaCloner.clone(
children[i],
(cloner, examinedFormula) -> {
// and if existing AND formula is found
if (examinedFormula instanceof AndFormula && cloner.allParentsMatch(formula -> FilterByVisitor.isConjunctiveFormula(formula.getClass()))) {
// simply add new facet group formula to the AND formula
return examinedFormula.getCloneWithInnerFormulas(
ArrayUtils.insertRecordIntoArray(
newFormula, examinedFormula.getInnerFormulas(), examinedFormula.getInnerFormulas().length
)
);
} else if (examinedFormula instanceof final CombinedFacetFormula combinedFacetFormula && cloner.allParentsMatch(formula -> FilterByVisitor.isConjunctiveFormula(formula.getClass()))) {
// if combined facet formula is found - we know there is combination of AND and OR formulas inside
// take the AND part of the combined formula
final Formula andFormula = combinedFacetFormula.getAndFormula();
// and replace combined formula with OR part untouched and AND part enriched with new facet formula
return examinedFormula.getCloneWithInnerFormulas(
FormulaFactory.and(
andFormula,
newFormula
),
combinedFacetFormula.getOrFormula()
);
} else {
return examinedFormula;
}
});

if (mutatedChild != children[i]) {
children[i] = mutatedChild;
Expand Down Expand Up @@ -642,7 +645,7 @@ protected MutableFormula createNewFacetGroupFormula() {
/**
* Method stores formula to the result of the visitor on current {@link #levelStack}.
*/
protected void storeFormula(Formula formula) {
protected void storeFormula(@Nonnull Formula formula) {
// store updated formula
if (levelStack.isEmpty()) {
this.result = formula;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,12 @@ public String toString() {
return getInnerFormula().toString();
}

@Nonnull
@Override
public String toStringVerbose() {
return getInnerFormula().toStringVerbose();
}

/**
* Returns the inner formula. If the pivot is set, the result is merged with the pivot.
* @return the inner formula
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public void visit(@Nonnull Formula formula) {

// we found formula to optimize for - duplicate current container with separate sub container
// for all other formulas except the one we optimize for
if (optimizationSet.contains(formula) && !(formula instanceof NotFormula)) {
if (optimizationSet.contains(formula) && !(formula instanceof NotFormula) && updatedChildren.length > 2) {

final CompositeObjectArray<Formula> newDivertedFormulas = new CompositeObjectArray<>(Formula.class);
final CompositeObjectArray<Formula> matchingFormulas = new CompositeObjectArray<>(Formula.class);
Expand Down Expand Up @@ -178,8 +178,8 @@ private void replaceFormula(Formula formula) {
if (levelStack.isEmpty()) {
this.result = formula;
} else {
levelStack.peek().add(formula);
optimizationSet.add(formula);
this.levelStack.peek().add(formula);
this.optimizationSet.add(formula);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ void shouldOptimizeBaseQuery() {
[#10] FACET BRAND OR (1 - [1]): ↦ [1]
[#11] FACET STORE OR (1 - [1]): ↦ [2]
""",
PrettyPrintingFormulaVisitor.toString(optimizedFormula)
PrettyPrintingFormulaVisitor.toStringVerbose(optimizedFormula)
);
}

Expand Down Expand Up @@ -128,7 +128,7 @@ void shouldOptimizeComplexBaseQuery() {
[#17] FACET BRAND OR (1 - [2]): ↦ [7]
[#18] FACET STORE OR (1 - [3]): ↦ [9]
""",
PrettyPrintingFormulaVisitor.toString(optimizedFormula)
PrettyPrintingFormulaVisitor.toStringVerbose(optimizedFormula)
);
}

Expand Down
Loading

0 comments on commit 5ac37a4

Please sign in to comment.