Skip to content

Commit

Permalink
feat: add in filter criterion (linkedin#113)
Browse files Browse the repository at this point in the history 
* feat: add in filter criterion

* validate in condition corresponds to string array

* add check that array elements are string type

* add tests and remove string check
  • Loading branch information
@kaliang1
kaliang1 authored Jul 12, 2021
1 parent 3a6860e commit 603e758
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 2 deletions.
5 changes: 5 additions & 0 deletions dao-api/src/main/pegasus/com/linkedin/metadata/query/Condition.pdl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ enum Condition {
*/
GREATER_THAN_OR_EQUAL_TO

/**
* Represent the relation: String field is one of the array values to, e.g. name in ["Profile", "Event"]
*/
IN

/**
* Represent the relation less than, e.g. ownerCount < 3
*/
Expand Down
1 change: 1 addition & 0 deletions dao-api/src/main/pegasus/com/linkedin/metadata/query/IndexValue.pdl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ namespace com.linkedin.metadata.query
* A union of all supported value types in the index table
*/
typeref IndexValue = union[
array[string],
boolean,
double
float,
Expand Down
29 changes: 28 additions & 1 deletion dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalDAO.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import java.net.URISyntaxException;
import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
Expand Down Expand Up @@ -94,6 +95,7 @@ static class GMAIndexPair {
put(Condition.EQUAL, "=");
put(Condition.GREATER_THAN, ">");
put(Condition.GREATER_THAN_OR_EQUAL_TO, ">=");
put(Condition.IN, "IN");
put(Condition.LESS_THAN, "<");
put(Condition.LESS_THAN_OR_EQUAL_TO, "<=");
put(Condition.START_WITH, "LIKE");
Expand Down Expand Up @@ -907,6 +909,9 @@ static GMAIndexPair getGMAIndexPair(@Nonnull IndexCriterion criterion) {
} else if (indexValue.isString()) {
object = getValueFromIndexCriterion(criterion);
return new GMAIndexPair(EbeanMetadataIndex.STRING_COLUMN, object);
} else if (indexValue.isArray() && indexValue.getArray().size() > 0 && indexValue.getArray().get(0).getClass() == String.class) {
object = indexValue.getArray();
return new GMAIndexPair(EbeanMetadataIndex.STRING_COLUMN, object);
} else {
throw new IllegalArgumentException("Invalid index value " + indexValue);
}
Expand Down Expand Up @@ -958,6 +963,16 @@ private static String getStringForOperator(@Nonnull Condition condition) {
return CONDITION_STRING_MAP.get(condition);
}

@Nonnull
private static void validateConditionAndValue(@Nonnull IndexCriterion criterion) {
final Condition condition = criterion.getPathParams().getCondition();
final IndexValue indexValue = criterion.getPathParams().getValue();

if (condition == Condition.IN && (!indexValue.isArray() || indexValue.getArray().size() == 0)) {
throw new IllegalArgumentException("Invalid condition " + condition + " for index value " + indexValue);
}
}

@Nonnull
static <ASPECT extends RecordTemplate> String getSortingColumn(@Nonnull IndexSortCriterion indexSortCriterion) {
final String[] pathSpecArray = RecordUtils.getPathSpecAsArray(indexSortCriterion.getPath());
Expand Down Expand Up @@ -998,6 +1013,17 @@ static <ASPECT extends RecordTemplate> String getSortingColumn(@Nonnull IndexSor
}
}

private static String getPlaceholderStringForValue(@Nonnull IndexValue indexValue) {
if (indexValue.isArray() && indexValue.getArray().size() > 0) {
List<Object> values = Arrays.asList(indexValue.getArray().toArray());
String placeholderString = "(";
placeholderString += String.join(",", values.stream().map(value -> "?").collect(Collectors.toList()));
placeholderString += ")";
return placeholderString;
}
return "?";
}

/**
* Constructs SQL query that contains positioned parameters (with `?`), based on whether {@link IndexCriterion} of
* a given condition has field `pathParams`.
Expand Down Expand Up @@ -1025,6 +1051,7 @@ private static String constructSQLQuery(@Nonnull IndexCriterionArray indexCriter

whereClause.append(" AND t").append(i).append(".aspect = ?");
if (criterion.getPathParams() != null) {
validateConditionAndValue(criterion);
whereClause.append(" AND t")
.append(i)
.append(".path = ? AND t")
Expand All @@ -1033,7 +1060,7 @@ private static String constructSQLQuery(@Nonnull IndexCriterionArray indexCriter
.append(getGMAIndexPair(criterion).valueType)
.append(" ")
.append(getStringForOperator(criterion.getPathParams().getCondition()))
.append("?");
.append(getPlaceholderStringForValue(criterion.getPathParams().getValue()));
}
});
final String orderByClause;
Expand Down
67 changes: 66 additions & 1 deletion dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/EbeanLocalDAOTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import com.linkedin.common.urn.Urn;
import com.linkedin.common.urn.Urns;
import com.linkedin.data.template.RecordTemplate;
import com.linkedin.data.template.StringArray;
import com.linkedin.metadata.backfill.BackfillMode;
import com.linkedin.metadata.dao.equality.AlwaysFalseEqualityTester;
import com.linkedin.metadata.dao.equality.DefaultEqualityTester;
Expand Down Expand Up @@ -991,6 +992,63 @@ public void testSorting() {
assertEquals(urns5, Arrays.asList(urn3, urn2, urn1));
}

@Test
public void testIn() {
EbeanLocalDAO<EntityAspectUnion, FooUrn> dao = createDao(FooUrn.class);
dao.enableLocalSecondaryIndex(true);
FooUrn urn1 = makeFooUrn(1);
FooUrn urn2 = makeFooUrn(2);
FooUrn urn3 = makeFooUrn(3);
String aspect = "aspect1" + System.currentTimeMillis();

addIndex(urn1, aspect, "/path1", "foo");
addIndex(urn1, FooUrn.class.getCanonicalName(), "/fooId", 1);

addIndex(urn2, aspect, "/path1", "baz");
addIndex(urn2, FooUrn.class.getCanonicalName(), "/fooId", 2);

addIndex(urn3, aspect, "/path1", "val");
addIndex(urn3, FooUrn.class.getCanonicalName(), "/fooId", 3);

IndexValue indexValue1 = new IndexValue();
indexValue1.setArray(new StringArray("foo", "baz"));
IndexCriterion criterion1 = new IndexCriterion().setAspect(aspect)
.setPathParams(new IndexPathParams().setPath("/path1").setValue(indexValue1).setCondition(Condition.IN));

IndexCriterionArray indexCriterionArray1 = new IndexCriterionArray(Collections.singletonList(criterion1));
final IndexFilter indexFilter1 = new IndexFilter().setCriteria(indexCriterionArray1);
List<FooUrn> urns1 = dao.listUrns(indexFilter1, null, 5);
assertEquals(urns1, Arrays.asList(urn1, urn2));

IndexValue indexValue2 = new IndexValue();
indexValue2.setArray(new StringArray("a", "b", "c"));
IndexCriterion criterion2 = new IndexCriterion().setAspect(aspect)
.setPathParams(new IndexPathParams().setPath("/path1").setValue(indexValue2).setCondition(Condition.IN));

IndexCriterionArray indexCriterionArray2 = new IndexCriterionArray(Collections.singletonList(criterion2));
final IndexFilter indexFilter2 = new IndexFilter().setCriteria(indexCriterionArray2);
List<FooUrn> urns2 = dao.listUrns(indexFilter2, null, 5);
assertEquals(urns2, Arrays.asList());

IndexValue indexValue3 = new IndexValue();
indexValue3.setString("test");
IndexCriterion criterion3 = new IndexCriterion().setAspect(aspect)
.setPathParams(new IndexPathParams().setPath("/path1").setValue(indexValue3).setCondition(Condition.IN));

IndexCriterionArray indexCriterionArray3 = new IndexCriterionArray(Collections.singletonList(criterion3));
final IndexFilter indexFilter3 = new IndexFilter().setCriteria(indexCriterionArray3);
assertThrows(IllegalArgumentException.class, () -> dao.listUrns(indexFilter3, null, 5));

IndexValue indexValue4 = new IndexValue();
indexValue4.setArray(new StringArray());
IndexCriterion criterion4 = new IndexCriterion().setAspect(aspect)
.setPathParams(new IndexPathParams().setPath("/path1").setValue(indexValue4).setCondition(Condition.IN));

IndexCriterionArray indexCriterionArray4 = new IndexCriterionArray(Collections.singletonList(criterion4));
final IndexFilter indexFilter4 = new IndexFilter().setCriteria(indexCriterionArray4);
assertThrows(IllegalArgumentException.class, () -> dao.listUrns(indexFilter4, null, 5));
}

@Test
public void testListUrns() {
EbeanLocalDAO<EntityAspectUnion, FooUrn> dao = createDao(FooUrn.class);
Expand Down Expand Up @@ -1549,12 +1607,19 @@ void testGetGMAIndexPair() {
gmaIndexPair = EbeanLocalDAO.getGMAIndexPair(indexCriterion);
assertEquals(EbeanMetadataIndex.LONG_COLUMN, gmaIndexPair.valueType);
assertEquals(lVal, gmaIndexPair.value);
// 6/ IndexValue pair corresponds to string
// 6. IndexValue pair corresponds to string
String sVal = "testVal";
indexValue.setString(sVal);
gmaIndexPair = EbeanLocalDAO.getGMAIndexPair(indexCriterion);
assertEquals(EbeanMetadataIndex.STRING_COLUMN, gmaIndexPair.valueType);
assertEquals(sVal, gmaIndexPair.value);
// 7. IndexValue pair corresponds to string array
StringArray sArrVal = new StringArray();
sArrVal.add("testVal");
indexValue.setArray(sArrVal);
gmaIndexPair = EbeanLocalDAO.getGMAIndexPair(indexCriterion);
assertEquals(EbeanMetadataIndex.STRING_COLUMN, gmaIndexPair.valueType);
assertEquals(sArrVal, gmaIndexPair.value);
}

@Test
Expand Down

0 comments on commit 603e758

Please sign in to comment.