Skip to content

Commit 7b8a4aa

Browse files
shirly121shirly121
authored
fix(interactive): Fix Bugs When Parsing Label From Path ElementMap Results (#4477)
<!-- Thanks for your contribution! please review https://github.com/alibaba/GraphScope/blob/main/CONTRIBUTING.md before opening an issue. --> ## What do these changes do? as titled. <!-- Please give a short brief about these changes. --> ## Related issue number <!-- Are there any issues opened that will be resolved by merging this change? --> Fixes
1 parent 764e31e commit 7b8a4aa

File tree

6 files changed

+269
-20
lines changed

6 files changed

+269
-20
lines changed

‎interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/tools/GraphRexBuilder.java‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package com.alibaba.graphscope.common.ir.tools;
1818

1919
import com.alibaba.graphscope.common.ir.rex.RexGraphDynamicParam;
20+
import com.alibaba.graphscope.common.ir.rex.RexGraphVariable;
2021
import com.google.common.collect.Lists;
2122

2223
import org.apache.calcite.rel.type.RelDataType;
@@ -115,4 +116,20 @@ private Object invoke(Method method, Object... args) {
115116
throw new RuntimeException(e);
116117
}
117118
}
119+
120+
@Override
121+
public RexNode makeCast(RelDataType type, RexNode operand) {
122+
if (operand instanceof RexGraphVariable) {
123+
RexGraphVariable var = (RexGraphVariable) operand;
124+
return var.getProperty() == null
125+
? RexGraphVariable.of(var.getAliasId(), var.getIndex(), var.getName(), type)
126+
: RexGraphVariable.of(
127+
var.getAliasId(),
128+
var.getProperty(),
129+
var.getIndex(),
130+
var.getName(),
131+
type);
132+
}
133+
return super.makeCast(type, operand);
134+
}
118135
}

‎interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/type/InterleavedVELabelTypes.java‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
/*
2+
*
3+
* * Copyright 2020 Alibaba Group Holding Limited.
4+
* *
5+
* * Licensed under the Apache License, Version 2.0 (the "License");
6+
* * you may not use this file except in compliance with the License.
7+
* * You may obtain a copy of the License at
8+
* *
9+
* * http://www.apache.org/licenses/LICENSE-2.0
10+
* *
11+
* * Unless required by applicable law or agreed to in writing, software
12+
* * distributed under the License is distributed on an "AS IS" BASIS,
13+
* * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* * See the License for the specific language governing permissions and
15+
* * limitations under the License.
16+
*
17+
*/
18+
19+
package com.alibaba.graphscope.common.ir.type;
20+
21+
import com.google.common.base.Objects;
22+
import com.google.common.base.Preconditions;
23+
import com.google.common.collect.ImmutableList;
24+
25+
import java.util.Iterator;
26+
import java.util.List;
27+
28+
/**
29+
* Maintain path element types in an interleaved way.
30+
*/
31+
public class InterleavedVELabelTypes extends GraphLabelType implements Iterator<GraphLabelType> {
32+
private final List<GraphLabelType> unionTypes;
33+
private int cursor;
34+
35+
public InterleavedVELabelTypes(GraphLabelType expandLabelType, GraphLabelType getVLabelType) {
36+
super(expandLabelType.getLabelsEntry());
37+
Preconditions.checkArgument(
38+
!(expandLabelType instanceof InterleavedVELabelTypes)
39+
&& !(getVLabelType instanceof InterleavedVELabelTypes));
40+
this.unionTypes = ImmutableList.of(expandLabelType, getVLabelType);
41+
this.cursor = 0;
42+
}
43+
44+
@Override
45+
public boolean equals(Object o) {
46+
if (this == o) return true;
47+
if (o == null || getClass() != o.getClass()) return false;
48+
if (!super.equals(o)) return false;
49+
InterleavedVELabelTypes that = (InterleavedVELabelTypes) o;
50+
return Objects.equal(unionTypes, that.unionTypes);
51+
}
52+
53+
@Override
54+
public int hashCode() {
55+
return Objects.hashCode(super.hashCode(), unionTypes);
56+
}
57+
58+
@Override
59+
protected void generateTypeString(StringBuilder sb, boolean withDetail) {
60+
sb.append("UNION_V_E(" + unionTypes + ")");
61+
}
62+
63+
@Override
64+
public boolean hasNext() {
65+
return true;
66+
}
67+
68+
@Override
69+
public GraphLabelType next() {
70+
cursor = 1 - cursor;
71+
return unionTypes.get(cursor);
72+
}
73+
74+
public void resetCursor() {
75+
cursor = 0;
76+
}
77+
}

