From 603e758942e71c7800e5f64ea206ba93d8407f46 Mon Sep 17 00:00:00 2001 From: Kathleen Liang <85576784+kaliang1@users.noreply.github.com> Date: Mon, 12 Jul 2021 16:51:52 -0700 Subject: [PATCH] feat: add in filter criterion (#113) * 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 --- .../com/linkedin/metadata/query/Condition.pdl | 5 ++ .../linkedin/metadata/query/IndexValue.pdl | 1 + .../linkedin/metadata/dao/EbeanLocalDAO.java | 29 +++++++- .../metadata/dao/EbeanLocalDAOTest.java | 67 ++++++++++++++++++- 4 files changed, 100 insertions(+), 2 deletions(-) diff --git a/dao-api/src/main/pegasus/com/linkedin/metadata/query/Condition.pdl b/dao-api/src/main/pegasus/com/linkedin/metadata/query/Condition.pdl index 6c7e32ab7..9fc214e8c 100644 --- a/dao-api/src/main/pegasus/com/linkedin/metadata/query/Condition.pdl +++ b/dao-api/src/main/pegasus/com/linkedin/metadata/query/Condition.pdl @@ -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 */ diff --git a/dao-api/src/main/pegasus/com/linkedin/metadata/query/IndexValue.pdl b/dao-api/src/main/pegasus/com/linkedin/metadata/query/IndexValue.pdl index 3a8085048..ce1753de3 100644 --- a/dao-api/src/main/pegasus/com/linkedin/metadata/query/IndexValue.pdl +++ b/dao-api/src/main/pegasus/com/linkedin/metadata/query/IndexValue.pdl @@ -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, diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalDAO.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalDAO.java index 5ec83a6c7..9bd3ba15b 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalDAO.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalDAO.java @@ -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; @@ -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"); @@ -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); } @@ -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 String getSortingColumn(@Nonnull IndexSortCriterion indexSortCriterion) { final String[] pathSpecArray = RecordUtils.getPathSpecAsArray(indexSortCriterion.getPath()); @@ -998,6 +1013,17 @@ static String getSortingColumn(@Nonnull IndexSor } } + private static String getPlaceholderStringForValue(@Nonnull IndexValue indexValue) { + if (indexValue.isArray() && indexValue.getArray().size() > 0) { + List 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`. @@ -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") @@ -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; diff --git a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/EbeanLocalDAOTest.java b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/EbeanLocalDAOTest.java index 2d0bd3e84..0382a1c4c 100644 --- a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/EbeanLocalDAOTest.java +++ b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/EbeanLocalDAOTest.java @@ -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; @@ -991,6 +992,63 @@ public void testSorting() { assertEquals(urns5, Arrays.asList(urn3, urn2, urn1)); } + @Test + public void testIn() { + EbeanLocalDAO 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 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 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 dao = createDao(FooUrn.class); @@ -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