feat(query):Support filter in match and support save db. by ColinLeeo · Pull Request #145 · TuGraph-family/miniGU

ColinLeeo · 2026-03-10T15:51:28Z

Title

Type

Scope

Description

Issue: #

Checklist

I have prepared the pull request title according to the requirements.
I have successfully run all unit tests and integration tests.
I have already rebased the latest master branch.
I have commented my code, particularly in hard-to-understand areas.
I have made corresponding changes to the documentation.

Copilot

Pull request overview

This PR extends MiniGU’s query engine to support MATCH ... WHERE ... predicates (including binary and property expressions) and adds initial on-disk database persistence by saving/loading a lightweight catalog plus file-backed graph data.

Changes:

Add WHERE predicate planning for MATCH by introducing a physical Filter over the generated scan/expand plan, and fix multi-hop expand input column indexing.
Extend expression binding/execution to support binary expressions and property access, including executor-time injection of vertex property scans when needed for predicate evaluation.
Implement on-disk database open/load behavior, plus catalog persistence (catalog.json) and file-backed graph storage for import/test-graph procedures; add persistence E2E tests and snapshots.

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
minigu/gql/planner/src/plan/scan.rs	Attach `var_labels` to node scan schema for downstream property/type resolution.
minigu/gql/planner/src/plan/expand.rs	Propagate `var_labels` through expand schema and set labels for target vertex vars.
minigu/gql/planner/src/optimizer/mod.rs	Allow `MATCH` predicate and emit `PhysicalFilter`; fix multi-hop expand input column indexing.
minigu/gql/planner/src/bound/value_expr.rs	Add bound expression kinds for `Binary` and `Property`, plus constructors.
minigu/gql/planner/src/binder/value_expr.rs	Bind binary expressions and property access to bound expressions.
minigu/gql/planner/src/binder/common.rs	Simplify vertex predicate handling (still not supported at vertex pattern level).
minigu/gql/execution/src/builder.rs	Build executors with “actual schema” tracking; inject property scans for filters/projects; add evaluator support for new bound expr kinds.
minigu/core/src/procedures/import_graph.rs	Import graphs into file-backed storage when `db_path` is set; persist catalog.
minigu/core/src/procedures/export_graph.rs	Update test callsite for new `import_internal` signature.
minigu/core/src/procedures/create_test_graph_data.rs	Create test graphs using file-backed storage and persist catalog when on-disk.
minigu/core/src/lib.rs	Export new `catalog_persistence` module.
minigu/core/src/error.rs	Add IO/JSON/storage error variants for persistence flows.
minigu/core/src/database.rs	Implement `Database::open` to create/load on-disk DB, restore graphs from catalog.
minigu/core/src/catalog_persistence.rs	New module: serialize/deserialize graph type catalog and derive graph data file paths.
minigu/core/src/procedures/mod.rs	Minor module header change.
minigu-test/src/insta_test.rs	Add new `match_filter` tests and persistence reopen E2E tests.
minigu-test/gql/**	Add new snapshots and `.gql` test cases for filtering and persistence.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-10T15:59:46Z

+                                        for property in vertex_type.properties().iter() {
+                                            property_ids.push(property.0);
+                                            property_names
+                                                .push(property.1.name().to_string());
+                                        }


This scans all properties for each referenced vertex variable (iterating vertex_type.properties()), even if the predicate only touches one property. That can be a significant overhead for wide schemas. Consider collecting the specific (source, property) accesses from the predicate and scanning only the required property IDs.

Copilot · 2026-03-10T15:59:46Z

                            let mut new_fields = updated_schema.fields().to_vec();
                            for prop_name in property_names.iter() {
                                let qualified_name = format!("{}_{}", var_name, prop_name);


In this property-scan insertion path, updated_schema is later rebuilt from new_fields only (dropping var_labels from the existing schema). Since build_evaluator relies on schema.get_var_label(variable) for VertexConstructor, this can lead to incorrect vertex _label values after a scan is inserted. Preserve/copy var_labels when reconstructing the schema (similar to how the planner propagates var_labels in Expand).

Copilot · 2026-03-10T15:59:47Z

+            Expr::Binary { op, left, right } => {
+                let bound_left = self.bind_value_expression(left.value())?;
+                let bound_right = self.bind_value_expression(right.value())?;
+                let bound_op = bind_binary_op(op.value());
+                Ok(BoundExpr::binary(bound_op, bound_left, bound_right))
+            }


Expr::Binary now binds every BinaryOp via bind_binary_op, but the execution layer still panics for some ops (e.g., Concat/Xor are unimplemented!() in the evaluator). To avoid runtime panics, either reject unsupported binary ops here with a proper NotImplemented bind error, or add full evaluator support for the newly-bindable ops.

Copilot · 2026-03-10T15:59:47Z

+
+/// Get the `.minigu` file path for a graph.
+pub fn graph_data_path(db_path: &Path, graph_name: &str) -> std::path::PathBuf {
+    db_path.join(format!("{}.minigu", graph_name))


graph_data_path uses graph_name directly to form a filename. If graph_name can contain path separators or .., this allows writing/reading outside db_path (path traversal) when creating/importing graphs. Validate/sanitize graph_name (e.g., allow only [A-Za-z0-9_\-]), or encode it (hash/escape) before building the path.

Suggested change

db_path.join(format!("{}.minigu", graph_name))

// Sanitize graph_name to prevent path traversal / invalid filenames.

// Allow only ASCII alphanumeric characters, '_' and '-'; replace others with '_'.

let sanitized_name: String = graph_name

.chars()

.map(|c| {

if c.is_ascii_alphanumeric() || c == '_' || c == '-' {

c

} else {

'_'

}

})

.collect();

db_path.join(format!("{}.minigu", sanitized_name))

Copilot · 2026-03-10T15:59:47Z

+        let label_id = graph_type
+            .add_label(label_def.name.clone())
+            .expect("add label failed");
+        debug_assert_eq!(label_id.get(), label_def.id, "label ID mismatch on restore");


Restoring label IDs relies on add_label assigning sequential IDs and only checks mismatches with debug_assert_eq!, which is compiled out in release builds. If label IDs are non-contiguous (e.g., labels were removed) or otherwise differ, this can silently restore an incorrect mapping in production. Prefer validating and returning an error on mismatch (or add an API to insert labels with explicit IDs).

Suggested change

debug_assert_eq!(label_id.get(), label_def.id, "label ID mismatch on restore");

assert_eq!(label_id.get(), label_def.id, "label ID mismatch on restore");

Copilot · 2026-03-10T15:59:48Z

@@ -1,3 +1,4 @@
+


File starts with an extra blank line. This is likely accidental and can create unnecessary diffs in future edits; consider removing the leading empty line to match the style of other modules.

Suggested change

Copilot · 2026-03-10T15:59:48Z

+                                    true,
+                                ));
+                            }
+                            updated_schema = Arc::new(DataSchema::new(new_fields));


When extending updated_schema with property columns, this rebuilds the DataSchema from fields only, which drops any var_labels metadata from the child schema. That causes downstream VertexConstructor to lose label specs (schema.get_var_label(...) becomes None) and can produce incorrect _label values. Preserve/copy var_labels into the new schema (or mutate a cloned DataSchema instead of recreating it).

Suggested change

updated_schema = Arc::new(DataSchema::new(new_fields));

// Preserve existing var_labels metadata when extending the schema

let var_labels = updated_schema.var_labels().clone();

let mut new_schema = DataSchema::new(new_fields);

new_schema.set_var_labels(var_labels);

updated_schema = Arc::new(new_schema);

Copilot · 2026-03-10T15:59:48Z

+                                new_fields.push(DataField::new(
+                                    qualified_name,
+                                    LogicalType::String,
+                                    true,
+                                ));


Property columns added for WHERE evaluation are declared as LogicalType::String regardless of the actual property type. This makes the executor-time schema inconsistent with the data returned by scan_vertex_property and can break any later logic that depends on schema types. Prefer using the real logical type from the catalog (or derive it from the scanned Arrow array types) when creating these DataFields.

qishipengqsp · 2026-04-02T12:19:50Z

Conflicts to be solved

qishipengqsp

Test case reviewed. LGTM

Copilot AI review requested due to automatic review settings March 10, 2026 15:51

Copilot started reviewing on behalf of ColinLeeo March 10, 2026 15:52 View session

Copilot AI reviewed Mar 10, 2026

View reviewed changes

ColinLeeo force-pushed the support_filter_save_db branch from 5f12d39 to b29bc4a Compare March 10, 2026 16:12

support match and filter.

72f6855

ColinLeeo force-pushed the support_filter_save_db branch from da88d0f to 72f6855 Compare March 10, 2026 16:34

qishipengqsp approved these changes Apr 15, 2026

View reviewed changes

Navigation Menu

Uh oh!

feat(query):Support filter in match and support save db. - #145

feat(query):Support filter in match and support save db.#145
ColinLeeo wants to merge 1 commit into
masterfrom
support_filter_save_db

ColinLeeo commented Mar 10, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

qishipengqsp commented Apr 2, 2026

Uh oh!

qishipengqsp left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-    db_path.join(format!("{}.minigu", graph_name))
+    // Sanitize graph_name to prevent path traversal / invalid filenames.
+    // Allow only ASCII alphanumeric characters, '_' and '-'; replace others with '_'.
+    let sanitized_name: String = graph_name
+        .chars()
+        .map(|c| {
+            if c.is_ascii_alphanumeric() || c == '_' || c == '-' {
+                c
+            } else {
+                '_'
+            }
+        })
+        .collect();
+    db_path.join(format!("{}.minigu", sanitized_name))

	debug_assert_eq!(label_id.get(), label_def.id, "label ID mismatch on restore");
	assert_eq!(label_id.get(), label_def.id, "label ID mismatch on restore");

-                            updated_schema = Arc::new(DataSchema::new(new_fields));
+                            // Preserve existing var_labels metadata when extending the schema
+                            let var_labels = updated_schema.var_labels().clone();
+                            let mut new_schema = DataSchema::new(new_fields);
+                            new_schema.set_var_labels(var_labels);
+                            updated_schema = Arc::new(new_schema);

Uh oh!

Conversation

ColinLeeo commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Title

Description

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

qishipengqsp commented Apr 2, 2026

Uh oh!

qishipengqsp left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ColinLeeo commented Mar 10, 2026 •

edited

Loading