‎interactive_engine/compiler/src/main/java/com/alibaba/graphscope/gremlin/antlr4x/visitor/ExpressionVisitor.java‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,8 +156,14 @@ private List<String> getProperties(List<String> ctxProperties, @Nullable String
156156
if (throwsOnPropertyNotFound || ctxProperties.isEmpty()) {
157157
return ctxProperties.isEmpty() ? getAllProperties(tag) : ctxProperties;
158158
}
159-
ctxProperties.retainAll(getAllProperties(tag));
160-
return ctxProperties;
159+
List<String> candidates = getAllProperties(tag);
160+
return ctxProperties.stream()
161+
.filter(
162+
k ->
163+
k.equals(T.label.getAccessor())
164+
|| k.equals(T.id.getAccessor())
165+
|| candidates.contains(k))
166+
.collect(Collectors.toList());
161167
}
162168

163169
private List<String> getElementMapProperties(List<String> properties) {

‎interactive_engine/compiler/src/main/java/com/alibaba/graphscope/gremlin/antlr4x/visitor/PathFunctionVisitor.java‎

Lines changed: 103 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@
2828
import com.alibaba.graphscope.common.ir.tools.GraphBuilder;
2929
import com.alibaba.graphscope.common.ir.tools.GraphStdOperatorTable;
3030
import com.alibaba.graphscope.common.ir.tools.config.GraphOpt;
31+
import com.alibaba.graphscope.common.ir.type.GraphLabelType;
3132
import com.alibaba.graphscope.common.ir.type.GraphPathType;
33+
import com.alibaba.graphscope.common.ir.type.InterleavedVELabelTypes;
3234
import com.alibaba.graphscope.grammar.GremlinGSBaseVisitor;
3335
import com.alibaba.graphscope.grammar.GremlinGSParser;
3436
import com.google.common.base.Preconditions;
@@ -42,15 +44,21 @@
4244
import org.apache.calcite.rel.type.RelDataTypeField;
4345
import org.apache.calcite.rex.RexCall;
4446
import org.apache.calcite.rex.RexNode;
47+
import org.apache.calcite.sql.SqlKind;
48+
import org.apache.calcite.util.Pair;
49+
import org.slf4j.Logger;
50+
import org.slf4j.LoggerFactory;
4551

52+
import java.util.Collections;
53+
import java.util.Comparator;
4654
import java.util.List;
4755
import java.util.Objects;
48-
import java.util.stream.Collectors;
4956

5057
/**
5158
* visit the antlr tree and generate the corresponding {@code RexNode} for path function
5259
*/
5360
public class PathFunctionVisitor extends GremlinGSBaseVisitor<RexNode> {
61+
private static final Logger logger = LoggerFactory.getLogger(PathFunctionVisitor.class);
5462
private final GraphBuilder parentBuilder;
5563
private final RexNode variable;
5664
private final GraphLogicalPathExpand pathExpand;
@@ -290,18 +298,82 @@ private RexNode unionProjection(
290298
+ getVProjection
291299
+ "]");
292300
if (expandProjection instanceof RexCall) {
293-
RexCall expandCall = (RexCall) expandProjection;
294-
RexCall getVCall = (RexCall) getVProjection;
295-
List<RexNode> newOperands = Lists.newArrayList(expandCall.getOperands());
296-
newOperands.addAll(getVCall.getOperands());
297-
Preconditions.checkArgument(
298-
!newOperands.isEmpty(),
299-
"invalid query given properties, not found in path expand");
300-
return builder.call(
301-
expandCall.getOperator(),
302-
newOperands.stream().distinct().collect(Collectors.toList()));
301+
switch (expandProjection.getKind()) {
302+
case MAP_VALUE_CONSTRUCTOR:
303+
List<KeyValuePair> expandKeyValues =
304+
convert(((RexCall) expandProjection).getOperands());
305+
List<KeyValuePair> getVKeyValues =
306+
convert(((RexCall) getVProjection).getOperands());
307+
Collections.sort(
308+
expandKeyValues, Comparator.comparing(pair -> pair.left.toString()));
309+
Collections.sort(
310+
getVKeyValues, Comparator.comparing(pair -> pair.left.toString()));
311+
List<RexNode> unionOperands = Lists.newArrayList();
312+
for (int i = 0, j = 0;
313+
i < expandKeyValues.size() || j < getVKeyValues.size(); ) {
314+
int compareTo;
315+
if (j == getVKeyValues.size()) {
316+
compareTo = -1;
317+
} else if (i == expandKeyValues.size()) {
318+
compareTo = 1;
319+
} else {
320+
String expandKey = expandKeyValues.get(i).getKey().toString();
321+
String getVKey = getVKeyValues.get(j).getKey().toString();
322+
compareTo = expandKey.compareTo(getVKey);
323+
}
324+
if (compareTo == 0) {
325+
addKeyValuePair(
326+
unionOperands,
327+
new KeyValuePair(
328+
expandKeyValues.get(i).getKey(),
329+
unionProjection(
330+
expandKeyValues.get(i).getValue(),
331+
getVKeyValues.get(j).getValue(),
332+
builder)));
333+
++i;
334+
++j;
335+
} else if (compareTo < 0) {
336+
addKeyValuePair(unionOperands, expandKeyValues.get(i++));
337+
} else {
338+
addKeyValuePair(unionOperands, getVKeyValues.get(j++));
339+
}
340+
}
341+
Preconditions.checkArgument(
342+
!unionOperands.isEmpty(),
343+
"invalid query given properties, not found in path expand");
344+
return builder.call(((RexCall) expandProjection).getOperator(), unionOperands);
345+
default:
346+
}
347+
}
348+
return unionTypes(expandProjection, getVProjection);
349+
}
350+
351+
private RexNode unionTypes(RexNode rex1, RexNode rex2) {
352+
if (!rex1.toString().equals(rex2.toString())) {
353+
logger.error(
354+
"cannot union {} and {}, they have different digests, return {} as the union"
355+
+ " results instead",
356+
rex1,
357+
rex2,
358+
rex1);
359+
return rex1;
360+
}
361+
if (rex1.getType().equals(rex2.getType())) return rex1;
362+
GraphLabelType unionType =
363+
new InterleavedVELabelTypes(
364+
(GraphLabelType) rex1.getType(), (GraphLabelType) rex2.getType());
365+
return resetType(rex1, unionType);
366+
}
367+
368+
private RexNode resetType(RexNode rex, RelDataType type) {
369+
RexNode rexWithType = parentBuilder.getRexBuilder().makeCast(type, rex);
370+
if (rexWithType.getKind() == SqlKind.CAST) {
371+
logger.error(
372+
"cannot reset type by 'CAST' operator, return {} as the reset results instead",
373+
rex);
374+
return rex;
303375
}
304-
return expandProjection;
376+
return rexWithType;
305377
}
306378

307379
private GraphOpt.PathExpandFunction getFuncOpt() {
@@ -314,4 +386,23 @@ private GraphOpt.PathExpandFunction getFuncOpt() {
314386
return GraphOpt.PathExpandFunction.VERTEX_EDGE;
315387
}
316388
}
389+
390+
private static class KeyValuePair extends Pair<RexNode, RexNode> {
391+
public KeyValuePair(RexNode left, RexNode right) {
392+
super(left, right);
393+
}
394+
}
395+
396+
public List<KeyValuePair> convert(List<RexNode> operands) {
397+
List<KeyValuePair> keyValuePairs = Lists.newArrayList();
398+
for (int i = 0; i < operands.size(); i += 2) {
399+
keyValuePairs.add(new KeyValuePair(operands.get(i), operands.get(i + 1)));
400+
}
401+
return keyValuePairs;
402+
}
403+
404+
public void addKeyValuePair(List<RexNode> operands, KeyValuePair keyValuePair) {
405+
operands.add(keyValuePair.left);
406+
operands.add(keyValuePair.right);
407+
}
317408
}

