Skip to content

Commit

Permalink
MID-7716: fixed new repo search with non-eq op on multi-value extensions
Browse files Browse the repository at this point in the history
This does NOT allow any serious indexing, but makes the search possible.
  • Loading branch information
virgo47 committed Mar 10, 2022
1 parent 12a9dc8 commit 757889d
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 39 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2010-2021 Evolveum and contributors
* Copyright (C) 2010-2022 Evolveum and contributors
*
* This work is dual-licensed under the Apache License 2.0
* and European Union Public License. See LICENSE file for details.
Expand Down Expand Up @@ -98,6 +98,6 @@ private Predicate createPredicate(
}
}

return singleValuePredicate(path, operation, customColumnPropertyType.getValue());
return singleValuePredicateWithNotTreated(path, operation, customColumnPropertyType.getValue());
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2010-2021 Evolveum and contributors
* Copyright (C) 2010-2022 Evolveum and contributors
*
* This work is dual-licensed under the Apache License 2.0
* and European Union Public License. See LICENSE file for details.
Expand All @@ -26,6 +26,8 @@
import com.querydsl.core.types.ExpressionUtils;
import com.querydsl.core.types.Predicate;
import com.querydsl.core.types.dsl.BooleanExpression;
import com.querydsl.core.types.dsl.StringTemplate;
import com.querydsl.sql.SQLQuery;

