Skip to content

Commit 0b08016

Browse files
authored
fix(interactive): Including primary keys when sinking vertex (#4436)
Fix #4433
1 parent fd66790 commit 0b08016

File tree

6 files changed

+84
-6
lines changed

6 files changed

+84
-6
lines changed

flex/engines/graph_db/runtime/common/rt_any.cc

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -724,8 +724,19 @@ void sink_vertex(const GraphReadInterface& graph, const VertexRecord& vertex,
724724
results::Vertex* v) {
725725
v->mutable_label()->set_id(vertex.label_);
726726
v->set_id(encode_unique_vertex_id(vertex.label_, vertex.vid_));
727-
// TODO: add properties
728727
const auto& names = graph.schema().get_vertex_property_names(vertex.label_);
728+
// Add primary keys first, since primary keys are also the properties of a
729+
// vertex
730+
auto& primary_key = graph.schema().get_vertex_primary_key(vertex.label_);
731+
if (primary_key.size() > 1) {
732+
LOG(ERROR) << "Currently only support single primary key";
733+
}
734+
auto pk_name = std::get<1>(primary_key[0]);
735+
auto pk_prop = v->add_properties();
736+
pk_prop->mutable_key()->set_name(pk_name);
737+
sink_any(graph.GetVertexId(vertex.label_, vertex.vid_),
738+
pk_prop->mutable_value());
739+
729740
for (size_t i = 0; i < names.size(); ++i) {
730741
auto prop = v->add_properties();
731742
prop->mutable_key()->set_name(names[i]);

flex/interactive/sdk/python/gs_interactive/tests/test_robustness.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -411,13 +411,20 @@ def test_x_csr_params(
411411
start_service_on_graph(
412412
interactive_session, create_graph_algo_graph_with_x_csr_params
413413
)
414+
ensure_compiler_schema_ready(
415+
interactive_session, neo4j_session, create_graph_algo_graph_with_x_csr_params
416+
)
414417
result = neo4j_session.run('MATCH (n) where n.id <> "" return count(n);')
415418
# expect return value 0
416419
records = result.fetch(1)
417420
print(records[0])
418421
assert len(records) == 1 and records[0]["$f0"] == 3506
419422

420423

424+
@pytest.mark.skipif(
425+
os.environ.get("RUN_ON_PROTO", None) != "ON",
426+
reason="var_char is only supported in proto",
427+
)
421428
def test_var_char_property(
422429
interactive_session, neo4j_session, create_graph_with_var_char_property
423430
):
@@ -426,6 +433,9 @@ def test_var_char_property(
426433
interactive_session, create_graph_with_var_char_property
427434
)
428435
start_service_on_graph(interactive_session, create_graph_with_var_char_property)
436+
ensure_compiler_schema_ready(
437+
interactive_session, neo4j_session, create_graph_with_var_char_property
438+
)
429439
result = neo4j_session.run("MATCH (n: person) return n.name AS personName;")
430440
records = result.fetch(10)
431441
assert len(records) == 4

flex/tests/hqps/hqps_robust_test.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ run_robust_test(){
106106
run_additional_robust_test(){
107107
pushd ${FLEX_HOME}/interactive/sdk/python/gs_interactive
108108
export RUN_ON_PROTO=ON
109-
cmd="python3 -m pytest -s tests/test_robustness.py -k test_call_proc_in_cypher"
109+
cmd='python3 -m pytest -s tests/test_robustness.py -k "test_call_proc_in_cypher or test_var_char_property"'
110110
echo "Run additional robust test with command: ${cmd}"
111111
eval ${cmd} || (err "Run additional robust test failed"; exit 1)
112112
info "Run additional robust test success"

interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/ir/meta/fetcher/DynamicIrMetaFetcher.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import com.alibaba.graphscope.common.ir.meta.IrMetaStats;
2727
import com.alibaba.graphscope.common.ir.meta.IrMetaTracker;
2828
import com.alibaba.graphscope.common.ir.meta.reader.IrMetaReader;
29-
import com.alibaba.graphscope.common.ir.meta.schema.SchemaSpec.Type;
3029
import com.alibaba.graphscope.groot.common.schema.api.GraphStatistics;
3130

3231
import org.slf4j.Logger;
@@ -79,9 +78,6 @@ public Optional<IrMeta> fetch() {
7978
private synchronized void syncMeta() {
8079
try {
8180
IrMeta meta = this.reader.readMeta();
82-
logger.debug(
83-
"schema from remote: {}",
84-
(meta == null) ? null : meta.getSchema().getSchemaSpec(Type.IR_CORE_IN_JSON));
8581
// if the graph id or schema version is changed, we need to update the statistics
8682
if (this.currentState == null
8783
|| !this.currentState.getGraphId().equals(meta.getGraphId())

interactive_engine/compiler/src/main/java/com/alibaba/graphscope/cypher/integration/suite/simple/SimpleMatchQueries.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,4 +230,21 @@ public static QueryContext get_simple_match_query_18_test() {
230230
List<String> expected = Arrays.asList("Record<{$f0: 39783}>");
231231
return new QueryContext(query, expected);
232232
}
233+
234+
public static QueryContext get_simple_match_query_19_test() {
235+
String query = "MATCH(a: PLACE) return a ORDER BY a.id limit 5;";
236+
List<String> expected =
237+
Arrays.asList(
238+
"[a: {name=India, id=0, type=country,"
239+
+ " url=http://dbpedia.org/resource/India}]",
240+
"[a: {name=China, id=1, type=country,"
241+
+ " url=http://dbpedia.org/resource/China}]",
242+
"[a: {name=Angola, id=2, type=country,"
243+
+ " url=http://dbpedia.org/resource/Angola}]",
244+
"[a: {name=Austria, id=3, type=country,"
245+
+ " url=http://dbpedia.org/resource/Austria}]",
246+
"[a: {name=Azerbaijan, id=4, type=country,"
247+
+ " url=http://dbpedia.org/resource/Azerbaijan}]");
248+
return new QueryContext(query, expected);
249+
}
233250
}

interactive_engine/compiler/src/test/java/com/alibaba/graphscope/cypher/integration/ldbc/SimpleMatchTest.java

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,23 @@
2525
import org.junit.BeforeClass;
2626
import org.junit.Test;
2727
import org.neo4j.driver.GraphDatabase;
28+
import org.neo4j.driver.Record;
2829
import org.neo4j.driver.Result;
2930
import org.neo4j.driver.Session;
31+
import org.neo4j.driver.Value;
32+
import org.neo4j.driver.internal.types.InternalTypeSystem;
33+
import org.neo4j.driver.types.Node;
34+
import org.slf4j.Logger;
35+
import org.slf4j.LoggerFactory;
36+
37+
import java.util.ArrayList;
38+
import java.util.List;
39+
import java.util.Map;
3040

3141
public class SimpleMatchTest {
3242

43+
private static final Logger logger = LoggerFactory.getLogger(SimpleMatchTest.class);
44+
3345
private static Session session;
3446

3547
@BeforeClass
@@ -169,6 +181,38 @@ public void run_simple_match_18_test() {
169181
Assert.assertEquals(testQuery.getExpectedResult().toString(), result.list().toString());
170182
}
171183

184+
@Test
185+
public void run_simple_match_19_test() {
186+
assumeTrue("hiactor".equals(System.getenv("ENGINE_TYPE")));
187+
QueryContext testQuery = SimpleMatchQueries.get_simple_match_query_19_test();
188+
Result result = session.run(testQuery.getQuery());
189+
List<Record> records = result.list();
190+
List<String> properties = new ArrayList<>();
191+
records.forEach(
192+
record -> {
193+
properties.add(fetchAllProperties(record));
194+
});
195+
logger.info(properties.toString());
196+
Assert.assertEquals(testQuery.getExpectedResult().toString(), properties.toString());
197+
}
198+
199+
private static String fetchAllProperties(Record record) {
200+
List<String> properties = new ArrayList<>();
201+
record.keys()
202+
.forEach(
203+
key -> {
204+
Value v = record.get(key);
205+
if (v.hasType(InternalTypeSystem.TYPE_SYSTEM.NODE())) {
206+
Node node = v.asNode();
207+
Map<String, Object> nodeProperties = node.asMap();
208+
properties.add(key + ": " + nodeProperties.toString());
209+
} else {
210+
properties.add(key + ": " + record.get(key).toString());
211+
}
212+
});
213+
return properties.toString();
214+
}
215+
172216
@AfterClass
173217
public static void afterClass() {
174218
if (session != null) {

0 commit comments

Comments
 (0)