‎interactive_engine/compiler/src/main/java/com/alibaba/graphscope/gremlin/resultx/GremlinRecordParser.java‎

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,7 @@
1616

1717
package com.alibaba.graphscope.gremlin.resultx;
1818

19-
import com.alibaba.graphscope.common.ir.type.ArbitraryArrayType;
20-
import com.alibaba.graphscope.common.ir.type.ArbitraryMapType;
21-
import com.alibaba.graphscope.common.ir.type.GraphLabelType;
22-
import com.alibaba.graphscope.common.ir.type.GraphPathType;
19+
import com.alibaba.graphscope.common.ir.type.*;
2320
import com.alibaba.graphscope.common.result.RecordParser;
2421
import com.alibaba.graphscope.common.result.Utils;
2522
import com.alibaba.graphscope.gaia.proto.Common;
@@ -33,7 +30,9 @@
3330
import org.apache.calcite.rel.type.RelDataType;
3431
import org.apache.calcite.rel.type.RelDataTypeField;
3532
import org.apache.calcite.rex.RexNode;
33+
import org.apache.calcite.sql.type.ArraySqlType;
3634
import org.apache.calcite.sql.type.MapSqlType;
35+
import org.apache.calcite.sql.type.MultisetSqlType;
3736
import org.apache.commons.lang3.NotImplementedException;
3837
import org.apache.tinkerpop.gremlin.structure.Edge;
3938
import org.apache.tinkerpop.gremlin.structure.Element;
@@ -57,6 +56,7 @@ public GremlinRecordParser(ResultSchema resultSchema) {
5756
@Override
5857
public List<Object> parseFrom(IrResult.Record record) {
5958
RelDataType outputType = resultSchema.outputType;
59+
resetTypeCursor(outputType);
6060
Preconditions.checkArgument(
6161
record.getColumnsCount() == outputType.getFieldCount(),
6262
"column size of results "
@@ -261,7 +261,9 @@ private List<Element> parseGraphPath(IrResult.GraphPath path, @Nullable RelDataT
261261
}
262262

263263
protected @Nullable Object parseValue(Common.Value value, @Nullable RelDataType dataType) {
264-
if (dataType instanceof GraphLabelType) {
264+
if (dataType instanceof InterleavedVELabelTypes) {
265+
return parseValue(value, ((InterleavedVELabelTypes) dataType).next());
266+
} else if (dataType instanceof GraphLabelType) {
265267
return Utils.parseLabelValue(value, (GraphLabelType) dataType);
266268
}
267269
switch (value.getItemCase()) {
@@ -322,4 +324,28 @@ private List<Element> parseGraphPath(IrResult.GraphPath path, @Nullable RelDataT
322324
throw new NotImplementedException(value.getItemCase() + " is unsupported yet");
323325
}
324326
}
327+
328+
private void resetTypeCursor(RelDataType type) {
329+
if (type.isStruct()) {
330+
type.getFieldList().forEach(k -> resetTypeCursor(k.getType()));
331+
} else if (type instanceof ArraySqlType || type instanceof MultisetSqlType) {
332+
resetTypeCursor(type.getComponentType());
333+
} else if (type instanceof MapSqlType) {
334+
resetTypeCursor(type.getKeyType());
335+
resetTypeCursor(type.getValueType());
336+
} else if (type instanceof ArbitraryArrayType) {
337+
((ArbitraryArrayType) type).getComponentTypes().forEach(this::resetTypeCursor);
338+
} else if (type instanceof ArbitraryMapType) {
339+
((ArbitraryMapType) type)
340+
.getKeyValueTypeMap()
341+
.values()
342+
.forEach(
343+
k -> {
344+
resetTypeCursor(k.getKey());
345+
resetTypeCursor(k.getValue());
346+
});
347+
} else if (type instanceof InterleavedVELabelTypes) {
348+
((InterleavedVELabelTypes) type).resetCursor();
349+
}
350+
}
325351
}

‎interactive_engine/compiler/src/test/java/com/alibaba/graphscope/gremlin/antlr4x/GraphBuilderTest.java‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1802,7 +1802,7 @@ public void g_V_path_as_a_select_a_valueMap() {
18021802
Assert.assertEquals(
18031803
"GraphLogicalProject($f0=[$f0], isAppend=[false])\n"
18041804
+ " GraphLogicalProject($f0=[PATH_FUNCTION(a, FLAG(VERTEX_EDGE),"
1805-
+ " MAP(_UTF-8'weight', a.weight, _UTF-8'name', a.name))], isAppend=[true])\n"
1805+
+ " MAP(_UTF-8'name', a.name, _UTF-8'weight', a.weight))], isAppend=[true])\n"
18061806
+ " GraphLogicalPathExpand(expand=[GraphLogicalExpand(tableConfig=[{isAll=true,"
18071807
+ " tables=[created, knows]}], alias=[_], opt=[OUT])\n"
18081808
+ "], getV=[GraphLogicalGetV(tableConfig=[{isAll=true, tables=[software,"
@@ -1886,4 +1886,36 @@ public void union_valueMap_test() {
18861886
RexGraphVariable var = (RexGraphVariable) expr;
18871887
Assert.assertEquals(2, var.getAliasId());
18881888
}
1889+
1890+
@Test
1891+
public void g_V_match_a_out_b_path_elementMap() {
1892+
GraphBuilder builder = Utils.mockGraphBuilder(optimizer, irMeta);
1893+
RelNode rel =
1894+
eval(
1895+
"g.V().match(as('a').out('3..4', 'knows').with('RESULT_OPT',"
1896+
+ " 'ALL_V_E').as('b').endV().as('c')).select('b').by(elementMap())",
1897+
builder);
1898+
RelNode after = optimizer.optimize(rel, new GraphIOProcessor(builder, irMeta));
1899+
Assert.assertEquals(
1900+
"RecordType(({_UTF-8'name'=<CHAR(4), VARCHAR>, _UTF-8'id'=<CHAR(2), BIGINT>,"
1901+
+ " _UTF-8'weight'=<CHAR(6), DOUBLE>, _UTF-8'age'=<CHAR(3), INTEGER>,"
1902+
+ " _UTF-8'~label'=<CHAR(6), UNION_V_E([[EdgeLabel(knows, person, person)],"
1903+
+ " [VertexLabel(person)]])>, _UTF-8'~id'=<CHAR(3), BIGINT>}) MAP ARRAY $f0)",
1904+
after.getRowType().toString());
1905+
}
1906+
1907+
@Test
1908+
public void g_V_match_a_out_b_path_label() {
1909+
GraphBuilder builder = Utils.mockGraphBuilder(optimizer, irMeta);
1910+
RelNode rel =
1911+
eval(
1912+
"g.V().match(as('a').out('3..4', 'knows').with('RESULT_OPT',"
1913+
+ " 'ALL_V_E').as('b').endV().as('c')).select('b').by('~label')",
1914+
builder);
1915+
RelNode after = optimizer.optimize(rel, new GraphIOProcessor(builder, irMeta));
1916+
Assert.assertEquals(
1917+
"RecordType(UNION_V_E([[EdgeLabel(knows, person, person)], [VertexLabel(person)]])"
1918+
+ " ARRAY $f0)",
1919+
after.getRowType().toString());
1920+
}
18891921
}

0 commit comments

Comments
 (0)