Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat : support collection type in secondary index #1474

Merged
merged 1 commit into from
Jul 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,15 @@ public List<Condition.Relation> relations() {
return relations;
}

public Relation relation(Id key){
for (Relation r : this.relations()) {
if (r.key().equals(key)) {
return r;
}
}
return null;
}

@Watched
public <T> T condition(Object key) {
List<Object> values = new ArrayList<>();
Expand Down Expand Up @@ -209,6 +218,16 @@ public boolean containsScanCondition() {
return this.containsCondition(Condition.RelationType.SCAN);
}

public boolean containsContainsCondition(Id key) {
for (Relation r : this.relations()) {
if (r.key().equals(key)) {
return r.relation().equals(RelationType.CONTAINS) ||
r.relation().equals(RelationType.TEXT_CONTAINS);
}
}
return false;
}

public boolean allSysprop() {
for (Condition c : this.conditions) {
if (!c.isSysprop()) {
Expand Down Expand Up @@ -310,10 +329,11 @@ public String userpropValuesString(List<Id> fields) {
boolean got = false;
for (Relation r : this.userpropRelations()) {
if (r.key().equals(field) && !r.isSysprop()) {
E.checkState(r.relation == RelationType.EQ,
E.checkState(r.relation == RelationType.EQ ||
r.relation == RelationType.CONTAINS,
"Method userpropValues(List<String>) only " +
"used for secondary index, " +
"relation must be EQ, but got %s",
"relation must be EQ or CONTAINS, but got %s",
r.relation());
values.add(r.serialValue());
got = true;
Expand Down Expand Up @@ -507,14 +527,29 @@ public static String concatValues(List<Object> values) {
return SplicingIdGenerator.concatValues(newValues);
}

public static String concatValues(Object value) {
if (value instanceof List) {
return concatValues((List<Object>)value);
jadepeng marked this conversation as resolved.
Show resolved Hide resolved
}

if (needConvertNumber(value)) {
return LongEncoding.encodeNumber(value);
}
return value.toString();
}

private static Object convertNumberIfNeeded(Object value) {
if (NumericUtil.isNumber(value) || value instanceof Date) {
// Numeric or date values should be converted to string
if (needConvertNumber(value)) {
return LongEncoding.encodeNumber(value);
}
return value;
}

private static boolean needConvertNumber(Object value) {
// Numeric or date values should be converted to number from string
return NumericUtil.isNumber(value) || value instanceof Date;
}

public enum OptimizedType {
NONE,
PRIMARY_KEY,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import java.util.function.Function;
import java.util.stream.Collectors;

import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.util.Strings;
import org.apache.tinkerpop.gremlin.structure.Edge;
import org.apache.tinkerpop.gremlin.structure.Vertex;
Expand Down Expand Up @@ -227,20 +228,36 @@ protected void updateIndex(Id ilId, HugeElement element, boolean removed) {
E.checkState(propValues.size() == 1,
"Expect only one property in search index");
value = propValues.get(0);
Set<String> words = this.segmentWords(value.toString());
Set<String> words =
this.segmentWords(propertyValueToString(value));
for (String word : words) {
this.updateIndex(indexLabel, word, element.id(),
expiredTime, removed);
}
break;
case SECONDARY:
// Secondary index maybe include multi prefix index
for (int i = 0, n = propValues.size(); i < n; i++) {
List<Object> prefixValues = propValues.subList(0, i + 1);
value = ConditionQuery.concatValues(prefixValues);
value = escapeIndexValueIfNeeded((String) value);
this.updateIndex(indexLabel, value, element.id(),
expiredTime, removed);
if (isCollectionIndex(propValues)) {
/*
* Property value is a collection
* we should create index for each item
*/
for (Object propValue :
(Collection<Object>) propValues.get(0)) {
value = ConditionQuery.concatValues(propValue);
value = escapeIndexValueIfNeeded((String) value);
this.updateIndex(indexLabel, value, element.id(),
expiredTime, removed);
}
} else {
for (int i = 0, n = propValues.size(); i < n; i++) {
List<Object> prefixValues =
propValues.subList(0, i + 1);
value = ConditionQuery.concatValues(prefixValues);
value = escapeIndexValueIfNeeded((String) value);
this.updateIndex(indexLabel, value, element.id(),
expiredTime, removed);
}
}
break;
case SHARD:
Expand Down Expand Up @@ -766,9 +783,11 @@ private ConditionQuery constructSearchQuery(ConditionQuery query,
if (key instanceof Id && indexFields.contains(key)) {
// This is an index field of search index
Id field = (Id) key;
String propValue = elem.<String>getPropertyValue(field);
String fvalue = (String) originQuery.userpropValue(field);
if (this.matchSearchIndexWords(propValue, fvalue)) {
assert elem != null;
HugeProperty<?> property = elem.getProperty(field);
String propValue = propertyValueToString(property.value());
String fieldValue = (String) originQuery.userpropValue(field);
if (this.matchSearchIndexWords(propValue, fieldValue)) {
continue;
}
return false;
Expand Down Expand Up @@ -1310,7 +1329,20 @@ private static boolean validQueryConditionValues(HugeGraph graph,
E.checkState(!values.isEmpty(),
"Expect user property values for key '%s', " +
"but got none", pk);
boolean hasContains = query.containsContainsCondition(key);
if (pk.cardinality().multiple()) {
// If contains collection index, relation should be contains
E.checkState(hasContains,
"The relation of property '%s' must be " +
"CONTAINS or TEXT_CONTAINS, but got %s",
pk.name(), query.relation(key).relation());
}

for (Object value : values) {
if (hasContains) {
value = toCollectionIfNeeded(pk, value);
}

if (!pk.checkValueType(value)) {
return false;
}
Expand All @@ -1319,6 +1351,39 @@ private static boolean validQueryConditionValues(HugeGraph graph,
return true;
}

private static Object toCollectionIfNeeded(PropertyKey pk, Object value) {
switch (pk.cardinality()) {
case SET:
if (!(value instanceof Set)) {
jadepeng marked this conversation as resolved.
Show resolved Hide resolved
value = CollectionUtil.toSet(value);
}
break;
case LIST:
if (!(value instanceof List)) {
value = CollectionUtil.toList(value);
}
break;
default:
break;
}
return value;
jadepeng marked this conversation as resolved.
Show resolved Hide resolved
}

private static boolean isCollectionIndex(List<Object> propValues) {
return propValues.size() == 1 &&
propValues.get(0) instanceof Collection;
}

private static String propertyValueToString(Object value) {
/*
* Join collection items with white space if the value is Collection,
* or else keep the origin value.
*/
return value instanceof Collection ?
StringUtils.join(((Collection<Object>) value).toArray(), " ") :
value.toString();
}

private static String escapeIndexValueIfNeeded(String value) {
for (int i = 0; i < value.length(); i++) {
char ch = value.charAt(i);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,6 @@ public void indexField(Id id) {
}

public Id indexField() {
E.checkState(this.indexType.isRange() || this.indexType.isSearch(),
"Can't call indexField() for %s index label",
this.indexType.string());
E.checkState(this.indexFields.size() == 1,
"There should be only one field in %s index label, " +
"but got: %s", this.indexType.string(), this.indexFields);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -466,10 +466,13 @@ private void checkFields(Set<Id> propertyIds) {
E.checkArgument(pkey.aggregateType().isIndexable(),
"The aggregate type %s is not indexable",
pkey.aggregateType());
E.checkArgument(pkey.cardinality().single(),
"Not allowed to build index on property key " +
"'%s' whose cardinality is list or set",
pkey.name());

if (pkey.cardinality().multiple()) {
E.checkArgument(fields.size() == 1,
"Not allowed to build union index on property" +
" key '%s' whose cardinality is multiple",
pkey.name());
}
}

List<String> properties = this.graph().mapPkId2Name(propertyIds);
Expand Down Expand Up @@ -506,6 +509,10 @@ private void checkFields4Range() {
"one field, but got %s fields: '%s'",
fields.size(), fields);
String field = fields.iterator().next();
PropertyKey property = this.graph().propertyKey(field);
E.checkArgument(!property.cardinality().multiple(),
"Not allowed to build range index on property " +
"'%s' whose cardinality is multiple", field);
DataType dataType = this.graph().propertyKey(field).dataType();
E.checkArgument(dataType.isNumber() || dataType.isDate(),
"Range index can only build on numeric or " +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
import com.baidu.hugegraph.testutil.FakeObjects.FakeEdge;
import com.baidu.hugegraph.testutil.Utils;
import com.baidu.hugegraph.testutil.Whitebox;
import com.baidu.hugegraph.traversal.optimize.ConditionP;
import com.baidu.hugegraph.traversal.optimize.Text;
import com.baidu.hugegraph.traversal.optimize.TraversalUtil;
import com.baidu.hugegraph.type.HugeType;
Expand Down Expand Up @@ -7055,6 +7056,103 @@ public void testQueryByHasIdEmptyListInPage() {
Assert.assertNull(page);
}

@Test
public void testAddEdgeWithCollectionIndex() {
jadepeng marked this conversation as resolved.
Show resolved Hide resolved
SchemaManager schema = graph().schema();
schema.propertyKey("tags").asText().valueSet().create();
schema.propertyKey("category").asText().valueSet().create();
schema.propertyKey("country").asText().create();

schema.vertexLabel("soft")
.properties("name", "tags", "country", "category")
.primaryKeys("name").create();

schema.edgeLabel("related")
.sourceLabel("soft")
.targetLabel("soft")
.properties("tags").create();

schema.indexLabel("edgeByTag").onE("related").secondary()
.by("tags")
.create();

HugeGraph graph = graph();

Vertex huge = graph.addVertex(
T.label, "soft",
"name", "hugegraph",
"country", "china",
"category", ImmutableList.of("graphdb", "db"),
"tags", ImmutableList.of("graphdb", "gremlin"));

Vertex neo4j = graph.addVertex(
T.label, "soft", "name", "neo4j",
"country", "usa",
"category", ImmutableList.of("graphdb", "db"),
"tags", ImmutableList.of("graphdb", "cypher"));

Vertex janus = graph.addVertex(
T.label, "soft", "name", "janusgraph",
"country", "usa",
"category", ImmutableList.of("graphdb", "db"),
"tags", ImmutableList.of("graphdb", "gremlin"));

huge.addEdge("related", neo4j, "tags", ImmutableList.of("graphdb"));

Edge huge2janus = huge.addEdge("related", janus, "tags",
ImmutableList.of("graphdb", "gremlin"));

graph.tx().commit();

Assert.assertThrows(IllegalStateException.class, () -> {
graph.traversal().E().has("related", "tags",
"gremlin").toList();
});


List<Edge> edges = graph.traversal().E()
.has("related","tags",
ConditionP.contains("gremlin"))
.toList();

Assert.assertEquals(1, edges.size());

edges = graph.traversal().E()
.has("related",
"tags", ConditionP.contains("graphdb"))
.toList();

Assert.assertEquals(2, edges.size());

// append a new tag
huge2janus.property("tags", ImmutableList.of("newTag"));
graph.tx().commit();

edges = graph.traversal().E()
.has("related",
"tags", ConditionP.contains("newTag"))
.toList();

Assert.assertEquals(1, edges.size());

edges = graph.traversal().E()
.has("related",
"tags", ConditionP.contains("graphdb"))
.toList();

Assert.assertEquals(2, edges.size());

// remove a edge
edges.get(0).remove();
graph.tx().commit();

edges = graph.traversal().E()
.has("related",
"tags", ConditionP.contains("graphdb"))
.toList();
Assert.assertEquals(1, edges.size());
}

private void init18Edges() {
this.init18Edges(true);
}
Expand Down
Loading