import com.evolveum.midpoint.prism.*;
import com.evolveum.midpoint.prism.polystring.PolyString;
Expand Down Expand Up @@ -209,17 +211,28 @@ private Predicate processString(
booleanTemplate("{0} @> {1}", path, jsonbValue(extItem, values.singleValue())));
}
} else {
// other non-EQ operations
if (extItem.cardinality == SCALAR) {
// {1s} means "as string", this is replaced before JDBC driver, just as path is,
// but for path types it's automagic, integer would turn to param and ?.
// IMPORTANT: To get string from JSONB we want to use ->> or #>>'{}' operators,
// that properly escape the value. Using ::TEXT cast would return the string with
// double-quotes. For more: https://dba.stackexchange.com/a/234047/157622
return singleValuePredicate(stringTemplate("{0}->>'{1s}'", path, extItem.id),
return singleValuePredicateWithNotTreated(stringTemplate("{0}->>'{1s}'", path, extItem.id),
operation, values.singleValue());
} else if (!values.isMultiValue()) {
// e.g. for substring: WHERE ... ext ? '421'
// AND exists (select val from jsonb_array_elements_text(ext->'421') as val where val ilike '%2%')
// This can't use index, but it functions. Sparse keys are helped a lot by indexed ext ? key condition.
StringTemplate valPath = stringTemplate("val");
SQLQuery<String> subselect = new SQLQuery<>().select(valPath)
.from(stringTemplate("jsonb_array_elements_text({0}->'{1s}') as val", path, extItem.id))
.where(singleValuePredicate(valPath, operation, values.singleValue()));
return booleanTemplate("{0} ?? '{1s}'", path, extItem.id)
.and(subselect.exists());
} else {
throw new QueryException("Only equals is supported for"
+ " multi-value extensions; used filter: " + filter);
throw new QueryException("Non-equal operation not supported for multi-value extensions"
+ " and multiple values on the right-hand; used filter: " + filter);
}
}
}
Expand All @@ -246,7 +259,7 @@ private Predicate processNumeric(
if (extItem.cardinality == SCALAR) {
// {1s} means "as string", this is replaced before JDBC driver, just as path is,
// but for path types it's automagic, integer would turn to param and ?.
return singleValuePredicate(
return singleValuePredicateWithNotTreated(
stringTemplate("({0}->'{1s}')::numeric", path, extItem.id),
operation,
values.singleValue());
Expand Down Expand Up @@ -313,11 +326,11 @@ private Predicate processPolyStringBoth(
JSONB_POLY_NORM_KEY, poly.getNorm()))));
} else if (extItem.cardinality == SCALAR) {
return ExpressionUtils.and(
singleValuePredicate(
singleValuePredicateWithNotTreated(
stringTemplate("{0}->'{1s}'->>'{2s}'",
path, extItem.id, JSONB_POLY_ORIG_KEY),
operation, poly.getOrig()),
singleValuePredicate(
singleValuePredicateWithNotTreated(
stringTemplate("{0}->'{1s}'->>'{2s}'",
path, extItem.id, JSONB_POLY_NORM_KEY),
operation, poly.getNorm()));
Expand All @@ -334,7 +347,7 @@ private Predicate processPolyStringComponent(MExtItem extItem,
return predicateWithNotTreated(path, booleanTemplate("{0} @> {1}", path,
jsonbValue(extItem, Map.of(subKey, Objects.requireNonNull(values.singleValue())))));
} else if (extItem.cardinality == SCALAR) {
return singleValuePredicate(
return singleValuePredicateWithNotTreated(
stringTemplate("{0}->'{1s}'->>'{2s}'", path, extItem.id, subKey),
operation, values.singleValue());
} else {
Expand All @@ -360,9 +373,9 @@ protected boolean isIgnoreCaseFilter(ValueFilter<?, ?> filter) {
private BooleanExpression extItemIsNull(MExtItem extItem) {
// ?? is "escaped" ? operator, PG JDBC driver understands it. Alternative is to use
// function jsonb_exists but that does NOT use GIN index, only operators do!
// We have to use parenthesis with AND shovelled into the template like this.
return booleanTemplate("({0} ?? {1} AND {0} is not null)",
path, extItem.id.toString()).not();
// We have to use parenthesis with AND shovelled into the template like this to apply NOT to it all.
return booleanTemplate("({0} ?? '{1s}' AND {0} is not null)",
path, extItem.id).not();
}

private String extractNorm(Object value) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1275,27 +1275,34 @@ public void test512SearchObjectWithoutExtensionMultivalueStringItem() throws Sch
}

@Test
public void test515SearchObjectWithMultivalueExtensionUsingNonEqualFilterFails() {
given("query for multi-value extension string item with non-equal operation");
OperationResult operationResult = createOperationResult();
ObjectQuery query = prismContext.queryFor(UserType.class)
.item(UserType.F_EXTENSION, new QName("string-mv")).gt("string-value2")
.build();

expect("searchObjects throws exception because of unsupported filter");
assertThatThrownBy(() -> searchObjects(UserType.class, query, operationResult))
.isInstanceOf(SystemException.class)
.hasMessageContaining("supported");
}

@Test
public void test516SearchObjectHavingAnyOfSpecifiedMultivalueStringExtension() throws SchemaException {
public void test513SearchObjectHavingAnyOfSpecifiedMultivalueStringExtension() throws SchemaException {
searchUsersTest("with multi-value extension string matching any of provided values",
f -> f.item(UserType.F_EXTENSION, new QName("string-mv"))
.eq("string-value2", "string-valueX"), // second value does not match, but that's OK
user1Oid, user2Oid); // both users have "string-value2" in "string-mv"
}

@Test
public void test515SearchObjectWithMultivalueExtensionUsingSubstring() throws SchemaException {
searchUsersTest("with multi-value extension string item with substring operation",
f -> f.item(UserType.F_EXTENSION, new QName("string-mv")).contains("1"), // matches string-value1
user1Oid);
}

@Test
public void test516SearchObjectWithMultivalueExtensionUsingComparison() throws SchemaException {
searchUsersTest("with multi-value extension string item with comparison operation",
f -> f.item(UserType.F_EXTENSION, new QName("string-mv")).gt("string-value2"),
user2Oid);
}

@Test
public void test517SearchObjectWithMultivalueExtensionUsingComparisonNegated() throws SchemaException {
searchUsersTest("with multi-value extension string item with NOT comparison operation",
f -> f.not().item(UserType.F_EXTENSION, new QName("string-mv")).gt("string-value2"),
creatorOid, modifierOid, user1Oid, user3Oid, user4Oid);
}

// integer tests
@Test
public void test520SearchObjectHavingSpecifiedIntegerExtension() throws SchemaException {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2010-2021 Evolveum and contributors
* Copyright (C) 2010-2022 Evolveum and contributors
*
* This work is dual-licensed under the Apache License 2.0
* and European Union Public License. See LICENSE file for details.
Expand Down Expand Up @@ -100,6 +100,6 @@ private Predicate createPredicate(
}
}

return singleValuePredicate(path, operation, customColumnPropertyType.getValue());
return singleValuePredicateWithNotTreated(path, operation, customColumnPropertyType.getValue());
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2010-2021 Evolveum and contributors
* Copyright (C) 2010-2022 Evolveum and contributors
*
* This work is dual-licensed under the Apache License 2.0
* and European Union Public License. See LICENSE file for details.
Expand Down Expand Up @@ -103,7 +103,7 @@ protected <T> Predicate createBinaryCondition(
}
}

return singleValuePredicate(path, operation, values.singleValue());
return singleValuePredicateWithNotTreated(path, operation, values.singleValue());
}

/**
Expand All @@ -112,6 +112,13 @@ protected <T> Predicate createBinaryCondition(
* otherwise the expression is passed as-is.
* Technically, any expression can be used on path side as well.
*/
protected Predicate singleValuePredicateWithNotTreated(
Expression<?> path, FilterOperation operation, Object value) {
Predicate predicate = singleValuePredicate(path, operation, value);
return predicateWithNotTreated(path, predicate);
}

/** Like {@link #singleValuePredicateWithNotTreated} but skips NOT treatment. */
protected Predicate singleValuePredicate(
Expression<?> path, FilterOperation operation, Object value) {
path = operation.treatPath(path);
Expand All @@ -120,9 +127,8 @@ protected Predicate singleValuePredicate(
} else {
value = operation.treatValue(value);
}
Predicate predicate = ExpressionUtils.predicate(operation.operator, path,
return ExpressionUtils.predicate(operation.operator, path,
value instanceof Expression ? (Expression<?>) value : ConstantImpl.create(value));
return predicateWithNotTreated(path, predicate);
}

/**
Expand All @@ -139,5 +145,4 @@ protected Predicate predicateWithNotTreated(Expression<?> path, Predicate predic
public Expression<?> rightHand(ValueFilter<?, ?> filter) throws RepositoryException {
throw new RepositoryException("Path " + filter.getRightHandSidePath() + "is not supported as right hand side.");
}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2010-2021 Evolveum and contributors
* Copyright (C) 2010-2022 Evolveum and contributors
*
* This work is dual-licensed under the Apache License 2.0
* and European Union Public License. See LICENSE file for details.
Expand Down Expand Up @@ -137,12 +137,12 @@ public Expression<?> rightHand(ValueFilter<?, ?> filter) throws RepositoryExcept
public Predicate process(PropertyValueFilter<T> filter, RightHandProcessor rightPath) throws RepositoryException {
FilterOperation operation = operation(filter);
if (rightPath instanceof PolyStringItemFilterProcessor) {
return processPoly(filter, (PolyStringItemFilterProcessor) rightPath);
return processPoly(filter, (PolyStringItemFilterProcessor<?>) rightPath);
}
return singleValuePredicate(this.normPath, operation, rightPath.rightHand(filter));
return singleValuePredicateWithNotTreated(this.normPath, operation, rightPath.rightHand(filter));
}

private Predicate processPoly(PropertyValueFilter<T> filter, PolyStringItemFilterProcessor rightPath) throws QueryException {
private Predicate processPoly(PropertyValueFilter<T> filter, PolyStringItemFilterProcessor<?> rightPath) throws QueryException {
String matchingRule = filter.getMatchingRule() != null
? filter.getMatchingRule().getLocalPart() : null;

Expand Down

0 comments on commit 757889d

Please sign in to comment.